Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/adr/0005-session-boundaries.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,10 @@ flowchart TB
account credentials; room tokens remain script-visible and are intentionally
short-lived with the room.

Login rechecks the stored credential after asynchronous passcode verification,
and profile writes compare the verified credential inside their transaction.
A concurrent password change therefore cannot be undone by an older profile
request or admit a login using the replaced passcode. Profile verification has
an account-level budget shared across rotated sessions and also consumes the
same address-level credential budget as login.

6 changes: 6 additions & 0 deletions docs/adr/0007-same-origin-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ foreign form targets remain blocked. Production session and deletion cookies
always carry the `Secure` attribute; development deployments may opt into it but
cannot opt production out.

Static serving validates normalized and real filesystem paths, including
directory links, before reading or caching bytes. Case and Windows path aliases
cannot bypass private-path checks. Only GET and HEAD serve assets. Cached
buffers use an 8 MiB least-recently-used budget with a 512 KiB per-file ceiling;
larger files stream with backpressure instead of being retained in memory.

7 changes: 5 additions & 2 deletions docs/adr/0012-shared-app-install-surface.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ manifest so every shared install action installs JavaScript Playground rather th
a game-specific application. The shell registers a root service worker that
uses network-first navigation and stale-while-revalidate static assets in a
versioned cache. Cache writes run in the background so a fetched response is
not delayed by storage. API requests are explicitly excluded from interception
and caching. Generated raster icons are
not delayed by storage. API requests, including encoded path aliases, are
explicitly excluded from interception and caching. Responses declaring
`Cache-Control: private` or `no-store` are never stored. Changes to this privacy
boundary advance the cache version so activation removes older cached responses.
Generated raster icons are
not part of the atomic app-shell precache, and the Node server creates any
missing icons before it begins listening so clean direct-server checkouts remain
installable.
Expand Down
6 changes: 6 additions & 0 deletions docs/adr/0017-websocket-resource-limits.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ per minute, 200 rooms across the arcade, 100 rooms per game, 5 rooms per
address, and 50 entries per public listing. Deployments may lower or raise these
limits through documented environment variables.

Every outbound room message also passes a shared 1 MiB per-socket queued-byte
ceiling, including the message being sent. Slow consumers are terminated and
can use the existing room resume flow; unsent result receipts remain pending.
Protocol errors terminate their offending socket without escaping as unhandled
process errors. Replaced sockets cannot issue commands on any game endpoint.

**Consequences:** A remote peer cannot allocate unbounded sockets, rooms, game
state, broadcast work, or lobby responses. Limits are intentionally
process-local because rooms are process-local. Multi-instance deployments need
Expand Down
75 changes: 75 additions & 0 deletions docs/security-review-2026-10-24.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Security and performance review — 2026-10-24

Reviewed from master `4bf2d39`. The review focuses on the network-facing Node
service, authentication and persistence, static-file isolation, shared browser
runtime and service worker, multiplayer lifecycle, and deployment/dependency
configuration. Game mechanics are covered at their trust boundaries and by the
regression suite; classic games and independent coding-challenge scripts were
not comprehensively audited.

## Fixed findings

| Priority | Finding and impact | Resolution and evidence |
| --- | --- | --- |
| High | Static isolation checked request text before filesystem normalization. Encoded separators could route a nominal game path into private server files or the default database directory. Windows aliases and filesystem links also bypassed the original checks. | Normalize private-path checks, reject ambiguous path syntax, and validate the real destination inside the public root before reads. Unit tests cover aliases and directory links; live HTTP tests deny encoded server, data, and package paths. |
| High | Accepted WebSockets had no `error` listener. One invalid protocol frame emitted an unhandled error and terminated the process. | Install the error handler before routing any game socket. A live test reproduces the original process failure and verifies the fixed server remains healthy after malformed frames on all three endpoints. |
| High | Authentication and profile writes trusted a passcode hash read before awaiting scrypt. Concurrent credential changes could permit an old login or let an older profile write restore the old hash. | Recheck the login credential and condition the transactional profile write on the verified hash. Tests replace credentials while verification is pending and verify both older operations are rejected. |
| Medium | Profile PATCH verified the current passcode without throttling. A valid session allowed repeated credential guesses and expensive scrypt work outside the login budget. | Limit profile verification to ten attempts per account per fifteen minutes, across session rotations, and share login's address-level budget. A live test checks successful rotation, failed attempts, and HTTP 429 with Retry-After. |
| Medium | Tic-tac-toe omitted the socket-ownership guard used by the other games. An action queued on a closing, replaced connection could still mutate the resumed match. | Check current socket ownership before dispatch. A live regression reproduces an unauthorized old-socket move when the guard is removed; the fixed test also completes the resumed match and rematch. |
| Medium | The service worker excluded only literal `/api/` URLs and ignored private/no-store response directives. Encoded API aliases could store account responses in persistent browser cache. | Exclude decoded API paths, honor response privacy directives, and advance the cache to v16 to purge previous entries. Executable worker tests cover encoded aliases, private/no-store responses, public static caching, and old-cache removal. |
| Medium | Room broadcasts queued data without an outbound byte ceiling. Slow receivers could accumulate state snapshots even while responding to heartbeat pings. | Route every room send through a 1 MiB queue ceiling and terminate slow consumers. Tests cover all room managers, UTF-8 byte accounting, arena delivery, and pending result receipts across reconnect. |

## Performance changes

- Static cached buffers now have an 8 MiB LRU budget and a 512 KiB per-file
ceiling. Larger files stream instead of being read into the cache. Existing
conditional responses, HEAD requests, and resolved-asset CSP remain intact.
Tests verify eviction and streaming without `readFile` allocations.
- Save listings select only slot metadata; screenshot retrieval selects only
image columns. Previously, listing five slots could copy up to roughly
2.5 MiB of unused state and image payloads into JavaScript.
- An illustrative Node 26.12.1 in-memory SQLite benchmark used five slots with
240 KiB of state data and 256 KiB screenshots. Across 300 listings, the old
SELECT-all query plus minimal metadata mapping took 169.4 ms; the revised
`GameSaves.list` took 16.3 ms. This measures one local workload, not a production
latency guarantee. Existing save isolation, revision, and storage-budget tests
still pass.

## Remaining follow-ups

- Profile and leaderboard reads aggregate persisted history. The leaderboard's
correlated best-result query and expression sort deserve a production-sized
benchmark before choosing a ranking index, window query, or materialized
summary. Any new index should ship as the next ordered migration.
- The browser service worker still uses an open-ended runtime cache of public
URLs. Consider a bounded entry policy and normalization of asset query keys if
offline storage usage becomes significant.
- In the phone smoke check at 390 × 844, Tic-tac-toe's board extended below the
initial viewport and New game started around y=1232. Natural scrolling works,
but a separate visual pass should bring the board and primary controls closer
together without reducing touch targets.
- Preserve the deployment assumptions: TLS at the reverse proxy, a correctly
configured TRUST_PROXY boundary, and persistent-data backups. Source review
does not establish the configuration of an existing deployment. Solo/local
result facts remain client-authored within server bounds, as accepted by
ADR 0003; they do not provide replay-proof anti-cheat.

## Verification

- Baseline: 341 passing tests. Final: 359 passing tests on Node 26.12.1 with the
locked ws 8.22.0 dependency.
- Focused regressions, full `npm test`, `npm run check`, and `git diff --check`.
- `npm audit --omit=dev`: zero reported vulnerabilities in the production
dependency graph. Locked dependencies installed with `npm ci --ignore-scripts`.
- Live HTTP/WebSocket tests run with temporary databases and ephemeral ports;
local mechanics and room disconnect/resume/rematch tests remain covered.
- Browser checks loaded the shared shell and Tic-tac-toe, exercised a local
move, checked phone and desktop navigation, and reported no console warnings
or errors. Screenshot capture repeatedly timed out, so visual screenshot QA
is incomplete; phone geometry was inspected through the DOM.
- Docker is unavailable in this environment. The container vulnerability scan
was not run locally; the existing CI build job performs the pinned scan.

The changes implement the existing security and resource boundaries in ADRs
0005, 0007, 0012, and 0017 without adding a framework, build step, dependency,
schema change, or new subsystem.
7 changes: 5 additions & 2 deletions server/accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ class Accounts {
async authenticate(gamertagValue, passcodeValue) {
const row = this.database.prepare('SELECT * FROM users WHERE gamertag = ? COLLATE NOCASE').get(normalizeGamertag(gamertagValue));
if (!row || !await verifyPasscode(String(passcodeValue || ''), row.passcode_hash)) throw new Error('Gamertag or passcode is incorrect.');
return publicUser(row);
const current = this.database.prepare('SELECT * FROM users WHERE id = ? AND passcode_hash = ?').get(row.id, row.passcode_hash);
if (!current) throw new Error('Gamertag or passcode is incorrect.');
return publicUser(current);
}

createSession(userId) {
Expand Down Expand Up @@ -76,7 +78,8 @@ class Accounts {
try {
const passcodeHash = passcode ? await hashPasscode(passcode) : current.passcode_hash;
this.database.exec('BEGIN IMMEDIATE');
this.database.prepare('UPDATE users SET gamertag = ?, passcode_hash = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?').run(gamertag, passcodeHash, userId);
const updated = this.database.prepare('UPDATE users SET gamertag = ?, passcode_hash = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ? AND passcode_hash = ?').run(gamertag, passcodeHash, userId, current.passcode_hash);
if (!updated.changes) throw new Error('Current passcode is incorrect.');
this.database.prepare('DELETE FROM sessions WHERE user_id = ?').run(userId);
this.database.exec('COMMIT');
} catch (error) {
Expand Down
6 changes: 4 additions & 2 deletions server/battle-tanks-rooms.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const { send, sendSerialized } = require('./websocket-messages');

const game = require('../battle-tanks/scripts/game');
const { createRoomCode, createRoomToken, normalizePasscode } = require('./room-identity');

Expand Down Expand Up @@ -155,8 +157,8 @@ class BattleTanksRooms {
}
}
disconnect(room, player, socket = player.socket) { if (player.socket !== socket) return false; Object.assign(player, { connected: false, socket: null, disconnectedAt: Date.now() }); room.paused = room.game.phase !== 'setup' && room.game.phase !== 'game-over'; room.touchedAt = Date.now(); return true; }
broadcast(room, message) { const body = JSON.stringify(message); room.players.forEach(item => { if (item?.socket?.readyState === 1) item.socket.send(body); }); }
broadcastState(room, { forceArena = false } = {}) { const arenaVersion = `${room.matchId}:${room.game.impactSerial || 0}`; room.players.forEach((item, side) => { if (item?.socket?.readyState !== 1) return; const includeArena = forceArena || item.arenaVersion !== arenaVersion; item.socket.send(JSON.stringify({ type: 'state', state: this.stateFor(room, side, { includeArena }) })); item.arenaVersion = arenaVersion; }); }
broadcast(room, message) { const body = JSON.stringify(message); room.players.forEach(item => sendSerialized(item?.socket, body)); }
broadcastState(room, { forceArena = false } = {}) { const arenaVersion = `${room.matchId}:${room.game.impactSerial || 0}`; room.players.forEach((item, side) => { if (item?.socket?.readyState !== 1) return; const includeArena = forceArena || item.arenaVersion !== arenaVersion; if (send(item.socket, { type: 'state', state: this.stateFor(room, side, { includeArena }) })) item.arenaVersion = arenaVersion; }); }
broadcastStates(now = Date.now()) { for (const room of this.rooms.values()) if (room.game.phase === 'projectile-flight' && now - room.lastBroadcast >= 40) { room.lastBroadcast = now; this.broadcastState(room); } }
}

Expand Down
5 changes: 3 additions & 2 deletions server/http-security.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const net = require('node:net');
const path = require('node:path');

class HttpError extends Error {
constructor(message, status = 400, closeConnection = false) { super(message); this.status = status; this.closeConnection = closeConnection; }
Expand Down Expand Up @@ -58,7 +59,7 @@ async function readJson(request, maximum = 10000, { guard = null, ip = 'unknown'
const PRIVATE_TOP_LEVEL = new Set([
'compose.yaml',
'compose.nas.yaml',
'Dockerfile',
'dockerfile',
'package.json',
'package-lock.json',
'releases.json',
Expand Down Expand Up @@ -133,7 +134,7 @@ function useSecureCookies(environment = process.env) {
}

function isPrivatePath(pathname) {
const segments = pathname.split('/').filter(Boolean);
const segments = path.posix.normalize('/' + pathname.replace(/\\/g, '/')).split('/').filter(Boolean).map(segment => segment.toLowerCase().replace(/[. ]+$/g, ''));
if (segments.some(segment => segment.startsWith('.'))) return true;
if (['server', 'tests', 'node_modules', 'data'].includes(segments[0])) return true;
return segments.length === 1 && PRIVATE_TOP_LEVEL.has(segments[0]);
Expand Down
8 changes: 6 additions & 2 deletions server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const { createBuildInformation } = require('./build-info');

const root = path.resolve(__dirname, '..');
generateIcons(root, { onlyMissing: true });
const port = Number(process.env.PORT) || 8080;
const port = process.env.PORT === '0' ? 0 : Number(process.env.PORT) || 8080;
const buildInformation = createBuildInformation();
const database = openDatabase();
const achievements = new Achievements(database);
Expand Down Expand Up @@ -79,6 +79,7 @@ function sessionCookie(token, expiresAt) { return `arcade_session=${encodeURICom
const loginLimiter = new RateLimiter(10, 15 * 60 * 1000);
const loginIpLimiter = new RateLimiter(50, 15 * 60 * 1000);
const registrationLimiter = new RateLimiter(5, 60 * 60 * 1000);
const profileLimiter = new RateLimiter(10, 15 * 60 * 1000);
const resultLimiter = new RateLimiter(60, 60 * 60 * 1000);
const achievementCheckpointLimiter = new RateLimiter(120, 60 * 60 * 1000);
const saveLimiter = new RateLimiter(120, 60 * 60 * 1000);
Expand Down Expand Up @@ -162,6 +163,7 @@ const server = http.createServer(async (request, response) => {
return json(response, 200, accounts.profile(user.id, parameters.get('page'), parameters.get('pageSize')));
}
if (pathname === '/api/profile' && request.method === 'PATCH') {
const retryAfter = Math.max(profileLimiter.consume(user.id), loginIpLimiter.consume(clientIp(request))); if (retryAfter) return throttle(response, retryAfter);
const updated = await accounts.update(user.id, await readRequestJson(request)), session = accounts.createSession(user.id);
return json(response, 200, { user: updated }, { 'set-cookie': sessionCookie(session.token, session.expiresAt) });
}
Expand Down Expand Up @@ -233,6 +235,7 @@ function acceptMessage(socket) {
}

websocketServer.on('connection', (socket, request) => {
socket.on('error', () => socket.terminate());
// Every websocket shares the same heartbeat. Tic-tac-toe used to return
// before these handlers were installed, so its healthy connections were
// terminated on the first heartbeat interval.
Expand Down Expand Up @@ -282,6 +285,7 @@ function handleTicSocket(socket, request) {
if (!acceptMessage(socket)) return;
try {
const message = parseMessage(raw);
if (membership && membership.player.socket !== socket) throw new Error('This connection has been replaced by a newer session.');
if (!membership && message.type === 'create-room') membership = createRoom(socket, ticRooms, () => ticRooms.create(socket, { ...message, user }));
else if (!membership && message.type === 'join-room') membership = joinRoom(socket, () => ticRooms.join(message.roomCode, socket, message.passcode, gamertag, user));
else if (!membership && message.type === 'resume') membership = ticRooms.resume(message.roomCode, message.playerToken, socket);
Expand Down Expand Up @@ -335,7 +339,7 @@ let lastTankTick = Date.now();
const tankSimulation = setInterval(() => { const now = Date.now(); tankRooms.tick((now - lastTankTick) / 1000, now); lastTankTick = now; tankRooms.broadcastStates(now); }, 1000 / 60);
const heartbeat = setInterval(() => websocketServer.clients.forEach(socket => { if (!socket.isAlive) return socket.terminate(); socket.isAlive = false; socket.ping(); }), 15000);

server.listen(port, '0.0.0.0', () => console.log(`JavaScript Playground listening on http://0.0.0.0:${port}`));
server.listen(port, '0.0.0.0', () => console.log(`JavaScript Playground listening on http://0.0.0.0:${server.address().port}`));
function shutdown() { clearInterval(simulation); clearInterval(ticCleanup); clearInterval(tankSimulation); clearInterval(broadcast); clearInterval(heartbeat); websocketServer.clients.forEach(socket => socket.close(1001, 'Server shutting down')); server.close(() => { database.close(); process.exit(0); }); }
process.on('SIGTERM', shutdown);
process.on('SIGINT', shutdown);
Expand Down
6 changes: 3 additions & 3 deletions server/rooms.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const { createGame, startGame, update, setInput, setColor, snapshot } = require('./game');
const { createRoomCode, createRoomToken, normalizePasscode } = require('./room-identity');
const { send, sendSerialized } = require('./websocket-messages');

class RoomManager {
constructor({ reconnectMs = 15000, roomTimeoutMs = 30 * 60 * 1000, random = Math.random, recordResult = null } = {}) {
Expand Down Expand Up @@ -111,8 +112,7 @@ class RoomManager {

deliverResult(player) {
if (!player.pendingResult || player.socket?.readyState !== 1) return;
player.socket.send(JSON.stringify({ type: 'result-recorded', result: player.pendingResult }));
player.pendingResult = null;
if (send(player.socket, { type: 'result-recorded', result: player.pendingResult })) player.pendingResult = null;
}

disconnect(room, player, socket = player.socket) {
Expand Down Expand Up @@ -143,7 +143,7 @@ class RoomManager {

broadcast(room, message) {
const body = JSON.stringify(message);
room.players.forEach(player => { if (player?.socket?.readyState === 1) player.socket.send(body); });
room.players.forEach(player => sendSerialized(player?.socket, body));
}

broadcastStates(serverTime = Date.now()) {
Expand Down
Loading
Loading