From 0ae0b5868be373122ab6a77a921c26288cad0f11 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 02:22:37 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(#146):=20returned-fresh=20publisher=20?= =?UTF-8?q?provenance=20=E2=80=94=20prove=20the=20Create->Apply=20shape=20?= =?UTF-8?q?bounded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dual of ownership transfer, interprocedural: a += on a PARAMETER publisher is locally indistinguishable from a DI-singleton bus, so it honestly warns (OWN001, injected tier). The extractor now runs a lazy, memoized, compilation-wide provenance pass: when the subscribing method is private/internal and EVERY visible caller passes a freshly-constructed local that escapes only into the call / its own return (and the callee never lets the param escape), the subscription is stamped source_provenance: "returned_fresh" and the bridge drops it — bounded by the returned publisher's lifetime, same boundedness as a locally-constructed source. Instance-level provenance deliberately beats the type-level DI hop. Precision-first denials (all keep the warning, pinned by ReturnedPublisherSample.cs in CI + test_ownir.py): public candidate, method-group reference, named/ref/omitted argument, non-fresh/non-local argument, any other use of the caller's local (field store, other callee, lambda capture, reassignment), param->param forwarding, zero visible callers. Constructors are excluded by construction (MethodKind gate), so the ctor-injected DI shape can never be silenced. Clears the mined Newtonsoft JsonSerializer.Create over-report (field-notes #8); its oracle-fp-baseline entry is REMOVED so a regression resurfaces in triage instead of being swallowed by the allowlist. Spec'd in OwnIR.md §4 + the JSON schema (additive optional field, no version bump per IR rules). Closes #146 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 18 ++ corpus/oracle-fp-baseline.txt | 7 +- docs/notes/field-notes-patterns.md | 12 + docs/notes/oracle-known-fps.md | 17 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 235 ++++++++++++++++-- .../roslyn/samples/ReturnedPublisherSample.cs | 130 ++++++++++ ownlang/ownir.py | 32 +++ spec/OwnIR.md | 10 + spec/ownir.schema.json | 4 + tests/test_ownir.py | 71 ++++++ 10 files changed, 509 insertions(+), 27 deletions(-) create mode 100644 frontend/roslyn/samples/ReturnedPublisherSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91c72c29..0cce1371 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -203,6 +203,7 @@ jobs: frontend/roslyn/samples/CloseReleaseSample.cs \ frontend/roslyn/samples/SemaphoreFieldSample.cs \ frontend/roslyn/samples/VoidSubscribeSample.cs \ + frontend/roslyn/samples/ReturnedPublisherSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -235,6 +236,23 @@ jobs: || { echo "FAIL: expected the lambda-handler subscription leak (warning)"; exit 1; } echo "$out" | grep -q "inline lambda it has no '-=' handle" \ || { echo "FAIL: expected the lambda no-handle wording"; exit 1; } + # #146 interprocedural publisher provenance (the Newtonsoft + # Create->ApplySerializerSettings shape): every caller of ApplyBounded + # constructs the publisher and returns it, so the param-publisher + # subscription is bounded -> the extractor stamps + # `source_provenance: "returned_fresh"` and the bridge drops it (SILENT). + grep -q '"source_provenance": "returned_fresh"' "$RUNNER_TEMP/facts.json" \ + || { echo "FAIL: expected the returned_fresh provenance stamp in the facts"; exit 1; } + if echo "$out" | grep -q "publisher.Error"; then + echo "FAIL: the proven returned-fresh publisher subscription must be silent"; exit 1 + fi + # ...and every denial case KEEPS the honest OWN001 warning — public + # candidate, mixed callers, field-stored fresh local, and the + # param->param DI dual this feature must never silence. + for ev in "pub.Faulted" "target.Mixed" "stored.Stored" "bus.Changed"; do + echo "$out" | grep -qE "ReturnedPublisherSample\.cs:[0-9]+: warning: \[OWN001\].*'$ev'" \ + || { echo "FAIL: expected the OWN001 warning to survive for '$ev' (provenance must deny)"; exit 1; } + done # P-004 provenance: a local that ALIASES an injected source (var src = # _bus) is NOT method-bounded — it must warn, not be silently dropped. A # local the scope CONSTRUCTS (var owned = new Calc()) IS bounded -> silent. diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index 7c964838..a368d242 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -59,7 +59,12 @@ protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product # INSTANCE facts (no ArrayPool set + the sink is a StringWriter), not a type-level no-op, so it stays # baselined — consistent with excluding writers from IsNoOpDisposeWrapper. See no-op-dispose-wrapper.md. JamesNK/Newtonsoft.Json | TraceJsonReader.cs | OWN001 | _textWriter | benign-by-instance, NOT a no-op type: JsonTextWriter.Close() returns a (possibly pooled) write buffer and auto-completes JSON tokens; harmless here only because no ArrayPool is set and the sink is a StringWriter — kept baselined (not soundly auto-fixable) -JamesNK/Newtonsoft.Json | JsonSerializer.cs | OWN001 | serializer.Error | intra-call self-subscription: the serializer is freshly created from the same JsonSerializerSettings whose .Error handler it subscribes; source and handler are co-lifetimed +# REMOVED (fixed by #146): `JsonSerializer.cs | OWN001 | serializer.Error` — the +# returned-fresh publisher provenance pass now proves the Create-> +# ApplySerializerSettings shape bounded (source_provenance: "returned_fresh") and +# the finding no longer fires. Deliberately NOT left baselined: if the provenance +# pass regresses, the finding must reappear in the triage queue, not be silently +# swallowed by this allowlist. (field-notes #8) # --- JoshClose/CsvHelper -------------------------------------------------------- JoshClose/CsvHelper | ConsoleHost.cs | OWN014 | AppDomain.CurrentDomain.ProcessExit | non-product (docs-src/ doc-generator) + process-lived subscriber: ConsoleHost itself lives for the whole process, so promoting it to a process-lived event source changes nothing diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index 27998a2f..8af218b6 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -257,6 +257,18 @@ interprocedural (the construct-and-return is in the caller), which is the hard p The honest interim posture — advisory warning, never a hard error — is already in place.** +**Status: FIXED (#146).** The extractor now runs a compilation-wide provenance +pass: when a `+=` publisher is a parameter of a private/internal method and +*every* visible caller passes a freshly-constructed local that escapes only into +the call / its own `return` (and the callee never lets the param escape), the +subscription is stamped `source_provenance: "returned_fresh"` and the bridge +drops it (bounded, silent). Any unprovable step — public candidate, method-group +reference, mixed callers, field-stored local, param→param forwarding — denies +the proof and the honest warning stands. Pinned by +`frontend/roslyn/samples/ReturnedPublisherSample.cs` (CI `wpf-extractor`) and +the `source_provenance` checks in `tests/test_ownir.py`; spec'd in +`spec/OwnIR.md` §4. + ## 9. Owning field whose IDisposable holds no unmanaged resource **Seen in:** Newtonsoft.Json `Src/Newtonsoft.Json/Serialization/TraceJsonReader.cs:37,38` diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index dd873661..4eea62b6 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -44,10 +44,17 @@ XsltMessageEncountered` — a `this`-capturing handler subscribed to an event on **fixed at the source** by `PropertyReturnsOwnedMember` (the self-owned-source exemption now covers a property receiver, not just `this`/fields/locals). A live protobuf re-run confirmed it: own-only **0**, the finding absent from own-only and baselined. See -root-cause #3. Corpus fixture: `subscription-self-owned-property`. Newtonsoft's -`serializer.Error` stays baselined — its source escapes (a returned `Create()` result) -and the handler is a parameter's delegate, so proving non-leak needs lifetime modelling; -the "may outlive" warning is honest, not a clear FP. +root-cause #3. Corpus fixture: `subscription-self-owned-property`. + +**Update (#146 landed).** Newtonsoft's `serializer.Error` is now **fixed at the +source** too: the extractor's compilation-wide returned-fresh publisher provenance +pass proves the `Create` → `ApplySerializerSettings` shape bounded +(`source_provenance: "returned_fresh"`) and the bridge drops it. Its baseline +entry is removed (deliberately — a regression must reappear in triage, not be +swallowed by the allowlist). Pinned by +`frontend/roslyn/samples/ReturnedPublisherSample.cs`; denial cases (public +candidate, mixed callers, field-stored local, param→param DI forwarding) keep the +honest warning. **Update (extractor fix landed — all NLog timers).** All 5 of the original NLog `WaitForDispose` timer FPs are now **fixed at the source**, baseline entries deleted. @@ -114,7 +121,7 @@ visible. | location | verdict | why | |---|---|---| | `TraceJsonReader.cs` `_textWriter` | **FP → baseline** | no-op dispose: a `JsonTextWriter` over an in-memory `StringWriter`/`StringBuilder` holds no unmanaged resource | -| `JsonSerializer.cs` `serializer.Error` | **FP → baseline** | intra-call self-subscription: the serializer is freshly built from the same `JsonSerializerSettings` whose `.Error` it subscribes; co-lifetimed | +| `JsonSerializer.cs` `serializer.Error` | **FP → fixed (#146)** | intra-call self-subscription: the serializer is freshly built from the same `JsonSerializerSettings` whose `.Error` it subscribes; co-lifetimed. Now proven bounded by the returned-fresh publisher provenance pass; baseline entry removed | ### JoshClose/CsvHelper — 2 findings diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index c625e621..8b84161b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -789,6 +789,165 @@ static string SubscriptionSourceKind(ExpressionSyntax left, IEventSymbol ev, static bool IsLambdaHandler(ExpressionSyntax right) => right is AnonymousFunctionExpressionSyntax; +// --- #146: interprocedural "constructed-and-returned" publisher provenance --- +// +// A `+=` whose publisher is a PARAMETER is tiered `injected` by +// SubscriptionSourceKind — honest locally, because inside the method a DI +// singleton bus (a real leak) and a caller-owned fresh publisher (bounded) are +// syntactically identical. This pass proves the bounded case ACROSS the +// compilation — the dual of ownership transfer (D5): if EVERY visible caller +// passes a publisher it freshly constructs and lets escape only into this call +// or its own `return`, and the callee itself never lets the parameter escape, +// then the handler lives exactly as long as the object the caller now holds — +// bounded, not a leak (the bridge drops the stamped fact; ownlang/ownir.py +// `source_provenance`). Precision-first: ANY unprovable step — public/protected +// candidate, method-group reference, named/ref/omitted argument, non-local or +// non-fresh argument, any other use of the caller's local, lambda capture, zero +// visible callers — denies the proof and the honest OWN001 warning stands. +// (Mined shape: Newtonsoft JsonSerializer.Create -> ApplySerializerSettings; +// field-notes #7.) + +static bool IsReturnedFreshParam(IParameterSymbol p, CSharpCompilation compilation, + Dictionary cache) +{ + if (cache.TryGetValue(p, out var hit)) return hit; + var ok = ComputeReturnedFreshParam(p, compilation); + cache[p] = ok; + return ok; +} + +static bool ComputeReturnedFreshParam(IParameterSymbol p, CSharpCompilation compilation) +{ + // Callee gate: only an in-compilation-callable method can have ALL its + // callers audited. `public`/`protected` (incl. `protected internal`) may be + // called from outside this compilation -> deny; `internal` is auditable + // here (this scan IS the assembly view). + if (p.RefKind != RefKind.None || p.IsParams) return false; + if (p.ContainingSymbol is not IMethodSymbol m) return false; + if (m.MethodKind is not (MethodKind.Ordinary or MethodKind.LocalFunction)) return false; + if (m.DeclaredAccessibility is not (Accessibility.Private or Accessibility.Internal + or Accessibility.ProtectedAndInternal)) return false; + // The callee must never let the publisher escape: every reference to the + // parameter must be a plain member-access RECEIVER (`p.Event += h`, + // `p.Prop = v`, `p.Method()`), outside any lambda (a capturing lambda can + // outlive the call). A bare `p` — as an argument, an assignment side, a + // return value, a conditional access — denies. + foreach (var declRef in m.DeclaringSyntaxReferences) + { + var mnode = declRef.GetSyntax(); + var mModel = compilation.GetSemanticModel(mnode.SyntaxTree); + foreach (var id in mnode.DescendantNodes().OfType()) + { + if (id.Identifier.Text != p.Name) continue; + if (mModel.GetSymbolInfo(id).Symbol is not IParameterSymbol rp + || !SymbolEqualityComparer.Default.Equals(rp, p)) continue; + if (id.Parent is not MemberAccessExpressionSyntax pma || pma.Expression != id) + return false; + if (id.Ancestors().OfType().Any()) + return false; + } + } + // Caller gate: every visible reference to the method must be a direct call + // whose argument for THIS parameter is a bounded fresh local. A method-group + // reference (delegate conversion) hides call paths -> deny. Zero visible + // calls (reflection-only / dead code) -> deny. (A `nameof` reference binds + // to no single symbol and creates no call path, so it is skipped, not + // denied.) + var target = m.OriginalDefinition; + var sawCall = false; + foreach (var tree in compilation.SyntaxTrees) + { + // name-prefilter: bind only trees/nodes that even mention the name. + List? hits = null; + foreach (var n in tree.GetRoot().DescendantNodes().OfType()) + if (n.Identifier.Text == m.Name) + (hits ??= new List()).Add(n); + if (hits is null) continue; + var tModel = compilation.GetSemanticModel(tree); + foreach (var id in hits) + { + if (tModel.GetSymbolInfo(id).Symbol is not IMethodSymbol called + || !SymbolEqualityComparer.Default.Equals(called.OriginalDefinition, target)) + continue; + var expr = id.Parent is MemberAccessExpressionSyntax ma && ma.Name == id + ? (ExpressionSyntax)ma : (ExpressionSyntax)id; + if (expr.Parent is not InvocationExpressionSyntax inv || inv.Expression != expr) + return false; + sawCall = true; + if (!CallPassesBoundedFreshLocal(inv, p.Ordinal, target, tModel)) + return false; + } + } + return sawCall; +} + +static bool CallPassesBoundedFreshLocal(InvocationExpressionSyntax inv, int ordinal, + IMethodSymbol target, SemanticModel model) +{ + var arg = ArgumentForOrdinal(inv, ordinal, target); + if (arg is null || !arg.RefKindKeyword.IsKind(SyntaxKind.None)) return false; + // the argument must be a LOCAL, freshly constructed at its declarator + // (`var x = new T()` / `Publisher x = new()`); a field, parameter, property + // or call result may alias a long-lived publisher -> deny. + if (arg.Expression is not IdentifierNameSyntax lid + || model.GetSymbolInfo(lid).Symbol is not ILocalSymbol local) + return false; + var decl = local.DeclaringSyntaxReferences + .Select(r => r.GetSyntax()).OfType().FirstOrDefault(); + if (decl?.Initializer?.Value is not (ObjectCreationExpressionSyntax + or ImplicitObjectCreationExpressionSyntax)) + return false; + // the local's scope: the enclosing method/accessor/local-function body. + SyntaxNode? scope = decl; + while (scope is not null + && scope is not BaseMethodDeclarationSyntax + && scope is not AccessorDeclarationSyntax + && scope is not LocalFunctionStatementSyntax + && scope is not AnonymousFunctionExpressionSyntax) + scope = scope.Parent; + if (scope is null) return false; + // every use of the local must be an argument feeding THIS parameter of THIS + // method, or a plain `return local;`. Anything else — a field/property + // store, another callee, a lambda capture, a reassignment, `ref`/`out` — + // denies. (The declarator identifier is a token, not an IdentifierName, so + // the declaration itself is not walked.) + foreach (var use in scope.DescendantNodes().OfType()) + { + if (use.Identifier.Text != local.Name) continue; + if (model.GetSymbolInfo(use).Symbol is not ILocalSymbol us + || !SymbolEqualityComparer.Default.Equals(us, local)) continue; + if (use.Ancestors().TakeWhile(a => a != scope) + .OfType().Any()) + return false; + if (use.Parent is ReturnStatementSyntax) continue; + if (use.Parent is ArgumentSyntax ua && ua.RefKindKeyword.IsKind(SyntaxKind.None) + && ua.Parent?.Parent is InvocationExpressionSyntax uinv + && model.GetSymbolInfo(uinv).Symbol is IMethodSymbol ucalled + && SymbolEqualityComparer.Default.Equals(ucalled.OriginalDefinition, target) + && ArgumentForOrdinal(uinv, ordinal, target) == ua) + continue; + return false; + } + return true; +} + +static ArgumentSyntax? ArgumentForOrdinal(InvocationExpressionSyntax inv, int ordinal, + IMethodSymbol target) +{ + var args = inv.ArgumentList.Arguments; + for (int i = 0; i < args.Count; i++) + { + var a = args[i]; + if (a.NameColon is { } nc) + { + if (nc.Name.Identifier.Text == target.Parameters[ordinal].Name) return a; + } + else if (i == ordinal) + return a; + } + return null; // omitted (optional) / params-collapsed -> unprovable +} + // P-004 source-lifetime tier for an ignored `.Subscribe()` chain (WPF004). A // self-rooted `this.WhenAnyValue(p => p.SelfProp)..Subscribe` // watches the component's OWN property: the observable, its handler and `this` @@ -3383,6 +3542,12 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) "own", parsed.Select(p => p.tree), references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); +// #146 — memo for the interprocedural "constructed-and-returned" publisher +// provenance (computed lazily, only for param-publisher subscriptions; the +// compilation-wide caller walk is name-prefiltered, so a repo with no such +// subscriptions pays nothing). +var returnedFreshCache = new Dictionary(SymbolEqualityComparer.Default); + // --flow-locals coverage counters (--stats). A method "with a local" here is one // that has a non-escaping `new` IDisposable worth tracking; of those we either // flow-analyse it or honestly skip it (an unmodelled for/do/try/switch/async made @@ -3552,27 +3717,55 @@ or ImplicitObjectCreationExpressionSyntax continue; var released = unsub.Contains($"{a.Left}|{NormalizeHandler(a.Right)}") || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv)); - subs.Add(new - { - @event = a.Left.ToString(), - handler = a.Right.ToString(), - line = LineOf(a.Left), - released, - // A static-source subscription (a process-lived event, or a - // static-field/property receiver) is a region escape, not a - // token leak: route it through the lifetime engine as a - // `capture` -> OWN014 (the WPF "escape to App"). The bridge - // skips a released capture (a `-=` on close), so a correctly - // unsubscribed static subscription stays silent. An injected/ - // unknown source stays a token `subscription` (OWN001, - // severity-tiered); timers are their own kind. (P-004 WPF005; - // see ownlang/ownir.py `capture`.) - resource = isTimer ? "timer" - : source == "static" ? "capture" - : "subscription", - source, - lambda = !isTimer && IsLambdaHandler(a.Right), - }); + // #146 — interprocedural publisher provenance: a `+=` on a PARAMETER + // publisher stays `injected` (locally honest — param→param syntax is + // identical for a DI singleton bus, a real leak, and for the + // Newtonsoft Create→ApplySerializerSettings shape, which is bounded). + // The compilation-wide check can prove the LATTER: every visible + // caller passes a freshly-constructed local that escapes only into + // this call / its own `return`, and the callee never lets the param + // escape — then the subscription dies with the returned publisher + // and the bridge drops it (ownlang/ownir.py `source_provenance`). + // Any uncertainty leaves the fact unstamped and the OWN001 warning + // stands. + var returnedFresh = !isTimer && source == "injected" + && a.Left is MemberAccessExpressionSyntax provRecv + && model.GetSymbolInfo(provRecv.Expression).Symbol is IParameterSymbol provParam + && IsReturnedFreshParam(provParam, compilation, returnedFreshCache); + // A static-source subscription (a process-lived event, or a + // static-field/property receiver) is a region escape, not a + // token leak: route it through the lifetime engine as a + // `capture` -> OWN014 (the WPF "escape to App"). The bridge + // skips a released capture (a `-=` on close), so a correctly + // unsubscribed static subscription stays silent. An injected/ + // unknown source stays a token `subscription` (OWN001, + // severity-tiered); timers are their own kind. (P-004 WPF005; + // see ownlang/ownir.py `capture`.) + if (returnedFresh) + subs.Add(new + { + @event = a.Left.ToString(), + handler = a.Right.ToString(), + line = LineOf(a.Left), + released, + resource = "subscription", + source, + lambda = IsLambdaHandler(a.Right), + source_provenance = "returned_fresh", + }); + else + subs.Add(new + { + @event = a.Left.ToString(), + handler = a.Right.ToString(), + line = LineOf(a.Left), + released, + resource = isTimer ? "timer" + : source == "static" ? "capture" + : "subscription", + source, + lambda = !isTimer && IsLambdaHandler(a.Right), + }); } else if (leftSymbol is null && IsHandler(a.Right)) { diff --git a/frontend/roslyn/samples/ReturnedPublisherSample.cs b/frontend/roslyn/samples/ReturnedPublisherSample.cs new file mode 100644 index 00000000..590753e9 --- /dev/null +++ b/frontend/roslyn/samples/ReturnedPublisherSample.cs @@ -0,0 +1,130 @@ +using System; + +// #146 — interprocedural "constructed-and-returned" publisher provenance. +// +// A `+=` on a PARAMETER publisher is tiered `injected` (unknown lifetime -> +// OWN001 warning) because inside the method a DI singleton bus and a +// caller-owned fresh publisher look identical. The compilation-wide provenance +// pass proves the bounded case: when EVERY visible caller passes a publisher it +// freshly constructs and lets escape only into the call / its own `return`, the +// handler dies with the returned publisher -> `source_provenance: +// "returned_fresh"` -> the bridge drops it (SILENT). Mined shape: Newtonsoft +// `JsonSerializer.Create` -> `ApplySerializerSettings` (field-notes #7). +// +// Every OTHER class in this file is a deliberate denial case and must KEEP the +// honest OWN001 warning: same param->param syntax, but the proof fails. + +public class ProvPublisher +{ + public event EventHandler? Error; // bounded case (silent) + public event EventHandler? Faulted; // public candidate (warning) + public event EventHandler? Mixed; // mixed callers (warning) + public event EventHandler? Stored; // field-stored fresh local (warning) +} + +public class ProvSettings +{ + public EventHandler? Error; +} + +public class ProvBus +{ + public event EventHandler? Changed; // param->param DI shape (warning) +} + +// BOUNDED (silent): the Newtonsoft shape. `Create` constructs the publisher, +// hands it to a private helper that subscribes, and returns it — the handler +// lives exactly as long as the serializer the caller now holds. +public static class ProvFactory +{ + public static ProvPublisher Create(ProvSettings? settings) + { + var publisher = new ProvPublisher(); + if (settings != null) ApplyBounded(publisher, settings); + return publisher; + } + + private static void ApplyBounded(ProvPublisher publisher, ProvSettings settings) + { + if (settings.Error != null) + publisher.Error += settings.Error; // provenance proven -> SILENT + } +} + +// PUBLIC candidate (warning stays): same body, but the subscribing method is +// public — a caller outside this compilation could pass anything, so the +// caller audit can never be complete. +public static class ProvPublicFactory +{ + public static ProvPublisher CreatePublic(ProvSettings settings) + { + var pub = new ProvPublisher(); + ApplyPublic(pub, settings); + return pub; + } + + public static void ApplyPublic(ProvPublisher pub, ProvSettings settings) + { + pub.Faulted += settings.Error; // stays the OWN001 warning + } +} + +// MIXED callers (warning stays): one caller passes a fresh local, another +// passes a long-lived FIELD — the field caller breaks the every-caller proof. +public class ProvMixedFactory +{ + private readonly ProvPublisher _shared = new ProvPublisher(); + + public static ProvPublisher CreateMixed(ProvSettings settings) + { + var fresh = new ProvPublisher(); + ApplyMixed(fresh, settings); + return fresh; + } + + public void WireShared(ProvSettings settings) => ApplyMixed(_shared, settings); + + private static void ApplyMixed(ProvPublisher target, ProvSettings settings) + { + target.Mixed += settings.Error; // stays the OWN001 warning + } +} + +// FIELD-STORED fresh local (warning stays): the caller constructs the publisher +// but ALSO parks it in a static field before the call — it escapes beyond the +// return, so "dies with the returned object" is no longer provable. +public static class ProvStoredFactory +{ + private static ProvPublisher? _cached; + + public static ProvPublisher CreateStored(ProvSettings settings) + { + var made = new ProvPublisher(); + _cached = made; + ApplyStored(made, settings); + return made; + } + + private static void ApplyStored(ProvPublisher stored, ProvSettings settings) + { + stored.Stored += settings.Error; // stays the OWN001 warning + } +} + +// PARAM->PARAM (warning stays) — the DUAL this feature must never silence: the +// caller forwards ITS OWN parameter (e.g. a DI-injected bus), not a fresh +// local. If `bus` is a singleton this is a genuine subscription leak. +public class ProvBusWiring +{ + public void Attach(ProvBus bus) + { + Wire(bus); + } + + private void Wire(ProvBus bus) + { + bus.Changed += OnChanged; // stays the OWN001 warning + } + + private void OnChanged(object? sender, EventArgs e) { } +} diff --git a/ownlang/ownir.py b/ownlang/ownir.py index c178e4d8..a7390ef7 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -532,6 +532,15 @@ def load(path: str) -> dict[str, Any]: if stp is not None and not isinstance(stp, str): raise OwnIRError( f"subscription 'source_type' must be a string, got {stp!r}") + # `source_provenance` (P-004, #146): interprocedural publisher + # provenance for an injected source. Additive/optional; only the + # exact value "returned_fresh" routes (bounded -> silent) — any + # other value keeps the honest OWN001 warning path. + spr = s.get("source_provenance") + if spr is not None and not isinstance(spr, str): + raise OwnIRError( + f"subscription 'source_provenance' must be a string, " + f"got {spr!r}") # Optional DI registration graph (DI001 — captive dependency, P-006). Additive # and optional: an older core simply ignores it. svcs = result.get("services", []) @@ -751,6 +760,14 @@ def to_own(facts: dict[str, Any]) -> tuple[str, dict[str, dict[str, Any]]]: cap_handles.append(handle) any_capture = True continue + # Interprocedural publisher provenance (#146, mirrors to_module): an + # injected `+=` whose publisher every in-compilation caller freshly + # constructs and lets escape only into this call / its own `return` + # is bounded by the returned publisher's lifetime — the handler dies + # with it. Same boundedness as a locally-constructed source: silent. + if (rkind == "subscription" and sub.get("source") == "injected" + and sub.get("source_provenance") == "returned_fresh"): + continue # DI-sourced escape (mirrors to_module): an injected source with a KNOWN # DI lifetime lowers to `subscribe self to ` under its DI region. # Only subscriptions reroute here — a non-subscription resource with an @@ -901,6 +918,21 @@ def to_module(facts: dict[str, Any], fn_lt = self_region any_capture = True continue + # Interprocedural publisher provenance (P-004, #146): an injected `+=` + # whose publisher is proven "constructed-and-returned" by EVERY + # in-compilation caller (the extractor's compilation-wide pass stamps + # `source_provenance: "returned_fresh"`) is bounded by the returned + # publisher's lifetime — the handler lives exactly as long as the + # object the caller now holds, and dies with it. That is the same + # boundedness as a locally-constructed source, so it is dropped + # (silent). The INSTANCE-level provenance fact deliberately beats the + # TYPE-level DI hop below: even if the publisher's type is DI- + # registered, THIS publisher was freshly constructed by the caller, + # not resolved from the container. Only the exact vocabulary value + # routes — an unknown provenance keeps the honest OWN001 warning. + if (rkind == "subscription" and sub.get("source") == "injected" + and sub.get("source_provenance") == "returned_fresh"): + continue # P-006 + P-004 DI-sourced escape: an injected subscription whose source # TYPE resolves (via the `services` graph) to a KNOWN DI lifetime routes # through the SAME region engine. singleton/scoped/transient become the diff --git a/spec/OwnIR.md b/spec/OwnIR.md index 725ce186..b64d97a9 100644 --- a/spec/OwnIR.md +++ b/spec/OwnIR.md @@ -111,6 +111,16 @@ every record as a `subscription`. `static`/external/`unknown` (process- or longer-lived → leak). The region model (`capture`) is precise where the token model only warns. +**Publisher provenance** (additive/optional, #146): an `injected` `subscription` +record may carry `source_provenance: "returned_fresh"` — the frontend's +compilation-wide pass proved that **every** in-compilation caller of the method +passes a publisher it freshly constructs and lets escape only into this call or +its own `return`. The subscription is then bounded by the returned publisher's +lifetime (the handler dies with it) and is dropped silently, like a +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. + ## 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 ff5b6fea..4a9b1ae5 100644 --- a/spec/ownir.schema.json +++ b/spec/ownir.schema.json @@ -125,6 +125,10 @@ "source_type": { "description": "The declared type of an injected event source, cross-referenced against `services` to derive its DI lifetime/region (P-006 + P-004). Additive/optional. Explicit null is accepted and preserved (Option).", "type": ["string", "null"] + }, + "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"] } } }, diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 3e56bd0f..0960c84a 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -181,6 +181,77 @@ def _one(source: str, lambda_: bool = False) -> Finding: if "inline lambda" not in lam.message or "-=" not in lam.message: fails.append(f"lambda handler should note the missing -= handle: {lam.message!r}") + # --- #146: interprocedural publisher provenance. An injected `+=` whose + # publisher is proven "constructed-and-returned" by EVERY in-compilation + # caller (the extractor's compilation-wide pass stamps + # `source_provenance: "returned_fresh"`) is bounded by the returned + # publisher's lifetime -> SILENT, like a locally-constructed source (the + # Newtonsoft `JsonSerializer.Create` -> `ApplySerializerSettings` shape). + # Precision-first: only the exact vocabulary value routes; anything else + # keeps the honest OWN001 warning. + def _prov(provenance: str | None, source_type: str | None = None, + services: list[dict[str, object]] | None = None) -> list[Finding]: + s: dict[str, object] = { + "event": "serializer.Error", "handler": "settings.Error", "line": 9, + "released": False, "resource": "subscription", "source": "injected"} + if provenance is not None: + s["source_provenance"] = provenance + if source_type is not None: + s["source_type"] = source_type + pfacts: dict[str, object] = {"module": "M", "components": [ + {"name": "SerializerFactory", "file": "F.cs", "subscriptions": [s]}]} + if services is not None: + pfacts["services"] = services + return check_facts(pfacts) + + # proven returned-fresh publisher -> silent. + checks += 1 + if _prov("returned_fresh"): + fails.append(f"returned-fresh publisher should be silent, got " + f"{[(x.code, x.severity) for x in _prov('returned_fresh')]}") + # an UNKNOWN provenance value never silences — the honest warning stays. + checks += 1 + if [(x.code, x.severity) for x in _prov("hearsay")] != [("OWN001", "warning")]: + fails.append(f"unknown provenance must keep the OWN001 warning, got " + f"{[(x.code, x.severity) for x in _prov('hearsay')]}") + # premise guard: WITHOUT provenance, a singleton-registered source_type + # escalates through the DI hop to OWN014 (the type-level path this test + # pits the instance-level fact against). + _prov_svcs: list[dict[str, object]] = [ + {"name": "IEventBus", "lifetime": "singleton", "file": "S.cs", "line": 3}] + checks += 1 + if [x.code for x in _prov(None, "IEventBus", _prov_svcs)] != ["OWN014"]: + fails.append(f"premise: singleton-typed injected source should escalate " + f"to OWN014, got " + f"{[x.code for x in _prov(None, 'IEventBus', _prov_svcs)]}") + # instance-level provenance BEATS the type-level DI hop: even with the + # publisher's type registered as a singleton, THIS publisher was freshly + # constructed by the caller, not resolved from the container -> silent. + checks += 1 + if _prov("returned_fresh", "IEventBus", _prov_svcs): + fails.append( + f"returned-fresh must beat the DI singleton escalation, got " + f"{[(x.code, x.severity) for x in _prov('returned_fresh', 'IEventBus', _prov_svcs)]}") + # the lowered sketch still parses with a provenance-skipped record present. + checks += 1 + try: + parse(to_own({"module": "M", "components": [ + {"name": "F", "file": "F.cs", "subscriptions": [ + {"event": "s.E", "handler": "h", "line": 2, "released": False, + "resource": "subscription", "source": "injected", + "source_provenance": "returned_fresh"}]}]})[0]) + except Exception as e: + fails.append(f"provenance-skipped facts do not lower/parse: {e}") + # 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": "e", "line": 1, + "source_provenance": 7}]}]}): + fails.append("non-string source_provenance 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 c3cb6567bdae16b070672a71fa4fc0f07d406a7b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 05:47:16 +0000 Subject: [PATCH 2/3] fix(#146): deny local-function closure captures in both provenance gates (Codex P2 x2) A stored local function is a closure exactly like a lambda: it can be invoked from a longer-lived root after the audited call returns. Both gates rejected only AnonymousFunctionExpressionSyntax, so a publisher parameter subscribed inside a stored 'void Later() { p.Event += h; }' (callee side), or a fresh local passed to the helper from a stored 'void Wire() => Apply(p, s);' (caller side), could still earn the returned_fresh stamp and silence a real warning. Both ancestor walks now also match LocalFunctionStatementSyntax, bounded at the audited node so a method never self-denies. Regression-pinned by two new denial cases in ReturnedPublisherSample.cs (deferred.Deferred / later.Later) asserted in the wpf-extractor CI job. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 8 ++-- frontend/roslyn/OwnSharp.Extractor/Program.cs | 17 ++++++- .../roslyn/samples/ReturnedPublisherSample.cs | 46 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0cce1371..bd72551d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -247,9 +247,11 @@ jobs: echo "FAIL: the proven returned-fresh publisher subscription must be silent"; exit 1 fi # ...and every denial case KEEPS the honest OWN001 warning — public - # candidate, mixed callers, field-stored fresh local, and the - # param->param DI dual this feature must never silence. - for ev in "pub.Faulted" "target.Mixed" "stored.Stored" "bus.Changed"; do + # candidate, mixed callers, field-stored fresh local, the param->param + # DI dual this feature must never silence, and the two local-function + # closure escapes (callee-side capture / caller-side capture, Codex P2). + for ev in "pub.Faulted" "target.Mixed" "stored.Stored" "bus.Changed" \ + "deferred.Deferred" "later.Later"; do echo "$out" | grep -qE "ReturnedPublisherSample\.cs:[0-9]+: warning: \[OWN001\].*'$ev'" \ || { echo "FAIL: expected the OWN001 warning to survive for '$ev' (provenance must deny)"; exit 1; } done diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 8b84161b..fdec3076 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -843,7 +843,15 @@ static bool ComputeReturnedFreshParam(IParameterSymbol p, CSharpCompilation comp || !SymbolEqualityComparer.Default.Equals(rp, p)) continue; if (id.Parent is not MemberAccessExpressionSyntax pma || pma.Expression != id) return false; - if (id.Ancestors().OfType().Any()) + // ANY nested function boundary between the reference and the method's + // own declaration is a closure that can be stored and invoked from a + // longer-lived root — a lambda OR a local function (Codex P2: a stored + // `void Later() { p.Event += h; }` escapes exactly like a lambda). + // Bounded at `mnode` so the method's own declaration node never + // self-denies. + if (id.Ancestors().TakeWhile(a => a != mnode) + .Any(a => a is AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax)) return false; } } @@ -916,8 +924,13 @@ static bool CallPassesBoundedFreshLocal(InvocationExpressionSyntax inv, int ordi if (use.Identifier.Text != local.Name) continue; if (model.GetSymbolInfo(use).Symbol is not ILocalSymbol us || !SymbolEqualityComparer.Default.Equals(us, local)) continue; + // a use inside ANY nested function below the scope — lambda or local + // function — is a closure capture: the closure can be stored and run + // after the local has escaped (Codex P2: `void Wire() => Apply(p, s);` + // stored into a delegate), so even a target-argument use there denies. if (use.Ancestors().TakeWhile(a => a != scope) - .OfType().Any()) + .Any(a => a is AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax)) return false; if (use.Parent is ReturnStatementSyntax) continue; if (use.Parent is ArgumentSyntax ua && ua.RefKindKeyword.IsKind(SyntaxKind.None) diff --git a/frontend/roslyn/samples/ReturnedPublisherSample.cs b/frontend/roslyn/samples/ReturnedPublisherSample.cs index 590753e9..af288a01 100644 --- a/frontend/roslyn/samples/ReturnedPublisherSample.cs +++ b/frontend/roslyn/samples/ReturnedPublisherSample.cs @@ -20,6 +20,8 @@ public class ProvPublisher public event EventHandler? Faulted; // public candidate (warning) public event EventHandler? Mixed; // mixed callers (warning) public event EventHandler? Stored; // field-stored fresh local (warning) + public event EventHandler? Deferred; // callee-side local-function capture (warning) + public event EventHandler? Later; // caller-side local-function capture (warning) } public class ProvSettings @@ -111,6 +113,50 @@ private static void ApplyStored(ProvPublisher stored, ProvSettings settings) } } +// CALLEE-SIDE LOCAL-FUNCTION CAPTURE (warning stays; Codex P2 regression): the +// `+=` sits inside a local function that captures the publisher parameter and is +// STORED into a delegate field — the closure can run from a longer-lived root +// after the call returns, so the parameter escaped and provenance must deny. +public static class ProvLocalFuncFactory +{ + private static Action? _pending; + + public static ProvPublisher CreateDeferred(ProvSettings settings) + { + var fresh = new ProvPublisher(); + ApplyDeferred(fresh, settings); + return fresh; + } + + private static void ApplyDeferred(ProvPublisher deferred, ProvSettings settings) + { + void Later() { deferred.Deferred += settings.Error; } // captures the param + _pending = Later; // closure escapes -> deny + } +} + +// CALLER-SIDE LOCAL-FUNCTION CAPTURE (warning stays; Codex P2 regression): the +// fresh local is passed to the audited helper FROM a stored local function — the +// closure may run after `made` escaped, so the target-argument use inside it +// must deny the "bounded" proof. +public static class ProvCallerLocalFuncFactory +{ + private static Action? _wire; + + public static ProvPublisher CreateLater(ProvSettings settings) + { + var made = new ProvPublisher(); + void Wire() => ApplyLater(made, settings); // capture of the fresh local + _wire = Wire; // closure escapes -> deny + return made; + } + + private static void ApplyLater(ProvPublisher later, ProvSettings settings) + { + later.Later += settings.Error; // stays the OWN001 warning + } +} + // PARAM->PARAM (warning stays) — the DUAL this feature must never silence: the // caller forwards ITS OWN parameter (e.g. a DI-injected bus), not a fresh // local. If `bus` is a singleton this is a genuine subscription leak. From d711ec1cd3d1c152fe091742cd9af567fa751c08 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 08:15:20 +0000 Subject: [PATCH 3/3] docs: fold #146 into the oracle disposition counts; list the local-function denial cases CodeRabbit nitpicks on #208: the returned-fresh denial-case enumerations in field-notes #8 and oracle-known-fps predate c3cb656, so they omitted the two local-function closure captures; and the disposition summary still counted serializer.Error as baselined (Fixed 6->7, Baselined 5->4, 8 findings/7 rules -> 7 findings/6 rules). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- docs/notes/field-notes-patterns.md | 6 ++++-- docs/notes/oracle-known-fps.md | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index 8af218b6..c7bb2202 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -263,8 +263,10 @@ pass: when a `+=` publisher is a parameter of a private/internal method and the call / its own `return` (and the callee never lets the param escape), the subscription is stamped `source_provenance: "returned_fresh"` and the bridge drops it (bounded, silent). Any unprovable step — public candidate, method-group -reference, mixed callers, field-stored local, param→param forwarding — denies -the proof and the honest warning stands. Pinned by +reference, mixed callers, field-stored local, param→param forwarding, or a +local-function closure capture on either side (callee-side `ProvLocalFuncFactory`, +caller-side `ProvCallerLocalFuncFactory` — a stored local function escapes exactly +like a lambda) — denies the proof and the honest warning stands. Pinned by `frontend/roslyn/samples/ReturnedPublisherSample.cs` (CI `wpf-extractor`) and the `source_provenance` checks in `tests/test_ownir.py`; spec'd in `spec/OwnIR.md` §4. diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index 4eea62b6..dfaa27d4 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -23,8 +23,8 @@ reason: we and the oracles occupy orthogonal niches. | disposition | count | what happens on re-run | |---|---:|---| -| **Fixed in the extractor** | 6 | no longer fire (5 NLog `WaitForDispose` timers + protobuf `XsltOptions` self-cycle — see below) | -| **Baselined FP** | 5 | moved to "Known FP (baselined)", out of the triage queue | +| **Fixed in the extractor** | 7 | no longer fire (5 NLog `WaitForDispose` timers + protobuf `XsltOptions` self-cycle + Newtonsoft `serializer.Error` returned-fresh provenance, #146 — see below) | +| **Baselined FP** | 4 | moved to "Known FP (baselined)", out of the triage queue | | **Non-product (path filter)** | 2 | dropped by `--exclude-tests` (`unittest` rule) | | **True positive — kept visible** | 4 | stays in "Own.NET only" (real catch, oracle can't express) | | **True-but-benign — kept, baselined-as-sample** | 3 | (protobuf `assorted/` samples) baselined as non-product | @@ -33,10 +33,11 @@ reason: we and the oracles occupy orthogonal niches. "True-but-benign sample" 2→3 — it is a real leak of a custom `IDisposable` `Nuxleus.Performance.Stopwatch`, not the BCL non-disposable type first assumed.) -The 5 baselined FPs + the 3 non-product-sample reals = 8 findings, covered by -**7 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies -share one basename-keyed rule); the 2 test-base findings are the `--exclude-tests` -drops; the 4 true positives are deliberately **not** suppressed. +The 4 baselined FPs + the 3 non-product-sample reals = 7 findings, covered by +**6 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies +share one basename-keyed rule; the Newtonsoft `serializer.Error` rule was removed +when #146 fixed it at the source); the 2 test-base findings are the +`--exclude-tests` drops; the 4 true positives are deliberately **not** suppressed. **Update (extractor fix landed — protobuf self-cycle).** `CommandLineOptions.XsltOptions. XsltMessageEncountered` — a `this`-capturing handler subscribed to an event on @@ -53,8 +54,9 @@ pass proves the `Create` → `ApplySerializerSettings` shape bounded entry is removed (deliberately — a regression must reappear in triage, not be swallowed by the allowlist). Pinned by `frontend/roslyn/samples/ReturnedPublisherSample.cs`; denial cases (public -candidate, mixed callers, field-stored local, param→param DI forwarding) keep the -honest warning. +candidate, mixed callers, field-stored local, param→param DI forwarding, and the +two local-function closure captures — callee-side `ProvLocalFuncFactory` and +caller-side `ProvCallerLocalFuncFactory`) keep the honest warning. **Update (extractor fix landed — all NLog timers).** All 5 of the original NLog `WaitForDispose` timer FPs are now **fixed at the source**, baseline entries deleted.