Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 1 addition & 2 deletions src/MeshWeaver.GitSync/GitHubActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ private static IObservable<string> TriggerAuthorizedAsSystem(
if (string.IsNullOrEmpty(userId) || caller?.IsVirtual == true)
userId = WellKnownUsers.Anonymous;

IObservable<string> AsSystem() =>
Observable.Using(() => accessService.ImpersonateAsSystem(), _ => runActivity());
IObservable<string> AsSystem() => accessService.RunAsSystem(runActivity);

if (string.Equals(userId, WellKnownUsers.System, StringComparison.Ordinal))
return AsSystem();
Expand Down
4 changes: 1 addition & 3 deletions src/MeshWeaver.GitSync/SystemOwnedAccessRetractionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,5 @@ private IObservable<Unit> RetractAll(string partition, IList<MeshNode> grants)
/// <summary>Establishes the System identity on the write's own subscribe thread — mirrors
/// <c>MeshExtensions.AsSystem</c>, so the deletes are attributed to the platform.</summary>
private IObservable<T> AsSystem<T>(Func<IObservable<T>> write)
=> accessService is null
? Observable.Defer(write)
: Observable.Using(() => accessService.ImpersonateAsSystem(), _ => write());
=> accessService.RunAsSystem(write);
}
2 changes: 1 addition & 1 deletion src/MeshWeaver.Graph/AccessGrantNotifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ internal static string ResolveGrantedNode(MeshNode assignmentNode)
}

private IObservable<T> AsSystem<T>(Func<IObservable<T>> factory)
=> Observable.Using(accessService.ImpersonateAsSystem, _ => Observable.Defer(factory));
=> accessService.RunAsSystem(factory);

public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;

Expand Down
2 changes: 1 addition & 1 deletion src/MeshWeaver.Graph/EventSubscriptionRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ private void MigrateLegacyScheduledActions()
/// identity — the runner has no ambient AccessContext, and both reads and writes capture identity
/// at construction/subscribe. <c>Defer</c> moves construction inside the impersonation scope.</summary>
private IObservable<T> AsSystem<T>(Func<IObservable<T>> factory)
=> Observable.Using(accessService.ImpersonateAsSystem, _ => Observable.Defer(factory));
=> accessService.RunAsSystem(factory);

/// <inheritdoc />
public Task StopAsync(CancellationToken cancellationToken)
Expand Down
7 changes: 1 addition & 6 deletions src/MeshWeaver.Graph/StaticRepoImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1602,12 +1602,7 @@ private static MeshNode Materialize(MeshNode node)
/// (which the sync-stream post then carries). See Doc/Architecture/AccessContextPropagation.md.
/// </summary>
private static IObservable<T> AsSystem<T>(IMessageHub hub, Func<IObservable<T>> write)
{
var access = hub.ServiceProvider.GetService<AccessService>();
return access is null
? Observable.Defer(write)
: Observable.Using(() => access.ImpersonateAsSystem(), _ => write());
}
=> hub.ServiceProvider.GetService<AccessService>().RunAsSystem(write);

/// <summary>
/// Surfaces a boot/startup import failure LOUDLY in the GUI. The activity at
Expand Down
5 changes: 1 addition & 4 deletions src/MeshWeaver.InstanceSync/InstanceSyncService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,5 @@ public bool ContentEquals(MeshNode? a, MeshNode? b)
/// the user's own context and never go through here.
/// </summary>
public IObservable<T> AsSystem<T>(Func<IObservable<T>> operation)
{
var accessService = hub.ServiceProvider.GetRequiredService<AccessService>();
return Observable.Using(() => accessService.ImpersonateAsSystem(), _ => operation());
}
=> hub.ServiceProvider.GetRequiredService<AccessService>().RunAsSystem(operation);
}
4 changes: 1 addition & 3 deletions src/MeshWeaver.Mesh.Contract/MeshExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2034,9 +2034,7 @@ root is not null
/// infrastructure-write). Mirrors <c>StaticRepoImporter.AsSystem</c>.
/// </summary>
private static IObservable<T> AsSystem<T>(AccessService? access, Func<IObservable<T>> write)
=> access is null
? Observable.Defer(write)
: Observable.Using(() => access.ImpersonateAsSystem(), _ => write());
=> access.RunAsSystem(write);

/// <summary>
/// True if the exception (or any inner) reports an "already exists" outcome — the idempotent-create
Expand Down
16 changes: 16 additions & 0 deletions src/MeshWeaver.Messaging.Hub/AccessContextCaptureExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,22 @@ public static IObservable<T> CarryAccessContext<T>(
return new CarryAccessContextObservable<T>(source, access, captured);
}

/// <summary>
/// Subscribes <paramref name="observer"/> to <paramref name="source"/> with every notification
/// delivered inside a <see cref="AccessService.SwitchAccessContext"/> scope keyed to
/// <paramref name="captured"/> — the same per-callback restore
/// <see cref="CarryAccessContext{T}(System.IObservable{T},MeshWeaver.Messaging.AccessService,bool)"/> applies.
///
/// <para>🚨 Exists so <see cref="ImpersonationScopeExtensions"/> 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, <c>CarryAccessContext</c>
/// snapshots the ambient at composition time, and <c>ContainIdentity</c> reads it at Subscribe.
/// Keeping one restore implementation is what stops those three drifting apart.</para>
/// </summary>
internal static IDisposable SubscribeRestoring<T>(
IObservable<T> source, IObserver<T> observer, AccessService access, AccessContext? captured) =>
source.Subscribe(new RestoringObserver<T>(observer, access, captured));

/// <summary>
/// Wraps a cold observable so every emission to the subscriber happens
/// inside an <see cref="AccessService.SwitchAccessContext"/> scope keyed
Expand Down
114 changes: 114 additions & 0 deletions src/MeshWeaver.Messaging.Hub/ImpersonationScopeExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
using System.Reactive.Linq;

namespace MeshWeaver.Messaging;

/// <summary>
/// 🚨 THE framework's impersonation boundary: a scope that <b>cannot escape the operation it was
/// opened for</b>.
///
/// <para><b>The defect this closes (issue #1444).</b> The idiomatic system read/write is
/// <c>Observable.Using(() =&gt; access.ImpersonateAsSystem(), _ =&gt; work)</c>, and it is correct for
/// <i>the work itself</i>. It is not self-contained. Rx forwards <c>OnNext</c> to the subscriber
/// <b>before</b> <c>Using</c> 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 <see cref="AccessService.Context"/> when
/// they are CALLED and re-stamp it around their own emissions
/// (<see cref="AccessContextCaptureExtensions.CarryAccessContext{T}(System.IObservable{T},System.IServiceProvider,bool)"/>),
/// so the System identity does not merely survive one hop — it is <b>re-acquired</b> at every hop
/// after it.</para>
///
/// <para><b>This is not hypothetical, and it has already reached an authorization decision.</b>
/// <c>AccessControlPipeline.HandleGetPermission</c> carries a comment describing exactly this shape:
/// <c>SecurityService</c>'s bootstrap-time <c>ImpersonateAsSystem</c> leaked past its using-block
/// onto the action-block thread, and trusting the ambient there "returned <c>Permission.All</c> 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.</para>
///
/// <para><b>What it does NOT do: invent an identity.</b> <see cref="ContainIdentity{T}"/> restores
/// exactly what was ambient at Subscribe — a real user for a user-driven flow, <c>system-security</c>
/// for a genuinely system-initiated one, and <b>nothing at all</b> 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 <c>restoreNullCapture: false</c> 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.</para>
/// </summary>
public static class ImpersonationScopeExtensions
{
/// <summary>
/// Runs <paramref name="work"/> 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 <c>Observable.Using</c>, never <c>Defer</c>-plus-<c>using</c>, rule),
/// and every notification that reaches the subscriber is delivered under the subscriber's OWN
/// identity, so nothing composed downstream inherits System.
///
/// <para>Prefer this over a hand-written <c>Observable.Using(access.ImpersonateAsSystem, …)</c>
/// at every site that RETURNS the scoped observable to a caller — returning it is precisely what
/// turns a scope into an inheritance.</para>
///
/// <para>A null <paramref name="access"/> (minimal test hosts) runs the work unimpersonated —
/// deferred, so it stays cold.</para>
/// </summary>
/// <typeparam name="T">What the scoped operation emits.</typeparam>
/// <param name="access">The access service, or null on a host without one.</param>
/// <param name="work">The cold operation to run as System.</param>
public static IObservable<T> RunAsSystem<T>(this AccessService? access, Func<IObservable<T>> work) =>
access is null
? Observable.Defer(work)
: Observable
.Using(access.ImpersonateAsSystem, _ => Observable.Defer(work))
.ContainIdentity(access);

/// <summary>
/// Runs <paramref name="work"/> as <paramref name="hub"/>'s own identity, sealed exactly as
/// <see cref="RunAsSystem{T}"/> is.
/// </summary>
/// <typeparam name="T">What the scoped operation emits.</typeparam>
/// <param name="access">The access service, or null on a host without one.</param>
/// <param name="hub">The hub whose address is stamped as the principal.</param>
/// <param name="work">The cold operation to run as the hub.</param>
public static IObservable<T> RunAsHub<T>(
this AccessService? access, IMessageHub hub, Func<IObservable<T>> work) =>
access is null
? Observable.Defer(work)
: Observable
.Using(() => access.ImpersonateAsHub(hub), _ => Observable.Defer(work))
.ContainIdentity(access);

/// <summary>
/// Seals an ALREADY-COMPOSED <paramref name="source"/>: every notification is delivered to the
/// subscriber under the identity that was ambient when they subscribed, whatever identity
/// produced the emission.
///
/// <para>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.</para>
///
/// <para>The restore is per-notification (entered on <c>OnNext</c>/<c>OnError</c>/
/// <c>OnCompleted</c>, disposed as the callback returns), so nothing is left behind on the
/// emitting thread — the same shape <c>CarryAccessContext</c> uses, and the same code.</para>
/// </summary>
/// <typeparam name="T">What the operation emits.</typeparam>
/// <param name="source">The already-composed operation whose identity must not escape.</param>
/// <param name="access">The access service, or null on a host without one.</param>
public static IObservable<T> ContainIdentity<T>(this IObservable<T> source, AccessService? access) =>
access is null ? source : new ContainedIdentityObservable<T>(source, access);

/// <summary>
/// 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.
/// </summary>
private sealed class ContainedIdentityObservable<T>(IObservable<T> source, AccessService access)
: IObservable<T>
{
public IDisposable Subscribe(IObserver<T> observer)
{
var caller = access.Context;
return caller is null
? source.Subscribe(observer)
: AccessContextCaptureExtensions.SubscribeRestoring(source, observer, access, caller);
}
}
}
5 changes: 1 addition & 4 deletions src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.</para></summary>
private IObservable<T> AsSystem<T>(Func<IObservable<T>> write)
{
var accessService = hub.ServiceProvider.GetRequiredService<AccessService>();
return Observable.Using(() => accessService.ImpersonateAsSystem(), _ => write());
}
=> hub.ServiceProvider.GetRequiredService<AccessService>().RunAsSystem(write);

// ══════════════════════════════════════════════════════════════════════════
// Making it visible
Expand Down
Loading
Loading