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

A failed command printed the whole usage text after its error, burying
it. The root command's PersistentPreRunE now turns usage off, so an
error from running the command is printed once on its own. Wrong arity,
an unknown flag, a bad flag value, a missing required flag and broken
flag groups still get usage: 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 was not used: in
this cobra version it hides usage for argument and flag errors too.
Cobra still prints the error; Entry is unchanged.

Model: opus-5-5

Makefile

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

README.md

@@ -506,6 +506,9 @@ them. We provide:
   `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

TODO.md

@@ -25,6 +25,29 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-04: A failed command prints its error once, without the usage
+  text after it (https://git.eeqj.de/sneak/secret/issues/41). Usage is
+  still printed for a command called wrongly: wrong number of arguments,
+  unknown flag, bad flag value, 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`
@@ -104,6 +127,13 @@ Bring the repo into policy compliance in one commit:
       being removed, encrypted keys included. Nothing deletes it; it
       must be deleted by hand
       (https://git.eeqj.de/sneak/secret/issues/75).
+- 2026-10-03: 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
@@ -203,8 +233,6 @@ 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,8 +68,3 @@ 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/create_vault_test.go

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

internal/cli/lock_test.go

@@ -147,7 +147,8 @@ func TestConcurrentAddsKeepEveryVersion(t *testing.T) {
 				value, err := vlt.GetSecretVersion("shared", version)
 				require.NoError(t, err)
 
-				values[string(value)] = true
+				values[string(value.Bytes())] = true
+				value.Destroy()
 			}
 
 			assert.Len(t, values, adds+1, "every add stored its own value")

internal/cli/move_test.go

@@ -178,7 +178,10 @@ func TestMoveOntoSameSecretUnderAnotherNameIsRejected(t *testing.T) {
 
 			value, err := vlt.GetSecret("x")
 			require.NoError(t, err)
-			require.Equal(t, "value", string(value))
+
+			defer value.Destroy()
+
+			require.Equal(t, []byte("value"), value.Bytes())
 
 			target, err := os.Readlink(link)
 			require.NoError(t, err)
@@ -222,7 +225,10 @@ func TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem(t *testing.T) {
 
 	value, err := vlt.GetSecret("foo")
 	require.NoError(t, err)
-	require.Equal(t, "upper", string(value))
+
+	defer value.Destroy()
+
+	require.Equal(t, []byte("upper"), value.Bytes())
 
 	_, err = vlt.GetSecret("Foo")
 	require.ErrorIs(t, err, vault.ErrSecretNotFound)

internal/cli/root.go

@@ -46,9 +46,30 @@ func newRootCmd() *cobra.Command {
 		Short: "A simple secrets manager",
 		Long: `A simple secrets manager to store and retrieve sensitive ` +
 			`information securely.`,
-		// Ensure usage is shown after errors
+		// Cobra prints the error a command returns; Entry does not.
-		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

@@ -414,9 +414,6 @@ 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 {
@@ -427,9 +424,10 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	if err != nil {
 		return err
 	}
+	defer value.Destroy()
 
-	// Print the secret value to stdout
+	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cli.Print(string(value))
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	return nil
 }
@@ -442,9 +440,6 @@ 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 {
@@ -460,22 +455,15 @@ func (cli *Instance) GetSecretWithVersion(
 
 		return err
 	}
+	defer value.Destroy()
 
-	secret.Debug("Got secret value", "valueLength", len(value))
+	secret.Debug("Got secret value", "valueLength", value.Size())
 
-	// Print the secret value to stdout
+	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cli.Print(string(value))
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	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
 }
 

internal/cli/secrets_size_test.go

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

internal/cli/unlockers.go

@@ -49,7 +49,6 @@ var (
 		"is already added as an unlocker")
 	errUnsupportedUnlockerType = errors.New("unsupported unlocker type")
 	errLastUnlocker            = errors.New("refusing to remove last unlocker")
-	errUnlockerExists          = errors.New("unlocker already exists")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -695,8 +694,15 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	// Check if this GPG key is already added
 	expectedID := "pgp-" + fingerprint
 
-	err = cli.checkUnlockerExists(vlt, expectedID)
+	exists, err := cli.checkUnlockerExists(vlt, expectedID)
 	if err != nil {
+		return fmt.Errorf(
+			"could not check whether GPG key %s is already an unlocker: %w",
+			gpgKeyID, err,
+		)
+	}
+
+	if exists {
 		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
 	}
 
@@ -714,7 +720,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	return nil
 }
 
-// UnlockersRemove removes an unlocker with safety checks
+// UnlockersRemove removes an unlocker, holding the state directory lock
+// while removeUnlocker runs
 func (cli *Instance) UnlockersRemove(
 	unlockerID string, force bool, cmd *cobra.Command,
 ) error {
@@ -724,6 +731,13 @@ 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 {
@@ -788,44 +802,65 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
-// checkUnlockerExists checks if an unlocker with the given ID exists
+// checkUnlockerExists reports whether the vault already has an unlocker
-func (cli *Instance) checkUnlockerExists(vlt *vault.Vault, unlockerID string) error {
+// with the given ID. It returns an error, and no answer, when unlockers.d
-	// Get the list of unlockers and check if any match the ID
+// or an unlocker's metadata file cannot be read; the caller must then not
-	unlockers, err := vlt.ListUnlockers()
+// create the unlocker. It reads unlockers.d itself because
-	if err != nil {
+// vault.ListUnlockers skips an unlocker it cannot read, which suits
-		secret.Warn("Could not list unlockers during duplicate check", "error", err)
+// `unlocker list` but not this check: the skipped unlocker may be the
-
+// duplicate. A directory whose metadata file is missing or corrupt is not
-		return nil // If we can't list unlockers, assume it doesn't exist
+// a working unlocker and is passed over.
-	}
+func (cli *Instance) checkUnlockerExists(
-
+	vlt *vault.Vault, unlockerID string,
-	// Get vault directory to construct unlocker instances
+) (bool, error) {
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		secret.Warn("Could not get vault directory during duplicate check",
+		return false, fmt.Errorf("failed to get vault directory: %w", err)
-			"error", err)
-
-		return nil
 	}
 
-	// Check each unlocker's ID
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
-	for _, metadata := range unlockers {
+	entries, err := afero.ReadDir(cli.fs, unlockersDir)
-		// Construct the unlocker matching this metadata to get its ID
+	if errors.Is(err, os.ErrNotExist) {
-		id, err := findUnlockerIDByMetadata(cli.fs, unlockersDir, metadata, true)
+		return false, nil
-		if err != nil {
+	}
-			secret.Warn(
-				"Could not read unlockers directory during duplicate check, "+
-					"skipping unlocker",
-				"unlockers_dir", unlockersDir, "error", err)
 
+	if err != nil {
+		return false, fmt.Errorf(
+			"failed to read unlockers directory %s: %w", unlockersDir, err,
+		)
+	}
+
+	for _, entry := range entries {
+		if !entry.IsDir() {
 			continue
 		}
 
-		if id != "" && id == unlockerID {
+		unlockerDir := filepath.Join(unlockersDir, entry.Name())
-			return errUnlockerExists
+
+		metadataBytes, err := afero.ReadFile(
+			cli.fs, filepath.Join(unlockerDir, "unlocker-metadata.json"))
+		if errors.Is(err, os.ErrNotExist) {
+			continue
+		}
+
+		if err != nil {
+			return false, fmt.Errorf(
+				"failed to read metadata of unlocker %s: %w", unlockerDir, 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
 		}
 	}
 
-	return nil
+	return false, nil
 }

internal/cli/unreadable_dir_test.go

@@ -0,0 +1,353 @@
+// 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, 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)
+
+	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

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

internal/cli/vault.go

@@ -401,8 +401,12 @@ func (cli *Instance) vaultImportPreflight(
 	// Check if vault already has a public key
 	pubKeyPath := vaultDir + "/pub.age"
 
-	_, err = cli.fs.Stat(pubKeyPath)
+	exists, err = afero.Exists(cli.fs, pubKeyPath)
-	if err == nil {
+	if err != nil {
+		return "", "", "", fmt.Errorf("failed to check %s: %w", pubKeyPath, err)
+	}
+
+	if exists {
 		return "", "", "", fmt.Errorf("vault '%s' %w",
 			vaultName, errVaultHasLongTermKey)
 	}
@@ -562,17 +566,26 @@ 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 {
+func (cli *Instance) vaultHasSecrets(vaultDir string) (bool, error) {
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
-	exists, _ := afero.DirExists(cli.fs, secretsDir)
+	exists, err := afero.DirExists(cli.fs, secretsDir)
+	if err != nil {
+		return false, fmt.Errorf("failed to check secrets directory %s: %w",
+			secretsDir, err)
+	}
+
 	if !exists {
-		return false
+		return false, nil
 	}
 
 	entries, err := afero.ReadDir(cli.fs, secretsDir)
+	if err != nil {
+		return false, fmt.Errorf("failed to read secrets directory %s: %w",
+			secretsDir, err)
+	}
 
-	return err == nil && len(entries) > 0
+	return len(entries) > 0, nil
 }
 
 // switchAwayFromVault selects another vault as current before removal
@@ -601,7 +614,8 @@ func (cli *Instance) switchAwayFromVault(
 	return nil
 }
 
-// RemoveVault removes a vault with safety checks
+// RemoveVault removes a vault, holding the state directory lock while
+// removeVault runs
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
@@ -609,6 +623,11 @@ func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) er
 	}
 	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 {
@@ -642,7 +661,10 @@ func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) er
 	}
 
 	// Check if vault has secrets
-	hasSecrets := cli.vaultHasSecrets(vaultDir)
+	hasSecrets, err := cli.vaultHasSecrets(vaultDir)
+	if err != nil {
+		return err
+	}
 
 	// Require --force if vault has secrets
 	if hasSecrets && !force {

internal/cli/version_test.go

@@ -7,6 +7,7 @@
 // - 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
 //
@@ -23,6 +24,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"unicode/utf8"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
@@ -188,7 +190,10 @@ func TestPromoteVersionCommand(t *testing.T) {
 	// Current should be version-2
 	value, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-2"), value.Bytes())
 
 	// Promote first version
 	firstVersion := versions[1] // Older version
@@ -211,9 +216,12 @@ func TestPromoteVersionCommand(t *testing.T) {
 	assert.Contains(t, outputStr, firstVersion)
 
 	// Verify current is now version-1
-	value, err = vlt.GetSecret("test/secret")
+	promoted, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1"), value)
+
+	defer promoted.Destroy()
+
+	assert.Equal(t, []byte("version-1"), promoted.Bytes())
 }
 
 //nolint:paralleltest // uses t.Setenv via setupTestVault
@@ -290,6 +298,46 @@ 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_test.go

@@ -342,7 +342,10 @@ func TestLongestNames(t *testing.T) {
 
 	got, err := vlt.GetSecret(name)
 	require.NoError(t, err)
-	assert.Equal(t, "long", string(got))
+
+	defer got.Destroy()
+
+	assert.Equal(t, []byte("long"), got.Bytes())
 
 	vaultDir, err := vlt.GetDirectory()
 	require.NoError(t, err)
@@ -380,7 +383,10 @@ func TestForcedCopyKeepsDestinationUntilReplaced(t *testing.T) {
 
 			value, err := dest.GetSecret("shared")
 			require.NoError(t, err)
-			assert.Equal(t, "old", string(value))
+
+			defer value.Destroy()
+
+			assert.Equal(t, []byte("old"), value.Bytes())
 		})
 	}
 }

internal/vault/integration_test.go

@@ -1,6 +1,7 @@
 package vault_test
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
 	"slices"
@@ -197,10 +198,11 @@ 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 string(retrievedValue) != string(expectedValue) {
+	if !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
 		t.Errorf("Retrieved value doesn't match. Expected %q, got %q",
-			string(expectedValue), string(retrievedValue))
+			expectedValue, retrievedValue.Bytes())
 	}
 }
 

internal/vault/integration_version_test.go

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

internal/vault/secrets.go

@@ -301,8 +301,9 @@ func updateVersionMetadata(
 	return nil
 }
 
-// GetSecret retrieves the current version of a secret from this vault
+// GetSecret retrieves the current version of a secret from this vault.
-func (v *Vault) GetSecret(name string) ([]byte, error) {
+// The caller must destroy the returned buffer.
+func (v *Vault) GetSecret(name string) (*memguard.LockedBuffer, error) {
 	secret.DebugWith("Getting secret from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -326,7 +327,10 @@ func (v *Vault) GetSecret(name string) ([]byte, error) {
 
 // GetSecretVersion retrieves a specific version of a secret. The version
 // must be one of the secret's versions; GetSecret gets the current one.
-func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
+// The caller must destroy the returned buffer.
+func (v *Vault) GetSecretVersion(
+	name string, version string,
+) (*memguard.LockedBuffer, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -372,26 +376,14 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 		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", len(result)),
+		slog.Int("decrypted_length", decryptedValue.Size()),
 	)
 
-	// Debug: Log metadata about the decrypted value without exposing the actual secret
+	return decryptedValue, nil
-	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,7 +131,10 @@ func TestVaultAddSecretCreatesVersion(t *testing.T) {
 	// Get the secret value
 	retrievedValue, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-	assert.Equal(t, expectedValue, retrievedValue)
+
+	defer retrievedValue.Destroy()
+
+	assert.Equal(t, expectedValue, retrievedValue.Bytes())
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -165,7 +168,10 @@ func TestVaultAddSecretMultipleVersions(t *testing.T) {
 	// Current value should be version-2
 	value, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-2"), value.Bytes())
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -192,15 +198,21 @@ func TestVaultGetSecretVersion(t *testing.T) {
 
 	// Get specific version (first one)
 	firstVersion := versions[1] // Last in list is first created
-	value, err := vault.GetSecretVersion(testSecretPath, firstVersion)
+	first, err := vault.GetSecretVersion(testSecretPath, firstVersion)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1"), value)
+
+	defer first.Destroy()
+
+	assert.Equal(t, []byte("version-1"), first.Bytes())
 
 	// Get specific version (second one)
 	secondVersion := versions[0] // First in list is most recent
-	value, err = vault.GetSecretVersion(testSecretPath, secondVersion)
+	second, err := vault.GetSecretVersion(testSecretPath, secondVersion)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer second.Destroy()
+
+	assert.Equal(t, []byte("version-2"), second.Bytes())
 
 	// An empty version is not one of the versions; GetSecret gets the
 	// current one

internal/vault/vault.go

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

internal/vault/vault_test.go

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

script/build

@@ -0,0 +1,29 @@
+#!/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 "$@"