diff --git a/Cargo.lock b/Cargo.lock index d32c707..9f42023 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -452,6 +452,7 @@ dependencies = [ "clap_complete", "cli-engine-macros", "inquire", + "is-ai-agent", "jmespath", "jsonwebtoken", "keyring", @@ -1366,6 +1367,12 @@ version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" +[[package]] +name = "is-ai-agent" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce5562708736fdf9e8dcc50719e342b0ac0c3d1e365ed76bc0762d5a9e36094a" + [[package]] name = "is-docker" version = "0.2.0" diff --git a/cli-engine/Cargo.toml b/cli-engine/Cargo.toml index 0e8c24c..8329afc 100644 --- a/cli-engine/Cargo.toml +++ b/cli-engine/Cargo.toml @@ -22,12 +22,13 @@ jsonwebtoken = { version = "11", optional = true, default-features = false } keyring = { version = "3.6.1", optional = true, default-features = false } open = { version = "5.4.1", optional = true } rand = { version = "0.9", optional = true } -sha2 = { version = "0.10.9", optional = true } +sha2 = "0.10.9" url = { version = "2.5.4", optional = true } zeroize = { version = "1.8", optional = true, features = ["derive"] } chrono = { version = "0.4.42", default-features = false, features = ["clock", "serde"] } clap = { version = "4.5.53", features = ["derive", "std", "string"] } clap_complete = "4" +is-ai-agent = "0.6" jmespath = "0.5.0" reqwest = { version = "0.13", default-features = false, features = ["json", "multipart", "form", "rustls"] } regex = "1.12.2" @@ -53,7 +54,7 @@ keyring = { version = "3.6.1", optional = true, default-features = false, featur keyring = { version = "3.6.1", optional = true, default-features = false, features = ["windows-native"] } [features] -pkce-auth = ["dep:jsonwebtoken", "dep:keyring", "dep:open", "dep:rand", "dep:sha2", "dep:url", "dep:zeroize"] +pkce-auth = ["dep:jsonwebtoken", "dep:keyring", "dep:open", "dep:rand", "dep:url", "dep:zeroize"] [dev-dependencies] pretty_assertions = "1.4.1" diff --git a/cli-engine/docs/attribution.md b/cli-engine/docs/attribution.md new file mode 100644 index 0000000..b066841 --- /dev/null +++ b/cli-engine/docs/attribution.md @@ -0,0 +1,52 @@ +# Client attribution + +Client attribution lets a CLI built on cli-engine tell the services it calls what kind of caller it is (a person at a terminal, a CI job, a script, or an AI coding agent) so usage can be understood without any telemetry channel. It adds a few tokens to the `User-Agent` header and, when an AI harness exposes a session id, one correlation header. It sends nothing to any endpoint the CLI was not already calling, writes nothing to disk, and never contacts a collector. + +It is off by default. A CLI opts in with `CliConfig::with_client_attribution`. + +## What is sent + +| Where | Value | When | +| --- | --- | --- | +| `User-Agent` | `/ mode/` | Always, once enabled | +| `User-Agent` | `agent/` (for example `agent/claude-code`) | A known AI harness is detected | +| `x-client-session` (name configurable) | 16 hex characters: a salted SHA-256 prefix of the harness session id | The harness exposes a session id, and the user has not opted out | + +`` is the first that applies: `agent` (a harness marker is present), `ci` (the `CI` variable is set to a non-blank value other than `0`, `false`, `no`, or `off`, compared case-insensitively), `interactive` (stdin and stderr are terminals), or `script` (none of the above). + +An example, from a Claude Code session: `User-Agent: gddy/1.4.0 mode/agent agent/claude-code` and `x-gddy-session: 3f9a0c51d27be8a4`. + +## What is not sent + +The raw session id never leaves the process. Harness ids are often meaningful on the user's machine (transcript file names, resume handles), so only a one-way hash is sent. The hash is salted with the CLI's app id and the agent slug, so the same session hashes differently in different CLIs and cannot be joined across unrelated products. + +No persistent identifier is created. For a person at a terminal, or any caller without a harness session id, no session header is sent at all. Nothing is stored between runs. + +## Opting out + +Users can drop the session header with `_NO_SESSION_ID=1`, where `` is the CLI's app id uppercased with non-alphanumerics replaced by `_` (`gddy` becomes `GDDY_NO_SESSION_ID`). The user-agent tokens are not affected by this variable: they describe the kind of caller in the same way the binary name and version already do. CLI authors can also disable the header entirely with `AttributionConfig::without_session_id`. + +## Seeing exactly what goes out + +Run any command with `--debug=transport` to print each outbound request's headers to stderr, including the user-agent and the session header. + +## How detection works, and its limits + +Detection reads environment variables that AI harnesses publish to the processes they launch, using the [`is-ai-agent`](https://github.com/sdairs/is-ai-agent) crate. It also checks whether a small fixed set of marker paths defined by that crate exists (currently only `/opt/.devin`, which identifies Devin). Those checks test existence only: no file contents are read and no directories are listed. It is cooperative and heuristic. A match does not prove a model issued this particular command (a human can run commands in a terminal a harness opened), and no match does not prove a human did. Treat the result as attribution to a harness, not as proof of intent. Session ids have different scopes per harness (a conversation, a thread, a single run), so the hash correlates calls within one harness only. + +## Enabling it (CLI authors) + +```rust +use cli_engine::{BuildInfo, CliConfig}; +use cli_engine::transport::AttributionConfig; + +let config = CliConfig::new("my-cli", "Team CLI", "my-cli") + .with_build(BuildInfo::new(env!("CARGO_PKG_VERSION"))) + .with_client_attribution( + AttributionConfig::new().with_session_header("x-my-cli-session"), + ); +``` + +The engine resolves attribution once per execution, before any command runs, and publishes it process-wide. Publishing happens in the `execute*` entrypoints, after argv0 resolution, so an argv0 personality publishes its own identity and not the dispatcher's. `Cli::run` deliberately does not publish (so running a `Cli` in tests never mutates process-wide state); a harness that drives `Cli::run` and needs outbound requests to carry the identity must publish it itself. The user-agent and default headers are published and read together, so a client never sees one without the other. It is applied to every `HttpClient` and to every client built from `transport::reqwest_client_builder()` (the entry point for generated or hand-rolled `reqwest` clients), **provided the client is created after publication**. Both capture the process identity at the moment they are created, so build clients inside command handlers (which run after `execute*` has published), not during module registration or `Cli::new`, which runs earlier. A client created too early keeps the identity that was current then and silently omits attribution. The user-agent tokens also reach the engine's own OAuth token requests; the session header does not. + +Because the headers are process-wide defaults, they are sent to whatever host those clients call. Build clients that talk to third-party hosts from a plain `reqwest::Client` if the session header should not reach them. diff --git a/cli-engine/docs/concepts.md b/cli-engine/docs/concepts.md index c8a52ea..d2e0330 100644 --- a/cli-engine/docs/concepts.md +++ b/cli-engine/docs/concepts.md @@ -783,6 +783,10 @@ The transport module provides a `reqwest`-based HTTP client with: Auth injectors include bearer token, provider bearer, cookie, basic auth, API key, client credentials, and no-op injectors. +Code that needs a plain `reqwest::Client` (progenitor-generated clients, hand-rolled streaming or multipart uploads) builds it from `transport::reqwest_client_builder()` instead of `reqwest::Client::builder()`. The builder comes preconfigured with the process-wide user-agent and default headers, a connect timeout, and an idle read timeout, so outbound policy is defined once in the engine. Callers layer their own settings on top. + +A CLI can opt into client attribution (user-agent tokens for the kind of caller, and a hashed harness session id) with `CliConfig::with_client_attribution`. See [attribution.md](attribution.md) for exactly what is sent, how to opt out, and the limits of detection. + ### HTTP debug logging The global `--debug` flag drives transport diagnostics through the `transport` component. Bare `--debug` enables every component; to select one, use the `=` form so the value is not mistaken for the command: `--debug=transport`, or `--debug='*,-transport'` to keep everything else but silence HTTP. (As an optional-value global flag, `--debug` only attaches a space-separated value when it appears after the leaf command; before the command, write `--debug=transport`.) `flags::debug_component_enabled` parses the comma-separated pattern. diff --git a/cli-engine/src/cli/argv0.rs b/cli-engine/src/cli/argv0.rs index ab39327..59fb89d 100644 --- a/cli-engine/src/cli/argv0.rs +++ b/cli-engine/src/cli/argv0.rs @@ -89,8 +89,15 @@ pub(super) enum Argv0Outcome { /// with a fully rendered result when a personality ran or an explicit `argv0` /// invocation was rejected. When no routes are registered this is inert and /// returns the arguments unchanged. `depth` counts chained hand-offs and -/// bounds recursion via [`MAX_ARGV0_DEPTH`]. -pub(super) async fn resolve_argv0(cli: &Cli, text_args: Vec, depth: usize) -> Argv0Outcome { +/// bounds recursion via [`MAX_ARGV0_DEPTH`]. `publish_identity` is forwarded to a +/// personality's own run so the CLI that actually executes publishes its +/// outbound identity, not the dispatcher's. +pub(super) async fn resolve_argv0( + cli: &Cli, + text_args: Vec, + depth: usize, + publish_identity: bool, +) -> Argv0Outcome { if cli.config.argv0_routes.is_empty() { return Argv0Outcome::Proceed(text_args); } @@ -162,7 +169,13 @@ pub(super) async fn resolve_argv0(cli: &Cli, text_args: Vec, depth: usiz alt_args.push(bin); alt_args.extend(rest); Argv0Outcome::Handled( - Box::pin(super::run::run_with_depth(&alt, alt_args, depth + 1)).await, + Box::pin(super::run::run_with_depth( + &alt, + alt_args, + depth + 1, + publish_identity, + )) + .await, ) } None if explicit => Argv0Outcome::Handled(render_argv0_error( diff --git a/cli-engine/src/cli/config.rs b/cli-engine/src/cli/config.rs index 89e96f4..28eebb2 100644 --- a/cli-engine/src/cli/config.rs +++ b/cli-engine/src/cli/config.rs @@ -134,6 +134,9 @@ pub struct CliConfig { /// the engine derives `name/version` from this config. See /// [`CliConfig::user_agent_string`]. pub user_agent: Option, + /// Opt-in client attribution (user-agent tokens and a hashed session + /// header). See [`CliConfig::with_client_attribution`]. + pub attribution: Option, /// Extra HTTP header names to redact in `--debug transport` output, on top /// of the built-in sensitive set (`authorization`, `proxy-authorization`, /// `cookie`, `set-cookie`, `x-api-key`). Set CLI-specific secret-bearing @@ -389,6 +392,24 @@ impl CliConfig { self } + /// Opts this CLI into client attribution. + /// + /// On execution the engine appends `mode/` + /// (and `agent/` for a detected AI harness) to the outbound + /// User-Agent, and, when the harness exposes a session id, sends a salted + /// hash of it in a correlation header. Nothing is sent to any endpoint the + /// CLI was not already calling. The identity is captured when each client is + /// created, so create clients inside command handlers, not during module + /// registration. See `docs/attribution.md`. + #[must_use] + pub fn with_client_attribution( + mut self, + attribution: crate::transport::AttributionConfig, + ) -> Self { + self.attribution = Some(attribution); + self + } + /// Adds HTTP header names to redact in `--debug transport` output, on top of /// the built-in sensitive set. /// diff --git a/cli-engine/src/cli/flags_apply.rs b/cli-engine/src/cli/flags_apply.rs index 2be580c..5655b1f 100644 --- a/cli-engine/src/cli/flags_apply.rs +++ b/cli-engine/src/cli/flags_apply.rs @@ -412,6 +412,7 @@ pub(super) fn prescan_env_flag(mut args: impl Iterator) -> Option mod user_agent_tests { use super::*; use crate::cli::{BuildInfo, Cli, CliConfig}; + use crate::transport::Signals; #[test] fn user_agent_string_derives_name_and_version_by_default() { @@ -435,7 +436,7 @@ mod user_agent_tests { } #[test] - fn install_default_user_agent_publishes_config_value() { + fn install_client_identity_publishes_config_value() { let _guard = crate::transport::client::UA_TEST_LOCK .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -444,13 +445,66 @@ mod user_agent_tests { let cli = Cli::new( CliConfig::new("uatest", "UA test", "uatest").with_build(BuildInfo::new("4.5.6")), ); - cli.install_default_user_agent(); + cli.install_client_identity(); assert_eq!( crate::transport::client::default_user_agent(), "uatest/4.5.6" ); } + fn harness_signals(app_id: &str) -> Signals { + Signals::from_lookup( + app_id, + |name| match name { + "CLAUDECODE" => Some("1".to_owned()), + "CLAUDE_CODE_SESSION_ID" => Some("session-abc".to_owned()), + _ => None, + }, + |_| false, + false, + ) + } + + #[test] + fn client_identity_without_attribution_is_the_plain_user_agent() { + let cli = Cli::new( + CliConfig::new("attr", "Attr test", "attr").with_build(BuildInfo::new("1.0.0")), + ); + + let (user_agent, headers) = cli.client_identity(&harness_signals("attr")); + + assert_eq!(user_agent, "attr/1.0.0"); + assert!(headers.is_empty()); + } + + #[test] + fn client_identity_with_attribution_extends_the_base_user_agent() { + let cli = Cli::new( + CliConfig::new("attr", "Attr test", "attr") + .with_build(BuildInfo::new("1.0.0")) + .with_client_attribution(crate::transport::AttributionConfig::new()), + ); + + let (user_agent, headers) = cli.client_identity(&harness_signals("attr")); + + assert_eq!(user_agent, "attr/1.0.0 mode/agent agent/claude-code"); + assert_eq!(headers.len(), 1); + assert!(headers.contains_key("x-client-session")); + } + + #[test] + fn client_identity_extends_an_explicit_user_agent_override() { + let cli = Cli::new( + CliConfig::new("attr", "Attr test", "attr") + .with_user_agent("custom/9") + .with_client_attribution(crate::transport::AttributionConfig::new()), + ); + + let (user_agent, _) = cli.client_identity(&harness_signals("attr")); + + assert!(user_agent.starts_with("custom/9 mode/agent")); + } + #[test] fn install_debug_transport_logger_tracks_the_debug_pattern() { // Asserts on `debug_transport_logger_for`'s decision directly rather diff --git a/cli-engine/src/cli/mod.rs b/cli-engine/src/cli/mod.rs index 5f6b40c..627ce36 100644 --- a/cli-engine/src/cli/mod.rs +++ b/cli-engine/src/cli/mod.rs @@ -33,6 +33,7 @@ use crate::{ flags::{register_global_flags, register_reason_flag}, module::ModuleContext, output::{global_human_view_registry_snapshot, global_schema_registry_snapshot}, + transport::{Attribution, Signals}, }; pub use argv0::{Argv0LinkMethod, Argv0Route}; @@ -402,15 +403,31 @@ impl Cli { run::execute_from_until_signal(self, args, stdout, stderr, shutdown).await } - /// Publishes the configured outbound User-Agent process-wide so that - /// command [`HttpClient`](crate::transport::HttpClient)s and the engine's - /// own OAuth token requests share it. + /// Publishes the configured outbound identity process-wide so that + /// command [`HttpClient`](crate::transport::HttpClient)s, clients from + /// [`reqwest_client_builder`](crate::transport::reqwest_client_builder), + /// and the engine's own OAuth token requests share it. /// /// Called from the execution entrypoints rather than [`Cli::new`] so that /// merely constructing a `Cli` (as tests do in bulk) does not mutate global /// state. See [`CliConfig::user_agent_string`] for resolution order. - fn install_default_user_agent(&self) { - crate::transport::set_default_user_agent(self.config.user_agent_string()); + fn install_client_identity(&self) { + let (user_agent, headers) = + self.client_identity(&Signals::from_process(&self.config.app_id)); + crate::transport::client::set_client_identity(user_agent, headers); + } + + /// Computes the outbound User-Agent and default headers for `signals`: + /// the configured base user-agent, plus attribution tokens and headers + /// when the CLI opted in. + fn client_identity(&self, signals: &Signals) -> (String, BTreeMap) { + let mut user_agent = self.config.user_agent_string(); + let Some(config) = &self.config.attribution else { + return (user_agent, BTreeMap::new()); + }; + let attribution = Attribution::resolve(config, &self.config.app_id, signals); + user_agent.push_str(&attribution.user_agent_suffix); + (user_agent, attribution.headers) } /// Registers an auth provider after construction. @@ -561,11 +578,17 @@ impl Cli { /// /// Same `--env`/tree-pruning caveat as [`Cli::execute_from`]: see /// [`CliConfig::with_startup_args`]. + /// + /// Unlike the `execute*` entrypoints, this does not publish the process-wide + /// outbound identity (user-agent, default headers, client attribution), so + /// merely running a `Cli` never mutates global state. Use an `execute*` + /// entrypoint, or `transport::set_default_user_agent`, when outbound + /// requests must carry the configured identity. pub async fn run(&self, args: I) -> CliRunOutput where I: IntoIterator, S: Into + Clone, { - run::run_with_depth(self, args, 0).await + run::run_with_depth(self, args, 0, false).await } } diff --git a/cli-engine/src/cli/run.rs b/cli-engine/src/cli/run.rs index 71048cb..d2e1a9d 100644 --- a/cli-engine/src/cli/run.rs +++ b/cli-engine/src/cli/run.rs @@ -87,8 +87,7 @@ where E: Write, Shutdown: Future, { - cli.install_default_user_agent(); - let output = run_until_signal(cli.run(args), shutdown).await; + let output = run_until_signal(run_with_depth(cli, args, 0, true), shutdown).await; if output.exit_code == 130 && output.rendered == "command interrupted\n" && let Some(on_shutdown) = &cli.on_shutdown @@ -105,7 +104,17 @@ where /// Runs the CLI like [`Cli::run`](super::Cli::run), threading the `argv0` dispatch recursion /// `depth` so a chain of personality hand-offs is bounded by [`MAX_ARGV0_DEPTH`](super::argv0::MAX_ARGV0_DEPTH). -pub(super) async fn run_with_depth(cli: &Cli, args: I, depth: usize) -> CliRunOutput +/// `publish_identity` installs the outbound identity process-wide once it is +/// known which CLI will actually run (after argv0 resolution), so a personality +/// hand-off publishes the personality's identity. Only the `execute*` +/// entrypoints set it: plain [`Cli::run`](super::Cli::run) must not mutate +/// process globals, since tests call it concurrently. +pub(super) async fn run_with_depth( + cli: &Cli, + args: I, + depth: usize, + publish_identity: bool, +) -> CliRunOutput where I: IntoIterator, S: Into + Clone, @@ -118,10 +127,14 @@ where .iter() .map(|arg| arg.to_string_lossy().into_owned()) .collect::>(); - let text_args = match super::argv0::resolve_argv0(cli, text_args, depth).await { + let text_args = match super::argv0::resolve_argv0(cli, text_args, depth, publish_identity).await + { Argv0Outcome::Handled(output) => return output, Argv0Outcome::Proceed(args) => args, }; + if publish_identity { + cli.install_client_identity(); + } let mut clap_args = normalize_optional_global_flags_before_command(&cli.root, &text_args); if has_root_version_flag(&text_args, &cli.root, &cli.config.name) { return finish_run( @@ -413,7 +426,8 @@ where &bool_flags, &value_flags, ); - return Box::pin(run_with_depth(cli, augmented, depth + 1)).await; + // Same CLI, identity already published above. + return Box::pin(run_with_depth(cli, augmented, depth + 1, false)).await; } return finish_run( cli, diff --git a/cli-engine/src/transport/attribution.rs b/cli-engine/src/transport/attribution.rs new file mode 100644 index 0000000..a14b8a5 --- /dev/null +++ b/cli-engine/src/transport/attribution.rs @@ -0,0 +1,233 @@ +//! Client attribution: how a CLI tells the services it calls what kind of +//! caller it is, without any telemetry channel of its own. +//! +//! Everything here rides on requests the user already asked for: +//! +//! - **User-Agent tokens** — `mode/` and, for a +//! detected AI harness, `agent/`. +//! - **A correlation header** — when the harness exposes a session id, a salted +//! hash of it (never the raw id) so a service can group one session's calls. +//! +//! Detection is cooperative and heuristic: it reads environment markers that +//! harnesses publish to their subprocesses, and checks whether a small fixed +//! set of marker paths defined by the detector exists (currently only +//! `/opt/.devin`). It only tests existence; it never reads file contents or +//! lists directories. A match does not prove a model issued the command, and no +//! match does not prove a human did. + +use std::{collections::BTreeMap, path::Path}; + +use sha2::{Digest, Sha256}; + +use crate::flags::{app_id_env_prefix, detect_interactive}; + +const DEFAULT_SESSION_HEADER: &str = "x-client-session"; +const SESSION_HASH_BYTES: usize = 8; + +/// Opts a CLI into client attribution: `mode/` +/// and `agent/` tokens on the User-Agent, plus a correlation header +/// carrying a salted hash of the harness session id when one is available. +/// The raw session id is never sent. See `docs/attribution.md` for the full +/// user-facing description. +/// +/// Attribution is off unless a CLI calls +/// [`CliConfig::with_client_attribution`](crate::CliConfig::with_client_attribution). +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct AttributionConfig { + session_header: String, + send_session: bool, +} + +impl Default for AttributionConfig { + fn default() -> Self { + Self { + session_header: DEFAULT_SESSION_HEADER.to_owned(), + send_session: true, + } + } +} + +impl AttributionConfig { + /// Creates the default configuration: user-agent tokens plus a hashed + /// session id in the `x-client-session` header when a harness supplies one. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Sets the request header that carries the hashed session id, for services + /// that expect a product-specific name (for example `x-gddy-session`). + /// + /// An invalid header name disables the session header rather than failing + /// startup; the user-agent tokens are unaffected. + #[must_use] + pub fn with_session_header(mut self, name: impl Into) -> Self { + self.session_header = name.into().to_ascii_lowercase(); + self + } + + /// Never sends a session header, only the user-agent tokens. + #[must_use] + pub fn without_session_id(mut self) -> Self { + self.send_session = false; + self + } +} + +/// Coarse caller classification, in precedence order. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ClientMode { + /// A known AI harness marker is present. + Agent, + /// `CI` is set. + Ci, + /// A person at a terminal. + Interactive, + /// Neither: a script, cron job, or other unattended caller. + Script, +} + +impl ClientMode { + fn as_str(self) -> &'static str { + match self { + Self::Agent => "agent", + Self::Ci => "ci", + Self::Interactive => "interactive", + Self::Script => "script", + } + } +} + +/// A detected AI harness. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct AgentSignal { + slug: &'static str, + session_id: Option, +} + +/// What the process environment says about the caller. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct Signals { + agent: Option, + ci: bool, + interactive: bool, + session_opted_out: bool, +} + +impl Signals { + /// Reads the live process environment and terminal state. + pub(crate) fn from_process(app_id: &str) -> Self { + Self::from_lookup( + app_id, + |name| std::env::var(name).ok(), + |path| Path::new(path).exists(), + detect_interactive(), + ) + } + + /// Builds signals from injected lookups so tests never touch the real + /// environment. + pub(crate) fn from_lookup( + app_id: &str, + env: impl Fn(&str) -> Option, + file_exists: impl Fn(&str) -> bool, + interactive: bool, + ) -> Self { + let agent = is_ai_agent::detect_with(&env, file_exists).map(|agent| AgentSignal { + slug: agent.id.as_str(), + session_id: agent.session_id, + }); + let truthy = |name: &str| env(name).is_some_and(|value| is_truthy(&value)); + Self { + agent, + ci: truthy("CI"), + interactive, + session_opted_out: truthy(&format!("{}_NO_SESSION_ID", app_id_env_prefix(app_id))), + } + } + + fn mode(&self) -> ClientMode { + if self.agent.is_some() { + ClientMode::Agent + } else if self.ci { + ClientMode::Ci + } else if self.interactive { + ClientMode::Interactive + } else { + ClientMode::Script + } + } +} + +/// The resolved outbound identity additions. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub(crate) struct Attribution { + /// Space-prefixed product tokens to append to the base user-agent. + pub(crate) user_agent_suffix: String, + /// Headers to send on every outbound request. + pub(crate) headers: BTreeMap, +} + +impl Attribution { + pub(crate) fn resolve(config: &AttributionConfig, app_id: &str, signals: &Signals) -> Self { + let mode = signals.mode(); + let mut user_agent_suffix = format!(" mode/{}", mode.as_str()); + if let Some(agent) = &signals.agent { + user_agent_suffix.push_str(" agent/"); + user_agent_suffix.push_str(agent.slug); + } + + let mut headers = BTreeMap::new(); + if config.send_session + && !signals.session_opted_out + && is_valid_header_name(&config.session_header) + && let Some(agent) = &signals.agent + && let Some(session_id) = &agent.session_id + { + headers.insert( + config.session_header.clone(), + hash_session(app_id, agent.slug, session_id), + ); + } + Self { + user_agent_suffix, + headers, + } + } +} + +/// Salted, truncated SHA-256 of the harness session id. +/// +/// Harness ids are often meaningful on the user's machine (transcript file +/// names, resume handles), so the raw value never leaves the process. The app +/// id and agent slug salt it so the same id hashes differently per CLI and the +/// result cannot be joined across unrelated products. +fn hash_session(app_id: &str, agent_slug: &str, session_id: &str) -> String { + let mut hasher = Sha256::new(); + for part in [app_id, agent_slug, session_id] { + hasher.update(part.as_bytes()); + hasher.update([0]); + } + hasher + .finalize() + .iter() + .take(SESSION_HASH_BYTES) + .map(|byte| format!("{byte:02x}")) + .collect() +} + +fn is_valid_header_name(name: &str) -> bool { + reqwest::header::HeaderName::from_bytes(name.as_bytes()).is_ok() +} + +/// Env-flag semantics: set, non-blank, and not an explicit "off" spelling. +fn is_truthy(value: &str) -> bool { + let value = value.trim(); + !value.is_empty() + && !["0", "false", "no", "off"] + .iter() + .any(|off| value.eq_ignore_ascii_case(off)) +} + +#[cfg(test)] +mod tests; diff --git a/cli-engine/src/transport/attribution/tests.rs b/cli-engine/src/transport/attribution/tests.rs new file mode 100644 index 0000000..13d4450 --- /dev/null +++ b/cli-engine/src/transport/attribution/tests.rs @@ -0,0 +1,171 @@ +use super::*; + +const SECRET_SESSION: &str = "0b7e9c52-raw-session-id"; + +fn signals(env: &[(&str, &str)], interactive: bool) -> Signals { + Signals::from_lookup( + "gddy", + |name| { + env.iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| (*value).to_owned()) + }, + |_| false, + interactive, + ) +} + +fn claude_env() -> Vec<(&'static str, &'static str)> { + vec![ + ("CLAUDECODE", "1"), + ("CLAUDE_CODE_SESSION_ID", SECRET_SESSION), + ] +} + +fn resolve(signals: &Signals) -> Attribution { + Attribution::resolve(&AttributionConfig::new(), "gddy", signals) +} + +#[test] +fn detected_harness_adds_mode_and_agent_tokens() { + let attribution = resolve(&signals(&claude_env(), false)); + + assert_eq!( + attribution.user_agent_suffix, + " mode/agent agent/claude-code" + ); +} + +#[test] +fn session_header_carries_a_hash_never_the_raw_id() { + let attribution = resolve(&signals(&claude_env(), false)); + + let value = attribution + .headers + .get("x-client-session") + .expect("session header is sent"); + assert_eq!(value.len(), 16); + assert!(value.bytes().all(|byte| byte.is_ascii_hexdigit())); + assert!(!value.contains("raw-session")); + assert!(!attribution.user_agent_suffix.contains(SECRET_SESSION)); +} + +#[test] +fn session_hash_is_stable_and_salted_per_app() { + let same_a = hash_session("gddy", "claude-code", SECRET_SESSION); + let same_b = hash_session("gddy", "claude-code", SECRET_SESSION); + let other_app = hash_session("other-cli", "claude-code", SECRET_SESSION); + let other_session = hash_session("gddy", "claude-code", "different"); + + assert_eq!(same_a, same_b); + assert_ne!(same_a, other_app); + assert_ne!(same_a, other_session); +} + +#[test] +fn harness_without_a_session_id_sends_tokens_but_no_header() { + let attribution = resolve(&signals(&[("CLAUDECODE", "1")], false)); + + assert_eq!( + attribution.user_agent_suffix, + " mode/agent agent/claude-code" + ); + assert!(attribution.headers.is_empty()); +} + +#[test] +fn modes_follow_precedence_agent_ci_interactive_script() { + assert_eq!( + resolve(&signals(&[("CI", "true")], true)).user_agent_suffix, + " mode/ci" + ); + assert_eq!( + resolve(&signals(&[], true)).user_agent_suffix, + " mode/interactive" + ); + assert_eq!( + resolve(&signals(&[], false)).user_agent_suffix, + " mode/script" + ); + + let mut agent_in_ci = claude_env(); + agent_in_ci.push(("CI", "true")); + assert!( + resolve(&signals(&agent_in_ci, true)) + .user_agent_suffix + .starts_with(" mode/agent") + ); +} + +#[test] +fn explicit_off_spellings_do_not_count_as_ci() { + for value in ["", " ", "0", "false", "FALSE", "no", "off"] { + assert_eq!( + resolve(&signals(&[("CI", value)], false)).user_agent_suffix, + " mode/script", + "CI={value:?}" + ); + } +} + +#[test] +fn app_scoped_env_var_opts_out_of_the_session_header_only() { + let mut env = claude_env(); + env.push(("GDDY_NO_SESSION_ID", "1")); + + let attribution = resolve(&signals(&env, false)); + + assert!(attribution.headers.is_empty()); + assert_eq!( + attribution.user_agent_suffix, + " mode/agent agent/claude-code" + ); +} + +#[test] +fn opt_out_env_var_name_is_derived_from_the_app_id() { + let env = [ + ("CLAUDECODE", "1"), + ("CLAUDE_CODE_SESSION_ID", SECRET_SESSION), + ("MY_CLI_NO_SESSION_ID", "true"), + ]; + let lookup = |name: &str| { + env.iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| (*value).to_owned()) + }; + + let signals = Signals::from_lookup("my-cli", lookup, |_| false, false); + + assert!(signals.session_opted_out); +} + +#[test] +fn config_can_disable_the_session_header() { + let config = AttributionConfig::new().without_session_id(); + + let attribution = Attribution::resolve(&config, "gddy", &signals(&claude_env(), false)); + + assert!(attribution.headers.is_empty()); + assert!(attribution.user_agent_suffix.contains("agent/claude-code")); +} + +#[test] +fn config_can_rename_the_session_header() { + let config = AttributionConfig::new().with_session_header("X-GDDY-Session"); + + let attribution = Attribution::resolve(&config, "gddy", &signals(&claude_env(), false)); + + assert!(attribution.headers.contains_key("x-gddy-session")); + assert!(!attribution.headers.contains_key("x-client-session")); +} + +#[test] +fn invalid_header_name_drops_the_header_but_keeps_the_tokens() { + let config = AttributionConfig::new().with_session_header("not a header"); + + let attribution = Attribution::resolve(&config, "gddy", &signals(&claude_env(), false)); + + assert!(attribution.headers.is_empty()); + assert!(attribution.user_agent_suffix.contains("mode/agent")); +} diff --git a/cli-engine/src/transport/client/factory.rs b/cli-engine/src/transport/client/factory.rs new file mode 100644 index 0000000..6fdb795 --- /dev/null +++ b/cli-engine/src/transport/client/factory.rs @@ -0,0 +1,610 @@ +//! Shared `reqwest` client construction. +//! +//! Code that needs a plain [`reqwest::Client`] — progenitor-generated API +//! clients, hand-rolled multipart or streaming uploads, anything that cannot go +//! through [`super::HttpClient`] — builds it from [`reqwest_client_builder`] so +//! outbound policy (user-agent, timeouts, and future correlation headers) is +//! defined once in the engine instead of per call site. + +use std::time::Duration; + +use super::client_identity_snapshot; + +/// Connect timeout applied to every client from [`reqwest_client_builder`]. +pub const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); + +/// Idle read timeout applied to every client from [`reqwest_client_builder`]. +/// +/// This bounds the wait for each read (first response byte included), not the +/// whole request, so a large streaming download that keeps making progress is +/// never cut off. +pub const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(30); + +/// Returns a [`reqwest::ClientBuilder`] preconfigured with the engine's +/// outbound policy. +/// +/// - `User-Agent` is the process-wide default (see +/// [`set_default_user_agent`](super::set_default_user_agent)), and the +/// headers published by client attribution (see +/// [`AttributionConfig`](crate::transport::AttributionConfig)) are sent on +/// every request, so attribution reaches every client built here. A later +/// `.default_headers(..)` on the returned builder adds to them; on a name +/// clash the later value wins. +/// - Both are captured when this function is called, as one snapshot. Call it +/// from a command handler (after the `execute*` entrypoints have published +/// the identity), not during module registration, which runs earlier. +/// - [`DEFAULT_CONNECT_TIMEOUT`] and [`DEFAULT_READ_TIMEOUT`] bound hangs. +/// There is deliberately no total-request timeout: callers that want one +/// (for example a short API call) set `.timeout(..)` on the returned builder, +/// and transfer-heavy callers keep the default. +/// +/// The builder is returned unbuilt so callers can layer their own settings on +/// top (default headers, redirect policy, a per-client user-agent override). +/// Later settings win. +pub fn reqwest_client_builder() -> reqwest::ClientBuilder { + let (user_agent, headers) = client_identity_snapshot(); + // The user-agent goes last: `reqwest` stores it as a default header, so + // applying it after the header map keeps a `user-agent` entry in that map + // from replacing the identity, matching how `HttpClient` ranks them. + timeout_policy_builder() + .default_headers(header_map(&headers)) + .user_agent(user_agent) +} + +/// A builder carrying only the timeout policy, with no process identity. +/// +/// [`super::HttpClient`] applies its user-agent and default headers per +/// request from the snapshot its builder captured, so its base client must not +/// read the process-wide identity a second time. +fn timeout_policy_builder() -> reqwest::ClientBuilder { + reqwest::Client::builder() + .connect_timeout(DEFAULT_CONNECT_TIMEOUT) + .read_timeout(DEFAULT_READ_TIMEOUT) +} + +/// Converts default headers to a `HeaderMap`. Published defaults are already +/// validated; an entry that still fails to convert is skipped rather than +/// failing client construction. +fn header_map(headers: &std::collections::BTreeMap) -> reqwest::header::HeaderMap { + headers + .iter() + .filter_map(|(name, value)| { + Some(( + reqwest::header::HeaderName::from_bytes(name.as_bytes()).ok()?, + reqwest::header::HeaderValue::from_str(value).ok()?, + )) + }) + .collect() +} + +/// Builds the timeout-only base client for [`super::HttpClient`], falling back +/// to a bare client if the TLS backend fails to initialize. +pub(super) fn build_default_client() -> reqwest::Client { + timeout_policy_builder() + .build() + .unwrap_or_else(|_| reqwest::Client::new()) +} + +#[cfg(test)] +mod tests { + use std::{ + io::{Read, Write}, + net::TcpListener, + }; + + use super::*; + use crate::transport::client::{RestoreDefaultUserAgent, UA_TEST_LOCK}; + + /// Serves one request on loopback and returns the request head (the bytes + /// up to the blank line that ends the headers). + /// + /// TCP may deliver the head across several reads, so this reads until the + /// terminator arrives. It then drains any request body still in flight + /// before closing, so the client never sees a reset mid-write. + fn serve_once() -> (String, std::thread::JoinHandle) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback"); + let url = format!("http://{}/", listener.local_addr().expect("local addr")); + let handle = std::thread::spawn(move || { + const HEAD_END: &[u8] = b"\r\n\r\n"; + let (mut stream, _) = listener.accept().expect("accept"); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .expect("set read timeout"); + let mut received = Vec::new(); + let mut buf = [0_u8; 4096]; + let head_len = loop { + if let Some(at) = received.windows(HEAD_END.len()).position(|w| w == HEAD_END) { + break at; + } + let read = stream.read(&mut buf).expect("read request head"); + assert!(read > 0, "connection closed before the request head ended"); + received.extend_from_slice(&buf[..read]); + }; + stream + .write_all(b"HTTP/1.1 200 OK\r\ncontent-length: 0\r\nconnection: close\r\n\r\n") + .expect("write response"); + stream + .set_read_timeout(Some(Duration::from_millis(50))) + .expect("set drain timeout"); + while matches!(stream.read(&mut buf), Ok(read) if read > 0) {} + String::from_utf8_lossy(&received[..head_len]).into_owned() + }); + (url, handle) + } + + /// Builds a client while the process-default user-agent is `default_ua`. + /// + /// The user-agent is captured when the builder is created, so the global + /// lock is released before any `.await` in the caller. + fn build_with_default_ua( + default_ua: &str, + configure: impl FnOnce(reqwest::ClientBuilder) -> reqwest::ClientBuilder, + ) -> reqwest::Client { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_user_agent(default_ua); + configure(reqwest_client_builder()) + .build() + .expect("client builds") + } + + /// Builds a client while the process defaults are `ua` and `headers`. + fn build_with_defaults( + ua: &str, + headers: &[(&str, &str)], + configure: impl FnOnce(reqwest::ClientBuilder) -> reqwest::ClientBuilder, + ) -> reqwest::Client { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_user_agent(ua); + super::super::set_default_headers( + headers + .iter() + .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())) + .collect(), + ); + configure(reqwest_client_builder()) + .build() + .expect("client builds") + } + + #[tokio::test] + async fn default_headers_reach_the_wire() { + let client = build_with_defaults("probe/1", &[("x-client-session", "abc123")], |b| b); + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("x-client-session: abc123"), "{head}"); + } + + /// Generated clients add `Authorization` via `.default_headers(..)` on this + /// builder; that must add to, not replace, the process defaults. + #[tokio::test] + async fn later_default_headers_add_to_process_defaults() { + let client = build_with_defaults("probe/1", &[("x-client-session", "abc123")], |b| { + let mut extra = reqwest::header::HeaderMap::new(); + extra.insert( + reqwest::header::AUTHORIZATION, + reqwest::header::HeaderValue::from_static("Bearer tok"), + ); + b.default_headers(extra) + }); + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("x-client-session: abc123"), "{head}"); + assert!(head.contains("authorization: bearer tok"), "{head}"); + } + + /// `HttpClient` layers its own headers over the process defaults: both are + /// sent, and the client's value wins on a name clash. + #[tokio::test] + async fn http_client_merges_process_defaults_under_its_own_headers() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_headers( + [("x-client-session", "process"), ("x-process-only", "1")] + .into_iter() + .map(|(name, value)| (name.to_owned(), value.to_owned())) + .collect(), + ); + crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .default_headers( + [("x-client-session", "client".to_owned())] + .into_iter() + .map(|(name, value)| (name.to_owned(), value)) + .collect(), + ) + .build() + }; + + client.get_bytes("/").await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("x-client-session: client"), "{head}"); + assert!(!head.contains("x-client-session: process"), "{head}"); + assert!(head.contains("x-process-only: 1"), "{head}"); + } + + /// A default header never duplicates or overrides a header the request + /// itself sets: `Content-Type` and `User-Agent` are request-owned. + #[tokio::test] + async fn default_headers_do_not_duplicate_request_owned_headers() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_headers( + [ + ("content-type".to_owned(), "text/plain".to_owned()), + ("user-agent".to_owned(), "sneaky/0".to_owned()), + ("x-extra".to_owned(), "1".to_owned()), + ] + .into(), + ); + crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .user_agent("real/1") + .build() + }; + + client + .post_without_response("/", &serde_json::json!({"a": 1})) + .await + .expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert_eq!(head.matches("content-type:").count(), 1, "{head}"); + assert!(head.contains("content-type: application/json"), "{head}"); + assert_eq!(head.matches("user-agent:").count(), 1, "{head}"); + assert!(head.contains("user-agent: real/1"), "{head}"); + assert!(head.contains("x-extra: 1"), "{head}"); + } + + /// Replacing the user-agent invalidates the published pair, so it must not + /// leave a previous execution's attribution headers behind. + #[test] + fn setting_the_user_agent_clears_previously_published_headers() { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_client_identity( + "cli-a/1".to_owned(), + [("x-client-session".to_owned(), "a-session-hash".to_owned())].into(), + ); + + super::super::set_default_user_agent("cli-b/2"); + + let (user_agent, headers) = client_identity_snapshot(); + assert_eq!(user_agent, "cli-b/2"); + assert!(headers.is_empty(), "stale headers survived: {headers:?}"); + } + + /// A `user-agent` entry among the published headers must not replace the + /// identity user-agent on clients from the factory. + #[tokio::test] + async fn header_map_cannot_override_the_identity_user_agent() { + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_client_identity( + "identity/1".to_owned(), + [("user-agent".to_owned(), "sneaky/0".to_owned())].into(), + ); + reqwest_client_builder().build().expect("client builds") + }; + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert_eq!(head.matches("user-agent:").count(), 1, "{head}"); + assert!(head.contains("user-agent: identity/1"), "{head}"); + } + + /// A client's own default `Content-Type` must not replace a multipart + /// request's generated `Content-Type`: that header carries the boundary + /// the server needs to parse the body. + #[tokio::test] + async fn own_default_content_type_does_not_strip_the_multipart_boundary() { + let upload = tempfile::NamedTempFile::new().expect("temp file"); + std::fs::write(upload.path(), b"file-bytes").expect("write upload"); + let (url, server) = serve_once(); + let client = crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .default_headers( + [( + "Content-Type".to_owned(), + "application/vnd.test+json".to_owned(), + )] + .into(), + ) + .build(); + + client + .post_multipart_without_response("/", "file", upload.path()) + .await + .expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert_eq!(head.matches("content-type:").count(), 1, "{head}"); + assert!( + head.contains("content-type: multipart/form-data; boundary="), + "{head}" + ); + } + + /// The client's own default `Content-Type` replaces the JSON one, exactly + /// once (a client opting into a vendor media type), while a process-wide + /// default of the same name does not. + #[tokio::test] + async fn own_default_content_type_replaces_json_once() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_headers( + [("content-type".to_owned(), "text/plain".to_owned())].into(), + ); + crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .default_headers( + [( + "Content-Type".to_owned(), + "application/vnd.test+json".to_owned(), + )] + .into(), + ) + .build() + }; + + client + .post_without_response("/", &serde_json::json!({"a": 1})) + .await + .expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert_eq!(head.matches("content-type:").count(), 1, "{head}"); + assert!( + head.contains("content-type: application/vnd.test+json"), + "{head}" + ); + } + + /// A client's `X-Client-Session` replaces a process default `x-client-session` + /// (header names are case-insensitive); only the client's value is sent. + #[tokio::test] + async fn http_client_header_override_is_case_insensitive() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_headers( + [("x-client-session".to_owned(), "process".to_owned())].into(), + ); + crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .default_headers([("X-Client-Session".to_owned(), "client".to_owned())].into()) + .build() + }; + + client.get_bytes("/").await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert_eq!(head.matches("x-client-session:").count(), 1, "{head}"); + assert!(head.contains("x-client-session: client"), "{head}"); + } + + /// `HttpClient` uses the identity captured when its builder was created; + /// publishing a different identity before `build()` must not leak into it. + #[tokio::test] + async fn http_client_does_not_resnapshot_identity_at_build_time() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_client_identity( + "app-a/1".to_owned(), + [("x-app".to_owned(), "a".to_owned())].into(), + ); + let builder = crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ); + super::super::set_client_identity( + "app-b/2".to_owned(), + [("x-app-b".to_owned(), "b".to_owned())].into(), + ); + builder.build() + }; + + client.get_bytes("/").await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("user-agent: app-a/1"), "{head}"); + assert!(head.contains("x-app: a"), "{head}"); + assert!(!head.contains("app-b"), "{head}"); + assert!(!head.contains("x-app-b"), "{head}"); + } + + /// The user-agent and default headers are published and read as a pair, so + /// a concurrent reader can never see one publish's user-agent with + /// another's headers. + #[test] + fn identity_pair_is_never_torn_across_concurrent_publishes() { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + let writers: Vec<_> = (0..4) + .map(|id| { + std::thread::spawn(move || { + for _ in 0..2_000 { + let tag = format!("app-{id}"); + super::super::set_client_identity( + tag.clone(), + std::collections::BTreeMap::from([("x-app".to_owned(), tag)]), + ); + } + }) + }) + .collect(); + let reader = std::thread::spawn(|| { + for _ in 0..20_000 { + let (user_agent, headers) = client_identity_snapshot(); + if let Some(app) = headers.get("x-app") { + assert_eq!(&user_agent, app, "torn identity: {user_agent} / {app}"); + } + } + }); + + for writer in writers { + writer.join().expect("writer thread"); + } + reader.join().expect("reader saw only consistent pairs"); + } + + #[test] + fn set_default_headers_drops_invalid_entries_at_publish_time() { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + + super::super::set_default_headers( + [ + ("not a header", "x"), + ("x-bad-value", "line\nbreak"), + ("x-ok", "1"), + ] + .into_iter() + .map(|(name, value)| (name.to_owned(), value.to_owned())) + .collect(), + ); + + let kept = super::super::default_headers(); + assert_eq!(kept.keys().collect::>(), vec!["x-ok"]); + } + + /// An invalid process default must not make `HttpClient` requests fail at + /// request-construction time. + #[tokio::test] + async fn http_client_requests_survive_an_invalid_process_default_header() { + let (url, server) = serve_once(); + let client = { + let _guard = UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = RestoreDefaultUserAgent; + super::super::set_default_headers( + [("not a header", "x"), ("x-ok", "1")] + .into_iter() + .map(|(name, value)| (name.to_owned(), value.to_owned())) + .collect(), + ); + crate::transport::HttpClientBuilder::new( + url.trim_end_matches('/'), + std::sync::Arc::new(crate::transport::NoopInjector), + ) + .build() + }; + + client + .get_bytes("/") + .await + .expect("request is built despite the invalid default"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("x-ok: 1"), "{head}"); + } + + #[tokio::test] + async fn invalid_default_header_is_skipped_not_fatal() { + let client = build_with_defaults("probe/1", &[("not a header", "x"), ("x-ok", "1")], |b| b); + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("x-ok: 1"), "{head}"); + } + + #[tokio::test] + async fn builder_sends_the_process_default_user_agent() { + let client = build_with_default_ua("factory-probe/1.2", |builder| builder); + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("user-agent: factory-probe/1.2"), "{head}"); + } + + #[tokio::test] + async fn caller_can_override_the_user_agent() { + let client = build_with_default_ua("factory-probe/1.2", |builder| { + builder.user_agent("override/9") + }); + let (url, server) = serve_once(); + + client.get(&url).send().await.expect("request succeeds"); + + let head = server.join().expect("server thread").to_lowercase(); + assert!(head.contains("user-agent: override/9"), "{head}"); + } + + #[tokio::test] + async fn stalled_server_hits_the_read_timeout_not_a_hang() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback"); + let url = format!("http://{}/", listener.local_addr().expect("local addr")); + // Accept and never respond; keep the socket open until the client gives up. + let server = std::thread::spawn(move || { + let (stream, _) = listener.accept().expect("accept"); + std::thread::sleep(Duration::from_millis(400)); + drop(stream); + }); + let client = reqwest_client_builder() + .read_timeout(Duration::from_millis(100)) + .build() + .expect("client builds"); + + let error = client.get(&url).send().await.expect_err("must time out"); + + assert!(error.is_timeout(), "{error}"); + server.join().expect("server thread"); + } +} diff --git a/cli-engine/src/transport/client/methods.rs b/cli-engine/src/transport/client/methods.rs index 3714483..63fcfe0 100644 --- a/cli-engine/src/transport/client/methods.rs +++ b/cli-engine/src/transport/client/methods.rs @@ -465,7 +465,7 @@ impl HttpClient { .header(header::IF_MATCH, etag) .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; self.log_request(&request); self.base .execute(request) @@ -497,18 +497,15 @@ impl HttpClient { form: reqwest::multipart::Form, ) -> Result { let url = format!("{}{}", self.base_url, path); - let mut builder = self + let builder = self .base .post(url) .header(header::USER_AGENT, self.user_agent.clone()) .multipart(form); - for (key, value) in &self.default_headers { - builder = builder.header(key, value); - } let mut request = builder .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; self.log_request(&request); self.base .execute(request) @@ -547,13 +544,10 @@ impl HttpClient { if !content_type.is_empty() { builder = builder.header(header::CONTENT_TYPE, content_type); } - for (key, value) in &self.default_headers { - builder = builder.header(key, value); - } let mut request = builder .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; self.log_request(&request); self.base .execute(request) @@ -591,17 +585,14 @@ impl HttpClient { async fn send_get_raw_once(&self, path: &str) -> Result { let url = format!("{}{}", self.base_url, path); - let mut builder = self + let builder = self .base .get(url) .header(header::USER_AGENT, self.user_agent.clone()); - for (key, value) in &self.default_headers { - builder = builder.header(key, value); - } let mut request = builder .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; self.log_request(&request); self.base .execute(request) @@ -618,7 +609,7 @@ impl HttpClient { .build_request(Method::POST, path, body)? .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; self.log_request(&request); self.base .execute(request) @@ -711,7 +702,7 @@ impl HttpClient { .build_request(method.clone(), path, body)? .build() .map_err(|err| CliCoreError::message(format!("transport: create request: {err}")))?; - self.inject_auth(&mut request).await?; + self.prepare_request(&mut request).await?; let method_text = method.as_str().to_owned(); self.log_request(&request); self.base @@ -738,9 +729,6 @@ impl HttpClient { .header(header::CONTENT_TYPE, "application/json") .body(body); } - for (key, value) in &self.default_headers { - builder = builder.header(key, value); - } Ok(builder) } @@ -847,13 +835,51 @@ impl HttpClient { Ok((status, body)) } - async fn inject_auth(&self, request: &mut reqwest::Request) -> Result<()> { + /// Finalizes a built request: applies default headers, then injects auth. + /// + /// The client's own defaults replace a same-named header the request set + /// by default (for example `Content-Type`), once. Process-wide defaults + /// rank lowest: they only fill in headers still absent, so they can never + /// duplicate or override anything. Applying both here, on the built + /// request, rather than appending on the builder, is what prevents + /// duplicate header lines and keeps the defaults visible in the + /// `--debug transport` trace. + async fn prepare_request(&self, request: &mut reqwest::Request) -> Result<()> { + self.apply_default_headers(request)?; self.auth .inject(request) .await .map_err(|err| CliCoreError::message(format!("transport: auth inject: {err}"))) } + fn apply_default_headers(&self, request: &mut reqwest::Request) -> Result<()> { + let parse = |name: &str, value: &str| -> Result<_> { + let invalid = |err: &dyn std::fmt::Display| { + CliCoreError::message(format!("transport: invalid default header {name:?}: {err}")) + }; + Ok(( + header::HeaderName::from_bytes(name.as_bytes()).map_err(|e| invalid(&e))?, + header::HeaderValue::from_str(value).map_err(|e| invalid(&e))?, + )) + }; + for (name, value) in &self.default_headers { + let (name, value) = parse(name, value)?; + // A multipart `Content-Type` carries the boundary the server needs + // to parse the body; replacing it would make the upload unreadable. + if name == header::CONTENT_TYPE && has_multipart_content_type(request) { + continue; + } + request.headers_mut().insert(name, value); + } + for (name, value) in &self.process_headers { + let (name, value) = parse(name, value)?; + if !request.headers().contains_key(&name) { + request.headers_mut().insert(name, value); + } + } + Ok(()) + } + async fn decode_json_response( &self, response: reqwest::Response, @@ -934,3 +960,18 @@ impl HttpClient { } } } + +/// Whether the request's `Content-Type` is a multipart type (its generated +/// value carries the part boundary). +fn has_multipart_content_type(request: &reqwest::Request) -> bool { + request + .headers() + .get(header::CONTENT_TYPE) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| { + value + .trim_start() + .to_ascii_lowercase() + .starts_with("multipart/") + }) +} diff --git a/cli-engine/src/transport/client/mod.rs b/cli-engine/src/transport/client/mod.rs index 7d34c41..145d462 100644 --- a/cli-engine/src/transport/client/mod.rs +++ b/cli-engine/src/transport/client/mod.rs @@ -9,12 +9,33 @@ use serde_json::Value; use super::{AuthInjector, Error}; +mod factory; mod methods; +pub use factory::{DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT, reqwest_client_builder}; + const MAX_RETRIES: usize = 3; const BASE_BACKOFF: Duration = Duration::from_millis(500); const BUILTIN_DEFAULT_USER_AGENT: &str = "cli/dev"; -static DEFAULT_USER_AGENT: OnceLock> = OnceLock::new(); + +/// Process-wide outbound identity: the user-agent and any headers every +/// client should carry. One lock covers both so they are published together. +#[derive(Clone, Debug)] +struct ClientIdentity { + user_agent: String, + headers: BTreeMap, +} + +static CLIENT_IDENTITY: OnceLock> = OnceLock::new(); + +fn client_identity() -> &'static RwLock { + CLIENT_IDENTITY.get_or_init(|| { + RwLock::new(ClientIdentity { + user_agent: BUILTIN_DEFAULT_USER_AGENT.to_owned(), + headers: BTreeMap::new(), + }) + }) +} /// Sets the process-wide default user-agent for outbound requests. /// @@ -23,11 +44,16 @@ static DEFAULT_USER_AGENT: OnceLock> = OnceLock::new(); /// outbound token traffic that reads this default — the PKCE provider's /// token/refresh requests and the client-credentials injector. A per-client /// user-agent still overrides it for that client. +/// +/// This replaces the whole process-wide outbound identity: it also drops any +/// headers published by client attribution, so the new user-agent is never +/// paired with a previous execution's session header. pub fn set_default_user_agent(user_agent: impl Into) { - let lock = - DEFAULT_USER_AGENT.get_or_init(|| RwLock::new(BUILTIN_DEFAULT_USER_AGENT.to_owned())); - if let Ok(mut current) = lock.write() { - *current = user_agent.into(); + if let Ok(mut current) = client_identity().write() { + current.user_agent = user_agent.into(); + // The headers belong to the identity this replaces; keeping them would + // pair the new user-agent with a previous execution's session hash. + current.headers.clear(); } } @@ -36,14 +62,72 @@ pub fn set_default_user_agent(user_agent: impl Into) { /// /// Used by [`HttpClientBuilder`] and by the engine's OAuth token requests so /// that all outbound traffic carries the same user-agent. -pub(crate) fn default_user_agent() -> String { - DEFAULT_USER_AGENT - .get_or_init(|| RwLock::new(BUILTIN_DEFAULT_USER_AGENT.to_owned())) +#[must_use] +pub fn default_user_agent() -> String { + client_identity().read().map_or_else( + |_| BUILTIN_DEFAULT_USER_AGENT.to_owned(), + |identity| identity.user_agent.clone(), + ) +} + +/// Test helper: replaces only the process-wide default headers. +/// +/// Production code publishes headers together with the user-agent through +/// [`set_client_identity`]; the engine's client attribution is the only +/// producer, which is why this is not public API. +#[cfg(test)] +pub(crate) fn set_default_headers(headers: BTreeMap) { + let headers = valid_headers(headers); + if let Ok(mut current) = client_identity().write() { + current.headers = headers; + } +} + +/// Replaces the user-agent and default headers under one lock acquisition, so +/// a concurrent reader never observes one app's user-agent with another app's +/// headers. +pub(crate) fn set_client_identity(user_agent: String, headers: BTreeMap) { + let headers = valid_headers(headers); + if let Ok(mut current) = client_identity().write() { + current.user_agent = user_agent; + current.headers = headers; + } +} + +/// Reads the user-agent and default headers under one lock acquisition. Client +/// constructors use this so the pair they apply is always self-consistent. +pub(crate) fn client_identity_snapshot() -> (String, BTreeMap) { + client_identity().read().map_or_else( + |_| (BUILTIN_DEFAULT_USER_AGENT.to_owned(), BTreeMap::new()), + |identity| (identity.user_agent.clone(), identity.headers.clone()), + ) +} + +/// Keeps only entries that are valid HTTP header names with valid values, with +/// names lowercased (header names are case-insensitive, so one canonical form +/// lets later merging detect a clash). +fn valid_headers(headers: BTreeMap) -> BTreeMap { + headers + .into_iter() + .filter(|(name, value)| { + let valid = header::HeaderName::from_bytes(name.as_bytes()).is_ok() + && header::HeaderValue::from_str(value).is_ok(); + if !valid { + tracing::warn!(header = %name, "ignoring invalid default header"); + } + valid + }) + .map(|(name, value)| (name.to_ascii_lowercase(), value)) + .collect() +} + +/// Test helper: the process-wide default headers currently published. +#[cfg(test)] +pub(crate) fn default_headers() -> BTreeMap { + client_identity() .read() - .map_or_else( - |_| BUILTIN_DEFAULT_USER_AGENT.to_owned(), - |value| value.clone(), - ) + .map(|identity| identity.headers.clone()) + .unwrap_or_default() } /// Serializes unit tests that mutate the process-wide default user-agent so @@ -52,10 +136,10 @@ pub(crate) fn default_user_agent() -> String { #[cfg(test)] pub(crate) static UA_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); -/// Restores the process-wide default user-agent to the builtin on drop, so a -/// panicking assertion in a test that mutates it cannot leak the value into -/// later tests in this binary. Declare it after acquiring [`UA_TEST_LOCK`] so -/// the reset runs while the lock is still held. +/// Restores the process-wide default user-agent and headers to the builtins on +/// drop, so a panicking assertion in a test that mutates them cannot leak the +/// values into later tests in this binary. Declare it after acquiring +/// [`UA_TEST_LOCK`] so the reset runs while the lock is still held. #[cfg(test)] pub(crate) struct RestoreDefaultUserAgent; @@ -63,6 +147,7 @@ pub(crate) struct RestoreDefaultUserAgent; impl Drop for RestoreDefaultUserAgent { fn drop(&mut self) { set_default_user_agent(BUILTIN_DEFAULT_USER_AGENT); + set_default_headers(BTreeMap::new()); } } @@ -224,6 +309,10 @@ pub struct HttpClient { base_url: String, auth: Arc, user_agent: String, + /// Process-wide defaults captured at builder creation; fill in headers a + /// request does not already carry. + process_headers: BTreeMap, + /// The client's own defaults; replace a same-named request header. default_headers: BTreeMap, logger: Arc, } @@ -234,18 +323,27 @@ pub struct HttpClientBuilder { base_url: String, auth: Arc, user_agent: String, + /// Process-wide default headers captured with `user_agent` in one snapshot. + process_headers: BTreeMap, default_headers: BTreeMap, logger: Arc, } impl HttpClientBuilder { /// Creates a builder with a base URL and auth injector. + /// + /// Captures the process-wide user-agent and default headers now, as one + /// snapshot. Create clients inside command handlers, which run after the + /// `execute*` entrypoints have published the CLI's identity, not during + /// module registration, which runs earlier. #[must_use] pub fn new(base_url: impl Into, auth: Arc) -> Self { + let (user_agent, process_headers) = client_identity_snapshot(); Self { base_url: base_url.into(), auth, - user_agent: default_user_agent(), + user_agent, + process_headers, default_headers: BTreeMap::new(), logger: default_transport_logger(), } @@ -265,6 +363,12 @@ impl HttpClientBuilder { } /// Sets headers sent on every request. + /// + /// A default replaces (never duplicates) a header the request sets by + /// default with the same name, for example `Content-Type`; this is how a + /// client opts into a vendor media type. Header names are case-insensitive. + /// An invalid name or value fails the request. Process-wide defaults rank + /// below these: they only fill in headers that are otherwise absent. #[must_use] pub fn default_headers(mut self, headers: BTreeMap) -> Self { self.default_headers = headers; @@ -294,10 +398,11 @@ impl HttpClientBuilder { #[must_use] pub fn build(self) -> HttpClient { HttpClient { - base: reqwest::Client::new(), + base: factory::build_default_client(), base_url: self.base_url, auth: self.auth, user_agent: self.user_agent, + process_headers: self.process_headers, default_headers: self.default_headers, logger: self.logger, } diff --git a/cli-engine/src/transport/mod.rs b/cli-engine/src/transport/mod.rs index 7b73d3b..4dd3b7c 100644 --- a/cli-engine/src/transport/mod.rs +++ b/cli-engine/src/transport/mod.rs @@ -12,16 +12,20 @@ use serde::{Deserialize, Serialize}; use crate::DetailedError; +mod attribution; /// HTTP client implementation. pub mod client; mod debug_logger; /// Request auth injectors. pub mod injector; +pub use attribution::AttributionConfig; +pub(crate) use attribution::{Attribution, Signals}; pub use client::{ - HttpClient, HttpClientBuilder, NoopTransportLogger, TransportLogEvent, TransportLogger, - debug_log_reqwest_request, debug_log_reqwest_response, default_transport_logger, - set_default_transport_logger, set_default_user_agent, + DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT, HttpClient, HttpClientBuilder, + NoopTransportLogger, TransportLogEvent, TransportLogger, debug_log_reqwest_request, + debug_log_reqwest_response, default_transport_logger, default_user_agent, + reqwest_client_builder, set_default_transport_logger, set_default_user_agent, }; pub use debug_logger::StderrTransportLogger; pub use injector::{ diff --git a/cli-engine/tests/foundation.rs b/cli-engine/tests/foundation.rs index f2e2743..abb1f91 100644 --- a/cli-engine/tests/foundation.rs +++ b/cli-engine/tests/foundation.rs @@ -6156,6 +6156,46 @@ async fn http_client_no_content_returns_default_result_preserves_legacy_skips_de assert_eq!(value, Thing::default()); } +/// A personality hand-off runs an independent `Cli` with its own config; the +/// outbound identity published for the process must be that CLI's, not the +/// dispatcher's. +#[tokio::test] +async fn execute_from_publishes_the_personality_identity_not_the_dispatchers() { + let _guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore = RestoreDefaultUserAgent; + let cli = Cli::new( + CliConfig::new("outer", "Outer", "outer") + .with_build(cli_engine::BuildInfo::new("1.0.0")) + .with_argv0_personality("inner", || { + CliConfig::new("inner", "Inner", "inner") + .with_build(cli_engine::BuildInfo::new("9.9.9")) + }), + ); + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + + cli.execute_from(["inner", "--version"], &mut stdout, &mut stderr) + .await + .expect("personality run completes"); + + assert_eq!(transport::default_user_agent(), "inner/9.9.9"); +} + +/// `Cli::run` never mutates process-wide state (tests call it concurrently); +/// only the `execute*` entrypoints publish the outbound identity. +#[tokio::test] +async fn run_does_not_publish_the_process_identity() { + let _guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore = RestoreDefaultUserAgent; + transport::set_default_user_agent("sentinel/1"); + let cli = Cli::new( + CliConfig::new("outer", "Outer", "outer").with_build(cli_engine::BuildInfo::new("1.0.0")), + ); + + cli.run(["outer", "--version"]).await; + + assert_eq!(transport::default_user_agent(), "sentinel/1"); +} + #[tokio::test] async fn http_client_common_method_helpers_cover_put_patch_and_delete_variants() { let put_server = TestServer::new(|request| {