[Silthus]

Warning

Merge risk: high (qa-swarm danger rubric)

• The SES send path changes for every workflow email.
• This layer alone has no recipient limit, so the env switch stays off until feat(workflows): restrict sandbox email to verified org members #111483 lands.

Problem

Teams can provision a sandbox sender, but their workflow and test emails cannot reach the email provider through it.

Stacked on #111368. This layer starts at 72ded8d9180; GitHub shows the cumulative fork diff because native stacks cannot cross repositories.

Closes Silthus#232.

Changes

• Sandbox emails use PostHog's fixed address, the managed display name, and the dedicated SES tenant and configuration set.
• Sandbox emails add organization identification after templating. Custom sender fields and Reply-To are ignored; List-Unsubscribe headers are absent.
• Sandbox emails fail before delivery if final HTML loses identification in either scripting mode, or both rendered content parts are empty.
• Valid templates keep their content and CSS. The footer has a contrasting background, and preheaders become escaped text.
• Sandbox emails remain untracked, record live workflow metrics, and leave own-sender tier budgets untouched.
• Saved sandbox emails include the delivered footer. Throttled sandbox retries retain the original email payload.
• A disabled global switch skips sandbox sends and explains why. Grouped analytics distinguish SES acceptance from switch-off skips.
• The SES route extraction is mechanical for own senders. Their command assertions and five snapshots remain unchanged.

Before:

flowchart LR
    Email[Workflow or test email] --> Own[Own sender]
    Own --> SES[Team SES route]
    Email --> Sandbox[Sandbox sender]
    Sandbox --> Missing[No sandbox route]
    classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
    classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
    classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
    class Email,Missing phYellow;
    class Own,Sandbox phBlue;
    class SES phRed;

Loading

After:

flowchart LR
    Email[Workflow or test email] --> Own[Own sender]
    Own --> SES[Team SES route]
    Email --> Sandbox[Sandbox sender]
    Sandbox --> Switch[Global sending switch]
    Switch -->|off| Skip[Skipped send]
    Switch -->|on| Identity[Fixed identity and footer]
    Identity --> Validate[Validate final HTML]
    Validate -->|valid| SandboxSES[Sandbox SES route]
    Validate -->|invalid| Error[Template error before delivery]
    classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
    classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
    classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
    class Email,Skip,Error phYellow;
    class Own,Sandbox,Switch,Identity,Validate phBlue;
    class SES,SandboxSES phRed;

Loading

The worker's new settings default to:

+WORKFLOWS_SANDBOX_SENDER_ENABLED=false
+SES_SANDBOX_TENANT_NAME=
+SES_SANDBOX_CONFIGURATION_SET=
+SES_SANDBOX_FROM_ADDRESS=

Warning

Keep the sending switch off until recipient membership checks, sandbox caps and sandbox pause handling are deployed. This PR provides routing, not those policies.

How did you test this code?

Test rationale: Cases at EmailService.executeSendEmail extend the existing own-sender and accepted-footer tests. Rejection cases need distinct assertions for errors and absent delivery side effects. Real Postgres and Valkey exercise budgets; only external SES and analytics are mocked.

• Accepted-send cases assert fixed identity, tracking, logs, metrics, assets, original retry payloads, and preserved templates, preheaders and fallback content.
• Footer assertions reparse the actual SES HTML in both scripting modes and require the correct identification directly in the body.
• No-send cases cover malformed body/template HTML and empty rendered content for live and test sends, including errors and absent delivery side effects.
• An own-sender send after each rejection proves its budget remains available. A text-only sandbox send retains identification without an HTML part.

Local verification and observed output

Run from the repository root:

.codex/with-flox env LD_LIBRARY_PATH="$PWD/.flox/run/aarch64-linux.posthog-dev/lib" pnpm --dir nodejs exec jest --runInBand --forceExit --runTestsByPath src/cdp/services/messaging/email.service.test.ts
.codex/with-flox pnpm --dir nodejs exec tsc --noEmit
.codex/with-flox hogli ci:preflight --strict --against 72ded8d9180

All 145 tests at the public send seam passed, including five unchanged own-sender snapshots. Scoped ESLint/Oxlint and the real pre-push checks passed.

Captured public-seam output included 19 accepted messages and six rejected messages with zero SES calls, saved assets or acceptance analytics. Native T3 browser checks rendered the captured SES HTML with a visible footer and contrasting background. Safe invented-data screenshots remain local because fork permissions prevent asset upload.

No real SES delivery or DNS changes were attempted. There are no web UI changes.

CI remains incomplete: Node.js CI and other fork workflows require maintainer approval.

The ancillary client-library reviewer workflow failed at startup with no jobs or logs. GitHub denied its rerun for insufficient admin rights.

Release status

• No feature flag controls this change
• This change is behind a feature flag and is not available to users
• This change makes a previously flagged feature available to everyone

Creation stays behind workflows-sandbox-sender; the send path uses the separate default-off global switch.

Automatic notifications

• Publish to changelog?

Docs update

Updated the existing email sending tiers guide with the sandbox budget exclusion and rollout dependency. No existing document covers footer validation; no new document was added.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Codex, GPT-6.1 Sol

Skills: /tdd, /writing-tests, /writing-user-facing-copy, /ingestion-pipeline-doctor-nodejs, /hogli, /running-ci-preflight, /stacking-prs, /writing-pr-descriptions, /writing-voice, /unslop, and /qa-swarm.

The duplicate search found the parent sandbox provisioning PR, which does not add this send route. The fixtures use invented example.com addresses.

The prior four-round QA cycle ended blocked on a footer failure. A separately authorized follow-up validates final output, rejects empty content and closes a Redis fixture leak.

Follow-up QA completed three rounds. The final cold panel used six GPT-6.1 Sol lenses and Claude Sonnet 5.5 on 0007a4f79c7.

After triage, the final round has no new fix or call. The shared preheader helper's existing replacement-token behavior remains a separate follow-up.

CodeRabbit CLI is signed out, and remote reviews remain paused after an older head. GitHub denied PR assignment and screenshot upload for the fork author.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: b01ce04a-33ea-4048-8bf7-e88b4ee3b903

📥 Commits

Reviewing files that changed from the base of the PR and between cad864b and 8dc512c.

📒 Files selected for processing (2)

• nodejs/src/cdp/services/messaging/email.service.test.ts
• nodejs/src/cdp/services/messaging/sandbox-email-sender.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds backend provisioning for managed sandbox email integrations and prevents users from verifying, editing, or deleting them. Node email sends can use a configured sandbox SES route and fixed sender identity. Disabled sends are skipped, and sandbox sends bypass team sending-budget claims. Tests cover provisioning, send behavior, and selected lifecycle and reputation operations.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 8dc51

Disabled sandbox sends may still error or miss skip accounting and the switch-off event. Resolve those paths before merging unless their behavior is explicitly accepted.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 8dc51

The sender is disabled by default, and project ownership and fixed-identity controls limit exposure. However, enabling it before independent recipient, volume and pause safeguards are deployed would allow tenant-controlled traffic through a shared sending identity without the ordinary project sending budget. Production enablement was not verified.

Retained concerns

• Medium · security · inferred: Enabling the new shared sandbox route before its independent safeguards are deployed creates an abuse-containment gap. Sandbox sends explicitly bypass the ordinary team sending budget, while the inspected delivery path does not enforce sandbox-specific recipient membership, caps or pause state. A tenant authorized to use the sender could consequently send tenant-controlled content to external recipients through the shared identity, affecting its reputation and delivery capacity. The default-off switch, existing suspension and suppression checks, and explicit rollout warning are important counterevidence; enabled production exposure is not established.

Security review details

Security Blast Radius

• inferred — When enabled, projects with a provisioned sandbox integration can direct tenant-authored messages through the configured shared sender and sandbox SES tenant. Abuse can affect other users of that route through reputation or capacity loss. Documentation states that workflow email also shares an SES account reputation pool; the actual account, environment and IAM exposure were not verified.

Security Findings and Attack Paths

• inferred — The supported conditional attack path is authorized tenant access to the sandbox integration, tenant-selected recipients and content, exemption from team budgets, and external delivery using the shared identity. Optional workflow pacing is not a sandbox abuse boundary and test sends bypass it. The global switch currently defaults off, and production activation is unknown; this is not an assertion of active exploitation.

Trust Boundaries and Controls

• observed — Existing team suspension, workflow pause and recipient suppression gates precede external delivery. Cross-project integration references are rejected. Sandbox sender address and SES routing come from worker configuration, while footer organization identity comes from the invocation team rather than message fields. These controls constrain identity substitution but do not substitute for the deferred sandbox recipient and volume policy.

Resilience and Maintainability Implications

• observed — Provisioning refreshes changed managed configuration and publishes worker invalidation. Lifecycle tests assert that sandbox rows are excluded from ordinary tenant migration and reputation sweeps, and that project deletion does not delete the shared SES identity. These are source and test expectations, not executed-test results or proof of recovery under concurrency and interruption.

Hardening Proposals

• proposed — Make sandbox enablement contingent on the independent recipient, volume and pause controls being available, with fail-closed behavior during partial rollout or rollback. This would turn the documented deployment prerequisite into an enforceable security boundary.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description covers the problem, user-visible changes, testing, release status, documentation, and agent context. It also explains the rollout dependency and reports incomplete CI. The required sec…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (2)

nodejs/src/cdp/services/messaging/email.service.ts-482-497 (1)

482-497: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Sandbox-disabled check runs after resolveFromSender, which can throw first.

For a disabled switch, resolveFromSender (Line 482) throws if the integration is unverified or lacks email/name. The send then fails with an error instead of the intended skipped result. Move the isSandbox && !enabled gate before resolveFromSender.

Proposed fix

-            const from = this.resolveFromSender(integration, params.from, addLog)
-
             if (isSandbox && !this.sandboxSender?.config.enabled) {
 ...
                 return result
             }
+            const from = this.resolveFromSender(integration, params.from, addLog)

posthog/api/integration.py-2447-2462 (1)

2447-2462: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require admin access before refreshing an existing sandbox sender.

When an existing sender’s config differs, ensure_sandbox_email_sender can overwrite it and reload integrations. The action uses the create-level permission chain, so a project member with integration:write access can make this edit without the admin access required by IntegrationManagementPermission. Keep member-level access for first-time creation, but require the edit permission before refreshing an existing row.

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 55c3e128-8522-4e68-ba03-e7f7cda9b08c

📥 Commits

Reviewing files that changed from the base of the PR and between 1148900 and 7b73ca1.

⛔ Files ignored due to path filters (1)

• products/integrations/frontend/generated/api.ts is excluded by !**/generated/**

📒 Files selected for processing (19)

• docs/internal/workflows-email-sending-tiers.md
• nodejs/src/cdp/cdp-services.ts
• nodejs/src/cdp/config.ts
• nodejs/src/cdp/services/messaging/email.service.test.ts
• nodejs/src/cdp/services/messaging/email.service.ts
• nodejs/src/cdp/services/messaging/sandbox-email-sender.ts
• nodejs/src/cdp/types.ts
• posthog/api/integration.py
• posthog/models/integration/__init__.py
• posthog/models/integration/email.py
• posthog/settings/ses.py
• products/integrations/mcp/tools.yaml
• products/workflows/backend/facade/api.py
• products/workflows/backend/facade/contracts.py
• products/workflows/backend/presentation/views/hog_flow.py
• products/workflows/backend/services/sandbox_sender.py
• products/workflows/backend/test/test_sandbox_sender_lifecycle.py
• products/workflows/backend/tests/api/test_email_reputation.py
• products/workflows/backend/tests/api/test_sandbox_sender.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟠 Major · Mark suspended sends as skipped. · email.service.ts:485

nodejs/src/cdp/services/messaging/email.service.ts:485
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Mark suspended sends as skipped.

When suspension returns from executeSendEmail, it only pushes { success: false } and leaves both skipped and deliveredToRecipient unset. hog_function.ts therefore enters the billing branch: !functionResult.skipped is true, and functionResult.deliveredToRecipient !== false is also true. This can bill an email that was not sent.

Set result.skipped in the suspension return. Keep suspension handling before the disabled sandbox-switch branch so it continues to record email_suspended, rather than incorrectly reporting switch_off.

🐛 Suggested fix

                 result.invocation.state.vmState?.stack.push({ success: false })
+                result.skipped = true
                 return result

Add coverage for a suspended sandbox team with the sandbox switch disabled.

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 635d848d-02f4-4cc2-916c-2aeafeb16e52

📥 Commits

Reviewing files that changed from the base of the PR and between 7b73ca1 and 5fd964a.

📒 Files selected for processing (2)

• nodejs/src/cdp/services/messaging/email.service.test.ts
• nodejs/src/cdp/services/messaging/email.service.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

nodejs/src/cdp/services/messaging/email.service.ts-497-497 (1)

497-497: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use the persisted sandbox sender address for SES.

The backend stores a non-empty integration.config.email, and resolveFromSender already validates it. The sandbox branch ignores that address and uses SES_SANDBOX_FROM_ADDRESS, whose Node default is empty. When the sandbox sender is enabled with that setting empty, sendEmailWithSES can pass an empty sender and feedback address to SES, which may reject the send. Use the persisted address:

🐛 Suggested fix

         if (integration.config.provider === 'sandbox') {
-            return { email: this.sandboxSender?.config.fromAddress ?? '', name: integration.config.name }
+            return { email: integration.config.email, name: integration.config.name }
         }

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 621ad070-3440-4603-bb7a-e7cefddbba12

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd964a and e63e981.

📒 Files selected for processing (2)

• nodejs/src/cdp/services/messaging/email.service.test.ts
• nodejs/src/cdp/services/messaging/email.service.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.