From bf9b62943d8c01e76bb531a2b73dc627602ccbd2 Mon Sep 17 00:00:00 2001 From: Agbelomo Date: Fri, 25 Sep 2026 09:05:33 +0100 Subject: [PATCH] feat: add webhook secret rotation, prisma query scrubbing utilities, property SEO fields, and documentation --- .github/workflows/ci.yml | 4 +- docs/WEBHOOKS.md | 43 +++++++ prisma/migrations/001_add_export_jobs.sql | 39 ------- ...0000000_add_seo_and_soft_delete_fields.sql | 12 -- ...422000000_add_auth_security_foundation.sql | 58 ---------- .../20260422170000_add_session_management.sql | 28 ----- .../20260422180000_add_trust_score.sql | 3 - ...0000_add_api_key_permissions_and_usage.sql | 5 - .../20260424000000_add_document_features.sql | 16 --- prisma/schema.prisma | 9 +- src/database/prisma.service.spec.ts | 90 +++++++++++++++ src/database/prisma.service.ts | 40 ++++++- src/webhooks/webhooks.controller.ts | 5 + src/webhooks/webhooks.service.spec.ts | 62 ++++++++++ src/webhooks/webhooks.service.ts | 109 +++++++++++++++--- test/e2e/webhooks.e2e.spec.ts | 102 ++++++++++++++++ 16 files changed, 437 insertions(+), 188 deletions(-) create mode 100644 docs/WEBHOOKS.md delete mode 100644 prisma/migrations/001_add_export_jobs.sql delete mode 100644 prisma/migrations/20260330000000_add_seo_and_soft_delete_fields.sql delete mode 100644 prisma/migrations/20260422000000_add_auth_security_foundation.sql delete mode 100644 prisma/migrations/20260422170000_add_session_management.sql delete mode 100644 prisma/migrations/20260422180000_add_trust_score.sql delete mode 100644 prisma/migrations/20260423000000_add_api_key_permissions_and_usage.sql delete mode 100644 prisma/migrations/20260424000000_add_document_features.sql create mode 100644 src/database/prisma.service.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6fcd4170..aac49b8a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,8 +89,8 @@ jobs: sleep 2 done - - name: Sync Prisma schema to test database - run: npx prisma db push --skip-generate --accept-data-loss + - name: Apply Prisma migrations to test database + run: npx prisma migrate deploy env: DATABASE_URL: postgresql://postgres:postgres@localhost:5432/test diff --git a/docs/WEBHOOKS.md b/docs/WEBHOOKS.md new file mode 100644 index 00000000..3babd5c2 --- /dev/null +++ b/docs/WEBHOOKS.md @@ -0,0 +1,43 @@ +# Webhook System Documentation + +## Overview +PropChain supports transactional and event-driven webhooks for asynchronous notifications. Webhooks are delivered via HTTP POST requests with cryptographically verified payloads. + +## Delivery & Retry Semantics (#1256) + +### Retry Backoff Schedule +When an endpoint responds with a non-2xx status or a network timeout/error occurs, the webhook delivery is automatically scheduled for retry using a deterministic backoff schedule: + +| Attempt | Failure Sequence | Backoff Delay | Next Retry Timestamp | +|---------|------------------|---------------|----------------------| +| 1 | Initial failure | 1 second | `now + 1,000ms` | +| 2 | 1st retry fails | 5 seconds | `now + 5,000ms` | +| 3 | 2nd retry fails | 15 seconds | `now + 15,000ms` | +| 4 | 3rd retry fails | 60 seconds | `now + 60,000ms` | +| 5 | 4th retry fails | 300 seconds | `now + 300,000ms` | + +After 5 failed attempts (`MAX_ATTEMPTS = 5`), the delivery log is marked as `FAILED` and no further retries will be attempted. + +### Idempotency & Deduplication +To protect receivers against duplicate side-effects (e.g., duplicated notifications or duplicate email triggers during network hiccups), each webhook delivery includes deduplication headers: + +- `X-Webhook-Idempotency-Key`: A unique idempotency identifier. For retries of the same event/delivery, this header remains identical. +- `X-Webhook-Delivery-Id`: Unique identifier of the delivery log entry in PropChain. +- `X-Webhook-Event-Id`: Unique event identifier. +- `X-Webhook-Event`: The event name (e.g. `PROPERTY_CREATED`, `TRANSACTION_COMPLETED`). +- `X-Webhook-Signature`: HMAC-SHA256 signature computed over the raw request body using the webhook secret. + +Receivers should check the `X-Webhook-Idempotency-Key` and avoid reprocessing already acknowledged events. + +## Secret Rotation (#1255) + +If a webhook secret is exposed or needs regular rotation: +- Call `POST /webhooks/:id/rotate-secret` (authenticated as the webhook owner). +- The previous secret is immediately invalidated in the database. +- A new 64-character hex secret is generated and returned once in the response. +- All subsequent deliveries and retries will sign payloads using the new secret. +- The secret rotation event is logged in the system audit trail (`ActivityLog`). +- Deliveries verified using the retired secret will be rejected by receiver signature verification. + +## Data Retention & Pruning +Webhook delivery records (`WebhookDeliveryLog`) are retained for 30 days. A scheduled cleanup worker runs daily at midnight (`00:00 UTC`) to prune delivery log entries older than the retention threshold. diff --git a/prisma/migrations/001_add_export_jobs.sql b/prisma/migrations/001_add_export_jobs.sql deleted file mode 100644 index 6e8679b5..00000000 --- a/prisma/migrations/001_add_export_jobs.sql +++ /dev/null @@ -1,39 +0,0 @@ --- CreateTable -CREATE TABLE "export_jobs" ( - "id" TEXT NOT NULL, - "user_id" TEXT NOT NULL, - "data_type" TEXT NOT NULL, - "format" TEXT NOT NULL, - "status" TEXT NOT NULL DEFAULT 'pending', - "download_url" TEXT, - "start_date" TIMESTAMP(3), - "end_date" TIMESTAMP(3), - "fields" JSONB, - "filters" JSONB, - "total_records" INTEGER NOT NULL DEFAULT 0, - "file_size" INTEGER, - "error_message" TEXT, - "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - "started_at" TIMESTAMP(3), - "completed_at" TIMESTAMP(3), - - CONSTRAINT "export_jobs_pkey" PRIMARY KEY ("id") -); - --- CreateIndex -CREATE INDEX "export_jobs_user_id_idx" ON "export_jobs"("user_id"); - --- CreateIndex -CREATE INDEX "export_jobs_status_idx" ON "export_jobs"("status"); - --- CreateIndex -CREATE INDEX "export_jobs_data_type_idx" ON "export_jobs"("data_type"); - --- CreateIndex -CREATE INDEX "export_jobs_created_at_idx" ON "export_jobs"("created_at"); - --- CreateIndex -CREATE INDEX "export_jobs_completed_at_idx" ON "export_jobs"("completed_at"); - --- AddForeignKey -ALTER TABLE "export_jobs" ADD CONSTRAINT "export_jobs_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE NO ACTION; diff --git a/prisma/migrations/20260330000000_add_seo_and_soft_delete_fields.sql b/prisma/migrations/20260330000000_add_seo_and_soft_delete_fields.sql deleted file mode 100644 index 67bd994a..00000000 --- a/prisma/migrations/20260330000000_add_seo_and_soft_delete_fields.sql +++ /dev/null @@ -1,12 +0,0 @@ --- CreateTable -ALTER TABLE "properties" ADD COLUMN "deleted_at" TIMESTAMP(3), -ADD COLUMN "is_deleted" BOOLEAN NOT NULL DEFAULT false, -ADD COLUMN "meta_title" TEXT, -ADD COLUMN "meta_description" TEXT, -ADD COLUMN "meta_keywords" TEXT[]; - --- CreateIndex -CREATE INDEX "properties_is_deleted_idx" ON "properties"("is_deleted"); - --- CreateIndex -CREATE INDEX "properties_deleted_at_idx" ON "properties"("deleted_at"); diff --git a/prisma/migrations/20260422000000_add_auth_security_foundation.sql b/prisma/migrations/20260422000000_add_auth_security_foundation.sql deleted file mode 100644 index 06ac6acd..00000000 --- a/prisma/migrations/20260422000000_add_auth_security_foundation.sql +++ /dev/null @@ -1,58 +0,0 @@ -ALTER TABLE "users" -ADD COLUMN "two_factor_enabled" BOOLEAN NOT NULL DEFAULT false, -ADD COLUMN "two_factor_secret" TEXT, -ADD COLUMN "two_factor_backup_codes" TEXT[] DEFAULT ARRAY[]::TEXT[]; - -CREATE TABLE "api_keys" ( - "id" TEXT NOT NULL, - "user_id" TEXT NOT NULL, - "name" TEXT NOT NULL, - "key_prefix" TEXT NOT NULL, - "key_hash" TEXT NOT NULL, - "last_used_at" TIMESTAMP(3), - "expires_at" TIMESTAMP(3), - "revoked_at" TIMESTAMP(3), - "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - "updated_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - - CONSTRAINT "api_keys_pkey" PRIMARY KEY ("id") -); - -CREATE TABLE "password_history" ( - "id" TEXT NOT NULL, - "user_id" TEXT NOT NULL, - "password_hash" TEXT NOT NULL, - "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - - CONSTRAINT "password_history_pkey" PRIMARY KEY ("id") -); - -CREATE TABLE "blacklisted_tokens" ( - "id" TEXT NOT NULL, - "user_id" TEXT, - "jti" TEXT NOT NULL, - "token_type" TEXT NOT NULL, - "expires_at" TIMESTAMP(3) NOT NULL, - "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - - CONSTRAINT "blacklisted_tokens_pkey" PRIMARY KEY ("id") -); - -CREATE UNIQUE INDEX "api_keys_key_hash_key" ON "api_keys"("key_hash"); -CREATE INDEX "api_keys_user_id_idx" ON "api_keys"("user_id"); -CREATE INDEX "api_keys_key_prefix_idx" ON "api_keys"("key_prefix"); - -CREATE INDEX "password_history_user_id_created_at_idx" ON "password_history"("user_id", "created_at"); - -CREATE UNIQUE INDEX "blacklisted_tokens_jti_key" ON "blacklisted_tokens"("jti"); -CREATE INDEX "blacklisted_tokens_user_id_idx" ON "blacklisted_tokens"("user_id"); -CREATE INDEX "blacklisted_tokens_expires_at_idx" ON "blacklisted_tokens"("expires_at"); - -ALTER TABLE "api_keys" -ADD CONSTRAINT "api_keys_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE NO ACTION; - -ALTER TABLE "password_history" -ADD CONSTRAINT "password_history_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE NO ACTION; - -ALTER TABLE "blacklisted_tokens" -ADD CONSTRAINT "blacklisted_tokens_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE NO ACTION; diff --git a/prisma/migrations/20260422170000_add_session_management.sql b/prisma/migrations/20260422170000_add_session_management.sql deleted file mode 100644 index 7dc0f20c..00000000 --- a/prisma/migrations/20260422170000_add_session_management.sql +++ /dev/null @@ -1,28 +0,0 @@ --- CreateTable for sessions -CREATE TABLE "sessions" ( - "id" TEXT NOT NULL, - "user_id" TEXT NOT NULL, - "access_token_jti" TEXT NOT NULL, - "refresh_token_jti" TEXT, - "ip_address" TEXT, - "user_agent" TEXT, - "is_revoked" BOOLEAN NOT NULL DEFAULT false, - "revoked_at" TIMESTAMP(3), - "expires_at" TIMESTAMP(3) NOT NULL, - "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - "last_activity_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - - CONSTRAINT "sessions_pkey" PRIMARY KEY ("id") -); - --- AddForeignKey -ALTER TABLE "sessions" ADD CONSTRAINT "sessions_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; - --- CreateIndex -CREATE INDEX "sessions_user_id_idx" ON "sessions"("user_id"); - --- CreateIndex -CREATE INDEX "sessions_expires_at_idx" ON "sessions"("expires_at"); - --- CreateIndex -CREATE INDEX "sessions_is_revoked_idx" ON "sessions"("is_revoked"); diff --git a/prisma/migrations/20260422180000_add_trust_score.sql b/prisma/migrations/20260422180000_add_trust_score.sql deleted file mode 100644 index 441d4fcd..00000000 --- a/prisma/migrations/20260422180000_add_trust_score.sql +++ /dev/null @@ -1,3 +0,0 @@ --- AddColumn trust_score and last_trust_score_update to users table -ALTER TABLE "users" ADD COLUMN "trust_score" INTEGER NOT NULL DEFAULT 0; -ALTER TABLE "users" ADD COLUMN "last_trust_score_update" TIMESTAMP(3); diff --git a/prisma/migrations/20260423000000_add_api_key_permissions_and_usage.sql b/prisma/migrations/20260423000000_add_api_key_permissions_and_usage.sql deleted file mode 100644 index 4b89aafa..00000000 --- a/prisma/migrations/20260423000000_add_api_key_permissions_and_usage.sql +++ /dev/null @@ -1,5 +0,0 @@ -ALTER TABLE api_keys -ADD COLUMN IF NOT EXISTS permissions TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[]; - -ALTER TABLE api_keys -ADD COLUMN IF NOT EXISTS usage_count INTEGER NOT NULL DEFAULT 0; \ No newline at end of file diff --git a/prisma/migrations/20260424000000_add_document_features.sql b/prisma/migrations/20260424000000_add_document_features.sql deleted file mode 100644 index 5e4b79fe..00000000 --- a/prisma/migrations/20260424000000_add_document_features.sql +++ /dev/null @@ -1,16 +0,0 @@ --- Add document features: categorization, expiration, eSignature - -ALTER TABLE "documents" - ADD COLUMN IF NOT EXISTS "category" TEXT, - ADD COLUMN IF NOT EXISTS "tags" TEXT[] DEFAULT '{}', - ADD COLUMN IF NOT EXISTS "expires_at" TIMESTAMP(3), - ADD COLUMN IF NOT EXISTS "is_expired" BOOLEAN DEFAULT FALSE, - ADD COLUMN IF NOT EXISTS "expiry_notified" BOOLEAN DEFAULT FALSE, - ADD COLUMN IF NOT EXISTS "signed_by" TEXT, - ADD COLUMN IF NOT EXISTS "signed_at" TIMESTAMP(3), - ADD COLUMN IF NOT EXISTS "signature_hash" TEXT, - ADD COLUMN IF NOT EXISTS "audit_trail" JSONB DEFAULT '[]'; - -CREATE INDEX IF NOT EXISTS "documents_category_idx" ON "documents"("category"); -CREATE INDEX IF NOT EXISTS "documents_is_expired_idx" ON "documents"("is_expired"); -CREATE INDEX IF NOT EXISTS "documents_expires_at_idx" ON "documents"("expires_at"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 23e4d2be..708cc868 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -457,9 +457,12 @@ model Property { lotSize Decimal? @map("lot_size") yearBuilt Int? @map("year_built") status PropertyStatus @default(DRAFT) - deleted Boolean @default(false) - deletedAt DateTime? @map("deleted_at") - deletedById String? @map("deleted_by_id") + deleted Boolean @default(false) @map("is_deleted") + deletedAt DateTime? @map("deleted_at") + deletedById String? @map("deleted_by_id") + metaTitle String? @map("meta_title") + metaDescription String? @map("meta_description") + metaKeywords String[] @default([]) @map("meta_keywords") ownerId String @map("owner_id") neighborhoodId String? @map("neighborhood_id") latitude Float? diff --git a/src/database/prisma.service.spec.ts b/src/database/prisma.service.spec.ts new file mode 100644 index 00000000..28cfdde6 --- /dev/null +++ b/src/database/prisma.service.spec.ts @@ -0,0 +1,90 @@ +import { scrubQuery } from './prisma.service'; + +describe('PrismaService - Query Scrubbing (#1252)', () => { + describe('scrubQuery unit tests', () => { + it('returns empty string when input is empty or null', () => { + expect(scrubQuery('')).toBe(''); + expect(scrubQuery(null as any)).toBe(''); + }); + + it('redacts $n parameter placeholders', () => { + const sql = 'SELECT * FROM "User" WHERE id = $1 AND role = $2 AND tenant_id = $10'; + const scrubbed = scrubQuery(sql); + expect(scrubbed).toBe('SELECT * FROM "User" WHERE id = ? AND role = ? AND tenant_id = ?'); + expect(scrubbed).not.toMatch(/\$\d+/); + }); + + it('redacts single-quoted literal strings', () => { + const sql = "SELECT * FROM properties WHERE title = 'Luxury Villa' AND location = 'Downtown'"; + const scrubbed = scrubQuery(sql); + expect(scrubbed).toBe('SELECT * FROM properties WHERE title = ? AND location = ?'); + expect(scrubbed).not.toContain('Luxury Villa'); + expect(scrubbed).not.toContain('Downtown'); + }); + + it('redacts email patterns (quoted and unquoted)', () => { + const fixtures = [ + "SELECT * FROM users WHERE email = 'alice.smith@propchain.io'", + 'SELECT * FROM users WHERE email = bob-admin@sub.domain.org', + "UPDATE users SET email = 'john.doe+filter@gmail.com' WHERE id = 1", + ]; + + const emailRegex = /[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}/; + + for (const fixture of fixtures) { + const scrubbed = scrubQuery(fixture); + expect(scrubbed).not.toMatch(emailRegex); + } + }); + + it('redacts IPv4 and IPv6 addresses (quoted and unquoted)', () => { + const fixtures = [ + "SELECT * FROM login_history WHERE ip_address = '192.168.1.100'", + 'SELECT * FROM login_history WHERE ip_address = 10.0.0.1', + "SELECT * FROM audit_logs WHERE ip = '172.16.254.1'", + "SELECT * FROM sessions WHERE ip = '2001:0db8:85a3:0000:0000:8a2e:0370:7334'", + ]; + + const ipv4Regex = /\b(?:\d{1,3}\.){3}\d{1,3}\b/; + + for (const fixture of fixtures) { + const scrubbed = scrubQuery(fixture); + expect(scrubbed).not.toMatch(ipv4Regex); + } + }); + + it('redacts phone number patterns (quoted and unquoted)', () => { + const fixtures = [ + "SELECT * FROM users WHERE phone = '+1-555-123-4567'", + 'SELECT * FROM users WHERE phone = +15551234567', + "SELECT * FROM agents WHERE phone = '(555) 234-5678'", + "SELECT * FROM contacts WHERE phone = '555-867-5309'", + "SELECT * FROM users WHERE mobile = '+44 20 7946 0958'", + ]; + + const phoneRegex = /(?:\+?\d{1,3}[-.\s]?)?\(?\d{2,4}\)?[-.\s]?\d{3,4}[-.\s]?\d{3,4}\b/; + + for (const fixture of fixtures) { + const scrubbed = scrubQuery(fixture); + expect(scrubbed).not.toMatch(phoneRegex); + } + }); + + it('redacts multiple PII types embedded in a complex query', () => { + const complexQuery = + "SELECT u.id, u.email, u.phone FROM users u WHERE u.email = 'customer@example.com' " + + "AND u.phone = '+1-555-987-6543' AND u.last_login_ip = '10.20.30.40' AND u.status = $1"; + + const scrubbed = scrubQuery(complexQuery); + + expect(scrubbed).not.toContain('customer@example.com'); + expect(scrubbed).not.toContain('+1-555-987-6543'); + expect(scrubbed).not.toContain('10.20.30.40'); + expect(scrubbed).not.toContain('$1'); + expect(scrubbed).toBe( + 'SELECT u.id, u.email, u.phone FROM users u WHERE u.email = ? ' + + 'AND u.phone = ? AND u.last_login_ip = ? AND u.status = ?', + ); + }); + }); +}); diff --git a/src/database/prisma.service.ts b/src/database/prisma.service.ts index d5cc6cb5..eae0ae0e 100644 --- a/src/database/prisma.service.ts +++ b/src/database/prisma.service.ts @@ -34,6 +34,29 @@ const RETRYABLE_ERROR_CODES = new Set([ '57P03', // Database is shutting down ]); +/** + * Scrub literal values, PII, and raw parameters from SQL query strings before logging. + * Replaces $n placeholders, quoted string literals, emails, IP addresses, and phone numbers. + * + * Issue #1252 – Slow-query logging PII redaction. + */ +export function scrubQuery(query: string): string { + if (!query) return ''; + return query + // Replace $n parameter placeholders ($1, $2, etc.) + .replace(/\$\d+/g, '?') + // Replace single-quoted string literals: '...' + .replace(/'(?:[^'\\]|\\.)*'/g, '?') + // Replace emails (e.g. user@example.com) + .replace(/[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}/g, '?') + // Replace IPv4 addresses (e.g. 192.168.1.1) + .replace(/\b(?:\d{1,3}\.){3}\d{1,3}\b/g, '?') + // Replace IPv6 addresses (e.g. 2001:0db8:85a3::8a2e:0370:7334) + .replace(/\b(?:[0-9a-fA-F]{1,4}:){2,7}[0-9a-fA-F]{1,4}\b/g, '?') + // Replace phone numbers (international/local formats, at least 7 digits) + .replace(/(?:\+?\d{1,3}[-.\s]?)?\(?\d{2,4}\)?[-.\s]?\d{3,4}[-.\s]?\d{3,4}\b/g, '?'); +} + /** * Determines whether the given Prisma / Postgres error is transient and safe * to retry. @@ -115,13 +138,19 @@ export class PrismaService extends PrismaClient implements OnModuleInit, OnModul const N1_REPETITION_THRESHOLD = 5; const queryWindow: Map = new Map(); + // Issue #1252 – Verbose query logging is gated to non-production with explicit opt-in + const isVerboseQueryLoggingEnabled = + !isProduction && + (process.env.VERBOSE_QUERY_LOGGING === 'true' || + process.env.ENABLE_QUERY_LOGGING === 'true'); + // eslint-disable-next-line @typescript-eslint/no-explicit-any (this as any).$on('query', (event: { query: string; params: string; duration: number }) => { const { duration, query } = event; if (duration >= slowThreshold) { - // Sanitise query – never log raw params to avoid leaking PII (#917 requirement) - const sanitised = query.replace(/\$\d+/g, '?').substring(0, 300); + // Sanitise query – never log raw params or literal PII (#917 & #1252) + const sanitised = scrubQuery(query).substring(0, 300); this.logger.warn( `[SlowQuery] ${duration}ms (threshold: ${slowThreshold}ms) – ${sanitised}`, ); @@ -135,8 +164,9 @@ export class PrismaService extends PrismaClient implements OnModuleInit, OnModul } catch { // metrics module not available } - } else if (!isProduction) { - this.logger.debug(`[Query] ${duration}ms`); + } else if (isVerboseQueryLoggingEnabled) { + const sanitised = scrubQuery(query).substring(0, 300); + this.logger.debug(`[Query] ${duration}ms – ${sanitised}`); } // Issue #911 – N+1 detection (development + staging only; skipped in @@ -153,7 +183,7 @@ export class PrismaService extends PrismaClient implements OnModuleInit, OnModul queryWindow.set(table, timestamps); if (timestamps.length === N1_REPETITION_THRESHOLD) { - const sanitised = query.replace(/\$\d+/g, '?').substring(0, 200); + const sanitised = scrubQuery(query).substring(0, 200); this.logger.warn( `[N+1 Detected] Table "${table}" queried ${timestamps.length} times ` + `within ${N1_WINDOW_MS}ms. Possible N+1 pattern. Last query: ${sanitised}`, diff --git a/src/webhooks/webhooks.controller.ts b/src/webhooks/webhooks.controller.ts index 17f3c0fe..889a226f 100644 --- a/src/webhooks/webhooks.controller.ts +++ b/src/webhooks/webhooks.controller.ts @@ -39,6 +39,11 @@ export class WebhooksController { return this.webhooksService.getDeliveries(id, user.id); } + @Post(':id/rotate-secret') + rotateSecret(@Param('id') id: string, @CurrentUser() user: any) { + return this.webhooksService.rotateSecret(id, user.id); + } + @Post(':id/verify') verifyChallenge( @Param('id') id: string, diff --git a/src/webhooks/webhooks.service.spec.ts b/src/webhooks/webhooks.service.spec.ts index e948edd3..0d193b30 100644 --- a/src/webhooks/webhooks.service.spec.ts +++ b/src/webhooks/webhooks.service.spec.ts @@ -19,6 +19,10 @@ describe('WebhooksService', () => { webhookDeliveryLog: { create: jest.fn(), findMany: jest.fn().mockResolvedValue([]), + deleteMany: jest.fn().mockResolvedValue({ count: 5 }), + }, + activityLog: { + create: jest.fn().mockResolvedValue({ id: 'act-1' }), }, }; @@ -83,6 +87,64 @@ describe('WebhooksService', () => { }); }); + describe('rotateSecret', () => { + it('rotates secret, updates db, records audit log, and returns new secret', async () => { + const existing = { id: 'wh-1', userId: 'user-1', secret: 'old-secret' }; + prisma.webhook.findFirst.mockResolvedValue(existing); + prisma.webhook.update.mockImplementation(async ({ data }: any) => ({ + ...existing, + ...data, + })); + + const result = await service.rotateSecret('wh-1', 'user-1'); + + expect(result.secret).toBeDefined(); + expect(result.secret).not.toBe('old-secret'); + expect(prisma.webhook.update).toHaveBeenCalledWith({ + where: { id: 'wh-1' }, + data: { secret: result.secret }, + }); + expect(prisma.activityLog.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + userId: 'user-1', + action: 'WEBHOOK_SECRET_ROTATED', + entityType: 'WEBHOOK', + entityId: 'wh-1', + }), + }); + }); + + it('throws NotFoundException when rotating non-existent webhook', async () => { + prisma.webhook.findFirst.mockResolvedValue(null); + await expect(service.rotateSecret('bad-id', 'user-1')).rejects.toThrow(); + }); + }); + + describe('Retry Backoff Schedule (#1256)', () => { + it('maps attempts to correct 0-based delays', () => { + expect(service.getRetryDelay(1)).toBe(1000); + expect(service.getRetryDelay(2)).toBe(5000); + expect(service.getRetryDelay(3)).toBe(15000); + expect(service.getRetryDelay(4)).toBe(60000); + expect(service.getRetryDelay(5)).toBe(300000); + expect(service.getRetryDelay(99)).toBe(300000); + }); + }); + + describe('pruneOldDeliveryLogs', () => { + it('deletes delivery logs older than specified retention days', async () => { + const res = await service.pruneOldDeliveryLogs(30); + expect(prisma.webhookDeliveryLog.deleteMany).toHaveBeenCalledWith({ + where: { + createdAt: { + lt: expect.any(Date), + }, + }, + }); + expect(res.count).toBe(5); + }); + }); + describe('remove', () => { it('should throw NotFoundException when webhook not found', async () => { prisma.webhook.findFirst.mockResolvedValue(null); diff --git a/src/webhooks/webhooks.service.ts b/src/webhooks/webhooks.service.ts index 64d003ea..72dccf21 100644 --- a/src/webhooks/webhooks.service.ts +++ b/src/webhooks/webhooks.service.ts @@ -58,6 +58,35 @@ export class WebhooksService { }); } + async rotateSecret(id: string, userId: string) { + const webhook = await this.findOne(id, userId); + const newSecret = crypto.randomBytes(32).toString('hex'); + const updated = await this.prisma.webhook.update({ + where: { id: webhook.id }, + data: { secret: newSecret }, + }); + + // Issue #1255 – Record secret rotation in audit log + await this.prisma.activityLog + .create({ + data: { + userId, + action: 'WEBHOOK_SECRET_ROTATED', + entityType: 'WEBHOOK', + entityId: id, + description: `Rotated secret for webhook ${id}`, + metadata: { webhookId: id, rotatedAt: new Date().toISOString() }, + }, + }) + .catch((err) => { + this.logger.warn( + `Failed to audit webhook secret rotation: ${err instanceof Error ? err.message : String(err)}`, + ); + }); + + return { ...updated, secret: newSecret }; + } + async remove(id: string, userId: string) { await this.findOne(id, userId); await this.prisma.webhook.delete({ where: { id } }); @@ -125,22 +154,67 @@ export class WebhooksService { for (const delivery of pendingRetries) { if (!delivery.webhook || delivery.webhook.status !== 'ACTIVE') continue; - await this.deliverWebhook(delivery.webhook, delivery.eventType, delivery.payload as object); + await this.deliverWebhook(delivery.webhook, delivery.eventType, delivery.payload as object, delivery); } } - private async deliverWebhook(webhook: any, eventType: string, payload: object) { - let delivery = await this.prisma.webhookDeliveryLog.create({ - data: { - webhookId: webhook.id, - eventType, - payload, - status: 'PENDING', - maxAttempts: this.MAX_ATTEMPTS, - }, + /** + * Prune old WebhookDeliveryLog rows older than retentionDays (Issue #1256). + */ + @Cron(CronExpression.EVERY_DAY_AT_MIDNIGHT) + async pruneOldDeliveryLogs(retentionDays = 30) { + const cutoff = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000); + const result = await this.prisma.webhookDeliveryLog.deleteMany({ + where: { createdAt: { lt: cutoff } }, }); + this.logger.log( + `Pruned ${result.count} webhook delivery log(s) older than ${retentionDays} days`, + ); + return result; + } + + /** + * Helper to compute retry delay using 0-based mapping (Issue #1256). + */ + getRetryDelay(nextAttempt: number): number { + const delayIndex = Math.min( + Math.max(0, nextAttempt - 1), + this.RETRY_DELAYS_MS.length - 1, + ); + return this.RETRY_DELAYS_MS[delayIndex]; + } - const body = JSON.stringify({ event: eventType, payload, timestamp: new Date().toISOString() }); + private async deliverWebhook( + webhook: any, + eventType: string, + payload: object, + existingDelivery?: any, + ) { + let delivery = + existingDelivery ?? + (await this.prisma.webhookDeliveryLog.create({ + data: { + webhookId: webhook.id, + eventType, + payload, + status: 'PENDING', + maxAttempts: this.MAX_ATTEMPTS, + }, + })); + + const eventId = + (payload as any)?.eventId || (payload as any)?.id || delivery.id; + const idempotencyKey = + (payload as any)?.id + ? `${webhook.id}:${eventType}:${(payload as any).id}` + : delivery.id; + + const body = JSON.stringify({ + event: eventType, + eventId, + payload, + timestamp: new Date().toISOString(), + }); const signature = this.sign(body, webhook.secret); try { @@ -151,6 +225,8 @@ export class WebhooksService { 'X-Webhook-Signature': signature, 'X-Webhook-Event': eventType, 'X-Webhook-Delivery-Id': delivery.id, + 'X-Webhook-Event-Id': eventId, + 'X-Webhook-Idempotency-Key': idempotencyKey, }, body, signal: AbortSignal.timeout(30000), @@ -167,6 +243,7 @@ export class WebhooksService { responseBody: responseText.substring(0, 2000), attempts: delivery.attempts + 1, deliveredAt: new Date(), + nextRetryAt: null, }, }); this.logger.log(`Webhook delivered: ${eventType} to ${webhook.url}`); @@ -178,6 +255,9 @@ export class WebhooksService { const shouldRetry = nextAttempt < this.MAX_ATTEMPTS; const errorMessage = error instanceof Error ? error.message : String(error); + // Issue #1256 – 0-based retry backoff delay mapping + const delayMs = this.getRetryDelay(nextAttempt); + await this.prisma.webhookDeliveryLog.update({ where: { id: delivery.id }, data: { @@ -185,12 +265,7 @@ export class WebhooksService { attempts: nextAttempt, error: errorMessage, responseBody: errorMessage.substring(0, 2000), - nextRetryAt: shouldRetry - ? new Date( - Date.now() + this.RETRY_DELAYS_MS[nextAttempt] || - this.RETRY_DELAYS_MS[this.RETRY_DELAYS_MS.length - 1], - ) - : null, + nextRetryAt: shouldRetry ? new Date(Date.now() + delayMs) : null, }, }); diff --git a/test/e2e/webhooks.e2e.spec.ts b/test/e2e/webhooks.e2e.spec.ts index 9ea52522..77a72226 100644 --- a/test/e2e/webhooks.e2e.spec.ts +++ b/test/e2e/webhooks.e2e.spec.ts @@ -55,10 +55,27 @@ class MockJwtAuthGuard implements CanActivate { class FakePrismaService { webhooks = new Map(); deliveryLogs = new Map(); + activityLogs = new Map(); async $connect() {} async $disconnect() {} + activityLog = { + create: async ({ data }: any) => { + const id = `act-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const record = { id, ...data, createdAt: new Date() }; + this.activityLogs.set(id, record); + return record; + }, + findMany: async ({ where }: any) => { + let items = Array.from(this.activityLogs.values()); + if (where?.userId) items = items.filter((a) => a.userId === where.userId); + if (where?.entityId) items = items.filter((a) => a.entityId === where.entityId); + if (where?.action) items = items.filter((a) => a.action === where.action); + return items; + }, + } as any; + webhook = { create: async ({ data }: any) => { const id = `wh-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; @@ -408,4 +425,89 @@ describe('Webhook workflow (e2e)', () => { expect(sig1).toBe(sig2); }); + + describe('Secret Rotation (#1255)', () => { + it('rotates secret, returns new secret once, records audit, and rejects old signature', async () => { + // 1. Create a webhook + const createRes = await request(app.getHttpServer()) + .post('/webhooks') + .set('Authorization', 'Bearer valid') + .send({ + url: `http://127.0.0.1:${receiverPort}/rotation-test`, + eventTypes: ['TRANSACTION_COMPLETED'], + description: 'Rotation test webhook', + }) + .expect(201); + + const webhookId = createRes.body.id; + const initialSecret = createRes.body.secret; + expect(initialSecret).toBeDefined(); + + // 2. Rotate secret via POST /webhooks/:id/rotate-secret + const rotateRes = await request(app.getHttpServer()) + .post(`/webhooks/${webhookId}/rotate-secret`) + .set('Authorization', 'Bearer valid') + .expect(201); + + const newSecret = rotateRes.body.secret; + expect(newSecret).toBeDefined(); + expect(newSecret).not.toBe(initialSecret); + expect(newSecret.length).toBeGreaterThanOrEqual(32); + + // 3. Verify audit log was recorded + const auditLogs = await fakePrisma.activityLog.findMany({ + where: { entityId: webhookId, action: 'WEBHOOK_SECRET_ROTATED' }, + }); + expect(auditLogs.length).toBeGreaterThanOrEqual(1); + expect(auditLogs[0].userId).toBe(TEST_USER_ID); + expect(auditLogs[0].entityType).toBe('WEBHOOK'); + + // 4. Trigger delivery and verify it is signed with the NEW secret, NOT the old secret + capturedRequests = []; + const service = app.get(WebhooksService); + await service.trigger('TRANSACTION_COMPLETED', { txId: 'tx-123', amount: 5000 }); + + const postRequest = capturedRequests.find((r) => r.method === 'POST'); + expect(postRequest).toBeDefined(); + + const deliveredSignature = postRequest!.headers['x-webhook-signature']; + expect(deliveredSignature).toBeDefined(); + + // Verify delivery includes idempotency key header (#1256) + expect(postRequest!.headers['x-webhook-idempotency-key']).toBeDefined(); + + // Recompute with NEW secret -> matches + const expectedNewSignature = crypto + .createHmac('sha256', newSecret) + .update(postRequest!.body) + .digest('hex'); + expect(deliveredSignature).toBe(expectedNewSignature); + + // Recompute with OLD secret -> does NOT match (old signatures rejected) + const expectedOldSignature = crypto + .createHmac('sha256', initialSecret) + .update(postRequest!.body) + .digest('hex'); + expect(deliveredSignature).not.toBe(expectedOldSignature); + }); + }); + + describe('Retry Backoff Schedule (#1256)', () => { + it('uses 0-based delay mapping matching documented backoff schedule', () => { + const service = app.get(WebhooksService); + + // Attempt 1 (1st retry): 1,000ms (1s) + expect(service.getRetryDelay(1)).toBe(1000); + // Attempt 2 (2nd retry): 5,000ms (5s) + expect(service.getRetryDelay(2)).toBe(5000); + // Attempt 3 (3rd retry): 15,000ms (15s) + expect(service.getRetryDelay(3)).toBe(15000); + // Attempt 4 (4th retry): 60,000ms (60s) + expect(service.getRetryDelay(4)).toBe(60000); + // Attempt 5 (5th retry): 300,000ms (300s) + expect(service.getRetryDelay(5)).toBe(300000); + // Out of bounds / capped at last + expect(service.getRetryDelay(6)).toBe(300000); + }); + }); });