diff --git a/CHANGELOG.md b/CHANGELOG.md index 99c9fc7..d16d971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,49 @@ All notable changes to bootintel-cli are documented here. Format follows [Keep a ## [Unreleased] +## [0.6.1] — 2026-09-27 — the gate survives a pipe, and history is opt-in + +**Read the second item before upgrading.** It changes a default, which the +policy above does not really call PATCH. It is numbered 0.6.1 by request; the +warning is here rather than in the version digit. + +### Fixed +- **`scan --gate-critical` could report success on a capture that trips the + gate.** Measured on the 0.6.0 release binary, piping into `head` with output + over the 64 KB pipe buffer: three runs gave exit 1, then 0, then 0, on a log + with telnet exposed. A CI job piping through `head` or `tee` would go green + on a device with a critical exposure, non-deterministically, which is the + worst direction for a gate to fail. + + Cause was ordering. The output write ran before the gate check, so a broken + pipe propagated up and the handler in main.rs turned it into exit 0 before + any verdict was evaluated. That handler is right that a reader closing the + pipe is not an error, but it must not become a verdict. The pipe error is now + swallowed at the write site only, and the gates decide on findings alone, + which do not depend on whether anyone was still reading. Any other write + error is still fatal, and `manpage | head` still exits 0 silently. + +### Changed +- **Scan history is now OPT IN. If you relied on it, it has stopped.** Enable + with `bootintel config set history true` or `BOOTINTEL_HISTORY=1`. + + It was on by default with an opt-out and a one-time notice. That is the wrong + default for a tool whose pitch is that it uploads nothing: a record of every + log path a consultant analysed should not appear on disk because nobody said + no. `bootintel doctor` disclosing it on a fresh machine is what prompted the + change. + + `no_history` and `BOOTINTEL_NO_HISTORY` still work and still mean off, so + anyone who had opted out stays opted out. An explicit off beats an explicit + on, so a machine-wide opt-out cannot be re-enabled by a config file. + +- Every "get an API key" message pointed at `bootintel.com/settings/api-keys`, + a route that has never existed; there is no `/settings` on the site. Keys + live at `/dashboard/developer`. Six places said otherwise: `doctor`, + `whoami` twice, `scan --api` twice, `analyze --api`. They now recommend + `bootintel login` first, which is the path for a person at a terminal and the + only one that works below Pro. + ### Fixed - **Every "get an API key" message pointed at a URL that 404s.** `https://bootintel.com/settings/api-keys` does not exist and never has; @@ -528,7 +571,8 @@ Initial release. All six subcommands live; five branch-based milestones (M1-M5) - PDF report download subcommand — server-side endpoint exists but no client-side wrapper yet. - Windows support — the Rust code compiles for Windows and the release workflow builds it, but install.sh doesn't handle Windows yet (`.ps1` installer is a follow-up). -[Unreleased]: https://github.com/bootintel/cli/compare/cli-v0.6.0...HEAD +[Unreleased]: https://github.com/bootintel/cli/compare/cli-v0.6.1...HEAD +[0.6.1]: https://github.com/bootintel/cli/releases/tag/cli-v0.6.1 [0.6.0]: https://github.com/bootintel/cli/releases/tag/cli-v0.6.0 [0.5.0]: https://github.com/bootintel/cli/releases/tag/cli-v0.5.0 [0.4.2]: https://github.com/bootintel/cli/releases/tag/cli-v0.4.2 diff --git a/Cargo.lock b/Cargo.lock index 4483efd..1f02def 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -131,7 +131,7 @@ checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" [[package]] name = "bootintel" -version = "0.6.0" +version = "0.6.1" dependencies = [ "anyhow", "arboard", @@ -154,7 +154,7 @@ dependencies = [ [[package]] name = "bootintel-detectors" -version = "0.6.0" +version = "0.6.1" dependencies = [ "regex", ] diff --git a/Cargo.toml b/Cargo.toml index 1a33644..d492ec2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = ["crates/detectors", "crates/cli"] resolver = "2" [workspace.package] -version = "0.6.0" +version = "0.6.1" edition = "2021" rust-version = "1.90" license = "Apache-2.0" diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 2aef007..9ea073a 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -24,7 +24,7 @@ path = "src/main.rs" # version when packaging for crates.io, which rejects a bare path dep. # This is what publish = false was working around; publishing # bootintel-detectors first makes the workaround unnecessary. -bootintel-detectors = { path = "../detectors", version = "0.6.0" } +bootintel-detectors = { path = "../detectors", version = "0.6.1" } clap = { version = "4.5", features = ["derive", "wrap_help", "env"] } # Shell-completion emitters for `bootintel completions `. Version # tracks the clap major/minor (4.5). No default features — we only use diff --git a/crates/cli/src/cmd/scan/mod.rs b/crates/cli/src/cmd/scan/mod.rs index 9d93452..3f939a7 100644 --- a/crates/cli/src/cmd/scan/mod.rs +++ b/crates/cli/src/cmd/scan/mod.rs @@ -219,16 +219,42 @@ pub fn run(args: Args) -> Result<()> { let stdout = io::stdout(); let color = output::resolve_color_mode(args.no_color, &stdout); let mut out = stdout.lock(); - output::write( + // A closed pipe must not decide the gate. + // + // `bootintel scan --gate-critical | head` previously reported success on a + // capture with telnet exposed: output::write hit EPIPE, `?` propagated it, + // and the broken-pipe handler in main.rs turned that into exit 0 before + // any gate check ran. The reader having seen enough is not an error, which + // is why that handler exists, but it must not become a verdict. + // + // So a broken pipe is swallowed HERE, and only here. Every gate below + // decides on `findings` alone, which do not depend on whether anyone was + // still reading. Any other write error is still fatal. + let mut stdout_closed = false; + if let Err(e) = output::write( &mut out, &findings, args.format, &raw, color, &log.source_label, - )?; - if args.context > 0 && matches!(args.format, Format::Text) { - context::write_context_blocks(&mut out, &findings, &raw, args.context, color)?; + ) { + if crate::is_broken_pipe(&e) { + stdout_closed = true; + } else { + return Err(e); + } + } + if !stdout_closed && args.context > 0 && matches!(args.format, Format::Text) { + if let Err(e) = + context::write_context_blocks(&mut out, &findings, &raw, args.context, color) + { + // Nothing reads stdout_closed past this point, so there is + // nothing to record: stop writing and let the gates decide. + if !crate::is_broken_pipe(&e) { + return Err(e); + } + } } // Pre-compute the values we'll write to history, so the per-exit diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index 76b4e2c..5756388 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -46,6 +46,9 @@ pub struct Config { /// API key. Overridden by `$BOOTINTEL_API_KEY`. No CLI flag — /// keys aren't accepted on argv for shell-history reasons. pub api_key: Option, + /// Opt IN to the local scan history. Default false. + #[serde(default)] + pub history: bool, /// Default output format for `scan` / `batch` / `view`. Any /// `--format` flag on the command line takes precedence. pub default_format: Option, @@ -210,21 +213,49 @@ pub fn effective_default_format(cfg: &Config) -> Effective { /// Resolve the `no_history` boolean. Env var `BOOTINTEL_NO_HISTORY=1` /// wins; config `no_history = true` is the fallback. Any other env /// value ("0", "false", empty, unset) means "consult the file". +/// Whether scan history is DISABLED. Now true unless explicitly opted in. +/// +/// It used to be on by default with an opt-out, disclosed in a one-time notice. +/// That was the wrong default for a tool whose pitch is that it uploads +/// nothing: a record of every log path a consultant analysed is exactly the +/// kind of thing that should not appear on disk because nobody said no. +/// +/// `no_history` / BOOTINTEL_NO_HISTORY still work and still mean off, so +/// anyone who had opted out stays opted out. Enabling now takes +/// `history = true` or BOOTINTEL_HISTORY=1. pub fn effective_no_history(cfg: &Config) -> (bool, Source) { + // An explicit off wins over an explicit on, so a machine-wide opt-out + // cannot be silently re-enabled by a config file. match std::env::var("BOOTINTEL_NO_HISTORY").ok().as_deref() { Some("1") | Some("true") | Some("yes") => return (true, Source::Env), _ => {} } - // With `no_history: bool` (post-P2-9 unification), we can't tell - // "explicitly set to false" from "unset" — both are false. Report - // Source::File when true, Source::Default when false. In practice - // the callsites don't care about that distinction: `config list` - // shows a value + source, and false-from-file vs false-from-default - // behave identically at runtime. if cfg.no_history { - (true, Source::File) - } else { - (false, Source::Default) + return (true, Source::File); + } + match std::env::var("BOOTINTEL_HISTORY").ok().as_deref() { + Some("1") | Some("true") | Some("yes") => return (false, Source::Env), + Some("0") | Some("false") | Some("no") => return (true, Source::Env), + _ => {} + } + if cfg.history { + return (false, Source::File); + } + // Default: no history. + return (true, Source::Default); + #[allow(unreachable_code)] + { + // With `no_history: bool` (post-P2-9 unification), we can't tell + // "explicitly set to false" from "unset" — both are false. Report + // Source::File when true, Source::Default when false. In practice + // the callsites don't care about that distinction: `config list` + // shows a value + source, and false-from-file vs false-from-default + // behave identically at runtime. + if cfg.no_history { + (true, Source::File) + } else { + (false, Source::Default) + } } } @@ -309,6 +340,15 @@ pub fn get_effective(cfg: &Config, key: ConfigKey) -> Effective { ConfigKey::ApiBase => effective_api_base(cfg), ConfigKey::ApiKey => effective_api_key(cfg), ConfigKey::DefaultFormat => effective_default_format(cfg), + ConfigKey::History => { + // Reported as the positive, which is what the user sets, while + // effective_no_history stays the single source of truth. + let (disabled, src) = effective_no_history(cfg); + Effective { + value: Some((!disabled).to_string()), + source: src, + } + } ConfigKey::NoHistory => { let (v, src) = effective_no_history(cfg); Effective { @@ -325,7 +365,7 @@ pub fn get_effective(cfg: &Config, key: ConfigKey) -> Effective { pub fn get_key(cfg: &Config, key: &str) -> Result { let k = ConfigKey::from_str(key).ok_or_else(|| { anyhow::anyhow!( - "unknown config key '{}' — valid: api_base, api_key, default_format, no_history", + "unknown config key '{}' — valid: api_base, api_key, default_format, history, no_history", key ) })?; @@ -343,6 +383,7 @@ pub enum ConfigKey { ApiBase, ApiKey, DefaultFormat, + History, NoHistory, } @@ -354,6 +395,7 @@ impl ConfigKey { ConfigKey::ApiBase => "api_base", ConfigKey::ApiKey => "api_key", ConfigKey::DefaultFormat => "default_format", + ConfigKey::History => "history", ConfigKey::NoHistory => "no_history", } } @@ -366,6 +408,7 @@ impl ConfigKey { "api_base" => Some(ConfigKey::ApiBase), "api_key" => Some(ConfigKey::ApiKey), "default_format" => Some(ConfigKey::DefaultFormat), + "history" => Some(ConfigKey::History), "no_history" => Some(ConfigKey::NoHistory), _ => None, } @@ -379,6 +422,7 @@ pub const ALL_KEYS: &[ConfigKey] = &[ ConfigKey::ApiBase, ConfigKey::ApiKey, ConfigKey::DefaultFormat, + ConfigKey::History, ConfigKey::NoHistory, ]; @@ -413,7 +457,7 @@ pub fn resolve_api_key() -> Option { fn apply_key(cfg: &mut Config, key: &str, value: &str) -> Result<()> { let k = ConfigKey::from_str(key).ok_or_else(|| { anyhow::anyhow!( - "unknown config key '{}' — valid: api_base, api_key, default_format, no_history", + "unknown config key '{}' — valid: api_base, api_key, default_format, history, no_history", key ) })?; @@ -428,6 +472,15 @@ fn apply(cfg: &mut Config, key: ConfigKey, value: &str) -> Result<()> { ConfigKey::ApiBase => cfg.api_base = Some(value.to_string()), ConfigKey::ApiKey => cfg.api_key = Some(value.to_string()), ConfigKey::DefaultFormat => cfg.default_format = Some(value.to_string()), + ConfigKey::History => { + cfg.history = parse_bool(value) + .with_context(|| format!("history must be a boolean (true/false), got: {value}"))?; + // Setting history=true clears a stale opt-out, otherwise the + // explicit-off precedence would silently ignore the request. + if cfg.history { + cfg.no_history = false; + } + } ConfigKey::NoHistory => { cfg.no_history = parse_bool(value).with_context(|| { format!("no_history must be a boolean (true/false), got: {value}") @@ -475,6 +528,7 @@ mod tests { std::env::remove_var("BOOTINTEL_API_KEY"); std::env::remove_var("BOOTINTEL_API_BASE"); std::env::remove_var("BOOTINTEL_NO_HISTORY"); + std::env::remove_var("BOOTINTEL_HISTORY"); } #[test] @@ -568,16 +622,65 @@ mod tests { assert_eq!(src, Source::File); } + /// The default INVERTED: history is off unless asked for. It used to be on + /// with an opt-out, which is the wrong default for a tool whose pitch is + /// that it uploads nothing. A record of every log path a consultant + /// analysed should not appear on disk because nobody said no. #[test] - fn no_history_default_false() { + fn history_is_off_by_default() { let _g = env_lock(); clear_env(); let cfg = Config::default(); - let (v, src) = effective_no_history(&cfg); - assert!(!v); + let (disabled, src) = effective_no_history(&cfg); + assert!(disabled, "history must be off unless opted in"); assert_eq!(src, Source::Default); } + #[test] + fn history_can_be_opted_into_by_file_or_env() { + let _g = env_lock(); + clear_env(); + let cfg = Config { + history: true, + ..Default::default() + }; + assert!( + !effective_no_history(&cfg).0, + "history=true should enable it" + ); + + clear_env(); + std::env::set_var("BOOTINTEL_HISTORY", "1"); + assert!( + !effective_no_history(&Config::default()).0, + "env should enable it" + ); + clear_env(); + } + + /// An explicit off beats an explicit on, so a machine-wide opt-out cannot + /// be silently re-enabled by a config file that happens to say history. + #[test] + fn an_explicit_opt_out_wins_over_an_opt_in() { + let _g = env_lock(); + clear_env(); + let cfg = Config { + history: true, + no_history: true, + ..Default::default() + }; + assert!(effective_no_history(&cfg).0); + + clear_env(); + std::env::set_var("BOOTINTEL_NO_HISTORY", "1"); + let cfg2 = Config { + history: true, + ..Default::default() + }; + assert!(effective_no_history(&cfg2).0); + clear_env(); + } + #[test] fn set_roundtrip_via_atomic_write() { // We can't easily point set_key at a temp dir without diff --git a/crates/cli/src/history.rs b/crates/cli/src/history.rs index dd461e6..9cfde6a 100644 --- a/crates/cli/src/history.rs +++ b/crates/cli/src/history.rs @@ -115,7 +115,7 @@ pub fn is_disabled() -> bool { fn try_append(entry: &Entry) -> Result<()> { if is_disabled() { - crate::vdebug!("history: writes disabled (BOOTINTEL_NO_HISTORY / no_history=true)"); + crate::vdebug!("history: off (default). Enable with `bootintel config set history true` or BOOTINTEL_HISTORY=1"); return Ok(()); } let path = history_path().context("no platform state dir resolvable")?; @@ -161,25 +161,24 @@ fn try_append(entry: &Entry) -> Result<()> { Ok(()) } -/// Tell the user, once, that we are keeping a local scan history. +/// Confirm, once, that the history the user asked for has started. /// -/// Called only when the history file is about to be created for the -/// first time. stderr, so it never lands in a redirected report. +/// This used to disclose a history that was on by default and offer a way +/// out. It is now opt-in, so the notice confirms a choice instead of +/// announcing a surprise, and no longer needs to explain how to escape. +/// Called only when the file is about to be created. stderr, so it never +/// lands in a redirected report. fn announce_first_write(path: &std::path::Path) { if crate::verbose::is_quiet() { return; } eprintln!( - "[bootintel] note: recording a local scan history at {}", + "[bootintel] scan history enabled; recording to {}", path.display() ); eprintln!("[bootintel] One line per scan: timestamp, log path, finding counts, exit code."); - eprintln!("[bootintel] It stays on this machine — bootintel uploads nothing without --api."); - eprintln!("[bootintel] Turn it off with `bootintel config set no_history true`, or"); - eprintln!( - "[bootintel] per-run with BOOTINTEL_NO_HISTORY=1. Inspect it with `bootintel history`." - ); - eprintln!("[bootintel] This notice prints once."); + eprintln!("[bootintel] It stays on this machine. Inspect it with `bootintel history`."); + eprintln!("[bootintel] Stop with `bootintel config set history false`. Prints once."); } /// Open the history file for append. On Unix we use `.mode(0o600)` @@ -402,6 +401,8 @@ mod tests { std::env::set_var("XDG_STATE_HOME", &td); // Make sure no_history isn't set anywhere in the env. std::env::remove_var("BOOTINTEL_NO_HISTORY"); + // History is opt-in now, so clearing the opt-out is no longer enough. + std::env::set_var("BOOTINTEL_HISTORY", "1"); for i in 0..5 { append_entry(&Entry { @@ -480,6 +481,8 @@ mod tests { let saved = std::env::var("XDG_STATE_HOME").ok(); std::env::set_var("XDG_STATE_HOME", &td); std::env::remove_var("BOOTINTEL_NO_HISTORY"); + // History is opt-in now, so clearing the opt-out is no longer enough. + std::env::set_var("BOOTINTEL_HISTORY", "1"); append_entry(&Entry { ts: "2026-08-29T00:00:00Z".into(), @@ -520,6 +523,8 @@ mod tests { let saved = std::env::var("XDG_STATE_HOME").ok(); std::env::set_var("XDG_STATE_HOME", &td); std::env::remove_var("BOOTINTEL_NO_HISTORY"); + // History is opt-in now, so clearing the opt-out is no longer enough. + std::env::set_var("BOOTINTEL_HISTORY", "1"); let path = history_path().unwrap(); std::fs::create_dir_all(path.parent().unwrap()).unwrap();