From ebf29f1187458f1ee8b14fe2a18596fd48656efb Mon Sep 17 00:00:00 2001
From: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Date: Wed, 24 Jun 2026 13:51:44 +0800
Subject: [PATCH] fix(core): parse QWEN_SERVE_MCP_CLIENT_BUDGET strictly as a
 decimal integer (#5752)

* fix(core): parse QWEN_SERVE_MCP_CLIENT_BUDGET strictly as a decimal integer

readBudgetFromEnv used Number(rawBudget), so Number("0x10")=16, Number("1e2")=100
and Number("1.0")=1 all passed isInteger and were silently accepted as a budget.
Require plain decimal digits (/^\d+$/) like the other integer env vars, keeping
the existing stderr breadcrumb for invalid values.

* fix(acp): apply strict budget-env parsing to createWorkspaceMcpBudget

The pooled path parsed QWEN_SERVE_MCP_CLIENT_BUDGET with a loose Number() +
isInteger, so 0x10, 1e2 and 1.0 silently set a budget while the manager's
readBudgetFromEnv (already strict) rejected them. Use the same /^\d+$/ +
isSafeInteger parse and emit the same stderr breadcrumb on invalid input.
Export the function and cover the parsing with unit tests.

* docs: sync readBudgetFromEnv JSDoc with the stderr breadcrumb

The doc still said invalid budget values are silently ignored, but the
function now writes a stderr breadcrumb when it rejects one. Match the
doc to the behavior.
---
 .../cli/src/acp-integration/acpAgent.test.ts  | 38 ++++++++++++++++++
 packages/cli/src/acp-integration/acpAgent.ts  | 39 ++++++++++---------
 .../core/src/tools/mcp-client-manager.test.ts | 19 +++++++++
 packages/core/src/tools/mcp-client-manager.ts | 11 ++++--
 4 files changed, 86 insertions(+), 21 deletions(-)

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