Repository navigation
fix(security): Compile is gated on Permission.Compile (#3128) - #3145
Conversation
MeshOperations.Compile carried NO permission check at all. Recycle requires Update on the target and Export requires Export per node; Compile required nothing, so a caller who may only READ a NodeType could trigger a Roslyn build of it and mint an _Activity node underneath it. Measured both BEFORE and AFTER #3127 — that PR fixed the session hub's inert evaluator, which is why create/recycle/export changed answer there and Compile did not. There was no check for the repaired evaluator to answer. The permission is Compile, not Update. Both were defensible and the issue asked for the choice to be deliberate: Permission.Compile already exists and is already scoped for exactly this. Role.Editor carries it ("Space editors ship releases by default"), Role.Viewer and Role.Commenter do not, and Admin/PlatformAdmin add it EXPLICITLY because it is deliberately excluded from Permission.All. The entitlement was modelled and never consulted; Update would have invented a second answer to a settled question. Tri-state, mirroring Recycle (#2901/#2742): a fold that hiccuped or completed without emitting is reported as retryable, never as "you may not compile this" — which would send a caller who already holds Compile to go and request it. Still fail-closed: nothing is triggered without a positive verdict. Control arm: with the gate asking for Read instead (which a Viewer holds, so the ungated behaviour is reproduced), exactly 1 of 13 fails — the Viewer denial — and the Editor fact still passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The permission gate is correctly implemented with existing tri-state patterns, and the added tests verify both the denied (Viewer) and allowed (Editor) cases.
Pull request overview
This PR closes a security gap where MeshOperations.Compile could be invoked without any permission gate, allowing read-only users to trigger a Roslyn compile and create an _Activity under the target NodeType. It adds an explicit Permission.Compile check (with the existing tri-state “granted/denied/undetermined” behavior) and verifies the behavior via the session-hub test suite.
Changes:
- Add a permission gate to
MeshOperations.CompilerequiringPermission.Compile, returning operation-shaped refusal/undetermined envelopes instead of throwing. - Add/extend session-hub tests to assert a Viewer is refused and an Editor is not refused by the compile gate.
- Add a “What’s New” entry documenting the behavior change.
File summaries
| File | Description |
|---|---|
| test/Memex.Portal.Shared.Test/SessionDenialIsAnAnswerTest.cs | Adds tests covering compile refusal for Viewer and pass-through for Editor on a real-RLS session hub. |
| src/MeshWeaver.Mesh.Operations/MeshOperations.cs | Introduces CompileDeniedMessage/CompileUndeterminedMessage and gates Compile(...) on Permission.Compile with tri-state handling. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-compiling-a-type-now-asks-permission.md | Documents the security fix and explains the refusal vs “could not decide” distinction. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results (shard 1)437 tests 437 ✅ 56s ⏱️ Results for commit fae634b. |
Test Results (shard 0)234 tests 234 ✅ 1m 42s ⏱️ Results for commit fae634b. |
Test Results (shard 3)340 tests 338 ✅ 47s ⏱️ Results for commit fae634b. |
Test Results (shard 2)576 tests 384 ✅ 2m 35s ⏱️ Results for commit fae634b. |
Test Results (shard 5) 4 files 4 suites 1m 42s ⏱️ Results for commit fae634b. |
Test Results (shard 4)2 133 tests 2 133 ✅ 2m 23s ⏱️ Results for commit fae634b. |
Test Results 15 files 15 suites 10m 7s ⏱️ Results for commit fae634b. |
Fixes #3128.
The hole
MeshOperations.Compilecarried no permission check at all. Its two neighbours in the same file do:RecyclerequiresUpdateon the target ("it disposes the node's hub"),ExportrequiresExportper node.Compilerequired nothing — so a caller who may only read a NodeType could trigger a Roslyn build of it, and mint an_Activitynode underneath it.Measured both before and after #3127. That PR fixed the session hub's inert evaluator, which is why
create/recycle/exportchanged answer there andCompiledid not: there was no check for the repaired evaluator to answer.The scope decision the issue asked for:
Permission.CompileThe issue deliberately did not fold this into #3127 because it needs a choice — is a compile a read or a write? — and named the two candidates. This picks
Permission.Compile, and the reason is that the entitlement was already modelled and simply never consulted:Compile?Admin,PlatformAdminPermission.All | Permission.CompileEditorViewer(Read|Execute|Api)CommenterUpdatewas the other candidate — it is what the nearest neighbour uses — but it would have invented a second answer to a question the role table had already settled, and it would gate "may ship a release" on a permission that means something else.🚨 One detail that makes this choice load-bearing rather than cosmetic:
Compileis deliberately excluded fromPermission.All(soHasFlag(All)/IsGlobalAdminstays byte-stable), which is exactly whyAdminandPlatformAdminadd it by hand. So "does this caller have it?" genuinely cannot be answered by anAllcheck — it has to be asked for by name, which is what this does.The compile execution is unaffected and still runs as System; what is gated is the request, which is the part that is not free.
The tri-state, not a swallow
Mirrors
Recycle's shape verbatim (#2901 / #2742), including why:CompileDeniedMessage, which names the permission by the name it has in the role table so the reader can ask for the right thing;CompileUndeterminedMessage— "this is not a statement about your access, try again". A fold that merely hiccuped or completed without emitting must not be reported as "you may not compile this", which would send a caller who already holdsCompileoff to request something they own..Timeoutalone cannot cover that second case: it faults on silence, and a fold that completes empty sails past it.Still fail-closed — nothing is triggered without a positive verdict. The refusal is rendered as the operation's own envelope, never raised, per
Doc/Architecture/DenialIsAnAnswer.A bare
Take(1), likeRecycle's and for its documented reason: leaving the fold's gate also leaves the hub's turn, and the trigger must stay ordered against the caller's turn.Verification
Added to
SessionDenialIsAnAnswerTest— the suite that measured this issue, on a real-RLS monolith mesh (ConfigureMeshBase, noPublicAdminAccess) through a realSessionHubFactorysession hub.Read, which a Viewer holds)CompilingANodeTypeWithoutCompilePermissionIsRefusedNotPerformedAnEditorHoldsCompile_AndTheGateLetsItThroughFailed: 1, Passed: 12Passed: 13in 5 sThe control arm asks for a permission the Viewer does hold, which reproduces the ungated behaviour exactly, and the result discriminates precisely: only the Viewer fact moves.
🚨 The second test is the case that could have falsified the first. A gate that refused everybody would satisfy the denial assertion just as well and would be a far worse bug than the hole it closes —
Compileis how a Space editor ships a release. It deliberately asserts only not-the-denial, not a successful compile: whether Roslyn then succeeds is not what this gate decides, and pinning it would make an access test fail for a compiler reason.dotnet build -c Release -warnaserroronMeshWeaver.Mesh.OperationsandMemex.Portal.Shared.Test: 0 Warning(s), 0 Error(s).Note
The two message constants follow their neighbours (
RecycleDeniedMessage,RecycleUndeterminedMessage) and are, like them, plain English rather than catalog keys. Localising one of a pair while its twin is not would be worse than either — if these operation envelopes should be localised, that is a change to all of them together and worth its own issue.Pairs-with: none — no public type is removed; two
internal conststrings are added.🤖 Generated with Claude Code