Skip to content

Fix/validate urls for http/https scheme only in auth flow#740

Merged
cliffhall merged 3 commits intomainfrom
fix/validate-urls
Aug 21, 2025
Merged

Fix/validate urls for http/https scheme only in auth flow#740
cliffhall merged 3 commits intomainfrom
fix/validate-urls

Conversation

@jenn-newton
Copy link
Copy Markdown
Contributor

Motivation and Context

We recently added scheme validation to authorization urls. #687
This pulls the validation out into a shared helper and applies it in a few more places:

  1. AuthDebugger.tsx - Fixed a direct window.location.href redirect that wasn't validating URLs
  2. OAuthFlowProgress.tsx - Fixed two vulnerable paths:
    - A window.open() call without validation
    - A direct anchor href attribute without validation

How Has This Been Tested?

With the oauth debugger and the typescript SDK (thanks @cliffhall 🙌)

Breaking Changes

I don't think this should break anything, just validate that we're using the expected schemes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

jenn-newton and others added 3 commits August 20, 2025 10:08
- Create shared validateRedirectUrl utility to centralize URL validation
- Only allow HTTP and HTTPS protocols in OAuth authorization URLs
- Fix three vulnerable redirect paths that bypassed validation:
  - AuthDebugger.tsx: Direct window.location.href redirect
  - OAuthFlowProgress.tsx: window.open() without validation
  - OAuthFlowProgress.tsx: Direct anchor href attribute
- Add comprehensive test coverage for URL validation
- Update existing auth.ts to use shared validation utility

This prevents malicious MCP servers from injecting javascript: or data:
protocol URLs that would execute XSS attacks when users are redirected
to the authorization endpoint.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@jenn-newton jenn-newton requested a review from cliffhall August 21, 2025 15:43
@github-actions
Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

✅  24 passed

Details

24 tests across 3 suites
 35.1 seconds
 a018a6d
ℹ️  Test Environment: Ubuntu Latest, Node.js v22.18.0
Browsers: Chromium, Firefox

📊 View Detailed HTML Report (download artifacts)

Copy link
Copy Markdown
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

Tested locally, and the OAuth flow problems identified earlier have been fixed with the provided change. Code looks good.

@cliffhall cliffhall merged commit 8a5a4e5 into main Aug 21, 2025
8 checks passed
@cliffhall cliffhall deleted the fix/validate-urls branch August 21, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants