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.

Model: opus-5-5

internal/cli/unlockers.go

@@ -721,8 +721,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	return nil
 }
 
-// UnlockersRemove removes an unlocker, holding the state directory lock
+// UnlockersRemove removes an unlocker with safety checks
-// while removeUnlocker runs
 func (cli *Instance) UnlockersRemove(
 	unlockerID string, force bool, cmd *cobra.Command,
 ) error {
@@ -732,13 +731,6 @@ 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,11 +6,6 @@
 // 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
@@ -247,7 +242,7 @@ func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
 			writeTestSecret(t, base, vaultDir)
 			instance, cmd := newTestInstance(&statFailFs{Fs: base, path: path})
 
-			err := instance.removeUnlocker(
+			err := instance.UnlockersRemove(
 				"pgp-"+listTestGPGKeyID+"A", false, cmd)
 
 			require.ErrorIs(t, err, errStatFailed)
@@ -294,7 +289,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)
 
@@ -316,7 +311,7 @@ func TestVaultImportAbortsWhenPubKeyUnreadable(t *testing.T) {
 		Fs: base, path: filepath.Join(testVaultDir(listTestVaultName), "pub.age"),
 	})
 
-	err := instance.importMnemonic(cmd, listTestVaultName)
+	err := instance.VaultImport(cmd, listTestVaultName)
 
 	require.ErrorIs(t, err, errStatFailed)
 }

internal/cli/vault.go

@@ -613,8 +613,7 @@ func (cli *Instance) switchAwayFromVault(
 	return nil
 }
 
-// RemoveVault removes a vault, holding the state directory lock while
+// RemoveVault removes a vault with safety checks
-// 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 {
@@ -622,11 +621,6 @@ 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 {