From 001d29ea8e4a54565802c678820f3d64e09aa3fd Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 8 Apr 2026 17:18:17 +0900 Subject: [PATCH] fix(skill-mcp): treat opencode-project and local scopes as untrusted for env var access Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../connection-env-vars.test.ts | 71 ++++++++++++++++++- src/features/skill-mcp-manager/connection.ts | 4 +- src/features/skill-mcp-manager/types.ts | 2 +- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/features/skill-mcp-manager/connection-env-vars.test.ts b/src/features/skill-mcp-manager/connection-env-vars.test.ts index a535bcb47..60cf20ce6 100644 --- a/src/features/skill-mcp-manager/connection-env-vars.test.ts +++ b/src/features/skill-mcp-manager/connection-env-vars.test.ts @@ -1,4 +1,4 @@ -import { afterAll, afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { afterAll, afterEach, beforeEach, describe, expect, it, mock, test } from "bun:test" import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types" import type { SkillMcpClientInfo, SkillMcpManagerState } from "./types" @@ -89,12 +89,15 @@ function createState(): SkillMcpManagerState { return state } -function createClientInfo(serverName: string): SkillMcpClientInfo { +function createClientInfo( + serverName: string, + scope?: SkillMcpClientInfo["scope"], +): SkillMcpClientInfo { return { serverName, skillName: "env-skill", sessionID: "session-env", - scope: "builtin", + ...(scope !== undefined ? { scope } : {}), } } @@ -126,6 +129,68 @@ afterEach(async () => { }) describe("getOrCreateClient env var expansion", () => { + describe("#given a scope-sensitive stdio skill MCP config", () => { + test.each([ + ["opencode-project", "Authorization:Bearer "], + ["local", "Authorization:Bearer "], + ["user", "Authorization:Bearer xoxp-scope-token"], + ["builtin", "Authorization:Bearer xoxp-scope-token"], + ] satisfies Array<[NonNullable, string]>) ( + "#when creating the client for %s scope #then args expand to %s", + async (scope, expectedAuthorizationHeader) => { + // given + process.env.SLACK_USER_TOKEN = "xoxp-scope-token" + const state = createState() + const info = createClientInfo(`scope-${scope}`, scope) + const clientKey = createClientKey(info) + const config: ClaudeCodeMcpServer = { + command: "npx", + args: [ + "-y", + "mcp-remote", + "https://mcp.slack.com/mcp", + "--header", + "Authorization:Bearer ${SLACK_USER_TOKEN}", + ], + } + + // when + await getOrCreateClient({ state, clientKey, info, config }) + + // then + expect(createdStdioTransports).toHaveLength(1) + expect(createdStdioTransports[0]?.options.args?.[4]).toBe(expectedAuthorizationHeader) + }, + ) + + it("#when creating the client without scope #then env vars remain trusted for backward compatibility", async () => { + // given + process.env.SLACK_USER_TOKEN = "xoxp-undefined-scope-token" + const state = createState() + const info = createClientInfo("scope-undefined") + const clientKey = createClientKey(info) + const config: ClaudeCodeMcpServer = { + command: "npx", + args: [ + "-y", + "mcp-remote", + "https://mcp.slack.com/mcp", + "--header", + "Authorization:Bearer ${SLACK_USER_TOKEN}", + ], + } + + // when + await getOrCreateClient({ state, clientKey, info, config }) + + // then + expect(createdStdioTransports).toHaveLength(1) + expect(createdStdioTransports[0]?.options.args?.[4]).toBe( + "Authorization:Bearer xoxp-undefined-scope-token", + ) + }) + }) + describe("#given a stdio skill MCP config with sensitive env vars in args", () => { it("#when creating the client #then sensitive env vars in args are expanded", async () => { // given diff --git a/src/features/skill-mcp-manager/connection.ts b/src/features/skill-mcp-manager/connection.ts index 2826492b0..2fa4dc3a3 100644 --- a/src/features/skill-mcp-manager/connection.ts +++ b/src/features/skill-mcp-manager/connection.ts @@ -14,6 +14,8 @@ function removeClientIfCurrent(state: SkillMcpManagerState, clientKey: string, c } } +const PROJECT_SCOPES = new Set(["project", "opencode-project", "local"]) + export async function getOrCreateClient(params: { state: SkillMcpManagerState clientKey: string @@ -38,7 +40,7 @@ export async function getOrCreateClient(params: { return pending } - const isTrusted = info.scope !== "project" + const isTrusted = !PROJECT_SCOPES.has(info.scope ?? "") const expandedConfig = expandEnvVarsInObject(config, { trusted: isTrusted }) let currentConnectionPromise!: Promise state.inFlightConnections.set(info.sessionID, (state.inFlightConnections.get(info.sessionID) ?? 0) + 1) diff --git a/src/features/skill-mcp-manager/types.ts b/src/features/skill-mcp-manager/types.ts index 3d2838d55..75ef396cf 100644 --- a/src/features/skill-mcp-manager/types.ts +++ b/src/features/skill-mcp-manager/types.ts @@ -11,7 +11,7 @@ export interface SkillMcpClientInfo { serverName: string skillName: string sessionID: string - scope?: SkillScope + scope?: SkillScope | "local" } export interface SkillMcpServerContext {