Rate limit POST, PUT and DELETE on /bins per client IP

docs/TODO.md

@@ -59,9 +59,9 @@ Do these in order. Each task is a thin end-to-end slice: test first, implement,
 - [x] Tests: each TTL rule, invalid TTLs, a lazy 404 with a faked clock, the sweep deleting only expired rows
 
 ## 8. Rate limiting
-- [ ] Middleware applies `CREATE_LIMITER` to POST and `WRITE_LIMITER` to PUT/DELETE, keyed on `CF-Connecting-IP`
-- [ ] Over the limit → `429 rate_limited` + `Retry-After`; `RATE_LIMIT_ENABLED=false` skips the check
-- [ ] Tests: use a stub limiter binding to check both branches and the disabled flag
+- [x] Middleware applies `CREATE_LIMITER` to POST and `WRITE_LIMITER` to PUT/DELETE, keyed on `CF-Connecting-IP`
+- [x] Over the limit → `429 rate_limited` + `Retry-After`; `RATE_LIMIT_ENABLED=false` skips the check
+- [x] Tests: use a stub limiter binding to check both branches and the disabled flag
 
 ## 9. CORS
 - [ ] Hono `cors()` driven by `CORS_ORIGINS` (`*` or an allowlist); methods, allowed headers and exposed headers as in api.md

docs/api.md

@@ -78,7 +78,7 @@ Browsers can read all of these via `Access-Control-Expose-Headers`, along with `
 
 ## Validation and errors
 
-Check in this order: route/id → auth (non-create routes) → `X-Bin-TTL` (create only) → content type → size → JSON parse → top-level type. `X-Bin-TTL` is a header, so a bad one is rejected before any of the body is read.
+Check in this order: rate limit (`POST`, `PUT`, `DELETE`) → route/id → auth (non-create routes) → `X-Bin-TTL` (create only) → content type → size → JSON parse → top-level type. The rate limit comes first because it protects the Worker: a limited client gets no D1 lookup, key check or body parsing. `X-Bin-TTL` is a header, so a bad one is rejected before any of the body is read.
 
 | Status | `code` | When |
 |---|---|---|
@@ -117,6 +117,9 @@ Two Workers Rate Limiting bindings, keyed on `CF-Connecting-IP`:
 
 Reads have no limit in the MVP. Setting `RATE_LIMIT_ENABLED=false` turns both off. The binding's period can only be 10 s or 60 s.
 
+- Over the limit → `429 rate_limited` with `Retry-After: <seconds>`. A binding can't report when its window resets, so `Retry-After` is the binding's whole period (60 s).
+- Requests without `CF-Connecting-IP` (Cloudflare always sends it) share a single bucket, keyed `unknown`, rather than going unlimited.
+
 ## CORS
 
 `CORS_ORIGINS` is `*` (the default) or a comma-separated allowlist. Allowed methods: `GET, POST, PUT, DELETE, OPTIONS`. Allowed headers: `Authorization, Content-Type, X-Bin-Private, X-Bin-TTL`. The docs must warn that a Write Key embedded in frontend code gives every visitor write access.

src/bins/rate-limit.ts

@@ -0,0 +1,36 @@
+import { createMiddleware } from "hono/factory";
+import type { AppEnv } from "../context";
+import { errorResponse } from "../errors";
+
+/**
+ * The period of both limiter bindings, in seconds: keep it equal to `ratelimits[].simple.period` in wrangler.jsonc.
+ * A binding can't report when its window resets, so `Retry-After` sends the whole period.
+ */
+export const RATE_LIMIT_PERIOD_SECONDS = 60;
+
+/**
+ * The limiter key for a request without `CF-Connecting-IP` (Cloudflare always sets it, so only local or unusual
+ * traffic lacks it). All such requests share this one bucket rather than going unlimited.
+ */
+export const MISSING_IP_KEY = "unknown";
+
+/** The rate limit bindings in wrangler.jsonc. */
+export type LimiterBinding = "CREATE_LIMITER" | "WRITE_LIMITER";
+
+/**
+ * Checks `binding` for the client's IP and answers `429 rate_limited` with `Retry-After` when it is over the limit
+ * (docs/api.md → Rate limiting). Needs `withConfig` first. With `RATE_LIMIT_ENABLED=false` the limiter isn't called.
+ */
+export function rateLimit(binding: LimiterBinding) {
+  return createMiddleware<AppEnv>(async (c, next) => {
+    if (c.var.config.rateLimitEnabled) {
+      const key = c.req.header("CF-Connecting-IP")?.trim() || MISSING_IP_KEY;
+      const { success } = await c.env[binding].limit({ key });
+      if (!success) {
+        c.header("Retry-After", String(RATE_LIMIT_PERIOD_SECONDS));
+        return errorResponse(c, 429, "rate_limited", "Too many requests: try again later");
+      }
+    }
+    await next();
+  });
+}

src/bins/routes.ts

@@ -6,12 +6,18 @@ import { parseBinBody } from "./body";
 import { resolveExpiresAt } from "./expiry";
 import { generateBinId, generateReadKey, generateWriteKey, hashKey, isValidBinId } from "./keys";
 import { binMetadataHeaders, toIso } from "./metadata";
+import { rateLimit } from "./rate-limit";
 import { type Bin, findLive, insert, remove, replaceContent } from "./repository";
 
 export const binRoutes = new OpenAPIHono<AppEnv>();
 
 binRoutes.use(withConfig);
 
+// Rate limiting comes first, before the id, auth and body checks (docs/api.md → Validation and errors): it protects
+// the Worker, so a limited client shouldn't get D1 lookups, key hashing or body parsing. Reads aren't limited.
+binRoutes.post("/", rateLimit("CREATE_LIMITER"));
+binRoutes.on(["PUT", "DELETE"], "/:id", rateLimit("WRITE_LIMITER"));
+
 binRoutes.post("/", async (c) => {
   const now = Date.now();
   // X-Bin-TTL is checked before the body (docs/api.md → Validation and errors): it is a header, so a bad one is

test/rate-limit.test.ts

@@ -0,0 +1,243 @@
+import { createExecutionContext, waitOnExecutionContext } from "cloudflare:test";
+import { env } from "cloudflare:workers";
+import { beforeEach, describe, expect, it } from "vitest";
+import { generateBinId, generateWriteKey, hashKey } from "../src/bins/keys";
+import { MISSING_IP_KEY, RATE_LIMIT_PERIOD_SECONDS } from "../src/bins/rate-limit";
+import { type Bin, findLive, insert } from "../src/bins/repository";
+import { app } from "../src/index";
+
+const BASE_URL = "https://keepjson.test";
+const CLIENT_IP = "203.0.113.7";
+
+/** A limiter binding that allows or denies every call and remembers the keys it was called with. */
+type StubLimiter = RateLimit & { keys: string[] };
+
+function stubLimiter(success: boolean): StubLimiter {
+  const keys: string[] = [];
+  return {
+    keys,
+    async limit({ key }) {
+      keys.push(key);
+      return { success };
+    },
+  };
+}
+
+type Limiters = { create: StubLimiter; write: StubLimiter };
+
+function limiters(success: boolean): Limiters {
+  return { create: stubLimiter(success), write: stubLimiter(success) };
+}
+
+/**
+ * Calls the app directly, so the limiter bindings can be stubs and rate limiting can be turned on: the test worker
+ * itself runs with RATE_LIMIT_ENABLED=false (vitest.config.ts).
+ */
+async function send(
+  stubs: Limiters,
+  path: string,
+  init: RequestInit = {},
+  options: { enabled?: boolean; ip?: string | null } = {},
+): Promise<Response> {
+  const headers = new Headers(init.headers);
+  const ip = options.ip === undefined ? CLIENT_IP : options.ip;
+  if (ip !== null) headers.set("CF-Connecting-IP", ip);
+  const testEnv: Env = {
+    ...env,
+    RATE_LIMIT_ENABLED: String(options.enabled ?? true),
+    CREATE_LIMITER: stubs.create,
+    WRITE_LIMITER: stubs.write,
+  };
+  const ctx = createExecutionContext();
+  const res = await app.fetch(new Request(`${BASE_URL}${path}`, { ...init, headers }), testEnv, ctx);
+  await waitOnExecutionContext(ctx);
+  return res;
+}
+
+function postBin(stubs: Limiters, body = '{"keep": true}', options?: { enabled?: boolean; ip?: string | null }) {
+  return send(stubs, "/bins", { method: "POST", headers: { "Content-Type": "application/json" }, body }, options);
+}
+
+function putBin(stubs: Limiters, id: string, writeKey: string, body = '{"replaced": true}') {
+  return send(stubs, `/bins/${id}`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Authorization: `Bearer ${writeKey}` },
+    body,
+  });
+}
+
+function deleteBin(stubs: Limiters, id: string, writeKey: string) {
+  return send(stubs, `/bins/${id}`, { method: "DELETE", headers: { Authorization: `Bearer ${writeKey}` } });
+}
+
+/** Inserts a public Bin through the repository, returning it with its plaintext Write Key. */
+async function storeBin(): Promise<{ bin: Bin; writeKey: string }> {
+  const writeKey = generateWriteKey();
+  const now = Date.now();
+  const bin: Bin = {
+    id: generateBinId(),
+    content: '{"keep": true}',
+    isPrivate: false,
+    writeKeyHash: await hashKey(writeKey),
+    readKeyHash: null,
+    createdAt: now,
+    updatedAt: now,
+    expiresAt: null,
+  };
+  await insert(env.DB, bin);
+  return { bin, writeKey };
+}
+
+async function countBins(): Promise<number> {
+  const row = await env.DB.prepare("SELECT COUNT(*) AS n FROM bins").first<{ n: number }>();
+  return row?.n ?? 0;
+}
+
+async function expectRateLimited(res: Response) {
+  expect(res.status).toBe(429);
+  expect(res.headers.get("Retry-After")).toBe(String(RATE_LIMIT_PERIOD_SECONDS));
+  expect(res.headers.get("Cache-Control")).toBe("no-store");
+  expect(res.headers.has("X-Bin-Id")).toBe(false);
+  expect(await res.json()).toEqual({ error: { code: "rate_limited", message: expect.any(String) } });
+}
+
+beforeEach(async () => {
+  await env.DB.exec("DELETE FROM bins");
+});
+
+describe("the test worker", () => {
+  it("runs with rate limiting turned off, so other test files can create many Bins", () => {
+    expect(env.RATE_LIMIT_ENABLED).toBe("false");
+  });
+});
+
+describe("rate limiting", () => {
+  it("sends the Retry-After period of the limiter bindings in wrangler.jsonc", () => {
+    expect(RATE_LIMIT_PERIOD_SECONDS).toBe(60);
+  });
+
+  describe("under the limit", () => {
+    it("creates a Bin, checking CREATE_LIMITER with the client's IP", async () => {
+      const stubs = limiters(true);
+      const res = await postBin(stubs);
+      expect(res.status).toBe(201);
+      expect(stubs.create.keys).toEqual([CLIENT_IP]);
+      expect(stubs.write.keys).toEqual([]);
+      expect(await countBins()).toBe(1);
+    });
+
+    it("replaces a Bin, checking WRITE_LIMITER with the client's IP", async () => {
+      const stubs = limiters(true);
+      const { bin, writeKey } = await storeBin();
+      const res = await putBin(stubs, bin.id, writeKey);
+      expect(res.status).toBe(200);
+      expect(stubs.write.keys).toEqual([CLIENT_IP]);
+      expect(stubs.create.keys).toEqual([]);
+      expect((await findLive(env.DB, bin.id, Date.now()))?.content).toBe('{"replaced": true}');
+    });
+
+    it("deletes a Bin, checking WRITE_LIMITER with the client's IP", async () => {
+      const stubs = limiters(true);
+      const { bin, writeKey } = await storeBin();
+      const res = await deleteBin(stubs, bin.id, writeKey);
+      expect(res.status).toBe(204);
+      expect(stubs.write.keys).toEqual([CLIENT_IP]);
+      expect(stubs.create.keys).toEqual([]);
+      expect(await findLive(env.DB, bin.id, 0)).toBeNull();
+    });
+
+    it("uses one shared fallback key when CF-Connecting-IP is missing", async () => {
+      const stubs = limiters(true);
+      const res = await postBin(stubs, "{}", { ip: null });
+      expect(res.status).toBe(201);
+      expect(stubs.create.keys).toEqual([MISSING_IP_KEY]);
+    });
+  });
+
+  describe("over the limit", () => {
+    it("rejects POST /bins with 429 rate_limited and creates nothing", async () => {
+      const stubs = limiters(false);
+      await expectRateLimited(await postBin(stubs));
+      expect(stubs.create.keys).toEqual([CLIENT_IP]);
+      expect(await countBins()).toBe(0);
+    });
+
+    it("rejects PUT /bins/:id with 429 rate_limited and leaves the Bin unchanged", async () => {
+      const stubs = limiters(false);
+      const { bin, writeKey } = await storeBin();
+      await expectRateLimited(await putBin(stubs, bin.id, writeKey));
+      expect(stubs.write.keys).toEqual([CLIENT_IP]);
+      expect(await findLive(env.DB, bin.id, Date.now())).toEqual(bin);
+    });
+
+    it("rejects DELETE /bins/:id with 429 rate_limited and keeps the Bin", async () => {
+      const stubs = limiters(false);
+      const { bin, writeKey } = await storeBin();
+      await expectRateLimited(await deleteBin(stubs, bin.id, writeKey));
+      expect(stubs.write.keys).toEqual([CLIENT_IP]);
+      expect(await findLive(env.DB, bin.id, Date.now())).toEqual(bin);
+    });
+
+    it("checks the limit before the body is validated", async () => {
+      const stubs = limiters(false);
+      await expectRateLimited(await postBin(stubs, "not json"));
+      await expectRateLimited(
+        await send(stubs, "/bins", { method: "POST", headers: { "Content-Type": "text/plain" }, body: "{}" }),
+      );
+      await expectRateLimited(
+        await send(stubs, "/bins", {
+          method: "POST",
+          headers: { "Content-Type": "application/json", "X-Bin-TTL": "-1" },
+          body: "{}",
+        }),
+      );
+    });
+
+    it("checks the limit before the id and the Write Key", async () => {
+      const stubs = limiters(false);
+      const { bin } = await storeBin();
+      await expectRateLimited(await putBin(stubs, generateBinId(), generateWriteKey()));
+      await expectRateLimited(await deleteBin(stubs, "not-an-id", generateWriteKey()));
+      await expectRateLimited(await send(stubs, `/bins/${bin.id}`, { method: "DELETE" }));
+      expect(await countBins()).toBe(1);
+    });
+
+    it("never limits GET /bins/:id or GET /health", async () => {
+      const stubs = limiters(false);
+      const { bin } = await storeBin();
+      expect((await send(stubs, `/bins/${bin.id}`)).status).toBe(200);
+      expect((await send(stubs, `/bins/${generateBinId()}`)).status).toBe(404);
+      expect((await send(stubs, "/health")).status).toBe(200);
+      expect(stubs.create.keys).toEqual([]);
+      expect(stubs.write.keys).toEqual([]);
+    });
+  });
+
+  describe("with RATE_LIMIT_ENABLED=false", () => {
+    it("never calls a limiter, even one that would deny", async () => {
+      const stubs = limiters(false);
+      expect((await postBin(stubs, "{}", { enabled: false })).status).toBe(201);
+      const { bin, writeKey } = await storeBin();
+      const put = await send(
+        stubs,
+        `/bins/${bin.id}`,
+        {
+          method: "PUT",
+          headers: { "Content-Type": "application/json", Authorization: `Bearer ${writeKey}` },
+          body: "[]",
+        },
+        { enabled: false },
+      );
+      expect(put.status).toBe(200);
+      const del = await send(
+        stubs,
+        `/bins/${bin.id}`,
+        { method: "DELETE", headers: { Authorization: `Bearer ${writeKey}` } },
+        { enabled: false },
+      );
+      expect(del.status).toBe(204);
+      expect(stubs.create.keys).toEqual([]);
+      expect(stubs.write.keys).toEqual([]);
+    });
+  });
+});

vitest.config.ts

@@ -5,7 +5,15 @@ export default defineConfig({
   plugins: [
     cloudflareTest(async () => ({
       wrangler: { configPath: "./wrangler.jsonc" },
-      miniflare: { bindings: { TEST_MIGRATIONS: await readD1Migrations("./migrations") } },
+      miniflare: {
+        bindings: {
+          TEST_MIGRATIONS: await readD1Migrations("./migrations"),
+          // These bindings override the wrangler.jsonc vars. Rate limiting is off for the test worker: the real
+          // CREATE_LIMITER (10 per 60 s) would start rejecting the many Bins the tests create from one client.
+          // test/rate-limit.test.ts turns it back on with stub limiter bindings.
+          RATE_LIMIT_ENABLED: "false",
+        },
+      },
     })),
   ],
   test: { setupFiles: ["./test/apply-migrations.ts"] },