Put the terminal back when a signal ends the mnemonic prompt (closes #91)

A SIGINT, SIGTERM or SIGHUP at the mnemonic prompt killed the tool
with echo still off. The prompt now catches them with signals.Notify
from before echo goes off, and reads the line in a goroutine. Once the
line is read or a signal arrives, it puts the terminal settings back
and stops catching; if a signal has arrived by then, it sends the tool
the same signal again, so the tool is still ended by it (130 for SIGINT
in a shell). A signal the tool was started with set to be ignored stays
ignored. A test sends SIGINT at the prompt on a pseudo-terminal.

Unchanged: the Windows prompt, which uses term.ReadPassword.

Model: opus-5-5

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
+At the prompt the terminal's settings are put back as they were first, so echo
-regular one, the signal makes it remove the unfinished file, leave a file
+is on again. While `age encrypt -o` or `age decrypt -o` is writing a new file or
-already at the named path as it was, and exit with status 1. That holds for a
+replacing a regular one, the signal makes it remove the unfinished file, leave a
-signal that has reached `keyfunc` when its input ends; a later one leaves the
+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

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 {

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
+// TestEndOfInputAtThePromptEndsTheTool starts "age pub" and, at the
-// given and a terminal on all three streams, waits for the prompt, and
+// prompt, types Ctrl-D. The tool has to end with status 1, say that the
-// 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
+// terminal shows from and types into.
-// deadline, so a prompt that never comes fails the test instead of
-// hanging it.
 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, terminal := prompted(t, ptmx, tty)
-	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)
-	}
 
 	_, 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
+}

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,11 @@ 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 has arrived by the time the line is read 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 +32,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 +49,63 @@ func readWithoutEcho() (string, error) {
 		return "", fmt.Errorf("turning echo off: %w", err)
 	}
 
-	defer func() { _ = unix.IoctlSetTermios(fd, ioctlWriteTermios, settings) }()
+	var (
+		typed  string
+		failed error
+	)
 
-	// With ICANON set, a read from the terminal returns at most one
+	read := make(chan struct{})
-	// line, so the buffer takes nothing past the newline.
+
-	typed, err := bufio.NewReader(os.Stdin).ReadString('\n')
+	go func() {
-	if errors.Is(err, io.EOF) {
+		// 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)
+	}()
+
+	var ending os.Signal
+
+	select {
+	case <-read:
+	case ending = <-received:
+	}
+
+	// The terminal is put back while the signals are still caught, so
+	// that none can end the tool with echo off. Stop returns only once
+	// every signal the tool has received has been handed over, so one
+	// that came as the line was read is in received by then.
+	_ = unix.IoctlSetTermios(fd, ioctlWriteTermios, settings)
+
+	signal.Stop(received)
+
+	if len(received) > 0 {
+		ending = <-received
+	}
+
+	if ending != nil {
+		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 {}
 }