From b3f87563c647da01237026377eecaa5f1b891465 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 18 May 2026 18:22:36 +0200 Subject: [PATCH] feat(audit-cleanup): finish all 15 outstanding items from verified backlog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit cleanup completion plan, all tiers shipped: Tier 1 (security + data integrity) - A.7 RTBF true wipe: redact email_messages body/subject/addresses for threads owned by deleted client; redact document_sends.recipient_email; collect file storage keys + delete blobs post-commit. - A.8 user_permission_overrides FK: documented inline why cascade is correct (not set-null as audit suggested) — overrides have no value without their user. - W2.14 PII redaction: camelCase normalization in audit.ts + error-events.service.ts isSensitiveKey; added city/postal/country/ birth fragments. firstName/lastName/dateOfBirth/postalCode etc. now caught in BOTH masker paths. 12 new test cases lock the coverage. Tier 2 (Documenso completion + refactor) - C.2: documentEvents.recipient_email column + partial unique index for per-recipient webhook dedup (migration 0075). handleDocumentSigned now sets recipient_email on insert. - Phase 2: completion_cc_emails distribution. handleDocumentCompleted reads documents.completionCcEmails, filters out signer-duplicates case-insensitively, fans signed PDF out to non-signer recipients. - C.4: extracted createPublicInterest() service from the 346-line api/public/interests route. Route becomes a thin shell (rate-limit, port resolution, audit log, email fan-out). The trio creation logic is now unit-testable without an HTTP fixture. - Phase 4: POST /api/v1/document-templates/[id]/detect-fields wired to document-field-detector.detectFields(). Sparkles "Auto-detect" button added to template-editor.tsx — maps DetectedField → marker with best-guess merge token (DATE / NAME / EMAIL); user retags. Tier 3 (reporting + recommender snapshot lockfiles) - W7.reports: extracted rollupStageRevenue / rollupStageCounts / computeTotalForecast / computeOccupancyRate / rollupBerthStatusCounts into src/lib/services/report-math.ts (pure functions). 16 new tests including an inline-snapshot lockfile on a representative 7-stage forecast. report-generators.ts now delegates. - W7.recommender: 18 new toMatchSnapshot tripwires on classifyTier boundaries + computeHeat at canonical input points. Tier 4 (rolling) - W6.attach: fixed outdated CLAUDE.md claim — threshold banner is informational and never depended on IMAP; bounce monitoring (the IMAP poller) is separate. - D.1 + D.2: documented deferral inline with full why-not-build-it reasoning so a future engineer sees the rationale. - G.1: representative formatDate sweep (audit-log-list, user-list, document-templates merge tokens, document-signing email). Rest of the ~100 sites stay rolling. Quality gates: 1420/1420 vitest (46 new tests above baseline of 1374), tsc clean, 0 lint errors. Plan: docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md Migration: 0075_c2_document_events_recipient_email.sql (applied to dev DB). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- .../2026-05-18-audit-cleanup-completion.md | 1235 +++++++++++++++++ src/app/api/public/interests/route.ts | 281 +--- .../[id]/detect-fields/route.ts | 66 + src/components/admin/audit/audit-log-list.tsx | 5 +- .../admin/templates/template-editor.tsx | 104 +- src/components/admin/users/user-list.tsx | 5 +- src/lib/audit.ts | 19 +- ...075_c2_document_events_recipient_email.sql | 21 + src/lib/db/schema/documents.ts | 14 +- src/lib/db/schema/users.ts | 5 + src/lib/email/templates/document-signing.tsx | 9 +- src/lib/queue/scheduler.ts | 12 +- src/lib/queue/workers/import.ts | 31 +- .../services/client-hard-delete.service.ts | 100 +- src/lib/services/document-templates.ts | 4 +- src/lib/services/documents.service.ts | 28 +- src/lib/services/error-events.service.ts | 17 +- src/lib/services/public-interest.service.ts | 303 ++++ src/lib/services/report-generators.ts | 57 +- src/lib/services/report-math.ts | 117 ++ tests/unit/audit.test.ts | 44 +- .../berth-recommender.test.ts.snap | 192 +++ tests/unit/services/berth-recommender.test.ts | 62 + tests/unit/services/report-math.test.ts | 186 +++ 25 files changed, 2569 insertions(+), 350 deletions(-) create mode 100644 docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md create mode 100644 src/app/api/v1/document-templates/[id]/detect-fields/route.ts create mode 100644 src/lib/db/migrations/0075_c2_document_events_recipient_email.sql create mode 100644 src/lib/services/public-interest.service.ts create mode 100644 src/lib/services/report-math.ts create mode 100644 tests/unit/services/__snapshots__/berth-recommender.test.ts.snap create mode 100644 tests/unit/services/report-math.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6186844e..54441033 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -164,7 +164,7 @@ After `db:push` or applying a migration via `psql` against a running dev server, ### Send-from accounts (sales send-outs) - Configurable via `system_settings`; defaults to `sales@portnimara.com` (human) + `noreply@portnimara.com` (automation). SMTP/IMAP passwords AES-256-GCM at rest; API returns only `*PassIsSet` markers. -- Audit → `document_sends` (separate from `audit_logs` for volume + binary refs). Body markdown rendered via `renderEmailBody()` (escape-then-allowlist; XSS-tested). Rate limit 50 sends/user/hour. Files > `email_attach_threshold_mb` ship as 24h signed-URL link (filename HTML-escaped against injection). Bounce monitoring needs IMAP creds in addition to SMTP — without them, the size-rejection banner is disabled. +- Audit → `document_sends` (separate from `audit_logs` for volume + binary refs). Body markdown rendered via `renderEmailBody()` (escape-then-allowlist; XSS-tested). Rate limit 50 sends/user/hour. Files > `email_attach_threshold_mb` ship as 24h signed-URL link (filename HTML-escaped against injection). The threshold banner in the compose UI is informational and shows whenever the preview API returns the per-port threshold — it does NOT depend on IMAP. Separately, bounce monitoring (`imap-bounce-poller.ts`) needs IMAP creds and no-ops cleanly when they're unset. ### Pre-commit diff --git a/docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md b/docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md new file mode 100644 index 00000000..a145c268 --- /dev/null +++ b/docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md @@ -0,0 +1,1235 @@ +# Audit Cleanup Completion — Master Plan (2026-05-18, Part 2) + +> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close out everything outstanding in `docs/BACKLOG.md` sections A, C, D, E, G, H over a sequenced ~4-week sweep, ending with the backlog drained down to "no concrete trigger" parked items only. + +**Architecture:** Six sequenced phases. Phase A is stop-ship priority (security + data integrity). Phase B layers in HIGH security + observability. Phase C re-opens the back-burnered Documenso build (phases 2-7 + Section C deferred items bundled). Phase D is mechanical React Compiler cleanup. Phase E does Wave 6 + 7 validation work on top of the Documenso changes. Phase F mops up Section G dependency migrations + Section D placeholders + Section E hidden tabs. Each phase ships a working, testable cut; commits are atomic per item with a phase-tagged prefix. + +**Tech Stack:** Same as the parent CRM — Next.js 16 App Router, React 19, TypeScript strict, Drizzle ORM on PostgreSQL, BullMQ + Redis, better-auth, React-Email + nodemailer + imapflow, Documenso v1/v2 client, pdf-lib for AcroForm fill. + +**Estimate:** ~4 weeks focused work end-to-end. Phase A is the only phase where "ship before deploy" is hard-blocking — everything after is post-launch hygiene. + +--- + +## Pre-flight (do once) + +- [ ] **P.1 — Verify session-end state** + +```bash +git log --oneline -5 +pnpm exec vitest run +pnpm exec tsc --noEmit +pnpm lint +``` + +Expected: clean tsc, 0 lint errors, 1374/1374 vitest pass (or current baseline). Capture baseline number for "no regressions" gate at end of each phase. + +- [ ] **P.2 — Verify Wave 2 #12 (admin subject editor) actually outstanding** + +```bash +grep -n "overrides?.subject" src/lib/email/templates/*.tsx | wc -l +``` + +Expected: count ≥ 8 (all templates honour `overrides.subject`). If yes, Wave 2 #12 is **closed in code** by this session's Phase 5 — only the admin UI needs verification. Confirm admin page `/admin/email-templates` exposes a subject input bound to that field; if so, mark #12 ✅ before Phase B starts. + +- [ ] **P.3 — Refresh backlog tracker** + +Open `docs/BACKLOG.md` and append a note: "2026-05-18 Part 2 plan live at `docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md`." + +--- + +## Phase A — Wave 1 stop-ship CRITICALs (~3-5 days) + +Nine items, half-day each, ship in priority order. **Each item is one commit** with prefix `fix(audit-wave-1): `. + +### Task A.1 — Real `db:migrate` runner + +**Why critical:** `0052_audit_critical_fixes.sql` uses `CREATE INDEX CONCURRENTLY` which silently never runs under `db:push`. Six composite indexes missing in prod. Without a real runner this only ever gets worse. + +**Files:** + +- Create: `scripts/db-migrate.ts` +- Modify: `package.json` (replace `db:migrate` script) +- Test: `tests/integration/scripts/db-migrate.spec.ts` + +- [ ] **Step 1: Write failing test** + +```ts +// tests/integration/scripts/db-migrate.spec.ts +import { applyPendingMigrations } from '@/scripts/db-migrate-lib'; +import { db } from '@/lib/db'; +import { sql } from 'drizzle-orm'; +import { describe, expect, it, beforeEach } from 'vitest'; + +describe('db-migrate runner', () => { + beforeEach(async () => { + await db.execute(sql`DROP TABLE IF EXISTS __drizzle_migrations`); + }); + + it('applies pending migrations idempotently', async () => { + const result1 = await applyPendingMigrations({ dir: 'src/lib/db/migrations' }); + expect(result1.applied.length).toBeGreaterThan(0); + const result2 = await applyPendingMigrations({ dir: 'src/lib/db/migrations' }); + expect(result2.applied.length).toBe(0); + }); + + it('honours --> statement-breakpoint split and runs CREATE INDEX CONCURRENTLY outside tx', async () => { + // Verify the runner does NOT wrap CONCURRENTLY statements in a transaction + // (Postgres rejects CONCURRENTLY inside BEGIN/COMMIT). + const result = await applyPendingMigrations({ dir: 'src/lib/db/migrations' }); + expect(result.errors).toEqual([]); + }); +}); +``` + +- [ ] **Step 2: Build the runner** + +Extract logic into `scripts/db-migrate-lib.ts` (testable) and a thin CLI `scripts/db-migrate.ts` that calls it. Runner contract: + +1. Read `src/lib/db/migrations/*.sql` in lexical order. +2. Track applied state in `__drizzle_migrations` table (`id text primary key, applied_at timestamptz`). +3. Split each file on `--> statement-breakpoint`. +4. For each statement: if it contains `CONCURRENTLY` → execute outside any tx; otherwise wrap the file's statements in a single tx. +5. On failure: log statement + abort; do not mark migration applied. + +- [ ] **Step 3: Rerun vitest, then wire script** + +```bash +pnpm exec vitest run tests/integration/scripts/db-migrate.spec.ts +``` + +Then in `package.json`: `"db:migrate": "tsx scripts/db-migrate.ts"`. + +- [ ] **Step 4: Backfill missing indexes in dev DB** + +```bash +PGPASSWORD=changeme psql -h localhost -p 5434 -U crm -d port_nimara_crm -c "DROP TABLE IF EXISTS __drizzle_migrations;" +pnpm db:migrate +``` + +Then verify the 6 indexes from `0052_audit_critical_fixes.sql` exist: + +```sql +SELECT indexname FROM pg_indexes WHERE indexname LIKE '%audit_critical%'; +``` + +- [ ] **Step 5: Commit** + +```bash +git add scripts/db-migrate.ts scripts/db-migrate-lib.ts tests/integration/scripts/db-migrate.spec.ts package.json +git commit -m "fix(audit-wave-1): real db:migrate runner with CONCURRENTLY support" +``` + +--- + +### Task A.2 — `EMAIL_REDIRECT_TO` production guard + +**Why critical:** Currently a dev safety net; if accidentally left set in prod, every customer email goes to one dev mailbox. + +**Files:** + +- Modify: `src/lib/env.ts` +- Modify: `src/lib/email/index.ts` +- Test: `tests/unit/lib/env.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +// tests/unit/lib/env.spec.ts (add to existing) +it('rejects EMAIL_REDIRECT_TO when NODE_ENV=production', () => { + expect(() => + serverEnvSchema.parse({ ...validBase, NODE_ENV: 'production', EMAIL_REDIRECT_TO: 'x@y.com' }), + ).toThrow(/EMAIL_REDIRECT_TO must not be set in production/); +}); + +it('accepts EMAIL_REDIRECT_TO when NODE_ENV=development', () => { + expect(() => + serverEnvSchema.parse({ ...validBase, NODE_ENV: 'development', EMAIL_REDIRECT_TO: 'x@y.com' }), + ).not.toThrow(); +}); +``` + +- [ ] **Step 2: Update env schema** + +In `src/lib/env.ts`, add a `superRefine` after the base parse: + +```ts +.superRefine((env, ctx) => { + if (env.NODE_ENV === 'production' && env.EMAIL_REDIRECT_TO) { + ctx.addIssue({ + code: 'custom', + path: ['EMAIL_REDIRECT_TO'], + message: 'EMAIL_REDIRECT_TO must not be set in production', + }); + } +}); +``` + +- [ ] **Step 3: Boot warning in mailer** + +In `src/lib/email/index.ts` at module top: + +```ts +if (env.EMAIL_REDIRECT_TO) { + logger.warn( + { emailRedirectTo: env.EMAIL_REDIRECT_TO }, + 'EMAIL_REDIRECT_TO is set — every outbound email will be rerouted', + ); +} +``` + +- [ ] **Step 4: Tests + commit** + +```bash +pnpm exec vitest run tests/unit/lib/env.spec.ts +git add src/lib/env.ts src/lib/email/index.ts tests/unit/lib/env.spec.ts +git commit -m "fix(audit-wave-1): reject EMAIL_REDIRECT_TO in production" +``` + +--- + +### Task A.3 — Orphan-blob fix in `handleDocumentCompleted` + +**Why critical:** Current catch-block leaves blob in storage AND marks `status='completed'` with no `signedFileId`. Storage cost leak + document marked done with no signed PDF retrievable. + +**Files:** + +- Modify: `src/lib/services/documents.service.ts:1100-1253` +- Test: `tests/integration/services/documents-completion.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +// tests/integration/services/documents-completion.spec.ts +it('rolls back storage + DB when files.insert fails mid-completion', async () => { + // Arrange: stub storage.put to succeed, files.insert to throw + const putSpy = vi.spyOn(storage, 'put').mockResolvedValueOnce({ key: 'k', sha256: 'x', size: 1 }); + vi.spyOn(db.insert(filesTable), 'values').mockRejectedValueOnce(new Error('boom')); + + await expect(handleDocumentCompleted({ documentId: doc.id, pdfBytes })).rejects.toThrow(); + + // Assert: blob deleted from storage + expect(await storage.head('k').catch(() => null)).toBeNull(); + // Assert: documents.status NOT updated to completed + const after = await db.query.documents.findFirst({ where: eq(documents.id, doc.id) }); + expect(after?.status).not.toBe('completed'); + expect(after?.signedFileId).toBeNull(); +}); +``` + +- [ ] **Step 2: Rewrite as saga** + +Inside `handleDocumentCompleted`: + +1. `storage.put(...)` → capture key. +2. `withTransaction(async (tx) => { insert files, update documents })`. +3. On tx failure: `await storage.delete(key)` in `finally`/`catch`, then rethrow. + +Idempotency early-return (`status='completed' && signedFileId`) stays at the top of the function — already handles 5xx retries. + +- [ ] **Step 3: Verify + commit** + +```bash +pnpm exec vitest run tests/integration/services/documents-completion.spec.ts +git add src/lib/services/documents.service.ts tests/integration/services/documents-completion.spec.ts +git commit -m "fix(audit-wave-1): handleDocumentCompleted saga compensating delete" +``` + +--- + +### Task A.4 — Escape URLs in email templates + +**Why critical:** Every template inlines `${data.link}` into `href="…"` without escaping. URL injection risk (template merge tokens could carry a crafted URL). + +**Note:** `safeUrl()` already exists in `src/lib/email/shell.ts` and is used by most templates. This task sweeps the few that don't use it. + +**Files (audit each):** + +- `src/lib/email/templates/portal-auth.tsx` +- `src/lib/email/templates/inquiry-client-confirmation.tsx` +- `src/lib/email/templates/inquiry-sales-notification.tsx` +- `src/lib/email/templates/notification-digest.tsx` +- `src/lib/email/templates/document-signing.tsx` +- `src/lib/email/templates/admin-email-change.tsx` +- `src/lib/email/templates/crm-invite.tsx` +- `src/lib/email/templates/residential-inquiry.tsx` + +- [ ] **Step 1: Grep for unsafe usages** + +```bash +grep -rn 'href={data\.\|href={`' src/lib/email/templates/ +``` + +For each match: wrap with `safeUrl(...)`. Schema: `safeUrl` already allows only http/https/mailto. + +- [ ] **Step 2: Add safeUrl coverage test** + +```ts +// tests/unit/lib/email/shell.spec.ts (add) +it.each([ + ['javascript:alert(1)', '#'], + ['data:text/html,xss', '#'], + ['ftp://x', '#'], + ['https://ok.com/path', 'https://ok.com/path'], + ['mailto:a@b.c', 'mailto:a@b.c'], +])('safeUrl(%s) → %s', (input, expected) => { + expect(safeUrl(input)).toBe(expected); +}); +``` + +- [ ] **Step 3: Commit** + +```bash +git add src/lib/email/templates/ tests/unit/lib/email/shell.spec.ts +git commit -m "fix(audit-wave-1): route all email template URLs through safeUrl" +``` + +--- + +### Task A.5 — Replace 16 native `window.confirm()` calls + +**Why critical:** Destructive flows bypass `ConfirmationDialog`/`AlertDialog`. ui-ux-auditor's C1 itemized them. Inconsistent UX + native confirm can't be styled or scripted. + +**Files (16 sites — grep to enumerate):** + +```bash +grep -rn 'window.confirm\|confirm(' src/components/ src/app/ --include='*.tsx' | grep -v node_modules | grep -v 'window\.confirmDocumentMooring' +``` + +Expected sites include: cancel signing, delete file, archive interest/company/yacht, delete folder, etc. + +- [ ] **Step 1: Identify shared dialog primitive** + +`ConfirmationDialog` exists at `src/components/shared/confirmation-dialog.tsx`. Pattern: `useState` for open, ``. + +- [ ] **Step 2: Sweep — 30 min per site × 16** + +For each `window.confirm(...)` call: + +1. Add `useState` for dialog open. +2. Replace the `if (window.confirm(...))` guard with `setConfirmOpen(true)`. +3. Move action body into `onConfirm` handler. +4. Render ``. + +- [ ] **Step 3: Add ESLint rule against `window.confirm`** + +In `eslint.config.mjs`: + +```js +{ + rules: { + 'no-restricted-globals': ['error', { name: 'confirm', message: 'Use instead' }], + }, +} +``` + +- [ ] **Step 4: Commit** + +```bash +git add src/components/ src/app/ eslint.config.mjs +git commit -m "fix(audit-wave-1): replace 16 window.confirm() with ConfirmationDialog" +``` + +--- + +### Task A.6 — GDPR Article-15 export completeness + +**Why critical:** Regulator-finding-level gap. Current bundle missing: portal_users, email_threads/messages, document_sends, reminders, files, scratchpadNotes, client_merge_log, contact_log, website_submissions, form_submissions. + +**Files:** + +- Modify: `src/lib/services/gdpr-bundle-builder.ts` +- Test: `tests/integration/services/gdpr-bundle.spec.ts` + +- [ ] **Step 1: Enumerate missing relations** + +For a single test client, seed at least one row in each missing table linking back via `clientId` (or polymorphic FK where applicable). + +- [ ] **Step 2: Failing test** + +```ts +it('includes every PII-bearing relation in the GDPR bundle', async () => { + const bundle = await buildGdprBundle(testClient.id); + expect(Object.keys(bundle)).toEqual( + expect.arrayContaining([ + 'client', + 'addresses', + 'contacts', + 'interests', + 'notes', + 'documents', + 'tags', + // newly required: + 'portalUsers', + 'emailThreads', + 'emailMessages', + 'documentSends', + 'reminders', + 'files', + 'scratchpadNotes', + 'clientMergeLog', + 'contactLog', + 'websiteSubmissions', + 'formSubmissions', + ]), + ); +}); +``` + +- [ ] **Step 3: Add the 10 missing relation loaders** + +Each loader is a Drizzle query bounded by `port_id` + `client_id` (or polymorphic resolver where the relation doesn't carry client_id directly — use the symmetric-reach pattern from `listFilesAggregatedByEntity`). + +- [ ] **Step 4: Commit** + +```bash +git add src/lib/services/gdpr-bundle-builder.ts tests/integration/services/gdpr-bundle.spec.ts +git commit -m "fix(audit-wave-1): GDPR bundle includes 10 missing relations" +``` + +--- + +### Task A.7 — Right-to-be-forgotten true wipe + +**Why critical:** `client-hard-delete.service.ts` nullifies FKs but leaves verbatim PII in `email_messages.body_html`, `files`, `document_sends.recipient_email`. Doesn't satisfy Article-17. + +**Files:** + +- Modify: `src/lib/services/client-hard-delete.service.ts` +- Test: `tests/integration/services/client-hard-delete.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +it('overwrites PII in email_messages.body_html and document_sends', async () => { + await hardDeleteClient({ clientId, requestedBy: actor }); + const msgs = await db.query.emailMessages.findMany({ + where: eq(emailMessages.clientId, clientId), + }); + msgs.forEach((m) => { + expect(m.bodyHtml).toBe(''); + expect(m.bodyText).toBe(''); + expect(m.subject).toBe('[erased]'); + }); + const sends = await db.query.documentSends.findMany({ + where: eq(documentSends.clientId, clientId), + }); + sends.forEach((s) => expect(s.recipientEmail).toBe('[erased]')); +}); + +it('deletes file blobs from storage', async () => { + const file = await db.query.files.findFirst({ where: eq(files.clientId, clientId) }); + await hardDeleteClient({ clientId, requestedBy: actor }); + expect(await storage.head(file!.storageKey).catch(() => null)).toBeNull(); +}); +``` + +- [ ] **Step 2: Add wipe pass** + +Inside the existing tx: + +1. `UPDATE email_messages SET body_html = '', body_text = '', subject = '[erased]' WHERE client_id = $1`. +2. `UPDATE document_sends SET recipient_email = '[erased]' WHERE client_id = $1`. +3. For each `files` row: delete blob via `storage.delete(key)`, then delete the row. Errors on storage.delete should not block tx — log and continue (blob orphan is acceptable; row deletion is the legally required action). + +- [ ] **Step 3: Commit** + +```bash +git add src/lib/services/client-hard-delete.service.ts tests/integration/services/client-hard-delete.spec.ts +git commit -m "fix(audit-wave-1): client hard-delete true-wipe PII text + blobs" +``` + +--- + +### Task A.8 — `user_permission_overrides.user_id` FK + `onDelete='set null'` + +**Why critical:** Orphan rows accumulate when users are deleted. + +**Files:** + +- Create: `src/lib/db/migrations/0076_user_permission_overrides_fk.sql` + +- [ ] **Step 1: Write migration** + +```sql +ALTER TABLE user_permission_overrides + ADD CONSTRAINT user_permission_overrides_user_id_fkey + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; +``` + +- [ ] **Step 2: Apply + verify** + +```bash +PGPASSWORD=changeme psql -h localhost -p 5434 -U crm -d port_nimara_crm -f src/lib/db/migrations/0076_user_permission_overrides_fk.sql +``` + +Then in the Drizzle schema `src/lib/db/schema/permissions.ts` (or wherever `userPermissionOverrides` lives): add `.references(() => users.id, { onDelete: 'set null' })` to the column. + +- [ ] **Step 3: Commit** + +```bash +git add src/lib/db/migrations/0076_user_permission_overrides_fk.sql src/lib/db/schema/ +git commit -m "fix(audit-wave-1): user_permission_overrides.user_id FK + set null" +``` + +--- + +### Task A.9 — Resolve-identifier endpoint replacement + +**Why critical:** Currently echoes the real canonical email on a successful username hit. Even rate-limited, this is a username-to-email mapping oracle. + +**Files:** + +- Delete (or refactor): `src/app/api/auth/resolve-identifier/route.ts` +- Modify: `src/app/(auth)/login/page.tsx` (form action) +- Create: `src/app/api/auth/sign-in-proxy/route.ts` +- Test: `tests/integration/api/auth-sign-in-proxy.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +it('never returns canonical email on identifier+password attempt', async () => { + // Success path + const ok = await POST(req({ identifier: 'username1', password: 'correct-pass' })); + const okBody = await ok.json(); + expect(okBody).not.toHaveProperty('email'); + expect(okBody.data).toMatchObject({ ok: true }); + + // Failure path + const bad = await POST(req({ identifier: 'username1', password: 'wrong' })); + const badBody = await bad.json(); + expect(badBody).not.toHaveProperty('email'); +}); +``` + +- [ ] **Step 2: Build proxy** + +New endpoint accepts `{ identifier, password }`, resolves canonical email server-side, calls `auth.api.signInEmail({ body: { email, password } })`, returns `{ data: { ok: true } }` with session cookie set on success, `{ error: { code: 'INVALID_CREDENTIALS' } }` on failure. Same response for "no such identifier" and "wrong password" (timing-equalize with a dummy bcrypt compare on miss). + +- [ ] **Step 3: Update login form** + +`src/app/(auth)/login/page.tsx` posts to the proxy directly with the user-entered identifier; no two-step flow. + +- [ ] **Step 4: Delete old endpoint** + +```bash +git rm src/app/api/auth/resolve-identifier/route.ts +``` + +- [ ] **Step 5: Commit** + +```bash +git add src/app/api/auth/sign-in-proxy/ src/app/\(auth\)/login/page.tsx tests/integration/api/auth-sign-in-proxy.spec.ts +git commit -m "fix(audit-wave-1): sign-in proxy replaces resolve-identifier email leak" +``` + +--- + +### Phase A acceptance gate + +- [ ] `pnpm exec vitest run` — all green, ≥ baseline count +- [ ] `pnpm exec tsc --noEmit` — clean +- [ ] `pnpm lint` — 0 errors +- [ ] `pnpm exec playwright test --project=smoke` — green +- [ ] All 9 commits land +- [ ] Update `docs/BACKLOG.md` §H Wave 1 — mark all 9 items ✅ +- [ ] Hand off Phase B + +--- + +## Phase B — Wave 2 HIGH security + observability (~5-7 days) + +Six items, ~half-day each. Prefix: `fix(audit-wave-2): `. + +### Task B.1 — `audit_logs.metadata` PII masking + 90-day retention + +**Files:** + +- Modify: `src/lib/audit.ts` (extend `maskSensitiveFields` to cover `metadata` JSONB) +- Create: `src/jobs/processors/audit-retention.ts` (90-day cron) +- Modify: `src/lib/queue/scheduler.ts` (register cron) +- Test: `tests/unit/lib/audit-mask.spec.ts` + +- [ ] **Step 1: Failing test for masking** + +```ts +it('masks email/phone/password/token in metadata JSONB', () => { + const masked = maskSensitiveFields({ + metadata: { email: 'a@b.c', phone: '+44...', password: 'x', other: 'keep' }, + }); + expect(masked.metadata.email).toBe('[redacted]'); + expect(masked.metadata.phone).toBe('[redacted]'); + expect(masked.metadata.password).toBe('[redacted]'); + expect(masked.metadata.other).toBe('keep'); +}); +``` + +- [ ] **Step 2: Add retention worker** + +Mirror the `error_events` retention worker. Daily cron deleting rows where `created_at < now() - interval '90 days'`. Register in scheduler. + +- [ ] **Step 3: Commit** + +```bash +git commit -m "fix(audit-wave-2): audit_logs metadata PII mask + 90d retention" +``` + +### Task B.2 — Webhook → error pipeline + +**Files (sweep):** + +- `src/app/api/webhooks/documenso/route.ts` +- `src/app/api/webhooks/imap-bounce/route.ts` (if any) +- Any other `/api/webhooks/*` routes + +- [ ] **Step 1: Locate webhook routes** + +```bash +ls src/app/api/webhooks/ +``` + +- [ ] **Step 2: Wrap each handler** + +```ts +try { + await handle(req); +} catch (err) { + await captureErrorEvent(err, { source: 'webhook:documenso', requestId }); + throw err; +} +``` + +- [ ] **Step 3: Test that thrown errors are captured** + +For each webhook, integration-test that a forced throw lands in `error_events`. + +- [ ] **Step 4: Commit** + +```bash +git commit -m "fix(audit-wave-2): webhooks emit errors to captureErrorEvent" +``` + +### Task B.3 — Admin email-template subject editor wiring verification + +**Status:** Phase 5 of the post-audit master plan rewrote all 8 templates to honour `overrides.subject`. This task confirms the admin UI surfaces it and adds a test. + +**Files:** + +- Verify: `src/app/(dashboard)/[portSlug]/admin/email-templates/page.tsx` +- Verify: `src/lib/services/mailer-config.ts` `getTemplateOverridesForKey()` +- Test: `tests/integration/services/mailer-config.spec.ts` + +- [ ] **Step 1: Manually walk admin page** — confirm subject input present + saves. +- [ ] **Step 2: Integration test that override is applied** + +```ts +it('honours subject override from email_templates table', async () => { + await db + .update(emailTemplates) + .set({ subject: 'Custom override' }) + .where(eq(emailTemplates.key, 'portal_activation')); + const result = await portalAuthEmail(testData, { + subject: await getTemplateOverridesForKey('portal_activation').subject, + }); + expect(result.subject).toBe('Custom override'); +}); +``` + +- [ ] **Step 3: Commit (or close as already-done if test passes against current code)** + +```bash +git commit -m "test(audit-wave-2): verify admin subject override flows to render" +``` + +### Task B.4 — Admin signature/footer fields wire-up OR delete UI + +**Files:** + +- Modify: `src/lib/email/shell.ts` (read `email_signature_html` + `email_footer_html`) +- Modify: `src/app/(dashboard)/[portSlug]/admin/email/page.tsx` (if deleting) + +- [ ] **Step 1: Decide — wire or delete** + +If admin already uses signature/footer in practice (check `system_settings` for non-null rows): wire. If unused: delete the UI and the columns. + +- [ ] **Step 2 (wire path): Extend shell** + +`renderShell({...})` accepts optional `signatureHtml` + `footerHtml` from per-port system_settings. Both run through allowlist sanitizer (`isomorphic-dompurify` already in deps). + +- [ ] **Step 3 (wire path): Plumb in `mailer-config.ts`** + +Load signature/footer alongside branding in every `send*` site. + +- [ ] **Step 4: Commit** + +```bash +git commit -m "fix(audit-wave-2): wire admin signature/footer through email shell" +``` + +### Task B.5 — PII redaction in error pipeline + +**Files:** + +- Modify: `src/lib/observability/error-events.ts` (or wherever `request_body_excerpt` sanitizer lives) +- Test: `tests/unit/lib/observability/error-sanitizer.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +it.each([ + ['email', { email: 'a@b.c' }, /\[redacted\]/], + ['phone', { phone: '+44 1234 567890' }, /\[redacted\]/], + ['firstName', { firstName: 'Alice' }, /\[redacted\]/], + ['dateOfBirth', { dob: '1990-01-01' }, /\[redacted\]/], + ['address', { addressLine1: '10 Downing St' }, /\[redacted\]/], +])('redacts %s', (_, input, expected) => { + expect(JSON.stringify(sanitize(input))).toMatch(expected); +}); +``` + +- [ ] **Step 2: Extend sanitizer key list** + +Add: `email`, `phone`, `firstName`, `lastName`, `fullName`, `dateOfBirth`, `dob`, `addressLine1`, `addressLine2`, `city`, `postalCode`, `country`. + +- [ ] **Step 3: Commit** + +```bash +git commit -m "fix(audit-wave-2): error sanitizer redacts PII keys" +``` + +### Task B.6 — Notification email worker XSS + +**Files:** + +- Modify: `src/lib/queue/workers/notifications.ts:65-71` +- Test: `tests/unit/queue/notifications-xss.spec.ts` + +- [ ] **Step 1: Failing test** + +```ts +it('escapes notification description and link in HTML', async () => { + const html = await buildNotificationHtml({ + description: '', + link: 'javascript:alert(1)', + }); + expect(html).not.toContain('