From d1fcd24f23e9f3c76bbb9d99cd1945b925a7fc0f Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Sun, 2 Aug 2026 00:48:02 +0530
Subject: [PATCH 1/9] fix(collab): validate and rate-limit comment mutations

Reject malformed comment/reply payloads, cap body length like chat,
and throttle mutations so peers cannot inflate the shared snapshot.
---
 tests/collab-comment-validate.test.ts  | 341 +++++++++++++++++++++++++
 workers/collab/src/comment-validate.ts | 159 ++++++++++++
 workers/collab/src/session.ts          | 117 +++++++--
 3 files changed, 599 insertions(+), 18 deletions(-)
 create mode 100644 tests/collab-comment-validate.test.ts
 create mode 100644 workers/collab/src/comment-validate.ts

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
new file mode 100644
index 000000000..a08984c27
--- /dev/null
+++ b/tests/collab-comment-validate.test.ts
@@ -0,0 +1,341 @@
+import assert from "node:assert/strict";
+import { describe, it } from "node:test";
+import {
+  MAX_COMMENT_BODY_LENGTH,
+  MAX_COMMENT_AUTHOR_LENGTH,
+  MIN_COMMENT_INTERVAL_MS,
+  validateAnchor,
+  validateAuthor,
+  validateComment,
+  validateReply,
+} from "../workers/collab/src/comment-validate";
+
+// -- constants ----------------------------------------------------------------
+
+describe("comment-validate constants", () => {
+  it("exports expected limits", () => {
+    assert.equal(MAX_COMMENT_BODY_LENGTH, 2000);
+    assert.equal(MAX_COMMENT_AUTHOR_LENGTH, 120);
+    assert.equal(MIN_COMMENT_INTERVAL_MS, 250);
+  });
+});
+
+// -- validateAnchor -----------------------------------------------------------
+
+describe("validateAnchor", () => {
+  it("accepts a valid point anchor", () => {
+    const result = validateAnchor({ type: "point", lngLat: [-122.4, 37.8] });
+    assert.deepEqual(result, { type: "point", lngLat: [-122.4, 37.8] });
+  });
+
+  it("rejects a point anchor with non-finite coordinates", () => {
+    assert.equal(validateAnchor({ type: "point", lngLat: [NaN, 37.8] }), null);
+    assert.equal(validateAnchor({ type: "point", lngLat: [Infinity, 37.8] }), null);
+    assert.equal(validateAnchor({ type: "point", lngLat: [-122.4, -Infinity] }), null);
+  });
+
+  it("rejects a point anchor with wrong lngLat length", () => {
+    assert.equal(validateAnchor({ type: "point", lngLat: [1] }), null);
+    assert.equal(validateAnchor({ type: "point", lngLat: [1, 2, 3] }), null);
+  });
+
+  it("rejects a point anchor with non-number coordinates", () => {
+    assert.equal(validateAnchor({ type: "point", lngLat: ["a", "b"] }), null);
+  });
+
+  it("rejects a point anchor with no lngLat", () => {
+    assert.equal(validateAnchor({ type: "point" }), null);
+  });
+
+  it("accepts a feature anchor with string featureId", () => {
+    const result = validateAnchor({
+      type: "feature",
+      layerId: "layer-1",
+      featureId: "feat-42",
+    });
+    assert.deepEqual(result, {
+      type: "feature",
+      layerId: "layer-1",
+      featureId: "feat-42",
+    });
+  });
+
+  it("accepts a feature anchor with numeric featureId", () => {
+    const result = validateAnchor({
+      type: "feature",
+      layerId: "layer-1",
+      featureId: 42,
+    });
+    assert.deepEqual(result, {
+      type: "feature",
+      layerId: "layer-1",
+      featureId: 42,
+    });
+  });
+
+  it("includes lngLat on a feature anchor when valid", () => {
+    const result = validateAnchor({
+      type: "feature",
+      layerId: "L",
+      featureId: "F",
+      lngLat: [10, 20],
+    });
+    assert.deepEqual(result, {
+      type: "feature",
+      layerId: "L",
+      featureId: "F",
+      lngLat: [10, 20],
+    });
+  });
+
+  it("drops non-finite lngLat on a feature anchor silently", () => {
+    const result = validateAnchor({
+      type: "feature",
+      layerId: "L",
+      featureId: "F",
+      lngLat: [NaN, 20],
+    });
+    assert.deepEqual(result, { type: "feature", layerId: "L", featureId: "F" });
+  });
+
+  it("rejects a feature anchor with empty layerId", () => {
+    assert.equal(validateAnchor({ type: "feature", layerId: "", featureId: "f" }), null);
+  });
+
+  it("rejects a feature anchor with empty string featureId", () => {
+    assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: "" }), null);
+  });
+
+  it("rejects a feature anchor with non-string/non-number featureId", () => {
+    assert.equal(
+      validateAnchor({ type: "feature", layerId: "L", featureId: true }),
+      null,
+    );
+  });
+
+  it("rejects unknown anchor types", () => {
+    assert.equal(validateAnchor({ type: "polygon", coords: [] }), null);
+  });
+
+  it("rejects null / undefined / primitives", () => {
+    assert.equal(validateAnchor(null), null);
+    assert.equal(validateAnchor(undefined), null);
+    assert.equal(validateAnchor("string"), null);
+    assert.equal(validateAnchor(42), null);
+  });
+});
+
+// -- validateAuthor -----------------------------------------------------------
+
+describe("validateAuthor", () => {
+  it("accepts a valid author", () => {
+    const result = validateAuthor({ name: "Alice", color: "#ff0000" });
+    assert.deepEqual(result, { name: "Alice", color: "#ff0000" });
+  });
+
+  it("accepts a 3-digit hex color", () => {
+    const result = validateAuthor({ name: "Bob", color: "#abc" });
+    assert.deepEqual(result, { name: "Bob", color: "#abc" });
+  });
+
+  it("trims the name", () => {
+    const result = validateAuthor({ name: "  Spaced  ", color: "#000" });
+    assert.equal(result?.name, "Spaced");
+  });
+
+  it("truncates a long name", () => {
+    const longName = "X".repeat(200);
+    const result = validateAuthor({ name: longName, color: "#000" });
+    assert.equal(result?.name.length, MAX_COMMENT_AUTHOR_LENGTH);
+  });
+
+  it("rejects an empty name after trimming", () => {
+    assert.equal(validateAuthor({ name: "   ", color: "#000" }), null);
+  });
+
+  it("rejects a non-string name", () => {
+    assert.equal(validateAuthor({ name: 42, color: "#000" }), null);
+  });
+
+  it("rejects an invalid hex color", () => {
+    assert.equal(validateAuthor({ name: "A", color: "red" }), null);
+    assert.equal(validateAuthor({ name: "A", color: "#gggggg" }), null);
+    assert.equal(validateAuthor({ name: "A", color: "#12345" }), null);
+  });
+
+  it("rejects a non-string color", () => {
+    assert.equal(validateAuthor({ name: "A", color: 0xff0000 }), null);
+  });
+
+  it("rejects null / undefined / primitives", () => {
+    assert.equal(validateAuthor(null), null);
+    assert.equal(validateAuthor(undefined), null);
+    assert.equal(validateAuthor("str"), null);
+  });
+});
+
+// -- validateReply ------------------------------------------------------------
+
+describe("validateReply", () => {
+  const validReply = {
+    id: "r-1",
+    author: { name: "Alice", color: "#abc" },
+    body: "Good point",
+    createdAt: "2026-01-01T00:00:00.000Z",
+  };
+
+  it("accepts a valid reply", () => {
+    const result = validateReply(validReply);
+    assert.deepEqual(result, validReply);
+  });
+
+  it("truncates a long body", () => {
+    const result = validateReply({ ...validReply, body: "Z".repeat(3000) });
+    assert.equal(result?.body.length, MAX_COMMENT_BODY_LENGTH);
+  });
+
+  it("rejects a whitespace-only body", () => {
+    assert.equal(validateReply({ ...validReply, body: "   " }), null);
+  });
+
+  it("rejects a non-string body", () => {
+    assert.equal(validateReply({ ...validReply, body: 123 }), null);
+  });
+
+  it("rejects an empty id", () => {
+    assert.equal(validateReply({ ...validReply, id: "" }), null);
+  });
+
+  it("rejects a non-string id", () => {
+    assert.equal(validateReply({ ...validReply, id: 42 }), null);
+  });
+
+  it("rejects an invalid author", () => {
+    assert.equal(validateReply({ ...validReply, author: { name: "", color: "#000" } }), null);
+    assert.equal(validateReply({ ...validReply, author: null }), null);
+  });
+
+  it("falls back to now for missing createdAt", () => {
+    const { createdAt: _, ...noCreated } = validReply;
+    const result = validateReply(noCreated);
+    assert.ok(result);
+    assert.ok(result.createdAt);
+    assert.ok(!isNaN(Date.parse(result.createdAt)));
+  });
+
+  it("rejects null / undefined / primitives", () => {
+    assert.equal(validateReply(null), null);
+    assert.equal(validateReply(undefined), null);
+    assert.equal(validateReply(42), null);
+    assert.equal(validateReply("str"), null);
+  });
+});
+
+// -- validateComment ----------------------------------------------------------
+
+describe("validateComment", () => {
+  const validComment = {
+    id: "c-1",
+    anchor: { type: "point" as const, lngLat: [10, 20] },
+    author: { name: "Alice", color: "#ff0000" },
+    body: "Fix this road segment",
+    createdAt: "2026-06-15T12:00:00.000Z",
+    resolved: false,
+    replies: [],
+  };
+
+  it("accepts a valid comment", () => {
+    const result = validateComment(validComment);
+    assert.deepEqual(result, validComment);
+  });
+
+  it("truncates a long body", () => {
+    const result = validateComment({ ...validComment, body: "B".repeat(3000) });
+    assert.ok(result);
+    assert.equal(result.body.length, MAX_COMMENT_BODY_LENGTH);
+  });
+
+  it("rejects a whitespace-only body", () => {
+    assert.equal(validateComment({ ...validComment, body: "  \n\t  " }), null);
+  });
+
+  it("rejects a non-string body", () => {
+    assert.equal(validateComment({ ...validComment, body: false }), null);
+  });
+
+  it("rejects an empty id", () => {
+    assert.equal(validateComment({ ...validComment, id: "" }), null);
+  });
+
+  it("rejects a non-string id", () => {
+    assert.equal(validateComment({ ...validComment, id: 99 }), null);
+  });
+
+  it("rejects an invalid anchor", () => {
+    assert.equal(validateComment({ ...validComment, anchor: { type: "bad" } }), null);
+    assert.equal(validateComment({ ...validComment, anchor: null }), null);
+  });
+
+  it("rejects an invalid author", () => {
+    assert.equal(
+      validateComment({ ...validComment, author: { name: "X", color: "not-hex" } }),
+      null,
+    );
+  });
+
+  it("coerces resolved to boolean", () => {
+    const result = validateComment({ ...validComment, resolved: 1 });
+    assert.equal(result?.resolved, true);
+    const result2 = validateComment({ ...validComment, resolved: undefined });
+    assert.equal(result2?.resolved, false);
+  });
+
+  it("validates replies within a comment", () => {
+    const withReplies = {
+      ...validComment,
+      replies: [
+        { id: "r-1", author: { name: "Bob", color: "#abc" }, body: "Agreed", createdAt: "2026-06-15T12:30:00.000Z" },
+        { id: "", author: { name: "Bad", color: "#abc" }, body: "nope", createdAt: "x" },
+        null,
+        42,
+      ],
+    };
+    const result = validateComment(withReplies);
+    assert.ok(result);
+    assert.equal(result.replies.length, 1);
+    assert.equal(result.replies[0]?.id, "r-1");
+  });
+
+  it("accepts a comment with a feature anchor", () => {
+    const result = validateComment({
+      ...validComment,
+      anchor: { type: "feature", layerId: "L", featureId: 7, lngLat: [1, 2] },
+    });
+    assert.ok(result);
+    assert.equal(result.anchor.type, "feature");
+  });
+
+  it("falls back to now for missing createdAt", () => {
+    const { createdAt: _, ...noCreated } = validComment;
+    const result = validateComment(noCreated);
+    assert.ok(result);
+    assert.ok(!isNaN(Date.parse(result.createdAt)));
+  });
+
+  it("rejects null / undefined / primitives", () => {
+    assert.equal(validateComment(null), null);
+    assert.equal(validateComment(undefined), null);
+    assert.equal(validateComment("string"), null);
+    assert.equal(validateComment(42), null);
+  });
+
+  it("strips extra fields (returns only known shape)", () => {
+    const result = validateComment({
+      ...validComment,
+      extraField: "should not appear",
+      __proto__: { evil: true },
+    });
+    assert.ok(result);
+    assert.equal((result as Record<string, unknown>).extraField, undefined);
+  });
+});
diff --git a/workers/collab/src/comment-validate.ts b/workers/collab/src/comment-validate.ts
new file mode 100644
index 000000000..831a300ca
--- /dev/null
+++ b/workers/collab/src/comment-validate.ts
@@ -0,0 +1,159 @@
+// Pure validators for comment-mutation payloads. These mirror the
+// `ProjectComment` / `CommentReply` shapes from `@geolibre/core` but operate on
+// untrusted `unknown` input, returning a sanitized object or `null`.
+
+/** Body length cap — matches the chat limit so comments can't store unbounded text. */
+export const MAX_COMMENT_BODY_LENGTH = 2000;
+
+/** Author name length cap — generous for display names but bounded. */
+export const MAX_COMMENT_AUTHOR_LENGTH = 120;
+
+/** Minimum gap between a socket's comment-mutation frames (ms). */
+export const MIN_COMMENT_INTERVAL_MS = 250;
+
+const HEX_COLOR_RE = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
+
+function finite(n: unknown): n is number {
+  return typeof n === "number" && Number.isFinite(n);
+}
+
+// -- anchor -------------------------------------------------------------------
+
+interface PointAnchor {
+  type: "point";
+  lngLat: [number, number];
+}
+
+interface FeatureAnchor {
+  type: "feature";
+  layerId: string;
+  featureId: string | number;
+  lngLat?: [number, number];
+}
+
+export type ValidatedAnchor = PointAnchor | FeatureAnchor;
+
+export function validateAnchor(raw: unknown): ValidatedAnchor | null {
+  if (!raw || typeof raw !== "object") return null;
+  const o = raw as Record<string, unknown>;
+
+  if (o.type === "point") {
+    if (!Array.isArray(o.lngLat) || o.lngLat.length !== 2) return null;
+    const [lng, lat] = o.lngLat;
+    if (!finite(lng) || !finite(lat)) return null;
+    return { type: "point", lngLat: [lng, lat] };
+  }
+
+  if (o.type === "feature") {
+    if (typeof o.layerId !== "string" || !o.layerId) return null;
+    if (typeof o.featureId !== "string" && typeof o.featureId !== "number") return null;
+    if (typeof o.featureId === "string" && !o.featureId) return null;
+    const anchor: FeatureAnchor = {
+      type: "feature",
+      layerId: o.layerId,
+      featureId: o.featureId,
+    };
+    if (Array.isArray(o.lngLat) && o.lngLat.length === 2) {
+      const [lng, lat] = o.lngLat;
+      if (finite(lng) && finite(lat)) {
+        anchor.lngLat = [lng, lat];
+      }
+    }
+    return anchor;
+  }
+
+  return null;
+}
+
+// -- author -------------------------------------------------------------------
+
+export interface ValidatedAuthor {
+  name: string;
+  color: string;
+}
+
+export function validateAuthor(raw: unknown): ValidatedAuthor | null {
+  if (!raw || typeof raw !== "object") return null;
+  const o = raw as Record<string, unknown>;
+  if (typeof o.name !== "string") return null;
+  const name = o.name.trim().slice(0, MAX_COMMENT_AUTHOR_LENGTH);
+  if (!name) return null;
+  if (typeof o.color !== "string" || !HEX_COLOR_RE.test(o.color)) return null;
+  return { name, color: o.color };
+}
+
+// -- comment ------------------------------------------------------------------
+
+export interface ValidatedComment {
+  id: string;
+  anchor: ValidatedAnchor;
+  author: ValidatedAuthor;
+  body: string;
+  createdAt: string;
+  resolved: boolean;
+  replies: ValidatedReply[];
+}
+
+export function validateComment(raw: unknown): ValidatedComment | null {
+  if (!raw || typeof raw !== "object") return null;
+  const o = raw as Record<string, unknown>;
+
+  if (typeof o.id !== "string" || !o.id) return null;
+
+  const anchor = validateAnchor(o.anchor);
+  if (!anchor) return null;
+
+  const author = validateAuthor(o.author);
+  if (!author) return null;
+
+  if (typeof o.body !== "string") return null;
+  const body = o.body.slice(0, MAX_COMMENT_BODY_LENGTH);
+  if (!body.trim()) return null;
+
+  const createdAt = typeof o.createdAt === "string" ? o.createdAt : new Date().toISOString();
+
+  const replies: ValidatedReply[] = [];
+  if (Array.isArray(o.replies)) {
+    for (const r of o.replies) {
+      const validated = validateReply(r);
+      if (validated) replies.push(validated);
+    }
+  }
+
+  return {
+    id: o.id,
+    anchor,
+    author,
+    body,
+    createdAt,
+    resolved: Boolean(o.resolved),
+    replies,
+  };
+}
+
+// -- reply --------------------------------------------------------------------
+
+export interface ValidatedReply {
+  id: string;
+  author: ValidatedAuthor;
+  body: string;
+  createdAt: string;
+}
+
+export function validateReply(raw: unknown): ValidatedReply | null {
+  if (!raw || typeof raw !== "object") return null;
+  const o = raw as Record<string, unknown>;
+
+  if (typeof o.id !== "string" || !o.id) return null;
+
+  const author = validateAuthor(o.author);
+  if (!author) return null;
+
+  if (typeof o.body !== "string") return null;
+  const body = o.body.slice(0, MAX_COMMENT_BODY_LENGTH);
+  if (!body.trim()) return null;
+
+  const createdAt = typeof o.createdAt === "string" ? o.createdAt : new Date().toISOString();
+
+  return { id: o.id, author, body, createdAt };
+}
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index b32b1fbdd..45a5cb973 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -10,6 +10,11 @@ import type {
   PresenceEntry,
   ServerMessage,
 } from "./protocol";
+import {
+  MIN_COMMENT_INTERVAL_MS,
+  validateComment,
+  validateReply,
+} from "./comment-validate";
 
 function finite(n: unknown): n is number {
   return typeof n === "number" && Number.isFinite(n);
@@ -107,6 +112,8 @@ interface SocketAttachment {
   editOverride?: boolean;
   /** Epoch-ms of this socket's last accepted chat frame, for rate-limiting. */
   lastChatTs?: number;
+  /** Epoch-ms of this socket's last accepted comment-mutation frame. */
+  lastCommentTs?: number;
 }
 
 /** Effective edit permission: the host always edits; otherwise a host-set
@@ -671,6 +678,79 @@ export class CollabSession extends DurableObject<Env> {
       return;
     }
 
+    // Rate-limit: same pattern as chat to prevent storage-op exhaustion.
+    const now = Date.now();
+    if (
+      attachment.lastCommentTs !== undefined &&
+      now - attachment.lastCommentTs < MIN_COMMENT_INTERVAL_MS
+    ) {
+      return;
+    }
+    attachment.lastCommentTs = now;
+    ws.serializeAttachment(attachment);
+
+    const action = message.action;
+    if (!action || typeof action !== "object") {
+      this.send(ws, {
+        type: "error",
+        code: "bad-message",
+        message: "Missing or invalid comment-mutation action.",
+      });
+      return;
+    }
+
+    // Validate payloads for add/reply; toggle-resolve and delete only need a
+    // string commentId and carry no untrusted object bodies.
+    let sanitizedAction = action;
+    if (action.type === "add") {
+      const validated = validateComment(action.comment);
+      if (!validated) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Invalid comment payload.",
+        });
+        return;
+      }
+      sanitizedAction = { type: "add", comment: validated };
+    } else if (action.type === "reply") {
+      if (typeof action.commentId !== "string" || !action.commentId) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Invalid reply target.",
+        });
+        return;
+      }
+      const validated = validateReply(action.reply);
+      if (!validated) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Invalid reply payload.",
+        });
+        return;
+      }
+      sanitizedAction = { type: "reply", commentId: action.commentId, reply: validated };
+    } else if (action.type === "toggle-resolve") {
+      if (typeof action.commentId !== "string" || !action.commentId) return;
+      sanitizedAction = {
+        type: "toggle-resolve",
+        commentId: action.commentId,
+        ...(action.resolved !== undefined ? { resolved: action.resolved === true } : {}),
+      };
+    } else if (action.type === "delete") {
+      if (typeof action.commentId !== "string" || !action.commentId) return;
+      sanitizedAction = { type: "delete", commentId: action.commentId };
+    } else {
+      return;
+    }
+
+    const sanitizedMessage: Extract<ClientMessage, { type: "comment-mutation" }> = {
+      type: "comment-mutation",
+      action: sanitizedAction,
+    };
+
     const rawSnapshot = await this.ctx.storage.get<string>("snapshot");
     if (rawSnapshot) {
       try {
@@ -678,18 +758,14 @@ export class CollabSession extends DurableObject<Env> {
         const comments = Array.isArray(parsed.comments)
           ? (parsed.comments as Record<string, unknown>[])
           : [];
-        const action = message.action;
-        if (!action || typeof action !== "object") return;
         let updatedComments = comments;
 
-        if (action.type === "add") {
-          if (!action.comment || typeof action.comment !== "object") return;
-          updatedComments = [...comments, action.comment as Record<string, unknown>];
-        } else if (action.type === "reply") {
-          if (!action.reply || typeof action.reply !== "object") return;
-          const replyObj = action.reply as Record<string, unknown>;
+        if (sanitizedAction.type === "add") {
+          updatedComments = [...comments, sanitizedAction.comment as Record<string, unknown>];
+        } else if (sanitizedAction.type === "reply") {
+          const replyObj = sanitizedAction.reply as Record<string, unknown>;
           updatedComments = comments.map((c) => {
-            if (!c || typeof c !== "object" || c.id !== action.commentId) return c;
+            if (!c || typeof c !== "object" || c.id !== sanitizedAction.commentId) return c;
             const existingReplies = Array.isArray(c.replies)
               ? (c.replies as Record<string, unknown>[])
               : [];
@@ -697,27 +773,32 @@ export class CollabSession extends DurableObject<Env> {
               return c;
             return { ...c, replies: [...existingReplies, replyObj] };
           });
-        } else if (action.type === "toggle-resolve") {
+        } else if (sanitizedAction.type === "toggle-resolve") {
           updatedComments = comments.map((c) =>
-            c && typeof c === "object" && c.id === action.commentId
-              ? { ...c, resolved: action.resolved !== undefined ? action.resolved : !c.resolved }
+            c && typeof c === "object" && c.id === sanitizedAction.commentId
+              ? {
+                  ...c,
+                  resolved:
+                    sanitizedAction.resolved !== undefined
+                      ? sanitizedAction.resolved
+                      : !c.resolved,
+                }
               : c,
           );
-        } else if (action.type === "delete") {
+        } else if (sanitizedAction.type === "delete") {
           updatedComments = comments.filter(
-            (c) => c && typeof c === "object" && c.id !== action.commentId,
+            (c) => c && typeof c === "object" && c.id !== sanitizedAction.commentId,
           );
         }
 
         parsed.comments = updatedComments;
         await this.ctx.storage.put("snapshot", JSON.stringify(parsed));
       } catch {
-        // Ignore snapshot mutation update errors defensively
+        // Snapshot mutation failed; still fan out the validated action below so
+        // peers stay in sync (the next full snapshot will reconcile storage).
       }
     }
 
-    // Exclude the sender (ws) so they don't receive their own mutation back.
-    // The sender already applied the change locally before calling sendCommentMutation.
-    this.broadcast(message, ws);
+    this.broadcast(sanitizedMessage, ws);
   }
 }

From 556ac09687c09b28b0ac9e8e5e2e657e7d19e521 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Sat, 1 Aug 2026 19:25:05 +0000
Subject: [PATCH 2/9] style: auto-format (ruff + oxfmt) [pre-commit.ci]

---
 tests/collab-comment-validate.test.ts | 12 +++++++-----
 workers/collab/src/session.ts         | 10 ++--------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index a08984c27..1965d61d5 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -107,10 +107,7 @@ describe("validateAnchor", () => {
   });
 
   it("rejects a feature anchor with non-string/non-number featureId", () => {
-    assert.equal(
-      validateAnchor({ type: "feature", layerId: "L", featureId: true }),
-      null,
-    );
+    assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: true }), null);
   });
 
   it("rejects unknown anchor types", () => {
@@ -294,7 +291,12 @@ describe("validateComment", () => {
     const withReplies = {
       ...validComment,
       replies: [
-        { id: "r-1", author: { name: "Bob", color: "#abc" }, body: "Agreed", createdAt: "2026-06-15T12:30:00.000Z" },
+        {
+          id: "r-1",
+          author: { name: "Bob", color: "#abc" },
+          body: "Agreed",
+          createdAt: "2026-06-15T12:30:00.000Z",
+        },
         { id: "", author: { name: "Bad", color: "#abc" }, body: "nope", createdAt: "x" },
         null,
         42,
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index 45a5cb973..52d0321da 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -10,11 +10,7 @@ import type {
   PresenceEntry,
   ServerMessage,
 } from "./protocol";
-import {
-  MIN_COMMENT_INTERVAL_MS,
-  validateComment,
-  validateReply,
-} from "./comment-validate";
+import { MIN_COMMENT_INTERVAL_MS, validateComment, validateReply } from "./comment-validate";
 
 function finite(n: unknown): n is number {
   return typeof n === "number" && Number.isFinite(n);
@@ -779,9 +775,7 @@ export class CollabSession extends DurableObject<Env> {
               ? {
                   ...c,
                   resolved:
-                    sanitizedAction.resolved !== undefined
-                      ? sanitizedAction.resolved
-                      : !c.resolved,
+                    sanitizedAction.resolved !== undefined ? sanitizedAction.resolved : !c.resolved,
                 }
               : c,
           );

From 52a926c3e7fa3bf0e1c0793da592438daccf909b Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Sun, 2 Aug 2026 04:23:59 +0530
Subject: [PATCH 3/9] fix: cap replies, validate createdAt, send bad-message
 for invalid targets

- Add MAX_REPLIES_PER_COMMENT (100) to bound reply arrays
- Reject NaN/Infinity numeric featureIds in anchor validation
- Validate createdAt is parseable (fall back to now if not)
- Send bad-message error for invalid toggle-resolve/delete targets
- Deduplicate add action by comment id (matches reply branch)
- Add regression tests for all new validations
---
 tests/collab-comment-validate.test.ts  | 35 ++++++++++++++++++++++++++
 workers/collab/src/comment-validate.ts | 15 +++++++++--
 workers/collab/src/session.ts          | 24 +++++++++++++++---
 3 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index 1965d61d5..7ba141140 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -3,6 +3,7 @@ import { describe, it } from "node:test";
 import {
   MAX_COMMENT_BODY_LENGTH,
   MAX_COMMENT_AUTHOR_LENGTH,
+  MAX_REPLIES_PER_COMMENT,
   MIN_COMMENT_INTERVAL_MS,
   validateAnchor,
   validateAuthor,
@@ -110,6 +111,14 @@ describe("validateAnchor", () => {
     assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: true }), null);
   });
 
+  it("rejects a feature anchor with NaN featureId", () => {
+    assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: NaN }), null);
+  });
+
+  it("rejects a feature anchor with Infinity featureId", () => {
+    assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: Infinity }), null);
+  });
+
   it("rejects unknown anchor types", () => {
     assert.equal(validateAnchor({ type: "polygon", coords: [] }), null);
   });
@@ -220,6 +229,13 @@ describe("validateReply", () => {
     assert.ok(!isNaN(Date.parse(result.createdAt)));
   });
 
+  it("falls back to now for unparseable createdAt", () => {
+    const result = validateReply({ ...validReply, createdAt: "not-a-date" });
+    assert.ok(result);
+    assert.notEqual(result.createdAt, "not-a-date");
+    assert.ok(!isNaN(Date.parse(result.createdAt)));
+  });
+
   it("rejects null / undefined / primitives", () => {
     assert.equal(validateReply(null), null);
     assert.equal(validateReply(undefined), null);
@@ -331,6 +347,25 @@ describe("validateComment", () => {
     assert.equal(validateComment(42), null);
   });
 
+  it("caps replies at MAX_REPLIES_PER_COMMENT", () => {
+    const manyReplies = Array.from({ length: MAX_REPLIES_PER_COMMENT + 20 }, (_, i) => ({
+      id: `r-${i}`,
+      author: { name: "Bob", color: "#abc" },
+      body: `Reply ${i}`,
+      createdAt: "2026-06-15T12:30:00.000Z",
+    }));
+    const result = validateComment({ ...validComment, replies: manyReplies });
+    assert.ok(result);
+    assert.equal(result.replies.length, MAX_REPLIES_PER_COMMENT);
+  });
+
+  it("falls back to now for unparseable createdAt", () => {
+    const result = validateComment({ ...validComment, createdAt: "garbage" });
+    assert.ok(result);
+    assert.notEqual(result.createdAt, "garbage");
+    assert.ok(!isNaN(Date.parse(result.createdAt)));
+  });
+
   it("strips extra fields (returns only known shape)", () => {
     const result = validateComment({
       ...validComment,
diff --git a/workers/collab/src/comment-validate.ts b/workers/collab/src/comment-validate.ts
index 831a300ca..207985fe7 100644
--- a/workers/collab/src/comment-validate.ts
+++ b/workers/collab/src/comment-validate.ts
@@ -11,6 +11,9 @@ export const MAX_COMMENT_AUTHOR_LENGTH = 120;
 /** Minimum gap between a socket's comment-mutation frames (ms). */
 export const MIN_COMMENT_INTERVAL_MS = 250;
 
+/** Maximum number of replies stored per comment. */
+export const MAX_REPLIES_PER_COMMENT = 100;
+
 const HEX_COLOR_RE = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
 
 function finite(n: unknown): n is number {
@@ -48,6 +51,7 @@ export function validateAnchor(raw: unknown): ValidatedAnchor | null {
     if (typeof o.layerId !== "string" || !o.layerId) return null;
     if (typeof o.featureId !== "string" && typeof o.featureId !== "number") return null;
     if (typeof o.featureId === "string" && !o.featureId) return null;
+    if (typeof o.featureId === "number" && !finite(o.featureId)) return null;
     const anchor: FeatureAnchor = {
       type: "feature",
       layerId: o.layerId,
@@ -110,11 +114,15 @@ export function validateComment(raw: unknown): ValidatedComment | null {
   const body = o.body.slice(0, MAX_COMMENT_BODY_LENGTH);
   if (!body.trim()) return null;
 
-  const createdAt = typeof o.createdAt === "string" ? o.createdAt : new Date().toISOString();
+  const createdAt =
+    typeof o.createdAt === "string" && !Number.isNaN(Date.parse(o.createdAt))
+      ? o.createdAt
+      : new Date().toISOString();
 
   const replies: ValidatedReply[] = [];
   if (Array.isArray(o.replies)) {
     for (const r of o.replies) {
+      if (replies.length >= MAX_REPLIES_PER_COMMENT) break;
       const validated = validateReply(r);
       if (validated) replies.push(validated);
     }
@@ -153,7 +161,10 @@ export function validateReply(raw: unknown): ValidatedReply | null {
   const body = o.body.slice(0, MAX_COMMENT_BODY_LENGTH);
   if (!body.trim()) return null;
 
-  const createdAt = typeof o.createdAt === "string" ? o.createdAt : new Date().toISOString();
+  const createdAt =
+    typeof o.createdAt === "string" && !Number.isNaN(Date.parse(o.createdAt))
+      ? o.createdAt
+      : new Date().toISOString();
 
   return { id: o.id, author, body, createdAt };
 }
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index 52d0321da..e326fb3f0 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -729,14 +729,28 @@ export class CollabSession extends DurableObject<Env> {
       }
       sanitizedAction = { type: "reply", commentId: action.commentId, reply: validated };
     } else if (action.type === "toggle-resolve") {
-      if (typeof action.commentId !== "string" || !action.commentId) return;
+      if (typeof action.commentId !== "string" || !action.commentId) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Invalid toggle-resolve target.",
+        });
+        return;
+      }
       sanitizedAction = {
         type: "toggle-resolve",
         commentId: action.commentId,
         ...(action.resolved !== undefined ? { resolved: action.resolved === true } : {}),
       };
     } else if (action.type === "delete") {
-      if (typeof action.commentId !== "string" || !action.commentId) return;
+      if (typeof action.commentId !== "string" || !action.commentId) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Invalid delete target.",
+        });
+        return;
+      }
       sanitizedAction = { type: "delete", commentId: action.commentId };
     } else {
       return;
@@ -757,7 +771,11 @@ export class CollabSession extends DurableObject<Env> {
         let updatedComments = comments;
 
         if (sanitizedAction.type === "add") {
-          updatedComments = [...comments, sanitizedAction.comment as Record<string, unknown>];
+          const newComment = sanitizedAction.comment as Record<string, unknown>;
+          const exists = comments.some(
+            (c) => c && typeof c === "object" && c.id === newComment.id,
+          );
+          updatedComments = exists ? comments : [...comments, newComment];
         } else if (sanitizedAction.type === "reply") {
           const replyObj = sanitizedAction.reply as Record<string, unknown>;
           updatedComments = comments.map((c) => {

From 6cbc6f5176731b8669194d8cef0384029ce0a487 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Sat, 1 Aug 2026 22:55:48 +0000
Subject: [PATCH 4/9] style: auto-format (ruff + oxfmt) [pre-commit.ci]

---
 workers/collab/src/session.ts | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index e326fb3f0..f04ba4035 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -772,9 +772,7 @@ export class CollabSession extends DurableObject<Env> {
 
         if (sanitizedAction.type === "add") {
           const newComment = sanitizedAction.comment as Record<string, unknown>;
-          const exists = comments.some(
-            (c) => c && typeof c === "object" && c.id === newComment.id,
-          );
+          const exists = comments.some((c) => c && typeof c === "object" && c.id === newComment.id);
           updatedComments = exists ? comments : [...comments, newComment];
         } else if (sanitizedAction.type === "reply") {
           const replyObj = sanitizedAction.reply as Record<string, unknown>;

From ac6e8450f8e8da8a47938da0a968cbb36090cce6 Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Sun, 2 Aug 2026 04:42:19 +0530
Subject: [PATCH 5/9] fix: slice reply input before looping, cap replies on
 append, reject unknown action types

- validateComment now slices o.replies to MAX_REPLIES_PER_COMMENT before
  iterating, capping inspected input length instead of only valid count.
- handleCommentMutation reply path checks target.replies.length against
  MAX_REPLIES_PER_COMMENT before appending; returns bad-message if full.
- Unsupported comment-mutation action types now return bad-message instead
  of silently dropping.
- Added validator test proving entries past the slice boundary are ignored.
---
 tests/collab-comment-validate.test.ts  | 18 +++++++++++++++++
 workers/collab/src/comment-validate.ts |  3 +--
 workers/collab/src/session.ts          | 28 +++++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index 7ba141140..bc5e0024c 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -359,6 +359,24 @@ describe("validateComment", () => {
     assert.equal(result.replies.length, MAX_REPLIES_PER_COMMENT);
   });
 
+  it("only inspects the first MAX_REPLIES_PER_COMMENT entries", () => {
+    const invalid = Array.from({ length: MAX_REPLIES_PER_COMMENT }, (_, i) => ({
+      id: `bad-${i}`,
+      author: { name: "", color: "#abc" },
+      body: `Reply ${i}`,
+      createdAt: "2026-06-15T12:30:00.000Z",
+    }));
+    const valid = Array.from({ length: 5 }, (_, i) => ({
+      id: `good-${i}`,
+      author: { name: "Alice", color: "#abc" },
+      body: `Reply ${i}`,
+      createdAt: "2026-06-15T12:30:00.000Z",
+    }));
+    const result = validateComment({ ...validComment, replies: [...invalid, ...valid] });
+    assert.ok(result);
+    assert.equal(result.replies.length, 0);
+  });
+
   it("falls back to now for unparseable createdAt", () => {
     const result = validateComment({ ...validComment, createdAt: "garbage" });
     assert.ok(result);
diff --git a/workers/collab/src/comment-validate.ts b/workers/collab/src/comment-validate.ts
index 207985fe7..90bef8a7b 100644
--- a/workers/collab/src/comment-validate.ts
+++ b/workers/collab/src/comment-validate.ts
@@ -121,8 +121,7 @@ export function validateComment(raw: unknown): ValidatedComment | null {
 
   const replies: ValidatedReply[] = [];
   if (Array.isArray(o.replies)) {
-    for (const r of o.replies) {
-      if (replies.length >= MAX_REPLIES_PER_COMMENT) break;
+    for (const r of o.replies.slice(0, MAX_REPLIES_PER_COMMENT)) {
       const validated = validateReply(r);
       if (validated) replies.push(validated);
     }
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index f04ba4035..bd7635c2b 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -10,7 +10,12 @@ import type {
   PresenceEntry,
   ServerMessage,
 } from "./protocol";
-import { MIN_COMMENT_INTERVAL_MS, validateComment, validateReply } from "./comment-validate";
+import {
+  MAX_REPLIES_PER_COMMENT,
+  MIN_COMMENT_INTERVAL_MS,
+  validateComment,
+  validateReply,
+} from "./comment-validate";
 
 function finite(n: unknown): n is number {
   return typeof n === "number" && Number.isFinite(n);
@@ -753,6 +758,11 @@ export class CollabSession extends DurableObject<Env> {
       }
       sanitizedAction = { type: "delete", commentId: action.commentId };
     } else {
+      this.send(ws, {
+        type: "error",
+        code: "bad-message",
+        message: "Unsupported comment-mutation action type.",
+      });
       return;
     }
 
@@ -776,6 +786,22 @@ export class CollabSession extends DurableObject<Env> {
           updatedComments = exists ? comments : [...comments, newComment];
         } else if (sanitizedAction.type === "reply") {
           const replyObj = sanitizedAction.reply as Record<string, unknown>;
+          const target = comments.find(
+            (c) => c && typeof c === "object" && c.id === sanitizedAction.commentId,
+          );
+          if (target) {
+            const targetReplies = Array.isArray(target.replies)
+              ? (target.replies as Record<string, unknown>[])
+              : [];
+            if (targetReplies.length >= MAX_REPLIES_PER_COMMENT) {
+              this.send(ws, {
+                type: "error",
+                code: "bad-message",
+                message: "Reply limit reached for this comment.",
+              });
+              return;
+            }
+          }
           updatedComments = comments.map((c) => {
             if (!c || typeof c !== "object" || c.id !== sanitizedAction.commentId) return c;
             const existingReplies = Array.isArray(c.replies)

From e45e87b41720ef5593db96d3371b19927b23c51d Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Sun, 2 Aug 2026 21:28:07 +0530
Subject: [PATCH 6/9] fix(collab): validate comment mutations before
 rate-limiting

Invalid payloads always get bad-message and no longer consume the
per-socket interval or block a following valid mutation.
---
 workers/collab/src/session.ts | 46 ++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index bd7635c2b..e30ed8bad 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -669,27 +669,6 @@ export class CollabSession extends DurableObject<Env> {
     attachment: SocketAttachment,
     message: Extract<ClientMessage, { type: "comment-mutation" }>,
   ): Promise<void> {
-    const mode = (await this.ctx.storage.get<CollaborationMode>("mode")) ?? "co-edit";
-    if (!canEdit(attachment, mode)) {
-      this.send(ws, {
-        type: "error",
-        code: "forbidden",
-        message: "You are in view-only mode and cannot comment.",
-      });
-      return;
-    }
-
-    // Rate-limit: same pattern as chat to prevent storage-op exhaustion.
-    const now = Date.now();
-    if (
-      attachment.lastCommentTs !== undefined &&
-      now - attachment.lastCommentTs < MIN_COMMENT_INTERVAL_MS
-    ) {
-      return;
-    }
-    attachment.lastCommentTs = now;
-    ws.serializeAttachment(attachment);
-
     const action = message.action;
     if (!action || typeof action !== "object") {
       this.send(ws, {
@@ -701,7 +680,9 @@ export class CollabSession extends DurableObject<Env> {
     }
 
     // Validate payloads for add/reply; toggle-resolve and delete only need a
-    // string commentId and carry no untrusted object bodies.
+    // string commentId and carry no untrusted object bodies. Do this before
+    // rate-limiting so invalid frames always get bad-message and never consume
+    // the per-socket interval.
     let sanitizedAction = action;
     if (action.type === "add") {
       const validated = validateComment(action.comment);
@@ -766,6 +747,27 @@ export class CollabSession extends DurableObject<Env> {
       return;
     }
 
+    const mode = (await this.ctx.storage.get<CollaborationMode>("mode")) ?? "co-edit";
+    if (!canEdit(attachment, mode)) {
+      this.send(ws, {
+        type: "error",
+        code: "forbidden",
+        message: "You are in view-only mode and cannot comment.",
+      });
+      return;
+    }
+
+    // Rate-limit accepted, authorized mutations before storage work.
+    const now = Date.now();
+    if (
+      attachment.lastCommentTs !== undefined &&
+      now - attachment.lastCommentTs < MIN_COMMENT_INTERVAL_MS
+    ) {
+      return;
+    }
+    attachment.lastCommentTs = now;
+    ws.serializeAttachment(attachment);
+
     const sanitizedMessage: Extract<ClientMessage, { type: "comment-mutation" }> = {
       type: "comment-mutation",
       action: sanitizedAction,

From 803d8a9e6874c5b845eaee3e8902e2ca830c0a7c Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Sun, 2 Aug 2026 21:41:14 +0530
Subject: [PATCH 7/9] fix(collab): persist early comments and bound identifier
 lengths

Seed an empty snapshot when none exists so comment mutations are stored
before the first full project sync, reject replies to missing comments,
and cap id/layerId/featureId lengths at 200 characters.
---
 tests/collab-comment-validate.test.ts  |  19 ++++
 workers/collab/src/comment-validate.ts |  16 ++-
 workers/collab/src/session.ts          | 135 ++++++++++++++-----------
 3 files changed, 108 insertions(+), 62 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index bc5e0024c..3d15674c1 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -3,6 +3,7 @@ import { describe, it } from "node:test";
 import {
   MAX_COMMENT_BODY_LENGTH,
   MAX_COMMENT_AUTHOR_LENGTH,
+  MAX_ID_LENGTH,
   MAX_REPLIES_PER_COMMENT,
   MIN_COMMENT_INTERVAL_MS,
   validateAnchor,
@@ -17,6 +18,7 @@ describe("comment-validate constants", () => {
   it("exports expected limits", () => {
     assert.equal(MAX_COMMENT_BODY_LENGTH, 2000);
     assert.equal(MAX_COMMENT_AUTHOR_LENGTH, 120);
+    assert.equal(MAX_ID_LENGTH, 200);
     assert.equal(MIN_COMMENT_INTERVAL_MS, 250);
   });
 });
@@ -119,6 +121,12 @@ describe("validateAnchor", () => {
     assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: Infinity }), null);
   });
 
+  it("rejects a feature anchor with oversized layerId or featureId", () => {
+    const long = "x".repeat(MAX_ID_LENGTH + 1);
+    assert.equal(validateAnchor({ type: "feature", layerId: long, featureId: "f" }), null);
+    assert.equal(validateAnchor({ type: "feature", layerId: "L", featureId: long }), null);
+  });
+
   it("rejects unknown anchor types", () => {
     assert.equal(validateAnchor({ type: "polygon", coords: [] }), null);
   });
@@ -216,6 +224,10 @@ describe("validateReply", () => {
     assert.equal(validateReply({ ...validReply, id: 42 }), null);
   });
 
+  it("rejects an oversized id", () => {
+    assert.equal(validateReply({ ...validReply, id: "r".repeat(MAX_ID_LENGTH + 1) }), null);
+  });
+
   it("rejects an invalid author", () => {
     assert.equal(validateReply({ ...validReply, author: { name: "", color: "#000" } }), null);
     assert.equal(validateReply({ ...validReply, author: null }), null);
@@ -284,6 +296,13 @@ describe("validateComment", () => {
     assert.equal(validateComment({ ...validComment, id: 99 }), null);
   });
 
+  it("rejects an oversized id", () => {
+    assert.equal(
+      validateComment({ ...validComment, id: "c".repeat(MAX_ID_LENGTH + 1) }),
+      null,
+    );
+  });
+
   it("rejects an invalid anchor", () => {
     assert.equal(validateComment({ ...validComment, anchor: { type: "bad" } }), null);
     assert.equal(validateComment({ ...validComment, anchor: null }), null);
diff --git a/workers/collab/src/comment-validate.ts b/workers/collab/src/comment-validate.ts
index 90bef8a7b..a96e1c083 100644
--- a/workers/collab/src/comment-validate.ts
+++ b/workers/collab/src/comment-validate.ts
@@ -8,12 +8,20 @@ export const MAX_COMMENT_BODY_LENGTH = 2000;
 /** Author name length cap — generous for display names but bounded. */
 export const MAX_COMMENT_AUTHOR_LENGTH = 120;
 
+/** Identifier length cap for comment/reply ids, layerId, and string featureId. */
+export const MAX_ID_LENGTH = 200;
+
 /** Minimum gap between a socket's comment-mutation frames (ms). */
 export const MIN_COMMENT_INTERVAL_MS = 250;
 
 /** Maximum number of replies stored per comment. */
 export const MAX_REPLIES_PER_COMMENT = 100;
 
+/** True when `value` is a non-empty string within {@link MAX_ID_LENGTH}. */
+export function isBoundedId(value: unknown): value is string {
+  return typeof value === "string" && value.length > 0 && value.length <= MAX_ID_LENGTH;
+}
+
 const HEX_COLOR_RE = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
 
 function finite(n: unknown): n is number {
@@ -48,9 +56,9 @@ export function validateAnchor(raw: unknown): ValidatedAnchor | null {
   }
 
   if (o.type === "feature") {
-    if (typeof o.layerId !== "string" || !o.layerId) return null;
+    if (!isBoundedId(o.layerId)) return null;
     if (typeof o.featureId !== "string" && typeof o.featureId !== "number") return null;
-    if (typeof o.featureId === "string" && !o.featureId) return null;
+    if (typeof o.featureId === "string" && !isBoundedId(o.featureId)) return null;
     if (typeof o.featureId === "number" && !finite(o.featureId)) return null;
     const anchor: FeatureAnchor = {
       type: "feature",
@@ -102,7 +110,7 @@ export function validateComment(raw: unknown): ValidatedComment | null {
   if (!raw || typeof raw !== "object") return null;
   const o = raw as Record<string, unknown>;
 
-  if (typeof o.id !== "string" || !o.id) return null;
+  if (!isBoundedId(o.id)) return null;
 
   const anchor = validateAnchor(o.anchor);
   if (!anchor) return null;
@@ -151,7 +159,7 @@ export function validateReply(raw: unknown): ValidatedReply | null {
   if (!raw || typeof raw !== "object") return null;
   const o = raw as Record<string, unknown>;
 
-  if (typeof o.id !== "string" || !o.id) return null;
+  if (!isBoundedId(o.id)) return null;
 
   const author = validateAuthor(o.author);
   if (!author) return null;
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index e30ed8bad..920c4a0fb 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -11,6 +11,7 @@ import type {
   ServerMessage,
 } from "./protocol";
 import {
+  isBoundedId,
   MAX_REPLIES_PER_COMMENT,
   MIN_COMMENT_INTERVAL_MS,
   validateComment,
@@ -696,7 +697,7 @@ export class CollabSession extends DurableObject<Env> {
       }
       sanitizedAction = { type: "add", comment: validated };
     } else if (action.type === "reply") {
-      if (typeof action.commentId !== "string" || !action.commentId) {
+      if (!isBoundedId(action.commentId)) {
         this.send(ws, {
           type: "error",
           code: "bad-message",
@@ -715,7 +716,7 @@ export class CollabSession extends DurableObject<Env> {
       }
       sanitizedAction = { type: "reply", commentId: action.commentId, reply: validated };
     } else if (action.type === "toggle-resolve") {
-      if (typeof action.commentId !== "string" || !action.commentId) {
+      if (!isBoundedId(action.commentId)) {
         this.send(ws, {
           type: "error",
           code: "bad-message",
@@ -729,7 +730,7 @@ export class CollabSession extends DurableObject<Env> {
         ...(action.resolved !== undefined ? { resolved: action.resolved === true } : {}),
       };
     } else if (action.type === "delete") {
-      if (typeof action.commentId !== "string" || !action.commentId) {
+      if (!isBoundedId(action.commentId)) {
         this.send(ws, {
           type: "error",
           code: "bad-message",
@@ -773,68 +774,86 @@ export class CollabSession extends DurableObject<Env> {
       action: sanitizedAction,
     };
 
+    // Persist even when no full project snapshot has been written yet, so early
+    // comments survive late joiners / reconnects. Seed an empty object when
+    // storage is empty or corrupt — the relay never inspects other project fields.
     const rawSnapshot = await this.ctx.storage.get<string>("snapshot");
+    let parsed: Record<string, unknown> = { comments: [] };
     if (rawSnapshot) {
       try {
-        const parsed = JSON.parse(rawSnapshot) as Record<string, unknown>;
-        const comments = Array.isArray(parsed.comments)
-          ? (parsed.comments as Record<string, unknown>[])
+        const value = JSON.parse(rawSnapshot) as unknown;
+        if (value && typeof value === "object" && !Array.isArray(value)) {
+          parsed = value as Record<string, unknown>;
+        }
+      } catch {
+        // Fall through with the empty seed; still fan out below.
+      }
+    }
+
+    try {
+      const comments = Array.isArray(parsed.comments)
+        ? (parsed.comments as Record<string, unknown>[])
+        : [];
+      let updatedComments = comments;
+
+      if (sanitizedAction.type === "add") {
+        const newComment = sanitizedAction.comment as Record<string, unknown>;
+        const exists = comments.some((c) => c && typeof c === "object" && c.id === newComment.id);
+        updatedComments = exists ? comments : [...comments, newComment];
+      } else if (sanitizedAction.type === "reply") {
+        const replyObj = sanitizedAction.reply as Record<string, unknown>;
+        const target = comments.find(
+          (c) => c && typeof c === "object" && c.id === sanitizedAction.commentId,
+        );
+        if (!target) {
+          this.send(ws, {
+            type: "error",
+            code: "bad-message",
+            message: "Invalid reply target.",
+          });
+          return;
+        }
+        const targetReplies = Array.isArray(target.replies)
+          ? (target.replies as Record<string, unknown>[])
           : [];
-        let updatedComments = comments;
-
-        if (sanitizedAction.type === "add") {
-          const newComment = sanitizedAction.comment as Record<string, unknown>;
-          const exists = comments.some((c) => c && typeof c === "object" && c.id === newComment.id);
-          updatedComments = exists ? comments : [...comments, newComment];
-        } else if (sanitizedAction.type === "reply") {
-          const replyObj = sanitizedAction.reply as Record<string, unknown>;
-          const target = comments.find(
-            (c) => c && typeof c === "object" && c.id === sanitizedAction.commentId,
-          );
-          if (target) {
-            const targetReplies = Array.isArray(target.replies)
-              ? (target.replies as Record<string, unknown>[])
-              : [];
-            if (targetReplies.length >= MAX_REPLIES_PER_COMMENT) {
-              this.send(ws, {
-                type: "error",
-                code: "bad-message",
-                message: "Reply limit reached for this comment.",
-              });
-              return;
-            }
-          }
-          updatedComments = comments.map((c) => {
-            if (!c || typeof c !== "object" || c.id !== sanitizedAction.commentId) return c;
-            const existingReplies = Array.isArray(c.replies)
-              ? (c.replies as Record<string, unknown>[])
-              : [];
-            if (existingReplies.some((r) => r && typeof r === "object" && r.id === replyObj.id))
-              return c;
-            return { ...c, replies: [...existingReplies, replyObj] };
+        if (targetReplies.length >= MAX_REPLIES_PER_COMMENT) {
+          this.send(ws, {
+            type: "error",
+            code: "bad-message",
+            message: "Reply limit reached for this comment.",
           });
-        } else if (sanitizedAction.type === "toggle-resolve") {
-          updatedComments = comments.map((c) =>
-            c && typeof c === "object" && c.id === sanitizedAction.commentId
-              ? {
-                  ...c,
-                  resolved:
-                    sanitizedAction.resolved !== undefined ? sanitizedAction.resolved : !c.resolved,
-                }
-              : c,
-          );
-        } else if (sanitizedAction.type === "delete") {
-          updatedComments = comments.filter(
-            (c) => c && typeof c === "object" && c.id !== sanitizedAction.commentId,
-          );
+          return;
         }
-
-        parsed.comments = updatedComments;
-        await this.ctx.storage.put("snapshot", JSON.stringify(parsed));
-      } catch {
-        // Snapshot mutation failed; still fan out the validated action below so
-        // peers stay in sync (the next full snapshot will reconcile storage).
+        updatedComments = comments.map((c) => {
+          if (!c || typeof c !== "object" || c.id !== sanitizedAction.commentId) return c;
+          const existingReplies = Array.isArray(c.replies)
+            ? (c.replies as Record<string, unknown>[])
+            : [];
+          if (existingReplies.some((r) => r && typeof r === "object" && r.id === replyObj.id))
+            return c;
+          return { ...c, replies: [...existingReplies, replyObj] };
+        });
+      } else if (sanitizedAction.type === "toggle-resolve") {
+        updatedComments = comments.map((c) =>
+          c && typeof c === "object" && c.id === sanitizedAction.commentId
+            ? {
+                ...c,
+                resolved:
+                  sanitizedAction.resolved !== undefined ? sanitizedAction.resolved : !c.resolved,
+              }
+            : c,
+        );
+      } else if (sanitizedAction.type === "delete") {
+        updatedComments = comments.filter(
+          (c) => c && typeof c === "object" && c.id !== sanitizedAction.commentId,
+        );
       }
+
+      parsed.comments = updatedComments;
+      await this.ctx.storage.put("snapshot", JSON.stringify(parsed));
+    } catch {
+      // Snapshot mutation failed; still fan out the validated action below so
+      // peers stay in sync (the next full snapshot will reconcile storage).
     }
 
     this.broadcast(sanitizedMessage, ws);

From fdcf713212dc68d6cae0d06e529d5db470cc9eae Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Sun, 2 Aug 2026 16:12:37 +0000
Subject: [PATCH 8/9] style: auto-format (ruff + oxfmt) [pre-commit.ci]

---
 tests/collab-comment-validate.test.ts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index 3d15674c1..f3d5a0bcd 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -297,10 +297,7 @@ describe("validateComment", () => {
   });
 
   it("rejects an oversized id", () => {
-    assert.equal(
-      validateComment({ ...validComment, id: "c".repeat(MAX_ID_LENGTH + 1) }),
-      null,
-    );
+    assert.equal(validateComment({ ...validComment, id: "c".repeat(MAX_ID_LENGTH + 1) }), null);
   });
 
   it("rejects an invalid anchor", () => {

From 0d753135199a2d4c7b7dbda3a7abe4200b0e4c4f Mon Sep 17 00:00:00 2001
From: giswqs <giswqs@gmail.com>
Date: Mon, 3 Aug 2026 08:09:20 -0400
Subject: [PATCH 9/9] Address review feedback

- Preserve stored comments across full-project snapshots. New pure helper
  `preserveStoredComments` merges the persisted `comments` list into an
  incoming snapshot that omits the key (`serializeProject` drops it when a
  peer holds none), so a peer that has not merged comment-mutation
  broadcasts can no longer clobber them. A project that carries its own
  `comments` still wins, so a delete is never resurrected. The merged
  project is broadcast, healing a drifted sender.
- Bound total comment growth with `MAX_COMMENTS_PER_SESSION` (500),
  mirroring `CHAT_HISTORY_LIMIT` for the chat log; an "add" past the cap
  gets a `bad-message` error instead of growing the snapshot forever.
- Check the serialized snapshot against `MAX_SNAPSHOT_BYTES` before the
  storage write, matching `handleSnapshot`.
- Stop broadcasting a comment mutation whose persistence failed. The
  sender now gets an error and the fan-out is skipped, so connected peers
  no longer hold a comment that a late joiner or reconnect (both of which
  read from storage) would never see.
- Cover `preserveStoredComments` and the new limit in
  tests/collab-comment-validate.test.ts.
---
 tests/collab-comment-validate.test.ts  | 39 +++++++++++++++++++++
 workers/collab/src/comment-validate.ts | 26 ++++++++++++++
 workers/collab/src/session.ts          | 48 ++++++++++++++++++++++----
 3 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/tests/collab-comment-validate.test.ts b/tests/collab-comment-validate.test.ts
index f3d5a0bcd..ebe051b0d 100644
--- a/tests/collab-comment-validate.test.ts
+++ b/tests/collab-comment-validate.test.ts
@@ -3,9 +3,11 @@ import { describe, it } from "node:test";
 import {
   MAX_COMMENT_BODY_LENGTH,
   MAX_COMMENT_AUTHOR_LENGTH,
+  MAX_COMMENTS_PER_SESSION,
   MAX_ID_LENGTH,
   MAX_REPLIES_PER_COMMENT,
   MIN_COMMENT_INTERVAL_MS,
+  preserveStoredComments,
   validateAnchor,
   validateAuthor,
   validateComment,
@@ -20,6 +22,43 @@ describe("comment-validate constants", () => {
     assert.equal(MAX_COMMENT_AUTHOR_LENGTH, 120);
     assert.equal(MAX_ID_LENGTH, 200);
     assert.equal(MIN_COMMENT_INTERVAL_MS, 250);
+    assert.equal(MAX_REPLIES_PER_COMMENT, 100);
+    assert.equal(MAX_COMMENTS_PER_SESSION, 500);
+  });
+});
+
+// -- preserveStoredComments ---------------------------------------------------
+
+describe("preserveStoredComments", () => {
+  const stored = { comments: [{ id: "c1", body: "Persisted" }] };
+
+  it("carries stored comments into a project that omits the key", () => {
+    const result = preserveStoredComments({ layers: [] }, stored) as Record<string, unknown>;
+    assert.deepEqual(result.comments, stored.comments);
+    assert.deepEqual(result.layers, []);
+  });
+
+  it("keeps the incoming comments when the project supplies its own", () => {
+    const incoming = { layers: [], comments: [{ id: "c2" }] };
+    assert.deepEqual(preserveStoredComments(incoming, stored), incoming);
+  });
+
+  it("does not resurrect comments the sender explicitly cleared", () => {
+    const incoming = { layers: [], comments: [] };
+    const result = preserveStoredComments(incoming, stored) as Record<string, unknown>;
+    assert.deepEqual(result.comments, []);
+  });
+
+  it("leaves the project alone when storage is empty or corrupt", () => {
+    assert.deepEqual(preserveStoredComments({ layers: [] }, null), { layers: [] });
+    assert.deepEqual(preserveStoredComments({ layers: [] }, "not an object"), { layers: [] });
+    assert.deepEqual(preserveStoredComments({ layers: [] }, { comments: [] }), { layers: [] });
+    assert.deepEqual(preserveStoredComments({ layers: [] }, { comments: "nope" }), { layers: [] });
+  });
+
+  it("passes a null or non-object project through untouched", () => {
+    assert.equal(preserveStoredComments(null, stored), null);
+    assert.deepEqual(preserveStoredComments([1, 2], stored), [1, 2]);
   });
 });
 
diff --git a/workers/collab/src/comment-validate.ts b/workers/collab/src/comment-validate.ts
index a96e1c083..b57cb555a 100644
--- a/workers/collab/src/comment-validate.ts
+++ b/workers/collab/src/comment-validate.ts
@@ -17,11 +17,37 @@ export const MIN_COMMENT_INTERVAL_MS = 250;
 /** Maximum number of replies stored per comment. */
 export const MAX_REPLIES_PER_COMMENT = 100;
 
+/** Maximum number of comments stored per session. Bounds the snapshot growth a
+ *  sustained stream of "add" mutations can cause, the way `CHAT_HISTORY_LIMIT`
+ *  bounds the chat log. */
+export const MAX_COMMENTS_PER_SESSION = 500;
+
 /** True when `value` is a non-empty string within {@link MAX_ID_LENGTH}. */
 export function isBoundedId(value: unknown): value is string {
   return typeof value === "string" && value.length > 0 && value.length <= MAX_ID_LENGTH;
 }
 
+/** Carry the stored `comments` list into an incoming full-project snapshot that
+ *  doesn't supply one of its own.
+ *
+ *  The relay writes comments straight into the stored snapshot (see
+ *  `handleCommentMutation`), but `serializeProject` omits the key entirely when
+ *  a peer holds none — so a peer that hasn't merged those broadcasts yet (a race
+ *  with its debounced snapshot, or a client that joined before them) would
+ *  otherwise replace the persisted comments with nothing. A project that carries
+ *  its own `comments` still wins, so a delete is never resurrected.
+ *
+ *  `stored` is the already-parsed stored snapshot, or `null` when absent/corrupt.
+ */
+export function preserveStoredComments(project: unknown, stored: unknown): unknown {
+  if (!project || typeof project !== "object" || Array.isArray(project)) return project;
+  if ("comments" in project) return project;
+  if (!stored || typeof stored !== "object" || Array.isArray(stored)) return project;
+  const comments = (stored as Record<string, unknown>).comments;
+  if (!Array.isArray(comments) || comments.length === 0) return project;
+  return { ...(project as Record<string, unknown>), comments };
+}
+
 const HEX_COLOR_RE = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
 
 function finite(n: unknown): n is number {
diff --git a/workers/collab/src/session.ts b/workers/collab/src/session.ts
index 920c4a0fb..7b9f637f9 100644
--- a/workers/collab/src/session.ts
+++ b/workers/collab/src/session.ts
@@ -12,8 +12,10 @@ import type {
 } from "./protocol";
 import {
   isBoundedId,
+  MAX_COMMENTS_PER_SESSION,
   MAX_REPLIES_PER_COMMENT,
   MIN_COMMENT_INTERVAL_MS,
+  preserveStoredComments,
   validateComment,
   validateReply,
 } from "./comment-validate";
@@ -424,9 +426,15 @@ export class CollabSession extends DurableObject<Env> {
     }
 
     // The project was parsed in webSocketMessage; re-serialize it only to
-    // persist a string for storage and forward the object verbatim. The relay
-    // never reads any field inside the project.
-    const project = message.project ?? null;
+    // persist a string for storage and forward the object verbatim. `comments`
+    // is the one field the relay touches (it also writes it directly in
+    // `handleCommentMutation`), so preserve the stored list when this snapshot
+    // doesn't carry one — see `preserveStoredComments`. Peers get the merged
+    // project below, which heals a sender that had drifted.
+    const project = preserveStoredComments(
+      message.project ?? null,
+      parseStoredSnapshot(await this.ctx.storage.get<string>("snapshot")),
+    );
     // `rev` is written during /init before any socket can join, so the stored
     // value is always present; the `?? 0` is a defensive floor, never the
     // client's counter (a server-owned monotonic value must not trust input).
@@ -799,6 +807,14 @@ export class CollabSession extends DurableObject<Env> {
       if (sanitizedAction.type === "add") {
         const newComment = sanitizedAction.comment as Record<string, unknown>;
         const exists = comments.some((c) => c && typeof c === "object" && c.id === newComment.id);
+        if (!exists && comments.length >= MAX_COMMENTS_PER_SESSION) {
+          this.send(ws, {
+            type: "error",
+            code: "bad-message",
+            message: "Comment limit reached for this session.",
+          });
+          return;
+        }
         updatedComments = exists ? comments : [...comments, newComment];
       } else if (sanitizedAction.type === "reply") {
         const replyObj = sanitizedAction.reply as Record<string, unknown>;
@@ -850,10 +866,30 @@ export class CollabSession extends DurableObject<Env> {
       }
 
       parsed.comments = updatedComments;
-      await this.ctx.storage.put("snapshot", JSON.stringify(parsed));
+      const serialized = JSON.stringify(parsed);
+      // `handleSnapshot` bounds a full project the same way. Check before the
+      // put so an oversized value surfaces as an error the sender can see
+      // rather than a throw the catch below would have to guess at.
+      if (ENCODER.encode(serialized).length > MAX_SNAPSHOT_BYTES) {
+        this.send(ws, {
+          type: "error",
+          code: "bad-message",
+          message: "Project is too large to store this comment.",
+        });
+        return;
+      }
+      await this.ctx.storage.put("snapshot", serialized);
     } catch {
-      // Snapshot mutation failed; still fan out the validated action below so
-      // peers stay in sync (the next full snapshot will reconcile storage).
+      // The mutation never reached storage, so a late joiner or a reconnect
+      // (both of which read from storage) would not see it. Tell the sender and
+      // skip the fan-out rather than leaving connected peers holding a comment
+      // that isn't persisted anywhere.
+      this.send(ws, {
+        type: "error",
+        code: "bad-message",
+        message: "Could not save the comment. Try again.",
+      });
+      return;
     }
 
     this.broadcast(sanitizedMessage, ws);