Signals end every command, not only ssh to and ssh install (closes #48)

SIGINT, SIGTERM and SIGHUP were caught for the whole run, but only the
ssh and sftp children acted on them: the mnemonic prompt waited for
Enter, and an interrupted `age encrypt -o` went on to put the
encryption of the cut-off input in place. Now `ssh to` and
`ssh install` catch them from once the mnemonic is read until their
cleanup has run, and `age encrypt -o` and `age decrypt -o` catch them
while they write, to remove the unfinished file and exit with status 1;
everywhere else they end the tool at once. Tests cover an interrupted
`age encrypt -o` and `age decrypt -o` and the install working directory
on a signal.

Model: opus-5-5

README.md

@@ -292,13 +292,10 @@ 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. It puts the file in place a tenth of a second
-after its input ends, and a signal in that time still counts: Ctrl-C on a
-pipeline also ends the program feeding it, so the input can end just before the
-signal arrives. 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.
+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
 

internal/cli/age/age.go

@@ -3,14 +3,12 @@
 package age
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"os"
 	"os/signal"
 	"path/filepath"
 	"syscall"
-	"time"
 
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/agekey"
@@ -18,18 +16,6 @@ import (
 	"sneak.berlin/go/keyfunc/internal/derive"
 )
 
-// ErrInterrupted is returned when SIGINT, SIGTERM or SIGHUP came before
-// the file --output names was put in place.
-var ErrInterrupted = errors.New(
-	"interrupted by a signal; the output file was left as it was",
-)
-
-// signalWait is how long after the work has ended a signal still keeps
-// the new file from being put in place. Ctrl-C on "producer | keyfunc
-// age encrypt -o file" ends the producer as well, and the end of the
-// input can reach the work a moment before the signal reaches the tool.
-const signalWait = 100 * time.Millisecond
-
 // Command returns the age command and everything under it.
 func Command() *cobra.Command {
 	group := &cobra.Command{
@@ -144,9 +130,8 @@ func runDecrypt(cmd *cobra.Command, args []string) error {
 	return through(cmd, args, key.Decrypt)
 }
 
-// through opens the input the arguments ask for and hands it to the
-// work, with the file --output names to write to, or the command's own
-// output when it names none.
+// through opens the input and the output the arguments ask for, hands
+// them to the work, and finishes the output afterwards either way.
 func through(
 	cmd *cobra.Command, args []string,
 	work func(io.Writer, io.Reader) error,
@@ -158,16 +143,14 @@ func through(
 
 	defer closeSrc()
 
-	name, err := cmd.Flags().GetString("output")
+	dst, done, err := output(cmd)
 	if err != nil {
-		return fmt.Errorf("reading the output file: %w", err)
+		return err
 	}
 
-	if name == "" {
-		return work(cmd.OutOrStdout(), src)
-	}
+	err = work(dst, src)
 
-	return output(name, src, work)
+	return done(err)
 }
 
 // input returns what to read from: the named file, or the command's
@@ -186,45 +169,57 @@ func input(cmd *cobra.Command, args []string) (io.Reader, func(), error) {
 	return file, func() { _ = file.Close() }, nil
 }
 
-// output has the work write a new file beside the named one, and puts
-// the new file in the named file's place only when the work succeeded,
-// so a file that is already there survives a run that failed.
-//
-// Meanwhile SIGINT, SIGTERM and SIGHUP are caught. One that comes while
-// the work runs, or within signalWait after it has ended, wins: the new
-// file is removed and ErrInterrupted returned at once, without waiting
-// for the work, which may be blocked reading its input.
-func output(
-	name string, src io.Reader, work func(io.Writer, io.Reader) error,
-) error {
+// output returns what to write to: a new file beside the one --output
+// names, or the command's own output when it names none. The second
+// result finishes the write, and is given whatever the work returned:
+// the new file takes the named file's place only when the work
+// succeeded, so a file that is already there survives a run that
+// failed.
+func output(cmd *cobra.Command) (io.Writer, func(error) error, error) {
+	name, err := cmd.Flags().GetString("output")
+	if err != nil {
+		return nil, nil, fmt.Errorf("reading the output file: %w", err)
+	}
+
+	if name == "" {
+		return cmd.OutOrStdout(), func(failed error) error {
+			return failed
+		}, 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)
 
-	defer signal.Stop(signals)
-
 	// 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 {
-		return fmt.Errorf("creating a file beside %s: %w", name, err)
+		signal.Stop(signals)
+
+		return nil, nil, fmt.Errorf("creating a file beside %s: %w", name, err)
 	}
 
-	worked := make(chan error, 1)
+	go func() {
+		if _, received := <-signals; received {
+			_ = os.Remove(file.Name())
 
-	go func() { worked <- work(file, src) }()
-
-	select {
-	case failed := <-worked:
-		select {
-		case <-signals:
-		case <-time.After(signalWait):
-			return finish(file, name, failed)
+			os.Exit(1)
 		}
-	case <-signals:
-	}
+	}()
 
-	return finish(file, name, ErrInterrupted)
+	return file, func(failed error) error {
+		finished := finish(file, name, failed)
+
+		signal.Stop(signals)
+		close(signals)
+
+		return finished
+	}, nil
 }
 
 // finish closes the new file and puts it in the named file's place, or

internal/cli/age_test.go

@@ -100,7 +100,7 @@ func TestASignalStopsAnEncryptionAndLeavesNoFile(t *testing.T) {
 	for _, ending := range []os.Signal{
 		syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP,
 	} {
-		interrupted(t, ending, "encrypt", "the start of the secret\n", false)
+		interrupted(t, ending, "encrypt", "the start of the secret\n")
 	}
 }
 
@@ -115,38 +115,19 @@ func TestASignalStopsADecryptionAndLeavesNoFile(t *testing.T) {
 	for _, ending := range []os.Signal{
 		syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP,
 	} {
-		interrupted(t, ending, "decrypt", cut, false)
-	}
-}
-
-func TestASignalAsTheInputEndsLeavesNoFile(t *testing.T) {
-	t.Setenv(mnemonic.Variable, example())
-
-	sealed := run(t, "age", "encrypt", written(t, "notes.txt", "the secret\n"))
-
-	// Ctrl-C on "producer | keyfunc age encrypt -o file" ends the
-	// producer too, so the input ends just as the signal comes, with
-	// enough of it in hand for a whole encryption or decryption. Which
-	// of the two reaches the tool first varies, so it is tried often.
-	for range 25 {
-		interrupted(t, syscall.SIGINT, "encrypt", "the start of the secret\n", true)
-		interrupted(t, syscall.SIGINT, "decrypt", sealed, true)
+		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. It waits until the tool has begun
-// writing the file beside the one it was named, and sends it the
-// signal, then ends the input if endInput says so and otherwise leaves
-// it open. The tool has to end 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; one that put that file in
-// place because the input ended would leave the named file.
-func interrupted(
-	t *testing.T, signal os.Signal, operation, input string, endInput bool,
-) {
+// 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()
@@ -176,11 +157,6 @@ func interrupted(
 	}, 5*time.Second, 5*time.Millisecond)
 
 	require.NoError(t, command.Process.Signal(signal))
-
-	if endInput {
-		require.NoError(t, producer.Close())
-	}
-
 	waitForTool(t, name, command)
 
 	require.Equal(t, 1, command.ProcessState.ExitCode(), name)