feat(auth): carry the global_sends rate-limit key on every Twilio Verify send - #504
Open
islandbitcoin wants to merge 3 commits into
Open
feat(auth): carry the global_sends rate-limit key on every Twilio Verify send#504islandbitcoin wants to merge 3 commits into
islandbitcoin wants to merge 3 commits into
Conversation
…ify 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8
… 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
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 cannot see that shape. The Twilio Verify geo-permission allowlist caught 106 of those sends (the app-layer country gate from #496 caught 230), and that allowlist is now being opened so Flash can sign up users globally. This PR is the replacement layer.
Twilio-side configuration (already applied to the Flash Verify service)
A Verify Programmable Rate Limit keyed on a constant value, so it is a service-wide send cap rather than a per-user one:
global_sendsglobal_sendsLegitimate peak during the 2026-09-01 signup wave was 18 sends/min, so the cap never touches real traffic; the 09:04Z probe would have been cut to 40 SMS.
Change
initiateVerifypassesrateLimits: { global_sends: "all" }on every sms/whatsappverifications.create(twilio SDK 4.23.0 serialises the object as theRateLimitsJSON param). The cap is inert until this deploys — Twilio only enforces a programmable rate limit for requests that carry its key.PhoneProviderRateLimitExceededError, so the user sees the existing rate-limit message instead of "unexpected error". One warn line per rejection with the masked number, channel, status, code andrateLimitKey. Deliberately not an ops-feed event: the cap bounds the rejection count, and a per-rejection embed would evict the rest of the 50-slot feed during exactly the burst this exists for.What a user sees when the cap trips
The existing
PhoneProviderRateLimitExceededErrorpath (rate-limit message, retry later). During a probe that is the intended outcome; a real user caught inside a probe minute retries a minute later.Not verified
Twilio's docs do not say whether a request whose key names a rate limit that no longer exists on the service is rejected or ignored. Treat the
global_sendsrate limit as load-bearing: do not delete it on the service without redeploying without this key.Verification
New spec
test/flash/unit/services/twilio.spec.ts: key present on sms and whatsapp sends, constant value, 429/60203 → rate-limit error with masked warn log, 429 with drifted text still mapped, geo-block message mapping unchanged.Follow-ups
request-code.ts.🤖 Generated with Claude Code
https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8