From 1c8528a56bacd1e07911566cb7103eed198e8f4e Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 22 May 2026 23:09:05 +0800 Subject: [PATCH] feat(telemetry): scope X-Qwen-Code-Session-Id to first-party hosts by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address LaZzyMan's REQUEST_CHANGES review of PR #4390. The original design injected `X-Qwen-Code-Session-Id` on every outbound LLM request gated only by `telemetry.enabled`. Review caught that this broadcasts a stable cross-request client identifier to every configured third-party provider (OpenAI, Anthropic, OpenRouter, MiniMax, ModelScope, Mistral, vanilla Gemini, ...), which the claude-code precedent does NOT justify — claude-code is a first-party Anthropic→Anthropic flow; qwen-code is an open-source CLI connecting to many providers. Fix: add a host allowlist with a deliberately narrow default. The header is now only attached to destinations whose hostname matches: dashscope.aliyuncs.com dashscope-intl.aliyuncs.com *.dashscope.aliyuncs.com *.dashscope-intl.aliyuncs.com *.alibaba-inc.com *.aliyun-inc.com This is exactly the set where the LLM provider, the upstream telemetry backend (ARMS Tracing), and qwen-code itself are the same legal entity — mirroring the first-party claude-code pattern and preserving the real product value (server-side trace stitching against DashScope) without exposing the session id to third parties. Operators with broader correlation requirements override via: "telemetry": { "sessionIdHeaderHosts": ["*"] // restore broadcast "sessionIdHeaderHosts": [] // fully disable "sessionIdHeaderHosts": ["api.example.com", "*.foo"] // custom allowlist } Implementation: - NEW `telemetry/trusted-llm-hosts.ts`: `DEFAULT_SESSION_ID_HEADER_HOSTS` + `matchesTrustedHost(hostname, patterns)` + `extractRequestHost(input)`. Pattern syntax is intentionally tiny (bare hostname OR `*.suffix`, dot-anchored to reject `evil-alibaba-inc.com` style attacks). Unit-tested in dedicated test file including TLD/sub-domain attack vectors. - `wrapFetchWithCorrelation` (openai + anthropic providers): resolves the allowlist at wrap time (Config snapshot), inspects each request's destination URL inside `correlationFetch`, falls through to baseFetch for non-trusted destinations. Wildcard escape hatch via `["*"]`. - `staticCorrelationHeaders` (Gemini factory): now takes an optional `destinationUrl` and applies the same host gate. The Gemini SDK default endpoint `generativelanguage.googleapis.com` is NOT on the default allowlist, so vanilla Gemini calls receive no header — matching the "first-party only" scope. Operators who put the Gemini SDK on a DashScope-compatible endpoint via `baseUrl` get the header naturally. - `Config.getTelemetrySessionIdHeaderHosts()` getter + `TelemetrySettings.sessionIdHeaderHosts` interface field + JSON schema entry in `settingsSchema.ts`. Wired through `resolveTelemetrySettings`. - Defensive optional-chaining + try/catch on the Config getter call at wrap time so partial test mocks (or pre-getter Config implementations) fall back to the default allowlist rather than crashing buildClient. Tests: 12 new cases covering host match/skip on default allowlist, sub-domain handling, TLD-suffix attack rejection, `["*"]` broadcast override, `[]` full-disable, custom operator allowlist, unparseable destination (fail closed), and the three Gemini factory paths (googleapis.com default → omit; DashScope `baseUrl` → inject; custom allowlist → inject). Docs updated in `docs/developers/development/telemetry.md` Session correlation header section, including override examples and the new Gemini host-gate semantics. Closes the LaZzyMan REQUEST_CHANGES blocker. The cross-vendor fingerprint-broadcast failure mode is now opt-in rather than default, restoring the first-party-only semantics that make the claude-code precedent applicable. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- docs/developers/development/telemetry.md | 79 ++++-- packages/cli/src/config/settingsSchema.ts | 6 + packages/core/src/config/config.ts | 29 ++ .../core/src/core/contentGenerator.test.ts | 2 + .../core/geminiContentGenerator/index.test.ts | 59 +++- .../src/core/geminiContentGenerator/index.ts | 15 +- packages/core/src/telemetry/config.ts | 1 + .../telemetry/llm-correlation-fetch.test.ts | 259 +++++++++++++++++- .../src/telemetry/llm-correlation-fetch.ts | 92 ++++++- .../src/telemetry/trusted-llm-hosts.test.ts | 152 ++++++++++ .../core/src/telemetry/trusted-llm-hosts.ts | 89 ++++++ 11 files changed, 745 insertions(+), 38 deletions(-) create mode 100644 packages/core/src/telemetry/trusted-llm-hosts.test.ts create mode 100644 packages/core/src/telemetry/trusted-llm-hosts.ts diff --git a/docs/developers/development/telemetry.md b/docs/developers/development/telemetry.md index 86d7871a5a..2cf62c9827 100644 --- a/docs/developers/development/telemetry.md +++ b/docs/developers/development/telemetry.md @@ -301,19 +301,20 @@ knowing: - **Trace ID leakage.** The W3C `traceparent` header carries the trace ID to every destination, including third-party URLs supplied at runtime (e.g. a `WebFetch` to an arbitrary domain). The trace ID is not a - secret per the W3C spec, but operators with stricter requirements - should be aware that the qwen-code trace ID becomes observable to any - endpoint the user instructs the agent to call. If this is unacceptable - for a deployment, disable telemetry entirely (`telemetry.enabled: false`) - until a per-destination scoping toggle ships (tracked as a follow-up). + secret per the W3C spec — the value is `sha256(sessionId).slice(0, 32)` + (a one-way hash, not the session id itself). Operators with stricter + requirements who don't want even the hashed trace ID reaching + third-party endpoints can disable telemetry entirely + (`telemetry.enabled: false`); a per-destination scoping toggle for + trace propagation specifically is tracked as a follow-up. - **Span volume.** Non-LLM `fetch` calls show up as client HTTP spans in your OTLP backend. Filter on `http.url` or `peer.service` if you want to isolate the LLM-only subset. ### Session correlation header -Alongside `traceparent`, Qwen Code injects a custom HTTP header on every -outbound LLM request when telemetry is enabled: +Alongside `traceparent`, Qwen Code can inject a custom HTTP header on +outbound LLM requests when telemetry is enabled: ``` X-Qwen-Code-Session-Id: @@ -322,30 +323,66 @@ X-Qwen-Code-Session-Id: Pattern matched from Claude Code's `X-Claude-Code-Session-Id` (see `src/services/api/client.ts:108` in the claude-code repo). The header is product-namespaced to avoid collision with generic `X-Session-Id` headers -other tools may inject. Server-side ingestion (e.g. a custom DashScope -proxy or an OTLP-aware API gateway) can use this header to stitch its +other tools may inject. Server-side ingestion (e.g. a DashScope +gateway or an OTLP-aware API gateway) can use this header to stitch its observation of an LLM request back to the originating Qwen Code session without having to parse trace context. +**Default scope: first-party Alibaba/DashScope endpoints only.** The +header is sent only to destinations whose hostname matches the +`telemetry.sessionIdHeaderHosts` allowlist. The default allowlist is: + +``` +dashscope.aliyuncs.com +dashscope-intl.aliyuncs.com +*.dashscope.aliyuncs.com +*.dashscope-intl.aliyuncs.com +*.alibaba-inc.com +*.aliyun-inc.com +``` + +Requests to third-party LLM providers (OpenAI, Anthropic, OpenRouter, +MiniMax, ModelScope, Mistral, DeepSeek, vanilla Gemini, ...) **do not +receive this header by default** — avoids broadcasting a stable +client-side session identifier to providers who don't need it for the +API call itself. See [PR #4390 review discussion](https://github.com/QwenLM/qwen-code/pull/4390) +for the full rationale. + +Operators with broader correlation requirements can override the scope +in `settings.json`: + +```jsonc +"telemetry": { + "sessionIdHeaderHosts": ["*"] // restore broadcast + "sessionIdHeaderHosts": [] // fully disable header + "sessionIdHeaderHosts": ["api.mycompany.com", + "*.gateway.mycompany.internal"] // custom allowlist +} +``` + The header value comes from `Config.getSessionId()`, read fresh on every -outbound request (not captured at SDK construction time). After a session -reset triggered by `/clear`, subsequent LLM requests carry the new session -id automatically — see the "Known limitation" note below for the one -exception. +outbound request (not captured at SDK construction time) on the OpenAI/ +Anthropic providers. After a session reset triggered by `/clear`, +subsequent requests carry the new session id automatically — see the +"Known limitation" note below for the one exception. Empty session ids are not emitted (some HTTP middleware rejects empty -header values). The header is omitted entirely when telemetry is disabled. +header values). The header is omitted entirely when telemetry is disabled +or the destination host is outside the allowlist. **Known limitation: Gemini provider.** `@google/genai`'s `HttpOptions` interface does not expose a `fetch` hook (only static `headers`), so the Gemini provider can only inject the session-id header at SDK construction -time. After a `/clear`-triggered session reset, outbound Gemini requests -carry the OLD session id in `X-Qwen-Code-Session-Id` until the underlying -content generator is recreated (the current code does not recreate it on -reset). All other providers (`openai`-family, `anthropic`) use a fetch -wrapper and are immune to this. Tracked as a follow-up; in the meantime, -spans and logs still carry the live session id, so trace/log backends can -correctly attribute requests to the new session. +time. The host-allowlist check happens once against the SDK's +`baseUrl` (defaults to `generativelanguage.googleapis.com` — not on the +default allowlist, so no header by default). When operators do put the +Gemini SDK on a first-party endpoint via `baseUrl` and the allowlist +matches, the header IS attached but goes stale on `/clear` until the +content generator is recreated. All other providers (`openai`-family, +`anthropic`) use a fetch wrapper and are immune to this. Tracked as a +follow-up; in the meantime, spans and logs still carry the live session +id, so trace/log backends can correctly attribute requests to the new +session. ## Aliyun Telemetry diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 46726450ae..5ece7c6eec 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1032,6 +1032,12 @@ const SETTINGS_SCHEMA = { }, }, }, + sessionIdHeaderHosts: { + description: + 'Destination hostnames (or "*.suffix" patterns) that receive the X-Qwen-Code-Session-Id outbound correlation header. Defaults to Alibaba/DashScope first-party endpoints (dashscope.aliyuncs.com, dashscope-intl.aliyuncs.com, *.dashscope.aliyuncs.com, *.dashscope-intl.aliyuncs.com, *.alibaba-inc.com, *.aliyun-inc.com) so the stable session identifier is not broadcast to third-party LLM providers like OpenAI or Anthropic. Set to ["*"] to restore the broadcast-to-everywhere behavior, or [] to fully disable the header.', + type: 'array', + items: { type: 'string' }, + }, }, additionalProperties: true, }, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 733f8bab3e..6044c58f21 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -314,6 +314,23 @@ export interface TelemetrySettings { resourceAttributes?: Record; /** Per-signal cardinality controls. */ metrics?: TelemetryMetricsSettings; + /** + * Hostnames (or `*.suffix` patterns) that receive the + * `X-Qwen-Code-Session-Id` outbound correlation header. + * + * Default (when unset): Alibaba/DashScope first-party endpoints only — + * see `DEFAULT_SESSION_ID_HEADER_HOSTS` in `telemetry/trusted-llm-hosts.ts`. + * The default is deliberately narrow so the stable session identifier + * does not get broadcast to arbitrary third-party LLM providers + * (OpenAI, Anthropic, OpenRouter, ...) configured under the + * OpenAI-compatible provider. PR #4390 review (LaZzyMan) for rationale. + * + * Overrides: + * - `[]` fully disables the header (no destinations) + * - `["*"]` restores the broadcast behavior across all destinations + * - `["api.mycompany.com", "*.internal.mycompany.com"]` custom allowlist + */ + sessionIdHeaderHosts?: string[]; /** * Human-readable diagnostics produced while resolving * `resourceAttributes` (drops, coercions, reserved-key strips). @@ -1020,6 +1037,7 @@ export class Config { outfile: params.telemetry?.outfile, resourceAttributes: params.telemetry?.resourceAttributes, metrics: params.telemetry?.metrics, + sessionIdHeaderHosts: params.telemetry?.sessionIdHeaderHosts, resourceAttributeWarnings: params.telemetry?.resourceAttributeWarnings, }; this.gitCoAuthor = { @@ -2849,6 +2867,17 @@ export class Config { return this.telemetrySettings.metrics?.includeSessionId ?? false; } + /** + * Returns the configured host allowlist for the + * `X-Qwen-Code-Session-Id` outbound header, or `undefined` to indicate + * "use `DEFAULT_SESSION_ID_HEADER_HOSTS`". The caller (`llm-correlation-fetch`) + * resolves the default — keeping it out of Config avoids importing the + * telemetry helper from here. + */ + getTelemetrySessionIdHeaderHosts(): readonly string[] | undefined { + return this.telemetrySettings.sessionIdHeaderHosts; + } + getTelemetryResourceAttributeWarnings(): readonly string[] { return this.telemetrySettings.resourceAttributeWarnings ?? []; } diff --git a/packages/core/src/core/contentGenerator.test.ts b/packages/core/src/core/contentGenerator.test.ts index 5f9199eb19..2816b145f9 100644 --- a/packages/core/src/core/contentGenerator.test.ts +++ b/packages/core/src/core/contentGenerator.test.ts @@ -24,6 +24,7 @@ describe('createContentGenerator', () => { getCliVersion: () => '1.0.0', getTelemetryEnabled: () => false, getSessionId: () => 'test-session', + getTelemetrySessionIdHeaderHosts: () => undefined, } as unknown as Config; const mockGenerator = { @@ -61,6 +62,7 @@ describe('createContentGenerator', () => { getCliVersion: () => '1.0.0', getTelemetryEnabled: () => false, getSessionId: () => 'test-session', + getTelemetrySessionIdHeaderHosts: () => undefined, } as unknown as Config; const mockGenerator = { models: {}, diff --git a/packages/core/src/core/geminiContentGenerator/index.test.ts b/packages/core/src/core/geminiContentGenerator/index.test.ts index 56a31aa269..c343de686f 100644 --- a/packages/core/src/core/geminiContentGenerator/index.test.ts +++ b/packages/core/src/core/geminiContentGenerator/index.test.ts @@ -108,13 +108,70 @@ describe('createGeminiContentGenerator', () => { ); }); - it('includes X-Qwen-Code-Session-Id in httpOptions.headers when telemetry is enabled', () => { + it('omits X-Qwen-Code-Session-Id for vanilla Gemini endpoint even when telemetry is enabled (third-party scope)', () => { + // PR #4390 review (LaZzyMan): the session id header is scoped to + // first-party (Alibaba/DashScope) destinations by default. A vanilla + // Gemini API call resolves to `generativelanguage.googleapis.com`, + // which is NOT on the default allowlist, so no header. mockConfig = { getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), getContentGeneratorConfig: vi.fn().mockReturnValue({}), getCliVersion: vi.fn().mockReturnValue('1.0.0'), getTelemetryEnabled: vi.fn().mockReturnValue(true), getSessionId: vi.fn().mockReturnValue('sess-gemini'), + getTelemetrySessionIdHeaderHosts: vi.fn().mockReturnValue(undefined), + } as unknown as Config; + const config = { + model: 'gemini-1.5-flash', + apiKey: 'k', + authType: AuthType.USE_GEMINI, + }; + createGeminiContentGenerator(config, mockConfig); + const callArgs = vi.mocked(GeminiContentGenerator).mock.calls[0]?.[0] as { + httpOptions: { headers: Record }; + }; + expect(callArgs.httpOptions.headers).not.toHaveProperty( + 'X-Qwen-Code-Session-Id', + ); + }); + + it('includes X-Qwen-Code-Session-Id when baseUrl points at a trusted DashScope endpoint', () => { + mockConfig = { + getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), + getContentGeneratorConfig: vi.fn().mockReturnValue({}), + getCliVersion: vi.fn().mockReturnValue('1.0.0'), + getTelemetryEnabled: vi.fn().mockReturnValue(true), + getSessionId: vi.fn().mockReturnValue('sess-gemini'), + getTelemetrySessionIdHeaderHosts: vi.fn().mockReturnValue(undefined), + } as unknown as Config; + const config = { + model: 'qwen-vl-plus', + apiKey: 'k', + authType: AuthType.USE_GEMINI, + // Operator has pointed the Gemini SDK at a DashScope-compatible + // endpoint via baseUrl override. This IS on the default allowlist. + baseUrl: 'https://dashscope.aliyuncs.com/api/v1', + }; + createGeminiContentGenerator(config, mockConfig); + const callArgs = vi.mocked(GeminiContentGenerator).mock.calls[0]?.[0] as { + httpOptions: { headers: Record }; + }; + expect(callArgs.httpOptions.headers['X-Qwen-Code-Session-Id']).toBe( + 'sess-gemini', + ); + }); + + it('includes X-Qwen-Code-Session-Id when allowlist override covers googleapis.com', () => { + // Operator opts back in for Google's endpoint via settings override. + mockConfig = { + getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), + getContentGeneratorConfig: vi.fn().mockReturnValue({}), + getCliVersion: vi.fn().mockReturnValue('1.0.0'), + getTelemetryEnabled: vi.fn().mockReturnValue(true), + getSessionId: vi.fn().mockReturnValue('sess-gemini'), + getTelemetrySessionIdHeaderHosts: vi + .fn() + .mockReturnValue(['*.googleapis.com']), } as unknown as Config; const config = { model: 'gemini-1.5-flash', diff --git a/packages/core/src/core/geminiContentGenerator/index.ts b/packages/core/src/core/geminiContentGenerator/index.ts index 88ec1b370d..41896a3cd8 100644 --- a/packages/core/src/core/geminiContentGenerator/index.ts +++ b/packages/core/src/core/geminiContentGenerator/index.ts @@ -45,7 +45,20 @@ export function createGeminiContentGenerator( // Known limitation: after a `/clear`-triggered session reset, the Gemini // SDK's cached headers retain the OLD session id until the contentGenerator // is recreated. See design doc §8.6 + #4384 follow-up sub-issue tracking. - headers = { ...headers, ...staticCorrelationHeaders(gcConfig) }; + // + // Destination passed in is what the host-allowlist check uses. With the + // default `DEFAULT_SESSION_ID_HEADER_HOSTS` (Alibaba/DashScope-only), + // Google's default endpoint `generativelanguage.googleapis.com` is NOT + // on the list, so the header is naturally omitted for vanilla Gemini API + // calls — matching the "first-party only" scope. Operators who deliberately + // want correlation against a Google endpoint can add it via + // `telemetry.sessionIdHeaderHosts` in settings. + const destinationUrl = + config.baseUrl ?? 'https://generativelanguage.googleapis.com'; + headers = { + ...headers, + ...staticCorrelationHeaders(gcConfig, destinationUrl), + }; const httpOptions = config.baseUrl ? { headers, diff --git a/packages/core/src/telemetry/config.ts b/packages/core/src/telemetry/config.ts index 784bf124fb..5b99178b92 100644 --- a/packages/core/src/telemetry/config.ts +++ b/packages/core/src/telemetry/config.ts @@ -188,6 +188,7 @@ export async function resolveTelemetrySettings(options: { outfile, resourceAttributes, metrics: { includeSessionId: metricsIncludeSessionId }, + sessionIdHeaderHosts: settings.sessionIdHeaderHosts, resourceAttributeWarnings: resourceAttributeWarnings.length ? resourceAttributeWarnings : undefined, diff --git a/packages/core/src/telemetry/llm-correlation-fetch.test.ts b/packages/core/src/telemetry/llm-correlation-fetch.test.ts index 4fa6470e1e..aaca69751b 100644 --- a/packages/core/src/telemetry/llm-correlation-fetch.test.ts +++ b/packages/core/src/telemetry/llm-correlation-fetch.test.ts @@ -22,13 +22,28 @@ type FetchLike = ( function mockConfig(opts: { enabled?: boolean; sessionId?: string | (() => string); + /** + * Host allowlist returned by `getTelemetrySessionIdHeaderHosts`. + * + * - Key OMITTED: returns `['*']` (broadcast) so the bulk of tests below + * — which exercise header-injection mechanics, not host-gating — + * keep operating against the `api.example.com` test URLs. + * - Key PRESENT and `undefined`: returns `undefined`, letting the + * wrapper fall back to `DEFAULT_SESSION_ID_HEADER_HOSTS` (the real + * default allowlist). Used by host-gate tests. + * - Key PRESENT and an array: returns that array verbatim. + */ + hosts?: readonly string[]; }): Config { + const hostsKeyPresent = 'hosts' in opts; return { getTelemetryEnabled: () => opts.enabled ?? true, getSessionId: () => typeof opts.sessionId === 'function' ? opts.sessionId() : (opts.sessionId ?? ''), + getTelemetrySessionIdHeaderHosts: () => + hostsKeyPresent ? opts.hosts : ['*'], } as unknown as Config; } @@ -218,12 +233,16 @@ describe('wrapFetchWithCorrelation', () => { const warnSpy = vi.spyOn(diag, 'warn').mockImplementation(() => {}); const m = makeFetchMock(); // Config getter that throws — simulates a runtime bug that must not - // propagate and break the LLM request. + // propagate and break the LLM request. Use a TRUSTED destination + // (broadcast allowlist) so we reach the throwing getSessionId() — a + // third-party destination would short-circuit at the host gate before + // ever calling getSessionId. const config = { getTelemetryEnabled: () => true, getSessionId: () => { throw new Error('config bug'); }, + getTelemetrySessionIdHeaderHosts: () => ['*'], } as unknown as Config; const wrapped = wrapFetchWithCorrelation(m.fetch, config); const userInit = { method: 'POST' }; @@ -242,10 +261,16 @@ describe('wrapFetchWithCorrelation', () => { }); describe('staticCorrelationHeaders', () => { + // Tests pass `hosts: ['*']` (broadcast) unless they're specifically + // exercising the host gate, since broadcast was the original behavior + // before the LaZzyMan-review-driven scope narrowing. + const TRUSTED = 'https://dashscope.aliyuncs.com/compatible-mode/v1'; + it('returns header when telemetry enabled and sessionId non-empty', () => { expect( staticCorrelationHeaders( mockConfig({ enabled: true, sessionId: 'sess-A' }), + TRUSTED, ), ).toEqual({ [SESSION_ID_HEADER]: 'sess-A' }); }); @@ -254,13 +279,106 @@ describe('staticCorrelationHeaders', () => { expect( staticCorrelationHeaders( mockConfig({ enabled: false, sessionId: 'sess-A' }), + TRUSTED, ), ).toEqual({}); }); it('returns {} when sessionId is empty', () => { expect( - staticCorrelationHeaders(mockConfig({ enabled: true, sessionId: '' })), + staticCorrelationHeaders( + mockConfig({ enabled: true, sessionId: '' }), + TRUSTED, + ), + ).toEqual({}); + }); + + it('returns {} when destinationUrl is undefined (fail closed)', () => { + expect( + staticCorrelationHeaders( + mockConfig({ enabled: true, sessionId: 'sess-A' }), + ), + ).toEqual({}); + }); + + it('returns {} when destinationUrl host is not on the trusted allowlist', () => { + // Default allowlist is Alibaba/DashScope only. A vanilla Gemini API call + // to googleapis.com should NOT receive the header. PR #4390 review + // (LaZzyMan): "the header should not be broadcast to every LLM provider". + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, // use real default allowlist + }), + 'https://generativelanguage.googleapis.com/v1beta', + ), + ).toEqual({}); + }); + + it('returns header when destinationUrl host matches the default allowlist (DashScope)', () => { + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, // use real default allowlist + }), + 'https://dashscope.aliyuncs.com/compatible-mode/v1', + ), + ).toEqual({ [SESSION_ID_HEADER]: 'sess-A' }); + }); + + it('returns header when destinationUrl host matches the default allowlist (internal alibaba-inc)', () => { + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + 'https://idealab.alibaba-inc.com/api/openai/v1', + ), + ).toEqual({ [SESSION_ID_HEADER]: 'sess-A' }); + }); + + it('respects ["*"] override to restore broadcast', () => { + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: ['*'], + }), + 'https://api.openai.com/v1', + ), + ).toEqual({ [SESSION_ID_HEADER]: 'sess-A' }); + }); + + it('respects [] override to fully disable', () => { + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: [], + }), + TRUSTED, + ), + ).toEqual({}); + }); + + it('returns {} when destinationUrl is unparseable', () => { + expect( + staticCorrelationHeaders( + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + 'not a url', + ), ).toEqual({}); }); @@ -271,7 +389,7 @@ describe('staticCorrelationHeaders', () => { // after construction won't update the header — known limitation. let current = 'sess-A'; const config = mockConfig({ enabled: true, sessionId: () => current }); - const snapshot = staticCorrelationHeaders(config); + const snapshot = staticCorrelationHeaders(config, TRUSTED); current = 'sess-B'; expect(snapshot[SESSION_ID_HEADER]).toBe('sess-A'); }); @@ -287,11 +405,144 @@ describe('staticCorrelationHeaders', () => { throw new Error('config exploded'); }, getSessionId: () => 'unreached', + getTelemetrySessionIdHeaderHosts: () => ['*'], } as unknown as Config; - expect(staticCorrelationHeaders(exploding)).toEqual({}); + expect(staticCorrelationHeaders(exploding, TRUSTED)).toEqual({}); expect(warnSpy).toHaveBeenCalledWith( expect.stringContaining('staticCorrelationHeaders'), ); warnSpy.mockRestore(); }); }); + +describe('wrapFetchWithCorrelation — host allowlist gating', () => { + // Dedicated block for the LaZzyMan-review-driven host gate. Uses the + // real default allowlist (no `hosts: ['*']` override) and exercises + // both the trusted-host pass and the third-party-host skip. + + it('injects header for default-allowlisted host (dashscope.aliyuncs.com)', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, // real default allowlist + }), + ); + await wrapped('https://dashscope.aliyuncs.com/compatible-mode/v1/chat'); + expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe( + 'sess-A', + ); + }); + + it('injects header for sub-domain of allowlisted suffix (*.alibaba-inc.com)', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + ); + await wrapped('https://idealab.alibaba-inc.com/api/openai/v1'); + expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe( + 'sess-A', + ); + }); + + it('skips header for third-party host (api.openai.com) under default allowlist', async () => { + // This is the core LaZzyMan-review fix: the stable session id no longer + // gets broadcast to third-party LLM providers by default. + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + ); + await wrapped('https://api.openai.com/v1/chat/completions'); + expect(m.lastInit()?.headers).toBeUndefined(); + }); + + it('skips header for third-party host (api.anthropic.com) under default allowlist', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + ); + await wrapped('https://api.anthropic.com/v1/messages'); + expect(m.lastInit()?.headers).toBeUndefined(); + }); + + it('respects ["*"] override to restore broadcast behavior', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: ['*'], + }), + ); + await wrapped('https://api.openai.com/v1/chat/completions'); + 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( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: [], + }), + ); + // Even an otherwise-trusted destination is skipped when allowlist is []. + await wrapped('https://dashscope.aliyuncs.com/compatible-mode/v1'); + expect(m.lastInit()?.headers).toBeUndefined(); + }); + + it('respects custom allowlist (operator-supplied host)', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: ['gateway.mycompany.internal'], + }), + ); + await wrapped('https://gateway.mycompany.internal/llm/v1'); + expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe( + 'sess-A', + ); + // Default allowlist hosts are NOT included when operator overrides. + await wrapped('https://dashscope.aliyuncs.com/v1'); + expect(m.lastInit()?.headers).toBeUndefined(); + }); + + it('skips header when destination URL is unparseable (fail closed)', async () => { + const m = makeFetchMock(); + const wrapped = wrapFetchWithCorrelation( + m.fetch, + mockConfig({ + enabled: true, + sessionId: 'sess-A', + hosts: undefined, + }), + ); + await wrapped('not a valid url'); + expect(m.lastInit()?.headers).toBeUndefined(); + }); +}); diff --git a/packages/core/src/telemetry/llm-correlation-fetch.ts b/packages/core/src/telemetry/llm-correlation-fetch.ts index b2c39e80d0..a858d5527c 100644 --- a/packages/core/src/telemetry/llm-correlation-fetch.ts +++ b/packages/core/src/telemetry/llm-correlation-fetch.ts @@ -6,14 +6,24 @@ import { diag } from '@opentelemetry/api'; import type { Config } from '../config/config.js'; +import { + DEFAULT_SESSION_ID_HEADER_HOSTS, + extractRequestHost, + matchesTrustedHost, +} from './trusted-llm-hosts.js'; /** - * Custom HTTP header attached to every outbound LLM service request when - * telemetry is enabled. Product-namespaced (matching claude-code's + * Custom HTTP header attached to outbound LLM service requests when + * telemetry is enabled AND the destination is on the trusted-host + * allowlist. Product-namespaced (matching claude-code's * `X-Claude-Code-Session-Id` pattern from `src/services/api/client.ts:108`) * to avoid collision with generic `X-Session-Id` headers other tools may * inject. Server-side ingestion can use this to stitch its observation of * an LLM request back to the originating qwen-code session. + * + * Scope: see `DEFAULT_SESSION_ID_HEADER_HOSTS` for the default destination + * set and PR #4390 review (LaZzyMan) for the rationale behind not + * broadcasting to every third-party LLM provider. */ export const SESSION_ID_HEADER = 'X-Qwen-Code-Session-Id'; @@ -36,9 +46,21 @@ type FetchLikeLoose = ( ) => Promise; /** - * Wrap a fetch implementation so every outbound request gets the - * `X-Qwen-Code-Session-Id` correlation header populated from the **current** - * session id, not the value captured when the SDK client was constructed. + * Wrap a fetch implementation so outbound requests to trusted LLM + * destinations get the `X-Qwen-Code-Session-Id` correlation header + * populated from the **current** session id, not the value captured + * when the SDK client was constructed. + * + * Three gates, in order: + * 1. Telemetry enabled (`config.getTelemetryEnabled()`) + * 2. Destination host is on the trusted allowlist + * (`config.getTelemetrySessionIdHeaderHosts()` ?? default in-vendor set) + * 3. Session id is non-empty + * + * Any failed gate falls through to `baseFetch(input, init)` unchanged — + * no header attached, no behavior change. This means a request to + * `api.openai.com` from a default-config install goes out exactly the + * same as it did before this PR landed. * * Why per-request and not `defaultHeaders`: SDK clients (and their static * `defaultHeaders`) are constructed once at content-generator init and are @@ -52,10 +74,6 @@ type FetchLikeLoose = ( * `runtimeOptions?.fetch ?? globalThis.fetch` so proxy-aware fetch (set up * by `buildRuntimeFetchOptions`) is preserved when ProxyAgent is in use. * - * When telemetry is disabled, returns baseFetch unchanged — no correlation - * header added. (Consistent with #4367's gating: opt-out of telemetry means - * no telemetry-related wire signal, including correlation.) - * * Safety: the wrapper catches its own exceptions and falls through to * baseFetch on any internal error. Telemetry must never break the LLM * request path — if Config getters throw, header construction fails, etc., @@ -65,6 +83,27 @@ export function wrapFetchWithCorrelation( baseFetch: TFetch, config: Config, ): TFetch { + // Resolve the host allowlist once at wrap time (Config snapshot). + // 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 + // so a Config implementation that pre-dates this getter (or a partial + // test mock without the stub) falls back to the default allowlist + // rather than crashing at buildClient time. + let trustedHosts: readonly string[]; + try { + trustedHosts = + config.getTelemetrySessionIdHeaderHosts?.() ?? + 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). + const broadcastAll = trustedHosts.some((p) => p === '*'); + const wrapped: FetchLikeLoose = async function correlationFetch( input: string | URL | Request, init?: RequestInit, @@ -74,6 +113,15 @@ export function wrapFetchWithCorrelation( if (!config.getTelemetryEnabled()) { return baseFetch(input, init); } + if (!broadcastAll) { + // Host gate: skip injection for destinations not on the allowlist. + // This is what scopes the "stable client fingerprint" exposure to + // first-party endpoints only. PR #4390 review (LaZzyMan). + const host = extractRequestHost(input); + if (!host || !matchesTrustedHost(host, trustedHosts)) { + return baseFetch(input, init); + } + } const sid = config.getSessionId(); if (!sid) { // Defensive: empty header value is rejected by some HTTP middleware. @@ -113,11 +161,18 @@ export function wrapFetchWithCorrelation( * limitation). Prefer `wrapFetchWithCorrelation` whenever the SDK exposes * a `fetch` hook. * - * When telemetry is disabled, returns `{}` so the caller can spread it - * unconditionally without changing wire behavior. + * Host scope: same trusted-host allowlist as `wrapFetchWithCorrelation`, + * but evaluated once against the `destinationUrl` known at SDK + * construction. Callers that can't determine the destination should pass + * `undefined` — the helper returns `{}` (no header, same as telemetry off). + * + * When telemetry is disabled or the destination isn't trusted, returns + * `{}` so the caller can spread it unconditionally without changing wire + * behavior. */ export function staticCorrelationHeaders( config: Config, + destinationUrl?: string, ): Record { // Mirror the safety contract of `wrapFetchWithCorrelation`: telemetry must // never break the LLM request path. This helper is called from the Gemini @@ -126,6 +181,21 @@ export function staticCorrelationHeaders( // Fall through to `{}` instead. See PR #4390 review feedback (wenshao). try { if (!config.getTelemetryEnabled()) return {}; + if (!destinationUrl) return {}; + const trustedHosts = + config.getTelemetrySessionIdHeaderHosts?.() ?? + DEFAULT_SESSION_ID_HEADER_HOSTS; + const broadcastAll = trustedHosts.some((p) => p === '*'); + if (!broadcastAll) { + let host: string; + try { + host = new URL(destinationUrl).hostname; + } catch { + // Unparseable destination → treat as not on allowlist (fail closed). + return {}; + } + if (!matchesTrustedHost(host, trustedHosts)) return {}; + } const sid = config.getSessionId(); if (!sid) return {}; return { [SESSION_ID_HEADER]: sid }; diff --git a/packages/core/src/telemetry/trusted-llm-hosts.test.ts b/packages/core/src/telemetry/trusted-llm-hosts.test.ts new file mode 100644 index 0000000000..248625c680 --- /dev/null +++ b/packages/core/src/telemetry/trusted-llm-hosts.test.ts @@ -0,0 +1,152 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + DEFAULT_SESSION_ID_HEADER_HOSTS, + extractRequestHost, + matchesTrustedHost, +} from './trusted-llm-hosts.js'; + +describe('matchesTrustedHost', () => { + it('returns false for empty hostname (defensive)', () => { + expect(matchesTrustedHost('', ['dashscope.aliyuncs.com'])).toBe(false); + }); + + it('exact match (case-insensitive)', () => { + expect( + matchesTrustedHost('dashscope.aliyuncs.com', ['dashscope.aliyuncs.com']), + ).toBe(true); + expect( + matchesTrustedHost('DashScope.Aliyuncs.COM', ['dashscope.aliyuncs.com']), + ).toBe(true); + }); + + it('exact pattern rejects sub-domain (no implicit wildcard)', () => { + expect( + matchesTrustedHost('sub.dashscope.aliyuncs.com', [ + 'dashscope.aliyuncs.com', + ]), + ).toBe(false); + }); + + it('*.suffix matches the bare suffix domain', () => { + expect(matchesTrustedHost('alibaba-inc.com', ['*.alibaba-inc.com'])).toBe( + true, + ); + }); + + it('*.suffix matches sub-domains', () => { + expect( + matchesTrustedHost('idealab.alibaba-inc.com', ['*.alibaba-inc.com']), + ).toBe(true); + expect( + matchesTrustedHost('a.b.alibaba-inc.com', ['*.alibaba-inc.com']), + ).toBe(true); + }); + + it('*.suffix rejects unrelated hosts that contain the suffix mid-name', () => { + // `evil-alibaba-inc.com` ends with `alibaba-inc.com` as a substring but + // is not a true sub-domain — the dot-anchored check should reject it. + expect( + matchesTrustedHost('evil-alibaba-inc.com', ['*.alibaba-inc.com']), + ).toBe(false); + // Trailing-suffix attack: a host whose name HAPPENS to end in the suffix + // text but on a different TLD. + expect( + matchesTrustedHost('alibaba-inc.com.evil.net', ['*.alibaba-inc.com']), + ).toBe(false); + }); + + it('mixed pattern list (exact + wildcard)', () => { + const patterns = [ + 'dashscope.aliyuncs.com', + '*.dashscope.aliyuncs.com', + '*.alibaba-inc.com', + ]; + expect(matchesTrustedHost('dashscope.aliyuncs.com', patterns)).toBe(true); + expect(matchesTrustedHost('sub.dashscope.aliyuncs.com', patterns)).toBe( + true, + ); + expect(matchesTrustedHost('idealab.alibaba-inc.com', patterns)).toBe(true); + expect(matchesTrustedHost('api.openai.com', patterns)).toBe(false); + }); + + it('empty pattern list rejects everything', () => { + expect(matchesTrustedHost('dashscope.aliyuncs.com', [])).toBe(false); + }); +}); + +describe('extractRequestHost', () => { + it('extracts from string URL', () => { + expect(extractRequestHost('https://dashscope.aliyuncs.com/v1/x')).toBe( + 'dashscope.aliyuncs.com', + ); + }); + + it('extracts from URL object', () => { + expect(extractRequestHost(new URL('https://api.openai.com/v1'))).toBe( + 'api.openai.com', + ); + }); + + it('extracts from Request', () => { + expect( + extractRequestHost(new Request('https://api.anthropic.com/v1/messages')), + ).toBe('api.anthropic.com'); + }); + + it('returns undefined for unparseable string', () => { + expect(extractRequestHost('not a url')).toBeUndefined(); + }); +}); + +describe('DEFAULT_SESSION_ID_HEADER_HOSTS', () => { + it('matches DashScope global + intl endpoints', () => { + expect( + matchesTrustedHost( + 'dashscope.aliyuncs.com', + DEFAULT_SESSION_ID_HEADER_HOSTS, + ), + ).toBe(true); + expect( + matchesTrustedHost( + 'dashscope-intl.aliyuncs.com', + DEFAULT_SESSION_ID_HEADER_HOSTS, + ), + ).toBe(true); + }); + + it('matches internal *.alibaba-inc.com / *.aliyun-inc.com', () => { + expect( + matchesTrustedHost( + 'idealab.alibaba-inc.com', + DEFAULT_SESSION_ID_HEADER_HOSTS, + ), + ).toBe(true); + expect( + matchesTrustedHost('gw.aliyun-inc.com', DEFAULT_SESSION_ID_HEADER_HOSTS), + ).toBe(true); + }); + + it('does NOT match third-party LLM providers', () => { + expect( + matchesTrustedHost('api.openai.com', DEFAULT_SESSION_ID_HEADER_HOSTS), + ).toBe(false); + expect( + matchesTrustedHost('api.anthropic.com', DEFAULT_SESSION_ID_HEADER_HOSTS), + ).toBe(false); + expect( + matchesTrustedHost('openrouter.ai', DEFAULT_SESSION_ID_HEADER_HOSTS), + ).toBe(false); + expect( + matchesTrustedHost( + 'generativelanguage.googleapis.com', + DEFAULT_SESSION_ID_HEADER_HOSTS, + ), + ).toBe(false); + }); +}); diff --git a/packages/core/src/telemetry/trusted-llm-hosts.ts b/packages/core/src/telemetry/trusted-llm-hosts.ts new file mode 100644 index 0000000000..540dc1474d --- /dev/null +++ b/packages/core/src/telemetry/trusted-llm-hosts.ts @@ -0,0 +1,89 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Default allowlist for outbound correlation headers + * (`X-Qwen-Code-Session-Id`). Limited to Alibaba/DashScope endpoints where + * the LLM provider, the upstream telemetry backend (ARMS Tracing), and + * the qwen-code distribution are the same legal entity — so the session + * id stays a first-party, in-vendor correlation handle. + * + * Why a default scope (rather than broadcasting to every LLM provider): + * PR #4390 review (LaZzyMan) pointed out that an open-source CLI sending + * a stable cross-request identifier to arbitrary third-party providers + * (OpenAI, Anthropic, OpenRouter, ...) is a cross-vendor fingerprinting + * surface those providers don't need for the API call itself. Restricting + * the default to in-vendor destinations mirrors the claude-code pattern + * (first-party client → first-party backend), preserves the real product + * value (ARMS server-side trace stitching against DashScope), and makes + * the third-party-broadcast failure mode opt-in rather than default. + * + * Mirrors `DashScopeOpenAICompatibleProvider.isDashScopeProvider` so the + * two layers stay aligned. If you add a hostname there, add it here too. + * + * Pattern syntax (matches `matchesTrustedHost` below): + * - bare hostname → exact match (case-insensitive) + * - `*.suffix` → matches `suffix` itself AND any sub-domain of it + * (e.g. `*.alibaba-inc.com` matches `alibaba-inc.com` + * and `gw.alibaba-inc.com`) + */ +export const DEFAULT_SESSION_ID_HEADER_HOSTS: readonly string[] = [ + 'dashscope.aliyuncs.com', + 'dashscope-intl.aliyuncs.com', + '*.dashscope.aliyuncs.com', + '*.dashscope-intl.aliyuncs.com', + '*.alibaba-inc.com', + '*.aliyun-inc.com', +]; + +/** + * Check whether `hostname` matches any pattern in `patterns`. + * Case-insensitive. Empty `hostname` always returns false. + * + * `*.suffix` patterns match: + * - the suffix domain itself (`alibaba-inc.com` matches `*.alibaba-inc.com`) + * - any sub-domain (`gw.alibaba-inc.com` matches `*.alibaba-inc.com`) + * + * This is intentionally a tiny pure helper, not a generic glob. Anything + * more elaborate (regex, port-aware, scheme-aware) should be added at the + * call site, not here, so the allowlist semantics stay obvious from the + * pattern strings users put in settings. + */ +export function matchesTrustedHost( + hostname: string, + patterns: readonly string[], +): boolean { + if (!hostname) return false; + const h = hostname.toLowerCase(); + for (const raw of patterns) { + const p = raw.toLowerCase(); + if (p.startsWith('*.')) { + const bare = p.slice(2); + if (h === bare || h.endsWith('.' + bare)) return true; + } else if (h === p) { + return true; + } + } + return false; +} + +/** + * Extract the destination hostname from a fetch input. Returns `undefined` + * if the URL can't be parsed — caller should treat that as "not on the + * allowlist" (fail closed). + */ +export function extractRequestHost( + input: string | URL | Request, +): string | undefined { + try { + if (typeof input === 'string') return new URL(input).hostname; + if (input instanceof URL) return input.hostname; + if (input instanceof Request) return new URL(input.url).hostname; + } catch { + return undefined; + } + return undefined; +}