diff --git a/scripts/largefiles.py b/scripts/largefiles.py index 13b09b1ad..367ba2e28 100644 --- a/scripts/largefiles.py +++ b/scripts/largefiles.py @@ -12,6 +12,7 @@ import os import sys import re import tempfile +import time import urllib.error import urllib.parse import urllib.request @@ -26,6 +27,13 @@ from hashing import compute_hashes LARGE_FILES_RELEASE = "large-files" LARGE_FILES_REPO = "Abdess/retrobios" LARGE_FILES_CACHE = ".cache/large" +# Verified revisions, hard-linked as / beside the cache. A name +# in the cache is replaced when the release serves new bytes under it, so a +# path handed out by name could hold other bytes by the time the caller read +# them. Nothing ever writes other bytes under a sha1. The asset name stays +# last: callers read the extension (an archive is packed as one). +VERIFIED_STORE = ".by-sha1" +ORPHAN_GRACE = 24 * 3600 GITIGNORE = Path(__file__).resolve().parent.parent / ".gitignore" _UNSAFE_ASSET_CHARS = re.compile(r"[^A-Za-z0-9._-]+") @@ -169,14 +177,14 @@ def _fetch_asset( if os.path.isdir(cached): return None - if os.path.exists(cached) and not (expected_sha1 or expected_md5): - return cached if os.path.exists(cached): snapshot = _read_once(cached) if snapshot is None: pass elif _matches(snapshot[0], expected_sha1, expected_md5): - return cached + pinned = _pin_verified(cached, snapshot, dest_dir, name) + if pinned: + return pinned elif not _replaced_on_release(name, snapshot[0]["sha256"], snapshot[1], offline): # A copy of this asset that answers another hash: the caller # wants a different revision under the same name, and the @@ -220,17 +228,71 @@ def _fetch_asset( print(f" large file {candidate}: {exc}", file=sys.stderr) if not downloaded: return None - if (expected_sha1 or expected_md5) and not _matches( - compute_hashes(tmp_path), expected_sha1, expected_md5 - ): + snapshot = _read_once(tmp_path) + if snapshot is None or not _matches(snapshot[0], expected_sha1, expected_md5): return None set_plain_mode(tmp_path) + pinned = _pin_verified(tmp_path, snapshot, dest_dir, name) os.replace(tmp_path, cached) - return cached + _prune_verified(dest_dir) + return pinned finally: _drop(tmp_path) +def _pin_verified( + path: str, snapshot: tuple[dict[str, str], int, int], dest_dir: str, name: str, +) -> str | None: + """The verified revision at *path*, as / in VERIFIED_STORE. + + *snapshot* is what _read_once read: the link is kept only if it holds + that inode, so a copy another run promoted after the hash is never + passed off as the one verified. None when the name moved in between. + """ + revision = os.path.join(dest_dir, VERIFIED_STORE, snapshot[0]["sha1"]) + os.makedirs(revision, exist_ok=True) + final = os.path.join(revision, name) + if os.path.isfile(final): + return final + scratch = os.path.join(revision, f".{os.getpid()}.{os.urandom(4).hex()}.tmp") + try: + os.link(path, scratch) + except FileNotFoundError: + return None + try: + if os.stat(scratch).st_ino != snapshot[2]: + return None + os.replace(scratch, final) + return final + finally: + with contextlib.suppress(FileNotFoundError): + os.unlink(scratch) + + +def _prune_verified(dest_dir: str) -> None: + """Drop the revisions no cache name has held for ORPHAN_GRACE seconds. + + Losing its cache name changes the inode's ctime, so a revision another + run pinned just before the release replaced it stays readable while that + run packs it. + """ + store = os.path.join(dest_dir, VERIFIED_STORE) + now = time.time() + for revision in os.scandir(store): + if not revision.is_dir(): + continue + for entry in os.scandir(revision.path): + if entry.name.endswith(".tmp"): + continue + with contextlib.suppress(FileNotFoundError): + stat = entry.stat() + if stat.st_nlink == 1 and now - stat.st_ctime > ORPHAN_GRACE: + os.unlink(entry.path) + # Another run may be linking a copy into it: left for the next prune. + with contextlib.suppress(FileNotFoundError, OSError): + os.rmdir(revision.path) + + def _asset_urls(name: str) -> list[tuple[str, str]]: """Release URLs an asset may be published under, with their names. @@ -252,9 +314,9 @@ def _asset_urls(name: str) -> list[tuple[str, str]]: ] -def _read_once(path: str) -> tuple[dict[str, str], int] | None: - """Digests and size of the file at *path*, all read from one file, or - None when it is gone. +def _read_once(path: str) -> tuple[dict[str, str], int, int] | None: + """Digests, size and inode of the file at *path*, all read from one + file, or None when it is gone. Every judgement on a cache copy reads this one snapshot. Hashed, then hashed again by path for the release check, a copy another run promoted @@ -271,7 +333,7 @@ def _read_once(path: str) -> tuple[dict[str, str], int] | None: except FileNotFoundError: return None if (before.st_ino, before.st_mtime_ns) == (after.st_ino, after.st_mtime_ns): - return hashes, after.st_size + return hashes, after.st_size, after.st_ino return None diff --git a/scripts/restore_large_files.py b/scripts/restore_large_files.py index 96a7a5010..973872fa3 100644 --- a/scripts/restore_large_files.py +++ b/scripts/restore_large_files.py @@ -17,11 +17,14 @@ Usage: from __future__ import annotations import argparse +import contextlib import hashlib import os import sys +import tempfile sys.path.insert(0, os.path.dirname(__file__)) +from artifacts import set_plain_mode from common import copy_file_atomic, load_database @@ -56,6 +59,36 @@ def index_cache(cache_dir: str) -> dict[str, str]: return index +def _copy_verified(source: str, path: str, sha1: str) -> bool: + """Copy *source* to *path* only if the copy hashes to *sha1*. + + The cache was hashed before the copy, and another run can replace a + cached name in between (a re-uploaded asset): the copy is what counts. + Whole or absent: a scan running beside a plain copy hashed the truncated + file and published that in the database. + """ + handle, scratch = tempfile.mkstemp( + dir=os.path.dirname(os.path.abspath(path)), + prefix=f".{os.path.basename(path)}.", suffix=".tmp", + ) + os.close(handle) + try: + copy_file_atomic(source, scratch) + digest = hashlib.sha1() + with open(scratch, "rb") as f: + for chunk in iter(lambda: f.read(1 << 20), b""): + digest.update(chunk) + if digest.hexdigest() != sha1: + print(f"Changed in the cache while restoring: {source}", file=sys.stderr) + return False + set_plain_mode(scratch) + os.replace(scratch, path) + return True + finally: + with contextlib.suppress(FileNotFoundError): + os.unlink(scratch) + + def restore( cache_dir: str, db_path: str, gitignore: str ) -> tuple[int, list[str]]: @@ -79,9 +112,9 @@ def restore( unsatisfied.append(path) continue os.makedirs(os.path.dirname(path), exist_ok=True) - # Whole or absent: a scan running beside a plain copy hashed the - # truncated file and published that in the database. - copy_file_atomic(source, path) + if not _copy_verified(source, path, sha1): + unsatisfied.append(path) + continue print(f"Restored: {path}") restored += 1 print(f"Total: {restored} files restored") diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 3df8850fe..6bfa03c80 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -2054,6 +2054,44 @@ class CheckoutCompletenessRegressions(unittest.TestCase): finally: os.chdir(cwd) + def test_restore_refuses_a_copy_replaced_after_the_index(self): + """The cache is hashed first and copied later: another run that + replaced the cached name in between put its bytes at a path the + database pins to the old ones.""" + from scripts import restore_large_files + + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: + root = Path(directory) + cache = root / "cache" + cache.mkdir() + payload = b"firmware bytes" + (cache / "asset.bin").write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + (root / ".gitignore").write_text("bios/Sony/big.pup\n", encoding="utf-8") + (root / "database.json").write_text( + json.dumps({"files": {sha1: {"path": "bios/Sony/big.pup"}}}), + encoding="utf-8", + ) + real_index = restore_large_files.index_cache + + def index_then_replace(cache_dir: str) -> dict[str, str]: + index = real_index(cache_dir) + (cache / "asset.bin").write_bytes(b"re-uploaded bytes") + return index + + cwd = os.getcwd() + os.chdir(root) + try: + with mock.patch.object(restore_large_files, "index_cache", index_then_replace): + restored, unsatisfied = restore_large_files.restore( + str(cache), "database.json", ".gitignore" + ) + self.assertEqual((restored, unsatisfied), (0, ["bios/Sony/big.pup"])) + self.assertFalse((root / "bios/Sony/big.pup").exists()) + self.assertEqual(os.listdir(root / "bios/Sony"), []) + finally: + os.chdir(cwd) + def test_restore_leaves_tracked_paths_alone(self): from scripts.restore_large_files import restore diff --git a/tests/test_large_file_cache.py b/tests/test_large_file_cache.py index ed6b9d16e..783c5541b 100644 --- a/tests/test_large_file_cache.py +++ b/tests/test_large_file_cache.py @@ -158,9 +158,9 @@ class LargeFileCacheTest(unittest.TestCase): raise AssertionError("must not download when the cache is valid") largefiles.urllib.request.urlopen = fail - self.assertEqual( - common.fetch_large_file("asset.bin", dest_dir=self.dir), str(cached) - ) + self.assertTrue(os.path.samefile( + common.fetch_large_file("asset.bin", dest_dir=self.dir), cached + )) def test_offline_cache_miss_never_opens_the_network(self): def fail(req, timeout=None): @@ -182,15 +182,15 @@ class LargeFileCacheTest(unittest.TestCase): raise AssertionError("offline cache hit must not open the network") largefiles.urllib.request.urlopen = fail - self.assertEqual( + self.assertTrue(os.path.samefile( common.fetch_large_file( "asset.bin", dest_dir=self.dir, expected_sha1=hashlib.sha1(PAYLOAD_A).hexdigest(), offline=True, ), - str(cached), - ) + cached, + )) def _serve(self, payload: bytes, gets: list): class _Head(io.BytesIO): @@ -222,7 +222,7 @@ class LargeFileCacheTest(unittest.TestCase): "asset.bin", dest_dir=self.dir, expected_sha1=hashlib.sha1(rebuilt).hexdigest(), ) - self.assertEqual(result, str(cached)) + self.assertTrue(os.path.samefile(result, cached)) self.assertEqual(cached.read_bytes(), rebuilt) self.assertEqual(len(gets), 1) @@ -582,7 +582,7 @@ class ReleaseAssetNames(unittest.TestCase): offline=True, registered=self.REGISTERED, ) - self.assertEqual(got, os.path.join(cache, names[self.RTCW])) + self.assertTrue(os.path.samefile(got, os.path.join(cache, names[self.RTCW]))) # The cache entry of the other asset is a valid file, not stale. self.assertTrue(Path(cache, names[self.ET]).exists()) @@ -762,7 +762,7 @@ class ASameSizeReuploadIsSeen(unittest.TestCase): "dsi_nand.bin", str(self.dir), hashlib.sha1(new).hexdigest(), "", offline=False, ) - self.assertEqual(found, str(self.cached)) + self.assertTrue(os.path.samefile(found, self.cached)) def test_the_release_check_compares_digests_at_equal_size(self): expected = {"bios/Nintendo/DSi/dsi_nand.bin": 8} @@ -792,3 +792,49 @@ class ASameSizeReuploadIsSeen(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class AVerifiedPathKeepsItsBytes(unittest.TestCase): + """fetch_large_file verified a cached copy and returned its name. Another + run that found the release re-uploaded then replaced that name with the + new revision, and the first run packed bytes it never verified, under a + manifest sha1 that describes the old ones.""" + + def setUp(self): + _test_registry(self) + self.dir = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp")) + self.addCleanup(shutil.rmtree, self.dir, True) + (self.dir / "asset.bin").write_bytes(PAYLOAD_A) + self.served = {"asset.bin": hashlib.sha256(PAYLOAD_B).hexdigest()} + + def _fetch(self, payload: bytes, offline: bool) -> str | None: + def urlopen(req, timeout=None): + return _SlowResponse(PAYLOAD_B) + + with mock.patch.object(largefiles, "_served_digests", lambda: self.served), \ + mock.patch.object(largefiles.urllib.request, "urlopen", urlopen): + return largefiles.fetch_large_file( + "asset.bin", dest_dir=str(self.dir), + expected_sha1=hashlib.sha1(payload).hexdigest(), offline=offline, + ) + + def test_a_revision_promoted_after_the_check_is_not_read(self): + first = self._fetch(PAYLOAD_A, offline=True) + second = self._fetch(PAYLOAD_B, offline=False) + self.assertEqual((self.dir / "asset.bin").read_bytes(), PAYLOAD_B) + self.assertEqual(Path(second).read_bytes(), PAYLOAD_B) + self.assertEqual(Path(first).read_bytes(), PAYLOAD_A) + self.assertEqual(Path(first).name, "asset.bin") + + def test_an_orphaned_revision_is_dropped_after_the_grace(self): + first = self._fetch(PAYLOAD_A, offline=True) + self._fetch(PAYLOAD_B, offline=False) + self.assertTrue(Path(first).exists()) + with mock.patch.object(largefiles, "ORPHAN_GRACE", -1): + largefiles._prune_verified(str(self.dir)) + self.assertFalse(Path(first).exists()) + self.assertEqual((self.dir / "asset.bin").read_bytes(), PAYLOAD_B) + self.assertEqual( + [p.name for p in (self.dir / largefiles.VERIFIED_STORE).iterdir()], + [hashlib.sha1(PAYLOAD_B).hexdigest()], + ) diff --git a/wiki/release-process.md b/wiki/release-process.md index 250ad6ad7..eb0f1ecd6 100644 --- a/wiki/release-process.md +++ b/wiki/release-process.md @@ -206,7 +206,12 @@ file's own name. A path whose asset name differs from its file name is uploaded from a copy carrying the asset name. **Local cache.** `generate_pack.py` calls `fetch_large_file()` which downloads -from the release and caches in `.cache/large/` for subsequent runs. +from the release and caches in `.cache/large/` for subsequent runs. The path it +returns is a hard link under `.cache/large/.by-sha1//`, never the cached +name: another run that finds the asset re-uploaded replaces the name, and the +first run still reads the bytes it verified. A revision no name holds is +dropped a day later, at the next download. `restore_large_files.py` hashes +each copy before it moves it into place. **Check.** The installer refuses a download whose `Content-Length` differs from the manifest size, and the manifest size is that of the local file. A