Keep the mnemonic variables from the mnemonic command (closes #80)

The mnemonic command ran with the tool's whole environment, so a
mnemonic set in KEYFUNC_MNEMONIC reached it, although the command is
there to supply one of its own. It now gets the environment ssh and
sftp already get, with KEYFUNC_MNEMONIC and KEYFUNC_MNEMONIC_COMMAND
removed. The function that builds that environment moves from the ssh
command group into internal/mnemonic, beside the two variable names,
so all three programs use the one copy. The README says so.

A mnemonic command that runs keyfunc itself no longer finds the main
mnemonic in either variable.

Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>

README.md

@@ -100,7 +100,9 @@ m/83696968'/<app>'/<n>'
   to one tool. Each key type's number is spelled the way BIP-85 spells its own
   RSA application (`828365` is the ASCII codes of `R`, `S`, `A` written out):
   SSH is `838372` (`S` `S` `H`), age is `657169` (`A` `G` `E`).
-- `n` is the key index: flag `--index` / `-n`, default `0`.
+- `n` is the key index, a decimal number: flag `--index` / `-n`, default `0`. A
+  leading zero is still decimal (`-n 010` is index 10); a base prefix, an
+  underscore or a sign (`0x10`, `1_0`, `+1`) is refused.
 
 ## Giving it the mnemonic
 
@@ -123,8 +125,9 @@ 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/cli_test.go

@@ -2,6 +2,7 @@ package cli_test
 
 import (
 	"bytes"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -106,6 +107,56 @@ func TestAnIndexWithNoHardenedChildIsRefused(t *testing.T) {
 	require.Empty(t, out)
 }
 
+func TestAnIndexWithALeadingZeroIsDecimal(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	line := run(t, "ssh", "pub", "-n", "010")
+	require.Equal(t, run(t, "ssh", "pub", "-n", "10"), line)
+	require.True(t,
+		strings.HasSuffix(strings.TrimSpace(line), " keyfunc/ssh/10"),
+	)
+}
+
+// The mnemonic command in these runs fails, so a refusal naming the
+// flag shows that the number was refused before the mnemonic was
+// looked for.
+func TestAnIndexThatIsNotDecimalDigitsIsRefused(t *testing.T) {
+	t.Parallel()
+
+	for _, index := range []string{"0x10", "1_0", "+1", "-1", ""} {
+		t.Run(index, func(t *testing.T) {
+			t.Parallel()
+
+			out, err := execute(t,
+				"ssh", "pub", "-n", index, "--mnemonic-command", "exit 1",
+			)
+			require.ErrorIs(t, err, strconv.ErrSyntax)
+			require.ErrorContains(t, err, "--index")
+			require.Empty(t, out)
+		})
+	}
+}
+
+// Neither a command group nor --version reads the index, so these
+// show that it is refused while the flags are parsed.
+func TestAnIndexNotInDecimalIsRefusedBeforeAnyCommandRuns(t *testing.T) {
+	t.Parallel()
+
+	for _, args := range [][]string{
+		{"ssh", "-n", "0x10"},
+		{"-n", "0x10", "--version"},
+	} {
+		t.Run(strings.Join(args, " "), func(t *testing.T) {
+			t.Parallel()
+
+			out, err := execute(t, args...)
+			require.ErrorIs(t, err, strconv.ErrSyntax)
+			require.ErrorContains(t, err, "--index")
+			require.Empty(t, out)
+		})
+	}
+}
+
 func TestTheMnemonicCommandIsUsed(t *testing.T) {
 	t.Setenv(mnemonic.Variable, "")
 
@@ -138,6 +189,22 @@ func TestALengthTheToolDoesNotOfferIsRefused(t *testing.T) {
 	require.Empty(t, out)
 }
 
+func TestTheWordCountIsDecimal(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	words := strings.Fields(run(t, "mnemonic", "--words", "012"))
+	require.Len(t, words, twelve)
+
+	// As for the index, the failing mnemonic command shows that the
+	// number was refused first.
+	out, err := execute(t,
+		"mnemonic", "--words", "0x18", "--mnemonic-command", "exit 1",
+	)
+	require.ErrorIs(t, err, strconv.ErrSyntax)
+	require.ErrorContains(t, err, "--words")
+	require.Empty(t, out)
+}
+
 // run executes the tool with the given arguments and returns what it
 // wrote to standard output.
 func run(t *testing.T, args ...string) string {

internal/cli/mnemonic/mnemonic.go

@@ -31,8 +31,9 @@ func Command() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().Uint32(
-		"words", childmnemonic.DefaultWords,
+	words := options.Decimal(childmnemonic.DefaultWords)
+	cmd.Flags().Var(
+		&words, "words",
 		"how long the child mnemonic is: 12, 18 or 24 words",
 	)
 

internal/cli/options/options.go

@@ -4,6 +4,7 @@ package options
 
 import (
 	"fmt"
+	"strconv"
 
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
@@ -16,8 +17,8 @@ func Add(cmd *cobra.Command) {
 		"mnemonic-command", "",
 		"shell command whose output is the mnemonic",
 	)
-	cmd.PersistentFlags().Uint32P(
-		"index", "n", 0,
+	cmd.PersistentFlags().VarP(
+		new(Decimal), "index", "n",
 		"which key to derive",
 	)
 }
@@ -41,3 +42,33 @@ func Mnemonic(cmd *cobra.Command) (string, error) {
 
 	return mnemonic.Read(cmd.Context(), command)
 }
+
+// Decimal is a flag value for a number written in decimal digits only.
+// The flag library's own uint32 flags read 010 as octal 8 and also
+// take 0x10 or 1_0.
+type Decimal uint32
+
+// String returns the number in decimal digits.
+func (d *Decimal) String() string {
+	return strconv.FormatUint(uint64(*d), 10)
+}
+
+// Set reads text as a decimal number that fits in 32 bits: a leading
+// zero is still decimal, and a base prefix, an underscore or a sign is
+// refused.
+func (d *Decimal) Set(text string) error {
+	number, err := strconv.ParseUint(text, 10, 32)
+	if err != nil {
+		return err
+	}
+
+	*d = Decimal(number)
+
+	return nil
+}
+
+// Type is what --help shows as the flag's type, and what GetUint32
+// checks before it reads the value back.
+func (d *Decimal) Type() string {
+	return "uint32"
+}

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

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

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, "")