Stop secret mv deleting a secret moved onto itself (closes #73)

`secret mv --force x x` deleted the secret: a move within one vault
removes an existing destination before renaming the source onto it. The
same happened for `work:x work:`, `work:x work` and `work:x ""`, and for
`work:x work/:x`, which named one vault two ways and so was taken for a
move between vaults.

A move whose two names are the same is now rejected before anything
changes. Every vault named with `vault:` must be one of the existing
vaults by exact name, checked before choosing between the two kinds of
move. A move within a named vault no longer makes it the current vault.

The test runs each rejected move on a copy of two in-memory vaults and
requires the exact error and an unchanged state directory.

Model: opus-5-5

TODO.md

@@ -25,6 +25,22 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-03: `secret mv` rejects a move whose destination is the
+  source (`mv --force x x`, `mv --force work:x work:`, or an empty
+  destination, which defaults to the source name) before changing
+  anything; before, `--force` removed the destination first and so
+  deleted the secret. Every vault name given with `vault:` must be one
+  of the existing vaults by exact name, so `work:x work/:x` is rejected
+  instead of being taken for a move between two vaults. A move within a
+  named vault no longer makes that vault the current one, whether it
+  succeeds or fails.
+- 2026-10-03: `version rm`, `version promote` and `get --version`
+  accept a version only if it is one of the versions `version list`
+  lists for that secret, compared as typed before any path is built
+  (`secret.VersionExists`), and touch nothing otherwise. An empty
+  `--version` is rejected instead of meaning the current version.
+  Before, `secret version rm x ../../..` deleted the whole vault,
+  `secret version rm x ..` the secret, and `.` or `""` every version.
 - 2026-10-03: Key material is wiped on every exit: `Entry()` returns
   the exit code after its deferred `memguard.Purge()` has run, and only
   `main` calls `os.Exit`. SIGINT and SIGTERM go through memguard's

internal/cli/integration_test.go

@@ -829,6 +829,14 @@ func test09GetSpecificVersion(t *testing.T, tempDir, testMnemonic string, runSec
 
 	require.NoError(t, err, "get current version should succeed")
 	assert.Equal(t, "newpassword456", strings.TrimSpace(output), "should return new secret value without --version")
+
+	// An empty --version is not a version; it does not mean the current one
+	output, err = runSecretWithEnv(map[string]string{
+		secret.EnvMnemonic: testMnemonic,
+	}, "get", "--version", "", "database/password")
+
+	require.Error(t, err, "get with an empty version should fail")
+	assert.Contains(t, output, "version '' not found", "should reject the empty version")
 }
 
 func test10PromoteVersion(t *testing.T, tempDir, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error)) {

internal/cli/move_test.go

@@ -0,0 +1,99 @@
+package cli_test
+
+import (
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRejectedMoveWithinVaultLeavesStateUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/73, where a forced move of a secret
+// onto itself deleted it, also when "work" was spelled two ways, and a failed
+// move within "work" left "work" the current vault. "default" is the current
+// vault in every case, and each case runs on its own copy of the state
+// directory.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+	require.Equal(t, "default", before[testStateDir+"/currentvault"])
+
+	const (
+		ontoItself = "secret 'x' cannot be moved onto itself"
+		workX      = "work:x"
+	)
+
+	tests := []struct {
+		command      string
+		source, dest string
+		force        bool
+		wantErr      string
+	}{
+		{"mv x x", "x", "x", false, ontoItself},
+		{"mv --force x x", "x", "x", true, ontoItself},
+		{"mv --force work:x work:", workX, "work:", true, ontoItself},
+		// An empty destination name defaults to the source name.
+		{`mv --force work:x ""`, workX, "", true, ontoItself},
+		// "work" is a vault name, so the destination is work:x.
+		{"mv --force work:x work", workX, "work", true, ontoItself},
+		{
+			"mv work:nosuch work:y", "work:nosuch", "work:y", false,
+			"secret 'nosuch' not found",
+		},
+		// Only an existing vault is used, so ".." cannot reach the state
+		// directory itself.
+		{
+			"mv --force ..:x ..:y", "..:x", "..:y", true,
+			"vault '..' does not exist",
+		},
+		// Each of these spells "work" a second way. The spelling is not an
+		// existing vault name, so the move is not taken for a move between
+		// two vaults, which would delete the destination, here the source.
+		{
+			"mv --force work:x work/:x", workX, "work/:x", true,
+			"vault 'work/' does not exist",
+		},
+		{
+			"mv --force work/:x work:", "work/:x", "work:", true,
+			"vault 'work/' does not exist",
+		},
+		{
+			"mv --force work:x ./work:x", workX, "./work:x", true,
+			"vault './work' does not exist",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			fs := newFsFromSnapshot(t, before)
+			c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+
+			err := c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, tt.force)
+
+			require.Equal(t, before, snapshotStateDir(t, fs))
+			require.EqualError(t, err, tt.wantErr)
+		})
+	}
+}
+
+// TestMoveWithinOtherVaultKeepsCurrentVault checks that `secret mv work:x
+// work:y`, with "default" the current vault, renames "x" to "y" in "work" and
+// leaves "default" the current vault.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestMoveWithinOtherVaultKeepsCurrentVault(t *testing.T) {
+	fs := newTwoVaultFs(t)
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+
+	err := c.MoveSecret(&cobra.Command{}, "work:x", "work:y", false)
+	require.NoError(t, err)
+
+	after := snapshotStateDir(t, fs)
+	workSecrets := testStateDir + "/vaults.d/work/secrets.d/"
+
+	require.Equal(t, "default", after[testStateDir+"/currentvault"])
+	require.Contains(t, after, workSecrets+"y/")
+	require.NotContains(t, after, workSecrets+"x/")
+}

internal/cli/path_traversal_test.go

@@ -1,6 +1,7 @@
 package cli_test
 
 import (
+	"fmt"
 	"maps"
 	"os"
 	"slices"
@@ -119,13 +120,13 @@ func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs {
 }
 
 // requireRejectedAndUnchanged runs a command on a copy of the state
-// directory recorded in before. It requires exactly the error
-// vault.ValidateSecretName gives for the rejected name, so that a later
-// check rejecting the name does not count, and everything under the state
-// directory as it was: the error alone proves nothing, since it could come
-// after the vault had already been deleted.
+// directory recorded in before. It requires an error with exactly the
+// message of want, so that a later check rejecting the argument does not
+// count, and everything under the state directory as it was: the error
+// alone proves nothing, since it could come after the vault had already
+// been deleted.
 func requireRejectedAndUnchanged(
-	t *testing.T, before map[string]string, rejected string,
+	t *testing.T, before map[string]string, want error,
 	run func(c *cli.Instance) error,
 ) {
 	t.Helper()
@@ -135,8 +136,7 @@ func requireRejectedAndUnchanged(
 	err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
 
 	require.Equal(t, before, snapshotStateDir(t, fs))
-	require.ErrorIs(t, err, vault.ErrInvalidSecretName)
-	require.EqualError(t, err, vault.ValidateSecretName(rejected).Error())
+	require.EqualError(t, err, want.Error())
 }
 
 // TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
@@ -229,11 +229,92 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.command, func(t *testing.T) {
-			requireRejectedAndUnchanged(t, before, tt.rejected, tt.run)
+			requireRejectedAndUnchanged(t, before, vault.ValidateSecretName(tt.rejected), tt.run)
 		})
 	}
 }
 
+// TestInvalidVersionLeavesVaultsUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/67, where
+// `secret version rm x ../../..` deleted the whole vault,
+// `secret version rm x ..` the secret x, and `secret version rm x .` or
+// `secret version rm x ""` every version of x. A version argument is
+// accepted only if it is one of the versions `secret version list` lists.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+
+	cmd := &cobra.Command{}
+
+	commands := []struct {
+		command string
+		run     func(c *cli.Instance, version string) error
+	}{
+		{"version rm x", func(c *cli.Instance, version string) error {
+			return c.RemoveVersion(cmd, "x", version)
+		}},
+		{"version promote x", func(c *cli.Instance, version string) error {
+			return c.PromoteVersion(cmd, "x", version)
+		}},
+		{"get x --version", func(c *cli.Instance, version string) error {
+			return c.GetSecretWithVersion(cmd, "x", version)
+		}},
+	}
+
+	for _, tt := range commands {
+		for _, version := range []string{"", ".", "..", "../../..", "a/b"} {
+			t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) {
+				want := fmt.Errorf("version '%s' %w '%s'",
+					version, vault.ErrVersionNotFound, "x")
+				requireRejectedAndUnchanged(t, before, want,
+					func(c *cli.Instance) error { return tt.run(c, version) })
+			})
+		}
+	}
+}
+
+// TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
+// with a version that is not the current one removes that version and
+// changes nothing else.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestRemoveVersionRemovesOnlyThatVersion(t *testing.T) {
+	fs := newTwoVaultFs(t)
+
+	vlt, err := vault.GetCurrentVault(fs, testStateDir)
+	require.NoError(t, err)
+
+	// A second version of "x" becomes the current one.
+	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("new")), true)
+	require.NoError(t, err)
+
+	secretDir := testStateDir + "/vaults.d/default/secrets.d/x"
+	versions, err := secret.ListVersions(fs, secretDir)
+	require.NoError(t, err)
+	require.Len(t, versions, 2)
+
+	// ListVersions lists the newest version first.
+	oldDir := secretDir + "/versions/" + versions[1] + "/"
+	before := snapshotStateDir(t, fs)
+	require.Contains(t, before, oldDir)
+
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	err = c.RemoveVersion(&cobra.Command{}, "x", versions[1])
+	require.NoError(t, err)
+
+	// Expected: the state as before without everything under oldDir.
+	want := map[string]string{}
+
+	for path, content := range before {
+		if !strings.HasPrefix(path, oldDir) {
+			want[path] = content
+		}
+	}
+
+	require.Equal(t, want, snapshotStateDir(t, fs))
+}
+
 // TestMoveToVaultNameRenamesInCurrentVault checks that `secret mv x work`,
 // where "work" is also the name of a vault, renames the secret "x" to "work"
 // in the current vault and changes nothing else.

internal/cli/secrets.go

@@ -40,6 +40,7 @@ var (
 	errVaultDoesNotExist           = errors.New("does not exist")
 	errCrossVaultSourceUnqualified = errors.New(
 		"source must specify vault (e.g., vault:secret) for cross-vault move")
+	errMoveOntoItself = errors.New("cannot be moved onto itself")
 )
 
 // bufferInfo tracks a protected buffer and the number of bytes used in it
@@ -109,6 +110,12 @@ func newGetCmd() *cobra.Command {
 				return fmt.Errorf("failed to initialize CLI: %w", err)
 			}
 
+			// Without --version, get the current version. A given
+			// --version is checked as typed, so an empty one is rejected.
+			if !cmd.Flags().Changed("version") {
+				return cli.GetSecret(cmd, args[0])
+			}
+
 			return cli.GetSecretWithVersion(cmd, args[0], version)
 		},
 	}
@@ -393,12 +400,32 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 	return nil
 }
 
-// GetSecret retrieves and prints a secret from the current vault
+// GetSecret retrieves and prints the current version of a secret
 func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
-	return cli.GetSecretWithVersion(cmd, secretName, "")
+	secret.Debug("GetSecret called", "secretName", secretName)
+
+	// Store the command for output
+	cli.cmd = cmd
+
+	// Get current vault
+	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+
+	value, err := vlt.GetSecret(secretName)
+	if err != nil {
+		return err
+	}
+
+	// Print the secret value to stdout
+	_, _ = cli.Print(string(value))
+
+	return nil
 }
 
-// GetSecretWithVersion retrieves and prints a specific version of a secret
+// GetSecretWithVersion retrieves and prints a specific version of a secret.
+// The version must be one of the secret's versions.
 func (cli *Instance) GetSecretWithVersion(
 	cmd *cobra.Command, secretName string, version string,
 ) error {
@@ -417,13 +444,7 @@ func (cli *Instance) GetSecretWithVersion(
 	}
 
 	// Get the secret value
-	var value []byte
-	if version == "" {
-		value, err = vlt.GetSecret(secretName)
-	} else {
-		value, err = vlt.GetSecretVersion(secretName, version)
-	}
-
+	value, err := vlt.GetSecretVersion(secretName, version)
 	if err != nil {
 		secret.Debug("Failed to get secret", "error", err)
 
@@ -723,8 +744,8 @@ func (cli *Instance) MoveSecret(
 	// with a new name
 	if srcQualified && !destQualified {
 		// Check if dest is actually a vault name
-		vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
-		if err == nil && slices.Contains(vaults, dest) {
+		_, err := cli.existingVault(dest)
+		if err == nil {
 			// dest is a vault name, use source secret name
 			destVaultName = dest
 			destSecretName = srcSecretName
@@ -740,8 +761,8 @@ func (cli *Instance) MoveSecret(
 		destSecretName = srcSecretName
 	}
 
-	// Check both names, for every form of the move, before selecting a vault
-	// below, so that a rejected move leaves the current vault as it was.
+	// Check both names, for every form of the move, before building any path
+	// from them.
 	err := vault.ValidateSecretName(srcSecretName)
 	if err != nil {
 		return err
@@ -752,38 +773,66 @@ func (cli *Instance) MoveSecret(
 		return err
 	}
 
-	// If neither is qualified, this is a simple within-vault rename
-	if !srcQualified && !destQualified {
-		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
-	}
-
-	// Same vault? Use simple rename if possible (optimization)
-	if srcVaultName == destVaultName {
-		// Select the vault and do a simple move
-		err = vault.SelectVault(cli.fs, cli.stateDir, srcVaultName)
+	// Neither name is qualified: a rename within the current vault.
+	if !srcQualified {
+		vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 		if err != nil {
-			return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
+			return err
 		}
 
-		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
+		return cli.moveSecretWithinVault(
+			cmd, vlt, srcSecretName, destSecretName, force)
 	}
 
-	// Cross-vault move
-	return cli.moveSecretCrossVault(
-		cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
-}
-
-// moveSecretWithinVault handles rename within the current vault. Its caller,
-// MoveSecret, has already checked both secret names.
-func (cli *Instance) moveSecretWithinVault(
-	cmd *cobra.Command, source, dest string, force bool,
-) error {
-	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	// Both vaults must be existing vaults by exact name, so that two
+	// spellings of one vault, such as "work" and "work/", are never taken for
+	// two vaults. A named vault does not become the current vault.
+	srcVault, err := cli.existingVault(srcVaultName)
 	if err != nil {
 		return err
 	}
 
-	vaultDir, err := currentVlt.GetDirectory()
+	destVault, err := cli.existingVault(destVaultName)
+	if err != nil {
+		return err
+	}
+
+	if srcVaultName == destVaultName {
+		return cli.moveSecretWithinVault(
+			cmd, srcVault, srcSecretName, destSecretName, force)
+	}
+
+	return cli.moveSecretCrossVault(
+		cmd, srcVault, srcSecretName, destVault, destSecretName, force)
+}
+
+// existingVault returns the vault with the given name, or an error if there
+// is none. Unlike vault.SelectVault, it leaves the current vault as it is.
+func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
+	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
+	if err != nil {
+		return nil, fmt.Errorf("failed to list vaults: %w", err)
+	}
+
+	if !slices.Contains(vaults, name) {
+		return nil, fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
+	}
+
+	return vault.NewVault(cli.fs, cli.stateDir, name), nil
+}
+
+// moveSecretWithinVault renames a secret within the vault vlt. Its caller,
+// MoveSecret, has already checked both secret names.
+func (cli *Instance) moveSecretWithinVault(
+	cmd *cobra.Command, vlt *vault.Vault, source, dest string, force bool,
+) error {
+	// With --force the destination is removed before the source is renamed
+	// onto it, which would delete the secret.
+	if source == dest {
+		return fmt.Errorf("secret '%s' %w", source, errMoveOntoItself)
+	}
+
+	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
 		return err
 	}
@@ -829,57 +878,34 @@ func (cli *Instance) moveSecretWithinVault(
 	return nil
 }
 
-// moveSecretCrossVault handles moving between different vaults. Its caller,
-// MoveSecret, has already checked both secret names.
+// moveSecretCrossVault handles moving between two different vaults. Its
+// caller, MoveSecret, has already checked both secret names and that both
+// vaults exist.
 func (cli *Instance) moveSecretCrossVault(
 	cmd *cobra.Command,
-	srcVaultName, srcSecretName,
-	destVaultName, destSecretName string,
+	srcVault *vault.Vault, srcSecretName string,
+	destVault *vault.Vault, 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' %w", srcVaultName, errVaultDoesNotExist)
-	}
-
 	// Verify source secret exists
 	srcStorageName := strings.ReplaceAll(srcSecretName, "/", "%")
 	srcSecretDir := filepath.Join(srcVaultDir, "secrets.d", srcStorageName)
 
-	exists, err = afero.DirExists(cli.fs, srcSecretDir)
+	exists, err := afero.DirExists(cli.fs, srcSecretDir)
 	if err != nil || !exists {
 		return fmt.Errorf("secret '%s' %w in vault '%s'",
-			srcSecretName, errSecretNotFound, 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' %w",
-			destVaultName, errVaultDoesNotExist)
+			srcSecretName, errSecretNotFound, srcVault.Name)
 	}
 
 	// 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)
+		return fmt.Errorf("failed to unlock destination vault '%s': %w", destVault.Name, err)
 	}
 
 	// Count versions for user feedback
@@ -899,13 +925,13 @@ func (cli *Instance) moveSecretCrossVault(
 		// 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)
+			srcVault.Name, srcSecretName, destVault.Name, destSecretName, versionCount)
 
 		return nil
 	}
 
 	cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",
-		srcVaultName, srcSecretName, destVaultName, destSecretName, versionCount)
+		srcVault.Name, srcSecretName, destVault.Name, destSecretName, versionCount)
 
 	return nil
 }

internal/cli/version.go

@@ -265,9 +265,7 @@ func (cli *Instance) PromoteVersion(
 	secretDir := filepath.Join(vaultDir, "secrets.d", encodedName)
 
 	// Check if version exists
-	versionDir := filepath.Join(secretDir, "versions", version)
-
-	exists, err := afero.DirExists(cli.fs, versionDir)
+	exists, err := secret.VersionExists(cli.fs, secretDir, version)
 	if err != nil {
 		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
@@ -323,9 +321,7 @@ func (cli *Instance) RemoveVersion(
 	}
 
 	// Check if version exists
-	versionDir := filepath.Join(secretDir, "versions", version)
-
-	exists, err = afero.DirExists(cli.fs, versionDir)
+	exists, err = secret.VersionExists(cli.fs, secretDir, version)
 	if err != nil {
 		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
@@ -348,6 +344,8 @@ func (cli *Instance) RemoveVersion(
 	}
 
 	// Remove the version directory
+	versionDir := filepath.Join(secretDir, "versions", version)
+
 	err = cli.fs.RemoveAll(versionDir)
 	if err != nil {
 		return fmt.Errorf("failed to remove version: %w", err)

internal/cli/version_test.go

@@ -276,8 +276,8 @@ func TestGetSecretWithVersion(t *testing.T) {
 	var buf bytes.Buffer
 	cmd.SetOut(&buf)
 
-	// Test getting current version (empty version string)
-	err = cli.GetSecretWithVersion(cmd, "test/secret", "")
+	// Test getting the current version
+	err = cli.GetSecret(cmd, "test/secret")
 	require.NoError(t, err)
 	assert.Equal(t, "version-2", buf.String())
 

internal/secret/version.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log/slog"
 	"path/filepath"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -524,6 +525,18 @@ func ListVersions(fs afero.Fs, secretDir string) ([]string, error) {
 	return versions, nil
 }
 
+// VersionExists reports whether version is one of the versions ListVersions
+// lists for the secret in secretDir. It only compares names, so a version
+// the user typed can be checked with it before any path is built from it.
+func VersionExists(fs afero.Fs, secretDir string, version string) (bool, error) {
+	versions, err := ListVersions(fs, secretDir)
+	if err != nil {
+		return false, err
+	}
+
+	return slices.Contains(versions, version), nil
+}
+
 // GetCurrentVersion returns the version that the "current" file points to
 // The file contains just the version name (e.g., "20231215.001")
 func GetCurrentVersion(fs afero.Fs, secretDir string) (string, error) {

internal/vault/errors.go

@@ -49,7 +49,7 @@ var (
 
 	// ErrVersionNotFound indicates the requested secret version does not
 	// exist. Composed as
-	// "version <version> not found for secret <name>".
+	// "version '<version>' not found for secret '<name>'".
 	ErrVersionNotFound = errors.New("not found for secret")
 
 	// ErrNoVersions indicates the source secret has no versions. Composed

internal/vault/integration_version_test.go

@@ -235,10 +235,10 @@ func testRetrieveSpecificVersions(
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-3-data"), value3)
 
-	// Empty version should return current
-	valueCurrent, err := vault.GetSecretVersion(secretName, "")
-	require.NoError(t, err)
-	assert.Equal(t, []byte("version-3-data"), valueCurrent)
+	// An empty version is not one of the versions; GetSecret gets the
+	// current one
+	_, err = vault.GetSecretVersion(secretName, "")
+	require.ErrorIs(t, err, ErrVersionNotFound)
 }
 
 func testPromoteOldVersion(

internal/vault/secrets.go

@@ -259,18 +259,31 @@ func updateVersionMetadata(
 	return nil
 }
 
-// GetSecret retrieves a secret from this vault
+// GetSecret retrieves the current version of a secret from this vault
 func (v *Vault) GetSecret(name string) ([]byte, error) {
 	secret.DebugWith("Getting secret from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
 	)
 
-	return v.GetSecretVersion(name, "")
+	// GetSecretObject validates the name and checks that the secret exists
+	secretObj, err := v.GetSecretObject(name)
+	if err != nil {
+		return nil, err
+	}
+
+	currentVersion, err := secret.GetCurrentVersion(v.fs, secretObj.Directory)
+	if err != nil {
+		secret.Debug("Failed to get current version", "error", err, "secret_name", name)
+
+		return nil, fmt.Errorf("failed to get current version: %w", err)
+	}
+
+	return v.GetSecretVersion(name, currentVersion)
 }
 
-// GetSecretVersion retrieves a specific version of a secret (empty version
-// means current)
+// GetSecretVersion retrieves a specific version of a secret. The version
+// must be one of the secret's versions; GetSecret gets the current one.
 func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
@@ -278,8 +291,8 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 		slog.String("version", version),
 	)
 
-	// Validate the name and resolve the version to fetch
-	version, err := v.resolveSecretVersion(name, version)
+	// Validate the name and check that the version exists
+	err := v.checkSecretVersion(name, version)
 	if err != nil {
 		return nil, err
 	}
@@ -640,15 +653,15 @@ func (v *Vault) updatePreviousVersion(
 	return nil
 }
 
-// resolveSecretVersion validates the secret name, verifies the secret and
-// version exist, and resolves an empty version to the current one.
-func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
+// checkSecretVersion validates the secret name and verifies that the secret
+// exists and that version is one of its versions.
+func (v *Vault) checkSecretVersion(name, version string) error {
 	// Validate secret name to prevent path traversal
 	err := ValidateSecretName(name)
 	if err != nil {
 		secret.Debug("Invalid secret name provided", "secret_name", name)
 
-		return "", err
+		return err
 	}
 
 	// Get vault directory
@@ -656,7 +669,7 @@ func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
 	if err != nil {
 		secret.Debug("Failed to get vault directory", "error", err, "vault_name", v.Name)
 
-		return "", err
+		return err
 	}
 
 	// Convert slashes to percent signs for storage
@@ -668,50 +681,30 @@ func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
 	if err != nil {
 		secret.Debug("Failed to check if secret exists", "error", err, "secret_name", name)
 
-		return "", fmt.Errorf("failed to check if secret exists: %w", err)
+		return fmt.Errorf("failed to check if secret exists: %w", err)
 	}
 
 	if !exists {
 		secret.Debug("Secret not found in vault", "secret_name", name, "vault_name", v.Name)
 
-		return "", fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
-	}
-
-	// Determine which version to get
-	if version == "" {
-		// Get current version
-		currentVersion, err := secret.GetCurrentVersion(v.fs, secretDir)
-		if err != nil {
-			secret.Debug("Failed to get current version", "error", err, "secret_name", name)
-
-			return "", fmt.Errorf("failed to get current version: %w", err)
-		}
-
-		version = currentVersion
-
-		secret.Debug("Using current version", "version", version, "secret_name", name)
+		return fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
 	}
 
 	// Check if version exists
-	versionPath := filepath.Join(secretDir, "versions", version)
-
-	exists, err = afero.DirExists(v.fs, versionPath)
+	exists, err = secret.VersionExists(v.fs, secretDir, version)
 	if err != nil {
 		secret.Debug("Failed to check if version exists", "error", err, "version", version)
 
-		return "", fmt.Errorf("failed to check if version exists: %w", err)
+		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
 
 	if !exists {
 		secret.Debug("Version not found", "version", version, "secret_name", name)
 
-		return "", fmt.Errorf(
-			"version %s %w %s",
-			version, ErrVersionNotFound, name,
-		)
+		return fmt.Errorf("version '%s' %w '%s'", version, ErrVersionNotFound, name)
 	}
 
-	return version, nil
+	return nil
 }
 
 // createAndSaveVersion generates a new version name, sets the version

internal/vault/secrets_version_test.go

@@ -202,10 +202,10 @@ func TestVaultGetSecretVersion(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-2"), value)
 
-	// Get current (empty version)
-	value, err = vault.GetSecretVersion(testSecretPath, "")
-	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+	// An empty version is not one of the versions; GetSecret gets the
+	// current one
+	_, err = vault.GetSecretVersion(testSecretPath, "")
+	require.ErrorIs(t, err, ErrVersionNotFound)
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv