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

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

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

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

`vault rm` and `unlocker rm` now take the state directory lock and call
an unexported function that does the work, as `vault import` does, so
the tests can reach their checks.

Model: opus-5-5

TODO.md

@@ -25,13 +25,6 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
-- 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

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"
 
@@ -320,28 +323,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
+// nor change anything while the lock is held, and must succeed once it is
-// the lock is released.
+// released.
 func requireWaitsForLock(
 	t *testing.T,
 	withUnlocker bool,
@@ -370,20 +355,14 @@ func requireWaitsForLock(
 
 	go func() { done <- run(cli, olderVersion, unlockerID) }()
 
-	timeout := time.After(lockWait)
+	select {
-
+	case err := <-done:
-	for !waitingForLock() {
+		t.Fatalf("finished while the lock was held, with error %v", err)
-		select {
+	case <-time.After(heldWait):
-		case err := <-done:
-			t.Fatalf("finished while the lock was held, with error %v", err)
-		case <-timeout:
-			t.Fatal("never waited for the lock")
-		case <-time.After(time.Millisecond):
-		}
 	}
 
 	assert.Equal(t, before, stateDirModTimes(t, fs),
-		"changed the state directory before waiting for the lock")
+		"changed the state directory while the lock was held")
 
 	release()
 

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"} {
+	for _, name := range []string{"work", "default"} {
-			vlt, err := vault.CreateVault(fs, testStateDir, name)
+		vlt, err := vault.CreateVault(fs, testStateDir, name)
-			require.NoError(t, err)
+		require.NoError(t, err)
 
-			err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
+		err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
-			require.NoError(t, err)
+		require.NoError(t, err)
 
-			_, err = vlt.CreatePassphraseUnlocker(
+		_, err = vlt.CreatePassphraseUnlocker(
-				memguard.NewBufferFromBytes([]byte(testPassphrase)))
+			memguard.NewBufferFromBytes([]byte(testPassphrase)))
-			require.NoError(t, err)
+		require.NoError(t, err)
-		}
+	}
 
-		twoVaults = snapshotStateDir(t, fs)
+	return fs
-	})
-
-	require.NotNil(t, twoVaults, "making the vaults failed in an earlier test")
-
-	return newFsFromSnapshot(t, twoVaults)
 }
 
 // snapshotStateDir maps every file under the state directory to its