Stop vault safety checks from reading unreadable state as empty (closes #51)

Adding a PGP unlocker checked unlockers.d for a duplicate and, when the
directory could not be read, reported no duplicate and went on. The
check now returns an error naming the directory and cause, and the add
stops; a duplicate found is still reported as one.

The same flaw guarded removing the last unlocker and removing a vault
(an unreadable secrets directory counted as no secrets) and vault
import (an unreadable pub.age counted as no long-term key). Those now
stop with an error too.

`unlocker list` keeps skipping entries it cannot read; a comment at the
duplicate check says why the two differ.

Model: opus-5-5

.dockerignore

@@ -1,3 +1,9 @@
+# .git is sent without its config. Without a VERSION build argument the
+# stage that compiles runs `git describe --tags --always` on .git, which
+# does not need .git/config; that file can hold a credential, such as a
+# password in a remote URL or the token the CI checkout step stores there.
+.git/config
+
 # Build artifacts
 secret
 coverage.out

Dockerfile

@@ -27,7 +27,20 @@ RUN go mod download
 COPY . .
 
 RUN make test
-RUN make build
+
+# The version stamped into the binary: the VERSION build argument when one
+# is given, otherwise `git describe --tags --always` of the .git the build
+# context carries: the tag on a tagged commit, tag-N-gHASH on a commit after
+# one, the short commit when no tag is reachable. A context that carries .git
+# and still yields no version fails the build.
+ARG VERSION
+RUN version="${VERSION:-$(git describe --tags --always)}"; \
+    if [ -e .git ] && { [ -z "$version" ] || [ "$version" = dev ] || \
+        [ "$version" = unknown ]; }; then \
+        echo "no version could be derived although the build context carries .git" >&2; \
+        exit 1; \
+    fi; \
+    make build VERSION="${version:-dev}"
 
 # Runtime stage
 # alpine 3.23 (2026-03-10)

Makefile

@@ -2,7 +2,7 @@ export CGO_ENABLED=1
 export DOCKER_HOST := ssh://root@ber1app1.local
 
 # Version information
-VERSION := 0.1.0
+VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
 GIT_COMMIT := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
 LDFLAGS := -X 'git.eeqj.de/sneak/secret/internal/cli.Version=$(VERSION)' \
            -X 'git.eeqj.de/sneak/secret/internal/cli.GitCommit=$(GIT_COMMIT)'

README.md

@@ -113,7 +113,9 @@ automatically switch to another vault if removing the current one.
 Adds a secret to the current vault. Reads the secret value from stdin.
 - `--force, -f`: Overwrite existing secret
 
-**Secret Name Format:** `[a-z0-9\.\-\_\/]+`
+**Secret Name Format:** only ASCII letters, digits, `.`, `-`, `_` and `/`
+are allowed, and a name must not be empty, start with `.` or `/`, end with
+`/`, contain `//`, or have `..` as a path segment.
 - Forward slashes (`/`) are converted to percent signs (`%`) for storage
 - Examples: `database/password`, `api.key`, `ssh_private_key`
 

TODO.md

@@ -25,6 +25,47 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-03: The checks run before changing a vault now stop with an
+  error naming the path and cause when they cannot read what they
+  inspect, instead of reading the failure as "nothing there": the
+  duplicate check before `unlocker add pgp` (an unreadable
+  `unlockers.d`), the secret count that guards removing the last
+  unlocker and removing a vault, and the existing long-term key check
+  before `vault import`.
+- 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
+  handler, which wipes every buffer before exiting; when the process is
+  in the terminal's foreground process group it first restores the
+  terminal settings from startup, so an interrupted passphrase prompt no
+  longer leaves echo off.
+- 2026-10-03: Every command that builds a path from a secret name
+  checks the name first with `vault.ValidateSecretName` and touches
+  nothing when it is invalid: `rm`, `mv` (both names, within a vault
+  and between vaults, before switching the current vault), `import`,
+  `version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error
+  and `README.md` state the naming rule. Before, `secret rm ..`
+  deleted the whole vault and `secret rm .` every secret in it.
+- 2026-10-03: The keychain unlocker's age key passphrase stays in
+  locked memory: it is generated into a locked buffer, and the
+  keychain JSON is written and read by `KeychainData` code in
+  `internal/secret/keychaindata.go` (tested on Linux) without
+  `encoding/json` holding it; the JSON field names are unchanged.
+- 2026-10-02: A plain `docker build .` builds again: the size tests
+  skip a case that needs more locked memory than the process can
+  lock, and run every case under `script/cibuild`. The image stamps the
+  `VERSION` build argument, else `git describe --tags --always`, into
+  `Version`, and fails if `.git` is present but yields no version;
+  `make build` stamps `git describe` too, not a fixed `0.1.0`.
+  `.dockerignore` keeps `.git/config` out; `script/docker` is the
+  canonical copy.
 - 2026-08-07: Updated golangci-lint to v2.12.2 with the canonical
   `.golangci.yml` (all linters enabled minus the standard disable
   list, `lll` 88, tests linted); bumped the `Dockerfile` lint-stage
@@ -80,16 +121,14 @@ Bring the repo into policy compliance in one commit:
     - Command injection: GPG key IDs passed unescaped to exec.Command
       (pgpunlocker.go:323-327); data.String() passed unescaped to the
       security command (keychainunlocker.go:472-476).
-    - Memory security: KeychainData stores AgePrivKeyPassphrase as a
+    - Memory security: age identity .String() creates unprotected
-      plain string (keychainunlocker.go:342,393-396); age identity
+      copies (keychainunlocker.go:356, pgpunlocker.go:256,
-      .String() creates unprotected copies (keychainunlocker.go:356,
+      version.go:155); age secret key held in a plain string in
-      pgpunlocker.go:256, version.go:155); age secret key held in a
+      cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
-      plain string in cli/crypto.go:86,91,113; private keys exposed via
+      to GPGEncryptFunc and EncryptWithPassphrase.
-      buffer.Bytes() to GPGEncryptFunc and EncryptWithPassphrase.
     - Race conditions: no file locking in vault/secrets.go:142-176;
       non-atomic writes can leave the vault inconsistent.
-    - Input validation: dots in secret names risk path traversal
+    - Input validation: no maximum secret size (DoS).
-      (vault/secrets.go:75-99); no maximum secret size (DoS).
     - Timing attacks: bytes.Equal passphrase compare (cli/init.go:
       209-216); non-constant-time public key compare (vault.go:95-100).
 - High priority:

cmd/secret/main.go

@@ -1,8 +1,12 @@
 // Package main is the entry point for the secret CLI application.
 package main
 
-import "git.eeqj.de/sneak/secret/internal/cli"
+import (
+	"os"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+)
 
 func main() {
-	cli.Entry()
+	os.Exit(cli.Entry())
 }

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/stretchr/testify v1.8.4
 	github.com/tyler-smith/go-bip39 v1.1.0
 	golang.org/x/crypto v0.38.0
+	golang.org/x/sys v0.33.0
 	golang.org/x/term v0.32.0
 )
 
@@ -31,7 +32,6 @@ require (
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/spf13/pflag v1.0.6 // indirect
-	golang.org/x/sys v0.33.0 // indirect
 	golang.org/x/text v0.25.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

internal/cli/crypto.go

@@ -122,6 +122,11 @@ func (cli *Instance) resolveEncryptionKey(
 
 // Encrypt encrypts data using an age secret key stored in a secret
 func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -191,6 +196,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
 
 // Decrypt decrypts data using an age secret key stored in a secret
 func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {

internal/cli/entry_test.go

@@ -0,0 +1,108 @@
+package cli_test
+
+import (
+	"bufio"
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"github.com/awnumar/memguard"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Entry must return its exit code rather than exit, so that its deferred
+// memguard purge runs on the success and the error path alike.
+//
+//nolint:paralleltest // sets os.Args, and Entry wipes every buffer in the process
+func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) {
+	savedArgs := os.Args
+
+	t.Cleanup(func() { os.Args = savedArgs })
+
+	tests := []struct {
+		args     []string
+		exitCode int
+	}{
+		{args: []string{"secret", "--help"}, exitCode: 0},
+		{args: []string{"secret", "no-such-command"}, exitCode: 1},
+	}
+
+	for _, tt := range tests {
+		buf := memguard.NewBufferFromBytes([]byte("key material"))
+		os.Args = tt.args
+
+		assert.Equal(t, tt.exitCode, cli.Entry(), "exit code for %v", tt.args)
+		assert.False(t, buf.IsAlive(), "Entry left a buffer unwiped for %v", tt.args)
+	}
+}
+
+// Ctrl-C while `secret add` waits for the value on stdin must end the
+// process through memguard's signal handler, which wipes every buffer and
+// exits with status 1, not through Go's default handling, which kills the
+// process with the buffers intact.
+func TestInterruptExitsThroughMemguard(t *testing.T) {
+	t.Parallel()
+
+	const waitingForValue = "Reading secret value from stdin"
+
+	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
+	defer cancel()
+
+	wd, err := filepath.Abs("../..")
+	require.NoError(t, err)
+
+	secretPath := filepath.Join(wd, "secret")
+	env := []string{
+		secret.EnvStateDir + "=" + t.TempDir(),
+		secret.EnvMnemonic + "=" + testMnemonic,
+		secret.EnvUnlockPassphrase + "=test-passphrase",
+		"PATH=/usr/bin:/bin",
+		// The debug log on stderr shows when add starts waiting for the value.
+		"GODEBUG=berlin.sneak.pkg.secret",
+	}
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	initCmd := exec.CommandContext(ctx, secretPath, "init")
+	initCmd.Env = env
+
+	output, err := initCmd.CombinedOutput()
+	require.NoError(t, err, "init should succeed: %s", output)
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	addCmd := exec.CommandContext(ctx, secretPath, "add", "test/secret")
+	addCmd.Env = env
+
+	// Held open and never written, so add keeps waiting for the value.
+	stdin, err := addCmd.StdinPipe()
+	require.NoError(t, err)
+
+	defer func() { _ = stdin.Close() }()
+
+	stderr, err := addCmd.StderrPipe()
+	require.NoError(t, err)
+	require.NoError(t, addCmd.Start())
+
+	waiting := false
+
+	scanner := bufio.NewScanner(stderr)
+	for !waiting && scanner.Scan() {
+		waiting = strings.Contains(scanner.Text(), waitingForValue)
+	}
+
+	require.True(t, waiting, "add never logged %q", waitingForValue)
+	require.NoError(t, addCmd.Process.Signal(os.Interrupt))
+
+	err = addCmd.Wait()
+
+	var exitErr *exec.ExitError
+
+	require.ErrorAs(t, err, &exitErr)
+	assert.Equal(t, 1, exitErr.ExitCode(), "add ended with %v", err)
+}

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

@@ -0,0 +1,348 @@
+package cli_test
+
+import (
+	"fmt"
+	"maps"
+	"os"
+	"slices"
+	"strings"
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// testStateDir is the in-memory state directory of the test vaults.
+	testStateDir = "/test/state"
+
+	// testPassphrase protects the passphrase unlocker of each test vault.
+	testPassphrase = "test-passphrase"
+
+	// testVersion is a version name in the format the vault uses.
+	testVersion = "20260101.001"
+
+	// missingFile is an import source that does not exist, so an import
+	// that opened it before checking the name would fail with another error.
+	missingFile = "/no/such/file"
+)
+
+// newTwoVaultFs returns an in-memory filesystem holding the vaults "work"
+// and "default", the current one. Each holds the secret "x" and a
+// passphrase unlocker, so both secrets.d and unlockers.d have contents.
+//
+//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
+func newTwoVaultFs(t *testing.T) afero.Fs {
+	t.Helper()
+
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	fs := afero.NewMemMapFs()
+
+	for _, name := range []string{"work", "default"} {
+		vlt, err := vault.CreateVault(fs, testStateDir, name)
+		require.NoError(t, err)
+
+		err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
+		require.NoError(t, err)
+
+		_, err = vlt.CreatePassphraseUnlocker(
+			memguard.NewBufferFromBytes([]byte(testPassphrase)))
+		require.NoError(t, err)
+	}
+
+	return fs
+}
+
+// snapshotStateDir maps every file under the state directory to its
+// contents, and every directory, written with a trailing "/", to "". Two
+// snapshots are equal only if nothing in it was added, removed or changed.
+func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
+	t.Helper()
+
+	tree := map[string]string{}
+
+	err := afero.Walk(fs, testStateDir, func(
+		path string, info os.FileInfo, err error,
+	) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			tree[path+"/"] = ""
+
+			return nil
+		}
+
+		content, err := afero.ReadFile(fs, path)
+		if err != nil {
+			return err
+		}
+
+		tree[path] = string(content)
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	return tree
+}
+
+// newFsFromSnapshot returns a new in-memory filesystem holding exactly the
+// directories and files recorded by snapshotStateDir.
+//
+//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
+func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs {
+	t.Helper()
+
+	fs := afero.NewMemMapFs()
+
+	// In sorted order every directory comes before its contents.
+	for _, path := range slices.Sorted(maps.Keys(tree)) {
+		dir, isDir := strings.CutSuffix(path, "/")
+		if isDir {
+			require.NoError(t, fs.MkdirAll(dir, secret.DirPerms))
+
+			continue
+		}
+
+		err := afero.WriteFile(fs, path, []byte(tree[path]), secret.FilePerms)
+		require.NoError(t, err)
+	}
+
+	return fs
+}
+
+// requireRejectedAndUnchanged runs a command on a copy of the state
+// 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, want error,
+	run func(c *cli.Instance) error,
+) {
+	t.Helper()
+
+	fs := newFsFromSnapshot(t, before)
+
+	err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
+
+	require.Equal(t, before, snapshotStateDir(t, fs))
+	require.EqualError(t, err, want.Error())
+}
+
+// TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/33, where `secret rm ..` deleted
+// the whole vault, and `secret rm .` or `secret rm ""` every secret in it.
+// Moves and imports use --force, so that only the name check stands in
+// the way.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
+	// Creating a passphrase unlocker is slow by design, so the vaults are
+	// created once and each case runs on its own copy of them.
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+
+	vaultDir := testStateDir + "/vaults.d/default"
+	require.Contains(t, before, vaultDir+"/secrets.d/x/")
+	require.Contains(t, before, vaultDir+"/unlockers.d/passphrase/")
+	require.Equal(t, "default", before[testStateDir+"/currentvault"])
+
+	cmd := &cobra.Command{}
+
+	tests := []struct {
+		command  string
+		rejected string // the secret name the command must reject
+		run      func(c *cli.Instance) error
+	}{
+		{"rm ..", "..", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "..", false)
+		}},
+		{"rm .", ".", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, ".", false)
+		}},
+		{`rm ""`, "", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "", false)
+		}},
+		{"rm ../../etc", "../../etc", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "../../etc", false)
+		}},
+		{"mv --force .. x", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "..", "x", true)
+		}},
+		{"mv --force x ..", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "x", "..", true)
+		}},
+		{`mv --force x ""`, "", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "x", "", true)
+		}},
+		// "work" is not the current vault: a move within it must not
+		// select it when a name is rejected.
+		{"mv --force work:.. work:x", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "work:..", "work:x", true)
+		}},
+		{"mv --force work:x work:..", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "work:x", "work:..", true)
+		}},
+		{"mv --force default:.. work", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "default:..", "work", true)
+		}},
+		{"mv --force default:.. work:y", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "default:..", "work:y", true)
+		}},
+		{"mv --force default:x work:..", "..", func(c *cli.Instance) error {
+			return c.MoveSecret(cmd, "default:x", "work:..", true)
+		}},
+		{"import --force ..", "..", func(c *cli.Instance) error {
+			return c.ImportSecret(cmd, "..", missingFile, true)
+		}},
+		{"import --force .", ".", func(c *cli.Instance) error {
+			return c.ImportSecret(cmd, ".", missingFile, true)
+		}},
+		{"import --force ../../etc", "../../etc", func(c *cli.Instance) error {
+			return c.ImportSecret(cmd, "../../etc", missingFile, true)
+		}},
+		{"version list ..", "..", func(c *cli.Instance) error {
+			return c.ListVersions(cmd, "..")
+		}},
+		{"version promote ..", "..", func(c *cli.Instance) error {
+			return c.PromoteVersion(cmd, "..", testVersion)
+		}},
+		{"version rm ..", "..", func(c *cli.Instance) error {
+			return c.RemoveVersion(cmd, "..", testVersion)
+		}},
+		{"encrypt ..", "..", func(c *cli.Instance) error {
+			return c.Encrypt("..", "", "")
+		}},
+		{"decrypt ..", "..", func(c *cli.Instance) error {
+			return c.Decrypt("..", "", "")
+		}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			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.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestMoveToVaultNameRenamesInCurrentVault(t *testing.T) {
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+	fs := newFsFromSnapshot(t, before)
+
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	err := c.MoveSecret(&cobra.Command{}, "x", "work", false)
+	require.NoError(t, err)
+
+	// Expected: the state as before, with everything under the current
+	// vault's secrets.d/x/ now under secrets.d/work/.
+	oldDir := testStateDir + "/vaults.d/default/secrets.d/x/"
+	newDir := testStateDir + "/vaults.d/default/secrets.d/work/"
+	want := map[string]string{}
+
+	for path, content := range before {
+		rest, found := strings.CutPrefix(path, oldDir)
+		if found {
+			path = newDir + rest
+		}
+
+		want[path] = content
+	}
+
+	require.Contains(t, want, newDir)
+	require.Equal(t, want, snapshotStateDir(t, fs))
+}

internal/cli/root.go

@@ -4,17 +4,38 @@ import (
 	"os"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
+	"github.com/awnumar/memguard"
 	"github.com/spf13/cobra"
+	"golang.org/x/sys/unix"
+	"golang.org/x/term"
 )
 
-// Entry is the entry point for the secret CLI application
+// Entry runs the secret CLI and returns the process exit code. It wipes
-func Entry() {
+// every memguard buffer before it returns, so the caller must do nothing
-	cmd := newRootCmd()
+// but exit with the code.
+func Entry() int {
+	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
+	// and exits with status 1. The passphrase prompt turns terminal echo
+	// off until the read finishes, so a signal there would leave echo off.
+	// Only a process in the terminal's foreground process group may reset
+	// it: one in the background that tries is stopped instead of exiting.
+	terminalState, terminalErr := term.GetState(unix.Stdin)
 
-	err := cmd.Execute()
+	memguard.CatchSignal(func(os.Signal) {
-	if err != nil {
+		foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP)
-		os.Exit(1)
+		if terminalErr == nil && err == nil && foreground == unix.Getpgrp() {
+			_ = term.Restore(unix.Stdin, terminalState)
 		}
+	}, os.Interrupt, unix.SIGTERM)
+
+	defer memguard.Purge()
+
+	err := newRootCmd().Execute()
+	if err != nil {
+		return 1
+	}
+
+	return 0
 }
 
 func newRootCmd() *cobra.Command {

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
 	}
 
-// GetSecretWithVersion retrieves and prints a specific version of a secret
+	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.
+// 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
+	value, err := vlt.GetSecretVersion(secretName, version)
-	if version == "" {
-		value, err = vlt.GetSecret(secretName)
-	} else {
-		value, err = vlt.GetSecretVersion(secretName, version)
-	}
-
 	if err != nil {
 		secret.Debug("Failed to get secret", "error", err)
 
@@ -603,6 +623,11 @@ func printSecretsTable(
 func (cli *Instance) ImportSecret(
 	cmd *cobra.Command, secretName, sourceFile string, force bool,
 ) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -649,6 +674,11 @@ func (cli *Instance) ImportSecret(
 
 // RemoveSecret removes a secret from the vault
 func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -702,13 +732,8 @@ func (cli *Instance) MoveSecret(
 	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 {
+	if !srcQualified && destQualified {
 		return errCrossVaultSourceUnqualified
 	}
 
@@ -716,31 +741,46 @@ func (cli *Instance) MoveSecret(
 	// Format: "work:secret default" means move to vault "default"
 	// Format: "work:secret default:newname" means move to vault "default"
 	// with a new name
-	if !destQualified {
+	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) {
 			// dest is a vault name, use source secret name
 			destVaultName = dest
 			destSecretName = srcSecretName
-		}
+		} else {
-
+			// dest is a secret name in source vault
-		// 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 destination secret name is empty, use source secret name. A plain
-	if destSecretName == "" {
+	// rename keeps it empty, so that the check below rejects it.
+	if srcQualified && destSecretName == "" {
 		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.
+	err := vault.ValidateSecretName(srcSecretName)
+	if err != nil {
+		return err
+	}
+
+	err = vault.ValidateSecretName(destSecretName)
+	if err != nil {
+		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)
+		err = vault.SelectVault(cli.fs, cli.stateDir, srcVaultName)
 		if err != nil {
 			return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
 		}
@@ -753,7 +793,8 @@ func (cli *Instance) MoveSecret(
 		cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
 }
 
-// moveSecretWithinVault handles rename within the current vault
+// 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 {
@@ -808,7 +849,8 @@ func (cli *Instance) moveSecretWithinVault(
 	return nil
 }
 
-// moveSecretCrossVault handles moving between different vaults
+// moveSecretCrossVault handles moving between different vaults. Its caller,
+// MoveSecret, has already checked both secret names.
 func (cli *Instance) moveSecretCrossVault(
 	cmd *cobra.Command,
 	srcVaultName, srcSecretName,

internal/cli/secrets_size_test.go

@@ -17,11 +17,51 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 // testVaultName is the vault name used by the size tests.
 const testVaultName = "test-vault"
 
+// lockedBytesPerSecretByte bounds the locked memory that storing a secret
+// holds at once: the buffers it is read into reach up to 1.5 times its
+// size, and they are then copied into one more buffer of its size.
+const lockedBytesPerSecretByte = 3
+
+// skipIfLockedMemoryTooLow skips the test when this process cannot lock
+// the memory a secret of size bytes needs, found by locking a buffer of
+// that size and releasing it. memguard panics, ending the whole test run,
+// when it cannot lock a buffer, and a plain `docker build .` runs the
+// tests under an 8 MiB locked-memory limit (RLIMIT_MEMLOCK). A process
+// allowed to lock past that limit runs every case.
+func skipIfLockedMemoryTooLow(t *testing.T, size int) {
+	t.Helper()
+
+	need := lockedBytesPerSecretByte * size
+
+	buf, err := unix.Mmap(-1, 0, need,
+		unix.PROT_READ|unix.PROT_WRITE, unix.MAP_PRIVATE|unix.MAP_ANON)
+	require.NoError(t, err)
+
+	lockErr := unix.Mlock(buf)
+
+	// Unmapping the buffer also unlocks it.
+	err = unix.Munmap(buf)
+	require.NoError(t, err)
+
+	if lockErr != nil {
+		var limit unix.Rlimit
+
+		err = unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit)
+		require.NoError(t, err)
+
+		t.Skipf("a %d-byte secret needs up to %d bytes of locked memory, "+
+			"which could not be locked under the locked-memory limit "+
+			"(RLIMIT_MEMLOCK) of %d bytes: %v",
+			size, need, limit.Cur, lockErr)
+	}
+}
+
 // newSizeTestVault creates an in-memory vault unlocked with the test
 // mnemonic and returns the filesystem and vault.
 //
@@ -59,6 +99,7 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) {
 // verifies the outcome.
 func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
 	t.Helper()
+	skipIfLockedMemoryTooLow(t, size)
 
 	fs, vlt := newSizeTestVault(t)
 
@@ -110,6 +151,7 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
 // verifies the outcome.
 func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
 	t.Helper()
+	skipIfLockedMemoryTooLow(t, size)
 
 	fs, vlt := newSizeTestVault(t)
 
@@ -300,6 +342,8 @@ func TestAddSecretBufferGrowth(t *testing.T) {
 
 	for _, size := range sizes {
 		t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) {
+			skipIfLockedMemoryTooLow(t, size)
+
 			fs, vlt := newSizeTestVault(t)
 
 			// Create test data of exactly the specified size

internal/cli/unlockers.go

@@ -49,7 +49,6 @@ var (
 		"is already added as an unlocker")
 	errUnsupportedUnlockerType = errors.New("unsupported unlocker type")
 	errLastUnlocker            = errors.New("refusing to remove last unlocker")
-	errUnlockerExists          = errors.New("unlocker already exists")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -346,9 +345,10 @@ func unlockerIDFromDir(
 // stored metadata matches the given type and creation time and returns
 // the matching unlocker's ID. It returns ("", nil) when the directory is
 // readable but holds no match, and a non-nil error when the directory
-// itself cannot be read. Callers must distinguish the two: an unreadable
+// itself cannot be read, which means the unlocker's ID cannot be known.
-// directory means the unlocker's real ID is unknowable, so the entry has
+// `unlocker list` then skips the entry rather than show a made-up ID; the
-// to be skipped rather than reported under a synthesized ID.
+// duplicate check before adding an unlocker must stop instead, because
+// the skipped entry may be the duplicate.
 func findUnlockerIDByMetadata(
 	fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
 	includeSecureEnclave bool,
@@ -691,8 +691,15 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	// Check if this GPG key is already added
 	expectedID := "pgp-" + fingerprint
 
-	err = cli.checkUnlockerExists(vlt, expectedID)
+	exists, err := cli.checkUnlockerExists(vlt, expectedID)
 	if err != nil {
+		return fmt.Errorf(
+			"could not check whether GPG key %s is already an unlocker: %w",
+			gpgKeyID, err,
+		)
+	}
+
+	if exists {
 		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
 	}
 
@@ -772,44 +779,38 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
-// checkUnlockerExists checks if an unlocker with the given ID exists
+// checkUnlockerExists reports whether the vault already has an unlocker
-func (cli *Instance) checkUnlockerExists(vlt *vault.Vault, unlockerID string) error {
+// with the given ID. It returns an error, and no answer, when unlockers.d
-	// Get the list of unlockers and check if any match the ID
+// cannot be read; the caller must then not create the unlocker.
-	unlockers, err := vlt.ListUnlockers()
+func (cli *Instance) checkUnlockerExists(
-	if err != nil {
+	vlt *vault.Vault, unlockerID string,
-		secret.Warn("Could not list unlockers during duplicate check", "error", err)
+) (bool, error) {
-
-		return nil // If we can't list unlockers, assume it doesn't exist
-	}
-
-	// Get vault directory to construct unlocker instances
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		secret.Warn("Could not get vault directory during duplicate check",
+		return false, fmt.Errorf("failed to get vault directory: %w", err)
-			"error", err)
-
-		return nil
 	}
 
-	// Check each unlocker's ID
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
+	unlockers, err := vlt.ListUnlockers()
+	if err != nil {
+		return false, fmt.Errorf(
+			"failed to list unlockers in %s: %w", unlockersDir, err,
+		)
+	}
+
 	for _, metadata := range unlockers {
 		// Construct the unlocker matching this metadata to get its ID
 		id, err := findUnlockerIDByMetadata(cli.fs, unlockersDir, metadata, true)
 		if err != nil {
-			secret.Warn(
+			// Unlike `unlocker list`, never skip here: a skipped entry may be the duplicate.
-				"Could not read unlockers directory during duplicate check, "+
+			return false, err
-					"skipping unlocker",
-				"unlockers_dir", unlockersDir, "error", err)
-
-			continue
 		}
 
 		if id != "" && id == unlockerID {
-			return errUnlockerExists
+			return true, nil
 		}
 	}
 
-	return nil
+	return false, nil
 }

internal/cli/unreadable_dir_test.go

@@ -0,0 +1,262 @@
+// Unreadable Directory Tests
+//
+// The checks that guard adding a PGP unlocker (is this key already an
+// unlocker?), removing the last unlocker and removing a vault (does the
+// vault hold secrets?), and importing a mnemonic (does the vault already
+// have a long-term key?) each look at the vault on disk before acting.
+// When that look fails they must refuse to act, not read the failure as
+// "nothing there" and go ahead.
+
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"context"
+	"errors"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"github.com/spf13/afero"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// unreadableTestGPGUserID is the user ID of the throwaway GPG key the
+	// PGP unlocker tests generate, and the --keyid they pass.
+	unreadableTestGPGUserID = "unlocker-test@example.com"
+
+	// unreadableTestSecretName is the secret stored in the vaults the
+	// removal tests remove from.
+	unreadableTestSecretName = "api-key"
+
+	// unreadableTestOtherVault is a second vault for the vault removal
+	// test, since the last vault can never be removed.
+	unreadableTestOtherVault = "work"
+
+	// unreadableTestSecretsDirName is the directory holding a vault's
+	// secrets, and unreadableTestCurrentFileName the per-secret file
+	// naming its current version.
+	unreadableTestSecretsDirName  = "secrets.d"
+	unreadableTestCurrentFileName = "current"
+)
+
+// errStatFailed is returned by statFailFs in place of a successful stat.
+var errStatFailed = errors.New("input/output error")
+
+// statFailFs fails every Stat of one path, as an I/O or permission error
+// on that path would.
+type statFailFs struct {
+	afero.Fs
+
+	path string
+}
+
+func (f *statFailFs) Stat(name string) (os.FileInfo, error) {
+	if name == f.path {
+		return nil, errStatFailed
+	}
+
+	return f.Fs.Stat(name)
+}
+
+// testVaultDir returns the directory of the named vault in the synthetic
+// state directory built by newListTestVault.
+func testVaultDir(vaultName string) string {
+	return filepath.Join(listTestStateDir, "vaults.d", vaultName)
+}
+
+// newTestInstance returns a CLI instance on fs whose output is discarded.
+func newTestInstance(fs afero.Fs) (*Instance, *cobra.Command) {
+	cmd := &cobra.Command{}
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+
+	return &Instance{fs: fs, stateDir: listTestStateDir, cmd: cmd}, cmd
+}
+
+// assertDirEntries asserts that dir holds exactly the named entries.
+func assertDirEntries(t *testing.T, fs afero.Fs, dir string, want ...string) {
+	t.Helper()
+
+	entries, err := afero.ReadDir(fs, dir)
+	require.NoError(t, err)
+
+	names := make([]string, 0, len(entries))
+	for _, entry := range entries {
+		names = append(names, entry.Name())
+	}
+
+	assert.ElementsMatch(t, want, names)
+}
+
+// newTestGPGKey points GNUPGHOME at a fresh directory, generates a GPG key
+// without a passphrase there, and returns the key's fingerprint.
+func newTestGPGKey(t *testing.T) string {
+	t.Helper()
+
+	t.Setenv("GNUPGHOME", t.TempDir())
+
+	t.Cleanup(func() {
+		// Stop the gpg-agent that key generation starts. t.Context is
+		// already canceled when cleanup runs.
+		ctx := context.WithoutCancel(t.Context())
+		_ = exec.CommandContext(ctx, "gpgconf", "--kill", "gpg-agent").Run()
+	})
+
+	output, err := exec.CommandContext(t.Context(), "gpg", "--batch",
+		"--pinentry-mode", "loopback", "--passphrase", "",
+		"--quick-gen-key", unreadableTestGPGUserID, "ed25519", "sign", "never",
+	).CombinedOutput()
+	require.NoError(t, err, "generating the test GPG key: %s", output)
+
+	fingerprint, err := secret.ResolveGPGKeyFingerprint(unreadableTestGPGUserID)
+	require.NoError(t, err)
+
+	return fingerprint
+}
+
+// addTestPGPUnlocker runs `secret unlocker add pgp` for the test key
+// against fs.
+func addTestPGPUnlocker(fs afero.Fs) error {
+	instance, cmd := newTestInstance(fs)
+	cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
+
+	return instance.addPGPUnlocker(cmd)
+}
+
+// TestAddPGPUnlockerDuplicateCheck asserts that adding a PGP unlocker
+// fails, and creates no unlocker directory, when unlockers.d cannot be
+// read for the duplicate check; and, as the control case, that a readable
+// unlockers.d holding the same key is still refused as a duplicate.
+//
+//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
+func TestAddPGPUnlockerDuplicateCheck(t *testing.T) {
+	fingerprint := newTestGPGKey(t)
+	unlockersDir := filepath.Join(
+		testVaultDir(listTestVaultName), listTestUnlockersDirName)
+
+	tests := []struct {
+		name       string
+		openBudget int
+	}{
+		// The vault's own enumeration of unlockers.d fails.
+		{name: "listing fails", openBudget: 0},
+		// The enumeration succeeds; the rescan that resolves IDs fails.
+		{name: "rescan fails", openBudget: 1},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			base := newListTestVault(t, 1)
+			fs := &unlockersDirFailFs{Fs: base, openBudget: tt.openBudget}
+
+			err := addTestPGPUnlocker(fs)
+
+			require.ErrorIs(t, err, errUnlockersDirUnreadable)
+			require.NotErrorIs(t, err, errGPGKeyAlreadyUnlocker)
+			assert.Contains(t, err.Error(), unlockersDir,
+				"the error must name the directory it could not read")
+			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
+		})
+	}
+
+	t.Run("duplicate refused", func(t *testing.T) {
+		base := newListTestVault(t, 1)
+		writePGPUnlocker(t, base, unlockersDir, listTestUnlockerDirTwo,
+			time.Date(2026, time.August, 10, 12, 30, 0, 0, time.UTC),
+			fingerprint)
+
+		err := addTestPGPUnlocker(base)
+
+		require.ErrorIs(t, err, errGPGKeyAlreadyUnlocker)
+		assertDirEntries(t, base, unlockersDir,
+			listTestUnlockerDirOne, listTestUnlockerDirTwo)
+	})
+}
+
+// writeTestSecret stores a secret with a current-version pointer, which is
+// what makes it count as a secret, in the given vault directory.
+func writeTestSecret(t *testing.T, fs afero.Fs, vaultDir string) {
+	t.Helper()
+
+	secretDir := filepath.Join(
+		vaultDir, unreadableTestSecretsDirName, unreadableTestSecretName)
+	require.NoError(t, fs.MkdirAll(secretDir, listTestDirPerm))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(secretDir, unreadableTestCurrentFileName),
+		[]byte("20260809.001"), listTestFilePerm))
+}
+
+// TestRemoveLastUnlockerAbortsWhenSecretsUnreadable asserts that the last
+// unlocker is kept when the secrets it protects cannot be counted.
+func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
+	t.Parallel()
+
+	vaultDir := testVaultDir(listTestVaultName)
+	unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
+	secretsDir := filepath.Join(vaultDir, unreadableTestSecretsDirName)
+
+	for _, path := range []string{
+		secretsDir,
+		filepath.Join(secretsDir, unreadableTestSecretName,
+			unreadableTestCurrentFileName),
+	} {
+		t.Run(filepath.Base(path), func(t *testing.T) {
+			t.Parallel()
+
+			base := newListTestVault(t, 1)
+			writeTestSecret(t, base, vaultDir)
+			instance, cmd := newTestInstance(&statFailFs{Fs: base, path: path})
+
+			err := instance.UnlockersRemove(
+				"pgp-"+listTestGPGKeyID+"A", false, cmd)
+
+			require.ErrorIs(t, err, errStatFailed)
+			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
+		})
+	}
+}
+
+// TestRemoveVaultAbortsWhenSecretsDirUnreadable asserts that a vault is
+// kept when whether it holds secrets cannot be determined.
+func TestRemoveVaultAbortsWhenSecretsDirUnreadable(t *testing.T) {
+	t.Parallel()
+
+	base := newListTestVault(t, 1)
+	vaultDir := testVaultDir(unreadableTestOtherVault)
+	writeTestSecret(t, base, vaultDir)
+	instance, cmd := newTestInstance(&statFailFs{
+		Fs: base, path: filepath.Join(vaultDir, unreadableTestSecretsDirName),
+	})
+
+	err := instance.RemoveVault(cmd, unreadableTestOtherVault, false)
+
+	require.ErrorIs(t, err, errStatFailed)
+
+	exists, err := afero.DirExists(base, vaultDir)
+	require.NoError(t, err)
+	assert.True(t, exists, "the vault must not be removed")
+}
+
+// TestVaultImportAbortsWhenPubKeyUnreadable asserts that a mnemonic import
+// stops when whether the vault already has a long-term key cannot be
+// determined.
+func TestVaultImportAbortsWhenPubKeyUnreadable(t *testing.T) {
+	t.Parallel()
+
+	base := newListTestVault(t, 1)
+	instance, cmd := newTestInstance(&statFailFs{
+		Fs: base, path: filepath.Join(testVaultDir(listTestVaultName), "pub.age"),
+	})
+
+	err := instance.VaultImport(cmd, listTestVaultName)
+
+	require.ErrorIs(t, err, errStatFailed)
+}

internal/cli/vault.go

@@ -388,8 +388,12 @@ func (cli *Instance) vaultImportPreflight(
 	// Check if vault already has a public key
 	pubKeyPath := vaultDir + "/pub.age"
 
-	_, err = cli.fs.Stat(pubKeyPath)
+	exists, err = afero.Exists(cli.fs, pubKeyPath)
-	if err == nil {
+	if err != nil {
+		return "", "", "", fmt.Errorf("failed to check %s: %w", pubKeyPath, err)
+	}
+
+	if exists {
 		return "", "", "", fmt.Errorf("vault '%s' %w",
 			vaultName, errVaultHasLongTermKey)
 	}
@@ -536,17 +540,26 @@ func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
 }
 
 // vaultHasSecrets reports whether the vault directory contains any secrets
-func (cli *Instance) vaultHasSecrets(vaultDir string) bool {
+func (cli *Instance) vaultHasSecrets(vaultDir string) (bool, error) {
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
-	exists, _ := afero.DirExists(cli.fs, secretsDir)
+	exists, err := afero.DirExists(cli.fs, secretsDir)
+	if err != nil {
+		return false, fmt.Errorf("failed to check secrets directory %s: %w",
+			secretsDir, err)
+	}
+
 	if !exists {
-		return false
+		return false, nil
 	}
 
 	entries, err := afero.ReadDir(cli.fs, secretsDir)
+	if err != nil {
+		return false, fmt.Errorf("failed to read secrets directory %s: %w",
+			secretsDir, err)
+	}
 
-	return err == nil && len(entries) > 0
+	return len(entries) > 0, nil
 }
 
 // switchAwayFromVault selects another vault as current before removal
@@ -610,7 +623,10 @@ func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) er
 	}
 
 	// Check if vault has secrets
-	hasSecrets := cli.vaultHasSecrets(vaultDir)
+	hasSecrets, err := cli.vaultHasSecrets(vaultDir)
+	if err != nil {
+		return err
+	}
 
 	// Require --force if vault has secrets
 	if hasSecrets && !force {

internal/cli/version.go

@@ -112,6 +112,11 @@ func VersionCommands(cli *Instance) *cobra.Command {
 func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
 	secret.Debug("ListVersions called", "secret_name", secretName)
 
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -239,6 +244,11 @@ func formatVersionTime(t *time.Time) string {
 func (cli *Instance) PromoteVersion(
 	cmd *cobra.Command, secretName string, version string,
 ) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -255,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 := secret.VersionExists(cli.fs, secretDir, version)
-
-	exists, err := afero.DirExists(cli.fs, versionDir)
 	if err != nil {
 		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
@@ -282,6 +290,11 @@ func (cli *Instance) PromoteVersion(
 func (cli *Instance) RemoveVersion(
 	cmd *cobra.Command, secretName string, version string,
 ) error {
+	err := vault.ValidateSecretName(secretName)
+	if err != nil {
+		return err
+	}
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -308,9 +321,7 @@ func (cli *Instance) RemoveVersion(
 	}
 
 	// Check if version exists
-	versionDir := filepath.Join(secretDir, "versions", version)
+	exists, err = secret.VersionExists(cli.fs, secretDir, version)
-
-	exists, err = afero.DirExists(cli.fs, versionDir)
 	if err != nil {
 		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
@@ -333,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)
+	// Test getting the current version
-	err = cli.GetSecretWithVersion(cmd, "test/secret", "")
+	err = cli.GetSecret(cmd, "test/secret")
 	require.NoError(t, err)
 	assert.Equal(t, "version-2", buf.String())
 

internal/secret/helpers_darwin.go

@@ -1,29 +0,0 @@
-//go:build darwin
-
-package secret
-
-import (
-	"crypto/rand"
-	"fmt"
-	"math/big"
-)
-
-// generateRandomString generates a random string of the specified length using the given character set
-func generateRandomString(length int, charset string) (string, error) {
-	if length <= 0 {
-		return "", fmt.Errorf("length must be positive")
-	}
-
-	result := make([]byte, length)
-	charsetLen := big.NewInt(int64(len(charset)))
-
-	for i := range length {
-		randomIndex, err := rand.Int(rand.Reader, charsetLen)
-		if err != nil {
-			return "", fmt.Errorf("failed to generate random number: %w", err)
-		}
-		result[i] = charset[randomIndex.Int64()]
-	}
-
-	return string(result), nil
-}

internal/secret/keychaindata.go

@@ -0,0 +1,142 @@
+package secret
+
+import (
+	"bytes"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/awnumar/memguard"
+)
+
+var (
+	errPassphraseLength = errors.New(
+		"passphrase length must be a positive even number")
+	errPassphraseNotHex = errors.New(
+		"keychain passphrase must be lowercase hex")
+	errNoKeychainPassphrase = errors.New(
+		"keychain data has no agePrivKeyPassphrase string")
+)
+
+// KeychainData is what a keychain unlocker stores in the macOS keychain.
+// It is stored as JSON, but encode and decodeKeychainData keep the
+// passphrase out of encoding/json, which would leave copies of it in
+// ordinary memory.
+type KeychainData struct {
+	AgePublicKey         string
+	AgePrivKeyPassphrase *memguard.LockedBuffer
+	EncryptedLongtermKey string
+}
+
+// generateRandomPassphrase returns length random lowercase hex characters
+// in a locked buffer. The caller must destroy it.
+func generateRandomPassphrase(length int) (*memguard.LockedBuffer, error) {
+	// Each random byte becomes two hex characters.
+	randomBytes := hex.DecodedLen(length)
+	if length <= 0 || hex.EncodedLen(randomBytes) != length {
+		return nil, errPassphraseLength
+	}
+
+	random := memguard.NewBufferRandom(randomBytes)
+	defer random.Destroy()
+
+	passphrase := memguard.NewBuffer(length)
+	hex.Encode(passphrase.Bytes(), random.Bytes())
+	passphrase.Freeze()
+
+	return passphrase, nil
+}
+
+// encode returns d as JSON in a locked buffer:
+// {"agePublicKey":"...","agePrivKeyPassphrase":"...","encryptedLongtermKey":"..."}.
+// The passphrase is copied straight into the buffer, so it must be hex,
+// which JSON does not escape. The caller must destroy the returned buffer.
+func (d *KeychainData) encode() (*memguard.LockedBuffer, error) {
+	if d.AgePrivKeyPassphrase == nil {
+		return nil, errNilPassphraseBuffer
+	}
+
+	if d.AgePrivKeyPassphrase.Size() == 0 {
+		return nil, errEmptyPassphrase
+	}
+
+	for _, c := range d.AgePrivKeyPassphrase.Bytes() {
+		if strings.IndexByte("0123456789abcdef", c) < 0 {
+			return nil, errPassphraseNotHex
+		}
+	}
+
+	publicKey, err := json.Marshal(d.AgePublicKey)
+	if err != nil {
+		return nil, fmt.Errorf("failed to encode age public key: %w", err)
+	}
+
+	longtermKey, err := json.Marshal(d.EncryptedLongtermKey)
+	if err != nil {
+		return nil, fmt.Errorf("failed to encode long-term key: %w", err)
+	}
+
+	parts := [][]byte{
+		[]byte(`{"agePublicKey":`), publicKey,
+		[]byte(`,"agePrivKeyPassphrase":"`), d.AgePrivKeyPassphrase.Bytes(),
+		[]byte(`","encryptedLongtermKey":`), longtermKey,
+		[]byte(`}`),
+	}
+
+	size := 0
+	for _, part := range parts {
+		size += len(part)
+	}
+
+	encoded := memguard.NewBuffer(size)
+
+	written := 0
+	for _, part := range parts {
+		written += copy(encoded.Bytes()[written:], part)
+	}
+
+	encoded.Freeze()
+
+	return encoded, nil
+}
+
+// decodeKeychainData parses keychain data written by encode. The caller
+// must destroy the returned AgePrivKeyPassphrase.
+func decodeKeychainData(data *memguard.LockedBuffer) (*KeychainData, error) {
+	if data == nil {
+		return nil, errNilDataBuffer
+	}
+
+	// json.Unmarshal gives a json.RawMessage field the field's JSON text
+	// unchanged, in the one copy RawMessage makes; it is wiped on return.
+	var fields struct {
+		AgePublicKey         string          `json:"agePublicKey"`
+		AgePrivKeyPassphrase json.RawMessage `json:"agePrivKeyPassphrase"`
+		EncryptedLongtermKey string          `json:"encryptedLongtermKey"`
+	}
+
+	defer func() { memguard.WipeBytes(fields.AgePrivKeyPassphrase) }()
+
+	err := json.Unmarshal(data.Bytes(), &fields)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse keychain data: %w", err)
+	}
+
+	// json.Unmarshal accepted the JSON, so text that starts with a quote is
+	// a whole string. The passphrase is hex, so it is the text between the
+	// quotes.
+	quoted := fields.AgePrivKeyPassphrase
+	if !bytes.HasPrefix(quoted, []byte(`"`)) {
+		return nil, errNoKeychainPassphrase
+	}
+
+	return &KeychainData{
+		AgePublicKey: fields.AgePublicKey,
+		// NewBufferFromBytes wipes the bytes it copies.
+		AgePrivKeyPassphrase: memguard.NewBufferFromBytes(
+			quoted[1 : len(quoted)-1]),
+		EncryptedLongtermKey: fields.EncryptedLongtermKey,
+	}, nil
+}

internal/secret/keychaindata_test.go

@@ -0,0 +1,118 @@
+//nolint:testpackage // white-box test of unexported internals
+package secret
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/awnumar/memguard"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGenerateRandomPassphrase(t *testing.T) {
+	t.Parallel()
+
+	first, err := generateRandomPassphrase(64)
+	require.NoError(t, err)
+
+	defer first.Destroy()
+
+	second, err := generateRandomPassphrase(64)
+	require.NoError(t, err)
+
+	defer second.Destroy()
+
+	assert.Regexp(t, `^[0-9a-f]{64}$`, first.String())
+	assert.NotEqual(t, first.String(), second.String())
+	assert.False(t, first.IsMutable())
+
+	for _, length := range []int{0, -2, 63} {
+		_, err := generateRandomPassphrase(length)
+		require.ErrorIs(t, err, errPassphraseLength, "length %d", length)
+	}
+}
+
+func TestKeychainDataEncodeDecode(t *testing.T) {
+	t.Parallel()
+
+	passphrase := memguard.NewBufferFromBytes([]byte("0a1b2c3d"))
+	defer passphrase.Destroy()
+
+	data := KeychainData{
+		AgePublicKey:         "age1example",
+		AgePrivKeyPassphrase: passphrase,
+		EncryptedLongtermKey: "beef",
+	}
+
+	encoded, err := data.encode()
+	require.NoError(t, err)
+
+	defer encoded.Destroy()
+
+	assert.JSONEq(t,
+		`{"agePublicKey":"age1example",`+
+			`"agePrivKeyPassphrase":"0a1b2c3d",`+
+			`"encryptedLongtermKey":"beef"}`,
+		encoded.String())
+	assert.False(t, encoded.IsMutable())
+
+	decoded, err := decodeKeychainData(encoded)
+	require.NoError(t, err)
+
+	defer decoded.AgePrivKeyPassphrase.Destroy()
+
+	assert.Equal(t, "age1example", decoded.AgePublicKey)
+	assert.Equal(t, "0a1b2c3d", decoded.AgePrivKeyPassphrase.String())
+	assert.Equal(t, "beef", decoded.EncryptedLongtermKey)
+}
+
+func TestKeychainDataEncodeRejectsBadPassphrase(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		passphrase *memguard.LockedBuffer
+		wantErr    error
+	}{
+		{"nil", nil, errNilPassphraseBuffer},
+		{"empty", memguard.NewBuffer(0), errEmptyPassphrase},
+		{
+			"not hex",
+			memguard.NewBufferFromBytes([]byte(`abc"def`)),
+			errPassphraseNotHex,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			data := KeychainData{AgePrivKeyPassphrase: tt.passphrase}
+			_, err := data.encode()
+			require.ErrorIs(t, err, tt.wantErr)
+		})
+	}
+}
+
+func TestDecodeKeychainDataRejectsBadData(t *testing.T) {
+	t.Parallel()
+
+	for _, text := range []string{
+		`{"agePublicKey":"age1example"}`,
+		`{"agePrivKeyPassphrase":42}`,
+	} {
+		data := memguard.NewBufferFromBytes([]byte(text))
+		_, err := decodeKeychainData(data)
+		data.Destroy()
+		require.ErrorIs(t, err, errNoKeychainPassphrase, text)
+	}
+
+	notJSON := memguard.NewBufferFromBytes([]byte(`{"agePrivKeyPassphrase":`))
+	defer notJSON.Destroy()
+
+	_, err := decodeKeychainData(notJSON)
+
+	var syntaxError *json.SyntaxError
+	require.ErrorAs(t, err, &syntaxError)
+}

internal/secret/keychainunlocker.go

@@ -45,13 +45,6 @@ type KeychainUnlocker struct {
 	fs        afero.Fs
 }
 
-// KeychainData represents the data stored in the macOS keychain
-type KeychainData struct {
-	AgePublicKey         string `json:"agePublicKey"`
-	AgePrivKeyPassphrase string `json:"agePrivKeyPassphrase"`
-	EncryptedLongtermKey string `json:"encryptedLongtermKey"`
-}
-
 // GetIdentity implements Unlocker interface for Keychain-based unlockers
 func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
 	DebugWith("Getting keychain unlocker identity",
@@ -81,13 +74,18 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
 		slog.Int("data_length", len(keychainDataBytes)),
 	)
 
+	// Move the keychain data into locked memory; this wipes keychainDataBytes
+	keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
+	defer keychainDataBuffer.Destroy()
+
 	// Step 3: Parse keychain data
-	var keychainData KeychainData
+	keychainData, err := decodeKeychainData(keychainDataBuffer)
-	if err := json.Unmarshal(keychainDataBytes, &keychainData); err != nil {
+	if err != nil {
 		Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID())
 
 		return nil, fmt.Errorf("failed to parse keychain data: %w", err)
 	}
+	defer keychainData.AgePrivKeyPassphrase.Destroy()
 
 	Debug("Parsed keychain data successfully", "unlocker_id", k.GetID())
 
@@ -109,11 +107,7 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
 
 	// Step 5: Decrypt the age private key using the passphrase from keychain
 	Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID())
-	// Create secure buffer for the keychain passphrase
+	agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase)
-	passphraseBuffer := memguard.NewBufferFromBytes([]byte(keychainData.AgePrivKeyPassphrase))
-	defer passphraseBuffer.Destroy()
-
-	agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, passphraseBuffer)
 	if err != nil {
 		Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID())
 
@@ -369,6 +363,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate age private key passphrase: %w", err)
 	}
+	defer agePrivKeyPassphrase.Destroy()
 
 	// Step 3: Store age recipient as plaintext
 	ageRecipient := ageIdentity.Recipient().String()
@@ -378,15 +373,12 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 
 	// Step 4: Encrypt age private key with the generated passphrase and store on disk
-	// Create secure buffers for both the private key and passphrase
+	// Create a secure buffer for the private key
 	agePrivKeyStr := ageIdentity.String()
 	agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
 	defer agePrivKeyBuffer.Destroy()
 
-	passphraseBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyPassphrase))
+	encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, agePrivKeyPassphrase)
-	defer passphraseBuffer.Destroy()
-
-	encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, passphraseBuffer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
 	}
@@ -422,13 +414,10 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		EncryptedLongtermKey: hex.EncodeToString(encryptedLtPrivKeyToAge),
 	}
 
-	keychainDataBytes, err := json.Marshal(keychainData)
+	keychainDataBuffer, err := keychainData.encode()
 	if err != nil {
-		return nil, fmt.Errorf("failed to marshal keychain data: %w", err)
+		return nil, fmt.Errorf("failed to encode keychain data: %w", err)
 	}
-
-	// Create a secure buffer for keychain data
-	keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
 	defer keychainDataBuffer.Destroy()
 
 	// Step 8: Store data in keychain
@@ -501,7 +490,7 @@ func storeInKeychain(itemName string, data *memguard.LockedBuffer) error {
 	item.SetAccount(itemName)
 	item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName))
 	item.SetDescription("Secret vault keychain data")
-	item.SetData([]byte(data.String()))
+	item.SetData(data.Bytes())
 	item.SetSynchronizable(keychain.SynchronizableNo)
 	// Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth
 	item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly)
@@ -576,8 +565,3 @@ func deleteFromKeychain(itemName string) error {
 
 	return nil
 }
-
-// generateRandomPassphrase generates a random passphrase for encrypting the age private key
-func generateRandomPassphrase(length int) (string, error) {
-	return generateRandomString(length, "0123456789abcdef")
-}

internal/secret/secret_test.go

@@ -310,64 +310,6 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
 	})
 }
 
-// For testing purposes only
-func isValidSecretName(name string) bool {
-	if name == "" {
-		return false
-	}
-	// Valid characters for secret names: letters, numbers, dash, dot, underscore, slash
-	for _, char := range name {
-		if (char < 'a' || char > 'z') && // lowercase letters
-			(char < 'A' || char > 'Z') && // uppercase letters
-			(char < '0' || char > '9') && // numbers
-			char != '-' && // dash
-			char != '.' && // dot
-			char != '_' && // underscore
-			char != '/' { // slash
-			return false
-		}
-	}
-
-	return true
-}
-
-func TestSecretNameValidation(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name  string
-		valid bool
-	}{
-		{"valid-name", true},
-		{"valid.name", true},
-		{"valid_name", true},
-		{"valid/path/name", true},
-		{"123valid", true},
-		{"", false},
-		{"Valid-Upper-Name", true}, // uppercase allowed
-		{"2025-11-21-ber1app1-vaultik-test-bucket-AKI", true}, // real-world uppercase key ID
-		{"MixedCase/Path/Name", true},                         // mixed case with path
-		{"invalid name", false},                               // space not allowed
-		{"invalid@name", false},                               // @ not allowed
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			result := isValidSecretName(test.name)
-			if result != test.valid {
-				t.Errorf(
-					"isValidSecretName(%q) = %v, want %v",
-					test.name,
-					result,
-					test.valid,
-				)
-			}
-		})
-	}
-}
-
 func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) {
 	// This test demonstrates the bug where GetValue uses hardcoded index 0
 	// instead of the vault's actual derivation index when using environment mnemonic

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

@@ -29,10 +29,11 @@ var (
 	// ErrNilValueBuffer indicates a nil value buffer was supplied.
 	ErrNilValueBuffer = errors.New("value buffer is nil")
 
-	// ErrInvalidSecretName indicates a secret name that does not match
+	// ErrInvalidSecretName indicates a secret name that breaks the naming
-	// the allowed pattern [a-z0-9.\-_/]+. Composed as
+	// rule: only ASCII letters, digits, '.', '-', '_' and '/'; not empty;
-	// "invalid secret name '<name>': must match pattern [a-z0-9.\-_/]+",
+	// no leading '.' or '/', no trailing '/', no '//', no '..' path segment.
-	// or as "invalid secret name: <name>" by GetSecretObject.
+	// Composed by ValidateSecretName as
+	// "invalid secret name '<name>': <the rule>".
 	ErrInvalidSecretName = errors.New("invalid secret name")
 
 	// ErrSecretExists indicates the secret already exists and --force
@@ -48,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
+	// An empty version is not one of the versions; GetSecret gets the
-	valueCurrent, err := vault.GetSecretVersion(secretName, "")
+	// current one
-	require.NoError(t, err)
+	_, err = vault.GetSecretVersion(secretName, "")
-	assert.Equal(t, []byte("version-3-data"), valueCurrent)
+	require.ErrorIs(t, err, ErrVersionNotFound)
 }
 
 func testPromoteOldVersion(

internal/vault/secrets.go

@@ -79,6 +79,7 @@ func (v *Vault) ListSecrets() ([]string, error) {
 // - No leading or trailing slashes
 // - No double slashes
 // - No names starting with dots
+// - No ".." path segments
 func isValidSecretName(name string) bool {
 	if name == "" {
 		return false
@@ -110,6 +111,22 @@ func isValidSecretName(name string) bool {
 	return matched
 }
 
+// ValidateSecretName returns an error wrapping ErrInvalidSecretName when
+// name is not a valid secret name. Call it on the name exactly as the user
+// gave it, before building any path from it.
+func ValidateSecretName(name string) error {
+	if !isValidSecretName(name) {
+		return fmt.Errorf(
+			"%w '%s': only ASCII letters, digits, '.', '-', '_' and '/' are allowed, "+
+				"and a name must not be empty, start with '.' or '/', end with '/', "+
+				"contain '//', or have '..' as a path segment",
+			ErrInvalidSecretName, name,
+		)
+	}
+
+	return nil
+}
+
 // AddSecret adds a secret to this vault
 func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error {
 	if value == nil {
@@ -124,13 +141,11 @@ func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool)
 	)
 
 	// Validate secret name
-	if !isValidSecretName(name) {
+	err := ValidateSecretName(name)
+	if err != nil {
 		secret.Debug("Invalid secret name provided", "secret_name", name)
 
-		return fmt.Errorf(
+		return err
-			"%w '%s': must match pattern [a-z0-9.\\-_/]+",
-			ErrInvalidSecretName, name,
-		)
 	}
 
 	secret.Debug("Secret name validation passed", "secret_name", name)
@@ -244,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
 	}
 
-// GetSecretVersion retrieves a specific version of a secret (empty version
+	currentVersion, err := secret.GetCurrentVersion(v.fs, secretObj.Directory)
-// means current)
+	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. 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),
@@ -263,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
+	// Validate the name and check that the version exists
-	version, err := v.resolveSecretVersion(name, version)
+	err := v.checkSecretVersion(name, version)
 	if err != nil {
 		return nil, err
 	}
@@ -358,8 +386,9 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
 
 // GetSecretObject retrieves a Secret object with metadata loaded from this vault
 func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
-	if !isValidSecretName(name) {
+	err := ValidateSecretName(name)
-		return nil, fmt.Errorf("%w: %s", ErrInvalidSecretName, name)
+	if err != nil {
+		return nil, err
 	}
 
 	// First check if the secret exists by checking for the metadata file
@@ -624,17 +653,15 @@ func (v *Vault) updatePreviousVersion(
 	return nil
 }
 
-// resolveSecretVersion validates the secret name, verifies the secret and
+// checkSecretVersion validates the secret name and verifies that the secret
-// version exist, and resolves an empty version to the current one.
+// exists and that version is one of its versions.
-func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
+func (v *Vault) checkSecretVersion(name, version string) error {
 	// Validate secret name to prevent path traversal
-	if !isValidSecretName(name) {
+	err := ValidateSecretName(name)
+	if err != nil {
 		secret.Debug("Invalid secret name provided", "secret_name", name)
 
-		return "", fmt.Errorf(
+		return err
-			"%w '%s': must match pattern [a-z0-9.\\-_/]+",
-			ErrInvalidSecretName, name,
-		)
 	}
 
 	// Get vault directory
@@ -642,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
@@ -654,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)
+		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)
 	}
 
 	// Check if version exists
-	versionPath := filepath.Join(secretDir, "versions", version)
+	exists, err = secret.VersionExists(v.fs, secretDir, version)
-
-	exists, err = afero.DirExists(v.fs, versionPath)
 	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(
+		return fmt.Errorf("version '%s' %w '%s'", version, ErrVersionNotFound, name)
-			"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)
+	// An empty version is not one of the versions; GetSecret gets the
-	value, err = vault.GetSecretVersion(testSecretPath, "")
+	// current one
-	require.NoError(t, err)
+	_, err = vault.GetSecretVersion(testSecretPath, "")
-	assert.Equal(t, []byte("version-2"), value)
+	require.ErrorIs(t, err, ErrVersionNotFound)
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv

internal/vault/vault.go

@@ -138,7 +138,12 @@ func (v *Vault) NumSecrets() (int, error) {
 
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
-	exists, _ := afero.DirExists(v.fs, secretsDir)
+	exists, err := afero.DirExists(v.fs, secretsDir)
+	if err != nil {
+		return 0, fmt.Errorf("failed to check secrets directory %s: %w",
+			secretsDir, err)
+	}
+
 	if !exists {
 		return 0, nil
 	}
@@ -162,7 +167,7 @@ func (v *Vault) NumSecrets() (int, error) {
 
 		exists, err := afero.Exists(v.fs, currentFile)
 		if err != nil {
-			continue // Skip directories we can't read
+			return 0, fmt.Errorf("failed to check %s: %w", currentFile, err)
 		}
 
 		if exists {

script/cibuild

@@ -1,8 +1,9 @@
 #!/bin/sh
 # script/cibuild: run the CI build. The Dockerfile runs script/check
 # (via make check), so a successful build implies all checks pass.
-# The Gitea workflow runs this on push. The memlock ulimit is required
+# The Gitea workflow runs this on push. The memlock ulimit lets the tests
-# because the test suite uses memguard, which mlocks memory.
+# that lock large secrets in memory (memguard mlocks them) run; under the
+# lower limit of a plain `docker build .` they are skipped.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"

script/docker

@@ -1,7 +1,8 @@
 #!/bin/sh
 # script/docker: build the Docker image tagged with the project name.
 # Identical in all repos; the tag comes from script/projectname.
-# Generic: needs no adaptation.
+# --no-cache because the gate phases the final stage depends on are RUN
+# steps, and a cached one is a check that did not run.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -9,7 +10,15 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build -t "$("$SCRIPT_DIR/projectname")" .
+    # Own line: a failing command substitution inside an argument does
+    # not trip `set -e`, so the inline form degrades silently to an
+    # empty constant. The VERSION build argument takes precedence over
+    # the version a build stage derives from the .git in the context.
+    version="$(git describe --tags --always --dirty 2>/dev/null || true)"
+    [ -n "$version" ] || version="unknown"
+    docker build --no-cache \
+        --build-arg VERSION="$version" \
+        -t "$("$SCRIPT_DIR/projectname")" .
 }
 
 main "$@"