Skip to content

feat(ops): Sprint 16 — Sentry + Mailgun webhooks#12

Merged
dobrodob merged 3 commits intomainfrom
feat/sprint-16-production-ops
Apr 19, 2026
Merged

feat(ops): Sprint 16 — Sentry + Mailgun webhooks#12
dobrodob merged 3 commits intomainfrom
feat/sprint-16-production-ops

Conversation

@dobrodob
Copy link
Copy Markdown
Member

@dobrodob dobrodob commented Apr 19, 2026

Summary

Two production-readiness features in one PR. Scheduled digest delivery (Sprint 11 follow-up) lands in Sprint 17 — its scope warrants its own focus.

Sentry observability

  • instrument.ts loaded FIRST in main.ts so @sentry/node can monkey-patch the runtime
  • No-op when SENTRY_DSN unset (dev default — boots without Sentry)
  • DomainExceptionFilter forwards 5xx only with request + user context
  • beforeSend strips cookie/auth headers + redacts secret-ish body keys
  • main.ts bootstrap.catch flushes Sentry (2s) before process.exit

Mailgun webhooks (email deliverability)

  • POST /webhooks/mailgun — HMAC-SHA256 signed, @public()
  • Four verification guardrails: signing-key-or-fail, 5-minute replay window, length check before timingSafeEqual, constant-time compare
  • Event actions: failed:permanent / complained / unsubscribed → auto-suppress; failed:temporary → no action; delivered/opened/clicked → ignored
  • Fan-out: suppression applies to all NewsletterSubscription rows for the email (community-wide, per-author, per-topic)

Tests (16 new, 439/439 total)

Mailgun:

  • Signature: tamper, wrong key, replay-too-old, missing field, non-numeric timestamp
  • Events: permanent failure, temporary failure, spam complaint, Mailgun unsub, all-rows fan-out, unknown recipient, already-suppressed, case-insensitive email, missing recipient, tracking events ignored

Sentry: pass-through to the SDK. Scrubbing logic in beforeSend is reviewable by eye; mocking Sentry.captureException would test the mock, not the integration.

Config

SENTRY_DSN=                 # empty = no-op, safe for dev
SENTRY_ENVIRONMENT=development
SENTRY_TRACES_SAMPLE_RATE=  # defaults: 1.0 dev, 0.1 prod

MAILGUN_WEBHOOK_SIGNING_KEY= # separate from API key

Pattern docs

  • 41-sentry-observability — init ordering, 5xx-only capture, secret scrubbing
  • 42-mailgun-webhooks — HMAC scheme, 4 guardrails, event decision tree

Test plan

  • npx tsc --noEmit clean
  • npx biome check . clean (pre-existing warnings unchanged)
  • npm run build:core + build:crdt clean
  • npx vitest run → 439/439 passing
  • CI green

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Sentry-based error reporting and request tracing with configurable sampling.
    • Added Mailgun webhook endpoint to verify signed events and automatically manage newsletter opt-outs.
  • Documentation

    • Added operational guides for Sentry observability and Mailgun webhook handling.
  • Tests

    • Added integration tests covering webhook signature verification and suppression behavior.

Two production-readiness features in one PR. Sprint 17 will land the
remaining scheduled-digest-cron piece (tracked separately — its
scope is bigger).

=== Sentry observability ===

- `apps/core/src/instrument.ts` — Sentry.init loaded BEFORE any other
  import in main.ts. @sentry/node v8+ monkey-patches require/http/fs
  at init; late init means untraced dependencies.
- Zero boot impact when SENTRY_DSN is empty (dev default) — SDK
  becomes a no-op, no warnings, no init failures.
- DomainExceptionFilter forwards 5xx-and-above to Sentry with
  request + user context (userId from req.user set by JwtAuthGuard).
  Client errors (4xx) stay out of Sentry by design — they're expected
  user behaviour, not alerting signal.
- `beforeSend` strips cookie/authorization/x-auth-token headers and
  redacts body keys matching /password|token|secret|otp|auth/i.
  Defense-in-depth: clients shouldn't be sending secrets in bodies
  but one misconfig away from Sentry holding access tokens.
- main.ts bootstrap.catch() now flushes Sentry (2s) before
  process.exit so prod boot crashes don't silently vanish.
- Config: SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_TRACES_SAMPLE_RATE
  (defaults: 1.0 dev, 0.1 prod).

=== Mailgun webhooks (deliverability) ===

- `POST /webhooks/mailgun` — @public() endpoint, HMAC-SHA256 signed.
- Signature scheme: hex(hmac-sha256(key, timestamp + token)) === signature
- Four guardrails on verification:
  1. Fail loud if signing key missing (400 every req, not silent accept)
  2. 5-minute replay window (+60s forward skew for clock drift)
  3. Length check BEFORE timingSafeEqual (throws on unequal length,
     leaks signature-length as oracle otherwise)
  4. timingSafeEqual on the hex buffers (constant-time compare)
- Events handled:
  - `failed severity=permanent` → suppress (hard bounce)
  - `failed severity=temporary` → log, no action (Mailgun retries)
  - `complained` → suppress (spam complaint)
  - `unsubscribed` → suppress (Mailgun-rendered unsub link click)
  - `delivered`/`opened`/`clicked` → ignored (tracking, future sprint)
- Suppression fans out across ALL NewsletterSubscription rows for the
  email — community-wide, per-author, per-topic all get
  unsubscribed=true. Sender-reputation damage is shared across
  destinations; consent withdrawal is too.
- Structured `{ ok, outcome: { action, recipient, event } }` response
  so Mailgun's webhook dashboard log makes incident triage easy.
- Config: MAILGUN_WEBHOOK_SIGNING_KEY (separate from API key).

=== Tests (16 new, 439/439 total) ===

MailgunWebhookService — all signature edge cases (tamper, wrong key,
replay, missing field, non-numeric timestamp) + every event type
(permanent/temporary failure, complaint, unsub, tracking-positive
ignored, unknown recipient, already-suppressed, case-insensitive
email, missing recipient).

Sentry integration isn't unit-tested — it's a pass-through to the
SDK with well-defined behaviour. The scrubbing logic in
`beforeSend` is tight enough to review by hand; adding mocks for
Sentry.captureException would test the mock, not the integration.

=== Pattern docs ===

- 41-sentry-observability — init ordering, 5xx-only capture,
  scrubbing, trade-offs
- 42-mailgun-webhooks — HMAC scheme, four guardrails, event
  decision tree, reuse checklist

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 19, 2026

Warning

Rate limit exceeded

@dobrodob has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 40 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 40 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: da8ca578-03e6-469a-88f8-23efc90d31c7

📥 Commits

Reviewing files that changed from the base of the PR and between 3d3b286 and 0af4c82.

📒 Files selected for processing (5)
  • apps/core/src/config/config.schema.ts
  • apps/core/src/instrument.ts
  • apps/core/src/modules/newsletter/dto/mailgun-webhook.dto.ts
  • apps/core/src/modules/newsletter/mailgun-webhook.controller.ts
  • apps/core/src/shared/errors/domain-exception.filter.ts
📝 Walkthrough

Walkthrough

Adds Sentry initialization and 5xx error forwarding with request/body scrubbing, plus a Mailgun HMAC-SHA256 webhook endpoint that verifies signatures, enforces replay protection, and suppresses newsletter subscriptions on permanent bounces/complaints/unsubscribes.

Changes

Cohort / File(s) Summary
Configuration & Env
\.env.example, apps/core/src/config/config.schema.ts
Added MAILGUN_WEBHOOK_SIGNING_KEY and SENTRY_TRACES_SAMPLE_RATE (optional, coerced to number, constrained 0–1).
Sentry Instrumentation
apps/core/src/instrument.ts, apps/core/src/main.ts, apps/core/src/shared/errors/domain-exception.filter.ts
New early-imported instrument.ts initializes @sentry/node with DSN/env/version and traces sample rate; beforeSend scrubs sensitive headers and redacts secret-like body fields. Bootstrap failure handler captures+flushes to Sentry before exit; DomainExceptionFilter forwards 5xx errors with request context and user id.
Mailgun Webhooks
apps/core/src/modules/newsletter/mailgun-webhook.controller.ts, apps/core/src/modules/newsletter/mailgun-webhook.service.ts, apps/core/src/modules/newsletter/newsletter.module.ts
New POST /webhooks/mailgun controller and service: verifies HMAC-SHA256 signature with timestamp/replay checks, uses timing-safe comparison and length check, decides suppression per event type, updates newsletter subscriptions via Prisma, and returns structured outcomes (suppressed/ignored/already-suppressed/not-found). Module registers controller/service.
Testing
apps/core/test/mailgun-webhook.integration.spec.ts
Integration tests covering signature verification edge cases, timestamp/replay protection, suppression logic across tenant/filters, case-insensitive email matching, and DB assertions.
Documentation
docs/discours-patterns/41-sentry-observability.md, docs/discours-patterns/42-mailgun-webhooks.md, docs/discours-patterns/README.md
New patterns documenting Sentry observability (init, scrubbing, 5xx-only capture) and Mailgun HMAC webhook handling with suppression rules; index updated.
Dependencies
package.json
Added @sentry/node (^10.49.0).

Sequence Diagram(s)

sequenceDiagram
    actor Mailgun
    participant Controller as MailgunWebhookController
    participant Service as MailgunWebhookService
    participant DB as PrismaDB

    Mailgun->>Controller: POST /webhooks/mailgun (payload + signature)
    Controller->>Service: verifySignature(signature)
    Service->>Service: load MAILGUN_WEBHOOK_SIGNING_KEY\nvalidate timestamp (replay/clock-skew)
    Service->>Service: compute HMAC-SHA256\nlength check + timingSafeEqual
    alt signature invalid
        Service-->>Controller: throw ValidationError
        Controller-->>Mailgun: 400 Bad Request
    else signature valid
        Controller->>Service: handleEvent(eventData)
        Service->>Service: determine action (suppress/ignore)
        alt suppress
            Service->>DB: updateMany(email, unsubscribed=true)
            DB-->>Service: updated count
        else ignore
            Service-->>Service: log outcome
        end
        Service-->>Controller: { ok: true, outcome }
        Controller-->>Mailgun: 200 OK
    end
Loading
sequenceDiagram
    participant Bootstrap as main.ts
    participant Instrument as instrument.ts
    participant Sentry as `@sentry/node`
    participant App as NestApp

    Bootstrap->>Instrument: import ./instrument (first)
    Instrument->>Sentry: init(dsn?, environment, tracesSampleRate, beforeSend)
    Sentry-->>Instrument: initialized (enabled or no-op)
    Bootstrap->>App: bootstrap NestJS
    alt bootstrap error
        App-->>Sentry: captureException(error)
        App->>Sentry: flush(2000ms)
        Sentry-->>App: flushed/timeout
        App->>App: process.exit(1)
    else running
        App->>DomainExceptionFilter: error (>=500)
        DomainExceptionFilter->>Sentry: withScope { setContext(request), setUser(id), captureException }
        Sentry-->>App: event queued
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped into code with a curious twitch,
HMAC shields and Sentry to stitch,
Scrubbed the secrets, stamped the time,
Bounces quieted—subscriptions fine.
A tiny rabbit, keeping mail snug and rich.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: introducing Sentry observability and Mailgun webhook handling for production operations in Sprint 16.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sprint-16-production-ops

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
docs/discours-patterns/41-sentry-observability.md (1)

41-54: Minor documentation inconsistency.

In the code snippet at line 51, environment is referenced but not defined within the snippet's scope. The actual implementation in instrument.ts defines environment on line 17 before using it. Consider adding the variable definition to the snippet for clarity, or referencing process.env.NODE_ENV directly.

📝 Suggested fix
 Sentry.init({
   dsn: process.env.SENTRY_DSN || undefined,  // no-op if empty (dev)
-  environment: process.env.SENTRY_ENVIRONMENT ?? process.env.NODE_ENV,
+  environment: process.env.SENTRY_ENVIRONMENT ?? process.env.NODE_ENV ?? 'development',
   release: process.env.APP_VERSION,
   tracesSampleRate:
     process.env.SENTRY_TRACES_SAMPLE_RATE !== undefined
       ? Number.parseFloat(process.env.SENTRY_TRACES_SAMPLE_RATE)
-      : environment === 'production' ? 0.1 : 1.0,
+      : (process.env.NODE_ENV === 'production') ? 0.1 : 1.0,
   beforeSend(event) { /* scrub */ return event; },
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/discours-patterns/41-sentry-observability.md` around lines 41 - 54, The
snippet passed to Sentry.init references the variable environment but doesn’t
define it; update the example so environment is defined (e.g., const environment
= process.env.SENTRY_ENVIRONMENT ?? process.env.NODE_ENV) before calling
Sentry.init, or change the Sentry.init call to reference process.env.NODE_ENV /
process.env.SENTRY_ENVIRONMENT directly; ensure the variable name matches the
one used in tracesSampleRate and environment fields to keep the snippet
consistent with instrument.ts and Sentry.init.
apps/core/test/mailgun-webhook.integration.spec.ts (1)

92-96: Minor edge case in tamper test.

The regex /[0-9a-f]/ replaces the first hex character with '0', but if the signature already starts with '0', this is a no-op and the test would incorrectly pass. Consider a more robust tampering approach.

🔧 Suggested fix
     it('rejects a request with a tampered signature', () => {
       const sig = makeValidSignature();
-      sig.signature = sig.signature.replace(/[0-9a-f]/, '0');
+      // XOR the first byte to guarantee a change
+      const firstChar = sig.signature[0];
+      const tampered = firstChar === '0' ? '1' : '0';
+      sig.signature = tampered + sig.signature.slice(1);
       expect(() => webhook.verifySignature(sig)).toThrow(ValidationError);
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core/test/mailgun-webhook.integration.spec.ts` around lines 92 - 96, The
tamper test can be a no-op if the replaced hex is already '0'; update the test
that uses makeValidSignature() and webhook.verifySignature to deterministically
alter sig.signature so it always changes (for example, inspect a specific
character like sig.signature[0] or last char and toggle it to a different hex
char — if it's '0' set to '1', otherwise set to '0' — or replace the last
character with a guaranteed different hex digit). Ensure the test still expects
verifySignature to throw ValidationError after this guaranteed modification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/core/src/config/config.schema.ts`:
- Line 152: SENTRY_TRACES_SAMPLE_RATE currently uses z.coerce.number() which
turns empty strings into 0; wrap the validator with the existing
emptyAsUndefined helper so empty env values become undefined and fall back to
defaults (use emptyAsUndefined(z.coerce.number().min(0).max(1).optional())),
keeping the min/max and optional constraints intact and referencing the
SENTRY_TRACES_SAMPLE_RATE symbol for the change.

In `@apps/core/src/instrument.ts`:
- Around line 43-47: The type error comes from assigning undefined to headers
typed as Record<string,string> in the event.request.headers scrub block; update
the scrub to set event.request.headers.cookie,
event.request.headers.authorization, and event.request.headers['x-auth-token']
to a string sentinel like '[REDACTED]' (instead of undefined) so it satisfies
the Sentry SDK types (alternatively, you may cast headers to any and delete the
keys, but prefer setting '[REDACTED]' for a minimal change).

In `@apps/core/src/modules/newsletter/mailgun-webhook.controller.ts`:
- Around line 33-38: The Mailgun webhook handler currently types the body as
MailgunWebhookPayload but lacks runtime checks, causing TypeError when payload
is malformed; change the controller parameter from `@Body`() payload:
MailgunWebhookPayload to `@Body`() payload: unknown and add a runtime guard that
verifies payload is an object with a non-empty signature property and an
"event-data" property before calling
this.webhook.verifySignature(payload.signature) and
this.webhook.handleEvent(payload['event-data']); if the check fails, throw
ValidationError('Mailgun webhook payload malformed', {}) to return a controlled
4xx response.

In `@apps/core/src/shared/errors/domain-exception.filter.ts`:
- Around line 66-69: Replace usages of request.url with request.path to avoid
sending query parameters to Sentry: update the object passed to
scope.setContext('request', { path: request.url, ... }) to use request.path, and
likewise update any logger calls in the DomainExceptionFilter that log
request.url (e.g., the this.logger.error / this.logger.warn calls inside the
filter) so they log request.path instead; search for request.url within the
DomainExceptionFilter class and change each occurrence to request.path to keep
telemetry consistent and avoid leaking query strings.

---

Nitpick comments:
In `@apps/core/test/mailgun-webhook.integration.spec.ts`:
- Around line 92-96: The tamper test can be a no-op if the replaced hex is
already '0'; update the test that uses makeValidSignature() and
webhook.verifySignature to deterministically alter sig.signature so it always
changes (for example, inspect a specific character like sig.signature[0] or last
char and toggle it to a different hex char — if it's '0' set to '1', otherwise
set to '0' — or replace the last character with a guaranteed different hex
digit). Ensure the test still expects verifySignature to throw ValidationError
after this guaranteed modification.

In `@docs/discours-patterns/41-sentry-observability.md`:
- Around line 41-54: The snippet passed to Sentry.init references the variable
environment but doesn’t define it; update the example so environment is defined
(e.g., const environment = process.env.SENTRY_ENVIRONMENT ??
process.env.NODE_ENV) before calling Sentry.init, or change the Sentry.init call
to reference process.env.NODE_ENV / process.env.SENTRY_ENVIRONMENT directly;
ensure the variable name matches the one used in tracesSampleRate and
environment fields to keep the snippet consistent with instrument.ts and
Sentry.init.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ed760a1-b1c6-4cee-be08-472bf3e2bf3a

📥 Commits

Reviewing files that changed from the base of the PR and between f025f26 and f24e8b3.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • .env.example
  • apps/core/src/config/config.schema.ts
  • apps/core/src/instrument.ts
  • apps/core/src/main.ts
  • apps/core/src/modules/newsletter/mailgun-webhook.controller.ts
  • apps/core/src/modules/newsletter/mailgun-webhook.service.ts
  • apps/core/src/modules/newsletter/newsletter.module.ts
  • apps/core/src/shared/errors/domain-exception.filter.ts
  • apps/core/test/mailgun-webhook.integration.spec.ts
  • docs/discours-patterns/41-sentry-observability.md
  • docs/discours-patterns/42-mailgun-webhooks.md
  • docs/discours-patterns/README.md
  • package.json

Comment thread apps/core/src/config/config.schema.ts Outdated
Comment thread apps/core/src/instrument.ts
Comment on lines +33 to +38
@Body() payload: MailgunWebhookPayload,
): Promise<{ ok: true; outcome: WebhookOutcome }> {
// Verify first — a malformed or forged event shouldn't even
// reach our subscription logic.
this.webhook.verifySignature(payload.signature);
const outcome = await this.webhook.handleEvent(payload['event-data']);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether runtime-validatable DTO classes exist for the webhook payload
# and whether controller currently relies on type-only payload annotations.

rg -n -C2 'type MailgunWebhookPayload|interface MailgunWebhookPayload|class .*Mailgun.*Payload' apps/core/src/modules/newsletter/mailgun-webhook.service.ts apps/core/src/modules/newsletter/mailgun-webhook.controller.ts
rg -n -C2 '@Body\\(\\) payload' apps/core/src/modules/newsletter/mailgun-webhook.controller.ts
rg -n -C2 'useGlobalPipes|ValidationPipe' apps/core/src/main.ts

Repository: Discours/publy-backend

Length of output: 1311


🏁 Script executed:

# Read the full definition of MailgunWebhookPayload and related interfaces
cat -n apps/core/src/modules/newsletter/mailgun-webhook.service.ts | head -80 | tail -50

Repository: Discours/publy-backend

Length of output: 2128


🏁 Script executed:

# Check if there are any class-validator decorators on the payload interface/class
rg -n '@Is|@Validate|class MailgunWebhookPayload' apps/core/src/modules/newsletter/mailgun-webhook.service.ts

Repository: Discours/publy-backend

Length of output: 48


🏁 Script executed:

# Check the full controller file to understand the endpoint and its decorators
cat -n apps/core/src/modules/newsletter/mailgun-webhook.controller.ts

Repository: Discours/publy-backend

Length of output: 1775


🏁 Script executed:

# Search for other webhook or `@Body`() patterns in controllers to see codebase conventions
fd -e controller.ts apps/core/src/modules | head -5 | xargs -I {} sh -c 'rg -l "@Body" {} && echo "---"'

Repository: Discours/publy-backend

Length of output: 455


🏁 Script executed:

# Check if MailgunWebhookPayload should have been defined as a class with validators
rg -B5 -A5 'export class.*DTO|export class.*Payload' apps/core/src/modules/newsletter/

Repository: Discours/publy-backend

Length of output: 48


🏁 Script executed:

# Look for other `@Body`() usages in controllers to understand codebase pattern
rg -B2 -A2 '@Body' apps/core/src/modules --type ts | head -100

Repository: Discours/publy-backend

Length of output: 8060


🏁 Script executed:

# Check ValidationPipe documentation or configuration for interface handling
rg -B3 -A3 'forbidUnknownValues|skipMissingProperties|transform' apps/core/src/main.ts

Repository: Discours/publy-backend

Length of output: 244


🏁 Script executed:

# Check if there's any custom validation or middleware for webhooks
rg -n 'webhook|Webhook' apps/core/src/main.ts

Repository: Discours/publy-backend

Length of output: 48


🏁 Script executed:

# Find ValidationError definition and its usage
rg -B3 -A3 'class ValidationError|export.*ValidationError' apps/core/src

Repository: Discours/publy-backend

Length of output: 597


🏁 Script executed:

# Check how body validation errors are handled in other controllers
rg -B2 -A2 'BadRequestException|throw.*Error.*payload|throw.*Error.*body' apps/core/src/modules --type ts | head -50

Repository: Discours/publy-backend

Length of output: 2915


🏁 Script executed:

# Verify if MailgunWebhookPayload should be a class with validators (check imports in service)
head -30 apps/core/src/modules/newsletter/mailgun-webhook.service.ts

Repository: Discours/publy-backend

Length of output: 1218


Add runtime payload guards to prevent 500s on malformed webhook bodies.

@Body() payload: MailgunWebhookPayload is compile-time only. NestJS's global ValidationPipe cannot validate plain TypeScript interfaces at runtime—it only validates class instances with class-validator decorators. Malformed input (missing signature or event-data fields) will bypass validation and reach lines 37–38, causing TypeError on property access and returning 500 instead of a controlled 4xx response.

Codebase pattern: all other controllers use DTO classes (e.g., ScheduleShoutDto, CreateDraftDto), which the ValidationPipe can validate. This endpoint is the only one using an interface.

Suggested fix: change @Body() payload: MailgunWebhookPayload to @Body() payload: unknown, add runtime guards to check for required fields, and throw ValidationError('Mailgun webhook payload malformed', {}) (already used elsewhere in the codebase for input validation) if validation fails. This aligns with the coding guideline to use unknown type with type guards instead of relying on compile-time type annotations.

🔧 Proposed fix
-import { Body, Controller, HttpCode, HttpStatus, Post } from '@nestjs/common';
+import { Body, Controller, HttpCode, HttpStatus, Post } from '@nestjs/common';
+import { ValidationError } from '../../shared/errors/domain-errors';
@@
-  async handle(
-    `@Body`() payload: MailgunWebhookPayload,
-  ): Promise<{ ok: true; outcome: WebhookOutcome }> {
+  async handle(
+    `@Body`() payload: unknown,
+  ): Promise<{ ok: true; outcome: WebhookOutcome }> {
+    if (
+      !payload ||
+      typeof payload !== 'object' ||
+      !('signature' in payload) ||
+      !('event-data' in payload)
+    ) {
+      throw new ValidationError('Mailgun webhook payload malformed', {});
+    }
+    const typed = payload as MailgunWebhookPayload;
+
     // Verify first — a malformed or forged event shouldn't even
     // reach our subscription logic.
-    this.webhook.verifySignature(payload.signature);
-    const outcome = await this.webhook.handleEvent(payload['event-data']);
+    this.webhook.verifySignature(typed.signature);
+    const outcome = await this.webhook.handleEvent(typed['event-data']);
     return { ok: true, outcome };
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core/src/modules/newsletter/mailgun-webhook.controller.ts` around lines
33 - 38, The Mailgun webhook handler currently types the body as
MailgunWebhookPayload but lacks runtime checks, causing TypeError when payload
is malformed; change the controller parameter from `@Body`() payload:
MailgunWebhookPayload to `@Body`() payload: unknown and add a runtime guard that
verifies payload is an object with a non-empty signature property and an
"event-data" property before calling
this.webhook.verifySignature(payload.signature) and
this.webhook.handleEvent(payload['event-data']); if the check fails, throw
ValidationError('Mailgun webhook payload malformed', {}) to return a controlled
4xx response.

Comment thread apps/core/src/shared/errors/domain-exception.filter.ts
dobrodob and others added 2 commits April 19, 2026 18:54
CI TypeScript strict mode (which my local run apparently used softer
settings for) rejects `event.request.headers.cookie = undefined`
because those fields are typed as `string` in @sentry/node — not
`string | undefined`. Reverting to `delete` with an inline
biome-ignore: the noDelete rule guards against hidden-class deopts,
but beforeSend fires at most a few times per hour on 5xx events, so
the perf concern is irrelevant here. `delete` is also the correct
semantic — remove the key entirely so Sentry's UI doesn't display an
empty-string value where the real one was scrubbed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verified all four CR comments against the code. Three were real
(the fourth — instrument.ts type error — was already resolved in the
previous fixup). Fixes:

1. **SENTRY_TRACES_SAMPLE_RATE empty-string coerces to 0** (CR, major)
   `.env.example` ships `SENTRY_TRACES_SAMPLE_RATE=` (empty). Without
   `emptyAsUndefined`, `z.coerce.number()` turns '' into 0 — silently
   disabling tracing instead of using the environment default. Wrapped
   the Zod schema with the existing helper for parity with other
   optional numerics.

   Also hardened instrument.ts: pulled the sample-rate resolution into
   a `resolveTracesSampleRate(env)` helper that treats empty / missing
   / unparseable / out-of-[0,1] as "use environment default" (1.0 dev,
   0.1 prod). parseFloat('') returns NaN — passing NaN to Sentry
   disables tracing too, so the old inline condition had the same bug.

2. **Mailgun webhook body: interface → DTO class** (CR, major)
   `@Body() payload: MailgunWebhookPayload` is compile-time only. The
   global ValidationPipe validates class instances with class-validator
   decorators, not bare interfaces — a malformed POST would reach the
   handler and crash on property access (500). Every other controller
   in the repo uses DTO classes; this endpoint was the lone holdout.

   New `apps/core/src/modules/newsletter/dto/mailgun-webhook.dto.ts`:
     - MailgunSignatureDto (timestamp, token, signature)
     - MailgunEventDataDto (event, severity?, recipient, message-id?)
     - MailgunWebhookBodyDto (signature + event-data with @ValidateNested)
   Controller now declares `@Body() payload: MailgunWebhookBodyDto` so
   ValidationPipe's whitelist+forbidNonWhitelisted rejects junk fields
   at the edge and produces a canonical 400 for malformed bodies.

3. **request.url leaks query strings into logs + Sentry** (CR, major)
   `req.url` includes the full query string. Our DomainExceptionFilter
   logged it into pino and Sentry `setContext('request', {path: url})`
   — any URL carrying a reset token, unsubscribe code, CSRF state, or
   email address would ship that tail to disk logs + external Sentry
   telemetry. Switched to `req.path` everywhere in the filter (severity
   logging paths, Sentry scope context, and the user-facing envelope).

All 439/439 tests still pass; mailgun-webhook.integration.spec.ts
re-run independently to verify DTO change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dobrodob dobrodob merged commit 1663d68 into main Apr 19, 2026
3 checks passed
@dobrodob dobrodob deleted the feat/sprint-16-production-ops branch April 19, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant