Stop vault creation when a vault's metadata cannot be read (closes #140)

GetNextDerivationIndex passed over a vault whose vault-metadata.json could
not be read or parsed, so the next vault made from the same mnemonic could
get that vault's derivation index and so the same long-term key. It now
returns an error naming the file, and vault create, vault import and init
stop before writing anything. A vault directory with no metadata file is
still passed over.

The regression test corrupts one vault's metadata and requires CreateVault
with the same mnemonic to fail, naming the file, and leave no new vault.

Unverified by test: an unreadable metadata file; checked by hand only.

Model: opus-5-5

README.md

@@ -381,11 +381,6 @@ 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

@@ -25,16 +25,6 @@ https://git.eeqj.de/sneak/secret/milestone/12
   passed over such a vault, so the next vault made from the same mnemonic could
   be given its derivation index, and so its long-term key. A vault directory
   with no `vault-metadata.json` is still passed over.
-- 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,7 +8,6 @@ import (
 	"os/exec"
 	"slices"
 	"strings"
-	"syscall"
 	"testing"
 
 	"github.com/awnumar/memguard"
@@ -198,10 +197,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 no terminal said "failed to read mnemonic:
-// failed to read passphrase: ...". The error must wrap
+// 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
 // secret.ErrMnemonicNotRead and name the mnemonic only. The message is
-// pinned on the built binary, run in a new session, which has no terminal.
+// pinned on the built binary, whose stdin is surely not a terminal.
 func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	t.Parallel()
 
@@ -217,15 +216,14 @@ 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: there is no terminal to ask on. "+
-		"Please set the SB_SECRET_MNEMONIC environment variable or run "+
-		"interactively\n", string(output))
+		"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))
 }
 
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for

internal/cli/init.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"log/slog"
+	"os"
 	"strings"
 
 	"github.com/awnumar/memguard"
@@ -56,11 +57,13 @@ 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", "error", err)
+		secret.Debug("Failed to read mnemonic from stdin", "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

@@ -1,204 +0,0 @@
-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,29 +15,16 @@ 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 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()))
-	}
+	// 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)
 
 	memguard.CatchSignal(func(os.Signal) {
-		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)
+		foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP)
+		if terminalErr == nil && err == nil && foreground == unix.Getpgrp() {
+			_ = term.Restore(unix.Stdin, terminalState)
 		}
 	}, os.Interrupt, unix.SIGTERM)
 

internal/cli/unlock_failure_test.go

@@ -18,7 +18,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"syscall"
 	"testing"
 
 	"filippo.io/age"
@@ -289,7 +288,7 @@ func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
 }
 
 // TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
-// binary without SB_UNLOCK_PASSPHRASE and in a new session, which has no
+// binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a
 // terminal, so the passphrase cannot be read. The unlocker was not tried,
 // 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.
@@ -320,16 +319,15 @@ 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: there is no terminal to ask on. Please "+
-		"set the SB_UNLOCK_PASSPHRASE environment variable or run "+
-		"interactively\n", string(output))
+		"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))
 }
 
 // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and

internal/secret/constants.go

@@ -17,10 +17,6 @@ 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,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"syscall"
 	"unsafe"
 
 	"filippo.io/age"
@@ -15,8 +16,11 @@ import (
 
 var (
 	errNilPassphraseBuffer = errors.New("passphrase buffer is nil")
-	errNoTerminal          = errors.New("there is no terminal to ask on")
-	errNothingEntered      = errors.New("nothing was entered")
+	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")
 )
 
 // ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no
@@ -189,33 +193,38 @@ func ReadMnemonic(prompt string) (*memguard.LockedBuffer, error) {
 	return readFromTerminal(prompt, ErrMnemonicNotRead, EnvMnemonic)
 }
 
-// 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.
+// 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.
 func readFromTerminal(
 	prompt string, notRead error, envVar string,
 ) (*memguard.LockedBuffer, error) {
-	terminal, err := os.OpenFile(TerminalPath, os.O_RDWR, 0)
-	if err != nil {
-		Debug("Failed to open the controlling terminal", "error", err)
-
+	// Check if stdin is a terminal
+	if !term.IsTerminal(syscall.Stdin) {
+		// Not a terminal - never read secrets from piped input
+		// for security reasons
 		return nil, fmt.Errorf(
 			"%w: %w. Please set the %s environment variable or run interactively",
-			notRead, errNoTerminal, envVar)
+			notRead, errStdinNotTerminal, envVar)
 	}
-	defer func() { _ = terminal.Close() }()
 
-	_, _ = fmt.Fprint(terminal, prompt)
+	// 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)
+	}
 
-	input, err := term.ReadPassword(int(terminal.Fd()))
+	// 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)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", notRead, err)
 	}
 
-	// The newline typed after the input, which ReadPassword did not echo
-	_, _ = fmt.Fprintln(terminal)
+	// Print newline to stderr since ReadPassword doesn't echo
+	fmt.Fprintln(os.Stderr)
 
 	if len(input) == 0 {
 		return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)