fix(mcp): escape OAuth callback errors (#32242)
This commit is contained in:
@@ -26,6 +26,15 @@ const HTML_SUCCESS = `<!DOCTYPE html>
|
|||||||
</body>
|
</body>
|
||||||
</html>`
|
</html>`
|
||||||
|
|
||||||
|
function escapeHtml(value: string) {
|
||||||
|
return value
|
||||||
|
.replaceAll("&", "&")
|
||||||
|
.replaceAll("<", "<")
|
||||||
|
.replaceAll(">", ">")
|
||||||
|
.replaceAll('"', """)
|
||||||
|
.replaceAll("'", "'")
|
||||||
|
}
|
||||||
|
|
||||||
const HTML_ERROR = (error: string) => `<!DOCTYPE html>
|
const HTML_ERROR = (error: string) => `<!DOCTYPE html>
|
||||||
<html>
|
<html>
|
||||||
<head>
|
<head>
|
||||||
@@ -42,7 +51,7 @@ const HTML_ERROR = (error: string) => `<!DOCTYPE html>
|
|||||||
<div class="container">
|
<div class="container">
|
||||||
<h1>Authorization Failed</h1>
|
<h1>Authorization Failed</h1>
|
||||||
<p>An error occurred during authorization.</p>
|
<p>An error occurred during authorization.</p>
|
||||||
<div class="error">${error}</div>
|
<div class="error">${escapeHtml(error)}</div>
|
||||||
</div>
|
</div>
|
||||||
</body>
|
</body>
|
||||||
</html>`
|
</html>`
|
||||||
@@ -87,7 +96,7 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
|||||||
// Enforce state parameter presence
|
// Enforce state parameter presence
|
||||||
if (!state) {
|
if (!state) {
|
||||||
const errorMsg = "Missing required state parameter - potential CSRF attack"
|
const errorMsg = "Missing required state parameter - potential CSRF attack"
|
||||||
res.writeHead(400, { "Content-Type": "text/html" })
|
res.writeHead(400, { "Content-Type": "text/html; charset=utf-8" })
|
||||||
res.end(HTML_ERROR(errorMsg))
|
res.end(HTML_ERROR(errorMsg))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -101,13 +110,13 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
|||||||
cleanupStateIndex(state)
|
cleanupStateIndex(state)
|
||||||
pending.reject(new Error(errorMsg))
|
pending.reject(new Error(errorMsg))
|
||||||
}
|
}
|
||||||
res.writeHead(200, { "Content-Type": "text/html" })
|
res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" })
|
||||||
res.end(HTML_ERROR(errorMsg))
|
res.end(HTML_ERROR(errorMsg))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!code) {
|
if (!code) {
|
||||||
res.writeHead(400, { "Content-Type": "text/html" })
|
res.writeHead(400, { "Content-Type": "text/html; charset=utf-8" })
|
||||||
res.end(HTML_ERROR("No authorization code provided"))
|
res.end(HTML_ERROR("No authorization code provided"))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -115,7 +124,7 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
|||||||
// Validate state parameter
|
// Validate state parameter
|
||||||
if (!pendingAuths.has(state)) {
|
if (!pendingAuths.has(state)) {
|
||||||
const errorMsg = "Invalid or expired state parameter - potential CSRF attack"
|
const errorMsg = "Invalid or expired state parameter - potential CSRF attack"
|
||||||
res.writeHead(400, { "Content-Type": "text/html" })
|
res.writeHead(400, { "Content-Type": "text/html; charset=utf-8" })
|
||||||
res.end(HTML_ERROR(errorMsg))
|
res.end(HTML_ERROR(errorMsg))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -127,7 +136,7 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
|||||||
cleanupStateIndex(state)
|
cleanupStateIndex(state)
|
||||||
pending.resolve(code)
|
pending.resolve(code)
|
||||||
|
|
||||||
res.writeHead(200, { "Content-Type": "text/html" })
|
res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" })
|
||||||
res.end(HTML_SUCCESS)
|
res.end(HTML_SUCCESS)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -31,4 +31,30 @@ describe("McpOAuthCallback.ensureRunning", () => {
|
|||||||
await McpOAuthCallback.ensureRunning("http://127.0.0.1:18000/custom/callback")
|
await McpOAuthCallback.ensureRunning("http://127.0.0.1:18000/custom/callback")
|
||||||
expect(McpOAuthCallback.isRunning()).toBe(true)
|
expect(McpOAuthCallback.isRunning()).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("escapes provider error markup in callback HTML", async () => {
|
||||||
|
const redirectUri = "http://127.0.0.1:18001/custom/callback"
|
||||||
|
await McpOAuthCallback.ensureRunning(redirectUri)
|
||||||
|
|
||||||
|
const error = `<script>alert("xss" & 'more')</script>`
|
||||||
|
const response = await fetch(
|
||||||
|
`${redirectUri}?state=test&error=access_denied&error_description=${encodeURIComponent(error)}`,
|
||||||
|
)
|
||||||
|
const body = await response.text()
|
||||||
|
|
||||||
|
expect(response.headers.get("content-type")).toBe("text/html; charset=utf-8")
|
||||||
|
expect(body).toContain("<script>alert("xss" & 'more')</script>")
|
||||||
|
expect(body).not.toContain(error)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("keeps normal provider errors readable", async () => {
|
||||||
|
const redirectUri = "http://127.0.0.1:18002/custom/callback"
|
||||||
|
await McpOAuthCallback.ensureRunning(redirectUri)
|
||||||
|
|
||||||
|
const response = await fetch(
|
||||||
|
`${redirectUri}?state=test&error=access_denied&error_description=${encodeURIComponent("The user denied access")}`,
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(await response.text()).toContain('<div class="error">The user denied access</div>')
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user