Stop the terminal tests reading a pty nobody opened (closes #126)
check / check (push) Successful in 3m27s
check / check (push) Successful in 3m27s
On Linux, pty.Open of github.com/creack/pty v1.1.24 passes the address of a local variable to ioctl as a plain number, through a function call. When Go moved the goroutine's stack in between, the kernel wrote the pty's number to the old place, and pty.Open opened /dev/pts/0, another terminal. secret rm wrote there, and the test read a terminal no program had open, which never ends. Require the commit on the library's main branch that passes a pointer; no release has it yet. Both terminal tests now stop reading when their one-minute context ends, and fail saying what was still waiting. Model: opus-5-5
This commit was merged in pull request #127.
This commit is contained in:
@@ -18,6 +18,18 @@ https://git.eeqj.de/sneak/secret/milestone/12
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-06: `TestRemoveIgnoresTerminalOnStdout` and
|
||||||
|
`TestRemoveAsksAtTerminalOnStdin` no longer wait until Go's test timeout
|
||||||
|
(https://git.eeqj.de/sneak/secret/issues/126). On Linux, `pty.Open` of
|
||||||
|
`github.com/creack/pty` v1.1.24 passed the address of a local variable to the
|
||||||
|
`ioctl` system call as a plain number, through a function call; when Go moved
|
||||||
|
the goroutine's stack in between, the kernel wrote the terminal's number to
|
||||||
|
the old place, and `pty.Open` opened `/dev/pts/0` instead of the terminal it
|
||||||
|
had created. `secret rm` then wrote to that other terminal, and the test read
|
||||||
|
a terminal no program had open, which never ends. `go.mod` now requires the
|
||||||
|
commit on that library's main branch that passes a pointer instead; no release
|
||||||
|
has it yet. Both tests stop reading the terminal when their one-minute context
|
||||||
|
ends and fail saying so.
|
||||||
- 2026-10-06: The tests run quickly with the race detector on
|
- 2026-10-06: The tests run quickly with the race detector on
|
||||||
(https://git.eeqj.de/sneak/secret/issues/120). Most of their time went to
|
(https://git.eeqj.de/sneak/secret/issues/120). Most of their time went to
|
||||||
deriving keys from passphrases with scrypt, which is slow on purpose. The new
|
deriving keys from passphrases with scrypt, which is slow on purpose. The new
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ require (
|
|||||||
github.com/btcsuite/btcd/btcec/v2 v2.1.3
|
github.com/btcsuite/btcd/btcec/v2 v2.1.3
|
||||||
github.com/btcsuite/btcd/btcutil v1.1.6
|
github.com/btcsuite/btcd/btcutil v1.1.6
|
||||||
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
|
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
|
||||||
github.com/creack/pty v1.1.24
|
github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 // v1.1.24's Open can return another pty's terminal
|
||||||
github.com/dustin/go-humanize v1.0.1
|
github.com/dustin/go-humanize v1.0.1
|
||||||
github.com/fatih/color v1.18.0
|
github.com/fatih/color v1.18.0
|
||||||
github.com/keybase/go-keychain v0.0.0-20230307172405-3e4884637dd1
|
github.com/keybase/go-keychain v0.0.0-20230307172405-3e4884637dd1
|
||||||
|
|||||||
@@ -35,8 +35,8 @@ github.com/btcsuite/snappy-go v1.0.0/go.mod h1:8woku9dyThutzjeg+3xrA5iCpBRH8XEEg
|
|||||||
github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
|
github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
|
||||||
github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
|
github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
|
||||||
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
|
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
|
||||||
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
|
github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 h1:CY3gjC7naqYGLMiywvj3suPfa1i0p/QEr7o8ujxL/2M=
|
||||||
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
|
github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
|
||||||
github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||||
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||||
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
|
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
|
||||||
|
|||||||
@@ -2601,7 +2601,12 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
|
|||||||
// and stderr, not both: whether it asks must depend on stdin alone, where
|
// and stderr, not both: whether it asks must depend on stdin alone, where
|
||||||
// the answer is read from. pty.Open returns the two ends of a new terminal:
|
// the answer is read from. pty.Open returns the two ends of a new terminal:
|
||||||
// tty is the end a program uses as its terminal, and ptmx the end the test
|
// tty is the end a program uses as its terminal, and ptmx the end the test
|
||||||
// reads what the terminal shows from and types into.
|
// reads what the terminal shows from and types into. Reading ptmx stops at
|
||||||
|
// the context's deadline, when secret rm is killed too, so a terminal that
|
||||||
|
// stays open fails the test then instead of hanging it. The deadline works
|
||||||
|
// only while ptmx stays non-blocking, as pty.Open of the github.com/creack/pty
|
||||||
|
// commit in go.mod leaves it: calling ptmx.Fd() or going back to v1.1.24
|
||||||
|
// makes the read ignore the deadline, without any error.
|
||||||
|
|
||||||
// TestRemoveIgnoresTerminalOnStdout runs `echo y | secret rm x` at a
|
// TestRemoveIgnoresTerminalOnStdout runs `echo y | secret rm x` at a
|
||||||
// terminal. stdin is a pipe, so nobody can answer there, and the command
|
// terminal. stdin is a pipe, so nobody can answer there, and the command
|
||||||
@@ -2619,6 +2624,9 @@ func TestRemoveIgnoresTerminalOnStdout(t *testing.T) {
|
|||||||
|
|
||||||
defer func() { _ = ptmx.Close() }()
|
defer func() { _ = ptmx.Close() }()
|
||||||
|
|
||||||
|
deadline, _ := ctx.Deadline()
|
||||||
|
require.NoError(t, ptmx.SetReadDeadline(deadline))
|
||||||
|
|
||||||
cmd.Stdin = strings.NewReader("y\n")
|
cmd.Stdin = strings.NewReader("y\n")
|
||||||
cmd.Stdout = tty
|
cmd.Stdout = tty
|
||||||
cmd.Stderr = tty
|
cmd.Stderr = tty
|
||||||
@@ -2628,9 +2636,15 @@ func TestRemoveIgnoresTerminalOnStdout(t *testing.T) {
|
|||||||
_ = tty.Close()
|
_ = tty.Close()
|
||||||
|
|
||||||
// The read ends once secret rm has exited and so closed the terminal.
|
// The read ends once secret rm has exited and so closed the terminal.
|
||||||
shown, _ := io.ReadAll(ptmx)
|
shown, err := io.ReadAll(ptmx)
|
||||||
|
require.NotErrorIs(t, err, os.ErrDeadlineExceeded,
|
||||||
|
"the terminal was still open a minute after secret rm started: %s",
|
||||||
|
shown)
|
||||||
|
|
||||||
require.Error(t, cmd.Wait())
|
err = cmd.Wait()
|
||||||
|
|
||||||
|
require.NoError(t, ctx.Err(), "secret rm did not exit within a minute")
|
||||||
|
require.Error(t, err)
|
||||||
assert.Contains(t, string(shown), "pass --force")
|
assert.Contains(t, string(shown), "pass --force")
|
||||||
assert.DirExists(t, secretDir)
|
assert.DirExists(t, secretDir)
|
||||||
}
|
}
|
||||||
@@ -2650,6 +2664,9 @@ func TestRemoveAsksAtTerminalOnStdin(t *testing.T) {
|
|||||||
|
|
||||||
defer func() { _ = ptmx.Close() }()
|
defer func() { _ = ptmx.Close() }()
|
||||||
|
|
||||||
|
deadline, _ := ctx.Deadline()
|
||||||
|
require.NoError(t, ptmx.SetReadDeadline(deadline))
|
||||||
|
|
||||||
cmd.Stdin = tty
|
cmd.Stdin = tty
|
||||||
// Not a file, so exec.Cmd connects stdout through a pipe.
|
// Not a file, so exec.Cmd connects stdout through a pipe.
|
||||||
cmd.Stdout = io.Discard
|
cmd.Stdout = io.Discard
|
||||||
@@ -2667,7 +2684,8 @@ func TestRemoveAsksAtTerminalOnStdin(t *testing.T) {
|
|||||||
terminal := bufio.NewReader(ptmx)
|
terminal := bufio.NewReader(ptmx)
|
||||||
for !bytes.HasSuffix(shown, []byte("[y/N] ")) {
|
for !bytes.HasSuffix(shown, []byte("[y/N] ")) {
|
||||||
char, err = terminal.ReadByte()
|
char, err = terminal.ReadByte()
|
||||||
require.NoError(t, err, "secret rm ended without asking: %s", shown)
|
require.NoError(t, err, "secret rm did not ask on the terminal: %s",
|
||||||
|
shown)
|
||||||
|
|
||||||
shown = append(shown, char)
|
shown = append(shown, char)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user