[claude-tmgbedu]

Summary

• F4, auth bypass: ReverbServer subscribed any socket to private-/presence- channels without checking auth, so any WebSocket client could read private broadcasts. /broadcasting/auth now requires socket_id and returns a Pusher signature, key:HMAC-SHA256(secret, socket_id:channel[:channel_data]). The server checks it with compare_digest and replies pusher:subscription_error (401) when it fails. Presence channels return signed channel_data that contains user_id.
• F3, 404: the empty-path WebSocket Mount shadowed /broadcasting/auth and every route added after boot. Both endpoints are now plain router routes (add_websocket_route / add_route), so route order no longer matters. This also fixes the unresolved Request annotation.
• The server key and secret come from broadcasting.connections.reverb.

Breaking

• /broadcasting/auth response changes from {"auth": true} to {"auth": "<key>:<sig>"}. Clients must send that signature in pusher:subscribe; unsigned private/presence subscriptions are now rejected.
• socket_id is now required for /broadcasting/auth. Laravel Echo already sends it.

Test plan

• uv run pytest tests/broadcasting passes (88 tests). The tests run real HTTP and WebSocket traffic through the fully booted app: route reachability in either provider order, unsigned/forged/wrong-socket/wrong-secret subscribes rejected with no broadcasts delivered, signed subscribes receiving broadcasts, and tampered presence data rejected.
• The full suite passes without Postgres: 3308 passed.
• ruff check and ruff format --check are clean.

Security review follow-up (9d6d0ef)

Addresses #314 (review).

• Critical, signature confusion: socket ids are now Pusher digits.digits (secrets.randbelow). /broadcasting/auth and verify reject any other socket id, and channel names outside the Pusher charset, including any :. channel_data is signed and verified only for presence- channels. /broadcasting/auth returns 403 for public channels and anonymous users. The server verifies against the connection's own socket id. Regression tests replay the reviewer's attack on private-orders.1 and presence-room.vip; before the fix both ended in subscription_succeeded.
• High, default secret: REVERB_APP_SECRET has no default. The provider raises at boot outside local/testing when the secret is empty or "secret", and an empty secret rejects every protected subscription and auth request.
• Medium, malformed frames: binary, non-JSON or wrongly shaped frames get pusher:error (4200) and the connection stays open. Frames over 10 000 characters close with 1009. Connection and subscription cleanup runs in finally, and empty channels are dropped.
• Low: malformed JSON gets 400 and a non-object body gets 422. Presence users without an id get 403.

Tests: uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3355 passed, 91.51% coverage. ruff and basedpyright are clean on broadcasting. A live check on uvicorn with a websockets client: the attack gets 422 and then subscription_error, and the connection count returns to 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QgHs6PRFJehnb1BbCNtLww

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[claude-tmgbedu]

Security review + QA of 55594c0: CHANGES NEEDED

The F3 route fix works. The F4 fix blocks the simple cases (no auth, forged auth, another socket's signature, tampered presence channel_data). It can still be bypassed without logging in, though, because the signed string is ambiguous.

🔴 Critical: signature confusion lets an anonymous client into any private/presence channel

signature.py:11 joins socket_id:channel[:channel_data] with : and doesn't constrain any of the parts. Two other choices make that exploitable:

• provider.py:117: /broadcasting/auth accepts any client-supplied socket_id string, colons included.
• provider.py:131 plus auth.py authorize(): the endpoint signs public channels for anonymous users, because public channels are always allowed.
• server.py:47: channel_data from the client is passed to verify() for private- channels too, not only presence-.

Repro (real uvicorn plus a websockets client, no user attached):

1. Connect and read your socket id R.
2. POST /broadcasting/auth with channel_name=news and socket_id=R:private-orders.1. You get 200 and a signature over R:private-orders.1:news.
3. Subscribe to private-orders.1 with that auth and channel_data: "news". You get subscription_succeeded, and every later broadcast on private-orders.1 reaches the attacker. presence-room.vip falls the same way.

Fix (all three are cheap; do at least the first two):

• Validate socket_id strictly in the endpoint, either the server's UUID format or Pusher's ^\d+\.\d+$, and reject channel names containing :.
• Use channel_data in verify only for presence- channels. Never use it for private-.
• Return 403 from /broadcasting/auth for non-private/presence channels, as Laravel does. Nothing legitimate needs a signature for a public channel.
• Add a regression test for the repro above.

🟠 High: the fallback secret "secret" is accepted

config.py:11 (env("REVERB_APP_SECRET", "secret")) and provider.py:42: if REVERB_APP_SECRET is unset, anyone can mint valid signatures with the public default. I confirmed that a self-signed sign("local","secret",sid,"private-orders.1") subscribes. The server should refuse to start, or reject every private subscribe, when the secret is unset, empty or the default, at least outside local/testing.

🟡 Medium: malformed frames crash the socket handler and skip cleanup

server.py:31-33: only WebSocketDisconnect is caught. A non-JSON frame, "data": "x" or a non-dict message raises (JSONDecodeError/AttributeError, traceback in the log). The socket then stays in ConnectionManager and ChannelManager forever, so a client can leak memory by repeating it. Put the cleanup in a finally and drop or ignore bad frames. The receive size limit is the ASGI server's default (uvicorn: 16 MB).

🟡 Low

• provider.py:113: a JSON body that isn't an object ([1]) returns 500. It should return 422.
• ADR: the file is 036-…, but its heading says # 035. and the docs/adr/AGENTS.md row is [035](035-broadcasting-channel-auth.md), which is a broken link.
• Presence user_id falls back to "" when the callback allows a None user. Consider 403 when there's no user on presence channels.
• Pre-existing, out of scope: no pusher:unsubscribe handling, client-* events are silently dropped (so nothing leaks), and the socket id is a UUID rather than Pusher's n.n format (pusher-js still works).
• CSRF/CORS: the endpoint accepts form posts. Cross-site requests can't read the signature unless CORS is permissive, so it's fine as is. Worth a note in the docs.

What passed

• uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3308 passed, 10 skipped, 91.46% coverage. tests/broadcasting: 88 passed.
• On a live server: subscribing with no auth or a forged auth gets subscription_error 401. An anonymous /broadcasting/auth for a private channel gets 403. Public channels still subscribe. /broadcasting/auth and routes added after boot are reachable (F3 fixed). compare_digest is used, and channel prefixes are case-sensitive in both the endpoint and the server, which is consistent.

[claude-tmgbedu]

Security re-review + QA of 9d6d0ef: APPROVE

All the earlier findings are fixed and verified on a live uvicorn server (APP_ENV=local, strong secret, websockets + httpx clients). I found no new blocking issues.

Earlier findings, re-run

Check	Result
Colon injection, private (socket_id=<R>:private-orders.1 on a public channel)	/broadcasting/auth 422 socket_id is invalid
Colon injection, presence	422
: in channel name	422
Private subscribe with a valid signature plus junk channel_data	Succeeds, but channel_data is ignored for private-, so the confusion is gone
/broadcasting/auth for a public channel (anonymous or user)	403
Anonymous request for private/presence	403
Signature replayed from another socket / reused on another channel	subscription_error 401
Forged with "secret" or ""	401
Presence: missing channel_data or spoofed user_id	401. Valid signed data subscribes
Bad JSON, data:"x", binary, JSON list, non-string event	pusher:error 4200, and the socket stays usable (ping → pong)
20 000-char frame	pusher:error, then close 1009
Cleanup	connections/channels back to 0/0 after close, and after 50 sockets each sent a garbage frame and subscribed
HTTP: malformed JSON / non-object / non-string channel	400 / 422 / 422

Fresh adversarial pass

• Socket id bypasses: 1.2\n, Arabic-Indic digits, 1.2.3, -1.2, 1e3.2 and 1.2 all return 422. fullmatch and re.ASCII close the $-newline and Unicode-digit gaps. Ids come from secrets.randbelow(10**10), and the server verifies against the connection's own id.
• Channel names: full-width digit, Cyrillic homoglyph, trailing \n, leading space and over 200 chars are rejected with 422 on /broadcasting/auth and 400 on subscribe. Private-… (different case) is a separate public channel and doesn't receive private- broadcasts. That matches Pusher.
• Secret guard: production/staging/Production/LOCAL with the secret unset, "" or "secret" raise RuntimeError at register. Production with a real secret boots. In local with an empty secret, all private auth is denied, so it fails closed. If APP_ENV is unset and there's no .env, the framework already raises before providers run. Note: PYTEST_CURRENT_TEST in the environment forces testing and therefore skips the guard. That behaviour is pre-existing in the framework and needs control of the environment, so it's informational only.
• Echo/pusher-js: the channel regex and 200-char limit match Pusher. The auth request (form socket_id/channel_name), the response shape, the connection_established data string and the pusher:error / subscription_error payloads all match the protocol.
• AGENTS.md: the diff adds no comments or docstrings. ADR 036's heading and index row are fixed.

Non-blocking follow-ups (Low / out of scope)

1. pusher:unsubscribe isn't handled, so after Echo leave() the socket keeps receiving that channel until it disconnects. This is pre-existing, but it matters more now that private channels are gated.
2. There's no cap on subscriptions per socket. A client can subscribe to unbounded public channel names (memory grows until it disconnects). Consider a limit.
3. Presence subscription_succeeded sends data: {}. pusher-js expects {presence:{ids,hash,count}}, so Echo here()/members won't work. This is a functionality gap, not a security one.
4. max_message_size is checked only after the ASGI server has buffered the frame (uvicorn ws_max_size defaults to 16 MB). Consider documenting --ws-max-size.

Tests

• uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3355 passed, 10 skipped, coverage 91.51%.
• tests/broadcasting: 135 passed.