Skip to content

feat(event-process): SignatureValidator registry + 7 provider validators (EVO-1210)#38

Merged
dpaes merged 3 commits into
developfrom
feat/EVO-1210
Jun 10, 2026
Merged

feat(event-process): SignatureValidator registry + 7 provider validators (EVO-1210)#38
dpaes merged 3 commits into
developfrom
feat/EVO-1210

Conversation

@nickoliveira23

Copy link
Copy Markdown

What

Story 3.4 of the Webhook Event Pipeline: signature validation for inbound provider webhooks. EventProcessService.handle() resolves a per-provider validator from a registry and runs it right after envelope validation — an invalid/unverifiable signature, or an unregistered platform, is dropped (ack, never nack) with a structured warning and the evo_webhook_signature_invalid_total{platform,reason} counter.

Validators

Plain, unit-testable classes resolved by SignatureValidatorRegistry with secrets from env via ConfigService:

Platform Scheme
evolution-api static token (apikey header / Bearer)
sparkpost HTTP Basic Auth
sendgrid ECDSA over timestamp+body, opt-in (passes through when no key)
mailersend HMAC-SHA256 (hex)
resend Svix
ses AWS SNS (sns-validator) + .amazonaws.com cert allowlist
mandrill HMAC-SHA1 (base64) over webhook URL + sorted params

Constant-time comparisons throughout; fail-closed on a missing secret (except opt-in sendgrid), with a boot-time "unconfigured-secrets" warning so misconfiguration doesn't masquerade as an attack in the metric.

Acceptance Criteria

  • AC1 valid signature → continues the pipeline
  • AC2 invalid signature → dropped (ack) + metric +1
  • AC3 unknown / unregistered platform → dropped with warning
  • AC4 each of the 7 validators has a valid + invalid fixture

Decisions / notes

  • validate is async (boolean | Promise<boolean>) — SES/SNS fetches the signing cert over HTTPS; handle() awaits.
  • New deps: svix, sns-validator (+ type shim). New env vars documented in .env.example.
  • SendGrid ECDSA assumes DER encoding (Node default); not exercised against a real SendGrid payload — that smoke is deferred to Growth per the story scope.

Tests

45 unit tests: per-validator (valid + invalid), registry resolution, and EventProcessService drop/pass/no-validator paths. tsc + eslint clean.

Story 3.4 — blocked-by EVO-1208 (Done). Unblocks EVO-1211 [3.5] (idempotency wires into the same handle()).

…tors (EVO-1210)

Adds webhook signature validation to the event-process pipeline (story 3.4).
EventProcessService.handle() now resolves a per-provider validator via the
registry and runs it right after envelope validation: an invalid or
unverifiable signature is dropped (ack, never nack — no redelivery) with a
structured warning and the evo_webhook_signature_invalid_total{platform,reason}
counter; an unregistered platform (e.g. `unknown`) is dropped the same way.

Validators (plain, unit-testable classes resolved by the registry with secrets
from env via ConfigService):
- evolution-api: static token (apikey header / Bearer)
- sparkpost:     HTTP Basic Auth
- sendgrid:      ECDSA over timestamp+body, opt-in (passes through with no key)
- mailersend:    HMAC-SHA256 (hex)
- resend:        Svix
- ses:           AWS SNS (sns-validator) + .amazonaws.com cert allowlist
- mandrill:      HMAC-SHA1 (base64) over webhook URL + sorted params

Fail-closed when a secret is missing (except opt-in sendgrid); the registry
logs an "unconfigured-secrets" warning at boot so misconfiguration does not
masquerade as an attack in the metric. Constant-time comparisons throughout.

Adds deps svix + sns-validator (+ type shim) and documents the new
*_WEBHOOK_* env vars in .env.example. Covered by per-validator specs (valid +
invalid fixtures, AC4), a registry spec and EventProcessService drop/pass specs
— 45 tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @nickoliveira23, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@dpaes

dpaes commented Jun 10, 2026

Copy link
Copy Markdown

🔴 Reprovado — EVO-1210 [3.4] SignatureValidator

Os 4 critérios de aceite passam no texto literal, mas a revisão adversarial achou 2 quebras de segurança blocker-grade (ambas verificadas no código real). Card volta para Todo; PR fica aberto para o ajuste.

B1 — SparkPost descarta 100% do tráfego legítimo

O SparkPostValidatorAuthorization: Basic …, mas a story 3.1 (payload-normalizer.service.ts, REDACTED_HEADERS) redige esse header para [REDACTED] antes de publicar o envelope. No consumidor, provided.startsWith('Basic ') é sempre false → todo evento SparkPost é descartado (fail-closed, mas não-funcional). O spec passa só porque monta o validator com um header vivo que nunca sobrevive à ingestão. O fix mora no normalizador da 3.1 (preservar Authorization) ou no esquema do validator.

B2 — SES fail-OPEN (forja de assinatura)

ses.validator.ts:6 define AMAZON_HOST = /(^|\.)amazonaws\.com$/ e o passa a new MessageValidator(AMAZON_HOST) (:21), enfraquecendo o default seguro da lib sns-validator (^sns\.…). Esse regex casa qualquer *.amazonaws.com → um atacante hospeda um cert .pem num bucket S3 público e forja eventos SNS (PoC reproduzido na review: keypair do atacante + cert em *.s3.amazonaws.comvalidate() ACCEPTED; default REJECTED). É exatamente o vetor que o FR15 existe pra impedir. Fix: remover o arg custom (usar default) ou exigir prefixo ^sns\.. Bônus: o regex custom também rejeita a região China.

Branch íntegro (base develop, CLEAN) — sem problema de rebase.

nickoliveira23 and others added 2 commits June 9, 2026 21:23
…idator (EVO-1210)

The custom `/(^|\.)amazonaws\.com$/` host pattern weakened sns-validator's
secure default: an attacker could host a forged signing cert on a public S3
bucket (*.s3.amazonaws.com) and pass verification. Drop the custom override
(use the library's default `^sns\.…\.amazonaws\.com` pattern) and tighten the
local cert-URL guard to the same strict SNS host shape. Adds a fixture proving
an S3-hosted cert is rejected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t (EVO-1210)

The normalizer redacted the Authorization header before publishing the
envelope, so the SparkPost validator (HTTP Basic Auth) always saw [REDACTED]
and dropped 100% of legitimate SparkPost traffic. Preserve Authorization for
the `sparkpost` platform only; every other credential header, and Authorization
on any other platform, stays redacted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@dpaes dpaes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Aprovado — EVO-1210 [3.4] SignatureValidator

Re-review dos fixes (head 85b191f): os 2 blockers fecharam e foram verificados no código final.

  • B1 (SparkPost): Authorization preservado exclusivamente quando platform === 'sparkpost' (payload-normalizer.shouldRedact); todas as outras plataformas e demais headers de credencial seguem redigidos — sem novo vazamento. O validator passa a verificar o Basic real (constant-time).
  • B2 (SES): removido o override frouxo; usa o default seguro do sns-validator + guard ^sns\.[a-z0-9-]{3,}\.amazonaws\.com(\.cn)?$. Cert forjado em *.s3.amazonaws.com é rejeitado.
  • tsc / jest (82) / eslint limpos (auto-reportados — CI do evo-flow não roda).

Follow-up registrado (não-bloqueante): o B1 faz o secret Basic-Auth do SparkPost ser persistido no event-store (escopado aos eventos sparkpost). Para não persistir a credencial, o end-state mais limpo é validar o Basic no receiver (antes da redação) — fica como melhoria futura, fora desta entrega.

Aprovado e mergeado para develop.

@dpaes dpaes merged commit e481726 into develop Jun 10, 2026
4 checks passed
@dpaes dpaes deleted the feat/EVO-1210 branch June 10, 2026 00:56
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.

2 participants