From ebe5fe6ed878893f6893ba59544c7a249d7b148d Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 13:07:21 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit):=20GDPR/merge=20=E2=80=94=20M6=20(dr?= =?UTF-8?q?op=20false=20merge-reversibility=20claims),=20M7=20(GDPR=20expo?= =?UTF-8?q?rt=20adds=204=20PII=20tables),=20L14=20(docstring),=20L15=20(ha?= =?UTF-8?q?rd-delete=20breadcrumb=20note)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../services/client-hard-delete.service.ts | 9 +++ src/lib/services/client-merge.service.ts | 31 +++++++---- src/lib/services/gdpr-bundle-builder.ts | 55 ++++++++++++++++++- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/lib/services/client-hard-delete.service.ts b/src/lib/services/client-hard-delete.service.ts index 069e3654..5046e1f7 100644 --- a/src/lib/services/client-hard-delete.service.ts +++ b/src/lib/services/client-hard-delete.service.ts @@ -25,6 +25,15 @@ * audit history is preserved without blocking the delete. * - non-cascade non-nullable FKs (interests, reservations, surviving * row in client_merge_log) are deleted explicitly inside the tx. + * - the `clients.merged_into_client_id` self-FK is ON DELETE SET NULL + * (migration 0042). If THIS client was a merge winner, any archived + * loser whose `merged_into_client_id` points here has that pointer + * auto-NULLed by the cascade when this row is deleted. That silently + * severs the loser's redirect breadcrumb (the loser is no longer + * resolvable to a surviving record) but is benign: no FK violation, + * no orphaned/cross-tenant data, and the loser stays archived. We do + * NOT proactively re-home those pointers — the winner is gone, so + * there is nothing valid left to redirect to. */ import { timingSafeEqual } from 'node:crypto'; diff --git a/src/lib/services/client-merge.service.ts b/src/lib/services/client-merge.service.ts index 09f6bf52..367f7869 100644 --- a/src/lib/services/client-merge.service.ts +++ b/src/lib/services/client-merge.service.ts @@ -8,11 +8,15 @@ * actually merges two pre-existing clients * - the migration script's `--apply` (eventually) * - * Reversibility: every merge writes a `client_merge_log` row containing - * the loser's full pre-merge state. Within the configured undo window - * (default 7 days, see `dedup_undo_window_days` in system_settings) a - * follow-up `unmergeClients` call can restore the loser and detach - * everything that was reattached. + * NOT reversible: a merge is permanent. Every merge writes a + * `client_merge_log` row containing a snapshot of the loser's pre-merge + * state, but this is a forensic/audit record only — there is NO + * `unmergeClients` implementation, and the child rows reattached to the + * winner are not restorable from the snapshot. Operators must treat + * merge as a destructive, one-way operation. (The original design called + * for a 7-day `dedup_undo_window_days` reversibility window; that undo + * pathway was never built, so the setting has no effect and the snapshot + * is retained purely as an audit trail.) * * Design reference: docs/superpowers/specs/2026-05-03-dedup-and-migration-design.md §6. */ @@ -138,8 +142,9 @@ export async function mergeClients(opts: MergeOptions): Promise { throw new ConflictError('Cannot merge into an archived client'); } - // ── Snapshot the loser's full state before any mutation. Used by - // `unmergeClients` to restore within the undo window. ────────────── + // ── Snapshot the loser's full state before any mutation. Written to + // `client_merge_log.mergeDetails` as a forensic/audit record only; + // NOT used to restore — merge is one-way (no `unmergeClients`). ───── const loserContacts = await tx .select() .from(clientContacts) @@ -245,8 +250,8 @@ export async function mergeClients(opts: MergeOptions): Promise { const key = `${c.channel}::${c.value.toLowerCase()}`; if (winnerContactKeys.has(key)) { // Winner already has this contact - drop loser's row (cascade - // will clean up when loser is archived). But we keep snapshot - // so undo restores it. + // will clean up when loser is archived). The snapshot records it + // for audit, but this drop is not reversible (merge is one-way). continue; } await tx @@ -393,9 +398,11 @@ export async function mergeClients(opts: MergeOptions): Promise { .returning({ id: invoices.id }) ).length; - // ── Archive the loser. Row stays in DB for the undo window; - // `mergedIntoClientId` is the redirect pointer for any stragglers - // (links / direct queries / saved views). ────────────────────────── + // ── Archive the loser. The row stays in the DB (soft-archived, not + // deleted) so `mergedIntoClientId` can act as the redirect pointer + // for any stragglers (links / direct queries / saved views). This + // is a permanent redirect — the loser is never un-archived by a + // reverse-merge, as no unmerge pathway exists. ───────────────────── await tx .update(clients) .set({ diff --git a/src/lib/services/gdpr-bundle-builder.ts b/src/lib/services/gdpr-bundle-builder.ts index 8ec5b039..a431ba18 100644 --- a/src/lib/services/gdpr-bundle-builder.ts +++ b/src/lib/services/gdpr-bundle-builder.ts @@ -35,6 +35,10 @@ import { emailThreads, emailMessages } from '@/lib/db/schema/email'; import { reminders, interestContactLog } from '@/lib/db/schema/operations'; import { documentSends } from '@/lib/db/schema/brochures'; import { websiteSubmissions } from '@/lib/db/schema/website-submissions'; +import { payments } from '@/lib/db/schema/pipeline'; +import { berthWaitingList } from '@/lib/db/schema/berths'; +import { supplementalFormTokens } from '@/lib/db/schema/supplemental-forms'; +import { interestFieldHistory } from '@/lib/db/schema/interest-field-history'; export interface GdprBundle { /** Bundle metadata for traceability. */ @@ -56,12 +60,16 @@ export interface GdprBundle { company: Record; }>; interests: Record[]; + interestFieldHistory: Record[]; contactLog: Record[]; tenancies: Record[]; + berthWaitingList: Record[]; + payments: Record[]; invoices: Record[]; documents: Record[]; files: Record[]; formSubmissions: Record[]; + supplementalFormTokens: Record[]; websiteSubmissions: Record[]; documentSends: Record[]; emailThreads: Array<{ @@ -77,8 +85,16 @@ export interface GdprBundle { /** * Loads every row that references this client across all tenant-scoped - * tables. Every query is filtered by `portId` as well, so a stale FK - * to another tenant never leaks across. + * tables. + * + * Port-scoping: queries on tables that carry a `port_id` column add a + * redundant `eq(..., portId)` predicate as defense-in-depth, so a stale + * cross-tenant FK never leaks across. Tables WITHOUT a `port_id` column + * (clientContacts, clientAddresses, clientRelationships, clientNotes, + * clientTags, formSubmissions, scratchpadNotes, portalUsers, + * berthWaitingList) are scoped by `clientId` ONLY — which is safe because + * `clientId` is a globally-unique UUID and the client itself was already + * validated against `portId` at the top of `buildClientBundle`. */ export async function buildClientBundle(clientId: string, portId: string): Promise { const client = await db.query.clients.findFirst({ where: eq(clients.id, clientId) }); @@ -95,11 +111,15 @@ export async function buildClientBundle(clientId: string, portId: string): Promi ownedYachts, membershipRows, interestRows, + fieldHistoryRows, tenancyRows, + waitingListRows, + paymentRows, invoiceRows, documentRows, fileRows, formSubmissionRows, + supplementalTokenRows, documentSendRows, threadRows, reminderRows, @@ -141,9 +161,26 @@ export async function buildClientBundle(clientId: string, portId: string): Promi db.query.interests.findMany({ where: and(eq(interests.clientId, clientId), eq(interests.portId, portId)), }), + // Field-level override history carries the denormalized clientId for + // direct-edit and supplemental-form overrides alike. Port-scoped too, + // since interest_field_history has a notNull port_id column. + db.query.interestFieldHistory.findMany({ + where: and( + eq(interestFieldHistory.portId, portId), + eq(interestFieldHistory.clientId, clientId), + ), + }), db.query.berthTenancies.findMany({ where: and(eq(berthTenancies.clientId, clientId), eq(berthTenancies.portId, portId)), }), + // berth_waiting_list has no port_id column — scope by clientId only + // (clientId is a global UUID, client already validated against portId). + db.query.berthWaitingList.findMany({ + where: eq(berthWaitingList.clientId, clientId), + }), + db.query.payments.findMany({ + where: and(eq(payments.portId, portId), eq(payments.clientId, clientId)), + }), db.query.invoices.findMany({ where: and( eq(invoices.portId, portId), @@ -160,6 +197,12 @@ export async function buildClientBundle(clientId: string, portId: string): Promi db.query.formSubmissions.findMany({ where: eq(formSubmissions.clientId, clientId), }), + db.query.supplementalFormTokens.findMany({ + where: and( + eq(supplementalFormTokens.portId, portId), + eq(supplementalFormTokens.clientId, clientId), + ), + }), db.query.documentSends.findMany({ where: and(eq(documentSends.portId, portId), eq(documentSends.clientId, clientId)), }), @@ -267,12 +310,16 @@ export async function buildClientBundle(clientId: string, portId: string): Promi company: toJsonRow(row.company), })), interests: interestRows.map(toJsonRow), + interestFieldHistory: fieldHistoryRows.map(toJsonRow), contactLog: contactLogRows.map(toJsonRow), tenancies: tenancyRows.map(toJsonRow), + berthWaitingList: waitingListRows.map(toJsonRow), + payments: paymentRows.map(toJsonRow), invoices: invoiceRows.map(toJsonRow), documents: documentRows.map(toJsonRow), files: fileRows.map(toJsonRow), formSubmissions: formSubmissionRows.map(toJsonRow), + supplementalFormTokens: supplementalTokenRows.map(toJsonRow), websiteSubmissions: websiteSubmissionRows.map(toJsonRow), documentSends: documentSendRows.map(toJsonRow), emailThreads: emailThreadBundle, @@ -360,12 +407,16 @@ export function renderBundleHtml(bundle: GdprBundle): string { })), ), tableSection('Interests', bundle.interests), + tableSection('Interest field history (overrides)', bundle.interestFieldHistory), tableSection('Contact log', bundle.contactLog), tableSection('Tenancies', bundle.tenancies), + tableSection('Berth waiting list', bundle.berthWaitingList), + tableSection('Payments (deposits / balances / refunds)', bundle.payments), tableSection('Invoices', bundle.invoices), tableSection('Documents', bundle.documents), tableSection('Files', bundle.files), tableSection('Form submissions', bundle.formSubmissions), + tableSection('Supplemental form tokens', bundle.supplementalFormTokens), tableSection('Website submissions (inquiry forms)', bundle.websiteSubmissions), tableSection('Document sends (PDFs / brochures emailed)', bundle.documentSends), tableSection(