From 65e23fdb45b58047708d0397325648557841363b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:42:24 +0000 Subject: [PATCH 1/4] feat(forms): derive the PDF password badge from the committed bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `data/forms-pdf-manifest.json` carries a `passwordProtected` flag per WA Mental Health Act statutory form, and the form detail page renders a clinician-facing badge from it. The manifest was entirely hand-maintained — no generator, no validator, no gate — and the existing test compared the flag only against itself plus one hardcoded literal for Form 12A, so a wrong flag on any of the other 50 forms would have passed silently. `scripts/build-forms-pdf-manifest.mjs` now derives sha256, bytes and passwordProtected from the committed bytes, offline, with a `--check` mode registered as `check:forms-pdf-manifest` in `verify:cheap:internal` and CI. The flag is derived by ATTEMPTING to open each PDF with an empty user password, not by looking for an `/Encrypt` marker. A PDF encrypted with an owner password but no user password carries `/Encrypt` and still opens freely, so the marker answers a different question than the badge asks. Every committed form happens to agree under both rules, which is exactly why the weaker rule would have survived review: a synthetic owner-password-only fixture is now the discriminating case, and it is the only test that fails if the deriver is ever simplified to a grep. Every error path — unreadable file, truncated body, malformed xref, unparseable encryption dictionary, unsupported revision — yields `passwordProtected: true` and a hard non-zero exit, never a silent `false`. `false` is the assertion that a clinician can open the form, so under-warning is the unsafe direction. `--check` fails on a manifest/bytes disagreement rather than auto-correcting, because the manifest also carries the sha256 provenance record. The generator never synthesises an `officialPdfUrl`, never reorders entries, hard-errors on a PDF with no entry, and now pins the publisher host rather than accepting any https URL. The manifest data itself is unchanged and regenerates byte-identically. The badge reads "Password required to open". It previously read "Password protected"; an interim wording claimed the password comes from the publisher, which nothing here establishes — the bytes prove only that opening requires a non-empty user password, not who holds it or whether a clinician can obtain it. On a statutory form that difference is the difference between a warning and a false errand at the bedside. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL --- .claude/skills/gates/SKILL.md | 2 +- .github/workflows/ci.yml | 7 + CLAUDE.md | 2 +- docs/scripts-index.md | 4 +- package.json | 3 +- scripts/build-forms-pdf-manifest.mjs | 236 ++++++++++++++++++++++ src/components/forms/form-detail-page.tsx | 4 +- tests/forms.test.ts | 78 +++++++ 8 files changed, 330 insertions(+), 6 deletions(-) create mode 100644 scripts/build-forms-pdf-manifest.mjs diff --git a/.claude/skills/gates/SKILL.md b/.claude/skills/gates/SKILL.md index f6ecc8e528..ca6477eb01 100644 --- a/.claude/skills/gates/SKILL.md +++ b/.claude/skills/gates/SKILL.md @@ -26,7 +26,7 @@ Check these before believing any result. for exactly this reason — if installed packages do not match `package-lock.json`, treat any test, lint, or typecheck result as void until `npm ci` has run. Its own failure message says as much. - **`verify:cheap` stops at the first failing check.** Everything after that point never ran. Do not - describe the change as broadly verified when the gate died at check 2 of 37. + describe the change as broadly verified when the gate died at check 2 of 38. - **Changed-file formatting is required in CI but is not part of `verify:cheap`.** A locally green `verify:cheap` can still fail CI on formatting. During iteration, format only task-owned files. Before a push, follow `AGENTS.md`: from an isolated or otherwise fully owned worktree run diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09554b1f1f..4d64804e70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -386,6 +386,13 @@ jobs: if: needs.changes.outputs.static_heavy_changed == 'true' run: npm run check:mha-act-sections + # Derives sha256/bytes/passwordProtected from the committed form PDFs. The badge a + # clinician reads before relying on a statutory form must match the file on disk; + # a hand-maintained flag had nothing checking it against the bytes. + - name: Forms PDF manifest drift + if: needs.changes.outputs.static_heavy_changed == 'true' + run: npm run check:forms-pdf-manifest + - name: Design-system contract if: needs.changes.outputs.static_heavy_changed == 'true' run: npm run check:design-system-contract diff --git a/CLAUDE.md b/CLAUDE.md index a1f2a06a41..a4c7200760 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,7 +142,7 @@ Verification pyramid — run the **smallest gate that covers the change**, then | Gate | What it is | | ----------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `npm run test:focused -- --files ` | Source-only iteration. Fails closed for deleted files and test infrastructure — then run `npm run test`. | -| `npm run verify:cheap` | The broad local gate: 34 static/consistency gates + `lint` + `typecheck` + full offline unit suite; use for cross-module risk, not automatically | +| `npm run verify:cheap` | The broad local gate: 35 static/consistency gates + `lint` + `typecheck` + full offline unit suite; use for cross-module risk, not automatically | | `npm run verify:pr-local` | Risk-routed PR mirror: focused docs/workflow contracts for recognised light scope, fail-closed heavy checks for executable or unknown scope. `-- --dry-run --files ` shows selection. | | `npm run verify:ui` | Chromium production journeys. Run `npm run ensure` first. | | `npm run verify:phone-chrome` | Phone-chrome changes; selects affected owners/journeys before escalating to `verify:ui` | diff --git a/docs/scripts-index.md b/docs/scripts-index.md index b141994388..aea31744a0 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (283 files) and the `package.json` script surface (284 entries), +Curated map of `scripts/` (284 files) and the `package.json` script surface (285 entries), grouped by purpose. This is orientation, not an exhaustive per-file listing — the authoritative command list is `package.json`, and `npm run docs:check-scripts` verifies every `npm run ` referenced in docs resolves to a real script. `npm run docs:update` refreshes the exact counts above. @@ -137,6 +137,8 @@ preserved, in which case generated assets return to their exact pre-review state `build-worker.mjs`, `build-analyze.mjs`, `build-therapies-index.mjs`, `build-cross-mode-differentials-index.mjs`, `build-ranking-snapshot.ts`, +`build-forms-pdf-manifest.mjs` (`check:forms-pdf-manifest` — derives each committed WA MHA form +PDF's sha256, size, and whether opening it needs a user password; offline, fails closed), `generate-site-map.ts`, `generate-brand-assets.ts`, `generate-sample-documents.ts`, `check-sample-extraction.ts`, `optimize-public-images.mjs`. diff --git a/package.json b/package.json index fa3799198c..ba8af15785 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "clean:worktree": "npm run worktrees:report", "verify:preflight": "npm run check:installed-lock-parity && npm run typecheck && npm run verify:cheap && npm run worktrees:report -- --self-test", "verify:cheap": "npm run verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:upload-limit-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:verification-plan && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:skills && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run check:ledger-write-discipline && npm run check:pr-mergeability && npm run sitemap:check && npm run check:repo-awareness-snapshot && npm run docs:check-index && npm run docs:check-inventory && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:mha-act-sections && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:upload-limit-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:verification-plan && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:skills && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run check:ledger-write-discipline && npm run check:pr-mergeability && npm run sitemap:check && npm run check:repo-awareness-snapshot && npm run docs:check-index && npm run docs:check-inventory && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:mha-act-sections && npm run check:forms-pdf-manifest && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:phone-chrome": "node scripts/verify-phone-chrome.mjs", "audit:final-merge": "node scripts/final-merge-audit.mjs", @@ -140,6 +140,7 @@ "check:therapy-data-index": "node scripts/build-therapies-index.mjs --check", "check:cross-mode-index": "node scripts/build-cross-mode-differentials-index.mjs --check", "check:mha-act-sections": "node scripts/build-mha-act-sections.mjs --check", + "check:forms-pdf-manifest": "node scripts/build-forms-pdf-manifest.mjs --check", "check:runtime": "node scripts/run-tsx.mjs scripts/check-runtime.ts", "check:source-catalogue": "node scripts/run-tsx.mjs scripts/check-source-catalogue.ts", "check:installed-lock-parity": "node scripts/check-installed-lock-parity.mjs", diff --git a/scripts/build-forms-pdf-manifest.mjs b/scripts/build-forms-pdf-manifest.mjs new file mode 100644 index 0000000000..2647fba1bb --- /dev/null +++ b/scripts/build-forms-pdf-manifest.mjs @@ -0,0 +1,236 @@ +#!/usr/bin/env node +/** + * build-forms-pdf-manifest.mjs — regenerate `data/forms-pdf-manifest.json` from the + * committed WA Mental Health Act 2014 form PDFs in `public/forms-pdf/`. + * + * node scripts/build-forms-pdf-manifest.mjs # rewrite the manifest + * node scripts/build-forms-pdf-manifest.mjs --check # gate, exit 1 on drift + * + * The manifest feeds `formCatalogDetails` (`src/lib/form-catalog.ts`), which puts the + * `passwordProtected` flag on the Forms detail page as a status badge. A psychiatrist + * reads that badge before relying on the file at the bedside for a statutory step, so + * the flag has to be derived from the bytes rather than maintained by hand — until this + * script existed nothing checked any of the 51 flags against the file it describes. + * + * WHAT `passwordProtected` MEANS + * ----------------------------- + * It means: **opening this PDF requires a user password**. It does NOT mean "the file + * carries an /Encrypt dictionary". Those are different facts. A PDF may be encrypted + * with an owner password (restricting printing or editing) and an EMPTY user password; + * it carries /Encrypt and still opens freely for any reader. Deriving the flag from the + * presence of /Encrypt would mislabel such a file, and a clinician who finds that a + * "password protected" form opens fine learns to disbelieve the badge on the files where + * it is true. + * + * The derivation is therefore behavioural: attempt to open the document with the empty + * user password. The flag is true only when that attempt is refused for a password + * reason (pdf.js `PasswordException`, either NEED_PASSWORD or INCORRECT_PASSWORD). + * + * FAILURE DIRECTION + * ----------------- + * `false` is an assertion that the clinician can open the file. Under-warning is the + * unsafe direction: planning to complete a Form 10A and discovering at the bedside that + * it will not open is a workflow failure at a time-critical statutory step. So any + * unreadable file, malformed PDF, unparseable encryption dictionary, or unsupported + * encryption revision yields `passwordProtected: true` AND a hard failure of this + * script. No error path may ever produce `false`. + * + * PROVENANCE + * ---------- + * Only `sha256`, `bytes` and `passwordProtected` are derived. `code`, `localPath`, + * `officialPdfUrl`, the entry order, and the top-level `generatedAt` / + * `sourceRegisterUrl` are carried over verbatim from the committed manifest. This script + * is fully offline and must never fetch anything: it cannot know a form's official + * publisher URL, so a PDF on disk with no existing manifest entry is a hard error rather + * than an invention. `--check` likewise fails on any disagreement instead of + * auto-correcting — the manifest also carries the sha256 provenance record, and silently + * rewriting it would erase the evidence that the file on disk changed. + */ +import { createHash } from "node:crypto"; +import { readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), ".."); +const manifestPath = join(root, "data", "forms-pdf-manifest.json"); +const pdfDirectory = join(root, "public", "forms-pdf"); +const pdfUrlPrefix = "/forms-pdf/"; +const officialPdfUrlPrefix = "https://www.chiefpsychiatrist.wa.gov.au/"; + +/** pdf.js in Node: the legacy build runs without a DOM and without a worker thread. */ +async function loadPdfjs() { + return import("pdfjs-dist/legacy/build/pdf.mjs"); +} + +/** + * Does opening `bytes` require a user password? + * + * Returns `{ passwordProtected, failure }`. `failure` is non-null when the file could + * not be classified at all — a corrupt header, a truncated body, an encryption + * dictionary pdf.js cannot parse. In that case `passwordProtected` is still `true`: the + * conservative answer is the one that warns, and the caller turns `failure` into a hard + * exit so a human resolves it rather than shipping a guess. + */ +export async function derivePdfPasswordProtection(bytes, label) { + const pdfjs = await loadPdfjs(); + // A fresh copy per call: pdf.js transfers/detaches the buffer it is handed. + const task = pdfjs.getDocument({ + data: new Uint8Array(bytes), + password: "", + isEvalSupported: false, + useSystemFonts: false, + disableFontFace: true, + verbosity: 0, + }); + try { + await task.promise; + return { passwordProtected: false, failure: null }; + } catch (error) { + if (error instanceof pdfjs.PasswordException) { + // NEED_PASSWORD (no password supplied is not enough) and INCORRECT_PASSWORD (the + // empty string is not the user password) both mean the same thing to a clinician. + return { passwordProtected: true, failure: null }; + } + const reason = error instanceof Error ? `${error.name}: ${error.message}` : String(error); + return { + passwordProtected: true, + failure: `${label} could not be opened or classified (${reason}). Recorded conservatively as password protected.`, + }; + } finally { + // Always tear the loading task down, including on the success path, so a failed run + // cannot leave pdf.js work pending and hang the process. + await task.destroy().catch(() => {}); + } +} + +const sha256 = (bytes) => createHash("sha256").update(bytes).digest("hex"); + +/** Committed PDF filenames, sorted so a run is deterministic across filesystems. */ +export function listFormPdfFilenames() { + return readdirSync(pdfDirectory) + .filter((name) => name.toLowerCase().endsWith(".pdf")) + .sort(); +} + +/** + * Rebuild every asset entry, preserving the committed manifest's entry order and its + * non-derived fields. Throws when the manifest and the directory disagree about which + * forms exist — neither side may be invented from the other. + */ +export async function buildManifest(existing) { + if (!Array.isArray(existing?.assets)) { + throw new Error(`Malformed manifest: expected an "assets" array in ${manifestPath}.`); + } + const byLocalPath = new Map(); + for (const asset of existing.assets) { + if (typeof asset?.localPath !== "string" || !asset.localPath.startsWith(pdfUrlPrefix)) { + throw new Error( + `Malformed manifest entry (localPath must start with "${pdfUrlPrefix}"): ${JSON.stringify(asset)}`, + ); + } + if (typeof asset.code !== "string" || asset.code.trim() === "") { + throw new Error(`Malformed manifest entry (missing "code"): ${JSON.stringify(asset)}`); + } + // Host-pinned, not merely https. These are statutory instruments; the manifest URL is + // the provenance record a reader follows to check the committed bytes against the + // publisher, and any other host would send them somewhere this repo cannot vouch for. + // `tests/forms.test.ts` pins the same host from the other direction. + if (typeof asset.officialPdfUrl !== "string" || !asset.officialPdfUrl.startsWith(officialPdfUrlPrefix)) { + throw new Error( + `Malformed manifest entry ("officialPdfUrl" must start with "${officialPdfUrlPrefix}"): ${JSON.stringify(asset)}`, + ); + } + if (byLocalPath.has(asset.localPath)) { + throw new Error(`Duplicate manifest entry for ${asset.localPath}.`); + } + byLocalPath.set(asset.localPath, asset); + } + + const onDisk = new Set(listFormPdfFilenames().map((name) => `${pdfUrlPrefix}${name}`)); + const undocumented = [...onDisk].filter((localPath) => !byLocalPath.has(localPath)); + if (undocumented.length > 0) { + // This script is offline by contract and cannot discover a form's publisher URL, so + // there is no honest entry it could synthesise here. + throw new Error( + `Committed PDF(s) with no manifest entry: ${undocumented.join(", ")}. ` + + "Add the entry by hand with its official chiefpsychiatrist.wa.gov.au URL, then re-run this script.", + ); + } + const missing = [...byLocalPath.keys()].filter((localPath) => !onDisk.has(localPath)); + if (missing.length > 0) { + throw new Error(`Manifest entr(ies) with no committed PDF: ${missing.join(", ")}.`); + } + + const failures = []; + const assets = []; + for (const asset of existing.assets) { + const filePath = join(pdfDirectory, asset.localPath.slice(pdfUrlPrefix.length)); + let bytes; + try { + bytes = readFileSync(filePath); + } catch (error) { + // Unreadable is a hard failure, and the flag still fails closed. + failures.push( + `Form ${asset.code}: cannot read ${asset.localPath} (${error instanceof Error ? error.message : String(error)}).`, + ); + assets.push({ ...asset, sha256: "", bytes: 0, passwordProtected: true }); + continue; + } + const { passwordProtected, failure } = await derivePdfPasswordProtection(bytes, `Form ${asset.code}`); + if (failure) failures.push(failure); + assets.push({ + code: asset.code, + localPath: asset.localPath, + officialPdfUrl: asset.officialPdfUrl, + sha256: sha256(bytes), + bytes: bytes.byteLength, + passwordProtected, + }); + } + + return { manifest: { ...existing, assets }, failures }; +} + +/** Exact committed bytes: Prettier's JSON output for this file is 2-space + newline. */ +export function serializeManifest(manifest) { + return `${JSON.stringify(manifest, null, 2)}\n`; +} + +async function run({ checkOnly }) { + const currentText = readFileSync(manifestPath, "utf8"); + const { manifest, failures } = await buildManifest(JSON.parse(currentText)); + if (failures.length > 0) { + throw new Error( + `Forms PDF manifest could not be derived from the committed bytes:\n - ${failures.join("\n - ")}`, + ); + } + const expected = serializeManifest(manifest); + const protectedCount = manifest.assets.filter((asset) => asset.passwordProtected).length; + + if (checkOnly) { + if (expected !== currentText) { + throw new Error( + `data/forms-pdf-manifest.json disagrees with the committed PDFs. ` + + "Inspect the difference before regenerating — sha256 is a provenance record, so a changed hash means the file on disk changed. " + + "Re-run `node scripts/build-forms-pdf-manifest.mjs` once the change is understood.", + ); + } + process.stdout.write( + `Forms PDF manifest is current (${manifest.assets.length} PDFs, ${protectedCount} require a user password).\n`, + ); + return; + } + + writeFileSync(manifestPath, expected); + process.stdout.write( + `Wrote ${manifest.assets.length} entries to data/forms-pdf-manifest.json ` + + `(${protectedCount} require a user password).\n`, + ); +} + +// `file://${process.argv[1]}` is not a valid comparison on Windows; use the same +// cross-platform conversion as this repository's other directly invoked scripts. +const isEntrypoint = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; +if (isEntrypoint) { + await run({ checkOnly: process.argv.includes("--check") }); +} diff --git a/src/components/forms/form-detail-page.tsx b/src/components/forms/form-detail-page.tsx index 346d35d982..c428756663 100644 --- a/src/components/forms/form-detail-page.tsx +++ b/src/components/forms/form-detail-page.tsx @@ -847,7 +847,7 @@ export function FormDetailPage({ form }: { form: FormRecord }) { details?.officialPdfPasswordProtected ? toneWarning : toneNeutral, )} > - {details?.officialPdfPasswordProtected ? "Password protected" : "Check source"} + {details?.officialPdfPasswordProtected ? "Password required to open" : "Check source"}
- {details?.officialPdfPasswordProtected ? "Password protected" : "Check source"} + {details?.officialPdfPasswordProtected ? "Password required to open" : "Check source"}
diff --git a/tests/forms.test.ts b/tests/forms.test.ts index 49c2fab740..019bc2e768 100644 --- a/tests/forms.test.ts +++ b/tests/forms.test.ts @@ -2,10 +2,12 @@ import { createHash } from "node:crypto"; import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; +import PDFDocument from "pdfkit"; import { describe, expect, it } from "vitest"; import formsActSectionCues from "../data/forms-act-section-cues.json"; import formsPdfManifest from "../data/forms-pdf-manifest.json"; +import { derivePdfPasswordProtection } from "../scripts/build-forms-pdf-manifest.mjs"; import { formDetailsClipboardText } from "@/components/forms/form-detail-page"; import { formCatalogDetails } from "@/lib/form-catalog"; @@ -191,6 +193,82 @@ describe("psychiatry form records", () => { expect(formCatalogDetails(form12a!)?.officialPdfPasswordProtected).toBe(false); }); + // The manifest flag is what the Forms detail page turns into a badge a psychiatrist + // reads before relying on the file at the bedside. The assertions above only compare + // the manifest against the catalogue that reads it, so a wrong flag would agree with + // itself. This pins every flag to the generated contract: the committed bytes. + it("derives every manifest passwordProtected flag from the committed PDF bytes", async () => { + const assets = ( + formsPdfManifest as { assets: Array<{ code: string; localPath: string; passwordProtected: boolean }> } + ).assets; + expect(assets).toHaveLength(51); + for (const asset of assets) { + const bytes = readFileSync(join(process.cwd(), "public", asset.localPath.replace(/^\//, ""))); + // "Requires a user password to open", not "carries an /Encrypt dictionary": a PDF + // with an owner password and an empty user password is encrypted yet opens freely, + // and badging that file as protected teaches clinicians to ignore the warning on + // the files where it is true. + const derived = await derivePdfPasswordProtection(bytes, `Form ${asset.code}`); + expect(derived.failure, asset.code).toBeNull(); + expect(derived.passwordProtected, asset.code).toBe(asset.passwordProtected); + } + // Form 12A is the one readable form on the register, and other assertions in this + // file extract its text. Keep that asymmetry visible rather than implied by a loop. + expect(assets.filter((asset) => !asset.passwordProtected).map((asset) => asset.code)).toEqual(["12A"]); + }); + + it("fails closed to password protected when a form PDF cannot be classified", async () => { + // `false` asserts the clinician can open the file, so under-warning is the unsafe + // direction: planning a Form 10A and finding at the bedside that it will not open is + // a workflow failure at a time-critical statutory step. Every unclassifiable input + // must therefore report `true` AND surface a failure the generator turns into a hard + // exit — never a silent `false`. + const readable = readFileSync(join(process.cwd(), "public", "forms-pdf", "form-12a.pdf")); + expect(await derivePdfPasswordProtection(readable, "Form 12A")).toEqual({ + passwordProtected: false, + failure: null, + }); + + for (const [label, corrupt] of [ + ["truncated", readable.subarray(0, 2048)], + ["not a pdf", Buffer.from("%PDF-1.7 this is not a document")], + ["empty", Buffer.alloc(0)], + ] as Array<[string, Buffer | Uint8Array]>) { + const derived = await derivePdfPasswordProtection(corrupt, `corrupt fixture (${label})`); + expect(derived.passwordProtected, label).toBe(true); + expect(derived.failure, label).toContain("could not be opened or classified"); + } + }); + + it("reports a PDF that carries /Encrypt but opens with an empty user password as not password protected", async () => { + // The whole reason this flag is derived by ATTEMPTING to open the file, rather than by + // looking for an /Encrypt marker, is that the two answers can differ: a PDF encrypted + // with an owner password but no user password carries /Encrypt and still opens freely. + // + // Every committed form happens to agree under both rules — 50 carry /Encrypt and refuse + // an empty user password, and form-12a.pdf carries neither — so nothing in this corpus + // would catch a future "simplification" of the deriver into a grep for /Encrypt. This + // synthetic fixture is the discriminating case, and it is the only test that fails if + // that shortcut is ever taken. + const ownerPasswordOnly = await new Promise((resolve, reject) => { + const doc = new PDFDocument({ ownerPassword: "owner-only-secret", permissions: { printing: "highResolution" } }); + const chunks: Buffer[] = []; + doc.on("data", (chunk: Buffer) => chunks.push(chunk)); + doc.on("end", () => resolve(Buffer.concat(chunks))); + doc.on("error", reject); + doc.text("Owner password only; the user password is empty."); + doc.end(); + }); + + // Precondition: the fixture really is the confusing shape, not merely an unencrypted file. + expect(ownerPasswordOnly.includes("/Encrypt")).toBe(true); + + expect(await derivePdfPasswordProtection(ownerPasswordOnly, "owner-password-only fixture")).toEqual({ + passwordProtected: false, + failure: null, + }); + }); + it("populates Form 12A statutory Authority and Criteria priority facts from readable approved PDF", () => { const form12a = getFormRecord("form-12a"); expect(form12a).toBeTruthy(); From 60f5e8ead33818b2f157b171c748c15bfe61fa0c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:58:42 +0000 Subject: [PATCH 2/4] issues: queue the #9P4XAE closure request Append-only inbox request recording that the forms PDF manifest now derives its password flag from committed bytes and gates it. Separate commit so it stays independently revertible from the product change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL --- .../e49dd511-974f-4f7e-af2c-e399b1c147ec.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 docs/outstanding-issues-inbox/e49dd511-974f-4f7e-af2c-e399b1c147ec.json diff --git a/docs/outstanding-issues-inbox/e49dd511-974f-4f7e-af2c-e399b1c147ec.json b/docs/outstanding-issues-inbox/e49dd511-974f-4f7e-af2c-e399b1c147ec.json new file mode 100644 index 0000000000..50abc42b22 --- /dev/null +++ b/docs/outstanding-issues-inbox/e49dd511-974f-4f7e-af2c-e399b1c147ec.json @@ -0,0 +1,11 @@ +{ + "version": 2, + "id": "e49dd511-974f-4f7e-af2c-e399b1c147ec", + "createdOn": "2026-09-02", + "action": "done", + "payload": { + "id": "#9P4XAE", + "outcome": "Resolved 2026-09-02 on branch claude/form-12a-warning. The 12A data defect was already corrected before this work: data/forms-pdf-manifest.json recorded passwordProtected false and tests/forms.test.ts pinned it. The outstanding half — a measured generation/validation workflow deriving the flag from committed PDF bytes — is what landed. scripts/build-forms-pdf-manifest.mjs derives sha256, bytes and passwordProtected offline from public/forms-pdf/, with a --check mode wired into verify:cheap:internal and CI as check:forms-pdf-manifest. The flag is derived by attempting to open each PDF with an empty user password, not by looking for an /Encrypt marker, because a PDF with an owner password and an empty user password carries /Encrypt yet opens freely. Every error path fails closed to passwordProtected true plus a hard non-zero exit. Regeneration reproduces the committed manifest byte for byte. tests/forms.test.ts now asserts the generated contract, keeps every prior assertion, and adds a synthetic owner-password-only fixture that is the only test which fails if the deriver is ever simplified to an /Encrypt grep.", + "baseRowFingerprint": "e85f7e6b68571acafd0cfe331ea7f28241e56a2c997cd8e52f5a8e503344fd5c" + } +} From 111a5ee089916c61472379227e3a4e4cf6d8f2c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 05:43:54 +0000 Subject: [PATCH 3/4] fix(forms): shorten the password badge so it stops crowding the document title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified in Chromium at a 320px viewport, A/B against the same page with only this string changed. In the PDF row the badge and the title share a two-column grid, and the badge is not allowed to shrink: "Password protected" (main) -> title/subtitle track 70px "Password required to open" -> title/subtitle track 35px "Password required" -> title/subtitle track 78px At 35px the document title rendered as "Rec…" and its publisher line as "Offic…". That is a real regression against main, so the longer wording is not worth its cost: it squeezed the name of the statutory form in order to describe the lock on it. "Password required" keeps everything the bytes establish — opening the file needs a non-empty user password — and still makes no claim about who holds it, which was the reason the earlier "…from the publisher" wording was rejected. It is also one character shorter than the string on main, so the row is slightly better off than before this branch. No layout or tone class changed; the fix is entirely in the wording. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL --- src/components/forms/form-detail-page.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/forms/form-detail-page.tsx b/src/components/forms/form-detail-page.tsx index c428756663..a130469287 100644 --- a/src/components/forms/form-detail-page.tsx +++ b/src/components/forms/form-detail-page.tsx @@ -847,7 +847,7 @@ export function FormDetailPage({ form }: { form: FormRecord }) { details?.officialPdfPasswordProtected ? toneWarning : toneNeutral, )} > - {details?.officialPdfPasswordProtected ? "Password required to open" : "Check source"} + {details?.officialPdfPasswordProtected ? "Password required" : "Check source"}
- {details?.officialPdfPasswordProtected ? "Password required to open" : "Check source"} + {details?.officialPdfPasswordProtected ? "Password required" : "Check source"}
From f938322e42e00388e0fa5c29c39908e65d09e093 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 06:06:13 +0000 Subject: [PATCH 4/4] fix(forms): preserve manifest fields the generator does not derive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review, verified. `buildManifest` rebuilt each asset by enumerating six keys, so any other field an asset carried was dropped on regeneration. The generator computes exactly three fields — sha256, bytes, passwordProtected — and everything else is provenance it cannot recompute. This script is offline by contract, so a dropped `officialPdfUrl` could never be restored from the PDFs themselves. The failure mode is quiet and then destructive: `--check` reports drift, and the regeneration it instructs the operator to run erases the field for good. The code already disagreed with itself, which is the clearest evidence the finding is real. The unreadable-file path a few lines above spreads `...asset` and only overrides the derived fields; the success path did not. They now match. Not hypothetical: an `editingRestricted` fact derived from the PDFs' /P permission bits is already a queued follow-up against this same manifest, so the first field added would have hit this. Pinned by a test that adds two extra properties to an asset and asserts they survive while the three derived fields stay authoritative. Negative control: restoring the enumerated-keys version fails it with `expected undefined to be '2026-09-02'`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL --- scripts/build-forms-pdf-manifest.mjs | 11 ++++++++--- tests/forms.test.ts | 29 +++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/scripts/build-forms-pdf-manifest.mjs b/scripts/build-forms-pdf-manifest.mjs index 2647fba1bb..3ad79e3b1a 100644 --- a/scripts/build-forms-pdf-manifest.mjs +++ b/scripts/build-forms-pdf-manifest.mjs @@ -178,10 +178,15 @@ export async function buildManifest(existing) { } const { passwordProtected, failure } = await derivePdfPasswordProtection(bytes, `Form ${asset.code}`); if (failure) failures.push(failure); + // Spread first, then override ONLY the three derived fields. Enumerating the + // known keys instead would silently drop any other field an asset carries, and + // the drop would be invisible until it mattered: `--check` would report drift, + // and the regeneration it tells the operator to run would erase the field for + // good. The provenance fields this generator cannot compute — `officialPdfUrl` + // above all — are exactly the ones that must survive it untouched. The + // unreadable-file path above already spreads; these two must not disagree. assets.push({ - code: asset.code, - localPath: asset.localPath, - officialPdfUrl: asset.officialPdfUrl, + ...asset, sha256: sha256(bytes), bytes: bytes.byteLength, passwordProtected, diff --git a/tests/forms.test.ts b/tests/forms.test.ts index 019bc2e768..59a1961c21 100644 --- a/tests/forms.test.ts +++ b/tests/forms.test.ts @@ -7,7 +7,7 @@ import { describe, expect, it } from "vitest"; import formsActSectionCues from "../data/forms-act-section-cues.json"; import formsPdfManifest from "../data/forms-pdf-manifest.json"; -import { derivePdfPasswordProtection } from "../scripts/build-forms-pdf-manifest.mjs"; +import { buildManifest, derivePdfPasswordProtection } from "../scripts/build-forms-pdf-manifest.mjs"; import { formDetailsClipboardText } from "@/components/forms/form-detail-page"; import { formCatalogDetails } from "@/lib/form-catalog"; @@ -269,6 +269,33 @@ describe("psychiatry form records", () => { }); }); + it("preserves manifest fields it does not derive when regenerating", async () => { + // The generator computes exactly three fields — sha256, bytes and passwordProtected. + // Everything else is provenance it cannot recompute: `officialPdfUrl` is the link a + // reader follows to check the committed bytes against the publisher, and this script + // is offline by contract, so a dropped URL could never be restored from the PDFs. + // + // Enumerating known keys when rebuilding an asset would drop any other field silently, + // and the failure mode is quiet then destructive: `--check` reports drift, and the + // regeneration it instructs the operator to run erases the field permanently. Adding a + // field to this manifest is a live prospect (an `editingRestricted` fact is already a + // queued follow-up), so this pins preservation before that lands rather than after. + const existing = JSON.parse(readFileSync(join(process.cwd(), "data", "forms-pdf-manifest.json"), "utf8")) as { + assets: Array>; + }; + const probe = { ...existing.assets[0], reviewedAt: "2026-09-02", editingRestricted: true }; + const { manifest } = await buildManifest({ ...existing, assets: [probe, ...existing.assets.slice(1)] }); + const rebuilt = manifest.assets[0] as Record; + + expect(rebuilt.reviewedAt).toBe("2026-09-02"); + expect(rebuilt.editingRestricted).toBe(true); + // The derived fields are still authoritative — preservation must not shadow them. + expect(rebuilt.officialPdfUrl).toBe(existing.assets[0].officialPdfUrl); + expect(rebuilt.sha256).toBe(existing.assets[0].sha256); + expect(rebuilt.bytes).toBe(existing.assets[0].bytes); + expect(rebuilt.passwordProtected).toBe(existing.assets[0].passwordProtected); + }); + it("populates Form 12A statutory Authority and Criteria priority facts from readable approved PDF", () => { const form12a = getFormRecord("form-12a"); expect(form12a).toBeTruthy();