From 312779c0c5d6a00eff428899e3a36a641b82c42a Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Tue, 5 May 2026 18:33:13 +0200 Subject: [PATCH] fix(security): tier-0 audit blockers (next CVE, role gate, perm traps, key validation, rate limits) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the five highest-risk findings from docs/audit-comprehensive-2026-05-05.md so the platform is not exposed while the rest of the audit backlog (1 CRIT + 18 HIGH + 32 MED + 23 LOW) is worked through: * CVE-2025-29927 — bump next 15.1.0 → 15.2.9; nginx strips X-Middleware-Subrequest at the edge as defense-in-depth. * Cross-tenant role escalation — POST/PATCH/DELETE on /admin/roles now require super-admin (was: any holder of admin.manage_users). Adds shared `requireSuperAdmin(ctx)` helper. * Silent-403 traps — `documents.edit` and `files.edit` keys added to RolePermissions; seeded role values updated; migration 0041 backfills the new keys on every existing roles+port_role_overrides JSONB. File routes remap the dead `create` action to `upload` / `manage_folders`. * Berth-PDF / brochure register endpoints — reject body.storageKey unless it matches the namespace the matching presign endpoint issued (prevents repointing a tenant's PDF at foreign-port bytes). * Portal auth rate limits — sign-in 5/15min/(ip,email), forgot-password 3/hr/IP, activate/reset/set-password 10/hr/IP. Adds `enforcePublicRateLimit()` for non-`withAuth` routes. Test status unchanged: 1168/1168 vitest, tsc clean. Refs: docs/audit-comprehensive-2026-05-05.md (CRITICAL, HIGH §§1–4) Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/audit-comprehensive-2026-05-05.md | 1126 +++++++++++++++++ nginx/conf.d/proxy_params.conf | 4 + package.json | 4 +- pnpm-lock.yaml | 107 +- src/app/api/auth/set-password/route.ts | 5 + src/app/api/portal/auth/activate/route.ts | 5 + .../api/portal/auth/forgot-password/route.ts | 7 + .../api/portal/auth/reset-password/route.ts | 5 + src/app/api/portal/auth/sign-in/route.ts | 15 +- .../v1/admin/brochures/[id]/versions/route.ts | 19 +- src/app/api/v1/admin/roles/[id]/route.ts | 63 +- src/app/api/v1/admin/roles/route.ts | 37 +- .../v1/berths/[id]/pdf-versions/handlers.ts | 17 + .../api/v1/files/folders/[...path]/route.ts | 2 +- src/app/api/v1/files/folders/route.ts | 2 +- src/app/api/v1/files/upload/route.ts | 2 +- src/lib/api/helpers.ts | 27 +- src/lib/api/route-helpers.ts | 63 +- .../0041_role_permissions_edit_keys.sql | 58 + src/lib/db/migrations/meta/_journal.json | 7 + src/lib/db/schema/users.ts | 2 + src/lib/db/seed.ts | 15 +- src/lib/rate-limit.ts | 11 + tests/helpers/factories.ts | 12 +- 24 files changed, 1489 insertions(+), 126 deletions(-) create mode 100644 docs/audit-comprehensive-2026-05-05.md create mode 100644 src/lib/db/migrations/0041_role_permissions_edit_keys.sql diff --git a/docs/audit-comprehensive-2026-05-05.md b/docs/audit-comprehensive-2026-05-05.md new file mode 100644 index 0000000..a3d4d74 --- /dev/null +++ b/docs/audit-comprehensive-2026-05-05.md @@ -0,0 +1,1126 @@ +# Comprehensive platform audit — 2026-05-05 + +12-domain agent-team audit of Port Nimara CRM. Each domain owned by a +single teammate working in an iTerm2 split pane; reports consolidated +here. + +## Test status at audit start + +- `pnpm exec vitest run` — **1168/1168 passing** (103 test files; 5.58s) +- `pnpm exec tsc --noEmit` — **clean** (exit 0) + +## Findings summary + +- **CRITICAL:** 1 +- **HIGH:** 18 +- **MEDIUM:** 32 +- **LOW:** 23 + +(94 total. Severity counts include all 12 domain reports; clean +domains and `audit-final-deferred.md` items are not duplicated here.) + +## Top 3 most critical + +1. **`pnpm-lock.yaml` + `src/middleware.ts` — Next.js CVE-2025-29927 + middleware authorization bypass.** `next@15.1.0` is vulnerable; + attacker sends `x-middleware-subrequest: middleware:middleware:…` + to skip middleware execution entirely, defeating the CSRF Origin + gate on every authed `/api/v1/**` mutation. **(auditor-K)** + +2. **`src/lib/services/roles.service.ts:11-141` — Global roles can be + mutated cross-tenant.** Any port admin holding `admin.manage_users` + can `PATCH /api/v1/admin/roles/{id}` to rewrite a role's + `permissions` JSONB; that role is assigned to users in every other + port via `userPortRoles`. Full cross-tenant privilege escalation. + **(auditor-B3)** + +3. **`src/app/api/v1/documents/[id]/route.ts:31` (+5 more) and + `src/app/api/v1/files/folders/route.ts:15` (+3 more) — silent-403 + traps from non-existent `withPermission` resource keys.** + `documents.edit`, `files.create`, `files.edit` are not in + `RolePermissions` (`src/lib/db/schema/users.ts:28-130`); every + non-superadmin call to those routes is a silent 403. Six document + mutation endpoints + four file mutation endpoints are unreachable + for any non-superadmin role no matter how flags are set. + **(auditor-A3)** + +--- + +## CRITICAL + +### `pnpm-lock.yaml` + `src/middleware.ts:71` — Next.js CVE-2025-29927 middleware authorization bypass + +- **Source:** auditor-K (Issue 1) +- **Description:** `next@15.1.0` (package.json:72, lockfile confirmed) + is vulnerable to CVE-2025-29927. Sending + `x-middleware-subrequest: middleware:middleware:middleware:middleware:middleware` + causes Next to skip middleware execution. Patched in `>=15.2.3`. +- **Impact:** `src/middleware.ts:46-86` is the only layer enforcing the + CSRF Origin-equality check on state-changing `/api/v1/**` requests. + Bypassing middleware lets a malicious origin issue cookie-bearing + POST/PUT/PATCH/DELETE without the origin header gate. Combined with + `SameSite=Lax` cookies and any stolen cross-site context, this + re-opens CSRF on every authed mutating route. Page-level auth + (login redirect) is also stripped. +- **Recommendation:** Bump to `next@15.2.3` or later. Add + `proxy_set_header X-Middleware-Subrequest ""` in nginx as + defense-in-depth. Add an integration test that sends the header and + asserts middleware still runs. + +--- + +## HIGH + +### `src/lib/services/roles.service.ts:11-141` — Global roles mutated by per-port admins + +- **Source:** auditor-B3 (Issue 1) +- **Description:** `roles` schema (`src/lib/db/schema/users.ts:213-227`) + has no `port_id`; `isGlobal` defaults true. + `createRole`/`updateRole`/`deleteRole` filter only by `roles.id` and + never by tenant. The PATCH route gates on the per-port + `admin.manage_users` permission — no `isSuperAdmin` check. +- **Impact:** Port-A admin can `PATCH /api/v1/admin/roles/{id}` to + rewrite the `permissions` JSONB of a role assigned via + `userPortRoles` to users in port-B/port-C. Full cross-tenant + privilege escalation. +- **Recommendation:** Gate role-mutating routes on `ctx.isSuperAdmin` + (mirrors `admin/ports/[id]/route.ts:15-20`), OR port-scope roles via + `port_role_overrides` only and refuse base-role mutation outside + super-admin. + +### `src/app/api/v1/documents/[id]/route.ts:31` (+5) — `documents.edit` is a silent-403 trap + +- **Source:** auditor-A3 (Issue 1) +- **Description:** `RolePermissions.documents` declares only + `view | create | send_for_signing | upload_signed | delete`. There + is no `edit` key, so `resourcePerms[action]` returns `undefined` and + `withPermission` 403s every non-superadmin. Affected: + `documents/[id]/route.ts:31` (PATCH), `documents/[id]/cancel/route.ts:8`, + `documents/[id]/remind/route.ts:15`, + `documents/[id]/upload-signed/route.ts:8`, + `documents/[id]/watchers/route.ts:25`, + `documents/[id]/watchers/[userId]/route.ts:8`. +- **Recommendation:** Replace the action with `'upload_signed'` / + `'send_for_signing'` (currently a dead key) per route, OR add an + `edit` key to the schema + every seeded role. + +### `src/app/api/v1/files/folders/route.ts:15` (+3) — `files.create`/`files.edit` are silent-403 traps + +- **Source:** auditor-A3 (Issue 2) +- **Description:** `RolePermissions.files` declares only + `view | upload | delete | manage_folders`. `create` and `edit` are + not keys. `files/folders/route.ts:15` (POST 'create'), + `files/upload/route.ts:9` (POST 'create'), + `files/folders/[...path]/route.ts:23` (PATCH 'edit'), + `files/[id]/route.ts:21` (PATCH 'edit') all silently 403 every + non-superadmin. +- **Recommendation:** Map POSTs to `('files','upload')`, folder PATCH + to `('files','manage_folders')`, files PATCH to a new `edit` key. + +### 12 sites — direct MinIO imports bypass the storage abstraction + +- **Source:** auditor-D (Issue 1) +- **Description:** CLAUDE.md says "Code never imports MinIO/S3 + directly. All file I/O goes through `getStorageBackend()`." Twelve + sites still call `minioClient.{put,get,remove}Object` / + `getPresignedUrl` directly: + `documents.service.ts:23,552,662,863`; + `document-templates.ts:16,528,664,813`; + `invoices.ts:22,598`; + `files.ts:10,53,96,107,187`; + `gdpr-export.service.ts:24,166`; + `portal.service.ts:14,304`; + `email-compose.service.ts:117-118`; + `queue/workers/maintenance.ts:11,90`; + `app/api/v1/files/folders/route.ts:7,33`; + `app/api/v1/files/folders/[...path]/route.ts:7,42,45,68`. +- **Impact:** Filesystem-mode deployments silently break — uploads, + GDPR export, invoice PDFs, file lists, portal downloads, folder + rename/delete all hit a non-existent S3 endpoint and return 500. + The factory cache, presigned-URL HMAC, and admin storage migration + are bypassed. +- **Recommendation:** Replace each site with + `(await getStorageBackend()).{put,get,delete}(...)` and + `presignDownload(...)`. Convert folder-marker objects in + `/files/folders/*` to a DB-backed virtual-folder representation + (filesystem mode has no zero-byte marker objects). Move + `getPresignedUrl` from `@/lib/minio` to a deprecation shim. + +### `src/lib/services/documenso-client.ts:23-45,244-256,358-371,390-419,470-486` — Documenso fetch helpers lack timeout + +- **Source:** auditor-D (Issue 2) +- **Description:** `documensoFetch`, `downloadSignedPdf`, v2 + `placeFields`, v1 per-field POST loop, and `voidDocument` all call + `fetch(...)` with no `signal`/AbortController. A hung Documenso + instance holds the calling worker indefinitely; `documents` queue + concurrency is 3, so three hung calls saturate the worker. +- **Recommendation:** Lift the 30s `AbortController` pattern from + `ai.ts:149-178` into a shared + `fetchWithTimeout(url, init, ms = 30_000)` helper and route every + external `fetch` through it (Documenso, OCR providers, Umami, + Frankfurter). + +### `src/lib/services/ocr-providers.ts:75-105,107-153` — OCR providers have no timeout + +- **Source:** auditor-D (Issue 3) +- **Description:** `runOpenAi` uses `new OpenAI({ apiKey })` with + default settings (no `timeout`). `runClaude` is raw `fetch` without + `signal`. A stuck connection holds the calling Bull `documents` + worker concurrency slot until OS reset (~15 min). +- **Impact:** One slow Anthropic incident → all three `documents` + worker slots tied up → receipt scans queue silently. +- **Recommendation:** Pass `timeout: 30_000` to the `OpenAI` + constructor; wrap `runClaude` with the shared timeout helper from + the prior finding. + +### `src/server.ts:22-60` — Web container has no SIGTERM handler + +- **Source:** auditor-K (Issue 2) +- **Description:** `worker.ts:36-37` registers SIGTERM/SIGINT and + awaits `worker.close()` before exiting. `server.ts` does not — the + HTTP server, Socket.io, and the inline BullMQ workers (dev) are + never closed. Compose default stop-grace is 10s; the process gets + SIGKILL'd. +- **Impact:** Every prod deploy / `docker compose up -d` rolling + restart drops every in-flight request, every Socket.io frame, and + every Postgres/Redis connection mid-statement. Affects file uploads + (50MB body), EOI generation, document signing webhooks, and + Documenso requests in flight. +- **Recommendation:** Add + `process.on('SIGTERM'|'SIGINT', () => httpServer.close(() => process.exit(0)))` + plus `io.close()` and `redis.disconnect()`. Set compose + `stop_grace_period: 30s`. + +### `src/lib/storage/filesystem.ts:192` + missing from `src/lib/env.ts` — `MULTI_NODE_DEPLOYMENT` runtime gate not in env schema + +- **Source:** auditor-K (Issue 3) +- **Description:** `filesystem.ts:192` reads + `process.env.MULTI_NODE_DEPLOYMENT === 'true'` directly. The + variable is not declared in `env.ts`'s zod schema and not in + `.env.example`. A typo (`MULTI_NODE_DEPLOY=true`, + `MULTINODE_DEPLOYMENT=true`, or simply unset) silently disables the + multi-node guard. +- **Impact:** The CLAUDE.md invariant "filesystem backend refuses to + start when multi-node" relies entirely on this string match. A + misconfigured prod deploy with two app nodes + filesystem backend + silently splits per-node storage — receipts visible from one node, + gone from the other. +- **Recommendation:** Add + `MULTI_NODE_DEPLOYMENT: z.enum(['true','false']).default('false').transform(v=>v==='true')` + to `env.ts` and import from `env`, not `process.env`. + +### `src/lib/env.ts:28-31` — Documenso recipient/template IDs hardcoded as global env defaults + +- **Source:** auditor-K (Issue 4) +- **Description:** `DOCUMENSO_TEMPLATE_ID_EOI=8`, + `DOCUMENSO_CLIENT_RECIPIENT_ID=192`, + `DOCUMENSO_DEVELOPER_RECIPIENT_ID=193`, + `DOCUMENSO_APPROVAL_RECIPIENT_ID=194` are tenant-specific magic + numbers from one Documenso instance, baked in as process-global + defaults. Used in `document-templates.ts:883-925`. +- **Impact:** Multi-tenant deploy is a lie — every port shares the + same EOI template + same Documenso recipient IDs. Adding a second + port with its own Documenso template requires running two CRM + processes per port, or redeploying with new envs and breaking the + first port. +- **Recommendation:** Move to `system_settings` keyed by `port_id` + (`documenso_template_id_eoi`, `documenso_recipient_*`); fall back + to env only as bootstrap. + +### `src/lib/services/reminders.service.ts:441` — N+1 client lookup inside hourly follow-up cron + +- **Source:** auditor-I (Issue 1) +- **Description:** `processFollowUpReminders` loops every port → every + enabled interest, then runs `db.query.clients.findFirst` per + interest plus `insert(reminders)` and `update(interests)` + sequentially per row. Three round-trips per interest. +- **Impact:** A port with 1000 follow-up-enabled interests does 3000+ + sequential DB calls per hour; tail latency dominated by network RTT + × N. Blocks the maintenance worker (concurrency 1). +- **Recommendation:** Pre-fetch all `clients` for the port via + `inArray(clients.id, clientIds)` once into a Map, then bulk-insert + reminders in a single `db.insert(...).values([...])` and + `update().where(inArray(...))`. + +### `src/lib/services/portal.service.ts:254` — Portal `getClientInvoices` fetches every port invoice + +- **Source:** auditor-I (Issue 2) +- **Description:** + `db.select().from(invoices).where(eq(invoices.portId, portId))` + returns every invoice in the port; `Array.filter` matches by + `billingEmail`. No `inArray` SQL filter, no `limit`. +- **Impact:** Every portal invoice page-load full-scans the invoices + table. After 12 months of operation this is the worst portal + endpoint. +- **Recommendation:** `where(and(eq(portId), inArray(sql\`lower(billingEmail)\`, emailContacts)))` + - `.limit(100)` defensively. + +### `src/lib/services/interest-scoring.service.ts:222` — `calculateBulkScores` fans out 4 `count()` per interest + +- **Source:** auditor-I (Issue 3) +- **Description:** `Promise.allSettled(allInterests.map(...))` — each + call issues 1 redis lookup, 1 interests.findFirst, then 4 `count()` + queries (notes, reminders, emails, interestBerths). +- **Impact:** Port with 1000 interests → ~6000 DB round trips on cold + cache. Used by the dashboard "interest scores" panel and the alerts + engine indirectly. Redis cache band-aid; flush is a latency cliff. +- **Recommendation:** Replace per-interest counts with one grouped + query each (`groupBy(notes.interestId)` filtered by `inArray` on + the port's interest IDs); merge in JS. + +### `src/lib/services/document-reminders.ts:204` → `sendReminderIfAllowed` — invariants re-fetched per doc + +- **Source:** auditor-I (Issue 4) +- **Description:** For every active document, `sendReminderIfAllowed` + re-fetches: doc, template-by-type, last reminder event, port + (timezone). Port lookup is invariant across the loop; templates by + `documentType` repeat heavily. +- **Impact:** A port with 500 in-flight `sent`/`partially_signed` + documents fires 5×500 = 2500 round trips per cron tick (every 15 + min default). +- **Recommendation:** Hoist port + per-type templateMap fetches above + the loop. Bulk-fetch `lastReminderAt` per doc with + `groupBy(documentEvents.documentId)`. Pre-load pendingSigners with + `inArray(documentSigners.documentId, docIds)`. + +### `src/app/api/v1/website-analytics/route.ts:110` (+10 more) — Manual 5xx responses bypass `error_events` persistence + +- **Source:** auditor-F (Issue 2), cross-checked auditor-G (Issue 3) +- **Description:** 11 routes manually emit 500/502/503 in catch + blocks instead of throwing. Sites: `portal/invoices/route.ts:13`, + `portal/dashboard/route.ts:18`, `portal/documents/route.ts:13`, + `portal/documents/[documentId]/download/route.ts:24`, + `portal/interests/route.ts:13`, `public/berths/route.ts:128`, + `public/berths/[mooringNumber]/route.ts:104`, + `public/website-inquiries/route.ts:67,176`, + `website-analytics/route.ts:110` (502), + `storage/[token]/route.ts:232`. +- **Impact:** Per docs/error-handling.md, every 5xx must round-trip + through `errorResponse()` so `captureErrorEvent` writes the + `error_events` row. These routes log to pino but produce no + `/admin/errors/` entry; users see "Failed to load X" with no + Reference ID to copy. +- **Recommendation:** Throw + `return errorResponse(err)`. For + website-analytics 502 (Umami upstream): introduce + `UMAMI_UPSTREAM_ERROR` (status 502). + +### `src/lib/services/documenso-client.ts:41,251,369,416,484` — Documenso surfaces upstream HTTP status as bare Error + +- **Source:** auditor-G (Issue 2), overlaps auditor-D (Issue 4) +- **Description:** Five throw sites format + `Documenso API error: ${res.status}`. These bubble as 500 with the + generic message; user can't distinguish "Documenso down" from "DB + issue". `error-classifier.ts` STACK_PATH_HINTS catches them via + stack but the user-facing toast is generic. +- **Recommendation:** Add `DOCUMENSO_UPSTREAM_ERROR` (502) and + `DOCUMENSO_AUTH_FAILURE` (502) codes. + +### 76 sites — Mutation/handler sites bypass `toastError(err)`, request IDs invisible to users + +- **Source:** auditor-H (Issue 1) +- **Description:** 38 distinct files; reps: + `src/components/shared/inline-editable-field.tsx:88`, + `inline-tag-editor.tsx:48`, `inline-phone-field.tsx:69`, + `addresses-editor.tsx:122,365`, + `send-document-dialog.tsx:140,152`, + `clients/contacts-editor.tsx:164,173,346,357`, + `clients/gdpr-export-button.tsx:82,95`, + `documents/document-detail.tsx:154,166,180,360`, + `documents/create-document-wizard.tsx:161`, + - 20 more files. All call `toast.error(err instanceof Error ? err.message : '…')` + directly. CLAUDE.md requires `toastError(err)` from + `src/lib/api/toast-error.ts` so the request-id + error-code surface + reaches users. +- **Impact:** Every failed inline-edit, save, send, archive, or + upload prints a bare message with no Reference ID and no Copy-ID + action. The new admin error inspector (`/errors/[id]`) is unusable + from user-reported issues because the user has no ID to give + support. +- **Recommendation:** Codemod sweep. Shared inline-\* editors (~6 + files) cover the highest call volume; do those first. + +### `src/components/admin/roles/role-form.tsx:130,204-206,286` (+6 admin form files) — Admin custom forms hand-roll `setError(message)` instead of `toastError` + +- **Source:** auditor-H (Issue 2) +- **Description:** 7 admin forms (roles, users, ports, webhooks, + custom-fields, document-templates, tags) capture + `err instanceof Error ? err.message : 'Something went wrong'` into + local state and render a plain banner. Same data loss as Issue + above — `ApiError.code` and `ApiError.requestId` stripped. +- **Recommendation:** Route through `toastError(err)` and drop the + inline banner, OR extract `getErrorDisplay(err)` from + `toast-error.ts` so the banner can show "Error code / Reference + ID" too. + +### Five auth-critical/admin-tier services have zero direct test coverage + +- **Source:** auditor-J (Issue 1, top-5 of 30) +- **Description:** 30 of 83 services lack any direct test import. + Top-5 by risk: `portal-auth.service.ts` (full lifecycle — + password hashing, token-hash equality, activation TTL, + multi-tenant), `users.service.ts` (admin-tier + create/update/remove), `email-accounts.service.ts` (AES-256-GCM at + rest), `document-sends.service.ts` (rate-limit, file-size + threshold, XSS-safe body, recipient port-match — + service-level audit-deferred isolation gap is uncovered), + `sales-email-config.service.ts` (decrypts SMTP/IMAP creds, redacts + on response). +- **Recommendation:** Each test should include happy path AND a + "different `portId` → not found / no-op / not-decrypted" assertion. + For Drizzle services that take `(id, portId)`, the negative-path + test is one extra `it()` per export. + +### `src/app/api/webhooks/documenso/route.ts:1` — Documenso webhook receiver has no integration test + +- **Source:** auditor-J (Issue 2) +- **Description:** The receiver handles `DOCUMENT_SIGNED`, + `DOCUMENT_COMPLETED`, etc. and dispatches by global `documensoId` + without `port_id` enforcement (audit-deferred). No test exists. + `tests/unit/webhook-event-map.test.ts` only validates the static + event-name map; `tests/integration/documents-expired-webhook.test.ts` + calls `handleDocumentExpired()` directly, bypassing the route, + signature check, dedup, and per-recipient loop. +- **Recommendation:** Add `tests/integration/documenso-webhook-route.test.ts`: + (a) valid secret + DOCUMENT_SIGNED writes a documentEvents row; + (b) wrong secret → 401; (c) duplicate body → second call no-op; + (d) two ports holding the same `documenso_id` — assert the wrong + port cannot mutate the right port's interest (locks in the + audit-deferred fix when it lands). + +### `src/app/api/portal/auth/sign-in/route.ts:15` (+4) — Portal & invite auth endpoints have no rate limit + +- **Source:** auditor-E3 (Issue 1), overlaps auditor-A3 (Issue 9) +- **Description:** `/api/portal/auth/sign-in`, `/forgot-password`, + `/reset-password`, `/activate`, and CRM `/api/auth/set-password` + parse credentials/tokens but never call `checkRateLimit`. The + shared `rateLimiters.auth` (5/15min) bucket is defined in + `src/lib/rate-limit.ts:73` but unused on these surfaces. +- **Impact:** Unauthenticated attackers can mount credential-stuffing + against portal sign-in; brute-force activation/reset tokens (32-byte + token is at most ~10⁷ attempts in 24h with parallel clients); + enumerate emails through forgot-password timing differences. +- **Recommendation:** Wrap each route with per-IP rate-limit. Portal + sign-in: 5 attempts per 15min per `(ip, email)`. Forgot-password: + 3/hour/IP. Activate/reset/set-password: 10/hour/IP. Mirror the 429 + - Retry-After response pattern from `public/interests/route.ts`. + +### `src/app/api/v1/berths/[id]/pdf-versions/handlers.ts:39` — Berth-PDF register trusts arbitrary `storageKey` from body + +- **Source:** auditor-E3 (Issue 2) +- **Description:** POST `/api/v1/berths/[id]/pdf-versions` accepts + `body.storageKey` and forwards it to `uploadBerthPdf` + (`berth-pdf.service.ts:213`). Service validates the berth is + in-port but never asserts the supplied key falls within the + expected `berths/${berthId}/...` namespace. Magic-byte check passes + for any genuine PDF; content-type check accepts + `application/octet-stream`. +- **Impact:** A rep with `berths.edit` on berth-X can repoint + berth-X's `currentPdfVersionId` at the storage key of any other + PDF — confidential signed EOI PDFs, brochure version blobs, or + another port's berth PDF if cross-tenant key paths leak via logs / + error inspector. Subsequent + `GET /api/v1/berths/[id]/pdf-download` serves those bytes under + berth-X's tenant-scoped permission gate, bypassing + `documents.view` / `brochures.view` resource checks. +- **Recommendation:** Reject any `storageKey` that doesn't match + `^berths/${berthId}/uploads/[0-9a-f-]{36}_` in the handler. Same + fix should land for brochures' analogous register-presigned + endpoint. + +### Seven schema FK columns lack DB constraints + +- **Source:** auditor-C3 (Issue 1) +- **Description:** Several columns are commented + `// FK wired in relations.ts` / `// references X.id` and exposed + via Drizzle `relations()`, but no Postgres FK constraint exists. + `relations()` only configures relational query JOINs — it does NOT + install constraints. Affected: + `documents.interest_id|yacht_id|company_id|reservation_id`, + `files.yacht_id|company_id`, `interests.yacht_id`, + `reminders.interest_id|berth_id`, + `berth_waiting_list.yacht_id`, `form_submissions.interest_id`. +- **Impact:** Orphan rows on hard-delete; service can write + `interest_id='nonexistent'` without DB rejection. Joins in + `relations.ts` happily resolve null; downstream code that doesn't + null-check silently misbehaves. +- **Recommendation:** Single migration `0041_missing_fk_constraints.sql` + adding all eight constraints with `ON DELETE SET NULL` (orphan + tolerance) for nullable columns, `RESTRICT` for + `formSubmissions.formTemplateId`. Update each schema column to use + `.references(() => …, { onDelete: '…' })` and remove the + misleading "FK wired in relations.ts" comments. + +--- + +## MEDIUM + +### `src/lib/services/*.ts` — 62 bare-Error throws surface as generic 500s with no code + +- **Source:** auditor-G (Issue 1) +- **Description:** Hotspots: `umami.service.ts` (6), `client-merge.service.ts` + (6), `reports.service.ts` (5), `notes.service.ts` (5), + `documenso-client.ts` (5), `brochures.service.ts` (3), + `email-threads.service.ts` (3), `documents.service.ts` (2), + `invoices.ts` (2), `residential.service.ts` (2), + `email-accounts.service.ts` (2), `expense-dedup.service.ts` (2), + `system-monitoring.service.ts` (2), `gdpr-export.service.ts` (2), + `ai-budget.service.ts` (2). Plus 1-shot in 13 other services. +- **Impact:** User has no actionable info; admin must resolve via + request-ID lookup. ~25 of 62 are "post-insert returning row + missing" — should be `CodedError('INTERNAL', { internalMessage })` + so the toast surfaces request-ID copy. +- **Recommendation:** Mass-rename "guard against null insert" sites + to `CodedError('INTERNAL', { internalMessage: '...' })`. Replace + user-impactful ones (client-merge self-merge, expense self-merge, + ai-budget cap validation) with new domain codes. Top 10 + user-facing migration candidates: interests stage transitions, + documents state machine, berth-pdf mooring mismatch, document-sends + email pipeline, expense-dedup, client-merge, companies duplicate + name, roles duplicate/system protected, users duplicate email, + portal-auth lifecycle codes. + +### 34 manual `NextResponse.json({error}, {status: 4xx})` sites bypass `errorResponse()` + +- **Source:** auditor-F (Issue 1), auditor-G (Issue 3) +- **Description:** Five sub-clusters: 13 admin "Insufficient + permissions"/"Super admin only" 403; 5 path-param "Missing id" + guards; 10 inline validation 400 sites; 3 profile/portal-user 404; + 3 AI feature-flag 404. Reps: `src/app/api/v1/admin/queues/route.ts:10`, + `admin/storage/migrate/route.ts:33`, `alerts/[id]/acknowledge/route.ts:8`, + `me/route.ts:46`, `ai/interest-score/route.ts:22`. +- **Impact:** Failed requests don't carry `code`/`requestId`. Toast + cannot show Reference ID. Admin error-inspector skips them + entirely. +- **Recommendation:** Replace each branch with the appropriate + `AppError` subclass throw. For admin gating, factor a + `requireSuperAdmin(ctx)` helper that throws `ForbiddenError`. + +### `src/lib/services/document-reminders.ts:204` and others — High-impact perf medium-tier (5 findings) + +- **Source:** auditor-I (Issues 5–9) +- **Description:** (5) `inquiry-notifications.service.ts:66` + sequential `createNotification` per user — public form does ~80 + round trips before responding. (6) `email-compose.service.ts:25,117` + N+1 attachment-port-check + full-buffer load (15MB resident per 3× + 5MB send). (7) `files.ts:187` + `maintenance.ts:90` direct + `minioClient.removeObject` (CLAUDE.md violation; filesystem mode + orphans bytes). (8) `validators/invoices.ts:33,65` uncapped + `expenseIds` array → Postgres parameter limit DoS vector. (9) + `expense-pdf.service.ts:444` unbounded `inArray` on file IDs (5000+ + bind list). + +### `src/app/api/v1/document-sends/{brochure,berth-pdf,preview}/route.ts` — Send routes lack `withPermission` + +- **Source:** auditor-A3 (Issue 4) +- **Description:** brochure/route.ts:15, berth-pdf/route.ts:17, + preview/route.ts:17, document-sends/route.ts:9 are bare + `withAuth`. Mutation POSTs fire nodemailer at client recipients; + the 50/user/hour rate limit is the only restriction. +- **Impact:** Any authenticated user (lowest-privilege role) at a + port can send brochures or per-berth PDFs to any recipient the + service permits. Bypasses `email.send` convention. +- **Recommendation:** Wrap brochure + berth-pdf in + `withPermission('email','send')`, preview/list in + `withPermission('email','view')`. + +### `src/app/api/v1/admin/users/options/route.ts:9` — User list returned with no admin gate + +- **Source:** auditor-A3 (Issue 6) +- **Description:** Lives under `/admin/...` but wrapped in `withAuth` + only. Returns `{id, displayName}` for every user assigned to the + caller's port. Caller is `reminder-form.tsx:72`. +- **Impact:** Any authenticated user at a port can enumerate every + colleague's user id + display name (PII enumeration; user-id + leakage helps targeted attacks). +- **Recommendation:** Gate on `withPermission('reminders','assign_others')` + (matches actual consumer); consider moving the route out of + `/admin/`. + +### `src/lib/services/client-merge.service.ts:82-360` — `mergeClients` does not verify caller's port + +- **Source:** auditor-B3 (Issue 2) +- **Description:** Takes no `callerPortId`. Lines 91-100 load both + clients by id only; line 104 asserts winner+loser same-port but + never that either equals caller's port. Sole caller pre-validates + by `ctx.portId`, so today the path is safe — but the service is a + footgun for any future caller. +- **Recommendation:** Add `callerPortId: string` to `MergeOptions`; + throw if `winnerRow.portId !== callerPortId`. + +### `src/lib/services/interest-berths.service.ts:273-282` — `removeInterestBerth` deletes junction without port assertion + +- **Source:** auditor-B3 (Issue 3) +- **Description:** `removeInterestBerth(interestId, berthId)` deletes + with no port check. `setPrimaryBerth` and `upsertInterestBerth` do + guard cross-port (lines 207-221), but `removeInterestBerth` is the + gap. +- **Recommendation:** Make `removeInterestBerth(interestId, berthId, portId)` + with the same `interestPortId/berthPortId` cross-check as + `upsertInterestBerthTx`. + +### Six service mutation sites: UPDATE/DELETE WHEREs missing `portId` despite available context + +- **Source:** auditor-B3 (Issue 4) +- **Description:** `form-templates.service.ts:87,110`, + `company-memberships.service.ts:138,176,230`, + `invoices.ts:765` (`detectOverdue` per-row update), + `notifications.service.ts:222` (markRead by userId only), + `clients.service.ts:835` (`deleteRelationship` by relId only). + Each precedes the mutation with a port-scoped read, then writes + keyed only by entity id. +- **Impact:** Today safe under sequential execution. A future + refactor that drops the read, switches to a stale cache, or runs + the write under different transaction isolation will silently + mutate a foreign-tenant row. +- **Recommendation:** Add `eq(.portId, portId)` to each WHERE. + +### `src/lib/services/portal.service.ts:284-304` — `getDocumentDownloadUrl` fetches file without port scope + +- **Source:** auditor-B3 (Issue 5) +- **Description:** Document lookup at 284-289 is correctly scoped; + follow-up file lookup at 298-300 is `where: eq(files.id, fileId)` + only — no `eq(files.portId, portId)`. +- **Impact:** If `documents.signedFileId / fileId` ever points at a + foreign-port file id (Documenso webhook bug — already deferred — + bulk-import drift, malicious admin write), a portal client receives + a presigned URL to that foreign file. +- **Recommendation:** Replace with + `where: and(eq(files.id, fileId), eq(files.portId, portId))`. Same + fix applies to `reports.service.ts:158-160`. + +### `src/lib/services/berth-reservations.service.ts:155-176` — `activate` accepts `contractFileId` without port verification + +- **Source:** auditor-B3 (Issue 7) +- **Description:** Reservation loaded port-scoped but + `data.contractFileId` written without verifying + `files.portId === portId`. +- **Impact:** Port-A user can attach a port-B file id to a port-A + reservation. Downstream code that resolves + `reservation.contractFileId` and presigns it (portal contract + download, audit export) without re-checking file.portId leaks + foreign-port file content. +- **Recommendation:** Add a port-scoped `findFirst` files lookup + before the write; throw `ValidationError` if missing. + +### `src/lib/db/migrations/0036_polymorphic_check_constraints.sql:7-13` — Polymorphic CHECK misses load-bearing discriminators + +- **Source:** auditor-C3 (Issue 2) +- **Description:** 0036 only added CHECK on + `yachts.current_owner_type` and `invoices.billing_entity_type`. + Other code-branching discriminators are still free-text: + `yacht_ownership_history.owner_type` (yachts.ts:67), + `document_sends.document_kind` (brochures.ts:117), and the + audit-style `*.entity_type` columns. +- **Recommendation:** Extend in `0041_polymorphic_check_constraints_round2.sql`: + CHECK on `yacht_ownership_history.owner_type IN ('client','company')` + and `document_sends.document_kind IN ('berth_pdf','brochure')`. + +### `src/lib/db/schema/financial.ts:103` — `invoices.billing_entity_id` default `''` defeats NOT NULL + +- **Source:** auditor-C3 (Issue 3) +- **Description:** `notNull().default('')` — combined with the 0036 + CHECK on `billing_entity_type`, an invoice can be inserted with + `billing_entity_type='client'` and `billing_entity_id=''`. The + polymorphic resolver looks up empty-string and returns null + without a DB-level signal. +- **Recommendation:** Drop default + add CHECK + `billing_entity_id <> ''`. Backfill empty rows from + `clientName`/`billingEmail` heuristic first. + +### Sixteen better-auth `user_id` columns lack FK to `user` table + +- **Source:** auditor-C3 (Issue 4) +- **Description:** `userProfiles.userId`, `userPortRoles.userId`, + `session.userId`, `savedViews.userId`, `notifications.userId`, + `emailAccounts.userId`, `reminders.assignedTo|createdBy`, + `*Notes.authorId`, etc. — 16 columns commented "references Better + Auth user ID" but no FK constraint. +- **Impact:** Hard-deleting a `user` row leaves `userProfiles` + orphaned with broken `unique(userId)` invariant. Lifecycle-critical + rows shouldn't float. +- **Recommendation:** Add FKs with `ON DELETE CASCADE` for + `userProfiles`/`userPortRoles`/`session`; `ON DELETE SET NULL` for + audit-style columns (need to be made nullable first). + +### `src/app/api/v1/expenses/export/parent-company/route.ts:11` — Gates on `isSuperAdmin` instead of `expenses.export` + +- **Source:** auditor-A3 (Issue 5) +- **Description:** Sibling exports use + `withPermission('expenses','export')`. parent-company diverges to + hard `if (!ctx.isSuperAdmin) 403`. Locks out port admins with + `expenses.export = true`. +- **Recommendation:** Use the same `withPermission` for parity. + +### `src/app/api/v1/ai/email-draft/route.ts:12` — POST has no permission gate + +- **Source:** auditor-A3 (Issue 7) +- **Description:** Feature-flag gated only. Spends OpenAI tokens and + renders client/interest-scoped draft content. +- **Recommendation:** Add `withPermission('email','send')`. + +### `src/app/api/v1/documents/[id]/send/route.ts:8` — `documents.send_for_signing` declared but never enforced + +- **Source:** auditor-A3 (Issue 3) +- **Description:** Schema declares + `send_for_signing: boolean` (users.ts:33) but no route uses it. + send/route.ts gates on `documents.create`; user with create-only + rights can still trigger Documenso send. +- **Recommendation:** Switch to + `withPermission('documents','send_for_signing')`. + +### `src/lib/services/files.ts:39` — `/api/v1/files/upload` trusts client-supplied MIME without magic-byte verification + +- **Source:** auditor-E3 (Issue 3) +- **Description:** `uploadFile()` validates browser-supplied + `file.mimeType` (fully attacker-controlled) against + `ALLOWED_MIME_TYPES` and stores it as MinIO `Content-Type`. No + magic-byte gate. Berth-PDF and brochure paths verify `%PDF-`; the + legacy generic uploader does not. +- **Recommendation:** Add a magic-byte switch keyed by claimed MIME + before the `putObject` (jpeg→`FF D8 FF`, png→`89 50 4E 47`, etc.). + Reject mismatches. Set `X-Content-Type-Options: nosniff` on any + download proxy that streams these bytes. + +### `src/app/api/v1/expenses/scan-receipt/route.ts:35` — Uploads to OCR provider without magic-byte verification + +- **Source:** auditor-E3 (Issue 4) +- **Description:** Forwards `buffer` to `runOcr` with + `mimeType = file.type || 'image/jpeg'`. No size cap, no magic-byte. + Authenticated rep can grief their own port's AI budget by burning + OCR tokens. +- **Recommendation:** Validate magic bytes against + JPEG/PNG/WEBP/HEIC; cap `file.size` at 10MB. + +### `src/app/api/v1/me/route.ts:21` — PATCH preferences uses `.passthrough()`, unbounded JSONB write + +- **Source:** auditor-E3 (Issue 5) +- **Description:** `updateProfileSchema.preferences` is + `z.object({ dark_mode, locale, timezone }).passthrough()`. Any + extra key the client supplies survives validation, gets merged into + `userProfiles.preferences` (line 56), and is returned by GET. No + size cap. +- **Recommendation:** Drop `.passthrough()` + explicit allow-list of + preference keys. Cap merged JSONB at 8 KB. + +### `src/lib/services/documenso-client.ts:138-159` — `createDocument` has no idempotency key + +- **Source:** auditor-D (Issue 5) +- **Description:** POST `/api/v1/documents` with no client-generated + idempotency token. Transient timeout + BullMQ retry → two Documenso + envelopes for the same EOI; second attached to no local document + row. +- **Recommendation:** Generate UUID per local `documents.id`, + persist on the row, pass as `Idempotency-Key`. Documenso 2.x + supports it; v1.13 ignores unknown headers. + +### `src/lib/env.ts:27` + webhooks/documenso/route.ts:54 — Documenso webhook secret is single global env var + +- **Source:** auditor-D (Issue 6) +- **Description:** Documenso URL/key are per-port + (`getPortDocumensoConfig`), but the webhook receiver verifies + against single `env.DOCUMENSO_WEBHOOK_SECRET`. Two ports pointed + at different Documenso instances must share the same plaintext + secret. +- **Recommendation:** Add `documenso_webhook_secret_encrypted` to + per-port `system_settings`, fall back to env. Try each enabled + port's secret with `timingSafeEqual` and use the matched portId as + the lookup scope (also addresses the deferred port_id finding + without needing instance/team in the body). + +### `src/lib/services/email-threads.service.ts:259-345` — IMAP `syncInbox` has no socket/idle timeout + +- **Source:** auditor-D (Issue 7) +- **Description:** `new ImapFlow({...})` constructed without + `socketTimeout`, `greetingTimeout`, `connectionTimeout`. Per-message + fetch loop has no per-iteration cap; one slow-streaming UID stalls + the whole sync. Errors propagate as raw `Error`. +- **Recommendation:** Pass `socketTimeout: 60_000` + a wall-clock cap + on `syncInbox` (5-min fuse via `Promise.race`). Wrap upstream + rejects in `CodedError('imap_upstream_error', { accountId })`. + +### `nginx/nginx.conf:77` — CSP allows `'unsafe-inline'` for script-src + +- **Source:** auditor-K (Issue 7) +- **Description:** CSP header is `script-src 'self' 'unsafe-inline'`. + Neuters CSP's primary XSS defense. +- **Recommendation:** Switch to nonce-based CSP via Next 15's + `experimental.cspNonce` (or middleware-generated nonce); drop + `'unsafe-inline'`. + +### Worker container has no healthcheck (compose files) + +- **Source:** auditor-K (Issue 8) +- **Description:** `crm-worker` service definition has no + `healthcheck:` block; the worker process exposes no HTTP port and + no exec probe is configured. +- **Impact:** A worker whose Redis connection has dropped but whose + process is alive (BullMQ silently rejects new jobs) is invisible to + compose/swarm. Jobs queue up; no restart. +- **Recommendation:** Add `healthcheck: test: ["CMD", "node", "-e", +"require('ioredis').Redis.createClient(process.env.REDIS_URL).ping()..."]` + or expose a tiny HTTP `/health` from worker.ts. + +### `.env.example` missing 13+ runtime envs + +- **Source:** auditor-K (Issue 9) +- **Description:** `IMAP_*`, `SMTP_USER/PASS/FROM`, + `EMAIL_REDIRECT_TO`, `MULTI_NODE_DEPLOYMENT`, + `MINIO_AUTO_CREATE_BUCKET`, `STORAGE_FILESYSTEM_ROOT`, + `EOI_TEMPLATE_PDF_PATH`, `DOCUMENSO_API_VERSION`, + `DOCUMENSO_TEMPLATE_ID_EOI`, `DOCUMENSO_*_RECIPIENT_ID`, `PORT`. + Example also ships `EMAIL_CREDENTIAL_KEY=000…000`. +- **Recommendation:** Add all consumed vars; reject + `EMAIL_CREDENTIAL_KEY` matching the example value when + `NODE_ENV==='production'`. + +### `Dockerfile`, `Dockerfile.dev`, `Dockerfile.worker` — pnpm@latest, non-reproducible + +- **Source:** auditor-K (Issue 5) +- **Description:** All three Dockerfiles run + `corepack prepare pnpm@latest --activate`. +- **Recommendation:** Pin to exact version (`pnpm@9.15.0`); add a + `packageManager` field to package.json. + +### `docker-compose.prod.yml:35,54` — Pulls `:latest` images + +- **Source:** auditor-K (Issue 6) +- **Description:** `image: …/crm-app:latest` and + `…/crm-worker:latest`. No digest pin, no version tag. +- **Recommendation:** Tag releases (`:v0.1.0` or `:sha-abc1234`); pin + to digest (`@sha256:…`) in CI. + +### `Dockerfile.worker:21-25` — Installs deps as root after creating worker user + +- **Source:** auditor-K (Issue 10) +- **Description:** Stage-3 runner runs `pnpm install --prod` at line + 22 (UID 0), then creates `worker:nodejs`, copies bundle, then + `USER worker`. `node_modules/` ends up root-owned. +- **Impact:** Tesseract.js, sharp, and other deps that lazily + download/cache binaries to `node_modules/.cache/*` will EACCES + under the `worker` user. OCR/image-processing failures only + manifest at first PDF parse in prod. +- **Recommendation:** Re-order — create user, `chown -R worker:nodejs /app`, + then `USER worker` before `pnpm install`. + +### `src/app/api/v1/website-analytics/route.ts:104` — 200 OK with error body breaks `res.ok` semantics + +- **Source:** auditor-F (Issue 5) +- **Description:** Returns `status: 200` + `{error: 'umami_not_configured', metric, range}`. + Clients branching on `res.ok` think the call succeeded; React-Query + never enters the error path. Same anti-pattern at + `admin/umami/test/route.ts:21`. +- **Recommendation:** 409 + `CodedError('UMAMI_NOT_CONFIGURED')` for + the unconfigured case, OR `{data: null}` 200 (configured-but-empty + contract). For umami/test: 502 with `{ok: false, error}`. + +### `src/app/api/portal/auth/sign-in/route.ts:25` — Conflates "malformed email" 400 and "invalid password" 401 + +- **Source:** auditor-F (Issue 9) +- **Description:** zod safeParse failure returns + `{error: 'Invalid email or password'}` with status 400. Inner + `signIn()` failure (via `errorResponse`) returns 401 with the same + string. Status differs but body is identical, so a probing client + can distinguish "malformed email" from "wrong password" by status + alone — partially defeats the enumeration-safe phrasing. +- **Recommendation:** Change the 400 path to + `'Email format is invalid'`; let `signIn()` own the 401 string. + +### Custom-fields suite never asserts cross-port nor cross-resource isolation + +- **Source:** auditor-J (Issue 3) +- **Description:** Suite seeds one `portId` and exercises CRUD inside + it. No `it(...)` covering "definition created in port A is invisible + from port B" or cross-resource permission gating. Combined with the + audit-deferred custom-fields-hardcoded-clients gap, no test would + catch a regression. +- **Recommendation:** Three negative tests — cross-port `getValues` + empty, `setValues` cross-port rejects, handler-level test that a + viewer with `companies.view` only is `403` on a clients-gated + custom-field route. + +### `tests/integration/documents-expired-webhook.test.ts:90` — Lacks cross-port assertion + +- **Source:** auditor-J (Issue 4) +- **Description:** Three `it()` blocks: happy path, interest cascade, + no-op for unknown documensoId. The "no-op" is enumeration, not + tenant. With two ports holding the same `documenso_id`, current + code mutates whichever document `findFirst` returns. +- **Recommendation:** Add + `it('does not flip a document in port B when port A receives the expired event', ...)`. + Will fail until the audit-deferred port_id fix ships, which is the + point. + +### Fresh `{ ok: true }` envelope drift on POST mutations + +- **Source:** auditor-F (Issue 4) +- **Description:** 6 sites: alerts/[id]/{acknowledge,dismiss}/route.ts:10; + expenses/[id]/{clear-duplicate,merge}/route.ts:13,23; + admin/ocr-settings/route.ts:67 (PUT); + storage/[token]/route.ts:235 (filesystem upload PUT). Deferred doc + flagged `{success: true}` and `{items}` drift but missed the + `{ok: …}` family. +- **Recommendation:** Pick one — 204 No-Content (admin/queues DELETE + already does this) or `{data: null}` 200. Migrate together. + +### `src/lib/services/inquiry-notifications.service.ts:66` — Sequential per-user createNotification + +- **Source:** auditor-I (Issue 5) +- **Description:** Inside a public inquiry POST, loops + `usersWithAccess` and `await`s `createNotification` per user (≥3 DB + round trips + 2 socket emits per call). +- **Impact:** Port with 20 users → ~80 round trips per public inquiry + before response. +- **Recommendation:** `await Promise.all(usersWithAccess.map(...))`, + or bulk insert + emit-once with `userIds[]` payload. + +--- + +## LOW + +### `src/lib/services/documenso-client.ts` — Throws raw Error not CodedError (5 sites) + +- **Source:** auditor-D (Issue 4) +- **Recommendation:** `throw new CodedError('documenso_upstream_error', { status, path })` + - `documenso_timeout` for AbortError. + +### `src/lib/error-classifier.ts:30-180` — Misses common error shapes + +- **Source:** auditor-G (Issue 4) +- **Description:** Uncategorized: `BetterAuthError`/`APIError`, BullMQ + errors, MinIO `S3Error`/`NoSuchBucket`/`AccessDenied`, `ImapFlowError`. +- **Recommendation:** Add to `ERROR_NAME_HINTS`; add a `bullmq` stack + path hint. + +### Public residential-inquiries route does direct service work + bare-throws + +- **Source:** auditor-G (Issue 6) +- **Description:** Two `throw new Error('Failed to create...')` in + `src/app/api/public/residential-inquiries/route.ts:92,105`. Dual + with audit-final-deferred "Public POST routes bypass service + layer". +- **Recommendation:** Move to `publicResidentialService.create(...)`; + throw `CodedError('INTERNAL')`. + +### `src/lib/db/schema/clients.ts:38` — `merged_into_client_id` text without self-FK + +- **Source:** auditor-C3 (Issue 5) +- **Recommendation:** `.references(() => clients.id, { onDelete: 'set null' })`. + +### `src/lib/db/schema/users.ts:217` — `roles.name` lacks uniqueness + +- **Source:** auditor-C3 (Issue 6) +- **Recommendation:** Either unique index on `name`, OR a partial + unique `uniqueIndex(...).on(name).where(sql\`is_global = true\`)` + if globals are meant to be unique-by-name. + +### `src/lib/db/schema/yachts.ts:39` — `current_owner_id` NOT NULL with no FK + no CHECK on emptiness + +- **Source:** auditor-C3 (Issue 7) +- **Recommendation:** Future generated-column CHECK; for now monitor + in services. + +### Reports list silently swallows download errors + +- **Source:** auditor-H (Issue 3) +- **File:** `src/components/reports/reports-list.tsx:63-73` +- **Description:** `handleDownload` catches every error with + `console.error('Download failed', err)` only — no toast, no UI + feedback. +- **Recommendation:** `toastError(err, 'Download failed')`. + +### Residential Clients list lacks PermissionGate / Skeleton / EmptyState / filter bar + +- **Source:** auditor-H (Issue 4) +- **File:** `src/components/residential/residential-clients-list.tsx:46-191` +- **Recommendation:** Wrap New button in PermissionGate; swap loading + for ``; swap empty for ``; add status + + source select filters. + +### Five admin list create buttons not wrapped in PermissionGate + +- **Source:** auditor-H (Issue 5) +- **Files:** roles/role-list.tsx, tags/tag-list.tsx, + ports/port-list.tsx, document-templates/template-list.tsx:194, + forms/form-template-list.tsx:53-62. +- **Recommendation:** Wrap each `New …` button in + ``. + +### Form-template list uses ad-hoc loading + empty states + +- **Source:** auditor-H (Issue 6) +- **File:** `src/components/admin/forms/form-template-list.tsx:65-70` +- **Recommendation:** Replace with `` + ``. + +### Icon-only buttons missing `aria-label` (≥10 sites) + +- **Source:** auditor-H (Issue 7) +- **Recommendation:** Add `aria-label` (preferred) or + `…`. + +### 11 admin endpoints use ad-hoc `if (!ctx.isSuperAdmin) 403` instead of `withPermission` + +- **Source:** auditor-A3 (Issue 8) +- **Description:** `admin/{health,connections,queues,storage,storage/migrate,errors,alerts/run-engine}/route.ts` + - `currency/rates/refresh`. Bypasses the + `permission_denied` audit row. +- **Recommendation:** Standardize on `withPermission('admin', )`. + Add a new `'admin', 'system'` (or reuse `system_backup`) for + queue/storage/health. + +### `src/middleware.ts:65-69` — CSRF middleware allows requests with neither Origin nor Referer + +- **Source:** auditor-A3 (Issue 10) +- **Description:** `originAllowed()` returns `true` when both Origin + and Referer are absent. SameSite=Lax mitigates but defense-in-depth + weakened. +- **Recommendation:** Default deny when both absent; whitelist + specific server-to-server flows. + +### `src/lib/validators/ports.ts:10`, `me/route.ts:14` — `z.string().url()` accepts `javascript:`/`data:` schemes + +- **Source:** auditor-E3 (Issue 6) +- **Description:** URL constructor accepts arbitrary schemes. Today + rendered only inside `` (neutralizes `javascript:`); the + day someone renders as ``, stored XSS / phishing. +- **Recommendation:** Add + `.refine((u) => /^https?:\/\//i.test(u), 'must be http(s)')` — + mirror `validators/webhooks.ts:92`. + +### `src/lib/services/residential.service.ts:220-231` — `getResidentialInterestById` joins client without port scope + +- **Source:** auditor-B3 (Issue 6) +- **Recommendation:** + `where: and(eq(residentialClients.id, ...), eq(residentialClients.portId, portId))`. + +### `src/app/api/public/health/route.ts:15-25` — Discloses NODE_ENV + APP_URL to anonymous internet + +- **Source:** auditor-K (Issue 11) +- **Recommendation:** Require shared secret header (mirror + `WEBSITE_INTAKE_SECRET`); or hash the response. + +### `src/server.ts:20` — PORT env unvalidated + +- **Source:** auditor-K (Issue 12) +- **Description:** `parseInt(process.env.PORT ?? '3000', 10)` — + `PORT=foo` → NaN → silent ephemeral-port listen. +- **Recommendation:** `PORT: z.coerce.number().int().positive().default(3000)` + in env.ts. + +### Webhook receiver returns 200 OK on auth failure + +- **Source:** auditor-F (Issue 8) +- **File:** `src/app/api/webhooks/documenso/route.ts:56` +- **Recommendation:** 401 + `{ok: false}` on auth failure. Coordinate + with auditor-D before flipping; check whether realapi + documenso-replay test asserts 200. + +### Naked-object success responses bypass `{data}` envelope + +- **Source:** auditor-F (Issue 7) +- **Description:** 3 fresh sites: + `ai/email-draft/route.ts:34`, `auth/set-password/route.ts:33`, + `portal/auth/sign-in/route.ts:30`. +- **Recommendation:** `{data: {jobId}}`, `{data: {email}}`, + `{data: null}` respectively. + +### portal-auth manual 4xx + a third envelope shape + +- **Source:** auditor-F (Issue 6) +- **Description:** 5 sites; `set-password` uses `{message}` while + others use `{error}`. +- **Recommendation:** Factor `parsePortalAuthBody(req, schema)` + helper that throws `ValidationError`. + +### `storage/[token]` line 232 disk-failure 500 doesn't persist + +- **Source:** auditor-F (Issue 10) +- **Recommendation:** Convert only line 232 to + `throw err; return errorResponse(err)`. Leave 4xx token-failure + paths opaque. + +### `inArray` chunking, audit-search super-admin path, notification cross-port test + +- **Source:** auditor-I, auditor-J +- **Description:** (perf 9) `expense-pdf.service.ts:444` chunk + `inArray` to 500-batch. (perf 10) `getClientDocuments` no limit. + (perf 11) `getThread` no limit. (perf 12) socket emit fan-out per + client/contact change. (perf 13) alertEngine sequential ports×rules. + (perf 14) AI worker concurrency 2. (testing 6) `audit-search.test.ts:172` + inverts assertion. (testing 7) notification-lifecycle suite no + cross-port read isolation. + +--- + +## Clean domains + +- **auditor-L (migrations + seed integrity).** Domain clean — + migrations 0000–0040 consistent with schema, journal order matches + file numbering, seed produces a bootable port. Spot-checked + hand-written 0034-0040 idempotency, backfill-before-NOT-NULL, + destructive-drop ordering (0028→0029 interest_berths ladder is + safe), schema-vs-SQL drift (only documented deferred items remain), + `seed-data/berths.json` freshness (refreshed 2026-05-02). + +## Confirmed-clean sub-areas (within otherwise non-clean domains) + +- **Redis** (`src/lib/redis.ts`) — `lazyConnect`, retryStrategy, + error/connect/reconnect logging. +- **Rate-limit** (`src/lib/rate-limit.ts`) — sliding-window via Redis + sorted set, per-window pexpire. +- **BullMQ queue config** (`src/lib/queue/index.ts`) — exponential + backoff, per-queue maxAttempts, bounded retention; per-Worker + connections so no shared `maxRetriesPerRequest:null` foot-gun. +- **NocoDB import** (`src/lib/services/berth-import.ts`) — pure + helpers, idempotent via `updated_at` window. +- **Filesystem proxy** — multi-node guard, key validation regex, + op-bound HMAC, realpath symlink check, atomic temp-rename. Open + items already in deferred list. +- **AI worker** (`src/lib/queue/workers/ai.ts`) — 30s + AbortController, 10KB output cap, port-scoped fetches, ai_usage + ledger write. Deferred "no cost-tracking" item is now resolved. +- **Pino logging** — no `console.error+rethrow` anywhere in + `src/lib/services/`, `src/lib/queue/`, `src/lib/storage/`, + `src/lib/email/`, `src/app/api/` (only legitimate boot-time + `console.error` in env.ts and seed.ts). +- **SQL injection** — every backtick-tagged `sql\`…\``template uses +Drizzle`${}`parameterization. Two`sql.raw` callers + (storage/migrate.ts:128, admin/storage/route.ts:32) interpolate + hardcoded constants only. +- **Documenso webhook signature verification** — timing-safe + (`documenso-webhook.ts:9`). +- **Markdown email rendering** (`renderEmailBody`) — escape-first + allow-list; merge values markdown-escaped via `escapeMergeValue`. +- **Brochure download-link filename HTML-escape** — confirmed. +- **Mass-assignment** — service `…data` spreads use Zod-validated + DTOs that omit `portId`/tenant cols. +- **Internal `` usage** — no `` for internal nav + anywhere in dashboard/portal pages. +- **Form submit `disabled-while-pending`** — spot-checked 22 forms; + all wire `disabled={isSubmitting || mutation.isPending}`. + +--- + +## Audit team — operational notes + +- **12 panes spawned.** 5 of the 12 (auditor-A code-reviewer, B + code-explorer, C code-reviewer, E code-reviewer, plus E2/A2/B2/C2 + respawns of those types) failed to bootstrap inside the team + workflow — they responded with `idle_notification` but never + claimed their task or sent a plan. Consistent symptom across 5 + spawns of the `feature-dev:code-reviewer` and + `feature-dev:code-explorer` subagent types. +- **Successful respawns.** A3, B3, C3, E3 spawned as + `general-purpose` and all delivered tight, well-scoped reports. + All 8 `general-purpose` spawns from the original batch (D, F, G, + H, I, J, K, L) bootstrapped correctly the first time. **Pattern: + use `general-purpose` for team-context spawns; the custom + subagent types appear incompatible.** +- **Plan-approval gate** worked as designed for the 8 successful + panes — each sent a plan, received feedback or approval, and + proceeded only after that handshake. +- **Cross-team dedup** done at consolidation time (no cross-pane + messaging needed): A3/B3 overlap on roles/permissions; C3/I no + overlap (C3 deliberately skipped index work since I had it); + D/E3 overlap on storage abstraction (D claimed it as + integrations-domain HIGH); F/G overlap on manual `NextResponse.json` + (F counted/grouped, G classified migration targets). diff --git a/nginx/conf.d/proxy_params.conf b/nginx/conf.d/proxy_params.conf index 2fa4139..d127198 100644 --- a/nginx/conf.d/proxy_params.conf +++ b/nginx/conf.d/proxy_params.conf @@ -4,6 +4,10 @@ proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header Connection ""; +# Defense-in-depth for CVE-2025-29927: strip the header attackers use to +# skip Next.js middleware. Patched in next>=15.2.3, but neutralizing the +# input at the edge means a future regression cannot reopen the bypass. +proxy_set_header X-Middleware-Subrequest ""; proxy_cache_bypass $http_upgrade; proxy_read_timeout 60s; proxy_send_timeout 60s; diff --git a/package.json b/package.json index e116f4e..97f132b 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,7 @@ "lucide-react": "^0.460.0", "mailparser": "^3.9.4", "minio": "^8.0.0", - "next": "15.1.0", + "next": "15.2.9", "next-themes": "^0.4.0", "nodemailer": "^6.9.0", "openai": "^6.27.0", @@ -110,7 +110,7 @@ "drizzle-kit": "^0.30.0", "esbuild": "^0.25.0", "eslint": "^9.0.0", - "eslint-config-next": "15.1.0", + "eslint-config-next": "15.2.9", "eslint-config-prettier": "^9.1.0", "husky": "^9.1.0", "lint-staged": "^15.2.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2366097..93eefdf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -109,7 +109,7 @@ importers: version: 7.0.1 better-auth: specifier: ^1.2.0 - version: 1.5.5(drizzle-kit@0.30.6)(drizzle-orm@0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4))(mongodb@7.1.0(socks@2.8.7))(next@15.1.0(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4))(react-dom@19.2.4(react@19.2.4))(react@19.2.4)(vitest@4.1.0(@types/node@22.19.15)(vite@8.0.0(@types/node@22.19.15)(esbuild@0.25.12)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))) + version: 1.5.5(drizzle-kit@0.30.6)(drizzle-orm@0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4))(mongodb@7.1.0(socks@2.8.7))(next@15.2.9(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4))(react-dom@19.2.4(react@19.2.4))(react@19.2.4)(vitest@4.1.0(@types/node@22.19.15)(vite@8.0.0(@types/node@22.19.15)(esbuild@0.25.12)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))) bullmq: specifier: ^5.25.0 version: 5.71.0 @@ -153,8 +153,8 @@ importers: specifier: ^8.0.0 version: 8.0.7 next: - specifier: 15.1.0 - version: 15.1.0(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) + specifier: 15.2.9 + version: 15.2.9(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) next-themes: specifier: ^0.4.0 version: 0.4.6(react-dom@19.2.4(react@19.2.4))(react@19.2.4) @@ -271,8 +271,8 @@ importers: specifier: ^9.0.0 version: 9.39.4(jiti@1.21.7) eslint-config-next: - specifier: 15.1.0 - version: 15.1.0(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3) + specifier: 15.2.9 + version: 15.2.9(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3) eslint-config-prettier: specifier: ^9.1.0 version: 9.1.2(eslint@9.39.4(jiti@1.21.7)) @@ -1494,60 +1494,60 @@ packages: '@napi-rs/wasm-runtime@1.1.1': resolution: {integrity: sha512-p64ah1M1ld8xjWv3qbvFwHiFVWrq1yFvV4f7w+mzaqiR4IlSgkqhcRdHwsGgomwzBH51sRY4NEowLxnaBjcW/A==} - '@next/env@15.1.0': - resolution: {integrity: sha512-UcCO481cROsqJuszPPXJnb7GGuLq617ve4xuAyyNG4VSSocJNtMU5Fsx+Lp6mlN8c7W58aZLc5y6D/2xNmaK+w==} + '@next/env@15.2.9': + resolution: {integrity: sha512-0JJ6OlIb1kZiAbY/Hi5XHb2ZT7B5/l8CyGX3GxtTY8LNl1Inm9EU8PnCtVzUR8N2Si3a1pX02PbKBlDcsHNvUQ==} - '@next/eslint-plugin-next@15.1.0': - resolution: {integrity: sha512-+jPT0h+nelBT6HC9ZCHGc7DgGVy04cv4shYdAe6tKlEbjQUtwU3LzQhzbDHQyY2m6g39m6B0kOFVuLGBrxxbGg==} + '@next/eslint-plugin-next@15.2.9': + resolution: {integrity: sha512-AgCS3+FYsSU4aHcmL+FutRWIJ52x9v/etDT+1ttWXEJILn3yo9ALp9lGgC6REtsj1/uPAsLFUh1uvs4LxW2KvQ==} - '@next/swc-darwin-arm64@15.1.0': - resolution: {integrity: sha512-ZU8d7xxpX14uIaFC3nsr4L++5ZS/AkWDm1PzPO6gD9xWhFkOj2hzSbSIxoncsnlJXB1CbLOfGVN4Zk9tg83PUw==} + '@next/swc-darwin-arm64@15.2.5': + resolution: {integrity: sha512-4OimvVlFTbgzPdA0kh8A1ih6FN9pQkL4nPXGqemEYgk+e7eQhsst/p35siNNqA49eQA6bvKZ1ASsDtu9gtXuog==} engines: {node: '>= 10'} cpu: [arm64] os: [darwin] - '@next/swc-darwin-x64@15.1.0': - resolution: {integrity: sha512-DQ3RiUoW2XC9FcSM4ffpfndq1EsLV0fj0/UY33i7eklW5akPUCo6OX2qkcLXZ3jyPdo4sf2flwAED3AAq3Om2Q==} + '@next/swc-darwin-x64@15.2.5': + resolution: {integrity: sha512-ohzRaE9YbGt1ctE0um+UGYIDkkOxHV44kEcHzLqQigoRLaiMtZzGrA11AJh2Lu0lv51XeiY1ZkUvkThjkVNBMA==} engines: {node: '>= 10'} cpu: [x64] os: [darwin] - '@next/swc-linux-arm64-gnu@15.1.0': - resolution: {integrity: sha512-M+vhTovRS2F//LMx9KtxbkWk627l5Q7AqXWWWrfIzNIaUFiz2/NkOFkxCFyNyGACi5YbA8aekzCLtbDyfF/v5Q==} + '@next/swc-linux-arm64-gnu@15.2.5': + resolution: {integrity: sha512-FMSdxSUt5bVXqqOoZCc/Seg4LQep9w/fXTazr/EkpXW2Eu4IFI9FD7zBDlID8TJIybmvKk7mhd9s+2XWxz4flA==} engines: {node: '>= 10'} cpu: [arm64] os: [linux] libc: [glibc] - '@next/swc-linux-arm64-musl@15.1.0': - resolution: {integrity: sha512-Qn6vOuwaTCx3pNwygpSGtdIu0TfS1KiaYLYXLH5zq1scoTXdwYfdZtwvJTpB1WrLgiQE2Ne2kt8MZok3HlFqmg==} + '@next/swc-linux-arm64-musl@15.2.5': + resolution: {integrity: sha512-4ZNKmuEiW5hRKkGp2HWwZ+JrvK4DQLgf8YDaqtZyn7NYdl0cHfatvlnLFSWUayx9yFAUagIgRGRk8pFxS8Qniw==} engines: {node: '>= 10'} cpu: [arm64] os: [linux] libc: [musl] - '@next/swc-linux-x64-gnu@15.1.0': - resolution: {integrity: sha512-yeNh9ofMqzOZ5yTOk+2rwncBzucc6a1lyqtg8xZv0rH5znyjxHOWsoUtSq4cUTeeBIiXXX51QOOe+VoCjdXJRw==} + '@next/swc-linux-x64-gnu@15.2.5': + resolution: {integrity: sha512-bE6lHQ9GXIf3gCDE53u2pTl99RPZW5V1GLHSRMJ5l/oB/MT+cohu9uwnCK7QUph2xIOu2a6+27kL0REa/kqwZw==} engines: {node: '>= 10'} cpu: [x64] os: [linux] libc: [glibc] - '@next/swc-linux-x64-musl@15.1.0': - resolution: {integrity: sha512-t9IfNkHQs/uKgPoyEtU912MG6a1j7Had37cSUyLTKx9MnUpjj+ZDKw9OyqTI9OwIIv0wmkr1pkZy+3T5pxhJPg==} + '@next/swc-linux-x64-musl@15.2.5': + resolution: {integrity: sha512-y7EeQuSkQbTAkCEQnJXm1asRUuGSWAchGJ3c+Qtxh8LVjXleZast8Mn/rL7tZOm7o35QeIpIcid6ufG7EVTTcA==} engines: {node: '>= 10'} cpu: [x64] os: [linux] libc: [musl] - '@next/swc-win32-arm64-msvc@15.1.0': - resolution: {integrity: sha512-WEAoHyG14t5sTavZa1c6BnOIEukll9iqFRTavqRVPfYmfegOAd5MaZfXgOGG6kGo1RduyGdTHD4+YZQSdsNZXg==} + '@next/swc-win32-arm64-msvc@15.2.5': + resolution: {integrity: sha512-gQMz0yA8/dskZM2Xyiq2FRShxSrsJNha40Ob/M2n2+JGRrZ0JwTVjLdvtN6vCxuq4ByhOd4a9qEf60hApNR2gQ==} engines: {node: '>= 10'} cpu: [arm64] os: [win32] - '@next/swc-win32-x64-msvc@15.1.0': - resolution: {integrity: sha512-J1YdKuJv9xcixzXR24Dv+4SaDKc2jj31IVUEMdO5xJivMTXuE6MAdIi4qPjSymHuFG8O5wbfWKnhJUcHHpj5CA==} + '@next/swc-win32-x64-msvc@15.2.5': + resolution: {integrity: sha512-tBDNVUcI7U03+3oMvJ11zrtVin5p0NctiuKmTGyaTIEAVj9Q77xukLXGXRnWxKRIIdFG4OTA2rUVGZDYOwgmAA==} engines: {node: '>= 10'} cpu: [x64] os: [win32] @@ -3669,8 +3669,8 @@ packages: resolution: {integrity: sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==} engines: {node: '>=10'} - eslint-config-next@15.1.0: - resolution: {integrity: sha512-gADO+nKVseGso3DtOrYX9H7TxB/MuX7AUYhMlvQMqLYvUWu4HrOQuU7cC1HW74tHIqkAvXdwgAz3TCbczzSEXw==} + eslint-config-next@15.2.9: + resolution: {integrity: sha512-MWpGYzLdkJ38OF1g1R4wQe9GVvoinCyIeYofITHh5D3FmHuIOgeWAK46M+iUYrIG1cJNX0HPh5fHpjmuC3dnrw==} peerDependencies: eslint: ^7.23.0 || ^8.0.0 || ^9.0.0 typescript: '>=3.3.1' @@ -4677,10 +4677,9 @@ packages: react: ^16.8 || ^17 || ^18 || ^19 || ^19.0.0-rc react-dom: ^16.8 || ^17 || ^18 || ^19 || ^19.0.0-rc - next@15.1.0: - resolution: {integrity: sha512-QKhzt6Y8rgLNlj30izdMbxAwjHMFANnLwDwZ+WQh5sMhyt4lEBqDK9QpvWHtIM4rINKPoJ8aiRZKg5ULSybVHw==} + next@15.2.9: + resolution: {integrity: sha512-jXEBIPi+kIkMe5KI4okvGIWvot9hyiDz2fT4OqxxsSeZTA6zhSwrQkJwTE3GmQ1HQlolcQjTNMjHMvc8hhog7g==} engines: {node: ^18.18.0 || ^19.8.0 || >= 20.0.0} - deprecated: This version has a security vulnerability. Please upgrade to a patched version. See https://nextjs.org/blog/CVE-2025-66478 for more details. hasBin: true peerDependencies: '@opentelemetry/api': ^1.1.0 @@ -7078,34 +7077,34 @@ snapshots: '@tybys/wasm-util': 0.10.1 optional: true - '@next/env@15.1.0': {} + '@next/env@15.2.9': {} - '@next/eslint-plugin-next@15.1.0': + '@next/eslint-plugin-next@15.2.9': dependencies: fast-glob: 3.3.1 - '@next/swc-darwin-arm64@15.1.0': + '@next/swc-darwin-arm64@15.2.5': optional: true - '@next/swc-darwin-x64@15.1.0': + '@next/swc-darwin-x64@15.2.5': optional: true - '@next/swc-linux-arm64-gnu@15.1.0': + '@next/swc-linux-arm64-gnu@15.2.5': optional: true - '@next/swc-linux-arm64-musl@15.1.0': + '@next/swc-linux-arm64-musl@15.2.5': optional: true - '@next/swc-linux-x64-gnu@15.1.0': + '@next/swc-linux-x64-gnu@15.2.5': optional: true - '@next/swc-linux-x64-musl@15.1.0': + '@next/swc-linux-x64-musl@15.2.5': optional: true - '@next/swc-win32-arm64-msvc@15.1.0': + '@next/swc-win32-arm64-msvc@15.2.5': optional: true - '@next/swc-win32-x64-msvc@15.1.0': + '@next/swc-win32-x64-msvc@15.2.5': optional: true '@noble/ciphers@1.3.0': {} @@ -8591,7 +8590,7 @@ snapshots: baseline-browser-mapping@2.10.8: {} - better-auth@1.5.5(drizzle-kit@0.30.6)(drizzle-orm@0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4))(mongodb@7.1.0(socks@2.8.7))(next@15.1.0(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4))(react-dom@19.2.4(react@19.2.4))(react@19.2.4)(vitest@4.1.0(@types/node@22.19.15)(vite@8.0.0(@types/node@22.19.15)(esbuild@0.25.12)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))): + better-auth@1.5.5(drizzle-kit@0.30.6)(drizzle-orm@0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4))(mongodb@7.1.0(socks@2.8.7))(next@15.2.9(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4))(react-dom@19.2.4(react@19.2.4))(react@19.2.4)(vitest@4.1.0(@types/node@22.19.15)(vite@8.0.0(@types/node@22.19.15)(esbuild@0.25.12)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))): dependencies: '@better-auth/core': 1.5.5(@better-auth/utils@0.3.1)(@better-fetch/fetch@1.1.21)(better-call@1.3.2(zod@3.25.76))(jose@6.2.1)(kysely@0.28.11)(nanostores@1.1.1) '@better-auth/drizzle-adapter': 1.5.5(@better-auth/core@1.5.5(@better-auth/utils@0.3.1)(@better-fetch/fetch@1.1.21)(better-call@1.3.2(zod@3.25.76))(jose@6.2.1)(kysely@0.28.11)(nanostores@1.1.1))(@better-auth/utils@0.3.1)(drizzle-orm@0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4)) @@ -8614,7 +8613,7 @@ snapshots: drizzle-kit: 0.30.6 drizzle-orm: 0.38.4(@types/react@19.2.14)(kysely@0.28.11)(postgres@3.4.8)(react@19.2.4) mongodb: 7.1.0(socks@2.8.7) - next: 15.1.0(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) + next: 15.2.9(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) react: 19.2.4 react-dom: 19.2.4(react@19.2.4) vitest: 4.1.0(@types/node@22.19.15)(vite@8.0.0(@types/node@22.19.15)(esbuild@0.25.12)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2)) @@ -9328,9 +9327,9 @@ snapshots: escape-string-regexp@4.0.0: {} - eslint-config-next@15.1.0(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3): + eslint-config-next@15.2.9(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3): dependencies: - '@next/eslint-plugin-next': 15.1.0 + '@next/eslint-plugin-next': 15.2.9 '@rushstack/eslint-patch': 1.16.1 '@typescript-eslint/eslint-plugin': 8.57.0(@typescript-eslint/parser@8.57.0(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3))(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3) '@typescript-eslint/parser': 8.57.0(eslint@9.39.4(jiti@1.21.7))(typescript@5.9.3) @@ -10416,9 +10415,9 @@ snapshots: react: 19.2.4 react-dom: 19.2.4(react@19.2.4) - next@15.1.0(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4): + next@15.2.9(@playwright/test@1.58.2)(react-dom@19.2.4(react@19.2.4))(react@19.2.4): dependencies: - '@next/env': 15.1.0 + '@next/env': 15.2.9 '@swc/counter': 0.1.3 '@swc/helpers': 0.5.15 busboy: 1.6.0 @@ -10428,14 +10427,14 @@ snapshots: react-dom: 19.2.4(react@19.2.4) styled-jsx: 5.1.6(react@19.2.4) optionalDependencies: - '@next/swc-darwin-arm64': 15.1.0 - '@next/swc-darwin-x64': 15.1.0 - '@next/swc-linux-arm64-gnu': 15.1.0 - '@next/swc-linux-arm64-musl': 15.1.0 - '@next/swc-linux-x64-gnu': 15.1.0 - '@next/swc-linux-x64-musl': 15.1.0 - '@next/swc-win32-arm64-msvc': 15.1.0 - '@next/swc-win32-x64-msvc': 15.1.0 + '@next/swc-darwin-arm64': 15.2.5 + '@next/swc-darwin-x64': 15.2.5 + '@next/swc-linux-arm64-gnu': 15.2.5 + '@next/swc-linux-arm64-musl': 15.2.5 + '@next/swc-linux-x64-gnu': 15.2.5 + '@next/swc-linux-x64-musl': 15.2.5 + '@next/swc-win32-arm64-msvc': 15.2.5 + '@next/swc-win32-x64-msvc': 15.2.5 '@playwright/test': 1.58.2 sharp: 0.33.5 transitivePeerDependencies: diff --git a/src/app/api/auth/set-password/route.ts b/src/app/api/auth/set-password/route.ts index 09b4141..bc568ba 100644 --- a/src/app/api/auth/set-password/route.ts +++ b/src/app/api/auth/set-password/route.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { errorResponse } from '@/lib/errors'; import { consumeCrmInvite } from '@/lib/services/crm-invite.service'; +import { enforcePublicRateLimit } from '@/lib/api/route-helpers'; const bodySchema = z.object({ token: z.string().min(1), @@ -10,6 +11,10 @@ const bodySchema = z.object({ }); export async function POST(req: NextRequest): Promise { + // 10/hour/IP — bounds brute-force against the CRM invite token. + const limited = await enforcePublicRateLimit(req, 'portalToken'); + if (limited) return limited; + let body: unknown; try { body = await req.json(); diff --git a/src/app/api/portal/auth/activate/route.ts b/src/app/api/portal/auth/activate/route.ts index e3ea429..6fe0eac 100644 --- a/src/app/api/portal/auth/activate/route.ts +++ b/src/app/api/portal/auth/activate/route.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { errorResponse } from '@/lib/errors'; import { activateAccount } from '@/lib/services/portal-auth.service'; +import { enforcePublicRateLimit } from '@/lib/api/route-helpers'; const bodySchema = z.object({ token: z.string().min(1), @@ -10,6 +11,10 @@ const bodySchema = z.object({ }); export async function POST(req: NextRequest): Promise { + // 10/hour/IP — bounds brute-force against the 32-byte activation token. + const limited = await enforcePublicRateLimit(req, 'portalToken'); + if (limited) return limited; + let body: unknown; try { body = await req.json(); diff --git a/src/app/api/portal/auth/forgot-password/route.ts b/src/app/api/portal/auth/forgot-password/route.ts index 983ba24..3180940 100644 --- a/src/app/api/portal/auth/forgot-password/route.ts +++ b/src/app/api/portal/auth/forgot-password/route.ts @@ -3,10 +3,17 @@ import { z } from 'zod'; import { logger } from '@/lib/logger'; import { requestPasswordReset } from '@/lib/services/portal-auth.service'; +import { enforcePublicRateLimit } from '@/lib/api/route-helpers'; const bodySchema = z.object({ email: z.string().email() }); export async function POST(req: NextRequest): Promise { + // 3/hour/IP — tightest of the portal limiters because each successful + // call sends an outbound email and timing differences here are the + // primary email-enumeration vector. + const limited = await enforcePublicRateLimit(req, 'portalForgot'); + if (limited) return limited; + let body: unknown; try { body = await req.json(); diff --git a/src/app/api/portal/auth/reset-password/route.ts b/src/app/api/portal/auth/reset-password/route.ts index 2c5542d..cd1363f 100644 --- a/src/app/api/portal/auth/reset-password/route.ts +++ b/src/app/api/portal/auth/reset-password/route.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { errorResponse } from '@/lib/errors'; import { resetPassword } from '@/lib/services/portal-auth.service'; +import { enforcePublicRateLimit } from '@/lib/api/route-helpers'; const bodySchema = z.object({ token: z.string().min(1), @@ -10,6 +11,10 @@ const bodySchema = z.object({ }); export async function POST(req: NextRequest): Promise { + // 10/hour/IP — bounds brute-force against the 32-byte reset token. + const limited = await enforcePublicRateLimit(req, 'portalToken'); + if (limited) return limited; + let body: unknown; try { body = await req.json(); diff --git a/src/app/api/portal/auth/sign-in/route.ts b/src/app/api/portal/auth/sign-in/route.ts index cee97ae..7d1b4cd 100644 --- a/src/app/api/portal/auth/sign-in/route.ts +++ b/src/app/api/portal/auth/sign-in/route.ts @@ -4,6 +4,7 @@ import { z } from 'zod'; import { errorResponse } from '@/lib/errors'; import { PORTAL_COOKIE } from '@/lib/portal/auth'; import { signIn } from '@/lib/services/portal-auth.service'; +import { enforcePublicRateLimit } from '@/lib/api/route-helpers'; const bodySchema = z.object({ email: z.string().email(), @@ -17,14 +18,24 @@ export async function POST(req: NextRequest): Promise { try { body = await req.json(); } catch { - return NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); + return NextResponse.json({ error: 'Email format is invalid' }, { status: 400 }); } const parsed = bodySchema.safeParse(body); if (!parsed.success) { - return NextResponse.json({ error: 'Invalid email or password' }, { status: 400 }); + return NextResponse.json({ error: 'Email format is invalid' }, { status: 400 }); } + // Per-(ip,email) bucket: 5 attempts / 15min. Keyed on email-lowercase so + // the limiter is per-account-per-IP, not just per-IP — a NATed network + // shouldn't be able to lock a single victim by burning their bucket. + const limited = await enforcePublicRateLimit( + req, + 'portalSignIn', + parsed.data.email.toLowerCase(), + ); + if (limited) return limited; + try { const result = await signIn(parsed.data); const res = NextResponse.json({ success: true }); diff --git a/src/app/api/v1/admin/brochures/[id]/versions/route.ts b/src/app/api/v1/admin/brochures/[id]/versions/route.ts index a58224d..027dbdd 100644 --- a/src/app/api/v1/admin/brochures/[id]/versions/route.ts +++ b/src/app/api/v1/admin/brochures/[id]/versions/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from 'next/server'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ValidationError } from '@/lib/errors'; import { generateBrochureStorageKey, registerBrochureVersion, @@ -46,11 +46,28 @@ export const GET = withAuth( }), ); +// Storage keys generated by `generateBrochureStorageKey` look like +// `/brochures//.pdf`. Reject anything else — +// without this, an admin holding manage_settings on port A could ship a +// foreign port's storage key (signed EOI bytes, another port's brochure) +// and have registerBrochureVersion repoint THIS port's brochure version +// at confidential bytes that subsequently serve under brochures.view. +const BROCHURE_KEY_RE = + /^[a-z0-9-]+\/brochures\/[A-Za-z0-9_-]+\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.pdf$/; + export const POST = withAuth( withPermission('admin', 'manage_settings', async (req, ctx, params) => { try { const id = params.id!; const input = await parseBody(req, registerBrochureVersionSchema); + if (!BROCHURE_KEY_RE.test(input.storageKey)) { + throw new ValidationError('storageKey is not in the expected brochure path'); + } + const segments = input.storageKey.split('/'); + // segments: [portSlug, 'brochures', brochureId, '.pdf'] + if (segments[2] !== id) { + throw new ValidationError('storageKey brochureId does not match route param'); + } const data = await registerBrochureVersion({ portId: ctx.portId, brochureId: id, diff --git a/src/app/api/v1/admin/roles/[id]/route.ts b/src/app/api/v1/admin/roles/[id]/route.ts index f77e55d..38ff9e4 100644 --- a/src/app/api/v1/admin/roles/[id]/route.ts +++ b/src/app/api/v1/admin/roles/[id]/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server'; -import { withAuth, withPermission } from '@/lib/api/helpers'; +import { withAuth, withPermission, requireSuperAdmin } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; import { getRole, updateRole, deleteRole } from '@/lib/services/roles.service'; import { updateRoleSchema } from '@/lib/validators/roles'; @@ -17,35 +17,34 @@ export const GET = withAuth( }), ); -export const PATCH = withAuth( - withPermission('admin', 'manage_users', async (req, ctx, params) => { - try { - const body = await parseBody(req, updateRoleSchema); - const data = await updateRole(params.id!, body, { - userId: ctx.userId, - portId: ctx.portId, - ipAddress: ctx.ipAddress, - userAgent: ctx.userAgent, - }); - return NextResponse.json({ data }); - } catch (error) { - return errorResponse(error); - } - }), -); +// Mutations on global roles are super-admin-only — see route.ts header. +export const PATCH = withAuth(async (req, ctx, params) => { + try { + requireSuperAdmin(ctx, 'roles.update'); + const body = await parseBody(req, updateRoleSchema); + const data = await updateRole(params.id!, body, { + userId: ctx.userId, + portId: ctx.portId, + ipAddress: ctx.ipAddress, + userAgent: ctx.userAgent, + }); + return NextResponse.json({ data }); + } catch (error) { + return errorResponse(error); + } +}); -export const DELETE = withAuth( - withPermission('admin', 'manage_users', async (_req, ctx, params) => { - try { - await deleteRole(params.id!, { - userId: ctx.userId, - portId: ctx.portId, - ipAddress: ctx.ipAddress, - userAgent: ctx.userAgent, - }); - return NextResponse.json({ success: true }); - } catch (error) { - return errorResponse(error); - } - }), -); +export const DELETE = withAuth(async (_req, ctx, params) => { + try { + requireSuperAdmin(ctx, 'roles.delete'); + await deleteRole(params.id!, { + userId: ctx.userId, + portId: ctx.portId, + ipAddress: ctx.ipAddress, + userAgent: ctx.userAgent, + }); + return NextResponse.json({ success: true }); + } catch (error) { + return errorResponse(error); + } +}); diff --git a/src/app/api/v1/admin/roles/route.ts b/src/app/api/v1/admin/roles/route.ts index 52812e7..ef2c4e5 100644 --- a/src/app/api/v1/admin/roles/route.ts +++ b/src/app/api/v1/admin/roles/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server'; -import { withAuth, withPermission } from '@/lib/api/helpers'; +import { withAuth, withPermission, requireSuperAdmin } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; import { listRoles, createRole } from '@/lib/services/roles.service'; import { createRoleSchema } from '@/lib/validators/roles'; @@ -17,19 +17,22 @@ export const GET = withAuth( }), ); -export const POST = withAuth( - withPermission('admin', 'manage_users', async (req, ctx) => { - try { - const body = await parseBody(req, createRoleSchema); - const data = await createRole(body, { - userId: ctx.userId, - portId: ctx.portId, - ipAddress: ctx.ipAddress, - userAgent: ctx.userAgent, - }); - return NextResponse.json({ data }, { status: 201 }); - } catch (error) { - return errorResponse(error); - } - }), -); +// Roles are global (no port_id) and assignments span every port via +// userPortRoles, so creation must be super-admin-only — a per-port admin +// holding admin.manage_users must never be able to mint a role that lives +// in another tenant. +export const POST = withAuth(async (req, ctx) => { + try { + requireSuperAdmin(ctx, 'roles.create'); + const body = await parseBody(req, createRoleSchema); + const data = await createRole(body, { + userId: ctx.userId, + portId: ctx.portId, + ipAddress: ctx.ipAddress, + userAgent: ctx.userAgent, + }); + return NextResponse.json({ data }, { status: 201 }); + } catch (error) { + return errorResponse(error); + } +}); diff --git a/src/app/api/v1/berths/[id]/pdf-versions/handlers.ts b/src/app/api/v1/berths/[id]/pdf-versions/handlers.ts index 53355ff..955adf9 100644 --- a/src/app/api/v1/berths/[id]/pdf-versions/handlers.ts +++ b/src/app/api/v1/berths/[id]/pdf-versions/handlers.ts @@ -34,6 +34,17 @@ export const getHandler: RouteHandler = async (_req, ctx, params) => { } }; +// Defense against the post-upload register endpoint trusting an arbitrary +// storageKey from the body. The companion presign endpoint always issues +// `berths//uploads/_` (see ./pdf-upload-url), +// and pdf-upload-url tenant-scopes the berth lookup. Without this regex, +// a rep with berths.edit could ship the storage key of a foreign-port +// PDF (signed EOI, brochure blob, another port's berth) and have the +// service repoint THIS berth's currentPdfVersionId at it — subsequent +// pdf-download serves those bytes under the rep's own permission gate. +const STORAGE_KEY_RE = + /^berths\/[A-Za-z0-9_-]+\/uploads\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}_/; + export const postHandler: RouteHandler = async (req, ctx, params) => { try { const body = (await req.json()) as Partial; @@ -46,6 +57,12 @@ export const postHandler: RouteHandler = async (req, ctx, params) => { if (!body.sha256 || typeof body.sha256 !== 'string') { throw new ValidationError('sha256 is required'); } + const expectedPrefix = `berths/${params.id!}/uploads/`; + if (!body.storageKey.startsWith(expectedPrefix) || !STORAGE_KEY_RE.test(body.storageKey)) { + throw new ValidationError( + 'storageKey must come from the matching presign endpoint for this berth', + ); + } const result = await uploadBerthPdf({ berthId: params.id!, portId: ctx.portId, diff --git a/src/app/api/v1/files/folders/[...path]/route.ts b/src/app/api/v1/files/folders/[...path]/route.ts index c59a6f5..4fcc63c 100644 --- a/src/app/api/v1/files/folders/[...path]/route.ts +++ b/src/app/api/v1/files/folders/[...path]/route.ts @@ -20,7 +20,7 @@ function sanitizeFolderPath(raw: string): string { } export const PATCH = withAuth( - withPermission('files', 'edit', async (req, ctx, params) => { + withPermission('files', 'manage_folders', async (req, ctx, params) => { try { const pathSegments = params.path; const currentPath = Array.isArray(pathSegments) diff --git a/src/app/api/v1/files/folders/route.ts b/src/app/api/v1/files/folders/route.ts index ffd3e9e..c48c5db 100644 --- a/src/app/api/v1/files/folders/route.ts +++ b/src/app/api/v1/files/folders/route.ts @@ -12,7 +12,7 @@ const createFolderSchema = z.object({ }); export const POST = withAuth( - withPermission('files', 'create', async (req, ctx) => { + withPermission('files', 'manage_folders', async (req, ctx) => { try { const body = await parseBody(req, createFolderSchema); diff --git a/src/app/api/v1/files/upload/route.ts b/src/app/api/v1/files/upload/route.ts index 3c9557e..b71cdb0 100644 --- a/src/app/api/v1/files/upload/route.ts +++ b/src/app/api/v1/files/upload/route.ts @@ -6,7 +6,7 @@ import { uploadFile } from '@/lib/services/files'; import { uploadFileSchema } from '@/lib/validators/files'; export const POST = withAuth( - withPermission('files', 'create', async (req, ctx) => { + withPermission('files', 'upload', async (req, ctx) => { try { const formData = await req.formData(); const file = formData.get('file') as File | null; diff --git a/src/lib/api/helpers.ts b/src/lib/api/helpers.ts index b68537b..871468c 100644 --- a/src/lib/api/helpers.ts +++ b/src/lib/api/helpers.ts @@ -8,7 +8,7 @@ import { db } from '@/lib/db'; import { portRoleOverrides, ports, userPortRoles, userProfiles } from '@/lib/db/schema'; import { type RolePermissions } from '@/lib/db/schema/users'; import { createAuditLog } from '@/lib/audit'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ForbiddenError } from '@/lib/errors'; import { logger } from '@/lib/logger'; import { runWithRequestContext, getRequestContext } from '@/lib/request-context'; import { @@ -250,6 +250,31 @@ export function withAuth( }; } +// ─── requireSuperAdmin ─────────────────────────────────────────────────────── + +/** + * Throws ForbiddenError when the caller is not a super-admin. Use inside + * route handlers (after withAuth) for endpoints that mutate global, cross- + * tenant state — global roles, cross-port migrations, system jobs. + * + * Logs the denied attempt to the audit trail (mirrors withPermission). + */ +export function requireSuperAdmin(ctx: AuthContext, attemptedAction = 'super_admin_only'): void { + if (ctx.isSuperAdmin) return; + logger.warn({ userId: ctx.userId, attemptedAction }, 'Super-admin gate denied'); + void createAuditLog({ + userId: ctx.userId, + portId: ctx.portId, + action: 'permission_denied', + entityType: 'super_admin', + entityId: '', + metadata: { attemptedAction }, + ipAddress: ctx.ipAddress, + userAgent: ctx.userAgent, + }); + throw new ForbiddenError('Super admin access required'); +} + // ─── withPermission ────────────────────────────────────────────────────────── /** diff --git a/src/lib/api/route-helpers.ts b/src/lib/api/route-helpers.ts index e278358..31c34d3 100644 --- a/src/lib/api/route-helpers.ts +++ b/src/lib/api/route-helpers.ts @@ -1,6 +1,13 @@ -import { NextRequest } from 'next/server'; +import { NextRequest, NextResponse } from 'next/server'; import { z, ZodSchema } from 'zod'; +import { + checkRateLimit, + rateLimiters, + rateLimitHeaders, + type RateLimiterName, +} from '@/lib/rate-limit'; + /** * Base list query schema shared by all paginated list endpoints. */ @@ -22,10 +29,7 @@ export type BaseListQuery = z.infer; * Parses URL search params against a Zod schema. * Throws a ZodError on validation failure (caught by `errorResponse`). */ -export function parseQuery( - req: NextRequest, - schema: T, -): z.infer { +export function parseQuery(req: NextRequest, schema: T): z.infer { const params = Object.fromEntries(req.nextUrl.searchParams.entries()); return schema.parse(params); } @@ -41,3 +45,52 @@ export async function parseBody( const body = await req.json(); return schema.parse(body); } + +/** + * Best-effort client IP from forwarded headers. The trusted proxy is + * nginx (which sets `x-forwarded-for` from `$proxy_add_x_forwarded_for`), + * so the leftmost token is the original client. Falls back to a literal + * `unknown` so the per-IP key still exists when running outside the + * proxy (dev, tests). + */ +export function clientIp(req: NextRequest): string { + const xff = req.headers.get('x-forwarded-for'); + if (xff) { + const first = xff.split(',')[0]?.trim(); + if (first) return first; + } + return req.headers.get('x-real-ip') ?? 'unknown'; +} + +/** + * Wraps an unauthenticated route handler with a per-IP (or per-key) rate + * limit. Used for portal/auth endpoints that have no session yet — the + * `withRateLimit` helper in api/helpers.ts is keyed on `ctx.userId` and + * cannot apply here. + * + * If `keySuffix` is provided, it's appended to the IP so a single client + * IP can't exhaust an unrelated user's bucket (e.g. for sign-in we key + * on `${ip}:${email}` so per-account brute force is the bottleneck and + * a noisy NAT IP doesn't deny everyone). + */ +export async function enforcePublicRateLimit( + req: NextRequest, + name: RateLimiterName, + keySuffix?: string, +): Promise { + const config = rateLimiters[name]; + const identifier = keySuffix ? `${clientIp(req)}:${keySuffix}` : clientIp(req); + const result = await checkRateLimit(identifier, config); + if (result.allowed) return null; + const retryAfterSec = Math.max(1, Math.ceil((result.resetAt - Date.now()) / 1000)); + return NextResponse.json( + { error: 'Too many requests. Please try again shortly.', retryAfter: retryAfterSec }, + { + status: 429, + headers: { + ...rateLimitHeaders(result), + 'Retry-After': retryAfterSec.toString(), + }, + }, + ); +} diff --git a/src/lib/db/migrations/0041_role_permissions_edit_keys.sql b/src/lib/db/migrations/0041_role_permissions_edit_keys.sql new file mode 100644 index 0000000..5a57afd --- /dev/null +++ b/src/lib/db/migrations/0041_role_permissions_edit_keys.sql @@ -0,0 +1,58 @@ +-- Backfill the new `documents.edit` and `files.edit` permission keys on +-- every existing row in `roles.permissions`. The schema (RolePermissions +-- in src/lib/db/schema/users.ts) added these keys to close the silent-403 +-- traps on PATCH /api/v1/documents/[id], /cancel, /remind, /watchers, and +-- PATCH /api/v1/files/[id] — each used a permission key that did not exist +-- in the schema, so withPermission()'s `resourcePerms[action]` returned +-- undefined and 403'd every non-superadmin call. +-- +-- Backfill rule: +-- documents.edit ← documents.create (anyone who can create can edit) +-- files.edit ← files.upload (same rationale) +-- +-- jsonb_set with create_missing=true (the default) inserts the key only +-- when it's absent, so re-runs are idempotent and the migration is safe +-- against a partial run. + +UPDATE roles +SET permissions = jsonb_set( + permissions, + '{documents,edit}', + COALESCE(permissions->'documents'->'create', 'false'::jsonb), + true +) +WHERE permissions->'documents' IS NOT NULL + AND NOT (permissions->'documents' ? 'edit'); + +UPDATE roles +SET permissions = jsonb_set( + permissions, + '{files,edit}', + COALESCE(permissions->'files'->'upload', 'false'::jsonb), + true +) +WHERE permissions->'files' IS NOT NULL + AND NOT (permissions->'files' ? 'edit'); + +-- Same backfill on per-port overrides (`port_role_overrides.permissions`) +-- so an override that flipped a sibling permission stays consistent. + +UPDATE port_role_overrides +SET permissions = jsonb_set( + permissions, + '{documents,edit}', + COALESCE(permissions->'documents'->'create', 'false'::jsonb), + true +) +WHERE permissions->'documents' IS NOT NULL + AND NOT (permissions->'documents' ? 'edit'); + +UPDATE port_role_overrides +SET permissions = jsonb_set( + permissions, + '{files,edit}', + COALESCE(permissions->'files'->'upload', 'false'::jsonb), + true +) +WHERE permissions->'files' IS NOT NULL + AND NOT (permissions->'files' ? 'edit'); diff --git a/src/lib/db/migrations/meta/_journal.json b/src/lib/db/migrations/meta/_journal.json index e75345d..5ca0813 100644 --- a/src/lib/db/migrations/meta/_journal.json +++ b/src/lib/db/migrations/meta/_journal.json @@ -288,6 +288,13 @@ "when": 1778300000000, "tag": "0040_error_events", "breakpoints": true + }, + { + "idx": 41, + "version": "7", + "when": 1778400000000, + "tag": "0041_role_permissions_edit_keys", + "breakpoints": true } ] } diff --git a/src/lib/db/schema/users.ts b/src/lib/db/schema/users.ts index f74afd6..580c4f0 100644 --- a/src/lib/db/schema/users.ts +++ b/src/lib/db/schema/users.ts @@ -30,6 +30,7 @@ export type RolePermissions = { documents: { view: boolean; create: boolean; + edit: boolean; send_for_signing: boolean; upload_signed: boolean; delete: boolean; @@ -54,6 +55,7 @@ export type RolePermissions = { files: { view: boolean; upload: boolean; + edit: boolean; delete: boolean; manage_folders: boolean; }; diff --git a/src/lib/db/seed.ts b/src/lib/db/seed.ts index 4567811..59fae86 100644 --- a/src/lib/db/seed.ts +++ b/src/lib/db/seed.ts @@ -42,6 +42,7 @@ const ALL_PERMISSIONS: RolePermissions = { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: true, @@ -63,7 +64,7 @@ const ALL_PERMISSIONS: RolePermissions = { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: true, manage_folders: true }, + files: { view: true, upload: true, edit: true, delete: true, manage_folders: true }, email: { view: true, send: true, configure_account: true }, reminders: { view_own: true, @@ -116,6 +117,7 @@ const DIRECTOR_PERMISSIONS: RolePermissions = { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: true, @@ -137,7 +139,7 @@ const DIRECTOR_PERMISSIONS: RolePermissions = { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: true, manage_folders: true }, + files: { view: true, upload: true, edit: true, delete: true, manage_folders: true }, email: { view: true, send: true, configure_account: true }, reminders: { view_own: true, @@ -190,6 +192,7 @@ const SALES_MANAGER_PERMISSIONS: RolePermissions = { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: false, @@ -211,7 +214,7 @@ const SALES_MANAGER_PERMISSIONS: RolePermissions = { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: false, manage_folders: true }, + files: { view: true, upload: true, edit: true, delete: false, manage_folders: true }, email: { view: true, send: true, configure_account: true }, reminders: { view_own: true, @@ -264,6 +267,7 @@ const SALES_AGENT_PERMISSIONS: RolePermissions = { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: false, @@ -285,7 +289,7 @@ const SALES_AGENT_PERMISSIONS: RolePermissions = { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: false, manage_folders: false }, + files: { view: true, upload: true, edit: false, delete: false, manage_folders: false }, email: { view: true, send: true, configure_account: true }, reminders: { view_own: true, @@ -338,6 +342,7 @@ const VIEWER_PERMISSIONS: RolePermissions = { documents: { view: true, create: false, + edit: false, send_for_signing: false, upload_signed: false, delete: false, @@ -359,7 +364,7 @@ const VIEWER_PERMISSIONS: RolePermissions = { record_payment: false, export: false, }, - files: { view: true, upload: false, delete: false, manage_folders: false }, + files: { view: true, upload: false, edit: false, delete: false, manage_folders: false }, email: { view: true, send: false, configure_account: false }, reminders: { view_own: true, diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index f7ded38..bceb9ab 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -91,6 +91,17 @@ export const rateLimiters = { * without dropping data. The shared-secret header gates abuse; this * limiter is just a defensive backstop in case the secret leaks. */ websiteIntake: { windowMs: 60 * 60 * 1000, max: 500, keyPrefix: 'websiteintake' }, + /** Portal sign-in: 5 attempts per 15min per (ip,email) bucket. Defends + * against credential stuffing on /api/portal/auth/sign-in. */ + portalSignIn: { windowMs: 15 * 60 * 1000, max: 5, keyPrefix: 'portal:signin' }, + /** Portal forgot-password: 3/hour/IP. Tighter than sign-in because it + * triggers an outbound email and is the primary email-enumeration + * vector (timing differences between known/unknown). */ + portalForgot: { windowMs: 60 * 60 * 1000, max: 3, keyPrefix: 'portal:forgot' }, + /** Portal activate / reset / set-password: 10/hour/IP. Bounds brute- + * force against the 32-byte token (random walk math is in our favour + * but a tight ceiling keeps the search space practically infeasible). */ + portalToken: { windowMs: 60 * 60 * 1000, max: 10, keyPrefix: 'portal:token' }, } as const satisfies Record; export type RateLimiterName = keyof typeof rateLimiters; diff --git a/tests/helpers/factories.ts b/tests/helpers/factories.ts index d3366f0..4a97310 100644 --- a/tests/helpers/factories.ts +++ b/tests/helpers/factories.ts @@ -316,6 +316,7 @@ export function makeFullPermissions(): RolePermissions { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: true, @@ -337,7 +338,7 @@ export function makeFullPermissions(): RolePermissions { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: true, manage_folders: true }, + files: { view: true, upload: true, edit: true, delete: true, manage_folders: true }, email: { view: true, send: true, configure_account: true }, reminders: { view_own: true, @@ -393,6 +394,7 @@ export function makeViewerPermissions(): RolePermissions { documents: { view: true, create: false, + edit: false, send_for_signing: false, upload_signed: false, delete: false, @@ -414,7 +416,7 @@ export function makeViewerPermissions(): RolePermissions { record_payment: false, export: false, }, - files: { view: true, upload: false, delete: false, manage_folders: false }, + files: { view: true, upload: false, edit: false, delete: false, manage_folders: false }, email: { view: true, send: false, configure_account: false }, reminders: { view_own: true, @@ -470,6 +472,7 @@ export function makeSalesAgentPermissions(): RolePermissions { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: false, @@ -491,7 +494,7 @@ export function makeSalesAgentPermissions(): RolePermissions { record_payment: false, export: false, }, - files: { view: true, upload: true, delete: false, manage_folders: false }, + files: { view: true, upload: true, edit: false, delete: false, manage_folders: false }, email: { view: true, send: true, configure_account: false }, reminders: { view_own: true, @@ -547,6 +550,7 @@ export function makeSalesManagerPermissions(): RolePermissions { documents: { view: true, create: true, + edit: true, send_for_signing: true, upload_signed: true, delete: true, @@ -568,7 +572,7 @@ export function makeSalesManagerPermissions(): RolePermissions { record_payment: true, export: true, }, - files: { view: true, upload: true, delete: true, manage_folders: true }, + files: { view: true, upload: true, edit: true, delete: true, manage_folders: true }, email: { view: true, send: true, configure_account: false }, reminders: { view_own: true,