fix: prevent command injection in example URL opening (#1553)
Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
This commit is contained in:
parent
c4ee360aac
commit
0096e4a1d3
3 changed files with 134 additions and 17 deletions
|
|
@ -36,12 +36,13 @@
|
|||
"dependencies": {
|
||||
"@modelcontextprotocol/client": "workspace:^",
|
||||
"ajv": "catalog:runtimeShared",
|
||||
"open": "^11.0.0",
|
||||
"zod": "catalog:runtimeShared"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@modelcontextprotocol/eslint-config": "workspace:^",
|
||||
"@modelcontextprotocol/examples-shared": "workspace:^",
|
||||
"@modelcontextprotocol/tsconfig": "workspace:^",
|
||||
"@modelcontextprotocol/eslint-config": "workspace:^",
|
||||
"@modelcontextprotocol/vitest-config": "workspace:^",
|
||||
"tsdown": "catalog:devTools"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@
|
|||
// URL elicitation allows servers to prompt the end-user to open a URL in their browser
|
||||
// to collect sensitive information.
|
||||
|
||||
import { exec } from 'node:child_process';
|
||||
import { createServer } from 'node:http';
|
||||
import { createInterface } from 'node:readline';
|
||||
|
||||
|
|
@ -29,6 +28,7 @@ import {
|
|||
UnauthorizedError,
|
||||
UrlElicitationRequiredError
|
||||
} from '@modelcontextprotocol/client';
|
||||
import open from 'open';
|
||||
|
||||
import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js';
|
||||
|
||||
|
|
@ -272,15 +272,25 @@ async function elicitationLoop(): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
async function openBrowser(url: string): Promise<void> {
|
||||
const command = `open "${url}"`;
|
||||
const ALLOWED_SCHEMES = new Set(['http:', 'https:']);
|
||||
|
||||
exec(command, error => {
|
||||
if (error) {
|
||||
console.error(`Failed to open browser: ${error.message}`);
|
||||
console.log(`Please manually open: ${url}`);
|
||||
async function openBrowser(url: string): Promise<void> {
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
if (!ALLOWED_SCHEMES.has(parsed.protocol)) {
|
||||
console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`);
|
||||
return;
|
||||
}
|
||||
});
|
||||
} catch {
|
||||
console.error(`Invalid URL: ${url}`);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await open(url);
|
||||
} catch {
|
||||
console.log(`Please manually open: ${url}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
import { exec } from 'node:child_process';
|
||||
import { createServer } from 'node:http';
|
||||
import { createInterface } from 'node:readline';
|
||||
import { URL } from 'node:url';
|
||||
|
|
@ -13,6 +12,7 @@ import {
|
|||
StreamableHTTPClientTransport,
|
||||
UnauthorizedError
|
||||
} from '@modelcontextprotocol/client';
|
||||
import open from 'open';
|
||||
|
||||
import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js';
|
||||
|
||||
|
|
@ -49,17 +49,27 @@ class InteractiveOAuthClient {
|
|||
/**
|
||||
* Opens the authorization URL in the user's default browser
|
||||
*/
|
||||
private static readonly ALLOWED_SCHEMES = new Set(['http:', 'https:']);
|
||||
|
||||
private async openBrowser(url: string): Promise<void> {
|
||||
console.log(`🌐 Opening browser for authorization: ${url}`);
|
||||
|
||||
const command = `open "${url}"`;
|
||||
|
||||
exec(command, error => {
|
||||
if (error) {
|
||||
console.error(`Failed to open browser: ${error.message}`);
|
||||
console.log(`Please manually open: ${url}`);
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
if (!InteractiveOAuthClient.ALLOWED_SCHEMES.has(parsed.protocol)) {
|
||||
console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`);
|
||||
return;
|
||||
}
|
||||
});
|
||||
} catch {
|
||||
console.error(`Invalid URL: ${url}`);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await open(url);
|
||||
} catch {
|
||||
console.log(`Please manually open: ${url}`);
|
||||
}
|
||||
}
|
||||
/**
|
||||
* Example OAuth callback handler - in production, use a more robust approach
|
||||
|
|
|
|||
Loading…
Reference in a new issue