Matt
0baca41693
Tier 0.2: src/lib/env.ts now refuses boot when NODE_ENV=production AND EMAIL_REDIRECT_TO is set. Sendmail logs the rewrite at warn (was debug) so dev/staging windows where someone forgets to unset are immediately visible. Tier 0.6: backup_jobs.storage_path added to TABLES_WITH_STORAGE_KEYS in src/lib/storage/migrate.ts. Flipping the storage backend used to silently orphan every pg_dump artefact — last-resort recovery path is now actually portable. Tier 1.7: createAuditLog now runs metadata through maskSensitiveFields (was only applied to old/new value diffs). Portal-auth, crm-invite, hard-delete and email-accounts services were writing raw emails into this column unbounded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
154 lines
19 KiB
Markdown
154 lines
19 KiB
Markdown
# Port Nimara CRM — Audit Triage (importance-grouped)
|
||
|
||
Companion to `AUDIT-2026-05-12.md`. Every line below is a real finding from the 33-agent audit, regrouped strictly by **impact × likelihood of biting you**, not by which domain found it. Tackle tiers top-down.
|
||
|
||
---
|
||
|
||
## Tier 0 — Stop-ship: do these in the next session
|
||
|
||
Anything here is a foot-gun that's actively armed in production right now.
|
||
|
||
| # | What | Where | Why now |
|
||
| --- | ------------------------------------------------------------------ | --------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||
| 0.1 | Build a real `db:migrate` runner | new tsx script | `pnpm db:push` silently skips `CREATE INDEX CONCURRENTLY` (6 indexes in 0052 never created) and skips 2 structural constraints. Every other "migration X exists" claim is unverifiable until this is fixed. |
|
||
| 0.2 | `EMAIL_REDIRECT_TO` prod refusal in `src/lib/env.ts` | env zod refine | One stray env value silently funnels every outbound (invites, EOI, portal magic links, contracts) to a single inbox. Only signal today is `logger.debug`. |
|
||
| 0.3 | Admin self-target audit-log retention + alerting | audit_logs metadata + retention cron | `audit_logs.metadata` not in `maskSensitiveFields`, no retention cron. PII grows unbounded; rotated-admin compromise is invisible. |
|
||
| 0.4 | Resolve-identifier hit-path still echoes the real email | `/api/auth/resolve-identifier/route.ts` | Rate-limit is in (just shipped), but on a hit we still return the canonical email. Replace with a server-side signIn proxy that takes `{identifier, password}` and never returns the email at all. |
|
||
| 0.5 | Orphan-blob windows in 9+ services | `documents`, `brochures`, `invoices`, `gdpr-export`, `backup`, `berth-pdf`… | Every `storage.put` runs outside the `db.insert(files)` tx. "Reaper handles it" comment is wrong — no reaper exists. Months of operation = hundreds of orphans. |
|
||
| 0.6 | `backup_jobs.storage_path` missing from `TABLES_WITH_STORAGE_KEYS` | `src/lib/storage/migrate.ts:55-60` | Flip the storage backend → silently orphans every pg_dump. Last-resort recovery path goes dark. |
|
||
|
||
---
|
||
|
||
## Tier 1 — Compliance / legal liability
|
||
|
||
Anything here puts the company in a regulator finding or a court case.
|
||
|
||
| # | What | Where |
|
||
| --- | ------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
||
| 1.1 | GDPR Article-15 export bundle is incomplete | `gdpr-bundle-builder.ts` — missing portal_users, email_threads/messages, document_sends, reminders, files, scratchpadNotes, client_merge_log, contact_log, website_submissions, form_submissions |
|
||
| 1.2 | Right-to-be-forgotten doesn't actually erase | `client-hard-delete.service.ts` — verbatim PII survives in `email_messages.body_html`, `files`, `document_sends.recipient_email` |
|
||
| 1.3 | Activation/reset tokens travel in `?token=` URL query strings | portal-auth flow — leaks to browser history, proxy logs, Referer headers |
|
||
| 1.4 | `error_events.request_body_excerpt` redacts password/token but not email/phone/name/dob/address | error-classifier sanitizer |
|
||
| 1.5 | `audit_logs` no retention cron + IP captured on routine events | `lib/audit.ts` — lawful-basis-questionable |
|
||
| 1.6 | S3 backend ships without `ServerSideEncryption` header | `S3Backend.put` — signed contracts, GDPR exports, pg_dumps cleartext at rest unless bucket default is set |
|
||
| 1.7 | `audit_logs.metadata` carries raw PII (full emails) at portal-auth, crm-invite, hard-delete, email-accounts service sites | `maskSensitiveFields` skips metadata |
|
||
|
||
---
|
||
|
||
## Tier 2 — Money/numbers correctness
|
||
|
||
Anything where the dashboard or a PDF lies to the user about money.
|
||
|
||
| # | What | Where |
|
||
| --- | -------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ |
|
||
| 2.1 | `pipelineValueUsd` sums mixed currencies as USD | `dashboard.service.ts:39-51`, KPI cards, pipeline-value tile, revenue forecast |
|
||
| 2.2 | Revenue PDF "TOTAL COMPLETED REVENUE" includes lost + cancelled | `report-generators.ts:126-140` — no outcome filter |
|
||
| 2.3 | Pipeline PDF crashes because `stageCounts` is missing `.groupBy()` | `report-generators.ts` |
|
||
| 2.4 | Hot-deals widget rank ladder uses wrong stage names (`'in_comms'`, `'deposit_10'`) | `dashboard.service.ts:198-208`, `hot-deals-card.tsx:26-36` |
|
||
| 2.5 | "Active interest" means **4 different things** across dashboard / kanban / hot deals / PDFs | extract `activeInterestsWhere(portId)` helper |
|
||
| 2.6 | Occupancy rate: KPI uses `berths.status`, analytics timeline uses `berth_reservations` — two different numbers on same dashboard | `dashboard.service.ts` |
|
||
| 2.7 | Revenue PDF unweighted vs dashboard weighted-by-`pipeline_weights` — will never reconcile | `report-generators.ts` |
|
||
| 2.8 | `expenses.amountUsd` snapshot uses edit-time rate not `expenseDate`; nulls when Frankfurter is down | `expenses.service.ts` |
|
||
| 2.9 | `convert()` rounds 2dp regardless of currency (JPY broken); invoice math has no rounding (sub-cent drift) | `currency.service.ts`, invoice math |
|
||
|
||
---
|
||
|
||
## Tier 3 — Customer-visible polish (embarrassing in front of clients)
|
||
|
||
| # | What | Where |
|
||
| ---- | ------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- |
|
||
| 3.1 | "Interest" / "lead" / "prospect" / "deal" used interchangeably in client-facing UI | `berth-detail-header.tsx`, `berth-tabs.tsx` "Deal Documents", `client-interests-tab.tsx`, `interest-tabs.tsx` |
|
||
| 3.2 | Portal renders raw machine enums to clients ("EOI: waiting_for_signatures", "hot lead") | `/portal/interests/page.tsx:80` |
|
||
| 3.3 | 16 destructive flows use native `window.confirm()` | cancel signing envelope, delete files, archive interest/company/yacht |
|
||
| 3.4 | Signing-status labels diverge across 5 surfaces (Hub / list / interest-tab / SigningProgress / notification-digest / realtime-toast) | normalize through one helper |
|
||
| 3.5 | 6× "Save" button variants ("Save" / "Save Changes" / "Save changes") + 6× "Saving..." vs "Saving…" | sweep |
|
||
| 3.6 | Live Documenso template missing `Berth Range` field — every multi-berth EOI ships with primary mooring only | Documenso admin |
|
||
| 3.7 | URL interpolations in every email template are unescaped (`href="${data.link}"`) — a `"` in any URL breaks out | escape + scheme allow-list in `shell.ts` |
|
||
| 3.8 | Admin email-template subject editor silently does nothing on 5 of 8 templates | wire `overrides.subject` |
|
||
| 3.9 | `/admin/email` Signature/Footer HTML fields write keys the shell never reads | wire `cfg.footerHtml` or delete fields |
|
||
| 3.10 | Mobile scan PWA "Save expense" sits flush against iPhone home indicator | safe-area-inset on ScanShell `<main>` |
|
||
|
||
---
|
||
|
||
## Tier 4 — Authz / cross-tenant integrity
|
||
|
||
| # | What | Where |
|
||
| --- | ---------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ |
|
||
| 4.1 | Port admin with only `admin.manage_users` can grant other users any leaf they don't hold themselves (sock-puppet escalation) | permission-overrides PUT + `updateUser` role reassignment — require caller-superset before write |
|
||
| 4.2 | `/api/v1/alerts` GET is ungated | add `admin.view_audit_log` |
|
||
| 4.3 | Webhooks bypass the platform-error pipeline entirely | `documenso/route.ts` — `captureErrorEvent` on handler throw, apply to all webhook routes |
|
||
| 4.4 | Search graph-expansion writes into all merged buckets without re-checking per-bucket `view` permission | `search.service.ts:1893-1915` — gate each merge call |
|
||
| 4.5 | "Convert to client" writes prefill qs params no consumer reads; inquiry_id linkage dropped forever | inquiry-inbox triage flow |
|
||
| 4.6 | Inquiry email dedup is case-sensitive (capital-letter resubmits = duplicate client+yacht+interest) | `lower()` on `clientContacts.value === data.email` |
|
||
|
||
---
|
||
|
||
## Tier 5 — Concurrency / data races
|
||
|
||
| # | What | Where |
|
||
| --- | -------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- |
|
||
| 5.1 | `handleDocumentCompleted` idempotency gate is TOCTTOU under webhook+poll race — duplicate files rows + orphan blob | `documents.service.ts:1100-1253` — `SELECT … FOR UPDATE` or pre-claim transition |
|
||
| 5.2 | **Zero BullMQ `jobId` usage repo-wide** — every queue.add is unkeyed, any double-fire creates a duplicate job | every `queue.add` site |
|
||
| 5.3 | `advanceStageIfBehind` reads stage outside any lock — parallel DOCUMENT_SIGNED + DOCUMENT_COMPLETED double-run berth rules | wrap in tx |
|
||
| 5.4 | `moveFolder` cycle check outside a tx — two concurrent moves can create A↔B cycles | wrap in tx |
|
||
| 5.5 | Berth-PDF upload writes blob _before_ acquiring advisory lock — orphans on tx-rollback | reorder |
|
||
| 5.6 | `user_email_changes` has no partial unique index on pending rows — spam-email vector | add partial unique |
|
||
|
||
---
|
||
|
||
## Tier 6 — Perf / scale (silent today, painful at 10× traffic)
|
||
|
||
| # | What | Where |
|
||
| --- | ----------------------------------------------------------------------------------------------------------- | ---------------------- |
|
||
| 6.1 | Documents tab opens with ~50 sequential queries via fetchWorkflowGroupRows | `documents.service.ts` |
|
||
| 6.2 | Recharts statically imported in `widget-registry.tsx` — every dashboard chart in initial bundle (~80-150KB) | lazy import |
|
||
| 6.3 | `DataTable` rebuilds `allColumns` every render (no useMemo) — resets TanStack internal state | memo |
|
||
| 6.4 | `tiptap-to-pdfme.ts` (571 lines) ships to client just to re-export TEMPLATE_VARIABLES | split |
|
||
| 6.5 | `listUsers` runs 2 sequential queries with no pagination, returns all super-admins globally | paginate |
|
||
| 6.6 | `command-search` invalidates 2 queries every dropdown open — defeats its own 30s staleTime | drop invalidates |
|
||
|
||
---
|
||
|
||
## Tier 7 — Build / deploy hardening
|
||
|
||
| # | What | Where |
|
||
| --- | --------------------------------------------------------------------------------------------------------------- | -------------- |
|
||
| 7.1 | No `.dockerignore` → 7.6 GB build context, secrets/.env leak risk via `COPY . .` | add |
|
||
| 7.2 | `socket.io` + `@socket.io/redis-adapter` not in `serverExternalPackages`; runner stage installs no runtime deps | next.config.ts |
|
||
| 7.3 | Prod CSP keeps `'unsafe-inline'` on script-src | tighten |
|
||
| 7.4 | `Dockerfile.dev` runs as root | non-root user |
|
||
| 7.5 | Compose has no memory/CPU/log-rotation limits | add |
|
||
| 7.6 | `@types/node@^25` against Node-20 runtime — type checker greenlights APIs that don't exist | pin to ^20 |
|
||
| 7.7 | `node:20-alpine` base image at/past EOL | bump to 22 |
|
||
|
||
---
|
||
|
||
## Tier 8 — Already fixed in this session (don't redo)
|
||
|
||
Already on `feat/documents-folders`:
|
||
|
||
- Permission-overrides self-target privilege escalation block + canonical allow-list + cross-tenant guard
|
||
- `/api/auth/resolve-identifier` rate-limit + synthetic miss email
|
||
- Admin email-change updates `account.accountId` + revokes sessions
|
||
- Middleware `PUBLIC_PATHS` for email confirm/cancel tokens
|
||
- NAV_CATALOG dead-link sweep (10 entries)
|
||
- formatRole / formatOutcome / stageLabel applied across user-list, user-card, role-list, sidebar, command-search, realtime-toasts, interest-detail-header, client-columns, yacht-tabs, interest-picker, next-in-line-notify, AI worker, PDF reports
|
||
- Optional username sign-in (migration 0054)
|
||
- Per-user permission overrides (migration 0055) + UserPermissionMatrix
|
||
- UserForm: first/last + admin email change + auto-notify template + PhoneInput
|
||
- User disable button
|
||
|
||
---
|
||
|
||
## Tier 9 — Nice-to-haves + AI opportunities (not blocking)
|
||
|
||
Forward-looking (improvements-auditor):
|
||
|
||
- **AI-where-it-actually-helps:** semantic search across notes + email threads, auto-summarise client history on detail-page open, anomaly detection on expenses paired with existing OCR.
|
||
- **What NOT to AI-ify:** legal docs, EOI/contract field merges, money flow, regulatory text.
|
||
- **Subtle UX wins:** keyboard shortcuts (j/k list nav, e to edit), smarter defaults (last-used port/currency/source), undo for accidental archives, "what changed since I last looked" digest.
|
||
|
||
---
|
||
|
||
_Pick a tier and we open it._
|