From b2287c99ee7c308ae22fa1b6672deb05f4e86d44 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 1 May 2026 20:14:27 -0700 Subject: [PATCH] =?UTF-8?q?--self-update=20(alias=20--update)=20on=20a=20s?= =?UTF-8?q?can=20or=20other=20command=20now=20**re-execs=20into=20the=20fr?= =?UTF-8?q?eshly=20installed=20binary**=20so=20the=20current=20invocation?= =?UTF-8?q?=20completes=20with=20the=20new=20code=20and=20the=20latest=20d?= =?UTF-8?q?etection=20rules.=20Previously=20the=20on-disk=20binary=20was?= =?UTF-8?q?=20replaced=20but=20the=20running=20process=20kept=20using=20th?= =?UTF-8?q?e=20old=20in-memory=20version,=20requiring=20a=20second=20invoc?= =?UTF-8?q?ation=20to=20pick=20up=20the=20changes.=20On=20Unix=20this=20is?= =?UTF-8?q?=20a=20true=20exec()=20(same=20PID);=20on=20Windows=20the=20new?= =?UTF-8?q?=20binary=20is=20spawned=20and=20the=20parent=20exits=20with=20?= =?UTF-8?q?its=20status=20code.=20The=20explicit=20kingfisher=20self-updat?= =?UTF-8?q?e=20subcommand=20still=20updates=20and=20exits=20without=20re-e?= =?UTF-8?q?xecing.=20Self-update=20now=20also=20covers=20Windows=20arm64?= =?UTF-8?q?=20(the=20asset=20was=20already=20published;=20the=20runtime=20?= =?UTF-8?q?cfg=20map=20gained=20the=20missing=20arm).=20See=20docs/ADVANCE?= =?UTF-8?q?D.md=20=E2=86=92=20*Update=20Checks*.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 1 + docs-site/docs/changelog.md | 1 + docs/ADVANCED.md | 12 +- src/main.rs | 127 ++++++++++++++++++-- src/update.rs | 233 +++++++++++++++++++++++++++++++++++- tests/smoke_update.rs | 41 +++++++ 6 files changed, 402 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6f3f6d..75a4ca8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. ## [v1.99.0] +- `--self-update` (alias `--update`) on a scan or other command now **re-execs into the freshly installed binary** so the current invocation completes with the new code and the latest detection rules. Previously the on-disk binary was replaced but the running process kept using the old in-memory version, requiring a second invocation to pick up the changes. On Unix this is a true `exec()` (same PID); on Windows the new binary is spawned and the parent exits with its status code. The explicit `kingfisher self-update` subcommand still updates and exits without re-execing. Self-update now also covers Windows arm64 (the asset was already published; the runtime cfg map gained the missing arm). See `docs/ADVANCED.md` → *Update Checks*. - `--include-contributors` now respects `--github-repo-type` when enumerating contributor-owned repositories: by default contributor forks are excluded (matching the existing `Source` default), previously they were always included regardless of the flag. Added a new `--github-repo-type all` option to opt into the prior behavior of scanning both source and fork repos for contributors, organizations, and users. - **Access Map:** Pinecone API keys (validated `kingfisher.pinecone.1`): caller resources via `GET /indexes` (with serverless cloud/region or pod environment metadata, deletion-protection state) and `GET /collections`; standalone `kingfisher access-map pinecone` (alias `pinecone.io`). - Added `--blast-radius` as an alias for `--access-map` on `kingfisher scan`, and `kingfisher blast-radius ` as an alias for the `kingfisher access-map ` subcommand, so the user-facing "blast radius" concept matches the CLI invocation. diff --git a/docs-site/docs/changelog.md b/docs-site/docs/changelog.md index 1992f8f..557fa19 100644 --- a/docs-site/docs/changelog.md +++ b/docs-site/docs/changelog.md @@ -8,6 +8,7 @@ description: "Kingfisher release history: new features, rules, bug fixes, and im All notable changes to this project will be documented in this file. ## [unreleased v1.99.0] +- `--self-update` (alias `--update`) on a scan or other command now **re-execs into the freshly installed binary** so the current invocation completes with the new code and the latest detection rules. Previously the on-disk binary was replaced but the running process kept using the old in-memory version, requiring a second invocation to pick up the changes. On Unix this is a true `exec()` (same PID); on Windows the new binary is spawned and the parent exits with its status code. The explicit `kingfisher self-update` subcommand still updates and exits without re-execing. Self-update now also covers Windows arm64 (the asset was already published; the runtime cfg map gained the missing arm). See `docs/ADVANCED.md` → *Update Checks*. - `--include-contributors` now respects `--github-repo-type` when enumerating contributor-owned repositories: by default contributor forks are excluded (matching the existing `Source` default), previously they were always included regardless of the flag. Added a new `--github-repo-type all` option to opt into the prior behavior of scanning both source and fork repos for contributors, organizations, and users. - **Access Map:** Pinecone API keys (validated `kingfisher.pinecone.1`): caller resources via `GET /indexes` (with serverless cloud/region or pod environment metadata, deletion-protection state) and `GET /collections`; standalone `kingfisher access-map pinecone` (alias `pinecone.io`). - Added `--blast-radius` as an alias for `--access-map` on `kingfisher scan`, and `kingfisher blast-radius ` as an alias for the `kingfisher access-map ` subcommand, so the user-facing "blast radius" concept matches the CLI invocation. diff --git a/docs/ADVANCED.md b/docs/ADVANCED.md index 8d78270..a8728c8 100644 --- a/docs/ADVANCED.md +++ b/docs/ADVANCED.md @@ -435,11 +435,17 @@ See [FINGERPRINT.md](FINGERPRINT.md) for complete details. ## Update Checks -Kingfisher automatically queries GitHub for a newer release when it starts and tells you whether an update is available. +Kingfisher automatically queries GitHub for a newer release when it starts and tells you whether an update is available. The check is informational only — the binary is not modified unless you explicitly opt in. -- **Manual update** – Run `kingfisher update` to update the binary without scanning +- **Update and exit** – Run `kingfisher self-update` (alias `kingfisher update`) to download the latest release, replace the running binary in place, and exit. No scanning occurs. -- **Disable version checks** – Pass `--no-update-check` to skip both the startup and shutdown checks entirely +- **Update then run with the new version** – Pass the global `--self-update` flag (alias `--update`) on any scan or other command. If a newer release exists, Kingfisher downloads it, replaces the on-disk binary, and **re-execs into the freshly installed binary** so the current invocation completes with the new code (including the latest detection rules). On Unix this is a true `exec()` (same PID); on Windows the new binary is spawned and the parent exits with its status code. If no update is available, the command runs normally with no extra steps. + +- **Disable version checks** – Pass `--no-update-check` to skip both the startup and shutdown checks entirely. Recommended for CI runs to keep behavior reproducible. + +Self-update writes to wherever the running binary lives, so it requires the calling user to have write access to that location. If you installed Kingfisher via a package manager (Homebrew, the `.deb`/`.rpm` packages, the PyPI wrapper, etc.), use that package manager's upgrade command instead — Kingfisher will detect the permission error and tell you so. + +Self-update supports all six release platforms: Linux x64/arm64, macOS x64/arm64, and Windows x64/arm64. ## Exit Codes diff --git a/src/main.rs b/src/main.rs index b0cd3ca..6572a79 100644 --- a/src/main.rs +++ b/src/main.rs @@ -79,7 +79,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::{load_and_record_rules, run_scan}, - update::check_for_update_async, + update::{check_for_update_async, rewrite_argv_for_reexec}, validation::set_user_agent_suffix, }; use serde_json::json; @@ -115,6 +115,13 @@ fn main() -> anyhow::Result<()> { handler.join().unwrap_or_else(|e| std::panic::resume_unwind(e)) } +/// Outcome of `async_main`. Used to signal that the runtime should be torn down +/// and the process should re-exec into a freshly self-updated binary. +enum AsyncMainOutcome { + Done, + Reexec, +} + fn run() -> anyhow::Result<()> { // Rustls 0.23 requires an explicit crypto provider selection when multiple // providers are present in the dependency graph. @@ -154,7 +161,97 @@ fn run() -> anyhow::Result<()> { .enable_all() .build() .context("Failed to create Tokio runtime")?; - runtime.block_on(async_main(args)) + let outcome = runtime.block_on(async_main(args))?; + // Drop the Tokio runtime before re-exec so background tasks, file descriptors, + // and signal handlers are torn down cleanly. On Unix `exec()` replaces the process + // image regardless, but draining the runtime first avoids surprising shutdown + // ordering when the re-exec happens to fail. + drop(runtime); + + match outcome { + AsyncMainOutcome::Done => Ok(()), + AsyncMainOutcome::Reexec => { + // On Unix, a successful exec() never returns; on Windows, reexec_with_new_binary + // calls process::exit. We only reach here if the re-exec failed before transferring + // control. The on-disk binary is now the updated version, so re-running the same + // command will work — but the original command has NOT executed, so we must not + // exit 0 and let CI think the run succeeded. + if let Err(e) = reexec_with_new_binary() { + error!( + "Binary was updated but re-exec failed: {e}. The original command did not \ + run. Re-run the command to use the new binary." + ); + std::process::exit(1); + } + Ok(()) + } + } +} + +/// Re-exec the current process into the binary at `current_exe()` so a freshly +/// self-updated binary takes over the current invocation. +/// +/// Argv is rewritten via [`rewrite_argv_for_reexec`] to prevent loops and to skip the +/// next update check. +/// +/// On Unix this calls `exec()` which replaces the process image — same PID, parent +/// shell sees the new binary's exit code directly. +/// +/// On Windows there is no true `exec()`. Standard practice (rustup, cargo) is to spawn +/// the new binary, wait, and propagate its exit code. This adds a parent process layer +/// but preserves the parent shell's child-process tracking. +fn reexec_with_new_binary() -> std::io::Result<()> { + use std::process::Command; + + let exe = std::env::current_exe()?; + let argv: Vec = rewrite_argv_for_reexec(std::env::args_os()); + + // Defensive: rewrite_argv_for_reexec returns an empty Vec only when args_os() was empty, + // which shouldn't happen for a real CLI invocation but would produce a child process with + // no argv[0]. Bail rather than spawn something nonsensical. + if argv.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "cannot re-exec: process started with empty argv", + )); + } + + // Make sure prior stderr/stdout output (e.g. "Updated to version X") is committed + // before we either replace the process image (Unix) or spawn the child (Windows). On + // Windows the child inherits the same handles, so leftover buffered output from the + // parent could otherwise interleave with the child's output unpredictably. + let _ = std::io::stdout().flush(); + let _ = writeln!(std::io::stderr(), "Restarting with updated binary..."); + let _ = std::io::stderr().flush(); + + // Safe by the is_empty() guard above. + let argv0 = argv[0].clone(); + + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + let err = Command::new(&exe).args(argv.iter().skip(1)).arg0(&argv0).exec(); + // exec() returns only on failure. + Err(err) + } + + #[cfg(windows)] + { + // arg0 spoofing isn't available on Windows; the child sees the resolved exe path + // as argv[0]. The user-visible difference is cosmetic. + let _ = argv0; + let status = Command::new(&exe).args(argv.iter().skip(1)).status()?; + std::process::exit(status.code().unwrap_or(1)); + } + + #[cfg(not(any(unix, windows)))] + { + let _ = (exe, argv0); + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "re-exec is not supported on this platform", + )) + } } fn setup_logging(global_args: &GlobalArgs) { @@ -235,20 +332,26 @@ pub fn determine_exit_code(datastore: &Arc> } } -async fn async_main(args: CommandLineArgs) -> Result<()> { +async fn async_main(args: CommandLineArgs) -> Result { setup_logging(&args.global_args); let global_args = args.global_args.clone(); match args.command { Command::SelfUpdate => { + // The explicit `kingfisher self-update` subcommand intentionally does NOT + // re-exec after updating: it has no further work to do, so simply exiting + // is the correct end-of-run behavior. The re-exec path is reserved for the + // global `--self-update` flag combined with another command (e.g. `scan`). let mut g = global_args; g.self_update = true; g.no_update_check = false; let _ = check_for_update_async(&g, None).await; - Ok(()) + Ok(AsyncMainOutcome::Done) + } + Command::View(view_args) => view::run(view_args).await.map(|_| AsyncMainOutcome::Done), + Command::AccessMap(identity_args) => { + access_map::run(identity_args).await.map(|_| AsyncMainOutcome::Done) } - Command::View(view_args) => view::run(view_args).await, - Command::AccessMap(identity_args) => access_map::run(identity_args).await, Command::Validate(validate_args) => { let results = direct_validate::run_direct_validation(&validate_args, &global_args).await?; @@ -256,7 +359,7 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { direct_validate::print_results(&results, &validate_args.format, use_color); // Exit with code 0 if any result is valid, 1 if all invalid if direct_validate::any_valid(&results) { - Ok(()) + Ok(AsyncMainOutcome::Done) } else { std::process::exit(1); } @@ -267,13 +370,19 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { direct_revoke::print_results(&results, &revoke_args.format, use_color); // Exit with code 0 if any result revoked, 1 if all failed if direct_revoke::any_revoked(&results) { - Ok(()) + Ok(AsyncMainOutcome::Done) } else { std::process::exit(1); } } command => { let update_status = check_for_update_async(&global_args, None).await; + // If the on-disk binary was just replaced by --self-update, return early so + // fn run() can drop the runtime and re-exec into the new binary. The current + // invocation will resume with the new code (e.g. updated rule set). + if update_status.was_self_updated { + return Ok(AsyncMainOutcome::Reexec); + } match command { Command::Scan(scan_command) => match scan_command.into_operation()? { ScanOperation::Scan(mut scan_args) => { @@ -503,7 +612,7 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { if let Some(message) = &update_status.message { info!("{}", message); } - Ok(()) + Ok(AsyncMainOutcome::Done) } } } diff --git a/src/update.rs b/src/update.rs index c525dd4..7eb29f4 100644 --- a/src/update.rs +++ b/src/update.rs @@ -15,6 +15,7 @@ // `style_finding_active_heading` style so that they stand out alongside normal // scan output. +use std::ffi::OsString; use std::io::{ErrorKind, Write}; use self_update::{backends::github::Update, cargo_crate_version, errors::Error as UpdError}; @@ -51,6 +52,10 @@ pub struct UpdateStatus { pub running_version: String, pub latest_version: Option, pub check_status: UpdateCheckStatus, + /// True only when the on-disk binary was just replaced by a successful self-update. + /// Callers use this signal to re-exec into the new binary so the current invocation + /// runs with the updated code. + pub was_self_updated: bool, } impl Default for UpdateStatus { @@ -62,6 +67,7 @@ impl Default for UpdateStatus { running_version: cargo_crate_version!().to_string(), latest_version: None, check_status: UpdateCheckStatus::Disabled, + was_self_updated: false, } } } @@ -73,7 +79,9 @@ fn styled_heading(styles: &Styles, text: &str) -> String { /// Check GitHub for a newer Kingfisher release and optionally self-update. /// /// * `base_url` lets tests point at a mock server. -/// * Self-update is skipped when the user disabled it **or** the binary is a Homebrew install. +/// * Self-update is performed only when `global_args.self_update` is set and `--no-update-check` +/// was not passed. If the running binary is installed via a package manager the underlying +/// `self_update` call surfaces a permission error which is reported to the user. pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> UpdateStatus { let running_version = cargo_crate_version!().to_string(); @@ -85,6 +93,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: None, check_status: UpdateCheckStatus::Disabled, + was_self_updated: false, }; } @@ -127,6 +136,9 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd #[cfg(all(target_os = "windows", target_arch = "x86_64"))] builder.target("windows-x64"); + #[cfg(all(target_os = "windows", target_arch = "aarch64"))] + builder.target("windows-arm64"); + // ────────────────────────────────────────────────────── // Disambiguate archive format to avoid picking .deb packages. // Linux and macOS releases use `.tgz`; Windows uses `.zip`. @@ -150,6 +162,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: None, check_status: UpdateCheckStatus::Failed, + was_self_updated: false, }; }; @@ -165,6 +178,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: None, check_status: UpdateCheckStatus::Failed, + was_self_updated: false, }; }; @@ -179,6 +193,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: Some(release.version), check_status: UpdateCheckStatus::Ok, + was_self_updated: false, }; } @@ -200,6 +215,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: Some(release.version), check_status: UpdateCheckStatus::Ok, + was_self_updated: false, }; } // else fall through to Case 3 (latest > running) @@ -211,11 +227,13 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd let _ = writeln!(std::io::stderr(), "{}", styled_message); // Attempt self-update when allowed and feasible. + let mut was_self_updated = false; if global_args.self_update { match updater.update() { Ok(status) => { let message = format!("Updated to version {}", status.version()); let _ = writeln!(std::io::stderr(), "{}", styled_heading(&styles, &message)); + was_self_updated = true; } Err(e) => match e { UpdError::Io(ref io_err) => match io_err.kind() { @@ -257,6 +275,7 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Upd running_version, latest_version: Some(release.version), check_status: UpdateCheckStatus::Ok, + was_self_updated, } } @@ -277,3 +296,215 @@ pub async fn check_for_update_async( } } } + +/// Rewrite the current process argv for re-execution into a freshly self-updated binary. +/// +/// - argv[0] is preserved unchanged. +/// - `--self-update` and `--update` (and their `--flag=value` forms) are stripped so the +/// re-exec'd binary does not loop back into another self-update. +/// - `--no-update-check` is appended (idempotently) since we just performed the check. +/// - Tokens after the first `--` separator are passed through untouched (they are positional +/// from clap's perspective and may legitimately contain anything). +/// - If the input has no argv[0] (theoretical — real-world processes always have one), the +/// output is empty too. This avoids producing a broken argv where `--no-update-check` would +/// be promoted to the new process's argv[0]. +pub fn rewrite_argv_for_reexec(argv: impl IntoIterator) -> Vec { + // Byte-level prefix check that works on both UTF-8 and non-UTF-8 OsStrings. + fn os_starts_with(tok: &OsString, prefix: &[u8]) -> bool { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStrExt; + tok.as_os_str().as_bytes().starts_with(prefix) + } + #[cfg(windows)] + { + // On Windows OsStrings are WTF-16; encode the ASCII prefix the same way and + // compare wide units. ASCII characters round-trip cleanly to single u16 units. + use std::os::windows::ffi::OsStrExt; + let prefix_wide: Vec = prefix.iter().map(|&b| b as u16).collect(); + let tok_wide: Vec = tok.as_os_str().encode_wide().collect(); + tok_wide.starts_with(&prefix_wide) + } + #[cfg(not(any(unix, windows)))] + { + // Fallback for unknown targets: best-effort UTF-8 conversion. + tok.to_str() + .map(|s| s.as_bytes().starts_with(prefix)) + .unwrap_or(false) + } + } + + let mut iter = argv.into_iter(); + let mut out: Vec = Vec::new(); + let mut already_has_no_update_check = false; + let mut hit_double_dash = false; + let had_argv0; + + if let Some(argv0) = iter.next() { + out.push(argv0); + had_argv0 = true; + } else { + had_argv0 = false; + } + + for tok in iter { + if hit_double_dash { + // After `--`, every token is positional and must be passed through verbatim. + out.push(tok); + continue; + } + + if tok == "--" { + hit_double_dash = true; + out.push(tok); + continue; + } + + // Strip the flags that would re-trigger a self-update on the next process. + if tok == "--self-update" || tok == "--update" { + continue; + } + if os_starts_with(&tok, b"--self-update=") || os_starts_with(&tok, b"--update=") { + continue; + } + + if tok == "--no-update-check" { + already_has_no_update_check = true; + } + + out.push(tok); + } + + // Only append --no-update-check when we actually preserved an argv[0]. In the + // theoretical empty-input case, returning an empty Vec keeps argv shape-consistent + // and lets the caller decide what to do. + if had_argv0 && !already_has_no_update_check { + out.push(OsString::from("--no-update-check")); + } + + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn os(s: &str) -> OsString { + OsString::from(s) + } + + fn argv(args: &[&str]) -> Vec { + args.iter().map(|s| os(s)).collect() + } + + #[test] + fn rewrite_argv_strips_self_update() { + let result = rewrite_argv_for_reexec(argv(&["kingfisher", "scan", ".", "--self-update"])); + assert_eq!(result, argv(&["kingfisher", "scan", ".", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_strips_update_alias() { + let result = rewrite_argv_for_reexec(argv(&["kingfisher", "scan", "foo", "--update"])); + assert_eq!(result, argv(&["kingfisher", "scan", "foo", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_strips_eq_form() { + let result = + rewrite_argv_for_reexec(argv(&["kingfisher", "--self-update=true", "scan", "foo"])); + assert_eq!(result, argv(&["kingfisher", "scan", "foo", "--no-update-check"])); + + let result = rewrite_argv_for_reexec(argv(&["kingfisher", "--update=true", "scan", "foo"])); + assert_eq!(result, argv(&["kingfisher", "scan", "foo", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_appends_no_update_check_when_absent() { + let result = rewrite_argv_for_reexec(argv(&["kingfisher", "scan", "."])); + assert_eq!(result, argv(&["kingfisher", "scan", ".", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_idempotent_when_no_update_check_already_present() { + let result = rewrite_argv_for_reexec(argv(&[ + "kingfisher", + "scan", + ".", + "--no-update-check", + "--self-update", + ])); + assert_eq!(result, argv(&["kingfisher", "scan", ".", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_preserves_argv0() { + let result = rewrite_argv_for_reexec(argv(&[ + "/weird path/kingfisher-bin", + "scan", + ".", + "--self-update", + ])); + assert_eq!(result, argv(&["/weird path/kingfisher-bin", "scan", ".", "--no-update-check"])); + } + + #[test] + fn rewrite_argv_preserves_tokens_after_double_dash() { + // --self-update appearing AFTER `--` is a positional and must be preserved. + let result = rewrite_argv_for_reexec(argv(&[ + "kingfisher", + "scan", + "--self-update", + "--", + "--self-update", + "--update", + ])); + assert_eq!( + result, + argv(&["kingfisher", "scan", "--", "--self-update", "--update", "--no-update-check"]) + ); + } + + #[test] + fn rewrite_argv_empty_input_returns_empty() { + // If args_os() somehow returns nothing (theoretical), we must not synthesize a + // bogus argv where --no-update-check becomes argv[0] of the new process. + let result: Vec = rewrite_argv_for_reexec(Vec::::new()); + assert!(result.is_empty(), "empty input must produce empty output, got {:?}", result); + } + + #[test] + fn rewrite_argv_does_not_strip_unrelated_update_prefixed_flags() { + // A future flag like --update-rules must NOT be stripped by the --update= prefix check. + let result = rewrite_argv_for_reexec(argv(&[ + "kingfisher", + "rules", + "--update-rules", + "--self-updateable=ignored", + ])); + assert_eq!( + result, + argv(&[ + "kingfisher", + "rules", + "--update-rules", + "--self-updateable=ignored", + "--no-update-check" + ]) + ); + } + + #[cfg(unix)] + #[test] + fn rewrite_argv_handles_non_utf8_value_in_eq_form() { + // On Unix, a --self-update= with non-UTF-8 bytes after the `=` must still + // be stripped — the byte-level prefix check makes this work even when to_str() fails. + use std::os::unix::ffi::OsStringExt; + let mut bad = b"--self-update=".to_vec(); + bad.extend_from_slice(&[0xff, 0xfe]); // invalid UTF-8 trailer + let bad_os = OsString::from_vec(bad); + let input: Vec = vec![os("kingfisher"), os("scan"), bad_os, os(".")]; + let result = rewrite_argv_for_reexec(input); + assert_eq!(result, argv(&["kingfisher", "scan", ".", "--no-update-check"])); + } +} diff --git a/tests/smoke_update.rs b/tests/smoke_update.rs index af542d7..3f7c742 100644 --- a/tests/smoke_update.rs +++ b/tests/smoke_update.rs @@ -51,4 +51,45 @@ async fn detects_new_release() { .expect("update check should return a message") .contains("99.999.0") ); + // Detection alone (without --self-update) must never flip the re-exec signal. + assert!(!status.was_self_updated); +} + +/// When --self-update is requested but the actual download/replace step fails (which +/// is what happens with the wiremock since `http://example.com/bin` won't deliver a +/// real archive), `was_self_updated` MUST stay false. This is the guardrail that the +/// re-exec path is never triggered on a failed update. +#[tokio::test] +async fn self_update_failure_does_not_set_reexec_flag() { + let server = MockServer::start().await; + + let body = serde_json::json!({ + "tag_name": "v99.999.0", + "created_at": "2025-01-01T00:00:00Z", + "name": "Kingfisher 99.999.0", + "body": "", + "assets": [{"url": "http://example.com/bin", "name": "bin"}] + }); + + for m in ["HEAD", "GET"] { + Mock::given(method(m)) + .and(path("/repos/mongodb/kingfisher/releases/latest")) + .respond_with(ResponseTemplate::new(200).set_body_json(&body)) + .mount(&server) + .await; + } + + let status = tokio::task::spawn_blocking({ + let uri = server.uri(); + let args = GlobalArgs { self_update: true, ..Default::default() }; + move || check_for_update(&args, Some(&uri)) + }) + .await + .expect("blocking task panicked"); + + assert!(status.is_outdated); + assert!( + !status.was_self_updated, + "self-update download failed against the mock; was_self_updated must remain false" + ); }