From 3249183d61e2edcbdbfa622b442d357a9a073553 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 3 Aug 2026 15:37:09 +0100 Subject: [PATCH 1/3] =?UTF-8?q?fix(reviewer-eval):=20proposers=20drop=20al?= =?UTF-8?q?ready-landed=20candidates=20=E2=80=94=20telemetry/GHSA=20paths?= =?UTF-8?q?=20never=20re-queue=20a=20promoted=20source.url?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit propose-telemetry re-offered all four batch-2 telemetry candidates because alreadyInCorpus is computed only in mine-bots' merge; the telemetry and GHSA paths never consulted the promoted corpus at all. collectCorpusUrls moves to mine-common, and propose-common's makeHardDrop composes the promoted-corpus check with each proposer's own drop rules (counted as already-in-corpus). Co-Authored-By: Claude Fable 5 --- .../review/eval/reviewers/mine-bots.mts | 39 +--------------- .../review/eval/reviewers/mine-common.mts | 45 +++++++++++++++++-- .../eval/reviewers/propose/propose-common.mts | 9 ++++ .../eval/reviewers/propose/propose-ghsa.mts | 3 +- .../reviewers/propose/propose-telemetry.mts | 3 +- 5 files changed, 56 insertions(+), 43 deletions(-) diff --git a/gate-engine/review/eval/reviewers/mine-bots.mts b/gate-engine/review/eval/reviewers/mine-bots.mts index 9a6c2a3a..d72c89d0 100644 --- a/gate-engine/review/eval/reviewers/mine-bots.mts +++ b/gate-engine/review/eval/reviewers/mine-bots.mts @@ -21,7 +21,7 @@ */ import { execFileSync } from 'node:child_process'; -import { existsSync, readdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs'; +import { existsSync, renameSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -39,6 +39,7 @@ import { sqlString, } from './mine-bots-lib.mts'; import { + collectCorpusUrls, collectRepoArgs, readCandidatesFile, sqlite3Available, @@ -56,7 +57,6 @@ const HUMAN_AUTHORS = new Set(['norvalbv']); const DEFAULT_REPOS = ['benord-labs/frink', 'norvalbv/devkit']; const TRUNCATE_LEN = 4000; const EXCERPT_LEN = 500; -const CORPUS_CASES_FILE_RE = /^cases-.*\.jsonl$/; function gh(args) { return execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); @@ -261,41 +261,6 @@ function scopeForPr(dbPath, cache, repoFull, repoShort, prNumber) { return scopeRows; } -// --------------------------------------------------------------------------------------------- -// Merge / dedupe against existing candidates.jsonl and the promoted corpus. -// --------------------------------------------------------------------------------------------- - -function collectCorpusUrls(dir) { - const urls = new Set(); - let entries = []; - try { - entries = readdirSync(dir); - } catch { - return urls; - } - for (const name of entries) { - if (!CORPUS_CASES_FILE_RE.test(name)) continue; - let content = ''; - try { - content = readFileSync(path.join(dir, name), 'utf8'); - } catch (e) { - console.error(`mine-bots: corpus read failed for ${name} (${e.message?.split('\n')[0]})`); - continue; - } - for (const line of content.split('\n')) { - if (!line.trim()) continue; - try { - const row = JSON.parse(line); - const u = row?.source?.url; - if (u) urls.add(u); - } catch { - // skip malformed line - } - } - } - return urls; -} - // --------------------------------------------------------------------------------------------- // Main sweep. // --------------------------------------------------------------------------------------------- diff --git a/gate-engine/review/eval/reviewers/mine-common.mts b/gate-engine/review/eval/reviewers/mine-common.mts index ee24878e..e58dcf28 100644 --- a/gate-engine/review/eval/reviewers/mine-common.mts +++ b/gate-engine/review/eval/reviewers/mine-common.mts @@ -1,13 +1,50 @@ // @ts-nocheck — BENCH-ONLY (excluded from tsc, see tsconfig.json exclude); loose types deliberate. /** - * mine-common — the plumbing both miners (mine-bots.mts, mine-telemetry.mts) share: url-keyed - * candidates-file reading, `--repo` argv collection, and read-only sqlite3 access. Extracted so - * the two stay byte-identical by construction instead of by copy (commit-guard caught the copies). + * mine-common — the plumbing the miners (mine-bots.mts, mine-telemetry.mts) and proposers share: + * url-keyed candidates-file reading, `--repo` argv collection, promoted-corpus url collection, + * and read-only sqlite3 access. Extracted so consumers stay byte-identical by construction + * instead of by copy (commit-guard caught the copies). */ import { execFileSync } from 'node:child_process'; -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; + +const CORPUS_CASES_FILE_RE = /^cases-.*\.jsonl$/; + +/** Collect every source.url already promoted into a cases-*.jsonl corpus file in `dir` — + * the dedup key that keeps miners and proposers from re-offering landed candidates. */ +export function collectCorpusUrls(dir) { + const urls = new Set(); + let entries = []; + try { + entries = readdirSync(dir); + } catch { + return urls; + } + for (const name of entries) { + if (!CORPUS_CASES_FILE_RE.test(name)) continue; + let content = ''; + try { + content = readFileSync(path.join(dir, name), 'utf8'); + } catch (e) { + console.error(`mine-common: corpus read failed for ${name} (${e.message?.split('\n')[0]})`); + continue; + } + for (const line of content.split('\n')) { + if (!line.trim()) continue; + try { + const row = JSON.parse(line); + const u = row?.source?.url; + if (u) urls.add(u); + } catch { + // skip malformed line + } + } + } + return urls; +} /** Parse an existing url-keyed candidates .jsonl into a Map; malformed lines are * skipped rather than aborting the whole merge. */ diff --git a/gate-engine/review/eval/reviewers/propose/propose-common.mts b/gate-engine/review/eval/reviewers/propose/propose-common.mts index 5ea22ab3..0f875e35 100644 --- a/gate-engine/review/eval/reviewers/propose/propose-common.mts +++ b/gate-engine/review/eval/reviewers/propose/propose-common.mts @@ -10,6 +10,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { collectCorpusUrls } from '../mine-common.mts'; /** kebab slug from free text, for queue ids. */ export function slugify(text, maxWords = 5) { @@ -82,6 +83,14 @@ export function requireFile(file, name, hint) { } } +/** Compose a suite-specific drop function with the promoted-corpus check. Telemetry and GHSA + * candidates carry no alreadyInCorpus flag (that's mine-bots' merge concern), so the proposers + * enforce it here — a landed source.url must never be re-queued. */ +export function makeHardDrop(reviewersDir, dropReason) { + const corpusUrls = collectCorpusUrls(reviewersDir); + return (c) => (corpusUrls.has(c.url) ? 'already-in-corpus' : dropReason(c)); +} + /** Run the hard-drop filter over candidates, bumping the histogram; returns survivors. */ export function partitionDrops(candidates, hardDropReason, bump) { const kept = []; diff --git a/gate-engine/review/eval/reviewers/propose/propose-ghsa.mts b/gate-engine/review/eval/reviewers/propose/propose-ghsa.mts index a1f0c386..c80e7787 100644 --- a/gate-engine/review/eval/reviewers/propose/propose-ghsa.mts +++ b/gate-engine/review/eval/reviewers/propose/propose-ghsa.mts @@ -22,6 +22,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { makeDropCounter, + makeHardDrop, parseMaxArg, partitionDrops, printSummary, @@ -80,7 +81,7 @@ function main() { const candidates = readJsonl(CANDIDATES_FILE); const { drops, bump } = makeDropCounter(); - const kept = partitionDrops(candidates, hardDropReason, bump); + const kept = partitionDrops(candidates, makeHardDrop(reviewersDir, hardDropReason), bump); kept.sort(compareCandidates); const seenIds = new Set(); diff --git a/gate-engine/review/eval/reviewers/propose/propose-telemetry.mts b/gate-engine/review/eval/reviewers/propose/propose-telemetry.mts index 67b80147..02b6d1b2 100644 --- a/gate-engine/review/eval/reviewers/propose/propose-telemetry.mts +++ b/gate-engine/review/eval/reviewers/propose/propose-telemetry.mts @@ -24,6 +24,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { makeDropCounter, + makeHardDrop, parseMaxArg, partitionDrops, printSummary, @@ -75,7 +76,7 @@ function main() { const candidates = readJsonl(CANDIDATES_FILE); const { drops, bump } = makeDropCounter(); - const kept = partitionDrops(candidates, hardDropReason, bump); + const kept = partitionDrops(candidates, makeHardDrop(reviewersDir, hardDropReason), bump); kept.sort(compareCandidates); const queued = kept.slice(0, max); if (kept.length > max) bump(`over-max:${kept.length - max}`); From 26d3269df5604e26bc92ffe47c623e054448b191 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 3 Aug 2026 16:06:57 +0100 Subject: [PATCH 2/3] =?UTF-8?q?bench(reviewer-eval):=20first=20known-answe?= =?UTF-8?q?r=20batch=20+=20telemetry=20batch=203=20=E2=80=94=20absolute=20?= =?UTF-8?q?recall=20arrives=20for=20the=20security=20suites=20(sc-1408,=20?= =?UTF-8?q?sc-1410)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 28 rows / 14 gold+decoy pairs. correctness 128 -> 140 (75 gold / 65 decoy), api-security 30 -> 44, frontend-security 19 -> 21. provenance:'known-answer' is live with 8 golds (7 api-security, 1 frontend-security), each adapted from a fix-commit-anchored GHSA advisory — the first corpus rows whose labels are public facts rather than mined judgments, so the security suites can report an absolute-recall slice instead of precision + relative recall only. The 6 correctness rows come from gate telemetry (all tier-1, fail-diff archived). Adversarial pair review ran one verifier per pair and rejected 6 of 14 first drafts, establishing a house rule now recorded on the epic axis and in the runbook: a PASS twin must satisfy the target suite checklist's OWN stated PASS shape, not merely be safer than its gold. Three decoys were 'fixed' in ways the checklist still FAILs (escaped-but-still-interpolated SQL; a widened shell metacharacter class still handing a shell request data; credentials stripped but the cross-origin hop still followed) — each a guaranteed false-FAIL charged to a correct reviewer. Two more carried unlabeled second defects in the PASS twin, and one left the gold's own misclassification reachable in its twin. All six were repaired symmetrically so each pair's sole behavioral difference remains the labeled defect. Also adds mine-ghsa to the weekly sweep (the sc-1415 follow-up). Co-Authored-By: Claude Fable 5 --- docs/benchmarks/corpus-growth.md | 22 ++++++++++++++++++- .../benchmarks-grow-from-telemetry.md | 1 + .../eval/reviewers/cases-api-security.jsonl | 14 ++++++++++++ .../eval/reviewers/cases-correctness.jsonl | 12 ++++++++++ .../reviewers/cases-frontend-security.jsonl | 2 ++ .../eval/reviewers/propose/weekly-mining.sh | 1 + 6 files changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/benchmarks/corpus-growth.md b/docs/benchmarks/corpus-growth.md index 28e99c0e..fb3b854d 100644 --- a/docs/benchmarks/corpus-growth.md +++ b/docs/benchmarks/corpus-growth.md @@ -105,7 +105,27 @@ runner); freshness goes stale exactly when one moves. 5. **A bench the reviewer aces has gone stale.** The gap is the signal; the loop's job is to keep regenerating it, not to be driven to 1.0. -## State as of 2026-08-02 +## State as of 2026-08-03 + +- Corpus: correctness 140 (75 gold / 65 decoy), api-security 44 (23 gold), frontend-security 21 + (12 gold). Backend-perf 13, frontend-perf 19 — untouched. **`known-answer` provenance is live**: + 8 GHSA-derived golds (7 api-security, 1 frontend-security), the first rows whose labels are + public facts rather than mined judgments, so the security suites can finally report an + absolute-recall slice (measurement-rule 2) instead of precision + relative recall only. +- **House rule, security suites (2026-08-03, from adversarial pair review):** a PASS twin must + satisfy the target suite checklist's OWN stated PASS shape — not merely be safer than its gold. + Six of fourteen first-draft pairs failed this. Three decoys were "fixed" in ways the checklist + still FAILs: better SQL escaping while still interpolating (bound parameters are the only + passing shape), a widened shell-metacharacter class while still handing a shell request data + (only `execFile` + argv passes), and origin-stripped credentials while still following the + cross-origin hop. Each is a guaranteed false-FAIL charged to a correct reviewer. **Corollary: + escaper-completeness defects can only be benchmarked as golds here — no escaping-based PASS + exists in these checklists.** Two further decoys carried unlabeled second defects (a dropped + mass-assignment allowlist; a `JSON.stringify` escaper spliced into a JSX *attribute* position, + where backslash is not an escape character) and one left the gold's own misclassification + reachable in its twin. Read the target SKILL.md's PASS clause before authoring any decoy. + +### Earlier: state as of 2026-08-02 - Corpus: correctness 66→120 (65 gold / 55 decoy), api-security 14→30 (first mined domain rows). Backend-perf 13, frontend pair 31 each — untouched this cycle. diff --git a/docs/decisions/benchmarks-grow-from-telemetry.md b/docs/decisions/benchmarks-grow-from-telemetry.md index 4c561da2..e9856454 100644 --- a/docs/decisions/benchmarks-grow-from-telemetry.md +++ b/docs/decisions/benchmarks-grow-from-telemetry.md @@ -25,3 +25,4 @@ created: 2026-08-01 - 2026-08-03 — rowHash scope narrowed for comparisons (behaviorHash): pairing/salvage/staleness now key on the behavior-bearing slice (reviewer, expected, expectItems, reasonPattern, repo) when both sides carry it, falling back to strict full-row rowHash for old baselines — additive, no epoch break. Forced by the accepted→stale bouncing the owner flagged: honest metadata corrections (sc-1400, sc-1416 both produced them) were paying full staleness+exclusion cost for edits that cannot change a verdict. Full rowHash retained as the written record; documentation fields (note, provenance, source, outcomeEvidence, scopeConfirmed, caseId, difficulty, holdout) excluded from the comparison key - 2026-08-03 — sc-1408 ratified + built (owner chose now, option (a)): mine-ghsa.mts sweeps npm GitHub Security Advisories keeping only fix-commit-anchored entries (165/400 on first sweep); propose/propose-ghsa.mts enriches each with the fix commit's per-file patches — pre-image = confirmed-vulnerable, commit = confirmed fix. Adapted rows will carry provenance:'known-answer' (new enum value), the only provenance whose golds support ABSOLUTE recall. SecBench.js deliberately unread (unlicensed — index only). Option (b) (CR-Bench recipe over SWE-Bench Multimodal, correctness suite) deferred post-epic. First adapted batch = the remaining sc-1408 step - 2026-08-03 — sc-1415 weekly routine ruled + built (owner chose automation over the wait-for-toil recommendation): local cron (Mon 09:00) runs propose/weekly-mining.sh — both miners + both propose stages, notify-only into ~/.claude-usage/weekly-mining.log. Local-not-cloud because mine-telemetry needs this machine's collector db/diff archive; adaptation (fixture authoring) deliberately stays un-automated — it is the judgment step. The cron entry points at the main checkout so it always runs the merged code +- 2026-08-03 — 2026-08-03 — first GHSA known-answer batch + telemetry batch 3 landed (sc-1408 final step, sc-1410): 28 rows / 14 pairs — correctness 128→140, api-security 30→44, frontend-security 19→21; provenance 'known-answer' is live with 8 golds (7 api-security, 1 frontend-security), the first slice supporting ABSOLUTE recall. Adversarial pair review (one verifier per pair) rejected 6 of 14 first drafts and forced a house-rule EXTENSION now binding on every future security row: a PASS twin must satisfy the target suite checklist's OWN stated PASS shape, not merely be safer than its gold. Three decoys were 'fixed' in a way the checklist still FAILs — better SQL escaping while still interpolating (bound parameters are the only passing shape), a widened shell-metacharacter class while still handing a shell request data (only execFile+argv passes), and origin-stripped credentials while still following the cross-origin hop (ssrf-prevention wants the hop refused). Each would have been a guaranteed false-FAIL scored against a correct reviewer. Two more carried unlabeled second defects in the PASS twin (a dropped mass-assignment allowlist; a JSON.stringify escaper spliced into a JSX attribute position, where backslash is not an escape) and one left the gold's own misclassification reachable in its decoy via a bare '#' matching TS private fields. Corollary: escaper-completeness defects can only be benchmarked as golds in these suites — the checklist admits no escaping-based PASS diff --git a/gate-engine/review/eval/reviewers/cases-api-security.jsonl b/gate-engine/review/eval/reviewers/cases-api-security.jsonl index efb7faf8..507bbc52 100644 --- a/gate-engine/review/eval/reviewers/cases-api-security.jsonl +++ b/gate-engine/review/eval/reviewers/cases-api-security.jsonl @@ -28,3 +28,17 @@ {"id": "apisec-json-escaped-secret-leak", "reviewer": "api-security-reviewer", "expected": "FAIL", "expectItems": ["output-security"], "reasonPattern": "escape|decode|unicode|raw|redact|secret|leak|json\\.parse", "repo": {"base": {"api/auditExport.ts": "import type { Request, Response } from 'express';\nimport { fetchAuditRecord } from './auditStore';\n\nconst SECRET_RE = /sk_live_[a-zA-Z0-9]+/;\n\n// Redact obvious secret-shaped tokens before an audit record ever reaches the\n// client — decode first, so a JSON-escaped token can't slip past the regex.\nfunction containsSecret(rawJson: string): boolean {\n const decoded = JSON.parse(rawJson);\n return SECRET_RE.test(JSON.stringify(decoded));\n}\n\nexport async function getAuditRecord(req: Request, res: Response): Promise {\n const record = await fetchAuditRecord(String(req.params.id));\n const rawJson = JSON.stringify(record);\n if (containsSecret(rawJson)) {\n res.status(500).json({ message: 'Audit record failed redaction check' });\n return;\n }\n res.json(record);\n}\n"}, "staged": {"api/auditExport.ts": "import type { Request, Response } from 'express';\nimport { fetchAuditRecord, fetchRawAuditLog } from './auditStore';\n\nconst SECRET_RE = /sk_live_[a-zA-Z0-9]+/;\n\n// Redact obvious secret-shaped tokens before an audit record ever reaches the\n// client — decode first, so a JSON-escaped token can't slip past the regex.\nfunction containsSecret(rawJson: string): boolean {\n const decoded = JSON.parse(rawJson);\n return SECRET_RE.test(JSON.stringify(decoded));\n}\n\nexport async function getAuditRecord(req: Request, res: Response): Promise {\n const record = await fetchAuditRecord(String(req.params.id));\n const rawJson = JSON.stringify(record);\n if (containsSecret(rawJson)) {\n res.status(500).json({ message: 'Audit record failed redaction check' });\n return;\n }\n res.json(record);\n}\n\n// New: stream the raw stored audit log lines straight back to the client,\n// scanning each line's on-disk text before it goes out.\nexport async function exportRawAuditLog(req: Request, res: Response): Promise {\n const lines = await fetchRawAuditLog(String(req.query.date));\n const clean = lines.filter((line) => !SECRET_RE.test(line));\n res.type('application/jsonl').send(clean.join('\\n'));\n}\n"}}, "note": "exportRawAuditLog tests SECRET_RE against each on-disk JSONL line's raw text — a token stored with JSON string escapes (e.g. `sk\\u005flive\\u005f...`) never matches the literal pattern, so it slips past the filter and reaches the client untouched. getAuditRecord above shows the safe pattern: decode, then re-stringify before scanning.", "difficulty": "borderline", "provenance": "mined", "caseId": "apisec-json-escaped-secret-leak", "sourcePr": 76, "variantOf": null, "holdout": true, "outcomeEvidence": "addressed-marker", "scopeConfirmed": "unverifiable"} {"id": "apisec-supply-chain-age-gate-disabled-fixed", "reviewer": "api-security-reviewer", "expected": "PASS", "repo": {"base": {"api/dependencyPolicy.toml": "# Third-party dependency install policy for this service's build pipeline.\n# New releases must sit for seven days before they are pulled into installs,\n# giving time to spot a bad or compromised release upstream.\nminimumReleaseAge = 604800\nminimumReleaseAgeExcludes = []\n"}, "staged": {"api/dependencyPolicy.toml": "# Third-party dependency install policy for this service's build pipeline.\n# New releases must sit for seven days before they are pulled into installs,\n# giving time to spot a bad or compromised release upstream.\nminimumReleaseAge = 604800\nminimumReleaseAgeExcludes = [\"internal-report-tool\"]\n"}}, "note": "The seven-day minimumReleaseAge delay stays in place; only a single, narrowly-scoped package is added to minimumReleaseAgeExcludes. No global weakening of the supply-chain gate. PASS.", "difficulty": "borderline", "provenance": "adapted", "caseId": "apisec-supply-chain-age-gate-disabled", "sourcePr": 77, "variantOf": "apisec-supply-chain-age-gate-disabled", "holdout": false, "outcomeEvidence": "resolved+line-touched", "scopeConfirmed": "unverifiable"} {"id": "apisec-supply-chain-age-gate-disabled", "reviewer": "api-security-reviewer", "expected": "FAIL", "expectItems": ["general-security"], "reasonPattern": "supply.?chain|release.?age|minimumReleaseAge|delay|compromised|disable", "repo": {"base": {"api/dependencyPolicy.toml": "# Third-party dependency install policy for this service's build pipeline.\n# New releases must sit for seven days before they are pulled into installs,\n# giving time to spot a bad or compromised release upstream.\nminimumReleaseAge = 604800\nminimumReleaseAgeExcludes = []\n"}, "staged": {"api/dependencyPolicy.toml": "# Third-party dependency install policy for this service's build pipeline.\n# New releases must sit for seven days before they are pulled into installs,\n# giving time to spot a bad or compromised release upstream.\nminimumReleaseAge = 0\nminimumReleaseAgeExcludes = []\n"}}, "note": "minimumReleaseAge is set to 0, removing the seven-day install delay for every third-party package the service pulls in — a newly published, compromised release becomes installable immediately instead of being held back long enough for it to be caught upstream.", "difficulty": "borderline", "provenance": "mined", "caseId": "apisec-supply-chain-age-gate-disabled", "sourcePr": 77, "variantOf": null, "holdout": false, "outcomeEvidence": "resolved+line-touched", "scopeConfirmed": "unverifiable"} +{"id":"apisec-cmd-escape-missing-parens-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/exportRun.ts":"import { exec } from 'node:child_process';\nimport { promisify } from 'node:util';\n\nconst runShell = promisify(exec);\nconst SHELL = 'cmd.exe';\n\nexport async function runNightlyExport(): Promise {\n const cmd = 'if defined EXPORT_DIR (report-tool --out %EXPORT_DIR% --label nightly)';\n const { stdout } = await runShell(cmd, { shell: SHELL });\n return stdout;\n}\n"},"staged":{"api/exportRun.ts":"import { exec, execFile } from 'node:child_process';\nimport { promisify } from 'node:util';\nimport type { Request, Response } from 'express';\n\nconst runShell = promisify(exec);\nconst runTool = promisify(execFile);\nconst SHELL = 'cmd.exe';\n\nexport async function runNightlyExport(): Promise {\n const cmd = 'if defined EXPORT_DIR (report-tool --out %EXPORT_DIR% --label nightly)';\n const { stdout } = await runShell(cmd, { shell: SHELL });\n return stdout;\n}\n\nexport async function runLabeledExport(req: Request, res: Response): Promise {\n const outDir = process.env.EXPORT_DIR;\n if (!outDir) {\n res.status(503).end();\n return;\n }\n const label = String(req.query.label);\n const { stdout } = await runTool('report-tool', ['--out', outDir, '--label', label]);\n res.type('text/plain').send(stdout);\n}\n"}},"note":"Same new labeled-export endpoint as the gold twin, but the caller value never reaches a command line: the tool is invoked with a fixed binary name and an argv array, so the label is delivered as one argument and no shell parses it. The parenthesised CMD block is gone with the shell, and the EXPORT_DIR conditional it provided is done in code instead. The pre-existing nightly job keeps its own fixed, value-free shell string, untouched by this change. PASS.","difficulty":"adversarial","provenance":"adapted","source":{"repo":"ericcornelissen/shescape","url":"https://github.com/advisories/GHSA-w4hw-qcx7-56pr"},"caseId":"apisec-cmd-escape-missing-parens","variantOf":"apisec-cmd-escape-missing-parens","holdout":false} +{"id":"apisec-cmd-escape-missing-parens","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["command-injection"],"reasonPattern":"inject|escap|metachar|parenthes|caret|shell|breakout|incomplete","repo":{"base":{"api/exportRun.ts":"import { exec } from 'node:child_process';\nimport { promisify } from 'node:util';\n\nconst runShell = promisify(exec);\nconst SHELL = 'cmd.exe';\n\nexport async function runNightlyExport(): Promise {\n const cmd = 'if defined EXPORT_DIR (report-tool --out %EXPORT_DIR% --label nightly)';\n const { stdout } = await runShell(cmd, { shell: SHELL });\n return stdout;\n}\n"},"staged":{"api/exportRun.ts":"import { exec } from 'node:child_process';\nimport { promisify } from 'node:util';\nimport type { Request, Response } from 'express';\n\nconst runShell = promisify(exec);\nconst SHELL = 'cmd.exe';\nconst SPECIALS = /([%&<>^|])/g;\n\nexport async function runNightlyExport(): Promise {\n const cmd = 'if defined EXPORT_DIR (report-tool --out %EXPORT_DIR% --label nightly)';\n const { stdout } = await runShell(cmd, { shell: SHELL });\n return stdout;\n}\n\nfunction shellArg(value: string): string {\n const flat = value.replace(/[\\0\\r]/g, '').replace(/\\n/g, ' ').replace(/\"/g, '\"\"');\n return flat.replace(SPECIALS, '^$1');\n}\n\nexport async function runLabeledExport(req: Request, res: Response): Promise {\n const label = shellArg(String(req.query.label));\n const cmd = `if defined EXPORT_DIR (report-tool --out %EXPORT_DIR% --label ${label})`;\n const { stdout } = await runShell(cmd, { shell: SHELL });\n res.type('text/plain').send(stdout);\n}\n"}},"note":"The new runLabeledExport endpoint interpolates req.query.label into a cmd.exe command line that wraps the tool invocation in a parenthesised block, and the helper it trusts only carets %, &, <, >, ^ and |. Round and closing brackets are absent from the character class, so a label of `x) else if a==a (whoami` closes the block early and CMD then executes a second, attacker-chosen command in the same line. Doubling the double quote does not help either: the payload needs no quote. The presence of a sanitiser is what makes this dangerous — the diff reads as defended while the one character class that matters is short. Same defect as GHSA-w4hw-qcx7-56pr (shescape < 2.1.14), whose fix added ( and ) to exactly this class. FAIL on command-injection is correct.","difficulty":"adversarial","provenance":"known-answer","source":{"repo":"ericcornelissen/shescape","url":"https://github.com/advisories/GHSA-w4hw-qcx7-56pr"},"caseId":"apisec-cmd-escape-missing-parens","holdout":true} +{"id":"apisec-orm-literal-filter-injection-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/threads-list.ts":"import { Op } from 'sequelize';\nimport { db } from './db';\n\ntype ListParams = { filter?: Record; limit?: number };\n\nexport async function listThreads(params: ListParams) {\n const { filter = {}, limit = 30 } = params;\n const clauses: any[] = [];\n if (filter.id) clauses.push({ id: filter.id });\n if (filter.name) clauses.push({ name: filter.name });\n const where = { [Op.and]: clauses };\n return db.model('threads').findAll({ where, limit: Math.min(limit, 100) });\n}\n"},"staged":{"api/threads-list.ts":"import { Op, Sequelize } from 'sequelize';\nimport { db, HttpError } from './db';\n\ntype ListParams = { filter?: Record; limit?: number };\n\nconst LAST_SEEN_SQL = '(SELECT MAX(n.seen_at) FROM notes n WHERE n.thread_id = threads.id)';\n\nfunction cutoffOf(raw: unknown): number | null {\n if (raw === undefined || raw === null) return null;\n const epoch = typeof raw === 'number' || typeof raw === 'string' ? Number(raw) : Number.NaN;\n if (!Number.isFinite(epoch)) throw new HttpError(400, 'bad lastSeenBefore');\n return epoch;\n}\n\nexport async function listThreads(params: ListParams) {\n const { filter = {}, limit = 30 } = params;\n const clauses: any[] = [];\n if (filter.id) clauses.push({ id: filter.id });\n if (filter.name) clauses.push({ name: filter.name });\n const before = cutoffOf(filter.lastSeenBefore?.$lt);\n if (before !== null) clauses.push(Sequelize.where(Sequelize.literal(LAST_SEEN_SQL), Op.lt, before));\n const where = { [Op.and]: clauses };\n return db.model('threads').findAll({ where, limit: Math.min(limit, 100) });\n}\n"}},"note":"Same lastSeenBefore filter feature as the gold twin, but the caller value goes through cutoffOf first: anything that is not a finite number (or a string that converts to one) is rejected with a 400, and the accepted number is handed to Sequelize.where as an operand rather than spliced into the literal. The literal now holds only the fixed subquery text, so no caller-controlled character reaches the SQL string. PASS.","difficulty":"borderline","provenance":"adapted","source":{"repo":"nocobase/nocobase","url":"https://github.com/advisories/GHSA-p849-8hwh-84j9"},"caseId":"apisec-orm-literal-filter-injection","variantOf":"apisec-orm-literal-filter-injection","holdout":false} +{"id":"apisec-orm-literal-filter-injection","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["sql-injection"],"reasonPattern":"inject|interpolat|parameteri|bind|unvalidated|sanitiz|sql","repo":{"base":{"api/threads-list.ts":"import { Op } from 'sequelize';\nimport { db } from './db';\n\ntype ListParams = { filter?: Record; limit?: number };\n\nexport async function listThreads(params: ListParams) {\n const { filter = {}, limit = 30 } = params;\n const clauses: any[] = [];\n if (filter.id) clauses.push({ id: filter.id });\n if (filter.name) clauses.push({ name: filter.name });\n const where = { [Op.and]: clauses };\n return db.model('threads').findAll({ where, limit: Math.min(limit, 100) });\n}\n"},"staged":{"api/threads-list.ts":"import { Op, Sequelize } from 'sequelize';\nimport { db } from './db';\n\ntype ListParams = { filter?: Record; limit?: number };\n\nconst LAST_SEEN_SQL = '(SELECT MAX(n.seen_at) FROM notes n WHERE n.thread_id = threads.id)';\n\nexport async function listThreads(params: ListParams) {\n const { filter = {}, limit = 30 } = params;\n const clauses: any[] = [];\n if (filter.id) clauses.push({ id: filter.id });\n if (filter.name) clauses.push({ name: filter.name });\n const before = filter.lastSeenBefore?.$lt;\n if (before) clauses.push(Sequelize.literal(`${LAST_SEEN_SQL} < ${before}`));\n const where = { [Op.and]: clauses };\n return db.model('threads').findAll({ where, limit: Math.min(limit, 100) });\n}\n"}},"note":"The new lastSeenBefore branch drops the caller-supplied filter.lastSeenBefore.$lt straight into a Sequelize.literal template string, so whatever arrives in that nested query parameter becomes raw SQL inside the WHERE clause. The value is never checked for numeric shape, never cast and never bound — `filter[lastSeenBefore][$lt]=0) OR 1=1 --`, or a stacked statement on Postgres, rewrites the query. Using an ORM elsewhere in the handler does not help: literal() is the documented escape hatch out of the ORM's binding. This is the shape of GHSA-p849-8hwh-84j9 (CVE-2026-52887), where the same nested-filter interpolation reached PG-superuser RCE. FAIL on sql-injection is correct.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"nocobase/nocobase","url":"https://github.com/advisories/GHSA-p849-8hwh-84j9"},"caseId":"apisec-orm-literal-filter-injection","holdout":false} +{"id":"apisec-predictable-key-math-random-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/api-key-service.ts":"import { randomBytes } from 'crypto';\nimport { saveKey } from './key-store';\n\nconst KEY_BYTES = 32;\n\nexport async function issueApiKey(accountId: string) {\n const secret = randomBytes(KEY_BYTES).toString('hex');\n await saveKey(accountId, secret);\n return { accountId, secret };\n}\n"},"staged":{"api/api-key-service.ts":"import { saveKey } from './key-store';\n\nconst KEY_BYTES = 32;\n\n// portable across browser preview and server runtimes\nfunction fillRandom(buf: Uint8Array) {\n if (typeof window !== 'undefined' && window.crypto?.getRandomValues) {\n window.crypto.getRandomValues(buf);\n return buf;\n }\n globalThis.crypto.getRandomValues(buf);\n return buf;\n}\n\nfunction toHex(buf: Uint8Array) {\n return Array.from(buf, (b) => b.toString(16).padStart(2, '0')).join('');\n}\n\nexport async function issueApiKey(accountId: string) {\n const secret = toHex(fillRandom(new Uint8Array(KEY_BYTES)));\n await saveKey(accountId, secret);\n return { accountId, secret };\n}\n"}},"note":"Same runtime-portable restructure as the gold twin, but the non-browser branch calls globalThis.crypto.getRandomValues — Node's Web Crypto CSPRNG (available since Node 19, and in workers/edge runtimes) — so key material stays cryptographically sourced on every path. This mirrors the upstream sm-crypto 0.5.0 fix, which moved off jsbn's window-gated SecureRandom to globalThis crypto. No other behavior differs from the gold twin. PASS.","difficulty":"clear","provenance":"adapted","source":{"repo":"JuneAndGreen/sm-crypto","url":"https://github.com/advisories/GHSA-vh45-f885-3848"},"caseId":"apisec-predictable-key-math-random","variantOf":"apisec-predictable-key-math-random","holdout":false} +{"id":"apisec-predictable-key-math-random","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["general-security"],"reasonPattern":"Math\\.random|predictab|guessab|weak|insecure|non-?cryptographic|csprng|entropy","repo":{"base":{"api/api-key-service.ts":"import { randomBytes } from 'crypto';\nimport { saveKey } from './key-store';\n\nconst KEY_BYTES = 32;\n\nexport async function issueApiKey(accountId: string) {\n const secret = randomBytes(KEY_BYTES).toString('hex');\n await saveKey(accountId, secret);\n return { accountId, secret };\n}\n"},"staged":{"api/api-key-service.ts":"import { saveKey } from './key-store';\n\nconst KEY_BYTES = 32;\n\n// portable across browser preview and server runtimes\nfunction fillRandom(buf: Uint8Array) {\n if (typeof window !== 'undefined' && window.crypto?.getRandomValues) {\n window.crypto.getRandomValues(buf);\n return buf;\n }\n for (let i = 0; i < buf.length; i++) {\n buf[i] = Math.floor(Math.random() * 256) ^ (Date.now() & 0xff);\n }\n return buf;\n}\n\nfunction toHex(buf: Uint8Array) {\n return Array.from(buf, (b) => b.toString(16).padStart(2, '0')).join('');\n}\n\nexport async function issueApiKey(accountId: string) {\n const secret = toHex(fillRandom(new Uint8Array(KEY_BYTES)));\n await saveKey(accountId, secret);\n return { accountId, secret };\n}\n"}},"note":"The staged change replaces Node's crypto.randomBytes with a runtime-portable helper whose non-browser branch fills every secret byte from Math.random() (V8 xorshift128+, state recoverable from a handful of outputs) XORed with the low byte of Date.now() (attacker-estimable wall clock). On the API server `window` is always undefined, so the seemingly-safe getRandomValues branch never runs and every issued API-key secret comes from the fallback — an attacker who observes a few Math.random outputs or brute-forces the timestamp can reconstruct issued keys. This is the exact mechanism of GHSA-vh45-f885-3848 (sm-crypto < 0.5.0): jsbn's SecureRandom checked window.crypto, which is undefined in Node, and silently seeded SM2 private keys from Math.random + new Date().getTime(). FAIL on general-security is correct.","difficulty":"clear","provenance":"known-answer","source":{"repo":"JuneAndGreen/sm-crypto","url":"https://github.com/advisories/GHSA-vh45-f885-3848"},"caseId":"apisec-predictable-key-math-random","holdout":true} +{"id":"apisec-proto-pollution-patch-path-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/documentPatch.ts":"import type { Request, Response } from 'express';\n\nconst store: Record> = {};\n\nconst EDITABLE = ['title', 'summary', 'status'];\n\nexport function patchDocument(req: Request, res: Response): void {\n const doc = (store[String(req.params.id)] ??= {});\n for (const field of EDITABLE) {\n if (Object.hasOwn(req.body, field)) doc[field] = req.body[field];\n }\n res.json({ ok: true });\n}\n"},"staged":{"api/documentPatch.ts":"import type { Request, Response } from 'express';\n\nconst store: Record> = {};\nconst EDITABLE = ['title', 'summary', 'status'];\nconst BLOCKED = new Set(['__proto__', 'constructor', 'prototype']);\n// New: callers may address nested fields with dotted keys in the patch body.\nfunction setPath(doc: Record, path: string, value: unknown): void {\n const parts = path.split('.');\n if (!EDITABLE.includes(parts[0])) throw new Error('unknown field');\n if (parts.some((part) => BLOCKED.has(part))) throw new Error('blocked field name');\n let node = doc as Record;\n for (const part of parts.slice(0, -1)) {\n if (typeof node[part] !== 'object' || node[part] === null) node[part] = {};\n node = node[part];\n }\n node[parts[parts.length - 1]] = value;\n}\n\nexport function patchDocument(req: Request, res: Response): void {\n const doc = (store[String(req.params.id)] ??= {});\n try {\n for (const [key, value] of Object.entries(req.body.$set ?? {})) setPath(doc, key, value);\n res.json({ ok: true });\n } catch {\n res.status(400).json({ message: 'Invalid field path' });\n }\n}\n"}},"note":"Same dotted-key patch handler as the gold twin, but setPath refuses any key whose segments include __proto__, constructor or prototype — every segment, not just the first — and the route answers 400 instead. No write can reach Object.prototype, so the nested-update feature is safe. The root-segment allowlist is unchanged from base in both twins, so field scope is not the axis under test here. PASS.","difficulty":"borderline","provenance":"adapted","source":{"repo":"apostrophecms/apostrophe","url":"https://github.com/advisories/GHSA-6h5j-32cf-4253"},"caseId":"apisec-proto-pollution-patch-path","variantOf":"apisec-proto-pollution-patch-path","holdout":false} +{"id":"apisec-proto-pollution-patch-path","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["input-validation"],"reasonPattern":"proto|pollut|constructor|prototype|unsafe (path|key|segment)|dangerous (path|key|segment)","repo":{"base":{"api/documentPatch.ts":"import type { Request, Response } from 'express';\n\nconst store: Record> = {};\n\nconst EDITABLE = ['title', 'summary', 'status'];\n\nexport function patchDocument(req: Request, res: Response): void {\n const doc = (store[String(req.params.id)] ??= {});\n for (const field of EDITABLE) {\n if (Object.hasOwn(req.body, field)) doc[field] = req.body[field];\n }\n res.json({ ok: true });\n}\n"},"staged":{"api/documentPatch.ts":"import type { Request, Response } from 'express';\n\nconst store: Record> = {};\nconst EDITABLE = ['title', 'summary', 'status'];\n// New: callers may address nested fields with dotted keys in the patch body.\nfunction setPath(doc: Record, path: string, value: unknown): void {\n const parts = path.split('.');\n if (!EDITABLE.includes(parts[0])) throw new Error('unknown field');\n let node = doc as Record;\n for (const part of parts.slice(0, -1)) {\n if (typeof node[part] !== 'object' || node[part] === null) node[part] = {};\n node = node[part];\n }\n node[parts[parts.length - 1]] = value;\n}\n\nexport function patchDocument(req: Request, res: Response): void {\n const doc = (store[String(req.params.id)] ??= {});\n try {\n for (const [key, value] of Object.entries(req.body.$set ?? {})) setPath(doc, key, value);\n res.json({ ok: true });\n } catch {\n res.status(400).json({ message: 'Invalid field path' });\n }\n}\n"}},"note":"setPath walks a request-supplied dotted key and auto-creates each intermediate object, with no rejection of __proto__/constructor/prototype. The root-segment allowlist does not help, because only the FIRST segment is checked: a PATCH body of { $set: { \"summary.__proto__.isAdmin\": true } } passes the allowlist on \"summary\", then walks into Object.prototype and writes there, so every plain object in the process inherits isAdmin — a process-wide pollution that poisons later authorization checks, not just this document. Server-side prototype pollution (CWE-1321), the same class as GHSA-6h5j-32cf-4253.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"apostrophecms/apostrophe","url":"https://github.com/advisories/GHSA-6h5j-32cf-4253"},"caseId":"apisec-proto-pollution-patch-path","holdout":false} +{"id":"apisec-quote-only-escape-breakout-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/eventStats.ts":"import type { Request, Response } from 'express';\nimport { columnStore } from './column-store';\n\n// The driver here takes finished statement text; values are rendered inline.\nfunction toText(value: number | boolean): string {\n return String(value);\n}\n\nexport async function eventStats(req: Request, res: Response): Promise {\n const days = Number(req.query.days) || 7;\n const stmt = `SELECT name, count() FROM events WHERE days_ago < ${toText(days)} GROUP BY name`;\n res.json(await columnStore.run(stmt));\n}\n"},"staged":{"api/eventStats.ts":"import type { Request, Response } from 'express';\nimport { columnStore } from './column-store';\n\n// The driver here takes finished statement text; values are rendered inline.\nfunction toText(value: number | boolean): string {\n return String(value);\n}\n\nexport async function eventStats(req: Request, res: Response): Promise {\n const days = Number(req.query.days) || 7;\n const source = String(req.query.source ?? 'web');\n if (source.length > 64) {\n res.status(400).json({ message: 'source too long' });\n return;\n }\n const stmt =\n `SELECT name, count() FROM events WHERE days_ago < ${toText(days)}` +\n ' AND source = {source:String} GROUP BY name';\n res.json(await columnStore.run(stmt, { source }));\n}\n"}},"note":"Same source-filter feature as the gold twin, but the caller value never enters the statement text: the filter is a server-side bound parameter ({source:String}) supplied out of band, so no caller-supplied character can reach the parser as syntax and toText is left exactly as base had it. The pre-existing numeric days interpolation is untouched by this change. PASS.","difficulty":"borderline","provenance":"adapted","source":{"repo":"hypequery/hypequery","url":"https://github.com/advisories/GHSA-6wcc-39rp-hh9p"},"caseId":"apisec-quote-only-escape-breakout","variantOf":"apisec-quote-only-escape-breakout","holdout":false} +{"id":"apisec-quote-only-escape-breakout","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["sql-injection"],"reasonPattern":"inject|escap|backslash|interpolat|parameteri|sanitiz|quot|concat|sql","repo":{"base":{"api/eventStats.ts":"import type { Request, Response } from 'express';\nimport { columnStore } from './column-store';\n\n// The driver here takes finished statement text; values are rendered inline.\nfunction toText(value: number | boolean): string {\n return String(value);\n}\n\nexport async function eventStats(req: Request, res: Response): Promise {\n const days = Number(req.query.days) || 7;\n const stmt = `SELECT name, count() FROM events WHERE days_ago < ${toText(days)} GROUP BY name`;\n res.json(await columnStore.run(stmt));\n}\n"},"staged":{"api/eventStats.ts":"import type { Request, Response } from 'express';\nimport { columnStore } from './column-store';\n\n// The driver here takes finished statement text; values are rendered inline.\nfunction toText(value: string | number | boolean): string {\n if (typeof value === 'string') return `'${value.replace(/'/g, \"''\")}'`;\n return String(value);\n}\n\nexport async function eventStats(req: Request, res: Response): Promise {\n const days = Number(req.query.days) || 7;\n const source = String(req.query.source ?? 'web');\n if (source.length > 64) {\n res.status(400).json({ message: 'source too long' });\n return;\n }\n const stmt =\n `SELECT name, count() FROM events WHERE days_ago < ${toText(days)}` +\n ` AND source = ${toText(source)} GROUP BY name`;\n res.json(await columnStore.run(stmt));\n}\n"}},"note":"The new string branch of toText doubles single quotes and nothing else, so a caller-supplied source value ending in a backslash leaves that backslash immediately before the closing delimiter. The column store honours C-style escapes as well as the doubled-quote form, so the trailing backslash consumes the closing delimiter and everything the attacker sends after it is parsed as statement syntax rather than as data (`?source=%5C` plus a second controlled field is enough to append OR 1=1 -- or a stacked statement). The length check bounds the payload but permits every character, so it does not close the hole. The days value is numeric and therefore harmless; the defect arrives entirely with the new string handling introduced in this change. This is GHSA-6wcc-39rp-hh9p / CVE-2026-54658. FAIL on sql-injection is correct.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"hypequery/hypequery","url":"https://github.com/advisories/GHSA-6wcc-39rp-hh9p"},"caseId":"apisec-quote-only-escape-breakout","holdout":true} +{"id":"apisec-redirect-credential-forward-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/connectors/credentials.ts":"import { store } from './store';\n\nexport type OutboundHeaders = Record;\n\nexport async function connectorHeaders(connectorId: string): Promise {\n const conn = await store.connector(connectorId);\n const headers: OutboundHeaders = { ...(conn.staticHeaders ?? {}) };\n if (conn.token) headers.Authorization = `Bearer ${conn.token}`;\n return headers;\n}\n","api/connectors/outbound.ts":"import { connectorHeaders, OutboundHeaders } from './credentials';\n\nexport type OutboundOptions = { connectorId: string; method?: string };\n\nexport async function callConnector(target: string, opts: OutboundOptions) {\n const headers: OutboundHeaders = await connectorHeaders(opts.connectorId);\n const init = { method: opts.method ?? 'GET', headers, redirect: 'manual' as const };\n const res = await fetch(target, init);\n if (res.status >= 300 && res.status <= 399) throw new Error('unexpected hop');\n return res;\n}\n"},"staged":{"api/connectors/credentials.ts":"import { store } from './store';\n\nexport type OutboundHeaders = Record;\n\nexport async function connectorHeaders(connectorId: string): Promise {\n const conn = await store.connector(connectorId);\n const headers: OutboundHeaders = { ...(conn.staticHeaders ?? {}) };\n if (conn.token) headers.Authorization = `Bearer ${conn.token}`;\n return headers;\n}\n","api/connectors/outbound.ts":"import { connectorHeaders, OutboundHeaders } from './credentials';\n\nexport type OutboundOptions = { connectorId: string; method?: string; maxHops?: number };\n\nexport async function callConnector(target: string, opts: OutboundOptions) {\n const headers: OutboundHeaders = await connectorHeaders(opts.connectorId);\n const init = { method: opts.method ?? 'GET', headers, redirect: 'manual' as const };\n let url = target;\n for (let hop = 0; hop < (opts.maxHops ?? 4); hop++) {\n const res = await fetch(url, init);\n if (res.status < 300 || res.status > 399) return res;\n const location = res.headers.get('location');\n if (!location) return res;\n const next = new URL(location, url);\n if (next.origin !== new URL(url).origin) throw new Error('cross-origin redirect refused');\n url = next.toString();\n }\n throw new Error('too many hops');\n}\n"}},"note":"Same redirect-following loop as the gold twin, but a hop whose target origin differs from the current one is refused outright rather than followed. The stored bearer and static connector headers can never be replayed to another host, and the loop cannot be steered at an internal address either, so the new capability stays confined to the origin the caller already named. PASS.","difficulty":"borderline","provenance":"adapted","source":{"repo":"Budibase/budibase","url":"https://github.com/advisories/GHSA-mqhr-6j6h-74p5"},"caseId":"apisec-redirect-credential-forward","variantOf":"apisec-redirect-credential-forward","holdout":false} +{"id":"apisec-redirect-credential-forward","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["ssrf-prevention"],"reasonPattern":"cross.?origin|different (origin|host)|same.?origin|redirect|authorization|credential|bearer|token|strip","repo":{"base":{"api/connectors/credentials.ts":"import { store } from './store';\n\nexport type OutboundHeaders = Record;\n\nexport async function connectorHeaders(connectorId: string): Promise {\n const conn = await store.connector(connectorId);\n const headers: OutboundHeaders = { ...(conn.staticHeaders ?? {}) };\n if (conn.token) headers.Authorization = `Bearer ${conn.token}`;\n return headers;\n}\n","api/connectors/outbound.ts":"import { connectorHeaders, OutboundHeaders } from './credentials';\n\nexport type OutboundOptions = { connectorId: string; method?: string };\n\nexport async function callConnector(target: string, opts: OutboundOptions) {\n const headers: OutboundHeaders = await connectorHeaders(opts.connectorId);\n const init = { method: opts.method ?? 'GET', headers, redirect: 'manual' as const };\n const res = await fetch(target, init);\n if (res.status >= 300 && res.status <= 399) throw new Error('unexpected hop');\n return res;\n}\n"},"staged":{"api/connectors/credentials.ts":"import { store } from './store';\n\nexport type OutboundHeaders = Record;\n\nexport async function connectorHeaders(connectorId: string): Promise {\n const conn = await store.connector(connectorId);\n const headers: OutboundHeaders = { ...(conn.staticHeaders ?? {}) };\n if (conn.token) headers.Authorization = `Bearer ${conn.token}`;\n return headers;\n}\n","api/connectors/outbound.ts":"import { connectorHeaders, OutboundHeaders } from './credentials';\n\nexport type OutboundOptions = { connectorId: string; method?: string; maxHops?: number };\n\nexport async function callConnector(target: string, opts: OutboundOptions) {\n const headers: OutboundHeaders = await connectorHeaders(opts.connectorId);\n const init = { method: opts.method ?? 'GET', headers, redirect: 'manual' as const };\n let url = target;\n for (let hop = 0; hop < (opts.maxHops ?? 4); hop++) {\n const res = await fetch(url, init);\n if (res.status < 300 || res.status > 399) return res;\n const location = res.headers.get('location');\n if (!location) return res;\n url = new URL(location, url).toString();\n }\n throw new Error('too many hops');\n}\n"}},"note":"The staged change turns a single-hop outbound call into a redirect-following loop, but reuses one `init` object — including the connector's Authorization: Bearer header and its stored static headers — for every hop. `new URL(location, url)` accepts an absolute Location on any host, so a 302 from the connector's own endpoint (or a connector URL an attacker can register/steer) hands the stored connector credentials to a server the operator never configured. The destination host is never compared against the hop that produced it, and nothing removes the credential headers before the next fetch. This is the redirect leg of GHSA-mqhr-6j6h-74p5: the upstream fix rejects (or strips on) a redirect whose target origin differs from the current one. Reviewers are told that redirect-following on an outbound request needs the same host scrutiny as the initial URL, so ssrf-prevention FAIL is the right call.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"Budibase/budibase","url":"https://github.com/advisories/GHSA-mqhr-6j6h-74p5"},"caseId":"apisec-redirect-credential-forward","holdout":false} +{"id":"apisec-sso-email-link-unverified-pair","reviewer":"api-security-reviewer","expected":"PASS","repo":{"base":{"api/sso-callback.ts":"import { findBySubject, createFromSso, issueSession } from './accounts';\n\nexport type IdTokenClaims = {\n sub: string;\n email?: string;\n email_verified?: boolean;\n};\n\nexport async function oidcCallback(claims: IdTokenClaims) {\n let account = await findBySubject(claims.sub);\n if (!account) {\n account = await createFromSso({ subject: claims.sub, email: claims.email });\n }\n return issueSession(account.id, account.roles);\n}\n"},"staged":{"api/sso-callback.ts":"import {\n findBySubject,\n findByEmail,\n oauthLink,\n createFromSso,\n issueSession,\n} from './accounts';\n\nexport type IdTokenClaims = {\n sub: string;\n email?: string;\n email_verified?: boolean;\n};\n\nexport async function oidcCallback(claims: IdTokenClaims) {\n let account = await findBySubject(claims.sub);\n if (!account && claims.email && claims.email_verified === true) {\n account = await findByEmail(claims.email);\n if (account) {\n await oauthLink(account.id, claims.sub);\n }\n }\n if (!account) {\n account = await createFromSso({ subject: claims.sub, email: claims.email });\n }\n return issueSession(account.id, account.roles);\n}\n"}},"note":"Same email-fallback change as the gold twin, but the by-email account link only runs when the ID token carries email_verified === true (strict equality, so an absent flag counts as not confirmed). An attacker who registers the victim's address at an IdP that has not confirmed it can no longer bind a fresh subject to the victim's account — the login falls through to createFromSso and lands in a brand-new account with no inherited roles. This is exactly the guard the GHSA-hp6v-6jw7-gv2f fix commit added in sso.ts (gate the getGlobalUserByEmail fallback on details.emailVerified). PASS is correct.","difficulty":"borderline","provenance":"adapted","source":{"repo":"Budibase/budibase","url":"https://github.com/advisories/GHSA-hp6v-6jw7-gv2f"},"caseId":"apisec-sso-email-link-unverified","variantOf":"apisec-sso-email-link-unverified","holdout":false} +{"id":"apisec-sso-email-link-unverified","reviewer":"api-security-reviewer","expected":"FAIL","expectItems":["oauth-security"],"reasonPattern":"email_?verified|unverified|verif|takeover|impersonat|link","repo":{"base":{"api/sso-callback.ts":"import { findBySubject, createFromSso, issueSession } from './accounts';\n\nexport type IdTokenClaims = {\n sub: string;\n email?: string;\n email_verified?: boolean;\n};\n\nexport async function oidcCallback(claims: IdTokenClaims) {\n let account = await findBySubject(claims.sub);\n if (!account) {\n account = await createFromSso({ subject: claims.sub, email: claims.email });\n }\n return issueSession(account.id, account.roles);\n}\n"},"staged":{"api/sso-callback.ts":"import {\n findBySubject,\n findByEmail,\n oauthLink,\n createFromSso,\n issueSession,\n} from './accounts';\n\nexport type IdTokenClaims = {\n sub: string;\n email?: string;\n email_verified?: boolean;\n};\n\nexport async function oidcCallback(claims: IdTokenClaims) {\n let account = await findBySubject(claims.sub);\n if (!account && claims.email) {\n account = await findByEmail(claims.email);\n if (account) {\n await oauthLink(account.id, claims.sub);\n }\n }\n if (!account) {\n account = await createFromSso({ subject: claims.sub, email: claims.email });\n }\n return issueSession(account.id, account.roles);\n}\n"}},"note":"The staged change adds an email fallback to the OIDC callback: when the IdP subject has no local match, the handler loads the existing account by the ID token's email claim and attaches the attacker's fresh subject to it — while never reading email_verified, even though the claim is declared right there in the claims type. Any configured IdP that lets a user register an address it has not confirmed can then emit a token with email = victim, and the attacker's subject gets permanently bound to the victim's account and inherits its roles on issueSession. Per OIDC Core §5.7 the email claim must not be used as an account-linking key unless email_verified is true. This is the mechanism of GHSA-hp6v-6jw7-gv2f (Budibase OIDC SSO account takeover: sub miss silently fell back to getGlobalUserByEmail with no email_verified check). FAIL on oauth-security is correct.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"Budibase/budibase","url":"https://github.com/advisories/GHSA-hp6v-6jw7-gv2f"},"caseId":"apisec-sso-email-link-unverified","holdout":true} diff --git a/gate-engine/review/eval/reviewers/cases-correctness.jsonl b/gate-engine/review/eval/reviewers/cases-correctness.jsonl index 6a757578..6a0a3da4 100644 --- a/gate-engine/review/eval/reviewers/cases-correctness.jsonl +++ b/gate-engine/review/eval/reviewers/cases-correctness.jsonl @@ -126,3 +126,15 @@ {"id": "corr-tel-shared-merge-key-unfiltered-writer", "reviewer": "correctness-reviewer", "expected": "FAIL", "expectItems": ["writer-reader-contracts"], "reasonPattern": "disposition|filter|waived|both loops|same key|collid|overwrit|clobber|merge key|blocking", "repo": {"base": {"api/events/export.ts": "export interface LensEvent {\n runId: string;\n check: string;\n disposition: 'blocking' | 'waived';\n}\n\nconst keyOf = (e: LensEvent) => `${e.runId}/${e.check}`;\n\n// Exports one record per keyed event for the downstream store (last write per key wins).\nexport function exportRecords(events: LensEvent[]): Map {\n const out = new Map();\n for (const e of events) out.set(keyOf(e), { kind: 'failure' });\n return out;\n}\n"}, "staged": {"api/events/export.ts": "export interface LensEvent {\n runId: string;\n check: string;\n disposition: 'blocking' | 'waived';\n}\n\nconst keyOf = (e: LensEvent) => `${e.runId}/${e.check}`;\n\n// Exports one record per keyed event for the downstream store (last write per key wins).\nexport function exportRecords(events: LensEvent[]): Map {\n const out = new Map();\n for (const e of events) out.set(keyOf(e), { kind: 'failure' });\n for (const e of events) {\n if (e.disposition === 'waived') out.set(keyOf(e), { kind: 'waiver' });\n }\n return out;\n}\n"}}, "note": "Both writer loops share keyOf as the record key, but the first loop emits EVERY event as a failure record without checking disposition. A waived event is first written as kind:'failure' and then re-written as kind:'waiver' under the same key — and a blocking event that shares runId/check with a later waived one is silently replaced. The failure loop must select only disposition:'blocking' events.", "difficulty": "borderline", "provenance": "mined", "source": {"repo": "norvalbv/devkit", "shipId": "commit-run-24DDB139-D6F2-4D4B-A204-B28DE9919E9F", "url": "telemetry://fail-fix/commit-run-24DDB139-D6F2-4D4B-A204-B28DE9919E9F/correctness-reviewer/writer-reader-contracts"}, "caseId": "corr-tel-shared-merge-key", "holdout": true} {"id": "corr-tel-validate-after-lint-rejected-shape-pair", "reviewer": "correctness-reviewer", "expected": "PASS", "repo": {"base": {"api/entries/check.ts": "import { lintEntry } from './lint';\n\nexport interface Entry {\n id: string;\n payload: { fields: Record };\n}\n\n// Collects every problem with a submitted entry (never throws to the caller).\nexport function checkEntry(entry: Entry): string[] {\n const problems: string[] = [];\n try {\n lintEntry(entry);\n } catch (e) {\n problems.push(e instanceof Error ? e.message : String(e));\n }\n return problems;\n}\n"}, "staged": {"api/entries/check.ts": "import { lintEntry } from './lint';\n\nexport interface Entry {\n id: string;\n payload: { fields: Record };\n}\n\n// Collects every problem with a submitted entry (never throws to the caller).\nexport function checkEntry(entry: Entry): string[] {\n const problems: string[] = [];\n try {\n lintEntry(entry);\n } catch (e) {\n problems.push(e instanceof Error ? e.message : String(e));\n }\n if (problems.length === 0) {\n if (Object.values(entry.payload.fields).some((v) => v.length === 0)) {\n problems.push('empty field value');\n }\n }\n return problems;\n}\n"}}, "note": "Correct: the shape-dependent field check now runs only when lint reported no problems, so a malformed entry gets its lint problem reported and the function returns normally — the never-throws contract holds for exactly the inputs lint rejects.", "difficulty": "clear", "provenance": "adapted", "source": {"repo": "norvalbv/devkit", "shipId": "commit-run-78E9BBB0-7F51-424A-A9EA-103376C81FB8", "url": "telemetry://fail-fix/commit-run-78E9BBB0-7F51-424A-A9EA-103376C81FB8/correctness-reviewer/writer-reader-contracts"}, "caseId": "corr-tel-check-after-lint", "variantOf": "corr-tel-validate-after-lint-rejected-shape", "holdout": false} {"id": "corr-tel-validate-after-lint-rejected-shape", "reviewer": "correctness-reviewer", "expected": "FAIL", "expectItems": ["writer-reader-contracts"], "reasonPattern": "after.*(lint|reject|fail)|assumes|shape|undefined|crash|missing payload|guard|dereference", "repo": {"base": {"api/entries/check.ts": "import { lintEntry } from './lint';\n\nexport interface Entry {\n id: string;\n payload: { fields: Record };\n}\n\n// Collects every problem with a submitted entry (never throws to the caller).\nexport function checkEntry(entry: Entry): string[] {\n const problems: string[] = [];\n try {\n lintEntry(entry);\n } catch (e) {\n problems.push(e instanceof Error ? e.message : String(e));\n }\n return problems;\n}\n"}, "staged": {"api/entries/check.ts": "import { lintEntry } from './lint';\n\nexport interface Entry {\n id: string;\n payload: { fields: Record };\n}\n\n// Collects every problem with a submitted entry (never throws to the caller).\nexport function checkEntry(entry: Entry): string[] {\n const problems: string[] = [];\n try {\n lintEntry(entry);\n } catch (e) {\n problems.push(e instanceof Error ? e.message : String(e));\n }\n if (Object.values(entry.payload.fields).some((v) => v.length === 0)) {\n problems.push('empty field value');\n }\n return problems;\n}\n"}}, "note": "The exact entries lintEntry rejects (missing/malformed payload) are the ones that reach the next check anyway: checkEntry catches the lint error into problems but then unconditionally dereferences entry.payload.fields. For a malformed entry that path throws to the caller — breaking checkEntry's own never-throws, collect-all-problems contract precisely on the inputs it exists to report.", "difficulty": "clear", "provenance": "mined", "source": {"repo": "norvalbv/devkit", "shipId": "commit-run-78E9BBB0-7F51-424A-A9EA-103376C81FB8", "url": "telemetry://fail-fix/commit-run-78E9BBB0-7F51-424A-A9EA-103376C81FB8/correctness-reviewer/writer-reader-contracts"}, "caseId": "corr-tel-check-after-lint", "holdout": false} +{"id":"corr-tel-banner-ratio-masks-logic-line-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/dup-report/noise-filter.ts":"export interface Match {\n fragment: string;\n files: [string, string];\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => m.files[0] !== m.files[1]);\n}\n"},"staged":{"src/dup-report/noise-filter.ts":"export interface Match {\n fragment: string;\n files: [string, string];\n}\n\n// Scaffolded banner comments are stamped into each module by the generator,\n// so two modules carrying an identical banner is expected output.\nconst BANNER_LINE_RE = /^(?:\\/\\/|\\/\\*|\\*\\/|\\* )/;\n\nexport function isBannerOnly(fragment: string): boolean {\n const lines = fragment.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n return lines.every((l) => BANNER_LINE_RE.test(l));\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches\n .filter((m) => m.files[0] !== m.files[1])\n .filter((m) => !isBannerOnly(m.fragment));\n}\n"}},"note":"Correct: isBannerOnly requires that EVERY trimmed line match the banner shape, so a fragment carrying even a single line of duplicated code fails the classification and reports normally. Only fragments that are banner comments end to end are filtered, which is exactly the generator-stamped noise the filter is meant to remove. No mixed fragment can be classified away.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-07EC0ADC-27E7-49F7-A942-66E1BAD9B9C1","url":"telemetry://fail-fix/commit-run-07EC0ADC-27E7-49F7-A942-66E1BAD9B9C1/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-banner-ratio","variantOf":"corr-tel-banner-ratio-masks-logic-line","holdout":false} +{"id":"corr-tel-banner-ratio-masks-logic-line","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"ratio|threshold|0\\.8|80\\s?%|misclassif|suppress|swallow|discard|mixed|real\\s+(logic|code)|logic-bearing|one\\s+line|every\\s+line|all\\s+lines","repo":{"base":{"src/dup-report/noise-filter.ts":"export interface Match {\n fragment: string;\n files: [string, string];\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => m.files[0] !== m.files[1]);\n}\n"},"staged":{"src/dup-report/noise-filter.ts":"export interface Match {\n fragment: string;\n files: [string, string];\n}\n\n// Scaffolded banner comments are stamped into each module by the generator,\n// so two modules carrying an identical banner is expected output.\nconst BANNER_LINE_RE = /^(?:\\/\\/|\\/\\*|\\*\\/|\\* )/;\nconst BANNER_SHARE_MIN = 0.8;\n\nexport function isBannerOnly(fragment: string): boolean {\n const lines = fragment.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n const bannerish = lines.filter((l) => BANNER_LINE_RE.test(l)).length;\n return bannerish / lines.length >= BANNER_SHARE_MIN;\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches\n .filter((m) => m.files[0] !== m.files[1])\n .filter((m) => !isBannerOnly(m.fragment));\n}\n"}},"note":"The added noise filter classifies a fragment as banner-only when >=80% of its trimmed lines are banner-shaped (BANNER_SHARE_MIN = 0.8). A 7-line fragment made of 6 banner lines plus 1 duplicated statement scores 6/7 ≈ 0.857 and is filtered out of the report — yet that single statement is the actual copied logic the report exists to surface. The ratio admits mixed fragments whose non-banner remainder is precisely the signal, so the classifier silently drops true positives at the edge.","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-07EC0ADC-27E7-49F7-A942-66E1BAD9B9C1","url":"telemetry://fail-fix/commit-run-07EC0ADC-27E7-49F7-A942-66E1BAD9B9C1/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-banner-ratio","holdout":true} +{"id":"corr-tel-banner-stray-line-excusal-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/report/banner-filter.ts":"export type Match = { text: string; file: string };\n\nconst CODE_FILE_RE = /\\.(?:ts|js|mjs)$/;\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => CODE_FILE_RE.test(m.file));\n}\n"},"staged":{"src/report/banner-filter.ts":"export type Match = { text: string; file: string };\n\nconst CODE_FILE_RE = /\\.(?:ts|js|mjs)$/;\nconst BANNER_LINE_RE = /^(?:\\/\\/|#|\\*+)\\s*(?:@generated|do not edit|source:)/i;\n\n// True when a matched block is nothing but a generated-file banner.\nexport function isBannerBlock(text: string): boolean {\n const lines = text.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n let stray = 0;\n let strayAt = -1;\n for (let i = 0; i < lines.length; i += 1) {\n if (BANNER_LINE_RE.test(lines[i])) continue;\n stray += 1;\n strayAt = i;\n }\n if (stray === 0) return true;\n if (stray !== 1 || strayAt !== lines.length - 1 || lines.length - 1 < 3) return false;\n // The sampler can clip a block mid-statement; tolerate a trailing bare token.\n return /^[\\w$]+$/.test(lines[strayAt]);\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => CODE_FILE_RE.test(m.file) && !isBannerBlock(m.text));\n}\n"}},"note":"Correct version of the same change: the escape hatch now requires (a) at least three lines that actually matched the banner regex, (b) the single unmatched line to be the final line of the block, and (c) that line to be a lone bare identifier token with no statement terminator — i.e. a genuine clip artifact. A one-line pure-logic match hits the lines.length - 1 < 3 guard and is reported; a complete duplicated statement fails the bare-token test and is reported. Only true banner blocks (optionally with a clipped trailing token) are filtered.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-328D1C0C-8D5C-4E88-B454-1EB84A6FB6AE","url":"telemetry://fail-fix/commit-run-328D1C0C-8D5C-4E88-B454-1EB84A6FB6AE/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-banner-stray-excusal","variantOf":"corr-tel-banner-stray-line-excusal","holdout":false} +{"id":"corr-tel-banner-stray-line-excusal","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"excus|misclassif|suppress|one[- ]line|single[- ]line|pure logic|exactly one|zero|non[- ]banner","repo":{"base":{"src/report/banner-filter.ts":"export type Match = { text: string; file: string };\n\nconst CODE_FILE_RE = /\\.(?:ts|js|mjs)$/;\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => CODE_FILE_RE.test(m.file));\n}\n"},"staged":{"src/report/banner-filter.ts":"export type Match = { text: string; file: string };\n\nconst CODE_FILE_RE = /\\.(?:ts|js|mjs)$/;\nconst BANNER_LINE_RE = /^(?:\\/\\/|#|\\*+)\\s*(?:@generated|do not edit|source:)/i;\n\n// True when a matched block is nothing but a generated-file banner.\nexport function isBannerBlock(text: string): boolean {\n const lines = text.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n let stray = 0;\n for (const line of lines) {\n if (!BANNER_LINE_RE.test(line)) stray += 1;\n }\n if (stray === 0) return true;\n // The sampler can clip a block mid-statement; tolerate a ragged trailing line.\n return stray === 1;\n}\n\nexport function reportable(matches: Match[]): Match[] {\n return matches.filter((m) => CODE_FILE_RE.test(m.file) && !isBannerBlock(m.text));\n}\n"}},"note":"isBannerBlock's escape hatch fires whenever exactly one line failed the banner regex, with no requirement that any banner line was actually seen, that the unmatched line is the final one, or that it looks like a clipped fragment. A one-line match of pure logic (zero banner content, e.g. `const total = items.reduce((n, x) => n + x.amount, 0);`) has stray === 1 and is classified as a banner block, so reportable() silently drops a real duplicated-logic finding; the same happens to any block whose single non-banner line is a complete duplicated statement anywhere in the block. The classifier's whole purpose is 'nothing but a banner', and this path returns true for blocks that are 100% non-banner.","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-328D1C0C-8D5C-4E88-B454-1EB84A6FB6AE","url":"telemetry://fail-fix/commit-run-328D1C0C-8D5C-4E88-B454-1EB84A6FB6AE/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-banner-stray-excusal","holdout":false} +{"id":"corr-tel-loose-closer-absorbs-statement-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/manifest/report.ts":"export interface Fragment {\n text: string;\n files: string[];\n}\n\n// Duplicated fragments seen in more than one file get reported.\nexport function reportable(fragments: Fragment[]): Fragment[] {\n return fragments.filter((f) => f.files.length > 1);\n}\n"},"staged":{"src/manifest/report.ts":"import { isHeaderOnly } from './header-only';\n\nexport interface Fragment {\n text: string;\n files: string[];\n}\n\n// Duplicated fragments seen in more than one file get reported.\nexport function reportable(fragments: Fragment[]): Fragment[] {\n return fragments\n .filter((f) => f.files.length > 1)\n .filter((f) => !isHeaderOnly(f.text));\n}\n","src/manifest/header-only.ts":"const OPEN_RE = /^load\\b/;\nconst ITEM_RE = /^[A-Za-z_]\\w*,?$/;\nconst CLOSER_RE = /^\\)\\s*from\\s+\"[^\"]+\";?$/;\nconst END_RE = /[\"'];?$/;\n\n// True when every line of the fragment belongs to a load declaration.\nexport function isHeaderOnly(text: string): boolean {\n const lines = text.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n let open = false;\n for (const line of lines) {\n if (open) {\n if (CLOSER_RE.test(line)) open = false;\n else if (!ITEM_RE.test(line)) return false;\n } else if (OPEN_RE.test(line)) {\n if (!END_RE.test(line)) open = true;\n } else {\n return false;\n }\n }\n return !open;\n}\n"}},"note":"Correct: the continuation state ends only on the explicit `) from \"...\"` CLOSER_RE line. A real statement following an open `load (` matches neither CLOSER_RE nor ITEM_RE, so the function returns false and the duplicate is still reported. END_RE is used only to recognize a single-line declaration at the opener, where it is safe. Under-filtering is the safe direction here; correctness stays silent.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-E981E37A-CAED-43A8-AD9F-F08D47139D50","url":"telemetry://fail-fix/commit-run-E981E37A-CAED-43A8-AD9F-F08D47139D50/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-header-loose-terminator","variantOf":"corr-tel-loose-closer-absorbs-statement","holdout":false} +{"id":"corr-tel-loose-closer-absorbs-statement","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"END_RE|ends? (in|with) a quote|quoted string|absorb|swallow|misclass|treated as|counts? .*(statement|code)|any line|arbitrary|hidden|hides|suppress","repo":{"base":{"src/manifest/report.ts":"export interface Fragment {\n text: string;\n files: string[];\n}\n\n// Duplicated fragments seen in more than one file get reported.\nexport function reportable(fragments: Fragment[]): Fragment[] {\n return fragments.filter((f) => f.files.length > 1);\n}\n"},"staged":{"src/manifest/report.ts":"import { isHeaderOnly } from './header-only';\n\nexport interface Fragment {\n text: string;\n files: string[];\n}\n\n// Duplicated fragments seen in more than one file get reported.\nexport function reportable(fragments: Fragment[]): Fragment[] {\n return fragments\n .filter((f) => f.files.length > 1)\n .filter((f) => !isHeaderOnly(f.text));\n}\n","src/manifest/header-only.ts":"const OPEN_RE = /^load\\b/;\nconst ITEM_RE = /^[A-Za-z_]\\w*,?$/;\nconst CLOSER_RE = /^\\)\\s*from\\s+\"[^\"]+\";?$/;\nconst END_RE = /[\"'];?$/;\n\n// True when every line of the fragment belongs to a load declaration.\nexport function isHeaderOnly(text: string): boolean {\n const lines = text.split('\\n').map((l) => l.trim()).filter(Boolean);\n if (lines.length === 0) return false;\n let open = false;\n for (const line of lines) {\n if (open) {\n if (CLOSER_RE.test(line) || END_RE.test(line)) open = false;\n else if (!ITEM_RE.test(line)) return false;\n } else if (OPEN_RE.test(line)) {\n if (!END_RE.test(line)) open = true;\n } else {\n return false;\n }\n }\n return !open;\n}\n"}},"note":"isHeaderOnly's continuation state accepts CLOSER_RE OR the loose END_RE (/[\"'];?$/) as the declaration terminator. END_RE matches ANY line ending in a quoted string, not just the `) from \"...\"` shape, so a fragment opening with an unterminated `load (` followed by a real duplicated statement such as `const KEY = \"x\";` is absorbed: the statement flips open=false, the walk finishes clean, and the fragment is misclassified as header-only — reportable() silently drops a genuine duplicate from the report.","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-E981E37A-CAED-43A8-AD9F-F08D47139D50","url":"telemetry://fail-fix/commit-run-E981E37A-CAED-43A8-AD9F-F08D47139D50/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-header-loose-terminator","holdout":true} +{"id":"corr-tel-person-branch-bot-reply-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/threads/label-outcomes.ts":"export interface Reply { author: string; }\n\nexport interface Thread {\n id: string;\n author: string;\n replies: Reply[];\n}\n\nconst SERVICE_ACCOUNTS = new Set(['auto-triage', 'ci-reporter']);\n\n// True when someone outside the tooling logins replied at all.\nexport function repliedByPerson(replies: Reply[]): boolean {\n return replies.some((r) => r.author && !SERVICE_ACCOUNTS.has(r.author));\n}\n\n// One outcome record per thread; downstream readers take label as-is.\nexport function labelThread(thread: Thread): { id: string; label: string } {\n const contested = repliedByPerson(thread.replies);\n return { id: thread.id, label: contested ? 'disputed' : 'accepted' };\n}\n"},"staged":{"src/threads/label-outcomes.ts":"export interface Reply { author: string; }\n\nexport interface Thread {\n id: string;\n author: string;\n raisedBy: 'person' | 'automation';\n replies: Reply[];\n}\n\nconst SERVICE_ACCOUNTS = new Set(['auto-triage', 'ci-reporter']);\n\n// True when someone outside the tooling logins replied at all.\nexport function repliedByPerson(replies: Reply[]): boolean {\n return replies.some((r) => r.author && !SERVICE_ACCOUNTS.has(r.author));\n}\n\n// One outcome record per thread; downstream readers take label as-is.\n// Threads raised by a person get a narrower signal: only a reply from\n// someone else counts (the author's own follow-ups do not).\nexport function labelThread(thread: Thread): { id: string; label: string } {\n const contested =\n thread.raisedBy === 'person'\n ? thread.replies.some(\n (r) => r.author && r.author !== thread.author && !SERVICE_ACCOUNTS.has(r.author),\n )\n : repliedByPerson(thread.replies);\n return { id: thread.id, label: contested ? 'disputed' : 'accepted' };\n}\n"}},"note":"Correct: the person-raised branch excludes SERVICE_ACCOUNTS in addition to the thread author, mirroring the automation branch's repliedByPerson filter. An automated responder's reply can never flip the label to 'disputed'; only a genuine reply from another person does, so the outcome signal downstream readers consume stays trustworthy. Correctness stays silent.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-00E1172E-A828-4F05-8940-6283D5D87748","url":"telemetry://fail-fix/commit-run-00E1172E-A828-4F05-8940-6283D5D87748/correctness-reviewer/writer-reader-contracts"},"caseId":"corr-tel-person-branch-reply","variantOf":"corr-tel-person-branch-bot-reply","holdout":false} +{"id":"corr-tel-person-branch-bot-reply","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"bot|service.?account|SERVICE_ACCOUNTS|automat|exclud|filter|disput|misclassif|pushback","repo":{"base":{"src/threads/label-outcomes.ts":"export interface Reply { author: string; }\n\nexport interface Thread {\n id: string;\n author: string;\n replies: Reply[];\n}\n\nconst SERVICE_ACCOUNTS = new Set(['auto-triage', 'ci-reporter']);\n\n// True when someone outside the tooling logins replied at all.\nexport function repliedByPerson(replies: Reply[]): boolean {\n return replies.some((r) => r.author && !SERVICE_ACCOUNTS.has(r.author));\n}\n\n// One outcome record per thread; downstream readers take label as-is.\nexport function labelThread(thread: Thread): { id: string; label: string } {\n const contested = repliedByPerson(thread.replies);\n return { id: thread.id, label: contested ? 'disputed' : 'accepted' };\n}\n"},"staged":{"src/threads/label-outcomes.ts":"export interface Reply { author: string; }\n\nexport interface Thread {\n id: string;\n author: string;\n raisedBy: 'person' | 'automation';\n replies: Reply[];\n}\n\nconst SERVICE_ACCOUNTS = new Set(['auto-triage', 'ci-reporter']);\n\n// True when someone outside the tooling logins replied at all.\nexport function repliedByPerson(replies: Reply[]): boolean {\n return replies.some((r) => r.author && !SERVICE_ACCOUNTS.has(r.author));\n}\n\n// One outcome record per thread; downstream readers take label as-is.\n// Threads raised by a person get a narrower signal: only a reply from\n// someone else counts (the author's own follow-ups do not).\nexport function labelThread(thread: Thread): { id: string; label: string } {\n const contested =\n thread.raisedBy === 'person'\n ? thread.replies.some((r) => r.author && r.author !== thread.author)\n : repliedByPerson(thread.replies);\n return { id: thread.id, label: contested ? 'disputed' : 'accepted' };\n}\n"}},"note":"The new person-raised branch computes contested as replies.some(r => r.author !== thread.author) with NO service-account exclusion, while the automation branch routes through repliedByPerson's SERVICE_ACCOUNTS filter. An automated responder (e.g. ci-reporter) replying on a person-raised thread now satisfies the predicate, and labelThread writes label:'disputed' — a corrupted outcome signal that downstream readers trust as a real person disagreeing. The person branch also needs && !SERVICE_ACCOUNTS.has(r.author).","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-00E1172E-A828-4F05-8940-6283D5D87748","url":"telemetry://fail-fix/commit-run-00E1172E-A828-4F05-8940-6283D5D87748/correctness-reviewer/writer-reader-contracts"},"caseId":"corr-tel-person-branch-reply","holdout":false} +{"id":"corr-tel-scoped-sweep-first-subentry-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/ingest/release-index.ts":"export interface Build {\n platform: string;\n version: string;\n minOs: string | null;\n}\n\nexport interface Report {\n id: string;\n builds?: Build[];\n}\n\n// True when any build in the report targets the given platform.\nexport const matchesPlatform = (report: Report, platform: string): boolean =>\n (report.builds ?? []).some((b) => b.platform === platform);\n"},"staged":{"src/ingest/release-index.ts":"export interface Build {\n platform: string;\n version: string;\n minOs: string | null;\n}\n\nexport interface Report {\n id: string;\n builds?: Build[];\n}\n\n// True when any build in the report targets the given platform.\nexport const matchesPlatform = (report: Report, platform: string): boolean =>\n (report.builds ?? []).some((b) => b.platform === platform);\n\n// Sweep a batch into index rows for one platform; only qualifying reports are kept.\nexport function indexReports(reports: Report[], platform: string) {\n const rows = [];\n for (const report of reports) {\n if (!matchesPlatform(report, platform)) continue;\n const build = report.builds?.find((b) => b.platform === platform);\n rows.push({ id: report.id, platform, version: build?.version ?? null, minOs: build?.minOs ?? null });\n }\n return rows;\n}\n"}},"note":"indexReports selects the build whose platform equals the sweep scope, and matchesPlatform already guarantees such a build exists for every kept report, so each row's version/minOs always come from the build that satisfied the filter regardless of its position in builds. The ?? null fallbacks only fire when that matched build genuinely lacks a value.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-599B655C-23FF-4DCA-9772-267C512EB8F9","url":"telemetry://fail-fix/commit-run-599B655C-23FF-4DCA-9772-267C512EB8F9/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-scoped-first-subentry","variantOf":"corr-tel-scoped-sweep-first-subentry","holdout":false} +{"id":"corr-tel-scoped-sweep-first-subentry","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"first|\\[0\\]|wrong|mismatch|different|unrelated","repo":{"base":{"src/ingest/release-index.ts":"export interface Build {\n platform: string;\n version: string;\n minOs: string | null;\n}\n\nexport interface Report {\n id: string;\n builds?: Build[];\n}\n\n// True when any build in the report targets the given platform.\nexport const matchesPlatform = (report: Report, platform: string): boolean =>\n (report.builds ?? []).some((b) => b.platform === platform);\n"},"staged":{"src/ingest/release-index.ts":"export interface Build {\n platform: string;\n version: string;\n minOs: string | null;\n}\n\nexport interface Report {\n id: string;\n builds?: Build[];\n}\n\n// True when any build in the report targets the given platform.\nexport const matchesPlatform = (report: Report, platform: string): boolean =>\n (report.builds ?? []).some((b) => b.platform === platform);\n\n// Sweep a batch into index rows for one platform; only qualifying reports are kept.\nexport function indexReports(reports: Report[], platform: string) {\n const rows = [];\n for (const report of reports) {\n if (!matchesPlatform(report, platform)) continue;\n const build = report.builds?.[0];\n rows.push({ id: report.id, platform, version: build?.version ?? null, minOs: build?.minOs ?? null });\n }\n return rows;\n}\n"}},"note":"matchesPlatform only guarantees the target platform appears SOMEWHERE in report.builds, but indexReports then reads builds[0], whichever platform that happens to be. A report whose builds list is [android, ios] swept for 'ios' passes the guard yet stores android's version/minOs on a row labeled platform:'ios' — every multi-platform report whose matching build is not in position 0 gets index fields describing another platform's build. The fix is to select the build whose platform equals the sweep scope.","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-599B655C-23FF-4DCA-9772-267C512EB8F9","url":"telemetry://fail-fix/commit-run-599B655C-23FF-4DCA-9772-267C512EB8F9/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-scoped-first-subentry","holdout":true} +{"id":"corr-tel-suffix-decl-enters-list-state-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/recipes/clone-report.ts":"import { scanRecipes } from './scan';\n\nexport interface CloneHit {\n fragment: string;\n files: [string, string];\n}\n\n// Reports fragments repeated verbatim across two recipe files.\nexport function findRecipeClones(dir: string): CloneHit[] {\n return scanRecipes(dir).filter((hit) => hit.files[0] !== hit.files[1]);\n}\n"},"staged":{"src/recipes/clone-report.ts":"import { scanRecipes } from './scan';\nimport { isLoadHeader } from './load-header';\n\nexport interface CloneHit {\n fragment: string;\n files: [string, string];\n}\n\n// Reports fragments repeated verbatim across two recipe files.\nexport function findRecipeClones(dir: string): CloneHit[] {\n return scanRecipes(dir)\n .filter((hit) => hit.files[0] !== hit.files[1])\n .filter((hit) => !isLoadHeader(hit.fragment));\n}\n","src/recipes/load-header.ts":"// Grammar: a directive is either single-line — load \"x\"; or load \"x\" as y; —\n// or a list: load ( opener, one \"item\" line each, closed by a lone ); line.\nconst OPEN_RE = /^load\\b/;\nconst DONE_RE = /(?:\"|\"\\s+as\\s+[\\w$]+);$/;\nconst LIST_CLOSE_RE = /^\\);$/;\n\n// Recipes pulling the same modules always share this block, so a repeated run of\n// them is treated as unavoidable overlap and dropped from the report upstream.\nexport function isLoadHeader(fragment: string): boolean {\n const lines = fragment\n .split('\\n')\n .map((l) => l.trim())\n .filter(Boolean);\n if (lines.length === 0) return false;\n let inList = false;\n for (const line of lines) {\n if (inList) {\n inList = !LIST_CLOSE_RE.test(line);\n } else if (!OPEN_RE.test(line)) {\n return false;\n } else if (!DONE_RE.test(line)) {\n inList = true;\n }\n }\n return true;\n}\n"}},"note":"DONE_RE now recognizes both documented single-line shapes — a bare quote terminator and the `\" as y;` renamed form — so an aliased directive is treated as finished and the list state is entered only by a genuine `load (` opener. A fragment that mixes one aliased directive with real duplicated recipe steps hits the OPEN_RE branch on the first non-directive line, returns false, and the clone stays in the report.","difficulty":"borderline","provenance":"adapted","source":{"repo":"norvalbv/devkit","shipId":"commit-run-B65C3DF2-F3A6-4DD1-A14E-B2AF1C36F65B","url":"telemetry://fail-fix/commit-run-B65C3DF2-F3A6-4DD1-A14E-B2AF1C36F65B/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-load-header-classifier","variantOf":"corr-tel-suffix-decl-enters-list-state","holdout":false} +{"id":"corr-tel-suffix-decl-enters-list-state","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"quote|complet(e|ion)|continuation|multi[ -]?line|absorb|swallow|suppress|misclassif|classified|alias|suffix|stays? (in|open)|never (close|exit)","repo":{"base":{"src/recipes/clone-report.ts":"import { scanRecipes } from './scan';\n\nexport interface CloneHit {\n fragment: string;\n files: [string, string];\n}\n\n// Reports fragments repeated verbatim across two recipe files.\nexport function findRecipeClones(dir: string): CloneHit[] {\n return scanRecipes(dir).filter((hit) => hit.files[0] !== hit.files[1]);\n}\n"},"staged":{"src/recipes/clone-report.ts":"import { scanRecipes } from './scan';\nimport { isLoadHeader } from './load-header';\n\nexport interface CloneHit {\n fragment: string;\n files: [string, string];\n}\n\n// Reports fragments repeated verbatim across two recipe files.\nexport function findRecipeClones(dir: string): CloneHit[] {\n return scanRecipes(dir)\n .filter((hit) => hit.files[0] !== hit.files[1])\n .filter((hit) => !isLoadHeader(hit.fragment));\n}\n","src/recipes/load-header.ts":"// Grammar: a directive is either single-line — load \"x\"; or load \"x\" as y; —\n// or a list: load ( opener, one \"item\" line each, closed by a lone ); line.\nconst OPEN_RE = /^load\\b/;\nconst DONE_RE = /\";$/;\nconst LIST_CLOSE_RE = /^\\);$/;\n\n// Recipes pulling the same modules always share this block, so a repeated run of\n// them is treated as unavoidable overlap and dropped from the report upstream.\nexport function isLoadHeader(fragment: string): boolean {\n const lines = fragment\n .split('\\n')\n .map((l) => l.trim())\n .filter(Boolean);\n if (lines.length === 0) return false;\n let inList = false;\n for (const line of lines) {\n if (inList) {\n inList = !LIST_CLOSE_RE.test(line);\n } else if (!OPEN_RE.test(line)) {\n return false;\n } else if (!DONE_RE.test(line)) {\n inList = true;\n }\n }\n return true;\n}\n"}},"note":"DONE_RE (/\";$/) decides a single-line load directive is finished only when it ends in a closing quote, but the grammar the file itself documents also allows `load \"x\" as y;`, which ends in the renamed token. That legal form fails DONE_RE, so the state machine flips inList=true exactly as if a `load (` list had opened; from there every following line is accepted unchecked until a bare `);` appears (or the fragment ends). A fragment holding one aliased directive followed by genuinely duplicated recipe steps therefore returns true and the filter in findRecipeClones silently drops the real clone from the report.","difficulty":"borderline","provenance":"mined","source":{"repo":"norvalbv/devkit","shipId":"commit-run-B65C3DF2-F3A6-4DD1-A14E-B2AF1C36F65B","url":"telemetry://fail-fix/commit-run-B65C3DF2-F3A6-4DD1-A14E-B2AF1C36F65B/correctness-reviewer/error-and-edge-classification"},"caseId":"corr-tel-load-header-classifier","holdout":false} diff --git a/gate-engine/review/eval/reviewers/cases-frontend-security.jsonl b/gate-engine/review/eval/reviewers/cases-frontend-security.jsonl index 2ff4bd78..19c7f3d0 100644 --- a/gate-engine/review/eval/reviewers/cases-frontend-security.jsonl +++ b/gate-engine/review/eval/reviewers/cases-frontend-security.jsonl @@ -17,3 +17,5 @@ {"reviewer": "frontend-security-reviewer", "variantOf": null, "holdout": false, "id": "fesec-auth-callback-console-token", "expected": "FAIL", "expectItems": ["debug-logging"], "reasonPattern": "console|devtools|leak|token|credential|sensitive|redact|remove", "repo": {"base": {"web/AuthCallback.tsx": "import React, { useEffect, useState } from 'react';\n\nimport { completeSignIn } from './session';\n\nexport function AuthCallback() {\n const [status, setStatus] = useState<'working' | 'done' | 'failed'>('working');\n useEffect(() => {\n completeSignIn(window.location.search)\n .then(() => setStatus('done'))\n .catch(() => setStatus('failed'));\n }, []);\n return

{status === 'working' ? 'Signing you in\u2026' : status}

;\n}\n"}, "staged": {"web/AuthCallback.tsx": "import React, { useEffect, useState } from 'react';\n\nimport { completeSignIn } from './session';\n\nexport function AuthCallback() {\n const [status, setStatus] = useState<'working' | 'done' | 'failed'>('working');\n useEffect(() => {\n // Trace sign-in issues reported by beta users.\n console.log('auth callback url', window.location.href);\n completeSignIn(window.location.search)\n .then((session) => {\n console.log('session token', session.accessToken);\n setStatus('done');\n })\n .catch((err) => {\n console.error('sign-in failed', err);\n setStatus('failed');\n });\n }, []);\n return

{status === 'working' ? 'Signing you in\u2026' : status}

;\n}\n"}}, "note": "Debug logging left in the sign-in callback prints the full callback URL (which carries auth parameters) and the session access token to the console, where extensions and shared-screen sessions can read them. Gold for debug-logging, previously uncovered.", "difficulty": "clear", "provenance": "authored"} {"reviewer": "frontend-security-reviewer", "variantOf": null, "holdout": false, "id": "fesec-event-reporter-redaction", "expected": "PASS", "expectItems": null, "reasonPattern": null, "repo": {"base": {"web/event-reporter.ts": "export interface CapturedEvent {\n kind: string;\n detail: string;\n}\n\nexport function describeEvent(evt: CapturedEvent): string {\n return `${evt.kind}: ${evt.detail}`;\n}\n"}, "staged": {"web/event-reporter.ts": "export interface CapturedEvent {\n kind: string;\n detail: string;\n authorization?: string;\n}\n\nconst MASK = '[redacted]';\n\nfunction redact(evt: CapturedEvent): CapturedEvent {\n // Diagnostics must never carry the bearer token or any credential material.\n return { ...evt, authorization: evt.authorization ? MASK : undefined };\n}\n\nexport function describeEvent(evt: CapturedEvent): string {\n return `${evt.kind}: ${evt.detail}`;\n}\n\nexport function reportEvent(evt: CapturedEvent): void {\n const safe = redact(evt);\n console.error('captured event', safe);\n}\n"}}, "note": "Near-miss decoy: console.error plus the word authorization trips the debug-logging and token-handling triggers, but the payload is explicitly redacted before logging \u2014 the credential field is masked and never printed. A judge pattern-matching on console+token without reading the redact helper false-blocks here.", "difficulty": "adversarial", "provenance": "authored"} {"reviewer": "frontend-security-reviewer", "variantOf": null, "holdout": true, "id": "fesec-invite-post-no-csrf", "expected": "FAIL", "expectItems": ["csrf-protection"], "reasonPattern": "csrf|cross[- ]site|same[- ]?site|anti[- ]forgery|token|header", "repo": {"base": {"web/invites.ts": "export interface InviteDraft {\n email: string;\n role: 'viewer' | 'editor';\n}\n\nexport async function loadInvites(): Promise {\n const res = await fetch('/api/invites', { credentials: 'include' });\n if (!res.ok) throw new Error('failed to load invites');\n return res.json();\n}\n"}, "staged": {"web/invites.ts": "export interface InviteDraft {\n email: string;\n role: 'viewer' | 'editor';\n}\n\nexport async function loadInvites(): Promise {\n const res = await fetch('/api/invites', { credentials: 'include' });\n if (!res.ok) throw new Error('failed to load invites');\n return res.json();\n}\n\nexport async function sendInvite(draft: InviteDraft): Promise {\n const res = await fetch('/api/invites', {\n method: 'POST',\n credentials: 'include',\n headers: { 'Content-Type': 'application/json' },\n body: JSON.stringify(draft),\n });\n if (!res.ok) throw new Error('failed to send invite');\n}\n"}}, "note": "State-changing POST authenticated purely by ambient cookies (credentials: include) with no CSRF token or custom header, in a codebase whose existing GET shows the same cookie-session pattern. Gold for csrf-protection, previously uncovered. Holdout.", "difficulty": "borderline", "provenance": "authored"} +{"id":"fesec-widget-binding-codegen-pair","reviewer":"frontend-security-reviewer","expected":"PASS","repo":{"base":{"web/src/builder/widget-codegen.ts":"type WidgetSchema = {\n label: string;\n};\n\n// Schemas are authored by workspace members in the visual builder.\nexport function renderWidgetSource(schema: WidgetSchema): string {\n const label = JSON.stringify(schema.label);\n return `export const DisplayWidget = () => {${label}};`;\n}\n"},"staged":{"web/src/builder/widget-codegen.ts":"type WidgetBinding = { property: string; field?: string };\n\ntype WidgetSchema = {\n label: string;\n binding?: WidgetBinding;\n};\n\nconst SAFE_ACCESSOR = /^[A-Za-z_$][\\w$]*$/;\n\nfunction buildBindingExpression(binding: WidgetBinding): string {\n const property = SAFE_ACCESSOR.test(binding.property) ? binding.property : 'undefined';\n if (binding.field === undefined) return property;\n return SAFE_ACCESSOR.test(binding.field) ? `${property}?.${binding.field}` : property;\n}\n\n// Schemas are authored by workspace members in the visual builder.\nexport function renderWidgetSource(schema: WidgetSchema): string {\n const label = JSON.stringify(schema.label);\n if (schema.binding === undefined) {\n return `export const DisplayWidget = () => {${label}};`;\n }\n const accessor = buildBindingExpression(schema.binding);\n return `export const DisplayWidget = (props) => ;`;\n}\n"}},"note":"Same binding feature, with the advisory's fix class applied: each accessor segment is allowlisted to a bare identifier before it is spliced into the emitted source, and any non-conforming segment degrades to a harmless props.undefined accessor. Schema text can select which prop the generated widget reads but can no longer change the shape of the emitted code, so the expression-binding path carries no injection. Mirrors GHSA-hf3j-86p7-mfw8's remediation (validate/filter component schema properties before converting them to expressions).","difficulty":"borderline","provenance":"adapted","source":{"repo":"aws-amplify/amplify-codegen-ui","url":"https://github.com/advisories/GHSA-hf3j-86p7-mfw8"},"caseId":"fesec-widget-binding-codegen","variantOf":"fesec-widget-binding-codegen","holdout":true} +{"id":"fesec-widget-binding-codegen","reviewer":"frontend-security-reviewer","expected":"FAIL","expectItems":["input-validation"],"reasonPattern":"inject|unvalidated|not validated|no validation|without validat|sanitiz|arbitrary|untrusted|escap","repo":{"base":{"web/src/builder/widget-codegen.ts":"type WidgetSchema = {\n label: string;\n};\n\n// Schemas are authored by workspace members in the visual builder.\nexport function renderWidgetSource(schema: WidgetSchema): string {\n const label = JSON.stringify(schema.label);\n return `export const DisplayWidget = () => {${label}};`;\n}\n"},"staged":{"web/src/builder/widget-codegen.ts":"type WidgetBinding = { property: string; field?: string };\n\ntype WidgetSchema = {\n label: string;\n binding?: WidgetBinding;\n};\n\nfunction buildBindingExpression(binding: WidgetBinding): string {\n return binding.field === undefined ? binding.property : `${binding.property}?.${binding.field}`;\n}\n\n// Schemas are authored by workspace members in the visual builder.\nexport function renderWidgetSource(schema: WidgetSchema): string {\n const label = JSON.stringify(schema.label);\n if (schema.binding === undefined) {\n return `export const DisplayWidget = () => {${label}};`;\n }\n const accessor = buildBindingExpression(schema.binding);\n return `export const DisplayWidget = (props) => ;`;\n}\n"}},"note":"buildBindingExpression splices the schema-supplied binding.property and binding.field verbatim into the emitted component source, inside value={props.}. Schemas are authored by workspace members (authenticated users of the builder), so a property of constructor.constructor('alert(document.domain)')() becomes executable code in every app that renders the generated widget — code injection through the expression-binding path with no check on the property text. This is the GHSA-hf3j-86p7-mfw8 / CVE-2025-4318 class (Amplify Studio codegen converted component schema properties to expressions without validating them; the fix filters the property before building the expression). Borderline because there is no eval in the diff — the reviewer must recognize source emission as an execution sink.","difficulty":"borderline","provenance":"known-answer","source":{"repo":"aws-amplify/amplify-codegen-ui","url":"https://github.com/advisories/GHSA-hf3j-86p7-mfw8"},"caseId":"fesec-widget-binding-codegen","holdout":true} diff --git a/gate-engine/review/eval/reviewers/propose/weekly-mining.sh b/gate-engine/review/eval/reviewers/propose/weekly-mining.sh index 5f7f2865..123655ca 100755 --- a/gate-engine/review/eval/reviewers/propose/weekly-mining.sh +++ b/gate-engine/review/eval/reviewers/propose/weekly-mining.sh @@ -49,6 +49,7 @@ if run_stage mine-telemetry 8 bun mine-telemetry.mts; then else echo "--- propose-telemetry skipped (mine-telemetry failed) ---" >>"$LOG" fi +run_stage mine-ghsa 4 bun mine-ghsa.mts run_stage propose-bots 4 bun propose/propose.mts --suite correctness --max 10 echo "" >>"$LOG" From 79ffcbb1863225c48d13488cba4645b08510a995 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 3 Aug 2026 16:47:49 +0100 Subject: [PATCH 3/3] fix(bench): fail closed when the promoted corpus cannot be read Review follow-up. collectCorpusUrls swallowed every read failure and returned whatever it had managed to collect. makeHardDrop then treated that set as the authoritative record of what has already landed -- its own docstring says "a landed source.url must never be re-queued" -- so an EACCES on the corpus dir, an unreadable cases-*.jsonl, or a single corrupt row silently degraded dedup and re-offered promoted candidates. The re-offer is indistinguishable from a genuine new find, so nothing downstream could catch it. Only ENOENT is a real empty now: a corpus that does not exist yet has promoted nothing. Every other failure throws rather than hand back a partial answer the caller cannot tell apart from a complete one. This matches the module's own precedent -- collectRepoArgs already fails loudly on an incomplete --repo pair for the same reason. Malformed rows throw too, which the review did not name. A corpus line that does not parse is a URL we cannot see, i.e. the identical silent under-count as an unreadable file; fixing only the read paths would have left an equally wide hole open. This is NOT the tolerated skip readCandidatesFile allows -- that input is an unlanded merge pool where a bad line costs a candidate, not a dedup guarantee. Verified every promoted row currently parses (0 malformed across 237 rows in 5 corpus files), so the stricter read is a no-op on the corpus as it stands: 75 urls collected before and after. Docs: the pending-work list still claimed api-security had 30 rows (it has 44), and still listed the known-answer import as unbuilt -- mine-ghsa/propose-ghsa ship in this batch and run weekly, and the first 8 golds landed here. Marked done under the same convention item 7 uses. New __tests__/mine-common.test.mts (6 cases): collection across corpus files, ENOENT dir returns empty, and each not-knowing path throws -- malformed row, unreadable file, unlistable dir. The two permission cases skip themselves when chmod cannot deny reads rather than assert a false pass. Co-Authored-By: Claude Opus 5 (1M context) --- docs/benchmarks/corpus-growth.md | 11 +- .../reviewers/__tests__/mine-common.test.mts | 108 ++++++++++++++++++ .../review/eval/reviewers/mine-common.mts | 41 +++++-- 3 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 gate-engine/review/eval/reviewers/__tests__/mine-common.test.mts diff --git a/docs/benchmarks/corpus-growth.md b/docs/benchmarks/corpus-growth.md index fb3b854d..afa4f0d5 100644 --- a/docs/benchmarks/corpus-growth.md +++ b/docs/benchmarks/corpus-growth.md @@ -169,11 +169,12 @@ runner); freshness goes stale exactly when one moves. was uninformative at 2 raters without model `pred_probs`; it needs per-row predicted probabilities from a bench run, not another labeler. 3. Calibration slice (localized vs full-context delta). -4. Domain-suite cascade re-bench (needs opus) now that api-security has 30 rows. -5. Known-answer import for absolute recall (security suites especially): ratify and build one of - the replacements in measurement-rule 2 (GHSA/npm advisory mining, or the CR-Bench recipe over - SWE-Bench Multimodal) — the original c-CRAB / CR-Bench TS/JS import was scoped 2026-08-02 and - is not viable. +4. Domain-suite cascade re-bench (needs opus) now that api-security has 44 rows. +5. ~~Known-answer import for absolute recall~~ — DONE 2026-08-03 (sc-1408): GHSA/npm advisory + mining was the ratified replacement in measurement-rule 2 (the original c-CRAB / CR-Bench + TS/JS import was scoped 2026-08-02 and is not viable). `mine-ghsa.mts` + `propose-ghsa.mts` + run weekly alongside the other miners; the first 8 golds (7 api-security, 1 frontend-security) + landed in this batch. Growing the slice is ordinary corpus work from here, not a build. 6. Mine the override-valve history + human review comments (decoy/gold sources already banked). 7. ~~Weekly scheduled routine~~ — DONE 2026-08-03 (sc-1415): `propose/weekly-mining.sh` runs both miners + both propose stages weekly via the owner's local crontab (Mon 09:00), diff --git a/gate-engine/review/eval/reviewers/__tests__/mine-common.test.mts b/gate-engine/review/eval/reviewers/__tests__/mine-common.test.mts new file mode 100644 index 00000000..f003886c --- /dev/null +++ b/gate-engine/review/eval/reviewers/__tests__/mine-common.test.mts @@ -0,0 +1,108 @@ +/** + * collectCorpusUrls fails CLOSED. makeHardDrop treats the returned set as the authoritative record + * of what has already been promoted ('a landed source.url must never be re-queued'), and a set that + * is silently short is indistinguishable from a complete one — the re-offered candidate looks like + * a genuine new find. So every way of not-knowing must throw; only a corpus that does not exist yet + * is a legitimate empty. + */ +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { collectCorpusUrls } from '../mine-common.mts'; + +const roots: string[] = []; +const mkTmp = () => { + const d = mkdtempSync(join(tmpdir(), 'devkit-corpus-')); + roots.push(d); + return d; +}; + +const row = (url: string) => `${JSON.stringify({ id: 'x', source: { url } })}\n`; + +afterEach(() => { + for (const d of roots.splice(0)) { + try { + chmodSync(d, 0o755); + } catch { + /* already readable */ + } + rmSync(d, { recursive: true, force: true }); + } +}); + +/** chmod is a no-op for root; skip the permission cases rather than assert a false pass. */ +const canDenyReads = () => { + const probe = mkTmp(); + const f = join(probe, 'cases-probe.jsonl'); + writeFileSync(f, row('u')); + chmodSync(f, 0o000); + try { + collectCorpusUrls(probe); + return false; // still readable → running privileged + } catch { + return true; + } +}; + +describe('collectCorpusUrls', () => { + it('collects source.url from every cases-*.jsonl and ignores other files', () => { + const dir = mkTmp(); + writeFileSync(join(dir, 'cases-api-security.jsonl'), row('https://a/1') + row('https://a/2')); + writeFileSync(join(dir, 'cases-correctness.jsonl'), row('https://c/1')); + writeFileSync(join(dir, 'candidates.jsonl'), row('https://not-promoted/1')); + writeFileSync(join(dir, 'notes.md'), 'not a corpus file'); + + expect([...collectCorpusUrls(dir)].sort()).toEqual([ + 'https://a/1', + 'https://a/2', + 'https://c/1', + ]); + }); + + it('returns an empty set when the corpus dir does not exist (ENOENT is a real empty)', () => { + expect(collectCorpusUrls(join(mkTmp(), 'nope')).size).toBe(0); + }); + + it('tolerates blank lines and rows carrying no source.url', () => { + const dir = mkTmp(); + writeFileSync( + join(dir, 'cases-x.jsonl'), + `${row('https://a/1')}\n${JSON.stringify({ id: 'no-source' })}\n`, + ); + expect([...collectCorpusUrls(dir)]).toEqual(['https://a/1']); + }); + + it('THROWS on a malformed corpus row rather than under-counting it', () => { + const dir = mkTmp(); + writeFileSync(join(dir, 'cases-x.jsonl'), `${row('https://a/1')}{not json\n`); + expect(() => collectCorpusUrls(dir)).toThrow(/malformed JSON in promoted corpus .*line 2/); + }); + + it.skipIf(!canDenyReads())( + 'THROWS when a corpus file cannot be read (EACCES, not ENOENT)', + () => { + const dir = mkTmp(); + const f = join(dir, 'cases-x.jsonl'); + writeFileSync(f, row('https://a/1')); + chmodSync(f, 0o000); + expect(() => collectCorpusUrls(dir)).toThrow(/cannot read promoted corpus/); + }, + ); + + it.skipIf(!canDenyReads())( + 'THROWS when the corpus dir cannot be listed (EACCES, not ENOENT)', + () => { + const parent = mkTmp(); + const dir = join(parent, 'reviewers'); + mkdirSync(dir); + writeFileSync(join(dir, 'cases-x.jsonl'), row('https://a/1')); + chmodSync(dir, 0o000); + try { + expect(() => collectCorpusUrls(dir)).toThrow(/cannot list corpus dir/); + } finally { + chmodSync(dir, 0o755); // an unreadable nested dir would defeat the afterEach rmSync + } + }, + ); +}); diff --git a/gate-engine/review/eval/reviewers/mine-common.mts b/gate-engine/review/eval/reviewers/mine-common.mts index e58dcf28..7d9ca227 100644 --- a/gate-engine/review/eval/reviewers/mine-common.mts +++ b/gate-engine/review/eval/reviewers/mine-common.mts @@ -14,33 +14,50 @@ import path from 'node:path'; const CORPUS_CASES_FILE_RE = /^cases-.*\.jsonl$/; /** Collect every source.url already promoted into a cases-*.jsonl corpus file in `dir` — - * the dedup key that keeps miners and proposers from re-offering landed candidates. */ + * the dedup key that keeps miners and proposers from re-offering landed candidates. + * + * This set is treated as AUTHORITATIVE by makeHardDrop ('a landed source.url must never be + * re-queued'), so it fails closed: a set that is silently short re-offers work already promoted, + * and the re-offer looks identical to a genuine new candidate. Only ENOENT is a real empty — + * a corpus that does not exist yet has promoted nothing. Every other failure (EACCES, EIO, a + * corpus file that reads but does not parse) means we cannot know what has landed, so it throws + * rather than hand back a partial answer the caller cannot tell apart from a complete one. */ export function collectCorpusUrls(dir) { const urls = new Set(); let entries = []; try { entries = readdirSync(dir); - } catch { - return urls; + } catch (e) { + if (e.code === 'ENOENT') return urls; + throw new Error(`mine-common: cannot list corpus dir ${dir} (${e.code ?? e.message}) — \ +refusing to dedup against an unknown corpus`); } for (const name of entries) { if (!CORPUS_CASES_FILE_RE.test(name)) continue; + const file = path.join(dir, name); let content = ''; try { - content = readFileSync(path.join(dir, name), 'utf8'); + content = readFileSync(file, 'utf8'); } catch (e) { - console.error(`mine-common: corpus read failed for ${name} (${e.message?.split('\n')[0]})`); - continue; + if (e.code === 'ENOENT') continue; // vanished between readdir and read + throw new Error(`mine-common: cannot read promoted corpus ${name} (${e.code ?? e.message}) — \ +refusing to dedup against an unknown corpus`); } - for (const line of content.split('\n')) { - if (!line.trim()) continue; + const lines = content.split('\n'); + for (let i = 0; i < lines.length; i += 1) { + if (!lines[i].trim()) continue; + let row = null; try { - const row = JSON.parse(line); - const u = row?.source?.url; - if (u) urls.add(u); + row = JSON.parse(lines[i]); } catch { - // skip malformed line + // A corpus row that does not parse is a URL we cannot see, i.e. the same silent + // under-count as an unreadable file — never the tolerated skip readCandidatesFile allows + // for an unlanded merge input. + throw new Error(`mine-common: malformed JSON in promoted corpus ${name} line ${i + 1} — \ +refusing to dedup against an unknown corpus`); } + const u = row?.source?.url; + if (u) urls.add(u); } } return urls;