fix(planner): require session row before accepting Yjs WebSocket upgrade
Addresses planner-audit #1 (CRITICAL — WebSocket joins unauthenticated) with the narrow fix. The upgrade handler used to immediately \`handleUpgrade\` for any \`/sync/<id>\` path, then \`getOrLoadDoc\` *created* a Y.Doc on demand. An attacker connecting to random sessionIds could: - exhaust process memory by spinning up Y.Doc objects for sessions that don't exist (no DB row backed them), and - resurrect closed/expired sessions by reconnecting (closed rows were still cacheable to \`docs\`). Now: the upgrade handler calls \`getSession(sessionId)\` first (\`SELECT \\* FROM planner.sessions WHERE id = ? AND closed = false\`). Missing or closed → \`socket.destroy()\` before handshake. DB unreachable also closes — fail closed; the client reconnects after backoff. Costs one DB roundtrip per upgrade. Upgrades are rare vs message volume, so the overhead is negligible. Note: we are NOT adding a join token. The planner is anonymous-by-design (see CLAUDE.md \"sessions are anonymous and ephemeral\") — knowing the URL still equals membership. The check here only guards against attacker-supplied sessionIds with no backing row. Full repo: pnpm typecheck / lint / test all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
643266e619
commit
4e2c5f7d6b
1 changed files with 23 additions and 3 deletions
|
|
@ -5,7 +5,7 @@ import * as awarenessProtocol from "y-protocols/awareness";
|
|||
import * as encoding from "lib0/encoding";
|
||||
import * as decoding from "lib0/decoding";
|
||||
import type { IncomingMessage, Server } from "node:http";
|
||||
import { saveSessionState, loadSessionState, touchSession } from "./sessions.ts";
|
||||
import { saveSessionState, loadSessionState, touchSession, getSession } from "./sessions.ts";
|
||||
import { plannerActiveSessions, plannerConnectedClients } from "./metrics.server.ts";
|
||||
|
||||
const messageSync = 0;
|
||||
|
|
@ -221,7 +221,7 @@ function handleMessage(ws: WebSocket, message: Uint8Array, sessionId: string) {
|
|||
export function setupYjsWebSocket(server: Server): WebSocketServer {
|
||||
const wss = new WebSocketServer({ noServer: true });
|
||||
|
||||
server.on("upgrade", (request, socket, head) => {
|
||||
server.on("upgrade", async (request, socket, head) => {
|
||||
const url = new URL(request.url ?? "", `http://${request.headers.host}`);
|
||||
const match = url.pathname.match(/^\/sync\/(.+)$/);
|
||||
|
||||
|
|
@ -230,8 +230,28 @@ export function setupYjsWebSocket(server: Server): WebSocketServer {
|
|||
return;
|
||||
}
|
||||
|
||||
const sessionId = match[1]!;
|
||||
// Reject upgrades for sessions that don't exist or are already
|
||||
// closed/expired. Before this guard, `getOrLoadDoc` happily
|
||||
// *created* a fresh Y.Doc for any sessionId, so an attacker
|
||||
// hitting /sync/<random-uuid> repeatedly could exhaust memory
|
||||
// and resurrect closed sessions. `getSession()` already filters
|
||||
// out `closed = true` rows.
|
||||
try {
|
||||
const session = await getSession(sessionId);
|
||||
if (!session) {
|
||||
socket.destroy();
|
||||
return;
|
||||
}
|
||||
} catch {
|
||||
// DB unreachable — fail closed (refuse) rather than open. The
|
||||
// client will reconnect after backoff once the DB recovers.
|
||||
socket.destroy();
|
||||
return;
|
||||
}
|
||||
|
||||
wss.handleUpgrade(request, socket, head, (ws) => {
|
||||
wss.emit("connection", ws, request, match[1]);
|
||||
wss.emit("connection", ws, request, sessionId);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue