fix(onramp): fail-closed when client IP cannot be determined

[piyalbasu]

Summary

Follow-up to #310. The IP-binding stopgap silently degraded to "no binding" on default deployments where FREIGHTER_TRUST_PROXY_RANGE was unset — the handler dropped clientIp and proceeded, minting an unbound Coinbase session against SDF's API credentials. SDF prod was not exposed (the env var is set explicitly to 172.16.0.0/12 in all three envs), but third-party deployers and any future SDF deployment with a topology change were vulnerable.

This PR closes the bug class and tightens the surrounding surface:

• Fail-closed in the handler. /onramp/token now returns 400 Could not determine client IP for Coinbase session binding when request.ip resolves to an internal address (loopback, RFC1918, link-local, IPv6 ULA), instead of dropping clientIp and proceeding (src/route/index.ts:1518-1532).
• Default for FREIGHTER_TRUST_PROXY_RANGE. Falls through to "loopback,linklocal,uniquelocal" when the value is empty. Covers typical k8s / load-balancer topologies without per-deployment configuration. The env var must still be declared in .env (it's in ENV_KEYS) — .env-EXAMPLE already declares it empty, which is the path of least resistance.
• Boot-time validation. initApiServer throws if FREIGHTER_TRUST_PROXY_RANGE is non-empty but parses to no subnets (whitespace, ", , ,", etc.). Catches misconfigurations before they go live.
• clientIp is now required. Was clientIp?: string in fetchOnrampSessionToken with a conditional spread that omitted the field when undefined. Locks the IP-binding invariant at the type level — future callers can't re-introduce the unbound-token bug.
• Outbound Coinbase fetch timeout. AbortSignal.timeout(10_000) prevents slow-Coinbase amplification of resource consumption.
• Per-route rate limit on /onramp/token. Tightened to 20 req/min/IP (vs the global 100 req/min/IP). Inherits the global Redis store via @fastify/rate-limit's parent-config merge.

This PR explicitly reverses the fail-open design from #310. PR #310's commit message described "dropping clientIp keeps the endpoint functional while we surface the misconfiguration via a warning log" — that trade was correct given the hotfix scope at the time, but a security report flagged that an unbound session is worse than no session (it converts the endpoint into an unauthenticated phishing primitive against SDF's Coinbase merchant credentials). The full ownership-proof mitigation (wallet-signature challenge / Origin + per-install HMAC) is still tracked separately and will close the remaining browser-based phishing vector.

The wallet-eng-runbooks PR adds a Cause D to the existing onramp incident runbook covering the new 400 — diagnosis (grep the warning log, inspect rawIp / socketRemote), and resolution (which trust ranges to use for which topology changes — cluster recreation, service mesh, CNI swap, new node groups).

Test plan

• npx jest — 140 passed, 3 skipped, 0 failures
• npx tsc --noEmit — clean
• Confirm /onramp/token still works in wallet-eng-stg after deploy (XFF header arrives, clientIp forwarded to Coinbase, session minted)
• Confirm the warning-log rate stays at 0 in observability (no real users hitting the fail-closed path)
• Manually verify boot-time validation by setting FREIGHTER_TRUST_PROXY_RANGE=" " in a dev container and observing the startup error

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Tightens Coinbase onramp session IP-binding by making /onramp/token fail closed when a public client IP can’t be reliably derived, and hardens the surrounding configuration and operational controls (defaults, boot-time validation, timeouts, and rate limiting).

Changes:

• Added boot-time parsing/validation for FREIGHTER_TRUST_PROXY_RANGE and updated Fastify trustProxy wiring.
• Updated /onramp/token to refuse issuing a Coinbase session when request.ip is internal/private, and added a stricter per-route rate limit.
• Made clientIp required for Coinbase token minting, added an outbound fetch timeout, and updated tests/docs accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/route/index.ts	Validates/compiles trust proxy configuration; tightens /onramp/token behavior and rate limiting.
src/route/index.test.ts	Adds coverage for trust-proxy boot validation and fail-closed behavior; updates onramp route expectations.
src/helper/test-helper.ts	Changes dev server default trustProxyRange to match new default trust behavior for tests.
src/helper/onramp.ts	Requires clientIp and adds an outbound timeout for Coinbase token requests.
src/helper/onramp.test.ts	Updates tests for required clientIp and timeout signal attachment.
src/config.ts	Adds a default value for trustProxyRange when unset/empty.
README.md	Documents required env key behavior and how to test /onramp/token locally under fail-closed semantics.
docs/runbook.md	Updates runbook configuration guidance for FREIGHTER_TRUST_PROXY_RANGE and the new fail-closed behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[piyalbasu]

I acknowledge that this causes some annoying overhead with running Freighter BE and using the onramp locally, but ultimately, we will be moving to authenticated requests to Freighter's backend, so this is some short-lived poor devx

[aristidesstaffieri]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[aristidesstaffieri]

The docs include several references to SDF specific infra and internal processes. Should the docs be more generic for general operators outside of the SDF?

[piyalbasu]

The docs include several references to SDF specific infra and internal processes. Should the docs be more generic for general operators outside of the SDF?

Good callout - I'll do that