diff --git a/packages/core/src/hooks/hookRunner.test.ts b/packages/core/src/hooks/hookRunner.test.ts index af7728fb6d7..2a5ed4768bb 100644 --- a/packages/core/src/hooks/hookRunner.test.ts +++ b/packages/core/src/hooks/hookRunner.test.ts @@ -123,6 +123,49 @@ describe('HookRunner', () => { expect(mockSpawn).toHaveBeenCalled(); }); + it('strips Qwen-internal daemon secrets from the hook 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 mockProcess = createMockProcess(0, 'hello'); + mockSpawn.mockImplementation(() => mockProcess); + + const hookConfig: HookConfig = { + type: HookType.Command, + command: 'echo hello', + source: HooksConfigSource.Project, + }; + + await hookRunner.executeHook( + hookConfig, + HookEventName.PreToolUse, + createMockInput(), + ); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + // A user-authored hook command is a child process launched on the + // agent's behalf; internal daemon secrets must not leak into it. + expect(spawnOptions.env['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(spawnOptions.env['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Benign inherited env and the hook's own vars are still present. + expect(spawnOptions.env['PATH']).toBeDefined(); + expect(spawnOptions.env['QWEN_PROJECT_DIR']).toBe('/test'); + } 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('should return failure for non-zero exit code', async () => { const mockProcess = createMockProcess(1, '', 'error'); mockSpawn.mockImplementation(() => mockProcess); diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index d09ae277084..704a478f527 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -31,6 +31,7 @@ import { PromptHookRunner } from './promptHookRunner.js'; import { AsyncHookRegistry, generateHookId } from './asyncHookRegistry.js'; import type { Config } from '../config/config.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; +import { sanitizeChildEnv } from '../utils/sanitize-child-env.js'; const debugLogger = createDebugLogger('TRUSTED_HOOKS'); @@ -584,7 +585,9 @@ export class HookRunner { ); const env = { - ...process.env, + // Hook commands are child processes launched on the agent's behalf, + // so they must not inherit Qwen-internal daemon secrets. + ...sanitizeChildEnv(process.env), GEMINI_PROJECT_DIR: input.cwd, CLAUDE_PROJECT_DIR: input.cwd, // For compatibility QWEN_PROJECT_DIR: input.cwd, // For Qwen Code compatibility diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 1d34af77a3e..8e46dc3449c 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -842,6 +842,96 @@ describe('ToolRegistry', () => { }); }); + it('strips Qwen-internal daemon secrets from the discovery and tool-call 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 { + mockConfigGetToolDiscoveryCommand.mockReturnValue( + 'my-discovery-command', + ); + vi.spyOn(config, 'getToolCallCommand').mockReturnValue( + 'my-call-command', + ); + + const toolDeclaration: FunctionDeclaration = { + name: 'secret-probe', + description: 'A tool', + parametersJsonSchema: { type: 'object', properties: {} }, + }; + + const mockSpawn = vi.mocked(spawn); + const discoveryProcess = { + stdout: { on: vi.fn(), removeListener: vi.fn() }, + stderr: { on: vi.fn(), removeListener: vi.fn() }, + on: vi.fn(), + }; + mockSpawn.mockReturnValueOnce(discoveryProcess as any); + discoveryProcess.stdout.on.mockImplementation((event, callback) => { + if (event === 'data') { + callback( + Buffer.from( + JSON.stringify([{ functionDeclarations: [toolDeclaration] }]), + ), + ); + } + }); + discoveryProcess.on.mockImplementation((event, callback) => { + if (event === 'close') { + callback(0); + } + }); + + await toolRegistry.discoverAllTools(); + const discoveredTool = toolRegistry.getTool('secret-probe'); + expect(discoveredTool).toBeDefined(); + + const executionProcess = { + stdout: { on: vi.fn(), removeListener: vi.fn() }, + stderr: { on: vi.fn(), removeListener: vi.fn() }, + stdin: { write: vi.fn(), end: vi.fn() }, + on: vi.fn(), + connected: true, + disconnect: vi.fn(), + removeListener: vi.fn(), + }; + mockSpawn.mockReturnValueOnce(executionProcess as any); + executionProcess.on.mockImplementation((event, callback) => { + if (event === 'close') { + callback(0); + } + }); + + await (discoveredTool as DiscoveredTool) + .build({}) + .execute(new AbortController().signal); + + // Both the discovery command and the tool-call command are child + // processes launched on the agent's behalf, so neither may inherit + // the internal daemon secrets. + for (const call of mockSpawn.mock.calls) { + const env = (call[2] as { env: NodeJS.ProcessEnv }).env; + expect(env['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(env['QWEN_DAEMON_TOKEN']).toBeUndefined(); + // Benign inherited env is preserved. + expect(env['PATH']).toBeDefined(); + } + expect(mockSpawn.mock.calls).toHaveLength(2); + } 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('should return a DISCOVERED_TOOL_EXECUTION_ERROR on tool failure', async () => { const discoveryCommand = 'my-discovery-command'; mockConfigGetToolDiscoveryCommand.mockReturnValue(discoveryCommand); diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 76a16c33f26..5e44dd7b601 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -24,6 +24,8 @@ import { ToolErrorType } from './tool-error.js'; import { safeJsonStringify } from '../utils/safeJsonStringify.js'; import type { EventEmitter } from 'node:events'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { sanitizeChildEnv } from '../utils/sanitize-child-env.js'; +import { normalizePathEnvForWindows } from '../utils/windowsPath.js'; import type { ReadResourceResult } from '@modelcontextprotocol/sdk/types.js'; import { normalizeMcpToolName } from '../utils/tool-name-utils.js'; @@ -61,7 +63,14 @@ class DiscoveredToolInvocation extends BaseToolInvocation< _updateOutput?: (output: ToolResultDisplay) => void, ): Promise { const callCommand = this.config.getToolCallCommand()!; - const child = spawn(callCommand, [this.toolName]); + // The user-configured tool-call command is a child process launched on the + // agent's behalf, so it must not inherit Qwen-internal daemon secrets. + // Passing `env` explicitly loses the native inheritance that resolved + // Windows' case-insensitive PATH keys, so normalize as the shell and MCP + // spawn sites do (a no-op off win32). + const child = spawn(callCommand, [this.toolName], { + env: normalizePathEnvForWindows(sanitizeChildEnv(process.env)), + }); child.stdin.write(JSON.stringify(this.params)); child.stdin.end(); @@ -592,7 +601,12 @@ export class ToolRegistry { 'Tool discovery command is empty or contains only whitespace.', ); } - const proc = spawn(cmdParts[0] as string, cmdParts.slice(1) as string[]); + // Same as the tool-call command above: the discovery command is + // agent-launched, must not inherit Qwen-internal daemon secrets, and + // needs the Windows PATH normalization that comes with an explicit env. + const proc = spawn(cmdParts[0] as string, cmdParts.slice(1) as string[], { + env: normalizePathEnvForWindows(sanitizeChildEnv(process.env)), + }); let stdout = ''; const stdoutDecoder = new StringDecoder('utf8'); let stderr = '';