sec: webhook SSRF guard, IMAP-sync owner check, watcher port membership

Three findings from a fourth-pass review:

1. MEDIUM — webhook URL SSRF. The validator only enforced HTTPS+URL
   parse; it accepted private/loopback/link-local/.internal hosts. The
   delivery worker fetched arbitrary URLs and persisted up to 1KB of
   response body into webhook_deliveries.response_body, which is then
   surfaced via the deliveries listing endpoint — a port admin could
   register a webhook to an internal HTTPS endpoint, hit the test
   endpoint to force immediate dispatch, and read the response back.
   Validator now rejects RFC-1918/loopback/link-local/CGNAT/ULA IPs
   (v4 + v6) and .internal/.local/.localhost/.lan/.intranet/.corp
   suffixes; the worker re-resolves the hostname at dispatch time and
   blocks before fetch (DNS rebinding defense). 21-case unit test
   covers the matrix.

2. MEDIUM — POST /api/v1/email/accounts/[id]/sync had no owner check.
   Any user with email:view could enqueue an inbox-sync job for any
   accountId, which the worker would honour using the foreign user's
   decrypted IMAP credentials and advance the account's lastSyncAt
   (data-loss risk on the legitimate owner's next sync). Route now
   asserts account.userId === ctx.userId before enqueueing, matching
   the toggle/disconnect endpoints.

3. MEDIUM — addDocumentWatcher (and the wizard / upload watcher
   inserts) didn't validate the watcher's userId belonged to the
   document's port. notifyDocumentEvent then emitted a real-time
   socket toast + email containing the document title to the foreign
   user. New assertWatchersInPort helper verifies each candidate has
   a userPortRoles row for the port (super-admin bypass).

818 vitest tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matt Ciaccio
2026-04-29 03:15:39 +02:00
parent 9a5479c2c7
commit 47a1a51832
5 changed files with 260 additions and 20 deletions

View File

@@ -1,9 +1,46 @@
import { Worker, type Job } from 'bullmq';
import { createHmac } from 'node:crypto';
import { lookup } from 'node:dns/promises';
import type { ConnectionOptions } from 'bullmq';
import { logger } from '@/lib/logger';
import { QUEUE_CONFIGS } from '@/lib/queue';
import { isLocalOrPrivateHost } from '@/lib/validators/webhooks';
/**
* Resolve the webhook hostname and reject if any returned address is in a
* disallowed range. Defends against DNS rebinding where the validator-time
* resolution returned a public address but dispatch-time resolution
* returns a private one.
*/
async function resolveAndCheckHost(
rawUrl: string,
): Promise<{ ok: true } | { ok: false; reason: string }> {
if (isLocalOrPrivateHost(rawUrl)) {
return { ok: false, reason: 'webhook URL host blocked by static check' };
}
let host: string;
try {
host = new URL(rawUrl).hostname;
} catch {
return { ok: false, reason: 'invalid URL' };
}
try {
const addresses = await lookup(host, { all: true });
for (const a of addresses) {
// Reuse the validator's literal-address checks on each resolved IP.
if (isLocalOrPrivateHost(`https://${a.family === 6 ? `[${a.address}]` : a.address}`)) {
return { ok: false, reason: `resolved address ${a.address} is in a blocked range` };
}
}
} catch (err) {
return {
ok: false,
reason: `DNS resolution failed: ${err instanceof Error ? err.message : 'unknown'}`,
};
}
return { ok: true };
}
// ─── Job Payload ─────────────────────────────────────────────────────────────
@@ -27,8 +64,7 @@ export const webhooksWorker = new Worker(
return;
}
const { webhookId, portId, event, deliveryId, payload } =
job.data as WebhookDeliverPayload;
const { webhookId, portId, event, deliveryId, payload } = job.data as WebhookDeliverPayload;
const { db } = await import('@/lib/db');
const { webhooks, webhookDeliveries } = await import('@/lib/db/schema/system');
@@ -44,9 +80,7 @@ export const webhooksWorker = new Worker(
if (!webhook) {
logger.info({ webhookId }, 'Webhook deleted — skipping delivery');
await db
.delete(webhookDeliveries)
.where(eq(webhookDeliveries.id, deliveryId));
await db.delete(webhookDeliveries).where(eq(webhookDeliveries.id, deliveryId));
return;
}
@@ -82,6 +116,32 @@ export const webhooksWorker = new Worker(
let responseBody: string | null = null;
let success = false;
// SSRF gate: re-resolve the hostname at dispatch time and reject if it
// points anywhere internal. The validator already filtered literal
// hostnames at create/update time, but DNS rebinding could swap the
// answer between then and now.
const hostCheck = await resolveAndCheckHost(webhook.url);
if (!hostCheck.ok) {
logger.warn(
{ webhookId, deliveryId, url: webhook.url, reason: hostCheck.reason },
'Webhook dispatch blocked by SSRF guard',
);
// Persist the failure so the deliveries listing reflects it.
const { db: dbInner } = await import('@/lib/db');
const { webhookDeliveries } = await import('@/lib/db/schema/system');
const { eq } = await import('drizzle-orm');
await dbInner
.update(webhookDeliveries)
.set({
status: 'dead_letter',
responseStatus: null,
responseBody: `Blocked: ${hostCheck.reason}`,
deliveredAt: new Date(),
})
.where(eq(webhookDeliveries.id, deliveryId));
return;
}
try {
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10_000);