From 51e1fb8fb92b490394427f18c36f4266c66457b8 Mon Sep 17 00:00:00 2001 From: Paulo Nascimento Date: Fri, 17 Jul 2026 20:17:36 -0400 Subject: [PATCH] fix(cron): accept UTF-8 BOM when reading jobs.json Windows Notepad and PowerShell 5.1 Set-Content -Encoding UTF8 write a leading UTF-8 BOM. json.load under encoding=utf-8 raises JSONDecodeError("Unexpected UTF-8 BOM"), and load_jobs wraps that as RuntimeError("Cron database corrupted and unrepairable"), taking down cron CRUD/scheduler for a hand-edited jobs.json. Read with utf-8-sig on all four independent jobs.json readers (load_jobs primary + strict=False repair, dump _cron_summary, status Scheduled Jobs). Write path stays plain utf-8 so the next save_jobs heals a BOM'd file. Matches the env-class dialect (#65123). Tests: BOM load (crash repro), bomless regression, empty store, BOM+bare-list auto-repair, BOM+control-char strict=False arm, dump and status CLI readers. --- cron/jobs.py | 7 +- hermes_cli/dump.py | 4 +- hermes_cli/status.py | 4 +- tests/cron/test_jobs.py | 117 ++++++++++++++++++++ tests/hermes_cli/test_jobs_json_utf8_bom.py | 68 ++++++++++++ 5 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 tests/hermes_cli/test_jobs_json_utf8_bom.py diff --git a/cron/jobs.py b/cron/jobs.py index 6c5222110..7ecd17abb 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -865,13 +865,16 @@ def load_jobs() -> List[Dict[str, Any]]: _strict_retry = False # track whether we used the strict=False fallback try: - with open(jobs_file, 'r', encoding='utf-8') as f: + # utf-8-sig: Windows Notepad / PowerShell 5.1 Set-Content -Encoding UTF8 + # write a leading BOM; json.load under plain utf-8 raises + # JSONDecodeError("Unexpected UTF-8 BOM") and takes down cron. + with open(jobs_file, 'r', encoding='utf-8-sig') as f: data = json.load(f) except json.JSONDecodeError: # Retry with strict=False to handle bare control chars in string values _strict_retry = True try: - with open(jobs_file, 'r', encoding='utf-8') as f: + with open(jobs_file, 'r', encoding='utf-8-sig') as f: data = json.loads(f.read(), strict=False) except Exception as e: logger.error("Failed to auto-repair jobs.json: %s", e) diff --git a/hermes_cli/dump.py b/hermes_cli/dump.py index 8f992e928..139be0b59 100644 --- a/hermes_cli/dump.py +++ b/hermes_cli/dump.py @@ -167,7 +167,9 @@ def _cron_summary(hermes_home: Path) -> str: if not jobs_file.exists(): return "0" try: - with open(jobs_file, encoding="utf-8") as f: + # utf-8-sig: same dialect as cron/jobs.load_jobs — Windows editors + # may leave a UTF-8 BOM that plain utf-8 json.load rejects. + with open(jobs_file, encoding="utf-8-sig") as f: data = json.load(f) jobs = data.get("jobs", []) active = sum(1 for j in jobs if j.get("enabled", True)) diff --git a/hermes_cli/status.py b/hermes_cli/status.py index dc58acaa6..7537b85a1 100644 --- a/hermes_cli/status.py +++ b/hermes_cli/status.py @@ -529,7 +529,9 @@ def show_status(args): if jobs_file.exists(): import json try: - with open(jobs_file, encoding="utf-8") as f: + # utf-8-sig: same dialect as cron/jobs.load_jobs — Windows editors + # may leave a UTF-8 BOM that plain utf-8 json.load rejects. + with open(jobs_file, encoding="utf-8-sig") as f: data = json.load(f) jobs = data.get("jobs", []) enabled_jobs = [j for j in jobs if j.get("enabled", True)] diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index f965428ee..09fc7d2fa 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -2046,3 +2046,120 @@ class TestLateEnvRepointScopesStore: assert new_file.is_file() # ...and the import-time file is byte-identical to the sentinel. assert old_file.read_text(encoding="utf-8") == sentinel + + +# ========================================================================= +# UTF-8 BOM on jobs.json (Windows Notepad / PowerShell 5.1) +# ========================================================================= + +class TestJobsJsonUtf8Bom: + """jobs.json readers must accept a leading UTF-8 BOM. + + Matching the env-class dialect (utf-8-sig): a BOM from Windows editors + must not raise JSONDecodeError / RuntimeError on load_jobs(). + """ + + def test_load_jobs_accepts_utf8_bom(self, tmp_cron_dir): + """BOM'd jobs.json loads — the pre-fix crash repro.""" + import json + from pathlib import Path + from cron.jobs import JOBS_FILE, load_jobs + + payload = { + "jobs": [ + { + "id": "bomjob01", + "name": "bom-test", + "enabled": True, + "prompt": "hello", + "schedule": {"kind": "interval", "minutes": 60, "display": "every 60m"}, + } + ] + } + JOBS_FILE.parent.mkdir(parents=True, exist_ok=True) + JOBS_FILE.write_bytes( + b"\xef\xbb\xbf" + json.dumps(payload).encode("utf-8") + ) + + loaded = load_jobs() + assert [j["id"] for j in loaded] == ["bomjob01"] + assert loaded[0]["name"] == "bom-test" + + def test_load_jobs_bomless_regression(self, tmp_cron_dir): + """BOM-less UTF-8 jobs.json must keep loading after utf-8-sig.""" + import json + from cron.jobs import JOBS_FILE, load_jobs + + payload = { + "jobs": [ + { + "id": "plainjob01", + "name": "plain", + "enabled": True, + "prompt": "hi", + "schedule": {"kind": "interval", "minutes": 30, "display": "every 30m"}, + } + ] + } + JOBS_FILE.parent.mkdir(parents=True, exist_ok=True) + JOBS_FILE.write_text(json.dumps(payload), encoding="utf-8") + + loaded = load_jobs() + assert [j["id"] for j in loaded] == ["plainjob01"] + + def test_load_jobs_bom_empty_jobs_list(self, tmp_cron_dir): + """Minimal BOM'd store ({"jobs": []}) must not raise.""" + from cron.jobs import JOBS_FILE, load_jobs + + JOBS_FILE.parent.mkdir(parents=True, exist_ok=True) + JOBS_FILE.write_bytes(b'\xef\xbb\xbf{"jobs": []}') + + assert load_jobs() == [] + + def test_load_jobs_bom_plus_bare_list_auto_repairs(self, tmp_cron_dir): + """BOM + bare list (hand-edited) must load and rewrap to dict envelope.""" + import json + from cron.jobs import JOBS_FILE, load_jobs + + bare = [ + { + "id": "barebom01", + "name": "bare", + "enabled": True, + "prompt": "x", + "schedule": {"kind": "interval", "minutes": 60, "display": "every 60m"}, + } + ] + JOBS_FILE.parent.mkdir(parents=True, exist_ok=True) + JOBS_FILE.write_bytes(b"\xef\xbb\xbf" + json.dumps(bare).encode("utf-8")) + + loaded = load_jobs() + assert [j["id"] for j in loaded] == ["barebom01"] + # Auto-repair rewrites via save_jobs (plain utf-8) — heals the BOM. + on_disk = JOBS_FILE.read_bytes() + assert not on_disk.startswith(b"\xef\xbb\xbf"), "save_jobs should rewrite without BOM" + rewritten = json.loads(on_disk.decode("utf-8")) + assert isinstance(rewritten, dict) + assert [j["id"] for j in rewritten["jobs"]] == ["barebom01"] + + def test_load_jobs_bom_plus_control_char_uses_strict_false_arm(self, tmp_cron_dir): + """BOM + bare control char in a string value exercises the strict=False arm. + + json.load (strict) rejects unescaped control chars; the retry path must + also open with utf-8-sig so the BOM does not re-crash the repair. + """ + from cron.jobs import JOBS_FILE, load_jobs + + # Valid JSON structure but with a raw newline inside a string value + # (invalid under strict=True). Leading UTF-8 BOM. + raw = ( + b'\xef\xbb\xbf{"jobs": [{"id": "ctrlbom01", "name": "has\nnewline",' + b' "enabled": true, "prompt": "x",' + b' "schedule": {"kind": "interval", "minutes": 60, "display": "every 60m"}}]}' + ) + JOBS_FILE.parent.mkdir(parents=True, exist_ok=True) + JOBS_FILE.write_bytes(raw) + + loaded = load_jobs() + assert [j["id"] for j in loaded] == ["ctrlbom01"] + assert "newline" in loaded[0]["name"] diff --git a/tests/hermes_cli/test_jobs_json_utf8_bom.py b/tests/hermes_cli/test_jobs_json_utf8_bom.py new file mode 100644 index 000000000..4efe45f66 --- /dev/null +++ b/tests/hermes_cli/test_jobs_json_utf8_bom.py @@ -0,0 +1,68 @@ +"""UTF-8 BOM tolerance for independent jobs.json readers (dump/status). + +cron/jobs.load_jobs is covered in tests/cron/test_jobs.py. dump and status +each open jobs.json themselves — keep them on the same utf-8-sig dialect. +""" + +from types import SimpleNamespace + + +def test_dump_cron_summary_accepts_utf8_bom(tmp_path): + from hermes_cli.dump import _cron_summary + + cron = tmp_path / "cron" + cron.mkdir() + (cron / "jobs.json").write_bytes( + b'\xef\xbb\xbf{"jobs": [{"id": "j1", "enabled": true},' + b' {"id": "j2", "enabled": false}]}' + ) + + assert _cron_summary(tmp_path) == "1 active / 2 total" + + +def test_dump_cron_summary_bomless_regression(tmp_path): + from hermes_cli.dump import _cron_summary + + cron = tmp_path / "cron" + cron.mkdir() + (cron / "jobs.json").write_text( + '{"jobs": [{"id": "j1", "enabled": true}]}', + encoding="utf-8", + ) + + assert _cron_summary(tmp_path) == "1 active / 1 total" + + +def test_status_scheduled_jobs_accepts_utf8_bom(monkeypatch, capsys, tmp_path): + """hermes status must not print '(error reading jobs file)' under BOM.""" + from hermes_cli import status as status_mod + import hermes_cli.auth as auth_mod + import hermes_cli.gateway as gateway_mod + + cron = tmp_path / "cron" + cron.mkdir() + (cron / "jobs.json").write_bytes( + b'\xef\xbb\xbf{"jobs": [{"id": "j1", "enabled": true},' + b' {"id": "j2", "enabled": true}]}' + ) + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(status_mod, "get_env_path", lambda: tmp_path / ".env", raising=False) + monkeypatch.setattr(status_mod, "get_hermes_home", lambda: tmp_path, raising=False) + monkeypatch.setattr(status_mod, "load_config", lambda: {"model": "gpt-5.4"}, raising=False) + monkeypatch.setattr( + status_mod, "resolve_requested_provider", lambda requested=None: "openai-codex", raising=False + ) + monkeypatch.setattr( + status_mod, "resolve_provider", lambda requested=None, **kwargs: "openai-codex", raising=False + ) + monkeypatch.setattr(status_mod, "provider_label", lambda provider: "OpenAI Codex", raising=False) + monkeypatch.setattr(auth_mod, "get_nous_auth_status", lambda: {}, raising=False) + monkeypatch.setattr(auth_mod, "get_codex_auth_status", lambda: {}, raising=False) + monkeypatch.setattr(auth_mod, "get_xai_oauth_auth_status", lambda: {}, raising=False) + monkeypatch.setattr(gateway_mod, "find_gateway_pids", lambda exclude_pids=None: [], raising=False) + + status_mod.show_status(SimpleNamespace(all=False, deep=False)) + out = capsys.readouterr().out + assert "(error reading jobs file)" not in out + assert "2 active, 2 total" in out