fix(self-update): stop a non-elevated repair from fabricating a broker failure#461
Merged
Merged
Conversation
…r failure `uffs --update repair` (which runs NON-elevated by design — the whole point of the Access Broker) reported `[FAIL] Service restart failed: broker` while the very same report showed `[OK] Broker pipe serving`. The broker was healthy the entire time (confirmed: `uffs-broker --status` → running, serving). Root cause: the broker runs as a LocalSystem service. A non-elevated uffs-update cannot `OpenProcess` a SYSTEM-owned pid, so `proc::is_alive(pid)` false-negatives on it → the broker is flagged "down" → `restore` tries to `uffs_winsvc::start` it → that ALSO needs elevation → fails → a hard FAIL (exit 1) for a perfectly running broker. Two surgical fixes: - doctor `check_services`: determine the broker's liveness from its serving pipe (the authoritative signal, same probe `check_broker` uses), not from pid `OpenProcess` liveness. So a serving broker reads as "Service up", never enters the down/restart path. - restore `start_broker`: make it idempotent — if the broker is already serving, return success without an elevation-gated `uffs_winsvc::start`. This also covers the quiesce→restore path. Net: a non-elevated repair/restore no longer invents a broker failure when the broker is up; it only acts (and only warns, never hard-fails non-elevated) when the pipe is genuinely not serving. Verified: builds + 46 tests green, clippy clean, windows-msvc cross-compile OK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
uffs --update repair(run non-elevated — by design; the Access Broker exists precisely so the daemon/updater run without UAC) reports a hard failure for a broker that is perfectly healthy:Confirmed healthy out-of-band:
uffs-broker --status→State: running, PID 1536, Pipe: serving.Root cause
The broker runs as a LocalSystem service. A non-elevated
uffs-updatecan'tOpenProcessa SYSTEM-owned PID, soproc::is_alive(pid)false-negatives → the broker is flagged "down" →restoretries touffs_winsvc::startit → that also needs elevation → fails → hard[FAIL]. The broker was up the whole time.Fix (two surgical changes)
check_services: judge the broker's liveness by its serving pipe (the authoritative signalcheck_brokeralready uses), not by PIDOpenProcessliveness. A serving broker readsService up: brokerand never enters the down/restart path.start_broker: make it idempotent — already serving ⇒ return success without an elevation-gateduffs_winsvc::start. Also hardens the quiesce→restore path.Net: a non-elevated repair/restore no longer invents a broker failure when the broker is up; it only acts when the pipe is genuinely not serving.
Verification
Builds + 46
uffs-updatetests green, clippy clean,x86_64-pc-windows-msvccross-compile OK.🤖 Generated with Claude Code