Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .github/workflows/oracle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,18 +161,21 @@ jobs:
echo "framework refs not found — own-check resolves runtime types only"
fi

# Own.NET — no build needed; scans .cs directly. --severity warning so the
# warning-tier leaks (injected-source subscriptions, e.g. VideoSource) are
# included, not just the provable static-source errors. OWN_EXTRA_REF_DIRS
# (above) is inherited by the extractor process.
# Own.NET — no build needed; scans .cs directly. --format sarif so the diff
# reads our findings through the SAME parser as the Infer#/CodeQL SARIF (no
# bespoke text parser, no parser drift; own.txt holds a SARIF log — the
# comparator sniffs the format). --severity warning so the warning-tier leaks
# (injected-source subscriptions, e.g. VideoSource) are included, not just the
# provable static-source errors. OWN_EXTRA_REF_DIRS (above) is inherited by the
# extractor process.
- name: Own.NET own-check
env:
PATHS: ${{ steps.t.outputs.paths }}
run: |
scan="target"; [[ -n "$PATHS" ]] && scan="target/$PATHS"
set +e
scripts/own-check.sh --format human --severity warning -- "$scan" > own.txt 2> own-extract.log
echo "own-check rc=$? ; $(wc -l < own.txt) finding line(s)"
scripts/own-check.sh --format sarif --severity warning -- "$scan" > own.txt 2> own-extract.log
echo "own-check rc=$? ; own.txt is a SARIF log ($(wc -c < own.txt) bytes)"

# CodeQL — database from source (no build). The dispose/leak queries
# (cs/local-not-disposed & friends) are *quality* queries, absent from the
Expand Down
5 changes: 3 additions & 2 deletions docs/notes/mining.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ scripts/mine.sh DapperLib/Dapper # whole repo
scripts/mine.sh --paths src JoshClose/CsvHelper # focus a subdir
```

Output → `corpus/mined/<slug>/` (gitignored): `findings.txt`, `extract.log`,
`report.md`, `report.json`. Seed targets live in `corpus/targets.txt`.
Output → `corpus/mined/<slug>/` (gitignored): `findings.txt` (a SARIF 2.1.0 log
by default — `mine_report` reads it structurally; see `sarif-export.md`),
`extract.log`, `report.md`, `report.json`. Seed targets live in `corpus/targets.txt`.

## What the report says — and how to read it

Expand Down
36 changes: 25 additions & 11 deletions docs/notes/sarif-export.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,31 @@ 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)
## Follow-up pass — SARIF is now the live path

- **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.
The two internal follow-ups are done (a second slice):

- **`mine_report.py` reads SARIF.** `parse()` sniffs a `{`-leading `runs` log and
yields the same finding dicts as the text path (level → severity: `error` →
error, `warning`/`note` → advisory, so OWN050 stays advisory and an
injected-source warning stays advisory; kind from `properties.resourceKind`; the
trailing `[resource: …]` split off the message), so the aggregation is identical
between formats. The regex parser is now off the default path for **both**
consumers — the oracle (slice 2 above) and the miner. Pinned by a SARIF selftest
(14/14) and a human-vs-SARIF aggregation-parity check.
- **The producers emit SARIF.** `mine.sh` now defaults to `--format sarif` and
`oracle.yml` runs own-check `--format sarif`; the intermediate (`findings.txt` /
`own.txt`) carries a SARIF log and the human-facing output stays the rendered
`report.md`. Both consumers sniff the format, so it is backward-compatible (a
text intermediate still parses). This retires the documented parser-drift bug on
the **live** eval paths, not just in capability. (Filenames kept to keep the CI
edit minimal; the content, not the extension, is what the consumers read.)

Still open:

- **GitHub code-scanning upload.** Deferred deliberately: this repo's C# is test
fixtures, so uploading own-check SARIF to *its own* Security tab is low-value.
Code scanning belongs in the consumer-facing distribution surface (P-013), run
on a real target — not here.
- **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).
8 changes: 6 additions & 2 deletions scripts/mine.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# spot-check, not a crawler. Be a good citizen.
#
# Usage:
# scripts/mine.sh [--ref <branch|tag|sha>] [--paths <subdir>] [--format human]
# scripts/mine.sh [--ref <branch|tag|sha>] [--paths <subdir>] [--format sarif|human]
# [--out <dir>] [--keep-src] <owner/repo | git-url>
#
# Output goes to corpus/mined/<slug>/ (gitignored): findings.txt, extract.log,
Expand All @@ -18,7 +18,11 @@ set -euo pipefail

ref=""
subpaths=""
format="human"
# SARIF by default: mine_report.py reads it structurally (no regex, no parser
# drift — the class of bug that silently dropped findings on the ScreenToGif run).
# findings.txt then holds a SARIF log; the human view is report.md. `--format human`
# still works (mine_report sniffs the format).
format="sarif"
outdir=""
keep_src=0
target=""
Expand Down
153 changes: 136 additions & 17 deletions scripts/mine_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
"""
Mining report aggregator — corpus mining (see docs/notes/mining.md).

Reads the human-format findings that `own-check` prints over a real C# repo and
turns them into a structured Markdown summary: counts by OWN code, severity
(errors = candidate leaks, warnings = advisory / OWN050 "unchecked"), resource
kind, the noisiest files, and a triage list of the error-severity findings to
eyeball.
Reads own-check findings — its human text **or** a `--format sarif` log — over a
real C# repo and turns them into a structured Markdown summary: counts by OWN
code, severity (errors = candidate leaks, warnings = advisory / OWN050
"unchecked"), resource kind, the noisiest files, and a triage list of the
error-severity findings to eyeball.

This is evaluation tooling for the analyser itself: a clean run is a precision
signal; a pile of OWN001s is either real bugs or a false-positive pattern to
harden; a high OWN050 count flags a coverage gap (unresolved external refs).

dotnet-free: the extractor (own-check) runs upstream; this only reads its text.
dotnet-free: the extractor (own-check) runs upstream; this only reads its output.

Usage:
own-check.sh --format human -- <repo> | mine_report.py --repo owner/name --commit SHA
mine_report.py findings.txt --repo owner/name --commit SHA [--json out.json]
own-check.sh --format sarif -- <repo> | mine_report.py --repo owner/name --commit SHA
mine_report.py findings.{txt,sarif} --repo owner/name --commit SHA [--json out.json]
mine_report.py --selftest
"""

Expand Down Expand Up @@ -72,15 +72,84 @@ def _net_open(s: str) -> int:
+ s.count("[") - s.count("]"))


def _as_dict(x: Any) -> dict[str, Any]:
"""`x` if it is a dict, else an empty one — lets the SARIF field walk be written
as plain `.get()` chains that degrade (rather than crash) on a malformed node,
since the findings file is external input."""
return x if isinstance(x, dict) else {}


def _sarif_finding(res: dict[str, Any]) -> dict[str, Any]:
"""Turn one SARIF result into the same finding dict the human-text parser yields
(file / line / severity / code / message / kind), so the aggregation is
format-agnostic. The SARIF level maps to the human severity (`error` -> error,
`warning`/`note` -> warning), so an advisory OWN050 `note` and an injected-source
`warning` both count as advisory exactly as in the text path; the resource kind
comes from `properties.resourceKind`, and a trailing ` [resource: kind]` is split
off the message for parity with the human format. Every access is shape-guarded
so a garbage sub-node yields a default, not a crash."""
locs = res.get("locations")
first = locs[0] if isinstance(locs, list) and locs else {}
phys = _as_dict(_as_dict(first).get("physicalLocation"))
uri = _as_dict(phys.get("artifactLocation")).get("uri", "")
line = _as_dict(phys.get("region")).get("startLine", 0)
msg = _as_dict(res.get("message")).get("text")
msg = msg if isinstance(msg, str) else ""
kind = _as_dict(res.get("properties")).get("resourceKind")
kind = kind if isinstance(kind, str) else ""
tail = _RESOURCE_TAIL.search(msg)
if tail:
kind = kind or tail["kind"]
msg = msg[:tail.start()].rstrip()
code = res.get("ruleId")
return {
"file": uri if isinstance(uri, str) else "",
"line": line if isinstance(line, int) else 0,
"severity": "error" if res.get("level") == "error" else "warning",
"code": code if isinstance(code, str) else "",
"message": msg,
"kind": kind,
}


def _parse_sarif(doc: dict[str, Any]) -> list[dict[str, Any]]:
"""Every result across every run of a SARIF log, as finding dicts. Malformed
nodes (a non-dict run/result, a non-list `results`) are skipped rather than
crashed on — own-check emits well-formed SARIF, but the input file is external."""
out: list[dict[str, Any]] = []
runs = doc.get("runs")
if not isinstance(runs, list):
return out
for run in runs:
results = run.get("results") if isinstance(run, dict) else None
if not isinstance(results, list):
continue
out.extend(_sarif_finding(res) for res in results if isinstance(res, dict))
return out


def parse(text: str) -> tuple[list[dict[str, Any]], int]:
"""Parse own-check human output into finding dicts; return (findings, unparsed).
A finding may span several lines — an inline lambda handler body is echoed across
lines, with the trailing `[resource: kind]` tag (when present) on the last. A
non-header line extends the current finding's message ONLY while that message's
brackets are still unbalanced (i.e. we are inside the lambda body); once balanced,
the finding is complete and a further non-matching line is stray drift, counted —
not silently absorbed. Build chatter shouldn't reach here (own-check sends it to
stderr); a stray line that does and isn't chatter is counted, not dropped."""
"""Parse own-check output into finding dicts; return (findings, unparsed).

Accepts either format own-check emits. **SARIF** (`--format sarif`, a `{`-leading
log with a `runs` array) is read structurally — no regex, `unparsed` is always 0.
Otherwise the **human text**: a finding may span several lines — an inline lambda
handler body is echoed across lines, with the trailing `[resource: kind]` tag
(when present) on the last. A non-header line extends the current finding's message
ONLY while that message's brackets are still unbalanced (i.e. we are inside the
lambda body); once balanced, the finding is complete and a further non-matching
line is stray drift, counted — not silently absorbed. Build chatter shouldn't reach
here (own-check sends it to stderr); a stray line that does and isn't chatter is
counted, not dropped."""
if text.lstrip().startswith("{"):
try:
doc = json.loads(text)
except json.JSONDecodeError:
doc = None
if isinstance(doc, dict) and isinstance(doc.get("runs"), list):
return _parse_sarif(doc), 0
# {-leading but not a SARIF log: fall through to the text parser, which
# surfaces it as unparsed drift rather than silently reporting "clean".
findings: list[dict[str, Any]] = []
unparsed = 0
cur: dict[str, Any] | None = None
Expand Down Expand Up @@ -248,9 +317,59 @@ def _selftest() -> int:
# the --stats coverage line is surfaced when supplied (header + clean note).
if "42 methods" not in render_md([], 0, "o/r", "abc123", "coverage: 1/42 methods"):
fails.append("coverage line not rendered")

# own-check can also feed SARIF (--format sarif); parse() sniffs it and yields the
# SAME finding dicts as the human path, so the aggregation is identical. The shape
# mirrors ownlang.ownir.build_sarif (note level = advisory OWN050).
def _res(code: str, level: str, uri: str, line: int, kind: str) -> dict[str, Any]:
return {"ruleId": code, "level": level,
"message": {"text": f"a leak [resource: {kind}]"},
"locations": [{"physicalLocation": {
"artifactLocation": {"uri": uri}, "region": {"startLine": line}}}],
"properties": {"resourceKind": kind}}
sarif_text = json.dumps({"version": "2.1.0", "runs": [{"tool": {"driver":
{"name": "Own.NET"}}, "results": [
_res("OWN001", "error", "src/A.cs", 12, "disposable"),
_res("OWN001", "error", "src/A.cs", 40, "disposable"),
_res("OWN002", "error", "src/A.cs", 30, "disposable"),
_res("OWN050", "note", "src/B.cs", 5, "unresolved reference"),
]}]})
sf, sf_unparsed = parse(sarif_text)
sagg = aggregate(sf)
if sf_unparsed != 0:
fails.append(f"SARIF input must not report unparsed, got {sf_unparsed}")
if sagg["by_code"] != {"OWN001": 2, "OWN002": 1, "OWN050": 1}:
fails.append(f"SARIF by_code wrong: {sagg['by_code']}")
if (sagg["errors"], sagg["advisories"]) != (3, 1):
fails.append(f"SARIF severity split wrong: {sagg['errors']}/{sagg['advisories']}")
if sagg["by_kind"].get("disposable") != 3 or sagg["by_file"].get("src/A.cs") != 3:
fails.append(f"SARIF by_kind/by_file wrong: {sagg['by_kind']} {sagg['by_file']}")
a0 = next(f for f in sf if f["code"] == "OWN050")
if a0["kind"] != "unresolved reference" or "[resource:" in a0["message"]:
fails.append(f"SARIF message/kind split wrong: {a0}")
# a {-leading input that is NOT a SARIF log falls through to the text parser and
# surfaces as unparsed drift, not a silent clean read.
_, ns_drift = parse('{"notruns": 1}\n')
if ns_drift != 1:
fails.append(f"non-SARIF JSON should surface as drift, got {ns_drift} unparsed")
# malformed SARIF nodes (non-dict run/result, non-list results, garbage
# sub-fields) are skipped, not crashed on — valid results still come through.
malformed = json.dumps({"runs": [
1, # non-dict run -> skipped
{"results": 7}, # non-list results -> skipped
{"results": [
2, # non-dict result -> skipped
{"ruleId": "OWN001", "level": "error", "message": "m",
"locations": "nope", "properties": 5}, # garbage sub-fields -> defaults
]},
]})
mf, mf_drift = parse(malformed)
if mf_drift != 0 or [f["code"] for f in mf] != ["OWN001"]:
fails.append(f"malformed SARIF not handled gracefully: {mf_drift} {mf}")

for f in fails:
print(f"MINE SELFTEST FAIL: {f}")
print(f"mine_report selftest: {8 - len(fails)}/8 checks passed")
print(f"mine_report selftest: {15 - len(fails)}/15 checks passed")
return 1 if fails else 0


Expand Down
2 changes: 1 addition & 1 deletion scripts/own-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# only extracts facts.
#
# Usage:
# scripts/own-check.sh [--format human|github|msbuild] [--severity error|warning]
# scripts/own-check.sh [--format human|github|msbuild|sarif] [--severity error|warning]
# [--fail-on-finding] [--legacy] [--stats] [--root <own.net checkout>]
# [--] <path|file> [more ...]
#
Expand Down
Loading