ssh install tells a missing .ssh from one it cannot enter (closes #10)

The first sftp connection now lists ~/.ssh before it fetches
authorized_keys. The file reads as empty in just two cases: sftp reports
~/.ssh itself as not there, or the listing succeeds and the fetch then
reports the file as not there. A directory that is there but cannot be
entered, or a file that cannot be read, fails the run and writes nothing,
so a ~/.ssh whose mode shuts the user out is no longer read as a host
with no file and replaced by one holding the new key alone. The write
connection makes ~/.ssh and sets 0700 only when the read found none; an
existing directory keeps its mode.

Model: opus-4-8

README.md

@@ -54,14 +54,8 @@ 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. `make build` stamps it; a binary
+`keyfunc --version` prints the version set at build time.
-installed with `go install` reports the module version instead.
 
 ## SSH keys: `keyfunc ssh`
 
@@ -146,9 +140,7 @@ 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. A
+socket and directory on the way out. The private key is never written to disk.
-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,13 +2,9 @@
 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"
@@ -17,43 +13,20 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// devVersion is what Version holds until a build stamps a real one.
+// Version is what --version prints. The build sets it.
-const devVersion = "dev"
-
-// Version is what --version prints. make build stamps it with -ldflags.
 //
 //nolint:gochecknoglobals // set at build time with -ldflags
-var Version = devVersion
+var Version = "dev"
-
-// 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:       resolveVersion(Version, info),
+		Version:       Version,
 		SilenceUsage:  true,
 		SilenceErrors: true,
 	}
@@ -69,19 +42,8 @@ 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 {
-	ctx, stop := signal.NotifyContext(
+	err := Root().Execute()
-		context.Background(),
-		syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP,
-	)
-	defer stop()
-
-	err := Root().ExecuteContext(ctx)
 	if err == nil {
 		return 0
 	}

internal/cli/ssh/install.go

@@ -166,7 +166,6 @@ 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

internal/cli/ssh/ssh.go

@@ -3,12 +3,9 @@ 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"
 )
@@ -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

@@ -7,7 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"slices"
-	"syscall"
 
 	"github.com/spf13/cobra"
 )
@@ -71,18 +70,10 @@ 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,14 +3,11 @@ 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"
@@ -18,23 +15,6 @@ 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 (
@@ -67,9 +47,6 @@ 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.
@@ -83,19 +60,13 @@ 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, writes down its
+// batch it is given, echoes each command as sftp does, and carries
-// own environment when a test asks for it, and carries the commands out
+// the commands out against a directory standing in for the host's
-// against a directory standing in for the host's home directory, so that
+// home directory, so that what keyfunc sends can be watched doing its
-// what keyfunc sends can be watched doing its work. A command that
+// work. A command that begins with a dash may fail; any other failure
-// begins with a dash may fail; any other failure ends the session, as it
+// ends the session, as it does in sftp's own batch mode.
-// does in sftp's own batch mode.
 //
 // 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.
@@ -110,7 +81,6 @@ const marker = "KEYFUNC_TEST_MARKER"
 // 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"
@@ -174,11 +144,9 @@ 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, writes down its own
+// there really is one at the path it was handed, and ends with the
-// environment when a test asks for it, and ends with the status the
+// status the test asked for.
-// 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
@@ -189,18 +157,6 @@ 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.
@@ -456,7 +412,7 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 
 	arguments, noted := pretendCall(t)
 
-	_, err := execute(t, subcommand, "to", host, remoteCommand)
+	_, err := execute(t, subcommand, "to", host, "uptime")
 
 	var passed ssh.StatusError
 
@@ -465,7 +421,7 @@ func TestSSHIsPointedAtTheAgentAndItsStatusIsHandedOn(t *testing.T) {
 
 	given := recorded(t, arguments)
 	require.Equal(t, "-o", given[0])
-	require.Equal(t, []string{host, remoteCommand}, given[2:])
+	require.Equal(t, []string{host, "uptime"}, given[2:])
 
 	// The stand-in wrote the path down only because there really was
 	// a socket there while it ran.
@@ -483,130 +439,11 @@ func TestTheToolEndsWithTheStatusSSHEndedWith(t *testing.T) {
 
 	t.Cleanup(func() { os.Args = given })
 
-	os.Args = []string{tool, subcommand, "to", host, remoteCommand}
+	os.Args = []string{tool, subcommand, "to", host, "uptime"}
 
 	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 {
@@ -755,28 +592,6 @@ 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

@@ -1,37 +0,0 @@
-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))
-	})
-}