fix(auth): make the rate limiter engage by making the failure budget global - #10
Merged
Conversation
The limiter keyed on the rightmost X-Forwarded-For entry, which on Render is a proxy hop that varies per request, so every failed attempt landed on a fresh counter and 429 never arrived — 25 bad tokens in a row all got 401 in production. Replace per-client tracking with one global fixed-window failure budget (render-auth-limiter.mjs): it needs no client address, cannot be evaded by rotating source addresses, and a valid token never touches it, so the lockout still can't be aimed at the token holder. Recording and deciding are one step, so the 10th failure is the first 429 rather than the 11th. Add node:test coverage for both the budget and the proxy's wiring, including the varying-X-Forwarded-For regression, and run it in CI ahead of the Docker smoke test.
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
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.
Summary
The gate's rate limiter never engaged in production. It keyed its failure counter on the rightmost
X-Forwarded-Forentry, which behind Render's edge is a proxy hop that varies per request — so every failed attempt landed on a fresh counter and the429never came.Failed attempts now share one global budget (10 per minute), extracted into
render-auth-limiter.mjsso the stateful part is testable on its own with an injected clock.A per-address budget can't provide the property worth guaranteeing here anyway: this gate fronts a single shared secret, so what matters is a ceiling on the total guess rate, and an attacker rotating source addresses just gets a fresh allowance. Only requests that were going to be refused anyway spend the budget, so a valid token never consumes it and is never subject to it — the lockout can't be aimed at the token holder.
Tests
Adds
npm run test:wrapper(node:test, stdlib only, no Docker) and a CI job that runs it ahead of the smoke test, so a broken gate fails in seconds instead of after pulling the base image. Covers the failure budget's window arithmetic and the real proxy process answering401/429/200/501over a stub upstream — including a direct regression test that429still arrives when the rightmostX-Forwarded-Forvaries per request.Verified end to end:
./render-smoke-test.shpasses all six checks against the built image, including bad tokens getting401up to the limit then429, with the valid token still returning200.Notes
Dockerfile.rendermust copy the new module too, or the container crashes at startup on a missing import.429rather than401— so a stale token looks like rate limiting. A request with the right token is unaffected.