Accept a version argument only if the secret has that version (closes #67)

version rm, version promote and get --version joined the version
argument into a path unchecked, so "", ".", "..", "../../.." removed or
read every version, the secret, the vault or directories above it.

A version is now accepted only if it is one of the versions
ListVersions lists for the secret, compared by name before any path is
built (secret.VersionExists, used by all three). An empty --version is
rejected instead of meaning the current version: GetSecretVersion no
longer treats "" as current, and GetSecret looks the current version
up itself.

Model: opus-5-5

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/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

@@ -109,6 +109,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 +399,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 +443,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)
 

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())