fix(bridge): stop discarding the fields the page actually sends

packages/zig/test/capabilities_test.zigmodified+266-0

Changes to packages/zig/test/capabilities_test.zig

		⋯415 linesShow the 415 lines above this hunk@@ -416,3 +416,269 @@ test "the manifest renders and parses" {
416	416	try testing.expect(channels.count() > 0);
417	417	try testing.expect(channels.get("craft:fs:change") != null);
418	418	}
	419	+
	420	+test "every bridge type the page can send is routed by the dispatcher" {
	421	+ // The direction nothing checked. `test "the registry covers exactly the
	422	+ // namespaces the dispatcher routes"` pins the registry against the
	423	+ // dispatcher, and the channel test pins the events — but what the page can
	424	+ // *call* was never compared against what native will answer.
	425	+ //
	426	+ // A new `craft.foo.bar()` added to the facade with no arm in
	427	+ // `handleBridgeMessageJSON` posts a message that is parsed, matched
	428	+ // against every arm, and dropped. `_send` resolves the moment the message
	429	+ // leaves, so the caller gets a fulfilled promise and nothing happens —
	430	+ // silent, and exactly the shape of #27, #47 and #69.
	431	+ const chain = blk: {
	432	+ const start = std.mem.indexOf(u8, dispatcher_source, "pub fn handleBridgeMessageJSON") orelse
	433	+ return error.DispatcherNotFound;
	434	+ const end = std.mem.indexOfPos(u8, dispatcher_source, start, "\npub fn handleBridgeMessage(") orelse
	435	+ dispatcher_source.len;
	436	+ break :blk dispatcher_source[start..end];
	437	+ };
	438	+
	439	+ var checked: usize = 0;
	440	+ var unrouted: usize = 0;
	441	+
	442	+ // `_send('type', 'action'` / `_req(` / `_post(` — the three ways the
	443	+ // facade posts a message.
	444	+ for ([_][]const u8{ "_send('", "_req('", "_post('" }) |opener| {
	445	+ var search: usize = 0;
	446	+ while (std.mem.indexOfPos(u8, bridge_js, search, opener)) |hit| {
	447	+ search = hit + opener.len;
	448	+ const rest = bridge_js[search..];
	449	+ const close = std.mem.indexOfScalar(u8, rest, '\'') orelse continue;
	450	+ const msg_type = rest[0..close];
	451	+ if (msg_type.len == 0 or msg_type.len > 64) continue;
	452	+
	453	+ checked += 1;
	454	+
	455	+ var needle_buf: [128]u8 = undefined;
	456	+ const needle = try std.fmt.bufPrint(&needle_buf, "msg_type, \"{s}\"", .{msg_type});
	457	+ if (std.mem.indexOf(u8, chain, needle) == null) {
	458	+ unrouted += 1;
	459	+ std.debug.print(
	460	+ "craft-bridge.js posts type '{s}' but handleBridgeMessageJSON does not route it\n",
	461	+ .{msg_type},
	462	+ );
	463	+ }
	464	+ }
	465	+ }
	466	+
	467	+ // Floor: the facade posts hundreds of messages. If a refactor changes how
	468	+ // they are written this scan finds none and would otherwise pass having
	469	+ // compared nothing at all.
	470	+ try testing.expect(checked >= 200);
	471	+ try testing.expectEqual(@as(usize, 0), unrouted);
	472	+}
	473	+
	474	+/// Bridges whose payload field names are checked against what the page sends.
	475	+///
	476	+/// A ratchet like `max_undeclared`: this list may only grow. Adding a bridge
	477	+/// here is what "the fields on both sides agree" costs, and it is cheap —
	478	+/// every entry below was added after the audit that found nine bridges reading
	479	+/// keys the page has never sent.
	480	+const field_checked = [_]struct {
	481	+ /// The bridge type as the page names it in `_send`/`_req`.
	482	+ namespace: []const u8,
	483	+ source: []const u8,
	484	+}{
	485	+ .{ .namespace = "fs", .source = @embedFile("src/bridge_fs.zig") },
	486	+ .{ .namespace = "window", .source = @embedFile("src/bridge_window.zig") },
	487	+ .{ .namespace = "updater", .source = @embedFile("src/bridge_updater.zig") },
	488	+ .{ .namespace = "bluetooth", .source = @embedFile("src/bridge_bluetooth.zig") },
	489	+ .{ .namespace = "serial", .source = @embedFile("src/bridge_serial.zig") },
	490	+ .{ .namespace = "localServer", .source = @embedFile("src/bridge_local_server.zig") },
	491	+};
	492	+
	493	+/// The body of `fn <action>(...)`, from its signature to the next ` fn `.
	494	+///
	495	+/// Narrowing the search to the handler is what makes this catch a name used in
	496	+/// the *wrong* handler — `writeFile` reading a key only `readFile` sends. A
	497	+/// file-wide search cannot: the first version of this test passed against the
	498	+/// exact bug it was written for, because the name still appeared elsewhere in
	499	+/// the same file.
	500	+fn handlerBody(source: []const u8, action: []const u8) ?[]const u8 {
	501	+ var needle_buf: [96]u8 = undefined;
	502	+ const needle = std.fmt.bufPrint(&needle_buf, "\n fn {s}(", .{action}) catch return null;
	503	+ const start = std.mem.indexOf(u8, source, needle) orelse return null;
	504	+ const after = start + needle.len;
	505	+
	506	+ // Ends at the next declaration *or* the next doc comment, whichever comes
	507	+ // first. Stopping only at `fn ` swallows the following function's `///`
	508	+ // block — and these bridges document their payloads in it, so
	509	+ // `appendFile`'s `/// JSON: {"content": "data"}` leaked into `writeFile`'s
	510	+ // span and made the check pass against the bug it was written for.
	511	+ const next_fn = std.mem.indexOfPos(u8, source, after, "\n fn ") orelse source.len;
	512	+ const next_doc = std.mem.indexOfPos(u8, source, after, "\n ///") orelse source.len;
	513	+ return source[start..@min(next_fn, next_doc)];
	514	+}
	515	+
	516	+/// Whether a bridge's source mentions `name` as a payload field.
	517	+///
	518	+/// Two spellings, because bridges read payloads two ways. Some scan for a
	519	+/// literal needle — `json_utils.getString(data, "path")` — where the name
	520	+/// appears quoted. Others parse into a struct, where it is an unquoted field
	521	+/// declaration: `path: []const u8 = ""`. A check that knew only the first
	522	+/// reports every struct-based bridge as broken, which is exactly what the
	523	+/// first version of this test did.
	524	+fn bridgeKnowsField(source: []const u8, name: []const u8) bool {
	525	+ var quoted_buf: [96]u8 = undefined;
	526	+ const quoted = std.fmt.bufPrint(&quoted_buf, "\"{s}\"", .{name}) catch return true;
	527	+ if (std.mem.indexOf(u8, source, quoted) != null) return true;
	528	+
	529	+ // A struct field: the name at the start of an indented line, followed by a
	530	+ // colon. Anchored on the newline so `path:` inside `filePath:` or a
	531	+ // comment does not count.
	532	+ var field_buf: [96]u8 = undefined;
	533	+ const field = std.fmt.bufPrint(&field_buf, "\n {s}: ", .{name}) catch return true;
	534	+ if (std.mem.indexOf(u8, source, field) != null) return true;
	535	+
	536	+ // A struct field at any indentation, identified by its default: every
	537	+ // payload struct in these bridges declares one (`baud: u32 = 9600`).
	538	+ //
	539	+ // The default is what tells a field from a *parameter*. `fn writeFile(self:
	540	+ // *Self, data: []const u8)` contains `data: `, and accepting that made the
	541	+ // check pass against the exact bug it was written for — the handler was
	542	+ // reading the wrong key while its own parameter supplied the name.
	543	+ var nested_buf: [96]u8 = undefined;
	544	+ const nested = std.fmt.bufPrint(&nested_buf, "{s}: ", .{name}) catch return true;
	545	+ var search: usize = 0;
	546	+ while (std.mem.indexOfPos(u8, source, search, nested)) |hit| {
	547	+ search = hit + nested.len;
	548	+ if (hit != 0) {
	549	+ const before = source[hit - 1];
	550	+ if (before != ' ' and before != '\n' and before != '\t') continue;
	551	+ }
	552	+ const line_end = std.mem.indexOfScalarPos(u8, source, hit, '\n') orelse source.len;
	553	+ if (std.mem.indexOfScalarPos(u8, source[0..line_end], hit, '=') != null) return true;
	554	+ }
	555	+ return false;
	556	+}
	557	+
	558	+/// Fields a handler legitimately does not name.
	559	+///
	560	+/// Narrowing the search to the handler is what makes the check catch a name
	561	+/// read in the wrong place — but it also flags two honest patterns: a handler
	562	+/// that reads the payload without keying off the field name, and one that
	563	+/// delegates the parsing to a helper. Each exemption costs a line and a
	564	+/// reason, which is the point: the list is short and every entry is arguable,
	565	+/// rather than the check being weakened for everyone.
	566	+const field_exempt = [_]struct {
	567	+ namespace: []const u8,
	568	+ action: []const u8,
	569	+ field: []const u8,
	570	+ why: []const u8,
	571	+}{
	572	+ .{
	573	+ .namespace = "window",
	574	+ .action = "setAlwaysOnTop",
	575	+ .field = "value",
	576	+ .why = "scans the payload for the literal \"false\", so it reads the boolean without naming the key",
	577	+ },
	578	+ .{ .namespace = "window", .action = "setResizable", .field = "value", .why = "same key-agnostic boolean scan" },
	579	+ .{ .namespace = "window", .action = "setMovable", .field = "value", .why = "same key-agnostic boolean scan" },
	580	+ .{ .namespace = "window", .action = "setHasShadow", .field = "value", .why = "same key-agnostic boolean scan" },
	581	+ .{
	582	+ .namespace = "bluetooth",
	583	+ .action = "connectDevice",
	584	+ .field = "id",
	585	+ .why = "delegates to `deviceAddress`, which accepts both \"id\" and \"address\"",
	586	+ },
	587	+ .{ .namespace = "bluetooth", .action = "disconnectDevice", .field = "id", .why = "same helper" },
	588	+};
	589	+
	590	+fn isExempt(namespace: []const u8, action: []const u8, field: []const u8) bool {
	591	+ for (field_exempt) |e| {
	592	+ if (std.mem.eql(u8, e.namespace, namespace) and
	593	+ std.mem.eql(u8, e.action, action) and
	594	+ std.mem.eql(u8, e.field, field)) return true;
	595	+ }
	596	+ return false;
	597	+}
	598	+
	599	+test "every field the page sends is a name the handler actually looks for" {
	600	+ // The bug class this exists for, from issue #69 and the audit that
	601	+ // followed it: the page posts `{"data": "..."}`, the handler reads
	602	+ // `"content"`, and nothing objects. Every bridge parses with
	603	+ // `ignore_unknown_fields = true` or scans for a literal needle, so a name
	604	+ // that does not match is not an error — the value is simply gone and the
	605	+ // caller's promise resolves.
	606	+ //
	607	+ // It cost `craft.fs.writeFile` writing empty files, `setFullscreen(false)`
	608	+ // entering fullscreen, `setVibrancy` removing vibrancy, and Bluetooth
	609	+ // connects that could never name a device.
	610	+ //
	611	+ // The check is deliberately loose: it asks only whether the field name
	612	+ // appears *somewhere* in the handling bridge, quoted. That is enough to
	613	+ // catch a name the native side has never heard of, which is the whole
	614	+ // failure, without trying to model which handler owns which action.
	615	+ var checked: usize = 0;
	616	+ var missing: usize = 0;
	617	+
	618	+ for ([_][]const u8{ "_send('", "_req('" }) |opener| {
	619	+ var search: usize = 0;
	620	+ while (std.mem.indexOfPos(u8, bridge_js, search, opener)) |hit| {
	621	+ search = hit + opener.len;
	622	+ const rest = bridge_js[search..];
	623	+
	624	+ const type_end = std.mem.indexOfScalar(u8, rest, '\'') orelse continue;
	625	+ const msg_type = rest[0..type_end];
	626	+
	627	+ // Only the bridges that have opted in.
	628	+ var source: ?[]const u8 = null;
	629	+ for (field_checked) |entry| {
	630	+ if (std.mem.eql(u8, entry.namespace, msg_type)) source = entry.source;
	631	+ }
	632	+ if (source == null) continue;
	633	+
	634	+ // The action, so the search can be narrowed to its handler.
	635	+ const after_type = rest[type_end + 1 ..];
	636	+ const a_open = std.mem.indexOf(u8, after_type, "'") orelse continue;
	637	+ const a_rest = after_type[a_open + 1 ..];
	638	+ const a_end = std.mem.indexOfScalar(u8, a_rest, '\'') orelse continue;
	639	+ const action_name = a_rest[0..a_end];
	640	+
	641	+ // `fn <action>(` when the handler is named after the action, which
	642	+ // is the convention throughout. Falls back to the whole file when
	643	+ // it is not — looser, but never a false alarm.
	644	+ const scope = handlerBody(source.?, action_name) orelse source.?;
	645	+
	646	+ // The `_stringify({ ... })` literal for this call, if it has one.
	647	+ const call_end = std.mem.indexOfScalarPos(u8, bridge_js, search, '\n') orelse bridge_js.len;
	648	+ const call = bridge_js[search..call_end];
	649	+ const brace = std.mem.indexOf(u8, call, "_stringify({") orelse continue;
	650	+ const obj_start = brace + "_stringify({".len;
	651	+ const obj_end = std.mem.indexOfScalarPos(u8, call, obj_start, '}') orelse continue;
	652	+ const obj = call[obj_start..obj_end];
	653	+
	654	+ // Field names: an identifier followed by a colon.
	655	+ var i: usize = 0;
	656	+ while (i < obj.len) {
	657	+ while (i < obj.len and !std.ascii.isAlphabetic(obj[i])) i += 1;
	658	+ const name_start = i;
	659	+ while (i < obj.len and (std.ascii.isAlphanumeric(obj[i]) or obj[i] == '_')) i += 1;
	660	+ if (i >= obj.len or name_start == i) break;
	661	+ const name = obj[name_start..i];
	662	+ var j = i;
	663	+ while (j < obj.len and obj[j] == ' ') j += 1;
	664	+ if (j >= obj.len or obj[j] != ':') continue;
	665	+
	666	+ checked += 1;
	667	+ if (isExempt(msg_type, action_name, name)) continue;
	668	+ if (!bridgeKnowsField(scope, name)) {
	669	+ missing += 1;
	670	+ std.debug.print(
	671	+ "craft-bridge.js sends '{s}' to the {s} bridge, which never mentions that name —\n" ++
	672	+ " the value is parsed and discarded, and the caller's promise still resolves.\n",
	673	+ .{ name, msg_type },
	674	+ );
	675	+ }
	676	+ }
	677	+ }
	678	+ }
	679	+
	680	+ // Floor: these six bridges carry dozens of fields between them. A scan
	681	+ // that matched nothing would pass having compared nothing.
	682	+ try testing.expect(checked >= 30);
	683	+ try testing.expectEqual(@as(usize, 0), missing);
	684	+}