diff --git a/src/MeshWeaver.Documentation/Data/Architecture.md b/src/MeshWeaver.Documentation/Data/Architecture.md index b0417274c0..5c683b4244 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture.md +++ b/src/MeshWeaver.Documentation/Data/Architecture.md @@ -70,7 +70,7 @@ The catalog below lists every page; these are the load-bearing reads per theme. | **Reactive core** | [Asynchronous Calls](AsynchronousCalls) — the no-`await` rulebook | [Actor Model](ActorModel) · [Controlled I/O Pooling](ControlledIoPooling) · [Subscription Ownership](SubscriptionOwnership) — a pending timer is a GC root · [Silent Completion](SilentCompletion) — an empty completion is invisible to every timeout · [AsyncLocal Across Scheduler Hops](AsyncLocalAcrossHops) · [Initialization Gates](InitializationGates) · [Aggregating Providers](AggregatingProviders) · [Hub Disposal Model](HubDisposalModel) · [Transient Node Probes](TransientNodeProbes) — a probe hub's own address is not a node | | **Reading & writing nodes** | [CQRS — Queries vs. Content Access](CqrsAndContentAccess) | [MeshNode Stream Cache](MeshNodeStreamCache) · [Request via Stream Update](RequestViaStreamUpdate) · [Data Access Patterns](DataAccessPatterns) · [Workspace References](WorkspaceReferences) · [Content Chunk Navigation](ContentChunkNavigation) · [Moved-Node Redirects](NodeRedirects) — keeping links alive after a move · [MainNode and Rebasing](MainNodeRebasing) — a `with { Namespace = … }` copy un-lists a node with nothing logged · [Write Verdict Totality](WriteVerdictTotality) — a write whose base read ends empty answers nobody, and arms no deadline either · [Live Mirrors and the Change Feed](LiveMirrorsAndTheChangeFeed) — a write must not end its own streams | | **Storage & partitions** | [Postgres Schema Architecture](PostgresSchemaArchitecture) | [Partition Storage Routing](PartitionStorageRouting) · [Partition Storage Hubs](PartitionStorageHubs) · [Partitioned Persistence](PartitionedPersistence) · [Storage Adapter Implementation](StorageAdapterImplementation) · [Change-Feed Isolation](ChangeFeedIsolation) — one throwing subscriber must never starve the others · [Static Repo Import](StaticRepoImport) · [Import Write Ordering](ImportWriteOrdering) — a NodeType lands before the instances that name it · [Vector Search](VectorSearch) · [Durable But Unreadable](DurableButUnreadable) — a write that is acknowledged, versioned and invisible | -| **Security** | [Access Control](AccessControl) | [Granting Access](GrantingAccess) · [AccessContext Propagation](AccessContextPropagation) · [Query Identity](QueryIdentity) — an unstamped read answers as Anonymous, which reads as absence · [Owner Injection](OwnerInjection) · [Permission API](PermissionApi) · [Invitation-Only Onboarding](InvitationOnlyOnboarding) · [Logon Actions](LogonActions) — per-user work at logon, run as the user · [Unanchored Security Reads](UnanchoredSecurityReads) — why the permission fold reads mesh-wide, and why pinning it to the viewer's partition is a silent revocation-fails-open bug | +| **Security** | [Access Control](AccessControl) | [Granting Access](GrantingAccess) · [AccessContext Propagation](AccessContextPropagation) · [Query Identity](QueryIdentity) — an unstamped read answers as Anonymous, which reads as absence · [Owner Injection](OwnerInjection) · [Permission API](PermissionApi) · [Invitation-Only Onboarding](InvitationOnlyOnboarding) · [Logon Actions](LogonActions) — per-user work at logon, run as the user · [Unanchored Security Reads](UnanchoredSecurityReads) — why the permission fold reads mesh-wide, and why pinning it to the viewer's partition is a silent revocation-fails-open bug · [A Denial Is an Answer](DenialIsAnAnswer) — a check on a hub with no evaluator grants Permission.All, and a refusal the mesh decided is rendered, never raised | | **Threads, activities & AI** | [Thread Operations](ThreadOperations) | [Thread Execution Streaming](ThreadExecutionStreaming) · [Activity Control Plane](ActivityControlPlane) · [Activity Operations](ActivityOperations) · [Notifications](Notifications) · [Agentic AI](AgenticAI) · [Script Execution](ScriptExecution) | | **UI** | [User Interface](UserInterface) | [Blazor Data Binding](BlazorDataBinding) · [Blazor Async](BlazorAsync) · [Available Controls](UserInterface/AvailableControls) · [Menus as Data](MenuAsData) — re-word a menu without a build · [Markdown Fence Extensions](MarkdownFenceExtensions) — the platform emits a marker, the clients hydrate it; a new fence is always a two-repo change | | **Node types** | [Adding a New Node Type](AddingANewNodeType) | [Retiring a NodeType](RetiringANodeType) — the prune keeps the definition and deletes its sources · [Dangling NodeTypes](DanglingNodeTypes) — a node whose type resolves to nothing, and the two write paths that allowed it · [Node Type Compilation](NodeTypeCompilation) · [Execute-Time Interlock](ExecuteTimeInterlock) — a build proven stale is never armed · [Graph / Compiler Layering](GraphCompilerLayering) — the four assemblies, the cycle, and the full-MVID size rule · [Toolchain Re-evaluation Lane](ToolchainReevaluationLane) — why a toolchain change stopped rebaking the world · [Rebake Waves](RebakeWaves) — why a roll rebakes the world anyway, and what one rebake writes · [Import Write Ordering](ImportWriteOrdering) — type before instance, and what a foreign type does · [Language Services](LanguageServices) · [Extensible Defaults](ExtensibleDefaults) | diff --git a/src/MeshWeaver.Documentation/Data/Architecture/DenialIsAnAnswer.md b/src/MeshWeaver.Documentation/Data/Architecture/DenialIsAnAnswer.md new file mode 100644 index 0000000000..bc47e9f17d --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/Architecture/DenialIsAnAnswer.md @@ -0,0 +1,191 @@ +--- +Name: A Denial Is an Answer +Category: Architecture +Description: A permission check on a hub that carries no evaluator grants Permission.All — which is how every session surface (MCP, REST, gRPC, CLI) shipped a pre-flight that could not fail. Plus the rule the crash on the other side taught: a refusal the mesh decided is rendered as the operation's answer, never raised as a fault. +Icon: +--- + +# A Denial Is an Answer + +Two rules, one incident ([#3121](https://github.com/Systemorph/MeshWeaver/issues/3121)). + +🚨 **A hub's configuration inherits NOTHING, and a permission check on a hub with no evaluator +answers `Permission.All`.** So a gate written on the wrong hub is not a weak gate — it is a gate +that cannot fail. + +🚨 **A refusal the mesh DECIDED is an answer the operation renders.** Deciding is the operation +succeeding at its job. Raising it as an exception tells the caller the tool is broken. + +## The incident + +An MCP caller without write access called `recycle` on `Store/Catalog`. The mesh refused — +correctly — and the client got this: + +``` +fail: ModelContextProtocol.Server.McpServer[1433779783] + "recycle" threw an unhandled exception. + System.UnauthorizedAccessException: Access denied: user 'rbuergi' lacks Update permission on 'Store/Catalog' + at MeshWeaver.Mcp.McpToolResult.AsToolResult[T](…) +``` + +`MeshOperations.Recycle` has carried an actionable refusal envelope since #2901 — *"Recycle +requires Update permission on the target node … ask someone with write access"* — gated on +`hub.CheckPermissionOutcome(path, Update)`. It never ran. Not on this call, and not on any call +through any session surface, since the day it was written. + +## Why the pre-flight could not fail + +`HubPermissionExtensions.ResolveEvaluator` is three lines: + +```csharp +private static EffectivePermissionsDelegate ResolveEvaluator(IMessageHub hub) => + hub.Configuration.Get() + ?? MessageHubPermissionExtensions.DefaultEvaluator; // (_, _, _) => Observable.Return(Permission.All) +``` + +It does not walk the parent chain — and `MessageHubExtensions.CreateMessageHub` builds a **fresh** +`MessageHubConfiguration` for every hub, inheriting nothing. `AddRowLevelSecurity()` installs the +evaluator on exactly two places: the **mesh hub** and every **per-node hub**. A session hub is +neither. + +`SessionHubFactory` is the one factory behind every API surface — MCP, REST, gRPC, the CLI, and the +headless local sidecar all issue their operations on a `portal/{prefix}-{session}-{instance}` hub it +materialises. It never copied the evaluator. So every client-side check those surfaces issued +answered *granted*, for every caller, on every path. + +Measured on a real mesh, a `Viewer` (`Read|Execute|Api`) asking the SAME question two ways: + +| Asked on | `Update` on a node the Viewer may only read | +|---|---| +| the session hub (`portal/mcp-…`) | **granted** | +| the mesh hub | denied | + +**For a `PatchDataRequest` this cost only legibility** — the owner's `AccessControlPipeline` was, and +remains, the authority on that write, and it fails closed. But not every operation's authorization +runs there. Measured before and after, same mesh, same Viewer, through a real session hub: + +| Operation | Before | After | +|---|---|---| +| `recycle` a **NodeType** | `THREW UnauthorizedAccessException` — the reported bug | the refusal envelope | +| `recycle` a **plain node** | `{"status":"Recycled"}` — **the `DisposeRequest` went out** | the refusal envelope | +| `create` | `Created: …` — **the node was written** | `Access denied: Create permission required` | +| `export` | every node in the subtree | only nodes the caller may `Export` | + +### The three that were more than legibility + +**`recycle` on a plain node.** Recycle is two halves of one operation: stamp a release request, then +dispose the hub. The [2026-08-30 fix](/Doc/WhatsNew/2026-08-30-a-refused-recycle-no-longer-tears-the-hub-down) +made a refused stamp refuse the whole recycle. But the stamp only writes anything on a **NodeType** +node — on anything else the update is the identity function, so nothing was posted, nothing was +gated, and the destructive half ran for free. Authorization that is a *side effect* of a write +disappears exactly when the write does. + +**`create`.** `AddRowLevelSecurity()` registers `RlsNodeValidator` as a **scoped** service, and it +asks `hub.CheckPermission(...)` on whichever hub it was resolved against. A session hub opts into +`WithNodeOperationExecution()`, so node CRUD runs *there* rather than on the shared +`NodeOperationExecutionHub` — which is the one place that had been taught to copy the evaluator. +`RlsNodeValidator` therefore granted every create issued through a session, which is not a +legibility problem at all. `MeshExtensions.NodeOperationExecutionHub`'s comment predicted this +exactly — *"moving node CRUD onto a hub that did not copy the evaluator would make RlsNodeValidator +grant EVERY write"* — and named the hazard for its own hub without noticing that the session hub had +the same one. + +**`export`.** `MeshOperations.Export` filters the subtree per node against the caller's `Export` +permission, and its own comment said the check "runs on the mesh hub". It did not: it resolves +`hub.ServiceProvider.GetRequiredService()`, which on a session surface hands back the +**session** hub. The one thing standing between an MCP `export` and a subtree the caller may not +read answered *granted* for every node. + +## The fix, and why it is two halves + +**Half one — `SessionHubFactory` copies the mesh hub's evaluator into every session hub.** The +pre-flight becomes a real gate, so a refusal is the normal, legible path and the destructive half +never runs. `MeshExtensions.NodeOperationExecutionHub` had already been fixed this way, for exactly +this reason, and its comment spells the hazard out; the session hub simply never got the same +treatment. + +Nothing is widened by this: the owner already refused everything the pre-flight now refuses. A mesh +deliberately built **without** RLS stays ungated, because there is then no delegate to copy. + +🚨 **Waking a gate means matching what the owner asks, not just asking something.** The owner's +delivery gate stopped being a raw `(path, permission)` fold in +[#3061/#3100](https://github.com/Systemorph/MeshWeaver/issues/3061): on a definitive denial it +re-decides through the node's registered `INodeTypeAccessRule`, which for every satellite type says +*a satellite's write is `Update` on its MainNode*. `Recycle`'s pre-flight was a raw check, and +switching it on without that second opinion would have **created** a false refusal — an editor +entitled through a satellite's MainNode, whom the owner grants, refused by a pre-flight that only +knows the path. It now takes the same second opinion, from the same `NodeTypeAccessRuleGate` +helpers, on the same condition (a definitive denial only — never an undetermined fold, which must +stay "we could not check"). Pinned by `RecyclingASatelliteFollowsItsNodeTypesRule_NotTheRawPath`, +whose fixture points a satellite's `MainNode` into another partition: a satellite living *under* its +main node inherits that node's grants by path, so raw and rule agree by construction and the case +would prove nothing. + +**Half two — the owner's verdict is rendered, not raised.** A pre-flight is check-then-act. It runs +on a different hub, at an earlier moment, against a permission fold that can have moved on: an +assignment revoked in between, a cross-silo fold, an identity lost across a scheduler hop. The owner +stays the authority, and when it refuses, `RecycleCore` now answers in the operation's own envelope +— the same sentence the pre-flight denial uses, because the caller should not be able to tell which +evaluator refused them, nor be told two different things about one fact. + +## 🚨 A verdict and a non-verdict are not the same answer + +Half two is where this change could have made things worse. "You lack permission" is actionable and +final; "I could not evaluate access" is transient and fail-closed. Collapsing the second into the +first sends a correctly-entitled caller to request rights they already hold — the lie [#974](https://github.com/Systemorph/MeshWeaver/issues/974) +removed from `AccessControlPipeline` and #2901 removed from `MeshOperations`' own pre-flight. + +`MeshOperations.IsWriteDenial` is the seam, and it decides on the **typed** failure — never by +matching a message, which drifts the moment someone rewords a banner: + +| Fault | Denial? | Because | +|---|---|---| +| `UnauthorizedAccessException` | **yes** | the write path mints it for one thing only: `DeliveryFailure{Unauthorized}`, which the pipeline posts only after a decisive refusal | +| `DeliveryFailureException{Unauthorized}` | **yes** | the same verdict, before `UpdateRemote` maps it | +| `DeliveryFailureException{ShuttingDown}` | no | the activation is going away — [re-probe](/Doc/Architecture/RidingOutAShuttingDownAddress), do not go asking for rights | +| `DeliveryFailureException{Unavailable}` | no | the fold reached no verdict (#974) | +| `TimeoutException`, anything else | no | **the default must never accuse** | + +That the tri-state survives the hop *as a type distinction* is not an accident: +`AccessControlPipeline` projects it onto the bus vocabulary exactly once, and +`MeshNodeStreamHandle.UpdateRemote` keeps the split by raising `MeshNodeStreamException` — not +`UnauthorizedAccessException` — for the other two. This is the write-side twin of +`NodeReadOutcome.FromReadFailure`, which does the same job for reads. + +## Writing a new operation + +- **Ask the permission question on a hub that can answer it.** If you are writing a + `CheckPermission` / `CheckPermissionOutcome` call, know which hub it lands on. A session hub now + answers correctly; a hub you build yourself does not, unless you copy the evaluator. +- **A pre-flight asks what the OWNER asks, or it is a new bug.** A raw `(path, permission)` fold is + not what the delivery gate does any more. Take the same `INodeTypeAccessRule` second opinion, on + the same condition — a definitive denial only. +- **Never let authorization be a side effect of a write.** Decide, then act. A stamp that happens to + be gated is not a gate — it stops being one the day the stamp becomes a no-op. +- **A refusal is a return value.** Render it in the operation's envelope. The tool transport's + `IObservable` → `Task` bridge is not an error handler, and an exception there reaches the client + as "the tool is broken". +- **Never widen a non-verdict into a denial.** Route it through `IsWriteDenial`, or say honestly + that you could not find out. + +## Not localised, deliberately + +The refusal sentences on this surface are JSON envelope fields on the **machine** tool surface (MCP, +REST, CLI), consumed by an agent runtime that reasons about them and renders its own text to the +human. They sit on the model side of the line [Localization](../Localization) draws around +tool-facing text — the same side as an LLM tool parameter's `[Description]` — and localising them +would make the answer depend on whichever locale happened to be attached to a session. The +human-facing surfaces render their own refusal — the portal's typed error card, via +`AreaErrorClassifier` — and that one **is** localised. + +## Related + +- [Access Control](../AccessControl) — the permission model these gates evaluate. +- [AccessContext Propagation](../AccessContextPropagation) — how the caller's identity reaches the + gate in the first place; a lost context is the other way a pre-flight and an owner disagree. +- [An Unreachable Store Is Not a Refusal](../StoreUnreachableIsNotARefusal) — the same + verdict/non-verdict rule, on the storage side. +- [Unanchored Security Reads](../UnanchoredSecurityReads) — why "no result" and "not allowed" are + the same value inside the fold, and what that forbids. +- [Error Propagation & Wedges](../ErrorPropagationAndWedges) — what an unhandled fault costs + downstream of a tool surface. diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-being-refused-now-says-so-instead-of-looking-broken.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-being-refused-now-says-so-instead-of-looking-broken.md new file mode 100644 index 0000000000..b84a78781c --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-being-refused-now-says-so-instead-of-looking-broken.md @@ -0,0 +1,49 @@ +--- +Name: Being refused now says so, instead of looking broken +Category: Fix +Description: An assistant that asked to change something it lacked access to got an internal error with a stack trace, and sometimes got the change anyway. Every tool surface now answers refusals in plain words — and the checks behind them, which had been granting everything, actually run. +Icon: ShieldCheck +Order: -20260902 +--- + +# Being refused now says so, instead of looking broken + +Ask an assistant to tidy up a space you can only read, and it would come back with something like +*"the recycle tool threw an unhandled exception"*, followed by a stack trace. The refusal itself was +right — you may not change that space. What was wrong was that nothing said so. From the outside, +"you are not allowed to do this" and "this tool is broken" looked identical, so the assistant would +often try again, or start guessing at repairs for a fault that never existed. + +Refusals now come back as answers, in the words you would want: *"Recycle requires Update permission +on the target node — ask someone with write access to the node (or a platform admin) to do it."* + +## The part that was worse + +Chasing the crash turned up why the friendly message had never appeared: the permission check meant +to produce it was, on these surfaces, always answering *yes*. + +Every assistant, script and API call runs its work on a per-session workspace, and that workspace +had never been handed the rules about who may do what. Asked whether you had permission, it said yes +to everything. The mesh itself still refused — the real gate sits with whoever owns the data, and it +held — but the polite pre-check in front of it had no opinion at all, so nothing ever reached the +readable explanation. + +Two things slipped through that gap, and both are now closed: + +- **Restarting a node you could only read.** Restarting is "note the request, then bounce it". On + most nodes there is nothing to note, so nothing was checked, and the bounce happened anyway. +- **Exporting a space.** An export filters out what you are not allowed to take with you. That + filter ran on the same opinionless workspace, so it kept everything. + +If you have been using an assistant against a space you only have read access to, it now behaves the +way you would expect: it tells you it cannot, and does not quietly do it. + +## Told apart on purpose + +One distinction is kept carefully. *"You do not have permission"* is final and worth acting on. +*"I could not check right now"* is temporary and worth retrying — and telling you the first when the +truth is the second would send you asking for access you already have. The two keep separate +wording, and the difference is decided from what the mesh actually reported, never guessed from how +a message reads. + +The engineering detail is in [A Denial Is an Answer](/Doc/Architecture/DenialIsAnAnswer). diff --git a/src/MeshWeaver.Hosting/SessionHubFactory.cs b/src/MeshWeaver.Hosting/SessionHubFactory.cs index c61d99ae16..61e7f30a9b 100644 --- a/src/MeshWeaver.Hosting/SessionHubFactory.cs +++ b/src/MeshWeaver.Hosting/SessionHubFactory.cs @@ -1,5 +1,6 @@ using MeshWeaver.Data; using MeshWeaver.Mesh; +using MeshWeaver.Mesh.Security; using MeshWeaver.Mesh.Services; using MeshWeaver.Messaging; using Microsoft.Extensions.DependencyInjection; @@ -66,20 +67,55 @@ public static IMessageHub Resolve(IMessageHub rootHub, string prefix, string ses var address = AddressExtensions.CreatePortalAddress($"{prefix}-{sessionId}-{InstanceId}"); logger.LogInformation("Materialising {Prefix} session hub at {Address}", prefix, address); + // 🚨 INHERIT THE MESH'S PERMISSION EVALUATOR — issue #3121. A hub's configuration starts + // EMPTY (MessageHubExtensions.CreateMessageHub builds a fresh MessageHubConfiguration and + // inherits nothing from the parent) and HubPermissionExtensions.ResolveEvaluator does NOT + // walk the parent chain: `hub.Configuration.Get() ?? + // DefaultEvaluator`, whose default returns Permission.All. So WITHOUT this line every + // client-side permission check a session issues — every CheckPermission / + // CheckPermissionOutcome in MeshOperations, on the MCP, REST, gRPC and CLI surfaces alike — + // answered GRANTED unconditionally, for every caller, on every path. + // + // Measured on this tree before the fix, with a Viewer (Read|Execute|Api) calling through a + // session hub: the pre-gate answered granted=True for Update on a node the SAME check + // answers granted=False for on the mesh hub. That is a gate that cannot fail, and it is the + // reason MeshOperations.Recycle's legible "Recycle requires Update permission …" refusal was + // dead code on the MCP surface: the only gate that ever fired was the OWNER's, whose + // DeliveryFailure arrives as an Rx fault — so a denial reached the caller as an + // UnauthorizedAccessException stack trace, and for a node whose recycle writes nothing (any + // non-NodeType node) it did not fire at all and the DisposeRequest went out unauthorised. + // + // Fail-closed is UNCHANGED and this widens nothing: the OWNER's AccessControlPipeline was, + // and remains, the authority on every write. What changes is that the session's own + // pre-flight now asks the same question the owner will, so the caller gets an ANSWER instead + // of a crash — and a mesh deliberately built WITHOUT RLS stays ungated, because there is + // then no delegate to copy. MeshExtensions.NodeOperationExecutionHub already does exactly + // this, for exactly this reason; this is the same fix, applied where every surface picks up + // its hub. It also repairs MeshOperations.Export, whose per-node Export filter believed it + // ran on the mesh hub and in fact runs on THIS one — so an `export` returned every node in + // the subtree regardless of entitlement. See Doc/Architecture/DenialIsAnAnswer. + var permissionEvaluator = rootHub.GetMeshHub().Configuration.Get(); + // AddData() ensures the session hub has its own IWorkspace so MeshOperations.Compile can // subscribe to the NodeType MeshNode stream and Update its compilationStatus through the // canonical write path. RegisterStream wires routing so every response (Get / Search / // Patch / ExecuteScript / …) lands back here. return rootHub.GetHostedHub( address, - sessionConfig => sessionConfig - .AddData() - // 🚨 Node CRUD from a session runs HERE, not on the mesh router. Without this opt-in - // MeshService falls back to the mesh hub and every create/move/delete executes on - // the router's action block, starving routing (see MeshExtensions.NodeOperationTarget). - .WithNodeOperationExecution() - .WithInitialization(hub => - hub.RegisterForDisposal(routingService.RegisterStream(hub))), + sessionConfig => + { + sessionConfig = sessionConfig + .AddData() + // 🚨 Node CRUD from a session runs HERE, not on the mesh router. Without this opt-in + // MeshService falls back to the mesh hub and every create/move/delete executes on + // the router's action block, starving routing (see MeshExtensions.NodeOperationTarget). + .WithNodeOperationExecution() + .WithInitialization(hub => + hub.RegisterForDisposal(routingService.RegisterStream(hub))); + return permissionEvaluator is null + ? sessionConfig + : sessionConfig.WithPermissionEvaluator(permissionEvaluator); + }, HostedHubCreation.Always) ?? throw new InvalidOperationException( $"Failed to materialise {prefix} session hub at {address}."); diff --git a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs index c982b88d5d..8f9b5a2d98 100644 --- a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs +++ b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs @@ -3121,10 +3121,20 @@ public IObservable Export(string path) ?? IoPool.Unbounded; // Access control: Export is its OWN permission (granted by Editor + Admin). Enforce it per - // node against the caller's real effective permissions — never bypass as system. The check - // runs on the mesh hub (which carries the RowLevelSecurity evaluator; when RLS is not - // configured the evaluator returns Permission.All, so an unsecured host exports everything). - // The caller identity is captured here, on the calling thread, before any Rx scheduler hop. + // node against the caller's real effective permissions — never bypass as system. When RLS is + // not configured the evaluator returns Permission.All, so an unsecured host exports + // everything. The caller identity is captured here, on the calling thread, before any Rx + // scheduler hop. + // + // 🚨 THIS RESOLVES THE HUB THE OPERATION RUNS ON, NOT THE MESH HUB, and the comment here + // claimed otherwise until #3121 measured it: on a session surface it hands back the SESSION + // hub (`portal/mcp-…`). That mattered, because a hub's configuration inherits nothing and + // HubPermissionExtensions.ResolveEvaluator falls back to DefaultEvaluator ⇒ Permission.All — + // so this per-node filter, the one thing standing between an MCP `export` and a subtree the + // caller may not read, answered GRANTED for every node. SessionHubFactory now copies the + // mesh's evaluator into every session hub, which is what makes this line correct rather than + // merely well-intentioned; the resolution is left as it is because "the hub this operation + // runs on" is the right question to ask, once every such hub can answer it. var permissionHub = hub.ServiceProvider.GetRequiredService(); var callerUserId = ResolveCallerUserId(); @@ -3617,6 +3627,22 @@ public IObservable Recycle(string path) MeshWeaver.Mesh.Security.PermissionCheckOutcome.Undetermined( $"the permission check for Recycle on '{resolvedPath}' did not complete: " + $"{ex.GetType().Name}"))) + // 🚨 THE PRE-FLIGHT MUST ASK THE QUESTION THE OWNER WILL ASK — issue #3121. The check + // above is a RAW (path, Update) fold, and the owner's delivery gate stopped being that + // in #3061/#3100: on a definitive denial it re-decides through the node's registered + // INodeTypeAccessRule, which for every satellite type says "a satellite's write is + // Update on its MainNode". Until #3121 the disagreement was invisible here, because + // this whole check was inert on a session hub. Making it live without this line would + // have introduced a NEW false refusal — an editor entitled via a satellite's MainNode, + // whom the owner grants, refused by a pre-flight that only knows the raw path. + // + // Reached ONLY on a definitive denial, exactly as in AccessControlPipeline, and for the + // same two reasons: it costs nothing on the granted path (which never reads a node), and + // it never turns an UNDETERMINED fold into a rule question — "we could not check" must + // stay "we could not check". + .SelectMany(outcome => outcome.IsGranted || outcome.IsUndetermined + ? Observable.Return(outcome) + : ReconsiderRecycleThroughNodeTypeRule(resolvedPath)) .SelectMany(outcome => { if (outcome.IsGranted) @@ -3629,26 +3655,145 @@ public IObservable Recycle(string path) + "as retryable, not as a denial: {Reason}", resolvedPath, outcome.UndeterminedReason); return Observable.Return(JsonSerializer.Serialize( - new - { - status = "Error", - path = resolvedPath, - message = "Could not determine whether you may recycle this node — the permission check did not complete. This is a temporary condition, not a refusal: try again in a moment." - }, + new { status = "Error", path = resolvedPath, message = RecycleUndeterminedMessage }, hub.JsonSerializerOptions)); } return Observable.Return(JsonSerializer.Serialize( - new - { - status = "Error", - path = resolvedPath, - message = "Recycle requires Update permission on the target node — it disposes the node's hub and forces re-initialization. Ask someone with write access to the node (or a platform admin) to do it." - }, + new { status = "Error", path = resolvedPath, message = RecycleDeniedMessage }, hub.JsonSerializerOptions)); }); } + /// + /// Re-decides a DENIED pre-flight through the target node's own + /// INodeTypeAccessRule — the same second opinion AccessControlPipeline takes since + /// #3061, built from the same NodeTypeAccessRuleGate helpers so the two seams cannot + /// drift into asking different questions. + /// + /// Every terminal is the honest one: no rule governs the type (or nothing is served at the + /// path) leaves the standard check's standing, and a + /// read that FAULTS is — a storage + /// blip must not be reported as a refusal nobody established. Undetermined carries + /// IsGranted = false, so the caller fails closed on every leg. + /// + /// The recycle target, already resolved. + private IObservable ReconsiderRecycleThroughNodeTypeRule( + string resolvedPath) + { + // Update is in the CRUD set SubjectOperationFor maps, so this is never null in practice — + // it is asked rather than assumed so that a change to that mapping cannot silently make + // this seam decide an operation no rule can speak for. + if (NodeTypeAccessRuleGate.SubjectOperationFor(MeshWeaver.Mesh.Security.Permission.Update) + is not { } operation) + return Observable.Return(MeshWeaver.Mesh.Security.PermissionCheckOutcome.Denied); + + return NodeTypeAccessRuleGate.ReadSubjectNode(hub, resolvedPath) + .SelectMany(node => + { + if (node is null + || NodeTypeAccessRuleGate.Find(hub, node.NodeType, operation) is not { } rule) + return Observable.Return(MeshWeaver.Mesh.Security.PermissionCheckOutcome.Denied); + + var accessService = hub.ServiceProvider.GetService(); + var context = new NodeValidationContext + { + Operation = operation, + Node = node, + AccessContext = accessService?.Context ?? accessService?.CircuitContext, + }; + logger.LogDebug( + "Recycle: Update on {Path} was denied by the standard check — re-deciding " + + "through the {NodeType} access rule, which governs this node type", + resolvedPath, node.NodeType); + return NodeTypeAccessRuleGate.Evaluate(rule, context, ResolveCallerUserId(), logger); + }) + .Catch((Exception ex) => + { + logger.LogWarning(ex, + "Recycle: could not read the node at {Path} to apply its node-type access rule " + + "— reporting UNAVAILABLE rather than a denial nobody established", + resolvedPath); + return Observable.Return(MeshWeaver.Mesh.Security.PermissionCheckOutcome.Undetermined( + $"the node at '{resolvedPath}' could not be read to apply its node-type access " + + $"rule ({ex.GetType().Name})")); + }); + } + + /// + /// The one sentence says when access was EVALUATED and the answer is "no" + /// — whoever reached it, the caller's own pre-flight or the owning hub's delivery gate. + /// + /// ONE constant, deliberately: those are two different evaluators at two different + /// moments (see ), and a caller who was refused should not be able + /// to tell them apart, nor be told two different things about the same fact. It is actionable + /// because a denial IS actionable — the caller cannot fix it themselves and the useful next + /// step is to ask someone who can. Nothing else in this operation may reuse it: an + /// unevaluated check gets . + /// + /// i18n. Deliberately NOT localised — this is a JSON envelope field on the machine + /// tool surface (MCP / REST / CLI), consumed by an agent runtime that reasons about it and + /// renders its own text to the human. Localising it here would translate a string on the model + /// side of the line AGENTS.md draws around tool-facing text, and would make the answer depend + /// on which locale happened to be attached to the session. The human-facing surfaces render + /// their own refusal (the portal's typed error card, via AreaErrorClassifier), and that + /// one IS localised. + /// + internal const string RecycleDeniedMessage = + "Recycle requires Update permission on the target node — it disposes the node's hub and " + + "forces re-initialization. Ask someone with write access to the node (or a platform " + + "admin) to do it."; + + /// + /// What says when the permission check reached NO verdict. Says the + /// opposite thing to on purpose: retry, do not go asking + /// for rights you may already hold (#974). + /// + internal const string RecycleUndeterminedMessage = + "Could not determine whether you may recycle this node — the permission check did not " + + "complete. This is a temporary condition, not a refusal: try again in a moment."; + + /// + /// Classifies a WRITE fault into "the mesh reached a verdict and it is no" versus everything + /// else — the write-side twin of , and decided on + /// the TYPED failure, never by pattern-matching a message (which drifts the moment someone + /// rewords a banner). + /// + /// Why an exception type is a sound verdict test here. + /// is minted on the write path in exactly one + /// situation: a carrying , + /// which AccessControlPipeline posts only after its permission fold reached a DECISIVE + /// denial. The fold's other two terminals are deliberately NOT this type — an undetermined fold + /// answers and a hub that is going away answers + /// — and MeshNodeStreamHandle.UpdateRemote keeps + /// that split, raising MeshNodeStreamException for them. So the tri-state survives the + /// hop as a type distinction, and this predicate reads the verdict half of it. + /// + /// 🚨 The default is NOT a denial. An unrecognised fault is by definition one we + /// have not reasoned about, and the safe direction is the one that does not accuse: a false + /// "transient" costs a retry, while a false "you lack permission" sends a correctly-entitled + /// caller to request rights they already hold — the exact lie #974 removed from the pipeline + /// and #2901 removed from this file's own pre-flight. + /// + /// The chain is walked because the write path wraps: a denial that arrives after + /// UpdateRemote's response bound is dispatched through + /// LatePatchResponseRegistry and can reach the caller nested. + /// + /// The fault the write terminated with. + /// True only when the mesh decided the caller may not write. + internal static bool IsWriteDenial(Exception error) + { + for (var e = error; e is not null; e = e.InnerException) + { + if (e is UnauthorizedAccessException) + return true; + if (e is DeliveryFailureException { Failure.ErrorType: ErrorType.Unauthorized }) + return true; + } + + return false; + } + private IObservable RecycleCore(string resolvedPath) { try @@ -3680,13 +3825,15 @@ private IObservable RecycleCore(string resolvedPath) // SettlesOkAndUnparks, which failed ~20-40% once #2748 stopped resuming test // continuations inline on the signalling thread and so stopped accidentally // holding this ordering.) - var stamped = hub.GetWorkspace().GetMeshNodeStream(resolvedPath).Update(curr => + // Emits the tool's ANSWER when the owner REFUSED the stamp — a verdict, so this is where + // the operation ends and the hub is NOT disposed — or null when the recycle may proceed. + var stampRefusal = hub.GetWorkspace().GetMeshNodeStream(resolvedPath).Update(curr => IsNodeTypeNode(curr) ? WithReleaseRequest(curr, DateTimeOffset.UtcNow) : curr) .Take(1) .Timeout(TimeSpan.FromSeconds(30)) - .Select(_ => true) + .Select(_ => (string?)null) .Catch((Exception ex) => { // 🚨 A DENIAL IS NOT A TRANSIENT FAULT. A caller who may not write this node @@ -3699,14 +3846,29 @@ private IObservable RecycleCore(string resolvedPath) // release request to act on and its watcher went silent. Refusing outright // leaves the caller exactly where they started, which is the right outcome // for someone who was not permitted to change anything. - if (ex is UnauthorizedAccessException) + // + // 🚨 AND THE REFUSAL IS AN ANSWER, NOT A FAULT — issue #3121. This arm used to + // re-raise (`Observable.Throw(ex)`), and nothing above it translated the + // fault: it travelled out of Recycle, out of MeshOperations, and hit the tool + // transport's ONE Task bridge (McpToolResult.AsToolResult, in + // MeshWeaver.Plugins), where the MCP SDK logged it as `"recycle" threw an + // unhandled exception` with a stack trace. The caller could not tell "you were + // refused" from "the tool is broken" — the exact refusal-legibility failure the + // 2026-08-30 fix closed on the UI surface, still open on this one. A denial is + // something the mesh DECIDED; deciding is the operation succeeding at its job, + // so it is rendered in the operation's own envelope, in the same words the + // pre-flight denial uses. Nothing is swallowed: the log line below is unchanged, + // and every non-denial keeps its own separate answer. + if (IsWriteDenial(ex)) { logger.LogWarning(ex, "Recycle REFUSED for {Path}: the release-request stamp was DENIED, so the " + "hub is NOT disposed — a caller who may not write this node may not " + "recycle it either. Nothing was changed.", resolvedPath); - return Observable.Throw(ex); + return Observable.Return(JsonSerializer.Serialize( + new { status = "Error", path = resolvedPath, message = RecycleDeniedMessage }, + hub.JsonSerializerOptions)); } // Anything else IS transient (a timeout, an owner mid-move): the hub bounce is @@ -3714,14 +3876,22 @@ private IObservable RecycleCore(string resolvedPath) // ask — and the compile trigger is recoverable by a second Recycle/Compile. // Surface it in the log rather than silently degrading to the old racy // behaviour. + // + // 🚨 This leg must NOT borrow the denial's words. "We could not evaluate access" + // and "you lack permission" are different answers (#974, #3017): a transient + // owner fault that told the caller to go ask for permissions would send a + // correctly-entitled user chasing rights they already hold. It stays a proceed. logger.LogWarning(ex, "Recycle: failed to stamp release request for {Path} — disposing the hub anyway", resolvedPath); - return Observable.Return(false); + return Observable.Return(null); }); - return stamped.Select(_ => + return stampRefusal.Select(refusal => { + if (refusal is not null) + return refusal; + var changeFeed = hub.ServiceProvider.GetService(); if (changeFeed != null) { diff --git a/test/Memex.Portal.Shared.Test/SessionDenialIsAnAnswerTest.cs b/test/Memex.Portal.Shared.Test/SessionDenialIsAnAnswerTest.cs new file mode 100644 index 0000000000..4cdf1135be --- /dev/null +++ b/test/Memex.Portal.Shared.Test/SessionDenialIsAnAnswerTest.cs @@ -0,0 +1,331 @@ +#pragma warning disable CS1591 + +using System.Reactive.Linq; +using System.Text.Json; +using MeshWeaver.AI; // MeshOperations — its namespace is a frozen binary contract (#2370) +using MeshWeaver.Fixture; +using MeshWeaver.Hosting; +using MeshWeaver.Hosting.Monolith.TestBase; +using MeshWeaver.Mesh; +using MeshWeaver.Mesh.Security; +using MeshWeaver.Messaging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Memex.Portal.Shared.Test; + +/// +/// 🚨 A PERMISSION DENIAL IS AN ANSWER THE TOOL SURFACE RENDERS, NEVER AN UNHANDLED EXCEPTION. +/// Issue #3121. +/// +/// What production showed. An MCP caller without write access invoked recycle +/// on Store/Catalog; the mesh refused correctly, and the refusal reached the client as +/// "recycle" threw an unhandled exception — System.UnauthorizedAccessException: Access denied: +/// user 'rbuergi' lacks Update permission on 'Store/Catalog', top frame +/// McpToolResult.AsToolResult. The caller cannot tell "you were refused" from "the tool is +/// broken", and the server logs a stack trace for a decision it made on purpose. +/// +/// Why the operation's own legible refusal never ran — the root cause, measured. +/// MeshOperations.Recycle has carried an actionable refusal envelope since #2901, gated on +/// hub.CheckPermissionOutcome(path, Update). That pre-flight was DEAD CODE on every session +/// surface. HubPermissionExtensions.ResolveEvaluator reads +/// hub.Configuration.Get<EffectivePermissionsDelegate>() ?? DefaultEvaluator, does not +/// walk the parent chain, and the default returns — while a hub's +/// configuration starts EMPTY, inheriting nothing. — the ONE +/// factory behind MCP, REST, gRPC and the CLI — never copied the mesh's evaluator, so every +/// client-side check a session issued answered GRANTED, for every caller, on every path. The only +/// gate that ever fired was the OWNER's, whose DeliveryFailure{Unauthorized} arrives as an +/// Rx fault. +/// pins that directly; +/// MeshExtensions.NodeOperationExecutionHub had already been fixed for the identical reason, +/// and MeshOperations.Export works around it by resolving the mesh hub by hand. +/// +/// And the two shapes it produced were different, which is why both are pinned. +/// Measured on this tree before the fix — with a throwaway probe whose fixture is the one below, +/// under its own path names — a Viewer (Read|Execute|Api) calling Recycle through a +/// real session hub got: +/// +/// a NodeType node — the stamp is a real write, the owner refuses it, +/// and RecycleCore re-raised: THREW System.UnauthorizedAccessException: Access +/// denied: user '…-viewer' lacks Update permission on '…/Ty' — the production line's +/// exception type and message template, reproduced. +/// a non-NodeType node — the stamp is an identity update that writes +/// nothing, so NOTHING was gated and the answer was +/// {"status":"Recycled"}: the DisposeRequest went out for a caller with no write +/// access at all. The 2026-08-30 guarantee that "a refused recycle no longer tears the hub +/// down" held only for NodeTypes, because the authorization was a SIDE EFFECT of the stamp +/// rather than a decision the operation made. +/// +/// +/// The fix has two halves and both are load-bearing. The session's pre-flight becomes +/// a real gate (so a refusal is the normal, legible path and the destructive half never runs), and +/// the OWNER's verdict — which remains the authority, at a different hub and a later moment — is +/// rendered in the same envelope instead of escaping as a fault. A pre-flight can only ever be +/// check-then-act; it does not make the owner's answer optional. +/// +/// 🚨 A verdict and a non-verdict are not the same answer (#974, #3017), and this +/// change must not blur them: see , which pins that only a +/// decisive refusal renders as "you lack permission" and that a hub going away or an unevaluated +/// check keeps its own, retryable answer. +/// +public class SessionDenialIsAnAnswerTest(ITestOutputHelper output) : MonolithMeshTestBase(output) +{ + private const string Partition = "SessionDenial"; + + /// An ordinary node: its recycle stamp writes NOTHING, so nothing gates it. + private const string PlainPath = Partition + "/Doc"; + + /// A NodeType: its recycle stamp is a real write, and the owner refuses it. + private const string NodeTypePath = Partition + "/Ty"; + + /// + /// A second partition, holding an Activity satellite whose MainNode points back + /// into . The split is what makes the case sharp: a satellite under its + /// own main node inherits that node's grants by path, so the raw check and the rule agree by + /// construction and nothing is being tested. MainNode is a POINTER, not a parent — the + /// hazard MainNodeRebasing documents — and SatelliteAccessRule delegates to + /// wherever it points. + /// + private const string Elsewhere = "SessionDenialElsewhere"; + + /// + /// The satellite: Update on its own path is DENIED for the Editor, and its registered + /// SatelliteAccessRule ("a satellite's write is Update on its MainNode") GRANTS. + /// + private const string SatellitePath = Elsewhere + "/_Activity/run1"; + + /// Read|Execute|Api — no Update anywhere in this partition. + private const string ViewerUser = "session-viewer"; + + /// + /// Update but no Delete — the entitlement that makes the satellite case sharp: the raw check on + /// the satellite's OWN path denies, and the node type's rule grants via the MainNode. + /// + private const string EditorUser = "session-editor"; + + /// 🚨 , never a literal — CI-scaled, and it reports what it + /// was waiting for instead of dying anonymously. + private static TimeSpan Budget => TestTimeouts.Convergence; + + // 🚨 ConfigureMeshBase, not base.ConfigureMesh: the latter chains PublicAdminAccess(), which + // grants Public the Admin role in every default partition — under it the Viewer below would + // hold Update outright and every assertion here would pass vacuously. + protected override MeshBuilder ConfigureMesh(MeshBuilder builder) + => ConfigureMeshBase(builder) + .AddMeshNodes( + new MeshNode(Partition) { Name = "Session Denial", NodeType = "Markdown" }, + new MeshNode("Doc", Partition) { Name = "Doc", NodeType = "Markdown" }, + new MeshNode("Ty", Partition) { Name = "Ty", NodeType = MeshNode.NodeTypePath }, + new MeshNode(Elsewhere) { Name = "Elsewhere", NodeType = "Markdown" }, + new MeshNode("run1", Elsewhere + "/_Activity") + { + Name = "Compile run", + NodeType = "Activity", + // The satellite pointer, into the OTHER partition. Without it + // SatelliteAccessRule takes its degenerate branch and falls back to the plain + // path check — i.e. the answer with no rule at all — so this line is what makes + // the case a satellite whose rule DISAGREES with the raw path. + MainNode = PlainPath, + }, + AssignmentNodeFactory.UserRole(ViewerUser, "Viewer", Partition), + // Scoped to `Partition` ONLY: Update on `SessionDenial/Doc`, nothing at all in + // `SessionDenialElsewhere`. + AssignmentNodeFactory.UserRole(EditorUser, "Editor", Partition)); + + /// + /// The session hub every API surface issues on, materialised exactly as MCP materialises it — + /// through , which is the subject here, not a stand-in for it. + /// + private IMessageHub SessionHub() => SessionHubFactory.Resolve( + Mesh, + "mcp", + "denial", + Mesh.ServiceProvider.GetRequiredService().CreateLogger(nameof(SessionDenialIsAnAnswerTest))); + + private void ActAs(string userId) => Mesh.ServiceProvider.GetRequiredService() + .SetContext(new AccessContext { ObjectId = userId, Name = userId }); + + private void ActAsViewer() => ActAs(ViewerUser); + + /// + /// 🚨 THE ROOT CAUSE, pinned as one comparison: the SAME question, asked on the session hub and + /// on the mesh hub, must get the SAME answer. + /// + /// Pre-fix the session hub answered granted=True and the mesh hub + /// granted=False — a gate that cannot fail, which AGENTS.md names as its own bug class. + /// Asserting the AGREEMENT rather than the denial is deliberate: a fix that disabled the mesh's + /// evaluator would satisfy "both deny" and is caught by the second assertion, which pins that + /// the shared answer is the RESTRICTIVE one. + /// + [Fact] + public async Task ASessionsPermissionCheckAnswersTheSameAsTheMeshs() + { + ActAsViewer(); + + var onSession = await SessionHub().CheckPermissionOutcome(PlainPath, Permission.Update) + .Take(1).Timeout(Budget).Await(TestContext.Current.CancellationToken); + var onMesh = await Mesh.CheckPermissionOutcome(PlainPath, ViewerUser, Permission.Update) + .Take(1).Timeout(Budget).Await(TestContext.Current.CancellationToken); + + onSession.IsGranted.Should().Be(onMesh.IsGranted, + "a session hub is where MCP / REST / gRPC / CLI issue every operation, so a permission " + + "check there must reach the mesh's verdict — not DefaultEvaluator's Permission.All"); + onMesh.IsGranted.Should().BeFalse( + "a Viewer holds Read|Execute|Api and no Update — if this flips, the agreement above is " + + "vacuous and the gate has been widened rather than wired"); + } + + /// + /// The production line, reproduced: a NodeType whose recycle stamp is a real write the owner + /// refuses. Pre-fix this threw out of the operation, + /// past MeshOperations, into the transport's one Task bridge. + /// + [Fact] + public async Task RecyclingANodeTypeWithoutUpdateAnswersInsteadOfThrowing() + => (await RecycleAsViewer(NodeTypePath)).Should().Be(MeshOperations.RecycleDeniedMessage, + "a denial is something the mesh DECIDED — the operation renders it in its own envelope, " + + "never as an unhandled exception the MCP server logs with a stack trace"); + + /// + /// The half the 2026-08-30 fix could not reach: an ordinary node, whose recycle stamp writes + /// nothing and is therefore gated by nothing. Pre-fix this answered {"status":"Recycled"} + /// and posted the DisposeRequest for a caller with no write access. + /// + /// The envelope IS the assertion that nothing was performed, and deliberately so: the + /// DisposeRequest is posted on exactly one code path, the one that returns + /// {"status":"Recycled"}. A refusal envelope therefore proves that path was not taken. + /// The alternative — waiting to observe that no dispose arrived — is settle-by-silence, which + /// passes for the wrong reason on a slow mesh and reds for the wrong reason on a fast one. + /// + [Fact] + public async Task RecyclingAPlainNodeWithoutUpdateIsRefusedNotPerformed() + => (await RecycleAsViewer(PlainPath)).Should().Be(MeshOperations.RecycleDeniedMessage, + "recycling disposes the node's hub — a caller who may not write the node may not tear " + + "it down either, and that must not depend on whether the stamp happens to be a write"); + + /// + /// 🚨 THE OTHER DIRECTION, and the regression this change had to avoid CREATING. Making the + /// pre-flight live is only safe if it asks the question the OWNER asks — and since #3061/#3100 + /// the owner's delivery gate re-decides a denied raw check through the node's registered + /// INodeTypeAccessRule. A pre-flight that stopped at the raw path would refuse an editor + /// the owner grants: a NEW false refusal, introduced by a fix for a legibility bug. + /// + /// The fixture is a satellite whose MainNode points into ANOTHER partition, which + /// is the only shape where the two answers can differ: a satellite living UNDER its main node + /// inherits that node's grants by path, so raw and rule agree by construction and the case + /// would be vacuous. MainNode is a pointer, not a parent. + /// + /// So: the Editor holds Update in and nothing in + /// . Raw Update on the satellite's own path — DENIED. + /// SatelliteAccessRule — "a satellite's write is Update on its MainNode" — GRANTED. + /// The recycle must go through. + /// + [Fact] + public async Task RecyclingASatelliteFollowsItsNodeTypesRule_NotTheRawPath() + { + ActAs(EditorUser); + + var rawCheck = await Mesh.CheckPermissionOutcome(SatellitePath, EditorUser, Permission.Update) + .Take(1).Timeout(Budget).Await(TestContext.Current.CancellationToken); + rawCheck.IsGranted.Should().BeFalse( + "the Editor's grant is scoped to the other partition — if the raw path check GRANTS, " + + "the rule is never consulted and this test proves nothing"); + + var answer = await new MeshOperations(SessionHub()).Recycle(SatellitePath) + .FirstAsync().Timeout(Budget).Await(TestContext.Current.CancellationToken); + Output.WriteLine($"Recycle({SatellitePath}) → {answer}"); + + using var envelope = JsonDocument.Parse(answer); + envelope.RootElement.GetProperty("status").GetString().Should().Be("Recycled", + "the satellite's own access rule grants what the raw path check denies, and the " + + "pre-flight must reach the same verdict the owner's delivery gate would"); + } + + /// + /// 🚨 ONLY A VERDICT READS AS A DENIAL. is the seam + /// that keeps "we checked; you may not" apart from "we could not check" / "this activation is + /// going away", and it decides on the TYPED failure — never by matching a message, which drifts + /// the moment someone rewords a banner. + /// + /// Pinned as a unit because the production shape that needs it — the owner refusing a + /// write the caller's own pre-flight granted — is a two-evaluator, two-moment race (a revoked + /// assignment between the check and the write; a cross-silo fold; an identity lost across a + /// scheduler hop). It is real and it is why the owner-side arm must stay, but it is not + /// reproducible fast and deterministically in a single-silo mesh, so the PREDICATE is pinned + /// directly — the same reasoning PatchLandedWriteCheckTest records for FieldsLandedIn. + /// + /// + [Theory] + [MemberData(nameof(WriteFailures))] + public void OnlyAVerdictReadsAsADenial(string shape, Exception failure, bool isDenial) + => MeshOperations.IsWriteDenial(failure).Should().Be(isDenial, shape); + + public static TheoryData WriteFailures() => new() + { + { + "a decisive refusal — the ONLY thing the write path mints UnauthorizedAccessException for", + new UnauthorizedAccessException("Access denied: user 'x' lacks Update permission on 'p'"), + true + }, + { + "the same verdict nested, as a late denial dispatched through LatePatchResponseRegistry arrives", + new InvalidOperationException("write failed", new UnauthorizedAccessException("Access denied")), + true + }, + { + "the NACK itself, before UpdateRemote maps it", + new DeliveryFailureException(Nack(ErrorType.Unauthorized, "Access denied: user 'x' lacks Update")), + true + }, + { + "a hub that is GONE — transient by contract, and the caller must RE-PROBE, not go asking for rights", + new DeliveryFailureException(Nack(ErrorType.ShuttingDown, "… is shutting down. Rejecting now")), + false + }, + { + "a fold that reached NO verdict — #974's whole point: this is not a statement about the user's rights", + new DeliveryFailureException(Nack(ErrorType.Unavailable, "Permission check unavailable — no verdict was reached")), + false + }, + { + "the owner never answered within the bound", + new TimeoutException("The operation has timed out."), + false + }, + { + "an unrecognised fault — the default must never accuse", + new InvalidOperationException("something else entirely"), + false + }, + }; + + private static DeliveryFailure Nack(ErrorType errorType, string message) + => new(new MessageDelivery + { + Message = new RawJson("{}"), + Sender = new Address("test/sender"), + Target = new Address(NodeTypePath), + }) + { + ErrorType = errorType, + Message = message + }; + + /// + /// Runs Recycle the way MCP runs it — as the Viewer, on a real session hub — and returns + /// the message field of the JSON envelope. A THROW propagates: the whole point is that + /// the operation answers. + /// + private async Task RecycleAsViewer(string path) + { + ActAsViewer(); + var answer = await new MeshOperations(SessionHub()).Recycle(path) + .FirstAsync().Timeout(Budget).Await(TestContext.Current.CancellationToken); + Output.WriteLine($"Recycle({path}) → {answer}"); + using var envelope = JsonDocument.Parse(answer); + envelope.RootElement.GetProperty("status").GetString().Should().Be("Error", + "the operation ran and produced a refusal, so its envelope must say so"); + return envelope.RootElement.GetProperty("message").GetString(); + } +}