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. When one
arrives it puts the terminal settings back, stops catching, and 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
-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

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
-// 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
+}

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 {}
 }