Repository navigation
An agent-facing read never runs as System when nobody is asking - #823
Merged
Merged
Conversation
Closes the paywall bypass: an unentitled user could read gated PAID content by
exact path (`get @AgenticPrimerDe/02-CodeWunsch` returned 79,650 chars of the
lesson), and the silo trace showed the read reaching the grain as
`userId=system-security`.
The framework deliberately treats an ABSENT AccessContext as System. That is
correct for background tasks — timers and seed jobs legitimately have no
identity — and is pinned by
SubscribeRequestIdentityRoutingTest.SubscribeRequest_WithNullContext_FallsBackToSystem
("System is the right fallback for 'no identity'"). But System holds
Permission.All, so applying that default to a REQUEST-shaped read means "nobody
is asking" is served everything.
So the fix is at the agent-facing surface, not in the global fallback: this
surface is only ever reached by a caller (MCP tool, agent round), so absence of a
user means UNAUTHENTICATED, not infrastructure. MeshOperations.FetchNode now
falls back to Anonymous, which sees public content and is denied gated content
exactly like a logged-out visitor. Background flows keep null → System untouched.
An earlier attempt made the cache gate itself fail closed on a null context. It
closed the hole but fought the architecture — it broke the pinned
null-falls-back-to-System contract and the static-repo import's verification
reads. Reverted in favour of this narrow one.
Tests — PaywallIdentityMatrixTests, one gated node read by every identity shape:
no ambient identity (the production case), Anonymous, signed-in-but-unentitled,
the buyer, and the public cover. The first case FAILED before this change and
passes after; the buyer and cover cases are what catch an over-strict gate.
Suites: Hosting.PostgreSql 619 passed / 0 failed; Hosting.Monolith 467 passed /
0 failed — including the two tests the broad version regressed.
Follow-up to #819 and #821, which hardened real seams but did not close the
breach because the identity was absent rather than merely lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a security issue where agent/MCP exact-path reads could execute under the System identity when no caller identity is present, potentially bypassing paywall/RLS gating. It narrows the fix to the agent-facing read surface by falling back to an Anonymous identity for MeshOperations.FetchNode when both AccessService.Context and CircuitContext are absent, and adds a PostgreSQL-backed identity-matrix test suite to pin the expected behavior.
Changes:
- Update
MeshOperations.FetchNodeto treat “no ambient identity” as unauthenticated (Anonymous) instead of allowing the framework’s global null→System fallback to apply. - Add
PaywallIdentityMatrixTestscovering no-identity, Anonymous, unentitled authenticated, entitled buyer, and public cover scenarios against PostgreSQL persistence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/MeshWeaver.AI/MeshOperations.cs | Falls back to Anonymous when FetchNode is invoked with no ambient identity, preventing null-context reads from running as System. |
| test/MeshWeaver.Hosting.PostgreSql.Test/PaywallIdentityMatrixTests.cs | Adds an integration test matrix to validate paywall behavior across identity shapes, including the “no identity” production case. |
Suppressed comments (1)
src/MeshWeaver.AI/MeshOperations.cs:798
- The anonymous fallback AccessContext uses a hard-coded display name ("Anonymous") that duplicates
WellKnownUsers.Anonymousand diverges from other anonymous contexts (typicallyName = WellKnownUsers.Anonymous). Keeping the name consistent helps log/audit correlation and avoids drift if the constant ever changes.
var callerIdentity = accessService?.Context ?? accessService?.CircuitContext
?? new AccessContext { ObjectId = WellKnownUsers.Anonymous, Name = "Anonymous", IsVirtual = true };
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+783
to
+798
| // 🚨 An AGENT-FACING read never proceeds with NO identity. The framework's global fallback | ||
| // deliberately treats an absent AccessContext as System — correct for background tasks | ||
| // (timers, seed jobs) that legitimately have none, and pinned by | ||
| // SubscribeRequestIdentityRoutingTest.SubscribeRequest_WithNullContext_FallsBackToSystem. | ||
| // But System holds Permission.All, so applying that default to a REQUEST-shaped read means | ||
| // "nobody is asking" is served everything — including gated, PAID content. Measured on | ||
| // memex 2026-08-05: an unentitled user read a full paid course lesson by exact path | ||
| // (`get @AgenticPrimerDe/02-CodeWunsch`), and the silo trace showed it reaching the grain | ||
| // as `userId=system-security`. | ||
| // | ||
| // This surface is only ever reached by a caller (MCP tool, agent round), so absence of a | ||
| // user here means UNAUTHENTICATED, not infrastructure: fall back to Anonymous. That sees | ||
| // public content and is denied gated content, exactly like a logged-out visitor — and it | ||
| // leaves the global background-task fallback untouched. | ||
| var callerIdentity = accessService?.Context ?? accessService?.CircuitContext | ||
| ?? new AccessContext { ObjectId = WellKnownUsers.Anonymous, Name = "Anonymous", IsVirtual = true }; |
Comment on lines
+101
to
+104
| var (_, gated) = await Seed("Anon"); | ||
| var result = await Read( | ||
| new AccessContext { ObjectId = WellKnownUsers.Anonymous, Name = "Guest", IsVirtual = true }, gated); | ||
| Output.WriteLine(result); |
Contributor
Test Results (shard 0)718 tests 714 ✅ 4m 41s ⏱️ Results for commit 1d93caf. |
Contributor
Test Results (shard 4)1 020 tests 1 013 ✅ 5m 20s ⏱️ Results for commit 1d93caf. |
Contributor
Test Results (shard 3) 11 files 11 suites 5m 26s ⏱️ Results for commit 1d93caf. |
Contributor
Test Results (shard 5)1 525 tests 1 421 ✅ 5m 36s ⏱️ Results for commit 1d93caf. |
Contributor
Test Results (shard 2)1 626 tests 1 625 ✅ 5m 1s ⏱️ Results for commit 1d93caf. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 1)1 616 tests 1 429 ✅ 7m 23s ⏱️ Results for commit 1d93caf. |
Contributor
Test Results 63 files 63 suites 33m 29s ⏱️ Results for commit 1d93caf. ♻️ This comment has been updated with latest results. |
rbuergi
added a commit
that referenced
this pull request
Aug 5, 2026
…s the paywall The bypass, finally at its root: the API token attaches the user's DB/platform roles (e.g. the portal-admin role) as CLAIM roles, and PermissionEvaluator folded AccessContext.Roles into every node's effective permissions AFTER the per-scope deny subtraction. A claim role was therefore a GLOBAL, UNDENIABLE grant on the entire mesh — while the SQL fold (search, listings, RLS) never sees claims and correctly denied the same nodes. The two read paths disagreeing IS the bypass: measured on memex 2026-08-05 with one credential in one second, `search` hid the gated paid lessons and `get` by exact path served them in full (79,650 chars of AgenticPrimerDe/02-CodeWunsch to an unentitled caller). It is also why SST was readable, and why three upstream identity fixes (#819, #821, #823) — all real hardening — could not close it: the identity was correct; the evaluation was wrong. The model (AGENTS.md): a platform role grants the PLATFORM gates, deliberately NOT cross-partition data access — it must not read a course it has not bought. The change, in PermissionEvaluator: - ComputeRoleState no longer folds claim roles into roleIds. Node data permissions come from AccessAssignment nodes and policies ONLY — matching the SQL path row for row. - Claim roles keep their one legitimate job: the API-token CAPABILITY gate. An IsApiToken caller passes when its NODE permissions carry Api or its claims include an Api-bearing built-in role (ClaimsCarryApi) — without this half, every MCP token would zero out on PublicRead-policy content. Tests — PaywallRealGateShapeTests: the gate seeded with the EXACT node shapes Store's PluginGate writes (root Public+Anonymous Viewer grants, per-child Viewer-DENIED RoleAssignments, the buyer's root grant), read through the same entry point the MCP get tool uses. Before the fix 4 of 7 were red — including DbRoleClaim_DoesNotGrantNodeRead, the production scenario verbatim. After: all 7 green, with the buyer, the public cover and the ApiToken-reads-public cases pinning against an over-strict gate. The earlier PaywallIdentityMatrixTests seeded permissions through the test fixture helper rather than assignment nodes, which is why its deny cases passed while production leaked. Harness fallout, fixed the honest way: tests free-rode on the claim-Admin superuser. Now - TestUsers.DevLoginAdminAccess() — real STATIC root Admin assignment nodes for the DevLogin identities, chained into the default ConfigureMesh and the suites that hand-roll theirs. The static grant also keeps the evaluator's synchronous fast path alive where tests deliberately stall synced queries (CompileSourceSnapshotWedgeTest). - SetupAccessRightsAsync runs under ImpersonateAsSystem — provisioning grants is a System act in tests exactly as in production (PluginGate/SystemInstall); it cannot bootstrap under a user who needs the grant being written. Suites: Hosting.Monolith 467 passed / 0 failed. Hosting.PostgreSql 623 passed / 3 failed — the 3 GroupMembershipRecomputeTests failures reproduce identically on clean main (shared-fixture state, pre-existing). Closes #822. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Aug 5, 2026
…s the paywall (#825) * Claim roles are a platform capability, never node data access — closes the paywall The bypass, finally at its root: the API token attaches the user's DB/platform roles (e.g. the portal-admin role) as CLAIM roles, and PermissionEvaluator folded AccessContext.Roles into every node's effective permissions AFTER the per-scope deny subtraction. A claim role was therefore a GLOBAL, UNDENIABLE grant on the entire mesh — while the SQL fold (search, listings, RLS) never sees claims and correctly denied the same nodes. The two read paths disagreeing IS the bypass: measured on memex 2026-08-05 with one credential in one second, `search` hid the gated paid lessons and `get` by exact path served them in full (79,650 chars of AgenticPrimerDe/02-CodeWunsch to an unentitled caller). It is also why SST was readable, and why three upstream identity fixes (#819, #821, #823) — all real hardening — could not close it: the identity was correct; the evaluation was wrong. The model (AGENTS.md): a platform role grants the PLATFORM gates, deliberately NOT cross-partition data access — it must not read a course it has not bought. The change, in PermissionEvaluator: - ComputeRoleState no longer folds claim roles into roleIds. Node data permissions come from AccessAssignment nodes and policies ONLY — matching the SQL path row for row. - Claim roles keep their one legitimate job: the API-token CAPABILITY gate. An IsApiToken caller passes when its NODE permissions carry Api or its claims include an Api-bearing built-in role (ClaimsCarryApi) — without this half, every MCP token would zero out on PublicRead-policy content. Tests — PaywallRealGateShapeTests: the gate seeded with the EXACT node shapes Store's PluginGate writes (root Public+Anonymous Viewer grants, per-child Viewer-DENIED RoleAssignments, the buyer's root grant), read through the same entry point the MCP get tool uses. Before the fix 4 of 7 were red — including DbRoleClaim_DoesNotGrantNodeRead, the production scenario verbatim. After: all 7 green, with the buyer, the public cover and the ApiToken-reads-public cases pinning against an over-strict gate. The earlier PaywallIdentityMatrixTests seeded permissions through the test fixture helper rather than assignment nodes, which is why its deny cases passed while production leaked. Harness fallout, fixed the honest way: tests free-rode on the claim-Admin superuser. Now - TestUsers.DevLoginAdminAccess() — real STATIC root Admin assignment nodes for the DevLogin identities, chained into the default ConfigureMesh and the suites that hand-roll theirs. The static grant also keeps the evaluator's synchronous fast path alive where tests deliberately stall synced queries (CompileSourceSnapshotWedgeTest). - SetupAccessRightsAsync runs under ImpersonateAsSystem — provisioning grants is a System act in tests exactly as in production (PluginGate/SystemInstall); it cannot bootstrap under a user who needs the grant being written. Suites: Hosting.Monolith 467 passed / 0 failed. Hosting.PostgreSql 623 passed / 3 failed — the 3 GroupMembershipRecomputeTests failures reproduce identically on clean main (shared-fixture state, pre-existing). Closes #822. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test harness stops free-riding on claim roles; contract tests flipped CI surfaced the full blast radius of the claim-role fix — 75 tests across 7 projects. Every one was a test relying on the removed behavior, none a product flow (DevLogin_BrandNewUser_SelfProvisions passing under the new evaluator proves onboarding needs no claim crutch). Three moves: - ConfigureMeshBase chains TestUsers.DevLoginAdminAccess(): a real STATIC root Admin assignment node for the harness's ambient DevLogin identity (Roland). Every suite funnels through the base, so test-body scaffolding writes work without per-suite patches. TestUser is deliberately EXCLUDED — suites use it as a SUBJECT (grant Viewer/Editor per test, assert the affordances), and a baked-in root Admin would shadow every such role. The node id deliberately differs from the runtime convention ({subject}_Access → Roland_DevLoginAccess): several suites CREATE _Access/Roland_Access themselves and a static node at the same path made that create collide (completes empty). The evaluator folds assignments by accessObject, not node id. - SecurityServiceTests: the two tests that PINNED the bypass as a feature are flipped to pin the new contract. ClaimBasedAdmin_GrantsImmediately_ EvenWhenNoStaticAssignment ("claim-based Admin must grant all permissions") is now ClaimBasedAdmin_DoesNotGrantNodePermissions; the delivery-context test asserts restored claim roles resolve Permission.None without assignment nodes. - The five PostgreSql suites that hand-roll ConfigureMesh (and so skip the base) chain DevLoginAdminAccess explicitly. Suites: Security 345/0, Monolith 467/0, Auth 116/0, Graph 802/0, AI 979/0, Content 245/0, NodeOperations 85/0, Grpc 8/0. PostgreSql 623/3 — the 3 GroupMembershipRecomputeTests failures are pre-existing local fixture pollution; green on CI's fresh database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Gate-shape tests: one lazy class-level seed instead of seven CI shard 5 timed out the per-test seeds' fold barriers (each of seven tests seeded its own plugin; the process-wide security-access scope streams pile up and the 60s deny-fold barrier expired late in the shard). Seed ONCE for the class instead — lazily from the first test body, because a top-level Space cannot be created during mesh INIT (SetupAccessRightsAsync-time creates of a partition-owning node complete empty; only assignment nodes work there), and with a per-run unique plugin name (the PostgreSql fixture database persists across runs — a constant name collides with the previous run's plugin and the seeding CreateNode completes empty). Barriers raised to 120s. Same nodes, same reads, same assertions — the red-before/green-after proof of the evaluator fix is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #822.
The bug
An unentitled user could read gated paid content by exact path —
get @AgenticPrimerDe/02-CodeWunschreturned 79,650 chars of the lesson — and the silo trace showed the read reaching the grain asuserId=system-security.Why it happened, and why the obvious fix is wrong
The framework deliberately treats an absent
AccessContextas System. That's correct for background tasks (timers, seed jobs) that legitimately have no identity, and it's pinned:But System holds
Permission.All. Applying that default to a request-shaped read means "nobody is asking" is served everything.I first made the cache gate itself fail closed on a null context. It closed the hole and fought the architecture — breaking that pinned contract and the static-repo import's verification reads. Reverted in favour of the narrow fix.
The fix
At the agent-facing surface only.
MeshOperations.FetchNodeis reached solely by a caller (MCP tool, agent round), so absence of a user there means unauthenticated, not infrastructure. It now falls back to Anonymous — which sees public content and is denied gated content, exactly like a logged-out visitor. The global background-task fallback is untouched.Tests —
PaywallIdentityMatrixTestsOne gated node, read by every identity shape:
Hosting.PostgreSql: 619 passed / 0 failedHosting.Monolith: 467 passed / 0 failed — including the two tests the broad version regressedRelationship to #819 / #821
Both hardened real seams (reads run as the caller) and are worth keeping, but neither closed the breach: the identity was absent, not merely lost, so a no-op fallback left it running as System.
Prod verification after deploy:
get @AgenticPrimerDe/02-CodeWunschas an unentitled account must be denied, andget @AgenticPrimerDemust still work.🤖 Generated with Claude Code