From 74cd5a70abf4c2d31fca3054595c22f27055919e Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Fri, 9 Oct 2026 18:52:42 +0300 Subject: [PATCH 1/4] Correct the remaining counts and names in docs/commands.md --- docs/commands.md | 13 +++++++------ src/output/render.rs | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index b4012cd..bd1d2e8 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1,6 +1,6 @@ # Implemented commands -Every command the CLI ships: five auth commands, 39 API operations (35 across 9 +Every command the CLI ships, including five auth commands, 39 API operations (35 across 9 command groups, plus `directions`, `isochrone`, `map-match` and `matrix`), the tilesets-cli proxy, `completion` and `generate-skills`. Each is shown in both of its renderings. Which one you get is decided by `--output`, whose default @@ -11,8 +11,8 @@ gets the right one. See Account names, style ids and tokens in the examples are replaced; everything else is as the API sent it. -**35 of the 39 were run against the live API and show what came back:** 25 -on 2026-09-01, `fonts list`, `fonts upload` and `fonts delete` on +**35 of the 39 were run against the live API and show what came back:** 24 +on 2026-09-01, `styles download` by 2026-10-01, `fonts list`, `fonts upload` and `fonts delete` on 2026-09-08, once `fonts:list`/`fonts:write` became registrable, `directions`, `isochrone` and `map-match` on 2026-09-23, `matrix`, `feedback list` and `feedback get` on 2026-09-24, and `feedback create` on @@ -121,6 +121,7 @@ nests, and is typed `mapbox styles draft get`. [styles.get](#mapbox-styles-get) · [styles.create](#mapbox-styles-create) · [styles.update](#mapbox-styles-update) · [styles.delete](#mapbox-styles-delete) · +[styles.download](#mapbox-styles-download) · [styles.draft.get](#mapbox-styles-draft-get) · [styles.draft.update](#mapbox-styles-draft-update) · [styles.draft.delete](#mapbox-styles-draft-delete) @@ -674,8 +675,8 @@ properties directly. A conforming response never reaches that case: `geocoder` requires `name`/`feature_type` on every feature, `tilesets query` requires `tilequery.layer`. -Two of the nine command groups can answer with bytes — `static` and -`tilesets`. Those bypass `--output` in both modes: +Three of the nine command groups can answer with bytes — `static`, +`styles` (`download`) and `tilesets`. Those bypass `--output` in both modes: @@ -1600,7 +1601,7 @@ mapbox isochrone mapbox/walking "-122.42,37.78" --contours-minutes 5,10 --polygo Captured live against `mapbox/walking`, two 5- and 10-minute contours as polygons. This response is a real GeoJSON `FeatureCollection`, unlike `mapbox directions`'s response, but isochrone isn't one of the three -services (`search`, `geocoder`, `tilequery`) this CLI has a bespoke +services (`search`, `geocoder`, `tilesets`) this CLI has a bespoke list-per-feature rendering for yet (`output/render.rs`'s `list_rendering` is an exact service allow-list, not a "looks like GeoJSON" test), so both output modes print the same JSON, `-o text` pretty-printed and `-o json` on one diff --git a/src/output/render.rs b/src/output/render.rs index 0a644af..eda817b 100644 --- a/src/output/render.rs +++ b/src/output/render.rs @@ -19,7 +19,7 @@ use super::style; /// /// The names are `Operation::service`, which is the command group, not the /// spec file: `tilesets query` comes from the tilequery spec but answers as -/// `tilesets`. An arm still named `tilequery` after #116 moved the command +/// `tilesets`. An arm still named `tilequery` after the 0.2.0 rename moved the command /// silently dropped its list for every release since; see /// `every_listed_service_is_a_real_command_group`. `tilesets`'s other /// commands answer with tile bytes, which never reach here. @@ -1785,7 +1785,7 @@ mod tests { /// The match is on a string, so a command group renamed or merged in the /// specs leaves its arm unreachable without a warning — the tests above - /// pass that string straight in and keep passing. #116 moved + /// pass that string straight in and keep passing. The 0.2.0 rename moved /// `tilequery get` to `tilesets query` exactly that way. Checking each /// name against the bundled specs is what ties the arm to a real command. #[test] From d69b75afdfb08aaf57bab8de4e3e1e40e20af794 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Fri, 9 Oct 2026 18:36:32 +0300 Subject: [PATCH 2/4] Record and send one cli.command telemetry event per run Each run builds one cli.command event from the run record, under the privacy rules reviewed in mapbox/event-schema#300: a value only for flags, fixed choices, numbers and language, country or feature-type codes; coordinates and tile addresses by name only; free strings as lengths; --data by the keys its spec declares; a userId replaced daily; no request ids. A detached child sends it to production Mapbox Events, so the command never waits. cli_token::send tries tokens in the order its caller declares: the user's --token or MAPBOX_ACCESS_TOKEN, then the login (refreshed only when about to expire), then the CLI's own token (MAPBOX_CLI_TOKEN, then a pk. token compiled in from MAPBOX_CLI_BUNDLED_TOKEN; build.rs refuses anything else). With no token the event is dropped. MAPBOX_CLI_NO_TELEMETRY=1 turns it off, and runs under sudo record nothing. MAPBOX_INTERNAL_TELEMETRY_URL redirects the send in builds that are not a production release, and MAPBOX_INTERNAL_TELEMETRY_LOG logs each delivery. On Windows, detached children no longer hold the caller's stdout pipe open. --- CHANGELOG.md | 9 + Cargo.lock | 1 + Cargo.toml | 10 +- README.md | 20 + build.rs | 16 + src/auth.rs | 7 +- src/cli_token.rs | 489 ++++++++++++++ src/detach.rs | 57 ++ src/executor.rs | 1 + src/http.rs | 2 +- src/main.rs | 29 +- src/run_record.rs | 30 +- src/spec.rs | 98 +++ src/telemetry.rs | 15 + src/telemetry_event.rs | 1253 +++++++++++++++++++++++++++++++++++ src/telemetry_sink.rs | 384 +++++++++++ src/update_check.rs | 14 +- tests/auth_profiles.rs | 3 + tests/completion.rs | 3 + tests/config.rs | 3 + tests/diagnostic_log.rs | 3 + tests/docs_contract.rs | 2 + tests/dry_run.rs | 3 + tests/generate_skills.rs | 3 + tests/history.rs | 2 + tests/mcp_proxy.rs | 15 + tests/non_interactive.rs | 5 + tests/output_contract.rs | 10 + tests/proxy.rs | 3 + tests/schema_contract.rs | 3 + tests/source_guards.rs | 30 + tests/telemetry_events.rs | 690 +++++++++++++++++++ tests/tilesets_cli_proxy.rs | 22 + 33 files changed, 3208 insertions(+), 27 deletions(-) create mode 100644 src/cli_token.rs create mode 100644 src/detach.rs create mode 100644 src/telemetry_event.rs create mode 100644 src/telemetry_sink.rs create mode 100644 tests/telemetry_events.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index da87b8a..7428211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -260,6 +260,15 @@ that may never merge. They are not releases and are not listed here. code; it does find a `~/.mapbox/history` directory it didn't before, and `mapbox config list` now reports a second key, `history`. +- Each run sends one `cli.command` telemetry event to Mapbox, from a + background process the command doesn't wait for, with your own token + (`--token`, `MAPBOX_ACCESS_TOKEN` or your login) or, when you have none, + one built into the CLI. A build from source has no built-in token, so with + no token of your own the event is dropped. It never touches stdout, the + exit code or how long a command takes, so a script sees no difference; a + machine that already has `~/.mapbox` finds a `.telemetry` directory in it. + `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. + - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API parameter this CLI's specs don't declare a flag for. diff --git a/Cargo.lock b/Cargo.lock index 6473342..318313f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -803,6 +803,7 @@ dependencies = [ "tar", "terminal-colorsaurus", "terminal_size", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 08fc692..714ae54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,8 +37,9 @@ path = "src/main.rs" # until somebody writes the obvious thing at two in the morning. [lints.rust] # There is no `unsafe` in this crate outside a couple of `env::set_var` calls -# in tests, and there is no reason for there to be: nothing here does FFI, -# manual memory management or lock-free anything. Denying it means adding some +# in tests and two Win32 calls in `src/detach.rs` (the reason is there), and +# there is no reason for more: nothing else here does FFI, manual memory +# management or lock-free anything. Denying it means adding some # is a deliberate `#[allow]` with a reason next to it rather than a diff nobody # looks twice at. unsafe_code = "deny" @@ -111,6 +112,11 @@ jsonc-parser = { version = "0.34.0", features = ["serde", "serde_json"] } # that shows help; see src/output/theme.rs. terminal-colorsaurus = "1" +[target.'cfg(windows)'.dependencies] +# For `src/detach.rs`: keeping the parent's standard handles out of a detached +# child. Already in the graph through tokio and others; the same version. +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_System_Console"] } + [dev-dependencies] # Integration tests build fake Mapbox tokens; same crate the CLI already uses. base64 = "0.22" diff --git a/README.md b/README.md index 5386e99..0a66cfa 100644 --- a/README.md +++ b/README.md @@ -285,6 +285,18 @@ Every request sends `User-Agent: mapbox-cli/` and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` keeps even future markers out of that header. +Each run also sends one event to Mapbox: the command's name, its options +(a value only when it comes from a fixed list, otherwise just its length or +size, and for a JSON body only the field names the API defines), how it +ended, and how long it took — never a token, a file path or free text you +typed. It carries a random ID that is replaced on the first run a day after +it was created. It is sent in the background, so a command never waits for +it, and dropped if it can't be delivered. It is sent with your own token +(`--token`, `MAPBOX_ACCESS_TOKEN` or your login) when you have one, and with +a token built into the CLI otherwise. A build from source has no built-in +token, so with no token of your own the event is dropped. +`MAPBOX_CLI_NO_TELEMETRY=1` turns it off. + ### Diagnostics and settings ```sh @@ -451,6 +463,14 @@ process sends, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API parameter this CLI's specs don't declare a flag for. `--debug` and `--dry-run` show it alongside everything else on the request. +### The CLI's own requests + +Requests the CLI makes for itself rather than for your commands use your own +token whenever you have one: `--token` or `MAPBOX_ACCESS_TOKEN`, then your +login. Only when you have none does it use the CLI's token: +`MAPBOX_CLI_TOKEN`, or one built into the binary. The CLI's token is only +used for what `src/cli_token.rs` lists for it, today just telemetry. + ### Proxies `HTTPS_PROXY`, `HTTP_PROXY`, `ALL_PROXY` and `NO_PROXY` are all honored, so diff --git a/build.rs b/build.rs index 41451f3..e22d60c 100644 --- a/build.rs +++ b/build.rs @@ -41,6 +41,8 @@ use std::path::Path; fn main() { + check_bundled_token(); + let manifest = Path::new("../internal/openapi-command-config/PINNED_SOURCE"); // The vendored specs and their provenance are ordinary build inputs now, @@ -91,6 +93,20 @@ fn main() { warn_if_stale(committed_at); } +/// `src/cli_token.rs` compiles `MAPBOX_CLI_BUNDLED_TOKEN` into the binary, +/// where `strings` can read it. Only a public `pk.` token may go there, so +/// anything else fails the build rather than shipping a secret. +fn check_bundled_token() { + println!("cargo:rerun-if-env-changed=MAPBOX_CLI_BUNDLED_TOKEN"); + let Ok(token) = std::env::var("MAPBOX_CLI_BUNDLED_TOKEN") else { + return; + }; + let token = token.trim(); + if !token.is_empty() && !token.starts_with("pk.") { + panic!("MAPBOX_CLI_BUNDLED_TOKEN must be a public pk. token"); + } +} + /// New API surface lands in `openapi-specs` at something closer to a monthly /// rate, and the maintainer-only sync that regenerates `openapi/` from it /// runs weekly, so two weeks without one is already long enough to be worth a diff --git a/src/auth.rs b/src/auth.rs index 0a0db62..e4a6473 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -479,8 +479,9 @@ fn credentials_path_readonly(profile: Option<&str>) -> Option { /// directory as a side effect of reading it — see /// [`credentials_path_readonly`]. What [`profiles`] reads each stored /// profile through, since listing what exists must not be the reason a -/// directory starts to exist or its permissions change. -fn load_credentials_readonly(profile: Option<&str>) -> Option { +/// directory starts to exist or its permissions change, and what +/// [`crate::cli_token`] reads the login through for the same reason. +pub(crate) fn load_credentials_readonly(profile: Option<&str>) -> Option { let data = std::fs::read_to_string(credentials_path_readonly(profile)?).ok()?; serde_json::from_str(&data).ok() } @@ -717,7 +718,7 @@ pub fn warn_if_environment_token_shadows_login(profile: Option<&str>, remedy: &s } } -fn token_needs_refresh(token: &str) -> bool { +pub(crate) fn token_needs_refresh(token: &str) -> bool { match token_expires_at(token) { Some(exp) => { let now = std::time::SystemTime::now() diff --git a/src/cli_token.rs b/src/cli_token.rs new file mode 100644 index 0000000..c98257c --- /dev/null +++ b/src/cli_token.rs @@ -0,0 +1,489 @@ +//! Requests the CLI makes for itself rather than for the user, and the token +//! they carry. +//! +//! The user's own token always comes first. The CLI's token exists only as +//! the fallback for someone who has none, so it is never sent while the user +//! has one that works. Tried highest first: +//! +//! 1. `--token` or `MAPBOX_ACCESS_TOKEN`, ranked the way a command ranks them +//! (see [`user_token`]). +//! 2. The stored login, loaded the way a command loads it, refresh included. +//! 3. `MAPBOX_CLI_TOKEN` at run time, the CLI's token for someone with none +//! of their own, such as a developer pointing at staging. +//! 4. The token compiled in from `MAPBOX_CLI_BUNDLED_TOKEN` at build time. A +//! plain `cargo build` has none. It is a different variable from the +//! run-time one so that a token exported in a dev shell is not baked into +//! every local build. +//! +//! A `401` moves on to the next token; any other answer is the caller's. +//! Callers never see the token, so every one of them gets the same fallback +//! without doing anything. +//! +//! # What may use the CLI's token +//! +//! Steps 3 and 4 are the CLI's token, and every request made with it is +//! billed to the account that owns it. So it is decided by what the request +//! is for, not where it goes: a request the CLI makes for itself passes +//! [`send`] an order that ends in [`Fallback::CliToken`], and every such order +//! is defined in this file (today only [`TELEMETRY`]); a command must be in +//! [`CLI_TOKEN_COMMANDS`]. Where the request is sent does not matter, so a URL +//! overridden from the environment keeps the same fallback. +//! +//! # The bundled token is public +//! +//! Anything compiled into a distributed binary can be pulled out with +//! `strings`. The bundled token must therefore be a dedicated `pk.` token with +//! the minimum scopes, and nothing may ever treat it as a secret. `build.rs` +//! refuses anything that is not `pk.`. + +use reqwest::blocking::{RequestBuilder, Response}; +use reqwest::StatusCode; + +use crate::{auth, http}; + +/// One place [`send`] may take a token from. A caller passes them in the +/// order to try. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Fallback { + /// `--token` or `MAPBOX_ACCESS_TOKEN`, as [`user_token`] ranks them. + UserToken, + /// The stored login, loaded as a command loads it, refresh included. + Login, + /// `MAPBOX_CLI_TOKEN` at run time, then the bundled token. Billed to the + /// CLI's account, so it only ever comes after every token of the user's. + CliToken, +} + +/// Telemetry's order. The CLI's own usage data, so there is no user's +/// account to bill when the user has no token. +pub(crate) const TELEMETRY: &[Fallback] = + &[Fallback::UserToken, Fallback::Login, Fallback::CliToken]; + +/// Every order defined here, for the test that holds the CLI's token last. +/// An order that uses it belongs in this file and in this list. +#[cfg(test)] +const ORDERS: &[&[Fallback]] = &[TELEMETRY]; + +/// Commands that may run on the CLI's token when the user has none, by the +/// name `--schema` reports, each with the reason the CLI's account should +/// carry their use. Command names are a compatibility promise, so an entry +/// does not quietly move to another API when a spec changes its URL. +const CLI_TOKEN_COMMANDS: &[(&str, &str)] = &[]; + +/// The CLI's token for a command's request, when the user has none of their +/// own and the command is in [`CLI_TOKEN_COMMANDS`]. +/// +/// Only for a user with no token at all: one whose token is rejected sees that +/// rejection rather than having it hidden behind the CLI's token. +pub(crate) fn for_command(op: &crate::spec::Operation) -> Option { + let command = op.command(); + if CLI_TOKEN_COMMANDS.iter().any(|(name, _)| *name == command) { + cli_token(std::env::var("MAPBOX_CLI_TOKEN").ok().as_deref()) + } else { + None + } +} + +/// `MAPBOX_CLI_TOKEN` at run time, then the bundled token. +fn cli_token(override_token: Option<&str>) -> Option { + usable(override_token) + .or_else(|| usable(option_env!("MAPBOX_CLI_BUNDLED_TOKEN"))) + .map(str::to_owned) +} + +/// The token the user gave this run, ranked as a command ranks it: with +/// `--use-login` only a typed `--token` counts, otherwise `MAPBOX_ACCESS_TOKEN` +/// does too. Resolved by the caller, which has the arguments, and handed to +/// [`send`]. +pub(crate) fn user_token(matches: &clap::ArgMatches) -> Option { + if matches.get_flag("use-login") { + auth::typed_token(matches) + } else { + matches.get_one::("token").cloned() + } +} + +/// Sends `request` with a token attached as `access_token`, trying the +/// next token whenever the API answers `401`. +/// +/// May block on the credentials lock and refresh the login over the network, +/// as a command does, so call it where nobody is waiting on the answer, such +/// as a detached child. +/// +/// `request` is called once per attempt, since a sent request cannot be +/// reused. `None` when there is no token to send with; an error when the +/// request could not be sent at all; otherwise the first answer that was not +/// a `401`, or the last `401` when every token was rejected. +pub(crate) fn send( + order: &[Fallback], + user_token: Option<&str>, + profile: Option<&str>, + request: impl Fn() -> RequestBuilder, +) -> Option> { + let override_token = std::env::var("MAPBOX_CLI_TOKEN").ok(); + resolve( + order, + Sources { + user: user_token, + override_token: override_token.as_deref(), + bundled: option_env!("MAPBOX_CLI_BUNDLED_TOKEN"), + }, + || login_token(profile), + |token| http::send(request().query(&[("access_token", token)])), + |response| response.status() == StatusCode::UNAUTHORIZED, + ) +} + +/// Whether [`send`] has a token it could try in `order`, judged without the +/// network or the credentials lock, so a caller can skip work that would only +/// find nothing to send with. A login counts by being there: it may still turn +/// out expired and unrefreshable, and then nothing is sent. +pub(crate) fn has_token_for( + order: &[Fallback], + user_token: Option<&str>, + profile: Option<&str>, +) -> bool { + order.iter().any(|fallback| match fallback { + Fallback::UserToken => usable(user_token).is_some(), + Fallback::Login => auth::load_credentials_readonly(profile).is_some(), + Fallback::CliToken => { + cli_token(std::env::var("MAPBOX_CLI_TOKEN").ok().as_deref()).is_some() + } + }) +} + +/// The stored login's token, refreshed when it is about to expire. +fn login_token(profile: Option<&str>) -> Option { + // Read without the lock first, and used as it is while it is good: + // `load_fresh_credentials` takes the credentials lock, which creates and + // hardens `~/.mapbox`, and a read-only command's telemetry must not be + // what changes it. Only a login about to expire pays for the lock. + let stored = auth::load_credentials_readonly(profile)?; + if !auth::token_needs_refresh(&stored.access_token) { + return Some(stored.access_token); + } + auth::load_fresh_credentials(false, profile).map(|c| c.access_token) +} + +struct Sources<'a> { + user: Option<&'a str>, + override_token: Option<&'a str>, + bundled: Option<&'a str>, +} + +/// The order and the fallback, with the login and the send passed in so each +/// path can be tested without the network or the disk. +/// +/// Tries each [`Fallback`] in `order`. The login is loaded only when its turn +/// comes, and a token that was already rejected is not tried again. A +/// failed send stops here: no other token fixes a request that never +/// arrived. +fn resolve( + order: &[Fallback], + sources: Sources<'_>, + login: impl FnOnce() -> Option, + mut attempt: impl FnMut(&str) -> Result, + rejected: impl Fn(&R) -> bool, +) -> Option> { + let mut login = Some(login); + let mut tried: Vec = Vec::new(); + let mut last = None; + for fallback in order { + let tokens: Vec = match fallback { + Fallback::UserToken => usable(sources.user) + .map(str::to_owned) + .into_iter() + .collect(), + Fallback::Login => login.take().and_then(|load| load()).into_iter().collect(), + Fallback::CliToken => [sources.override_token, sources.bundled] + .into_iter() + .filter_map(|t| usable(t).map(str::to_owned)) + .collect(), + }; + for token in tokens { + if tried.contains(&token) { + continue; + } + let response = match attempt(&token) { + Ok(response) => response, + Err(error) => return Some(Err(error)), + }; + if !rejected(&response) { + return Some(Ok(response)); + } + tried.push(token); + last = Some(response); + } + } + last.map(Ok) +} + +fn usable(token: Option<&str>) -> Option<&str> { + token.map(str::trim).filter(|t| !t.is_empty()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::{Cell, RefCell}; + + /// Runs [`resolve`] against a fake API that accepts only `accepted`, and + /// returns the answer, every token sent, and whether the login was loaded. + fn run( + sources: Sources<'_>, + login: Option<&str>, + accepted: &[&str], + ) -> (Option, Vec, bool) { + run_in(TELEMETRY, sources, login, accepted) + } + + fn run_in( + order: &[Fallback], + sources: Sources<'_>, + login: Option<&str>, + accepted: &[&str], + ) -> (Option, Vec, bool) { + let sent = RefCell::new(Vec::new()); + let loaded = Cell::new(false); + let answer = resolve( + order, + sources, + || { + loaded.set(true); + login.map(str::to_owned) + }, + |token| { + sent.borrow_mut().push(token.to_owned()); + Ok::<_, ()>(if accepted.contains(&token) { 200 } else { 401 }) + }, + |status| *status == 401, + ); + (answer.map(|a| a.unwrap()), sent.into_inner(), loaded.get()) + } + + fn sources<'a>( + user: Option<&'a str>, + override_token: Option<&'a str>, + bundled: Option<&'a str>, + ) -> Sources<'a> { + Sources { + user, + override_token, + bundled, + } + } + + #[test] + fn an_accepted_user_token_is_all_that_is_sent() { + let (answer, sent, loaded) = run( + sources(Some("pk.typed"), Some("pk.override"), Some("pk.bundled")), + Some("sk.login"), + &["pk.typed"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["pk.typed"]); + assert!( + !loaded, + "the login is not touched when the user's token works" + ); + } + + #[test] + fn the_login_comes_after_the_users_token() { + let (answer, sent, _) = run( + sources(Some("pk.typed"), Some("pk.override"), None), + Some("sk.login"), + &["sk.login"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["pk.typed", "sk.login"]); + } + + #[test] + fn the_login_is_used_when_the_user_typed_nothing() { + let (answer, sent, _) = run(sources(None, None, None), Some("sk.login"), &["sk.login"]); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["sk.login"]); + } + + #[test] + fn the_clis_token_is_sent_only_once_every_user_token_is_rejected() { + let (answer, sent, _) = run( + sources(Some("pk.typed"), Some("pk.override"), Some("pk.bundled")), + Some("sk.login"), + &["pk.override"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["pk.typed", "sk.login", "pk.override"]); + } + + #[test] + fn the_clis_token_is_never_sent_by_an_order_without_it() { + let (answer, sent, _) = run_in( + &[Fallback::UserToken, Fallback::Login], + sources(Some("pk.typed"), Some("pk.override"), Some("pk.bundled")), + None, + &["pk.override", "pk.bundled"], + ); + assert_eq!(answer, Some(401)); + assert_eq!(sent, ["pk.typed"]); + } + + #[test] + fn the_order_given_is_the_order_tried() { + let (_, sent, _) = run_in( + &[Fallback::Login, Fallback::UserToken], + sources(Some("pk.typed"), None, None), + Some("sk.login"), + &[], + ); + assert_eq!(sent, ["sk.login", "pk.typed"]); + } + + #[test] + fn every_order_keeps_the_clis_token_last() { + for order in ORDERS { + if let Some(at) = order.iter().position(|f| *f == Fallback::CliToken) { + assert_eq!(at, order.len() - 1, "{order:?}"); + } + } + } + + #[test] + fn override_comes_before_bundled() { + let (answer, sent, _) = run( + sources(None, Some("pk.override"), Some("pk.bundled")), + None, + &["pk.override", "pk.bundled"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["pk.override"]); + } + + #[test] + fn a_rejected_token_is_not_sent_twice() { + // The login and the typed token can be the same token. + let (answer, sent, _) = run( + sources(Some("sk.same"), None, Some("pk.bundled")), + Some("sk.same"), + &["pk.bundled"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["sk.same", "pk.bundled"]); + } + + #[test] + fn every_token_rejected_returns_the_last_answer() { + let (answer, sent, _) = run( + sources(Some("pk.typed"), Some("pk.override"), Some("pk.bundled")), + Some("sk.login"), + &[], + ); + assert_eq!(answer, Some(401)); + assert_eq!(sent, ["pk.typed", "sk.login", "pk.override", "pk.bundled"]); + } + + #[test] + fn a_failed_send_stops_without_trying_other_tokens() { + let mut sent = Vec::new(); + let answer = resolve( + TELEMETRY, + sources(Some("pk.typed"), Some("pk.override"), Some("pk.bundled")), + || Some("sk.login".to_owned()), + |token| { + sent.push(token.to_owned()); + Err::("unreachable") + }, + |status| *status == 401, + ); + assert_eq!(answer, Some(Err("unreachable"))); + assert_eq!(sent, ["pk.typed"]); + } + + #[test] + fn blank_tokens_are_skipped_and_the_rest_trimmed() { + let (answer, sent, _) = run( + sources(Some(" "), Some(""), Some(" pk.bundled\n")), + None, + &["pk.bundled"], + ); + assert_eq!(answer, Some(200)); + assert_eq!(sent, ["pk.bundled"]); + } + + #[test] + fn nothing_to_send_with_sends_nothing() { + let (answer, sent, _) = run(sources(None, None, None), None, &[]); + assert_eq!(answer, None); + assert!(sent.is_empty()); + } + + fn user_token_for(args: &[&str]) -> Option { + let matches = crate::build_app(&[]) + .try_get_matches_from(args) + .expect("arguments parse"); + user_token(&matches) + } + + // The `MAPBOX_ACCESS_TOKEN` half is not covered here: setting it would + // race every other test in this process that reads the environment. + #[test] + fn a_typed_token_is_the_users_with_or_without_use_login() { + for args in [ + &["mapbox", "--token", "pk.typed", "auth", "whoami"][..], + &[ + "mapbox", + "--use-login", + "--token", + "pk.typed", + "auth", + "whoami", + ][..], + ] { + assert_eq!( + user_token_for(args).as_deref(), + Some("pk.typed"), + "{args:?}" + ); + } + } + + #[test] + fn use_login_without_a_typed_token_leaves_the_user_none() { + assert_eq!( + user_token_for(&["mapbox", "--use-login", "auth", "whoami"]), + None + ); + } + + #[test] + fn every_cli_token_command_exists_and_says_why() { + let specs = crate::spec::effective_services().expect("the bundled specs parse"); + let commands: Vec = specs + .iter() + .flat_map(|svc| &svc.operations) + .map(|op| op.command()) + .collect(); + for (name, reason) in CLI_TOKEN_COMMANDS { + assert!( + commands.iter().any(|c| c == name), + "`{name}` is in CLI_TOKEN_COMMANDS but is not a command" + ); + assert!(!reason.trim().is_empty(), "`{name}` needs a reason"); + } + } + + #[test] + fn the_runtime_override_comes_before_the_bundled_token() { + assert_eq!( + cli_token(Some(" pk.override ")).as_deref(), + Some("pk.override") + ); + assert_eq!( + cli_token(Some(" ")).as_deref(), + option_env!("MAPBOX_CLI_BUNDLED_TOKEN") + .map(str::trim) + .filter(|t| !t.is_empty()) + ); + } +} diff --git a/src/detach.rs b/src/detach.rs new file mode 100644 index 0000000..3a49165 --- /dev/null +++ b/src/detach.rs @@ -0,0 +1,57 @@ +//! Starting a process the CLI will not wait for: the update check's refresh +//! and the telemetry send. +//! +//! Detached so it outlives the command and never reaches its terminal. On +//! Windows that is not enough. A child inherits every inheritable handle its +//! parent holds, whatever its own stdio is set to, and the parent's standard +//! handles are inheritable whenever whoever started it made them so: the pipe +//! behind `$(mapbox …)`, PowerShell's `$x = mapbox …`, a test's `output()`. +//! The child would then hold the caller's pipe open, and the caller would wait +//! for it — up to the send's whole budget — before seeing the command finish. +//! So on Windows the parent's own standard handles stop being inheritable +//! first. A later child that asks to inherit stdio still gets it: the standard +//! library hands it a duplicate made inheritable for that child alone. + +use std::process::Command; + +/// Sets `command` up to run on its own; the caller still chooses its stdio. +pub(crate) fn detach(command: &mut Command) { + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + command.process_group(0); + } + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const DETACHED_PROCESS: u32 = 0x0000_0008; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW); + keep_standard_handles_from_children(); + } +} + +/// The crate's one `unsafe`: two Win32 calls the standard library has no safe +/// form of. Neither takes a pointer, and the second only clears a flag on a +/// handle this process already owns; a failure leaves the flag as it was, +/// which is today's behavior. +#[cfg(windows)] +#[allow(unsafe_code)] +fn keep_standard_handles_from_children() { + use windows_sys::Win32::Foundation::{ + SetHandleInformation, HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, + }; + use windows_sys::Win32::System::Console::{ + GetStdHandle, STD_ERROR_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, + }; + + for which in [STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE] { + // SAFETY: see the function's doc; the handle is checked before use. + unsafe { + let handle = GetStdHandle(which); + if !handle.is_null() && handle != INVALID_HANDLE_VALUE { + SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 0); + } + } + } +} diff --git a/src/executor.rs b/src/executor.rs index ebfcaf4..14b1469 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1571,6 +1571,7 @@ mod tests { required: false, content_types: content_types.iter().map(|s| s.to_string()).collect(), multipart_field: multipart_field.map(|s| s.to_string()), + json_fields: vec![], } } diff --git a/src/http.rs b/src/http.rs index e72ed60..164d4fa 100644 --- a/src/http.rs +++ b/src/http.rs @@ -292,7 +292,7 @@ pub fn send( /// What went wrong with a request that got no response. Not `err`'s own /// `Display`, which appends the URL — token and all. -fn failure(err: &reqwest::Error) -> String { +pub(crate) fn failure(err: &reqwest::Error) -> String { let kind = if err.is_timeout() { "timed out" } else if err.is_connect() { diff --git a/src/main.rs b/src/main.rs index e4327eb..f3f0959 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,11 +16,13 @@ mod agent_skills; #[cfg(test)] mod api_command_surface; mod auth; +mod cli_token; mod completion; mod config; mod confirm; mod dated_jsonl; mod deprecation; +mod detach; mod doctor; mod executor; mod generate_skills; @@ -38,6 +40,8 @@ mod schema; mod skill_dest; mod spec; mod telemetry; +mod telemetry_event; +mod telemetry_sink; mod tilesets_cli; mod uninstall; mod update_check; @@ -797,6 +801,7 @@ fn no_stored_credentials(profile: Option<&str>) -> anyhow::Error { /// Answers `--schema`, from either of the two places it can be noticed. fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 { run_record::set_parsed(app, specs, matches, run_record::Invocation::Schema); + telemetry_sink::remember_user_token(cli_token::user_token(matches)); let mode = Mode::from_matches(matches); match schema::emit(mode, app, specs, matches) { Ok(()) => 0, @@ -809,11 +814,11 @@ fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 /// The process, either of the two things it can be. /// -/// Almost always a command. The exception is the update-check refresher, -/// which this binary re-executes as a detached child of itself — it parses no -/// command line, loads no specs and prints nothing, so it is answered before -/// any of that happens. See `update_check` for why it is a mode rather than a -/// hidden subcommand. +/// Almost always a command. The exceptions are the update-check refresher +/// and the telemetry sender, which this binary re-executes as detached +/// children of itself — they parse no command line, load no specs and print +/// nothing, so they are answered before any of that happens. See +/// `update_check` for why each is a mode rather than a hidden subcommand. /// /// The notice is the last thing that happens, and deliberately outside /// `cli()`: it belongs to every way out, `--version` and a usage error @@ -824,6 +829,9 @@ fn main() -> ExitCode { if update_check::is_refresh_child() { return update_check::run_refresh_child(); } + if telemetry_sink::is_send_child() { + return telemetry_sink::run_send_child(); + } run_record::start(&std::env::args_os().collect::>()); let code = cli(); @@ -902,6 +910,7 @@ fn cli() -> u8 { } run_record::set_parsed(&app, &specs, &matches, run_record::Invocation::Execute); + telemetry_sink::remember_user_token(cli_token::user_token(&matches)); output::banner::show(&matches); let mode = Mode::from_matches(&matches); match run(&app, &specs, &matches, mode) { @@ -1453,6 +1462,16 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - if let Some(token) = &token { run_record::set_resolved_token(matches, use_login, token); } + // After the record above, which knows only the user's own + // sources. `--use-login` already returned when it found nothing. + let token = token.or_else(|| { + let token = cli_token::for_command(op)?; + output::progress( + "No token of your own, so this request uses the CLI's token. \ + Run `mapbox auth login` to use yours.", + ); + Some(token) + }); let username: Option = matches .get_one::("username") .cloned() diff --git a/src/run_record.rs b/src/run_record.rs index fbaa914..d979e7c 100644 --- a/src/run_record.rs +++ b/src/run_record.rs @@ -21,7 +21,8 @@ use clap::{ArgMatches, Command}; use crate::spec::ServiceSpec; use crate::{ - auth, completion, confirm, executor, http, output, run_history, run_log, tilesets_cli, + auth, completion, confirm, executor, http, output, run_history, run_log, telemetry_event, + tilesets_cli, }; const TILESETS: &str = tilesets_cli::COMMAND; @@ -125,6 +126,8 @@ pub(crate) struct Record { /// clap's name for why it refused the command line. pub usage_error: Option, pub args: Vec, + /// Top-level fields the operation's spec declares for a JSON body. + pub body_fields: Vec, pub options: Option, pub token: Option, pub requests: Vec, @@ -147,6 +150,7 @@ static RECORD: Mutex = Mutex::new(Record { tilesets_word: None, usage_error: None, args: Vec::new(), + body_fields: Vec::new(), options: None, token: None, requests: Vec::new(), @@ -222,10 +226,12 @@ pub fn set_parsed( }) .flatten(); let mut args = vec![]; + let mut fields = vec![]; if path.first().map(String::as_str) != Some(TILESETS) { let numeric = numeric_args(specs, &path); args.extend(given_args(leaf_command, leaf_matches, &numeric, false)); args.extend(given_args(app, matches, &[], true)); + fields = body_fields(specs, &path); } let options = options(matches, leaf_matches); with_record(|record| { @@ -233,6 +239,7 @@ pub fn set_parsed( record.invocation = Some(invocation); record.tilesets_word = tilesets_word; record.args = args; + record.body_fields = fields; record.options = Some(options); }); } @@ -353,9 +360,10 @@ fn finish_locked(record: &mut Record, exit_code: Option) { run_log::write(record, at); } run_log::expire_with_history(); + telemetry_event::deliver(record); } -fn uuid_v4(mut bytes: [u8; 16]) -> String { +pub(crate) fn uuid_v4(mut bytes: [u8; 16]) -> String { bytes[6] = (bytes[6] & 0x0f) | 0x40; bytes[8] = (bytes[8] & 0x3f) | 0x80; let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); @@ -404,7 +412,7 @@ fn command_from_argv(app: &Command, argv: &[OsString]) -> Vec { /// Subcommand names from `words`, in order, for as long as each word names /// a subcommand of the one before. Flags and their values are skipped; the /// first word that is neither ends the walk. -fn tree_path(app: &Command, words: impl IntoIterator) -> Vec { +pub(crate) fn tree_path(app: &Command, words: impl IntoIterator) -> Vec { let mut path = vec![]; let mut command = app; for word in words { @@ -480,6 +488,22 @@ fn numeric_args(specs: &[ServiceSpec], path: &[String]) -> Vec { .collect() } +/// The top-level fields the spec declares for the JSON body of the +/// operation at `path`. +fn body_fields(specs: &[ServiceSpec], path: &[String]) -> Vec { + let Some((service, rest)) = path.split_first() else { + return vec![]; + }; + specs + .iter() + .filter(|spec| &spec.name == service) + .flat_map(|spec| &spec.operations) + .find(|op| op.command_path == rest) + .and_then(|op| op.body.as_ref()) + .map(|body| body.json_fields.clone()) + .unwrap_or_default() +} + /// `command`'s arguments that were given on the command line or through /// their environment variable. A leaf `Command` does not list the globals it /// inherits, so those are read separately from the root, as `global`. diff --git a/src/spec.rs b/src/spec.rs index 11f8ecb..0d93fed 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -225,6 +225,10 @@ pub struct RequestBody { /// since the API matches on this field name exactly; guessing it wrong /// would fail the request with no clear error. pub multipart_field: Option, + /// Top-level field names the JSON body's schema declares. Telemetry + /// records a `--data` body's keys only when they are on this list, so a + /// name the user made up never leaves the machine. + pub json_fields: Vec, } pub const MULTIPART: &str = "multipart/form-data"; @@ -1155,6 +1159,7 @@ fn parse_request_body(op: &Value, full_spec: &Value) -> Option { required, content_types: vec![], multipart_field: None, + json_fields: vec![], }) } }; @@ -1170,10 +1175,17 @@ fn parse_request_body(op: &Value, full_spec: &Value) -> Option { .find(|(key, _)| key.as_str().map(essence).as_deref() == Some(MULTIPART)) .and_then(|(_, media_type)| multipart_file_field(&media_type["schema"], full_spec)); + let json_fields = content + .iter() + .find(|(key, _)| key.as_str().is_some_and(is_json)) + .map(|(_, media_type)| declared_fields(&media_type["schema"], full_spec)) + .unwrap_or_default(); + Some(RequestBody { required, content_types, multipart_field, + json_fields, }) } @@ -1236,6 +1248,44 @@ fn body_field_parameters( .collect() } +/// The property names an object schema declares, through `$ref` and the +/// `allOf`/`oneOf`/`anyOf` it is composed of. `additionalProperties` adds +/// nothing: what it allows is whatever the caller chose to call a field. +fn declared_fields(schema: &Value, full_spec: &Value) -> Vec { + let mut fields = vec![]; + collect_declared_fields(schema, full_spec, 0, &mut fields); + fields.sort(); + fields.dedup(); + fields +} + +fn collect_declared_fields(schema: &Value, full_spec: &Value, depth: u8, fields: &mut Vec) { + // A `$ref` cycle would otherwise recurse forever. + if depth > 8 { + return; + } + let schema = match schema["$ref"].as_str() { + Some(reference) => match resolve_ref(full_spec, reference) { + Some(resolved) => resolved, + None => return, + }, + None => schema, + }; + if let Some(properties) = schema["properties"].as_mapping() { + fields.extend( + properties + .keys() + .filter_map(|key| key.as_str()) + .map(str::to_string), + ); + } + for key in ["allOf", "oneOf", "anyOf"] { + for part in schema[key].as_sequence().into_iter().flatten() { + collect_declared_fields(part, full_spec, depth + 1, fields); + } + } +} + /// The multipart property that carries the uploaded files. /// /// We prefer a property whose schema is `format: binary` (directly, or as @@ -1902,6 +1952,54 @@ paths: assert_eq!(body.multipart_field.as_deref(), Some("attachments")); } + /// Telemetry sends a `--data` body's keys only from this list, so it + /// must follow `$ref` and composition, and never admit what + /// `additionalProperties` allows. + #[test] + fn a_json_body_lists_the_fields_its_schema_declares() { + let svc = service( + r#" +openapi: 3.0.0 +info: { title: T } +components: + schemas: + Base: + type: object + properties: { version: { type: number }, name: { type: string } } +paths: + /json: + post: + operationId: createThing + requestBody: + content: + application/json: + schema: + allOf: + - $ref: '#/components/schemas/Base' + - type: object + properties: { layers: { type: array } } + additionalProperties: true + /open: + post: + operationId: openThing + requestBody: + content: + application/json: + schema: { type: object } +"#, + ); + let fields = |id| { + operation(&svc, id) + .body + .as_ref() + .expect("body") + .json_fields + .clone() + }; + assert_eq!(fields("create-thing"), ["layers", "name", "version"]); + assert!(fields("open-thing").is_empty()); + } + /// A `requestBody` that declares no content still means the operation /// takes one. That is what the old `has_body` bool recorded, and `--data` /// has to keep working for it. diff --git a/src/telemetry.rs b/src/telemetry.rs index 1396369..afe4176 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -58,6 +58,21 @@ fn arch_marker() -> String { format!("arch/{}", spelled_arch(std::env::consts::ARCH)) } +/// The CPU architecture as the event's `env.arch` spells it. +pub(crate) fn arch() -> &'static str { + spelled_arch(std::env::consts::ARCH) +} + +/// Whether this run is in CI, by the same rule as the `env/ci` marker. +pub(crate) fn in_ci() -> bool { + ci_marker().is_some() +} + +/// For `crate::telemetry_event`, which may not reach for stdout itself. +pub(crate) fn stdout_is_terminal() -> bool { + std::io::stdout().is_terminal() +} + fn spelled_arch(arch: &str) -> &str { match arch { "aarch64" => "arm64", diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs new file mode 100644 index 0000000..6a67e92 --- /dev/null +++ b/src/telemetry_event.rs @@ -0,0 +1,1253 @@ +//! One `cli.command` event per run: what ran and how it ended. +//! +//! Built from the [`crate::run_record::Record`] at the end of each run and +//! handed to [`crate::telemetry_sink`], which decides where it is delivered; +//! this module decides only what it contains. The record holds raw facts — +//! the command line, URLs, error messages — and every field of the event is +//! chosen from it here, explicitly, so this file is the whole of what can +//! leave the machine. +//! +//! What this refuses to send is the point of it. Argument values leave +//! only when they come from a fixed set (an enum, a boolean, a number the +//! spec types, an allowlisted code); a free string is sent as its length, a +//! file as its size, a coordinate as its name alone, and a JSON body as its +//! size and the top-level field names its spec declares. Command names come +//! from the command tree, never from argv. A token is read for its prefix and +//! its account claim and nothing else. No response's request id is sent: it +//! leads to the request in CloudFront logs, which hold the caller's IP. +//! +//! Best-effort throughout: nothing here can change a command's output, its +//! exit code, or how long it takes to return. With telemetry off +//! (`MAPBOX_CLI_NO_TELEMETRY`), nothing is built or written. + +use std::io::IsTerminal; +use std::path::Path; +use std::sync::{Mutex, OnceLock}; +use std::time::{Duration, SystemTime}; + +use clap::Command; +use serde::Serialize; + +use crate::run_record::{self, uuid_v4, Invocation, Record}; +use crate::{agent_detect, confirm, executor, http, output, schema, telemetry, telemetry_sink}; + +const EVENT: &str = "cli.command"; +const SCHEMA_VERSION: &str = "2.0"; +const SDK_IDENTIFIER: &str = "mapbox-cli"; +const CURRENT: &str = env!("CARGO_PKG_VERSION"); + +/// How long a `userId` lives before the next run replaces it, so it is never +/// a permanent identifier. +const USER_ID_LIFETIME: Duration = Duration::from_secs(24 * 60 * 60); + +/// The largest `--data @` file read for its top-level keys; above +/// it, only the size is recorded. +const MAX_DATA_TO_PARSE: u64 = 256 * 1024; + +/// Set by a workflow on each step it launches, to its own [`event_id`], so +/// a `mapbox` run inside a workflow step records which run started it. +const PARENT_EVENT_ENV: &str = "MAPBOX_CLI_PARENT_EVENT"; + +const USER_ID_FILE: &str = "user-id"; +const LAST_VERSION_FILE: &str = "last-version"; + +// Bounds from the schema. An event over any of them is rejected whole at +// ingest, so they are enforced here rather than trusted. +const MAX_PARAMS: usize = 50; +const MAX_VALUE: usize = 64; +const MAX_KEYS: usize = 50; +const MAX_COMMAND_LEVELS: usize = 8; +const MAX_STEPS: usize = 20; + +// The schema leaves these strings unbounded; they are clipped here so that +// an unexpected input can't make the event large. +const MAX_NAME: usize = 64; +const MAX_COMMAND_NAME: usize = 32; +const MAX_CODE: usize = 64; +const MAX_VERSION: usize = 32; + +/// Options that are top-level fields or `invocation`, so never `params`. +const NOT_PARAMS: &[&str] = &[ + "token", + "profile", + "use-login", + "debug", + confirm::ARG, + http::TIMEOUT_ARG, + output::ARG, + schema::ARG, + executor::DRY_RUN_ARG, + "help", + "version", +]; + +/// Global options with no top-level field, sent as free strings. They are +/// read from the root matches: a leaf `Command` does not list the globals +/// it inherits. +const GLOBAL_PARAMS: &[&str] = &["username", output::FILTER_ARG]; + +/// Free strings whose values are codes, not user data — sent only when every +/// comma-separated item looks like one (see [`is_code`]). The specs type them +/// as plain strings, so anything else is text the user chose and goes out as +/// a length like any other free string. +const ALLOWLISTED: &[&str] = &["language", "country", "types"]; + +/// Feature types the geocoding and search APIs define, for `types`. +const FEATURE_TYPES: &[&str] = &[ + "address", + "block", + "brand", + "category", + "city", + "country", + "district", + "locality", + "neighborhood", + "place", + "poi", + "postcode", + "region", + "secondary_address", + "street", +]; + +/// Numbers that are sent by name only: together they are a location. A tile +/// address is one too — `z`/`x`/`y` at zoom 18 pins about 150 m. +const COORDINATES: &[&str] = &["lon", "lat", "longitude", "latitude", "x", "y", "z", "zoom"]; + +/// The Python `tilesets` CLI's own commands (`mapbox_tilesets/scripts/cli.py`). +/// A forwarded first word outside this list is recorded as `other`, since it +/// is whatever the user typed. +const TILESETS_COMMANDS: &[&str] = &[ + "add-source", + "create", + "delete", + "delete-changeset", + "delete-source", + "estimate-area", + "job", + "jobs", + "list", + "list-activity", + "list-sources", + "publish", + "publish-changesets", + "status", + "tilejson", + "update", + "update-recipe", + "upload-changeset", + "upload-raster-source", + "upload-source", + "validate-recipe", + "validate-source", + "view-changeset", + "view-recipe", + "view-source", +]; + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +pub(crate) struct Param { + name: String, + #[serde(skip_serializing_if = "Option::is_none")] + value: Option, + #[serde(skip_serializing_if = "Option::is_none")] + length: Option, + #[serde(skip_serializing_if = "Option::is_none")] + bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + keys: Option>, +} + +/// Where a workflow came from. Only Mapbox names its `Builtin` and +/// `Marketplace` workflows; a `Custom` one is named by the user, so its name +/// is never recorded. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] // Used by the workflow commands, which don't exist yet. +pub(crate) enum WorkflowSource { + Builtin, + Marketplace, + Custom, +} + +impl WorkflowSource { + fn as_str(self) -> &'static str { + match self { + WorkflowSource::Builtin => "builtin", + WorkflowSource::Marketplace => "marketplace", + WorkflowSource::Custom => "custom", + } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Workflow { + source: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + step_count: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + steps: Vec, +} + +/// One step of a workflow run. Built only by [`cli_step`] and +/// [`script_step`], so a script step can never carry a command name or an +/// error category: nothing about a user's script is recorded beyond how it +/// exited and how long it took. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Step { + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + duration_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Auth { + source: &'static str, + #[serde(rename = "type")] + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + account: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Network { + request_count: u32, + #[serde(skip_serializing_if = "Option::is_none")] + status: Option, + #[serde(skip_serializing_if = "Option::is_none")] + response_bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + request_body_bytes: Option, + network_ms: u64, + #[serde(skip_serializing_if = "std::ops::Not::not")] + more_pages: bool, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Cli { + #[serde(skip_serializing_if = "Option::is_none")] + build_channel: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + build_id: Option<&'static str>, + install_method: &'static str, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Env { + arch: &'static str, + ci: bool, + #[serde(skip_serializing_if = "Option::is_none")] + agent: Option<&'static str>, + stdin_tty: bool, + stdout_tty: bool, +} + +/// The event as sent. Field order is the schema's. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Event { + event: &'static str, + version: &'static str, + created: String, + event_id: String, + #[serde(skip_serializing_if = "Option::is_none")] + parent_event_id: Option, + user_id: String, + sdk_identifier: &'static str, + sdk_version: &'static str, + operating_system: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + invocation: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + workflow: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + params: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + usage_error: Option, + #[serde(skip_serializing_if = "Option::is_none")] + output: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + output_source: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + dry_run: Option, + #[serde(skip_serializing_if = "Option::is_none")] + debug: Option, + #[serde(skip_serializing_if = "Option::is_none")] + yes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + profile: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + timeout_seconds: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auth: Option, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + stdout_bytes: u64, + duration_ms: u64, + #[serde(skip_serializing_if = "Option::is_none")] + auth_step: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + update_notice: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_version: Option, + #[serde(skip_serializing_if = "Option::is_none")] + network: Option, + cli: Cli, + env: Env, +} + +/// The workflow this run belongs to. Kept here rather than in the record: +/// the workflow commands that report it don't exist yet. +static WORKFLOW: Mutex> = Mutex::new(None); +static EVENT_ID: OnceLock = OnceLock::new(); + +fn with_workflow(f: impl FnOnce(&mut Option)) { + if let Ok(mut workflow) = WORKFLOW.lock() { + f(&mut workflow); + } +} + +/// Builds the run's event and hands it to the sink, unless telemetry is off +/// or the run is under `sudo`. +/// +/// `sudo` keeps `HOME`, so a root run would create `~/.mapbox/.telemetry` and +/// its state files as root, and every later run as the user could read none +/// of them — a fresh `userId` each time. Skipped for the same reason +/// `run_history` skips it (aws/aws-cli#10031). +pub(crate) fn deliver(record: &Record) { + if !telemetry::telemetry_allowed() || std::env::var_os("SUDO_USER").is_some() { + return; + } + let event = build(record); + if let Ok(line) = serde_json::to_string(&event) { + let profile = record.options.as_ref().and_then(|o| o.profile.as_deref()); + telemetry_sink::deliver(&line, profile); + } +} + +fn build(record: &Record) -> Event { + let options = record.options.as_ref(); + Event { + event: EVENT, + version: SCHEMA_VERSION, + created: timestamp(SystemTime::now()), + event_id: event_id().to_string(), + parent_event_id: parent_event_id(), + user_id: user_id(), + sdk_identifier: SDK_IDENTIFIER, + sdk_version: CURRENT, + operating_system: std::env::consts::OS, + command: command_field(record), + invocation: record.invocation.map(Invocation::as_str), + workflow: WORKFLOW.lock().ok().and_then(|workflow| workflow.clone()), + params: params(record), + usage_error: record + .usage_error + .as_deref() + .map(|kind| clip(kind, MAX_CODE)), + output: options.map(|o| o.output), + output_source: options.map(|o| o.output_source), + dry_run: options.map(|o| o.dry_run), + debug: options.map(|o| o.debug), + yes: options.map(|o| o.yes), + profile: options.map(|o| match o.profile.as_deref() { + None | Some("default") => "default", + Some(_) => "named", + }), + timeout_seconds: options.and_then(|o| o.timeout).map(|t| t.as_secs_f64()), + auth: record.token.as_ref().map(|token| Auth { + source: token.source.as_str(), + kind: token.kind, + account: token.account.as_deref().map(|a| clip(a, MAX_NAME)), + }), + exit_code: record.exit_code, + error_code: record.error.as_ref().map(|e| clip(&e.code, MAX_CODE)), + stdout_bytes: record.stdout_bytes, + duration_ms: record.duration.as_millis() as u64, + auth_step: record.auth_step, + update_notice: record + .update_notice + .as_deref() + .map(|v| clip(v, MAX_VERSION)), + previous_version: previous_version(), + network: network(record), + cli: Cli { + build_channel: option_env!("MAPBOX_CLI_BUILD_ENV"), + build_id: option_env!("MAPBOX_CLI_BUILD_ID"), + install_method: install_method(std::env::current_exe().ok().as_deref()), + }, + env: Env { + arch: telemetry::arch(), + ci: telemetry::in_ci(), + agent: agent_detect::detect_agent(), + stdin_tty: std::io::stdin().is_terminal(), + stdout_tty: telemetry::stdout_is_terminal(), + }, + } +} + +const TILESETS: &str = crate::tilesets_cli::COMMAND; + +fn command_field(record: &Record) -> Option> { + let first = record.command.first()?; + if first == TILESETS { + let mut command = vec![TILESETS.to_string()]; + if let Some(word) = &record.tilesets_word { + command.push(tilesets_word(word).to_string()); + } + return Some(command); + } + Some(clip_command(record.command.clone())) +} + +/// Arguments of an executed command, as the schema allows them: the leaf's +/// own, less [`NOT_PARAMS`], then the [`GLOBAL_PARAMS`]. `tilesets-cli`'s +/// forwarded words are not arguments of this CLI and are never sent. +fn params(record: &Record) -> Vec { + if record.invocation != Some(Invocation::Execute) { + return vec![]; + } + let leaf = record + .args + .iter() + .filter(|arg| !arg.global && !NOT_PARAMS.contains(&arg.id.as_str())); + let global = record + .args + .iter() + .filter(|arg| arg.global && GLOBAL_PARAMS.contains(&arg.id.as_str())); + leaf.chain(global) + .take(MAX_PARAMS) + .map(|arg| { + classify( + &arg.name, + &arg.values, + arg.takes_values, + arg.enumerated, + arg.numeric, + &record.body_fields, + ) + }) + .collect() +} + +fn network(record: &Record) -> Option { + if record.requests.is_empty() && !record.more_pages { + return None; + } + let mut network = Network { + more_pages: record.more_pages, + ..Network::default() + }; + for request in &record.requests { + network.request_count = network.request_count.saturating_add(1); + network.status = request.status; + network.response_bytes = sum(network.response_bytes, request.response_bytes); + network.request_body_bytes = sum(network.request_body_bytes, request.request_body_bytes); + network.network_ms = network + .network_ms + .saturating_add(request.elapsed.as_millis() as u64); + } + Some(network) +} + +/// The workflow this run adds, removes or runs. `name` is dropped for a +/// `Custom` workflow, whatever the caller passes. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow(source: WorkflowSource, name: Option<&str>) { + with_workflow(|workflow| *workflow = Some(workflow_field(source, name))); +} + +fn workflow_field(source: WorkflowSource, name: Option<&str>) -> Workflow { + let name = match source { + WorkflowSource::Custom => None, + _ => name.map(|name| clip(name, MAX_NAME)), + }; + Workflow { + source: source.as_str(), + name, + step_count: None, + steps: Vec::new(), + } +} + +/// How many steps the running workflow has, which can be more than the +/// [`MAX_STEPS`] recorded. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow_step_count(count: usize) { + with_workflow(|workflow| { + if let Some(workflow) = workflow.as_mut() { + workflow.step_count = Some(u32::try_from(count).unwrap_or(u32::MAX)); + } + }); +} + +/// A step that ran a `mapbox` command. `words` is the step's command line; +/// only the names the command tree has are kept, so an argument can't pass +/// for a command name. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) { + let step = cli_step(app, words, exit_code, error_code, duration); + with_workflow(|workflow| push_step(workflow, step)); +} + +/// A step that ran one of the user's own scripts. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_script_step(exit_code: Option, duration: Duration) { + let step = script_step(exit_code, duration); + with_workflow(|workflow| push_step(workflow, step)); +} + +fn cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) -> Step { + let path = run_record::tree_path(app, words.iter().map(|word| word.to_string())); + Step { + kind: "cli", + command: (!path.is_empty()).then(|| clip_command(path)), + exit_code, + error_code: error_code.map(|code| clip(code, MAX_CODE)), + duration_ms: duration.as_millis() as u64, + } +} + +fn script_step(exit_code: Option, duration: Duration) -> Step { + Step { + kind: "script", + command: None, + exit_code, + error_code: None, + duration_ms: duration.as_millis() as u64, + } +} + +/// Steps belong to a workflow: one reported before [`set_workflow`] has +/// nowhere to go and is dropped. +fn push_step(workflow: &mut Option, step: Step) { + if let Some(workflow) = workflow.as_mut() { + if workflow.steps.len() < MAX_STEPS { + workflow.steps.push(step); + } + } +} + +/// This run's event id, fixed on first use so a workflow can hand it to the +/// steps it launches before the event itself is built. +pub(crate) fn event_id() -> &'static str { + EVENT_ID.get_or_init(|| uuid_v4(rand::random())) +} + +/// The run that started this one, when a workflow step set it. Read back +/// only when it has the shape of an event id. +fn parent_event_id() -> Option { + let value = std::env::var(PARENT_EVENT_ENV).ok()?; + let value = value.trim(); + is_uuid(value).then(|| value.to_string()) +} + +fn sum(total: Option, more: Option) -> Option { + match (total, more) { + (Some(a), Some(b)) => Some(a.saturating_add(b)), + (a, b) => a.or(b), + } +} + +fn tilesets_word(word: &str) -> &str { + TILESETS_COMMANDS + .iter() + .find(|known| **known == word) + .copied() + .unwrap_or("other") +} + +fn clip_command(path: Vec) -> Vec { + path.into_iter() + .take(MAX_COMMAND_LEVELS) + .map(|name| clip(&name, MAX_COMMAND_NAME)) + .collect() +} + +fn classify( + name: &str, + values: &[String], + takes_values: bool, + enumerated: bool, + numeric: bool, + body_fields: &[String], +) -> Param { + let joined = values.join(","); + let mut param = Param { + name: clip(name, MAX_NAME), + ..Param::default() + }; + if COORDINATES.contains(&name) { + return param; + } + if !takes_values || enumerated || numeric || is_code(name, values) { + param.value = Some(clip(&joined, MAX_VALUE)); + return param; + } + match name { + "file" => { + param.bytes = values + .iter() + .filter_map(|path| std::fs::metadata(path).ok()) + .map(|meta| meta.len()) + .reduce(u64::saturating_add); + } + "data" => { + let (bytes, keys) = data_shape(&joined, body_fields); + param.bytes = bytes; + param.keys = keys; + } + _ => param.length = Some(joined.chars().count() as u64), + } + param +} + +/// Whether every item of an [`ALLOWLISTED`] value is a code: a language or +/// country code (`en`, `zh-Hans`, `pt-BR`, `us`) or a known feature type. +fn is_code(name: &str, values: &[String]) -> bool { + fn language_or_country(item: &str) -> bool { + let mut parts = item.splitn(2, ['-', '_']); + let head = parts.next().unwrap_or_default(); + (2..=3).contains(&head.len()) + && head.chars().all(|c| c.is_ascii_alphabetic()) + && parts.next().is_none_or(|tail| { + (2..=8).contains(&tail.len()) && tail.chars().all(|c| c.is_ascii_alphanumeric()) + }) + } + if !ALLOWLISTED.contains(&name) { + return false; + } + let items = || values.iter().flat_map(|v| v.split(',')).map(str::trim); + match name { + "language" | "country" => items().all(language_or_country), + "types" => items().all(|item| FEATURE_TYPES.contains(&item)), + _ => false, + } +} + +/// Size and top-level keys of a `--data` body: inline JSON, `@`, or +/// `@-` (stdin, which is not read twice, so nothing but the name). Only keys +/// in `declared` — the fields the operation's spec names — are kept; any +/// other is text the user chose. +fn data_shape(value: &str, declared: &[String]) -> (Option, Option>) { + if value == "@-" { + return (None, None); + } + let text = match value.strip_prefix('@') { + Some(path) => { + let Ok(meta) = std::fs::metadata(path) else { + return (None, None); + }; + // Parsed only when small enough to be a request body worth + // describing; the size alone is still recorded above that. + if meta.len() > MAX_DATA_TO_PARSE { + return (Some(meta.len()), None); + } + match std::fs::read_to_string(path) { + Ok(text) => text, + Err(_) => return (Some(meta.len()), None), + } + } + None => value.to_string(), + }; + let keys = serde_json::from_str::(&text) + .ok() + .and_then(|json| { + json.as_object().map(|object| { + object + .keys() + .filter(|key| declared.contains(key)) + .take(MAX_KEYS) + .map(|key| clip(key, MAX_NAME)) + .collect::>() + }) + }) + .filter(|keys| !keys.is_empty()); + (Some(text.len() as u64), keys) +} + +/// How this binary was installed, from where it lives. Only the category +/// leaves; the path does not. `MAPBOX_INSTALL_DIR` moves an install-script +/// binary anywhere, so those read as `other`. +fn install_method(exe: Option<&Path>) -> &'static str { + let Some(exe) = exe else { + return "other"; + }; + let path = exe + .to_string_lossy() + .replace('\\', "/") + .to_ascii_lowercase(); + if path.contains("/cellar/") || path.contains("/homebrew/") { + "homebrew" + } else if path.contains("/.cargo/bin/") { + "cargo" + } else if path.contains("/.local/bin/") || path.contains("/programs/mapbox/") { + "install-script" + } else { + "other" + } +} + +fn clip(text: &str, max_chars: usize) -> String { + text.chars().take(max_chars).collect() +} + +/// A random id, stored as ` ` and replaced by the +/// first run at least [`USER_ID_LIFETIME`] after it was created. Nothing +/// rotates it in the background; an unused CLI sends nothing either. +/// +/// Two first runs in parallel can each create one; `create_new` makes the +/// second read the first's instead of replacing it. When nothing can be +/// stored, the run still gets an id — just not one the next run will share. +fn user_id() -> String { + let now = SystemTime::now(); + let stored = telemetry_sink::read_state(USER_ID_FILE); + if let Some(id) = stored.as_deref().and_then(|s| live_user_id(s, now)) { + return id; + } + let fresh = uuid_v4(rand::random()); + let contents = format!("{fresh} {}", unix_seconds(now)); + if stored.is_some() { + telemetry_sink::replace_state(USER_ID_FILE, &contents); + return fresh; + } + if telemetry_sink::create_state(USER_ID_FILE, &contents) { + return fresh; + } + telemetry_sink::read_state(USER_ID_FILE) + .and_then(|s| live_user_id(&s, now)) + .unwrap_or(fresh) +} + +/// The stored id, unless it is malformed or due for replacement. One dated +/// in the future, from a clock that moved back, is replaced too: its age +/// can't be trusted. +fn live_user_id(stored: &str, now: SystemTime) -> Option { + let (id, created) = stored.split_once(' ')?; + let created = created.parse::().ok()?; + let age = unix_seconds(now).checked_sub(created)?; + (is_uuid(id) && age < USER_ID_LIFETIME.as_secs()).then(|| id.to_string()) +} + +fn unix_seconds(at: SystemTime) -> u64 { + at.duration_since(SystemTime::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) +} + +fn is_uuid(text: &str) -> bool { + text.len() == 36 + && text.char_indices().all(|(i, c)| match i { + 8 | 13 | 18 | 23 => c == '-', + _ => c.is_ascii_hexdigit(), + }) +} + +/// The version the last run recorded, when it differs from this one — the +/// first run after an upgrade. Checked against the same shape the update +/// check trusts, since it is read back from disk. +fn previous_version() -> Option { + let last = telemetry_sink::read_state(LAST_VERSION_FILE); + if last.as_deref() == Some(CURRENT) { + return None; + } + telemetry_sink::replace_state(LAST_VERSION_FILE, CURRENT); + last.filter(|version| is_version(version)) + .map(|version| clip(&version, MAX_VERSION)) +} + +fn is_version(text: &str) -> bool { + !text.is_empty() + && text.len() <= MAX_VERSION + && text + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')) +} + +fn timestamp(at: SystemTime) -> String { + crate::dated_jsonl::timestamp(at) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::UNIX_EPOCH; + + #[test] + fn a_script_step_records_only_how_it_exited_and_how_long_it_took() { + let step = script_step(Some(3), Duration::from_millis(2300)); + assert_eq!( + serde_json::to_value(&step).unwrap(), + serde_json::json!({ "kind": "script", "exitCode": 3, "durationMs": 2300 }) + ); + } + + #[test] + fn a_cli_step_keeps_only_names_the_command_tree_has() { + let app = Command::new("mapbox") + .subcommand(Command::new("styles").subcommand(Command::new("get"))); + let step = cli_step( + &app, + &["styles", "get", "my-secret-style"], + Some(0), + None, + Duration::ZERO, + ); + assert_eq!( + step.command, + Some(vec!["styles".to_string(), "get".to_string()]) + ); + + let unknown = cli_step( + &app, + &["/Users/someone/run.sh"], + Some(1), + Some("error"), + Duration::ZERO, + ); + assert_eq!(unknown.command, None); + assert_eq!(unknown.error_code.as_deref(), Some("error")); + } + + #[test] + fn steps_need_a_workflow_and_stop_at_the_bound() { + let mut workflow = None; + push_step(&mut workflow, script_step(Some(0), Duration::ZERO)); + assert!(workflow.is_none(), "a step without a workflow is dropped"); + + workflow = Some(workflow_field(WorkflowSource::Builtin, Some("style-clone"))); + for _ in 0..MAX_STEPS + 5 { + push_step(&mut workflow, script_step(Some(0), Duration::ZERO)); + } + assert_eq!(workflow.unwrap().steps.len(), MAX_STEPS); + } + + #[test] + fn a_custom_workflow_never_records_its_name() { + assert_eq!( + workflow_field(WorkflowSource::Custom, Some("acme-client-export")).name, + None + ); + assert_eq!( + workflow_field(WorkflowSource::Marketplace, Some("style-clone")).name, + Some("style-clone".to_string()) + ); + assert_eq!(workflow_field(WorkflowSource::Builtin, None).name, None); + } + + #[test] + fn a_uuid_is_version_4_and_well_formed() { + let id = uuid_v4([0xff; 16]); + assert!(is_uuid(&id), "{id}"); + assert_eq!(&id[14..15], "4"); + assert!(matches!(&id[19..20], "8" | "9" | "a" | "b"), "{id}"); + assert!(!is_uuid("not-a-uuid")); + } + + #[test] + fn timestamps_are_utc_to_the_millisecond() { + let at = UNIX_EPOCH + Duration::from_millis(1_790_000_000_123); + assert_eq!(timestamp(at), "2026-09-21T14:13:20.123Z"); + assert_eq!(timestamp(UNIX_EPOCH), "1970-01-01T00:00:00.000Z"); + } + + #[test] + fn free_strings_send_their_length_and_codes_send_their_value() { + let values = |v: &str| vec![v.to_string()]; + let q = classify( + "q", + &values("1600 Pennsylvania Ave"), + true, + false, + false, + &[], + ); + assert_eq!(q.value, None); + assert_eq!(q.length, Some(21)); + + // Digits are still a free string unless the spec types them. + let postcode = classify("postcode", &values("10001"), true, false, false, &[]); + assert_eq!((postcode.value, postcode.length), (None, Some(5))); + + let limit = classify("limit", &values("5"), true, false, true, &[]); + assert_eq!(limit.value.as_deref(), Some("5")); + + let language = classify("language", &values("en"), true, false, false, &[]); + assert_eq!(language.value.as_deref(), Some("en")); + + let flag = classify("download", &values("true"), false, false, false, &[]); + assert_eq!(flag.value.as_deref(), Some("true")); + } + + #[test] + fn coordinates_send_their_name_only() { + for name in COORDINATES { + let param = classify(name, &["12.5".to_string()], true, false, true, &[]); + assert_eq!( + param, + Param { + name: name.to_string(), + ..Param::default() + } + ); + } + } + + #[test] + fn a_data_body_sends_its_size_and_only_the_keys_its_spec_declares() { + let declared = ["layers".to_string(), "name".to_string()]; + let (bytes, keys) = data_shape( + r#"{"name":"secret","layers":[],"acme-client-x":1}"#, + &declared, + ); + assert_eq!(bytes, Some(47)); + assert_eq!(keys, Some(vec!["layers".to_string(), "name".to_string()])); + + // With nothing declared, no key the user wrote can pass. + assert_eq!(data_shape(r#"{"name":"x"}"#, &[]), (Some(12), None)); + assert_eq!(data_shape("[1,2]", &declared), (Some(5), None)); + assert_eq!(data_shape("@-", &declared), (None, None)); + } + + #[test] + fn the_user_id_is_replaced_once_it_is_a_day_old() { + let id = uuid_v4([0x11; 16]); + let created = 1_790_000_000; + let at = |secs: u64| UNIX_EPOCH + Duration::from_secs(secs); + let stored = format!("{id} {created}"); + + assert_eq!(live_user_id(&stored, at(created)), Some(id.clone())); + assert_eq!( + live_user_id(&stored, at(created + USER_ID_LIFETIME.as_secs() - 1)), + Some(id.clone()) + ); + assert_eq!( + live_user_id(&stored, at(created + USER_ID_LIFETIME.as_secs())), + None + ); + assert_eq!( + live_user_id(&stored, at(created - 1)), + None, + "from the future" + ); + assert_eq!(live_user_id(&id, at(created)), None, "no creation time"); + assert_eq!(live_user_id("not-a-uuid 1790000000", at(created)), None); + } + + #[test] + fn no_request_id_is_sent() { + let mut record = Record::default(); + record.requests = vec![run_record::Request { + method: "GET".to_string(), + url: "https://api.mapbox.com/styles/v1/x".to_string(), + status: Some(200), + request_id: Some("Hbq3kQ2x9mTzL0pWvR7yN4cD8sF1gJ6aE5uK".to_string()), + request_body_bytes: None, + response_bytes: Some(10), + elapsed: Duration::from_millis(5), + error: None, + }]; + let sent = serde_json::to_string(&network(&record)).unwrap(); + assert!( + !sent.contains("Hbq3kQ2x9mTzL0pWvR7yN4cD8sF1gJ6aE5uK"), + "{sent}" + ); + assert!(sent.contains("\"requestCount\":1"), "{sent}"); + } + + #[test] + fn a_long_value_is_clipped_to_the_schema_bound() { + let param = classify("mode", &["x".repeat(500)], true, true, false, &[]); + assert_eq!(param.value.map(|v| v.len()), Some(MAX_VALUE)); + } + + #[test] + fn allowlisted_values_go_out_only_when_they_are_codes() { + for (name, value) in [ + ("language", "en"), + ("language", "zh-Hans,pt-BR"), + ("country", "us,cn"), + ("types", "address,poi"), + ] { + let param = classify(name, &[value.to_string()], true, false, false, &[]); + assert_eq!(param.value.as_deref(), Some(value), "{name}={value}"); + } + for (name, value) in [ + ("country", "my secret project name"), + ("language", "acme-corporation-internal"), + ("types", "notes about client acme"), + ("types", "address,acme"), + ] { + let param = classify(name, &[value.to_string()], true, false, false, &[]); + assert_eq!( + (param.value, param.length), + (None, Some(value.chars().count() as u64)), + "{name}={value}" + ); + } + } + + #[test] + fn an_unknown_tilesets_word_is_other() { + assert_eq!(tilesets_word("upload-source"), "upload-source"); + assert_eq!(tilesets_word("my-secret-tileset"), "other"); + } + + #[test] + fn the_install_method_is_a_category_never_the_path() { + let method = |p: &str| install_method(Some(Path::new(p))); + assert_eq!( + method("/opt/homebrew/Cellar/mapbox/0.3.0/bin/mapbox"), + "homebrew" + ); + assert_eq!(method("/Users/a/.cargo/bin/mapbox"), "cargo"); + assert_eq!(method("/home/a/.local/bin/mapbox"), "install-script"); + assert_eq!( + method(r"C:\Users\a\AppData\Local\Programs\mapbox\mapbox.exe"), + "install-script" + ); + assert_eq!(method("/srv/tools/mapbox"), "other"); + assert_eq!(install_method(None), "other"); + } + + /// Every field path the `cli.command` schema in mapbox/event-schema + /// declares. Ingestion accepts undeclared fields, so this list is what + /// holds "only declared fields are sent": add a path here only together + /// with the schema. + const DECLARED_FIELDS: &[&str] = &[ + "event", + "version", + "created", + "eventId", + "parentEventId", + "userId", + "sdkIdentifier", + "sdkVersion", + "operatingSystem", + "command", + "invocation", + "workflow", + "workflow.source", + "workflow.name", + "workflow.stepCount", + "workflow.steps", + "workflow.steps[].kind", + "workflow.steps[].command", + "workflow.steps[].exitCode", + "workflow.steps[].errorCode", + "workflow.steps[].durationMs", + "params", + "params[].name", + "params[].value", + "params[].length", + "params[].bytes", + "params[].keys", + "usageError", + "output", + "outputSource", + "dryRun", + "debug", + "yes", + "profile", + "timeoutSeconds", + "auth", + "auth.source", + "auth.type", + "auth.account", + "exitCode", + "errorCode", + "stdoutBytes", + "durationMs", + "authStep", + "updateNotice", + "previousVersion", + "network", + "network.requestCount", + "network.status", + "network.responseBytes", + "network.requestBodyBytes", + "network.networkMs", + "network.morePages", + "cli", + "cli.buildChannel", + "cli.buildId", + "cli.installMethod", + "env", + "env.arch", + "env.ci", + "env.agent", + "env.stdinTty", + "env.stdoutTty", + ]; + + /// An event with every optional field set, so `skip_serializing_if` + /// hides nothing from the check below. + fn fully_populated_event() -> Event { + let words = || Some(vec!["styles".to_string(), "get".to_string()]); + Event { + event: EVENT, + version: SCHEMA_VERSION, + created: "2026-09-21T14:13:20.123Z".to_string(), + event_id: uuid_v4([0x11; 16]), + parent_event_id: Some(uuid_v4([0x22; 16])), + user_id: uuid_v4([0x33; 16]), + sdk_identifier: SDK_IDENTIFIER, + sdk_version: CURRENT, + operating_system: "macos", + command: words(), + invocation: Some("execute"), + workflow: Some(Workflow { + source: "builtin", + name: Some("style-clone".to_string()), + step_count: Some(1), + steps: vec![Step { + kind: "cli", + command: words(), + exit_code: Some(1), + error_code: Some("not_found".to_string()), + duration_ms: 10, + }], + }), + params: vec![Param { + name: "data".to_string(), + value: Some("x".to_string()), + length: Some(1), + bytes: Some(2), + keys: Some(vec!["name".to_string()]), + }], + usage_error: Some("missing_argument".to_string()), + output: Some("json"), + output_source: Some("flag"), + dry_run: Some(false), + debug: Some(false), + yes: Some(false), + profile: Some("default"), + timeout_seconds: Some(30.0), + auth: Some(Auth { + source: "env", + kind: "pk", + account: Some("someone".to_string()), + }), + exit_code: Some(1), + error_code: Some("not_found".to_string()), + stdout_bytes: 3, + duration_ms: 4, + auth_step: Some("login"), + update_notice: Some("9.9.9".to_string()), + previous_version: Some("0.1.0".to_string()), + network: Some(Network { + request_count: 1, + status: Some(404), + response_bytes: Some(5), + request_body_bytes: Some(6), + network_ms: 7, + more_pages: true, + }), + cli: Cli { + build_channel: Some("release"), + build_id: Some("abc123"), + install_method: "cargo", + }, + env: Env { + arch: "aarch64", + ci: false, + agent: Some("claude-code"), + stdin_tty: false, + stdout_tty: false, + }, + } + } + + /// Every object key in `value` as a dotted path, with `[]` for an array's + /// elements. An array of strings is a leaf: its elements have no keys. + fn field_paths(value: &serde_json::Value, prefix: &str, paths: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + for (key, child) in object { + let path = if prefix.is_empty() { + key.clone() + } else { + format!("{prefix}.{key}") + }; + field_paths(child, &path, paths); + paths.push(path); + } + } + serde_json::Value::Array(items) => { + for item in items { + field_paths(item, &format!("{prefix}[]"), paths); + } + } + _ => {} + } + } + + #[test] + fn the_event_sends_only_fields_the_schema_declares() { + let sent = serde_json::to_value(fully_populated_event()).unwrap(); + let mut paths = vec![]; + field_paths(&sent, "", &mut paths); + paths.sort(); + paths.dedup(); + + let undeclared: Vec<_> = paths + .iter() + .filter(|path| !DECLARED_FIELDS.contains(&path.as_str())) + .collect(); + assert!( + undeclared.is_empty(), + "the cli.command event sends fields its schema does not declare:\n {}\n\n\ + Ingestion accepts undeclared fields, so this test is the only thing holding \ + the promise that nothing else leaves the machine. Add each field to the \ + cli.command schema in mapbox/event-schema first, then to DECLARED_FIELDS.", + undeclared + .iter() + .map(|p| p.as_str()) + .collect::>() + .join("\n ") + ); + + let unset: Vec<_> = DECLARED_FIELDS + .iter() + .filter(|path| !paths.iter().any(|p| p == *path)) + .copied() + .collect(); + assert!( + unset.is_empty(), + "these declared fields are missing from the fully populated event:\n {}\n\n\ + Either fully_populated_event leaves them unset, so the check above can't see \ + them, or the event no longer has them. Set them in the fixture, or remove them \ + from DECLARED_FIELDS once the event has really dropped them.", + unset.join("\n ") + ); + } +} diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs new file mode 100644 index 0000000..ec48954 --- /dev/null +++ b/src/telemetry_sink.rs @@ -0,0 +1,384 @@ +//! Where the run's telemetry event goes once [`crate::telemetry_event`] has +//! built it. +//! +//! This module never decides what an event contains; it is handed one +//! finished JSON line. That keeps the privacy rules in one file and lets a +//! delivery change — a new endpoint, batching — happen without touching them. +//! +//! [`deliver`] hands the event to [`HttpSink`], which sends it to Mapbox +//! Events, when [`crate::cli_token`] has a token it could send it with. +//! Otherwise the event is dropped: nothing would ever send a copy kept on +//! disk, so keeping one would only store usage data for nobody. The state +//! files below are still written, since building the event reads them. +//! +//! It also owns the directory, `~/.mapbox/.telemetry` (or under +//! `$MAPBOX_CONFIG_DIR`), written only once the config directory exists: the +//! two small state files the event reads — the installation id and the last +//! version seen — and the delivery log, written and pruned through +//! [`crate::dated_jsonl`]. + +use std::io::{Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitCode, Stdio}; +use std::sync::OnceLock; +use std::time::{Duration, Instant, SystemTime}; + +use reqwest::header::{CONTENT_TYPE, USER_AGENT}; +use serde_json::json; + +use crate::auth; +use crate::cli_token; +use crate::dated_jsonl; +use crate::http; +use crate::telemetry; + +const DIR: &str = ".telemetry"; +/// Days of delivery-log files kept, today included. +const KEEP_DAYS: u64 = 7; + +/// Production Mapbox Events, for every build. A login is a production login, +/// so a build from source sends where a release does rather than to a +/// staging host its tokens mean nothing to. +const ENDPOINT: &str = "https://events.mapbox.com/events/v2"; + +/// Set by [`HttpSink`] on the child it spawns, and read by [`is_send_child`] +/// at the top of `main` — a mode of this binary rather than a hidden +/// subcommand, for the reason `update_check` gives. +const SEND_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_SEND"; + +/// The URL to send to, overriding [`ENDPOINT`] — in a build that is not a +/// production release only (see [`send_url`]). The token is added by +/// [`cli_token::send`], never carried here. The parent hands it to the child; +/// it is also the seam `tests/telemetry_events.rs` points at a loopback +/// server, and how a developer reaches staging +/// (`https://api-events-staging.tilestream.net/events/v2`). +const URL_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_URL"; + +/// The token the user gave this run, from [`remember_user_token`]. Kept +/// apart from the run record, which goes to history and must never hold a +/// token. +static USER_TOKEN: OnceLock> = OnceLock::new(); + +/// Turns on the delivery log: one line when an event is handed to the child +/// and one with how the send ended, in `deliveries/.jsonl` under +/// this module's directory. For measuring the send's success rate while +/// debugging, so it is off unless set. Counting the hand-offs, not only the +/// outcomes, is what shows a child that died before reporting — a CI +/// container torn down right after the command, say. +const DELIVERY_LOG_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_LOG"; +const DELIVERY_DIR: &str = "deliveries"; + +/// Each attempt's budget. Nobody is waiting for the child; with every token +/// in [`cli_token::TELEMETRY`] tried it can run several of these. +const SEND_TIMEOUT: Duration = Duration::from_secs(5); + +/// Ample for one event and its envelope; anything larger on stdin is not one +/// of ours. +const MAX_EVENT_BYTES: u64 = 64 * 1024; + +/// Sends each event to Mapbox Events from a detached child, so the command +/// never waits on the network. +/// +/// The event goes to the child on stdin rather than argv, which is visible +/// in `ps`, and so does the user's token, which would otherwise sit in the +/// child's environment. The child sends through [`cli_token::send`], which +/// may refresh the login: the reason it runs there and not in the parent, +/// which is about to exit. One attempt per token; the event is dropped on +/// any failure. Best-effort by contract: it reports nothing and must not fail +/// the command, block its exit, or write to stdout. +struct HttpSink { + url: String, + user_token: Option, + profile: Option, +} + +/// What the parent writes to the child's stdin. +#[derive(serde::Serialize, serde::Deserialize)] +struct Envelope { + event: serde_json::Value, + user_token: Option, + profile: Option, +} + +impl HttpSink { + /// Detached the way `update_check::spawn_refresh` is, and for the same + /// reasons; stdout and stderr are `/dev/null` so nothing the child does + /// can land on the parent's terminal after it has exited. + fn deliver(&self, line: &str) { + let Ok(exe) = std::env::current_exe() else { + log_delivery(line, json!({ "stage": "spawn_failed" })); + return; + }; + let mut command = Command::new(exe); + // The user's token travels in the envelope; the child has no use for + // it in its environment. + command + .env(SEND_ENV, "1") + .env(URL_ENV, &self.url) + .env_remove(auth::CLAP_TOKEN_ENV) + .env_remove("MapboxAccessToken") + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + crate::detach::detach(&mut command); + + let Ok(event) = serde_json::from_str(line) else { + log_delivery(line, json!({ "stage": "bad_event" })); + return; + }; + let envelope = Envelope { + event, + user_token: self.user_token.clone(), + profile: self.profile.clone(), + }; + let Ok(envelope) = serde_json::to_string(&envelope) else { + log_delivery(line, json!({ "stage": "bad_event" })); + return; + }; + let Ok(mut child) = command.spawn() else { + log_delivery(line, json!({ "stage": "spawn_failed" })); + return; + }; + // Before the write: once the child has its input it can log its + // outcome, and the hand-off must come first in the log. + log_delivery(line, json!({ "stage": "queued" })); + // One event fits in a pipe buffer, so this does not wait on the + // child. Dropping `stdin` closes it, which is the child's end of input. + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(envelope.as_bytes()); + } + // Not waited on: the parent is about to exit. + } +} + +/// Keeps the token the user gave this run for [`deliver`], resolved by +/// [`cli_token::user_token`] while the arguments are still at hand. +pub(crate) fn remember_user_token(token: Option) { + let _ = USER_TOKEN.set(token); +} + +/// Hands `line` to [`HttpSink`] when [`cli_token`] has a token it could send +/// it with, and drops it otherwise. `profile` is the run's `--profile`, whose +/// login it may send with. +pub(crate) fn deliver(line: &str, profile: Option<&str>) { + let url = send_url(is_production_build(), from_environment(URL_ENV)); + // Help, `--version` and usage errors return before the arguments are + // parsed, so nothing was remembered; the environment is still the user's. + let user_token = match USER_TOKEN.get() { + Some(remembered) => remembered.clone(), + None => from_environment(auth::CLAP_TOKEN_ENV), + }; + if cli_token::has_token_for(cli_token::TELEMETRY, user_token.as_deref(), profile) { + HttpSink { + url, + user_token, + profile: profile.map(str::to_owned), + } + .deliver(line) + } else { + log_delivery(line, json!({ "stage": "no_token" })); + } +} + +/// Whether this process is the sender rather than a command. +pub fn is_send_child() -> bool { + from_environment(SEND_ENV).is_some() +} + +/// The whole of the child: read the event from stdin and post it. +/// +/// Always succeeds, and says nothing; nobody reads its exit code. The opt-out +/// is checked again here, because this is the process that makes the request. +pub fn run_send_child() -> ExitCode { + if telemetry::telemetry_allowed() { + // Through `send_url` again, so a production child cannot be pointed + // elsewhere either. + let url = send_url(is_production_build(), from_environment(URL_ENV)); + let mut input = String::new(); + let read = std::io::stdin() + .take(MAX_EVENT_BYTES) + .read_to_string(&mut input); + match (read, serde_json::from_str::(input.trim())) { + (Ok(_), Ok(envelope)) => send(&url, &envelope), + _ => log_delivery("", json!({ "stage": "bad_envelope" })), + } + } + ExitCode::SUCCESS +} + +/// Posts `[]`, the batch shape Mapbox Events takes. The `User-Agent` +/// is the product token alone: Mapbox Events stores it in every record, and +/// the markers `http::client` adds (an `agent/` among them) must not ride +/// along. +fn send(url: &str, envelope: &Envelope) { + let line = envelope.event.to_string(); + let outcome = match post(url, envelope) { + Ok((status, elapsed)) => json!({ + "stage": "responded", + "status": status, + "elapsedMs": elapsed.as_millis() as u64, + }), + Err(error) => json!({ "stage": "failed", "error": error }), + }; + log_delivery(&line, outcome); +} + +/// The response's status and how long it took, or why there was none. +fn post(url: &str, envelope: &Envelope) -> Result<(u16, Duration), String> { + let body = serde_json::to_string(&[&envelope.event]).map_err(|err| err.to_string())?; + let client = http::client().map_err(|err| err.to_string())?; + let started = Instant::now(); + let response = cli_token::send( + cli_token::TELEMETRY, + envelope.user_token.as_deref(), + envelope.profile.as_deref(), + || { + client + .post(url) + .header(USER_AGENT, telemetry::PRODUCT_TOKEN) + .header(CONTENT_TYPE, "application/json") + .body(body.clone()) + .timeout(SEND_TIMEOUT) + }, + ) + .ok_or_else(|| "no token to send with".to_string())? + .map_err(|err| http::failure(&err))?; + Ok((response.status().as_u16(), started.elapsed())) +} + +/// Appends one line to the delivery log, when [`DELIVERY_LOG_ENV`] is set. +/// Keyed by the event's `eventId`, so a hand-off and its outcome pair up. +fn log_delivery(line: &str, mut entry: serde_json::Value) { + if from_environment(DELIVERY_LOG_ENV).is_none() { + return; + } + let Some(dir) = dir().map(|dir| dir.join(DELIVERY_DIR)) else { + return; + }; + if std::fs::create_dir_all(&dir).is_err() { + return; + } + let now = SystemTime::now(); + let event_id = serde_json::from_str::(line) + .ok() + .and_then(|event| event.get("eventId").cloned()); + entry["eventId"] = event_id.unwrap_or(serde_json::Value::Null); + entry["at"] = json!(now + .duration_since(SystemTime::UNIX_EPOCH) + .map_or(0, |since| since.as_millis() as u64)); + dated_jsonl::append(&dir, &entry.to_string(), now, KEEP_DAYS); +} + +fn from_environment(name: &str) -> Option { + let value = std::env::var(name).ok()?; + let value = value.trim(); + (!value.is_empty()).then(|| value.to_string()) +} + +/// Where to send: [`ENDPOINT`], or [`URL_ENV`]'s override in a build that is +/// not a production release. A release ignores the override, so an injected +/// variable cannot send a user's token to a host of its choosing. +fn send_url(production: bool, override_url: Option) -> String { + match override_url { + Some(url) if !production => url, + _ => ENDPOINT.to_string(), + } +} + +/// `MAPBOX_CLI_BUILD_ENV` is how the release pipeline marks a production +/// build; see `update_check`. +const fn is_production_build() -> bool { + match option_env!("MAPBOX_CLI_BUILD_ENV") { + Some(env) => same(env, "production"), + None => false, + } +} + +/// `==` on `&str`, which is not available in a `const` context. +const fn same(a: &str, b: &str) -> bool { + let (a, b) = (a.as_bytes(), b.as_bytes()); + if a.len() != b.len() { + return false; + } + let mut i = 0; + while i < a.len() { + if a[i] != b[i] { + return false; + } + i += 1; + } + true +} + +/// The contents of a state file, trimmed, if it is there. +pub(crate) fn read_state(name: &str) -> Option { + let text = std::fs::read_to_string(dir()?.join(name)).ok()?; + Some(text.trim().to_string()) +} + +/// Creates a state file, only if it does not exist yet. `false` when it +/// already did or could not be written — two first runs racing each get +/// one answer this way, rather than the second replacing the first. +pub(crate) fn create_state(name: &str, contents: &str) -> bool { + let Some(dir) = dir() else { + return false; + }; + match create_private(&dir.join(name)) { + Ok(mut file) => file.write_all(contents.as_bytes()).is_ok(), + Err(_) => false, + } +} + +/// Replaces a state file's contents. +pub(crate) fn replace_state(name: &str, contents: &str) { + let Some(dir) = dir() else { + return; + }; + let path = dir.join(name); + let _ = std::fs::remove_file(&path); + if let Ok(mut file) = create_private(&path) { + let _ = file.write_all(contents.as_bytes()); + } +} + +/// Creates `path` `0600`, failing if it already exists. +fn create_private(path: &Path) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + +/// The directory, created `0700` inside the config directory — only when +/// that already exists. Recording must not be what creates `~/.mapbox` or +/// changes its permissions: read-only commands promise to leave it alone +/// (`tests/auth_profiles.rs`, `tests/non_interactive.rs`). `None` otherwise, +/// and nothing is written. +/// +/// Stricter than command history, which creates a missing config directory +/// because history has to work for someone who has never logged in; this +/// directory only holds the event's state files and the delivery log. +fn dir() -> Option { + if !auth::config_dir_path()?.is_dir() { + return None; + } + dated_jsonl::private_dir(DIR) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_a_build_that_is_not_production_takes_the_url_override() { + let custom = || Some("http://127.0.0.1:1/events/v2".to_string()); + assert_eq!(send_url(false, custom()), "http://127.0.0.1:1/events/v2"); + assert_eq!(send_url(true, custom()), ENDPOINT); + assert_eq!(send_url(false, None), ENDPOINT); + assert_eq!(send_url(true, None), ENDPOINT); + } +} diff --git a/src/update_check.rs b/src/update_check.rs index 649c92e..8171b42 100644 --- a/src/update_check.rs +++ b/src/update_check.rs @@ -463,19 +463,7 @@ fn spawn_refresh(url: &str) { .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); - - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - command.process_group(0); - } - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const DETACHED_PROCESS: u32 = 0x0000_0008; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - command.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW); - } + crate::detach::detach(&mut command); // Not waited on, and not reaped: the parent is one statement from // exiting, at which point the child is reparented and finishes on its diff --git a/tests/auth_profiles.rs b/tests/auth_profiles.rs index fbc70eb..c5bdf0e 100644 --- a/tests/auth_profiles.rs +++ b/tests/auth_profiles.rs @@ -45,6 +45,9 @@ fn write_credentials(dir: &Path, filename: &str, account: &str, exp: Option fn command(home: &Path) -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/completion.rs b/tests/completion.rs index 2e9fb56..886d45f 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -41,6 +41,9 @@ fn sandbox_home() -> PathBuf { fn command() -> Command { let home = sandbox_home(); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/config.rs b/tests/config.rs index ca1caef..3357850 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -23,6 +23,9 @@ fn config_dir(home: &Path) -> PathBuf { /// credential store the same way `tests/update_check.rs` isolates it. fn command(home: &Path) -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/diagnostic_log.rs b/tests/diagnostic_log.rs index 16d54f4..e8fdf93 100644 --- a/tests/diagnostic_log.rs +++ b/tests/diagnostic_log.rs @@ -41,6 +41,9 @@ fn history_dir(home: &Path) -> PathBuf { fn command(home: &Path) -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/docs_contract.rs b/tests/docs_contract.rs index e3a302e..c94f4fb 100644 --- a/tests/docs_contract.rs +++ b/tests/docs_contract.rs @@ -69,6 +69,8 @@ fn sandbox_home() -> PathBuf { fn schema() -> Value { let home = sandbox_home(); let out = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/dry_run.rs b/tests/dry_run.rs index 2cd4e42..0f8ca90 100644 --- a/tests/dry_run.rs +++ b/tests/dry_run.rs @@ -38,6 +38,9 @@ fn config_dir(test: &str) -> PathBuf { /// the shape of every assertion. fn command(config: &Path) -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/generate_skills.rs b/tests/generate_skills.rs index 99ac164..1e01316 100644 --- a/tests/generate_skills.rs +++ b/tests/generate_skills.rs @@ -34,6 +34,9 @@ fn scratch(name: &str) -> PathBuf { fn command(name: &str) -> Command { let home = scratch(&format!("{name}-home")); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/history.rs b/tests/history.rs index bc3ff80..0c57cb2 100644 --- a/tests/history.rs +++ b/tests/history.rs @@ -48,6 +48,8 @@ fn command(home: &Path) -> Command { .env_remove("NO_PROXY") .env_remove("no_proxy") .env("MAPBOX_NO_UPDATE_CHECK", "1") + // Telemetry keeps its own directory beside `history/`. + .env("MAPBOX_CLI_NO_TELEMETRY", "1") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", config_dir(home)); diff --git a/tests/mcp_proxy.rs b/tests/mcp_proxy.rs index fee31cd..51f61ce 100644 --- a/tests/mcp_proxy.rs +++ b/tests/mcp_proxy.rs @@ -83,6 +83,9 @@ fn isolate_every_client(cmd: &mut Command) -> &mut Command { /// Runs `mapbox mcp ` with `MAPBOX_CLAUDE_CLI` pointed at the stub. fn run(stub: &Path, marker: &Path, get_exit: &str, add_exit: &str, args: &[&str]) -> Output { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); isolate_every_client(&mut cmd); cmd.arg("mcp") .args(args) @@ -245,6 +248,9 @@ fn a_client_not_on_path_is_skipped_and_named() { // Every client's binary override points at nothing at all, so // auto-detection finds none of them reachable. let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); isolate_every_client(&mut cmd); let output = cmd .args(["mcp", "install"]) @@ -336,6 +342,9 @@ fn run_codex( args: &[&str], ) -> Output { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); isolate_every_client(&mut cmd); cmd.arg("mcp") .args(args) @@ -478,6 +487,9 @@ fn run_vscode( args: &[&str], ) -> Output { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); isolate_every_client(&mut cmd); cmd.arg("mcp") .args(args) @@ -701,6 +713,9 @@ fn cursor_reads_its_own_settings_json_shape() { .expect("seed settings.json"); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); isolate_every_client(&mut cmd); let output = cmd .arg("mcp") diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index a5475dd..d08f5a2 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -33,6 +33,9 @@ fn scratch(test: &str) -> PathBuf { /// these assertions pass for the wrong reason. fn command(home: &Path) -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") @@ -347,6 +350,8 @@ fn under_a_pty(home: &Path, out_path: &Path, args: &str, expect: &str) -> Option .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") .env_remove("MAPBOX_YES") + .env_remove("MAPBOX_CLI_TOKEN") + .env("MAPBOX_CLI_NO_TELEMETRY", "1") .env("HOME", home) .env("MAPBOX_CONFIG_DIR", home.join("config")) .stdin(std::process::Stdio::null()) diff --git a/tests/output_contract.rs b/tests/output_contract.rs index 2d34129..bca6ffe 100644 --- a/tests/output_contract.rs +++ b/tests/output_contract.rs @@ -45,6 +45,9 @@ fn sandbox_home() -> PathBuf { fn command() -> Command { let home = sandbox_home(); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") @@ -100,6 +103,8 @@ fn a_file_where_the_credential_directory_goes_fails_before_the_browser() { std::fs::write(home.join(".mapbox"), "pk.a-legacy-token").expect("write the legacy file"); let out = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["-o", "text", "auth", "login"]) .env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") @@ -155,6 +160,8 @@ fn a_command_that_is_not_about_credentials_warns_in_one_line() { std::fs::write(home.join(".mapbox"), "pk.a-legacy-token").expect("write the legacy file"); let out = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["-o", "text", "styles", "list"]) .env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") @@ -460,6 +467,9 @@ fn an_agent_is_pointed_at_the_schema() { fn without_an_agent_the_help_has_no_hint() { let home = sandbox_home(); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_clear() .env("HOME", &home) .env("MAPBOX_CONFIG_DIR", home.join(".mapbox")); diff --git a/tests/proxy.rs b/tests/proxy.rs index f8755d6..238baae 100644 --- a/tests/proxy.rs +++ b/tests/proxy.rs @@ -129,6 +129,9 @@ fn accept_within( /// test waiting for a connection that was never going to come. fn mapbox() -> Command { let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.args(["styles", "list", "--username", "someone", "-o", "json"]) .env_remove("HTTP_PROXY") .env_remove("HTTPS_PROXY") diff --git a/tests/schema_contract.rs b/tests/schema_contract.rs index 03984ec..6ceb64c 100644 --- a/tests/schema_contract.rs +++ b/tests/schema_contract.rs @@ -32,6 +32,9 @@ fn sandbox_home() -> PathBuf { fn command() -> Command { let home = sandbox_home(); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + // Telemetry is not under test here, and would post to Mapbox. + cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN"); cmd.env_remove("MAPBOX_ACCESS_TOKEN") .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") diff --git a/tests/source_guards.rs b/tests/source_guards.rs index cc38221..24b9d5d 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -44,6 +44,7 @@ fn sources() -> Vec<(String, String)> { /// size limit, matched by exact `YYYY-MM-DD.jsonl` names inside the /// directory it writes to, and the scratch file a trim leaves when its /// rename fails. +/// - `telemetry_sink` — a state file in `~/.mapbox/.telemetry` it replaces. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. @@ -55,6 +56,7 @@ const MAY_DELETE: &[&str] = &[ "executor.rs", "generate_skills.rs", "skill_dest.rs", + "telemetry_sink.rs", "uninstall.rs", ]; @@ -514,3 +516,31 @@ fn prose_is_american_english() { found.join("\n ") ); } + +/// Every end-to-end test file that runs the binary says what it does about +/// telemetry. +/// +/// A run with a token posts an event to Mapbox from a detached child, so a +/// test that spawns `mapbox` with a fake `--token` — or on a machine with +/// `MAPBOX_CLI_TOKEN` exported — would send real requests, and could fall back +/// to the developer's own login. Naming `MAPBOX_CLI_NO_TELEMETRY` is the +/// decision this asks for: set it, or test telemetry on purpose as +/// `telemetry_events.rs`, `doctor.rs` and `update_check.rs` do. +#[test] +fn every_test_that_runs_the_binary_decides_about_telemetry() { + let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests"); + let mut undecided: Vec = files_under(&dir, &["rs"]) + .into_iter() + .filter_map(|(name, path)| { + let text = std::fs::read_to_string(&path).expect("read a test"); + (text.contains("CARGO_BIN_EXE_mapbox") && !text.contains("MAPBOX_CLI_NO_TELEMETRY")) + .then_some(name) + }) + .collect(); + undecided.sort(); + assert!( + undecided.is_empty(), + "these run `mapbox` without setting MAPBOX_CLI_NO_TELEMETRY, so a run with a \ + token sends real telemetry: {undecided:?}. Set it on the command they build." + ); +} diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs new file mode 100644 index 0000000..78fbec6 --- /dev/null +++ b/tests/telemetry_events.rs @@ -0,0 +1,690 @@ +//! End-to-end tests for the run's `cli.command` event. +//! +//! The unit tests in `src/telemetry_event.rs` cover the pure parts — how an +//! argument is classified, what a timestamp looks like. What they cannot show is what a real run leaves behind: that the +//! event lands where it should, carries what the command did and nothing the +//! user typed, disappears when telemetry is off, and never changes stdout. +//! +//! `MAPBOX_INTERNAL_TELEMETRY_URL` points the binary at a loopback server, +//! which is where these tests read the event. `cargo test` builds no token +//! in, and a run with no token drops its event, so most runs here are given +//! `MAPBOX_CLI_TOKEN` to send with. + +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +use serde_json::Value; + +/// A token whose payload claims the account `example-user`. The signature +/// is the part that must never appear in an event. +const TOKEN: &str = "pk.eyJ1IjoiZXhhbXBsZS11c2VyIiwiYSI6IngifQ.SIGNATURE-NOT-FOR-EVENTS"; +const ADDRESS: &str = "1600 Pennsylvania Ave"; + +fn scratch(name: &str) -> PathBuf { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("events-{name}")); + let _ = std::fs::remove_dir_all(&home); + // With `.mapbox` already there, as on any machine that has logged in: + // recording never creates it. + std::fs::create_dir_all(config_dir(&home)).expect("create the scratch config dir"); + home +} + +fn config_dir(home: &Path) -> PathBuf { + home.join(".mapbox") +} + +fn command(home: &Path) -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + cmd.env_remove("MAPBOX_ACCESS_TOKEN") + .env_remove("MapboxAccessToken") + .env_remove("MAPBOX_USERNAME") + .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_CLI_NO_TELEMETRY") + .env_remove("MAPBOX_INTERNAL_TELEMETRY_URL") + .env_remove("MAPBOX_CLI_TOKEN") + .env_remove("MAPBOX_INTERNAL_TELEMETRY_SEND") + .env_remove("MAPBOX_INTERNAL_TELEMETRY_LOG") + .env("MAPBOX_NO_UPDATE_CHECK", "1") + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", config_dir(home)); + cmd +} + +fn run(home: &Path, args: &[&str]) -> Output { + command(home).args(args).output().expect("run mapbox") +} + +/// Runs `cmd` with its event sent to a loopback server, and returns the run +/// and the event the server received. `MAPBOX_CLI_TOKEN` gives the run a +/// token to send with whether or not it has one of its own. +fn sent(cmd: &mut Command) -> (Output, Value) { + let (received, url) = events_server(true); + let out = cmd + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .output() + .expect("run mapbox"); + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + let batch: Value = serde_json::from_str(&request.body).expect("the body is JSON"); + (out, batch[0].clone()) +} + +fn run_sent(home: &Path, args: &[&str]) -> (Output, Value) { + sent(command(home).args(args)) +} + +#[test] +fn a_run_writes_one_event_with_what_it_did() { + let home = scratch("one"); + let data = r#"{"name":"secret-style-name","layers":[]}"#; + let (out, event) = run_sent( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + data, + "--dry-run", + "-t", + TOKEN, + ], + ); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + + assert_eq!(event["event"], "cli.command"); + assert_eq!(event["sdkIdentifier"], "mapbox-cli"); + assert_eq!(event["command"], serde_json::json!(["styles", "create"])); + assert_eq!(event["invocation"], "execute"); + assert_eq!(event["exitCode"], 0); + assert_eq!(event["dryRun"], true); + assert_eq!(event["auth"]["source"], "flag"); + assert_eq!(event["auth"]["type"], "pk"); + assert_eq!(event["auth"]["account"], "example-user"); + assert_eq!( + event["stdoutBytes"].as_u64(), + Some(out.stdout.len() as u64), + "stdoutBytes should be what was written" + ); + let params = event["params"].as_array().expect("params"); + assert!(params.contains(&serde_json::json!({ + "name": "data", "bytes": data.len(), "keys": ["layers", "name"] + }))); + assert!(params.contains(&serde_json::json!({ "name": "username", "length": 7 }))); +} + +#[test] +fn nothing_the_user_typed_reaches_the_event() { + let home = scratch("private"); + // No network needed: a usage error still records, and `--dry-run` sends + // nothing. + let (_, first) = run_sent( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + "{\"x\":1}", + "--dry-run", + "-t", + TOKEN, + ], + ); + let (_, second) = run_sent( + &home, + &[ + "geocoder", + "forward", + "--q", + ADDRESS, + "--no-such-flag", + "-t", + TOKEN, + ], + ); + let (_, third) = run_sent(&home, &["/Users/someone/secret/path"]); + + let written = [first, second, third].map(|e| e.to_string()).concat(); + for secret in [ + "SIGNATURE-NOT-FOR-EVENTS", + ADDRESS, + "someone", + "secret/path", + ] { + assert!( + !written.contains(secret), + "`{secret}` reached an event:\n{written}" + ); + } +} + +#[test] +fn help_version_and_usage_errors_record_their_invocation() { + let home = scratch("invocation"); + let events = [ + &["--version"][..], + &["styles", "--help"], + &["styles", "list", "--schema"], + &["nosuchcommand"], + ] + .map(|args| run_sent(&home, args).1); + let seen: Vec<(&str, &Value)> = events + .iter() + .map(|e| (e["invocation"].as_str().unwrap_or(""), &e["command"])) + .collect(); + assert_eq!( + seen, + [ + ("version", &Value::Null), + ("help", &serde_json::json!(["styles"])), + ("schema", &serde_json::json!(["styles", "list"])), + ("execute", &Value::Null), + ] + ); + assert_eq!(events[3]["usageError"], "InvalidSubcommand"); + assert_eq!(events[3]["errorCode"], "usage"); + assert_eq!(events[3]["exitCode"], 2); +} + +#[test] +fn the_opt_out_records_nothing() { + let home = scratch("opt-out-env"); + let out = command(&home) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "MAPBOX_CLI_NO_TELEMETRY=1 still wrote telemetry" + ); +} + +#[test] +fn stdout_is_identical_with_telemetry_on_and_off() { + // On with a token and somewhere to send, so the child really runs. + let (on, _) = run_sent(&scratch("stdout-on"), &["-o", "json", "config", "list"]); + let off = command(&scratch("stdout-off")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["-o", "json", "config", "list"]) + .output() + .expect("run mapbox"); + assert_eq!(on.stdout, off.stdout); + assert_eq!(on.stderr, off.stderr); +} + +#[test] +fn completion_records_nothing() { + let home = scratch("completion"); + assert!(run(&home, &["completion", "zsh"]).status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "`completion` wrote telemetry" + ); +} + +/// Recording never creates the config directory: a machine that has never +/// logged in or set a config keeps no `~/.mapbox` at all. +#[test] +fn without_a_config_directory_nothing_is_created() { + let home = scratch("no-config-dir"); + std::fs::remove_dir(config_dir(&home)).expect("remove the scratch config dir"); + assert!(run(&home, &["styles", "--help"]).status.success()); + assert!( + !config_dir(&home).exists(), + "recording created {}", + config_dir(&home).display() + ); +} + +#[test] +fn a_run_started_by_a_workflow_step_records_its_parent() { + let parent = "5f0c1e9a-7b2d-4c1e-9f3a-2d8e6b1a0c47"; + let home = scratch("parent"); + let (out, first) = sent( + command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", parent) + .args(["config", "list"]), + ); + assert!(out.status.success()); + // Anything that isn't an event id is ignored rather than recorded. + let (_, second) = sent( + command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", "/Users/someone/secret") + .args(["config", "list"]), + ); + + let events = [first, second]; + assert_eq!(events[0]["parentEventId"], parent); + assert_ne!(events[0]["eventId"], parent); + assert!(events[1].get("parentEventId").is_none(), "{:?}", events[1]); +} + +/// One request as the loopback server saw it. +struct Received { + head: String, + body: String, +} + +/// A loopback stand-in for Mapbox Events. Reports the first request it gets, +/// and answers it only when `respond` is set — otherwise it holds the +/// connection open, as an unreachable service would. +fn events_server(respond: bool) -> (mpsc::Receiver, String) { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let addr = listener.local_addr().expect("the bound address"); + let (sender, receiver) = mpsc::channel(); + std::thread::spawn(move || { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + let _ = stream.set_read_timeout(Some(Duration::from_secs(10))); + let mut head = Vec::new(); + let mut byte = [0u8; 1]; + while !head.ends_with(b"\r\n\r\n") { + match stream.read(&mut byte) { + Ok(1) => head.push(byte[0]), + _ => break, + } + } + let head = String::from_utf8_lossy(&head).into_owned(); + let length = head + .lines() + .find_map(|l| { + let (name, value) = l.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().ok())? + }) + .unwrap_or(0); + let mut body = vec![0u8; length]; + let _ = stream.read_exact(&mut body); + let body = String::from_utf8_lossy(&body).into_owned(); + let _ = sender.send(Received { head, body }); + if respond { + let _ = stream.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n"); + } else { + std::thread::sleep(Duration::from_secs(30)); + } + }); + (receiver, format!("http://{addr}/events/v2")) +} + +#[test] +fn with_a_send_url_the_event_is_posted() { + let home = scratch("send"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + // A marker `http::client` would add, which must not reach Mapbox Events. + .env("CLAUDECODE", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + let request_line = request.head.lines().next().unwrap_or_default(); + assert_eq!( + request_line, + "POST /events/v2?access_token=pk.test HTTP/1.1" + ); + let user_agent = request + .head + .lines() + .find_map(|l| l.strip_prefix("user-agent: ")) + .expect("a user agent"); + assert_eq!( + user_agent, + concat!("mapbox-cli/", env!("CARGO_PKG_VERSION")), + "the user agent should be the product token alone" + ); + + let batch: Value = serde_json::from_str(&request.body).expect("the body is JSON"); + let batch = batch.as_array().expect("the body is a batch"); + assert_eq!(batch.len(), 1); + assert_eq!(batch[0]["event"], "cli.command"); + assert_eq!(batch[0]["command"], serde_json::json!(["config", "list"])); +} + +#[test] +fn the_command_does_not_wait_for_the_send() { + let home = scratch("send-hangs"); + let (received, url) = events_server(false); + let started = Instant::now(); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + let elapsed = started.elapsed(); + assert!(out.status.success()); + // The server proves the send was under way while the command returned. + assert!(received.recv_timeout(Duration::from_secs(10)).is_ok()); + assert!( + elapsed < Duration::from_secs(3), + "the command waited {elapsed:?} on a send that never answers" + ); +} + +#[test] +fn the_opt_out_sends_nothing() { + let home = scratch("send-opt-out"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!( + received.recv_timeout(Duration::from_secs(3)).is_err(), + "MAPBOX_CLI_NO_TELEMETRY=1 still sent an event" + ); +} + +/// Every line of the delivery log under `home`, waiting up to ten seconds +/// for `count` of them: the outcome is written by a child the command did +/// not wait for. +fn deliveries(home: &Path, count: usize) -> Vec { + let dir = config_dir(home).join(".telemetry").join("deliveries"); + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let lines: Vec = std::fs::read_dir(&dir) + .into_iter() + .flatten() + .flat_map(|entry| { + let text = + std::fs::read_to_string(entry.expect("an entry").path()).unwrap_or_default(); + text.lines() + .map(|l| serde_json::from_str(l).expect("a delivery line is JSON")) + .collect::>() + }) + .collect(); + if lines.len() >= count || Instant::now() > deadline { + return lines; + } + std::thread::sleep(Duration::from_millis(50)); + } +} + +#[test] +fn the_delivery_log_pairs_each_hand_off_with_its_response() { + let home = scratch("delivery-log"); + let (_received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + + let lines = deliveries(&home, 2); + assert_eq!(lines.len(), 2, "{lines:?}"); + assert_eq!(lines[0]["stage"], "queued"); + assert_eq!(lines[1]["stage"], "responded"); + assert_eq!(lines[1]["status"], 204); + assert!(lines[0]["eventId"].is_string()); + assert_eq!(lines[0]["eventId"], lines[1]["eventId"]); + let logged = lines.iter().map(Value::to_string).collect::(); + assert!( + !logged.contains("pk.test"), + "the delivery log holds the token: {logged}" + ); +} + +#[test] +fn the_delivery_log_says_why_a_send_failed() { + let home = scratch("delivery-log-failed"); + // A port that was bound and released: nothing is listening. + let port = TcpListener::bind("127.0.0.1:0") + .and_then(|l| l.local_addr()) + .expect("a loopback port") + .port(); + let out = command(&home) + .env( + "MAPBOX_INTERNAL_TELEMETRY_URL", + format!("http://127.0.0.1:{port}/events/v2"), + ) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + + let lines = deliveries(&home, 2); + assert_eq!(lines.len(), 2, "{lines:?}"); + assert_eq!(lines[1]["stage"], "failed"); + assert!( + lines[1]["error"] + .as_str() + .is_some_and(|e| e.starts_with("could not connect")), + "{lines:?}" + ); +} + +#[test] +fn without_the_switch_there_is_no_delivery_log() { + let home = scratch("no-delivery-log"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!(received.recv_timeout(Duration::from_secs(10)).is_ok()); + // The child logs after the response; give it time to have done so. + std::thread::sleep(Duration::from_millis(500)); + assert!(!config_dir(&home) + .join(".telemetry") + .join("deliveries") + .exists()); +} + +#[test] +fn a_typed_token_reaches_the_send() { + let home = scratch("send-typed-token"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .args(["--token", "pk.typed", "config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + assert_eq!( + request.head.lines().next().unwrap_or_default(), + "POST /events/v2?access_token=pk.typed HTTP/1.1" + ); +} + +/// Telemetry may fall back to the CLI's token whatever URL it is sent to, so +/// an overridden one keeps it. +#[test] +fn the_clis_token_sends_the_event_for_someone_with_none() { + let home = scratch("send-cli-token-only"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + assert_eq!( + request.head.lines().next().unwrap_or_default(), + "POST /events/v2?access_token=pk.cli HTTP/1.1" + ); +} + +#[test] +fn use_login_keeps_the_environment_token_out_of_the_send() { + let home = scratch("send-use-login"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.env") + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["--use-login", "config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!( + sent_with(&received), + "pk.cli", + "--use-login still sent MAPBOX_ACCESS_TOKEN" + ); +} + +/// The `access_token` the loopback server's one request carried. +fn sent_with(received: &mpsc::Receiver) -> String { + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + let line = request.head.lines().next().unwrap_or_default().to_string(); + line.split("access_token=") + .nth(1) + .and_then(|rest| rest.split([' ', '&']).next()) + .unwrap_or_default() + .to_string() +} + +#[test] +fn the_users_token_comes_before_the_clis() { + let home = scratch("send-user-first"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["--token", "pk.typed", "config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(sent_with(&received), "pk.typed"); +} + +/// `--version`, help and usage errors return before the arguments are parsed, +/// and still send with the environment's token rather than the CLI's. +#[test] +fn a_run_that_never_parses_still_sends_with_the_environment_token() { + for args in [ + &["--version"][..], + &["styles", "--help"], + &["nosuchcommand"], + ] { + let home = scratch("send-unparsed"); + let (received, url) = events_server(true); + let _ = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.env") + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(args) + .output() + .expect("run mapbox"); + assert_eq!(sent_with(&received), "pk.env", "{args:?}"); + } +} + +/// The child's login step reads without the lock while the login is good, so +/// a read-only command's telemetry neither creates a lock file nor tightens +/// the directory's mode. +#[cfg(unix)] +#[test] +fn sending_with_a_login_leaves_the_config_directory_alone() { + use std::os::unix::fs::PermissionsExt; + + let home = scratch("send-login-readonly"); + let dir = config_dir(&home); + // No `exp`, so it never needs a refresh. + std::fs::write( + dir.join("credentials.json"), + r#"{"access_token":"pk.login"}"#, + ) + .expect("write a login"); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(sent_with(&received), "pk.login"); + + let mode = std::fs::metadata(&dir) + .expect("the config dir") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o755, "the send changed the directory's mode"); + assert!( + !dir.join("credentials.json.lock").exists(), + "the send took the credentials lock" + ); +} + +/// With a token to send with, the child runs; it still must not be what +/// creates `~/.mapbox` on a machine that has none. +#[test] +fn sending_never_creates_the_config_directory() { + let home = scratch("send-no-config-dir"); + std::fs::remove_dir(config_dir(&home)).expect("remove the scratch config dir"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["styles", "--help"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(sent_with(&received), "pk.cli"); + assert!(!config_dir(&home).exists(), "the send created ~/.mapbox"); +} + +#[test] +fn without_a_token_the_event_is_dropped() { + let home = scratch("no-token"); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + let lines = deliveries(&home, 1); + assert_eq!(lines.len(), 1, "{lines:?}"); + assert_eq!(lines[0]["stage"], "no_token"); + let kept: Vec = std::fs::read_dir(config_dir(&home).join(".telemetry")) + .expect("the telemetry directory") + .map(|e| e.expect("an entry").path()) + .filter(|p| p.extension().is_some_and(|ext| ext == "jsonl")) + .collect(); + assert!(kept.is_empty(), "the event was kept on disk: {kept:?}"); +} diff --git a/tests/tilesets_cli_proxy.rs b/tests/tilesets_cli_proxy.rs index 19bc52e..16a4a66 100644 --- a/tests/tilesets_cli_proxy.rs +++ b/tests/tilesets_cli_proxy.rs @@ -58,6 +58,8 @@ fn sandbox_home() -> PathBuf { /// it, and `HOME` is redirected so a stored login cannot either. fn run(stub: &Path, args: &[&str]) -> Output { Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(args) .env("MAPBOX_TILESETS_CLI", stub) .env_remove("MAPBOX_ACCESS_TOKEN") @@ -271,6 +273,8 @@ fn the_tilesets_name_does_not_reach_the_proxy() { fn an_environment_token_is_passed_through_untouched() { let stub = stub_for("env-token"); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["tilesets-cli", "list", "someone"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_ACCESS_TOKEN", "pk.from-the-environment") @@ -290,6 +294,8 @@ fn use_login_prefers_the_stored_token_over_the_environment() { let home = home_with_login("use-login", Some("stored-account")); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["--use-login", "tilesets-cli", "list", "stored-account"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_ACCESS_TOKEN", "pk.from-the-environment") @@ -314,6 +320,8 @@ fn without_use_login_the_environment_still_wins() { let env_token = fake_token("pk", "someone-else"); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["tilesets-cli", "list", "stored-account"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_ACCESS_TOKEN", &env_token) @@ -341,6 +349,8 @@ fn use_login_without_a_login_is_an_error() { let home = home_with_login("use-login-empty", None); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["--use-login", "tilesets-cli", "list", "someone"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_ACCESS_TOKEN", "pk.from-the-environment") @@ -372,6 +382,8 @@ fn the_config_dir_override_is_where_a_login_is_read_from() { write_login(&store, "stored-account"); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["--use-login", "tilesets-cli", "list", "stored-account"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_ACCESS_TOKEN", "pk.from-the-environment") @@ -401,6 +413,8 @@ fn an_environment_token_for_another_account_is_called_out() { let home = home_with_login("shadowed-login", Some("me")); let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["tilesets-cli", "list", "me"]) .env("MAPBOX_TILESETS_CLI", &stub) // Parseable, unlike the `pk.from-the-environment` the precedence @@ -423,6 +437,8 @@ fn an_environment_token_for_another_account_is_called_out() { #[test] fn a_missing_binary_prints_install_guidance() { let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args([TOKEN[0], TOKEN[1], "tilesets-cli", "list", "someone"]) .env_remove("MAPBOX_TILESETS_CLI") .env("PATH", "") @@ -438,6 +454,8 @@ fn a_missing_binary_prints_install_guidance() { #[test] fn a_broken_override_blames_the_env_var() { let output = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args([TOKEN[0], TOKEN[1], "tilesets-cli", "list", "someone"]) .env("MAPBOX_TILESETS_CLI", "/nonexistent/tilesets") .output() @@ -480,6 +498,8 @@ fn yes_is_reported_as_not_reaching_the_child() { fn the_environment_variable_is_not_warned_about() { let stub = stub_for("yes-env-quiet"); let out = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["tilesets-cli", "list", "someone"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_YES", "1") @@ -525,6 +545,8 @@ fn a_forwarded_token_is_not_printed_by_debug() { let stub = stub_for("debug-redaction"); let out = Command::new(env!("CARGO_BIN_EXE_mapbox")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .env_remove("MAPBOX_CLI_TOKEN") .args(["tilesets-cli", "--token", FAKE_TOKEN, "list", "someone"]) .env("MAPBOX_TILESETS_CLI", &stub) .env("MAPBOX_DEBUG", "1") From df71cd19cbbfd9096a066934a271b8593f840d14 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Fri, 9 Oct 2026 18:35:59 +0300 Subject: [PATCH 3/4] Say in README exactly what the telemetry event carries The Privacy section still said only the service is collected and never credentials. It now lists the command and the shape of its options, the token kind and account, and that Mapbox Events keeps the token an event is sent with. The API commands section lists the event's fields. --- README.md | 50 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 0a66cfa..ac4fd3a 100644 --- a/README.md +++ b/README.md @@ -281,20 +281,30 @@ deeper where that reads better, as in `mapbox styles draft get`. [docs/commands.md](./docs/commands.md) lists every command with its parameters and sample output. -Every request sends `User-Agent: mapbox-cli/` and nothing else -about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` keeps even future -markers out of that header. - -Each run also sends one event to Mapbox: the command's name, its options -(a value only when it comes from a fixed list, otherwise just its length or -size, and for a JSON body only the field names the API defines), how it -ended, and how long it took — never a token, a file path or free text you -typed. It carries a random ID that is replaced on the first run a day after -it was created. It is sent in the background, so a command never waits for -it, and dropped if it can't be delivered. It is sent with your own token -(`--token`, `MAPBOX_ACCESS_TOKEN` or your login) when you have one, and with -a token built into the CLI otherwise. A build from source has no built-in -token, so with no token of your own the event is dropped. +Every API request a command makes sends `User-Agent: mapbox-cli/` +and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` +keeps even future markers out of that header. + +Each run also sends one event to Mapbox: + +- the command's name and its options: a value only for a flag, a choice from + a fixed list, a number, or a language, country or feature-type code; + otherwise just its length or size. A coordinate or tile address is sent + by name only, and a JSON body only by the field names the API defines; +- how the run ended, how long it took, and how many requests it made; +- your OS and architecture, how the CLI was installed, the previous version + after an upgrade, and whether it ran in CI or under a detected AI coding + agent; +- which kind of token the command used and the Mapbox account it belongs to. + +It never includes a file path or free text you typed. It carries a random ID +that is replaced on the first run a day after it was created. It is sent in +the background, so a command never waits for it, and dropped if it can't be +delivered. It is sent with your own token (`--token`, `MAPBOX_ACCESS_TOKEN` +or your login) when you have one, and with a token built into the CLI +otherwise; Mapbox Events keeps the token an event was sent with, and the +account it belongs to, alongside the event. A build from source has no +built-in token, so with no token of your own the event is dropped. `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. ### Diagnostics and settings @@ -618,8 +628,13 @@ Mapbox collects telemetry data from our CLIs to better understand how our tools are used and how to improve our products. - **What Telemetry Data We Collect:** Usage metrics include installs, the - service a Mapbox API command belongs to (e.g. `styles` or `geocoder`, - never the operation or its arguments), CLI version, OS/architecture, + service a Mapbox API command belongs to (e.g. `styles` or `geocoder`), the + command that ran and the shape of its options as described under + [API commands](#api-commands) (values only for flags, fixed choices, + numbers and language, country or feature-type codes; never coordinates, + file paths or free text), how it ended and how long it took, the kind of + token it used and the Mapbox account that token belongs to, the Mapbox + access token the event itself was sent with, CLI version, OS/architecture, whether stdin and stdout are attached to a terminal, an identifier for the detected AI coding agent (if any) running the command (based on signals such as the presence of the `CLAUDECODE` or `COPILOT_MODEL` @@ -632,7 +647,8 @@ tools are used and how to improve our products. adoption, prioritize investments, and improve the reliability, performance, and developer experience of our CLIs. - **What We Do Not Collect:** Code completion outputs, source code, project - file names, directory contents, non-Mapbox API keys, or credentials. + file names, directory contents, non-Mapbox API keys, or credentials other + than the Mapbox access token an event is sent with. - **Who has Access:** Telemetry data will not be disclosed to, or accessed by, third parties other than Mapbox affiliates and passive cloud storage and hosting providers necessary to maintain our infrastructure. From b733f7ae0bd83197e3f4004d13d51a74b03e6bb4 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Fri, 9 Oct 2026 19:02:40 +0300 Subject: [PATCH 4/4] Tighten what the telemetry event sends and which token it uses - Send a language or country value only when it is exactly a code, so a short word can't pass as one. - Read `--data @` again only when it is a regular file, so a FIFO can't hang the exit. - Count `MapboxAccessToken` as the user's token, and honor `--use-login` on runs that never parse. - Skip the event for `uninstall` on Windows, where the sender would keep `mapbox.exe` locked. - Make `auth logout` wait for a refresh in flight so it can't write the login back. - Keep `cargo test` from posting to production Events, and test the sudo skip, `--q`'s privacy and new numeric location parameters. - List every field the event carries in README's Privacy section. --- AGENTS.md | 5 ++- CONTRIBUTING.md | 3 +- README.md | 11 +++-- src/auth.rs | 3 ++ src/cli_token.rs | 21 +++++---- src/telemetry_event.rs | 94 +++++++++++++++++++++++++++++++++++---- src/telemetry_sink.rs | 6 ++- tests/doctor.rs | 3 ++ tests/output_contract.rs | 6 +-- tests/source_guards.rs | 12 ++--- tests/telemetry_events.rs | 86 ++++++++++++++++++++++++++++++----- tests/update_check.rs | 5 +++ 12 files changed, 211 insertions(+), 44 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8941278..3b4a38a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -149,8 +149,9 @@ same for the completion scripts. responses are quoted rather than written, and the `cancelled` error code, which is a compatibility promise rather than a spelling. - **Four rules the compiler holds rather than a reviewer**, declared in - `Cargo.toml` with the reasoning beside each: no `unsafe`, no `println!`, no - `dbg!`, no `todo!`/`unimplemented!`. `print_stderr` is deliberately *not* + `Cargo.toml` with the reasoning beside each: no `unsafe` (one `#[allow]`, + in `src/detach.rs`, says why), no `println!`, no `dbg!`, no + `todo!`/`unimplemented!`. `print_stderr` is deliberately *not* denied — progress belongs there. - **The toolchain is pinned, and the floor is a different number.** `rust-toolchain.toml` names the exact Rust every clone and every workflow diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 547b95e..ebb0b81 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,8 @@ installers end to end without touching the network, and `scripts/test-completion.sh` does the same for the completion scripts. Four rules the compiler holds rather than a reviewer, declared in -`Cargo.toml` with the reasoning beside each: no `unsafe`, no `println!` +`Cargo.toml` with the reasoning beside each: no `unsafe` (outside one +`#[allow]` in `src/detach.rs`), no `println!` (stdout belongs to `output::emit`, the single place `--output` is honored), no `dbg!`, no `todo!`/`unimplemented!`. diff --git a/README.md b/README.md index ac4fd3a..29734ae 100644 --- a/README.md +++ b/README.md @@ -632,9 +632,14 @@ tools are used and how to improve our products. command that ran and the shape of its options as described under [API commands](#api-commands) (values only for flags, fixed choices, numbers and language, country or feature-type codes; never coordinates, - file paths or free text), how it ended and how long it took, the kind of - token it used and the Mapbox account that token belongs to, the Mapbox - access token the event itself was sent with, CLI version, OS/architecture, + file paths or free text), the global options it used (such as `--output`, + `--dry-run` or whether a named profile was used), how it ended and how + long it took, how many requests it made with their HTTP status and sizes, + the kind of token it used and the Mapbox account that token belongs to, + the Mapbox access token the event itself was sent with, a random + identifier that is replaced every day, which run started this one when it + ran inside another, CLI version, how the CLI was installed, the version + it was updated from and any newer one it offered, OS/architecture, whether stdin and stdout are attached to a terminal, an identifier for the detected AI coding agent (if any) running the command (based on signals such as the presence of the `CLAUDECODE` or `COPILOT_MODEL` diff --git a/src/auth.rs b/src/auth.rs index e4a6473..f497e2f 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1068,6 +1068,9 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> { let path = credentials_path(profile)?; let had_credentials = path.exists(); if had_credentials { + // Waits out a refresh in flight, such as the telemetry sender's from + // the previous run, which would otherwise write the login back. + let _lock = CredentialLock::acquire(profile).ok(); crate::run_record::set_auth_step("remove_credentials"); std::fs::remove_file(&path)?; } diff --git a/src/cli_token.rs b/src/cli_token.rs index c98257c..f4648ac 100644 --- a/src/cli_token.rs +++ b/src/cli_token.rs @@ -7,7 +7,8 @@ //! //! 1. `--token` or `MAPBOX_ACCESS_TOKEN`, ranked the way a command ranks them //! (see [`user_token`]). -//! 2. The stored login, loaded the way a command loads it, refresh included. +//! 2. The stored login, read as it is on disk and refreshed, under the +//! credentials lock, only when it is about to expire. //! 3. `MAPBOX_CLI_TOKEN` at run time, the CLI's token for someone with none //! of their own, such as a developer pointing at staging. //! 4. The token compiled in from `MAPBOX_CLI_BUNDLED_TOKEN` at build time. A @@ -16,8 +17,8 @@ //! every local build. //! //! A `401` moves on to the next token; any other answer is the caller's. -//! Callers never see the token, so every one of them gets the same fallback -//! without doing anything. +//! [`send`]'s callers never see the token, so every one of them gets the same +//! fallback without doing anything. //! //! # What may use the CLI's token //! @@ -47,7 +48,7 @@ use crate::{auth, http}; pub(crate) enum Fallback { /// `--token` or `MAPBOX_ACCESS_TOKEN`, as [`user_token`] ranks them. UserToken, - /// The stored login, loaded as a command loads it, refresh included. + /// The stored login, refreshed only when it is about to expire. Login, /// `MAPBOX_CLI_TOKEN` at run time, then the bundled token. Billed to the /// CLI's account, so it only ever comes after every token of the user's. @@ -92,14 +93,18 @@ fn cli_token(override_token: Option<&str>) -> Option { } /// The token the user gave this run, ranked as a command ranks it: with -/// `--use-login` only a typed `--token` counts, otherwise `MAPBOX_ACCESS_TOKEN` -/// does too. Resolved by the caller, which has the arguments, and handed to -/// [`send`]. +/// `--use-login` only a typed `--token` counts, otherwise the environment +/// does too, `MapboxAccessToken` included, since that is the user's token +/// for `tilesets-cli`. Resolved by the caller, which has the arguments, and +/// handed to [`send`]. pub(crate) fn user_token(matches: &clap::ArgMatches) -> Option { if matches.get_flag("use-login") { auth::typed_token(matches) } else { - matches.get_one::("token").cloned() + matches + .get_one::("token") + .cloned() + .or_else(|| auth::environment_token().map(|(_, token)| token)) } } diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs index 6a67e92..a988616 100644 --- a/src/telemetry_event.rs +++ b/src/telemetry_event.rs @@ -331,10 +331,19 @@ fn with_workflow(f: impl FnOnce(&mut Option)) { /// its state files as root, and every later run as the user could read none /// of them — a fresh `userId` each time. Skipped for the same reason /// `run_history` skips it (aws/aws-cli#10031). +/// +/// `uninstall` on Windows is skipped too: its helper deletes `mapbox.exe` a +/// second after exit, and a sender still running from that image would keep +/// the file locked and the delete would fail. pub(crate) fn deliver(record: &Record) { if !telemetry::telemetry_allowed() || std::env::var_os("SUDO_USER").is_some() { return; } + if cfg!(windows) + && record.command.first().map(String::as_str) == Some(crate::uninstall::COMMAND) + { + return; + } let event = build(record); if let Ok(line) = serde_json::to_string(&event) { let profile = record.options.as_ref().and_then(|o| o.profile.as_deref()); @@ -631,16 +640,21 @@ fn classify( param } -/// Whether every item of an [`ALLOWLISTED`] value is a code: a language or -/// country code (`en`, `zh-Hans`, `pt-BR`, `us`) or a known feature type. +/// Whether every item of an [`ALLOWLISTED`] value is a code: a country code +/// (`us`), a language code with an optional script or region (`en`, +/// `zh-Hans`, `pt-BR`, `es-419`), or a known feature type. The shapes are +/// tight on purpose: anything looser lets a short word through. fn is_code(name: &str, values: &[String]) -> bool { - fn language_or_country(item: &str) -> bool { + fn letters(part: &str, len: usize) -> bool { + part.len() == len && part.chars().all(|c| c.is_ascii_alphabetic()) + } + fn language(item: &str) -> bool { let mut parts = item.splitn(2, ['-', '_']); - let head = parts.next().unwrap_or_default(); - (2..=3).contains(&head.len()) - && head.chars().all(|c| c.is_ascii_alphabetic()) + letters(parts.next().unwrap_or_default(), 2) && parts.next().is_none_or(|tail| { - (2..=8).contains(&tail.len()) && tail.chars().all(|c| c.is_ascii_alphanumeric()) + letters(tail, 4) + || letters(tail, 2) + || (tail.len() == 3 && tail.chars().all(|c| c.is_ascii_digit())) }) } if !ALLOWLISTED.contains(&name) { @@ -648,7 +662,8 @@ fn is_code(name: &str, values: &[String]) -> bool { } let items = || values.iter().flat_map(|v| v.split(',')).map(str::trim); match name { - "language" | "country" => items().all(language_or_country), + "country" => items().all(|item| letters(item, 2)), + "language" => items().all(language), "types" => items().all(|item| FEATURE_TYPES.contains(&item)), _ => false, } @@ -667,6 +682,11 @@ fn data_shape(value: &str, declared: &[String]) -> (Option, Option MAX_DATA_TO_PARSE { @@ -938,6 +958,31 @@ mod tests { assert_eq!(data_shape("@-", &declared), (None, None)); } + /// A FIFO was already drained by the command; opening it again at exit + /// would block until something else wrote to it. + #[cfg(unix)] + #[test] + fn a_data_path_that_is_not_a_regular_file_is_not_read_again() { + let dir = std::env::temp_dir().join(format!("mapbox-fifo-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("scratch dir"); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock") + .as_nanos(); + let fifo = dir.join(format!("body-{nanos}")); + let made = std::process::Command::new("mkfifo") + .arg(&fifo) + .status() + .expect("run mkfifo"); + assert!(made.success()); + + let (done, finished) = std::sync::mpsc::channel(); + let arg = format!("@{}", fifo.display()); + std::thread::spawn(move || done.send(data_shape(&arg, &[]))); + let shape = finished.recv_timeout(std::time::Duration::from_secs(5)); + assert_eq!(shape.expect("data_shape returned"), (None, None)); + } + #[test] fn the_user_id_is_replaced_once_it_is_a_day_old() { let id = uuid_v4([0x11; 16]); @@ -994,7 +1039,7 @@ mod tests { fn allowlisted_values_go_out_only_when_they_are_codes() { for (name, value) in [ ("language", "en"), - ("language", "zh-Hans,pt-BR"), + ("language", "zh-Hans,pt-BR,es-419"), ("country", "us,cn"), ("types", "address,poi"), ] { @@ -1003,7 +1048,11 @@ mod tests { } for (name, value) in [ ("country", "my secret project name"), + ("country", "us,my-secret"), + ("country", "joe"), ("language", "acme-corporation-internal"), + ("language", "ab-project1"), + ("language", "joe"), ("types", "notes about client acme"), ("types", "address,acme"), ] { @@ -1016,6 +1065,33 @@ mod tests { } } + /// A number's value is sent unless its name is in [`COORDINATES`], so a + /// location parameter a spec adds later, such as `proximity-lng`, would + /// leak. This fails until it is listed. + #[test] + fn every_numeric_location_parameter_is_a_coordinate() { + let specs = crate::spec::effective_services().expect("the bundled specs"); + let mut unlisted: Vec = specs + .iter() + .flat_map(|spec| &spec.operations) + .flat_map(|op| op.path_params.iter().chain(&op.query_params)) + .filter(|param| param.numeric.is_some()) + .map(|param| param.arg_name.clone()) + .filter(|name| { + ["lon", "lat", "lng", "coord"] + .iter() + .any(|part| name.contains(part)) + && !COORDINATES.contains(&name.as_str()) + }) + .collect(); + unlisted.sort(); + unlisted.dedup(); + assert!( + unlisted.is_empty(), + "numeric location parameters not in COORDINATES: {unlisted:?}" + ); + } + #[test] fn an_unknown_tilesets_word_is_other() { assert_eq!(tilesets_word("upload-source"), "upload-source"); diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs index ec48954..c300a72 100644 --- a/src/telemetry_sink.rs +++ b/src/telemetry_sink.rs @@ -163,10 +163,12 @@ pub(crate) fn remember_user_token(token: Option) { pub(crate) fn deliver(line: &str, profile: Option<&str>) { let url = send_url(is_production_build(), from_environment(URL_ENV)); // Help, `--version` and usage errors return before the arguments are - // parsed, so nothing was remembered; the environment is still the user's. + // parsed, so nothing was remembered; the environment is still the user's, + // unless `--use-login` was typed to set it aside. let user_token = match USER_TOKEN.get() { Some(remembered) => remembered.clone(), - None => from_environment(auth::CLAP_TOKEN_ENV), + None if std::env::args_os().any(|arg| arg == "--use-login") => None, + None => auth::environment_token().map(|(_, token)| token), }; if cli_token::has_token_for(cli_token::TELEMETRY, user_token.as_deref(), profile) { HttpSink { diff --git a/tests/doctor.rs b/tests/doctor.rs index e1b3c29..9e4d08e 100644 --- a/tests/doctor.rs +++ b/tests/doctor.rs @@ -42,6 +42,9 @@ fn command(home: &Path) -> Command { .env_remove("NO_PROXY") .env_remove("no_proxy") .env_remove("MAPBOX_CLI_NO_TELEMETRY") + // Telemetry stays on, as it is for a user, but goes nowhere. + .env_remove("MAPBOX_CLI_TOKEN") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", "http://127.0.0.1:9/") .env_remove("MAPBOX_NO_UPDATE_CHECK") .env_remove("MAPBOX_TIMEOUT") .env("HOME", home) diff --git a/tests/output_contract.rs b/tests/output_contract.rs index bca6ffe..475553c 100644 --- a/tests/output_contract.rs +++ b/tests/output_contract.rs @@ -467,10 +467,10 @@ fn an_agent_is_pointed_at_the_schema() { fn without_an_agent_the_help_has_no_hint() { let home = sandbox_home(); let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); - // Telemetry is not under test here, and would post to Mapbox. - cmd.env("MAPBOX_CLI_NO_TELEMETRY", "1") - .env_remove("MAPBOX_CLI_TOKEN"); + // Telemetry is not under test here, and would post to Mapbox. Set after + // `env_clear`, which would otherwise clear it too. cmd.env_clear() + .env("MAPBOX_CLI_NO_TELEMETRY", "1") .env("HOME", &home) .env("MAPBOX_CONFIG_DIR", home.join(".mapbox")); // Windows processes expect this one; nothing reads it as an agent. diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 24b9d5d..04a4db6 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -523,9 +523,10 @@ fn prose_is_american_english() { /// A run with a token posts an event to Mapbox from a detached child, so a /// test that spawns `mapbox` with a fake `--token` — or on a machine with /// `MAPBOX_CLI_TOKEN` exported — would send real requests, and could fall back -/// to the developer's own login. Naming `MAPBOX_CLI_NO_TELEMETRY` is the -/// decision this asks for: set it, or test telemetry on purpose as -/// `telemetry_events.rs`, `doctor.rs` and `update_check.rs` do. +/// to the developer's own login. Setting `MAPBOX_CLI_NO_TELEMETRY=1`, or +/// pointing `MAPBOX_INTERNAL_TELEMETRY_URL` at loopback as +/// `telemetry_events.rs`, `doctor.rs` and `update_check.rs` do, is the +/// decision this asks for. Removing the variable alone is not. #[test] fn every_test_that_runs_the_binary_decides_about_telemetry() { let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests"); @@ -533,8 +534,9 @@ fn every_test_that_runs_the_binary_decides_about_telemetry() { .into_iter() .filter_map(|(name, path)| { let text = std::fs::read_to_string(&path).expect("read a test"); - (text.contains("CARGO_BIN_EXE_mapbox") && !text.contains("MAPBOX_CLI_NO_TELEMETRY")) - .then_some(name) + let decided = text.contains(r#".env("MAPBOX_CLI_NO_TELEMETRY", "1")"#) + || text.contains(r#".env("MAPBOX_INTERNAL_TELEMETRY_URL""#); + (text.contains("CARGO_BIN_EXE_mapbox") && !decided).then_some(name) }) .collect(); undecided.sort(); diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index 78fbec6..d270f0b 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -48,6 +48,7 @@ fn command(home: &Path) -> Command { .env_remove("MAPBOX_CLI_TOKEN") .env_remove("MAPBOX_INTERNAL_TELEMETRY_SEND") .env_remove("MAPBOX_INTERNAL_TELEMETRY_LOG") + .env_remove("SUDO_USER") .env("MAPBOX_NO_UPDATE_CHECK", "1") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) @@ -144,17 +145,21 @@ fn nothing_the_user_typed_reaches_the_event() { TOKEN, ], ); - let (_, second) = run_sent( - &home, - &[ - "geocoder", - "forward", - "--q", - ADDRESS, - "--no-such-flag", - "-t", - TOKEN, - ], + // A request that parses and then fails offline, through a proxy that + // isn't there, so `--q` is classified rather than lost to a usage error. + let (_, second) = sent( + command(&home) + .env("HTTPS_PROXY", "http://127.0.0.1:9") + .env_remove("ALL_PROXY") + .env_remove("NO_PROXY") + .args(["geocoder", "forward", "--q", ADDRESS, "-t", TOKEN]), + ); + assert!( + second["params"] + .as_array() + .expect("params") + .contains(&serde_json::json!({ "name": "q", "length": ADDRESS.chars().count() })), + "{second}" ); let (_, third) = run_sent(&home, &["/Users/someone/secret/path"]); @@ -402,6 +407,27 @@ fn the_opt_out_sends_nothing() { ); } +/// `sudo` keeps `HOME`, so a root run would leave root-owned files in the +/// user's `~/.mapbox`. +#[test] +fn a_run_under_sudo_sends_nothing() { + let home = scratch("send-sudo"); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("SUDO_USER", "someone") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!( + received.recv_timeout(Duration::from_secs(3)).is_err(), + "a run under sudo still sent an event" + ); + assert!(!config_dir(&home).join(".telemetry").exists()); +} + /// Every line of the delivery log under `home`, waiting up to ten seconds /// for `count` of them: the outcome is written by a child the command did /// not wait for. @@ -614,6 +640,44 @@ fn a_run_that_never_parses_still_sends_with_the_environment_token() { } } +#[test] +fn a_run_that_never_parses_still_honors_use_login() { + for args in [ + &["--use-login", "--version"][..], + &["--use-login", "styles", "--help"], + &["--use-login", "nosuchcommand"], + ] { + let home = scratch("send-unparsed-use-login"); + let (received, url) = events_server(true); + let _ = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "pk.env") + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(args) + .output() + .expect("run mapbox"); + assert_eq!(sent_with(&received), "pk.cli", "{args:?}"); + } +} + +/// `MapboxAccessToken` is the user's token for `tilesets-cli`, so it comes +/// before the CLI's even though clap doesn't read it. +#[test] +fn the_legacy_token_variable_is_the_users_too() { + for args in [&["config", "list"][..], &["--version"]] { + let home = scratch("send-legacy-env"); + let (received, url) = events_server(true); + let _ = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MapboxAccessToken", "pk.legacy") + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(args) + .output() + .expect("run mapbox"); + assert_eq!(sent_with(&received), "pk.legacy", "{args:?}"); + } +} + /// The child's login step reads without the lock while the login is good, so /// a read-only command's telemetry neither creates a lock file nor tightens /// the directory's mode. diff --git a/tests/update_check.rs b/tests/update_check.rs index 1f49b6c..dc48cd7 100644 --- a/tests/update_check.rs +++ b/tests/update_check.rs @@ -82,6 +82,9 @@ fn command(home: &Path) -> Command { .env_remove("MAPBOX_OUTPUT") .env_remove("MAPBOX_NO_UPDATE_CHECK") .env_remove("MAPBOX_CLI_NO_TELEMETRY") + // Telemetry stays on, as it is for a user, but goes nowhere. + .env_remove("MAPBOX_CLI_TOKEN") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", "http://127.0.0.1:9/") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", config_dir(home)); @@ -443,6 +446,8 @@ fn under_a_pty( .env_remove("MAPBOX_OUTPUT") .env_remove("MAPBOX_NO_UPDATE_CHECK") .env_remove("MAPBOX_CLI_NO_TELEMETRY") + .env_remove("MAPBOX_CLI_TOKEN") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", "http://127.0.0.1:9/") .env("HOME", home) .env("MAPBOX_CONFIG_DIR", config_dir(home)) .stdin(std::process::Stdio::null())
Terminal — refusesRedirected — raw bytes