fix: seal an impersonation scope into the operation it was opened for (#1444) by rbuergi · Pull Request #1614 · Systemorph/MeshWeaver

rbuergi · 2026-08-14T19:47:38Z

Closes #1444.

What was established first

The issue asks to establish where the identity is acquired, whether it is load-bearing, and whether _UserActivity's correct attribution is the anomaly or the norm — before changing anything. All three, in order:

Where. Not in an ImpersonateAsSystem scope anyone wrote around the writes — that observation in the issue is correct. The idiom Observable.Using(access.ImpersonateAsSystem, _ => work) 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 — the next phase, a write, a permission check — is built while the impersonation is still open.

Why it does not stop at one hop. The mesh write primitives eager-capture AccessService.Context when they are CALLED and re-stamp it around their own emissions (CarryAccessContext). So a leaked System identity is not inherited once — it is re-acquired at every hop after it. That is a whole install chain, from one scope.

_UserActivity is the norm, not the anomaly. It is the one write on that path posted directly from the request thread rather than composed on an emission. It behaved as documented because nothing had leaked into it. Every other write was downstream of a system scope.

The first test in this PR pins the raw Rx ordering all of that rests on, so if a future Rx ever disposes before forwarding, that fact is learned from a red test rather than from a production log.

Why this is fixed at the root and not in the log

The issue separates attribution from authorization and asks for the second to be established first. It is not hypothetical, and the framework already carries the scar:

AccessControlPipeline.HandleGetPermission — "That AsyncLocal at handler-entry holds system-security from SecurityService's bootstrap-time ImpersonateAsSystem scope (it leaks past the using-block because the bootstrap action-block thread captured the context at construction). Trusting it 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 PR 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+using, rule) and every notification reaches the subscriber 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 attaches to a pipeline cannot drift apart.

// ❌ the write is CONSTRUCTED while the scope is still open → lands as system-security
Observable.Using(access.ImpersonateAsSystem, _ => ReadGatedSource())
    .SelectMany(rows => meshService.CreateNodes(Plan(rows)));

// ✅ the scope covers the read and stops at its boundary → lands as the CALLER
access.RunAsSystem(ReadGatedSource)
    .SelectMany(rows => meshService.CreateNodes(Plan(rows)));

🚨 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. 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 by it. The only behaviour that changes is the one the issue describes.

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 — several carrying a comment saying they mirror one of the others. They now delegate to the one sealed primitive.

What this PR deliberately does NOT claim

The reported install path lives in the Plugins repo and is already fixed there (Store/Plugin/Source/SystemIdentity.cs, which cites this issue). Its analysis settles the authorization question for that path: nothing landed anywhere the learner could not have written — every path was under their own partition root, where onboarding had just granted them Admin — so the realised damage is an audit trail saying the platform did what a learner asked for. The one permission check on that path resolved its subject from the same ambient context, and no shipped path handed it a user-chosen destination; that is a fact about Rx's disposal order rather than a defence anyone designed, which is why it is fixed anyway.

That plugin-local helper is what this framework primitive should have been all along. This PR is the framework half.

Verification


`SystemScopeDoesNotEscapeTest`	8/8 — including `leaked == system-security` vs `sealed == alice` on the same composed write
`MeshWeaver.Messaging.Hub.Test`	251/251
`MeshWeaver.AccessControl.Test`	29/29
`MeshWeaver.InstanceSync.Test`	26/26
`MeshWeaver.GitSync.Test`	163/163
`MeshWeaver.Graph.Test`	1127/1127

-c Release -warnaserror clean for all six touched projects.

…#1444) 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmBFaHTqhy7h742wkAaJKj

Copilot

Pull request overview

This PR fixes an identity-propagation bug where Observable.Using(access.ImpersonateAsSystem, ...) could leak the System identity into downstream composition, causing subsequent writes/permission checks to be attributed/authorized as system-security instead of the caller. It introduces a sealed impersonation boundary (RunAsSystem / RunAsHub / ContainIdentity), updates existing “AsSystem” helpers to use it, and pins the underlying Rx ordering with targeted tests and documentation.

Changes:

Add ImpersonationScopeExtensions to seal impersonation scopes so the work runs under the impersonated identity but downstream observers see (and compose under) the subscriber’s identity.
Replace several hand-rolled AsSystem helpers across the codebase to delegate to RunAsSystem.
Add a comprehensive regression test suite and update docs/What’s New to codify the new “impersonation must not escape” rule.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/MeshWeaver.Messaging.Hub.Test/SystemScopeDoesNotEscapeTest.cs	Adds regression tests that pin Rx `Using` ordering and validate the new sealing semantics across success/error/completion and cross-thread emissions.
src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs	Introduces the sealed impersonation boundary APIs (`RunAsSystem`, `RunAsHub`, `ContainIdentity`).
src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs	Exposes an internal “restore around notifications” helper so sealing reuses the same restoring observer semantics as `CarryAccessContext`.
src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs	Replaces local `AsSystem` helper with `AccessService.RunAsSystem(...)`.
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs	Replaces local `AsSystem` helper with `access.RunAsSystem(...)`.
src/MeshWeaver.InstanceSync/InstanceSyncService.cs	Replaces local `AsSystem` helper with `AccessService.RunAsSystem(...)`.
src/MeshWeaver.Graph/StaticRepoImporter.cs	Replaces local `AsSystem` helper with `RunAsSystem(...)` (null-safe).
src/MeshWeaver.Graph/EventSubscriptionRunner.cs	Switches system-scoped factory wrapper to `RunAsSystem(...)`.
src/MeshWeaver.Graph/AccessGrantNotifier.cs	Switches system-scoped factory wrapper to `RunAsSystem(...)`.
src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs	Replaces local `AsSystem` helper with `RunAsSystem(...)`.
src/MeshWeaver.GitSync/GitHubActivityExtensions.cs	Uses `RunAsSystem(...)` for system-authorized activity triggering.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-14-your-name-stays-on-what-you-asked-for.md	Adds a release note describing the user-visible/audit and authorization impact.
src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md	Documents the “impersonation scope must not escape” rule and the new APIs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+
+        await done.Task;


github-actions · 2026-08-14T20:01:15Z