Sanitising URLs

This commit is contained in:
Glen Maddern 2025-06-26 20:47:51 +10:00
commit 960631bb74
5 changed files with 30 additions and 10 deletions

View file

@ -29,7 +29,8 @@
"deploy:site": "npx wrangler deploy" "deploy:site": "npx wrangler deploy"
}, },
"dependencies": { "dependencies": {
"@modelcontextprotocol/sdk": "^1.11.0" "@modelcontextprotocol/sdk": "^1.11.0",
"strict-url-sanitise": "^0.0.1"
}, },
"devDependencies": { "devDependencies": {
"@axodotdev/oranda": "^0.6.5", "@axodotdev/oranda": "^0.6.5",

8
pnpm-lock.yaml generated
View file

@ -11,6 +11,9 @@ importers:
'@modelcontextprotocol/sdk': '@modelcontextprotocol/sdk':
specifier: ^1.11.0 specifier: ^1.11.0
version: 1.11.0 version: 1.11.0
strict-url-sanitise:
specifier: ^0.0.1
version: 0.0.1
devDependencies: devDependencies:
'@axodotdev/oranda': '@axodotdev/oranda':
specifier: ^0.6.5 specifier: ^0.6.5
@ -1346,6 +1349,9 @@ packages:
resolution: {integrity: sha512-KXDYZ9dszj6bzvnEMRYvxgeTHU74QBFL54XKtP3nyMuJ81CFYtABZ3bAzL2EdFUaEwJOBOgENyFj3R7oTzDyyw==} resolution: {integrity: sha512-KXDYZ9dszj6bzvnEMRYvxgeTHU74QBFL54XKtP3nyMuJ81CFYtABZ3bAzL2EdFUaEwJOBOgENyFj3R7oTzDyyw==}
engines: {node: '>=4', npm: '>=6'} engines: {node: '>=4', npm: '>=6'}
strict-url-sanitise@0.0.1:
resolution: {integrity: sha512-nuFtF539K8jZg3FjaWH/L8eocCR6gegz5RDOsaWxfdbF5Jqr2VXWxZayjTwUzsWJDC91k2EbnJXp6FuWW+Z4hg==}
string-width@4.2.3: string-width@4.2.3:
resolution: {integrity: sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==} resolution: {integrity: sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==}
engines: {node: '>=8'} engines: {node: '>=8'}
@ -2668,6 +2674,8 @@ snapshots:
stoppable@1.1.0: {} stoppable@1.1.0: {}
strict-url-sanitise@0.0.1: {}
string-width@4.2.3: string-width@4.2.3:
dependencies: dependencies:
emoji-regex: 8.0.0 emoji-regex: 8.0.0

View file

@ -1,6 +1,7 @@
// browser-provider.ts // browser-provider.ts
import { OAuthClientInformation, OAuthMetadata, OAuthTokens, OAuthClientMetadata } from '@modelcontextprotocol/sdk/shared/auth.js' import { OAuthClientInformation, OAuthMetadata, OAuthTokens, OAuthClientMetadata } from '@modelcontextprotocol/sdk/shared/auth.js'
import { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js' import { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'
import { sanitizeUrl } from 'strict-url-sanitise'
// Assuming StoredState is defined in ./types.js and includes fields for provider options // Assuming StoredState is defined in ./types.js and includes fields for provider options
import { StoredState } from './types.js' // Adjust path if necessary import { StoredState } from './types.js' // Adjust path if necessary
@ -29,15 +30,16 @@ export class BrowserOAuthClientProvider implements OAuthClientProvider {
this.serverUrlHash = this.hashString(serverUrl) this.serverUrlHash = this.hashString(serverUrl)
this.clientName = options.clientName || 'MCP Browser Client' this.clientName = options.clientName || 'MCP Browser Client'
this.clientUri = options.clientUri || (typeof window !== 'undefined' ? window.location.origin : '') this.clientUri = options.clientUri || (typeof window !== 'undefined' ? window.location.origin : '')
this.callbackUrl = this.callbackUrl = sanitizeUrl(
options.callbackUrl || options.callbackUrl ||
(typeof window !== 'undefined' ? new URL('/oauth/callback', window.location.origin).toString() : '/oauth/callback') (typeof window !== 'undefined' ? new URL('/oauth/callback', window.location.origin).toString() : '/oauth/callback'),
)
} }
// --- SDK Interface Methods --- // --- SDK Interface Methods ---
get redirectUrl(): string { get redirectUrl(): string {
return this.callbackUrl return sanitizeUrl(this.callbackUrl)
} }
get clientMetadata(): OAuthClientMetadata { get clientMetadata(): OAuthClientMetadata {
@ -143,13 +145,16 @@ export class BrowserOAuthClientProvider implements OAuthClientProvider {
authorizationUrl.searchParams.set('state', state) authorizationUrl.searchParams.set('state', state)
const authUrlString = authorizationUrl.toString() const authUrlString = authorizationUrl.toString()
// Sanitize the authorization URL to prevent XSS attacks
const sanitizedAuthUrl = sanitizeUrl(authUrlString)
// Persist the exact auth URL in case the popup fails and manual navigation is needed // Persist the exact auth URL in case the popup fails and manual navigation is needed
localStorage.setItem(this.getKey('last_auth_url'), authUrlString) localStorage.setItem(this.getKey('last_auth_url'), sanitizedAuthUrl)
// Attempt to open the popup // Attempt to open the popup
const popupFeatures = 'width=600,height=700,resizable=yes,scrollbars=yes,status=yes' // Make configurable if needed const popupFeatures = 'width=600,height=700,resizable=yes,scrollbars=yes,status=yes' // Make configurable if needed
try { try {
const popup = window.open(authUrlString, `mcp_auth_${this.serverUrlHash}`, popupFeatures) const popup = window.open(sanitizedAuthUrl, `mcp_auth_${this.serverUrlHash}`, popupFeatures)
if (!popup || popup.closed || typeof popup.closed === 'undefined') { if (!popup || popup.closed || typeof popup.closed === 'undefined') {
console.warn( console.warn(
@ -175,7 +180,8 @@ export class BrowserOAuthClientProvider implements OAuthClientProvider {
* Retrieves the last URL passed to `redirectToAuthorization`. Useful for manual fallback. * Retrieves the last URL passed to `redirectToAuthorization`. Useful for manual fallback.
*/ */
getLastAttemptedAuthUrl(): string | null { getLastAttemptedAuthUrl(): string | null {
return localStorage.getItem(this.getKey('last_auth_url')) const storedUrl = localStorage.getItem(this.getKey('last_auth_url'))
return storedUrl ? sanitizeUrl(storedUrl) : null
} }
clearStorage(): number { clearStorage(): number {

View file

@ -6,6 +6,7 @@ import { SSEClientTransport, SSEClientTransportOptions } from '@modelcontextprot
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' // Added import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' // Added
import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { Client } from '@modelcontextprotocol/sdk/client/index.js'
import { auth, UnauthorizedError, OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js' import { auth, UnauthorizedError, OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'
import { sanitizeUrl } from 'strict-url-sanitise'
import { BrowserOAuthClientProvider } from '../auth/browser-provider.js' // Adjust path import { BrowserOAuthClientProvider } from '../auth/browser-provider.js' // Adjust path
import { assert } from '../utils/assert.js' // Adjust path import { assert } from '../utils/assert.js' // Adjust path
import type { UseMcpOptions, UseMcpResult } from './types.js' // Adjust path import type { UseMcpOptions, UseMcpResult } from './types.js' // Adjust path
@ -23,7 +24,9 @@ export function useMcp(options: UseMcpOptions): UseMcpResult {
url, url,
clientName, clientName,
clientUri, clientUri,
callbackUrl = typeof window !== 'undefined' ? new URL('/oauth/callback', window.location.origin).toString() : '/oauth/callback', callbackUrl = typeof window !== 'undefined'
? sanitizeUrl(new URL('/oauth/callback', window.location.origin).toString())
: '/oauth/callback',
storageKeyPrefix = 'mcp:auth', storageKeyPrefix = 'mcp:auth',
clientConfig = {}, clientConfig = {},
customHeaders = {}, customHeaders = {},
@ -189,7 +192,9 @@ export function useMcp(options: UseMcpOptions): UseMcpResult {
authProvider: authProviderRef.current, authProvider: authProviderRef.current,
requestInit: { headers: customHeaders }, requestInit: { headers: customHeaders },
} }
const targetUrl = new URL(url) // Sanitize the URL to prevent XSS attacks from malicious server URLs
const sanitizedUrl = sanitizeUrl(url)
const targetUrl = new URL(sanitizedUrl)
if (transportType === 'http') { if (transportType === 'http') {
transportInstance = new StreamableHTTPClientTransport(targetUrl, commonOptions) transportInstance = new StreamableHTTPClientTransport(targetUrl, commonOptions)