Refuse to create a vault that already exists (closes #74)

vault.CreateVault now checks for the vault before writing anything and
fails with "vault NAME already exists" (vault.ErrVaultExists). secret
init and secret vault create call it while holding the state directory
lock, so two creates at once cannot both pass the check. Before, either
command over an existing vault replaced its metadata, passphrase
unlocker and longterm.age, so none of its secrets could be decrypted.

Both commands now ask for the unlocker passphrase before creating the
vault, so one stopped at that prompt leaves no vault without an
unlocker behind, which they would then refuse to create again.

The lock tests set up the vault "work" instead of "default", which init
now refuses to create again.

Model: opus-5-5

.dockerignore

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

.gitignore

@@ -1,34 +1,12 @@
-# OS
 .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.
+**/.DS_Store
 /secret
 *.log
+cli.test
+vault.test
 *.test
 settings.local.json
+
+# Stale files
+.cursorrules
+coverage.out

Dockerfile

@@ -6,16 +6,10 @@ WORKDIR /src
 COPY go.mod go.sum ./
 RUN go mod download
 
-# script/cibuild sets CHECK_EPOCH to the current time, so the RUN steps
-# below run again on each build, an unchanged tree included, while the
-# steps above stay cached. ARG is per stage: the build stage declares it too.
-ARG CHECK_EPOCH
-
 COPY . .
 
 RUN make fmt-check
-# Not make lint: script/lint is a docker build, which cannot run in here.
-RUN golangci-lint run --config .golangci.yml ./...
+RUN make lint
 
 # Build stage — tests and compilation
 # golang 1.24.13-alpine (2026-03-10)
@@ -30,9 +24,6 @@ WORKDIR /build
 COPY go.mod go.sum ./
 RUN go mod download
 
-# As in the lint stage: the RUN steps below run again on each script/cibuild.
-ARG CHECK_EPOCH
-
 COPY . .
 
 RUN make test

Dockerfile.lint

@@ -1,19 +0,0 @@
-# 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 ./...

Makefile

@@ -1,7 +1,13 @@
 export CGO_ENABLED=1
+export DOCKER_HOST := ssh://root@ber1app1.local
 
-.PHONY: default bootstrap setup build test lint fmt fmt-check check docker \
-	docker-run clean install hooks
+# Version information
+VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
+GIT_COMMIT := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
+LDFLAGS := -X 'git.eeqj.de/sneak/secret/internal/cli.Version=$(VERSION)' \
+           -X 'git.eeqj.de/sneak/secret/internal/cli.GitCommit=$(GIT_COMMIT)'
+
+.PHONY: default bootstrap setup test lint fmt fmt-check check docker hooks vet
 
 default: check
 
@@ -11,9 +17,13 @@ bootstrap:
 setup:
 	@script/setup
 
-# Build ./secret; `make build VERSION=x` stamps x instead of `git describe`
-build:
-	@script/build
+build: ./secret
+
+./secret: ./internal/*/*.go ./pkg/*/*.go ./cmd/*/*.go ./go.*
+	go build -v -ldflags "$(LDFLAGS)" -o $@ cmd/secret/main.go
+
+vet:
+	go vet ./...
 
 test:
 	@script/test
@@ -39,7 +49,7 @@ docker-run:
 clean:
 	rm -f ./secret
 
-install: build
+install: ./secret
 	cp ./secret $(HOME)/bin/secret
 
 fmt-check:

README.md

@@ -91,9 +91,6 @@ Lists all available vaults. The current vault is marked.
 
 Creates a new vault with the specified name.
 
-**Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
-are allowed, and a name must not be empty, `.` or `..`.
-
 #### `secret vault select <name>`
 
 Switches to the specified vault for subsequent operations.
@@ -142,9 +139,6 @@ 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
@@ -201,9 +195,7 @@ Creates a new unlocker of the specified type:
 
 **DANGER**: Permanently removes an unlocker. Like Unix `rm`, this command
 does not ask for confirmation.  Cannot remove the last unlocker if the vault
-has secrets unless --force is used. An unlocker directory that
-`secret unlocker list` skips with a warning, because its metadata cannot be
-read or parsed, is removed by the directory name the warning gives.
+has secrets unless --force is used.
 - `--force, -f`: Force removal of last unlocker even if vault has secrets
 - **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase,
   vault data will be PERMANENTLY INACCESSIBLE
@@ -504,21 +496,15 @@ 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, Go module
-  download), idempotently; golangci-lint is not installed, it runs in
-  docker
+- `script/bootstrap` — install all dependencies (Go, golangci-lint, Go
+  module download), idempotently
 - `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/build` — build the `secret` binary into the repo root, stamping
-  the version (`VERSION` from the environment, else `git describe`) and
-  the git commit
 - `script/test` — run `go vet` and the test suite (verbose rerun on
   failure)
-- `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/lint` — run `golangci-lint`
 - `script/fmt` — format all Go code (writes)
 - `script/fmt-check` — check formatting without writing
 - `script/check` — run `script/test`, `script/lint`, and
@@ -526,8 +512,7 @@ them. We provide:
 - `script/docker` — build the Docker image tagged with the project name
 - `script/cibuild` — CI entrypoint: `docker build --ulimit
   memlock=-1:-1 .` (memguard needs mlock; the Dockerfile runs the
-  checks), with a new `CHECK_EPOCH` build argument on every run so the
-  checks run again on an unchanged tree
+  checks)
 - `script/precommit` — pre-commit checks: `go mod tidy` verification,
   then `script/check`
 - `script/install-precommit` — install the git pre-commit hook that

TODO.md

@@ -25,82 +25,6 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
-- 2026-10-04: `secret unlocker add pgp` works on Linux
-  (https://git.eeqj.de/sneak/secret/issues/88). `CreatePGPUnlocker` gets
-  the vault's long-term key as adding a passphrase unlocker does, with the
-  vault's `GetOrDeriveLongTermKey`, now part of `VaultInterface`: from the
-  mnemonic, checked against the vault, or else from the current unlocker.
-  Before, it used the keychain unlocker's helper, which on every platform
-  but macOS always failed. A test adds a PGP unlocker for a throwaway GPG
-  key, getting the long-term key once from the mnemonic and once from a
-  passphrase unlocker, and reads a secret through the new unlocker.
-- 2026-10-04: A vault name may use only lowercase ASCII letters, digits,
-  `.`, `-` and `_`, and must not be empty, `.` or `..`
-  (https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md`
-  state the rule. `vault create`, `vault import`, `vault select`,
-  `vault remove`, both vault names of `mv` and shell completion of a
-  `vault:secret` argument check the name as typed with
-  `vault.ValidateVaultName` before building any path from it. Before,
-  `vault import ..` wrote a long-term key and an unlocker into the state
-  directory itself, and `vault select ..` made that the current vault.
-- 2026-10-04: `script/cibuild` runs the checks again on an unchanged
-  tree (https://git.eeqj.de/sneak/secret/issues/54). It passes the
-  current time as the `CHECK_EPOCH` build argument, which both the lint
-  and the build stage of the `Dockerfile` declare after their module
-  download, so the `RUN` steps below the argument run again on each
-  build while the base images and module downloads stay cached. Before,
-  a second run on the same tree took every check from the build cache
-  and reported success having run nothing.
-- 2026-10-04: A failed unlocker add no longer leaves a partial unlocker
-  directory (https://git.eeqj.de/sneak/secret/issues/48).
-  `secret unlocker add pgp` resolves the GPG key's fingerprint once, for
-  its duplicate check, and passes it to `CreatePGPUnlocker` to record.
-  `CreatePGPUnlocker` and `CreateKeychainUnlocker` get the long-term key
-  and encrypt everything before writing anything. All four unlocker
-  types write their files through `secret.WriteDir`: a new unlocker is
-  built in a temporary directory, renamed into place when complete and
-  removed on a failure. One added under the directory name of an
-  existing unlocker is still written into that directory in place
-  (https://git.eeqj.de/sneak/secret/issues/71).
-- 2026-10-04: `secret unlocker select` and `secret unlocker remove`
-  skip, with the warning `unlocker list` gives, an unlocker directory
-  whose metadata file cannot be checked for, read or parsed, instead of
-  failing when it sorts before the unlocker asked for. Such a directory,
-  or one without a metadata file, is removed by its directory name, the
-  name the warning gives; only the directory is removed, since its type
-  is unknown. Removing one whose metadata file is missing or corrupt
-  never counts as removing the last unlocker. Removing one whose metadata
-  file cannot be checked for or read always does, since it may be the
-  only working unlocker, so in a vault with secrets it needs `--force`.
-- 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, missing required flag, or flags that
-  break a flag group (mutually exclusive, required together, one
-  required). The root command's `PersistentPreRunE` turns usage off.
-  Cobra checks arguments and flag values before that hook but required
-  flags and flag groups only after it, so the hook checks those two
-  first. Root `SilenceUsage` would have hidden usage for all of these.
-- 2026-10-04: `secret get` keeps the secret in locked memory until it
-  writes it out (https://git.eeqj.de/sneak/secret/issues/37):
-  `Vault.GetSecret` and `Vault.GetSecretVersion` return a
-  `*memguard.LockedBuffer`, which every caller destroys, and `secret get`
-  writes its bytes straight to stdout, still with no trailing newline.
-  Before, the value was copied into ordinary memory that nothing wiped,
-  and `get --version` also wrote it to the debug log.
-- 2026-10-04: The `Makefile` no longer sets `DOCKER_HOST`, so its docker
-  targets use the local docker daemon, or whatever `DOCKER_HOST` the
-  environment sets. `make build` calls the new `script/build`, which
-  stamps the version (`VERSION` from the environment, else
-  `git describe`) and the git commit as before. `build`, `clean`,
-  `install` and `docker-run` are in `.PHONY`; `make install` depends on
-  `build`. The `vet` target is gone: `script/test` runs `go vet` first.
-- 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`,
@@ -110,27 +34,6 @@ Bring the repo into policy compliance in one commit:
   `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
@@ -166,18 +69,14 @@ Bring the repo into policy compliance in one commit:
     - 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;
-    - data under a `.tmp-` name in the state directory: a secret,
-      version or unlocker being added, or the secret, version, unlocker
-      or vault being removed, encrypted keys included. Nothing deletes
-      it; it must be deleted by hand
+    - from an unlocker add stopped before its metadata is written, a
+      directory that `unlocker list` warns about and `unlocker rm`
+      cannot remove;
+    - data under a `.tmp-` name in the state directory: a secret or
+      version being added, or the secret, version, unlocker or vault
+      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: The checks run before changing a vault now stop with an
-  error naming the path and cause when they cannot read what they
-  inspect, instead of reading the failure as "nothing there": the
-  duplicate check before `unlocker add pgp` (an unreadable
-  `unlockers.d` or unlocker metadata file), the secret count that
-  guards removing the last unlocker and removing a vault, and the
-  existing long-term key check before `vault import`.
 - 2026-10-03: `version rm`, `version promote` and `get --version`
   accept a version only if it is one of the versions `version list`
   lists for that secret, compared as typed before any path is built
@@ -277,6 +176,8 @@ Bring the repo into policy compliance in one commit:
       209-216); non-constant-time public key compare (vault.go:95-100).
 - High priority:
     - 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).

internal/cli/cli.go

@@ -68,3 +68,8 @@ func (cli *Instance) SetStateDir(stateDir string) {
 func (cli *Instance) GetStateDir() string {
 	return cli.stateDir
 }
+
+// Print outputs to the command's configured output writer
+func (cli *Instance) Print(a ...any) (int, error) {
+	return fmt.Fprint(cli.cmd.OutOrStdout(), a...)
+}

internal/cli/completions.go

@@ -123,9 +123,7 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
 }
 
 // completeVaultQualifiedSecrets completes "vault:secret" references once a
-// colon is present in the input. It completes nothing when the vault part
-// is not a valid vault name, so that a name such as ".." cannot list a
-// directory outside vaults.d.
+// colon is present in the input
 func completeVaultQualifiedSecrets(
 	fs afero.Fs, stateDir, toComplete string,
 ) []string {
@@ -136,10 +134,6 @@ func completeVaultQualifiedSecrets(
 	vaultName := parts[0]
 	secretPrefix := parts[1]
 
-	if vault.ValidateVaultName(vaultName) != nil {
-		return nil
-	}
-
 	vlt := vault.NewVault(fs, stateDir, vaultName)
 
 	secrets, err := vlt.ListSecrets()

internal/cli/completions_test.go

@@ -1,41 +0,0 @@
-//nolint:testpackage // white-box test of unexported internals
-package cli
-
-import (
-	"path/filepath"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// TestVaultSecretCompletionRejectsInvalidVaultName is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/68: completing a `vault:secret`
-// argument lists nothing when the vault part is not a valid vault name, even
-// where that name, joined onto vaults.d, leads to a secrets.d directory.
-func TestVaultSecretCompletionRejectsInvalidVaultName(t *testing.T) {
-	t.Parallel()
-
-	const (
-		stateDir = "/state"
-		dirPerm  = 0o700
-	)
-
-	fs := afero.NewMemMapFs()
-
-	// The vault "work" holds the secret "x". So does every directory an
-	// invalid name below would lead to from vaults.d.
-	for _, vaultName := range []string{"work", ".", "..", "a/b"} {
-		secretDir := filepath.Join(stateDir, "vaults.d", vaultName, "secrets.d", "x")
-		require.NoError(t, fs.MkdirAll(secretDir, dirPerm))
-	}
-
-	assert.Equal(t, []string{"work:x"},
-		completeVaultQualifiedSecrets(fs, stateDir, "work:"))
-
-	for _, toComplete := range []string{".:", "..:", "a/b:"} {
-		assert.Empty(t, completeVaultQualifiedSecrets(fs, stateDir, toComplete),
-			"completing %q", toComplete)
-	}
-}

internal/cli/create_vault_test.go

@@ -1,7 +1,6 @@
 package cli_test
 
 import (
-	"bytes"
 	"testing"
 
 	"git.eeqj.de/sneak/secret/internal/cli"
@@ -19,8 +18,6 @@ import (
 // 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)
@@ -90,11 +87,7 @@ func TestCreateExistingVaultChangesNothing(t *testing.T) {
 	for _, name := range vaults {
 		value, err := vault.NewVault(fs, testStateDir, name).GetSecret("x")
 		require.NoError(t, err)
-
-		unchanged := bytes.Equal([]byte("value"), value.Bytes())
-		value.Destroy()
-
-		require.True(t, unchanged, "vault %q kept its secret", name)
+		require.Equal(t, "value", string(value))
 	}
 }
 
@@ -103,8 +96,6 @@ func TestCreateExistingVaultChangesNothing(t *testing.T) {
 // `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)
 

internal/cli/lock_test.go

@@ -2,11 +2,9 @@
 package cli
 
 import (
-	"bytes"
 	"io"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -27,6 +25,11 @@ 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"
 
@@ -147,8 +150,7 @@ func TestConcurrentAddsKeepEveryVersion(t *testing.T) {
 				value, err := vlt.GetSecretVersion("shared", version)
 				require.NoError(t, err)
 
-				values[string(value.Bytes())] = true
-				value.Destroy()
+				values[string(value)] = true
 			}
 
 			assert.Len(t, values, adds+1, "every add stored its own value")
@@ -322,28 +324,10 @@ 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 before it waits for the lock, and must succeed once
-// the lock is released.
+// nor change anything while the lock is held, and must succeed once it is
+// released.
 func requireWaitsForLock(
 	t *testing.T,
 	withUnlocker bool,
@@ -372,20 +356,14 @@ func requireWaitsForLock(
 
 	go func() { done <- run(cli, olderVersion, unlockerID) }()
 
-	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):
-		}
+	select {
+	case err := <-done:
+		t.Fatalf("finished while the lock was held, with error %v", err)
+	case <-time.After(heldWait):
 	}
 
 	assert.Equal(t, before, stateDirModTimes(t, fs),
-		"changed the state directory before waiting for the lock")
+		"changed the state directory while the lock was held")
 
 	release()
 

internal/cli/move_test.go

@@ -1,15 +1,9 @@
 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"
 )
@@ -48,25 +42,26 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
 			"mv work:nosuch work:y", "work:nosuch", "work:y", false,
 			"secret 'nosuch' not found",
 		},
-		// Only an existing vault is used.
+		// Only an existing vault is used, so ".." cannot reach the state
+		// directory itself.
 		{
-			"mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
-			"vault 'nosuch' does not exist",
+			"mv --force ..:x ..:y", "..:x", "..:y", true,
+			"vault '..' does not exist",
 		},
-		// Each of these spells "work" a second way. The spelling is not a
-		// valid vault name, so the move is not taken for a move between two
-		// vaults, which would delete the destination, here the source.
+		// 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.ValidateVaultName("work/").Error(),
+			"vault 'work/' does not exist",
 		},
 		{
 			"mv --force work/:x work:", "work/:x", "work:", true,
-			vault.ValidateVaultName("work/").Error(),
+			"vault 'work/' does not exist",
 		},
 		{
 			"mv --force work:x ./work:x", workX, "./work:x", true,
-			vault.ValidateVaultName("./work").Error(),
+			"vault './work' does not exist",
 		},
 	}
 
@@ -102,133 +97,3 @@ func TestMoveWithinOtherVaultKeepsCurrentVault(t *testing.T) {
 	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)
-
-			defer value.Destroy()
-
-			require.Equal(t, []byte("value"), value.Bytes())
-
-			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)
-
-	defer value.Destroy()
-
-	require.Equal(t, []byte("upper"), value.Bytes())
-
-	_, err = vlt.GetSecret("Foo")
-	require.ErrorIs(t, err, vault.ErrSecretNotFound)
-}

internal/cli/path_traversal_test.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"slices"
 	"strings"
-	"sync"
 	"testing"
 
 	"git.eeqj.de/sneak/secret/internal/cli"
@@ -33,20 +32,9 @@ const (
 	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 {
@@ -54,27 +42,21 @@ func newTwoVaultFs(t *testing.T) afero.Fs {
 
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	twoVaultsOnce.Do(func() {
-		fs := afero.NewMemMapFs()
+	fs := afero.NewMemMapFs()
 
-		for _, name := range []string{"work", "default"} {
-			vlt, err := vault.CreateVault(fs, testStateDir, name)
-			require.NoError(t, err)
+	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.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
+		require.NoError(t, err)
 
-			_, err = vlt.CreatePassphraseUnlocker(
-				memguard.NewBufferFromBytes([]byte(testPassphrase)))
-			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)
+	return fs
 }
 
 // snapshotStateDir maps every file under the state directory to its
@@ -292,58 +274,6 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
 	}
 }
 
-// TestInvalidVaultNameLeavesStateUnchanged is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/68, where
-// `secret vault import ..` wrote a long-term key and an unlocker into the
-// state directory itself, and `secret vault select ..` made it the current
-// vault. Each command that takes a vault name must reject an invalid one
-// before building a path from it. The mnemonic and the passphrase are set,
-// and moves and removals use --force, so that only the name check stands
-// in the way.
-//
-//nolint:paralleltest // newTwoVaultFs uses t.Setenv
-func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
-	before := snapshotStateDir(t, newTwoVaultFs(t))
-
-	t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
-
-	cmd := &cobra.Command{}
-
-	// Each command is a format with %q where the vault name goes.
-	commands := []struct {
-		command string
-		run     func(c *cli.Instance, name string) error
-	}{
-		{"vault create %q", func(c *cli.Instance, name string) error {
-			return c.CreateVault(cmd, name)
-		}},
-		{"vault import %q", func(c *cli.Instance, name string) error {
-			return c.VaultImport(cmd, name)
-		}},
-		{"vault select %q", func(c *cli.Instance, name string) error {
-			return c.SelectVault(cmd, name)
-		}},
-		{"vault remove --force %q", func(c *cli.Instance, name string) error {
-			return c.RemoveVault(cmd, name, true)
-		}},
-		{"mv --force %q:x work:x", func(c *cli.Instance, name string) error {
-			return c.MoveSecret(cmd, name+":x", "work:x", true)
-		}},
-		{"mv --force default:x %q:x", func(c *cli.Instance, name string) error {
-			return c.MoveSecret(cmd, "default:x", name+":x", true)
-		}},
-	}
-
-	for _, tt := range commands {
-		for _, name := range []string{"", ".", "..", "a/b"} {
-			t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) {
-				requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name),
-					func(c *cli.Instance) error { return tt.run(c, name) })
-			})
-		}
-	}
-}
-
 // TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
 // with a version that is not the current one removes that version and
 // changes nothing else.

internal/cli/root.go

@@ -46,30 +46,9 @@ func newRootCmd() *cobra.Command {
 		Short: "A simple secrets manager",
 		Long: `A simple secrets manager to store and retrieve sensitive ` +
 			`information securely.`,
-		// Cobra prints the error a command returns; Entry does not.
+		// Ensure usage is shown after errors
+		SilenceUsage:  false,
 		SilenceErrors: false,
-		// Usage belongs only to a command called wrongly. Cobra has
-		// checked its arguments and flag values before this runs, but
-		// checks required flags (ValidateRequiredFlags) and flag groups
-		// (ValidateFlagGroups) only after it, so both are checked here
-		// to keep usage for them. 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
-			}
-
-			err = cmd.ValidateFlagGroups()
-			if err != nil {
-				return err
-			}
-
-			cmd.SilenceUsage = true
-
-			return nil
-		},
 	}
 
 	secret.Debug("Adding subcommands to root command")

internal/cli/secrets.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"os"
 	"path/filepath"
 	"slices"
 	"strings"
@@ -414,6 +413,9 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	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 {
@@ -424,10 +426,9 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	if err != nil {
 		return err
 	}
-	defer value.Destroy()
 
-	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	// Print the secret value to stdout
+	_, _ = cli.Print(string(value))
 
 	return nil
 }
@@ -440,6 +441,9 @@ func (cli *Instance) GetSecretWithVersion(
 	secret.Debug("GetSecretWithVersion called",
 		"secretName", secretName, "version", version)
 
+	// Store the command for output
+	cli.cmd = cmd
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -455,15 +459,22 @@ func (cli *Instance) GetSecretWithVersion(
 
 		return err
 	}
-	defer value.Destroy()
 
-	secret.Debug("Got secret value", "valueLength", value.Size())
+	secret.Debug("Got secret value", "valueLength", len(value))
 
-	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	// Print the secret value to stdout
+	_, _ = cli.Print(string(value))
 
 	secret.Debug("Printed value to stdout")
 
+	// Debug: Log what we're actually printing
+	secret.Debug("Secret retrieval debug info",
+		"secretName", secretName,
+		"version", version,
+		"valueLength", len(value),
+		"valueAsString", string(value),
+		"isEmpty", len(value) == 0)
+
 	return nil
 }
 
@@ -811,9 +822,9 @@ func (cli *Instance) moveSecret(
 			cmd, vlt, srcSecretName, destSecretName, force)
 	}
 
-	// Both vault names must be valid and name existing vaults exactly, 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.
+	// 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
@@ -833,15 +844,9 @@ func (cli *Instance) moveSecret(
 		cmd, srcVault, srcSecretName, destVault, destSecretName, force)
 }
 
-// existingVault returns the vault with the given name, or an error if the
-// name is not a valid vault name or there is no such vault. Unlike
-// vault.SelectVault, it leaves the current vault as it is.
+// 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) {
-	err := vault.ValidateVaultName(name)
-	if err != nil {
-		return nil, err
-	}
-
 	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list vaults: %w", err)
@@ -885,18 +890,6 @@ 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)
@@ -923,31 +916,6 @@ func (cli *Instance) moveSecretWithinVault(
 	return nil
 }
 
-// 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.
@@ -972,27 +940,6 @@ func (cli *Instance) moveSecretCrossVault(
 			srcSecretName, errSecretNotFound, srcVault.Name)
 	}
 
-	// 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)
-	}
-
-	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 {

internal/cli/secrets_size_test.go

@@ -143,10 +143,7 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
+	assert.Equal(t, testData, retrievedValue,
 		"Retrieved secret should match original (without newline)")
 }
 
@@ -196,11 +193,7 @@ func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
-		"Retrieved secret should match original")
+	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
 }
 
 // TestAddSecretVariousSizes tests adding secrets of various sizes through stdin
@@ -382,10 +375,7 @@ func TestAddSecretBufferGrowth(t *testing.T) {
 			// Verify the secret was stored correctly
 			retrievedValue, err := vlt.GetSecret(secretName)
 			require.NoError(t, err)
-
-			defer retrievedValue.Destroy()
-
-			assert.Equal(t, testData, retrievedValue.Bytes(),
+			assert.Equal(t, testData, retrievedValue,
 				"Retrieved secret should match original exactly")
 		})
 	}
@@ -426,11 +416,7 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret("streaming-test")
 	require.NoError(t, err)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
-		"Retrieved secret should match original")
+	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
 }
 
 // slowReader simulates a reader that returns data in small chunks

internal/cli/unlockers.go

@@ -49,6 +49,7 @@ var (
 		"is already added as an unlocker")
 	errUnsupportedUnlockerType = errors.New("unsupported unlocker type")
 	errLastUnlocker            = errors.New("refusing to remove last unlocker")
+	errUnlockerExists          = errors.New("unlocker already exists")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -685,8 +686,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	// Resolve the GPG key ID to its fingerprint, once: the duplicate check
-	// and the new unlocker's metadata both use this result
+	// Resolve the GPG key ID to its fingerprint
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -695,20 +695,12 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	// Check if this GPG key is already added
 	expectedID := "pgp-" + fingerprint
 
-	exists, err := cli.checkUnlockerExists(vlt, expectedID)
+	err = cli.checkUnlockerExists(vlt, expectedID)
 	if err != nil {
-		return fmt.Errorf(
-			"could not check whether GPG key %s is already an unlocker: %w",
-			gpgKeyID, err,
-		)
-	}
-
-	if exists {
 		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
 	}
 
-	pgpUnlocker, err := secret.CreatePGPUnlocker(
-		cli.fs, cli.stateDir, gpgKeyID, fingerprint)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID)
 	if err != nil {
 		return err
 	}
@@ -722,8 +714,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	return nil
 }
 
-// UnlockersRemove removes an unlocker, holding the state directory lock
-// while removeUnlocker runs
+// UnlockersRemove removes an unlocker with safety checks
 func (cli *Instance) UnlockersRemove(
 	unlockerID string, force bool, cmd *cobra.Command,
 ) error {
@@ -733,56 +724,20 @@ func (cli *Instance) UnlockersRemove(
 	}
 	defer release()
 
-	return cli.removeUnlocker(unlockerID, force, cmd)
-}
-
-// removeUnlocker removes an unlocker with safety checks
-func (cli *Instance) removeUnlocker(
-	unlockerID string, force bool, cmd *cobra.Command,
-) error {
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
 	}
 
-	// Get list of unlockers. It leaves out a directory whose metadata file
-	// is missing or cannot be checked for, read or parsed.
+	// Get list of unlockers
 	unlockers, err := vlt.ListUnlockers()
 	if err != nil {
 		return fmt.Errorf("failed to list unlockers: %w", err)
 	}
 
-	vaultDir, err := vlt.GetDirectory()
-	if err != nil {
-		return fmt.Errorf("failed to get vault directory: %w", err)
-	}
-
-	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
-
 	// Check if we're removing the last unlocker
-	removingLast := false
-
 	if len(unlockers) == 1 {
-		lastID, err := findUnlockerIDByMetadata(
-			cli.fs, unlockersDir, unlockers[0], true)
-		if err != nil {
-			return err
-		}
-
-		removingLast = lastID == unlockerID
-	}
-
-	// unlockerID may instead name a directory left out of the list. If its
-	// metadata file is missing or corrupt it is not a working unlocker, so
-	// removing it never removes the last one. If the file cannot be checked
-	// for or read, the unlocker may be the only working one, so removing it
-	// counts as removing the last unlocker.
-	if metadataUnreadable(cli.fs, filepath.Join(unlockersDir, unlockerID)) {
-		removingLast = true
-	}
-
-	if removingLast {
 		// Check if vault has secrets
 		numSecrets, err := vlt.NumSecrets()
 		if err != nil {
@@ -816,20 +771,6 @@ func (cli *Instance) removeUnlocker(
 	return nil
 }
 
-// metadataUnreadable reports whether checking for or reading the metadata
-// file in the unlocker directory unlockerDir fails. A missing file is not
-// a failure.
-func metadataUnreadable(fs afero.Fs, unlockerDir string) bool {
-	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
-
-	exists, err := afero.Exists(fs, metadataPath)
-	if err == nil && exists {
-		_, err = afero.ReadFile(fs, metadataPath)
-	}
-
-	return err != nil
-}
-
 // UnlockerSelect selects an unlocker as current
 func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
@@ -847,65 +788,44 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
-// checkUnlockerExists reports whether the vault already has an unlocker
-// with the given ID. It returns an error, and no answer, when unlockers.d
-// or an unlocker's metadata file cannot be read; the caller must then not
-// create the unlocker. It reads unlockers.d itself because
-// vault.ListUnlockers skips an unlocker it cannot read, which suits
-// `unlocker list` but not this check: the skipped unlocker may be the
-// duplicate. A directory whose metadata file is missing or corrupt is not
-// a working unlocker and is passed over.
-func (cli *Instance) checkUnlockerExists(
-	vlt *vault.Vault, unlockerID string,
-) (bool, error) {
+// checkUnlockerExists checks if an unlocker with the given ID exists
+func (cli *Instance) checkUnlockerExists(vlt *vault.Vault, unlockerID string) error {
+	// Get the list of unlockers and check if any match the ID
+	unlockers, err := vlt.ListUnlockers()
+	if err != nil {
+		secret.Warn("Could not list unlockers during duplicate check", "error", err)
+
+		return nil // If we can't list unlockers, assume it doesn't exist
+	}
+
+	// Get vault directory to construct unlocker instances
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		return false, fmt.Errorf("failed to get vault directory: %w", err)
+		secret.Warn("Could not get vault directory during duplicate check",
+			"error", err)
+
+		return nil
 	}
 
+	// Check each unlocker's ID
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
-	entries, err := afero.ReadDir(cli.fs, unlockersDir)
-	if errors.Is(err, os.ErrNotExist) {
-		return false, nil
-	}
-
-	if err != nil {
-		return false, fmt.Errorf(
-			"failed to read unlockers directory %s: %w", unlockersDir, err,
-		)
-	}
-
-	for _, entry := range entries {
-		if !entry.IsDir() {
-			continue
-		}
-
-		unlockerDir := filepath.Join(unlockersDir, entry.Name())
-
-		metadataBytes, err := afero.ReadFile(
-			cli.fs, filepath.Join(unlockerDir, "unlocker-metadata.json"))
-		if errors.Is(err, os.ErrNotExist) {
-			continue
-		}
-
+	for _, metadata := range unlockers {
+		// Construct the unlocker matching this metadata to get its ID
+		id, err := findUnlockerIDByMetadata(cli.fs, unlockersDir, metadata, true)
 		if err != nil {
-			return false, fmt.Errorf(
-				"failed to read metadata of unlocker %s: %w", unlockerDir, err,
-			)
-		}
+			secret.Warn(
+				"Could not read unlockers directory during duplicate check, "+
+					"skipping unlocker",
+				"unlockers_dir", unlockersDir, "error", err)
 
-		var metadata secret.UnlockerMetadata
-
-		err = json.Unmarshal(metadataBytes, &metadata)
-		if err != nil {
 			continue
 		}
 
-		if unlockerIDFromDir(cli.fs, unlockerDir, metadata, true) == unlockerID {
-			return true, nil
+		if id != "" && id == unlockerID {
+			return errUnlockerExists
 		}
 	}
 
-	return false, nil
+	return nil
 }

internal/cli/unlockers_add_test.go

@@ -1,105 +0,0 @@
-//nolint:testpackage // white-box test of unexported internals
-package cli
-
-import (
-	"path/filepath"
-	"testing"
-
-	"git.eeqj.de/sneak/secret/internal/secret"
-	"git.eeqj.de/sneak/secret/internal/vault"
-	"github.com/awnumar/memguard"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// unknownTestGPGUserID is a GPG user ID that no key in the test keyring has.
-const unknownTestGPGUserID = "not-in-keyring@example.com"
-
-// The secret TestAddPGPUnlocker stores, then reads through the new unlocker.
-const (
-	addTestSecretName  = "api-key"
-	addTestSecretValue = "value"
-)
-
-// TestAddPGPUnlocker adds a PGP unlocker for a throwaway GPG key to a vault
-// with a passphrase unlocker, getting the vault's long-term key from the
-// mnemonic or, with the mnemonic unset, from the passphrase unlocker. It
-// then reads a secret with neither the mnemonic nor the passphrase set, so
-// through the new unlocker, which the add selects.
-func TestAddPGPUnlocker(t *testing.T) {
-	newTestGPGKey(t)
-
-	tests := []struct {
-		name string
-		// mnemonic is the mnemonic set while the unlocker is added.
-		mnemonic string
-	}{
-		{"long-term key from the mnemonic", testMnemonic},
-		{"long-term key from the current unlocker", ""},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Setenv(secret.EnvMnemonic, testMnemonic)
-			t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
-
-			fs := afero.NewMemMapFs()
-			vlt, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName)
-			require.NoError(t, err)
-
-			err = vlt.AddSecret(addTestSecretName,
-				memguard.NewBufferFromBytes([]byte(addTestSecretValue)), false)
-			require.NoError(t, err)
-
-			_, err = vlt.CreatePassphraseUnlocker(
-				memguard.NewBufferFromBytes([]byte(testPassphrase)))
-			require.NoError(t, err)
-
-			t.Setenv(secret.EnvMnemonic, test.mnemonic)
-
-			instance, cmd := newTestInstance(fs)
-			cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
-			require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd))
-
-			t.Setenv(secret.EnvMnemonic, "")
-			t.Setenv(secret.EnvUnlockPassphrase, "")
-
-			reopened := vault.NewVault(fs, listTestStateDir, listTestVaultName)
-
-			current, err := reopened.GetCurrentUnlocker()
-			require.NoError(t, err)
-			assert.Equal(t, unlockerTypePGP, current.GetType())
-
-			value, err := reopened.GetSecret(addTestSecretName)
-			require.NoError(t, err)
-
-			defer value.Destroy()
-
-			assert.Equal(t, addTestSecretValue, value.String())
-		})
-	}
-}
-
-// TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
-// the keyring does not hold fails at looking up the key's fingerprint and
-// leaves no new unlocker directory. The error must come from the lookup: a
-// lookup moved after anything is written would also come after getting the
-// vault's long-term key, which fails first here: this vault's unlockers hold
-// no keys.
-//
-//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
-func TestAddPGPUnlockerUnknownKey(t *testing.T) {
-	newTestGPGKey(t)
-
-	base := newListTestVault(t, 1)
-	instance, cmd := newTestInstance(base)
-	cmd.Flags().String("keyid", unknownTestGPGUserID, "")
-
-	err := instance.addPGPUnlocker(cmd)
-
-	require.ErrorContains(t, err, "failed to resolve GPG key fingerprint")
-	assertDirEntries(t, base,
-		filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
-		listTestUnlockerDirOne)
-}

internal/cli/unlockers_corrupt_test.go

@@ -1,167 +0,0 @@
-// Corrupt Unlocker Tests
-//
-// `secret unlocker select` and `secret unlocker remove` find an unlocker
-// by its ID. These tests give the first unlocker, which sorts before the
-// one the commands act on, metadata that is not JSON, and check that the
-// commands step past it, and that it can itself be removed by its
-// directory name, which `secret unlocker list` names in its warning. A
-// last test checks that an unlocker whose metadata file cannot be read is
-// removed by its directory name only as the last unlocker is.
-
-//nolint:testpackage // white-box test of unexported internals
-package cli
-
-import (
-	"path/filepath"
-	"testing"
-
-	"git.eeqj.de/sneak/secret/internal/vault"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// newCorruptUnlockerVault returns the two-unlocker test vault with the
-// metadata of the first unlocker replaced by text that is not JSON.
-func newCorruptUnlockerVault(t *testing.T) *afero.MemMapFs {
-	t.Helper()
-
-	fs := newListTestVault(t, 2)
-	require.NoError(t, afero.WriteFile(fs,
-		filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName,
-			listTestUnlockerDirOne, listTestMetadataFileName),
-		[]byte("not json"), listTestFilePerm))
-
-	return fs
-}
-
-// TestUnlockerSelectSkipsCorruptUnlocker asserts that the second unlocker
-// can be selected, and that the corrupt one, having no type to be used as,
-// cannot be selected by its directory name.
-func TestUnlockerSelectSkipsCorruptUnlocker(t *testing.T) {
-	t.Parallel()
-
-	fs := newCorruptUnlockerVault(t)
-	instance, _ := newTestInstance(fs)
-
-	require.NoError(t, instance.UnlockerSelect("pgp-"+listTestGPGKeyID+"B"))
-
-	current, err := afero.ReadFile(fs,
-		filepath.Join(testVaultDir(listTestVaultName), "current-unlocker"))
-	require.NoError(t, err)
-	assert.Equal(t, listTestUnlockerDirTwo, string(current))
-
-	err = instance.UnlockerSelect(listTestUnlockerDirOne)
-	require.ErrorIs(t, err, vault.ErrUnlockerNotFound)
-}
-
-// TestUnlockerRemoveWithCorruptUnlocker asserts that the second unlocker
-// can be removed, unless the vault holds secrets: the corrupt unlocker
-// cannot unlock the vault, so the second is its last. The corrupt one can
-// be removed by its directory name without --force even then.
-func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name        string
-		unlockerID  string
-		withSecret  bool
-		wantErr     error
-		wantEntries []string
-	}{
-		{
-			name:        "the other unlocker",
-			unlockerID:  "pgp-" + listTestGPGKeyID + "B",
-			wantEntries: []string{listTestUnlockerDirOne},
-		},
-		{
-			name:       "the other unlocker, the last one, with secrets",
-			unlockerID: "pgp-" + listTestGPGKeyID + "B",
-			withSecret: true,
-			wantErr:    errLastUnlocker,
-			wantEntries: []string{
-				listTestUnlockerDirOne, listTestUnlockerDirTwo,
-			},
-		},
-		{
-			name:        "the corrupt unlocker by its directory name",
-			unlockerID:  listTestUnlockerDirOne,
-			withSecret:  true,
-			wantEntries: []string{listTestUnlockerDirTwo},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			fs := newCorruptUnlockerVault(t)
-			if tt.withSecret {
-				writeTestSecret(t, fs, testVaultDir(listTestVaultName))
-			}
-
-			instance, cmd := newTestInstance(fs)
-
-			err := instance.UnlockersRemove(tt.unlockerID, false, cmd)
-			require.ErrorIs(t, err, tt.wantErr)
-
-			assertDirEntries(t, fs,
-				filepath.Join(testVaultDir(listTestVaultName),
-					listTestUnlockersDirName),
-				tt.wantEntries...)
-		})
-	}
-}
-
-// TestUnlockerRemoveWithUnreadableMetadata asserts that removing the only
-// unlocker of a vault with secrets by its directory name, when its
-// metadata file cannot be checked for or read, is refused without --force:
-// listing leaves it out, but it may still be the vault's only working
-// unlocker. With --force it is removed. The state directory lock refuses
-// the failing filesystem, so the test calls removeUnlocker, which
-// UnlockersRemove runs once it holds the lock.
-func TestUnlockerRemoveWithUnreadableMetadata(t *testing.T) {
-	t.Parallel()
-
-	vaultDir := testVaultDir(listTestVaultName)
-	unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
-	failingPath := filepath.Join(unlockersDir, 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()
-
-			base := newListTestVault(t, 1)
-			writeTestSecret(t, base, vaultDir)
-
-			instance, cmd := newTestInstance(tt.wrap(base))
-
-			err := instance.removeUnlocker(listTestUnlockerDirOne, false, cmd)
-			require.ErrorIs(t, err, errLastUnlocker)
-			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
-
-			require.NoError(t,
-				instance.removeUnlocker(listTestUnlockerDirOne, true, cmd))
-			assertDirEntries(t, base, unlockersDir)
-		})
-	}
-}

internal/cli/unreadable_dir_test.go

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

internal/cli/usage_test.go

@@ -1,46 +0,0 @@
-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

@@ -401,12 +401,8 @@ func (cli *Instance) vaultImportPreflight(
 	// Check if vault already has a public key
 	pubKeyPath := vaultDir + "/pub.age"
 
-	exists, err = afero.Exists(cli.fs, pubKeyPath)
-	if err != nil {
-		return "", "", "", fmt.Errorf("failed to check %s: %w", pubKeyPath, err)
-	}
-
-	if exists {
+	_, err = cli.fs.Stat(pubKeyPath)
+	if err == nil {
 		return "", "", "", fmt.Errorf("vault '%s' %w",
 			vaultName, errVaultHasLongTermKey)
 	}
@@ -462,11 +458,6 @@ func updateVaultImportMetadata(
 // VaultImport imports a mnemonic into a specific vault, holding the state
 // directory lock while importMnemonic runs
 func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
-	err := vault.ValidateVaultName(vaultName)
-	if err != nil {
-		return err
-	}
-
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
@@ -571,26 +562,17 @@ func (cli *Instance) importMnemonic(cmd *cobra.Command, vaultName string) error
 }
 
 // vaultHasSecrets reports whether the vault directory contains any secrets
-func (cli *Instance) vaultHasSecrets(vaultDir string) (bool, error) {
+func (cli *Instance) vaultHasSecrets(vaultDir string) bool {
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
-	exists, err := afero.DirExists(cli.fs, secretsDir)
-	if err != nil {
-		return false, fmt.Errorf("failed to check secrets directory %s: %w",
-			secretsDir, err)
-	}
-
+	exists, _ := afero.DirExists(cli.fs, secretsDir)
 	if !exists {
-		return false, nil
+		return false
 	}
 
 	entries, err := afero.ReadDir(cli.fs, secretsDir)
-	if err != nil {
-		return false, fmt.Errorf("failed to read secrets directory %s: %w",
-			secretsDir, err)
-	}
 
-	return len(entries) > 0, nil
+	return err == nil && len(entries) > 0
 }
 
 // switchAwayFromVault selects another vault as current before removal
@@ -619,25 +601,14 @@ func (cli *Instance) switchAwayFromVault(
 	return nil
 }
 
-// RemoveVault removes a vault, holding the state directory lock while
-// removeVault runs
+// RemoveVault removes a vault with safety checks
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
-	err := vault.ValidateVaultName(name)
-	if err != nil {
-		return err
-	}
-
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
 	}
 	defer release()
 
-	return cli.removeVault(cmd, name, force)
-}
-
-// removeVault removes a vault with safety checks
-func (cli *Instance) removeVault(cmd *cobra.Command, name string, force bool) error {
 	// Get list of all vaults
 	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
 	if err != nil {
@@ -671,10 +642,7 @@ func (cli *Instance) removeVault(cmd *cobra.Command, name string, force bool) er
 	}
 
 	// Check if vault has secrets
-	hasSecrets, err := cli.vaultHasSecrets(vaultDir)
-	if err != nil {
-		return err
-	}
+	hasSecrets := cli.vaultHasSecrets(vaultDir)
 
 	// Require --force if vault has secrets
 	if hasSecrets && !force {

internal/cli/version_test.go

@@ -7,7 +7,6 @@
 // - TestPromoteVersionCommand: Tests `secret version promote` command
 // - TestPromoteNonExistentVersion: Tests error handling for invalid promotion
 // - TestGetSecretWithVersion: Tests `secret get --version` flag functionality
-// - TestGetSecretWritesBinaryValue: Tests `secret get` output of binary values
 // - TestVersionCommandStructure: Tests command structure and help text
 // - TestListVersionsEmptyOutput: Tests edge case with no versions
 //
@@ -24,7 +23,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-	"unicode/utf8"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
@@ -190,10 +188,7 @@ func TestPromoteVersionCommand(t *testing.T) {
 	// Current should be version-2
 	value, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-2"), value.Bytes())
+	assert.Equal(t, []byte("version-2"), value)
 
 	// Promote first version
 	firstVersion := versions[1] // Older version
@@ -216,12 +211,9 @@ func TestPromoteVersionCommand(t *testing.T) {
 	assert.Contains(t, outputStr, firstVersion)
 
 	// Verify current is now version-1
-	promoted, err := vlt.GetSecret("test/secret")
+	value, err = vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-
-	defer promoted.Destroy()
-
-	assert.Equal(t, []byte("version-1"), promoted.Bytes())
+	assert.Equal(t, []byte("version-1"), value)
 }
 
 //nolint:paralleltest // uses t.Setenv via setupTestVault
@@ -298,46 +290,6 @@ func TestGetSecretWithVersion(t *testing.T) {
 	assert.Equal(t, "version-1", buf.String())
 }
 
-//nolint:paralleltest // uses t.Setenv via setupTestVault
-func TestGetSecretWritesBinaryValue(t *testing.T) {
-	fs := afero.NewMemMapFs()
-	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
-
-	setupTestVault(t, fs)
-
-	vlt, err := vault.GetCurrentVault(fs, testStateDir)
-	require.NoError(t, err)
-
-	value := []byte{0x00, 'a', 0xff, 0xfe, 0x00, 0xc3, 0x28, 'z', 0x00}
-	require.False(t, utf8.Valid(value))
-	// A copy, since storing a value wipes the slice it came from
-	addTestSecret(t, vlt, bytes.Clone(value), false)
-
-	vaultDir, err := vlt.GetDirectory()
-	require.NoError(t, err)
-
-	versions, err := secret.ListVersions(fs,
-		filepath.Join(vaultDir, "secrets.d", "test%secret"))
-	require.NoError(t, err)
-	require.Len(t, versions, 1)
-
-	cmd := newRootCmd()
-
-	var buf bytes.Buffer
-	cmd.SetOut(&buf)
-
-	// Each writes exactly the stored bytes, with no trailing newline
-	err = cli.GetSecret(cmd, "test/secret")
-	require.NoError(t, err)
-	assert.Equal(t, value, buf.Bytes())
-
-	buf.Reset()
-
-	err = cli.GetSecretWithVersion(cmd, "test/secret", versions[0])
-	require.NoError(t, err)
-	assert.Equal(t, value, buf.Bytes())
-}
-
 //nolint:paralleltest // reads process environment to determine the state dir
 func TestVersionCommandStructure(t *testing.T) {
 	// Test that version commands are properly structured

internal/secret/atomic.go

@@ -1,7 +1,6 @@
 package secret
 
 import (
-	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -62,52 +61,6 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
-// WriteDir calls write to write the files of the directory dir. When dir does
-// not exist yet, write writes them into a temporary directory from TempDirFor,
-// which is then renamed to dir, so that neither a failure nor a crash leaves
-// dir half-written; on a failure the temporary directory is removed, and a
-// failure to remove it is returned along with the first. A directory cannot be
-// renamed over one that has files in it, so when dir already exists, write
-// writes into it in place; dir is then never removed.
-func WriteDir(fs afero.Fs, dir string, write func(dir string) error) error {
-	exists, err := afero.Exists(fs, dir)
-	if err != nil {
-		return fmt.Errorf("failed to check for %s: %w", dir, err)
-	}
-
-	if exists {
-		return write(dir)
-	}
-
-	// Create the directory the finished one is renamed into
-	err = fs.MkdirAll(filepath.Dir(dir), DirPerms)
-	if err != nil {
-		return fmt.Errorf("failed to create %s: %w", filepath.Dir(dir), err)
-	}
-
-	tmp, err := TempDirFor(fs, dir)
-	if err != nil {
-		return err
-	}
-
-	err = write(tmp)
-	if err == nil {
-		err = fs.Rename(tmp, dir)
-	}
-
-	if err != nil {
-		removeErr := fs.RemoveAll(tmp)
-		if removeErr != nil {
-			err = errors.Join(err,
-				fmt.Errorf("failed to remove %s: %w", tmp, removeErr))
-		}
-
-		return err
-	}
-
-	return nil
-}
-
 // RemoveDirAtomic deletes the directory dir so that it disappears in one
 // rename: dir is moved into a new directory from TempDirFor, which is then
 // deleted. A crash part-way leaves only that temporary directory behind.

internal/secret/atomic_test.go

@@ -35,10 +35,6 @@ const currentFile = "current"
 // unlockerMetadataFile is the file a new unlocker writes last.
 const unlockerMetadataFile = "unlocker-metadata.json"
 
-// privKeyFile is the file that holds the encrypted private key of a version
-// or of a passphrase unlocker.
-const privKeyFile = "priv.age"
-
 // unlockerPassphrase protects the passphrase unlockers the tests create.
 //
 //nolint:gosec // G101: test data, not a real credential
@@ -346,10 +342,7 @@ func TestLongestNames(t *testing.T) {
 
 	got, err := vlt.GetSecret(name)
 	require.NoError(t, err)
-
-	defer got.Destroy()
-
-	assert.Equal(t, []byte("long"), got.Bytes())
+	assert.Equal(t, "long", string(got))
 
 	vaultDir, err := vlt.GetDirectory()
 	require.NoError(t, err)
@@ -387,10 +380,7 @@ func TestForcedCopyKeepsDestinationUntilReplaced(t *testing.T) {
 
 			value, err := dest.GetSecret("shared")
 			require.NoError(t, err)
-
-			defer value.Destroy()
-
-			assert.Equal(t, []byte("old"), value.Bytes())
+			assert.Equal(t, "old", string(value))
 		})
 	}
 }
@@ -457,7 +447,7 @@ func TestVersionSaveIsWholeOrAbsent(t *testing.T) {
 
 				if exists {
 					assert.ElementsMatch(t,
-						[]string{"pub.age", "value.age", privKeyFile, "metadata.age"},
+						[]string{"pub.age", "value.age", "priv.age", "metadata.age"},
 						dirNames(t, base, versionDir),
 						"version directory visible before it was complete")
 				}
@@ -500,7 +490,7 @@ func TestVersionSaveFailureLeavesNothing(t *testing.T) {
 			writeLongTermKey(t, base, stateDir)
 
 			fs := hookFs{Fs: base, before: func(op, path string) error {
-				if op == opRename && filepath.Base(path) == privKeyFile {
+				if op == opRename && filepath.Base(path) == "priv.age" {
 					return errInjected
 				}
 
@@ -646,119 +636,37 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 	require.Error(t, err)
 }
 
-// TestPassphraseUnlockerIsWholeOrAbsent checks, before every change that
-// creating a passphrase unlocker makes, that the unlocker's directory either
-// does not exist or holds all of its files: a crash or a failure at any point
-// leaves no partial unlocker.
-//
-//nolint:paralleltest // t.Setenv forbids t.Parallel
-func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
+// TestPassphraseUnlockerWritesMetadataLast checks that the last file a new
+// passphrase unlocker writes in its directory is its metadata: an unlocker
+// directory without metadata is never used, so one interrupted earlier
+// cannot be.
+func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	files := []string{"pub.age", privKeyFile, "longterm.age", unlockerMetadataFile}
+	base := afero.NewMemMapFs()
+	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
+	require.NoError(t, err)
 
-	for _, tfs := range testFilesystems {
-		t.Run(tfs.name, func(t *testing.T) {
-			base, stateDir := tfs.open(t)
-			vlt, err := vault.CreateVault(base, stateDir, testVaultName)
-			require.NoError(t, err)
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
 
-			vaultDir, err := vlt.GetDirectory()
-			require.NoError(t, err)
+	unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
 
-			unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
+	var last string
 
-			fs := hookFs{Fs: base, before: func(string, string) error {
-				exists, err := afero.DirExists(base, unlockerDir)
-				require.NoError(t, err)
+	fs := hookFs{Fs: base, before: func(_, path string) error {
+		if filepath.Dir(path) == unlockerDir {
+			last = filepath.Base(path)
+		}
 
-				if exists {
-					assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
-						"unlocker directory visible before it was complete")
-				}
+		return nil
+	}}
 
-				return nil
-			}}
+	passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
+	defer passphrase.Destroy()
 
-			passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
-			defer passphrase.Destroy()
-
-			_, err = vault.NewVault(fs, stateDir, testVaultName).
-				CreatePassphraseUnlocker(passphrase)
-			require.NoError(t, err)
-			assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir))
-		})
-	}
-}
-
-// TestWriteDirFailureLeavesNothing makes writing a new directory fail after
-// a file has been written in it, and checks that neither the directory nor
-// its temporary directory is left behind; and, when the temporary directory
-// cannot be removed either, that both failures are reported.
-func TestWriteDirFailureLeavesNothing(t *testing.T) {
-	t.Parallel()
-
-	for _, tfs := range testFilesystems {
-		t.Run(tfs.name, func(t *testing.T) {
-			t.Parallel()
-
-			base, dir := tfs.open(t)
-			listed := filepath.Join(dir, "unlockers.d")
-			target := filepath.Join(listed, "new")
-
-			writeThenFail := func(tmp string) error {
-				require.NoError(t, secret.WriteFileAtomic(base,
-					filepath.Join(tmp, unlockerMetadataFile), []byte("{}")))
-
-				return errInjected
-			}
-
-			err := secret.WriteDir(base, target, writeThenFail)
-			require.ErrorIs(t, err, errInjected)
-
-			// Nothing in the directory that is listed, nor beside it
-			assert.Empty(t, dirNames(t, base, listed))
-			assert.Equal(t, []string{"unlockers.d"}, dirNames(t, base, dir))
-
-			fs := hookFs{Fs: base, before: func(op, _ string) error {
-				if op == opRemove {
-					return os.ErrPermission
-				}
-
-				return nil
-			}}
-
-			err = secret.WriteDir(fs, target, writeThenFail)
-			require.ErrorIs(t, err, errInjected)
-			require.ErrorIs(t, err, os.ErrPermission)
-			assert.Empty(t, dirNames(t, base, listed))
-		})
-	}
-}
-
-// TestWriteDirKeepsExistingDir makes writing into a directory that already
-// exists fail, and checks that the directory, with what was in it, is still
-// there: WriteDir writes into it in place and never removes it.
-func TestWriteDirKeepsExistingDir(t *testing.T) {
-	t.Parallel()
-
-	for _, tfs := range testFilesystems {
-		t.Run(tfs.name, func(t *testing.T) {
-			t.Parallel()
-
-			fs, dir := tfs.open(t)
-			target := filepath.Join(dir, "unlockers.d", "passphrase")
-			require.NoError(t, fs.MkdirAll(target, secret.DirPerms))
-			require.NoError(t, secret.WriteFileAtomic(fs,
-				filepath.Join(target, unlockerMetadataFile), []byte("{}")))
-
-			err := secret.WriteDir(fs, target, func(got string) error {
-				assert.Equal(t, target, got)
-
-				return errInjected
-			})
-			require.ErrorIs(t, err, errInjected)
-			assert.Equal(t, []string{unlockerMetadataFile}, dirNames(t, fs, target))
-		})
-	}
+	_, err = vault.NewVault(fs, testVaultStateDir, testVaultName).
+		CreatePassphraseUnlocker(passphrase)
+	require.NoError(t, err)
+	assert.Equal(t, unlockerMetadataFile, last)
 }

internal/secret/derivation_index_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"filippo.io/age"
 	"git.eeqj.de/sneak/secret/pkg/agehd"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
@@ -33,7 +32,6 @@ func (v *realVault) GetFilesystem() afero.Fs { return v.fs }
 // Unused by getLongTermPrivateKey — these satisfy VaultInterface.
 func (v *realVault) AddSecret(string, *memguard.LockedBuffer, bool) error { panic("not used") }
 func (v *realVault) GetCurrentUnlocker() (Unlocker, error)                { panic("not used") }
-func (v *realVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { panic("not used") }
 func (v *realVault) CreatePassphraseUnlocker(*memguard.LockedBuffer) (*PassphraseUnlocker, error) {
 	panic("not used")
 }

internal/secret/keychainunlocker.go

@@ -341,13 +341,16 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate keychain item name: %w", err)
 	}
 
-	// The unlocker directory is named after the keychain item
+	// Create unlocker directory using the keychain item name as the directory name
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", keychainItemName)
+	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
+		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
+	}
 
 	// Step 1: Generate a new age keypair for the keychain unlocker
 	ageIdentity, err := age.GenerateX25519Identity()
@@ -355,8 +358,6 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
-	ageRecipient := ageIdentity.Recipient().String()
-
 	// Step 2: Generate a random passphrase for encrypting the age private key
 	agePrivKeyPassphrase, err := generateRandomPassphrase(agePrivKeyPassphraseLength)
 	if err != nil {
@@ -364,7 +365,14 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer agePrivKeyPassphrase.Destroy()
 
-	// Step 3: Encrypt age private key with the generated passphrase
+	// Step 3: Store age recipient as plaintext
+	ageRecipient := ageIdentity.Recipient().String()
+	recipientPath := filepath.Join(unlockerDir, "pub.txt")
+	if err := WriteFileAtomic(fs, recipientPath, []byte(ageRecipient)); err != nil {
+		return nil, fmt.Errorf("failed to write age recipient: %w", err)
+	}
+
+	// Step 4: Encrypt age private key with the generated passphrase and store on disk
 	// Create a secure buffer for the private key
 	agePrivKeyStr := ageIdentity.String()
 	agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
@@ -375,20 +383,31 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
 	}
 
-	// Step 4: Get or derive the long-term private key
+	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age")
+	if err := WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey); err != nil {
+		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
+	}
+
+	// Step 5: Get or derive the long-term private key
 	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
 	if err != nil {
 		return nil, err
 	}
 	defer ltPrivKeyData.Destroy()
 
-	// Step 5: Encrypt long-term private key to the new age unlocker
+	// Step 6: Encrypt long-term private key to the new age unlocker
 	encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient())
 	if err != nil {
 		return nil, fmt.Errorf("failed to encrypt long-term private key to age unlocker: %w", err)
 	}
 
-	// Step 6: Prepare keychain data
+	// Write encrypted long-term private key
+	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
+	if err := WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge); err != nil {
+		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	// Step 7: Prepare keychain data
 	keychainData := KeychainData{
 		AgePublicKey:         ageRecipient,
 		AgePrivKeyPassphrase: agePrivKeyPassphrase,
@@ -401,7 +420,12 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer keychainDataBuffer.Destroy()
 
-	// Step 7: Prepare enhanced metadata
+	// Step 8: Store data in keychain
+	if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
+		return nil, fmt.Errorf("failed to store data in keychain: %w", err)
+	}
+
+	// Step 9: Create and write enhanced metadata
 	keychainMetadata := KeychainUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      "keychain",
@@ -416,37 +440,10 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	// Step 8: Write the unlocker's files and store the data in the keychain,
-	// the metadata last
-	err = WriteDir(fs, unlockerDir, func(dir string) error {
-		pubPath := filepath.Join(dir, "pub.txt")
-		if err := WriteFileAtomic(fs, pubPath, []byte(ageRecipient)); err != nil {
-			return fmt.Errorf("failed to write age recipient: %w", err)
-		}
-
-		privPath := filepath.Join(dir, "priv.age")
-		if err := WriteFileAtomic(fs, privPath, encryptedAgePrivKey); err != nil {
-			return fmt.Errorf("failed to write encrypted age private key: %w", err)
-		}
-
-		ltKeyPath := filepath.Join(dir, "longterm.age")
-		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtPrivKeyToAge); err != nil {
-			return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-		}
-
-		if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
-			return fmt.Errorf("failed to store data in keychain: %w", err)
-		}
-
-		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
-		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
-			return fmt.Errorf("failed to write unlocker metadata: %w", err)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return nil, err
+	if err := WriteFileAtomic(fs,
+		filepath.Join(unlockerDir, "unlocker-metadata.json"),
+		metadataBytes); err != nil {
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
 	}
 
 	return &KeychainUnlocker{

internal/secret/keychainunlocker_stub.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 
 	"filippo.io/age"
+	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 )
 
@@ -78,3 +79,10 @@ func (k *KeychainUnlocker) Remove() error {
 func CreateKeychainUnlocker(_ afero.Fs, _ string) (*KeychainUnlocker, error) {
 	return nil, errKeychainNotSupported
 }
+
+// getLongTermPrivateKey returns an error on non-Darwin platforms
+func getLongTermPrivateKey(
+	_ afero.Fs, _ VaultInterface,
+) (*memguard.LockedBuffer, error) {
+	return nil, errKeychainNotSupported
+}

internal/secret/pgpunlock_test.go

@@ -290,7 +290,7 @@ Passphrase: ` + testPassphrase + `
 		}
 
 		// Now create a PGP unlock key (this will use our custom GPGEncryptFunc)
-		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID, fingerprint)
+		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID)
 		if err != nil {
 			t.Fatalf("Failed to create PGP unlock key: %v", err)
 		}

internal/secret/pgpunlocker.go

@@ -222,13 +222,20 @@ func generatePGPUnlockerName() (string, error) {
 	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
 }
 
-// pgpUnlockerDir returns the current vault and the directory in it for a
-// new PGP unlocker, named after the host and the day.
+// preparePGPUnlockerDir checks GPG availability and creates the
+// unlocker directory in the current vault, returning the vault and the
+// directory path.
 //
 //nolint:ireturn // the vault is only available behind VaultInterface
-func pgpUnlockerDir(
+func preparePGPUnlockerDir(
 	fs afero.Fs, stateDir string,
 ) (VaultInterface, string, error) {
+	// Check if GPG is available
+	err := checkGPGAvailable()
+	if err != nil {
+		return nil, "", err
+	}
+
 	// Get current vault
 	vault, err := GetCurrentVault(fs, stateDir)
 	if err != nil {
@@ -241,29 +248,27 @@ func pgpUnlockerDir(
 		return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)
 	}
 
+	// Create unlocker directory using the generated name
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
-	return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
-}
+	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerName)
 
-// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault.
-// It encrypts to the GPG key gpgKeyID and records fingerprint, that key's
-// fingerprint as ResolveGPGKeyFingerprint returns it, in the metadata.
-// Everything that can fail short of writing a file is done before anything
-// is written, and the files are written through WriteDir, so a failure
-// leaves no partial unlocker.
-func CreatePGPUnlocker(
-	fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
-) (*PGPUnlocker, error) {
-	err := checkGPGAvailable()
+	err = fs.MkdirAll(unlockerDir, DirPerms)
 	if err != nil {
-		return nil, err
+		return nil, "", fmt.Errorf("failed to create unlocker directory: %w", err)
 	}
 
-	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
+	return vault, unlockerDir, nil
+}
+
+// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault
+func CreatePGPUnlocker(
+	fs afero.Fs, stateDir string, gpgKeyID string,
+) (*PGPUnlocker, error) {
+	vault, unlockerDir, err := preparePGPUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err
 	}
@@ -274,13 +279,77 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
-	// Step 2: Encrypt the long-term private key to the new keypair, and the
-	// keypair's private key to the GPG key
-	encryptedLtPrivKey, encryptedAgePrivKey, err := encryptPGPUnlockerKeys(
-		vault, ageIdentity, gpgKeyID)
+	// Step 2: Store age recipient as plaintext
+	ageRecipient := ageIdentity.Recipient().String()
+	recipientPath := filepath.Join(unlockerDir, "pub.txt")
+
+	err = WriteFileAtomic(fs, recipientPath, []byte(ageRecipient))
+	if err != nil {
+		return nil, fmt.Errorf("failed to write age recipient: %w", err)
+	}
+
+	// Step 3: Get or derive the long-term private key
+	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
 	if err != nil {
 		return nil, err
 	}
+	defer ltPrivKeyData.Destroy()
+
+	// Step 7: Encrypt long-term private key to the new age unlocker
+	encryptedLtPrivKeyToAge, err := EncryptToRecipient(
+		ltPrivKeyData, ageIdentity.Recipient())
+	if err != nil {
+		return nil, fmt.Errorf(
+			"failed to encrypt long-term private key to age unlocker: %w", err)
+	}
+
+	// Write encrypted long-term private key
+	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
+
+	err = WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge)
+	if err != nil {
+		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	// Step 8: Encrypt age private key to the GPG key ID
+	// Use memguard to protect the private key in memory
+	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
+	defer agePrivateKeyBuffer.Destroy()
+
+	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to encrypt age private key with GPG: %w", err)
+	}
+
+	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age.gpg")
+
+	err = WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey)
+	if err != nil {
+		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
+	}
+
+	// Steps 9-10: Resolve the fingerprint and write enhanced metadata
+	pgpMetadata, err := writePGPUnlockerMetadata(fs, unlockerDir, gpgKeyID)
+	if err != nil {
+		return nil, err
+	}
+
+	return &PGPUnlocker{
+		Directory: unlockerDir,
+		Metadata:  pgpMetadata.UnlockerMetadata,
+		fs:        fs,
+	}, nil
+}
+
+// writePGPUnlockerMetadata resolves the GPG key fingerprint and writes
+// the unlocker metadata file, returning the metadata written.
+func writePGPUnlockerMetadata(
+	fs afero.Fs, unlockerDir string, gpgKeyID string,
+) (*PGPUnlockerMetadata, error) {
+	fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
+	}
 
 	pgpMetadata := PGPUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
@@ -296,86 +365,13 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	// Step 3: Write the unlocker's files, the metadata last
-	err = WriteDir(fs, unlockerDir, func(dir string) error {
-		return writePGPUnlockerFiles(fs, dir, ageIdentity.Recipient(),
-			encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes)
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	return &PGPUnlocker{
-		Directory: unlockerDir,
-		Metadata:  pgpMetadata.UnlockerMetadata,
-		fs:        fs,
-	}, nil
-}
-
-// encryptPGPUnlockerKeys returns the vault's long-term private key encrypted
-// to the new PGP unlocker's age keypair, and that keypair's private key
-// encrypted to the GPG key gpgKeyID.
-func encryptPGPUnlockerKeys(
-	vault VaultInterface, ageIdentity *age.X25519Identity, gpgKeyID string,
-) ([]byte, []byte, error) {
-	// From the mnemonic or the current unlocker, as for a passphrase unlocker
-	ltIdentity, err := vault.GetOrDeriveLongTermKey()
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to get long-term key: %w", err)
-	}
-
-	ltPrivKeyData := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
-	defer ltPrivKeyData.Destroy()
-
-	encryptedLtPrivKey, err := EncryptToRecipient(
-		ltPrivKeyData, ageIdentity.Recipient())
-	if err != nil {
-		return nil, nil, fmt.Errorf(
-			"failed to encrypt long-term private key to age unlocker: %w", err)
-	}
-
-	// Use memguard to protect the private key in memory
-	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
-	defer agePrivateKeyBuffer.Destroy()
-
-	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
-	if err != nil {
-		return nil, nil, fmt.Errorf(
-			"failed to encrypt age private key with GPG: %w", err)
-	}
-
-	return encryptedLtPrivKey, encryptedAgePrivKey, nil
-}
-
-// writePGPUnlockerFiles writes the files of a PGP unlocker into dir, the
-// metadata last.
-func writePGPUnlockerFiles(
-	fs afero.Fs, dir string, ageRecipient *age.X25519Recipient,
-	encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes []byte,
-) error {
-	err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"),
-		[]byte(ageRecipient.String()))
-	if err != nil {
-		return fmt.Errorf("failed to write age recipient: %w", err)
-	}
-
-	err = WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKey)
-	if err != nil {
-		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	err = WriteFileAtomic(fs, filepath.Join(dir, "priv.age.gpg"), encryptedAgePrivKey)
-	if err != nil {
-		return fmt.Errorf("failed to write encrypted age private key: %w", err)
-	}
-
 	err = WriteFileAtomic(fs,
-		filepath.Join(dir, "unlocker-metadata.json"), metadataBytes)
+		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
 	if err != nil {
-		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
 	}
 
-	return nil
+	return &pgpMetadata, nil
 }
 
 // validateGPGKeyID validates that a GPG key ID is safe for command execution

internal/secret/pgpunlocker_test.go

@@ -1,65 +0,0 @@
-package secret_test
-
-import (
-	"os"
-	"path/filepath"
-	"testing"
-
-	"git.eeqj.de/sneak/secret/internal/secret"
-	"git.eeqj.de/sneak/secret/internal/vault"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
-const (
-	testGPGKeyID       = "0123456789ABCDEF"
-	testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
-)
-
-// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
-// else fails.
-const fakeGPGScript = `#!/bin/sh
-[ "$*" = --version ]
-`
-
-// installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
-func installFakeGPG(t *testing.T) {
-	t.Helper()
-
-	dir := t.TempDir()
-
-	//nolint:gosec // G306: the script must be executable
-	err := os.WriteFile(filepath.Join(dir, "gpg"), []byte(fakeGPGScript), 0o700)
-	require.NoError(t, err)
-
-	t.Setenv("PATH", dir)
-}
-
-// TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at
-// getting the vault's long-term key, which used to come after part of the
-// unlocker was written, and asserts that nothing is written. Getting the key
-// fails because there is no mnemonic and no current unlocker.
-func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
-	installFakeGPG(t)
-	t.Setenv(secret.EnvMnemonic, "")
-
-	base := afero.NewMemMapFs()
-	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
-	require.NoError(t, err)
-
-	fs := hookFs{Fs: base, before: func(_, path string) error {
-		t.Errorf("changed %s", path)
-
-		return nil
-	}}
-
-	_, err = secret.CreatePGPUnlocker(
-		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint)
-	require.Error(t, err)
-
-	vaultDir, err := vlt.GetDirectory()
-	require.NoError(t, err)
-	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
-}

internal/secret/secret.go

@@ -35,7 +35,6 @@ type VaultInterface interface {
 	GetName() string
 	GetFilesystem() afero.Fs
 	GetCurrentUnlocker() (Unlocker, error)
-	GetOrDeriveLongTermKey() (*age.X25519Identity, error)
 	CreatePassphraseUnlocker(
 		passphrase *memguard.LockedBuffer) (*PassphraseUnlocker, error)
 }

internal/secret/secret_test.go

@@ -107,10 +107,6 @@ func (m *MockVault) GetCurrentUnlocker() (Unlocker, error) {
 	return nil, errNotImplementedInMock
 }
 
-func (m *MockVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
-	return nil, errNotImplementedInMock
-}
-
 func (m *MockVault) CreatePassphraseUnlocker(
 	_ *memguard.LockedBuffer,
 ) (*PassphraseUnlocker, error) {

internal/secret/seunlocker_darwin.go

@@ -254,7 +254,7 @@ func CreateSecureEnclaveUnlocker(
 		)
 	}
 
-	// Step 4: Prepare the unlocker directory's path and metadata
+	// Step 4: Create unlocker directory and write files
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
@@ -262,7 +262,23 @@ func CreateSecureEnclaveUnlocker(
 
 	unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel))
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName)
+	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
+		return nil, fmt.Errorf(
+			"failed to create unlocker directory: %w",
+			err,
+		)
+	}
 
+	// Write SE-encrypted long-term key
+	ltKeyPath := filepath.Join(unlockerDir, seLongtermFilename)
+	if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
+		return nil, fmt.Errorf(
+			"failed to write SE-encrypted long-term key: %w",
+			err,
+		)
+	}
+
+	// Write metadata
 	seMetadata := SecureEnclaveUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      seUnlockerType,
@@ -278,25 +294,9 @@ func CreateSecureEnclaveUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// Step 5: Write the SE-encrypted long-term key, then the metadata
-	err = WriteDir(fs, unlockerDir, func(dir string) error {
-		ltKeyPath := filepath.Join(dir, seLongtermFilename)
-		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
-			return fmt.Errorf(
-				"failed to write SE-encrypted long-term key: %w",
-				err,
-			)
-		}
-
-		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
-		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
-			return fmt.Errorf("failed to write metadata: %w", err)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return nil, err
+	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
+	if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
+		return nil, fmt.Errorf("failed to write metadata: %w", err)
 	}
 
 	return &SecureEnclaveUnlocker{

internal/secret/version_test.go

@@ -87,10 +87,6 @@ func (m *MockVersionVault) GetCurrentUnlocker() (secret.Unlocker, error) {
 	return nil, errNotImplementedInMock
 }
 
-func (m *MockVersionVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
-	return nil, errNotImplementedInMock
-}
-
 func (m *MockVersionVault) CreatePassphraseUnlocker(
 	_ *memguard.LockedBuffer,
 ) (*secret.PassphraseUnlocker, error) {

internal/vault/errors.go

@@ -17,10 +17,9 @@ var (
 		"derived public key does not match vault: mnemonic may be incorrect",
 	)
 
-	// ErrInvalidVaultName indicates a vault name that breaks the naming
-	// rule: only lowercase ASCII letters, digits, '.', '-' and '_'; not
-	// empty, "." or "..". Composed by ValidateVaultName as
-	// "invalid vault name '<name>': <the rule>".
+	// ErrInvalidVaultName indicates a vault name that does not match the
+	// allowed pattern [a-z0-9.\-_]+. Composed as
+	// "invalid vault name '<name>': must match pattern [a-z0-9.\-_]+".
 	ErrInvalidVaultName = errors.New("invalid vault name")
 
 	// ErrVaultNotFound indicates the named vault does not exist. Composed

internal/vault/integration_test.go

@@ -1,7 +1,6 @@
 package vault_test
 
 import (
-	"bytes"
 	"os"
 	"path/filepath"
 	"slices"
@@ -198,11 +197,10 @@ func testDeepPathSecrets(t *testing.T, fs afero.Fs, tempDir string) {
 	if err != nil {
 		t.Fatalf("Failed to retrieve deep path secret: %v", err)
 	}
-	defer retrievedValue.Destroy()
 
-	if !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
+	if string(retrievedValue) != string(expectedValue) {
 		t.Errorf("Retrieved value doesn't match. Expected %q, got %q",
-			expectedValue, retrievedValue.Bytes())
+			string(expectedValue), string(retrievedValue))
 	}
 }
 

internal/vault/integration_version_test.go

@@ -119,10 +119,7 @@ func testCreateInitialVersion(
 	// Verify secret can be retrieved
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-1-data"), value.Bytes())
+	assert.Equal(t, []byte("version-1-data"), value)
 
 	// Verify version directory structure
 	secretDir := filepath.Join(vaultDir, "secrets.d", "integration%test")
@@ -169,10 +166,7 @@ func testCreateSecondVersion(
 	// Verify new value is current
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-2-data"), value.Bytes())
+	assert.Equal(t, []byte("version-2-data"), value)
 
 	// Verify we now have two versions
 	versions, err = secret.ListVersions(fs, secretDir)
@@ -215,10 +209,7 @@ func testCreateThirdVersion(
 	// Current should be version-3
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-3-data"), value.Bytes())
+	assert.Equal(t, []byte("version-3-data"), value)
 }
 
 func testRetrieveSpecificVersions(
@@ -234,24 +225,15 @@ func testRetrieveSpecificVersions(
 	// Get each version by its name
 	value1, err := vault.GetSecretVersion(secretName, versions[2]) // oldest
 	require.NoError(t, err)
-
-	defer value1.Destroy()
-
-	assert.Equal(t, []byte("version-1-data"), value1.Bytes())
+	assert.Equal(t, []byte("version-1-data"), value1)
 
 	value2, err := vault.GetSecretVersion(secretName, versions[1]) // middle
 	require.NoError(t, err)
-
-	defer value2.Destroy()
-
-	assert.Equal(t, []byte("version-2-data"), value2.Bytes())
+	assert.Equal(t, []byte("version-2-data"), value2)
 
 	value3, err := vault.GetSecretVersion(secretName, versions[0]) // newest
 	require.NoError(t, err)
-
-	defer value3.Destroy()
-
-	assert.Equal(t, []byte("version-3-data"), value3.Bytes())
+	assert.Equal(t, []byte("version-3-data"), value3)
 
 	// An empty version is not one of the versions; GetSecret gets the
 	// current one
@@ -277,10 +259,7 @@ func testPromoteOldVersion(
 	// Verify current now returns the old version's value
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-1-data"), value.Bytes())
+	assert.Equal(t, []byte("version-1-data"), value)
 
 	// Verify the version metadata hasn't changed
 	// (promoting shouldn't modify timestamps)
@@ -374,13 +353,8 @@ func TestVersionConcurrency(t *testing.T) {
 				value, err := vault.GetSecret(secretName)
 				if err != nil {
 					errCh <- err
-				} else {
-					if value.String() != "initial" {
-						errCh <- fmt.Errorf("%w: %s",
-							errUnexpectedValue, value.Bytes())
-					}
-
-					value.Destroy()
+				} else if string(value) != "initial" {
+					errCh <- fmt.Errorf("%w: %s", errUnexpectedValue, value)
 				}
 
 				done <- true

internal/vault/management.go

@@ -24,12 +24,10 @@ func init() {
 		})
 }
 
-// isValidVaultName reports whether name is a valid vault name: only
-// lowercase ASCII letters, digits, '.', '-' and '_', and not empty, "." or
-// "..". With no path separator allowed, a vault is always one directory
-// directly under vaults.d.
+// isValidVaultName validates vault names according to the format [a-z0-9\.\-\_]+
+// Note: We don't allow slashes in vault names unlike secret names
 func isValidVaultName(name string) bool {
-	if name == "" || name == "." || name == ".." {
+	if name == "" {
 		return false
 	}
 
@@ -38,21 +36,6 @@ func isValidVaultName(name string) bool {
 	return matched
 }
 
-// ValidateVaultName returns an error wrapping ErrInvalidVaultName when name
-// is not a valid vault name. Call it on the name exactly as the user gave it,
-// before building any path from it.
-func ValidateVaultName(name string) error {
-	if !isValidVaultName(name) {
-		return fmt.Errorf(
-			"%w '%s': only lowercase ASCII letters, digits, '.', '-' and '_' "+
-				"are allowed, and a name must not be empty, '.' or '..'",
-			ErrInvalidVaultName, name,
-		)
-	}
-
-	return nil
-}
-
 // ResolveVaultSymlink reads the currentvault file to get the path to the current vault
 // The file contains just the vault name (e.g., "default")
 func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
@@ -216,11 +199,14 @@ func processMnemonicForVault(
 func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
-	err := ValidateVaultName(name)
-	if err != nil {
+	// Validate vault name
+	if !isValidVaultName(name) {
 		secret.Debug("Invalid vault name provided", "vault_name", name)
 
-		return nil, err
+		return nil, fmt.Errorf(
+			"%w '%s': must match pattern [a-z0-9.\\-_]+",
+			ErrInvalidVaultName, name,
+		)
 	}
 
 	secret.Debug("Vault name validation passed", "vault_name", name)
@@ -299,11 +285,14 @@ func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
 func SelectVault(fs afero.Fs, stateDir string, name string) error {
 	secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir)
 
-	err := ValidateVaultName(name)
-	if err != nil {
+	// Validate vault name
+	if !isValidVaultName(name) {
 		secret.Debug("Invalid vault name provided", "vault_name", name)
 
-		return err
+		return fmt.Errorf(
+			"%w '%s': must match pattern [a-z0-9.\\-_]+",
+			ErrInvalidVaultName, name,
+		)
 	}
 
 	secret.Debug("Vault name validation passed", "vault_name", name)

internal/vault/secrets.go

@@ -301,9 +301,8 @@ func updateVersionMetadata(
 	return nil
 }
 
-// GetSecret retrieves the current version of a secret from this vault.
-// The caller must destroy the returned buffer.
-func (v *Vault) GetSecret(name string) (*memguard.LockedBuffer, error) {
+// 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),
@@ -327,10 +326,7 @@ func (v *Vault) GetSecret(name string) (*memguard.LockedBuffer, error) {
 
 // GetSecretVersion retrieves a specific version of a secret. The version
 // must be one of the secret's versions; GetSecret gets the current one.
-// The caller must destroy the returned buffer.
-func (v *Vault) GetSecretVersion(
-	name string, version string,
-) (*memguard.LockedBuffer, error) {
+func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -376,14 +372,26 @@ func (v *Vault) GetSecretVersion(
 		return nil, fmt.Errorf("failed to decrypt version: %w", err)
 	}
 
+	// Create a copy to return since the buffer will be destroyed
+	result := make([]byte, decryptedValue.Size())
+	copy(result, decryptedValue.Bytes())
+	decryptedValue.Destroy()
+
 	secret.DebugWith("Successfully decrypted secret version",
 		slog.String("secret_name", name),
 		slog.String("version", version),
 		slog.String("vault_name", v.Name),
-		slog.Int("decrypted_length", decryptedValue.Size()),
+		slog.Int("decrypted_length", len(result)),
 	)
 
-	return decryptedValue, nil
+	// Debug: Log metadata about the decrypted value without exposing the actual secret
+	secret.Debug("Vault secret decryption debug info",
+		"secret_name", name,
+		"version", version,
+		"decrypted_value_length", len(result),
+		"is_empty", len(result) == 0)
+
+	return result, nil
 }
 
 // UnlockVault unlocks the vault and returns the long-term private key

internal/vault/secrets_version_test.go

@@ -131,10 +131,7 @@ func TestVaultAddSecretCreatesVersion(t *testing.T) {
 	// Get the secret value
 	retrievedValue, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, expectedValue, retrievedValue.Bytes())
+	assert.Equal(t, expectedValue, retrievedValue)
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -168,10 +165,7 @@ func TestVaultAddSecretMultipleVersions(t *testing.T) {
 	// Current value should be version-2
 	value, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-2"), value.Bytes())
+	assert.Equal(t, []byte("version-2"), value)
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -198,21 +192,15 @@ func TestVaultGetSecretVersion(t *testing.T) {
 
 	// Get specific version (first one)
 	firstVersion := versions[1] // Last in list is first created
-	first, err := vault.GetSecretVersion(testSecretPath, firstVersion)
+	value, err := vault.GetSecretVersion(testSecretPath, firstVersion)
 	require.NoError(t, err)
-
-	defer first.Destroy()
-
-	assert.Equal(t, []byte("version-1"), first.Bytes())
+	assert.Equal(t, []byte("version-1"), value)
 
 	// Get specific version (second one)
 	secondVersion := versions[0] // First in list is most recent
-	second, err := vault.GetSecretVersion(testSecretPath, secondVersion)
+	value, err = vault.GetSecretVersion(testSecretPath, secondVersion)
 	require.NoError(t, err)
-
-	defer second.Destroy()
-
-	assert.Equal(t, []byte("version-2"), second.Bytes())
+	assert.Equal(t, []byte("version-2"), value)
 
 	// An empty version is not one of the versions; GetSecret gets the
 	// current one

internal/vault/unlockers.go

@@ -126,11 +126,7 @@ func (v *Vault) resolveUnlockerDirectory(currentUnlockerPath string) (string, er
 }
 
 // findUnlockerByID finds an unlocker by its ID and returns the unlocker
-// instance and its directory path. A directory that ListUnlockers skips is
-// skipped here too, with the same warning. Such a directory has no ID: if
-// no unlocker has the ID unlockerID but such a directory is named
-// unlockerID, that directory is returned with a nil unlocker, so that
-// RemoveUnlocker can remove it.
+// instance and its directory path
 //
 //nolint:ireturn // returns one of several concrete unlocker implementations
 func (v *Vault) findUnlockerByID(
@@ -141,24 +137,42 @@ func (v *Vault) findUnlockerByID(
 		return nil, "", fmt.Errorf("failed to read unlockers directory: %w", err)
 	}
 
-	skippedDirPath := ""
-
 	for _, file := range files {
 		if !file.IsDir() {
 			continue
 		}
 
-		unlockerDirPath := filepath.Join(unlockersDir, file.Name())
+		// Read metadata file
+		metadataPath := filepath.Join(unlockersDir, file.Name(), "unlocker-metadata.json")
 
-		metadata, ok := v.readUnlockerMetadataOrWarn(unlockersDir, file.Name())
-		if !ok {
-			if file.Name() == unlockerID {
-				skippedDirPath = unlockerDirPath
-			}
+		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)
+		}
 
+		if !exists {
+			// Skip directories without metadata - they might not be unlockers
 			continue
 		}
 
+		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)
+		}
+
+		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)
+		}
+
+		unlockerDirPath := filepath.Join(unlockersDir, file.Name())
+
 		// Create the appropriate unlocker instance
 		var tempUnlocker secret.Unlocker
 
@@ -181,7 +195,7 @@ func (v *Vault) findUnlockerByID(
 		}
 	}
 
-	return nil, skippedDirPath, nil
+	return nil, "", nil
 }
 
 // ListUnlockers returns a list of available unlockers for this vault
@@ -212,12 +226,44 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
 	var unlockers []UnlockerMetadata
 
 	for _, file := range files {
-		if !file.IsDir() {
-			continue
-		}
+		if file.IsDir() {
+			// Read metadata file
+			metadataPath := filepath.Join(unlockersDir, file.Name(),
+				"unlocker-metadata.json")
+
+			exists, err := afero.Exists(v.fs, metadataPath)
+			if err != nil {
+				secret.Warn("Skipping unlocker directory whose metadata file cannot be checked",
+					"directory", file.Name(), "error", err)
+
+				continue
+			}
+
+			if !exists {
+				secret.Warn("Skipping unlocker directory with missing metadata file",
+					"directory", file.Name())
+
+				continue
+			}
+
+			metadataBytes, err := afero.ReadFile(v.fs, metadataPath)
+			if err != nil {
+				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 {
+				secret.Warn("Skipping unlocker directory with corrupt metadata file",
+					"directory", file.Name(), "error", err)
+
+				continue
+			}
 
-		metadata, ok := v.readUnlockerMetadataOrWarn(unlockersDir, file.Name())
-		if ok {
 			unlockers = append(unlockers, metadata)
 		}
 	}
@@ -225,54 +271,7 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
 	return unlockers, nil
 }
 
-// readUnlockerMetadataOrWarn reads the metadata of the unlocker directory
-// name in unlockersDir. If the metadata file cannot be checked for, is
-// missing, or cannot be read or parsed, it warns, naming the directory,
-// and returns false: the caller skips that directory.
-func (v *Vault) readUnlockerMetadataOrWarn(
-	unlockersDir, name string,
-) (UnlockerMetadata, bool) {
-	metadataPath := filepath.Join(unlockersDir, name, "unlocker-metadata.json")
-
-	var metadata UnlockerMetadata
-
-	exists, err := afero.Exists(v.fs, metadataPath)
-	if err != nil {
-		secret.Warn("Skipping unlocker directory whose metadata file cannot be checked",
-			"directory", name, "error", err)
-
-		return metadata, false
-	}
-
-	if !exists {
-		secret.Warn("Skipping unlocker directory with missing metadata file",
-			"directory", name)
-
-		return metadata, false
-	}
-
-	metadataBytes, err := afero.ReadFile(v.fs, metadataPath)
-	if err != nil {
-		secret.Warn("Skipping unlocker directory with unreadable metadata file",
-			"directory", name, "error", err)
-
-		return metadata, false
-	}
-
-	err = json.Unmarshal(metadataBytes, &metadata)
-	if err != nil {
-		secret.Warn("Skipping unlocker directory with corrupt metadata file",
-			"directory", name, "error", err)
-
-		return metadata, false
-	}
-
-	return metadata, true
-}
-
-// RemoveUnlocker removes an unlocker from this vault. An unlocker
-// directory that ListUnlockers skips is removed by its directory name; its
-// type is unknown, so only the directory is removed.
+// RemoveUnlocker removes an unlocker from this vault
 func (v *Vault) RemoveUnlocker(unlockerID string) error {
 	vaultDir, err := v.GetDirectory()
 	if err != nil {
@@ -283,17 +282,13 @@ func (v *Vault) RemoveUnlocker(unlockerID string) error {
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
 	// Find the unlocker by ID
-	unlocker, unlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
+	unlocker, _, err := v.findUnlockerByID(unlockersDir, unlockerID)
 	if err != nil {
 		return err
 	}
 
-	if unlockerDir == "" {
-		return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
-	}
-
 	if unlocker == nil {
-		return secret.RemoveDirAtomic(v.fs, unlockerDir)
+		return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
 	}
 
 	// Use the unlocker's Remove method
@@ -311,14 +306,12 @@ func (v *Vault) SelectUnlocker(unlockerID string) error {
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
 	// Find the unlocker by ID
-	unlocker, targetUnlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
+	_, targetUnlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
 	if err != nil {
 		return err
 	}
 
-	// A directory found without an unlocker is one ListUnlockers skips; it
-	// cannot be selected.
-	if unlocker == nil {
+	if targetUnlockerDir == "" {
 		return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
 	}
 
@@ -357,14 +350,26 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
+	// Create unlocker directory
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase)
 
+	err = v.fs.MkdirAll(unlockerDir, secret.DirPerms)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
+	}
+
 	// Generate new age keypair for unlocker
 	unlockerIdentity, err := age.GenerateX25519Identity()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate unlocker: %w", err)
 	}
 
+	// Write the unlocker keypair (public and passphrase-encrypted private)
+	err = v.writeUnlockerKeypair(unlockerDir, unlockerIdentity, passphrase)
+	if err != nil {
+		return nil, err
+	}
+
 	// Encrypt long-term private key to this unlocker
 	ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
 	defer ltPrivKeyBuffer.Destroy()
@@ -375,6 +380,15 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to encrypt long-term private key: %w", err)
 	}
 
+	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
+
+	err = secret.WriteFileAtomic(v.fs, ltPrivKeyPath, encryptedLtPrivKey)
+	if err != nil {
+		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	// Write the metadata last: readers skip an unlocker directory without
+	// it, so an unlocker interrupted before this point is never used.
 	metadata := UnlockerMetadata{
 		Type:      unlockerTypePassphrase,
 		CreatedAt: time.Now(),
@@ -386,13 +400,11 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// Write the unlocker's files, the metadata last
-	err = secret.WriteDir(v.fs, unlockerDir, func(dir string) error {
-		return v.writeUnlockerFiles(dir, unlockerIdentity, passphrase,
-			encryptedLtPrivKey, metadataBytes)
-	})
+	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
+
+	err = secret.WriteFileAtomic(v.fs, metadataPath, metadataBytes)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
 	}
 
 	// Create the unlocker instance
@@ -438,14 +450,12 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro
 	return metadata, nil
 }
 
-// writeUnlockerFiles writes the files of a passphrase unlocker into
-// unlockerDir: its public key, its passphrase-encrypted private key, the
-// long-term private key encrypted to it, and its metadata, last.
-func (v *Vault) writeUnlockerFiles(
+// writeUnlockerKeypair writes the unlocker's public key and its
+// passphrase-encrypted private key into the unlocker directory.
+func (v *Vault) writeUnlockerKeypair(
 	unlockerDir string,
 	unlockerIdentity *age.X25519Identity,
 	passphrase *memguard.LockedBuffer,
-	encryptedLtPrivKey, metadataBytes []byte,
 ) error {
 	// Write public key
 	pubKeyPath := filepath.Join(unlockerDir, "pub.age")
@@ -475,17 +485,5 @@ func (v *Vault) writeUnlockerFiles(
 		return fmt.Errorf("failed to write encrypted unlocker private key: %w", err)
 	}
 
-	err = secret.WriteFileAtomic(v.fs,
-		filepath.Join(unlockerDir, "longterm.age"), encryptedLtPrivKey)
-	if err != nil {
-		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	err = secret.WriteFileAtomic(v.fs,
-		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
-	if err != nil {
-		return fmt.Errorf("failed to write unlocker metadata: %w", err)
-	}
-
 	return nil
 }

internal/vault/vault.go

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

internal/vault/vault_test.go

@@ -1,7 +1,6 @@
 package vault_test
 
 import (
-	"bytes"
 	"path/filepath"
 	"slices"
 	"testing"
@@ -185,11 +184,10 @@ func testSecretOperations(t *testing.T, fs afero.Fs) {
 	if err != nil {
 		t.Fatalf("Failed to get secret: %v", err)
 	}
-	defer retrievedValue.Destroy()
 
-	if !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
+	if string(retrievedValue) != string(expectedValue) {
 		t.Errorf("Expected secret value '%s', got '%s'",
-			expectedValue, retrievedValue.Bytes())
+			string(expectedValue), string(retrievedValue))
 	}
 }
 

script/bootstrap

@@ -6,7 +6,6 @@
 # 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
@@ -137,6 +136,12 @@ 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/build

@@ -1,29 +0,0 @@
-#!/bin/sh
-# script/build: build the `secret` binary into the repo root, with its
-# version and git commit stamped in (`secret info` shows both).
-set -eu
-
-ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
-
-main() {
-    cd "$ROOT"
-    # CGO is required (Makefile exports this too)
-    export CGO_ENABLED=1
-    # A VERSION set in the environment wins (`make build VERSION=x`, as
-    # the Dockerfile does); otherwise `git describe` of this checkout.
-    version="${VERSION:-}"
-    if [ -z "$version" ]; then
-        version="$(git describe --tags --always --dirty 2>/dev/null ||
-            echo dev)"
-    fi
-    commit="$(git rev-parse HEAD 2>/dev/null || echo unknown)"
-    pkg=git.eeqj.de/sneak/secret/internal/cli
-    # Build the file, not the package `./cmd/secret`: a package build
-    # also stamps git status into the binary and fails where git cannot
-    # read the checkout, instead of falling back to `dev`/`unknown`.
-    go build -v \
-        -ldflags "-X '$pkg.Version=$version' -X '$pkg.GitCommit=$commit'" \
-        -o secret cmd/secret/main.go
-}
-
-main "$@"

script/cibuild

@@ -4,17 +4,13 @@
 # The Gitea workflow runs this on push. The memlock ulimit lets the tests
 # that lock large secrets in memory (memguard mlocks them) run; under the
 # lower limit of a plain `docker build .` they are skipped.
-# A cached build checks nothing: a new CHECK_EPOCH on every run makes the
-# Dockerfile's check steps run again on an unchanged tree, while its base
-# images and module downloads stay cached.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build --ulimit memlock=-1:-1 \
-        --build-arg CHECK_EPOCH="$(date +%s)" .
+    docker build --ulimit memlock=-1:-1 .
 }
 
 main "$@"

script/lint

@@ -1,24 +1,14 @@
 #!/bin/sh
-# 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.
+# script/lint: run the linter.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build \
-        --progress=plain \
-        --target lint \
-        --no-cache-filter=lint \
-        --output=type=cacheonly \
-        -f Dockerfile.lint .
+    # CGO is required (Makefile exports this too)
+    export CGO_ENABLED=1
+    golangci-lint run --timeout 5m
 }
 
 main "$@"