Print usage only for a command called wrongly (closes #41)

A failed command printed the whole usage text after its error, burying
it. The root command's PersistentPreRunE now turns usage off once cobra
has checked the arguments and flags, so an error from running the
command is printed once on its own. Wrong arity, an unknown flag, a bad
flag value and a missing required flag still get usage; cobra checks
required flags after that hook, so the hook checks them first. Root
SilenceUsage was not used: in this cobra version it hides usage for
argument and flag errors too. Cobra still prints the error; Entry is
unchanged.

Model: opus-5-5

.dockerignore

@@ -16,6 +16,9 @@ coverage.out
 *.swo
 *~
 
+# Dependencies
+node_modules
+
 # macOS
 .DS_Store
 
@@ -23,4 +26,4 @@ coverage.out
 .claude/
 
 # Local settings
-.claude/settings.local.json
+.claude/settings.local.json

.gitignore

@@ -1,12 +1,34 @@
+# OS
 .DS_Store
-**/.DS_Store
+Thumbs.db
+
+# Editors
+*.swp
+*.swo
+*~
+*.bak
+.idea/
+.vscode/
+*.sublime-*
+
+# Agent scratch (worktrees of this repo, created and destroyed by
+# in-flight tooling). Unanchored: .gitignore patterns already match at
+# every depth, so no prefix is wanted here. This is not a .dockerignore
+# entry and must not be given a `**/` prefix on the way into one.
+.claude/
+
+# Node
+node_modules/
+
+# Environment / secrets
+.env
+.env.*
+*.pem
+*.key
+
+# This repo. /secret is the built binary, anchored so that it does not
+# also match the internal/secret/ package directory.
 /secret
 *.log
-cli.test
-vault.test
 *.test
 settings.local.json
-
-# Stale files
-.cursorrules
-coverage.out

Dockerfile

@@ -9,7 +9,8 @@ RUN go mod download
 COPY . .
 
 RUN make fmt-check
-RUN make lint
+# Not make lint: script/lint is a docker build, which cannot run in here.
+RUN golangci-lint run --config .golangci.yml ./...
 
 # Build stage — tests and compilation
 # golang 1.24.13-alpine (2026-03-10)

Dockerfile.lint

@@ -0,0 +1,19 @@
+# Lint image, built by script/lint: golangci-lint runs as a build step, so a
+# successful build is a clean lint. Works where the docker daemon is remote
+# and bind mounts are impossible.
+
+# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
+FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
+
+WORKDIR /src
+
+COPY go.mod go.sum ./
+RUN go mod download
+
+# script/lint rebuilds this stage on every run, by this name; the module
+# download above stays cached.
+FROM deps AS lint
+
+COPY . .
+
+RUN golangci-lint run --config .golangci.yml ./...

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`
 
@@ -137,6 +139,9 @@ matching.
 
 Moves or renames a secret within the current vault.
 - Fails if the destination already exists
+- Fails if the destination is the source under another name, such as `foo`
+  for `Foo` on a case-insensitive filesystem (the macOS default); there, to
+  change only the case of a name, move the secret to a third name first
 - Preserves all versions and metadata
 
 ### Version Management
@@ -494,15 +499,18 @@ standard: normalized scripts in `script/` are the entrypoints for the
 development workflow, and the Makefile targets are thin shims that call
 them. We provide:
 
-- `script/bootstrap` — install all dependencies (Go, golangci-lint, Go
-  module download), idempotently
+- `script/bootstrap` — install all dependencies (Go, Go module
+  download), idempotently; golangci-lint is not installed, it runs in
+  docker
 - `script/setup` — make a fresh clone ready for development: runs
   `script/bootstrap`, then `script/install-precommit`
 - `script/projectname` — output the project name (`secret`); used by
   other scripts such as `script/docker`
 - `script/test` — run `go vet` and the test suite (verbose rerun on
   failure)
-- `script/lint` — run `golangci-lint`
+- `script/lint` — run `golangci-lint` in docker only: builds
+  `Dockerfile.lint`, where the linter is a build step that runs on every
+  call, also on an unchanged tree
 - `script/fmt` — format all Go code (writes)
 - `script/fmt-check` — check formatting without writing
 - `script/check` — run `script/test`, `script/lint`, and

TODO.md

@@ -25,6 +25,64 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-04: A failed command prints its error once, without the usage
+  text after it (https://git.eeqj.de/sneak/secret/issues/41). Usage is
+  still printed for a command called wrongly: wrong number of arguments,
+  unknown flag, bad flag value or missing required flag. The root
+  command's `PersistentPreRunE` turns usage off once cobra has checked
+  the arguments and flags; root `SilenceUsage` would have hidden usage
+  for those too.
+- 2026-10-04: `.gitignore` is the org's standard file, which ignores
+  `.env`, `.env.*`, `*.pem` and `*.key` and editor and OS files, plus
+  this repo's `/secret`, `*.log`, `*.test` and `settings.local.json`
+  (https://git.eeqj.de/sneak/secret/issues/40). `.dockerignore` also
+  leaves out `node_modules`; `.git` stays in the build context for the
+  version stamp.
+- 2026-10-04: `secret init` refuses when the default vault exists, and
+  `secret vault create NAME` when `NAME` does, with "vault NAME already
+  exists", before writing anything. The check is in `vault.CreateVault`,
+  which both commands call while holding the state directory lock, so two
+  creates of one vault at once cannot both pass the check. Before, either
+  command replaced the vault's metadata, passphrase unlocker and
+  `longterm.age`, so none of its secrets could be decrypted any more. Both
+  commands now ask for the unlocker passphrase before creating the vault,
+  so one stopped at that prompt leaves no vault behind.
+- 2026-10-04: The `internal/cli` tests are back to about their time
+  before the state directory lock
+  (https://git.eeqj.de/sneak/secret/issues/80). The test that each
+  changing command waits for the lock releases it as soon as it sees the
+  command waiting there, instead of after a fixed 100 ms. The two vaults
+  with passphrase unlockers that the path and move tests start from are
+  made once and copied for each test.
+- 2026-10-04: `secret mv` rejects a move whose destination is the source
+  under another name, such as `foo` for `Foo` on a case-insensitive
+  filesystem (the macOS default) or a name reached through a symbolic
+  link, before changing anything, with or without `--force`, within a
+  vault and between vaults; before, `--force` removed the destination and
+  so deleted the secret. A rename that changes only letter case works on a
+  case-sensitive filesystem as before.
+- 2026-10-04: Lint runs only in docker: `script/lint` builds
+  `Dockerfile.lint`, where golangci-lint is a build step rebuilt on
+  every run (`--no-cache-filter`), so an unchanged tree is linted too;
+  the module download stays cached. `script/bootstrap` no longer
+  installs golangci-lint, and the `Dockerfile` lint stage calls it
+  directly instead of `make lint`. `golangci-lint config verify` is not
+  run: it fetches its schema live over unpinned HTTPS.
+- 2026-10-04: A PGP unlocker whose metadata has no usable GPG key ID
+  no longer panics: `GetID()` warns with the unlocker's directory and
+  returns `pgp-unknown`. `ListUnlockers` skips, with a warning, an
+  unlocker whose metadata file cannot be checked for, read or parsed
+  instead of failing, so `secret unlocker list` still lists the others;
+  the listing's ID lookup no longer warns about that directory again.
+- 2026-10-03: `secret mv` rejects a move whose destination is the
+  source (`mv --force x x`, `mv --force work:x work:`, or an empty
+  destination, which defaults to the source name) before changing
+  anything; before, `--force` removed the destination first and so
+  deleted the secret. Every vault name given with `vault:` must be one
+  of the existing vaults by exact name, so `work:x work/:x` is rejected
+  instead of being taken for a move between two vaults. A move within a
+  named vault no longer makes that vault the current one, whether it
+  succeeds or fails.
 - 2026-10-03: Commands that change the state directory hold one lock
   (`flock` on `lock` in the state directory; a mutex on the in-memory
   test filesystem), so concurrent commands no longer lose versions or
@@ -42,9 +100,9 @@ Bring the repo into policy compliance in one commit:
       has one, and to a PGP, keychain or Secure Enclave unlocker added
       on the same host and day as another of its type
       (https://git.eeqj.de/sneak/secret/issues/71);
-    - from `vault create` stopped at the passphrase prompt, a new vault
-      with no unlocker that is already the current vault; from `init`
-      stopped there, the default vault with no unlocker;
+    - from `init` or `vault create` killed after the passphrase prompt
+      but before the unlocker is written, a vault with no unlocker,
+      which `vault create` has already made the current vault;
     - from an unlocker add stopped before its metadata is written, a
       directory that `unlocker list` warns about and `unlocker rm`
       cannot remove;
@@ -53,6 +111,27 @@ Bring the repo into policy compliance in one commit:
       being removed, encrypted keys included. Nothing deletes it; it
       must be deleted by hand
       (https://git.eeqj.de/sneak/secret/issues/75).
+- 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
@@ -126,16 +205,11 @@ Bring the repo into policy compliance in one commit:
       version.go:155); age secret key held in a plain string in
       cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
       to GPGEncryptFunc and EncryptWithPassphrase.
-    - Input validation: dots in secret names risk path traversal
-      (vault/secrets.go:75-99); no maximum secret size (DoS).
+    - Input validation: 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:
-    - Return errors instead of panicking on corrupted metadata
-      (pgpunlocker.go:116, keychainunlocker.go:141).
     - Secure temporary file handling and cleanup.
-    - Print cobra usage only for argument errors, not internal
-      failures.
     - Initialize a default unlock key at vault creation.
     - Confirmation prompts for destructive operations (keys rm, vault
       deletion).

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

internal/cli/create_vault_test.go

@@ -0,0 +1,148 @@
+package cli_test
+
+import (
+	"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"
+)
+
+// TestCreateExistingVaultChangesNothing is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/74, where running `secret init`
+// a second time, or `secret vault create` with the name of an existing
+// vault, replaced that vault's keys, so that none of its secrets could be
+// decrypted any more. Each must refuse, change nothing, and leave every
+// vault's secret readable through its passphrase unlocker.
+//
+//nolint:paralleltest // t.Setenv forbids parallel subtests
+func TestCreateExistingVaultChangesNothing(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+	t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
+
+	// `secret init`, `secret vault create work`, `secret vault select
+	// default`, and the secret "x" in each vault. "work" is then not the
+	// current vault, which creating it again must not change.
+	fs := afero.NewMemMapFs()
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	cmd := &cobra.Command{}
+
+	require.NoError(t, c.Init(cmd))
+	require.NoError(t, c.CreateVault(cmd, "work"))
+	require.NoError(t, c.SelectVault(cmd, "default"))
+
+	vaults, err := vault.ListVaults(fs, testStateDir)
+	require.NoError(t, err)
+	require.Len(t, vaults, 2)
+
+	for _, name := range vaults {
+		value := memguard.NewBufferFromBytes([]byte("value"))
+		err := vault.NewVault(fs, testStateDir, name).AddSecret("x", value, false)
+		require.NoError(t, err)
+	}
+
+	before := snapshotStateDir(t, fs)
+
+	tests := []struct {
+		command string
+		want    string
+		run     func(c *cli.Instance) error
+	}{
+		{
+			"init",
+			"failed to create default vault: vault default already exists",
+			func(c *cli.Instance) error { return c.Init(cmd) },
+		},
+		{
+			"vault create default",
+			"vault default already exists",
+			func(c *cli.Instance) error { return c.CreateVault(cmd, "default") },
+		},
+		{
+			"vault create work",
+			"vault work already exists",
+			func(c *cli.Instance) error { return c.CreateVault(cmd, "work") },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			fs := newFsFromSnapshot(t, before)
+
+			err := tt.run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
+
+			require.EqualError(t, err, tt.want)
+			require.Equal(t, before, snapshotStateDir(t, fs))
+		})
+	}
+
+	// Every case left the state directory exactly as recorded in before, so
+	// reading each vault's secret once from it shows that it still decrypts
+	// after each case. Without the mnemonic, reading a secret goes through
+	// the vault's passphrase unlocker, which is slow.
+	t.Setenv(secret.EnvMnemonic, "")
+
+	for _, name := range vaults {
+		value, err := vault.NewVault(fs, testStateDir, name).GetSecret("x")
+		require.NoError(t, err)
+		require.Equal(t, "value", string(value))
+	}
+}
+
+// TestStopAtPassphrasePromptLeavesNothing is a regression test for the
+// review of https://git.eeqj.de/sneak/secret/pulls/82: `secret init` or
+// `secret vault create` stopped at the passphrase prompt left a vault with
+// no unlocker, which neither command would then create again. Each must ask
+// for the passphrase before writing anything.
+//
+//nolint:paralleltest // t.Setenv forbids parallel subtests
+func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	// Without the passphrase in the environment, both commands prompt for
+	// it, which fails because the tests do not run in a terminal.
+	t.Setenv(secret.EnvUnlockPassphrase, "")
+
+	// An empty state directory for `secret init`, and one holding the vault
+	// "default" for `secret vault create work`.
+	empty := afero.NewMemMapFs()
+	require.NoError(t, empty.MkdirAll(testStateDir, secret.DirPerms))
+
+	withDefault := afero.NewMemMapFs()
+	_, err := vault.CreateVault(withDefault, testStateDir, "default")
+	require.NoError(t, err)
+
+	cmd := &cobra.Command{}
+
+	tests := []struct {
+		command string
+		fs      afero.Fs
+		run     func(c *cli.Instance) error
+	}{
+		{
+			"init",
+			empty,
+			func(c *cli.Instance) error { return c.Init(cmd) },
+		},
+		{
+			"vault create work",
+			withDefault,
+			func(c *cli.Instance) error { return c.CreateVault(cmd, "work") },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			before := snapshotStateDir(t, tt.fs)
+
+			err := tt.run(cli.NewCLIInstanceWithStateDir(tt.fs, testStateDir))
+
+			require.ErrorContains(t, err, "failed to read passphrase")
+			require.Equal(t, before, snapshotStateDir(t, tt.fs))
+		})
+	}
+}

internal/cli/crypto.go

@@ -70,12 +70,11 @@ func newDecryptCmd() *cobra.Command {
 	)
 }
 
-// resolveEncryptionKey returns a secure buffer holding the age secret key
-// for the named secret, generating and storing a new key if the secret
-// does not exist. The caller must destroy the returned buffer. It holds the
-// state directory lock itself, so that Encrypt streams its input and output
-// unlocked and cannot block a secret command at the other end of a pipe.
-func (cli *Instance) resolveEncryptionKey(
+// storeNewEncryptionKey generates an age secret key and stores it as the
+// named secret, holding the state directory lock while it does. It fails
+// with vault.ErrSecretExists if another command stored the secret first.
+// The caller must destroy the returned buffer.
+func (cli *Instance) storeNewEncryptionKey(
 	vlt *vault.Vault, secretName string,
 ) (*memguard.LockedBuffer, error) {
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
@@ -84,6 +83,34 @@ func (cli *Instance) resolveEncryptionKey(
 	}
 	defer release()
 
+	identity, err := age.GenerateX25519Identity()
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate age key: %w", err)
+	}
+
+	// Store the generated key directly in a secure buffer
+	secureBuffer := memguard.NewBufferFromBytes([]byte(identity.String()))
+
+	err = vlt.AddSecret(secretName, secureBuffer, false)
+	if err != nil {
+		secureBuffer.Destroy()
+
+		return nil, fmt.Errorf("failed to store age key: %w", err)
+	}
+
+	return secureBuffer, nil
+}
+
+// resolveEncryptionKey returns a secure buffer holding the age secret key
+// for the named secret, generating and storing a new key if the secret
+// does not exist. The caller must destroy the returned buffer. Only storing
+// a new key takes the state directory lock, so that reading an existing key
+// works on a read-only state directory and keeps no other command waiting
+// at the passphrase prompt, and Encrypt streams its input and output
+// unlocked.
+func (cli *Instance) resolveEncryptionKey(
+	vlt *vault.Vault, secretName string,
+) (*memguard.LockedBuffer, error) {
 	// Check if secret exists
 	secretObj := secret.NewSecret(vlt, secretName)
 
@@ -93,23 +120,11 @@ func (cli *Instance) resolveEncryptionKey(
 	}
 
 	if !exists {
-		// Secret doesn't exist, generate new age key and store it
-		identity, err := age.GenerateX25519Identity()
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate age key: %w", err)
+		key, err := cli.storeNewEncryptionKey(vlt, secretName)
+		if !errors.Is(err, vault.ErrSecretExists) {
+			return key, err
 		}
-
-		// Store the generated key directly in a secure buffer
-		secureBuffer := memguard.NewBufferFromBytes([]byte(identity.String()))
-
-		err = vlt.AddSecret(secretName, secureBuffer, false)
-		if err != nil {
-			secureBuffer.Destroy()
-
-			return nil, fmt.Errorf("failed to store age key: %w", err)
-		}
-
-		return secureBuffer, nil
+		// Another command stored the key since the check above: read it
 	}
 
 	// Secret exists, get the age secret key from it
@@ -130,6 +145,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 {
@@ -199,6 +219,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/init.go

@@ -160,6 +160,14 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 			errInvalidMnemonicPhrase)
 	}
 
+	// Ask for the unlocker passphrase before creating the vault, so that
+	// stopping at the prompt leaves no vault without an unlocker behind
+	passphraseBuffer, err := resolvePassphrase()
+	if err != nil {
+		return err
+	}
+	defer passphraseBuffer.Destroy()
+
 	// Set mnemonic in environment for CreateVault to use
 	restoreMnemonicEnv := setMnemonicEnv(mnemonicStr)
 	defer restoreMnemonicEnv()
@@ -175,13 +183,6 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 	// Unlock the vault with the derived long-term key
 	vlt.Unlock(ltIdentity)
 
-	// Prompt for passphrase for unlocker
-	passphraseBuffer, err := resolvePassphrase()
-	if err != nil {
-		return err
-	}
-	defer passphraseBuffer.Destroy()
-
 	// Create passphrase-protected unlocker
 	secret.Debug("Creating passphrase-protected unlocker")
 

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

@@ -2,9 +2,11 @@
 package cli
 
 import (
+	"bytes"
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -25,11 +27,6 @@ const (
 	// once the lock is free.
 	lockWait = 10 * time.Second
 
-	// heldWait is how long a test watches a command that must wait for the
-	// lock. A command that takes no lock changes the state directory well
-	// within it.
-	heldWait = 100 * time.Millisecond
-
 	// testPassphrase protects the passphrase unlockers the tests create.
 	testPassphrase = "test-passphrase"
 
@@ -274,11 +271,12 @@ func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
 }
 
 // setupEveryCommand makes what each command in
-// TestChangingCommandsWaitForLock needs: the current vault "default" with
-// two versions of "test/secret", the vault "other" without a long-term key,
-// for vault import, and the file testInput. If withUnlocker is set, it also
-// gives "default" a passphrase unlocker, which is slow. It returns the older
-// version and the unlocker's ID.
+// TestChangingCommandsWaitForLock needs: the current vault "work" with two
+// versions of "test/secret", the vault "other" without a long-term key, for
+// vault import, and the file testInput. There is no vault "default", which
+// init creates. If withUnlocker is set, it also gives "work" a passphrase
+// unlocker, which is slow. It returns the older version and the unlocker's
+// ID.
 func setupEveryCommand(
 	t *testing.T, fs afero.Fs, withUnlocker bool,
 ) (string, string) {
@@ -291,7 +289,7 @@ func setupEveryCommand(
 	require.NoError(t, err)
 	require.NoError(t, fs.Remove(filepath.Join(otherDir, "pub.age")))
 
-	vlt, err := vault.CreateVault(fs, testStateDir, "default")
+	vlt, err := vault.CreateVault(fs, testStateDir, "work")
 	require.NoError(t, err)
 
 	addTestSecret(t, vlt, []byte("older"), false)
@@ -323,10 +321,28 @@ func setupEveryCommand(
 	return versions[1], unlockerID
 }
 
+// waitingForLock reports whether a goroutine is stopped in
+// vault.LockStateDir, waiting for the in-memory filesystem's lock. The
+// stack trace of such a goroutine starts with the reason it waits,
+// "[sync.Mutex.Lock]", and names LockStateDir.
+func waitingForLock() bool {
+	stacks := make([]byte, 1<<20)
+	stacks = stacks[:runtime.Stack(stacks, true)]
+
+	for goroutine := range bytes.SplitSeq(stacks, []byte("\n\n")) {
+		if bytes.Contains(goroutine, []byte("[sync.Mutex.Lock")) &&
+			bytes.Contains(goroutine, []byte("vault.LockStateDir(")) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // requireWaitsForLock runs a command, given what setupEveryCommand made,
 // while holding the state directory lock. The command must neither finish
-// nor change anything while the lock is held, and must succeed once it is
-// released.
+// nor change anything before it waits for the lock, and must succeed once
+// the lock is released.
 func requireWaitsForLock(
 	t *testing.T,
 	withUnlocker bool,
@@ -355,14 +371,20 @@ func requireWaitsForLock(
 
 	go func() { done <- run(cli, olderVersion, unlockerID) }()
 
-	select {
-	case err := <-done:
-		t.Fatalf("finished while the lock was held, with error %v", err)
-	case <-time.After(heldWait):
+	timeout := time.After(lockWait)
+
+	for !waitingForLock() {
+		select {
+		case err := <-done:
+			t.Fatalf("finished while the lock was held, with error %v", err)
+		case <-timeout:
+			t.Fatal("never waited for the lock")
+		case <-time.After(time.Millisecond):
+		}
 	}
 
 	assert.Equal(t, before, stateDirModTimes(t, fs),
-		"changed the state directory while the lock was held")
+		"changed the state directory before waiting for the lock")
 
 	release()
 
@@ -442,12 +464,46 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
 	}
 }
 
+// TestEncryptWithExistingKeyTakesNoLock checks that secret encrypt with a
+// key that already exists, which only reads the state directory, finishes
+// while another command holds the state directory lock.
+func TestEncryptWithExistingKeyTakesNoLock(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	fs := afero.NewMemMapFs()
+	_, err := vault.CreateVault(fs, testStateDir, "default")
+	require.NoError(t, err)
+	require.NoError(t, afero.WriteFile(fs, testInput, []byte("input"), 0o600))
+
+	encrypt := NewCLIInstanceWithStateDir(fs, testStateDir)
+	encrypt.cmd = &cobra.Command{}
+	encrypt.cmd.SetOut(io.Discard)
+
+	// Stores the key
+	require.NoError(t, encrypt.Encrypt("key", testInput, ""))
+
+	release, err := vault.LockStateDir(fs, testStateDir)
+	require.NoError(t, err)
+	// Also frees a waiting encrypt if the test fails, so that it releases
+	// the lock the other tests use
+	defer release()
+
+	done := make(chan error, 1)
+
+	go func() { done <- encrypt.Encrypt("key", testInput, "") }()
+
+	select {
+	case err := <-done:
+		require.NoError(t, err)
+	case <-time.After(lockWait):
+		t.Fatal("secret encrypt with an existing key waited for the lock")
+	}
+}
+
 // TestEncryptStreamsUnlocked checks that secret encrypt has released the
 // state directory lock by the time it writes its output. Holding it while
 // streaming would stall every other changing command for as long as the
 // stream lasts, and forever when the other end of the pipe is one of them.
-//
-//nolint:paralleltest // t.Setenv forbids t.Parallel
 func TestEncryptStreamsUnlocked(t *testing.T) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
@@ -484,6 +540,7 @@ func TestEncryptStreamsUnlocked(t *testing.T) {
 		// Let encrypt finish, so that it releases the lock, then free it
 		// again for the tests that follow
 		_, _ = io.Copy(io.Discard, outputReader)
+
 		(<-taken)()
 		t.Fatal("secret encrypt held the lock while streaming")
 	}

internal/cli/move_test.go

@@ -0,0 +1,229 @@
+package cli_test
+
+import (
+	"os"
+	"path/filepath"
+	"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"
+)
+
+// TestRejectedMoveWithinVaultLeavesStateUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/73, where a forced move of a secret
+// onto itself deleted it, also when "work" was spelled two ways, and a failed
+// move within "work" left "work" the current vault. "default" is the current
+// vault in every case, and each case runs on its own copy of the state
+// directory.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+	require.Equal(t, "default", before[testStateDir+"/currentvault"])
+
+	const (
+		ontoItself = "secret 'x' cannot be moved onto itself"
+		workX      = "work:x"
+	)
+
+	tests := []struct {
+		command      string
+		source, dest string
+		force        bool
+		wantErr      string
+	}{
+		{"mv x x", "x", "x", false, ontoItself},
+		{"mv --force x x", "x", "x", true, ontoItself},
+		{"mv --force work:x work:", workX, "work:", true, ontoItself},
+		// An empty destination name defaults to the source name.
+		{`mv --force work:x ""`, workX, "", true, ontoItself},
+		// "work" is a vault name, so the destination is work:x.
+		{"mv --force work:x work", workX, "work", true, ontoItself},
+		{
+			"mv work:nosuch work:y", "work:nosuch", "work:y", false,
+			"secret 'nosuch' not found",
+		},
+		// Only an existing vault is used, so ".." cannot reach the state
+		// directory itself.
+		{
+			"mv --force ..:x ..:y", "..:x", "..:y", true,
+			"vault '..' does not exist",
+		},
+		// Each of these spells "work" a second way. The spelling is not an
+		// existing vault name, so the move is not taken for a move between
+		// two vaults, which would delete the destination, here the source.
+		{
+			"mv --force work:x work/:x", workX, "work/:x", true,
+			"vault 'work/' does not exist",
+		},
+		{
+			"mv --force work/:x work:", "work/:x", "work:", true,
+			"vault 'work/' does not exist",
+		},
+		{
+			"mv --force work:x ./work:x", workX, "./work:x", true,
+			"vault './work' does not exist",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			fs := newFsFromSnapshot(t, before)
+			c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+
+			err := c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, tt.force)
+
+			require.Equal(t, before, snapshotStateDir(t, fs))
+			require.EqualError(t, err, tt.wantErr)
+		})
+	}
+}
+
+// TestMoveWithinOtherVaultKeepsCurrentVault checks that `secret mv work:x
+// work:y`, with "default" the current vault, renames "x" to "y" in "work" and
+// leaves "default" the current vault.
+//
+//nolint:paralleltest // newTwoVaultFs uses t.Setenv
+func TestMoveWithinOtherVaultKeepsCurrentVault(t *testing.T) {
+	fs := newTwoVaultFs(t)
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+
+	err := c.MoveSecret(&cobra.Command{}, "work:x", "work:y", false)
+	require.NoError(t, err)
+
+	after := snapshotStateDir(t, fs)
+	workSecrets := testStateDir + "/vaults.d/work/secrets.d/"
+
+	require.Equal(t, "default", after[testStateDir+"/currentvault"])
+	require.Contains(t, after, workSecrets+"y/")
+	require.NotContains(t, after, workSecrets+"x/")
+}
+
+// TestMoveOntoSameSecretUnderAnotherNameIsRejected is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/78: on a case-insensitive
+// filesystem "Foo" and "foo" are one secret, and `secret mv --force Foo foo`
+// removed the destination, which was the source. Symbolic links on the real
+// filesystem give one secret two names here: in "default", "y" is a link to
+// the secret "x", and the secrets.d of "other" is a link to that of
+// "default", so other:x is default:x. Each move must be rejected and leave
+// the secret and the links as they were.
+//
+//nolint:paralleltest // t.Setenv
+func TestMoveOntoSameSecretUnderAnotherNameIsRejected(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	const isSame = "is the same secret on this filesystem"
+
+	tests := []struct {
+		command      string
+		source, dest string
+		force        bool
+		wantErr      string
+	}{
+		{
+			"mv --force y x", "y", "x", true,
+			"secret 'y' cannot be moved onto itself: 'x' " + isSame,
+		},
+		{
+			"mv --force x y", "x", "y", true,
+			"secret 'x' cannot be moved onto itself: 'y' " + isSame,
+		},
+		{
+			"mv x y", "x", "y", false,
+			"secret 'x' cannot be moved onto itself: 'y' " + isSame,
+		},
+		{
+			"mv --force default:x other:x", "default:x", "other:x", true,
+			"secret 'default:x' cannot be moved onto itself: 'other:x' " +
+				isSame,
+		},
+		{
+			"mv default:x other", "default:x", "other", false,
+			"secret 'default:x' cannot be moved onto itself: 'other:x' " +
+				isSame,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			fs := afero.NewOsFs()
+			stateDir := t.TempDir()
+			vaultsDir := filepath.Join(stateDir, "vaults.d")
+
+			// "default" is created last, so it is the current vault.
+			_, err := vault.CreateVault(fs, stateDir, "other")
+			require.NoError(t, err)
+
+			vlt, err := vault.CreateVault(fs, stateDir, "default")
+			require.NoError(t, err)
+
+			err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
+			require.NoError(t, err)
+
+			defaultSecrets := filepath.Join(vaultsDir, "default", "secrets.d")
+			otherSecrets := filepath.Join(vaultsDir, "other", "secrets.d")
+			link := filepath.Join(defaultSecrets, "y")
+
+			require.NoError(t, os.Symlink("x", link))
+			require.NoError(t, os.Remove(otherSecrets))
+			require.NoError(t, os.Symlink(defaultSecrets, otherSecrets))
+
+			c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
+			moveErr := c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, tt.force)
+
+			value, err := vlt.GetSecret("x")
+			require.NoError(t, err)
+			require.Equal(t, "value", string(value))
+
+			target, err := os.Readlink(link)
+			require.NoError(t, err)
+			require.Equal(t, "x", target)
+
+			target, err = os.Readlink(otherSecrets)
+			require.NoError(t, err)
+			require.Equal(t, defaultSecrets, target)
+
+			require.EqualError(t, moveErr, tt.wantErr)
+		})
+	}
+}
+
+// TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem checks that where "Foo"
+// and "foo" are two secrets, `secret mv --force Foo foo` still replaces "foo"
+// with "Foo".
+func TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	fs := afero.NewOsFs()
+	stateDir := t.TempDir()
+
+	vlt, err := vault.CreateVault(fs, stateDir, "default")
+	require.NoError(t, err)
+
+	err = vlt.AddSecret("Foo", memguard.NewBufferFromBytes([]byte("upper")), false)
+	require.NoError(t, err)
+
+	_, err = os.Stat(filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "foo"))
+	if err == nil {
+		t.Skip("the temporary directory is on a case-insensitive filesystem")
+	}
+
+	err = vlt.AddSecret("foo", memguard.NewBufferFromBytes([]byte("lower")), false)
+	require.NoError(t, err)
+
+	c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
+	err = c.MoveSecret(&cobra.Command{}, "Foo", "foo", true)
+	require.NoError(t, err)
+
+	value, err := vlt.GetSecret("foo")
+	require.NoError(t, err)
+	require.Equal(t, "upper", string(value))
+
+	_, err = vlt.GetSecret("Foo")
+	require.ErrorIs(t, err, vault.ErrSecretNotFound)
+}

internal/cli/path_traversal_test.go

@@ -0,0 +1,366 @@
+package cli_test
+
+import (
+	"fmt"
+	"maps"
+	"os"
+	"slices"
+	"strings"
+	"sync"
+	"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"
+)
+
+// The state directory newTwoVaultFs copies, recorded by snapshotStateDir.
+// Creating a passphrase unlocker is slow by design, so the vaults are made
+// once, by the first test that needs them.
+//
+//nolint:gochecknoglobals // shared by the tests that use newTwoVaultFs
+var (
+	twoVaultsOnce sync.Once
+	twoVaults     map[string]string
+)
+
+// 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.
+// Every call returns a new copy of the same vaults.
+//
+//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
+func newTwoVaultFs(t *testing.T) afero.Fs {
+	t.Helper()
+
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	twoVaultsOnce.Do(func() {
+		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)
+		}
+
+		twoVaults = snapshotStateDir(t, fs)
+	})
+
+	require.NotNil(t, twoVaults, "making the vaults failed in an earlier test")
+
+	return newFsFromSnapshot(t, twoVaults)
+}
+
+// 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
-func Entry() {
-	cmd := newRootCmd()
+// Entry runs the secret CLI and returns the process exit code. It wipes
+// every memguard buffer before it returns, so the caller must do nothing
+// 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) {
+		foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP)
+		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 {
-		os.Exit(1)
+		return 1
 	}
+
+	return 0
 }
 
 func newRootCmd() *cobra.Command {
@@ -25,9 +46,24 @@ func newRootCmd() *cobra.Command {
 		Short: "A simple secrets manager",
 		Long: `A simple secrets manager to store and retrieve sensitive ` +
 			`information securely.`,
-		// Ensure usage is shown after errors
-		SilenceUsage:  false,
+		// Cobra prints the error a command returns; Entry does not.
 		SilenceErrors: false,
+		// Usage belongs only to a command called wrongly. Cobra has
+		// checked its arguments and flags before this runs, except for
+		// required flags, which are checked here so they still get usage.
+		// An error after that comes from running the command, and usage
+		// would only bury it. A subcommand that sets its own
+		// PersistentPreRun replaces this one.
+		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
+			err := cmd.ValidateRequiredFlags()
+			if err != nil {
+				return err
+			}
+
+			cmd.SilenceUsage = true
+
+			return nil
+		},
 	}
 
 	secret.Debug("Adding subcommands to root command")

internal/cli/secrets.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"os"
 	"path/filepath"
 	"slices"
 	"strings"
@@ -40,6 +41,7 @@ var (
 	errVaultDoesNotExist           = errors.New("does not exist")
 	errCrossVaultSourceUnqualified = errors.New(
 		"source must specify vault (e.g., vault:secret) for cross-vault move")
+	errMoveOntoItself = errors.New("cannot be moved onto itself")
 )
 
 // bufferInfo tracks a protected buffer and the number of bytes used in it
@@ -109,6 +111,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)
 		},
 	}
@@ -402,12 +410,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 {
@@ -426,13 +454,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)
 
@@ -612,6 +634,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 {
@@ -666,6 +693,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
+	}
+
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
@@ -717,7 +749,8 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
 	return nil
 }
 
-// MoveSecret moves or renames a secret (within or across vaults)
+// MoveSecret moves or renames a secret (within or across vaults), holding
+// the state directory lock.
 func (cli *Instance) MoveSecret(
 	cmd *cobra.Command, source, dest string, force bool,
 ) error {
@@ -727,17 +760,20 @@ func (cli *Instance) MoveSecret(
 	}
 	defer release()
 
+	return cli.moveSecret(cmd, source, dest, force)
+}
+
+// moveSecret does the work of MoveSecret. The caller holds the state
+// directory lock.
+func (cli *Instance) moveSecret(
+	cmd *cobra.Command, source, dest string, force bool,
+) error {
 	// Parse source and destination
 	srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
 	destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
 
-	// If neither is qualified, this is a simple within-vault rename
-	if !srcQualified && !destQualified {
-		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
-	}
-
 	// Cross-vault move requires source to be qualified
-	if !srcQualified {
+	if !srcQualified && destQualified {
 		return errCrossVaultSourceUnqualified
 	}
 
@@ -745,53 +781,97 @@ 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) {
+		_, err := cli.existingVault(dest)
+		if err == nil {
 			// dest is a vault name, use source secret name
 			destVaultName = dest
 			destSecretName = srcSecretName
-		}
-
-		// If destVaultName is still empty, dest is a secret name in source vault
-		if destVaultName == "" {
+		} else {
+			// dest is a secret name in source vault
 			destVaultName = srcVaultName
-			destSecretName = dest
 		}
 	}
 
-	// If destination secret name is empty, use source secret name
-	if destSecretName == "" {
+	// If destination secret name is empty, use source secret name. A plain
+	// rename keeps it empty, so that the check below rejects it.
+	if srcQualified && destSecretName == "" {
 		destSecretName = srcSecretName
 	}
 
-	// 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)
-		if err != nil {
-			return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
-		}
-
-		return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
-	}
-
-	// Cross-vault move
-	return cli.moveSecretCrossVault(
-		cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
-}
-
-// moveSecretWithinVault handles rename within the current vault
-func (cli *Instance) moveSecretWithinVault(
-	cmd *cobra.Command, source, dest string, force bool,
-) error {
-	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	// Check both names, for every form of the move, before building any path
+	// from them.
+	err := vault.ValidateSecretName(srcSecretName)
 	if err != nil {
 		return err
 	}
 
-	vaultDir, err := currentVlt.GetDirectory()
+	err = vault.ValidateSecretName(destSecretName)
+	if err != nil {
+		return err
+	}
+
+	// Neither name is qualified: a rename within the current vault.
+	if !srcQualified {
+		vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+		if err != nil {
+			return err
+		}
+
+		return cli.moveSecretWithinVault(
+			cmd, vlt, srcSecretName, destSecretName, force)
+	}
+
+	// Both vaults must be existing vaults by exact name, so that two
+	// spellings of one vault, such as "work" and "work/", are never taken for
+	// two vaults. A named vault does not become the current vault.
+	srcVault, err := cli.existingVault(srcVaultName)
+	if err != nil {
+		return err
+	}
+
+	destVault, err := cli.existingVault(destVaultName)
+	if err != nil {
+		return err
+	}
+
+	if srcVaultName == destVaultName {
+		return cli.moveSecretWithinVault(
+			cmd, srcVault, srcSecretName, destSecretName, force)
+	}
+
+	return cli.moveSecretCrossVault(
+		cmd, srcVault, srcSecretName, destVault, destSecretName, force)
+}
+
+// existingVault returns the vault with the given name, or an error if there
+// is none. Unlike vault.SelectVault, it leaves the current vault as it is.
+func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
+	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
+	if err != nil {
+		return nil, fmt.Errorf("failed to list vaults: %w", err)
+	}
+
+	if !slices.Contains(vaults, name) {
+		return nil, fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
+	}
+
+	return vault.NewVault(cli.fs, cli.stateDir, name), nil
+}
+
+// moveSecretWithinVault renames a secret within the vault vlt. Its caller,
+// MoveSecret, has already checked both secret names.
+func (cli *Instance) moveSecretWithinVault(
+	cmd *cobra.Command, vlt *vault.Vault, source, dest string, force bool,
+) error {
+	// With --force the destination is removed before the source is renamed
+	// onto it, which would delete the secret.
+	if source == dest {
+		return fmt.Errorf("secret '%s' %w", source, errMoveOntoItself)
+	}
+
+	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
 		return err
 	}
@@ -811,6 +891,18 @@ func (cli *Instance) moveSecretWithinVault(
 	destEncoded := strings.ReplaceAll(dest, "/", "%")
 	destDir := filepath.Join(vaultDir, "secrets.d", destEncoded)
 
+	// Removing a destination that is the source under another name, such as
+	// "foo" for "Foo" on a case-insensitive filesystem, would delete it too.
+	same, err := cli.sameDirectory(sourceDir, destDir)
+	if err != nil {
+		return err
+	}
+
+	if same {
+		return fmt.Errorf("secret '%s' %w: '%s' is the same secret on "+
+			"this filesystem", source, errMoveOntoItself, dest)
+	}
+
 	exists, err = afero.DirExists(cli.fs, destDir)
 	if err != nil {
 		return fmt.Errorf("failed to check if destination secret exists: %w", err)
@@ -837,56 +929,80 @@ func (cli *Instance) moveSecretWithinVault(
 	return nil
 }
 
-// moveSecretCrossVault handles moving between different vaults
+// sameDirectory reports whether the existing directory dir and the path
+// other are one directory under two names, as secrets.d/Foo and
+// secrets.d/foo are on a case-insensitive filesystem, or a directory and a
+// symbolic link to it. Removing other to make room for dir would then delete
+// dir. It is false if other does not exist, and always false on the
+// in-memory filesystem, which has no such aliasing and whose files
+// os.SameFile does not compare.
+func (cli *Instance) sameDirectory(dir, other string) (bool, error) {
+	dirInfo, err := cli.fs.Stat(dir)
+	if err != nil {
+		return false, fmt.Errorf("failed to check %s: %w", dir, err)
+	}
+
+	otherInfo, err := cli.fs.Stat(other)
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	}
+
+	if err != nil {
+		return false, fmt.Errorf("failed to check %s: %w", other, err)
+	}
+
+	return os.SameFile(dirInfo, otherInfo), nil
+}
+
+// moveSecretCrossVault handles moving between two different vaults. Its
+// caller, MoveSecret, has already checked both secret names and that both
+// vaults exist.
 func (cli *Instance) moveSecretCrossVault(
 	cmd *cobra.Command,
-	srcVaultName, srcSecretName,
-	destVaultName, destSecretName string,
+	srcVault *vault.Vault, srcSecretName string,
+	destVault *vault.Vault, destSecretName string,
 	force bool,
 ) error {
-	// Get source vault
-	srcVault := vault.NewVault(cli.fs, cli.stateDir, srcVaultName)
-
 	srcVaultDir, err := srcVault.GetDirectory()
 	if err != nil {
 		return fmt.Errorf("failed to get source vault directory: %w", err)
 	}
 
-	// Verify source vault exists
-	exists, err := afero.DirExists(cli.fs, srcVaultDir)
-	if err != nil || !exists {
-		return fmt.Errorf("source vault '%s' %w", srcVaultName, errVaultDoesNotExist)
-	}
-
 	// Verify source secret exists
 	srcStorageName := strings.ReplaceAll(srcSecretName, "/", "%")
 	srcSecretDir := filepath.Join(srcVaultDir, "secrets.d", srcStorageName)
 
-	exists, err = afero.DirExists(cli.fs, srcSecretDir)
+	exists, err := afero.DirExists(cli.fs, srcSecretDir)
 	if err != nil || !exists {
 		return fmt.Errorf("secret '%s' %w in vault '%s'",
-			srcSecretName, errSecretNotFound, srcVaultName)
+			srcSecretName, errSecretNotFound, srcVault.Name)
 	}
 
-	// Get destination vault
-	destVault := vault.NewVault(cli.fs, cli.stateDir, destVaultName)
-
+	// The source is removed after the copy, so a destination that is the
+	// source under another name would be lost with it.
 	destVaultDir, err := destVault.GetDirectory()
 	if err != nil {
 		return fmt.Errorf("failed to get destination vault directory: %w", err)
 	}
 
-	// Verify destination vault exists
-	exists, err = afero.DirExists(cli.fs, destVaultDir)
-	if err != nil || !exists {
-		return fmt.Errorf("destination vault '%s' %w",
-			destVaultName, errVaultDoesNotExist)
+	destStorageName := strings.ReplaceAll(destSecretName, "/", "%")
+	destSecretDir := filepath.Join(destVaultDir, "secrets.d", destStorageName)
+
+	same, err := cli.sameDirectory(srcSecretDir, destSecretDir)
+	if err != nil {
+		return err
+	}
+
+	if same {
+		return fmt.Errorf("secret '%s:%s' %w: '%s:%s' is the same secret on "+
+			"this filesystem", srcVault.Name, srcSecretName, errMoveOntoItself,
+			destVault.Name, destSecretName)
 	}
 
 	// Unlock destination vault (will fail if neither mnemonic nor unlocker available)
 	_, err = destVault.GetOrDeriveLongTermKey()
 	if err != nil {
-		return fmt.Errorf("failed to unlock destination vault '%s': %w", destVaultName, err)
+		return fmt.Errorf("failed to unlock destination vault '%s': %w", destVault.Name, err)
 	}
 
 	// Count versions for user feedback
@@ -906,13 +1022,13 @@ func (cli *Instance) moveSecretCrossVault(
 		// Copy succeeded but delete failed - warn but don't fail
 		cmd.Printf("Warning: copied secret but failed to remove source: %v\n", err)
 		cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",
-			srcVaultName, srcSecretName, destVaultName, destSecretName, versionCount)
+			srcVault.Name, srcSecretName, destVault.Name, destSecretName, versionCount)
 
 		return nil
 	}
 
 	cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",
-		srcVaultName, srcSecretName, destVaultName, destSecretName, versionCount)
+		srcVault.Name, srcSecretName, destVault.Name, destSecretName, versionCount)
 
 	return nil
 }

internal/cli/unlockers.go

@@ -349,6 +349,10 @@ func unlockerIDFromDir(
 // itself cannot be read. Callers must distinguish the two: an unreadable
 // directory means the unlocker's real ID is unknowable, so the entry has
 // to be skipped rather than reported under a synthesized ID.
+//
+// A metadata file that cannot be read or parsed is skipped without a
+// warning: every caller gets metadata from vault.ListUnlockers first,
+// which has already warned about that directory.
 func findUnlockerIDByMetadata(
 	fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
 	includeSecureEnclave bool,
@@ -371,9 +375,6 @@ func findUnlockerIDByMetadata(
 		// Check if this is the right unlocker by comparing metadata
 		metadataBytes, err := afero.ReadFile(fs, metadataPath)
 		if err != nil {
-			secret.Warn("Could not read unlocker metadata file",
-				"path", metadataPath, "error", err)
-
 			continue
 		}
 
@@ -381,9 +382,6 @@ func findUnlockerIDByMetadata(
 
 		err = json.Unmarshal(metadataBytes, &diskMetadata)
 		if err != nil {
-			secret.Warn("Could not parse unlocker metadata file",
-				"path", metadataPath, "error", err)
-
 			continue
 		}
 

internal/cli/unlockers_list_test.go

@@ -1,13 +1,19 @@
 // Unlocker List Tests
 //
-// Tests for `secret unlocker list` behavior when the unlockers.d directory
-// cannot be read while the listing is being rendered:
+// Tests for `secret unlocker list` behavior when the unlockers.d directory,
+// or an unlocker's metadata in it, cannot be read while the listing is
+// being rendered:
 //
 //   - TestUnlockersListSkipsUnreadableUnlockersDir: an unreadable
 //     unlockers.d yields no rows rather than rows bearing synthesized IDs.
 //   - TestUnlockersListSkipsOnlyUnreadableEntries: a readable entry is
 //     still listed, with its real ID and its current-unlocker marker,
 //     when a later entry's scan fails.
+//   - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt
+//     metadata does not stop the others from being listed.
+//   - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata
+//     file cannot be checked for or read is left out, and the other is
+//     still listed.
 //
 // The listing resolves each unlocker's real ID by rescanning unlockers.d
 // after the vault has already enumerated it. If that rescan fails the ID
@@ -22,6 +28,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -92,6 +99,49 @@ func (f *unlockersDirFailFs) Open(name string) (afero.File, error) {
 	return f.Fs.Open(name)
 }
 
+// errMetadataUnreadable is returned by the test filesystem in place of a
+// successful open of one unlocker's metadata file.
+var errMetadataUnreadable = errors.New("input/output error")
+
+// metadataReadFailFs fails every open of the file at unreadablePath. The
+// file still exists, so checking for it succeeds and only reading it fails.
+type metadataReadFailFs struct {
+	afero.Fs
+
+	unreadablePath string
+}
+
+//nolint:ireturn // afero.File is the interface required by afero.Fs
+func (f *metadataReadFailFs) Open(name string) (afero.File, error) {
+	if name == f.unreadablePath {
+		return nil, errMetadataUnreadable
+	}
+
+	//nolint:wrapcheck // test double must return the wrapped Fs error as-is
+	return f.Fs.Open(name)
+}
+
+// errMetadataUncheckable is returned by the test filesystem in place of a
+// successful check for one unlocker's metadata file.
+var errMetadataUncheckable = errors.New("permission denied")
+
+// metadataStatFailFs fails every check for whether the file at
+// uncheckablePath exists, as when its unlocker directory cannot be entered.
+type metadataStatFailFs struct {
+	afero.Fs
+
+	uncheckablePath string
+}
+
+func (f *metadataStatFailFs) Stat(name string) (os.FileInfo, error) {
+	if name == f.uncheckablePath {
+		return nil, errMetadataUncheckable
+	}
+
+	//nolint:wrapcheck // test double must return the wrapped Fs error as-is
+	return f.Fs.Stat(name)
+}
+
 // writePGPUnlocker writes a PGP unlocker directory with metadata that
 // yields the real ID "pgp-<keyID>".
 func writePGPUnlocker(
@@ -227,3 +277,102 @@ func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
 	assert.True(t, unlockers[0].IsCurrent)
 	assert.False(t, unlockers[1].IsCurrent)
 }
+
+// TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with
+// corrupt metadata does not stop the listing. Metadata that is not JSON
+// leaves that unlocker out; PGP metadata without a usable GPG key ID lists
+// it as "pgp-unknown". The healthy unlocker is listed with its real ID.
+func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
+	t.Parallel()
+
+	healthyID := "pgp-" + listTestGPGKeyID + "A"
+
+	tests := []struct {
+		name     string
+		metadata string
+		wantIDs  []string
+	}{
+		{
+			name:     "not JSON",
+			metadata: "not json",
+			wantIDs:  []string{healthyID},
+		},
+		{
+			name:     "GPG key ID of the wrong type",
+			metadata: `{"type": "pgp", "gpgKeyId": 42}`,
+			wantIDs:  []string{healthyID, "pgp-unknown"},
+		},
+		{
+			name:     "GPG key ID missing",
+			metadata: `{"type": "pgp"}`,
+			wantIDs:  []string{healthyID, "pgp-unknown"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			fs := newListTestVault(t, 2)
+			metadataPath := filepath.Join(listTestStateDir, "vaults.d",
+				listTestVaultName, listTestUnlockersDirName,
+				listTestUnlockerDirTwo, listTestMetadataFileName)
+			require.NoError(t, afero.WriteFile(
+				fs, metadataPath, []byte(tt.metadata), listTestFilePerm,
+			))
+
+			unlockers := listUnlockersJSON(t, fs)
+			require.Len(t, unlockers, len(tt.wantIDs))
+
+			for i, wantID := range tt.wantIDs {
+				assert.Equal(t, wantID, unlockers[i].ID)
+			}
+		})
+	}
+}
+
+// TestUnlockersListSkipsUnreadableMetadata asserts that an unlocker whose
+// metadata file cannot be checked for or cannot be read is left out of the
+// listing, and the other unlocker is still listed with its real ID. The
+// failing one sorts first, so finding the other's ID has to step past it
+// as well.
+func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
+	t.Parallel()
+
+	failingPath := filepath.Join(listTestStateDir, "vaults.d",
+		listTestVaultName, listTestUnlockersDirName,
+		listTestUnlockerDirOne, listTestMetadataFileName)
+
+	tests := []struct {
+		name string
+		wrap func(base afero.Fs) afero.Fs
+	}{
+		{
+			name: "checking for the file fails",
+			wrap: func(base afero.Fs) afero.Fs {
+				return &metadataStatFailFs{Fs: base, uncheckablePath: failingPath}
+			},
+		},
+		{
+			name: "reading the file fails",
+			wrap: func(base afero.Fs) afero.Fs {
+				return &metadataReadFailFs{Fs: base, unreadablePath: failingPath}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			fs := tt.wrap(newListTestVault(t, 2))
+
+			unlockers := listUnlockersJSON(t, fs)
+
+			require.Len(t, unlockers, 1,
+				"only the unlocker with usable metadata may be listed")
+			assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[0].ID,
+				"the listed row must carry the real unlocker ID")
+		})
+	}
+}

internal/cli/usage_test.go

@@ -0,0 +1,46 @@
+package cli_test
+
+import (
+	"strings"
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// usageHeading starts the usage text cobra prints after an error.
+const usageHeading = "Usage:"
+
+// A command called wrongly gets usage after its error; a command that
+// fails while running gets its error alone. Either way the command fails
+// and its error is shown exactly once.
+//
+//nolint:paralleltest // executes the CLI in-process and sets the environment
+func TestUsageOnlyForCallErrors(t *testing.T) {
+	// No vault in the state directory, so `get x` fails while running.
+	env := map[string]string{secret.EnvStateDir: t.TempDir()}
+
+	tests := []struct {
+		call      string
+		wantUsage bool
+	}{
+		{call: "get", wantUsage: true},
+		{call: "get x y", wantUsage: true},
+		{call: "get --no-such-flag x", wantUsage: true},
+		{call: "generate secret x --length abc", wantUsage: true},
+		{call: "import x", wantUsage: true},
+		{call: "get x", wantUsage: false},
+	}
+
+	for _, tt := range tests {
+		output, err := cli.ExecuteCommandInProcess(strings.Fields(tt.call), "", env)
+		require.Error(t, err, "%q should fail", tt.call)
+
+		assert.Equal(t, 1, strings.Count(output, err.Error()),
+			"%q should show its error once:\n%s", tt.call, output)
+		assert.Equal(t, tt.wantUsage, strings.Contains(output, usageHeading),
+			"usage shown for %q:\n%s", tt.call, output)
+	}
+}

internal/cli/vault.go

@@ -309,6 +309,14 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 		return errInvalidMnemonicPhrase
 	}
 
+	// Ask for the unlocker passphrase before creating the vault, so that
+	// stopping at the prompt leaves no vault without an unlocker behind
+	passphraseBuffer, err := resolvePassphrase()
+	if err != nil {
+		return err
+	}
+	defer passphraseBuffer.Destroy()
+
 	// Set mnemonic in environment for CreateVault to use
 	restoreMnemonicEnv := setMnemonicEnv(mnemonicStr)
 	defer restoreMnemonicEnv()
@@ -336,13 +344,6 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	// Unlock the vault with the derived long-term key
 	vlt.Unlock(ltIdentity)
 
-	// Get or prompt for passphrase
-	passphraseBuffer, err := resolvePassphrase()
-	if err != nil {
-		return err
-	}
-	defer passphraseBuffer.Destroy()
-
 	// Create passphrase-protected unlocker
 	secret.Debug("Creating passphrase-protected unlocker")
 

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
+	}
+
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
@@ -261,9 +271,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)
 	}
@@ -288,6 +296,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
+	}
+
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
@@ -320,9 +333,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)
 	}
@@ -345,6 +356,8 @@ func (cli *Instance) RemoveVersion(
 	}
 
 	// Remove the version directory
+	versionDir := filepath.Join(secretDir, "versions", version)
+
 	err = secret.RemoveDirAtomic(cli.fs, versionDir)
 	if err != nil {
 		return fmt.Errorf("failed to remove version: %w", err)

internal/cli/version_test.go

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

internal/secret/atomic_test.go

@@ -36,6 +36,8 @@ const currentFile = "current"
 const unlockerMetadataFile = "unlocker-metadata.json"
 
 // unlockerPassphrase protects the passphrase unlockers the tests create.
+//
+//nolint:gosec // G101: test data, not a real credential
 const unlockerPassphrase = "unlocker passphrase"
 
 // hookFs passes every call through to Fs, but first calls before for each
@@ -612,8 +614,6 @@ func TestWriteFileAtomicTempFile(t *testing.T) {
 // vault whose long-term key cannot be had: it must fail without writing
 // anything, so that it never leaves a partial unlocker, nor breaks the one
 // it would replace.
-//
-//nolint:paralleltest // t.Setenv forbids t.Parallel
 func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 	// No mnemonic, and no current unlocker to get the key from
 	t.Setenv(secret.EnvMnemonic, "")
@@ -640,8 +640,6 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 // passphrase unlocker writes in its directory is its metadata: an unlocker
 // directory without metadata is never used, so one interrupted earlier
 // cannot be.
-//
-//nolint:paralleltest // t.Setenv forbids t.Parallel
 func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 

internal/secret/pgpunlocker.go

@@ -155,14 +155,18 @@ func (p *PGPUnlocker) GetDirectory() string {
 	return p.Directory
 }
 
-// GetID implements Unlocker interface - generates ID from GPG key ID
+// GetID implements Unlocker interface - generates ID from GPG key ID.
+// If the metadata has no usable GPG key ID, it warns with the unlocker's
+// directory and returns "pgp-unknown", so listing the other unlockers
+// still works.
 func (p *PGPUnlocker) GetID() string {
 	// Generate ID using GPG key ID: pgp-<keyid>
 	gpgKeyID, err := p.GetGPGKeyID()
 	if err != nil {
-		// The vault metadata is corrupt - this is a fatal error
-		// We cannot continue with a fallback ID as that would mask data corruption
-		panic(fmt.Sprintf("PGP unlocker metadata is corrupt or missing GPG key ID: %v", err))
+		Warn("PGP unlocker metadata is corrupt or missing its GPG key ID",
+			"directory", p.Directory, "error", err)
+
+		return "pgp-unknown"
 	}
 
 	return "pgp-" + gpgKeyID
@@ -197,6 +201,10 @@ func (p *PGPUnlocker) GetGPGKeyID() (string, error) {
 		return "", fmt.Errorf("failed to parse PGP metadata: %w", err)
 	}
 
+	if pgpMetadata.GPGKeyID == "" {
+		return "", fmt.Errorf("PGP metadata: %w", errGPGKeyIDEmpty)
+	}
+
 	return pgpMetadata.GPGKeyID, nil
 }
 

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"
@@ -545,6 +546,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

@@ -26,13 +26,18 @@ var (
 	// as "vault <name> does not exist".
 	ErrVaultNotFound = errors.New("does not exist")
 
+	// ErrVaultExists indicates that a vault to be created already exists.
+	// Composed as "vault <name> already exists".
+	ErrVaultExists = errors.New("already exists")
+
 	// ErrNilValueBuffer indicates a nil value buffer was supplied.
 	ErrNilValueBuffer = errors.New("value buffer is nil")
 
-	// ErrInvalidSecretName indicates a secret name that does not match
-	// the allowed pattern [a-z0-9.\-_/]+. Composed as
-	// "invalid secret name '<name>': must match pattern [a-z0-9.\-_/]+",
-	// or as "invalid secret name: <name>" by GetSecretObject.
+	// ErrInvalidSecretName indicates a secret name that breaks the naming
+	// rule: only ASCII letters, digits, '.', '-', '_' and '/'; not empty;
+	// no leading '.' or '/', no trailing '/', no '//', no '..' path segment.
+	// 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 +53,7 @@ var (
 
 	// ErrVersionNotFound indicates the requested secret version does not
 	// exist. Composed as
-	// "version <version> not found for secret <name>".
+	// "version '<version>' not found for secret '<name>'".
 	ErrVersionNotFound = errors.New("not found for secret")
 
 	// ErrNoVersions indicates the source secret has no versions. Composed

internal/vault/integration_version_test.go

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

internal/vault/management.go

@@ -191,7 +191,11 @@ func processMnemonicForVault(
 	return derivationIndex, publicKeyHash, familyHash, nil
 }
 
-// CreateVault creates a new vault
+// CreateVault creates a new vault and selects it as the current vault. It
+// refuses a vault that already exists before writing anything: creating it
+// again would replace its keys, and its secrets could no longer be
+// decrypted. The commands that call it hold the state directory lock, so no
+// other command can create the vault between the check and the writes.
 func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
@@ -207,12 +211,22 @@ func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
 
 	secret.Debug("Vault name validation passed", "vault_name", name)
 
-	// Create vault directory structure
 	vaultDir := filepath.Join(stateDir, "vaults.d", name)
+
+	exists, err := afero.DirExists(fs, vaultDir)
+	if err != nil {
+		return nil, fmt.Errorf("failed to check if vault exists: %w", err)
+	}
+
+	if exists {
+		return nil, fmt.Errorf("vault %s %w", name, ErrVaultExists)
+	}
+
+	// Create vault directory structure
 	secret.Debug("Creating vault directory structure", "vault_dir", vaultDir)
 
 	// Create main vault directory
-	err := fs.MkdirAll(vaultDir, secret.DirPerms)
+	err = fs.MkdirAll(vaultDir, secret.DirPerms)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create vault directory: %w", err)
 	}

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(
-			"%w '%s': must match pattern [a-z0-9.\\-_/]+",
-			ErrInvalidSecretName, name,
-		)
+		return err
 	}
 
 	secret.Debug("Secret name validation passed", "secret_name", name)
@@ -286,18 +301,31 @@ func updateVersionMetadata(
 	return nil
 }
 
-// GetSecret retrieves a secret from this vault
+// GetSecret retrieves the current version of a secret from this vault
 func (v *Vault) GetSecret(name string) ([]byte, error) {
 	secret.DebugWith("Getting secret from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
 	)
 
-	return v.GetSecretVersion(name, "")
+	// GetSecretObject validates the name and checks that the secret exists
+	secretObj, err := v.GetSecretObject(name)
+	if err != nil {
+		return nil, err
+	}
+
+	currentVersion, err := secret.GetCurrentVersion(v.fs, secretObj.Directory)
+	if err != nil {
+		secret.Debug("Failed to get current version", "error", err, "secret_name", name)
+
+		return nil, fmt.Errorf("failed to get current version: %w", err)
+	}
+
+	return v.GetSecretVersion(name, currentVersion)
 }
 
-// GetSecretVersion retrieves a specific version of a secret (empty version
-// means current)
+// GetSecretVersion retrieves a specific version of a secret. The version
+// must be one of the secret's versions; GetSecret gets the current one.
 func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
@@ -305,8 +333,8 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 		slog.String("version", version),
 	)
 
-	// Validate the name and resolve the version to fetch
-	version, err := v.resolveSecretVersion(name, version)
+	// Validate the name and check that the version exists
+	err := v.checkSecretVersion(name, version)
 	if err != nil {
 		return nil, err
 	}
@@ -400,8 +428,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) {
-		return nil, fmt.Errorf("%w: %s", ErrInvalidSecretName, name)
+	err := ValidateSecretName(name)
+	if err != nil {
+		return nil, err
 	}
 
 	// First check if the secret exists by checking for the metadata file
@@ -650,17 +679,15 @@ func (v *Vault) updatePreviousVersion(
 	return nil
 }
 
-// resolveSecretVersion validates the secret name, verifies the secret and
-// version exist, and resolves an empty version to the current one.
-func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
+// checkSecretVersion validates the secret name and verifies that the secret
+// exists and that version is one of its versions.
+func (v *Vault) checkSecretVersion(name, version string) error {
 	// Validate secret name to prevent path traversal
-	if !isValidSecretName(name) {
+	err := ValidateSecretName(name)
+	if err != nil {
 		secret.Debug("Invalid secret name provided", "secret_name", name)
 
-		return "", fmt.Errorf(
-			"%w '%s': must match pattern [a-z0-9.\\-_/]+",
-			ErrInvalidSecretName, name,
-		)
+		return err
 	}
 
 	// Get vault directory
@@ -668,7 +695,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
@@ -680,50 +707,30 @@ func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
 	if err != nil {
 		secret.Debug("Failed to check if secret exists", "error", err, "secret_name", name)
 
-		return "", fmt.Errorf("failed to check if secret exists: %w", err)
+		return fmt.Errorf("failed to check if secret exists: %w", err)
 	}
 
 	if !exists {
 		secret.Debug("Secret not found in vault", "secret_name", name, "vault_name", v.Name)
 
-		return "", fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
-	}
-
-	// Determine which version to get
-	if version == "" {
-		// Get current version
-		currentVersion, err := secret.GetCurrentVersion(v.fs, secretDir)
-		if err != nil {
-			secret.Debug("Failed to get current version", "error", err, "secret_name", name)
-
-			return "", fmt.Errorf("failed to get current version: %w", err)
-		}
-
-		version = currentVersion
-
-		secret.Debug("Using current version", "version", version, "secret_name", name)
+		return fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
 	}
 
 	// Check if version exists
-	versionPath := filepath.Join(secretDir, "versions", version)
-
-	exists, err = afero.DirExists(v.fs, versionPath)
+	exists, err = secret.VersionExists(v.fs, secretDir, version)
 	if err != nil {
 		secret.Debug("Failed to check if version exists", "error", err, "version", version)
 
-		return "", fmt.Errorf("failed to check if version exists: %w", err)
+		return fmt.Errorf("failed to check if version exists: %w", err)
 	}
 
 	if !exists {
 		secret.Debug("Version not found", "version", version, "secret_name", name)
 
-		return "", fmt.Errorf(
-			"version %s %w %s",
-			version, ErrVersionNotFound, name,
-		)
+		return fmt.Errorf("version '%s' %w '%s'", version, ErrVersionNotFound, name)
 	}
 
-	return version, nil
+	return nil
 }
 
 // createAndSaveVersion generates a new version name, sets the version

internal/vault/secrets_version_test.go

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

internal/vault/unlockers.go

@@ -233,9 +233,10 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
 
 			exists, err := afero.Exists(v.fs, metadataPath)
 			if err != nil {
-				return nil, fmt.Errorf(
-					"failed to check if metadata exists for unlocker %s: %w",
-					file.Name(), err)
+				secret.Warn("Skipping unlocker directory whose metadata file cannot be checked",
+					"directory", file.Name(), "error", err)
+
+				continue
 			}
 
 			if !exists {
@@ -247,16 +248,20 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
 
 			metadataBytes, err := afero.ReadFile(v.fs, metadataPath)
 			if err != nil {
-				return nil, fmt.Errorf(
-					"failed to read metadata for unlocker %s: %w", file.Name(), err)
+				secret.Warn("Skipping unlocker directory with unreadable metadata file",
+					"directory", file.Name(), "error", err)
+
+				continue
 			}
 
 			var metadata UnlockerMetadata
 
 			err = json.Unmarshal(metadataBytes, &metadata)
 			if err != nil {
-				return nil, fmt.Errorf(
-					"failed to parse metadata for unlocker %s: %w", file.Name(), err)
+				secret.Warn("Skipping unlocker directory with corrupt metadata file",
+					"directory", file.Name(), "error", err)
+
+				continue
 			}
 
 			unlockers = append(unlockers, metadata)

script/bootstrap

@@ -6,6 +6,7 @@
 # make, node, yarn, go, or python). Node is used directly if installed;
 # otherwise a pinned version is installed via nvm (installing nvm
 # itself first, from a hash-verified release archive, never curl | sh).
+# golangci-lint is never installed: script/lint runs it in docker.
 #
 # Uncomment the language sections in main() that apply to this repo.
 set -eu
@@ -136,12 +137,6 @@ main() {
 
     # ---- Go repos ----
     if missing go; then pkg_install go golang go go; fi
-    # golangci-lint: packaged in nix, brew, and apk. On apt there is no
-    # package: download a specific release archive from GitHub and
-    # verify its hash (verify_sha256), never curl | sh.
-    if missing golangci-lint; then
-        pkg_install golangci-lint golangci-lint golangci-lint golangci-lint
-    fi
     go mod download
 
     # ---- Python repos ----

script/lint

@@ -1,14 +1,24 @@
 #!/bin/sh
-# script/lint: run the linter.
+# script/lint: run the linter, in docker only. Builds Dockerfile.lint,
+# where golangci-lint runs as a build step.
+#
+# A cached build lints nothing, so --no-cache-filter rebuilds the lint
+# stage on every run, an unchanged tree included. It ignores a stage name
+# that does not exist, so --target names the same stage: a rename then
+# fails the build instead of serving the lint from cache. cacheonly keeps
+# no image; only the build's success matters.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    # CGO is required (Makefile exports this too)
-    export CGO_ENABLED=1
-    golangci-lint run --timeout 5m
+    docker build \
+        --progress=plain \
+        --target lint \
+        --no-cache-filter=lint \
+        --output=type=cacheonly \
+        -f Dockerfile.lint .
 }
 
 main "$@"