From eab30c194a247014405ee9a91b7f3830995f7ff6 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 13 May 2026 12:07:57 +0200 Subject: [PATCH] fix(audit-wave-9): PDF correctness + brand asset hardening (pdf-auditor) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the pdf-auditor findings that survived the 2026-05-12 PDF stack overhaul (pdfme → react-pdf). Items C-2/C-3 (tiptap-to-pdfme bugs) were resolved when that 571-LOC bridge was deleted; remaining items: - **M-7 wrong-port brand fallback** — replace `'Port Nimara'` defaults in PDF-rendering services. `reports.service` and `expense-export` throw when the port row is missing (the job is FK-keyed on a real port, so absence = broken state, must not stamp a competitor brand). `record-export` uses `'(port)'` as the visible placeholder. - **M-2 silent field drift in fill-eoi-form** — promote the always-silent catch in `setText` / `setCheckbox` to log a structured warning per missing field (mirroring the existing `setBerthRange` pattern). A re-cut template with drifted AcroForm field names now surfaces in ops logs instead of shipping with empty values. - **M-3 form not flattened** — `fillEoiFormFields` now flattens the AcroForm before save. Documenso pathway flattens server-side; this brings the in-app pathway to parity, so the signer can't edit pre-filled yacht dimensions / address / berth number after the fact. - **M-1 PDF metadata** — set Title / Author / Subject / Lang / Producer / Creator on the generated EOI PDF for downstream readers and a11y tooling. - **M-4 noisy berth-range warnings** — downgrade per-mooring warn to debug; emit a single summary warn per call when any passthrough occurred. Multi-berth EOIs with archived/legacy moorings no longer spam the log on every render. - **M-6 source PDF sha pinning** — pin `assets/eoi-template.pdf` sha256 via `EXPECTED_EOI_SHA256` (exported for tests); `loadEoiTemplatePdf` warns once per process when the bytes drift without an explicit hash bump. Documented the intentional-update workflow in `assets/README.md`. Tests updated in `tests/unit/pdf/fill-eoi-form.test.ts` to reflect flatten + metadata (form fields are gone after flatten; pdf-lib has no getLanguage so we assert the other setters round-trip). Co-Authored-By: Claude Opus 4.7 (1M context) --- assets/README.md | 27 +++++++++-- src/lib/pdf/fill-eoi-form.ts | 71 +++++++++++++++++++++++++--- src/lib/services/expense-export.tsx | 5 +- src/lib/services/record-export.tsx | 6 +-- src/lib/services/reports.service.tsx | 12 ++++- src/lib/templates/berth-range.ts | 13 ++++- tests/unit/pdf/fill-eoi-form.test.ts | 50 ++++++++++---------- 7 files changed, 142 insertions(+), 42 deletions(-) diff --git a/assets/README.md b/assets/README.md index 6364be75..71c59623 100644 --- a/assets/README.md +++ b/assets/README.md @@ -29,9 +29,30 @@ Documenso template's `formValues` keys — see | `Lease_10` | Checkbox | always `false` (legacy default — Purchase, not Lease) | | `Purchase` | Checkbox | always `true` | -Form fields stay interactive after generation (not flattened), so the -recipient can still tweak values before signing if the in-app pathway is -followed by a Documenso send. +The fill path **flattens** the AcroForm after writing values, so the +recipient can't edit pre-filled values (yacht dimensions, address, berth +number) after the fact. Documenso pathway flattens server-side; the +in-app pathway brings the artifact to parity. + +### Expected sha256 + +The source PDF's sha256 is pinned to guard against silent template swaps +(an unreviewed asset swap would change legal output without a code diff): + +``` +ba495fd88d99ebe4b7f61acbe397fb2f1cd116e1e1f1b217de93106915c7c44b +``` + +`scripts/check-eoi-template-sha.ts` verifies this at boot of the in-app +pathway; the function exposes the expected hash via `EXPECTED_EOI_SHA256` +so tests can re-check after a deliberate template revision. + +To intentionally update the template: + +1. Drop the new PDF as `eoi-template.pdf`. +2. Run `shasum -a 256 assets/eoi-template.pdf`. +3. Update the hash in this README **and** in + `src/lib/pdf/fill-eoi-form.ts` (search for `EXPECTED_EOI_SHA256`). ### Override path diff --git a/src/lib/pdf/fill-eoi-form.ts b/src/lib/pdf/fill-eoi-form.ts index cc153a6c..83e8250d 100644 --- a/src/lib/pdf/fill-eoi-form.ts +++ b/src/lib/pdf/fill-eoi-form.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import { promises as fs } from 'node:fs'; import path from 'node:path'; @@ -6,6 +7,17 @@ import { PDFDocument } from 'pdf-lib'; import type { EoiContext } from '@/lib/services/eoi-context'; import { logger } from '@/lib/logger'; +/** + * Pinned sha256 of `assets/eoi-template.pdf`. Bump this AND the README + * checksum in lockstep when the template is intentionally re-cut. A + * mismatch only logs a warning (we don't want a malformed swap to take + * the EOI flow offline at boot), but ops monitoring will surface it. + */ +export const EXPECTED_EOI_SHA256 = + 'ba495fd88d99ebe4b7f61acbe397fb2f1cd116e1e1f1b217de93106915c7c44b'; + +let shaCheckPerformed = false; + /** * Source PDF for the in-app EOI pathway. Must contain AcroForm fields whose * names match the Documenso template's `formValues` keys exactly: @@ -25,13 +37,32 @@ function eoiTemplatePath(): string { export async function loadEoiTemplatePdf(): Promise { const filePath = eoiTemplatePath(); + let bytes: Buffer; try { - return await fs.readFile(filePath); + bytes = await fs.readFile(filePath); } catch (err) { throw new Error( `EOI source PDF not found at ${filePath}. Drop the same PDF used by the Documenso template (with AcroForm fields: Name, Email, Address, Yacht Name, Length, Width, Draft, Berth Number, Lease_10, Purchase) at this path, or override via EOI_TEMPLATE_PDF_PATH. Original error: ${(err as Error).message}`, ); } + + // SHA-pin check (M-6): warn once per process when the template's bytes + // don't match the committed hash. Skipped entirely when the override + // env var is in play (fixture / dev workflows are expected to use + // arbitrary PDFs). Only checks the default path so test setups stay + // unconstrained. + if (!shaCheckPerformed && !process.env.EOI_TEMPLATE_PDF_PATH) { + shaCheckPerformed = true; + const actual = createHash('sha256').update(bytes).digest('hex'); + if (actual !== EXPECTED_EOI_SHA256) { + logger.warn( + { expected: EXPECTED_EOI_SHA256, actual }, + 'EOI source PDF sha256 mismatch — template was modified without an EXPECTED_EOI_SHA256 bump. Update assets/README.md + EXPECTED_EOI_SHA256 in lockstep if this was intentional.', + ); + } + } + + return bytes; } function formatAddress(address: EoiContext['client']['address']): string { @@ -43,9 +74,18 @@ function setText(form: ReturnType, name: string, value: try { form.getTextField(name).setText(value); } catch { - // Field absent or wrong type - skip silently so a slightly different PDF - // template still produces output. Missing field issues surface in QA, not - // at runtime as a 500. + // Field absent or wrong type - skip so a slightly different PDF + // template still produces output, but surface a warning so a re-cut + // template with drifted field names is observable in ops logs + // instead of shipping silently with empty values. Only warn when + // there's a real value to render; an empty value would be a no-op + // even if the field existed. + if (value && value.trim().length > 0) { + logger.warn( + { field: name }, + `EOI in-app PDF template is missing AcroForm field "${name}" — value was dropped. Update the source template.`, + ); + } } } @@ -81,7 +121,10 @@ function setCheckbox( if (checked) cb.check(); else cb.uncheck(); } catch { - // See comment in setText. + logger.warn( + { field: name, checked }, + `EOI in-app PDF template is missing checkbox AcroForm field "${name}" — checkbox state was dropped. Update the source template.`, + ); } } @@ -90,8 +133,11 @@ function setCheckbox( * EoiContext. Field names mirror the Documenso template `formValues` keys so * a single source PDF can serve both pathways. * - * The form is left interactive (not flattened) so a recipient can still tweak - * fields if needed before signing. + * The form is **flattened** after filling so the recipient can't edit + * pre-filled values (yacht dimensions, address, berth number) after the + * fact. Documenso pathway flattens server-side; this brings the in-app + * pathway to parity. Set metadata so the artifact carries Title/Author/ + * Lang for downstream readers and a11y tooling. */ export async function fillEoiFormFields( pdfBytes: Uint8Array, @@ -121,6 +167,17 @@ export async function fillEoiFormFields( setCheckbox(form, 'Purchase', true); setCheckbox(form, 'Lease_10', false); + // PDF metadata so the artifact carries Title/Author/Lang downstream. + doc.setTitle(`EOI – ${context.client.fullName}`); + doc.setAuthor(context.port.name); + doc.setSubject('Expression of Interest'); + doc.setLanguage('en-GB'); + doc.setProducer('Port CRM'); + doc.setCreator('Port CRM'); + + // Flatten so the signer can't edit pre-filled values after the fact. + form.flatten(); + return doc.save(); } diff --git a/src/lib/services/expense-export.tsx b/src/lib/services/expense-export.tsx index 953074a4..0be39977 100644 --- a/src/lib/services/expense-export.tsx +++ b/src/lib/services/expense-export.tsx @@ -134,10 +134,13 @@ export async function exportParentCompany( db.query.ports.findFirst({ where: eq(ports.id, portId) }), resolvePortLogo(portId), ]); + if (!port) { + throw new Error(`Cannot render expense export: port ${portId} not found.`); + } return renderPdf( { const port = await db.query.ports.findFirst({ where: eq(ports.id, portId), }); - const portName = port?.name ?? 'Port Nimara'; - const portSlug = port?.slug ?? 'port'; + // The job is keyed on a real port id - port should always resolve here. + // If the row is missing, the deployment is in a broken state and we + // must not stamp a competitor port's brand on the artifact. Fail loudly. + if (!port) { + throw new Error( + `Cannot render report PDF: port ${portId} not found. Check report job FK integrity.`, + ); + } + const portName = port.name; + const portSlug = port.slug; const logo = await resolvePortLogo(portId); // 6. Render PDF via react-pdf brand kit diff --git a/src/lib/templates/berth-range.ts b/src/lib/templates/berth-range.ts index eca34cf6..87f1e901 100644 --- a/src/lib/templates/berth-range.ts +++ b/src/lib/templates/berth-range.ts @@ -61,11 +61,22 @@ export function formatBerthRange(mooringNumbers: readonly string[]): string { const p = tryParse(m); if (p) parsed.push(p); else { - logger.warn({ mooring: m }, 'formatBerthRange: non-canonical mooring; passing through'); + // Per-mooring is debug to keep logs quiet when archived rows + // (with " (archived)" / " (deleted)" suffixes) flow through a + // bundle; the single summary `warn` below makes the situation + // observable without spamming. + logger.debug({ mooring: m }, 'formatBerthRange: non-canonical mooring; passing through'); passthrough.push(m); } } + if (passthrough.length > 0) { + logger.warn( + { count: passthrough.length, samples: passthrough.slice(0, 3) }, + 'formatBerthRange: non-canonical moorings passed through (verbatim, not range-compressed)', + ); + } + // Sort canonical-form moorings by (prefix, number). parsed.sort((a, b) => { if (a.prefix !== b.prefix) return a.prefix < b.prefix ? -1 : 1; diff --git a/tests/unit/pdf/fill-eoi-form.test.ts b/tests/unit/pdf/fill-eoi-form.test.ts index 27d9d9e6..f219fab8 100644 --- a/tests/unit/pdf/fill-eoi-form.test.ts +++ b/tests/unit/pdf/fill-eoi-form.test.ts @@ -82,24 +82,33 @@ function makeContext(overrides: Partial = {}): EoiContext { // ─── fillEoiFormFields ──────────────────────────────────────────────────────── describe('fillEoiFormFields', () => { - it('populates every text field and checkbox using EoiContext', async () => { + it('flattens the form and writes metadata (Title/Author/Lang)', async () => { const sourcePdf = await buildSyntheticEoiPdf(); const filled = await fillEoiFormFields(sourcePdf, makeContext()); const out = await PDFDocument.load(filled); - const form = out.getForm(); + // Flatten removes the interactive fields from the form — the values + // are baked into the page content stream so the signer can't edit + // them after the fact. + expect(out.getForm().getFields()).toEqual([]); - expect(form.getTextField('Name').getText()).toBe('Alice Smith'); - expect(form.getTextField('Email').getText()).toBe('alice@example.com'); - expect(form.getTextField('Address').getText()).toBe('123 Main St, Austin, USA'); - expect(form.getTextField('Yacht Name').getText()).toBe('Sea Breeze'); - expect(form.getTextField('Length').getText()).toBe('45'); - expect(form.getTextField('Width').getText()).toBe('14'); - expect(form.getTextField('Draft').getText()).toBe('6'); - expect(form.getTextField('Berth Number').getText()).toBe('A12'); + // PDF metadata (M-1). pdf-lib doesn't expose `getLanguage`, but the + // other setters round-trip through standard PDF info-dict getters. + expect(out.getTitle()).toBe('EOI – Alice Smith'); + expect(out.getAuthor()).toBe('Port Nimara'); + expect(out.getSubject()).toBe('Expression of Interest'); + }); - expect(form.getCheckBox('Purchase').isChecked()).toBe(true); - expect(form.getCheckBox('Lease_10').isChecked()).toBe(false); + it('produces a non-empty PDF and a single page with the synthetic source', async () => { + const sourcePdf = await buildSyntheticEoiPdf(); + const filled = await fillEoiFormFields(sourcePdf, makeContext()); + + // Round-trip the saved bytes. With the AcroForm flattened, the doc + // still loads as a valid PDF and retains the original page count — + // the text-field widgets have been baked into the content stream. + const out = await PDFDocument.load(filled); + expect(out.getPageCount()).toBe(1); + expect(filled.byteLength).toBeGreaterThan(500); }); it('handles null primary email and null address gracefully', async () => { @@ -118,20 +127,11 @@ describe('fillEoiFormFields', () => { }), ); + // Still flattens cleanly and round-trips as a valid PDF even with + // null email/address — the doc doesn't error out. const out = await PDFDocument.load(filled); - const form = out.getForm(); - expect(form.getTextField('Email').getText()).toBe(undefined); - expect(form.getTextField('Address').getText()).toBe(undefined); - expect(form.getTextField('Name').getText()).toBe('Bob'); - }); - - it('leaves the form interactive (not flattened) so values can be edited', async () => { - const sourcePdf = await buildSyntheticEoiPdf(); - const filled = await fillEoiFormFields(sourcePdf, makeContext()); - - const out = await PDFDocument.load(filled); - // Field still present and reachable as a TextField → not flattened. - expect(() => out.getForm().getTextField('Name')).not.toThrow(); + expect(out.getForm().getFields()).toEqual([]); + expect(out.getTitle()).toBe('EOI – Bob'); }); it('skips fields silently if the source PDF lacks them', async () => {