Use gpg.conf's default-key when unlocker add pgp has no --keyid (closes #141)

getDefaultGPGKey took the default-key value from `gpgconf --list-options
gpg` as printed. gpgconf prints a string value with a leading `"` and
percent-escaped, so gpg rejected the key ID. The parsing moved into
parseDefaultGPGKey, which drops the `"` and percent-decodes the rest with
url.PathUnescape, which leaves `+` alone. A value that does not decode is
an error, not a fallback to the first secret key.

Rule suppressed: gosec G703 on the test's write of gpg.conf into the
GNUPGHOME directory newTestGPGKey made.

Model: opus-5-5

README.md

@@ -381,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,24 @@ 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).

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
+// SB_SECRET_MNEMONIC and with no terminal said "failed to read mnemonic:
-// read mnemonic: failed to read passphrase: ...". The error must wrap
+// 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 "+
+		"Error: failed to read mnemonic: there is no terminal to ask on. "+
-		"or script). Please set the SB_SECRET_MNEMONIC environment variable "+
+		"Please set the SB_SECRET_MNEMONIC environment variable or run "+
-		"or run interactively\n", string(output))
+		"interactively\n", string(output))
 }
 
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for

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
+	// and exits with status 1. The passphrase prompt turns echo off on the
-	// off until the read finishes, so a signal there would leave echo off.
+	// controlling terminal until the read finishes, so a signal there would
-	// Only a process in the terminal's foreground process group may reset
+	// leave echo off. Only a process in the terminal's foreground process
-	// it: one in the background that tries is stopped instead of exiting.
+	// group may reset it: one in the background that tries is stopped
-	terminalState, terminalErr := term.GetState(unix.Stdin)
+	// 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 {
-		if terminalErr == nil && err == nil && foreground == unix.Getpgrp() {
+			return
-			_ = term.Restore(unix.Stdin, terminalState)
+		}
+
+		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/unlock_failure_test.go

@@ -1,12 +1,12 @@
 // 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
+// file the unlocker needs is missing, the error keeps its cause and ends by
-// keeps its cause and ends by saying that the mnemonic still opens that
+// saying that the mnemonic still opens that vault, but only for a vault that
-// vault, but only for a vault that the mnemonic does open, and not when the
+// the mnemonic does open, and not when the passphrase could not be read at
-// passphrase could not be read at all. When the current unlocker does not
+// all. A wrong passphrase gives an error that says so, without that advice.
-// exist and the vault has another that `secret unlocker select` accepts, the
+// When the current unlocker does not exist and the vault has another that
-// error says to select one instead.
+// `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.
 
@@ -18,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"
@@ -164,26 +164,48 @@ func TestUnlockFailureNamesMnemonic(t *testing.T) {
 	}
 }
 
-// TestWrongPassphraseNamesMnemonic checks the error of `secret get` given a
+// TestWrongPassphraseNamesPassphrase checks the error of `secret get` given
-// passphrase that does not decrypt the passphrase unlocker: it keeps age's
+// a passphrase that does not decrypt the passphrase unlocker: it says that
-// error and ends with the advice that the mnemonic still opens the vault.
+// the passphrase is wrong, without the advice to use the mnemonic, since the
-func TestWrongPassphraseNamesMnemonic(t *testing.T) {
+// 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.ErrorIs(t, err, secret.ErrWrongPassphrase)
-
+	assert.NotContains(t, err.Error(), mnemonicAdvice)
-	require.ErrorAs(t, err, &noMatch)
-
 	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: "+
+		"wrong passphrase for unlocker "+filepath.Base(unlockerDir))
-		noMatch.Error()+mnemonicAdvice)
+
+	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
@@ -288,7 +310,7 @@ func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
 }
 
 // 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.
@@ -319,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 "+
+		"failed to read passphrase: there is no terminal to ask on. Please "+
-		"script). Please set the SB_UNLOCK_PASSPHRASE environment variable or "+
+		"set the SB_UNLOCK_PASSPHRASE environment variable or run "+
-		"run interactively\n", string(output))
+		"interactively\n", string(output))
 }
 
 // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and

internal/cli/unlockers.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"maps"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -66,12 +67,13 @@ func getDefaultGPGKey() (string, error) {
 
 	output, err := cmd.Output()
 	if err == nil {
-		for line := range strings.SplitSeq(string(output), "\n") {
+		defaultKey, parseErr := parseDefaultGPGKey(string(output))
-			fields := strings.Split(line, ":")
+		if parseErr != nil {
-			if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" {
+			return "", parseErr
-				// The default key is in field 10 (index 9)
+		}
-				return fields[9], nil
+
-			}
+		if defaultKey != "" {
+			return defaultKey, nil
 		}
 	}
 
@@ -98,6 +100,27 @@ func getDefaultGPGKey() (string, error) {
 	return "", errNoGPGSecretKeys
 }
 
+// parseDefaultGPGKey returns the default-key value in the output of
+// `gpgconf --list-options gpg`, or "" when gpg.conf sets none. gpgconf(1)
+// prints a string value with a leading `"` and percent-escaped.
+func parseDefaultGPGKey(gpgconfOutput string) (string, error) {
+	for line := range strings.SplitSeq(gpgconfOutput, "\n") {
+		fields := strings.Split(line, ":")
+		if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" {
+			// The default key is in field 10 (index 9)
+			keyID, err := url.PathUnescape(strings.TrimPrefix(fields[9], `"`))
+			if err != nil {
+				return "", fmt.Errorf("failed to decode gpgconf default-key %s: %w",
+					fields[9], err)
+			}
+
+			return keyID, nil
+		}
+	}
+
+	return "", nil
+}
+
 func newUnlockerCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "unlocker",

internal/cli/unlockers_add_test.go

@@ -2,6 +2,8 @@
 package cli
 
 import (
+	"bytes"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -83,6 +85,77 @@ func TestAddPGPUnlocker(t *testing.T) {
 	}
 }
 
+// TestAddPGPUnlockerDefaultKey is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/141: with default-key set in
+// gpg.conf and no --keyid, `secret unlocker add pgp` uses that key.
+// gpg.conf names the key by its user ID, and the printed key ID must be that
+// user ID. The first secret key, which is used when gpg.conf sets no
+// default-key, prints as a hex key ID.
+func TestAddPGPUnlockerDefaultKey(t *testing.T) {
+	newTestGPGKey(t)
+	// The key comes from neither --keyid nor the environment.
+	t.Setenv(secret.EnvGPGKeyID, "")
+
+	gpgConf := filepath.Join(os.Getenv("GNUPGHOME"), "gpg.conf")
+	//nolint:gosec // G703: GNUPGHOME is the directory newTestGPGKey made
+	require.NoError(t, os.WriteFile(gpgConf,
+		[]byte("default-key "+unreadableTestGPGUserID+"\n"), 0o600))
+
+	fs := afero.NewMemMapFs()
+	_, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName,
+		testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
+	instance, cmd := newTestInstance(fs)
+	instance.Mnemonic = testMnemonicBuffer(t)
+
+	var output bytes.Buffer
+	cmd.SetOut(&output)
+
+	require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd))
+	assert.Contains(t, output.String(),
+		"Using default GPG key: "+unreadableTestGPGUserID+"\n")
+}
+
+// TestParseDefaultGPGKey is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/141: gpgconf prints the
+// default-key value with a leading `"` and percent-escaped, and the key ID
+// is that value decoded.
+func TestParseDefaultGPGKey(t *testing.T) {
+	t.Parallel()
+
+	// gpgconfLine is the default-key line of `gpgconf --list-options gpg`
+	// up to the value, its last field.
+	const gpgconfLine = "default-key:0:0:use NAME as default secret key:" +
+		"1:1:NAME:::"
+
+	tests := []struct {
+		name  string
+		value string
+		want  string
+	}{
+		{
+			"fingerprint", `"56163D7F3FAFF709608855BEC1A360216D007F41`,
+			"56163D7F3FAFF709608855BEC1A360216D007F41",
+		},
+		{
+			"user ID with a comma", `"Doe%2c Alice <alice@example.com>`,
+			"Doe, Alice <alice@example.com>",
+		},
+		{"no default-key in gpg.conf", "", ""},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			keyID, err := parseDefaultGPGKey(gpgconfLine + test.value + "\n")
+			require.NoError(t, err)
+			assert.Equal(t, test.want, keyID)
+		})
+	}
+}
+
 // TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
 // the keyring does not hold fails at looking up the key's fingerprint and
 // leaves no new unlocker directory. The error must come from the lookup: a

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(
+	errNoTerminal          = errors.New("there is no terminal to ask on")
-		"stdin is not a terminal (piped input or script)")
+	errNothingEntered      = errors.New("nothing was entered")
-	errStderrNotTerminal = errors.New(
-		"stderr is not a terminal (running in non-interactive mode)")
-	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
+// readFromTerminal asks on the controlling terminal and reads the answer
-// error it returns wraps notRead; without a terminal, the error says to set
+// there without echoing it, whatever stdin is: in
-// envVar instead.
+// `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
+	terminal, err := os.OpenFile(TerminalPath, os.O_RDWR, 0)
-	if !term.IsTerminal(syscall.Stdin) {
+	if err != nil {
-		// Not a terminal - never read secrets from piped input
+		Debug("Failed to open the controlling terminal", "error", err)
-		// for security reasons
+
 		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
+	_, _ = fmt.Fprint(terminal, prompt)
-	// interactive prompting
-	if !term.IsTerminal(syscall.Stderr) {
-		return nil, fmt.Errorf("%w: %w. Please set the %s environment variable",
-			notRead, errStderrNotTerminal, envVar)
-	}
 
-	// Both stdin and stderr are terminals - use secure password reading
+	input, err := term.ReadPassword(int(terminal.Fd()))
-	fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout
-
-	input, err := term.ReadPassword(syscall.Stdin)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", notRead, err)
 	}
 
-	// Print newline to stderr since ReadPassword doesn't echo
+	// The newline typed after the input, which ReadPassword did not echo
-	fmt.Fprintln(os.Stderr)
+	_, _ = 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/vault.go

@@ -291,10 +291,13 @@ 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. Nor is it added when the current unlocker does not exist and the
+// same way. Nor is it added when the passphrase is wrong: the unlocker still
-// vault has others: the error already says to select one of them.
+// 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) ||
+		errors.Is(err, secret.ErrWrongPassphrase) ||
 		errors.Is(err, ErrUnlockerNotFound) {
 		return err
 	}

internal/vault/vault_error_test.go

@@ -5,7 +5,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"filippo.io/age"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -140,9 +139,7 @@ func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
 
 	err = vlt.AddSecret(testSecretName, second, true)
 
-	var noMatch *age.NoIdentityMatchError
+	require.ErrorIs(t, err, secret.ErrWrongPassphrase)
-
-	require.ErrorAs(t, err, &noMatch)
 	assert.Equal(t, before, dirContents(t, fs, secretDir))
 }