Repository navigation
cnx-ios-api: per-user signaling hub (users on one team never see each other's devices) - #22260
Closed
azooz2003-bit wants to merge 13 commits into
Closed
azooz2003-bit wants to merge 13 commits into
azooz2003-bit wants to merge 13 commits into
Conversation
Two users on one team must not see, list, signal or reach each other's devices through any REST endpoint or hub message. Fails on the per-team TeamHub: presence and team Mac lists cross users. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The TeamHub Durable Object is now one per user (idFromName("user:" +
owner key from the verified Stack token)). Signaling, presence,
introductions and wg-endpoints only ever span one user's own devices.
GET /v1/teams/:teamId/macs lists only the caller's own Macs. The hub binds
its owner on first use and refuses any other. The signal URL is unchanged,
so old phone and Mac builds reconnect to their user hub after deploy.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
As with team hubs, only a Mac that re-registers into another team is closed with 4001; a phone's link keeps signaling (admission is by user). Document hub_unavailable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hibernated sockets survive a deploy, so a Mac could stay parked in its old team hub while its phone reconnects into the user hub and sees it offline. The first time an isolate serves a team (after the membership check), its old team hub closes its sockets with 1012 and deletes its rows, which mixed the team's users. A user hub is never retired. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
…are retired Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Found 14 test failures on Blacksmith runners: Failures
...and 4 more test failures. View all on Blacksmith
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.svg)
- retireTeamHub recreates its tables after deleteAll (a second call on the same live object threw), skips sockets it already closed, and logs a failure instead of retrying on every request. - A hub binds only an owner key that names the object itself (idFromName(user:<key>) equals its id). An older Worker can no longer bind a team hub during the deploy overlap and so exempt it from retirement. - Seeding runs once per hub and skips devices deleted there, so a database read that raced a deletion cannot bring a Mac back. - Tests for each, and for a device id re-registered by another user. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Probably yours: CI failed on
Matched log linesNot re-run automatically: Written by
|
Collaborator
Author
Correctness-first review (fresh subagent) and resolutionIsolation: no leak found.
Should-fix, all fixed in
Nits:
Test gaps added:
Cross-project same-sub isolation was already covered in After the fixes: 57/57 Worker tests pass. On the redeployed |
Collaborator
Author
|
Deployed 0f6b3e6 (API 0.10.0) on the user's directive, 2026-10-20:
|
A retire RPC can fail while the hub restarts onto the new class during a deploy; since 0f6b3e6 the isolate then never retries, and a Mac stays parked in its old team hub, offline to its phone (transport verify finding 5). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ach minute
Root cause of transport-verify finding 5: around the 0.10.0 deploy the
lazy retire RPC reached team hubs still on the previous class ("The RPC
receiver does not implement the method retireTeamHub"). 0f6b3e6 made
an isolate remember a team as retired before the RPC succeeded and never
retry, so the Mac's socket stayed parked in its team hub and its phone
saw it offline until the socket dropped.
- Only a successful retire is remembered; a failure is retried after
10 s.
- A minute cron (retireAllTeamHubs) retires the hub of every team id in
devices and team_prefs, in each project's naming, without waiting for
a request. A retired hub records it, so later sweeps do no writes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Live on a repro Worker: after a deploy, steady retire calls kept the team hub warm on the previous class for minutes; every retireTeamHub failed with "does not implement" and the Mac stayed parked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A Durable Object keeps the previous class after a deploy until evicted, and traffic keeps it warm, so retireTeamHub could fail for minutes. On "does not implement", call the previous class's forgetMac for each device the database has in that team (closes 4001 moved; hostd and the app reconnect, into their user hub). The cron sweep and lazy retires pass the team's device ids. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
Incident: Macs stayed parked in old team hubs after the 0.10.0 deploy (transport-verify finding 5), fixedRoot cause. After a deploy, the Worker serves the new code at once. A Durable Object keeps running the previous class until it is evicted. Steady traffic, such as a polling phone or the retire calls themselves, can keep it warm for minutes.
Reproduced live on a separate Worker: 0.9.0 base, an old Fix (
Tests: 60/60 Worker tests pass. New tests cover:
Verified live on the repro Worker: 0.9.0 base, old hostd connected, then a fix deploy while polling every 2.5 s.
Rollout:
The repro Worker was deleted after the test. Its test user's data was purged. |
- scheduled awaits the sweep and logs a failure instead of an unhandled rejection. - The sweep skips teams with no devices (no hub to create) and ignores a request's 10 s backoff; the signal path passes its device id. - A bad TEAM_HUB_RETIRE_RETRY_MS falls back to 10 s. - PROTOCOL: the fallback reaches devices the database has in that team; the sweep is temporary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
Review of the incident fix (fresh subagent) and resolutionNo blocker. The subagent confirmed:
Fixed in
Redeployed:
Sweep logs at 05:00 UTC:
The counts are one lower than before because prefs-only teams are no longer swept. 60/60 Worker tests pass. |
azooz2003-bit
added a commit
that referenced
this pull request
Oct 20, 2026
…into feat-cnx-ios-dogfood-20261020 # Conflicts: # cnx-ios/backend/src/env.ts # cnx-ios/backend/test/http.test.ts
This was referenced Oct 21, 2026
Closed
cnx-ios: after a redial the shown browser tab keeps its video; terminals follow a Mac restart
#22419
Closed
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.
Summary
The cnx-ios-api signaling hub (
TeamHubDurable Object) was one per team, so two users on the same team shared presence,helloMac lists andGET /v1/teams/:teamId/macs. It is now one per user. Different users never see, list, signal or reach each other's devices, even on a shared team.idFromName("user:" + owner key). The owner key is the verified Stack token'ssub(<projectId>:<sub>outside the primary project), never a client value. A hub binds its owner on first use and refuses calls for any other owner, so a routing bug cannot mix two users.teamIdmust be a team the caller belongs to, and the device must be registered there (else403 wrong_team).helloandpresencelist only the caller's Macs in the socket's team, as before. Signal admission is by user only, so a phone may dial any of its own Macs in any team.GET /v1/teams/:teamId/macslists only the caller's own Macs;ownerUserIdis always the caller. Every other route was already owner-scoped and now reads presence from the caller's hub.wg-endpoints, and the retired-relay refusal (relay_retired). Push (notify) never went through the hub and stays owner-scoped.Migration
cnx-hostdstayed connected through the deploy. So a Mac could stay parked in its old team hub while its phone lands in the user hub and sees it offline. Fix: the first time an isolate serves a team (after the membership check), the Worker callsretireTeamHubon the old hub. That closes its sockets with1012, so clients reconnect into their user hub, and deletes its rows, which mixed the team's users. A user hub is never retired: a hub binds only an owner key whoseuser:<key>name is the object's own id. Remove this once no team hub holds sockets.forgetMac(4001moved); and a cron every minute sweeps every team's hub. See the incident comment below.hellostill lists a registered Mac that has not reconnected yet. Devices deleted in the hub are never seeded back.4001 moved. A phone's socket stays open, as with team hubs, so its links keep signaling.Verification
Worker tests (vitest in workerd, run on mini-cmux11s, not the laptop): 60 passed.
test/isolation.test.tscovers two users on one team across REST lists, bootstrap,/me/*, takeover, delete, push, notify, hello, presence, signals of every kind includingwg-endpoints, session reuse in both directions, a spoofedfrom, opening a socket as a teammate's device, and the owner binding.ac971780df0).Live, against a separate Worker
cnx-ios-api-userhub-dev(dev env, shared dev DB through Hyperdrive, fresh dev Stack userscmux-next-ios-userhub-{c,d}@example.com). Nothing was deployed to the shared dev or prod Worker.scripts/smoke.ts, user C, personal space): 31/32 pass. The failure isice turn: this Worker has no current TURN key (see below).cnx-hostd): 18/18 pass. D's/teams/:id/macs,/me/macs,/me/devicesand bootstrap show nothing of C. Takeover, delete, notify,/teams/<C>/macsand/teams/<C>/hubare refused. Sockets opened as C's Mac are refused. D'swebrtc-offer,wg-init,wg-endpointsandbyeto C's online Mac all getnot_owner. D's phone still reaches D's own Mac. D's sockets heard nothing of C.cnx-hostdconnected, oldcnx-dialWebRTC direct in 298 ms./me/macsshowed it offline./me/macsretired the hub. The Mac loggedsignaling closed by server: 1012 moved to the user hub, re-registered and reconnected in 0.9 s, and was online 3 s later.cnx-dialthen connected to the old Mac over WebRTC: direct, 400-1500 ms.turn, 462 ms). I could not set TURN on this Worker: the local copy of the TURN key is rotated (403), andopis not signed in.Not live-tested: two different users on the same Stack team. No two test users share a team. Stack refuses client-side membership adds (
INSUFFICIENT_ACCESS_TYPE), no Stack server key is available, and an invitation needs an email code. The same-team case is covered by the Worker tests above, and the per-user hub makes the team irrelevant to isolation.No Swift or Rust code changed, so no Swift or Rust tests ran. hostd and the iOS app keep the same URL, and the app already filters other users' Macs.
Review
A correctness-first review subagent found no isolation leak and no blocker. Its three should-fix findings in the migration and seeding paths are fixed in
0f6b3e64007; details are in the PR comment.Changelog
none
🤖 Generated with Claude Code