fix(telemetry): defensive allowlist normalization + positive proxy test

Three issues found by wenshao reviewing the R3 host-allowlist scoping.

1. **[Critical] `broadcastAll` outside safety try/catch**
   (llm-correlation-fetch.ts wrapFetchWithCorrelation)
   The try/catch only fires when `getTelemetrySessionIdHeaderHosts()`
   throws. If it returns a malformed value — a bare string (settings.json
   typo `"sessionIdHeaderHosts": "host"` instead of `["host"]`), an array
   containing `null`/`undefined`/number entries, or whitespace-padded
   entries — `.some((p) => p.trim() === '*')` throws TypeError at
   buildClient time, bricking the LLM session before the first prompt.
   `staticCorrelationHeaders` already handled this via its end-to-end
   try/catch but the sister helper diverged. Settings loader does no
   runtime schema validation so this is reachable via a single typo.

   Fix: normalize the allowlist at wrap time:
     1. catch a throwing getter (existing)
     2. reject non-array → default allowlist (NEW — bare string typo)
     3. filter out non-string elements (NEW — [null, ...] typo)
     4. trim every surviving entry uniformly (NEW — see #2 below)
   Then `trustedHosts.includes('*')` instead of `.some((p) => p.trim() === '*')`,
   since patterns are already pre-trimmed.

2. **Trim asymmetry between `*` detection and host-pattern match**
   (llm-correlation-fetch.ts)
   `[" * "]` was tolerated (trimmed before `===` compare) but
   `[" dashscope.aliyuncs.com "]` silently never matched. The
   normalization above fixes this by trimming uniformly upstream.

3. **Proxy fetch test: only negative assertions**
   (openaiContentGenerator/provider/default.test.ts)
   The test asserted `callArg.fetch !== proxyFetch` and `!== globalThis.fetch`
   but both passed for ANY wrapper, including a buggy one that
   accidentally wraps globalThis.fetch instead of proxyFetch. Added a
   positive assertion: call the wrapped fetch and verify proxyFetch was
   the delegation target.

Tests: 4 new cases — whitespace-padded host pattern, bare-string
malformed config (both wrapper and static), null/number-containing
array malformed config (both wrapper and static), positive proxy fetch
delegation. All pass; pre-existing Anthropic User-Agent failure
unrelated.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

packages/core/src/core/openaiContentGenerator/provider/default.test.ts

@@ -181,12 +181,14 @@ describe('DefaultOpenAICompatibleProvider', () => {
       expect(callArg.fetch).not.toBe(globalThis.fetch);
     });
 
-    it('wraps the proxy fetch (not globalThis.fetch) when runtimeOptions provides one', () => {
+    it('wraps the proxy fetch (not globalThis.fetch) when runtimeOptions provides one', async () => {
       // Regression guard for design §4.3: when proxy is configured,
       // buildRuntimeFetchOptions returns { fetch: <bundled undici fetch> }
       // so the proxy dispatcher and fetch share a single undici version.
       // The correlation wrapper must wrap THAT fetch, not globalThis.fetch.
-      const proxyFetch = vi.fn() as unknown as typeof fetch;
+      const proxyFetch = vi.fn(
+        async () => new Response(),
+      ) as unknown as typeof fetch;
       const mockedBuildRuntimeFetchOptions =
         buildRuntimeFetchOptions as unknown as MockedFunction<
           (sdkType: 'openai', proxyUrl?: string) => OpenAIRuntimeFetchOptions
@@ -202,6 +204,13 @@ describe('DefaultOpenAICompatibleProvider', () => {
       // Wrapped, not raw — the wrapper is a different function reference.
       expect(callArg.fetch).not.toBe(proxyFetch);
       expect(callArg.fetch).not.toBe(globalThis.fetch);
+      // Positive assertion (PR #4390 review): the negatives above pass
+      // for ANY wrapper, including a buggy one that wraps globalThis.fetch
+      // instead of proxyFetch. Call the wrapped fetch and verify the
+      // delegation target is actually proxyFetch (telemetry off in
+      // mockCliConfig, so wrapper short-circuits straight to baseFetch).
+      await callArg.fetch!('https://api.example.com/v1');
+      expect(proxyFetch).toHaveBeenCalledTimes(1);
     });
   });
 

packages/core/src/telemetry/llm-correlation-fetch.test.ts

@@ -394,6 +394,35 @@ describe('staticCorrelationHeaders', () => {
     expect(snapshot[SESSION_ID_HEADER]).toBe('sess-A');
   });
 
+  it('returns {} (does not throw) when allowlist is a bare string instead of array', () => {
+    // Same defensive normalization as wrapFetchWithCorrelation. Bare
+    // string from a malformed settings.json must not crash construction.
+    const malformedConfig = {
+      getTelemetryEnabled: () => true,
+      getSessionId: () => 'sess-A',
+      getTelemetrySessionIdHeaderHosts: () =>
+        'dashscope.aliyuncs.com' as unknown as readonly string[],
+    } as unknown as Config;
+    // Falls back to DEFAULT — DashScope destination matches.
+    expect(staticCorrelationHeaders(malformedConfig, TRUSTED)).toEqual({
+      [SESSION_ID_HEADER]: 'sess-A',
+    });
+  });
+
+  it('returns {} (does not throw) when allowlist array contains non-string entries', () => {
+    const malformedConfig = {
+      getTelemetryEnabled: () => true,
+      getSessionId: () => 'sess-A',
+      getTelemetrySessionIdHeaderHosts: () =>
+        [null, 'dashscope.aliyuncs.com', 42] as unknown as
+          | readonly string[]
+          | undefined,
+    } as unknown as Config;
+    expect(staticCorrelationHeaders(malformedConfig, TRUSTED)).toEqual({
+      [SESSION_ID_HEADER]: 'sess-A',
+    });
+  });
+
   it('falls through to {} when config getters throw (telemetry must never break LLM path)', () => {
     // Mirror the safety contract of `wrapFetchWithCorrelation`. This helper
     // is called from the Gemini factory at construction time, so a throw
@@ -517,6 +546,72 @@ describe('wrapFetchWithCorrelation — host allowlist gating', () => {
     );
   });
 
+  it('tolerates whitespace around host patterns (parity with "*" trim)', async () => {
+    // Same defensive normalization applies to host patterns, not just the
+    // broadcast wildcard. Without it, `" dashscope.aliyuncs.com"` would
+    // silently never match — inconsistent with the `[" * "]` tolerance.
+    const m = makeFetchMock();
+    const wrapped = wrapFetchWithCorrelation(
+      m.fetch,
+      mockConfig({
+        enabled: true,
+        sessionId: 'sess-A',
+        hosts: [' dashscope.aliyuncs.com '],
+      }),
+    );
+    await wrapped('https://dashscope.aliyuncs.com/compatible-mode/v1');
+    expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe(
+      'sess-A',
+    );
+  });
+
+  it('does not throw at buildClient when settings returns a bare string instead of array (malformed JSON)', async () => {
+    // Regression: `broadcastAll` computation was outside the safety
+    // try/catch. A typo like `"sessionIdHeaderHosts": "dashscope..."`
+    // (forgot brackets) would let `.some(...)` throw TypeError at
+    // wrap time — bricking buildClient and the LLM session.
+    const m = makeFetchMock();
+    const malformedConfig = {
+      getTelemetryEnabled: () => true,
+      getSessionId: () => 'sess-A',
+      getTelemetrySessionIdHeaderHosts: () =>
+        'dashscope.aliyuncs.com' as unknown as readonly string[],
+    } as unknown as Config;
+    // The wrap itself must not throw — fall back to default allowlist.
+    expect(() =>
+      wrapFetchWithCorrelation(m.fetch, malformedConfig),
+    ).not.toThrow();
+    const wrapped = wrapFetchWithCorrelation(m.fetch, malformedConfig);
+    // Default allowlist applies → header IS attached for DashScope.
+    await wrapped('https://dashscope.aliyuncs.com/v1');
+    expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe(
+      'sess-A',
+    );
+  });
+
+  it('does not throw when settings array contains non-string entries (malformed JSON)', async () => {
+    // Regression: `[null, "*.dashscope.aliyuncs.com"]` typo placeholder
+    // would let `null.trim()` throw in the broadcastAll predicate.
+    const m = makeFetchMock();
+    const malformedConfig = {
+      getTelemetryEnabled: () => true,
+      getSessionId: () => 'sess-A',
+      getTelemetrySessionIdHeaderHosts: () =>
+        [null, 'dashscope.aliyuncs.com', undefined, 42] as unknown as
+          | readonly string[]
+          | undefined,
+    } as unknown as Config;
+    expect(() =>
+      wrapFetchWithCorrelation(m.fetch, malformedConfig),
+    ).not.toThrow();
+    const wrapped = wrapFetchWithCorrelation(m.fetch, malformedConfig);
+    // Non-string entries filtered out, surviving string matched.
+    await wrapped('https://dashscope.aliyuncs.com/v1');
+    expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe(
+      'sess-A',
+    );
+  });
+
   it('respects [] override to fully disable injection', async () => {
     const m = makeFetchMock();
     const wrapped = wrapFetchWithCorrelation(

packages/core/src/telemetry/llm-correlation-fetch.ts

@@ -96,24 +96,43 @@ export function wrapFetchWithCorrelation<TFetch extends FetchLikeLoose>(
   // Operators override via `telemetry.sessionIdHeaderHosts` in
   // settings.json (e.g. `["*.api.openai.com"]` for a specific OpenAI-
   // compatible proxy, or `["*"]` to restore the broadcast behavior the
-  // initial design proposed). Defensive try/catch + optional-chaining
+  // initial design proposed).
-  // so a Config implementation that pre-dates this getter (or a partial
+  //
-  // test mock without the stub) falls back to the default allowlist
+  // Defensive normalization (PR #4390 review feedback): qwen-code's
-  // rather than crashing at buildClient time.
+  // settings loader does not enforce JSON schema at runtime, so the
+  // getter can legitimately return a malformed value — a bare string
+  // ("dashscope.aliyuncs.com" instead of ["dashscope.aliyuncs.com"]),
+  // an array containing non-strings, or whitespace-padded entries
+  // (" * " or " dashscope.aliyuncs.com"). The chain below:
+  //   1. catches a throwing getter (mock without stub, pre-getter Config)
+  //   2. rejects a non-array value (bare string typo) → default allowlist
+  //   3. filters out non-string elements ([null, "..."] typo placeholder)
+  //   4. trims every surviving entry uniformly, so the `*` broadcast
+  //      escape hatch and the host-pattern match path have parity
+  // Violating any of these would let `.includes / .some / matchesTrustedHost`
+  // throw at buildClient time — bricking the LLM session before the first
+  // prompt and violating the "telemetry must never break the LLM request
+  // path" contract that `staticCorrelationHeaders` already honors via its
+  // end-to-end try/catch.
   let trustedHosts: readonly string[];
   try {
-    trustedHosts =
+    const raw =
       config.getTelemetrySessionIdHeaderHosts?.() ??
       DEFAULT_SESSION_ID_HEADER_HOSTS;
+    trustedHosts = Array.isArray(raw)
+      ? raw
+          .filter((p): p is string => typeof p === 'string')
+          .map((p) => p.trim())
+      : DEFAULT_SESSION_ID_HEADER_HOSTS;
   } catch {
     trustedHosts = DEFAULT_SESSION_ID_HEADER_HOSTS;
   }
   // Wildcard escape hatch so operators who want the old broadcast
   // behavior can opt in via `["*"]` without us extending the pattern
   // grammar in `matchesTrustedHost` (which would tempt other globbing).
-  // `.trim()` so `[" * "]` (whitespace from settings.json hand-edit) still
+  // Pre-trimmed above, so `.includes('*')` covers `["*"]` / `[" * "]` /
-  // triggers broadcast rather than silently degrading to "no host matches".
+  // `["\t*\n"]` uniformly.
-  const broadcastAll = trustedHosts.some((p) => p.trim() === '*');
+  const broadcastAll = trustedHosts.includes('*');
 
   const wrapped: FetchLikeLoose = async function correlationFetch(
     input: string | URL | Request,
@@ -193,10 +212,19 @@ export function staticCorrelationHeaders(
   try {
     if (!config.getTelemetryEnabled()) return {};
     if (!destinationUrl) return {};
-    const trustedHosts =
+    // Same defensive normalization as `wrapFetchWithCorrelation`. Bare
+    // string / array-with-nulls / whitespace-padded entries from a
+    // hand-edited settings.json would otherwise crash `.includes` /
+    // `matchesTrustedHost`. See sibling helper for full rationale.
+    const raw =
       config.getTelemetrySessionIdHeaderHosts?.() ??
       DEFAULT_SESSION_ID_HEADER_HOSTS;
-    const broadcastAll = trustedHosts.some((p) => p.trim() === '*');
+    const trustedHosts: readonly string[] = Array.isArray(raw)
+      ? raw
+          .filter((p): p is string => typeof p === 'string')
+          .map((p) => p.trim())
+      : DEFAULT_SESSION_ID_HEADER_HOSTS;
+    const broadcastAll = trustedHosts.includes('*');
     if (!broadcastAll) {
       let host: string;
       try {