Say why a mnemonic is refused (closes #76)

Every way a mnemonic could be invalid was reported as a checksum
failure, including an empty one. It is now refused as empty, as having
a word count other than 12, 15, 18, 21 or 24, as having a word that is
not on the BIP-39 word list, or as failing the checksum. A word not on
the list is named by its position, never by the word itself.

Model: opus-5-5

README.md

@@ -119,14 +119,17 @@ order; the first one found wins:
 4. A prompt on the terminal with echo turned off.
 
 If none of these is available and standard input is not a terminal, the tool
-refuses and exits with status 1. A mnemonic that fails the BIP-39 checksum is
-refused with a message saying so. Keys are derived from the mnemonic's words
-joined by single spaces, whatever whitespace is around or between them, so one
-word per line, tabs or extra spaces give the same keys.
+refuses and exits with status 1. A mnemonic that is empty, does not have 12, 15,
+18, 21 or 24 words, has a word that is not on the BIP-39 word list, or fails the
+BIP-39 checksum is refused with a message saying which. A word that is not on
+the list is named by its position, never printed. Keys are derived from the
+mnemonic's words joined by single spaces, whatever whitespace is around or
+between them, so one word per line, tabs or extra spaces give the same keys.
 
 `KEYFUNC_MNEMONIC` and `KEYFUNC_MNEMONIC_COMMAND` are removed from the
-environment before the system `ssh` (`keyfunc ssh to`) and `sftp`
-(`keyfunc ssh install`) are started, so the mnemonic is never handed on to them.
+environment before the mnemonic command, the system `ssh` (`keyfunc ssh to`) and
+`sftp` (`keyfunc ssh install`) are started, so the mnemonic is never handed on
+to them.
 
 Every command takes `--index` / `-n` and `--mnemonic-command`, and has `--help`.
 `keyfunc --version` prints the version. `make build` stamps it; a binary

internal/cli/ssh/install.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/cli/signals"
+	"sneak.berlin/go/keyfunc/internal/mnemonic"
 )
 
 // Where the key goes on the host and what the file it arrives in is
@@ -220,7 +221,7 @@ func session(
 
 	//nolint:gosec // the options are the user's own, meant for sftp
 	command := exec.CommandContext(cmd.Context(), "sftp", argv...)
-	command.Env = childEnv()
+	command.Env = mnemonic.ChildEnv()
 	command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
 	command.Stdout = &said
 	command.Stderr = &said

internal/cli/ssh/ssh.go

@@ -3,13 +3,10 @@ package ssh
 
 import (
 	"fmt"
-	"os"
-	"strings"
 
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/cli/options"
 	"sneak.berlin/go/keyfunc/internal/derive"
-	"sneak.berlin/go/keyfunc/internal/mnemonic"
 	"sneak.berlin/go/keyfunc/internal/sshkey"
 )
 
@@ -87,26 +84,6 @@ func write(cmd *cobra.Command, text string) error {
 	return nil
 }
 
-// childEnv is the tool's environment with the mnemonic variables taken
-// out, for the ssh and sftp children it starts. "ssh to" exists so the
-// private key never leaves the tool; the mnemonic, from either variable,
-// must not leave it either.
-func childEnv() []string {
-	environ := os.Environ()
-	kept := make([]string, 0, len(environ))
-
-	for _, entry := range environ {
-		name, _, _ := strings.Cut(entry, "=")
-		if name == mnemonic.Variable || name == mnemonic.CommandVariable {
-			continue
-		}
-
-		kept = append(kept, entry)
-	}
-
-	return kept
-}
-
 // addComment gives a command its comment flag.
 func addComment(cmd *cobra.Command) {
 	cmd.Flags().String(

internal/cli/ssh/to.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/cli/signals"
+	"sneak.berlin/go/keyfunc/internal/mnemonic"
 )
 
 // StatusError says the tool should end with the status ssh ended with.
@@ -79,7 +80,7 @@ func to() *cobra.Command {
 func connect(ctx context.Context, argv []string) error {
 	//nolint:gosec // the arguments are the user's own, meant for ssh
 	command := exec.CommandContext(ctx, "ssh", argv...)
-	command.Env = childEnv()
+	command.Env = mnemonic.ChildEnv()
 	command.Stdin = os.Stdin
 	command.Stdout = os.Stdout
 	command.Stderr = os.Stderr

internal/mnemonic/mnemonic.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"slices"
 	"strings"
 
 	"golang.org/x/term"
@@ -29,6 +30,28 @@ var ErrMissing = errors.New(
 	"no mnemonic given and standard input is not a terminal",
 )
 
+// The lengths a BIP-39 mnemonic may have.
+const (
+	twelve     = 12
+	fifteen    = 15
+	eighteen   = 18
+	twentyOne  = 21
+	twentyFour = 24
+)
+
+// ErrEmpty is returned for a mnemonic with no words in it, such as the
+// output of a mnemonic command that printed nothing.
+var ErrEmpty = errors.New("the mnemonic is empty")
+
+// ErrWordCount is returned for a mnemonic with a number of words that
+// BIP-39 does not allow.
+var ErrWordCount = errors.New("a mnemonic has 12, 15, 18, 21 or 24 words")
+
+// ErrUnknownWord is returned for a mnemonic with a word that is not on
+// the BIP-39 English word list. The error gives the word's position,
+// never the word, so the mnemonic stays out of the message.
+var ErrUnknownWord = errors.New("not on the BIP-39 word list")
+
 // ErrChecksum is returned for a mnemonic that fails the BIP-39
 // checksum.
 var ErrChecksum = errors.New("the mnemonic fails its BIP-39 checksum")
@@ -63,6 +86,7 @@ func Read(ctx context.Context, command string) (string, error) {
 func run(ctx context.Context, command string) (string, error) {
 	//nolint:gosec // running the user's own command is the point
 	shell := exec.CommandContext(ctx, "sh", "-c", command)
+	shell.Env = ChildEnv()
 
 	var complaint bytes.Buffer
 
@@ -83,6 +107,25 @@ func run(ctx context.Context, command string) (string, error) {
 	return string(out), nil
 }
 
+// ChildEnv is the tool's environment with the mnemonic variables taken
+// out, for the programs the tool starts: the mnemonic command, ssh and
+// sftp. The mnemonic, from either variable, never leaves the tool.
+func ChildEnv() []string {
+	environ := os.Environ()
+	kept := make([]string, 0, len(environ))
+
+	for _, entry := range environ {
+		name, _, _ := strings.Cut(entry, "=")
+		if name == Variable || name == CommandVariable {
+			continue
+		}
+
+		kept = append(kept, entry)
+	}
+
+	return kept
+}
+
 // ask prompts on the terminal with echo turned off.
 func ask() (string, error) {
 	fd := int(os.Stdin.Fd())
@@ -106,13 +149,34 @@ func ask() (string, error) {
 
 // checked joins the words with single spaces, whatever whitespace
 // separated them, since the seed is computed over the string itself,
-// and refuses a mnemonic that does not pass the BIP-39 checksum.
-func checked(words string) (string, error) {
-	words = strings.Join(strings.Fields(words), " ")
+// and refuses a mnemonic that is not a valid BIP-39 mnemonic, with an
+// error that says what is wrong with it.
+func checked(text string) (string, error) {
+	words := strings.Fields(text)
 
-	if !bip39.IsMnemonicValid(words) {
+	switch len(words) {
+	case 0:
+		return "", ErrEmpty
+	case twelve, fifteen, eighteen, twentyOne, twentyFour:
+	default:
+		return "", fmt.Errorf("%w, not %d", ErrWordCount, len(words))
+	}
+
+	wordList := bip39.English()
+
+	for i, word := range words {
+		if !slices.Contains(wordList, word) {
+			return "", fmt.Errorf(
+				"word %d of the mnemonic is %w", i+1, ErrUnknownWord,
+			)
+		}
+	}
+
+	joined := strings.Join(words, " ")
+
+	if !bip39.IsMnemonicValid(joined) {
 		return "", ErrChecksum
 	}
 
-	return words, nil
+	return joined, nil
 }

internal/mnemonic/mnemonic_test.go

@@ -1,6 +1,8 @@
 package mnemonic_test
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -61,6 +63,31 @@ func TestTheMnemonicInTheEnvironmentComesLast(t *testing.T) {
 	require.Equal(t, fromVariable, words)
 }
 
+func TestTheMnemonicIsNotHandedToTheCommand(t *testing.T) {
+	const marker = "KEYFUNC_TEST_MARKER"
+
+	t.Setenv(mnemonic.CommandVariable, prints(fromCommandVariable))
+	t.Setenv(mnemonic.Variable, fromVariable)
+	t.Setenv(marker, "reaches the command")
+
+	path := filepath.Join(t.TempDir(), "environment")
+
+	words, err := mnemonic.Read(
+		t.Context(), "env > '"+path+"'; "+prints(example()),
+	)
+	require.NoError(t, err)
+	require.Equal(t, example(), words)
+
+	//nolint:gosec // the path is a temporary file of the test's own
+	content, err := os.ReadFile(path)
+	require.NoError(t, err)
+
+	environment := string(content)
+	require.NotContains(t, environment, mnemonic.Variable+"=")
+	require.NotContains(t, environment, mnemonic.CommandVariable+"=")
+	require.Contains(t, environment, marker+"=")
+}
+
 func TestAFailingCommandSaysWhatWentWrong(t *testing.T) {
 	t.Setenv(mnemonic.CommandVariable, "")
 	t.Setenv(mnemonic.Variable, "")
@@ -78,6 +105,38 @@ func TestABadChecksumIsRefused(t *testing.T) {
 	require.ErrorIs(t, err, mnemonic.ErrChecksum)
 }
 
+func TestACommandThatPrintsNothingGivesAnEmptyMnemonic(t *testing.T) {
+	t.Setenv(mnemonic.CommandVariable, "")
+	t.Setenv(mnemonic.Variable, "")
+
+	_, err := mnemonic.Read(t.Context(), "true")
+	require.ErrorIs(t, err, mnemonic.ErrEmpty)
+	require.NotContains(t, err.Error(), "checksum")
+}
+
+func TestAWrongWordCountIsRefused(t *testing.T) {
+	t.Setenv(mnemonic.CommandVariable, "")
+	t.Setenv(mnemonic.Variable, strings.Repeat("abandon ", 3))
+
+	_, err := mnemonic.Read(t.Context(), "")
+	require.ErrorIs(t, err, mnemonic.ErrWordCount)
+	require.ErrorContains(t, err, "not 3")
+	require.NotContains(t, err.Error(), "checksum")
+}
+
+func TestAWordNotOnTheListIsNamedByPositionOnly(t *testing.T) {
+	const typo = "abuot"
+
+	t.Setenv(mnemonic.CommandVariable, "")
+	t.Setenv(mnemonic.Variable, strings.Repeat("abandon ", 11)+typo)
+
+	_, err := mnemonic.Read(t.Context(), "")
+	require.ErrorIs(t, err, mnemonic.ErrUnknownWord)
+	require.ErrorContains(t, err, "word 12 ")
+	require.NotContains(t, err.Error(), typo)
+	require.NotContains(t, err.Error(), "checksum")
+}
+
 func TestNothingToReadAndNobodyToAsk(t *testing.T) {
 	t.Setenv(mnemonic.CommandVariable, "")
 	t.Setenv(mnemonic.Variable, "")