Name only the mnemonic when it cannot be read (closes #115) #116

Merged
clawbot merged 1 commits from issue-115-mnemonic-read-error into next 2026-10-05 01:43:00 +02:00
5 changed files with 86 additions and 23 deletions
Showing only changes of commit b53052b057 - Show all commits
+12
View File
@@ -18,6 +18,18 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps # 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 - 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 it (https://git.eeqj.de/sneak/secret/issues/113). `internal/cli` no longer
keeps its own copies of `vault.ErrSecretNotFound`, `ErrVaultNotFound`, keeps its own copies of `vault.ErrSecretNotFound`, `ErrVaultNotFound`,
+32
View File
@@ -5,6 +5,7 @@ import (
"io" "io"
"maps" "maps"
"os" "os"
"os/exec"
"slices" "slices"
"strings" "strings"
"testing" "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 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for
// https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault // https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault
// create` killed after the passphrase prompt but before the unlocker was // create` killed after the passphrase prompt but before the unlocker was
+2 -2
View File
@@ -55,11 +55,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
secret.Debug("Prompting user for mnemonic phrase") secret.Debug("Prompting user for mnemonic phrase")
// Read mnemonic securely without echo // 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 { if err != nil {
secret.Debug("Failed to read mnemonic from stdin", "error", err) 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 fmt.Fprintln(os.Stderr) // Add newline after hidden input
+3 -3
View File
@@ -260,9 +260,9 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
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: cannot read passphrase from non-terminal "+ "failed to read passphrase: stdin is not a terminal (piped input or "+
"stdin (piped input or script). Please set the SB_UNLOCK_PASSPHRASE "+ "script). Please set the SB_UNLOCK_PASSPHRASE environment variable or "+
"environment variable or run interactively\n", string(output)) "run interactively\n", string(output))
} }
// TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and
+37 -18
View File
@@ -17,16 +17,17 @@ import (
var ( var (
errNilPassphraseBuffer = errors.New("passphrase buffer is nil") errNilPassphraseBuffer = errors.New("passphrase buffer is nil")
errStdinNotTerminal = errors.New( errStdinNotTerminal = errors.New(
"cannot read passphrase from non-terminal stdin " + "stdin is not a terminal (piped input or script)")
"(piped input or script). Please set the SB_UNLOCK_PASSPHRASE " +
"environment variable or run interactively")
errStderrNotTerminal = errors.New( errStderrNotTerminal = errors.New(
"cannot prompt for passphrase: stderr is not a terminal " + "stderr is not a terminal (running in non-interactive mode)")
"(running in non-interactive mode). Please set the " + errNothingEntered = errors.New("nothing was entered")
"SB_UNLOCK_PASSPHRASE environment variable")
errEmptyPassphrase = errors.New("passphrase cannot be empty") 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 // EncryptToRecipient encrypts data to a recipient using age
// The data parameter should be a LockedBuffer for secure memory handling // The data parameter should be a LockedBuffer for secure memory handling
func EncryptToRecipient( func EncryptToRecipient(
@@ -169,40 +170,58 @@ func DecryptWithPassphrase(
// Returns a LockedBuffer containing the passphrase for secure memory handling. // Returns a LockedBuffer containing the passphrase for secure memory handling.
// Every error it returns wraps ErrPassphraseNotRead. // Every error it returns wraps ErrPassphraseNotRead.
func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) { 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 // Check if stdin is a terminal
if !term.IsTerminal(syscall.Stdin) { 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 // 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 // stdin is a terminal, check if stderr is also a terminal for
// interactive prompting // interactive prompting
if !term.IsTerminal(syscall.Stderr) { 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 // Both stdin and stderr are terminals - use secure password reading
fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout 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 { 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 // Print newline to stderr since ReadPassword doesn't echo
fmt.Fprintln(os.Stderr) fmt.Fprintln(os.Stderr)
if len(passphrase) == 0 { if len(input) == 0 {
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errEmptyPassphrase) return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)
} }
// Create a secure buffer and copy the passphrase // Create a secure buffer and copy the input
secureBuffer := memguard.NewBufferFromBytes(passphrase) secureBuffer := memguard.NewBufferFromBytes(input)
// Clear the original passphrase slice // Clear the original input slice
for i := range passphrase { for i := range input {
passphrase[i] = 0 input[i] = 0
} }
return secureBuffer, nil return secureBuffer, nil