From f059cb118c556d1f0279350507f74b8a47d5a98a Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Fri, 11 Sep 2026 00:19:06 +0200 Subject: docs: remove the documents MESHBAY_DESIGN.md replaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twenty-four files, about 17 000 lines: the two architecture drafts, the three security reviews, eleven design notes, the roadmap, the decisions file, the v1–v4 archive, the deprecated user guide and the stale quickstart. Their content is in MESHBAY_DESIGN.md, and git history holds the originals. The reason to delete rather than keep bannered: a document that is superseded but present still gets read, and a reader cannot always tell which of two accounts of one mechanism is the live one. That was the argument for retiring the user guide rather than repairing it, and it applies to the whole set. What made this safe is the concordance. Roughly 290 comments and docstrings cite these files by section — `musicbay.md §6`, `mediacenter.md §5.5`, `draft-v6 §2.11` — and section 16 maps every one onto its replacement, so not a single comment needs editing to stay followable. It now says plainly that the files are gone and where to recover them, and it gained rows for the three reviews (their findings are section 13), and for the two guides. Four kept documents pointed into the set and were repointed first: `playlists.md` (nine references — it is a live proposal and must not dangle), `WINDOWS-PORT.md`, and CLAUDE.md's example. No dangling reference remains outside section 16. Two files were dropped from the list after checking what they hold. `HTTPS.md` is an operational runbook — Caddy, certificate renewal, DNS, troubleshooting — and MESHBAY_DESIGN.md deliberately covers no operations, so nothing would replace it; the versioned Caddyfile is the config, not the procedure. `cast-smart-tv.md` is the plan for the unbuilt DLNA phase of a feature whose first two phases ship, and section 11.4 summarises it in four lines rather than carrying the SSDP/UPnP work. There is no user guide now, and section 0.1 says so rather than leaving a reader to discover it. Suites green: 2258 passed, 4 skipped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YVoHVCcfBqud6ZjG4db3y7 --- docs/third-review.md | 723 --------------------------------------------------- 1 file changed, 723 deletions(-) delete mode 100644 docs/third-review.md (limited to 'docs/third-review.md') diff --git a/docs/third-review.md b/docs/third-review.md deleted file mode 100644 index 62ab5f6..0000000 --- a/docs/third-review.md +++ /dev/null @@ -1,723 +0,0 @@ -# MeshBay — Third Architecture & Security Review - -> **Superseded by `MESHBAY_DESIGN.md`.** This was the third security review; its design -> content now lives in §13.4, and the invariants each finding names throughout. -> -> It is kept because code comments, tests and other documents cite its -> sections and its labels, and because it records reasoning a synthesis -> compresses. **Where it disagrees with `MESHBAY_DESIGN.md`, the design -> document is right; where either disagrees with the code, the code is.** -> `MESHBAY_DESIGN.md` §16 maps every section reference here onto its -> replacement, and §13 defines every label. - -> Date: 2026-09-01 -> Scope: the code as it stands on `main` at `8a6294b`, with emphasis on what -> changed since `second-review.md` (2026-08-13): the unified handshake -> (`meshbay_common/handshake.py`), device linking, the invite/pairing rewrite, -> account recovery and passphrase change (`docs/auth-confirm.md`), email -> verification, reCAPTCHA, the hub instance-policy store, MHP federation, -> the community relay registry, chat link previews, the TMDB/MusicBrainz -> enrichment path, and the node's token-gated loopback control API. -> -> Method: code reading of `packages/`. The test suite was not run and no live -> testing was done against meshbay.org. This is a code and design review, not a -> penetration test. Finding numbers are independent of the first two reviews. -> -> The v5/v6 convention is kept: **a claim names the adversary it holds against.** -> The adversaries referenced below are the ones the project already uses — passive -> hub, active hub, malicious node operator, malicious group member, network -> attacker, local attacker — plus two the newer features introduce: **any -> registered hub user with no group membership**, and **a federated peer hub**. - ---- - -## 1. Executive summary - -**The critical and high findings from the second review have genuinely been -closed, and closed well.** The unified handshake is the right shape: one -length-prefixed, domain-separated, role-bound transcript; mandatory channel -binding; a mutual proof where the node demonstrates GEK possession over the -client's nonce *and* signs the transcript with its long-term key; `scope="user"` -enforced by default; `group_id` mandatory. It is now run by **both** the WebRTC -and the QUIC transports — the C6 divergence that produced most of the second -review is structurally gone. C1 (the unauthenticated node HTTP file API) was deleted outright -rather than patched. C2, C3, C5a, C5b, H1, H4, H5, H6, H7, M7, M8 are all -addressed in the code, and the invite rewrite closed H3/M3. Device linking, -the password split, Argon2id-256 MB on the hub verifier, refresh-token family -rotation, email-at-rest encryption, and session/device teardown on passphrase -change are all present and correct. - -**What this review finds is a second generation of the same pattern:** new -surface was added faster than the authorization model was extended to cover it, -and a few of the second-review fixes did not reach every path. - -- The **QUIC transport** got the new handshake but not the new *chat* rules: - `_do_chat_message_sync` took `sender_id` from the wire (NS6), broadcast - through a connection-global peer registry regardless of group (H1), and ran a - 30-second synchronous `ffmpeg` on the event loop with no concurrency cap (H6). - **Fixed 2026-09-01** — handlers brought to WebRTC parity, and the QUIC - listener is now off by default (`[node] quic_enabled`) since nothing ships a - QUIC client. -- The **hub moderation surface** had a privilege-escalation hole: a *moderator* - could promote any other account to *admin* (`PATCH /v1/admin/users/{id}` was - gated by `require_moderator` but wrote `role`). **Fixed 2026-09-01.** -- **`POST /v1/reports`** was unauthenticated, unthrottled, and auto-blocked a - content hash after **two** reports — a network-wide censorship/DoS primitive - for anyone who learns a public file's blake3 id. **Fixed 2026-09-01** (auth, - rate limit, distinct-reporter counting, refused when public groups are off). -- The **registration reCAPTCHA** was inert: the server only checked it when - `auth_key` was absent, and the real web client always sends `auth_key`, so a - bot skipped it by including that field. **Fixed 2026-09-01** — gate is now - unconditional when a captcha is configured; the desktop client renders the - widget too. -- **Chat link previews** are a real SSRF surface (correctly identified as such in - the module) but the gate had gaps: no per-member rate limit, no port - restriction, and DNS rebinding a documented residual. **Fixed 2026-09-01** - (rate limit, port allowlist, connect-address re-check, bomb guard). -- **MHP federation** trusted any registered peer hub to push directory rows and - revocations, never checked the token audience, and the revocation-propagation - path was a silent no-op. **Fixed 2026-09-01** (source bound to the signer, - push capped, revocation acts on the peer's own directory entries, replay - rejected). -- There was **no CSP or security-header policy** on the hub-served SPA (second - review L5). **Fixed 2026-09-01** — a middleware applies the same policy the - desktop client already enforces on these files. Wants a pass against the - running SPA. - -None of this breaks the architecture. The cryptographic core and the trust model -are unchanged and still sound. H1, H2 and M1–M5 were -fixed on 2026-09-01; what is left is the L-list — opportunistic hardening, not a -hole — plus verifying the SPA CSP (M5) against the live app. - ---- - -## 2. What is solid (the delta since the second review) - -Worth recording, because the remediation was substantial and mostly correct: - -1. **`meshbay_common/handshake.py`** — one implementation, called by - `webrtc_server.py` and `quic_server.py`. `handshake_transcript()` is - length-prefixed and domain-separated (`meshbay:mnp:handshake:v1`), the role is - bound so a client proof can never be replayed as a node proof, and - `make_proof()` **raises** on an empty channel binding instead of degrading to - nonce-only (L4). `authorize_token()` enforces `scope == "user"` by default - (M9), requires `group_id` (M1), checks the denylist, the `groups` claim and - `hosted_groups`. -2. **Mutual authentication (C3).** `_complete_handshake` returns - `HMAC(GEK, node-transcript)` over the client's nonce **and** - `Ed25519(sk_node)` over the same transcript; `transport.js` verifies both - (`verifyNodeSignature`), refuses a bare `handshake_ack`, and TOFU-pins - `node_pk` in `localStorage` with an explicit change warning - (`_checkNodePin`). -3. **C1 deleted.** The per-group HTTP file API is gone from `daemon.py` - (step 9 is now a comment explaining why). Every client path goes through the - MNP handshake. -4. **C2 closed.** `_authorize_node_ws` resolves the node against the DB, checks - `scope == "node"`, checks `node.user_id == token.sub`, derives `group_ids` - from `GroupMember`, and refuses to displace a live registration. -5. **C5a closed.** Uploads: `SAFE_UPLOAD_NAME` allowlist, `_free_name()` - no-overwrite, `MAX_UPLOAD_BYTES` cap, strict chunk ordering, a quarantine - subdirectory, and an operator-signed `OP_MEMBER_UPLOAD` kill switch enforced - by the node (`_do_file_upload`), not by hiding a button. -6. **C5b closed.** `gek_bundle_store` is deleted; `gek_rotate` is an - operator-signed op where the node generates the key with its own CSPRNG - (`_admin_exec_gek_rotate` → `ops.set_gek(rotate=True)`). -7. **H1 (WebRTC) closed.** `_do_chat_message` / `_do_chat_history` read - `self._group_ctx().get("chat_store")`, `_peer_registry()` is per-group, and - `sender_id` is forced to `self._user_id`. -8. **H4 closed.** `Denylist` persists to `denylist.json`; `on_revocation` - handles `user`/`group`/`jti`, and `group` also drops live sessions - (`_drop_group_sessions`); `webrtc_offer` refuses when the shared group is not - `active`. -9. **H5 closed.** `adminop.admin_transcript()` — domain-separated, names the - operation, subject, node key, group, nonce and timestamp; `ADMIN_CHALLENGE_TTL` - 120 s; verified against `roster.operator_pks()` rebuilt from node state, never - from the response. -10. **H6 (WebRTC) closed.** 64 KB pre-handshake buffer, a transcode semaphore, - per-user pending-offer caps and a membership check in `signaling.py`, - `notify_incoming` requires `peer_ip == caller_ip`. -11. **H7 closed.** The swarm route is mounted correctly, nodes filter by - visibility, and `GET /v1/swarm/{hash}` requires auth. -12. **M8 closed.** `announce_node` requires a signed proof of possession. -13. **Device linking** (`_do_device_add`, `_verify_device_signer`): a new device - is admitted only by a signature from a **live pinned device of the same - account**; the one-time code never reaches the node (it lists candidate - hashes and the approver recomputes the match); requests are single-use and - capped by `MAX_DEVICES_PER_USER`. The hub holds no user keys and so cannot - countersign — this holds against an active hub. -14. **Account lifecycle** (`docs/auth-confirm.md`): passphrase change and reset - both revoke every refresh token; reset also deletes every `UserDevice` so a - stored device key cannot sign back in past the reset. The recovery key is a - pure client-side pass-through — never stored, never logged. - ---- - -## 3. High findings - -### H1 — A moderator can promote any account to admin (privilege escalation) - -> **Fixed 2026-09-01.** `admin_patch_user` now splits authorization by field: -> `status` between `active`/`suspended` stays at `require_moderator`; setting -> `role`, setting `status = "revoked"`, and touching an admin's account at all -> require `user_is_admin(current_user)` (new helper in `deps.py`). Regression -> test: `test_moderator_cannot_change_roles_or_revoke`. - -**Location:** `api/admin.py:185-241` (`admin_patch_user`), `api/deps.py:75-92` - -`PATCH /v1/admin/users/{user_id}` depends on `require_moderator`, but its body -accepts `role`, and the handler writes it with no check that the caller is an -admin: - -```python -if body.role is not None: - if body.role not in ("user", "moderator", "admin"): - raise HTTPException(status_code=422, ...) - user.role = body.role # ← moderator can set "admin" -``` - -The only guard is `user.id == current_user.id` ("Cannot modify your own -account"). So a moderator cannot self-promote directly, but can: - -- promote a second account they control, or an accomplice, to `admin`; -- **demote existing admins** to `user`, or set their `status` to `revoked`. - -`admin` is the real instance boundary: `admin_patch_settings` (public-groups -switch), `admin_delete_user` (irreversible erasure), `admin_revoke` -(user/group revocation broadcast to every node), `register_peer`, -`admin_add_blocklist`. A moderator reaching `admin` reaches all of it. - -**Impact.** Full instance takeover from the moderator role. Moderator is meant to -be a content-moderation role (suspend/revoke groups, read logs), not an -administrative one — `admin_delete_user`'s own docstring draws exactly that line -("Admin rather than moderator: suspension is reversible … this is not"). - -**Fix.** Split the handler: `status` changes among `active`/`suspended` stay at -`require_moderator`; `role` changes and `status = "revoked"` require -`require_admin`. Also forbid granting a role higher than the caller's, and forbid -demoting an equal-or-higher role. - ---- - -### H2 — Unauthenticated, unthrottled, permanent global content blocklisting - -> **Fixed 2026-09-01.** `POST /v1/reports` now requires a signed-in account -> (`get_current_user`), is rate-limited (`10/hour`), counts **distinct reporting -> accounts** (one vote per account per hash via `reporter_id`), and is refused -> outright (`403`) when the hub has public groups switched off — a private-only -> hub brokers no public content and nothing syncs the blocklist, so an open write -> endpoint there is pure abuse surface. `AUTO_BLOCK_THRESHOLD` raised 2 → 3. -> Tests rewritten in `test_moderation.py`. -> -> Note also confirmed while fixing: **no node currently consumes -> `ContentBlocklist`** — `GET /v1/blocklist` exists ("nodes sync on startup") but -> nothing fetches it, and `swarm_register` checks the *CSAM* list, not this one. -> So the network-wide censorship effect was latent (it activates when node sync -> ships); the DB-fill / poisoned-moderation-signal / admin-panel-garbage surface -> was live. The auto-block path should stay gated as above when sync lands. - -**Location:** `api/moderation.py:39,58-103` (`report_content`) - -`POST /v1/reports` has **no authentication and no rate limit**. It counts *all* -existing `ContentReport` rows for a hash — regardless of who filed them or from -where — and: - -```python -AUTO_BLOCK_THRESHOLD = 2 -... -if count + 1 >= AUTO_BLOCK_THRESHOLD: - ... db.add(ContentBlocklist(content_hash=..., added_by="auto")) -``` - -So **two unauthenticated HTTP requests** naming the same 64-hex blake3 id add -that id to `ContentBlocklist`. Nodes sync the blocklist -(`GET /v1/blocklist`, unauthenticated) and `swarm_register` refuses a blocked -hash with HTTP 451. Removal is a manual admin action -(`DELETE /v1/admin/blocklist/{hash}`). - -**Impact.** Anyone who learns the blake3 id of a public file — trivially, any -group member sees ids in the index; any registered user can probe -`GET /v1/swarm/{hash}` — can suppress that file across the whole network with two -anonymous requests. It is also a self-inflicted amplifier: one script can block -thousands of hashes. `content_hash` is the only validated field (`group_id`, -`reason`, `detail` are free-form and rendered in the admin UI). - -**Fix.** Require authentication on `POST /v1/reports`; dedupe reports by -`(content_hash, reporter)` so the threshold means *distinct* reporters; add a -rate limit; raise `AUTO_BLOCK_THRESHOLD` and/or make auto-block queue for human -review rather than take effect immediately; authenticate `GET /v1/blocklist` and -`/v1/blocklist/check` (node scope). - ---- - -## 4. Medium findings - -### M1 — The registration CAPTCHA is inert and trivially bypassed - -> **Fixed 2026-09-01 (Option A).** The server gate is now `if -> _cfg.captcha.enabled:` — no `auth_key` carve-out, no client exemption. The web -> client (`registerUser` in `keyderive.js`) forwards `captcha.token`, and the -> desktop client, being Chromium, renders the same widget from the shared UI -> assets. `captcha.reset()` is called on a failed attempt so the single-use -> token is refreshed. Tests: `test_register_captcha.py`. -> -> Consequence to check on the desktop side: the Electron CSP must allow -> `https://www.google.com` and `https://www.gstatic.com` for `script-src` / -> `frame-src`, or the widget will not render and the (already-disabled) submit -> button stays disabled. A headless/CLI `register` has no widget and is the one -> path with no human check — which is the path you would want gated anyway; a CLI -> can open a browser window for it. - -**Location:** `api/users.py:130-191` (`register`), `static/keyderive.js:279-303` -(`registerUser`), `static/auth-page.js:229-256` - -Server side: - -```python -# Captcha gate — web path only (native clients send auth_key) -if _cfg and _cfg.captcha.enabled and not body.auth_key: - await _verify_captcha_or_raise(body.captcha_token, request) -``` - -The CAPTCHA is checked **only when `auth_key` is absent**. But the real web -client's registration path (`window.MeshBayKeys` present, which is always) -calls `registerUser()`, which sends `{ username, email, auth_key }` and **no -`captcha_token`** at all. The branch that sends `captcha_token` -(`auth-page.js:248`) is a dead `else` for a client without `MeshBayKeys`. - -So: a human filling the Register form solves a reCAPTCHA whose token is never -transmitted and never checked, and a bot registers accounts at will by including -any `auth_key`-shaped string. `@limiter.limit("5/minute")` is the only remaining -brake (and see L10 for why that may also be weak). - -Password reset is unaffected — `password_reset_request` checks the CAPTCHA -unconditionally when enabled. - -**Fix.** Gate on `_cfg.captcha.enabled` alone (drop `and not body.auth_key`), and -have `registerUser()` include `captcha_token`. If native clients genuinely cannot -present one, gate on the *client type* explicitly (a header or a scope), not on -the presence of a field any caller can supply. - ---- - -### M2 — QUIC transport: chat sender spoofing, cross-group broadcast, and a blocking ffmpeg - -> **Fixed 2026-09-01, two ways.** -> 1. **Exposure removed:** the QUIC listener is now off by default — -> `[node] quic_enabled = false` (`MESHBAY_QUIC_ENABLED` overrides), gated in -> `daemon.py`. Nothing ships a QUIC client, so a node started none for no one. -> 2. **Handlers brought to parity** anyway, for when a client does exist: -> `_do_chat_message_sync` forces `sender_id` from the authenticated session -> (M2a), resolves `chat_store` and the peer set per group via `_group_ctx()` / -> `_peer_registry()` (M2b, kept separate from the WebRTC peer set), and -> `STREAM_SEGMENT` extraction runs in a thread behind a small semaphore as a -> tracked task (M2c). Stale C6 docstring corrected. -> -> Original finding text kept below for the record. - -**Location:** `transport/quic_server.py:220-247, 402-463, 509-525` - -The QUIC server is started in production (`daemon.py:540`, `host="::"`, default -port 19000, `groups=groups_ctx`). It got the new unified handshake — and the -GEK proof *is* implemented in `_do_handshake_response_sync`, so the docstring at -`quic_server.py:259-263` ("NOT YET DONE — finding C6 remains open on this -transport") is simply stale. But the chat and streaming handlers were never -brought up to the WebRTC path's rules: - -**M2a — `sender_id` is taken from the wire (NS6 regression).** - -```python -asyncio.ensure_future(chat_store.save_message( - sender_id=msg.get("sender_id", self._user_id), ...)) -... -broadcast = { ... "sender_id": msg.get("sender_id", self._user_id), ... } -``` - -An authenticated QUIC peer can post chat as any `sender_id`. The WebRTC path -forces `sender_id=self._user_id` (`webrtc_server.py:3493,3504`). - -**M2b — the peer registry is connection-global, not per-group (H1 regression).** -`_do_chat_message_sync` broadcasts to `self._ctx.get("_peers", {})`, which is a -single dict on the `QuicChunkServer` instance shared across every group. A member -of group A, connected over QUIC, has their (spoofable) message fanned out to -QUIC peers of every other group on the node. (`chat_store` is never set in the -QUIC ctx, so messages are dropped rather than persisted — but still broadcast.) - -**M2c — synchronous ffmpeg on the event loop, no concurrency cap (H6 -regression).** `_do_stream_segment_sync` → `_extract_segment` runs -`subprocess.run([... "ffmpeg" ...], timeout=30)` directly inside -`quic_event_received`. One request blocks the whole node for up to 30 s; there is -no transcode semaphore. `_do_file_request_sync` likewise does blocking file I/O -in the loop. - -**Impact.** Limited to peers reachable over QUIC/UDP 19000 — LAN, a -port-forwarded node, or hub-less `group://` — and to native clients (browsers use -WebRTC, a separate registry). Still: chat impersonation and cross-group leakage -against exactly the "malicious group member" adversary the WebRTC fixes were -written for, plus a one-request node stall. - -**Fix.** Route the QUIC chat and stream handlers through the same per-group -context and `sender_id`-from-session logic as WebRTC (ideally shared helpers in -`meshbay_common`, the same move that fixed C6); make `_extract_segment` async and -put it behind the transcode semaphore, or disable the QUIC `STREAM_SEGMENT` -handler until it is at parity. Update the stale docstring. - ---- - -### M3 — Chat link previews: SSRF gate has no rate limit, no port restriction, and a known rebinding hole - -> **Fixed 2026-09-01.** -> - **Rate limit:** `_do_link_preview_request` is now bounded per connection (15) -> and node-wide (60) over a 60 s window; a cache hit is free, and over the -> ceiling the reply is a plain `ok: false` (bare link), not cached. -> - **Port allowlist:** `safe_url()` restricts the port to `{80, 443, 8080, -> 8443}` — every real OpenGraph page, none of the SSH/mail/DB/cache/search -> ports. Deliberately not just 80/443, to keep legitimate sites on alt-HTTP -> working. -> - **DNS rebinding:** the connection's actual peer address is re-checked -> against the public-address rule (`_reject_if_rebound`) before the body is -> read. Best-effort — a full literal-pin with cert-for-name is noted as -> remaining hardening. -> - **Decompression bomb:** `_downscale` refuses an image whose header -> dimensions exceed ~40 MP before decode. -> -> Not extended to `MEDIA_META_REQ` / `TMDB_SEARCH_REQ`: those reach a fixed -> host, so they carry a quota concern but not an SSRF one — left for a separate -> pass. - -**Location:** `node/linkpreview.py`, `webrtc_server.py:3588-3636` -(`_do_link_preview_request`) - -The design is right — the *node* fetches, not the browser or the hub — and -`safe_url()` blocks non-http(s) schemes, embedded credentials, and any resolved -address that is not globally routable, re-checking every redirect hop by hand. -But: - -1. **No rate limit / no per-member cap.** `_do_link_preview_request` is reachable - by any group member after the handshake, and the in-memory cache - (`_LINK_PREVIEW_MAX = 256`, TTL 1 h) only dedupes exact repeats. A member - pasting many distinct URLs drives unbounded outbound HTTP from the operator's - machine — an amplification/DoS vector and a way to disclose the operator's IP - to arbitrary hosts on demand. -2. **Port is not restricted.** `safe_url()` validates the scheme and the resolved - IP but passes `parts.port` straight through. A member can point the node at - `http://:` — third-party port scanning from - the operator's address, and reaching services that are internet-routable but - firewalled to the node's network. -3. **DNS rebinding.** `safe_url()` resolves and checks the address, then - `httpx.get()` resolves again at connect time. The module documents this as a - deferred residual ("closed properly by pinning the checked IP"). Until the - pin lands, a name that answers public on check and internal on connect is a - way in. -4. **Image decode.** `fetch_image` → `_downscale` opens attacker-supplied bytes - with Pillow; `Image.open` + `thumbnail` after a full decode. Pillow's default - decompression-bomb guard applies, but a 2 MB input is allowed and the guard is - the only ceiling. - -**Fix.** Add a per-connection and per-node rate limit on `LINK_PREVIEW_REQ` -(and the same for `MEDIA_META_REQ` / `TMDB_SEARCH_REQ`); restrict the port to -80/443; pin the checked IP for the actual connection (resolve once, connect to -the literal, send `Host:`); set `PIL.Image.MAX_IMAGE_PIXELS` low and cap decoded -dimensions before `thumbnail`. - ---- - -### M4 — MHP federation: peer hubs are over-trusted, token audience is unchecked, revocation propagation is a no-op - -> **Fixed 2026-09-01.** -> - `receive_directory` binds `source_hub` to the token's verified `iss`, so a -> peer cannot relay or spoof a third hub's groups; the push is capped -> (500/request, 2000/peer), rows are type/length-checked, and a federated id -> that collides with a local group is refused. -> - `receive_revocation` no longer forwards a foreign-signed token to local nodes -> (the no-op). It verifies the inner token against the sending peer's key and, -> for `target == "group"`, prunes our copy of that peer's directory entry — a -> peer cannot revoke our users or a group it did not advertise. -> - `POST /mhp/directory` and `/mhp/revoke` reject a replayed `jti` within the -> token TTL. Audience binding is still unavailable (the sending side that would -> set `aud` is unbuilt); the replay check covers that concern for now. Tests in -> `test_federation.py`. - -**Location:** `api/federation.py:59-190`, `api/revocation.py:66-80` (node -`verify_and_apply`), `node/daemon.py:575-596` (`on_revocation`) - -1. **Audience never verified.** `_verify_mhp_token` accepts an optional - `expected_aud` but **no caller passes it** — `export_directory`, - `receive_directory` and `receive_revocation` all call - `_verify_mhp_token(token, db)`. `_issue_mhp_token` sets `aud = target_hub_id`, - so the check is available and deliberately unused. A token hub B minted for - hub C (valid 300 s) is replayable at any other hub that has B registered as a - peer. -2. **Any registered peer can inject the directory.** `receive_directory` iterates - an unbounded `body.groups` list with peer-chosen `id` and `name`, upserting - `FederatedGroup` rows. No cap, no validation. `name` is rendered in the SPA - Explore view; `id` is peer-chosen and shares the UUID space with local groups. -3. **Revocation propagation does nothing on nodes.** `receive_revocation` passes - the peer's token straight to `broadcast_revocation`, which forwards it to - local nodes. Nodes verify a revocation token against **their own hub's public - key** (`session.hub_pk_pem`), so a peer-signed token fails - `jwt.decode(...)` and is dropped with a warning. The federated - `/mhp/revoke` path therefore silently accomplishes nothing — false assurance - that "revocations propagate" across a federation. - -**Impact.** A malicious or compromised peer hub can flood/poison the local public -directory and cannot be relied on to actually revoke anything. Cross-hub token -replay within a federation. All of this is bounded by the admin having explicitly -run `POST /mhp/peers` — federation is opt-in and manual — so the adversary is "a -peer the admin chose to trust", which is exactly the adversary MHP's own auth is -supposed to constrain. - -**Fix.** Pass `expected_aud=_hub_id` in every `_verify_mhp_token` call; cap -`body.groups` and validate each row; namespace `FederatedGroup.id` or refuse an -`id` that collides with a local group; for revocation, either re-sign accepted -peer revocations with the local hub key before broadcasting (with a policy on -which peers may revoke which targets) or drop the endpoint and document that -revocation does not federate. - ---- - -### M5 — No CSP or security headers on the hub-served SPA (second review L5, still open) - -> **Fixed 2026-09-01.** A middleware in `create_app` adds `Content-Security-Policy`, -> `X-Content-Type-Options: nosniff`, `Referrer-Policy` and `X-Frame-Options: DENY` -> to every response. `webapp.CSP` is the same policy the desktop client already -> enforces on these exact UI files (`default-src 'none'`, `script-src 'self' -> 'wasm-unsafe-eval' ` — the hub origin is not a script source, -> `frame-ancestors 'none'`, `base-uri 'none'`, `form-action 'none'`), plus the -> reCAPTCHA hosts. The shell's dead `window.__MB_ASSET_V` inline script is -> removed so no inline `'unsafe-inline'`/nonce is needed for scripts. **Wants a -> pass against the running SPA** — a mis-tuned CSP shows as a blank page — but it -> matches a policy already proven with these files under Electron. SRI on the -> `/a//` scripts is still not done (same-origin, so lower value than the -> CSP). Tests in `test_security_headers.py`. - -**Location:** `api/webapp.py:80-123`, `app.py:160-214` - -The SPA shell is returned with only `Cache-Control: no-store`. There is no -`Content-Security-Policy`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`, -`X-Frame-Options` / `frame-ancestors`, and no Subresource Integrity on the -scripts loaded from `/a//` (including the vendored `argon2.min.js`). The -hub sets no CORS middleware (correct) but also no protective headers at all. - -For an application whose threat model explicitly includes "the hub could inject -JS" (T3) and which now renders third-party OpenGraph images and TMDB/MusicBrainz -metadata inside the group UI, a strict CSP (`default-src 'none'`, an explicit -`connect-src`/`img-src`, `frame-ancestors 'none'`, `base-uri 'none'`) plus SRI is -the cheap mitigation that makes a *silent* injection harder and gives a browser -extension something to pin against. The node's loopback API already sets exactly -this kind of header block (`ui/app.py:100-117`); the hub does not. - -**Fix.** Add a response-header middleware on the hub with a strict CSP for the -SPA routes and `nosniff`/`Referrer-Policy`/`frame-ancestors` globally; add SRI -hashes to the `