Artifact b6ff30f41362b736858771aa7226fbf8aea8e0a1d28028c4ebdfeb8c6ab64cb5:
- File
src/release_assert_test.zig
— part of check-in
[da904fbe1c]
at
2026-08-17 22:37:39
on branch trunk
— build: ship ReleaseFast
zit ships ReleaseFast, so --release names it. This reverses the ReleaseSafe pin; pack runs ~25x faster and read verbs are indistinguishable between the modes (ls over tcl's 300MB store is 0.31s fast against 0.33s safe, leaves 0.01s in both).
The cost is real: over a thousand std.debug.assert calls compile to nothing, so release_assert_test.zig stops being belt-and-braces and becomes the thing keeping an assert from standing in for validation of untrusted input. Its rationale is rewritten around that.
Stating a preference makes it the ONLY reachable release mode, which the old comment got wrong: it documented --release=fast as an escape hatch from the ReleaseSafe pin, and that flag produced a ReleaseSafe binary anyway -- safety strings present to prove it. Anything benchmarked through it before now measured the checked build. The ci pin moves from 'keeps assertions alive' to 'the mode is stated, not left to whoever packages', which is the part that survives the reversal. (user: dmestas, size: 38808) [annotate] [blame] [check-ins using]
//! release_assert: a guard against `std.debug.assert` used as input validation. //! //! `std.debug.assert` is the right tool for an *internal invariant* — a fact the //! function itself established, held to as a crash-on-violation. It is the wrong //! tool for a rule about data the function did not produce, and it is wrong in //! two different ways depending on how the binary was built. Neither of them is //! "the input gets rejected", which is what the code reads as if it does. //! //! Under ReleaseFast — which build.zig makes the default for `--release`, and //! which is the mode zit SHIPS — the assert **compiles to nothing** and the //! malformed input flows straight through into the work the assert was meant to //! stop. There is no crash to notice, no diagnostic, and on the write path no //! barrier between a detected inconsistency and a durable bad frame in an //! append-only store. This is the case that makes this guard load-bearing rather //! than belt-and-braces: in the shipped binary an assert about untrusted input //! is not a weak check, it is no check. //! //! Under ReleaseSafe the assert survives and **panics** instead. On the write //! path that is a good trade: a crash beats a durable bad frame. On a //! *peer-reachable* path it is not a trade at all, it is a remote denial of //! service — anyone who can hand this process a malformed card can halt it. A //! parser reached from the sync wire must reject its input, not assert about it, //! however safe the build mode is. //! //! So the same finding names a missing check in the shipped build and a DoS in a //! safe one, and the same one-line fix — `return error.X` — closes both. The //! guard does not depend on which mode is default, and deliberately so: a guard //! that only holds while nobody passes a flag is not a guard. //! //! Three defects of exactly this shape were found and fixed recently: //! //! * `putShun` asserted a bound on caller-supplied bytes and, with the assert //! compiled out, wrote a durable frame that could never be opened again. //! * `ShunSet.contains` asserted a card's shape and crashed on a peer's //! malformed card instead of rejecting it. //! * `gatherCandidates` asserted a caller-supplied bound and, without it, left //! a scan unbounded. //! //! One of each: `ShunSet.contains` is the panic-on-a-peer's-input shape that //! ReleaseSafe does nothing to help, and the other two are the compiled-out //! shape. All three were `pub fn`s that already returned an error union — the //! correct fix in each case was `return error.X` rather than `assert`. This //! guard exists so the fourth one is caught by `zig build test` instead of by a //! user or a peer. //! //! What it flags — all of these must hold at once: //! //! 1. the enclosing function is `pub fn` (it is reachable from outside), //! 2. its return type is an error union (`!` or a named `…Error…` type), so //! rejecting the input costs nothing structurally, //! 3. the assert expression names one of that function's own parameters //! (`self`, `gpa`, `io`, `allocator`, `arena` are conventional plumbing and //! do not count), //! 4. that parameter is *untrusted*: a slice type, or a name from the //! `untrusted_names` list below, and //! 5. the assert is NOT already dominated by a `return` on that parameter. //! //! Rules 3 and 5 are what make this signal rather than noise, and `container.zig` //! is the worked example of each — it holds exactly two asserts and neither is a //! finding, for two different correct reasons: //! //! * `encode`'s assert names the parameter `bytes`, but the function opens with //! `if (bytes.len > …) return error.InputTooLarge`. The rejection already //! happened; the assert restates an established fact. Rule 5 clears it. //! * `decode`'s assert names only locals (`out`, `total`, `declared`). It is a //! postcondition about the function's own output, not a rule about input. //! Rule 3 clears it. //! //! `delta.applyCopy` is the same shape as the first (and is private besides, so //! rule 1 excludes it too). Because those zeros are load-bearing — a parser that //! silently stopped seeing asserts would also produce them — the test //! "container.zig's two asserts are attributed and correctly cleared" below pins //! the attribution directly rather than trusting the absence. //! //! The baseline is keyed on **(file, function name)** with a COUNT, deliberately //! not on line numbers: line numbers churn on every edit above them and would //! turn this into a maintenance tax. The count is what stops a function that //! already asserts badly — the likeliest place for the next bad assert — from //! absorbing a new one for free. A count that rises fails; a count that falls //! also fails, so the list can only shrink. //! //! The parse is driven by the Zig tokenizer, so an `assert` written inside a //! comment or a string literal is not read as a call, and a `{` inside one //! cannot skew a function's body extent. It is still a heuristic, not semantic //! analysis: it does not resolve aliases (`const assert = std.debug.assert`) or //! types. That is a knowing trade — the shape it does catch is the shape that //! produced all three defects. const std = @import("std"); /// The repository root, wired in build.zig's `build_unit_test_module`. A test /// binary's working directory is not the checkout, so the src tree is opened by /// this absolute path. Same handle src/tidy.zig and src/packaging_test.zig use. const repo_options = @import("repo_options"); /// Parameter names that are plumbing, not data: an assert on one of these is /// about the call's own wiring, not about untrusted content. const plumbing_names = [_][]const u8{ "self", "gpa", "io", "allocator", "arena" }; /// Substrings that mark a parameter as carrying data the function did not /// produce. A slice-typed parameter is untrusted regardless of its name; these /// catch the non-slice carriers (a length, an index, a scalar id read off the /// wire) whose names say where the value came from. const untrusted_names = [_][]const u8{ "hash", "name", "card", "payload", "bytes", "buf", "slice", "priority", "deltasrc", "token", "reason", "digest", "wire", "input", }; /// Files not scanned. Test and simulation sources assert about fixtures they /// themselves built — the parameter is trusted by construction — and the three /// harness roots (`e2e`, `smoke`, `dst`) are drivers, not library surface. const excluded_prefixes = [_][]const u8{"sim/"}; const excluded_files = [_][]const u8{ "e2e.zig", "smoke.zig", "dst.zig" }; /// Vacuity floor: the walk must find at least this many scannable `.zig` files /// or the tree was not actually scanned and every check below would pass by /// finding nothing. ~45 files survive the exclusions today; 30 is a floor with /// headroom, not a running count to keep in sync. const min_expected_files = 30; /// Read cap per source file. The largest src file is well under 1 MiB; 8 MiB is /// a tripwire against a runaway read, not an expected size. const max_source_bytes = 8 << 20; /// One baselined function and how many undominated asserts on untrusted input it /// held when the guard landed. const Baseline = struct { file: []const u8, function: []const u8, count: usize }; /// The pre-existing population, as measured by this guard's own parse. Each /// entry is a `pub fn` returning an error union that asserts on untrusted input /// without a dominating `return` — an input path that panics where it should /// reject (and, in a `--release=fast` build, does not check at all). /// /// These are recorded rather than fixed in one sweep because each fix is an API /// decision (which error, and what every caller does with it). The 29 entries in /// `sync.zig` are the ones to fix first: they are wire-card parsers, so under the /// ReleaseSafe default they are live remote-DoS surface rather than a latent /// fast-build hole. /// /// Counts may only fall. Raising one is not a fix — see issue #19. const baseline = [_]Baseline{ .{ .file = "annotate.zig", .function = "attribute", .count = 1 }, .{ .file = "artifact.zig", .function = "resolvePrefix", .count = 2 }, .{ .file = "checkout.zig", .function = "read", .count = 1 }, .{ .file = "cli.zig", .function = "composeAmendTags", .count = 1 }, .{ .file = "delta.zig", .function = "create", .count = 2 }, .{ .file = "logstore.zig", .function = "intern", .count = 1 }, .{ .file = "logstore.zig", .function = "pickBase", .count = 2 }, .{ .file = "logstore.zig", .function = "put", .count = 1 }, .{ .file = "logstore.zig", .function = "putArtifact", .count = 2 }, .{ .file = "logstore.zig", .function = "putArtifactPacked", .count = 2 }, .{ .file = "logstore.zig", .function = "putContainer", .count = 1 }, .{ .file = "logstore.zig", .function = "putDelta", .count = 1 }, .{ .file = "logstore.zig", .function = "putDeltaContainer", .count = 1 }, .{ .file = "logstore.zig", .function = "putZlib", .count = 1 }, .{ .file = "logstore.zig", .function = "resolveHexPrefix", .count = 1 }, .{ .file = "main.zig", .function = "mintControlArtifact", .count = 1 }, .{ .file = "repoimage.zig", .function = "serialize", .count = 1 }, .{ .file = "repostate.zig", .function = "bisectSpan", .count = 2 }, .{ .file = "repostate.zig", .function = "currentTrunk", .count = 1 }, .{ .file = "repostate.zig", .function = "ingestControlTags", .count = 1 }, .{ .file = "repostate.zig", .function = "openLeafOf", .count = 2 }, .{ .file = "repostate.zig", .function = "openLeavesOf", .count = 2 }, .{ .file = "repostate.zig", .function = "putWire", .count = 1 }, .{ .file = "repostate.zig", .function = "putWireDelta", .count = 2 }, .{ .file = "repostate.zig", .function = "resolve", .count = 1 }, .{ .file = "repostate.zig", .function = "resolveName", .count = 1 }, .{ .file = "repoview.zig", .function = "buildFromImage", .count = 1 }, .{ .file = "repoview.zig", .function = "buildFromRecordBody", .count = 1 }, .{ .file = "sync.zig", .function = "apply", .count = 2 }, .{ .file = "sync.zig", .function = "consumeConfigPayload", .count = 1 }, .{ .file = "sync.zig", .function = "consumeContentPayload", .count = 1 }, .{ .file = "sync.zig", .function = "consumeReply", .count = 1 }, .{ .file = "sync.zig", .function = "consumeUvfilePayload", .count = 1 }, .{ .file = "sync.zig", .function = "fossilUnquote", .count = 1 }, .{ .file = "sync.zig", .function = "parseCfileCard", .count = 3 }, .{ .file = "sync.zig", .function = "parseConfigCard", .count = 2 }, .{ .file = "sync.zig", .function = "parseFileCard", .count = 3 }, .{ .file = "sync.zig", .function = "parseGimmeCard", .count = 2 }, .{ .file = "sync.zig", .function = "parseIgotCard", .count = 2 }, .{ .file = "sync.zig", .function = "parseUvfileCard", .count = 2 }, .{ .file = "sync.zig", .function = "parseUvgimmeCard", .count = 2 }, .{ .file = "sync.zig", .function = "parseUvigotCard", .count = 2 }, .{ .file = "sync.zig", .function = "readLine", .count = 2 }, .{ .file = "sync.zig", .function = "recordPeerHas", .count = 1 }, .{ .file = "sync.zig", .function = "recordPeerLacks", .count = 1 }, .{ .file = "sync.zig", .function = "writeCiLock", .count = 3 }, .{ .file = "sync.zig", .function = "writeCiLockFail", .count = 2 }, .{ .file = "sync.zig", .function = "writeCiUnlock", .count = 2 }, .{ .file = "sync.zig", .function = "writeClientVersion", .count = 1 }, .{ .file = "sync.zig", .function = "writeCloneCard", .count = 1 }, .{ .file = "sync.zig", .function = "writeConfigCard", .count = 2 }, .{ .file = "sync.zig", .function = "writeFlagPragma", .count = 2 }, .{ .file = "sync.zig", .function = "writeLogin", .count = 3 }, .{ .file = "sync.zig", .function = "writePushPullCard", .count = 3 }, .{ .file = "sync.zig", .function = "writeReqconfig", .count = 3 }, .{ .file = "sync.zig", .function = "writeScalarConfig", .count = 1 }, .{ .file = "sync.zig", .function = "writeServerVersion", .count = 1 }, .{ .file = "undo.zig", .function = "record", .count = 1 }, }; test "no new std.debug.assert guards a release-build rule on untrusted input" { const io = std.testing.io; var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); const src_abs = try std.fmt.allocPrint(arena, "{s}/src", .{repo_options.root}); var src_dir = std.Io.Dir.cwd().openDir(io, src_abs, .{ .iterate = true }) catch |err| switch (err) { // The only legitimate skip: the wired root is not a directory here, so // the machine cannot answer the question. Anything else is a real fault. error.FileNotFound, error.NotDir => return error.SkipZigTest, else => { std.debug.print("\nrelease_assert: cannot open {s}: {s}\n", .{ src_abs, @errorName(err) }); return error.ReleaseAssertWalkFailed; }, }; defer src_dir.close(io); var files: std.ArrayList([]const u8) = .empty; { var walker = try src_dir.walk(arena); defer walker.deinit(); while (try walker.next(io)) |entry| { if (entry.kind != .file) continue; if (!std.mem.endsWith(u8, entry.path, ".zig")) continue; if (isExcluded(entry.path)) continue; try files.append(arena, try arena.dupe(u8, entry.path)); } } std.mem.sort([]const u8, files.items, {}, lessThanPath); if (files.items.len < min_expected_files) { std.debug.print("\nrelease_assert: walked {s} and found only {d} scannable .zig files; " ++ "expected at least {d}. The walk is not seeing the source tree — this guard " ++ "would pass vacuously.\n", .{ src_abs, files.items.len, min_expected_files }); return error.ReleaseAssertWalkFailed; } var scan: Scan = .empty; for (files.items) |file| { const source = try src_dir.readFileAllocOptions(io, file, arena, .limited(max_source_bytes), .of(u8), 0); try scanFile(arena, file, source, &scan); } var failed = false; // Forward direction: more asserts in a function than the baseline records. // A brand-new offender in a clean function and a second offender bolted onto // an already-listed one are the same failure and read the same way. for (scan.entries.values()) |entry| { const recorded = baselineCount(entry.file, entry.function); if (entry.sites.items.len <= recorded) continue; failed = true; std.debug.print( \\ \\release_assert: src/{s}: `{s}` has {d} assert(s) on untrusted input; the baseline records {d}. \\ , .{ entry.file, entry.function, entry.sites.items.len, recorded }); for (entry.sites.items) |site| { std.debug.print(" line {d} (`{s}`): {s}\n", .{ site.line, site.param, site.source_line }); } std.debug.print( \\ An assert is not a rejection. Under the default ReleaseSafe build these \\ panic, so if that data can reach `{s}` from a peer or a repository file, \\ that is a remote denial of service. Under an explicitly chosen \\ `--release=fast` build they compile to nothing and the data flows through \\ unvalidated instead. One fix closes both: `{s}` already returns an error \\ union — return an error. \\ if (!condition) return error.SomethingSpecific; \\ If the fact really is an invariant this function established, establish it \\ first (a preceding `return` on that parameter) and the assert then restates \\ it, which this guard accepts. \\ Every site in the function is listed above, not just the new one — the \\ baseline records a count, not positions. See issue #19. \\ , .{ entry.function, entry.function }); } // Reverse direction: fewer than recorded means someone fixed one. Say so as // progress, and require the number to come down so the list can only shrink. var key_buffer: [key_max]u8 = undefined; for (baseline) |entry| { const actual = if (scan.entries.get(entryKey(&key_buffer, entry.file, entry.function))) |found| found.sites.items.len else 0; if (actual >= entry.count) continue; failed = true; if (actual == 0) { std.debug.print( \\ \\release_assert: the baseline lists src/{s} `{s}` with {d} assert(s), but it now \\ has none. That is one fewer input path that panics instead of rejecting — \\ remove the entry from the baseline in src/release_assert_test.zig so the \\ list keeps shrinking. \\ (If instead the function was renamed or moved, update the entry to match.) \\ , .{ entry.file, entry.function, entry.count }); } else { std.debug.print( \\ \\release_assert: the baseline lists src/{s} `{s}` with {d} assert(s), but it now \\ has {d}. That is progress — lower the count in the baseline in \\ src/release_assert_test.zig so it keeps shrinking. \\ , .{ entry.file, entry.function, entry.count, actual }); } } if (failed) return error.ReleaseAssertOnUntrustedInput; } /// A source fixture with a known answer, so the parser's attribution is pinned by /// something that cannot drift underneath it. Every shape here previously had to /// be reasoned about by hand; now it is executable. const fixture = \\const std = @import("std"); \\ \\pub fn flagged(card: []const u8) !void { \\ std.debug.assert(card.len > 4); \\} \\ \\pub fn cleared(card: []const u8) !void { \\ if (card.len < 4) return error.Short; \\ std.debug.assert(card.len > 4); \\} \\ \\pub fn bracesInText(payload: []const u8) !void { \\ // an unbalanced brace in a comment: { \\ const s = "an unbalanced brace in a string: {"; \\ _ = s; \\ switch (payload.len) { \\ 0 => {}, \\ else => {}, \\ } \\ errdefer { \\ _ = payload; \\ } \\ std.debug.assert(payload.len > 1); \\} \\ \\pub fn outer(bytes: []const u8) !void { \\ const Inner = struct { \\ pub fn nested(digest: []const u8) !void { \\ std.debug.assert(digest.len == 32); \\ } \\ }; \\ try Inner.nested(bytes); \\ std.debug.assert(bytes.len != 0); \\} \\ \\pub fn notAnErrorUnion(buf: []const u8) void { \\ std.debug.assert(buf.len > 0); \\} \\ \\fn notPublic(buf: []const u8) !void { \\ std.debug.assert(buf.len > 0); \\} \\ \\pub fn onlyLocals(wire: []const u8) !void { \\ const n = wire.len; \\ const doubled = n * 2; \\ std.debug.assert(doubled >= 0); \\} \\ \\pub fn twoAsserts(hash: []const u8) !void { \\ std.debug.assert(hash.len == 40); \\ std.debug.assert(hash[0] != 0); \\} ; test "the scanner attributes asserts to the right function" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); var scan: Scan = .empty; try scanFile(arena, "fixture.zig", fixture, &scan); // Vacuity: every `std.debug.assert` in the fixture must have been seen and // attributed, or the filters below would be judging an empty set. try std.testing.expectEqual(@as(usize, 10), scan.asserts_seen); // Attribution, independent of whether the assert is a finding. `nested` is // the inner `pub fn` of a struct declared inside `outer`, so it exercises the // innermost-span rule; `notPublic` has no enclosing *candidate* and resolves // to "". const expected_owners = [_][]const u8{ "flagged", "cleared", "bracesInText", "nested", "outer", "", "", "onlyLocals", "twoAsserts", "twoAsserts", }; try std.testing.expectEqual(expected_owners.len, scan.attributions.items.len); for (expected_owners, scan.attributions.items) |want, got| { try std.testing.expectEqualStrings(want, got.function); // Self-check the line number rather than hand-counting it: the line the // scanner reported must itself hold the assert. try std.testing.expect(std.mem.indexOf(u8, nthLine(fixture, got.line), "std.debug.assert") != null); } // Findings, with counts. `cleared` is dominated (rule 5), `notAnErrorUnion` // and `notPublic` fail rules 2 and 1, and `onlyLocals` names no parameter // (rule 3) even though `wire` is in scope and untrusted. const expected = [_]struct { function: []const u8, count: usize }{ .{ .function = "flagged", .count = 1 }, .{ .function = "bracesInText", .count = 1 }, .{ .function = "nested", .count = 1 }, .{ .function = "outer", .count = 1 }, .{ .function = "twoAsserts", .count = 2 }, }; try std.testing.expectEqual(expected.len, scan.entries.count()); var key_buffer: [key_max]u8 = undefined; for (expected) |want| { const got = scan.entries.get(entryKey(&key_buffer, "fixture.zig", want.function)) orelse { std.debug.print("\nrelease_assert fixture: expected a finding in `{s}`, got none\n", .{want.function}); return error.FixtureMismatch; }; try std.testing.expectEqual(want.count, got.sites.items.len); } } test "container.zig's two asserts are attributed and correctly cleared" { const io = std.testing.io; var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); const path = try std.fmt.allocPrint(arena, "{s}/src/container.zig", .{repo_options.root}); const source = std.Io.Dir.cwd().readFileAllocOptions(io, path, arena, .limited(max_source_bytes), .of(u8), 0) catch |err| switch (err) { error.FileNotFound => return error.SkipZigTest, else => return err, }; var scan: Scan = .empty; try scanFile(arena, "container.zig", source, &scan); // The file holds exactly two asserts, one per public function. Pinning the // attribution is the point: the zero findings below are only meaningful if // the scanner actually resolved both, rather than losing them to a body-extent // bug and reporting nothing. try std.testing.expectEqual(@as(usize, 2), scan.asserts_seen); try std.testing.expectEqual(@as(usize, 2), scan.attributions.items.len); try std.testing.expectEqualStrings("encode", scan.attributions.items[0].function); try std.testing.expectEqualStrings("decode", scan.attributions.items[1].function); // Neither is a finding: `encode`'s is dominated by its opening // `return error.InputTooLarge` on `bytes` (rule 5), and `decode`'s names only // locals (rule 3). See the module doc. try std.testing.expectEqual(@as(usize, 0), scan.entries.count()); } /// The `line`-th line of `source`, 1-indexed. fn nthLine(source: []const u8, line: usize) []const u8 { var it = std.mem.splitScalar(u8, source, '\n'); var n: usize = 1; while (it.next()) |text| : (n += 1) { if (n == line) return text; } return ""; } /// One assert that is a finding. const Site = struct { line: usize, param: []const u8, source_line: []const u8, }; /// All findings for one (file, function) pair. const Entry = struct { file: []const u8, function: []const u8, sites: std.ArrayList(Site), }; /// Every assert the scanner resolved, finding or not. Kept so the fixture tests /// can pin attribution independently of the filters, and so a parser that stopped /// seeing asserts cannot masquerade as a clean tree. const Attribution = struct { line: usize, function: []const u8 }; const Scan = struct { entries: std.StringArrayHashMapUnmanaged(Entry), attributions: std.ArrayList(Attribution), asserts_seen: usize, const empty: Scan = .{ .entries = .empty, .attributions = .empty, .asserts_seen = 0 }; }; /// The map key for one (file, function) pair, written into a caller-owned /// buffer. The map compares keys by bytes, so a lookup key only has to outlive /// the call; `scanFile` dupes into the arena on insert. /// /// A path plus an identifier cannot approach 512 bytes in this tree, and the /// alternative to a fixed buffer — allocating a key for every lookup — buys /// nothing. Truncation would silently merge two functions' counts, so it fails /// loudly instead. const key_max = 512; fn entryKey(buffer: *[key_max]u8, file: []const u8, function: []const u8) []const u8 { return std.fmt.bufPrint(buffer, "{s}\x00{s}", .{ file, function }) catch @panic("release_assert: (file, function) key exceeds 512 bytes"); } fn lessThanPath(_: void, a: []const u8, b: []const u8) bool { return std.mem.order(u8, a, b) == .lt; } fn isExcluded(path: []const u8) bool { if (std.mem.endsWith(u8, path, "_test.zig")) return true; for (excluded_prefixes) |prefix| { if (std.mem.startsWith(u8, path, prefix)) return true; } for (excluded_files) |file| { if (std.mem.eql(u8, path, file)) return true; } return false; } fn baselineCount(file: []const u8, function: []const u8) usize { for (baseline) |entry| { if (std.mem.eql(u8, entry.file, file) and std.mem.eql(u8, entry.function, function)) return entry.count; } return 0; } const Param = struct { name: []const u8, untrusted: bool }; /// One `pub fn` with an error-union return type, and the byte span of its body. const Function = struct { name: []const u8, params: []const Param, body_start: usize, body_end: usize, }; /// Tokenize `source`, collect every candidate `pub fn`, then attribute each /// `std.debug.assert(...)` call to the innermost one containing it and apply the /// untrusted-parameter and dominance rules. fn scanFile( arena: std.mem.Allocator, file: []const u8, source: [:0]const u8, scan: *Scan, ) !void { var tokens: std.ArrayList(std.zig.Token) = .empty; { var tokenizer = std.zig.Tokenizer.init(source); while (true) { const token = tokenizer.next(); try tokens.append(arena, token); if (token.tag == .eof) break; } } const toks = tokens.items; var functions: std.ArrayList(Function) = .empty; for (toks, 0..) |token, i| { if (token.tag != .keyword_pub) continue; if (i + 1 >= toks.len or toks[i + 1].tag != .keyword_fn) continue; const function = try parseFunction(arena, source, toks, i + 1) orelse continue; try functions.append(arena, function); } var i: usize = 0; while (i + 5 < toks.len) : (i += 1) { if (!isDottedCall(source, toks, i, "std", "debug", "assert")) continue; const arg_end = matchingParen(toks, i + 5) orelse continue; const assert_start = toks[i].loc.start; scan.asserts_seen += 1; const enclosing = innermost(functions.items, assert_start); try scan.attributions.append(arena, .{ .line = lineNumber(source, assert_start), .function = if (enclosing) |function| function.name else "", }); const owner = enclosing orelse continue; for (owner.params) |param| { if (!param.untrusted) continue; if (!spanMentionsIdentifier(source, toks, i + 6, arg_end, param.name)) continue; if (dominated(source, owner.body_start, assert_start, param.name)) continue; const site: Site = .{ .line = lineNumber(source, assert_start), .param = param.name, .source_line = std.mem.trim(u8, lineAt(source, assert_start), " \t"), }; var key_buffer: [key_max]u8 = undefined; const gop = try scan.entries.getOrPut(arena, entryKey(&key_buffer, file, owner.name)); if (!gop.found_existing) { gop.key_ptr.* = try arena.dupe(u8, gop.key_ptr.*); gop.value_ptr.* = .{ .file = file, .function = owner.name, .sites = .empty }; } try gop.value_ptr.sites.append(arena, site); // One finding per assert, not per matching parameter: an assert that // names two untrusted parameters is still one thing to fix. break; } } } /// True when tokens `[i..i+5]` spell `a.b.c` followed by `(`. fn isDottedCall( source: []const u8, toks: []const std.zig.Token, i: usize, a: []const u8, b: []const u8, c: []const u8, ) bool { if (toks[i].tag != .identifier) return false; if (!std.mem.eql(u8, tokenText(source, toks[i]), a)) return false; if (toks[i + 1].tag != .period) return false; if (toks[i + 2].tag != .identifier or !std.mem.eql(u8, tokenText(source, toks[i + 2]), b)) return false; if (toks[i + 3].tag != .period) return false; if (toks[i + 4].tag != .identifier or !std.mem.eql(u8, tokenText(source, toks[i + 4]), c)) return false; return toks[i + 5].tag == .l_paren; } fn tokenText(source: []const u8, token: std.zig.Token) []const u8 { return source[token.loc.start..token.loc.end]; } /// Index of the `)` matching the `(` at `open`, or null if unbalanced. fn matchingParen(toks: []const std.zig.Token, open: usize) ?usize { var depth: usize = 0; var i = open; while (i < toks.len) : (i += 1) { switch (toks[i].tag) { .l_paren => depth += 1, .r_paren => { depth -= 1; if (depth == 0) return i; }, .eof => return null, else => {}, } } return null; } /// Index of the `}` matching the `{` at `open`, or null if unbalanced. fn matchingBrace(toks: []const std.zig.Token, open: usize) ?usize { var depth: usize = 0; var i = open; while (i < toks.len) : (i += 1) { switch (toks[i].tag) { .l_brace => depth += 1, .r_brace => { depth -= 1; if (depth == 0) return i; }, .eof => return null, else => {}, } } return null; } /// Parse the `pub fn` whose `fn` keyword is at `fn_index`. Returns null when it /// is not a candidate: no body (a prototype), or a return type that is not an /// error union — a function that cannot report a rejection is out of scope here. fn parseFunction( arena: std.mem.Allocator, source: []const u8, toks: []const std.zig.Token, fn_index: usize, ) !?Function { var i = fn_index + 1; if (i >= toks.len or toks[i].tag != .identifier) return null; const name = tokenText(source, toks[i]); i += 1; if (i >= toks.len or toks[i].tag != .l_paren) return null; const params_open = i; const params_close = matchingParen(toks, params_open) orelse return null; // The return type runs from just past `)` to the `{` that opens the body. // A `struct`/`union`/`enum`/`opaque`/`error` brace group inside the return // type is skipped so it is not mistaken for the body. var body_open: ?usize = null; var j = params_close + 1; while (j < toks.len) : (j += 1) { switch (toks[j].tag) { .semicolon, .eof => return null, // prototype, or a malformed parse .l_brace => { const introducer = if (j == 0) std.zig.Token.Tag.invalid else toks[j - 1].tag; const is_container = switch (introducer) { .keyword_struct, .keyword_union, .keyword_enum, .keyword_opaque, .keyword_error => true, else => false, }; if (is_container) { j = matchingBrace(toks, j) orelse return null; continue; } body_open = j; break; }, else => {}, } } const open = body_open orelse return null; const close = matchingBrace(toks, open) orelse return null; if (!returnsError(source, toks, params_close + 1, open)) return null; var params: std.ArrayList(Param) = .empty; try parseParams(arena, source, toks, params_open, params_close, ¶ms); if (params.items.len == 0) return null; return .{ .name = name, .params = params.items, .body_start = toks[open].loc.end, .body_end = toks[close].loc.start, }; } /// An error union return type: an explicit `!`, or a named type whose spelling /// carries `Error` (the `Foo.Error` / `SyncError` convention this repo uses). fn returnsError(source: []const u8, toks: []const std.zig.Token, from: usize, to: usize) bool { var i = from; while (i < to) : (i += 1) { if (toks[i].tag == .bang) return true; if (toks[i].tag == .identifier and std.mem.indexOf(u8, tokenText(source, toks[i]), "Error") != null) return true; } return false; } /// Split the parameter list at top-level commas and record each parameter's name /// and whether it is untrusted. Parameters without a `name: type` shape /// (`anytype` alone, `...`) contribute nothing and are skipped. fn parseParams( arena: std.mem.Allocator, source: []const u8, toks: []const std.zig.Token, open: usize, close: usize, out: *std.ArrayList(Param), ) !void { var start = open + 1; var i = start; var depth: usize = 0; while (i < close) : (i += 1) { switch (toks[i].tag) { .l_paren, .l_bracket, .l_brace => depth += 1, .r_paren, .r_bracket, .r_brace => depth -|= 1, .comma => if (depth == 0) { try pushParam(arena, source, toks, start, i, out); start = i + 1; }, else => {}, } } if (start < close) try pushParam(arena, source, toks, start, close, out); } fn pushParam( arena: std.mem.Allocator, source: []const u8, toks: []const std.zig.Token, from: usize, to: usize, out: *std.ArrayList(Param), ) !void { var i = from; while (i < to and (toks[i].tag == .keyword_comptime or toks[i].tag == .keyword_noalias)) i += 1; if (i >= to or toks[i].tag != .identifier) return; const name = tokenText(source, toks[i]); if (i + 1 >= to or toks[i + 1].tag != .colon) return; if (i + 2 >= to) return; // `name:` with no type — malformed, nothing to judge for (plumbing_names) |plumbing| { if (std.mem.eql(u8, name, plumbing)) return; } const type_text = std.mem.trim(u8, source[toks[i + 2].loc.start..toks[to - 1].loc.end], " \t\n"); var untrusted = std.mem.startsWith(u8, type_text, "[]"); if (!untrusted) { for (untrusted_names) |needle| { if (std.mem.indexOf(u8, name, needle) != null) { untrusted = true; break; } } } try out.append(arena, .{ .name = name, .untrusted = untrusted }); } /// The function with the smallest body span containing `offset`, so an assert in /// a nested `pub fn` is attributed to the nested one. fn innermost(functions: []const Function, offset: usize) ?Function { var best: ?Function = null; for (functions) |function| { if (offset < function.body_start or offset >= function.body_end) continue; const current = best orelse { best = function; continue; }; if (function.body_end - function.body_start < current.body_end - current.body_start) best = function; } return best; } /// True when some identifier token in `[from..to)` is exactly `name`. fn spanMentionsIdentifier( source: []const u8, toks: []const std.zig.Token, from: usize, to: usize, name: []const u8, ) bool { var i = from; while (i < to) : (i += 1) { if (toks[i].tag != .identifier) continue; if (std.mem.eql(u8, tokenText(source, toks[i]), name)) return true; } return false; } /// Rule 5. True when an earlier line of the same body both mentions `param` and /// contains `return` — the function has already had its chance to reject the /// input, so a following assert restates an established fact rather than /// standing in for validation. /// /// This is deliberately line-shaped rather than flow-sensitive: it is the /// pattern a reviewer reads off the page, and over-clearing here costs a missed /// finding while under-clearing would cost a false alarm on correct code. fn dominated(source: []const u8, body_start: usize, assert_start: usize, param: []const u8) bool { if (assert_start <= body_start) return false; const before = source[body_start..assert_start]; // Drop the partial final line: the assert's own line is not "earlier". const cut = std.mem.lastIndexOfScalar(u8, before, '\n') orelse return false; var it = std.mem.splitScalar(u8, before[0..cut], '\n'); while (it.next()) |line| { if (std.mem.indexOf(u8, line, "return") == null) continue; if (mentionsIdentifier(line, param)) return true; } return false; } /// Substring search for `name` bounded by non-identifier characters, so `hash` /// does not match inside `hash_index`. fn mentionsIdentifier(haystack: []const u8, name: []const u8) bool { var from: usize = 0; while (std.mem.indexOfPos(u8, haystack, from, name)) |at| { const before_ok = at == 0 or !isIdentChar(haystack[at - 1]); const after = at + name.len; const after_ok = after >= haystack.len or !isIdentChar(haystack[after]); if (before_ok and after_ok) return true; from = at + 1; } return false; } fn isIdentChar(c: u8) bool { return std.ascii.isAlphanumeric(c) or c == '_'; } fn lineNumber(source: []const u8, offset: usize) usize { return std.mem.count(u8, source[0..offset], "\n") + 1; } fn lineAt(source: []const u8, offset: usize) []const u8 { const start = if (std.mem.lastIndexOfScalar(u8, source[0..offset], '\n')) |at| at + 1 else 0; const rest = source[start..]; const end = std.mem.indexOfScalar(u8, rest, '\n') orelse rest.len; return rest[0..end]; }