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

internal/cli/unlockers.go

@@ -721,7 +721,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 {
@@ -731,6 +732,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 {

internal/cli/unreadable_dir_test.go

@@ -6,6 +6,11 @@
 // 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
@@ -242,7 +247,7 @@ func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
 			writeTestSecret(t, base, vaultDir)
 			instance, cmd := newTestInstance(&statFailFs{Fs: base, path: path})
 
-			err := instance.UnlockersRemove(
+			err := instance.removeUnlocker(
 				"pgp-"+listTestGPGKeyID+"A", false, cmd)
 
 			require.ErrorIs(t, err, errStatFailed)
@@ -289,7 +294,7 @@ func TestRemoveVaultAbortsWhenSecretsDirUnreadable(t *testing.T) {
 			writeTestSecret(t, base, vaultDir)
 			instance, cmd := newTestInstance(tt.failFs(base))
 
-			err := instance.RemoveVault(cmd, unreadableTestOtherVault, false)
+			err := instance.removeVault(cmd, unreadableTestOtherVault, false)
 
 			require.ErrorIs(t, err, tt.wantErr)
 
@@ -311,7 +316,7 @@ func TestVaultImportAbortsWhenPubKeyUnreadable(t *testing.T) {
 		Fs: base, path: filepath.Join(testVaultDir(listTestVaultName), "pub.age"),
 	})
 
-	err := instance.VaultImport(cmd, listTestVaultName)
+	err := instance.importMnemonic(cmd, listTestVaultName)
 
 	require.ErrorIs(t, err, errStatFailed)
 }

internal/cli/vault.go

@@ -613,7 +613,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 {
@@ -621,6 +622,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 {