fix(pdf): upgrade pdfjs-dist past GHSA-hq66-cqwq-w95j (5.7.284 → 6.3.289)

[mrsibe]

Closes #121.

pdfjs-dist resolved to 5.7.284, inside the affected range of GHSA-hq66-cqwq-w95j / CVE-2026-16633 (>=5.6.83 <6.2.108, fixed 6.2.108). Untrusted documents are this app's normal input, so a known-high in that path is a v1.4 release blocker.

Reachability: the current exploit path is not reachable under the inspected call sites

That wording is deliberate — it is a statement about our code, not a claim that the library is safe. The distinction is the whole reason this is worth writing down.

• Both paths call getDocument with no scripting option, and neither constructs an AnnotationLayer nor imports pdfjs-dist/web/*.
• PDFScriptingManager — the component that actually executes document JavaScript — exists only in the viewer build. grep -c PDFScriptingManager against the API build build/pdf.mjs is 0, and bare pdfjs-dist resolves to main: build/pdf.mjs. The only other pdfjs import in the repo is the parsing worker.
• In the API build the annotation layer's _bindJSAction only binds DOM listeners that dispatch into a sandbox the viewer is expected to provide.

So the path is not reachable today — because of what we do or don't construct.

Correction to this issue's own acceptance

Item 2 asked for enableScripting: false to be passed explicitly on both getDocument calls. That is not implementable, and I did not force it:

src/main/services/loaders/PdfLoader.ts: error TS2339 / TS2353:
  'enableScripting' does not exist in type 'DocumentInitParameters'

enableScripting is declared on AnnotationLayer, AnnotationLayerBuilder and PDFViewer — it is not a getDocument parameter. The advisory's workaround ("set enableScripting to false or set a CSP") targets the viewer, which this app does not use. There is no switch to flip at our call sites.

So the mitigation is structural, and it is now a test instead of a one-time inspection: test/pdfjsScriptingBoundary.test.ts fails if any source file

• imports the viewer build (pdfjs-dist/web/* / pdf_viewer), where the scripting manager lives,
• constructs AnnotationLayer / AnnotationLayerBuilder / PDFScriptingManager / ScriptingManager, or
• sets enableScripting: true.

It carries a self-check (the guard actually scans the source tree) so it cannot pass by scanning nothing, and I confirmed it fails when a new AnnotationLayer() probe is injected. That is what turns "we happen not to do this" into a CI-enforced invariant — and it is what makes the "not reachable" conclusion above durable rather than historical.

The CSP item is already satisfied

The issue asked whether a CSP is additionally warranted for the reader. It is already in place, in src/renderer/index.html:

default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; ... worker-src 'self' blob:;

script-src 'self' with no 'unsafe-inline' and no 'unsafe-eval'. The advisory requires "no CSP for disallowing script-src"; that condition is not met. So the renderer — the process that renders untrusted documents — has two independent barriers: no scripting machinery is constructed, and injected script would be blocked by CSP anyway.

The upgrade: 5.7.284 → 6.3.289

The entire API breakage is one moved method. destroy() is gone from PDFDocumentProxy (which keeps cleanup()) and now lives on PDFDocumentLoadingTask. Three mechanical call sites:

Site	Change
PdfLoader.ts	destroy the loading task it already holds
PdfSourceReader.tsx	hold the loading task for teardown instead of the document; clear it before destroying on the cancelled path so unmount cannot destroy the same task twice

Nothing else broke. engines on 6.x is >=22.13.0 \|\| >=24; CI and CONTRIBUTING both pin Node 24.

Rollback plan

Mechanically two steps: restore ^5.x, revert the three destroy() call sites.

It is deliberately not the default, because it re-opens this issue — the version would be inside a known-high range again. The structural guard and the CSP survive a rollback, so the residual risk is specifically "a known-high version ships", which is what the release policy forbids. A rollback therefore needs an explicit maintainer decision and a note in the release, not a quiet revert.

Acceptance

Criterion	Status
Exploit conditions re-confirmed on both paths, recorded	Done — above, and readable in the code comments at both call sites
enableScripting: false on both getDocument calls	Not implementable — corrected above; replaced by the structural boundary guard
CSP evaluation for the reader	Already present and sufficient — quoted above
Upgrade to >=6.2.108	Done — 6.3.289
Reader regression (render, text layer, bbox highlight, worker, zoom)	Manual QA — see below
main PdfLoader regression	Covered — packaged smoke test parses the fixture; parser tests pass
Packaged artifact on Windows and macOS	CI on this PR
npm audit clean for pdfjs-dist	Done — found 0 vulnerabilities
Rollback plan recorded	Done — above

Verification

• npm run typecheck — all three projects
• npm test — 258 pass / 0 fail (4 new: the boundary guard)
• npm run build:unpack && npm run smoke:packaged — executed on the packaged app, 20/20 checks, including pdfjs-dist loads and polyfilled DOMMatrix (the historical startup crash point) and a real PDF import extracts text fixture parse under 6.3.289:

  [SmokeTest] ok   pdfjs-dist loads and polyfilled DOMMatrix from @napi-rs/canvas
  [SmokeTest] ok   PDF import extracts text (pdfjs-dist)
  [SmokeTest] PASS - 20 checks passed
  

• npm audit --omit=dev --registry=https://registry.npmjs.org — found 0 vulnerabilities
• npm run check:design, npm run lint (0 errors, 107 warning baseline unchanged), npx prettier --check

Not verified

The reader's rendering path — canvas render, text-layer selection (#73 excerpts depend on it), bbox highlighting, worker loading, zoom — needs a real window and remains manual QA. Typecheck and the packaged build cover it structurally, and PdfSourceReader had to change for the destroy() move, so this is the one place worth a human pass before release. CI covers the packaged main-process parse on Windows and macOS.

isEvalSupported (font-data eval) is a different vector from this CVE and is not addressed here.