fix(security): the rate-limit key was whatever the caller said it was

__tests__/unit/client-ip-key.test.ts

@@ -0,0 +1,56 @@
+/**
+ * clientIpKey — the limiter is only as honest as its notion of "who".
+ *
+ * Caddy APPENDS to X-Forwarded-For, so a request that reached us through it
+ * carries `<whatever the caller sent>, <what Caddy actually saw>`. Reading the
+ * first entry — which three payment routes did, while rate-limit.ts read the
+ * header whole — keys every limiter on a value the caller chooses.
+ *
+ * The consequence is not a weakened limiter. Vary the header per request and
+ * each request lands in a fresh bucket, so no bucket ever fills: no limiter at
+ * all, on routes that mint a real Lightning invoice through the recipient's
+ * wallet. bitbaum/orangecat#563 finding 2.
+ */
+
+import { clientIpKey } from '@/lib/client-ip';
+
+const req = (headers: Record<string, string>) =>
+  ({ headers: { get: (k: string) => headers[k.toLowerCase()] ?? null } }) as unknown as Request;
+
+describe('clientIpKey', () => {
+  it('returns the hop Caddy wrote, not the one the caller sent', () => {
+    expect(clientIpKey(req({ 'x-forwarded-for': '9.9.9.9, 203.0.113.7' }))).toBe('203.0.113.7');
+  });
+
+  it('gives one key for one client however the prefix is spoofed', () => {
+    // The bypass itself: three requests from the same client, three forged
+    // prefixes. Before the fix these produced three keys and no bucket filled.
+    const keys = new Set(
+      ['1.1.1.1', '2.2.2.2', 'not-even-an-ip'].map((spoof) =>
+        clientIpKey(req({ 'x-forwarded-for': `${spoof}, 203.0.113.7` })),
+      ),
+    );
+    expect([...keys]).toEqual(['203.0.113.7']);
+  });
+
+  it('handles the ordinary single-hop request', () => {
+    expect(clientIpKey(req({ 'x-forwarded-for': '203.0.113.7' }))).toBe('203.0.113.7');
+  });
+
+  it('tolerates the whitespace Caddy leaves after the comma', () => {
+    expect(clientIpKey(req({ 'x-forwarded-for': '9.9.9.9,   203.0.113.7  ' }))).toBe('203.0.113.7');
+  });
+
+  it('falls back to x-real-ip, then to one shared bucket', () => {
+    expect(clientIpKey(req({ 'x-real-ip': '8.8.8.8' }))).toBe('8.8.8.8');
+    expect(clientIpKey(req({}))).toBe('anonymous');
+  });
+
+  it('never returns an empty key from a malformed header', () => {
+    // An empty key would collapse every caller into one bucket silently, or
+    // worse, produce the key "l402:" and look like a real identity.
+    expect(clientIpKey(req({ 'x-forwarded-for': '' }))).toBe('anonymous');
+    expect(clientIpKey(req({ 'x-forwarded-for': ' , , ' }))).toBe('anonymous');
+    expect(clientIpKey(req({ 'x-forwarded-for': ',' , 'x-real-ip': '8.8.8.8' }))).toBe('8.8.8.8');
+  });
+});

package.json

@@ -37,9 +37,10 @@
     "check:schema-columns": "node scripts/check-schema-columns.mjs",
     "check:currency-units": "node scripts/check-currency-units.mjs",
     "check:rpc-exists": "node scripts/check-rpc-exists.mjs",
+    "check:client-ip": "node scripts/check-client-ip.mjs",
     "check:ai-models": "node scripts/check-ai-models.mjs",
     "check:mdx": "node scripts/check-mdx.mjs",
-    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:mdx && npm run test:unit -- --watchAll=false",
+    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:client-ip && npm run check:mdx && npm run test:unit -- --watchAll=false",
     "audit:schema": "node scripts/db/audit-schema-drift.mjs",
     "audit:routes": "node scripts/audit-routes.mjs",
     "gen:types": "bash scripts/db/gen-types.sh",

scripts/check-client-ip.mjs

@@ -0,0 +1,69 @@
+#!/usr/bin/env node
+/* eslint-disable no-console */
+/**
+ * check-client-ip.mjs — nothing parses `x-forwarded-for` except the one helper.
+ *
+ * `X-Forwarded-For` is a LIST, and Caddy APPENDS to it. A request that reached
+ * us through Caddy carries `<whatever the caller sent>, <what Caddy actually
+ * saw>`, so the only entry the caller cannot forge is the LAST one.
+ *
+ * Six places in this repo read it, and every one of them read it wrong:
+ *
+ *   - `rate-limit.ts` used the header WHOLE, so any value at all was a new key;
+ *   - three payment routes each copied `split(',')[0]` — the caller's own value;
+ *   - the entity audit log recorded that value, an audit trail the subject
+ *     writes;
+ *   - the captcha route forwarded it to the provider as `remoteip`.
+ *
+ * The limiter consequence is the sharp one: vary the header per request and
+ * every request lands in a fresh bucket, so no bucket ever fills. That is not a
+ * weakened limiter, it is no limiter, while the route reads as protected — and
+ * on `/api/v1/pay/...` and `POST /api/v1/payments/public` each such request
+ * mints a real Lightning invoice through the recipient's own wallet, which is
+ * how you get a seller rate-limited by their wallet provider.
+ *
+ * Six instances is far past the point where fixing them one more time is the
+ * answer. `clientIpKey()` in src/lib/rate-limit.ts is the single definition;
+ * this gate keeps it single. See bitbaum/orangecat#563 finding 2, and
+ * `limitkit@0.2.0`, which carried the identical bug for the same reason.
+ */
+
+import { readFileSync } from 'node:fs';
+import { execSync } from 'node:child_process';
+
+// The one file allowed to look at the raw header: the definition itself.
+const OWNER = 'src/lib/client-ip.ts';
+
+// Any read of the header. Deliberately broad — the failure was never one
+// spelling, it was six people each reaching for the header directly.
+const PATTERN = /['"`]x-forwarded-for['"`]/i;
+
+const files = execSync(
+  "git ls-files 'src/**/*.ts' 'src/**/*.tsx'",
+  { encoding: 'utf8' },
+)
+  .split('\n')
+  .filter(Boolean)
+  .filter((f) => f !== OWNER);
+
+const offenders = [];
+for (const file of files) {
+  const lines = readFileSync(file, 'utf8').split('\n');
+  lines.forEach((line, i) => {
+    if (PATTERN.test(line)) offenders.push(`${file}:${i + 1}: ${line.trim()}`);
+  });
+}
+
+if (offenders.length > 0) {
+  console.error('✗ x-forwarded-for is read outside the one helper that knows which hop to trust:');
+  for (const o of offenders) console.error(`    ${o}`);
+  console.error('');
+  console.error(`  Caddy APPENDS to that header, so the first entry is whatever the caller`);
+  console.error(`  sent and only the last one is evidence. Use clientIpKey(request) from`);
+  console.error(`  ${OWNER} — it counts from the right and falls back honestly.`);
+  console.error('');
+  console.error('  A limiter keyed on a caller-controlled value can never be tripped.');
+  process.exit(1);
+}
+
+console.log(`✓ client IP: ${files.length} file(s) checked, x-forwarded-for read only in ${OWNER}`);

src/app/api/auth/verify-captcha/route.ts

@@ -3,6 +3,7 @@ import { verifyCaptchaToken } from '@/lib/captcha';
 import { logger } from '@/utils/logger';
 import { apiSuccess, apiBadRequest, apiInternalError } from '@/lib/api/standardResponse';
 import { rateLimit, createRateLimitResponse } from '@/lib/rate-limit';
+import { clientIpOrUndefined } from '@/lib/client-ip';
 
 /**
  * POST /api/auth/verify-captcha
@@ -32,9 +33,10 @@ export async function POST(request: NextRequest) {
       return apiBadRequest('CAPTCHA token is required');
     }
 
-    // Get client IP for additional validation
-    const forwardedFor = request.headers.get('x-forwarded-for');
-    const remoteIp = forwardedFor?.split(',')[0]?.trim();
+    // Get client IP for additional validation — the hop Caddy wrote, not the
+    // caller-supplied first one, or the provider's IP heuristics are being fed
+    // whatever the caller chose.
+    const remoteIp = clientIpOrUndefined(request);
 
     const result = await verifyCaptchaToken(token, remoteIp);
 

src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts

@@ -12,6 +12,7 @@ import { publicSupportCreateSchema } from '@/lib/validation/finance';
 import { createPublicClient } from '@/lib/supabase/public';
 import { rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit';
 import { logger } from '@/utils/logger';
+import { clientIpKey } from '@/lib/client-ip';
 
 /**
  * GET /api/v1/pay/{entity_type}/{entity_id}?amount_btc=X — HTTP 402 inline payment.
@@ -31,8 +32,7 @@ import { logger } from '@/utils/logger';
  */
 
 function requestKey(request: Request): string {
-  const forwarded = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim();
-  return `l402:${forwarded || request.headers.get('x-real-ip') || 'anonymous'}`;
+  return `l402:${clientIpKey(request)}`;
 }
 
 export async function GET(

src/app/api/v1/payments/public/[id]/route.ts

@@ -11,11 +11,11 @@ import {
 import { publicPaymentActionSchema } from '@/lib/validation/finance';
 import { validateUUID, getValidationError } from '@/lib/api/validation';
 import { rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit';
+import { clientIpKey } from '@/lib/client-ip';
 
 // Same per-IP keying as ../route.ts — anonymous callers, so IP is the only handle.
 function requestKey(request: Request): string {
-  const forwarded = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim();
-  return `public-payment-action:${forwarded || request.headers.get('x-real-ip') || 'anonymous'}`;
+  return `public-payment-action:${clientIpKey(request)}`;
 }
 
 function readToken(request: Request): string | null {

src/app/api/v1/payments/public/route.ts

@@ -9,10 +9,10 @@ import { publicSupportCreateSchema } from '@/lib/validation/finance';
 import { createPublicClient } from '@/lib/supabase/public';
 import { rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit';
 import { logger } from '@/utils/logger';
+import { clientIpKey } from '@/lib/client-ip';
 
 function requestKey(request: Request): string {
-  const forwarded = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim();
-  return `public-support:${forwarded || request.headers.get('x-real-ip') || 'anonymous'}`;
+  return `public-support:${clientIpKey(request)}`;
 }
 
 export async function POST(request: Request) {

src/lib/api/entityPostHandler.ts

@@ -57,6 +57,7 @@ import {
 } from '@/services/idempotency/idempotencyResults';
 import { enqueueWebhookEvent } from '@/services/webhooks/deliveryService';
 import { auditLog, AUDIT_ACTIONS } from '@/lib/api/auditLog';
+import { clientIpOrUndefined } from '@/lib/client-ip';
 
 // Type for the awaited Supabase client
 type SupabaseClient = Awaited<ReturnType<typeof createServerClient>>;
@@ -334,7 +335,9 @@ export function createEntityPostHandler(config: EntityPostHandlerConfig) {
           entityType,
           entityId,
           metadata: { actorId: resolvedActor.id, source: auth.source },
-          ipAddress: request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || undefined,
+          // The hop Caddy wrote. The first hop is caller-supplied, so recording
+          // it let anyone choose what the audit trail said about them.
+          ipAddress: clientIpOrUndefined(request),
           userAgent: request.headers.get('user-agent') || undefined,
         });
       };

src/lib/client-ip.ts

@@ -0,0 +1,65 @@
+/**
+ * Who is calling — the one definition, and the one place that reads
+ * `x-forwarded-for`.
+ *
+ * WHICH HOP, AND WHY NOT THE FIRST
+ *
+ * `X-Forwarded-For` is a list, and Caddy APPENDS to it. A request that arrived
+ * through Caddy carries `<whatever the caller sent>, <what Caddy actually
+ * saw>`, so the only entry the caller cannot forge is the LAST one.
+ *
+ * Six places here read that header and every one read it wrong: `rate-limit.ts`
+ * used it whole, so any value at all was a fresh key; three payment routes each
+ * copied `split(',')[0]`, the caller's own value; the entity audit log recorded
+ * that value, making the trail writable by its subject; and the captcha route
+ * forwarded it to the provider as `remoteip`.
+ *
+ * The limiter consequence is the sharp one. Vary the header per request and
+ * every request lands in a fresh bucket, so no bucket ever fills — not a
+ * weakened limiter but no limiter, on a route that reads as protected. On
+ * `GET /api/v1/pay/...` and `POST /api/v1/payments/public` each such request
+ * mints a real Lightning invoice through the recipient's own wallet, which is
+ * how an attacker gets a seller throttled or banned by their wallet provider.
+ *
+ * Verified rather than assumed, 2026-08-28: orangecat.ch resolves straight to
+ * the box, no CDN in front (`via: 1.1 Caddy`), one Caddy 2.11.4 with no
+ * `trusted_proxies` configured — exactly one appended hop.
+ *
+ * Its own module, not a corner of `rate-limit.ts`, so the audit log and the
+ * captcha route can ask who is calling without importing an Upstash client.
+ *
+ * See bitbaum/orangecat#563 finding 2. `limitkit@0.2.0` ships the same
+ * correction as `clientIp()`; adopting it here is ADR-0002's remaining work.
+ */
+
+type HeadersLike = { get(name: string): string | null };
+type RequestLike = { headers: HeadersLike };
+
+/**
+ * The caller's IP as seen by Caddy, or a shared `anonymous` bucket.
+ *
+ * The fallback throttles unattributable traffic collectively rather than
+ * letting it past, and is never empty — an empty key would read as a real
+ * identity in `l402:${key}` while collapsing every caller into one bucket.
+ */
+export function clientIpKey(request: RequestLike): string {
+  const hops = (request.headers.get('x-forwarded-for') ?? '')
+    .split(',')
+    .map((hop) => hop.trim())
+    .filter(Boolean);
+
+  // One trusted proxy (Caddy), so the rightmost hop is the one it wrote.
+  if (hops.length > 0) return hops[hops.length - 1]!;
+
+  return request.headers.get('x-real-ip')?.trim() || 'anonymous';
+}
+
+/**
+ * The same answer where an ADDRESS is wanted rather than a bucket key —
+ * an audit row or a captcha `remoteip` wants nothing at all rather than the
+ * word "anonymous", which is a limiter concept and not a place.
+ */
+export function clientIpOrUndefined(request: RequestLike): string | undefined {
+  const key = clientIpKey(request);
+  return key === 'anonymous' ? undefined : key;
+}

src/lib/rate-limit.ts

@@ -17,6 +17,7 @@
 import { Ratelimit } from '@upstash/ratelimit';
 import { Redis } from '@upstash/redis';
 import { logger } from '@/utils/logger';
+import { clientIpKey } from '@/lib/client-ip';
 
 // ==================== TYPES ====================
 
@@ -176,14 +177,9 @@ const fallbackDomainSearchLimiter = new InMemoryRateLimiter({
 
 // ==================== RATE LIMIT FUNCTIONS ====================
 
-/**
- * The caller's IP, as seen through Caddy.
- *
- * One definition, because a per-IP limiter is only as correct as its notion of
- * "IP" — and three limiters had already copied these two lines verbatim.
- */
+/** The caller's IP — defined once in ./client-ip, re-exported for callers. */
 function clientIp(request: RequestLike): string {
-  return request.headers.get('x-forwarded-for') || request.headers.get('x-real-ip') || 'anonymous';
+  return clientIpKey(request);
 }
 
 /**
@@ -507,3 +503,5 @@ export function applyRateLimitHeaders<T extends Response>(response: T, result: R
   }
   return response;
 }
+
+export { clientIpKey };