fix(hooks): close four trust-boundary holes in hook execution

docs/users/features/hooks.md

@@ -117,7 +117,7 @@ By default, HTTP hooks cannot target private or link-local IP ranges. In platfor
 
 - This setting is **only honored from User, System, and SystemDefaults settings scopes**. A value set in Workspace (project) settings is ignored and logged as a warning, so a cloned repository can never self-grant this bypass.
 - The flag relaxes only the general private/CGNAT/link-local **range** checks. Cloud metadata endpoints stay blocked in every configuration: the `BLOCKED_HOSTS` list is matched literally (`metadata.google.internal`, `metadata.azure.internal`, ...), and the metadata IPs `169.254.169.254` and `100.100.100.200` are blocked in all serialized forms (including IPv4-mapped IPv6 such as `::ffff:a9fe:a9fe`) and after DNS resolution.
-- The `security.allowedHttpHookUrls` whitelist still applies independently. In managed environments, pair this flag with a whitelist so only the intended internal endpoints are reachable.
+- The `security.allowedHttpHookUrls` whitelist still applies independently. In managed environments, pair this flag with a whitelist so only the intended internal endpoints are reachable. Like this flag, the whitelist is **honored from User, System, and SystemDefaults settings scopes**; a value set in Workspace (project) settings can only _narrow_ the User or SystemDefaults whitelist and is logged as a warning: workspace entries that no higher-scope entry covers are dropped, when no higher scope sets a whitelist the workspace value is ignored entirely, and a System-scope whitelist always takes precedence over the workspace value (an empty whitelist means "allow all", so a repository can neither widen where hook payloads may be sent nor establish a whitelist of its own).
 
 > **Warning:** Enabling this flag lets hooks reach internal infrastructure on your network. Enable it only in trusted, managed settings — never in a repository you do not control.
 

integration-tests/terminal-capture/skill-review-harness/text-capture.tsx

@@ -230,11 +230,28 @@ async function main() {
   const coreSrcUrl = pathToFileURL(
     path.join(repoRoot, 'packages', 'core', 'index.ts'),
   ).href;
+  // Core's package.json maps the ./envVarResolver subpath to dist, which a
+  // fresh checkout lacks (and a built one may keep stale). `after` mode
+  // pulls it in via SkillReviewDialog -> config/settings.js, so it needs
+  // the same source remap as the bare specifier (mirrors scripts/dev.js).
+  const envVarResolverSrcUrl = pathToFileURL(
+    path.join(
+      repoRoot,
+      'packages',
+      'core',
+      'src',
+      'utils',
+      'envVarResolver.ts',
+    ),
+  ).href;
   const loader = `
     export function resolve(specifier, context, nextResolve) {
       if (specifier === '@qwen-code/qwen-code-core') {
         return { shortCircuit: true, url: '${coreSrcUrl}', format: 'module' };
       }
+      if (specifier === '@qwen-code/qwen-code-core/envVarResolver') {
+        return { shortCircuit: true, url: '${envVarResolverSrcUrl}', format: 'module' };
+      }
       return nextResolve(specifier, context);
     }
   `;

packages/cli/src/commands/channel/config-utils.test.ts

@@ -805,3 +805,84 @@ describe('parseChannelConfig', () => {
     );
   });
 });
+
+describe('internal-secret denylist', () => {
+  const SECRET = 'QWEN_SERVER_TOKEN';
+
+  afterEach(() => {
+    delete process.env[SECRET];
+    delete process.env[SECRET.toLowerCase()];
+  });
+
+  it('resolveEnvVars rejects Qwen-internal secrets instead of yielding the name as a value', () => {
+    process.env[SECRET] = 'daemon-secret';
+    expect(() => resolveEnvVars(`$${SECRET}`)).toThrow(
+      `${SECRET} is a Qwen-internal secret`,
+    );
+  });
+
+  it('resolveEnvVars rejects case variants (Windows-style process.env)', () => {
+    process.env[SECRET.toLowerCase()] = 'daemon-secret';
+    expect(() => resolveEnvVars(`$${SECRET.toLowerCase()}`)).toThrow(
+      'is a Qwen-internal secret',
+    );
+  });
+
+  it('parseChannelConfig rejects internal secrets in channel credentials', async () => {
+    process.env[SECRET] = 'daemon-secret';
+
+    await expect(
+      parseChannelConfig('bot', {
+        type: 'github',
+        token: `$${SECRET}`,
+      }),
+    ).rejects.toThrow(`${SECRET} is a Qwen-internal secret`);
+
+    await expect(
+      parseChannelConfig(
+        'bot',
+        { type: 'github', token: `$${SECRET}` },
+        undefined,
+        { resolveEnvVars: 'available' },
+      ),
+    ).rejects.toThrow(`${SECRET} is a Qwen-internal secret`);
+  });
+
+  it('webhook secret and secretEnv reject Qwen-internal secrets', async () => {
+    process.env[SECRET] = 'daemon-secret';
+
+    await expect(
+      parseChannelConfig('dingtalk-main', {
+        type: 'bare',
+        token: 'token',
+        webhooks: {
+          sources: {
+            custom: {
+              secret: `$${SECRET}`,
+              targets: { default: { chatId: 'group-1', senderId: 'webhook' } },
+            },
+          },
+        },
+      }),
+    ).rejects.toThrow(`${SECRET} is a Qwen-internal secret`);
+
+    // secretEnv used to return the variable *name* as the secret — a
+    // public constant that makes HMAC verification bypassable.
+    await expect(
+      parseChannelConfig('dingtalk-main', {
+        type: 'bare',
+        token: 'token',
+        webhooks: {
+          sources: {
+            custom: {
+              secretEnv: SECRET,
+              targets: { default: { chatId: 'group-1', senderId: 'webhook' } },
+            },
+          },
+        },
+      }),
+    ).rejects.toThrow(
+      `references a Qwen-internal secret (${SECRET}); internal secrets are never resolved into channel config`,
+    );
+  });
+});

packages/cli/src/commands/channel/config-utils.ts

@@ -4,6 +4,7 @@ import type {
   ChannelWebhookSourceConfig,
   ChannelWebhookTargetConfig,
 } from '@qwen-code/channel-base';
+import { isInternalSecretEnvVar } from '@qwen-code/qwen-code-core/envVarResolver';
 import { resolveChannelCwd } from './channel-cwd.js';
 import { getPlugin, supportedTypes } from './channel-registry.js';
 
@@ -29,6 +30,16 @@ export function resolveEnvVars(
   }
   if (value.startsWith('$')) {
     const envName = value.substring(1);
+    // Qwen-internal secrets never resolve into channel config: resolved
+    // values are sent to repo-configured endpoints. The core resolver
+    // preserves placeholders, but channel config throws at config time
+    // like every other unresolvable reference — silently keeping the
+    // literal would turn a secret *name* into a public, guessable value.
+    if (isInternalSecretEnvVar(envName)) {
+      throw new Error(
+        `Environment variable ${envName} is a Qwen-internal secret; internal secrets are never resolved into channel config (referenced as ${value})`,
+      );
+    }
     const envValue = env[envName];
     if (envValue === undefined) {
       throw new Error(
@@ -76,6 +87,11 @@ function resolveConfigEnvVar(value: string, mode: EnvResolution): string {
   if (value.startsWith('$$')) return value.substring(1);
   if (mode === 'available' && value.startsWith('$')) {
     const envName = value.substring(1);
+    if (isInternalSecretEnvVar(envName)) {
+      throw new Error(
+        `Environment variable ${envName} is a Qwen-internal secret; internal secrets are never resolved into channel config (referenced as ${value})`,
+      );
+    }
     const envValue = process.env[envName];
     if (envValue === undefined) {
       throw new Error(
@@ -290,6 +306,11 @@ function resolveWebhookSecretEnv(
       `Channel "${channelName}" field "${path}.secretEnv" must be an environment variable name or $-prefixed reference.`,
     );
   }
+  if (isInternalSecretEnvVar(envName)) {
+    throw new Error(
+      `Channel "${channelName}" field "${path}.secretEnv" references a Qwen-internal secret (${envName}); internal secrets are never resolved into channel config.`,
+    );
+  }
   const envValue = env[envName];
   if (envValue === undefined) {
     throw new Error(

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

@@ -3508,6 +3508,100 @@ describe('loadCliConfig allowPrivateNetworkHooks', () => {
   });
 });
 
+describe('loadCliConfig allowedHttpHookUrls', () => {
+  const originalArgv = process.argv;
+
+  beforeEach(() => {
+    vi.resetAllMocks();
+    vi.mocked(os.homedir).mockReturnValue('/mock/home/user');
+    vi.stubEnv('GEMINI_API_KEY', 'test-api-key');
+  });
+
+  afterEach(() => {
+    process.argv = originalArgv;
+    vi.unstubAllEnvs();
+    vi.restoreAllMocks();
+  });
+
+  it('should pass through security.allowedHttpHookUrls from settings', async () => {
+    process.argv = ['node', 'script.js'];
+    const settings: Settings = {
+      security: {
+        allowedHttpHookUrls: ['https://hooks.corp.com/*'],
+      },
+    };
+    const argv = await parseArguments();
+    const config = await loadCliConfig(settings, argv, undefined, []);
+    expect(config.getAllowedHttpHookUrls()).toEqual([
+      'https://hooks.corp.com/*',
+    ]);
+  });
+
+  it('should read a malformed non-array allowedHttpHookUrls as an empty list instead of crashing', async () => {
+    process.argv = ['node', 'script.js'];
+    // A hand-edited settings file can hold a bare string here (settings
+    // are validated only as top-level JSON objects); it must be coerced
+    // before reaching the UrlValidator instead of aborting startup.
+    const settings: Settings = {
+      security: {
+        allowedHttpHookUrls: 'https://hooks.corp.com/*' as unknown as string[],
+      },
+    };
+    const argv = await parseArguments();
+    const config = await loadCliConfig(settings, argv, undefined, []);
+    expect(config.getAllowedHttpHookUrls()).toEqual([]);
+  });
+
+  it('should warn that a malformed allowedHttpHookUrls leaves HTTP hooks unrestricted', async () => {
+    process.argv = ['node', 'script.js'];
+    // The coercion to [] reads as "allow all" in the UrlValidator; the
+    // user's attempted restriction vanishes, so it must be surfaced.
+    const settings: Settings = {
+      security: {
+        allowedHttpHookUrls: 'https://hooks.corp.com/*' as unknown as string[],
+      },
+    };
+    const argv = await parseArguments();
+    const config = await loadCliConfig(settings, argv, undefined, []);
+    expect(config.getAllowedHttpHookUrls()).toEqual([]);
+    expect(
+      config.getWarnings().some((w) => w.includes('allowedHttpHookUrls')),
+    ).toBe(true);
+  });
+
+  it('should not warn about allowedHttpHookUrls when the value is a valid list', async () => {
+    process.argv = ['node', 'script.js'];
+    const settings: Settings = {
+      security: {
+        allowedHttpHookUrls: ['https://hooks.corp.com/*'],
+      },
+    };
+    const argv = await parseArguments();
+    const config = await loadCliConfig(settings, argv, undefined, []);
+    expect(
+      config.getWarnings().some((w) => w.includes('allowedHttpHookUrls')),
+    ).toBe(false);
+  });
+
+  it('should drop non-string allowedHttpHookUrls entries', async () => {
+    process.argv = ['node', 'script.js'];
+    const settings: Settings = {
+      security: {
+        allowedHttpHookUrls: [
+          42,
+          'https://hooks.corp.com/ci/*',
+          null,
+        ] as unknown as string[],
+      },
+    };
+    const argv = await parseArguments();
+    const config = await loadCliConfig(settings, argv, undefined, []);
+    expect(config.getAllowedHttpHookUrls()).toEqual([
+      'https://hooks.corp.com/ci/*',
+    ]);
+  });
+});
+
 describe('loadCliConfig with includeDirectories', () => {
   const originalArgv = process.argv;
 

packages/cli/src/config/config.ts

@@ -2328,10 +2328,27 @@ export async function loadCliConfig(
     warnings: resolvedCliConfig.warnings,
     bareMode,
     safeMode,
-    allowedHttpHookUrls:
-      bareMode || safeMode
-        ? []
-        : (settings.security?.allowedHttpHookUrls ?? []),
+    allowedHttpHookUrls: (() => {
+      if (bareMode || safeMode) {
+        return [];
+      }
+      // Settings files are validated only as top-level JSON objects, so a
+      // hand-edited file can put a bare string or non-string entries here;
+      // reduce it to a valid list before it reaches the UrlValidator, which
+      // maps over it and would abort startup on a non-array.
+      const hookUrls = settings.security?.allowedHttpHookUrls;
+      if (hookUrls !== undefined && !Array.isArray(hookUrls)) {
+        // The coercion to [] reads as "allow all" in the UrlValidator, so
+        // surface the lost restriction instead of silently starting
+        // unrestricted.
+        resolvedCliConfig.warnings.push(
+          'Warning: security.allowedHttpHookUrls is not a list and was ignored; HTTP hooks are unrestricted apart from SSRF protection.',
+        );
+      }
+      return Array.isArray(hookUrls)
+        ? hookUrls.filter((entry): entry is string => typeof entry === 'string')
+        : [];
+    })(),
     allowPrivateNetworkHooks:
       bareMode || safeMode
         ? false