From 64b65465730dea60e23d7717eb26f12ea5a5b04b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Fri, 14 Aug 2026 21:47:09 +0200 Subject: [PATCH] fix: seal an impersonation scope into the operation it was opened for (#1444) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every write a package install made was logged `by system-security` — the synthesized home root, all five copy batches, the manifest — while `_UserActivity`, written moments later in the same flow, attributed correctly to the learner. None of those writes was inside an ImpersonateAsSystem scope. MECHANISM (established, then pinned by a test that fails without the fix) The idiomatic system read/write is `Observable.Using(access.ImpersonateAsSystem, _ => work)`. It is correct for the work itself and is not self-contained: Rx forwards OnNext to the SUBSCRIBER before Using disposes its resource, so everything the subscriber composes on that emission is built while the impersonation is still open. The mesh write primitives eager-capture AccessService.Context when CALLED and re-stamp it around their own emissions (CarryAccessContext), so the leaked identity is not merely inherited one hop — it is RE-ACQUIRED at every hop after it. `_UserActivity` attributed correctly because it is the one write on that path posted directly from the request thread rather than composed on an emission; it was the norm, and the rest were the anomaly. WHY IT IS FIXED AT THE ROOT RATHER THAN IN THE LOG The issue asks to separate attribution from authorization, and the second half is why this is not a logging defect. AccessControlPipeline.HandleGetPermission already carries the scar in a comment: SecurityService's bootstrap-time system scope leaked past its using-block onto the action-block thread, and trusting the ambient there returned Permission.All for EVERY caller, anonymous included. That call site defends itself by resolving the identity explicitly. This closes the class at the source, so the next call site does not have to know Rx's disposal order to be safe. THE FIX `ImpersonationScopeExtensions` — RunAsSystem / RunAsHub / ContainIdentity. The scope is entered at Subscribe (so the cold work is covered — the Observable.Using, never Defer-plus-using, rule) and every notification is delivered under the SUBSCRIBER's own identity. Restore semantics are not re-implemented: it reuses CarryAccessContext's per-callback RestoringObserver, so the three ways an identity can be attached to a pipeline cannot drift apart. 🚨 It never invents an identity. What is restored is exactly what was ambient at Subscribe — a real user for a user-driven flow, system-security for a genuinely system-initiated one, and NOTHING when the subscriber had no identity. That last case is what makes adoption safe at an existing call site: a background worker that never had a user is not fail-closed. The only behaviour that changes is the defect itself. ADOPTED BY THE EIGHT COPIES OF THE SAME HELPER `AsSystem` had been hand-written eight times across src/ — Graph (×3), Mesh.Contract, GitSync (×2), InstanceSync, PluginCatalog — each carrying a comment saying it mirrors one of the others. They now delegate to the one sealed primitive. NOT DONE, DELIBERATELY The reported install path lives in the Plugins repo and is already fixed there (Store/Plugin/Source/SystemIdentity.cs, which cites this issue). Its own analysis establishes the realised damage: nothing landed anywhere the learner could not have written — every path was under their own partition root — so the measured cost is an audit trail that says the platform did what a learner asked for. That plugin-local helper is what this framework primitive should have been all along. Verified: SystemScopeDoesNotEscapeTest 8/8 (its first case pins the raw Rx ordering the rest depend on); Messaging.Hub 251, AccessControl 29, InstanceSync 26, GitSync 163, Graph 1127 — all green. -c Release -warnaserror clean for all six touched projects. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KmBFaHTqhy7h742wkAaJKj --- .../Architecture/AccessContextPropagation.md | 32 +++ ...4-your-name-stays-on-what-you-asked-for.md | 33 +++ .../GitHubActivityExtensions.cs | 3 +- .../SystemOwnedAccessRetractionHandler.cs | 4 +- src/MeshWeaver.Graph/AccessGrantNotifier.cs | 2 +- .../EventSubscriptionRunner.cs | 2 +- src/MeshWeaver.Graph/StaticRepoImporter.cs | 7 +- .../InstanceSyncService.cs | 5 +- .../MeshExtensions.cs | 4 +- .../AccessContextCaptureExtensions.cs | 16 ++ .../ImpersonationScopeExtensions.cs | 114 ++++++++ .../ModuleDiscoveryService.cs | 5 +- .../SystemScopeDoesNotEscapeTest.cs | 248 ++++++++++++++++++ 13 files changed, 451 insertions(+), 24 deletions(-) create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-14-your-name-stays-on-what-you-asked-for.md create mode 100644 src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs create mode 100644 test/MeshWeaver.Messaging.Hub.Test/SystemScopeDoesNotEscapeTest.cs diff --git a/src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md b/src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md index 846a6d4bd8..cab67ec41a 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md @@ -370,6 +370,38 @@ A `SynchronizationStream.OnNext` that unconditionally stamps `ImpersonateAsHub(H > Every `ImpersonateAsSystem` callsite is a deliberate choice to bypass all access checks. Grep for them; review each. If a narrower identity would suffice, use that instead. +### 🚨 An impersonation scope must not ESCAPE the operation it was opened for + +`accessService.RunAsSystem(work)` — **not** a hand-written `Observable.Using(access.ImpersonateAsSystem, _ => work)` — wherever the scoped observable is **returned to a caller**. + +The hand-written idiom is correct for the work itself and leaks everywhere after it: + +```csharp +// ❌ the scope escapes: Rx forwards OnNext to the subscriber BEFORE Using disposes its resource, +// so the write below is CONSTRUCTED while the impersonation is still open — and the write +// primitives eager-capture AccessService.Context when they are CALLED. +Observable.Using(access.ImpersonateAsSystem, _ => ReadGatedSource()) + .SelectMany(rows => meshService.CreateNodes(Plan(rows))); // lands as `system-security` + +// ✅ the scope covers the read and stops at its boundary +access.RunAsSystem(ReadGatedSource) + .SelectMany(rows => meshService.CreateNodes(Plan(rows))); // lands as the CALLER +``` + +Because those primitives also **re-stamp** the captured identity around their own emissions (`CarryAccessContext` — "MessageHub sets, framework primitive preserves"), a leaked System identity is not merely inherited one hop — it is **re-acquired at every hop after it**. That is how a package install wrote its home root, five copy batches and its manifest as `system-security` while `_UserActivity` — the one write on that path posted directly from the request thread rather than composed on an emission — attributed to the real user (#1444). + +**It is an authorization concern, not only an audit one.** `AccessControlPipeline.HandleGetPermission` carries the scar: `SecurityService`'s bootstrap-time system scope leaked past its using-block onto the action-block thread, and trusting the ambient there returned `Permission.All` for **every** caller, anonymous included. That call site defends itself by resolving the identity explicitly; `RunAsSystem` fixes the class at the source so the next one does not have to know Rx's disposal order. + +| API | Use it for | +|---|---| +| `access.RunAsSystem(work)` | A system read/write whose result a caller composes on. Enters the scope at Subscribe (so the cold work is covered), delivers every notification under the **subscriber's** identity. | +| `access.RunAsHub(hub, work)` | The same seal for a hub identity. | +| `source.ContainIdentity(access)` | Sealing an **already-composed** chain — several system calls where the caller builds on the LAST one's emission. | + +🚨 **It never invents an identity.** What is restored is exactly what was ambient at Subscribe: a real user for a user-driven flow, `system-security` for a genuinely system-initiated one, and **nothing at all** when the subscriber had no identity — so a background worker that never had a user is not fail-closed by adopting it. The only behaviour that changes is the defect: a caller who *had* an identity, silently continuing as System. + +Pinned by `SystemScopeDoesNotEscapeTest` (test/MeshWeaver.Messaging.Hub.Test), whose first case asserts the raw Rx ordering every other case rests on. + ### Implementation: define + grant + test For each sanctioned identity, you need all three: diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-14-your-name-stays-on-what-you-asked-for.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-14-your-name-stays-on-what-you-asked-for.md new file mode 100644 index 0000000000..936a2c28c6 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-14-your-name-stays-on-what-you-asked-for.md @@ -0,0 +1,33 @@ +--- +Name: Your name stays on what you asked for +Category: Fix +Description: Work you start is now recorded — and authorised — as you, not as the platform. An internal identity opened for one step could previously carry on into everything composed after it. +Icon: PersonAccounts +Order: -20260814 +--- + +# Your name stays on what you asked for + +Some steps genuinely run as the platform rather than as you: reading content you are entitled to but +cannot see directly, creating the storage a brand-new space lives in. That is by design, and it is +scoped to the step that needs it. + +It was not staying scoped. The platform identity, once established for one step, carried on into +everything built on that step's result — so an operation you started could have its **later** writes +recorded as the platform instead of as you. A package install did exactly that: the home it created, +every batch of content it copied and the manifest it finished with were all attributed to the +platform, while the activity entry beside them correctly named the person who asked. + +Two things were wrong with that, and the second is the reason it is fixed at the root rather than in +the log: + +- **The history read wrong.** "Who created this?" answered *the platform* for things a person asked + for. Nothing in the affected installs landed anywhere the person could not have written — but the + record no longer matched what happened. +- **Permission questions were being answered for the wrong identity.** The platform identity is + allowed everything. Any check that ran after the leak was asking about the platform rather than + about you, so a step you should have been refused could have quietly succeeded. + +An internal identity is now sealed into the step it was opened for. What runs as the platform is +exactly what has to; everything composed afterwards is yours again — audited as you, and checked +against your permissions. diff --git a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs index 8f6d617de1..4c2c091629 100644 --- a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs +++ b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs @@ -72,8 +72,7 @@ private static IObservable TriggerAuthorizedAsSystem( if (string.IsNullOrEmpty(userId) || caller?.IsVirtual == true) userId = WellKnownUsers.Anonymous; - IObservable AsSystem() => - Observable.Using(() => accessService.ImpersonateAsSystem(), _ => runActivity()); + IObservable AsSystem() => accessService.RunAsSystem(runActivity); if (string.Equals(userId, WellKnownUsers.System, StringComparison.Ordinal)) return AsSystem(); diff --git a/src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs b/src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs index d104402fe7..33fc94c8e0 100644 --- a/src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs +++ b/src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs @@ -175,7 +175,5 @@ private IObservable RetractAll(string partition, IList grants) /// Establishes the System identity on the write's own subscribe thread — mirrors /// MeshExtensions.AsSystem, so the deletes are attributed to the platform. private IObservable AsSystem(Func> write) - => accessService is null - ? Observable.Defer(write) - : Observable.Using(() => accessService.ImpersonateAsSystem(), _ => write()); + => accessService.RunAsSystem(write); } diff --git a/src/MeshWeaver.Graph/AccessGrantNotifier.cs b/src/MeshWeaver.Graph/AccessGrantNotifier.cs index 938c4c4358..5b97f01aec 100644 --- a/src/MeshWeaver.Graph/AccessGrantNotifier.cs +++ b/src/MeshWeaver.Graph/AccessGrantNotifier.cs @@ -208,7 +208,7 @@ internal static string ResolveGrantedNode(MeshNode assignmentNode) } private IObservable AsSystem(Func> factory) - => Observable.Using(accessService.ImpersonateAsSystem, _ => Observable.Defer(factory)); + => accessService.RunAsSystem(factory); public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; diff --git a/src/MeshWeaver.Graph/EventSubscriptionRunner.cs b/src/MeshWeaver.Graph/EventSubscriptionRunner.cs index 20f15072e0..52923a6b65 100644 --- a/src/MeshWeaver.Graph/EventSubscriptionRunner.cs +++ b/src/MeshWeaver.Graph/EventSubscriptionRunner.cs @@ -506,7 +506,7 @@ private void MigrateLegacyScheduledActions() /// identity — the runner has no ambient AccessContext, and both reads and writes capture identity /// at construction/subscribe. Defer moves construction inside the impersonation scope. private IObservable AsSystem(Func> factory) - => Observable.Using(accessService.ImpersonateAsSystem, _ => Observable.Defer(factory)); + => accessService.RunAsSystem(factory); /// public Task StopAsync(CancellationToken cancellationToken) diff --git a/src/MeshWeaver.Graph/StaticRepoImporter.cs b/src/MeshWeaver.Graph/StaticRepoImporter.cs index a4657c35d9..dd777aff95 100644 --- a/src/MeshWeaver.Graph/StaticRepoImporter.cs +++ b/src/MeshWeaver.Graph/StaticRepoImporter.cs @@ -1602,12 +1602,7 @@ private static MeshNode Materialize(MeshNode node) /// (which the sync-stream post then carries). See Doc/Architecture/AccessContextPropagation.md. /// private static IObservable AsSystem(IMessageHub hub, Func> write) - { - var access = hub.ServiceProvider.GetService(); - return access is null - ? Observable.Defer(write) - : Observable.Using(() => access.ImpersonateAsSystem(), _ => write()); - } + => hub.ServiceProvider.GetService().RunAsSystem(write); /// /// Surfaces a boot/startup import failure LOUDLY in the GUI. The activity at diff --git a/src/MeshWeaver.InstanceSync/InstanceSyncService.cs b/src/MeshWeaver.InstanceSync/InstanceSyncService.cs index 11d9ed8f1b..7e02910e60 100644 --- a/src/MeshWeaver.InstanceSync/InstanceSyncService.cs +++ b/src/MeshWeaver.InstanceSync/InstanceSyncService.cs @@ -312,8 +312,5 @@ public bool ContentEquals(MeshNode? a, MeshNode? b) /// the user's own context and never go through here. /// public IObservable AsSystem(Func> operation) - { - var accessService = hub.ServiceProvider.GetRequiredService(); - return Observable.Using(() => accessService.ImpersonateAsSystem(), _ => operation()); - } + => hub.ServiceProvider.GetRequiredService().RunAsSystem(operation); } diff --git a/src/MeshWeaver.Mesh.Contract/MeshExtensions.cs b/src/MeshWeaver.Mesh.Contract/MeshExtensions.cs index 67fd8fe28f..3add83a51d 100644 --- a/src/MeshWeaver.Mesh.Contract/MeshExtensions.cs +++ b/src/MeshWeaver.Mesh.Contract/MeshExtensions.cs @@ -2034,9 +2034,7 @@ root is not null /// infrastructure-write). Mirrors StaticRepoImporter.AsSystem. /// private static IObservable AsSystem(AccessService? access, Func> write) - => access is null - ? Observable.Defer(write) - : Observable.Using(() => access.ImpersonateAsSystem(), _ => write()); + => access.RunAsSystem(write); /// /// True if the exception (or any inner) reports an "already exists" outcome — the idempotent-create diff --git a/src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs b/src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs index 89a73e6a9c..f26f91e086 100644 --- a/src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs +++ b/src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs @@ -164,6 +164,22 @@ public static IObservable CarryAccessContext( return new CarryAccessContextObservable(source, access, captured); } + /// + /// Subscribes to with every notification + /// delivered inside a scope keyed to + /// — the same per-callback restore + /// applies. + /// + /// 🚨 Exists so can SEAL an impersonation scope + /// with these exact semantics rather than re-implement them. The difference between the two is + /// only WHEN the identity is decided — this helper is handed one, CarryAccessContext + /// snapshots the ambient at composition time, and ContainIdentity reads it at Subscribe. + /// Keeping one restore implementation is what stops those three drifting apart. + /// + internal static IDisposable SubscribeRestoring( + IObservable source, IObserver observer, AccessService access, AccessContext? captured) => + source.Subscribe(new RestoringObserver(observer, access, captured)); + /// /// Wraps a cold observable so every emission to the subscriber happens /// inside an scope keyed diff --git a/src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs b/src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs new file mode 100644 index 0000000000..7a1904e3e0 --- /dev/null +++ b/src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs @@ -0,0 +1,114 @@ +using System.Reactive.Linq; + +namespace MeshWeaver.Messaging; + +/// +/// 🚨 THE framework's impersonation boundary: a scope that cannot escape the operation it was +/// opened for. +/// +/// The defect this closes (issue #1444). The idiomatic system read/write is +/// Observable.Using(() => access.ImpersonateAsSystem(), _ => work), and it is correct for +/// the work itself. It is not self-contained. Rx forwards OnNext to the subscriber +/// before Using disposes its resource, so everything the subscriber composes on that +/// emission — the next phase, a write, a permission check — is built while the impersonation is +/// still OPEN. And the mesh write primitives EAGER-CAPTURE when +/// they are CALLED and re-stamp it around their own emissions +/// (), +/// so the System identity does not merely survive one hop — it is re-acquired at every hop +/// after it. +/// +/// This is not hypothetical, and it has already reached an authorization decision. +/// AccessControlPipeline.HandleGetPermission carries a comment describing exactly this shape: +/// SecurityService's bootstrap-time ImpersonateAsSystem leaked past its using-block +/// onto the action-block thread, and trusting the ambient there "returned Permission.All for +/// every caller — including anonymous deliveries". That call site defends itself by resolving the +/// identity explicitly instead of reading the ambient. This type fixes the same class at the SOURCE, +/// so the next call site does not have to know Rx's disposal order to be safe. +/// +/// What it does NOT do: invent an identity. restores +/// exactly what was ambient at Subscribe — a real user for a user-driven flow, system-security +/// for a genuinely system-initiated one, and nothing at all when the subscriber had no +/// identity. That last case is deliberate and is what makes adopting this safe at an existing call +/// site: a background worker with no ambient context keeps whatever the framework leaves it (the +/// documented restoreNullCapture: false behaviour of read pipelines), so the only behaviour +/// that changes is the one the defect describes — a caller who HAD an identity, silently continuing +/// as System. +/// +public static class ImpersonationScopeExtensions +{ + /// + /// Runs under the well-known System identity with the scope SEALED: the + /// impersonation is entered at Subscribe (so the cold reads/writes the factory yields carry the + /// system context — the Observable.Using, never Defer-plus-using, rule), + /// and every notification that reaches the subscriber is delivered under the subscriber's OWN + /// identity, so nothing composed downstream inherits System. + /// + /// Prefer this over a hand-written Observable.Using(access.ImpersonateAsSystem, …) + /// at every site that RETURNS the scoped observable to a caller — returning it is precisely what + /// turns a scope into an inheritance. + /// + /// A null (minimal test hosts) runs the work unimpersonated — + /// deferred, so it stays cold. + /// + /// What the scoped operation emits. + /// The access service, or null on a host without one. + /// The cold operation to run as System. + public static IObservable RunAsSystem(this AccessService? access, Func> work) => + access is null + ? Observable.Defer(work) + : Observable + .Using(access.ImpersonateAsSystem, _ => Observable.Defer(work)) + .ContainIdentity(access); + + /// + /// Runs as 's own identity, sealed exactly as + /// is. + /// + /// What the scoped operation emits. + /// The access service, or null on a host without one. + /// The hub whose address is stamped as the principal. + /// The cold operation to run as the hub. + public static IObservable RunAsHub( + this AccessService? access, IMessageHub hub, Func> work) => + access is null + ? Observable.Defer(work) + : Observable + .Using(() => access.ImpersonateAsHub(hub), _ => Observable.Defer(work)) + .ContainIdentity(access); + + /// + /// Seals an ALREADY-COMPOSED : every notification is delivered to the + /// subscriber under the identity that was ambient when they subscribed, whatever identity + /// produced the emission. + /// + /// Use this where the impersonated region is not one call — a chain of system writes whose + /// LAST emission is what the caller composes on. Sealing the chain's boundary is what stops the + /// caller inheriting it. + /// + /// The restore is per-notification (entered on OnNext/OnError/ + /// OnCompleted, disposed as the callback returns), so nothing is left behind on the + /// emitting thread — the same shape CarryAccessContext uses, and the same code. + /// + /// What the operation emits. + /// The already-composed operation whose identity must not escape. + /// The access service, or null on a host without one. + public static IObservable ContainIdentity(this IObservable source, AccessService? access) => + access is null ? source : new ContainedIdentityObservable(source, access); + + /// + /// Reads the subscriber's identity at Subscribe — before any scope this pipeline opens, which is + /// the last moment the ambient context is reliably the subscriber's own — and restores it around + /// every notification. A null read passes straight through: nothing to restore, nothing invented. + /// + private sealed class ContainedIdentityObservable(IObservable source, AccessService access) + : IObservable + { + public IDisposable Subscribe(IObserver observer) + { + var caller = access.Context; + return caller is null + ? source.Subscribe(observer) + : AccessContextCaptureExtensions.SubscribeRestoring(source, observer, access, caller); + } + } +} diff --git a/src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs b/src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs index 144839c5f7..674c1eb05d 100644 --- a/src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs +++ b/src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs @@ -731,10 +731,7 @@ private static string Subdirectory(string? sourceSubdir, string moduleFolder) /// failure this whole service exists to prevent. Failing to resolve must be a startup error, not /// a silent downgrade. private IObservable AsSystem(Func> write) - { - var accessService = hub.ServiceProvider.GetRequiredService(); - return Observable.Using(() => accessService.ImpersonateAsSystem(), _ => write()); - } + => hub.ServiceProvider.GetRequiredService().RunAsSystem(write); // ══════════════════════════════════════════════════════════════════════════ // Making it visible diff --git a/test/MeshWeaver.Messaging.Hub.Test/SystemScopeDoesNotEscapeTest.cs b/test/MeshWeaver.Messaging.Hub.Test/SystemScopeDoesNotEscapeTest.cs new file mode 100644 index 0000000000..403571ef15 --- /dev/null +++ b/test/MeshWeaver.Messaging.Hub.Test/SystemScopeDoesNotEscapeTest.cs @@ -0,0 +1,248 @@ +using System; +using System.Collections.Generic; +using System.Reactive.Linq; +using System.Reactive.Subjects; +using System.Threading.Tasks; +using Xunit; + +namespace MeshWeaver.Messaging.Hub.Test; + +/// +/// Issue #1444 — an impersonation scope that ESCAPES the operation it was opened for. +/// +/// The mechanism. Observable.Using(access.ImpersonateAsSystem, _ => work) reads +/// as "run this work as System", and for the work itself it is. But Rx forwards OnNext to the +/// subscriber BEFORE Using disposes its resource, so anything the subscriber composes on that +/// emission is built while the impersonation is still open. The first test below pins that raw Rx +/// behaviour, because every conclusion here rests on it and it must fail loudly if a future Rx +/// changes it. +/// +/// Why it does not stop at one hop. The mesh write primitives eager-capture +/// when CALLED and re-stamp it around their own emissions +/// (CarryAccessContext), so a leaked System identity is re-acquired at every hop after it — +/// which is how a whole install chain landed as system-security in education CI run +/// 31704948831 while _UserActivity, posted directly from the request thread rather than +/// composed on an emission, attributed correctly. +/// +/// Why it is a permission concern, not only an audit one. +/// AccessControlPipeline.HandleGetPermission already carries the scar: trusting the ambient +/// context there returned Permission.All for every caller including anonymous, because +/// SecurityService's bootstrap-time system scope leaked past its using-block. That call site +/// defends itself; fixes the class at the source. +/// +public class SystemScopeDoesNotEscapeTest +{ + private static readonly AccessContext Alice = new() { ObjectId = "alice", Name = "Alice" }; + + private const string System = "system-security"; + + /// + /// The premise, pinned. Not a test of our code — a test of the Rx ordering every other case here + /// depends on. If this ever goes green-by-accident (Rx disposing before it forwards), the seal + /// below stops being load-bearing and that is worth learning from a failure rather than from a + /// production log. + /// + [Fact] + public void TheIdiomaticSystemScopeIsStillOpenWhenTheSubscriberRuns() + { + var access = new AccessService(); + access.SetContext(Alice); + + string? seenInsideCallback = null; + Observable + .Using(access.ImpersonateAsSystem, _ => Observable.Return(1)) + .Subscribe(_ => seenInsideCallback = access.Context?.ObjectId); + + seenInsideCallback.Should().Be(System, + "Rx forwards OnNext BEFORE Using disposes its resource — so a subscriber composing a " + + "write on this emission builds it under System. That is #1444's mechanism, and the " + + "seal exists because of it"); + access.Context?.ObjectId.Should().Be("alice", + "the scope is correctly restored once the pipeline terminates — the leak is confined to " + + "the notification window, which is exactly where composition happens"); + } + + /// + /// The fix: the same work, the same System identity INSIDE, and the subscriber's own identity on + /// the way out. + /// + [Fact] + public void RunAsSystemRunsTheWorkAsSystemAndHandsBackTheCallersIdentity() + { + var access = new AccessService(); + access.SetContext(Alice); + + string? seenByTheWork = null; + string? seenByTheSubscriber = null; + + access + .RunAsSystem(() => Observable.Defer(() => + { + seenByTheWork = access.Context?.ObjectId; + return Observable.Return(1); + })) + .Subscribe(_ => seenByTheSubscriber = access.Context?.ObjectId); + + seenByTheWork.Should().Be(System, + "the scope must still cover the work — sealing it must not make a system read fail " + + "closed, which would trade one defect for another"); + seenByTheSubscriber.Should().Be("alice", + "the identity was established FOR that read; it is never an inheritance"); + } + + /// + /// The half that matters most: the leak reaches whatever the subscriber BUILDS, because the mesh + /// write primitives capture the ambient when they are called. This is the shape the install path + /// had. + /// + [Fact] + public void WhatTheSubscriberCOMPOSESNoLongerCapturesSystem() + { + var access = new AccessService(); + access.SetContext(Alice); + + // Stands in for a write primitive: it snapshots the ambient identity at CALL time, exactly + // as CreateNode/CreateNodes/Update do, and that snapshot is what would be authorised. + IObservable WriteCapturingCallerIdentity() + { + var capturedWhenCalled = access.Context?.ObjectId; + return Observable.Return(capturedWhenCalled); + } + + var leaked = Observable + .Using(access.ImpersonateAsSystem, _ => Observable.Return(1)) + .SelectMany(_ => WriteCapturingCallerIdentity()) + .Wait(); + + var sealedOff = access + .RunAsSystem(() => Observable.Return(1)) + .SelectMany(_ => WriteCapturingCallerIdentity()) + .Wait(); + + leaked.Should().Be(System, "the unsealed idiom is what filed #1444"); + sealedOff.Should().Be("alice", + "the write a user's operation composes must be authorised and attributed as the USER — " + + "System silently succeeding where the user might have been refused is the serious half"); + } + + /// + /// Terminal notifications are composed on too (Catch, Finally, a completion arm + /// that writes a manifest), so the seal has to cover them or it just moves the leak. + /// + [Fact] + public void TheSealCoversErrorAndCompletionToo() + { + var access = new AccessService(); + access.SetContext(Alice); + + string? onCompleted = null; + access.RunAsSystem(() => Observable.Empty()) + .Subscribe(_ => { }, () => onCompleted = access.Context?.ObjectId); + + string? onError = null; + access.RunAsSystem(() => Observable.Throw(new InvalidOperationException("boom"))) + .Subscribe(_ => { }, _ => onError = access.Context?.ObjectId, () => { }); + + onCompleted.Should().Be("alice", "a completion arm that writes must write as the caller"); + onError.Should().Be("alice", "so must an error arm that records the failure"); + } + + /// + /// 🚨 The property that makes adopting this safe at an existing call site: a subscriber with NO + /// identity keeps whatever the framework leaves it. Nothing is invented and nothing is clamped, + /// so a background flow that never had a user is not fail-closed by the seal. + /// + [Fact] + public void ASubscriberWithNoIdentityIsLeftExactlyAsItWas() + { + var access = new AccessService(); + access.SetContext(null); + + string? seen = null; + access.RunAsSystem(() => Observable.Return(1)) + .Subscribe(_ => seen = access.Context?.ObjectId); + + seen.Should().Be(System, + "with nothing to restore, the seal is a pass-through — clamping to null here would " + + "fail-close background flows that never had a user, which is not what #1444 is about"); + } + + /// + /// Nested scopes restore to the ENCLOSING identity, not to a null or to the process default — + /// the case a system-initiated install hits (System all the way down, and correctly so). + /// + [Fact] + public void ASystemInitiatedFlowStaysSystem() + { + var access = new AccessService(); + access.SetContext(Alice); + + string? inner = null; + using (access.ImpersonateAsSystem()) + { + access.RunAsSystem(() => Observable.Return(1)) + .Subscribe(_ => inner = access.Context?.ObjectId); + } + + inner.Should().Be(System, + "the seal restores the SUBSCRIBER's identity, which here genuinely is System — a " + + "system-initiated install must not be rewritten into a user's"); + access.Context?.ObjectId.Should().Be("alice"); + } + + /// + /// A host without an (minimal test fixtures) must still run the work, + /// and must still run it COLD — the side effect belongs to Subscribe, not to composition. + /// + [Fact] + public void WithoutAnAccessServiceTheWorkStillRunsAndStaysCold() + { + AccessService? none = null; + var runs = 0; + + var pipeline = none.RunAsSystem(() => + { + runs++; + return Observable.Return(1); + }); + + runs.Should().Be(0, "composition must not run the work — every framework primitive is cold"); + pipeline.Wait().Should().Be(1); + runs.Should().Be(1); + } + + /// + /// on an already-composed chain — + /// the case where the impersonated region is several calls and it is the LAST one's emission the + /// caller builds on. + /// + [Fact] + public async Task ContainIdentitySealsAnAlreadyComposedChainAcrossThreads() + { + var access = new AccessService(); + access.SetContext(Alice); + + var source = new Subject(); + var seen = new List(); + var done = new TaskCompletionSource(); + + source.ContainIdentity(access).Subscribe(_ => + { + seen.Add(access.Context?.ObjectId); + done.TrySetResult(true); + }); + + // Emit from a thread that never had the AsyncLocal set AND is inside a system scope, which is + // what a hub action block running an impersonated write looks like from the subscriber's side. + await Task.Run(() => + { + using (access.ImpersonateAsSystem()) + source.OnNext(1); + }, TestContext.Current.CancellationToken); + + await done.Task; + seen.Should().ContainSingle().Which.Should().Be("alice", + "the emitting side's identity is its own business — what the SUBSCRIBER observes is the " + + "identity it subscribed with"); + } +}