A secret too large for the locked-memory limit fails with an error, not a panic (closes #145)

secret add, import and generate secret crashed with memguard's panic once a
value outgrew the locked-memory limit, and because the value was held twice,
half the usual 8 MiB limit was enough. secret now raises the limit to the hard
limit at startup. The value is read a page at a time into a memguard.Stream,
which keeps it encrypted, then copied once into locked memory after its size is
checked against 100 MiB and against the limit less 16 pages for the command's
keys. It comes out of the stream through Next, not Read: Read re-encrypts a
leftover piece, and memguard hangs if that runs out of locked memory.

Judgement call: with a limit of 64 KiB or less, nothing can be stored.

Model: opus-5-5

README.md

@@ -127,6 +127,17 @@ one.
 
 ### Secret Management
 
+#### Secret Size
+
+A secret's value is held in locked memory, which is never written to swap.
+`secret` raises its locked-memory limit (`ulimit -l`) to the hard limit when it
+starts, and `secret add`, `secret import` and `secret generate secret` refuse a
+value that does not fit in that limit with 16 memory pages to spare (64 KiB with
+4 KiB pages), with an error that names the limit. Linux's usual limit of 8 MiB
+allows a value of nearly 8 MiB; a larger one needs a higher hard limit, such as
+`memlock` in `/etc/security/limits.conf`. No value may be over 100 MiB, whatever
+the limit.
+
 #### `secret add <secret-name> [--force]`
 
 Adds a secret to the current vault. Reads the secret value from stdin.
@@ -197,7 +208,8 @@ for confirmation, naming the version, the secret and its vault (see
 
 #### `secret generate mnemonic`
 
-Generates a cryptographically secure BIP39 mnemonic phrase.
+Generates a cryptographically secure BIP39 mnemonic phrase and prints it to
+stdout. The warning to keep it safe goes to stderr.
 
 #### `secret generate secret <name> [--length=16] [--type=base58] [--force]`
 
@@ -241,6 +253,14 @@ 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.
+Removing the current unlocker makes the first of the vault's other unlockers
+that `secret unlocker select` accepts, in the order `secret unlocker list` shows
+them, the current one, and says which; when there is none, the vault is left
+without a current unlocker. `secret unlocker select` refuses an unlocker whose
+type is not `passphrase` or `pgp`. When `current-unlocker` names an unlocker
+that does not exist and the vault has another that `secret unlocker select`
+accepts, opening the vault fails with an error that says to select one with
+`secret unlocker select`.
 
 - `--force, -f`: Remove without asking, even the last unlocker
 - **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase,

TODO.md

@@ -18,6 +18,46 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
+- 2026-10-14: A secret too large for the locked-memory limit (`ulimit -l`) makes
+  `secret add`, `secret import` and `secret generate secret` fail with an error
+  that names the limit, not crash with a panic
+  (https://git.eeqj.de/sneak/secret/issues/145). memguard panics when it cannot
+  lock memory, and the value was held twice, so with the usual 8 MiB limit any
+  value over about 4 MiB crashed. `secret` now raises the limit to the hard
+  limit at startup. The value is read a page at a time into a `memguard.Stream`,
+  which keeps it encrypted, and copied once into locked memory only after its
+  size is checked against 100 MiB and against the limit less 16 pages for the
+  command's keys. `generate secret` checks the length before it generates
+  anything.
+- 2026-10-14: `secret generate mnemonic`, `secret vault list` and
+  `secret unlocker list` print their output to stdout, as `secret list` does, so
+  it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138).
+  They printed with cobra's `cmd.Println` and `cmd.Printf`, which write to
+  stderr when no output writer is set, so `secret vault list --json | jq`
+  received nothing. "No versions found" from `secret version list` moved to
+  stdout too. Messages such as "Created vault", and the warning after the
+  mnemonic, stay on stderr.
+- 2026-10-14: Removing the current unlocker no longer leaves the vault unable to
+  open through the unlockers it still has
+  (https://git.eeqj.de/sneak/secret/issues/137). `current-unlocker` went on
+  naming the removed unlocker, so every command that opened the vault failed,
+  with advice to use the mnemonic. `secret unlocker remove` of the current
+  unlocker now makes the first of the vault's other unlockers that
+  `secret unlocker select` accepts, in the order `secret unlocker list` shows,
+  current before removing it, and says so; when there is none, it also removes
+  `current-unlocker`. `vault.RemoveUnlocker` returns the ID of the unlocker it
+  made current. When `current-unlocker` names an unlocker that does not exist
+  and the vault has another that `secret unlocker select` accepts, the error
+  wraps `vault.ErrUnlockerNotFound` and says to run `secret unlocker select`,
+  without the advice to use the mnemonic.
+- 2026-10-14: A `secret add --force` that cannot unlock the vault, given a wrong
+  passphrase or mnemonic, leaves the secret as it was
+  (https://git.eeqj.de/sneak/secret/issues/136). The new version used to be
+  written before the vault was unlocked to set `notAfter` on the version it
+  replaces, so each failed add left one more version that `secret version list`
+  showed. The vault is now unlocked, and that version's metadata read, before
+  anything is written. `generate secret --force` and `import --force` take the
+  same path.
 - 2026-10-12: A mnemonic typed with extra spaces, tabs or line breaks gives the
   same long-term key as its words separated by single spaces
   (https://git.eeqj.de/sneak/secret/issues/135). The key was derived from the

internal/cli/generate.go

@@ -103,7 +103,7 @@ func (cli *Instance) GenerateMnemonic(cmd *cobra.Command) error {
 	}
 
 	// Output mnemonic to stdout
-	cmd.Println(mnemonic)
+	_, _ = fmt.Fprintln(cmd.OutOrStdout(), mnemonic)
 
 	// Output helpful information to stderr
 	fmt.Fprintln(os.Stderr, "")
@@ -133,10 +133,16 @@ func (cli *Instance) GenerateSecret(
 		return errLengthTooSmall
 	}
 
-	var (
+	if length > maxSecretSize {
-		secretValue string
+		return errSecretTooLarge
-		err         error
+	}
-	)
+
+	err := checkLockedMemoryLimit(length)
+	if err != nil {
+		return err
+	}
+
+	var secretValue string
 
 	switch secretType {
 	case "base58":

internal/cli/root.go

@@ -14,6 +14,8 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
+	raiseLockedMemoryLimit()
+
 	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
 	// and exits with status 1. The passphrase prompt turns terminal echo
 	// off until the read finishes, so a signal there would leave echo off.
@@ -38,6 +40,19 @@ func Entry() int {
 	return 0
 }
 
+// raiseLockedMemoryLimit raises this process's limit on locked memory, which
+// holds every secret value, to the hard limit, the most a process may set
+// without privileges. When that fails the limit stays as it was, and
+// checkLockedMemoryLimit names it in its error.
+func raiseLockedMemoryLimit() {
+	var limit unix.Rlimit
+
+	if unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit) == nil {
+		limit.Cur = limit.Max
+		_ = unix.Setrlimit(unix.RLIMIT_MEMLOCK, &limit)
+	}
+}
+
 func newRootCmd() *cobra.Command {
 	secret.Debug("newRootCmd starting")
 

internal/cli/secrets.go

@@ -14,6 +14,7 @@ import (
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
+	"golang.org/x/sys/unix"
 	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
 )
@@ -24,26 +25,28 @@ const (
 	// vaultSecretParts is the number of parts when splitting vault:secret
 	vaultSecretParts = 2
 
-	// initialBufferSize is the starting size for secret read buffers (4KB)
-	initialBufferSize = 4 * 1024
 	// maxSecretSize is the maximum allowed size of a secret (100MB)
 	maxSecretSize = 100 * 1024 * 1024
+
+	// lockedMemoryReservePages is the locked memory, in pages, that a command
+	// needs besides the value it stores, for its keys and its read buffer,
+	// with room to spare: memguard locks whole pages, at least one for each
+	// buffer, and `secret add` needs 7.
+	lockedMemoryReservePages = 16
+	// kibibyte is the unit of the locked-memory limit in `ulimit -l`.
+	kibibyte = 1024
 )
 
 // Sentinel errors for secret operations
 var (
-	errSecretTooLarge              = errors.New("secret too large: exceeds 100MB limit")
+	errSecretTooLarge    = errors.New("secret too large: exceeds 100MB limit")
+	errLockedMemoryLimit = errors.New(
+		"secret too large for the locked-memory limit")
 	errCrossVaultSourceUnqualified = errors.New(
 		"source must specify vault (e.g., vault:secret) for cross-vault move")
 	errMoveOntoItself = errors.New("cannot be moved onto itself")
 )
 
-// bufferInfo tracks a protected buffer and the number of bytes used in it
-type bufferInfo struct {
-	buffer *memguard.LockedBuffer
-	used   int
-}
-
 // ParseVaultSecretRef parses a "vault:secret" or just "secret" reference
 // Returns (vaultName, secretName, isQualified)
 // If no vault is specified, returns empty vaultName and isQualified=false
@@ -270,88 +273,100 @@ The source secret is deleted after successful copy.`,
 	return cmd
 }
 
-// updateBufferSize updates the buffer size based on usage pattern
+// checkLockedMemoryLimit returns an error naming the locked-memory limit
-func updateBufferSize(currentSize int, sameSize *int) int {
+// (`ulimit -l`) when a value of size bytes does not fit in it with
-	const (
+// lockedMemoryReservePages. memguard, which holds secret values in locked
-		doubleAfterBuffers = 2
+// memory, crashes the process when it cannot lock more, so this check comes
-		growthFactor       = 2
+// before a buffer for the value is allocated.
-	)
+func checkLockedMemoryLimit(size int) error {
+	var limit unix.Rlimit
 
-	*sameSize++
+	err := unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit)
-	if *sameSize >= doubleAfterBuffers {
+	if err != nil {
-		*sameSize = 0
+		return fmt.Errorf("failed to read the locked-memory limit: %w", err)
-
-		return currentSize * growthFactor
 	}
 
-	return currentSize
+	//nolint:gosec // G115: a size and a page count are never negative
-}
+	needed := uint64(size + lockedMemoryReservePages*os.Getpagesize())
-
+	if needed > limit.Cur {
-// destroyBuffers destroys every buffer in the list
+		return fmt.Errorf("%w of %d KiB (ulimit -l)",
-func destroyBuffers(buffers []bufferInfo) {
+			errLockedMemoryLimit, limit.Cur/kibibyte)
-	for _, b := range buffers {
-		b.buffer.Destroy()
 	}
+
+	return nil
 }
 
-// readSecretFromReader reads all data from reader into protected buffers,
+// readSecretFromReader reads all of reader into a new locked buffer, which
-// enforcing the maximum secret size. On failure the accumulated buffers
+// the caller must destroy, leaving out one trailing newline when trimNewline
-// are destroyed; on success the caller must destroy them.
+// is set. It reads a page at a time into a memguard.Stream, which keeps what
-func readSecretFromReader(reader io.Reader) ([]bufferInfo, int, error) {
+// it is given encrypted, so that the value is in locked memory only once, in
-	var buffers []bufferInfo
+// the buffer returned, and both size checks come before that buffer is
+// allocated. It takes the value back out with Stream.Next, not Stream.Read:
+// Read puts back the part of a piece that does not fit, and memguard hangs
+// when it runs out of locked memory doing that.
+func readSecretFromReader(
+	reader io.Reader, trimNewline bool,
+) (*memguard.LockedBuffer, error) {
+	stream := memguard.NewStream()
 
-	totalSize := 0
+	chunk := memguard.NewBuffer(os.Getpagesize())
-	currentBufferSize := initialBufferSize
+	defer chunk.Destroy()
-	sameSize := 0
+
+	size := 0
+	endsWithNewline := false
 
 	for {
-		// Create a new buffer
+		n, err := io.ReadFull(reader, chunk.Bytes())
-		buffer := memguard.NewBuffer(currentBufferSize)
+		if n > 0 {
-		n, err := io.ReadFull(reader, buffer.Bytes())
+			size += n
-
+			if size > maxSecretSize {
-		if n == 0 {
+				return nil, errSecretTooLarge
-			// No data read, destroy the unused buffer
-			buffer.Destroy()
-		} else {
-			buffers = append(buffers, bufferInfo{buffer: buffer, used: n})
-			totalSize += n
-
-			if totalSize > maxSecretSize {
-				destroyBuffers(buffers)
-
-				return nil, 0, errSecretTooLarge
 			}
 
-			// If we filled the buffer, consider growing for next iteration
+			endsWithNewline = chunk.Bytes()[n-1] == '\n'
-			if n == currentBufferSize {
+
-				currentBufferSize = updateBufferSize(currentBufferSize, &sameSize)
+			// Write encrypts the bytes and wipes them from chunk.
-			}
+			_, _ = stream.Write(chunk.Bytes()[:n])
 		}
 
-		if err == io.EOF || errors.Is(err, io.ErrUnexpectedEOF) {
+		if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
 			break
-		} else if err != nil {
+		}
-			destroyBuffers(buffers)
 
-			return nil, 0, err
+		if err != nil {
+			return nil, err
 		}
 	}
 
-	return buffers, totalSize, nil
+	// Freed before the value's buffer is allocated, so that the two are
-}
+	// never locked at once.
+	chunk.Destroy()
 
-// combineBuffers copies the used portions of buffers into a single
+	if trimNewline && endsWithNewline {
-// protected buffer of totalSize bytes
+		size--
-func combineBuffers(buffers []bufferInfo, totalSize int) *memguard.LockedBuffer {
-	valueBuffer := memguard.NewBuffer(totalSize)
-
-	offset := 0
-	for _, b := range buffers {
-		copy(valueBuffer.Bytes()[offset:], b.buffer.Bytes()[:b.used])
-		offset += b.used
 	}
 
-	return valueBuffer
+	err := checkLockedMemoryLimit(size)
+	if err != nil {
+		return nil, err
+	}
+
+	value := memguard.NewBuffer(size)
+
+	// copy stops at the value's size, so a trimmed newline is not copied.
+	for offset := 0; offset < size; {
+		decrypted, err := stream.Next()
+		if err != nil {
+			value.Destroy()
+
+			return nil, fmt.Errorf("failed to decrypt the value read: %w", err)
+		}
+
+		offset += copy(value.Bytes()[offset:], decrypted.Bytes())
+		decrypted.Destroy()
+	}
+
+	return value, nil
 }
 
 // AddSecret adds a secret to the current vault
@@ -370,35 +385,21 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 
 	secret.Debug("Got current vault", "vault_name", vlt.GetName())
 
-	// Read secret value directly into protected buffers
+	// Read the secret value, less a trailing newline, into locked memory
-	secret.Debug("Reading secret value from stdin into protected buffers")
+	secret.Debug("Reading secret value from stdin into a protected buffer")
 
-	buffers, totalSize, err := readSecretFromReader(cli.cmd.InOrStdin())
+	valueBuffer, err := readSecretFromReader(cli.cmd.InOrStdin(), true)
 	if err != nil {
-		if errors.Is(err, errSecretTooLarge) {
+		if errors.Is(err, errSecretTooLarge) || errors.Is(err, errLockedMemoryLimit) {
 			return err
 		}
 
 		return fmt.Errorf("failed to read secret value: %w", err)
 	}
-	defer destroyBuffers(buffers)
-
-	// Check for trailing newline in the last buffer
-	if len(buffers) > 0 && totalSize > 0 {
-		lastBuffer := &buffers[len(buffers)-1]
-		if lastBuffer.buffer.Bytes()[lastBuffer.used-1] == '\n' {
-			lastBuffer.used--
-			totalSize--
-		}
-	}
-
-	secret.Debug("Read secret value from stdin",
-		"value_length", totalSize, "buffers", len(buffers))
-
-	// Combine all buffers into a single protected buffer
-	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
+	secret.Debug("Read secret value from stdin", "value_length", valueBuffer.Size())
+
 	// Locked only now that stdin has been read: in `secret encrypt key |
 	// secret add name`, holding the lock while reading would leave each
 	// command waiting for the other.
@@ -653,7 +654,7 @@ func (cli *Instance) ImportSecret(
 
 	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 
-	// Read secret value from the source file into protected buffers
+	// Read secret value from the source file into a protected buffer
 	file, err := cli.fs.Open(sourceFile)
 	if err != nil {
 		return fmt.Errorf("failed to open file %s: %w", sourceFile, err)
@@ -665,14 +666,10 @@ func (cli *Instance) ImportSecret(
 		}
 	}()
 
-	buffers, totalSize, err := readSecretFromReader(file)
+	valueBuffer, err := readSecretFromReader(file, false)
 	if err != nil {
 		return fmt.Errorf("failed to read secret from file %s: %w", sourceFile, err)
 	}
-	defer destroyBuffers(buffers)
-
-	// Combine all buffers into a single protected buffer
-	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
 	// Locked only now that the file has been read, as in AddSecret: the

internal/cli/secrets_size_test.go

@@ -3,17 +3,23 @@ package cli
 
 import (
 	"bytes"
+	"context"
 	"crypto/rand"
 	"fmt"
 	"io"
+	"os"
+	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
 	"sneak.berlin/go/secret/pkg/agehd"
 )
@@ -315,6 +321,77 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 		"Retrieved secret should match original")
 }
 
+// TestSecretsAtLockedMemoryLimit runs the secret binary with the
+// locked-memory limit (ulimit -l) lowered. add must store the largest value
+// that fits, which takes holding it in locked memory only once. add, import
+// and generate secret must refuse a value one byte larger, and generate secret
+// one over 100 MB, with exit status 1 and an error saying why, where memguard
+// would crash the process with a panic.
+func TestSecretsAtLockedMemoryLimit(t *testing.T) {
+	t.Parallel()
+
+	const (
+		limitKiB    = 1024
+		commandWait = 10 * time.Second
+	)
+
+	fits := limitKiB*kibibyte - lockedMemoryReservePages*os.Getpagesize()
+	limitError := fmt.Sprintf("locked-memory limit of %d KiB", limitKiB)
+
+	stateDir := t.TempDir()
+	_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
+		testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
+	secretPath, err := filepath.Abs("../../secret")
+	require.NoError(t, err)
+
+	tooBigFile := filepath.Join(t.TempDir(), "too-big")
+	require.NoError(t, os.WriteFile(tooBigFile, make([]byte, fits+1), 0o600))
+
+	run := func(stdin []byte, args ...string) (string, error) {
+		ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+		defer cancel()
+
+		// The shell lowers the limit for itself and the binary only.
+		script := fmt.Sprintf(`ulimit -l %d && exec "$0" "$@"`, limitKiB)
+
+		//nolint:gosec // G204: runs the freshly built secret binary
+		cmd := exec.CommandContext(ctx, "sh",
+			append([]string{"-c", script, secretPath}, args...)...)
+		cmd.Env = []string{
+			secret.EnvStateDir + "=" + stateDir,
+			secret.EnvMnemonic + "=" + testMnemonic,
+		}
+		cmd.Stdin = bytes.NewReader(stdin)
+
+		output, err := cmd.CombinedOutput()
+
+		return string(output), err
+	}
+
+	output, err := run(make([]byte, fits), "add", "fits")
+	require.NoError(t, err, "the largest value that fits: %s", output)
+
+	refused := func(wantError string, stdin []byte, args ...string) {
+		output, err := run(stdin, args...)
+
+		var exitErr *exec.ExitError
+
+		require.ErrorAs(t, err, &exitErr, "%v: %s", args, output)
+		assert.Equal(t, 1, exitErr.ExitCode(), "%v: %s", args, output)
+		assert.Contains(t, output, wantError, args)
+		assert.NotContains(t, output, "panic", args)
+	}
+
+	refused(limitError, make([]byte, fits+1), "add", "too-big")
+	refused(limitError, nil, "import", "too-big", "--source", tooBigFile)
+	refused(limitError, nil, "generate", "secret", "too-big",
+		"--length", strconv.Itoa(fits+1))
+	refused(errSecretTooLarge.Error(), nil, "generate", "secret", "too-big",
+		"--length", strconv.Itoa(maxSecretSize+1))
+}
+
 // slowReader simulates a reader that returns data in small chunks
 type slowReader struct {
 	data      []byte

internal/cli/stdout_stderr_test.go

@@ -2,6 +2,7 @@ package cli_test
 
 import (
 	"bytes"
+	"encoding/json"
 	"os/exec"
 	"path/filepath"
 	"strings"
@@ -9,6 +10,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/bip39"
 	"sneak.berlin/go/secret/internal/secret"
 )
 
@@ -90,3 +92,66 @@ func TestGetCommandOutputsToStdout(t *testing.T) {
 	// Nothing should be in stderr
 	assert.Empty(t, stderr.String(), "stderr should be empty")
 }
+
+// TestMnemonicAndListingsOutputToStdout tests that 'secret generate
+// mnemonic', 'secret vault list' and 'secret unlocker list' print their
+// output to stdout, so that it can be piped
+func TestMnemonicAndListingsOutputToStdout(t *testing.T) {
+	t.Parallel()
+
+	tempDir := t.TempDir()
+	secretPath := secretBinaryPath(t)
+
+	// run runs the secret binary and returns what it wrote to stdout and to
+	// stderr
+	run := func(args ...string) (string, string) {
+		//nolint:gosec // G204: test executes the freshly built secret binary
+		cmd := exec.CommandContext(t.Context(), secretPath, args...)
+		cmd.Env = []string{
+			secret.EnvStateDir + "=" + tempDir,
+			secret.EnvMnemonic + "=" + testMnemonic,
+			secret.EnvUnlockPassphrase + "=test-passphrase",
+			"PATH=/usr/bin:/bin",
+		}
+
+		var stdout, stderr bytes.Buffer
+
+		cmd.Stdout = &stdout
+		cmd.Stderr = &stderr
+
+		err := cmd.Run()
+		require.NoError(t, err, "%v should succeed: %s", args, stderr.String())
+
+		return stdout.String(), stderr.String()
+	}
+
+	stdout, stderr := run("generate", "mnemonic")
+	assert.True(t, bip39.IsMnemonicValid(strings.TrimSpace(stdout)),
+		"stdout should hold the mnemonic, not %q", stdout)
+	assert.Contains(t, stderr, "Save this mnemonic phrase securely",
+		"the warning should be on stderr")
+
+	run("init")
+
+	stdout, _ = run("vault", "list", "--json")
+
+	var vaults map[string]any
+
+	require.NoError(t, json.Unmarshal([]byte(stdout), &vaults),
+		"stdout should hold the vaults as JSON, not %q", stdout)
+	assert.Equal(t, "default", vaults["currentVault"])
+
+	stdout, _ = run("unlocker", "list", "--json")
+
+	var unlockers map[string]any
+
+	require.NoError(t, json.Unmarshal([]byte(stdout), &unlockers),
+		"stdout should hold the unlockers as JSON, not %q", stdout)
+	assert.Len(t, unlockers["unlockers"], 1)
+
+	stdout, _ = run("vault", "list")
+	assert.Contains(t, stdout, "default (current)")
+
+	stdout, _ = run("unlocker", "list")
+	assert.Contains(t, stdout, "Total: 1 unlocker(s)")
+}

internal/cli/unlock_failure_test.go

@@ -4,9 +4,11 @@
 // file the unlocker needs is missing or the passphrase is wrong, the error
 // keeps its cause and ends by saying that the mnemonic still opens that
 // vault, but only for a vault that the mnemonic does open, and not when the
-// passphrase could not be read at all. When a secret's current file is
+// passphrase could not be read at all. When the current unlocker does not
-// missing, the error says how to make a version current again. Each test
+// exist and the vault has another that `secret unlocker select` accepts, the
-// that pins such advice also follows it.
+// error says to select one instead.
+// When a secret's current file is missing, the error says how to make a
+// version current again. Each test that pins such advice also follows it.
 
 package cli_test
 
@@ -222,6 +224,69 @@ func TestMoveUnlockFailureNamesVault(t *testing.T) {
 	assert.Equal(t, "value", getSecret(t, c, "y"))
 }
 
+// TestMissingCurrentUnlockerNamesSelect checks the error of `secret get`
+// when current-unlocker names an unlocker that does not exist and the vault
+// has another: it names the missing unlocker and says to select another
+// one, without the advice to use the mnemonic. The test then follows that
+// advice.
+func TestMissingCurrentUnlockerNamesSelect(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	unlockerDir, c := newUnlockTestCLI(t, fs)
+
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(unlockTestVaultDir, "current-unlocker"),
+		[]byte("missing"), secret.FilePerms))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	require.ErrorIs(t, err, vault.ErrUnlockerNotFound)
+	require.EqualError(t, err, "failed to unlock vault: "+
+		"failed to get long-term key: failed to get current unlocker: "+
+		"unlocker with ID missing not found; the vault 'default' has other "+
+		"unlockers: run 'secret unlocker select' with the ID of one of them "+
+		"to make it the current unlocker")
+
+	require.NoError(t, c.UnlockerSelect(filepath.Base(unlockerDir)))
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
+// TestMissingCurrentUnlockerNamesMnemonic checks the error of `secret get`
+// when the unlocker that current-unlocker names does not exist and the
+// vault's only other unlocker is the directory "odd", whose metadata names a
+// type that `secret unlocker select` refuses: the error ends with the advice
+// that the mnemonic still opens the vault, not with advice to select another
+// unlocker. The test then follows that advice.
+func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	unlockerDir, c := newUnlockTestCLI(t, fs)
+
+	require.NoError(t, fs.RemoveAll(unlockerDir))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(unlockTestVaultDir, "unlockers.d", "odd",
+			"unlocker-metadata.json"),
+		[]byte(`{"type": "odd"}`), secret.FilePerms))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	var cause *os.PathError
+
+	require.ErrorAs(t, err, &cause)
+	require.NotErrorIs(t, err, vault.ErrUnlockerNotFound)
+	require.EqualError(t, err, "failed to unlock vault: "+
+		"failed to get long-term key: failed to get current unlocker: "+
+		"failed to read unlocker metadata: "+cause.Error()+mnemonicAdvice)
+
+	c.Mnemonic = testMnemonicBuffer(t)
+	require.NoError(t, c.UnlockersAdd("passphrase", discardCmd()))
+
+	c.Mnemonic = nil
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
 // TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
 // binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a
 // terminal, so the passphrase cannot be read. The unlocker was not tried,

internal/cli/unlockers.go

@@ -222,8 +222,11 @@ func newUnlockerRemoveCmd() *cobra.Command {
 		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 ` +
+			`is given. Removing the current unlocker makes another of the ` +
-			`mnemonic, vault data will be permanently inaccessible.`,
+			`vault's unlockers current, if it has one that ` +
+			`'secret unlocker select' accepts. 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 {
@@ -323,22 +326,25 @@ func (cli *Instance) printUnlockersJSON(
 		return fmt.Errorf("failed to marshal JSON: %w", err)
 	}
 
-	cli.cmd.Println(string(jsonBytes))
+	_, _ = fmt.Fprintln(cli.cmd.OutOrStdout(), string(jsonBytes))
 
 	return nil
 }
 
 // printUnlockersTable prints unlockers in a formatted table
 func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
+	out := cli.cmd.OutOrStdout()
+
 	if len(unlockers) == 0 {
-		cli.cmd.Println("No unlockers found in current vault.")
+		_, _ = fmt.Fprintln(out, "No unlockers found in current vault.")
-		cli.cmd.Println("Run 'secret unlocker add passphrase' to create one.")
+		_, _ = fmt.Fprintln(out, "Run 'secret unlocker add passphrase' to create one.")
 
 		return nil
 	}
 
-	cli.cmd.Printf("  %-40s %-12s %-20s %s\n", "UNLOCKER ID", "TYPE", "CREATED", "FLAGS")
+	_, _ = fmt.Fprintf(out, "  %-40s %-12s %-20s %s\n",
-	cli.cmd.Printf("  %-40s %-12s %-20s %s\n",
+		"UNLOCKER ID", "TYPE", "CREATED", "FLAGS")
+	_, _ = fmt.Fprintf(out, "  %-40s %-12s %-20s %s\n",
 		strings.Repeat("-", unlockerIDWidth), strings.Repeat("-", unlockerTypeWidth),
 		strings.Repeat("-", unlockerDateWidth), strings.Repeat("-", unlockerFlagsWidth))
 
@@ -353,7 +359,7 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
 			prefix = "* "
 		}
 
-		cli.cmd.Printf("%s%-40s %-12s %-20s %s\n",
+		_, _ = fmt.Fprintf(out, "%s%-40s %-12s %-20s %s\n",
 			prefix,
 			unlocker.ID,
 			unlocker.Type,
@@ -361,7 +367,7 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
 			flags)
 	}
 
-	cli.cmd.Printf("\nTotal: %d unlocker(s)\n", len(unlockers))
+	_, _ = fmt.Fprintf(out, "\nTotal: %d unlocker(s)\n", len(unlockers))
 
 	return nil
 }
@@ -612,13 +618,17 @@ func (cli *Instance) removeUnlocker(
 			"have your mnemonic phrase to access this vault again!")
 	}
 
-	err := found.vlt.RemoveUnlocker(unlockerID)
+	newCurrentID, err := found.vlt.RemoveUnlocker(unlockerID)
 	if err != nil {
 		return err
 	}
 
 	cmd.Printf("Removed unlocker '%s'\n", unlockerID)
 
+	if newCurrentID != "" {
+		cmd.Printf("Switched current unlocker to '%s'\n", newCurrentID)
+	}
+
 	return nil
 }
 

internal/cli/unlockers_remove_test.go

@@ -0,0 +1,94 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"bytes"
+	"path/filepath"
+	"testing"
+
+	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/secret"
+	"sneak.berlin/go/secret/internal/vault"
+)
+
+// TestUnlockerRemoveCurrent is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/137. A vault has a passphrase
+// unlocker and a PGP unlocker, the current one. Removing the PGP unlocker
+// makes the passphrase unlocker current and says so, and the vault then
+// opens through it. Removing the passphrase unlocker too removes
+// current-unlocker.
+//
+// The second case also gives the vault the unlocker directory "odd", whose
+// metadata names a type that `secret unlocker select` refuses, and which
+// sorts before the passphrase unlocker: neither removal makes it current.
+//
+//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
+func TestUnlockerRemoveCurrent(t *testing.T) {
+	tests := []struct {
+		name    string
+		withOdd bool
+	}{
+		{name: "two unlockers", withOdd: false},
+		{name: "and the directory odd", withOdd: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			newTestGPGKey(t)
+
+			passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+			t.Cleanup(passphrase.Destroy)
+
+			fs := afero.NewMemMapFs()
+			vlt, err := vault.CreateVault(fs, listTestStateDir,
+				listTestVaultName, testMnemonicBuffer(t), nil)
+			require.NoError(t, err)
+
+			passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphrase)
+			require.NoError(t, err)
+
+			if tt.withOdd {
+				require.NoError(t, afero.WriteFile(fs,
+					filepath.Join(testVaultDir(listTestVaultName), "unlockers.d",
+						"odd", "unlocker-metadata.json"),
+					[]byte(`{"type": "odd"}`), secret.FilePerms))
+			}
+
+			instance, cmd := newTestInstance(fs)
+			instance.UnlockPassphrase = passphrase
+
+			cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
+			require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd))
+
+			pgpUnlocker, err := vlt.GetCurrentUnlocker()
+			require.NoError(t, err)
+			require.Equal(t, unlockerTypePGP, pgpUnlocker.GetType())
+
+			var output bytes.Buffer
+
+			cmd.SetOut(&output)
+			require.NoError(t,
+				instance.UnlockersRemove(pgpUnlocker.GetID(), true, cmd))
+			assert.Contains(t, output.String(),
+				"Switched current unlocker to '"+passphraseUnlocker.GetID()+"'")
+
+			reopened := vault.NewVault(fs, listTestStateDir, listTestVaultName)
+			reopened.UnlockPassphrase = passphrase
+
+			_, err = reopened.GetOrDeriveLongTermKey()
+			require.NoError(t, err)
+
+			require.NoError(t,
+				instance.UnlockersRemove(passphraseUnlocker.GetID(), true, cmd))
+
+			exists, err := afero.Exists(fs, filepath.Join(
+				testVaultDir(listTestVaultName), "current-unlocker"))
+			require.NoError(t, err)
+			assert.False(t, exists,
+				"current-unlocker is left after the passphrase unlocker")
+		})
+	}
+}

internal/cli/vault.go

@@ -186,6 +186,8 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
 		return err
 	}
 
+	out := cmd.OutOrStdout()
+
 	if jsonOutput { //nolint:nestif // Separate JSON and text output formatting logic
 		// Get current vault name for context
 		currentVault := ""
@@ -205,13 +207,13 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
 			return err
 		}
 
-		cmd.Println(string(jsonBytes))
+		_, _ = fmt.Fprintln(out, string(jsonBytes))
 	} else {
 		// Text output
-		cmd.Println("Available vaults:")
+		_, _ = fmt.Fprintln(out, "Available vaults:")
 
 		if len(vaults) == 0 {
-			cmd.Println("  (none)")
+			_, _ = fmt.Fprintln(out, "  (none)")
 		} else {
 			// Try to get current vault for marking
 			currentVault := ""
@@ -223,9 +225,9 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
 
 			for _, vaultName := range vaults {
 				if vaultName == currentVault {
-					cmd.Printf("  %s (current)\n", vaultName)
+					_, _ = fmt.Fprintf(out, "  %s (current)\n", vaultName)
 				} else {
-					cmd.Printf("  %s\n", vaultName)
+					_, _ = fmt.Fprintf(out, "  %s\n", vaultName)
 				}
 			}
 		}

internal/cli/version.go

@@ -167,7 +167,7 @@ func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
 	}
 
 	if len(versions) == 0 {
-		cmd.Println("No versions found")
+		_, _ = fmt.Fprintln(cmd.OutOrStdout(), "No versions found")
 
 		return nil
 	}

internal/vault/errors_test.go

@@ -81,7 +81,9 @@ func TestVaultErrors(t *testing.T) {
 			return vlt.CopySecretAllVersions(vlt, versionless, "copy", false)
 		}, vault.ErrNoVersions},
 		{"remove a missing unlocker", func(vlt *vault.Vault) error {
-			return vlt.RemoveUnlocker(missingName)
+			_, err := vlt.RemoveUnlocker(missingName)
+
+			return err
 		}, vault.ErrUnlockerNotFound},
 		{"select a missing unlocker", func(vlt *vault.Vault) error {
 			return vlt.SelectUnlocker(missingName)

internal/vault/secrets.go

@@ -219,6 +219,15 @@ func (v *Vault) addVersion(
 	name, secretDir string, value *memguard.LockedBuffer,
 	previousVersion *secret.Version,
 ) error {
+	// Unlock the vault before writing anything, so that a wrong passphrase
+	// or mnemonic leaves the secret as it was
+	err := v.loadPreviousVersionMetadata(previousVersion)
+	if err != nil {
+		return err
+	}
+
+	// Taken after the unlock, which can wait for the user to type the
+	// passphrase, so that the new version starts when it is created
 	now := time.Now()
 
 	// Create the new version and save the encrypted value
@@ -632,8 +641,35 @@ func (v *Vault) checkExistingSecret(
 	return true, previousVersion, nil
 }
 
+// loadPreviousVersionMetadata unlocks the vault and loads the metadata of
+// the version being superseded. It is a no-op when previousVersion is nil.
+func (v *Vault) loadPreviousVersionMetadata(
+	previousVersion *secret.Version,
+) error {
+	if previousVersion == nil {
+		return nil
+	}
+
+	ltIdentity, err := v.GetOrDeriveLongTermKey()
+	if err != nil {
+		secret.Debug("Failed to get long-term key for previous version", "error", err)
+
+		return fmt.Errorf("failed to get long-term key: %w", err)
+	}
+
+	err = previousVersion.LoadMetadata(ltIdentity)
+	if err != nil {
+		secret.Debug("Failed to load previous version metadata", "error", err)
+
+		return fmt.Errorf("failed to load previous version metadata: %w", err)
+	}
+
+	return nil
+}
+
 // updatePreviousVersion sets the notAfter timestamp on the version being
-// superseded. It is a no-op when previousVersion is nil.
+// superseded, whose metadata loadPreviousVersionMetadata has loaded. It is
+// a no-op when previousVersion is nil.
 func (v *Vault) updatePreviousVersion(
 	previousVersion *secret.Version, now *time.Time,
 ) error {
@@ -641,22 +677,13 @@ func (v *Vault) updatePreviousVersion(
 		return nil
 	}
 
-	// Get long-term key to decrypt/encrypt metadata
+	// loadPreviousVersionMetadata unlocked the vault, so this returns the
+	// key it keeps in memory
 	ltIdentity, err := v.GetOrDeriveLongTermKey()
 	if err != nil {
-		secret.Debug("Failed to get long-term key for metadata update", "error", err)
-
 		return fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	// Load previous version metadata
-	err = previousVersion.LoadMetadata(ltIdentity)
-	if err != nil {
-		secret.Debug("Failed to load previous version metadata", "error", err)
-
-		return fmt.Errorf("failed to load previous version metadata: %w", err)
-	}
-
 	// Update notAfter timestamp
 	previousVersion.Metadata.NotAfter = now
 

internal/vault/secrets_version_test.go

@@ -10,6 +10,8 @@
 //   version
 // - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter)
 //   across versions
+// - TestVaultVersionTimestampsAfterUnlock: Tests that those timestamps are
+//   taken after AddSecret unlocks the vault
 // - TestVaultGetNonExistentVersion: Tests error handling for invalid versions
 // - TestUpdateVersionMetadata: Tests metadata update functionality
 //
@@ -295,6 +297,70 @@ func TestVaultVersionTimestamps(t *testing.T) {
 	assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version
 }
 
+// longTermKeyReadFs records when longterm.age, the last file an unlock
+// reads, was last opened.
+type longTermKeyReadFs struct {
+	afero.Fs
+
+	readAt time.Time
+}
+
+//nolint:ireturn // afero.File is the interface required by afero.Fs
+func (f *longTermKeyReadFs) Open(name string) (afero.File, error) {
+	if filepath.Base(name) == "longterm.age" {
+		f.readAt = time.Now()
+	}
+
+	return f.Fs.Open(name)
+}
+
+// TestVaultVersionTimestampsAfterUnlock checks that when AddSecret has to
+// unlock the vault to replace a version, the new version's notBefore and the
+// replaced version's notAfter are taken after the unlock, which can wait for
+// the user to type the passphrase.
+func TestVaultVersionTimestampsAfterUnlock(t *testing.T) {
+	t.Parallel()
+
+	fs := &longTermKeyReadFs{Fs: afero.NewMemMapFs()}
+
+	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
+	defer mnemonic.Destroy()
+
+	passphrase := memguard.NewBufferFromBytes([]byte("test-passphrase"))
+	defer passphrase.Destroy()
+
+	created, err := CreateVault(fs, testStateDir, "test", mnemonic, passphrase)
+	require.NoError(t, err)
+
+	addTestSecretToVault(t, created, testSecretPath, []byte("version-1"), false)
+
+	// A new instance is locked; AddSecret unlocks it with the passphrase
+	vault := NewVault(fs, testStateDir, "test")
+	vault.SetUnlockPassphrase(passphrase)
+
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
+	require.False(t, fs.readAt.IsZero(), "AddSecret did not unlock the vault")
+
+	ltIdentity, err := vault.GetOrDeriveLongTermKey()
+	require.NoError(t, err)
+
+	vaultDir, _ := vault.GetDirectory()
+	secretDir := filepath.Join(vaultDir, "secrets.d", "test%secret")
+
+	versions, err := secret.ListVersions(fs, secretDir)
+	require.NoError(t, err)
+	require.Len(t, versions, 2)
+
+	newVersion := secret.NewVersion(vault, testSecretPath, versions[0])
+	require.NoError(t, newVersion.LoadMetadata(ltIdentity))
+
+	replacedVersion := secret.NewVersion(vault, testSecretPath, versions[1])
+	require.NoError(t, replacedVersion.LoadMetadata(ltIdentity))
+
+	assert.False(t, newVersion.Metadata.NotBefore.Before(fs.readAt))
+	assert.False(t, replacedVersion.Metadata.NotAfter.Before(fs.readAt))
+}
+
 func TestVaultGetNonExistentVersion(t *testing.T) {
 	t.Parallel()
 

internal/vault/unlockers.go

@@ -5,8 +5,10 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -19,6 +21,7 @@ import (
 // Unlocker metadata type strings.
 const (
 	unlockerTypePassphrase = "passphrase"
+	unlockerTypePGP        = "pgp"
 )
 
 // GetCurrentUnlocker returns the current unlocker for this vault
@@ -52,6 +55,11 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
 		return nil, err
 	}
 
+	err = v.checkCurrentUnlockerExists(unlockerDir)
+	if err != nil {
+		return nil, err
+	}
+
 	secret.DebugWith("Resolved unlocker directory",
 		slog.String("unlocker_dir", unlockerDir),
 		slog.String("vault_name", v.Name),
@@ -74,7 +82,7 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
 		passphraseUnlocker := secret.NewPassphraseUnlocker(v.fs, unlockerDir, metadata)
 		passphraseUnlocker.Passphrase = v.UnlockPassphrase
 		unlocker = passphraseUnlocker
-	case "pgp":
+	case unlockerTypePGP:
 		secret.Debug("Creating PGP unlocker instance", "unlocker_type", metadata.Type)
 
 		unlocker = secret.NewPGPUnlocker(v.fs, unlockerDir, metadata)
@@ -119,6 +127,37 @@ func (v *Vault) resolveUnlockerDirectory(currentUnlockerPath string) (string, er
 	return absolutePath, nil
 }
 
+// checkCurrentUnlockerExists fails with ErrUnlockerNotFound, saying to
+// select another unlocker, when unlockerDir, the unlocker directory that
+// current-unlocker names, does not exist and the vault has another unlocker
+// that SelectUnlocker accepts. When it has none, it returns nil: reading the
+// missing unlocker then fails, with the advice that the mnemonic still opens
+// the vault.
+func (v *Vault) checkCurrentUnlockerExists(unlockerDir string) error {
+	exists, err := afero.DirExists(v.fs, unlockerDir)
+	if err != nil {
+		return fmt.Errorf("failed to check for the current unlocker: %w", err)
+	}
+
+	if exists {
+		return nil
+	}
+
+	otherID, err := v.firstOtherUnlockerID(filepath.Base(unlockerDir))
+	if err != nil {
+		return err
+	}
+
+	if otherID == "" {
+		return nil
+	}
+
+	return fmt.Errorf("unlocker with ID %s %w; the vault '%s' has other "+
+		"unlockers: run %s with the ID of one of them to make it the current "+
+		"unlocker", filepath.Base(unlockerDir), ErrUnlockerNotFound, v.Name,
+		v.withVaultSelect("'secret unlocker select'"))
+}
+
 // findUnlockerByID finds an unlocker by its ID and returns the unlocker
 // instance and its directory path. A directory that ListUnlockers skips is
 // skipped here too, with the same warning. Such a directory has no ID: if
@@ -159,7 +198,7 @@ func (v *Vault) findUnlockerByID(
 		switch metadata.Type {
 		case unlockerTypePassphrase:
 			tempUnlocker = secret.NewPassphraseUnlocker(v.fs, unlockerDirPath, metadata)
-		case "pgp":
+		case unlockerTypePGP:
 			tempUnlocker = secret.NewPGPUnlocker(v.fs, unlockerDirPath, metadata)
 		default:
 			continue
@@ -282,10 +321,15 @@ func (v *Vault) HasUnlocker(unlockerID string) (bool, error) {
 // 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.
-func (v *Vault) RemoveUnlocker(unlockerID string) error {
+//
+// It returns the ID of the unlocker it made current, or "" when it made none
+// current. If the removed unlocker is the current one, the unlocker
+// firstOtherUnlockerID returns is made current before the removal; with
+// none, current-unlocker is removed after it.
+func (v *Vault) RemoveUnlocker(unlockerID string) (string, error) {
 	vaultDir, err := v.GetDirectory()
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	// Find the unlocker directory and create the unlocker instance
@@ -294,19 +338,92 @@ func (v *Vault) RemoveUnlocker(unlockerID string) error {
 	// Find the unlocker by ID
 	unlocker, unlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	if unlockerDir == "" {
-		return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
+		return "", fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
+	}
+
+	currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
+
+	currentDir, err := v.resolveUnlockerDirectory(currentUnlockerPath)
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return "", err
+	}
+
+	isCurrent := currentDir == unlockerDir
+	newCurrentID := ""
+
+	if isCurrent {
+		newCurrentID, err = v.selectOtherUnlocker(unlockerID)
+		if err != nil {
+			return "", err
+		}
 	}
 
 	if unlocker == nil {
-		return secret.RemoveDirAtomic(v.fs, unlockerDir)
+		err = secret.RemoveDirAtomic(v.fs, unlockerDir)
+	} else {
+		err = unlocker.Remove()
 	}
 
-	// Use the unlocker's Remove method
+	if err != nil {
-	return unlocker.Remove()
+		return "", err
+	}
+
+	if isCurrent && newCurrentID == "" {
+		err = v.fs.Remove(currentUnlockerPath)
+		if err != nil {
+			return "", fmt.Errorf("failed to remove current-unlocker file: %w", err)
+		}
+	}
+
+	return newCurrentID, nil
+}
+
+// selectOtherUnlocker makes the unlocker firstOtherUnlockerID returns for
+// unlockerID the current unlocker and returns its ID. It returns "" when
+// there is no such unlocker.
+func (v *Vault) selectOtherUnlocker(unlockerID string) (string, error) {
+	otherID, err := v.firstOtherUnlockerID(unlockerID)
+	if err != nil {
+		return "", err
+	}
+
+	if otherID == "" {
+		return "", nil
+	}
+
+	err = v.SelectUnlocker(otherID)
+	if err != nil {
+		return "", err
+	}
+
+	return otherID, nil
+}
+
+// firstOtherUnlockerID returns the ID of the vault's first unlocker other
+// than unlockerID, in ID order, the order `secret unlocker list` shows, that
+// SelectUnlocker accepts, or "" when there is none. SelectUnlocker refuses
+// an unlocker whose metadata names a type other than passphrase or pgp,
+// which ListUnlockers still returns.
+func (v *Vault) firstOtherUnlockerID(unlockerID string) (string, error) {
+	unlockers, err := v.ListUnlockers()
+	if err != nil {
+		return "", err
+	}
+
+	for _, id := range slices.Sorted(maps.Keys(unlockers)) {
+		unlockerType := unlockers[id].Type
+
+		if id != unlockerID && (unlockerType == unlockerTypePassphrase ||
+			unlockerType == unlockerTypePGP) {
+			return id, nil
+		}
+	}
+
+	return "", nil
 }
 
 // SelectUnlocker selects an unlocker as current for this vault

internal/vault/vault.go

@@ -291,9 +291,11 @@ func (v *Vault) unlockLongTermKey(
 // without a mnemonic records none, and without its metadata the key cannot
 // be derived. It is not added when the passphrase could not be read: the
 // unlocker was not tried, and adding one would need a passphrase read the
-// same way.
+// same way. Nor is it added when the current unlocker does not exist and the
+// vault has others: the error already says to select one of them.
 func (v *Vault) withMnemonicAdvice(err error) error {
-	if errors.Is(err, secret.ErrPassphraseNotRead) {
+	if errors.Is(err, secret.ErrPassphraseNotRead) ||
+		errors.Is(err, ErrUnlockerNotFound) {
 		return err
 	}
 
@@ -304,15 +306,21 @@ func (v *Vault) withMnemonicAdvice(err error) error {
 		return err
 	}
 
-	// 'secret unlocker add' acts on the current vault only.
-	steps := "'secret unlocker add passphrase'"
-
-	current, currentErr := GetCurrentVault(v.fs, v.stateDir)
-	if currentErr != nil || current.Name != v.Name {
-		steps = fmt.Sprintf("'secret vault select %s', then %s", v.Name, steps)
-	}
-
 	return fmt.Errorf("%w; the vault '%s' still opens with its mnemonic: run "+
 		"%s with %s set to the mnemonic to give it a new unlocker",
-		err, v.Name, steps, secret.EnvMnemonic)
+		err, v.Name, v.withVaultSelect("'secret unlocker add passphrase'"),
+		secret.EnvMnemonic)
+}
+
+// withVaultSelect returns command, a quoted 'secret unlocker' command, for
+// advice about this vault. Such a command acts on the current vault only,
+// so when this vault is not the current one, the command that selects it
+// comes first.
+func (v *Vault) withVaultSelect(command string) string {
+	current, err := GetCurrentVault(v.fs, v.stateDir)
+	if err != nil || current.Name != v.Name {
+		return fmt.Sprintf("'secret vault select %s', then %s", v.Name, command)
+	}
+
+	return command
 }

internal/vault/vault_error_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"filippo.io/age"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -103,3 +104,78 @@ func TestAddSecretCleansUpOnFailure(t *testing.T) {
 	require.Len(t, entries, 1)
 	assert.Equal(t, "pub.age", entries[0].Name())
 }
+
+// TestAddSecretForceWithWrongPassphraseChangesNothing checks that adding a
+// version to an existing secret, with a passphrase that does not open the
+// vault, fails and leaves the secret's directory as it was.
+func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+
+	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(passphrase.Destroy)
+
+	created, err := vault.CreateVault(fs, testStateDir, testVaultName,
+		testMnemonicBuffer(t), passphrase)
+	require.NoError(t, err)
+
+	first := memguard.NewBufferFromBytes([]byte("first"))
+	t.Cleanup(first.Destroy)
+
+	require.NoError(t, created.AddSecret(testSecretName, first, false))
+
+	wrong := memguard.NewBufferFromBytes([]byte("wrong passphrase"))
+	t.Cleanup(wrong.Destroy)
+
+	vlt := vault.NewVault(fs, testStateDir, testVaultName)
+	vlt.SetUnlockPassphrase(wrong)
+
+	secretDir := filepath.Join(testStateDir, "vaults.d", testVaultName,
+		"secrets.d", testSecretName)
+	before := dirContents(t, fs, secretDir)
+
+	second := memguard.NewBufferFromBytes([]byte("second"))
+	t.Cleanup(second.Destroy)
+
+	err = vlt.AddSecret(testSecretName, second, true)
+
+	var noMatch *age.NoIdentityMatchError
+
+	require.ErrorAs(t, err, &noMatch)
+	assert.Equal(t, before, dirContents(t, fs, secretDir))
+}
+
+// dirContents maps every file under dir to its contents, and every
+// directory, written with a trailing "/", to "".
+func dirContents(t *testing.T, fs afero.Fs, dir string) map[string]string {
+	t.Helper()
+
+	contents := map[string]string{}
+
+	err := afero.Walk(fs, dir, func(
+		path string, info os.FileInfo, err error,
+	) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			contents[path+"/"] = ""
+
+			return nil
+		}
+
+		content, err := afero.ReadFile(fs, path)
+		if err != nil {
+			return err
+		}
+
+		contents[path] = string(content)
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	return contents
+}