Read --index and --words as decimal numbers (closes #75)

Both flags were uint32 flags, which the flag library reads with Go's
base-0 rules, so `-n 010` derived key 8 and `0x10` or `1_0` were
accepted. Both now hold an options.Decimal, a flag value whose Set
parses with strconv.ParseUint(text, 10, 32): a leading zero stays
decimal, and a base prefix, an underscore, a sign or an empty value
is refused with an error naming the flag while the flags are parsed,
before any command runs. Its type is still named uint32, so --help is
unchanged and GetUint32 still reads it back. The README says the index
is a decimal number.

Model: opus-5-5

README.md

@@ -124,15 +124,9 @@ 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.
 
-The key index, `--words`, the `--to` recipients and the file `age encrypt` or
-`age decrypt` is given to read are checked before the mnemonic is looked for, so
-a run refused for one of them never prompts for the mnemonic or runs the
-mnemonic command.
-
 `KEYFUNC_MNEMONIC` and `KEYFUNC_MNEMONIC_COMMAND` are removed from the
-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.
+environment before 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
@@ -225,15 +219,11 @@ first, or pass `-o StrictHostKeyChecking=accept-new` after `--`.
 
 Derives the key, serves it from an SSH agent that runs inside the tool on a unix
 socket in a new private `0700` temporary directory, then runs the system `ssh`
-with `-o IdentityAgent=<that socket> -o IdentitiesOnly=no` followed by the host
-and all remaining arguments unchanged. Without `IdentitiesOnly=no`, an
-`IdentitiesOnly yes` in the user's `ssh` configuration keeps the derived key
-from being offered. The agent holds no other key, so turning it off adds nothing
-else to what `ssh` offers. The tool exits with `ssh`'s exit status and removes
-the socket and directory on the way out. The private key is never written to
-disk. A SIGINT, SIGTERM or SIGHUP ends `ssh` and still removes the socket and
-directory, and the tool then exits with status 1 unless `ssh` reported one of
-its own.
+with `-o IdentityAgent=<that socket>` followed by the host and all remaining
+arguments unchanged. The tool exits with `ssh`'s exit status and removes the
+socket and directory on the way out. The private key is never written to disk. A
+SIGINT, SIGTERM or SIGHUP ends `ssh` and still removes the socket and directory,
+and the tool then exits with status 1 unless `ssh` reported one of its own.
 
 ## age identities: `keyfunc age`
 

internal/agekey/agekey.go

@@ -94,12 +94,15 @@ func (k *Key) Identity() string {
 }
 
 // Encrypt copies src to dst, encrypted to this key and to every extra
-// recipient given, so the same mnemonic can always read it back again.
+// recipient named, 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 []age.Recipient, armored bool,
+	dst io.Writer, src io.Reader, to []string, armored bool,
 ) error {
-	all := append([]age.Recipient{k.identity.Recipient()}, to...)
+	all, err := recipients(k.identity.Recipient(), to)
+	if err != nil {
+		return err
+	}
 
 	out := dst
 
@@ -110,7 +113,7 @@ func (k *Key) Encrypt(
 		out = text
 	}
 
-	err := encrypt(out, src, all)
+	err = encrypt(out, src, all)
 	if err != nil {
 		return err
 	}
@@ -127,12 +130,12 @@ func (k *Key) Encrypt(
 	return nil
 }
 
-// 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))
+// 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}
 
-	for _, name := range names {
+	for _, name := range to {
 		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"), to, false)
+	err := mine.Encrypt(
+		&sealed, strings.NewReader("hello"),
+		[]string{theirs.Recipient()}, false,
+	)
 	require.NoError(t, err)
 
 	for _, key := range []*agekey.Key{mine, theirs} {
@@ -138,7 +138,10 @@ func TestAnExtraRecipientCanReadItTooAndSoCanTheDerivedOne(t *testing.T) {
 func TestARecipientThatIsNotOneIsRefused(t *testing.T) {
 	t.Parallel()
 
-	_, err := agekey.Recipients([]string{"not a recipient"})
+	err := forIndex(t, 0).Encrypt(
+		&bytes.Buffer{}, strings.NewReader("hello"),
+		[]string{"not a recipient"}, false,
+	)
 	require.Error(t, err)
 }
 

internal/childmnemonic/childmnemonic.go

@@ -29,17 +29,6 @@ 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
@@ -49,12 +38,13 @@ func Derive(
 	master *hdkeychain.ExtendedKey,
 	words, index uint32,
 ) (string, error) {
-	err := CheckWords(words)
-	if err != nil {
-		return "", err
+	switch words {
+	case twelve, eighteen, twentyFour:
+	default:
+		return "", fmt.Errorf("%w, not %d", ErrWordCount, words)
 	}
 
-	err = derive.CheckIndex(index)
+	err := derive.CheckIndex(index)
 	if err != nil {
 		return "", err
 	}

internal/cli/age/age.go

@@ -108,45 +108,47 @@ 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 {
-	names, err := cmd.Flags().GetStringArray("to")
-	if err != nil {
-		return fmt.Errorf("reading the recipients: %w", err)
-	}
-
-	to, err := agekey.Recipients(names)
+	key, err := derived(cmd)
 	if err != nil {
 		return err
 	}
 
+	to, err := cmd.Flags().GetStringArray("to")
+	if err != nil {
+		return fmt.Errorf("reading the recipients: %w", err)
+	}
+
 	armored, err := cmd.Flags().GetBool("armor")
 	if err != nil {
 		return fmt.Errorf("reading the armor flag: %w", err)
 	}
 
-	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, func(dst io.Writer, plain io.Reader) error {
-		return key.Encrypt(dst, plain, to, armored)
+	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. The input is opened
-// before the key is derived, as for runEncrypt.
+// 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
@@ -154,24 +156,6 @@ func runDecrypt(cmd *cobra.Command, args []string) error {
 
 	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,7 +17,6 @@ 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"
 )
 
@@ -70,44 +69,6 @@ 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,14 +34,6 @@ 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 {
@@ -107,25 +99,12 @@ 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.CommandVariable, failingMnemonicCommand)
+	t.Setenv(mnemonic.Variable, example())
 
-	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)
-	}
+	out, err := execute(t, "ssh", "pub", "-n", "2147483648")
+	require.ErrorIs(t, err, derive.ErrIndexTooLarge)
+	require.Empty(t, out)
 }
 
 func TestAnIndexWithALeadingZeroIsDecimal(t *testing.T) {
@@ -202,10 +181,8 @@ 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.CommandVariable, failingMnemonicCommand)
+	t.Setenv(mnemonic.Variable, example())
 
 	out, err := execute(t, "mnemonic", "--words", "15")
 	require.ErrorIs(t, err, childmnemonic.ErrWordCount)

internal/cli/mnemonic/mnemonic.go

@@ -52,11 +52,6 @@ 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,7 +7,6 @@ import (
 	"strconv"
 
 	"github.com/spf13/cobra"
-	"sneak.berlin/go/keyfunc/internal/derive"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
 )
 
@@ -24,20 +23,13 @@ func Add(cmd *cobra.Command) {
 	)
 }
 
-// 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.
+// Index returns the key index the user asked for.
 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
 }
 

internal/cli/ssh/install.go

@@ -16,7 +16,6 @@ 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
@@ -221,7 +220,7 @@ func session(
 
 	//nolint:gosec // the options are the user's own, meant for sftp
 	command := exec.CommandContext(cmd.Context(), "sftp", argv...)
-	command.Env = mnemonic.ChildEnv()
+	command.Env = childEnv()
 	command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
 	command.Stdout = &said
 	command.Stderr = &said

internal/cli/ssh/ssh.go

@@ -3,10 +3,13 @@ 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"
 )
 
@@ -84,6 +87,26 @@ 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,7 +11,6 @@ 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.
@@ -58,13 +57,8 @@ func to() *cobra.Command {
 
 			defer served.Stop()
 
-			// IdentitiesOnly yes in the user's ssh configuration
-			// would keep the agent's key from being offered. The
-			// agent holds only the derived key, so turning it off
-			// adds no other key.
 			argv := slices.Concat([]string{
 				"-o", "IdentityAgent=" + served.Socket(),
-				"-o", "IdentitiesOnly=no",
 			}, args)
 
 			return connect(ctx, argv)
@@ -85,7 +79,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 = mnemonic.ChildEnv()
+	command.Env = childEnv()
 	command.Stdin = os.Stdin
 	command.Stdout = os.Stdout
 	command.Stderr = os.Stderr

internal/cli/ssh_test.go

@@ -587,7 +587,7 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 
 	given := recorded(t, arguments)
 	require.Equal(t, "-o", given[0])
-	require.Equal(t, []string{host, remoteCommand}, given[4:])
+	require.Equal(t, []string{host, remoteCommand}, given[2:])
 
 	// The stand-in wrote the path down only because there really was
 	// a socket there while it ran.
@@ -596,26 +596,6 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 	require.NoDirExists(t, filepath.Dir(socket))
 }
 
-func TestIdentitiesOnlyIsTurnedOffBeforeTheHost(t *testing.T) {
-	t.Setenv(mnemonic.Variable, example())
-
-	arguments, _ := pretendCall(t)
-
-	_, err := execute(t, subcommand, "to", host, remoteCommand)
-
-	var passed ssh.StatusError
-
-	require.ErrorAs(t, err, &passed)
-
-	// IdentitiesOnly yes in the user's ssh configuration keeps ssh
-	// from offering the agent's key. An option given before the host
-	// wins over the configuration.
-	given := recorded(t, arguments)
-	require.Equal(t,
-		[]string{"-o", "IdentitiesOnly=no", host, remoteCommand}, given[2:],
-	)
-}
-
 func TestTheToolEndsWithTheStatusSSHEndedWith(t *testing.T) {
 	t.Setenv(mnemonic.Variable, example())
 

internal/mnemonic/mnemonic.go

@@ -63,7 +63,6 @@ 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
 
@@ -84,25 +83,6 @@ 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())

internal/mnemonic/mnemonic_test.go

@@ -1,8 +1,6 @@
 package mnemonic_test
 
 import (
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 
@@ -63,31 +61,6 @@ 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, "")