Skip to content

feat(workflows): restrict sandbox email to verified org members - #434

Closed
Silthus wants to merge 2 commits into
workflows-sandbox/validationfrom
workflows-sandbox/recipients
Closed

Silthus wants to merge 2 commits into
workflows-sandbox/validationfrom
workflows-sandbox/recipients

Conversation

@Silthus

@Silthus Silthus commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Problem

A team that has not verified its own domain sends from PostHog's shared sandbox sender. Without a recipient check, a workflow could send that email to anyone, such as customers or an imported list. Complaints from those recipients would lower the reputation of the shared sender for every team that uses it.

The sandbox sender is for test email to the team itself. So every recipient, including CC and BCC, must be a verified member of the project's organization.

Staff can deactivate an organization in Django admin. Until now that locked only the app and the API: events still arrived, and a live workflow kept sending sandbox email. Deactivation now also stops sandbox email, which gives a per-organization kill switch with an existing admin tool.

Supersedes PostHog#111483 (left open as the original draft)

Stack: layer 4 of 12, stacked on #430. Next: #441.

This fork PR moves to an upstream PostHog/posthog PR after review.

Caution

🔴 Merge risk: high · one-way door (merge-risk rubric)

  • Recipient parsing and membership checks decide who receives real email. A mistaken allowlist or mailbox parse cannot recall delivery.
  • The worker switch defaults off. Primary reads, bounded snapshots, final checks, and send-path tests restrict To, CC, and BCC.
  • The deactivation check treats only an explicit false as deactivated. A NULL is_active still sends, so organizations that were never deactivated keep their sandbox email.

Changes

  • A deactivated organization (Organization.is_active = false) has no verified members, so the sandbox sender blocks every send within the one-minute member snapshot. Only an explicit false deactivates, as in the web middleware; NULL counts as active. Own-sender email is unchanged.
  • Organization snapshots retain their expiry through member refreshes. A mapping that expires while members load skips the send.
  • Blocked analytics sample at most one event per team, reason, and send kind (test or live) per hour; every invocation retains its run log.
  • Sandbox sends require active organization membership and a verified email address for every recipient, including test sends.
  • A blocked recipient who is an active member with an unverified email address gets guidance to verify that address, not a domain. The skip reason is recipient_unverified, also a new reason value on the workflows sandbox email blocked event.
  • A recipient without an email address skips as a missing recipient, before the membership check runs.
  • Membership snapshots expire within one minute. A final check runs immediately before SES delivery and fails closed on unavailable membership data.
  • Recipient names and copied-address lists are normalized before sending; UTM preparation preserves the checked payload.
  • Blocked-address logs retain every address and the guidance when one message would be truncated.

Before:

flowchart LR
    Send[Sandbox email] --> Route[Fixed identity] --> SES[SES 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 Send phYellow;
    class Route phBlue;
    class SES phRed;
Loading

After:

flowchart LR
    Send[Sandbox email] --> Members[Check all recipients] --> Route[Normalize identity and content] --> Recheck[Recheck membership] --> SES[SES delivery]
    Members --> Block[Explain blocked addresses]
    Recheck --> Block
    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 Send,Block phYellow;
    class Members,Route,Recheck phBlue;
    class SES phRed;
Loading

Known follow-ups (found in the UI walk, not fixed here):

  • A sandbox test send to a non-member on a domain without mail servers shows the hard-bounce reason, not the members-only reason, because the mail-server check runs first. The send is refused either way.
  • A send blocked because the organization is deactivated reads like a membership problem. The run log lists the verified member as a blocked address and says to verify a domain, and the workflows sandbox email blocked event reports recipient_not_member. So the team's run log and any Security review of blocked events cannot tell a deactivated organization from an outsider. A distinct reason, such as organization_deactivated, is a follow-up.

How did you test this code?

Current evidence, 2026-10-26, head 222ac4e5617.

Lane gate: Node tsc clean; prettier, eslint and oxlint clean on the 2 changed files; email.service.test.ts and organization-members.service.test.ts: 204 passed at this head, 236 passed at the restacked tip 8418420305e. The strict pre-push preflight reported no failures on every pushed layer. Fork CI ran only its label and sync checks; application CI does not run on the fork, so it is a gap.

Owed proofs (fleet proof gate: proof, owed and quality pass; review fails, see Open below):

  • Live run at this PR's own head (8 of 8 pass): the real cdp-api test-run endpoint, a test Postgres and a local SES fake.
    • A verified member gets email from the fixed identity.
    • An outsider on To, CC or BCC, or an unverified member, gets the block guidance and no SES request.
    • Own-sender sends still go out.
    • An organization with is_active NULL sends. Once deactivated, a new worker sends nothing, and a worker that loaded members earlier stops after the one-minute snapshot (it still sent once inside the window). Own-sender email from the deactivated organization still goes out, and reactivating sends again.
  • Shadow diff, base feat(workflows): validate sandbox sender use at save time #430 f279cc92971 against this head over the same 14 sends: 8 are identical, and the 6 that differ are each a send this PR blocks (outsiders, an unverified member, a deactivated organization). A second run over every organization in the dev database compared the old and new member queries. The only lost row belonged to the deactivated organization, and a NULL organization kept all its members.
  • Prod shape: gap. Measuring the share of stored organizations with is_active true, NULL or false needs read-only production access, which arrives on 2026-11-02 (accepted by standing rule).

Gaps: real AWS SES delivery (every run uses a local SES fake), clustered Valkey, the production data shape, and application CI on the fork.

Test seams for the deactivation check: EmailService.executeSendEmail against the test Postgres: the "blocks all delivery for %s / %s" table gains a deactivated-organization row (red before the fix: one SES call), and "sends to a verified member of an organization that was never deactivated" covers is_active NULL and true.

Evidence at head 30fb28b0ec9, 2026-10-24

Evidence at head 30fb28b0ec9, 2026-10-24.

Owed proofs:

  • Live run: rerun at the stack tip 8ac8853cc82, which contains this layer. The stack's cdp-api sent through the workflow test-run endpoint to a local SES fake that records each request. 9 of 9 cases pass. 12 of 13 recorded cases match the run at fa33d78fb36. Case 2c (an unverified member on To) now shows the verify-your-email guidance with reason recipient_unverified, and makes no SES request. For this layer: cases 2a, 2b and 2d (an outsider on To, a non-member on CC, a non-member on BCC) skip with members-only guidance, and case 2c (an unverified member) skips with the verify-your-email guidance. None makes an SES request. Case 1 delivers to a verified member.
  • Shadow diff: carried from the stack tip fa33d78fb36, because round 4 does not touch the own-sender path or the base worker. Own-sender SES requests from fork master 448d802 and that tip are identical across 7 cases (stored flows, editor drafts, a custom From name, unverified and missing senders), apart from the per-run signatures of tracking tokens. One intended difference: an editor test run with an empty recipient reports skipped instead of success, because feat(workflows): test templates with the sandbox sender #451 reports skipped email steps in test runs. Live sends are unchanged.

Changes in this layer since the 2026-10-19 heads (restacked onto fork master 448d802: cdp-services.ts imports only):

  • Round 1 fixed: an empty To skips as a missing recipient before the membership check, as master's empty-recipient gate does (skips a sandbox send to a person without an email address as a missing recipient).
  • Round 2 fixed: the unreachable '(empty address)' fallback in the blocked-address log is removed. The existing suites cover it.
  • Final read: GPT-6.1 Sol read the G2 layer range-diffs and found nothing.
  • Round 4 fixed (UI-F3): the members snapshot keeps verified and unverified active members. When every blocked address belongs to an unverified member, the skip reason is recipient_unverified and the message asks the user to verify their email address, not a domain (email.service.test.ts "blocks all delivery for %s / %s", unverified and legacy cases, red with 2 failed, then green).
  • Final read 2: GPT-6.1 Sol read the round 4 range-diffs per group and found nothing.
Local gate at the stack tip 8ac8853cc82, shared by every layer

Test seams: membership lookup public methods and EmailService delivery outcomes, SES commands, run logs and telemetry.

Test rationale: Existing membership cases now use an injected clock and cover cached organization lookups. Reduced matrices retain one explicit test-send case per gate; hourly event cases verify bounded analytics.

Gaps: real AWS SES delivery and the AWS-side tenant pause (every proof uses a local SES fake or an unreachable SES), clustered Valkey, the production data shape (prod-shape gap until production read access on 2026-11-02), and fork CI, which fails during setup: path selection fails without a GitHub App client ID, and the Security aggregate rejects its fork-skipped path-filter job. Application CI is a verification gap.

Earlier evidence (superseded heads)

Round 3 heads, 2026-10-23. This layer's head: ea5c03c1ad3. Stack tip fa33d78fb36.

Whole-stack QA swarm, 2026-10-19. Reviewed tip cfee7e148ae. This layer's head: 58c7c938739.

  • Live run at b9bd346ed0e (this stack's cdp-api against a local SES fake, workflow test-run endpoint): a sandbox send to a verified member went out from the fixed identity through the sandbox tenant and configuration set. The step's From name, From address and Reply-To were ignored with a log line. A non-member on CC and an unverified member were blocked with the clear message. A paused tenant state skipped sandbox sends while own-sender sends continued.
  • Shadow diff: an own-sender send produced the same SES request and the same stored message on master and on this stack.
  • Cross-version: a worker on master reports a sandbox send as sent without calling SES. See the deploy order in feat(workflows): provision a managed sandbox email sender #426's merge risk.
  • Checks at the tip (Node suites at 1a0fe3663c2; later fixes touch no Node code): Node CDP messaging, members and rate limiter suites (including email-tracking.service.test.ts, which now runs), the cdp-api suite, the Python sandbox, tenant, tier, reputation and validation suites, the frontend Jest suites of this stack, ruff, and the strict pre-push preflight: pass.
  • Fixed in this layer: a live block right after a test block with the same reason now emits its own workflows sandbox email blocked event (the hourly sample key includes the send kind). The extended dedupe test failed before the fix.
  • Gaps: real AWS SES delivery, clustered Valkey, and the production data shape (no production read access yet). Fork CI still fails during setup.

Rebased onto master 734468db6f9 (2026-10-19), head fe5078a82f5.

  • Drift: Master's email service now applies the sender's default Reply-To (paramsWithReplyTo) to the step parameters.
  • Adapted: The default Reply-To now builds on this layer's recipient-normalized deliveryParams. Sandbox sends still drop Reply-To in withIdentificationFooter.
  • This layer: Node messaging, managers, and rate-limiter suites at this layer: 11 suites, 300 passed.
  • Gaps at modernize time: email-tracking.service.test.ts did not run then. It runs now; see the QA swarm block above. Fork CI still fails during setup, as noted below. Real AWS delivery and clustered Valkey remain unchecked.
Checks at the stack tip

Stack tip b9bd346ed0e (#456), shared by every layer:

  • Python, 354 passed: sandbox sender, sender validation, CDP validation, reputation, SES tenant state and events, sending tier.
  • Node CDP messaging, managers, and rate limiter: 17 suites, 599 passed.
  • Frontend Jest, 37 suites, 487 passed. Five suites timed out under host load in the batch run and passed when rerun alone.
  • Frontend tsgo: no errors. Node tsgo: no errors in CDP. The 23 errors it reports are all in session replay files that need the unbuilt Rust @posthog/replay-anonymizer module.
  • ruff, oxfmt, oxlint, prettier, eslint, product:lint (workflows, cdp), IDOR coverage, hogli build:openapi (no diff), and the strict pre-push preflight: pass.
  • Review: GPT-6.1 Sol read every conflict resolution in the stack and found no issues.
Observed round-two snapshot verification
Red: an expired warmed organization mapping allowed a refreshed member response
organization-members.service.test.ts: 3 passed
Affected delivery freshness and transfer cases: 6 passed
Node.js typecheck and build: exit 0
Layer strict preflight: failures 0
Pre-push cumulative strict preflight: failures 0
Observed targeted verification after QA fixes
organization-members.service.test.ts: 2 passed
email.service.test.ts -t 'sandbox sender': 65 passed, 116 skipped
Observed combined-stack gate
Node.js typecheck and build: exit 0
Frontend typecheck and build: exit 0
Success: no issues found in 23388 source files
product:lint workflows: All checks passed
ci:preflight --strict: failures 0

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

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

Docs update

Provisioning uses workflows-sandbox-sender; delivery uses the default-off worker switch WORKFLOWS_SANDBOX_SENDER_ENABLED.

Keep provisioning and delivery switches off until daily caps and sandbox tenant pause enforcement are deployed.

The workflow email sending tiers document describes eligible recipients, bounded membership snapshots, and blocked-address logs.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Decisions

Question Answer Who Where
Per-organization kill switch for sandbox email Deactivating the organization in Django admin (is_active = false) stops its sandbox email 👤 Human pending approval
Which is_active values deactivate Only an explicit false; NULL counts as active, as in the web middleware 👤 Human pending approval
Block reason for a deactivated organization Keep the existing recipient_not_member path, one-line change; a distinct reason is a known follow-up 👤 Human pending approval
Copy for a member whose own email address is not verified (UI-F3) Tell them to verify their email address, not a domain. Replaces the recommendation to keep the domain wording until the prod-shape read 👤 Human decision comment
Blocked analytics sampling Keep the hourly sample, keyed by team, reason, and send kind. Alternative: one event per blocked send, as the spec words it 🤖 Agent QA handoff
Gate order for a non-member on a domain without mail servers (UI-F1) Keep the mail-server check first. The send is refused either way; reordering the gates is a follow-up with its own proof 🤖 Agent decision comment
Test-only cleanup of the recipient and cap tests (G2-4) Moved to a test-only follow-up PR on the stack tip. Folding it into #434 and #441 meant rewriting each later layer's tests by hand 🤖 Agent decision comment
Structural rewrites of the recipient gate (G2-7, G2-9, G2-10, G2-11) Follow-up PRs with their own proof: a typed delivery envelope, nodemailer addressparser, simpler block logs, and the email service split. Review found no behavior defect in the current gate 🤖 Agent decision comment
Worker skip reason for unverified members When every blocked address belongs to an unverified member, the skip reason is recipient_unverified and its message says to verify the email address. The worker does not know who sent the test, so it addresses both the sender and the members 🤖 Agent decision comment

Open: fleet proof gate fails its review check because round 1 findings G2-4 (singleton test matrices, an unreachable cleanup branch) and G2-12 (the organization-refresh test asserts query calls) are deferred, not fixed. Recommendation: fix both as a test-only change in this layer and restack, since later layers leave those tests unchanged. This was held back because the deactivation change was asked to stay narrow.

Involvement: 2026-10-26 deactivation change with review round 3 by GPT-6.1 Sol (no findings), a live run at this head and a base/head shadow diff, then a restack of #441 to #456; earlier: restack onto fork master 448d802 on 2026-10-23; round 1 with GPT-6.1 Sol lanes plus Claude Opus 5.5 fresh eyes for group G2; round 2 on the fix diff; final read 1 by GPT-6.1 Sol per group, with no findings; proofs captured; final read 2 by GPT-6.1 Sol on the round 4 decisions, with no findings; the round 4 live run and a UI walk of the two changed surfaces. Before that: the 2026-10-19 whole-stack QA swarm and the modernize rebase (see Earlier evidence). No review bots run on the fork.

Agent: Codex with GPT-6.1 Sol (gpt-6.1-sol) built the original layers, orchestrated by Claude Code with Claude Opus 5.5 (claude-opus-5-5). Restack, review fixes, the deactivation check and this description refresh: Claude Opus 5.5 (claude-opus-5-5). Review: GPT-6.1 Sol (gpt-6.1-sol). Fresh eyes: Claude Opus 5.5 (claude-opus-5-5).

Skills: tdd, pr, stacking-prs, writing-pr-descriptions, merge-risk, writing-tests, writing-code-comments, running-ci-preflight, and writing-voice. The description refresh used pr-description, writing-pr-descriptions, writing-voice and merge-risk.

The rebuild preserves master's UTM tagging and throttle retries. The UTM payload now retains recipient normalization introduced by this layer. Public repository sources and invented test addresses informed the change.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus force-pushed the workflows-sandbox/recipients branch from ea5c03c to 30fb28b Compare October 24, 2026 13:12
@Silthus

Silthus commented Oct 24, 2026

Copy link
Copy Markdown
Owner Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decision lines for this PR. The Decisions table in the description links each row here.

  • 👤 Copy for a member whose own email address is not verified (UI-F3) Tell them to verify their email address, not a domain. Replaces the recommendation to keep the domain wording until the prod-shape read.
  • 🤖 Gate order for a non-member on a domain without mail servers (UI-F1) Keep the mail-server check first. The send is refused either way; reordering the gates is a follow-up with its own proof
  • 🤖 Test-only cleanup of the recipient and cap tests (G2-4) Moved to a test-only follow-up PR on the stack tip. Folding it into feat(workflows): restrict sandbox email to verified org members #434 and feat(workflows): cap sandbox sends and refund ses throttles #441 meant rewriting each later layer's tests by hand
  • 🤖 Structural rewrites of the recipient gate (G2-7, G2-9, G2-10, G2-11) Follow-up PRs with their own proof: a typed delivery envelope, nodemailer addressparser, simpler block logs, and the email service split. Review found no behavior defect in the current gate
  • 🤖 Worker skip reason for unverified members When every blocked address belongs to an unverified member, the skip reason is recipient_unverified and its message says to verify the email address. The worker does not know who sent the test, so it addresses both the sender and the members.

A deactivated organization now has no verified members, so the sandbox
sender blocks every send within the one-minute member snapshot window.
Only an explicit false deactivates, matching the web middleware.

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

Silthus commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Replaced by the re-sliced fork stack: #594. Tracked in Silthus/personal#71.

@Silthus Silthus closed this Oct 27, 2026
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