From a2cb1a8261b2669ae7e656a418999b70a37f6b4f Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Thu, 16 Jul 2026 19:26:08 +0800 Subject: [PATCH] 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 | 50 +++++++++---- .../unit/doc-engine/python-docs-parse.test.ts | 72 +++++++++++++++++++ 2 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 tests/unit/doc-engine/python-docs-parse.test.ts diff --git a/packages/doc-engine/src/python-docs.ts b/packages/doc-engine/src/python-docs.ts index f9142595..b29e6135 100644 --- a/packages/doc-engine/src/python-docs.ts +++ b/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(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 { 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 { /** Flatten forms/annotations into page content (PyMuPDF bake). */ export async function pdfFlattenPy(inPath: string, outPath: string): Promise { 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 { 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("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> { const stdout = await runDocsScript("doc_metadata", { path: inPath, mode: "get" }); - const parsed = JSON.parse(stdout.trim()) as { metadata?: Record; error?: string }; + const parsed = parseDocsJson<{ metadata?: Record; 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}`); } diff --git a/tests/unit/doc-engine/python-docs-parse.test.ts b/tests/unit/doc-engine/python-docs-parse.test.ts new file mode 100644 index 00000000..65800ea5 --- /dev/null +++ b/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("500 Internal Server Error"); + + 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"); + }); +});