diff --git a/benchmark/edgebench/checkpoint_worker.py b/benchmark/edgebench/checkpoint_worker.py index bd50ef4956..3ae6fd5879 100644 --- a/benchmark/edgebench/checkpoint_worker.py +++ b/benchmark/edgebench/checkpoint_worker.py @@ -7,6 +7,7 @@ from __future__ import annotations from contextlib import closing +from copy import copy import hashlib import json import math @@ -110,7 +111,7 @@ def read(self, *args): def _archive(container, remote: str, destination: Path, *, deadline: float | None = None) -> Path: - """Read through the daemon, rejecting links/devices and archive traversal. + """Read through the daemon, rejecting symlinks/devices and archive traversal. Docker's archive reader works while paused, unlike exec-based file readers. Keep only the declared object, not parent filesystem or process credentials. @@ -150,24 +151,44 @@ def check(): with tarfile.open(fileobj=_ArchiveReader(raw, deadline) if deadline is not None else raw) as archive: members = [] root = PurePosixPath(remote).name - names = set() + names = {} for member in archive: check() path = PurePosixPath(member.name) if (path.is_absolute() or ".." in path.parts or not path.parts or path.parts[0] != root or str(path) in names or - not (member.isfile() or member.isdir())): + not (member.isfile() or member.isdir() or member.islnk())): raise ValueError("Worker archive contains undeclared, linked or special members") - names.add(str(path)) + names[str(path)] = member members.append(member) + # The typed artifact owner observes regular files, not inode + # sharing. Docker encodes a second name for the same inode as a + # tar hardlink (common in build outputs). Materialize its bytes + # from an inventoried regular member; never resolve a link on + # the host filesystem or permit a link chain/symlink escape. + extracted = [] + for member in members: + check() + if member.islnk(): + target_path = PurePosixPath(member.linkname) + target = names.get(str(target_path)) + if (target_path.is_absolute() or ".." in target_path.parts or + target is None or not target.isfile()): + raise ValueError("Worker archive contains undeclared, linked or special members") + regular = copy(target) + regular.name, regular.mode = member.name, member.mode + extracted.append(regular) + else: + extracted.append(member) + def filtered(member, path): check() result = tarfile.data_filter(member, path) check() return result - archive.extractall(destination, members=members, filter=filtered) + archive.extractall(destination, members=extracted, filter=filtered) # The data filter intentionally drops directory modes. This is a # private raw transport witness, so restore only ordinary permission # bits after safe extraction; never restore owners or special bits. diff --git a/benchmark/tests/test_checkpoint_archive_transport.py b/benchmark/tests/test_checkpoint_archive_transport.py index bf363f7503..0e5b41ec86 100644 --- a/benchmark/tests/test_checkpoint_archive_transport.py +++ b/benchmark/tests/test_checkpoint_archive_transport.py @@ -191,10 +191,50 @@ def test_archive_rejects_unsafe_inventory_before_extracting_any_file(tmp_path, u if unsafe in {"symlink", "hardlink", "fifo"}: second.type = {"symlink": tarfile.SYMTYPE, "hardlink": tarfile.LNKTYPE, "fifo": tarfile.FIFOTYPE}[unsafe] - second.linkname = "data/retained" + second.linkname = "../outside" if unsafe == "hardlink" else "data/retained" archive.addfile(second) container, response = _tar_transport(raw.getvalue()) destination = tmp_path / "rejected" with pytest.raises(ValueError, match="undeclared, linked or special"): worker._archive(container, "/data", destination) assert response.closed and not (destination / "data").exists() + + +@pytest.mark.parametrize("forward", [False, True]) +def test_archive_materializes_internal_hardlinks_as_independent_regular_files(tmp_path, forward): + raw = io.BytesIO() + original, alias = tarfile.TarInfo("data/original"), tarfile.TarInfo("data/alias") + payload = b"committed build artifact\x00" + original.size, original.mode = len(payload), 0o640 + alias.type, alias.linkname, alias.mode = tarfile.LNKTYPE, original.name, 0o600 + with tarfile.open(fileobj=raw, mode="w") as archive: + for member in ([alias, original] if forward else [original, alias]): + archive.addfile(member, io.BytesIO(payload) if member.isfile() else None) + container, response = _tar_transport(raw.getvalue()) + copied = worker._archive(container, "/data", tmp_path / "copy") + assert response.closed + assert (copied / "original").read_bytes() == (copied / "alias").read_bytes() == payload + assert (copied / "original").stat().st_mode & 0o777 == 0o640 + assert (copied / "alias").stat().st_mode & 0o777 == 0o600 + (copied / "alias").write_bytes(b"independent receiver edit") + assert (copied / "original").read_bytes() == payload + + +@pytest.mark.parametrize("target", ["/data/original", "data/../original", "data/missing", "data", "data/alias"]) +def test_invalid_hardlink_target_is_rejected_before_extraction(tmp_path, target): + raw = io.BytesIO() + with tarfile.open(fileobj=raw, mode="w") as archive: + directory = tarfile.TarInfo("data") + directory.type = tarfile.DIRTYPE + archive.addfile(directory) + original = tarfile.TarInfo("data/original") + original.size = 4 + archive.addfile(original, io.BytesIO(b"safe")) + alias = tarfile.TarInfo("data/alias") + alias.type, alias.linkname = tarfile.LNKTYPE, target + archive.addfile(alias) + container, response = _tar_transport(raw.getvalue()) + destination = tmp_path / "rejected" + with pytest.raises(ValueError, match="undeclared, linked or special"): + worker._archive(container, "/data", destination) + assert response.closed and not (destination / "data").exists() diff --git a/benchmark/tests/test_checkpoint_worker.py b/benchmark/tests/test_checkpoint_worker.py index b575d0f342..d0160a92da 100644 --- a/benchmark/tests/test_checkpoint_worker.py +++ b/benchmark/tests/test_checkpoint_worker.py @@ -39,6 +39,8 @@ def writer(tmp_path): Path('/data').mkdir() Path('/phase').write_text(str(int(time.time())+7200)) Path('/link').symlink_to('/data', target_is_directory=True) +Path('/data/build-output').write_bytes(b'compiled artifact') +Path('/data/build-alias').hardlink_to('/data/build-output') db=sqlite3.connect('/data/research.sqlite') db.execute('PRAGMA journal_mode=WAL') db.execute('PRAGMA wal_autocheckpoint=0') @@ -386,6 +388,9 @@ def while_frozen(*args, **kwargs): assert metadata["native_cursor_qualified"] is False and metadata["process_memory_captured"] is False assert metadata["native_capsule_component_id"] == "native_capsule_1" assert int((unit / "components/native_capsule/counter").read_text()) == observed[0] + copied = unit / "components/native_capsule" + assert (copied / "build-output").read_bytes() == (copied / "build-alias").read_bytes() == b"compiled artifact" + assert (copied / "build-output").stat().st_ino != (copied / "build-alias").stat().st_ino nested = unit / "components/native_capsule_1" capsule = json.loads((nested / "checkpoint.json").read_bytes()) native = capsule["point_metadata"]["native_reference"]