Skip to content

Commit 12c052a

Browse files
committed
fix(extractor): close two soundness holes in the #228 app-scoped gate (Codex review)
Both Codex P2s verified real, both fixed: - Handler target must be `this`: a method group qualified with ANOTHER instance of the same App-derived type (`_twin!.OnTheme`) has the right ContainingType but pins that other instance, not the process-lived singleton. IsOwnMethodGroupHandler now accepts only a bare identifier (implicit this) or an explicit `this.X` receiver. - Stale resolver binding: `var tm = helper.GetThemeManager(); tm = injectedBus; tm.ThemeChanged += H` passed on the declaration-site initializer alone. IsNeverReassigned kills the proof on ANY assignment to the local or ref/out escape anywhere in the enclosing member — conservative (order-insensitive), worst case keeps today's warning. Two new flagged controls in AppScopedSourceSample.cs (TwinApp, ReassignApp) + CI assertions. Verified with the real extractor: all five controls warn, both positives silent; full-sample-set output still byte-identical to main. Refs #228 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
1 parent f403bdb commit 12c052a

3 files changed

Lines changed: 92 additions & 2 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,9 +853,17 @@ jobs:
853853
# exact hole that sank the rejected clsIsStatic broadening;
854854
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'LambdaApp'" \
855855
|| { echo "FAIL: expected OWN001 on the lambda-handler subscription inside the App (capture hole)"; exit 1; }
856-
# (3) App + method-group handler, but a NON-curated resolver.
856+
# (3) App + method-group handler, but a NON-curated resolver;
857857
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'CuratedOnlyApp'" \
858858
|| { echo "FAIL: expected OWN001 on the non-curated resolver source inside the App"; exit 1; }
859+
# (4, Codex P2) the method group is qualified with ANOTHER instance of the
860+
# same App type — right ContainingType, wrong delegate target;
861+
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'TwinApp'" \
862+
|| { echo "FAIL: expected OWN001 when the handler target is another instance, not this"; exit 1; }
863+
# (5, Codex P2) the local is REASSIGNED to an injected publisher after the
864+
# curated initializer — the stale declaration binding must not exempt.
865+
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'ReassignApp'" \
866+
|| { echo "FAIL: expected OWN001 when the resolver-bound local is reassigned before the +="; exit 1; }
859867
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) + #218 DP old->new subscription rotation (silent; controls flagged) + #228 curated app-scoped source in App (silent; controls flagged) at the C# location"
860868
- name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
861869
run: |

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,11 @@ static bool ResolvesToAppScopedCall(ExpressionSyntax expr, SemanticModel model,
929929
}
930930
if (model.GetSymbolInfo(expr).Symbol is not ILocalSymbol local)
931931
return false;
932+
// The binding is only trustworthy if the local still HOLDS it at the `+=`:
933+
// a reassignment anywhere in the enclosing member (or a ref/out escape that
934+
// could rebind it) makes the initializer stale — no exemption (Codex P2).
935+
if (!IsNeverReassigned(local, model))
936+
return false;
932937
foreach (var r in local.DeclaringSyntaxReferences)
933938
switch (r.GetSyntax())
934939
{
@@ -946,6 +951,34 @@ when des.Ancestors().OfType<IsPatternExpressionSyntax>().FirstOrDefault()
946951
return false;
947952
}
948953

954+
// #228 (Codex P2): the declaration-site binding proves the local's value at the
955+
// `+=` only if nothing rebinds it. Conservative whole-member scan: ANY assignment
956+
// whose LHS is this local, or ANY `ref`/`out` argument passing it, kills the proof
957+
// (even one after the subscription — cheaper than flow order and precision-safe:
958+
// the worst case is keeping today's honest warning).
959+
static bool IsNeverReassigned(ILocalSymbol local, SemanticModel model)
960+
{
961+
foreach (var r in local.DeclaringSyntaxReferences)
962+
{
963+
var scope = r.GetSyntax().Ancestors().FirstOrDefault(n =>
964+
n is MethodDeclarationSyntax or ConstructorDeclarationSyntax
965+
or AccessorDeclarationSyntax or LocalFunctionStatementSyntax
966+
or AnonymousFunctionExpressionSyntax);
967+
if (scope is null)
968+
return false;
969+
foreach (var asg in scope.DescendantNodes().OfType<AssignmentExpressionSyntax>())
970+
if (SymbolEqualityComparer.Default.Equals(
971+
model.GetSymbolInfo(asg.Left).Symbol, local))
972+
return false;
973+
foreach (var arg in scope.DescendantNodes().OfType<ArgumentSyntax>())
974+
if (!arg.RefOrOutKeyword.IsKind(SyntaxKind.None)
975+
&& SymbolEqualityComparer.Default.Equals(
976+
model.GetSymbolInfo(arg.Expression).Symbol, local))
977+
return false;
978+
}
979+
return true;
980+
}
981+
949982
// #228: the handler must be a METHOD GROUP declared on the subscribing class itself —
950983
// its delegate target is then the App singleton (already process-lived, nothing to
951984
// promote). A lambda/anonymous method is rejected outright: it may capture an
@@ -955,7 +988,19 @@ when des.Ancestors().OfType<IsPatternExpressionSyntax>().FirstOrDefault()
955988
static bool IsOwnMethodGroupHandler(ExpressionSyntax right, SemanticModel model,
956989
INamedTypeSymbol? cls)
957990
{
958-
if (cls is null || right is AnonymousFunctionExpressionSyntax)
991+
if (cls is null)
992+
return false;
993+
// The delegate TARGET must be `this` — a bare identifier (implicit this) or an
994+
// explicit `this.X`. A method group qualified with ANOTHER instance of the same
995+
// App-derived type (`otherApp.OnTheme`) has the right ContainingType but pins
996+
// that other instance, not the process-lived singleton (Codex P2).
997+
var receiverIsThis = right switch
998+
{
999+
IdentifierNameSyntax => true,
1000+
MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } => true,
1001+
_ => false,
1002+
};
1003+
if (!receiverIsThis)
9591004
return false;
9601005
var info = model.GetSymbolInfo(right);
9611006
var sym = info.Symbol as IMethodSymbol

‎frontend/roslyn/samples/AppScopedSourceSample.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,41 @@ public void OnStartup()
102102

103103
void OnTheme(object? sender, EventArgs e) { }
104104
}
105+
106+
// CONTROL 4 (flagged, Codex P2): the method group is qualified with ANOTHER
107+
// instance of the same App-derived type — right ContainingType, wrong delegate
108+
// TARGET: the process-lived source pins `_twin`, not `this`.
109+
public class TwinApp : Application
110+
{
111+
readonly TwinApp? _twin;
112+
113+
public TwinApp(TwinApp? twin) => _twin = twin;
114+
115+
public void OnStartup()
116+
{
117+
if (new PaletteHelper().GetThemeManager() is { } themeManager)
118+
themeManager.ThemeChanged += _twin!.OnTheme; // OWN001 warning
119+
}
120+
121+
public void OnTheme(object? sender, EventArgs e) { }
122+
}
123+
124+
// CONTROL 5 (flagged, Codex P2): the local STARTS as the curated resolver result
125+
// but is REASSIGNED to an injected publisher before the `+=` — the declaration-site
126+
// binding is stale, so the exemption must not trust it.
127+
public class ReassignApp : Application
128+
{
129+
readonly ThemeManagerLike _injectedBus;
130+
131+
public ReassignApp(ThemeManagerLike bus) => _injectedBus = bus;
132+
133+
public void OnStartup()
134+
{
135+
var tm = new PaletteHelper().GetThemeManager()!;
136+
tm = _injectedBus;
137+
tm.ThemeChanged += OnTheme; // OWN001 warning
138+
}
139+
140+
void OnTheme(object? sender, EventArgs e) { }
141+
}
105142
}

0 commit comments

Comments
 (0)