Signals end every command, not only ssh to and ssh install (closes #48)
check / check (push) Successful in 2m20s
check / check (push) Successful in 2m20s
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
This commit is contained in:
@@ -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
|
||||
|
||||
+25
-1
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+8
-14
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
},
|
||||
}
|
||||
|
||||
+14
-3
@@ -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 {
|
||||
|
||||
@@ -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())
|
||||
|
||||
Reference in New Issue
Block a user