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/cli.go

@@ -3,6 +3,7 @@ package cli
 
 import (
 	"fmt"
+	"io"
 	"os"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
@@ -21,6 +22,10 @@ type Instance struct {
 	// none.
 	Mnemonic         *memguard.LockedBuffer
 	UnlockPassphrase *memguard.LockedBuffer
+	// terminal, when set, stands in for the terminal that confirm reads
+	// the user's answer from; only tests set it. When it is nil, confirm
+	// reads stdin, and only when stdin is a terminal.
+	terminal io.Reader
 }
 
 // NewCLIInstance creates a new CLI instance with the real filesystem

internal/cli/confirm.go

@@ -0,0 +1,108 @@
+package cli
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/spf13/cobra"
+	"golang.org/x/term"
+)
+
+// Sentinel errors for asking the user to confirm a removal
+var (
+	errNoTerminal = errors.New("stdin is not a terminal, so there is " +
+		"nobody to ask for confirmation; pass --force to remove without asking")
+	errNotConfirmed       = errors.New("cancelled; nothing was removed")
+	errChangedWhileAsking = errors.New("what was to be removed changed " +
+		"while waiting for the answer; nothing was removed")
+)
+
+// askThenLock asks the user to confirm a removal, unless force is set, and
+// then takes the state directory lock and returns the function that
+// releases it. find makes the command's checks, keeps what it found for
+// the caller to remove, and returns the question that names it. find runs
+// before the question, which is asked without the lock so that no other
+// command waits while the user answers, and runs again once the lock is
+// taken. That run is the last, so the caller removes what find found under
+// the lock. If its question then differs from the one the user answered,
+// something changed in between, and askThenLock fails.
+func (cli *Instance) askThenLock(
+	cmd *cobra.Command, force bool, find func() (string, error),
+) (func(), error) {
+	asked := ""
+
+	if !force {
+		question, err := find()
+		if err != nil {
+			return nil, err
+		}
+
+		err = cli.confirm(cmd, question)
+		if err != nil {
+			return nil, err
+		}
+
+		asked = question
+	}
+
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return nil, err
+	}
+
+	question, err := find()
+	if err == nil && !force && question != asked {
+		err = errChangedWhileAsking
+	}
+
+	if err != nil {
+		release()
+
+		return nil, err
+	}
+
+	return release, nil
+}
+
+// confirm asks question and returns nil only when the user answers y or
+// yes; any other answer, a bare Enter included, cancels. When stdin is not
+// a terminal it asks nothing and fails at once: nobody is there to answer,
+// and waiting for an answer would hang a script. Stdin decides, not
+// stdout, because the answer is read from stdin: `secret rm foo | tee log`
+// still asks. The question goes to stderr.
+func (cli *Instance) confirm(cmd *cobra.Command, question string) error {
+	answers := cli.terminal
+	if answers == nil {
+		answers = cmd.InOrStdin()
+
+		if !isTerminal(answers) {
+			return errNoTerminal
+		}
+	}
+
+	_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "%s [y/N] ", question)
+
+	answer, err := bufio.NewReader(answers).ReadString('\n')
+	if err != nil && !errors.Is(err, io.EOF) {
+		return fmt.Errorf("failed to read the answer: %w", err)
+	}
+
+	switch strings.ToLower(strings.TrimSpace(answer)) {
+	case "y", "yes":
+		return nil
+	default:
+		return errNotConfirmed
+	}
+}
+
+// isTerminal reports whether r is a terminal.
+func isTerminal(r io.Reader) bool {
+	file, ok := r.(*os.File)
+
+	return ok && term.IsTerminal(int(file.Fd()))
+}

internal/cli/confirm_test.go

@@ -0,0 +1,410 @@
+// Confirmation Tests
+//
+// `secret rm`, `secret version rm`, `secret vault remove` and
+// `secret unlocker remove` ask the user to confirm on a terminal, naming
+// what they are about to remove, and remove it only on y or yes. --force
+// skips the question. Without --force, a command whose stdin is not a
+// terminal fails at once, since nobody is there to answer.
+//
+// The tests answer through Instance.terminal, which stands in for a
+// terminal; without it, the command's input decides, and a test's input is
+// never a terminal.
+
+//nolint:testpackage // sets the unexported terminal field of Instance
+package cli
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/spf13/afero"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// confirmTestSecret is the secret the tests remove, or remove a
+	// version of, in the vault "work".
+	confirmTestSecret = "test/secret"
+
+	// lastUnlockerRemoval names the case that removes the only unlocker.
+	lastUnlockerRemoval = "unlocker rm, the last one"
+)
+
+// removal is one removal command, set up on its own state directory.
+type removal struct {
+	fs  afero.Fs
+	run func(cli *Instance, cmd *cobra.Command, force bool) error
+	// removed is the directory the command removes.
+	removed string
+	// question is the question the command asks.
+	question string
+}
+
+// newConfirmTestVaults returns an in-memory state directory with the
+// vaults "other" and "work", the current one. "work" holds two versions of
+// confirmTestSecret and the given number of PGP unlockers. It returns the
+// directory of "work" and the older version.
+func newConfirmTestVaults(
+	t *testing.T, unlockers int,
+) (*afero.MemMapFs, string, string) {
+	t.Helper()
+
+	fs := &afero.MemMapFs{}
+	mnemonic := testMnemonicBuffer(t)
+
+	_, err := vault.CreateVault(fs, testStateDir, "other", mnemonic)
+	require.NoError(t, err)
+
+	vlt, err := vault.CreateVault(fs, testStateDir, "work", mnemonic)
+	require.NoError(t, err)
+
+	addTestSecret(t, vlt, []byte("older"), false)
+	addTestSecret(t, vlt, []byte("newer"), true)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+
+	versions, err := secret.ListVersions(fs,
+		filepath.Join(vaultDir, "secrets.d", "test%secret"))
+	require.NoError(t, err)
+	require.Len(t, versions, 2)
+
+	for i := range unlockers {
+		writePGPUnlocker(t, fs, filepath.Join(vaultDir, "unlockers.d"),
+			fmt.Sprintf("pgp-%d", i),
+			time.Date(2026, time.October, 4, 12, i, 0, 0, time.UTC),
+			listTestGPGKeyID+string(rune('A'+i)))
+	}
+
+	// ListVersions lists the newest version first.
+	return fs, vaultDir, versions[1]
+}
+
+// newRemoval sets up the removal the command names.
+func newRemoval(t *testing.T, command string) removal {
+	t.Helper()
+
+	unlockers := 2
+	if command == lastUnlockerRemoval {
+		unlockers = 1
+	}
+
+	fs, workDir, older := newConfirmTestVaults(t, unlockers)
+	unlockerID := "pgp-" + listTestGPGKeyID + "A"
+
+	removeFirstUnlocker := func(cli *Instance, cmd *cobra.Command, force bool) error {
+		return cli.UnlockersRemove(unlockerID, force, cmd)
+	}
+
+	switch command {
+	case "rm":
+		return removal{
+			fs: fs,
+			run: func(cli *Instance, cmd *cobra.Command, force bool) error {
+				return cli.RemoveSecret(cmd, confirmTestSecret, force)
+			},
+			removed: filepath.Join(workDir, "secrets.d", "test%secret"),
+			question: "Permanently remove secret 'test/secret' and its 2 " +
+				"version(s) from vault 'work'?",
+		}
+	case "version rm":
+		return removal{
+			fs: fs,
+			run: func(cli *Instance, cmd *cobra.Command, force bool) error {
+				return cli.RemoveVersion(cmd, confirmTestSecret, older, force)
+			},
+			removed: filepath.Join(
+				workDir, "secrets.d", "test%secret", "versions", older),
+			question: "Permanently remove version " + older +
+				" of secret 'test/secret' from vault 'work'?",
+		}
+	case "vault rm":
+		return removal{
+			fs: fs,
+			run: func(cli *Instance, cmd *cobra.Command, force bool) error {
+				return cli.RemoveVault(cmd, "work", force)
+			},
+			removed:  workDir,
+			question: "Permanently remove vault 'work' and its 1 secret(s)?",
+		}
+	case "unlocker rm":
+		return removal{
+			fs:      fs,
+			run:     removeFirstUnlocker,
+			removed: filepath.Join(workDir, "unlockers.d", "pgp-0"),
+			question: "Permanently remove unlocker '" + unlockerID +
+				"' from vault 'work'? It is not the vault's last unlocker.",
+		}
+	case lastUnlockerRemoval:
+		return removal{
+			fs:      fs,
+			run:     removeFirstUnlocker,
+			removed: filepath.Join(workDir, "unlockers.d", "pgp-0"),
+			question: "Permanently remove unlocker '" + unlockerID +
+				"', the last unlocker of vault 'work', which holds 1 " +
+				"secret(s)? Without an unlocker the vault opens only " +
+				"with its mnemonic.",
+		}
+	}
+
+	t.Fatalf("no removal %q", command)
+
+	return removal{}
+}
+
+// removalCommands lists the commands newRemoval sets up.
+func removalCommands() []string {
+	return []string{
+		"rm", "version rm", "vault rm", "unlocker rm", lastUnlockerRemoval,
+	}
+}
+
+// newConfirmTestCommand returns a command whose output is discarded and
+// whose stderr, where the question goes, is the returned buffer.
+func newConfirmTestCommand() (*cobra.Command, *bytes.Buffer) {
+	var stderr bytes.Buffer
+
+	cmd := &cobra.Command{}
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(&stderr)
+
+	return cmd, &stderr
+}
+
+// requireExists asserts whether the directory dir exists.
+func requireExists(t *testing.T, fs afero.Fs, dir string, want bool) {
+	t.Helper()
+
+	exists, err := afero.DirExists(fs, dir)
+	require.NoError(t, err)
+	require.Equal(t, want, exists, dir)
+}
+
+// TestConfirmAnswers checks which answers confirm accepts: y or yes, in
+// any case, around which spaces do not matter.
+func TestConfirmAnswers(t *testing.T) {
+	t.Parallel()
+
+	for answer, want := range map[string]error{
+		"y\n":       nil,
+		"Y\n":       nil,
+		"yes\n":     nil,
+		"  YES  \n": nil,
+		"y":         nil,
+		"\n":        errNotConfirmed,
+		"":          errNotConfirmed,
+		"n\n":       errNotConfirmed,
+		"yy\n":      errNotConfirmed,
+		"no\ny\n":   errNotConfirmed,
+	} {
+		t.Run(fmt.Sprintf("%q", answer), func(t *testing.T) {
+			t.Parallel()
+
+			cli := &Instance{terminal: strings.NewReader(answer)}
+			cmd, stderr := newConfirmTestCommand()
+
+			err := cli.confirm(cmd, "Remove it?")
+
+			require.ErrorIs(t, err, want)
+			assert.Equal(t, "Remove it? [y/N] ", stderr.String())
+		})
+	}
+}
+
+// TestRemovalAnsweredYesRemoves checks that each removal asks its question
+// and removes what it names when the user answers y.
+func TestRemovalAnsweredYesRemoves(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range removalCommands() {
+		t.Run(command, func(t *testing.T) {
+			t.Parallel()
+
+			r := newRemoval(t, command)
+			requireExists(t, r.fs, r.removed, true)
+
+			cli := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+			cli.terminal = strings.NewReader("y\n")
+			cmd, stderr := newConfirmTestCommand()
+
+			require.NoError(t, r.run(cli, cmd, false))
+
+			assert.Equal(t, r.question+" [y/N] ", stderr.String())
+			requireExists(t, r.fs, r.removed, false)
+		})
+	}
+}
+
+// TestRemovalDeclinedLeavesEverything checks that each removal changes
+// nothing when the user answers anything but y or yes, a bare Enter
+// included.
+func TestRemovalDeclinedLeavesEverything(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range removalCommands() {
+		for _, answer := range []string{"\n", "n\n", ""} {
+			t.Run(fmt.Sprintf("%s %q", command, answer), func(t *testing.T) {
+				t.Parallel()
+
+				r := newRemoval(t, command)
+				before := stateDirModTimes(t, r.fs)
+
+				cli := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+				cli.terminal = strings.NewReader(answer)
+				cmd, stderr := newConfirmTestCommand()
+
+				err := r.run(cli, cmd, false)
+
+				require.ErrorIs(t, err, errNotConfirmed)
+				assert.Equal(t, r.question+" [y/N] ", stderr.String())
+				assert.Equal(t, before, stateDirModTimes(t, r.fs))
+			})
+		}
+	}
+}
+
+// TestRemovalForcedAsksNothing checks that each removal with --force
+// removes what it would have named without asking, and without reading
+// its input, which is not a terminal.
+func TestRemovalForcedAsksNothing(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range removalCommands() {
+		t.Run(command, func(t *testing.T) {
+			t.Parallel()
+
+			r := newRemoval(t, command)
+
+			input := strings.NewReader("n\n")
+			cli := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+			cmd, stderr := newConfirmTestCommand()
+			cmd.SetIn(input)
+
+			require.NoError(t, r.run(cli, cmd, true))
+
+			assert.Empty(t, stderr.String(), "asked with --force")
+			assert.Equal(t, 2, input.Len(), "read its input with --force")
+			requireExists(t, r.fs, r.removed, false)
+		})
+	}
+}
+
+// TestRemovalWithoutTerminalFailsAtOnce checks that each removal without
+// --force, whose input is not a terminal, fails at once telling the user
+// to pass --force, and changes nothing. The input is a pipe that nobody
+// writes to or closes, so reading it would block for good.
+func TestRemovalWithoutTerminalFailsAtOnce(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range removalCommands() {
+		t.Run(command, func(t *testing.T) {
+			t.Parallel()
+
+			r := newRemoval(t, command)
+			before := stateDirModTimes(t, r.fs)
+
+			input, inputWriter, err := os.Pipe()
+			require.NoError(t, err)
+
+			t.Cleanup(func() {
+				_ = inputWriter.Close()
+				_ = input.Close()
+			})
+
+			cli := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+			cmd, stderr := newConfirmTestCommand()
+			cmd.SetIn(input)
+
+			done := make(chan error, 1)
+
+			go func() { done <- r.run(cli, cmd, false) }()
+
+			select {
+			case err := <-done:
+				require.ErrorIs(t, err, errNoTerminal)
+				assert.Contains(t, err.Error(), "pass --force")
+			case <-time.After(lockWait):
+				// Closing the pipe ends the read, and frees the lock if
+				// the command holds it.
+				_ = inputWriter.Close()
+
+				t.Fatal("waited for an answer on input that is not a terminal")
+			}
+
+			assert.Empty(t, stderr.String(), "asked without a terminal")
+			assert.Equal(t, before, stateDirModTimes(t, r.fs))
+		})
+	}
+}
+
+// TestRemovalAsksWithoutHoldingLock checks that while `secret rm` waits
+// for its answer, another command can take the state directory lock and
+// change the secret, and that the removal then removes nothing, since the
+// secret is no longer what the question named.
+func TestRemovalAsksWithoutHoldingLock(t *testing.T) {
+	t.Parallel()
+
+	r := newRemoval(t, "rm")
+
+	answers, answerWriter := io.Pipe()
+	questions, questionWriter := io.Pipe()
+
+	// Closing the answers ends the read if the test fails while the
+	// command waits for one.
+	t.Cleanup(func() { _ = answerWriter.Close() })
+
+	rm := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+	rm.terminal = answers
+	cmd := &cobra.Command{}
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(questionWriter)
+
+	done := make(chan error, 1)
+
+	go func() { done <- r.run(rm, cmd, false) }()
+
+	question, err := bufio.NewReader(questions).ReadString(']')
+	require.NoError(t, err)
+	require.Equal(t, r.question+" [y/N]", question)
+
+	// Adds a third version while rm waits for its answer.
+	add := NewCLIInstanceWithStateDir(r.fs, testStateDir)
+	add.Mnemonic = testMnemonicBuffer(t)
+	add.cmd = &cobra.Command{}
+	add.cmd.SetIn(strings.NewReader("newest"))
+	add.cmd.SetOut(io.Discard)
+
+	added := make(chan error, 1)
+
+	go func() { added <- add.AddSecret(confirmTestSecret, true) }()
+
+	select {
+	case err := <-added:
+		require.NoError(t, err)
+	case <-time.After(lockWait):
+		t.Fatal("secret add waited for the lock while secret rm asked")
+	}
+
+	_, err = answerWriter.Write([]byte("y\n"))
+	require.NoError(t, err)
+
+	select {
+	case err := <-done:
+		require.ErrorIs(t, err, errChangedWhileAsking)
+	case <-time.After(lockWait):
+		t.Fatal("secret rm did not finish once answered")
+	}
+
+	requireExists(t, r.fs, r.removed, true)
+}

internal/cli/integration_test.go

@@ -16,7 +16,10 @@ import (
 
 	"git.eeqj.de/sneak/secret/internal/cli"
 	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
 	"git.eeqj.de/sneak/secret/pkg/agehd"
+	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -2542,3 +2545,53 @@ func copyFile(src, dst string) error {
 
 	return nil
 }
+
+// TestRemoveWithoutTerminalFailsAtOnce runs `secret rm` without --force,
+// with a stdin that is not a terminal and never delivers anything, as in a
+// script or a CI job. It must fail at once, telling the user to pass
+// --force, instead of waiting for an answer, and remove nothing.
+func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+
+	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
+	defer mnemonic.Destroy()
+
+	vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default", mnemonic)
+	require.NoError(t, err)
+
+	value := memguard.NewBufferFromBytes([]byte("value"))
+	defer value.Destroy()
+
+	require.NoError(t, vlt.AddSecret("x", value, false))
+
+	// Nobody writes to or closes the pipe, so reading it would block for good.
+	stdin, stdinWriter, err := os.Pipe()
+	require.NoError(t, err)
+
+	defer func() {
+		_ = stdinWriter.Close()
+		_ = stdin.Close()
+	}()
+
+	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
+	defer cancel()
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), "rm", "x")
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
+	cmd.Stdin = stdin
+
+	output, err := cmd.CombinedOutput()
+
+	require.NoError(t, ctx.Err(), "secret rm waited for an answer")
+	require.Error(t, err)
+	assert.Contains(t, string(output), "pass --force")
+	assert.DirExists(t,
+		filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "x"))
+}

internal/cli/lock_test.go

@@ -241,8 +241,10 @@ func TestFailedCommandReleasesLock(t *testing.T) {
 	fs := afero.NewMemMapFs()
 	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
 
-	// Fails once it holds the lock: there is no current vault
-	err := cli.RemoveSecret(&cobra.Command{}, "missing", false)
+	// Fails once it holds the lock: there is no current vault. Without
+	// --force it would fail before taking the lock, on the check it makes
+	// before asking.
+	err := cli.RemoveSecret(&cobra.Command{}, "missing", true)
 	require.Error(t, err)
 
 	select {
@@ -431,8 +433,8 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
 		{"encrypt", false, func(cli *Instance, _, _ string) error {
 			return cli.Encrypt("key", testInput, "")
 		}},
-		{"rm", false, func(cli *Instance, _, _ string) error {
-			return cli.RemoveSecret(cli.cmd, "test/secret", false)
+		{"rm --force", false, func(cli *Instance, _, _ string) error {
+			return cli.RemoveSecret(cli.cmd, "test/secret", true)
 		}},
 		{"move", false, func(cli *Instance, _, _ string) error {
 			return cli.MoveSecret(cli.cmd, "test/secret", "moved", false)
@@ -440,8 +442,8 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
 		{"version promote", false, func(cli *Instance, olderVersion, _ string) error {
 			return cli.PromoteVersion(cli.cmd, "test/secret", olderVersion)
 		}},
-		{"version rm", false, func(cli *Instance, olderVersion, _ string) error {
-			return cli.RemoveVersion(cli.cmd, "test/secret", olderVersion)
+		{"version rm --force", false, func(cli *Instance, olderVersion, _ string) error {
+			return cli.RemoveVersion(cli.cmd, "test/secret", olderVersion, true)
 		}},
 		{"vault create", false, func(cli *Instance, _, _ string) error {
 			return cli.CreateVault(cli.cmd, "created")
@@ -452,13 +454,13 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
 		{"vault import", false, func(cli *Instance, _, _ string) error {
 			return cli.VaultImport(cli.cmd, "other")
 		}},
-		{"vault rm", false, func(cli *Instance, _, _ string) error {
-			return cli.RemoveVault(cli.cmd, "other", false)
+		{"vault rm --force", false, func(cli *Instance, _, _ string) error {
+			return cli.RemoveVault(cli.cmd, "other", true)
 		}},
 		{"unlocker add", false, func(cli *Instance, _, _ string) error {
 			return cli.UnlockersAdd("passphrase", cli.cmd)
 		}},
-		{"unlocker rm", true, func(cli *Instance, _, unlockerID string) error {
+		{"unlocker rm --force", true, func(cli *Instance, _, unlockerID string) error {
 			return cli.UnlockersRemove(unlockerID, true, cli.cmd)
 		}},
 		{"unlocker select", true, func(cli *Instance, _, unlockerID string) error {

internal/cli/path_traversal_test.go

@@ -170,8 +170,8 @@ func requireRejectedAndUnchanged(
 // TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/33, where `secret rm ..` deleted
 // the whole vault, and `secret rm .` or `secret rm ""` every secret in it.
-// Moves and imports use --force, so that only the name check stands in
-// the way.
+// Removals, moves and imports use --force, so that only the name check
+// stands in the way.
 //
 //nolint:paralleltest // the cases share cmd
 func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
@@ -191,17 +191,17 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
 		rejected string // the secret name the command must reject
 		run      func(c *cli.Instance) error
 	}{
-		{"rm ..", "..", func(c *cli.Instance) error {
-			return c.RemoveSecret(cmd, "..", false)
+		{"rm --force ..", "..", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "..", true)
 		}},
-		{"rm .", ".", func(c *cli.Instance) error {
-			return c.RemoveSecret(cmd, ".", false)
+		{"rm --force .", ".", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, ".", true)
 		}},
-		{`rm ""`, "", func(c *cli.Instance) error {
-			return c.RemoveSecret(cmd, "", false)
+		{`rm --force ""`, "", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "", true)
 		}},
-		{"rm ../../etc", "../../etc", func(c *cli.Instance) error {
-			return c.RemoveSecret(cmd, "../../etc", false)
+		{"rm --force ../../etc", "../../etc", func(c *cli.Instance) error {
+			return c.RemoveSecret(cmd, "../../etc", true)
 		}},
 		{"mv --force .. x", "..", func(c *cli.Instance) error {
 			return c.MoveSecret(cmd, "..", "x", true)
@@ -244,8 +244,8 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
 		{"version promote ..", "..", func(c *cli.Instance) error {
 			return c.PromoteVersion(cmd, "..", testVersion)
 		}},
-		{"version rm ..", "..", func(c *cli.Instance) error {
-			return c.RemoveVersion(cmd, "..", testVersion)
+		{"version rm --force ..", "..", func(c *cli.Instance) error {
+			return c.RemoveVersion(cmd, "..", testVersion, true)
 		}},
 		{"encrypt ..", "..", func(c *cli.Instance) error {
 			return c.Encrypt("..", "", "")
@@ -279,8 +279,8 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
 		command string
 		run     func(c *cli.Instance, version string) error
 	}{
-		{"version rm x", func(c *cli.Instance, version string) error {
-			return c.RemoveVersion(cmd, "x", version)
+		{"version rm --force x", func(c *cli.Instance, version string) error {
+			return c.RemoveVersion(cmd, "x", version, true)
 		}},
 		{"version promote x", func(c *cli.Instance, version string) error {
 			return c.PromoteVersion(cmd, "x", version)
@@ -361,9 +361,9 @@ func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
 	}
 }
 
-// TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
-// with a version that is not the current one removes that version and
-// changes nothing else.
+// TestRemoveVersionRemovesOnlyThatVersion checks that
+// `secret version rm --force` with a version that is not the current one
+// removes that version and changes nothing else.
 func TestRemoveVersionRemovesOnlyThatVersion(t *testing.T) {
 	t.Parallel()
 
@@ -389,7 +389,7 @@ func TestRemoveVersionRemovesOnlyThatVersion(t *testing.T) {
 	require.Contains(t, before, oldDir)
 
 	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
-	err = c.RemoveVersion(&cobra.Command{}, "x", versions[1])
+	err = c.RemoveVersion(&cobra.Command{}, "x", versions[1], true)
 	require.NoError(t, err)
 
 	// Expected: the state as before without everything under oldDir.

internal/cli/secrets.go

@@ -205,19 +205,25 @@ func newRemoveCmd() *cobra.Command {
 		Aliases: []string{"rm"},
 		Short:   "Remove a secret from the vault",
 		Long: `Remove a secret and all its versions from the current ` +
-			`vault. This action is permanent and cannot be undone.`,
+			`vault. This action is permanent and cannot be undone. ` +
+			`Asks for confirmation first; when stdin is not a terminal, ` +
+			`fails unless --force is given.`,
 		Args:              cobra.ExactArgs(1),
 		ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			force, _ := cmd.Flags().GetBool("force")
+
 			cli, err := NewCLIInstance()
 			if err != nil {
 				return fmt.Errorf("failed to initialize CLI: %w", err)
 			}
 
-			return cli.RemoveSecret(cmd, args[0], false)
+			return cli.RemoveSecret(cmd, args[0], force)
 		},
 	}
 
+	cmd.Flags().BoolP("force", "f", false, "Remove without asking for confirmation")
+
 	return cmd
 }
 
@@ -699,29 +705,64 @@ func (cli *Instance) ImportSecret(
 	return nil
 }
 
-// RemoveSecret removes a secret from the vault
-func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
+// RemoveSecret removes a secret and all its versions from the current
+// vault, after asking the user to confirm unless force is set.
+func (cli *Instance) RemoveSecret(
+	cmd *cobra.Command, secretName string, force bool,
+) error {
 	err := vault.ValidateSecretName(secretName)
 	if err != nil {
 		return err
 	}
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	var found secretToRemove
+
+	release, err := cli.askThenLock(cmd, force, func() (string, error) {
+		var err error
+
+		found, err = cli.findSecretToRemove(secretName)
+
+		return found.question, err
+	})
 	if err != nil {
 		return err
 	}
 	defer release()
 
-	// Get current vault
-	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	err = secret.RemoveDirAtomic(cli.fs, found.dir)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to remove secret: %w", err)
+	}
+
+	cmd.Printf("Removed secret '%s' (%d version(s) deleted)\n",
+		secretName, found.versions)
+
+	return nil
+}
+
+// secretToRemove is what removing a secret removes, as findSecretToRemove
+// found it.
+type secretToRemove struct {
+	// dir is the secret's directory, which holds all its versions.
+	dir      string
+	versions int
+	// question names what is removed, for the user to confirm.
+	question string
+}
+
+// findSecretToRemove checks that the secret exists in the current vault
+// and counts its versions.
+func (cli *Instance) findSecretToRemove(
+	secretName string,
+) (secretToRemove, error) {
+	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	if err != nil {
+		return secretToRemove{}, err
 	}
 
-	// Check if secret exists
 	vaultDir, err := currentVlt.GetDirectory()
 	if err != nil {
-		return err
+		return secretToRemove{}, err
 	}
 
 	encodedName := strings.ReplaceAll(secretName, "/", "%")
@@ -729,32 +770,30 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
 
 	exists, err := afero.DirExists(cli.fs, secretDir)
 	if err != nil {
-		return fmt.Errorf("failed to check if secret exists: %w", err)
+		return secretToRemove{},
+			fmt.Errorf("failed to check if secret exists: %w", err)
 	}
 
 	if !exists {
-		return fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
+		return secretToRemove{},
+			fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
 	}
 
-	// Count versions for information
-	versionsDir := filepath.Join(secretDir, "versions")
-	versionCount := 0
-
-	entries, err := afero.ReadDir(cli.fs, versionsDir)
-	if err == nil {
-		versionCount = len(entries)
+	// A secret without a versions directory has no versions, and can
+	// still be removed.
+	versions, err := afero.ReadDir(cli.fs, filepath.Join(secretDir, "versions"))
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return secretToRemove{}, fmt.Errorf(
+			"failed to count the versions of secret '%s': %w", secretName, err)
 	}
 
-	// Remove the secret directory
-	err = secret.RemoveDirAtomic(cli.fs, secretDir)
-	if err != nil {
-		return fmt.Errorf("failed to remove secret: %w", err)
-	}
-
-	cmd.Printf("Removed secret '%s' (%d version(s) deleted)\n",
-		secretName, versionCount)
-
-	return nil
+	return secretToRemove{
+		dir:      secretDir,
+		versions: len(versions),
+		question: fmt.Sprintf("Permanently remove secret '%s' and its %d "+
+			"version(s) from vault '%s'?",
+			secretName, len(versions), currentVlt.GetName()),
+	}, nil
 }
 
 // MoveSecret moves or renames a secret (within or across vaults), holding

internal/cli/unlockers.go

@@ -47,7 +47,6 @@ var (
 	errGPGKeyAlreadyUnlocker = errors.New(
 		"is already added as an unlocker")
 	errUnsupportedUnlockerType = errors.New("unsupported unlocker type")
-	errLastUnlocker            = errors.New("refusing to remove last unlocker")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -267,10 +266,11 @@ func newUnlockerRemoveCmd() *cobra.Command {
 		Use:     "remove <unlocker-id>",
 		Aliases: []string{"rm"},
 		Short:   "Remove an unlocker",
-		Long: `Remove an unlocker from the current vault. Cannot remove ` +
-			`the last unlocker if the vault has secrets unless --force is ` +
-			`used. Warning: Without unlockers and without your mnemonic, ` +
-			`vault data will be permanently inaccessible.`,
+		Long: `Remove an unlocker from the current vault. Asks for ` +
+			`confirmation first, saying whether it is the vault's last ` +
+			`unlocker; when stdin is not a terminal, fails unless --force ` +
+			`is given. Warning: Without unlockers and without your ` +
+			`mnemonic, vault data will be permanently inaccessible.`,
 		Args:              cobra.ExactArgs(1),
 		ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -286,7 +286,7 @@ func newUnlockerRemoveCmd() *cobra.Command {
 	}
 
 	cmd.Flags().BoolP("force", "f", false,
-		"Force removal of last unlocker even if vault has secrets")
+		"Remove without asking for confirmation, even the last unlocker")
 
 	return cmd
 }
@@ -726,55 +726,91 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	return nil
 }
 
-// UnlockersRemove removes an unlocker, holding the state directory lock
-// while removeUnlocker runs
+// UnlockersRemove removes an unlocker from the current vault, after asking
+// the user to confirm unless force is set.
 func (cli *Instance) UnlockersRemove(
 	unlockerID string, force bool, cmd *cobra.Command,
 ) error {
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	var found unlockerToRemove
+
+	release, err := cli.askThenLock(cmd, force, func() (string, error) {
+		var err error
+
+		found, err = cli.findUnlockerToRemove(unlockerID)
+
+		return found.question, err
+	})
 	if err != nil {
 		return err
 	}
 	defer release()
 
-	return cli.removeUnlocker(unlockerID, force, cmd)
+	return cli.removeUnlocker(unlockerID, found, cmd)
 }
 
-// removeUnlocker removes an unlocker with safety checks
-func (cli *Instance) removeUnlocker(
-	unlockerID string, force bool, cmd *cobra.Command,
-) error {
-	// Get current vault
+// unlockerToRemove is what removing an unlocker removes, as
+// findUnlockerToRemove found it.
+type unlockerToRemove struct {
+	vlt *vault.Vault
+	// last is set when the unlocker counts as the vault's last one, and
+	// secrets is then the number of secrets in the vault.
+	last    bool
+	secrets int
+	// question names what is removed, for the user to confirm.
+	question string
+}
+
+// findUnlockerToRemove checks that the current vault has the unlocker and
+// finds whether it is the vault's last one.
+func (cli *Instance) findUnlockerToRemove(
+	unlockerID string,
+) (unlockerToRemove, error) {
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
-		return err
+		return unlockerToRemove{}, err
+	}
+
+	exists, err := vlt.HasUnlocker(unlockerID)
+	if err != nil {
+		return unlockerToRemove{}, err
+	}
+
+	if !exists {
+		return unlockerToRemove{}, fmt.Errorf("unlocker with ID %s %w",
+			unlockerID, vault.ErrUnlockerNotFound)
 	}
 
 	// Get list of unlockers. It leaves out a directory whose metadata file
 	// is missing or cannot be checked for, read or parsed.
 	unlockers, err := vlt.ListUnlockers()
 	if err != nil {
-		return fmt.Errorf("failed to list unlockers: %w", err)
+		return unlockerToRemove{},
+			fmt.Errorf("failed to list unlockers: %w", err)
 	}
 
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		return fmt.Errorf("failed to get vault directory: %w", err)
+		return unlockerToRemove{},
+			fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
 	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
-	// Check if we're removing the last unlocker
-	removingLast := false
+	found := unlockerToRemove{
+		vlt: vlt,
+		question: fmt.Sprintf("Permanently remove unlocker '%s' from vault "+
+			"'%s'? It is not the vault's last unlocker.",
+			unlockerID, vlt.GetName()),
+	}
 
 	if len(unlockers) == 1 {
 		lastID, err := findUnlockerIDByMetadata(
 			cli.fs, unlockersDir, unlockers[0], true)
 		if err != nil {
-			return err
+			return unlockerToRemove{}, err
 		}
 
-		removingLast = lastID == unlockerID
+		found.last = lastID == unlockerID
 	}
 
 	// unlockerID may instead name a directory left out of the list. If its
@@ -783,34 +819,36 @@ func (cli *Instance) removeUnlocker(
 	// for or read, the unlocker may be the only working one, so removing it
 	// counts as removing the last unlocker.
 	if metadataUnreadable(cli.fs, filepath.Join(unlockersDir, unlockerID)) {
-		removingLast = true
+		found.last = true
 	}
 
-	if removingLast {
-		// Check if vault has secrets
-		numSecrets, err := vlt.NumSecrets()
+	if found.last {
+		found.secrets, err = vlt.NumSecrets()
 		if err != nil {
-			return fmt.Errorf("failed to count secrets: %w", err)
+			return unlockerToRemove{},
+				fmt.Errorf("failed to count secrets: %w", err)
 		}
 
-		if numSecrets > 0 && !force {
-			cmd.Println("ERROR: Cannot remove the last unlocker when the " +
-				"vault contains secrets.")
-			cmd.Println("WARNING: Without unlockers, you MUST have your " +
-				"mnemonic phrase to decrypt the vault.")
-			cmd.Println("If you want to proceed anyway, use --force")
-
-			return errLastUnlocker
-		}
-
-		if numSecrets > 0 && force {
-			cmd.Println("WARNING: Removing the last unlocker. You MUST " +
-				"have your mnemonic phrase to access this vault again!")
-		}
+		found.question = fmt.Sprintf("Permanently remove unlocker '%s', "+
+			"the last unlocker of vault '%s', which holds %d secret(s)? "+
+			"Without an unlocker the vault opens only with its mnemonic.",
+			unlockerID, vlt.GetName(), found.secrets)
 	}
 
-	// Remove the unlocker
-	err = vlt.RemoveUnlocker(unlockerID)
+	return found, nil
+}
+
+// removeUnlocker removes the unlocker that findUnlockerToRemove found. The
+// caller holds the state directory lock.
+func (cli *Instance) removeUnlocker(
+	unlockerID string, found unlockerToRemove, cmd *cobra.Command,
+) error {
+	if found.last && found.secrets > 0 {
+		cmd.Println("WARNING: Removing the last unlocker. You MUST " +
+			"have your mnemonic phrase to access this vault again!")
+	}
+
+	err := found.vlt.RemoveUnlocker(unlockerID)
 	if err != nil {
 		return err
 	}

internal/cli/unlockers_corrupt_test.go

@@ -5,14 +5,15 @@
 // one the commands act on, metadata that is not JSON, and check that the
 // commands step past it, and that it can itself be removed by its
 // directory name, which `secret unlocker list` names in its warning. A
-// last test checks that an unlocker whose metadata file cannot be read is
-// removed by its directory name only as the last unlocker is.
+// last test checks that an unlocker whose metadata file cannot be read
+// counts as the last unlocker when it is removed by its directory name.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli
 
 import (
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"git.eeqj.de/sneak/secret/internal/vault"
@@ -56,37 +57,27 @@ func TestUnlockerSelectSkipsCorruptUnlocker(t *testing.T) {
 }
 
 // TestUnlockerRemoveWithCorruptUnlocker asserts that the second unlocker
-// can be removed, unless the vault holds secrets: the corrupt unlocker
-// cannot unlock the vault, so the second is its last. The corrupt one can
-// be removed by its directory name without --force even then.
+// counts as the vault's last one, since the corrupt unlocker cannot unlock
+// the vault, and that the corrupt one, removed by its directory name, does
+// not. Either is removed once the user confirms.
 func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
 		name        string
 		unlockerID  string
-		withSecret  bool
-		wantErr     error
+		wantLast    bool
 		wantEntries []string
 	}{
 		{
 			name:        "the other unlocker",
 			unlockerID:  "pgp-" + listTestGPGKeyID + "B",
+			wantLast:    true,
 			wantEntries: []string{listTestUnlockerDirOne},
 		},
-		{
-			name:       "the other unlocker, the last one, with secrets",
-			unlockerID: "pgp-" + listTestGPGKeyID + "B",
-			withSecret: true,
-			wantErr:    errLastUnlocker,
-			wantEntries: []string{
-				listTestUnlockerDirOne, listTestUnlockerDirTwo,
-			},
-		},
 		{
 			name:        "the corrupt unlocker by its directory name",
 			unlockerID:  listTestUnlockerDirOne,
-			withSecret:  true,
 			wantEntries: []string{listTestUnlockerDirTwo},
 		},
 	}
@@ -96,14 +87,16 @@ func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
 			t.Parallel()
 
 			fs := newCorruptUnlockerVault(t)
-			if tt.withSecret {
-				writeTestSecret(t, fs, testVaultDir(listTestVaultName))
-			}
+			writeTestSecret(t, fs, testVaultDir(listTestVaultName))
 
 			instance, cmd := newTestInstance(fs)
 
-			err := instance.UnlockersRemove(tt.unlockerID, false, cmd)
-			require.ErrorIs(t, err, tt.wantErr)
+			found, err := instance.findUnlockerToRemove(tt.unlockerID)
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantLast, found.last)
+
+			instance.terminal = strings.NewReader("y\n")
+			require.NoError(t, instance.UnlockersRemove(tt.unlockerID, false, cmd))
 
 			assertDirEntries(t, fs,
 				filepath.Join(testVaultDir(listTestVaultName),
@@ -113,13 +106,14 @@ func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
 	}
 }
 
-// TestUnlockerRemoveWithUnreadableMetadata asserts that removing the only
-// unlocker of a vault with secrets by its directory name, when its
-// metadata file cannot be checked for or read, is refused without --force:
-// listing leaves it out, but it may still be the vault's only working
-// unlocker. With --force it is removed. The state directory lock refuses
-// the failing filesystem, so the test calls removeUnlocker, which
-// UnlockersRemove runs once it holds the lock.
+// TestUnlockerRemoveWithUnreadableMetadata asserts that the only unlocker
+// of a vault with secrets, removed by its directory name when its metadata
+// file cannot be checked for or read, counts as the vault's last unlocker,
+// so the question warns that it is: listing leaves it out, but it may
+// still be the vault's only working unlocker. It is then removed. The
+// state directory lock refuses the failing filesystem, so the test calls
+// findUnlockerToRemove and removeUnlocker, which UnlockersRemove runs to
+// make its checks and, once it holds the lock, to remove the unlocker.
 func TestUnlockerRemoveWithUnreadableMetadata(t *testing.T) {
 	t.Parallel()
 
@@ -155,12 +149,13 @@ func TestUnlockerRemoveWithUnreadableMetadata(t *testing.T) {
 
 			instance, cmd := newTestInstance(tt.wrap(base))
 
-			err := instance.removeUnlocker(listTestUnlockerDirOne, false, cmd)
-			require.ErrorIs(t, err, errLastUnlocker)
-			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
+			found, err := instance.findUnlockerToRemove(listTestUnlockerDirOne)
+			require.NoError(t, err)
+			assert.True(t, found.last)
+			assert.Contains(t, found.question, "the last unlocker")
 
 			require.NoError(t,
-				instance.removeUnlocker(listTestUnlockerDirOne, true, cmd))
+				instance.removeUnlocker(listTestUnlockerDirOne, found, cmd))
 			assertDirEntries(t, base, unlockersDir)
 		})
 	}

internal/cli/unreadable_dir_test.go

@@ -9,8 +9,10 @@
 //
 // 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 +287,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 +333,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)
 

internal/cli/vault.go

@@ -31,8 +31,6 @@ var (
 	errPassphraseEnvNotSet = errors.New(
 		"SB_UNLOCK_PASSPHRASE environment variable not set")
 	errCannotRemoveLastVault = errors.New("cannot remove the last vault")
-	errVaultContainsSecrets  = errors.New(
-		"contains secrets; use --force to remove")
 )
 
 func newVaultCmd() *cobra.Command {
@@ -156,9 +154,12 @@ func newVaultRemoveCmd() *cobra.Command {
 		Use:     "remove <name>",
 		Aliases: []string{"rm"},
 		Short:   "Remove a vault",
-		Long: `Remove a vault. Requires --force if the vault contains ` +
-			`secrets. Will automatically switch to another vault if ` +
-			`removing the currently selected one.`,
+		Long: `Remove a vault and all its secrets. Asks for ` +
+			`confirmation first, naming how many secrets the vault ` +
+			`holds; when stdin is not a terminal, fails unless --force ` +
+			`is given. Will automatically switch to another vault if ` +
+			`removing the currently selected one. The last vault ` +
+			`cannot be removed.`,
 		Args:              cobra.ExactArgs(1),
 		ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -173,7 +174,8 @@ func newVaultRemoveCmd() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().BoolP("force", "f", false, "Force removal even if vault contains secrets")
+	cmd.Flags().BoolP("force", "f", false,
+		"Remove without asking for confirmation, even a vault that contains secrets")
 
 	return cmd
 }
@@ -537,27 +539,27 @@ func (cli *Instance) importMnemonic(cmd *cobra.Command, vaultName string) error
 	return nil
 }
 
-// vaultHasSecrets reports whether the vault directory contains any secrets
-func (cli *Instance) vaultHasSecrets(vaultDir string) (bool, error) {
+// countVaultSecrets returns the number of secrets in the vault directory
+func (cli *Instance) countVaultSecrets(vaultDir string) (int, error) {
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
 	exists, err := afero.DirExists(cli.fs, secretsDir)
 	if err != nil {
-		return false, fmt.Errorf("failed to check secrets directory %s: %w",
+		return 0, fmt.Errorf("failed to check secrets directory %s: %w",
 			secretsDir, err)
 	}
 
 	if !exists {
-		return false, nil
+		return 0, nil
 	}
 
 	entries, err := afero.ReadDir(cli.fs, secretsDir)
 	if err != nil {
-		return false, fmt.Errorf("failed to read secrets directory %s: %w",
+		return 0, fmt.Errorf("failed to read secrets directory %s: %w",
 			secretsDir, err)
 	}
 
-	return len(entries) > 0, nil
+	return len(entries), nil
 }
 
 // switchAwayFromVault selects another vault as current before removal
@@ -586,88 +588,107 @@ func (cli *Instance) switchAwayFromVault(
 	return nil
 }
 
-// RemoveVault removes a vault, holding the state directory lock while
-// removeVault runs
+// RemoveVault removes a vault and all its secrets, after asking the user
+// to confirm unless force is set.
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
 	err := vault.ValidateVaultName(name)
 	if err != nil {
 		return err
 	}
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	var found vaultToRemove
+
+	release, err := cli.askThenLock(cmd, force, func() (string, error) {
+		var err error
+
+		found, err = cli.findVaultToRemove(name)
+
+		return found.question, err
+	})
 	if err != nil {
 		return err
 	}
 	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 {
-		return fmt.Errorf("failed to list vaults: %w", err)
-	}
-
-	// Check if vault exists
-	if !slices.Contains(vaults, name) {
-		return fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
-	}
-
-	// Don't allow removing the last vault
-	if len(vaults) == 1 {
-		return errCannotRemoveLastVault
-	}
-
-	// Check if this is the current vault
-	currentVault, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
-	if err != nil {
-		return fmt.Errorf("failed to get current vault: %w", err)
-	}
-
-	isCurrentVault := currentVault.GetName() == name
-
-	// Load the vault to check for secrets
-	vlt := vault.NewVault(cli.fs, cli.stateDir, name)
-
-	vaultDir, err := vlt.GetDirectory()
-	if err != nil {
-		return fmt.Errorf("failed to get vault directory: %w", err)
-	}
-
-	// Check if vault has secrets
-	hasSecrets, err := cli.vaultHasSecrets(vaultDir)
-	if err != nil {
-		return err
-	}
-
-	// Require --force if vault has secrets
-	if hasSecrets && !force {
-		return fmt.Errorf("vault '%s' %w", name, errVaultContainsSecrets)
-	}
-
 	// If removing current vault, switch to another vault first
-	if isCurrentVault {
-		err = cli.switchAwayFromVault(cmd, vaults, name)
+	if found.isCurrent {
+		err = cli.switchAwayFromVault(cmd, found.vaults, name)
 		if err != nil {
 			return err
 		}
 	}
 
 	// Remove the vault directory
-	err = secret.RemoveDirAtomic(cli.fs, vaultDir)
+	err = secret.RemoveDirAtomic(cli.fs, found.dir)
 	if err != nil {
 		return fmt.Errorf("failed to remove vault directory: %w", err)
 	}
 
 	cmd.Printf("Removed vault '%s'\n", name)
 
-	if hasSecrets {
+	if found.secrets > 0 {
 		cmd.Printf("Warning: Vault contained secrets that have been " +
 			"permanently deleted\n")
 	}
 
 	return nil
 }
+
+// vaultToRemove is what removing a vault removes, as findVaultToRemove
+// found it.
+type vaultToRemove struct {
+	// dir is the vault's directory, which holds all its secrets.
+	dir     string
+	secrets int
+	// vaults lists every vault, this one included, and isCurrent is set
+	// when this one is the current vault.
+	vaults    []string
+	isCurrent bool
+	// question names what is removed, for the user to confirm.
+	question string
+}
+
+// findVaultToRemove checks that the vault exists and is not the last one,
+// and counts its secrets.
+func (cli *Instance) findVaultToRemove(name string) (vaultToRemove, error) {
+	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
+	if err != nil {
+		return vaultToRemove{}, fmt.Errorf("failed to list vaults: %w", err)
+	}
+
+	if !slices.Contains(vaults, name) {
+		return vaultToRemove{},
+			fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
+	}
+
+	if len(vaults) == 1 {
+		return vaultToRemove{}, errCannotRemoveLastVault
+	}
+
+	currentVault, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	if err != nil {
+		return vaultToRemove{},
+			fmt.Errorf("failed to get current vault: %w", err)
+	}
+
+	vaultDir, err := vault.NewVault(cli.fs, cli.stateDir, name).GetDirectory()
+	if err != nil {
+		return vaultToRemove{},
+			fmt.Errorf("failed to get vault directory: %w", err)
+	}
+
+	secrets, err := cli.countVaultSecrets(vaultDir)
+	if err != nil {
+		return vaultToRemove{}, err
+	}
+
+	return vaultToRemove{
+		dir:       vaultDir,
+		secrets:   secrets,
+		vaults:    vaults,
+		isCurrent: currentVault.GetName() == name,
+		question: fmt.Sprintf(
+			"Permanently remove vault '%s' and its %d secret(s)?",
+			name, secrets),
+	}, nil
+}

internal/cli/version.go

@@ -89,7 +89,8 @@ func VersionCommands(cli *Instance) *cobra.Command {
 		Aliases: []string{"rm"},
 		Short:   "Remove a specific version of a secret",
 		Long: "Remove a specific version of a secret. Cannot remove the " +
-			"current version.",
+			"current version. Asks for confirmation first; when stdin " +
+			"is not a terminal, fails unless --force is given.",
 		Args: cobra.ExactArgs(2), //nolint:mnd // secret-name and version args
 		ValidArgsFunction: func(
 			cmd *cobra.Command, args []string, toComplete string,
@@ -102,10 +103,15 @@ func VersionCommands(cli *Instance) *cobra.Command {
 			return nil, cobra.ShellCompDirectiveNoFileComp
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return cli.RemoveVersion(cmd, args[0], args[1])
+			force, _ := cmd.Flags().GetBool("force")
+
+			return cli.RemoveVersion(cmd, args[0], args[1], force)
 		},
 	}
 
+	removeCmd.Flags().BoolP("force", "f", false,
+		"Remove without asking for confirmation")
+
 	versionCmd.AddCommand(listCmd, promoteCmd, removeCmd)
 
 	return versionCmd
@@ -297,30 +303,62 @@ func (cli *Instance) PromoteVersion(
 	return nil
 }
 
-// RemoveVersion removes a specific version of a secret
+// RemoveVersion removes a specific version of a secret, after asking the
+// user to confirm unless force is set.
 func (cli *Instance) RemoveVersion(
-	cmd *cobra.Command, secretName string, version string,
+	cmd *cobra.Command, secretName string, version string, force bool,
 ) error {
 	err := vault.ValidateSecretName(secretName)
 	if err != nil {
 		return err
 	}
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	var found versionToRemove
+
+	release, err := cli.askThenLock(cmd, force, func() (string, error) {
+		var err error
+
+		found, err = cli.findVersionToRemove(secretName, version)
+
+		return found.question, err
+	})
 	if err != nil {
 		return err
 	}
 	defer release()
 
-	// Get current vault
+	err = secret.RemoveDirAtomic(cli.fs, found.dir)
+	if err != nil {
+		return fmt.Errorf("failed to remove version: %w", err)
+	}
+
+	cmd.Printf("Removed version %s of secret '%s'\n", version, secretName)
+
+	return nil
+}
+
+// versionToRemove is what removing a version removes, as
+// findVersionToRemove found it.
+type versionToRemove struct {
+	// dir is the version's directory.
+	dir string
+	// question names what is removed, for the user to confirm.
+	question string
+}
+
+// findVersionToRemove checks that the version exists in the secret in the
+// current vault and is not its current version.
+func (cli *Instance) findVersionToRemove(
+	secretName, version string,
+) (versionToRemove, error) {
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
-		return err
+		return versionToRemove{}, err
 	}
 
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
-		return err
+		return versionToRemove{}, err
 	}
 
 	// Get the encoded secret name
@@ -330,45 +368,44 @@ func (cli *Instance) RemoveVersion(
 	// Check if secret exists
 	exists, err := afero.DirExists(cli.fs, secretDir)
 	if err != nil {
-		return fmt.Errorf("failed to check if secret exists: %w", err)
+		return versionToRemove{},
+			fmt.Errorf("failed to check if secret exists: %w", err)
 	}
 
 	if !exists {
-		return fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
+		return versionToRemove{},
+			fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
 	}
 
 	// Check if version exists
 	exists, err = secret.VersionExists(cli.fs, secretDir, version)
 	if err != nil {
-		return fmt.Errorf("failed to check if version exists: %w", err)
+		return versionToRemove{},
+			fmt.Errorf("failed to check if version exists: %w", err)
 	}
 
 	if !exists {
-		return fmt.Errorf("version '%s' %w '%s'",
+		return versionToRemove{}, fmt.Errorf("version '%s' %w '%s'",
 			version, errVersionNotFound, secretName)
 	}
 
 	// Get current version
 	currentVersion, err := secret.GetCurrentVersion(cli.fs, secretDir)
 	if err != nil {
-		return fmt.Errorf("failed to get current version: %w", err)
+		return versionToRemove{},
+			fmt.Errorf("failed to get current version: %w", err)
 	}
 
 	// Don't allow removing the current version
 	if version == currentVersion {
-		return fmt.Errorf("cannot remove the current version '%s'; %w",
+		return versionToRemove{}, fmt.Errorf(
+			"cannot remove the current version '%s'; %w",
 			version, errCannotRemoveCurrentVersion)
 	}
 
-	// Remove the version directory
-	versionDir := filepath.Join(secretDir, "versions", version)
-
-	err = secret.RemoveDirAtomic(cli.fs, versionDir)
-	if err != nil {
-		return fmt.Errorf("failed to remove version: %w", err)
-	}
-
-	cmd.Printf("Removed version %s of secret '%s'\n", version, secretName)
-
-	return nil
+	return versionToRemove{
+		dir: filepath.Join(secretDir, "versions", version),
+		question: fmt.Sprintf("Permanently remove version %s of secret "+
+			"'%s' from vault '%s'?", version, secretName, vlt.GetName()),
+	}, nil
 }

internal/vault/unlockers.go

@@ -272,6 +272,24 @@ func (v *Vault) readUnlockerMetadataOrWarn(
 	return metadata, true
 }
 
+// HasUnlocker reports whether RemoveUnlocker finds something to remove by
+// the ID unlockerID: an unlocker with that ID, or an unlocker directory of
+// that name that ListUnlockers skips.
+func (v *Vault) HasUnlocker(unlockerID string) (bool, error) {
+	vaultDir, err := v.GetDirectory()
+	if err != nil {
+		return false, err
+	}
+
+	_, unlockerDir, err := v.findUnlockerByID(
+		filepath.Join(vaultDir, "unlockers.d"), unlockerID)
+	if err != nil {
+		return false, err
+	}
+
+	return unlockerDir != "", nil
+}
+
 // RemoveUnlocker removes an unlocker from this vault. An unlocker
 // directory that ListUnlockers skips is removed by its directory name; its
 // type is unknown, so only the directory is removed.