From 77f8e20bbe9fd7dd512c079fb670b9d18dc3dd61 Mon Sep 17 00:00:00 2001 From: MAC Date: Fri, 26 Jun 2026 00:35:33 +0100 Subject: [PATCH] fix(api): truncate primary-RPC error before logging it [BUG-007] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit withRpcFallback logged primaryErr.message raw via logger.warn — the one call site in this codebase that didn't wrap an error message with truncateErrorMessage() before logging it (every other site already does: health.ts, markets.ts, etc.). Solana RPC connection errors can embed the full endpoint URL in .message, and per .env.example, paid RPC providers (Helius/Alchemy) embed an API key directly in that URL. truncateErrorMessage only bounds length — it doesn't redact secrets — so this doesn't fully eliminate the exposure (a short enough message could still leak a key even truncated), but it brings this site in line with the same mitigation already applied everywhere else in the codebase instead of being the one outlier with a strictly larger exposure window. Added tests covering the no-fallback-configured passthrough, the normal fallback path, and the truncation itself. Verified the truncation test fails against the pre-fix code (273-char raw message logged) and passes against the fix (bounded to 120 chars + "..."). Co-authored-by: Claude Sonnet 4.6 --- src/utils/rpc-fallback.ts | 15 +++++- tests/utils/rpc-fallback.test.ts | 88 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 tests/utils/rpc-fallback.test.ts diff --git a/src/utils/rpc-fallback.ts b/src/utils/rpc-fallback.ts index 49f5b7f..07968a3 100644 --- a/src/utils/rpc-fallback.ts +++ b/src/utils/rpc-fallback.ts @@ -12,7 +12,7 @@ */ import type { Connection } from "@solana/web3.js"; -import { getFallbackConnection, createLogger } from "@percolator/shared"; +import { getFallbackConnection, createLogger, truncateErrorMessage } from "@percolator/shared"; import { withRpcTimeout } from "./rpc-timeout.js"; const logger = createLogger("api:rpc-fallback"); @@ -35,7 +35,18 @@ export async function withRpcFallback( logger.warn("Primary RPC failed, trying fallback", { operation, - error: primaryErr instanceof Error ? primaryErr.message : String(primaryErr), + // Truncated like every other error-log call site in this codebase + // (health.ts, markets.ts, etc.) — this was the one place that logged + // the raw, untruncated message. truncateErrorMessage only bounds + // length (it doesn't redact secrets), so this brings the exposure + // window in line with the rest of the codebase rather than fully + // eliminating it: RPC connection errors can embed the full endpoint + // URL, and paid providers (Helius/Alchemy) embed an API key in that + // URL, so a short error message could still leak it even truncated. + error: truncateErrorMessage( + primaryErr instanceof Error ? primaryErr.message : String(primaryErr), + 120, + ), }); return await withRpcTimeout( diff --git a/tests/utils/rpc-fallback.test.ts b/tests/utils/rpc-fallback.test.ts new file mode 100644 index 0000000..1fc7e61 --- /dev/null +++ b/tests/utils/rpc-fallback.test.ts @@ -0,0 +1,88 @@ +/** + * Tests for the RPC failover utility, including BUG-007: the primary-RPC + * failure log must be truncated like every other error-log call site in + * this codebase, not log the raw, potentially URL/API-key-bearing message. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const warnSpy = vi.fn(); + +vi.mock("@percolator/shared", () => ({ + createLogger: vi.fn(() => ({ + info: vi.fn(), + warn: warnSpy, + error: vi.fn(), + debug: vi.fn(), + })), + getFallbackConnection: vi.fn(() => ({})), + // Real implementation (mirrors @percolator/shared's actual behavior): + // bounds length only, does not redact secrets. + truncateErrorMessage: (msg: unknown, maxLength = 120) => { + const str = typeof msg === "string" ? msg : String(msg ?? ""); + return str.length > maxLength ? str.slice(0, maxLength) + "..." : str; + }, +})); + +async function loadWithRpcFallback() { + vi.resetModules(); + const mod = await import("../../src/utils/rpc-fallback.js"); + return mod.withRpcFallback; +} + +describe("withRpcFallback", () => { + beforeEach(() => { + warnSpy.mockClear(); + }); + + afterEach(() => { + delete process.env.FALLBACK_RPC_URL; + }); + + it("re-throws the original error unchanged when no fallback RPC is configured", async () => { + delete process.env.FALLBACK_RPC_URL; + const withRpcFallback = await loadWithRpcFallback(); + + const primaryErr = new Error("boom"); + const fn = vi.fn().mockRejectedValue(primaryErr); + + await expect(withRpcFallback(fn, {} as any, "test-op")).rejects.toThrow("boom"); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("falls back to the secondary connection and returns its result on primary failure", async () => { + process.env.FALLBACK_RPC_URL = "https://fallback.example.com"; + const withRpcFallback = await loadWithRpcFallback(); + + const fn = vi.fn() + .mockRejectedValueOnce(new Error("primary down")) + .mockResolvedValueOnce("fallback-result"); + + const result = await withRpcFallback(fn, {} as any, "test-op"); + expect(result).toBe("fallback-result"); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it("truncates a long primary-RPC error message before logging it, instead of logging it raw (BUG-007)", async () => { + process.env.FALLBACK_RPC_URL = "https://fallback.example.com"; + const withRpcFallback = await loadWithRpcFallback(); + + // Simulates an RPC connection error whose message embeds the full + // endpoint URL — paid providers (Helius/Alchemy) embed an API key in + // that URL per .env.example. + const longUrl = + "https://rpc.helius.xyz/?api-key=SUPER-SECRET-KEY-1234567890" + "x".repeat(200); + const primaryErr = new Error(`fetch failed: ${longUrl}`); + const fn = vi.fn() + .mockRejectedValueOnce(primaryErr) + .mockResolvedValueOnce("fallback-result"); + + const result = await withRpcFallback(fn, {} as any, "test-op"); + expect(result).toBe("fallback-result"); + + expect(warnSpy).toHaveBeenCalledTimes(1); + const loggedError = warnSpy.mock.calls[0][1].error as string; + // 120 chars + the "..." suffix truncateErrorMessage appends. + expect(loggedError.length).toBeLessThanOrEqual(123); + expect(loggedError).not.toBe(primaryErr.message); + }); +});