fix: harden c2c relay authorization and error handling - #27
Draft
sidmorizon wants to merge 2 commits into
Draft
Conversation
- Reject client-to-client messages whose sender socket has not joined the target room, closing an unauthenticated cross-room injection path (cancelTransfer / verifyPairingCode / forged-response hijack) that was gated only by knowledge of the room id. - Drop the stack trace from E2eeError.toJSON so server internals are not serialized to clients; pino still records it server-side via err.stack. - Document that the room encryptionKey is not the E2EE key and is currently unused, reserved for a future transport-layer encryption. - Add a smoke-test assertion that a non-member cannot inject c2c traffic.
The corsOrigins allowlist (read from CORS_ORIGINS) was built but never enforced: the origin callback returned true for every origin, with the reject branch commented out. Replace it with an explicit permissive config and document why Origin is not an auth boundary here - the primary clients (native, desktop file://) send no usable Origin, there are no cookie credentials to protect, and Origin is forgeable by non-browser clients. Access control remains the out-of-band pairing code plus the c2c room membership check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security review of the Prime Transfer E2EE relay. The transport is genuine end-to-end encryption — the real key is derived client-side from the out-of-band pairing code plus an ECDHE exchange and never reaches the server — so this focuses on the relay's authorization and hardening, not the crypto.
Changes
1. Enforce room membership on the client-to-client relay (main fix)
JsBridgeE2EEServerrelayede2ee-c2c-request/e2ee-c2c-responseto theroomIdtaken from the client envelope without checking the sender had joined that room. Any connected socket that knew a room id could inject c2c traffic into a live session —cancelTransfer(rate-limit exempt) to abort a transfer,verifyPairingCodespam to exhaust the peer's pairing-attempt budget, or a forged response to a pending request. Confidentiality was never at risk (payloads use the pairing-code-derived key the attacker lacks), but it was an unauthenticated disruption primitive.The relay now drops any c2c message whose sender socket is not a member of the target room (
socket.rooms.has(roomId)). Legitimate peers always join viajoinRoombefore sending c2c, so genuine traffic is unaffected.2. Stop leaking server stack traces to clients
E2eeError.toJSON()serializedstackonto the payload sent to clients. Removed it; pino still records the stack server-side.3. Clarify the room
encryptionKeyDocumented that it is not the E2EE key (clients never use it) and is reserved for a future transport-layer encryption, so it isn't mistaken for the secret protecting user data.
4. Remove misleading unenforced CORS allowlist
The
corsOriginsallowlist (read fromCORS_ORIGINS) was built but never enforced — the origin callback returnedtruefor every origin. Replaced with an explicit permissive config (same behavior) and documented why Origin is not an auth boundary here: native/desktop clients send no usable Origin, there are no cookie credentials to protect, and Origin is forgeable by non-browser clients.Testing
Extended
test/smoke.ts: a socket that never joined a room cannot inject c2c into it, and the two real members keep communicating afterward.🤖 Generated with Claude Code
https://claude.ai/code/session_01AuowqYp863fJaNNjEKvhPT
Generated by Claude Code