Fix --keyid flag scope and implement secret move command
- Restrict --keyid flag to PGP unlocker type only - Add validation to prevent --keyid usage with non-PGP unlockers - Implement 'secret move' command with 'mv' and 'rename' aliases - Add comprehensive tests for move functionality - Update documentation to reflect optional nature of --keyid for PGP The move command allows renaming or moving secrets within a vault while preserving all versions and metadata. It fails if the destination already exists to prevent accidental overwrites.
This commit is contained in:
parent
a73a409fe4
commit
a6f24e9581
@ -106,6 +106,11 @@ Lists all secrets in the current vault. Optional filter for substring matching.
|
|||||||
- **NO RECOVERY**: Once removed, the secret cannot be recovered
|
- **NO RECOVERY**: Once removed, the secret cannot be recovered
|
||||||
- **ALL VERSIONS DELETED**: Every version of the secret will be permanently deleted
|
- **ALL VERSIONS DELETED**: Every version of the secret will be permanently deleted
|
||||||
|
|
||||||
|
#### `secret move <source> <destination>` / `secret mv` / `secret rename`
|
||||||
|
Moves or renames a secret within the current vault.
|
||||||
|
- Fails if the destination already exists
|
||||||
|
- Preserves all versions and metadata
|
||||||
|
|
||||||
### Version Management
|
### Version Management
|
||||||
|
|
||||||
#### `secret version list <secret-name>` / `secret version ls`
|
#### `secret version list <secret-name>` / `secret version ls`
|
||||||
@ -144,7 +149,7 @@ Creates a new unlocker of the specified type:
|
|||||||
- `keychain`: macOS Keychain integration (macOS only)
|
- `keychain`: macOS Keychain integration (macOS only)
|
||||||
|
|
||||||
**Options:**
|
**Options:**
|
||||||
- `--keyid <id>`: GPG key ID (required for PGP type)
|
- `--keyid <id>`: GPG key ID (optional for PGP type, uses default key if not specified)
|
||||||
|
|
||||||
#### `secret unlocker remove <unlocker-id> [--force]` / `secret unlocker rm` ⚠️ 🛑
|
#### `secret unlocker remove <unlocker-id> [--force]` / `secret unlocker rm` ⚠️ 🛑
|
||||||
**DANGER**: Permanently removes an unlocker. Like Unix `rm`, this command does not ask for confirmation.
|
**DANGER**: Permanently removes an unlocker. Like Unix `rm`, this command does not ask for confirmation.
|
||||||
@ -439,3 +444,4 @@ Released as a free software gift to the world, no strings attached, under the [W
|
|||||||
Contact: [sneak@sneak.berlin](mailto:sneak@sneak.berlin)
|
Contact: [sneak@sneak.berlin](mailto:sneak@sneak.berlin)
|
||||||
|
|
||||||
[https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2](https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2)
|
[https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2](https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2)
|
||||||
|
|
||||||
|
@ -189,6 +189,12 @@ func TestSecretManagerIntegration(t *testing.T) {
|
|||||||
// Expected: Proper filesystem encoding (/ -> %)
|
// Expected: Proper filesystem encoding (/ -> %)
|
||||||
test12SecretNameFormats(t, tempDir, testMnemonic, runSecretWithEnv, runSecretWithStdin)
|
test12SecretNameFormats(t, tempDir, testMnemonic, runSecretWithEnv, runSecretWithStdin)
|
||||||
|
|
||||||
|
// Test 12b: Move/rename secrets
|
||||||
|
// Commands: secret move, secret mv, secret rename
|
||||||
|
// Purpose: Test moving and renaming secrets
|
||||||
|
// Expected: Secret moved to new location, old location removed
|
||||||
|
test12bMoveSecret(t, testMnemonic, runSecret, runSecretWithStdin)
|
||||||
|
|
||||||
// Test 13: Unlocker management
|
// Test 13: Unlocker management
|
||||||
// Commands: secret unlocker list, secret unlocker add pgp
|
// Commands: secret unlocker list, secret unlocker add pgp
|
||||||
// Purpose: Test multiple unlocker types
|
// Purpose: Test multiple unlocker types
|
||||||
@ -1091,6 +1097,80 @@ func test12SecretNameFormats(t *testing.T, tempDir, testMnemonic string, runSecr
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func test12bMoveSecret(t *testing.T, testMnemonic string, runSecret func(...string) (string, error), runSecretWithStdin func(string, map[string]string, ...string) (string, error)) {
|
||||||
|
// First, create a secret to move
|
||||||
|
_, err := runSecretWithStdin("original-value", map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "add", "test/original")
|
||||||
|
require.NoError(t, err, "add test/original should succeed")
|
||||||
|
|
||||||
|
// Test move command
|
||||||
|
output, err := runSecret("move", "test/original", "test/renamed")
|
||||||
|
require.NoError(t, err, "move should succeed")
|
||||||
|
assert.Contains(t, output, "Moved secret 'test/original' to 'test/renamed'", "should show move confirmation")
|
||||||
|
|
||||||
|
// Need to create a runSecretWithEnv for get operations
|
||||||
|
runSecretWithEnv := func(env map[string]string, args ...string) (string, error) {
|
||||||
|
return cli.ExecuteCommandInProcess(args, "", env)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify original doesn't exist
|
||||||
|
_, err = runSecretWithEnv(map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "get", "test/original")
|
||||||
|
assert.Error(t, err, "get original should fail after move")
|
||||||
|
|
||||||
|
// Verify new location exists and has correct value
|
||||||
|
getOutput, err := runSecretWithEnv(map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "get", "test/renamed")
|
||||||
|
require.NoError(t, err, "get renamed should succeed")
|
||||||
|
assert.Equal(t, "original-value", getOutput, "renamed secret should have original value")
|
||||||
|
|
||||||
|
// Test mv alias
|
||||||
|
_, err = runSecretWithStdin("another-value", map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "add", "test/another")
|
||||||
|
require.NoError(t, err, "add test/another should succeed")
|
||||||
|
|
||||||
|
output, err = runSecret("mv", "test/another", "test/moved-with-mv")
|
||||||
|
require.NoError(t, err, "mv alias should work")
|
||||||
|
assert.Contains(t, output, "Moved secret", "should show move confirmation")
|
||||||
|
|
||||||
|
// Test rename alias
|
||||||
|
_, err = runSecretWithStdin("rename-test-value", map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "add", "test/rename-me")
|
||||||
|
require.NoError(t, err, "add test/rename-me should succeed")
|
||||||
|
|
||||||
|
output, err = runSecret("rename", "test/rename-me", "test/renamed-with-alias")
|
||||||
|
require.NoError(t, err, "rename alias should work")
|
||||||
|
assert.Contains(t, output, "Moved secret", "should show move confirmation")
|
||||||
|
|
||||||
|
// Test error cases
|
||||||
|
// Try to move non-existent secret
|
||||||
|
output, err = runSecret("move", "test/nonexistent", "test/destination")
|
||||||
|
assert.Error(t, err, "move non-existent should fail")
|
||||||
|
assert.Contains(t, output, "not found", "should indicate source not found")
|
||||||
|
|
||||||
|
// Try to move to existing destination
|
||||||
|
_, err = runSecretWithStdin("dest-value", map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "add", "test/existing-dest")
|
||||||
|
require.NoError(t, err, "add test/existing-dest should succeed")
|
||||||
|
|
||||||
|
output, err = runSecret("move", "test/renamed", "test/existing-dest")
|
||||||
|
assert.Error(t, err, "move to existing destination should fail")
|
||||||
|
assert.Contains(t, output, "already exists", "should indicate destination exists")
|
||||||
|
|
||||||
|
// Verify the source wasn't removed since move failed
|
||||||
|
getOutput, err = runSecretWithEnv(map[string]string{
|
||||||
|
"SB_SECRET_MNEMONIC": testMnemonic,
|
||||||
|
}, "get", "test/renamed")
|
||||||
|
require.NoError(t, err, "get source should still work after failed move")
|
||||||
|
assert.Equal(t, "original-value", getOutput, "source should still have original value")
|
||||||
|
}
|
||||||
|
|
||||||
func test13UnlockerManagement(t *testing.T, tempDir, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error)) {
|
func test13UnlockerManagement(t *testing.T, tempDir, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error)) {
|
||||||
// Make sure we're in default vault
|
// Make sure we're in default vault
|
||||||
_, err := runSecret("vault", "select", "default")
|
_, err := runSecret("vault", "select", "default")
|
||||||
|
@ -35,6 +35,7 @@ func newRootCmd() *cobra.Command {
|
|||||||
cmd.AddCommand(newGetCmd())
|
cmd.AddCommand(newGetCmd())
|
||||||
cmd.AddCommand(newListCmd())
|
cmd.AddCommand(newListCmd())
|
||||||
cmd.AddCommand(newRemoveCmd())
|
cmd.AddCommand(newRemoveCmd())
|
||||||
|
cmd.AddCommand(newMoveCmd())
|
||||||
cmd.AddCommand(newUnlockerCmd())
|
cmd.AddCommand(newUnlockerCmd())
|
||||||
cmd.AddCommand(newImportCmd())
|
cmd.AddCommand(newImportCmd())
|
||||||
cmd.AddCommand(newEncryptCmd())
|
cmd.AddCommand(newEncryptCmd())
|
||||||
|
@ -125,6 +125,24 @@ func newRemoveCmd() *cobra.Command {
|
|||||||
return cmd
|
return cmd
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func newMoveCmd() *cobra.Command {
|
||||||
|
cmd := &cobra.Command{
|
||||||
|
Use: "move <source> <destination>",
|
||||||
|
Aliases: []string{"mv", "rename"},
|
||||||
|
Short: "Move or rename a secret",
|
||||||
|
Long: `Move or rename a secret within the current vault. ` +
|
||||||
|
`If the destination already exists, the operation will fail.`,
|
||||||
|
Args: cobra.ExactArgs(2), //nolint:mnd // Command requires exactly 2 arguments: source and destination
|
||||||
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
|
cli := NewCLIInstance()
|
||||||
|
|
||||||
|
return cli.MoveSecret(cmd, args[0], args[1])
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
return cmd
|
||||||
|
}
|
||||||
|
|
||||||
// updateBufferSize updates the buffer size based on usage pattern
|
// updateBufferSize updates the buffer size based on usage pattern
|
||||||
func updateBufferSize(currentSize int, sameSize *int) int {
|
func updateBufferSize(currentSize int, sameSize *int) int {
|
||||||
*sameSize++
|
*sameSize++
|
||||||
@ -517,3 +535,51 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MoveSecret moves or renames a secret
|
||||||
|
func (cli *Instance) MoveSecret(cmd *cobra.Command, sourceName, destName string) error {
|
||||||
|
// Get current vault
|
||||||
|
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get vault directory
|
||||||
|
vaultDir, err := currentVlt.GetDirectory()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if source exists
|
||||||
|
sourceEncoded := strings.ReplaceAll(sourceName, "/", "%")
|
||||||
|
sourceDir := filepath.Join(vaultDir, "secrets.d", sourceEncoded)
|
||||||
|
|
||||||
|
exists, err := afero.DirExists(cli.fs, sourceDir)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check if source secret exists: %w", err)
|
||||||
|
}
|
||||||
|
if !exists {
|
||||||
|
return fmt.Errorf("secret '%s' not found", sourceName)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if destination already exists
|
||||||
|
destEncoded := strings.ReplaceAll(destName, "/", "%")
|
||||||
|
destDir := filepath.Join(vaultDir, "secrets.d", destEncoded)
|
||||||
|
|
||||||
|
exists, err = afero.DirExists(cli.fs, destDir)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check if destination secret exists: %w", err)
|
||||||
|
}
|
||||||
|
if exists {
|
||||||
|
return fmt.Errorf("secret '%s' already exists", destName)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Perform the move
|
||||||
|
if err := cli.fs.Rename(sourceDir, destDir); err != nil {
|
||||||
|
return fmt.Errorf("failed to move secret: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cmd.Printf("Moved secret '%s' to '%s'\n", sourceName, destName)
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
@ -99,24 +99,27 @@ func newUnlockerAddCmd() *cobra.Command {
|
|||||||
}
|
}
|
||||||
|
|
||||||
cmd := &cobra.Command{
|
cmd := &cobra.Command{
|
||||||
Use: "add <type> [keyid]",
|
Use: "add <type>",
|
||||||
Short: "Add a new unlocker",
|
Short: "Add a new unlocker",
|
||||||
Long: fmt.Sprintf(`Add a new unlocker of the specified type (%s).`, supportedTypes),
|
Long: fmt.Sprintf(`Add a new unlocker of the specified type (%s).
|
||||||
Args: cobra.RangeArgs(1, 2), //nolint:mnd // Command accepts 1 or 2 arguments
|
|
||||||
|
For PGP unlockers, you can optionally specify a GPG key ID with --keyid.
|
||||||
|
If not specified, the default GPG key will be used.`, supportedTypes),
|
||||||
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
cli := NewCLIInstance()
|
cli := NewCLIInstance()
|
||||||
|
unlockerType := args[0]
|
||||||
|
|
||||||
// For PGP type, check if keyid is provided as positional argument
|
// Check if --keyid was used with non-PGP type
|
||||||
if args[0] == "pgp" && len(args) == 2 {
|
if unlockerType != "pgp" && cmd.Flags().Changed("keyid") {
|
||||||
// Override any flag value with the positional argument
|
return fmt.Errorf("--keyid flag is only valid for PGP unlockers")
|
||||||
_ = cmd.Flags().Set("keyid", args[1])
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return cli.UnlockersAdd(args[0], cmd)
|
return cli.UnlockersAdd(unlockerType, cmd)
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
cmd.Flags().String("keyid", "", "GPG key ID for PGP unlockers")
|
cmd.Flags().String("keyid", "", "GPG key ID for PGP unlockers (optional, uses default key if not specified)")
|
||||||
|
|
||||||
return cmd
|
return cmd
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user