diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-12 09:47:07 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-12 16:36:54 +0200 |
| commit | 4cce50f09a73739387d5058a6f8183ebac65ae2c (patch) | |
| tree | 30e9d72280295d913983910a956b0d3306696ec8 /packages/meshbay-hub | |
| parent | 9cc2909cb4a360c81b471ceab1d9578a7655a88e (diff) | |
| download | meshbay-4cce50f09a73739387d5058a6f8183ebac65ae2c.tar.gz | |
fix: an empty group claim is a claim on nothing
A node that hosts no groups sends no `group_ids` on its hub socket, and the
hub resolved the claim with `set(claimed_groups or authorized)` — so "I host
nothing" arrived as "I host every group this account belongs to", other
members' included. Such a node can serve none of them: it holds no GEK, and
its own handshake refuses them with "Group not hosted on this node".
`/v1/groups/{id}/nodes` answers in registration order and `_node_groups` is
in-memory, so which node a client was sent to depended on who reconnected
first after a hub restart. GroupPage took `nodes[0]` with no fallback. On
2026-09-11 a hub deploy at 20:14 reshuffled the registry, a second member's
unconfigured node won the race, and a group stopped opening for everyone in
it with its only real host online throughout. Any member could take one of
their groups down, by accident, by leaving an empty node running.
Four changes, because no one of them is sufficient:
- the hub never widens an absent claim, and `update_groups` goes through
the same ceiling as registration — it assigned its list verbatim, so the
bound that makes C2 hold at authentication was one message wide
- the node states the empty set rather than omitting the field
- the refusal carries `not_hosted`, so a client can tell "try the next
node" from "you, here, must do something first"
- GroupPage walks the list instead of indexing into it
The three lines involved date from 13, 20 and 23 August and each is
defensible alone. The defect is in the seam, which is where the last two
also were: a falsy empty collection must never mean "unspecified".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4YmK41VsEURWFdop4EEeT
Diffstat (limited to 'packages/meshbay-hub')
5 files changed, 272 insertions, 22 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/api/revocation.py b/packages/meshbay-hub/src/meshbay_hub/api/revocation.py index 003e396..7c37a1e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/revocation.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/revocation.py @@ -171,6 +171,41 @@ async def _reject(ws: WebSocket, detail: str, code: int) -> None: await ws.close(code=code) +async def _authorized_groups(user_id: str) -> set[str]: + """The groups this account belongs to — the ceiling on what its nodes may claim. + + Read fresh rather than captured once at registration: a node WebSocket lives + for hours, and a group joined in the meantime has to become claimable through + `update_groups` without reconnecting. + """ + from meshbay_hub.db.engine import get_session_factory + + async with get_session_factory()() as db: + result = await db.execute( + select(GroupMember.group_id).where(GroupMember.user_id == user_id)) + return {gid for (gid,) in result.all()} + + +def _claimable(claimed_groups, authorized: set[str]) -> list[str]: + """What a node actually gets registered for. Two rules. + + A node may only *narrow* the set: `authorized` is the ceiling, or a node + could advertise itself as a source for any group on the hub (finding C2). + + And an empty claim means **no groups**, never "all of them". This used to + read `set(claimed_groups or authorized)`, so a node hosting nothing — which + sends no `group_ids` at all — was registered as a host for every group its + owner belonged to, other people's included. Such a node cannot serve any of + them: it holds no GEK, and its own handshake refuses them with "Group not + hosted on this node". But `/v1/groups/{id}/nodes` returns nodes in + registration order, so once one of them won the reconnection race after a + hub restart it became `nodes[0]` and captured the group's entire client + traffic. Any member could take a group down for everyone, by accident, + merely by leaving an unconfigured node running. + """ + return sorted(authorized & set(claimed_groups or ())) + + async def _authorize_node_ws(token: str, claimed_id: str, claimed_groups) -> tuple: """ Resolve a node WS registration against the database. @@ -212,8 +247,7 @@ async def _authorize_node_ws(token: str, claimed_id: str, claimed_groups) -> tup select(GroupMember.group_id).where(GroupMember.user_id == user_id)) authorized = {gid for (gid,) in result.all()} - claimed = set(claimed_groups or authorized) - return claimed_id, sorted(authorized & claimed) + return claimed_id, _claimable(claimed_groups, authorized) @router.websocket("/v1/nodes/ws") @@ -286,7 +320,12 @@ async def node_websocket(ws: WebSocket): from meshbay_hub.api.signaling import handle_webrtc_answer handle_webrtc_answer(msg) elif msg.get("type") == "update_groups": - new_gids = msg.get("group_ids", []) + # Through the same gate as the registration above. This used to + # assign the message's list verbatim, so the ceiling that makes + # C2 hold at authentication could be stepped over one message + # later: a node had only to reload to claim any group on the hub. + new_gids = _claimable(msg.get("group_ids"), + await _authorized_groups(user_id)) _node_groups[node_id] = new_gids await _mark_hosted(new_gids) log.info("Node %s updated groups: %d", node_id[:8], len(new_gids)) diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index c59d097..18d3a8f 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -311,7 +311,6 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, const sessionKeys = null; setStatus('connecting'); - const nodeId = nodesData.nodes[0].node_id; // Renewed here rather than taken from the prop. This effect no longer // re-runs when the token rotates (see the dependency list below), so // the captured one can be older than the session's — and it is used to @@ -319,25 +318,59 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, // connection at all. Renewals are shared, so if one is already in // flight this waits for it instead of starting a second. const live = (await ensureFreshToken()) || token; - // The same base the API calls use: signaling is a hub endpoint like - // any other, and two sources for one address is how they drift. - const transport = new window.MeshBayTransport(HUB, live); - transportRef.current = transport; - // Consulted only by the automatic reconnect after a WebRTC failure - // (transport.js's _reconnectLoop) — the token captured by this - // connect() call can be stale by then, since the whole point is that - // some real time (screen lock, a dead NAT mapping) passed unnoticed. - transport.onNeedToken = async () => (await ensureFreshToken()) || token; - // Set before connect(), because connect() is where device_hello runs — - // and again on every reconnect it makes, which is the case this exists - // for: nothing else tells the page the answer changed. - transport.onDeviceIdentity = (ok) => { - if (!cancelled) setDeviceReady(ok); - }; - const ack = await transport.connect( - nodeId, live, groupId, null, sessionKeys, session.bundleKey, username, - userId, session.pendingJoinCode, session.recoveryKey); + // Every node the hub lists, in turn — not `nodes[0]` and nothing else. + // The list is in hub registration order, and its head is not + // necessarily a node that can serve the group: one whose config does + // not list it refuses the handshake with "Group not hosted on this + // node". Stopping at the first made that refusal indistinguishable + // from the group being down, while a node that *could* serve it stood + // second in the same list — which is how `media` went dark on + // 2026-09-11 with its only real host online the whole time. The hub no + // longer registers a node for a group it does not claim; trying the + // rest is what keeps one bad entry from being fatal again. + // `transport` holds the attempt in progress, and keeps whichever one + // answers — so it is null after the loop exactly when none did. + let transport = null, ack = null, lastErr = null; + for (const n of nodesData.nodes) { + // The same base the API calls use: signaling is a hub endpoint like + // any other, and two sources for one address is how they drift. + transport = new window.MeshBayTransport(HUB, live); + transportRef.current = transport; + // Consulted only by the automatic reconnect after a WebRTC failure + // (transport.js's _reconnectLoop) — the token captured by this + // connect() call can be stale by then, since the whole point is that + // some real time (screen lock, a dead NAT mapping) passed unnoticed. + transport.onNeedToken = async () => (await ensureFreshToken()) || token; + // Set before connect(), because connect() is where device_hello runs — + // and again on every reconnect it makes, which is the case this exists + // for: nothing else tells the page the answer changed. + transport.onDeviceIdentity = (ok) => { + if (!cancelled) setDeviceReady(ok); + }; + try { + ack = await transport.connect( + n.node_id, live, groupId, null, sessionKeys, session.bundleKey, + username, userId, session.pendingJoinCode, session.recoveryKey); + break; + } catch (e) { + lastErr = e; + // Closed and dropped before anything else looks at either: an + // attempt that failed must not be handed to `releaseWhenIdle` by + // the unmount cleanup, which exists to keep a *working* connection + // alive for a download still using it. + try { transport.close(); } catch { /* never opened */ } + transport = null; + transportRef.current = null; + if (cancelled) return; + // A refusal that names a state of *this browser* — a code to enter, + // a passphrase, a device to approve — is the same answer from every + // node, and the operator to act on is this one's. Trying the next + // node would only replace it with a less useful message. + if (e.reason && e.reason !== 'not_hosted') throw e; + } + } + if (!transport) throw (lastErr || new Error('no node served this group')); session.pendingJoinCode = null; if (cancelled) return; setIsNodeAdmin(!!ack.is_node_admin); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index 32005b7..a21da19 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -283,6 +283,10 @@ const HANDSHAKE_REFUSALS = { version_too_new: 'This node is running an older MeshBay than this page needs. ' + 'Its operator has to update it.', version_unreadable: 'The node could not read this page\'s protocol version.', + // Surfaced only when it was the *last* node the hub offered for the group — + // group-page.js moves on to the next one on this code rather than stopping. + not_hosted: 'No node the hub offered for this group is hosting it. Its ' + + 'operator has to attach it on the node that holds its files.', }; const JOIN_REFUSALS = { diff --git a/packages/meshbay-hub/tests/test_node_ws_auth.py b/packages/meshbay-hub/tests/test_node_ws_auth.py index 1391722..4ead0d7 100644 --- a/packages/meshbay-hub/tests/test_node_ws_auth.py +++ b/packages/meshbay-hub/tests/test_node_ws_auth.py @@ -344,3 +344,123 @@ async def test_announce_with_valid_proof_succeeds_and_is_idempotent(client): assert second.status_code == 201, second.text assert second.json()["node_id"] == first.json()["node_id"], ( "re-announcing the same key must not create a second node record (M8)") + + +# ── An empty claim is not a claim on everything ────────────────────────────── +# +# 2026-09-11, found on a live deployment. `_claimable` used to read +# `set(claimed_groups or authorized)`, and the node omits `group_ids` entirely +# when it hosts nothing — so "I host no groups" was read as "I host all of +# yours". The node cannot serve any of them (no GEK, and its own handshake +# refuses them), but `/v1/groups/{id}/nodes` lists nodes in registration order, +# so whenever such a node won the reconnection race after a hub restart it +# became `nodes[0]` and the group stopped opening for every member. + + +@pytest.mark.asyncio +async def test_ws_absent_claim_registers_no_groups(client): + """A node that declares nothing hosts nothing — it must not inherit the set.""" + from meshbay_hub.api.revocation import _authorize_node_ws + + user = await _make_user(client, "empty1") + node_id = await _announce_node(client, user) + for name in ("has-one", "has-two"): + r = await client.post( + "/v1/groups", + json={"name": name, "visibility": "private", "join_policy": "invite"}, + headers={"Authorization": f"Bearer {user['token']}"}, + ) + assert r.status_code == 201, r.text + + resolved, groups = await _authorize_node_ws(_node_token(user), node_id, None) + assert resolved == node_id + assert groups == [], ( + "a node hosting nothing was registered as a host for its owner's groups") + + +@pytest.mark.asyncio +async def test_ws_explicit_empty_claim_registers_no_groups(client): + """And the same when the node says so out loud, which it now does.""" + from meshbay_hub.api.revocation import _authorize_node_ws + + user = await _make_user(client, "empty2") + node_id = await _announce_node(client, user) + r = await client.post( + "/v1/groups", + json={"name": "lonely", "visibility": "private", "join_policy": "invite"}, + headers={"Authorization": f"Bearer {user['token']}"}, + ) + assert r.status_code == 201, r.text + + resolved, groups = await _authorize_node_ws(_node_token(user), node_id, []) + assert resolved == node_id + assert groups == [] + + +@pytest.mark.asyncio +async def test_empty_node_cannot_shadow_another_members_group(client): + """ + The outage itself: two members, one group, and only one of them hosts it. + The other's node — running, configured with nothing — must not appear as a + source for that group, because it is the one clients would reach first. + """ + from meshbay_hub.api.revocation import ( + _authorize_node_ws, _node_groups, get_online_nodes_for_group) + + host = await _make_user(client, "hoster") + guest = await _make_user(client, "guest") + host_node = await _announce_node(client, host) + guest_node = await _announce_node(client, guest) + + r = await client.post( + "/v1/groups", + json={"name": "shared", "visibility": "private", "join_policy": "invite"}, + headers={"Authorization": f"Bearer {host['token']}"}, + ) + assert r.status_code == 201, r.text + group_id = r.json()["group_id"] + + r = await client.post( + f"/v1/groups/{group_id}/members/{'guest'}", + headers={"Authorization": f"Bearer {host['token']}"}, + ) + assert r.status_code == 201, r.text + + # The guest's node registers first — the order that made this fatal. + _, guest_groups = await _authorize_node_ws(_node_token(guest), guest_node, None) + _, host_groups = await _authorize_node_ws( + _node_token(host), host_node, [group_id]) + _node_groups[guest_node] = guest_groups + _node_groups[host_node] = host_groups + try: + assert get_online_nodes_for_group(group_id) == [host_node], ( + "a member's empty node shadowed the node actually hosting the group") + finally: + _node_groups.pop(guest_node, None) + _node_groups.pop(host_node, None) + + +@pytest.mark.asyncio +async def test_update_groups_is_held_to_the_same_ceiling(client): + """ + `update_groups` assigned the message's list verbatim, so the C2 ceiling held + at authentication could be stepped over one message later: a node had only + to reload to claim any group on the hub. It now goes through the same gate, + which is what this asserts — the socket loop itself needs a WebSocket the + ASGI harness has not got (see this module's docstring). + """ + from meshbay_hub.api.revocation import _authorized_groups, _claimable + + user = await _make_user(client, "reloader") + r = await client.post( + "/v1/groups", + json={"name": "owned", "visibility": "private", "join_policy": "invite"}, + headers={"Authorization": f"Bearer {user['token']}"}, + ) + assert r.status_code == 201, r.text + own = r.json()["group_id"] + + authorized = await _authorized_groups(user["user_id"]) + assert _claimable([own, "someone-elses-group"], authorized) == [own] + assert _claimable([], authorized) == [] + assert _claimable(None, authorized) == [] diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py index f2f4372..c506ff1 100644 --- a/packages/meshbay-hub/tests/test_transport_contracts.py +++ b/packages/meshbay-hub/tests/test_transport_contracts.py @@ -452,3 +452,57 @@ def test_the_index_is_never_reported_from_a_failed_decrypt(transport): assert "catch" not in body, ( "_applyIndexMessage swallows its own failure instead of letting " "_queueIndexMessage end the session") + + +# ── One node refusing must not take a group down (2026-09-11) ──────────────── +# +# `/v1/groups/{id}/nodes` returns every node registered for the group, in hub +# registration order. GroupPage took `nodesData.nodes[0]` and stopped there, so +# a node that could not serve the group — refusing the handshake with "Group +# not hosted on this node" — made the group unopenable while the node that +# *did* host it sat second in the same list. +# +# These strip comments first. The lesson this repeats otherwise is the CSP read +# out of the comment above the meta tag, and the packaging unit whose test +# matched the comment explaining why `User=` was absent: a source-level check +# that can match prose is not a check. + +def _code_only(src: str) -> str: + """Source with // and /* */ comments removed. Crude, and enough here: no + string literal in these files carries a comment marker.""" + src = re.sub(r"/\*.*?\*/", "", src, flags=re.S) + return re.sub(r"^\s*//.*$", "", src, flags=re.M) + + +def test_the_group_page_tries_every_node_the_hub_offers(group_page): + code = _code_only(group_page) + assert "for (const n of nodesData.nodes)" in code, ( + "the connect effect must walk the list, not index into it") + assert "nodesData.nodes[0]" not in code, ( + "taking the head and stopping is the defect — one wrongly registered " + "node captured the whole group's traffic") + + +def test_a_not_hosted_refusal_moves_on_to_the_next_node(group_page): + code = _code_only(group_page) + body = code[code.index("for (const n of nodesData.nodes)"):] + body = body[:body.index("if (!transport)")] + assert "not_hosted" in body, ( + "without the code, a refusal this browser cannot act on is " + "indistinguishable from one it must stop for") + assert "throw e" in body, ( + "a refusal naming a state of this browser — a pairing code, a " + "passphrase, a device — is the same from every node and must stop here") + + +def test_the_last_refusal_is_what_the_reader_is_told(group_page): + code = _code_only(group_page) + assert "if (!transport) throw (lastErr" in code, ( + "exhausting the list must report why, not fall through silently") + + +def test_the_refusal_the_loop_keys_on_has_a_message(transport): + """A code the page routes on, with nothing to show, is a blank error.""" + refusals = transport[transport.index("const HANDSHAKE_REFUSALS"):] + refusals = refusals[:refusals.index("};")] + assert "not_hosted:" in refusals |