From 3f237fe0ed8042455c4c314f63de3fec601fb5a8 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 11 Jun 2026 16:53:32 +0000 Subject: [PATCH 1/2] fix: stop detach key tails from leaking into the surrounding shell 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. --- src/client.zig | 76 ++++++++++++++++++++++++++-- src/daemon.zig | 13 ++++- src/keys.zig | 36 ++++++------- test/integration.zig | 118 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 220 insertions(+), 23 deletions(-) diff --git a/src/client.zig b/src/client.zig index dfe2b1d..f60e1af 100644 --- a/src/client.zig +++ b/src/client.zig @@ -75,7 +75,10 @@ pub fn attach(alloc: std.mem.Allocator, socket_path: []const u8) !Outcome { var raw = saved; rawMode(&raw); try posix.tcsetattr(tty, .FLUSH, raw); - defer restoreTty(tty, saved); + // Set by the outcome paths below when a held C-d may still be + // repeating; read by the deferred restore. + var drain_guard_ms: i64 = drain_guard_short_ms; + defer restoreTty(tty, saved, drain_guard_ms); try protocol.writeAll(1, enter_sequence); // Handshake with our current size. @@ -143,9 +146,19 @@ pub fn attach(alloc: std.mem.Allocator, socket_path: []const u8) !Outcome { switch (msg.type) { .output => try protocol.writeAll(1, msg.payload), .detached => { - return if (std.mem.eql(u8, msg.payload, "stolen")) .stolen else .detached; + if (std.mem.eql(u8, msg.payload, "stolen")) return .stolen; + if (std.mem.eql(u8, msg.payload, "detached-eof")) { + drain_guard_ms = drain_guard_eof_ms; + } + return .detached; + }, + .exit => { + // Sessions often end because the user typed + // C-d at the session's shell; treat the tail + // as EOF-dangerous. + drain_guard_ms = drain_guard_eof_ms; + return .ended; }, - .exit => return .ended, else => {}, } } @@ -176,8 +189,63 @@ pub fn rawMode(t: *posix.termios) void { t.cc[@intFromEnum(posix.V.TIME)] = 0; } -fn restoreTty(tty: posix.fd_t, saved: posix.termios) void { +/// Read and discard terminal input until it goes quiet. +/// +/// When a detach is triggered by a key the user is still holding, the +/// terminal keeps producing input after the daemon has already decided +/// to detach: auto-repeats of the command key, kitty release reports, +/// impatient re-presses, and (on a remote connection) anything in +/// flight during the round trip. The final TCSAFLUSH only discards +/// what has reached the tty queue at that instant, so without this +/// wait the tail is delivered to the shell that regains the terminal: +/// a stray `d` typed at the prompt, or worse, a leaked C-d that EOFs +/// the login shell and ends the SSH session. +/// +/// Runs while the terminal is still in raw mode, so the discarded +/// bytes are never echoed. Two timers bound the wait: `guard_ms` +/// covers the silence between the triggering press and the first +/// auto-repeat (keyboard repeat delays reach ~660ms on common +/// configurations, so EOF-dangerous detaches use the long guard), +/// then each absorbed chunk extends the wait by a short tail until +/// the input stays quiet, all capped at drain_cap_ms. +fn drainInput(tty: posix.fd_t, guard_ms: i64) void { + const start = std.time.milliTimestamp(); + const cap = start + drain_cap_ms; + var deadline = start + guard_ms; + var buf: [256]u8 = undefined; + while (true) { + const now = std.time.milliTimestamp(); + const until = @min(deadline, cap); + if (now >= until) return; + var fds = [_]posix.pollfd{ + .{ .fd = tty, .events = posix.POLL.IN, .revents = 0 }, + }; + const ready = posix.poll(&fds, @intCast(until - now)) catch return; + if (ready == 0) return; + const n = posix.read(tty, &buf) catch return; + if (n == 0) return; + deadline = @max(deadline, std.time.milliTimestamp() + drain_tail_ms); + } +} + +/// Guard for detaches with no reason to expect a held key. +const drain_guard_short_ms = 300; +/// Guard for flows where the user plausibly holds C-d, the byte that +/// EOFs a cooked-mode shell: a C-a C-d detach and a session that ends +/// while attached (often a C-d typed at the session's own shell). +const drain_guard_eof_ms = 800; +const drain_tail_ms = 100; +const drain_cap_ms = 1500; + +fn restoreTty(tty: posix.fd_t, saved: posix.termios, guard_ms: i64) void { + // Screen restore first: the user sees the detach immediately, and + // a kitty-mode terminal stops CSI-u key reporting as soon as the + // reset reaches it, so a still-held key repeats in legacy bytes + // that the drain below absorbs. Only then hand the tty back; the + // FLUSH discards anything that slips in between the last drained + // read and the mode switch. protocol.writeAll(1, restore_sequence) catch {}; + drainInput(tty, guard_ms); posix.tcsetattr(tty, .FLUSH, saved) catch {}; } diff --git a/src/daemon.zig b/src/daemon.zig index 540f32c..6693e61 100644 --- a/src/daemon.zig +++ b/src/daemon.zig @@ -285,11 +285,22 @@ pub const Daemon = struct { } fn handleKeyCommand(self: *Daemon, conn: *Conn, cmd: keys.Command) !void { + // A detach earlier in the same input batch ended the + // attachment. Bytes after the detach key (auto-repeats of a + // held C-d arriving coalesced, or keys typed during the + // detach round trip) belong to no window; forwarding them + // would EOF or garble the program the user just left. + if (!conn.attached) return; switch (cmd) { .forward => |bytes| if (self.liveWindow()) |w| { w.writeInput(bytes) catch {}; }, - .detach => self.detachConn(conn, "detached"), + .detach => |byte| self.detachConn( + conn, + // A C-d triggered detach warns the client that the + // user may be holding the byte that EOFs shells. + if (byte == 0x04) "detached-eof" else "detached", + ), .redraw => try self.repaintTo(conn), .unknown => |byte| if (std.ascii.isPrint(byte)) self.message(conn, "unknown key: ^A {c}", .{byte}) diff --git a/src/keys.zig b/src/keys.zig index 3733477..2455965 100644 --- a/src/keys.zig +++ b/src/keys.zig @@ -17,7 +17,9 @@ pub const escape_byte: u8 = 0x01; // C-a pub const Command = union(enum) { /// Bytes to forward to the window. forward: []const u8, - detach, + /// The command key that triggered the detach; C-d (0x04) marks + /// the detach as EOF-dangerous for the client's input drain. + detach: u8, redraw, unknown: u8, }; @@ -216,7 +218,7 @@ pub const Parser = struct { fn dispatch(byte: u8, handler: anytype) !void { switch (byte) { - 'd', 0x04 => try handler.command(.detach), + 'd', 0x04 => try handler.command(.{ .detach = byte }), 'l', 0x0c => try handler.command(.redraw), 'a' => try handler.command(.{ .forward = &.{escape_byte} }), else => try handler.command(.{ .unknown = byte }), @@ -299,7 +301,7 @@ test "prefix commands" { try std.testing.expectEqualStrings("abdef", h.forwarded.items); try std.testing.expectEqual(@as(usize, 2), h.cmds.items.len); try std.testing.expectEqual(Command.redraw, h.cmds.items[0]); - try std.testing.expectEqual(Command.detach, h.cmds.items[1]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[1]); } test "literal escape via C-a a" { @@ -320,7 +322,7 @@ test "prefix split across feeds" { try std.testing.expectEqual(@as(usize, 0), h.cmds.items.len); try p.feed("d", false, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); } test "control variants match screen defaults" { @@ -329,7 +331,7 @@ test "control variants match screen defaults" { var p: Parser = .{}; try p.feed("\x01\x04\x01\x0c", false, &h); try std.testing.expectEqual(@as(usize, 2), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(Command.redraw, h.cmds.items[1]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -342,7 +344,7 @@ test "holding the prefix key stays armed until a command key" { // the window (an unconsumed 0x04 would EOF a shell). try p.feed("\x01\x01\x01\x04", false, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -360,7 +362,7 @@ test "kitty: encoded Ctrl+A starts the prefix, plain d detaches" { var p: Parser = .{}; try p.feed("\x1b[97;5ud", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -370,7 +372,7 @@ test "kitty: encoded Ctrl+A then encoded Ctrl+D detaches" { var p: Parser = .{}; try p.feed("\x1b[97;5u\x1b[100;5u", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -380,7 +382,7 @@ test "kitty: report-all plain d arrives as CSI-u and detaches" { var p: Parser = .{}; try p.feed("\x1b[97;5u\x1b[100u", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); } test "kitty: sequence split across feeds" { @@ -391,7 +393,7 @@ test "kitty: sequence split across feeds" { try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); try p.feed("ud", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); } test "kitty: press and release events" { @@ -401,7 +403,7 @@ test "kitty: press and release events" { // Press (explicit event), release while pending, then the command. try p.feed("\x1b[97;5:1u\x1b[97;5:3ud", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); // A stray prefix release outside a pending sequence is swallowed. try p.feed("\x1b[97;5:3u", true, &h); @@ -416,7 +418,7 @@ test "kitty: prefix auto-repeat stays armed" { // With event types: press, repeat, then encoded Ctrl+D. try p.feed("\x1b[97;5u\x1b[97;5:2u\x1b[100;5u", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -427,7 +429,7 @@ test "kitty: prefix repeat without event types stays armed" { // Without the event-types flag a repeat looks like a second press. try p.feed("\x1b[97;5u\x1b[97;5u\x1b[100;5u", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -439,7 +441,7 @@ test "kitty: modifier key events while armed do not eat the command" { // (kitty report-all-keys flag) is not the command key. try p.feed("\x1b[97;5u\x1b[57442;5u\x1b[100;5u", true, &h); try std.testing.expectEqual(@as(usize, 1), h.cmds.items.len); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); try std.testing.expectEqual(@as(usize, 0), h.forwarded.items.len); } @@ -449,7 +451,7 @@ test "kitty: lock modifiers do not hide the prefix" { var p: Parser = .{}; // mods 69 = 1 + ctrl(4) + caps lock(64). try p.feed("\x1b[97;69ud", true, &h); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); } test "kitty: other CSI-u keys pass through verbatim" { @@ -487,7 +489,7 @@ test "kitty: raw 0x01 still works" { defer h.deinit(); var p: Parser = .{}; try p.feed("\x01d", true, &h); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 'd' }, h.cmds.items[0]); } test "kitty: CSI-u encodings pass through when kitty mode is off" { @@ -517,5 +519,5 @@ test "kitty: pending command key in CSI-u form split across feeds" { try p.feed("\x1b[97;5u\x1b[100;", true, &h); try std.testing.expectEqual(@as(usize, 0), h.cmds.items.len); try p.feed("5u", true, &h); - try std.testing.expectEqual(Command.detach, h.cmds.items[0]); + try std.testing.expectEqual(Command{ .detach = 0x04 }, h.cmds.items[0]); } diff --git a/test/integration.zig b/test/integration.zig index d385272..c454f23 100644 --- a/test/integration.zig +++ b/test/integration.zig @@ -205,6 +205,19 @@ const PtyClient = struct { argv: []const []const u8, rows: u16, cols: u16, + ) !PtyClient { + return spawnProgram(harness, exe_path, argv, rows, cols); + } + + /// Like spawn, but runs an arbitrary program instead of the boo + /// binary, e.g. a shell wrapping an attach the way a login shell + /// does for a user. + fn spawnProgram( + harness: *Harness, + program: []const u8, + argv: []const []const u8, + rows: u16, + cols: u16, ) !PtyClient { const alloc = harness.alloc; @@ -240,7 +253,7 @@ const PtyClient = struct { defer arena_state.deinit(); const arena = arena_state.allocator(); - const argv0 = try arena.dupeZ(u8, exe_path); + const argv0 = try arena.dupeZ(u8, program); const argv_z = try arena.allocSentinel(?[*:0]const u8, argv.len + 1, null); argv_z[0] = argv0; for (argv, 1..) |arg, i| argv_z[i] = try arena.dupeZ(u8, arg); @@ -656,6 +669,109 @@ test "auto-repeated C-a stays armed until the command key" { try std.testing.expect(std.mem.indexOf(u8, result.stdout, "detached") != null); } +/// Drive a detach with a command key that is still held while the +/// detach completes, the way a human does it over a remote link: the +/// key keeps auto-repeating until the restored screen is visible, and +/// one final repeat lands after the client has already restored. The +/// attach runs under a wrapping shell on the same tty (a login shell +/// stand-in) whose `read` exposes any input that boo leaks: leaked +/// repeats corrupt the typed probe line, and a leaked C-d (EOF) ends +/// the read prematurely, which is the SSH-session-killer variant. +fn expectNoDetachKeyLeak( + h: *Harness, + session: []const u8, + key: []const u8, + late_repeat_ms: ?u64, +) !void { + const alloc = h.alloc; + + var exe_buf: [std.fs.max_path_bytes]u8 = undefined; + const exe_abs = try std.fs.cwd().realpath(exe_path, &exe_buf); + const script = try std.fmt.allocPrint( + alloc, + "{s} attach {s}; IFS= read -r line; printf 'GOT[%s]\\n' \"$line\"", + .{ exe_abs, session }, + ); + defer alloc.free(script); + + var client = try PtyClient.spawnProgram(h, "/bin/sh", &.{ "-c", script }, 24, 80); + defer client.deinit(); + try client.waitFor("\x1b[?1049h"); + + // Arm the prefix, then press the command key. The key arrives + // with its first auto-repeat coalesced into one read, the way a + // remote link batches keystrokes; the repeat must not be + // forwarded into the window after the detach dispatches. + try client.send("\x01"); + std.Thread.sleep(50 * std.time.ns_per_ms); + const pressed = try std.mem.concat(alloc, u8, &.{ key, key }); + defer alloc.free(pressed); + try client.send(pressed); + + // Hold the key: auto-repeats keep arriving until the user sees + // the restored screen. + var deadline = Deadline.init(default_timeout_ms); + while (std.mem.indexOf(u8, client.output.items, "\x1b[?1049l") == null) { + try client.send(key); + _ = try client.pump(30); + try deadline.tick("detach restore never arrived"); + } + // The release lags the restored screen by one repeat interval, so + // this repeat arrives after the client's own restore write. It + // must be absorbed before the shell regains the terminal. + try client.send(key); + + // A keyboard with a long repeat delay sends its first repeat well + // after the detach; the EOF guard has to outlast it. + if (late_repeat_ms) |ms| { + std.Thread.sleep(ms * std.time.ns_per_ms); + try client.send(key); + } + + // Type at the shell prompt once the key is long released. The + // pause must outlast the client's post-detach drain window so the + // probe reaches the shell, not the drain. + std.Thread.sleep(1500 * std.time.ns_per_ms); + try client.send("ok\n"); + try client.waitFor("GOT[ok]"); + try std.testing.expectEqual(@as(u32, 0), try client.waitExit()); + + // The detach happened and the session survived. + const result = try h.run(&.{"ls"}); + defer alloc.free(result.stdout); + defer alloc.free(result.stderr); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, session) != null); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "detached") != null); + + // Nothing leaked into the window either: the idle cat session's + // screen stays empty. + const peek = try h.run(&.{ "peek", session }); + defer alloc.free(peek.stdout); + defer alloc.free(peek.stderr); + try std.testing.expect(peek.term.Exited == 0); + try std.testing.expect(std.mem.indexOf(u8, peek.stdout, "d") == null); +} + +test "held C-a d: repeats do not leak into the shell after detach" { + const alloc = std.testing.allocator; + var h = try Harness.init(alloc); + defer h.deinit(); + + try h.startDetached("dleak", &.{"cat"}); + try expectNoDetachKeyLeak(&h, "dleak", "d", null); +} + +test "held C-a C-d: repeats do not EOF the shell after detach" { + const alloc = std.testing.allocator; + var h = try Harness.init(alloc); + defer h.deinit(); + + // The late repeat models a default Linux keyboard (repeat delay + // ~660ms): only the long EOF guard absorbs it. + try h.startDetached("eofleak", &.{"cat"}); + try expectNoDetachKeyLeak(&h, "eofleak", "\x04", 600); +} + test "alt screen apps: toggles are filtered and screens repaint from state" { const alloc = std.testing.allocator; var h = try Harness.init(alloc); From 995f3c342b8d41d565dd134cc8f20b66a6d3489a Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 11 Jun 2026 17:02:34 +0000 Subject: [PATCH 2/2] fix(test): anchor the late detach repeat at the press for slow runners 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. --- test/integration.zig | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/integration.zig b/test/integration.zig index c454f23..ddbd0cb 100644 --- a/test/integration.zig +++ b/test/integration.zig @@ -681,7 +681,7 @@ fn expectNoDetachKeyLeak( h: *Harness, session: []const u8, key: []const u8, - late_repeat_ms: ?u64, + late_repeat_ms: ?i64, ) !void { const alloc = h.alloc; @@ -704,6 +704,7 @@ fn expectNoDetachKeyLeak( // forwarded into the window after the detach dispatches. try client.send("\x01"); std.Thread.sleep(50 * std.time.ns_per_ms); + const pressed_at = std.time.milliTimestamp(); const pressed = try std.mem.concat(alloc, u8, &.{ key, key }); defer alloc.free(pressed); try client.send(pressed); @@ -722,9 +723,15 @@ fn expectNoDetachKeyLeak( try client.send(key); // A keyboard with a long repeat delay sends its first repeat well - // after the detach; the EOF guard has to outlast it. + // after the detach; the EOF guard has to outlast it. Real repeat + // delays anchor at the press, so anchor there too: the margin to + // the 800ms guard stays wide even on slow CI runners, while still + // proving the 300ms short guard alone would have missed it. if (late_repeat_ms) |ms| { - std.Thread.sleep(ms * std.time.ns_per_ms); + const elapsed = std.time.milliTimestamp() - pressed_at; + if (elapsed < ms) { + std.Thread.sleep(@intCast((ms - elapsed) * std.time.ns_per_ms)); + } try client.send(key); } @@ -766,10 +773,11 @@ test "held C-a C-d: repeats do not EOF the shell after detach" { var h = try Harness.init(alloc); defer h.deinit(); - // The late repeat models a default Linux keyboard (repeat delay - // ~660ms): only the long EOF guard absorbs it. + // The late repeat models a keyboard with a slow repeat delay + // (450ms after the press): past the short guard, only the long + // EOF guard absorbs it. try h.startDetached("eofleak", &.{"cat"}); - try expectNoDetachKeyLeak(&h, "eofleak", "\x04", 600); + try expectNoDetachKeyLeak(&h, "eofleak", "\x04", 450); } test "alt screen apps: toggles are filtered and screens repaint from state" {