From b53052b0578d44ecd2134a8dd43ec0d1d560b700 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 23:14:08 +0000 Subject: [PATCH] Name only the mnemonic when it cannot be read (closes #115) secret init and secret vault create read the mnemonic with the new secret.ReadMnemonic, whose every error wraps the new secret.ErrMnemonicNotRead. It shares the terminal read with ReadPassphrase, whose errors still wrap ErrPassphraseNotRead. Without a terminal the error names the environment variable that gives the value instead: SB_SECRET_MNEMONIC for the mnemonic, SB_UNLOCK_PASSPHRASE for the passphrase. A test pins the message of init without a terminal. Model: opus-5-5 --- TODO.md | 12 +++++++ internal/cli/create_vault_test.go | 32 +++++++++++++++++ internal/cli/init.go | 4 +-- internal/cli/unlock_failure_test.go | 6 ++-- internal/secret/crypto.go | 55 +++++++++++++++++++---------- 5 files changed, 86 insertions(+), 23 deletions(-) diff --git a/TODO.md b/TODO.md index f431534..537ee82 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,18 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-04: A mnemonic that cannot be read, in `secret init` and + `secret vault create`, gives an error that names the mnemonic only + (https://git.eeqj.de/sneak/secret/issues/115). It is read with + `secret.ReadMnemonic`, whose every error wraps the new + `secret.ErrMnemonicNotRead`; before, it was read with `ReadPassphrase`, so + the message said "failed to read mnemonic: failed to read passphrase:" and + advised setting `SB_UNLOCK_PASSPHRASE`. Without a terminal it now says + "failed to read mnemonic: stdin is not a terminal (piped input or script). + Please set the SB_SECRET_MNEMONIC environment variable or run + interactively". The passphrase messages no longer repeat "cannot read + passphrase" after "failed to read passphrase:", and empty input gives + "nothing was entered". - 2026-10-04: A failure returns the same error value whichever command hits it (https://git.eeqj.de/sneak/secret/issues/113). `internal/cli` no longer keeps its own copies of `vault.ErrSecretNotFound`, `ErrVaultNotFound`, diff --git a/internal/cli/create_vault_test.go b/internal/cli/create_vault_test.go index 40f3111..a1cf25d 100644 --- a/internal/cli/create_vault_test.go +++ b/internal/cli/create_vault_test.go @@ -5,6 +5,7 @@ import ( "io" "maps" "os" + "os/exec" "slices" "strings" "testing" @@ -194,6 +195,37 @@ 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 +// secret.ErrMnemonicNotRead and name the mnemonic only. The message is +// pinned on the built binary, whose stdin is surely not a terminal. +func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) { + t.Parallel() + + c := cli.NewCLIInstanceWithStateDir(afero.NewMemMapFs(), testStateDir) + require.ErrorIs(t, c.Init(discardCmd()), secret.ErrMnemonicNotRead) + + stateDir := t.TempDir() + + //nolint:gosec // G204: test executes the freshly built secret binary + cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), "init") + cmd.Env = []string{ + secret.EnvStateDir + "=" + stateDir, + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + 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)) +} + // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for // https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault // create` killed after the passphrase prompt but before the unlocker was diff --git a/internal/cli/init.go b/internal/cli/init.go index 1b9142e..2a854be 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -55,11 +55,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) { secret.Debug("Prompting user for mnemonic phrase") // Read mnemonic securely without echo - mnemonicBuffer, err := secret.ReadPassphrase("Enter your BIP39 mnemonic phrase: ") + mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ") if err != nil { secret.Debug("Failed to read mnemonic from stdin", "error", err) - return nil, nil, fmt.Errorf("failed to read mnemonic: %w", err) + return nil, nil, err } fmt.Fprintln(os.Stderr) // Add newline after hidden input diff --git a/internal/cli/unlock_failure_test.go b/internal/cli/unlock_failure_test.go index 86e4396..c0db454 100644 --- a/internal/cli/unlock_failure_test.go +++ b/internal/cli/unlock_failure_test.go @@ -260,9 +260,9 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) { assert.Equal(t, "Error: failed to unlock vault: "+ "failed to get long-term key: failed to get unlocker identity: "+ - "failed to read passphrase: cannot read passphrase from non-terminal "+ - "stdin (piped input or script). 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 diff --git a/internal/secret/crypto.go b/internal/secret/crypto.go index 5f33278..9c017a1 100644 --- a/internal/secret/crypto.go +++ b/internal/secret/crypto.go @@ -17,16 +17,17 @@ import ( var ( errNilPassphraseBuffer = errors.New("passphrase buffer is nil") errStdinNotTerminal = errors.New( - "cannot read passphrase from non-terminal stdin " + - "(piped input or script). Please set the SB_UNLOCK_PASSPHRASE " + - "environment variable or run interactively") + "stdin is not a terminal (piped input or script)") errStderrNotTerminal = errors.New( - "cannot prompt for passphrase: stderr is not a terminal " + - "(running in non-interactive mode). Please set the " + - "SB_UNLOCK_PASSPHRASE environment variable") + "stderr is not a terminal (running in non-interactive mode)") + errNothingEntered = errors.New("nothing was entered") errEmptyPassphrase = errors.New("passphrase cannot be empty") ) +// ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no +// terminal to read the mnemonic from, reading it failed, or it was empty. +var ErrMnemonicNotRead = errors.New("failed to read mnemonic") + // EncryptToRecipient encrypts data to a recipient using age // The data parameter should be a LockedBuffer for secure memory handling func EncryptToRecipient( @@ -169,40 +170,58 @@ func DecryptWithPassphrase( // Returns a LockedBuffer containing the passphrase for secure memory handling. // Every error it returns wraps ErrPassphraseNotRead. func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) { + return readFromTerminal(prompt, ErrPassphraseNotRead, EnvUnlockPassphrase) +} + +// ReadMnemonic reads a mnemonic from the terminal as ReadPassphrase reads a +// passphrase. Every error it returns wraps ErrMnemonicNotRead. +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. +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 passphrases from piped input + // Not a terminal - never read secrets from piped input // for security reasons - return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errStdinNotTerminal) + return nil, fmt.Errorf( + "%w: %w. Please set the %s environment variable or run interactively", + notRead, errStdinNotTerminal, envVar) } // 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", ErrPassphraseNotRead, errStderrNotTerminal) + 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 fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout - passphrase, err := term.ReadPassword(syscall.Stdin) + input, err := term.ReadPassword(syscall.Stdin) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, err) + return nil, fmt.Errorf("%w: %w", notRead, err) } // Print newline to stderr since ReadPassword doesn't echo fmt.Fprintln(os.Stderr) - if len(passphrase) == 0 { - return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errEmptyPassphrase) + if len(input) == 0 { + return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered) } - // Create a secure buffer and copy the passphrase - secureBuffer := memguard.NewBufferFromBytes(passphrase) + // Create a secure buffer and copy the input + secureBuffer := memguard.NewBufferFromBytes(input) - // Clear the original passphrase slice - for i := range passphrase { - passphrase[i] = 0 + // Clear the original input slice + for i := range input { + input[i] = 0 } return secureBuffer, nil -- 2.54.0