diff --git a/src/services/twilio.ts b/src/services/twilio.ts index 9c29ed04b..99ab1d8c6 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,49 @@ 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". +// 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. +// +// 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 + +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 +92,36 @@ 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 } + // 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, + }, + "verify send rejected with HTTP 429 (rate limit, cause unconfirmed — not necessarily the global send cap)", + ) + 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..f94b0a2a4 --- /dev/null +++ b/test/flash/unit/services/twilio.spec.ts @@ -0,0 +1,188 @@ +/** + * 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, + }), + "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 }), + ) + + const result = await TwilioClient().initiateVerify({ to: phone, channel: "sms" }) + + 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( + 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) + }) +})