Skip to content

feat(workflows): refuse edits to the managed sandbox sender - #128948

Closed
Silthus wants to merge 22 commits into
workflows-sandbox-split/sender-surfacesfrom
workflows-sandbox-split/managed-sender-guards
Closed

Silthus wants to merge 22 commits into
workflows-sandbox-split/sender-surfacesfrom
workflows-sandbox-split/managed-sender-guards

Conversation

@Silthus

@Silthus Silthus commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • A team new to Workflows cannot send a single email until it verifies its own sending domain.
  • The stack adds one PostHog-managed sandbox sender per project. A team must not change it, and features built for own senders must skip it.
  • Layer 2 of the product stack, split from #128252, on #128947 (sender surfaces). Next: #128949, #128950, #128951. The Node stack starts at #128943.

Merge danger

Warning

🟡 Merge risk: medium · two-way door (merge-risk rubric)

  • The edit, verify, delete and default paths of every email integration, and the sender pick for transactional sends from code, now run a sandbox check. A bug blocks a team from managing or sending from its own sender: a broken core action that reaches some teams.
  • What limits it: the check matches only provider sandbox, which no row has until #128949. The domain block is off while its setting is empty. Tests pair each refusal with the own-sender path.
  • Revert: git revert the merge commit. Nothing to clean up: this PR creates no rows.

Changes

  • Editing, verifying, deleting or checking the status of the sandbox sender answers "The sandbox sender is managed by PostHog."
  • Making it the project default answers "The sandbox sender can't be the project default. Choose a sender on your own verified domain."
  • A team cannot add its own sender on the sandbox domain or a subdomain of it.
  • Making a template transactional with the sandbox sender, or saving one whose workflow sends from it, answers "The sandbox sender can't send transactional email. Choose a sender on your own verified domain."
  • A send from code never picks the sandbox sender, by address or by default.
  • The setup guide, AI data suggestions and the email health view skip the sandbox sender.
  • SES tenant migration, the tenant state sweep and project deletion already skip it through their existing provider == "ses" filters. New tests pin that.
  • WORKFLOWS_SANDBOX_SENDER_DOMAIN joins posthog/settings/ses.py, because the domain check reads it.

How did you test this code?

Test rationale: each group catches a managed row that a team can change, or that leaks into a feature built for own senders. The tests create the sandbox row directly, before #128949 adds the endpoint.

  • test_sandbox_sender.py: every refused action, and the reserved domain with its subdomains.
  • test_sandbox_sender_lifecycle.py: the sweep runs the real synchronizer with only SES mocked, and looks up only the team with an own ses sender.
  • Live run at this head: Not run yet.
Earlier local run, at 3aa6850 to e92a320 (not saved as proof)
Call Result
Edit, verify, check status of, or delete the sandbox row 400 "The sandbox sender is managed by PostHog." The row survives.
Make the sandbox row the project default 400 "The sandbox sender can't be the project default. Choose a sender on your own verified domain." The own sender: 200
Add an own sender on the sandbox domain 400 "Email domain … belongs to the PostHog sandbox sender. Please use a domain you own."
Make a template transactional with the sandbox sender Before: 400 "No verified email sender." After: 400 "The sandbox sender can't send transactional email."
Setup guide, project with only the sandbox row Before: Channel done. After: Channel not done.
Save a transactional template whose workflow sender was switched to the sandbox row Before (13f6268): 200, new live version. After (e92a320): 400 "The sandbox sender can't send transactional email."
Send from code naming an address on the sandbox domain (merge with master, 4944a97) Without the filter: accepted. With it: 400 sender_not_verified
CI report findings
  • The PR's diff against master includes Python, so Trunk runs it in the backend lane.
  • Backend coverage lists the sandbox guards as uncovered, but tests run them: test_sandbox_sender.py (PATCH email/, verify, status, delete) and test_team_workflows_config.py (sandbox sender as the default). The coverage run does not attribute these products/workflows tests to the posthog/ lines.
  • Hogbox preview is an optional preview build, not a test of this change.
  • Playwright reports one flaky test outside this diff.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

  • docs/internal/workflows-email-sending-tiers.md gains a "Sandbox sender" section with the guards and WORKFLOWS_SANDBOX_SENDER_DOMAIN.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /improving-drf-endpoints, /writing-tests, /stacking-prs, /writing-pr-descriptions, /qa-swarm

Decisions for the whole sandbox sender stack
Question Answer Who Where
Who gets a sandbox email Verified members of the team's organization, checked at send time over To, CC and BCC 👤 Human decision record
Sender identity Fixed From address on a dedicated PostHog domain, "Acme via PostHog" display name, no From or Reply-To override 👤 Human decision record
Where it can send Workflow email steps and test sends only, not broadcasts or transactional email 👤 Human decision record
How the sender is stored One managed email integration per team, provider sandbox, created only by PostHog code 👤 Human decision record
SES setup One shared identity, configuration set and tenant for all sandbox sends 👤 Human decision record
Landing order Three-PR skeleton, then the flag to our team, then caps and polish (replaced landing the twelve-layer stack at once) 👤 Human decision record
Node gate Environment switch WORKFLOWS_SANDBOX_SENDER_ENABLED, no feature flag in CDP Node code 👤 Human decision record
Member check in the skeleton Keep it, although the ask was a skeleton with no limits 👍 Accepted default decision record
Behavior with everything off No change while the flag and switch are off 👍 Accepted default decision record
Earlier PRs Close #127149 to #127153, #127136 and #127137, each linked to its successor 👍 Accepted default decision record
PR shape Two native stacks of small PRs: Node (4) and product (5) 👍 Accepted default decision record
Member lookup cache in the skeleton Dropped; every check is one fresh lookup, the cache returns with the caps stack 👍 Accepted default decision record
Tier budget Sandbox sends never spend the team's tier budget 🤖 Agent decision record
When members are checked Before preparing the send and again fresh before delivery 🤖 Agent decision record
Member lookup fails Skip the send with a log 🤖 Agent decision record
Blocked-send events At most one per team, reason and hour 🤖 Agent decision record
How a team gets the sender Opening Channels with the flag on creates it 🤖 Agent decision record
Managed sender guards Nobody can edit, verify, delete or default the sandbox sender 🤖 Agent decision record
Quoted member addresses Skip the send, and canonicalize in the caps stack instead of respinning the skeleton 🤖 Agent decision record

No open questions.

More context for AI agents
  • Session: Claude Code. Review rounds 1, 2 and final reads 1 and 2 ran on Claude Opus 5.5 only, because the GPT reviewers were unreachable for the whole session.
  • Decisions: SANDBOX_EMAIL_INTEGRATION_ID and the domain setting live here, so the guards stand without the endpoint.
  • Decisions: the guards stay explicit ensure_user_managed() calls at each management entry point, not a throwing EmailIntegration constructor, so read-only uses of the adapter keep working.
  • Decisions: review dropped the forwarding is_sandbox_sender property and made the sweep test real (4742c86, ba1b846, e2a237c).
  • Follow-ups in this stack: #129061 adds the save-time check that stops the workflow API from setting the sandbox sender.
  • Read closely: perform_destroy and email_update in posthog/api/integration.py, and posthog/models/integration/email.py.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • create experiment via wizard, add metrics, and launch (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

⚠️ Backend coverage — 91.0% of changed backend lines covered — 15 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ██████████████████░░ 91.0% (165 / 180)

File Patch Uncovered changed lines
posthog/api/integration.py 33.3% 3016–3017, 3019, 3022
posthog/api/team/team_config.py 33.3% 443–444
products/workflows/backend/services/transactional_email_workflows.py 72.2% 165–166, 171, 181, 193
posthog/models/integration/email.py 81.2% 76, 95, 240
products/messaging/backend/services/transactional_email.py 85.7% 604

🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 231709626052783 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
data_tools ████████████░░░░░░░░ 60.9% 95 / 156
warehouse_sources_queue █████████████░░░░░░░ 65.2% 2,161 / 3,314
demo ██████████████░░░░░░ 71.2% 1,982 / 2,783
knowledge_writer ███████████████░░░░░ 76.5% 1,289 / 1,685
batch_exports █████████████████░░░ 82.6% 24,173 / 29,278
engineering_analytics █████████████████░░░ 85.5% 12,384 / 14,482
ml_inference █████████████████░░░ 86.3% 524 / 607
commands █████████████████░░░ 86.5% 1,645 / 1,901
mcp_analytics █████████████████░░░ 86.8% 6,235 / 7,180
posthog_ai ██████████████████░░ 88.2% 4,502 / 5,104
apm ██████████████████░░ 88.2% 1,973 / 2,236
notebooks ██████████████████░░ 88.8% 15,454 / 17,409
mcp_registry ██████████████████░░ 89.0% 1,596 / 1,794
dashboards ██████████████████░░ 89.5% 8,130 / 9,087
product_tours ██████████████████░░ 89.6% 1,355 / 1,512
warehouse_suggestions ██████████████████░░ 90.2% 3,879 / 4,300
ai_gateway ██████████████████░░ 90.3% 971 / 1,075
cohorts ██████████████████░░ 90.5% 9,621 / 10,633
visual_review ██████████████████░░ 90.7% 9,870 / 10,883
signals ██████████████████░░ 90.8% 105,661 / 116,317
data_warehouse ██████████████████░░ 91.1% 15,703 / 17,236
canvas ██████████████████░░ 91.2% 8,205 / 8,996
cdp ██████████████████░░ 91.4% 10,806 / 11,817
today ██████████████████░░ 91.5% 3,975 / 4,346
tasks ██████████████████░░ 91.5% 85,362 / 93,291
autoresearch ██████████████████░░ 91.6% 12,047 / 13,151
metrics ██████████████████░░ 91.9% 11,564 / 12,581
streamlit_apps ██████████████████░░ 91.9% 3,170 / 3,448
data_modeling ██████████████████░░ 92.0% 12,440 / 13,526
business_knowledge ██████████████████░░ 92.1% 10,871 / 11,801
managed_warehouse ██████████████████░░ 92.3% 14,337 / 15,533
wizard ██████████████████░░ 92.3% 7,031 / 7,616
conversations ██████████████████░░ 92.3% 31,407 / 34,013
review_hog ██████████████████░░ 92.4% 18,136 / 19,636
early_access_features ██████████████████░░ 92.5% 1,339 / 1,448
error_tracking ███████████████████░ 92.5% 20,776 / 22,453
actions ███████████████████░ 92.8% 1,360 / 1,466
notifications ███████████████████░ 92.8% 1,219 / 1,313
slack_app ███████████████████░ 92.9% 24,198 / 26,051
alerts_platform ███████████████████░ 92.9% 9,006 / 9,695
alerts ███████████████████░ 93.2% 12,009 / 12,885
stamphog ███████████████████░ 93.2% 10,903 / 11,697
webmcp ███████████████████░ 93.3% 277 / 297
ai_training ███████████████████░ 93.3% 598 / 641
web_analytics ███████████████████░ 93.5% 27,004 / 28,886
surveys ███████████████████░ 93.5% 7,456 / 7,973
exports ███████████████████░ 93.6% 4,394 / 4,696
approvals ███████████████████░ 93.7% 5,004 / 5,343
cross_project_dashboards ███████████████████░ 93.7% 858 / 916
workflows ███████████████████░ 93.8% 28,971 / 30,897
context_layer ███████████████████░ 93.8% 4,066 / 4,333
marketing_analytics ███████████████████░ 93.9% 22,170 / 23,617
authorization ███████████████████░ 94.0% 671 / 714
mcp_store ███████████████████░ 94.0% 9,324 / 9,920
billing_alerts ███████████████████░ 94.0% 2,322 / 2,470
messaging ███████████████████░ 94.1% 8,884 / 9,445
ai_observability ███████████████████░ 94.1% 34,380 / 36,546
customer_analytics ███████████████████░ 94.1% 30,872 / 32,803
managed_migrations ███████████████████░ 94.1% 1,877 / 1,994
legal_documents ███████████████████░ 94.3% 2,313 / 2,453
replay_vision ███████████████████░ 94.4% 33,357 / 35,351
logs ███████████████████░ 94.4% 19,543 / 20,692
aeo ███████████████████░ 94.6% 3,138 / 3,318
access_control ███████████████████░ 94.7% 8,515 / 8,991
reminders ███████████████████░ 94.8% 782 / 825
experiments ███████████████████░ 94.8% 45,809 / 48,311
warehouse_sources ███████████████████░ 95.0% 141,344 / 148,847
growth ███████████████████░ 95.0% 11,589 / 12,202
endpoints ███████████████████░ 95.0% 9,720 / 10,234
annotations ███████████████████░ 95.0% 1,097 / 1,155
skills ███████████████████░ 95.2% 7,656 / 8,043
uptime_monitoring ███████████████████░ 95.3% 306 / 321
data_catalog ███████████████████░ 95.6% 4,740 / 4,956
subscriptions ███████████████████░ 95.7% 14,021 / 14,654
ui_customization ███████████████████░ 95.7% 714 / 746
product_analytics ███████████████████░ 95.8% 31,105 / 32,468
feature_flags ███████████████████░ 95.8% 30,674 / 32,018
revenue_analytics ███████████████████░ 95.9% 1,877 / 1,958
data_quality ███████████████████░ 96.0% 8,564 / 8,918
tracing ███████████████████░ 96.5% 4,572 / 4,739
account_audits ███████████████████░ 96.8% 783 / 809
security ███████████████████░ 96.8% 1,597 / 1,650
pulse ███████████████████░ 97.3% 2,036 / 2,093
analytics_platform ████████████████████ 98.0% 2,953 / 3,013
groups ████████████████████ 98.1% 1,623 / 1,655
field_notes ████████████████████ 98.3% 172 / 175

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

❌ Hogbox preview — build failed

The preview didn't come up for commit c0e60dd. See the build log for the failing step. It'll retry on the next push.

Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this section is the status.

Silthus and others added 10 commits November 4, 2026 14:04
…split/managed-sender-guards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rors

A transactional email that names the sandbox sender now says the sandbox sender can't send transactional email, instead of asking for a verified domain. The setup guide's Channel step no longer counts the sandbox sender as the team's own channel. The docs name WORKFLOWS_SANDBOX_SENDER_DOMAIN.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… update

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Master added sends from code (#127123), which pick any verified sender.
The sandbox row is stored as verified, so the code-send choice now skips
it like the template path does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus marked this pull request as ready for review November 4, 2026 22:54
@Silthus Silthus added posthog-review hogbox-preview Spin up a per-PR PostHog preview on a hogland hogbox labels Nov 4, 2026
@parameterai

parameterai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Risk: No findings

This delta replaces the upsert in EmailIntegration.create_native_integration with a domain-scoped advisory lock, an explicit duplicate-sender rejection, and a plain insert with an IntegrityError backstop, so a re-submitted sender can no longer silently overwrite an existing row (resetting verified and created_by) and concurrent cross-organization domain claims now serialize. I traced the locking, the duplicate-check/insert value flow, the sandbox-sender interplay, and the error messages, and found no new security issue.

Sentinel reviewed c0e60dd · Review settings

@posthog

posthog Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

PostHog Review · Deep · reviewed e3dc001

Nothing to post.

Step Status Result
Prepare the diff done 13 files, 1 chunk
Pick perspectives done 3 perspectives
Run review passes done 0 issues
Merge overlapping findings done 0 → 0
Validate findings done
Publish done Nothing to post

The dancing man in the red room from Twin Peaks

@stamphog stamphog 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.

Not approved yet — waiting on the conditions below.

PostHog Review is still reviewing this commit, so stamphog does not approve yet. The review runs again on the next push, or you can re-request it once that review finishes.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 129L, 12F substantive, 424L/21F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (424L, 21F, cross-cutting, feat)
stamphog 2.7.0 .stamphog/policy.yml @ e3dc001 · reviewed head e3dc001

stamphog[bot]

This comment was marked as outdated.

stamphog[bot]

This comment was marked as outdated.

Silthus and others added 3 commits November 6, 2026 14:16
Covers the sandbox-sender message on make-transactional and template
updates, runs the real tenant-state sync in the sweep test with only SES
mocked, and drops the forwarding is_sandbox_sender property.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…split/managed-sender-guards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…split/managed-sender-guards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stamphog
stamphog Bot dismissed their stale review November 6, 2026 19:00

New commits were pushed — dismissing the stamphog approval from an earlier head. Re-add the trigger label to request a fresh review.

Silthus and others added 4 commits November 6, 2026 19:55
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t state

The mocked SES client returned a MagicMock for the tenant lookup, so the sweep
went on to apply mock values to the own sender's team. Return None so the test
only checks which teams the sweep looks up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…split/managed-sender-guards

# Conflicts:
#	products/workflows/backend/services/data_suggestions/team_context.py
stamphog[bot]

This comment was marked as outdated.

@stamphog
stamphog Bot dismissed their stale review November 7, 2026 00:47

New commits were pushed — dismissing the stamphog approval from an earlier head. Re-add the trigger label to request a fresh review.

stamphog[bot]

This comment was marked as outdated.

…split/managed-sender-guards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stamphog
stamphog Bot dismissed their stale review November 9, 2026 07:40

New commits were pushed — dismissing the stamphog approval from an earlier head. Re-add the trigger label to request a fresh review.

@veria-ai

veria-ai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Reviews

@Silthus

Silthus commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Replaced by #131840 in the isolated sandbox rebuild.

@Silthus Silthus closed this Nov 9, 2026

This branch had an error being deployed

1 failed deployment
hogbox-preview — c0e60dde Deployed Nov 9, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hogbox-preview Spin up a per-PR PostHog preview on a hogland hogbox

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant