fix(security): default TRUSTED_PROXY_DEPTH to fail closed

[Bayyan16]

Summary

Fixes #230.

This PR makes client-IP extraction fail closed by default by changing the default TRUSTED_PROXY_DEPTH from 1 to 0.

Previously, when TRUSTED_PROXY_DEPTH was unset, the API assumed one trusted proxy and consumed X-Forwarded-For as trusted input. Since X-Forwarded-For is client-controllable unless it is stripped or rewritten by a trusted reverse proxy, this could allow spoofed-IP behavior to influence HTTP rate limiting, IP blocklist decisions, and WebSocket abuse controls.

With this change, forwarded headers are ignored unless operators explicitly opt in by setting TRUSTED_PROXY_DEPTH=1.

What Changed

This PR updates all client-IP extraction paths that previously defaulted to trusting one proxy:

• src/middleware/rate-limit.ts
• src/middleware/ip-blocklist.ts
• src/routes/ws.ts

The default trusted proxy depth is changed from:

process.env.TRUSTED_PROXY_DEPTH ?? 1

to:

process.env.TRUSTED_PROXY_DEPTH ?? 0

The invalid-config fallback is also changed from fail-open to fail-closed:

- return 1;
+ return 0;

This means that if TRUSTED_PROXY_DEPTH is missing or invalid, the application no longer trusts client-supplied forwarded headers by default.

Why This Fix Is Needed

X-Forwarded-For should only be trusted when the application is deployed behind a trusted proxy or load balancer that sanitizes the header.

Before this change, the application defaulted to TRUSTED_PROXY_DEPTH=1, which means requests with an X-Forwarded-For header could influence the client IP used by abuse-control logic even when the operator did not explicitly configure proxy trust.

That client IP is used in multiple security-sensitive paths:

• HTTP read rate limiting
• HTTP write rate limiting
• IP blocklist enforcement
• WebSocket IP blocklist checks
• WebSocket auth-failure bans
• WebSocket authenticated per-IP connection caps
• WebSocket unauthenticated per-IP connection caps

Defaulting to 0 makes the behavior fail closed. Deployments that intentionally rely on trusted proxy headers can still opt in explicitly with:

TRUSTED_PROXY_DEPTH=1

Test Changes

This PR adds regression coverage in:

tests/middleware/proxy-depth-default.test.ts

The new tests verify that:

1. When TRUSTED_PROXY_DEPTH is unset, spoofed X-Forwarded-For values are ignored for HTTP rate limiting.
2. When TRUSTED_PROXY_DEPTH is unset, the IP blocklist decision uses the socket IP instead of spoofed X-Forwarded-For.
3. Explicit trusted-proxy mode still works when TRUSTED_PROXY_DEPTH=1.

This PR also updates the existing rate-limit middleware tests to explicitly exercise trusted-proxy behavior with TRUSTED_PROXY_DEPTH=1.

That keeps existing X-Forwarded-For rate-limit behavior covered, while the new regression test covers the secure default.

Testing

Relevant middleware tests passed locally:

./node_modules/.bin/vitest run tests/middleware --reporter=verbose

Result:

Test Files  7 passed (7)
Tests       82 passed (82)

I also ran the broader Vitest suite locally. The full suite currently reaches unrelated failures in tests/sdk-smoke.test.ts because local devnet v17 PROGRAM_ID / MATCHER_PROGRAM_ID environment variables are not configured:

Percolator v17 program is not deployed for devnet
Percolator v17 matcher program is not deployed for devnet

Those SDK smoke failures are unrelated to this PR's middleware changes.

Notes

This PR does not remove trusted-proxy support.

It only changes the default behavior so that forwarded-header trust is explicit instead of enabled by default. Operators who deploy behind a trusted proxy can continue using forwarded headers by setting:

TRUSTED_PROXY_DEPTH=1

Summary by CodeRabbit

• Bug Fixes

  • Improved how client IPs are determined when proxy depth is not configured or is invalid.
  • Requests now default to using the direct connection IP instead of trusting forwarded headers by default.
  • This changes rate limiting and IP blocklist behavior to be more secure when proxy settings are absent.

• Tests

  • Added and updated coverage for proxy-depth defaults and forwarded IP handling.

[vercel]

@Bayyan16 is attempting to deploy a commit to the Khubair Nasir's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

TRUSTED_PROXY_DEPTH now defaults to 0 and falls back to 0 on invalid values in middleware and WebSocket client-IP parsing. New tests cover socket-IP-based rate limiting and blocklist behavior, plus explicit TRUSTED_PROXY_DEPTH=1 proxy handling.

Changes

Trusted proxy depth default

Layer / File(s)	Summary
Proxy-depth fallback logic src/middleware/ip-blocklist.ts, src/middleware/rate-limit.ts, src/routes/ws.ts	TRUSTED_PROXY_DEPTH now defaults to 0 and invalid values return 0 in the blocklist, rate-limit, and WebSocket client-IP parsing paths.
Proxy-depth regression tests tests/middleware/proxy-depth-default.test.ts, tests/middleware/rate-limit.test.ts	Vitest coverage adds deterministic socket-IP mocking, environment isolation, and assertions for unset and explicit TRUSTED_PROXY_DEPTH behavior in rate limiting and blocklist checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• dcccrypto/percolator-api#105: Changes the PROXY_DEPTH=0 client-IP path that this PR now reaches by default.
• dcccrypto/percolator-api#110: Updates rate-limit client-IP resolution under trusted-proxy handling, which overlaps with the rate-limit change here.
• dcccrypto/percolator-api#111: Updates ip-blocklist client-IP resolution under proxy-depth handling, which overlaps with the blocklist change here.

Poem

A bunny hopped through proxy dew,
with zero trust, the headers flew.
The socket knew the path to stay,
and spoofed-up foxes lost their way. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: defaulting TRUSTED_PROXY_DEPTH to fail closed.
Linked Issues check	✅ Passed	The changes match issue #230 by switching the default/fallback proxy depth to 0 across rate limiting, blocklist, and WebSocket IP handling, with tests covering the new behavior.
Out of Scope Changes check	✅ Passed	The PR stays focused on proxy-depth defaults and regression tests, with no obvious unrelated code changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/middleware/proxy-depth-default.test.ts (1)

45-123: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add malformed-depth and WebSocket regression cases.

This suite proves the unset HTTP middleware path and the explicit TRUSTED_PROXY_DEPTH=1 opt-in, but the new security contract is broader: invalid values must also fail closed, and src/routes/ws.ts has its own parser. Without those cases, a future regression can reopen spoofed-IP influence over WS blocklist/auth-ban/connection-cap logic without tripping tests.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/middleware/proxy-depth-default.test.ts` around lines 45 - 123, The
proxy-depth tests only cover the unset default and TRUSTED_PROXY_DEPTH=1 path,
so add regression coverage for malformed TRUSTED_PROXY_DEPTH values that must
fail closed and ignore spoofed X-Forwarded-For; use the existing middleware
setup in proxy-depth-default.test.ts with the same mockSocketIp, readRateLimit,
and ipBlocklist symbols. Also add WebSocket regression cases for
src/routes/ws.ts, verifying its depth parsing follows the same secure default
and does not let malformed or unset trusted-proxy settings affect
blocklist/auth-ban/connection-cap decisions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/middleware/proxy-depth-default.test.ts`:
- Around line 45-123: The proxy-depth tests only cover the unset default and
TRUSTED_PROXY_DEPTH=1 path, so add regression coverage for malformed
TRUSTED_PROXY_DEPTH values that must fail closed and ignore spoofed
X-Forwarded-For; use the existing middleware setup in
proxy-depth-default.test.ts with the same mockSocketIp, readRateLimit, and
ipBlocklist symbols. Also add WebSocket regression cases for src/routes/ws.ts,
verifying its depth parsing follows the same secure default and does not let
malformed or unset trusted-proxy settings affect
blocklist/auth-ban/connection-cap decisions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e6cbf76-8f90-4083-b61b-c584094acfc3

📥 Commits

Reviewing files that changed from the base of the PR and between b2751f4 and f45b574.

📒 Files selected for processing (5)

• src/middleware/ip-blocklist.ts
• src/middleware/rate-limit.ts
• src/routes/ws.ts
• tests/middleware/proxy-depth-default.test.ts
• tests/middleware/rate-limit.test.ts

[Bayyan16]

A quick note on the failing CI check: the failure happens during pnpm install --frozen-lockfile before this PR's build or test steps run.

The log shows:

ENOENT: no such file or directory, scandir '/home/runner/work/percolator-sdk'

This appears to be caused by the existing local SDK dependency resolution path (../../percolator-sdk) not being available in the GitHub Actions runner. It is unrelated to this PR's TRUSTED_PROXY_DEPTH changes.

Relevant middleware tests for this PR passed locally:

./node_modules/.bin/vitest run tests/middleware --reporter=verbose

Result:

Test Files  7 passed (7)
Tests       82 passed (82)

Vercel also appears to require team authorization before deployment, which is separate from the code changes in this PR.