Repository navigation
fix(ws,router): post-merge audit hardening for the upgrade seam (#13) - #92
Merged
Merged
Conversation
- WsRouter.allowOrigin: exact-match Origin allowlist; once set, missing
or unlisted Origin headers reject the handshake with 403 (CSWSH).
- Sec-WebSocket-Key must decode from base64 to a 16-byte nonce
(RFC 6455 4.2.1); Connection needs no check because node:http only
emits upgrade when the token is present.
- Peer close reason is UTF-8 validated; invalid closes with 1007.
- upgradeHandler() catches a throwing dispatch and destroys the socket
instead of crashing the process; UpgradeFn re-exported.
- Docs: handshake table + origin section, fix the header-only 1009
overclaim for fragmented messages, honest timeout limitation; README
wires server.on("upgrade"). Tests: key format, token-list Connection,
origin allowlist, fragmented UTF-8, maxPending feed refusal,
zero-length messages.
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Deploying chttpjs with
|
| Latest commit: |
bd93e23
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac806100.chttpjs.pages.dev |
| Branch Preview URL: | https://devin-1792395841-ws-audit.chttpjs.pages.dev |
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
1 participant
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
Post-merge audit follow-up for issue #13 (
@chttp/wsupgrade seam). The package and most audit items (handler error boundaries, linearfeed(),_fchunksassembly, backlog cap) already landed on main; this PR closes the remaining findings.WsRouter.allowOrigin(origin)— chainable exact-match allowlist. Once at least one origin is registered, handshakes with a missing or unlistedOriginheader are rejected403. Default stays open (same asws), documented as a CSWSH risk for cookie-authenticated apps.Sec-WebSocket-Keyformat check — must be base64 decoding to a 16-byte nonce (RFC 6455 4.2.1); else400.1007.upgradeHandler()now catches a throwing dispatch and destroys the socket instead of crashing the process;UpgradeFnis re-exported;contracts/upgrade-contract.mddispatch semantics updated to match.docs/websocket.mdhandshake table gains the 400/403 rows plus an Origin allowlist section; the "frames rejected from the header alone" claim is narrowed to single frames (fragmented messages buffer up to ~maxPayloadbefore the 1009 fires);packages/ws/README.mdnow wiresserver.on("upgrade", app.upgradeHandler())sinceserve()never attaches it.Not changed, deliberately:
Connectionheader checking is dead code on node:http (the server only emits"upgrade"when the token is already present — verified by probe; docs say so). Close-handshake timeout remains unimplemented and is now listed as a known limitation.Tests
packages/ws: +5 test blocks — key format 400s, token-list/case-insensitive headers, origin allowlist 101/403, fragmented invalid UTF-8 → 1007, close-reason invalid UTF-8 → 1007, single-writemaxPendingrefusal → 1009, zero-length text/binary round-trip. 21/21 pass.Gates:
pnpm --filter @chttp/router testgreen,pnpm lintclean (pre-existing warnings only),scripts/scriptc-gate.sh10/10 gates at 100%,pnpm smoke:ws:ffibuilds the vendored ws-echo ELF and serves FFI routes.Refs #13
Link to Devin session: https://app.devin.ai/sessions/b44c98ebb2794946b91a700970496547
Open in Devin Desktop: https://app.devin.ai/desktop/session/b44c98ebb2794946b91a700970496547?variant=devin
Requested by: @espetro