diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 4c71ae2435..e4c3e1198d 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -562,6 +562,7 @@ import { normalizeCoreSettingValue, extractFilesFromTarGz, fetchAllowedGitHub, + createWorkspaceMcpBudget, } from './acpAgent.js'; import { gzipSync } from 'node:zlib'; import type { Config } from '@qwen-code/qwen-code-core'; @@ -7291,3 +7292,40 @@ describe('sessionLanguage multi-session propagation', () => { await agentPromise; }); }); + +describe('createWorkspaceMcpBudget — env parsing', () => { + const KEY = 'QWEN_SERVE_MCP_CLIENT_BUDGET'; + const MODE = 'QWEN_SERVE_MCP_BUDGET_MODE'; + const onEvent = vi.fn(); + + afterEach(() => { + delete process.env[KEY]; + delete process.env[MODE]; + vi.clearAllMocks(); + }); + + it('accepts a plain positive decimal integer', () => { + process.env[KEY] = '100'; + expect(createWorkspaceMcpBudget(onEvent)).toBeDefined(); + }); + + it('accepts a trimmed decimal integer', () => { + process.env[KEY] = ' 42 '; + expect(createWorkspaceMcpBudget(onEvent)).toBeDefined(); + }); + + // Mirrors McpClientManager.readBudgetFromEnv: a loose Number() would coerce + // these (0x10=16, 1e2=100, 1.0=1) and silently set a budget. The strict + // /^\d+$/ + isSafeInteger parse must reject them. + it.each(['0x10', '1e2', '1.0', '0b101', '5 abc', 'abc', '-5', '0', ' '])( + 'rejects non-decimal-integer value %j', + (raw) => { + process.env[KEY] = raw; + expect(createWorkspaceMcpBudget(onEvent)).toBeUndefined(); + }, + ); + + it('returns undefined when the budget env var is unset', () => { + expect(createWorkspaceMcpBudget(onEvent)).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 4492e55e92..7aecceb017 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2395,34 +2395,37 @@ function parsePoolDrainMs(envValue: string | undefined): number { * invokes `tryReserve`/`release`; this helper produces the controller * and wires the event callback. */ -function createWorkspaceMcpBudget( +export function createWorkspaceMcpBudget( onEvent: (event: McpBudgetEvent) => void, ): WorkspaceMcpBudget | undefined { const rawBudget = process.env['QWEN_SERVE_MCP_CLIENT_BUDGET']; const rawMode = process.env['QWEN_SERVE_MCP_BUDGET_MODE']; - // Match `McpClientManager.readBudgetFromEnv`'s parsing exactly. - // Use `Number(...)` + `Number.isInteger` so the pool and the manager + // Match `McpClientManager.readBudgetFromEnv`'s parsing exactly: only plain + // decimal digits set a budget. A loose `Number(...)` would silently accept + // `0x10`=16, `1e2`=100, and `1.0`=1 (all pass `isInteger`); the strict + // `/^\d+$/` + `isSafeInteger` check rejects them so the pool and the manager // honor the same env values. - const budget = - rawBudget !== undefined && rawBudget !== '' ? Number(rawBudget) : undefined; + let budget: number | undefined; + if (rawBudget !== undefined && rawBudget !== '') { + const trimmed = rawBudget.trim(); + const parsed = Number(trimmed); + if (/^\d+$/.test(trimmed) && Number.isSafeInteger(parsed) && parsed > 0) { + budget = parsed; + } else { + process.stderr.write( + `qwen serve: ignoring invalid QWEN_SERVE_MCP_CLIENT_BUDGET=` + + `'${rawBudget}' (expected positive integer); ` + + `MCP budget enforcement disabled for this child.\n`, + ); + } + } const mode: McpBudgetMode = (() => { if (rawMode === 'enforce' || rawMode === 'warn' || rawMode === 'off') { return rawMode; } - return budget !== undefined && - Number.isFinite(budget) && - Number.isInteger(budget) && - budget > 0 - ? 'warn' - : 'off'; + return budget !== undefined ? 'warn' : 'off'; })(); - if ( - mode === 'off' || - budget === undefined || - !Number.isFinite(budget) || - !Number.isInteger(budget) || - budget <= 0 - ) { + if (mode === 'off' || budget === undefined) { return undefined; } return new WorkspaceMcpBudget({ diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index cf53ed0bd1..0d635b1067 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -2627,6 +2627,25 @@ describe('McpClientManager — PR 14 guardrails', () => { } }); + it('readBudgetFromEnv rejects non-decimal budget values (hex / scientific / float)', async () => { + const writeSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + try { + for (const bad of ['0x10', '1e2', '1.0']) { + process.env['QWEN_SERVE_MCP_CLIENT_BUDGET'] = bad; + const manager = mkManager({ config: configWithServers({}) }); + // Pre-fix Number('0x10')=16 / Number('1e2')=100 slipped through as a budget. + expect(manager.getMcpClientBudget()).toBeUndefined(); + } + // a plain decimal integer is still accepted. + process.env['QWEN_SERVE_MCP_CLIENT_BUDGET'] = '16'; + const ok = mkManager({ config: configWithServers({}) }); + expect(ok.getMcpClientBudget()).toBe(16); + } finally { + writeSpy.mockRestore(); + delete process.env['QWEN_SERVE_MCP_CLIENT_BUDGET']; + } + }); + it('readResource rejects existing-but-now-disabled servers (wenshao R7 #5 line 1342)', async () => { // Pre-fix: a server connected pre-disable and then operator- // disabled mid-session via settings reload would still serve diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index b28a89d98a..c882e7294c 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -269,7 +269,8 @@ export function mcpTransportOf(config: MCPServerConfig): McpTransportKind { * behavior, no enforcement. * * `QWEN_SERVE_MCP_CLIENT_BUDGET` — positive integer; non-numeric / - * zero / negative / NaN are silently ignored (treated as unset). + * zero / negative / NaN are rejected (treated as unset) and a + * stderr breadcrumb is written so the misconfiguration is visible. * `QWEN_SERVE_MCP_BUDGET_MODE` — `enforce|warn|off`. Defaults to * `warn` when a budget is set, `off` otherwise. */ @@ -278,8 +279,12 @@ function readBudgetFromEnv(): McpBudgetConfig { const rawMode = process.env['QWEN_SERVE_MCP_BUDGET_MODE']; let clientBudget: number | undefined; if (rawBudget !== undefined && rawBudget !== '') { - const parsed = Number(rawBudget); - if (Number.isFinite(parsed) && Number.isInteger(parsed) && parsed > 0) { + // Parse strictly as a decimal integer: Number('0x10')=16, Number('1e2')=100 + // and Number('1.0')=1 all pass isInteger, so a loose parse would silently + // accept them. Only plain decimal digits should set a budget. + const trimmed = rawBudget.trim(); + const parsed = Number(trimmed); + if (/^\d+$/.test(trimmed) && Number.isSafeInteger(parsed) && parsed > 0) { clientBudget = parsed; } else { // operator typos