fix(rate-limit): key IPv6 rate buckets by /64 instead of /128 (#205)

src/middleware/rate-limit.ts

@@ -2,6 +2,7 @@ import type { Context, Next } from "hono";
 import { getConnInfo } from "@hono/node-server/conninfo";
 import { createLogger } from "@percolator/shared";
 import { getSharedStore } from "./shared-store.js";
+import { rateLimitKey } from "../utils/ip-key.js";
 
 const logger = createLogger("api:rate-limit");
 
@@ -78,8 +79,10 @@ export function readRateLimit() {
       return c.json({ error: "Bad request" }, 400);
     }
 
+    // #205: key IPv6 clients by /64, not by the full /128. `ip` itself stays
+    // the exact address so logs and error paths keep full fidelity.
     const bucket = await getSharedStore().incrementRateBucket(
-      `read:${ip}`,
+      `read:${rateLimitKey(ip)}`,
       WINDOW_MS,
       MAX_RATE_LIMIT_ENTRIES
     );
@@ -114,8 +117,9 @@ export function writeRateLimit() {
       return c.json({ error: "Bad request" }, 400);
     }
 
+    // #205: see readRateLimit — IPv6 buckets are keyed by /64.
     const bucket = await getSharedStore().incrementRateBucket(
-      `write:${ip}`,
+      `write:${rateLimitKey(ip)}`,
       WINDOW_MS,
       MAX_RATE_LIMIT_ENTRIES
     );

src/utils/ip-key.ts

@@ -0,0 +1,103 @@
+/**
+ * Shared client-IP bucket keys (#205).
+ *
+ * Rate limiting, the IP blocklist and the WS per-IP counters each had their own
+ * copy of `normalizeIp`, which only unwrapped `::ffff:`-mapped addresses and
+ * otherwise used the full 128-bit IPv6 address as the bucket key. A single host
+ * controlling a /64 — the standard cloud and residential allocation — could
+ * rotate source addresses to mint a fresh quota per request.
+ *
+ * IPv6 clients are therefore keyed by their /64 prefix. IPv4 behaviour is
+ * unchanged: the exact address stays the key.
+ *
+ * No new dependency: #205 suggested a vetted IP library, but adding one now
+ * would mean regenerating pnpm-lock.yaml, which is exactly the file broken by
+ * #232 (it pins @percolatorct/sdk to a local filesystem path). Instead the
+ * expansion below is validated in tests against Node's built-in
+ * `net.BlockList`, which does its own subnet masking and so acts as an
+ * independent oracle.
+ */
+import { isIPv4 } from "node:net";
+
+/** A /64 covers the first 4 of the 8 hextets. */
+const IPV6_PREFIX_GROUPS = 4;
+
+/** Drop a zone/scope id (`fe80::1%eth0`). */
+function stripZone(ip: string): string {
+  const i = ip.indexOf("%");
+  return i === -1 ? ip : ip.slice(0, i);
+}
+
+/**
+ * Expand an IPv6 literal into exactly 8 numeric hextets.
+ * Returns null when the input does not parse as IPv6.
+ */
+function expandIpv6(ip: string): number[] | null {
+  let addr = stripZone(ip);
+
+  // Embedded IPv4 tail (`::ffff:1.2.3.4`, `2001:db8::1.2.3.4`) → two hextets.
+  const lastColon = addr.lastIndexOf(":");
+  if (lastColon === -1) return null;
+  const tail = addr.slice(lastColon + 1);
+  if (tail.includes(".")) {
+    if (!isIPv4(tail)) return null;
+    const o = tail.split(".").map(Number);
+    const hi = (((o[0] << 8) | o[1]) >>> 0).toString(16);
+    const lo = (((o[2] << 8) | o[3]) >>> 0).toString(16);
+    addr = `${addr.slice(0, lastColon + 1)}${hi}:${lo}`;
+  }
+
+  const halves = addr.split("::");
+  if (halves.length > 2) return null;
+
+  const parseGroups = (s: string): number[] | null => {
+    if (s === "") return [];
+    const out: number[] = [];
+    for (const g of s.split(":")) {
+      if (g === "" || g.length > 4 || !/^[0-9a-fA-F]+$/.test(g)) return null;
+      out.push(parseInt(g, 16));
+    }
+    return out;
+  };
+
+  const head = parseGroups(halves[0] ?? "");
+  if (head === null) return null;
+
+  if (halves.length === 2) {
+    const rest = parseGroups(halves[1] ?? "");
+    if (rest === null) return null;
+    const fill = 8 - head.length - rest.length;
+    if (fill < 0) return null;
+    return [...head, ...new Array(fill).fill(0), ...rest];
+  }
+
+  return head.length === 8 ? head : null;
+}
+
+/**
+ * Bucket key for a client IP.
+ *
+ * - IPv4 (incl. `::ffff:`-mapped): the exact address, unchanged.
+ * - IPv6: the `/64` prefix, so address rotation inside one allocation shares
+ *   a single quota.
+ * - Unparseable input: the address verbatim. That is the pre-#205 behaviour —
+ *   a per-address bucket. Collapsing unparseable input into one shared key
+ *   would let a malformed value evict or throttle unrelated clients.
+ */
+export function rateLimitKey(ip: string): string {
+  const norm = ip.startsWith("::ffff:") ? ip.slice(7) : ip;
+  if (!norm.includes(":")) return norm; // IPv4 fast path
+
+  const groups = expandIpv6(norm);
+  if (!groups) return norm;
+
+  return (
+    groups
+      .slice(0, IPV6_PREFIX_GROUPS)
+      .map((g) => g.toString(16))
+      .join(":") + "::/64"
+  );
+}
+
+/** Exported for tests that cross-check expansion against `net.BlockList`. */
+export const __ipKeyInternals = { expandIpv6 };

tests/utils/ip-key.test.ts

@@ -0,0 +1,108 @@
+/**
+ * #205: IPv6 rate-limit buckets must key on /64, not /128.
+ *
+ * A host controlling a /64 (the standard cloud and residential allocation)
+ * could rotate source addresses to mint a fresh 100 req/min bucket per request.
+ *
+ * The expansion in ip-key.ts is hand-written (see the module header for why no
+ * new dependency), so it is cross-checked here against Node's built-in
+ * `net.BlockList`. BlockList masks host bits itself, so `addSubnet(a, 64)`
+ * followed by `check(b)` is an independent answer to "are a and b in the same
+ * /64?" — derived without reference to our implementation.
+ */
+import { describe, it, expect } from "vitest";
+import { BlockList, isIPv6 } from "node:net";
+import { rateLimitKey } from "../../src/utils/ip-key.js";
+
+const V6 = [
+  "2001:db8::1",
+  "2001:db8::dead:beef",
+  "2001:0db8:0000:0000:0000:0000:0000:0002",
+  "2001:db8:0:0:aaaa:bbbb:cccc:dddd",
+  "2001:db8:0:1::1",
+  "2001:db8:0:1:ffff::9",
+  "fe80::1",
+  "fe80:0:0:0::abcd",
+  "::1",
+  "::",
+  "2001:db8:85a3::8a2e:370:7334",
+  "2001:db8:85a3:0:0:8a2e:370:7335",
+  "2600:1f18:abcd:1234::5",
+  "2600:1f18:abcd:1234:9999::5",
+  "2001:db8::1.2.3.4",
+  "2001:db8::5.6.7.8",
+  "fd00::1",
+  "fd00:0:0:1::1",
+];
+
+/** Independent answer: are these two addresses in the same /64? */
+function sameSlash64(a: string, b: string): boolean {
+  const list = new BlockList();
+  list.addSubnet(a, 64, "ipv6");
+  return list.check(b, "ipv6");
+}
+
+describe("#205 rateLimitKey", () => {
+  it("agrees with net.BlockList on every address pair", () => {
+    const mismatches: string[] = [];
+    let checked = 0;
+
+    for (const a of V6) {
+      for (const b of V6) {
+        if (!isIPv6(a) || !isIPv6(b)) continue;
+        checked++;
+        const oracle = sameSlash64(a, b);
+        const mine = rateLimitKey(a) === rateLimitKey(b);
+        if (oracle !== mine) {
+          mismatches.push(`${a} vs ${b}: oracle=${oracle} mine=${mine}`);
+        }
+      }
+    }
+
+    expect(checked).toBeGreaterThan(300);
+    expect(mismatches).toEqual([]);
+  });
+
+  it("collapses rotation within one /64 to a single bucket (the bypass)", () => {
+    // What an attacker rotates through. Pre-fix each of these was its own
+    // /128 bucket and therefore its own full quota.
+    const rotated = [
+      "2001:db8::1",
+      "2001:db8::2",
+      "2001:db8::dead:beef",
+      "2001:db8:0:0:ffff:ffff:ffff:ffff",
+    ];
+    const keys = new Set(rotated.map(rateLimitKey));
+    expect(keys.size).toBe(1);
+  });
+
+  it("keeps distinct /64s in distinct buckets", () => {
+    expect(rateLimitKey("2001:db8:0:1::1")).not.toBe(rateLimitKey("2001:db8:0:2::1"));
+  });
+
+  it("leaves IPv4 keys exactly as-is", () => {
+    for (const v4 of ["1.2.3.4", "203.0.113.9", "10.0.0.1", "255.255.255.255"]) {
+      expect(rateLimitKey(v4)).toBe(v4);
+    }
+  });
+
+  it("unwraps IPv4-mapped IPv6 to the bare IPv4 address", () => {
+    expect(rateLimitKey("::ffff:1.2.3.4")).toBe("1.2.3.4");
+  });
+
+  it("keeps two different IPv4 addresses in different buckets", () => {
+    expect(rateLimitKey("1.2.3.4")).not.toBe(rateLimitKey("1.2.3.5"));
+  });
+
+  it("ignores a zone id when keying", () => {
+    expect(rateLimitKey("fe80::2%eth0")).toBe(rateLimitKey("fe80::2"));
+  });
+
+  it("falls back to the verbatim value for unparseable input", () => {
+    // Per-value bucket, matching pre-#205 behaviour. Collapsing malformed
+    // input into one shared key would let it throttle unrelated clients.
+    for (const junk of ["not-an-ip", "2001:db8:::1", "xyz::1"]) {
+      expect(rateLimitKey(junk)).toBe(junk);
+    }
+  });
+});