diff --git a/auditor/checks/static.ts b/auditor/checks/static.ts index 5c8a329..d05591b 100644 --- a/auditor/checks/static.ts +++ b/auditor/checks/static.ts @@ -105,10 +105,20 @@ export function runStaticCheck(diff: string): Finding[] { // elsewhere might exist). The point is: if NEITHER this diff nor // any other line in the diff reads the field, the PR is shipping // state without a consumer. + // + // Serde exemption: if the field's parent struct derives Serialize + // or Deserialize, the read-site is the macro itself — JSON + // round-trips consume every public field. Without this exemption + // the check produces false positives on every response/request + // struct shipped through `/v1/*`. const addedLines = lines.filter(l => l.startsWith("+") && !l.startsWith("+++")) .map(l => l.slice(1)); - const newFields = extractNewFields(addedLines); - for (const field of newFields) { + const newFields = extractNewFieldsWithLine(lines); + const seenNames = new Set(); + for (const { name: field, lineIdx } of newFields) { + if (seenNames.has(field)) continue; + seenNames.add(field); + if (parentStructHasSerdeDerive(lines, lineIdx)) continue; const readPattern = new RegExp(`[\\.:]\\s*${escape(field)}\\b|\\b${escape(field)}\\s*:`); // The definition line itself matches readPattern — filter it out // by requiring at least TWO lines in the diff mention the field @@ -146,18 +156,68 @@ function splitDiffByFile(diff: string): Map { return out; } -// Extract new `pub name: Type,` fields from added lines. Rust syntax. -// Narrowly-scoped: only matches at the start of a trimmed line, -// requires `pub ` prefix, ignores `pub fn` / `pub struct` / etc. -function extractNewFields(addedLines: string[]): string[] { - const fields = new Set(); - for (const line of addedLines) { - const t = line.trim(); - // pub NAME: Type, +// Extract new `pub name: Type,` fields from the per-file diff lines, +// keeping each occurrence's line index so the caller can resolve the +// parent struct. Same narrow rules as before: starts with `pub `, +// excludes `pub fn` / `pub struct` / etc. +function extractNewFieldsWithLine(lines: string[]): Array<{ name: string; lineIdx: number }> { + const out: Array<{ name: string; lineIdx: number }> = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (!line.startsWith("+") || line.startsWith("+++")) continue; + const t = line.slice(1).trim(); const m = t.match(/^pub\s+(?!fn\b|struct\b|enum\b|mod\b|use\b|trait\b|impl\b|const\b|static\b|type\b)(\w+)\s*:/); - if (m) fields.add(m[1]); + if (m) out.push({ name: m[1], lineIdx: i }); } - return Array.from(fields); + return out; +} + +// True if the field at `fieldLineIdx` lives inside a struct whose +// declaration carries `#[derive(... Serialize|Deserialize ...)]`. We +// walk backward through the diff (added + context lines both count — +// a struct declaration unchanged by the PR still appears as context) +// to find the nearest `pub struct` boundary, then scan a few lines +// above it for derive attributes. Conservative bounds: +// - 80 lines back to find `struct` (struct definitions can grow large) +// - 8 lines above the `struct` keyword for attribute lines +// Stops the struct-search early if we hit a `}` at zero indent +// (the previous scope) or another `pub struct` (we left ours). +function parentStructHasSerdeDerive(lines: string[], fieldLineIdx: number): boolean { + let structLineIdx = -1; + for (let i = fieldLineIdx - 1; i >= 0 && i >= fieldLineIdx - 80; i--) { + const raw = lines[i]; + if (typeof raw !== "string" || raw.length === 0) continue; + const body = stripDiffPrefix(raw); + const trimmed = body.trim(); + if (/^pub\s+struct\s+\w/.test(trimmed)) { + structLineIdx = i; + break; + } + // Closing brace at column 0 means the enclosing scope ended above + // the field — we're not actually inside a struct. + if (body.startsWith("}")) return false; + } + if (structLineIdx < 0) return false; + + for (let j = structLineIdx - 1; j >= 0 && j >= structLineIdx - 8; j--) { + const raw = lines[j]; + if (typeof raw !== "string") continue; + const trimmed = stripDiffPrefix(raw).trim(); + if (trimmed === "" || trimmed.startsWith("//") || trimmed.startsWith("///")) continue; + if (!trimmed.startsWith("#[")) break; + if (/derive\s*\([^)]*\b(Serialize|Deserialize)\b/.test(trimmed)) return true; + } + return false; +} + +// Strip leading +/-/space from a unified-diff line, leaving the raw +// source line. Handles the case where the line is shorter than 1 char +// (rare but real for empty-context lines). +function stripDiffPrefix(line: string): string { + if (line.length === 0) return line; + const c = line[0]; + if (c === "+" || c === "-" || c === " ") return line.slice(1); + return line; } // True if `pos` falls inside a double- or single-quoted string on this