diff --git a/README.md b/README.md index 3f9ff56..d412b80 100644 --- a/README.md +++ b/README.md @@ -288,6 +288,15 @@ girl mad pet galaxy egg matter matrix prison refuse sense ordinary nose Errors go to standard error and the exit status is 1, except for `ssh to`, which passes through `ssh`'s own exit status. +SIGINT, SIGTERM and SIGHUP end any command at once, at the mnemonic prompt too, +with the status a shell gives a program killed by that signal (130 for SIGINT). +An interrupted `age encrypt -o` or `age decrypt -o` leaves no file: it removes +the unfinished file it was writing, leaves a file already at the named path as +it was, and exits with status 1. While `ssh to` or `ssh install` has `ssh` or +`sftp` running, the signal ends that program instead, the tool removes its agent +socket or working files, and it exits with status 1, or for `ssh to` with +`ssh`'s own status if `ssh` reported one. + ## Entrypoints The repo adheres to the diff --git a/internal/cli/age/age.go b/internal/cli/age/age.go index 700fdf2..e5fe1b4 100644 --- a/internal/cli/age/age.go +++ b/internal/cli/age/age.go @@ -6,7 +6,9 @@ import ( "fmt" "io" "os" + "os/signal" "path/filepath" + "syscall" "github.com/spf13/cobra" "sneak.berlin/go/keyfunc/internal/agekey" @@ -185,16 +187,38 @@ func output(cmd *cobra.Command) (io.Writer, func(error) error, error) { }, nil } + // From before the new file is made until it is renamed or removed, + // a signal removes it and ends the tool with status 1, even while + // the work is blocked reading its input, so an interrupted run + // leaves no file. + signals := make(chan os.Signal, 1) + signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + // The file is made in the same directory so that putting it in // place is a rename and never a copy, and it is readable only by // its owner, which is the mode it keeps once renamed. file, err := os.CreateTemp(filepath.Dir(name), filepath.Base(name)+".") if err != nil { + signal.Stop(signals) + return nil, nil, fmt.Errorf("creating a file beside %s: %w", name, err) } + go func() { + if _, received := <-signals; received { + _ = os.Remove(file.Name()) + + os.Exit(1) + } + }() + return file, func(failed error) error { - return finish(file, name, failed) + finished := finish(file, name, failed) + + signal.Stop(signals) + close(signals) + + return finished }, nil } diff --git a/internal/cli/age_test.go b/internal/cli/age_test.go index 1b37231..ae05202 100644 --- a/internal/cli/age_test.go +++ b/internal/cli/age_test.go @@ -1,10 +1,14 @@ package cli_test import ( + "io" "os" + "os/exec" "path/filepath" "strings" + "syscall" "testing" + "time" "github.com/stretchr/testify/require" "sneak.berlin/go/keyfunc/internal/agekey" @@ -90,6 +94,78 @@ func TestARefusedDecryptionLeavesTheOutputFileAlone(t *testing.T) { require.Equal(t, "what was already there\n", string(kept)) } +func TestASignalStopsAnEncryptionAndLeavesNoFile(t *testing.T) { + t.Setenv(mnemonic.Variable, example()) + + for _, ending := range []os.Signal{ + syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP, + } { + interrupted(t, ending, "encrypt", "the start of the secret\n") + } +} + +func TestASignalStopsADecryptionAndLeavesNoFile(t *testing.T) { + t.Setenv(mnemonic.Variable, example()) + + // All of an encryption but its last byte, so the tool reads the + // header and then waits for the rest. + sealed := run(t, "age", "encrypt", written(t, "notes.txt", "the secret\n")) + cut := sealed[:len(sealed)-1] + + for _, ending := range []os.Signal{ + syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP, + } { + interrupted(t, ending, "decrypt", cut) + } +} + +// interrupted runs "age encrypt -o" or "age decrypt -o", as the +// operation says, as a subprocess writing into a directory of its own +// and reading the input from a pipe that stays open. It waits until the +// tool has begun writing the file beside the one it was named, and +// sends it the signal. The tool has to end on the signal alone, with +// status 1, and leave the directory empty. A tool that went on reading +// would not end until the input did; one that did not remove the file +// it was writing would leave it there, with what it had written so far. +func interrupted(t *testing.T, signal os.Signal, operation, input string) { + t.Helper() + + name := operation + " " + signal.String() + directory := t.TempDir() + + //nolint:gosec // the binary is this test's own, re-run as the tool + command := exec.CommandContext( + t.Context(), os.Args[0], "age", operation, + "-o", filepath.Join(directory, "notes"), + ) + + command.Env = append(os.Environ(), runAsTool+"=1") + + producer, err := command.StdinPipe() + require.NoError(t, err) + require.NoError(t, command.Start()) + + _, err = io.WriteString(producer, input) + require.NoError(t, err) + + // The file beside the named one is made once the mnemonic has been + // read, before any input is. + require.Eventually(t, func() bool { + entries, err := os.ReadDir(directory) + + return err == nil && len(entries) > 0 + }, 5*time.Second, 5*time.Millisecond) + + require.NoError(t, command.Process.Signal(signal)) + waitForTool(t, name, command) + + require.Equal(t, 1, command.ProcessState.ExitCode(), name) + + left, err := os.ReadDir(directory) + require.NoError(t, err) + require.Empty(t, left, name) +} + // written puts the contents in a file of that name in a directory of // this test's own and returns the path to it. func written(t *testing.T, name, contents string) string { diff --git a/internal/cli/cli.go b/internal/cli/cli.go index e49c853..631acae 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -2,13 +2,10 @@ package cli import ( - "context" "errors" "fmt" "os" - "os/signal" "runtime/debug" - "syscall" "github.com/spf13/cobra" "sneak.berlin/go/keyfunc/internal/cli/age" @@ -70,18 +67,15 @@ func Root() *cobra.Command { // 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. +// SIGINT, SIGTERM and SIGHUP end the tool at once, as they end any Go +// program, so a command waiting at the mnemonic prompt or reading what +// it encrypts or decrypts goes no further. The exceptions catch the +// signals to clean up first: "ssh to" and "ssh install" while they +// have ssh or sftp running, so the child ends and their own cleanup +// still runs, and "age encrypt -o" and "age decrypt -o" while they +// write, so the unfinished file is removed. func Main() int { - ctx, stop := signal.NotifyContext( - context.Background(), - syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, - ) - defer stop() - - err := Root().ExecuteContext(ctx) + err := Root().Execute() if err == nil { return 0 } diff --git a/internal/cli/ssh/install.go b/internal/cli/ssh/install.go index 162bd23..5891d39 100644 --- a/internal/cli/ssh/install.go +++ b/internal/cli/ssh/install.go @@ -8,9 +8,11 @@ import ( "fmt" "os" "os/exec" + "os/signal" "path/filepath" "slices" "strings" + "syscall" "github.com/spf13/cobra" ) @@ -62,6 +64,17 @@ func install() *cobra.Command { return err } + // From here on a signal cancels the context, which + // sftp runs under, instead of ending the tool, so sftp + // ends and the working directory is still removed. + ctx, stop := signal.NotifyContext( + cmd.Context(), + syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, + ) + defer stop() + + cmd.SetContext(ctx) + return add(cmd, args[0], args[1:], line) }, } diff --git a/internal/cli/ssh/to.go b/internal/cli/ssh/to.go index 6569c5c..b5ecaa9 100644 --- a/internal/cli/ssh/to.go +++ b/internal/cli/ssh/to.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "os/signal" "slices" "syscall" @@ -42,7 +43,17 @@ func to() *cobra.Command { return err } - served, err := key.Serve(cmd.Context(), comment) + // From here until the agent is taken down, a signal + // cancels the context instead of ending the tool, so + // ssh ends and the socket and its directory are still + // removed. + ctx, stop := signal.NotifyContext( + cmd.Context(), + syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, + ) + defer stop() + + served, err := key.Serve(ctx, comment) if err != nil { return err } @@ -53,7 +64,7 @@ func to() *cobra.Command { "-o", "IdentityAgent=" + served.Socket(), }, args) - return connect(cmd.Context(), argv) + return connect(ctx, argv) }, } @@ -76,7 +87,7 @@ func connect(ctx context.Context, argv []string) error { command.Stdout = os.Stdout command.Stderr = os.Stderr - // A cancelled context means a signal ended the tool. Send ssh a + // A cancelled context means a signal arrived. 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 { diff --git a/internal/cli/ssh_test.go b/internal/cli/ssh_test.go index 04749f4..4776237 100644 --- a/internal/cli/ssh_test.go +++ b/internal/cli/ssh_test.go @@ -20,13 +20,13 @@ import ( // 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. +// signal tests can drive the real signal path in a process they 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. +// set, and otherwise runs the suite. The signal tests start the tool +// this way, as a subprocess they can signal and watch end. func TestMain(m *testing.M) { if os.Getenv(runAsTool) == "1" { os.Exit(cli.Main()) @@ -209,6 +209,16 @@ fi sleep 5 ` +// stalled is a stand-in for the system sftp that notes it has started +// and then blocks, so a test can signal the tool while sftp is running +// and watch it remove its working directory. The exec keeps the shell +// from leaving a sleep behind that holds the output the tool reads sftp +// through. +const stalled = ` +touch "$KEYFUNC_TEST_STARTED" +exec 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. @@ -544,7 +554,7 @@ func TestASignalTakesTheAgentDirectoryDown(t *testing.T) { // 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 +// goes through Main and the command's 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) { @@ -611,6 +621,56 @@ func waitForSocket(t *testing.T, noted string) string { return socket } +func TestASignalTakesTheInstallWorkingDirectoryDown(t *testing.T) { + t.Setenv(mnemonic.Variable, example()) + + for _, ending := range []os.Signal{ + syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP, + } { + signalEndsTheInstall(t, ending.String(), ending) + } +} + +// signalEndsTheInstall runs "ssh install" as a subprocess against a +// stand-in sftp that blocks, with a temporary directory of the test's +// own, waits until sftp is running, sends the tool the signal, and +// requires the working directory the tool made there to be gone once +// the tool has ended. +func signalEndsTheInstall(t *testing.T, name string, signal os.Signal) { + t.Helper() + + temporary := t.TempDir() + started := filepath.Join(t.TempDir(), "started") + t.Setenv("KEYFUNC_TEST_STARTED", started) + standIn(t, "sftp", stalled) + + //nolint:gosec // the binary is this test's own, re-run as the tool + command := exec.CommandContext( + t.Context(), os.Args[0], subcommand, installing, host, + ) + + command.Env = append(os.Environ(), runAsTool+"=1", "TMPDIR="+temporary) + require.NoError(t, command.Start()) + + // sftp is started only once the working directory has been made. + require.Eventually(t, func() bool { + _, err := os.Stat(started) + + return err == nil + }, 5*time.Second, 5*time.Millisecond) + + working, err := os.ReadDir(temporary) + require.NoError(t, err) + require.Len(t, working, 1, name) + + require.NoError(t, command.Process.Signal(signal)) + waitForTool(t, name, command) + + left, err := os.ReadDir(temporary) + require.NoError(t, err) + require.Empty(t, left, name) +} + func TestTheMnemonicIsNotHandedToSFTP(t *testing.T) { t.Setenv(mnemonic.CommandVariable, "echo "+example()) t.Setenv(mnemonic.Variable, example())