From 98c45e9870c6c61c09c4186dc4ce3306641f5c36 Mon Sep 17 00:00:00 2001 From: Sagar Patil Date: Tue, 9 Jun 2026 13:18:04 -0700 Subject: [PATCH 1/3] Handle malformed/missing builds.json gracefully in refresh & validate_json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two scripts could crash with an unhandled exception (and a Python traceback) instead of the project's clean common.die()/exit-1 path when builds.json is malformed or missing — e.g. after a merge conflict or a hand-edit during a release. refresh.py: builds.load() / find_cli() sat one line above the try block whose `except ValueError` / `except OSError` handlers were clearly meant to cover them (JSONDecodeError is a ValueError subclass; FileNotFoundError an OSError). Move both calls inside the try so a bad builds.json is reported via common.die() instead of escaping as a raw traceback. validate_json.py: main() re-parsed builds.json/builds.schema.json with no guard. check_sorted_keys() catches JSONDecodeError but only logs and continues, so execution still reached the unguarded re-parse and crashed — ironic for the very tool meant to validate that file. Guard the parse and return 1 with a clean error. Co-Authored-By: Claude Fable 5 --- scripts/refresh.py | 4 ++-- scripts/validate_json.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/refresh.py b/scripts/refresh.py index 252c278..f329cb1 100644 --- a/scripts/refresh.py +++ b/scripts/refresh.py @@ -116,10 +116,10 @@ def main(argv: list[str] | None = None) -> int: common.preflight_checks(["git", "buildx"]) cli = args.stellar_cli_version - data = builds.load() - entry = builds.find_cli(data, cli) try: + data = builds.load() + entry = builds.find_cli(data, cli) if args.rust_versions: labels = [k.strip() for k in args.rust_versions.split(",") if k.strip()] common.log(f"rust base labels (from --rust-versions): {' '.join(labels)}") diff --git a/scripts/validate_json.py b/scripts/validate_json.py index c5338e4..a38abbe 100755 --- a/scripts/validate_json.py +++ b/scripts/validate_json.py @@ -90,8 +90,12 @@ def main(argv: list[str] | None = None) -> int: builds_path = root / "builds.json" schema_path = root / "builds.schema.json" - builds_data = json.loads(builds_path.read_text()) - schema = json.loads(schema_path.read_text()) + try: + builds_data = json.loads(builds_path.read_text()) + schema = json.loads(schema_path.read_text()) + except (OSError, json.JSONDecodeError) as exc: + common.err(f"could not load builds.json/builds.schema.json: {exc}") + return 1 ok &= check_schema(builds_data, schema) From 820b478a752da0756345153704283de8c019f26c Mon Sep 17 00:00:00 2001 From: Sagar Patil Date: Tue, 9 Jun 2026 13:21:48 -0700 Subject: [PATCH 2/3] Also catch UnicodeDecodeError when loading builds.json in validate_json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Path.read_text() raises UnicodeDecodeError on a non-UTF-8 file, which is a ValueError subclass and so was not caught by the (OSError, json.JSONDecodeError) guard — it would still crash with a traceback. Add it to the guard so a non-UTF-8 builds.json also exits 1 cleanly. (refresh.py is already covered: its handler is the broader `except ValueError`, of which UnicodeDecodeError is a subclass.) Addresses Copilot review feedback on PR #24. Co-Authored-By: Claude Fable 5 --- scripts/validate_json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/validate_json.py b/scripts/validate_json.py index a38abbe..8e12a35 100755 --- a/scripts/validate_json.py +++ b/scripts/validate_json.py @@ -93,7 +93,7 @@ def main(argv: list[str] | None = None) -> int: try: builds_data = json.loads(builds_path.read_text()) schema = json.loads(schema_path.read_text()) - except (OSError, json.JSONDecodeError) as exc: + except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: common.err(f"could not load builds.json/builds.schema.json: {exc}") return 1 From 78db2cb0141306419dc0a407b4b4caec08139597 Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Tue, 9 Jun 2026 17:01:08 -0700 Subject: [PATCH 3/3] Add tests for malformed and missing builds.json. --- tests/integration/test_refresh.py | 11 +++++++++++ tests/unit/test_validate_json.py | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/tests/integration/test_refresh.py b/tests/integration/test_refresh.py index 3abe4ab..c3dd7af 100644 --- a/tests/integration/test_refresh.py +++ b/tests/integration/test_refresh.py @@ -156,6 +156,17 @@ def test_creates_new_cli_entry(monkeypatch: pytest.MonkeyPatch, staged_minimal: assert [e["version"] for e in data["stellar_cli_versions"]] == ["26.0.0", "27.0.0"] +def test_malformed_builds_dies_cleanly(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + # A malformed builds.json must route through common.die() (SystemExit), + # not surface a raw JSONDecodeError from builds.load(). + target = tmp_path / "builds.json" + target.write_text("{ this is not valid json") + monkeypatch.setattr(builds, "DEFAULT_PATH", target) + monkeypatch.setattr(refresh.common, "preflight_checks", lambda _: None) + with pytest.raises(SystemExit): + refresh.main(["--stellar-cli-version", "26.0.0", "--rust-versions", "1.94.0-slim-trixie"]) + + def test_unresolvable_tag_dies(monkeypatch: pytest.MonkeyPatch, staged_minimal: Path) -> None: monkeypatch.setattr(refresh.common, "preflight_checks", lambda _: None) monkeypatch.setattr(refresh.git_remote, "resolve_tag_commit", lambda repo, tag: None) diff --git a/tests/unit/test_validate_json.py b/tests/unit/test_validate_json.py index d6e6dbc..a19e47f 100644 --- a/tests/unit/test_validate_json.py +++ b/tests/unit/test_validate_json.py @@ -91,3 +91,26 @@ def test_main_fails_on_invalid_entry( ) monkeypatch.setattr(validate_json.common, "repo_root", lambda: tmp_path) assert validate_json.main([]) == 1 + + +def test_main_returns_1_on_malformed_builds( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fixtures_dir: Path +) -> None: + # A malformed builds.json must exit 1 cleanly, not raise JSONDecodeError. + (tmp_path / "builds.json").write_text("{ this is not valid json") + (tmp_path / "builds.schema.json").write_text( + (fixtures_dir.parent.parent / "builds.schema.json").read_text() + ) + monkeypatch.setattr(validate_json.common, "repo_root", lambda: tmp_path) + assert validate_json.main([]) == 1 + + +def test_main_returns_1_on_missing_builds( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fixtures_dir: Path +) -> None: + # A missing builds.json must exit 1 cleanly, not raise FileNotFoundError. + (tmp_path / "builds.schema.json").write_text( + (fixtures_dir.parent.parent / "builds.schema.json").read_text() + ) + monkeypatch.setattr(validate_json.common, "repo_root", lambda: tmp_path) + assert validate_json.main([]) == 1