Repository navigation
Issue SignalR token validation on its own hub, not the mesh router - #1066
Merged
Merged
Conversation
ROUTER_TRAFFIC ERROR lines in production trace to the ROOT MESH HUB being an
END of a delivery, not a hop:
- MeshExtensions.NodeOperationTarget fell back to hub.GetMeshHub().Address for
every hub with no WithNodeOperationExecution ancestor — i.e. every per-node
hub. The create ran on the router's action block AND its response came back
stamped Sender = mesh/{id} (the prod "RawJson has the mesh hub as sender"
line, reported once per per-node hub). It now falls back to a dedicated
portal/nodeops-{meshId} execution hub.
- SignalRConnectionRegistry issued its ValidateTokenRequest on the DI-injected
IMessageHub, which IS the root mesh hub. It now uses its own transport portal
hub, mirroring GrpcConnectionRegistry.TransportHub.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A hosted hub's MessageHubConfiguration starts EMPTY (CreateMessageHub builds a fresh one; nothing is inherited from the parent), and ResolveEvaluator does not walk the parent chain — it is `hub.Configuration.Get<EffectivePermissionsDelegate>() ?? DefaultEvaluator`, and the default returns Permission.All. So the new nodeops execution hub granted every write until it copied the mesh hub's evaluator. Caught by OwnerlessPartitionRepairTest: a stranger's create into an ownerless partition succeeded where it must be refused. Also re-gates EnsurePartitionBootstrap on "is the central node-operation hub" instead of "is the mesh hub" — the bootstrap runs where the create executes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SignalRConnectionRegistry.Authenticate posted its per-connection
ValidateTokenRequest on the DI-injected IMessageHub — which IS the root
mesh/{id} hub, the mesh's ROUTER. That makes the router an END of the delivery
in both directions: the request reaches ApiToken/{hashPrefix} stamped
Sender = mesh/{id}, and the response is addressed straight back at mesh/{id}.
Both are what the ROUTER_TRAFFIC detector reports, and in production they were
part of the largest single source of ERROR lines (~2,385/24h across the three
portals).
GrpcConnectionRegistry already had the fix (its TransportHub); SignalR is now
wired the same way — a stable portal/signalr-{instance} hub with the
RegisterStream registration Orleans needs for the response to land cross-silo.
RouterTrafficTest pins it by capturing the detector's own output and asserting
the mesh hub never ORIGINATES a delivery. The assertion is on the sender role
only, deliberately: "mesh hub as target" also fires for a pure routing hop
(HierarchicalRouting.RouteAlongHostingHierarchy:218 hands every non-local
delivery to parentHub.DeliverMessage, and that parent is the mesh hub for every
hub in the process), and those hops are the design.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents SignalR token validation from originating on the root mesh/{id} hub (the mesh router) by issuing the ValidateTokenRequest from a dedicated, stream-registered transport portal hub instead. This aligns SignalR with the existing gRPC transport pattern and reduces ROUTER_TRAFFIC ERROR noise caused by the router becoming an end-of-delivery.
Changes:
- Introduces a lazily-created
TransportHub(portal/signalr-{instance}) inSignalRConnectionRegistryand uses it to issueValidateTokenRequest. - Adds an integration-style regression test (
RouterTrafficTest) that capturesROUTER_TRAFFIClogs and asserts the router never sends such deliveries. - Adds the corresponding “What’s New” entry documenting the user-visible impact.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/RouterTrafficTest.cs | Adds a regression test that fails if the mesh router originates ROUTER_TRAFFIC during SignalR authentication. |
| test/MeshWeaver.Hosting.Monolith.Test/MeshWeaver.Hosting.Monolith.Test.csproj | Adds a project reference needed to compile/run the new SignalR-focused test. |
| src/MeshWeaver.Hosting.SignalR/SignalRConnectionRegistry.cs | Routes token validation through a dedicated, stream-registered transport hub rather than the root mesh hub. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-09-token-checks-no-longer-run-on-the-router.md | Documents the behavioral change as a fix in the changelog. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Test Results (shard 1)1 195 tests - 26 1 195 ✅ - 26 6m 21s ⏱️ +48s Results for commit 0651681. ± Comparison against base commit bd250ef. This pull request removes 67 and adds 41 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Test Results (shard 2)1 968 tests +27 1 966 ✅ +27 6m 45s ⏱️ +21s Results for commit 0651681. ± Comparison against base commit bd250ef. This pull request removes 41 and adds 68 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
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
MeshWeaver.Messaging.MessageHublogsROUTER_TRAFFICat ERROR ~2,385 times per 24 h across thethree production portals — the single largest source of red log lines. The detector fires when the
ROOT MESH HUB (the mesh's router) is an end of a delivery.
SignalRConnectionRegistry.Authenticateissued its per-connectionValidateTokenRequeston theDI-injected
IMessageHub— which is the rootmesh/{id}hub. That makes the router an end of thedelivery in both directions:
ApiToken/{hashPrefix}stampedSender = mesh/{id}→RawJson has the mesh hub as sender (sender: mesh/…, target: ApiToken/…)ValidateTokenResponse has the mesh hub as target (sender: ApiToken/…, target: mesh/…)Both shapes are in the production logs verbatim.
GrpcConnectionRegistryhad already been fixed (itsTransportHub, with a 🚨 doc block explaining exactly this); SignalR had not.The fix
SignalRConnectionRegistrygets the sameTransportHub— a stableportal/signalr-{instance}hub,created lazily, with the
RegisterStreamregistration Orleans needs for the response to landcross-silo (
portalis a stream-routed address type). One line changes at the call site; the rest isthe hub property and its rationale.
The test
RouterTrafficTestcaptures the detector's own ERROR output and asserts the mesh hub neveroriginates a delivery.
🚨 The assertion is on the sender role only, deliberately. "Mesh hub as SENDER" is unambiguous —
the router actually originated that message. "Mesh hub as TARGET" is reported by
MessageHub.DeliverMessagefor the receiving hub, so it also fires for a pure routing hop:HierarchicalRouting.RouteAlongHostingHierarchy:218hands every delivery a hosted hub cannot routelocally to
parentHub.DeliverMessage, and for every hub in the process that parent is the mesh hub.Asserting on those would pin an invariant the architecture does not hold — see the note below.
Verified to bite: with the fix reverted the test fails with
What this does NOT fix — and what I learned about the rest
A deterministic repro (
MonolithMeshTestBase, capturing everyROUTER_TRAFFICline) showed theremaining lines split into two groups:
ValidateTokenRequest/ValidateTokenResponse/SubscribeRequest/DataChangedEvent/SubscribeAckreported as "mesh hub as target". These come fromHierarchicalRouting.RouteAlongHostingHierarchy:218, not from any application caller. They cannotbe removed: that hop is what gives
RoutingServiceBase.RouteInMeshits single-threadedarrival-order guarantee for per-address activation. The detector is untouched — no level
change, no predicate change, no mute.
MeshExtensions.NodeOperationTargetfalls back tohub.GetMeshHub().Addressfor every hub with noWithNodeOperationExecutionancestor, i.e. everyper-node hub. This is the genuine, high-volume half (the production
RawJson has the mesh hub as sender (… target: …/Source/FNodeTypeAtomicSolution)line, reportedonce per per-node hub). It is deliberately not in this PR — see the review notes for why it
needs its own cycle.