From 9755a67014e4b7b7d5221752bde1f695ae62501b Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Thu, 22 Oct 2026 00:31:00 -0400 Subject: [PATCH] fix(update): tolerate a corrupt worker_pid in the in-progress guard _update_is_stale coerced the stored worker_pid with int() and no guard, so a status file carrying a non-numeric value raised out of the "an update is already in progress" check in start_update: ValueError: invalid literal for int() with base 10: 'abc' The function exists so that a running status left behind by a dead worker does not block every later update, which is exactly the situation where the status file is least trustworthy. read_update_status above it already tolerates a missing file, an OSError, invalid JSON and a non-dict payload, so the module validated the container but not the fields it then coerced. Fall through to the timestamp check when the pid will not parse. Reporting the run stale instead would unblock an update that may still be running, which is the opposite of what the guard is for: a corrupt pid is no evidence the worker died. A fresh run therefore still blocks, and an old one is still reported stale. Co-Authored-By: Claude Opus 5 --- cli/update_service.py | 8 +++++++- tests/test_web_console_update.py | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/cli/update_service.py b/cli/update_service.py index 18880c62..e9ff3478 100644 --- a/cli/update_service.py +++ b/cli/update_service.py @@ -403,7 +403,13 @@ def _worker_alive(pid: int) -> bool: def _update_is_stale(status: Dict[str, Any]) -> bool: """A running/restarting status whose worker is gone (killed, OOM, reboot) would otherwise block every later update.""" - worker_pid = int(status.get("worker_pid") or 0) + try: + worker_pid = int(status.get("worker_pid") or 0) + except (TypeError, ValueError): + # A corrupt pid tells us nothing about the worker, so fall through to the + # timestamp check instead of raising. Reporting the run stale here would + # unblock an update that may still be running. + worker_pid = 0 if worker_pid and not _worker_alive(worker_pid): return True started = status.get("started_at") or status.get("updated_at") diff --git a/tests/test_web_console_update.py b/tests/test_web_console_update.py index a3b3b7c5..55e41e72 100644 --- a/tests/test_web_console_update.py +++ b/tests/test_web_console_update.py @@ -264,6 +264,30 @@ def test_a_running_status_left_by_a_dead_worker_does_not_block(tmp_path, monkeyp ) is True +def test_a_corrupt_worker_pid_does_not_crash_the_in_progress_guard(tmp_path, monkeypatch): + """A status file carrying a non-numeric worker_pid must not raise. + + read_update_status already tolerates a missing file, unreadable bytes, invalid + JSON and a non-dict payload, but it does not validate the fields it hands on. + int() on a corrupt pid raised ValueError out of the "already in progress" check. + """ + from cli import update_service + + monkeypatch.setattr(update_service, "_worker_alive", lambda pid: True) + fresh = update_service.datetime.now(update_service.timezone.utc).isoformat() + + for bad in ("abc", "", "12abc", [], {}, 1.5e3): + status = {"state": "running", "worker_pid": bad, "started_at": fresh} + # A corrupt pid is no evidence the worker died, so a fresh run still blocks. + assert update_service._update_is_stale(status) is False, bad + + # ... and an old run is still reported stale rather than blocking forever. + for bad in ("abc", [], {}): + assert update_service._update_is_stale( + {"state": "running", "worker_pid": bad, "started_at": "2020-01-01T00:00:00+00:00"} + ) is True, bad + + def test_web_handlers_auth_and_no_github_on_version(monkeypatch): if "web" not in sys.modules: web_stub = types.ModuleType("web")