-
Notifications
You must be signed in to change notification settings - Fork 12.1k
docs: update MCP OAuth error handling documentation #28128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
/bonk review this PR. |
|
Now I have full context. Let me review this PR thoroughly. PR Review: docs: surface MCP OAuth errors to browser instead of silently swallowingOverviewThis PR updates two documentation files to reflect improved error handling from File-by-file review
Issues found1. XSS vulnerability in HTML template (medium severity) At <p style="color:#666;">${result.authError}</p>If the error message contains HTML/script tags, this is an XSS vector. The same pattern appears in the "Complete example" at line 356: `<script>alert('Authorization failed: ${result.authError}'); window.close();</script>`This second instance (line 356 in the "Complete example") was not updated by this PR - it still uses the old 2. Inconsistency between guide and API reference error examples The guide ( 3. The complete example at the bottom was not updated
return new Response(
`<script>alert('Authorization failed: ${result.authError}'); window.close();</script>`,
{ headers: { "content-type": "text/html" } },
);This contradicts the improved error handling documented earlier in the same page. The complete example should be updated to use the HTML error page approach (or at least not use
SummaryThe PR is a solid documentation update. The main actionable item is the complete example at the bottom of |
|
/bonk fix the examples. be precise. validate they are issues, but examples should be "safe by default" for users. |
|
Three fixes applied across 2 files: What was fixedXSS in all
Why this matters: These are copy-paste examples. Developers will use them as-is. The examples should be safe by default rather than requiring readers to notice they need to add escaping themselves. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/bonk consider more reliable ways to handle HTML escaping. don't leave users at risk when they copy our examples.
|
Here's what I changed and why: Problem with
|
Updates documentation to reflect improved OAuth error handling from cloudflare/agents#850. Changes: - Update customHandler examples to properly check result.authSuccess - Document MCPClientOAuthResult type with authSuccess and authError fields - Add configureOAuthCallback() API reference documentation - Document default error page behavior when no callback config is provided - Add error field to MCPServersState type for failed connections - Add examples showing proper error handling in React applications Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
cf0cf3d to
a85e174
Compare
|
/bonk re-review and make sure the latest commit didn't break our changes |
Use JSON.stringify + textContent for HTML examples to avoid injecting untrusted authError strings into the DOM via innerHTML. Add explicit content-type: text/plain to API reference plain-text error responses to prevent browser content sniffing.
SummaryThe latest commit ( What I fixed
Why this matters
The
|
elithrar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good now.
Summary
Updates MCP OAuth client documentation to reflect improved error handling from cloudflare/agents#850.
Changes
customHandlerexamples to properly checkresult.authSuccessand handle both success and error cases with user-friendly HTML error pagesconfigureOAuthCallback()API reference with complete documentation of parameters andMCPClientOAuthResulttypeerrorfield toMCPServersStatetype for displaying error messages in failed connection statesauthSuccessto avoid silently swallowing authentication errorsContext
MCP OAuth errors were previously silently swallowed in most configurations. The SDK now:
handleCallbackRequestthrough the normal response path (fixes unhandled throws after hibernation)errorRedirectorcustomHandleris configuredMCPClientOAuthResult.authErrorandserver.errorfieldsThis documentation update ensures developers understand how to properly handle and display these errors to their users.
Related PR