diff --git a/docs/admin-ux-backlog.md b/docs/admin-ux-backlog.md new file mode 100644 index 0000000..4acd3df --- /dev/null +++ b/docs/admin-ux-backlog.md @@ -0,0 +1,196 @@ +# Admin / settings UX backlog — STATUS + +Living tracker for the admin/UX backlog. Items are marked DONE or +REMAINING based on what landed in the autonomous-push session. + +--- + +## DONE in the autonomous push + +### Foundations + +- **Currency API verified end-to-end**. `scripts/test-currency-api.ts` + fetches live Frankfurter rates → upserts → reads back → converts. + Inverse-rate drift confirmed at ≤0.001. +- **Storage abstraction audit complete**. Every byte path + (signed EOIs, contracts, brochures, berth PDFs, files, avatars, + branding logos) goes through `getStorageBackend()`. `/api/ready` + and the system-monitoring health probe now check the active + backend (S3 or filesystem) instead of always probing MinIO. + +### User settings + +- Country + Timezone selectors with cross-defaulting + auto-detect + banner ("Looks like you're in Europe/Paris — Update?") +- Email change with verification flow (`user_email_changes` table, + `/api/v1/me/email/confirm/`, `/api/v1/me/email/cancel/`) +- Password reset triggered via better-auth `requestPasswordReset` +- Profile photo upload + crop (square 256×256) via shared + `` + `/api/v1/me/avatar` + +### Branding + +- Logo upload + crop modal in admin/branding (uses the same shared + cropper, persists via `/api/v1/admin/settings/image` → storage backend) +- Email header/footer HTML defaults injectable via "Insert default" button +- Brand colour picker, app-name field, logo URL all in one card + +### Storage admin + +- New layout: S3 config form FIRST, swap action SECOND +- Test connection button before any switch +- Two-button switch: "Switch + migrate" vs "Switch only" with warning modal +- `runMigration()` honours `skipMigration` flag + +### Backup management + +- Real `/admin/backup` page driven by new `backup_jobs` table +- `runBackup()` service spawns `pg_dump --format=custom`, streams to + active storage backend, records size + path +- Download button presigns the .dump for offline restore +- Super-admin gated + +### AI admin panel + +- Dedicated `/admin/ai` page consolidating master switch + + monthly token cap + provider credentials +- Per-feature settings (OCR, berth-PDF parser, recommender) + linked from the same page + +### Onboarding + +- Real `/admin/onboarding` page with auto-checked steps +- Reads each setting key + lists endpoint (roles / users / tags) to + decide completion +- Manual checkboxes for steps without an auto-detect signal +- Progress bar + "Mark done"/"Mark incomplete" buttons +- State persisted in `system_settings.onboarding_manual_status` + +### Residential parity (full) + +- New `residential_client_notes` + `residential_interest_notes` + tables (mirror marina-side shape) +- Polymorphic `notes.service.ts` extended with two new entity types + through verifyParent + listForEntity + create + update + delete +- New `` accepts `residential_clients` / + `residential_interests` entity types +- Activity endpoints: `/api/v1/residential/clients/[id]/activity` + + `/api/v1/residential/interests/[id]/activity` +- Notes endpoints: 4 new routes covering GET/POST/PATCH/DELETE +- `residential-client-tabs.tsx` + `residential-interest-tabs.tsx` + built using the marina-side `DetailLayout` pattern (Overview + + Notes + Activity tabs, Interests tab on the client) +- Detail header components mirror the marina-side strip +- `useBreadcrumbHint` wired into both detail components + +### Residential pipeline stages — configurable + +- New `residential-stages.service.ts` with list/save + orphan-check +- `/api/v1/residential/stages` GET/PUT +- `/admin/residential-stages` admin UI with reassign-on-remove + modal (select new stage per affected interest before save) +- Validators relaxed from `z.enum(...)` to `z.string()` so any + admin-defined stage id round-trips + +### Documenso Phase 1 (EOI generate flow polish) + +- Schema migrations applied: + `document_signers.invited_at / opened_at / last_reminder_sent_at / signing_token`, + `documents.completion_cc_emails / auto_reminder_interval_days` +- `transformSigningUrl()` now maps SignerRole → URL segment correctly + (approver→cc, witness→witness) so emails don't land on `/sign/error` +- New `POST /api/v1/documents/[id]/send-invitation` endpoint with + next-pending-signer auto-pick +- Per-port settings added: `documenso_developer_label`, + `documenso_approver_label`, `documenso_developer_user_id`, + `documenso_approver_user_id` (Phase 7 RBAC binding fields) + +### Misc UI/UX + +- Sidebar collapse removed (always expanded) +- Audit log filter inputs sized + dates widened +- Custom Settings section got a long-form description +- Reminder digest timezone uses `TimezoneCombobox` +- Port form: currency dropdown + timezone combobox + brand color +- Permissions count badge opens a modal with granted/denied +- Role names display-normalized via `prettifyRoleName` +- Sales email config: token list + tooltips on threshold + body fields +- Custom Fields page: amber heads-up about non-integration with + search / recommender / audit / merge tokens +- Tag form: native `` +- FilterBar Select crash fixed (no empty-string item values) + +--- + +## REMAINING — large pieces that didn't fit this push + +### 1. Documenso Phase 2 — Webhook handler enhancement (~3-4 hours) + +Cascading "your turn" emails when each signer completes; on-completion +PDF distribution; token-based recipient matching; idempotency lock. +File to extend: `src/app/api/webhooks/documenso/route.ts`. The +schema columns are already in place (Phase 1). + +### 2. Documenso Phase 3 — Custom doc upload-to-Documenso (~6-8 hours) + +Backend service `custom-document-upload.service.ts` + endpoint +`POST /api/v1/interests/[id]/upload-for-signing`. Accepts a PDF + +recipient list + field-placement JSON, calls `createDocument` → +`placeFields` → `sendDocument` on the per-port Documenso client. +Persists a row in `documents` table. + +### 3. Documenso Phase 4 — Field placement UI (~10-14 hours) + +The biggest piece. Needs: + +- 4a: Recipient configurator dialog (~2-3h) +- 4b: PDF rendering with `react-pdf` (~3-4h) +- 4c: Auto-detect anchor scanner via `pdfjs-dist.getTextContent` (~4-6h) +- 4d: Drag-drop overlay using `dnd-kit` (~3-4h) +- 4e: Send button → calls Phase 3 endpoint (~1h) + +Plan locked in `docs/documenso-build-plan.md` Phase 4 — the +field-detector regexes, the anchor patterns, and the type-to-bbox +sizing table are all spelled out. + +### 4. Documenso Phase 5 — Embedded signing URL emission verification (~1-2 hours) + +Verify the website's `/sign//` page handles every signer +role + every documentType combination. Update website's +`signerMessages` map keyed on `(documentType, role)`. Apply the +nginx CORS block from `docs/documenso-integration-audit.md`. + +### 5. Documenso Phase 6 — Polish items (deferred) + +Auto-send delay, audit-log additions, per-document customisation, +document expiration, reminder rate-limit display, failed-webhook +recovery UI. Each ~2-3 hours; all deferred until Phases 1-4 ship. + +### 6. Project Director — UI binding for the developer-user fields + +Schema + setting keys are now in place +(`documenso_developer_user_id`, `documenso_approver_user_id` + +`documenso_developer_label` / `_approver_label`). The remaining +work is: add a "Linked to CRM user" dropdown in +`/admin/documenso/page.tsx` that lists port users; when bound, +auto-fill name/email from the user profile and mark name/email +fields read-only. Webhook handler can then match against the +linked user's email for in-CRM signing-status updates. + +### 7. Custom-fields hardening (~ongoing) + +Remediation paths for the heads-up banner concerns: + +- **Search index**: extend the GIN tsvector to include + customFieldValues content +- **Audit diff**: extend `diffEntity` to walk the + customFieldValues blob +- **Merge tokens**: add `{{custom.}}` handling at + template-render time, plus surface them in the merge-tokens UI + +### 8. Documenso v2 webhook payload audit (small) + +Risk #4 from `docs/documenso-build-plan.md` — confirm v2 payload +shape (`payload.documentId` vs `payload.id`, recipient.token vs +`recipient.recipientId`) against a live v2 instance before relying +on Phase 2 cascading emails. diff --git a/docs/audit-comprehensive-2026-05-06.md b/docs/audit-comprehensive-2026-05-06.md new file mode 100644 index 0000000..7c1b231 --- /dev/null +++ b/docs/audit-comprehensive-2026-05-06.md @@ -0,0 +1,753 @@ +# Comprehensive Audit — 2026-05-06 + +Conducted directly after the smart-archive / hard-delete / bulk-wizard / +audit-overhaul / synthetic-seed batches landed (commits `d07f1ed` +through `9890d06`). Prior comprehensive audit: +`docs/audit-comprehensive-2026-05-05.md`. + +Findings are sorted by severity. Each has a concrete file:line, a +scenario, and a fix recommendation. + +--- + +## CRITICAL + +### C1. 5 of 10 BullMQ workers are never imported (production + dev) + +**Files:** `src/worker.ts:13-17`, `src/server.ts:72-76` + +`src/worker.ts` (production) and `src/server.ts` (dev fallback) both +import only: + +- `emailWorker` +- `documentsWorker` +- `notificationsWorker` +- `importWorker` +- `exportWorker` + +**Missing:** `aiWorker`, `bulkWorker`, `maintenanceWorker`, `reportsWorker`, `webhooksWorker`. + +Because BullMQ workers are constructed at the top of each worker +module and only "start" when the module is imported, never importing +them means: + +- **Webhooks never deliver.** `webhooksWorker` is what processes the + `webhooks` queue; the admin "Replay" button we just shipped enqueues + jobs that pile up in `pending` forever. +- **All maintenance crons silently no-op.** `maintenanceWorker` handles + `database-backup`, `backup-cleanup`, `session-cleanup`, + `currency-refresh`, `gdpr-export-cleanup`, `ai-usage-retention`, + `error-events-retention`, `website-submissions-retention`, + `alerts-evaluate`, `analytics-refresh`, `calendar-sync`, + `temp-file-cleanup`, `form-expiry-check` — none run. +- **Scheduled reports never generate.** `reportsWorker` handles + `report-scheduler` (every minute). +- **Bulk jobs never process** (the synchronous bulk endpoints work, but + any deferred-bulk path is dead). +- **AI usage features never run.** + +**Impact:** Production CRM has been silently shedding webhook +deliveries, never running retention/cleanup, never sending scheduled +reports. + +**Fix:** + +```ts +// Append to src/worker.ts AND the inline section of src/server.ts: +import { aiWorker } from '@/lib/queue/workers/ai'; +import { bulkWorker } from '@/lib/queue/workers/bulk'; +import { maintenanceWorker } from '@/lib/queue/workers/maintenance'; +import { reportsWorker } from '@/lib/queue/workers/reports'; +import { webhooksWorker } from '@/lib/queue/workers/webhooks'; + +const workers = [ + emailWorker, + documentsWorker, + notificationsWorker, + importWorker, + exportWorker, + aiWorker, + bulkWorker, + maintenanceWorker, + reportsWorker, + webhooksWorker, +]; +``` + +After fix, run `pnpm dev` and watch `/admin/webhooks/{id}` deliveries +go from `pending` → `success` to confirm. + +--- + +## HIGH + +### H1. Hard-delete request endpoints have zero rate limiting + +**Files:** + +- `src/app/api/v1/clients/[id]/hard-delete-request/route.ts:1-37` +- `src/app/api/v1/clients/bulk-hard-delete-request/route.ts:1-32` + +Each call writes a fresh code to Redis and emails it to the operator's +address. No `withRateLimit(...)`. An attacker who has compromised an +admin account (or even just the new `permanently_delete_clients` +permission) can: + +1. Email-bomb the admin's own inbox (every request → email). +2. Probe whether arbitrary client IDs exist (200 + `sentToMaskedEmail` + vs 404 `client not found` is a UID oracle). +3. Burn SMTP quota. + +**Fix:** add `withRateLimit('auth', ...)` or a new dedicated bucket +(e.g. 5 per hour per user). Pattern is already in +`src/app/api/v1/clients/[id]/gdpr-export/route.ts`. + +### H2. Audit-page view fires on every paginated reload (log spam) + +**File:** `src/app/api/v1/admin/audit/route.ts:48-72` + +I added a "watch the watchers" `view` audit row for first-page audit +fetches. That's the right idea, but the page also re-fires the request +on every filter change (severity, source, action, date range, search). +A diligent admin filtering through the inspector for an investigation +will write dozens of `view` audit rows per minute — making it harder to +find the actual events they're looking for. + +**Fix:** dedupe in Redis with a 60-second per-user TTL key, only emit +if the key didn't exist. Or only fire when no filters are active. + +### H3. Hard-delete error messages distinguish "no code" vs "wrong code" + +**File:** `src/lib/services/client-hard-delete.service.ts:166-174` + +```ts +if (!stored) throw new ValidationError('Confirmation code expired or not requested'); +if (!safeEqualStr(stored, args.code.trim())) { + throw new ValidationError('Confirmation code is incorrect'); +} +``` + +The two messages let an attacker distinguish "you've never requested a +code" (so spam the request endpoint to open the window) from "wrong +code" (so brute-force more codes). 4-digit space is only 10,000 — with +distinguishable feedback an attacker can confirm code validity in +≤5,000 attempts on average. + +**Fix:** collapse to a single `'Invalid or expired code'` message; the +operator already has the email open and knows what they typed. + +### H4. Synthetic seed leaves `super_admin` linked-port-roles empty + +**File:** `src/lib/db/seed-bootstrap.ts:147-160` + +The bootstrap creates the `userProfiles` row with +`isSuperAdmin: true` for `super-admin-matt-portnimara`, but doesn't +create `userPortRoles` rows. The actual real `user` rows (admin@, +agent@, viewer@) are only created via the Playwright global-setup. +Anyone running `pnpm db:seed:synthetic` then `pnpm dev` and trying to +log in via the UI hits an unauthenticated state until they also run +playwright setup or sign up via better-auth manually. + +**Fix:** either document this in `CLAUDE.md` Quick Reference, or add a +`pnpm db:seed:dev-users` companion script that signs up the three +test users + links roles. Today's synthetic-seed flow felt clean +because the playwright setup was still applied; in a fresh clone it +will surprise. + +### H5. Documenso bad-secret 200 response is correct, but enables enum oracle + +**File:** `src/app/api/webhooks/documenso/route.ts:67-86` + +The route returns `200 ok=false error=Invalid secret` for a wrong +secret. That's webhook best-practice (don't leak signal to attackers), +but combined with the new audit row that captures +`metadata.providedLen`, an attacker can probe secret-length over time +without being detected (just a "warning" row per attempt). On an admin +inspector with 1000s of rows, a slow-rate probe is invisible. + +**Fix:** add per-IP rate limit (5/min) to `/api/webhooks/documenso/` +when secret check fails. Don't block real Documenso traffic — it +shouldn't fail the secret check. + +### H6. The audit-log inspector page itself isn't backed by a real "view" gate beyond `admin.view_audit_log` + +**File:** `src/app/api/v1/admin/audit/route.ts:31` + +Audit log has the most sensitive cross-cutting data in the system +(every login attempt with attempted email, every secret-regenerate, +every hard-delete). It's gated only by `admin.view_audit_log`. The +seed grants this to `director` AND `super_admin`. Consider: + +- making the page super-admin-only for production, OR +- adding a secondary confirmation when viewing rows that contain + attempted emails / IP ranges (PII). + +**Fix:** change `withPermission('admin', 'view_audit_log', ...)` to +add `if (!ctx.isSuperAdmin) check sensitive_audit_view`. Or accept +the current model but document it in the role docs. + +### H7. Three "coming soon" stubs in production UI + +**Files:** + +- `src/components/clients/client-tabs.tsx:276` — "File attachments coming soon." +- `src/components/clients/client-reservations-tab.tsx:41` — "History is coming soon." +- `src/components/berths/berth-tabs.tsx:327` — "{label} coming soon" + +Visible to every user on every client / berth detail page. Either ship +the feature or hide the tab. + +**Fix:** for `client-tabs.tsx` line 276 (Files), the `files` table +already exists and supports clientId — ship a list view. +For `berth-tabs.tsx` line 327 — find the calling tab labels and +either implement or remove from the tabs array. +For `client-reservations-tab.tsx` line 41 — query past reservations +when the user toggles a "show history" filter. + +--- + +## MEDIUM + +### M1. `attachWorkerAudit` recurring job names list duplicates scheduler.ts (drift risk) + +**File:** `src/lib/queue/audit-helpers.ts:23-46` + +The 20 recurring job names are hardcoded in the audit helper; the +scheduler also has its own list. If someone adds a new cron without +updating both, the cron_run audit row never fires for that job. + +**Fix:** export the list from `scheduler.ts` and import it in +`audit-helpers.ts`. Single source of truth. + +### M2. `client-merge-log.surviving_client_id` deleted by hard-delete (history loss) + +**File:** `src/lib/services/client-hard-delete.service.ts:200-202` + +Hard-delete drops every `client_merge_log` row whose surviving id +matches. Those rows are the audit trail of WHO was merged INTO this +client. Once deleted, you've lost evidence of the prior merge. + +**Fix:** replace `delete` with a column nullification, or move the row +to a `client_merge_log_archive` table. Audit trail per GDPR Article 5 +should outlive the data. + +### M3. Bulk hard-delete loops one-shot codes through Redis (5x writes) + +**File:** `src/lib/services/client-hard-delete.service.ts:382-396` + +For a 100-client bulk delete, the function writes 100 single-client +codes to Redis just to satisfy `hardDeleteClient`'s expectation. Each +write is a round-trip; on a Redis hiccup mid-loop, you can end up +with a half-deleted batch. + +**Fix:** refactor `hardDeleteClient` so the inner deletion can be called +without the per-client code check (extract `_doHardDelete()` private +helper used by both single and bulk paths). Keeps Redis clean. + +### M4. Smart-restore wizard has dead reversal applier for `berth_released` + +**File:** `src/lib/services/client-restore.service.ts:360-372` + +The `applyReversal` switch case for `'berth_released'` does nothing — +it just leaves the berth available. The wizard surfaces this as +"auto-reversible" if the berth is still free, but the actual restore +doesn't re-attach the berth to any interest. Operator clicks Restore +expecting their berth back; nothing changes on the berth. + +**Fix:** either (a) at archive time, persist the original interestId +in the decision metadata so we can re-link, or (b) update the wizard +copy to make clear the berth is "available for re-attach" rather than +"will be re-attached." + +### M5. Several services use `void createAuditLog(...)` without `.catch()` + +**Files:** widespread; e.g. `src/lib/services/client-hard-delete.service.ts:127-136, 230-240`, +`src/lib/services/portal-auth.service.ts:269-276` + +`createAuditLog` is documented as never-throwing (catches internally), +but defense-in-depth: a `void` Promise that throws produces an +unhandled rejection event. Most paths are fine because the helper +catches; if anyone refactors `createAuditLog` and removes the catch, +this becomes a process-killer. + +**Fix:** convention rule: every `void someAsync()` must have a `.catch()`. +Codify with a custom ESLint rule, or wrap at call sites: +`void createAuditLog({...}).catch(() => undefined);` + +### M6. Hard-delete audit metadata leaks client `fullName` + +**File:** `src/lib/services/client-hard-delete.service.ts:241-247` + +After the hard-delete the audit row carries +`metadata: { fullName: client.fullName }`. The client record itself is +gone but their name lives on in the audit log. For a GDPR data subject +who exercised their right-to-erasure, this is technically a retention +of personal data in audit history. Not necessarily wrong (audit logs +have a legitimate-interest basis), but should be conscious. + +**Fix:** decide policy: either (a) keep as-is and document, (b) replace +with a hash of the name, or (c) substitute a tombstone identifier. + +### M7. Webhook delivery DLQ admin-replay can re-trigger downstream side-effects + +**File:** `src/lib/services/webhooks.service.ts:282-326` + +Replaying a successful webhook (operator presses Replay on a delivery +that already had `status: 'success'`) re-fires the same payload to the +recipient. If the recipient's idempotency check is weak, you've just +caused a duplicate. The replay payload includes `retried_from` / +`retried_at` markers, which is good — but most recipients won't honor +them. + +**Fix:** disable the Replay button when `status === 'success'`. The UI +already gates on `'failed' || 'dead_letter'` — verify it stays that +way (`webhook-delivery-log.tsx:118-131` looks correct; double-check +no regressions). + +### M8. `audit_logs` table has no DELETE permission gate + +**Files:** schema and routes + +There's no admin endpoint to delete audit rows (good). But there's no +DB-level guard either. A super_admin who runs `db:reset` wipes audit +history. Audit retention should be enforced at the schema level so +even a misconfigured operator can't blow away the trail. + +**Fix:** create a `audit_logs_no_delete_role` postgres role that lacks +DELETE on the table; document that the app's DB user should not have +DELETE on `audit_logs` in production deployments. + +### M9. Documenso void worker uses dynamic import every time + +**File:** `src/lib/queue/workers/documents.ts:25` + +```ts +const { voidDocument } = await import('@/lib/services/documenso-client'); +``` + +Dynamic import inside a hot per-job path is fine the first time but +slows every subsequent call slightly. Move to top-of-file import +unless there's a deliberate reason (circular dep?). + +**Fix:** test moving to top-level import; if it works (no circular +deps), keep it there. + +### M10. Bulk archive wizard "blocked" reason copy truncates at first line + +**File:** `src/components/clients/bulk-archive-wizard.tsx:153-163` + +The wizard shows `b.blockers[0]` for blocked clients. If the dossier +has multiple blockers, only the first is shown. Operators may fix the +first one, retry, and discover a second. + +**Fix:** show all blockers (joined with `·`) or a "+N more" badge +with click-to-expand. + +--- + +## LOW + +### L1. `next-in-line-notify.service.ts` could double-fire on archive retry + +**File:** `src/app/api/v1/clients/[id]/archive/route.ts:114-135` + +If the smart-archive request succeeds at the DB transaction level but +the response upload-side fails (network blip, browser closes), the +operator may retry. Each retry re-fires the next-in-line notification +to all sales recipients. The `dedupeKey: berth-released:{berthId}` +inside the notification helper deduplicates within a cooldown window — +so this is mitigated, but worth verifying the cooldown is set and +not 0. + +### L2. `interests.berth_id` reference in `seed-data.ts` (legacy seed) + +**File:** `src/lib/db/seed-data.ts:973` + +The realistic seed inserts `berthId: ...` on the interests table. Per +`CLAUDE.md`, that column was dropped in migration 0029 and replaced +with `interest_berths` junction. The synthetic seed uses the junction +correctly. The realistic seed will FAIL at insert time if anyone +tries to run it on a freshly-migrated DB. + +**Fix:** rewrite `seed-data.ts:969-982` to insert into `interests` +without `berthId`, then insert the junction rows separately (mirror +the synthetic seed's pattern). + +### L3. Audit log entry for failed login uses `entityId = attemptedEmail` (unbounded) + +**File:** `src/app/api/auth/[...all]/route.ts:53-68` + +If the entityId is very long (a 500-char "email"), it goes into the +DB column. The column is `text` (unbounded) so no DB error, but FTS +search-text may bloat. + +**Fix:** truncate attempted email to 256 chars before using as +entityId. + +### L4. The "watch the watchers" audit fires for filtered queries too + +**File:** `src/app/api/v1/admin/audit/route.ts:48-72` + +(See H2 above for the page-spam variant.) Even on a single search, +an audit row containing the search term is written. If the search +term itself is sensitive (e.g. an admin searches for a specific +client's name in audit logs), it's now in the audit log of audit-log +viewing. Acceptable but worth documenting. + +### L5. Import worker is a stub + +**File:** `src/lib/queue/workers/import.ts:13` + +`// TODO(L2): implement import job handlers` — the worker is wired +into the queue and registered, but does nothing. If anyone enqueues +an `import:*` job, it returns immediately. Either ship the feature +or remove the queue. + +### L6. `interest-form.tsx` two TODOs about company-yacht filter + add-yacht inline + +**File:** `src/components/interests/interest-form.tsx:332-333` + +Real product gaps. When creating an interest for a client who's a +member of a company, you can't pick a yacht owned by that company. +And there's no inline "Add yacht" shortcut in the form. + +### L7. `berth-spec-template.ts` defaults to `'Price: TBD'` when price is null + +**File:** `src/lib/pdf/templates/berth-spec-template.ts:128` + +Generated berth-spec PDFs say "Price: TBD" for any berth without a +price. Cosmetic — verify whether sales considers this an acceptable +fallback or wants to suppress the line entirely. + +--- + +## Things checked and found OK (so we don't re-audit) + +- Tenant isolation on hard-delete (`portId` filter on every query and + inside the tx). +- `withPermission` gates on every new route (bulk-archive-preflight, + hard-delete-_, bulk-hard-delete-_, redeliver). +- Audit log: no public DELETE endpoint, no PATCH endpoint. +- Sidebar nav properly gates marina sections from `residential_partner` + via `hasMarinaAccess`. +- Auth wrapper rebuilds the request body correctly so the upstream + better-auth handler can re-read it (no body-already-consumed bug). +- Webhook outbound SSRF guard with DNS rebinding protection still + intact. +- 1175/1175 vitest suite passing as of last run. + +--- + +## Recommended fix order (ROUND 1 + 2 combined — see below for Round 2) + +See **"Triage list" at the end** of this document — combined ranking +across both audit rounds. + +--- + +## Round 2 — focused agents (added 2026-05-06 evening) + +After the original synthesis above, four scoped agents (smaller blast +radius, hard finding caps) successfully audited their domains and +produced dedicated docs. Findings are linked here with `R2-`-prefixed +IDs. Detail in: + +- [audit-reliability-2026-05-06.md](audit-reliability-2026-05-06.md) — 11 findings +- [audit-frontend-2026-05-06.md](audit-frontend-2026-05-06.md) — 12 findings +- [audit-permissions-2026-05-06.md](audit-permissions-2026-05-06.md) — 9 findings +- [audit-missing-features-2026-05-06.md](audit-missing-features-2026-05-06.md) — 12 findings + +### Round 2 — CRITICAL + +**R2-C1. Bulk archive discards post-commit side effects** ([reliability C1](audit-reliability-2026-05-06.md)) + +- File: `src/app/api/v1/clients/bulk/route.ts:68-134` +- The bulk wizard's `runBulk` callback discards the return value from + `archiveClientWithDecisions`. **Documenso envelopes marked + `void_documenso` are never queued for void; "next-in-line" sales + notifications never fire**. The CRM ends up showing `documents.status='cancelled'` + while the live envelope is still out for signature — a signer can + legally complete a doc the CRM thinks is voided. +- Same severity tier as the original C1 (worker-imports). + +**R2-C2. Frontend: Restore icon hovers destructive-red on archived clients** ([frontend C1](audit-frontend-2026-05-06.md)) + +- File: `src/components/clients/client-detail-header.tsx:174-186` +- Conditional `hover:text-destructive` is overridden by an unconditional + `hover:text-foreground` earlier in the class string. Result: the + Restore button on archived clients hovers blood-red, signalling + "destructive" on a fully reversible action. Users hesitate to click. + Promoted to "critical UX" because it's directly misleading on every + archived client view. + +### Round 2 — HIGH + +**R2-H1. Smart-restore wizard's `berth_released` reversal is a no-op but the audit log claims success** +([reliability H1](audit-reliability-2026-05-06.md)) + +- File: `src/lib/services/client-restore.service.ts:359-372` +- Already noted as M4 in the original synthesis. Round-2 reliability + agent escalated to HIGH because the wizard counter increments and + the audit log records "1 auto-reversed" — operator believes the berth + was re-attached when nothing happened. Same fix path: persist the + original `interestId` in the decision detail and re-link on restore. + +**R2-H2. Smart-archive berth status update has TOCTOU race** +([reliability H2](audit-reliability-2026-05-06.md)) + +- File: `src/lib/services/client-archive.service.ts:191-207` +- Berth row read outside tx, mutated inside tx without `for update` + lock. Concurrent archive + sale of the same berth can race: the + archive flow flips a freshly-sold berth back to `available`. Add + `select … for update` on `berths` before the status flip. + +**R2-H3. Bulk archive can pick the wrong interest for berth release** +([reliability H3](audit-reliability-2026-05-06.md)) + +- File: `src/app/api/v1/clients/bulk/route.ts:95-103` +- Lookup by `primaryBerthMooring` falls back to `dossier.interests[0]?.interestId ?? ''`. + Empty-string `interestId` reaches the delete and silently matches + zero rows; the link is silently retained while the audit log claims + it was removed. + +**R2-H4. External EOI runs five operations outside a transaction** +([reliability H4](audit-reliability-2026-05-06.md)) + +- File: `src/lib/services/external-eoi.service.ts:67-155` +- Storage upload + 4 DB writes are independent. Mid-flight failure + leaves orphan PDFs in S3/MinIO and partial DB state. + +**R2-H5. Bulk wizard double-submit treats `ConflictError('already archived')` as a per-row error** +([reliability H5](audit-reliability-2026-05-06.md)) + +- File: `src/app/api/v1/clients/bulk/route.ts:68-120` +- No idempotency key on the bulk endpoint. A double-submit (network + retry, double click) makes the second response look like all rows + failed even though the first succeeded. + +**R2-H6. Webhook replay button has no UI permission gate (403 toast spam)** +([permissions H1](audit-permissions-2026-05-06.md)) + +- File: `src/components/admin/webhooks/webhook-delivery-log.tsx:118-131` +- Replay button renders for any user who can load the page. Server gates + on `admin.manage_webhooks`. Non-admins see enabled buttons; clicking + surfaces a generic 403 toast. + +**R2-H7. Bulk Archive bulk action exposed to roles without `clients.delete`** +([permissions H2](audit-permissions-2026-05-06.md)) + +- File: `src/components/clients/client-list.tsx:182-190` +- `sales_agent` and `viewer` see the Archive bulk action; clicking + surfaces a 403 from preflight. Mirror the `canHardDelete` pattern: + `const canBulkArchive = can('clients', 'delete');` + +**R2-H8. Bulk add_tag / remove_tag exposed to viewer** +([permissions H3](audit-permissions-2026-05-06.md)) + +- File: `src/components/clients/client-list.tsx:165-181` +- Same pattern as R2-H7 — no UI gate; server gates on `clients.edit`. + +**R2-H9. Bulk hard-delete silently skips rows that vanish between preflight and execute** +([permissions H4](audit-permissions-2026-05-06.md)) + +- File: `src/lib/services/client-hard-delete.service.ts:377` +- `if (!c) continue;` swallows any client that was archived/restored/ + deleted by another operator between preflight and execute. Operator + sees a `deletedCount` lower than requested and no signal which IDs + were skipped. + +**R2-H10. Frontend: `webhook-delivery-log` and `audit-log-list` swallow fetch errors silently** +([frontend H3, H4](audit-frontend-2026-05-06.md)) + +- Files: `src/components/admin/webhooks/webhook-delivery-log.tsx:61-74`, + `src/components/admin/audit/audit-log-list.tsx:150-175` +- Both wrap fetches in `try/finally` with no `catch`. Failed loads show + spinner forever or stale data; user has no signal that anything + failed. Surface via `toast.error` + inline retry banner. + +**R2-H11. Frontend: `audit-log-card` renders as `` — page-jumps on mobile tap** +([frontend H5](audit-frontend-2026-05-06.md)) + +- File: `src/components/admin/audit/audit-log-card.tsx:96` +- Card view rows on mobile insert `#` in URL on tap (back-button trap). + Render as button or div, or link to a useful destination. + +**R2-H12. Frontend: `smart-archive-dialog` doesn't invalidate the dossier or single-client query** +([frontend H6](audit-frontend-2026-05-06.md)) + +- File: `src/components/clients/smart-archive-dialog.tsx:197-212` +- Detail page header keeps showing client as un-archived after a + successful archive until hard reload. Add + `qc.invalidateQueries({queryKey: ['clients', clientId]})` and + `qc.removeQueries({queryKey: ['client-archive-dossier', clientId]})`. + +**R2-H13. Frontend: bulk tag mutation uses `alert()` and lacks `onError`** +([frontend H2](audit-frontend-2026-05-06.md)) + +- File: `src/components/clients/client-list.tsx:88-106` +- Native `alert()` blocks the page on partial failure; pure network + failure shows nothing. Replace with `toast.warning` / `toast.error`. + +**R2-H14. Email-template subject overrides are no-ops for 6 of 8 templates** +([missing-features V1](audit-missing-features-2026-05-06.md)) + +- Files: `src/components/admin/email-templates-admin.tsx:24-72` (UI), + `src/lib/services/portal-auth.service.ts:120,332` (only consumers) +- Admin sees an "Overridden" badge after saving a custom subject for + CRM invite, inquiry confirmation, residential templates, etc. — but + the senders ship the hardcoded subject regardless. Wire + `loadSubjectOverride(portId, key)` into the 6 missing senders. + +**R2-H15. Branding admin saves 5 settings that nothing reads** +([missing-features V2](audit-missing-features-2026-05-06.md)) + +- Files: `src/app/(dashboard)/[portSlug]/admin/branding/page.tsx`, + `src/lib/services/port-config.ts:240-272` +- Logo URL, app name, primary color, header HTML, footer HTML all + dead-end. `getPortBrandingConfig` has zero callers. **Multi-tenant + promise broken — every port's emails ship Port Nimara's branding.** + +**R2-H16. Reminder admin saves digest defaults that no scheduler applies** +([missing-features V3](audit-missing-features-2026-05-06.md)) + +- Files: `src/app/(dashboard)/[portSlug]/admin/reminders/page.tsx`, + `src/lib/services/port-config.ts:284-306` +- Sales reps think they configured a daily digest at 09:00 in their + TZ; they get fire-as-they-hit notifications instead. The digest + scheduler doesn't exist. + +### Round 2 — MEDIUM (selected highlights) + +**R2-M1. Portal "My Memberships" tile is a dead-end** ([missing-features V4](audit-missing-features-2026-05-06.md)) + +- Tile on `/portal/dashboard` has no `href`; route doesn't exist. Either + ship `/portal/memberships` or remove the tile. + +**R2-M2. Company detail Documents tab is a "Coming soon" stub** ([missing-features V5](audit-missing-features-2026-05-06.md)) + +- `src/components/companies/company-tabs.tsx:230-234`. Same problem + as the three already-noted "coming soon" stubs but on a different + entity. + +**R2-M3. Onboarding page is a static checklist not the wizard it advertises** ([missing-features V6](audit-missing-features-2026-05-06.md)) + +- The page literally says "what this page will become". Either build + the wizard or relabel the landing card. + +**R2-M4. Backup admin page is a docs page despite landing copy promising "on-demand exports"** ([missing-features V7](audit-missing-features-2026-05-06.md)) + +- Once C1 (worker imports) is fixed, the existing `database-backup` + job is reachable; small lift to wire a "Take backup now" button. + +**R2-M5. Inquiry inbox has zero triage actions** ([missing-features V8](audit-missing-features-2026-05-06.md)) + +- No "Convert to client", no "Resolve", no "Assign". `website_submissions` + table is permanent; sales has to copy-paste emails into client forms. + +**R2-M6. external-eoi grants only `documents.upload_signed` but mutates interest state** ([permissions M1](audit-permissions-2026-05-06.md)) + +- A custom role with `documents.upload_signed:true` + `interests.edit:false` + can flip an interest to "signed" via the external-EOI route. + +**R2-M7. `InlineStagePicker` never sends `override:true` — `override_stage` permission unreachable from the most-used UI path** ([permissions M2](audit-permissions-2026-05-06.md)) + +- Users with the perm have to fall back to the modal `InterestStagePicker` + to actually use it. + +**R2-M8. `sales_agent` granted `interests.override_stage:true` — likely copy-paste from sales_manager** ([permissions M3](audit-permissions-2026-05-06.md)) + +- All other trust-elevated flags are stripped from sales_agent. Needs a + product decision; either flip to false or document intent. + +**R2-M9. `bulk-archive-preflight` leaks dossier-loader error text in `blockers`** ([permissions M4](audit-permissions-2026-05-06.md)) + +- An attacker enumerating UUIDs can distinguish "doesn't exist" vs + "exists but you can't see it". Replace with generic "Could not load + dossier". + +**R2-M10. Documenso void worker has no max-retry alert hook** ([reliability M2](audit-reliability-2026-05-06.md)) + +- A persistent 401/403 retries forever. On exhaustion, write back to + `documents` (`cancellation_failed=true`) and notify admin. + +**R2-M11. Mobile More-sheet missing residential, notifications, berth-reservations, website-analytics** ([missing-features V9](audit-missing-features-2026-05-06.md)) + +- Mobile users have zero path to entire feature domains. Add to + `MORE_ITEMS`. + +**R2-M12. Portal has no profile / change-password surface** ([missing-features V10](audit-missing-features-2026-05-06.md)) + +- Forces every portal user to use the forgot-password flow even when + they remember their old password. Ship `/portal/profile`. + +**R2-M13. Portal invoices show amounts but no PDF download** ([missing-features V11](audit-missing-features-2026-05-06.md)) + +- Documents page does have downloads; mirror the pattern. + +(Plus several more medium/low items in the dedicated docs; see those +for the full set.) + +--- + +## TRIAGE LIST (combined Round 1 + Round 2) + +### Ship now — CRITICAL + +1. **C1** — wire the 5 missing BullMQ workers (`worker.ts`, `server.ts`) + — 5-line fix; every webhook + cron flow is currently dead. +2. **R2-C1** — make bulk archive enqueue Documenso voids + next-in-line + notifications (return value plumbing in `bulk/route.ts`). +3. **R2-C2** — fix the destructive-red hover on the Restore button + (`client-detail-header.tsx`). Trivial CSS fix. + +### Ship this week — HIGH (security/UX with concrete user impact) + +4. **H1** — rate-limit the hard-delete-request endpoints. +5. **H3** — collapse "no code" vs "wrong code" into one error message. +6. **H7** — three "coming soon" stubs in client/berth tabs. +7. **R2-H1** — fix smart-restore's silent `berth_released` no-op (or + reclassify as `reversibleWithPrompt`). +8. **R2-H2** — add `for update` lock on the smart-archive berth status + flip (TOCTOU race). +9. **R2-H3** — bulk-archive's wrong-interest fallback — empty-string + interestId silently no-ops. +10. **R2-H6, R2-H7, R2-H8** — three permission UI-gate misses on + bulk actions and the webhook-replay button. ~30 lines total. +11. **R2-H10, R2-H12, R2-H13** — frontend swallowed errors + missing + invalidation + alert() instead of toast. Small fixes, immediate UX + win. +12. **R2-H11** — `audit-log-card` `href="#"` mobile back-button trap. +13. **R2-H14** — wire 6 missing email-subject overrides through their + senders. + +### Next sprint — HIGH/MEDIUM (operational + multi-tenant correctness) + +14. **R2-H4** — wrap external-EOI in a transaction. +15. **R2-H5** — bulk-archive idempotency key + treat already-archived as + success in bulk. +16. **R2-H9** — bulk hard-delete should return `skipped: string[]`. +17. **R2-H15, R2-H16** — branding + reminder admin pages save settings + nothing reads (silently broken multi-tenancy). +18. **H2** — audit-page-view de-dupe (don't spam on every filter change). +19. **H4** — synthetic seed needs documented dev-user setup or its own + bootstrap script. +20. **H5** — Documenso bad-secret rate-limit per IP. +21. **R2-M1 through R2-M5** — portal memberships dead-end, company + Documents stub, onboarding wizard, backup page, inquiry inbox triage. + +### Backlog — MEDIUM/LOW + remaining items + +22. The remaining MEDIUM/LOW from both rounds — see the dedicated docs. + +--- + +## Headline numbers (combined) + +- **3 CRITICAL** (worker imports, bulk-archive side-effects, restore-button hover) +- **22 HIGH** (security + UX with concrete impact) +- **~15 MEDIUM** (operational hygiene, multi-tenancy gaps, unfinished features) +- **~10 LOW** (cleanup, defensive) + +Round 1 was a manual synthesis after agent-pool stalls; Round 2 was +four focused agents with hard finding caps that all completed inside +the watchdog window. Every finding is grounded in code references. diff --git a/docs/audit-frontend-2026-05-06.md b/docs/audit-frontend-2026-05-06.md new file mode 100644 index 0000000..4b81c71 --- /dev/null +++ b/docs/audit-frontend-2026-05-06.md @@ -0,0 +1,223 @@ +# Frontend audit — 2026-05-06 + +Scope: new archive/restore/hard-delete dialogs, bulk archive wizard, client +detail header, audit log inspector, webhook delivery log, client list bulk +section. Companion to `docs/audit-comprehensive-2026-05-06.md` (does NOT +re-flag the Files-tab / reservations / berth-tab "coming soon" stubs already +covered there). + +--- + +## Critical + +### C1 — `client-detail-header` opens restore dialog from the Archive icon for archived clients + +**File:** `src/components/clients/client-detail-header.tsx:174-186` + +**Scenario:** On an archived client the icon button still renders `` +when `isArchived` is true (`isArchived ? : ` is +correct), BUT both states use the same `setArchiveOpen(true)` handler and +the conditional below routes `` vs `` +off of `isArchived`. That part is fine. The real problem: the destructive +hover colour `hover:text-destructive` is applied via +`isArchived ? 'hover:text-foreground' : 'hover:text-destructive'` — but the +preceding class string already sets `hover:text-foreground` unconditionally, +so the conditional is dead and the restore button hovers red the same as +archive. Misleading colour signal on a reversible action; users hesitate to +click it. + +**Fix:** Drop the always-applied `hover:text-foreground` from the base class +list and let the conditional own the hover colour, or just colour the +restore icon emerald to differentiate. + +--- + +## High + +### H1 — `bulk-archive-wizard` lets users skip the reasons step by clicking Continue while preflight is loading then Cancel/reopen + +**File:** `src/components/clients/bulk-archive-wizard.tsx:253-267, 80-107` + +**Scenario:** In the `preflight` stage the Continue button is only disabled +when `archivable.length === 0 || preflight.isLoading`. But `archivable` is +derived from `items = preflight.data ?? []`. While loading, `archivable` is +`[]` so Continue is disabled — good. After load with all-blocked selection, +`archivable.length === 0` so still disabled — good. However, the +`reasonsByClientId: reasons` payload is sent verbatim, so a user who advances +to "reasons", types into one client's box, then uses the carousel back arrow +and edits another, can submit reasons for clients NOT in `archivable` (e.g. +if the preflight is refetched on stale-time). Reasons for blocked or removed +client IDs are forwarded to the API. Minor data-quality issue. + +**Fix:** Filter `reasons` to `archivable` IDs before mutating: +`reasonsByClientId: Object.fromEntries(Object.entries(reasons).filter(([id]) => archivable.some(a => a.clientId === id)))`. + +### H2 — `client-list` bulk tag mutation uses `alert()` for partial failures and has no `onError` + +**File:** `src/components/clients/client-list.tsx:88-106` + +**Scenario:** User bulk-adds a tag to 50 clients; backend returns 200 with +`{succeeded: 30, failed: 20}` → user sees a native browser `alert()` blocking +the page. If the request itself errors (network drop, 500), there is no +`onError` so the dialog closes via `onSettled` and the user sees nothing — +silent failure. Inconsistent UX vs. every other mutation in this audit which +uses `toast`. + +**Fix:** Replace `alert(...)` with `toast.warning(...)`, add an +`onError: (err) => toast.error(...)` branch matching the pattern used in +`bulk-archive-wizard.tsx` and `bulk-hard-delete-dialog.tsx`. + +### H3 — `webhook-delivery-log` swallows fetch errors silently + +**File:** `src/components/admin/webhooks/webhook-delivery-log.tsx:61-74` + +**Scenario:** Admin opens a webhook detail page while the API is down or the +webhook was just deleted. `load()` catches and discards the error +(`} catch { /* ignore */ }`). UI shows "Loading deliveries…" forever on the +first load, or stays on the last successful page on subsequent loads, with +no indication that anything failed. No error state, no toast, no retry. + +**Fix:** Surface errors via `toast.error` and show an inline error state +("Couldn't load deliveries — Retry") instead of swallowing. + +### H4 — `audit-log-list` first-page fetch swallows errors and shows no error state + +**File:** `src/components/admin/audit/audit-log-list.tsx:150-175` + +**Scenario:** Filter form is fully interactive, user changes a date — request +fires, server 500s. The `try/finally` has no `catch`, so the rejected promise +becomes an unhandled rejection. The list shows whatever was previously +loaded (or empty state), and the user has no idea their filter didn't apply. +Same applies to `loadMore`. + +**Fix:** Add `catch` blocks that set an error state and render an inline +error banner above the table, with a Retry button. + +### H5 — `audit-log-card` renders as a link to `href="#"` — clicking jumps the page + +**File:** `src/components/admin/audit/audit-log-card.tsx:96` + +**Scenario:** On mobile / card view the audit log entries become clickable +cards with `href="#"`. Tapping any card scrolls the page to top and inserts +`#` in the URL (back-button trap). There's no detail view to navigate to. + +**Fix:** Either render a non-link wrapper (button or div) when no detail +target exists, or link to a useful destination like +`/{portSlug}/{entityType}/{entityId}` when the entity is resolvable. + +### H6 — `smart-archive-dialog` `archiveMutation` doesn't invalidate the dossier or single-client query + +**File:** `src/components/clients/smart-archive-dialog.tsx:197-212` + +**Scenario:** User archives a client successfully. The dialog invalidates +`['clients']`, `['berths']`, `['interests']` but NOT +`['client-archive-dossier', clientId]` nor `['clients', clientId]`. If the +parent screen (e.g. detail page) keeps the client query mounted, the +detail header continues to show the client as un-archived until a hard +reload. The Restore icon won't appear. + +**Fix:** Add `qc.invalidateQueries({queryKey: ['clients', clientId]})` and +`qc.removeQueries({queryKey: ['client-archive-dossier', clientId]})` so a +re-open re-fetches a fresh dossier (e.g. if user re-archives after restoring +in the same session). + +--- + +## Medium + +### M1 — `smart-archive-dialog` derives `interestId` from a name match against `primaryBerthMooring` — wrong key + +**File:** `src/components/clients/smart-archive-dialog.tsx:158-167` + +**Scenario:** When building per-berth decisions the code does +`dossier.interests.find((i) => i.primaryBerthMooring === b.mooringNumber)?.interestId`. +Multiple interests can share the same primary mooring (rare, but possible +historically), and worse, when no interest has this berth as primary it +falls back to `dossier.interests[0]?.interestId` regardless of which berth +is being decided. The wrong interest gets credited with the release, which +is then audit-logged. + +**Fix:** Have the dossier API return `interestId` per berth row (it already +joins `interest_berths`), or look up by membership not by primary flag. + +### M2 — `hard-delete-dialog` doesn't reset state when switching from intent → confirm if request fails midway + +**File:** `src/components/clients/hard-delete-dialog.tsx:39-46, 64-79` + +**Scenario:** User submits hard delete with wrong code → backend returns 400 +→ toast fires, but the dialog stays on `confirm` stage with the bad code +still in the input and no clear cue. If the user then closes (X) and +reopens, the `useEffect` resets correctly. But if the email code expired +(10 min) and they request a fresh one, there's no "Resend code" button — +they must cancel and start over from intent. Minor. + +**Fix:** Add a "Send a new code" link in the confirm stage that calls +`requestCode.mutate()` again and clears `code`. + +### M3 — `bulk-hard-delete-dialog` doesn't refetch / invalidate after partial failure shows totals + +**File:** `src/components/clients/bulk-hard-delete-dialog.tsx:64-85` + +**Scenario:** Bulk delete returns `{deletedCount: 7}` for 10 selected; toast +warns but `qc.invalidateQueries({queryKey: ['clients']})` is invalidated +unconditionally — fine. However, the dialog closes immediately +(`onOpenChange(false)`), so the user can't see WHICH 3 failed. The toast +just says "see audit log". For a destructive bulk op this is too sparse; +users will repeat the action thinking it didn't work. + +**Fix:** Stay open on partial failure and render a list of failed IDs (the +API likely already returns per-item results — if not, return them). + +### M4 — `audit-log-list` doesn't validate that `dateFrom <= dateTo` + +**File:** `src/components/admin/audit/audit-log-list.tsx:142-146` + +**Scenario:** User picks From=2026-06-01, To=2026-05-01. Query fires with an +empty result range; user sees "No audit log entries found" and assumes +their data isn't there. No client-side validation hint. + +**Fix:** Show an inline warning "From date must be before To date" and skip +the request when invalid. + +### M5 — `bulk-archive-wizard` `Cancel` during `archiveMutation.isPending` discards mutation tracking + +**File:** `src/components/clients/bulk-archive-wizard.tsx:248-251, 293-307` + +**Scenario:** User clicks "Archive 50" → mutation in flight (10s) → user +clicks Cancel. The dialog closes; the mutation continues server-side and +its onSuccess fires later, showing a toast for an action the user thought +they cancelled. Worse, the dialog is gone so they can't tell which clients +got archived. + +**Fix:** Disable Cancel while `archiveMutation.isPending`, or relabel to +"Cancel (won't stop in-progress)" and keep the mutation visible. + +--- + +## Low + +### L1 — `audit-log-list` filter row overflows on narrow viewports + +**File:** `src/components/admin/audit/audit-log-list.tsx:321-467` + +**Scenario:** 8 filter controls (`Search` 288px, `Entity` 144px, `Action` +176px, `Severity` 128px, `Source` 128px, `User id` 176px, `From` 144px, +`To` 144px, total ~1330px) sit in a single `flex-wrap` row. At <1280px +viewports they wrap onto multiple lines pushing the table down 200+px; +at <640px (mobile) each control wraps onto its own line and the "Clear" +button (`ml-auto`) lands on the wrong row. + +**Fix:** Collapse rarely-used filters (User id / Severity / Source) into a +"More filters" Popover for sm: viewports. + +### L2 — `audit-log-card` action map missing entries silently fall back to grey "Activity" icon and grey badge + +**File:** `src/components/admin/audit/audit-log-card.tsx:27-44, 46-52` + +**Scenario:** New webhook/cron/job actions are in `audit-log-list.tsx` +ACTION_COLORS but absent from `audit-log-card.tsx` ACTION_BADGE_COLORS and +ACTION_ACCENT. Card view of these entries looks identical to a generic +"unknown" entry — visual loss vs. table view. + +**Fix:** Sync the two maps; consider extracting to a shared module so they +can't drift. diff --git a/docs/audit-missing-features-2026-05-06.md b/docs/audit-missing-features-2026-05-06.md new file mode 100644 index 0000000..8fcd2f9 --- /dev/null +++ b/docs/audit-missing-features-2026-05-06.md @@ -0,0 +1,405 @@ +# Missing-Features Audit — 2026-05-06 + +Focused pass on **features that look done in the UI but aren't fully +wired through the service layer**, plus **admin settings exposed to +users that no code reads**. Companion to +`docs/audit-comprehensive-2026-05-06.md` — the three "coming soon" stubs +already documented there (client Files tab, client reservations history, +berth tabs), the import-worker stub, the two interest-form TODOs, and +the EOI "Price: TBD" finding are NOT re-flagged here. + +Hard cap: 12 findings. Severity tiers below. + +--- + +## VISIBLE-BROKEN (admin sees a control, click is a no-op or wrong) + +### V1. 6 of 8 admin-editable email subject overrides are silently ignored at send time + +**Files:** + +- `src/components/admin/email-templates-admin.tsx:24-72` (UI) +- `src/lib/email/template-catalog.ts:16-25` (catalog of 8 keys) +- `src/lib/services/portal-auth.service.ts:120-127, 332-339` (the only + consumers of `loadSubjectOverride`) + +The `/admin/email-templates` page lets an admin override the subject +line on **eight** transactional templates: +`portal_activation`, `portal_reset`, `portal_invite_resend`, +`crm_invite`, `inquiry_client_confirmation`, +`inquiry_sales_notification`, `residential_inquiry_client_confirmation`, +`residential_inquiry_sales_alert`. The save endpoint persists each one +to `system_settings` (`email_template__subject`). + +Only **two** of those eight are ever read at send time — +`portal_activation` and `portal_reset` in `portal-auth.service.ts`. +A repo-wide search for `loadSubjectOverride` / `settingKeyForSubject` +returns no other consumers. The other six templates use their hardcoded +subject regardless of the admin override. + +**Impact:** sales/ops teams will customize an inquiry confirmation +subject, hit Save, see the "Overridden" badge, and silently ship the +default subject to every prospect. + +**Fix:** small per template — call `loadSubjectOverride(portId, key)` +in each sender (`crm-invite.service.ts`, the inquiry sender, the +residential inquiry sender, the portal-invite-resend path) and pass the +result through as the email subject. + +**Scope:** small (5 callsites + tests). + +--- + +### V2. Branding admin (logo / app name / primary color / email header & footer HTML) saves to settings but no code reads them + +**Files:** + +- `src/app/(dashboard)/[portSlug]/admin/branding/page.tsx:7-46` — UI + with five fields. +- `src/lib/services/port-config.ts:240-272` — `getPortBrandingConfig()` + resolves the five `branding_*` settings into a typed config. +- Repo-wide: `getPortBrandingConfig` has **zero callers** outside its + declaration. The five `SETTING_KEYS.branding*` constants are only + read inside `getPortBrandingConfig` itself. + +The admin panel is functional end-to-end (write hits the settings API, +"Reset to default" works), and the email-templates module hardcodes +`s3.portnimara.com/...` for the logo URL plus a fixed table layout. +None of the email-rendering helpers (`renderEmail`, the template +modules in `src/lib/email/templates/`) call `getPortBrandingConfig`, +and the `` component sources its logo + colors from +constants too. + +**Impact:** every multi-tenant assumption made about branding is +broken. A second port wired into this CRM will see Port Nimara's logo + +- colors in every transactional email and on the auth pages, even + after their admin "configures branding" successfully. + +**Fix:** plumb `getPortBrandingConfig(portId)` through the email +renderer (header/footer HTML + primary button color), and through +`` via a server-fetched prop. + +**Scope:** medium (touches every transactional email + auth shell). + +--- + +### V3. Reminder admin page configures defaults that no service applies + +**Files:** + +- `src/app/(dashboard)/[portSlug]/admin/reminders/page.tsx:7-50` — UI + for default-enabled, default-days, digest-enabled, digest-time, + digest-timezone. +- `src/lib/services/port-config.ts:284-306` — + `getPortReminderConfig()` defines the schema. +- Repo-wide: the keys (`reminder_default_*`, `reminder_digest_*`) and + `getPortReminderConfig` have **zero callers**. + +Same pattern as V2. The admin sets "enable reminders by default on new +interests" → toggles to true → save succeeds → newly-created interests +still default to `reminderEnabled=false`. The digest-time + +timezone fields go nowhere — there is no scheduler that batches +pending reminders into a daily digest. + +**Impact:** the entire reminder UX is decorative. Sales reps think +they configured a daily digest at 09:00 Europe/Warsaw, get +fire-as-they-hit notifications instead. + +**Fix:** wire `getPortReminderConfig` into (a) the interest-create +service (defaults), (b) the maintenance/notifications worker that +fires reminders (digest batching + delivery window). The `digest` +behavior didn't exist before this audit — needs a new scheduled job. + +**Scope:** medium (defaults are small, digest job is new code). + +--- + +### V4. Portal dashboard "My Memberships" tile has no link, no destination page, and isn't reachable from nav + +**Files:** + +- `src/app/(portal)/portal/dashboard/page.tsx:58-63` — `` — note no `href` + prop. +- `src/components/portal/portal-nav.tsx:8-15` — six nav entries, no + memberships. +- Filesystem: `src/app/(portal)/portal/memberships/` does not exist. + +The dashboard shows a count of "memberships" (companies the portal +user belongs to) but the tile is non-clickable and there is no +`/portal/memberships` route. A user with 3 memberships sees the tile, +clicks → nothing happens. + +**Impact:** dead-end on the portal home for any client tied to a +company (the residential and yacht-ownership use-cases). + +**Fix:** ship `/portal/memberships/page.tsx` listing the companies +returned by the existing `companyMemberships` query (already +aggregated in `getPortalDashboard`), and add it to `PortalNav`. Or +pull the tile if memberships isn't a portal feature. + +**Scope:** small. + +--- + +### V5. Company detail page Documents tab is a "Coming soon" stub + +**File:** `src/components/companies/company-tabs.tsx:230-234` + +```ts +{ + id: 'documents', + label: 'Documents', + content: , +}, +``` + +Visible alongside the working Notes / Activity / Addresses / Members +tabs on every company detail page. NOT covered by the existing audit +doc's H7 (which lists clients, client reservations, and berths). + +**Impact:** the same UX problem H7 calls out for clients. + +**Fix:** mirror what client-Files-tab needs — query `documents` joined +to a polymorphic billing-entity = company link, render a list, ship a +download button. Or hide the tab. + +**Scope:** small to medium. + +--- + +## HALF-WIRED (the page works but the surrounding promise overstates it) + +### V6. "Onboarding" admin page is a static checklist, not the wizard the page itself promises + +**File:** `src/app/(dashboard)/[portSlug]/admin/onboarding/page.tsx` + +The page renders 8 stepwise links and explicitly says (lines 71-72, +98-110): "The future onboarding wizard will track progress per port…", +"What this page will become", "The wizard will record completion per +port in `system_settings`, gate the public marketing-site cutover…". + +The admin landing card describes it as the "Initial-setup wizard for +fresh ports" — admins clicking through expect a wizard, get a static +table of contents. + +**Impact:** the only "fresh port" workflow doesn't exist; cutover +gating logic mentioned in the page body is also unimplemented. + +**Fix:** either (a) build the wizard with progress in `system_settings` + +- banner integration, or (b) re-label both this page and the admin + landing card to "Setup checklist" so expectations match reality. + +**Scope:** large for the wizard; tiny for the relabel. + +--- + +### V7. Backup & Restore admin page is informational only — admin landing card promises actions + +**Files:** + +- `src/app/(dashboard)/[portSlug]/admin/backup/page.tsx` +- `src/app/(dashboard)/[portSlug]/admin/page.tsx:148` — landing card + description: "Database snapshots and on-demand exports." + +The landing card sells "on-demand exports". The actual page renders a +two-card explainer: "Current backup posture" (read-only) and "What +this page will become" (the entire interactive surface — list +snapshots, "Take backup now" button, per-port logical export, restore +preview, GDPR per-client export). None of those exist. + +**Impact:** the "Backup & Restore" tile is functionally a docs page. +Compliance officers / users expecting a self-serve GDPR export +button have to file a support ticket. + +**Fix:** match the language on the landing card to the page reality +("Backup posture" → docs only) until the snapshot/export buttons +ship. The maintenance worker already runs `database-backup` (per +`docs/audit-comprehensive-2026-05-06.md` C1 — though that worker isn't +imported), so wiring "Take backup now" against the existing job is +small once C1 is fixed. + +**Scope:** small (doc tweak) or medium (button + per-port export +endpoint). + +--- + +### V8. Inquiry inbox is read-only — no "Convert to Client" / "Mark resolved" / "Assign" actions + +**File:** `src/components/admin/inquiry-inbox.tsx` (entire file, 207 +lines, ends at the View payload toggle) + +The inbox lists website-form submissions (berth_inquiry, +residence_inquiry, contact_form) with filter chips and a +"View payload" expand. There is no action to: + +- create a client/interest from the submission, +- assign the inquiry to a sales rep, +- mark it resolved / triaged, +- reply directly, +- archive or trash the row, +- export. + +The `website_submissions` table appears to be permanent — every +inquiry ever received remains in the inbox forever, with no triage +state. Sales has to manually copy the email into a new client form +and back-reference the original submission. + +**Impact:** the inquiry-to-pipeline conversion step isn't supported in +the CRM. The marketing-site cutover (per the user's +`project_email_ownership_at_cutover.md` memory) will increase volume +on this surface and make the missing triage UX painful. + +**Fix:** add a per-submission "Convert" action that prefills the +client + interest forms with the payload, plus a `triage_state` +column (open / converted / dismissed) and a default filter that hides +non-open rows. + +**Scope:** medium. + +--- + +## MOBILE PARITY + +### V9. Mobile More-sheet is missing several real top-nav destinations + +**File:** `src/components/layout/mobile/more-sheet.tsx:38-50` + +`MORE_ITEMS` lists 11 entries. The dashboard route directory has at +least these top-level segments not represented anywhere in the mobile +bottom-tabs OR more-sheet: + +- `residential` — exists at `/[portSlug]/residential/...` +- `notifications` — exists at `/[portSlug]/notifications/...` +- `berth-reservations` — exists at `/[portSlug]/berth-reservations/...` +- `documents` — exists as a top-level page (separate from the bottom + tab `documents`, which IS in mobile-bottom-tabs) +- `website-analytics` — exists at `/[portSlug]/website-analytics/...` + +A mobile-only user has no path to any of them. The Documents bottom +tab does cover the doc list, but residential is an entire feature +domain (per the `(dashboard)/.../residential` directory) with no +mobile entry point. + +**Impact:** anyone using the mobile chrome to triage on the go can't +reach residential clients/interests, alerts (`alerts` IS in the +sheet), or notifications. + +**Fix:** add the missing segments to `MORE_ITEMS`. If the grid feels +too dense, reorganize into sections. + +**Scope:** small. + +--- + +### V10. Portal has no "Profile" / "Change password" surface + +**Files:** + +- `src/components/portal/portal-nav.tsx:8-15` — six tabs, no profile. +- Filesystem: no `src/app/(portal)/portal/profile/` directory. + +A portal user who wants to change their email, phone, mailing address, +or password has no UI. The portal sign-in flow goes through the +better-auth session but the app exposes zero account-management +controls. The "Need assistance?" card on the dashboard tells the user +to contact the port team — which is the explicit answer for data +edits, but does not cover password changes (a security expectation, +not a per-port-staff burden). + +**Impact:** every portal user who forgets their password (after +already activating) has to use `/portal/forgot-password` even if they +remember the old one. There's no proactive password rotation. A user +who changes their phone number has to email the port to update it. + +**Fix:** ship `/portal/profile` with at minimum: read-only PII view + +"Change password" form (re-uses the existing reset-password endpoint +or a new `change-password` endpoint that takes the current pw). +Phone/address editing is a longer fix because of the audit-trail +implications. + +**Scope:** small for password; medium with PII edits. + +--- + +### V11. Portal invoices page lists invoices but offers no view/download — even though documents do + +**File:** `src/app/(portal)/portal/invoices/page.tsx:53-99` + +Each invoice row shows number, status, due/paid dates, amount, and a +small payment-status caption. There is no link, no PDF view, no +download. By contrast, the portal Documents page (peer route) ends +each row with a `` that +fetches a signed S3 URL. + +Compare to admin/CRM where invoices have a full PDF render flow +(invoice service generates the PDF + signed URL). + +**Impact:** a portal user can see they owe money and cannot retrieve +the actual invoice document. They have to email the port to ask for a +PDF copy. + +**Fix:** add an invoice-PDF endpoint under `/api/portal/invoices/[id]/ +download` mirroring the documents one, and a download button on each +row. The invoice PDF generator already exists (`src/lib/services/ +invoices.ts`). + +**Scope:** small. + +--- + +## DEV-NOTES (legitimately staged-for-later, calling out so they're not forgotten) + +### V12. Email-templates admin only edits subject lines — body editing is a documented "next iteration" + +**Files:** + +- `src/components/admin/email-templates-admin.tsx:78-79` — + "Customize the subject line of transactional emails per port. Body + editing is the next iteration; for now the layout and HTML stay + locked to the default template." +- `src/lib/email/template-catalog.ts:5-9` — same statement in the + catalog header. + +The page is honest about the limitation, so this isn't a "broken" +finding. But it's a notable shipped-without-the-killer-feature gap: +the multi-tenant promise of per-port email customization can't deliver +the body changes that ports actually want (logo placement, signature, +language). Combined with V2 (branding HTML fragments aren't read at +all), there is currently NO way for a non-super-admin per-port admin +to customize the email body in any way. + +**Impact:** confined to admin expectations — most ports will assume +"Email templates" = "edit the email", click in, see only a subject +field, and request the missing body editor. + +**Fix:** scope a body-editing flow that reuses the +`merge_fields.ts` token catalog (the validator already exists for +document templates) for safety. Until that's built, V2 + this finding +together mean a "rebrand the emails" task is single-tenant only. + +**Scope:** large (HTML editor + token validator + per-port override +storage + render-side composition). + +--- + +## Summary + +12 findings, four severity tiers: + +- **Visible-broken (V1-V5):** five admin/portal controls produce no + effect. V1 (email overrides) and V2 (branding) are the highest + impact — both silently break the multi-tenant promise. +- **Half-wired (V6-V8):** three pages where the surrounding wrapper + oversells what's there. V8 (inquiry inbox) is the largest scope. +- **Mobile parity (V9-V11):** mobile users can't reach several real + features; portal users have no profile/password surface and can't + download invoices. +- **Dev-notes (V12):** documented limitations called out for the + roadmap. + +The two highest-leverage quick wins are **V1** (wire 6 missing +template subject overrides — a few hours) and **V11** (portal invoice +download — small, fixes a real customer pain point). diff --git a/docs/audit-permissions-2026-05-06.md b/docs/audit-permissions-2026-05-06.md new file mode 100644 index 0000000..d2a0514 --- /dev/null +++ b/docs/audit-permissions-2026-05-06.md @@ -0,0 +1,266 @@ +# Per-role permission audit — 2026-05-06 + +Focused review of UI/server permission divergence on the new endpoints +shipped during the smart-archive / hard-delete / bulk-wizard / +external-EOI / webhook-replay work bundle. Skips items already covered +in `docs/audit-comprehensive-2026-05-06.md` (audit-log gating H6, +residential_partner sidebar nav). + +The pattern hunted for: `` (or `usePermissions().can`) +on the UI side hides a control under permission **X**, while the +matching API route gates on permission **Y** (or doesn't gate at all, +or gates strictly — producing 403 toast spam for users who can see the +button but can't use it). + +Scope: 8 routes + 5 components + the seed permission matrix. Hard cap +of 10 findings, ranked by impact. Critical/High/Medium/Low. + +--- + +## CRITICAL + +_None._ The four new hard-delete endpoints all gate on +`admin.permanently_delete_clients` on both layers (UI hides the button +via `` +in `client-detail-header.tsx:162` and via `canHardDelete = can('admin', +'permanently_delete_clients')` in `client-list.tsx:53`; the four routes +all wrap with `withPermission('admin', 'permanently_delete_clients', …)`). +The webhook-replay route gates on `admin.manage_webhooks` — see H1 below +for the matching UI gap. + +--- + +## HIGH + +### H1. Webhook replay button has no UI permission gate (403 toast for non-admins) + +- **UI:** `src/components/admin/webhooks/webhook-delivery-log.tsx:118-131` + — the Replay `