fix(pdf): stop page tools failing on short and encrypted PDFs (#594)

Empty the hardcoded page-range default in remove/split/extract PDF tools (remove-pages defaulted to "2,4-6", out of range for any PDF under 6 pages) and disable submit until a range is entered. Reject password-protected PDFs up front for PDF-only tools with guidance to unlock first, instead of failing cryptically in the qpdf worker. Adds integration + e2e coverage.

apps/api/src/modality/document-input.ts

@@ -43,7 +43,9 @@ export async function validatePdfPath(
   opts.signal?.throwIfAborted();
   if (passwordProtected) {
     if (opts.rejectPasswordProtected) {
-      throw new InputValidationError("Password-protected PDFs cannot be processed by this tool");
+      throw new InputValidationError(
+        "This PDF is password-protected. Unlock it first with the Unlock PDF tool, then try again.",
+      );
     }
     // Without a password qpdf cannot safely inspect the structure or pages.
     return;

apps/api/src/routes/tool-factory.ts

@@ -118,6 +118,14 @@ export interface ToolRouteConfig<T> {
    * tools that intentionally accept damaged inputs (e.g. repair-pdf).
    */
   skipStructuralValidation?: boolean;
+  /**
+   * When set, the factory does NOT reject password-protected PDFs at input
+   * validation. Only unlock-pdf sets this: it takes an encrypted PDF plus a
+   * password and decrypts it. Every other document tool leaves this off, so
+   * the factory rejects encrypted PDFs up front (400) with guidance to unlock
+   * first, instead of letting qpdf fail cryptically in the worker.
+   */
+  allowPasswordProtectedPdf?: boolean;
   /**
    * When set, produces a redacted copy of settings for the durable DB
    * row. Passwords and other secrets are replaced so they do not persist
@@ -398,6 +406,15 @@ export function createToolRoute<T>(app: FastifyInstance, config: ToolRouteConfig
             const prepared = await handlerForPosition(i).prepare(fileBuffer, fname, {
               scratchDir,
               lenient: config.skipStructuralValidation,
+              // Reject encrypted PDFs up front only for PDF-only tools (qpdf
+              // page ops etc.). Scoped to acceptedInputs === [".pdf"] so the
+              // flag never forces a %PDF- header on non-PDF document tools
+              // (markdown/epub/docx converters). unlock-pdf opts out.
+              rejectPasswordProtected:
+                modality === "document" &&
+                !config.allowPasswordProtectedPdf &&
+                !!accepted?.length &&
+                accepted.every((e) => e === ".pdf"),
             });
             fileBuffer = prepared.buffer;
             fname = prepared.filename;

apps/api/src/routes/tools/sign-pdf.ts

@@ -123,7 +123,13 @@ export function registerSignPdf(app: FastifyInstance) {
 
     const pdfBuffer = await getObjectBuffer(pdfKey);
     try {
-      await inputHandlerFor("document").prepare(pdfBuffer, filename, { scratchDir: tmpdir() });
+      // Signing needs a readable PDF; reject encrypted ones up front (the
+      // "unlock first" guidance rides in details) with the same policy the
+      // factory gives other PDF-only tools, instead of failing in the worker.
+      await inputHandlerFor("document").prepare(pdfBuffer, filename, {
+        scratchDir: tmpdir(),
+        rejectPasswordProtected: true,
+      });
     } catch (err) {
       return reply.status(400).send({
         error: "Invalid PDF",

apps/api/src/routes/tools/unlock-pdf.ts

@@ -13,6 +13,10 @@ export function registerUnlockPdf(app: FastifyInstance) {
   createToolRoute(app, {
     toolId: "unlock-pdf",
     settingsSchema,
+    // unlock-pdf's whole job is to decrypt: its input is an encrypted PDF plus
+    // the password, so it must opt out of the factory's password-protected
+    // rejection that every other document tool gets by default.
+    allowPasswordProtectedPdf: true,
     redactSettingsForAudit: (settings) => {
       const s = settings as z.infer<typeof settingsSchema>;
       return {

apps/web/src/components/tools/extract-pages-settings.tsx

@@ -12,7 +12,7 @@ export function ExtractPagesSettings() {
   const { processFiles, processAllFiles, processing, error, progress } =
     useToolProcessor("extract-pages");
 
-  const [range, setRange] = useState("1-3");
+  const [range, setRange] = useState("");
 
   const hasFile = files.length > 0;
   const hasMultiple = files.length > 1;
@@ -36,6 +36,7 @@ export function ExtractPagesSettings() {
           id="ep-range"
           type="text"
           value={range}
+          placeholder="1-3"
           onChange={(e) => setRange(e.target.value)}
           className="w-full mt-0.5 px-2 py-1.5 rounded border border-border bg-background text-sm text-foreground"
         />
@@ -58,7 +59,7 @@ export function ExtractPagesSettings() {
           type="button"
           data-testid="extract-pages-submit"
           onClick={handleProcess}
-          disabled={!hasFile || processing}
+          disabled={!hasFile || processing || !range.trim()}
           className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium disabled:opacity-50 disabled:cursor-not-allowed"
         >
           {hasMultiple ? format(s.submitBatch, { count: files.length }) : s.submit}

apps/web/src/components/tools/remove-pages-settings.tsx

@@ -12,7 +12,7 @@ export function RemovePagesSettings() {
   const { processFiles, processAllFiles, processing, error, progress } =
     useToolProcessor("remove-pages");
 
-  const [pages, setPages] = useState("2,4-6");
+  const [pages, setPages] = useState("");
 
   const hasFile = files.length > 0;
   const hasMultiple = files.length > 1;
@@ -36,6 +36,7 @@ export function RemovePagesSettings() {
           id="rp-pages"
           type="text"
           value={pages}
+          placeholder="2,4-6"
           onChange={(e) => setPages(e.target.value)}
           className="w-full mt-0.5 px-2 py-1.5 rounded border border-border bg-background text-sm text-foreground"
         />
@@ -58,7 +59,7 @@ export function RemovePagesSettings() {
           type="button"
           data-testid="remove-pages-submit"
           onClick={handleProcess}
-          disabled={!hasFile || processing}
+          disabled={!hasFile || processing || !pages.trim()}
           className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium disabled:opacity-50 disabled:cursor-not-allowed"
         >
           {hasMultiple ? format(s.submitBatch, { count: files.length }) : s.submit}

apps/web/src/components/tools/split-pdf-settings.tsx

@@ -15,7 +15,7 @@ export function SplitPdfSettings() {
     useToolProcessor("split-pdf");
 
   const [mode, setMode] = useState<SplitMode>("range");
-  const [range, setRange] = useState("1-3,5");
+  const [range, setRange] = useState("");
   const [everyN, setEveryN] = useState(1);
 
   const hasFile = files.length > 0;
@@ -97,7 +97,7 @@ export function SplitPdfSettings() {
           type="button"
           data-testid="split-pdf-submit"
           onClick={handleProcess}
-          disabled={!hasFile || processing}
+          disabled={!hasFile || processing || (mode === "range" && !range.trim())}
           className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium disabled:opacity-50 disabled:cursor-not-allowed"
         >
           {hasMultiple ? format(s.submitBatch, { count: files.length }) : s.submit}

tests/e2e/pdf-page-range-default.spec.ts

@@ -0,0 +1,85 @@
+import path from "node:path";
+import { expect, test, waitForProcessing } from "./helpers";
+
+// Regression guard: the page-range box in remove/extract/split PDF tools used
+// to ship a hardcoded default (remove-pages "2,4-6") that is out of range for
+// any PDF with fewer pages, so clicking the tool on a typical short PDF failed
+// with "page out of range". The box now starts empty and submit stays disabled
+// until a range is entered.
+const PDF_FIXTURE = path.join(
+  process.cwd(),
+  "tests",
+  "fixtures",
+  "document",
+  "valid",
+  "test-3page.pdf",
+);
+
+async function uploadPdf(page: import("@playwright/test").Page) {
+  const fileChooserPromise = page.waitForEvent("filechooser");
+  const uploadButton = page.getByRole("button", { name: /upload from computer/i }).first();
+  if (await uploadButton.isVisible({ timeout: 2000 }).catch(() => false)) {
+    await uploadButton.click();
+  } else {
+    await page.locator("[class*='border-dashed']").first().click();
+  }
+  const fileChooser = await fileChooserPromise;
+  await fileChooser.setFiles(PDF_FIXTURE);
+  await page.waitForTimeout(500);
+}
+
+test.describe("PDF page-range default (out-of-range footgun fix)", () => {
+  test("remove-pages: box starts empty, submit disabled until a range is typed, then deletes", async ({
+    loggedInPage: page,
+  }) => {
+    await page.goto("/pdf/remove-pages");
+    await uploadPdf(page);
+
+    const pagesInput = page.locator("#rp-pages");
+    await expect(pagesInput).toHaveValue("");
+    await expect(pagesInput).toHaveAttribute("placeholder", "2,4-6");
+
+    const submit = page.getByTestId("remove-pages-submit");
+    await expect(submit).toBeDisabled();
+
+    // Typing a valid page on the 3-page fixture enables submit and processes.
+    await pagesInput.fill("2");
+    await expect(submit).toBeEnabled();
+
+    await submit.click();
+    await waitForProcessing(page, 60_000);
+    await expect(page.getByText("Download").first()).toBeVisible({ timeout: 30_000 });
+  });
+
+  test("extract-pages: submit disabled while the range box is empty", async ({
+    loggedInPage: page,
+  }) => {
+    await page.goto("/pdf/extract-pages");
+    await uploadPdf(page);
+
+    const rangeInput = page.locator("#ep-range");
+    await expect(rangeInput).toHaveValue("");
+
+    const submit = page.getByTestId("extract-pages-submit");
+    await expect(submit).toBeDisabled();
+
+    await rangeInput.fill("1");
+    await expect(submit).toBeEnabled();
+  });
+
+  test("split-pdf: submit disabled while the range box is empty in range mode", async ({
+    loggedInPage: page,
+  }) => {
+    await page.goto("/pdf/split-pdf");
+    await uploadPdf(page);
+
+    const rangeInput = page.locator("#sp-range");
+    await expect(rangeInput).toHaveValue("");
+
+    const submit = page.getByTestId("split-pdf-submit");
+    await expect(submit).toBeDisabled();
+
+    await rangeInput.fill("1-2");
+    await expect(submit).toBeEnabled();
+  });
+});

tests/integration/sign-pdf.test.ts

@@ -5,6 +5,7 @@ import { buildTestApp, createMultipartPayload, loginAsAdmin, type TestApp } from
 
 const PDF = readFixture(fixtures.document.pdf3);
 const SIG = readFixture(fixtures.image.base.png200);
+const ENCRYPTED_PDF = readFixture(fixtures.document.encrypted);
 
 // The stamping test invokes the docs profile's doc_sign script (PyMuPDF) and is
 // gated on fitz so it skips where PyMuPDF is not installed (e.g. CI integration
@@ -136,6 +137,28 @@ describe("sign-pdf", () => {
     expect(JSON.parse(res.body)).toMatchObject({ error: "Invalid PDF" });
   });
 
+  it("rejects a password-protected PDF before enqueueing work", async () => {
+    // A signature can't be stamped onto an encrypted PDF without the password;
+    // reject it up front (before any Python call) with guidance to unlock first.
+    const res = await postFields([
+      {
+        name: "file",
+        filename: "encrypted.pdf",
+        contentType: "application/pdf",
+        content: ENCRYPTED_PDF,
+      },
+      { name: "sig0", filename: "sig0.png", contentType: "image/png", content: SIG },
+      {
+        name: "placements",
+        content: JSON.stringify([{ sig: 0, page: 0, x: 0, y: 0, w: 0.25, h: 0.1 }]),
+      },
+    ]);
+
+    expect(res.statusCode).toBe(400);
+    const body = JSON.parse(res.body);
+    expect(body.details || body.error).toMatch(/password-protected|unlock/i);
+  });
+
   it("rejects an invalid signature image before enqueueing work", async () => {
     const res = await postFields([
       { name: "file", filename: "in.pdf", contentType: "application/pdf", content: PDF },

tests/integration/tools/document/remove-pages.test.ts

@@ -13,6 +13,7 @@ import {
 
 const PDF = readFixture(fixtures.document.pdf3);
 const PDF_PATH = fixtures.document.pdf3;
+const ENCRYPTED_PDF = readFixture(fixtures.document.encrypted);
 
 let testApp: TestApp;
 let adminToken: string;
@@ -107,6 +108,30 @@ describe.skipIf(!qpdfAvailable())("remove-pages (requires qpdf)", () => {
     expect(body.details || body.error || body.message).toMatch(/out of range/i);
   }, 60_000);
 
+  it("rejects a password-protected PDF up front with a clear message", async () => {
+    // A page tool cannot operate on an encrypted PDF without the password.
+    // The factory must reject it at input validation (400) with guidance to
+    // unlock first, instead of letting qpdf fail cryptically in the worker.
+    const { body, contentType } = createMultipartPayload([
+      {
+        name: "file",
+        filename: "encrypted.pdf",
+        contentType: "application/pdf",
+        content: ENCRYPTED_PDF,
+      },
+      { name: "settings", content: JSON.stringify({ pages: "1" }) },
+    ]);
+    const res = await testApp.app.inject({
+      method: "POST",
+      url: "/api/v1/tools/pdf/remove-pages",
+      headers: { authorization: `Bearer ${adminToken}`, "content-type": contentType },
+      body,
+    });
+    expect(res.statusCode).toBe(400);
+    const parsed = JSON.parse(res.body);
+    expect(parsed.error).toMatch(/password-protected|unlock/i);
+  }, 60_000);
+
   it("removes page 1 from a 96-page pdf (large keepSpec path)", async () => {
     // The keep-spec for 95 pages as a raw comma list would be ~280 chars,
     // exceeding the 200-char assertValidRange cap. compressPageRuns compresses

tests/qa/settings.qa.spec.ts

@@ -1948,7 +1948,7 @@ test.describe("DOCUMENT: split-pdf", () => {
   }) => {
     const issues = instrument(page);
     await setupTool(page, "split-pdf", PDF_3PAGE);
-    // Default mode is range, range="1-3,5"
+    // Default mode is range; the range box starts empty (placeholder "1-3,5")
     await fillInput(page, "sp-range", "1-2");
     const dl = await processAndDownload(page, "split-pdf", "long");
     if (!dl.ok) {