store keystore-relative names in wallets.key_path

handler/wallet/export_keys.go

@@ -148,22 +148,19 @@ func exportOneKey(db *gorm.DB, ks keystore.KeyStore, actor legacyActorRow) (expo
 		return false, fmt.Sprintf("actor %s: db query failed: %v", actor.ID, err)
 	}
 
-	// save key to keystore
-	keyPath, _, err := ks.Put(actor.PrivateKey)
+	keyName, _, err := ks.Put(actor.PrivateKey)
 	if err != nil {
 		return false, fmt.Sprintf("actor %s: keystore write failed: %v", actor.ID, err)
 	}
 
-	// create wallet record
 	w := model.Wallet{
-		KeyPath:  keyPath,
+		KeyPath:  keyName,
 		KeyStore: "local",
 		Address:  addr.String(),
 		ActorID:  &actor.ID,
 	}
 	if err := db.Create(&w).Error; err != nil {
-		// cleanup keystore file on db failure
-		ks.Delete(keyPath)
+		ks.Delete(keyName)
 		return false, fmt.Sprintf("actor %s: wallet create failed: %v", actor.ID, err)
 	}
 

handler/wallet/export_keys_test.go

@@ -195,7 +195,7 @@ func TestExportKeysHandler_MissingKeyFile(t *testing.T) {
 
 		// create wallet record pointing to a nonexistent key file
 		require.NoError(t, db.Create(&model.Wallet{
-			KeyPath:  "/nonexistent/key",
+			KeyPath:  "nonexistent",
 			KeyStore: "local",
 			Address:  testutil.TestWalletAddr,
 		}).Error)
@@ -218,12 +218,11 @@ func TestExportKeysHandler_CorruptKeyFile(t *testing.T) {
 		addLegacyColumn(t, db)
 		createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
 
-		// write garbage to the key file path
-		corruptPath := dir + "/corrupt"
-		require.NoError(t, os.WriteFile(corruptPath, []byte("garbage"), 0600))
+		// write garbage as a key file inside the keystore dir
+		require.NoError(t, os.WriteFile(dir+"/corrupt", []byte("garbage"), 0600))
 
 		require.NoError(t, db.Create(&model.Wallet{
-			KeyPath:  corruptPath,
+			KeyPath:  "corrupt",
 			KeyStore: "local",
 			Address:  testutil.TestWalletAddr,
 		}).Error)

handler/wallet/import_keystore.go

@@ -49,16 +49,16 @@ func (DefaultHandler) ImportKeystoreHandler(
 		return nil, errors.Wrap(handlererror.ErrInvalidParameter, err.Error())
 	}
 
-	keyPath, _, err := ks.Put(request.PrivateKey)
+	keyName, _, err := ks.Put(request.PrivateKey)
 	if err != nil {
 		logger.Errorw("failed to save key to keystore", "err", err)
 		return nil, errors.WithStack(err)
 	}
 
-	logger.Infow("saved key to keystore", "address", addr.String(), "path", keyPath)
+	logger.Infow("saved key to keystore", "address", addr.String(), "name", keyName)
 
 	walletRecord := model.Wallet{
-		KeyPath:  keyPath,
+		KeyPath:  keyName,
 		KeyStore: "local",
 		Address:  addr.String(),
 		Name:     request.Name,
@@ -70,12 +70,12 @@ func (DefaultHandler) ImportKeystoreHandler(
 	})
 
 	if util.IsDuplicateKeyError(err) {
-		// don't delete the key file — it belongs to the existing wallet record
+		// don't delete the key file -- it belongs to the existing wallet record
 		return nil, errors.Wrap(handlererror.ErrDuplicateRecord, "wallet already imported")
 	}
 
 	if err != nil {
-		ks.Delete(keyPath) // cleanup only for non-duplicate failures
+		ks.Delete(keyName) // cleanup only for non-duplicate failures
 		return nil, errors.WithStack(err)
 	}
 

model/wallet.go

@@ -5,7 +5,7 @@ package model
 type Wallet struct {
 	ID uint `gorm:"primaryKey" json:"id"`
 
-	KeyPath  string `gorm:"uniqueIndex;not null" json:"keyPath"`      // absolute path to key file
+	KeyPath  string `gorm:"uniqueIndex;not null" json:"keyPath"`      // keystore-relative name (typically the address)
 	KeyStore string `gorm:"default:'local';not null" json:"keyStore"` // local, yubikey, aws-kms, etc
 	Address  string `gorm:"index;not null" json:"address"`            // filecoin address (f1.../f3...)
 	Name     string `json:"name,omitempty"`                           // optional label

util/keystore/keystore.go

@@ -4,22 +4,26 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/data-preservation-programs/go-synapse/signer"
 	"github.com/filecoin-project/go-address"
 )
 
+// KeyStore stores keys by a short name (typically the filecoin address).
+// The name is a relative identifier -- implementations resolve it against
+// their own storage root. Callers must pass back the name returned by Put.
 type KeyStore interface {
-	Put(privateKey string) (path string, addr address.Address, err error) // saves key, returns path and address
-	Get(path string) (privateKey string, err error)                       // loads key from path
-	List() ([]KeyInfo, error)                                             // lists all keys
-	Delete(path string) error                                             // removes key
-	Has(path string) bool                                                 // checks if key exists
+	Put(privateKey string) (name string, addr address.Address, err error)
+	Get(name string) (privateKey string, err error)
+	List() ([]KeyInfo, error)
+	Delete(name string) error
+	Has(name string) bool
 }
 
 type KeyInfo struct {
 	Address address.Address
-	Path    string
+	Name    string // relative identifier within the keystore
 }
 
 // filesystem keystore implementation
@@ -34,26 +38,39 @@ func NewLocalKeyStore(dir string) (*LocalKeyStore, error) {
 	return &LocalKeyStore{dir: dir}, nil
 }
 
+// reject names that would escape the keystore dir or address another directory
+func validateName(name string) error {
+	if name == "" {
+		return fmt.Errorf("empty key name")
+	}
+	if strings.ContainsRune(name, os.PathSeparator) || name == "." || name == ".." {
+		return fmt.Errorf("invalid key name %q: must be a basename", name)
+	}
+	return nil
+}
+
 // lotus wallet export format expected (hex-encoded JSON with Type and PrivateKey)
 func (ks *LocalKeyStore) Put(privateKey string) (string, address.Address, error) {
 	addr, err := AddressFromExport(privateKey)
 	if err != nil {
 		return "", address.Undef, fmt.Errorf("failed to derive address from private key: %w", err)
 	}
 
-	// file named by address (f1.../f3...)
-	filename := addr.String()
-	path := filepath.Join(ks.dir, filename)
+	name := addr.String()
+	path := filepath.Join(ks.dir, name)
 
 	if err := os.WriteFile(path, []byte(privateKey), 0600); err != nil {
 		return "", address.Undef, fmt.Errorf("failed to write key file: %w", err)
 	}
 
-	return path, addr, nil
+	return name, addr, nil
 }
 
-func (ks *LocalKeyStore) Get(path string) (string, error) {
-	data, err := os.ReadFile(path)
+func (ks *LocalKeyStore) Get(name string) (string, error) {
+	if err := validateName(name); err != nil {
+		return "", err
+	}
+	data, err := os.ReadFile(filepath.Join(ks.dir, name))
 	if err != nil {
 		return "", fmt.Errorf("failed to read key file: %w", err)
 	}
@@ -72,8 +89,8 @@ func (ks *LocalKeyStore) List() ([]KeyInfo, error) {
 			continue
 		}
 
-		path := filepath.Join(ks.dir, entry.Name())
-		data, err := os.ReadFile(path)
+		name := entry.Name()
+		data, err := os.ReadFile(filepath.Join(ks.dir, name))
 		if err != nil {
 			continue // skip unreadable
 		}
@@ -85,22 +102,28 @@ func (ks *LocalKeyStore) List() ([]KeyInfo, error) {
 
 		keys = append(keys, KeyInfo{
 			Address: addr,
-			Path:    path,
+			Name:    name,
 		})
 	}
 
 	return keys, nil
 }
 
-func (ks *LocalKeyStore) Delete(path string) error {
-	if err := os.Remove(path); err != nil {
+func (ks *LocalKeyStore) Delete(name string) error {
+	if err := validateName(name); err != nil {
+		return err
+	}
+	if err := os.Remove(filepath.Join(ks.dir, name)); err != nil {
 		return fmt.Errorf("failed to delete key file: %w", err)
 	}
 	return nil
 }
 
-func (ks *LocalKeyStore) Has(path string) bool {
-	_, err := os.Stat(path)
+func (ks *LocalKeyStore) Has(name string) bool {
+	if err := validateName(name); err != nil {
+		return false
+	}
+	_, err := os.Stat(filepath.Join(ks.dir, name))
 	return err == nil
 }
 

util/keystore/keystore_test.go

@@ -32,23 +32,14 @@ func TestLocalKeyStore_PutAndGet(t *testing.T) {
 	// Use test key
 	privateKey := getTestKey(0)
 
-	// Put the key
-	path, addr, err := ks.Put(privateKey)
+	name, addr, err := ks.Put(privateKey)
 	require.NoError(t, err)
-	require.NotEmpty(t, path)
 	require.NotEqual(t, address.Undef, addr)
 
-	// Verify file exists
-	require.FileExists(t, path)
+	require.Equal(t, addr.String(), name)
+	require.FileExists(t, filepath.Join(tmpdir, name))
 
-	// Verify path is in the keystore directory
-	require.Contains(t, path, tmpdir)
-
-	// Verify filename matches address
-	require.Equal(t, filepath.Join(tmpdir, addr.String()), path)
-
-	// Get the key back
-	loadedKey, err := ks.Get(path)
+	loadedKey, err := ks.Get(name)
 	require.NoError(t, err)
 	require.Equal(t, privateKey, loadedKey)
 }
@@ -66,42 +57,33 @@ func TestLocalKeyStore_List(t *testing.T) {
 	// Add a key (we only have one unique test key, so add it once)
 	key1 := getTestKey(0)
 
-	path1, addr1, err := ks.Put(key1)
+	name1, addr1, err := ks.Put(key1)
 	require.NoError(t, err)
 
-	// List should return it
 	keys, err = ks.List()
 	require.NoError(t, err)
 	require.Len(t, keys, 1)
 
-	// Verify address matches
 	require.Equal(t, addr1, keys[0].Address)
-
-	// Verify path is correct
-	require.Equal(t, path1, keys[0].Path)
+	require.Equal(t, name1, keys[0].Name)
 }
 
 func TestLocalKeyStore_Delete(t *testing.T) {
 	tmpdir := t.TempDir()
 	ks, err := NewLocalKeyStore(tmpdir)
 	require.NoError(t, err)
 
-	// Add a key
 	privateKey := getTestKey(0)
-	path, _, err := ks.Put(privateKey)
+	name, _, err := ks.Put(privateKey)
 	require.NoError(t, err)
 
-	// Verify it exists
-	require.True(t, ks.Has(path))
+	require.True(t, ks.Has(name))
 
-	// Delete it
-	err = ks.Delete(path)
+	err = ks.Delete(name)
 	require.NoError(t, err)
 
-	// Verify it's gone
-	require.False(t, ks.Has(path))
+	require.False(t, ks.Has(name))
 
-	// List should be empty
 	keys, err := ks.List()
 	require.NoError(t, err)
 	require.Empty(t, keys)
@@ -112,16 +94,17 @@ func TestLocalKeyStore_Has(t *testing.T) {
 	ks, err := NewLocalKeyStore(tmpdir)
 	require.NoError(t, err)
 
-	// Non-existent key
-	require.False(t, ks.Has(filepath.Join(tmpdir, "nonexistent")))
+	require.False(t, ks.Has("nonexistent"))
 
-	// Add a key
 	privateKey := getTestKey(0)
-	path, _, err := ks.Put(privateKey)
+	name, _, err := ks.Put(privateKey)
 	require.NoError(t, err)
 
-	// Should exist
-	require.True(t, ks.Has(path))
+	require.True(t, ks.Has(name))
+
+	// paths with separators are rejected
+	require.False(t, ks.Has(filepath.Join(tmpdir, name)))
+	require.False(t, ks.Has("../escape"))
 }
 
 func TestLocalKeyStore_InvalidKey(t *testing.T) {
@@ -166,20 +149,17 @@ func TestLocalKeyStore_PutSameKeyTwice(t *testing.T) {
 	ks, err := NewLocalKeyStore(tmpdir)
 	require.NoError(t, err)
 
-	// Add a key
 	privateKey := getTestKey(0)
-	path1, addr1, err := ks.Put(privateKey)
+	name1, addr1, err := ks.Put(privateKey)
 	require.NoError(t, err)
 
-	// Add the same key again (should overwrite)
-	path2, addr2, err := ks.Put(privateKey)
+	// same key again should overwrite
+	name2, addr2, err := ks.Put(privateKey)
 	require.NoError(t, err)
 
-	// Paths and addresses should be the same
-	require.Equal(t, path1, path2)
+	require.Equal(t, name1, name2)
 	require.Equal(t, addr1, addr2)
 
-	// Should only have one key in the list
 	keys, err := ks.List()
 	require.NoError(t, err)
 	require.Len(t, keys, 1)