security: rate-limit the auth and engine-facing endpoints (#63) - #101
Merged
Conversation
This was referenced Jul 31, 2026
Ceilings on what an unauthenticated caller can reach. Not the last line of defence — authentication is — so these are sized to stop floods, not to meter usage. | Bucket | Keyed on | Default | |---|---|---| | `/auth/login`, `/auth/callback` | client address | 20/min | | Rejected engine tokens | client address | 30/min | | Accepted engine requests | **engine id** | 600/min | Three decisions are the substance here, because the alternatives all look reasonable until you think about the failure mode: * **It fails open.** An unreachable counter store allows the request and logs a warning. A limiter that locks every operator out of the console when Redis hiccups has done more damage than the traffic it was defending against. * **Engines are charged per engine, not per address.** A fleet behind one NAT would otherwise share a budget and throttle itself, and `--scale engine=N` would stop meaning anything. * **Only *rejected* token presentations are charged to an address.** That is what lets the credential-stuffing bucket be small without any risk to a healthy fleet — a working engine never touches it. `X-Forwarded-For` is ignored unless `ICEBERG_TRUSTED_PROXY_HOPS` says how many proxies are in front. The header is caller-controlled, so trusting it by default would let an attacker take a fresh identity per request — the same as having no limit. Behind a load balancer this must be set, which the docs say in as many words. Counters are fixed-window in Redis, so a limit means the same thing across replicas; in-memory is the fallback and is a real limit, just a per-replica one. Refusals answer 429 with `Retry-After`; allowed requests carry advisory `RateLimit-*` headers, attached in middleware so no route can forget them. Closes #63 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
richardmhope
force-pushed
the
m4-rate-limiting
branch
from
July 31, 2026 08:21
fe7b675 to
0401ef8
Compare
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.
Ceilings on what an unauthenticated caller can reach. These are not the last line of defence —
authentication is — so they are sized to stop floods rather than to meter usage.
/auth/login,/auth/callbackThe three decisions worth reviewing
Each alternative looks reasonable until you consider the failure mode:
that locks every operator out of the console when Redis hiccups has done more damage than the
traffic it was defending against. There is a test for this specifically.
a budget and throttle itself —
--scale engine=Nwould stop meaning anything. Tested byexhausting one engine's budget and showing another is unaffected from the same address.
credential-stuffing bucket be small without any risk to a healthy fleet, which never touches it.
X-Forwarded-For
Ignored unless
ICEBERG_TRUSTED_PROXY_HOPSsays how many proxies sit in front. The header iscaller-controlled: trusting it by default would let an attacker take a fresh identity per request,
which is the same as having no limit. Behind a load balancer this must be set, or every request
is charged to the balancer —
.env.exampleanddocs/security.mdboth say so in as many words.Mechanics
Fixed-window counters in Redis (INCR + EXPIRE), so a limit means the same thing across replicas;
in-memory is the fallback and is a real limit, just a per-replica one. Refusals answer
429withRetry-After; allowed requests carry advisoryRateLimit-Limit/-Remaining/-Reset, attached inmiddleware so no rate-limited route can forget them.
21 tests: window behaviour, key isolation, fail-open, Redis key expiry, all five
X-Forwarded-Forcases,enforcesemantics, and end-to-end refusal through the app on login,callback, bad engine tokens, and an over-budget engine.
make lint,make typeand the full suite pass.Closes #63
🤖 Generated with Claude Code