fix(doc-engine): guard sidecar JSON parsing against non-JSON stdout (#532)

Route every doc_* helper JSON.parse through a guarded helper; non-JSON stdout now yields a diagnosable SafeError with the raw output in the cause instead of a bare SyntaxError.

packages/doc-engine/src/python-docs.ts

@@ -1,10 +1,31 @@
 import { runDocsScript } from "@snapotter/ai";
-import type { SignPlacement } from "@snapotter/shared";
+import { SafeError, type SignPlacement } from "@snapotter/shared";
+
+/**
+ * Parse the JSON line the docs dispatcher prints on stdout. Every doc_* script
+ * is contract-bound to emit a single JSON object, but a crashed interpreter, a
+ * library warning, or a partial write can leave non-JSON on stdout. A bare
+ * JSON.parse then throws an opaque SyntaxError that discards the real output;
+ * wrap it so the failure carries a safe, authored message and the raw stdout
+ * (in the cause) survives for triage instead of a context-free SyntaxError.
+ */
+function parseDocsJson<T>(script: string, stdout: string): T {
+  const trimmed = stdout.trim();
+  try {
+    return JSON.parse(trimmed) as T;
+  } catch {
+    throw new SafeError("Document tool returned non-JSON output", {
+      kind: "bug",
+      code: script,
+      cause: new Error(`${script} stdout (first 200 chars): ${trimmed.slice(0, 200)}`),
+    });
+  }
+}
 
 /** Page count via the docs-profile Python dispatcher (pikepdf). */
 export async function pdfPageCountPy(absPath: string): Promise<number> {
   const stdout = await runDocsScript("doc_pagecount", { path: absPath });
-  const parsed = JSON.parse(stdout.trim()) as { pages?: number; error?: string };
+  const parsed = parseDocsJson<{ pages?: number; error?: string }>("doc_pagecount", stdout);
   if (parsed.error || typeof parsed.pages !== "number") {
     throw new Error(`doc_pagecount failed: ${parsed.error ?? stdout.slice(0, 200)}`);
   }
@@ -14,7 +35,7 @@ export async function pdfPageCountPy(absPath: string): Promise<number> {
 /** Flatten forms/annotations into page content (PyMuPDF bake). */
 export async function pdfFlattenPy(inPath: string, outPath: string): Promise<void> {
   const stdout = await runDocsScript("doc_flatten", { path: inPath, out: outPath });
-  const parsed = JSON.parse(stdout.trim()) as { ok?: boolean; error?: string };
+  const parsed = parseDocsJson<{ ok?: boolean; error?: string }>("doc_flatten", stdout);
   if (parsed.error) {
     throw new Error(`doc_flatten failed: ${parsed.error}`);
   }
@@ -27,7 +48,7 @@ export async function pdfFlattenPy(inPath: string, outPath: string): Promise<voi
  */
 export async function pdfScrubProducerPy(inPath: string, outPath: string): Promise<void> {
   const stdout = await runDocsScript("doc_scrub_meta", { path: inPath, out: outPath });
-  const parsed = JSON.parse(stdout.trim()) as { ok?: boolean; error?: string };
+  const parsed = parseDocsJson<{ ok?: boolean; error?: string }>("doc_scrub_meta", stdout);
   if (parsed.error) {
     throw new Error(`doc_scrub_meta failed: ${parsed.error}`);
   }
@@ -46,11 +67,11 @@ export async function pdfRedactPy(
     terms,
     caseSensitive,
   });
-  const parsed = JSON.parse(stdout.trim()) as {
+  const parsed = parseDocsJson<{
     found?: number;
     verified?: boolean;
     error?: string;
-  };
+  }>("doc_redact", stdout);
   if (parsed.error) {
     throw new Error(`doc_redact failed: ${parsed.error}`);
   }
@@ -63,7 +84,7 @@ export async function pdfRedactPy(
 /** Extract plain text from a PDF (PyMuPDF get_text). */
 export async function pdfTextPy(inPath: string, outTxtPath: string): Promise<{ chars: number }> {
   const stdout = await runDocsScript("doc_text", { path: inPath, out: outTxtPath });
-  const parsed = JSON.parse(stdout.trim()) as { chars?: number; error?: string };
+  const parsed = parseDocsJson<{ chars?: number; error?: string }>("doc_text", stdout);
   if (parsed.error) {
     throw new Error(`doc_text failed: ${parsed.error}`);
   }
@@ -80,7 +101,7 @@ export async function pdfToWordPy(inPath: string, outPath: string): Promise<void
     { path: inPath, out: outPath },
     { timeoutMs: 300_000 },
   );
-  const parsed = JSON.parse(stdout.trim()) as { ok?: boolean; error?: string };
+  const parsed = parseDocsJson<{ ok?: boolean; error?: string }>("doc_to_word", stdout);
   if (parsed.error) {
     throw new Error(`doc_to_word failed: ${parsed.error}`);
   }
@@ -89,7 +110,10 @@ export async function pdfToWordPy(inPath: string, outPath: string): Promise<void
 /** Read PDF document metadata (pikepdf docinfo). */
 export async function pdfMetadataGetPy(inPath: string): Promise<Record<string, string>> {
   const stdout = await runDocsScript("doc_metadata", { path: inPath, mode: "get" });
-  const parsed = JSON.parse(stdout.trim()) as { metadata?: Record<string, string>; error?: string };
+  const parsed = parseDocsJson<{ metadata?: Record<string, string>; error?: string }>(
+    "doc_metadata",
+    stdout,
+  );
   if (parsed.error) {
     throw new Error(`doc_metadata get failed: ${parsed.error}`);
   }
@@ -111,7 +135,7 @@ export async function pdfMetadataSetPy(
     mode: "set",
     metadata,
   });
-  const parsed = JSON.parse(stdout.trim()) as { ok?: boolean; error?: string };
+  const parsed = parseDocsJson<{ ok?: boolean; error?: string }>("doc_metadata", stdout);
   if (parsed.error) {
     throw new Error(`doc_metadata set failed: ${parsed.error}`);
   }
@@ -128,7 +152,7 @@ export async function htmlToPdfPy(
     { path: inPath, out: outPath, mode },
     { timeoutMs: 120_000 },
   );
-  const parsed = JSON.parse(stdout.trim()) as { ok?: boolean; error?: string };
+  const parsed = parseDocsJson<{ ok?: boolean; error?: string }>("doc_html_pdf", stdout);
   if (parsed.error) {
     throw new Error(`doc_html_pdf failed: ${parsed.error}`);
   }
@@ -147,11 +171,11 @@ export async function pdfSignPy(
     signatures,
     placements,
   });
-  const parsed = JSON.parse(stdout.trim()) as {
+  const parsed = parseDocsJson<{
     ok?: boolean;
     placed?: number;
     error?: string;
-  };
+  }>("doc_sign", stdout);
   if (parsed.error) {
     throw new Error(`doc_sign failed: ${parsed.error}`);
   }

tests/unit/doc-engine/python-docs-parse.test.ts

@@ -0,0 +1,72 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+// Stub the docs dispatcher so we control exactly what stdout the helpers parse.
+const runDocsScript = vi.fn();
+vi.mock("@snapotter/ai", () => ({
+  runDocsScript: (...args: unknown[]) => runDocsScript(...args),
+}));
+
+import { isSafeMessageError } from "@snapotter/shared";
+import { pdfPageCountPy, pdfRedactPy } from "../../../packages/doc-engine/src/python-docs.js";
+
+/** Flatten an error's message, code, and cause-chain messages into one string. */
+function errorText(err: unknown): string {
+  const parts: string[] = [];
+  let cur: unknown = err;
+  for (let depth = 0; cur instanceof Error && depth < 5; depth++) {
+    parts.push(cur.message);
+    const code = (cur as { code?: unknown }).code;
+    if (typeof code === "string") parts.push(code);
+    cur = (cur as { cause?: unknown }).cause;
+  }
+  return parts.join(" | ");
+}
+
+describe("python-docs sidecar JSON parsing", () => {
+  beforeEach(() => {
+    runDocsScript.mockReset();
+  });
+
+  it("parses valid JSON output normally", async () => {
+    runDocsScript.mockResolvedValue(JSON.stringify({ found: 3, verified: true }));
+    await expect(pdfRedactPy("/in.pdf", "/out.pdf", ["x"], false)).resolves.toEqual({ found: 3 });
+  });
+
+  it("throws a safe, diagnosable error (not a bare SyntaxError) when stdout is not JSON", async () => {
+    const garbage =
+      'Traceback (most recent call last):\n  File "redact.py", line 9\nRuntimeError: boom';
+    runDocsScript.mockResolvedValue(garbage);
+
+    let caught: unknown;
+    try {
+      await pdfRedactPy("/in.pdf", "/out.pdf", ["secret"], false);
+    } catch (e) {
+      caught = e;
+    }
+
+    expect(caught).toBeInstanceOf(Error);
+    expect((caught as Error).name).not.toBe("SyntaxError");
+    // The user gets an authored, safe message rather than a raw parser error.
+    expect(isSafeMessageError(caught)).toBe(true);
+    const text = errorText(caught);
+    // The real sidecar output survives for Sentry.
+    expect(text).toContain("Traceback");
+    // And it identifies which script produced it.
+    expect(text).toContain("doc_redact");
+  });
+
+  it("guards every helper, not only redact", async () => {
+    runDocsScript.mockResolvedValue("<html>500 Internal Server Error</html>");
+
+    let caught: unknown;
+    try {
+      await pdfPageCountPy("/in.pdf");
+    } catch (e) {
+      caught = e;
+    }
+
+    expect((caught as Error | undefined)?.name).not.toBe("SyntaxError");
+    expect(isSafeMessageError(caught)).toBe(true);
+    expect(errorText(caught)).toContain("doc_pagecount");
+  });
+});