From 34b66511b0098255b3ea5f5e9ba217c491e1b36f Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 29 Apr 2026 16:31:53 -0400 Subject: [PATCH 1/4] fix(onramp): bind Coinbase session token to caller IP Pass `request.ip` as `clientIp` to the Coinbase create-session-token API so the resulting Onramp session is bound to the requesting client. Coinbase documents this parameter as ensuring the quote can only be used by the requesting user. Stopgap addressing the unauthenticated `/api/v1/onramp/token` endpoint allowing arbitrary callers to mint Coinbase sessions for arbitrary Stellar destinations under SDF's API credentials. Full ownership-proof mitigation (wallet-signature challenge / Origin + per-install HMAC) is tracked separately. Relies on `FREIGHTER_TRUST_PROXY_RANGE` matching the actual upstream proxy CIDR (currently the EKS pod range). A code comment flags the invariant for future maintainers. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/helper/onramp.test.ts | 71 +++++++++++++++++++++++++++++++++++++++ src/helper/onramp.ts | 3 ++ src/route/index.test.ts | 24 ++++++++----- src/route/index.ts | 8 +++++ 4 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 src/helper/onramp.test.ts diff --git a/src/helper/onramp.test.ts b/src/helper/onramp.test.ts new file mode 100644 index 0000000..c3c9fec --- /dev/null +++ b/src/helper/onramp.test.ts @@ -0,0 +1,71 @@ +import jwt from "jsonwebtoken"; +import { fetchOnrampSessionToken } from "./onramp"; + +const coinbaseConfig = { + coinbaseApiKey: "test-key", + coinbaseApiSecret: "test-secret", +}; + +describe("fetchOnrampSessionToken", () => { + let fetchMock: jest.Mock; + + beforeEach(() => { + jest.spyOn(jwt, "sign").mockReturnValue("test-jwt" as any); + fetchMock = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ token: "test-token" }), + }); + global.fetch = fetchMock as any; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("forwards clientIp into the Coinbase request body when provided", async () => { + await fetchOnrampSessionToken({ + address: "GFOO", + clientIp: "203.0.113.42", + coinbaseConfig, + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [, options] = fetchMock.mock.calls[0]; + const body = JSON.parse(options.body); + + expect(body).toEqual({ + addresses: [ + { address: "GFOO", blockchains: ["stellar"], assets: ["XLM"] }, + ], + clientIp: "203.0.113.42", + }); + }); + + it("omits clientIp from the body when not provided", async () => { + await fetchOnrampSessionToken({ + address: "GFOO", + coinbaseConfig, + }); + + const [, options] = fetchMock.mock.calls[0]; + const body = JSON.parse(options.body); + + expect(body).not.toHaveProperty("clientIp"); + expect(body.addresses).toEqual([ + { address: "GFOO", blockchains: ["stellar"], assets: ["XLM"] }, + ]); + }); + + it("omits clientIp from the body when passed an empty string", async () => { + await fetchOnrampSessionToken({ + address: "GFOO", + clientIp: "", + coinbaseConfig, + }); + + const [, options] = fetchMock.mock.calls[0]; + const body = JSON.parse(options.body); + + expect(body).not.toHaveProperty("clientIp"); + }); +}); diff --git a/src/helper/onramp.ts b/src/helper/onramp.ts index 67b08aa..37f31ea 100644 --- a/src/helper/onramp.ts +++ b/src/helper/onramp.ts @@ -48,9 +48,11 @@ export const generateJWT = ({ export const fetchOnrampSessionToken = async ({ address, + clientIp, coinbaseConfig, }: { address: string; + clientIp?: string; coinbaseConfig: { coinbaseApiKey: string; coinbaseApiSecret: string; @@ -65,6 +67,7 @@ export const fetchOnrampSessionToken = async ({ }, body: JSON.stringify({ addresses: [{ address, blockchains: ["stellar"], assets: ["XLM"] }], + ...(clientIp ? { clientIp } : {}), }), }; const res = await fetch(`https://${requestHost}${requestPath}`, options); diff --git a/src/route/index.test.ts b/src/route/index.test.ts index c0ff36c..4bd1ce2 100644 --- a/src/route/index.test.ts +++ b/src/route/index.test.ts @@ -1110,14 +1110,16 @@ describe("API routes", () => { }); it("can fetch an onramp token", async () => { - jest.spyOn(OnrampHelpers, "fetchOnrampSessionToken").mockReturnValueOnce( - Promise.resolve({ - data: { - token: "token", - }, - error: null, - }), - ); + const fetchSpy = jest + .spyOn(OnrampHelpers, "fetchOnrampSessionToken") + .mockReturnValueOnce( + Promise.resolve({ + data: { + token: "token", + }, + error: null, + }), + ); const server = await getDevServer(); const url = new URL( @@ -1139,6 +1141,12 @@ describe("API routes", () => { expect(response.status).toEqual(200); expect(resJson.data.token).toEqual("token"); + expect(fetchSpy).toHaveBeenCalledWith( + expect.objectContaining({ + address: "GFOO", + clientIp: expect.any(String), + }), + ); await server.close(); }); it("does not fetch a token without Coinbase config", async () => { diff --git a/src/route/index.ts b/src/route/index.ts index b6180e9..321afa5 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -1475,6 +1475,13 @@ export async function initApiServer( reply, ) => { const { address } = request.body; + // Forwarded to Coinbase to bind the resulting Onramp session to the + // requesting client. Relies on FREIGHTER_TRUST_PROXY_RANGE matching + // the actual upstream proxy CIDR — currently the EKS pod range. If + // Cloudflare (or any new hop) is ever added in front of this service, + // that env var must include its egress ranges or request.ip will + // silently resolve to the proxy and defeat the IP binding. + const clientIp = request.ip; if ( !coinbaseConfig.coinbaseApiKey || !coinbaseConfig.coinbaseApiSecret @@ -1485,6 +1492,7 @@ export async function initApiServer( try { const { data, error } = await fetchOnrampSessionToken({ address, + clientIp, coinbaseConfig, }); From e1a1ed5ebdad022c52a16ea20fe439e5132f9e19 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 29 Apr 2026 17:33:11 -0400 Subject: [PATCH 2/4] chore(onramp): temporary diagnostic log for request.ip resolution Logs clientIp alongside x-forwarded-for, x-real-ip, and the socket remote address so we can verify the trustProxy chain resolves to the real client IP. To be removed once verified in staging/prod. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/route/index.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/route/index.ts b/src/route/index.ts index 321afa5..9540393 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -1482,6 +1482,18 @@ export async function initApiServer( // that env var must include its egress ranges or request.ip will // silently resolve to the proxy and defeat the IP binding. const clientIp = request.ip; + // TODO(remove): temporary diagnostic to verify trustProxy chain + // resolves request.ip to the real client IP rather than an + // intra-cluster hop. Drop once verified. + logger.info( + { + clientIp, + xff: request.headers["x-forwarded-for"], + xRealIp: request.headers["x-real-ip"], + socketRemote: request.socket.remoteAddress, + }, + "onramp.token request", + ); if ( !coinbaseConfig.coinbaseApiKey || !coinbaseConfig.coinbaseApiSecret From bd159688e29a3e82bf1ad46694cd7f00d8014d6c Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 30 Apr 2026 11:29:54 -0400 Subject: [PATCH 3/4] fix(onramp): guard against internal IPs and surface Coinbase errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs surfaced once the diagnostic log landed: 1. request.ip was resolving to the nginx-ingress pod IP (172.22.x.x) because FREIGHTER_TRUST_PROXY_RANGE (172.23.0.0/16) doesn't cover the actual proxy CIDR. We were forwarding an RFC1918 address as clientIp to Coinbase, which rejected it as invalid. 2. The route handler destructured `error` from the helper result, but the helper put the error message inside `data` on 4xx — so the route surfaced "Unable to retrieve token: undefined" with no detail. Fixes: - Add isLikelyInternalIp helper; drop clientIp and log a warn when request.ip is loopback/RFC1918/link-local. Endpoint stays functional if the trust chain is misconfigured; the warn signals the misconfig. - Move 4xx error to top-level of the helper return shape and include Coinbase's actual response body, so route logs say what's wrong. Removes the temporary onramp.token info-log; the warn-only-on-misconfig path replaces it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/helper/onramp.test.ts | 48 ++++++++++++++++++++++++++++++++++++++- src/helper/onramp.ts | 28 ++++++++++++++++++++++- src/route/index.test.ts | 5 +++- src/route/index.ts | 39 ++++++++++++++++--------------- 4 files changed, 99 insertions(+), 21 deletions(-) diff --git a/src/helper/onramp.test.ts b/src/helper/onramp.test.ts index c3c9fec..efaf2cd 100644 --- a/src/helper/onramp.test.ts +++ b/src/helper/onramp.test.ts @@ -1,5 +1,5 @@ import jwt from "jsonwebtoken"; -import { fetchOnrampSessionToken } from "./onramp"; +import { fetchOnrampSessionToken, isLikelyInternalIp } from "./onramp"; const coinbaseConfig = { coinbaseApiKey: "test-key", @@ -68,4 +68,50 @@ describe("fetchOnrampSessionToken", () => { expect(body).not.toHaveProperty("clientIp"); }); + + it("surfaces Coinbase's response body in the top-level error on 4xx", async () => { + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 400, + json: async () => ({ error: "invalid clientIp" }), + text: async () => '{"error":"invalid clientIp"}', + }); + + const result = await fetchOnrampSessionToken({ + address: "GFOO", + clientIp: "10.0.0.1", + coinbaseConfig, + }); + + expect(result.data.token).toBe(""); + expect(result.error).toMatch(/Coinbase 400/); + expect(result.error).toMatch(/invalid clientIp/); + }); +}); + +describe("isLikelyInternalIp", () => { + it.each([ + ["::1"], + ["127.0.0.1"], + ["10.0.0.1"], + ["10.255.255.255"], + ["172.16.0.1"], + ["172.22.89.212"], + ["172.31.255.255"], + ["192.168.1.1"], + ["169.254.1.1"], + [""], + ])("classifies %s as internal", (ip) => { + expect(isLikelyInternalIp(ip)).toBe(true); + }); + + it.each([ + ["8.8.8.8"], + ["203.0.113.42"], + ["172.15.0.1"], + ["172.32.0.1"], + ["1.1.1.1"], + ])("classifies %s as public", (ip) => { + expect(isLikelyInternalIp(ip)).toBe(false); + }); }); diff --git a/src/helper/onramp.ts b/src/helper/onramp.ts index 37f31ea..8a17bc3 100644 --- a/src/helper/onramp.ts +++ b/src/helper/onramp.ts @@ -12,6 +12,19 @@ export interface CoinbaseConfig { coinbaseApiSecret: string; } +// RFC1918 private, loopback, and link-local addresses (IPv4 and IPv4-mapped +// IPv6). Used to avoid forwarding intra-cluster IPs to Coinbase when the +// trustProxy chain is misconfigured — Coinbase rejects private addresses, +// so dropping clientIp keeps the endpoint functional while we surface the +// misconfiguration via a warning log at the call site. +export const isLikelyInternalIp = (ip: string): boolean => { + if (!ip) return true; + if (ip === "::1" || ip.startsWith("127.")) return true; + return /(?:^|:)(10\.|192\.168\.|169\.254\.|172\.(?:1[6-9]|2[0-9]|3[01])\.)/.test( + ip, + ); +}; + export const generateJWT = ({ coinbaseConfig, }: { @@ -76,7 +89,20 @@ export const fetchOnrampSessionToken = async ({ if (res.status >= 500 && res.status < 600) { throw new Error("Server error when requesting token"); } - return { data: { token: "", error: "Error fetching token request" } }; + let detail = ""; + try { + detail = JSON.stringify(await res.json()); + } catch { + try { + detail = await res.text(); + } catch { + // swallow + } + } + return { + data: { token: "" }, + error: `Coinbase ${res.status}${detail ? `: ${detail}` : ""}`, + }; } const resJson = await res.json(); diff --git a/src/route/index.test.ts b/src/route/index.test.ts index 4bd1ce2..cd179f1 100644 --- a/src/route/index.test.ts +++ b/src/route/index.test.ts @@ -1141,10 +1141,13 @@ describe("API routes", () => { expect(response.status).toEqual(200); expect(resJson.data.token).toEqual("token"); + // Local test traffic comes from a loopback address, which is + // classified as internal and dropped (Coinbase rejects private IPs). + // Real client traffic in prod resolves to a public IP and is forwarded. expect(fetchSpy).toHaveBeenCalledWith( expect.objectContaining({ address: "GFOO", - clientIp: expect.any(String), + clientIp: undefined, }), ); await server.close(); diff --git a/src/route/index.ts b/src/route/index.ts index 9540393..a216689 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -46,7 +46,11 @@ import { getSdk } from "../helper/stellar"; import { getUseMercury } from "../helper/mercury"; import { getHttpRequestDurationLabels } from "../helper/metrics"; import { mode } from "../helper/env"; -import { fetchOnrampSessionToken, CoinbaseConfig } from "../helper/onramp"; +import { + fetchOnrampSessionToken, + isLikelyInternalIp, + CoinbaseConfig, +} from "../helper/onramp"; import Blockaid from "@blockaid/client"; import { PriceClient } from "../service/prices"; import { TokenPriceData } from "../service/prices/types"; @@ -1477,23 +1481,22 @@ export async function initApiServer( const { address } = request.body; // Forwarded to Coinbase to bind the resulting Onramp session to the // requesting client. Relies on FREIGHTER_TRUST_PROXY_RANGE matching - // the actual upstream proxy CIDR — currently the EKS pod range. If - // Cloudflare (or any new hop) is ever added in front of this service, - // that env var must include its egress ranges or request.ip will - // silently resolve to the proxy and defeat the IP binding. - const clientIp = request.ip; - // TODO(remove): temporary diagnostic to verify trustProxy chain - // resolves request.ip to the real client IP rather than an - // intra-cluster hop. Drop once verified. - logger.info( - { - clientIp, - xff: request.headers["x-forwarded-for"], - xRealIp: request.headers["x-real-ip"], - socketRemote: request.socket.remoteAddress, - }, - "onramp.token request", - ); + // the actual upstream proxy CIDR. If request.ip looks like an + // intra-cluster address, the trust chain is misconfigured — drop + // clientIp (Coinbase rejects private addresses) and surface a warn. + const rawIp = request.ip; + const clientIp = isLikelyInternalIp(rawIp) ? undefined : rawIp; + if (!clientIp) { + logger.warn( + { + rawIp, + xff: request.headers["x-forwarded-for"], + xRealIp: request.headers["x-real-ip"], + socketRemote: request.socket.remoteAddress, + }, + "onramp.token: request.ip resolved to private/internal address; FREIGHTER_TRUST_PROXY_RANGE likely misconfigured. Skipping clientIp.", + ); + } if ( !coinbaseConfig.coinbaseApiKey || !coinbaseConfig.coinbaseApiSecret From 190933dcf9d585df8b91fbec5ba01759b92dc9c3 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 30 Apr 2026 17:13:15 -0400 Subject: [PATCH 4/4] fix(onramp): address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace hand-rolled regex in isLikelyInternalIp with proxy-addr's pre-defined classifications (loopback, linklocal, uniquelocal). Now correctly covers IPv6 link-local (fe80::/10) and unique-local (fc00::/7) ranges that the regex missed, and properly handles IPv4-mapped IPv6 via the library's normalization. proxy-addr is already a dep via Fastify. - Drop the dead res.text() fallback in fetchOnrampSessionToken's 4xx handler. The Fetch body is a stream — once res.json() reads bytes, res.text() in the catch would throw "body already used", so the fallback never ran. Coinbase always returns JSON, so reading body as text once is sufficient. - Extend tests to cover IPv6 link-local, unique-local, and IPv4-mapped IPv6 cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/helper/onramp.test.ts | 6 +++++- src/helper/onramp.ts | 30 +++++++++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/helper/onramp.test.ts b/src/helper/onramp.test.ts index efaf2cd..7e9c032 100644 --- a/src/helper/onramp.test.ts +++ b/src/helper/onramp.test.ts @@ -73,7 +73,6 @@ describe("fetchOnrampSessionToken", () => { fetchMock.mockResolvedValueOnce({ ok: false, status: 400, - json: async () => ({ error: "invalid clientIp" }), text: async () => '{"error":"invalid clientIp"}', }); @@ -100,6 +99,10 @@ describe("isLikelyInternalIp", () => { ["172.31.255.255"], ["192.168.1.1"], ["169.254.1.1"], + ["fe80::1"], + ["fc00::1"], + ["fd12:3456:789a::1"], + ["::ffff:10.0.0.1"], [""], ])("classifies %s as internal", (ip) => { expect(isLikelyInternalIp(ip)).toBe(true); @@ -111,6 +114,7 @@ describe("isLikelyInternalIp", () => { ["172.15.0.1"], ["172.32.0.1"], ["1.1.1.1"], + ["2001:db8::1"], ])("classifies %s as public", (ip) => { expect(isLikelyInternalIp(ip)).toBe(false); }); diff --git a/src/helper/onramp.ts b/src/helper/onramp.ts index 8a17bc3..a9a2db3 100644 --- a/src/helper/onramp.ts +++ b/src/helper/onramp.ts @@ -1,5 +1,6 @@ import jwt from "jsonwebtoken"; import * as crypto from "crypto"; +import proxyaddr from "proxy-addr"; const requestMethod = "POST"; const requestHost = "api.developer.coinbase.com"; @@ -12,17 +13,20 @@ export interface CoinbaseConfig { coinbaseApiSecret: string; } -// RFC1918 private, loopback, and link-local addresses (IPv4 and IPv4-mapped -// IPv6). Used to avoid forwarding intra-cluster IPs to Coinbase when the -// trustProxy chain is misconfigured — Coinbase rejects private addresses, -// so dropping clientIp keeps the endpoint functional while we surface the -// misconfiguration via a warning log at the call site. +// Loopback, link-local, and unique-local (RFC1918 + IPv6 ULA) addresses. +// Used to avoid forwarding intra-cluster IPs to Coinbase when the trustProxy +// chain is misconfigured — Coinbase rejects private addresses, so dropping +// clientIp keeps the endpoint functional while we surface the misconfiguration +// via a warning log at the call site. +const internalAddr = proxyaddr.compile([ + "loopback", + "linklocal", + "uniquelocal", +]); + export const isLikelyInternalIp = (ip: string): boolean => { if (!ip) return true; - if (ip === "::1" || ip.startsWith("127.")) return true; - return /(?:^|:)(10\.|192\.168\.|169\.254\.|172\.(?:1[6-9]|2[0-9]|3[01])\.)/.test( - ip, - ); + return internalAddr(ip, 0); }; export const generateJWT = ({ @@ -91,13 +95,9 @@ export const fetchOnrampSessionToken = async ({ } let detail = ""; try { - detail = JSON.stringify(await res.json()); + detail = await res.text(); } catch { - try { - detail = await res.text(); - } catch { - // swallow - } + // body unreadable } return { data: { token: "" },