Skip to content

fix(rate-limit): trust proxy hops explicitly for client IP resolution - #264

Merged
Emmy123222 merged 4 commits into
Emmy123222:mainfrom
meem08:fix/trust-proxy-rate-limiting
Sep 3, 2026
Merged

fix(rate-limit): trust proxy hops explicitly for client IP resolution#264
Emmy123222 merged 4 commits into
Emmy123222:mainfrom
meem08:fix/trust-proxy-rate-limiting

Conversation

@meem08

@meem08 meem08 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

PR - Configure Express trust proxy before IP-based rate limiting

Security, privacy & abuse prevention - Repo Avatar: Emmy123222/Stellar-Search

Summary

Adds an explicit, per-deployment Express trust proxy configuration (TRUST_PROXY_HOPS) so that express-rate-limit resolves the real client IP instead of the nearest reverse proxy's IP when the app is deployed behind Vercel, nginx, or a load balancer.

Branch: fix/trust-proxy-rate-limiting -> main
Stack: Express ^4.18.2 + express-rate-limit ^8.6.2 + supertest + vitest ^4.1.11


Motivation / Context

Closes #177.

From the issue:

Behind Vercel or another proxy, req.ip and express-rate-limit behavior may reflect the proxy or accept spoofed X-Forwarded-For data.

There was no trust proxy configuration anywhere in the repo. The global limiter (server/index.ts) uses express-rate-limit with the default keyGenerator, which reads req.ip. Consequences when deployed behind a proxy:

  • False collapse: every distinct client shares the proxy's IP -> one rate-limit bucket -> legitimate users get throttled as a group.
  • Spoof risk without validation: if a deployment ever trusted X-Forwarded-For implicitly (or relied on it in logging), a single client could masquerade as many IPs and multiply its allowance.

Shipping requires the hop count to be explicit per deployment (acceptance criterion) so the operator chooses the security posture rather than inheriting an ambiguous default.


Solution Overview

  1. server/index.ts - app.set('trust proxy', ...) driven by a new TRUST_PROXY_HOPS env var:
    • unset / 0 -> trust no proxy (Express default; req.ip ignores X-Forwarded-For entirely, so spoofing cannot add buckets).
    • <n> -> trust exactly n hops (e.g. 1 for Vercel).
    • true -> trust all proxies (opaque, fully-controlled networks only).
  2. server/trustProxy.test.ts - 6 new tests proving:
    • the hop count is explicit and configurable per deployment,
    • distinct clients behind one trusted proxy are limited separately,
    • spoofed X-Forwarded-For headers do not bypass the limit when no proxy is trusted,
    • a legitimate client under the limit is not blocked.
  3. Docs & config updated where the behavior changes: .env.example, .env.production (Vercel => TRUST_PROXY_HOPS=1), README.md env table, CONTRIBUTING.md (trust-proxy primer + troubleshooting row).

Acceptance Criteria - How This PR Satisfies

Criterion Implementation Evidence
Trusted proxy hops are explicit per deployment TRUST_PROXY_HOPS env var -> app.set('trust proxy', ...) in server/index.ts; documented in .env.example, .env.production, README.md, CONTRIBUTING.md Config tests assert app.get('trust proxy') equals false, 1, true for the documented values
Tests prove distinct clients are limited correctly trustProxy.test.ts - two clients behind one trusted proxy get separate buckets (198.51.100.11 exhausted -> 429, .22 -> 200) and a legitimate under-limit client passes 6/6 tests pass
Tests prove spoofed headers do not bypass limits trustProxy.test.ts - with no trusted proxy, requests flipping X-Forwarded-For still collapse to the single real socket and hit the limit on the 3rd request 6/6 tests pass
Automated coverage and documentation are updated New test file (adds server/index.ts coverage); docs updated in the same PR Coverage runs in CI

How to Test Locally

npm ci
npm run typecheck   # passes
npm run lint        # 0 errors
npx vitest run server/trustProxy.test.ts   # 6/6 pass

Test matrix (what each case proves)

Test Config under test Verifies
defaults to no trust proxy unset TRUST_PROXY_HOPS req.ip ignores X-Forwarded-For (spoof-safe by default)
TRUST_PROXY_HOPS=1 1 trusts exactly 1 hop
TRUST_PROXY_HOPS=true true trusts all proxies
distinct clients own buckets 1 client A exhausted -> 429; client B (different IP) -> 200
spoofed XFF cannot bypass unset (0) 3 different spoofed X-Forwarded-For values still capped at 2/min -> 3rd is 429
legitimate under-limit client 1, limit 5 5 requests -> 200; 6th -> 429

Changes by File

File Change
server/index.ts Add resolveTrustProxy() + app.set('trust proxy', ...) from TRUST_PROXY_HOPS. No change to limiter config, x402 middleware, validateQuery, or any route handler.
server/trustProxy.test.ts New - 6 tests for hop configuration, client isolation, and spoofing defense (uses @vitest-environment node; server tests need no DOM).
.env.example Document TRUST_PROXY_HOPS with values.
.env.production Set TRUST_PROXY_HOPS=1 (Vercel = 1 proxy hop).
README.md Add TRUST_PROXY_HOPS row to the Environment Variables table.
CONTRIBUTING.md Add Trusting proxies primer + troubleshooting entry.

Cross-Runtime Alignment - Verified

The delivery note requires Express / Vercel / browser / MCP alignment where this concern crosses runtime boundaries. The rate limiter lives only in the Express server (server/index.ts); the Vercel serverless handlers (api/) do not use express-rate-limit and are unchanged. The browser and MCP consumers are unchanged.

x402 settlement semantics for paid routes are preserved - no edits to paymentMiddlewareFromConfig, ExactStellarScheme, the x402Accepts config (scheme:exact price:0.001 amount:10000 network payTo), the payment replay-protection middleware, or src/lib/paymentIntegrity.ts. The already-verified server/payment.test.ts suite is untouched.


Breaking Changes / Risks

  • Behavioral default is unchanged - TRUST_PROXY_HOPS unset behaves exactly as before (Express default, no proxy trusted), so this is safe for direct / single-process deployments.
  • Explicit opt-in required for proxy deployments (set TRUST_PROXY_HOPS=1 on Vercel). server/index.ts documents this at the config site and .env.production sets it for Vercel.
  • No dependency changes - package.json / package-lock.json are untouched.

Checklist

  • npm run typecheck passes
  • npm run lint passes (0 errors)
  • npx vitest run server/trustProxy.test.ts passes (6/6)
  • Trusted proxy hops explicit per deployment (TRUST_PROXY_HOPS)
  • Tests prove distinct clients limited correctly + spoofed headers do not bypass
  • Docs (.env.example, .env.production, README.md, CONTRIBUTING.md) updated in same PR
  • x402 settlement semantics for paid routes preserved (no related code touched)

Closes #177

Configure Express trust proxy behind Vercel/nginx/load balancers so
express-rate-limit keys clients by their real IP instead of the proxy's.

- TRUST_PROXY_HOPS env (unset/0 = no proxy trusted, <n> = trust n hops,
  true = trust all) wired via app.set('trust proxy', ...)
- add server/trustProxy.test.ts proving distinct clients are limited
  separately and spoofed X-Forwarded-For headers cannot bypass limits
- document TRUST_PROXY_HOPS in .env.example, .env.production (Vercel=1),
  README, and CONTRIBUTING

No changes to x402 settlement semantics for paid routes.

Closes Emmy123222#177
@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Emmanuel's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Aug 31, 2026

Copy link
Copy Markdown

@meem08 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@meem08

meem08 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@Emmy123222 review and merge

@AbuJulaybeeb AbuJulaybeeb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve conflicts

@meem08

meem08 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Done @AbuJulaybeeb

@meem08
meem08 force-pushed the fix/trust-proxy-rate-limiting branch from 12e9670 to d7e08eb Compare September 1, 2026 14:24
🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@meem08
meem08 force-pushed the fix/trust-proxy-rate-limiting branch from d7e08eb to f47ae2c Compare September 1, 2026 20:12
…main

Accept upstream deletion of .env.production (production config via Vercel),
merge TRUST_PROXY_HOPS row with upstream's RATE_LIMIT_PER_MINUTE,
PAYMENT_AMOUNT_USDC/STROOPS, VITE_SERVER_URL, and deployment config.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@AbuJulaybeeb

Copy link
Copy Markdown
Collaborator

Done @AbuJulaybeeb

it didn't work

@Emmy123222
Emmy123222 merged commit 5bd52c9 into Emmy123222:main Sep 3, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configure Express trust proxy before IP-based rate limiting

3 participants