diff --git a/CHANGELOG.md b/CHANGELOG.md index 273b384..121ec34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,13 @@ that may never merge. They are not releases and are not listed here. 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. +- `mapbox doctor` — a read-only snapshot of what the next command would + see: which token wins and its state, which proxy variables are in effect, + and where the update-check and telemetry switches currently stand. + `--verify` additionally checks that `api.mapbox.com` is reachable, the + only part of this that makes a request — the same precedent + `auth whoami --verify` sets. + - A native `aarch64-pc-windows-msvc` build. Windows on Arm ran the x64 build under emulation before this — including inside a VM on Apple Silicon, the larger of the two populations this serves — which `install.ps1` already diff --git a/docs/commands.md b/docs/commands.md index 972f0c5..449ae91 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -64,6 +64,8 @@ nests, and is typed `mapbox styles draft get`. [config.set](#mapbox-config-set) · [config.list](#mapbox-config-list) · [config.unset](#mapbox-config-unset) +**[Doctor](#doctor)** — [doctor](#mapbox-doctor) + **[Usage](#usage)** — [usage](#mapbox-usage) **[Accounts](#accounts)** — @@ -3280,6 +3282,89 @@ update-check cleared, now on (default). --- +## Doctor + +### `mapbox doctor` + +A read-only snapshot of what the next command would see: which token wins +and its state, which proxy variables are in effect, and where the +update-check and telemetry switches currently stand. `auth whoami` answers +which token the next command will use; this answers the rest of what +commonly goes wrong before a real command finds out the hard way — a proxy +variable that silently isn't doing what someone thinks, or a switch +resolving to something other than what was intended. + +Nothing here is sent unless `--verify` asks for the one check that needs a +request — the same precedent `auth whoami --verify` sets. + +`Proxy:` names which of `HTTPS_PROXY`/`HTTP_PROXY`/`ALL_PROXY`/`NO_PROXY` are +set — upper- or lowercase, exactly those two spellings each, which is what +`reqwest` itself reads. It answers "is a proxy variable set", not "would +this request actually use one" — `NO_PROXY` can exempt `api.mapbox.com` +specifically, a scheme-specific variable only ever applied to that scheme +in the first place, and on macOS a proxy configured only through system +Network settings (rather than an environment variable) is invisible here +even though `reqwest` would still use it. + +#### Parameters + +| Parameter | Effect | +| --- | --- | +| `--verify` | Also check that `api.mapbox.com` is reachable, through the same client and proxy handling every other request uses. Honors `--timeout`/`MAPBOX_TIMEOUT` like any other request; defaults to 5 seconds when neither is given, since this is a diagnostic someone is waiting on, not a request whose payload bounds a longer budget. | + +#### Examples + +```sh +mapbox doctor + +mapbox doctor --verify +``` + +#### Outputs + + + + +
textjson
+ +``` +mapbox 0.3.0 (production) +Token: available, from login (sk) +Proxy: none set +Update check: on +Telemetry: on +``` + + + +```json +{ + "build": { "version": "0.3.0", "channel": "production" }, + "proxy": { "active": [] }, + "switches": { + "telemetry_allowed": true, + "update_check_env_opt_out": false, + "update_check_persisted": true + }, + "token": { + "available": true, + "source": "login", + "account": "user", + "usage": "sk", + "expires_at": 1788276540 + } +} +``` + +
+ +With `--verify`, a `connectivity` object joins the JSON and a `Reachable:` +line joins the text — `{ "reachable": true, "status": 200 }`, or `{ +"reachable": false }` (plus an `error` field under `--debug`) when the +request itself failed rather than answered. + +--- + ## Usage ### `mapbox usage` diff --git a/src/auth.rs b/src/auth.rs index bd81c80..ea003e3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -520,7 +520,7 @@ fn save_credentials(creds: &Credentials, profile: Option<&str>) -> Result<()> { .with_context(|| format!("Failed to write credentials to {}", path.display())) } -fn token_expires_at(token: &str) -> Option { +pub(crate) fn token_expires_at(token: &str) -> Option { let payload_b64 = token.split('.').nth(1)?; let decoded = URL_SAFE_NO_PAD.decode(payload_b64).ok()?; let json: serde_json::Value = serde_json::from_slice(&decoded).ok()?; @@ -932,7 +932,7 @@ impl TokenSource { /// The name a program reads. The prose form is built in /// [`Identity::source_prose`], where it can name the variable or profile /// the bare word leaves out. - fn as_str(self) -> &'static str { + pub(crate) fn as_str(self) -> &'static str { match self { TokenSource::Flag => "flag", TokenSource::Environment => "environment", @@ -973,7 +973,7 @@ pub(crate) fn resolve_source<'a>( /// Read off the prefix rather than the payload because that is where the API /// itself reports it — and because the prefix is the one part of a token that /// is safe to print. -fn token_usage(token: &str) -> Option<&str> { +pub(crate) fn token_usage(token: &str) -> Option<&str> { let (usage, rest) = token.split_once('.')?; (!rest.is_empty() && matches!(usage, "pk" | "sk" | "tk")).then_some(usage) } diff --git a/src/doctor.rs b/src/doctor.rs new file mode 100644 index 0000000..0350178 --- /dev/null +++ b/src/doctor.rs @@ -0,0 +1,396 @@ +//! `mapbox doctor` — a read-only snapshot of the environment the next +//! command would run in. +//! +//! `auth whoami` already answers which token the next command will use; this +//! answers the rest of what commonly goes wrong before a real command finds +//! out the hard way: a proxy variable that silently isn't doing what someone +//! thinks, or the update-check/telemetry switches resolving to something +//! other than what was intended. +//! +//! Nothing here is sent unless `--verify` asks for the one check that needs +//! a request — the same precedent `auth whoami --verify` already sets: a +//! diagnostic command should not itself be the request that reveals the +//! problem it exists to describe. +//! +//! One [`Report`], read by both renderers. Building the text and the JSON +//! from two independently assembled values is exactly how the update-check +//! line first shipped forgetting `telemetry_allowed` — the JSON carried the +//! fact, the text line's own condition just didn't ask it. A single struct +//! removes the seam that let the two drift. + +use std::time::Duration; + +use anyhow::Result; +use clap::{Arg, ArgAction, ArgMatches, Command}; +use serde::Serialize; + +use crate::auth; +use crate::config; +use crate::http; +use crate::output::{self, Mode}; +use crate::telemetry; +use crate::update_check; + +pub const COMMAND: &str = "doctor"; + +/// The one host worth checking reachability against: what every bundled +/// spec resolves to when it names no `servers` entry of its own — see +/// `spec::parse_spec`'s own fallback. Not a `use` of that constant, because +/// this is diagnostic and that one is a build-time default; two names for +/// the same string would be one more thing to keep in sync for a value that +/// is already effectively pinned by nearly every operation's own tests. +const API_HOST: &str = "https://api.mapbox.com"; + +/// Overrides [`API_HOST`], for the same reason `update_check.rs`'s +/// `MAPBOX_INTERNAL_UPDATE_URL` exists: this crate makes no other outbound +/// request nothing here controls, so without a seam `--verify` could only +/// ever be tested against the real network — undocumented, and read only by +/// `tests/doctor.rs`. +const HOST_OVERRIDE_ENV: &str = "MAPBOX_INTERNAL_DOCTOR_URL"; + +fn api_host() -> String { + std::env::var(HOST_OVERRIDE_ENV) + .ok() + .filter(|value| !value.trim().is_empty()) + .unwrap_or_else(|| API_HOST.to_string()) +} + +/// The fallback when neither `--timeout` nor `MAPBOX_TIMEOUT` named one — +/// short on purpose: this is a diagnostic a person is waiting on at a +/// terminal, not a request whose payload a command line bounds, so the +/// default fails fast rather than waiting out a stalled connection for as +/// long as a real command would. An explicit budget always wins, the same +/// as it does everywhere else — see [`http::requested`]. +const DEFAULT_VERIFY_TIMEOUT: Duration = Duration::from_secs(5); + +/// The proxy variables `reqwest` reads, paired with the lowercase spelling +/// it also honors — exactly these two spellings and no others, confirmed by +/// reading `hyper-util`'s own `get_first_env(&["HTTPS_PROXY", +/// "https_proxy"])` (`hyper-util/src/client/proxy/matcher.rs`) rather than +/// assumed from curl's reputation for case-insensitivity: a value under +/// `Https_Proxy` or any other mixed case would not be honored by this +/// client either, on a case-sensitive OS, so not checking for it here is +/// reporting the client's real behavior rather than a gap in this list. +/// Checked as a pair rather than as four independent names because Windows +/// environment variables are case-insensitive: +/// `HTTPS_PROXY` and `https_proxy` are the same variable there, so checking +/// both spellings as separate entries reported it twice on Windows the +/// first time this shipped, once under each spelling, for a value set only +/// once. Reporting under the uppercase name when either is set avoids that +/// without pretending Unix's two independently-settable variables don't +/// exist — it just means this asks "is *a* proxy configured for this +/// concern", not "list every spelling that happens to be set", which is the +/// question a person actually has. +/// +/// Named here, once, so a variable added or removed from `reqwest`'s +/// behavior is at least this file's problem to notice, not only +/// `tests/proxy.rs`'s. +/// +/// This still only answers "is a proxy variable set", not "would this +/// request actually use one" — `NO_PROXY` can exempt a specific host, and a +/// scheme-specific variable only applies to that scheme. On macOS there is +/// a fourth gap the same shape: `hyper-util` falls back to the system's +/// Dynamic Store proxy settings (Network settings → Proxies) when no env +/// var is set at all, and a proxy configured only that way is invisible +/// here — this reads the environment, not that store. All are real gaps +/// against the true question, kept because reqwest has no public API that +/// answers "was a proxy applied to this request" for a caller to read back; +/// closing them means asking upstream for one, not maintaining a longer list +/// here. +const PROXY_VARS: &[(&str, &str)] = &[ + ("HTTPS_PROXY", "https_proxy"), + ("HTTP_PROXY", "http_proxy"), + ("ALL_PROXY", "all_proxy"), + ("NO_PROXY", "no_proxy"), +]; + +pub fn command() -> Command { + Command::new(COMMAND) + .about("Show the environment the next command would run in") + .long_about( + "A read-only snapshot of what the next command would see: which token wins \ + and its state, which proxy variables are in effect, and where the \ + update-check and telemetry switches currently stand. `--verify` additionally \ + checks that api.mapbox.com is reachable — the one part of this that makes a \ + request.", + ) + .arg( + Arg::new("verify") + .long("verify") + .action(ArgAction::SetTrue) + .help("Also check that api.mapbox.com is reachable"), + ) +} + +#[derive(Serialize)] +struct Build { + version: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + channel: Option<&'static str>, +} + +impl Build { + fn current() -> Self { + Build { + version: env!("CARGO_PKG_VERSION"), + channel: option_env!("MAPBOX_CLI_BUILD_ENV"), + } + } + + fn line(&self) -> String { + format!( + "mapbox {} ({})", + self.version, + self.channel.unwrap_or("dev") + ) + } +} + +#[derive(Serialize)] +struct TokenReport { + available: bool, + #[serde(skip_serializing_if = "Option::is_none")] + source: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + account: Option, + #[serde(skip_serializing_if = "Option::is_none")] + usage: Option, + #[serde(skip_serializing_if = "Option::is_none")] + expires_at: Option, +} + +impl TokenReport { + /// Reuses [`auth::resolve_source`] rather than re-deriving the + /// precedence — that function is the one place the order is decided, + /// and `auth::whoami`'s own docs explain why a second opinion here + /// would be worse than none. + fn resolve(matches: &ArgMatches, use_login: bool, profile: Option<&str>) -> Self { + let flag = auth::typed_token(matches); + let environment = flag + .is_none() + .then(|| matches.get_one::("token").cloned()) + .flatten(); + let stored = auth::load_credentials(profile); + + match auth::resolve_source( + flag.as_deref(), + environment.as_deref(), + stored.as_ref().map(|c| c.access_token.as_str()), + use_login, + ) { + Some((source, token)) => TokenReport { + available: true, + source: Some(source.as_str()), + account: auth::token_account(token), + usage: auth::token_usage(token).map(str::to_string), + expires_at: auth::token_expires_at(token), + }, + None => TokenReport { + available: false, + source: None, + account: None, + usage: None, + expires_at: None, + }, + } + } + + fn line(&self) -> String { + if self.available { + format!( + "Token: available, from {} ({})", + self.source.unwrap_or("unknown"), + self.usage.as_deref().unwrap_or("unrecognized prefix"), + ) + } else { + "Token: none available — run `mapbox auth login` or set MAPBOX_ACCESS_TOKEN" + .to_string() + } + } +} + +#[derive(Serialize)] +struct ProxyReport { + active: Vec<&'static str>, +} + +impl ProxyReport { + fn current() -> Self { + let set = |name: &str| std::env::var_os(name).is_some_and(|v| !v.is_empty()); + let active = PROXY_VARS + .iter() + .filter(|(upper, lower)| set(upper) || set(lower)) + .map(|(upper, _)| *upper) + .collect(); + ProxyReport { active } + } + + fn line(&self) -> String { + if self.active.is_empty() { + "Proxy: none set".to_string() + } else { + format!("Proxy: {}", self.active.join(", ")) + } + } +} + +#[derive(Serialize)] +struct SwitchesReport { + telemetry_allowed: bool, + update_check_env_opt_out: bool, + update_check_persisted: bool, +} + +impl SwitchesReport { + fn current() -> Self { + let env_opted_out = + std::env::var(update_check::NO_UPDATE_CHECK_ENV).is_ok_and(|v| !v.trim().is_empty()); + SwitchesReport { + telemetry_allowed: telemetry::telemetry_allowed(), + update_check_env_opt_out: env_opted_out, + update_check_persisted: config::update_check_enabled(), + } + } + + /// Whether the update check would actually run right now, on every + /// switch a person can adjust — not the full gate `update_check::enabled` + /// applies, which also asks about the build channel and whether stderr + /// is a terminal. Those two are facts about this run, reported in + /// `build`/the shell rather than here. `MAPBOX_CLI_NO_TELEMETRY` silences + /// the update check too (see `update_check.rs`'s module docs), which the + /// first version of this line forgot: it read `update_check_persisted` + /// and `update_check_env_opt_out` alone, so `MAPBOX_CLI_NO_TELEMETRY=1` + /// printed "Update check: on" for a check that would not run. + fn update_check_on(&self) -> bool { + self.update_check_persisted && !self.update_check_env_opt_out && self.telemetry_allowed + } + + fn update_check_line(&self) -> String { + let reason = if self.update_check_env_opt_out { + format!(" ({} is set)", update_check::NO_UPDATE_CHECK_ENV) + } else if !self.telemetry_allowed { + " (MAPBOX_CLI_NO_TELEMETRY silences this too)".to_string() + } else if !self.update_check_persisted { + " (mapbox config set update-check off)".to_string() + } else { + String::new() + }; + format!( + "Update check: {}{reason}", + if self.update_check_on() { "on" } else { "off" } + ) + } + + fn telemetry_line(&self) -> String { + format!( + "Telemetry: {}", + if self.telemetry_allowed { "on" } else { "off" } + ) + } +} + +#[derive(Serialize)] +struct ConnectivityReport { + reachable: bool, + #[serde(skip_serializing_if = "Option::is_none")] + status: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error: Option, +} + +impl ConnectivityReport { + /// Whether `host` answers at all, through the same client and proxy + /// handling every other request in this crate uses — so this reports + /// what a real command would actually experience, not a bare TCP probe + /// that a proxy-unaware check would get wrong in either direction. Any + /// HTTP response counts as reachable, including an error one: this is + /// asking whether the network path works, not whether the endpoint + /// likes an unauthenticated request to its root. + /// + /// `error` is left out of the JSON entirely outside `--debug` (via + /// `skip_serializing_if`), not set to `null` — the same shape the rest + /// of this crate uses `Option` for, and what the docs promise. + fn check(debug: bool, host: &str, timeout: Duration) -> Self { + let outcome = http::client().and_then(|client| { + client + .get(host) + .timeout(timeout) + .send() + .map_err(anyhow::Error::from) + }); + + match outcome { + Ok(response) => ConnectivityReport { + reachable: true, + status: Some(response.status().as_u16()), + error: None, + }, + Err(e) => ConnectivityReport { + reachable: false, + status: None, + error: debug.then(|| e.to_string()), + }, + } + } + + fn line(&self, host: &str) -> String { + match self.status { + Some(status) => format!("Reachable: yes ({host} answered {status})"), + None => format!("Reachable: no ({host})"), + } + } +} + +#[derive(Serialize)] +struct Report { + build: Build, + token: TokenReport, + proxy: ProxyReport, + switches: SwitchesReport, + #[serde(skip_serializing_if = "Option::is_none")] + connectivity: Option, +} + +impl Report { + fn text(&self, host: &str) -> String { + let mut lines = vec![ + self.build.line(), + self.token.line(), + self.proxy.line(), + self.switches.update_check_line(), + self.switches.telemetry_line(), + ]; + if let Some(connectivity) = &self.connectivity { + lines.push(connectivity.line(host)); + } + lines.join("\n") + } +} + +/// `matches` is the top-level parse, the same one `whoami` reads `--token` +/// and `--timeout` from — `verify` is not global, so it comes from +/// `doctor_matches`, the subcommand's own. +pub fn run( + matches: &ArgMatches, + use_login: bool, + debug: bool, + profile: Option<&str>, + mode: Mode, + doctor_matches: &ArgMatches, +) -> Result<()> { + let verify = doctor_matches.get_flag("verify"); + let host = api_host(); + let timeout = http::requested(matches).unwrap_or(DEFAULT_VERIFY_TIMEOUT); + + let report = Report { + build: Build::current(), + token: TokenReport::resolve(matches, use_login, profile), + proxy: ProxyReport::current(), + switches: SwitchesReport::current(), + connectivity: verify.then(|| ConnectivityReport::check(debug, &host, timeout)), + }; + + let text = report.text(&host); + let json = serde_json::to_value(&report)?; + output::emit(mode, &text, json) +} diff --git a/src/main.rs b/src/main.rs index a3a7961..743b0ed 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,6 +20,7 @@ mod completion; mod config; mod confirm; mod deprecation; +mod doctor; mod executor; mod generate_skills; mod http; @@ -596,6 +597,13 @@ fn build_app(specs: &[ServiceSpec]) -> Command { // machine, never the network. app = app.subcommand(config::command()); + // Reads what the other hand-written commands above also read — the + // token store, the proxy environment, the config and telemetry + // switches — so it belongs beside them rather than the API surface + // below. `--verify` is its one exception, the same opt-in + // `auth whoami --verify` already sets a precedent for. + app = app.subcommand(doctor::command()); + app = app.subcommand(account_usage::command()); app.subcommand(tilesets_cli::command()) @@ -1056,6 +1064,13 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - Some(("unset", unset_matches)) => config::unset(unset_matches, mode)?, _ => unreachable!("`config` sets subcommand_required(true)"), }, + // Also ahead of the generic service arm: read-only except for the + // opt-in `--verify` request, and needs no credential load of its own + // — it reports what one would resolve to, not what a fresh one + // would be. + Some((doctor::COMMAND, doctor_matches)) => { + doctor::run(matches, use_login, debug, profile, mode, doctor_matches)? + } // Token resolution mirrors the service arm below, minus path // placeholders, a request body, and `--dry-run` — this GET always refreshes. Some((account_usage::COMMAND, usage_matches)) => { diff --git a/src/schema.rs b/src/schema.rs index d49d655..27c5e57 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -379,6 +379,10 @@ fn commands(app: &Command, specs: &[ServiceSpec], path: &[String]) -> Vec PathBuf { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("doctor-{name}")); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(home.join("config")).expect("create the scratch config dir"); + home +} + +fn config_dir(home: &Path) -> PathBuf { + home.join("config") +} + +/// The real binary, isolated the same way the other command-level test +/// files isolate it — no token, no output mode, no config directory from +/// the developer's own environment. +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("HTTPS_PROXY") + .env_remove("https_proxy") + .env_remove("HTTP_PROXY") + .env_remove("http_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .env_remove("MAPBOX_CLI_NO_TELEMETRY") + .env_remove("MAPBOX_NO_UPDATE_CHECK") + .env_remove("MAPBOX_TIMEOUT") + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", config_dir(home)); + cmd +} + +fn stdout(output: &Output) -> serde_json::Value { + let text = String::from_utf8_lossy(&output.stdout); + serde_json::from_str(text.trim()) + .unwrap_or_else(|e| panic!("expected JSON, got {text:?} ({e})")) +} + +/// A loopback stand-in for `api.mapbox.com`, answering every request with +/// the given status and closing. +fn server(status_line: &str) -> (std::thread::JoinHandle<()>, String) { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let addr = listener.local_addr().expect("the bound address"); + let status_line = status_line.to_string(); + + let handle = std::thread::spawn(move || { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + let _ = stream.set_read_timeout(Some(Duration::from_secs(10))); + // Enough to know a request landed; the content doesn't matter here. + let mut buf = [0u8; 1024]; + let _ = stream.read(&mut buf); + let response = format!("{status_line}\r\nContent-Length: 0\r\n\r\n"); + let _ = stream.write_all(response.as_bytes()); + }); + + (handle, format!("http://{addr}/")) +} + +/// A port nothing is listening on, so a connection is refused rather than +/// merely slow — the unreachable case, answered quickly instead of by the +/// 5-second budget expiring. +fn closed_port_url() -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let addr = listener.local_addr().expect("the bound address"); + drop(listener); + format!("http://{addr}/") +} + +/// Accepts a connection and then never answers it — unlike +/// [`closed_port_url`], where the connection is refused immediately, this is +/// what actually exercises a timeout budget rather than an instant refusal. +/// The thread outlives the test (nothing tells it to stop), which is fine: +/// it holds one socket open until the process exits. +fn hanging_server() -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let addr = listener.local_addr().expect("the bound address"); + + std::thread::spawn(move || { + if let Ok((stream, _)) = listener.accept() { + // Held open, on purpose, for as long as this process runs. + std::mem::forget(stream); + } + }); + + format!("http://{addr}/") +} + +#[test] +fn with_no_token_and_nothing_configured() { + let home = scratch("bare"); + + let out = command(&home) + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + + let value = stdout(&out); + assert_eq!(value["token"]["available"], false); + assert_eq!(value["proxy"]["active"], serde_json::json!([])); + assert_eq!(value["switches"]["update_check_persisted"], true); + assert_eq!(value["switches"]["update_check_env_opt_out"], false); + assert_eq!(value["switches"]["telemetry_allowed"], true); + assert!( + value.get("connectivity").is_none(), + "no --verify, no connectivity field" + ); +} + +#[test] +fn a_token_in_the_environment_is_reported() { + let home = scratch("env-token"); + + let out = command(&home) + .env("MAPBOX_ACCESS_TOKEN", "sk.eyJ1IjoiYWxpY2UifQ.sig") + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + + let value = stdout(&out); + assert_eq!(value["token"]["available"], true); + assert_eq!(value["token"]["source"], "environment"); + assert_eq!(value["token"]["account"], "alice"); + assert_eq!(value["token"]["usage"], "sk"); +} + +#[test] +fn the_persisted_and_environment_switches_are_both_reflected() { + let home = scratch("switches"); + + let set = command(&home) + .args(["config", "set", "update-check", "off"]) + .output() + .expect("run mapbox config set"); + assert!(set.status.success()); + + let out = command(&home) + .env("MAPBOX_NO_UPDATE_CHECK", "1") + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + + let value = stdout(&out); + assert_eq!(value["switches"]["update_check_persisted"], false); + assert_eq!(value["switches"]["update_check_env_opt_out"], true); + assert_eq!(value["switches"]["telemetry_allowed"], false); +} + +#[test] +fn a_proxy_variable_is_named() { + let home = scratch("proxy"); + + let out = command(&home) + .env("HTTPS_PROXY", "http://localhost:9") + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + + let active = stdout(&out)["proxy"]["active"].clone(); + assert_eq!(active, serde_json::json!(["HTTPS_PROXY"])); +} + +#[test] +fn without_verify_nothing_is_sent() { + let home = scratch("no-verify"); + // A port nothing answers: if this were reached, the process would hang + // for the budget's duration instead of returning immediately. + let url = closed_port_url(); + + let start = std::time::Instant::now(); + let out = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + assert!( + start.elapsed() < Duration::from_secs(2), + "doctor without --verify should not have touched the network at all" + ); + assert!(stdout(&out).get("connectivity").is_none()); +} + +#[test] +fn verify_reports_a_reachable_host() { + let home = scratch("verify-ok"); + let (thread, url) = server("HTTP/1.1 200 OK"); + + let out = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "doctor", "--verify"]) + .output() + .expect("run mapbox doctor --verify"); + assert!(out.status.success()); + + let value = stdout(&out); + assert_eq!(value["connectivity"]["reachable"], true); + assert_eq!(value["connectivity"]["status"], 200); + thread.join().expect("the loopback server thread"); +} + +#[test] +fn verify_reports_an_unreachable_host() { + let home = scratch("verify-fail"); + let url = closed_port_url(); + + let out = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "doctor", "--verify"]) + .output() + .expect("run mapbox doctor --verify"); + assert!( + out.status.success(), + "a failed check is still a successful run" + ); + + let value = stdout(&out); + assert_eq!(value["connectivity"]["reachable"], false); +} + +#[test] +fn a_lowercase_proxy_variable_is_reported_under_its_uppercase_name() { + let home = scratch("proxy-lowercase"); + + let out = command(&home) + .env("https_proxy", "http://localhost:9") + .args(["-o", "json", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + + // Reported as `HTTPS_PROXY`, not `https_proxy`: Windows environment + // variables are case-insensitive, so the two spellings are the same + // variable there, and reporting under whichever was literally set would + // print `HTTPS_PROXY` on Windows for this exact test regardless — the + // canonical name is what both platforms agree on. + let active = stdout(&out)["proxy"]["active"].clone(); + assert_eq!(active, serde_json::json!(["HTTPS_PROXY"])); +} + +/// The bug this pins: the text line used to read only +/// `update_check_persisted`/`update_check_env_opt_out`, so +/// `MAPBOX_CLI_NO_TELEMETRY=1` alone — with the persisted setting still on +/// its default and the dedicated env switch unset — printed +/// "Update check: on" for a check that `update_check::enabled` would not +/// actually run, since that silences it too. +#[test] +fn telemetry_off_alone_is_enough_to_turn_the_reported_update_check_off() { + let home = scratch("telemetry-only"); + + let out = command(&home) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["-o", "text", "doctor"]) + .output() + .expect("run mapbox doctor"); + assert!(out.status.success()); + + let text = String::from_utf8_lossy(&out.stdout).to_string(); + assert!( + text.lines() + .any(|line| line.trim_start() == "Update check: off" + || line.starts_with("Update check: off")), + "{text}" + ); +} + +#[test] +fn verify_honors_an_explicit_timeout() { + let home = scratch("verify-timeout"); + let url = hanging_server(); + + let start = std::time::Instant::now(); + let out = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "--timeout", "1", "doctor", "--verify"]) + .output() + .expect("run mapbox --timeout 1 doctor --verify"); + let elapsed = start.elapsed(); + + assert!(out.status.success()); + assert_eq!(stdout(&out)["connectivity"]["reachable"], false); + assert!( + elapsed < Duration::from_secs(3), + "an explicit --timeout 1 should have cut this short, took {elapsed:?}" + ); +} + +#[test] +fn the_error_field_is_absent_without_debug_and_present_with_it() { + let home = scratch("verify-error-field"); + let url = closed_port_url(); + + let without_debug = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "doctor", "--verify"]) + .output() + .expect("run mapbox doctor --verify"); + assert!(without_debug.status.success()); + let value = stdout(&without_debug); + assert!( + value["connectivity"].get("error").is_none(), + "error should be absent, not null, outside --debug: {value}" + ); + + let with_debug = command(&home) + .env("MAPBOX_INTERNAL_DOCTOR_URL", &url) + .args(["-o", "json", "--debug", "doctor", "--verify"]) + .output() + .expect("run mapbox --debug doctor --verify"); + assert!(with_debug.status.success()); + let value = stdout(&with_debug); + assert!(value["connectivity"]["error"].is_string(), "{value}"); +}