From 571d7cee0553a530c1e1d65bc4e3aeb649b20c61 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 15:54:09 +0000 Subject: [PATCH] =?UTF-8?q?audit(xaml):=20join=20=E2=80=94=20anchor=20XAML?= =?UTF-8?q?203=20on=20code-behind=20to=20dedup=20with=20OWN001?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XAML203 previously anchored on the .xaml while own-check's OWN001 for the same unreleased subscription anchored on the .xaml.cs — different basenames, so the aggregator never clustered them and the one leak was DOUBLE-REPORTED (two P1 findings). Since XAML203 always derives from the same OwnIR released=false record own-check turns into OWN001 (no own-check facts -> no join), the honest value is a cross-source confirmation, not a second report. Anchor XAML203 at the code-behind subscription site (component.file : subscription.line) — where the matching -= goes anyway — so it lands at the same file+line as OWN001 and CLUSTERS with it into one high-confidence finding; the XAML view + the lifecycle handler that wired it ride in the message. One finding per leaking subscription (each pairs with its OWN001); a component with no file metadata falls back to the markup. - xaml_join: re-anchor + reshaped message; docstring corrected (it DOES cluster). - taxonomy: XAML203 comment corrected (clusters with OWN001 -> high confidence). - run_static selftest: stubbed own-check now emits the matching OWN001; asserts XAML203 + OWN001 collapse to exactly one high-confidence cluster (no double). - xaml_join selftest: code-behind anchoring (path/line/message) across all cases. - README + design note: corrected the anchoring claim and the join's honest value (confidence upgrade + view-lifecycle framing, not new detection). Selftests: xaml_join 13, run_static 22. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb --- audit/README.md | 4 +- audit/static/run_static.py | 20 ++++- audit/static/taxonomy/categories.yml | 9 ++- audit/static/tools/xaml_join.py | 108 +++++++++++++++------------ docs/notes/xaml-analyzer-design.md | 16 ++-- 5 files changed, 96 insertions(+), 61 deletions(-) diff --git a/audit/README.md b/audit/README.md index 13b42600..5a344f5b 100644 --- a/audit/README.md +++ b/audit/README.md @@ -144,7 +144,9 @@ python audit/static/run_static.py --selftest # full pipeline end-to-end on fix OwnIR facts own-check now persists (`--emit-facts` → `own-check.facts.json`) by the deterministic XAML naming convention (`x:Class`→type, handler→method) — **build-free, no `.g.cs`/build needed**. First rule **XAML203** (view subscribes from a load-lifecycle handler but the OwnIR verdict is - `released=false` → closed view retained), anchored at the XAML site and naming the C# subscription. + `released=false` → closed view retained), anchored at the code-behind subscription site so it + clusters with own-check's `OWN001` into one high-confidence finding (no double-report) and names the + XAML view that wired it. `run_static.py` runs the join whenever both fact sources are present and folds its SARIF into the pipeline. Binding-path-hotness rules (XAML200/204, need the DataContext type) and an optional `.g.cs` ground-truth cross-check are documented build-tier follow-ups. Phase 3 (runtime correlation) deferred. diff --git a/audit/static/run_static.py b/audit/static/run_static.py index be956c6c..2a63eff6 100755 --- a/audit/static/run_static.py +++ b/audit/static/run_static.py @@ -367,10 +367,19 @@ def _fake_own_check(target_, out_dir_, severity_, root=None): {"name": "CustomerView", "file": "Views/CustomerView.xaml.cs", "subscriptions": [ {"event": "_bus.Changed", "handler": "OnChanged", "line": 21, "released": False}]}]}), encoding="utf-8") + # own-check ALSO reports this leak as OWN001 on the code-behind: XAML203 anchors + # at the same .xaml.cs:21, so the two must CLUSTER into one high-confidence + # finding, not double-report. sarif = Path(out_dir_) / "own-check.sarif" - sarif.write_text('{"version":"2.1.0","runs":[{"results":[]}]}', encoding="utf-8") + sarif.write_text(json.dumps({"version": "2.1.0", "runs": [{ + "tool": {"driver": {"name": "Own.NET"}}, "results": [{"ruleId": "OWN001", + "level": "warning", "message": {"text": + "event subscribed but never unsubscribed [resource: subscription token]"}, + "locations": [{"physicalLocation": { + "artifactLocation": {"uri": "Views/CustomerView.xaml.cs"}, + "region": {"startLine": 21}}}]}]}]}), encoding="utf-8") return {"tool": "own-check", "tier": "build-free", "available": True, - "sarif": str(sarif), "facts": str(facts), "findings": 0, "reason": ""} + "sarif": str(sarif), "facts": str(facts), "findings": 1, "reason": ""} with tempfile.TemporaryDirectory() as td4: out4 = Path(td4) / "out" @@ -391,8 +400,11 @@ def _fake_own_check(target_, out_dir_, severity_, root=None): _self.run_own_check = real_own_check check(any(t["tool"] == "xaml-join" and t["available"] for t in res4["tiers"]), "xaml-join tier must run when this run produced both fact sources") - check(res4["totals"]["clusters"] >= 1, - "a XAML203 join finding must flow through to a scored cluster") + # XAML203 anchors on the code-behind, so it CLUSTERS with own-check's OWN001 + # into ONE high-confidence finding — no double-report of the same leak. + check(res4["totals"]["clusters"] == 1 and res4["totals"]["high_confidence"] == 1, + f"XAML203 + OWN001 must collapse to one high-confidence cluster, " + f"got {res4['totals']}") # The join must NOT run on stale facts: a re-run with own-check NOT a producer this # time (and no SDK) must skip the join even though own-check.facts.json is on disk. diff --git a/audit/static/taxonomy/categories.yml b/audit/static/taxonomy/categories.yml index 514da3aa..2b573d7a 100644 --- a/audit/static/taxonomy/categories.yml +++ b/audit/static/taxonomy/categories.yml @@ -37,10 +37,11 @@ rules: # (vm->App, SystemEvents) — NOT a # generic subscription-leak # XAML Phase-2 join: a view (x:Class) that subscribes from a load-lifecycle handler - # but never releases (OwnIR released=false). Categorized as a subscription leak so - # it scores as the P1 leak it is; the join already stitches the markup site to the - # C# subscription it names (the leak lives in the .xaml.cs, a different basename, so - # it does not spatially cluster with the own-check OWN001 on that file). + # but never releases (OwnIR released=false). Categorized as a subscription leak and + # anchored at the code-behind subscription site, so it lands at the same file+line as + # own-check's OWN001 and CLUSTERS with it into one high-confidence finding (two + # sources agree — Plan.md §3.5), tagging it as a view-lifecycle leak rather than + # double-reporting it on a separate .xaml line. "XAML203": {category: 2, name: xaml-subscription-leak} # ── Category 5: INPC correctness ───────────────────────────────────────────── diff --git a/audit/static/tools/xaml_join.py b/audit/static/tools/xaml_join.py index d3e26ed8..98a36edf 100644 --- a/audit/static/tools/xaml_join.py +++ b/audit/static/tools/xaml_join.py @@ -33,14 +33,16 @@ lifetime owner that subscribed but never releases, so a closed view is retained. ``released: false`` is authoritative — the engine already checked for a matching ``-=`` anywhere in the class (code-behind included) — so no XAML ``Unloaded`` - heuristic is needed. The finding is anchored at the XAML lifecycle attribute (where - a developer adds the cleanup) and names the C# subscription site, stitching markup - and code into one finding. It is mapped to category 2 (subscription leak) so it is - classified and scored as the P1 leak it is. (It does not auto-cluster with the - own-check ``OWN001`` on the ``.xaml.cs``: clustering keys on basename and the leak - lives in a different file — the join *is* the cross-source link here, carried in - the message rather than by spatial agreement.) Phase 3 promotes it to a measured - retention path via the heap walker. + heuristic is needed. The finding is anchored at the **code-behind subscription + site** (where the matching ``-=`` goes), with the XAML view + the lifecycle handler + that wired it named in the message. It is mapped to category 2 (subscription leak), + so it lands at the same file+line as own-check's ``OWN001`` and **clusters with it + into one high-confidence finding** — two independent sources agreeing on the leak + (the static→static agreement of Plan.md §3.5) — instead of double-reporting it on a + separate ``.xaml`` line. The join's value is exactly this: it upgrades the + own-check subscription finding to high-confidence and tags it as a view-lifecycle + (closed-view-retained) leak, not a second copy of it. Phase 3 promotes it to a + measured retention path via the heap walker. Binding-path-hotness rules (XAML200/204) need the DataContext type, which markup rarely declares statically; they are a later increment, deliberately not guessed @@ -142,7 +144,15 @@ def _unreleased(component: dict[str, Any]) -> list[dict[str, Any]]: def join(xaml_facts: dict[str, Any], ownir: dict[str, Any]) -> list[dict[str, Any]]: """Produce XAML203 link findings from the two fact sources. Each finding is a - plain dict ``{rule, path, line, message, resource}`` ready for SARIF.""" + plain dict ``{rule, path, line, message, resource}`` ready for SARIF. + + One finding **per unreleased subscription**, anchored at the **code-behind + subscription site** (``component.file : subscription.line``) — the same spot + own-check reports OWN001, and where the matching ``-=`` actually goes. That makes + the join cluster with own-check into one **high-confidence** finding (two sources + agree) instead of double-reporting the same leak on a separate ``.xaml`` line; the + XAML view + the lifecycle handler that wired it ride in the message. (Only a + component with no file metadata falls back to anchoring on the XAML itself.)""" by_name = _component_index(ownir) out: list[dict[str, Any]] = [] @@ -154,32 +164,33 @@ def join(xaml_facts: dict[str, Any], ownir: dict[str, Any]) -> list[dict[str, An if not components: continue # x:Class resolves to no (unambiguous) OwnIR component - # All unreleased subscriptions across the matched component(s). - leaks = [s for c in components for s in _unreleased(c)] - if not leaks: - continue - load_handlers = [h for h in (doc.get("event_handlers") or []) if h.get("event") in LOAD_LIFECYCLE_EVENTS] if not load_handlers: continue # the leak exists but is not wired from a view-lifecycle handler - # Anchor at the first load-lifecycle handler (where the dev adds cleanup). anchor = min(load_handlers, key=lambda h: h.get("line") or 0) - site = ", ".join( - f"{s.get('event', '?')} ({c.get('file', '?')}:{s.get('line', '?')})" - for c in components for s in _unreleased(c)) - out.append({ - "rule": "XAML203", - "path": doc.get("file", "?"), - "line": anchor.get("line") or 0, - "message": ( - f"view {str(x_class).rsplit('.', 1)[-1]} wires {anchor['event']}=" - f"{anchor.get('handler', '?')} and subscribes without releasing " - f"[{site}]; a closed view is retained — add the matching unsubscribe " - "[resource: subscription token]"), - "resource": "subscription token", - }) + view = str(x_class).rsplit(".", 1)[-1] + xaml_file = doc.get("file", "?") + wired = f"{anchor['event']}={anchor.get('handler', '?')}" + for c in components: + cb = c.get("file") + for s in _unreleased(c): + ev = s.get("event", "?") + if cb: # anchor on the code-behind + path, line = cb, (s.get("line") or 0) + else: # no file -> fall back to markup + path, line = xaml_file, (anchor.get("line") or 0) + out.append({ + "rule": "XAML203", + "path": path, + "line": line, + "message": ( + f"view {view} ({xaml_file}, {wired}) subscribes to {ev} " + "without releasing — a closed view is retained; add the " + "matching unsubscribe [resource: subscription token]"), + "resource": "subscription token", + }) return out @@ -312,31 +323,34 @@ def check(ok: bool, msg: str) -> None: ]} findings = join(xaml_facts, ownir) + # One finding per leaking subscription, ANCHORED ON THE CODE-BEHIND (.xaml.cs) so it + # clusters with own-check's OWN001 — keyed here by that path. by_path = {f["path"]: f for f in findings} check(len(findings) == 3, f"expected exactly 3 XAML203, got {len(findings)}: {findings}") - check("Views/CaptureView.xaml" in by_path, + check("Views/CaptureView.xaml.cs" in by_path, "a 'capture' (region-escape) subscription must be a XAML203") - f = by_path.get("Views/CustomerView.xaml") - check(f is not None and f["rule"] == "XAML203" and f["line"] == 4, - f"XAML203 must anchor at the Loaded handler line (4): {f}") + f = by_path.get("Views/CustomerView.xaml.cs") + check(f is not None and f["rule"] == "XAML203" and f["line"] == 21, + f"XAML203 must anchor at the code-behind subscription line (21): {f}") check(f is not None and "_bus.Changed" in f["message"] - and "CustomerView.xaml.cs:21" in f["message"], - f"XAML203 message must name the C# subscription site: {f}") + and "CustomerView" in f["message"] and "Views/CustomerView.xaml" in f["message"] + and "Loaded=OnLoaded" in f["message"], + f"XAML203 message must name the view, its XAML, the handler and the event: {f}") check(f is not None and f["resource"] == "subscription token", "XAML203 must carry the subscription-token resource tag (cat-2 mapping)") - check("Views/CleanView.xaml" not in by_path, + check("Views/CleanView.xaml.cs" not in by_path, "a released subscription must NOT produce a finding") - check("Views/NoLifecycle.xaml" not in by_path, + check("Views/NoLifecycle.xaml.cs" not in by_path, "a leak with no view-lifecycle handler must NOT be XAML203 (own-check covers it)") - check("Views/TimerView.xaml" not in by_path, + check("Views/TimerView.xaml.cs" not in by_path, "an unreleased TIMER (not an event subscription) must NOT be a cat-2 XAML203") - # same-name disambiguation: the Sales leak lands on Sales's XAML, not Admin's. - check("Views/Admin/OrderView.xaml" not in by_path, - "a leak in Sales.OrderView must NOT cross-link to Admin.OrderView's XAML") - so = by_path.get("Views/Sales/OrderView.xaml") - check(so is not None and "Sales/OrderView.xaml.cs:21" in so["message"], - f"XAML203 must land on Sales's XAML and name Sales's code-behind: {so}") + # same-name disambiguation: the Sales leak lands on Sales's code-behind, not Admin's. + check("Views/Admin/OrderView.xaml.cs" not in by_path, + "a leak in Sales.OrderView must NOT cross-link to Admin.OrderView") + so = by_path.get("Views/Sales/OrderView.xaml.cs") + check(so is not None and "Views/Sales/OrderView.xaml" in so["message"], + f"XAML203 must anchor on Sales's code-behind and name Sales's view: {so}") # x:Class that resolves to no component -> nothing (not analysed / no leak). unknown_doc = {"documents": [{"file": "x.xaml", "x_class": "App.Unknown", @@ -362,10 +376,10 @@ def check(ok: bool, msg: str) -> None: try: from oracle_compare import parse_sarif parsed = parse_sarif(json.dumps(sarif), "xaml-join", []) - cust = next((p for p in parsed if p.path.endswith("CustomerView.xaml")), None) + cust = next((p for p in parsed if p.path.endswith("CustomerView.xaml.cs")), None) check(len(parsed) == len(findings) and cust is not None - and cust.rule == "XAML203" and cust.line == 4, - "SARIF round-trip must preserve every XAML203 with its anchor line (4)") + and cust.rule == "XAML203" and cust.line == 21, + "SARIF round-trip must preserve every XAML203 at its code-behind line (21)") except ImportError: check(False, "could not import scripts/oracle_compare.parse_sarif for round-trip") diff --git a/docs/notes/xaml-analyzer-design.md b/docs/notes/xaml-analyzer-design.md index 45055f8b..03a73733 100644 --- a/docs/notes/xaml-analyzer-design.md +++ b/docs/notes/xaml-analyzer-design.md @@ -220,11 +220,17 @@ even further down: a post-build binary, only for source-less audits — not on t **First slice built.** `audit/static/tools/xaml_join.py` implements **XAML203**: a view whose `x:Class` component has an OwnIR subscription the engine flagged `released=false`, wired from a -load-lifecycle handler (`Loaded`/`Initialized`/`DataContextChanged`) — the closed-view-retained leak, -anchored at the XAML site and naming the C# subscription. own-check persists its OwnIR facts via a new -`--emit-facts`, and `run_static.py` runs the join whenever `xaml-facts.json` + `own-check.facts.json` -are both present, folding the result into the pipeline. Binding-path-hotness (XAML200/204) needs the -DataContext type — rarely static in markup — so it is the next increment, deliberately not guessed. +load-lifecycle handler (`Loaded`/`Initialized`/`DataContextChanged`) — the closed-view-retained leak. +It is anchored at the **code-behind subscription site** (where the matching `-=` goes), so it lands at +the same file+line as own-check's `OWN001` and **clusters with it into one high-confidence finding** +rather than double-reporting the same leak on a separate `.xaml` line; the XAML view that wired it +rides in the message. The join's honest value here is *not* new detection (own-check already finds the +subscription leak in the `.cs`) but the cross-source **confidence upgrade + view-lifecycle framing** — +where the join finds what neither source can alone (binding→`List` getter, allocating converter on a +hot binding) it needs the DataContext type and OwnIR getter/converter facts, which is the XAML200/204 +increment, deliberately not guessed here. own-check persists its OwnIR facts via a new `--emit-facts`, +and `run_static.py` runs the join whenever both fact sources are produced *this run*, folding the +result into the pipeline. **The Phase-1 → Phase-2 seam is built.** The markup pass now emits a structured fact document alongside its SARIF — `audit/static/tools/xaml_facts.py` writes `xaml-facts.json` from the *same*