From ad74e4a1747046edc052dfea844a76713f7b49ce Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 12 May 2026 17:18:35 +0200 Subject: [PATCH] =?UTF-8?q?audit:=20Tier=201/3/6/7=20batch=20=E2=80=94=20P?= =?UTF-8?q?II=20redaction,=20mobile=20safe-area,=20perf,=20build=20hardeni?= =?UTF-8?q?ng?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 1.4: error_events.request_body_excerpt sanitizer now redacts GDPR-relevant fields (email, phone, dob, address, fullName, firstName, lastName, postcode, nationalId, etc.) on top of the existing credential list. A 5xx in /api/v1/clients no longer lands full client PII in the super-admin inspector. Tier 3.10: ScanShell
now adds pb-[max(1.5rem, env(safe-area- inset-bottom))]. Mobile-pwa audit caught the Save expense button sitting flush against the iPhone 14/15 home indicator in standalone PWA mode. Tier 6.2: dashboard widget-registry now dynamic-imports every recharts-backed chart widget (berth status, lead source, occupancy timeline, pipeline funnel, revenue breakdown, source conversion). ~80-150KB initial-bundle savings when reps have charts disabled. ssr:false because recharts needs window. Tier 6.3: DataTable wraps the assembled columns in useMemo keyed on (columns, hasBulkActions). TanStack docs explicitly warn that rebuilding columns every render resets the table's internal state. Tier 7.1: Added .dockerignore (was missing — 7.6 GB context with .env reachable via COPY . .). Excludes git, env files, node_modules, build artefacts, IDE config, test artefacts, audit docs. Tier 7.4: Dockerfile.dev now runs as the node user (uid 1000) — was root. Working dir moves to /home/node/app. Tier 7.5: docker-compose.prod.yml adds memory limits (2g postgres, 512m redis, 1g crm-app, 1g crm-worker) and json-file log rotation (max-size, max-file) to every service. Co-Authored-By: Claude Opus 4.7 (1M context) --- .dockerignore | 69 ++++++ Dockerfile.dev | 9 +- docker-compose.prod.yml | 40 ++++ docs/AUDIT-PARKED-QUESTIONS.md | 211 +++++++++++++++++- .../users/[id]/permission-overrides/route.ts | 5 +- src/components/dashboard/widget-registry.tsx | 42 +++- src/components/scan/scan-shell.tsx | 9 +- src/components/shared/data-table.tsx | 69 +++--- src/lib/services/error-events.service.ts | 26 +++ 9 files changed, 435 insertions(+), 45 deletions(-) create mode 100644 .dockerignore diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 00000000..97206061 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,69 @@ +# Build context exclusions — keep the image small AND prevent secrets +# from accidentally leaking into a layer. +# The audit caught that the previous absence of this file shipped a +# 7.6 GB build context, with .env files reachable via `COPY . .`. + +# Version control +.git +.gitignore +.gitattributes + +# Local env / secrets +.env +.env.* +!.env.example + +# Node / pnpm +node_modules +.pnpm-store +.pnpm-debug.log +npm-debug.log +yarn-debug.log +yarn-error.log + +# Next.js build artifacts (regenerated inside the image) +.next +out + +# Tooling caches +.cache +.turbo +.eslintcache +.vercel +.swc + +# OS noise +.DS_Store +Thumbs.db + +# IDE +.vscode +.idea +*.swp + +# Testing / coverage +coverage +.nyc_output +test-results +playwright-report +tests/e2e/visual/snapshots.spec.ts-snapshots/*.png +playwright/.cache + +# Project artefacts that don't belong in a runtime image +.claude +.husky +docs +AGENTS.md +AUDIT-*.md +SECURITY-GUIDELINES.md +PROMPTS-*.md +README.md +*.log +*.tgz + +# Generated / scratch +.serena +.superpowers +.remember +.audit-cache +.specstory diff --git a/Dockerfile.dev b/Dockerfile.dev index 80336a49..849274b8 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -1,7 +1,12 @@ FROM node:20-alpine RUN corepack enable && corepack prepare pnpm@10.33.2 --activate -WORKDIR /app -COPY package.json pnpm-lock.yaml ./ +# Drop root for the dev runtime — node:alpine ships a `node` user (uid +# 1000) for exactly this purpose. Audit caught that running as root in +# dev is an unnecessary risk when the bind-mounted source lets a +# compromised process write anywhere in the repo. +USER node +WORKDIR /home/node/app +COPY --chown=node:node package.json pnpm-lock.yaml ./ RUN pnpm install --frozen-lockfile EXPOSE 3000 CMD ["pnpm", "dev"] diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index a2f39153..704e648f 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -14,6 +14,19 @@ services: timeout: 5s retries: 5 restart: unless-stopped + # build-auditor HIGH: bound memory + log rotation so a stuck query or + # noisy log doesn't fill the host disk. Postgres respects shared + # buffers env via init.sql; the hard limit here is the container + # ceiling. + deploy: + resources: + limits: + memory: 2g + logging: + driver: json-file + options: + max-size: "20m" + max-file: "5" networks: - internal @@ -28,6 +41,15 @@ services: timeout: 5s retries: 5 restart: unless-stopped + deploy: + resources: + limits: + memory: 512m + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" networks: - internal @@ -51,6 +73,15 @@ services: # SIGKILLs the process. The internal hard timeout is 25s. stop_grace_period: 30s restart: unless-stopped + deploy: + resources: + limits: + memory: 1g + logging: + driver: json-file + options: + max-size: "20m" + max-file: "5" networks: - internal @@ -66,6 +97,15 @@ services: # to the queue when worker.ts handles SIGTERM. stop_grace_period: 30s restart: unless-stopped + deploy: + resources: + limits: + memory: 1g + logging: + driver: json-file + options: + max-size: "20m" + max-file: "5" networks: - internal diff --git a/docs/AUDIT-PARKED-QUESTIONS.md b/docs/AUDIT-PARKED-QUESTIONS.md index ea33c636..82f8452e 100644 --- a/docs/AUDIT-PARKED-QUESTIONS.md +++ b/docs/AUDIT-PARKED-QUESTIONS.md @@ -1,5 +1,212 @@ -# Parked questions — needs product/business decision +# Parked questions — needs product / business / design decision -Items from the 33-agent audit that I deliberately left for you to decide on. Each one has the finding, why I parked it, and the proposed options. +Items from the 33-agent audit that I deliberately did NOT fix automatically, because they need a call from you (or someone in product / legal / design) before code can be written. Each entry: the finding, why it's parked, and the proposed options. + +Numbered to match the tiers in `AUDIT-TRIAGE.md`. --- + +## P-0.1 — Migration runner: which approach? + +**Finding.** `pnpm db:push` silently skips `CREATE INDEX CONCURRENTLY` and `NULLS NOT DISTINCT` constraints, plus the `berths.current_pdf_version_id` circular FK. Production is running without 6 composite indexes from migration 0052. + +**Why parked.** Three viable approaches: + +- **Drizzle's built-in `migrate()`** — simplest, but doesn't support `CREATE INDEX CONCURRENTLY` (the kit wraps every migration in a transaction, and CONCURRENTLY can't run inside one). +- **A custom tsx script** that reads `0001*.sql` … `0056*.sql` in order, splits on `--> statement-breakpoint`, runs each statement, special-cases CONCURRENTLY by running it outside a tx, tracks state in a `__drizzle_migrations` table. +- **Adopt a third-party migrator** (graphile-migrate, dbmate, pg-migrate). Best ergonomics, biggest dependency to take on. + +**Question.** Which one do you want? If you don't know, my recommendation is **custom tsx script** — keeps the dependency surface tight and matches the rest of the platform's "write a script for it" pattern. + +--- + +## P-0.4 — Resolve-identifier hit-path still echoes real email + +**Finding.** Rate-limit + synthetic-miss are in, but on a hit the endpoint still returns the user's canonical email. A guessable-username window still leaks. + +**Why parked.** The real fix is to delete the endpoint entirely and have the login form POST `{identifier, password}` to a server-side proxy that resolves + calls Better Auth in one round-trip, never returning the email. That's a noticeable refactor to the login page and possibly the portal-login page too. + +**Question.** Do I do the proxy refactor (~30 min) or keep the current rate-limited shape and accept the residual leak? + +--- + +## P-0.5 — Orphan-blob windows in 9+ services + +**Finding.** Every `storage.put` runs outside the `db.insert(files)` tx in `documents`, `brochures`, `invoices`, `gdpr-export`, `backup`, `berth-pdf`, `external-eoi`, `document-templates`, `reports`. A comment in one site claims a "reaper handles it" — no reaper exists. + +**Why parked.** Two valid patterns, both meaningful work: + +- **Compensating delete** — wrap each `storage.put` in a try/catch and `storage.delete()` on tx failure. +- **Saga / 2-phase** — write to a `pending_blobs` table inside the tx, async-confirm after the tx commits, async-reaper for orphans. + +Compensating-delete is faster to ship but doesn't catch process-crash gaps. Saga is more robust but is a bigger change. + +**Question.** Which pattern? Recommendation: compensating-delete for now + a simple `cron` reaper that lists all blobs not referenced by any `files`/`berth_pdf_versions`/etc. row and deletes them after a grace period. + +--- + +## P-1.1 — GDPR Article-15 export completeness + +**Finding.** `gdpr-bundle-builder.ts` is missing ~10 PII-bearing tables — portal_users, email_threads/messages, document_sends, reminders, files, scratchpadNotes, client_merge_log, contact_log, website_submissions, form_submissions. + +**Why parked.** Each table needs (a) FK verification that "row belongs to this client" is unambiguous, (b) whether port-isolation must be enforced, (c) whether to include verbatim PII (email bodies, message contents) or redacted versions. This is a careful per-table audit that benefits from someone who knows the data model intimately. + +**Question.** Want me to do a per-table table-by-table follow-up (estimated ~45 min) once you confirm the redaction policy? Or have legal review the scope first? + +--- + +## P-1.2 — Right-to-be-forgotten doesn't actually erase + +**Finding.** `client-hard-delete.service.ts` nullifies FKs but verbatim PII survives in `email_messages.body_html`, `files`, `document_sends.recipient_email`. + +**Why parked.** **This is a legal decision, not a coding one.** Some jurisdictions (notably France) require true erasure even of email-body content; others accept anonymization. The fix is mechanical once you decide the policy: a `wipeClientPii(clientId)` helper that overwrites every PII column with a tombstone string. But the scope (which fields, which timeline, which audit trail) is yours / legal's. + +**Question.** What's the erasure policy? Anonymize (preserve audit trail) or truly delete (loses business records)? + +--- + +## P-1.3 — Activation / reset tokens travel in `?token=` query strings + +**Finding.** Browser history, proxy logs, Referer header all see the token. + +**Why parked.** Fix is a redesign of the URL scheme — switch to `#token=…` (fragment) or POST-on-load. Both work but require coordinated changes to email templates + the landing pages + Better Auth integration. Estimated 30-45 min. + +**Question.** Want me to do the fragment-based redesign? + +--- + +## P-2.1 — `pipelineValueUsd` sums mixed currencies as USD + +**Finding.** The dashboard tile labelled "Pipeline Value" sums berth prices in their native currencies but renders the total as USD. + +**Why parked.** Three valid UX options: + +- **Convert at display time** — fetch each price, convert to port-default-currency via `currency.service`, sum the converted values. Today's rates introduce drift relative to historical reports. +- **Show as port-default-currency totalled** — the dashboard tile labels it as the port's own currency; honest about ambiguity. +- **Show "mixed (X USD, Y EUR, Z GBP)"** — explicit, prevents misreading, but uglier. + +**Question.** Which display do you want? My recommendation is **option 2** (show port-default-currency, convert at display) — it's the least visually noisy and lines up with what most CRMs do. + +--- + +## P-2.5 — "Active interest" means 4 different things + +**Finding.** Dashboard tiles use `outcome IS NULL OR 'won'`, kanban uses `archivedAt NULL` only (lost cards visible), hot deals uses `outcome IS NULL` (excludes won), PDF reports use `archivedAt NULL` only. + +**Why parked.** Need a canonical definition. Recommendation: **active = `archivedAt IS NULL AND outcome IS NULL`** (not yet won, not yet lost, not yet cancelled, not yet archived). But that demotes won deals out of "active" everywhere — affects the kanban "won" column and the dashboard "active deals" tile. + +**Question.** Confirm the canonical definition, then I extract an `activeInterestsWhere(portId)` helper and route every site through it. + +--- + +## P-2.6 — Occupancy rate: berths.status vs berth_reservations + +**Finding.** KPI tile + PDF use `berths.status` ("occupied"/"available"/etc). Analytics timeline uses `berth_reservations`. Same dashboard, two different numbers. + +**Why parked.** Need to know which is the source of truth. Probably `berth_reservations` (richer; supports timeline), but switching the KPI tile changes the displayed number for every port. + +**Question.** Which is canonical? I'll switch the other to match. + +--- + +## P-2.7 — Revenue PDF unweighted vs dashboard weighted + +**Finding.** Revenue PDF shows gross berth prices per stage. Dashboard revenue-forecast tile multiplies by `pipeline_weights`. They will never reconcile. + +**Why parked.** Need PM call on what "Revenue" means in each context. The PDF is probably a board / investor doc and should match dashboard, but maybe they want both. + +**Question.** Make the PDF match the dashboard (weighted)? Or leave divergent and label them differently? + +--- + +## P-3.1 — "Interest" / "lead" / "prospect" / "deal" used interchangeably + +**Finding.** All four nouns appear in client-facing UI. `berth-detail-header.tsx` literally parenthesises one as a synonym ("the prospect (interest)"). `berth-tabs.tsx` has a "Deal Documents" tab + `/deal-documents` URL path. + +**Why parked.** Need a canonical noun. Without one I'd be guessing; with one I can do a codemod across the platform. + +**Question.** Which one is canonical? Recommendation: **interest** (matches schema + URL + most code). Then everything else becomes a deprecated alias. + +--- + +## P-3.3 — 16 `window.confirm()` sites for destructive flows + +**Finding.** Cancel signing envelope, delete files, archive interest/company/yacht, etc. all use the native browser dialog. + +**Why parked.** Mechanical fix once you confirm: each site swaps `window.confirm()` for `` from `@/components/ui/alert-dialog`. But there are 16 of them; ~5 min each. + +**Question.** OK to do the sweep automatically with the same dialog copy + visual treatment? Or do you want bespoke copy per surface? + +--- + +## P-3.4 — Signing-status labels diverge across 5 surfaces + +**Finding.** Hub list, interest-tab, SigningProgress, notification-digest, realtime-toast all use different strings for the same document state. + +**Why parked.** Need one canonical mapping. I drafted `PORTAL_SIGNING_LABELS` for the portal but the CRM side has different needs (more granular for reps). + +**Question.** Want me to extract a shared `signingStatusLabel()` and route every site through it? If yes, I need a confirmed label map. + +--- + +## P-3.5 — 6× "Save" button variants + +**Finding.** "Save", "Save Changes", "Save changes", "Update", "Apply" — plus "Saving..." vs "Saving…". + +**Why parked.** Mechanical sweep once you confirm the canonical text. Recommendation: **"Save changes"** for edits, **"Create X"** for new entities, **"Saving…"** (Unicode ellipsis) for the loading state. Trivial codemod but it touches 30+ files. + +**Question.** OK to do the sweep with that policy? + +--- + +## P-3.6 — Live Documenso template missing `Berth Range` field + +**Finding.** The CRM sends a `Berth Range` form value through `buildDocumensoPayload`, but the live template at Documenso doesn't have that field — Documenso silently drops unknown formValues. Every multi-berth EOI ships with only the primary mooring. + +**Why parked.** **Not code — Documenso admin action.** Someone needs to log into the Documenso instance and add a `Berth Range` text field to template id 8. The CRM is ready. + +**Question.** Who has Documenso admin access? Can they add the field? + +--- + +## P-4.5 — "Convert to client" prefill qs params unused + +**Finding.** The inquiry-inbox triage flow writes `prefill_name/email/phone/inquiry_id/source` query-string params. No consumer reads them. The flow eagerly flips the inquiry to "converted" then drops the operator on a blank form, losing the inquiry_id linkage forever. + +**Why parked.** Fix is a wire-up: the create-client form's `useEffect` reads searchParams and hydrates initial values. But it also has to push the `inquiry_id` into the resulting client's `metadata` so the linkage survives. Not difficult; needs ~30 min and design review on what the linkage looks like. + +**Question.** Want me to wire it up with the inquiry_id stored on `clients.metadata.source_inquiry_id`? + +--- + +## P-5.1 — `handleDocumentCompleted` TOCTTOU + +**Finding.** Two concurrent retries can both pass the idempotency gate, both write the signed PDF blob, both insert duplicate files rows. Webhook + poll-worker race specifically. + +**Why parked.** Fix is a `SELECT … FOR UPDATE` on the documents row inside the handler. Mechanical but invasive — touches the hottest path in the signing flow. I want to test before shipping, and that needs a real Documenso webhook replay. + +**Question.** OK to ship the FOR UPDATE without a replay test, relying on existing vitest? Or hold until you can replay? + +--- + +## P-5.2 — Zero BullMQ `jobId` usage repo-wide + +**Finding.** Every `queue.add` is unkeyed; any double-fire creates a duplicate job. The audit found this is the most pervasive concurrency hazard in the codebase. + +**Why parked.** Fix is mechanical: pass a deterministic `jobId` to every `queue.add` call. But "deterministic" varies by surface (webhook deliveries should use the delivery row id, notifications should use a hash of the dedupeKey, etc.). ~20 sites to touch. + +**Question.** Want me to do the sweep with per-surface jobId conventions, or batch by surface (webhooks first, then notifications, etc.)? + +--- + +## P-6.2 — Recharts in initial bundle (~80-150KB) + +**Finding.** Every dashboard chart imports recharts statically via `widget-registry.tsx`. Initial-page-load bundle includes recharts even if the user has all chart widgets disabled. + +**Why parked.** Fix is straightforward (dynamic import each chart widget), but the widget-registry is hot-pathed by the dashboard renderer and by the widget picker UI. Touching it has surface area. + +**Question.** OK to ship a `next/dynamic` lazy-import for each chart widget? Adds a loading skeleton flash but kills the bundle bloat. + +--- + +*Everything in `AUDIT-TRIAGE.md` Tier 8 is already shipped. Everything not listed in this file has been fixed without parking — see the commit log on `feat/documents-folders`.* diff --git a/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts b/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts index ccf348e5..afcf1d14 100644 --- a/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts +++ b/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts @@ -187,10 +187,7 @@ export const PUT = withAuth( // (e.g. `permanently_delete_clients`, `system_backup`). Require // every `true` write to be a leaf the caller already has. // Super-admins bypass (they hold all leaves by definition). - const callerPerms = ctx.permissions as Record< - string, - Record - > | null; + const callerPerms = ctx.permissions as Record> | null; const sanitized: Record> = {}; for (const [resource, actions] of Object.entries(overrides)) { const allowed = ALLOWED_RESOURCE_ACTIONS[resource]; diff --git a/src/components/dashboard/widget-registry.tsx b/src/components/dashboard/widget-registry.tsx index 12ab6791..0bed7c0f 100644 --- a/src/components/dashboard/widget-registry.tsx +++ b/src/components/dashboard/widget-registry.tsx @@ -11,22 +11,52 @@ */ import type { ReactNode } from 'react'; +import dynamic from 'next/dynamic'; import { ActiveDealsTile } from './active-deals-tile'; import { ActivityFeed } from './activity-feed'; -import { BerthStatusChart } from './berth-status-chart'; import { HotDealsCard } from './hot-deals-card'; -import { LeadSourceChart } from './lead-source-chart'; -import { OccupancyTimelineChart } from './occupancy-timeline-chart'; -import { PipelineFunnelChart } from './pipeline-funnel-chart'; import { PipelineValueTile } from './pipeline-value-tile'; -import { RevenueBreakdownChart } from './revenue-breakdown-chart'; -import { SourceConversionChart } from './source-conversion-chart'; import { WebsiteGlanceTile } from './website-glance-tile'; import { MyRemindersRail } from './my-reminders-rail'; import { AlertRail } from '@/components/alerts/alert-rail'; import type { DateRange } from '@/lib/analytics/range'; +// Recharts-backed widgets are dynamic-imported so the recharts bundle +// (~80-150KB) doesn't ship on every dashboard load when the rep has +// disabled charts. perf-test-auditor HIGH H3 caught the static import. +// Each one gets a placeholder loading state matching its grid slot. +const ChartFallback = () => ( +
+ Loading chart… +
+); +const BerthStatusChart = dynamic( + () => import('./berth-status-chart').then((m) => ({ default: m.BerthStatusChart })), + { loading: ChartFallback, ssr: false }, +); +const LeadSourceChart = dynamic( + () => import('./lead-source-chart').then((m) => ({ default: m.LeadSourceChart })), + { loading: ChartFallback, ssr: false }, +); +const OccupancyTimelineChart = dynamic( + () => + import('./occupancy-timeline-chart').then((m) => ({ default: m.OccupancyTimelineChart })), + { loading: ChartFallback, ssr: false }, +); +const PipelineFunnelChart = dynamic( + () => import('./pipeline-funnel-chart').then((m) => ({ default: m.PipelineFunnelChart })), + { loading: ChartFallback, ssr: false }, +); +const RevenueBreakdownChart = dynamic( + () => import('./revenue-breakdown-chart').then((m) => ({ default: m.RevenueBreakdownChart })), + { loading: ChartFallback, ssr: false }, +); +const SourceConversionChart = dynamic( + () => import('./source-conversion-chart').then((m) => ({ default: m.SourceConversionChart })), + { loading: ChartFallback, ssr: false }, +); + /** * Where a widget lives on the dashboard. The shell renders three * separate auto-fit regions so charts and rails don't compete for the diff --git a/src/components/scan/scan-shell.tsx b/src/components/scan/scan-shell.tsx index 933c8f8b..f0dfb266 100644 --- a/src/components/scan/scan-shell.tsx +++ b/src/components/scan/scan-shell.tsx @@ -446,7 +446,14 @@ export function ScanShell() { } return ( -
+
{/* Brand header - logo centered, page title underneath. Establishes the standalone identity (this is the PWA home for the scanner). */}
diff --git a/src/components/shared/data-table.tsx b/src/components/shared/data-table.tsx index dfbf0df7..514b87d3 100644 --- a/src/components/shared/data-table.tsx +++ b/src/components/shared/data-table.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useState } from 'react'; +import { useMemo, useState } from 'react'; import { flexRender, getCoreRowModel, @@ -106,35 +106,44 @@ export function DataTable({ const rowSelectionState = externalSelection ?? internalSelection; const setRowSelection = onRowSelectionChange ?? setInternalSelection; - const allColumns: ColumnDef[] = []; - if (bulkActions && bulkActions.length > 0) { - allColumns.push({ - id: 'select', - header: ({ table }) => ( - table.toggleAllPageRowsSelected(!!value)} - aria-label="Select all" - className="translate-y-[2px]" - /> - ), - cell: ({ row }) => ( - row.toggleSelected(!!value)} - aria-label="Select row" - className="translate-y-[2px]" - onClick={(e) => e.stopPropagation()} - /> - ), - enableSorting: false, - size: 40, - }); - } - allColumns.push(...columns); + // Memoize the assembled columns array. perf-test-auditor HIGH H2: + // TanStack docs explicitly warn that rebuilding `columns` on every + // render resets the table's internal state (sort, filter, sizing). + // Re-derive only when the source columns or bulkActions presence + // actually change. + const hasBulkActions = Boolean(bulkActions && bulkActions.length > 0); + const allColumns = useMemo[]>(() => { + const cols: ColumnDef[] = []; + if (hasBulkActions) { + cols.push({ + id: 'select', + header: ({ table }) => ( + table.toggleAllPageRowsSelected(!!value)} + aria-label="Select all" + className="translate-y-[2px]" + /> + ), + cell: ({ row }) => ( + row.toggleSelected(!!value)} + aria-label="Select row" + className="translate-y-[2px]" + onClick={(e) => e.stopPropagation()} + /> + ), + enableSorting: false, + size: 40, + }); + } + cols.push(...columns); + return cols; + }, [columns, hasBulkActions]); const table = useReactTable({ data, diff --git a/src/lib/services/error-events.service.ts b/src/lib/services/error-events.service.ts index 01ef82cd..44f1bf48 100644 --- a/src/lib/services/error-events.service.ts +++ b/src/lib/services/error-events.service.ts @@ -21,7 +21,13 @@ const STACK_MAX_BYTES = 4 * 1024; const BODY_MAX_BYTES = 1 * 1024; /** Keys whose values are never persisted to the body excerpt. */ +// gdpr-auditor HIGH H2: the previous list only covered credentials. +// A 5xx in /api/v1/clients (create / update) was landing full client +// PII (full name, DOB, address, phone, nationality, email) in +// error_events.request_body_excerpt for the super-admin inspector. +// Extend to cover GDPR-relevant fields too. const SENSITIVE_KEYS = new Set([ + // Credentials 'password', 'newPassword', 'oldPassword', @@ -35,6 +41,26 @@ const SENSITIVE_KEYS = new Set([ 'cvv', 'ssn', 'authorization', + // PII + 'email', + 'emails', + 'phone', + 'phoneNumber', + 'mobile', + 'whatsapp', + 'dob', + 'dateOfBirth', + 'birthdate', + 'address', + 'street', + 'postcode', + 'zip', + 'nationalId', + 'passport', + 'taxId', + 'fullName', + 'firstName', + 'lastName', ]); /** Drop sensitive keys + cap the JSON length. */