From e33c0a7aa14cd23a0199754cb8d1fd30711b5871 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 21:24:44 +0000 Subject: [PATCH 1/2] feat(suppression): implement [OwnIgnore("reason")] end to end (P-004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the per-site suppression attribute the FP-policy page marked "designed, not implemented". A `[OwnIgnore("reason")]` on an IDisposable field now suppresses its OWN001 — but with VISIBILITY OVER SILENCE: the finding is still minted, kept out of the exit code and the human findings stream, yet COUNTED (a run-summary tally) and carried in SARIF `suppressions` (kind "inSource", the reason as justification). A suppressed finding never fails the run. The reason is mandatory by design (a suppression is a documented decision): a reason-less `[OwnIgnore]` or an empty `[OwnIgnore("")]` does NOT suppress — never a silent accept. Consumed core-side (P-013 "one checker"): the extractor emits the finding fact with an additive-optional `ignore_reason` marker; the core is the sole authority on the verdict. No OWNIR_VERSION bump (additive optional, mirrors source_provenance). - Extractor: OwnIgnoreReason() reads `[OwnIgnore]` (matched by simple name, so a project may declare its own attribute) on IDisposable field declarations via the SemanticModel's constant folding; stamps `ignore_reason` only for a non-empty reason. - Core: Finding.ignore_reason + `suppressed` property; check_facts stamps it; cmd_ownir excludes suppressed from the exit code and prints a tally; SARIF emits the `suppressions` array; load() validates the field type; dedup key updated. - Sample OwnIgnoreSample.cs: unsuppressed / suppressed / reason-less / empty-reason contrast, wired into the C# leak-extractor CI job with a SARIF suppressions assertion. - Pinned in tests/test_ownir.py; documented in spec/OwnIR.md §4 + ownir.schema.json; docs/suppression-and-fp-policy.md flipped to shipped (P-015 config kept draft). Locally validated (real extractor + core): suppressed leak silent-but-counted, present in SARIF with inSource justification; reason-less/empty fire OWN001; a suppressed-only run exits 0; non-string ignore_reason fails loud at load. Gates: run_tests.py, ruff, mypy --strict on ownlang all green. Closes #209 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG --- .github/workflows/ci.yml | 31 ++++++++- docs/suppression-and-fp-policy.md | 34 ++++++---- frontend/roslyn/OwnSharp.Extractor/Program.cs | 66 ++++++++++++++++--- frontend/roslyn/samples/OwnIgnoreSample.cs | 58 ++++++++++++++++ ownlang/__main__.py | 19 ++++-- ownlang/ownir.py | 45 +++++++++++-- spec/OwnIR.md | 14 ++++ spec/ownir.schema.json | 4 ++ tests/test_ownir.py | 58 ++++++++++++++++ 9 files changed, 298 insertions(+), 31 deletions(-) create mode 100644 frontend/roslyn/samples/OwnIgnoreSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8d4473f6..1942a222 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -205,6 +205,7 @@ jobs: frontend/roslyn/samples/SemaphoreFieldSample.cs \ frontend/roslyn/samples/VoidSubscribeSample.cs \ frontend/roslyn/samples/ReturnedPublisherSample.cs \ + frontend/roslyn/samples/OwnIgnoreSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -723,7 +724,35 @@ jobs: if echo "$out" | grep -qE "(ScopeUsingService|ClockCachingService)"; then echo "FAIL: a correct scope use (used-in-scope, or a cached singleton) was wrongly flagged DI005"; exit 1 fi - echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) at the C# location" + # issue #209 — inline [OwnIgnore("reason")] per-site suppression (P-004), on an + # IDisposable field. Four contrasting shapes in OwnIgnoreSample.cs: the un-annotated + # leak, a reason-less [OwnIgnore], and an empty [OwnIgnore("")] all FIRE OWN001; only + # [OwnIgnore("reason")] is silent-but-COUNTED (SARIF suppressions), never failing the run. + echo "$out" | grep -qE "OwnIgnoreSample\.cs:[0-9]+: error: \[OWN001\].*'UnsuppressedLeak'" \ + || { echo "FAIL: an un-annotated IDisposable field must raise OWN001"; exit 1; } + echo "$out" | grep -qE "\[OWN001\].*'ReasonlessLeak'" \ + || { echo "FAIL: a reason-less [OwnIgnore] must NOT suppress (OWN001 must still fire)"; exit 1; } + echo "$out" | grep -qE "\[OWN001\].*'EmptyReasonLeak'" \ + || { echo "FAIL: an empty [OwnIgnore(\"\")] reason must NOT suppress (OWN001 must still fire)"; exit 1; } + # the suppressed leak is SILENT in the human findings stream... + if echo "$out" | grep -q "'SuppressedLeak'"; then + echo "FAIL: a [OwnIgnore(\"reason\")] finding must be silent in the human output"; exit 1 + fi + # ...but COUNTED in the run summary (visibility over silence). + echo "$out" | grep -qE "[0-9]+ suppressed \(\[OwnIgnore\]\)" \ + || { echo "FAIL: the suppressed finding must be counted in the summary tally"; exit 1; } + # SARIF carries it as a result WITH a `suppressions` array (kind inSource + the + # mandatory reason as justification) — a consumer counts it, GitHub shows it + # suppressed rather than an open alert, and it never fails the run. + python -m ownlang ownir "$RUNNER_TEMP/facts.json" --format sarif > "$RUNNER_TEMP/own.sarif" 2>/dev/null || true + jq -e '[.runs[0].results[] | select(.properties.component == "SuppressedLeak")] as $s + | ($s | length) == 1 + and ($s[0].suppressions[0].kind == "inSource") + and ($s[0].suppressions[0].justification | contains("owned and disposed by the DI container"))' \ + "$RUNNER_TEMP/own.sarif" >/dev/null \ + || { echo "FAIL: SARIF must carry SuppressedLeak once, with an inSource suppressions justification"; \ + jq '.runs[0].results[]|{ruleId,component:.properties.component,suppressions}' "$RUNNER_TEMP/own.sarif"; exit 1; } + echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) at the C# location" - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) run: | # Path-sensitive flow analysis of local IDisposables — bugs the flat D1 diff --git a/docs/suppression-and-fp-policy.md b/docs/suppression-and-fp-policy.md index 096fdd75..b9c5ecbf 100644 --- a/docs/suppression-and-fp-policy.md +++ b/docs/suppression-and-fp-policy.md @@ -45,30 +45,36 @@ fire on unprovable input. |---|---|---| | `--severity warning` | **works today** (P-013) | Global: downgrades every error-tier finding for that run to advisory. Per-run, not per-finding — an escape hatch for "show me everything, but don't fail the build yet," not a way to silence one specific site. | | `--fail-on-finding` set to off | **works today** (P-013) | Global: findings still print/annotate, but the process/step exit code stays 0. The CLI (`own-check.sh`) is off by default — you must pass the flag to make findings fail the shell. The GitHub Action inverts that for safety: its `fail-on-finding` input defaults to `"true"` (fails the step on a finding), so to get the "annotate but don't fail" behavior in CI you must explicitly set `fail-on-finding: "false"`. | -| `[OwnIgnore("reason")]` | **designed, not implemented** (P-004) | Inline, per-site suppression attribute — the intended fine-grained escape hatch for a specific subscription/field the checker can't see enough context to clear. Referenced across P-001/P-004/P-010/P-014/P-017 as the standing design; there is no code behind it yet. If you need this today, the honest answer is: you don't have it — file the case so it informs the implementation. | +| `[OwnIgnore("reason")]` | **works today** on `IDisposable` fields (P-004, #209) | Inline, per-site suppression attribute — the fine-grained escape hatch for a specific site the checker can't see enough context to clear. Put `[OwnIgnore("reason")]` on the field; the finding is then **silent-but-counted** — kept out of the exit code and the human findings stream, but tallied in the run summary and carried in SARIF `suppressions` (`kind: "inSource"`, your reason as the `justification`) so nothing is lost and a consumer can audit it. The **reason is mandatory**: a reason-less `[OwnIgnore]` (or an empty `[OwnIgnore("")]`) does **not** suppress — a suppression is a documented decision, never a silent accept. The attribute is matched by simple name, so you can declare your own `OwnIgnoreAttribute`. Currently reads on `IDisposable` **field** declarations (the clearest attribute site); other sites (subscriptions, timers) are follow-up increments. | | Project-wide config (`.ownrc`/`own.toml`) | **draft, not implemented** (P-015) | Per-check-category enable/disable + severity + per-path overrides (e.g. relax a category under `tests/`). Stub status — format (TOML vs INI vs JSON) and enforcement point are still open questions in the proposal. | | `corpus/oracle-fp-baseline.txt` | **exists, but not a user-facing suppression tool** | An allowlist the *oracle comparator* (`scripts/oracle_compare.py`, a dev/maintainer tool) uses to keep already-triaged false positives out of the `own-only` bucket on re-runs. It doesn't change what `own-check`/the Action reports — it only keeps the oracle's own triage queue from re-showing confirmed noise. | -So today, honestly: there is no way to suppress **one specific finding** in -your own repo. The two escape hatches for that (`[OwnIgnore]`, project config) -are designed and drafted respectively, not shipped. What you have is a global -severity dial and the extractor's own honest-skip behavior, which is why the -precision bar above matters as much as the (currently thin) suppression -surface — the fewer false positives reach you, the less suppression UX has to -carry. +So today: you **can** suppress one specific finding with an inline +`[OwnIgnore("reason")]` on the field it fires on (shipped, #209) — the finding +goes silent but stays counted (summary tally + SARIF `suppressions`). The +project-wide counterpart (`.ownrc`/`own.toml`, P-015) is drafted, not shipped. +Together with the global severity dial and the extractor's own honest-skip +behavior, that covers per-site and per-run; the per-*category*, per-*path* +config is the remaining gap. The precision bar above still matters as much as +the suppression surface — the fewer false positives reach you, the less +suppression UX has to carry. -## The designed shape (so you know what's coming) +## The full shape (`[OwnIgnore]` shipped; config still to come) -Precedence, once both land (P-015's draft order): +Precedence (P-015's draft order — the inline attribute half is shipped, #209; +the config-file half is still draft): ```text CLI flag > inline [OwnIgnore] > config file > built-in default ``` -`[OwnIgnore("reason")]` (P-004) is a per-site attribute — the *reason* string -is mandatory by design, so a suppression is a documented decision, not a -silent one. Project config (P-015) is the per-category, project-wide -counterpart — "treat subscriptions as warnings, keep disposables as errors, +`[OwnIgnore("reason")]` (P-004, **shipped** #209) is a per-site attribute — the +*reason* string is mandatory by design, so a suppression is a documented +decision, not a silent one. It is consumed **core-side** (the extractor emits +the finding fact with the reason marker; the core decides the verdict, keeps it +out of the exit code, and stamps SARIF `suppressions`), so a suppression is +counted, never a silent drop. Project config (P-015, draft) is the +per-category, project-wide counterpart — "treat subscriptions as warnings, keep disposables as errors, skip pool checks under `tests/`" — discovered by walking up from the scanned path, the same convention as `.editorconfig`/`ruff.toml`. Both are consumed **core-side** ([P-013](proposals/P-013-distribution-surface.md)'s "one diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 24d64ddf..ad4c832f 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -3448,6 +3448,39 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) return false; } +// [OwnIgnore("reason")] per-site suppression (P-004 / issue #209). The reason string is +// MANDATORY by design — a suppression is a documented decision, not a silent one — so this +// returns the reason ONLY when the attribute carries a constant, non-empty string first +// argument; it returns null when the attribute is absent, reason-less (`[OwnIgnore]`), or +// carries an empty/non-constant reason. A null therefore never suppresses (the finding fires), +// which is exactly P-004's "never a silent accept" posture. Matched by SIMPLE name +// (`OwnIgnore` / `OwnIgnoreAttribute`), namespace-agnostic, like the BCL `[SuppressMessage]` +// convention — so a user may declare their own attribute type; the core (not this frontend) is +// the sole authority on the resulting verdict (P-013 "one checker"). Uses the SemanticModel's +// constant folding so a `const`/`nameof` reason resolves, not only a bare string literal. +static string? OwnIgnoreReason(SyntaxList attrLists, SemanticModel model) +{ + foreach (var al in attrLists) + foreach (var a in al.Attributes) + { + var simple = a.Name switch + { + QualifiedNameSyntax q => q.Right.Identifier.Text, + SimpleNameSyntax s => s.Identifier.Text, + _ => a.Name.ToString(), + }; + if (simple is not ("OwnIgnore" or "OwnIgnoreAttribute")) + continue; + var arg = a.ArgumentList?.Arguments.FirstOrDefault(); + if (arg is null) + return null; // reason-less [OwnIgnore] -> does not suppress + if (model.GetConstantValue(arg.Expression).Value is string r && r.Length > 0) + return r; + return null; // empty / non-constant reason -> does not suppress + } + return null; +} + var components = new List(); // P-016 B0b/B2: per-method flow bodies (only when --flow-locals). var flowFunctions = new List(); @@ -3965,14 +3998,31 @@ or ImplicitObjectCreationExpressionSyntax && fieldCtors.Count > 0 && fieldCtors.All(c => IsNoOpDisposeWrapper(c, model))) continue; - subs.Add(new - { - @event = v.Identifier.Text, - line = LineOf(v), - released = disposed.Contains(v.Identifier.Text), - resource = "disposable", - type = tname, - }); + // [OwnIgnore("reason")] on the field declaration (P-004 / #209): emit the + // record WITH the reason marker rather than dropping it, so the core can + // count the suppression and SARIF carries it in `suppressions` (visibility + // over silence). Additive/optional: absent when there is no valid reason, so + // an older core ignores it — exactly the source_provenance convention. + var ignoreReason = OwnIgnoreReason(fd.AttributeLists, model); + if (ignoreReason is not null) + subs.Add(new + { + @event = v.Identifier.Text, + line = LineOf(v), + released = disposed.Contains(v.Identifier.Text), + resource = "disposable", + type = tname, + ignore_reason = ignoreReason, + }); + else + subs.Add(new + { + @event = v.Identifier.Text, + line = LineOf(v), + released = disposed.Contains(v.Identifier.Text), + resource = "disposable", + type = tname, + }); } } diff --git a/frontend/roslyn/samples/OwnIgnoreSample.cs b/frontend/roslyn/samples/OwnIgnoreSample.cs new file mode 100644 index 00000000..2a151381 --- /dev/null +++ b/frontend/roslyn/samples/OwnIgnoreSample.cs @@ -0,0 +1,58 @@ +using System; + +namespace Own.Samples; + +// Issue #209 — [OwnIgnore("reason")] per-site suppression (P-004), on an IDisposable field. +// The reason string is MANDATORY by design: a suppression is a documented decision, never a +// silent one. Three contrasting shapes prove the contract end to end: +// - UnsuppressedLeak : the plain leak, no attribute -> OWN001 fires (control) +// - SuppressedLeak : same leak + [OwnIgnore("reason")] -> silent-but-COUNTED +// (SARIF `suppressions`), +// never fails the run +// - ReasonlessLeak : [OwnIgnore] with no reason -> must NOT suppress (fires) +// - EmptyReasonLeak : [OwnIgnore("")] -> must NOT suppress (fires) +// +// The attribute is matched by SIMPLE name, so a project may declare its own; this sample +// declares a local one (two ctors) so all four shapes compile. + +[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property + | AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false)] +public sealed class OwnIgnoreAttribute : Attribute +{ + public OwnIgnoreAttribute() { } + public OwnIgnoreAttribute(string reason) { Reason = reason; } + public string? Reason { get; } +} + +// A plain owned IDisposable — no BCL special-casing, so it is an unambiguous OWN001. +public sealed class Handle : IDisposable +{ + public void Dispose() { } +} + +// CONTROL: a `new`'d IDisposable field never disposed -> OWN001 fires. +public sealed class UnsuppressedLeak +{ + private readonly Handle _h = new Handle(); +} + +// SUPPRESSED: the same leak, but a documented [OwnIgnore("reason")] -> silent-but-counted. +public sealed class SuppressedLeak +{ + [OwnIgnore("owned and disposed by the DI container, not by this type")] + private readonly Handle _h = new Handle(); +} + +// REASON-LESS: [OwnIgnore] carries no reason -> must NOT suppress (never a silent accept). +public sealed class ReasonlessLeak +{ + [OwnIgnore] + private readonly Handle _h = new Handle(); +} + +// EMPTY REASON: [OwnIgnore("")] is not a documented decision -> must NOT suppress. +public sealed class EmptyReasonLeak +{ + [OwnIgnore("")] + private readonly Handle _h = new Handle(); +} diff --git a/ownlang/__main__.py b/ownlang/__main__.py index 75b89b1e..4b3da8d8 100644 --- a/ownlang/__main__.py +++ b/ownlang/__main__.py @@ -349,15 +349,22 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", # Advisory findings (OWN050 "leakage analysis skipped", OBL005 "dead protocol # rule") are always shown as warnings regardless of --severity, and never # affect the exit code — they are coverage/hygiene notes, not verdicts. - 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 + # Inline `[OwnIgnore("reason")]` suppressions (P-004, #209) are counted and carried in + # SARIF `suppressions`, but kept OUT of the human findings stream and the exit code — + # visibility over silence, without failing the run. Everything else is "active". + suppressed = [f for f in findings if f.suppressed] + active = [f for f in findings if not f.suppressed] + leaks = [f for f in active if not f.advisory] + notes = [f for f in active if f.advisory] + shown = leaks if verbosity == "quiet" else active 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. + # Suppressed findings ride along (marked with a `suppressions` array) so a + # SARIF consumer can count them rather than losing them. import json - print(json.dumps(build_sarif(shown, severity), indent=2)) + print(json.dumps(build_sarif(shown + suppressed, severity), indent=2)) else: for f in shown: # Severity is the weaker of the host's --severity and the finding's own @@ -382,6 +389,10 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", note_codes = "/".join(sorted({x.code for x in notes})) summary += (f" ({len(notes)} advisory hidden)" if verbosity == "quiet" else f", {len(notes)} advisory ({note_codes})") + if suppressed: + # counted, never silent: [OwnIgnore] suppressions are tallied here and carried in + # SARIF `suppressions`, but they do not print as findings and do not fail the run. + summary += f", {len(suppressed)} suppressed ([OwnIgnore])" print(summary + ".", file=summary_to) if verbosity == "verbose" and findings: by_code: dict[str, int] = {} diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 12721f04..3303a0f3 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -342,6 +342,18 @@ class Finding: # chain, each hop anchored at a real registration site. Rides along as a SARIF `codeFlows` # (the step-through trace `relatedLocations` cannot express). Empty for a single-point finding. flow: tuple[tuple[str, int, str], ...] = () + # P-004 `[OwnIgnore("reason")]` per-site suppression (#209): a non-None reason means the + # finding is SUPPRESSED — excluded from the exit code and the human findings stream, but + # still COUNTED (a summary tally) and carried in SARIF `suppressions` (visibility over + # silence, never a silent drop). The reason is mandatory by design (a documented decision): + # the extractor emits it only for an `[OwnIgnore]` with a non-empty reason, so a reason-less + # attribute never suppresses. Declared LAST (positional-constructor safe). + ignore_reason: str | None = None + + @property + def suppressed(self) -> bool: + """Whether an inline `[OwnIgnore("reason")]` suppresses this finding (P-004, #209).""" + return self.ignore_reason is not None def render(self, severity: str = "error") -> str: return (f"{self.file}:{self.line}: {severity}: [{self.code}] " @@ -438,6 +450,15 @@ def _sarif_result(f: Finding, severity: str) -> dict[str, Any]: flows = code_flow(f.flow) if flows: result["codeFlows"] = flows + # P-004 `[OwnIgnore("reason")]` (#209): a suppressed finding stays in `results` (so a + # consumer counts it) but carries the SARIF 2.1.0 `suppressions` property — `inSource` + # because the suppression is an in-code attribute, with the mandatory reason as the + # `justification`. GitHub code scanning and other consumers then show it as suppressed + # rather than an open alert, and it never fails the run. + if f.suppressed: + result["suppressions"] = [ + {"kind": "inSource", "justification": f.ignore_reason}, + ] return result @@ -541,6 +562,14 @@ def load(path: str) -> dict[str, Any]: raise OwnIRError( f"subscription 'source_provenance' must be a string, " f"got {spr!r}") + # `ignore_reason` (P-004 `[OwnIgnore("reason")]`, #209): the mandatory + # justification for an inline per-site suppression. Additive/optional; an + # older core ignores it and still reports the finding. When present it must + # be a string (an empty one does not suppress — see check_facts). + igr = s.get("ignore_reason") + if igr is not None and not isinstance(igr, str): + raise OwnIRError( + f"subscription 'ignore_reason' must be a string, got {igr!r}") # Optional DI registration graph (DI001 — captive dependency, P-006). Additive # and optional: an older core simply ignores it. svcs = result.get("services", []) @@ -2462,6 +2491,13 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: handler = sub.get("handler", "?") component = sub["component"] rkind = sub.get("resource", "subscription") + # P-004 `[OwnIgnore("reason")]` (#209): a non-empty reason on this record suppresses + # the finding — the core stays the sole authority (P-013 "one checker"), so we still + # MINT the finding and mark it suppressed rather than dropping it (counted, in SARIF + # `suppressions`, excluded from the exit code). An empty string is not a documented + # decision and does not suppress. Flow-local records never carry it (a method local + # cannot bear a C# attribute), so their branches below leave the finding unsuppressed. + ir = sub.get("ignore_reason") or None if rkind == "flow-local": # P-016 B0b/B2: path-sensitive local-IDisposable verdicts. The code is # the core's (OWN001/002/003/009); phrase it for the C# local. @@ -2562,7 +2598,8 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: findings.append(Finding( file=sub["file"], line=int(sub.get("line", 0)), code=d.code, component=component, event=event, handler=handler, - message=message, kind="subscription token", flow=esc_flow)) + message=message, kind="subscription token", flow=esc_flow, + ignore_reason=ir)) continue if rkind == "capture": # OWN014 region escape (P-004): the lifetime engine proved the event @@ -2590,7 +2627,7 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: findings.append(Finding( file=sub["file"], line=int(sub.get("line", 0)), code=d.code, component=component, event=event, handler=handler, - message=message, kind="subscription token")) + message=message, kind="subscription token", ignore_reason=ir)) continue _, kind = _RESOURCES.get(rkind, _RESOURCES["subscription"]) # P-004 tiering: only the plain `event += handler` leak (the else branch @@ -2653,7 +2690,7 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: findings.append(Finding( file=sub["file"], line=int(sub.get("line", 0)), code=d.code, component=component, event=event, handler=handler, - message=message, kind=kind, severity=fsev)) + message=message, kind=kind, severity=fsev, ignore_reason=ir)) # DI001 (captive dependency): a separate core analysis over the registration # graph, not the acquire/release model — the bridge just routes the facts to @@ -2707,7 +2744,7 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: deduped: list[Finding] = [] for f in findings: key = (f.file, f.line, f.code, f.component, f.event, f.handler, - f.message, f.kind, f.advisory, f.severity) + f.message, f.kind, f.advisory, f.severity, f.ignore_reason) if key in seen: continue seen.add(key) diff --git a/spec/OwnIR.md b/spec/OwnIR.md index b64d97a9..f5908e2b 100644 --- a/spec/OwnIR.md +++ b/spec/OwnIR.md @@ -121,6 +121,20 @@ locally-constructed source. The instance-level provenance beats the type-level DI hop (§6). Only this exact value routes; any other string keeps the honest OWN001 warning, and a non-string value is rejected at load. +**Inline suppression** (additive/optional, #209): any owned-resource record may +carry `ignore_reason: ""` — the mandatory justification of an inline +`[OwnIgnore("reason")]` at that site (P-004). When present and **non-empty**, the +core still **mints** the finding but marks it **suppressed**: it is excluded from +the exit code and the human findings stream, yet still **counted** (a summary +tally) and carried in SARIF `suppressions` (`kind: "inSource"`, the reason as +`justification`) — visibility over silence, never a silent drop. The reason is +mandatory by design: an **empty** string (or an absent field) does **not** +suppress — a reason-less `[OwnIgnore]` is never a silent accept. The core, not the +frontend, decides the verdict (P-013 "one checker"): the extractor only records +the reason it read. A non-string value is rejected at load. The Roslyn frontend +currently reads `[OwnIgnore]` on **`IDisposable` field declarations** (the clearest +attribute site — the record anchors at the field); other sites are follow-ups. + ## 5. Flow bodies (`functions[]`) A flow function has a `name`, a `file`, and a `body`: an ordered list of flow diff --git a/spec/ownir.schema.json b/spec/ownir.schema.json index 4a9b1ae5..97389be1 100644 --- a/spec/ownir.schema.json +++ b/spec/ownir.schema.json @@ -129,6 +129,10 @@ "source_provenance": { "description": "Interprocedural publisher provenance for an injected source (#146). The only routing value is \"returned_fresh\": every in-compilation caller passes a publisher it freshly constructs and lets escape only into this call or its own return, so the subscription is bounded by the returned publisher's lifetime (silent). Any other string keeps the honest OWN001 warning. Additive/optional.", "type": ["string", "null"] + }, + "ignore_reason": { + "description": "The mandatory justification of an inline [OwnIgnore(\"reason\")] per-site suppression (P-004, #209). When present and non-empty the core marks this record's finding SUPPRESSED: excluded from the exit code and the human findings stream, but still counted and carried in SARIF `suppressions` (visibility over silence). An empty string does not suppress (never a silent accept). Additive/optional; an older core ignores it and reports the finding.", + "type": ["string", "null"] } } }, diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 7c46d09d..de81839c 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -252,6 +252,64 @@ def _prov(provenance: str | None, source_type: str | None = None, fails.append("non-string source_provenance was accepted " "(should raise OwnIRError)") + # --- #209: inline `[OwnIgnore("reason")]` per-site suppression (P-004). A + # disposable-field record carrying a non-empty `ignore_reason` still MINTS + # the OWN001 finding, but the core marks it SUPPRESSED (counted, in SARIF + # `suppressions`, excluded from the exit code) — visibility over silence. An + # empty/absent reason never suppresses (never a silent accept). The reason is + # mandatory by design. + def _ign(reason: str | None) -> list[Finding]: + s: dict[str, object] = { + "event": "_h", "line": 5, "released": False, + "resource": "disposable", "type": "Handle"} + if reason is not None: + s["ignore_reason"] = reason + return check_facts({"module": "M", "components": [ + {"name": "Leaky", "file": "F.cs", "subscriptions": [s]}]}) + + # a documented reason -> the finding is still produced (visibility) but suppressed. + checks += 1 + _supp = _ign("owned by the container") + if not (len(_supp) == 1 and _supp[0].code == "OWN001" + and _supp[0].suppressed + and _supp[0].ignore_reason == "owned by the container"): + fails.append(f"[OwnIgnore] should mint a suppressed OWN001, got " + f"{[(x.code, x.suppressed, x.ignore_reason) for x in _supp]}") + # no attribute -> a normal, UN-suppressed OWN001. + checks += 1 + _plain = _ign(None) + if not (len(_plain) == 1 and _plain[0].code == "OWN001" + and not _plain[0].suppressed): + fails.append(f"a plain disposable field must be an un-suppressed OWN001, got " + f"{[(x.code, x.suppressed) for x in _plain]}") + # an EMPTY reason is not a documented decision -> must NOT suppress (fires). + checks += 1 + _empty = _ign("") + if not (len(_empty) == 1 and not _empty[0].suppressed): + fails.append(f"an empty [OwnIgnore] reason must not suppress, got " + f"{[(x.code, x.suppressed) for x in _empty]}") + # SARIF carries the suppressed finding as a result WITH a `suppressions` array + # (kind inSource, the reason as justification) — counted, not dropped. + checks += 1 + _sar = build_sarif(_supp) + _results = _sar["runs"][0]["results"] + _sup_results = [r for r in _results if "suppressions" in r] + if not (len(_results) == 1 and len(_sup_results) == 1 + and _sup_results[0]["suppressions"][0]["kind"] == "inSource" + and _sup_results[0]["suppressions"][0]["justification"] + == "owned by the container"): + fails.append(f"SARIF must carry the suppressed finding with a `suppressions` " + f"array (inSource + justification), got {_sup_results!r}") + # load() validates the field's type (additive optional, but never garbage). + checks += 1 + if not _load_raises({"ownir_version": OWNIR_VERSION, "module": "M", + "components": [{"name": "F", "file": "F.cs", + "subscriptions": [ + {"event": "_h", "line": 1, + "resource": "disposable", + "ignore_reason": 7}]}]}): + fails.append("non-string ignore_reason was accepted (should raise OwnIRError)") + # --- P-004 source-lifetime tiering for `subscribe` (ignored `.Subscribe()` # result) — the WalletWasabi precision win. A SELF-rooted subscribe # (`this.WhenAnyValue(x => x.SelfProp)`) is a GC-collectible self-cycle -> From c0f52aac15ec5cc0ecc702fe64c3d2f69fb5dea9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 02:34:49 +0000 Subject: [PATCH 2/2] ci: rc-capture for the OwnIgnore SARIF step instead of || true (CodeRabbit) Blanket `|| true` + 2>/dev/null swallowed a real ownir error (rc >= 2): the jq assertion would still fail the job, but the log showed only the assertion text, not the Python error. Mirror the established rc-capture pattern (same file, proj.sarif step): keep stderr, allow the expected rc<=1, fail loud on rc>=2. Per the PR #211 learning. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c657cd02..bb04d95b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -744,7 +744,8 @@ jobs: # SARIF carries it as a result WITH a `suppressions` array (kind inSource + the # mandatory reason as justification) — a consumer counts it, GitHub shows it # suppressed rather than an open alert, and it never fails the run. - python -m ownlang ownir "$RUNNER_TEMP/facts.json" --format sarif > "$RUNNER_TEMP/own.sarif" 2>/dev/null || true + rc=0; python -m ownlang ownir "$RUNNER_TEMP/facts.json" --format sarif > "$RUNNER_TEMP/own.sarif" || rc=$? + [ "$rc" -le 1 ] || { echo "FAIL: SARIF generation errored (rc=$rc)"; exit 1; } jq -e '[.runs[0].results[] | select(.properties.component == "SuppressedLeak")] as $s | ($s | length) == 1 and ($s[0].suppressions[0].kind == "inSource")