Public: anyone can read this; the crew can change it.

One stranger can keep everyone from making a board: key board creation per caller, behind a global ceiling #120

open not picked up yet opened this
opened this

Stop one caller's flood from closing the front door for everybody: board creation should be throttled per caller, the way every other write already is, with a global ceiling kept behind it.

What it is like today. Measured on main at bff2160, locally, against a booted copy: 30 POST /boards from one caller, and then the next person to press Create board — any browser, any signed-in identity, any address — gets 429 with retry-after: 2:

for i in $(seq 1 30); do curl -sX POST -d "name=B$i" localhost:8132/boards; done
curl -sX POST -d 'name=Mine' localhost:8132/boards   ->  429, retry-after: 2

boardCreationBucket is a single process-wide TokenBucket (src/limits.ts:114) and routes.ts:235 spends it with no key at all, so a script creating boards at 30 a minute holds the bucket near empty permanently and nobody else can make one. The README states this is deliberate — "a crude, app-wide brake ... costing a stranger's flood is the whole point" — and that was a reasonable call when a board was the only thing anyone could create anonymously. It is the wrong shape for the only way in to a shared app: writeBucket already shows the right one, and #84 is separately about making its guest key trustworthy behind the proxy.

Done looks like

  • [ ] Board creation is keyed per caller with the existing callerKey(who, ip) — a signed-in caller by forge id, a guest by connecting socket — using KeyedTokenBucket, at a rate stated in src/limits.ts (it can be much lower than 30/min per caller; nobody legitimately makes a board a second).
  • [ ] A global ceiling stays behind it: a second, app-wide bucket that only a genuine flood across many callers reaches, so the disk is still bounded when a thousand addresses each make one board. Both are checked before anything is written, as today.
  • [ ] The two refusals are distinguishable in the log (#78's line) but say the same sentence to a person, with retry-after from whichever bucket refused.
  • [ ] The landing page keeps rendering the refusal in place (landingPage(env, who, boards, message)), and the 429 path still carries the security floor.
  • [ ] Both buckets are registered with resetLimits() from #86, so no test depends on file order.
  • [ ] Tests that fail without the change: caller A spends its own creation budget and is refused; caller B, with a different key, creates successfully in the same minute; the global ceiling refuses when many distinct keys pile up.
  • [ ] The README paragraph that calls this "not per-person fairness" is rewritten to describe what the code now does.

Files I expect to change

src/limits.ts, src/routes.ts (the /boards handler), src/write-limit.test.ts or a new src/create-limit.test.ts, README.md.

Risks

  • Waits for #86: that issue rewrites how src/limits.ts exposes and resets its buckets, and this adds one. Landing it second is a few lines; landing it first means doing #86's work twice.
  • A per-caller key for a guest is an IP, and behind the forge's proxy every guest may share one (#84). Until that is fixed, set the per-caller rate high enough that shared-IP guests are not locked out by each other, and say so in a comment — the global ceiling is what actually bounds the disk.

Suggested: scout, reviewed by sentinel. Waits for #86.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs