Repository navigation
fix(#1140): token validation is issued off the ROUTER - #4227
Merged
Merged
Conversation
UserContextMiddleware resolves its hub as
`RequestServices.GetService<PortalApplication>()?.Hub ?? GetRequiredService<IMessageHub>()`.
The first is the portal hub and is fine. The FALLBACK resolves the ROOT MESH HUB
in the root container — which is what a portal without the Blazor shell
(Features:Gui:Blazor=false) gets, and what any request outside a circuit scope
gets. A ValidateTokenRequest posted there reaches the ApiToken node's hub stamped
`Sender = mesh/{id}` and its response is addressed straight back at the router.
`ValidateTokenRequest/Response` is named among the observed types in #1140's own
evidence from memex, and the new test captures the pair verbatim on main:
RawJson sender=mesh/5rc-… target=ApiToken/24eb4c912dad
ValidateTokenResponse sender=ApiToken/… target=mesh/5rc-…
ReadIssuingHub, not NodeOperationIssuingHub. This is a bounded READ with an HTTP
request waiting on it: portal/reads-{meshId} registers NO handlers, so its block
only ever dispatches replies to reads issued on it, whereas the node-CRUD
execution hub would put this reply behind every write in flight — #2901's
~10.3 s-then-503 shape. Both seams are the identity function for any hub that is
not the router, so the portal-hub path is byte-for-byte what it was.
The test's "the token is real" assertion is not decoration: on its first run the
validation faulted with a DeliveryFailure (nothing writes the ApiToken/{prefix}
routing index automatically — ApiTokenService does, in production), and both
ROUTER_TRAFFIC assertions passed having observed no exchange at all. The fixture
now creates the index too, and the Success assertion is what caught it.
Verified: the test fails without the change and passes with it;
Memex.Portal.Shared.Test 1468/1468; `dotnet build -c Release -warnaserror` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The teardown fallback can recreate router traffic, and test-related review findings remain unresolved.
Pull request overview
Moves API-token validation off the root message router onto the dedicated read hub.
Changes:
- Routes validation through
ReadIssuingHub(). - Adds regression and positive-control integration tests.
- Documents the fix in the What's New entry.
File summaries
| File | Summary |
|---|---|
test/Memex.Portal.Shared.Test/TokenValidationIsIssuedOffTheRouterTest.cs |
Adds validation and router-isolation coverage; documentation scope and capture collection require changes. |
src/MeshWeaver.Hosting.AspNetCore/Portal/UserContextMiddleware.cs |
Uses the read hub, but teardown fallback can still post through the root router. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-13-api-token-checks-no-longer-run-through-the-message-router.md |
Adds documentation for the API-token validation fix. |
Review details
Suppressed comments (4)
src/MeshWeaver.Hosting.AspNetCore/Portal/UserContextMiddleware.cs:556
ReadIssuingHub()is not guaranteed to be off-router:MeshReadHub()returnsnullonce the mesh reachesDisposeHostedHubs, and the extension explicitly falls back to the originalhub. If a request races teardown, this call therefore postsValidateTokenRequestfrom the root mesh hub again, recreating the exact ROUTER_TRAFFIC path this fix claims to eliminate. Handle an unavailable read hub as a validation-unavailable result instead of posting through the router.
() => hub.ReadIssuingHub().Observe(
new ValidateTokenRequest(rawToken),
o => o.WithTarget(tokenAddress))
.Select(d => (ValidateTokenResponse?)d.Message))
src/MeshWeaver.Hosting.AspNetCore/Portal/UserContextMiddleware.cs:540
- The fallback is not what every out-of-circuit request gets:
PortalApplicationis registered as scoped, and its own contract says SSR/prerender/middleware scopes without a circuit use aportal/{userId}hub. Only scopes wherePortalApplicationis unavailable (such as a no-Blazor installation) reach the rootIMessageHub; please narrow this comment so it does not misstate which requests this fix affects.
// 🚨 ISSUED OFF THE ROUTER (#1140). `hub` above is the portal hub when there is one — but
// the fallback, `RequestServices.GetRequiredService<IMessageHub>()`, resolves the ROOT MESH
// HUB in the root container, which is what a portal without the Blazor shell
// (Features:Gui:Blazor=false) and any request outside a circuit scope gets. Posting there
// makes the router an END of the delivery in both directions: the request reaches the
test/Memex.Portal.Shared.Test/TokenValidationIsIssuedOffTheRouterTest.cs:32
- This test documentation repeats the same overbroad claim: SSR/prerender/middleware requests can have no circuit while still resolving the scoped
PortalApplication, so they use aportal/{userId}hub rather than this root fallback. Please describe the fallback as applying whenPortalApplicationis not registered; otherwise the test claims to cover a request shape that its setup does not establish.
/// <para><c>UserContextMiddleware</c> resolves its hub as
/// <c>RequestServices.GetService<PortalApplication>()?.Hub ?? RequestServices.GetRequiredService<IMessageHub>()</c>.
/// The first is the portal hub and is fine. The FALLBACK resolves the ROOT MESH HUB in the root
/// container — which is what a portal running without the Blazor shell
/// (<c>Features:Gui:Blazor=false</c>) gets, and what any request outside a circuit scope gets. A
test/Memex.Portal.Shared.Test/TokenValidationIsIssuedOffTheRouterTest.cs:212
- This new capture stores records in a mutable
ConcurrentQueue. The repository's collection policy prohibits mutable collections intest/(the only concurrent-mutation exception is an instanceConcurrentDictionary); use an immutable snapshot updated atomically, or an approved capture abstraction, so concurrent logger callbacks remain safe without introducing a forbidden collection.
private readonly ConcurrentQueue<RouterTrafficRecord> _records = new();
internal RouterTrafficRecord[] Records => _records.ToArray();
- 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.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Sep 13, 2026
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.
The defect
UserContextMiddlewareresolves its hub asThe first is the portal hub and is fine. The fallback resolves the ROOT MESH HUB in the root container — which is what a portal running without the Blazor shell (
Features:Gui:Blazor=false) gets, and what any request arriving outside a circuit scope gets.ValidateTokenViaHubthen posts from it, so the request reaches the ApiToken node's hub stampedSender = mesh/{id}and the response is addressed straight back at the router.ValidateTokenRequest/Responseis named among the observed types in #1140's own evidence frommemex, and the new test captures the pair verbatim onmain:This is the second call site of #1140 and the one this repo owns; the first —
StartThread, which produced the 09-11/09-12 samples — is MeshWeaver.Plugins#1803.Why
ReadIssuingHuband notNodeOperationIssuingHubThis is a bounded read with an HTTP request waiting on it.
portal/reads-{meshId}registers no handlers, so its block only ever dispatches replies to reads issued on it. The node-CRUD execution hub would put this reply behind every write in flight — #2901's ~10.3 s-then-503 shape, on the authentication path.Both seams are the identity function for any hub that is not the router, so the portal-hub path is byte-for-byte what it was.
Negative control, and the thing it caught
The fixture carries a positive control —
TheCapture_RecordsAViolation_WhenTheRouterGenuinelyPostsWorkmakes the router an end on purpose and requires the line, asserting on role and ends rather than the message type.It also carries a non-vacuity assertion that earned its place on the first run: the validation faulted with a
DeliveryFailure, because nothing writes theApiToken/{hashPrefix}routing index automatically (ApiTokenServicedoes, in production). BothROUTER_TRAFFICassertions passed — having observed no exchange at all. The fixture now creates the index too, andresponse.Success.Should().BeTrue(...)is what caught it. An assertion that cannot fail is not an assertion, and this one could have shipped as a green test over a path that never ran.Verified locally
Memex.Portal.Shared.Testdotnet build -c Release -warnaserrorMeshWeaver.Hosting.AspNetCoreand the test projectRelationship to the other #1140 PRs
ROUTER_TRAFFIC ORIGIN:). Independent of this.StartThreadsite, the one the recent samples name.Refs #1140; does not close it. That closes on a boot of
memexwith zeroROUTER_TRAFFIClines, which needs all three merged and rolled.🤖 Generated with Claude Code