scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-08-28 05:27:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: security hardening for MCP OAuth callback server and auth flow

Browse source 
- Increase CSRF state token from 8 to 16 bytes (128-bit entropy per OAuth 2.1)
- Add Content-Security-Policy header to callback HTML response
- Close callback server on CSRF state validation failure (prevent resource leak)
- Make stopCallbackServer robust against already-closed servers
- Guard MCP_OAUTH_TEST_MODE check with VSCODE_PID to reduce test backdoor risk
This commit is contained in:
Roo Code 2026-03-20 00:00:25 +00:00
parent 3d0eb2775e
commit bde2bc375c
2 changed files with 22 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/services/mcp/McpOAuthClientProvider.ts
View file

@ -88,7 +88,8 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
const scopes: string[] = authServerMeta?.scopes_supported ?? []
// Generate a CSRF state token for the OAuth flow.
const state = Array.from(crypto.getRandomValues(new Uint8Array(8)))
// Use 16 bytes (128 bits) to meet OAuth 2.1 entropy recommendations.
const state = Array.from(crypto.getRandomValues(new Uint8Array(16)))
.map((b) => b.toString(16).padStart(2, "0"))
.join("")

25
src/services/mcp/utils/callbackServer.ts
View file

@ -21,8 +21,10 @@ export function startCallbackServer(
port: number
result: Promise<CallbackResult>
}> {
// In test mode, immediately resolve with mock data
if (process.env.MCP_OAUTH_TEST_MODE === "true") {
// In test mode (only active in VS Code e2e test runner), immediately resolve
// with mock data so the OAuth flow can complete without a real browser.
// This env var is set exclusively by the e2e test suite (mcp-oauth.test.ts).
if (process.env.MCP_OAUTH_TEST_MODE === "true" && process.env.VSCODE_PID) {
return new Promise((resolve) => {
const mockServer = http.createServer()
resolve({
@ -77,6 +79,10 @@ export function startCallbackServer(
// Verify state for CSRF protection
if (expectedState && state !== expectedState) {
res.writeHead(400, { "Content-Type": "text/html" })
// Close the server after rejecting -- no valid callback will follow.
res.on("finish", () => {
server.close()
})
res.end(`
<!DOCTYPE html>
<html>
@ -93,8 +99,12 @@ export function startCallbackServer(
return
}
// Send HTML response
res.writeHead(200, { "Content-Type": "text/html" })
// Send HTML response with CSP to restrict inline script to this page only.
res.writeHead(200, {
"Content-Type": "text/html",
"Content-Security-Policy":
"default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline'",
})
res.end(`
<!DOCTYPE html>
<html>
@ -192,6 +202,11 @@ export function startCallbackServer(
*/
export function stopCallbackServer(server: http.Server): Promise<void> {
return new Promise((resolve) => {
server.close(() => resolve())
try {
server.close(() => resolve())
} catch {
// Server may already be closed; resolve immediately.
resolve()
}
})
}