From cfc5a96501d72d0c43a73a9ed2f74c6381ba046c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 18 Aug 2026 12:07:36 -0700 Subject: [PATCH] terminal/kitty: validate graphics query image data Graphics query commands previously initialized their loading state but returned success before completing the image load. This allowed truncated, malformed, or otherwise invalid image data to return OK, giving capability probes a false positive. Complete and validate queried images through the normal load path, then discard the result without modifying image storage. Add coverage for invalid data and preserving an existing image with the queried ID. --- src/terminal/kitty/graphics_exec.zig | 83 +++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index 8a7cb645b..84ca7116a 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -138,13 +138,21 @@ fn query( .placement_id = t.placement_id, }; - // Attempt to load the image. If we cannot, then set an appropriate error. + // A query must attempt a complete load, then discard the result without + // changing image storage. + // https://sw.kovidgoyal.net/kitty/graphics-protocol/#querying-support-and-available-transmission-mediums const storage = &terminal.screens.active.kitty_images; var loading = LoadingImage.init(io, alloc, cmd, storage.image_limits) catch |err| { encodeError(&result, err); return result; }; - loading.deinit(alloc); + defer loading.deinit(alloc); + + var img = loading.complete(alloc) catch |err| { + encodeError(&result, err); + return result; + }; + img.deinit(alloc); return result; } @@ -481,6 +489,77 @@ fn encodeError(r: *Response, err: EncodeableError) void { } } +test "kittygfx query validates image data" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var terminal = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer terminal.deinit(alloc); + + var cmd: Command = .{ + .control = .{ .query = .{ + .format = .rgb, + .width = 1, + .height = 1, + .image_id = 31, + } }, + // A 1x1 RGB image requires three bytes. + .data = try alloc.dupe(u8, &.{ 0, 0 }), + }; + defer cmd.deinit(alloc); + + const resp = execute(io, alloc, &terminal, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(@as(u32, 31), resp.id); + try testing.expectEqualStrings("EINVAL: invalid data", resp.message); + try testing.expectEqual( + @as(usize, 0), + terminal.screens.active.kitty_images.images.count(), + ); +} + +test "kittygfx valid query does not replace or store image" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var terminal = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer terminal.deinit(alloc); + const storage = &terminal.screens.active.kitty_images; + + // Store a red pixel under the same ID used by the query. + { + const cmd = try command.Parser.parseString( + alloc, + "a=t,f=24,s=1,v=1,i=31;/wAA", + ); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &terminal, &cmd).?.ok()); + } + + // Successfully validate a black pixel without replacing the red one. + var cmd: Command = .{ + .control = .{ .query = .{ + .format = .rgb, + .width = 1, + .height = 1, + .image_id = 31, + } }, + .data = try alloc.dupe(u8, &.{ 0, 0, 0 }), + }; + defer cmd.deinit(alloc); + + const resp = execute(io, alloc, &terminal, &cmd).?; + try testing.expect(resp.ok()); + try testing.expectEqual(@as(usize, 1), storage.images.count()); + try testing.expectEqualSlices( + u8, + &.{ 255, 0, 0 }, + storage.imageById(31).?.data.bytes().?, + ); +} + test "kittygfx image id and number are mutually exclusive for every action" { const testing = std.testing; const alloc = testing.allocator;