Build with the local docker daemon; add script/build (closes #44)

The Makefile exported DOCKER_HOST pointing at one private machine, so
every docker call made through make, `make lint` and `make check`
included, failed everywhere else. The line is gone: docker uses the
local daemon, or a DOCKER_HOST set in the environment.

`make build` now calls the new `script/build`, which stamps the version
and commit as the Makefile did. A VERSION set in the environment now
wins over `git describe`, not only one given as `make build VERSION=x`.
build, clean, install and docker-run are phony; install depends on
build. The vet target is removed: `script/test` runs `go vet` first.

Model: opus-5-5

Makefile

@@ -1,13 +1,7 @@
 export CGO_ENABLED=1
-export DOCKER_HOST := ssh://root@ber1app1.local
 
-# 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
+.PHONY: default bootstrap setup build test lint fmt fmt-check check docker \
+	docker-run clean install hooks
 
 default: check
 
@@ -17,13 +11,9 @@ bootstrap:
 setup:
 	@script/setup
 
-build: ./secret
-
-./secret: ./internal/*/*.go ./pkg/*/*.go ./cmd/*/*.go ./go.*
-	go build -v -ldflags "$(LDFLAGS)" -o $@ cmd/secret/main.go
-
-vet:
-	go vet ./...
+# Build ./secret; `make build VERSION=x` stamps x instead of `git describe`
+build:
+	@script/build
 
 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,13 +25,13 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
-- 2026-10-04: A failed command prints its error once, without the usage
-  text after it (https://git.eeqj.de/sneak/secret/issues/41). Usage is
-  still printed for a command called wrongly: wrong number of arguments,
-  unknown flag, bad flag value or missing required flag. The root
-  command's `PersistentPreRunE` turns usage off once cobra has checked
-  the arguments and flags; root `SilenceUsage` would have hidden usage
-  for those too.
+- 2026-10-04: 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`
@@ -111,6 +111,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
@@ -210,6 +217,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/root.go

@@ -46,24 +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 flags before this runs, except for
-		// required flags, which are checked here so they still get usage.
-		// An error after that comes from running the command, and usage
-		// would only bury it. A subcommand that sets its own
-		// PersistentPreRun replaces this one.
-		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
-			err := cmd.ValidateRequiredFlags()
-			if err != nil {
-				return err
-			}
-
-			cmd.SilenceUsage = true
-
-			return nil
-		},
 	}
 
 	secret.Debug("Adding subcommands to root command")

internal/cli/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
-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
+// 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) {
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		secret.Warn("Could not get vault directory during duplicate check",
-			"error", err)
-
-		return nil
+		return false, fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
-	// Check each unlocker's ID
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
-	for _, metadata := range unlockers {
-		// Construct the unlocker matching this metadata to get its ID
-		id, err := findUnlockerIDByMetadata(cli.fs, unlockersDir, metadata, true)
-		if err != nil {
-			secret.Warn(
-				"Could not read unlockers directory during duplicate check, "+
-					"skipping unlocker",
-				"unlockers_dir", unlockersDir, "error", err)
+	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
 		}
 
-		if id != "" && id == unlockerID {
-			return errUnlockerExists
+		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
+		}
+
+		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

@@ -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,8 +401,12 @@ func (cli *Instance) vaultImportPreflight(
 	// Check if vault already has a public key
 	pubKeyPath := vaultDir + "/pub.age"
 
-	_, err = cli.fs.Stat(pubKeyPath)
-	if err == nil {
+	exists, err = afero.Exists(cli.fs, pubKeyPath)
+	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/vault/vault.go

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

script/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 "$@"