fix(audit): permission UI gates + preflight leak (R2-H6/H7/H8/H9 + R2-M9)

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) <noreply@anthropic.com>
This commit is contained in:
Matt Ciaccio
2026-05-06 22:15:01 +02:00
parent 94331bd6ec
commit a8c6c071e6
5 changed files with 170 additions and 67 deletions

View File

@@ -49,13 +49,17 @@ export const POST = withAuth(
).length, ).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({ items.push({
clientId: id, clientId: id,
fullName: '(unknown)', fullName: '(unknown)',
stakeLevel: 'low', stakeLevel: 'low',
highStakesStage: null, 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 }, summary: { berths: 0, yachts: 0, reservations: 0, signedDocs: 0 },
}); });
} }

View File

@@ -14,6 +14,7 @@ import {
TableRow, TableRow,
} from '@/components/ui/table'; } from '@/components/ui/table';
import { apiFetch } from '@/lib/api/client'; import { apiFetch } from '@/lib/api/client';
import { usePermissions } from '@/hooks/use-permissions';
interface Delivery { interface Delivery {
id: string; id: string;
@@ -42,6 +43,8 @@ export function WebhookDeliveryLog({ webhookId }: Props) {
const [page, setPage] = useState(1); const [page, setPage] = useState(1);
const [loading, setLoading] = useState(true); const [loading, setLoading] = useState(true);
const [retrying, setRetrying] = useState<string | null>(null); const [retrying, setRetrying] = useState<string | null>(null);
const { can } = usePermissions();
const canReplay = can('admin', 'manage_webhooks');
async function retry(deliveryId: string) { async function retry(deliveryId: string) {
setRetrying(deliveryId); setRetrying(deliveryId);
@@ -98,7 +101,7 @@ export function WebhookDeliveryLog({ webhookId }: Props) {
<TableHead>HTTP</TableHead> <TableHead>HTTP</TableHead>
<TableHead>Attempt</TableHead> <TableHead>Attempt</TableHead>
<TableHead>Time</TableHead> <TableHead>Time</TableHead>
<TableHead className="w-16 text-right">Replay</TableHead> {canReplay && <TableHead className="w-16 text-right">Replay</TableHead>}
</TableRow> </TableRow>
</TableHeader> </TableHeader>
<TableBody> <TableBody>
@@ -115,6 +118,7 @@ export function WebhookDeliveryLog({ webhookId }: Props) {
? new Date(d.deliveredAt).toLocaleString() ? new Date(d.deliveredAt).toLocaleString()
: new Date(d.createdAt).toLocaleString()} : new Date(d.createdAt).toLocaleString()}
</TableCell> </TableCell>
{canReplay && (
<TableCell className="text-right"> <TableCell className="text-right">
{(d.status === 'failed' || d.status === 'dead_letter') && ( {(d.status === 'failed' || d.status === 'dead_letter') && (
<Button <Button
@@ -131,6 +135,7 @@ export function WebhookDeliveryLog({ webhookId }: Props) {
</Button> </Button>
)} )}
</TableCell> </TableCell>
)}
</TableRow> </TableRow>
))} ))}
</TableBody> </TableBody>

View File

@@ -25,7 +25,12 @@ interface Props {
onDeleted?: (deletedCount: number) => void; 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) { export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted }: Props) {
const qc = useQueryClient(); const qc = useQueryClient();
@@ -33,6 +38,8 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
const [code, setCode] = useState(''); const [code, setCode] = useState('');
const [typedPhrase, setTypedPhrase] = useState(''); const [typedPhrase, setTypedPhrase] = useState('');
const [maskedEmail, setMaskedEmail] = useState<string | null>(null); const [maskedEmail, setMaskedEmail] = useState<string | null>(null);
const [skipped, setSkipped] = useState<SkippedRow[]>([]);
const [partialDeleted, setPartialDeleted] = useState(0);
const expectedPhrase = `DELETE ${clientIds.length} CLIENT${clientIds.length === 1 ? '' : 'S'}`; const expectedPhrase = `DELETE ${clientIds.length} CLIENT${clientIds.length === 1 ? '' : 'S'}`;
@@ -42,6 +49,8 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
setCode(''); setCode('');
setTypedPhrase(''); setTypedPhrase('');
setMaskedEmail(null); setMaskedEmail(null);
setSkipped([]);
setPartialDeleted(0);
} }
}, [open]); }, [open]);
@@ -63,21 +72,29 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
const bulkDelete = useMutation({ const bulkDelete = useMutation({
mutationFn: () => 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', method: 'POST',
body: { ids: clientIds, code, typedPhrase }, body: { ids: clientIds, code, typedPhrase },
}), }),
onSuccess: (res) => { onSuccess: (res) => {
const n = res.data.deletedCount; const n = res.data.deletedCount;
const failed = clientIds.length - n; const skippedRows = res.data.skipped ?? [];
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).`);
}
qc.invalidateQueries({ queryKey: ['clients'] }); qc.invalidateQueries({ queryKey: ['clients'] });
if (skippedRows.length === 0) {
toast.success(`${n} client${n === 1 ? '' : 's'} permanently deleted.`);
onOpenChange(false); onOpenChange(false);
onDeleted?.(n); 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) => { onError: (err: unknown) => {
toast.error(err instanceof Error ? err.message : 'Bulk delete failed'); toast.error(err instanceof Error ? err.message : 'Bulk delete failed');
@@ -100,7 +117,7 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
</DialogDescription> </DialogDescription>
</DialogHeader> </DialogHeader>
{stage === 'intent' ? ( {stage === 'intent' && (
<div className="space-y-3 text-sm text-muted-foreground"> <div className="space-y-3 text-sm text-muted-foreground">
<p> <p>
We&rsquo;ll email a 4-digit confirmation code to your account address. The code is We&rsquo;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. files and reminders are detached but kept.
</div> </div>
</div> </div>
) : ( )}
{stage === 'confirm' && (
<div className="space-y-3"> <div className="space-y-3">
<div className="flex items-start gap-2 rounded-md border border-blue-300 bg-blue-50 p-3 text-xs text-blue-900"> <div className="flex items-start gap-2 rounded-md border border-blue-300 bg-blue-50 p-3 text-xs text-blue-900">
<Mail className="h-4 w-4 shrink-0 mt-0.5" /> <Mail className="h-4 w-4 shrink-0 mt-0.5" />
@@ -150,11 +169,42 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
</div> </div>
)} )}
{stage === 'partial' && (
<div className="space-y-3 text-sm">
<div className="rounded-md border border-amber-300 bg-amber-50 p-3 text-amber-900">
{partialDeleted} of {clientIds.length} permanently deleted. {skipped.length} skipped
see below.
</div>
<div className="rounded-md border max-h-60 overflow-y-auto">
<table className="w-full text-xs">
<thead className="bg-muted/50 sticky top-0">
<tr>
<th className="text-left px-2 py-1.5 font-medium">Client ID</th>
<th className="text-left px-2 py-1.5 font-medium">Reason</th>
</tr>
</thead>
<tbody>
{skipped.map((row) => (
<tr key={row.clientId} className="border-t">
<td className="px-2 py-1.5 font-mono text-[11px]">
{row.clientId.slice(0, 8)}
</td>
<td className="px-2 py-1.5">{row.reason}</td>
</tr>
))}
</tbody>
</table>
</div>
</div>
)}
<DialogFooter> <DialogFooter>
{stage !== 'partial' && (
<Button variant="outline" onClick={() => onOpenChange(false)}> <Button variant="outline" onClick={() => onOpenChange(false)}>
Cancel Cancel
</Button> </Button>
{stage === 'intent' ? ( )}
{stage === 'intent' && (
<Button <Button
variant="destructive" variant="destructive"
onClick={() => requestCode.mutate()} onClick={() => requestCode.mutate()}
@@ -168,7 +218,8 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
'Send confirmation code' 'Send confirmation code'
)} )}
</Button> </Button>
) : ( )}
{stage === 'confirm' && (
<Button <Button
variant="destructive" variant="destructive"
onClick={() => bulkDelete.mutate()} onClick={() => bulkDelete.mutate()}
@@ -183,6 +234,16 @@ export function BulkHardDeleteDialog({ open, onOpenChange, clientIds, onDeleted
)} )}
</Button> </Button>
)} )}
{stage === 'partial' && (
<Button
onClick={() => {
onOpenChange(false);
onDeleted?.(partialDeleted);
}}
>
Done
</Button>
)}
</DialogFooter> </DialogFooter>
</DialogContent> </DialogContent>
</Dialog> </Dialog>

View File

@@ -51,6 +51,8 @@ export function ClientList() {
const { can } = usePermissions(); const { can } = usePermissions();
const canHardDelete = can('admin', 'permanently_delete_clients'); const canHardDelete = can('admin', 'permanently_delete_clients');
const canBulkArchive = can('clients', 'delete');
const canBulkTag = can('clients', 'edit');
const { const {
data, data,
@@ -161,10 +163,12 @@ export function ClientList() {
isLoading={isFetching && !isLoading} isLoading={isFetching && !isLoading}
getRowId={(row) => row.id} getRowId={(row) => row.id}
bulkActions={[ bulkActions={[
...(canBulkTag
? [
{ {
label: 'Add tag', label: 'Add tag',
icon: TagIcon, icon: TagIcon,
onClick: (ids) => { onClick: (ids: string[]) => {
if (ids.length === 0) return; if (ids.length === 0) return;
setTagChoice([]); setTagChoice([]);
setTagDialog({ ids, mode: 'add' }); setTagDialog({ ids, mode: 'add' });
@@ -173,21 +177,27 @@ export function ClientList() {
{ {
label: 'Remove tag', label: 'Remove tag',
icon: TagsIcon, icon: TagsIcon,
onClick: (ids) => { onClick: (ids: string[]) => {
if (ids.length === 0) return; if (ids.length === 0) return;
setTagChoice([]); setTagChoice([]);
setTagDialog({ ids, mode: 'remove' }); setTagDialog({ ids, mode: 'remove' });
}, },
}, },
]
: []),
...(canBulkArchive
? [
{ {
label: 'Archive', label: 'Archive',
icon: Archive, icon: Archive,
variant: 'destructive', variant: 'destructive' as const,
onClick: (ids) => { onClick: (ids: string[]) => {
if (ids.length === 0) return; if (ids.length === 0) return;
setBulkArchiveIds(ids); setBulkArchiveIds(ids);
}, },
}, },
]
: []),
...(canHardDelete ...(canHardDelete
? [ ? [
{ {

View File

@@ -333,6 +333,14 @@ export async function requestBulkHardDeleteCode(args: {
return { count: args.clientIds.length, sentToMaskedEmail: maskEmail(u.email) }; 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: { export async function bulkHardDeleteClients(args: {
clientIds: string[]; clientIds: string[];
portId: string; portId: string;
@@ -340,7 +348,7 @@ export async function bulkHardDeleteClients(args: {
code: string; code: string;
typedPhrase: string; typedPhrase: string;
meta: AuditMeta; meta: AuditMeta;
}): Promise<{ deletedCount: number }> { }): Promise<BulkHardDeleteResult> {
if (args.clientIds.length === 0) { if (args.clientIds.length === 0) {
throw new ValidationError('No clients selected'); throw new ValidationError('No clients selected');
} }
@@ -361,6 +369,7 @@ export async function bulkHardDeleteClients(args: {
await redis.del(key); await redis.del(key);
let deleted = 0; let deleted = 0;
const skipped: BulkHardDeleteResult['skipped'] = [];
for (const id of args.clientIds) { for (const id of args.clientIds) {
try { try {
// Reuse the single-client path so the cascade logic stays in one // Reuse the single-client path so the cascade logic stays in one
@@ -370,11 +379,21 @@ export async function bulkHardDeleteClients(args: {
const oneShot = generateCode(); const oneShot = generateCode();
await redis.set(singleKey, oneShot, 'EX', 60); await redis.set(singleKey, oneShot, 'EX', 60);
const [c] = await db const [c] = await db
.select({ fullName: clients.fullName }) .select({ fullName: clients.fullName, archivedAt: clients.archivedAt })
.from(clients) .from(clients)
.where(and(eq(clients.id, id), eq(clients.portId, args.portId))) .where(and(eq(clients.id, id), eq(clients.portId, args.portId)))
.limit(1); .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({ await hardDeleteClient({
clientId: id, clientId: id,
portId: args.portId, portId: args.portId,
@@ -386,10 +405,14 @@ export async function bulkHardDeleteClients(args: {
deleted += 1; deleted += 1;
} catch (err) { } catch (err) {
logger.error({ err, clientId: id }, 'bulk hard-delete: client failed, continuing'); 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 { function escapeHtml(s: string): string {