Skip to content

fix(security): an API token's authority is read live, never from the role snapshot taken at mint - #2974

Merged
github-actions[bot] merged 5 commits into
mainfrom
fix/api-token-role-snapshot-staleness
Sep 1, 2026
Merged

github-actions[bot] merged 5 commits into
mainfrom
fix/api-token-role-snapshot-staleness

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

The defect

ApiToken.Roles is a list of role ids captured when the token was created. It rides ValidateTokenResponse.Roles and is stamped onto AccessContext.Roles by UserContextMiddleware. PermissionEvaluator's API-token clamp — which zeroes the whole permission set for a Bearer context that lacks Permission.Api — consulted that snapshot as its escape hatch (ClaimsCarryApi).

A snapshot answers a question about now with a fact from then, so it was wrong in both directions at once:

What a stale snapshot does Consequence
Too restrictive cannot see a grant made after the mint A token minted before its owner held anything Api-bearing could not read a publicly-readable partition the same person's browser renders fine — and no later grant could fix it. Most IdPs emit no role claims at all, so ApiToken.Roles is usually empty and re-minting produced the same empty list.
Too permissive cannot lose a capability revoked after the mint A token whose mint-time claims carried an Api-bearing role name kept the API surface open forever, over the top of a PartitionAccessPolicy written afterwards saying api: false. Withdrawing API reach could not withdraw it from tokens that already existed.

The second row is the security half and is what drove the design: the failure that must not ship is a token retaining authority someone took away.

The rationale for the snapshot was obsolete

Four comments (UserContextMiddleware, ApiToken.Roles, ValidateTokenResponse.Roles, ApiTokenNodeType) justified the stamp with:

per-node hubs intentionally don't register the synced AccessAssignment query (recursion avoidance), so without it an API-token request sees 0 roles → 0 perms → DENY.

That stopped being true months ago and nobody noticed:

  • the fold moved onto the process-wide IMeshNodeStreamCache — AddRowLevelSecurity registers no synced query at all, and the mesh hub and every per-node hub run the same evaluator over the same cached $security-* queries. There is no hub on which a grant is invisible.
  • the 2026-08-05 paywall fix removed claim roles from node permissions outright. After it, the snapshot's only remaining effect was this one capability hatch.

Those comments are corrected here rather than left to mislead a third time.

The fix

if (currentContext?.IsApiToken == true && !p.HasFlag(Permission.Api)
    && !PublicSurfaceCarriesApi(publicGrant, permissionCap))
    p = Permission.None;

private static bool PublicSurfaceCarriesApi(Permission publicGrant, Permission permissionCap)
    => publicGrant.HasFlag(Permission.Read) && permissionCap.HasFlag(Permission.Api);

A publicly-readable scope (a PartitionAccessPolicy.PublicRead, or a declared NodeTypeGate segment) is public on every surface — a page anyone may read is not secret from an API client. That is what keeps tokens working on Doc/, Agent/ and every installed package partition, which PackageInstaller publishes through exactly that policy rather than through an AccessAssignment. And api: false anywhere on the scope chain now genuinely means "not reachable through the API", because the cap binds the capability the public grant confers.

AccessContext.Roles is now read nowhere in the evaluator.

Why this shape — against the three constraints in the brief

  • Fresh, and freshness is free. Both inputs are already computed by the fold for this path on every emission. No new query, no new subscription, nothing added to the hot path.
  • Not a fan-out. "Does this person hold an Api-bearing role anywhere?" is the unanchored all-partitions question Doc/Architecture/CrossSchemaFanOutElimination (Every page render fires unanchored cross-schema queries (access, notifications, threads, mesh_nodes) — ~2 s each over 186 partition schemas #2640) exists to forbid — a 188-schema UNION is a measured lock bomb. The question worth asking is path-scoped, and the scope walk already answers it.
  • No recursion. Resolving roles inside token validation would put a permission read inside the authentication bootstrap. Nothing here re-enters the evaluator, so the recursion the per-node-hub story was built to avoid is not reintroduced.
  • The third state stays a third state. The predicate is only consulted on a fold emission, where both inputs are known. A leg that cannot reach a verdict still terminates with an error → Undetermined / ErrorType.Unavailable, refusing the delivery. Undetermined never grants.

Proof each test can fail (invert once, see red, restore)

test/MeshWeaver.Graph.Test/ApiTokenCapabilityFreshnessTest.cs, 6 cases, on a real monolith mesh with ConfigureMeshBase (no PublicAdminAccess, so nothing passes vacuously).

Inversion 1 — restore the pre-fix claim hatch: Failed: 3, Passed: 3

  • StaleClaimRoles_CannotDefeat — expected None … but found Read. This is the security defect, live: api: false on the partition, and a token remembering ["Admin","Editor"] reads it anyway.
  • TokenWithNoClaims_ReadsAPubliclyReadablePartition — expected true … but found False. The lockout direction, on a token no re-mint can repair.
  • ClaimRoles_ChangeNoVerdict — same node, same person, verdict differed on what the token remembered.
  • The three "must still work" cases stayed green, so the suite is not merely asserting the new code.

Inversion 2 — disable the clamp entirely (if (false) …): Failed: 1, Passed: 5 — StaleClaimRoles_CannotDefeat reds, proving the clamp is still load-bearing and the fix did not neuter it.

Restored: Passed: 6, Failed: 0.

Verified

  • dotnet build … -c Release -warnaserror, one project per invocation, 0 Error(s) / 0 Warning(s): MeshWeaver.Mesh.Contract, MeshWeaver.Graph, MeshWeaver.Hosting.AspNetCore, MeshWeaver.Documentation, test/MeshWeaver.Graph.Test, test/MeshWeaver.Documentation.Test.
  • ApiTokenCapabilityFreshnessTest — 6/6 green.
  • DocumentationLinkIntegrityTest + WhatsNewEntryIntegrityTest + DocumentationEmbedIntegrityTest — 12/12 green (the new doc section and What's New entry).

Blast radius reviewed

  • Core: the only other IsApiToken construction is PermissionProbeCacheKeyTests, which tests the cache-key builder directly and is unaffected.
  • Plugins PaywallRealGateShapeTests: the public cover is granted by Public/Anonymous Viewer AccessAssignment nodes, and Role.Viewer carries Api, so p already holds Api and the clamp never fires — ApiToken_WithClaimRoles_StillReadsPublicContent stays green. The gated child carries explicit Public/Anonymous denies, so there is no public grant and …_IsStillDeniedGatedContent, DbRoleClaim_DoesNotGrantNodeRead and ViewerClaim_DoesNotGrantNodeRead stay red-on-read as intended.
  • Plugins McpAccessControlTests: its LoginWithToken builds a context without IsApiToken, so the clamp is not on that path at all; its Roles = ["Editor"] comment is now stale but inert.

Work product conserved

  • Doc/Architecture/AccessControl → new section "API tokens and the Api capability": what the clamp is, the two live ways past it, why the mint-time snapshot existed, why its rationale expired, and the general rule — a credential must not carry a copy of an authorization fact; copies go stale in both directions and the permissive direction has no expiry.
  • What's New entry (Category: Fix).

On SocialMedia#92

Refs Systemorph/MeshWeaver.SocialMedia#92 — deliberately not Closes. This fixes a real defect of exactly that issue's shape, but a live read of the mesh shows it is not carson's cause: Profiles/_Access/carson_Access grants Editor, Role.Editor carries Permission.Api, and there is no PartitionAccessPolicy anywhere under Profiles — so on that path p already holds Api and the clamp cannot fire, with or without claims. Details posted on the issue; it stays open.

🤖 Generated with Claude Code

…role snapshot taken at mint

`ApiToken.Roles` is captured when a token is created, rides
`ValidateTokenResponse.Roles`, and is stamped onto `AccessContext.Roles` by
`UserContextMiddleware`. `PermissionEvaluator`'s API-token clamp — which zeroes
the WHOLE permission set for a Bearer context lacking `Permission.Api` — used
that snapshot as its escape hatch (`ClaimsCarryApi`). A snapshot answers a
question about NOW with a fact from THEN, and it was wrong in both directions:

* too RESTRICTIVE — a token minted before its owner held anything Api-bearing
  could not read a PublicRead partition the same person's browser renders fine,
  and no later grant could fix it, because no later grant rewrites a minted
  token. Most IdPs emit no role claims at all, so `ApiToken.Roles` is usually
  empty and re-minting produced the same empty list.
* too PERMISSIVE (the security half) — the hatch outlived whatever produced it.
  A token whose mint-time claims carried an Api-bearing role name kept the API
  surface open FOREVER, over the top of a `PartitionAccessPolicy` written
  afterwards that said `api: false`. Withdrawing API reach could not withdraw it
  from the tokens that already existed.

The clamp now derives the capability from `(publicGrant, permissionCap)` —
values the fold already computes for the target path on every emission. A
publicly readable scope (a `PublicRead` policy or a declared `NodeTypeGate`) is
public on EVERY surface, and a scope that caps `Api` out is closed to the API
even when it is public in a browser. `AccessContext.Roles` is now read NOWHERE
in the evaluator.

Why this shape, against the three constraints:
* no new data source, no extra subscription on the hot path;
* no cross-schema fan-out — "does this person hold an Api role anywhere" is the
  unanchored all-partitions question Doc/Architecture/CrossSchemaFanOutElimination
  forbids; the question worth asking is path-scoped and the scope walk already
  answers it;
* no recursion — resolving roles inside token validation would put a permission
  read inside the auth bootstrap. Nothing re-enters the fold.
* undetermined is not a grant: the predicate is only consulted on an emission,
  where both inputs are known; a starving leg still terminates as an error →
  Undetermined / Unavailable, never a permissive default.

The rationale the old comments gave for the stamp ("per-node hubs intentionally
don't register the synced AccessAssignment query, so a token would see 0 roles →
DENY") had been obsolete for months: the fold moved onto the process-wide
IMeshNodeStreamCache, `AddRowLevelSecurity` registers no synced query at all, and
the 2026-08-05 paywall fix had already removed claim roles from node
permissions. Those comments are corrected here rather than left to mislead again.

Proven non-vacuous: restoring the claim hatch reds exactly
StaleClaimRoles_CannotDefeat, TokenWithNoClaims_ReadsAPubliclyReadablePartition
and ClaimRoles_ChangeNoVerdict while the three "must still work" cases stay
green; disabling the clamp outright reds StaleClaimRoles_CannotDefeat, so the
clamp is still load-bearing.

Refs Systemorph/MeshWeaver.SocialMedia#92

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…posed (#2976)

`AccessControlPipeline` restores a delivery's AccessContext only when its Roles
list is non-empty — a condition written for claim-based role resolution, which no
longer exists. A token minted with no claims (the ordinary case) therefore reaches
a per-node hub with no context, so `IsApiToken` is never seen and the clamp does
not run on that path. The exact-read gate is unaffected (it captures the caller's
context itself), so this is not a hole in MCP `get`.

Filed as #2976 rather than fixed here: it is a context-propagation defect on a
per-delivery hot path and wants its own measurement and its own failing test, not
a ride-along in a staleness fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Second commit (doc only): removing the stamp's rationale exposed a second, distinct defect that the doc would otherwise have read as benign.

AccessControlPipeline (src/MeshWeaver.Hosting/Security/AccessControlPipeline.cs:~177) restores a delivery's AccessContext only when its Roles list is non-empty — a condition written for claim-based role resolution, which no longer exists. A token minted with no claims (the ordinary case) therefore reaches a per-node hub with no restored context, so capturedContext is null, IsApiToken is never seen, and the clamp does not run on that path.

Not a hole in MCP get: MeshNodeStreamCache.GetStreamRaw captures the caller's context itself and wraps the evaluation in SwitchAccessContext(captured), so the exact-read gate clamps correctly. The exposure is message-routed permission checks on the receiving hub.

Filed as #2976 rather than fixed here — it is a context-propagation defect on a per-delivery hot path and wants its own before/after measurement and its own failing test, not a ride-along in a staleness fix. Named in Doc/Architecture/AccessControl so the next reader does not have to rediscover it.

@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

216 tests   216 ✅  2m 28s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 3437cfd.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a security/authorization defect where API-token capability decisions could be influenced by a mint-time role snapshot (ApiToken.Roles), ensuring API token authority is derived from live, path-scoped access data on every evaluation.

Changes:

  • Updates PermissionEvaluator to remove the claim-role escape hatch and instead allow API-token access to public surfaces only when the live public grant is present and the live policy cap still permits Api.
  • Clarifies contract/documentation comments across token validation and middleware to state that token role snapshots are diagnostic only and not used for authorization.
  • Adds a focused monolith-based regression test suite plus documentation/What’s New entries describing the defect and the fix.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/MeshWeaver.Graph.Test/ApiTokenCapabilityFreshnessTest.cs Adds regression coverage proving API-token capability is derived from live grants/policy, not mint-time roles.
src/MeshWeaver.Mesh.Contract/Security/ValidateTokenRequest.cs Updates ValidateTokenResponse.Roles documentation to reflect diagnostic-only role snapshots.
src/MeshWeaver.Mesh.Contract/Security/PermissionEvaluator.cs Changes API-token clamp logic to use (publicGrant, permissionCap) instead of claim roles; expands rationale comments.
src/MeshWeaver.Mesh.Contract/Security/ApiToken.cs Updates ApiToken.Roles documentation to explicitly forbid treating it as authority.
src/MeshWeaver.Hosting.AspNetCore/Portal/UserContextMiddleware.cs Updates middleware comment to state stamped roles are not used for permission decisions.
src/MeshWeaver.Graph/Configuration/ApiTokenNodeType.cs Updates validate-token handler comment to reflect diagnostic-only role snapshots.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-01-an-api-token-no-longer-remembers-what-you-used-to-be-allowed.md Adds user-facing release note for the behavior change.
src/MeshWeaver.Documentation/Data/Architecture/AccessControl.md Adds an architecture section explaining the API-token clamp and why role snapshots must not be trusted.
Suppressed comments (1)

src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-01-an-api-token-no-longer-remembers-what-you-used-to-be-allowed.md:9

  • Same grammar issue as the front-matter Name: the H1 heading is missing an object after “allowed”.
# An API token no longer remembers what you used to be allowed

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

Comment thread src/MeshWeaver.Documentation/Data/Architecture/AccessControl.md
@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

578 tests   578 ✅  56s ⏱️
  2 suites    0 💤
  2 files      0 ❌

Results for commit 3437cfd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 126 tests   934 ✅  1m 2s ⏱️
    3 suites  192 💤
    3 files      0 ❌

Results for commit 3437cfd.

♻️ This comment has been updated with latest results.

…nt second, no re-mint

The static cases prove the token reads what its grant allows; this one proves the
ORDER that made #92 look like a token problem: the token exists, the read is
REFUSED (the control arm, asserted first so the test cannot pass vacuously), an
administrator writes the grant, and the SAME token — never re-minted, never
re-validated — reads. Nothing about the token changes between the two reads;
only the mesh does.

The wait is on the condition (the fold re-emits when the grant lands), never a
sleep, and its own partition so it cannot perturb the no-grant control arm.

7/7 green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 649 tests   1 649 ✅  1m 36s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 3437cfd.

♻️ This comment has been updated with latest results.

rbuergi and others added 2 commits September 1, 2026 16:30
Copilot review:
* What's New title — taken. "…what you used to be allowed" trails off; the
  entry is now "An API token no longer works from a copy of your old
  permissions", which also names the mechanism rather than only the symptom.
* Heading level — declined, measured: AccessControl.md uses '#' for all 18 of its
  top-level sections (grep '^# ' → 223, 246, 341, 598, 641, 682, 755, 825, 932,
  …). The new section at 825 sits between '# Type-declared subtree gates' (755)
  and '# Hierarchical access pattern' (932) and is a sibling of both. The '##'
  immediately above it is a SUBSECTION of the previous top-level section, which
  is what the diff context showed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…30 s literal

TestTimeoutLiteralRatchetGuard caught it: 388 vs baseline 387, i.e. exactly the
one `TimeSpan.FromSeconds(30)` this suite added. Converted the site rather than
raising the baseline, which the guard names as the defect.

`Quick` is the right member: every wait here settles a LOCAL in-memory fold over
cached queries. It is also CI-scaled and stays strictly BELOW each case's
[Fact(Timeout = 60_000)] on CI as well as locally (12 s / 36 s vs 60 s), so a wait
that does not converge loses first and reports WHAT it was waiting for instead of
xunit killing the method and reporting an anonymous timeout — the inner/outer
ordering TestTimeouts exists to keep by construction.

🚨 Worth recording how this was nearly missed: the earlier targeted run
(--filter on three doc tests) passed and was a FALSE PASS — the guard that fails
is in the same project but was not in the filter. Running the whole
MeshWeaver.Documentation.Test project is what caught it: 165 passed / 1 failed,
now 166 / 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Local guard sweep caught one of mine before CI did: TestTimeoutLiteralRatchetGuard at 388 vs baseline 387 — exactly the one TimeSpan.FromSeconds(30) this suite added. Converted the site (TestTimeouts.Quick) rather than raising the baseline, which the guard names as the defect. 166/166 green now.

Quick is the right member — every wait here settles a local in-memory fold over cached queries — and it is CI-scaled, staying strictly below each case's [Fact(Timeout = 60_000)] on CI as well as locally (12 s / 36 s vs 60 s), so a wait that does not converge loses first and names what it was waiting for.

🚨 Worth recording, because it nearly slipped: my earlier targeted run (--filter on the three doc-integrity tests) passed and was a false pass — the guard that fails lives in the same project but was outside the filter. Running the whole MeshWeaver.Documentation.Test project is what surfaced it: 165 passed / 1 failed → 166 / 0. A filter narrow enough to be fast is narrow enough to miss the thing you added.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

419 tests   419 ✅  1m 16s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 3437cfd.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

507 tests   507 ✅  1m 32s ⏱️
  3 suites    0 💤
  3 files      0 ❌

Results for commit 3437cfd.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results

4 495 tests   4 303 ✅  8m 53s ⏱️
   13 suites    192 💤
   13 files        0 ❌

Results for commit 3437cfd.

@github-actions
github-actions Bot merged commit 0d541f2 into main Sep 1, 2026
26 checks passed
@rbuergi
rbuergi deleted the fix/api-token-role-snapshot-staleness branch September 13, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants