Hotfix/client update reinstall hardening

[danylo-babenko-flamingo]

Harden tool supervision, updates and reinstall against partial failures

Problem

A Windows endpoint went dark for 12 days on 7 Sep. The update to 1.3.10 succeeded; 9 seconds later the agent exited.

20:21:36  Update to 1.3.10 succeeded (running binary matches target)
20:21:42  Found 2 process(es) to stop for tool: fleetmdm-agent
20:21:45  ERROR Failed to stop process 6280 after 3 attempts
20:21:45  INFO  Service core completed          ← nothing for 12 days

One leftover fleetmdm-agent was wedged in the kernel and survived three force-kills. Before launching a tool we clear its leftovers, and that error propagated through five ? frames — tool_kill_service → run_tool → run() → Client::start() → service core. One stuck tool killed the whole agent.

Nobody noticed because the service reported SERVICE_STOPPED with Win32(0) — identical to a clean stop — so SCM never fired the 10s/60s/300s recovery ladder the agent configures for itself. The failure reason went to eprintln! and never reached the log.

An audit found the same shape elsewhere: operations that fail partway leave the agent, a tool or its enrollment worse off than before. Those are fixed here too.

Fix

Supervision — the kill moved inside the supervisor loop, where a stuck leftover is now a retryable launch failure (delay escalates 5s → 300s) instead of a fatal error. It never launches beside a live leftover, and self-heals when the process clears. run() also isolates per tool, and lib.rs no longer lets the tool lane end the service core.

Windows recovery — service.rs reports ServiceSpecific(1) on failure and Win32(0) only on a graceful stop, so the configured recovery ladder can finally fire. Reason now logged via tracing::error!.

Updates — tool_agent_update_service takes tool_lock as its first action, before the record is read, like install/uninstall/restart already do, and uses the Drop-based UpdatingGuard so a leaked flag can't park the supervisor. Asset-only updates now restart the tool (nothing did before: run_tool returns early for Installation::Service and run() never starts services, so a routine osqueryd bump stopped orbit for good) — Service via start_service, macOS GuiApp via a fresh supervisor. The stop uses stop_installed_tool rather than a process kill — on macOS a pattern kill leaves the launchd job loaded, making the follow-up launchctl load a silent no-op.

Retry and start-verification are deliberately not implemented here — they belong in system_service::start_service, which #2230 gives a backoff schedule and non-retryable-code classification. This PR only adds the missing restart call; #2230 makes starting reliable. No file overlap between the two.

Don't destroy before replacing — the macOS .app is moved aside in prepare and restored by a real rollback, and a backup left behind by an interrupted update is adopted rather than clobbered; the Standard updater honours the recorded executable_path; and seed_if_missing no longer seeds the last-known-good reserve from a binary that disagrees with the anchor (prerequisite for #1783, whose rollback trusts that anchor).

Durability & security — initial_config.json uses atomic_write; macOS preferences go through launchctl asuser (from a LaunchDaemon there's no session bootstrap, so defaults write failed and the GUI app launched with no serverUrl); archive extraction rejects absolute and .. entry paths, which could otherwise write anywhere as root.

Verification

Both Rust legs are green on 051613f11. Test Rust (windows-latest) is the one that matters — service.rs is #[cfg(windows)] and cannot be built on macOS (cross-compiling needs an MSVC toolchain for zstd-sys), so that job is the only real compile of the exit-code change; a red or skipped Windows check means it is unverified. Scan Code fails repo-wide (same on #2341) and is unrelated to this branch.

Compatible with the other open client PRs — merge order doesn't matter. Merged locally with #2230, #2231 and #2287: no conflicts, clippy clean, tests pass, and every side's logic survives byte-identically. Only #2287 shares a file (tool_agent_update_service.rs), and the resulting order is the right one — its downgrade guard refuses a stale message before this PR takes the tool lock.

Locally: lint clean, 195 tests (was 187) — new coverage for safe_join.

Still needs a real endpoint: wedge an agent.exe so it survives TerminateProcess and confirm the agent stays up; kill the service core and confirm SCM restarts it.

Risk & follow-ups

• The exit-code change alters restart semantics fleet-wide — a deterministically failing startup now restart-loops every 5 min instead of going quiet. Worth a deliberate yes/no given the LCW-PC-Test crash-loop history.
• Deliberately unchanged: run_tool is infallible by design, so an install can report success for a tool that never launches; a failed defaults write blocks the GuiApp launch entirely; install and uninstall still pair the updating flag by hand instead of using UpdatingGuard.
• Left to the PRs that own those files, to keep this one non-overlapping: the duplicate path resolver in ServiceToolUpdater (fix(client): flush the written binary and back off the Windows service start (os error 32) #2230) and sweeping stale .update-backup bundles on uninstall (fix(client): recover from a locked tool directory on reinstall #2231).
• Separate work: the crash-loop guard can't fire (record_boot_attempt skips when target == running); force reinstall silently mints a new machine identity, a likely source of the duplicate-host and 401 enrollment loops on this tenant.

[github-actions]

🦩 Flamingo Code Review

2 finding(s) — 0 action required · 2 recommended · 0 informational

Mode: advisory · 2 defect(s) outside any rule

Inline comments: 2 new

---

Need another pass? Commits pushed after this review are not reviewed automatically.

• Review the new commits — the commits added since this review
• Review the whole diff again — ignoring what was already reviewed

Prefer typing? Comment @flamingo-review, or @flamingo-review full. To review every push on this pull request, add the flamingo-review-always label.

React 👍/👎 on inline comments to teach the reviewer.

Started 2026-09-23 10:50 UTC · updated 2026-09-23 10:51 UTC · workflow run

[denys-gif]

please remove extensive comments or shorten them