fix(attachments): make PDF screening actually work (pdf-parse v2 API + bundling) (ABCA-745)

PDF attachment screening was silently broken on EVERY channel — live-caught
when the Linear P4 path fetched a PDF and got 'PDF processing is unavailable'.
Two independent bugs, both platform-wide (Jira aws-samples#619 equally affected):

1. API mismatch: code called pdf-parse as the v1 callable `pdfParseFn(buf)`,
   but the pinned pdf-parse@^2.4.5 exposes a `PDFParse` CLASS
   (`new PDFParse({data}).getText()`). A stale `@types/pdf-parse@^1` + a hand-
   written v1 module shim (src/types/pdf-parse.d.ts) made the wrong shape
   compile. Rewrote extractPdfText to the v2 class API (first:N page cap,
   destroy() teardown); removed both v1 type sources so v2's bundled types apply.

2. Bundling: the Linear + Jira webhook processors bundled pdf-parse with esbuild
   (only `externalModules:['@aws-sdk/*']`), mangling its pdfjs/@napi-rs/canvas
   deps → 'DOMMatrix is not defined' at import. Added `nodeModules:['pdf-parse']`
   so it resolves natively at runtime — matching the TaskApi/orchestrator
   attachment-screening bundling that already had the carve-out.

Verified: PDFParse.getText extracts text headless in raw node24 (= the Lambda
runtime). Tests mock the v2 class (ts-jest can't drive pdfjs's ESM worker); the
real extraction is validated on the live deploy. text/csv/etc. unaffected.

Author: isadeks

cdk/package.json

@@ -48,7 +48,6 @@
     "@types/jest": "^30.0.0",
     "@types/js-yaml": "^4.0.9",
     "@types/node": "^26",
-    "@types/pdf-parse": "^1.1.5",
     "@types/ws": "^8.18.1",
     "@typescript-eslint/eslint-plugin": "^8",
     "@typescript-eslint/parser": "^8",

cdk/src/constructs/jira-integration.ts

@@ -207,6 +207,15 @@ export class JiraIntegration extends Construct {
     const commonBundling: lambda.BundlingOptions = {
       externalModules: ['@aws-sdk/*'],
     };
+    // pdf-parse (v2, pdfjs-based) can't be esbuild-bundled — its pdfjs/native
+    // (@napi-rs/canvas) deps break at import (`DOMMatrix is not defined`,
+    // ABCA-745). Ship it unbundled via `nodeModules` so it resolves natively at
+    // runtime. Mirrors TaskApi's attachment-screening bundling. Jira's #619
+    // attachment path screens PDFs, so its webhook processor needs the carve-out.
+    const attachmentScreeningBundling: lambda.BundlingOptions = {
+      ...commonBundling,
+      nodeModules: ['pdf-parse'],
+    };
 
     // --- Task creation environment (matches LinearIntegration / SlackIntegration pattern) ---
     const createTaskEnv: Record<string, string> = {
@@ -258,7 +267,8 @@ export class JiraIntegration extends Construct {
         JIRA_USER_MAPPING_TABLE_NAME: this.userMappingTable.tableName,
         JIRA_WORKSPACE_REGISTRY_TABLE_NAME: this.workspaceRegistryTable.tableName,
       },
-      bundling: commonBundling,
+      // Uses the PDF attachment-screening path (#619) — pdf-parse must stay unbundled.
+      bundling: attachmentScreeningBundling,
     });
     this.projectMappingTable.grantReadData(webhookProcessorFn);
     this.userMappingTable.grantReadData(webhookProcessorFn);

cdk/src/constructs/linear-integration.ts

@@ -195,6 +195,15 @@ export class LinearIntegration extends Construct {
     const commonBundling: lambda.BundlingOptions = {
       externalModules: ['@aws-sdk/*'],
     };
+    // pdf-parse (v2, pdfjs-based) can't be esbuild-bundled — its pdfjs/native
+    // (@napi-rs/canvas) deps break at import (`DOMMatrix is not defined`,
+    // ABCA-745). Ship it unbundled via `nodeModules` so it resolves natively at
+    // runtime. Mirrors TaskApi's attachment-screening bundling (task-api.ts) and
+    // the task-orchestrator. Used by the webhook processor's PDF attachment path.
+    const attachmentScreeningBundling: lambda.BundlingOptions = {
+      ...commonBundling,
+      nodeModules: ['pdf-parse'],
+    };
 
     // --- Task creation environment (matches TaskApi / SlackIntegration pattern) ---
     const createTaskEnv: Record<string, string> = {
@@ -262,7 +271,8 @@ export class LinearIntegration extends Construct {
           MAX_CONCURRENT_TASKS_PER_USER: String(props.maxConcurrentTasksPerUser ?? 10),
         }),
       },
-      bundling: commonBundling,
+      // Uses the PDF attachment-screening path — pdf-parse must stay unbundled.
+      bundling: attachmentScreeningBundling,
     });
     this.projectMappingTable.grantReadData(webhookProcessorFn);
     this.userMappingTable.grantReadData(webhookProcessorFn);

cdk/src/handlers/shared/attachment-screening.ts

@@ -249,13 +249,19 @@ const PDF_MAX_TEXT_BYTES = 1024 * 1024; // 1 MB extracted text cap
 const PDF_EXTRACT_TIMEOUT_MS = 15_000;
 
 async function extractPdfText(content: Buffer, filename: string): Promise<string> {
-  // Dynamic import — pdf-parse is only used for PDF attachments.
-
-  let pdfParseFn: (data: Buffer, options?: { max?: number }) => Promise<{ text: string }>;
+  // pdf-parse v2 (^2.4.5) exposes a `PDFParse` CLASS — `new PDFParse({ data }).getText()` —
+  // NOT the v1 callable default export. It runs headless in the Node Lambda runtime
+  // (the `DOMMatrix`/`Path2D` warnings from pdfjs's optional canvas layer are benign
+  // for text extraction). Two things made this fail before (ABCA-745): the code called
+  // the v1 `pdfParseFn(buf)` shape (undefined for v2), AND the webhook-processor Lambdas
+  // bundled pdf-parse with esbuild instead of shipping it via `nodeModules`, mangling its
+  // pdfjs/native deps at import — see linear-integration/jira-integration bundling.
+  let PDFParse;
   try {
-    // pdf-parse uses a default export; handle both CJS and ESM module shapes.
-    const mod = await import(/* webpackIgnore: true */ 'pdf-parse');
-    pdfParseFn = (mod as any).default ?? mod;
+    // Destructure the class from the dynamic import and let TS infer its type from
+    // the value — a cross-mode `typeof import('pdf-parse').PDFParse` annotation trips
+    // the ESM-vs-CJS dual-`.d.ts` hazard under moduleResolution:nodenext.
+    ({ PDFParse } = await import(/* webpackIgnore: true */ 'pdf-parse'));
   } catch (importErr) {
     logger.error('pdf-parse module could not be imported — PDF screening unavailable', {
       error: importErr instanceof Error ? importErr.message : String(importErr),
@@ -268,13 +274,20 @@ async function extractPdfText(content: Buffer, filename: string): Promise<string
   }
 
   let timeoutId: ReturnType<typeof setTimeout>;
+  // A TypedArray is preferred (pdf-parse transfers ownership to its worker, lowering
+  // main-thread memory). Slice to the exact PDF bytes so a pooled Buffer's backing
+  // ArrayBuffer isn't handed over wholesale.
+  const parser = new PDFParse({ data: new Uint8Array(content.buffer, content.byteOffset, content.byteLength) });
   try {
     const timeoutPromise = new Promise<never>((_, reject) => {
       timeoutId = setTimeout(() => reject(new Error('PDF extraction timed out')), PDF_EXTRACT_TIMEOUT_MS);
     });
 
+    // `first: N` parses only pages 1..N (the v2 page-cap knob; the v1 `max` option
+    // is gone). Screening the first PDF_MAX_PAGES pages is enough to catch injected
+    // instructions without unbounded work on a huge PDF.
     const result = await Promise.race([
-      pdfParseFn(content, { max: PDF_MAX_PAGES }),
+      parser.getText({ first: PDF_MAX_PAGES }),
       timeoutPromise,
     ]);
 
@@ -291,6 +304,8 @@ async function extractPdfText(content: Buffer, filename: string): Promise<string
     );
   } finally {
     clearTimeout(timeoutId!);
+    // Release the pdfjs worker/document (v2 holds native + worker handles).
+    await parser.destroy().catch(() => { /* best-effort teardown */ });
   }
 }
 

cdk/src/types/pdf-parse.d.ts

@@ -20,6 +20,23 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import type { BedrockRuntimeClient } from '@aws-sdk/client-bedrock-runtime';
+
+// pdf-parse v2 exposes a `PDFParse` CLASS (`new PDFParse({data}).getText()`), not
+// the v1 callable default. Mock it at that shape so the PDF-screening tests drive
+// the real v2 contract (see the ABCA-745 regression tests below). pdfjs runs
+// headless in the nodejs24.x Lambda, but ts-jest's CJS transform can't spin its
+// ESM worker under jest — so unit-mock the class and prove the wiring here; the
+// real extraction is validated on the live deploy.
+const pdfGetTextMock = jest.fn();
+const pdfDestroyMock = jest.fn().mockResolvedValue(undefined);
+jest.mock('pdf-parse', () => ({
+  __esModule: true,
+  PDFParse: jest.fn().mockImplementation(() => ({
+    getText: (...args: unknown[]) => pdfGetTextMock(...args),
+    destroy: (...args: unknown[]) => pdfDestroyMock(...args),
+  })),
+}));
+
 import {
   assertImageUploadBytes,
   AttachmentScreeningError,
@@ -318,6 +335,19 @@ describe('screenImage', () => {
 });
 
 describe('screenTextFile', () => {
+  // jest config sets clearMocks:true, which wipes the PDFParse constructor's
+  // implementation + pdfDestroyMock's resolved value before each test. Re-establish
+  // them so `new PDFParse({data})` keeps returning the getText/destroy stubs.
+  beforeEach(() => {
+    (pdfDestroyMock as jest.Mock).mockResolvedValue(undefined);
+    // eslint-disable-next-line @typescript-eslint/no-require-imports -- re-arm the mocked ctor after clearMocks
+    const { PDFParse } = require('pdf-parse') as { PDFParse: jest.Mock };
+    PDFParse.mockImplementation(() => ({
+      getText: (...args: unknown[]) => pdfGetTextMock(...args),
+      destroy: (...args: unknown[]) => pdfDestroyMock(...args),
+    }));
+  });
+
   test('screens plain text content', async () => {
     const config = {
       bedrockClient: mockBedrockPass(),
@@ -360,24 +390,37 @@ describe('screenTextFile', () => {
     }
   });
 
-  test('throws for PDF with no extractable text', async () => {
-    // Mock pdf-parse to return empty text
-    jest.mock('pdf-parse', () => ({
-      __esModule: true,
-      default: jest.fn().mockResolvedValue({ text: '' }),
-    }), { virtual: true });
+  // pdf-parse v2 is mocked at the PDFParse-class level (see the jest.mock at the
+  // top of this file). The prior code called the v1 callable shape
+  // (`pdfParseFn(buf)`), undefined on v2, so every PDF fail-closed as
+  // "processing unavailable" (ABCA-745). These assert the v2 contract:
+  // `new PDFParse({data}).getText()` → `{ text }`. Real end-to-end PDF extraction
+  // is validated on the live deploy (pdfjs runs headless in nodejs24.x; ts-jest's
+  // CJS transform can't drive its ESM worker, so a class mock is the right unit).
 
+  test('extracts and screens a PDF via the v2 PDFParse class (ABCA-745 regression)', async () => {
+    pdfGetTextMock.mockResolvedValueOnce({ text: 'Design spec: add CONTRIBUTORS', total: 1, pages: [] });
     const config = {
       bedrockClient: mockBedrockPass(),
       guardrailId: 'test-guardrail',
       guardrailVersion: '1',
     };
+    const result = await screenTextFile(Buffer.from('%PDF-1.4 real'), 'application/pdf', 'design.pdf', config);
+    expect(result.screening.status).toBe('passed');
+    // The v2 API was actually driven: constructed with { data } + getText called.
+    expect(pdfGetTextMock).toHaveBeenCalledTimes(1);
+    expect(pdfDestroyMock).toHaveBeenCalledTimes(1); // worker released
+  });
 
-    // A minimal PDF-like buffer (pdf-parse is mocked so content doesn't matter)
-    const content = Buffer.from('%PDF-1.4 empty');
-
+  test('throws for PDF with no extractable text', async () => {
+    pdfGetTextMock.mockResolvedValueOnce({ text: '', total: 1, pages: [] });
+    const config = {
+      bedrockClient: mockBedrockPass(),
+      guardrailId: 'test-guardrail',
+      guardrailVersion: '1',
+    };
     await expect(
-      screenTextFile(content, 'application/pdf', 'empty.pdf', config),
+      screenTextFile(Buffer.from('%PDF-1.4 empty'), 'application/pdf', 'empty.pdf', config),
     ).rejects.toThrow(/no extractable text/);
   });