[MustafaYamin]

Fixes #216

What changed

Sign-in attempts shared one global counter, so anyone who could reach the API could use up the 30 attempts per minute and block the owner's correct key.

POST /api/session now uses the existing rateLimit middleware with a limit of 30 attempts per minute per client. The sign-in error message is unchanged, and sign-out (DELETE /api/session) is not limited by it. Access keys must be at least 24 characters, so per-client counting should not materially raise brute-force risk.

Verification

• Added a regression test in tests/rate-limit.test.ts: one client uses up its attempts, a second client signs in with the correct key, and the first client can still sign out. It failed with 429 before the fix and passes after.
• pnpm test on Windows 11 with Node 26.12: 518 passed, 3 skipped, 1 failed out of 522. The failure is worker recovery preserves downloads when metadata cannot be inspected, a Windows symlink permission error (EPERM) in a test unrelated to this change.
• pnpm typecheck, pnpm --dir apps/worker typecheck and pnpm build:server pass.
• Biome passes on the three changed files. Full pnpm lint fails locally on formatting in unchanged files, which still have CRLF line endings in my Windows checkout.
• Not run: build:web, build:ios and build:android. No mobile code changed.

Integration limits

Clients are only told apart when the server can see their real address. Behind a reverse proxy, without TRUST_PROXY, all clients may still share one limit.

I did not set TRUST_PROXY in render.yaml. Community reports suggest Render keeps caller-supplied X-Forwarded-For values, and resolveClientKey trusts the first entry, so enabling it there could allow spoofing. I have not verified this on a deployed instance, and a Render-specific fix may need a follow-up.

[caspianchan31]

Read through the diff and the current helper before reviewing. The switch to the shared limiter is clean, and the test covers exactly the regression: 30 failures from one client 429 that client, a second client with the correct key still gets 200, and DELETE sign-out stays open. A few non-blocking points; the first is the tradeoff worth signing off on:

1. Per-client keys remove the lockout but multiply the brute-force budget. The old counter was a global 30/min on the endpoint; this is 30/min per source, so aggregate guesses scale with the number of sources — rotating IPv6, proxy pools, or CGNAT egress each carry their own 30. Against an access-key endpoint that is a real weakening, even with a high-entropy key. Worth keeping both limits: the loose 30/min per client plus a low global circuit breaker on failed (401) sign-ins, with a successful owner sign-in passing or resetting the global gate. That keeps the owner unlockable without letting throughput scale with attacker IPs.
2. Counting only failures would sharpen it. The middleware runs before the handler, so every POST counts — including the owner's successful sign-in and malformed bodies. Behind a shared NAT (carrier or corporate egress), 30 unrelated sign-in POSTs from one IPv4 can 429 the owner, which is the lockout returning through address sharing. Incrementing after the handler when the response is 401 would count only actual guesses.
3. X-Forwarded-For under trustProxy is the bypass to document. resolveClientKey takes the leftmost XFF entry. With trustProxy=true behind an edge that appends rather than overwrites, that entry is caller-supplied (random, realip), so one client mints unlimited buckets and the per-IP limit evaporates. The existing spoof test covers trustProxy=false; the auth deployment needs the precondition stated (edge must strip or overwrite XFF), or a trusted hop counted a fixed number in from the right.
4. Store saturation fails closed. At maxEntries (10k), take() returns false for unknown keys, so a flood of one-request unique sources 429s an owner signing in from a new address — the same lockout this removes. Failing open at cap for this route and leaning on point 1's global breaker closes that path.
   Minor: fixed 60s windows allow a 30+30 burst across a boundary; fine if point 1 lands.

[MustafaYamin]

Thanks for the review. I checked each point against the code:

1. Global breaker: I agree per-client limits raise total guess capacity. The catch is the breaker itself. If a correct key still passes while it's tripped, every guess is still being tested, so it doesn't cap anything. If it blocks everyone, that's the lockout from [Bug] Shared sign-in limit lets anyone with the URL block new sign-ins on public live deployments #216 again. A key is the only credential here, so I don't see how to have both. I've kept per-client limits since the key is at least 24 characters, though that's only a length check. If maintainers prefer a tighter limit, one option is lowering the per-client budget while global failures are high. I'd like their call before adding it.
2. Failures only: Fixed. Each attempt reserves a slot up front, so parallel guesses can't race past the limit, and it's refunded unless the response is 401. Added a regression test.
3. X-Forwarded-For: Documented the requirement in SECURITY.md. The hop-count option seems better as a separate change.
4. Store saturation: Failing open on this route alone wouldn't help, because the general /api/* limiter runs first with the same cap and fails closed. tests/rate-limit.test.ts expects that behavior, so I've left it out of this PR.
5. Window bursts: Agreed it's minor, so no change here.

[MustafaYamin]

@caspianchan31 Please share what is the update on the PR.