mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:50:43 +00:00
fix: narrow MCP stdio env safety filter (#69540)
This commit is contained in:
@@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Lobster/TaskFlow: allow managed approval resumes to use `approvalId` without a resume token, and persist that id in approval wait state. (#69559) Thanks @kirkluokun.
|
||||
- Plugins/startup: install bundled runtime dependencies into each plugin's own runtime directory, reuse source-checkout repair caches after rebuilds, and log only packages that were actually installed so repeated Gateway starts stay quiet once deps are present.
|
||||
- Plugins/startup: ignore pnpm's `npm_execpath` when repairing bundled plugin runtime dependencies and skip workspace-only package specs so npm-only install flags or local workspace links do not break packaged plugin startup.
|
||||
- MCP: block interpreter-startup env keys such as `NODE_OPTIONS` for stdio servers while preserving ordinary credential and proxy env vars. (#69540) Thanks @drobison00.
|
||||
- Setup/TUI: relaunch the setup hatch TUI in a fresh process while preserving the configured gateway target and auth source, so onboarding recovers terminal state cleanly without exposing gateway secrets on command-line args. (#69524) Thanks @shakkernerd.
|
||||
- Codex: avoid re-exposing the image-generation tool on native vision turns with inbound images, and keep bare image-model overrides on the configured image provider. (#65061) Thanks @zhulijin1991.
|
||||
- Sessions/reset: clear auto-sourced model, provider, and auth-profile overrides on `/new` and `/reset` while preserving explicit user selections, so channel sessions stop staying pinned to runtime fallback choices. (#69419) Thanks @sk7n4k3d.
|
||||
|
||||
@@ -1,7 +1,4 @@
|
||||
import {
|
||||
isDangerousHostEnvOverrideVarName,
|
||||
isDangerousHostEnvVarName,
|
||||
} from "../infra/host-env-security.js";
|
||||
import { isDangerousHostEnvVarName } from "../infra/host-env-security.js";
|
||||
|
||||
export function isMcpConfigRecord(value: unknown): value is Record<string, unknown> {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value);
|
||||
@@ -11,15 +8,18 @@ function toMcpFilteredStringRecord(
|
||||
value: unknown,
|
||||
options?: {
|
||||
onDroppedEntry?: (key: string, value: unknown) => void;
|
||||
preserveEmptyWhenKeysDropped?: boolean;
|
||||
shouldDropKey?: (key: string) => boolean;
|
||||
},
|
||||
): Record<string, string> | undefined {
|
||||
if (!isMcpConfigRecord(value)) {
|
||||
return undefined;
|
||||
}
|
||||
let droppedByKey = false;
|
||||
const entries = Object.entries(value)
|
||||
.map(([key, entry]) => {
|
||||
if (options?.shouldDropKey?.(key)) {
|
||||
droppedByKey = true;
|
||||
options?.onDroppedEntry?.(key, entry);
|
||||
return null;
|
||||
}
|
||||
@@ -33,6 +33,9 @@ function toMcpFilteredStringRecord(
|
||||
return null;
|
||||
})
|
||||
.filter((entry): entry is readonly [string, string] => entry !== null);
|
||||
if (entries.length === 0 && droppedByKey && options?.preserveEmptyWhenKeysDropped) {
|
||||
return {};
|
||||
}
|
||||
return entries.length > 0 ? Object.fromEntries(entries) : undefined;
|
||||
}
|
||||
|
||||
@@ -49,8 +52,8 @@ export function toMcpEnvRecord(
|
||||
): Record<string, string> | undefined {
|
||||
return toMcpFilteredStringRecord(value, {
|
||||
...options,
|
||||
shouldDropKey: (key) =>
|
||||
isDangerousHostEnvVarName(key) || isDangerousHostEnvOverrideVarName(key),
|
||||
preserveEmptyWhenKeysDropped: true,
|
||||
shouldDropKey: (key) => isDangerousHostEnvVarName(key),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -11,7 +11,10 @@ type StdioMcpServerLaunchResult =
|
||||
| { ok: true; config: StdioMcpServerLaunchConfig }
|
||||
| { ok: false; reason: string };
|
||||
|
||||
export function resolveStdioMcpServerLaunchConfig(raw: unknown): StdioMcpServerLaunchResult {
|
||||
export function resolveStdioMcpServerLaunchConfig(
|
||||
raw: unknown,
|
||||
options?: { onDroppedEnv?: (key: string, value: unknown) => void },
|
||||
): StdioMcpServerLaunchResult {
|
||||
if (!isMcpConfigRecord(raw)) {
|
||||
return { ok: false, reason: "server config must be an object" };
|
||||
}
|
||||
@@ -35,7 +38,7 @@ export function resolveStdioMcpServerLaunchConfig(raw: unknown): StdioMcpServerL
|
||||
config: {
|
||||
command: raw.command,
|
||||
args: toMcpStringArray(raw.args),
|
||||
env: toMcpEnvRecord(raw.env),
|
||||
env: toMcpEnvRecord(raw.env, { onDroppedEntry: options?.onDroppedEnv }),
|
||||
cwd,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,7 +1,14 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { logWarn } from "../logger.js";
|
||||
import { resolveMcpTransportConfig } from "./mcp-transport-config.js";
|
||||
|
||||
vi.mock("../logger.js", () => ({ logWarn: vi.fn() }));
|
||||
|
||||
describe("resolveMcpTransportConfig", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(logWarn).mockClear();
|
||||
});
|
||||
|
||||
it("resolves stdio config with connection timeout", () => {
|
||||
const resolved = resolveMcpTransportConfig("probe", {
|
||||
command: "node",
|
||||
@@ -25,6 +32,8 @@ describe("resolveMcpTransportConfig", () => {
|
||||
SAFE_VALUE: "ok",
|
||||
PORT: 3000,
|
||||
ENABLED: true,
|
||||
GITHUB_TOKEN: "token",
|
||||
HTTP_PROXY: "http://proxy.example",
|
||||
NODE_OPTIONS: "--require=./evil.js",
|
||||
LD_PRELOAD: "/tmp/pwn.so",
|
||||
BASH_ENV: "/tmp/pwn.sh",
|
||||
@@ -40,11 +49,38 @@ describe("resolveMcpTransportConfig", () => {
|
||||
SAFE_VALUE: "ok",
|
||||
PORT: "3000",
|
||||
ENABLED: "true",
|
||||
GITHUB_TOKEN: "token",
|
||||
HTTP_PROXY: "http://proxy.example",
|
||||
},
|
||||
cwd: undefined,
|
||||
description: "node",
|
||||
connectionTimeoutMs: 30_000,
|
||||
});
|
||||
expect(logWarn).toHaveBeenCalledWith(
|
||||
'bundle-mcp: server "probe": env "NODE_OPTIONS" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenCalledWith(
|
||||
'bundle-mcp: server "probe": env "LD_PRELOAD" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenCalledWith(
|
||||
'bundle-mcp: server "probe": env "BASH_ENV" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
});
|
||||
|
||||
it("uses an explicit empty stdio env when all configured env keys are blocked", () => {
|
||||
const resolved = resolveMcpTransportConfig("probe", {
|
||||
command: "node",
|
||||
env: {
|
||||
NODE_OPTIONS: "--require=./evil.js",
|
||||
BASH_ENV: "/tmp/pwn.sh",
|
||||
},
|
||||
});
|
||||
|
||||
expect(resolved).toMatchObject({
|
||||
kind: "stdio",
|
||||
command: "node",
|
||||
env: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves SSE config by default", () => {
|
||||
|
||||
@@ -96,7 +96,13 @@ export function resolveMcpTransportConfig(
|
||||
rawServer: unknown,
|
||||
): ResolvedMcpTransportConfig | null {
|
||||
const requestedTransport = getRequestedTransport(rawServer);
|
||||
const stdioLaunch = resolveStdioMcpServerLaunchConfig(rawServer);
|
||||
const stdioLaunch = resolveStdioMcpServerLaunchConfig(rawServer, {
|
||||
onDroppedEnv: (key) => {
|
||||
logWarn(
|
||||
`bundle-mcp: server "${serverName}": env "${key}" is blocked for stdio startup safety and was ignored.`,
|
||||
);
|
||||
},
|
||||
});
|
||||
if (stdioLaunch.ok) {
|
||||
return {
|
||||
kind: "stdio",
|
||||
|
||||
Reference in New Issue
Block a user