Skip to content

Cap each WebSocket action frame at one legal action - #39

Open
JacobStephens2 wants to merge 1 commit into
masterfrom
issue-38
Open

JacobStephens2 wants to merge 1 commit into
masterfrom
issue-38

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

upgrade /ws
  authenticate seat
  WebSocketServer maxPayload = one compact action (181 bytes)
    message
      parse action
      rooms.action
      broadcast snapshot
    frame larger than maxPayload
      ws closes 1009
      message handler does not run

A seated player's socket accepted frames up to the ws 8.21.1 default of 100 MiB and parsed them in the message handler. attachGameSockets now sets maxPayload to the compact JSON size of the longest action the engine can mint: PLAY_OPEN with two follow-<safe integer> instance ids and a safe-integer sequence.

Seam: the authenticated /ws seat socket (packages/server/src/sockets.ts, attachGameSockets). One action message per frame. The room either applies it and broadcasts a snapshot, or ws closes a larger frame before the handler runs.

Evidence

  • Before: new WebSocketServer({ noServer: true }) in packages/server/src/index.ts left maxPayload at 104857600. The message handler ran JSON.parse on the frame that arrived.
    After: npx vitest run packages/server/tests/action-frame.test.ts passed (2 tests, 96ms). A compact CONFIRM_HAND moves the room sequence from 0 to 1 and is broadcast. The same action padded to ACTION_FRAME_MAX_BYTES + 1 closes with 1009, sends no further message, and leaves the sequence at 0.

Merge Danger

Door: two-way

Revert the commit and the socket returns to the 100 MiB default. No data migration.

Blast Radius: online seat sockets

One oversized frame closes that socket. A compact legal action still applies. The WS_ERR_UNSUPPORTED_MESSAGE_LENGTH error is ignored so the close does not exit the process. Any other socket error is still thrown.

Unaddressed findings

Standards

Judgement call - Duplicated Code (packages/server/src/sockets.ts lines 10-22, largestInstanceId and ACTION_FRAME_MAX_BYTES). The reviewer says the cap re-encodes the engine instance-id shape `${role}-${nextInstance}` and the PLAY_OPEN field list, and that a longer action would not fail the build.

Refuted by what those lines are: a byte length for maxPayload, not a second mint and not a second PLAY_OPEN. Instance ids are still minted only in packages/engine/src/game.ts (makeInstances and newOwnedInstance, `${role}-${nextInstance++}`). GameActionSchema in packages/engine/src/schemas.ts leaves those strings unbounded. Issue 38 asks for a frame cap that still accepts one legal action, which is this measurement of the longest compact message the engine can mint. The engine should not own WebSocket frame bytes, so there is no shared helper.

The finding does not say the behaviour is wrong and does not include a failing test.

Spec

None.

Security

None.

Security review outcome

  • Security review incomplete: session failed, ended early or omitted a valid final line; see Session log

Review coverage

Both axes read every changed file: packages/server/src/index.ts, packages/server/src/sockets.ts, and packages/server/tests/action-frame.test.ts. No changed file was left unread.

Closes #38

Built with grok · grok-4.7 · xhigh

A seated player's socket accepted frames up to the ws default of 100 MiB and parsed them in the message handler. The server now sets maxPayload to the compact JSON size of the longest action the engine can mint: PLAY_OPEN with two follow-<safe integer> instance ids and a safe-integer sequence (181 bytes).

ws closes a larger frame with 1009 before it emits message, so trailing whitespace after a CONFIRM_HAND does not apply that action. A compact CONFIRM_HAND still applies and is broadcast. The payload error is ignored so the close does not exit the process.

The socket wiring now lives in packages/server/src/sockets.ts. Tests: packages/server/tests/action-frame.test.ts.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cap the server's WebSocket action frames

1 participant