Skip to content

Sandbox sender: block sends to anyone outside the organization #233

Closed

Description

@Silthus

Map: Map: let teams send a first email before verifying their domain
Spec: Spec: sandbox sender

Cast: GPT-6.1 Sol (Michael, 2026-10-03)

Question

Can the send path block every sandbox send that has a To, CC or BCC address outside the team's verified organization members?

Delivers end to end. Spec section "Send path", "Recipient allowlist":

  • An address passes only if it belongs to an active user who is a member of the team's organization and has is_email_verified = true (legacy null does not pass). Compared bare and lower-cased.
  • Any blocked address blocks the whole send. Never rerouted. A lookup error blocks the send.
  • Members are cached per organization for at most one minute.
  • Log line names the blocked addresses and says that verifying the project's own domain lifts the restriction (/writing-user-facing-copy).
  • skipped = true, { success: false } on the VM stack, workflows sandbox email blocked with reason = recipient_not_member (or check_failed) and blocked_recipient_count, never the addresses.
  • Applies to test sends and live sends alike.

Test seams. EmailService.executeSendEmail with real Postgres user, organization and membership fixtures, sesV2Client.send mocked. Cases: member To; non-member To; member To with a non-member CC; non-member BCC; unverified member; null verification; member of another organization; lookup failure.

Write scope. A new organization-member lookup service under nodejs/src/cdp/services/managers/, its wiring into EmailService and the CDP hub dependencies, email.service.ts, and tests.

Acceptance criteria

  • Each case above behaves as stated, verified through the SES mock and the run log.
  • A membership change shows up within a minute.
  • Own-sender sends never query organization members.

How to build (map #225)

  • /tdd red-green-refactor, outside-in, at the seams named above. Real Postgres, Valkey and ClickHouse; mock only the SES client (Node and boto3) and the PostHog analytics client.
  • Repo skills apply where the diff touches their area: /writing-tests, /improving-drf-endpoints, /django-migrations, /writing-ui-components, /writing-kea-logics, /writing-user-facing-copy, /writing-code-comments.
  • Public repo: state sandbox rules as product rules. Cap values come from the private note the map points at; never quote it. Fixture addresses use example.com.
  • Shipped means a draft PR handed off by /qa-swarm (see the map's Notes for remotes and branches). Never mark ready, enqueue or merge.

Activity

  1. self-assigned this
    on Oct 4, 2026
  2. Silthus commented on Oct 4, 2026

    @Silthus
    OwnerAuthor

    Note

    🤖 GPT-6.1 Sol responding on behalf of Michael

    Runtime: T3 Code app-owned child, Codex GPT-6.1 Sol
    Task: node:delegated-task:command%3Amcp%3Abaef64af-4783-4753-823f-c9d15c6a1fe6%3Adelegate-task%3Asandbox233-org-recipients-20261004
    Child: thread:delegated-task:command%3Amcp%3Abaef64af-4783-4753-823f-c9d15c6a1fe6%3Adelegate-task%3Asandbox233-org-recipients-20261004
    Worktree: /home/coder/.t3/worktrees/posthog/sandbox-233-recipients
    Branch: feat/workflows-sandbox-recipients
    Base: 0007a4f from draft PR PostHog#111401
    Cap: four hours, self-deleting deadline wake
    CodeRabbit slot: 1 of 3

    The sandbox route shipped as a draft with a ready QA handoff. This worker now owns recipient membership checks, real-fixture test-first proof and a fresh draft-PR QA handoff for this ticket alone. Pause and tier-history slices touch the same sending path and will follow serially. Parent owns the map and downstream dispatch; no merge authority.

  3. Silthus commented on Oct 4, 2026

    @Silthus
    OwnerAuthor

    BLOCKED handoff from GPT-6.1 Sol: draft PR #111483, stacked on #111401, is at c64bd97aa69269c96932f3ae059b04efc85dafc9.

    The committed local gate passes 217 tests and five unchanged own-sender snapshots, Node-wide TypeScript, scoped lint/security checks and strict preflight. Final send-method observation captured accepted delivery and whole-send blocking with real organization/member fixtures and mocked external SES/analytics.

    The fourth fresh QA round found a valid verified-member quoted address that is incorrectly blocked in CC/BCC display-name syntax. A real-member reproduction confirms this in all four live/test CC/BCC cases. The requested four-round limit stops further fixes and review here, so this issue remains open.

    Fork code Actions still require maintainer approval. The latest ancillary reviewer workflow skipped; an earlier zero-job startup failure remains an infrastructure gap. Hosted CodeRabbit reported no actionable comments for the latest increment, but the reproduced QA finding remains.

    May this ticket receive one additional fix-and-QA round for quoted-address parsing? The recommendation is yes, while keeping the PR draft until a fresh round clears the fix.

  4. Silthus commented on Oct 4, 2026

    @Silthus
    OwnerAuthor

    Claim: impl-233-r5 (round 5 of a raised cap of 5)

    Michael answered the park with "Fix it": one additional bounded fix-and-fresh-QA round is authorized for the quoted-address parser (R4-01). This is round 5 of a raised cap of 5. If round 5 surfaces another new fix item, this ticket hands off blocked without a further fix.

    • Worker: impl-233-r5, starting from head c64bd97aa69.
    • Cap: 4 hours from 10:00 UTC.
    • CodeRabbit slot: 1 of 3.
    • Casting: implementation runs on Claude Opus 5.5 instead of GPT-6.1 Sol, because Codex is rate-limited right now. Native Claude Code agents only for non-UI work.
    • Scope: fix R4-01 test-first at the email service seam (live and test sends, CC and BCC), then a cold full qa-swarm panel on the new head. R4-02 (footer organization label) stays a parent-layer follow-up.
  5. Silthus commented on Oct 4, 2026

    @Silthus
    OwnerAuthor

    Park: impl-233-r5, round 5 of 5, blocked

    The R4-01 quoted-address fix landed, but round 5 found a new fix item. That makes this round blocked, as agreed, and no further fix was made.

    PR #111483 is a draft at 720632842ed.

    R4-01, fixed in 720632842ed

    • CC/BCC splitting and angle-address extraction are now quote-aware, so Name <"a<b>"@example.com> resolves to the member mailbox.
    • The existing quoted-mailbox matrix gained a display-wrapped dimension. Its eight wrapped cases (live/test × CC/BCC × suppressed) failed on c64bd97aa69 and pass now.
    • The local gate passes: 225 tests, five unchanged snapshots, Node TypeScript, ESLint, Oxlint, Semgrep, and strict preflight.

    Round 5 panel

    • All seven lenses reported back with full coverage.
    • Correctness, Security and Production found nothing.
    • Codex returned 429 twice and Fable once, so Opus 5.5 and Sonnet 5.5 stood in for them. This round had no GPT reviewer.

    Locked decisions

    • R4-01 is fixed.
    • The round count stays at 5.
    • Three nits are rejected or moved to follow-ups: two test nits rejected, three design nits (reason union, checkDeadline name, suppression override) moved to follow-ups.
    • R4-02 (the footer organization label after a project transfer) stays a parent-layer follow-up.

    Open items

    • R5-01 (fix, tests): member emails stored with uppercase letters are folded case-insensitively, but no test covers it. With the fold removed, all 219 email service tests still pass. Accounts created before signup lowercased emails can still hold mixed case. The behavior at this head is correct; only the test is missing.
    • R5-02 (decision, logs): three lenses flagged this independently. A very long blocked CC/BCC list writes one log row per address. My recommendation is to pack the addresses into as few full-length log entries as possible. Every address stays visible, and the row count then follows the total characters rather than the address count.

    Gaps

    • CodeRabbit auto-paused on this head. Triggering it needs a PR comment, which was not posted.
    • Fork code CI is waiting for approval.

    One pending question: may I run one more bounded round to add the R5-01 test and pack the R5-02 log rows, followed by a cold QA round? The alternative is to take the PR to review as it is, with both items listed as follow-ups.

  6. Silthus commented on Oct 4, 2026

    @Silthus
    OwnerAuthor

    Conductor: park verified (comment, wayfinder:awaiting-human, PR body at 720632842ed). Worker impl-233-r5 ended on a provider rate limit after posting it; the lane is retired. CodeRabbit slot 1 released. A successor starts only on Michael's answer.

  7. Silthus commented on Oct 5, 2026

    @Silthus
    OwnerAuthor

    Start: impl-233-r6, round 6 (raised cap)

    Michael answered the round 5 park: "Yes, run round 6 and GPT is back."

    That authorizes one more bounded fix-and-cold-QA round on PR #111483, starting from 720632842ed.

    Scope

    • R5-01 (fix, tests): add a stored-mixed-case member dimension to the case-insensitive membership matrix. A mutant without the case fold must fail the new cases.
    • R5-02 (decision, as recommended): pack the blocked CC/BCC addresses into as few full-length log entries as possible. Every address stays visible, and the row count follows the total characters. Tested at the public send seam with a long blocked list.

    Nothing else. R5-05 to R5-07 and R4-02 stay follow-ups. If round 6 finds another new fix item, I hand off as blocked and park without fixing it.

    Setup

    • Worker: impl-233-r6, Claude Opus 5.5 as lane owner.
    • Cap: 4 hours from 03:56 UTC. Round count 6.
    • CodeRabbit: slot 1 of 3. I resume the paused bot with one review request on the new head.
    • Casting: GPT-6.1 Sol through Codex for implementation and review lanes, Claude for the second family. At most 2 reviewers at once.
  8. Silthus commented on Oct 5, 2026

    @Silthus
    OwnerAuthor

    Park: impl-233-r6, round 6, blocked

    Both round 5 items are fixed, but round 6 found one new fix item. As agreed, that blocks the round, and I made no further fix.

    PR #111483 is a draft at c267c6d8d43.

    Fixed in c267c6d8d43

    • R5-01: the case-insensitive membership matrix now also stores member emails with mixed case. With the stored-side case fold removed, its eight new cases fail.
    • R5-02: a long blocked CC/BCC list now fills each log row with as many addresses as fit. Every address stays visible. 200 blocked addresses now write 5 rows instead of 200.
    • The local gate passes: three Node suites, unchanged snapshots, Node TypeScript, ESLint, Oxlint, Prettier, Semgrep, and strict preflight.

    Round 6 panel

    • All seven lenses reported back with full coverage. GPT-6.1 Sol ran six lenses and Claude Sonnet 5.5 ran fresh eyes. No lane hit a rate limit.
    • Correctness, Security, Production, Design and Adversary found nothing.
    • Hosted CodeRabbit reviewed c267c6d8d43 and reported no actionable comments.

    Locked decisions

    • R5-01 and R5-02 are fixed. The round count stays at 6.
    • R6-02 (fresh eyes nit: two ways to check that a log row fits) joins the follow-ups with R5-05 to R5-07 and R4-02.

    Open item

    • R6-01 (fix, tests): nothing tests the guard that blocks a send when the member lookup itself takes a minute or more. With the guard removed, all 115 sandbox tests still pass. Without it, a very slow lookup could approve a send from membership data older than one minute. The behavior at this head is correct; only the test is missing. The fix is one clock-controlled live/test case through the send path, proven with the same mutant.

    Gaps

    • Fork code CI is waiting for maintainer approval.

    One pending question: may I run one more bounded round to add the R6-01 test, followed by a cold QA round? The alternative is to take the PR to review as it is, with R6-01 listed as a follow-up.

  9. Silthus commented on Oct 5, 2026

    @Silthus
    OwnerAuthor

    Conductor: round 6 park verified (comment, wayfinder:awaiting-human, PR body at c267c6d8d43, clean tree). Worker impl-233-r6 is retired and CodeRabbit slot 1 is released. A successor starts only on Michael's answer.

  10. Silthus commented on Oct 5, 2026

    @Silthus
    OwnerAuthor

    Start: impl-233-r7, round 7 (raised cap)

    Michael answered the round 6 park: "Yeah, go ahead."

    That authorizes one more bounded fix-and-cold-QA round on PR #111483, starting from c267c6d8d43.

    Scope

    • R6-01 (fix, tests only): add a clock-controlled live/test case through the send path where the member lookup takes a minute or more. It must show that SES is not called, the send is skipped, the step result is false, and check_failed counts every unchecked recipient. With the deadline guard removed, the new case must fail.

    The production code stays as it is unless the red run shows a real defect. Nothing else is in scope. R6-02, R5-05 to R5-07 and R4-02 stay follow-ups. If round 7 finds another new fix item, I hand off as blocked and park without fixing it.

    Setup

    • Worker: impl-233-r7, Claude Opus 5.5 as lane owner.
    • Cap: 4 hours from 05:47 UTC. Round count 7.
    • CodeRabbit: slot 1 of 3. I resume the paused bot with one review request on the new head.
    • Casting: GPT-6.1 Sol through Codex for implementation and review lanes, Claude for the second family. At most 2 reviewers at once.
  11. Silthus commented on Oct 5, 2026

    @Silthus
    OwnerAuthor

    Resolution: impl-233-r7, round 7, ready

    Round 7 came back clean. PR #111483 is a draft at 754efd702a4, ready for Michael's review.

    Fixed in 754efd702a4 (test only)

    • R6-01: the lookup-failure case now also runs a member query that takes a minute. It expects no SES call, a skipped send, a false step result, and check_failed for every unchecked recipient. Own senders still bypass the check.
    • With the deadline guard removed, the two new cases fail because SES is called. At this head, all 117 sandbox cases pass. The production code did not change.
    • The local gate passes: three Node suites, unchanged snapshots, Node TypeScript, ESLint, Oxlint, Prettier, Semgrep, and strict preflight.

    Round 7 panel

    • All seven lenses reported back with full coverage and no findings. GPT-6.1 Sol ran six lenses and Claude Sonnet 5.5 ran fresh eyes. No lane hit a rate limit.
    • Hosted CodeRabbit reviewed 754efd702a4 and reported no actionable comments. Its shared-sender volume note is the activation prerequisite that the parent layer and later tickets own.

    Calls

    • The new case extends the existing lookup-failure test as a second failure mode instead of adding a standalone test.

    Follow-ups

    • R6-02: two ways to check that a log row fits.
    • R5-05 to R5-07: design tidy-ups in the block reason, the deadline name, and the suppression reason helper.
    • R4-02: the footer organization label after a project transfer (parent layer).

    Gaps

    • Fork code CI is waiting for maintainer approval, so it is not green yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions