A wrong passphrase gives an error that says so (closes #157)

A passphrase that did not decrypt the passphrase unlocker failed with age's
"no identity matched any of the recipients" and advice to give the vault a
new unlocker from its mnemonic, which would replace an unlocker that works.
GetIdentity now turns that age error into "wrong passphrase for unlocker
<id>", wrapping the new secret.ErrWrongPassphrase, and withMnemonicAdvice
leaves it alone, as it does when no passphrase was read. A passphrase from
SB_UNLOCK_PASSPHRASE and a typed one both reach this code. A priv.age that
is not an age file keeps the advice.

Judgement call: age cannot tell a wrong passphrase from a damaged key inside
a priv.age whose header reads, so that case now reads as a wrong passphrase.

Model: opus-5-5

README.md

@@ -197,7 +197,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 +242,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,
@@ -372,6 +381,11 @@ which every command except `secret vault import` offers when the variable is not
 set, is the safer default; `secret vault import` has no prompt and needs both
 variables.
 
+The prompt asks on the terminal the command runs in, not on stdin, so it also
+asks when stdin is a pipe, as in `echo value | secret add name --force` or
+`cat file | secret encrypt key`. With no controlling terminal, as under cron or
+in a CI job, the command fails and names the variable to set.
+
 ## Security Features
 
 ### Encryption

TODO.md

@@ -18,6 +18,53 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
+- 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the
+  prompt, gives the error `wrong passphrase for unlocker <id>`
+  (https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no
+  identity matched any of the recipients", followed by advice to give the vault
+  a new unlocker from its mnemonic, which would have replaced an unlocker that
+  works. The error now wraps `secret.ErrWrongPassphrase` and carries no advice
+  to use the mnemonic. A `priv.age` that is not an age file still gets that
+  advice.
+- 2026-10-14: The passphrase and mnemonic prompts ask on the controlling
+  terminal (`/dev/tty`) instead of stdin, so `echo v | secret add s1 --force`,
+  `cat file | secret encrypt key` and `cat file.age | secret decrypt key` ask
+  for the passphrase at a terminal instead of failing
+  (https://git.eeqj.de/sneak/secret/issues/144). Only a process with no
+  controlling terminal gets the error that names the variable to set. On SIGINT
+  or SIGTERM, `Entry()` restores the settings of that terminal, not of stdin,
+  since that is where the prompt turns echo off. `secret init` and
+  `secret vault create` no longer write a newline to stderr after the mnemonic
+  is typed, since the prompt ends that line on the terminal.
+- 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/create_vault_test.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"slices"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/awnumar/memguard"
@@ -197,10 +198,10 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
 
 // TestMnemonicNotReadNamesOnlyMnemonic is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/115: `secret init` without
-// SB_SECRET_MNEMONIC and with a stdin that is not a terminal said "failed to
-// read mnemonic: failed to read passphrase: ...". The error must wrap
+// SB_SECRET_MNEMONIC and with no terminal said "failed to read mnemonic:
+// failed to read passphrase: ...". The error must wrap
 // secret.ErrMnemonicNotRead and name the mnemonic only. The message is
-// pinned on the built binary, whose stdin is surely not a terminal.
+// pinned on the built binary, run in a new session, which has no terminal.
 func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	t.Parallel()
 
@@ -216,14 +217,15 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 		"PATH=" + os.Getenv("PATH"),
 		"HOME=" + os.Getenv("HOME"),
 	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
 	require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+
-		"Error: failed to read mnemonic: stdin is not a terminal (piped input "+
-		"or script). Please set the SB_SECRET_MNEMONIC environment variable "+
-		"or run interactively\n", string(output))
+		"Error: failed to read mnemonic: there is no terminal to ask on. "+
+		"Please set the SB_SECRET_MNEMONIC environment variable or run "+
+		"interactively\n", string(output))
 }
 
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for

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, "")

internal/cli/init.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"log"
 	"log/slog"
-	"os"
 	"strings"
 
 	"github.com/awnumar/memguard"
@@ -57,13 +56,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
 	// Read mnemonic securely without echo
 	mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ")
 	if err != nil {
-		secret.Debug("Failed to read mnemonic from stdin", "error", err)
+		secret.Debug("Failed to read mnemonic", "error", err)
 
 		return nil, nil, err
 	}
 
-	fmt.Fprintln(os.Stderr) // Add newline after hidden input
-
 	return mnemonicBuffer, mnemonicBuffer.Destroy, nil
 }
 

internal/cli/passphrase_prompt_test.go

@@ -0,0 +1,204 @@
+package cli_test
+
+import (
+	"bufio"
+	"bytes"
+	"context"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+
+	"filippo.io/age"
+	"github.com/awnumar/memguard"
+	"github.com/creack/pty"
+	"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"
+)
+
+// TestPassphraseAskedAtTerminalWithPipedStdin is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/144: run at a terminal with stdin
+// a pipe, as in `echo v2 | secret add s1 --force`, a command that needed
+// the passphrase failed instead of asking for it. Each command here must ask
+// on the terminal and succeed once the passphrase is typed there.
+func TestPassphraseAskedAtTerminalWithPipedStdin(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+
+	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+	defer passphrase.Destroy()
+
+	vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
+		testMnemonicBuffer(t), passphrase)
+	require.NoError(t, err)
+
+	value := memguard.NewBufferFromBytes([]byte("v1"))
+	defer value.Destroy()
+
+	require.NoError(t, vlt.AddSecret("s1", value, false))
+
+	identity, err := age.GenerateX25519Identity()
+	require.NoError(t, err)
+
+	key := secret.IdentityToLockedBuffer(identity)
+	defer key.Destroy()
+
+	require.NoError(t, vlt.AddSecret("mykey", key, false))
+
+	runAtTerminal(t, stateDir, "v2\n", "add", "s1", "--force")
+
+	vlt.UnlockPassphrase = passphrase
+
+	added, err := vlt.GetSecret("s1")
+	require.NoError(t, err)
+
+	defer added.Destroy()
+
+	assert.Equal(t, "v2", added.String())
+
+	encrypted := runAtTerminal(t, stateDir, "a document\n", "encrypt", "mykey")
+	decrypted := runAtTerminal(t, stateDir, encrypted, "decrypt", "mykey")
+	assert.Equal(t, "a document\n", decrypted)
+}
+
+// TestInitWritesNoBlankLineAfterMnemonic runs `secret init 2>log` at a
+// terminal: the terminal is its controlling terminal and none of stdin,
+// stdout and stderr. The mnemonic is typed on the terminal and the prompt
+// ends its line there, so stderr must not get a newline for it as well, a
+// blank line in the log. pty.Open is described above
+// TestRemoveIgnoresTerminalOnStdout.
+func TestInitWritesNoBlankLineAfterMnemonic(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	stateDir := t.TempDir()
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), "init")
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		secret.EnvUnlockPassphrase + "=" + testPassphrase,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	deadline, _ := ctx.Deadline()
+	require.NoError(t, ptmx.SetReadDeadline(deadline))
+
+	var stderr strings.Builder
+
+	cmd.Stderr = &stderr
+	cmd.ExtraFiles = []*os.File{tty}
+	// Ctty is a descriptor in the new process: 3, the first of ExtraFiles.
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 3}
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	var (
+		shown []byte
+		char  byte
+	)
+
+	terminal := bufio.NewReader(ptmx)
+	for !bytes.HasSuffix(shown, []byte("Enter your BIP39 mnemonic phrase: ")) {
+		char, err = terminal.ReadByte()
+		require.NoError(t, err, "secret init did not ask on the terminal: %s",
+			shown)
+
+		shown = append(shown, char)
+	}
+
+	_, err = ptmx.WriteString(testMnemonic + "\n")
+	require.NoError(t, err)
+
+	// The read ends once the command has exited and so closed the terminal.
+	rest, _ := io.ReadAll(terminal)
+
+	err = cmd.Wait()
+	require.NoError(t, err, "secret init: %s%s%s", shown, rest, stderr.String())
+
+	assert.True(t, strings.HasPrefix(stderr.String(),
+		"Initialized secrets manager at: "+stateDir+"\n\n"+
+			"Default vault created and configured\n"),
+		"stderr: %q", stderr.String())
+}
+
+// runAtTerminal runs the secret binary with args and input on stdin, in a
+// new session whose controlling terminal is a new terminal, which is also
+// its stderr: a command typed at a terminal with its input piped. pty.Open
+// is described above TestRemoveIgnoresTerminalOnStdout. runAtTerminal waits
+// for the passphrase prompt on the terminal, types the passphrase there,
+// and returns what the command wrote to stdout once it has succeeded.
+func runAtTerminal(t *testing.T, stateDir, input string, args ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), args...)
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	deadline, _ := ctx.Deadline()
+	require.NoError(t, ptmx.SetReadDeadline(deadline))
+
+	var stdout strings.Builder
+
+	cmd.Stdin = strings.NewReader(input)
+	cmd.Stdout = &stdout
+	cmd.Stderr = tty
+	// Ctty is a descriptor in the new process: 2, its stderr.
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 2}
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	var (
+		shown []byte
+		char  byte
+	)
+
+	terminal := bufio.NewReader(ptmx)
+	for !bytes.HasSuffix(shown, []byte("Enter unlock passphrase: ")) {
+		char, err = terminal.ReadByte()
+		require.NoError(t, err, "secret %v did not ask on the terminal: %s",
+			args, shown)
+
+		shown = append(shown, char)
+	}
+
+	_, err = ptmx.WriteString(testPassphrase + "\n")
+	require.NoError(t, err)
+
+	// The read ends once the command has exited and so closed the terminal.
+	rest, _ := io.ReadAll(terminal)
+
+	require.NoError(t, cmd.Wait(), "secret %v: %s%s", args, shown, rest)
+
+	return stdout.String()
+}

internal/cli/root.go

@@ -15,16 +15,29 @@ import (
 // but exit with the code.
 func Entry() int {
 	// 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.
-	// Only a process in the terminal's foreground process group may reset
-	// it: one in the background that tries is stopped instead of exiting.
-	terminalState, terminalErr := term.GetState(unix.Stdin)
+	// and exits with status 1. The passphrase prompt turns echo off on the
+	// controlling terminal until the read finishes, so a signal there would
+	// leave echo off. Only a process in the terminal's foreground process
+	// group may reset it: one in the background that tries is stopped
+	// instead of exiting.
+	terminal, terminalErr := os.Open(secret.TerminalPath)
+
+	var terminalState *term.State
+
+	if terminalErr == nil {
+		defer func() { _ = terminal.Close() }()
+
+		terminalState, terminalErr = term.GetState(int(terminal.Fd()))
+	}
 
 	memguard.CatchSignal(func(os.Signal) {
-		foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP)
-		if terminalErr == nil && err == nil && foreground == unix.Getpgrp() {
-			_ = term.Restore(unix.Stdin, terminalState)
+		if terminalErr != nil {
+			return
+		}
+
+		foreground, err := unix.IoctlGetInt(int(terminal.Fd()), unix.TIOCGPGRP)
+		if err == nil && foreground == unix.Getpgrp() {
+			_ = term.Restore(int(terminal.Fd()), terminalState)
 		}
 	}, os.Interrupt, unix.SIGTERM)
 

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

@@ -1,12 +1,14 @@
 // Unlock Failure Tests
 //
 // When a vault cannot be opened through its current unlocker, because a
-// 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
-// missing, the error says how to make a version current again. Each test
-// that pins such advice also follows it.
+// file the unlocker needs is missing, 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. A wrong passphrase gives an error that says so, without that advice.
+// When the current unlocker does not exist and the vault has another that
+// `secret unlocker select` accepts, the 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
 
@@ -16,9 +18,9 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"syscall"
 	"testing"
 
-	"filippo.io/age"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
@@ -162,26 +164,48 @@ func TestUnlockFailureNamesMnemonic(t *testing.T) {
 	}
 }
 
-// TestWrongPassphraseNamesMnemonic checks the error of `secret get` given a
-// passphrase that does not decrypt the passphrase unlocker: it keeps age's
-// error and ends with the advice that the mnemonic still opens the vault.
-func TestWrongPassphraseNamesMnemonic(t *testing.T) {
+// TestWrongPassphraseNamesPassphrase checks the error of `secret get` given
+// a passphrase that does not decrypt the passphrase unlocker: it says that
+// the passphrase is wrong, without the advice to use the mnemonic, since the
+// unlocker still opens the vault with the right passphrase. The test then
+// gives the right passphrase.
+func TestWrongPassphraseNamesPassphrase(t *testing.T) {
 	t.Parallel()
 
-	_, c := newUnlockTestCLI(t, newTwoVaultFs(t))
+	unlockerDir, c := newUnlockTestCLI(t, newTwoVaultFs(t))
 	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte("wrong passphrase"))
 	t.Cleanup(c.UnlockPassphrase.Destroy)
 
 	err := c.GetSecret(discardCmd(), "x")
 
-	var noMatch *age.NoIdentityMatchError
-
-	require.ErrorAs(t, err, &noMatch)
-
+	require.ErrorIs(t, err, secret.ErrWrongPassphrase)
+	assert.NotContains(t, err.Error(), mnemonicAdvice)
 	require.EqualError(t, err, "failed to unlock vault: "+
 		"failed to get long-term key: failed to get unlocker identity: "+
-		"failed to decrypt unlocker private key: failed to create decryptor: "+
-		noMatch.Error()+mnemonicAdvice)
+		"wrong passphrase for unlocker "+filepath.Base(unlockerDir))
+
+	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(c.UnlockPassphrase.Destroy)
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
+// TestDamagedUnlockerNamesMnemonic checks the error of `secret get` when the
+// passphrase unlocker's priv.age is not an age file: the passphrase is not
+// blamed, and the error ends with the advice that the mnemonic still opens
+// the vault.
+func TestDamagedUnlockerNamesMnemonic(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	unlockerDir, c := newUnlockTestCLI(t, fs)
+
+	require.NoError(t, afero.WriteFile(fs, filepath.Join(unlockerDir, "priv.age"),
+		[]byte("damaged"), secret.FilePerms))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	require.NotErrorIs(t, err, secret.ErrWrongPassphrase)
+	require.ErrorContains(t, err, mnemonicAdvice)
 }
 
 // TestMoveUnlockFailureNamesVault checks the error of `secret move` into
@@ -222,8 +246,71 @@ 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
+// binary without SB_UNLOCK_PASSPHRASE and in a new session, which has no
 // terminal, so the passphrase cannot be read. The unlocker was not tried,
 // and adding one would need a passphrase read the same way, so the error
 // is the cause alone, without the advice to use the mnemonic.
@@ -254,15 +341,16 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
 		"PATH=" + os.Getenv("PATH"),
 		"HOME=" + os.Getenv("HOME"),
 	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
 	assert.Equal(t, "Error: failed to unlock vault: "+
 		"failed to get long-term key: failed to get unlocker identity: "+
-		"failed to read passphrase: stdin is not a terminal (piped input or "+
-		"script). Please set the SB_UNLOCK_PASSPHRASE environment variable or "+
-		"run interactively\n", string(output))
+		"failed to read passphrase: there is no terminal to ask on. Please "+
+		"set the SB_UNLOCK_PASSPHRASE environment variable or run "+
+		"interactively\n", string(output))
 }
 
 // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and

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 ` +
-			`mnemonic, vault data will be permanently inaccessible.`,
+			`is given. Removing the current unlocker makes another of the ` +
+			`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.")
-		cli.cmd.Println("Run 'secret unlocker add passphrase' to create one.")
+		_, _ = fmt.Fprintln(out, "No unlockers found in current vault.")
+		_, _ = 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")
-	cli.cmd.Printf("  %-40s %-12s %-20s %s\n",
+	_, _ = fmt.Fprintf(out, "  %-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/secret/constants.go

@@ -17,6 +17,10 @@ const (
 	// EnvGPGKeyID is the environment variable for providing the GPG key ID
 	EnvGPGKeyID = "SB_GPG_KEY_ID"
 
+	// TerminalPath opens the controlling terminal of the process that opens
+	// it, where the passphrase and mnemonic prompts ask.
+	TerminalPath = "/dev/tty"
+
 	// UnlockerTimeFormat is the layout of the time, in UTC, in the name of a
 	// new unlocker's directory. It runs to the nanosecond, so that every new
 	// unlocker, even one added right after another, gets a directory of its

internal/secret/crypto.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"syscall"
 	"unsafe"
 
 	"filippo.io/age"
@@ -16,11 +15,8 @@ import (
 
 var (
 	errNilPassphraseBuffer = errors.New("passphrase buffer is nil")
-	errStdinNotTerminal    = errors.New(
-		"stdin is not a terminal (piped input or script)")
-	errStderrNotTerminal = errors.New(
-		"stderr is not a terminal (running in non-interactive mode)")
-	errNothingEntered = errors.New("nothing was entered")
+	errNoTerminal          = errors.New("there is no terminal to ask on")
+	errNothingEntered      = errors.New("nothing was entered")
 )
 
 // ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no
@@ -193,38 +189,33 @@ func ReadMnemonic(prompt string) (*memguard.LockedBuffer, error) {
 	return readFromTerminal(prompt, ErrMnemonicNotRead, EnvMnemonic)
 }
 
-// readFromTerminal reads input from the terminal without echoing it. Every
-// error it returns wraps notRead; without a terminal, the error says to set
-// envVar instead.
+// readFromTerminal asks on the controlling terminal and reads the answer
+// there without echoing it, whatever stdin is: in
+// `echo value | secret add name --force`, stdin is a pipe. Every error it
+// returns wraps notRead; without a terminal, the error says to set envVar
+// instead.
 func readFromTerminal(
 	prompt string, notRead error, envVar string,
 ) (*memguard.LockedBuffer, error) {
-	// Check if stdin is a terminal
-	if !term.IsTerminal(syscall.Stdin) {
-		// Not a terminal - never read secrets from piped input
-		// for security reasons
+	terminal, err := os.OpenFile(TerminalPath, os.O_RDWR, 0)
+	if err != nil {
+		Debug("Failed to open the controlling terminal", "error", err)
+
 		return nil, fmt.Errorf(
 			"%w: %w. Please set the %s environment variable or run interactively",
-			notRead, errStdinNotTerminal, envVar)
+			notRead, errNoTerminal, envVar)
 	}
+	defer func() { _ = terminal.Close() }()
 
-	// stdin is a terminal, check if stderr is also a terminal for
-	// interactive prompting
-	if !term.IsTerminal(syscall.Stderr) {
-		return nil, fmt.Errorf("%w: %w. Please set the %s environment variable",
-			notRead, errStderrNotTerminal, envVar)
-	}
+	_, _ = fmt.Fprint(terminal, prompt)
 
-	// Both stdin and stderr are terminals - use secure password reading
-	fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout
-
-	input, err := term.ReadPassword(syscall.Stdin)
+	input, err := term.ReadPassword(int(terminal.Fd()))
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", notRead, err)
 	}
 
-	// Print newline to stderr since ReadPassword doesn't echo
-	fmt.Fprintln(os.Stderr)
+	// The newline typed after the input, which ReadPassword did not echo
+	_, _ = fmt.Fprintln(terminal)
 
 	if len(input) == 0 {
 		return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)

internal/secret/passphraseunlocker.go

@@ -16,6 +16,10 @@ import (
 // empty. A passphrase unlocker that fails with it was not tried.
 var ErrPassphraseNotRead = errors.New("failed to read passphrase")
 
+// ErrWrongPassphrase is wrapped in the error of GetIdentity when the
+// passphrase does not decrypt the unlocker's private key.
+var ErrWrongPassphrase = errors.New("wrong passphrase")
+
 // PassphraseUnlocker represents a passphrase-protected unlocker
 type PassphraseUnlocker struct {
 	Directory  string
@@ -73,6 +77,12 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) {
 		Debug("Failed to decrypt unlocker private key",
 			"error", err, "unlocker_id", p.GetID())
 
+		// age says that no identity matched when the passphrase is wrong
+		var noMatch *age.NoIdentityMatchError
+		if errors.As(err, &noMatch) {
+			return nil, fmt.Errorf("%w for unlocker %s", ErrWrongPassphrase, p.GetID())
+		}
+
 		return nil, fmt.Errorf("failed to decrypt unlocker private key: %w", err)
 	}
 	defer privKeyBuffer.Destroy()

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
-	return unlocker.Remove()
+	if err != nil {
+		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,14 @@ 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 passphrase is wrong: the unlocker still
+// opens the vault with the right one. 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, secret.ErrWrongPassphrase) ||
+		errors.Is(err, ErrUnlockerNotFound) {
 		return err
 	}
 
@@ -304,15 +309,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

@@ -103,3 +103,76 @@ 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)
+
+	require.ErrorIs(t, err, secret.ErrWrongPassphrase)
+	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
+}