From a8c6c071e6a2f5d27d81c2ee66727d087c919ff2 Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Wed, 6 May 2026 22:15:01 +0200 Subject: [PATCH] fix(audit): permission UI gates + preflight leak (R2-H6/H7/H8/H9 + R2-M9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R2-H6: webhook-delivery-log Replay column was rendered for any user who could load the page; the route gates on admin.manage_webhooks. Now the entire Replay column is hidden when the user lacks the perm. R2-H7: Bulk Archive action was visible to sales_agent + viewer (clients.delete:false). Now wrapped in canBulkArchive (clients.delete). R2-H8: Bulk Add tag / Remove tag were visible to viewer (clients.edit: false). Now wrapped in canBulkTag (clients.edit). R2-H9: bulk-hard-delete silently dropped clients that became unarchived between preflight and execute. The service now returns {deletedCount, skipped[]} and the dialog stays open on partial success showing the per-row reason table — operators can see exactly which IDs were skipped and why. R2-M9: bulk-archive-preflight catch block was leaking dossier-loader error messages, letting an attacker enumerate "not found" vs "exists in another port". Replaced with a generic 'Could not load dossier — client may have been removed' blocker. 1175/1175 vitest passing. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../clients/bulk-archive-preflight/route.ts | 8 +- .../admin/webhooks/webhook-delivery-log.tsx | 39 ++++---- .../clients/bulk-hard-delete-dialog.tsx | 95 +++++++++++++++---- src/components/clients/client-list.tsx | 64 +++++++------ .../services/client-hard-delete.service.ts | 31 +++++- 5 files changed, 170 insertions(+), 67 deletions(-) diff --git a/src/app/api/v1/clients/bulk-archive-preflight/route.ts b/src/app/api/v1/clients/bulk-archive-preflight/route.ts index 3f6ba2b..07bccde 100644 --- a/src/app/api/v1/clients/bulk-archive-preflight/route.ts +++ b/src/app/api/v1/clients/bulk-archive-preflight/route.ts @@ -49,13 +49,17 @@ export const POST = withAuth( ).length, }, }); - } catch (err) { + } catch { + // Generic blocker text — never include the inner error so an + // attacker can't distinguish "not found" from "in another port" + // by enumerating UUIDs (audit R2-M9). The operator already + // selected these IDs so they don't need to know the cause. items.push({ clientId: id, fullName: '(unknown)', stakeLevel: 'low', highStakesStage: null, - blockers: [err instanceof Error ? err.message : 'Failed to load dossier'], + blockers: ['Could not load dossier — client may have been removed'], summary: { berths: 0, yachts: 0, reservations: 0, signedDocs: 0 }, }); } diff --git a/src/components/admin/webhooks/webhook-delivery-log.tsx b/src/components/admin/webhooks/webhook-delivery-log.tsx index 2266ad7..d96add9 100644 --- a/src/components/admin/webhooks/webhook-delivery-log.tsx +++ b/src/components/admin/webhooks/webhook-delivery-log.tsx @@ -14,6 +14,7 @@ import { TableRow, } from '@/components/ui/table'; import { apiFetch } from '@/lib/api/client'; +import { usePermissions } from '@/hooks/use-permissions'; interface Delivery { id: string; @@ -42,6 +43,8 @@ export function WebhookDeliveryLog({ webhookId }: Props) { const [page, setPage] = useState(1); const [loading, setLoading] = useState(true); const [retrying, setRetrying] = useState(null); + const { can } = usePermissions(); + const canReplay = can('admin', 'manage_webhooks'); async function retry(deliveryId: string) { setRetrying(deliveryId); @@ -98,7 +101,7 @@ export function WebhookDeliveryLog({ webhookId }: Props) { HTTP Attempt Time - Replay + {canReplay && Replay} @@ -115,22 +118,24 @@ export function WebhookDeliveryLog({ webhookId }: Props) { ? new Date(d.deliveredAt).toLocaleString() : new Date(d.createdAt).toLocaleString()} - - {(d.status === 'failed' || d.status === 'dead_letter') && ( - - )} - + {canReplay && ( + + {(d.status === 'failed' || d.status === 'dead_letter') && ( + + )} + + )} ))} diff --git a/src/components/clients/bulk-hard-delete-dialog.tsx b/src/components/clients/bulk-hard-delete-dialog.tsx index fd56ae4..611fb67 100644 --- a/src/components/clients/bulk-hard-delete-dialog.tsx +++ b/src/components/clients/bulk-hard-delete-dialog.tsx @@ -25,7 +25,12 @@ interface Props { onDeleted?: (deletedCount: number) => void; } -type Stage = 'intent' | 'confirm'; +type Stage = 'intent' | 'confirm' | 'partial'; + +interface SkippedRow { + clientId: string; + reason: string; +} export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted }: Props) { const qc = useQueryClient(); @@ -33,6 +38,8 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted const [code, setCode] = useState(''); const [typedPhrase, setTypedPhrase] = useState(''); const [maskedEmail, setMaskedEmail] = useState(null); + const [skipped, setSkipped] = useState([]); + const [partialDeleted, setPartialDeleted] = useState(0); const expectedPhrase = `DELETE ${clientIds.length} CLIENT${clientIds.length === 1 ? '' : 'S'}`; @@ -42,6 +49,8 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted setCode(''); setTypedPhrase(''); setMaskedEmail(null); + setSkipped([]); + setPartialDeleted(0); } }, [open]); @@ -63,21 +72,29 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted const bulkDelete = useMutation({ mutationFn: () => - apiFetch<{ data: { deletedCount: number } }>('/api/v1/clients/bulk-hard-delete', { + apiFetch<{ + data: { deletedCount: number; skipped: SkippedRow[] }; + }>('/api/v1/clients/bulk-hard-delete', { method: 'POST', body: { ids: clientIds, code, typedPhrase }, }), onSuccess: (res) => { const n = res.data.deletedCount; - const failed = clientIds.length - n; - if (failed === 0) { - toast.success(`${n} client${n === 1 ? '' : 's'} permanently deleted.`); - } else { - toast.warning(`${n} of ${clientIds.length} deleted. ${failed} failed (see audit log).`); - } + const skippedRows = res.data.skipped ?? []; qc.invalidateQueries({ queryKey: ['clients'] }); - onOpenChange(false); - onDeleted?.(n); + if (skippedRows.length === 0) { + toast.success(`${n} client${n === 1 ? '' : 's'} permanently deleted.`); + onOpenChange(false); + onDeleted?.(n); + } else { + // Stay open so the operator can see exactly which IDs were + // skipped and why (e.g. unarchived between preflight + execute, + // already deleted by another operator). + setSkipped(skippedRows); + setPartialDeleted(n); + setStage('partial'); + toast.warning(`${n} of ${clientIds.length} deleted. ${skippedRows.length} skipped.`); + } }, onError: (err: unknown) => { toast.error(err instanceof Error ? err.message : 'Bulk delete failed'); @@ -100,7 +117,7 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted - {stage === 'intent' ? ( + {stage === 'intent' && (

We’ll email a 4-digit confirmation code to your account address. The code is @@ -112,7 +129,9 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted files and reminders are detached but kept.

- ) : ( + )} + + {stage === 'confirm' && (
@@ -150,11 +169,42 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
)} + {stage === 'partial' && ( +
+
+ {partialDeleted} of {clientIds.length} permanently deleted. {skipped.length} skipped — + see below. +
+
+ + + + + + + + + {skipped.map((row) => ( + + + + + ))} + +
Client IDReason
+ {row.clientId.slice(0, 8)}… + {row.reason}
+
+
+ )} + - - {stage === 'intent' ? ( + {stage !== 'partial' && ( + + )} + {stage === 'intent' && ( - ) : ( + )} + {stage === 'confirm' && ( )} + {stage === 'partial' && ( + + )} diff --git a/src/components/clients/client-list.tsx b/src/components/clients/client-list.tsx index c2a066a..3485019 100644 --- a/src/components/clients/client-list.tsx +++ b/src/components/clients/client-list.tsx @@ -51,6 +51,8 @@ export function ClientList() { const { can } = usePermissions(); const canHardDelete = can('admin', 'permanently_delete_clients'); + const canBulkArchive = can('clients', 'delete'); + const canBulkTag = can('clients', 'edit'); const { data, @@ -161,33 +163,41 @@ export function ClientList() { isLoading={isFetching && !isLoading} getRowId={(row) => row.id} bulkActions={[ - { - label: 'Add tag', - icon: TagIcon, - onClick: (ids) => { - if (ids.length === 0) return; - setTagChoice([]); - setTagDialog({ ids, mode: 'add' }); - }, - }, - { - label: 'Remove tag', - icon: TagsIcon, - onClick: (ids) => { - if (ids.length === 0) return; - setTagChoice([]); - setTagDialog({ ids, mode: 'remove' }); - }, - }, - { - label: 'Archive', - icon: Archive, - variant: 'destructive', - onClick: (ids) => { - if (ids.length === 0) return; - setBulkArchiveIds(ids); - }, - }, + ...(canBulkTag + ? [ + { + label: 'Add tag', + icon: TagIcon, + onClick: (ids: string[]) => { + if (ids.length === 0) return; + setTagChoice([]); + setTagDialog({ ids, mode: 'add' }); + }, + }, + { + label: 'Remove tag', + icon: TagsIcon, + onClick: (ids: string[]) => { + if (ids.length === 0) return; + setTagChoice([]); + setTagDialog({ ids, mode: 'remove' }); + }, + }, + ] + : []), + ...(canBulkArchive + ? [ + { + label: 'Archive', + icon: Archive, + variant: 'destructive' as const, + onClick: (ids: string[]) => { + if (ids.length === 0) return; + setBulkArchiveIds(ids); + }, + }, + ] + : []), ...(canHardDelete ? [ { diff --git a/src/lib/services/client-hard-delete.service.ts b/src/lib/services/client-hard-delete.service.ts index ac36360..e63b674 100644 --- a/src/lib/services/client-hard-delete.service.ts +++ b/src/lib/services/client-hard-delete.service.ts @@ -333,6 +333,14 @@ export async function requestBulkHardDeleteCode(args: { return { count: args.clientIds.length, sentToMaskedEmail: maskEmail(u.email) }; } +export interface BulkHardDeleteResult { + deletedCount: number; + /** Ids that were requested but not deleted, with a per-id reason + * (e.g. became unarchived between preflight and execute, removed by + * another operator, transient failure inside the inner deletion). */ + skipped: Array<{ clientId: string; reason: string }>; +} + export async function bulkHardDeleteClients(args: { clientIds: string[]; portId: string; @@ -340,7 +348,7 @@ export async function bulkHardDeleteClients(args: { code: string; typedPhrase: string; meta: AuditMeta; -}): Promise<{ deletedCount: number }> { +}): Promise { if (args.clientIds.length === 0) { throw new ValidationError('No clients selected'); } @@ -361,6 +369,7 @@ export async function bulkHardDeleteClients(args: { await redis.del(key); let deleted = 0; + const skipped: BulkHardDeleteResult['skipped'] = []; for (const id of args.clientIds) { try { // Reuse the single-client path so the cascade logic stays in one @@ -370,11 +379,21 @@ export async function bulkHardDeleteClients(args: { const oneShot = generateCode(); await redis.set(singleKey, oneShot, 'EX', 60); const [c] = await db - .select({ fullName: clients.fullName }) + .select({ fullName: clients.fullName, archivedAt: clients.archivedAt }) .from(clients) .where(and(eq(clients.id, id), eq(clients.portId, args.portId))) .limit(1); - if (!c) continue; + if (!c) { + skipped.push({ clientId: id, reason: 'client no longer exists' }); + continue; + } + if (!c.archivedAt) { + skipped.push({ + clientId: id, + reason: 'client is no longer archived (un-archived since preflight)', + }); + continue; + } await hardDeleteClient({ clientId: id, portId: args.portId, @@ -386,10 +405,14 @@ export async function bulkHardDeleteClients(args: { deleted += 1; } catch (err) { logger.error({ err, clientId: id }, 'bulk hard-delete: client failed, continuing'); + skipped.push({ + clientId: id, + reason: err instanceof Error ? err.message : 'unknown failure', + }); } } - return { deletedCount: deleted }; + return { deletedCount: deleted, skipped }; } function escapeHtml(s: string): string {