fix(cli): expand ${VAR} placeholders when loading project .mcp.json

docs/users/features/mcp.md

@@ -63,6 +63,14 @@ qwen mcp add --scope user --transport http my-server http://localhost:3000/mcp
 >
 > For advanced configuration layers (system defaults/system settings and precedence rules), see [Settings](../configuration/settings).
 
+### Project `.mcp.json` (shipped with the repository)
+
+A `.mcp.json` file in the project root (the Claude Code convention) declares servers that travel with the repository. Because the file comes from the repository, its servers are untrusted until you approve them: an interactive session asks before connecting, and `qwen mcp approve <name>` (or `--all`) records the approval for non-interactive use. Approval is bound to the exact server configuration — editing the entry, or changing the value of an environment variable it references, asks again.
+
+Reference secrets by name instead of embedding them: `$VAR` / `${VAR}` in `command`, `args`, `env`, `cwd`, `url`, `httpUrl`, `headers`, `tcp`, `oauth`, `targetAudience` and `targetServiceAccount` is expanded from the environment (including the `.env` files Qwen Code loads), as in `settings.json`. An unset variable is left as literal text; other fields such as `description` are never expanded.
+
+With `--yolo` nothing asks before a server connects, so `.mcp.json` placeholders are deliberately **not** expanded there: the placeholder is sent as written and a warning on stderr names the server. Bare mode and safe mode do not load `.mcp.json` at all. A server that needs its expanded value in a `--yolo` run belongs in `.qwen/settings.json`, which is resolved regardless of approval mode.
+
 ## Configure servers
 
 ### Choose a transport

packages/cli/src/acp-integration/acpAgent.test.ts

@@ -174,7 +174,12 @@ const { mockMcpApprovals, mockGetPendingGatedMcpServers } = vi.hoisted(() => ({
   mockGetPendingGatedMcpServers: vi.fn().mockReturnValue([]),
 }));
 
-vi.mock('../config/mcpApprovals.js', () => ({
+vi.mock('../config/mcpApprovals.js', async (importOriginal) => ({
+  // The gate predicate is pure; keep the real one so the ACP reload path runs
+  // the real armed/off decision instead of a stand-in.
+  isMcpApprovalGateArmed: (
+    await importOriginal<typeof import('../config/mcpApprovals.js')>()
+  ).isMcpApprovalGateArmed,
   loadMcpApprovals: () => mockMcpApprovals,
   getPendingGatedMcpServers: mockGetPendingGatedMcpServers,
   getPromptableMcpServers: vi.fn().mockReturnValue([]),
@@ -252,6 +257,10 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => ({
   SessionSourceError: (
     await importOriginal<typeof import('@qwen-code/qwen-code-core')>()
   ).SessionSourceError,
+  // Pure; the real `.mcp.json` loader needs it for the reload expansion tests.
+  normalizeClaudeMcpServer: (
+    await importOriginal<typeof import('@qwen-code/qwen-code-core')>()
+  ).normalizeClaudeMcpServer,
   BranchPointInvalidError: class BranchPointInvalidError extends Error {
     constructor(readonly recordId: string) {
       super(`Invalid or inactive branch point: ${recordId}`);
@@ -2276,6 +2285,9 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
     });
 
     mockConfig = {
+      getApprovalMode: vi.fn().mockReturnValue('default'),
+      getBareMode: vi.fn().mockReturnValue(false),
+      isSafeMode: vi.fn().mockReturnValue(false),
       initialize: vi.fn().mockResolvedValue(undefined),
       closeSessionWriter: vi.fn().mockResolvedValue(undefined),
       shutdown: vi.fn().mockResolvedValue(undefined),
@@ -6133,6 +6145,51 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
     await agentPromise;
   });
 
+  // Gate-off Configs hold `.mcp.json` unexpanded: their digest cannot match the store.
+  it.each([
+    ['yolo', undefined, 0],
+    ['default', 'pending', 1],
+  ])(
+    'workspace status reads approval state only from a gate-armed Config (approval mode %s)',
+    async (approvalMode, expectedApprovalState, expectedReads) => {
+      const manager = {
+        getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED),
+        getMcpClientAccounting: vi.fn().mockReturnValue({
+          total: 0,
+          reservedSlots: [],
+          refusedServerNames: [],
+        }),
+        getMcpClientBudget: vi.fn().mockReturnValue(undefined),
+        getMcpBudgetMode: vi.fn().mockReturnValue('off'),
+        getServerStatus: vi.fn().mockReturnValue(MCPServerStatus.DISCONNECTED),
+      };
+      mockMcpApprovals.getState.mockReturnValue('pending');
+      mockConfig = {
+        ...mockConfig,
+        getMcpServers: vi.fn().mockReturnValue({
+          proj: { httpUrl: 'https://h.example/mcp', scope: 'project' },
+        }),
+        getWorkingDir: vi.fn().mockReturnValue('/tmp'),
+        getTargetDir: vi.fn().mockReturnValue('/tmp'),
+        getApprovalMode: vi.fn().mockReturnValue(approvalMode),
+        isMcpServerDisabled: vi.fn().mockReturnValue(false),
+        getToolRegistry: vi.fn().mockReturnValue({
+          getMcpClientManager: vi.fn().mockReturnValue(manager),
+        }),
+      } as unknown as Config;
+      const { agent, agentPromise } = await bootAcpAgent();
+      const status = (await agent.extMethod(
+        SERVE_STATUS_EXT_METHODS.workspaceMcp,
+        {},
+      )) as { servers: Array<{ name: string; approvalState?: string }> };
+      expect(status.servers.map((s) => s.name)).toEqual(['proj']);
+      expect(status.servers[0].approvalState).toBe(expectedApprovalState);
+      expect(mockMcpApprovals.getState).toHaveBeenCalledTimes(expectedReads);
+      mockConnectionState.resolve();
+      await agentPromise;
+    },
+  );
+
   it('uses the pool status for the top-level MCP server status', async () => {
     const manager = {
       getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED),
@@ -28246,6 +28303,67 @@ describe('QwenAgent extMethod runtime MCP add/remove (T2.8)', () => {
     await agentPromise;
   });
 
+  // With the approval gate off (--yolo) nothing shows the user a
+  // repository-supplied server before it connects, so this reload path must
+  // not turn a `.mcp.json` placeholder into the real value. Asserted on what
+  // reaches reinitializeMcpServers, through the real loader.
+  it.each([
+    ['yolo', 'Bearer ${ACP_RELOAD_TOKEN}'],
+    ['default', 'Bearer real-secret'],
+  ])(
+    'workspaceMcpReload expands .mcp.json placeholders only while the approval gate is armed (approval mode %s)',
+    async (approvalMode, expectedHeader) => {
+      const projectDir = await fs.mkdtemp(
+        path.join(os.tmpdir(), 'acp-mcp-json-'),
+      );
+      try {
+        await fs.writeFile(
+          path.join(projectDir, '.mcp.json'),
+          JSON.stringify({
+            mcpServers: {
+              proj: {
+                httpUrl: 'https://proj.example/mcp',
+                headers: { Authorization: 'Bearer ${ACP_RELOAD_TOKEN}' },
+              },
+            },
+          }),
+        );
+        process.env['ACP_RELOAD_TOKEN'] = 'real-secret';
+        mockConfig.getTargetDir = vi.fn().mockReturnValue(projectDir);
+        mockConfig.getApprovalMode = vi.fn().mockReturnValue(approvalMode);
+        vi.mocked(loadSettings).mockReturnValue({
+          merged: { mcpServers: {} },
+          forScope: vi.fn().mockReturnValue({ settings: {} }),
+          getUserHooks: vi.fn().mockReturnValue({}),
+          getProjectHooks: vi.fn().mockReturnValue({}),
+        } as unknown as LoadedSettings);
+
+        const { agent, agentPromise } = await getAgent();
+        await expect(
+          agent.extMethod(SERVE_CONTROL_EXT_METHODS.workspaceMcpReload, {}),
+        ).resolves.toEqual({ accepted: true });
+        await vi.waitFor(() =>
+          expect(mockConfig.reinitializeMcpServers).toHaveBeenCalled(),
+        );
+        const servers = vi.mocked(mockConfig.reinitializeMcpServers).mock
+          .calls[0][0] as Record<
+          string,
+          { headers?: Record<string, string>; scope?: string }
+        >;
+        expect(servers['proj']?.scope).toBe('project');
+        expect(servers['proj']?.headers).toEqual({
+          Authorization: expectedHeader,
+        });
+
+        mockConnectionState.resolve();
+        await agentPromise;
+      } finally {
+        delete process.env['ACP_RELOAD_TOKEN'];
+        await fs.rm(projectDir, { recursive: true, force: true });
+      }
+    },
+  );
+
   it('bare mode: workspaceMcpReload does NOT leak settings.mcpServers/mcp.allowed into an already-running session', async () => {
     // Bare-mode counterpart of the safe-mode test above — suggested by an
     // automated review pass on PR #7827: the safe-mode half alone doesn't
@@ -28468,6 +28586,9 @@ describe('QwenAgent extMethod runtime MCP add/remove (T2.8)', () => {
       getTargetDir: vi.fn().mockReturnValue('/work/project'),
       getWorkingDir: vi.fn().mockReturnValue('/work/project'),
       getExcludedMcpServers: vi.fn(() => excluded),
+      getApprovalMode: vi.fn().mockReturnValue('default'),
+      getBareMode: vi.fn().mockReturnValue(false),
+      isSafeMode: vi.fn().mockReturnValue(false),
       setExcludedMcpServers: vi.fn((next: string[]) => {
         excluded = next;
       }),
@@ -28623,6 +28744,40 @@ describe('QwenAgent extMethod runtime MCP add/remove (T2.8)', () => {
     await agentPromise;
   });
 
+  // Gate-off Configs hold `.mcp.json` unexpanded: approving would persist a literal digest.
+  it('refuses to approve from a gate-off (YOLO) session', async () => {
+    const server = {
+      httpUrl: 'https://h.example/mcp',
+      headers: { Authorization: 'Bearer ${WSAPPROBE_TOK}' },
+      scope: 'project' as const,
+    };
+    const approveMcpServerForSession = vi.fn();
+    mockConfig = {
+      ...mockConfig,
+      getMcpServers: vi.fn().mockReturnValue({ proj: server }),
+      getWorkingDir: vi.fn().mockReturnValue('/work/project'),
+      getApprovalMode: vi.fn().mockReturnValue('yolo'),
+      approveMcpServerForSession,
+      getToolRegistry: vi.fn().mockReturnValue({
+        getMcpClientManager: vi.fn().mockReturnValue(mockManager),
+        discoverToolsForServer: vi.fn(),
+      }),
+    } as unknown as Config;
+
+    const { agent, agentPromise } = await getAgent();
+    await expect(
+      agent.extMethod(SERVE_CONTROL_EXT_METHODS.workspaceMcpManage, {
+        serverName: 'proj',
+        action: 'approve',
+      }),
+    ).rejects.toThrow(/MCP approval is off for this session/);
+    expect(mockMcpApprovals.setState).not.toHaveBeenCalled();
+    expect(approveMcpServerForSession).not.toHaveBeenCalled();
+
+    mockConnectionState.resolve();
+    await agentPromise;
+  });
+
   it('runtime-remove forwards to manager and returns success result', async () => {
     mockManager.removeRuntimeMcpServer.mockResolvedValue({
       name: 'my-srv',

packages/cli/src/acp-integration/acpAgent.ts

@@ -223,7 +223,10 @@ import {
   normalizeSessionIdForLookup,
   parseCallerSuppliedSessionId,
 } from '../config/session-id.js';
-import { loadMcpApprovals } from '../config/mcpApprovals.js';
+import {
+  isMcpApprovalGateArmed,
+  loadMcpApprovals,
+} from '../config/mcpApprovals.js';
 import { assembleMcpServers } from '../config/mcpServers.js';
 import { recomputeMcpGating } from '../config/hot-reload.js';
 import {
@@ -4035,24 +4038,22 @@ class QwenAgent implements Agent {
       for (const config of liveConfigs) {
         try {
           const cwd = config.getTargetDir();
-          // Same bare/safe guard as registerMcpHotReload (config/hot-reload.ts)
-          // — each live Config in this Set may independently be bare/safe or
-          // not, so the check is per-config, not hoisted outside the loop.
-          // Without it, this control-endpoint reload path (workspaceMcpReload)
-          // would fold settings.merged.mcpServers/mcp.allowed/excluded — LOCAL
-          // state safe/bare mode is supposed to distrust — back into an
-          // already-running safe/bare session, silently stranding or
-          // filtering out the caller's own top-tier server. Same bug class as
-          // the loadCliConfig (boot) and registerMcpHotReload (settings-file
-          // watcher) fixes earlier in this PR, found here in the third
-          // reload path.
+          // Per-Config, as in registerMcpHotReload: a bare/safe Config in this
+          // set must not receive settings.mcpServers / mcp.allowed / excluded.
           const isBareOrSafe = config.getBareMode() || config.isSafeMode();
+          // One value for both expansion and `pending`.
+          const gateArmed = isMcpApprovalGateArmed(
+            config.getBareMode(),
+            config.isSafeMode(),
+            config.getApprovalMode(),
+          );
           const mcpServers = isBareOrSafe
             ? { ...config.getTopTierMcpServers() }
             : assembleMcpServers(
                 settings.merged.mcpServers,
                 cwd,
                 config.getTopTierMcpServers(),
+                { expandEnv: gateArmed },
               );
           const bootAllowed = config.getCliAllowedMcpServerNames();
           const gating = isBareOrSafe
@@ -4062,7 +4063,7 @@ class QwenAgent implements Agent {
                 mcpServers,
                 cwd,
                 bootAllowed,
-                config.getApprovalMode() === ApprovalMode.YOLO,
+                !gateArmed,
               );
           config.setExcludedMcpServers(gating.excluded ?? []);
           config.setAllowedMcpServers(gating.allowed);
@@ -6734,7 +6735,14 @@ class QwenAgent implements Agent {
       const systemDefaultServers =
         settings.systemDefaults?.settings.mcpServers ?? {};
       const servers = config.getMcpServers() ?? {};
-      const approvals = loadMcpApprovals();
+      // Gate-off Configs hold `.mcp.json` unexpanded: their digest cannot match the store.
+      const approvals = isMcpApprovalGateArmed(
+        config.getBareMode(),
+        config.isSafeMode(),
+        config.getApprovalMode(),
+      )
+        ? loadMcpApprovals()
+        : undefined;
 
       // Pool snapshot for per-server `entryCount` + `entrySummary`.
       // Captured once outside the per-server loop. Absent when the
@@ -6861,7 +6869,7 @@ class QwenAgent implements Agent {
               ...(hasOAuthTokens !== undefined ? { hasOAuthTokens } : {}),
               ...(requiresAuth ? { requiresAuth: true } : {}),
             };
-            if (isGatedMcpScope(server.scope)) {
+            if (approvals && isGatedMcpScope(server.scope)) {
               const approvalState = approvals.getState(
                 config.getWorkingDir(),
                 name,
@@ -10218,6 +10226,20 @@ class QwenAgent implements Agent {
               `MCP server is not approval-gated: ${serverName}`,
             );
           }
+          if (
+            !isMcpApprovalGateArmed(
+              config.getBareMode(),
+              config.isSafeMode(),
+              config.getApprovalMode(),
+            )
+          ) {
+            // Gate-off: the digest would be of the literal form.
+            throw RequestError.invalidParams(
+              undefined,
+              `MCP approval is off for this session; ${serverName} was not approved — ` +
+                `use \`qwen mcp approve\``,
+            );
+          }
           const approvals = loadMcpApprovals();
           await approvals.setState(
             config.getWorkingDir(),

packages/cli/src/commands/mcp/approve.test.ts

@@ -147,4 +147,24 @@ describe('qwen mcp approve / reject', () => {
     writeMcpJson({ slack: { command: 'curl', args: ['slack.js'] } });
     expect(stateOf('slack')).toBe('pending');
   });
+
+  // Digest is of the resolved config: approve hashes what boot hashes; rotation re-opens.
+  it('hashes the resolved config: rotating a referenced variable, file untouched, reverts to pending', async () => {
+    process.env['MCPAPPROVE_ROT'] = 'a';
+    try {
+      writeMcpJson({
+        slack: {
+          httpUrl: 'https://h.example/mcp',
+          headers: { Authorization: 'Bearer ${MCPAPPROVE_ROT}' },
+        },
+      });
+      await run(approveCommand, { name: 'slack', all: false });
+      expect(stateOf('slack')).toBe('approved');
+
+      process.env['MCPAPPROVE_ROT'] = 'b';
+      expect(stateOf('slack')).toBe('pending');
+    } finally {
+      delete process.env['MCPAPPROVE_ROT'];
+    }
+  });
 });