End the mnemonic prompt at end of input (closes #78)

term.ReadPassword reads the terminal with a raw read that returns 0
bytes at the end of input and takes that as nothing yet, so Ctrl-D at
the prompt did nothing. On the Unix systems the prompt now sets the
terminal as ReadPassword does and reads the line from standard input,
which reports the end of input; elsewhere, such as Windows, it still
uses ReadPassword. Ctrl-D with nothing typed gives an empty mnemonic,
refused with status 1, and the terminal settings are put back.

A test runs the tool on a pseudo-terminal (github.com/creack/pty, at a
commit past v1.1.24, whose Open on Linux can return another pty's
terminal) and types Ctrl-D at the prompt.

Model: opus-5-5

README.md

@@ -65,12 +65,10 @@ calls into `internal/`. The packages there are:
   and decrypts with it.
 - `internal/childmnemonic` derives a child mnemonic from the main one using
   BIP-85's own mnemonic application.
-- `internal/signals` catches SIGINT, SIGTERM and SIGHUP for the commands that
-  clean up before they end, and for the mnemonic prompt, which puts the terminal
-  back before the signal ends the tool.
 - `internal/cli` builds the cobra command tree and runs it. Under it,
-  `cli/options` holds the flags every command shares, and `cli/ssh`, `cli/age`
+  `cli/options` holds the flags every command shares, `cli/signals` catches
-  and `cli/mnemonic` are the command groups.
+  SIGINT, SIGTERM and SIGHUP for the commands that clean up before they end, and
+  `cli/ssh`, `cli/age` and `cli/mnemonic` are the command groups.
 - `internal/bip39` is a copy of `github.com/tyler-smith/go-bip39` v1.1.0,
   trimmed to what keyfunc uses.
 
@@ -330,11 +328,10 @@ 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).
-At the prompt the terminal's settings are put back as they were first, so echo
+While `age encrypt -o` or `age decrypt -o` is writing a new file or replacing a
-is on again. While `age encrypt -o` or `age decrypt -o` is writing a new file or
+regular one, the signal makes it remove the unfinished file, leave a file
-replacing a regular one, the signal makes it remove the unfinished file, leave a
+already at the named path as it was, and exit with status 1. That holds for a
-file already at the named path as it was, and exit with status 1. That holds for
+signal that has reached `keyfunc` when its input ends; a later one leaves the
-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/age/age.go

@@ -16,8 +16,8 @@ import (
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/keyfunc/internal/agekey"
 	"sneak.berlin/go/keyfunc/internal/cli/options"
+	"sneak.berlin/go/keyfunc/internal/cli/signals"
 	"sneak.berlin/go/keyfunc/internal/derive"
-	"sneak.berlin/go/keyfunc/internal/signals"
 )
 
 // ErrInterrupted is returned when SIGINT, SIGTERM or SIGHUP has been

internal/cli/cli.go

@@ -87,9 +87,6 @@ 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,25 +6,26 @@ 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 starts "age pub" and, at the
+// TestEndOfInputAtThePromptEndsTheTool runs "age pub" with no mnemonic
-// prompt, types Ctrl-D. The tool has to end with status 1, say that the
+// 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
 // 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.
+// 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.
 func TestEndOfInputAtThePromptEndsTheTool(t *testing.T) {
 	t.Setenv(mnemonic.Variable, "")
 	t.Setenv(mnemonic.CommandVariable, "")
@@ -35,10 +36,33 @@ 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)
 
-	command, terminal := prompted(t, ptmx, tty)
+	//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)
+	}
 
 	_, err = ptmx.WriteString("\x04") // Ctrl-D
 	require.NoError(t, err)
@@ -57,77 +81,3 @@ 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/cli/signals/signals.go

@@ -1,9 +1,5 @@
-// Package signals catches SIGINT, SIGTERM and SIGHUP, the signals that
+// Package signals catches the signals that end the tool, for the
-// end the tool, for the commands that clean up before they end and for
+// commands that clean up before they end.
-// the mnemonic prompt, which puts the terminal back before the signal
-// ends the tool. Go keeps an inherited ignore only for SIGHUP and
-// SIGINT, so a SIGTERM the tool was started with set to be ignored is
-// not left ignored.
 package signals
 
 import (
@@ -15,8 +11,8 @@ import (
 
 // Context is signal.NotifyContext for SIGINT, SIGTERM and SIGHUP: the
 // context it returns is cancelled when one of them arrives, and stop
-// stops catching them. It leaves out SIGHUP or SIGINT if the tool was
+// stops catching them. It leaves out any of the three the tool was
-// started with it set to be ignored, as nohup does with SIGHUP, because
+// started with set to be ignored, as nohup does with SIGHUP, because
 // catching a signal turns an ignored one back on and would end a run
 // that was meant to survive it.
 func Context(parent context.Context) (context.Context, context.CancelFunc) {
@@ -41,8 +37,8 @@ func Notify(c chan<- os.Signal) {
 	}
 }
 
-// caught returns SIGINT, SIGTERM and SIGHUP, less any that
+// caught returns those of SIGINT, SIGTERM and SIGHUP that the tool was
-// signal.Ignored reports as ignored.
+// not started with set to be ignored.
 func caught() []os.Signal {
 	endings := []os.Signal{syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP}
 	kept := make([]os.Signal, 0, len(endings))

internal/cli/ssh/install.go

@@ -15,8 +15,8 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"sneak.berlin/go/keyfunc/internal/cli/signals"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
-	"sneak.berlin/go/keyfunc/internal/signals"
 )
 
 // Where the key goes on the host and what the file it arrives in is

internal/cli/ssh/to.go

@@ -10,8 +10,8 @@ import (
 	"syscall"
 
 	"github.com/spf13/cobra"
+	"sneak.berlin/go/keyfunc/internal/cli/signals"
 	"sneak.berlin/go/keyfunc/internal/mnemonic"
-	"sneak.berlin/go/keyfunc/internal/signals"
 )
 
 // StatusError says the tool should end with the status ssh ended with.

internal/mnemonic/prompt_unix.go

@@ -8,10 +8,8 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/signal"
 
 	"golang.org/x/sys/unix"
-	"sneak.berlin/go/keyfunc/internal/signals"
 )
 
 // readWithoutEcho reads one line from the terminal on standard input
@@ -19,11 +17,6 @@ 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())
 
@@ -32,13 +25,6 @@ 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
@@ -49,63 +35,14 @@ func readWithoutEcho() (string, error) {
 		return "", fmt.Errorf("turning echo off: %w", err)
 	}
 
-	var (
+	defer func() { _ = unix.IoctlSetTermios(fd, ioctlWriteTermios, settings) }()
-		typed  string
-		failed error
-	)
 
-	read := make(chan struct{})
+	// With ICANON set, a read from the terminal returns at most one
-
+	// line, so the buffer takes nothing past the newline.
-	go func() {
+	typed, err := bufio.NewReader(os.Stdin).ReadString('\n')
-		// With ICANON set, a read from the terminal returns at most one
+	if errors.Is(err, io.EOF) {
-		// 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, failed
+	return typed, err
-}
-
-// 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 {}
 }