From 9bdd3bd6f9a173ca1603804d5174c0ff242b0398 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 25 May 2026 13:42:32 +0800 Subject: [PATCH] refactor(telemetry): split outbound correlation out of telemetry scope (R4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address LaZzyMan round-8 follow-up review on PR #4390: even though R3's host allowlist made the default behavior safe, the meta-architectural concern remains: telemetry's namespace and consent flow shouldn't quietly extend to wire-level behavior aimed at third-party LLM provider request streams. The recipient sets differ; the consent decisions differ; they deserve separate namespaces, separate threat models, separate PRs. This commit (called R4 in the design doc) collapses the PR scope so it lands ONLY telemetry observability work: REMOVED from this PR: - packages/core/src/telemetry/llm-correlation-fetch.ts(.test.ts) - packages/core/src/telemetry/trusted-llm-hosts.ts(.test.ts) - telemetry.sessionIdHeaderHosts setting + Config getter + resolveTelemetrySettings wiring + settingsSchema entry - wrapFetchWithCorrelation usage from four provider construction points (default.ts, dashscope.ts, anthropicContentGenerator.ts, geminiContentGenerator/index.ts) - All session-id provider tests across the four providers + the contentGenerator.test.ts mock stub - "Session correlation header" section in telemetry.md ADDED: - OutboundCorrelationSettings interface in packages/core/src/config/config.ts, standalone top-level namespace separate from TelemetrySettings — SECURITY-RELEVANT label, all defaults off - Config.getOutboundCorrelationPropagateTraceContext() getter - outboundCorrelation top-level entry in settingsSchema.ts with propagateTraceContext: { default: false } and explicit SECURITY-RELEVANT framing in the description - CLI config-load pipeline passes settings.outboundCorrelation into ConfigParameters - NOOP_PROPAGATOR (TextMapPropagator no-op) in sdk.ts, conditionally installed on NodeSDK when propagateTraceContext is false (default). When true, omits textMapPropagator from NodeSDK options so the SDK keeps its default W3C composite propagator - 2 new sdk.test.ts cases covering the propagator gate behavior UNCHANGED: - UndiciInstrumentation registration + OTLP feedback-loop guard + HttpInstrumentation OTLP guard from R2/R3 stay intact — they are pure telemetry (client HTTP spans into the operator's own OTLP collector), no wire-level data egress - Documentation rewrites telemetry.md to split "client-side HTTP span on outbound fetch" (telemetry) from a new "Outbound correlation (SECURITY-RELEVANT)" top-level section - design doc gets R4 revision row + new §12 "R4 Scope Conflation Split" capturing the rationale and follow-up PR outline The session-id apparatus (R3 code) lives in git history at commits 1c8528a56 / cb162e716 / 7a1b4f8d0 / 40e1efc1f / 106598ca2; the follow-up PR can cherry-pick or restore those files under the new outboundCorrelation.* namespace as LazzyMan suggested. Vscode-ide-companion settings.schema.json regenerated. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- .../telemetry-outbound-propagation-design.md | 112 ++- docs/developers/development/telemetry.md | 153 ++-- packages/cli/src/config/config.ts | 1 + packages/cli/src/config/settingsSchema.ts | 30 +- packages/core/src/config/config.ts | 80 ++- .../anthropicContentGenerator.test.ts | 24 - .../anthropicContentGenerator.ts | 19 - .../core/src/core/contentGenerator.test.ts | 2 - .../core/geminiContentGenerator/index.test.ts | 99 --- .../src/core/geminiContentGenerator/index.ts | 22 - .../provider/dashscope.test.ts | 11 - .../provider/dashscope.ts | 5 - .../provider/default.test.ts | 44 -- .../provider/default.ts | 9 - packages/core/src/telemetry/config.ts | 1 - .../telemetry/llm-correlation-fetch.test.ts | 662 ------------------ .../src/telemetry/llm-correlation-fetch.ts | 247 ------- packages/core/src/telemetry/sdk.test.ts | 45 ++ packages/core/src/telemetry/sdk.ts | 43 +- .../src/telemetry/trusted-llm-hosts.test.ts | 181 ----- .../core/src/telemetry/trusted-llm-hosts.ts | 89 --- .../schemas/settings.schema.json | 19 +- 22 files changed, 334 insertions(+), 1564 deletions(-) delete mode 100644 packages/core/src/telemetry/llm-correlation-fetch.test.ts delete mode 100644 packages/core/src/telemetry/llm-correlation-fetch.ts delete mode 100644 packages/core/src/telemetry/trusted-llm-hosts.test.ts delete mode 100644 packages/core/src/telemetry/trusted-llm-hosts.ts diff --git a/docs/design/telemetry-outbound-propagation-design.md b/docs/design/telemetry-outbound-propagation-design.md index 44c817f9f2..0cf1f5f215 100644 --- a/docs/design/telemetry-outbound-propagation-design.md +++ b/docs/design/telemetry-outbound-propagation-design.md @@ -7,13 +7,14 @@ ## 修订历史 -| 修订 | 日期 | 触发 | 摘要 | -| ---- | ---------- | ------------------------ | ----------------------------------------------------------------------------------------------------------------- | -| R1 | 2026-05-21 | 初稿 | 全广播：所有出站 LLM 请求都带 `X-Qwen-Code-Session-Id` + `traceparent` | -| R2 | 2026-05-22 | wenshao R2/R3 review | 边界安全：URL normalize、port matching、quote 对齐、staticCorrelationHeaders try/catch、host:port fallback strip | -| R3 | 2026-05-23 | LaZzyMan REQUEST_CHANGES | **重大语义改动**：`X-Qwen-Code-Session-Id` 默认作用域收窄到 first-party（Alibaba/DashScope）host 白名单。详见 §11 | +| 修订 | 日期 | 触发 | 摘要 | +| ---- | ---------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| R1 | 2026-05-21 | 初稿 | 全广播：所有出站 LLM 请求都带 `X-Qwen-Code-Session-Id` + `traceparent` | +| R2 | 2026-05-22 | wenshao R2/R3 review | 边界安全：URL normalize、port matching、quote 对齐、staticCorrelationHeaders try/catch、host:port fallback strip | +| R3 | 2026-05-23 | LaZzyMan REQUEST_CHANGES | **重大语义改动**：`X-Qwen-Code-Session-Id` 默认作用域收窄到 first-party（Alibaba/DashScope）host 白名单。详见 §11 | +| R4 | 2026-05-25 | LaZzyMan round-8 follow-up (scope conflation) | **PR scope 大幅收窄**：本 PR 仅保留 client HTTP span + OTLP loop guard；`traceparent` 默认 off（NoopTextMapPropagator）；新增 `outboundCorrelation.*` 顶级 namespace 放安全相关 toggle；R3 落地的整套 `X-Qwen-Code-Session-Id` 机器**移除本 PR**，搬到独立 follow-up PR。详见 §12 | -**特别提示**：阅读 §3.1（目标）/ §3.2（非目标）/ §4.3（Part B 设计）/ §4.4（配置 schema 影响）/ §5（文件改动清单）/ §9（与 claude-code 对比）/ §10（未来工作）时，请同时参考 §11 —— **R3 修订让上述章节中关于"统一向所有 LLM provider 发送 session id 不区分第一方/第三方"的论断不再成立**。原文保留作为决策路径记录。 +**特别提示**：阅读 §3.1（目标）/ §3.2（非目标）/ §4.3（Part B 设计）/ §4.4（配置 schema 影响）/ §5（文件改动清单）/ §9（与 claude-code 对比）/ §10（未来工作）/ §11（R3 host-allowlist scoping）时，请同时参考 §12 —— **R4 修订让 R1-R3 关于"本 PR 同时落地 traceparent + session id header"的论断不再成立**：本 PR 现仅为 telemetry observability + 独立的 outbound trace-context toggle，所有 outbound correlation header 工作（包括 R3 的 host allowlist）整体搬到独立 follow-up PR。R3 工作代码本身没浪费，挪到 follow-up PR 即可复用。 ## 1. 背景 @@ -764,3 +765,102 @@ Gemini SDK 有两个不可见 default endpoint（`generativelanguage.googleapis. - **Per-request random UUID** (`X-Qwen-Code-Request-Id`) — LazzyMan 提的替代方案，列入 §10 - **Gemini staleness lazy-invalidate** (§8.6 选项 A) — 与 R3 解耦，独立 sub-issue - **`matchesTrustedHost` IPv6 支持** — 当前 IPv6 destination 永不在 allowlist 上（`URL.hostname` 返回 `[::1]` 带方括号，pattern 语法无对应形式）。当前满足"命名 first-party endpoint"用例。若将来有 raw IP allowlist 需求再扩展。 + +## 12. R4 修订 — Scope Conflation Split + +> 触发：[LaZzyMan round-8 follow-up review on PR #4390](https://github.com/QwenLM/qwen-code/pull/4390) +> 落地：本 PR 收窄；R3 落地的 session-id 整套挪到独立 follow-up PR + +### 12.1 触发与论证 + +R3 化解了 LaZzyMan 第一轮 review 的「广播稳定指纹给第三方 provider」担忧（severity: high）。但在 round-8 follow-up 中他升级到更深的架构原则反对： + +> "Telemetry is not a container for adjacent features. The `traceparent` cross-process propagation and the `X-Qwen-Code-Session-Id` header injection are **not telemetry**. They are outbound-identity / outbound-correlation work that uses some OTel APIs internally as an implementation detail." + +他的核心元论点： + +- **"telemetry" namespace 暗示 recipient = 用户自己的 OTLP collector** +- 但 `traceparent` 和 `X-Qwen-Code-Session-Id` 的 recipient = **第三方 LLM provider** +- 两类不同 recipient 应该有两类不同的同意决策树 +- 即使默认行为安全（R3 已实现），把 wire-level 行为放在 `telemetry.*` 下**设了坏先例**：未来 telemetry PR 可以继续偷渡 wire 行为给第三方 +- "If we accept that principle, the split is mechanical. If we don't, this PR is the wrong place to debate it because the technical fixes are already in." + +### 12.2 解法概要（"方案 C" hybrid split） + +经过几轮内部讨论（含 yiliang 提出的 customHeader 模板替代方案，最终判定 customHeader 不能携带 runtime-dynamic 值），决定走 **方案 C**： + +**本 PR 留下**： + +- `UndiciInstrumentation` 注册（产 client HTTP span → 用户自家 OTLP collector） +- OTLP feedback-loop guard（前者的必要副作用） +- **`NoopTextMapPropagator` 默认安装** → `propagation.inject()` 是 no-op → outbound `fetch` 上**不再有 `traceparent`** +- **新增 `outboundCorrelation.propagateTraceContext: bool` (默认 false)** 作为独立 namespace 顶级设置；设 true 时安装默认 W3C composite propagator +- 整套 `R3 session-id` 代码（`llm-correlation-fetch.ts` / `trusted-llm-hosts.ts` / `telemetry.sessionIdHeaderHosts` setting / 4 个 provider 集成点 / 所有相关测试）**全部移除** + +**搬到 follow-up PR**： + +- `X-Qwen-Code-Session-Id` header 整套机器（R3 实现复用） +- 进入新 `outboundCorrelation.*` namespace（具体 setting key TBD，但**不会**叫 `telemetry.*`） +- Follow-up PR 自带：threat model section、独立 review、security-relevant 标注的 docs +- `X-Qwen-Code-Request-Id` per-request UUID（LazzyMan 在 R3 round 提出的替代设计）也归入此 follow-up 的考虑范围 + +### 12.3 与 R3 R1 论点的映射 + +| R1/R3 论点 | R4 后状态 | +| --------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------- | +| §3.1 "所有出站 LLM 请求带 traceparent" | ❌ **R4 默认 off**；需 `outboundCorrelation.propagateTraceContext: true` 才开 | +| §3.1 "所有出站 LLM 请求带 `X-Qwen-Code-Session-Id`" | ❌ **R4 整套移出本 PR**，搬到 follow-up PR | +| §4.3 fetch wrapper 注入 session id | ❌ 整段代码不在本 PR；复用到 follow-up PR | +| §11 host allowlist (R3 设计) | ❌ 同上；整体迁移 follow-up PR | +| §4.4 不引入新 setting | ❌ **本 PR 新增 `outboundCorrelation.propagateTraceContext`** 一个 boolean；session id 相关 setting 在 follow-up PR | +| §10 future work "`X-Qwen-Code-Request-Id`" | ✅ 仍是 future work；与 session-id follow-up 一起设计 | + +### 12.4 新 namespace 设计意图 + +`outboundCorrelation.*` 顶级 namespace 在本 PR 只有一个 boolean (`propagateTraceContext`)，看起来过度结构化。但这是**精心选择的**： + +- **建立命名空间作为承诺**：让后续 session-id / request-id / etc. 自然进入这个 namespace +- **标注为 security-relevant**：`settingsSchema.ts` description 显式写 "SECURITY-RELEVANT"，文档化为"安全设置"而非"observability 设置" +- **defaults 全部 off**：符合 LazzyMan 提出的"open-source 客户端不应未经显式同意向第三方发稳定 id"原则 +- **与 telemetry.\* 解耦**：用户读 settings.json 看到 `outboundCorrelation.*` 立刻能识别这是出站 wire 行为，不是 observability + +### 12.5 实施 + +| 文件 | 改动 | +| ------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `packages/core/src/telemetry/llm-correlation-fetch.ts` | **删除** | +| `packages/core/src/telemetry/llm-correlation-fetch.test.ts` | **删除** | +| `packages/core/src/telemetry/trusted-llm-hosts.ts` | **删除** | +| `packages/core/src/telemetry/trusted-llm-hosts.test.ts` | **删除** | +| `packages/core/src/telemetry/sdk.ts` | + `NoopTextMapPropagator`；按 `getOutboundCorrelationPropagateTraceContext()` 决定 SDK textMapPropagator | +| `packages/core/src/core/openaiContentGenerator/provider/default.ts` | 移除 `wrapFetchWithCorrelation` 引用 | +| `packages/core/src/core/openaiContentGenerator/provider/dashscope.ts` | 同上 | +| `packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts` | 同上 | +| `packages/core/src/core/geminiContentGenerator/index.ts` | 移除 `staticCorrelationHeaders` 引用 | +| 上述 4 个 provider 的 `*.test.ts` | 删 session-id 相关测试 case | +| `packages/core/src/config/config.ts` | 删 `TelemetrySettings.sessionIdHeaderHosts`、`getTelemetrySessionIdHeaderHosts`；**新增 `OutboundCorrelationSettings` 接口 + `outboundCorrelationSettings` 字段 + `getOutboundCorrelationPropagateTraceContext()` getter** | +| `packages/core/src/telemetry/config.ts` | 删 `resolveTelemetrySettings` 中 sessionIdHeaderHosts 透传 | +| `packages/cli/src/config/settingsSchema.ts` | 删 `sessionIdHeaderHosts` schema；**新增 `outboundCorrelation` 顶级 schema 项** | +| `packages/cli/src/config/config.ts` | 透传 `outboundCorrelation: settings.outboundCorrelation` 进 `ConfigParameters` | +| `packages/vscode-ide-companion/schemas/settings.schema.json` | `npm run generate:settings-schema` 重新生成 | +| `docs/developers/development/telemetry.md` | 重写 "Trace context propagation" → "Client-side HTTP span on outbound fetch"；删 "Session correlation header" 整节；新增 "Outbound correlation (SECURITY-RELEVANT)" 顶级 section | +| `docs/design/telemetry-outbound-propagation-design.md` | 本节 + R4 表头 + 修订指针 | + +### 12.6 对 LazzyMan 元论点的回应 + +| 论点 | R4 后状态 | +| ----------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| "Telemetry namespace 暗示自家 collector 接收方" | ✅ wire 行为已搬出 `telemetry.*`；新 `outboundCorrelation.*` namespace 显式标识"出站第三方"语义 | +| "默认行为不应未经显式同意向第三方发标识符" | ✅ `propagateTraceContext` 默认 false；session-id 整套 follow-up PR 也将默认 off | +| "telemetry PR 不应偷渡 wire-level 行为" | ✅ 本 PR 不再添加任何"telemetry 控制 wire 行为"的代码路径；wire 行为统一由 `outboundCorrelation.*` 管 | +| "split is mechanical, work isn't wasted" | ✅ R3 落地代码物理删除自本 branch，留在 git history 里给 follow-up PR 复用（或 cherry-pick） | + +### 12.7 follow-up PR 大纲（信息性，不在本 PR 范围） + +未来 follow-up PR 应包含： + +- `outboundCorrelation.sessionIdHeader: { enabled, trustedHosts }` 或类似 setting +- 复用 R3 已实现的 `wrapFetchWithCorrelation` / `matchesTrustedHost` / `DEFAULT_SESSION_ID_HEADER_HOSTS` 代码骨架 +- threat model 一节，明确：recipient 集合、稳定 id 的去匿名化窗口、可选 per-request UUID 配套 +- **默认 off**（无 default allowlist —— 比 R3 更严，符合 LazzyMan 的开源 CLI 原则） +- security-relevant 标注 + docs/users/configuration/settings.md 收录 diff --git a/docs/developers/development/telemetry.md b/docs/developers/development/telemetry.md index 2cf62c9827..b826d0472f 100644 --- a/docs/developers/development/telemetry.md +++ b/docs/developers/development/telemetry.md @@ -262,25 +262,23 @@ and logs still carry `session.id`, and trace / log backends (Jaeger, Tempo, Loki, Aliyun SLS / ARMS Tracing) handle per-session slicing natively without cardinality pressure. -### Trace context propagation +### Client-side HTTP span on outbound fetch -When telemetry is enabled, Qwen Code instruments outgoing `fetch()` requests -(used by `openai`, `@google/genai`, and `@anthropic-ai/sdk`) and automatically -injects the standard W3C `traceparent` header on every LLM service call: +When telemetry is enabled, Qwen Code registers `UndiciInstrumentation` +which creates a client-side HTTP span for every outbound `fetch()` +request originated by the process — including the LLM SDKs (`openai`, +`@google/genai`, `@anthropic-ai/sdk`), the MCP StreamableHTTP client, the +`WebFetch` tool, and any IDE-extension out-of-process calls. The span +lets you see network latency (TTFB / response body transfer) separately +from upstream model processing time, which the existing +`api.generateContent` span alone can't distinguish. -``` -traceparent: 00-<32-hex traceId>-<16-hex parentSpanId>-<01-sampled | 00-not-sampled> -``` - -The traceId is the qwen-code interaction trace id; the parent span is the -active `api.generateContent` span. Any OTel-instrumented LLM service (e.g. -DashScope serving from an ARMS Tracing backend) that reads this header will -make its server-side spans children of qwen-code's trace, giving you a single -end-to-end trace tree across the process boundary. - -You also get a free client-side HTTP span per LLM call — useful for separating -network latency (TTFB / response body transfer) from upstream model processing -time, which the existing `api.generateContent` span alone can't distinguish. +These spans go to your **own** OTLP collector (or file outfile) just like +the rest of the telemetry — they do not affect what is written onto the +outbound HTTP request itself. Whether the W3C `traceparent` header is +also written into the outgoing request stream is controlled by a +**separate, security-relevant setting** documented in +[outbound correlation](#outbound-correlation-security-relevant) below. **Feedback-loop avoidance.** OTel SDK uses `fetch` internally to upload OTLP data. Without protection, instrumenting `fetch` would trace those uploads, @@ -291,98 +289,53 @@ URLs matching the configured `telemetry.otlpEndpoint` / `telemetry.otlpMetricsEndpoint` prefixes. In file-outfile mode there are no outbound HTTP uploads, so the hook is a no-op. -**Scope: all `fetch()` calls, not just LLM.** The undici instrumentation -patches `globalThis.fetch` for the entire process, so a client span + -`traceparent` injection happens on every outbound `fetch` — including the -`WebFetch` tool, MCP StreamableHTTP clients, and any IDE-extension -out-of-process calls — not only LLM SDK traffic. Two consequences worth -knowing: +## Outbound correlation (SECURITY-RELEVANT) -- **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 — 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. +These settings live in a **separate top-level namespace** from `telemetry.*` +on purpose: telemetry controls data flow into the operator's own +observability backend, while `outboundCorrelation.*` controls what +client-side correlation data qwen-code writes **into outbound LLM API +request streams** that reach third-party LLM provider endpoints +(DashScope, OpenAI, Anthropic, etc.). Different recipients, different +consent decision. **All values default to off.** See PR #4390 review +discussion for the framing rationale. -### Session correlation header - -Alongside `traceparent`, Qwen Code can inject a custom HTTP header on -outbound LLM requests when telemetry is enabled: - -``` -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 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`: +### `outboundCorrelation.propagateTraceContext` ```jsonc -"telemetry": { - "sessionIdHeaderHosts": ["*"] // restore broadcast - "sessionIdHeaderHosts": [] // fully disable header - "sessionIdHeaderHosts": ["api.mycompany.com", - "*.gateway.mycompany.internal"] // custom allowlist +"outboundCorrelation": { + "propagateTraceContext": false // default } ``` -The header value comes from `Config.getSessionId()`, read fresh on every -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. +When `false` (default), Qwen Code installs a no-op `TextMapPropagator` on +the OTel SDK. UndiciInstrumentation still creates client HTTP spans for +your OTLP collector, but `propagation.inject()` is a no-op so **no +`traceparent` is written onto outbound requests**. Trace IDs stay +internal to the operator's collector. -Empty session ids are not emitted (some HTTP middleware rejects empty -header values). The header is omitted entirely when telemetry is disabled -or the destination host is outside the allowlist. +When `true`, the SDK's default W3C composite propagator +(`tracecontext` + `baggage`) is installed and the standard `traceparent` +header is written on every outbound `fetch`: -**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. 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. +``` +traceparent: 00-<32-hex traceId>-<16-hex parentSpanId>-<01-sampled | 00-not-sampled> +``` + +Opt in only when the LLM provider also reports into your OTel collector +for cross-process trace stitching — e.g. ARMS Tracing serving DashScope. +For most operators the value is `false`; cross-vendor trace continuation +is niche. + +### Other outbound correlation headers + +`X-Qwen-Code-Session-Id` and `X-Qwen-Code-Request-Id` are **not part of +this PR**. They will be designed and proposed in their own follow-up +PR(s) under the same `outboundCorrelation.*` namespace, each with its +own threat model and operator-consent flow. PR #4390 review (LaZzyMan) +established the principle: "telemetry's scope of work doesn't include +sending identifiers to LLM providers"; correlation-header work moves to +its own design discussion rather than landing under telemetry. ## Aliyun Telemetry diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index baea941922..88df7dcebc 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1710,6 +1710,7 @@ export async function loadCliConfig( screenReader, }, telemetry: telemetrySettings, + outboundCorrelation: settings.outboundCorrelation, usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled ?? true, clearContextOnIdle: settings.context?.clearContextOnIdle, fileFiltering: settings.context?.fileFiltering, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 5ece7c6eec..161c4afe5d 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -8,6 +8,7 @@ import type { MCPServerConfig, BugCommandSettings, TelemetrySettings, + OutboundCorrelationSettings, AuthType, ChatCompressionSettings, ModelProvidersConfig, @@ -1032,17 +1033,34 @@ 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, }, }, + outboundCorrelation: { + type: 'object', + label: 'Outbound Correlation', + category: 'Advanced', + requiresRestart: true, + default: undefined as OutboundCorrelationSettings | undefined, + description: + "SECURITY-RELEVANT. Controls what client-side correlation data qwen-code writes into outbound LLM API requests (DashScope, OpenAI, Anthropic, etc.) — separate from `telemetry.*` which governs data flow into the operator's OWN OTLP collector. All values default to off. Opt in only when the LLM provider also reports into your OTel collector for cross-process trace stitching (e.g. ARMS Tracing + DashScope).", + showInDialog: false, + jsonSchemaOverride: { + type: 'object', + properties: { + propagateTraceContext: { + description: + "Inject W3C `traceparent` header on outbound `fetch` requests (LLM SDK calls, MCP StreamableHTTP, WebFetch, ...). Default: false — trace context stays internal to the operator's OTLP collector and is NOT written onto third-party request streams. Set true only when you want cross-process trace stitching with an OTel-aware LLM provider (e.g. ARMS+DashScope). Note: client HTTP spans are still emitted in either case; this flag only governs the wire `traceparent` header.", + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + }, + fastModel: { type: 'string', label: 'Fast Model', diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 6044c58f21..1938836b92 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -314,23 +314,6 @@ 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). @@ -354,6 +337,42 @@ export interface TelemetryMetricsSettings { includeSessionId?: boolean; } +/** + * Security-relevant settings controlling what client-side correlation + * data qwen-code writes into outbound LLM API requests. + * + * **Why this is a separate namespace from `telemetry.*`:** telemetry + * controls data flow into the user's OWN observability backend (OTLP + * collector / file outfile). The settings here control data flow OUT of + * the qwen-code process and INTO third-party LLM provider request + * streams (DashScope, OpenAI, Anthropic, etc.). Different recipients = + * different consent decision, so a different settings tree. See PR + * #4390 review (LaZzyMan) for the framing rationale. + * + * All values default to off / no propagation. Operators who want to + * propagate trace context for server-side trace stitching (e.g. ARMS + * Tracing + DashScope) opt in explicitly. + */ +export interface OutboundCorrelationSettings { + /** + * Inject W3C `traceparent` header on outbound HTTP requests + * originated by undici / global `fetch` (LLM SDK calls, MCP + * StreamableHTTP clients, WebFetch tool, etc.). Default: `false`. + * + * When `false`, the SDK is configured with a no-op + * `TextMapPropagator` so trace context stays internal to the user's + * OTLP collector (operator still gets client HTTP spans, but the + * trace id is not written onto third-party request streams). + * + * When `true`, the OTel default W3C composite propagator + * (`tracecontext` + `baggage`) is installed and `traceparent` is + * written on every outbound `fetch`. Useful when the LLM provider + * also reports into the operator's OTel collector — e.g. ARMS + * Tracing + DashScope — for cross-process trace stitching. + */ + propagateTraceContext?: boolean; +} + export interface OutputSettings { format?: OutputFormat; } @@ -582,6 +601,7 @@ export interface ConfigParameters { contextFileName?: string | string[]; accessibility?: AccessibilitySettings; telemetry?: TelemetrySettings; + outboundCorrelation?: OutboundCorrelationSettings; gitCoAuthor?: GitCoAuthorParam; usageStatisticsEnabled?: boolean; /** @@ -871,6 +891,7 @@ export class Config { private autoModeDenialState: AutoModeDenialState = createDenialState(); private readonly accessibility: AccessibilitySettings; private readonly telemetrySettings: TelemetrySettings; + private readonly outboundCorrelationSettings: OutboundCorrelationSettings; private readonly gitCoAuthor: GitCoAuthorSettings; private readonly usageStatisticsEnabled: boolean; private readonly fileReadCacheDisabled: boolean; @@ -1037,9 +1058,12 @@ export class Config { outfile: params.telemetry?.outfile, resourceAttributes: params.telemetry?.resourceAttributes, metrics: params.telemetry?.metrics, - sessionIdHeaderHosts: params.telemetry?.sessionIdHeaderHosts, resourceAttributeWarnings: params.telemetry?.resourceAttributeWarnings, }; + this.outboundCorrelationSettings = { + propagateTraceContext: + params.outboundCorrelation?.propagateTraceContext ?? false, + }; this.gitCoAuthor = { ...normalizeGitCoAuthor(params.gitCoAuthor), name: 'Qwen-Coder', @@ -2867,21 +2891,19 @@ 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 ?? []; } + /** + * Whether to inject W3C `traceparent` on outbound `fetch` requests + * (LLM SDKs, MCP, WebFetch, etc.). Default false — see + * `OutboundCorrelationSettings` for rationale. + */ + getOutboundCorrelationPropagateTraceContext(): boolean { + return this.outboundCorrelationSettings.propagateTraceContext ?? false; + } + getTelemetryOutfile(): string | undefined { return this.telemetrySettings.outfile; } diff --git a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts index dd4d70b2c5..3d3bb251fb 100644 --- a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts +++ b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts @@ -106,30 +106,6 @@ describe('AnthropicContentGenerator', () => { vi.restoreAllMocks(); }); - it('installs the correlation fetch wrapper on the Anthropic client', async () => { - const { AnthropicContentGenerator } = await importGenerator(); - void new AnthropicContentGenerator( - { - model: 'claude-test', - apiKey: 'test-key', - baseUrl: 'https://api.anthropic.com', - timeout: 10_000, - maxRetries: 2, - samplingParams: {}, - schemaCompliance: 'auto', - }, - mockConfig, - ); - const fetchFn = anthropicState.constructorOptions?.['fetch'] as - | unknown - | undefined; - // Wrapper installed regardless of telemetry-enabled state — per-call - // header injection is gated inside the wrapper itself. Exhaustive - // behavior is in llm-correlation-fetch.test.ts. - expect(typeof fetchFn).toBe('function'); - expect(fetchFn).not.toBe(globalThis.fetch); - }); - it('uses claude-cli identity (User-Agent + x-app + Bearer auth) for non-Anthropic baseURLs', async () => { // Non-Anthropic-native baseURL → IdeaLab-style proxy path: // - User-Agent presents as `claude-cli/ (external, cli)` diff --git a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts index b9b613bd2e..2f6ba63cb5 100644 --- a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts +++ b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts @@ -33,7 +33,6 @@ import { buildRuntimeFetchOptions, redactProxyError, } from '../../utils/runtimeFetchOptions.js'; -import { wrapFetchWithCorrelation } from '../../telemetry/llm-correlation-fetch.js'; import { DEFAULT_TIMEOUT } from '../openaiContentGenerator/constants.js'; import { createDebugLogger } from '../../utils/debugLogger.js'; import { runtimeDiagnostics } from '../../utils/runtimeDiagnostics.js'; @@ -204,13 +203,6 @@ export class AnthropicContentGenerator implements ContentGenerator { // key as `X-Api-Key` to the IdeaLab proxy — leaking the credential to // a third-party endpoint. Explicit `null` suppresses the back-fill // and forces the intended auth path. - // Wrap fetch with per-request correlation header injection. Read the - // base fetch from runtimeOptions (proxy-aware) when present, else - // globalThis. See design doc §4.3 — fetch wrapper (not defaultHeaders) - // is required so the header tracks live session id across /clear resets. - const baseFetch = - (runtimeOptions as { fetch?: typeof fetch } | undefined)?.fetch ?? - globalThis.fetch; this.client = new Anthropic({ ...(useProxyIdentity ? { authToken: contentGeneratorConfig.apiKey, apiKey: null } @@ -220,17 +212,6 @@ export class AnthropicContentGenerator implements ContentGenerator { maxRetries: contentGeneratorConfig.maxRetries, defaultHeaders, ...runtimeOptions, - // Cast through unknown: Anthropic SDK's `Fetch` type uses the older - // DOM-style `RequestInfo` (no URL) which `typeof fetch` (Node WHATWG - // overloads) is not structurally assignable to, even though they're - // call-compatible at runtime. The cast is safe because the wrapper - // delegates to baseFetch (= runtimeOptions.fetch ?? globalThis.fetch) - // without altering its call shape. - fetch: wrapFetchWithCorrelation( - baseFetch, - this.cliConfig, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as unknown as any, }); this.converter = new AnthropicContentConverter( diff --git a/packages/core/src/core/contentGenerator.test.ts b/packages/core/src/core/contentGenerator.test.ts index 2816b145f9..5f9199eb19 100644 --- a/packages/core/src/core/contentGenerator.test.ts +++ b/packages/core/src/core/contentGenerator.test.ts @@ -24,7 +24,6 @@ describe('createContentGenerator', () => { getCliVersion: () => '1.0.0', getTelemetryEnabled: () => false, getSessionId: () => 'test-session', - getTelemetrySessionIdHeaderHosts: () => undefined, } as unknown as Config; const mockGenerator = { @@ -62,7 +61,6 @@ 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 d9d9f92350..cebbba390b 100644 --- a/packages/core/src/core/geminiContentGenerator/index.test.ts +++ b/packages/core/src/core/geminiContentGenerator/index.test.ts @@ -91,103 +91,4 @@ describe('createGeminiContentGenerator', () => { }), ); }); - - it('omits X-Qwen-Code-Session-Id from httpOptions.headers when telemetry is disabled', () => { - 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 } } - | undefined; - expect(callArgs?.httpOptions?.headers).toBeDefined(); - expect(callArgs?.httpOptions?.headers ?? {}).not.toHaveProperty( - 'X-Qwen-Code-Session-Id', - ); - }); - - 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('omits X-Qwen-Code-Session-Id when baseUrl is unset, even with allowlist override (factory fail-closed for unknown destination)', () => { - // The factory only passes a destinationUrl to staticCorrelationHeaders - // when `config.baseUrl` is set, because the Gemini SDK has two - // invisible defaults (generativelanguage.googleapis.com for public, - // {region}-aiplatform.googleapis.com for Vertex). Guessing one would - // mis-bucket the other under a googleapis-covering allowlist override. - // Operators who want correlation against Google endpoints must set - // `baseUrl` explicitly. - 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(['*']), - } 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', - ); - }); }); diff --git a/packages/core/src/core/geminiContentGenerator/index.ts b/packages/core/src/core/geminiContentGenerator/index.ts index 3d0f86ef8a..9507ebfaad 100644 --- a/packages/core/src/core/geminiContentGenerator/index.ts +++ b/packages/core/src/core/geminiContentGenerator/index.ts @@ -11,7 +11,6 @@ import type { } from '../contentGenerator.js'; import type { Config } from '../../config/config.js'; import { InstallationManager } from '../../utils/installationManager.js'; -import { staticCorrelationHeaders } from '../../telemetry/llm-correlation-fetch.js'; export { GeminiContentGenerator } from './geminiContentGenerator.js'; @@ -39,27 +38,6 @@ export function createGeminiContentGenerator( 'x-gemini-api-privileged-user-id': `${installationId}`, }; } - // Merge the session-id correlation header. `@google/genai`'s HttpOptions - // does not expose a `fetch` hook (unlike `openai` / `@anthropic-ai/sdk`), - // so we can only inject a static header here — captured at construction. - // 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. - // - // Destination resolution: only pass when we KNOW the destination — i.e. - // when the operator has explicitly set `config.baseUrl`. The SDK has two - // different invisible defaults (`generativelanguage.googleapis.com` for - // public Gemini, `{region}-aiplatform.googleapis.com` for Vertex AI) and - // guessing wrong here would mis-bucket Vertex traffic if the operator - // overrides `telemetry.sessionIdHeaderHosts` to include any googleapis - // domain. Passing `undefined` makes `staticCorrelationHeaders` return - // `{}` (no header) — the fail-closed default. Operators who want - // correlation against a Google endpoint set `baseUrl` explicitly - // (which is the same input the SDK uses to resolve the destination). - headers = { - ...headers, - ...staticCorrelationHeaders(gcConfig, config.baseUrl), - }; const httpOptions = config.baseUrl ? { headers, diff --git a/packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts b/packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts index 20098df342..3becf07f18 100644 --- a/packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts +++ b/packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts @@ -492,17 +492,6 @@ describe('DashScopeOpenAICompatibleProvider', () => { }), ); }); - - it('installs the correlation fetch wrapper on the OpenAI client', () => { - provider.buildClient(); - const callArg = vi.mocked(OpenAI).mock.calls[0]![0] as { - fetch?: typeof fetch; - }; - // Wrapper is always installed; per-request behavior depends on - // telemetry-enabled (verified end-to-end in llm-correlation-fetch.test.ts). - expect(typeof callArg.fetch).toBe('function'); - expect(callArg.fetch).not.toBe(globalThis.fetch); - }); }); describe('buildMetadata', () => { diff --git a/packages/core/src/core/openaiContentGenerator/provider/dashscope.ts b/packages/core/src/core/openaiContentGenerator/provider/dashscope.ts index aa4eba9f2f..644232b58b 100644 --- a/packages/core/src/core/openaiContentGenerator/provider/dashscope.ts +++ b/packages/core/src/core/openaiContentGenerator/provider/dashscope.ts @@ -16,7 +16,6 @@ import type { ChatCompletionToolWithCache, } from './types.js'; import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js'; -import { wrapFetchWithCorrelation } from '../../../telemetry/llm-correlation-fetch.js'; import { createDebugLogger } from '../../../utils/debugLogger.js'; import { DefaultOpenAICompatibleProvider } from './default.js'; @@ -139,9 +138,6 @@ export class DashScopeOpenAICompatibleProvider extends DefaultOpenAICompatiblePr 'openai', this.cliConfig.getProxy(), ); - const baseFetch = - (runtimeOptions as { fetch?: typeof fetch } | undefined)?.fetch ?? - globalThis.fetch; return new OpenAI({ apiKey, baseURL: baseUrl, @@ -149,7 +145,6 @@ export class DashScopeOpenAICompatibleProvider extends DefaultOpenAICompatiblePr maxRetries, defaultHeaders, ...(runtimeOptions || {}), - fetch: wrapFetchWithCorrelation(baseFetch, this.cliConfig), }); } diff --git a/packages/core/src/core/openaiContentGenerator/provider/default.test.ts b/packages/core/src/core/openaiContentGenerator/provider/default.test.ts index 37cb1c854b..ce57b5965f 100644 --- a/packages/core/src/core/openaiContentGenerator/provider/default.test.ts +++ b/packages/core/src/core/openaiContentGenerator/provider/default.test.ts @@ -168,50 +168,6 @@ describe('DefaultOpenAICompatibleProvider', () => { }), ); }); - - it('installs the correlation fetch wrapper on the OpenAI client', () => { - provider.buildClient(); - const callArg = vi.mocked(OpenAI).mock.calls[0]![0] as { - fetch?: typeof fetch; - }; - // Wrapper is always installed (so it can read live config per request); - // per-call header injection behavior is verified exhaustively in - // llm-correlation-fetch.test.ts. - expect(typeof callArg.fetch).toBe('function'); - expect(callArg.fetch).not.toBe(globalThis.fetch); - }); - - 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: } - // 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( - async () => new Response(), - ) as unknown as typeof fetch; - const mockedBuildRuntimeFetchOptions = - buildRuntimeFetchOptions as unknown as MockedFunction< - (sdkType: 'openai', proxyUrl?: string) => OpenAIRuntimeFetchOptions - >; - mockedBuildRuntimeFetchOptions.mockReturnValue({ - fetch: proxyFetch, - } as OpenAIRuntimeFetchOptions); - provider.buildClient(); - const callArg = vi.mocked(OpenAI).mock.calls[0]![0] as { - fetch?: typeof fetch; - }; - expect(typeof callArg.fetch).toBe('function'); - // 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); - }); }); describe('buildRequest', () => { diff --git a/packages/core/src/core/openaiContentGenerator/provider/default.ts b/packages/core/src/core/openaiContentGenerator/provider/default.ts index c0c27ccfa2..9ddd8d6b01 100644 --- a/packages/core/src/core/openaiContentGenerator/provider/default.ts +++ b/packages/core/src/core/openaiContentGenerator/provider/default.ts @@ -5,7 +5,6 @@ import type { ContentGeneratorConfig } from '../../contentGenerator.js'; import { DEFAULT_TIMEOUT, DEFAULT_MAX_RETRIES } from '../constants.js'; import type { OpenAICompatibleProvider } from './types.js'; import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js'; -import { wrapFetchWithCorrelation } from '../../../telemetry/llm-correlation-fetch.js'; import { tokenLimit, CAPPED_DEFAULT_MAX_TOKENS, @@ -89,13 +88,6 @@ export class DefaultOpenAICompatibleProvider 'openai', this.cliConfig.getProxy(), ); - // Wrap fetch with per-request correlation header injection. Read the base - // fetch from runtimeOptions (proxy-aware) when present, else globalThis. - // Spread runtimeOptions FIRST, then override `fetch:` so our wrapper wraps - // the proxy fetch instead of being replaced by it. See design doc §4.3. - const baseFetch = - (runtimeOptions as { fetch?: typeof fetch } | undefined)?.fetch ?? - globalThis.fetch; return new OpenAI({ apiKey, baseURL: baseUrl, @@ -103,7 +95,6 @@ export class DefaultOpenAICompatibleProvider maxRetries, defaultHeaders, ...(runtimeOptions || {}), - fetch: wrapFetchWithCorrelation(baseFetch, this.cliConfig), }); } diff --git a/packages/core/src/telemetry/config.ts b/packages/core/src/telemetry/config.ts index 5b99178b92..784bf124fb 100644 --- a/packages/core/src/telemetry/config.ts +++ b/packages/core/src/telemetry/config.ts @@ -188,7 +188,6 @@ 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 deleted file mode 100644 index b7bce39ba3..0000000000 --- a/packages/core/src/telemetry/llm-correlation-fetch.test.ts +++ /dev/null @@ -1,662 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, it, expect, vi } from 'vitest'; -import { diag } from '@opentelemetry/api'; -import type { Config } from '../config/config.js'; -import { - SESSION_ID_HEADER, - staticCorrelationHeaders, - wrapFetchWithCorrelation, -} from './llm-correlation-fetch.js'; - -// Local alias for tests; the public helper is generic. -type FetchLike = ( - input: string | URL | Request, - init?: RequestInit, -) => Promise; - -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; -} - -// Typed fetch mock returning a recorded-args view that avoids the -// `mock.calls[0]![1] as RequestInit` cast — calls is properly typed as -// `[input, init?][]` and `init` is optional. -function makeFetchMock(): { - fetch: FetchLike; - spy: ReturnType; - lastInit: () => RequestInit | undefined; - lastInput: () => string | URL | Request | undefined; - callCount: () => number; -} { - const spy = vi.fn( - async (_input: string | URL | Request, _init?: RequestInit) => - new Response(), - ); - return { - fetch: spy as unknown as FetchLike, - spy, - lastInit: () => - spy.mock.calls.length > 0 - ? (spy.mock.calls[spy.mock.calls.length - 1]?.[1] as - | RequestInit - | undefined) - : undefined, - lastInput: () => - spy.mock.calls.length > 0 - ? (spy.mock.calls[spy.mock.calls.length - 1]?.[0] as - | string - | URL - | Request - | undefined) - : undefined, - callCount: () => spy.mock.calls.length, - }; -} - -describe('wrapFetchWithCorrelation', () => { - it('attaches X-Qwen-Code-Session-Id when telemetry is enabled', async () => { - const m = makeFetchMock(); - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: 'sess-A' }), - ); - await wrapped('https://api.example.com/v1/chat'); - expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe( - 'sess-A', - ); - }); - - it('does not attach header when telemetry is disabled (passes init through unchanged)', async () => { - const m = makeFetchMock(); - const userInit = { method: 'POST', headers: { 'X-Custom': 'keep' } }; - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: false, sessionId: 'sess-A' }), - ); - await wrapped('https://api.example.com/v1/chat', userInit); - expect(m.spy).toHaveBeenCalledWith( - 'https://api.example.com/v1/chat', - userInit, - ); - }); - - it('does not attach header when sessionId is empty (skips defensively)', async () => { - const m = makeFetchMock(); - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: '' }), - ); - await wrapped('https://api.example.com/v1/chat'); - expect(m.spy).toHaveBeenCalledWith( - 'https://api.example.com/v1/chat', - undefined, - ); - }); - - it('overrides existing same-name header on init (server gets real session id, not user-supplied spoof)', async () => { - const m = makeFetchMock(); - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: 'real-sess' }), - ); - await wrapped('https://api.example.com/v1/chat', { - headers: { [SESSION_ID_HEADER]: 'spoofed' }, - }); - expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe( - 'real-sess', - ); - }); - - it('preserves other headers and init fields when injecting', async () => { - const m = makeFetchMock(); - const signal = new AbortController().signal; - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: 'sess' }), - ); - await wrapped('https://api.example.com/v1/chat', { - method: 'POST', - headers: { - Authorization: 'Bearer sk-xxx', - 'Content-Type': 'application/json', - }, - body: '{"x":1}', - signal, - }); - const init = m.lastInit()!; - expect(init.method).toBe('POST'); - expect(init.body).toBe('{"x":1}'); - expect(init.signal).toBe(signal); - const h = init.headers as Headers; - expect(h.get('Authorization')).toBe('Bearer sk-xxx'); - expect(h.get('Content-Type')).toBe('application/json'); - expect(h.get(SESSION_ID_HEADER)).toBe('sess'); - }); - - it('reads fresh session id after a session reset (staleness regression — design §4.3 critical)', async () => { - let current = 'sess-A'; - const m = makeFetchMock(); - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: () => current }), - ); - - await wrapped('https://api.example.com/1'); - expect( - ( - (m.spy.mock.calls[0]?.[1] as RequestInit | undefined)?.headers as - | Headers - | undefined - )?.get(SESSION_ID_HEADER), - ).toBe('sess-A'); - - // Simulate /clear updating Config.sessionId without recreating SDK clients. - current = 'sess-B'; - - await wrapped('https://api.example.com/2'); - expect( - ( - (m.spy.mock.calls[1]?.[1] as RequestInit | undefined)?.headers as - | Headers - | undefined - )?.get(SESSION_ID_HEADER), - ).toBe('sess-B'); - }); - - it('propagates baseFetch rejection unchanged', async () => { - const err = new Error('network unreachable'); - const spy = vi.fn(async () => { - throw err; - }); - const wrapped = wrapFetchWithCorrelation( - spy as unknown as FetchLike, - mockConfig({ enabled: true, sessionId: 'sess' }), - ); - await expect(wrapped('https://api.example.com/x')).rejects.toBe(err); - }); - - it('preserves Request input headers when init is undefined (defends Authorization etc.)', async () => { - // PR #4393 review feedback: previously `new Headers(init?.headers)` with - // undefined init dropped the Request's own headers (e.g. Authorization) - // because we then passed `{...init, headers}` which had only our session - // header. Fix seeds from input.headers when input is a Request. - const m = makeFetchMock(); - const req = new Request('https://api.example.com/v1/chat', { - method: 'POST', - headers: { - Authorization: 'Bearer sk-xxx', - 'Content-Type': 'application/json', - }, - }); - const wrapped = wrapFetchWithCorrelation( - m.fetch, - mockConfig({ enabled: true, sessionId: 'sess' }), - ); - await wrapped(req); - const init = m.lastInit()!; - const h = init.headers as Headers; - expect(h.get('Authorization')).toBe('Bearer sk-xxx'); - expect(h.get('Content-Type')).toBe('application/json'); - expect(h.get(SESSION_ID_HEADER)).toBe('sess'); - }); - - it('falls through to baseFetch + diag.warn when header construction throws (telemetry never breaks LLM)', async () => { - 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. 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' }; - const res = await wrapped('https://api.example.com/v1/chat', userInit); - expect(res).toBeInstanceOf(Response); - // baseFetch was called with the ORIGINAL init (no correlation header added) - expect(m.spy).toHaveBeenCalledWith( - 'https://api.example.com/v1/chat', - userInit, - ); - expect(warnSpy).toHaveBeenCalledWith( - expect.stringContaining('correlation header'), - ); - warnSpy.mockRestore(); - }); -}); - -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' }); - }); - - it('returns {} when telemetry disabled', () => { - expect( - staticCorrelationHeaders( - mockConfig({ enabled: false, sessionId: 'sess-A' }), - TRUSTED, - ), - ).toEqual({}); - }); - - it('returns {} when sessionId is empty', () => { - expect( - 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({}); - }); - - it('captures session id at call time — caller responsible for re-invoking on reset (Gemini staleness §8.6)', () => { - // Document by behavior: this helper takes a snapshot. Caller (e.g. - // geminiContentGenerator/index.ts factory) calls it once and bakes the - // value into SDK-construction `httpOptions.headers`. A session reset - // after construction won't update the header — known limitation. - let current = 'sess-A'; - const config = mockConfig({ enabled: true, sessionId: () => current }); - const snapshot = staticCorrelationHeaders(config, TRUSTED); - current = 'sess-B'; - 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 - // here would propagate up and crash content-generator init for the - // whole session. PR #4390 review feedback (wenshao). - const warnSpy = vi.spyOn(diag, 'warn').mockImplementation(() => {}); - const exploding = { - getTelemetryEnabled: () => { - throw new Error('config exploded'); - }, - getSessionId: () => 'unreached', - getTelemetrySessionIdHeaderHosts: () => ['*'], - } as unknown as Config; - 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('tolerates whitespace around "*" so [" * "] still enables broadcast', async () => { - // Defensive: settings.json hand-edits are easy to get a stray space - // wrong. Without trim() the operator would silently get "no host - // matches" instead of broadcast. - 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('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( - 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 deleted file mode 100644 index 6cf5dae1ff..0000000000 --- a/packages/core/src/telemetry/llm-correlation-fetch.ts +++ /dev/null @@ -1,247 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -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 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'; - -/** - * Loose fetch-like signature the wrapper internally programs against. - * - * Exists because the SDKs we wrap have **incompatible** Fetch types: - * - `openai@5.11`: `(input: string | URL | Request, init?) => Promise` - * - `@anthropic-ai/sdk`: `(input: RequestInfo, init?) => Promise` - * where `RequestInfo = string | Request` (NOT including URL). - * - * No single concrete signature satisfies both as a structural subtype, so the - * public `wrapFetchWithCorrelation` is generic and preserves the caller's - * exact type (the SDK's own Fetch). This type only describes the input shape - * we touch inside the wrapper. - */ -type FetchLikeLoose = ( - input: string | URL | Request, - init?: RequestInit, -) => Promise; - -/** - * 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 - * NOT recreated on `/clear`-triggered session reset (`Config.resetSession()` - * updates `this.sessionId` but doesn't rebuild the contentGenerator). A - * static header would therefore go stale immediately after the first reset. - * Reading `config.getSessionId()` from inside the wrapper on every call - * gives the live value. - * - * Note on `trustedHosts`: snapshotted once at wrap time, not read per - * request. The session id is live-read but the allowlist is not — a - * mid-session change to `telemetry.sessionIdHeaderHosts` in settings.json - * takes effect at next content-generator init (any change that mutates - * Config snapshot the wrapper retains is by definition a Config-level - * concern, not a request-time concern). Operators tuning the scope live - * should restart, or call the openai/anthropic clients via a fresh - * provider after settings reload. - * - * The caller is responsible for choosing the base fetch — usually - * `runtimeOptions?.fetch ?? globalThis.fetch` so proxy-aware fetch (set up - * by `buildRuntimeFetchOptions`) is preserved when ProxyAgent is in use. - * - * 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., - * we still want the model call to proceed. - */ -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 normalization (PR #4390 review feedback): qwen-code's - // 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 { - 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). - // Pre-trimmed above, so `.includes('*')` covers `["*"]` / `[" * "]` / - // `["\t*\n"]` uniformly. - const broadcastAll = trustedHosts.includes('*'); - - const wrapped: FetchLikeLoose = async function correlationFetch( - input: string | URL | Request, - init?: RequestInit, - ): Promise { - let headers: Headers; - try { - 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. - // Skip injection rather than send `X-Qwen-Code-Session-Id: `. - return baseFetch(input, init); - } - // Seed headers: prefer init.headers when caller provided them (their - // intent overrides). Otherwise, if input is a Request, copy its own - // headers so they aren't lost when we pass `{...init, headers}` back - // to baseFetch — `new Headers(undefined)` would otherwise produce an - // empty Headers and drop the Request's headers (including - // Authorization). See PR #4393 review feedback. - headers = new Headers(init?.headers); - if (init?.headers === undefined && input instanceof Request) { - input.headers.forEach((value, key) => headers.set(key, value)); - } - headers.set(SESSION_ID_HEADER, sid); - } catch (err) { - // Telemetry must never break the LLM request path. Log and fall through. - diag.warn( - `wrapFetchWithCorrelation: header construction failed, sending request without correlation header: ${err instanceof Error ? err.message : String(err)}`, - ); - return baseFetch(input, init); - } - return baseFetch(input, { ...init, headers }); - }; - // Cast back to TFetch: runtime behavior matches whatever signature the - // caller's baseFetch has (we delegate to it without altering shape). - return wrapped as unknown as TFetch; -} - -/** - * Static correlation headers. Captures the session id at call time — - * subject to staleness if the host SDK keeps these headers in a - * captured-at-construction slot (e.g. `@google/genai`'s - * `httpOptions.headers` — see design doc §8.6 for the known Gemini - * limitation). Prefer `wrapFetchWithCorrelation` whenever the SDK exposes - * a `fetch` hook. - * - * 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 - // content-generator factory at construction time — a throw here would - // propagate up and crash content-generator init for the entire session. - // Fall through to `{}` instead. See PR #4390 review feedback (wenshao). - try { - if (!config.getTelemetryEnabled()) return {}; - if (!destinationUrl) return {}; - // 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 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 { - 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 }; - } catch (err) { - diag.warn( - `staticCorrelationHeaders: config read failed, omitting correlation header: ${err instanceof Error ? err.message : String(err)}`, - ); - return {}; - } -} diff --git a/packages/core/src/telemetry/sdk.test.ts b/packages/core/src/telemetry/sdk.test.ts index 5d1a66127d..dc3155fe5e 100644 --- a/packages/core/src/telemetry/sdk.test.ts +++ b/packages/core/src/telemetry/sdk.test.ts @@ -147,6 +147,7 @@ describe('Telemetry SDK', () => { getDebugMode: () => false, getSessionId: () => 'test-session', getCliVersion: () => '1.0.0-test', + getOutboundCorrelationPropagateTraceContext: () => false, } as unknown as Config; }); @@ -640,6 +641,49 @@ describe('Telemetry SDK', () => { }); }); + describe('Outbound trace-context propagation gate', () => { + function getTextMapPropagator(): unknown { + const constructorCall = vi.mocked(NodeSDK).mock.calls[0]![0]!; + return (constructorCall as { textMapPropagator?: unknown }) + .textMapPropagator; + } + + it('installs a no-op TextMapPropagator by default (propagateTraceContext=false)', () => { + // Default behavior per PR #4390 R4 split: traceparent is NOT written + // onto outbound wire. The propagator's inject() must be a no-op so + // UndiciInstrumentation's `propagation.inject(carrier)` call writes + // nothing into the outgoing request's headers. + initializeTelemetry(mockConfig); + const propagator = getTextMapPropagator() as + | { inject: (...args: unknown[]) => void; fields: () => string[] } + | undefined; + expect(propagator).toBeDefined(); + expect(typeof propagator!.inject).toBe('function'); + // Sanity: fields() returns empty array → instrumentation knows there + // are no headers to clear / no propagator state. + expect(propagator!.fields()).toEqual([]); + // inject is a no-op — does not throw, does not mutate the carrier. + const carrier: Record = { existing: 'h' }; + expect(() => + propagator!.inject({} as never, carrier, {} as never), + ).not.toThrow(); + expect(carrier).toEqual({ existing: 'h' }); + }); + + it('uses the SDK default propagator when propagateTraceContext=true (operator opt-in)', () => { + vi.spyOn( + mockConfig, + 'getOutboundCorrelationPropagateTraceContext', + ).mockReturnValue(true); + initializeTelemetry(mockConfig); + // textMapPropagator is omitted from NodeSDK options → SDK installs + // its default W3C composite propagator. Test asserts the absence + // because the actual W3CTraceContextPropagator instance is created + // inside @opentelemetry/sdk-node which is auto-mocked here. + expect(getTextMapPropagator()).toBeUndefined(); + }); + }); + describe('Instrumentations', () => { function getInstrumentations(): unknown[] { const constructorCall = vi.mocked(NodeSDK).mock.calls[0]![0]!; @@ -1115,6 +1159,7 @@ describe('refreshSessionContext', () => { getDebugMode: () => false, getSessionId: () => 'test-session', getCliVersion: () => '1.0.0-test', + getOutboundCorrelationPropagateTraceContext: () => false, } as unknown as Config; }); diff --git a/packages/core/src/telemetry/sdk.ts b/packages/core/src/telemetry/sdk.ts index 7d9c3a272b..20b2a8ecf7 100644 --- a/packages/core/src/telemetry/sdk.ts +++ b/packages/core/src/telemetry/sdk.ts @@ -5,7 +5,11 @@ */ import { DiagLogLevel, diag } from '@opentelemetry/api'; -import type { DiagLogger } from '@opentelemetry/api'; +import type { + Context, + DiagLogger, + TextMapPropagator, +} from '@opentelemetry/api'; import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc'; import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-grpc'; import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-grpc'; @@ -90,6 +94,29 @@ export function resolveHttpOtlpUrl( // (2s) and overall (5s) timeouts, so this value is effectively unreachable there. const SHUTDOWN_TIMEOUT_MS = 10_000; +/** + * `TextMapPropagator` that emits nothing. Installed when + * `outboundCorrelation.propagateTraceContext` is false (the default), so + * trace context stays internal to the user's OTLP collector and is not + * written into outbound `fetch` requests to third-party LLM providers. + * + * UndiciInstrumentation still creates client HTTP spans — the propagator + * only governs whether `propagation.inject()` writes `traceparent` into + * the outgoing request's header carrier. With this propagator installed, + * inject is a no-op and outbound requests carry no trace headers. PR + * #4390 review (LaZzyMan): split outbound-wire behavior out of telemetry + * default-on. + */ +const NOOP_PROPAGATOR: TextMapPropagator = { + inject() {}, + extract(context: Context): Context { + return context; + }, + fields(): string[] { + return []; + }, +}; + let sdk: NodeSDK | undefined; let telemetryInitialized = false; let telemetryShutdownPromise: Promise | undefined; @@ -397,12 +424,26 @@ export function initializeTelemetry(config: Config): void { return path.slice(0, cut); }; + // Outbound trace-context propagation gate (PR #4390 review, LaZzyMan): + // by default, install a no-op propagator so `traceparent` does NOT get + // written onto outbound `fetch` requests to LLM providers. Operators + // who want server-side trace stitching (e.g. ARMS+DashScope) opt in via + // `outboundCorrelation.propagateTraceContext: true`, which leaves the + // SDK's default W3C composite propagator in place. UndiciInstrumentation + // still creates client HTTP spans either way — the propagator only + // governs whether trace ids leak onto third-party request streams. + const textMapPropagator: TextMapPropagator | undefined = + config.getOutboundCorrelationPropagateTraceContext() + ? undefined // undefined → NodeSDK keeps its default W3C propagator + : NOOP_PROPAGATOR; + sdk = new NodeSDK({ resource, // Disable async host/process/env resource detectors: they leave attributes // pending and trigger an OTel diag.error on any resource attribute read // before the detectors settle (e.g. during HttpInstrumentation span creation). autoDetectResources: false, + ...(textMapPropagator && { textMapPropagator }), spanProcessors: spanExporter ? [new BatchSpanProcessor(spanExporter)] : [], logRecordProcessors: logExporter ? [new BatchLogRecordProcessor(logExporter)] diff --git a/packages/core/src/telemetry/trusted-llm-hosts.test.ts b/packages/core/src/telemetry/trusted-llm-hosts.test.ts deleted file mode 100644 index 06fb8dd15d..0000000000 --- a/packages/core/src/telemetry/trusted-llm-hosts.test.ts +++ /dev/null @@ -1,181 +0,0 @@ -/** - * @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('strips explicit port (URL.hostname does not include port)', () => { - expect(extractRequestHost('https://dashscope.aliyuncs.com:443/v1')).toBe( - 'dashscope.aliyuncs.com', - ); - }); - - it('strips userinfo (URL.hostname does not include user/password)', () => { - expect( - extractRequestHost('https://user:pw@dashscope.aliyuncs.com/v1'), - ).toBe('dashscope.aliyuncs.com'); - }); - - it('ignores query string and fragment', () => { - expect( - extractRequestHost('https://dashscope.aliyuncs.com/v1?key=secret#frag'), - ).toBe('dashscope.aliyuncs.com'); - }); - - it('returns bracketed form for IPv6 (will never match a string pattern, behaves as fail-closed)', () => { - // Document the IPv6 behavior so it's intentional rather than a - // surprise: URL.hostname returns `[::1]` (with brackets), our - // pattern grammar has no `[…]` form, so IPv6 destinations are - // effectively never on the allowlist. This is acceptable because - // qwen-code's allowlist is for named first-party endpoints, not - // raw IPs. If/when raw-IP correlation becomes a real ask, extend - // matchesTrustedHost — don't change extractRequestHost. - expect(extractRequestHost('http://[::1]:8080/x')).toBe('[::1]'); - }); - - 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 deleted file mode 100644 index 540dc1474d..0000000000 --- a/packages/core/src/telemetry/trusted-llm-hosts.ts +++ /dev/null @@ -1,89 +0,0 @@ -/** - * @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; -} diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 276cb8b3d8..c86fb1b4ed 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -435,18 +435,23 @@ "default": false } } - }, - "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, "description": "Telemetry configuration." }, + "outboundCorrelation": { + "type": "object", + "properties": { + "propagateTraceContext": { + "description": "Inject W3C `traceparent` header on outbound `fetch` requests (LLM SDK calls, MCP StreamableHTTP, WebFetch, ...). Default: false — trace context stays internal to the operator's OTLP collector and is NOT written onto third-party request streams. Set true only when you want cross-process trace stitching with an OTel-aware LLM provider (e.g. ARMS+DashScope). Note: client HTTP spans are still emitted in either case; this flag only governs the wire `traceparent` header.", + "type": "boolean", + "default": false + } + }, + "additionalProperties": false, + "description": "SECURITY-RELEVANT. Controls what client-side correlation data qwen-code writes into outbound LLM API requests (DashScope, OpenAI, Anthropic, etc.) — separate from `telemetry.*` which governs data flow into the operator's OWN OTLP collector. All values default to off. Opt in only when the LLM provider also reports into your OTel collector for cross-process trace stitching (e.g. ARMS Tracing + DashScope)." + }, "fastModel": { "description": "Model used for generating prompt suggestions and speculative execution. Leave empty to use the main model. A smaller/faster model (e.g., qwen3-coder-flash) reduces latency and cost.", "type": "string",