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/path_traversal_test.go

@@ -1,6 +1,7 @@
 package cli_test
 
 import (
+	"fmt"
 	"maps"
 	"os"
 	"slices"
@@ -119,13 +120,12 @@ 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 exactly the error message 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 string,
 	run func(c *cli.Instance) error,
 ) {
 	t.Helper()
@@ -135,8 +135,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)
 }
 
 // TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
@@ -229,11 +228,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)
+			want := vault.ValidateSecretName(tt.rejected).Error()
+			requireRejectedAndUnchanged(t, before, want, 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 := "version '" + version + "' not found for secret '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.