A session's permission check can actually fail, and a denial is an answer (#3121) by rbuergi · Pull Request #3127 · Systemorph/MeshWeaver

rbuergi · 2026-09-02T15:01:40Z

Closes #3121.

What the issue reported, and what re-running its measurement found

The report is right about the symptom and wrong about the scope. An MCP recycle on a node the caller could not write reached the client as "recycle" threw an unhandled exception with a stack trace, top frame McpToolResult.AsToolResult. The report concludes "the permission layer behaved as designed". It did not: the layer that should have produced the legible refusal could not fail, and had never fired on any session surface.

I reproduced the production line byte-for-byte on a real mesh — a Viewer (Read|Execute|Api) calling through a hub materialised by the real SessionHubFactory:

Recycle(Probe3121/Ty) THREW: System.UnauthorizedAccessException:
    Access denied: user 'probe-viewer' lacks Update permission on 'Probe3121/Ty'

against production's

System.UnauthorizedAccessException: Access denied: user 'rbuergi' lacks Update permission on 'Store/Catalog'

and then measured every other operation on the same mesh, same identity. Before → after:

Operation (as a Viewer, no Update)	Before	After
`recycle` a NodeType	`THREW UnauthorizedAccessException`	the refusal envelope
`recycle` a plain node	`{"status":"Recycled"}` — the `DisposeRequest` went out	the refusal envelope
`create`	`Created: Probe3121/New` — the node was written	`Access denied: Create permission required`
`export`	every node in the subtree	only nodes the caller may `Export`
`patch` / `move`	already legible	unchanged

Root cause

HubPermissionExtensions.ResolveEvaluator is three lines:

hub.Configuration.Get<EffectivePermissionsDelegate>()
?? MessageHubPermissionExtensions.DefaultEvaluator;   // => Observable.Return(Permission.All)

It does not walk the parent chain, and CreateMessageHub builds a fresh configuration per hub, inheriting nothing. AddRowLevelSecurity() installs the evaluator on the mesh hub and every per-node hub. A session hub is neither — and SessionHubFactory is the one factory behind MCP, REST, gRPC, the CLI and the headless sidecar.

Measured, same question two ways:

Asked on	`Update` on a node the Viewer may only read
`portal/mcp-…` (session hub)	granted
`mesh/…`	denied

That is a gate that cannot fail — AGENTS.md's own bug class, in runtime form. For a PatchDataRequest it cost only legibility: the owner's AccessControlPipeline remains the authority on that write and fails closed. But three operations do not authorize there, and for them it cost more:

recycle a plain node — authorization was a side effect of the release-request stamp, and the stamp only writes on a NodeType. On anything else nothing was posted, nothing was gated, and the destructive half ran free.
create — RlsNodeValidator is scoped and asks hub.CheckPermission on whichever hub resolved it. A session hub opts into WithNodeOperationExecution(), so CRUD runs there, not on the shared NodeOperationExecutionHub that had been taught to copy the evaluator. That hub's own comment predicted this verbatim — "moving node CRUD onto a hub that did not copy the evaluator would make RlsNodeValidator grant EVERY write" — for its own hub, without noticing the session hub had the same hazard.
export — the per-node Export filter ran on the same opinionless hub, so it kept everything.

MeshExtensions.NodeOperationExecutionHub was already fixed this way, and its comment spells the hazard out verbatim. The session hub never got the same treatment.

The fix — two halves, both load-bearing

1. 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. Nothing is widened — the owner already refused everything the pre-flight now refuses — and a mesh built without RLS stays ungated, because there is then no delegate to copy (verified: Memex.LocalMesh never calls AddRowLevelSecurity(), so the sidecar is bit-for-bit unaffected; memex/Memex.Portal.Shared/MemexConfiguration.cs:501 is where the real portal wires it).

2. RecycleCore renders the OWNER's refusal instead of re-raising it. A pre-flight is check-then-act: it runs on a different hub, at an earlier moment, against a fold that can have moved on. The owner stays the authority, and its verdict is now the operation's answer, in the same sentence the pre-flight denial uses.

🚨 Waking a gate means matching what the owner asks — not just asking something

Half 1 had a regression built into it, which I found by asking what else the owner's gate does that the pre-flight does not. Since #3061/#3100 the delivery gate is no longer a raw (path, permission) fold: 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 raw — so switching it on 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 (definitive denial only — an undetermined fold must stay "we could not check").

Pinned by RecyclingASatelliteFollowsItsNodeTypesRule_NotTheRawPath, and it is a real negative control: with the reconsideration removed it fails Expected "Recycled" … but found "Error". Its fixture points the satellite's MainNode into another partition deliberately — a satellite living under its own main node inherits that node's grants by path, so raw and rule agree by construction and the case would prove nothing. The test asserts the raw check denies before it asserts anything else, so it cannot pass vacuously.

Not a try/catch, and not a swallow

The refusal is not caught-and-prettified. MeshOperations.IsWriteDenial classifies on the typed failure and only a verdict renders as a denial:

Fault	Denial?
`UnauthorizedAccessException`	yes — the write path mints it for `DeliveryFailure{Unauthorized}` and nothing else
`DeliveryFailureException{Unauthorized}`	yes
`DeliveryFailureException{ShuttingDown}`	no — transient, re-probe
`DeliveryFailureException{Unavailable}`	no — no verdict was reached (#974)
`TimeoutException`, anything unrecognised	no — the default must never accuse

That the tri-state survives the hop as a type distinction is by construction: AccessControlPipeline flattens it onto the bus vocabulary exactly once, and UpdateRemote keeps the split by raising MeshNodeStreamException for the other two. This is the write-side twin of NodeReadOutcome.FromReadFailure, and it is #3017's lesson applied here. The transient log line and the transient behaviour (proceed) are unchanged.

`Export`'s comment was wrong, and it mattered

MeshOperations.Export filters the subtree per node against the caller's Export permission, and its comment said the check "runs on the mesh hub". It resolves hub.ServiceProvider.GetRequiredService<IMessageHub>(), which on a session surface hands back the session hub — measured portal/mcp-probe-8c70164f. Comment corrected; half 1 is what makes the line correct rather than merely well-intentioned.

i18n

Not localised, deliberately — and the PR says which side of the line these strings are on. RecycleDeniedMessage / RecycleUndeterminedMessage 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. Same side as an LLM tool parameter's [Description]. Localising them would also make the answer depend on whichever locale happened to be attached to a session.

Precedent, measured: there are zero Localize calls in the whole of MeshWeaver.Mesh.Operations. The human-facing refusal is the portal's typed error card via AreaErrorClassifier, and that one is localised. No new key, so no strings.{en,de}.json change and no plugins-repo mirror PR.

Core-only — checked, not assumed

McpToolResult.AsToolResult (where the exception surfaced) lives in MeshWeaver.Plugins, but it is a transport bridge, not an error handler: hardening it would need a catch-all that erases the verdict/non-verdict distinction. Every fix here is in core, SessionHubFactory.Resolve's signature is unchanged, and no public type is removed — so no Pairs-with counterpart is needed. Plugins picks the behaviour up with the platform.

Verification

Negative control, by reverting each half separately (git diff/git apply -R, never git stash):

	`ASessionsPermissionCheckAnswersTheSameAsTheMeshs`	`Recycling…NodeType…`	`Recycling…PlainNode…`
both halves reverted	fails	throws (the reported bug)	fails (`"Recycled"`)
half 1 reverted	fails (`found True`)	passes (half 2 renders it)	fails (`found "Recycled"`)
half 2 reverted	passes	passes (pre-flight refuses first)	passes
full fix	passes	passes	passes

Half 2's own production shape (owner refuses what the pre-flight granted) is a two-evaluator, two-moment race and is not reproducible fast and deterministically in a single-silo mesh, so the predicate is pinned directly by a seven-case theory — the same reasoning PatchLandedWriteCheckTest records for FieldsLandedIn.

dotnet build -c Release -warnaserror, one project per invocation, 0 Error(s) / 0 Warning(s): MeshWeaver.Hosting, MeshWeaver.Mesh.Operations, MeshWeaver.Documentation, Memex.Portal.Shared, and the five test projects below.

Full, unfiltered runs (Release):

Memex.Portal.Shared.Test — 603 passed (incl. the 11 new)
MeshWeaver.Graph.Test — 1210 passed
MeshWeaver.Hosting.Test — 318 passed
MeshWeaver.PluginTester.Test — 318 passed
MeshWeaver.Hosting.Orleans.Test — 234 passed
MeshWeaver.Documentation.Test — 202 passed (incl. DocumentationLinkIntegrityTest over the new page)

No hand-written timeout literals — the new tests use TestTimeouts.Convergence.

Work products conserved

Doc/Architecture/DenialIsAnAnswer — the rule, the measurement, the classifier table, and what a new operation must do; linked from the Architecture index's Security row.
A What's New entry (Category: Fix).

Out of scope, filed as #3128

MeshOperations.Compile has no permission gate at all — a Viewer with no Compile permission successfully compiled a NodeType and minted its _Activity node, both before and after this change. Different defect, different decision (whether compile is a read or a write), so it is not smuggled in here.

🤖 Generated with Claude Code

…an answer (#3121) An MCP `recycle` on a node the caller could not write reached the client as `"recycle" threw an unhandled exception` plus a stack trace — indistinguishable from a broken tool. `MeshOperations.Recycle` has carried a legible refusal since #2901; it had never run. `HubPermissionExtensions.ResolveEvaluator` reads the evaluator off the hub's own Configuration, does not walk the parent chain, and falls back to `DefaultEvaluator` => `Permission.All`. A hub's configuration starts empty, and `AddRowLevelSecurity()` installs the evaluator on the mesh hub and per-node hubs only. `SessionHubFactory` — the one factory behind MCP, REST, gRPC and the CLI — never copied it, so every client-side check those surfaces issued answered granted, for every caller, on every path. Measured on a real mesh with a Viewer (Read|Execute|Api), through a real session hub: recycle a NodeType THREW UnauthorizedAccessException -> refusal envelope recycle a plain node {"status":"Recycled"} (disposed!) -> refusal envelope create Created: ... (node written!) -> Access denied export every node in the subtree -> only permitted ones The recycle-a-plain-node case is why the 2026-08-30 fix could not close this: the release-request stamp only writes on a NodeType, so on anything else nothing was posted, nothing was gated, and the destructive half ran free. Authorization that is a side effect of a write disappears when the write does. Two halves: - `SessionHubFactory` copies the mesh hub's `EffectivePermissionsDelegate` into every session hub — exactly what `MeshExtensions.NodeOperationExecutionHub` already does, for the reason its comment spells out. Nothing is widened: the owner already refused everything the pre-flight now refuses, and a mesh built without RLS stays ungated because there is no delegate to copy. - `RecycleCore` renders the OWNER's refusal in the operation's own envelope instead of re-raising it. A pre-flight is check-then-act; the owner stays the authority, at a different hub and a later moment. A verdict and a non-verdict stay apart. `MeshOperations.IsWriteDenial` decides on the typed failure — `UnauthorizedAccessException` / `DeliveryFailure{Unauthorized}` are the only verdicts the write path mints; `ShuttingDown`, `Unavailable`, a timeout and anything unrecognised are not, because the default must never accuse (#974, #3017). Also corrects `MeshOperations.Export`'s comment, which claimed its per-node Export filter ran on the mesh hub. It runs on the session hub — which is why the filter was inert, and why this change repairs it. Doc: Doc/Architecture/DenialIsAnAnswer + a What's New entry. Not localised, deliberately: these are JSON envelope fields on the machine tool surface, read by an agent runtime that renders its own text to the human — the same side of the line as an LLM tool parameter's [Description]. There are zero `Localize` calls in MeshWeaver.Mesh.Operations; the human-facing refusal is the portal's typed error card, which is localised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…he envelope proves nothing was performed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The new tests reference MeshOperations.RecycleDeniedMessage and MeshOperations.IsWriteDenial, but both are currently internal without an InternalsVisibleTo bridge, so the test project will not compile as-is.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes permission-denial legibility on session-based tool surfaces (MCP/REST/gRPC/CLI) by ensuring session hubs can evaluate effective permissions (instead of defaulting to Permission.All), and by returning a denial envelope from Recycle rather than letting owner-side denials escape as unhandled exceptions. It also adds documentation and tests to pin the “denial vs non-verdict” distinction.

Changes:

Copy the mesh hub’s permission evaluator into session hubs created by SessionHubFactory.
Update MeshOperations.Recycle/RecycleCore to render owner-side denial outcomes as a tool-result envelope (not a thrown exception), while preserving non-verdict behavior.
Add a new architecture page + What’s New entry, and introduce a test suite that reproduces and pins the regression and its fixes.

File summaries

File	Description
test/Memex.Portal.Shared.Test/SessionDenialIsAnAnswerTest.cs	Adds regression tests covering session-vs-mesh permission agreement, recycle denial behavior, and denial classification.
src/MeshWeaver.Mesh.Operations/MeshOperations.cs	Clarifies `Export` permission-hub behavior; adds reusable recycle denial/undetermined messages and a denial classifier; returns denial envelopes from `RecycleCore`.
src/MeshWeaver.Hosting/SessionHubFactory.cs	Copies the mesh permission evaluator into session hub configuration when available.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-being-refused-now-says-so-instead-of-looking-broken.md	Adds a user-facing What’s New entry describing the behavioral change.
src/MeshWeaver.Documentation/Data/Architecture/DenialIsAnAnswer.md	Adds an architecture note documenting the failure mode and the required operational pattern.
src/MeshWeaver.Documentation/Data/Architecture.md	Links the new architecture page from the Security section.

Review details

Files reviewed: 6/6 changed files
Comments generated: 2
Review effort level: Lite

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

…the session hub is not a legibility problem Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… creates no false refusal Making the session's Recycle pre-flight live (previous commit) was only safe if it asks what the OWNER asks. Since #3061/#3100 the delivery gate re-decides a denied raw check through the node's registered INodeTypeAccessRule — 'a satellite's write is Update on its MainNode'. The pre-flight was a raw path fold, so switching it on would have refused an editor entitled via a satellite's MainNode whom the owner grants: a new false refusal shipped by a fix for a legibility bug. It now takes the same second opinion, from the same NodeTypeAccessRuleGate helpers, on the same condition — a definitive denial only, never an undetermined fold. Pinned by RecyclingASatelliteFollowsItsNodeTypesRule_NotTheRawPath. Negative control: with the reconsideration removed it fails 'Expected "Recycled" ... but found "Error"'. Its fixture points the satellite's MainNode into another partition on purpose — a satellite under its own main node inherits that node's grants by path, so raw and rule agree by construction and the case would prove nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-09-02T15:32:34Z