Ask for the passphrase on the controlling terminal, not stdin (closes #144)
check / check (push) Waiting to run

The passphrase and mnemonic prompts open /dev/tty and ask there, so
`echo v | secret add s1 --force`, `secret encrypt` and `secret decrypt`
with piped input ask at a terminal instead of failing. Only a process
with no controlling terminal still gets the error naming the variable to
set. `secret init` and `secret vault create` no longer write a newline
to stderr after the mnemonic.

On SIGINT or SIGTERM, Entry() restores the settings of /dev/tty instead
of stdin, since that is where the prompt turns echo off.

The two tests that pin the no-terminal message run the binary in a new
session, so it has no terminal wherever the tests run.

Unverified by a test: the echo restore after Ctrl-C, checked by hand.

Model: opus-5-5
This commit was merged in pull request #165.
This commit is contained in:
2026-10-14 23:02:23 +02:00
parent fe94e99801
commit 46c395e045
9 changed files with 276 additions and 48 deletions
+5
View File
@@ -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 set, is the safer default; `secret vault import` has no prompt and needs both
variables. 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 ## Security Features
### Encryption ### Encryption
+10
View File
@@ -18,6 +18,16 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps # Completed Steps
- 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 - 2026-10-14: `secret generate mnemonic`, `secret vault list` and
`secret unlocker list` print their output to stdout, as `secret list` does, so `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). it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138).
+8 -6
View File
@@ -8,6 +8,7 @@ import (
"os/exec" "os/exec"
"slices" "slices"
"strings" "strings"
"syscall"
"testing" "testing"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -197,10 +198,10 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
// TestMnemonicNotReadNamesOnlyMnemonic is a regression test for // TestMnemonicNotReadNamesOnlyMnemonic is a regression test for
// https://git.eeqj.de/sneak/secret/issues/115: `secret init` without // 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 // 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) { func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
t.Parallel() t.Parallel()
@@ -216,14 +217,15 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
"PATH=" + os.Getenv("PATH"), "PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"), "HOME=" + os.Getenv("HOME"),
} }
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
output, err := cmd.CombinedOutput() output, err := cmd.CombinedOutput()
require.Error(t, err) require.Error(t, err)
require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+ 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 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for
+1 -4
View File
@@ -5,7 +5,6 @@ import (
"fmt" "fmt"
"log" "log"
"log/slog" "log/slog"
"os"
"strings" "strings"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -57,13 +56,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
// Read mnemonic securely without echo // Read mnemonic securely without echo
mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ") mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ")
if err != nil { 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 return nil, nil, err
} }
fmt.Fprintln(os.Stderr) // Add newline after hidden input
return mnemonicBuffer, mnemonicBuffer.Destroy, nil return mnemonicBuffer, mnemonicBuffer.Destroy, nil
} }
+204
View File
@@ -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()
}
+21 -8
View File
@@ -15,16 +15,29 @@ import (
// but exit with the code. // but exit with the code.
func Entry() int { func Entry() int {
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer // 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) { 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) }, os.Interrupt, unix.SIGTERM)
+6 -4
View File
@@ -18,6 +18,7 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"syscall"
"testing" "testing"
"filippo.io/age" "filippo.io/age"
@@ -288,7 +289,7 @@ func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
} }
// TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built // 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, // 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 // 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. // is the cause alone, without the advice to use the mnemonic.
@@ -319,15 +320,16 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
"PATH=" + os.Getenv("PATH"), "PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"), "HOME=" + os.Getenv("HOME"),
} }
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
output, err := cmd.CombinedOutput() output, err := cmd.CombinedOutput()
require.Error(t, err) require.Error(t, err)
assert.Equal(t, "Error: failed to unlock vault: "+ assert.Equal(t, "Error: failed to unlock vault: "+
"failed to get long-term key: failed to get unlocker identity: "+ "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 // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and
+4
View File
@@ -17,6 +17,10 @@ const (
// EnvGPGKeyID is the environment variable for providing the GPG key ID // EnvGPGKeyID is the environment variable for providing the GPG key ID
EnvGPGKeyID = "SB_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 // 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 // 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 // unlocker, even one added right after another, gets a directory of its
+17 -26
View File
@@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"syscall"
"unsafe" "unsafe"
"filippo.io/age" "filippo.io/age"
@@ -16,11 +15,8 @@ import (
var ( var (
errNilPassphraseBuffer = errors.New("passphrase buffer is nil") 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 // 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) 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( func readFromTerminal(
prompt string, notRead error, envVar string, prompt string, notRead error, envVar string,
) (*memguard.LockedBuffer, error) { ) (*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( return nil, fmt.Errorf(
"%w: %w. Please set the %s environment variable or run interactively", "%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 { if err != nil {
return nil, fmt.Errorf("%w: %w", notRead, err) 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 { if len(input) == 0 {
return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered) return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)