The report relay has brakes, and they count nobody's address - #47
Merged
Conversation
ADDED: The KV binding the rate limiter reads its state from, which had been left commented out — the caps in `[vars]` were never reached without it, so the public endpoint had no brakes at all. ADDED: `rotate-rl-salt.sh`, which generates the key the per-IP counters are hashed under and hands it to Cloudflare without printing it or writing it down. ADDED: Tests for the whole-relay cap, the number of KV writes a report and a refusal each cost, the hashing, and the hourly pruning. CHANGED: An address is stored as `HMAC-SHA-256(RL_SALT, address)` truncated to 12 bytes, never as itself. - HMAC and not a plain digest because an IPv4 address is 32 bits: a hash of one is a lookup table away from being the address again, which would have looked like protection without being any. CHANGED: Both caps share one `rl:<date>` entry, so an accepted report costs one KV write and a refused one costs none. Entries older than an hour are dropped on the next write, which is also what bounds the value's size. CHANGED: The whole-relay cap is 600 reports a day, up from 300. - One write per report makes the cap the day's write budget directly. The free tier allows 1000, and the 400 left over are margin rather than slack: a limiter that cannot write fails open, and a KV read can be a minute stale, so both caps are approximate in the direction of letting too much through. CHANGED: `PRIVACY.md` describes the two counters and the keyed hash. It said the Worker stores nothing, which was true only while the limiter was switched off. REMOVED: `bump`, the two-key counter helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Review Notes
ADDED: The KV binding the rate limiter reads its state from, which had been left commented out — the caps in
[vars]were never reached without it, so the public endpoint had no brakes at all.ADDED:
rotate-rl-salt.sh, which generates the key the per-IP counters are hashed under and hands it to Cloudflare without printing it or writing it down.ADDED: Tests for the whole-relay cap, the number of KV writes a report and a refusal each cost, the hashing, and the hourly pruning.
CHANGED: An address is stored as
HMAC-SHA-256(RL_SALT, address)truncated to 12 bytes, never as itself.CHANGED: Both caps share one
rl:<date>entry, so an accepted report costs one KV write and a refused one costs none. Entries older than an hour are dropped on the next write, which is also what bounds the value's size.CHANGED: The whole-relay cap is 600 reports a day, up from 300.
CHANGED:
PRIVACY.mddescribes the two counters and the keyed hash. It said the Worker stores nothing, which was true only while the limiter was switched off.REMOVED:
bump, the two-key counter helper.