fix(onramp): bind Coinbase session token to caller IP

[piyalbasu]

Summary

• Pass request.ip as clientIp to Coinbase's create-session-token API so each Onramp session is bound to the requesting client.
• Stopgap for an issue flagging that POST /api/v1/onramp/token is unauthenticated and currently mints sessions under SDF's Coinbase credentials for any attacker-supplied Stellar address with no client/session binding. Full ownership-proof mitigation (wallet-signature challenge or Origin + per-install HMAC) is tracked separately.
• Coinbase documents clientIp as: "The client IP address of the end user. This parameter ensures the quote can only be used by the requesting user."

Why this is the right stopgap

• Highest-leverage change available without UX cost — no signing, no schema change, no extension release.
• The reporter explicitly called out the absence of clientIp as part of the missing-binding finding.
• Coinbase's hard- vs soft-enforcement of clientIp is undocumented; verification (see test plan) determines the post-fix severity.

⚠️ Reviewer note: temporary diagnostic logging is intentional

This PR includes a logger.warn(...) block in the onramp route handler that fires whenever request.ip resolves to a private/internal address. It is intentionally left in until we verify the trust chain end-to-end in production, then removed in a follow-up PR.

Reason it has to stay in for prod validation: staging cannot fully validate this fix. Staging traffic enters via ingress-private, which does not set use-forwarded-headers on nginx and does not enable preserve_client_ip on the NLB — so the original client IP is dropped at the NLB before it ever reaches freighter-backend. Production uses ingress-public, which has both settings configured correctly. We need the warn-log present in prod logs to confirm request.ip resolves to a real external IP after rollout.

Cleanup PR will follow once verification is complete (warn rate goes to ~0 in prod and onramp 4xx rate is normal).

Companion change (must merge first / land together)

stellar/kube#4310: widens FREIGHTER_TRUST_PROXY_RANGE from 172.23.0.0/16 to 172.16.0.0/12 across prod/staging/dev. The previous /16 did not actually cover the nginx-ingress pod CIDR (172.22.x.x observed in staging logs), so Fastify treated nginx as untrusted and refused to consume X-Forwarded-For. Without that kube change, this PR's clientIp would always be the proxy's private IP and Coinbase would 4xx every request — the in-code private-IP guard here keeps the endpoint functional in that misconfigured state.

Defense in depth in this PR

• Private-IP guard (isLikelyInternalIp): if request.ip resolves to RFC1918/loopback/link-local, drop clientIp and warn. Endpoint stays functional even when the trust chain is misconfigured; the warn is the forcing function to fix it.
• Surface Coinbase's actual error: previously, the route returned Unable to retrieve token: undefined on Coinbase 4xx because the helper put the error inside data instead of at top level. Fixed; route logs now include Coinbase's response body.

Test plan

• src/helper/onramp.test.ts (new) — fetchOnrampSessionToken: forwards clientIp when present, omits when undefined/empty, surfaces Coinbase 4xx body on top-level error.
• src/helper/onramp.test.ts (new) — isLikelyInternalIp: parametrized over RFC1918, loopback, link-local, and public IPs.
• src/route/index.test.ts — onramp route test asserts the handler drops clientIp for loopback test traffic and would forward it for public IPs.
• npx tsc --noEmit clean.
• Post-deploy prod smoke check: confirm warn-log rate goes to ~0 and onramp 4xx rate is normal after rollout. (Cannot validate in staging — see reviewer note above.)
• Prod IP-mismatch verification: mint a token from one IP via prod, open the resulting pay.coinbase.com URL from a different IP (phone hotspot/VPN), observe whether Coinbase blocks or completes. Hard enforcement → severity downgrades to Low; soft scoring → severity stays Medium with mitigations in place. Coordinate with @stellar/ops-team due to the existing freighter-backend-onramp-token-error-rate-high alert.
• Cleanup PR: remove the logger.warn diagnostic once prod is verified clean.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Binds Coinbase Onramp session tokens to the requesting client by forwarding the caller IP (request.ip) as clientIp into Coinbase’s create-session-token request.

Changes:

• Forward request.ip from POST /api/v1/onramp/token into fetchOnrampSessionToken as clientIp.
• Include clientIp in the Coinbase /onramp/v1/token request body when present.
• Add/extend Jest coverage to ensure clientIp is forwarded and conditionally omitted.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/route/index.ts	Captures request.ip and forwards it to the onramp helper for Coinbase session binding.
src/route/index.test.ts	Asserts the route forwards a clientIp string into fetchOnrampSessionToken.
src/helper/onramp.ts	Extends helper signature and request body to optionally include clientIp.
src/helper/onramp.test.ts	New unit tests validating clientIp inclusion/omission behavior in request body.

Comments suppressed due to low confidence (1)

src/helper/onramp.ts:59

• fetchOnrampSessionToken defines coinbaseConfig with an inline object type even though this module already exports a CoinbaseConfig interface. Reusing CoinbaseConfig here would avoid type duplication and prevent future drift if the config shape changes.

  coinbaseConfig: {
    coinbaseApiKey: string;
    coinbaseApiSecret: string;
  };

---

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

[piyalbasu]

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]

Can you explain this piece?

The response body is a stream so it only should throw when its been consumed, in that case await res.text() should also throw and would leave detail to be an empty string. I would think it would be more like -

 let detail = "";
 try {
   const text = await res.text();
   try {
     detail = JSON.stringify(JSON.parse(text));
   } catch {
     detail = text; // not JSON — use the raw body
   }
 } catch {
   // body genuinely unreadable (network / decode error)
 }

but I'm surprised we would need to handle potential non JSON bodies from this API.

[piyalbasu]

ah yeah - you're right. Let me clean this up

[piyalbasu]

Done — pushed in 190933d. Agreed on both points: the inner res.text() was dead code (body already consumed by res.json()), and Coinbase always returns JSON so we don't need the parse-normalize roundtrip. Just await res.text() once.

[aristidesstaffieri]

we can use proxy-addr which is already in the project in order to not have to rely on a hand rolled regex here.

[piyalbasu]

Done — pushed in 190933d. Switched to proxyaddr.compile(["loopback", "linklocal", "uniquelocal"]). Bonus: it correctly covers IPv6 link-local (fe80::/10) and unique-local (fc00::/7), which the regex missed, and handles IPv4-mapped IPv6 via the library's normalization. Tests extended for those cases.