Ask before removing a secret, version, vault or unlocker (closes #39)

secret rm, secret version rm, secret vault remove and secret unlocker
remove ask [y/N] on a terminal, naming what they remove, and go ahead
only on y or yes. Without --force, a command whose stdin is not a
terminal fails at once. --force, now also on rm and version rm,
removes without asking; it replaces the old refusals to remove a vault
with secrets or the last unlocker without --force. The checks run and
the question is asked before the state directory lock is taken; under
the lock the checks run again, and nothing is removed if they would
ask a different question.

Model: opus-5-5

internal/cli/unreadable_dir_test.go

@@ -2,15 +2,18 @@
 //
 // 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.
+// vault hold secrets?), removing a secret (how many versions does it
+// have?), 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.
+// function each command runs once it holds the lock, such as addPGPUnlocker
+// for UnlockersAdd, or, for a removal, the function that makes its checks,
+// such as findVaultToRemove for RemoveVault, which runs again under the
+// lock before anything is removed, with --force or without.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli
@@ -285,10 +288,9 @@ func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
 
 			base := newListTestVault(t, 1)
 			writeTestSecret(t, base, vaultDir)
-			instance, cmd := newTestInstance(&statFailFs{Fs: base, path: path})
+			instance, _ := newTestInstance(&statFailFs{Fs: base, path: path})
 
-			err := instance.removeUnlocker(
-				"pgp-"+listTestGPGKeyID+"A", false, cmd)
+			_, err := instance.findUnlockerToRemove("pgp-" + listTestGPGKeyID + "A")
 
 			require.ErrorIs(t, err, errStatFailed)
 			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
@@ -332,9 +334,9 @@ func TestRemoveVaultAbortsWhenSecretsDirUnreadable(t *testing.T) {
 
 			base := newListTestVault(t, 1)
 			writeTestSecret(t, base, vaultDir)
-			instance, cmd := newTestInstance(tt.failFs(base))
+			instance, _ := newTestInstance(tt.failFs(base))
 
-			err := instance.removeVault(cmd, unreadableTestOtherVault, false)
+			_, err := instance.findVaultToRemove(unreadableTestOtherVault)
 
 			require.ErrorIs(t, err, tt.wantErr)
 
@@ -345,6 +347,31 @@ func TestRemoveVaultAbortsWhenSecretsDirUnreadable(t *testing.T) {
 	}
 }
 
+// TestRemoveSecretAbortsWhenVersionsUnreadable asserts that a secret is
+// kept when its versions directory exists but cannot be listed, so that
+// the question cannot say how many versions would be removed.
+func TestRemoveSecretAbortsWhenVersionsUnreadable(t *testing.T) {
+	t.Parallel()
+
+	secretDir := filepath.Join(testVaultDir(listTestVaultName),
+		unreadableTestSecretsDirName, unreadableTestSecretName)
+	versionsDir := filepath.Join(secretDir, "versions")
+
+	base := newListTestVault(t, 1)
+	writeTestSecret(t, base, testVaultDir(listTestVaultName))
+	require.NoError(t, base.MkdirAll(versionsDir, listTestDirPerm))
+
+	instance, _ := newTestInstance(&openFailFs{Fs: base, path: versionsDir})
+
+	_, err := instance.findSecretToRemove(unreadableTestSecretName)
+
+	require.ErrorIs(t, err, errOpenFailed)
+
+	exists, err := afero.DirExists(base, secretDir)
+	require.NoError(t, err)
+	assert.True(t, exists, "the secret must not be removed")
+}
+
 // TestVaultImportAbortsWhenPubKeyUnreadable asserts that a mnemonic import
 // stops when whether the vault already has a long-term key cannot be
 // determined.