From 686d6bd5f27c1061e0b90be961de97a0e485d5b6 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:36:25 +0000 Subject: [PATCH 01/17] feat(cdp): skip sandbox sends of a blocked organization The email worker reads SandboxSenderOrganizationBlock in the team organization lookup. While a block row exists, the organization's sandbox sends skip with the reason organization_blocked and a run-log message that points to support. The check runs inside the recipient check, so a blocked organization spends no daily caps and the final check before sending also blocks. A block reaches the workers within one minute. Own-sender sends do not read it. Co-Authored-By: Claude Opus 5.5 --- .../internal/workflows-email-sending-tiers.md | 3 ++ .../managers/organization-members.service.ts | 27 +++++++++-- .../services/messaging/email.service.test.ts | 47 +++++++++++++++++++ .../cdp/services/messaging/email.service.ts | 32 +++++++++++-- .../messaging/sandbox-email-sender.ts | 1 + .../rate-limiter/rate-limiter.service.test.ts | 12 ++++- 6 files changed, 111 insertions(+), 11 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 420cfec8fdd0..4fe2f428974e 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -80,6 +80,9 @@ Decay, suspension drops, admin recomputes, and the backfill stay silent. Every sandbox send, including test sends, skips when that tenant's mirrored sending status is `DISABLED` or the pause lookup fails. A missing row allows sending, with SES enforcing its own state. The worker rechecks pauses after the daily-cap claim and returns the claim when that recheck blocks delivery. A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Own-sender sends do not read this state. + Staff turn the sandbox sender off for one organization by adding a `SandboxSenderOrganizationBlock` row in the Django admin, and lift the block by deleting the row. + A blocked organization's sandbox sends skip with the reason `organization_blocked`, and its projects get no new sandbox sender. + The block rides on the organization snapshot, so it reaches the workers within one minute. Own-sender sends do not read it. Test sends run inline in the CDP API, so the API reads the same `SES_RATE_LIMITER_VALKEY_*` settings as the email worker. Without them, every sandbox test send is skipped. CDP API opens the SES Valkey pool only when sandbox delivery is enabled; otherwise its MX validation uses the existing local cache and DNS. The CDP API's SES webhook captures `workflows sandbox email delivered` for a delivery on `SES_SANDBOX_CONFIGURATION_SET`, test sends included. Without that setting on the CDP API, the event never fires. diff --git a/nodejs/src/cdp/services/managers/organization-members.service.ts b/nodejs/src/cdp/services/managers/organization-members.service.ts index 9c39ab4194ee..7d73bd1430f1 100644 --- a/nodejs/src/cdp/services/managers/organization-members.service.ts +++ b/nodejs/src/cdp/services/managers/organization-members.service.ts @@ -9,7 +9,11 @@ type OrganizationMemberSnapshot = { export type BlockedRecipient = { readonly address: string; readonly unverifiedMember: boolean } -export type TeamOrganizationSnapshot = { readonly organizationId: string; readonly expiresAt: number } +export type TeamOrganizationSnapshot = { + readonly organizationId: string + readonly sandboxSenderBlocked: boolean + readonly expiresAt: number +} const MAX_SNAPSHOT_AGE_MS = 60_000 const REFRESH_AGE_MS = MAX_SNAPSHOT_AGE_MS - 1_000 @@ -29,14 +33,29 @@ export class OrganizationMembersService { bufferMs: 0, loader: async (teamIds) => { const expiresAt = this.now() + MAX_SNAPSHOT_AGE_MS - const { rows } = await postgres.query<{ id: number; organization_id: string }>( + const { rows } = await postgres.query<{ + id: number + organization_id: string + sandbox_sender_blocked: boolean + }>( PostgresUse.COMMON_WRITE, - 'SELECT id, organization_id FROM posthog_team WHERE id = ANY($1::bigint[])', + `SELECT team.id, team.organization_id, block.id IS NOT NULL AS sandbox_sender_blocked + FROM posthog_team AS team + LEFT JOIN workflows_sandboxsenderorganizationblock AS block + ON block.organization_id = team.organization_id + WHERE team.id = ANY($1::bigint[])`, [teamIds.map(Number)], 'fetch-sandbox-team-organization' ) return Object.fromEntries( - rows.map((team) => [String(team.id), { organizationId: team.organization_id, expiresAt }]) + rows.map((team) => [ + String(team.id), + { + organizationId: team.organization_id, + sandboxSenderBlocked: team.sandbox_sender_blocked === true, + expiresAt, + }, + ]) ) }, }) diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index b5a365b0e39e..80961927c7eb 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -501,6 +501,53 @@ describe('EmailService', () => { ) }) + it.each([false, true])( + 'blocks every sandbox send of a blocked organization until the block is removed (isTest=%s)', + async (isTest) => { + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + `INSERT INTO workflows_sandboxsenderorganizationblock (organization_id, reason, created_at) + VALUES ($1, 'Abuse report', now())`, + [team.organization_id], + 'test:block-sandbox-organization' + ) + service = createSandboxService(true) + + const blocked = await service.executeSendEmail(invocation, isTest) + + expect(sendEmailSpy).not.toHaveBeenCalled() + expect(blocked).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(blocked.invocation.state.vmState?.stack).toEqual([{ success: false }]) + expect(blocked.logs).toContainEqual( + expect.objectContaining({ + level: 'info', + message: + 'Skipping send: the sandbox sender is turned off for this organization. Contact support to find out why.', + }) + ) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + { reason: 'organization_blocked', is_test: isTest, blocked_recipient_count: 0 } + ) + + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM workflows_sandboxsenderorganizationblock WHERE organization_id = $1', + [team.organization_id], + 'test:unblock-sandbox-organization' + ) + service = createSandboxService(true) + invocation.state.vmState = { stack: [] } as any + + const unblocked = await service.executeSendEmail(invocation, isTest) + + expect(unblocked.error).toBeUndefined() + expect(unblocked.skipped).not.toBe(true) + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + } + ) + it.each([false, true])('blocks a non-member To address (isTest=%s)', async (isTest) => { service = createSandboxService(true) invocation.queueParameters = createSandboxParams({ diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index 89bfb9edbf95..e5b19c7349dd 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -426,6 +426,9 @@ const SANDBOX_ADDRESS_BLOCK_COPY: Record< }, } +const SANDBOX_ORGANIZATION_BLOCKED_MESSAGE = + 'Skipping send: the sandbox sender is turned off for this organization. Contact support to find out why.' + const SANDBOX_PAUSE_SKIP_MESSAGES = { paused: 'Skipping send: the sandbox sender is paused right now. Verify your own domain to keep sending.', check_failed: @@ -1095,15 +1098,16 @@ export class EmailService { const invocation = result.invocation let blockedRecipients: string[] let reason: Exclude + let organizationBlocked = false try { if (!this.organizationMembers) { throw new Error('Organization member checks are not configured') } - const blocked = await this.organizationMembers.getBlockedRecipients( - invocation.teamId, - recipients, - organization - ) + const snapshot = organization ?? (await this.organizationMembers.getOrganizationSnapshot(invocation.teamId)) + organizationBlocked = snapshot.sandboxSenderBlocked + const blocked = organizationBlocked + ? [] + : await this.organizationMembers.getBlockedRecipients(invocation.teamId, recipients, snapshot) blockedRecipients = blocked.map(({ address }) => address) reason = blocked.every(({ unverifiedMember }) => unverifiedMember) ? 'recipient_unverified' @@ -1116,6 +1120,9 @@ export class EmailService { error: error instanceof Error ? error.message : 'Unknown error', }) } + if (organizationBlocked) { + return await this.skipBlockedSandboxOrganization(result, isTest) + } if (!blockedRecipients.length) { return true } @@ -1130,6 +1137,21 @@ export class EmailService { return false } + private async skipBlockedSandboxOrganization( + result: CyclotronJobInvocationResult, + isTest: boolean + ): Promise { + createAddLogFunction(result.logs)('info', SANDBOX_ORGANIZATION_BLOCKED_MESSAGE) + result.skipped = true + result.invocation.state.vmState?.stack.push({ success: false }) + await this.sandboxSender!.capture(result.invocation.teamId, isTest, { + type: 'blocked', + reason: 'organization_blocked', + blockedRecipientCount: 0, + }) + return false + } + private async claimSandboxDailyCaps( result: CyclotronJobInvocationResult, recipients: string[], diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index 234bcae3a756..84dee6486ac5 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -61,6 +61,7 @@ type SandboxEmailOutcome = | 'check_failed' | 'cap_reached' | 'paused' + | 'organization_blocked' blockedRecipientCount: number } diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts index 387ba0b030e5..227691102021 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts @@ -459,8 +459,16 @@ describe('RateLimiterService', () => { ) try { - const firstOrganization = { organizationId: organizations[1], expiresAt: Infinity } - const otherOrganization = { organizationId: organizations[3], expiresAt: Infinity } + const firstOrganization = { + organizationId: organizations[1], + sandboxSenderBlocked: false, + expiresAt: Infinity, + } + const otherOrganization = { + organizationId: organizations[3], + sandboxSenderBlocked: false, + expiresAt: Infinity, + } expect(await sender.claimDailyCaps(1, ['member@example.com'], firstOrganization)).toMatchObject({ type: 'granted', }) From 4e16aa2d0832b2435afce5fd5dfced7537576182 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:09:30 +0000 Subject: [PATCH 02/17] test(cdp): prove a running worker picks up a block and its removal Co-Authored-By: Claude Opus 5.5 --- .../services/messaging/email.service.test.ts | 85 ++++++++++--------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 80961927c7eb..b8e302600cbb 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -502,49 +502,58 @@ describe('EmailService', () => { }) it.each([false, true])( - 'blocks every sandbox send of a blocked organization until the block is removed (isTest=%s)', + 'a running worker stops sandbox sends within a minute of a block and resumes within a minute of its removal (isTest=%s)', async (isTest) => { - await hub.postgres.query( - PostgresUse.COMMON_WRITE, - `INSERT INTO workflows_sandboxsenderorganizationblock (organization_id, reason, created_at) - VALUES ($1, 'Abuse report', now())`, - [team.organization_id], - 'test:block-sandbox-organization' - ) - service = createSandboxService(true) + const setBlocked = (blocked: boolean): Promise => + hub.postgres.query( + PostgresUse.COMMON_WRITE, + blocked + ? `INSERT INTO workflows_sandboxsenderorganizationblock (organization_id, reason, created_at) + VALUES ($1, 'Abuse report', now())` + : 'DELETE FROM workflows_sandboxsenderorganizationblock WHERE organization_id = $1', + [team.organization_id], + 'test:set-sandbox-organization-block' + ) + const sendAt = (time: number) => { + clock.mockReturnValue(time) + invocation.state.vmState = { stack: [] } as any + return service.executeSendEmail(invocation, isTest) + } + const now = Date.now() + const clock = jest.spyOn(Date, 'now').mockReturnValue(now) + try { + service = createSandboxService(true) + await sendAt(now) + expect(sendEmailSpy).toHaveBeenCalledTimes(1) - const blocked = await service.executeSendEmail(invocation, isTest) + await setBlocked(true) + const blocked = await sendAt(now + 61_000) - expect(sendEmailSpy).not.toHaveBeenCalled() - expect(blocked).toMatchObject({ finished: true, skipped: true, metrics: [] }) - expect(blocked.invocation.state.vmState?.stack).toEqual([{ success: false }]) - expect(blocked.logs).toContainEqual( - expect.objectContaining({ - level: 'info', - message: - 'Skipping send: the sandbox sender is turned off for this organization. Contact support to find out why.', - }) - ) - expect(capture).toHaveBeenCalledWith( - expect.objectContaining({ id: team.id }), - 'workflows sandbox email blocked', - { reason: 'organization_blocked', is_test: isTest, blocked_recipient_count: 0 } - ) - - await hub.postgres.query( - PostgresUse.COMMON_WRITE, - 'DELETE FROM workflows_sandboxsenderorganizationblock WHERE organization_id = $1', - [team.organization_id], - 'test:unblock-sandbox-organization' - ) - service = createSandboxService(true) - invocation.state.vmState = { stack: [] } as any + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + expect(blocked).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(blocked.invocation.state.vmState?.stack).toEqual([{ success: false }]) + expect(blocked.logs).toContainEqual( + expect.objectContaining({ + level: 'info', + message: + 'Skipping send: the sandbox sender is turned off for this organization. Contact support to find out why.', + }) + ) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + { reason: 'organization_blocked', is_test: isTest, blocked_recipient_count: 0 } + ) - const unblocked = await service.executeSendEmail(invocation, isTest) + await setBlocked(false) + const unblocked = await sendAt(now + 122_000) - expect(unblocked.error).toBeUndefined() - expect(unblocked.skipped).not.toBe(true) - expect(sendEmailSpy).toHaveBeenCalledTimes(1) + expect(unblocked.error).toBeUndefined() + expect(unblocked.skipped).not.toBe(true) + expect(sendEmailSpy).toHaveBeenCalledTimes(2) + } finally { + clock.mockRestore() + } } ) From e531969fc331ae46e6467c655b1210b248b729bd Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 05:21:39 +0000 Subject: [PATCH 03/17] feat(workflows): restrict sandbox email to verified org members A send through a `sandbox` email integration now skips unless the worker switch and its three SES route settings are set, and unless every To, CC and BCC address belongs to an active, verified member of the team's active organization. The check runs again right before delivery. Each skip logs a reason and emits a deduplicated blocked event. The sandbox route does not exist yet, so a send that passes every check still skips as unavailable instead of reporting success. Co-Authored-By: Claude Opus 5.5 --- .../internal/workflows-email-sending-tiers.md | 11 + nodejs/src/cdp/cdp-services.ts | 19 +- nodejs/src/cdp/config.ts | 9 + .../organization-members.service.test.ts | 41 ++ .../managers/organization-members.service.ts | 86 ++++ .../services/messaging/email.service.test.ts | 406 +++++++++++++++++- .../cdp/services/messaging/email.service.ts | 178 +++++++- .../messaging/sandbox-email-sender.ts | 93 ++++ 8 files changed, 829 insertions(+), 14 deletions(-) create mode 100644 nodejs/src/cdp/services/managers/organization-members.service.test.ts create mode 100644 nodejs/src/cdp/services/managers/organization-members.service.ts create mode 100644 nodejs/src/cdp/services/messaging/sandbox-email-sender.ts diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 6914badeaa51..b0d50f3b664d 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -66,3 +66,14 @@ Decay, suspension drops, admin recomputes, and the backfill stay silent. A send denied because the limiter itself failed keeps the shorter token bucket cadence, at most 5 to 10 minutes. - Gmail provides no per-message feedback loop, so complaint rates (ours and AWS's) cannot see Gmail complaints at all. - The `HOGFLOW_BATCH_TRIGGER_ELEVATED_TEAM_IDS` allowlist elevates a team to the top tier in Django only: the UI, the admin, and the batch audience cap show top-tier numbers, but the worker's send-time buckets read the stored tier. An allowlisted team below the top tier is therefore throttled at its stored tier while being told otherwise, and its shadow-mode delay counts overstate real impact. Before enforcing, pin each allowlisted team at the top tier from the Django admin and empty the allowlist, rather than plumbing the list into the worker. + +### Sandbox delivery + +The worker sends through a `sandbox` email integration only when `WORKFLOWS_SANDBOX_SENDER_ENABLED` is on and `SES_SANDBOX_TENANT_NAME`, `SES_SANDBOX_CONFIGURATION_SET`, and `SES_SANDBOX_FROM_ADDRESS` are set. +Otherwise every send through it skips with a reason in the run log and a `workflows sandbox email blocked` event with reason `switch_off`. +Every To, CC, and BCC address must belong to an active member of the team's active organization with a verified email address. +Membership snapshots expire within one minute, and the worker checks recipients again right before delivery. +A failed or stale membership check skips the send and lists the blocked addresses in the run log. +The blocked event fires at most once an hour per team, reason, and send kind (workflow or test). +The sandbox route is not built yet, so a send that passes every check still skips as unavailable. +Keep the switch off in production until the sandbox caps are deployed. diff --git a/nodejs/src/cdp/cdp-services.ts b/nodejs/src/cdp/cdp-services.ts index 27b2a785b8ac..3b60a0e40a33 100644 --- a/nodejs/src/cdp/cdp-services.ts +++ b/nodejs/src/cdp/cdp-services.ts @@ -31,6 +31,7 @@ import { InvocationResultsService } from './services/invocation-results.service' import { HogFunctionManagerService } from './services/managers/hog-function-manager.service' import { HogFunctionTemplateManagerService } from './services/managers/hog-function-template-manager.service' import { IntegrationManagerService } from './services/managers/integration-manager.service' +import { OrganizationMembersService } from './services/managers/organization-members.service' import { PersonsManagerService } from './services/managers/persons-manager.service' import { RecipientsManagerService } from './services/managers/recipients-manager.service' import { TeamWorkflowsConfigService } from './services/managers/team-workflows-config.service' @@ -42,6 +43,7 @@ import { MessageAssetsService } from './services/messaging/message-assets.servic import { PushNotificationService } from './services/messaging/push-notification.service' import { RecipientPreferencesService } from './services/messaging/recipient-preferences.service' import { RecipientTokensService } from './services/messaging/recipient-tokens.service' +import { SandboxEmailSender } from './services/messaging/sandbox-email-sender' import { HogFunctionMonitoringService } from './services/monitoring/hog-function-monitoring.service' import { HogInvocationResultsService } from './services/monitoring/hog-invocation-results.service' import { HogWatcherService } from './services/monitoring/hog-watcher.service' @@ -167,6 +169,10 @@ export type CdpCoreServicesConfig = Pick< | 'SES_ENDPOINT' | 'SES_TRACKED_CONFIGURATION_SET' | 'SES_UNTRACKED_CONFIGURATION_SET' + | 'WORKFLOWS_SANDBOX_SENDER_ENABLED' + | 'SES_SANDBOX_TENANT_NAME' + | 'SES_SANDBOX_CONFIGURATION_SET' + | 'SES_SANDBOX_FROM_ADDRESS' | 'EMAIL_SUPPRESSION_TRANSIENT_BOUNCE_THRESHOLD' | 'EMAIL_TEAM_SENDING_CAP_MODE' | 'EMAIL_TEAM_SENDING_CAP_HOURLY_BY_TIER' @@ -473,7 +479,18 @@ export function createCdpCoreServices( recipientsManager, messageAssetsService, workflowEmailRateLimiter, - teamEmailRateLimiter + teamEmailRateLimiter, + new SandboxEmailSender( + { + enabled: config.WORKFLOWS_SANDBOX_SENDER_ENABLED, + tenantName: config.SES_SANDBOX_TENANT_NAME, + configurationSetName: config.SES_SANDBOX_CONFIGURATION_SET, + fromAddress: config.SES_SANDBOX_FROM_ADDRESS, + }, + deps.teamManager, + redis + ), + new OrganizationMembersService(deps.postgres) ) const recipientTokensService = new RecipientTokensService(config.ENCRYPTION_SALT_KEYS, config.SITE_URL) const hogInputsService = new HogInputsService(deps.integrationManager, recipientTokensService, deps.encryptedFields) diff --git a/nodejs/src/cdp/config.ts b/nodejs/src/cdp/config.ts index dd7198cb4a09..33d291e3b187 100644 --- a/nodejs/src/cdp/config.ts +++ b/nodejs/src/cdp/config.ts @@ -166,6 +166,11 @@ export type CdpConfig = ClickhouseConfig & { // Configuration set without open/click tracking (same delivery/bounce/complaint event destination). // Empty means not provisioned: tracking-off sends fall back to the tracked set with a warning. SES_UNTRACKED_CONFIGURATION_SET: string + // The sandbox sender delivers only with the switch on and all three route settings set. + WORKFLOWS_SANDBOX_SENDER_ENABLED: boolean + SES_SANDBOX_TENANT_NAME: string + SES_SANDBOX_CONFIGURATION_SET: string + SES_SANDBOX_FROM_ADDRESS: string // Comma-separated allowlist of SNS Topic ARNs the SES webhook accepts events from. Empty string // means no restriction (dev/test); production should set this to the workflow SES topic ARN(s). SES_ALLOWED_SNS_TOPIC_ARNS: string @@ -407,6 +412,10 @@ export function getDefaultCdpConfig(): CdpConfig { SES_REGION: isTestEnv() || isDevEnv() ? 'us-east-1' : '', SES_TRACKED_CONFIGURATION_SET: 'posthog-messaging', SES_UNTRACKED_CONFIGURATION_SET: '', + WORKFLOWS_SANDBOX_SENDER_ENABLED: false, + SES_SANDBOX_TENANT_NAME: '', + SES_SANDBOX_CONFIGURATION_SET: '', + SES_SANDBOX_FROM_ADDRESS: '', SES_ALLOWED_SNS_TOPIC_ARNS: '', EMAIL_SUPPRESSION_TRANSIENT_BOUNCE_THRESHOLD: 5, diff --git a/nodejs/src/cdp/services/managers/organization-members.service.test.ts b/nodejs/src/cdp/services/managers/organization-members.service.test.ts new file mode 100644 index 000000000000..1f9440626105 --- /dev/null +++ b/nodejs/src/cdp/services/managers/organization-members.service.test.ts @@ -0,0 +1,41 @@ +import { PostgresRouter } from '~/common/utils/db/postgres' + +import { OrganizationMembersService } from './organization-members.service' + +describe('OrganizationMembersService', () => { + const memberRow = (email: string): { team_id: number; email: string; is_email_verified: boolean } => ({ + team_id: 1, + email, + is_email_verified: true, + }) + + it('fails closed when every member lookup outlasts its one-minute decision', async () => { + let now = 0 + const query = jest.fn(() => { + now += 60_000 + return Promise.resolve({ rows: [memberRow('member@example.com')] }) + }) + const service = new OrganizationMembersService({ query } as unknown as PostgresRouter, () => now) + + await expect(service.getBlockedRecipients(1, ['member@example.com'])).rejects.toThrow('expired') + expect(query).toHaveBeenCalledTimes(2) + }) + + it('shares one member lookup across sends and reloads it after a minute', async () => { + let now = 0 + let member = 'member@example.com' + const query = jest.fn(() => Promise.resolve({ rows: [memberRow(member)] })) + const service = new OrganizationMembersService({ query } as unknown as PostgresRouter, () => now) + + expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([]) + expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([]) + expect(query).toHaveBeenCalledTimes(1) + + now = 60_001 + member = 'new-member@example.com' + expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([ + { address: 'member@example.com', unverifiedMember: false }, + ]) + expect(query).toHaveBeenCalledTimes(2) + }) +}) diff --git a/nodejs/src/cdp/services/managers/organization-members.service.ts b/nodejs/src/cdp/services/managers/organization-members.service.ts new file mode 100644 index 000000000000..1362b35123fa --- /dev/null +++ b/nodejs/src/cdp/services/managers/organization-members.service.ts @@ -0,0 +1,86 @@ +import { PostgresRouter, PostgresUse } from '~/common/utils/db/postgres' +import { LazyLoader } from '~/common/utils/lazy-loader' + +type OrganizationMemberSnapshot = { + verifiedEmails: ReadonlySet + unverifiedEmails: ReadonlySet + expiresAt: number +} + +export type BlockedRecipient = { readonly address: string; readonly unverifiedMember: boolean } + +const MAX_SNAPSHOT_AGE_MS = 60_000 +const REFRESH_AGE_MS = MAX_SNAPSHOT_AGE_MS - 1_000 + +const normalizeEmail = (email: string): string => email.trim().toLowerCase() + +export class OrganizationMembersService { + private snapshots: LazyLoader + + constructor( + postgres: PostgresRouter, + private now: () => number = Date.now + ) { + this.snapshots = new LazyLoader({ + name: 'sandbox_team_organization_members', + refreshAgeMs: REFRESH_AGE_MS, + refreshJitterMs: 0, + bufferMs: 0, + loader: async (teamIds) => { + const expiresAt = this.now() + MAX_SNAPSHOT_AGE_MS + // Only an explicit false deactivates an organization, matching the web middleware. + const { rows } = await postgres.query<{ team_id: number; email: string; is_email_verified: boolean }>( + PostgresUse.COMMON_WRITE, + `SELECT team.id AS team_id, users.email, users.is_email_verified + FROM posthog_team AS team + JOIN posthog_organization AS organization ON organization.id = team.organization_id + JOIN posthog_organizationmembership AS membership ON membership.organization_id = organization.id + JOIN posthog_user AS users ON users.id = membership.user_id + WHERE team.id = ANY($1::bigint[]) + AND organization.is_active IS NOT FALSE + AND users.is_active = true`, + [teamIds.map(Number)], + 'fetch-sandbox-team-organization-members' + ) + const members = new Map( + teamIds.map((id) => [id, { verified: new Set(), unverified: new Set() }]) + ) + for (const row of rows) { + const { verified, unverified } = members.get(String(row.team_id))! + ;(row.is_email_verified === true ? verified : unverified).add(normalizeEmail(row.email)) + } + return Object.fromEntries( + [...members].map(([id, { verified, unverified }]) => [ + id, + { verifiedEmails: verified, unverifiedEmails: unverified, expiresAt }, + ]) + ) + }, + }) + } + + public async getBlockedRecipients(teamId: number, recipients: string[]): Promise { + const snapshot = await this.getFreshSnapshot(String(teamId)) + return recipients + .filter((email) => !snapshot.verifiedEmails.has(normalizeEmail(email))) + .map((address) => ({ address, unverifiedMember: snapshot.unverifiedEmails.has(normalizeEmail(address)) })) + } + + // A snapshot older than a minute never decides a send: it is reloaded once, and a reload that + // itself outlasts the minute fails the check rather than trusting stale membership. + private async getFreshSnapshot(teamId: string): Promise { + let snapshot = await this.snapshots.get(teamId) + if (snapshot && this.now() >= snapshot.expiresAt) { + this.snapshots.markForRefresh(teamId) + snapshot = await this.snapshots.get(teamId) + } + if (!snapshot) { + throw new Error('Could not check organization members') + } + if (this.now() >= snapshot.expiresAt) { + this.snapshots.markForRefresh(teamId) + throw new Error('The organization member check expired') + } + return snapshot + } +} diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 7c3cd05338a0..770f9e77b87c 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -7,16 +7,23 @@ import { CyclotronInvocationQueueParametersEmailSchema, CyclotronInvocationQueueParametersEmailType, } from '~/cdp/schema/cyclotron' -import { CyclotronJobInvocationHogFunction } from '~/cdp/types' +import { CyclotronJobInvocationHogFunction, CyclotronJobInvocationResult } from '~/cdp/types' import { IngestionOutputs } from '~/common/outputs/ingestion-outputs' -import { createRedisV2PoolFromConfig } from '~/common/redis/redis-v2' +import { RedisClient, createRedisV2PoolFromConfig } from '~/common/redis/redis-v2' import { closeHub, createHub } from '~/common/utils/db/hub' -import { PostgresUse } from '~/common/utils/db/postgres' +import { PostgresRouter, PostgresUse } from '~/common/utils/db/postgres' +import * as posthog from '~/common/utils/posthog' import { waitForExpect } from '~/tests/helpers/expectations' -import { createTestTeamFixture } from '~/tests/helpers/sql' +import { + createOrganization, + createOrganizationMembership, + createTestTeamFixture, + createUser, +} from '~/tests/helpers/sql' import { INVALID_TEAM_ID_KEYS } from '~/tests/helpers/team' import { Hub, Team } from '../../../types' +import { OrganizationMembersService } from '../managers/organization-members.service' import { RecipientsManagerService } from '../managers/recipients-manager.service' import { TeamWorkflowsConfigService } from '../managers/team-workflows-config.service' import { RateLimiterService } from '../rate-limiter/rate-limiter.service' @@ -32,6 +39,7 @@ import { import { MailDevAPI } from './helpers/maildev' import { EmailTrackingCodeSigner } from './helpers/tracking-code' import { MessageAssetsService } from './message-assets.service' +import { SandboxEmailSender, SandboxEmailSenderConfig } from './sandbox-email-sender' class ThrottlingException extends Error { constructor(message: string) { @@ -218,6 +226,396 @@ describe('EmailService', () => { sendEmailSpy = jest.spyOn(service.sesV2Client!, 'send') as any sendEmailSpy.mockResolvedValue({ MessageId: 'test-message-id' }) }) + describe('sandbox sender', () => { + const UNAVAILABLE_LOG = + 'Skipping send: the sandbox sender is unavailable right now. Verify your own domain to keep sending.' + let capture: jest.SpyInstance + let memberId: number + let memberIds: number[] + let extraOrganizationIds: string[] + const memberEmail = (name = 'test'): string => `${name}-${team.id}@example.com` + const createSandboxParams = ( + params: Partial = {} + ): CyclotronInvocationQueueParametersEmailType => + createEmailParams({ + from: { integrationId: 4 }, + to: { email: memberEmail(), name: 'Example member' }, + ...params, + }) + const createMember = async (email: string, organizationId = team.organization_id): Promise => { + const id = await createUser(hub.postgres, email) + memberIds.push(id) + await createOrganizationMembership(hub.postgres, organizationId, id) + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_user SET email = $1, is_active = true, is_email_verified = true WHERE id = $2', + [email, id], + 'test:verify-sandbox-member' + ) + return id + } + const createSandboxService = ( + enabled: boolean, + membersPostgres: PostgresRouter = hub.postgres, + routeSettings: Partial = {} + ): EmailService => { + const sandboxService = new EmailService( + { + sesAccessKeyId: hub.SES_ACCESS_KEY_ID, + sesSecretAccessKey: hub.SES_SECRET_ACCESS_KEY, + sesRegion: hub.SES_REGION, + sesEndpoint: hub.SES_ENDPOINT, + sesTrackedConfigurationSet: hub.SES_TRACKED_CONFIGURATION_SET, + sesUntrackedConfigurationSet: hub.SES_UNTRACKED_CONFIGURATION_SET, + }, + hub.integrationManager, + new TeamWorkflowsConfigService(hub.postgres, hub.pubSub), + hub.ENCRYPTION_SALT_KEYS, + hub.SITE_URL, + new EmailTrackingCodeSigner(hub.ENCRYPTION_SALT_KEYS, hub.CDP_EMAIL_TRACKING_URL), + new EmailSuppressionService(hub.postgres, emailSuppressionConfigFromEnv()), + new RecipientsManagerService(hub.postgres), + undefined, + null, + null, + new SandboxEmailSender( + { + enabled, + tenantName: 'sandbox-tenant', + configurationSetName: 'sandbox-email', + fromAddress: 'fixed-sandbox@example.com', + ...routeSettings, + }, + hub.teamManager, + { + useClient: async (_options, callback) => { + const client = await hub.redisPool.acquire() + try { + return await callback(client as unknown as RedisClient) + } finally { + await hub.redisPool.release(client) + } + }, + } + ), + new OrganizationMembersService(membersPostgres) + ) + sendEmailSpy = jest.spyOn(sandboxService.sesV2Client!, 'send') as jest.SpyInstance + sendEmailSpy.mockResolvedValue({ MessageId: 'sandbox-message-id' }) + return sandboxService + } + const expectSkipped = (result: CyclotronJobInvocationResult): void => { + expect(sendEmailSpy).not.toHaveBeenCalled() + expect(result).toMatchObject({ finished: true, skipped: true, metrics: [], messageAssets: [] }) + expect(result.error).toBeUndefined() + expect(result.emailSendResult).toMatchObject({ status: 'skipped' }) + expect(result.invocation.state.vmState?.stack.at(-1)).toEqual({ success: false }) + } + const expectBlockedEvent = (reason: string, isTest: boolean, blockedRecipientCount: number): void => { + expect(capture).toHaveBeenLastCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + { reason, is_test: isTest, blocked_recipient_count: blockedRecipientCount } + ) + } + + beforeEach(async () => { + capture = jest.spyOn(posthog, 'captureTeamEvent').mockImplementation(() => {}) + memberIds = [] + extraOrganizationIds = [] + memberId = await createMember(memberEmail()) + await createMember(memberEmail('cc')) + await createMember(memberEmail('bcc')) + await insertIntegration(hub.postgres, team.id, { + id: getIntegrationId(4), + kind: 'email', + config: { + provider: 'sandbox', + email: 'sandbox@example.com', + domain: 'example.com', + name: 'Example organization via PostHog', + verified: true, + }, + }) + invocation.queueParameters = createSandboxParams() + Object.assign(invocation, { hogFlow: { id: invocation.functionId, team_id: team.id } }) + }) + + afterEach(async () => { + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM posthog_organizationmembership WHERE user_id = ANY($1::integer[])', + [memberIds], + 'test:delete-sandbox-memberships' + ) + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM posthog_user WHERE id = ANY($1::integer[])', + [memberIds], + 'test:delete-sandbox-members' + ) + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM posthog_organization WHERE id = ANY($1::uuid[])', + [extraOrganizationIds], + 'test:delete-sandbox-organizations' + ) + const client = await hub.redisPool.acquire() + try { + const keys = await client.keys(`@posthog/workflows-sandbox-blocked/${team.id}/*`) + if (keys.length) { + await client.del(...keys) + } + } finally { + await hub.redisPool.release(client) + } + capture.mockRestore() + }) + + it.each([ + ['the switch is off', false, false, {}], + ['the switch is off', true, false, {}], + ['the tenant is empty', false, true, { tenantName: '' }], + ['the configuration set is blank', true, true, { configurationSetName: ' ' }], + ['the From address is empty', false, true, { fromAddress: '' }], + ] as const)( + 'skips when the sandbox sender is switched off because %s (isTest=%s)', + async (_reason, isTest, enabled, routeSettings: Partial) => { + service = createSandboxService(enabled, hub.postgres, routeSettings) + + const result = await service.executeSendEmail(invocation, isTest) + + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ level: 'info', message: UNAVAILABLE_LOG }) + ) + expectBlockedEvent('switch_off', isTest, 0) + } + ) + + it.each([false, true])( + 'permits sandbox destination invocations only as explicit tests (isTest=%s)', + async (isTest) => { + service = createSandboxService(true) + const destination = createExampleInvocation({ team_id: team.id, id: 'email-destination' }) + destination.state.vmState = { stack: [] } as any + destination.queueParameters = createSandboxParams() + + const result = await service.executeSendEmail(destination, isTest) + + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ + message: isTest + ? UNAVAILABLE_LOG + : 'Skipping send: the sandbox sender is only available for workflows and test sends.', + }) + ) + } + ) + + it.each([false, true])( + 'skips a member send as unavailable until the sandbox route exists (isTest=%s)', + async (isTest) => { + service = createSandboxService(true) + + const result = await service.executeSendEmail(invocation, isTest) + + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ level: 'info', message: UNAVAILABLE_LOG }) + ) + expect(capture).not.toHaveBeenCalled() + } + ) + + it.each([ + ['a display name', 'Example colleague', false], + ['a quoted comma', '"Example, colleague"', false], + ['an escaped quote', '"Example \\"colleague, teammate\\""', false], + ['mixed-case stored addresses', 'Example colleague', true], + ])('passes members through the gate whatever the case and %s', async (_case, name, storedMixedCase) => { + if (storedMixedCase) { + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_user SET email = initcap(email) WHERE id = ANY($1::integer[])', + [memberIds], + 'test:store-mixed-case-sandbox-members' + ) + } + service = createSandboxService(true) + invocation.queueParameters = createSandboxParams({ + to: { email: memberEmail().toUpperCase() }, + cc: `${name} <${memberEmail('cc').toUpperCase()}>`, + bcc: ` ${name} <${memberEmail('bcc').toUpperCase()}> `, + }) + + const result = await service.executeSendEmail(invocation) + + expectSkipped(result) + expect(result.logs).toContainEqual(expect.objectContaining({ message: UNAVAILABLE_LOG })) + expect(capture).not.toHaveBeenCalled() + }) + + it.each([false, true])('blocks a non-member To address (isTest=%s)', async (isTest) => { + service = createSandboxService(true) + invocation.queueParameters = createSandboxParams({ to: { email: 'outside@example.com' } }) + + const result = await service.executeSendEmail(invocation, isTest) + + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ + level: 'info', + message: + 'Skipping send: the sandbox sender only sends to active organization members with verified email addresses. Blocked addresses: outside@example.com. Verify your own domain to send to anyone.', + }) + ) + expectBlockedEvent('recipient_not_member', isTest, 1) + }) + + it.each([ + 'cc', + 'bcc', + 'cc list', + 'multiple', + 'ambiguous cc mailbox', + 'unverified', + 'legacy', + 'inactive', + 'other organization', + 'deactivated organization', + ])('blocks all delivery for %s', async (recipient) => { + let blocked = ['outside@example.com'] + let params = createSandboxParams() + if (recipient === 'cc' || recipient === 'bcc') { + params = { ...params, [recipient]: 'Outside member ' } + blocked = ['OUTSIDE@example.com'] + } else if (recipient === 'cc list') { + params = { ...params, cc: `"Example, colleague" <${memberEmail('cc')}>, outside@example.com` } + } else if (recipient === 'multiple') { + params = { ...params, cc: 'outside@example.com', bcc: 'another@example.com' } + blocked = ['outside@example.com', 'another@example.com'] + } else if (recipient === 'ambiguous cc mailbox') { + params = { ...params, cc: `Example <${memberEmail('cc')}> ` } + blocked = [`Example <${memberEmail('cc')}> `] + } else if (recipient === 'other organization') { + const organizationId = await createOrganization(hub.postgres) + extraOrganizationIds.push(organizationId) + blocked = [memberEmail('outside')] + await createMember(blocked[0], organizationId) + params = { ...params, to: { email: blocked[0] } } + } else if (recipient === 'deactivated organization') { + blocked = [memberEmail()] + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_organization SET is_active = false WHERE id = $1', + [team.organization_id], + 'test:deactivate-sandbox-organization' + ) + } else { + blocked = [memberEmail()] + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_user SET is_active = $1, is_email_verified = $2 WHERE id = $3', + [recipient !== 'inactive', recipient === 'legacy' ? null : recipient === 'inactive', memberId], + 'test:change-sandbox-member-status' + ) + } + service = createSandboxService(true) + invocation.queueParameters = params + + const result = await service.executeSendEmail(invocation) + + expectSkipped(result) + const unverifiedMember = recipient === 'unverified' || recipient === 'legacy' + expect(result.logs).toContainEqual( + expect.objectContaining({ + level: 'info', + message: unverifiedMember + ? `Skipping send: the sandbox sender only sends to organization members with verified email addresses. Unverified addresses: ${blocked.join(', ')}. Verify your email address in PostHog, or ask these members to verify theirs.` + : `Skipping send: the sandbox sender only sends to active organization members with verified email addresses. Blocked addresses: ${blocked.join(', ')}. Verify your own domain to send to anyone.`, + }) + ) + expect(capture).toHaveBeenCalledTimes(1) + expectBlockedEvent( + unverifiedMember ? 'recipient_unverified' : 'recipient_not_member', + false, + blocked.length + ) + }) + + it('fails closed when the member check fails', async () => { + const membersPostgres = new PostgresRouter(hub) + await membersPostgres.end() + service = createSandboxService(true, membersPostgres) + invocation.queueParameters = createSandboxParams({ cc: memberEmail('cc') }) + + const result = await service.executeSendEmail(invocation) + + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ + message: `Skipping send: could not check organization members for these addresses: ${memberEmail()}, ${memberEmail('cc')}. Try again, or verify your own domain to send to anyone.`, + }) + ) + expectBlockedEvent('check_failed', false, 2) + }) + + it('checks members again right before delivery', async () => { + const now = Date.now() + const clock = jest.spyOn(Date, 'now').mockReturnValue(now) + const preparation = jest.spyOn(EmailSuppressionService.prototype, 'isSuppressed') + const isSuppressed = preparation.getMockImplementation()! + try { + service = createSandboxService(true) + await service.executeSendEmail(invocation) + expect(capture).not.toHaveBeenCalled() + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM posthog_organizationmembership WHERE user_id = $1', + [memberId], + 'test:revoke-sandbox-member-during-preparation' + ) + clock.mockReturnValue(now + 58_000) + preparation.mockImplementation(async function (this: EmailSuppressionService, ...args) { + const suppressed = await isSuppressed.apply(this, args) + clock.mockReturnValue(now + 63_000) + return suppressed + }) + invocation.state.vmState = { stack: [] } as any + + const result = await service.executeSendEmail(invocation) + + expectSkipped(result) + expectBlockedEvent('recipient_not_member', false, 1) + } finally { + clock.mockRestore() + preparation.mockRestore() + } + }) + + it('captures one blocked event per reason and send kind while logging every skip', async () => { + for (let attempt = 0; attempt < 2; attempt++) { + service = createSandboxService(true) + invocation.queueParameters = createSandboxParams({ to: { email: 'outside@example.com' } }) + const result = await service.executeSendEmail(invocation) + expect(result.logs).toContainEqual( + expect.objectContaining({ message: expect.stringContaining('outside@example.com') }) + ) + } + expect(capture).toHaveBeenCalledTimes(1) + + await createSandboxService(true).executeSendEmail(invocation, true) + expect(capture).toHaveBeenCalledTimes(2) + expectBlockedEvent('recipient_not_member', true, 1) + + invocation.queueParameters = createSandboxParams() + await createSandboxService(false).executeSendEmail(invocation) + expect(capture).toHaveBeenCalledTimes(3) + expectBlockedEvent('switch_off', false, 0) + }) + }) describe('integration validation', () => { beforeEach(async () => { await insertIntegration(hub.postgres, team.id, { diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index ff83d8746de4..43e8f843eb41 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -17,6 +17,7 @@ import { createInvocationResult } from '~/cdp/utils/invocation-utils' import { logger } from '~/common/utils/logger' import { IntegrationManagerService } from '../managers/integration-manager.service' +import { OrganizationMembersService } from '../managers/organization-members.service' import { RecipientManagerRecipient, RecipientsManagerService } from '../managers/recipients-manager.service' import { TeamWorkflowsConfigService } from '../managers/team-workflows-config.service' import { RateLimiterService } from '../rate-limiter/rate-limiter.service' @@ -34,6 +35,7 @@ import { EmailTrackingCodeSigner, TRACKING_CODE_HEADER_NAME } from './helpers/tr import { addUtmTagsToEmail, renderUtmOverrides, resolveUtmTags } from './helpers/utm' import { MessageAssetsService } from './message-assets.service' import { RecipientTokensService } from './recipient-tokens.service' +import { SandboxEmailBlockReason, SandboxEmailSender } from './sandbox-email-sender' const sesThrottleResponsesTotal = new Counter({ name: 'cdp_ses_throttle_responses_total', @@ -344,6 +346,98 @@ export function parseAddressList(value?: string): string[] | undefined { return result.length > 0 ? result : undefined } +type QuoteScan = { unquotedIndexes: number[]; balanced: boolean } + +function scanQuotes(text: string): QuoteScan { + const unquotedIndexes: number[] = [] + let quoted = false + let escaped = false + for (let index = 0; index < text.length; index++) { + const character = text[index] + if (escaped) { + escaped = false + } else if (quoted && character === '\\') { + escaped = true + } else if (character === '"') { + quoted = !quoted + } else if (!quoted) { + unquotedIndexes.push(index) + } + } + return { unquotedIndexes, balanced: !quoted } +} + +function splitOutsideQuotes(text: string): string[] { + const commas = scanQuotes(text).unquotedIndexes.filter((index) => text[index] === ',') + return [-1, ...commas].map((comma, position) => text.slice(comma + 1, commas[position] ?? text.length)) +} + +function angleAddress(entry: string): string | undefined { + const { unquotedIndexes, balanced } = scanQuotes(entry) + const opening = unquotedIndexes.find((index) => entry[index] === '<') + const closing = entry.length - 1 + if (!balanced || opening === undefined || entry[closing] !== '>' || /[\r\n]/.test(entry)) { + return undefined + } + const unquotedBetween = (start: number, end: number): string => + unquotedIndexes + .filter((index) => index >= start && index < end) + .map((index) => entry[index]) + .join('') + const mailbox = entry.slice(opening + 1, closing).trim() + const isFramed = !/[>@;]/.test(unquotedBetween(0, opening)) && !/[<>]/.test(unquotedBetween(opening + 1, closing)) + return isFramed && mailbox ? mailbox : undefined +} + +// Unlike `extractEmailsFromAddressList`, this keeps quoted commas inside one entry and leaves an +// ambiguous mailbox whole, so the member check sees exactly the addresses SES would deliver to. +function sandboxAddressList(value?: string): string[] { + return splitOutsideQuotes(value ?? '') + .map((entry) => entry.trim()) + .filter(Boolean) + .map((entry) => angleAddress(entry) ?? entry) +} + +function recipientsOf(params: CyclotronInvocationQueueParametersEmailType): string[] { + return [ + params.to.email.trim(), + ...extractEmailsFromAddressList(params.cc), + ...extractEmailsFromAddressList(params.bcc), + ] +} + +function sandboxRecipientsOf(params: CyclotronInvocationQueueParametersEmailType): string[] { + return [params.to.email.trim(), ...sandboxAddressList(params.cc), ...sandboxAddressList(params.bcc)] +} + +type SandboxRecipientBlockReason = Exclude + +const SANDBOX_UNAVAILABLE_LOG = + 'Skipping send: the sandbox sender is unavailable right now. Verify your own domain to keep sending.' + +const SANDBOX_RECIPIENT_BLOCK_COPY: Record< + SandboxRecipientBlockReason, + { explanation: string; label: string; guidance: string } +> = { + recipient_not_member: { + explanation: + 'Skipping send: the sandbox sender only sends to active organization members with verified email addresses', + label: '. Blocked addresses: ', + guidance: 'Verify your own domain to send to anyone.', + }, + recipient_unverified: { + explanation: + 'Skipping send: the sandbox sender only sends to organization members with verified email addresses', + label: '. Unverified addresses: ', + guidance: 'Verify your email address in PostHog, or ask these members to verify theirs.', + }, + check_failed: { + explanation: 'Skipping send: could not check organization members', + label: ' for these addresses: ', + guidance: 'Try again, or verify your own domain to send to anyone.', + }, +} + export class EmailService { sesV2Client: SESv2Client | null @@ -361,7 +455,9 @@ export class EmailService { private recipientsManager: RecipientsManagerService, private messageAssetsService?: MessageAssetsService, private workflowEmailRateLimiter: RateLimiterService | null = null, - private teamEmailRateLimiter: RateLimiterService | null = null + private teamEmailRateLimiter: RateLimiterService | null = null, + private sandboxSender: SandboxEmailSender | null = null, + private organizationMembers: OrganizationMembersService | null = null ) { this.sesV2Client = this.sesConfig.sesRegion ? new SESv2Client({ @@ -398,6 +494,7 @@ export class EmailService { const params = invocation.queueParameters const integrationId = selectEmailSenderIntegrationId(invocation.id, params.from) const integration = await this.integrationManager.get(integrationId) + const isSandbox = integration?.config.provider === 'sandbox' let success: boolean = false let throttled: boolean = false @@ -485,6 +582,19 @@ export class EmailService { ) } + if (isSandbox && !isTest && !('hogFlow' in invocation)) { + this.skipSandboxSend( + result, + 'Skipping send: the sandbox sender is only available for workflows and test sends.' + ) + return result + } + if (isSandbox && !this.sandboxSender?.canSend) { + this.skipSandboxSend(result, SANDBOX_UNAVAILABLE_LOG) + await this.sandboxSender?.captureBlocked(invocation.teamId, isTest, 'switch_off', 0) + return result + } + const from = this.resolveFromSender(integration, params.from, addLog) // An empty `to` comes from the customer's data (a person without an email property, or no @@ -511,11 +621,16 @@ export class EmailService { return result } + const recipients = isSandbox ? sandboxRecipientsOf(params) : recipientsOf(params) + if (isSandbox && !(await this.checkSandboxRecipients(result, recipients, isTest))) { + return result + } + // Single choke point for the suppression check — every send path lands here regardless // of whether the invocation came from a workflow action or an email destination hog // function. Checking here means callers can't bypass it by taking a different upstream // route. Covers `to`, `cc`, and `bcc`; a suppressed address anywhere blocks the send. - const skipReason = await this.buildSuppressionSkipReason(invocation.teamId, params, { + const skipReason = await this.buildSuppressionSkipReason(invocation.teamId, recipients, { transactional: invocation.hogFunction?.metadata?.message_category_type === 'transactional', }) if (skipReason) { @@ -646,6 +761,11 @@ export class EmailService { case 'ses': await this.sendEmailWithSES(result, sendParams, from, trackingEnabled, isTest) break + case 'sandbox': + if (await this.checkSandboxRecipients(result, recipients, isTest)) { + this.skipSandboxSend(result, SANDBOX_UNAVAILABLE_LOG) + } + return result case 'unsupported': throw new Error('Email delivery mode not supported') @@ -861,6 +981,52 @@ export class EmailService { return null } + private skipSandboxSend( + result: CyclotronJobInvocationResult, + reason: string + ): void { + createAddLogFunction(result.logs)('info', reason) + result.emailSendResult = { status: 'skipped', reason } + result.skipped = true + result.invocation.state.vmState?.stack.push({ success: false }) + } + + // Runs once before the send is prepared and again right before delivery, because a member can + // leave the organization while the send waits on suppression checks and rate limits. + private async checkSandboxRecipients( + result: CyclotronJobInvocationResult, + recipients: string[], + isTest: boolean + ): Promise { + const { teamId } = result.invocation + let blockedRecipients: string[] + let reason: SandboxRecipientBlockReason + try { + if (!this.organizationMembers) { + throw new Error('Organization member checks are not configured') + } + const blocked = await this.organizationMembers.getBlockedRecipients(teamId, recipients) + blockedRecipients = blocked.map(({ address }) => address) + reason = blocked.every(({ unverifiedMember }) => unverifiedMember) + ? 'recipient_unverified' + : 'recipient_not_member' + } catch (error) { + blockedRecipients = recipients + reason = 'check_failed' + logger.warn('Could not check sandbox email recipients', { + teamId, + error: error instanceof Error ? error.message : 'Unknown error', + }) + } + if (!blockedRecipients.length) { + return true + } + const { explanation, label, guidance } = SANDBOX_RECIPIENT_BLOCK_COPY[reason] + this.skipSandboxSend(result, `${explanation}${label}${blockedRecipients.join(', ')}. ${guidance}`) + await this.sandboxSender?.captureBlocked(teamId, isTest, reason, blockedRecipients.length) + return false + } + // Returns a human-readable log string when any destination address is suppressed for the team, // or null when the send should proceed. Scans to + cc + bcc — SES delivers to every list, so a // suppressed address anywhere blocks the whole send. `cc` and `bcc` can be comma-separated @@ -868,15 +1034,9 @@ export class EmailService { // matching against the normalized suppression identifier. private async buildSuppressionSkipReason( teamId: number, - params: CyclotronInvocationQueueParametersEmailType, + recipients: string[], options: SuppressionCheckOptions ): Promise { - const recipients: string[] = [] - if (params.to?.email && params.to.email.trim()) { - recipients.push(params.to.email.trim()) - } - recipients.push(...extractEmailsFromAddressList(params.cc)) - recipients.push(...extractEmailsFromAddressList(params.bcc)) if (recipients.length === 0) { return null } diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts new file mode 100644 index 000000000000..37f4a9ada68c --- /dev/null +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -0,0 +1,93 @@ +import { RedisV2 } from '~/common/redis/redis-v2' +import { logger } from '~/common/utils/logger' +import { captureTeamEvent } from '~/common/utils/posthog' +import { TeamManager } from '~/common/utils/team-manager' + +export interface SandboxEmailSenderConfig { + enabled: boolean + tenantName: string + configurationSetName: string + fromAddress: string +} + +const ROUTE_SETTING_NAMES = { + tenantName: 'SES_SANDBOX_TENANT_NAME', + configurationSetName: 'SES_SANDBOX_CONFIGURATION_SET', + fromAddress: 'SES_SANDBOX_FROM_ADDRESS', +} as const + +function emptyRouteSettings(config: SandboxEmailSenderConfig): string[] { + return (Object.keys(ROUTE_SETTING_NAMES) as (keyof typeof ROUTE_SETTING_NAMES)[]) + .filter((setting) => !config[setting].trim()) + .map((setting) => ROUTE_SETTING_NAMES[setting]) +} + +export type SandboxEmailBlockReason = 'switch_off' | 'recipient_not_member' | 'recipient_unverified' | 'check_failed' + +const BLOCKED_EVENT_WINDOW_SECONDS = 3600 + +export class SandboxEmailSender { + public readonly canSend: boolean + + constructor( + public readonly config: SandboxEmailSenderConfig, + private teamManager: TeamManager, + private blockedEventRedis: Pick + ) { + this.config = { + ...config, + tenantName: config.tenantName.trim(), + configurationSetName: config.configurationSetName.trim(), + fromAddress: config.fromAddress.trim(), + } + const emptySettings = emptyRouteSettings(config) + if (config.enabled && emptySettings.length) { + logger.error('The sandbox sender is switched on with empty route settings, so it skips every send', { + emptySettings, + }) + } + this.canSend = config.enabled && !emptySettings.length + } + + public async captureBlocked( + teamId: number, + isTest: boolean, + reason: SandboxEmailBlockReason, + blockedRecipientCount: number + ): Promise { + try { + if (!(await this.isFirstBlockInWindow(teamId, isTest, reason))) { + return + } + const team = await this.teamManager.getTeam(teamId) + if (team) { + captureTeamEvent(team, 'workflows sandbox email blocked', { + reason, + blocked_recipient_count: blockedRecipientCount, + is_test: isTest, + }) + } + } catch (error) { + logger.warn('Could not capture sandbox email event', { teamId, error }) + } + } + + private async isFirstBlockInWindow( + teamId: number, + isTest: boolean, + reason: SandboxEmailBlockReason + ): Promise { + const firstBlock = await this.blockedEventRedis.useClient( + { name: 'sandbox-blocked-event', timeout: 1000 }, + (client) => + client.set( + `@posthog/workflows-sandbox-blocked/${teamId}/${reason}/${isTest ? 'test' : 'live'}`, + '1', + 'EX', + BLOCKED_EVENT_WINDOW_SECONDS, + 'NX' + ) + ) + return firstBlock === 'OK' + } +} From 1951b5badd87c492f3c8092cc5dd1e1046b4ed16 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 05:54:17 +0000 Subject: [PATCH 04/17] fix(workflows): keep sandbox sends off the team sending budget A sandbox send reaches only the team's own members, so it must not spend or wait on the team's tier budget that its real sends share. Co-Authored-By: Claude Opus 5.5 --- .../internal/workflows-email-sending-tiers.md | 1 + .../services/messaging/email.service.test.ts | 27 ++++++++++++++++--- .../cdp/services/messaging/email.service.ts | 3 ++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index b0d50f3b664d..9dc38a4944b5 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -75,5 +75,6 @@ Every To, CC, and BCC address must belong to an active member of the team's acti Membership snapshots expire within one minute, and the worker checks recipients again right before delivery. A failed or stale membership check skips the send and lists the blocked addresses in the run log. The blocked event fires at most once an hour per team, reason, and send kind (workflow or test). +Sandbox sends never spend the team's tier budget, because they reach only its own members. The sandbox route is not built yet, so a send that passes every check still skips as unavailable. Keep the switch off in production until the sandbox caps are deployed. diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 770f9e77b87c..c67b799610e2 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -257,8 +257,11 @@ describe('EmailService', () => { const createSandboxService = ( enabled: boolean, membersPostgres: PostgresRouter = hub.postgres, - routeSettings: Partial = {} + routeSettings: Partial = {}, + teamEmailRateLimiter: RateLimiterService | null = null ): EmailService => { + const workflowsConfig = new TeamWorkflowsConfigService(hub.postgres, hub.pubSub) + jest.spyOn(workflowsConfig, 'getEmailSendingTier').mockResolvedValue(0) const sandboxService = new EmailService( { sesAccessKeyId: hub.SES_ACCESS_KEY_ID, @@ -267,9 +270,12 @@ describe('EmailService', () => { sesEndpoint: hub.SES_ENDPOINT, sesTrackedConfigurationSet: hub.SES_TRACKED_CONFIGURATION_SET, sesUntrackedConfigurationSet: hub.SES_UNTRACKED_CONFIGURATION_SET, + teamEmailCapMode: 'enforce', + teamEmailTierHourlyCaps: [1], + teamEmailTierDailyCaps: [1], }, hub.integrationManager, - new TeamWorkflowsConfigService(hub.postgres, hub.pubSub), + workflowsConfig, hub.ENCRYPTION_SALT_KEYS, hub.SITE_URL, new EmailTrackingCodeSigner(hub.ENCRYPTION_SALT_KEYS, hub.CDP_EMAIL_TRACKING_URL), @@ -277,7 +283,7 @@ describe('EmailService', () => { new RecipientsManagerService(hub.postgres), undefined, null, - null, + teamEmailRateLimiter, new SandboxEmailSender( { enabled, @@ -429,6 +435,21 @@ describe('EmailService', () => { } ) + it('never spends the team sending budget on a sandbox send', async () => { + const claimAllOrNothingPair = jest + .fn() + .mockResolvedValue({ granted: false, deniedIndex: 0, retryAfterMs: 60_000, reserved: false }) + service = createSandboxService(true, hub.postgres, {}, { + claimAllOrNothingPair, + } as unknown as RateLimiterService) + + const result = await service.executeSendEmail(invocation) + + expectSkipped(result) + expect(result.logs).toContainEqual(expect.objectContaining({ level: 'info', message: UNAVAILABLE_LOG })) + expect(claimAllOrNothingPair).not.toHaveBeenCalled() + }) + it.each([ ['a display name', 'Example colleague', false], ['a quoted comma', '"Example, colleague"', false], diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index 43e8f843eb41..283f94b90dea 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -703,7 +703,8 @@ export class EmailService { // own quota, so a send with many copies must spend that many tokens. const capRecipients = 1 + extractEmailsFromAddressList(params.cc).length + extractEmailsFromAddressList(params.bcc).length - const capDelay = await this.claimTeamSendingBudget(invocation, isTest, capRecipients) + // Sandbox sends never spend the team's tier budget: they reach only its own members. + const capDelay = isSandbox ? null : await this.claimTeamSendingBudget(invocation, isTest, capRecipients) if (capDelay) { result.finished = false // Re-attach the email payload before rescheduling, for the same reason as the From 8f17a80f9521b77028bddeec11ee7997f7b104be Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 05:40:08 +0000 Subject: [PATCH 05/17] feat(workflows): route sandbox email through its fixed ses identity A sandbox send that passes the member gate now goes out through the fixed SES tenant, configuration set and From address, with a footer that names the sender and the organization. Tracking, unsubscribe headers, the team's tier budget and custom From/Reply-To settings do not apply. The check on the final recipient list runs right before delivery, and each delivered send emits a sandbox sent event. Co-Authored-By: Claude Opus 5.5 --- .../internal/workflows-email-sending-tiers.md | 10 +- nodejs/package.json | 1 + .../services/messaging/email.service.test.ts | 165 +++++++++++--- .../cdp/services/messaging/email.service.ts | 105 +++++++-- .../messaging/sandbox-email-sender.test.ts | 201 ++++++++++++++++++ .../messaging/sandbox-email-sender.ts | 143 ++++++++++++- pnpm-lock.yaml | 3 + 7 files changed, 570 insertions(+), 58 deletions(-) create mode 100644 nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 9dc38a4944b5..d310355a10c0 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -75,6 +75,12 @@ Every To, CC, and BCC address must belong to an active member of the team's acti Membership snapshots expire within one minute, and the worker checks recipients again right before delivery. A failed or stale membership check skips the send and lists the blocked addresses in the run log. The blocked event fires at most once an hour per team, reason, and send kind (workflow or test). -Sandbox sends never spend the team's tier budget, because they reach only its own members. -The sandbox route is not built yet, so a send that passes every check still skips as unavailable. +A send that passes every check goes out through the fixed SES tenant, configuration set, and From address, whatever the step's custom sender and Reply-To say. +Django reads the same `SES_SANDBOX_FROM_ADDRESS` for the address it stores and shows, and the address must belong to the shared SES identity. +Sandbox sends skip the team's tier budget, click and open tracking, and unsubscribe headers, because they reach only members of the sending organization. +The worker parses and normalizes the HTML, then appends a footer that names the sender and the organization ID. +It overrides root clipping and visibility styles and keeps the template's typography, so malformed markup may render differently from an own-domain send. +A template whose HTML cannot keep the footer fails the send. +The visible footer is best effort across email clients and template CSS; the signed tracking header still attributes the message. +Each delivered send emits a `workflows sandbox email sent` event. Keep the switch off in production until the sandbox caps are deployed. diff --git a/nodejs/package.json b/nodejs/package.json index c53880215676..25b30431b7ec 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -137,6 +137,7 @@ "node-schedule": "^2.1.0", "nodemailer": "^8.0.8", "p-limit": "3.1.0", + "parse5": "7.1.2", "pg": "^8.6.0", "pino": "^8.6.0", "posthog-node": "5.57.0", diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index c67b799610e2..3dedc6f60306 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -1,6 +1,11 @@ import { mockFetch } from '~/tests/helpers/mocks/request.mock' -import { MessageRejected, SendingPausedException, TooManyRequestsException } from '@aws-sdk/client-sesv2' +import { + MessageRejected, + SendEmailCommand, + SendingPausedException, + TooManyRequestsException, +} from '@aws-sdk/client-sesv2' import { createExampleInvocation, insertIntegration } from '~/cdp/_tests/fixtures' import { @@ -258,7 +263,8 @@ describe('EmailService', () => { enabled: boolean, membersPostgres: PostgresRouter = hub.postgres, routeSettings: Partial = {}, - teamEmailRateLimiter: RateLimiterService | null = null + teamEmailRateLimiter: RateLimiterService | null = null, + messageAssetsService?: MessageAssetsService ): EmailService => { const workflowsConfig = new TeamWorkflowsConfigService(hub.postgres, hub.pubSub) jest.spyOn(workflowsConfig, 'getEmailSendingTier').mockResolvedValue(0) @@ -281,7 +287,7 @@ describe('EmailService', () => { new EmailTrackingCodeSigner(hub.ENCRYPTION_SALT_KEYS, hub.CDP_EMAIL_TRACKING_URL), new EmailSuppressionService(hub.postgres, emailSuppressionConfigFromEnv()), new RecipientsManagerService(hub.postgres), - undefined, + messageAssetsService, null, teamEmailRateLimiter, new SandboxEmailSender( @@ -409,29 +415,96 @@ describe('EmailService', () => { const result = await service.executeSendEmail(destination, isTest) - expectSkipped(result) - expect(result.logs).toContainEqual( - expect.objectContaining({ - message: isTest - ? UNAVAILABLE_LOG - : 'Skipping send: the sandbox sender is only available for workflows and test sends.', - }) - ) + expect(result.error).toBeUndefined() + expect(sendEmailSpy).toHaveBeenCalledTimes(isTest ? 1 : 0) + expect(result.invocation.state.vmState?.stack).toEqual([{ success: isTest }]) + if (!isTest) { + expectSkipped(result) + expect(result.logs).toContainEqual( + expect.objectContaining({ + message: + 'Skipping send: the sandbox sender is only available for workflows and test sends.', + }) + ) + } } ) - it.each([false, true])( - 'skips a member send as unavailable until the sandbox route exists (isTest=%s)', - async (isTest) => { - service = createSandboxService(true) + it.each([ + [false, 'Hello there.', 'Hello ', false], + [true, 'Hello there.', 'Hello ', false], + [false, '

Hello

', '

Hello

', true], + ] as const)( + 'sends untracked with the fixed identity and organization footer (isTest=%s, html=%s)', + async (isTest, html, expectedContent, htmlOnly) => { + const outputs = { + produce: jest.fn().mockResolvedValue(undefined), + } as unknown as IngestionOutputs<'message_assets'> + service = createSandboxService(true, hub.postgres, {}, null, new MessageAssetsService(outputs)) + invocation.state.actionId = 'send-email' + invocation.queueParameters = createSandboxParams({ + from: { integrationId: 4, email: 'override@example.com', name: 'Custom sender' }, + replyTo: 'reply@example.com', + cc: memberEmail('cc'), + bcc: memberEmail('bcc'), + text: htmlOnly ? undefined : 'Hello there.', + html, + }) + invocation.hogFunction.metadata = { + message_category_type: 'marketing', + tracking_enabled: true, + utm_tags_enabled: true, + } const result = await service.executeSendEmail(invocation, isTest) - expectSkipped(result) - expect(result.logs).toContainEqual( - expect.objectContaining({ level: 'info', message: UNAVAILABLE_LOG }) + expect(result.error).toBeUndefined() + expect(result).toMatchObject({ finished: true, emailSendResult: { status: 'accepted' } }) + expect(result.skipped).not.toBe(true) + expect(result.invocation.state.vmState?.stack).toEqual([{ success: true }]) + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + const input = (sendEmailSpy.mock.calls[0][0] as SendEmailCommand).input + const footer = `This email was sent by Example organization via PostHog with the PostHog sandbox sender. Organization ID: ${team.organization_id}.` + expect(input).toMatchObject({ + TenantName: 'sandbox-tenant', + ConfigurationSetName: 'sandbox-email', + FromEmailAddress: '"Example organization via PostHog" ', + FeedbackForwardingEmailAddress: 'fixed-sandbox@example.com', + Destination: { + ToAddresses: [`"Example member" <${memberEmail()}>`], + CcAddresses: [memberEmail('cc')], + BccAddresses: [memberEmail('bcc')], + }, + }) + expect(input.Content?.Simple?.Body?.Text).toEqual( + htmlOnly ? undefined : { Data: `Hello there.\n\n${footer}`, Charset: 'UTF-8' } + ) + const sentHtml = input.Content?.Simple?.Body?.Html?.Data + expect(sentHtml).toContain(expectedContent) + if (html.includes('${footer}`)).toBe(true) + expect(input.ReplyToAddresses).toBeUndefined() + const headerNames = input.Content?.Simple?.Headers?.map((header) => header.Name) + expect(headerNames).toContain('X-PostHog-Tracking-Code') + expect(headerNames).not.toContain('List-Unsubscribe') + expect(result.logs.filter((log) => log.message.startsWith('Ignoring custom sender'))).toEqual([ + expect.objectContaining({ + level: 'info', + message: + 'Ignoring custom sender and Reply-To settings: the sandbox sender uses a fixed identity.', + }), + ]) + expect(result.metrics.map((metric) => metric.metric_name)).toEqual( + isTest ? [] : ['email_sent', 'email_untracked'] + ) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email sent', + { is_test: isTest, recipient_count: 3, source: isTest ? 'test' : 'workflow' } ) - expect(capture).not.toHaveBeenCalled() + expect(result.messageAssets).toEqual(isTest ? [] : [expect.objectContaining({ html: sentHtml })]) } ) @@ -445,17 +518,31 @@ describe('EmailService', () => { const result = await service.executeSendEmail(invocation) - expectSkipped(result) - expect(result.logs).toContainEqual(expect.objectContaining({ level: 'info', message: UNAVAILABLE_LOG })) + expect(result.error).toBeUndefined() + expect(result.finished).toBe(true) + expect(sendEmailSpy).toHaveBeenCalledTimes(1) expect(claimAllOrNothingPair).not.toHaveBeenCalled() }) + it('fails the send when the template cannot keep its identification footer', async () => { + service = createSandboxService(true) + invocation.queueParameters = createSandboxParams({ html: '

Hello

', + '

Hello

', + null, + ], + [ + '

Hello

', + '

Hello

', + null, + ], + ['', null], + ['Hello </body>', '<pre>Hello &lt;/body&gt;</pre>', null], + ['<body><p>Hello</p><template><plaintext>Example', '<p>Hello</p><template><pre>Example</pre></template>', null], + ['<head><template><plaintext>Example', '<head><template><pre>Example</pre></template></head>', null], + [ + '<body><p>Hello</p><noscript><style>p {color:blue}', + '<p>Hello</p><div><style>p {color:blue}</style></div>', + null, + ], + [ + '<body><p>Hello</p><noscript>&lt;plaintext&gt;Example</noscript></body>', + '<p>Hello</p><div>&lt;plaintext&gt;Example</div>', + null, + ], + [ + '<body><table><tbody><tr><td>Hello</td></tr></tbody></table></body>', + '<td>Hello</td>', + '<textarea>Preview text', + ], + ['<body><p>Hello</p><noscript><style></noscript><!--', '<p>Hello</p>', null], + [ + '<body><p>Hello</p><noscript><style>/* </noscript><plaintext> */ p {color:blue}</style></noscript>', + '<style>/* </noscript><plaintext> */ p {color:blue}</style>', + null, + ], + [ + '<body><p>Hello</p><noscript><p title="</noscript><style>">Fallback</p></noscript></body>', + '<p title="</noscript><style>">Fallback</p>', + null, + ], + [ + '<body><noscript><style></noscript><script></style></noscript>Hello', + '<style></noscript><script></style>', + null, + ], + ['<body style="background:#525252;color:white"><p>Hello</p></body>', '<p>Hello</p>', null], + ])('retains the footer outside template content: %s', async (html, expectedContent, preheader) => { + const result = await sender.withIdentificationFooter( + params(html, undefined, preheader ?? undefined), + 'Example organization via PostHog', + 1 + ) + expect(result.html).toContain(expectedContent) + for (const scriptingEnabled of [false, true]) { + const root = parse(result.html!, { scriptingEnabled }).childNodes.find(defaultTreeAdapter.isElementNode) + const body = root?.childNodes + .filter(defaultTreeAdapter.isElementNode) + .find((node) => node.tagName === 'body') + expect(body?.childNodes.at(-1)).toMatchObject({ + tagName: 'div', + childNodes: [{ nodeName: '#text', value: footer }], + }) + } + expect(result.preheader).toBeUndefined() + }) + it.each([ + ['<body><p>Hello</p><script><!--<script>', null, null], + ['<body><p>Hello</p><template><script><!--<script>', null, null], + ['', '', 'The sandbox email template must include HTML or text content. Update the template and try again.'], + ])('rejects content that cannot retain the footer: %s', async (html, text, error) => { + await expect( + sender.withIdentificationFooter(params(html, text ?? undefined), 'Example organization via PostHog', 1) + ).rejects.toThrow( + error ?? + 'The sandbox email template could not retain its identification footer. Update the template and try again.' + ) + }) +}) diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index 37f4a9ada68c..a3bc0457604c 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -1,8 +1,13 @@ +import { type DefaultTreeAdapterMap, defaultTreeAdapter, parse, parseFragment, serialize } from 'parse5' + +import { CyclotronInvocationQueueParametersEmailType } from '~/cdp/schema/cyclotron' import { RedisV2 } from '~/common/redis/redis-v2' import { logger } from '~/common/utils/logger' import { captureTeamEvent } from '~/common/utils/posthog' import { TeamManager } from '~/common/utils/team-manager' +import { maybeAddPreheaderToEmail } from './helpers/preheader' + export interface SandboxEmailSenderConfig { enabled: boolean tenantName: string @@ -26,6 +31,87 @@ export type SandboxEmailBlockReason = 'switch_off' | 'recipient_not_member' | 'r const BLOCKED_EVENT_WINDOW_SECONDS = 3600 +function htmlBody(document: DefaultTreeAdapterMap['document']): DefaultTreeAdapterMap['element'] | undefined { + const root = document.childNodes.find(defaultTreeAdapter.isElementNode) + return root?.childNodes.find( + (node): node is DefaultTreeAdapterMap['element'] => + defaultTreeAdapter.isElementNode(node) && node.tagName === 'body' && node.namespaceURI === root.namespaceURI + ) +} + +function protectFooterAncestors( + document: DefaultTreeAdapterMap['document'], + body: DefaultTreeAdapterMap['element'] +): void { + const root = document.childNodes.find(defaultTreeAdapter.isElementNode)! + const visibleLayout = + 'display:block!important;position:static!important;overflow:visible!important;height:auto!important;max-height:none!important;clip:auto!important;clip-path:none!important;transform:none!important;opacity:1!important;visibility:visible!important;filter:none!important;contain:none!important;content-visibility:visible!important;zoom:1!important' + for (const ancestor of [root, body]) { + const style = ancestor.attrs.find((attribute) => attribute.name === 'style') + if (style) { + style.value = `${style.value};${visibleLayout}` + } else { + ancestor.attrs.push({ name: 'style', value: visibleLayout }) + } + } +} + +function appendHtmlFooter(html: string, footer: string, preheader?: string): string { + const preheaderText = parseFragment(preheader ?? '', { scriptingEnabled: false }) + const document = parse(maybeAddPreheaderToEmail(html, serialize(preheaderText, { scriptingEnabled: false })), { + scriptingEnabled: false, + }) + const body = htmlBody(document) + if (!body) { + throw new Error('The sandbox email template must have an HTML body. Update the template and try again.') + } + const nodes: DefaultTreeAdapterMap['childNode'][] = [...document.childNodes] + for (const node of nodes) { + if (defaultTreeAdapter.isElementNode(node)) { + if (node.tagName === 'plaintext' && node.namespaceURI === body.namespaceURI) { + node.tagName = 'pre' + } + if (node.tagName === 'noscript' && node.namespaceURI === body.namespaceURI) { + node.tagName = 'div' + } + nodes.push(...node.childNodes) + if (node.tagName === 'template' && node.namespaceURI === body.namespaceURI) { + nodes.push( + ...defaultTreeAdapter.getTemplateContent(node as DefaultTreeAdapterMap['template']).childNodes + ) + } + } + } + protectFooterAncestors(document, body) + const paragraph = defaultTreeAdapter.createElement('div', body.namespaceURI, [ + { + name: 'style', + value: 'all:initial!important;display:block!important;visibility:visible!important;opacity:1!important;font:12px sans-serif!important;color:#525252!important;background:#fff!important;padding:16px 0!important', + }, + ]) + defaultTreeAdapter.insertText(paragraph, footer) + defaultTreeAdapter.appendChild(body, paragraph) + const serialized = serialize(document, { scriptingEnabled: false }) + for (const scriptingEnabled of [false, true]) { + const deliveredFooter = htmlBody(parse(serialized, { scriptingEnabled }))?.childNodes.at(-1) + if ( + !deliveredFooter || + !defaultTreeAdapter.isElementNode(deliveredFooter) || + deliveredFooter.tagName !== 'div' || + deliveredFooter.namespaceURI !== body.namespaceURI || + deliveredFooter.attrs.find((attribute) => attribute.name === 'style')?.value !== paragraph.attrs[0].value || + deliveredFooter.childNodes.length !== 1 || + !defaultTreeAdapter.isTextNode(deliveredFooter.childNodes[0]) || + deliveredFooter.childNodes[0].value !== footer + ) { + throw new Error( + 'The sandbox email template could not retain its identification footer. Update the template and try again.' + ) + } + } + return serialized +} + export class SandboxEmailSender { public readonly canSend: boolean @@ -49,26 +135,69 @@ export class SandboxEmailSender { this.canSend = config.enabled && !emptySettings.length } + public async withIdentificationFooter( + params: CyclotronInvocationQueueParametersEmailType, + senderName: string, + teamId: number + ): Promise<CyclotronInvocationQueueParametersEmailType> { + if (!params.text && !params.html) { + throw new Error( + 'The sandbox email template must include HTML or text content. Update the template and try again.' + ) + } + const team = await this.teamManager.getTeam(teamId) + if (!team) { + throw new Error('Could not identify the organization sending this sandbox email. Try again.') + } + const footer = `This email was sent by ${senderName} with the PostHog sandbox sender. Organization ID: ${team.organization_id}.` + return { + ...params, + from: { ...params.from, name: senderName, email: this.config.fromAddress }, + replyTo: undefined, + preheader: undefined, + ...(params.text ? { text: `${params.text}\n\n${footer}` } : {}), + ...(params.html ? { html: appendHtmlFooter(params.html, footer, params.preheader) } : {}), + } + } + + public async captureSent(teamId: number, isTest: boolean, recipientCount: number): Promise<void> { + await this.captureEvent(teamId, 'workflows sandbox email sent', { + recipient_count: recipientCount, + source: isTest ? 'test' : 'workflow', + is_test: isTest, + }) + } + public async captureBlocked( teamId: number, isTest: boolean, reason: SandboxEmailBlockReason, blockedRecipientCount: number + ): Promise<void> { + await this.captureEvent( + teamId, + 'workflows sandbox email blocked', + { reason, blocked_recipient_count: blockedRecipientCount, is_test: isTest }, + () => this.isFirstBlockInWindow(teamId, isTest, reason) + ) + } + + private async captureEvent( + teamId: number, + event: string, + properties: Record<string, string | number | boolean>, + shouldCapture: () => Promise<boolean> = () => Promise.resolve(true) ): Promise<void> { try { - if (!(await this.isFirstBlockInWindow(teamId, isTest, reason))) { + if (!(await shouldCapture())) { return } const team = await this.teamManager.getTeam(teamId) if (team) { - captureTeamEvent(team, 'workflows sandbox email blocked', { - reason, - blocked_recipient_count: blockedRecipientCount, - is_test: isTest, - }) + captureTeamEvent(team, event, properties) } } catch (error) { - logger.warn('Could not capture sandbox email event', { teamId, error }) + logger.warn('Could not capture sandbox email event', { teamId, event, error }) } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 697c7316664a..6ce3592ce056 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1754,6 +1754,9 @@ importers: p-limit: specifier: 3.1.0 version: 3.1.0 + parse5: + specifier: 7.1.2 + version: 7.1.2 pg: specifier: ^8.6.0 version: 8.10.0 From b485589a98d77c001d822355ede3326f4aebd25e Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 06:10:47 +0000 Subject: [PATCH 06/17] chore(workflows): drop a sandbox docs line the worker does not back yet Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- docs/internal/workflows-email-sending-tiers.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index d310355a10c0..2e261ff92583 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -76,7 +76,6 @@ Membership snapshots expire within one minute, and the worker checks recipients A failed or stale membership check skips the send and lists the blocked addresses in the run log. The blocked event fires at most once an hour per team, reason, and send kind (workflow or test). A send that passes every check goes out through the fixed SES tenant, configuration set, and From address, whatever the step's custom sender and Reply-To say. -Django reads the same `SES_SANDBOX_FROM_ADDRESS` for the address it stores and shows, and the address must belong to the shared SES identity. Sandbox sends skip the team's tier budget, click and open tracking, and unsubscribe headers, because they reach only members of the sending organization. The worker parses and normalizes the HTML, then appends a footer that names the sender and the organization ID. It overrides root clipping and visibility styles and keeps the template's typography, so malformed markup may render differently from an own-domain send. From 189acafec5a4220f4ea26e07186c8dd91cca627e Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 06:17:10 +0000 Subject: [PATCH 07/17] chore(workflows): keep the sandbox switch off until its sends leave tier history Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- docs/internal/workflows-email-sending-tiers.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 2e261ff92583..e2fb2b62bb15 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -82,4 +82,5 @@ It overrides root clipping and visibility styles and keeps the template's typogr A template whose HTML cannot keep the footer fails the send. The visible footer is best effort across email clients and template CSS; the signed tracking header still attributes the message. Each delivered send emits a `workflows sandbox email sent` event. -Keep the switch off in production until the sandbox caps are deployed. +Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. +Keep the switch off in production until the sandbox caps and that metric are deployed. From 58e01dcfecefe97d76059d529666818180ee94ab Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 06:42:04 +0000 Subject: [PATCH 08/17] feat(workflows): cap sandbox sends per project and recipient each day Sandbox sends claim a daily allowance per project and per recipient address across the organization right before delivery. A reached allowance skips the send. SES throttles give the claim back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../internal/workflows-email-sending-tiers.md | 8 +- nodejs/src/cdp/cdp-services.ts | 26 +- nodejs/src/cdp/config.ts | 5 + .../services/messaging/email.service.test.ts | 285 ++++++++++++++++-- .../cdp/services/messaging/email.service.ts | 71 ++++- .../messaging/sandbox-email-sender.test.ts | 8 +- .../messaging/sandbox-email-sender.ts | 90 +++++- .../rate-limiter/rate-limiter.service.test.ts | 109 +++++++ .../rate-limiter/rate-limiter.service.ts | 151 ++++++++++ nodejs/src/server.ts | 23 +- 10 files changed, 722 insertions(+), 54 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index e2fb2b62bb15..539917773f47 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -81,6 +81,12 @@ The worker parses and normalizes the HTML, then appends a footer that names the It overrides root clipping and visibility styles and keeps the template's typography, so malformed markup may render differently from an own-domain send. A template whose HTML cannot keep the footer fails the send. The visible footer is best effort across email clients and template CSS; the signed tracking header still attributes the message. +Right before delivery, every sandbox send, test sends included, claims a daily allowance per project (`WORKFLOWS_SANDBOX_DAILY_TEAM_CAP`, one token per recipient copy) and per recipient address across the organization (`WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP`). +The allowances are token buckets that refill their capacity over 24 hours, not calendar-day counters. +A reached allowance skips the send instead of delaying it. A limiter error, or an empty or invalid cap value, skips every sandbox send. +The worker checks members after the claim and gives the claim back when that check skips the send. +An SES throttle response also gives the claim back before the delivery retry. Returns are deduplicated, and Valkey ignores a return that arrives more than five minutes after it was issued. +Test sends run inline in the CDP API, so a sandbox-enabled CDP API opens the SES Valkey pool (`SES_RATE_LIMITER_VALKEY_*`) for the allowances. Without it, every sandbox test send skips. Each delivered send emits a `workflows sandbox email sent` event. Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. -Keep the switch off in production until the sandbox caps and that metric are deployed. +Keep the switch off in production until that metric is deployed. diff --git a/nodejs/src/cdp/cdp-services.ts b/nodejs/src/cdp/cdp-services.ts index 3b60a0e40a33..7950d0666c12 100644 --- a/nodejs/src/cdp/cdp-services.ts +++ b/nodejs/src/cdp/cdp-services.ts @@ -173,6 +173,8 @@ export type CdpCoreServicesConfig = Pick< | 'SES_SANDBOX_TENANT_NAME' | 'SES_SANDBOX_CONFIGURATION_SET' | 'SES_SANDBOX_FROM_ADDRESS' + | 'WORKFLOWS_SANDBOX_DAILY_TEAM_CAP' + | 'WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP' | 'EMAIL_SUPPRESSION_TRANSIENT_BOUNCE_THRESHOLD' | 'EMAIL_TEAM_SENDING_CAP_MODE' | 'EMAIL_TEAM_SENDING_CAP_HOURLY_BY_TIER' @@ -214,13 +216,15 @@ export interface CdpCoreServicesDeps { /** * SES Valkey pool shared with the SES rate limiter, opened only on pods whose * capabilities actually execute email actions (hogflow/email cyclotron workers). - * `null` on every other CDP consumer and on cdp-api so idle pods don't hold - * open connections against the SES Valkey instance. + * Sandbox-enabled cdp-api opens it too, for the sandbox daily caps of test sends. + * `null` on every other CDP consumer so idle pods don't hold open connections + * against the SES Valkey instance. */ emailValidationValkey: RedisV2 | null /** - * Dedicated frequency cap Valkey pool, opened only on the same pods as `emailValidationValkey`. - * `null` elsewhere, and when no host is configured, which turns the cap off. + * Dedicated frequency cap Valkey pool, opened only on the email workers. `null` elsewhere, + * cdp-api included because its test sends skip the cap, and when no host is configured, + * which turns the cap off. */ frequencyCapValkey: RedisV2 | null } @@ -458,6 +462,9 @@ export function createCdpCoreServices( const teamEmailRateLimiter = deps.emailValidationValkey ? new RateLimiterService(deps.emailValidationValkey, { name: 'team-email' }) : null + const sandboxEmailRateLimiter = deps.emailValidationValkey + ? new RateLimiterService(deps.emailValidationValkey, { name: 'sandbox-email' }) + : null const emailService = new EmailService( { sesAccessKeyId: config.SES_ACCESS_KEY_ID, @@ -486,9 +493,12 @@ export function createCdpCoreServices( tenantName: config.SES_SANDBOX_TENANT_NAME, configurationSetName: config.SES_SANDBOX_CONFIGURATION_SET, fromAddress: config.SES_SANDBOX_FROM_ADDRESS, + dailyTeamCap: config.WORKFLOWS_SANDBOX_DAILY_TEAM_CAP, + dailyRecipientCap: config.WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP, }, deps.teamManager, - redis + redis, + sandboxEmailRateLimiter ), new OrganizationMembersService(deps.postgres) ) @@ -556,9 +566,9 @@ export function createCdpCoreServices( : undefined ) // MX verdicts live on the dedicated SES Valkey (same instance as the SES rate - // limiter, separate pool). The pool is created by the server only on pods - // whose capabilities execute email actions; everywhere else this is null - // and EmailValidationService degrades to the local cache + DNS. + // limiter, separate pool). The server creates the pool only on pods whose + // capabilities execute email actions and on sandbox-enabled cdp-api; everywhere + // else this is null and EmailValidationService degrades to the local cache + DNS. const emailValidationService = new EmailValidationService(deps.emailValidationValkey) const cohortMembershipRepository = new PostgresCohortMembershipRepository(deps.postgres) // Observer writes to both stores; the read source decides which verdict drives the metric. diff --git a/nodejs/src/cdp/config.ts b/nodejs/src/cdp/config.ts index 33d291e3b187..1e09831724e0 100644 --- a/nodejs/src/cdp/config.ts +++ b/nodejs/src/cdp/config.ts @@ -171,6 +171,9 @@ export type CdpConfig = ClickhouseConfig & { SES_SANDBOX_TENANT_NAME: string SES_SANDBOX_CONFIGURATION_SET: string SES_SANDBOX_FROM_ADDRESS: string + // Daily sandbox allowances per project and per recipient address. An empty or invalid value skips every sandbox send. + WORKFLOWS_SANDBOX_DAILY_TEAM_CAP: string + WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP: string // Comma-separated allowlist of SNS Topic ARNs the SES webhook accepts events from. Empty string // means no restriction (dev/test); production should set this to the workflow SES topic ARN(s). SES_ALLOWED_SNS_TOPIC_ARNS: string @@ -416,6 +419,8 @@ export function getDefaultCdpConfig(): CdpConfig { SES_SANDBOX_TENANT_NAME: '', SES_SANDBOX_CONFIGURATION_SET: '', SES_SANDBOX_FROM_ADDRESS: '', + WORKFLOWS_SANDBOX_DAILY_TEAM_CAP: '25', + WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP: '5', SES_ALLOWED_SNS_TOPIC_ARNS: '', EMAIL_SUPPRESSION_TRANSIENT_BOUNCE_THRESHOLD: 5, diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 3dedc6f60306..c787988f003c 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -14,7 +14,8 @@ import { } from '~/cdp/schema/cyclotron' import { CyclotronJobInvocationHogFunction, CyclotronJobInvocationResult } from '~/cdp/types' import { IngestionOutputs } from '~/common/outputs/ingestion-outputs' -import { RedisClient, createRedisV2PoolFromConfig } from '~/common/redis/redis-v2' +import { deleteKeysWithPrefix } from '~/common/redis/_tests/redis' +import { RedisClient, RedisV2, createRedisV2PoolFromConfig } from '~/common/redis/redis-v2' import { closeHub, createHub } from '~/common/utils/db/hub' import { PostgresRouter, PostgresUse } from '~/common/utils/db/postgres' import * as posthog from '~/common/utils/posthog' @@ -259,12 +260,32 @@ describe('EmailService', () => { ) return id } + const sandboxRedis: RedisV2 = { + useClient: async (_options, callback) => { + const client = await hub.redisPool.acquire() + try { + return await callback(client as unknown as RedisClient) + } finally { + await hub.redisPool.release(client) + } + }, + usePipeline: () => Promise.reject(new Error('The sandbox tests do not use pipelines')), + } const createSandboxService = ( enabled: boolean, - membersPostgres: PostgresRouter = hub.postgres, - routeSettings: Partial<SandboxEmailSenderConfig> = {}, - teamEmailRateLimiter: RateLimiterService | null = null, - messageAssetsService?: MessageAssetsService + { + membersPostgres = hub.postgres, + senderSettings = {}, + teamEmailRateLimiter = null, + messageAssetsService, + dailyCapLimiter = new RateLimiterService(sandboxRedis, { name: 'sandbox-email' }), + }: { + membersPostgres?: PostgresRouter + senderSettings?: Partial<SandboxEmailSenderConfig> + teamEmailRateLimiter?: RateLimiterService | null + messageAssetsService?: MessageAssetsService + dailyCapLimiter?: RateLimiterService | null + } = {} ): EmailService => { const workflowsConfig = new TeamWorkflowsConfigService(hub.postgres, hub.pubSub) jest.spyOn(workflowsConfig, 'getEmailSendingTier').mockResolvedValue(0) @@ -296,19 +317,13 @@ describe('EmailService', () => { tenantName: 'sandbox-tenant', configurationSetName: 'sandbox-email', fromAddress: 'fixed-sandbox@example.com', - ...routeSettings, + dailyTeamCap: '100', + dailyRecipientCap: '100', + ...senderSettings, }, hub.teamManager, - { - useClient: async (_options, callback) => { - const client = await hub.redisPool.acquire() - try { - return await callback(client as unknown as RedisClient) - } finally { - await hub.redisPool.release(client) - } - }, - } + sandboxRedis, + dailyCapLimiter ), new OrganizationMembersService(membersPostgres) ) @@ -372,15 +387,8 @@ describe('EmailService', () => { [extraOrganizationIds], 'test:delete-sandbox-organizations' ) - const client = await hub.redisPool.acquire() - try { - const keys = await client.keys(`@posthog/workflows-sandbox-blocked/${team.id}/*`) - if (keys.length) { - await client.del(...keys) - } - } finally { - await hub.redisPool.release(client) - } + await deleteKeysWithPrefix(sandboxRedis, `@posthog/workflows-sandbox-blocked/${team.id}/`) + await deleteKeysWithPrefix(sandboxRedis, `@posthog/workflows-sandbox-daily/{${team.organization_id}}`) capture.mockRestore() }) @@ -393,7 +401,7 @@ describe('EmailService', () => { ] as const)( 'skips when the sandbox sender is switched off because %s (isTest=%s)', async (_reason, isTest, enabled, routeSettings: Partial<SandboxEmailSenderConfig>) => { - service = createSandboxService(enabled, hub.postgres, routeSettings) + service = createSandboxService(enabled, { senderSettings: routeSettings }) const result = await service.executeSendEmail(invocation, isTest) @@ -440,7 +448,7 @@ describe('EmailService', () => { const outputs = { produce: jest.fn().mockResolvedValue(undefined), } as unknown as IngestionOutputs<'message_assets'> - service = createSandboxService(true, hub.postgres, {}, null, new MessageAssetsService(outputs)) + service = createSandboxService(true, { messageAssetsService: new MessageAssetsService(outputs) }) invocation.state.actionId = 'send-email' invocation.queueParameters = createSandboxParams({ from: { integrationId: 4, email: 'override@example.com', name: 'Custom sender' }, @@ -512,9 +520,9 @@ describe('EmailService', () => { const claimAllOrNothingPair = jest .fn() .mockResolvedValue({ granted: false, deniedIndex: 0, retryAfterMs: 60_000, reserved: false }) - service = createSandboxService(true, hub.postgres, {}, { - claimAllOrNothingPair, - } as unknown as RateLimiterService) + service = createSandboxService(true, { + teamEmailRateLimiter: { claimAllOrNothingPair } as unknown as RateLimiterService, + }) const result = await service.executeSendEmail(invocation) @@ -679,7 +687,7 @@ describe('EmailService', () => { it('fails closed when the member check fails', async () => { const membersPostgres = new PostgresRouter(hub) await membersPostgres.end() - service = createSandboxService(true, membersPostgres) + service = createSandboxService(true, { membersPostgres }) invocation.queueParameters = createSandboxParams({ cc: memberEmail('cc') }) const result = await service.executeSendEmail(invocation) @@ -747,6 +755,221 @@ describe('EmailService', () => { expect(capture).toHaveBeenCalledTimes(3) expectBlockedEvent('switch_off', false, 0) }) + + describe('daily caps', () => { + const TEAM_CAP_REACHED = + "Skipping send: this email would go over the sandbox sender's daily limit for this project. Verify your own domain to send more." + const recipientCapReached = (addresses: string): string => + `Skipping send: these addresses reached the sandbox sender's daily limit per address: ${addresses}. Verify your own domain to send more.` + const CAP_CHECK_FAILED = + "Skipping send: could not check the sandbox sender's daily limit. Try again later, or verify your own domain to send more." + + const send = async ( + isTest: boolean, + params: Partial<CyclotronInvocationQueueParametersEmailType> = {} + ): Promise<CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>> => { + invocation.state.vmState = { stack: [] } as any + invocation.queueParameters = createSandboxParams(params) + return await service.executeSendEmail(invocation, isTest) + } + const expectSent = (result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>): void => { + expect(result.error).toBeUndefined() + expect(result.skipped).not.toBe(true) + expect(result.invocation.state.vmState?.stack).toEqual([{ success: true }]) + } + const expectCapSkip = ( + result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, + isTest: boolean, + message: string, + reason: 'cap_reached' | 'check_failed' + ): void => { + expect(result).toMatchObject({ finished: true, skipped: true, metrics: [], messageAssets: [] }) + expect(result.error).toBeUndefined() + expect(result.invocation.queueScheduledAt).toBeUndefined() + expect(result.invocation.state.vmState?.stack).toEqual([{ success: false }]) + expect(result.logs).toContainEqual(expect.objectContaining({ level: 'info', message })) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + expect.objectContaining({ reason, is_test: isTest }) + ) + } + + it.each([false, true])( + 'holds the project to its daily cap without charging denied or own-sender sends (isTest=%s)', + async (isTest) => { + service = createSandboxService(true, { + senderSettings: { dailyTeamCap: '3', dailyRecipientCap: '1' }, + }) + + expectSent(await send(isTest, { from: { integrationId: 1 } })) + expectSent(await send(isTest)) + expectCapSkip( + await send(isTest, { cc: memberEmail('cc'), bcc: memberEmail('bcc') }), + isTest, + TEAM_CAP_REACHED, + 'cap_reached' + ) + expectSent(await send(isTest, { to: { email: memberEmail('cc') } })) + expectSent(await send(isTest, { to: { email: memberEmail('bcc') } })) + expectCapSkip(await send(isTest), isTest, TEAM_CAP_REACHED, 'cap_reached') + expect(sendEmailSpy).toHaveBeenCalledTimes(4) + } + ) + + it.each([false, true])( + 'skips a send with recipients at the daily cap and leaves the others their budget (isTest=%s)', + async (isTest) => { + service = createSandboxService(true, { + senderSettings: { dailyTeamCap: '5', dailyRecipientCap: '1' }, + }) + + expectSent(await send(isTest, { to: { email: memberEmail('cc') }, cc: memberEmail('bcc') })) + expectCapSkip( + await send(isTest, { cc: memberEmail('cc').toUpperCase(), bcc: memberEmail('bcc') }), + isTest, + recipientCapReached(`${memberEmail('cc').toUpperCase()}, ${memberEmail('bcc')}`), + 'cap_reached' + ) + expectSent(await send(isTest)) + expect(sendEmailSpy).toHaveBeenCalledTimes(2) + } + ) + + it('charges a repeated address once per address and once per copy for the project', async () => { + service = createSandboxService(true, { + senderSettings: { dailyTeamCap: '3', dailyRecipientCap: '1' }, + }) + + expectSent(await send(false, { cc: memberEmail().toUpperCase(), bcc: ` ${memberEmail()} ` })) + expectCapSkip( + await send(false, { to: { email: memberEmail('cc') } }), + false, + TEAM_CAP_REACHED, + 'cap_reached' + ) + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + }) + + it('refills a spent allowance over a day rather than seconds', async () => { + service = createSandboxService(true, { + senderSettings: { dailyTeamCap: '1', dailyRecipientCap: '1' }, + }) + expectSent(await send(true)) + const ageBuckets = async (ageMs: number): Promise<void> => { + await sandboxRedis.useClient({ name: 'test:age-sandbox-daily-caps' }, async (client) => { + const [seconds, microseconds] = await client.time() + const redisNow = Number(seconds) * 1000 + Math.floor(Number(microseconds) / 1000) + const keys = await client.keys( + `@posthog/workflows-sandbox-daily/{${team.organization_id}}/*` + ) + for (const key of keys.filter((key) => !key.includes('/claimed/'))) { + await client.hset(key, 'ts', redisNow - ageMs) + } + }) + } + + await ageBuckets(12 * 60 * 60 * 1000) + expectCapSkip(await send(true), true, TEAM_CAP_REACHED, 'cap_reached') + await ageBuckets(24 * 60 * 60 * 1000 + 1000) + expectSent(await send(true)) + expect(sendEmailSpy).toHaveBeenCalledTimes(2) + }) + + it.each([false, true])( + 'spends the daily caps once for a send that SES throttles before accepting it (isTest=%s)', + async (isTest) => { + service = createSandboxService(true, { + senderSettings: { dailyTeamCap: '2', dailyRecipientCap: '1' }, + }) + sendEmailSpy.mockRejectedValueOnce(new ThrottlingException('Rate exceeded')) + + const throttled = await send(isTest, { cc: memberEmail('cc') }) + expect(throttled.finished).toBe(false) + expectSent(await service.executeSendEmail(throttled.invocation, isTest)) + expectCapSkip( + await send(isTest, { to: { email: memberEmail('bcc') } }), + isTest, + TEAM_CAP_REACHED, + 'cap_reached' + ) + expect(sendEmailSpy).toHaveBeenCalledTimes(2) + } + ) + + it('checks members after a slow cap claim and gives the claim back when one left', async () => { + const now = Date.now() + const clock = jest.spyOn(Date, 'now').mockReturnValue(now) + const dailyCapLimiter = new RateLimiterService(sandboxRedis, { name: 'sandbox-email' }) + const claim = dailyCapLimiter.claimAllOrNothing.bind(dailyCapLimiter) + jest.spyOn(dailyCapLimiter, 'claimAllOrNothing').mockImplementationOnce(async (buckets) => { + const grant = await claim(buckets) + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_user SET is_email_verified = false WHERE id = $1', + [memberId], + 'test:revoke-sandbox-verification-during-cap-claim' + ) + clock.mockReturnValue(now + 61_000) + return grant + }) + try { + service = createSandboxService(true, { + dailyCapLimiter, + senderSettings: { dailyTeamCap: '1', dailyRecipientCap: '1' }, + }) + + const blocked = await send(false) + expect(blocked).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(sendEmailSpy).not.toHaveBeenCalled() + expectBlockedEvent('recipient_unverified', false, 1) + + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'UPDATE posthog_user SET is_email_verified = true WHERE id = $1', + [memberId], + 'test:restore-sandbox-verification' + ) + clock.mockReturnValue(now + 122_000) + expectSent(await send(false)) + } finally { + clock.mockRestore() + } + }) + + it.each([ + [ + 'the limiter is unreachable', + false, + { + dailyCapLimiter: new RateLimiterService( + { + useClient: () => Promise.reject(new Error('Connection is closed.')), + usePipeline: () => Promise.reject(new Error('Connection is closed.')), + }, + { name: 'sandbox-email' } + ), + }, + ], + ['no limiter is configured', true, { dailyCapLimiter: null }], + ['the project cap is empty', false, { senderSettings: { dailyTeamCap: '' } }], + ['the recipient cap is blank', true, { senderSettings: { dailyRecipientCap: ' ' } }], + ['the project cap is zero', true, { senderSettings: { dailyTeamCap: '0' } }], + ['the recipient cap is fractional', false, { senderSettings: { dailyRecipientCap: '1.5' } }], + ['the project cap has trailing text', true, { senderSettings: { dailyTeamCap: '25 per day' } }], + ['the recipient cap is negative', false, { senderSettings: { dailyRecipientCap: '-5' } }], + ] as const)('skips every send when %s (isTest=%s)', async (_name, isTest, options) => { + service = createSandboxService(true, options) + + expectCapSkip( + await send(isTest, { cc: memberEmail('cc') }), + isTest, + CAP_CHECK_FAILED, + 'check_failed' + ) + expect(sendEmailSpy).not.toHaveBeenCalled() + }) + }) }) describe('integration validation', () => { beforeEach(async () => { diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index b6410a5b2e65..1c1cebaf3b5c 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -35,7 +35,7 @@ import { EmailTrackingCodeSigner, TRACKING_CODE_HEADER_NAME } from './helpers/tr import { addUtmTagsToEmail, renderUtmOverrides, resolveUtmTags } from './helpers/utm' import { MessageAssetsService } from './message-assets.service' import { RecipientTokensService } from './recipient-tokens.service' -import { SandboxEmailBlockReason, SandboxEmailSender } from './sandbox-email-sender' +import { SandboxDailyCapGrant, SandboxEmailBlockReason, SandboxEmailSender } from './sandbox-email-sender' const sesThrottleResponsesTotal = new Counter({ name: 'cdp_ses_throttle_responses_total', @@ -441,8 +441,20 @@ const SANDBOX_RECIPIENT_BLOCK_COPY: Record< label: ' for these addresses: ', guidance: 'Try again, or verify your own domain to send to anyone.', }, + cap_reached: { + explanation: "Skipping send: these addresses reached the sandbox sender's daily limit per address", + label: ': ', + guidance: 'Verify your own domain to send more.', + }, } +const SANDBOX_CAP_SKIP_MESSAGES = { + project_cap_reached: + "Skipping send: this email would go over the sandbox sender's daily limit for this project. Verify your own domain to send more.", + check_failed: + "Skipping send: could not check the sandbox sender's daily limit. Try again later, or verify your own domain to send more.", +} as const + export class EmailService { sesV2Client: SESv2Client | null @@ -789,10 +801,15 @@ export class EmailService { ...(route.ccAddresses ?? []), ...(route.bccAddresses ?? []), ] + const capGrant = await this.claimSandboxDailyCaps(result, deliveryRecipients, isTest) + if (!capGrant) { + return result + } if (!(await this.checkSandboxRecipients(result, deliveryRecipients, isTest))) { + await this.sandboxSender!.returnDailyCaps(capGrant) return result } - await this.sendEmailWithSES(result, deliveryParams, from, false, isTest, route) + await this.sendSandboxEmail(result, deliveryParams, from, isTest, route, capGrant) await this.sandboxSender!.captureSent(invocation.teamId, isTest, deliveryRecipients.length) break } @@ -1051,10 +1068,56 @@ export class EmailService { if (!blockedRecipients.length) { return true } + await this.blockSandboxRecipients(result, blockedRecipients, reason, isTest) + return false + } + + private async blockSandboxRecipients( + result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, + blockedRecipients: string[], + reason: SandboxRecipientBlockReason, + isTest: boolean + ): Promise<void> { const { explanation, label, guidance } = SANDBOX_RECIPIENT_BLOCK_COPY[reason] this.skipSandboxSend(result, `${explanation}${label}${blockedRecipients.join(', ')}. ${guidance}`) - await this.sandboxSender?.captureBlocked(teamId, isTest, reason, blockedRecipients.length) - return false + await this.sandboxSender?.captureBlocked(result.invocation.teamId, isTest, reason, blockedRecipients.length) + } + + private async claimSandboxDailyCaps( + result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, + recipients: string[], + isTest: boolean + ): Promise<SandboxDailyCapGrant | null> { + const claim = await this.sandboxSender!.claimDailyCaps(result.invocation.teamId, recipients) + if (claim.type === 'granted') { + return claim + } + if (claim.type === 'recipient_cap_reached') { + await this.blockSandboxRecipients(result, claim.addresses, 'cap_reached', isTest) + return null + } + this.skipSandboxSend(result, SANDBOX_CAP_SKIP_MESSAGES[claim.type]) + const reason = claim.type === 'check_failed' ? 'check_failed' : 'cap_reached' + await this.sandboxSender!.captureBlocked(result.invocation.teamId, isTest, reason, recipients.length) + return null + } + + private async sendSandboxEmail( + result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, + params: CyclotronInvocationQueueParametersEmailType, + from: { email: string; name: string }, + isTest: boolean, + route: SesRoute, + capGrant: SandboxDailyCapGrant + ): Promise<void> { + try { + await this.sendEmailWithSES(result, params, from, false, isTest, route) + } catch (error) { + if (error instanceof SESThrottleError) { + await this.sandboxSender!.returnDailyCaps(capGrant) + } + throw error + } } // Returns a human-readable log string when any destination address is suppressed for the team, diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts index 5224c728a867..08ace91b93af 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts @@ -17,9 +17,12 @@ const sender = new SandboxEmailSender( tenantName: 'sandbox-tenant', configurationSetName: 'sandbox-email', fromAddress: 'fixed-sandbox@example.com', + dailyTeamCap: '25', + dailyRecipientCap: '5', }, teamManager, - blockedEventRedis + blockedEventRedis, + null ) const params = (html: string, text?: string, preheader?: string): CyclotronInvocationQueueParametersEmailType => ({ type: 'email', @@ -68,7 +71,8 @@ describe('sandbox email content', () => { fromAddress: ' fixed-sandbox@example.com ', }, teamManager, - blockedEventRedis + blockedEventRedis, + null ) const result = await paddedSender.withIdentificationFooter( params('<p>Hello</p>'), diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index a3bc0457604c..604858b86682 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -1,3 +1,4 @@ +import { createHash, randomUUID } from 'crypto' import { type DefaultTreeAdapterMap, defaultTreeAdapter, parse, parseFragment, serialize } from 'parse5' import { CyclotronInvocationQueueParametersEmailType } from '~/cdp/schema/cyclotron' @@ -6,13 +7,19 @@ import { logger } from '~/common/utils/logger' import { captureTeamEvent } from '~/common/utils/posthog' import { TeamManager } from '~/common/utils/team-manager' +import { ClaimRequest, RateLimiterService } from '../rate-limiter/rate-limiter.service' import { maybeAddPreheaderToEmail } from './helpers/preheader' +const SECONDS_PER_DAY = 86400 +const DAILY_CAP_TTL_SECONDS = 2 * SECONDS_PER_DAY + export interface SandboxEmailSenderConfig { enabled: boolean tenantName: string configurationSetName: string fromAddress: string + dailyTeamCap: string + dailyRecipientCap: string } const ROUTE_SETTING_NAMES = { @@ -27,7 +34,49 @@ function emptyRouteSettings(config: SandboxEmailSenderConfig): string[] { .map((setting) => ROUTE_SETTING_NAMES[setting]) } -export type SandboxEmailBlockReason = 'switch_off' | 'recipient_not_member' | 'recipient_unverified' | 'check_failed' +export type SandboxEmailBlockReason = + | 'switch_off' + | 'recipient_not_member' + | 'recipient_unverified' + | 'check_failed' + | 'cap_reached' + +export type SandboxDailyCapGrant = { type: 'granted'; claimId: string; buckets: ClaimRequest[] } + +export type SandboxDailyCapClaim = + | SandboxDailyCapGrant + | { type: 'project_cap_reached' } + | { type: 'recipient_cap_reached'; addresses: string[] } + | { type: 'check_failed' } + +function dailyBucket(key: string, requested: number, capacity: number): ClaimRequest { + return { key, requested, capacity, refillPerSecond: capacity / SECONDS_PER_DAY, ttlSeconds: DAILY_CAP_TTL_SECONDS } +} + +function parseDailyCap(value: string): number | null { + const cap = /^\d+$/.test(value) ? Number(value) : null + return cap !== null && Number.isSafeInteger(cap) && cap > 0 ? cap : null +} + +// The organization hash tag keeps the project and recipient buckets in one Valkey cluster slot. +function dailyCapKeyPrefix(organizationId: string): string { + return `@posthog/workflows-sandbox-daily/{${organizationId}}` +} + +function addressDigest(address: string): string { + return createHash('sha256').update(address.toLowerCase()).digest('hex') +} + +function distinctAddresses(recipients: string[]): string[] { + const addresses = new Map<string, string>() + for (const recipient of recipients) { + const trimmed = recipient.trim() + if (!addresses.has(trimmed.toLowerCase())) { + addresses.set(trimmed.toLowerCase(), trimmed) + } + } + return [...addresses.values()] +} const BLOCKED_EVENT_WINDOW_SECONDS = 3600 @@ -118,7 +167,8 @@ export class SandboxEmailSender { constructor( public readonly config: SandboxEmailSenderConfig, private teamManager: TeamManager, - private blockedEventRedis: Pick<RedisV2, 'useClient'> + private blockedEventRedis: Pick<RedisV2, 'useClient'>, + private dailyCapLimiter: RateLimiterService | null ) { this.config = { ...config, @@ -135,6 +185,42 @@ export class SandboxEmailSender { this.canSend = config.enabled && !emptySettings.length } + public async claimDailyCaps(teamId: number, recipients: string[]): Promise<SandboxDailyCapClaim> { + const dailyTeamCap = parseDailyCap(this.config.dailyTeamCap) + const dailyRecipientCap = parseDailyCap(this.config.dailyRecipientCap) + if (!this.dailyCapLimiter || dailyTeamCap === null || dailyRecipientCap === null) { + logger.error('Sandbox email daily caps are not configured correctly', { teamId }) + return { type: 'check_failed' } + } + const team = await this.teamManager.getTeam(teamId) + if (!team) { + return { type: 'check_failed' } + } + const prefix = dailyCapKeyPrefix(team.organization_id) + const addresses = distinctAddresses(recipients) + const buckets = [ + dailyBucket(`${prefix}/project/${teamId}`, recipients.length, dailyTeamCap), + ...addresses.map((address) => + dailyBucket(`${prefix}/recipient/${addressDigest(address)}`, 1, dailyRecipientCap) + ), + ] + const claim = await this.dailyCapLimiter.claimAllOrNothing(buckets) + if (claim.granted) { + return { type: 'granted', claimId: randomUUID(), buckets } + } + if (claim.deniedIndexes === null) { + return { type: 'check_failed' } + } + if (claim.deniedIndexes.includes(0)) { + return { type: 'project_cap_reached' } + } + return { type: 'recipient_cap_reached', addresses: claim.deniedIndexes.map((index) => addresses[index - 1]) } + } + + public async returnDailyCaps(grant: SandboxDailyCapGrant): Promise<void> { + await this.dailyCapLimiter?.returnClaim(grant.claimId, grant.buckets) + } + public async withIdentificationFooter( params: CyclotronInvocationQueueParametersEmailType, senderName: string, diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts index 490f0fee49ef..effd952bdee7 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts @@ -4,8 +4,10 @@ import { register } from 'prom-client' import { deleteKeysWithPrefix } from '~/common/redis/_tests/redis' import { RedisV2, createRedisV2PoolFromConfig } from '~/common/redis/redis-v2' import { closeHub, createHub } from '~/common/utils/db/hub' +import { TeamManager } from '~/common/utils/team-manager' import { Hub } from '~/types' +import { SandboxEmailSender } from '../messaging/sandbox-email-sender' import { RateLimiterService } from './rate-limiter.service' const KEY = `@posthog-test/ses-rate-limiter/${randomUUID()}/bucket` @@ -413,4 +415,111 @@ describe('RateLimiterService', () => { expect([first.reserved, second.reserved, third.reserved]).toEqual([true, false, false]) }) }) + + it('shares recipient allowances across projects in an organization while keeping project allowances separate', async () => { + const organizations = { 1: randomUUID(), 3: randomUUID() } + const teamManager = { + getTeam: (id: number) => + Promise.resolve({ organization_id: id === 3 ? organizations[3] : organizations[1] }), + } as unknown as TeamManager + const sender = new SandboxEmailSender( + { + enabled: true, + tenantName: 'sandbox', + configurationSetName: 'sandbox', + fromAddress: 'sender@example.com', + dailyTeamCap: '1', + dailyRecipientCap: '1', + }, + teamManager, + redis, + limiter + ) + + try { + expect(await sender.claimDailyCaps(1, ['member@example.com'])).toMatchObject({ type: 'granted' }) + expect(await sender.claimDailyCaps(2, ['member@example.com'])).toEqual({ + type: 'recipient_cap_reached', + addresses: ['member@example.com'], + }) + expect(await sender.claimDailyCaps(2, ['other@example.com'])).toMatchObject({ type: 'granted' }) + expect(await sender.claimDailyCaps(3, ['member@example.com'])).toMatchObject({ type: 'granted' }) + expect(await sender.claimDailyCaps(2, ['third@example.com'])).toEqual({ type: 'project_cap_reached' }) + } finally { + for (const organizationId of Object.values(organizations)) { + await deleteKeysWithPrefix(redis, `@posthog/workflows-sandbox-daily/{${organizationId}}`) + } + } + }) + + describe('claimAllOrNothing', () => { + it('spends an unanswered claim once when the same EVAL is replayed', async () => { + const bucket = { key: `${KEY}/claim-replay`, requested: 1, capacity: 2, refillPerSecond: 0 } + const replayingRedis: RedisV2 = { + ...redis, + useClient: async (options, callback) => + redis.useClient(options, async (client) => { + const evaluate = client.eval.bind(client) + const replay = jest.spyOn(client, 'eval').mockImplementation(async (...args) => { + await evaluate(...args) + return evaluate(...args) + }) + try { + return await callback(client) + } finally { + replay.mockRestore() + } + }), + } + const replayingLimiter = new RateLimiterService(replayingRedis, { name: 'ses-rate-limiter' }) + + expect(await replayingLimiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) + }) + + it('reports a limiter error without naming a denied bucket', async () => { + const unreachable = new RateLimiterService( + { + useClient: () => Promise.reject(new Error('Connection is closed.')), + usePipeline: () => Promise.reject(new Error('Connection is closed.')), + }, + { name: 'ses-rate-limiter' } + ) + + expect( + await unreachable.claimAllOrNothing([ + { key: `${KEY}/unreachable`, requested: 1, capacity: 1, refillPerSecond: 0 }, + ]) + ).toEqual({ granted: false, deniedIndexes: null }) + }) + }) + + describe('returnClaim', () => { + it('gives a claim back once, even when the return is replayed', async () => { + const bucket = { key: `${KEY}/return`, requested: 1, capacity: 2, refillPerSecond: 0 } + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + + await limiter.returnClaim('claim-1', [bucket]) + await limiter.returnClaim('claim-1', [bucket]) + + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) + }) + + it('ignores a return that reaches Valkey after its window', async () => { + const bucket = { key: `${KEY}/late-return`, requested: 1, capacity: 1, refillPerSecond: 0 } + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + + const issuedLongAgo = jest.spyOn(Date, 'now').mockReturnValue(Date.now() - 60 * 60 * 1000) + try { + await limiter.returnClaim('claim-1', [bucket]) + } finally { + issuedLongAgo.mockRestore() + } + + expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) + }) + }) }) diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts index 1d4b178a0574..3e09da89b2cb 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'crypto' import { Counter, Histogram } from 'prom-client' import { RedisV2 } from '~/common/redis/redis-v2' @@ -249,6 +250,77 @@ end return {1, 0, 0, 0} ` +const CLAIM_ALL_OR_NOTHING_LUA = ` +local time = redis.call('TIME') +local now = tonumber(time[1]) * 1000 + math.floor(tonumber(time[2]) / 1000) +local claimKey = KEYS[#KEYS] +local previous = redis.call('GET', claimKey) +if previous then + return cjson.decode(previous) +end +if now > tonumber(ARGV[#ARGV - 1]) then + return {0} +end +local replayTtlMs = tonumber(ARGV[#ARGV]) +local available = {} +local denied = {0} +for i = 1, #KEYS - 1 do + local offset = (i - 1) * 4 + local requested = tonumber(ARGV[offset + 1]) + local capacity = tonumber(ARGV[offset + 2]) + local refillPerSecond = tonumber(ARGV[offset + 3]) + local existing = redis.call('hmget', KEYS[i], 'ts', 'pool') + local avail + if existing[1] == false then + avail = capacity + else + local elapsedMs = math.max(0, now - tonumber(existing[1])) + local currentTokens = capacity + if existing[2] ~= false then + currentTokens = tonumber(existing[2]) + end + avail = math.min(capacity, currentTokens + (elapsedMs / 1000.0) * refillPerSecond) + end + available[i] = avail + if avail < requested then + table.insert(denied, i) + end +end +if #denied > 1 then + redis.call('SET', claimKey, cjson.encode(denied), 'NX', 'PX', replayTtlMs) + return denied +end +for i = 1, #KEYS - 1 do + local offset = (i - 1) * 4 + redis.call('hset', KEYS[i], 'ts', now, 'pool', available[i] - tonumber(ARGV[offset + 1])) + redis.call('expire', KEYS[i], tonumber(ARGV[offset + 4])) +end +local granted = {1} +redis.call('SET', claimKey, cjson.encode(granted), 'NX', 'PX', replayTtlMs) +return granted +` + +// ioredis can replay unanswered commands, so returns need deduplication and an expiry. +const RETURN_CLAIM_LUA = ` +local time = redis.call('TIME') +local now = tonumber(time[1]) * 1000 + math.floor(tonumber(time[2]) / 1000) +local buckets = #KEYS - 1 +if now > tonumber(ARGV[buckets + 1]) then + return 0 +end +if not redis.call('set', KEYS[#KEYS], 1, 'NX', 'PX', ARGV[buckets + 2]) then + return 0 +end +for i = 1, buckets do + if redis.call('hexists', KEYS[i], 'pool') == 1 then + redis.call('hincrbyfloat', KEYS[i], 'pool', ARGV[i]) + end +end +return 1 +` + +const RETURN_WINDOW_MS = 5 * 60 * 1000 + export interface RateLimiterConfig { /** Logical name for metrics/logging only (e.g. 'ses'). */ name: string @@ -373,6 +445,61 @@ export class RateLimiterService { } } + // All bucket keys must share a hash tag because clustered Valkey rejects cross-slot Lua calls. + public async claimAllOrNothing( + buckets: ClaimRequest[] + ): Promise<{ granted: true } | { granted: false; deniedIndexes: number[] | null }> { + const endTimer = claimLatency.startTimer({ limiter: this.config.name }) + const keys = buckets.map((bucket) => bucket.key) + const claimId = randomUUID() + const deadline = Date.now() + RETURN_WINDOW_MS + try { + const result = await this.valkey.useClient( + { name: `rate-limiter:${this.config.name}:claimAllOrNothing`, timeout: 1000 }, + (client) => + client.eval( + CLAIM_ALL_OR_NOTHING_LUA, + buckets.length + 1, + ...keys, + `${keys[0]}/claimed/${claimId}`, + ...buckets.flatMap((bucket) => + [bucket.requested, bucket.capacity, bucket.refillPerSecond, bucket.ttlSeconds ?? 3600].map( + String + ) + ), + String(deadline), + String(2 * RETURN_WINDOW_MS) + ) + ) + if (Array.isArray(result) && result.length === 1 && result[0] === 1) { + claimCounter.inc({ limiter: this.config.name, result: 'granted_full' }) + return { granted: true } + } + if ( + Array.isArray(result) && + result[0] === 0 && + result.length > 1 && + result.slice(1).every((index) => Number.isInteger(index) && index >= 1 && index <= buckets.length) && + new Set(result.slice(1)).size === result.length - 1 + ) { + claimCounter.inc({ limiter: this.config.name, result: 'denied' }) + return { granted: false, deniedIndexes: result.slice(1).map((index) => index - 1) } + } + logger.warn('🪙', `RateLimiterService(${this.config.name}) claim returned invalid result`, { + keys, + raw: result, + }) + claimCounter.inc({ limiter: this.config.name, result: 'valkey_error' }) + return { granted: false, deniedIndexes: null } + } catch (err) { + logger.warn('🪙', `RateLimiterService(${this.config.name}) claim threw`, { keys, error: String(err) }) + claimCounter.inc({ limiter: this.config.name, result: 'valkey_error' }) + return { granted: false, deniedIndexes: null } + } finally { + endTimer() + } + } + /** * Atomically claim `requested` tokens from BOTH buckets, or neither. A denial consumes no * tokens, so a caller that retries a multi-token claim cannot drain the buckets while never @@ -449,4 +576,28 @@ export class RateLimiterService { endTimer() } } + + public async returnClaim(claimId: string, buckets: Pick<ClaimRequest, 'key' | 'requested'>[]): Promise<void> { + const keys = buckets.map((bucket) => bucket.key) + try { + await this.valkey.useClient( + { name: `rate-limiter:${this.config.name}:returnClaim`, timeout: 1000 }, + (client) => + client.eval( + RETURN_CLAIM_LUA, + buckets.length + 1, + ...keys, + `${keys[0]}/returned/${claimId}`, + ...buckets.map((bucket) => String(bucket.requested)), + String(Date.now() + RETURN_WINDOW_MS), + String(2 * RETURN_WINDOW_MS) + ) + ) + } catch (err) { + logger.warn('🪙', `RateLimiterService(${this.config.name}) claim return threw`, { + keys, + error: String(err), + }) + } + } } diff --git a/nodejs/src/server.ts b/nodejs/src/server.ts index 81391973e04b..a95a45ab3c39 100644 --- a/nodejs/src/server.ts +++ b/nodejs/src/server.ts @@ -137,8 +137,9 @@ export class PluginServer implements NodeServer { // Build typed deps objects for consumers. `emailValidationValkey` and `frequencyCapValkey` // are null in the shared deps and set only by the cyclotron workers that run the hogflow - // email action (see `withEmailActionValkeys` at their loaders below) — no - // other consumer touches the SES or frequency cap Valkey. + // email action (see `withEmailActionValkeys` at their loaders below). Sandbox-enabled + // cdp-api gets only the SES pool, because its test sends skip the frequency cap. + // No other consumer touches the SES or frequency cap Valkey. const cdpDeps: CdpConsumerBaseDeps | undefined = needsCdp ? { postgres: this.postgres!, @@ -245,7 +246,18 @@ export class PluginServer implements NodeServer { : null const api = new CdpApi( this.config, - { ...cdpDeps!, idempotencyValkey: createIdempotencyValkeyPool(this.config) }, + { + ...cdpDeps!, + ...(this.config.WORKFLOWS_SANDBOX_SENDER_ENABLED + ? { + emailValidationValkey: createSesRateLimiterValkeyPool( + this.config, + 'email-mx-validation' + ), + } + : {}), + idempotencyValkey: createIdempotencyValkeyPool(this.config), + }, { hogQueue: kafkaQueue, hogflowQueue: postgresV2Queue }, batchResolverProducer ) @@ -456,9 +468,8 @@ export class PluginServer implements NodeServer { /** * Grants the SES Valkey pool that backs the shared MX-verdict cache to a worker's - * deps. Only the cyclotron workers that run the hogflow email action call this, so - * the pool is never opened on other CDP consumers or cdp-api — an idle Valkey sized - * for the SES rate limiter shouldn't hold connections from pods that never validate. + * deps. Only the email workers call this. Sandbox-enabled cdp-api opens the SES pool on its + * own, without the frequency cap pool, and the pool stays closed on other CDP consumers. * Null when no SES Valkey host is configured (local dev), in which case * EmailValidationService degrades to its local cache + DNS. * From d68a2090b9eb08fc324d04c8c9562b335572e623 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 07:27:17 +0000 Subject: [PATCH 09/17] fix(workflows): keep sandbox cap failures inside the send The CDP API's SES Valkey pool no longer stops the process on repeated errors, and it bounds connection and command waits, so an outage skips sandbox sends instead. A claim now carries its own return deadline, so an old claim cannot refund into a refilled bucket. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../messaging/sandbox-email-sender.ts | 10 ++-- .../rate-limiter/rate-limiter-valkey-pool.ts | 10 +++- .../rate-limiter/rate-limiter.service.test.ts | 58 ++++++++++++++----- .../rate-limiter/rate-limiter.service.ts | 15 +++-- nodejs/src/server.ts | 3 +- 5 files changed, 67 insertions(+), 29 deletions(-) diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index 604858b86682..4df643e09d7c 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -1,4 +1,4 @@ -import { createHash, randomUUID } from 'crypto' +import { createHash } from 'crypto' import { type DefaultTreeAdapterMap, defaultTreeAdapter, parse, parseFragment, serialize } from 'parse5' import { CyclotronInvocationQueueParametersEmailType } from '~/cdp/schema/cyclotron' @@ -7,7 +7,7 @@ import { logger } from '~/common/utils/logger' import { captureTeamEvent } from '~/common/utils/posthog' import { TeamManager } from '~/common/utils/team-manager' -import { ClaimRequest, RateLimiterService } from '../rate-limiter/rate-limiter.service' +import { ClaimRequest, IssuedClaim, RateLimiterService } from '../rate-limiter/rate-limiter.service' import { maybeAddPreheaderToEmail } from './helpers/preheader' const SECONDS_PER_DAY = 86400 @@ -41,7 +41,7 @@ export type SandboxEmailBlockReason = | 'check_failed' | 'cap_reached' -export type SandboxDailyCapGrant = { type: 'granted'; claimId: string; buckets: ClaimRequest[] } +export type SandboxDailyCapGrant = { type: 'granted'; claim: IssuedClaim; buckets: ClaimRequest[] } export type SandboxDailyCapClaim = | SandboxDailyCapGrant @@ -206,7 +206,7 @@ export class SandboxEmailSender { ] const claim = await this.dailyCapLimiter.claimAllOrNothing(buckets) if (claim.granted) { - return { type: 'granted', claimId: randomUUID(), buckets } + return { type: 'granted', claim, buckets } } if (claim.deniedIndexes === null) { return { type: 'check_failed' } @@ -218,7 +218,7 @@ export class SandboxEmailSender { } public async returnDailyCaps(grant: SandboxDailyCapGrant): Promise<void> { - await this.dailyCapLimiter?.returnClaim(grant.claimId, grant.buckets) + await this.dailyCapLimiter?.returnClaim(grant.claim, grant.buckets) } public async withIdentificationFooter( diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter-valkey-pool.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter-valkey-pool.ts index 7e9eaed1011b..e6dfdb3842fc 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter-valkey-pool.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter-valkey-pool.ts @@ -24,9 +24,12 @@ export type RateLimiterValkeyConfig = Pick< * callers treat that as "feature disabled in this environment" and proceed * without it. */ +const OPTIONAL_POOL_TIMEOUT_MS = 1000 + export function createSesRateLimiterValkeyPool( config: RateLimiterValkeyConfig, - name = 'ses-rate-limiter' + name = 'ses-rate-limiter', + { optional = false }: { optional?: boolean } = {} ): RedisV2 | null { if (!config.SES_RATE_LIMITER_VALKEY_HOST) { logger.info('🪙', `[${name}] no SES Valkey host configured — feature disabled`) @@ -47,10 +50,13 @@ export function createSesRateLimiterValkeyPool( port: config.SES_RATE_LIMITER_VALKEY_PORT, password: config.SES_RATE_LIMITER_VALKEY_PASSWORD, tls, + ...(optional ? { commandTimeout: OPTIONAL_POOL_TIMEOUT_MS } : {}), }, name, + exitOnRepeatedErrors: !optional, }, - poolMinSize: config.REDIS_POOL_MIN_SIZE, + poolMinSize: optional ? 0 : config.REDIS_POOL_MIN_SIZE, poolMaxSize: config.REDIS_POOL_MAX_SIZE, + ...(optional ? { acquireTimeoutMillis: OPTIONAL_POOL_TIMEOUT_MS } : {}), }) } diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts index effd952bdee7..6ef291ebd57b 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts @@ -8,7 +8,8 @@ import { TeamManager } from '~/common/utils/team-manager' import { Hub } from '~/types' import { SandboxEmailSender } from '../messaging/sandbox-email-sender' -import { RateLimiterService } from './rate-limiter.service' +import { createSesRateLimiterValkeyPool } from './rate-limiter-valkey-pool' +import { ClaimRequest, IssuedClaim, RateLimiterService } from './rate-limiter.service' const KEY = `@posthog-test/ses-rate-limiter/${randomUUID()}/bucket` @@ -473,11 +474,33 @@ describe('RateLimiterService', () => { } const replayingLimiter = new RateLimiterService(replayingRedis, { name: 'ses-rate-limiter' }) - expect(await replayingLimiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) - expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await replayingLimiter.claimAllOrNothing([bucket])).toMatchObject({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toMatchObject({ granted: true }) expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) }) + it('fails closed through an optional pool whose Valkey is unreachable', async () => { + const unreachableValkey = createSesRateLimiterValkeyPool( + { + SES_RATE_LIMITER_VALKEY_HOST: '127.0.0.1', + SES_RATE_LIMITER_VALKEY_PORT: 1, + SES_RATE_LIMITER_VALKEY_PASSWORD: '', + SES_RATE_LIMITER_VALKEY_TLS: false, + REDIS_POOL_MIN_SIZE: 1, + REDIS_POOL_MAX_SIZE: 2, + }, + 'unreachable-test', + { optional: true } + )! + const unreachable = new RateLimiterService(unreachableValkey, { name: 'sandbox-email' }) + + expect( + await unreachable.claimAllOrNothing([ + { key: `${KEY}/optional-pool`, requested: 1, capacity: 1, refillPerSecond: 0 }, + ]) + ).toEqual({ granted: false, deniedIndexes: null }) + }) + it('reports a limiter error without naming a denied bucket', async () => { const unreachable = new RateLimiterService( { @@ -496,28 +519,31 @@ describe('RateLimiterService', () => { }) describe('returnClaim', () => { + const grantedClaim = async (bucket: ClaimRequest): Promise<IssuedClaim> => { + const claim = await limiter.claimAllOrNothing([bucket]) + if (!claim.granted) { + throw new Error('Expected the claim to be granted.') + } + return claim + } + it('gives a claim back once, even when the return is replayed', async () => { const bucket = { key: `${KEY}/return`, requested: 1, capacity: 2, refillPerSecond: 0 } - expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) - expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + const claim = await grantedClaim(bucket) + await grantedClaim(bucket) - await limiter.returnClaim('claim-1', [bucket]) - await limiter.returnClaim('claim-1', [bucket]) + await limiter.returnClaim(claim, [bucket]) + await limiter.returnClaim(claim, [bucket]) - expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + expect(await limiter.claimAllOrNothing([bucket])).toMatchObject({ granted: true }) expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) }) - it('ignores a return that reaches Valkey after its window', async () => { + it('ignores the return of a claim issued longer ago than its window', async () => { const bucket = { key: `${KEY}/late-return`, requested: 1, capacity: 1, refillPerSecond: 0 } - expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: true }) + const claim = await grantedClaim(bucket) - const issuedLongAgo = jest.spyOn(Date, 'now').mockReturnValue(Date.now() - 60 * 60 * 1000) - try { - await limiter.returnClaim('claim-1', [bucket]) - } finally { - issuedLongAgo.mockRestore() - } + await limiter.returnClaim({ ...claim, returnBy: Date.now() - 1000 }, [bucket]) expect(await limiter.claimAllOrNothing([bucket])).toEqual({ granted: false, deniedIndexes: [0] }) }) diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts index 3e09da89b2cb..f5d72eee2efd 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.ts @@ -326,6 +326,8 @@ export interface RateLimiterConfig { name: string } +export type IssuedClaim = { granted: true; claimId: string; returnBy: number } + export interface ClaimRequest { key: string requested: number @@ -448,7 +450,7 @@ export class RateLimiterService { // All bucket keys must share a hash tag because clustered Valkey rejects cross-slot Lua calls. public async claimAllOrNothing( buckets: ClaimRequest[] - ): Promise<{ granted: true } | { granted: false; deniedIndexes: number[] | null }> { + ): Promise<IssuedClaim | { granted: false; deniedIndexes: number[] | null }> { const endTimer = claimLatency.startTimer({ limiter: this.config.name }) const keys = buckets.map((bucket) => bucket.key) const claimId = randomUUID() @@ -473,7 +475,7 @@ export class RateLimiterService { ) if (Array.isArray(result) && result.length === 1 && result[0] === 1) { claimCounter.inc({ limiter: this.config.name, result: 'granted_full' }) - return { granted: true } + return { granted: true, claimId, returnBy: deadline } } if ( Array.isArray(result) && @@ -577,7 +579,10 @@ export class RateLimiterService { } } - public async returnClaim(claimId: string, buckets: Pick<ClaimRequest, 'key' | 'requested'>[]): Promise<void> { + public async returnClaim( + claim: Pick<IssuedClaim, 'claimId' | 'returnBy'>, + buckets: Pick<ClaimRequest, 'key' | 'requested'>[] + ): Promise<void> { const keys = buckets.map((bucket) => bucket.key) try { await this.valkey.useClient( @@ -587,9 +592,9 @@ export class RateLimiterService { RETURN_CLAIM_LUA, buckets.length + 1, ...keys, - `${keys[0]}/returned/${claimId}`, + `${keys[0]}/returned/${claim.claimId}`, ...buckets.map((bucket) => String(bucket.requested)), - String(Date.now() + RETURN_WINDOW_MS), + String(claim.returnBy), String(2 * RETURN_WINDOW_MS) ) ) diff --git a/nodejs/src/server.ts b/nodejs/src/server.ts index a95a45ab3c39..d623943478fe 100644 --- a/nodejs/src/server.ts +++ b/nodejs/src/server.ts @@ -252,7 +252,8 @@ export class PluginServer implements NodeServer { ? { emailValidationValkey: createSesRateLimiterValkeyPool( this.config, - 'email-mx-validation' + 'email-mx-validation', + { optional: true } ), } : {}), From e8da31621c6adce761428f2e74029e55484f6384 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 07:54:23 +0000 Subject: [PATCH 10/17] feat(workflows): mirror the sandbox ses tenant state in django Django records the pause state of the shared sandbox SES tenant in SandboxSenderTenantState, from the tenant-state webhook and the daily reconciliation, and announces each committed change to the workers. Nothing reads the table yet; the worker gate follows in its own PR. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .github/scripts/check-idor-model-coverage.py | 1 + .../internal/workflows-email-sending-tiers.md | 2 + posthog/plugins/plugin_server_api.py | 5 + posthog/settings/ses.py | 2 + .../0039_sandbox_sender_tenant_state.py | 32 +++++ .../backend/migrations/max_migration.txt | 2 +- products/workflows/backend/models/__init__.py | 2 + .../models/sandbox_sender_tenant_state.py | 9 ++ products/workflows/backend/providers/ses.py | 79 ++++++----- .../backend/services/ses_tenant_events.py | 34 ++++- .../backend/services/ses_tenant_state.py | 42 +++++- .../backend/tasks/ses_tenant_state.py | 19 ++- .../backend/test/test_ses_tenant_events.py | 29 +++++ .../backend/test/test_ses_tenant_state.py | 123 ++++++++++++++++++ 14 files changed, 339 insertions(+), 42 deletions(-) create mode 100644 products/workflows/backend/migrations/0039_sandbox_sender_tenant_state.py create mode 100644 products/workflows/backend/models/sandbox_sender_tenant_state.py diff --git a/.github/scripts/check-idor-model-coverage.py b/.github/scripts/check-idor-model-coverage.py index ffe2c08f1681..aa7d0b87aa9c 100644 --- a/.github/scripts/check-idor-model-coverage.py +++ b/.github/scripts/check-idor-model-coverage.py @@ -324,6 +324,7 @@ def get_scoped_models() -> tuple[dict[str, set[str]], set[str], set[str], set[st "DevStackImageTarget", "DevStackImageAttempt", "License", + "SandboxSenderTenantState", # instance-global mirror of the shared sandbox SES tenant state # --- Deprecated --- "Prompt", "PromptSequence", diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 539917773f47..67ce8d73d38d 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -87,6 +87,8 @@ A reached allowance skips the send instead of delaying it. A limiter error, or a The worker checks members after the claim and gives the claim back when that check skips the send. An SES throttle response also gives the claim back before the delivery retry. Returns are deduplicated, and Valkey ignores a return that arrives more than five minutes after it was issued. Test sends run inline in the CDP API, so a sandbox-enabled CDP API opens the SES Valkey pool (`SES_RATE_LIMITER_VALKEY_*`) for the allowances. Without it, every sandbox test send skips. +Django mirrors the tenant named by `SES_SANDBOX_TENANT_NAME` into `SandboxSenderTenantState` from the SES tenant-state webhook and the daily reconciliation, without emailing project admins. Set the same `SES_SANDBOX_TENANT_NAME` on Django web, Celery, and the worker. +A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Each delivered send emits a `workflows sandbox email sent` event. Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. Keep the switch off in production until that metric is deployed. diff --git a/posthog/plugins/plugin_server_api.py b/posthog/plugins/plugin_server_api.py index 4176b4f87395..7cbb333e652b 100644 --- a/posthog/plugins/plugin_server_api.py +++ b/posthog/plugins/plugin_server_api.py @@ -82,6 +82,11 @@ def reload_team_workflows_config_on_workers(team_id: int): publish_message("reload-team-workflows-config", {"teamId": team_id}) +def reload_sandbox_sender_state_on_workers(tenant_name: str) -> None: + logger.info(f"Reloading sandbox sender state for tenant {tenant_name} on workers") + publish_message("reload-sandbox-sender-state", {"tenantName": tenant_name}) + + def populate_plugin_capabilities_on_workers(plugin_id: str): logger.info(f"Populating plugin capabilities for plugin {plugin_id} on workers") publish_message("populate-plugin-capabilities", {"pluginId": plugin_id}) diff --git a/posthog/settings/ses.py b/posthog/settings/ses.py index 6cd9beaff4c7..578442af8cda 100644 --- a/posthog/settings/ses.py +++ b/posthog/settings/ses.py @@ -29,6 +29,8 @@ if cs.strip() ] +SES_SANDBOX_TENANT_NAME: str = os.getenv("SES_SANDBOX_TENANT_NAME", "").strip() + # SNS topics allowed to deliver SES tenant reputation events (EventBridge -> SNS -> webhook). # Empty (the default) leaves the webhook inert: the SNS signature proves a message came from AWS, # but only the allowlist proves it came from *our* topic. diff --git a/products/workflows/backend/migrations/0039_sandbox_sender_tenant_state.py b/products/workflows/backend/migrations/0039_sandbox_sender_tenant_state.py new file mode 100644 index 000000000000..e88cfac2bca9 --- /dev/null +++ b/products/workflows/backend/migrations/0039_sandbox_sender_tenant_state.py @@ -0,0 +1,32 @@ +# Generated by Django 5.2.18 on 2026-10-27 07:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("workflows", "0038_hogflow_team_source_record_idx"), + ] + + operations = [ + migrations.CreateModel( + name="SandboxSenderTenantState", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("tenant_name", models.CharField(max_length=64, unique=True)), + ( + "sending_status", + models.CharField(blank=True, db_default="", default="", max_length=32), + ), + ("synced_at", models.DateTimeField(blank=True, null=True)), + ], + ), + ] diff --git a/products/workflows/backend/migrations/max_migration.txt b/products/workflows/backend/migrations/max_migration.txt index 2ce22c5893e0..c3daea1fde11 100644 --- a/products/workflows/backend/migrations/max_migration.txt +++ b/products/workflows/backend/migrations/max_migration.txt @@ -1 +1 @@ -0038_hogflow_team_source_record_idx +0039_sandbox_sender_tenant_state diff --git a/products/workflows/backend/models/__init__.py b/products/workflows/backend/models/__init__.py index bbc6b3a22a6c..fdf755e421f6 100644 --- a/products/workflows/backend/models/__init__.py +++ b/products/workflows/backend/models/__init__.py @@ -4,6 +4,7 @@ from .hog_flow_optimization import HogFlowOptimization from .hog_flow_revision import HogFlowRevision from .hog_flow_schedule.hog_flow_schedule import HogFlowSchedule +from .sandbox_sender_tenant_state import SandboxSenderTenantState from .team_workflows_config import TeamWorkflowsConfig from .workflow_idea import WorkflowIdea from .workflow_proposal import WorkflowProposal @@ -15,6 +16,7 @@ "HogFlowRevision", "HogFlowSchedule", "HogFlowTemplate", + "SandboxSenderTenantState", "TeamWorkflowsConfig", "WorkflowIdea", "WorkflowProposal", diff --git a/products/workflows/backend/models/sandbox_sender_tenant_state.py b/products/workflows/backend/models/sandbox_sender_tenant_state.py new file mode 100644 index 000000000000..f6b9172850bc --- /dev/null +++ b/products/workflows/backend/models/sandbox_sender_tenant_state.py @@ -0,0 +1,9 @@ +from django.db import models + + +class SandboxSenderTenantState(models.Model): + """Instance-level mirror of the shared sandbox SES tenant; platform state, not tenant data, so no team_id.""" + + tenant_name = models.CharField(max_length=64, unique=True) + sending_status = models.CharField(max_length=32, blank=True, default="", db_default="") + synced_at = models.DateTimeField(null=True, blank=True) diff --git a/products/workflows/backend/providers/ses.py b/products/workflows/backend/providers/ses.py index 4ecb6490fda4..763abd7a8495 100644 --- a/products/workflows/backend/providers/ses.py +++ b/products/workflows/backend/providers/ses.py @@ -395,6 +395,13 @@ def _is_throttling(error: Exception) -> bool: return isinstance(error, ClientError) and error.response["Error"]["Code"] in ("Throttling", "ThrottlingException") +@frozen +class TenantSendingState: + sending_status: str + reputation_impact: str | None + tenant_arn: str | None + + class SESProvider: ses_client: "SESClient" ses_v2_client: "SESV2Client" @@ -469,7 +476,17 @@ def get_tenant_reputation(self, team_id: int) -> dict[str, Any] | None: provider feedback loops, third-party listings), so this is the authoritative health source; our own app metrics only provide the per-workflow diagnosis. """ - tenant_name = self._tenant_name_for_team(team_id) + state = self.get_tenant_sending_state(self._tenant_name_for_team(team_id)) + if state is None: + return None + return { + "sending_status": state.sending_status, + "reputation_impact": state.reputation_impact, + "findings": self._open_findings(state.tenant_arn) if state.tenant_arn else [], + } + + def get_tenant_sending_state(self, tenant_name: str) -> TenantSendingState | None: + """Sending status and reputation impact of the named SES tenant, or None when it doesn't exist.""" try: tenant = self.ses_v2_client.get_tenant(TenantName=tenant_name)["Tenant"] except ClientError as e: @@ -479,40 +496,38 @@ def get_tenant_reputation(self, team_id: int) -> dict[str, Any] | None: sending_status: str = tenant.get("SendingStatus", "ENABLED") tenant_arn = tenant.get("TenantArn") - reputation_impact: str | None = None - findings: list[dict[str, Any]] = [] + if not tenant_arn: + return TenantSendingState(sending_status=sending_status, reputation_impact=None, tenant_arn=None) - if tenant_arn: - try: - entity = self.ses_v2_client.get_reputation_entity( - ReputationEntityReference=tenant_arn, ReputationEntityType="RESOURCE" - )["ReputationEntity"] - except ClientError as e: - # A tenant with no attributed sends yet has no reputation entity. - if e.response["Error"]["Code"] != "NotFoundException": - raise - entity = {} - reputation_impact = entity.get("ReputationImpact") + entity = self._reputation_entity(tenant_arn) + return TenantSendingState( # The aggregate folds in both AWS-managed and customer-managed pauses. - sending_status = entity.get("SendingStatusAggregate", sending_status) - - # RESOURCE_ARN is the only filter key AWS documents as usable on its own with this - # scoping (see _iter_open_recommendations for why STATUS is filtered locally). - findings.extend( - { - "finding_type": recommendation.get("Type", ""), - "impact": recommendation.get("Impact", "LOW"), - "description": recommendation.get("Description", ""), - "last_updated_at": recommendation.get("LastUpdatedTimestamp"), - } - for recommendation in self._iter_open_recommendations({"RESOURCE_ARN": tenant_arn}) - ) + sending_status=entity.get("SendingStatusAggregate", sending_status), + reputation_impact=entity.get("ReputationImpact"), + tenant_arn=tenant_arn, + ) - return { - "sending_status": sending_status, - "reputation_impact": reputation_impact, - "findings": findings, - } + def _reputation_entity(self, tenant_arn: str) -> Mapping[str, Any]: + try: + return self.ses_v2_client.get_reputation_entity( + ReputationEntityReference=tenant_arn, ReputationEntityType="RESOURCE" + )["ReputationEntity"] + except ClientError as e: + # A tenant with no attributed sends yet has no reputation entity. + if e.response["Error"]["Code"] != "NotFoundException": + raise + return {} + + def _open_findings(self, tenant_arn: str) -> list[dict[str, Any]]: + return [ + { + "finding_type": recommendation.get("Type", ""), + "impact": recommendation.get("Impact", "LOW"), + "description": recommendation.get("Description", ""), + "last_updated_at": recommendation.get("LastUpdatedTimestamp"), + } + for recommendation in self._iter_open_recommendations({"RESOURCE_ARN": tenant_arn}) + ] def _iter_open_recommendations( self, finding_filter: dict[Any, str] | None, client: "SESV2Client | None" = None diff --git a/products/workflows/backend/services/ses_tenant_events.py b/products/workflows/backend/services/ses_tenant_events.py index 77672ddc31cf..423cd5837aa5 100644 --- a/products/workflows/backend/services/ses_tenant_events.py +++ b/products/workflows/backend/services/ses_tenant_events.py @@ -15,21 +15,22 @@ from collections.abc import Mapping from typing import Any +from django.conf import settings + import requests import structlog from posthog.ingress.contracts import WebhookDelivery from posthog.ingress.verify.sns_signature import is_valid_sns_url -from products.workflows.backend.tasks.ses_tenant_state import sync_ses_tenant_state_task +from products.workflows.backend.tasks.ses_tenant_state import sync_sandbox_tenant_state_task, sync_ses_tenant_state_task logger = structlog.get_logger(__name__) _TENANT_TEAM_RE = re.compile(r"\bteam-(\d+)\b") -def _extract_team_id(event: dict[str, Any]) -> int | None: - """Pull the `team-<id>` tenant name out of an EventBridge event, wherever AWS put it.""" +def _tenant_candidates(event: dict[str, Any]) -> list[str]: detail = event.get("detail") or {} candidates: list[Any] = [ detail.get("tenantName"), @@ -39,8 +40,24 @@ def _extract_team_id(event: dict[str, Any]) -> int | None: resources = event.get("resources") if isinstance(resources, list): candidates.extend(resources) - for candidate in candidates: - if isinstance(candidate, str) and (match := _TENANT_TEAM_RE.search(candidate)): + return [candidate for candidate in candidates if isinstance(candidate, str)] + + +def _names_sandbox_tenant(event: dict[str, Any]) -> bool: + tenant_name = settings.SES_SANDBOX_TENANT_NAME + if not tenant_name: + return False + tenant_path = re.compile(rf"(?:/|:)tenant/{re.escape(tenant_name)}(?:/|$)") + return any( + candidate == tenant_name or (candidate.startswith("arn:") and tenant_path.search(candidate) is not None) + for candidate in _tenant_candidates(event) + ) + + +def _extract_team_id(event: dict[str, Any]) -> int | None: + """Pull the `team-<id>` tenant name out of an EventBridge event, wherever AWS put it.""" + for candidate in _tenant_candidates(event): + if match := _TENANT_TEAM_RE.search(candidate): return int(match.group(1)) return None @@ -61,7 +78,7 @@ def _confirm_subscription(message: Mapping[str, Any]) -> None: def _handle_notification(message: Mapping[str, Any]) -> None: - """Enqueue a tenant-state sync for the team the EventBridge event names. + """Enqueue a tenant-state sync for the sandbox tenant or the team the EventBridge event names. Every unusable payload is dropped rather than raised, because a redelivery would carry the same unusable payload. @@ -73,6 +90,11 @@ def _handle_notification(message: Mapping[str, Any]) -> None: if not isinstance(event, dict) or event.get("source") != "aws.ses": return + if _names_sandbox_tenant(event): + logger.info("ses_tenant_events_webhook_accepted", tenant="sandbox", detail_type=event.get("detail-type")) + sync_sandbox_tenant_state_task.delay() + return + team_id = _extract_team_id(event) if team_id is None: logger.warning("ses_tenant_events_webhook_no_tenant", detail_type=event.get("detail-type")) diff --git a/products/workflows/backend/services/ses_tenant_state.py b/products/workflows/backend/services/ses_tenant_state.py index 99f9f5c3c3de..d713c4671204 100644 --- a/products/workflows/backend/services/ses_tenant_state.py +++ b/products/workflows/backend/services/ses_tenant_state.py @@ -1,21 +1,26 @@ from collections.abc import Callable from typing import Any +from django.conf import settings from django.db import transaction from django.utils import timezone import structlog from posthog.models.team import Team -from posthog.plugins.plugin_server_api import reload_team_workflows_config_on_workers +from posthog.plugins.plugin_server_api import ( + reload_sandbox_sender_state_on_workers, + reload_team_workflows_config_on_workers, +) from posthog.tasks.email import ( send_email_sending_reputation_finding, send_email_sending_suspended, send_email_sending_unsuspended, ) +from products.workflows.backend.models.sandbox_sender_tenant_state import SandboxSenderTenantState from products.workflows.backend.models.team_workflows_config import TeamWorkflowsConfig -from products.workflows.backend.providers.ses import SESProvider +from products.workflows.backend.providers.ses import SESProvider, TenantSendingState logger = structlog.get_logger(__name__) @@ -58,6 +63,39 @@ def _announce_state_change(team_id: int) -> None: logger.exception("Failed to announce SES tenant state change to workers", team_id=team_id) +def _apply_sandbox_tenant_state(state: SandboxSenderTenantState, tenant: TenantSendingState) -> None: + previous_status = state.sending_status + state.synced_at = timezone.now() + if previous_status == tenant.sending_status: + state.save(update_fields=["synced_at"]) + logger.info("Sandbox SES tenant state unchanged", tenant_name=state.tenant_name, status=previous_status) + return + + state.sending_status = tenant.sending_status + state.save(update_fields=["sending_status", "synced_at"]) + logger.info( + "Sandbox SES tenant state changed", + tenant_name=state.tenant_name, + from_status=previous_status, + to_status=tenant.sending_status, + ) + tenant_name = state.tenant_name + transaction.on_commit(lambda: reload_sandbox_sender_state_on_workers(tenant_name), robust=True) + + +def sync_sandbox_tenant_state(provider: SESProvider | None = None) -> None: + # Serialize SES reads so a stale sweep cannot overwrite a newer pause. + tenant_name = settings.SES_SANDBOX_TENANT_NAME + if not tenant_name: + return + SandboxSenderTenantState.objects.get_or_create(tenant_name=tenant_name) + with transaction.atomic(): + state = SandboxSenderTenantState.objects.select_for_update().get(tenant_name=tenant_name) + tenant = (provider or SESProvider()).get_tenant_sending_state(tenant_name) + if tenant is not None: + _apply_sandbox_tenant_state(state, tenant) + + def sync_ses_tenant_state(team_id: int, provider: SESProvider | None = None, *, verify_team: bool = True) -> None: """ Fetch the authoritative AWS SES tenant state for a team and apply it. Called from the diff --git a/products/workflows/backend/tasks/ses_tenant_state.py b/products/workflows/backend/tasks/ses_tenant_state.py index 55b41d59978f..d8c01d8c355f 100644 --- a/products/workflows/backend/tasks/ses_tenant_state.py +++ b/products/workflows/backend/tasks/ses_tenant_state.py @@ -6,7 +6,7 @@ from posthog.tasks.utils import CeleryQueue from products.workflows.backend.providers.ses import SESProvider -from products.workflows.backend.services.ses_tenant_state import sync_ses_tenant_state +from products.workflows.backend.services.ses_tenant_state import sync_sandbox_tenant_state, sync_ses_tenant_state logger = get_logger(__name__) @@ -27,6 +27,19 @@ def sync_ses_tenant_state_task(team_id: int) -> None: sync_ses_tenant_state(team_id) +@shared_task( + ignore_result=True, + queue=CeleryQueue.DEFAULT.value, + autoretry_for=(Exception,), + retry_backoff=60, + retry_backoff_max=600, + max_retries=5, + retry_jitter=True, +) +def sync_sandbox_tenant_state_task() -> None: + sync_sandbox_tenant_state() + + @shared_task(ignore_result=True, queue=CeleryQueue.LONG_RUNNING.value) @skip_team_scope_audit def reconcile_ses_tenant_states() -> None: @@ -44,6 +57,10 @@ def reconcile_ses_tenant_states() -> None: # One provider for the whole sweep: a fresh SESProvider per team would rebuild its boto3 # clients (and re-resolve credentials) thousands of times. provider = SESProvider() + try: + sync_sandbox_tenant_state(provider=provider) + except Exception: + logger.exception("SES sandbox tenant reconciliation failed") synced = 0 failed = 0 for team_id in team_ids.iterator(chunk_size=500): diff --git a/products/workflows/backend/test/test_ses_tenant_events.py b/products/workflows/backend/test/test_ses_tenant_events.py index 8b4f7aa2d9e8..fb4fe7f6aa6a 100644 --- a/products/workflows/backend/test/test_ses_tenant_events.py +++ b/products/workflows/backend/test/test_ses_tenant_events.py @@ -23,6 +23,7 @@ from products.workflows.backend.facade.api import accept_ses_event TOPIC = "arn:aws:sns:us-east-1:123456789012:ses-tenant-events" +SANDBOX_TENANT = "sandbox-team-1" WEBHOOK_PATH = "/webhooks/workflows/ses-events" SUBSCRIBE_URL = "https://sns.us-east-1.amazonaws.com/?Action=ConfirmSubscription&Token=tok" _CERT_URL = "https://sns.us-east-1.amazonaws.com/SimpleNotificationService-01d088a6f77103d0fe307c0069e40ed6.pem" @@ -106,6 +107,9 @@ def setUp(self) -> None: sync = patch("products.workflows.backend.services.ses_tenant_events.sync_ses_tenant_state_task") self.sync_mock = sync.start() self.addCleanup(sync.stop) + sandbox_sync = patch("products.workflows.backend.services.ses_tenant_events.sync_sandbox_tenant_state_task") + self.sandbox_sync_mock = sandbox_sync.start() + self.addCleanup(sandbox_sync.stop) def test_enqueues_a_sync_for_the_tenant_named_in_the_event(self) -> None: accept_ses_event(_delivery(_notification(_eventbridge_event()))) @@ -119,6 +123,31 @@ def test_finds_the_tenant_in_resource_arns_when_detail_has_no_name(self) -> None self.sync_mock.delay.assert_called_once_with(7) + @parameterized.expand( + [ + ("sandbox_tenant_by_name", {"tenantName": SANDBOX_TENANT}, [], "sandbox"), + ( + "sandbox_tenant_by_arn", + {}, + [f"arn:aws:ses:us-east-1:123456789012:tenant/{SANDBOX_TENANT}/abc"], + "sandbox", + ), + ("team_tenant", {"tenantName": "team-42"}, [], "team"), + ] + ) + @override_settings(SES_SANDBOX_TENANT_NAME=SANDBOX_TENANT) + def test_routes_the_sandbox_tenant_to_its_own_sync( + self, _name: str, detail: dict[str, Any], resources: list[str], expected_sync: str + ) -> None: + accept_ses_event(_delivery(_notification(_eventbridge_event(detail=detail, resources=resources)))) + + if expected_sync == "sandbox": + self.sandbox_sync_mock.delay.assert_called_once_with() + self.sync_mock.delay.assert_not_called() + else: + self.sync_mock.delay.assert_called_once_with(42) + self.sandbox_sync_mock.delay.assert_not_called() + def test_ignores_events_from_other_sources(self) -> None: accept_ses_event(_delivery(_notification(_eventbridge_event(source="aws.health")))) diff --git a/products/workflows/backend/test/test_ses_tenant_state.py b/products/workflows/backend/test/test_ses_tenant_state.py index e00b17a1df33..214aa4f1f8fb 100644 --- a/products/workflows/backend/test/test_ses_tenant_state.py +++ b/products/workflows/backend/test/test_ses_tenant_state.py @@ -1,10 +1,18 @@ +from datetime import UTC, datetime + +import time_machine from posthog.test.base import BaseTest from unittest.mock import MagicMock, patch +from django.test import override_settings from django.utils import timezone +from botocore.exceptions import ClientError from parameterized import parameterized +from posthog.models.integration import Integration + +from products.workflows.backend.models.sandbox_sender_tenant_state import SandboxSenderTenantState from products.workflows.backend.models.team_workflows_config import TeamWorkflowsConfig from products.workflows.backend.services.ses_tenant_state import ( PROVIDER_PAUSE_REASON, @@ -12,6 +20,10 @@ apply_ses_tenant_state, sync_ses_tenant_state, ) +from products.workflows.backend.tasks.ses_tenant_state import ( + reconcile_ses_tenant_states, + sync_sandbox_tenant_state_task, +) class TestApplySesTenantState(BaseTest): @@ -183,3 +195,114 @@ def test_unknown_team_is_skipped_without_calling_ses(self) -> None: provider.get_tenant_reputation.assert_not_called() assert not TeamWorkflowsConfig.objects.filter(team_id=self.team.id + 99_999).exists() + + +SANDBOX_TENANT = "workflows-sandbox" +SANDBOX_TENANT_ARN = f"arn:aws:ses:us-east-1:123456789012:tenant/{SANDBOX_TENANT}/abc" + + +@override_settings(SES_SANDBOX_TENANT_NAME=SANDBOX_TENANT) +class TestSandboxTenantStateSync(BaseTest): + def setUp(self) -> None: + super().setUp() + boto3_client = patch("products.workflows.backend.providers.ses.boto3.client") + self.ses = boto3_client.start().return_value + self.addCleanup(boto3_client.stop) + publish = patch("posthog.plugins.plugin_server_api.publish_message") + self.publish = publish.start() + self.addCleanup(publish.stop) + self.ses.list_recommendations.return_value = {"Recommendations": []} + + def _aws_tenant(self, sending_status: str, impact: str) -> None: + self.ses.get_tenant.return_value = {"Tenant": {"TenantName": SANDBOX_TENANT, "TenantArn": SANDBOX_TENANT_ARN}} + self.ses.get_reputation_entity.return_value = { + "ReputationEntity": {"ReputationImpact": impact, "SendingStatusAggregate": sending_status} + } + + def _sync(self) -> None: + with self.captureOnCommitCallbacks(execute=True): + sync_sandbox_tenant_state_task() + + def test_a_changed_state_is_stored_and_announced_after_commit(self) -> None: + self._aws_tenant("ENABLED", "NONE") + self._sync() + self.publish.reset_mock() + self._aws_tenant("DISABLED", "HIGH") + + with self.captureOnCommitCallbacks(execute=True): + sync_sandbox_tenant_state_task() + self.publish.assert_not_called() + + state = SandboxSenderTenantState.objects.get(tenant_name=SANDBOX_TENANT) + assert state.sending_status == "DISABLED" + self.ses.get_tenant.assert_called_with(TenantName=SANDBOX_TENANT) + self.publish.assert_called_once_with("reload-sandbox-sender-state", {"tenantName": SANDBOX_TENANT}) + + @parameterized.expand([("same_impact", "HIGH"), ("only_the_impact_changed", "LOW")]) + def test_an_unchanged_sending_status_refreshes_synced_at_without_announcing(self, _name: str, impact: str) -> None: + self._aws_tenant("DISABLED", "HIGH") + with time_machine.travel("2026-10-05T10:00:00Z", tick=False): + self._sync() + self.publish.reset_mock() + self._aws_tenant("DISABLED", impact) + + with time_machine.travel("2026-10-05T11:00:00Z", tick=False): + self._sync() + + state = SandboxSenderTenantState.objects.get(tenant_name=SANDBOX_TENANT) + assert state.sending_status == "DISABLED" + assert state.synced_at == datetime(2026, 10, 5, 11, tzinfo=UTC) + self.publish.assert_not_called() + + def test_a_pause_is_stored_even_when_findings_cannot_be_listed(self) -> None: + self._aws_tenant("DISABLED", "HIGH") + self.ses.list_recommendations.side_effect = ClientError( + {"Error": {"Code": "TooManyRequestsException"}}, "ListRecommendations" + ) + + self._sync() + + assert SandboxSenderTenantState.objects.get(tenant_name=SANDBOX_TENANT).sending_status == "DISABLED" + + def test_an_unknown_sandbox_tenant_is_never_paused(self) -> None: + self.ses.get_tenant.side_effect = ClientError({"Error": {"Code": "NotFoundException"}}, "GetTenant") + + self._sync() + + state = SandboxSenderTenantState.objects.get(tenant_name=SANDBOX_TENANT) + assert (state.sending_status, state.synced_at) == ("", None) + self.publish.assert_not_called() + + def test_the_daily_sweep_syncs_the_sandbox_tenant_once_beside_team_tenants(self) -> None: + Integration.objects.create(team=self.team, kind="email", config={"provider": "ses"}) + self._aws_tenant("ENABLED", "LOW") + + with self.captureOnCommitCallbacks(execute=True): + reconcile_ses_tenant_states() + + tenant_names = [call.kwargs["TenantName"] for call in self.ses.get_tenant.call_args_list] + assert sorted(tenant_names) == sorted([SANDBOX_TENANT, f"team-{self.team.id}"]) + assert SandboxSenderTenantState.objects.get(tenant_name=SANDBOX_TENANT).sending_status == "ENABLED" + + def test_a_failed_sandbox_sync_does_not_stop_the_team_sweep(self) -> None: + Integration.objects.create(team=self.team, kind="email", config={"provider": "ses"}) + + def get_tenant(TenantName: str) -> dict: + if TenantName == SANDBOX_TENANT: + raise ClientError({"Error": {"Code": "TooManyRequestsException"}}, "GetTenant") + return {"Tenant": {"TenantName": TenantName, "SendingStatus": "ENABLED"}} + + self.ses.get_tenant.side_effect = get_tenant + + with self.captureOnCommitCallbacks(execute=True): + reconcile_ses_tenant_states() + + assert TeamWorkflowsConfig.objects.get(team=self.team).ses_tenant_sending_status == "ENABLED" + + @override_settings(SES_SANDBOX_TENANT_NAME="") + def test_nothing_is_synced_while_no_sandbox_tenant_is_configured(self) -> None: + sync_sandbox_tenant_state_task() + reconcile_ses_tenant_states() + + self.ses.get_tenant.assert_not_called() + assert not SandboxSenderTenantState.objects.exists() From 0e5e737c2d27ec36bdc8ffeb7d90553bd1acab9b Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:07:08 +0000 Subject: [PATCH 11/17] feat(workflows): skip sandbox sends while the shared ses tenant is paused The email worker reads the sandbox SES tenant state that Django mirrors, skips every sandbox send while the tenant is paused or the state cannot be read, and reloads the state on the reload-sandbox-sender-state message. After the daily allowance claim it checks the pause again and gives the claim back when the pause blocks delivery. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../internal/workflows-email-sending-tiers.md | 4 +- nodejs/src/cdp/cdp-services.ts | 4 +- .../sandbox-sender-state.service.test.ts | 24 ++ .../managers/sandbox-sender-state.service.ts | 52 +++++ .../services/messaging/email.service.test.ts | 211 +++++++++++++++++- .../cdp/services/messaging/email.service.ts | 29 ++- .../messaging/sandbox-email-sender.test.ts | 7 +- .../messaging/sandbox-email-sender.ts | 14 +- .../rate-limiter/rate-limiter.service.test.ts | 3 +- 9 files changed, 339 insertions(+), 9 deletions(-) create mode 100644 nodejs/src/cdp/services/managers/sandbox-sender-state.service.test.ts create mode 100644 nodejs/src/cdp/services/managers/sandbox-sender-state.service.ts diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 67ce8d73d38d..fa96812f88c0 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -87,8 +87,10 @@ A reached allowance skips the send instead of delaying it. A limiter error, or a The worker checks members after the claim and gives the claim back when that check skips the send. An SES throttle response also gives the claim back before the delivery retry. Returns are deduplicated, and Valkey ignores a return that arrives more than five minutes after it was issued. Test sends run inline in the CDP API, so a sandbox-enabled CDP API opens the SES Valkey pool (`SES_RATE_LIMITER_VALKEY_*`) for the allowances. Without it, every sandbox test send skips. +Every sandbox send, test sends included, skips while the shared sandbox SES tenant is paused (`DISABLED`), or when the pause state cannot be read. A tenant Django has never synced counts as open, and SES enforces its own state. +The worker checks the pause again after the daily allowance claim and gives the claim back when the pause blocks delivery. Django mirrors the tenant named by `SES_SANDBOX_TENANT_NAME` into `SandboxSenderTenantState` from the SES tenant-state webhook and the daily reconciliation, without emailing project admins. Set the same `SES_SANDBOX_TENANT_NAME` on Django web, Celery, and the worker. -A committed state change reaches the workers through the `reload-sandbox-sender-state` message. +A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Own-sender sends do not read this state. Each delivered send emits a `workflows sandbox email sent` event. Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. Keep the switch off in production until that metric is deployed. diff --git a/nodejs/src/cdp/cdp-services.ts b/nodejs/src/cdp/cdp-services.ts index 7950d0666c12..c9e73bfa3f7a 100644 --- a/nodejs/src/cdp/cdp-services.ts +++ b/nodejs/src/cdp/cdp-services.ts @@ -34,6 +34,7 @@ import { IntegrationManagerService } from './services/managers/integration-manag import { OrganizationMembersService } from './services/managers/organization-members.service' import { PersonsManagerService } from './services/managers/persons-manager.service' import { RecipientsManagerService } from './services/managers/recipients-manager.service' +import { SandboxSenderStateService } from './services/managers/sandbox-sender-state.service' import { TeamWorkflowsConfigService } from './services/managers/team-workflows-config.service' import { EmailSuppressionService } from './services/messaging/email-suppression.service' import { EmailValidationService } from './services/messaging/email-validation.service' @@ -498,7 +499,8 @@ export function createCdpCoreServices( }, deps.teamManager, redis, - sandboxEmailRateLimiter + sandboxEmailRateLimiter, + new SandboxSenderStateService(deps.postgres, deps.pubSub) ), new OrganizationMembersService(deps.postgres) ) diff --git a/nodejs/src/cdp/services/managers/sandbox-sender-state.service.test.ts b/nodejs/src/cdp/services/managers/sandbox-sender-state.service.test.ts new file mode 100644 index 000000000000..6bdde7cfa59e --- /dev/null +++ b/nodejs/src/cdp/services/managers/sandbox-sender-state.service.test.ts @@ -0,0 +1,24 @@ +import { DependencyUnavailableError } from '~/common/utils/db/error' +import { PostgresRouter } from '~/common/utils/db/postgres' +import { PubSub } from '~/common/utils/pubsub' + +import { SandboxSenderStateService } from './sandbox-sender-state.service' + +describe('SandboxSenderStateService', () => { + it('rides out a transient database error instead of failing the sandbox send', async () => { + const query = jest + .fn() + .mockRejectedValueOnce( + new DependencyUnavailableError('server closed the connection unexpectedly', 'Postgres', new Error()) + ) + .mockResolvedValueOnce({ rows: [{ tenant_name: 'sandbox-tenant', sending_status: 'DISABLED' }] }) + const service = new SandboxSenderStateService( + { query } as unknown as PostgresRouter, + { + on: jest.fn(), + } as unknown as PubSub + ) + + await expect(service.isPaused('sandbox-tenant')).resolves.toBe(true) + }) +}) diff --git a/nodejs/src/cdp/services/managers/sandbox-sender-state.service.ts b/nodejs/src/cdp/services/managers/sandbox-sender-state.service.ts new file mode 100644 index 000000000000..3a82caf91e31 --- /dev/null +++ b/nodejs/src/cdp/services/managers/sandbox-sender-state.service.ts @@ -0,0 +1,52 @@ +import { PostgresRouter, PostgresUse } from '~/common/utils/db/postgres' +import { DEFAULT_LOADER_RETRY, LazyLoader } from '~/common/utils/lazy-loader' +import { PubSub } from '~/common/utils/pubsub' + +type SandboxSenderState = { sendingStatus: string | undefined; generation: number } + +export class SandboxSenderStateService { + private lazyLoader: LazyLoader<SandboxSenderState> + private generation = 0 + + constructor( + private postgres: PostgresRouter, + pubSub: PubSub + ) { + this.lazyLoader = new LazyLoader({ + name: 'sandbox_sender_state', + refreshAgeMs: 30_000, + refreshJitterMs: 5_000, + loader: async (tenantNames) => await this.fetchStates(tenantNames), + loaderRetry: DEFAULT_LOADER_RETRY, + }) + pubSub.on<{ tenantName: string }>('reload-sandbox-sender-state', ({ tenantName }): void => { + this.generation++ + this.lazyLoader.markForRefresh(tenantName) + }) + } + + public async isPaused(tenantName: string): Promise<boolean> { + for (let attempt = 0; attempt < 2; attempt++) { + const state = await this.lazyLoader.get(tenantName) + if (state?.generation === this.generation) { + // No row means never synced; SES stays the final word. + return state.sendingStatus === 'DISABLED' + } + this.lazyLoader.markForRefresh(tenantName) + } + throw new Error('Sandbox sender state changed during lookup') + } + + private async fetchStates(tenantNames: string[]): Promise<Record<string, SandboxSenderState>> { + const generation = this.generation + const { rows } = await this.postgres.query<{ tenant_name: string; sending_status: string }>( + // The reload message fires right after the commit, so a lagging replica would cache the old state again. + PostgresUse.COMMON_WRITE, + 'SELECT tenant_name, sending_status FROM workflows_sandboxsendertenantstate WHERE tenant_name = ANY($1)', + [tenantNames], + 'fetch-sandbox-sender-state' + ) + const statuses = new Map(rows.map((row) => [row.tenant_name, row.sending_status])) + return Object.fromEntries(tenantNames.map((name) => [name, { sendingStatus: statuses.get(name), generation }])) + } +} diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index c787988f003c..92870928aa44 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -31,6 +31,7 @@ import { INVALID_TEAM_ID_KEYS } from '~/tests/helpers/team' import { Hub, Team } from '../../../types' import { OrganizationMembersService } from '../managers/organization-members.service' import { RecipientsManagerService } from '../managers/recipients-manager.service' +import { SandboxSenderStateService } from '../managers/sandbox-sender-state.service' import { TeamWorkflowsConfigService } from '../managers/team-workflows-config.service' import { RateLimiterService } from '../rate-limiter/rate-limiter.service' import { selectEmailSenderIntegrationId } from './email-sender-selection' @@ -279,12 +280,14 @@ describe('EmailService', () => { teamEmailRateLimiter = null, messageAssetsService, dailyCapLimiter = new RateLimiterService(sandboxRedis, { name: 'sandbox-email' }), + senderState = new SandboxSenderStateService(hub.postgres, hub.pubSub), }: { membersPostgres?: PostgresRouter senderSettings?: Partial<SandboxEmailSenderConfig> teamEmailRateLimiter?: RateLimiterService | null messageAssetsService?: MessageAssetsService dailyCapLimiter?: RateLimiterService | null + senderState?: SandboxSenderStateService } = {} ): EmailService => { const workflowsConfig = new TeamWorkflowsConfigService(hub.postgres, hub.pubSub) @@ -323,7 +326,8 @@ describe('EmailService', () => { }, hub.teamManager, sandboxRedis, - dailyCapLimiter + dailyCapLimiter, + senderState ), new OrganizationMembersService(membersPostgres) ) @@ -756,6 +760,211 @@ describe('EmailService', () => { expectBlockedEvent('switch_off', false, 0) }) + describe('pause gate', () => { + const pausedMessage = + 'Skipping send: the sandbox sender is paused right now. Verify your own domain to keep sending.' + const tenantName = (): string => `sandbox-tenant-${team.id}` + const setTenantStatus = async (status: string): Promise<void> => { + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + `INSERT INTO workflows_sandboxsendertenantstate (tenant_name, sending_status, synced_at) + VALUES ($1, $2, now()) + ON CONFLICT (tenant_name) DO UPDATE SET sending_status = $2`, + [tenantName(), status], + 'test:set-sandbox-tenant-status' + ) + } + + afterEach(async () => { + await hub.postgres.query( + PostgresUse.COMMON_WRITE, + 'DELETE FROM workflows_sandboxsendertenantstate WHERE tenant_name = $1', + [tenantName()], + 'test:delete-sandbox-tenant-status' + ) + }) + + it.each([false, true])('skips while the sandbox tenant is paused (isTest=%s)', async (isTest) => { + await setTenantStatus('DISABLED') + service = createSandboxService(true, { senderSettings: { tenantName: tenantName() } }) + + const result = await service.executeSendEmail(invocation, isTest) + + expect(sendEmailSpy).not.toHaveBeenCalled() + expect(result).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(result.error).toBeUndefined() + expect(result.invocation.state.vmState?.stack).toEqual([{ success: false }]) + expect(result.logs).toEqual( + expect.arrayContaining([expect.objectContaining({ level: 'info', message: pausedMessage })]) + ) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + { reason: 'paused', is_test: isTest, blocked_recipient_count: 0 } + ) + }) + + it.each(['ENABLED', 'REINSTATED'])('sends while the sandbox tenant is %s', async (status) => { + await setTenantStatus(status) + service = createSandboxService(true, { senderSettings: { tenantName: tenantName() } }) + + const result = await service.executeSendEmail(invocation) + + expect(result.error).toBeUndefined() + expect(result.skipped).toBeFalsy() + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + }) + + it('skips when the pause state cannot be read', async () => { + const brokenPostgres = new PostgresRouter(hub) + await brokenPostgres.end() + service = createSandboxService(true, { + senderSettings: { tenantName: tenantName() }, + senderState: new SandboxSenderStateService(brokenPostgres, hub.pubSub), + }) + + const result = await service.executeSendEmail(invocation) + + expect(sendEmailSpy).not.toHaveBeenCalled() + expect(result).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(result.invocation.state.vmState?.stack).toEqual([{ success: false }]) + expect(result.logs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + level: 'info', + message: + 'Skipping send: could not check whether the sandbox sender is paused. Try again later, or verify your own domain to keep sending.', + }), + ]) + ) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email blocked', + { reason: 'check_failed', is_test: false, blocked_recipient_count: 0 } + ) + }) + + it('leaves own-sender sends alone while the sandbox tenant is paused', async () => { + await setTenantStatus('DISABLED') + service = createSandboxService(true, { senderSettings: { tenantName: tenantName() } }) + invocation.queueParameters = createEmailParams({ from: { integrationId: 1 } }) + + const result = await service.executeSendEmail(invocation) + + expect(result.error).toBeUndefined() + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + }) + + it('rechecks a pause after a delayed cap claim and returns the grant', async () => { + await setTenantStatus('ENABLED') + const now = Date.now() + const clock = jest.spyOn(Date, 'now').mockReturnValue(now) + const dailyCapLimiter = new RateLimiterService(sandboxRedis, { name: 'sandbox-email' }) + service = createSandboxService(true, { + dailyCapLimiter, + senderSettings: { tenantName: tenantName(), dailyTeamCap: '1', dailyRecipientCap: '1' }, + }) + const claim = dailyCapLimiter.claimAllOrNothing.bind(dailyCapLimiter) + const delayedClaim = jest + .spyOn(dailyCapLimiter, 'claimAllOrNothing') + .mockImplementationOnce(async (buckets) => { + const grant = await claim(buckets) + await setTenantStatus('DISABLED') + clock.mockReturnValue(now + 40_000) + return grant + }) + try { + const blocked = await service.executeSendEmail(invocation) + expect(blocked).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(blocked.error).toBeUndefined() + expect(sendEmailSpy).not.toHaveBeenCalled() + expect(blocked.logs).toContainEqual(expect.objectContaining({ message: pausedMessage })) + + await setTenantStatus('ENABLED') + clock.mockReturnValue(now + 80_000) + invocation.state.vmState = { stack: [] } as any + const resumed = await service.executeSendEmail(invocation) + expect(resumed.error).toBeUndefined() + expect(resumed.skipped).toBeFalsy() + expect(sendEmailSpy).toHaveBeenCalledTimes(1) + } finally { + delayedClaim.mockRestore() + clock.mockRestore() + } + }) + + it('skips when a pause invalidates a pending state read', async () => { + await setTenantStatus('ENABLED') + service = createSandboxService(true, { senderSettings: { tenantName: tenantName() } }) + let snapshotLoaded!: () => void + let releaseRead!: () => void + const snapshot = new Promise<void>((resolve) => { + snapshotLoaded = resolve + }) + const heldRead = new Promise<void>((resolve) => { + releaseRead = resolve + }) + let acknowledgeReload!: () => void + const reload = new Promise<void>((resolve) => { + acknowledgeReload = resolve + }) + hub.pubSub.on<{ tenantName: string }>('reload-sandbox-sender-state', ({ tenantName: name }) => { + if (name === tenantName()) { + acknowledgeReload() + } + }) + const query = hub.postgres.query.bind(hub.postgres) + let held = false + const delayedRead = jest.spyOn(hub.postgres, 'query').mockImplementation(async (...args) => { + const result = await query(...args) + if (args[3] === 'fetch-sandbox-sender-state' && !held) { + held = true + snapshotLoaded() + await heldRead + } + return result + }) + try { + const pendingSend = service.executeSendEmail(invocation) + await snapshot + await setTenantStatus('DISABLED') + await hub.pubSub.publish( + 'reload-sandbox-sender-state', + JSON.stringify({ tenantName: tenantName() }) + ) + await reload + releaseRead() + const result = await pendingSend + expect(result).toMatchObject({ finished: true, skipped: true, metrics: [] }) + expect(result.error).toBeUndefined() + expect(result.logs).toContainEqual(expect.objectContaining({ message: pausedMessage })) + expect(sendEmailSpy).not.toHaveBeenCalled() + } finally { + releaseRead() + delayedRead.mockRestore() + } + }) + + it('picks up a pause announced while the state is cached', async () => { + await setTenantStatus('ENABLED') + service = createSandboxService(true, { senderSettings: { tenantName: tenantName() } }) + expect((await service.executeSendEmail(invocation)).skipped).toBeFalsy() + + await setTenantStatus('DISABLED') + + await waitForExpect(async () => { + await hub.pubSub.publish( + 'reload-sandbox-sender-state', + JSON.stringify({ tenantName: tenantName() }) + ) + const result = await service.executeSendEmail(invocation) + expect(result.logs).toEqual( + expect.arrayContaining([expect.objectContaining({ message: pausedMessage })]) + ) + }, 3000) + }) + }) + describe('daily caps', () => { const TEAM_CAP_REACHED = "Skipping send: this email would go over the sandbox sender's daily limit for this project. Verify your own domain to send more." diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index 1c1cebaf3b5c..be25934f7f80 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -410,7 +410,7 @@ function sandboxRecipientsOf(params: CyclotronInvocationQueueParametersEmailType return [params.to.email.trim(), ...sandboxAddressList(params.cc), ...sandboxAddressList(params.bcc)] } -type SandboxRecipientBlockReason = Exclude<SandboxEmailBlockReason, 'switch_off'> +type SandboxRecipientBlockReason = Exclude<SandboxEmailBlockReason, 'switch_off' | 'paused'> type SesRoute = { tenantName: string @@ -448,6 +448,12 @@ const SANDBOX_RECIPIENT_BLOCK_COPY: Record< }, } +const SANDBOX_PAUSE_SKIP_MESSAGES = { + paused: 'Skipping send: the sandbox sender is paused right now. Verify your own domain to keep sending.', + check_failed: + 'Skipping send: could not check whether the sandbox sender is paused. Try again later, or verify your own domain to keep sending.', +} as const + const SANDBOX_CAP_SKIP_MESSAGES = { project_cap_reached: "Skipping send: this email would go over the sandbox sender's daily limit for this project. Verify your own domain to send more.", @@ -615,6 +621,9 @@ export class EmailService { await this.sandboxSender?.captureBlocked(invocation.teamId, isTest, 'switch_off', 0) return result } + if (isSandbox && !(await this.passesSandboxPauseGate(result, isTest))) { + return result + } const from = this.resolveFromSender(integration, params.from, addLog) @@ -805,7 +814,10 @@ export class EmailService { if (!capGrant) { return result } - if (!(await this.checkSandboxRecipients(result, deliveryRecipients, isTest))) { + if ( + !(await this.passesSandboxPauseGate(result, isTest)) || + !(await this.checkSandboxRecipients(result, deliveryRecipients, isTest)) + ) { await this.sandboxSender!.returnDailyCaps(capGrant) return result } @@ -1072,6 +1084,19 @@ export class EmailService { return false } + private async passesSandboxPauseGate( + result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, + isTest: boolean + ): Promise<boolean> { + const gate = await this.sandboxSender!.pauseGate() + if (gate === 'open') { + return true + } + this.skipSandboxSend(result, SANDBOX_PAUSE_SKIP_MESSAGES[gate]) + await this.sandboxSender!.captureBlocked(result.invocation.teamId, isTest, gate, 0) + return false + } + private async blockSandboxRecipients( result: CyclotronJobInvocationResult<CyclotronJobInvocationHogFunction>, blockedRecipients: string[], diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts index 08ace91b93af..08a78499abd0 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.test.ts @@ -11,6 +11,7 @@ const teamManager = { getTeam: jest.fn().mockResolvedValue({ organization_id: '00000000-0000-0000-0000-000000000001' }), } as unknown as TeamManager const blockedEventRedis = { useClient: jest.fn() } +const senderState = { isPaused: jest.fn().mockResolvedValue(false) } const sender = new SandboxEmailSender( { enabled: true, @@ -22,7 +23,8 @@ const sender = new SandboxEmailSender( }, teamManager, blockedEventRedis, - null + null, + senderState ) const params = (html: string, text?: string, preheader?: string): CyclotronInvocationQueueParametersEmailType => ({ type: 'email', @@ -72,7 +74,8 @@ describe('sandbox email content', () => { }, teamManager, blockedEventRedis, - null + null, + senderState ) const result = await paddedSender.withIdentificationFooter( params('<p>Hello</p>'), diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index 4df643e09d7c..0bb188858895 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -7,6 +7,7 @@ import { logger } from '~/common/utils/logger' import { captureTeamEvent } from '~/common/utils/posthog' import { TeamManager } from '~/common/utils/team-manager' +import { SandboxSenderStateService } from '../managers/sandbox-sender-state.service' import { ClaimRequest, IssuedClaim, RateLimiterService } from '../rate-limiter/rate-limiter.service' import { maybeAddPreheaderToEmail } from './helpers/preheader' @@ -40,6 +41,7 @@ export type SandboxEmailBlockReason = | 'recipient_unverified' | 'check_failed' | 'cap_reached' + | 'paused' export type SandboxDailyCapGrant = { type: 'granted'; claim: IssuedClaim; buckets: ClaimRequest[] } @@ -168,7 +170,8 @@ export class SandboxEmailSender { public readonly config: SandboxEmailSenderConfig, private teamManager: TeamManager, private blockedEventRedis: Pick<RedisV2, 'useClient'>, - private dailyCapLimiter: RateLimiterService | null + private dailyCapLimiter: RateLimiterService | null, + private senderState: Pick<SandboxSenderStateService, 'isPaused'> ) { this.config = { ...config, @@ -185,6 +188,15 @@ export class SandboxEmailSender { this.canSend = config.enabled && !emptySettings.length } + public async pauseGate(): Promise<'open' | 'paused' | 'check_failed'> { + try { + return (await this.senderState.isPaused(this.config.tenantName)) ? 'paused' : 'open' + } catch (error) { + logger.warn('Could not check the sandbox sender pause state', { error }) + return 'check_failed' + } + } + public async claimDailyCaps(teamId: number, recipients: string[]): Promise<SandboxDailyCapClaim> { const dailyTeamCap = parseDailyCap(this.config.dailyTeamCap) const dailyRecipientCap = parseDailyCap(this.config.dailyRecipientCap) diff --git a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts index 6ef291ebd57b..15095c35f6ab 100644 --- a/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts +++ b/nodejs/src/cdp/services/rate-limiter/rate-limiter.service.test.ts @@ -434,7 +434,8 @@ describe('RateLimiterService', () => { }, teamManager, redis, - limiter + limiter, + { isPaused: () => Promise.resolve(false) } ) try { From 44c305718ba9ee8dc2499124966eaf5659afbad6 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:07:44 +0000 Subject: [PATCH 12/17] feat(workflows): subtract sandbox sends from the sending tier The tier sweep subtracts email_sandbox_sent from email_sent per day, and the sending allowance subtracts it per workflow, both floored at zero. No code records email_sandbox_sent yet, so the tier does not change until the worker metric lands in its own PR. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../internal/workflows-email-sending-tiers.md | 2 + .../backend/services/email_health.py | 5 +- .../backend/services/email_sending_tier.py | 12 ++- .../backend/test/test_email_sending_tier.py | 97 ++++++++++++++++++- .../tests/api/test_email_reputation.py | 22 +++++ 5 files changed, 132 insertions(+), 6 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index fa96812f88c0..6411b2e9ce2d 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -39,6 +39,7 @@ None of them are wired there yet; the charts change is part of turning the rollo In order: staff suspension or an AWS-paused tenant drops to tier 0; a dirty 7-day window, a workflow auto-pause, or a HIGH tenant reputation impact demotes one tier (at most once per cooldown, anchored on the last rate demotion); a tier above 0 with zero sends for the decay period drops one step per period; promotion needs the dwell served, real use of the tier (at least the use ratio of the daily cap on the minimum number of separate days, counted only after the tier anchor), a clean 30-day window, and a clean tenant reputation. Rates only count on a meaningful denominator; below the complaint floor, the absolute complaint backstop still applies, including in windows with no sends. +Sandbox sender sends never count as sends: the sweep subtracts `email_sandbox_sent` from `email_sent` per day, floored at zero. Their bounces and complaints still count. Pinned teams never move automatically. While the tier is enforced, the sweep notifies teams it moved: a rate demotion sends an in-app notification plus an email to project admins, and an earned promotion sends an in-app notification only. @@ -93,4 +94,5 @@ Django mirrors the tenant named by `SES_SANDBOX_TENANT_NAME` into `SandboxSender A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Own-sender sends do not read this state. Each delivered send emits a `workflows sandbox email sent` event. Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. +The sending tier subtracts `email_sandbox_sent` from `email_sent` per day, and the team's sending allowance per workflow, both floored at zero. Sandbox bounces and complaints still count. Keep the switch off in production until that metric is deployed. diff --git a/products/workflows/backend/services/email_health.py b/products/workflows/backend/services/email_health.py index 5be6acff5ff5..ab19f2293176 100644 --- a/products/workflows/backend/services/email_health.py +++ b/products/workflows/backend/services/email_health.py @@ -22,6 +22,7 @@ from products.workflows.backend.models.hog_flow.hog_flow import HogFlow from products.workflows.backend.providers.ses import SESProvider from products.workflows.backend.services.email_sending_attribution import fold_email_totals_by_flow +from products.workflows.backend.services.email_sending_tier import SANDBOX_SENT_METRIC, SENT_METRIC, own_sends from products.workflows.backend.utils.email_sending_tiers import max_email_sending_tier, resolve_team_email_sending_tier logger = structlog.get_logger(__name__) @@ -89,9 +90,9 @@ def team_email_sending_allowance(team_id: int) -> EmailSendingAllowance: def _team_email_sends_since(team_id: int, after: datetime) -> int: totals = fetch_app_metric_totals_by_team_and_source( - app_source="hog_flow", name=["email_sent"], after=after, team_ids=[team_id] + app_source="hog_flow", name=[SENT_METRIC, SANDBOX_SENT_METRIC], after=after, team_ids=[team_id] ) - return sum(counts.get("email_sent", 0) for counts in totals.get(team_id, {}).values()) + return sum(own_sends(counts) for counts in totals.get(team_id, {}).values()) AWS_TENANT_REPUTATION_CACHE_SECONDS = 5 * 60 diff --git a/products/workflows/backend/services/email_sending_tier.py b/products/workflows/backend/services/email_sending_tier.py index 4f3b83feae6b..ee8a3126eac4 100644 --- a/products/workflows/backend/services/email_sending_tier.py +++ b/products/workflows/backend/services/email_sending_tier.py @@ -25,6 +25,7 @@ APP_SOURCE = "hog_flow" SENT_METRIC = "email_sent" +SANDBOX_SENT_METRIC = "email_sandbox_sent" HARD_BOUNCE_METRIC = "email_bounced_hard" # SES complaint events are recorded under this name (see the SES webhook handler in the email # worker), so a complaint rate reads `email_blocked`, not a metric called "complaint". @@ -330,7 +331,7 @@ def _fetch_daily_metrics( workload: Workload = Workload.DEFAULT, ) -> dict[int, dict[str, dict[str, int]]]: auto_pause_metrics = [name for name in settings.WORKFLOWS_EMAIL_TIER_AUTO_PAUSE_METRIC_NAMES if name] - metric_names = [SENT_METRIC, HARD_BOUNCE_METRIC, COMPLAINT_METRIC, *auto_pause_metrics] + metric_names = [SENT_METRIC, SANDBOX_SENT_METRIC, HARD_BOUNCE_METRIC, COMPLAINT_METRIC, *auto_pause_metrics] # Tag the query so it is attributable in query-cost analysis and does not trip the untagged-query # guard in local dev. Celery adds only task identity, not a product or feature. This derives tier # state in the background, so it is enrichment, not a customer-facing query. One context here @@ -346,17 +347,22 @@ def _fetch_daily_metrics( ) +def own_sends(counts: dict[str, int]) -> int: + return max(0, counts.get(SENT_METRIC, 0) - counts.get(SANDBOX_SENT_METRIC, 0)) + + def _history_from_daily(team_id: int, days: dict[str, dict[str, int]]) -> TeamSendingHistory: # The tier decision only reads team totals, so metrics recorded under a batch job id and under # its parent workflow id sum the same either way and need no per-source resolution. auto_pause_metrics = [name for name in settings.WORKFLOWS_EMAIL_TIER_AUTO_PAUSE_METRIC_NAMES if name] + daily_sends = {day: sent for day, counts in days.items() if (sent := own_sends(counts)) > 0} return TeamSendingHistory( team_id=team_id, - sent=sum(counts.get(SENT_METRIC, 0) for counts in days.values()), + sent=sum(daily_sends.values()), hard_bounced=sum(counts.get(HARD_BOUNCE_METRIC, 0) for counts in days.values()), complained=sum(counts.get(COMPLAINT_METRIC, 0) for counts in days.values()), auto_paused=any(counts.get(metric, 0) > 0 for counts in days.values() for metric in auto_pause_metrics), - daily_sends={day: counts.get(SENT_METRIC, 0) for day, counts in days.items() if counts.get(SENT_METRIC, 0)}, + daily_sends=daily_sends, ) diff --git a/products/workflows/backend/test/test_email_sending_tier.py b/products/workflows/backend/test/test_email_sending_tier.py index 6509c8d0b0d4..dc564e1824d1 100644 --- a/products/workflows/backend/test/test_email_sending_tier.py +++ b/products/workflows/backend/test/test_email_sending_tier.py @@ -1,6 +1,7 @@ from datetime import timedelta -from posthog.test.base import BaseTest +import time_machine +from posthog.test.base import BaseTest, ClickhouseTestMixin from unittest.mock import patch from django.test import SimpleTestCase, override_settings @@ -8,6 +9,8 @@ from parameterized import parameterized +from posthog.test.fixtures import create_app_metric2 + from products.workflows.backend.management.commands.backfill_workflows_email_sending_tiers import ( Command as BackfillCommand, ) @@ -18,8 +21,10 @@ TeamSendingHistory, TierDecision, apply_tier_decision, + build_sending_histories, decide_tier, highest_qualifying_tier, + recompute_email_sending_tier_for_team, recompute_email_sending_tiers, ) from products.workflows.backend.utils.email_sending_tiers import get_email_sending_tier_limits @@ -446,6 +451,96 @@ def test_pinned_team_never_moves(self, _name: str, tier: int) -> None: assert TeamWorkflowsConfig.objects.get(team=self.team).email_sending_tier == tier +@time_machine.travel("2026-10-05T12:00:00Z", tick=False) +@override_settings(**TIER_SETTINGS) +class TestSandboxSendsInTierHistory(ClickhouseTestMixin, BaseTest): + def _record_metrics(self, *, days_ago: int, **counts: int) -> None: + for metric_name, count in counts.items(): + if count: + create_app_metric2( + team_id=self.team.pk, + app_source="hog_flow", + metric_kind="email", + metric_name=metric_name, + count=count, + timestamp=timezone.now() - timedelta(days=days_ago), + ) + + def _record_sends(self, *, days_ago: int, sent: int, sandbox_sent: int) -> None: + self._record_metrics(days_ago=days_ago, email_sent=sent, email_sandbox_sent=sandbox_sent) + + @parameterized.expand( + [ + ("no sends decay", 1, 0, 0, 0, "inactive"), + ("only sandbox sends decay like no sends", 1, 600, 600, 0, "inactive"), + ("sandbox sends do not lift own sends over the use bar", 0, 600, 300, 0, "tier_not_used_enough"), + ("own sends over the use bar still promote", 0, 900, 300, 1, "clean_and_used"), + ] + ) + def test_only_own_sends_count_toward_tier_history( + self, _name: str, current_tier: int, sent: int, sandbox_sent: int, expected_tier: int, expected_reason: str + ) -> None: + TeamWorkflowsConfig.objects.update_or_create( + team=self.team, + defaults={ + "email_sending_tier": current_tier, + "email_sending_tier_updated_at": timezone.now() - timedelta(days=40), + }, + ) + for days_ago in (1, 2): + self._record_sends(days_ago=days_ago, sent=sent, sandbox_sent=sandbox_sent) + + decision = recompute_email_sending_tier_for_team(self.team.id) + + assert decision is not None + assert (decision.new_tier, decision.reason) == (expected_tier, expected_reason) + assert TeamWorkflowsConfig.objects.get(team=self.team).email_sending_tier == expected_tier + + @parameterized.expand( + [ + ( + "sandbox excess on one day does not erase own sends of another day", + { + 1: {"email_sent": 100, "email_sandbox_sent": 300}, + 2: {"email_sent": 200, "email_sandbox_sent": 50}, + }, + {2: 150}, + 0, + 0, + ), + ( + "bounces and complaints of sandbox sends still count", + {1: {"email_sent": 50, "email_sandbox_sent": 50, "email_bounced_hard": 2, "email_blocked": 1}}, + {}, + 2, + 1, + ), + ] + ) + def test_sandbox_subtraction_is_per_day_and_keeps_feedback( + self, + _name: str, + metrics_by_days_ago: dict[int, dict[str, int]], + expected_daily_sends_by_days_ago: dict[int, int], + expected_hard_bounced: int, + expected_complained: int, + ) -> None: + for days_ago, counts in metrics_by_days_ago.items(): + self._record_metrics(days_ago=days_ago, **counts) + + history = build_sending_histories(after=timezone.now() - timedelta(days=30), team_ids=[self.team.pk])[ + self.team.pk + ] + + expected_daily_sends = { + (timezone.now() - timedelta(days=days_ago)).strftime("%Y-%m-%d"): count + for days_ago, count in expected_daily_sends_by_days_ago.items() + } + assert history.daily_sends == expected_daily_sends + assert history.sent == sum(expected_daily_sends.values()) + assert (history.hard_bounced, history.complained) == (expected_hard_bounced, expected_complained) + + @override_settings(**TIER_SETTINGS) class TestBackfillEmailSendingTiers(BaseTest): def test_suspended_team_is_not_promoted_despite_qualifying_history(self) -> None: diff --git a/products/workflows/backend/tests/api/test_email_reputation.py b/products/workflows/backend/tests/api/test_email_reputation.py index 6f4a65ffa139..8a2567b75df7 100644 --- a/products/workflows/backend/tests/api/test_email_reputation.py +++ b/products/workflows/backend/tests/api/test_email_reputation.py @@ -91,6 +91,28 @@ def test_reputation_endpoint_returns_empty_shape_when_nothing_was_sent(self): "email_sending_suspension_reason": "", } + def test_sending_allowance_leaves_out_sandbox_sends(self): + sends_by_source = { + self.team.id: {"flow-a": {"email_sent": 5, "email_sandbox_sent": 3}, "flow-b": {"email_sent": 2}} + } + + def requested_metrics(*, name: list[str], **_kwargs: object) -> dict[int, dict[str, dict[str, int]]]: + return { + team_id: { + source: {metric: count for metric, count in counts.items() if metric in name} + for source, counts in sources.items() + } + for team_id, sources in sends_by_source.items() + } + + with patch( + "products.workflows.backend.services.email_health.fetch_app_metric_totals_by_team_and_source", + side_effect=requested_metrics, + ): + allowance = self._get_reputation({})["sending_allowance"] + assert allowance["emails_sent_last_hour"] == 4 + assert allowance["emails_sent_last_day"] == 4 + @parameterized.expand( [ ("no_findings", "ENABLED", None, "healthy"), From 315e059a8e2998c5d43a0c39f066e644e60a25b0 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:07:56 +0000 Subject: [PATCH 13/17] feat(workflows): record sandbox deliveries as their own metric A successful sandbox send records email_sandbox_sent next to email_sent, so the sending tier can leave it out while the workflow metrics still show it. The CDP API's SES webhook captures a workflows sandbox email delivered event for deliveries on the sandbox configuration set. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../internal/workflows-email-sending-tiers.md | 5 +- nodejs/src/cdp/cdp-api.ts | 3 +- nodejs/src/cdp/cdp-services.ts | 31 ++--- .../messaging/email-tracking.service.test.ts | 114 ++++++++++++++++++ .../messaging/email-tracking.service.ts | 24 +++- .../services/messaging/email.service.test.ts | 2 +- .../cdp/services/messaging/email.service.ts | 11 ++ .../src/cdp/services/messaging/helpers/ses.ts | 20 ++- .../messaging/sandbox-email-sender.ts | 7 ++ nodejs/src/cdp/types.ts | 1 + 10 files changed, 197 insertions(+), 21 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 6411b2e9ce2d..9ce0d07ccebc 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -93,6 +93,7 @@ The worker checks the pause again after the daily allowance claim and gives the Django mirrors the tenant named by `SES_SANDBOX_TENANT_NAME` into `SandboxSenderTenantState` from the SES tenant-state webhook and the daily reconciliation, without emailing project admins. Set the same `SES_SANDBOX_TENANT_NAME` on Django web, Celery, and the worker. A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Own-sender sends do not read this state. Each delivered send emits a `workflows sandbox email sent` event. -Successful sandbox sends still count as `email_sent`, which the sending tier reads, until sandbox sends get their own metric. +A successful sandbox send records `email_sandbox_sent` next to `email_sent`, so the workflow's metrics still show it. The sending tier subtracts `email_sandbox_sent` from `email_sent` per day, and the team's sending allowance per workflow, both floored at zero. Sandbox bounces and complaints still count. -Keep the switch off in production until that metric is deployed. +The CDP API's SES webhook captures `workflows sandbox email delivered` for a delivery on `SES_SANDBOX_CONFIGURATION_SET`, test sends included. Without that setting on the CDP API, the event never fires. +Deploy this metric to the worker and Django before you enable the switch in production. diff --git a/nodejs/src/cdp/cdp-api.ts b/nodejs/src/cdp/cdp-api.ts index 649511b14f71..078f2b387690 100644 --- a/nodejs/src/cdp/cdp-api.ts +++ b/nodejs/src/cdp/cdp-api.ts @@ -246,7 +246,8 @@ export class CdpApi { services.capturedEventsService, services.teamWorkflowsConfigService, new EmailTrackingCodeSigner(config.ENCRYPTION_SALT_KEYS, config.CDP_EMAIL_TRACKING_URL), - services.emailSuppressionService + services.emailSuppressionService, + services.sandboxEmailSender ) this.groupsManager = new GroupsManagerService(deps.teamManager, deps.groupRepository) this.batchExportHogFunctionService = new BatchExportHogFunctionService( diff --git a/nodejs/src/cdp/cdp-services.ts b/nodejs/src/cdp/cdp-services.ts index c9e73bfa3f7a..18bf9d36fd76 100644 --- a/nodejs/src/cdp/cdp-services.ts +++ b/nodejs/src/cdp/cdp-services.ts @@ -118,6 +118,7 @@ export interface CdpCoreServices { /** Resolved outputs shared across every CDP service/consumer. */ outputs: CdpOutputs emailService: EmailService + sandboxEmailSender: SandboxEmailSender /** Buffers rendered-email asset rows and bulk-flushes them at the batch boundary. */ messageAssetsService: MessageAssetsService } @@ -466,6 +467,20 @@ export function createCdpCoreServices( const sandboxEmailRateLimiter = deps.emailValidationValkey ? new RateLimiterService(deps.emailValidationValkey, { name: 'sandbox-email' }) : null + const sandboxEmailSender = new SandboxEmailSender( + { + enabled: config.WORKFLOWS_SANDBOX_SENDER_ENABLED, + tenantName: config.SES_SANDBOX_TENANT_NAME, + configurationSetName: config.SES_SANDBOX_CONFIGURATION_SET, + fromAddress: config.SES_SANDBOX_FROM_ADDRESS, + dailyTeamCap: config.WORKFLOWS_SANDBOX_DAILY_TEAM_CAP, + dailyRecipientCap: config.WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP, + }, + deps.teamManager, + redis, + sandboxEmailRateLimiter, + new SandboxSenderStateService(deps.postgres, deps.pubSub) + ) const emailService = new EmailService( { sesAccessKeyId: config.SES_ACCESS_KEY_ID, @@ -488,20 +503,7 @@ export function createCdpCoreServices( messageAssetsService, workflowEmailRateLimiter, teamEmailRateLimiter, - new SandboxEmailSender( - { - enabled: config.WORKFLOWS_SANDBOX_SENDER_ENABLED, - tenantName: config.SES_SANDBOX_TENANT_NAME, - configurationSetName: config.SES_SANDBOX_CONFIGURATION_SET, - fromAddress: config.SES_SANDBOX_FROM_ADDRESS, - dailyTeamCap: config.WORKFLOWS_SANDBOX_DAILY_TEAM_CAP, - dailyRecipientCap: config.WORKFLOWS_SANDBOX_DAILY_RECIPIENT_CAP, - }, - deps.teamManager, - redis, - sandboxEmailRateLimiter, - new SandboxSenderStateService(deps.postgres, deps.pubSub) - ), + sandboxEmailSender, new OrganizationMembersService(deps.postgres) ) const recipientTokensService = new RecipientTokensService(config.ENCRYPTION_SALT_KEYS, config.SITE_URL) @@ -638,6 +640,7 @@ export function createCdpCoreServices( recipientTokensService, outputs, emailService, + sandboxEmailSender, messageAssetsService, } } diff --git a/nodejs/src/cdp/services/messaging/email-tracking.service.test.ts b/nodejs/src/cdp/services/messaging/email-tracking.service.test.ts index c76983c6f596..1b373db67e52 100644 --- a/nodejs/src/cdp/services/messaging/email-tracking.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email-tracking.service.test.ts @@ -17,6 +17,7 @@ import { KAFKA_APP_METRICS_2, KAFKA_LOG_ENTRIES } from '~/common/config/kafka-to import { closeHub, createHub } from '~/common/utils/db/hub' import { PostgresUse } from '~/common/utils/db/postgres' import * as envUtils from '~/common/utils/env-utils' +import * as posthog from '~/common/utils/posthog' import { createCdpConsumerDeps } from '~/tests/helpers/cdp' import { waitForExpect } from '~/tests/helpers/expectations' import { createTestTeamFixture } from '~/tests/helpers/sql' @@ -819,6 +820,119 @@ describe('EmailTrackingService', () => { }) }) + describe('SES webhook records sandbox deliveries', () => { + const sandboxConfigurationSet = 'posthog-messaging-sandbox' + let app: express.Application + let server: Server + let verifySignatureSpy: jest.SpyInstance + let capture: jest.SpyInstance + + beforeEach(() => { + hub.SES_SANDBOX_CONFIGURATION_SET = sandboxConfigurationSet + const api = new CdpApi(hub, createCdpConsumerDeps(hub), { + hogQueue: createMockJobQueue(), + hogflowQueue: createMockJobQueue(), + }) + app = setupExpressApp() + app.use('/', api.router()) + server = app.listen(0, () => {}) + + verifySignatureSpy = jest + .spyOn(SesWebhookHandler.prototype as any, 'verifySnsSignature') + .mockResolvedValue(true) + capture = jest.spyOn(posthog, 'captureTeamEvent').mockImplementation(() => {}) + }) + + afterEach(() => { + server.close() + verifySignatureSpy.mockRestore() + capture.mockRestore() + }) + + const postDelivery = async ( + configurationSet: string, + isTest: boolean, + signedCode = true, + recipients: string[] | null = ['member@example.com'] + ): Promise<supertest.Response> => { + const signedTrackingCode = signer.generate( + { functionId: 'function-id', id: 'invocation-id', teamId: team.id }, + isTest + ) + const [unsignedTrackingCode] = signedTrackingCode.split('.') + const sesRecord = { + eventType: 'Delivery', + mail: { + timestamp: '2024-01-01T00:00:00.000Z', + source: 'sandbox@example.com', + messageId: 'ses-message-id', + destination: ['member@example.com'], + headers: signedCode ? [{ name: TRACKING_CODE_HEADER_NAME, value: signedTrackingCode }] : [], + tags: { + 'ses:configuration-set': [configurationSet], + ...(signedCode ? {} : { ph_id: [unsignedTrackingCode] }), + }, + }, + delivery: { + timestamp: '2024-01-01T00:00:01.000Z', + ...(recipients ? { recipients } : {}), + }, + } + const envelope = { + Type: 'Notification', + MessageId: 'sns-message-id', + TopicArn: 'arn:aws:sns:us-east-1:123456789012:ses-events', + Message: JSON.stringify(sesRecord), + Timestamp: '2024-01-01T00:00:01.000Z', + SignatureVersion: '1', + Signature: 'stubbed', + SigningCertURL: 'https://sns.us-east-1.amazonaws.com/cert.pem', + } + return await supertest(app) + .post('/public/m/ses_webhook') + .set('Content-Type', 'text/plain') + .send(JSON.stringify(envelope)) + } + + it.each([false, true])( + 'captures one delivered event for a delivery on the sandbox set (test send: %s)', + async (isTest) => { + const res = await postDelivery(sandboxConfigurationSet, isTest) + + expect(res.status).toBe(200) + expect(capture).toHaveBeenCalledTimes(1) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id, organization_id: team.organization_id }), + 'workflows sandbox email delivered', + { is_test: isTest, recipient_count: 1 } + ) + } + ) + + it.each([ + ['a delivery on another configuration set', 'posthog-messaging-tracked', true], + ['a sandbox delivery whose tracking code is not signed', sandboxConfigurationSet, false], + ])('captures no delivered event for %s', async (_name, configurationSet, signedCode) => { + const res = await postDelivery(configurationSet, false, signedCode) + + expect(res.status).toBe(200) + expect(capture).not.toHaveBeenCalled() + }) + + it.each([ + { recipients: ['member@example.com', 'another@example.com'], count: 2 }, + { recipients: null, count: 0 }, + ])('counts recipient copies in a delivery notification ($count)', async ({ recipients, count }) => { + const res = await postDelivery(sandboxConfigurationSet, false, true, recipients) + expect(res.status).toBe(200) + expect(capture).toHaveBeenCalledWith( + expect.objectContaining({ id: team.id }), + 'workflows sandbox email delivered', + { is_test: false, recipient_count: count } + ) + }) + }) + describe('resolveEmailEngagementDistinctId', () => { const buildInvocation = ( globals: Partial<NonNullable<CyclotronJobInvocationHogFunction['state']['globals']>> diff --git a/nodejs/src/cdp/services/messaging/email-tracking.service.ts b/nodejs/src/cdp/services/messaging/email-tracking.service.ts index 645866c70885..bfe56f14e91b 100644 --- a/nodejs/src/cdp/services/messaging/email-tracking.service.ts +++ b/nodejs/src/cdp/services/messaging/email-tracking.service.ts @@ -24,6 +24,7 @@ import { EmailSuppressionService } from './email-suppression.service' import { escapeHtml } from './helpers/escape-html' import { SES_LINK_INDEX_TAG, SesWebhookHandler } from './helpers/ses' import { EmailTrackingCodeSigner, trackingCodeFormatCounter } from './helpers/tracking-code' +import { SandboxEmailSender } from './sandbox-email-sender' export const PIXEL_GIF = Buffer.from('R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==', 'base64') const LINK_REGEX = @@ -280,10 +281,15 @@ export class EmailTrackingService { private capturedEventsService: CapturedEventsService, private teamWorkflowsConfigService: TeamWorkflowsConfigService, private trackingCodeSigner: EmailTrackingCodeSigner, - private emailSuppressionService: EmailSuppressionService + private emailSuppressionService: EmailSuppressionService, + private sandboxEmailSender: SandboxEmailSender ) { const allowedTopicArns = (process.env.SES_ALLOWED_SNS_TOPIC_ARNS ?? '').split(',') - this.sesWebhookHandler = new SesWebhookHandler(this.trackingCodeSigner, allowedTopicArns) + this.sesWebhookHandler = new SesWebhookHandler( + this.trackingCodeSigner, + allowedTopicArns, + this.sandboxEmailSender.config.configurationSetName + ) } public async trackMetric({ @@ -493,6 +499,7 @@ export class EmailTrackingService { hardBounceRecipients, complainedRecipients, deliveredRecipients, + sandboxDeliveries, } = await this.sesWebhookHandler.handleWebhook({ body: parseJSON(req.body), headers: req.headers, @@ -577,6 +584,8 @@ export class EmailTrackingService { emailTrackingErrorsCounter.inc({ error_type: 'suppression_update_failed', source: 'ses' }) } + await this.captureSandboxDeliveries(sandboxDeliveries || []) + return { status, message: body as string } } catch (error) { emailTrackingErrorsCounter.inc({ error_type: error.name || 'unknown' }) @@ -585,6 +594,17 @@ export class EmailTrackingService { } } + private async captureSandboxDeliveries( + deliveries: { teamId: string; isTest: boolean; recipientCount: number }[] + ): Promise<void> { + for (const { teamId, isTest, recipientCount } of deliveries) { + const parsedTeamId = Number(teamId) + if (Number.isSafeInteger(parsedTeamId) && parsedTeamId > 0) { + await this.sandboxEmailSender.captureDelivered(parsedTeamId, isTest, recipientCount) + } + } + } + private parseTrackingParams(query: Record<string, any>): { functionId?: string invocationId?: string diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 92870928aa44..5e0f68c486e2 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -509,7 +509,7 @@ describe('EmailService', () => { }), ]) expect(result.metrics.map((metric) => metric.metric_name)).toEqual( - isTest ? [] : ['email_sent', 'email_untracked'] + isTest ? [] : ['email_sent', 'email_untracked', 'email_sandbox_sent'] ) expect(capture).toHaveBeenCalledWith( expect.objectContaining({ id: team.id }), diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index be25934f7f80..679651ceaf4d 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -897,6 +897,17 @@ export class EmailService { }) } + if (success && isSandbox) { + result.metrics.push({ + team_id: invocation.teamId, + app_source_id: invocation.parentRunId ?? invocation.functionId, + instance_id: invocation.state.actionId || invocation.id, + metric_kind: 'email', + metric_name: 'email_sandbox_sent', + count: 1, + }) + } + if (success && assetRow) { result.messageAssets.push(assetRow) } diff --git a/nodejs/src/cdp/services/messaging/helpers/ses.ts b/nodejs/src/cdp/services/messaging/helpers/ses.ts index 04a6ced57eac..e98fedec8b24 100644 --- a/nodejs/src/cdp/services/messaging/helpers/ses.ts +++ b/nodejs/src/cdp/services/messaging/helpers/ses.ts @@ -396,7 +396,8 @@ export class SesWebhookHandler { private trackingCodeSigner: EmailTrackingCodeSigner, // Empty set means no restriction (dev/test); prod is expected to configure the workflow SES // topic ARN. - allowedTopicArns: string[] = [] + allowedTopicArns: string[] = [], + private sandboxConfigurationSetName = '' ) { this.allowedTopicArns = new Set(allowedTopicArns.map((arn) => arn.trim()).filter(Boolean)) } @@ -612,6 +613,7 @@ export class SesWebhookHandler { emailAddresses: string[] timestamp?: string }[] + sandboxDeliveries?: { teamId: string; isTest: boolean; recipientCount: number }[] }> { // Payloads carry recipients and clicked links, including reset links from sensitive emails, // so the logs keep identifiers only. @@ -714,6 +716,7 @@ export class SesWebhookHandler { emailAddresses: string[] timestamp?: string }[] = [] + const sandboxDeliveries: { teamId: string; isTest: boolean; recipientCount: number }[] = [] for (const rec of records) { logger.info('[SesWebhookHandler] processing record', { @@ -877,6 +880,20 @@ export class SesWebhookHandler { // carrier is only used for engagement metrics/log entries above. const codeIsTrusted = parsedCode?.format === 'signed' + if ( + rec.eventType === 'Delivery' && + codeIsTrusted && + teamId && + this.sandboxConfigurationSetName && + rec.mail.tags?.['ses:configuration-set']?.[0] === this.sandboxConfigurationSetName + ) { + sandboxDeliveries.push({ + teamId, + isTest: isTest ?? false, + recipientCount: rec.delivery.recipients?.length ?? 0, + }) + } + // Suppression writes (below) skip test sends — editor "Run test" traffic must not be // able to perturb production suppression state by targeting a bad recipient. const suppressionAllowed = teamId && codeIsTrusted && !isTest @@ -937,6 +954,7 @@ export class SesWebhookHandler { hardBounceRecipients, complainedRecipients, deliveredRecipients, + sandboxDeliveries, } } } diff --git a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts index 0bb188858895..f3c7b6e5ad40 100644 --- a/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts +++ b/nodejs/src/cdp/services/messaging/sandbox-email-sender.ts @@ -266,6 +266,13 @@ export class SandboxEmailSender { }) } + public async captureDelivered(teamId: number, isTest: boolean, recipientCount: number): Promise<void> { + await this.captureEvent(teamId, 'workflows sandbox email delivered', { + recipient_count: recipientCount, + is_test: isTest, + }) + } + public async captureBlocked( teamId: number, isTest: boolean, diff --git a/nodejs/src/cdp/types.ts b/nodejs/src/cdp/types.ts index 79aa6c4a11f3..53869cefe606 100644 --- a/nodejs/src/cdp/types.ts +++ b/nodejs/src/cdp/types.ts @@ -269,6 +269,7 @@ export type MinimalAppMetric = { | 'email_blocked' | 'email_unsubscribed' | 'email_untracked' + | 'email_sandbox_sent' | 'push_sent' | 'push_failed' | 'push_skipped' From f7e9df3170c5d5be27f40c60de9c7273759aa30a Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:27:54 +0000 Subject: [PATCH 14/17] test(workflows): pin the per-workflow floor of the sandbox allowance subtraction The allowance case now gives one workflow more sandbox sends than sends, so a team-wide subtraction would erase the other workflow's own sends. Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../workflows/backend/tests/api/test_email_reputation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/products/workflows/backend/tests/api/test_email_reputation.py b/products/workflows/backend/tests/api/test_email_reputation.py index 8a2567b75df7..a00c1755252a 100644 --- a/products/workflows/backend/tests/api/test_email_reputation.py +++ b/products/workflows/backend/tests/api/test_email_reputation.py @@ -93,7 +93,7 @@ def test_reputation_endpoint_returns_empty_shape_when_nothing_was_sent(self): def test_sending_allowance_leaves_out_sandbox_sends(self): sends_by_source = { - self.team.id: {"flow-a": {"email_sent": 5, "email_sandbox_sent": 3}, "flow-b": {"email_sent": 2}} + self.team.id: {"flow-a": {"email_sent": 1, "email_sandbox_sent": 3}, "flow-b": {"email_sent": 2}} } def requested_metrics(*, name: list[str], **_kwargs: object) -> dict[int, dict[str, dict[str, int]]]: @@ -110,8 +110,8 @@ def requested_metrics(*, name: list[str], **_kwargs: object) -> dict[int, dict[s side_effect=requested_metrics, ): allowance = self._get_reputation({})["sending_allowance"] - assert allowance["emails_sent_last_hour"] == 4 - assert allowance["emails_sent_last_day"] == 4 + assert allowance["emails_sent_last_hour"] == 2 + assert allowance["emails_sent_last_day"] == 2 @parameterized.expand( [ From 69afe3e4b1f246ae73f1a621c27a7516a189d786 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:28:41 +0000 Subject: [PATCH 15/17] test(workflows): assert own-domain sends never record the sandbox metric Refs Silthus/personal#199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- nodejs/src/cdp/services/messaging/email.service.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 5e0f68c486e2..19c33b0738bb 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -2048,6 +2048,7 @@ describe('EmailService', () => { const normal = await service.executeSendEmail(invocation) expect(normal.metrics.map((m) => m.metric_name)).toContain('email_sent') + expect(normal.metrics.map((m) => m.metric_name)).not.toContain('email_sandbox_sent') const testSend = await service.executeSendEmail(invocation, true) expect(testSend.metrics).toEqual([]) From c30c802a210961001839922f3c5586868555fc2c Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:41:01 +0000 Subject: [PATCH 16/17] fix(cdp): decide the organization block and the members from one snapshot Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../organization-members.service.test.ts | 25 ++++++++++++------- .../managers/organization-members.service.ts | 16 +++++++----- .../cdp/services/messaging/email.service.ts | 7 +++--- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/nodejs/src/cdp/services/managers/organization-members.service.test.ts b/nodejs/src/cdp/services/managers/organization-members.service.test.ts index 856579622a2d..75a8682319a5 100644 --- a/nodejs/src/cdp/services/managers/organization-members.service.test.ts +++ b/nodejs/src/cdp/services/managers/organization-members.service.test.ts @@ -9,34 +9,41 @@ describe('OrganizationMembersService', () => { email, is_email_verified: true, }) + const rowsFor = (name: string, member: string): { rows: unknown[] } => ({ + rows: name === 'fetch-sandbox-team-organization-members' ? [memberRow(member)] : [], + }) + const allowed = { organizationBlocked: false, blockedRecipients: [] } it('fails closed when every member lookup outlasts its one-minute decision', async () => { let now = 0 - const query = jest.fn(() => { + const query = jest.fn((_use: unknown, _sql: string, _params: unknown, name: string) => { now += 60_000 - return Promise.resolve({ rows: [memberRow('member@example.com')] }) + return Promise.resolve(rowsFor(name, 'member@example.com')) }) const service = new OrganizationMembersService({ query } as unknown as PostgresRouter, () => now) - await expect(service.getBlockedRecipients(1, ['member@example.com'])).rejects.toThrow('expired') + await expect(service.checkRecipients(1, ['member@example.com'])).rejects.toThrow('expired') expect(query).toHaveBeenCalledTimes(2 * queriesPerLookup) }) it('shares one member lookup across sends and reloads it after a minute', async () => { let now = 0 let member = 'member@example.com' - const query = jest.fn(() => Promise.resolve({ rows: [memberRow(member)] })) + const query = jest.fn((_use: unknown, _sql: string, _params: unknown, name: string) => + Promise.resolve(rowsFor(name, member)) + ) const service = new OrganizationMembersService({ query } as unknown as PostgresRouter, () => now) - expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([]) - expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([]) + expect(await service.checkRecipients(1, ['member@example.com'])).toEqual(allowed) + expect(await service.checkRecipients(1, ['member@example.com'])).toEqual(allowed) expect(query).toHaveBeenCalledTimes(queriesPerLookup) now = 60_001 member = 'new-member@example.com' - expect(await service.getBlockedRecipients(1, ['member@example.com'])).toEqual([ - { address: 'member@example.com', unverifiedMember: false }, - ]) + expect(await service.checkRecipients(1, ['member@example.com'])).toEqual({ + organizationBlocked: false, + blockedRecipients: [{ address: 'member@example.com', unverifiedMember: false }], + }) expect(query).toHaveBeenCalledTimes(2 * queriesPerLookup) }) }) diff --git a/nodejs/src/cdp/services/managers/organization-members.service.ts b/nodejs/src/cdp/services/managers/organization-members.service.ts index 6c289871ee97..5aa250befae7 100644 --- a/nodejs/src/cdp/services/managers/organization-members.service.ts +++ b/nodejs/src/cdp/services/managers/organization-members.service.ts @@ -10,6 +10,10 @@ type OrganizationMemberSnapshot = { export type BlockedRecipient = { readonly address: string; readonly unverifiedMember: boolean } +export type RecipientCheck = + | { readonly organizationBlocked: true } + | { readonly organizationBlocked: false; readonly blockedRecipients: BlockedRecipient[] } + const MAX_SNAPSHOT_AGE_MS = 60_000 const REFRESH_AGE_MS = MAX_SNAPSHOT_AGE_MS - 1_000 @@ -78,15 +82,15 @@ export class OrganizationMembersService { }) } - public async getBlockedRecipients(teamId: number, recipients: string[]): Promise<BlockedRecipient[]> { + public async checkRecipients(teamId: number, recipients: string[]): Promise<RecipientCheck> { const snapshot = await this.getFreshSnapshot(String(teamId)) - return recipients + if (snapshot.sandboxSenderBlocked) { + return { organizationBlocked: true } + } + const blockedRecipients = recipients .filter((email) => !snapshot.verifiedEmails.has(normalizeEmail(email))) .map((address) => ({ address, unverifiedMember: snapshot.unverifiedEmails.has(normalizeEmail(address)) })) - } - - public async isSandboxSenderBlocked(teamId: number): Promise<boolean> { - return (await this.getFreshSnapshot(String(teamId))).sandboxSenderBlocked + return { organizationBlocked: false, blockedRecipients } } // A snapshot older than a minute never decides a send: it is reloaded once, and a reload that diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index 5eac5967aea0..1e22af68039b 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -1079,10 +1079,9 @@ export class EmailService { if (!this.organizationMembers) { throw new Error('Organization member checks are not configured') } - organizationBlocked = await this.organizationMembers.isSandboxSenderBlocked(teamId) - const blocked = organizationBlocked - ? [] - : await this.organizationMembers.getBlockedRecipients(teamId, recipients) + const check = await this.organizationMembers.checkRecipients(teamId, recipients) + organizationBlocked = check.organizationBlocked + const blocked = check.organizationBlocked ? [] : check.blockedRecipients blockedRecipients = blocked.map(({ address }) => address) reason = blocked.every(({ unverifiedMember }) => unverifiedMember) ? 'recipient_unverified' From 53833496c7332df3692180accd10ed7d6bbeeff3 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:06:31 +0000 Subject: [PATCH 17/17] chore(cdp): say where a blocked organization's sandbox sends skip The block layer documents the admin and the endpoint, so this layer only adds the worker side. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- docs/internal/workflows-email-sending-tiers.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 6fd31eddc428..273a1b920724 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -29,7 +29,7 @@ Only workflows that send email are subject to the tiers; SMS, push, and webhook The `workflows-sandbox-sender` flag enables a PostHog-managed email integration per project through `POST /api/projects/:project_id/integrations/email_sandbox_sender/`. The endpoint requires `integration:write` and project membership. Repeated requests keep the same integration and refresh its display name from the organization name. Staff turn the sandbox sender off for one organization by adding a `SandboxSenderOrganizationBlock` row in the Django admin, and lift the block by deleting the row. -While the block exists, the endpoint returns 404 for the organization's projects. Existing sandbox senders stay, and the email worker does not read the block. +While the block exists, the endpoint returns 404 for the organization's projects. Existing sandbox senders stay, but their sends skip. Set `WORKFLOWS_SANDBOX_SENDER_DOMAIN`, `SES_SANDBOX_FROM_ADDRESS`, and `SES_SANDBOX_TENANT_NAME` in Django; any empty value leaves the endpoint unavailable. The worker sends from the same `SES_SANDBOX_FROM_ADDRESS`, so set it once for Django and the worker. The address the UI shows is then the address the worker sends from. Workflow email steps and test sends offer it as a sender. @@ -117,8 +117,7 @@ Every sandbox send, test sends included, skips while the shared sandbox SES tena The worker checks the pause again after the daily allowance claim and gives the claim back when the pause blocks delivery. Django mirrors the tenant named by `SES_SANDBOX_TENANT_NAME` into `SandboxSenderTenantState` from the SES tenant-state webhook and the daily reconciliation, without emailing project admins. Set the same `SES_SANDBOX_TENANT_NAME` on Django web, Celery, and the worker. A committed state change reaches the workers through the `reload-sandbox-sender-state` message. Own-sender sends do not read this state. -Staff turn the sandbox sender off for one organization by adding a `SandboxSenderOrganizationBlock` row in the Django admin, and lift the block by deleting the row. -A blocked organization's sandbox sends skip with the reason `organization_blocked`, and its projects get no new sandbox sender. +A blocked organization's sandbox sends skip with the reason `organization_blocked`. The worker reads the block with the organization members, so a block and its removal reach the workers within one minute. Own-sender sends do not read it. Each delivered send emits a `workflows sandbox email sent` event. Test sends never record `email_sent`, so they do not count toward the sending tier.