Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

// ============================================================================
Expand Down
61 changes: 61 additions & 0 deletions packages/core/src/services/shellExecutionService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,36 @@ 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 () => {
// 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 });
});

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) => {
Expand Down Expand Up @@ -2253,6 +2283,37 @@ 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 () => {
// 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);
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) => {
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/services/shellExecutionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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, {
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/tools/mcp-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/tools/mcp-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 || {}),
};

Expand Down
33 changes: 33 additions & 0 deletions packages/core/src/tools/monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/tools/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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, {
Expand Down
72 changes: 72 additions & 0 deletions packages/core/src/utils/sanitize-child-env.test.ts
Original file line number Diff line number Diff line change
@@ -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',
]);
});
});
45 changes: 45 additions & 0 deletions packages/core/src/utils/sanitize-child-env.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Loading