Skip to content
Go back

Garrul: The Audit Found My Rate Limiter

By SumGuy 14 min read
Garrul: The Audit Found My Rate Limiter
Contents

The Fix That Was Supposed to Be the End of It

The comment box at the bottom of this page is my own software. I wrote Garrul, a Cloudflare Workers-native comment system, because I didn’t want a VPS to babysit just to host a comment thread, and on 2026-08-01 it went through a full security audit. It came back with 2 critical findings, 5 high, and 14 medium. The part that actually stung: both criticals were the exact same root cause as a bug I’d already fixed two months earlier, in June, and one of the two live copies of that bug was sitting inside the rate limiter, the piece of code whose entire job is to stop abuse.

It took a paid outsider actually looking for it to find the second and third copies of a mistake I thought I’d already killed. No victory lap here: I didn’t catch these, an audit did. What’s worth walking through is why my first fix didn’t generalize, because the pattern (a wrong assumption about cost, copy pasted across a codebase without anyone noticing the copies) is not unique to me or to Cloudflare Workers.

Act One: The Bug That Was Supposed to Be Over

Back on 2026-06-07, Garrul’s comment tree cache and the /api/v1/counts endpoint both wrote through to Workers KV on every cache miss. That felt reasonable at the time. KV is the obvious place to cache things in a Worker.

The problem is the free tier’s write cap: 1,000 writes a day, and that limit is scoped to your entire Cloudflare account, across every namespace you have. Not per Worker. Not per site. Per account. So a crawler sweeping a popular post could drain that quota by lunchtime, and once it’s gone, it’s gone for everything else that touches KV too: sessions, rate limit counters, OAuth state, all of it, locked out until midnight UTC.

The fix, shipped the next day as v1.13.1, moved both read caches off KV and onto the Cache API (caches.default), which is free with no daily write cap at all. First-page cache TTL went to 60 seconds, the handling colo gets invalidated immediately on any write, other colos serve stale for at most that minute, and signed-in viewers bypass the cache entirely. KV got reserved for the things that actually need durability: sessions, rate limits, OAuth state, resolved settings.

I closed that ticket and moved on. That was the mistake. Not the original bug, the assumption that fixing one instance of “KV writes are free, right?” meant the assumption itself was gone.

Act Two: The Audit Finds the Other Two Copies

The 2026-08-01 audit found the same assumption sitting in two more places I hadn’t touched.

The first was the rate limiter itself. It wrote two KV rows per allowed request, and this is what turns mere inefficiency into a genuine DoS vector: a cold bucket always passes and always writes. About 500 source addresses were enough to drain the account-wide daily quota on their own, no coordination required, no actual flood of comments, just enough distinct callers hitting an endpoint that had never seen them before. The rate limiter wasn’t protecting the account from a denial of service. It was the denial of service.

The fix, in v2.0.0 on 2026-08-02, moved the limiter onto the Cache API behind a RateLimitStore interface, one operation per request instead of two, with per-route scoping and a shared global envelope so no single endpoint could spend the whole budget alone.

rate-limit-store.ts
export interface RateLimitStore {
// one read-modify-write per request, not two KV puts
check(key: string, limit: number, windowSeconds: number): Promise<{
allowed: boolean;
remaining: number;
}>;
}
// default backend: Cache API, free, no daily write cap,
// counters are scoped per Cloudflare colo
export class CacheApiRateLimitStore implements RateLimitStore {
async check(key: string, limit: number, windowSeconds: number) {
const cache = caches.default;
const cacheKey = new Request(`https://ratelimit.internal/${key}`);
const hit = await cache.match(cacheKey);
const count = hit ? Number(await hit.text()) : 0;
if (count >= limit) return { allowed: false, remaining: 0 };
await cache.put(
cacheKey,
new Response(String(count + 1), {
headers: { "cache-control": `max-age=${windowSeconds}` },
})
);
return { allowed: true, remaining: limit - count - 1 };
}
}

The documented tradeoff is that counters are now per-colo instead of global, so a distributed flood is undercounted relative to a true cross-datacenter limit. A Durable Object backend that closes both gaps shipped three days later as an opt-in (v2.3.0, issue #65, which finally closed #53). Set the RATE_LIMIT_DO binding and the counter moves into a Durable Object with a real compare-and-swap. Then the same joke lands a third time: Durable Object requests count against a 100k/day account-wide free-tier quota, including the requests you block. Under exactly the flood that backend exists to stop, you burn the quota and the limiter fails open until UTC midnight. The docs say to pair it with a Cloudflare WAF rule or stay on the Cache API.

The concurrency race in the Cache API limiter is not something a future patch fixes, because a read-modify-write whose write is a full overwrite cannot be made indivisible. That is precisely why the Durable Object backend is an opt-in and not the new default. Where a counter can live in D1 instead, atomicity comes free: v2.4.0 put a hard ceiling on outbound subscription-confirmation mail with a single UPDATE carrying the cap inside its own WHERE clause, making check-and-increment indivisible. Same class of problem, a storage layer that can actually express the constraint, one statement of SQL.

The second copy was worse, because nothing was even trying to guard it. GET /auth/:provider/start did an unconditional kv.put to stash OAuth state, no rate limit, no cookie, no token, no Origin check. About 1,000 requests in under 10 seconds took the whole account offline for KV writes. The fix made OAuth state a stateless HMAC payload signed with the existing JWT_SECRET, riding along in the per-flow cookie that was already there. Zero KV writes on start, because there was never a reason to write anything in the first place.

Act Three: The Thread That Ate Its Own Page

Buried in the same v2.0.0 release was an availability bug that had nothing to do with KV. Nothing checked reply depth anywhere in the codebase, and the comment tree walk was O(N squared). At depth 5,000 that walk took 223 milliseconds, against a 10 millisecond CPU budget on Cloudflare Workers.

You don’t need 5,000 replies to hit the wall. Around 700 chained comments was enough to blow the budget and return Error 1102 to every single reader of that page. Permanently. Not “until the next deploy,” permanently, because the response never completes, which means the edge cache never gets a chance to populate, which means the page can never self heal on its own. Every request just fails the same way forever, until someone ships code.

The tree walk is now O(N), iterative instead of recursive, with a cycle guard, and replies past depth 8 get rejected outright with a plain 400. Eight levels of nesting is already more than most readers will scroll through anyway; the widget flattens the display well before that.

Act Four: The Gates That Were Lying to Me

This is the part I’d tell you about even if nobody paid for an audit, because it’s not a Garrul specific lesson. While fixing the two criticals I went digging around the project’s own tooling and found three things that had been quietly useless for a long time.

tsconfig.json had include: test/**/*, pointing at a directory that has always actually been named tests/. One character off, and it meant no test file had ever been typechecked, not once, across every release up to that point. Fixing the typo surfaced 118 errors in one shot, including an ADMIN_ACTIONS list in a test fixture that had drifted down to 15 of the real 34 entries without anyone noticing, because nothing was checking it.

npm run lint was this:

package.json (before)
{
"scripts": {
"lint": "echo 'lint placeholder' && exit 0"
}
}

Every release up to that point had a green lint gate, by construction. It couldn’t fail. There were no lint rules being enforced at all, just a script that printed a sentence and returned success no matter what the code looked like. That’s now Biome, and it actually runs.

The CLI Disqus importer was the quietest failure of the three. It misparsed wrangler’s own response envelope, so every probe request came back read as “already exists,” and a completed import run printed DONE with zero counters incremented and zero rows written. If you ran that importer against an old Disqus export at any point before this release and it looked like it did nothing, that’s because it did nothing. Re-run it.

None of these three were in the audit report. They came from actually being in the code with fresh eyes, which is exactly the argument for treating an audit as a prompt to go looking rather than a checklist to close.

Everything Else the Audit Turned Up

The rest of the findings are less dramatic individually but worth a fast list, because a few of them are patterns you’ll recognize in your own code:

Supply Chain: Pinning What You Actually Trust

release.yml, the only workflow in the repo with contents: write, referenced a third-party GitHub Action by a mutable tag. A tag can be repointed by whoever controls that repository, at any time, and your CI would just run whatever showed up next. Every action reference is now pinned to a full commit SHA, with Dependabot keeping them current.

Releases also publish a SHA256SUMS file, echoed straight into the public job log before the publishing step runs, so you can compare what got built against what got signed for. I want to be precise about what that buys you, because it’s tempting to oversell: npm run upgrade builds from a git checkout of the tagged release, not from downloaded release assets. What it verifies is that the release-manifest.json inside that checkout matches the one the upgrade plan was generated from. That’s a consistency check at the last cheap point before migrations run, not a cryptographic signature, and the code and the docs both say so plainly.

The Eight Ways I Broke Things on Purpose

v2.0.0 shipped as a major version because eight breaking changes landed together, and three of them fail hard if you skip the prep work: IP_HASH_SECRET and JWT_SECRET are now enforced, and every route answers 500 without them; IP-keyed write endpoints require cf-connecting-ip or return 400; and the iframe embed only posts messages to origins listed in ALLOWED_ORIGINS, with the old document.referrer fallback gone entirely.

The other five are one-time blips rather than outages: the session cookie renamed to __Host-garrul_sess, so everyone signs out once; IPv6 ip_hash values change once because addresses now normalize to a /64 prefix before hashing; replies past depth 8 get rejected; two operator scripts got stricter about their inputs; and Node 24 became the hard minimum. You also need to run npm run rerender after deploying, because the sanitizer’s output changed and stored comment HTML doesn’t retroactively update itself.

What Two Months Actually Taught Me

Two lessons, both concrete enough to act on. First: a wrong assumption about cost doesn’t live in one function. “KV writes are basically free” was a belief baked into the code in more than one place, not a single bug, and beliefs get copy-pasted right along with the code that expresses them. You don’t find the other copies by staring harder at the function that already bit you. You find them by having someone else, or some other pass through the codebase, look for the belief itself rather than the specific line.

Second: a CI gate you’ve never actually verified is worse than having no gate at all, because it buys you confidence you never earned. A lint script that always exits 0 and a tsconfig.json typo that excludes your entire test suite both look, from the outside, exactly like a project with working tests and working lint. They aren’t. They’re a project with a passing badge and no idea what’s actually being checked.

The June fix felt like closing the book on a bad afternoon. It was act one of three. If you’re running a Workers project on the free tier, go check whether your rate limiter writes to KV on every request, whether your CI gates actually fail on anything, and whether your tsconfig.json include path matches a directory that actually exists. I would rather you find that out here than in your own audit findings.

If you want the other half of the same two months, the features rather than the wreckage, that got its own post too.

Common Questions

Is the Cloudflare Workers KV free-tier write limit really account-wide?

Yes. The 1,000 writes/day cap on Workers KV’s free tier applies across your entire Cloudflare account, not per namespace or per Worker. A single script that writes to KV on every cache miss can exhaust the quota for every other Worker on the account, including ones handling sessions or rate limits that have nothing to do with the offending script.

Can the Cache API replace Workers KV for rate limiting?

For most self-hosted installs, yes, with one tradeoff: the Cache API has no daily write cap and no cross-colo compare-and-swap, so counters are scoped per Cloudflare data center rather than globally. That undercounts a distributed flood. A Durable Object backend closes that gap but costs real Durable Object requests, which have their own free-tier ceiling.

What happens if I skip npm run rerender after a Garrul upgrade?

Existing comments keep rendering with their old, pre-upgrade HTML indefinitely. Garrul stores rendered comment HTML once at write time and serves it verbatim, so a sanitizer change only affects new comments until you run the rerender step, which regenerates stored HTML for every existing comment against the current renderer version.

Is a 10 millisecond CPU budget a real constraint on Cloudflare Workers?

Yes, for CPU time specifically, not wall clock time spent waiting on network calls. The free tier caps CPU time per request at 10 milliseconds of actual execution, so an O(N squared) algorithm that tests fine at small N can blow that budget once N grows, even if the request would otherwise finish in under a second.


Share this post on:

Send a Webmention

Written about this post on your own site? Send a webmention and it'll show up above once verified.


Previous Post
HyperDX vs OpenObserve
Next Post
Home Assistant Backups That Actually Restore

Discussion

Powered by Garrul . Sign in with GitHub or Google, or post anonymously.

Related Posts