diff --git a/apps/backend/src/app/api/admin/webhooks/dlq/route.ts b/apps/backend/src/app/api/admin/webhooks/dlq/route.ts index f6ec4c13..31acbaf0 100644 --- a/apps/backend/src/app/api/admin/webhooks/dlq/route.ts +++ b/apps/backend/src/app/api/admin/webhooks/dlq/route.ts @@ -1,15 +1,18 @@ import { NextRequest, NextResponse } from 'next/server'; import { withRole } from '@/lib/api/with-role'; +import { withGitHubWebhookAuth } from '@/lib/github/github-webhook'; import { webhookDLQ } from '@/lib/webhook-dlq/dead-letter-queue'; /** * GET /api/admin/webhooks/dlq - * List all dead-letter queue entries (admin only). + * List all dead-letter queue entries (admin only, signature-verified). */ -export const GET = withRole('admin', async (_req: NextRequest) => { - const entries = webhookDLQ.list(); - return NextResponse.json({ total: entries.length, entries }); -}); +export const GET = withGitHubWebhookAuth( + withRole('admin', async (_req: NextRequest) => { + const entries = webhookDLQ.list(); + return NextResponse.json({ total: entries.length, entries }); + }) +); /** * POST /api/admin/webhooks/dlq/:id/reprocess @@ -17,32 +20,34 @@ export const GET = withRole('admin', async (_req: NextRequest) => { * * Body: { id: string } */ -export const POST = withRole('admin', async (req: NextRequest) => { - let body: unknown; - try { - body = await req.json(); - } catch { - return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 }); - } +export const POST = withGitHubWebhookAuth( + withRole('admin', async (req: NextRequest) => { + let body: unknown; + try { + body = await req.json(); + } catch { + return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 }); + } - const id = (body as Record)?.id; - if (!id || typeof id !== 'string') { - return NextResponse.json({ error: 'Missing required field: id' }, { status: 400 }); - } + const id = (body as Record)?.id; + if (!id || typeof id !== 'string') { + return NextResponse.json({ error: 'Missing required field: id' }, { status: 400 }); + } - const entry = webhookDLQ.get(id); - if (!entry) { - return NextResponse.json({ error: 'DLQ entry not found' }, { status: 404 }); - } + const entry = webhookDLQ.get(id); + if (!entry) { + return NextResponse.json({ error: 'DLQ entry not found' }, { status: 404 }); + } - const result = await webhookDLQ.reprocess(id); + const result = await webhookDLQ.reprocess(id); - if (!result.success) { - return NextResponse.json( - { error: result.error, entry: webhookDLQ.get(id) }, - { status: 422 } - ); - } + if (!result.success) { + return NextResponse.json( + { error: result.error, entry: webhookDLQ.get(id) }, + { status: 422 } + ); + } - return NextResponse.json({ success: true, entry: webhookDLQ.get(id) }); -}); + return NextResponse.json({ success: true, entry: webhookDLQ.get(id) }); + }) +); diff --git a/apps/backend/src/app/api/admin/webhooks/replay/route.ts b/apps/backend/src/app/api/admin/webhooks/replay/route.ts index 71b3b250..eadc7152 100644 --- a/apps/backend/src/app/api/admin/webhooks/replay/route.ts +++ b/apps/backend/src/app/api/admin/webhooks/replay/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { createLogger, resolveCorrelationId, CORRELATION_ID_HEADER } from '@/lib/api/logger'; import { webhookDeliveryService } from '@/services/webhook-delivery.service'; import { githubDeliveryFetcherService } from '@/services/github-delivery-fetcher.service'; +import { withGitHubWebhookAuth } from '@/lib/github/github-webhook'; /** * GET /api/admin/webhooks/replay @@ -15,7 +16,7 @@ import { githubDeliveryFetcherService } from '@/services/github-delivery-fetcher * - deliveries: Array of deliveries that can be replayed * - count: Total number of deliveries available for replay */ -export async function GET(req: NextRequest) { +export const GET = withGitHubWebhookAuth(async (req: NextRequest) => { const correlationId = resolveCorrelationId(req); const log = createLogger({ correlationId, service: 'webhook-replay-admin' }); @@ -64,7 +65,7 @@ export async function GET(req: NextRequest) { { status: 500 } ); } -} +}); /** * POST /api/admin/webhooks/replay @@ -81,7 +82,7 @@ export async function GET(req: NextRequest) { * - replayed: number - Count of deliveries replayed * - errors: Array of errors encountered during replay */ -export async function POST(req: NextRequest) { +export const POST = withGitHubWebhookAuth(async (req: NextRequest) => { const correlationId = resolveCorrelationId(req); const log = createLogger({ correlationId, service: 'webhook-replay-admin' }); @@ -154,7 +155,6 @@ export async function POST(req: NextRequest) { let replayedCount = 0; for (const delivery of deliveries) { - // For missed deliveries, we need to fetch the full payload from GitHub if (delivery.source === 'missed') { if (!hookId) { errors.push({ @@ -164,9 +164,6 @@ export async function POST(req: NextRequest) { continue; } - // Fetch delivery detail from GitHub - // Note: GitHub API uses numeric delivery ID, but we store GUID - // This is a limitation - we'd need to store the numeric ID as well log.warn('Missed delivery replay not fully implemented', { deliveryId: delivery.deliveryId, reason: 'Need numeric delivery ID for GitHub API', @@ -178,7 +175,6 @@ export async function POST(req: NextRequest) { continue; } - // Replay failed delivery const result = await webhookDeliveryService.replayDelivery(delivery.deliveryId); if (result.success) { @@ -215,18 +211,12 @@ export async function POST(req: NextRequest) { return res; } - return NextResponse.json( - { error: 'Invalid request' }, - { status: 400 } - ); + return NextResponse.json({ error: 'Invalid request' }, { status: 400 }); } catch (error: any) { log.error('Unexpected error during webhook replay', error); - return NextResponse.json( - { error: 'Internal server error' }, - { status: 500 } - ); + return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); } -} +}); /** * PUT /api/admin/webhooks/detect-missed @@ -242,7 +232,7 @@ export async function POST(req: NextRequest) { * - success: boolean * - missedCount: number - Count of missed deliveries detected */ -export async function PUT(req: NextRequest) { +export const PUT = withGitHubWebhookAuth(async (req: NextRequest) => { const correlationId = resolveCorrelationId(req); const log = createLogger({ correlationId, service: 'webhook-missed-detection' }); @@ -251,10 +241,7 @@ export async function PUT(req: NextRequest) { const { hookId, lookbackHours = 24 } = body; if (!hookId) { - return NextResponse.json( - { error: 'hookId is required' }, - { status: 400 } - ); + return NextResponse.json({ error: 'hookId is required' }, { status: 400 }); } log.info('Detecting missed deliveries', { hookId, lookbackHours }); @@ -284,9 +271,6 @@ export async function PUT(req: NextRequest) { return res; } catch (error: any) { log.error('Unexpected error detecting missed deliveries', error); - return NextResponse.json( - { error: 'Internal server error' }, - { status: 500 } - ); + return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); } -} +}); diff --git a/apps/backend/src/lib/github/github-webhook.ts b/apps/backend/src/lib/github/github-webhook.ts new file mode 100644 index 00000000..18393e69 --- /dev/null +++ b/apps/backend/src/lib/github/github-webhook.ts @@ -0,0 +1,83 @@ +/** + * GitHub App Webhook Authentication Middleware + * + * Wraps route handlers with HMAC-SHA256 signature verification for all + * /api/admin/webhooks/ routes. Secret is read from GITHUB_WEBHOOK_SECRET env var. + * + * On failure: returns 401 with WWW-Authenticate: HMAC, logs source IP. + * On success: calls the handler. + */ + +import { NextRequest, NextResponse } from 'next/server'; +import { createLogger } from '@/lib/api/logger'; +import { verifyGitHubWebhookSignature } from './webhook-verification'; + +const log = createLogger({ service: 'github-webhook-auth' }); + +/** + * Timing-safe HMAC-SHA256 verification — thin wrapper over + * verifyGitHubWebhookSignature for consumers that prefer the shorter name. + */ +export function verifyGitHubSignature( + payload: string, + signature: string | null, + secret: string +): boolean { + return verifyGitHubWebhookSignature(payload, signature, secret); +} + +type RouteHandler = (req: NextRequest, ctx?: any) => Promise; + +/** + * Middleware that verifies the X-Hub-Signature-256 header before calling the + * wrapped handler. + * + * - Reads the webhook secret from GITHUB_WEBHOOK_SECRET env var. + * - Returns 401 + WWW-Authenticate: HMAC on missing or invalid signature. + * - Logs signature failures with the source IP (x-forwarded-for or remote addr). + */ +export function withGitHubWebhookAuth(handler: RouteHandler): RouteHandler { + return async (req: NextRequest, ctx?: any): Promise => { + const secret = process.env.GITHUB_WEBHOOK_SECRET; + if (!secret) { + log.warn('GITHUB_WEBHOOK_SECRET is not configured; rejecting request'); + return _unauthorized(); + } + + const signature = req.headers.get('x-hub-signature-256'); + const sourceIp = + req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? 'unknown'; + + let payload: string; + try { + payload = await req.text(); + } catch { + return _unauthorized(); + } + + if (!verifyGitHubSignature(payload, signature, secret)) { + log.warn('GitHub webhook signature verification failed', { sourceIp }); + return _unauthorized(); + } + + // Re-construct a request with the already-consumed body so the handler + // can call req.json() if it needs to. + const cloned = new Request(req.url, { + method: req.method, + headers: req.headers, + body: payload, + }); + + return handler(new NextRequest(cloned), ctx); + }; +} + +function _unauthorized(): NextResponse { + return NextResponse.json( + { error: 'Unauthorized: invalid or missing webhook signature' }, + { + status: 401, + headers: { 'WWW-Authenticate': 'HMAC' }, + } + ); +} diff --git a/apps/backend/tests/admin/webhooks/signature-verification.test.ts b/apps/backend/tests/admin/webhooks/signature-verification.test.ts new file mode 100644 index 00000000..b0b3b418 --- /dev/null +++ b/apps/backend/tests/admin/webhooks/signature-verification.test.ts @@ -0,0 +1,156 @@ +// @vitest-environment node +/** + * Tests for GitHub App webhook signature verification (#765) + * + * Covers: + * - verifyGitHubSignature: valid, invalid, missing header + * - withGitHubWebhookAuth middleware: 401 on missing/invalid sig, passes through on valid + * - Timing-safe comparison (no early exit on length mismatch) + * - WWW-Authenticate: HMAC header on failure + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { NextRequest, NextResponse } from 'next/server'; +import crypto from 'crypto'; + +// ── Mocks ───────────────────────────────────────────────────────────────────── + +vi.mock('@/lib/api/logger', () => ({ + createLogger: () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + audit: vi.fn(), + }), +})); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +const SECRET = 'test-webhook-secret'; + +function sign(payload: string, secret = SECRET): string { + const hmac = crypto.createHmac('sha256', secret); + hmac.update(payload, 'utf8'); + return `sha256=${hmac.digest('hex')}`; +} + +function makeRequest(payload: string, signature?: string | null): NextRequest { + const headers: Record = { 'content-type': 'application/json' }; + if (signature !== undefined && signature !== null) { + headers['x-hub-signature-256'] = signature; + } + return new NextRequest('http://localhost/api/admin/webhooks/test', { + method: 'POST', + headers, + body: payload, + }); +} + +// ── verifyGitHubSignature ───────────────────────────────────────────────────── + +describe('verifyGitHubSignature', () => { + // Dynamically import to allow env changes per test + const load = async () => { + const mod = await import('@/lib/github/github-webhook'); + return mod.verifyGitHubSignature; + }; + + it('returns true for a valid signature', async () => { + const verifyGitHubSignature = await load(); + const payload = '{"action":"push"}'; + expect(verifyGitHubSignature(payload, sign(payload), SECRET)).toBe(true); + }); + + it('returns false for an invalid signature', async () => { + const verifyGitHubSignature = await load(); + const payload = '{"action":"push"}'; + expect(verifyGitHubSignature(payload, sign(payload, 'wrong-secret'), SECRET)).toBe(false); + }); + + it('returns false for a null (missing) signature', async () => { + const verifyGitHubSignature = await load(); + expect(verifyGitHubSignature('payload', null, SECRET)).toBe(false); + }); + + it('returns false for a signature without sha256= prefix', async () => { + const verifyGitHubSignature = await load(); + expect(verifyGitHubSignature('payload', 'deadbeef', SECRET)).toBe(false); + }); + + it('returns false for a tampered payload', async () => { + const verifyGitHubSignature = await load(); + const original = '{"action":"push"}'; + const sig = sign(original); + expect(verifyGitHubSignature('{"action":"tampered"}', sig, SECRET)).toBe(false); + }); + + it('uses timing-safe comparison (does not throw on length mismatch)', async () => { + const verifyGitHubSignature = await load(); + // Short signature — should return false, never throw + expect(() => + verifyGitHubSignature('payload', 'sha256=short', SECRET) + ).not.toThrow(); + expect(verifyGitHubSignature('payload', 'sha256=short', SECRET)).toBe(false); + }); +}); + +// ── withGitHubWebhookAuth middleware ────────────────────────────────────────── + +describe('withGitHubWebhookAuth', () => { + const ORIGINAL_ENV = { ...process.env }; + + beforeEach(() => { + process.env.GITHUB_WEBHOOK_SECRET = SECRET; + vi.resetModules(); + }); + + afterEach(() => { + process.env = { ...ORIGINAL_ENV }; + vi.resetModules(); + }); + + async function load() { + const mod = await import('@/lib/github/github-webhook'); + return mod.withGitHubWebhookAuth; + } + + it('calls the handler when signature is valid', async () => { + const withGitHubWebhookAuth = await load(); + const payload = '{"action":"push"}'; + const handler = vi.fn().mockResolvedValue(NextResponse.json({ ok: true })); + const wrapped = withGitHubWebhookAuth(handler); + const res = await wrapped(makeRequest(payload, sign(payload))); + expect(res.status).toBe(200); + expect(handler).toHaveBeenCalledOnce(); + }); + + it('returns 401 with WWW-Authenticate: HMAC when signature is missing', async () => { + const withGitHubWebhookAuth = await load(); + const handler = vi.fn().mockResolvedValue(NextResponse.json({ ok: true })); + const wrapped = withGitHubWebhookAuth(handler); + const res = await wrapped(makeRequest('payload', null)); + expect(res.status).toBe(401); + expect(res.headers.get('www-authenticate')).toBe('HMAC'); + expect(handler).not.toHaveBeenCalled(); + }); + + it('returns 401 with WWW-Authenticate: HMAC when signature is invalid', async () => { + const withGitHubWebhookAuth = await load(); + const handler = vi.fn().mockResolvedValue(NextResponse.json({ ok: true })); + const wrapped = withGitHubWebhookAuth(handler); + const res = await wrapped(makeRequest('payload', 'sha256=badsignature')); + expect(res.status).toBe(401); + expect(res.headers.get('www-authenticate')).toBe('HMAC'); + expect(handler).not.toHaveBeenCalled(); + }); + + it('returns 401 when GITHUB_WEBHOOK_SECRET is not set', async () => { + delete process.env.GITHUB_WEBHOOK_SECRET; + const withGitHubWebhookAuth = await load(); + const handler = vi.fn().mockResolvedValue(NextResponse.json({ ok: true })); + const wrapped = withGitHubWebhookAuth(handler); + const res = await wrapped(makeRequest('payload', sign('payload'))); + expect(res.status).toBe(401); + expect(handler).not.toHaveBeenCalled(); + }); +});