fix(server): sanitize error responses to prevent stack trace exposure

[cliffhall]

Summary

Resolves the 11 open CodeQL js/stack-trace-exposure alerts in server/src/index.ts. Each was a spot where a raw caught error object was handed to res.json(...), which can leak internal error details (including stack-trace-derived data) to clients.

The fix introduces a small sendErrorResponse(res, status, message) helper that returns a generic sanitized JSON body, and replaces every flagged call site with it. Full error objects continue to be logged server-side via the existing console.error calls, so debuggability is unchanged.

Alerts addressed

• #19 — line 512 (GET /mcp catch)
• #22 — line 599 (POST /mcp outer catch)
• #23 — line 630 (DELETE /mcp catch)
• #25 — line 739 (/stdio catch)
• #27 — line 790 (/sse 404 branch)
• #28 — line 794 (/sse ECONNREFUSED branch)
• #29 — line 797 (/sse generic 500)
• #30 — line 827 (/message catch)
• #43 — line 903 (/config catch)
• #51 — line 95 (sendProxiedUnauthorized fallback)
• #52 — line 548 (POST /mcp inner catch)

Incidental fix

The /sse handler's ECONNREFUSED branch was missing a return, so after sending its 500 it fell through to the generic Error in /sse route branch and attempted a second response on the already-sent headers. Added the missing return since the sanitization pass touched those lines anyway.

Behavior changes

• Clients no longer receive raw serialized Error objects on 4xx/5xx failures from /mcp, /stdio, /sse, /message, or /config. They now receive { "error": "<generic message>" }.
• sendProxiedUnauthorized no longer takes an error parameter — the upstream-capture path was already ignoring it, and the fallback path now returns a plain { error: "Unauthorized" } instead of JSON-encoding the caught exception.
• Server-side logs are unchanged — every site still logs the full error via console.error before responding.

Test plan

• npm run build-server passes (tsc)
• npm run prettier-fix clean
• Manually verify an error path (e.g. connect /mcp to an unreachable URL) returns { "error": "Internal server error" } rather than a serialized exception, and that console.error still logs full details server-side
• Manually verify OAuth 401 forwarding from an upstream MCP server still returns the captured WWW-Authenticate header and body (the non-fallback path in sendProxiedUnauthorized)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR addresses CodeQL js/stack-trace-exposure findings in the server by ensuring Express routes no longer serialize raw caught Error objects into HTTP JSON responses, reducing the risk of leaking stack traces/internal details to clients.

Changes:

• Added a sendErrorResponse(res, status, message) helper to return sanitized { error: "<message>" } responses.
• Replaced multiple res.status(...).json(error) call sites across /mcp, /stdio, /sse, /message, and /config with sanitized responses.
• Fixed a /sse error-path fallthrough by adding a missing return after responding in the ECONNREFUSED branch.

---

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

[olaservo]

Nice work — this is a clean, well-scoped security fix. The sendErrorResponse helper is consistent, the call sites are all updated correctly, and the incidental ECONNREFUSED missing-return fix is a real bug fix. Two things worth considering:

1. /fetch route catch block not updated

The /fetch catch block at server/src/index.ts:906-909 still exposes error.message to clients:

} catch (error) {
  res.status(500).json({
    error: error instanceof Error ? error.message : String(error),
  });
}

It also lacks a console.error call, so errors are silently swallowed. Since this PR's goal is to eliminate stack trace / internal detail exposure, it might be worth bringing this route in line too:

} catch (error) {
  console.error("Error in /fetch route:", error);
  sendErrorResponse(res, 500, "Internal server error");
}

2. JSON.stringify(error).includes("ECONNREFUSED") detection is fragile

At line 809, the ECONNREFUSED check uses JSON.stringify(error). Since Error properties are non-enumerable, JSON.stringify(new Error("ECONNREFUSED")) produces "{}" — the check only works when the error happens to have enumerable properties (like a cause field). Since you already touched these lines to add the return fix, it might be worth switching to something more reliable:

} else if (
  error instanceof Error &&
  (error.message.includes("ECONNREFUSED") ||
    (error.cause && String(error.cause).includes("ECONNREFUSED")))
) {

This would also be more consistent with mcpProxy.ts:36 which uses JSON.stringify(error.cause) rather than the whole error.

Neither of these are blockers — happy to approve as-is if you'd prefer to address them in a follow-up.

---

This review was conducted with assistance from Claude Code.

[cliffhall]

Thanks @olaservo — both great catches. Addressed in f08dc65:

1. /fetch route

Agreed, this one slipped through because it was already wrapping error.message rather than passing the raw object, so it didn't trigger the CodeQL rule — but it's the same class of leak and the missing console.error meant failures were invisible. Now logs and returns a sanitized response, matching the pattern used elsewhere.

2. ECONNREFUSED detection

Good point about JSON.stringify on a bare Error producing "{}". Switched to the suggested form:

} else if (
  error instanceof Error &&
  (error.message.includes("ECONNREFUSED") ||
    (error.cause && String(error.cause).includes("ECONNREFUSED")))
) {

Using String(error.cause) rather than JSON.stringify(error.cause) since cause is also typically an Error with non-enumerable fields — toString() gives "Error: connect ECONNREFUSED ..." reliably. Not touching mcpProxy.ts:36 in this PR to keep the scope focused on the CodeQL alerts, but the same fragility exists there and is worth a follow-up.

[olaservo]

Both feedback items addressed cleanly in f08dc65:

1. /fetch route now logs via console.error and returns a sanitized response instead of leaking error.message.
2. ECONNREFUSED detection replaced with proper error instanceof Error check against error.message and String(error.cause) — no more fragile JSON.stringify(error).

Verified that all remaining error.message usages are server-side only (boolean checks and console.error calls). CI green. LGTM.

This review was conducted with assistance from Claude Code.