2 Commits
Author SHA1 Message Date
clawbot 28606075c6 A secret too large for the locked-memory limit fails with an error, not a panic (closes #145)
check / check (push) Canceled after 0s
secret add, import and generate secret crashed with memguard's panic once a
value outgrew the locked-memory limit, and because the value was held twice,
half the usual 8 MiB limit was enough. secret now raises the limit to the hard
limit at startup. The value is read a page at a time into a memguard.Stream,
which keeps it encrypted, then copied once into locked memory after its size is
checked against 100 MiB and against the limit less 16 pages for the command's
keys. It comes out of the stream through Next, not Read: Read re-encrypts a
leftover piece, and memguard hangs if that runs out of locked memory.

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

Model: opus-5-5
2026-10-14 21:22:58 +00:00
clawbot 46c395e045 Ask for the passphrase on the controlling terminal, not stdin (closes #144)
check / check (push) Canceled after 0s
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
2026-10-14 23:02:23 +02:00
9 changed files with 276 additions and 48 deletions
+5
View File
@@ -392,6 +392,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
+10
View File
@@ -29,6 +29,16 @@ https://git.eeqj.de/sneak/secret/milestone/12
size is checked against 100 MiB and against the limit less 16 pages for the
command's keys. `generate secret` checks the length before it generates
anything.
- 2026-10-14: 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).
+8 -6
View File
@@ -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
+1 -4
View File
@@ -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
}
+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
@@ -17,16 +17,29 @@ func Entry() int {
raiseLockedMemoryLimit()
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
// and exits with status 1. The passphrase prompt turns terminal echo
// off until the read finishes, so a signal there would leave echo off.
// 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)
+6 -4
View File
@@ -18,6 +18,7 @@ import (
"os"
"os/exec"
"path/filepath"
"syscall"
"testing"
"filippo.io/age"
@@ -288,7 +289,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 +320,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
+4
View File
@@ -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
+17 -26
View File
@@ -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)