From edd0df9084eb8446e586305b68a4d62202278afd Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Thu, 24 Sep 2026 18:35:17 +0000 Subject: [PATCH] Reuse verified V8 checksum manifests from the artifact cache (#47915) ## Why V8 artifact fetching always downloaded the checksum manifest, requiring network access even when all cached files were valid. ## What changed Reuse a cached manifest when it is not a symlink and matches the checkout's trusted SHA-256 pin. Continue verifying both the archive and bindings against that manifest, downloading replacements when needed. Refresh missing, invalid, or symlinked manifests. Preserve existing cache contents if the refresh is interrupted, and reject downloaded manifests that do not match the current pin. ## Testing Add regression coverage for network-free cache reuse, manifest refresh, corrupt artifact repair, changed pins, symlink replacement, interrupted downloads, and source or paired artifact overrides. All 12 tests in `scripts/codex_package/test_v8.py` pass. GitOrigin-RevId: d3bdb1ae5f6bc48dd6d081df33dec8db9d3257a8 --- scripts/codex_package/test_v8.py | 160 +++++++++++++++++++++++++++++++ scripts/codex_package/v8.py | 23 ++++- 2 files changed, 179 insertions(+), 4 deletions(-) diff --git a/scripts/codex_package/test_v8.py b/scripts/codex_package/test_v8.py index a47f15a348..e7bd2033ab 100644 --- a/scripts/codex_package/test_v8.py +++ b/scripts/codex_package/test_v8.py @@ -1,4 +1,5 @@ import hashlib +import io import sys import tempfile import unittest @@ -147,6 +148,165 @@ class FetchCodexV8ArtifactsTest(unittest.TestCase): download.assert_called_once() self.assertEqual(download.call_args.args[1].name, manifest_name) + def test_verified_cache_needs_no_network(self) -> None: + with self.release("x86_64-unknown-linux-gnu") as (spec, download, _): + first = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + download.reset_mock() + download.side_effect = AssertionError("unexpected network request") + + second = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + self.assertEqual(second, first) + download.assert_not_called() + + def test_missing_or_corrupt_manifest_refresh_retains_valid_artifacts(self) -> None: + with self.release("x86_64-unknown-linux-gnu") as (spec, download, name): + artifacts = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + manifest = artifacts.archive.parent / name + expected = manifest.read_bytes() + for contents in (None, b"corrupt"): + with self.subTest(contents=contents): + if contents is None: + manifest.unlink() + else: + manifest.write_bytes(contents) + download.reset_mock() + refreshed = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + self.assertEqual(refreshed, artifacts) + self.assertEqual(manifest.read_bytes(), expected) + download.assert_called_once() + self.assertEqual(download.call_args.args[1], manifest) + + def test_cached_manifest_still_authenticates_both_artifacts(self) -> None: + with self.release("x86_64-unknown-linux-gnu") as (spec, download, _): + artifacts = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + artifacts.archive.write_bytes(b"corrupt archive") + artifacts.binding.write_bytes(b"corrupt binding") + download.reset_mock() + repaired = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + self.assertEqual(repaired, artifacts) + self.assertEqual( + (repaired.archive.read_bytes(), repaired.binding.read_bytes()), + (b"trusted V8 archive", b"trusted V8 binding"), + ) + self.assertEqual( + [call.args[1] for call in download.call_args_list], + [artifacts.archive, artifacts.binding], + ) + + def test_changed_repository_pin_rejects_old_cache_and_bad_refresh(self) -> None: + with self.release("x86_64-unknown-linux-gnu") as (spec, download, name): + artifacts = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + pins = ( + self.root / "third_party/v8/rusty_v8_150_4_0_release_manifests.sha256" + ) + pins.write_text(f"{'0' * 64} {name}\n") + download.reset_mock() + with self.assertRaisesRegex( + RuntimeError, "does not match its trusted SHA-256" + ): + v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + download.assert_called_once() + self.assertEqual( + download.call_args.args[1], artifacts.archive.parent / name + ) + self.assertFalse((artifacts.archive.parent / name).exists()) + self.assertEqual(artifacts.archive.read_bytes(), b"trusted V8 archive") + self.assertEqual(artifacts.binding.read_bytes(), b"trusted V8 binding") + + @unittest.skipIf(sys.platform == "win32", "requires unprivileged symlinks") + def test_symlink_manifest_is_replaced_without_writing_through_link(self) -> None: + actual_download = v8.download_file + with self.release("x86_64-unknown-linux-gnu") as (spec, download, name): + artifacts = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + manifest = artifacts.archive.parent / name + expected = manifest.read_bytes() + backing = manifest.with_suffix(".original") + manifest.rename(backing) + manifest.symlink_to(backing) + download.reset_mock() + download.side_effect = actual_download + with patch.object(v8, "urlopen", return_value=io.BytesIO(expected)): + refreshed = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + self.assertEqual(refreshed, artifacts) + self.assertFalse(manifest.is_symlink()) + self.assertEqual( + (manifest.read_bytes(), backing.read_bytes()), (expected, expected) + ) + download.assert_called_once() + + def test_interrupted_refresh_preserves_cache(self) -> None: + actual_download = v8.download_file + + class InterruptedResponse(io.BytesIO): + def read(self, size=-1): + if self.tell(): + raise OSError("interrupted download") + return super().read(size) + + with self.release("x86_64-unknown-linux-gnu") as (spec, download, name): + artifacts = v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + manifest = artifacts.archive.parent / name + manifest.write_bytes(b"stale manifest") + download.reset_mock() + download.side_effect = actual_download + with ( + patch.object( + v8, "urlopen", return_value=InterruptedResponse(b"partial") + ), + self.assertRaisesRegex(OSError, "interrupted download"), + ): + v8.fetch_codex_v8_artifacts( + spec, version=self.version, cache_root=self.root / "cache" + ) + self.assertEqual(manifest.read_bytes(), b"stale manifest") + self.assertEqual(artifacts.archive.read_bytes(), b"trusted V8 archive") + self.assertEqual(artifacts.binding.read_bytes(), b"trusted V8 binding") + self.assertFalse(list(manifest.parent.glob("*.tmp"))) + + def test_source_and_paired_overrides_do_not_touch_cache(self) -> None: + spec = TARGET_SPECS["x86_64-unknown-linux-gnu"] + for environ in ( + {"V8_FROM_SOURCE": "1"}, + { + "RUSTY_V8_ARCHIVE": "archive.a", + "RUSTY_V8_SRC_BINDING_PATH": "binding.rs", + }, + ): + with ( + self.subTest(environ=environ), + patch.object( + v8, + "fetch_codex_v8_artifacts", + side_effect=AssertionError("unexpected fetch"), + ) as fetch, + ): + self.assertEqual( + v8.resolve_codex_v8_cargo_env(spec, environ=environ), {} + ) + fetch.assert_not_called() + if __name__ == "__main__": unittest.main() diff --git a/scripts/codex_package/v8.py b/scripts/codex_package/v8.py index f2ac15dd09..5208377cca 100644 --- a/scripts/codex_package/v8.py +++ b/scripts/codex_package/v8.py @@ -73,8 +73,20 @@ def fetch_codex_v8_artifacts( binding = cache_dir / binding_name checksums = cache_dir / checksums_name - download_file(f"{release_url}/{checksums.name}", checksums) - verify_release_checksum_manifest(checksums, version=version) + # A cached manifest is sufficient only while it matches this checkout's + # release pin. Probe without deleting it in case refresh is unavailable. + cached_manifest_valid = False + try: + if not checksums.is_symlink(): + verify_release_checksum_manifest( + checksums, version=version, remove_invalid=False + ) + cached_manifest_valid = True + except (OSError, RuntimeError, ValueError): + pass + if not cached_manifest_valid: + download_file(f"{release_url}/{checksums.name}", checksums) + verify_release_checksum_manifest(checksums, version=version) expected_checksums = load_checksums(checksums, {archive.name, binding.name}) for artifact in [archive, binding]: ensure_valid_artifact( @@ -108,7 +120,9 @@ def default_cache_root() -> Path: return Path(tempfile.gettempdir()) / "codex-package" -def verify_release_checksum_manifest(checksums_path: Path, *, version: str) -> None: +def verify_release_checksum_manifest( + checksums_path: Path, *, version: str, remove_invalid: bool = True +) -> None: version_suffix = version.replace(".", "_") trusted_checksums = ( REPO_ROOT @@ -124,7 +138,8 @@ def verify_release_checksum_manifest(checksums_path: Path, *, version: str) -> N if has_checksum(checksums_path, digest): return - checksums_path.unlink(missing_ok=True) + if remove_invalid: + checksums_path.unlink(missing_ok=True) raise RuntimeError( f"V8 checksum manifest {checksums_path} does not match its trusted SHA-256." )