diff --git a/README.md b/README.md index 381ff1b..558dec4 100644 --- a/README.md +++ b/README.md @@ -328,10 +328,11 @@ 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). -While `age encrypt -o` or `age decrypt -o` is writing a new file or replacing a -regular one, the signal makes it remove the unfinished file, leave a file -already at the named path as it was, and exit with status 1. That holds for a -signal that has reached `keyfunc` when its input ends; a later one leaves the +At the prompt the terminal's settings are put back as they were first, so echo +is on again. While `age encrypt -o` or `age decrypt -o` is writing a new file or +replacing a regular one, the signal makes it remove the unfinished file, leave a +file already at the named path as it was, and exit with status 1. That holds for +a signal that has reached `keyfunc` when its input ends; a later one leaves the whole file in place. Ctrl-C on a pipeline ends the input at the same moment, and on Linux `keyfunc` sees the signal first, though no system promises that. A named pipe or a device at the `-o` path, or a path that is the same file as diff --git a/internal/cli/cli.go b/internal/cli/cli.go index dabad05..03a107e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -87,6 +87,9 @@ func init() { // still runs, and "age encrypt -o" and "age decrypt -o" while they // write a new file to rename over the named one, so the unfinished file // is removed. +// +// At the mnemonic prompt the signal is caught only to put the terminal +// back, and then ends the tool as above. func Main() int { err := Root().Execute() if err == nil { diff --git a/internal/cli/prompt_test.go b/internal/cli/prompt_test.go index 6090182..3c99dfb 100644 --- a/internal/cli/prompt_test.go +++ b/internal/cli/prompt_test.go @@ -6,26 +6,25 @@ import ( "os" "os/exec" "strings" + "syscall" "testing" "time" "github.com/creack/pty" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/term" "sneak.berlin/go/keyfunc/internal/mnemonic" ) -// TestEndOfInputAtThePromptEndsTheTool runs "age pub" with no mnemonic -// given and a terminal on all three streams, waits for the prompt, and -// types Ctrl-D. The tool has to end with status 1, say that the +// TestEndOfInputAtThePromptEndsTheTool starts "age pub" and, at the +// prompt, types Ctrl-D. The tool has to end with status 1, say that the // mnemonic is empty, and leave the terminal set as it found it, so with // echo on again. // // pty.Open returns the two ends of a new terminal: tty is the end the // tool uses as its terminal, and ptmx the end the test reads what the -// terminal shows from and types into. Reading ptmx stops at the -// deadline, so a prompt that never comes fails the test instead of -// hanging it. +// terminal shows from and types into. func TestEndOfInputAtThePromptEndsTheTool(t *testing.T) { t.Setenv(mnemonic.Variable, "") t.Setenv(mnemonic.CommandVariable, "") @@ -36,33 +35,10 @@ func TestEndOfInputAtThePromptEndsTheTool(t *testing.T) { defer func() { _ = ptmx.Close() }() defer func() { _ = tty.Close() }() - require.NoError(t, ptmx.SetReadDeadline(time.Now().Add(10*time.Second))) - before, err := term.GetState(int(tty.Fd())) require.NoError(t, err) - //nolint:gosec // this test's own binary as the tool - command := exec.CommandContext(t.Context(), os.Args[0], "age", "pub") - - command.Env = append(os.Environ(), runAsTool+"=1") - command.Stdin = tty - command.Stdout = tty - command.Stderr = tty - - require.NoError(t, command.Start()) - - var ( - shown string - char byte - ) - - terminal := bufio.NewReader(ptmx) - for !strings.HasSuffix(shown, "mnemonic: ") { - char, err = terminal.ReadByte() - require.NoError(t, err, "the tool did not prompt: %q", shown) - - shown += string(char) - } + command, terminal := prompted(t, ptmx, tty) _, err = ptmx.WriteString("\x04") // Ctrl-D require.NoError(t, err) @@ -81,3 +57,77 @@ func TestEndOfInputAtThePromptEndsTheTool(t *testing.T) { said, _ := io.ReadAll(terminal) require.Contains(t, string(said), mnemonic.ErrEmpty.Error()) } + +// TestASignalAtThePromptPutsTheTerminalBack starts "age pub" and, at +// the prompt, once the tool has turned echo off, sends it SIGINT. The +// tool has to be ended by SIGINT, which a shell reports as status 130, +// and leave the terminal set as it found it, so with echo on again. +func TestASignalAtThePromptPutsTheTerminalBack(t *testing.T) { + t.Setenv(mnemonic.Variable, "") + t.Setenv(mnemonic.CommandVariable, "") + + ptmx, tty, err := pty.Open() + require.NoError(t, err) + + defer func() { _ = ptmx.Close() }() + defer func() { _ = tty.Close() }() + + before, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + + command, _ := prompted(t, ptmx, tty) + + // The prompt is shown just before echo is turned off, and the + // signal has to come while it is off. + require.Eventually(t, func() bool { + now, err := term.GetState(int(tty.Fd())) + + return err == nil && !assert.ObjectsAreEqual(before, now) + }, 5*time.Second, 5*time.Millisecond, "the tool did not turn echo off") + + require.NoError(t, command.Process.Signal(syscall.SIGINT)) + waitForTool(t, "SIGINT at the prompt", command) + + status, ok := command.ProcessState.Sys().(syscall.WaitStatus) + require.True(t, ok) + require.True(t, status.Signaled(), "the tool was not ended by the signal") + require.Equal(t, syscall.SIGINT, status.Signal()) + + after, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + require.Equal(t, before, after, "the terminal settings were not restored") +} + +// prompted starts "age pub" as a subprocess, for a test that has set no +// mnemonic, with the terminal's tty end on all three streams, and +// returns once the tool has shown the prompt. The reader returned reads +// what the terminal shows from its ptmx end. Reading ptmx stops at a +// deadline, so a prompt that never comes fails the test instead of +// hanging it. +func prompted(t *testing.T, ptmx, tty *os.File) (*exec.Cmd, *bufio.Reader) { + t.Helper() + + require.NoError(t, ptmx.SetReadDeadline(time.Now().Add(10*time.Second))) + + //nolint:gosec // this test's own binary as the tool + command := exec.CommandContext(t.Context(), os.Args[0], "age", "pub") + + command.Env = append(os.Environ(), runAsTool+"=1") + command.Stdin = tty + command.Stdout = tty + command.Stderr = tty + + require.NoError(t, command.Start()) + + var shown string + + terminal := bufio.NewReader(ptmx) + for !strings.HasSuffix(shown, "mnemonic: ") { + char, err := terminal.ReadByte() + require.NoError(t, err, "the tool did not prompt: %q", shown) + + shown += string(char) + } + + return command, terminal +} diff --git a/internal/mnemonic/prompt_unix.go b/internal/mnemonic/prompt_unix.go index e7701b2..d52b4ed 100644 --- a/internal/mnemonic/prompt_unix.go +++ b/internal/mnemonic/prompt_unix.go @@ -8,8 +8,10 @@ import ( "fmt" "io" "os" + "os/signal" "golang.org/x/sys/unix" + "sneak.berlin/go/keyfunc/internal/cli/signals" ) // readWithoutEcho reads one line from the terminal on standard input @@ -17,6 +19,10 @@ import ( // input (Ctrl-D). The terminal is set as term.ReadPassword sets it; // ReadPassword itself is not used here because it never sees the end // of input and goes on waiting. +// +// Meanwhile SIGINT, SIGTERM and SIGHUP are caught, as signals.Notify +// catches them. One that arrives puts the terminal back as it was and +// then ends the tool, as it would have ended it if it were not caught. func readWithoutEcho() (string, error) { fd := int(os.Stdin.Fd()) @@ -25,6 +31,13 @@ func readWithoutEcho() (string, error) { return "", fmt.Errorf("reading the terminal settings: %w", err) } + // Caught from before echo is turned off, so that no signal can end + // the tool with echo off. + received := make(chan os.Signal, 1) + signals.Notify(received) + + defer signal.Stop(received) + hidden := *settings hidden.Lflag &^= unix.ECHO hidden.Lflag |= unix.ICANON | unix.ISIG @@ -35,14 +48,54 @@ func readWithoutEcho() (string, error) { return "", fmt.Errorf("turning echo off: %w", err) } - defer func() { _ = unix.IoctlSetTermios(fd, ioctlWriteTermios, settings) }() + restore := func() { + _ = unix.IoctlSetTermios(fd, ioctlWriteTermios, settings) + } + defer restore() - // With ICANON set, a read from the terminal returns at most one - // line, so the buffer takes nothing past the newline. - typed, err := bufio.NewReader(os.Stdin).ReadString('\n') - if errors.Is(err, io.EOF) { + var ( + typed string + failed error + ) + + read := make(chan struct{}) + + go func() { + // With ICANON set, a read from the terminal returns at most one + // line, so the buffer takes nothing past the newline. + typed, failed = bufio.NewReader(os.Stdin).ReadString('\n') + + close(read) + }() + + select { + case <-read: + case ending := <-received: + restore() + signal.Stop(received) + + return "", endBy(ending) + } + + if errors.Is(failed, io.EOF) { return typed, nil } - return typed, err + return typed, failed +} + +// endBy sends the tool the signal again, now that nothing catches it, +// so that it ends the tool, and waits for that. It returns only if the +// signal could not be sent. +func endBy(ending os.Signal) error { + tool, err := os.FindProcess(os.Getpid()) + if err == nil { + err = tool.Signal(ending) + } + + if err != nil { + return fmt.Errorf("ending the tool by %s: %w", ending, err) + } + + select {} }