Check arguments before looking for the mnemonic (closes #79)

The key index, --words, the --to recipients and the input file of
age encrypt and age decrypt were checked only after the mnemonic had
been obtained, so a run that could not succeed still prompted for it or
ran the mnemonic command. options.Index now refuses an index above
derive.MaxIndex, which covers every command group; the mnemonic command
calls the new childmnemonic.CheckWords first; age encrypt reads its
recipients with the new agekey.Recipients, and both age commands open
their input, before the key is derived. The README says which checks
come first.

Judgement call: the -o path is still looked at after the mnemonic.

Model: opus-5-5

internal/agekey/agekey.go

@@ -94,15 +94,12 @@ func (k *Key) Identity() string {
 }
 
 // Encrypt copies src to dst, encrypted to this key and to every extra
-// recipient named, so the same mnemonic can always read it back again.
+// recipient given, so the same mnemonic can always read it back again.
 // Armored output is the text form age also reads.
 func (k *Key) Encrypt(
-	dst io.Writer, src io.Reader, to []string, armored bool,
+	dst io.Writer, src io.Reader, to []age.Recipient, armored bool,
 ) error {
-	all, err := recipients(k.identity.Recipient(), to)
-	if err != nil {
-		return err
-	}
+	all := append([]age.Recipient{k.identity.Recipient()}, to...)
 
 	out := dst
 
@@ -113,7 +110,7 @@ func (k *Key) Encrypt(
 		out = text
 	}
 
-	err = encrypt(out, src, all)
+	err := encrypt(out, src, all)
 	if err != nil {
 		return err
 	}
@@ -130,12 +127,12 @@ func (k *Key) Encrypt(
 	return nil
 }
 
-// recipients returns the key's own recipient followed by the ones
-// named on the command line, so what the key encrypts it can read.
-func recipients(mine age.Recipient, to []string) ([]age.Recipient, error) {
-	list := []age.Recipient{mine}
+// Recipients reads the recipients named on the command line, for
+// Encrypt to add to the key's own.
+func Recipients(names []string) ([]age.Recipient, error) {
+	list := make([]age.Recipient, 0, len(names))
 
-	for _, name := range to {
+	for _, name := range names {
 		parsed, err := age.ParseX25519Recipient(name)
 		if err != nil {
 			return nil, fmt.Errorf("reading recipient %q: %w", name, err)

internal/agekey/agekey_test.go

@@ -119,12 +119,12 @@ func TestAnExtraRecipientCanReadItTooAndSoCanTheDerivedOne(t *testing.T) {
 
 	mine, theirs := forIndex(t, 0), forIndex(t, 1)
 
+	to, err := agekey.Recipients([]string{theirs.Recipient()})
+	require.NoError(t, err)
+
 	var sealed bytes.Buffer
 
-	err := mine.Encrypt(
-		&sealed, strings.NewReader("hello"),
-		[]string{theirs.Recipient()}, false,
-	)
+	err = mine.Encrypt(&sealed, strings.NewReader("hello"), to, false)
 	require.NoError(t, err)
 
 	for _, key := range []*agekey.Key{mine, theirs} {
@@ -138,10 +138,7 @@ func TestAnExtraRecipientCanReadItTooAndSoCanTheDerivedOne(t *testing.T) {
 func TestARecipientThatIsNotOneIsRefused(t *testing.T) {
 	t.Parallel()
 
-	err := forIndex(t, 0).Encrypt(
-		&bytes.Buffer{}, strings.NewReader("hello"),
-		[]string{"not a recipient"}, false,
-	)
+	_, err := agekey.Recipients([]string{"not a recipient"})
 	require.Error(t, err)
 }
 

internal/childmnemonic/childmnemonic.go

@@ -29,6 +29,17 @@ const DefaultWords = twelve
 // ErrWordCount is returned for a length this tool does not offer.
 var ErrWordCount = errors.New("a child mnemonic has 12, 18 or 24 words")
 
+// CheckWords refuses a length this tool does not offer, so that the
+// command can turn it down before it looks for the mnemonic.
+func CheckWords(words uint32) error {
+	switch words {
+	case twelve, eighteen, twentyFour:
+		return nil
+	default:
+		return fmt.Errorf("%w, not %d", ErrWordCount, words)
+	}
+}
+
 // Derive returns the English child mnemonic of that many words at that
 // key index, taken from the master key with BIP-85's own mnemonic
 // application, number 39. The words come straight from the BIP-85
@@ -38,13 +49,12 @@ func Derive(
 	master *hdkeychain.ExtendedKey,
 	words, index uint32,
 ) (string, error) {
-	switch words {
-	case twelve, eighteen, twentyFour:
-	default:
-		return "", fmt.Errorf("%w, not %d", ErrWordCount, words)
+	err := CheckWords(words)
+	if err != nil {
+		return "", err
 	}
 
-	err := derive.CheckIndex(index)
+	err = derive.CheckIndex(index)
 	if err != nil {
 		return "", err
 	}

internal/cli/age/age.go

@@ -108,47 +108,25 @@ func decrypt() *cobra.Command {
 }
 
 // runEncrypt encrypts to the derived recipient and any others given.
+// The recipients are read and the input opened before the key is
+// derived, so that a run refused for either never asks for the
+// mnemonic.
 func runEncrypt(cmd *cobra.Command, args []string) error {
-	key, err := derived(cmd)
-	if err != nil {
-		return err
-	}
-
-	to, err := cmd.Flags().GetStringArray("to")
+	names, err := cmd.Flags().GetStringArray("to")
 	if err != nil {
 		return fmt.Errorf("reading the recipients: %w", err)
 	}
 
+	to, err := agekey.Recipients(names)
+	if err != nil {
+		return err
+	}
+
 	armored, err := cmd.Flags().GetBool("armor")
 	if err != nil {
 		return fmt.Errorf("reading the armor flag: %w", err)
 	}
 
-	return through(cmd, args, func(dst io.Writer, src io.Reader) error {
-		return key.Encrypt(dst, src, to, armored)
-	})
-}
-
-// runDecrypt decrypts with the derived identity.
-func runDecrypt(cmd *cobra.Command, args []string) error {
-	key, err := derived(cmd)
-	if err != nil {
-		return err
-	}
-
-	return through(cmd, args, key.Decrypt)
-}
-
-// through opens the input the arguments ask for and hands it to the
-// work, with the file --output names to write to, or the command's own
-// output when it names none or names the same file as that output, and
-// the command's own error output when it names the same file as that.
-// Those two are the streams the tool already has, so whatever they are
-// redirected to is written as the redirect says, never replaced.
-func through(
-	cmd *cobra.Command, args []string,
-	work func(io.Writer, io.Reader) error,
-) error {
 	src, closeSrc, err := input(cmd, args)
 	if err != nil {
 		return err
@@ -156,6 +134,44 @@ func through(
 
 	defer closeSrc()
 
+	key, err := derived(cmd)
+	if err != nil {
+		return err
+	}
+
+	return through(cmd, src, func(dst io.Writer, plain io.Reader) error {
+		return key.Encrypt(dst, plain, to, armored)
+	})
+}
+
+// runDecrypt decrypts with the derived identity. The input is opened
+// before the key is derived, as for runEncrypt.
+func runDecrypt(cmd *cobra.Command, args []string) error {
+	src, closeSrc, err := input(cmd, args)
+	if err != nil {
+		return err
+	}
+
+	defer closeSrc()
+
+	key, err := derived(cmd)
+	if err != nil {
+		return err
+	}
+
+	return through(cmd, src, key.Decrypt)
+}
+
+// through hands the input to the work, with the file --output names to
+// write to, or the command's own output when it names none or names the
+// same file as that output, and the command's own error output when it
+// names the same file as that. Those two are the streams the tool
+// already has, so whatever they are redirected to is written as the
+// redirect says, never replaced.
+func through(
+	cmd *cobra.Command, src io.Reader,
+	work func(io.Writer, io.Reader) error,
+) error {
 	name, err := cmd.Flags().GetString("output")
 	if err != nil {
 		return fmt.Errorf("reading the output file: %w", err)

internal/cli/age_test.go

@@ -17,6 +17,7 @@ import (
 	"sneak.berlin/go/keyfunc/internal/agekey"
 	"sneak.berlin/go/keyfunc/internal/cli"
 	"sneak.berlin/go/keyfunc/internal/cli/age"
+	"sneak.berlin/go/keyfunc/internal/derive"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
 )
 
@@ -69,6 +70,44 @@ func TestAnotherRecipientIsAddedAndTheDerivedOneStays(t *testing.T) {
 	)
 }
 
+// The mnemonic command fails in these runs, so the refusal of the index
+// shows that it was checked before the mnemonic was looked for.
+func TestAnAgeIndexWithNoHardenedChildIsRefused(t *testing.T) {
+	t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand)
+
+	for _, command := range []string{"pub", "priv", "encrypt", "decrypt"} {
+		out, err := execute(t, "age", command, "-n", tooLarge)
+		require.ErrorIs(t, err, derive.ErrIndexTooLarge, command)
+		require.Empty(t, out, command)
+	}
+}
+
+// The mnemonic command fails in this run, so the refusal of the
+// recipient shows that it was read before the mnemonic was looked for.
+func TestAnEncryptionToARecipientThatIsNotOneIsRefused(t *testing.T) {
+	t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand)
+
+	plain := written(t, "notes.txt", "the secret\n")
+
+	out, err := execute(t, "age", "encrypt", "--to", "bogus", plain)
+	require.ErrorContains(t, err, `reading recipient "bogus"`)
+	require.Empty(t, out)
+}
+
+// The mnemonic command fails in these runs, so the refusal of the input
+// file shows that it was opened before the mnemonic was looked for.
+func TestAnInputFileThatCannotBeOpenedIsRefused(t *testing.T) {
+	t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand)
+
+	missing := filepath.Join(t.TempDir(), "missing")
+
+	for _, operation := range []string{"encrypt", "decrypt"} {
+		out, err := execute(t, "age", operation, missing)
+		require.ErrorIs(t, err, fs.ErrNotExist, operation)
+		require.Empty(t, out, operation)
+	}
+}
+
 func TestAFileForAnotherKeyIsRefused(t *testing.T) {
 	t.Setenv(mnemonic.Variable, example())
 

internal/cli/cli_test.go

@@ -34,6 +34,14 @@ const (
 	twentyFour = 24
 )
 
+// tooLarge is the smallest key index with no hardened child.
+const tooLarge = "2147483648"
+
+// failingMnemonicCommand fails loudly, so a run that looks for the
+// mnemonic ends with that failure. A run that ends with the refusal of
+// an argument instead shows that the argument was checked first.
+const failingMnemonicCommand = "echo the mnemonic was looked for >&2; exit 1"
+
 // example returns the mnemonic the README gives its test vectors for:
 // eleven abandons and about.
 func example() string {
@@ -99,12 +107,25 @@ func TestThePrivateKeyMatchesThePublicOne(t *testing.T) {
 	require.Equal(t, vectorOne, back)
 }
 
+// The mnemonic command fails in these runs, so the refusal of the index
+// shows that it was checked before the mnemonic was looked for. The age
+// commands have a test of their own.
 func TestAnIndexWithNoHardenedChildIsRefused(t *testing.T) {
-	t.Setenv(mnemonic.Variable, example())
+	t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand)
 
-	out, err := execute(t, "ssh", "pub", "-n", "2147483648")
-	require.ErrorIs(t, err, derive.ErrIndexTooLarge)
-	require.Empty(t, out)
+	for _, args := range [][]string{
+		{subcommand, "pub", "-n", tooLarge},
+		{subcommand, "priv", "-n", tooLarge},
+		{subcommand, installing, "-n", tooLarge, host},
+		{subcommand, "to", "-n", tooLarge, host},
+		{"mnemonic", "-n", tooLarge},
+	} {
+		name := strings.Join(args, " ")
+
+		out, err := execute(t, args...)
+		require.ErrorIs(t, err, derive.ErrIndexTooLarge, name)
+		require.Empty(t, out, name)
+	}
 }
 
 func TestAnIndexWithALeadingZeroIsDecimal(t *testing.T) {
@@ -181,8 +202,10 @@ func TestAChildMnemonicIsPrinted(t *testing.T) {
 	require.NotEqual(t, short, next)
 }
 
+// The mnemonic command fails in this run, so the refusal of the length
+// shows that it was checked before the mnemonic was looked for.
 func TestALengthTheToolDoesNotOfferIsRefused(t *testing.T) {
-	t.Setenv(mnemonic.Variable, example())
+	t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand)
 
 	out, err := execute(t, "mnemonic", "--words", "15")
 	require.ErrorIs(t, err, childmnemonic.ErrWordCount)

internal/cli/mnemonic/mnemonic.go

@@ -52,6 +52,11 @@ func derived(cmd *cobra.Command) (string, error) {
 		return "", fmt.Errorf("reading the word count: %w", err)
 	}
 
+	err = childmnemonic.CheckWords(words)
+	if err != nil {
+		return "", err
+	}
+
 	parent, err := options.Mnemonic(cmd)
 	if err != nil {
 		return "", err

internal/cli/options/options.go

@@ -7,6 +7,7 @@ import (
 	"strconv"
 
 	"github.com/spf13/cobra"
+	"sneak.berlin/go/keyfunc/internal/derive"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
 )
 
@@ -23,13 +24,20 @@ func Add(cmd *cobra.Command) {
 	)
 }
 
-// Index returns the key index the user asked for.
+// Index returns the key index the user asked for, and refuses one above
+// derive.MaxIndex. Every command calls it before it looks for the
+// mnemonic.
 func Index(cmd *cobra.Command) (uint32, error) {
 	index, err := cmd.Flags().GetUint32("index")
 	if err != nil {
 		return 0, fmt.Errorf("reading the index: %w", err)
 	}
 
+	err = derive.CheckIndex(index)
+	if err != nil {
+		return 0, err
+	}
+
 	return index, nil
 }