4c5334d471
Three findings from the branch security review:
1. HIGH — Privilege escalation via super-admin invite. POST
/api/v1/admin/invitations was gated only by manage_users (held by the
port-scoped director role). The body schema accepted isSuperAdmin
from the request, createCrmInvite persisted it verbatim, and
consumeCrmInvite copied it into userProfiles.isSuperAdmin — granting
the new account cross-tenant access. Now the route rejects
isSuperAdmin=true unless ctx.isSuperAdmin, and createCrmInvite
requires invitedBy.isSuperAdmin as defense-in-depth.
2. HIGH — Receipt-image exfiltration via OCR settings. The route
/api/v1/admin/ocr-settings (and the sibling /test) were wrapped only
in withAuth — any port role including viewer could PUT a swapped
provider apiKey + flip aiEnabled, redirecting every subsequent
receipt scan to attacker infrastructure. Both are now wrapped in
withPermission('admin','manage_settings',…) matching the sibling
admin routes (ai-budget, settings).
3. MEDIUM — Cross-tenant alert IDOR. dismissAlert / acknowledgeAlert
issued UPDATE … WHERE id=? with no portId predicate. Any
authenticated user with a foreign alert UUID could mutate it. Both
service functions now require portId and add it to the WHERE; the
route handlers pass ctx.portId.
The dev-trigger-crm-invite script passes a synthetic super-admin caller
identity since it runs out-of-band.
The two public-form tests randomize their IP prefix per run so a fresh
test process doesn't collide with leftover redis sliding-window entries
from a prior run (publicForm limiter pexpires after 1h).
Two new regression test files cover the fixes (6 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
90 lines
3.0 KiB
TypeScript
90 lines
3.0 KiB
TypeScript
/**
|
|
* Security regression: dismissAlert / acknowledgeAlert must filter by
|
|
* portId so an alert UUID from port A can't be mutated by a session
|
|
* scoped to port B.
|
|
*/
|
|
|
|
import { describe, it, expect, beforeAll } from 'vitest';
|
|
import { eq } from 'drizzle-orm';
|
|
|
|
import { db } from '@/lib/db';
|
|
import { alerts } from '@/lib/db/schema/insights';
|
|
import { user } from '@/lib/db/schema/users';
|
|
import { dismissAlert, acknowledgeAlert } from '@/lib/services/alerts.service';
|
|
import { makePort } from '../helpers/factories';
|
|
|
|
let TEST_USER_ID = '';
|
|
|
|
beforeAll(async () => {
|
|
// dismissedBy / acknowledgedBy reference user.id; pull any seeded user.
|
|
const [u] = await db.select({ id: user.id }).from(user).limit(1);
|
|
if (!u) throw new Error('No user available; run pnpm db:seed first');
|
|
TEST_USER_ID = u.id;
|
|
});
|
|
|
|
async function makeAlert(portId: string) {
|
|
const [row] = await db
|
|
.insert(alerts)
|
|
.values({
|
|
portId,
|
|
ruleId: 'document_overdue',
|
|
severity: 'medium',
|
|
title: 'Test alert',
|
|
link: '/test',
|
|
fingerprint: `fp-${Math.random().toString(36).slice(2)}`,
|
|
metadata: {},
|
|
})
|
|
.returning({ id: alerts.id });
|
|
if (!row) throw new Error('failed to insert alert');
|
|
return row.id;
|
|
}
|
|
|
|
describe('alerts service — tenant isolation', () => {
|
|
it('dismissAlert is a no-op when called with the wrong portId', async () => {
|
|
const portA = await makePort();
|
|
const portB = await makePort();
|
|
const alertId = await makeAlert(portA.id);
|
|
|
|
// Attacker session scoped to port B tries to dismiss port A's alert.
|
|
await dismissAlert(alertId, portB.id, TEST_USER_ID);
|
|
|
|
const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) });
|
|
expect(after?.dismissedAt).toBeNull();
|
|
expect(after?.dismissedBy).toBeNull();
|
|
});
|
|
|
|
it('dismissAlert succeeds when portId matches', async () => {
|
|
const port = await makePort();
|
|
const alertId = await makeAlert(port.id);
|
|
|
|
await dismissAlert(alertId, port.id, TEST_USER_ID);
|
|
|
|
const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) });
|
|
expect(after?.dismissedAt).not.toBeNull();
|
|
expect(after?.dismissedBy).toBe(TEST_USER_ID);
|
|
});
|
|
|
|
it('acknowledgeAlert is a no-op when called with the wrong portId', async () => {
|
|
const portA = await makePort();
|
|
const portB = await makePort();
|
|
const alertId = await makeAlert(portA.id);
|
|
|
|
await acknowledgeAlert(alertId, portB.id, TEST_USER_ID);
|
|
|
|
const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) });
|
|
expect(after?.acknowledgedAt).toBeNull();
|
|
expect(after?.acknowledgedBy).toBeNull();
|
|
});
|
|
|
|
it('acknowledgeAlert succeeds when portId matches', async () => {
|
|
const port = await makePort();
|
|
const alertId = await makeAlert(port.id);
|
|
|
|
await acknowledgeAlert(alertId, port.id, TEST_USER_ID);
|
|
|
|
const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) });
|
|
expect(after?.acknowledgedAt).not.toBeNull();
|
|
expect(after?.acknowledgedBy).toBe(TEST_USER_ID);
|
|
});
|
|
});
|