From 58beee40401f7a9db8cd7411974a913ec705f346 Mon Sep 17 00:00:00 2001 From: Test User Date: Sun, 1 Feb 2026 18:26:39 -0800 Subject: [PATCH] fix: security vulnerabilities and bug fixes ## Security Fixes ### Critical: Remove code injection vulnerability in debug agent - Removed `new Function()` eval fallback in `parseToolParams` - Now only accepts valid JSON, preventing arbitrary code execution - Location: src/cli/cmd/debug/agent.ts ### High: Fix XSS vulnerability in OAuth callback - Added HTML escaping for error messages in OAuth callback HTML - Prevents XSS attacks from malicious OAuth provider responses - Location: src/mcp/oauth-callback.ts ## Bug Fixes ### High: Fix race condition in OAuth cancelPending - `cancelPending` was using wrong key to lookup pending auths - Now correctly iterates through Map to find by mcpName - Location: src/mcp/oauth-callback.ts, src/mcp/index.ts ### Medium: Fix unhandled exception in read tool - `fs.readdirSync` now wrapped in try-catch - Returns empty array instead of throwing when directory doesn't exist - Location: src/tool/read.ts Co-Authored-By: Claude --- packages/opencode/src/cli/cmd/debug/agent.ts | 23 ++++++-------------- packages/opencode/src/mcp/index.ts | 2 +- packages/opencode/src/mcp/oauth-callback.ts | 23 +++++++++++++------- packages/opencode/src/tool/read.ts | 8 ++++++- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/opencode/src/cli/cmd/debug/agent.ts b/packages/opencode/src/cli/cmd/debug/agent.ts index fe300348597..9e4cafd39a3 100644 --- a/packages/opencode/src/cli/cmd/debug/agent.ts +++ b/packages/opencode/src/cli/cmd/debug/agent.ts @@ -91,24 +91,15 @@ function parseToolParams(input?: string) { const trimmed = input.trim() if (trimmed.length === 0) return {} - const parsed = iife(() => { - try { - return JSON.parse(trimmed) - } catch (jsonError) { - try { - return new Function(`return (${trimmed})`)() - } catch (evalError) { - throw new Error( - `Failed to parse --params. Use JSON or a JS object literal. JSON error: ${jsonError}. Eval error: ${evalError}.`, - ) - } + try { + const parsed = JSON.parse(trimmed) + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error("Tool params must be an object.") } - }) - - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { - throw new Error("Tool params must be an object.") + return parsed as Record + } catch (e) { + throw new Error(`Failed to parse --params as JSON: ${e instanceof Error ? e.message : e}`) } - return parsed as Record } async function createToolContext(agent: Agent.Info) { diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 29e958fe357..f00ba42d9ca 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -801,7 +801,7 @@ export namespace MCP { // Register the callback BEFORE opening the browser to avoid race condition // when the IdP has an active SSO session and redirects immediately - const callbackPromise = McpOAuthCallback.waitForCallback(oauthState) + const callbackPromise = McpOAuthCallback.waitForCallback(oauthState, mcpName) try { const subprocess = await open(authorizationUrl) diff --git a/packages/opencode/src/mcp/oauth-callback.ts b/packages/opencode/src/mcp/oauth-callback.ts index bb3b56f2e95..fb5c19ac69a 100644 --- a/packages/opencode/src/mcp/oauth-callback.ts +++ b/packages/opencode/src/mcp/oauth-callback.ts @@ -3,6 +3,10 @@ import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "./oauth-provider" const log = Log.create({ service: "mcp.oauth-callback" }) +function escapeHtml(str: string): string { + return str.replace(/[&<>"']/g, (char) => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" })[char] ?? char) +} + const HTML_SUCCESS = ` @@ -39,12 +43,13 @@ const HTML_ERROR = (error: string) => `

Authorization Failed

An error occurred during authorization.

-
${error}
+
${escapeHtml(error)}
` interface PendingAuth { + mcpName: string resolve: (code: string) => void reject: (error: Error) => void timeout: ReturnType @@ -136,7 +141,7 @@ export namespace McpOAuthCallback { log.info("oauth callback server started", { port: OAUTH_CALLBACK_PORT }) } - export function waitForCallback(oauthState: string): Promise { + export function waitForCallback(oauthState: string, mcpName: string): Promise { return new Promise((resolve, reject) => { const timeout = setTimeout(() => { if (pendingAuths.has(oauthState)) { @@ -145,16 +150,18 @@ export namespace McpOAuthCallback { } }, CALLBACK_TIMEOUT_MS) - pendingAuths.set(oauthState, { resolve, reject, timeout }) + pendingAuths.set(oauthState, { mcpName, resolve, reject, timeout }) }) } export function cancelPending(mcpName: string): void { - const pending = pendingAuths.get(mcpName) - if (pending) { - clearTimeout(pending.timeout) - pendingAuths.delete(mcpName) - pending.reject(new Error("Authorization cancelled")) + for (const [state, pending] of pendingAuths) { + if (pending.mcpName === mcpName) { + clearTimeout(pending.timeout) + pendingAuths.delete(state) + pending.reject(new Error("Authorization cancelled")) + return + } } } diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 13236d44dd4..5b0f1229f05 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -43,7 +43,13 @@ export const ReadTool = Tool.define("read", { const dir = path.dirname(filepath) const base = path.basename(filepath) - const dirEntries = fs.readdirSync(dir) + const dirEntries = (() => { + try { + return fs.readdirSync(dir) + } catch { + return [] + } + })() const suggestions = dirEntries .filter( (entry) =>