mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
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
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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."
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user