From 3f78f172d9f9fe1a9422334c5ae03cd5ac685ebd Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Wed, 16 Sep 2026 18:43:23 +0300 Subject: fix: address release promotion review findings --- scripts/changelog.py | 10 ++++++---- scripts/release-workflows.test.ts | 15 +++++++++++++++ scripts/release.py | 4 ++-- scripts/test_changelog.py | 11 +++++++++-- scripts/test_release.py | 15 +++++++++++++++ 5 files changed, 47 insertions(+), 8 deletions(-) (limited to 'scripts') diff --git a/scripts/changelog.py b/scripts/changelog.py index cc41c4f3..71f7969f 100644 --- a/scripts/changelog.py +++ b/scripts/changelog.py @@ -374,7 +374,8 @@ def default_release_assets(version: str, repo: str = DEFAULT_RELEASE_REPO) -> di def manifest_from_release_payload( - payload: dict[str, Any], version: str, protocol: int | None = None + payload: dict[str, Any], version: str, protocol: int | None = None, + endpoint_generation: int | None = None, ) -> dict[str, Any]: normalized_version = normalize_version(version) tag_name = str(payload.get("tagName") or "") @@ -427,7 +428,7 @@ def manifest_from_release_payload( return { "version": normalized_version, "protocol": protocol if protocol is not None else read_protocol_version(), - "endpoint_generation": read_endpoint_protocol_generation(), + "endpoint_generation": endpoint_generation if endpoint_generation is not None else read_endpoint_protocol_generation(), "notes": notes, "assets": manifest_assets, "sha256": manifest_sha256, @@ -705,7 +706,7 @@ def cmd_sync_latest_json(args: argparse.Namespace) -> int: ensure_manifest_is_outdated(current_manifest, version) release_payload = fetch_release_payload(version, args.repo) - new_manifest = manifest_from_release_payload(release_payload, version, args.protocol) + new_manifest = manifest_from_release_payload(release_payload, version, args.protocol, args.endpoint_generation) announcement_path = Path(args.announcement) announcement = load_product_announcement(announcement_path) output = build_latest_json( @@ -756,7 +757,7 @@ def cmd_validate_product_announcement(args: argparse.Namespace) -> int: def cmd_verify_release_state(args: argparse.Namespace) -> int: version = normalize_version(args.version) release_payload = fetch_release_payload(version, args.repo) - expected_manifest = manifest_from_release_payload(release_payload, version, args.protocol) + expected_manifest = manifest_from_release_payload(release_payload, version, args.protocol, args.endpoint_generation) local_raw_manifest = load_json(Path(args.output)) local_manifest = ensure_manifest_matches_expected( @@ -832,6 +833,7 @@ def build_parser() -> argparse.ArgumentParser: verify_release_state.add_argument("--output", default=str(DEFAULT_LATEST_JSON_PATH)) verify_release_state.add_argument("--live-url", default=DEFAULT_LIVE_MANIFEST_URL) verify_release_state.add_argument("--protocol", type=int) + verify_release_state.add_argument("--endpoint-generation", type=int) verify_release_state.set_defaults(func=cmd_verify_release_state) return parser diff --git a/scripts/release-workflows.test.ts b/scripts/release-workflows.test.ts index a169204f..75097710 100644 --- a/scripts/release-workflows.test.ts +++ b/scripts/release-workflows.test.ts @@ -33,6 +33,21 @@ describe("official publishing workflow boundaries", () => { expect(adminGate.run).not.toContain("ogulcancelik"); }); + test("release arguments are not interpolated into executable shell text", () => { + const input = `untrusted'\"$(echo unexpected-command)`; + for (const args of [ + ["preview", input], + ["release-prepare", input, input], + ["release-publish", input, input], + ["release", input, input], + ]) { + const result = spawnSync("just", ["--dry-run", ...args], { encoding: "utf8" }); + expect(result.status).toBe(0); + expect(result.stdout + result.stderr).not.toContain(input); + expect(result.stdout + result.stderr).not.toContain("unexpected-command"); + } + }); + test.skipIf(process.platform === "win32")("admin gate permits admins and fails closed for other roles or API errors", () => { const dir = mkdtempSync("/var/tmp/herdr-admin-gate-"); try { diff --git a/scripts/release.py b/scripts/release.py index b02c89cf..db9bd3fc 100644 --- a/scripts/release.py +++ b/scripts/release.py @@ -98,8 +98,8 @@ def validate_diff(preview: str, candidate: str, version: str | None = None) -> N raise ValueError(f"unpreviewed change: {path}; publish a new preview first") # Documentation exceptions must not turn into symlinks or submodules. entry = git("ls-tree", candidate, "--", path) - if entry and not entry.startswith("100644 blob "): - raise ValueError(f"release preparation must use regular files: {path}") + if (not entry and path in RELEASE_FILES) or (entry and not entry.startswith("100644 blob ")): + raise ValueError(f"release preparation must preserve regular files: {path}") versions = [normalized_cargo(git("show", f"{candidate}:{path}"), path)[1] for path in ("Cargo.toml", "Cargo.lock")] if versions[0] != versions[1] or (version is not None and versions[0] != version): diff --git a/scripts/test_changelog.py b/scripts/test_changelog.py index 07897c25..2a24b241 100644 --- a/scripts/test_changelog.py +++ b/scripts/test_changelog.py @@ -9,6 +9,7 @@ from scripts.changelog import ( ChangelogError, archived_releases_from_current_manifest, build_latest_json, + build_parser, canonicalize_manifest, DEFAULT_PRODUCT_ANNOUNCEMENT_PATH, default_release_assets, @@ -368,7 +369,11 @@ class ChangelogScriptTests(unittest.TestCase): }, ) - def test_manifest_from_release_payload_uses_explicit_protocol(self) -> None: + def test_manifest_from_release_payload_uses_explicit_compatibility_versions(self) -> None: + args = build_parser().parse_args([ + "verify-release-state", "--version", "0.1.1", + "--protocol", "42", "--endpoint-generation", "7", + ]) manifest = manifest_from_release_payload( { "tagName": "v0.1.1", @@ -378,10 +383,12 @@ class ChangelogScriptTests(unittest.TestCase): "assets": release_assets_with_digests(), }, "0.1.1", - protocol=42, + protocol=args.protocol, + endpoint_generation=args.endpoint_generation, ) self.assertEqual(manifest["protocol"], 42) + self.assertEqual(manifest["endpoint_generation"], 7) def test_manifest_from_release_payload_rejects_missing_digest(self) -> None: assets = release_assets_with_digests() diff --git a/scripts/test_release.py b/scripts/test_release.py index ba5cfb85..f495c89c 100644 --- a/scripts/test_release.py +++ b/scripts/test_release.py @@ -22,6 +22,9 @@ class ReleaseTests(unittest.TestCase): self.put("Cargo.toml", '[package]\nname = "herdr"\nversion = "1.0.0"\n[dependencies]\nserde = "1"\n') self.put("Cargo.lock", 'version = 4\n[[package]]\nname = "herdr"\nversion = "1.0.0"\n[[package]]\nname = "serde"\nversion = "1.0.0"\n') self.put("src/main.rs", "fn main() {}\n") + self.put("docs/next/CHANGELOG.md", "# Changelog\n") + self.put("skills/herdr/SKILL.md", "Stable skill\n") + self.put("docs/next/website/src/content/docs/obsolete.mdx", "Old page\n") self.put("distribution/latest.json", json.dumps({"version": "1.0.0"})) self.put("scripts/release.py", "# promotion tooling\n") self.put(".github/workflows/release.yml", "run: python3 scripts/release.py check-tag\n") @@ -94,6 +97,18 @@ class ReleaseTests(unittest.TestCase): with self.assertRaisesRegex(ValueError, "regular files"): release.validate_diff(self.preview, self.commit("symlink")) + def test_required_release_files_cannot_be_deleted_but_website_pages_can(self): + for path in ("docs/next/CHANGELOG.md", "skills/herdr/SKILL.md"): + with self.subTest(path=path): + self.git("reset", "--hard", self.preview) + self.git("rm", path) + candidate = self.commit("delete required release file") + with self.assertRaisesRegex(ValueError, "regular files"): + release.validate_diff(self.preview, candidate) + self.git("reset", "--hard", self.preview) + self.git("rm", "docs/next/website/src/content/docs/obsolete.mdx") + release.validate_diff(self.preview, self.commit("remove obsolete website page")) + def test_preview_source_accepts_master_history_and_rejects_unpublished_branch(self): self.assertEqual(release.select_preview(self.preview), self.preview) self.assertEqual(release.select_preview("v1.0.0"), self.git("rev-parse", "v1.0.0")) -- cgit v1.2.3