Delete .tmp- leftovers of a killed command when the lock is next taken (closes #75)

A command killed part-way could leave a temporary file or directory of
secret.WriteFileAtomic or secret.TempDirFor, encrypted keys included,
for good. LockStateDir now empties the lock file once it holds the lock
and writes "finished" there just before releasing it. A holder that
does not find that deletes such leftovers from the state directory,
each vault, each secret and each version, the only places those helpers
make them, matching names that start with "." and hold ".tmp-". After a
command that finished nothing is searched, so the added time does not
grow with the number of secrets and versions. A test shows that
`unlocker remove` removes an unlocker directory with no metadata file.

Model: opus-5-5

README.md

@@ -70,6 +70,24 @@ make build
 
 ## Commands Reference
 
+### Confirmation Before Removal
+
+`secret rm`, `secret version rm`, `secret vault remove` and
+`secret unlocker remove` destroy data that exists nowhere else. On a terminal
+each one first asks `[y/N]`, naming exactly what it is about to remove, and
+goes ahead only on `y` or `yes`; any other answer, a bare Enter included,
+cancels and removes nothing. The question is asked only after the command's
+checks have passed, and before it changes anything.
+
+Whether to ask is decided by stdin, where the answer is read from, so
+`secret rm foo | tee log` still asks. When stdin is not a terminal, as in a
+script or a CI job, nobody is there to answer: the command fails at once,
+removes nothing, and says to pass `--force`.
+
+`--force` (`-f`) removes without asking, whatever the command removes: a vault
+that holds secrets and the last unlocker of a vault included. Scripts that
+remove things pass `--force`.
+
 ### Initialization
 
 #### `secret init`
@@ -100,13 +118,13 @@ Switches to the specified vault for subsequent operations.
 
 #### `secret vault remove <name> [--force]` / `secret vault rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes a vault and all its secrets. Like Unix `rm`,
-this command does not ask for confirmation.
+**DANGER**: Permanently removes a vault and all its secrets. It first asks
+for confirmation, naming the vault and how many secrets it holds (see
+[Confirmation Before Removal](#confirmation-before-removal)). The last vault
+cannot be removed. Removing the current vault makes another vault the current
+one.
 
-Requires --force if the vault contains secrets. With --force, will
-automatically switch to another vault if removing the current one.
-
-- `--force, -f`: Force removal even if vault contains secrets
+- `--force, -f`: Remove without asking, also a vault that contains secrets
 - **NO RECOVERY**: All secrets in the vault will be permanently deleted
 
 ### Secret Management
@@ -132,9 +150,12 @@ Retrieves and outputs a secret value to stdout.
 Lists all secrets in the current vault. Optional filter for substring
 matching.
 
-#### `secret remove <secret-name>` / `secret rm` ⚠️ 🛑
+#### `secret remove <secret-name> [--force]` / `secret rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes a secret and ALL its versions. Like Unix `rm`, this command does not ask for confirmation.
+**DANGER**: Permanently removes a secret and ALL its versions. It first asks
+for confirmation, naming the secret, its vault and how many versions it has
+(see [Confirmation Before Removal](#confirmation-before-removal)).
+- `--force, -f`: Remove without asking
 - **NO RECOVERY**: Once removed, the secret cannot be recovered
 - **ALL VERSIONS DELETED**: Every version of the secret will be permanently deleted
 
@@ -158,10 +179,12 @@ Lists all versions of a secret showing creation time, status, and validity perio
 Promotes a specific version to current by updating the symlink. Does not
 modify any timestamps, allowing for rollback scenarios.
 
-#### `secret version remove <secret-name> <version>` / `secret version rm` ⚠️ 🛑
+#### `secret version remove <secret-name> <version> [--force]` / `secret version rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes a specific version of a secret. Like Unix
-`rm`, this command does not ask for confirmation.
+**DANGER**: Permanently removes a specific version of a secret. It first asks
+for confirmation, naming the version, the secret and its vault (see
+[Confirmation Before Removal](#confirmation-before-removal)).
+- `--force, -f`: Remove without asking
 - **NO RECOVERY**: Once removed, this version cannot be recovered
 - Cannot remove the current version (must promote another version first)
 
@@ -202,12 +225,15 @@ has, which is removed only once the new one is the current unlocker.
 
 #### `secret unlocker remove <unlocker-id> [--force]` / `secret unlocker rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes an unlocker. Like Unix `rm`, this command
-does not ask for confirmation.  Cannot remove the last unlocker if the vault
-has secrets unless --force is used. An unlocker directory that
-`secret unlocker list` skips with a warning, because its metadata cannot be
-read or parsed, is removed by the directory name the warning gives.
-- `--force, -f`: Force removal of last unlocker even if vault has secrets
+**DANGER**: Permanently removes an unlocker. It first asks for confirmation,
+naming the unlocker and its vault and saying whether it is the vault's last
+unlocker; for the last one it says how many secrets the vault holds and warns
+that the vault then opens only with its mnemonic (see
+[Confirmation Before Removal](#confirmation-before-removal)). An unlocker
+directory that `secret unlocker list` skips with a warning, because its
+metadata cannot be read or parsed, is removed by the directory name the
+warning gives.
+- `--force, -f`: Remove without asking, even the last unlocker
 - **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase,
   vault data will be PERMANENTLY INACCESSIBLE
 - **NO RECOVERY**: Removing all unlockers without having your mnemonic means
@@ -377,7 +403,7 @@ secret list
 secret get database/prod/password
 secret get services/api/key
 
-# Remove a secret ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!)
+# Remove a secret ⚠️ 🛑 (asks first - PERMANENT!)
 secret remove ssh/servers/web01
 ```
 
@@ -400,7 +426,7 @@ echo "personal-email-pass" | secret add email/password
 # List all vaults
 secret vault list
 
-# Remove a vault ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!)
+# Remove a vault ⚠️ 🛑 (--force: NO CONFIRMATION - PERMANENT!)
 secret vault remove personal --force
 ```
 
@@ -418,7 +444,7 @@ secret unlocker list
 # Select a specific unlocker
 secret unlocker select <unlocker-id>
 
-# Remove an unlocker ⚠️ 🛑 (NO CONFIRMATION!)
+# Remove an unlocker ⚠️ 🛑 (asks first!)
 secret unlocker remove <unlocker-id>
 ```
 
@@ -431,7 +457,7 @@ secret version list database/prod/password
 # Promote an older version to current
 secret version promote database/prod/password 20231215.001
 
-# Remove an old version ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!)
+# Remove an old version ⚠️ 🛑 (asks first - PERMANENT!)
 secret version remove database/prod/password 20231214.001
 ```
 

TODO.md

@@ -25,6 +25,35 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-04: What a command killed part-way left under a `.tmp-` name
+  (https://git.eeqj.de/sneak/secret/issues/75), the temporary directories
+  of `secret.TempDirFor` and the temporary files of
+  `secret.WriteFileAtomic`, encrypted keys included, is deleted by the next
+  command that takes the state directory lock. Before, it stayed until
+  deleted by hand. A command writes `finished` into the lock file just
+  before it releases the lock; the next one to take the lock searches only
+  when it does not find that, so after a command that finished nothing is
+  searched, however many secrets and versions there are. The search looks
+  in the state directory, each vault, each secret and each version, the
+  only directories those helpers make them in. A command that only reads
+  takes no lock and deletes nothing. A failure to delete is warned about
+  and the command goes on. An unlocker directory with no metadata file was
+  already removed by `secret unlocker remove` given its directory name; a
+  test now shows it.
+- 2026-10-04: `secret rm`, `secret version rm`, `secret vault remove` and
+  `secret unlocker remove` ask `[y/N]` before removing anything
+  (https://git.eeqj.de/sneak/secret/issues/39), naming what they remove: the
+  secret, its vault and its version count; the version, secret and vault; the
+  vault and its secret count; the unlocker, its vault and whether it is the
+  last, and for the last the vault's secret count and that the vault then
+  opens only with its mnemonic. Only `y` or `yes` goes ahead. 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 of one
+  without `--force`, which the question now covers. The checks run, and the
+  question is asked, before the state directory lock is taken; under the
+  lock the checks run again, and if they would ask a different question,
+  nothing is removed. `secret rm` fails when it cannot count the versions.
 - 2026-10-04: A crash while an unlocker is being replaced no longer leaves a
   current unlocker that cannot open the vault
   (https://git.eeqj.de/sneak/secret/issues/71). Every new unlocker gets a
@@ -188,15 +217,10 @@ Bring the repo into policy compliance in one commit:
   cross-vault copies are built in a temporary directory and renamed
   into place, and removals rename out of the way first, so a version
   or secret is never half-added and never half-removed. An
-  interrupted command can still leave:
-    - from `init` or `vault create` killed after the passphrase prompt
-      but before the unlocker is written, a vault with no unlocker,
-      which `vault create` has already made the current vault;
-    - data under a `.tmp-` name in the state directory: a secret,
-      version or unlocker being added, or the secret, version, unlocker
-      or vault being removed, encrypted keys included. Nothing deletes
-      it; it must be deleted by hand
-      (https://git.eeqj.de/sneak/secret/issues/75).
+  interrupted command can still leave, from `init` or `vault create`
+  killed after the passphrase prompt but before the unlocker is
+  written, a vault with no unlocker, which `vault create` has already
+  made the current vault.
 - 2026-10-03: The checks run before changing a vault now stop with an
   error naming the path and cause when they cannot read what they
   inspect, instead of reading the failure as "nothing there": the
@@ -304,8 +328,6 @@ Bring the repo into policy compliance in one commit:
 - High priority:
     - Secure temporary file handling and cleanup.
     - Initialize a default unlock key at vault creation.
-    - Confirmation prompts for destructive operations (keys rm, vault
-      deletion).
     - Add secret rm and vault deletion commands.
 - Medium priority:
     - Standardize error messages; stop leaking internals.

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/btcsuite/btcd/btcec/v2 v2.1.3
 	github.com/btcsuite/btcd/btcutil v1.1.6
 	github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
+	github.com/creack/pty v1.1.24
 	github.com/keybase/go-keychain v0.0.0-20230307172405-3e4884637dd1
 	github.com/oklog/ulid/v2 v2.1.1
 	github.com/spf13/afero v1.14.0

go.sum

@@ -35,6 +35,8 @@ github.com/btcsuite/snappy-go v1.0.0/go.mod h1:8woku9dyThutzjeg+3xrA5iCpBRH8XEEg
 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
 github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
+github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
+github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

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, whether stdin is a terminal decides; the tests in
+// integration_test.go that run `secret rm` on a pseudo-terminal cover that.
+
+//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

@@ -2,10 +2,13 @@
 package cli_test
 
 import (
+	"bufio"
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -16,7 +19,11 @@ 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/creack/pty"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -2543,3 +2550,145 @@ func copyFile(src, dst string) error {
 
 	return nil
 }
+
+// secretRmCommand makes a state directory whose vault "default" holds the
+// secret "x", and returns `secret rm x` on the built binary against it, and
+// the directory of "x". The vault has no unlocker, so making it derives no
+// key from a passphrase.
+func secretRmCommand(ctx context.Context, t *testing.T) (*exec.Cmd, string) {
+	t.Helper()
+
+	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))
+
+	//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"),
+	}
+
+	return cmd, filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "x")
+}
+
+// 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()
+
+	// 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()
+
+	cmd, secretDir := secretRmCommand(ctx, t)
+	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, secretDir)
+}
+
+// The next two tests run `secret rm` with a terminal on stdin or on stdout
+// and stderr, not both: whether it asks must depend on stdin alone, where
+// the answer is read from. pty.Open returns the two ends of a new terminal:
+// tty is the end a program uses as its terminal, and ptmx the end the test
+// reads what the terminal shows from and types into.
+
+// TestRemoveIgnoresTerminalOnStdout runs `echo y | secret rm x` at a
+// terminal. stdin is a pipe, so nobody can answer there, and the command
+// must fail as in a script, removing nothing.
+func TestRemoveIgnoresTerminalOnStdout(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
+	defer cancel()
+
+	cmd, secretDir := secretRmCommand(ctx, t)
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	cmd.Stdin = strings.NewReader("y\n")
+	cmd.Stdout = tty
+	cmd.Stderr = tty
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	// The read ends once secret rm has exited and so closed the terminal.
+	shown, _ := io.ReadAll(ptmx)
+
+	require.Error(t, cmd.Wait())
+	assert.Contains(t, string(shown), "pass --force")
+	assert.DirExists(t, secretDir)
+}
+
+// TestRemoveAsksAtTerminalOnStdin runs `secret rm x | cat` at a terminal.
+// It must ask on the terminal, and remove the secret when y is typed there.
+func TestRemoveAsksAtTerminalOnStdin(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
+	defer cancel()
+
+	cmd, secretDir := secretRmCommand(ctx, t)
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	cmd.Stdin = tty
+	// Not a file, so exec.Cmd connects stdout through a pipe.
+	cmd.Stdout = io.Discard
+	cmd.Stderr = tty
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	var (
+		shown []byte
+		char  byte
+	)
+
+	terminal := bufio.NewReader(ptmx)
+	for !bytes.HasSuffix(shown, []byte("[y/N] ")) {
+		char, err = terminal.ReadByte()
+		require.NoError(t, err, "secret rm ended without asking: %s", shown)
+
+		shown = append(shown, char)
+	}
+
+	_, err = ptmx.WriteString("y\n")
+	require.NoError(t, err)
+	require.NoError(t, cmd.Wait())
+	assert.NoDirExists(t, secretDir)
+}

internal/cli/leftovers_test.go

@@ -0,0 +1,74 @@
+package cli_test
+
+import (
+	"io"
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"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/require"
+)
+
+// TestLeftoversRemovedByNextChangingCommand is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/75. It plants what a command
+// killed part-way leaves in each directory where secret.TempDirFor and
+// secret.WriteFileAtomic make temporary entries: a temporary directory
+// holding a vault, secret, unlocker or version being added or removed, and
+// a temporary file beside a file being replaced. `secret list` must leave
+// them all, and the next command that takes the state directory lock, here
+// `secret vault select` of the vault already current, must delete exactly
+// them: a vault named like a temporary directory stays. The state directory
+// holds no lock file, so, as after a killed command, the lock file does not
+// say that the last holder of the lock finished.
+func TestLeftoversRemovedByNextChangingCommand(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+
+	_, err := vault.CreateVault(fs, testStateDir, ".tmp-0", nil)
+	require.NoError(t, err)
+	require.NoError(t, vault.SelectVault(fs, testStateDir, "default"))
+
+	before := snapshotStateDir(t, fs)
+
+	vaultDir := testStateDir + "/vaults.d/default"
+	secretDir := vaultDir + "/secrets.d/x"
+
+	versions, err := secret.ListVersions(fs, secretDir)
+	require.NoError(t, err)
+	require.Len(t, versions, 1)
+
+	for _, dir := range []string{
+		testStateDir + "/.tmp-1/default",
+		vaultDir + "/.tmp-2/x",
+		secretDir + "/.tmp-3/" + testVersion,
+	} {
+		require.NoError(t, fs.MkdirAll(dir, secret.DirPerms))
+		require.NoError(t, afero.WriteFile(fs, dir+"/value.age",
+			[]byte("encrypted"), secret.FilePerms))
+	}
+
+	for _, file := range []string{
+		testStateDir + "/.currentvault.tmp-4",
+		vaultDir + "/.current-unlocker.tmp-5",
+		secretDir + "/.current.tmp-6",
+		secretDir + "/versions/" + versions[0] + "/.metadata.age.tmp-7",
+	} {
+		require.NoError(t, afero.WriteFile(fs, file,
+			[]byte("partial"), secret.FilePerms))
+	}
+
+	planted := snapshotStateDir(t, fs)
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	cmd := &cobra.Command{}
+	cmd.SetOut(io.Discard)
+
+	require.NoError(t, c.ListSecrets(cmd, false, false, ""))
+	require.Equal(t, planted, snapshotStateDir(t, fs))
+
+	require.NoError(t, c.SelectVault(cmd, "default"))
+	require.Equal(t, before, snapshotStateDir(t, fs))
+}

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 {
@@ -360,7 +362,6 @@ func requireWaitsForLock(
 
 	fs := afero.NewMemMapFs()
 	olderVersion, unlockerID := setupEveryCommand(t, fs, withUnlocker)
-	before := stateDirModTimes(t, fs)
 
 	release, err := vault.LockStateDir(fs, testStateDir)
 	require.NoError(t, err)
@@ -370,6 +371,9 @@ func requireWaitsForLock(
 	release = sync.OnceFunc(release)
 	defer release()
 
+	// Taken only now, since taking the lock writes the lock file.
+	before := stateDirModTimes(t, fs)
+
 	unlockPassphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
 	defer unlockPassphrase.Destroy()
 
@@ -431,8 +435,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 +444,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 +456,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

@@ -90,6 +90,8 @@ func newTwoVaultFs(t *testing.T) afero.Fs {
 // snapshotStateDir maps every file under the state directory to its
 // contents, and every directory, written with a trailing "/", to "". Two
 // snapshots are equal only if nothing in it was added, removed or changed.
+// The lock file, which every command that takes the lock writes, is left
+// out.
 func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
 	t.Helper()
 
@@ -102,6 +104,10 @@ func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
 			return err
 		}
 
+		if path == testStateDir+"/lock" {
+			return nil
+		}
+
 		if info.IsDir() {
 			tree[path+"/"] = ""
 
@@ -170,8 +176,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 +197,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 +250,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 +285,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 +367,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 +395,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

@@ -4,15 +4,17 @@
 // by its ID. These tests give the first unlocker, which sorts before the
 // 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.
+// directory name, which `secret unlocker list` names in its warning, as can
+// one with no metadata file. A 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 +58,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 +88,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 +107,42 @@ 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.
+// TestUnlockerRemoveWithoutMetadata asserts that a partial unlocker
+// directory, one with no metadata file, removed by its directory name from
+// a vault with secrets, does not count as the vault's last unlocker, since
+// it cannot unlock the vault, so the question says it is not. It is
+// removed once the user confirms.
+func TestUnlockerRemoveWithoutMetadata(t *testing.T) {
+	t.Parallel()
+
+	fs := newListTestVault(t, 2)
+	vaultDir := testVaultDir(listTestVaultName)
+	unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
+
+	require.NoError(t, fs.Remove(filepath.Join(
+		unlockersDir, listTestUnlockerDirOne, listTestMetadataFileName)))
+	writeTestSecret(t, fs, vaultDir)
+
+	instance, cmd := newTestInstance(fs)
+
+	found, err := instance.findUnlockerToRemove(listTestUnlockerDirOne)
+	require.NoError(t, err)
+	assert.False(t, found.last)
+	assert.Contains(t, found.question, "not the vault's last unlocker")
+
+	instance.terminal = strings.NewReader("y\n")
+	require.NoError(t, instance.UnlockersRemove(listTestUnlockerDirOne, false, cmd))
+	assertDirEntries(t, fs, unlockersDir, listTestUnlockerDirTwo)
+}
+
+// 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 +178,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

@@ -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.

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/secret/atomic.go

@@ -5,10 +5,15 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/afero"
 )
 
+// tempNamePart is in the name of every temporary file WriteFileAtomic makes,
+// ".NAME.tmp-123", and every temporary directory TempDirFor makes, ".tmp-123".
+const tempNamePart = ".tmp-"
+
 // WriteFileAtomic replaces the file at path with data so that a reader, or
 // a crash at any moment, finds either the old content or the new, never a
 // partial file. The data goes into a temporary file that afero.TempFile
@@ -17,7 +22,7 @@ import (
 // temporary file is removed if any step fails.
 func WriteFileAtomic(fs afero.Fs, path string, data []byte) error {
 	tmp, err := afero.TempFile(fs, filepath.Dir(path),
-		"."+filepath.Base(path)+".tmp-*")
+		"."+filepath.Base(path)+tempNamePart+"*")
 	if err != nil {
 		return fmt.Errorf("failed to create temporary file for %s: %w", path, err)
 	}
@@ -54,7 +59,7 @@ func WriteFileAtomic(fs afero.Fs, path string, data []byte) error {
 // Its name leaves out target's, which may already be as long as a file name
 // can be.
 func TempDirFor(fs afero.Fs, target string) (string, error) {
-	dir, err := afero.TempDir(fs, filepath.Dir(filepath.Dir(target)), ".tmp-")
+	dir, err := afero.TempDir(fs, filepath.Dir(filepath.Dir(target)), tempNamePart)
 	if err != nil {
 		return "", fmt.Errorf(
 			"failed to create temporary directory for %s: %w", target, err)
@@ -63,6 +68,40 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
+// RemoveLeftovers deletes from dir the temporary files of WriteFileAtomic
+// and the temporary directories of TempDirFor that a command killed
+// part-way left there: each entry whose name starts with "." and holds
+// tempNamePart. The caller must hold the state directory lock, so that no
+// running command is still using one. A dir that does not exist holds none.
+func RemoveLeftovers(fs afero.Fs, dir string) error {
+	entries, err := afero.ReadDir(fs, dir)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil
+	}
+
+	if err != nil {
+		return fmt.Errorf("failed to read %s: %w", dir, err)
+	}
+
+	for _, entry := range entries {
+		name := entry.Name()
+		if !strings.HasPrefix(name, ".") || !strings.Contains(name, tempNamePart) {
+			continue
+		}
+
+		path := filepath.Join(dir, name)
+
+		err = fs.RemoveAll(path)
+		if err != nil {
+			return fmt.Errorf("failed to remove %s: %w", path, err)
+		}
+
+		Debug("Removed what an interrupted command left", "path", path)
+	}
+
+	return nil
+}
+
 // WriteDir calls write to write the files of the new directory dir into a
 // temporary directory from TempDirFor, which is then renamed to dir, so that
 // neither a failure nor a crash leaves dir half-written; on a failure the

internal/vault/lock.go

@@ -1,6 +1,7 @@
 package vault
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -14,6 +15,11 @@ import (
 // lockFileName is the file in the state directory that LockStateDir locks.
 const lockFileName = "lock"
 
+// finishedMark is what the lock file holds once the command that last held
+// the lock has released it. A command killed while holding it leaves the
+// file empty.
+const finishedMark = "finished\n"
+
 // memFsLock stands in for the lock file on the in-memory filesystem, which
 // has no file locks. Every in-memory filesystem in the process shares it.
 //
@@ -25,6 +31,12 @@ var memFsLock sync.Mutex
 // it. While one command holds it, the next one waits here. Reads take no
 // lock: each file or directory a command changes is replaced in a single
 // rename, so a reader finds it as it was before or after, never half-made.
+// Once it holds the lock, it empties the lock file, and the function it
+// returns writes finishedMark there just before releasing the lock, so a
+// command killed while holding the lock leaves the mark missing. Finding it
+// missing, LockStateDir first deletes the temporary files and directories
+// such a command may have left, since no command still using them can be
+// running. After a command that finished, it searches nothing.
 //
 // On the real filesystem the lock is flock(2) on the file "lock" in
 // stateDir, which the kernel releases when the process dies, so a killed
@@ -32,16 +44,97 @@ var memFsLock sync.Mutex
 // use has no file locks, so a process-wide mutex stands in for flock there.
 // Any other filesystem is refused rather than left unlocked.
 func LockStateDir(fs afero.Fs, stateDir string) (func(), error) {
+	var release func()
+
 	switch fs.(type) {
 	case *afero.OsFs:
-		return flockStateDir(stateDir)
+		var err error
+
+		release, err = flockStateDir(stateDir)
+		if err != nil {
+			return nil, err
+		}
 	case *afero.MemMapFs:
 		memFsLock.Lock()
 
-		return memFsLock.Unlock, nil
+		release = memFsLock.Unlock
 	default:
 		return nil, fmt.Errorf("%w %T", ErrNoLockForFilesystem, fs)
 	}
+
+	// The lock file is written in place, never replaced: a command waiting
+	// for flock on the old file would then take a lock nobody else checks.
+	lockPath := filepath.Join(stateDir, lockFileName)
+
+	mark, err := afero.ReadFile(fs, lockPath)
+	if err != nil || string(mark) != finishedMark {
+		removeLeftovers(fs, stateDir)
+	}
+
+	err = afero.WriteFile(fs, lockPath, nil, secret.FilePerms)
+	if err != nil {
+		release()
+
+		return nil, fmt.Errorf("failed to empty lock file %s: %w", lockPath, err)
+	}
+
+	return func() {
+		// If this fails, the next command searches when it need not.
+		_ = afero.WriteFile(fs, lockPath, []byte(finishedMark), secret.FilePerms)
+
+		release()
+	}, nil
+}
+
+// removeLeftovers deletes the temporary files and directories that commands
+// killed part-way left in each directory where secret.WriteFileAtomic and
+// secret.TempDirFor make them: the state directory, each vault, each secret
+// and each version. Unlocker directories are written whole by
+// secret.WriteDir and never changed after, so they hold none. A failure is
+// only warned about, and the command goes on.
+func removeLeftovers(fs afero.Fs, stateDir string) {
+	dirs := []string{stateDir}
+
+	for _, vaultDir := range subdirs(fs, filepath.Join(stateDir, "vaults.d")) {
+		dirs = append(dirs, vaultDir)
+
+		for _, secretDir := range subdirs(fs, filepath.Join(vaultDir, "secrets.d")) {
+			dirs = append(dirs, secretDir)
+			dirs = append(dirs, subdirs(fs, filepath.Join(secretDir, "versions"))...)
+		}
+	}
+
+	for _, dir := range dirs {
+		err := secret.RemoveLeftovers(fs, dir)
+		if err != nil {
+			secret.Warn("Failed to remove what an interrupted command left",
+				"error", err)
+		}
+	}
+}
+
+// subdirs returns the directories in dir: none if dir does not exist, and
+// none, with a warning, if it cannot be read.
+func subdirs(fs afero.Fs, dir string) []string {
+	entries, err := afero.ReadDir(fs, dir)
+	if err != nil {
+		if !errors.Is(err, os.ErrNotExist) {
+			secret.Warn("Failed to look for what an interrupted command left",
+				"directory", dir, "error", err)
+		}
+
+		return nil
+	}
+
+	var dirs []string
+
+	for _, entry := range entries {
+		if entry.IsDir() {
+			dirs = append(dirs, filepath.Join(dir, entry.Name()))
+		}
+	}
+
+	return dirs
 }
 
 // flockStateDir takes flock(2) on the lock file in stateDir, creating the

internal/vault/lock_test.go

@@ -1,9 +1,11 @@
 package vault_test
 
 import (
+	"path/filepath"
 	"testing"
 	"time"
 
+	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -121,6 +123,51 @@ func TestLockStateDirFreeAfterPanic(t *testing.T) {
 	}
 }
 
+// TestLockStateDirRemovesLeftoversOnlyAfterKill checks that taking the lock
+// deletes a temporary directory a killed command left only when the last
+// holder of the lock did not release it. A holder killed while it holds the
+// lock leaves the lock file as it is at that moment.
+func TestLockStateDirRemovesLeftoversOnlyAfterKill(t *testing.T) {
+	t.Parallel()
+
+	for _, lfs := range lockFilesystems(t) {
+		t.Run(lfs.name, func(t *testing.T) {
+			t.Parallel()
+
+			lockFile := filepath.Join(lfs.stateDir, "lock")
+			leftover := filepath.Join(lfs.stateDir, ".tmp-1")
+
+			release, err := vault.LockStateDir(lfs.fs, lfs.stateDir)
+			require.NoError(t, err)
+
+			whileHeld, err := afero.ReadFile(lfs.fs, lockFile)
+			require.NoError(t, err)
+			release()
+
+			require.NoError(t, lfs.fs.MkdirAll(leftover, secret.DirPerms))
+
+			release, err = vault.LockStateDir(lfs.fs, lfs.stateDir)
+			require.NoError(t, err)
+			release()
+
+			exists, err := afero.DirExists(lfs.fs, leftover)
+			require.NoError(t, err)
+			assert.True(t, exists, "searched after a holder that finished")
+
+			require.NoError(t, afero.WriteFile(lfs.fs, lockFile, whileHeld,
+				secret.FilePerms))
+
+			release, err = vault.LockStateDir(lfs.fs, lfs.stateDir)
+			require.NoError(t, err)
+			release()
+
+			exists, err = afero.DirExists(lfs.fs, leftover)
+			require.NoError(t, err)
+			assert.False(t, exists, "not searched after a holder that was killed")
+		})
+	}
+}
+
 // TestLockStateDirRefusesOtherFilesystems checks that a filesystem with no
 // lock implementation is refused instead of being used unlocked.
 func TestLockStateDirRefusesOtherFilesystems(t *testing.T) {

internal/vault/unlockers.go

@@ -274,6 +274,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.