Add cross-vault move command for secrets

Implement syntax: secret move/mv <vault>:<secret> <vault>[:<secret>] - Copies all versions to destination vault with re-encryption - Deletes source after successful copy (true move) - Add --force flag to overwrite existing destination - Support both within-vault rename and cross-vault move - Add shell completion for vault:secret syntax - Include integration tests for cross-vault move
2025-12-23 15:24:13 +07:00
parent 7264026d66
commit 128c53a11d
5 changed files with 559 additions and 30 deletions
--- a/internal/cli/completions.go
+++ b/internal/cli/completions.go
@@ -142,3 +142,58 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
 		return completions, cobra.ShellCompDirectiveNoFileComp
 	}
 }
+
+// getVaultSecretCompletionFunc returns a completion function for vault:secret format
+// It completes vault names with ":" suffix, and after ":" it completes secrets from that vault
+func getVaultSecretCompletionFunc(fs afero.Fs, stateDir string) func(
+	cmd *cobra.Command, args []string, toComplete string,
+) ([]string, cobra.ShellCompDirective) {
+	return func(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		var completions []string
+
+		// Check if we're completing after a vault: prefix
+		if strings.Contains(toComplete, ":") {
+			// Complete secret names for the specified vault
+			const vaultSecretParts = 2
+			parts := strings.SplitN(toComplete, ":", vaultSecretParts)
+			vaultName := parts[0]
+			secretPrefix := parts[1]
+
+			vlt := vault.NewVault(fs, stateDir, vaultName)
+			secrets, err := vlt.ListSecrets()
+			if err == nil {
+				for _, secretName := range secrets {
+					if strings.HasPrefix(secretName, secretPrefix) {
+						completions = append(completions, vaultName+":"+secretName)
+					}
+				}
+			}
+
+			return completions, cobra.ShellCompDirectiveNoFileComp
+		}
+
+		// Complete vault names with ":" suffix
+		vaults, err := vault.ListVaults(fs, stateDir)
+		if err == nil {
+			for _, v := range vaults {
+				if strings.HasPrefix(v, toComplete) {
+					completions = append(completions, v+":")
+				}
+			}
+		}
+
+		// Also complete secrets from current vault (for within-vault moves)
+		if currentVlt, err := vault.GetCurrentVault(fs, stateDir); err == nil {
+			secrets, err := currentVlt.ListSecrets()
+			if err == nil {
+				for _, secretName := range secrets {
+					if strings.HasPrefix(secretName, toComplete) {
+						completions = append(completions, secretName)
+					}
+				}
+			}
+		}
+
+		return completions, cobra.ShellCompDirectiveNoSpace
+	}
+}
--- a/internal/cli/integration_test.go
+++ b/internal/cli/integration_test.go
@@ -200,6 +200,12 @@ func TestSecretManagerIntegration(t *testing.T) {
 	// Expected: Secret moved to new location, old location removed
 	test12bMoveSecret(t, testMnemonic, runSecret, runSecretWithStdin)

+	// Test 12c: Cross-vault move
+	// Commands: secret move work:secret default, secret move work:secret default:newname
+	// Purpose: Test moving secrets between vaults with re-encryption
+	// Expected: Secret copied to destination vault with all versions, source deleted
+	test12cCrossVaultMove(t, testMnemonic, runSecretWithEnv, runSecretWithStdin)
+
 	// Test 13: Unlocker management
 	// Commands: secret unlocker list, secret unlocker add pgp
 	// Purpose: Test multiple unlocker types
@@ -1164,6 +1170,89 @@ func test12bMoveSecret(t *testing.T, testMnemonic string, runSecret func(...stri
 	assert.Equal(t, "original-value", getOutput, "source should still have original value")
 }

+func test12cCrossVaultMove(t *testing.T, testMnemonic string, runSecretWithEnv func(map[string]string, ...string) (string, error), runSecretWithStdin func(string, map[string]string, ...string) (string, error)) {
+	env := map[string]string{
+		"SB_SECRET_MNEMONIC": testMnemonic,
+	}
+
+	// Create a test secret in the work vault
+	_, err := runSecretWithEnv(env, "vault", "select", "work")
+	require.NoError(t, err, "select work vault should succeed")
+
+	// Add a secret with a version
+	_, err = runSecretWithStdin("cross-vault-value-v1", env, "add", "cross/move/test")
+	require.NoError(t, err, "add cross/move/test should succeed")
+
+	// Add another version
+	_, err = runSecretWithStdin("cross-vault-value-v2", env, "add", "--force", "cross/move/test")
+	require.NoError(t, err, "add cross/move/test v2 should succeed")
+
+	// Move to default vault using cross-vault syntax
+	output, err := runSecretWithEnv(env, "move", "work:cross/move/test", "default")
+	require.NoError(t, err, "cross-vault move should succeed")
+	assert.Contains(t, output, "Moved secret", "should show move confirmation")
+	assert.Contains(t, output, "2 version(s)", "should show version count")
+
+	// Verify secret exists in default vault
+	_, err = runSecretWithEnv(env, "vault", "select", "default")
+	require.NoError(t, err, "select default vault should succeed")
+
+	value, err := runSecretWithEnv(env, "get", "cross/move/test")
+	require.NoError(t, err, "get from default vault should succeed")
+	assert.Equal(t, "cross-vault-value-v2", value, "should have latest version value")
+
+	// Verify secret no longer exists in work vault
+	_, err = runSecretWithEnv(env, "vault", "select", "work")
+	require.NoError(t, err, "select work vault should succeed")
+
+	_, err = runSecretWithEnv(env, "get", "cross/move/test")
+	assert.Error(t, err, "get from work vault should fail after move")
+
+	// Test cross-vault move with rename
+	_, err = runSecretWithStdin("rename-test", env, "add", "rename/source")
+	require.NoError(t, err, "add rename/source should succeed")
+
+	output, err = runSecretWithEnv(env, "move", "work:rename/source", "default:renamed/dest")
+	require.NoError(t, err, "cross-vault move with rename should succeed")
+	assert.Contains(t, output, "Moved secret", "should show move confirmation")
+
+	// Verify renamed secret exists in default vault
+	_, err = runSecretWithEnv(env, "vault", "select", "default")
+	require.NoError(t, err, "select default vault should succeed")
+
+	value, err = runSecretWithEnv(env, "get", "renamed/dest")
+	require.NoError(t, err, "get renamed secret should succeed")
+	assert.Equal(t, "rename-test", value, "should have correct value")
+
+	// Test --force flag for overwriting
+	_, err = runSecretWithStdin("existing-secret", env, "add", "force/test")
+	require.NoError(t, err, "add force/test in default should succeed")
+
+	_, err = runSecretWithEnv(env, "vault", "select", "work")
+	require.NoError(t, err, "select work vault should succeed")
+
+	_, err = runSecretWithStdin("new-value", env, "add", "force/test")
+	require.NoError(t, err, "add force/test in work should succeed")
+
+	// Move without force should fail
+	output, err = runSecretWithEnv(env, "move", "work:force/test", "default")
+	assert.Error(t, err, "move without force should fail when dest exists")
+	assert.Contains(t, output, "already exists", "should indicate destination exists")
+
+	// Move with force should succeed
+	output, err = runSecretWithEnv(env, "move", "--force", "work:force/test", "default")
+	require.NoError(t, err, "move with force should succeed")
+	assert.Contains(t, output, "Moved secret", "should show move confirmation")
+
+	// Verify value was overwritten
+	_, err = runSecretWithEnv(env, "vault", "select", "default")
+	require.NoError(t, err, "select default vault should succeed")
+
+	value, err = runSecretWithEnv(env, "get", "force/test")
+	require.NoError(t, err, "get overwritten secret should succeed")
+	assert.Equal(t, "new-value", value, "should have new value after force move")
+}
+
 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
 	_, err := runSecret("vault", "select", "default")
--- a/internal/cli/secrets.go
+++ b/internal/cli/secrets.go
@@ -14,6 +14,25 @@ import (
 	"github.com/spf13/cobra"
 )

+const (
+	// vaultSecretSeparator is the delimiter between vault name and secret name
+	vaultSecretSeparator = ":"
+	// vaultSecretParts is the number of parts when splitting vault:secret
+	vaultSecretParts = 2
+)
+
+// ParseVaultSecretRef parses a "vault:secret" or just "secret" reference
+// Returns (vaultName, secretName, isQualified)
+// If no vault is specified, returns empty vaultName and isQualified=false
+func ParseVaultSecretRef(ref string) (vaultName, secretName string, isQualified bool) {
+	parts := strings.SplitN(ref, vaultSecretSeparator, vaultSecretParts)
+	if len(parts) == vaultSecretParts {
+		return parts[0], parts[1], true
+	}
+
+	return "", ref, false
+}
+
 func newAddCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "add <secret-name>",
@@ -135,24 +154,32 @@ func newMoveCmd() *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.`,
+		Long: `Move a secret within a vault or between vaults.
+
+For within-vault moves (rename):
+  secret move old-name new-name
+
+For cross-vault moves:
+  secret move source-vault:secret-name dest-vault
+  secret move source-vault:secret-name dest-vault:new-name
+
+Cross-vault moves copy ALL versions of the secret, preserving history.
+The source secret is deleted after successful copy.`,
 		Args: cobra.ExactArgs(2), //nolint:mnd // Command requires exactly 2 arguments: source and destination
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-			// Only complete the first argument (source)
-			if len(args) == 0 {
-				return getSecretNamesCompletionFunc(cli.fs, cli.stateDir)(cmd, args, toComplete)
-			}
-
-			return nil, cobra.ShellCompDirectiveNoFileComp
+			// Complete vault:secret format
+			return getVaultSecretCompletionFunc(cli.fs, cli.stateDir)(cmd, args, toComplete)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
+			force, _ := cmd.Flags().GetBool("force")
 			cli := NewCLIInstance()

-			return cli.MoveSecret(cmd, args[0], args[1])
+			return cli.MoveSecret(cmd, args[0], args[1], force)
 		},
 	}

+	cmd.Flags().BoolP("force", "f", false, "Overwrite if destination secret already exists")
+
 	return cmd
 }

@@ -566,50 +593,188 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
 	return nil
 }

-// MoveSecret moves or renames a secret
-func (cli *Instance) MoveSecret(cmd *cobra.Command, sourceName, destName string) error {
-	// Get current vault
+// MoveSecret moves or renames a secret (within or across vaults)
+func (cli *Instance) MoveSecret(cmd *cobra.Command, source, dest string, force bool) error {
+	// Parse source and destination
+	srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
+	destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
+
+	// If neither is qualified, this is a simple within-vault rename
+	if !srcQualified && !destQualified {
+		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
+	}
+
+	// Cross-vault move requires source to be qualified
+	if !srcQualified {
+		return fmt.Errorf("source must specify vault (e.g., vault:secret) for cross-vault move")
+	}
+
+	// If destination is not qualified (no colon), check if it's a vault name
+	// Format: "work:secret default" means move to vault "default"
+	// Format: "work:secret default:newname" means move to vault "default" with new name
+	if !destQualified {
+		// Check if dest is actually a vault name
+		vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
+		if err == nil {
+			for _, v := range vaults {
+				if v == dest {
+					// dest is a vault name, use source secret name
+					destVaultName = dest
+					destSecretName = srcSecretName
+
+					break
+				}
+			}
+		}
+
+		// If destVaultName is still empty, dest is a secret name in source vault
+		if destVaultName == "" {
+			destVaultName = srcVaultName
+			destSecretName = dest
+		}
+	}
+
+	// If destination secret name is empty, use source secret name
+	if destSecretName == "" {
+		destSecretName = srcSecretName
+	}
+
+	// Same vault? Use simple rename if possible (optimization)
+	if srcVaultName == destVaultName {
+		// Select the vault and do a simple move
+		if err := vault.SelectVault(cli.fs, cli.stateDir, srcVaultName); err != nil {
+			return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
+		}
+
+		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
+	}
+
+	// Cross-vault move
+	return cli.moveSecretCrossVault(cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
+}
+
+// moveSecretWithinVault handles rename within the current vault
+func (cli *Instance) moveSecretWithinVault(cmd *cobra.Command, source, dest string, force bool) error {
 	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, "/", "%")
+	sourceEncoded := strings.ReplaceAll(source, "/", "%")
 	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)
+		return fmt.Errorf("secret '%s' not found", source)
 	}

-	// Check if destination already exists
-	destEncoded := strings.ReplaceAll(destName, "/", "%")
+	destEncoded := strings.ReplaceAll(dest, "/", "%")
 	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)
+		if !force {
+			return fmt.Errorf("secret '%s' already exists (use --force to overwrite)", dest)
+		}
+
+		if err := cli.fs.RemoveAll(destDir); err != nil {
+			return fmt.Errorf("failed to remove existing destination: %w", err)
+		}
 	}

-	// 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)
+	cmd.Printf("Moved secret '%s' to '%s'\n", source, dest)
+
+	return nil
+}
+
+// moveSecretCrossVault handles moving between different vaults
+func (cli *Instance) moveSecretCrossVault(
+	cmd *cobra.Command,
+	srcVaultName, srcSecretName,
+	destVaultName, destSecretName string,
+	force bool,
+) error {
+	// Get source vault
+	srcVault := vault.NewVault(cli.fs, cli.stateDir, srcVaultName)
+	srcVaultDir, err := srcVault.GetDirectory()
+
+	if err != nil {
+		return fmt.Errorf("failed to get source vault directory: %w", err)
+	}
+
+	// Verify source vault exists
+	exists, err := afero.DirExists(cli.fs, srcVaultDir)
+	if err != nil || !exists {
+		return fmt.Errorf("source vault '%s' does not exist", srcVaultName)
+	}
+
+	// Verify source secret exists
+	srcStorageName := strings.ReplaceAll(srcSecretName, "/", "%")
+	srcSecretDir := filepath.Join(srcVaultDir, "secrets.d", srcStorageName)
+
+	exists, err = afero.DirExists(cli.fs, srcSecretDir)
+	if err != nil || !exists {
+		return fmt.Errorf("secret '%s' not found in vault '%s'", srcSecretName, srcVaultName)
+	}
+
+	// Get destination vault
+	destVault := vault.NewVault(cli.fs, cli.stateDir, destVaultName)
+	destVaultDir, err := destVault.GetDirectory()
+
+	if err != nil {
+		return fmt.Errorf("failed to get destination vault directory: %w", err)
+	}
+
+	// Verify destination vault exists
+	exists, err = afero.DirExists(cli.fs, destVaultDir)
+	if err != nil || !exists {
+		return fmt.Errorf("destination vault '%s' does not exist", destVaultName)
+	}
+
+	// Unlock destination vault (will fail if neither mnemonic nor unlocker available)
+	_, err = destVault.GetOrDeriveLongTermKey()
+	if err != nil {
+		return fmt.Errorf("failed to unlock destination vault '%s': %w", destVaultName, err)
+	}
+
+	// Count versions for user feedback
+	versions, _ := secret.ListVersions(cli.fs, srcSecretDir)
+	versionCount := len(versions)
+
+	// Copy all versions
+	if err := destVault.CopySecretAllVersions(srcVault, srcSecretName, destSecretName, force); err != nil {
+		return err
+	}
+
+	// Delete source secret
+	if err := cli.fs.RemoveAll(srcSecretDir); err != nil {
+		// Copy succeeded but delete failed - warn but don't fail
+		cmd.Printf("Warning: copied secret but failed to remove source: %v\n", err)
+		cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",
+			srcVaultName, srcSecretName, destVaultName, destSecretName, versionCount)
+
+		return nil
+	}
+
+	cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",
+		srcVaultName, srcSecretName, destVaultName, destSecretName, versionCount)

 	return nil
 }