mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(mcp): dedupe blocked stdio env warnings (#128102)
* fix(mcp): dedupe blocked stdio env warnings * test(mcp): cover warning dedupe boundaries Punchcard-Session: clear-willow-orchard-hd * fix(mcp): encode warning dedupe identity Punchcard-Session: quiet-valley-harbor-ep --------- Co-authored-by: gaoanze <gaoanze@meituan.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -119,6 +119,72 @@ describe("resolveMcpTransportConfig", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("warns once per blocked stdio env key and server", () => {
|
||||
const repeatedResolutions = Array.from({ length: 3 }, () =>
|
||||
resolveMcpTransportConfig("repeat-server", {
|
||||
command: "node",
|
||||
env: {
|
||||
PYTHONPATH: "/tmp/workspace",
|
||||
},
|
||||
}),
|
||||
);
|
||||
const sameKeyDifferentServer = resolveMcpTransportConfig("other-server", {
|
||||
command: "node",
|
||||
env: {
|
||||
PYTHONPATH: "/tmp/other-workspace",
|
||||
},
|
||||
});
|
||||
const sameServerDifferentKey = resolveMcpTransportConfig("repeat-server", {
|
||||
command: "node",
|
||||
env: {
|
||||
NODE_OPTIONS: "--require=./evil.js",
|
||||
},
|
||||
});
|
||||
const firstCollidingPair = resolveMcpTransportConfig("svc", {
|
||||
command: "node",
|
||||
env: {
|
||||
"LD_A:LD_B": "/tmp/workspace",
|
||||
},
|
||||
});
|
||||
const secondCollidingPair = resolveMcpTransportConfig("svc:LD_A", {
|
||||
command: "node",
|
||||
env: {
|
||||
LD_B: "/tmp/workspace",
|
||||
},
|
||||
});
|
||||
|
||||
for (const resolved of [
|
||||
...repeatedResolutions,
|
||||
sameKeyDifferentServer,
|
||||
sameServerDifferentKey,
|
||||
firstCollidingPair,
|
||||
secondCollidingPair,
|
||||
]) {
|
||||
expect(resolved).toEqual(expect.objectContaining({ env: {} }));
|
||||
}
|
||||
expect(logWarn).toHaveBeenCalledTimes(5);
|
||||
expect(logWarn).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
'bundle-mcp: server "repeat-server": env "PYTHONPATH" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'bundle-mcp: server "other-server": env "PYTHONPATH" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
'bundle-mcp: server "repeat-server": env "NODE_OPTIONS" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenNthCalledWith(
|
||||
4,
|
||||
'bundle-mcp: server "svc": env "LD_A:LD_B" is blocked for stdio startup safety and was ignored.',
|
||||
);
|
||||
expect(logWarn).toHaveBeenNthCalledWith(
|
||||
5,
|
||||
'bundle-mcp: server "svc:LD_A": env "LD_B" 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",
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";
|
||||
import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js";
|
||||
import { resolveOpenClawMcpTransportAlias } from "../config/mcp-config-normalize.js";
|
||||
import { createDedupeCache } from "../infra/dedupe.js";
|
||||
import { logWarn } from "../logger.js";
|
||||
import { readTrimmedStringAlias } from "../utils/string-readers.js";
|
||||
import {
|
||||
@@ -62,6 +63,24 @@ type ResolvedMcpTransportConfig = ResolvedStdioMcpTransportConfig | ResolvedHttp
|
||||
|
||||
const DEFAULT_CONNECTION_TIMEOUT_MS = 30_000;
|
||||
const DEFAULT_REQUEST_TIMEOUT_MS = 60_000;
|
||||
const MAX_WARNED_DROPPED_STDIO_ENV_KEYS = 4096;
|
||||
// Warning state spans repeated MCP transport resolutions in one gateway process;
|
||||
// bounding it means evicted server/env pairs can re-warn instead of growing unbounded.
|
||||
const warnedDroppedStdioEnvKeys = createDedupeCache({
|
||||
ttlMs: 0,
|
||||
maxSize: MAX_WARNED_DROPPED_STDIO_ENV_KEYS,
|
||||
});
|
||||
|
||||
function warnDroppedStdioEnvOnce(serverName: string, key: string): void {
|
||||
const logServerName = sanitizeForLog(serverName);
|
||||
const logKey = sanitizeForLog(key);
|
||||
if (warnedDroppedStdioEnvKeys.check(JSON.stringify([serverName, key]))) {
|
||||
return;
|
||||
}
|
||||
logWarn(
|
||||
`bundle-mcp: server "${logServerName}": env "${logKey}" is blocked for stdio startup safety and was ignored.`,
|
||||
);
|
||||
}
|
||||
|
||||
function getPositiveNumber(rawServer: unknown, keys: readonly string[]): number | undefined {
|
||||
if (!rawServer || typeof rawServer !== "object") {
|
||||
@@ -205,7 +224,6 @@ export function resolveMcpTransportConfig(
|
||||
rawServer: unknown,
|
||||
options?: { logWarnings?: boolean },
|
||||
): ResolvedMcpTransportConfig | null {
|
||||
const logServerName = sanitizeForLog(serverName);
|
||||
const logWarnings = options?.logWarnings !== false;
|
||||
const requestedTransport = getRequestedTransport(rawServer);
|
||||
const requestedTransportAlias = requestedTransport ? "" : getRequestedTransportAlias(rawServer);
|
||||
@@ -215,9 +233,7 @@ export function resolveMcpTransportConfig(
|
||||
logWarnings
|
||||
? {
|
||||
onDroppedEnv: (key: string) => {
|
||||
logWarn(
|
||||
`bundle-mcp: server "${logServerName}": env "${sanitizeForLog(key)}" is blocked for stdio startup safety and was ignored.`,
|
||||
);
|
||||
warnDroppedStdioEnvOnce(serverName, key);
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
@@ -246,7 +262,7 @@ export function resolveMcpTransportConfig(
|
||||
) {
|
||||
if (logWarnings) {
|
||||
logWarn(
|
||||
`bundle-mcp: skipped server "${logServerName}" because transport "${sanitizeForLog(effectiveTransport)}" is not supported.`,
|
||||
`bundle-mcp: skipped server "${sanitizeForLog(serverName)}" because transport "${sanitizeForLog(effectiveTransport)}" is not supported.`,
|
||||
);
|
||||
}
|
||||
return null;
|
||||
@@ -273,7 +289,7 @@ export function resolveMcpTransportConfig(
|
||||
const httpReason = httpLaunch.ok ? "not an HTTP MCP server" : httpLaunch.reason;
|
||||
if (logWarnings) {
|
||||
logWarn(
|
||||
`bundle-mcp: skipped server "${logServerName}" because ${stdioLaunch.reason} and ${httpReason}.`,
|
||||
`bundle-mcp: skipped server "${sanitizeForLog(serverName)}" because ${stdioLaunch.reason} and ${httpReason}.`,
|
||||
);
|
||||
}
|
||||
return null;
|
||||
|
||||
Reference in New Issue
Block a user