From b85fff9495ad95a829c0fb8b76ea773cd98d10fc Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Tue, 28 Apr 2026 12:13:06 -0700 Subject: [PATCH] fix(update): protect dashboard wmic scan against UnicodeDecodeError on Windows non-UTF-8 locales (#17049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes update` calls `_warn_stale_dashboard_processes()` to warn about dashboard processes still running the pre-update Python backend. On Windows, that scan shells out to `wmic process get ProcessId,CommandLine /FORMAT:LIST` with `text=True` and no explicit encoding. `wmic` emits text in the system code page (e.g. cp936 on zh-CN locales), not UTF-8. Without an explicit `encoding=`, Python's default UTF-8 decoder crashes the subprocess reader thread with `UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd0 ...`. In Python 3.11 that crash is silently absorbed: `subprocess.run()` returns a `CompletedProcess` with `result.stdout = None`, the next line calls `result.stdout.split("\n")`, and `hermes update` aborts with the exact `AttributeError: 'NoneType' object has no attribute 'split'` trace reported in #17049. Fix: pass `encoding="utf-8", errors="ignore"` so undecodable bytes cannot take down the reader thread (the parsing only matches the ASCII prefixes `CommandLine=` and `ProcessId=`, so dropping non-UTF-8 bytes is safe), and short-circuit when `result.stdout is None` as a defensive guard for environments where the reader thread still fails for other reasons. This is the same root cause as #17074 (which patches `hermes_cli/gateway._scan_gateway_pids` for the `hermes setup` path). That PR does not touch `_warn_stale_dashboard_processes`, so `hermes update` remains broken on the same locales until this lands. Regression test in `tests/hermes_cli/test_update_stale_dashboard.py`: - `test_wmic_invoked_with_utf8_ignore_errors` asserts the explicit encoding/errors kwargs reach `subprocess.run`. - `test_wmic_returns_none_stdout_does_not_crash` simulates the reader-thread-crashed `result.stdout=None` aftermath and asserts the function returns silently instead of raising AttributeError. Both new tests fail against clean origin/main (7d4648461) reproducing the original AttributeError; both pass with this patch. The remaining 3 failures in `tests/hermes_cli/test_cmd_update.py` and `test_update_autostash.py` are pre-existing baselines on origin/main — they reproduce identically without this change and are unrelated to the wmic scan. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/main.py | 9 +++- .../hermes_cli/test_update_stale_dashboard.py | 54 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index ba526354..c56fc1e3 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5274,12 +5274,19 @@ def _warn_stale_dashboard_processes() -> None: try: if sys.platform == "win32": + # wmic emits text in the system code page (e.g. cp936 on zh-CN + # locales), not UTF-8. Without an explicit encoding, Python's + # default UTF-8 decoder crashes the subprocess reader thread + # with UnicodeDecodeError, leaving result.stdout=None and the + # subsequent .split() call crashing `hermes update` with an + # AttributeError on Windows non-UTF-8 locales (#17049). result = subprocess.run( ["wmic", "process", "get", "ProcessId,CommandLine", "/FORMAT:LIST"], capture_output=True, text=True, timeout=10, + encoding="utf-8", errors="ignore", ) - if result.returncode != 0: + if result.returncode != 0 or result.stdout is None: return current_cmd = "" for line in result.stdout.split("\n"): diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index 20c5eee9..b36f9e5a 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -175,3 +175,57 @@ class TestWarnStaleDashboardProcesses: output = capsys.readouterr().out assert "PID 99999" not in output assert "PID 12345" in output + + +class TestWindowsWmicEncoding: + """Regression tests for #17049 — the Windows wmic branch must not crash + `hermes update` on non-UTF-8 system locales (e.g. cp936 on zh-CN). + """ + + def test_wmic_invoked_with_utf8_ignore_errors(self): + """The wmic subprocess.run call must pass encoding='utf-8' and + errors='ignore' so the subprocess reader thread cannot raise + UnicodeDecodeError on non-UTF-8 wmic output.""" + with patch("hermes_cli.main.sys") as mock_sys, \ + patch("subprocess.run") as mock_run: + mock_sys.platform = "win32" + # Provide a minimal valid wmic /FORMAT:LIST response. + mock_run.return_value = MagicMock( + returncode=0, + stdout=( + "CommandLine=python -m hermes_cli.main dashboard\n" + "ProcessId=12345\n" + ), + stderr="", + ) + _warn_stale_dashboard_processes() + + assert mock_run.called, "subprocess.run was not invoked" + kwargs = mock_run.call_args.kwargs + assert kwargs.get("encoding") == "utf-8", ( + "encoding kwarg must be 'utf-8' so wmic output is decoded " + "deterministically rather than via the implicit reader-thread " + "default that crashes on non-UTF-8 locales (#17049)." + ) + assert kwargs.get("errors") == "ignore", ( + "errors kwarg must be 'ignore' so undecodable bytes don't take " + "down the reader thread (#17049)." + ) + + def test_wmic_returns_none_stdout_does_not_crash(self, capsys): + """If subprocess.run returns successfully but stdout is None — which + is what Python 3.11 leaves behind when the reader thread silently + crashed on UnicodeDecodeError before this fix landed — the warning + must short-circuit instead of raising AttributeError on + ``None.split('\\n')`` and aborting `hermes update` (#17049).""" + with patch("hermes_cli.main.sys") as mock_sys, \ + patch("subprocess.run") as mock_run: + mock_sys.platform = "win32" + mock_run.return_value = MagicMock( + returncode=0, stdout=None, stderr="" + ) + # Must not raise. + _warn_stale_dashboard_processes() + + output = capsys.readouterr().out + assert "dashboard process" not in output