fix(site-builder): enforce the payload cap unconditionally, bound description

Review found the 512KB cap on buildReportPayload only ever measured on the
includeCanvas+canvasState branch -- opt-out and null-canvas paths returned
early without checking size at all, and an oversized non-canvas field
(description straight from a user's textarea) could slip through with a
canvas_state_omitted: 'size' marker that falsely claimed the drop had fixed
things.

- Truncate description to 5000 chars (matches the server-side validator's
  future limit), silently: unlike canvas_state, a truncated free-text
  description is exactly what it looks like, not a misleadingly-plausible
  partial structure.
- Route every return path through finalize(), which measures the actual
  candidate payload and throws rather than returning an oversized body --
  so 'size' can never be attached to a payload that's still over cap.
- Keep the opt-out early return structurally separate so canvas_state is
  never populated from input on that path, regardless of the cap check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

craft/src/utils/report-payload.test.ts

@@ -65,3 +65,41 @@ describe('buildReportPayload', () => {
     expect(p.canvas_state_omitted).toBeUndefined();
   });
 });
+
+describe('description size bounding', () => {
+  const hugeDescription = 'y'.repeat(600 * 1024);
+
+  test('oversized description does not blow the cap with canvas included', () => {
+    const p = buildReportPayload({ ...base, description: hugeDescription });
+    expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES);
+  });
+
+  test('oversized description does not blow the cap when opted out', () => {
+    const p = buildReportPayload({ ...base, description: hugeDescription, includeCanvas: false });
+    expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES);
+    expect(p.canvas_state).toBeNull();
+    expect(p.canvas_state_omitted).toBe('opt-out');
+  });
+
+  test('oversized description does not blow the cap with no canvas state', () => {
+    const p = buildReportPayload({ ...base, description: hugeDescription, canvasState: null });
+    expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES);
+    expect(p.canvas_state).toBeNull();
+    expect(p.canvas_state_omitted).toBeUndefined();
+  });
+
+  test('description is truncated to 5000 characters', () => {
+    const p = buildReportPayload({ ...base, description: hugeDescription });
+    expect(p.description.length).toBeLessThanOrEqual(5000);
+  });
+});
+
+describe('honest size markers', () => {
+  test('the size marker never appears on a payload that is still oversized after dropping canvas_state', () => {
+    // consoleErrors is normally bounded upstream (console-buffer.ts caps it at
+    // 20 entries x 500 chars), but this function must not trust that -- it's
+    // just an array of the exported type as far as the signature is concerned.
+    const massiveErrors = Array.from({ length: 5000 }, (_, i) => ({ ts: i, message: 'x'.repeat(200) }));
+    expect(() => buildReportPayload({ ...base, consoleErrors: massiveErrors })).toThrow();
+  });
+});

craft/src/utils/report-payload.ts

@@ -15,6 +15,18 @@ import type { ConsoleErrorEntry } from './console-buffer';
 
 export const MAX_PAYLOAD_BYTES = 512 * 1024;
 
+/**
+ * Matches the limit the server-side validator enforces (a later task) so the
+ * client never builds a body the server would reject outright. Truncated
+ * silently, not flagged: unlike canvas_state (a serialized tree, where a cut
+ * mid-structure looks like a smaller-but-still-valid tree and actively
+ * misleads whoever reads it), a truncated free-text description is exactly
+ * what it looks like -- text that stops partway through. Nothing about the
+ * cut invents false structure, and the limit mirrors what the server would
+ * have discarded anyway.
+ */
+export const MAX_DESCRIPTION_CHARS = 5000;
+
 export type ReportCategory = 'bug' | 'confusing' | 'feature';
 
 export interface BuildReportPayloadInput {
@@ -53,14 +65,44 @@ export interface ReportPayload {
 }
 
 function byteLength(value: string): number {
+  // TextEncoder is standard in every environment this module actually runs
+  // in (Node/vitest and every real browser target). The fallback below is
+  // unreachable by design -- kept only so a missing global degrades to a
+  // conservative-ish count rather than throwing, not because it's expected
+  // to fire. (It undercounts multi-byte UTF-8, so treat it as dead code.)
   if (typeof TextEncoder !== 'undefined') return new TextEncoder().encode(value).length;
   return value.length;
 }
 
+function payloadBytes(payload: ReportPayload): number {
+  return byteLength(JSON.stringify(payload));
+}
+
+/**
+ * Every return path funnels through here so the cap is checked
+ * unconditionally, not just on the canvas-included branch. If the payload
+ * is still over budget after every available reduction (dropping
+ * canvas_state, truncating description), there is nothing left to cut --
+ * returning it anyway would ship an oversized body that, if it carries
+ * `canvas_state_omitted: 'size'`, falsely claims the drop fixed things.
+ * Throwing surfaces that as a distinct, honest failure instead.
+ */
+function finalize(payload: ReportPayload): ReportPayload {
+  if (payloadBytes(payload) > MAX_PAYLOAD_BYTES) {
+    throw new Error(
+      `Report payload is ${payloadBytes(payload)} bytes, over the ${MAX_PAYLOAD_BYTES}-byte cap, ` +
+        'even with canvas_state dropped. Nothing left to reduce.'
+    );
+  }
+  return payload;
+}
+
 export function buildReportPayload(input: BuildReportPayloadInput): ReportPayload {
-  const payload: ReportPayload = {
+  const description = input.description.trim().slice(0, MAX_DESCRIPTION_CHARS);
+
+  const base: ReportPayload = {
     category: input.category,
-    description: input.description.trim(),
+    description,
     site_id: input.siteId,
     site_domain: input.siteDomain,
     page_id: input.pageId,
@@ -75,16 +117,22 @@ export function buildReportPayload(input: BuildReportPayloadInput): ReportPayloa
   };
 
   if (!input.includeCanvas) {
-    payload.canvas_state_omitted = 'opt-out';
+    // The opt-out promise is inviolable: canvas_state is never populated
-    return payload;
+    // from input.canvasState on this path, no matter what else changes
+    // below it. Only the cap is checked here, not whether to include canvas.
+    return finalize({ ...base, canvas_state_omitted: 'opt-out' });
   }
 
-  if (!input.canvasState) return payload;
+  if (!input.canvasState) return finalize(base);
 
-  payload.canvas_state = input.canvasState;
+  const withCanvas: ReportPayload = { ...base, canvas_state: input.canvasState };
-  if (byteLength(JSON.stringify(payload)) > MAX_PAYLOAD_BYTES) {
+  if (payloadBytes(withCanvas) <= MAX_PAYLOAD_BYTES) return withCanvas;
-    payload.canvas_state = null;
+
-    payload.canvas_state_omitted = 'size';
+  // canvas_state alone pushed this over budget: drop it whole (never
-  }
+  // truncate -- a partial Craft tree looks valid but is missing nodes,
-  return payload;
+  // which is worse than no tree) and re-measure. The 'size' marker is only
+  // attached once we've confirmed dropping the canvas actually brought the
+  // payload back under the cap; finalize() throws instead of returning it
+  // if it didn't.
+  return finalize({ ...base, canvas_state: null, canvas_state_omitted: 'size' });
 }