fix(cli): extend the #8663 loader denylist and harden its scrub lifecycle

docs/users/configuration/settings.md

@@ -724,7 +724,7 @@ For authentication-related variables (like `OPENAI_*`) and the recommended `.qwe
 
 > [!warning]
 >
-> **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `NODE_EXTRA_CA_CERTS`, or `DEV` (the dev-harness launch marker); those stay settable from the shell environment or a user-level `.env`. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses.
+> **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `OPENSSL_CONF` (dlopens an attacker OpenSSL engine at startup), `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `DEV` (the dev-harness launch marker), the TLS trust-anchor variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, `GIT_SSL_CAINFO` — an attacker CA there would enable MITM of the token-bearing traffic a session's `git`/`npm`/`pip`/`curl` calls make), the git command-execution variables (`GIT_SSH_COMMAND`, `GIT_EXTERNAL_DIFF`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT` and the numbered `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` pairs — git runs these on any session `git` invocation), or the node-gyp interpreter-selection variables (`NODE_GYP_FORCE_PYTHON`, `npm_config_python`, `PYTHON` — run as the build Python during native-addon installs). Those stay settable from the shell environment or a user-level `.env`; unlike the loader list above they are rejected from project files only, so a value you export yourself is preserved. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses.
 
 ### Environment Variables Table
 

packages/cli/src/commands/channel/daemon-worker.test.ts

@@ -66,6 +66,7 @@ const mockResolveProxyUrl = vi.hoisted(() =>
   vi.fn((_cliProxy?: string, settingsProxy?: string) => settingsProxy),
 );
 const mockWriteStderrLine = vi.hoisted(() => vi.fn());
+const mockWriteStderrLineSafe = vi.hoisted(() => vi.fn());
 const mockWriteStdoutLine = vi.hoisted(() => vi.fn());
 const mockSelectFirstModel = vi.hoisted(() =>
   vi.fn(
@@ -219,7 +220,7 @@ vi.mock('@qwen-code/qwen-code-core', () => ({
 
 vi.mock('../../utils/stdioHelpers.js', () => ({
   writeStderrLine: mockWriteStderrLine,
-  writeStderrLineSafe: vi.fn(),
+  writeStderrLineSafe: mockWriteStderrLineSafe,
   writeStdoutLine: mockWriteStdoutLine,
 }));
 
@@ -2033,6 +2034,13 @@ describe('daemonWorkerCommand', () => {
 
     expect(process.env['NODE_OPTIONS']).toBeUndefined();
     expect(process.env['npm_config_node-options']).toBeUndefined();
+    // Pin the channel-boundary breadcrumb, not just the removal: a refactor
+    // onto the silent scrubInheritedLoaderEnv variant deletes the keys the
+    // same way but drops the operator diagnostic — the reason the *AndReport*
+    // helper exists.
+    expect(mockWriteStderrLineSafe).toHaveBeenCalledWith(
+      expect.stringContaining('scrubbed inherited loader env vars'),
+    );
   });
 
   it('scrubs daemon connection env when required env validation fails', async () => {

packages/cli/src/config/environment.test.ts

@@ -38,6 +38,16 @@ const TRACKED_ENV = [
   'Node_Options',
   'ZDOTDIR',
   'BASH_FUNC_id%%',
+  'OPENSSL_CONF',
+  'NODE_REPL_EXTERNAL_MODULE',
+  'npm_config_node_gyp',
+  'npm_config_init_module',
+  'SSL_CERT_FILE',
+  'GIT_SSH_COMMAND',
+  'GIT_CONFIG_COUNT',
+  'GIT_CONFIG_KEY_0',
+  'GIT_CONFIG_VALUE_0',
+  'PYTHON',
   'NODE_COMPILE_CACHE',
   'NODE_DISABLE_COMPILE_CACHE',
   'NODE_EXTRA_CA_CERTS',
@@ -307,6 +317,61 @@ describe('loadEnvironment', () => {
     expect(process.env['RUNTIME_DOTENV']).toBe('allowed');
   });
 
+  // #8663 follow-up: pure-injection loader keys (dlopen/require/exec redirects)
+  // join the scrubbed loader set and are rejected from every .env scope.
+  it('never applies the follow-up code-injection loader keys from .env', () => {
+    const workspace = makeWorkspace();
+    fs.writeFileSync(
+      path.join(workspace, '.env'),
+      [
+        'OPENSSL_CONF=/workspace-a/evil.cnf',
+        'NODE_REPL_EXTERNAL_MODULE=/workspace-a/hook.js',
+        'npm_config_node_gyp=/workspace-a/evil-gyp.js',
+        'npm_config_init_module=/workspace-a/evil-init.js',
+        'RUNTIME_DOTENV=allowed',
+        '',
+      ].join('\n'),
+    );
+
+    loadEnvironment(testSettings({}), workspace);
+
+    expect(process.env['OPENSSL_CONF']).toBeUndefined();
+    expect(process.env['NODE_REPL_EXTERNAL_MODULE']).toBeUndefined();
+    expect(process.env['npm_config_node_gyp']).toBeUndefined();
+    expect(process.env['npm_config_init_module']).toBeUndefined();
+    expect(process.env['RUNTIME_DOTENV']).toBe('allowed');
+  });
+
+  // #8663 follow-up: TLS trust anchors, the git command-exec family (incl.
+  // numbered GIT_CONFIG_KEY_/VALUE_ pairs), and node-gyp interpreter selection
+  // join the hardcoded reject-from-project-.env tier.
+  it('never applies the follow-up TLS/git/interpreter keys from a project .env', () => {
+    const workspace = makeWorkspace();
+    fs.writeFileSync(
+      path.join(workspace, '.env'),
+      [
+        'SSL_CERT_FILE=/workspace-a/evil-ca.pem',
+        'GIT_SSH_COMMAND=/workspace-a/evil-ssh.sh',
+        'GIT_CONFIG_COUNT=1',
+        'GIT_CONFIG_KEY_0=core.hooksPath',
+        'GIT_CONFIG_VALUE_0=/workspace-a/evil-hooks',
+        'PYTHON=/workspace-a/evil-python',
+        'RUNTIME_DOTENV=allowed',
+        '',
+      ].join('\n'),
+    );
+
+    loadEnvironment(testSettings({}), workspace);
+
+    expect(process.env['SSL_CERT_FILE']).toBeUndefined();
+    expect(process.env['GIT_SSH_COMMAND']).toBeUndefined();
+    expect(process.env['GIT_CONFIG_COUNT']).toBeUndefined();
+    expect(process.env['GIT_CONFIG_KEY_0']).toBeUndefined();
+    expect(process.env['GIT_CONFIG_VALUE_0']).toBeUndefined();
+    expect(process.env['PYTHON']).toBeUndefined();
+    expect(process.env['RUNTIME_DOTENV']).toBe('allowed');
+  });
+
   // The privileged <workspace>/.qwen/.env scope deliberately bypasses
   // excludedEnvVars and is discovered before the plain .env, so exempting it
   // from the loader denylist must not ship green.

packages/cli/src/config/shared-env-keys.test.ts

@@ -4,17 +4,22 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { describe, it, expect, vi } from 'vitest';
+import { describe, it, expect, vi, afterEach } from 'vitest';
 import {
+  acquireInheritedLoaderEnvScrub,
+  clearLoaderKeyRejectionReporterIfCurrent,
   HOME_ENV_BOOTSTRAP_KEYS,
   INHERITED_LOADER_ENV_KEYS,
   isHardcodedProjectEnvExclusion,
   isLoaderEnvKey,
+  type LoaderKeyRejectionReporter,
   PROJECT_ENV_HARDCODED_EXCLUSIONS,
   reportRejectedLoaderKeys,
+  resetInheritedLoaderEnvScrubForTesting,
   resetLoaderKeyRejectionReportingForTesting,
   scrubAndReportInheritedLoaderEnv,
   scrubInheritedLoaderEnv,
+  setLoaderKeyRejectionReporter,
 } from './shared-env-keys.js';
 
 describe('PROJECT_ENV_HARDCODED_EXCLUSIONS', () => {
@@ -68,6 +73,49 @@ describe('PROJECT_ENV_HARDCODED_EXCLUSIONS', () => {
     expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain('DEV');
   });
 
+  // The non-Node TLS trust-anchor vars reach the same MITM outcome as
+  // NODE_EXTRA_CA_CERTS for the curl/git/openssl/python tools a session
+  // shells out to; a project .env must not inject an attacker CA.
+  it('excludes the non-Node TLS trust-anchor vars', () => {
+    for (const key of [
+      'SSL_CERT_FILE',
+      'SSL_CERT_DIR',
+      'CURL_CA_BUNDLE',
+      'REQUESTS_CA_BUNDLE',
+      'GIT_SSL_CAINFO',
+    ]) {
+      expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain(key);
+    }
+  });
+
+  // git executes these on any session git invocation (SSH command, external
+  // diff, config-injected core.hooksPath); a project .env setting them is
+  // code execution as the daemon user.
+  it('excludes the git command-execution env family', () => {
+    for (const key of [
+      'GIT_SSH_COMMAND',
+      'GIT_EXTERNAL_DIFF',
+      'GIT_CONFIG_GLOBAL',
+      'GIT_CONFIG_SYSTEM',
+      'GIT_CONFIG_COUNT',
+    ]) {
+      expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain(key);
+    }
+  });
+
+  // node-gyp interpreter selection runs the pointed-at file as the build
+  // Python; a project .env must not redirect it.
+  it('excludes the node-gyp interpreter-selection and git-binary vars', () => {
+    for (const key of [
+      'NODE_GYP_FORCE_PYTHON',
+      'npm_config_python',
+      'PYTHON',
+      'npm_config_git',
+    ]) {
+      expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain(key);
+    }
+  });
+
   // Workspace settings.env QWEN_SERVER_TOKEN is an intentional fast-path
   // feature (fast-path.test.ts loads it without the full settings loader);
   // it stays reload-only rather than hardcoded-excluded.
@@ -95,6 +143,28 @@ describe('isHardcodedProjectEnvExclusion', () => {
     expect(isHardcodedProjectEnvExclusion('QWEN_SERVER_TOKEN')).toBe(false);
     expect(isHardcodedProjectEnvExclusion('NODE_OPTIONS')).toBe(false);
   });
+
+  it('matches the newly added hardcoded exclusions case-insensitively', () => {
+    expect(isHardcodedProjectEnvExclusion('SSL_CERT_FILE')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('ssl_cert_file')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('GIT_SSH_COMMAND')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('git_ssh_command')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('PYTHON')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('python')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('npm_config_python')).toBe(true);
+  });
+
+  // Numbered GIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n> pairs are an unbounded
+  // index, matched by prefix rather than literal membership.
+  it('matches numbered GIT_CONFIG_KEY_/VALUE_ pairs by prefix', () => {
+    expect(isHardcodedProjectEnvExclusion('GIT_CONFIG_KEY_0')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('GIT_CONFIG_VALUE_0')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('git_config_key_12')).toBe(true);
+    expect(isHardcodedProjectEnvExclusion('GIT_CONFIG_VALUE_7')).toBe(true);
+    // A key that merely starts with GIT_CONFIG but is not a KEY_/VALUE_ pair
+    // (and not a listed literal) is not excluded.
+    expect(isHardcodedProjectEnvExclusion('GIT_CONFIG_PARAMETERS')).toBe(false);
+  });
 });
 
 describe('isLoaderEnvKey', () => {
@@ -137,6 +207,29 @@ describe('isLoaderEnvKey', () => {
     expect(isLoaderEnvKey('bash_func_id%%')).toBe(true);
   });
 
+  // Pure code-injection vectors with no benign cross-workspace inheritance:
+  // OPENSSL_CONF (dlopen an engine at crypto init), NODE_REPL_EXTERNAL_MODULE
+  // (require at REPL start), and the npm node-gyp/init script redirects.
+  it('matches the pure-injection loader keys added for the #8653 follow-up', () => {
+    expect(isLoaderEnvKey('OPENSSL_CONF')).toBe(true);
+    expect(isLoaderEnvKey('openssl_conf')).toBe(true);
+    expect(isLoaderEnvKey('NODE_REPL_EXTERNAL_MODULE')).toBe(true);
+    expect(isLoaderEnvKey('npm_config_node_gyp')).toBe(true);
+    expect(isLoaderEnvKey('npm_config_node-gyp')).toBe(true);
+    expect(isLoaderEnvKey('npm_config_init_module')).toBe(true);
+    expect(isLoaderEnvKey('npm_config_init-module')).toBe(true);
+  });
+
+  // The interpreter-selection vars stay in the reject-only hardcoded tier
+  // (they have a legitimate operator-shell use), NOT the scrubbed loader set.
+  it('does not scrub interpreter-selection or TLS-anchor keys', () => {
+    expect(isLoaderEnvKey('PYTHON')).toBe(false);
+    expect(isLoaderEnvKey('npm_config_python')).toBe(false);
+    expect(isLoaderEnvKey('NODE_GYP_FORCE_PYTHON')).toBe(false);
+    expect(isLoaderEnvKey('SSL_CERT_FILE')).toBe(false);
+    expect(isLoaderEnvKey('GIT_SSH_COMMAND')).toBe(false);
+  });
+
   // Library search paths and the interactive-sh-only ENV are deliberately
   // reload-only: scrubbing them breaks mainstream toolchains.
   it('does not match search paths or the ENV convention', () => {
@@ -247,8 +340,12 @@ describe('scrubInheritedLoaderEnv', () => {
       'LD_PRELOAD',
       'NODE_OPTIONS',
       'NODE_PATH',
+      'NODE_REPL_EXTERNAL_MODULE',
+      'OPENSSL_CONF',
       'ZDOTDIR',
       'npm_config_globalconfig',
+      'npm_config_init_module',
+      'npm_config_node_gyp',
       'npm_config_node_options',
       'npm_config_prefix',
       'npm_config_script_shell',
@@ -370,3 +467,114 @@ describe('scrubAndReportInheritedLoaderEnv', () => {
     expect(breadcrumb).toBe('');
   });
 });
+
+// #8663 follow-up (concurrency): overlapping embedded daemons in one process
+// share process.env; a per-daemon scrub+restore races and re-poisons the
+// survivor. acquireInheritedLoaderEnvScrub reference-counts so only the last
+// release restores.
+describe('acquireInheritedLoaderEnvScrub', () => {
+  afterEach(() => {
+    resetInheritedLoaderEnvScrubForTesting();
+    delete process.env['NODE_OPTIONS'];
+    delete process.env['LD_PRELOAD'];
+  });
+
+  it('does not restore loader vars while a second holder is still active', () => {
+    resetInheritedLoaderEnvScrubForTesting();
+    const poison = '--import file:///workspace-a/register.mjs';
+    process.env['NODE_OPTIONS'] = poison;
+
+    const write = vi
+      .spyOn(process.stderr, 'write')
+      .mockImplementation(() => true);
+    try {
+      // Daemon A boots and scrubs the shared env.
+      const daemonA = acquireInheritedLoaderEnvScrub(
+        process.env,
+        'qwen serve',
+        'daemon',
+      );
+      expect(daemonA.removedKeys).toContain('NODE_OPTIONS');
+      expect(process.env['NODE_OPTIONS']).toBeUndefined();
+
+      // Daemon B boots into the already-scrubbed env: nothing left to remove.
+      const daemonB = acquireInheritedLoaderEnvScrub(
+        process.env,
+        'qwen serve',
+        'daemon',
+      );
+      expect(daemonB.removedKeys).toEqual([]);
+      expect(process.env['NODE_OPTIONS']).toBeUndefined();
+
+      // A closes first. Its restore must NOT re-poison B's still-live
+      // session subprocesses — the regression this guard closes.
+      daemonA.release();
+      expect(process.env['NODE_OPTIONS']).toBeUndefined();
+
+      // Only when the last holder releases is the original value restored.
+      daemonB.release();
+      expect(process.env['NODE_OPTIONS']).toBe(poison);
+    } finally {
+      write.mockRestore();
+    }
+  });
+
+  it('release is idempotent and a later assignment wins over the restore', () => {
+    resetInheritedLoaderEnvScrubForTesting();
+    process.env['LD_PRELOAD'] = '/evil.so';
+    const write = vi
+      .spyOn(process.stderr, 'write')
+      .mockImplementation(() => true);
+    try {
+      const handle = acquireInheritedLoaderEnvScrub(
+        process.env,
+        'qwen serve',
+        'daemon',
+      );
+      expect(process.env['LD_PRELOAD']).toBeUndefined();
+      // A legitimate re-assignment before release must survive the restore.
+      process.env['LD_PRELOAD'] = '/legit.so';
+      handle.release();
+      handle.release(); // idempotent
+      expect(process.env['LD_PRELOAD']).toBe('/legit.so');
+    } finally {
+      write.mockRestore();
+    }
+  });
+});
+
+describe('clearLoaderKeyRejectionReporterIfCurrent', () => {
+  afterEach(() => {
+    setLoaderKeyRejectionReporter(undefined);
+    resetLoaderKeyRejectionReportingForTesting();
+  });
+
+  it('clears only when the given reporter is still the active one', () => {
+    resetLoaderKeyRejectionReportingForTesting();
+    const calls: string[] = [];
+    const reporterA: LoaderKeyRejectionReporter = () => calls.push('A');
+    const reporterB: LoaderKeyRejectionReporter = () => calls.push('B');
+
+    const write = vi
+      .spyOn(process.stderr, 'write')
+      .mockImplementation(() => true);
+    try {
+      // Two co-resident daemons install their reporters in turn.
+      setLoaderKeyRejectionReporter(reporterA);
+      setLoaderKeyRejectionReporter(reporterB);
+
+      // Daemon A's close must not drop daemon B's reporter.
+      clearLoaderKeyRejectionReporterIfCurrent(reporterA);
+      reportRejectedLoaderKeys('/ws-b/.env', ['NODE_OPTIONS']);
+      expect(calls).toEqual(['B']);
+
+      // Daemon B's close clears it; further rejections fall back to stderr.
+      clearLoaderKeyRejectionReporterIfCurrent(reporterB);
+      resetLoaderKeyRejectionReportingForTesting();
+      reportRejectedLoaderKeys('/ws-b/.env', ['LD_PRELOAD']);
+      expect(calls).toEqual(['B']);
+    } finally {
+      write.mockRestore();
+    }
+  });
+});