Skip to content

fix: stop detach key tails from leaking into the surrounding shell#36

Merged
kylecarbs merged 2 commits into
mainfrom
fix/attach-detach-input-leak
Jun 11, 2026
Merged

fix: stop detach key tails from leaking into the surrounding shell#36
kylecarbs merged 2 commits into
mainfrom
fix/attach-detach-input-leak

Conversation

@kylecarbs

Copy link
Copy Markdown
Member

Fixes the reported bug: detaching from boo attach with C-a C-d could kill the user's SSH session, and C-a d could leave a stray d typed at the prompt of the shell underneath. boo ui was unaffected.

Root cause

The detach round trip races the user's keyboard. The client exits as soon as the daemon acknowledges the detach, but the terminal keeps producing events for the key that triggered it:

  • auto-repeats once the keyboard's repeat delay elapses (~225-660ms depending on OS), for as long as the key is held;
  • kitty-protocol release reports, when the session app had mirrored event reporting onto the terminal;
  • impatient re-presses (the detach gives no feedback for a full SSH round trip);
  • anything already in flight across the SSH link.

The client's final TCSAFLUSH only discards bytes queued at that instant. Everything arriving a few milliseconds later is read by the login shell that regains the tty: a stray d at the prompt, or, for C-a C-d, a raw C-d that the shell reads as EOF, exiting it and tearing down the SSH session.

A second, daemon-side hole compounded the C-d case: input frames are parsed as a batch, and SSH commonly coalesces an auto-repeat right behind the command key (0x04 0x04 in one read). Bytes decoded after the detach dispatched in the same batch were still forwarded to the window, where the trailing C-d EOFed the session's program.

boo ui is immune because its prefix parsing is client-side and instant (no round trip, so the chord is released before repeats begin) and it never mirrors kitty key reporting onto the real terminal.

Reproduced with a PTY harness emulating a human typist (separate keystrokes, press/release/repeat timing, optional kitty CSI-u encoding, simulated SSH latency) under a real bash login shell: held C-a C-d reliably exited bash at 50ms latency, and held C-a d printed dddd at the restored prompt.

Fix

Client (src/client.zig): after writing the screen restore, keep reading and discarding tty input while still in raw mode (so nothing echoes), then hand the terminal back. Two timers bound the wait:

  • an initial guard covering the silent gap between the triggering press and the first auto-repeat, then short quiet extensions while events keep arriving, capped at 1.5s;
  • the guard is 300ms by default, 800ms when the tail is EOF-dangerous: a C-a C-d detach, or a session ending while attached (commonly a C-d typed at the session's own shell). Keyboard repeat delays reach ~660ms (Linux console default), and a leaked C-d is the SSH-session-killer, so that case gets the long guard; plain C-a d keeps the snappy guard so prompt handover stays quick and deliberate typing right after detach is not swallowed.

keys.Command.detach now carries the triggering byte, and the daemon reports C-d detaches as detached-eof. Old clients treat any non-stolen payload as a plain detach, so the protocol change is wire-compatible.

Daemon (src/daemon.zig): handleKeyCommand drops commands once the connection is no longer attached, so bytes coalesced behind the detach key in the same input batch can never reach the window.

Tradeoff, stated honestly

A tty hands us a byte stream, not key-up events, so "user still holds d with a slow repeat delay" and "user released and is about to type" are indistinguishable until the first repeat arrives. The 300ms guard for C-a d means a held d on a keyboard with a repeat delay above 300ms can still print a few ds; raising the guard would instead delay every detach and eat keystrokes typed deliberately right after. The catastrophic variant (C-d) always gets the long guard. If reports persist for the cosmetic case, a config knob (e.g. BOO_DETACH_DRAIN_MS) is the obvious follow-up.

Validation

  • zig build test-all passes (Debug), zig fmt --check clean.
  • Two new PTY integration tests run attach under a wrapping /bin/sh on the same tty (a login-shell stand-in) whose read exposes leaks: the command key is sent with a coalesced repeat, repeated until the restore sequence appears, plus one repeat after it (and, for the C-d case, a late repeat 600ms out modeling a slow keyboard). Both tests fail on main (the d variant corrupts the probe line; the C-d variant EOFs the shell and, before the daemon guard, killed the session's cat) and pass with the fix.
  • 15 parser unit tests updated for the detach payload byte.
  • The Python typist harness (legacy and kitty flags 1/3, 0-150ms simulated latency, tap/hold/release matrices) confirms: bash survives held C-a C-d everywhere, kitty release reports no longer spill 0;5:3u garbage, and single taps remain leak-free with no behavior change.

This PR was generated by Coder Agents on behalf of @kylecarbs.

Detaching with a key the user is still pressing leaked input into the
shell that regains the terminal. The client exited as soon as the
daemon acknowledged the detach, but the terminal keeps producing
events for that key: auto-repeats once the keyboard's repeat delay
elapses, kitty release reports, impatient re-presses, and anything in
flight across an SSH round trip. The final TCSAFLUSH only discards
bytes already queued, so the tail arrived after the client exited and
was read by the login shell instead: C-a d left a stray 'd' at the
prompt, and a held C-a C-d delivered a raw C-d that EOFed the login
shell and ended the user's SSH session. boo ui is immune because its
prefix parsing is local and instant, so the chord is released before
any of this begins.

Client: after writing the screen restore, keep reading and discarding
tty input while still in raw mode, then hand the terminal back. Two
timers bound the wait: an initial guard covering the press-to-repeat
gap, extended in short quiet steps while events keep arriving, with a
hard cap. Detaches triggered by C-d (and sessions that end while
attached, commonly a C-d typed at the session's shell) use a longer
guard, since keyboard repeat delays reach ~660ms and a leaked C-d is
the SSH-session-killer; plain C-a d keeps a snappier guard so prompt
handover stays quick and deliberate typing right after detach is not
swallowed. keys.Command.detach now carries the triggering byte and
the daemon reports such detaches as "detached-eof", which old clients
read as a plain detach.

Daemon: input frames are parsed as a batch, and a C-d auto-repeat
often arrives coalesced behind the command key (SSH batches
keystrokes), so bytes decoded after a detach in the same batch were
still forwarded to the window, where a trailing C-d EOFed the
program. handleKeyCommand now drops commands once the connection is
no longer attached.

Integration tests drive both chords from a wrapping shell on the
PTY, with coalesced repeats, repeats held until the restore appears,
and a late repeat modeling a slow keyboard; the shell's read exposes
any leak, and a leaked C-d ends it prematurely.
The late C-d repeat was timed from observing the restore sequence,
but the client's EOF guard starts at the detach acknowledgement;
macOS CI scheduling jitter consumed the 200ms slack and the repeat
landed after the guard expired. Real keyboards anchor the repeat
delay at the key press, so the test now does the same (450ms after
the press), keeping the repeat past the short guard while leaving
wide margin under the long one.
@kylecarbs kylecarbs merged commit 54d5f6a into main Jun 11, 2026
4 checks passed
kylecarbs added a commit that referenced this pull request Jun 11, 2026
Detach key tail leak fix (#36).
kylecarbs added a commit that referenced this pull request Jun 11, 2026
Detach key tail leak fix (#36).
@kylecarbs kylecarbs deleted the fix/attach-detach-input-leak branch June 11, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant