From 278aa21d7d011ec91f33174d44377af80f25c741 Mon Sep 17 00:00:00 2001 From: chinesepowered <22500229+chinesepowered@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:13:32 -0700 Subject: [PATCH 1/3] fix(core): strip Qwen-internal daemon secrets from agent-spawned child env Shell subprocesses (and the monitor tool and stdio MCP servers) inherited the full daemon process.env, including QWEN_SERVER_TOKEN (the serve-daemon bearer credential), so an agent-run command like printenv QWEN_SERVER_TOKEN could read an internal secret. Add a shared sanitizeChildEnv() that removes Qwen-internal daemon/server tokens (QWEN_SERVER_TOKEN, QWEN_DAEMON_TOKEN) before spawning, and apply it at the shell child_process + PTY paths, monitor.ts, and the mcp-client stdio transport. The denylist is deliberately narrow: it does NOT strip third-party credentials (GH_TOKEN, AWS_*, NPM_TOKEN, ...) that real shell workflows legitimately inherit -- only Qwen-internal secrets. Exported from the package root so the desktop denylists can consolidate onto it later. Fixes #6601. --- packages/core/src/index.ts | 1 + .../services/shellExecutionService.test.ts | 51 +++++++++++++ .../src/services/shellExecutionService.ts | 5 +- packages/core/src/tools/mcp-client.ts | 3 +- packages/core/src/tools/monitor.ts | 3 +- .../core/src/utils/sanitize-child-env.test.ts | 72 +++++++++++++++++++ packages/core/src/utils/sanitize-child-env.ts | 45 ++++++++++++ 7 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 packages/core/src/utils/sanitize-child-env.test.ts create mode 100644 packages/core/src/utils/sanitize-child-env.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0f760933d94..ba335185f12 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -322,6 +322,7 @@ export * from './services/usageHistoryService.js'; export * from './services/usage-dashboard-service.js'; export * from './utils/bareMode.js'; export * from './utils/safe-mode.js'; +export * from './utils/sanitize-child-env.js'; export * from './utils/toolResultDisplayCompaction.js'; // ============================================================================ diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index b3ac6a0e08d..24de33cb158 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -317,6 +317,31 @@ describe('ShellExecutionService', () => { return { result, handle, abortController }; }; + describe('child environment sanitization (#6601)', () => { + it('strips Qwen-internal daemon secrets from the pty child env while keeping user vars and third-party credentials', async () => { + process.env['QWEN_SERVER_TOKEN'] = 'serve-secret'; + process.env['QWEN_DAEMON_TOKEN'] = 'daemon-secret'; + process.env['GH_TOKEN'] = 'gh-abc'; + process.env['PATH'] = '/usr/bin'; + + await simulateExecution('echo hi', (pty) => { + pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); + }); + + const spawnEnv = ( + mockPtySpawn.mock.calls[0][2] as { env: NodeJS.ProcessEnv } + ).env; + // Internal daemon secrets must not leak into agent-run commands. + expect(spawnEnv['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(spawnEnv['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Benign vars + third-party credentials user commands rely on are kept. + expect(spawnEnv['PATH']).toContain('/usr/bin'); + expect(spawnEnv['GH_TOKEN']).toBe('gh-abc'); + // The shell tool's own marker is still applied on top. + expect(spawnEnv['QWEN_CODE']).toBe('1'); + }); + }); + describe('Successful Execution', () => { it('should execute a command and capture output', async () => { const { result, handle } = await simulateExecution('ls -l', (pty) => { @@ -2253,6 +2278,32 @@ describe('ShellExecutionService child_process fallback', () => { return { result, handle, abortController }; }; + describe('child environment sanitization (#6601)', () => { + it('strips Qwen-internal daemon secrets from the child_process env while keeping user vars and third-party credentials', async () => { + process.env['QWEN_SERVER_TOKEN'] = 'serve-secret'; + process.env['QWEN_DAEMON_TOKEN'] = 'daemon-secret'; + process.env['GH_TOKEN'] = 'gh-abc'; + process.env['PATH'] = '/usr/bin'; + + await simulateExecution('echo hi', (cp) => { + cp.emit('exit', 0, null); + cp.emit('close', 0, null); + }); + + const spawnEnv = ( + mockCpSpawn.mock.calls[0][2] as { env: NodeJS.ProcessEnv } + ).env; + // Internal daemon secrets must not leak into agent-run commands. + expect(spawnEnv['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(spawnEnv['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Benign vars + third-party credentials user commands rely on are kept. + expect(spawnEnv['PATH']).toContain('/usr/bin'); + expect(spawnEnv['GH_TOKEN']).toBe('gh-abc'); + // The shell tool's own marker is still applied on top. + expect(spawnEnv['QWEN_CODE']).toBe('1'); + }); + }); + describe('Successful Execution', () => { it('should execute a command and capture stdout and stderr', async () => { const { result, handle } = await simulateExecution('ls -l', (cp) => { diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index eb2a258ce8d..9a573c6b474 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -21,6 +21,7 @@ import { type AnsiOutput, } from '../utils/terminalSerializer.js'; import { normalizePathEnvForWindows } from '../utils/windowsPath.js'; +import { sanitizeChildEnv } from '../utils/sanitize-child-env.js'; import { formatMemoryUsage } from '../utils/formatters.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; import { createDebugLogger } from '../utils/debugLogger.js'; @@ -764,7 +765,7 @@ export class ShellExecutionService { detached: !isWindows, windowsHide: isWindows, env: { - ...normalizePathEnvForWindows(process.env), + ...normalizePathEnvForWindows(sanitizeChildEnv(process.env)), QWEN_CODE: '1', TERM: 'xterm-256color', ...getShellPagerEnv(pager, { @@ -1467,7 +1468,7 @@ export class ShellExecutionService { cols, rows, env: { - ...normalizePathEnvForWindows(process.env), + ...normalizePathEnvForWindows(sanitizeChildEnv(process.env)), QWEN_CODE: '1', TERM: 'xterm-256color', ...getShellPagerEnv(shellExecutionConfig.pager, { diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index ea7ead8fe8f..57ea7fcc563 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -59,6 +59,7 @@ import { getErrorMessage, getErrorStatus } from '../utils/errors.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { retryWithBackoff } from './mcp-retry.js'; import { normalizePathEnvForWindows } from '../utils/windowsPath.js'; +import { sanitizeChildEnv } from '../utils/sanitize-child-env.js'; import type { Unsubscribe, WorkspaceContext, @@ -2148,7 +2149,7 @@ export async function createTransport( // config providing its own PATH fully replaces the parent value instead of // being merged with a stale case-variant. const env = { - ...normalizePathEnvForWindows({ ...process.env }), + ...normalizePathEnvForWindows(sanitizeChildEnv(process.env)), ...(mcpServerConfig.env || {}), }; diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 181ac8d942e..ab2c925c467 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -56,6 +56,7 @@ import { import { getCurrentAgentId } from '../agents/runtime/agent-context.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; import { getShellPagerEnv } from '../utils/shell-pager-env.js'; +import { sanitizeChildEnv } from '../utils/sanitize-child-env.js'; const debugLogger = createDebugLogger('MONITOR'); @@ -365,7 +366,7 @@ class MonitorToolInvocation extends BaseToolInvocation< stdio: ['ignore', 'pipe', 'pipe'], detached: true, env: { - ...process.env, + ...sanitizeChildEnv(process.env), QWEN_CODE: '1', TERM: 'dumb', // no color codes for streaming ...getShellPagerEnv(this.config.getShellExecutionConfig().pager, { diff --git a/packages/core/src/utils/sanitize-child-env.test.ts b/packages/core/src/utils/sanitize-child-env.test.ts new file mode 100644 index 00000000000..c7dce534cd3 --- /dev/null +++ b/packages/core/src/utils/sanitize-child-env.test.ts @@ -0,0 +1,72 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + INTERNAL_SECRET_ENV_VARS, + sanitizeChildEnv, +} from './sanitize-child-env.js'; + +describe('sanitizeChildEnv', () => { + it('removes Qwen-internal daemon/server secrets', () => { + const result = sanitizeChildEnv({ + QWEN_SERVER_TOKEN: 'super-secret', + QWEN_DAEMON_TOKEN: 'also-secret', + PATH: '/usr/bin', + }); + expect(result['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(result['QWEN_DAEMON_TOKEN']).toBeUndefined(); + }); + + it('preserves benign vars and third-party credentials that shell workflows need', () => { + const result = sanitizeChildEnv({ + QWEN_SERVER_TOKEN: 'super-secret', + PATH: '/usr/bin', + GH_TOKEN: 'gh-abc', + GITHUB_TOKEN: 'gh-def', + AWS_ACCESS_KEY_ID: 'aws-key', + NPM_TOKEN: 'npm-tok', + HOME: '/home/user', + }); + expect(result['PATH']).toBe('/usr/bin'); + expect(result['GH_TOKEN']).toBe('gh-abc'); + expect(result['GITHUB_TOKEN']).toBe('gh-def'); + expect(result['AWS_ACCESS_KEY_ID']).toBe('aws-key'); + expect(result['NPM_TOKEN']).toBe('npm-tok'); + expect(result['HOME']).toBe('/home/user'); + }); + + it('does not mutate the input environment', () => { + const source: NodeJS.ProcessEnv = { + QWEN_SERVER_TOKEN: 'super-secret', + PATH: '/usr/bin', + }; + sanitizeChildEnv(source); + expect(source['QWEN_SERVER_TOKEN']).toBe('super-secret'); + }); + + it('returns a fresh object each call', () => { + const source: NodeJS.ProcessEnv = { PATH: '/usr/bin' }; + const a = sanitizeChildEnv(source); + const b = sanitizeChildEnv(source); + expect(a).not.toBe(source); + expect(a).not.toBe(b); + }); + + it('is a no-op for an env without internal secrets', () => { + const result = sanitizeChildEnv({ PATH: '/usr/bin', GH_TOKEN: 'x' }); + expect(result).toEqual({ PATH: '/usr/bin', GH_TOKEN: 'x' }); + }); + + it('keeps the denylist scoped to internal secrets only', () => { + // Guardrail: this list must not grow to include third-party credentials, + // which the shell tool legitimately inherits (see #6601 discussion). + expect([...INTERNAL_SECRET_ENV_VARS].sort()).toEqual([ + 'QWEN_DAEMON_TOKEN', + 'QWEN_SERVER_TOKEN', + ]); + }); +}); diff --git a/packages/core/src/utils/sanitize-child-env.ts b/packages/core/src/utils/sanitize-child-env.ts new file mode 100644 index 00000000000..f8c8ead3588 --- /dev/null +++ b/packages/core/src/utils/sanitize-child-env.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Env vars that carry Qwen-internal daemon/server bearer credentials. These + * must never be inherited by a child process the agent launches on the user's + * behalf — shell commands, the monitor tool, or a stdio MCP server — because + * no legitimate user command needs them and leaking them to arbitrary + * agent-run commands is a credential-exposure gap (issue #6601). + * + * `QWEN_SERVER_TOKEN` is the serve-daemon bearer token and `QWEN_DAEMON_TOKEN` + * is the channel-daemon worker token; the daemon worker already scrubs both + * from its own `process.env` after reading them. + * + * This denylist is intentionally NARROW: it strips only Qwen-internal secrets, + * NOT third-party credentials such as `GH_TOKEN`, `AWS_*`, or `NPM_TOKEN`. + * Real shell workflows legitimately depend on inheriting those (`gh`, the AWS + * CLI, `npm publish`, …), so stripping them here would break user commands. + * Broader third-party-credential stripping stays scoped to the sandbox / MCP + * infrastructure paths where it already lives. + */ +export const INTERNAL_SECRET_ENV_VARS: readonly string[] = [ + 'QWEN_SERVER_TOKEN', + 'QWEN_DAEMON_TOKEN', +]; + +/** + * Return a shallow copy of `env` with Qwen-internal daemon/server secrets + * removed, so it is safe to pass to a child process spawned on the user's + * behalf. Does not mutate the input. + * + * @param env The source environment (defaults to `process.env`). + */ +export function sanitizeChildEnv( + env: NodeJS.ProcessEnv = process.env, +): NodeJS.ProcessEnv { + const sanitized: NodeJS.ProcessEnv = { ...env }; + for (const key of INTERNAL_SECRET_ENV_VARS) { + delete sanitized[key]; + } + return sanitized; +} From 92e7036322673f82ce94000d4da367fd2522f288 Mon Sep 17 00:00:00 2001 From: chinesepowered <22500229+chinesepowered@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:30:45 -0700 Subject: [PATCH 2/3] test(core): cover daemon-secret stripping on monitor and mcp-client spawn sites --- packages/core/src/tools/mcp-client.test.ts | 21 ++++++++++++++ packages/core/src/tools/monitor.test.ts | 33 ++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 9fde941f130..9b98ee40f1b 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -2372,6 +2372,27 @@ describe('mcp-client', () => { }); }); + it('strips Qwen-internal daemon secrets from the stdio child env (#6601)', async () => { + process.env = { + ...ORIGINAL_ENV, + QWEN_SERVER_TOKEN: 'serve-secret', + QWEN_DAEMON_TOKEN: 'daemon-secret', + GH_TOKEN: 'gh-abc', + }; + const mockedTransport = vi + .spyOn(SdkClientStdioLib, 'StdioClientTransport') + .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport); + + await createTransport('test-server', { command: 'test-command' }, false); + + const transportEnv = mockedTransport.mock.calls[0]?.[0]?.env ?? {}; + // Internal daemon secrets must never reach an agent-launched stdio server. + expect(transportEnv['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(transportEnv['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Third-party credentials the server may legitimately need are preserved. + expect(transportEnv['GH_TOKEN']).toBe('gh-abc'); + }); + it('should normalize PATH-like env keys on Windows for stdio transport', async () => { vi.spyOn(process, 'platform', 'get').mockReturnValue('win32'); process.env = { diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 48839ae8262..e442690383c 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -745,6 +745,39 @@ describe('MonitorTool', () => { expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); }); + it('strips Qwen-internal daemon secrets from the monitor child env (#6601)', async () => { + const originalServerToken = process.env['QWEN_SERVER_TOKEN']; + const originalDaemonToken = process.env['QWEN_DAEMON_TOKEN']; + process.env['QWEN_SERVER_TOKEN'] = 'serve-secret'; + process.env['QWEN_DAEMON_TOKEN'] = 'daemon-secret'; + try { + const invocation = createInvocation({ + command: 'tail -f /var/log/app.log', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + // Internal daemon secrets must not leak into an agent-run monitor. + expect(spawnOptions.env['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(spawnOptions.env['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Benign inherited env is preserved and the monitor marker still applied. + expect(spawnOptions.env['PATH']).toBeDefined(); + expect(spawnOptions.env['QWEN_CODE']).toBe('1'); + } finally { + if (originalServerToken === undefined) { + delete process.env['QWEN_SERVER_TOKEN']; + } else { + process.env['QWEN_SERVER_TOKEN'] = originalServerToken; + } + if (originalDaemonToken === undefined) { + delete process.env['QWEN_DAEMON_TOKEN']; + } else { + process.env['QWEN_DAEMON_TOKEN'] = originalDaemonToken; + } + } + }); + it('does not spawn when the turn signal is already aborted', async () => { const invocation = createInvocation({ command: 'tail -f /var/log/app.log', From 2f2d10f2f38b86e09aac6eda6a97d0e22ca01e01 Mon Sep 17 00:00:00 2001 From: chinesepowered <22500229+chinesepowered@users.noreply.github.com> Date: Tue, 21 Jul 2026 08:52:22 -0700 Subject: [PATCH 3/3] test(core): replace process.env instead of mutating in shell sanitization tests The file restores process.env by reference in afterEach, so in-place key mutations leaked into later tests. Use the replacement pattern already used by setupConflictingPathEnv. --- .../services/shellExecutionService.test.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 24de33cb158..37ece77da29 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -319,10 +319,15 @@ describe('ShellExecutionService', () => { describe('child environment sanitization (#6601)', () => { it('strips Qwen-internal daemon secrets from the pty child env while keeping user vars and third-party credentials', async () => { - process.env['QWEN_SERVER_TOKEN'] = 'serve-secret'; - process.env['QWEN_DAEMON_TOKEN'] = 'daemon-secret'; - process.env['GH_TOKEN'] = 'gh-abc'; - process.env['PATH'] = '/usr/bin'; + // Replace (not mutate in place): this file restores process.env by + // reference in afterEach, so in-place keys would leak to later tests. + process.env = { + ...originalProcessEnv, + QWEN_SERVER_TOKEN: 'serve-secret', + QWEN_DAEMON_TOKEN: 'daemon-secret', + GH_TOKEN: 'gh-abc', + PATH: '/usr/bin', + }; await simulateExecution('echo hi', (pty) => { pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); @@ -2280,10 +2285,15 @@ describe('ShellExecutionService child_process fallback', () => { describe('child environment sanitization (#6601)', () => { it('strips Qwen-internal daemon secrets from the child_process env while keeping user vars and third-party credentials', async () => { - process.env['QWEN_SERVER_TOKEN'] = 'serve-secret'; - process.env['QWEN_DAEMON_TOKEN'] = 'daemon-secret'; - process.env['GH_TOKEN'] = 'gh-abc'; - process.env['PATH'] = '/usr/bin'; + // Replace (not mutate in place): this file restores process.env by + // reference in afterEach, so in-place keys would leak to later tests. + process.env = { + ...originalProcessEnv, + QWEN_SERVER_TOKEN: 'serve-secret', + QWEN_DAEMON_TOKEN: 'daemon-secret', + GH_TOKEN: 'gh-abc', + PATH: '/usr/bin', + }; await simulateExecution('echo hi', (cp) => { cp.emit('exit', 0, null);