Clean up the agent socket and working files when a signal ends the tool (closes #17)

Main ran the command tree on a background context, so SIGINT, SIGTERM or
SIGHUP killed the process before the deferred cleanup ran: the agent's
temporary directory and socket, and the install working directory, were
left behind.

Main now runs the tree on a signal.NotifyContext for those three signals.
A signal cancels the context, which ends the child ssh or sftp started
with exec.CommandContext, and the deferred cleanup then runs. The exit
status after a signal stays 1 unless ssh reported one of its own.

For "ssh to" the child is cancelled with SIGTERM rather than the default
kill, so ssh restores the terminal before it goes.

Model: opus-4-8

README.md

@@ -54,8 +54,14 @@ 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.
 
+`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`.
-`keyfunc --version` prints the version set at build time.
+`keyfunc --version` prints the version. `make build` stamps it; a binary
+installed with `go install` reports the module version instead.
 
 ## SSH keys: `keyfunc ssh`
 
@@ -94,22 +100,24 @@ Adds the `pub` line to `~/.ssh/authorized_keys` on the host. No command is run
 on the host: the file is fetched, changed here, and written back with the
 system `sftp` client in batch mode.
 
-The first connection fetches `~/.ssh/authorized_keys`. The file reads as empty
+The first connection lists `~/.ssh` and then fetches
-only when `sftp` reported that file as not being there — the one line naming
+`~/.ssh/authorized_keys` from it. The file reads as empty in two cases only:
-that path. The same wording anywhere else in the session does not count: `ssh`
+`sftp` reported `~/.ssh` itself as not being there, or the listing came up and
-writes `No such file or directory` about an `-i` it cannot find, on a session
+the file was not in it. Any other outcome of that connection fails the run — a
-that then authenticates through the agent. When `sftp` failed for any other
+`~/.ssh` that is there but cannot be entered, an `authorized_keys` that is there
-reason — the file is there and cannot be read, the connection did not come up —
+but cannot be read, or a connection that did not come up — and the tool prints
-the tool prints what `sftp` said and exits with status 1 without writing
+what `sftp` said and exits with status 1 without writing anything, rather than
-anything, rather than put a file back holding the new key alone. What `sftp`
+put a file back holding the new key alone. The listing is what tells a missing
-cannot tell apart is a missing file and one in a directory it cannot enter, so a
+directory from one shut to the user, which `sftp` reports on a fetch the same
-`~/.ssh` whose mode shuts the user out reads as a host with no file; the second
+way; the wording of a missing file elsewhere does not count either, since `ssh`
-connection sets that mode to `0700` and writes, as on a host that has none. If
+writes `No such file or directory` about an `-i` it cannot find on a session
-an identical line is already in the file, the tool prints
+that then authenticates through the agent. If an identical line is already in
-`already present` and connects no further. Otherwise the line is added (after a
+the file, the tool prints `already present` and connects no further. Otherwise
-newline, if the file did not end with one) and a second connection:
+the line is added (after a newline, if the file did not end with one) and a
+second connection:
 
-- creates `~/.ssh` and sets it to mode `0700`;
+- makes `~/.ssh` and sets it to mode `0700`, but only when the first connection
+  found none; a `~/.ssh` that was already there keeps the mode it had;
 - uploads the new file as `~/.ssh/authorized_keys.keyfunc-<random>` and sets it
   to mode `0600`;
 - renames that file over `~/.ssh/authorized_keys`.
@@ -138,7 +146,9 @@ 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>` 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.
+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/cli/cli.go

@@ -2,9 +2,13 @@
 package cli
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
+	"os/signal"
+	"runtime/debug"
+	"syscall"
 
 	"git.eeqj.de/sneak/keyfunc/internal/cli/age"
 	"git.eeqj.de/sneak/keyfunc/internal/cli/mnemonic"
@@ -13,20 +17,43 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// Version is what --version prints. The build sets it.
+// devVersion is what Version holds until a build stamps a real one.
+const devVersion = "dev"
+
+// Version is what --version prints. make build stamps it with -ldflags.
 //
 //nolint:gochecknoglobals // set at build time with -ldflags
-var Version = "dev"
+var Version = devVersion
+
+// resolveVersion chooses what --version reports. A value stamped at
+// build time wins. Otherwise, for a binary from go install, the module
+// version recorded in the build info is used, unless that is empty or
+// the "(devel)" of a local build. When neither names a version, the
+// "dev" fallback stays.
+func resolveVersion(stamped string, info *debug.BuildInfo) string {
+	if stamped != devVersion {
+		return stamped
+	}
+
+	if info != nil && info.Main.Version != "" &&
+		info.Main.Version != "(devel)" {
+		return info.Main.Version
+	}
+
+	return devVersion
+}
 
 // Root returns the whole command tree.
 func Root() *cobra.Command {
+	info, _ := debug.ReadBuildInfo()
+
 	root := &cobra.Command{
 		Use:   "keyfunc",
 		Short: "derive key pairs from a BIP-39 mnemonic",
 		Long: "keyfunc turns a BIP-39 mnemonic into key pairs that can " +
 			"be recreated from that mnemonic at any time. The same " +
 			"mnemonic, key type and index always give the same key.",
-		Version:       Version,
+		Version:       resolveVersion(Version, info),
 		SilenceUsage:  true,
 		SilenceErrors: true,
 	}
@@ -42,8 +69,19 @@ func Root() *cobra.Command {
 // status of its own, which "ssh to" uses to hand on the status ssh
 // ended with. ssh has already said whatever it had to say in that
 // case, so nothing more is printed.
+//
+// SIGINT, SIGTERM and SIGHUP cancel the command's context instead of
+// killing the process outright, so the child ssh or sftp ends and the
+// deferred cleanup that removes the agent socket and the install
+// working directory still runs.
 func Main() int {
-	err := Root().Execute()
+	ctx, stop := signal.NotifyContext(
+		context.Background(),
+		syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP,
+	)
+	defer stop()
+
+	err := Root().ExecuteContext(ctx)
 	if err == nil {
 		return 0
 	}

internal/cli/ssh/install.go

@@ -76,7 +76,7 @@ func add(cmd *cobra.Command, host string, options []string, line string) error {
 
 	defer func() { _ = os.RemoveAll(work) }()
 
-	content, err := fetch(cmd, host, options,
+	content, present, err := fetch(cmd, host, options,
 		filepath.Join(work, "authorized_keys"),
 	)
 	if err != nil {
@@ -88,16 +88,18 @@ func add(cmd *cobra.Command, host string, options []string, line string) error {
 		return write(cmd, "already present\n")
 	}
 
-	return upload(cmd, host, options, work, merged)
+	return upload(cmd, host, options, work, merged, present)
 }
 
 // upload writes the new file to the host and renames it over
 // authorized_keys, which is the step that either happens or does not.
 // Nothing is removed when a step fails: the file left behind is named
-// so that it can be looked at and cleared away by hand.
+// so that it can be looked at and cleared away by hand. The directory
+// is made and set to its mode only when the read found none: an .ssh
+// that was already there is left with the mode it had.
 func upload(
 	cmd *cobra.Command, host string, options []string,
-	work, merged string,
+	work, merged string, present bool,
 ) error {
 	local := filepath.Join(work, "authorized_keys.merged")
 
@@ -111,14 +113,24 @@ func upload(
 		return err
 	}
 
-	// The mkdir may fail: the directory is usually there already.
+	var batch []string
-	said, err := session(cmd, host, options, []string{
+
-		"-mkdir " + directory,
+	if !present {
-		"chmod " + directoryMode + " " + directory,
+		// The mkdir is allowed to fail in case the directory appeared
-		"put " + quoted(local) + " " + sidecar,
+		// between the read and now; the chmod then sets its mode.
-		"chmod " + fileMode + " " + sidecar,
+		batch = append(batch,
-		"rename " + sidecar + " " + authorized,
+			"-mkdir "+directory,
-	})
+			"chmod "+directoryMode+" "+directory,
+		)
+	}
+
+	batch = append(batch,
+		"put "+quoted(local)+" "+sidecar,
+		"chmod "+fileMode+" "+sidecar,
+		"rename "+sidecar+" "+authorized,
+	)
+
+	said, err := session(cmd, host, options, batch)
 	if err != nil {
 		// sftp echoes each command as it runs it and stops at the
 		// first that fails, so the name is in what it said only once
@@ -154,6 +166,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.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
 	command.Stdout = &said
 	command.Stderr = &said
@@ -185,31 +198,82 @@ func merge(content, line string) (string, bool) {
 }
 
 // fetch brings the host's authorized_keys into the given path and
-// returns what is in it. A host that has no such file reads as empty,
+// returns what is in it, and whether the .ssh directory was already
-// but only when that is what sftp said about it: a file that is there
+// there. The one session lists .ssh and then gets the file, so the
-// and cannot be read fails the run, because writing back over it
+// listing settles the state of the directory before the get is read.
-// would leave the host with the new key and nothing else.
+//
+// The file reads as empty in just two cases: sftp reported .ssh itself
+// as not there, or the listing succeeded and the get then reported the
+// file as not there. Anything else — the listing refused, the file
+// there but unreadable, the connection down — fails the run and writes
+// nothing, because writing back over what was not read would leave the
+// host with the new key and nothing else. sftp cannot tell a missing
+// file from one in a directory it cannot enter, so the listing does:
+// a directory that is there but cannot be read is a failure, not an
+// empty file.
 func fetch(
 	cmd *cobra.Command, host string, options []string, into string,
-) (string, error) {
+) (string, bool, error) {
 	said, err := session(cmd, host, options, []string{
+		"ls -1 " + directory,
 		"get " + authorized + " " + quoted(into),
 	})
 	if err != nil {
-		if absent(said) {
+		if directoryAbsent(said) {
-			return "", nil
+			return "", false, nil
 		}
 
-		return "", err
+		if absent(said) {
+			return "", true, nil
+		}
+
+		return "", false, err
 	}
 
 	//nolint:gosec // the path is a temporary file of the tool's own
 	content, err := os.ReadFile(into)
 	if err != nil {
-		return "", fmt.Errorf("reading the fetched file: %w", err)
+		return "", false, fmt.Errorf("reading the fetched file: %w", err)
 	}
 
-	return string(content), nil
+	return string(content), true, nil
+}
+
+// directoryAbsent says whether sftp reported .ssh itself as not being
+// there, which is the one listing failure read as a host that has no
+// authorized_keys yet. The reading is taken only from the line in which
+// sftp reports on that directory: any other failure of the listing, in
+// particular a directory that is there but cannot be entered, is left
+// as a failure, so that no key is written to a host whose keys were
+// never read.
+func directoryAbsent(said string) bool {
+	for line := range strings.Lines(said) {
+		named, is := reportedCannotList(strings.TrimSpace(line))
+		if is && (named == directory ||
+			strings.HasSuffix(named, "/"+directory)) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// reportedCannotList returns the path an sftp line reports it cannot
+// list for want of the directory, and whether the line is such a
+// report. The client writes this one wording when the directory a
+// listing names is not there, giving the path the server expanded.
+func reportedCannotList(line string) (string, bool) {
+	const (
+		before = `Can't ls: "`
+		after  = `" not found`
+	)
+
+	if !strings.HasPrefix(line, before) ||
+		!strings.HasSuffix(line, after) {
+		return "", false
+	}
+
+	return strings.TrimSuffix(strings.TrimPrefix(line, before), after), true
 }
 
 // absent says whether sftp reported the file that was asked for as

internal/cli/ssh/install_test.go

@@ -10,6 +10,7 @@ import "testing"
 const (
 	echoed = `sftp> get .ssh/authorized_keys "/tmp/keyfunc/authorized_keys"
 `
+	listed  = "sftp> ls -1 .ssh\n"
 	warning = `Warning: Identity file /gone not accessible: ` +
 		"No such file or directory.\n"
 )
@@ -76,3 +77,57 @@ func TestAbsenceIsReadOnlyFromWhatSFTPSaidAboutAuthorizedKeys(t *testing.T) {
 		})
 	}
 }
+
+// TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo holds the
+// wordings the OpenSSH client was seen to use when a listing fails: a
+// directory it cannot find is reported one way, and one it cannot enter
+// another, and only the first is read as a host with no .ssh yet.
+func TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo(t *testing.T) {
+	t.Parallel()
+
+	listings := map[string]struct {
+		said string
+		want bool
+	}{
+		"the directory is not there": {
+			said: listed + `Can't ls: "/home/someone/.ssh" not found` + "\n",
+			want: true,
+		},
+		"the directory is not there, named as it was asked for": {
+			said: listed + `Can't ls: ".ssh" not found` + "\n",
+			want: true,
+		},
+		"the directory is not there and an identity file is not either": {
+			said: warning + listed +
+				`Can't ls: "/home/someone/.ssh" not found` + "\n",
+			want: true,
+		},
+		"the directory is there and cannot be entered": {
+			said: listed +
+				`remote readdir("/home/someone/.ssh/"): Permission denied` + "\n",
+			want: false,
+		},
+		"some other directory is not there": {
+			said: listed + `Can't ls: "/home/someone/.config" not found` + "\n",
+			want: false,
+		},
+		"the connection did not come up": {
+			said: "ssh: connect to host example.com port 22: " +
+				"Connection refused\nConnection closed\n",
+			want: false,
+		},
+	}
+
+	for name, listing := range listings {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			if directoryAbsent(listing.said) != listing.want {
+				t.Errorf(
+					"read as absent: %t, wanted %t, from:\n%s",
+					!listing.want, listing.want, listing.said,
+				)
+			}
+		})
+	}
+}

internal/cli/ssh/ssh.go

@@ -3,9 +3,12 @@ package ssh
 
 import (
 	"fmt"
+	"os"
+	"strings"
 
 	"git.eeqj.de/sneak/keyfunc/internal/cli/options"
 	"git.eeqj.de/sneak/keyfunc/internal/derive"
+	"git.eeqj.de/sneak/keyfunc/internal/mnemonic"
 	"git.eeqj.de/sneak/keyfunc/internal/sshkey"
 	"github.com/spf13/cobra"
 )
@@ -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

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"slices"
+	"syscall"
 
 	"github.com/spf13/cobra"
 )
@@ -70,10 +71,18 @@ 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.Stdin = os.Stdin
 	command.Stdout = os.Stdout
 	command.Stderr = os.Stderr
 
+	// A cancelled context means a signal ended the tool. Send ssh a
+	// SIGTERM rather than the default kill, so it puts the terminal
+	// back the way it found it before it goes.
+	command.Cancel = func() error {
+		return command.Process.Signal(syscall.SIGTERM)
+	}
+
 	err := command.Run()
 	if err == nil {
 		return nil

internal/cli/ssh_test.go

@@ -3,11 +3,14 @@ package cli_test
 import (
 	"bytes"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
+	"syscall"
 	"testing"
+	"time"
 
 	"git.eeqj.de/sneak/keyfunc/internal/cli"
 	"git.eeqj.de/sneak/keyfunc/internal/cli/ssh"
@@ -15,6 +18,23 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// runAsTool, set in the environment of a re-executed test binary, tells
+// TestMain to run the tool through Main rather than the suite, so the
+// signal test can drive the real signal path in a process it can send a
+// signal to.
+const runAsTool = "KEYFUNC_TEST_RUN_AS_TOOL"
+
+// TestMain re-executes the test binary as the tool when runAsTool is
+// set, and otherwise runs the suite. The signal test starts the tool
+// this way, as a subprocess it can signal and watch clean up.
+func TestMain(m *testing.M) {
+	if os.Getenv(runAsTool) == "1" {
+		os.Exit(cli.Main())
+	}
+
+	os.Exit(m.Run())
+}
+
 // The modes the host is supposed to end up with, and the mode the
 // stand-ins need so that they can be run at all.
 const (
@@ -47,6 +67,9 @@ const (
 	keptIn    = "authorized_keys"
 )
 
+// remoteCommand is the command the "to" tests hand ssh after the host.
+const remoteCommand = "uptime"
+
 // The tool's own name, as it stands in the arguments a test hands to
 // Main, the ssh subcommand both commands the tests here drive live
 // under, and the one of those two these tests name most.
@@ -60,22 +83,34 @@ const (
 // an authorized_keys file.
 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
 // 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 two ways a get can fail are worded as the OpenSSH client words
+// The listing and the two ways a get can fail are worded as the
-// them, both naming the path the server expanded: a file that is not
+// OpenSSH client words them, each naming the path the server expanded.
-// there, which is the one failure the tool reads as an empty file, and
+// A listing fails one way when .ssh is not there and another when it is
-// a file that is there and cannot be read, which is not. An -i naming
+// there but shut to the user; the first is the only failure read as a
-// a file that is not here draws the warning ssh writes for it, which
+// host with no file. A get fails one way for a file that is not there,
-// carries the wording of a missing file into a session that goes on to
+// which after a listing that came up empty is also read as no file, and
-// authenticate.
+// another for a file that is there and cannot be read, which is a
+// failure. A directory shut to the user is stood in for by mode 000,
+// which the listing reads off the mode itself so that the test does not
+// turn on the user it runs as. An -i naming a file that is not here
+// draws the warning ssh writes for it, which carries the wording of a
+// missing file into a session that goes on to authenticate.
 const installer = `
+[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
 previous=
 for argument in "$@"; do
 	printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
@@ -99,6 +134,23 @@ while IFS= read -r line; do
 	eval "set -- $line"
 	worked=yes
 	case "$1" in
+	ls)
+		dir=$2
+		[ "$dir" = -1 ] && dir=$3
+		if [ ! -e "$home/$dir" ]; then
+			worked=no
+			printf 'Can'\''t ls: "%s" not found\n' "$home/$dir" >&2
+		elif [ -d "$home/$dir" ] && [ "$(stat -c '%a' "$home/$dir")" = 0 ]; then
+			worked=no
+			printf 'remote readdir("%s/"): Permission denied\n' \
+				"$home/$dir" >&2
+		else
+			for entry in "$home/$dir"/*; do
+				[ -e "$entry" ] || continue
+				printf '%s/%s\n' "$dir" "$(basename "$entry")"
+			done
+		fi
+		;;
 	get)
 		if [ ! -e "$home/$2" ]; then
 			worked=no
@@ -122,9 +174,11 @@ done
 
 // 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
-// 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 = `
+[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
 for argument in "$@"; do
 	printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
 done
@@ -135,6 +189,18 @@ fi
 exit "$KEYFUNC_TEST_STATUS"
 `
 
+// sleeper is a stand-in for the system ssh that notes the agent socket
+// and then blocks, so a test can cancel the context while it is running
+// and watch the tool take the agent down. The wait ends on its own only
+// as a backstop, well after the test has cancelled and looked.
+const sleeper = `
+socket=${2#IdentityAgent=}
+if [ -S "$socket" ]; then
+	printf '%s\n' "$socket" > "$KEYFUNC_TEST_SOCKET"
+fi
+sleep 5
+`
+
 // pretended is where a stand-in writes down what it was asked to do.
 type pretended struct {
 	// home stands in for the home directory on the host.
@@ -176,8 +242,8 @@ func TestAKeyThatIsAlreadyThereIsLeftAlone(t *testing.T) {
 	require.Equal(t, "already present\n", install(t, host))
 	require.Equal(t, "somebody else\n"+keyLine, read(t, path))
 
-	// The fetch and nothing after it: the tool did not connect again.
+	// The read and nothing after it: the tool did not connect again.
-	require.Len(t, recorded(t, pretend.batch), 1)
+	require.Equal(t, 1, connections(t, pretend))
 }
 
 func TestAnEmptyFileGetsTheKeyAndNoBlankLineBeforeIt(t *testing.T) {
@@ -218,7 +284,9 @@ func TestTheFileIsUploadedBesideTheOldOneAndThenRenamedOverIt(t *testing.T) {
 		strings.HasPrefix(beside, ".ssh/authorized_keys.keyfunc-"),
 	)
 
-	require.True(t, strings.HasPrefix(sent[0], "get .ssh/authorized_keys "))
+	// The listing fails on a host with no .ssh, so the get never runs;
+	// the write session then makes the directory and puts the file.
+	require.Equal(t, "ls -1 .ssh", sent[0])
 	require.Equal(t, "-mkdir .ssh", sent[1])
 	require.Equal(t, "chmod 700 .ssh", sent[2])
 	require.Equal(t, "put", strings.Fields(sent[3])[0])
@@ -237,12 +305,57 @@ func TestAFileThatCannotBeReadIsNotWrittenOver(t *testing.T) {
 	require.Empty(t, printed)
 	require.Contains(t, said, "Permission denied")
 
-	// The fetch and nothing after it, and what was on the host is
+	// The read and nothing after it, and what was on the host is
 	// still what is on the host.
-	require.Len(t, recorded(t, pretend.batch), 1)
+	require.Equal(t, 1, connections(t, pretend))
 	require.DirExists(t, unreadable)
 }
 
+func TestAnUnreadableDirectoryIsNotWrittenInto(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	pretend := pretendHost(t)
+	unlistable(t, pretend)
+
+	// The listing is refused, which is not the same as no directory, so
+	// the tool writes nothing rather than treat a directory it cannot
+	// enter as a host with no file.
+	printed, said, err := attempt(t, host)
+	require.Error(t, err)
+	require.Empty(t, printed)
+	require.Contains(t, said, "Permission denied")
+
+	// The read and nothing after it: no second connection wrote a key.
+	require.Equal(t, 1, connections(t, pretend))
+}
+
+func TestAnExistingDirectoryKeepsItsModeAndIsNotRemade(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	pretend := pretendHost(t)
+
+	// A directory that is there but holds no file yet, made with a mode
+	// of its own so that a stray chmod would show.
+	const ownMode = 0o755
+
+	directory := filepath.Join(pretend.home, keptUnder)
+	require.NoError(t, os.Mkdir(directory, ownMode))
+
+	require.Equal(t, "added\n", install(t, host))
+
+	// The key is added and the directory keeps the mode it had: the
+	// write session neither made it nor set its mode.
+	require.Equal(t, keyLine, read(t, filepath.Join(directory, keptIn)))
+
+	kept, err := os.Stat(directory)
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(ownMode), kept.Mode().Perm())
+
+	sent := recorded(t, pretend.batch)
+	require.NotContains(t, sent, "-mkdir .ssh")
+	require.NotContains(t, sent, "chmod 700 .ssh")
+}
+
 func TestAWarningAboutAnotherFileIsNotTakenForTheOneAskedFor(t *testing.T) {
 	t.Setenv(mnemonic.Variable, example())
 
@@ -259,7 +372,7 @@ func TestAWarningAboutAnotherFileIsNotTakenForTheOneAskedFor(t *testing.T) {
 	require.Contains(t, said, "No such file or directory")
 	require.Contains(t, said, "Permission denied")
 
-	require.Len(t, recorded(t, pretend.batch), 1)
+	require.Equal(t, 1, connections(t, pretend))
 	require.DirExists(t, unreadable)
 
 	// The same run again, this way for the status it ends with.
@@ -279,9 +392,9 @@ func TestAFailedStepNamesTheUploadedFileAndChangesNothing(t *testing.T) {
 
 	pretend := pretendHost(t)
 
-	// A file where the .ssh directory belongs: nothing is there to
+	// A file where the .ssh directory belongs: the listing shows it and
-	// fetch, and then the put has nowhere to put anything, so the
+	// so the directory reads as already there, but then the put has
-	// write session ends at the put.
+	// nowhere to put anything, so the write session ends at the put.
 	inTheWay := filepath.Join(pretend.home, keptUnder)
 	require.NoError(t,
 		os.WriteFile(inTheWay, []byte(notADirectory), fileMode),
@@ -292,12 +405,12 @@ func TestAFailedStepNamesTheUploadedFileAndChangesNothing(t *testing.T) {
 	require.Empty(t, printed)
 	require.Contains(t, said, "put failed")
 
-	// The put is the last command the session got to, and the file it
+	// The put is the first and last command the write session got to,
-	// was uploading is the one the message names.
+	// and the file it was uploading is the one the message names.
 	sent := recorded(t, pretend.batch)
-	require.Len(t, sent, 4)
+	require.Len(t, sent, 3)
-	require.Equal(t, "put", strings.Fields(sent[3])[0])
+	require.Equal(t, "put", strings.Fields(sent[2])[0])
-	require.Contains(t, err.Error(), strings.Fields(sent[3])[2])
+	require.Contains(t, err.Error(), strings.Fields(sent[2])[2])
 
 	require.Equal(t, notADirectory, read(t, inTheWay))
 
@@ -343,7 +456,7 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 
 	arguments, noted := pretendCall(t)
 
-	_, err := execute(t, subcommand, "to", host, "uptime")
+	_, err := execute(t, subcommand, "to", host, remoteCommand)
 
 	var passed ssh.StatusError
 
@@ -352,7 +465,7 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 
 	given := recorded(t, arguments)
 	require.Equal(t, "-o", given[0])
-	require.Equal(t, []string{host, "uptime"}, given[2:])
+	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.
@@ -370,11 +483,130 @@ func TestTheToolEndsWithTheStatusSSHEndedWith(t *testing.T) {
 
 	t.Cleanup(func() { os.Args = given })
 
-	os.Args = []string{tool, subcommand, "to", host, "uptime"}
+	os.Args = []string{tool, subcommand, "to", host, remoteCommand}
 
 	require.Equal(t, failingStatus, cli.Main())
 }
 
+func TestASignalTakesTheAgentDirectoryDown(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	// The three signals the tool handles, checked one after another.
+	signals := []struct {
+		name   string
+		signal os.Signal
+	}{
+		{"SIGTERM", syscall.SIGTERM},
+		{"SIGINT", syscall.SIGINT},
+		{"SIGHUP", syscall.SIGHUP},
+	}
+
+	for _, ending := range signals {
+		signalEndsTheTool(t, ending.name, ending.signal)
+	}
+}
+
+// signalEndsTheTool runs the tool as a subprocess against a stand-in
+// ssh that blocks, waits until the agent is up and ssh is running
+// against it, sends the tool the signal, and requires the agent socket
+// and its directory to be gone once the tool has ended. The subprocess
+// goes through Main and its signal handling, so with that handling
+// removed the signal kills the tool outright, no deferred cleanup runs,
+// the directory is left behind, and the check fails.
+func signalEndsTheTool(t *testing.T, name string, signal os.Signal) {
+	t.Helper()
+
+	noted := filepath.Join(t.TempDir(), "socket")
+	t.Setenv("KEYFUNC_TEST_SOCKET", noted)
+	standIn(t, "ssh", sleeper)
+
+	//nolint:gosec // the binary is this test's own, re-run as the tool
+	command := exec.CommandContext(
+		t.Context(), os.Args[0], subcommand, "to", host, remoteCommand,
+	)
+
+	command.Env = append(os.Environ(), runAsTool+"=1")
+	require.NoError(t, command.Start())
+
+	// The stand-in notes the socket only once the agent is up and ssh
+	// is running against it, so this is where the signal lands.
+	socket := waitForSocket(t, noted)
+
+	require.NoError(t, command.Process.Signal(signal))
+	waitForTool(t, name, command)
+
+	// The signal ended the tool, and its deferred cleanup still ran:
+	// the agent socket and its directory are gone.
+	require.NoDirExists(t, filepath.Dir(socket), name)
+}
+
+// waitForTool waits for the subprocess to end, and fails the test if it
+// does not end in time.
+func waitForTool(t *testing.T, name string, command *exec.Cmd) {
+	t.Helper()
+
+	done := make(chan error, 1)
+	go func() { done <- command.Wait() }()
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatalf("the tool did not end after %s", name)
+	}
+}
+
+// waitForSocket waits for the stand-in to write down the agent socket
+// and gives back the path, which means the agent is up and ssh is
+// running against it.
+func waitForSocket(t *testing.T, noted string) string {
+	t.Helper()
+
+	var socket string
+
+	require.Eventually(t, func() bool {
+		content, err := os.ReadFile(noted) //nolint:gosec // test path
+		if err != nil {
+			return false
+		}
+
+		socket = strings.TrimSpace(string(content))
+
+		return socket != ""
+	}, 5*time.Second, 5*time.Millisecond)
+
+	return socket
+}
+
+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, remoteCommand)
+
+	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
 // places it writes to.
 func pretendHost(t *testing.T) pretended {
@@ -455,6 +687,38 @@ func unfetchable(t *testing.T, pretend pretended) string {
 	return path
 }
 
+// unlistable puts a .ssh on the stand-in host that is there but shut to
+// the user, a directory of mode 000, and gives back its path. Its mode
+// is put back before the temporary directory is cleared so that it can
+// be.
+func unlistable(t *testing.T, pretend pretended) string {
+	t.Helper()
+
+	directory := filepath.Join(pretend.home, keptUnder)
+	require.NoError(t, os.Mkdir(directory, directoryMode))
+	require.NoError(t, os.Chmod(directory, 0))
+
+	t.Cleanup(func() { _ = os.Chmod(directory, directoryMode) })
+
+	return directory
+}
+
+// connections returns how many times the tool ran sftp, counted from
+// the -b that opens each session's arguments.
+func connections(t *testing.T, pretend pretended) int {
+	t.Helper()
+
+	count := 0
+
+	for _, argument := range recorded(t, pretend.arguments) {
+		if argument == "-b" {
+			count++
+		}
+	}
+
+	return count
+}
+
 // pretendCall puts the to stand-in on the path and gives back the file
 // the arguments are written down in and the file the agent socket is
 // noted in.
@@ -491,6 +755,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.
 func read(t *testing.T, path string) string {
 	t.Helper()

internal/cli/version_internal_test.go

@@ -0,0 +1,37 @@
+package cli
+
+import (
+	"runtime/debug"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestResolveVersion(t *testing.T) {
+	t.Parallel()
+
+	release := &debug.BuildInfo{Main: debug.Module{Version: "v1.2.3"}}
+	local := &debug.BuildInfo{Main: debug.Module{Version: "(devel)"}}
+	empty := &debug.BuildInfo{}
+
+	t.Run("stamped value wins over build info", func(t *testing.T) {
+		t.Parallel()
+		require.Equal(t, "v0.1.0", resolveVersion("v0.1.0", release))
+	})
+
+	t.Run("go install reports the module version", func(t *testing.T) {
+		t.Parallel()
+		require.Equal(t, "v1.2.3", resolveVersion(devVersion, release))
+	})
+
+	t.Run("a local build stays dev", func(t *testing.T) {
+		t.Parallel()
+		require.Equal(t, devVersion, resolveVersion(devVersion, local))
+	})
+
+	t.Run("no version anywhere stays dev", func(t *testing.T) {
+		t.Parallel()
+		require.Equal(t, devVersion, resolveVersion(devVersion, empty))
+		require.Equal(t, devVersion, resolveVersion(devVersion, nil))
+	})
+}