diff --git a/.github/workflows/test-cli.yml b/.github/workflows/test-cli.yml index d6dea0e..58a4e92 100644 --- a/.github/workflows/test-cli.yml +++ b/.github/workflows/test-cli.yml @@ -29,5 +29,12 @@ jobs: - name: Clippy run: cargo clippy -p clickhousectl -- -D warnings + # Telemetry must compile out entirely for distro packagers. + - name: Check without default features (telemetry compiled out) + run: cargo check -p clickhousectl --no-default-features + + - name: Clippy without default features + run: cargo clippy -p clickhousectl --all-targets --no-default-features -- -D warnings + - name: Test run: cargo test -p clickhousectl diff --git a/README.md b/README.md index 1b364c9..61c8d70 100644 --- a/README.md +++ b/README.md @@ -835,6 +835,40 @@ clickhousectl update --check The CLI checks for updates in the background (at most once per 24 hours) and caches the result. When a newer version is available, a one-line notice is printed to stderr at the end of every command that produces human-readable output. JSON output (`--json` or a detected coding agent) is never affected, so machine consumers stay clean. Running `clickhousectl update` clears the cached notice. +## Telemetry + +`clickhousectl` collects anonymous usage data to help us understand which commands matter and improve the CLI. Full details: . + +Each event contains exactly: + +- the command path (e.g. `local start`) +- the **names** of the flags passed (e.g. `json`, `org-id`) — never flag values, never positional arguments +- the exit code (`gh`-style: 0 success, 1 error, 2 cancelled, 4 auth required) +- the CLI version, OS, and architecture +- whether it ran in CI (`CI` env var) +- whether it ran under a detected coding agent, and if so which one (e.g. `claude-code`) + +There is no install ID, no device ID, and no fingerprinting of any kind. The payload is built from the clap command definitions rather than the raw command line, so leaking an argument value is structurally impossible — the code that builds the event has no access to values at all. + +Nothing is ever sent before you have seen the notice: the first run prints a one-time notice to stderr, records that it was shown in `~/.clickhouse/telemetry.json`, and sends nothing. Sending starts from the following run. The send happens in a short-lived detached process, so command latency is unaffected even when the endpoint is unreachable. + +Opt out any of these ways: + +```bash +# Persistently, per machine +clickhousectl telemetry disable + +# Check the current state +clickhousectl telemetry status + +# Per environment/shell (https://consoledonottrack.com) +export DO_NOT_TRACK=1 +``` + +To see exactly what would be sent without sending it, set `CHCTL_TELEMETRY_DEBUG=1` — the payload is printed to stderr and nothing leaves the machine. + +Distribution packagers can compile telemetry out entirely (including the `telemetry` subcommand) with `cargo build --no-default-features`. + ## Cloud integration testing Cloud API integration is tested against a real ClickHouse Cloud workspace via the library crate. All changes to cloud commands must pass CI testing before merge. Tests live in three binaries, each a single `#[tokio::test]` lifecycle: diff --git a/crates/clickhousectl/Cargo.toml b/crates/clickhousectl/Cargo.toml index 6040931..c2f681e 100644 --- a/crates/clickhousectl/Cargo.toml +++ b/crates/clickhousectl/Cargo.toml @@ -16,6 +16,11 @@ pkg-fmt = "tgz" bin-dir = "{ name }-{ target }-v{ version }/{ bin }{ binary-ext }" [features] +default = ["telemetry"] +# Anonymous usage telemetry (first-run notice, `telemetry` subcommand, detached +# send). On by default; distro packagers can compile it out entirely with +# `--no-default-features`. +telemetry = [] # Forwards to the library feature of the same name: when enabled, deprecated API # fields are present on the response structs and surfaced in CLI output. Off by # default so the CLI never shows a field the API has deprecated. diff --git a/crates/clickhousectl/src/cli.rs b/crates/clickhousectl/src/cli.rs index cad666a..40b8159 100644 --- a/crates/clickhousectl/src/cli.rs +++ b/crates/clickhousectl/src/cli.rs @@ -76,6 +76,41 @@ CONTEXT FOR AGENTS: Self-update command. Downloads the latest clickhousectl release from GitHub and replaces the current binary. Use --check to see if an update is available without installing.")] Update(UpdateArgs), + + /// Manage anonymous usage telemetry + #[cfg(feature = "telemetry")] + #[command(after_help = "\ +CONTEXT FOR AGENTS: + clickhousectl collects anonymous usage data: command name, flag names (never values or + arguments), success/failure, version, OS/arch, and CI/agent detection. No user or machine IDs. + Opt out with `clickhousectl telemetry disable` or DO_NOT_TRACK=1. + Details: https://clickhouse.com/docs/interfaces/cli#telemetry")] + Telemetry(TelemetryArgs), +} + +#[cfg(feature = "telemetry")] +#[derive(Args, Debug)] +pub struct TelemetryArgs { + #[command(subcommand)] + pub command: TelemetryCommands, +} + +#[cfg(feature = "telemetry")] +#[derive(Subcommand, Debug)] +pub enum TelemetryCommands { + /// Enable anonymous usage telemetry + Enable, + /// Disable anonymous usage telemetry + Disable, + /// Show whether telemetry is enabled and why + /// + /// On a machine that has never seen the first-run notice, this reports + /// "not yet configured" and then completes the first run itself (writes + /// the marker file and prints the notice). + Status, + /// (internal) Fire one telemetry POST from CHCTL_TELEMETRY_PAYLOAD and exit + #[command(hide = true)] + Send, } #[derive(Args, Debug)] @@ -160,4 +195,27 @@ mod tests { assert!(args.global); assert_eq!(args.agents, vec!["claude", "codex", "agents"]); } + + #[cfg(feature = "telemetry")] + #[test] + fn parses_telemetry_subcommands() { + for (arg, expected) in [ + ("enable", "Enable"), + ("disable", "Disable"), + ("status", "Status"), + ("send", "Send"), + ] { + let cli = Cli::try_parse_from(["clickhousectl", "telemetry", arg]).unwrap(); + let Commands::Telemetry(args) = cli.command else { + panic!("expected telemetry command for {arg}"); + }; + assert_eq!(format!("{:?}", args.command), expected); + } + } + + #[cfg(feature = "telemetry")] + #[test] + fn telemetry_requires_a_subcommand() { + assert!(Cli::try_parse_from(["clickhousectl", "telemetry"]).is_err()); + } } diff --git a/crates/clickhousectl/src/main.rs b/crates/clickhousectl/src/main.rs index 0abde89..863c9d7 100644 --- a/crates/clickhousectl/src/main.rs +++ b/crates/clickhousectl/src/main.rs @@ -7,12 +7,14 @@ mod init; mod local; mod paths; mod skills; +#[cfg(feature = "telemetry")] +mod telemetry; mod update; mod user_agent; mod version_manager; -use clap::Parser; use clap::error::ErrorKind; +use clap::{CommandFactory, FromArgMatches}; use cli::{ ActivityCommands, AuthCommands, BackupCommands, BackupConfigCommands, Cli, ClickPipeCommands, ClickPipeCreateCommands, ClickPipeSettingsCommands, CloudArgs, CloudCommands, Commands, @@ -32,8 +34,12 @@ async fn main() { // libc's environ. dotenv::init(); - let cli = match Cli::try_parse() { - Ok(cli) => cli, + // Parse via ArgMatches (rather than `Cli::try_parse()`) so the telemetry + // capture below can read the command path and passed-flag *names* from the + // clap definitions — argument values are never consulted. + let mut cmd = Cli::command(); + let matches = match cmd.try_get_matches_from_mut(std::env::args_os()) { + Ok(matches) => matches, Err(e) => { match e.kind() { // --version always hits the network to refresh the cache + timer, @@ -50,11 +56,32 @@ async fn main() { update::print_cached_update_notice(); std::process::exit(0); } + // Parse errors exit here, before telemetry capture: a mistyped + // invocation never produces an event. _ => e.exit(), } } }; + #[cfg(feature = "telemetry")] + let telemetry_invocation = telemetry::capture(&cmd, &matches); + + let cli = Cli::from_arg_matches(&matches).unwrap_or_else(|e| e.exit()); + + // The hidden send child does exactly one POST and exits: no update-cache + // refresh, no dispatch, and no telemetry hook of its own (a send can never + // trigger another send). + #[cfg(feature = "telemetry")] + if matches!( + cli.command, + Commands::Telemetry(cli::TelemetryArgs { + command: cli::TelemetryCommands::Send + }) + ) { + telemetry::run_child_send().await; + std::process::exit(0); + } + // Spawn a background task to refresh the update cache for non-update // commands. The refresh is gated to one network call per 24h; the notice // below is driven off whatever the cache currently holds. @@ -92,6 +119,11 @@ async fn main() { update::print_cached_update_notice(); } + // Consent is evaluated here, after the command ran, so `telemetry disable` + // silences its own event and `telemetry enable` sends one. + #[cfg(feature = "telemetry")] + telemetry::finalize(telemetry_invocation, exit_code); + std::process::exit(exit_code); } @@ -105,6 +137,8 @@ fn command_json_flag(cmd: &Commands) -> Option { Commands::Local(args) => Some(args.json), Commands::Cloud(args) => Some(args.json), Commands::Skills(_) => Some(false), + #[cfg(feature = "telemetry")] + Commands::Telemetry(_) => Some(false), } } @@ -132,6 +166,8 @@ async fn run(cmd: Commands) -> Result<()> { Commands::Skills(args) => run_skills(args).await, Commands::Cloud(args) => run_cloud(*args).await, Commands::Update(args) => run_update(args).await, + #[cfg(feature = "telemetry")] + Commands::Telemetry(args) => telemetry::run_command(args.command), } } @@ -1273,6 +1309,7 @@ async fn run_postgres( #[cfg(test)] mod tests { use super::*; + use clap::Parser; use cloud::{CloudError, CloudErrorKind}; #[test] @@ -1317,6 +1354,12 @@ mod tests { ); // The update command never surfaces the notice. assert_eq!(command_json_flag(&parse(&["clickhousectl", "update"])), None); + // Telemetry management commands are human-readable output. + #[cfg(feature = "telemetry")] + assert_eq!( + command_json_flag(&parse(&["clickhousectl", "telemetry", "status"])), + Some(false) + ); } #[test] diff --git a/crates/clickhousectl/src/telemetry.rs b/crates/clickhousectl/src/telemetry.rs new file mode 100644 index 0000000..fba3379 --- /dev/null +++ b/crates/clickhousectl/src/telemetry.rs @@ -0,0 +1,620 @@ +//! Anonymous usage telemetry (issue #283). +//! +//! Consent model follows Homebrew: nothing is ever sent before the first-run +//! notice has been shown. State lives in `~/.clickhouse/telemetry.json` +//! (`{"disabled": false}`), which doubles as the first-run marker — the notice +//! is only printed after the file has been written successfully, so an +//! unwritable config dir fails open to disabled (no send, no error, no +//! repeated notice). `DO_NOT_TRACK` (donottrack.sh convention) overrides +//! everything: no notice, no file write, no send. +//! +//! The payload carries the command path and flag *names* only — never flag +//! values, never positional arguments. It is built from the clap definitions +//! ([`capture`] walks `ArgMatches` ids and `Arg` metadata, never touching +//! `get_one`/`get_raw`), so leaking a value is structurally impossible. +//! +//! Transport is a detached child process (`clickhousectl telemetry send`, +//! hidden): the parent spawns it with all stdio nulled and never waits, so +//! command latency is unaffected even when the endpoint is unreachable. The +//! child fires one POST with a short timeout and dies silently on any failure. + +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use crate::error::{Error, Result}; +use crate::paths; + +/// Public documentation for what is collected and how to opt out. +const DOCS_URL: &str = "https://clickhouse.com/docs/interfaces/cli#telemetry"; + +/// Production ingest endpoint (Cloudflare worker in front of ClickHouse Cloud). +const DEFAULT_ENDPOINT: &str = "https://chctl.clickhouse.com/v1/telemetry"; + +/// Overrides the ingest endpoint (integration tests, local worker dev). +const URL_ENV: &str = "CHCTL_TELEMETRY_URL"; +/// Carries the serialized payload from the parent to the hidden send child. +const PAYLOAD_ENV: &str = "CHCTL_TELEMETRY_PAYLOAD"; +/// When truthy: print the exact payload to stderr and send nothing. +const DEBUG_ENV: &str = "CHCTL_TELEMETRY_DEBUG"; +/// donottrack.sh convention: when truthy, telemetry is fully silent. +const DNT_ENV: &str = "DO_NOT_TRACK"; +/// Standard CI marker, sent as a boolean so pipelines can be filtered out. +const CI_ENV: &str = "CI"; + +const SEND_TIMEOUT: Duration = Duration::from_secs(2); + +/// The ingest worker caps `flags` at 64 entries; truncate client-side too. +const MAX_FLAGS: usize = 64; + +// --------------------------------------------------------------------------- +// Consent state +// --------------------------------------------------------------------------- + +#[derive(serde::Serialize, serde::Deserialize)] +struct StateFile { + disabled: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum State { + /// No `telemetry.json` yet: the first-run notice has not been shown. + Missing, + Enabled, + Disabled, +} + +/// `~/.clickhouse/telemetry.json`. `None` when the home directory cannot be +/// determined, in which case telemetry is silently off. +fn state_path() -> Option { + paths::base_dir().ok().map(|dir| dir.join("telemetry.json")) +} + +/// A corrupt or unreadable state file counts as `Disabled`, not `Missing`: +/// the notice was shown once already, and when in doubt we don't send. +fn load_state_from(path: &Path) -> State { + match std::fs::read_to_string(path) { + Ok(contents) => match serde_json::from_str::(&contents) { + Ok(state) if !state.disabled => State::Enabled, + _ => State::Disabled, + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => State::Missing, + Err(_) => State::Disabled, + } +} + +fn save_state_to(path: &Path, disabled: bool) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let json = serde_json::to_string(&StateFile { disabled }) + .expect("StateFile serialization cannot fail"); + std::fs::write(path, json) +} + +// --------------------------------------------------------------------------- +// Environment +// --------------------------------------------------------------------------- + +/// Lookup function for reading process environment variables. Production +/// callers pass a wrapper around `std::env::var`; tests pass a closure over a +/// synthetic map (edition 2024 makes `set_var` unsafe, so tests never mutate +/// the real environment). +type EnvLookup<'a> = &'a dyn Fn(&str) -> Option; + +fn real_env_lookup(key: &str) -> Option { + std::env::var(key).ok() +} + +/// donottrack.sh-style truthiness: set, and not `""`/`"0"`/`"false"`. +fn env_truthy(value: Option) -> bool { + matches!(value.as_deref(), Some(v) if !v.is_empty() && v != "0" && v != "false") +} + +// --------------------------------------------------------------------------- +// Payload +// --------------------------------------------------------------------------- + +/// The wire payload. Field names match the ingest worker's contract exactly +/// (the worker renames `ci`→`is_ci` server-side). +/// +/// Agent and version facts are set here, client-side, rather than derived in +/// the worker from the User-Agent header: the CLI already holds both as +/// structured values (`is_ai_agent::detect()`, `CARGO_PKG_VERSION`), so +/// putting them in the payload avoids brittle string extraction on the +/// ingest side. The User-Agent still carries the same facts as transport +/// metadata for prefix-based request filtering. +#[derive(Debug, serde::Serialize)] +struct Payload { + command: String, + flags: Vec, + /// gh-style exit code (`Error::exit_code`): 0 success, 1 error, + /// 2 cancelled, 4 auth required. + exit_code: i32, + is_agent: bool, + /// Canonical id of the detected coding agent (e.g. "claude-code"); + /// `null` for human invocations. + agent: Option, + ci: bool, + version: &'static str, + os: &'static str, + arch: &'static str, +} + +fn build_payload(invocation: &Invocation, exit_code: i32, env: EnvLookup<'_>) -> Payload { + let mut flags = invocation.flags.clone(); + flags.truncate(MAX_FLAGS); + let detected = is_ai_agent::detect(); + Payload { + command: invocation.command.clone(), + flags, + exit_code, + is_agent: detected.is_some(), + agent: detected.map(|a| a.id.as_str().to_string()), + ci: env_truthy(env(CI_ENV)), + version: env!("CARGO_PKG_VERSION"), + os: std::env::consts::OS, + arch: std::env::consts::ARCH, + } +} + +// --------------------------------------------------------------------------- +// Invocation capture +// --------------------------------------------------------------------------- + +/// What the user invoked: the subcommand path (e.g. `"local start"`) and the +/// long names of the flags they passed. No values, no positionals. +pub struct Invocation { + command: String, + flags: Vec, +} + +/// Derive the command path and passed-flag names from the parsed matches. +/// +/// Only ids and `Arg` metadata are consulted — never `get_one`/`get_raw`/ +/// `get_many` — so argument *values* are structurally unreachable here. +/// Positionals are skipped entirely (their names could still describe user +/// data), default-valued and env-fed args are excluded by the +/// `ValueSource::CommandLine` filter, and clap's propagation of global flags +/// into subcommand matches is deduplicated by the set. +pub fn capture(root: &clap::Command, matches: &clap::ArgMatches) -> Invocation { + use clap::parser::ValueSource; + + let mut path: Vec<&str> = Vec::new(); + // Ancestor commands, innermost last: global args propagate into + // subcommand matches but their `Arg` definition lives on an ancestor. + let mut stack: Vec<&clap::Command> = vec![root]; + let mut flags = std::collections::BTreeSet::new(); + let mut current = matches; + loop { + for id in current.ids() { + // Global args are propagated upward into ancestor matches whose + // command doesn't define them; `value_source` would panic on such + // an id, so skip it here — it is captured again at the level that + // does define it (globals are propagated downward at build time). + if !matches!(current.try_contains_id(id.as_str()), Ok(true)) { + continue; + } + if current.value_source(id.as_str()) != Some(ValueSource::CommandLine) { + continue; + } + // Resolve the id to its definition; unresolvable ids (groups) are + // skipped rather than reported. + let Some(arg) = stack + .iter() + .rev() + .find_map(|cmd| cmd.get_arguments().find(|a| a.get_id() == id)) + else { + continue; + }; + if arg.is_positional() { + continue; + } + flags.insert(arg.get_long().unwrap_or(id.as_str()).to_string()); + } + let Some((name, sub_matches)) = current.subcommand() else { + break; + }; + let Some(sub_cmd) = stack + .last() + .expect("stack starts non-empty and only grows") + .find_subcommand(name) + else { + break; + }; + path.push(sub_cmd.get_name()); + stack.push(sub_cmd); + current = sub_matches; + } + Invocation { + command: path.join(" "), + flags: flags.into_iter().collect(), + } +} + +// --------------------------------------------------------------------------- +// Finalize (the per-invocation hook) +// --------------------------------------------------------------------------- + +/// What `finalize` should do for this invocation. Split from the side effects +/// so the state machine is unit-testable with injected env and paths. +#[derive(Debug, PartialEq, Eq)] +enum Action { + /// DO_NOT_TRACK, disabled, unwritable config dir: do nothing at all. + Silent, + /// First run, marker written successfully: show the notice, send nothing. + Notice, + /// Enabled: hand the serialized payload to the detached send child. + Send(String), + /// Enabled + debug: print the payload to stderr, send nothing. + Debug(String), +} + +fn decide(path: &Path, invocation: &Invocation, exit_code: i32, env: EnvLookup<'_>) -> Action { + if env_truthy(env(DNT_ENV)) { + return Action::Silent; + } + match load_state_from(path) { + State::Missing => { + // Write first, notice only on success: if the dir is unwritable + // we stay silent forever rather than nagging or erroring, and we + // never send without having recorded that the notice was shown. + if save_state_to(path, false).is_ok() { + Action::Notice + } else { + Action::Silent + } + } + State::Disabled => Action::Silent, + State::Enabled => { + let json = serde_json::to_string(&build_payload(invocation, exit_code, env)) + .expect("Payload serialization cannot fail"); + if env_truthy(env(DEBUG_ENV)) { + Action::Debug(json) + } else { + Action::Send(json) + } + } + } +} + +/// The telemetry hook, called once at the very end of `main` (after the +/// command has run, so `telemetry disable` silences its own event), with the +/// gh-style exit code the process is about to exit with. Never errors, never +/// blocks beyond spawning a detached child. +pub fn finalize(invocation: Invocation, exit_code: i32) { + let Some(path) = state_path() else { return }; + match decide(&path, &invocation, exit_code, &real_env_lookup) { + Action::Silent => {} + Action::Notice => print_first_run_notice(), + Action::Debug(json) => eprintln!("{json}"), + Action::Send(json) => spawn_send_child(&json), + } +} + +/// Printed to stderr regardless of TTY so agent/non-interactive usage still +/// sees it exactly once (stdout stays machine-parseable). +fn print_first_run_notice() { + eprintln!( + "\nNote: clickhousectl collects anonymous usage data to help improve the CLI:\n\ + command name, flag names (never values or arguments), success/failure, version,\n\ + OS/arch, and CI/agent detection. No user or machine IDs. Nothing was sent this run.\n\ + Opt out: `clickhousectl telemetry disable` or DO_NOT_TRACK=1.\n\ + Details: {DOCS_URL}" + ); +} + +// --------------------------------------------------------------------------- +// Transport +// --------------------------------------------------------------------------- + +/// Re-invoke this binary as `clickhousectl telemetry send` with the payload +/// in the child's environment, all stdio nulled, and never wait: the parent +/// exits immediately and the child dies silently on any failure. +fn spawn_send_child(payload_json: &str) { + use std::process::{Command, Stdio}; + + let Ok(exe) = std::env::current_exe() else { + return; + }; + let _ = Command::new(exe) + .args(["telemetry", "send"]) + .env(PAYLOAD_ENV, payload_json) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn(); +} + +/// The hidden send child's entire job: one POST, short timeout, ignore every +/// failure. Short-circuited in `main` before the update-cache refresh and the +/// telemetry hook, so a send can never trigger another send. +pub async fn run_child_send() { + let Ok(payload) = std::env::var(PAYLOAD_ENV) else { + // Invoked without a payload (directly by a user): do nothing. + return; + }; + let url = std::env::var(URL_ENV).unwrap_or_else(|_| DEFAULT_ENDPOINT.to_string()); + let Ok(client) = crate::http::client_builder().timeout(SEND_TIMEOUT).build() else { + return; + }; + let _ = client + .post(&url) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .body(payload) + .send() + .await; +} + +// --------------------------------------------------------------------------- +// `clickhousectl telemetry` subcommand +// --------------------------------------------------------------------------- + +pub fn run_command(cmd: crate::cli::TelemetryCommands) -> Result<()> { + use crate::cli::TelemetryCommands; + + match cmd { + TelemetryCommands::Enable => { + set_disabled(false)?; + println!("Telemetry enabled."); + Ok(()) + } + TelemetryCommands::Disable => { + set_disabled(true)?; + println!("Telemetry disabled."); + Ok(()) + } + TelemetryCommands::Status => { + print_status(); + Ok(()) + } + TelemetryCommands::Send => unreachable!("handled before dispatch in main"), + } +} + +fn set_disabled(disabled: bool) -> Result<()> { + let path = state_path().ok_or_else(|| { + Error::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + "Could not determine home directory", + )) + })?; + save_state_to(&path, disabled).map_err(Error::Io) +} + +fn print_status() { + if env_truthy(real_env_lookup(DNT_ENV)) { + println!("Telemetry is disabled (DO_NOT_TRACK environment variable is set)."); + return; + } + let Some(path) = state_path() else { + println!("Telemetry is disabled (could not determine home directory)."); + return; + }; + match load_state_from(&path) { + State::Missing => { + println!("Telemetry is not yet configured (first-run notice pending)."); + } + State::Disabled => { + println!("Telemetry is disabled ({}).", path.display()); + } + State::Enabled => { + println!( + "Telemetry is enabled. Disable with `clickhousectl telemetry disable` or DO_NOT_TRACK=1.\nDetails: {DOCS_URL}" + ); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use clap::CommandFactory; + + /// Env lookup over a synthetic map; `set_var` is unsafe in edition 2024. + fn env_of(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let map: std::collections::HashMap = pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + move |key: &str| map.get(key).cloned() + } + + fn invocation() -> Invocation { + Invocation { + command: "local list".into(), + flags: vec!["json".into()], + } + } + + #[test] + fn env_truthy_truth_table() { + assert!(!env_truthy(None)); + assert!(!env_truthy(Some("".into()))); + assert!(!env_truthy(Some("0".into()))); + assert!(!env_truthy(Some("false".into()))); + assert!(env_truthy(Some("1".into()))); + assert!(env_truthy(Some("true".into()))); + assert!(env_truthy(Some("anything".into()))); + } + + #[test] + fn do_not_track_wins_over_everything() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + // Even with an enabled state file present, DNT is fully silent. + save_state_to(&path, false).unwrap(); + let env = env_of(&[("DO_NOT_TRACK", "1")]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + } + + #[test] + fn do_not_track_prevents_first_run_marker() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + let env = env_of(&[("DO_NOT_TRACK", "1")]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + assert!(!path.exists(), "DNT must not write the marker file"); + } + + #[test] + fn first_run_writes_marker_and_notices_without_sending() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + let env = env_of(&[]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Notice); + let contents = std::fs::read_to_string(&path).unwrap(); + assert_eq!(contents, r#"{"disabled":false}"#); + } + + #[test] + fn unwritable_dir_fails_open_to_silent() { + // Parent path is a file, so create_dir_all fails. + let dir = tempfile::tempdir().unwrap(); + let blocker = dir.path().join("blocker"); + std::fs::write(&blocker, "").unwrap(); + let path = blocker.join("telemetry.json"); + let env = env_of(&[]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + // And again: still silent, never a notice, never a send. + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + } + + #[test] + fn disabled_state_is_silent() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + save_state_to(&path, true).unwrap(); + let env = env_of(&[]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + } + + #[test] + fn corrupt_state_file_is_treated_as_disabled() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + std::fs::write(&path, "not json{{").unwrap(); + assert_eq!(load_state_from(&path), State::Disabled); + let env = env_of(&[]); + assert_eq!(decide(&path, &invocation(), 0, &env), Action::Silent); + } + + #[test] + fn enabled_state_sends_payload() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + save_state_to(&path, false).unwrap(); + let env = env_of(&[("CI", "1")]); + let Action::Send(json) = decide(&path, &invocation(), 4, &env) else { + panic!("expected Send"); + }; + let value: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(value["command"], "local list"); + assert_eq!(value["flags"], serde_json::json!(["json"])); + assert_eq!(value["exit_code"], 4); + assert_eq!(value["ci"], true); + assert_eq!(value["version"], env!("CARGO_PKG_VERSION")); + assert_eq!(value["os"], std::env::consts::OS); + assert_eq!(value["arch"], std::env::consts::ARCH); + } + + #[test] + fn debug_env_prints_instead_of_sending() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("telemetry.json"); + save_state_to(&path, false).unwrap(); + let env = env_of(&[("CHCTL_TELEMETRY_DEBUG", "1")]); + assert!(matches!( + decide(&path, &invocation(), 0, &env), + Action::Debug(_) + )); + } + + #[test] + fn payload_serializes_exactly_the_wire_fields() { + let payload = build_payload(&invocation(), 0, &env_of(&[])); + let value = serde_json::to_value(&payload).unwrap(); + let keys: Vec<&str> = value.as_object().unwrap().keys().map(|k| k.as_str()).collect(); + assert_eq!( + keys, + ["command", "flags", "exit_code", "is_agent", "agent", "ci", "version", "os", "arch"] + ); + // The two agent fields are set from the same single detection and can + // never disagree. + assert_eq!(value["is_agent"].as_bool().unwrap(), !value["agent"].is_null()); + } + + #[test] + fn flags_truncated_to_worker_cap() { + let inv = Invocation { + command: "x".into(), + flags: (0..100).map(|i| format!("flag-{i}")).collect(), + }; + let payload = build_payload(&inv, 0, &env_of(&[])); + assert_eq!(payload.flags.len(), MAX_FLAGS); + } + + // -- capture: values are structurally unreachable ------------------------ + + fn capture_from(args: &[&str]) -> Invocation { + let mut cmd = crate::cli::Cli::command(); + let matches = cmd.try_get_matches_from_mut(args).unwrap(); + capture(&cmd, &matches) + } + + #[test] + fn capture_reports_names_only_never_values_or_positionals() { + let inv = capture_from(&[ + "clickhousectl", + "cloud", + "--json", + "service", + "get", + "SECRET-SERVICE-ID", + "--org-id", + "SECRET-ORG", + ]); + assert_eq!(inv.command, "cloud service get"); + assert_eq!(inv.flags, ["json", "org-id"]); + let json = serde_json::to_string(&build_payload(&inv, 0, &env_of(&[]))).unwrap(); + assert!(!json.contains("SECRET"), "payload leaked a value: {json}"); + } + + #[test] + fn capture_dedupes_propagated_global_flags() { + let inv = capture_from(&["clickhousectl", "cloud", "--json", "service", "list"]); + assert_eq!(inv.command, "cloud service list"); + assert_eq!(inv.flags, ["json"]); + } + + #[test] + fn capture_excludes_default_valued_args() { + use clap::{Arg, ArgAction, Command}; + let mut cmd = Command::new("root").subcommand( + Command::new("sub") + .arg(Arg::new("level").long("level").default_value("info")) + .arg(Arg::new("verbose").long("verbose").action(ArgAction::SetTrue)) + .arg(Arg::new("target")), + ); + let matches = cmd + .try_get_matches_from_mut(["root", "sub", "--verbose", "user-data"]) + .unwrap(); + let inv = capture(&cmd, &matches); + assert_eq!(inv.command, "sub"); + // `level` has a default (ValueSource::DefaultValue) and `target` is + // positional — only the explicitly passed named flag is reported. + assert_eq!(inv.flags, ["verbose"]); + } + + #[test] + fn capture_with_no_flags_is_empty() { + let inv = capture_from(&["clickhousectl", "local", "list"]); + assert_eq!(inv.command, "local list"); + assert!(inv.flags.is_empty()); + } + + #[test] + fn state_path_is_telemetry_json_under_base_dir() { + let path = state_path().unwrap(); + assert!(path.ends_with(".clickhouse/telemetry.json")); + } +} diff --git a/crates/clickhousectl/tests/cli_request_shape_test.rs b/crates/clickhousectl/tests/cli_request_shape_test.rs index 205e390..35431d3 100644 --- a/crates/clickhousectl/tests/cli_request_shape_test.rs +++ b/crates/clickhousectl/tests/cli_request_shape_test.rs @@ -84,7 +84,11 @@ async fn invoke_cli_capture_body(mock: &MockServer, cli_args: &[&str]) -> Value full_args.push("--json"); full_args.extend(cli_args); + // DO_NOT_TRACK (here and on every other spawn in this file) keeps the + // binary's telemetry fully silent: no `~/.clickhouse/telemetry.json` write + // in the developer's real home, no POST to the production endpoint. let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args(&full_args) .env("CLICKHOUSE_CLOUD_API_KEY", "fake-key-for-tests") .env("CLICKHOUSE_CLOUD_API_SECRET", "fake-secret-for-tests") @@ -1220,6 +1224,7 @@ async fn dotenv_creds_produce_basic_auth_request() { let url = mock.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args(["cloud", "--url", &url, "--json", "org", "list"]) .current_dir(dir.path()) .env_remove("CLICKHOUSE_CLOUD_API_KEY") @@ -1319,6 +1324,7 @@ async fn service_query_with_oauth_sends_bearer_and_never_provisions() { let url = control.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args([ "cloud", "--url", @@ -1408,6 +1414,7 @@ async fn service_query_with_stored_key_sends_basic_auth_with_that_key() { let url = control.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args([ "cloud", "--url", @@ -1516,6 +1523,7 @@ async fn service_query_resends_with_wake_header_when_service_is_idle() { let url = control.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args([ "cloud", "--url", @@ -1580,6 +1588,7 @@ async fn service_query_fails_with_start_hint_when_service_is_stopped() { let url = control.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args([ "cloud", "--url", @@ -1646,6 +1655,7 @@ async fn shell_env_overrides_dotenv_creds_in_request() { let url = mock.uri(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args(["cloud", "--url", &url, "--json", "org", "list"]) .current_dir(dir.path()) .env("CLICKHOUSE_CLOUD_API_KEY", "shell-key") @@ -1702,6 +1712,7 @@ async fn agent_session_and_trace_headers_are_forwarded() { let url = mock.uri(); let traceparent = "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"; let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .args(["cloud", "--url", &url, "--json", "org", "list"]) .env("CLICKHOUSE_CLOUD_API_KEY", "fake-key-for-tests") .env("CLICKHOUSE_CLOUD_API_SECRET", "fake-secret-for-tests") diff --git a/crates/clickhousectl/tests/local_install_local_first_test.rs b/crates/clickhousectl/tests/local_install_local_first_test.rs index 5e6c4fb..fcf02bd 100644 --- a/crates/clickhousectl/tests/local_install_local_first_test.rs +++ b/crates/clickhousectl/tests/local_install_local_first_test.rs @@ -33,6 +33,7 @@ fn local_install_minor_with_existing_match_does_not_hit_network() { std::fs::set_permissions(&binary, perms).unwrap(); let output = Command::new(clickhousectl_binary()) + .env("DO_NOT_TRACK", "1") .env("HOME", tempdir.path()) .args(["local", "install", "25.12", "--json"]) .output() diff --git a/crates/clickhousectl/tests/telemetry_test.rs b/crates/clickhousectl/tests/telemetry_test.rs new file mode 100644 index 0000000..7d64833 --- /dev/null +++ b/crates/clickhousectl/tests/telemetry_test.rs @@ -0,0 +1,379 @@ +//! Telemetry end-to-end tests (issue #283). +//! +//! Each test invokes the real `clickhousectl` binary as a subprocess with +//! `HOME` pointed at a temp dir (sandboxing `~/.clickhouse/telemetry.json`) +//! and `CHCTL_TELEMETRY_URL` pointed at a local `wiremock` server, then +//! asserts on the consent flow (notice/marker/silence) and on the recorded +//! payload shape — in particular that flag values and positional arguments +//! never appear on the wire. +//! +//! The send happens in a detached child process, so tests that expect an +//! event poll the mock briefly; tests that expect *no* event give the +//! (nonexistent) child a moment before asserting zero requests. +//! +//! cargo test -p clickhousectl --test telemetry_test + +#![cfg(feature = "telemetry")] + +use std::path::PathBuf; +use std::process::{Command, Output}; +use std::time::{Duration, Instant}; + +use serde_json::Value; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +fn clickhousectl_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_clickhousectl")) +} + +/// A sandboxed home directory plus the telemetry ingest mock. +struct Sandbox { + home: tempfile::TempDir, + mock: MockServer, +} + +impl Sandbox { + async fn new() -> Self { + let mock = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/telemetry")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true})), + ) + .mount(&mock) + .await; + Sandbox { + home: tempfile::tempdir().unwrap(), + mock, + } + } + + fn state_path(&self) -> PathBuf { + self.home.path().join(".clickhouse").join("telemetry.json") + } + + fn write_state(&self, disabled: bool) { + let dir = self.state_path(); + std::fs::create_dir_all(dir.parent().unwrap()).unwrap(); + std::fs::write(&dir, format!(r#"{{"disabled":{disabled}}}"#)).unwrap(); + } + + /// Run the binary sandboxed: `HOME` at the temp dir, telemetry pointed at + /// the mock, and every env var that would alter the consent flow or the + /// payload cleared for determinism (the harness itself may run under CI + /// or a coding agent). + fn command(&self, args: &[&str]) -> Command { + let mut cmd = Command::new(clickhousectl_binary()); + cmd.args(args) + .env("HOME", self.home.path()) + .env( + "CHCTL_TELEMETRY_URL", + format!("{}/v1/telemetry", self.mock.uri()), + ) + .env_remove("DO_NOT_TRACK") + .env_remove("CHCTL_TELEMETRY_DEBUG") + .env_remove("CHCTL_TELEMETRY_PAYLOAD") + .env_remove("CI"); + cmd + } + + fn run(&self, args: &[&str]) -> Output { + self.command(args).output().expect("failed to spawn binary") + } + + /// Poll until the mock has seen `n` requests; panics after ~5s. The send + /// child is detached, so arrival is asynchronous. + async fn wait_for_requests(&self, n: usize) -> Vec { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + let requests = self.mock.received_requests().await.unwrap_or_default(); + if requests.len() >= n { + return requests + .iter() + .map(|r| serde_json::from_slice(&r.body).expect("payload must be JSON")) + .collect(); + } + assert!( + Instant::now() < deadline, + "telemetry event did not arrive within 5s (saw {})", + requests.len() + ); + tokio::time::sleep(Duration::from_millis(100)).await; + } + } + + /// Assert the mock saw no requests, giving a hypothetical stray child a + /// moment to fire first. + async fn assert_no_requests(&self) { + tokio::time::sleep(Duration::from_millis(750)).await; + let requests = self.mock.received_requests().await.unwrap_or_default(); + assert!( + requests.is_empty(), + "expected no telemetry, saw: {:?}", + requests.iter().map(|r| &r.body).collect::>() + ); + } +} + +fn stderr_of(output: &Output) -> String { + String::from_utf8_lossy(&output.stderr).into_owned() +} + +fn stdout_of(output: &Output) -> String { + String::from_utf8_lossy(&output.stdout).into_owned() +} + +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn first_run_writes_marker_prints_notice_sends_nothing() { + let sandbox = Sandbox::new().await; + + let output = sandbox.run(&["local", "list"]); + assert!(output.status.success()); + + let stderr = stderr_of(&output); + assert!( + stderr.contains("anonymous usage data") && stderr.contains("telemetry disable"), + "first run must print the notice, got stderr: {stderr}" + ); + assert_eq!( + std::fs::read_to_string(sandbox.state_path()).unwrap(), + r#"{"disabled":false}"# + ); + sandbox.assert_no_requests().await; + + // Second run: the notice appears exactly once, ever. + let output = sandbox.run(&["local", "list"]); + assert!(!stderr_of(&output).contains("anonymous usage data")); +} + +#[tokio::test] +async fn enabled_run_sends_payload_with_expected_shape() { + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + let output = sandbox.run(&["local", "list"]); + assert!(output.status.success()); + + let payloads = sandbox.wait_for_requests(1).await; + let event = &payloads[0]; + assert_eq!(event["command"], "local list"); + assert!(event["flags"].as_array().unwrap().is_empty()); + assert_eq!(event["exit_code"], 0); + // Whether an agent is detected depends on the harness environment; pin + // that the two fields exist and agree (one detection feeds both). + assert!(event["is_agent"].is_boolean()); + assert_eq!( + event["is_agent"].as_bool().unwrap(), + event["agent"].is_string(), + "is_agent and agent must come from the same detection: {event}" + ); + assert_eq!(event["ci"], false); + assert_eq!(event["version"], env!("CARGO_PKG_VERSION")); + assert_eq!(event["os"], std::env::consts::OS); + assert_eq!(event["arch"], std::env::consts::ARCH); + + // The send goes through the canonical http::client_builder(), so it + // carries the same User-Agent as every other outbound request. The + // ingest worker relies on the `clickhousectl/` prefix to + // reject non-CLI traffic (an ` (agent=...)` comment may follow). + let requests = sandbox.mock.received_requests().await.unwrap(); + let ua = requests[0] + .headers + .get("user-agent") + .expect("telemetry POST must carry a User-Agent") + .to_str() + .unwrap(); + let prefix = format!("clickhousectl/{}", env!("CARGO_PKG_VERSION")); + assert!( + ua == prefix || ua.starts_with(&format!("{prefix} (")), + "unexpected User-Agent: {ua}" + ); +} + +#[tokio::test] +async fn failure_reported_and_positional_value_never_leaks() { + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + let output = sandbox.run(&["local", "remove", "no-such-version-xyz"]); + assert!(!output.status.success()); + + let payloads = sandbox.wait_for_requests(1).await; + let event = &payloads[0]; + assert_eq!(event["command"], "local remove"); + // The event carries the gh-style exit code the process exited with. + assert_eq!(event["exit_code"], 1); + assert_eq!(event["exit_code"], output.status.code().unwrap()); + let raw = serde_json::to_string(event).unwrap(); + assert!( + !raw.contains("no-such-version-xyz"), + "positional argument leaked into the payload: {raw}" + ); +} + +#[tokio::test] +async fn flag_names_sent_but_values_never_leak() { + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + // --json is a real flag; its name may appear but the CI-style value + // asserts cover named flags with values via the unit tests. Here we pin + // the end-to-end shape: flags is an array of known names only. + let output = sandbox.run(&["local", "--json", "list"]); + assert!(output.status.success()); + + let payloads = sandbox.wait_for_requests(1).await; + let event = &payloads[0]; + assert_eq!(event["command"], "local list"); + assert_eq!(event["flags"], serde_json::json!(["json"])); +} + +#[tokio::test] +async fn do_not_track_is_fully_silent() { + let sandbox = Sandbox::new().await; + + let output = sandbox + .command(&["local", "list"]) + .env("DO_NOT_TRACK", "1") + .output() + .unwrap(); + assert!(output.status.success()); + + assert!(!stderr_of(&output).contains("anonymous usage data")); + assert!( + !sandbox.state_path().exists(), + "DO_NOT_TRACK must not write the marker file" + ); + sandbox.assert_no_requests().await; +} + +#[tokio::test] +async fn disable_persists_and_silences() { + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + let output = sandbox.run(&["telemetry", "disable"]); + assert!(output.status.success()); + assert!(stdout_of(&output).contains("Telemetry disabled.")); + assert_eq!( + std::fs::read_to_string(sandbox.state_path()).unwrap(), + r#"{"disabled":true}"# + ); + + let output = sandbox.run(&["local", "list"]); + assert!(output.status.success()); + assert!(!stderr_of(&output).contains("anonymous usage data")); + + let output = sandbox.run(&["telemetry", "status"]); + assert!(stdout_of(&output).contains("disabled")); + + // Neither the disable itself, nor anything after it, sent an event. + sandbox.assert_no_requests().await; +} + +#[tokio::test] +async fn enable_sends_an_event_for_itself() { + let sandbox = Sandbox::new().await; + sandbox.write_state(true); + + let output = sandbox.run(&["telemetry", "enable"]); + assert!(output.status.success()); + assert!(stdout_of(&output).contains("Telemetry enabled.")); + + // Consent is evaluated after the command ran, so the enable run itself + // is the first event. + let payloads = sandbox.wait_for_requests(1).await; + assert_eq!(payloads[0]["command"], "telemetry enable"); +} + +#[tokio::test] +async fn debug_mode_prints_payload_without_sending() { + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + let output = sandbox + .command(&["local", "list"]) + .env("CHCTL_TELEMETRY_DEBUG", "1") + .output() + .unwrap(); + assert!(output.status.success()); + + let stderr = stderr_of(&output); + assert!( + stderr.contains(r#""command":"local list""#), + "debug mode must print the payload to stderr, got: {stderr}" + ); + sandbox.assert_no_requests().await; +} + +#[cfg(unix)] +#[tokio::test] +async fn unwritable_home_fails_open_to_silent() { + use std::os::unix::fs::PermissionsExt; + + let sandbox = Sandbox::new().await; + let perms = std::fs::Permissions::from_mode(0o555); + std::fs::set_permissions(sandbox.home.path(), perms).unwrap(); + + // Twice: silent every run, never a repeated notice, never an error. + for _ in 0..2 { + let output = sandbox.run(&["telemetry", "status"]); + assert!(output.status.success()); + assert!(!stderr_of(&output).contains("anonymous usage data")); + assert!(stdout_of(&output).contains("not yet configured")); + } + assert!(!sandbox.state_path().exists()); + sandbox.assert_no_requests().await; + + // Restore so TempDir cleanup can delete the directory. + let perms = std::fs::Permissions::from_mode(0o755); + std::fs::set_permissions(sandbox.home.path(), perms).unwrap(); +} + +#[tokio::test] +async fn parent_never_waits_for_a_slow_endpoint() { + let mock = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/telemetry")) + .respond_with(ResponseTemplate::new(200).set_delay(Duration::from_secs(10))) + .mount(&mock) + .await; + + let sandbox = Sandbox::new().await; + sandbox.write_state(false); + + let started = Instant::now(); + let output = sandbox + .command(&["local", "list"]) + .env("CHCTL_TELEMETRY_URL", format!("{}/v1/telemetry", mock.uri())) + .output() + .unwrap(); + let elapsed = started.elapsed(); + + assert!(output.status.success()); + // The send child is detached; the parent must return well before the + // mock's 10s delay (generous bound to absorb slow CI machines). + assert!( + elapsed < Duration::from_secs(5), + "parent waited on the telemetry send: {elapsed:?}" + ); +} + +/// Check the marker file used by `Sandbox::state_path` matches what the +/// binary actually writes — guards against the test suite silently diverging +/// from the real path. +#[tokio::test] +async fn marker_lives_in_dot_clickhouse_telemetry_json() { + let sandbox = Sandbox::new().await; + let output = sandbox.run(&["local", "list"]); + assert!(output.status.success()); + assert!(sandbox.state_path().exists()); + let entries: Vec<_> = std::fs::read_dir(sandbox.home.path().join(".clickhouse")) + .unwrap() + .map(|e| e.unwrap().file_name().into_string().unwrap()) + .collect(); + assert!(entries.contains(&"telemetry.json".to_string()), "{entries:?}"); +}