feat(local-node-host): coordinate tenant session transitions - #3244
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
thought (non-blocking): Accessibility audit (advisory)The sharded axe audit is report-only while the baseline and runtime budget mature.
Shard 1 reportShard 2 reportShard 3 report |
The installation home-decision authority dispatches on the coordinator row's command type, and the switch command type had no arm. Every real tenant switch therefore fell to the default arm, which admits only the coordinator's own membership-mutation schema, and threw at the first tenant-head finalization: a 500, a coordinator wedged at Committing behind a deterministic correlation id, and prepared intents left dangling in both tenant authority documents. Register the arm through a new ValidateStoredSwitchTenants adapter. It runs the existing stored-payload validation and returns the ordinal-ordered old/target tenant pair the sibling arms' contract expects, which is the same pair the authority already writes to the coordinator row. The card's existing switch tests could not see this: they drive the authority through a hand-rolled membership-store double that fabricates its own receipts and never crosses the fence. With the arm deliberately deleted those four tests still pass. Two gates now close that hole: - a real-seam test that drives a switch through the real encrypted tenant membership stores and the real home-decision authority, with the old session minted by the real challenge and selection authorities over a real Argon2id credential. It goes red when the arm is removed. - an architecture test asserting every command type declared under Data/Identity resolves to its own arm rather than the default one, so the next coordinator author gets a failing build instead of a production incident. Full local-node-host suite: 1586 passed, 18 skipped, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N7ge8CQJE4tyGHQSddBYmJ
1 participant
Intent
Complete the MTW-01C session/audience isolation and coordinated-transition authorities:
miss, refusal, or expired selected handle cannot fall through to a legacy credential;
revocation receipt, target-tenant selection receipt, and installation audit head are all durable;
The existing red fixtures remain unchanged.
Review round 2 — both blockers closed (
fc9fd72e6)Deep review (
council-verdict-2026-07-28T1021Z) returned CHANGES REQUESTED with two blockers.Both are fixed in
fc9fd72e6.BLOCKER 1 —
WebTenantSwitchwas not admitted by the home-decision fenceInstallationIdentityHomeDecisionAuthority.RequireAsyncdispatches onrow.CommandType;WebTenantSwitchAuthority.CommandTypehad no arm, so every real switch fell to the default arm(
ValidateStoredCoordinator, which demandsTenantMembershipMutation/ schema 2) and threwidentity.coordinator_payload_invalidat the first tenant-head finalization — a 500, a coordinatorwedged at
Committingbehind a deterministic correlation id, and dangling prepared intents in bothtenant authority documents.
The arm is now registered through a new
WebTenantSwitchAuthority.ValidateStoredSwitchTenantsadapter.
On
ValidateStoredSwitchitself: it needed nothing. It was already correct and current — it iscalled on the live path (
SwitchAsyncresume + post-create revalidation), so it could not havedrifted from the payload. It validates command type + schema version, payload presence, the
recomputed payload digest, the recomputed correlation id and fingerprint, the account security
version, old-vs-target tenant distinctness, and
TenantIdsJsonagainst the ordinal-ordered expectedpair. The only gap was shape: the sibling arms return
string[](admitted tenant ids) whileValidateStoredSwitchreturns the payload. The adapter runs the same validation and returns[OldTenantId, TargetTenantId]in the ordinal orderCreateHomeAsyncwrites — the same setValidateStoredSwitchhas just proven the row still carries. No behaviour change to the validator.BLOCKER 2 — the test seam could not see BLOCKER 1
WebTenantSwitchRealSeamTests(new) drives a switch through the REALEncryptedTenantMembershipAuthorityStoreover REALSqlCipherEncryptedStores, constructed with theREAL
InstallationIdentityHomeDecisionAuthorityover the REAL migrated identity database — thecomposition
Program.csbuilds viaTeamContextTenantIdentityAuthorityPartitionResolver. Bothmemberships are created by the REAL coordinator, and the old selected session is minted by the REAL
challenge issuer + REAL tenant-selection authority over a REAL Argon2id credential. No session,
membership, or receipt row is inserted directly. Substituted (mirroring the Mtw2 recipe, all
non-teeth): the partition RESOLVER, the party reader, the lease coordinator, and the admission seam.
Two tests: the end-to-end switch (asserting real fence-minted home-decision digests, not the
double's constants), and a direct probe that the arm admits exactly the two tenants of the durable
switch and refuses a third with
identity.home_decision_mismatch.Falsification evidence (arm deliberately deleted, then restored):
WebTenantSwitchRealSeamTests+ registry arch testWebTenantSwitchAuthorityTests+TenantSwitchRoutesTestsEvery red carried the reviewer's exact stack:
The middle row is BLOCKER 2 stated empirically: the card's original 606-line suite stays fully green
with the defect present, because
RecordingMembershipStorefabricates its own receipts and nevercrosses the fence.
Drift canary (reviewer's recommendation, adopted)
InstallationIdentityHomeDecisionRegistryArchTestsscans everyconst string CommandTypedeclaredunder
Data/Identity/and drives each one through the REAL fence over a REAL identity database. Aregistered command type refuses with its OWN error code; only the default arm answers
identity.coordinator_payload_invalid. A new command type without an arm now fails the buildinstead of production. It found no other unregistered command type — the four declared today
(
WebTenantSelection,WebSelectedSessionLogout,WebTenantSwitch, and the coordinator's ownTenantMembershipMutation, which legitimately owns the default arm) all resolve correctly.Round-2 verification
WebTenantSwitchRealSeamTests, registry arch test,WebTenantSwitchAuthorityTests,TenantSwitchRoutesTests,InstallationIdentityCoordinatorServiceTests,TenantMembershipAuthorityStoreTests,WebTenantSelectionAuthorityTests,WebSelectedSessionLogoutAuthorityTests,Mtw2TwoUserAcceptanceE2E,InstallationIdentityDormancyArchTests): 45/45 passedhost inactivity abort seen on the first round did not reproduce)
dotnet format --verify-no-changeson all four changed files: passedgit diff --check: passedKnown follow-ups (NOT in this PR — carded separately)
with no re-issue path.
TenantSwitchRoutes.SwitchAsyncconsumes before calling the authority andreturns 401 on refusal without emitting a replacement;
IWebAntiforgeryPolicyhasRotateChallengeAsyncbut noRotateSelectedAsync, andIssueAnonymousAsyncrefuses when aselected cookie is present. One refused switch therefore disables every state-changing
selected-audience request including logout — and clicking your CURRENT tenant in a switcher is
enough to trigger it. Not introduced by this PR (four sibling routes consume-without-rotate), but
switch is the first such route whose refusal is routine. Sized before punting: the fix is a new
interface member, a small
RotateAudienceAsyncgeneralisation inWebAntiforgeryPolicy(
ResolveSubjectAsyncalready handles the selected audience), one call on the refusal path, plusthe seven test fakes that implement the interface — roughly 11 files. That is an interface change
on a security boundary and warrants its own review, so it is not folded in here.
Committing/Finalizing/Completedbranches do not callRequireCurrentActiveSessionAsync, which thePreparingbranchdoes, and neither
RotateSessionsAsyncnorHasSameAuthorityCoordinateschecksIdleExpiresAtUtc. Two upstream gates make it unreachable today; it is an asymmetry worth closing.Round-1 red-green-red proof (unchanged)
MTW00C_RUN_RED=1 dotnet test apps/local-node-host/tests/tests.csproj --filter "Category=RedFixture"failed all 18 red fixtures. The seven MTW-01C fixtures each failed withits named
MissingAuthorityException.filter failed 7/7 with the original named
MissingAuthorityExceptionreasons.The deep review correctly identified this proof as tautological (a red fixture's pass/fail is a pure
function of a string literal in a
HashSet). The real-seam test above is the gate that actuallyexercises the wiring; the red-fixture harness itself is tracked on its own card and is deliberately
untouched here.
Closes #3239