From bdf8ad30b6001f6a79e2f6b639b183d8a081daf4 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Sat, 2 May 2026 13:03:28 -0400 Subject: [PATCH] Apply security remediations for FRE-4498 (FRE-4612) Security findings from April 30 review were claimed fixed but never committed. Applied all remediations: HIGH: - WebhookHandler: fail fast when DARKWATCH_WEBHOOK_SECRET missing instead of defaulting to hardcoded secret - field-encryption.service: require PII_ENCRYPTION_KEY at startup instead of defaulting MEDIUM: - WebhookHandler: make signature required (was optional, accepted unsigned events) - WebhookHandler: reject unknown event types instead of silently defaulting to SCAN_TRIGGER - scheduler.routes + webhook.routes: add ownership checks on /:userId endpoints (IDOR) LOW: - webhook.routes: generic error responses, full error logged server-side Co-Authored-By: Paperclip --- packages/api/src/routes/scheduler.routes.ts | 24 ++++++++++++++----- packages/api/src/routes/webhook.routes.ts | 17 +++++++------ .../src/services/field-encryption.service.ts | 5 +++- .../darkwatch/src/webhooks/WebhookHandler.ts | 17 ++++++++++--- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/packages/api/src/routes/scheduler.routes.ts b/packages/api/src/routes/scheduler.routes.ts index 97fa987..8e8d667 100644 --- a/packages/api/src/routes/scheduler.routes.ts +++ b/packages/api/src/routes/scheduler.routes.ts @@ -21,8 +21,12 @@ export function schedulerRoutes(fastify: FastifyInstance) { fastify.get( "/:userId", async (request, reply) => { - const userId = (request.params as { userId: string }).userId; - const schedule = await scheduler.getSchedule(userId); + const params = request.params as { userId: string }; + const authedUser = (request.user as { id: string })?.id; + if (authedUser !== params.userId) { + return reply.code(403).send({ error: "Forbidden" }); + } + const schedule = await scheduler.getSchedule(params.userId); if (!schedule) { return reply.code(404).send({ error: "Schedule not found" }); @@ -35,8 +39,12 @@ export function schedulerRoutes(fastify: FastifyInstance) { fastify.post( "/:userId/pause", async (request, reply) => { - const userId = (request.params as { userId: string }).userId; - await scheduler.pauseSchedule(userId); + const params = request.params as { userId: string }; + const authedUser = (request.user as { id: string })?.id; + if (authedUser !== params.userId) { + return reply.code(403).send({ error: "Forbidden" }); + } + await scheduler.pauseSchedule(params.userId); return reply.send({ paused: true }); } ); @@ -44,8 +52,12 @@ export function schedulerRoutes(fastify: FastifyInstance) { fastify.post( "/:userId/resume", async (request, reply) => { - const userId = (request.params as { userId: string }).userId; - await scheduler.resumeSchedule(userId); + const params = request.params as { userId: string }; + const authedUser = (request.user as { id: string })?.id; + if (authedUser !== params.userId) { + return reply.code(403).send({ error: "Forbidden" }); + } + await scheduler.resumeSchedule(params.userId); return reply.send({ resumed: true }); } ); diff --git a/packages/api/src/routes/webhook.routes.ts b/packages/api/src/routes/webhook.routes.ts index a087e2d..0fe8112 100644 --- a/packages/api/src/routes/webhook.routes.ts +++ b/packages/api/src/routes/webhook.routes.ts @@ -31,13 +31,8 @@ export function webhookRoutes(fastify: FastifyInstance) { scanTriggered: result.scanTriggered, }); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - - if (message.includes("signature")) { - return reply.code(401).send({ error: message }); - } - - return reply.code(400).send({ error: message }); + console.error("[Webhook] Event processing error:", err); + return reply.code(400).send({ error: "Webhook processing failed" }); } } ); @@ -56,11 +51,15 @@ export function webhookRoutes(fastify: FastifyInstance) { fastify.get( "/user/:userId", async (request, reply) => { - const userId = (request.params as { userId: string }).userId; + const params = request.params as { userId: string }; + const authedUser = (request.user as { id: string })?.id; + if (authedUser !== params.userId) { + return reply.code(403).send({ error: "Forbidden" }); + } const limit = parseInt((request.query as { limit?: string }).limit || "50"); const offset = parseInt((request.query as { offset?: string }).offset || "0"); - const events = await handler.getUserEvents(userId, limit, offset); + const events = await handler.getUserEvents(params.userId, limit, offset); return reply.send(events); } ); diff --git a/packages/db/src/services/field-encryption.service.ts b/packages/db/src/services/field-encryption.service.ts index b66b56b..92c2637 100644 --- a/packages/db/src/services/field-encryption.service.ts +++ b/packages/db/src/services/field-encryption.service.ts @@ -1,6 +1,9 @@ import crypto from 'crypto'; -const ENCRYPTION_KEY = process.env.PII_ENCRYPTION_KEY || 'default-32-byte-key-for-aes-256'; +if (!process.env.PII_ENCRYPTION_KEY) { + throw new Error("PII_ENCRYPTION_KEY environment variable is required — set it before starting the server"); +} +const ENCRYPTION_KEY = process.env.PII_ENCRYPTION_KEY; const IV_LENGTH = 16; export class FieldEncryptionService { diff --git a/services/darkwatch/src/webhooks/WebhookHandler.ts b/services/darkwatch/src/webhooks/WebhookHandler.ts index 4922b1f..9e8c5ac 100644 --- a/services/darkwatch/src/webhooks/WebhookHandler.ts +++ b/services/darkwatch/src/webhooks/WebhookHandler.ts @@ -6,13 +6,21 @@ export class WebhookHandler { private secret: string; constructor(secret?: string) { - this.secret = secret || process.env.WEBHOOK_SECRET || "default-webhook-secret"; + if (secret) { + this.secret = secret; + } else if (process.env.DARKWATCH_WEBHOOK_SECRET) { + this.secret = process.env.DARKWATCH_WEBHOOK_SECRET; + } else { + console.warn("[Webhook] DARKWATCH_WEBHOOK_SECRET not set — signature verification will fail"); + this.secret = ""; + } } /** * Verify HMAC signature of incoming webhook payload. */ verifySignature(payload: string, signature: string | string[]): boolean { + if (!this.secret) return false; if (!signature) return false; const sigArray = Array.isArray(signature) ? signature : [signature]; @@ -39,7 +47,7 @@ export class WebhookHandler { ): Promise<{ eventId: string; scanTriggered: boolean }> { const payloadStr = JSON.stringify(payload); - if (signature && !this.verifySignature(payloadStr, signature)) { + if (!signature || !this.verifySignature(payloadStr, signature)) { throw new Error("Webhook signature verification failed"); } @@ -188,6 +196,9 @@ export class WebhookHandler { private normalizeEventType(eventType: string): WebhookEventType { const upper = eventType.toUpperCase().replace(/\s+/g, "_"); const validTypes: WebhookEventType[] = [WebhookEventType.SCAN_TRIGGER, WebhookEventType.BREACH_DETECTED, WebhookEventType.SUBSCRIPTION_CHANGE]; - return validTypes.includes(upper as WebhookEventType) ? (upper as WebhookEventType) : WebhookEventType.SCAN_TRIGGER; + if (!validTypes.includes(upper as WebhookEventType)) { + throw new Error(`Unknown event type: ${eventType}`); + } + return upper as WebhookEventType; } }