Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions docs/SECRETS.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,12 @@ Content-Type: application/json
*Response (200 OK):*
```json
{
"webhookId": "wh_9876543210abcdef",
"newSecret": "d9f8e7d6c5b4a392817263544536271809a8b7c6d5e4f3a2b1c0d9e8f7a6b5c4",
"rotatedAt": "2026-07-24T00:30:00.000Z",
"previousSecretExpiresAt": "2026-07-25T00:30:00.000Z"
"success": true,
"data": {
"webhookId": "wh_9876543210abcdef",
"newSecret": "d9f8e7d6c5b4a392817263544536271809a8b7c6d5e4f3a2b1c0d9e8f7a6b5c4",
"rotatedAt": "2026-07-24T00:30:00.000Z",
"previousSecretExpiresAt": "2026-07-25T00:30:00.000Z"
}
}
```
12 changes: 12 additions & 0 deletions src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ import { sunsetHeaderMiddleware } from "./middleware/sunsetHeader.js";
import { createOutboxAdminRouter } from "./routes/admin/outbox.js";
import { structuredLoggingMiddleware } from "./middleware/structuredLogging.js";
import { createWebhookReplayRouter } from "./routes/webhookReplay.js";
import { createWebhookRouter } from "./routes/webhooks.js";
import { PostgresWebhookRepository } from "./db/repositories/webhookRepository.js";
import { auditLogService } from "./services/audit/index.js";
import { createExportRouter } from "./routes/export/index.js";

const app = express();
Expand Down Expand Up @@ -257,6 +260,15 @@ app.use("/api/admin/outbox", createOutboxAdminRouter());
// Webhook DLQ replay — GET /api/webhooks/dlq, POST /api/webhooks/dlq/:id/replay
app.use("/api/webhooks/dlq", createWebhookReplayRouter(pool));

// Webhook signing-secret rotation — POST /api/webhooks/:webhookId/rotate-secret
// (audited, safe-rollout dual-secret grace period — see docs/SECRETS.md and
// docs/WEBHOOK_SIGNING.md). Mounted after /api/webhooks/dlq so it never
// intercepts that router's requests.
app.use(
"/api/webhooks",
createWebhookRouter(new PostgresWebhookRepository(pool), auditLogService),
);

app.use("/api/orgs/:orgId/policies", createPolicyRouter());

const analyticsThresholdSeconds = Number(
Expand Down
107 changes: 107 additions & 0 deletions src/db/repositories/webhookRepository.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { describe, it, expect, beforeEach } from 'vitest'
import { newDb } from 'pg-mem'
import type { Pool } from 'pg'
import { PostgresWebhookRepository } from './webhookRepository.js'
import type { WebhookConfig } from '../../services/webhooks/types.js'

/**
* Exercises PostgresWebhookRepository against a real (in-memory) Postgres so
* that schema drift between this repository's SQL and the migrations that
* actually create `webhook_configs` gets caught here instead of only at
* runtime against a real database. This is the schema exactly as it exists
* after migrations 005, 007, 009, and 032 have all run.
*/
async function buildTestDb(): Promise<Pool> {
const db = newDb()

db.public.registerFunction({
name: 'gen_random_uuid',
returns: 'uuid',
implementation: () => crypto.randomUUID(),
} as Parameters<typeof db.public.registerFunction>[0])

const adapter = db.adapters.createPg()
const pool = new adapter.Pool() as unknown as Pool

await pool.query(`
CREATE TABLE IF NOT EXISTS webhook_configs (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
url TEXT NOT NULL,
secret TEXT NOT NULL,
previous_secret VARCHAR(255),
previous_secret_expires_at TIMESTAMPTZ,
secret_updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
active BOOLEAN NOT NULL DEFAULT TRUE,
events TEXT[] NOT NULL,
timeout_ms INTEGER NOT NULL DEFAULT 5000,
max_attempts INTEGER NOT NULL DEFAULT 3,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);
`)

return pool
}

const SEED: WebhookConfig = {
id: '',
url: 'https://example.com/hook',
events: ['bond.created'],
secret: 'initial-secret',
secretUpdatedAt: new Date(),
active: true,
}

describe('PostgresWebhookRepository', () => {
let pool: Pool
let repo: PostgresWebhookRepository
let webhookId: string

beforeEach(async () => {
pool = await buildTestDb()
repo = new PostgresWebhookRepository(pool)
webhookId = crypto.randomUUID()
await repo.set({ ...SEED, id: webhookId })
})

it('rotateSecret() atomically swaps the secret and persists previousSecretExpiresAt', async () => {
const expiresAt = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString()

const updated = await repo.rotateSecret(webhookId, 'brand-new-secret', 'initial-secret', expiresAt)

expect(updated.secret).toBe('brand-new-secret')
expect(updated.previousSecret).toBe('initial-secret')
expect(updated.previousSecretExpiresAt).toBeTruthy()
expect(new Date(updated.previousSecretExpiresAt!).toISOString()).toBe(new Date(expiresAt).toISOString())
})

it('rotateSecret() throws when the webhook does not exist', async () => {
const nonExistentId = crypto.randomUUID()
await expect(
repo.rotateSecret(nonExistentId, 'new-secret', 'old-secret', new Date().toISOString()),
).rejects.toThrow('Webhook not found')
})

it('get() reflects the rotated secret and expiry after rotation', async () => {
const expiresAt = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString()
await repo.rotateSecret(webhookId, 'rotated-secret', 'initial-secret', expiresAt)

const fetched = await repo.get(webhookId)

expect(fetched).not.toBeNull()
expect(fetched!.secret).toBe('rotated-secret')
expect(fetched!.previousSecret).toBe('initial-secret')
expect(fetched!.previousSecretExpiresAt).toBeTruthy()
})

it('a second rotation replaces previousSecret rather than accumulating history', async () => {
const firstExpiry = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString()
await repo.rotateSecret(webhookId, 'second-secret', 'initial-secret', firstExpiry)

const secondExpiry = new Date(Date.now() + 48 * 60 * 60 * 1000).toISOString()
const updated = await repo.rotateSecret(webhookId, 'third-secret', 'second-secret', secondExpiry)

expect(updated.secret).toBe('third-secret')
expect(updated.previousSecret).toBe('second-secret')
})
})
24 changes: 24 additions & 0 deletions src/migrations/032_add_webhook_previous_secret_expires_at.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { MigrationBuilder } from 'node-pg-migrate'

/**
* Migration: add webhook_configs.previous_secret_expires_at
*
* `PostgresWebhookRepository.rotateSecret()` (src/db/repositories/webhookRepository.ts)
* and `WebhookRotationService` (src/services/webhooks/rotationService.ts) have relied
* on this column since the safe-rollout secret rotation endpoint was written, but no
* migration ever created it — rotation against a real Postgres-backed store would fail
* with "column previous_secret_expires_at does not exist". This column stores the
* explicit ISO timestamp (set at rotation time, 24h TTL) after which `previous_secret`
* should no longer be accepted for signature verification.
*
* Nullable, no default: existing rows have no previous secret to expire.
*/
export async function up(pgm: MigrationBuilder): Promise<void> {
pgm.addColumns('webhook_configs', {
previous_secret_expires_at: { type: 'timestamptz', notNull: false },
})
}

export async function down(pgm: MigrationBuilder): Promise<void> {
pgm.dropColumns('webhook_configs', ['previous_secret_expires_at'])
}
22 changes: 20 additions & 2 deletions src/routes/webhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export function createWebhookRouter(store: WebhookStore, audit: AuditLogService)
webhookId,
actor.id,
actor.email,
actor.tenantId,
ipAddress,
)

Expand All @@ -87,11 +88,28 @@ export function createWebhookRouter(store: WebhookStore, audit: AuditLogService)
})
} catch (err) {
if (err instanceof WebhookNotFoundError) {
sendError(res, ErrorCode.NOT_FOUND, err.message)
res.status(404).json({
error: 'NotFound',
code: 'WebhookNotFound',
message: err.message,
})
return
}

sendError(res, ErrorCode.INTERNAL_SERVER_ERROR, err instanceof Error ? err.message : 'Unknown error')
const message = err instanceof Error ? err.message : 'Unknown error'
void audit.logAction({
tenantId: (req as AuthenticatedRequest).user?.tenantId ?? tenantId,
actorId: (req as AuthenticatedRequest).user?.id ?? 'unknown',
actorEmail: (req as AuthenticatedRequest).user?.email ?? 'unknown',
action: AuditAction.ROTATE_WEBHOOK_SECRET,
resourceType: 'webhook',
resourceId: req.params.webhookId,
details: { webhookId: req.params.webhookId, reason: 'unexpected_error' },
status: 'failure',
errorMessage: message,
ipAddress: req.ip ?? req.socket.remoteAddress,
})
sendError(res, ErrorCode.INTERNAL_SERVER_ERROR, message)
} finally {
// Restore original tenant context
if (!originalTenant) {
Expand Down
19 changes: 10 additions & 9 deletions src/services/webhooks/__tests__/rotation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ async function buildTestDb(): Promise<{ db: IMemoryDb; pool: Pool }> {
const db = newDb();
db.public.none(`
CREATE TABLE webhook_configs (
id UUID PRIMARY KEY,
url TEXT NOT NULL,
secret TEXT NOT NULL,
previous_secret TEXT,
secret_updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
active BOOLEAN NOT NULL DEFAULT TRUE,
events TEXT[] NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
id UUID PRIMARY KEY,
url TEXT NOT NULL,
secret TEXT NOT NULL,
previous_secret TEXT,
previous_secret_expires_at TIMESTAMPTZ,
secret_updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
active BOOLEAN NOT NULL DEFAULT TRUE,
events TEXT[] NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);
`);
const adapter = db.adapters.createPg();
Expand Down
7 changes: 4 additions & 3 deletions src/services/webhooks/rotationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { AuditLogService } from '../audit/index.js'
import { AuditAction } from '../audit/index.js'

/** Grace period during which the previous secret remains valid for client verification. */
const PREVIOUS_SECRET_TTL_MS = 24 * 60 * 60 * 1000 // 24 hours
export const PREVIOUS_SECRET_TTL_MS = 24 * 60 * 60 * 1000 // 24 hours

export class WebhookNotFoundError extends Error {
constructor(webhookId: string) {
Expand All @@ -30,13 +30,14 @@ export class WebhookRotationService {
webhookId: string,
actorId: string,
actorEmail: string,
tenantId: string,
ipAddress?: string,
): Promise<WebhookSecretRotationResult> {
const webhook = await this.store.get(webhookId)

if (!webhook) {
void this.audit.logAction({
tenantId: 'tenant-unknown',
tenantId,
actorId,
actorEmail,
action: AuditAction.ROTATE_WEBHOOK_SECRET,
Expand All @@ -58,7 +59,7 @@ export class WebhookRotationService {
await this.store.rotateSecret(webhookId, newSecret, webhook.secret, previousSecretExpiresAt)

void this.audit.logAction({
tenantId: 'tenant-unknown',
tenantId,
actorId,
actorEmail,
action: AuditAction.ROTATE_WEBHOOK_SECRET,
Expand Down
50 changes: 50 additions & 0 deletions src/services/webhooks/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,56 @@ describe('WebhookService', () => {
expect(duration).toBeLessThan(200)
})

describe('rotateSecret', () => {
it('rotates via the atomic store.rotateSecret(), not read-modify-write via store.set()', async () => {
const service = new WebhookService(mockStore)

const updated = await service.rotateSecret('wh_1')

// The whole point of this method: a concurrent rotation of the same
// webhook elsewhere can't be silently clobbered by a stale
// read-then-set(), because there is no read-then-set at all.
expect(mockStore.rotateSecret).toHaveBeenCalledTimes(1)
expect(mockStore.set).not.toHaveBeenCalled()
expect(updated.secret).not.toBe('secret1')
expect(updated.previousSecret).toBe('secret1')
expect(updated.previousSecretExpiresAt).toBeTruthy()
})

it('computes previousSecretExpiresAt roughly 24h in the future', async () => {
const service = new WebhookService(mockStore)
const before = Date.now()

const updated = await service.rotateSecret('wh_2')

const expiresMs = new Date(updated.previousSecretExpiresAt!).getTime()
expect(expiresMs).toBeGreaterThan(before + 23 * 60 * 60 * 1000)
expect(expiresMs).toBeLessThan(before + 25 * 60 * 60 * 1000)
})

it('throws when the webhook does not exist', async () => {
const service = new WebhookService(mockStore)
await expect(service.rotateSecret('wh_nonexistent')).rejects.toThrow('Webhook not found')
})

it('audit-logs success with the actor tenant and the computed previousSecretExpiresAt', async () => {
const mockAudit = { logAction: vi.fn().mockResolvedValue(undefined) }
const actor = { id: 'admin_1', email: 'admin@example.com', tenantId: 'tenant_1' }
const service = new WebhookService(mockStore, undefined, undefined, mockAudit as any)

await service.rotateSecret('wh_1', actor, 'req-123')

expect(mockAudit.logAction).toHaveBeenCalledTimes(1)
const [tenantId, actorId, actorEmail, action, resourceId, , details] = mockAudit.logAction.mock.calls[0]
expect(tenantId).toBe('tenant_1')
expect(actorId).toBe('admin_1')
expect(actorEmail).toBe('admin@example.com')
expect(action).toBe('ROTATE_WEBHOOK_SECRET')
expect(resourceId).toBe('wh_1')
expect(details).toMatchObject({ previousSecretExpiresAt: expect.any(String) })
})
})

describe('replayWebhook', () => {
it('throws if DLQ is not configured', async () => {
const service = new WebhookService(mockStore)
Expand Down
22 changes: 13 additions & 9 deletions src/services/webhooks/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { randomBytes } from 'crypto'
import type { WebhookStore, WebhookEventType, WebhookPayload, WebhookDeliveryResult, WebhookConfig, DlqStore, WebhookEmitOptions } from './types.js'
import { deliverWebhook, type DeliveryOptions } from './delivery.js'
import { type AuditLogService, AuditAction } from '../audit/index.js'
import { PREVIOUS_SECRET_TTL_MS } from './rotationService.js'
import { buildDlqEntry } from './dlq.js'
import {
recordJobDeadLetter,
Expand All @@ -25,20 +26,23 @@ export class WebhookService {

/**
* Rotate a webhook's signing secret.
* Moves current secret to previousSecret and generates a new one.
*
* Uses the store's atomic `rotateSecret()` (a single UPDATE ... RETURNING)
* rather than read-modify-write via `get()` + `set()`, so a concurrent
* rotation of the same webhook (e.g. via the other rotation entry point,
* POST /api/webhooks/:webhookId/rotate-secret) can't silently clobber the
* secret written by this call with a stale in-memory copy.
*/
async rotateSecret(id: string, admin?: { id: string, email: string, tenantId: string }, requestId?: string): Promise<WebhookConfig> {
const webhook = await this.store.get(id)
if (!webhook) {
throw new Error('Webhook not found')
}

// Move current to previous and generate new
webhook.previousSecret = webhook.secret
webhook.secret = randomBytes(32).toString('hex')
webhook.secretUpdatedAt = new Date()
const newSecret = randomBytes(32).toString('hex')
const previousSecretExpiresAt = new Date(Date.now() + PREVIOUS_SECRET_TTL_MS).toISOString()

await this.store.set(webhook)
const updated = await this.store.rotateSecret(id, newSecret, webhook.secret, previousSecretExpiresAt)

if (this.auditLog && admin) {
this.auditLog.logAction(
Expand All @@ -47,16 +51,16 @@ export class WebhookService {
admin.email,
AuditAction.ROTATE_WEBHOOK_SECRET,
id,
webhook.url,
{ rotatedAt: webhook.secretUpdatedAt },
updated.url,
{ rotatedAt: updated.secretUpdatedAt, previousSecretExpiresAt },
undefined,
undefined,
undefined,
requestId
)
}

return webhook
return updated
}

/**
Expand Down
Loading
Loading