Keep the mnemonic out of the ssh and sftp children (closes #16)
check / check (push) Failing after 0s

`keyfunc ssh to` and `keyfunc ssh install` started the system `ssh` and `sftp` with the tool's whole environment, so a mnemonic given in `KEYFUNC_MNEMONIC` stayed readable in the child's environment and could be forwarded to the host by a `SendEnv` line. Both children now get the environment with `KEYFUNC_MNEMONIC` and `KEYFUNC_MNEMONIC_COMMAND` removed, through one helper, `childEnv`, in the ssh cli package. The mnemonic command still runs with the full environment. Two tests drive the real commands against the stand-in `ssh` and `sftp` and check that a third variable still arrives.

Model: opus-4-8 (implementation, review); fable-5-1 (merge message)
This commit was merged in pull request #29.
This commit is contained in:
2026-09-21 14:58:26 +02:00
parent 3d90ac87f1
commit 64dcc7f42b
5 changed files with 98 additions and 7 deletions
+5
View File
@@ -54,6 +54,11 @@ 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 refuses and exits with status 1. A mnemonic that fails the BIP-39 checksum is
refused with a message saying so. refused with a message saying so.
`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.
Every command takes `--index` / `-n` and `--mnemonic-command`, and has `--help`. Every command takes `--index` / `-n` and `--mnemonic-command`, and has `--help`.
`keyfunc --version` prints the version. `make build` stamps it; a binary `keyfunc --version` prints the version. `make build` stamps it; a binary
installed with `go install` reports the module version instead. installed with `go install` reports the module version instead.
+1
View File
@@ -166,6 +166,7 @@ func session(
//nolint:gosec // the options are the user's own, meant for sftp //nolint:gosec // the options are the user's own, meant for sftp
command := exec.CommandContext(cmd.Context(), "sftp", argv...) command := exec.CommandContext(cmd.Context(), "sftp", argv...)
command.Env = childEnv()
command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n") command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
command.Stdout = &said command.Stdout = &said
command.Stderr = &said command.Stderr = &said
+23
View File
@@ -3,9 +3,12 @@ package ssh
import ( import (
"fmt" "fmt"
"os"
"strings"
"git.eeqj.de/sneak/keyfunc/internal/cli/options" "git.eeqj.de/sneak/keyfunc/internal/cli/options"
"git.eeqj.de/sneak/keyfunc/internal/derive" "git.eeqj.de/sneak/keyfunc/internal/derive"
"git.eeqj.de/sneak/keyfunc/internal/mnemonic"
"git.eeqj.de/sneak/keyfunc/internal/sshkey" "git.eeqj.de/sneak/keyfunc/internal/sshkey"
"github.com/spf13/cobra" "github.com/spf13/cobra"
) )
@@ -84,6 +87,26 @@ func write(cmd *cobra.Command, text string) error {
return nil 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. // addComment gives a command its comment flag.
func addComment(cmd *cobra.Command) { func addComment(cmd *cobra.Command) {
cmd.Flags().String( cmd.Flags().String(
+1
View File
@@ -70,6 +70,7 @@ func to() *cobra.Command {
func connect(ctx context.Context, argv []string) error { func connect(ctx context.Context, argv []string) error {
//nolint:gosec // the arguments are the user's own, meant for ssh //nolint:gosec // the arguments are the user's own, meant for ssh
command := exec.CommandContext(ctx, "ssh", argv...) command := exec.CommandContext(ctx, "ssh", argv...)
command.Env = childEnv()
command.Stdin = os.Stdin command.Stdin = os.Stdin
command.Stdout = os.Stdout command.Stdout = os.Stdout
command.Stderr = os.Stderr command.Stderr = os.Stderr
+68 -7
View File
@@ -60,13 +60,19 @@ const (
// an authorized_keys file. // an authorized_keys file.
const keyLine = vectorZero + " keyfunc/ssh/0\n" const keyLine = vectorZero + " keyfunc/ssh/0\n"
// marker is a variable set beside the mnemonic ones and expected to
// reach the stand-in, so a scrubbed environment is told apart from an
// empty one.
const marker = "KEYFUNC_TEST_MARKER"
// installer is a stand-in for the system sftp for the install // installer is a stand-in for the system sftp for the install
// command. It writes down the arguments and every command of the // command. It writes down the arguments and every command of the
// batch it is given, echoes each command as sftp does, and carries // batch it is given, echoes each command as sftp does, writes down its
// the commands out against a directory standing in for the host's // own environment when a test asks for it, and carries the commands out
// home directory, so that what keyfunc sends can be watched doing its // against a directory standing in for the host's home directory, so that
// work. A command that begins with a dash may fail; any other failure // what keyfunc sends can be watched doing its work. A command that
// ends the session, as it does in sftp's own batch mode. // begins with a dash may fail; any other failure ends the session, as it
// does in sftp's own batch mode.
// //
// The listing and the two ways a get can fail are worded as the // The listing and the two ways a get can fail are worded as the
// OpenSSH client words them, each naming the path the server expanded. // OpenSSH client words them, each naming the path the server expanded.
@@ -81,6 +87,7 @@ const keyLine = vectorZero + " keyfunc/ssh/0\n"
// draws the warning ssh writes for it, which carries the wording of a // draws the warning ssh writes for it, which carries the wording of a
// missing file into a session that goes on to authenticate. // missing file into a session that goes on to authenticate.
const installer = ` const installer = `
[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
previous= previous=
for argument in "$@"; do for argument in "$@"; do
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS" printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
@@ -144,9 +151,11 @@ done
// caller is a stand-in for the system ssh for the to command. It // caller is a stand-in for the system ssh for the to command. It
// writes down the arguments it was given, notes the agent socket if // writes down the arguments it was given, notes the agent socket if
// there really is one at the path it was handed, and ends with the // there really is one at the path it was handed, writes down its own
// status the test asked for. // environment when a test asks for it, and ends with the status the
// test asked for.
const caller = ` const caller = `
[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
for argument in "$@"; do for argument in "$@"; do
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS" printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
done done
@@ -444,6 +453,36 @@ func TestTheToolEndsWithTheStatusSSHEndedWith(t *testing.T) {
require.Equal(t, failingStatus, cli.Main()) require.Equal(t, failingStatus, cli.Main())
} }
func TestTheMnemonicIsNotHandedToSFTP(t *testing.T) {
t.Setenv(mnemonic.CommandVariable, "echo "+example())
t.Setenv(mnemonic.Variable, example())
t.Setenv(marker, "reaches the stand-in")
pretendHost(t)
environment := recordEnvironment(t)
install(t, host)
mnemonicWithheld(t, read(t, environment))
}
func TestTheMnemonicIsNotHandedToSSH(t *testing.T) {
t.Setenv(mnemonic.CommandVariable, "echo "+example())
t.Setenv(mnemonic.Variable, example())
t.Setenv(marker, "reaches the stand-in")
pretendCall(t)
environment := recordEnvironment(t)
_, err := execute(t, subcommand, "to", host, "uptime")
var passed ssh.StatusError
require.ErrorAs(t, err, &passed)
mnemonicWithheld(t, read(t, environment))
}
// pretendHost puts the install stand-in on the path and gives back the // pretendHost puts the install stand-in on the path and gives back the
// places it writes to. // places it writes to.
func pretendHost(t *testing.T) pretended { func pretendHost(t *testing.T) pretended {
@@ -592,6 +631,28 @@ func standIn(t *testing.T, name, body string) {
) )
} }
// recordEnvironment asks the stand-in to write its environment down and
// gives back the file it writes it to.
func recordEnvironment(t *testing.T) string {
t.Helper()
path := filepath.Join(t.TempDir(), "environment")
t.Setenv("KEYFUNC_TEST_ENVIRONMENT", path)
return path
}
// mnemonicWithheld requires that neither mnemonic variable reached the
// stand-in and that the marker set beside them did, so an empty
// environment does not pass for a scrubbed one.
func mnemonicWithheld(t *testing.T, environment string) {
t.Helper()
require.NotContains(t, environment, mnemonic.Variable+"=")
require.NotContains(t, environment, mnemonic.CommandVariable+"=")
require.Contains(t, environment, marker+"=")
}
// read returns what is in a file. // read returns what is in a file.
func read(t *testing.T, path string) string { func read(t *testing.T, path string) string {
t.Helper() t.Helper()