From 4e2c5f7d6b87d687ed8e6b6e0df8e1551fe68dfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= <hi+github@ullrich.is>
Date: Tue, 26 May 2026 00:24:16 +0200
Subject: [PATCH] fix(planner): require session row before accepting Yjs
 WebSocket upgrade
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 apps/planner/app/lib/yjs-server.ts | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/apps/planner/app/lib/yjs-server.ts b/apps/planner/app/lib/yjs-server.ts
index de07987..de48bfb 100644
--- a/apps/planner/app/lib/yjs-server.ts
+++ b/apps/planner/app/lib/yjs-server.ts
@@ -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);
     });
   });