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..ddbd0cb 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,117 @@ 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: ?i64, +) !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_at = std.time.milliTimestamp(); + 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. 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| { + 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); + } + + // 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 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", 450); +} + test "alt screen apps: toggles are filtered and screens repaint from state" { const alloc = std.testing.allocator; var h = try Harness.init(alloc);