pn-new-crm/docs/audit-permissions-2026-05-06.md

# 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: `<PermissionGate>` (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 `<PermissionGate resource="admin" action="permanently_delete_clients">`
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 `<Button>` renders for any user who can load the page,
  with no `<PermissionGate>` wrapper and no `usePermissions().can('admin',
'manage_webhooks')` check.
- **Server:** `src/app/api/v1/admin/webhooks/[webhookId]/deliveries/[deliveryId]/redeliver/route.ts:15`
  — `withPermission('admin', 'manage_webhooks', …)`.

**Divergence:** A `sales_manager` / `sales_agent` / `viewer` who
somehow lands on `/admin/webhooks/{id}` (e.g. via a deep link from a
shared message) sees enabled Replay buttons. Clicking surfaces a
generic 403 toast — the user has no signal that the action is
restricted, just that "Replay failed".

**Fix:** wrap the Replay `<Button>` in
`<PermissionGate resource="admin" action="manage_webhooks">…</PermissionGate>`,
or skip rendering the entire "Replay" column when
`!can('admin', 'manage_webhooks')`. The page-level guard on
`/admin/webhooks` should prevent non-admins from reaching the route in
the first place, but defense-in-depth is cheap and the toast UX is
poor.

---

### H2. Bulk-archive bulk action exposed to roles without `clients.delete`

- **UI:** `src/components/clients/client-list.tsx:182-190` — the
  "Archive" entry in `bulkActions` is unconditionally rendered (only
  the "Permanently delete" entry checks `canHardDelete`).
- **Server:** `src/app/api/v1/clients/bulk/route.ts:40-57` — gates
  `archive` action on `clients.delete`. Also
  `src/app/api/v1/clients/bulk-archive-preflight/route.ts:30` —
  `withPermission('clients', 'delete', …)`.

**Divergence:** `sales_agent` (`clients.delete:false`,
seed-permissions.ts:246) and `viewer` (`clients.delete:false`,
seed-permissions.ts:323) both see the Archive bulk action. Selecting
clients and pressing it fires the `BulkArchiveWizard`, which calls
`bulk-archive-preflight` (returns 403) followed by `bulk` archive
(also 403). The wizard surfaces this as an opaque error.

**Fix:** mirror the `canHardDelete` pattern — compute
`const canBulkArchive = can('clients', 'delete');` near
`client-list.tsx:53` and conditionally include the Archive entry.

---

### H3. Bulk add_tag / remove_tag exposed to viewer (clients.edit:false)

- **UI:** `src/components/clients/client-list.tsx:165-181` — the "Add
  tag" / "Remove tag" bulk actions render with no permission check.
- **Server:** `src/app/api/v1/clients/bulk/route.ts:40-57` — both gate
  on `clients.edit`.

**Divergence:** A `viewer` can multi-select rows, click "Add tag" or
"Remove tag", pick a tag in the dialog, hit "Apply", and receive a 403. The standalone bulk tag dialog has no inline gating to prevent
this.

**Fix:** the bulk action menu entries should gate on
`can('clients', 'edit')`. (Sales agent and above pass; only `viewer`
and `residential_partner` see the bug.)

---

### H4. `client-merge-log.surviving_client_id` enforcement absent from per-row port check on bulk hard-delete

- **Server:** `src/lib/services/client-hard-delete.service.ts:269-272`

The bulk preflight loads **every** row in the port
(`db.select(...).from(clients).where(eq(clients.portId, args.portId))`)
into memory, then validates the requested `clientIds` against that map.
That's correct for tenant isolation — a foreign-port id can't appear in
the map — but the inner loop at lines 364-389 then re-fetches each
client by `(id, portId)` and **silently skips** rows where the second
fetch returns nothing (line 377: `if (!c) continue;`). If a client is
archived between preflight and execute by another operator, the bulk
delete reports `deletedCount` lower than the requested set with no
error — the operator has no way to tell which ids were skipped.

**Divergence (perm-adjacent):** the per-row gate is enforced for
tenancy but the failure mode masquerades as success. Combined with
the route's all-or-nothing `withPermission` at the top, a
`permanently_delete_clients`-bearing operator can quietly under-delete.

**Fix:** when `c` is null, push the id into a `skipped: string[]`
array and return it in the response so the UI can surface "3
deleted, 1 skipped (not archived / removed by another user)".

---

## MEDIUM

### M1. `external-eoi` upload allows any role with `documents.upload_signed` regardless of `interests.edit`

- **UI:** `src/components/interests/interest-detail-header.tsx:382-395`
  — `<PermissionGate resource="documents" action="upload_signed">`.
- **Server:** `src/app/api/v1/interests/[id]/external-eoi/route.ts:8`
  — `withPermission('documents', 'upload_signed', …)`.

**Divergence:** UI and server agree on the permission, but the seed
matrix has `documents.upload_signed:true` for `sales_agent` (line 264) AND any custom role with that flag — uploading an externally
signed EOI mutates the **interest** (it's the operative `signedDocument`
that flips the interest into a "signed" state inside
`uploadExternallySignedEoi`). The user only needs `documents.upload_signed`,
not `interests.edit`. A custom role with `documents.upload_signed:true`

- `interests.edit:false` can mutate the interest's effective state.

**Fix:** add a second gate inside the route handler:
`if (!ctx.isSuperAdmin && !ctx.permissions?.interests?.edit) throw new ForbiddenError(...)`.
Rationale: signing a doc against an interest is an interest-state
change, not just a document upload. Mirror the same check in
`<PermissionGate>` (use `<PermissionGate resource="interests" action="edit">`
nested inside the `documents.upload_signed` gate).

---

### M2. `change_stage` UI doesn't expose override checkbox in `InlineStagePicker` — server still accepts override

- **UI:** `src/components/interests/inline-stage-picker.tsx:52-58` —
  the inline picker (used in the detail header at
  `interest-detail-header.tsx:221`) sends only
  `{ pipelineStage, reason }` and never sets `override:true`. Users
  with `override_stage` get no UI affordance to actually use the
  permission from the inline picker; they have to open the modal
  `InterestStagePicker` (which does expose the checkbox at line 137).
  Worse, when a user picks a stage that isn't a legal forward
  transition, the inline picker just shows the toast from the server's
  `ConflictError` — instead of "you need override; toggle this box".
- **Server:** `src/app/api/v1/interests/[id]/stage/route.ts:14-22` —
  reads `body.override` and re-checks `interests.override_stage`
  permission.

**Divergence:** UI and permission map diverge in the affordance, not
the gate. End-result: the `override_stage` permission is partially
unreachable from the inline picker. Sales managers / agents can
override only via the modal picker.

**Fix:** when the inline picker sees a transition that isn't allowed
by `canTransitionStage(currentStage, newStage)`, check
`can('interests', 'override_stage')` and either auto-set
`override:true` (with a confirmation) or surface a "Use override"
secondary action. Keep the inline picker UX; just don't let the
override permission be silently inaccessible from the most-used
path.

---

### M3. `sales_agent` granted `interests.override_stage:true` — possible copy-paste from sales_manager

- **Seed:** `src/lib/db/seed-permissions.ts:253` — `SALES_AGENT_PERMISSIONS.interests.override_stage = true`.

This is identical to `SALES_MANAGER_PERMISSIONS.interests.override_stage = true`
at line 176. The same `sales_agent` block has `delete:false` for
clients/interests/yachts/companies/files/etc — all the other
"trust-elevated" flags are explicitly stripped from sales_agent. The
ability to bypass the pipeline-stage transition table is a meaningful
trust elevation: it lets an agent skip prerequisites (e.g. mark an
interest as `eoi_signed` without an actual signed doc) which has
downstream implications for the public berths feed (`Under Offer`
status), the recommender's tier ladder, and the EOI bundle.

**Divergence:** likely intent vs. permission map. Worth confirming
with a product owner; if intentional, leave a code comment. If
unintentional, flip to `false`.

**Fix:** product decision. If demoted, also update
`src/components/admin/roles/role-form.tsx → DEFAULT_PERMISSIONS`
(noted in the file header at seed-permissions.ts:9) so the UI
default for new roles matches.

---

### M4. `bulk-archive-preflight` returns dossier even when client is in another port (defense-in-depth)

- **Server:** `src/app/api/v1/clients/bulk-archive-preflight/route.ts:33-62`

The route loops through `ids` and calls `getClientArchiveDossier(id, ctx.portId)`
for each. If a `clientId` belongs to another port, `getClientArchiveDossier`
throws and the route catches it (line 52-61) and returns a fallback row
with `blockers: ['<error message>']`. This leaks **the existence of an
unknown client id** — an attacker enumerating UUIDs can distinguish
"client doesn't exist" from "client exists but you can't see it" by
parsing the blocker text. The bulk hard-delete route has the same
shape but returns `NotFoundError`.

**Divergence (perm-adjacent):** the preflight route doesn't enforce a
per-id port check before falling through to the dossier service, and
the catch block leaks the failure mode in the response.

**Fix:** in the catch block, replace the dossier error message with a
generic `'Could not load dossier'` blocker. The operator already
selected these ids so they know the count; they don't need the inner
error.

---

## LOW

### L1. `external-eoi` route doesn't enforce `interests.edit` defense-in-depth on the interest port

- **Server:** `src/app/api/v1/interests/[id]/external-eoi/route.ts:8-14`

The route receives `interestId` from the URL and passes it +
`ctx.portId` into `uploadExternallySignedEoi`. The service is
expected to enforce port isolation, but the route itself does no
upfront `(interestId, portId)` existence check before reading the
multipart body — meaning a cross-port id will fully process the
upload (read the file into memory) before the service rejects.

**Divergence:** not strictly a permission divergence; it's resource
waste from missing early port-ownership check. Low because the
service-level reject does close the security hole.

**Fix:** add a one-row `select` on `interests` matching `id` + `portId`
before parsing form data, throw `NotFoundError` on miss.

---

## Summary

- 0 critical
- 4 high (H1–H4)
- 4 medium (M1–M4)
- 1 low (L1)

Top recommendation: H1 (webhook-replay UI gate) is a
ten-line fix that closes a 403-toast UX bug. H2 + H3 (bulk-archive +
bulk-tag UI gates) are also trivial and remove the same class of bug
across the bulk actions menu. M3 (sales_agent override_stage) needs a
product decision, not code; flag it before shipping the audit.