diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-07 21:35:57 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-07 21:35:57 +0200 |
| commit | f7b1111c250902315e454c155c147e58d69ee9f0 (patch) | |
| tree | c2bfa30464b13b6c5da65c3d702042f2b8f0c491 | |
| parent | 3bd31db9d4fa2352f1095dcc630c77915d0774b8 (diff) | |
| download | meshbay-f7b1111c250902315e454c155c147e58d69ee9f0.tar.gz | |
fix(chat): the operator was missing from the roster they host
Found on two live machines within minutes of deploying: every message from the
person running the node arrived at every other member under "this account is
using a key you have not seen before".
An operator's authority is node-wide and is recorded in `members` with an
**empty** group_id — `is_authorized` has always said so, in a clause written for
exactly that. `group_devices` spelled the rule out a second time as
`WHERE m.group_id = ?`, which excludes them. So the operator was absent from the
roster relayed to members, no chain could reach their device key, and Tier 2
reported the most ordinary event there is — the operator talking in their own
group — as a key substitution.
A notice that fires on normal use is worse than no notice: it is the one people
learn to dismiss, and §4.8 budgets exactly one for the whole feature. That makes
this a defect in the property, not only in a query.
The clause now lives once, as `_MEMBER_OF_GROUP`, shared by both callers so they
cannot drift again. `DISTINCT` because an operator who is also an explicit member
of the group matches both halves of it. `get_member` is untouched: it is a raw
lookup and its callers already fall back to `get_member("", user_id)` themselves.
Three tests, and the first fails against the old query with the reported
symptom: the operator appears in the roster of a group they host and
`is_authorized` agrees; an operator who is also a member is listed once; a
revoked one comes back through neither.
No stored state to clean up — nothing was written to a client's pins when the
account was missing, so the notice stops as soon as the node serves the roster
correctly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TZZxYjz8YeWRz13xDi8LJr
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/roster.py | 39 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_group_roster.py | 72 |
2 files changed, 102 insertions, 9 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/roster.py b/packages/meshbay-node/src/meshbay_node/roster.py index 2288ae4..3d3a143 100644 --- a/packages/meshbay-node/src/meshbay_node/roster.py +++ b/packages/meshbay-node/src/meshbay_node/roster.py @@ -418,6 +418,16 @@ class Roster: - it is scoped to *this* group. A person in two groups on one node is not disclosed to the second by being in the first. + "Member of this group" is `_MEMBER_OF_GROUP`, shared with + `is_authorized` — **the operator belongs to every group this node + hosts**, with their authority recorded under an empty group_id. Spelling + that out a second time here is exactly what went wrong first time: + the operator was missing from the roster, so every one of their messages + reached other members as a key nobody could vouch for. + + `DISTINCT` because an operator who is *also* an explicit member of the + group matches both halves of that clause. + `add_sig`/`add_nonce`/`add_ts` are empty for a device pinned before the evidence was kept, and for the first device of any account — which an operator code admitted, not a countersignature. Both read as @@ -426,13 +436,12 @@ class Roster: """ assert self._db async with self._db.execute( - "SELECT i.user_id, i.username, i.pk_ed25519, i.pk_x25519, " - " i.added_by_pk, i.add_sig, i.add_nonce, i.add_ts, i.pinned_at " - "FROM identities i " - "JOIN members m ON m.user_id = i.user_id " - "WHERE m.group_id = ? AND m.status = 'active' " - " AND i.revoked_at IS NULL " - "ORDER BY i.user_id, i.pinned_at", (group_id,) + f"SELECT DISTINCT i.user_id, i.username, i.pk_ed25519, i.pk_x25519, " + f" i.added_by_pk, i.add_sig, i.add_nonce, i.add_ts, i.pinned_at " + f"FROM identities i " + f"JOIN members m ON m.user_id = i.user_id " + f"WHERE i.revoked_at IS NULL AND m.{self._MEMBER_OF_GROUP} " + f"ORDER BY i.user_id, i.pinned_at", (group_id,) ) as cur: rows = await cur.fetchall() return [ @@ -546,6 +555,18 @@ class Roster: async def has_operator(self) -> bool: return bool(await self.operator_pks()) + # Who counts as a member of a group, in SQL, in **one** place. + # + # The operator's authority is node-wide and is stored with an empty + # group_id, so "belongs to this group" is not `group_id = ?`. Writing that + # clause a second time is how `group_devices` came to omit the operator + # from the roster it relays — and the symptom was a member seeing "this + # account is using a key you have not seen before" on every single message + # from the person running the node. Found on a live pair of machines, not + # by a test. + _MEMBER_OF_GROUP = ("status = 'active' AND " + "(group_id = ? OR (group_id = '' AND role = 'operator'))") + async def is_authorized(self, group_id: str, user_id: str) -> bool: """ May this person be handed the group key? @@ -560,8 +581,8 @@ class Roster: """ assert self._db async with self._db.execute( - "SELECT 1 FROM members WHERE user_id = ? AND status = 'active' " - "AND (group_id = ? OR (group_id = '' AND role = 'operator')) LIMIT 1", + f"SELECT 1 FROM members WHERE user_id = ? AND {self._MEMBER_OF_GROUP} " + f"LIMIT 1", (user_id, group_id), ) as cur: return await cur.fetchone() is not None diff --git a/packages/meshbay-node/tests/test_group_roster.py b/packages/meshbay-node/tests/test_group_roster.py index d7ca7dc..cb9828c 100644 --- a/packages/meshbay-node/tests/test_group_roster.py +++ b/packages/meshbay-node/tests/test_group_roster.py @@ -240,3 +240,75 @@ async def test_a_substituted_key_carries_no_evidence(tmp_path, roster): assert devices[pk_evil]["add_sig"] == "", ( "a fabricated device cannot come with a countersignature — if this ever " "holds evidence, the node has been handed a way to mint trust") + + +# ── the operator is a member of every group this node hosts ────────────────── + +async def test_the_operator_is_in_the_roster_of_a_group_they_host(tmp_path, + roster): + """ + Found on two live machines, and it is the shape this file exists to stop. + + An operator's authority is node-wide and is stored with an **empty** + group_id (`is_authorized` says so, and has always said so). `group_devices` + wrote that rule out a second time as `WHERE m.group_id = ?`, which excludes + them — so the person running the node was absent from the roster relayed to + everyone else, their device key could be vouched for by nobody, and every + single message they sent arrived under "this account is using a key you + have not seen before". + + A notice that fires on the most ordinary event there is — the operator + talking in their own group — is worse than no notice, because it is the one + people learn to dismiss. Both queries now share `_MEMBER_OF_GROUP`. + """ + _sk_op, pk_op, px_op = _keys() + _sk_m, pk_m, px_m = _keys() + # The operator pairs node-wide: group_id is empty, exactly as + # `pairOperator` sends it and `_pin_and_admit` records it. + await roster.pin_identity("toto", "toto", pk_op, px_op, via="code") + await roster.set_member(group_id="", user_id="toto", role="operator", + status="active", approved_by="self") + # An invited member of one group this node hosts. + await roster.pin_identity("cbesson", "cbesson", pk_m, px_m, via="invite") + await roster.set_member(group_id=GROUP, user_id="cbesson", + role=ROLE_MEMBER, status="active", + approved_by="toto") + + users = {d["user_id"] for d in await roster.group_devices(GROUP)} + assert users == {"toto", "cbesson"}, ( + "the operator must appear in the roster of a group they host — " + "otherwise every message they send reads as an unknown key") + + # And the two rules genuinely agree, rather than happening to agree here. + assert await roster.is_authorized(GROUP, "toto") is True + + +async def test_an_operator_who_is_also_a_member_appears_once(tmp_path, roster): + """ + Both halves of the clause match such a person. Listed twice, a client would + see the same key arrive as two devices — harmless today, and exactly the + kind of thing that grows teeth later. + """ + _sk_op, pk_op, px_op = _keys() + await roster.pin_identity("toto", "toto", pk_op, px_op, via="code") + await roster.set_member(group_id="", user_id="toto", role="operator", + status="active", approved_by="self") + await roster.set_member(group_id=GROUP, user_id="toto", role=ROLE_MEMBER, + status="active", approved_by="self") + + devices = await roster.group_devices(GROUP) + assert [d["pk_ed25519"] for d in devices] == [pk_op] + + +async def test_an_operator_of_another_node_is_not_invented(tmp_path, roster): + """ + The clause admits an operator, not anyone with an empty group_id. A + revoked or suspended one must not come back through it. + """ + _sk_op, pk_op, px_op = _keys() + await roster.pin_identity("toto", "toto", pk_op, px_op, via="code") + await roster.set_member(group_id="", user_id="toto", role="operator", + status="revoked", approved_by="self") + + assert await roster.group_devices(GROUP) == [] + assert await roster.is_authorized(GROUP, "toto") is False |