feat(workflows): email admins on AWS SES tenant reputation changes - #75249
feat(workflows): email admins on AWS SES tenant reputation changes#75249meikelmosby wants to merge 3 commits into
Conversation
🦔 ReviewHog reviewed this pull requestFound 0 must fix, 3 should fix, 5 consider. Published 8 findings (view the review). |
🤖 CI report
|
478cbc7 to
4094b6a
Compare
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
There was a problem hiding this comment.
ReviewHog Report
Business logic
Issues: 5 issues
Files (7)
products/workflows/backend/migrations/0017_teamworkflowsconfig_ses_tenant_state.pyproducts/workflows/backend/models/team_workflows_config.pyproducts/workflows/backend/services/ses_tenant_state.pyproducts/workflows/backend/tasks/ses_tenant_state.pyposthog/tasks/email.pyposthog/templates/email/email_sending_reputation_finding.htmlposthog/tasks/scheduled.py
What were the main changes
- Migration 0017 adds ses_tenant_sending_status, ses_tenant_reputation_impact, ses_tenant_state_synced_at to TeamWorkflowsConfig with db_default so raw non-Django INSERTs keep working
- apply_ses_tenant_state/sync_ses_tenant_state persist authoritative SES tenant state and email project admins only on meaningful transitions (pause, unpause/reinstate, reputation escalation); unchanged state and de-escalations are silent, and the first-ever sync sets a baseline without notifying
- New Celery tasks: sync_ses_tenant_state_task (webhook-triggered) and reconcile_ses_tenant_states (daily sweep over teams with an SES email integration), the latter registered in scheduled.py at 6:30 AM UTC
- New send_email_sending_reputation_finding email task plus email_sending_reputation_finding.html template distinguishing LOW (warning) vs HIGH (action required) impact
Api
Issues: 3 issues
Files (4)
products/workflows/backend/api/ses_events_webhook.pyproducts/workflows/backend/services/sns_verification.pyposthog/urls.pyposthog/settings/ses.py
What were the main changes
- New POST /webhooks/workflows/ses-events endpoint that extracts the team- tenant from EventBridge SES events and enqueues sync_ses_tenant_state_task rather than trusting event payload state
- Handles SNS SubscriptionConfirmation with SubscribeURL host validation, and Notification messages by parsing the embedded EventBridge event and filtering to source aws.ses
- New sns_verification.py implementing full SNS signature verification: cert-URL host validation, cached certificate fetch, canonical string-to-sign construction, and SignatureVersion 1/2 RSA verification
- Two-layer auth: SNS signature proves the message is from AWS, new WORKFLOWS_SES_EVENTS_SNS_TOPIC_ARNS allowlist (empty by default, ships inert) proves it's from the project's own topic
- URL routing wires the new webhook view into posthog/urls.py
4094b6a to
0fa3620
Compare
a274ba2 to
66fc617
Compare
0fa3620 to
f20392b
Compare
0670f50 to
f58b5cb
Compare
Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…elines Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
ddb0b9d to
b62336f
Compare
Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
Problem
Stacked on #75237 (which stacks on #74762). With the
Standardreputation policy, AWS can pause a project's email sending — but until now nobody would tell the customer. This is the top-priority follow-up from the AWS-tenant switch: when AWS changes a tenant's sending status or raises reputation findings, the project's admins must hear about it.Changes
Design principle: events are change signals, not state. The webhook never trusts an event payload — it extracts the
team-<id>tenant name and enqueues a sync that reads the authoritative state from the SES API (SESProvider.get_tenant_reputation, from #75237). Duplicate, reordered, or partially-shaped events are therefore harmless, and the reconciliation sweep shares the exact same code path.State + transitions –
TeamWorkflowsConfiggainsses_tenant_sending_status,ses_tenant_reputation_impact,ses_tenant_state_synced_at(migration 0017;db_defaultso raw non-Django INSERTs keep working).apply_ses_tenant_statepersists the new state and emails project admins on meaningful transitions only:→ DISABLED): reuses the existing suspension email with a provider-pause reasonDISABLED → ENABLED/REINSTATED): reuses the re-enabled emailnone → LOW,LOW/none → HIGH): newemail_sending_reputation_findingtemplate — LOW is a fix-this warning, HIGH is "[Action required] … at risk of being paused"The stored state is the dedupe mechanism (unchanged state sends nothing), the first-ever sync sets a baseline without notifying (so rollout doesn't mass-email), and de-escalations stay silent. Recipient selection reuses
_get_project_admins_to_notify_of_email_sending_suspension— admin+, not gated by notification settings.Webhook –
POST /webhooks/workflows/ses-events, following the repo's external-webhook pattern (@csrf_exemptfunction view, ack fast, Celery for the work). Auth is two-layer: full SNS signature verification (newsns_verification.py, ported from the Node SES webhook — cert-URL host validation, cached cert fetch, canonical string-to-sign, SignatureVersion 1/2) plus a topic-ARN allowlist (WORKFLOWS_SES_EVENTS_SNS_TOPIC_ARNS, empty by default so the endpoint ships inert). HandlesSubscriptionConfirmationwith SubscribeURL host validation. Only SNS SignatureVersion 2 (SHA256) is accepted — version 1 signs with SHA1, so instead of verifying a weak hash we require the topic be configured withSignatureVersion=2(one-timeaws sns set-topic-attributes --attribute-name SignatureVersion --attribute-value 2, added to the wiring runbook below).Reconciliation sweep – daily Celery beat task sweeping every team with an SES email integration through the same sync, since EventBridge delivery is best-effort. Kept daily (3 SES API calls per team) to stay inside SES API rate limits; the webhook is the real-time path.
Note
AWS-side wiring is a runbook step, not code: EventBridge rule on
source = aws.sesfor the tenant detail-types ("Sending Status Disabled/Enabled", "Advisor Recommendation Status Open/Resolved") → SNS topic withSignatureVersion=2→ HTTPS subscription to this endpoint, then setWORKFLOWS_SES_EVENTS_SNS_TOPIC_ARNSto the topic ARN. In-app notifications (create_notification) are a possible follow-up; this PR ships the email path.How did you test this code?
detail.tenantName, tenant extraction from resource ARNs, non-aws.sessources acked but ignored, unknown topic → 403, bad signature → 403, empty allowlist → inert 500, subscription confirmation fetches the SubscribeURL, off-AWS SubscribeURL rejected.sns.us-east-1.amazonaws.com.evil.comspoof.makemigrations --checkclean; repo-wide mypy clean.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with PostHog Code (Claude). Skills followed: django-migrations (additive fields with db_default, checked external table writers), writing-tests (parameterized transition table), improving-drf-endpoints n/a (no DRF surface — webhook is a plain view per the repo's webhook convention). Key decision: fetch-on-event instead of trusting event payloads, so the webhook and reconciliation sweep converge on one code path and AWS's evolving event schema can't break state handling.
Created with PostHog Code