D5.3: broaden the BCL fresh-factory table (crypto Create() + File.OpenHandle)

docs/notes/d5-ownership-transfer.md

@@ -334,17 +334,22 @@ escape-without-transfer and all `unknown`/`may` lower to **silence** in the defa
   false positive). (Bridge branch-scope fix: Codex P2 on #116; loop exclusion Codex P1, hoist
   safety predicate + pool-kind preservation CodeRabbit on #120.)
 - **D5.3 — Tier B breadth.**
-  - **Producer side — `fresh` factories (shipped, first slice).** A curated
-    `_BCL_FRESH_FACTORIES` table in the OwnIR bridge (`ownir.py`) marks well-known BCL
-    factories whose return the caller owns (`File.OpenRead/OpenText/OpenWrite/Open/Create/
-    CreateText/AppendText`). A `call` to one binds a `fresh` result via the SAME `_callee_
-    returns_fresh` path the first-party T1 inference uses (now the single source of truth for
-    the leak pre-scan, branch-hoist safety, and lowering), so a leaked `var s =
-    File.OpenRead(p)` surfaces as OWN001 *at the factory call* — invisible before (no body to
-    infer from; see `corpus-benchmark.md`). Matched conservatively (Codex): ONLY the bare
-    `File.Method` or the fully-qualified `System.IO.File.Method` — a same-named factory in
-    another namespace (`MyCompany.File.OpenRead`) is **not** a match, so we never fabricate
-    ownership for a look-alike. A **first-party summary overrides** the table (`_callee_
+  - **Producer side — `fresh` factories (shipped).** A curated `_BCL_FRESH_BY_NS` table in the
+    OwnIR bridge (`ownir.py`), grouped by namespace, marks well-known BCL factories whose
+    return the caller owns: **System.IO** stream factories (`File.OpenRead/OpenText/OpenWrite/
+    Open/Create/CreateText/AppendText/OpenHandle`) and **System.Security.Cryptography**
+    algorithm factories (`SHA1/SHA256/SHA384/SHA512/MD5.Create`, `Aes/RSA/ECDsa.Create` — a
+    leaked `using var sha = SHA256.Create()` is a common real leak). A `call` to one binds a
+    `fresh` result via the SAME `_callee_returns_fresh` path the first-party T1 inference uses
+    (now the single source of truth for the leak pre-scan, branch-hoist safety, and lowering),
+    so a leaked `var s = File.OpenRead(p)` surfaces as OWN001 *at the factory call* — invisible
+    before (no body to infer from; see `corpus-benchmark.md`). Matched conservatively (Codex):
+    ONLY the bare `Type.Method` or the fully-qualified identity under that type's real namespace
+    (`System.IO.File.OpenRead`, `System.Security.Cryptography.SHA256.Create`) — a same-named
+    factory in another namespace (`MyCompany.File.OpenRead`, `MyCrypto.SHA256.Create`) is **not**
+    a match. Overload-ambiguous names are excluded (`new StreamReader(stream)` adopts its arg;
+    `Process.Start` is also an instance method returning `bool`), so we never fabricate ownership
+    for a look-alike. A **first-party summary overrides** the table (`_callee_
     returns_fresh` trusts a known body over Tier B), and a first-party **wrapper** that
     returns a factory result (`Make(){ return File.OpenRead(p) }`) is itself `fresh`, so a
     dropped `Make()` leaks too (the return skeleton propagates BCL freshness instead of

ownlang/ownir.py

@@ -836,6 +836,13 @@ def to_module(facts: dict[str, Any]) -> tuple[Module, dict[str, dict[str, Any]]]
             mos: dict[str, Any] = solve(_build_skeletons(raw_fns))
         except Exception:
             mos = {}
+        # every first-party method name (including overloaded ones dropped from `mos`): the
+        # Tier B BCL table must never fire for a callee we compiled from source — Tier A is
+        # authoritative even with no usable summary (CodeRabbit). Threaded into the freshness
+        # checks so direct calls agree with the wrapper-return path in `_infer_return_skeleton`.
+        first_party = frozenset(
+            _canonical_callee_name(str(fn.get("name", ""))) for fn in raw_fns
+            if isinstance(fn, dict) and fn.get("name"))
         for fn in raw_fns:
             if not isinstance(fn, dict):
                 continue
@@ -856,7 +863,7 @@ def to_module(facts: dict[str, Any]) -> tuple[Module, dict[str, dict[str, Any]]]
             # after the merge) must be declared once at the function's outer scope, or
             # the core rejects the post-merge reference as OWN030 (the bridge branch-
             # scope fix). Pre-declare them here, then skip their in-branch acquire.
-            hoist = _hoisted_branch_locals(nodes, mos)
+            hoist = _hoisted_branch_locals(nodes, mos, first_party)
             hoisted_lets: list[Stmt] = []
             for hname in sorted(hoist):
                 hh = f"loc_{loc[0]}"
@@ -869,7 +876,7 @@ def to_module(facts: dict[str, Any]) -> tuple[Module, dict[str, dict[str, Any]]]
                 hoisted_lets.append(Let(hh, Acquire("Disposable", [], hline), hline))
             fbody = [*hoisted_lets,
                      *_lower_flow(nodes, ffile, fname, handles, loc, localmap,
-                                  released, mos, set(hoist))]
+                                  released, mos, set(hoist), first_party)]
             # A body that returns a value gets an owned return type, so the core
             # models `return s` as a valid ESCAPE (the value is discharged to the
             # caller) instead of a void-return mismatch that would leave `s` looking
@@ -1009,7 +1016,8 @@ def visit(ns: Any) -> None:
     return out
 
 
-def _infer_return_skeleton(nodes: Any, param_names: set[str]) -> ReturnSkeleton:
+def _infer_return_skeleton(nodes: Any, param_names: set[str],
+                           first_party: frozenset[str] = frozenset()) -> ReturnSkeleton:
     """Infer a method's owned-return kind for the D5.0 solver (P-005 D5.2, T1).
 
     `fresh` — every `return <var>` path returns a local the body itself `acquire`d
@@ -1041,11 +1049,15 @@ def _infer_return_skeleton(nodes: Any, param_names: set[str]) -> ReturnSkeleton:
         (v,) = tuple(returned)
         callee = call_results.get(v)
         if callee and v not in param_names and v not in acquired:
-            if _is_bcl_fresh_factory(callee):
+            if _canonical_callee_name(callee) not in first_party \
+                    and _is_bcl_fresh_factory(callee):
                 # a thin wrapper returning a BCL factory's result is itself `fresh` — the
                 # caller owns it (Codex). Without this the return is a `forward` to an
                 # external (bodyless) callee, which the solver degrades to `unknown`, so a
                 # dropped `Make()` whose body is `return File.OpenRead(p)` leaks invisibly.
+                # Tier A overrides Tier B: if `callee` is a first-party method (it has its
+                # own summary, fresh or not), do NOT apply the bare BCL table — `forward` so
+                # the solver resolves the wrapper through that summary instead (Codex P2).
                 return ReturnSkeleton("fresh")
             return ReturnSkeleton("forward", callee=callee)
     return ReturnSkeleton()  # not provably owned -> no claim
@@ -1111,36 +1123,75 @@ def _infer_return_skeleton(nodes: Any, param_names: set[str]) -> ReturnSkeleton:
 # deliberately excluded — that is the sink / T4 case, not a pure factory. Keyed by
 # `Type.Method`; a callee matches on its last two dotted segments so a namespace-qualified
 # `System.IO.File.OpenRead` resolves the same.
-_BCL_FRESH_FACTORIES = frozenset({
-    "File.OpenRead", "File.OpenText", "File.OpenWrite",
-    "File.Open", "File.Create", "File.CreateText", "File.AppendText",
-})
-# the fully-qualified `System.IO.File.*` identities — accepted alongside the bare forms.
-_BCL_FRESH_FQNS = frozenset("System.IO." + e for e in _BCL_FRESH_FACTORIES)
+# The table, grouped by namespace so each entry's fully-qualified identity is exact. Only
+# UNAMBIGUOUS static factories whose return the caller owns are listed — overload-ambiguous
+# names are excluded on purpose (e.g. `new StreamReader(stream)` ADOPTS its arg; `Process.Start`
+# is a static owned-Process factory but ALSO an instance method returning `bool`, so a bare
+# match would fabricate ownership for `proc.Start()`). Crypto algorithm `Create()` factories are
+# the high-value addition: `using var sha = SHA256.Create()` not disposed is a common real leak.
+_BCL_FRESH_BY_NS = {
+    "System.IO": (
+        "File.OpenRead", "File.OpenText", "File.OpenWrite", "File.Open",
+        "File.Create", "File.CreateText", "File.AppendText", "File.OpenHandle",
+    ),
+    "System.Security.Cryptography": (
+        "SHA1.Create", "SHA256.Create", "SHA384.Create", "SHA512.Create", "MD5.Create",
+        "Aes.Create", "RSA.Create", "ECDsa.Create",
+    ),
+}
+_BCL_FRESH_FACTORIES = frozenset(e for es in _BCL_FRESH_BY_NS.values() for e in es)
+# the fully-qualified identities (each under its real namespace), accepted beside the bare forms.
+_BCL_FRESH_FQNS = frozenset(f"{ns}.{e}" for ns, es in _BCL_FRESH_BY_NS.items() for e in es)
+
+
+def _canonical_callee_name(name: str) -> str:
+    """The identity a callee/method name is matched on: its `global::`-stripped form. The
+    extractor may emit a fully-qualified call as `global::System.…` while the corresponding
+    method definition (and so `mos` / `first_party`) carries the bare form. Stripping the
+    qualifier on BOTH sides keeps a source-visible call from missing Tier A and falling through
+    to the Tier B BCL table (CodeRabbit)."""
+    return name.removeprefix("global::")
 
 
 def _is_bcl_fresh_factory(callee: str) -> bool:
     """True if `callee` names a curated BCL factory whose return the caller owns. Accepts
-    ONLY the bare `Type.Method` (`File.OpenRead`) or the fully-qualified `System.IO.File.*`
-    identity (with an optional `global::` qualifier) — a same-named type in another namespace
-    (`MyCompany.File.OpenRead`) is NOT a match. Precision-first: we never fabricate ownership
-    for a non-BCL look-alike (Codex / CodeRabbit)."""
+    ONLY the bare `Type.Method` (`File.OpenRead`, `SHA256.Create`) or its fully-qualified
+    identity under that type's real namespace (`System.IO.File.OpenRead`,
+    `System.Security.Cryptography.SHA256.Create`), with an optional `global::` qualifier — a
+    same-named type in ANOTHER namespace (`MyCrypto.SHA256.Create`) is NOT a match.
+    Precision-first: we never fabricate ownership for a non-BCL look-alike (Codex / CodeRabbit)."""
     if not callee:
         return False
-    name = callee.removeprefix("global::")
+    name = _canonical_callee_name(callee)
     return name in _BCL_FRESH_FACTORIES or name in _BCL_FRESH_FQNS
 
 
-def _callee_returns_fresh(callee: str, mos: dict[str, Any] | None) -> bool:
+def _callee_returns_fresh(callee: str, mos: dict[str, Any] | None,
+                          first_party: frozenset[str] = frozenset()) -> bool:
     """Whether a `call` to `callee` yields a fresh owned result the caller must release.
     A first-party summary is AUTHORITATIVE — if one exists we trust its `returns`, so a
     same-named first-party `File.OpenRead` (Tier A) overrides the BCL table (Tier B) and is
     never given a fabricated `fresh` (Codex). Only a callee we have no body for falls back to
     the curated BCL factory table. The single source of truth shared by the leak pre-scan,
-    the branch-hoist safety walk, and the flow lowering, so all three agree."""
-    summ = mos.get(callee) if (mos is not None and callee) else None
+    the branch-hoist safety walk, and the flow lowering, so all three agree.
+
+    `first_party` carries Tier A's reach beyond the summary set: a method we compiled from
+    source but DROPPED from `mos` (an overloaded name — see `_build_skeletons`) has no summary,
+    yet the BCL table still must not apply to it, else a dropped/overloaded source `SHA256.Create`
+    would look fresh on a direct call while a wrapper around it (which `_infer_return_skeleton`
+    already excludes) stays silent. Suppress the Tier B fallback for any first-party name so
+    direct and wrapper-returned paths agree (CodeRabbit). Precision-safe: no summary + first-party
+    => no claim (a dropped overload we cannot prove owns its result)."""
+    if not callee:
+        return False
+    identity = _canonical_callee_name(callee)
+    summ = mos.get(callee) if mos is not None else None
+    if summ is None and mos is not None and identity != callee:
+        summ = mos.get(identity)  # a `global::`-qualified call resolves to its bare summary
     if summ is not None:
         return getattr(summ, "returns", None) == "fresh"
+    if identity in first_party:
+        return False
     return _is_bcl_fresh_factory(callee)
 
 
@@ -1239,6 +1290,11 @@ def _build_skeletons(raw_fns: list[Any]) -> list[MethodSkeleton]:
     distinguishable — note's open question 2 — so a forward to such a name stays
     `unknown` → silent, the precision-safe choice)."""
     counts = Counter(str(fn.get("name", "")) for fn in raw_fns if isinstance(fn, dict))
+    # every first-party method name (even overloaded/dropped ones): the BCL fresh-factory
+    # table must NOT apply to a call whose target we compile from source — Tier A (the
+    # first-party summary) authoritatively overrides Tier B (the curated BCL table) even
+    # when the source method happens to share a BCL factory's name (Codex P2).
+    first_party = frozenset(_canonical_callee_name(k) for k in counts if k)
     skels: list[MethodSkeleton] = []
     for fn in raw_fns:
         if not isinstance(fn, dict):
@@ -1282,7 +1338,7 @@ def _build_skeletons(raw_fns: list[Any]) -> list[MethodSkeleton]:
                     paths = ()
             params.append(ParamSkeleton(i, cname, True, paths))
         pnames = {str(p.get("name", "")) for p in raw_params if isinstance(p, dict)}
-        ret = _infer_return_skeleton(body, pnames)
+        ret = _infer_return_skeleton(body, pnames, first_party)
         skels.append(MethodSkeleton(key, tuple(params), ret))
     return skels
 
@@ -1361,7 +1417,8 @@ def _lower_fn_params(fn: dict[str, Any], ffile: str, fname: str,
     return out
 
 
-def _branch_hoist_safe(nodes: Any, name: str, mos: dict[str, Any] | None) -> bool:
+def _branch_hoist_safe(nodes: Any, name: str, mos: dict[str, Any] | None,
+                       first_party: frozenset[str] = frozenset()) -> bool:
     """Whether hoisting `name` to an *unconditional* outer-scope acquire is leak-safe.
 
     Hoisting makes a conditional acquire unconditional, so it is only safe if no path
@@ -1376,7 +1433,7 @@ def acquires(n: dict[str, Any]) -> bool:
         if n.get("op") == "acquire" and str(n.get("var", "")) == name:
             return True
         if n.get("op") == "call" and str(n.get("result", "")) == name:
-            return _callee_returns_fresh(str(n.get("callee", "")), mos)
+            return _callee_returns_fresh(str(n.get("callee", "")), mos, first_party)
         return False
 
     def analyze(seq: Any, acquired: bool) -> tuple[bool, bool]:
@@ -1410,7 +1467,9 @@ def analyze(seq: Any, acquired: bool) -> tuple[bool, bool]:
 
 
 def _hoisted_branch_locals(nodes: Any,
-                           mos: dict[str, Any] | None) -> dict[str, tuple[int, bool]]:
+                           mos: dict[str, Any] | None,
+                           first_party: frozenset[str] = frozenset(),
+                           ) -> dict[str, tuple[int, bool]]:
     """Locals acquired INSIDE an `if`/`while` branch but referenced after the merge.
 
     The core resolver is strictly lexical: an `if` pushes a scope per branch and pops
@@ -1444,7 +1503,7 @@ def fresh_result(n: dict[str, Any]) -> str | None:
         callee, res = n.get("callee"), n.get("result")
         if not (isinstance(res, str) and res and isinstance(callee, str) and callee):
             return None
-        return res if _callee_returns_fresh(callee, mos) else None
+        return res if _callee_returns_fresh(callee, mos, first_party) else None
 
     def note_ref(name: str, depth: int) -> None:
         if name not in ref_depth or depth < ref_depth[name]:
@@ -1490,15 +1549,16 @@ def walk(ns: Any, depth: int, in_loop: bool) -> None:
     return {name: (acq_line[name], acq_pool.get(name, False))
             for name, d in acq_depth.items()
             if d >= 1 and ref_depth.get(name, -1) == 0 and name not in loop_acq
-            and _branch_hoist_safe(nodes, name, mos)}
+            and _branch_hoist_safe(nodes, name, mos, first_party)}
 
 
 def _lower_flow(nodes: list[Any], ffile: str, fname: str,
                 handles: dict[str, dict[str, Any]], loc: list[int],
                 localmap: dict[str, str],
                 released_vars: set[str],
                 mos: dict[str, Any] | None = None,
-                hoisted: set[str] | None = None) -> list[Stmt]:
+                hoisted: set[str] | None = None,
+                first_party: frozenset[str] = frozenset()) -> list[Stmt]:
     """Lower one OwnIR flow body (B0b/B2) into core statements. acquire/use/release/
     return reference a C# local by name (`var`); `if` carries `then`/`else`
     sub-bodies; `while` carries a `body` (a back-edge — the core's worklist fixpoint
@@ -1562,14 +1622,17 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str,
             tn = n.get("then", [])
             en = n.get("else", [])
             then_b = _lower_flow(tn if isinstance(tn, list) else [], ffile, fname,
-                                 handles, loc, localmap, released_vars, mos, hoisted)
+                                 handles, loc, localmap, released_vars, mos, hoisted,
+                                 first_party)
             else_b = _lower_flow(en if isinstance(en, list) else [], ffile, fname,
-                                 handles, loc, localmap, released_vars, mos, hoisted)
+                                 handles, loc, localmap, released_vars, mos, hoisted,
+                                 first_party)
             body.append(If("?", then_b, else_b, line))
         elif op == "while":
             bn = n.get("body", [])
             body_b = _lower_flow(bn if isinstance(bn, list) else [], ffile, fname,
-                                 handles, loc, localmap, released_vars, mos, hoisted)
+                                 handles, loc, localmap, released_vars, mos, hoisted,
+                                 first_party)
             body.append(While("?", body_b, line))
         elif op == "call":
             # A call to a CONTRACTED callee (a function/extern whose signature the
@@ -1608,7 +1671,7 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str,
             if isinstance(result, str) and result and result not in hoisted:
                 localmap.pop(result, None)
             if (isinstance(result, str) and result and result not in hoisted
-                    and _callee_returns_fresh(callee, mos)):
+                    and _callee_returns_fresh(callee, mos, first_party)):
                 handle = f"loc_{loc[0]}"
                 loc[0] += 1
                 localmap[result] = handle