From 1d4f4176019c80f244c1b7c0d62805413d269e55 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 13:05:02 +0000 Subject: [PATCH 1/2] feat(sarif): emit SARIF 2.1.0 + let the oracle read own-check through it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `own-check … --format sarif`, a SARIF 2.1.0 log alongside human/github/ msbuild, and teach scripts/oracle_compare.py to ingest own-check SARIF through the SAME parse_sarif it already uses for Infer#/CodeQL — retiring the bespoke own-check text parser from the oracle's critical path (the parser-drift class of bug that silently dropped findings on the ScreenToGif run). Slice 1 — emitter (ownlang/ownir.py): build_sarif(findings, severity) builds one run whose tool.driver is Own.NET (rules catalogue = the OWN codes present, titled from diagnostics.TITLES) and one result per finding (ruleId = OWN code, C# file/line as physicalLocation, message with the [resource:] tag, kind + subscription triple in properties). Wired as a fourth --format; JSON to stdout, summary to stderr, exit code unchanged. SARIF's note level carries advisory OWN050 (a coverage skip, not a warning-tier leak) — semantics the flat surfaces cannot express. Slice 2 — oracle ingestion (scripts/oracle_compare.py): build_own sniffs SARIF vs text; _oracle_class classifies tool=="own" by OWN code so the two own input formats bucket identically. Selftest pins a hand-built own-SARIF case (23/23); tests/test_ownir.py pins the real round-trip build_sarif -> parse_sarif -> leak (88/88). ruff + mypy --strict clean. Realizes the first step of the SARIF-exporter backlog item; design + follow-ups (flip oracle.yml to feed SARIF, mine_report parser->aggregator, code-scanning upload) in docs/notes/sarif-export.md. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- docs/notes/oracle.md | 10 +++-- docs/notes/sarif-export.md | 66 ++++++++++++++++++++++++++++++ ownlang/__main__.py | 44 ++++++++++++-------- ownlang/ownir.py | 84 +++++++++++++++++++++++++++++++++++++- scripts/oracle_compare.py | 55 ++++++++++++++++++++++--- tests/test_ownir.py | 71 ++++++++++++++++++++++++++++++++ 6 files changed, 304 insertions(+), 26 deletions(-) create mode 100644 docs/notes/sarif-export.md diff --git a/docs/notes/oracle.md b/docs/notes/oracle.md index 75e72c33..1c1bf547 100644 --- a/docs/notes/oracle.md +++ b/docs/notes/oracle.md @@ -81,9 +81,13 @@ python scripts/oracle_compare.py \ --target DapperLib/Dapper --commit "$SHA" --json report.json ``` -`--own` is `own-check`'s human output (same format the miner reads). The two -oracle inputs are SARIF — Infer# and CodeQL both emit it, so one parser handles -both. Extra SARIF oracles can be added with `--sarif tool=path`. +`--own` is `own-check`'s output — either its human text (the format the miner +reads) **or** a SARIF 2.1.0 log (`own-check … --format sarif`), which is read +through the *same* `parse_sarif` as the oracles below, so own-check joins the +diff with no bespoke text parser and no parser-drift (`build_own` sniffs the +format; see `docs/notes/sarif-export.md`). The two oracle inputs are SARIF — +Infer# and CodeQL both emit it, so one parser handles both. Extra SARIF oracles +can be added with `--sarif tool=path`. ## What "agree" buys us, concretely diff --git a/docs/notes/sarif-export.md b/docs/notes/sarif-export.md new file mode 100644 index 00000000..6c01fd63 --- /dev/null +++ b/docs/notes/sarif-export.md @@ -0,0 +1,66 @@ +# SARIF export — own-check as a standard analyzer + +`own-check … --format sarif` emits a **SARIF 2.1.0** log. SARIF is the OASIS +standard interchange format for static-analysis results; this is the first step +of the `SARIF exporter` backlog item from `docs/notes/research-landscape-2026.md`. + +## Why SARIF earns its place (three payoffs, internal one first) + +1. **It kills the bespoke own-check text parser in the oracle.** Until now + `scripts/oracle_compare.py` was asymmetric: Infer# and CodeQL were read with + `parse_sarif`, but *our own* findings went through a regex over human text + (`mine_report.parse`), which carries an explicit "unparsed-line" failure + bucket — the class of bug that silently dropped 38 of ~36 findings on the + ScreenToGif run (`real-world-mining.md`). With own-check emitting SARIF, the + oracle reads **all three tools through one reader**. The fragile parser stops + being on the critical path. +2. **GitHub code-scanning native.** A SARIF log uploads straight to code scanning + (inline PR annotations, the Security tab) — no bespoke `::warning` emitter + needed for that surface. +3. **Reproducibility.** A frozen, diffable run artifact: the rule set + every + result with a stable location, for benchmark/regression use. + +## What was built (two slices, one PR) + +- **Slice 1 — the emitter (`ownlang/ownir.py`).** `build_sarif(findings, + severity)` returns one SARIF `run`: `tool.driver` is **Own.NET** with a `rules` + catalogue of the OWN codes present (titles from `diagnostics.TITLES`), and one + `result` per finding — `ruleId` = the OWN code, the C# file/line as a + `physicalLocation`, the message (with its `[resource: …]` tag), and the + resource kind + subscription triple in `properties`. Wired as a fourth + `--format` alongside `human`/`github`/`msbuild`; like the other machine formats + the JSON goes to stdout and the summary to stderr, and the exit code is + unchanged. +- **Slice 2 — the oracle reads it (`scripts/oracle_compare.py`).** `build_own` + sniffs the input: a leading `{` → SARIF → `parse_sarif(text, "own", …)`; else + the legacy text path. `_oracle_class` classifies `tool == "own"` by OWN code + (`_own_class`), so the two own input formats bucket **identically** (OWN001/014 + → leak, OWN002/009 → use-after, OWN003 → double, everything else → other). The + selftest pins a hand-built own-SARIF case; `tests/test_ownir.py` pins the real + round-trip (`build_sarif` → `parse_sarif` → classed as a leak). + +## One design decision: the `note` level + +SARIF has four levels (`error`/`warning`/`note`/`none`); the flat surfaces have +only error/warning. The mapping (`_sarif_level`) keeps the CLI's per-finding +severity but uses **`note`** for advisory OWN050 ("declaring type unresolved — +analysis skipped"). That is the honest SARIF semantics — a *coverage skip* is not +a *warning-tier leak* — and it lets a consumer (incl. our own oracle) tell them +apart, which error/warning cannot. Leak verdicts still map error / warning +(intrinsic-warning, e.g. an injected-source subscription) / and downgrade under a +`--severity warning` host. + +## Follow-ups (recorded, not done here) + +- **Flip the `oracle.yml` workflow to feed SARIF.** The comparator now accepts + both formats, so this is a backward-compatible 2-line CI edit (own-check + `--format sarif` → `own.sarif`, `--own own.sarif`). Left out of this PR to + avoid changing CI blind; the capability + selftests land first. +- **`mine_report.py`: parser → aggregator-over-SARIF.** The aggregation/triage + half (counts by code/severity/kind, noisiest files, triage list) stays useful; + re-point it at SARIF input so the regex parser is retired everywhere, not just + in the oracle. +- **GitHub code-scanning upload.** Add a CI step that uploads the SARIF, and + decide whether to keep the bespoke `::warning` annotation emitter or drop it. +- **Per-rule `helpUri`.** Once a stable per-code docs anchor exists, point each + `rules[]` entry at it (intentionally omitted now rather than link a 404). diff --git a/ownlang/__main__.py b/ownlang/__main__.py index 3880001e..a7a4fca0 100644 --- a/ownlang/__main__.py +++ b/ownlang/__main__.py @@ -6,10 +6,11 @@ python -m ownlang cfg file.own # dump the control-flow graph python -m ownlang report file.own # buffer storage report + .ownreport.json python -m ownlang ownir facts.json # check OwnIR facts extracted from C# (P-001) - python -m ownlang ownir facts.json --format github|msbuild|human [--severity error|warning] + python -m ownlang ownir facts.json --format github|msbuild|human|sarif `--format` (ownir only) selects the finding surface: `human` (default CLI line), -`github` (CI annotations on the PR diff), or `msbuild` (VS Error List). +`github` (CI annotations on the PR diff), `msbuild` (VS Error List), or `sarif` +(a SARIF 2.1.0 log — GitHub code scanning, and the cross-tool oracle reads it too). `--severity` (ownir only) picks how the host shows a finding — `error` (default, fails a build / red check) or `warning` (advisory). It is a presentation choice; the finding is still the core's verdict. @@ -195,11 +196,12 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", verbosity: str = "normal") -> int: """Check OwnIR facts (extracted from real C# by the Roslyn frontend) through the same core, surfacing findings at their C# locations (P-001). `fmt` - selects the surface: human (CLI), github (CI annotations), msbuild (VS); + selects the surface: human (CLI), github (CI annotations), msbuild (VS), + sarif (SARIF 2.1.0 log); `severity` picks how the host shows them (error/warning); `verbosity` is `quiet` (errors only — hide the advisory OWN050 notes), `normal` (default), or `verbose` (also print a per-code breakdown).""" - from .ownir import OwnIRError, check_facts, load, render_finding + from .ownir import OwnIRError, build_sarif, check_facts, load, render_finding try: findings = check_facts(load(path)) except OwnIRError as e: @@ -209,7 +211,7 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", # In a machine format, stdout carries only the annotations/diagnostics a host # (GitHub, MSBuild/VS) parses; the human summary goes to stderr so it cannot # pollute that stream. - machine = fmt in {"github", "msbuild"} + machine = fmt in {"github", "msbuild", "sarif"} summary_to = sys.stderr if machine else sys.stdout # OWN050 "leakage analysis skipped" notes are advisory (P-014 Tier A): always # shown as warnings regardless of --severity, and never affect the exit code — @@ -217,17 +219,25 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", leaks = [f for f in findings if not f.advisory] notes = [f for f in findings if f.advisory] shown = leaks if verbosity == "quiet" else findings - for f in shown: - # Severity is the weaker of the host's --severity and the finding's own - # intrinsic level: an advisory note (OWN050) is always a warning; a global - # `--severity warning` downgrades everything; and a finding the extractor - # could not prove a leak (an injected-source subscription, f.severity == - # "warning") shows as a warning even at the default error level (P-004). - if f.advisory or severity == "warning" or f.severity == "warning": - fsev = "warning" - else: - fsev = severity - print(render_finding(f, fmt, fsev)) + if fmt == "sarif": + # SARIF is one document for the whole run (not a line per finding): stdout + # carries only the JSON; the summary goes to stderr like the other machine + # formats. build_sarif applies the same per-finding severity policy below. + import json + print(json.dumps(build_sarif(shown, severity), indent=2)) + else: + for f in shown: + # Severity is the weaker of the host's --severity and the finding's own + # intrinsic level: an advisory note (OWN050) is always a warning; a + # global `--severity warning` downgrades everything; and a finding the + # extractor could not prove a leak (an injected-source subscription, + # f.severity == "warning") shows as a warning even at the default error + # level (P-004). + if f.advisory or severity == "warning" or f.severity == "warning": + fsev = "warning" + else: + fsev = severity + print(render_finding(f, fmt, fsev)) if not shown: print(f"{path}: ok — no subscription leaks found", file=summary_to) n = len(leaks) @@ -245,7 +255,7 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", return 1 if leaks else 0 -_FORMATS = {"human", "github", "msbuild"} +_FORMATS = {"human", "github", "msbuild", "sarif"} _SEVERITIES = {"error", "warning"} _VERBOSITY = {"quiet", "normal", "verbose"} diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 9a455243..179cf8e7 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -125,7 +125,7 @@ ) from .di import LIFETIMES as DI_LIFETIMES from .di import Service, find_captive_dependencies -from .diagnostics import Severity +from .diagnostics import TITLES, Severity # The OwnIR schema version this core understands. Bump it whenever the fact # vocabulary changes incompatibly; the extractor stamps the same number so a @@ -303,6 +303,88 @@ def render_finding(f: Finding, fmt: str, severity: str = "error") -> str: return f.render(severity) +# SARIF 2.1.0 — the OASIS-standard static-analysis interchange format. Unlike the +# line-per-finding surfaces above, a SARIF log is ONE document for the whole run, +# so it is built from the finding *list*, not rendered per finding. +_SARIF_SCHEMA = ( + "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/" + "Schemata/sarif-schema-2.1.0.json" +) +_SARIF_INFO_URI = "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/PhysShell/Own.NET" + + +def _sarif_level(f: Finding, severity: str) -> str: + """The SARIF result level for a finding. Mirrors the CLI's per-finding severity + (an advisory OWN050 and an intrinsic-warning finding both stay sub-error), but + uses SARIF's dedicated `note` level for the advisory coverage notes — so a + consumer can tell a *skip* ("could not check this") from a real warning-tier + leak, which the flat error/warning surfaces cannot express.""" + if f.advisory: + return "note" + if severity == "warning" or f.severity == "warning": + return "warning" + return "error" + + +def _sarif_result(f: Finding, severity: str) -> dict[str, Any]: + """One SARIF `result` for a finding: the OWN code is the `ruleId`, the C# + location is a `physicalLocation`, and the resource kind / subscription triple + ride along in `properties` for any consumer that wants them.""" + phys: dict[str, Any] = { + "artifactLocation": {"uri": f.file.replace("\\", "/")}, + } + if f.line >= 1: # SARIF region.startLine is 1-based; omit it for a file-level finding + phys["region"] = {"startLine": f.line} + props: dict[str, Any] = {"resourceKind": f.kind} + for key, val in (("component", f.component), ("event", f.event), + ("handler", f.handler)): + if val: + props[key] = val + return { + "ruleId": f.code, + "level": _sarif_level(f, severity), + "message": {"text": f"{f.message} [resource: {f.kind}]"}, + "locations": [{"physicalLocation": phys}], + "properties": props, + } + + +def build_sarif(findings: list[Finding], severity: str = "error") -> dict[str, Any]: + """Render the findings as a single SARIF 2.1.0 log: one `run` whose + `tool.driver` is Own.NET (with a `rules` catalogue of the OWN codes present and + their titles) and whose `results` carry each finding's code, C# location, + message and resource kind. `severity` is the same presentation choice as the + other surfaces (it only sets each result's `level`). + + SARIF earns its place three ways: it is GitHub code-scanning native, it is a + frozen/diffable run artifact (reproducibility), and it is the *same* shape + `scripts/oracle_compare.py` already parses for Infer# and CodeQL — so own-check + can join the cross-tool diff through one SARIF reader instead of the bespoke + text parser (the parser-drift class of bug it documents).""" + codes = sorted({f.code for f in findings}) + rules = [ + {"id": code, "shortDescription": {"text": TITLES.get(code, code)}} + for code in codes + ] + return { + "$schema": _SARIF_SCHEMA, + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "Own.NET", + "informationUri": _SARIF_INFO_URI, + "rules": rules, + "properties": {"ownirSchemaVersion": OWNIR_VERSION}, + }, + }, + "results": [_sarif_result(f, severity) for f in findings], + }, + ], + } + + def load(path: str) -> dict[str, Any]: """Load and shape-check an OwnIR facts file (it is external input — a malformed file should fail with a clear error, not a deep traceback).""" diff --git a/scripts/oracle_compare.py b/scripts/oracle_compare.py index feae76de..ff272e27 100644 --- a/scripts/oracle_compare.py +++ b/scripts/oracle_compare.py @@ -98,6 +98,11 @@ def _own_class(code: str) -> str: def _oracle_class(tool: str, rule: str) -> str: r = rule or "" + if tool == "own": + # own-check emitting SARIF (--format sarif): classify by OWN code, exactly + # like the human-text path (build_own), so the two own input formats bucket + # identically. + return _own_class(r) if tool == "infersharp": # Infer's Pulse engine renamed the rule (RESOURCE_LEAK -> PULSE_RESOURCE_LEAK); # match the family by substring so version drift doesn't silence it. @@ -130,9 +135,18 @@ def norm_path(raw: str, strips: list[str]) -> str: def build_own(text: str, strips: list[str]) -> tuple[list[Finding], int]: - """Parse own-check human output into Findings (reusing the miner's parser). - Also returns the unparsed-line count, so drift in the own-check format is - surfaced rather than silently dropped (which would inflate `oracle-only`).""" + """Parse own-check output into Findings, in either format it emits: + + - **SARIF** (own-check `--format sarif`): read through the *same* `parse_sarif` + as the Infer#/CodeQL oracles. No bespoke text parser, so no parser-drift — + the class of bug that silently dropped 38 lines on the ScreenToGif run. + - **human text** (legacy default): the miner's regex parser. + + The second tuple element is the unparsed-line count (always 0 for SARIF), so + text-format drift is surfaced rather than silently dropped (which would inflate + `oracle-only`).""" + if text.lstrip().startswith("{"): + return parse_sarif(text, "own", strips), 0 sys.path.insert(0, str(Path(__file__).resolve().parent)) from mine_report import parse # local import: scripts/ is on the path now raw, unparsed = parse(text) @@ -361,7 +375,8 @@ def _is_test_path(path: str) -> bool: def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser( description="Diff Own.NET leak findings against Infer#/CodeQL (oracle).") - ap.add_argument("--own", help="own-check human output file") + ap.add_argument("--own", + help="own-check output — human text or a --format sarif log") ap.add_argument("--infersharp", help="Infer# SARIF (e.g. infer-out/report.sarif)") ap.add_argument("--codeql", help="CodeQL SARIF") ap.add_argument("--sarif", action="append", default=[], metavar="TOOL=PATH", @@ -532,7 +547,37 @@ def _selftest() -> int: if "product code only" not in render_md(r, "o/r", "abc", ["codeql"], 3, 0, 0, True): fails.append("scope note must render when exclude-tests mode is on (even at 0)") - total = 19 + # own-check can now emit SARIF (--format sarif); build_own reads it through the + # SAME parser as the oracles — no bespoke text parser, no drift. The shape + # mirrors ownlang.ownir.build_sarif. Codes must classify exactly like the text + # path, SARIF never reports "unparsed", and it diffs against an oracle the same. + own_sarif = json.dumps({"version": "2.1.0", "runs": [{"tool": {"driver": + {"name": "Own.NET"}}, "results": [ + {"ruleId": "OWN001", "level": "error", + "message": {"text": "leak [resource: disposable]"}, "locations": + [{"physicalLocation": {"artifactLocation": {"uri": "src/A.cs"}, + "region": {"startLine": 12}}}]}, + {"ruleId": "OWN003", "level": "error", + "message": {"text": "double [resource: disposable]"}, "locations": + [{"physicalLocation": {"artifactLocation": {"uri": "src/C.cs"}, + "region": {"startLine": 9}}}]}, + {"ruleId": "OWN050", "level": "note", "message": {"text": "skipped"}, + "locations": [{"physicalLocation": {"artifactLocation": {"uri": "src/D.cs"}, + "region": {"startLine": 3}}}]}, + ]}]}) + own_s, own_s_drift = build_own(own_sarif, []) + if own_s_drift != 0: + fails.append(f"own SARIF must never report unparsed, got {own_s_drift}") + cls_by_rule = {f.rule: f.cls for f in own_s} + if cls_by_rule != {"OWN001": "leak", "OWN003": "double", "OWN050": "other"}: + fails.append(f"own SARIF classes wrong: {cls_by_rule}") + r_s = compare(own_s, parse_sarif(infer, "infersharp", []), tol=3) + if len(r_s["agree"]) != 1 or (r_s["agree"] and r_s["agree"][0][0].fkey != "a.cs"): + fails.append(f"own SARIF agree wrong: {[f.fkey for f, _ in r_s['agree']]}") + if [f.rule for f in r_s["own_unique"]] != ["OWN003"]: + fails.append(f"own SARIF own_unique wrong: {[f.rule for f in r_s['own_unique']]}") + + total = 23 for f in fails: print(f"ORACLE SELFTEST FAIL: {f}") print(f"oracle_compare selftest: {total - len(fails)}/{total} checks passed") diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 300ad86c..a7c1ae29 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -25,10 +25,12 @@ import tempfile +from ownlang.diagnostics import TITLES from ownlang.ownir import ( OWNIR_VERSION, Finding, OwnIRError, + build_sarif, check_facts, load, render_finding, @@ -837,6 +839,75 @@ def _sub(source: str | None) -> list[Finding]: if not render_finding(fnd, "msbuild").startswith("src/A.cs(42): error OWN001:"): fails.append("msbuild default severity should remain error") + # --- SARIF 2.1.0 export (build_sarif) ------------------------------------ + canon = check_facts(facts) # the canonical one-leak CustomerViewModel fixture + sf = build_sarif(canon) + checks += 1 + if sf.get("version") != "2.1.0" or "$schema" not in sf or len(sf["runs"]) != 1: + fails.append(f"SARIF envelope wrong: version={sf.get('version')!r}") + driver = sf["runs"][0]["tool"]["driver"] + checks += 1 + if driver.get("name") != "Own.NET": + fails.append(f"SARIF tool.driver.name wrong: {driver.get('name')!r}") + checks += 1 + if [(r["id"], r["shortDescription"]["text"]) for r in driver["rules"]] != \ + [("OWN001", TITLES["OWN001"])]: + fails.append(f"SARIF rules catalogue wrong: {driver['rules']}") + checks += 1 + if len(sf["runs"][0]["results"]) != len(canon): + fails.append(f"SARIF results {len(sf['runs'][0]['results'])} != " + f"findings {len(canon)}") + else: + res0 = sf["runs"][0]["results"][0] + pl = res0["locations"][0]["physicalLocation"] + checks += 1 + if (res0["ruleId"], res0["level"], pl["artifactLocation"]["uri"], + pl["region"]["startLine"]) != \ + ("OWN001", "error", "CustomerViewModel.cs", 12): + fails.append(f"SARIF result0 wrong: {res0['ruleId']} {res0['level']} " + f"{pl['artifactLocation']['uri']}:{pl['region']['startLine']}") + checks += 1 + if "[resource: subscription token]" not in res0["message"]["text"]: + fails.append("SARIF result0 message lost the resource tag") + checks += 1 + if res0["properties"].get("resourceKind") != "subscription token" or \ + res0["properties"].get("event") != "bus.CustomerChanged": + fails.append(f"SARIF result0 properties wrong: {res0['properties']}") + + # level mapping: advisory -> note, intrinsic-warning -> warning, else error; + # a --severity warning host downgrades a hard leak (advisory stays a note). + _adv = Finding(file="A.cs", line=3, code="OWN050", component="", event="", + handler="", message="skip", kind="subscription token", advisory=True) + _wrn = Finding(file="A.cs", line=4, code="OWN001", component="C", event="e", + handler="h", message="leak", kind="subscription token", + severity="warning") + _err = Finding(file="A.cs", line=5, code="OWN001", component="C", event="e", + handler="h", message="leak", kind="subscription token") + checks += 1 + if [r["level"] for r in build_sarif([_adv, _wrn, _err])["runs"][0]["results"]] != \ + ["note", "warning", "error"]: + fails.append("SARIF level mapping (advisory/warning/error) wrong") + checks += 1 + if [r["level"] for r in build_sarif([_adv, _err], "warning")["runs"][0]["results"]] != \ + ["note", "warning"]: + fails.append("SARIF --severity warning should downgrade a hard leak") + # empty findings -> a valid, empty SARIF run (no crash, no bogus rules). + checks += 1 + empty = build_sarif([]) + if empty["runs"][0]["results"] or empty["runs"][0]["tool"]["driver"]["rules"]: + fails.append("SARIF empty run should have no results/rules") + + # the thesis: own-check SARIF is read by the oracle's EXISTING SARIF parser and + # classified as a leak — own joins the cross-tool diff with no bespoke text parser. + checks += 1 + sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts")) + import oracle_compare as _oc + rt = _oc.parse_sarif(json.dumps(build_sarif(canon)), "own", []) + if [(p.path, p.line, p.rule, p.cls) for p in rt] != \ + [("CustomerViewModel.cs", 12, "OWN001", "leak")]: + fails.append("SARIF oracle round-trip wrong: " + f"{[(p.path, p.line, p.rule, p.cls) for p in rt]}") + for f in fails: print(f"OWNIR FAIL: {f}") print(f"ownir: {checks - len(fails)}/{checks} bridge checks passed") From 3a8430d9042a9adc763c91ef410dc582b34d5f54 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 13:25:00 +0000 Subject: [PATCH 2/2] fix(sarif): gate own-SARIF sniff on `runs` shape; fix doc count wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two CodeRabbit review comments on #42. - oracle_compare.build_own: a leading `{` no longer blindly means SARIF. Parse and require a `runs` array before routing to parse_sarif; otherwise (a wrong file or malformed JSON) fall through to the text parser, so bad input surfaces as unparsed drift instead of a silent "0 findings, clean" — and a malformed-JSON crash is avoided too. Pinned by a new selftest case (24/24). - docs/notes/sarif-export.md: "dropped 38 of ~36 findings" was self-contradictory; the real incident is 38 lines left unparsed, so only 3 of ~36 findings reached the diff. ruff + mypy --strict clean; ownir 88/88; oracle selftest 24/24. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- docs/notes/sarif-export.md | 5 +++-- scripts/oracle_compare.py | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/notes/sarif-export.md b/docs/notes/sarif-export.md index 6c01fd63..ba30659a 100644 --- a/docs/notes/sarif-export.md +++ b/docs/notes/sarif-export.md @@ -10,8 +10,9 @@ of the `SARIF exporter` backlog item from `docs/notes/research-landscape-2026.md `scripts/oracle_compare.py` was asymmetric: Infer# and CodeQL were read with `parse_sarif`, but *our own* findings went through a regex over human text (`mine_report.parse`), which carries an explicit "unparsed-line" failure - bucket — the class of bug that silently dropped 38 of ~36 findings on the - ScreenToGif run (`real-world-mining.md`). With own-check emitting SARIF, the + bucket — the class of bug that silently left 38 lines unparsed, so only 3 of + ~36 findings reached the diff on the ScreenToGif run (`real-world-mining.md`). + With own-check emitting SARIF, the oracle reads **all three tools through one reader**. The fragile parser stops being on the critical path. 2. **GitHub code-scanning native.** A SARIF log uploads straight to code scanning diff --git a/scripts/oracle_compare.py b/scripts/oracle_compare.py index ff272e27..621ae063 100644 --- a/scripts/oracle_compare.py +++ b/scripts/oracle_compare.py @@ -146,7 +146,15 @@ def build_own(text: str, strips: list[str]) -> tuple[list[Finding], int]: text-format drift is surfaced rather than silently dropped (which would inflate `oracle-only`).""" if text.lstrip().startswith("{"): - return parse_sarif(text, "own", strips), 0 + try: + doc = json.loads(text) + except json.JSONDecodeError: + doc = None + if isinstance(doc, dict) and isinstance(doc.get("runs"), list): + return parse_sarif(text, "own", strips), 0 + # {-leading but not a SARIF log (wrong file? malformed JSON): fall through + # to the text parser, which surfaces it as unparsed drift rather than + # silently reporting "clean" (0 findings, 0 unparsed). sys.path.insert(0, str(Path(__file__).resolve().parent)) from mine_report import parse # local import: scripts/ is on the path now raw, unparsed = parse(text) @@ -576,8 +584,15 @@ def _selftest() -> int: fails.append(f"own SARIF agree wrong: {[f.fkey for f, _ in r_s['agree']]}") if [f.rule for f in r_s["own_unique"]] != ["OWN003"]: fails.append(f"own SARIF own_unique wrong: {[f.rule for f in r_s['own_unique']]}") - - total = 23 + # a {-leading input that is NOT a SARIF log (wrong file / malformed JSON) must + # not be silently treated as "clean" — it falls through to the text parser and + # surfaces as unparsed drift, not 0 findings / 0 unparsed. + not_sarif, ns_drift = build_own('{"notruns": []}\n', []) + if not_sarif or ns_drift < 1: + fails.append(f"non-SARIF JSON masked as clean: {len(not_sarif)} findings, " + f"{ns_drift} unparsed") + + total = 24 for f in fails: print(f"ORACLE SELFTEST FAIL: {f}") print(f"oracle_compare selftest: {total - len(fails)}/{total} checks passed")