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