From 45c6c4ddc7adee0e519872cbb3bd181ad920d44f Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 2 Sep 2026 08:00:20 -0700 Subject: [PATCH 1/3] feat(auth): carry the global_sends rate-limit key on every Twilio Verify send The 2026-09-02 09:04Z SMS-pumping probe fired 392 OTP requests in one minute from 364 rotating IPs. Per-IP and per-number limits are blind to that shape; the Verify geo-permission allowlist caught 106 of them, and that list is now being opened for global signups. The replacement is a Twilio Verify Programmable Rate Limit on the Flash Verify service keyed on a CONSTANT value, so it applies to every send regardless of IP or destination: unique_name `global_sends`, buckets 40 per 60s and 600 per 3600s (legitimate peak on 2026-09-01 was 18/min). Twilio only enforces it when the verification request carries the key, so `initiateVerify` now passes `rateLimits: { global_sends: "all" }` on every sms and whatsapp send. An exhausted bucket comes back as HTTP 429 / error 60203 ("Max send attempts reached"); it is now recognised by status/code as well as by message text and answered with PhoneProviderRateLimitExceededError, with one warn line per rejection carrying the masked number, channel and Twilio code. Not an ops-feed event on purpose: the cap bounds how many rejections a probe can produce, and a per-rejection embed would evict the rest of the feed during the burst. Whether Twilio rejects a request whose key names a rate limit that no longer exists is not documented: do not delete the `global_sends` rate limit on the service without first redeploying without this key. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8 --- src/services/twilio.ts | 56 +++++++++- test/flash/unit/services/twilio.spec.ts | 140 ++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 test/flash/unit/services/twilio.spec.ts diff --git a/src/services/twilio.ts b/src/services/twilio.ts index 9c29ed04b..3c8f42de8 100644 --- a/src/services/twilio.ts +++ b/src/services/twilio.ts @@ -20,6 +20,7 @@ import { UnknownPhoneProviderServiceError, UnsubscribedRecipientPhoneProviderError, } from "@domain/phone-provider" +import { maskPhone } from "@services/alerts/ops-events" import { baseLogger } from "@services/logger" import { TestAccountsChecker } from "@domain/accounts/test-accounts-checker" @@ -32,6 +33,36 @@ import { wrapAsyncFunctionsToRunInSpan } from "./tracing" export const TWILIO_ACCOUNT_TEST = "AC_twilio_id" +// Service-wide send cap, enforced by Twilio (Verify Programmable Rate Limits). +// +// The 2026-09-02 09:04Z SMS-pumping probe fired 392 OTP requests in one +// minute from 364 rotating IPs; per-IP and per-number limits are blind to that +// shape, and the Verify geo-permission allowlist that caught most of it is +// being opened for global signups. A rate limit keyed on a CONSTANT value +// applies to every send regardless of IP or destination, so the service can +// never send faster than the buckets allow. Configured on the Flash Verify +// service: unique_name `global_sends`, buckets 40 per 60s and 600 per 3600s +// (legitimate peak on 2026-09-01 was 18 per minute). +// +// Twilio only enforces a programmable rate limit when the verification request +// carries its key, so this must go on every send. Whether Twilio rejects a +// request whose key names a rate limit that no longer exists on the service +// is not documented; do not delete the rate limit without redeploying without +// this key. +export const VERIFY_GLOBAL_SEND_CAP_KEY = "global_sends" +export const VERIFY_GLOBAL_SEND_CAP_VALUE = "all" + +// Twilio answers an exhausted Verify rate-limit bucket (built-in per-number or +// programmable) with HTTP 429 and error 60203 "Max send attempts reached". +const TWILIO_RATE_LIMIT_HTTP_STATUS = 429 +const TWILIO_RATE_LIMIT_ERROR_CODE = 60203 + +const isTwilioRateLimitRejection = (err: unknown): boolean => { + if (!err || typeof err !== "object") return false + const { status, code } = err as { status?: unknown; code?: unknown } + return status === TWILIO_RATE_LIMIT_HTTP_STATUS || code === TWILIO_RATE_LIMIT_ERROR_CODE +} + export const TwilioClient = (): IPhoneProviderService => { const accountSid = TWILIO_ACCOUNT_SID const authToken = TWILIO_AUTH_TOKEN @@ -48,8 +79,31 @@ export const TwilioClient = (): IPhoneProviderService => { channel: ChannelType }): Promise => { try { - await verify.verifications.create({ to, channel }) + await verify.verifications.create({ + to, + channel, + rateLimits: { [VERIFY_GLOBAL_SEND_CAP_KEY]: VERIFY_GLOBAL_SEND_CAP_VALUE }, + }) } catch (err) { + if (isTwilioRateLimitRejection(err)) { + // One line per rejection is fine here: the cap itself bounds how many + // of these a probe can produce. Not an ops-feed event on purpose — a + // per-rejection embed would evict the rest of the feed during exactly + // the burst this exists for (see the coalescing in request-code.ts). + const { status, code } = err as { status?: unknown; code?: unknown } + baseLogger.warn( + { + to: maskPhone(to), + channel, + twilioStatus: status, + twilioCode: code, + rateLimitKey: VERIFY_GLOBAL_SEND_CAP_KEY, + }, + "verify send rejected by twilio rate limit", + ) + return new PhoneProviderRateLimitExceededError(parseErrorMessageFromUnknown(err)) + } + baseLogger.error({ err }, "impossible to send text") return handleCommonErrors(err) } diff --git a/test/flash/unit/services/twilio.spec.ts b/test/flash/unit/services/twilio.spec.ts new file mode 100644 index 000000000..897aec686 --- /dev/null +++ b/test/flash/unit/services/twilio.spec.ts @@ -0,0 +1,140 @@ +/** + * Every Twilio Verify send must carry the `global_sends` programmable + * rate-limit key: Twilio only enforces the service-wide send cap (40/min, + * 600/h, configured on the Verify service) for requests that name it. The + * 2026-09-02 09:04Z probe (392 OTP requests in one minute from 364 IPs) is + * the shape this cap exists for. + */ +const verificationsCreate = jest.fn() +const verificationChecksCreate = jest.fn() + +jest.mock("twilio", () => + jest.fn(() => ({ + verify: { + v2: { + services: jest.fn(() => ({ + verifications: { create: verificationsCreate }, + verificationChecks: { create: verificationChecksCreate }, + })), + }, + }, + lookups: { v1: { phoneNumbers: jest.fn() } }, + })), +) + +jest.mock("@config", () => ({ + TWILIO_ACCOUNT_SID: "ACtest", + TWILIO_AUTH_TOKEN: "token", + TWILIO_VERIFY_SERVICE_ID: "VAtest", + UNSECURE_DEFAULT_LOGIN_CODE: "000000", + getTestAccounts: () => [], +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +jest.mock("@services/tracing", () => ({ + wrapAsyncFunctionsToRunInSpan: ({ fns }: { fns: unknown }) => fns, +})) + +import { baseLogger } from "@services/logger" +import { + KnownTwilioErrorMessages, + TwilioClient, + VERIFY_GLOBAL_SEND_CAP_KEY, + VERIFY_GLOBAL_SEND_CAP_VALUE, +} from "@services/twilio" +import { + PhoneProviderRateLimitExceededError, + RestrictedRegionPhoneProviderError, +} from "@domain/phone-provider" + +const phone = "+18765550100" as PhoneNumber +const mockedLogger = baseLogger as unknown as { warn: jest.Mock; error: jest.Mock } + +// Shape of the twilio SDK's RestException on a 429. +const rateLimitRejection = () => + Object.assign(new Error("Max send attempts reached"), { status: 429, code: 60203 }) + +describe("TwilioClient.initiateVerify", () => { + beforeEach(() => { + verificationsCreate.mockReset() + mockedLogger.warn.mockReset() + mockedLogger.error.mockReset() + }) + + it.each(["sms", "whatsapp"] as ChannelType[])( + "carries the global_sends rate-limit key on a %s send", + async (channel) => { + verificationsCreate.mockResolvedValue({ status: "pending" }) + + const result = await TwilioClient().initiateVerify({ to: phone, channel }) + + expect(result).toBe(true) + expect(verificationsCreate).toHaveBeenCalledTimes(1) + expect(verificationsCreate).toHaveBeenCalledWith({ + to: phone, + channel, + rateLimits: { [VERIFY_GLOBAL_SEND_CAP_KEY]: VERIFY_GLOBAL_SEND_CAP_VALUE }, + }) + }, + ) + + it("uses a constant key value so the cap is service-wide, not per user", () => { + expect(VERIFY_GLOBAL_SEND_CAP_KEY).toBe("global_sends") + expect(VERIFY_GLOBAL_SEND_CAP_VALUE).toBe("all") + }) + + it("maps an exhausted bucket (HTTP 429 / 60203) to the rate-limit error and warns with a masked number", async () => { + verificationsCreate.mockRejectedValue(rateLimitRejection()) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + expect(result).toBeInstanceOf(PhoneProviderRateLimitExceededError) + expect(mockedLogger.warn).toHaveBeenCalledWith( + expect.objectContaining({ + to: "+1876…00", + channel: "sms", + twilioStatus: 429, + twilioCode: 60203, + rateLimitKey: "global_sends", + }), + "verify send rejected by twilio rate limit", + ) + expect(mockedLogger.error).not.toHaveBeenCalled() + expect(JSON.stringify(mockedLogger.warn.mock.calls)).not.toContain(phone) + }) + + it("still maps a 429 whose text drifts away from the known regex", async () => { + verificationsCreate.mockRejectedValue( + Object.assign(new Error("Too Many Requests"), { status: 429 }), + ) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + expect(result).toBeInstanceOf(PhoneProviderRateLimitExceededError) + }) + + it("keeps the existing message-based mapping for everything else", async () => { + verificationsCreate.mockRejectedValue( + Object.assign( + new Error( + "The destination phone number has been blocked by Verify Geo-Permissions. SN is blocked for sms channel for all services", + ), + { status: 403, code: 60605 }, + ), + ) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + expect(result).toBeInstanceOf(RestrictedRegionPhoneProviderError) + expect(mockedLogger.error).toHaveBeenCalledWith( + expect.objectContaining({ err: expect.any(Error) }), + "impossible to send text", + ) + expect( + KnownTwilioErrorMessages.RateLimitsExceeded.test("Max send attempts reached"), + ).toBe(true) + }) +}) From a3a37896e8d9508402563356c07625f1b69ec1fd Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 2 Sep 2026 10:25:55 -0700 Subject: [PATCH 2/3] fix(auth): don't misattribute Twilio's concurrent-request throttle to the global send cap isTwilioRateLimitRejection() treated every HTTP 429 from verifications.create as the new global_sends cap tripping, including Twilio's unrelated "Too many concurrent requests" throttle (which also returns 429 but a different error code). The warn log asserted rateLimitKey: "global_sends" and "verify send rejected by twilio rate limit" for both cases, so an on-call engineer grepping that line during a real concurrent-request event would misdiagnose it as the send cap. Only attach the global_sends framing (message + rateLimitKey field) when the error code confirms it (60203, "Max send attempts reached"). Any other 429 is still mapped to PhoneProviderRateLimitExceededError (preserving the existing lenient-429 fallback) but logs a cause-unconfirmed message with no rateLimitKey. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8 --- src/services/twilio.ts | 20 ++++++++++++++-- test/flash/unit/services/twilio.spec.ts | 31 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/services/twilio.ts b/src/services/twilio.ts index 3c8f42de8..5123fdb91 100644 --- a/src/services/twilio.ts +++ b/src/services/twilio.ts @@ -54,6 +54,13 @@ export const VERIFY_GLOBAL_SEND_CAP_VALUE = "all" // Twilio answers an exhausted Verify rate-limit bucket (built-in per-number or // programmable) with HTTP 429 and error 60203 "Max send attempts reached". +// +// Twilio also answers its unrelated "Too many concurrent requests" throttle +// with HTTP 429 (a different, undocumented-here error code). We still treat +// any 429 as a rate-limit rejection — Twilio's error text/codes for the +// bucket case have drifted before — but only 60203 confirms it was the +// global_sends cap specifically. See the isConfirmedGlobalCap branch below: +// don't attach the global_sends framing to a 429 we can't attribute to it. const TWILIO_RATE_LIMIT_HTTP_STATUS = 429 const TWILIO_RATE_LIMIT_ERROR_CODE = 60203 @@ -91,15 +98,24 @@ export const TwilioClient = (): IPhoneProviderService => { // per-rejection embed would evict the rest of the feed during exactly // the burst this exists for (see the coalescing in request-code.ts). const { status, code } = err as { status?: unknown; code?: unknown } + // Only error code 60203 confirms this 429 was the global_sends cap + // tripping. Twilio's unrelated "Too many concurrent requests" + // throttle (and any other 429 shape) also lands here, so don't claim + // the global_sends cause or key for those — an on-call engineer + // grepping this line during a real concurrent-request throttling + // event would otherwise misdiagnose it as the send cap tripping. + const isConfirmedGlobalCap = code === TWILIO_RATE_LIMIT_ERROR_CODE baseLogger.warn( { to: maskPhone(to), channel, twilioStatus: status, twilioCode: code, - rateLimitKey: VERIFY_GLOBAL_SEND_CAP_KEY, + ...(isConfirmedGlobalCap ? { rateLimitKey: VERIFY_GLOBAL_SEND_CAP_KEY } : {}), }, - "verify send rejected by twilio rate limit", + isConfirmedGlobalCap + ? "verify send rejected by twilio rate limit" + : "verify send rejected with HTTP 429 (rate limit, cause unconfirmed — not necessarily the global send cap)", ) return new PhoneProviderRateLimitExceededError(parseErrorMessageFromUnknown(err)) } diff --git a/test/flash/unit/services/twilio.spec.ts b/test/flash/unit/services/twilio.spec.ts index 897aec686..f8dac584a 100644 --- a/test/flash/unit/services/twilio.spec.ts +++ b/test/flash/unit/services/twilio.spec.ts @@ -116,6 +116,37 @@ describe("TwilioClient.initiateVerify", () => { expect(result).toBeInstanceOf(PhoneProviderRateLimitExceededError) }) + it("does not attribute Twilio's unrelated concurrent-request throttle (429, non-60203) to the global_sends cap", async () => { + // Twilio's "Too many concurrent requests" throttle also returns HTTP 429, + // but with a different error code than the exhausted-bucket case (60203). + // The log line must not claim the global_sends cap tripped, or an + // on-call engineer grepping for it during a real concurrent-request + // event will misdiagnose it as the send cap. + verificationsCreate.mockRejectedValue( + Object.assign(new Error("Too many concurrent requests"), { + status: 429, + code: 20429, + }), + ) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + expect(result).toBeInstanceOf(PhoneProviderRateLimitExceededError) + expect(mockedLogger.warn).toHaveBeenCalledTimes(1) + const [fields, message] = mockedLogger.warn.mock.calls[0] + expect(fields).toEqual( + expect.objectContaining({ + to: "+1876…00", + channel: "sms", + twilioStatus: 429, + twilioCode: 20429, + }), + ) + expect(fields).not.toHaveProperty("rateLimitKey") + expect(message).not.toBe("verify send rejected by twilio rate limit") + expect(mockedLogger.error).not.toHaveBeenCalled() + }) + it("keeps the existing message-based mapping for everything else", async () => { verificationsCreate.mockRejectedValue( Object.assign( From e6485cd21f945e6f52f77fdc972f21573ee45136 Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 2 Sep 2026 10:33:39 -0700 Subject: [PATCH 3/3] fix(auth): stop claiming 60203 confirms the global_sends cap tripped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 narrowed the log's rate-limit-cause claim from "any 429" to "code 60203", but 60203 is Twilio's answer for both the new global_sends cap AND its built-in per-number Verify resend limit — the file's own comment already said as much. A legitimate user mashing "resend code" (routine, frequent) triggers the exact same code as an actual service-wide-cap trip (rare), so the log kept misattributing ordinary per-number throttling to the attack mitigation. There is no code-level signal in Twilio's response that distinguishes which bucket exhausted, so drop the confirmed/unconfirmed branch entirely: every 429/60203 rejection now logs status + code only, with the cause-unconfirmed message, and never claims the global_sends key. Adds a test asserting a 60203 rejection does not carry rateLimitKey or the "rejected by twilio rate limit" message. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8 --- src/services/twilio.ts | 32 +++++++++++++------------ test/flash/unit/services/twilio.spec.ts | 21 ++++++++++++++-- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/services/twilio.ts b/src/services/twilio.ts index 5123fdb91..99ab1d8c6 100644 --- a/src/services/twilio.ts +++ b/src/services/twilio.ts @@ -54,13 +54,19 @@ export const VERIFY_GLOBAL_SEND_CAP_VALUE = "all" // Twilio answers an exhausted Verify rate-limit bucket (built-in per-number or // programmable) with HTTP 429 and error 60203 "Max send attempts reached". -// // Twilio also answers its unrelated "Too many concurrent requests" throttle // with HTTP 429 (a different, undocumented-here error code). We still treat // any 429 as a rate-limit rejection — Twilio's error text/codes for the -// bucket case have drifted before — but only 60203 confirms it was the -// global_sends cap specifically. See the isConfirmedGlobalCap branch below: -// don't attach the global_sends framing to a 429 we can't attribute to it. +// bucket case have drifted before. +// +// Critically, 60203 does NOT confirm the global_sends cap tripped: Twilio +// returns the exact same code for its built-in per-number Verify limit, +// which a single legitimate user hits just by mashing "resend code" — a +// routine, frequent event, unlike an actual attack tripping the +// service-wide cap. There is no code-level signal in Twilio's response +// that distinguishes which bucket exhausted, so never attach the +// global_sends key or framing to any 429/60203 rejection below — doing so +// would misdiagnose ordinary per-number throttling as the send cap tripping. const TWILIO_RATE_LIMIT_HTTP_STATUS = 429 const TWILIO_RATE_LIMIT_ERROR_CODE = 60203 @@ -98,24 +104,20 @@ export const TwilioClient = (): IPhoneProviderService => { // per-rejection embed would evict the rest of the feed during exactly // the burst this exists for (see the coalescing in request-code.ts). const { status, code } = err as { status?: unknown; code?: unknown } - // Only error code 60203 confirms this 429 was the global_sends cap - // tripping. Twilio's unrelated "Too many concurrent requests" - // throttle (and any other 429 shape) also lands here, so don't claim - // the global_sends cause or key for those — an on-call engineer - // grepping this line during a real concurrent-request throttling - // event would otherwise misdiagnose it as the send cap tripping. - const isConfirmedGlobalCap = code === TWILIO_RATE_LIMIT_ERROR_CODE + // No code Twilio returns (60203 included) distinguishes the + // global_sends cap from its built-in per-number Verify limit, so + // never claim the global_sends cause or key here — an on-call + // engineer grepping this line during ordinary per-number + // throttling would otherwise misdiagnose it as the send cap + // tripping. See the comment above TWILIO_RATE_LIMIT_HTTP_STATUS. baseLogger.warn( { to: maskPhone(to), channel, twilioStatus: status, twilioCode: code, - ...(isConfirmedGlobalCap ? { rateLimitKey: VERIFY_GLOBAL_SEND_CAP_KEY } : {}), }, - isConfirmedGlobalCap - ? "verify send rejected by twilio rate limit" - : "verify send rejected with HTTP 429 (rate limit, cause unconfirmed — not necessarily the global send cap)", + "verify send rejected with HTTP 429 (rate limit, cause unconfirmed — not necessarily the global send cap)", ) return new PhoneProviderRateLimitExceededError(parseErrorMessageFromUnknown(err)) } diff --git a/test/flash/unit/services/twilio.spec.ts b/test/flash/unit/services/twilio.spec.ts index f8dac584a..f94b0a2a4 100644 --- a/test/flash/unit/services/twilio.spec.ts +++ b/test/flash/unit/services/twilio.spec.ts @@ -98,14 +98,31 @@ describe("TwilioClient.initiateVerify", () => { channel: "sms", twilioStatus: 429, twilioCode: 60203, - rateLimitKey: "global_sends", }), - "verify send rejected by twilio rate limit", + "verify send rejected with HTTP 429 (rate limit, cause unconfirmed — not necessarily the global send cap)", ) expect(mockedLogger.error).not.toHaveBeenCalled() expect(JSON.stringify(mockedLogger.warn.mock.calls)).not.toContain(phone) }) + it("does NOT attribute a 60203 rejection to the global_sends cap, since Twilio's built-in per-number Verify limit returns the identical code", async () => { + // 60203 ("Max send attempts reached") is Twilio's answer for BOTH the + // built-in per-number Verify limit (a normal user mashing "resend + // code") and the programmable global_sends cap (an actual attack). + // There is no field in the error that tells them apart, so the log + // must never claim the global_sends key/cause just because code === + // 60203 — that would misdiagnose routine per-number throttling as the + // attack-mitigation cap tripping. + verificationsCreate.mockRejectedValue(rateLimitRejection()) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + expect(result).toBeInstanceOf(PhoneProviderRateLimitExceededError) + const [fields, message] = mockedLogger.warn.mock.calls[0] + expect(fields).not.toHaveProperty("rateLimitKey") + expect(message).not.toBe("verify send rejected by twilio rate limit") + }) + it("still maps a 429 whose text drifts away from the known regex", async () => { verificationsCreate.mockRejectedValue( Object.assign(new Error("Too Many Requests"), { status: 429 }),