From 1db49c37ea01db8498694613b3d0e2d54bd0d96d Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 19 Sep 2026 10:47:28 +0200 Subject: fix: the two ceilings §13.5b was still missing, as AV27 and AV28 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **A free-text TMDB search spends the operator's credential.** TMDB rates it, and the automatic matching every member sees runs on the same one, so a member holding down the search box — or a script doing it — degrades the library for everyone and empties a quota the operator pays for. The handler had no ceiling of any kind, where link previews beside it carry two. §6.5's standing rule is a bound and a named adversary in the same commit; this arrived with neither. Per member and not per connection, unlike link previews: three tabs is one person, and a ceiling a tab can multiply is not a ceiling. Kept in the group context, so a reconnect does not reset it — a client that drops its channel between searches would otherwise have no ceiling at all. The node-wide window stays too, because the two answer different questions: one keeps a member from spending everyone's quota, the other keeps a roomful of them from doing it together. Ten a minute each, thirty for the node — a search every six seconds, sustained, is past what anyone types. The refusal is an error rather than an empty list. An empty list is what "no such film" looks like, and telling somebody their film is unknown when the node simply declined to ask is a worse answer than the truth; `video-app.js` already puts `detail` on screen. **How many node keys one account may announce.** Each is a row in `nodes` plus a row in the IP log, and the IP log is kept for a year, so an account in a loop writes a year of storage on the operator's disk having paid only for signatures. M8 settled whose key it is and said nothing about how many. Ten: a node is a machine left running, and an account wanting an eleventh *identity* rather than an eleventh machine is the case this refuses. Counted only where a row is added. Applied to every announce it would freeze the address of every node an account already runs the moment it reached the limit, and a node that cannot re-announce is unreachable after its ISP renumbers it — an outage caused by the protection. There is a test for exactly that. Both tests are two accounts, per §13.5b: a ceiling one person can exhaust for another is not a ceiling but a queue, and a ceiling shared between accounts would let one member stop every other from bringing a machine online. Checked by removing each ceiling: seven tests fail. `test_season_and_search_requests.py` built its session without a `_user_id`, which production guarantees — `_dispatch_message` refuses every message until the handshake settles it. The fixture was narrower than the node, so it could not exercise a per-member bound at all; it has one now. Twelve `test_sticky_header.py[firefox]` setup errors in a full run here: Firefox is open on this machine, the trap CLAUDE.md describes, and its twelve `[chrome]` tests covering the same geometry pass. Co-Authored-By: Claude Opus 5 --- packages/meshbay-hub/src/meshbay_hub/api/nodes.py | 33 +++- .../tests/test_availability_between_members.py | 67 ++++++++ .../src/meshbay_node/transport/webrtc_server.py | 59 +++++++ .../tests/test_season_and_search_requests.py | 6 + .../meshbay-node/tests/test_tmdb_search_bound.py | 186 +++++++++++++++++++++ 5 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 packages/meshbay-node/tests/test_tmdb_search_bound.py (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/api/nodes.py b/packages/meshbay-hub/src/meshbay_hub/api/nodes.py index 67e65f2..83b60f2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/nodes.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/nodes.py @@ -8,7 +8,7 @@ from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey from cryptography.exceptions import InvalidSignature from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.auth import issue_access_token @@ -89,6 +89,22 @@ class NodeAnnounceRequest(BaseModel): signature: str | None = None # base64 Ed25519 over the announce message +# How many distinct node keys one account may announce. +# +# M8 closed the half of this that was about *whose* key it is: the announcer now +# proves possession. What it did not close is *how many*. Each new key is a row +# in `nodes` plus a row in the IP log, and the IP log is kept for a year — so an +# account in a loop writes a year of storage on somebody else's disk, having paid +# only for the signatures. +# +# Ten is past what the feature is for. A node is a machine left running: a +# desktop, a laptop, a box in a cupboard, a second home. Someone who genuinely +# reaches it deletes one, which is a thing the operator surface already does — +# and an account that wants an eleventh *identity* rather than an eleventh +# machine is the case this refuses. +MAX_NODES_PER_ACCOUNT = 10 + + @router.post("/announce", status_code=201) async def announce_node( body: NodeAnnounceRequest, @@ -146,6 +162,21 @@ async def announce_node( await db.commit() return {"node_id": node.id} + # Counted only where a row is actually added: re-announcing a key this + # account already holds takes the branch above and must keep working at the + # ceiling, or a node that has reached it can never refresh its address again. + held = (await db.execute( + select(func.count()).select_from(Node) + .where(Node.user_id == current_user.id))).scalar() or 0 + if held >= MAX_NODES_PER_ACCOUNT: + db.add(IPLog(user_id=current_user.id, event="node_announce_refused", + ip_address=seen_from, detail=f"{held} nodes")) + await db.commit() + raise HTTPException( + status_code=409, + detail=f"This account already has {held} nodes, which is the limit of " + f"{MAX_NODES_PER_ACCOUNT}. Remove one you no longer run.") + node = Node( user_id=current_user.id, pk_node=body.pk_node, diff --git a/packages/meshbay-hub/tests/test_availability_between_members.py b/packages/meshbay-hub/tests/test_availability_between_members.py index d1a4dcb..2be7c03 100644 --- a/packages/meshbay-hub/tests/test_availability_between_members.py +++ b/packages/meshbay-hub/tests/test_availability_between_members.py @@ -718,3 +718,70 @@ async def test_a_private_groups_node_list_is_for_its_members(client): finally: rev._connected_nodes.pop(node_id, None) rev._node_groups.pop(node_id, None) + + +async def _announce_key(client, user: dict, sk) -> int: + """Announce a *distinct* node key, and return the status code.""" + from meshbay_common.crypto import pk_to_b64 + + pk = pk_to_b64(sk.public_key()) + ts = int(time.time()) + msg = f"meshbay:node_announce:{user['user_id']}:{pk}:{ts}".encode() + r = await client.post("/v1/nodes/announce", json={ + "pk_node": pk, "endpoint_hint": "test", "timestamp": ts, + "signature": base64.b64encode(sk.sign(msg)).decode(), + }, headers={"Authorization": f"Bearer {user['token']}"}) + return r.status_code + + +async def test_one_account_cannot_announce_unlimited_nodes(client, monkeypatch): + """ + Each new node key is a row in `nodes` and a row in the IP log, and the IP log + is kept for a year. Proof of possession (M8) settles *whose* key it is and + says nothing about how many: an account in a loop wrote a year of storage on + the operator's disk having paid only for signatures. + + Two accounts, because the ceiling has to be per account. One that is shared + would let a single member deny every other member the ability to bring a + machine online, which is the same defect with better manners. + """ + from meshbay_hub.api import nodes as nodes_api + + monkeypatch.setattr(nodes_api, "MAX_NODES_PER_ACCOUNT", 3) + alice = await _make_user(client, "av_nodecap_alice") + bob = await _make_user(client, "av_nodecap_bob") + + keys = [Ed25519PrivateKey.generate() for _ in range(4)] + for sk in keys[:3]: + assert await _announce_key(client, alice, sk) == 201 + + assert await _announce_key(client, alice, keys[3]) == 409, ( + "an account announced past the ceiling") + + # Bob has announced nothing and must be unaffected. + assert await _announce_key(client, bob, Ed25519PrivateKey.generate()) == 201, ( + "one account's ceiling was charged to another's" + ) + + +async def test_a_node_at_the_ceiling_can_still_refresh_its_address(client, monkeypatch): + """ + The ceiling counts rows, so it must be checked only where a row is added. + Applied to every announce, it would freeze the address of every node an + account already runs the moment it reached the limit — and a node that + cannot re-announce is a node nobody can reach after their ISP renumbers + them, which is an outage caused by the protection. + """ + from meshbay_hub.api import nodes as nodes_api + + monkeypatch.setattr(nodes_api, "MAX_NODES_PER_ACCOUNT", 2) + alice = await _make_user(client, "av_nodecap_refresh") + + keys = [Ed25519PrivateKey.generate() for _ in range(2)] + for sk in keys: + assert await _announce_key(client, alice, sk) == 201 + assert await _announce_key(client, alice, Ed25519PrivateKey.generate()) == 409 + + for sk in keys: + assert await _announce_key(client, alice, sk) == 201, ( + "a node already known could not re-announce at the ceiling") diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index 4540f2f..773d3dc 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -213,6 +213,23 @@ _LINK_PREVIEW_RATE_WINDOW = 60.0 _LINK_PREVIEW_RATE_PER_CONN = 15 _LINK_PREVIEW_RATE_NODE = 60 +# A free-text TMDB search spends the *operator's* credential, which is rated by +# TMDB and shared by everyone in the group: one member typing in the search box +# can exhaust what every other member's automatic matching depends on, and the +# operator is the one who has to notice. §6.5's rule is a bound and a named +# adversary in the same commit; this one arrived without either. +# +# Per member rather than per connection, unlike link previews above: three tabs +# is one person, and a ceiling a tab can multiply is not a ceiling. Kept in the +# group context so it survives a reconnect, which is the other thing a per-session +# count cannot do. +# +# Generous next to what a person types — ten searches a minute is a search every +# six seconds, sustained — and small next to a loop. +_TMDB_SEARCH_WINDOW = 60.0 +_TMDB_SEARCH_PER_MEMBER = 10 +_TMDB_SEARCH_NODE = 30 + # Chat limits. A message is a member-supplied write onto the operator's disk # (`chat.db`, where retention is a manual CLI command — §6.6), relayed from there # to every other connected member and turned into a notification for every member @@ -4471,6 +4488,21 @@ class WebRTCPeerSession: "query": query, "media_type": media_type, "results": []}) return + # Refused out loud, not as an empty result: "no matches" is what the + # client draws for an empty list, and telling somebody their film is + # unknown when the node simply declined to ask is a worse answer than + # the truth. `video-app.js`'s `runSearch` puts `detail` on screen. + if not self._tmdb_search_rate_ok(): + log.info("tmdb_search_req: rate-limited (user=%s)", (self._user_id or "")[:8]) + self._send({ + "type": "error", + "detail": "Too many searches in the last minute. This spends the " + "operator's search quota, which everyone in the group " + "shares — try again shortly.", + "code": "tmdb_search_rate_limited", + }) + return + raw = (await tmdb_client.search_movie_results(query) if media_type == "movie" else await tmdb_client.search_tv_results(query)) results = [] @@ -5103,6 +5135,33 @@ class WebRTCPeerSession: if isinstance(m.payload, bytes) else m.payload) return row + def _tmdb_search_rate_ok(self) -> bool: + """ + True when this search is within both the member's window and the node's; + records it when so, and trims both to the window on every call so neither + list can grow without bound. + + Both are checked because they answer different questions: the member's + keeps one person from spending everyone's quota, and the node's keeps a + group of them from doing it together. + """ + now = time.monotonic() + w = _TMDB_SEARCH_WINDOW + ctx = self._group_ctx() + by_member = ctx.setdefault("tmdb_search_hits", {}) + who = self._user_id or "" + mine = [t for t in by_member.get(who, []) if now - t < w] + node = [t for t in self._ctx.get("tmdb_search_hits_node", []) if now - t < w] + if len(mine) >= _TMDB_SEARCH_PER_MEMBER or len(node) >= _TMDB_SEARCH_NODE: + by_member[who] = mine + self._ctx["tmdb_search_hits_node"] = node + return False + mine.append(now) + node.append(now) + by_member[who] = mine + self._ctx["tmdb_search_hits_node"] = node + return True + def _link_preview_rate_ok(self) -> bool: """ True when this preview fetch is within both the per-connection and the diff --git a/packages/meshbay-node/tests/test_season_and_search_requests.py b/packages/meshbay-node/tests/test_season_and_search_requests.py index 9dcc6ce..4141d13 100644 --- a/packages/meshbay-node/tests/test_season_and_search_requests.py +++ b/packages/meshbay-node/tests/test_season_and_search_requests.py @@ -22,6 +22,12 @@ def _session(media_cache=None, tmdb_client=None) -> WebRTCPeerSession: session = WebRTCPeerSession.__new__(WebRTCPeerSession) session._ctx = {"media_cache": media_cache, "tmdb_client": tmdb_client} session._group_id = None + # Set because production always has one: `_dispatch_message` refuses every + # message until the handshake settles `_user_id`, so a session reaching any + # of these handlers without it does not exist. Left out, this fixture was + # narrower than the node and the per-member search ceiling could not be + # exercised by it at all. + session._user_id = "u1" session.sent = [] session._send = session.sent.append return session diff --git a/packages/meshbay-node/tests/test_tmdb_search_bound.py b/packages/meshbay-node/tests/test_tmdb_search_bound.py new file mode 100644 index 0000000..486a2c2 --- /dev/null +++ b/packages/meshbay-node/tests/test_tmdb_search_bound.py @@ -0,0 +1,186 @@ +""" +One member's typing must not spend what the whole group depends on. + +`tmdb_search_req` takes a member's free text and calls TMDB with the +**operator's** credential. That credential is rated by TMDB and shared: the +automatic matching every other member sees runs on it too. So a member holding +down a search box — or a script doing it — degrades the library for everyone and +costs the operator their quota, and the node had no ceiling of any kind on it. +§6.5's standing rule is a bound and a named adversary in the same commit; this +handler shipped with neither. + +Two members in every test here, which is the point: a ceiling that one person +can exhaust for another is not a ceiling, it is a queue. The per-member window +is what keeps them apart, and the node-wide one is what keeps them together +from emptying the operator's quota — they answer different questions and both +are checked. + +The refusal is an error rather than an empty result. An empty list is what "no +such film" looks like, and telling somebody their film is unknown when the node +simply declined to ask is a worse answer than the truth. +""" + +import pytest +from meshbay_node.transport import webrtc_server +from meshbay_node.transport.webrtc_server import WebRTCPeerSession + +GROUP = "g" * 32 + + +class _FakeTmdb: + """Counts what would have been spent.""" + + def __init__(self): + self.calls = 0 + + async def search_movie_results(self, query): + self.calls += 1 + return [{"id": 1, "title": "Some Saga", "release_date": "1999-01-01", + "poster_path": None}] + + async def search_tv_results(self, query): + self.calls += 1 + return [] + + +class _FakeMediaCache: + async def get_thumb_hash_by_file_id(self, _file_id): + return None + + +@pytest.fixture +def group(): + """One group's context, shared by every session in it, as a node has.""" + return { + "gek": b"k" * 32, + "tmdb_enabled": True, + } + + +@pytest.fixture +def node(group): + tmdb = _FakeTmdb() + ctx = { + "groups": {GROUP: group}, + "media_cache": _FakeMediaCache(), + "tmdb_client": tmdb, + } + return ctx, tmdb + + +def _member(ctx, user_id: str) -> WebRTCPeerSession: + s = WebRTCPeerSession.__new__(WebRTCPeerSession) + s._ctx = ctx + s._group_id = GROUP + s._user_id = user_id + s._peer_id = user_id + s.sent = [] + s._send = s.sent.append + s._audit = lambda *a, **k: None + return s + + +async def _search(session, query="a film"): + await session._do_tmdb_search_request( + {"query": query, "media_type": "movie"}) + + +def _refusals(session): + return [m for m in session.sent + if m.get("code") == "tmdb_search_rate_limited"] + + +async def test_a_member_at_the_ceiling_does_not_stop_another_one(node, monkeypatch): + """ + The property a one-member test cannot state. + + Alice exhausts her own window; Bob, who has typed nothing, must be served + exactly as if she had not been there. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 3) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 100) + ctx, tmdb = node + + alice = _member(ctx, "alice") + for i in range(4): + await _search(alice, f"film {i}") + assert tmdb.calls == 3, "the ceiling did not stop the fourth search" + assert len(_refusals(alice)) == 1 + + bob = _member(ctx, "bob") + await _search(bob, "something else") + assert tmdb.calls == 4 + assert _refusals(bob) == [] + + +async def test_one_member_cannot_spend_the_whole_node_quota(node, monkeypatch): + """ + And the other half: two members together still meet a node-wide ceiling, + because the operator's credential is one credential however many people + hold the search box down. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 100) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 2) + ctx, tmdb = node + + alice, bob = _member(ctx, "alice"), _member(ctx, "bob") + await _search(alice) + await _search(bob) + await _search(bob) + + assert tmdb.calls == 2 + assert len(_refusals(bob)) == 1 + + +async def test_a_members_count_survives_their_reconnection(node, monkeypatch): + """ + Kept in the group context, not on the session: otherwise the ceiling is one + reconnect wide, and a client that drops its DataChannel between searches has + no ceiling at all. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 2) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 100) + ctx, tmdb = node + + first = _member(ctx, "alice") + await _search(first, "one") + await _search(first, "two") + + reconnected = _member(ctx, "alice") # same person, new connection + await _search(reconnected, "three") + + assert tmdb.calls == 2, "a reconnect reset the member's window" + assert len(_refusals(reconnected)) == 1 + + +async def test_a_refusal_is_said_out_loud_and_not_drawn_as_no_matches(node, monkeypatch): + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 0) + ctx, _ = node + + alice = _member(ctx, "alice") + await _search(alice) + + (msg,) = alice.sent + assert msg["type"] == "error" + assert msg["code"] == "tmdb_search_rate_limited" + assert msg.get("results") is None, ( + "a refusal that carries an empty result list reads as 'no such film'") + + +async def test_the_windows_do_not_grow_without_bound(node, monkeypatch): + """ + The lists are trimmed on every call, so the thing that bounds a member also + bounds what remembering them costs. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_WINDOW", 0.0) + ctx, tmdb = node + + alice = _member(ctx, "alice") + for i in range(12): + await _search(alice, f"film {i}") + + # Every entry ages out before the next call, so nothing is refused, and what + # is kept is the one just recorded rather than one per search ever made. + assert tmdb.calls == 12 + assert len(ctx["groups"][GROUP]["tmdb_search_hits"]["alice"]) == 1 + assert len(ctx["tmdb_search_hits_node"]) == 1 -- cgit v1.2.3