Skip to content

fix(security): the API-token clamp now reaches message-routed checks (#2976) - #3002

Merged
rbuergi merged 1 commit into
mainfrom
fix/2976-restore-access-context-unconditionally
Sep 1, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/2976-restore-access-context-unconditionally

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2976.

The defect

AccessControlPipeline restored the sender's AccessContext onto the receiving per-node hub only when it carried role claims:

// Restore the sender's AccessContext onto this scope's AccessService
// so SecurityService.GetEffectivePermissions — which reads claim-based
// roles from _accessService.Context.Roles — can resolve them. Trigger:
// delivery carries non-empty user Roles (signed claim payload).
if (delivery.AccessContext is { Roles: { Count: > 0 } } userCtx)

The hypothesis in the issue is verified: AccessContext.Roles is read nowhere in PermissionEvaluator. The 2026-08-05 paywall fix took claim roles out of node permissions and #2974 removed their last foothold (the API-token capability hatch), so the condition survived with its rationale gone — and it was never the right condition. The fold reads exactly two flags off the restored context:

Flag What it decides
IsApiToken The API-token clamp — zeroes the permission set for a Bearer context this path's live policy does not admit (api: false).
IsHub The hub-credential early return.

Neither has anything to do with roles.

Most identity providers emit no role claims, so ApiToken.Roles is normally empty. Such a token reached a per-node hub with Roles = [], the restore was skipped, capturedContext was null in the fold, IsApiToken was never seen and the clamp never ran — the Bearer delivery was evaluated as if it were an interactive session. Silently: no exception, no log, a served read.

Not a hole in the exact-read path — MeshNodeStreamCache.GetStreamRaw captures the caller's context itself and wraps evaluation in SwitchAccessContext, so MCP get clamps correctly. api: false therefore held for a read that named the page and was skipped for every message-routed check against it.

The shape is worth naming: the gate's own INPUT decided whether the gate ran, so "the check did not run" and "the check passed" were indistinguishable from outside. That is the runtime cousin of AGENTS.md's no-skip-trapdoor rule for CI — which this very file already states for its missing-evaluator branch.

The fix

Restore whenever the delivery carries a real principal — a non-empty ObjectId that is not hub-shaped — rather than on a payload field the decision does not read.

Hub-shaped ids (sync/, mesh/, node/, activity/, portal/) keep exactly the treatment the rest of the mesh already gives them (AccessService.SetContext's leak tripwire, UserServiceDeliveryPipeline's shouldStamp, MeshNodeStreamCache's pass-through): a hub address is not a user identity and is never installed as one. So nothing about hub credentials changes, and the SetContext error tripwire is not turned into a per-delivery log flood. The empty-ObjectId anonymous context is likewise skipped — it names nobody to restore, and shadowing a circuit context with it would change verdicts for anonymous rendering without fixing anything.

Cost on the per-delivery hot path

The issue flags that this sits inside the per-delivery path on every per-node hub, so what the change adds is worth naming rather than assuming. Per [RequiresPermission] delivery whose context previously carried no claims: one string.IsNullOrEmpty, one LooksLikeHubPrincipal (five StartsWith over a short id), and one AccessService.SetContext — an AsyncLocal write plus a LogDebug that fires only when the principal actually changes. The new StackTrace(...) inside SetContext is on the hub-shaped-principal branch, which this condition never reaches. No new allocation, no new DI resolve, no new query, and nothing added to the fold itself.

Test evidence — measured, not assumed

New test/MeshWeaver.Graph.Test/RoutedApiTokenClampTest.cs: a [RequiresPermission(Read)] message posted from a client hub at the per-node hub that owns a PublicRead = true, Api = false partition, carrying an API-token AccessContext with empty Roles. Real monolith mesh (MonolithMeshTestBase), no mocks.

Before the fix (src/ unchanged, test added):

[xUnit.net] RoutedApiTokenClampTest.ABearerDeliveryWithNoClaims_IsClampedOnThePerNodeHub [FAIL]
  Expected value not to be <null> because a claimless Bearer delivery reached the gate
  and was SERVED — the API-token clamp never ran …
Failed! - Failed: 1, Passed: 1, Skipped: 0, Total: 2

After the fix:

Passed! - Failed: 0, Passed: 2, Skipped: 0, Total: 2

The control arm — the same person, the same node, the same routed message without the Bearer flag — is served both before and after, so the pin measures a capability decision rather than a blanket deny.

Full local suites, Debug: MeshWeaver.Graph.Test 1086/1086, MeshWeaver.Documentation.Test 166/166, plus MeshWeaver.Hosting.Test and MeshWeaver.Data.Test. Release -warnaserror, 0 warnings / 0 errors: MeshWeaver.Hosting, MeshWeaver.Hosting.AspNetCore, MeshWeaver.Documentation, MeshWeaver.Graph.Test, MeshWeaver.Documentation.Test.

Work products conserved

  • Doc/Architecture/AccessContextPropagation gains "Restoring the caller on the receiving hub" — the third restore site, why it exists (the gate composes outside UserServiceDeliveryPipeline, so Phase 4 has not happened yet when it evaluates), the two flags the fold reads, and the AccessControlPipeline restores a delivery's AccessContext only when Roles is non-empty — an API token with no claims reaches a per-node hub unclamped #2976 finding written as the rule it is an instance of.
  • Doc/Architecture/AccessControl's "Known gap" note (which the issue cites) is closed and points at the new section.
  • The stale rationale in UserContextMiddleware — "…and because AccessControlPipeline uses a non-empty Roles list as its cue" — is corrected; AccessContext.Roles is now a diagnostic and nothing else.
  • What's New entry (Category: Fix): A partition closed to the API is now closed on every path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa

…2976)

`AccessControlPipeline` restored the sender's `AccessContext` onto the receiving
per-node hub only when it carried role CLAIMS
(`delivery.AccessContext is { Roles: { Count: > 0 } }`). The comment justified
that in terms of claim-based role resolution — a mechanism that no longer
exists: the 2026-08-05 paywall fix took claim roles out of node permissions and
#2974 removed their last foothold (the API-token capability hatch).
`AccessContext.Roles` is read NOWHERE in `PermissionEvaluator`.

So the condition outlived its reason, and it was never the right one. The fold
reads two flags off the restored context — `IsApiToken` (the API-token clamp,
which zeroes the permission set for a Bearer context this path's live policy
does not admit) and `IsHub` (the hub-credential early return) — neither of which
has anything to do with roles. Most IdPs emit no role claims, so `ApiToken.Roles`
is normally empty: the ordinary token reached a per-node hub with `Roles = []`,
the restore was skipped, `capturedContext` was null, `IsApiToken` was never seen,
and the clamp DID NOT RUN. The Bearer delivery was then evaluated as an
interactive session — silently, with no exception and no log.

The exact-read path was never exposed (`MeshNodeStreamCache.GetStreamRaw`
captures the caller itself and wraps evaluation in `SwitchAccessContext`), so
`api: false` held for a read that named the page and was skipped for every
message-routed check against it.

The restore is now keyed on the delivery carrying a REAL PRINCIPAL — a non-empty
`ObjectId` that is not hub-shaped — rather than on a payload field the decision
does not read. Hub-shaped ids (`sync/`, `mesh/`, `node/`, `activity/`, `portal/`)
keep the mesh-wide treatment they already had everywhere else
(`AccessService.SetContext`'s leak tripwire, `UserServiceDeliveryPipeline`'s
`shouldStamp`, `MeshNodeStreamCache`'s pass-through), so nothing about hub
credentials changes.

Note the shape, because it is why the condition survived: the gate's own INPUT
decided whether the gate ran, so "the check did not run" and "the check passed"
were indistinguishable — the runtime cousin of AGENTS.md's no-skip-trapdoor rule
for CI, which this very file already states for its missing-evaluator branch.

Evidence (measured, not assumed):
  * pre-fix  — RoutedApiTokenClampTest.ABearerDeliveryWithNoClaims_IsClampedOnThePerNodeHub
               FAILS: the claimless Bearer read of an `api: false` partition is SERVED.
  * post-fix — both cases pass; the control arm (same person, same node, same
               routed message, no Bearer flag) is served both before and after,
               so the pin measures a capability decision, not a blanket deny.

Docs: AccessContextPropagation gains "Restoring the caller on the receiving hub"
(the third restore site and why it exists); AccessControl's "Known gap" note is
closed; the stale rationale in `UserContextMiddleware` is corrected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa
Copilot AI lite review requested due to automatic review settings September 1, 2026 19:12
@github-actions
github-actions Bot enabled auto-merge September 1, 2026 19:12
@rbuergi
rbuergi disabled auto-merge September 1, 2026 19:14

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 propagation defect where message-routed permission checks on per-node hubs could skip restoring the caller’s AccessContext (previously gated on Roles.Count > 0), causing the API-token clamp (IsApiToken) to be missed for claimless tokens and allowing reads that should be blocked when api: false is in effect.

Changes:

  • Update AccessControlPipeline to restore the delivery AccessContext based on “real principal” criteria (non-empty ObjectId, not hub-shaped) rather than role-claim presence.
  • Add a regression test that reproduces the routed-path clamp failure for an API token with empty role claims, plus companion assertions.
  • Update documentation (AccessControl + AccessContextPropagation) and related comments, and add a What’s New entry describing the fix.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/MeshWeaver.Graph.Test/RoutedApiTokenClampTest.cs Adds regression coverage for the routed-path API-token clamp behavior.
test/MeshWeaver.Graph.Test/ApiTokenCapabilityFreshnessTest.cs Updates commentary to reflect that AccessContext.Roles is diagnostic-only and references the new regression.
src/MeshWeaver.Hosting/Security/AccessControlPipeline.cs Fixes the restore condition so routed checks see IsApiToken/IsHub when appropriate.
src/MeshWeaver.Hosting.AspNetCore/Portal/UserContextMiddleware.cs Updates rationale/comments to match the new restore behavior and defect history.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-a-partition-closed-to-the-api-is-now-closed-on-every-path.md Adds a user-facing release note about the security fix.
src/MeshWeaver.Documentation/Data/Architecture/AccessControl.md Closes the documented “known gap” and links to the new propagation explanation section.
src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md Documents the third restore site and why it must not be keyed on role-claim shape.

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

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

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

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

586 tests   586 ✅  54s ⏱️
  2 suites    0 💤
  2 files      0 ❌

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    3 files      3 suites   1m 16s ⏱️
1 218 tests 1 026 ✅ 192 💤 0 ❌
1 219 runs  1 027 ✅ 192 💤 0 ❌

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

529 tests   527 ✅  1m 33s ⏱️
  4 suites    2 💤
  4 files      0 ❌

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

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

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 687 tests   1 687 ✅  1m 45s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 1ac38a3.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test Results

   14 files     14 suites   9m 31s ⏱️
4 655 tests 4 461 ✅ 194 💤 0 ❌
4 656 runs  4 462 ✅ 194 💤 0 ❌

Results for commit 1ac38a3.

@rbuergi
rbuergi merged commit 7965504 into main Sep 1, 2026
27 checks passed
@rbuergi
rbuergi deleted the fix/2976-restore-access-context-unconditionally branch September 13, 2026 10:27
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.

AccessControlPipeline restores a delivery's AccessContext only when Roles is non-empty — an API token with no claims reaches a per-node hub unclamped

2 participants