a73x

f429a992

fix: cell text carrying a control byte is refused at the decoder

a73x   2026-09-04 18:04

Commit message
fix: cell text carrying a control byte is refused at the decoder

The client has no VT parser any more: paint.rowToVtFrom writes a decoded
cell's text to the user's real terminal verbatim. `CellRowReader.next`
accepted any byte, so a daemon could spell an escape sequence — an OSC 52
clipboard write, an alt-screen flip — across the cells of a row and have the
user's emulator honour it. On main the client's own ghostty absorbed such
bytes; nothing replaced it when the cells wire removed that engine.

Refused in the READER, so every consumer of the wire is covered by
construction. An ascii-run cell must satisfy `asciiByte`, the writer's own
predicate, now one shared function; a head-form cluster may carry no byte
below 0x20 and no 0x7F. Rationale in docs/decisions.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CsWfuJFQbTfGtKZLS5qw4q
docs/decisions.md
Old New
@@ -9172,3 +9172,35 @@ and a client must not depend on ghostty's page layout to paint a row. So mux
9172 keeps `dumpState` for the exec and cells for the wire, and the cell format is 9172 keeps `dumpState` for the exec and cells for the wire, and the cell format is
9173 mux's own: a client can be written against it without ghostty in the picture, 9173 mux's own: a client can be written against it without ghostty in the picture,
9174 which is the point of the change. 9174 which is the point of the change.
9175
9176 ## 2026-09-04 — cell text is filtered at the decoder, not at the painter
9177
9178 On main the client fed the daemon's bytes into its own ghostty engine and
9179 painted from that engine's cell dump. That engine was a sanitiser as a side
9180 effect: an escape sequence in the stream became at most a cell ghostty chose
9181 to keep, and nothing the daemon sent could reach the host terminal AS an
9182 escape.
9183
9184 The cells wire removed that engine. `paint.rowToVtFrom` writes a decoded
9185 cell's text between its own SGRs and `renderClipped` writes the result to the
9186 client's real terminal fd, so a cell's bytes now reach the user's emulator
9187 verbatim. A daemon could spell `\x1b]52;c;<base64>\x07` across the cells of a
9188 row — one byte per cell in an ascii run, or up to 63 in a head-form cluster —
9189 and the emulator would honour it: a clipboard write, an alt-screen flip, a
9190 title change. `CellRowWriter.cell` asserts only that a cluster fits in 63
9191 bytes, and `asciiCell` gated the WRITER alone; the reader returned whatever
9192 byte was there.
9193
9194 The check lives in `CellRowReader.next`, so every consumer of the wire — the
9195 tui painter, the wasm core, muxa, any future one — is covered by construction
9196 rather than by each painter remembering. An ascii-run cell must satisfy
9197 `asciiByte` (0x20..0x7E), which is now one function the writer's run decision
9198 and the reader's refusal share; a head-form cluster must carry no byte below
9199 0x20 and no 0x7F. Either is `error.BadPayload`, which becomes `.resync` on a
9200 delta and `error.SnapshotAborted` on a snapshot.
9201
9202 An honest ghostty page cell never holds a C0 or C1 byte, so this can only fire
9203 on a malformed or hostile daemon — which is exactly the case validating a
9204 payload is for. It is refused in the reader rather than stripped in the
9205 painter because a stripped row is a row the client and the daemon disagree
9206 about silently, and the dump-parity rule would go with it.
src/engine/protocol.zig
Old New
@@ -1360,8 +1360,27 @@ pub const cell_text_max = 63;
1360 1360
1361 pub const DecodedCell = struct { style: CellStyle, wide: Wide, text: []const u8 }; 1361 pub const DecodedCell = struct { style: CellStyle, wide: Wide, text: []const u8 };
1362 1362
1363 /// The one printable-ascii predicate, shared by the writer's run decision and
1364 /// the reader's refusal so the two cannot drift apart.
1365 pub fn asciiByte(b: u8) bool {
1366 return b >= 0x20 and b <= 0x7E;
1367 }
1368
1363 fn asciiCell(wide: Wide, text: []const u8) bool { 1369 fn asciiCell(wide: Wide, text: []const u8) bool {
1364 return wide == .narrow and text.len == 1 and text[0] >= 0x20 and text[0] <= 0x7E; 1370 return wide == .narrow and text.len == 1 and asciiByte(text[0]);
1371 }
1372
1373 /// Whether a head-form cell's text is a cluster no terminal will ACT on.
1374 ///
1375 /// The client writes a cell's text straight to the user's real terminal
1376 /// (`paint.rowToVtFrom`), and there is no VT parser on the client side any
1377 /// more to absorb what the daemon sent. So a control byte in a cell is
1378 /// refused HERE, in the reader, where every consumer of the wire is covered
1379 /// at once: an honest ghostty grid never holds one, and a daemon that sends
1380 /// one is spelling an escape sequence across cells onto somebody's screen.
1381 fn plainText(text: []const u8) bool {
1382 for (text) |b| if (b < 0x20 or b == 0x7F) return false;
1383 return true;
1365 } 1384 }
1366 1385
1367 pub const CellRowWriter = struct { 1386 pub const CellRowWriter = struct {
@@ -1560,6 +1579,10 @@ pub const CellRowReader = struct {
1560 self.read += 1; 1579 self.read += 1;
1561 if (self.run_ascii) { 1580 if (self.run_ascii) {
1562 if (self.rest.len < 1) return error.BadPayload; 1581 if (self.rest.len < 1) return error.BadPayload;
1582 // The same predicate the writer applies before it opens an ascii
1583 // run: a byte outside it in an ascii run is not a cell any encoder
1584 // of ours produced.
1585 if (!asciiByte(self.rest[0])) return error.BadPayload;
1563 const text = self.rest[0..1]; 1586 const text = self.rest[0..1];
1564 self.rest = self.rest[1..]; 1587 self.rest = self.rest[1..];
1565 return .{ .style = self.cur, .wide = .narrow, .text = text }; 1588 return .{ .style = self.cur, .wide = .narrow, .text = text };
@@ -1569,6 +1592,7 @@ pub const CellRowReader = struct {
1569 const len: usize = head & 0x3f; 1592 const len: usize = head & 0x3f;
1570 if (self.rest.len < 1 + len) return error.BadPayload; 1593 if (self.rest.len < 1 + len) return error.BadPayload;
1571 const text = self.rest[1 .. 1 + len]; 1594 const text = self.rest[1 .. 1 + len];
1595 if (!plainText(text)) return error.BadPayload;
1572 self.rest = self.rest[1 + len ..]; 1596 self.rest = self.rest[1 + len ..];
1573 return .{ .style = self.cur, .wide = @enumFromInt(head >> 6), .text = text }; 1597 return .{ .style = self.cur, .wide = @enumFromInt(head >> 6), .text = text };
1574 } 1598 }
@@ -2895,6 +2919,60 @@ test "cellrow: two default ascii cells are one ascii run" {
2895 }, list.items); 2919 }, list.items);
2896 } 2920 }
2897 2921
2922 test "cellrow: an ascii run carrying a control byte is refused" {
2923 // The wire is hand-buildable by whatever is on the far side, so the run
2924 // header can claim ascii over a byte the writer would never have put
2925 // there. The reader applies the writer's own predicate rather than
2926 // trusting the flag.
2927 const alloc = std.testing.allocator;
2928 var list: std.ArrayList(u8) = .empty;
2929 defer list.deinit(alloc);
2930 try list.appendSlice(alloc, &[_]u8{ 1, 0, 1, 0, 0x80, 0x1b });
2931 var r = try CellRowReader.init(list.items);
2932 try std.testing.expectError(error.BadPayload, r.next());
2933 }
2934
2935 test "cellrow: a head-form cell whose cluster holds a control byte is refused" {
2936 const alloc = std.testing.allocator;
2937 var list: std.ArrayList(u8) = .empty;
2938 defer list.deinit(alloc);
2939 // ncells=1; one non-ascii run; head byte says narrow, 2 bytes; "\x1b[".
2940 try list.appendSlice(alloc, &[_]u8{ 1, 0, 1, 0, 0x00, 0x02, 0x1b, '[' });
2941 var r = try CellRowReader.init(list.items);
2942 try std.testing.expectError(error.BadPayload, r.next());
2943 }
2944
2945 test "cellrow: an OSC 52 spelled across cells never decodes into a row" {
2946 // The trust boundary this check exists for. The client has no VT parser:
2947 // paint.rowToVtFrom writes a cell's text to the user's real terminal
2948 // verbatim, so a daemon that spread an escape sequence one byte per cell
2949 // would be typing on that user's screen — here, a clipboard write. The
2950 // row must be refused, not decoded.
2951 const alloc = std.testing.allocator;
2952 var list: std.ArrayList(u8) = .empty;
2953 defer list.deinit(alloc);
2954 const osc = "\x1b]52;c;bXV4\x07";
2955 try list.appendSlice(alloc, &[_]u8{ @intCast(osc.len), 0 });
2956 try list.appendSlice(alloc, &[_]u8{ @intCast(osc.len), 0, 0x80 });
2957 try list.appendSlice(alloc, osc);
2958
2959 var r = try CellRowReader.init(list.items);
2960 var decoded: usize = 0;
2961 const refused = while (decoded <= osc.len) {
2962 const cell_or_end = r.next() catch break true;
2963 if (cell_or_end == null) break false;
2964 decoded += 1;
2965 } else false;
2966 if (!refused) {
2967 std.debug.print(
2968 "an OSC 52 clipboard write spelled across {d} cells decoded as a row; " ++
2969 "the client would write those bytes to the user's terminal\n",
2970 .{decoded},
2971 );
2972 return error.EscapeSequenceDecoded;
2973 }
2974 }
2975
2898 test "cellrow: the writer owns each cell's text, so a reused caller buffer is safe" { 2976 test "cellrow: the writer owns each cell's text, so a reused caller buffer is safe" {
2899 const alloc = std.testing.allocator; 2977 const alloc = std.testing.allocator;
2900 var list: std.ArrayList(u8) = .empty; 2978 var list: std.ArrayList(u8) = .empty;