Skip to content

Commit 4df1191

Browse files
cliffhallclaudeolaservo
authored
fix(server): sanitize error responses to prevent stack trace exposure (#1187)
* fix(server): sanitize error responses to prevent stack trace exposure Replace raw error objects passed to res.json() with generic sanitized messages so internal error details are not leaked to clients. Full errors continue to be logged server-side via console.error. Also adds a missing return in the /sse ECONNREFUSED branch, which previously fell through and attempted a second response after headers had already been sent. Resolves CodeQL js/stack-trace-exposure alerts in server/src/index.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(server): address review feedback on error sanitization - Sanitize /fetch route catch block: previously leaked error.message to clients and silently swallowed errors with no server-side logging. Now logs via console.error and returns a generic sanitized response. - Replace JSON.stringify(error).includes("ECONNREFUSED") in the /sse handler with a check against error.message and String(error.cause). Error fields are non-enumerable so JSON.stringify(new Error(...)) produces "{}" — the old check only worked by accident when the error happened to carry enumerable properties. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Ola Hungerford <olahungerford@gmail.com>
1 parent 0d7757e commit 4df1191

1 file changed

Lines changed: 45 additions & 20 deletions

File tree

server/src/index.ts

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,26 @@ const is401Error = (error: unknown): boolean => {
7272
return false;
7373
};
7474

75+
/**
76+
* Sends a sanitized JSON error response to the client without exposing
77+
* stack traces or internal error details. The full error is expected to
78+
* have already been logged server-side via console.error.
79+
*/
80+
const sendErrorResponse = (
81+
res: express.Response,
82+
status: number,
83+
message: string,
84+
) => {
85+
res.status(status).json({ error: message });
86+
};
87+
7588
/**
7689
* Prefer forwarding the upstream MCP 401 (WWW-Authenticate + body) so the browser
77-
* matches direct-mode OAuth behavior. Falls back to JSON-encoding `error` if unknown.
90+
* matches direct-mode OAuth behavior. Falls back to a generic 401 JSON response
91+
* when no upstream details were captured, to avoid leaking stack traces.
7892
*/
7993
const sendProxiedUnauthorized = (
8094
res: express.Response,
81-
error: unknown,
8295
headerHolder?: ProxyHeaderHolder,
8396
) => {
8497
const captured = headerHolder?.lastUpstream401;
@@ -92,7 +105,7 @@ const sendProxiedUnauthorized = (
92105
delete headerHolder.lastUpstream401;
93106
return;
94107
}
95-
res.status(401).json(error);
108+
sendErrorResponse(res, 401, "Unauthorized");
96109
};
97110

98111
// Function to get HTTP headers.
@@ -509,7 +522,7 @@ app.get(
509522
}
510523
} catch (error) {
511524
console.error("Error in /mcp route:", error);
512-
res.status(500).json(error);
525+
sendErrorResponse(res, 500, "Internal server error");
513526
}
514527
},
515528
);
@@ -545,7 +558,7 @@ app.post(
545558
}
546559
} catch (error) {
547560
console.error("Error in /mcp route:", error);
548-
res.status(500).json(error);
561+
sendErrorResponse(res, 500, "Internal server error");
549562
}
550563
} else {
551564
console.log("New StreamableHttp connection request");
@@ -592,11 +605,11 @@ app.post(
592605
"Received 401 Unauthorized from MCP server:",
593606
error instanceof Error ? error.message : error,
594607
);
595-
sendProxiedUnauthorized(res, error, streamableHeaderHolder);
608+
sendProxiedUnauthorized(res, streamableHeaderHolder);
596609
return;
597610
}
598611
console.error("Error in /mcp POST route:", error);
599-
res.status(500).json(error);
612+
sendErrorResponse(res, 500, "Internal server error");
600613
}
601614
}
602615
},
@@ -627,7 +640,7 @@ app.delete(
627640
res.status(200).end();
628641
} catch (error) {
629642
console.error("Error in /mcp route:", error);
630-
res.status(500).json(error);
643+
sendErrorResponse(res, 500, "Internal server error");
631644
}
632645
}
633646
},
@@ -732,11 +745,11 @@ app.get(
732745
console.error(
733746
"Received 401 Unauthorized from MCP server. Authentication failure.",
734747
);
735-
sendProxiedUnauthorized(res, error, undefined);
748+
sendProxiedUnauthorized(res, undefined);
736749
return;
737750
}
738751
console.error("Error in /stdio route:", error);
739-
res.status(500).json(error);
752+
sendErrorResponse(res, 500, "Internal server error");
740753
}
741754
},
742755
);
@@ -781,20 +794,33 @@ app.get(
781794
console.error(
782795
"Received 401 Unauthorized from MCP server. Authentication failure.",
783796
);
784-
sendProxiedUnauthorized(res, error, sseHeaderHolder);
797+
sendProxiedUnauthorized(res, sseHeaderHolder);
785798
return;
786799
} else if (error instanceof SseError && error.code === 404) {
787800
console.error(
788801
"Received 404 not found from MCP server. Does the MCP server support SSE?",
789802
);
790-
res.status(404).json(error);
803+
sendErrorResponse(
804+
res,
805+
404,
806+
"MCP server returned 404. Does it support SSE?",
807+
);
791808
return;
792-
} else if (JSON.stringify(error).includes("ECONNREFUSED")) {
809+
} else if (
810+
error instanceof Error &&
811+
(error.message.includes("ECONNREFUSED") ||
812+
(error.cause && String(error.cause).includes("ECONNREFUSED")))
813+
) {
793814
console.error("Connection refused. Is the MCP server running?");
794-
res.status(500).json(error);
815+
sendErrorResponse(
816+
res,
817+
500,
818+
"Connection refused. Is the MCP server running?",
819+
);
820+
return;
795821
}
796822
console.error("Error in /sse route:", error);
797-
res.status(500).json(error);
823+
sendErrorResponse(res, 500, "Internal server error");
798824
}
799825
},
800826
);
@@ -824,7 +850,7 @@ app.post(
824850
await transport.handlePostMessage(req, res);
825851
} catch (error) {
826852
console.error("Error in /message route:", error);
827-
res.status(500).json(error);
853+
sendErrorResponse(res, 500, "Internal server error");
828854
}
829855
},
830856
);
@@ -882,9 +908,8 @@ app.post(
882908
body: responseBody,
883909
});
884910
} catch (error) {
885-
res.status(500).json({
886-
error: error instanceof Error ? error.message : String(error),
887-
});
911+
console.error("Error in /fetch route:", error);
912+
sendErrorResponse(res, 500, "Internal server error");
888913
}
889914
},
890915
);
@@ -900,7 +925,7 @@ app.get("/config", originValidationMiddleware, authMiddleware, (req, res) => {
900925
});
901926
} catch (error) {
902927
console.error("Error in /config route:", error);
903-
res.status(500).json(error);
928+
sendErrorResponse(res, 500, "Internal server error");
904929
}
905930
});
906931

0 commit comments

Comments
 (0)