From 7ed2723da0de3875159ec7703c0aa9224bc9555c Mon Sep 17 00:00:00 2001 From: James Liounis Date: Tue, 19 May 2026 13:53:53 +0000 Subject: [PATCH] security: sanitize Perplexity API error messages (CWE-200) Re-implementation of #103. Credit: @sebastiondev for the original analysis and patch. Problem ------- makeApiRequest, performChatCompletion, and performSearch previously embedded raw upstream details into thrown Error messages: throw new Error(`Network error while calling Perplexity API: ${error}`); throw new Error(`Perplexity API error: ${status} ${statusText}\n${errorText}`); throw new Error(`Failed to parse JSON response from Perplexity API: ${error}`); The MCP SDK returns thrown tool errors verbatim to remote callers in `result.content[].text`. Because this server can also run as an unauthenticated HTTP service bound to 0.0.0.0, a remote MCP client could trigger upstream failures and read back: - the upstream response body (provider internal traces, account hints) - raw exception text from the network layer (proxy/DNS details, etc.) - raw JSON-parse exception text (may include partial payloads) That is CWE-200 (Exposure of Sensitive Information to an Unauthorized Actor): medium severity, reachable via the HTTP transport, mitigable without losing operator visibility. Fix --- Separate operator diagnostics from caller-visible errors: - The full upstream body, parser exception, and network exception text are now logged server-side via the existing structured logger (writes to stderr; survives STDIO transport). - The Error that bubbles up to the MCP SDK now contains only a stable, generic, non-secret message: "Perplexity API error: " "Network error while calling Perplexity API" "Failed to parse JSON response from Perplexity API" "Failed to parse JSON response from Perplexity Search API" - Timeout and validation errors are unchanged (they were already sanitized). Tests ----- - Updated "should handle error text parse failures" to assert the new sanitized output and the absence of the underlying exception text. - Added 3 new CWE-200 regression tests: * JSON parse error must not leak parser exception text * Network error must not leak network exception text * Upstream 4xx/5xx must not leak the response body - npm test: 81 passed / 81 (was 78 on main). - npm run build: clean. Co-authored-by: @sebastiondev --- src/index.test.ts | 66 +++++++++++++++++++++++++++++++++++++++++++++-- src/server.ts | 23 ++++++++++++++--- 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 21897b8..b715a45 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -418,7 +418,7 @@ describe("Perplexity MCP Server", () => { ); }); - it("should handle error text parse failures", async () => { + it("should handle error text parse failures with a generic message", async () => { global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 500, @@ -430,8 +430,70 @@ describe("Perplexity MCP Server", () => { const messages = [{ role: "user", content: "test" }]; + // Sanitized: the client should only see the status + statusText, never + // the underlying "Unable to parse error response" or raw exception text. await expect(performChatCompletion(messages)).rejects.toThrow( - "Unable to parse error response" + "Perplexity API error: 500 Internal Server Error" + ); + await expect(performChatCompletion(messages)).rejects.not.toThrow( + "Cannot read error" + ); + }); + + // CWE-200 regression: thrown error must not contain JSON-parser exception text. + it("should not expose JSON parse error details", async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => { + throw new Error("Invalid JSON containing secret_token=abc123"); + }, + } as unknown as Response); + + const messages = [{ role: "user", content: "test" }]; + + await expect(performChatCompletion(messages)).rejects.toThrow( + "Failed to parse JSON response from Perplexity API" + ); + await expect(performChatCompletion(messages)).rejects.not.toThrow( + "secret_token=abc123" + ); + }); + + // CWE-200 regression: thrown error must not contain network exception text. + it("should not expose network error details", async () => { + global.fetch = vi.fn().mockRejectedValue( + new Error("Network failure with credential=private-token") + ); + + const messages = [{ role: "user", content: "test" }]; + + await expect(performChatCompletion(messages)).rejects.toThrow( + "Network error while calling Perplexity API" + ); + await expect(performChatCompletion(messages)).rejects.not.toThrow( + "credential=private-token" + ); + }); + + // CWE-200 regression: thrown error must not contain the upstream response body. + it("should not expose upstream error response details", async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 500, + statusText: "Internal Server Error", + text: async () => "internal_trace=abc123; account=private-tier", + } as Response); + + const messages = [{ role: "user", content: "test" }]; + + await expect(performChatCompletion(messages)).rejects.toThrow( + "Perplexity API error: 500 Internal Server Error" + ); + await expect(performChatCompletion(messages)).rejects.not.toThrow( + "internal_trace=abc123" + ); + await expect(performChatCompletion(messages)).rejects.not.toThrow( + "private-tier" ); }); diff --git a/src/server.ts b/src/server.ts index d649ccb..36a3f8b 100644 --- a/src/server.ts +++ b/src/server.ts @@ -10,6 +10,7 @@ import type { UndiciRequestOptions } from "./types.js"; import { ChatCompletionResponseSchema, SearchResponseSchema } from "./validation.js"; +import { logger } from "./logger.js"; const PERPLEXITY_API_KEY = process.env.PERPLEXITY_API_KEY; const PERPLEXITY_BASE_URL = process.env.PERPLEXITY_BASE_URL || "https://api.perplexity.ai"; @@ -99,7 +100,10 @@ async function makeApiRequest( if (error instanceof Error && error.name === "AbortError") { throw new Error(`Request timeout: Perplexity API did not respond within ${TIMEOUT_MS}ms. Consider increasing PERPLEXITY_TIMEOUT_MS.`); } - throw new Error(`Network error while calling Perplexity API: ${error}`); + // CWE-200: do not leak raw exception text to MCP clients. Log server-side + // for operator diagnostics, return a stable generic message to the caller. + logger.error("Network error while calling Perplexity API", { error: String(error) }); + throw new Error("Network error while calling Perplexity API"); } clearTimeout(timeoutId); @@ -110,8 +114,15 @@ async function makeApiRequest( } catch (parseError) { errorText = "Unable to parse error response"; } + // CWE-200: keep upstream response body out of the client-facing error. + // Operators still see the full body in structured server logs. + logger.error("Perplexity API error", { + status: response.status, + statusText: response.statusText, + body: errorText, + }); throw new Error( - `Perplexity API error: ${response.status} ${response.statusText}\n${errorText}` + `Perplexity API error: ${response.status} ${response.statusText}` ); } @@ -228,7 +239,9 @@ export async function performChatCompletion( throw new Error("Invalid API response: missing or empty choices array"); } } - throw new Error(`Failed to parse JSON response from Perplexity API: ${error}`); + // CWE-200: do not leak parser exception text (may include partial body). + logger.error("Failed to parse JSON response from Perplexity API", { error: String(error) }); + throw new Error("Failed to parse JSON response from Perplexity API"); } const firstChoice = data.choices[0]; @@ -292,7 +305,9 @@ export async function performSearch( const json = await response.json(); data = SearchResponseSchema.parse(json); } catch (error) { - throw new Error(`Failed to parse JSON response from Perplexity Search API: ${error}`); + // CWE-200: do not leak parser exception text (may include partial body). + logger.error("Failed to parse JSON response from Perplexity Search API", { error: String(error) }); + throw new Error("Failed to parse JSON response from Perplexity Search API"); } return formatSearchResults(data);