From 473f5a2eaab7ef7c19a449136077737d50afe059 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 18 Oct 2026 22:32:41 -0700 Subject: [PATCH 01/14] feat(build-context): write each image only the part of a shared build context its Dockerfile reads write_pruned(archive, {folder: dockerfile}) writes into each folder what that image's build reads of the context tar.gz: its COPY and ADD sources in every stage and its bind mounts' sources, less what the ignore file and the OS or Python leave out, with the Dockerfile, the ignore file and the targets of kept links. Images sharing one context are written from it in two passes, never unpacking the rest. A Dockerfile whose reads can't be told (a variable in a source, a flag it doesn't follow) keeps everything the context sends, and says why. The Dockerfile reading moves into utils/dockerfile.py (instructions with continuations, the escape directive, comments and heredocs; COPY and ADD arguments), which the Modal rewrite now uses too. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_image_build.py | 57 ++---- src/agent_env/utils/build_context.py | 185 ++++++++++++++++++ src/agent_env/utils/dockerfile.py | 110 +++++++++++ tst/integration/utils/__init__.py | 0 .../utils/build_context_prune_test.py | 123 ++++++++++++ tst/unit/utils/build_context_prune_test.py | 162 +++++++++++++++ tst/unit/utils/dockerfile_test.py | 71 +++++++ 7 files changed, 670 insertions(+), 38 deletions(-) create mode 100644 src/agent_env/utils/dockerfile.py create mode 100644 tst/integration/utils/__init__.py create mode 100644 tst/integration/utils/build_context_prune_test.py create mode 100644 tst/unit/utils/build_context_prune_test.py create mode 100644 tst/unit/utils/dockerfile_test.py diff --git a/src/agent_env/providers/sandbox_providers/modal_image_build.py b/src/agent_env/providers/sandbox_providers/modal_image_build.py index c7eca715..280c2581 100644 --- a/src/agent_env/providers/sandbox_providers/modal_image_build.py +++ b/src/agent_env/providers/sandbox_providers/modal_image_build.py @@ -5,9 +5,7 @@ import asyncio import glob -import json import logging -import re import tarfile import tempfile import weakref @@ -18,6 +16,7 @@ from agent_env.config import get_config from agent_env.utils.build_context import extract, ignore_file +from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions if TYPE_CHECKING: from agent_env.artifact.artifacts.docker_image import DockerImageArtifact @@ -76,7 +75,6 @@ def fmt_exc(e: BaseException) -> str: return f"{type_name}: {msg}" if msg else type_name -_COPY_OR_ADD = re.compile(r"\s*(COPY|ADD)\s+(.*)", re.IGNORECASE | re.DOTALL) # The flags COPY takes as ADD does; ADD's own (--checksum, --keep-git-dir, --unpack) keep an ADD as it is. _COPY_FLAGS = ("--chown=", "--chmod=", "--link") @@ -86,43 +84,26 @@ def modal_dockerfile(text: str, context: Path) -> str: a JSON-form COPY or ADD whose paths hold no whitespace in shell form, which Modal's context parser reads, and an ADD of files and folders in ``context``, none of them a tar archive, as the COPY it is, since Modal's ADD only fetches a URL. Every other line is left as written.""" - lines, instruction = [], [] - for line in text.splitlines(keepends=True): - instruction.append(line) - if not line.rstrip("\r\n").endswith("\\"): - lines.append(_for_modal("".join(instruction), context)) - instruction = [] - return "".join(lines + instruction) - - -def _for_modal(instruction: str, context: Path) -> str: - match = _COPY_OR_ADD.fullmatch(re.sub(r"\\\r?\n", " ", instruction).strip()) - if match is None: - return instruction - keyword, rest = match.group(1).upper(), match.group(2).strip() - flags = [] - while rest.startswith("--"): - flag, _, rest = rest.partition(" ") - flags.append(flag) - rest = rest.strip() - if rest.startswith("["): - try: - args = json.loads(rest) - except ValueError: - return instruction - if not (isinstance(args, list) and len(args) >= 2 - and all(isinstance(arg, str) and arg and not any(c.isspace() for c in arg) for arg in args)): - return instruction - elif keyword == "ADD" and not rest.startswith("<<") and not any(c in rest for c in "\"'"): - args = rest.split() - else: - return instruction + return "".join(_for_modal(instruction, context) for instruction in instructions(text)) + + +def _for_modal(instruction: Instruction, context: Path) -> str: + args = copy_arguments(instruction) + if args is None or args.heredoc: + return instruction.text + paths = (*args.sources, args.destination) + if args.json_form: + if not all(path and not any(c.isspace() for c in path) for path in paths): + return instruction.text + elif args.keyword != "ADD": + return instruction.text + keyword = args.keyword if keyword == "ADD": - if len(args) < 2 or not all(flag.startswith(_COPY_FLAGS) for flag in flags) \ - or not all(_local_and_not_an_archive(source, context) for source in args[:-1]): - return instruction + if not all(flag.startswith(_COPY_FLAGS) for flag in args.flags) \ + or not all(_local_and_not_an_archive(source, context) for source in args.sources): + return instruction.text keyword = "COPY" - return " ".join([keyword, *flags, *args]) + "\n" + return " ".join([keyword, *args.flags, *paths]) + "\n" def _local_and_not_an_archive(source: str, context: Path) -> bool: diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index c00ad88f..d7f4f884 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -7,14 +7,19 @@ import hashlib import json import os +import posixpath +import shutil import stat import tarfile import unicodedata +from collections.abc import Mapping from dataclasses import dataclass +from fnmatch import fnmatchcase from pathlib import Path, PurePosixPath from modal import FilePatternMatcher +from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions from agent_env.utils.paths import LEAVINGS # Bumped when what source_digest covers changes, so a digest is never compared across meanings. @@ -107,6 +112,186 @@ def extract(archive: Path, out: Path) -> None: tar.extractall(out, filter=_context_member) +@dataclass(frozen=True) +class ContextReads: + """What a Dockerfile's build reads of its context: the context paths in ``paths``, each of which may hold Docker's + wildcards, or all of it when ``whole`` says why: it copies all of it, or reads what can't be told before a build.""" + + paths: tuple[str, ...] = () + whole: str | None = None + + +@dataclass(frozen=True) +class Pruned: + """What ``write_pruned`` wrote into a folder: ``files`` files of ``size`` bytes, and why it wrote everything the + context sends when it did.""" + + files: int + size: int + whole: str | None + + +# The COPY and ADD flags that change how a source is written, not which one it is. +_SOURCE_KEEPING_FLAGS = ("--chown=", "--chmod=", "--link", "--checksum=", "--keep-git-dir", "--unpack") + + +def context_reads(dockerfile: str) -> ContextReads: + """What a build of ``dockerfile`` reads of its context: each local COPY and ADD source of every stage and each bind + mount's source, but not a COPY --from, a URL or git source, or a heredoc.""" + paths: list[str] = [] + for instruction in instructions(dockerfile): + if instruction.keyword in ("COPY", "ADD"): + whole = _copy_reads(instruction, paths) + elif instruction.keyword == "RUN": + whole = _mount_reads(instruction, paths) + else: + continue + if whole: + return ContextReads(whole=whole) + return ContextReads(paths=tuple(dict.fromkeys(paths))) + + +def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned]: + """Write into each folder ``images`` names the part of the build context tar.gz at ``archive`` that its + Dockerfile, at the path ``images`` gives in the context, reads: what ``context_reads`` finds, less what the + context's ignore file leaves out and what the OS or Python leaves behind, with the Dockerfile, the ignore file + and the targets of kept links. Reads the + archive twice and never unpacks the rest of it. Raises ValueError when a Dockerfile isn't in the archive, or on a + member ``extract`` refuses.""" + dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items()} + wanted = {name for dockerfile in dockerfiles.values() for name in (dockerfile, *_ignore_files(dockerfile))} + links: dict[str, str | None] = {} + texts: dict[str, str] = {} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + if (name := _member_name(member)) is None: + continue + links[name] = member.linkname if member.issym() else None + if name in wanted and member.isreg(): + texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + plans: dict[Path, tuple[set[str], str | None]] = {} + for folder, dockerfile in dockerfiles.items(): + if dockerfile not in texts: + raise ValueError(f"the build context has no {dockerfile}") + ignore = next((name for name in _ignore_files(dockerfile) if name in texts), None) + matcher = FilePatternMatcher(*_patterns(texts[ignore])) if ignore else None + reads = context_reads(texts[dockerfile]) + plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher), reads.whole) + counts = {folder: [0, 0] for folder in images} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + name = _member_name(member) + first: Path | None = None + for folder in (folder for folder, (kept, _) in plans.items() if name in kept): + if member.isreg() and first is not None: + (folder / name).parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(first, folder / name) + else: + folder.mkdir(parents=True, exist_ok=True) + tar.extract(member, folder, filter=_context_member) + first = folder / name if member.isreg() else None + if member.isreg(): + counts[folder][0] += 1 + counts[folder][1] += member.size + return {folder: Pruned(*counts[folder], plans[folder][1]) for folder in images} + + +def _copy_reads(instruction: Instruction, paths: list[str]) -> str | None: + """Add the context paths a COPY or ADD reads to ``paths``; why it reads all of the context, when it does.""" + args = copy_arguments(instruction) + if args is None: + return f"{instruction.keyword} {instruction.arguments} can't be read" + if any(flag.startswith("--from=") for flag in args.flags): + return None + if unknown := next((flag for flag in args.flags if not flag.startswith(_SOURCE_KEEPING_FLAGS)), None): + return f"{args.keyword} {unknown} isn't followed" + for source in args.sources: + if args.keyword == "ADD" and ("://" in source or source.startswith("git@")): + continue + if "$" in source: + return f"{args.keyword} {source} names a variable" + if (path := _context_path(source)) is None: + return f"{args.keyword} {source} copies all of it" + paths.append(path) + return None + + +def _mount_reads(instruction: Instruction, paths: list[str]) -> str | None: + """Add the context paths a RUN's bind mounts read to ``paths``; why it reads all of the context, when it does. A + bind mount names no other image or stage with ``from``, and its source defaults to the whole context.""" + rest = instruction.arguments + while rest.startswith("--"): + flag, _, rest = rest.partition(" ") + rest = rest.strip() + if not flag.startswith("--mount="): + continue + options = dict(option.partition("=")[::2] for option in flag.removeprefix("--mount=").split(",")) + if options.get("type", "bind") != "bind" or "from" in options: + continue + source = options.get("source") or options.get("src") or "." + if "$" in source: + return f"RUN --mount source {source} names a variable" + if (path := _context_path(source)) is None: + return f"RUN --mount binds {source}, all of it" + paths.append(path) + return None + + +def _context_path(source: str) -> str | None: + """``source``'s path in the context; None for the context itself, or a path that would leave it.""" + path = posixpath.normpath(source.lstrip("/")) + return None if path == "." or path == ".." or path.startswith("../") else path + + +def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, + matcher: FilePatternMatcher | None) -> set[str]: + """The names in ``links`` (each name's link target, None when it's no link) that a build reads, as ``write_pruned`` + keeps them, and the folders above them. What the OS or Python leaves behind is dropped, as ``BuildContext`` drops it.""" + def sent(name: str) -> bool: + return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) + + if reads.whole is not None: + kept = {name for name in links if sent(name)} + else: + kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} + kept |= {name for name in (dockerfile, ignore) if name in links} + pending = [name for name in kept if links[name]] + while pending: + name = pending.pop() + target = posixpath.normpath(posixpath.join(posixpath.dirname(name), links[name])) + if links[name].startswith("/") or target == ".." or target.startswith("../"): + continue + for other in links: + if other not in kept and (other == target or other.startswith(f"{target}/")) and sent(other): + kept.add(other) + if links[other]: + pending.append(other) + return kept | {str(parent) for name in kept for parent in PurePosixPath(name).parents if str(parent) in links} + + +def _matches(path: str, name: str) -> bool: + """Whether ``name`` is ``path`` or under it, each part matched as Docker matches a COPY source's.""" + want, have = path.split("/"), name.split("/") + return len(have) >= len(want) and all(fnmatchcase(part, glob.replace("[^", "[!")) for part, glob in zip(have, want)) + + +def _ignore_files(dockerfile: str) -> tuple[str, ...]: + """Where ``docker build`` looks for its ignore file, in order: ``.dockerignore`` beside the + Dockerfile, then the context's ``.dockerignore``.""" + return f"{dockerfile}.dockerignore", ".dockerignore" + + +def _patterns(ignore_file: str) -> list[str]: + """An ignore file's patterns, its comment lines left out as Docker leaves them out.""" + return [line for line in ignore_file.splitlines() if line.strip() and not line.lstrip().startswith("#")] + + +def _member_name(member: tarfile.TarInfo) -> str | None: + """``member``'s path in the context; None for the context's own folder.""" + name = posixpath.normpath(member.name.lstrip("/")) + return None if name == "." else name + + class _Walk: """Lists a context folder in sorted order.""" diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py new file mode 100644 index 00000000..77922a6c --- /dev/null +++ b/src/agent_env/utils/dockerfile.py @@ -0,0 +1,110 @@ +"""A Dockerfile's instructions as ``docker build`` reads them, and the arguments of its COPY and ADD instructions.""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass + +_DIRECTIVE = re.compile(r"#\s*([A-Za-z][A-Za-z0-9_-]*)\s*=\s*(\S+)\s*") +_HEREDOC = re.compile(r"<<(-?)([\"']?)([A-Za-z_][A-Za-z0-9_]*)\2") +# The instructions whose arguments can open a heredoc. +_HEREDOC_KEYWORDS = frozenset({"RUN", "COPY", "ADD"}) + + +@dataclass(frozen=True) +class Instruction: + """One instruction, or one line between instructions (``keyword`` None: blank, a comment, a parser directive). + ``text`` is what the Dockerfile holds for it, so the texts of all of them joined are the Dockerfile.""" + + keyword: str | None # upper case + arguments: str # on one line: escaped line breaks and the comment lines among them removed, heredoc bodies left out + text: str + heredocs: tuple[str, ...] = () # each heredoc's body, in order + + +@dataclass(frozen=True) +class CopyArguments: + """A COPY or ADD's flags as written (``--name`` or ``--name=value``), sources and destination.""" + + keyword: str + flags: tuple[str, ...] + sources: tuple[str, ...] # a heredoc's ``< list[Instruction]: + """``dockerfile``'s instructions and the lines between them, in order: an instruction's escaped line breaks + continue it, a comment line among them is dropped from its arguments, and a heredoc's body is part of it.""" + lines = dockerfile.splitlines(keepends=True) + escape, at = "\\", 0 + while at < len(lines) and (directive := _DIRECTIVE.fullmatch(lines[at].strip())): + if directive.group(1).lower() == "escape": + escape = directive.group(2) + at += 1 + found = [Instruction(None, "", line) for line in lines[:at]] + while at < len(lines): + line = lines[at] + if not line.strip() or line.lstrip().startswith("#"): + found.append(Instruction(None, "", line)) + at += 1 + continue + text, parts = [], [] + while at < len(lines): + line = lines[at] + text.append(line) + at += 1 + if parts and (not line.strip() or line.lstrip().startswith("#")): + continue + body = line.rstrip("\r\n").rstrip() + if not body.endswith(escape): + parts.append(body) + break + parts.append(body[:-len(escape)]) + keyword, _, arguments = " ".join(part.strip() for part in parts).partition(" ") + keyword, arguments = keyword.upper(), arguments.strip() + heredocs = [] + if keyword in _HEREDOC_KEYWORDS: + for strip_tabs, _, name in _HEREDOC.findall(arguments): + body = [] + while at < len(lines): + line = lines[at] + text.append(line) + at += 1 + if (line.rstrip("\r\n").lstrip("\t") if strip_tabs else line.rstrip("\r\n")) == name: + break + body.append(line) + heredocs.append("".join(body)) + found.append(Instruction(keyword, arguments, "".join(text), tuple(heredocs))) + return found + + +def copy_arguments(instruction: Instruction) -> CopyArguments | None: + """A COPY or ADD's arguments; None for any other instruction, or one whose arguments it can't read: JSON that + isn't a list of at least two strings, a quoted shell-form path, or fewer than a source and a destination.""" + if instruction.keyword not in ("COPY", "ADD"): + return None + rest, flags = instruction.arguments, [] + while rest.startswith("--"): + flag, _, rest = rest.partition(" ") + flags.append(flag) + rest = rest.strip() + json_form = rest.startswith("[") + if json_form: + try: + args = json.loads(rest) + except ValueError: + return None + if not (isinstance(args, list) and all(isinstance(arg, str) for arg in args)): + return None + else: + if any(quote in rest for quote in "\"'"): + return None + args = rest.split() + if len(args) < 2: + return None + sources = tuple(source for source in args[:-1] if not _HEREDOC.fullmatch(source)) + return CopyArguments(instruction.keyword, tuple(flags), sources, args[-1], json_form, + heredoc=len(sources) < len(args) - 1) diff --git a/tst/integration/utils/__init__.py b/tst/integration/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/build_context_prune_test.py new file mode 100644 index 00000000..03e18d39 --- /dev/null +++ b/tst/integration/utils/build_context_prune_test.py @@ -0,0 +1,123 @@ +"""An image built from the part of a context ``write_pruned`` keeps holds exactly the files one built from the whole +context does: a multi-stage build with wildcards, a local archive ADD unpacks, a bind mount, an ignore file and a linked +folder, from a context full of what the build never reads. Needs Docker.""" + +import hashlib +import io +import shutil +import subprocess +import tarfile +import uuid +from pathlib import Path + +import pytest + +from agent_env.utils.build_context import write_pruned +from tst.util.capabilities import missing_capability_reason + +pytestmark = [pytest.mark.integration, pytest.mark.int_test_slow] + +BASE = "mirror.gcr.io/library/python:3.12-slim@sha256:423ed6ab25b1921a477529254bfeeabf5855151dc2c3141699a1bfc852199fbf" +DOCKERFILE = f"""FROM {BASE} AS build +COPY tools/gen.sh /gen.sh +RUN sh /gen.sh > /built.txt + +FROM {BASE} +COPY --from=build /built.txt /out/built.txt +COPY app/ /app/ +COPY conf/*.cfg /etc/svc/ +ADD vendor.tar.gz /vendor/ +COPY current/ /release/ +RUN --mount=type=bind,source=scripts,target=/scripts cp /scripts/run.sh /out/run.sh +""" +# What the build reads, beside a context ten times bigger it never does. +CONTEXT = { + "server/Dockerfile": DOCKERFILE, + ".dockerignore": "app/tests\n**/*.pyc\n", + "tools/gen.sh": "echo generated\n", "app/main.py": "print('main')\n", "app/util.py": "u = 1\n", + "app/tests/test_main.py": "assert True\n", "app/cache.pyc": "compiled", "conf/a.cfg": "a=1\n", + "conf/b.cfg": "b=2\n", "conf/readme.md": "not a cfg\n", "scripts/run.sh": "exec python /app/main.py\n", + "releases/v2/app.txt": "release 2\n", "releases/v1/app.txt": "release 1\n", + **{f"websites/site{n}/index.html": f"

{n}

" * 2000 for n in range(10)}, +} +VENDOR = {"lib/vendored.py": "V = True\n"} + + +def _add(tar: tarfile.TarFile, name: str, data: bytes, mode: int = 0o644) -> None: + info = tarfile.TarInfo(name) + info.size, info.mode = len(data), mode + tar.addfile(info, io.BytesIO(data)) + + +def _stored_context(path: Path) -> Path: + """The context as a store might hold it, ignored files and all, with a vendored archive and a release link.""" + vendor = io.BytesIO() + with tarfile.open(fileobj=vendor, mode="w:gz") as tar: + for name, text in VENDOR.items(): + _add(tar, name, text.encode()) + with tarfile.open(path, "w:gz") as tar: + for name, text in CONTEXT.items(): + _add(tar, name, text.encode(), 0o755 if name.endswith(".sh") else 0o644) + _add(tar, "vendor.tar.gz", vendor.getvalue()) + link = tarfile.TarInfo("current") + link.type, link.linkname = tarfile.SYMTYPE, "releases/v2" + tar.addfile(link) + return path + + +def _build(context: Path, tag: str) -> None: + result = subprocess.run(["docker", "build", "-q", "-f", str(context / "server" / "Dockerfile"), "-t", tag, + str(context)], capture_output=True, text=True) + assert result.returncode == 0, result.stderr + + +def _image_files(tag: str) -> dict[str, tuple[int, str]]: + """Each file, folder and link the image holds under what the build wrote, by its path: its mode and content.""" + container = subprocess.run(["docker", "create", tag], capture_output=True, text=True, check=True).stdout.strip() + try: + exported = subprocess.run(["docker", "export", container], capture_output=True, check=True).stdout + finally: + subprocess.run(["docker", "rm", container], capture_output=True) + found = {} + with tarfile.open(fileobj=io.BytesIO(exported)) as tar: + for member in tar: + if member.name.split("/", 1)[0] not in ("app", "out", "etc", "vendor", "release"): + continue + content = hashlib.sha256(tar.extractfile(member).read()).hexdigest() if member.isreg() else member.linkname + found[member.name] = (member.mode, content) + return found + + +@pytest.fixture +def docker(): + if shutil.which("docker") is None or subprocess.run(["docker", "info"], capture_output=True).returncode != 0: + pytest.skip(missing_capability_reason("docker_daemon")) + tags = [] + yield tags + for tag in tags: + subprocess.run(["docker", "rmi", "-f", tag], capture_output=True) + + +def test_an_image_built_from_the_pruned_context_holds_what_one_from_the_whole_context_does(tmp_path, docker): + archive = _stored_context(tmp_path / "ctx.tar.gz") + full = tmp_path / "full" + with tarfile.open(archive) as tar: + tar.extractall(full, filter="tar") + + pruned = write_pruned(archive, {tmp_path / "pruned": "server/Dockerfile"}) + + kept = {str(path.relative_to(tmp_path / "pruned")) for path in (tmp_path / "pruned").rglob("*") + if path.is_file() or path.is_symlink()} + assert kept == {"server/Dockerfile", ".dockerignore", "tools/gen.sh", "app/main.py", "app/util.py", "conf/a.cfg", + "conf/b.cfg", "vendor.tar.gz", "current", "releases/v2/app.txt", "scripts/run.sh"} + assert pruned[tmp_path / "pruned"].whole is None + suffix = uuid.uuid4().hex[:8] + docker.extend([f"prune-full-{suffix}", f"prune-kept-{suffix}"]) + _build(full, docker[0]) + _build(tmp_path / "pruned", docker[1]) + + built = _image_files(docker[1]) + assert built == _image_files(docker[0]) + assert {"app/main.py", "out/built.txt", "out/run.sh", "etc/svc/a.cfg", "vendor/lib/vendored.py", + "release/app.txt"} <= set(built) + assert not any(name.startswith("app/tests") or name.endswith(".pyc") for name in built) diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py new file mode 100644 index 00000000..d64ccd73 --- /dev/null +++ b/tst/unit/utils/build_context_prune_test.py @@ -0,0 +1,162 @@ +"""What a Dockerfile's build reads of its context, and writing each of several images only that part of a context +they share, from its tar.gz.""" + +import io +import tarfile +from pathlib import Path + +import pytest + +from agent_env.utils.build_context import ContextReads, context_reads, write_pruned + + +@pytest.mark.parametrize(("dockerfile", "paths"), [ + ("COPY gmail/requirements.txt .\nCOPY BaseService/ ./svc/BaseService/\n", ("gmail/requirements.txt", "BaseService")), + ("FROM node AS cli\nFROM python\nCOPY --from=cli /usr/bin/node /usr/bin/node\nCOPY app /app\n", ("app",)), + ("ADD https://example.com/f /f\nADD git@github.com:o/r.git /r\nADD local.tar.gz /x\n", ("local.tar.gz",)), + ("COPY < Path: + """A context tar.gz holding ``files`` and ``links``, everything in it as another tool might have written it.""" + with tarfile.open(path, "w:gz") as tar: + for name, text in files.items(): + info = tarfile.TarInfo(name) + data = text.encode() + info.size, info.mode = len(data), 0o755 if name in executable else 0o644 + tar.addfile(info, io.BytesIO(data)) + for name, target in (links or {}).items(): + info = tarfile.TarInfo(name) + info.type, info.linkname = tarfile.SYMTYPE, target + tar.addfile(info) + return path + + +def _files(folder: Path) -> dict[str, str]: + return {str(path.relative_to(folder)): path.read_text() for path in sorted(folder.rglob("*")) + if path.is_file() and not path.is_symlink()} + + +SERVERS = { + "__init__.py": "", "BaseService/base.py": "base", "wheels/x.whl": "wheel", "gsuite/g.py": "g", + "gmail/Dockerfile": "FROM python\nCOPY __init__.py BaseService/ gsuite/ gmail/ wheels/ /app/\n", + "gmail/main.py": "gmail", "slack/Dockerfile": "FROM python\nCOPY BaseService/ /app/BaseService/\nCOPY slack/ /s/\n", + "slack/main.py": "slack", "websites/huge.bin": "x" * 4096, +} + + +def test_each_image_gets_only_what_its_dockerfile_reads_from_one_archive_they_share(tmp_path): + archive = _archive(tmp_path / "ctx.tar.gz", SERVERS, executable=("gmail/main.py",)) + + pruned = write_pruned(archive, {tmp_path / "gmail": "gmail/Dockerfile", tmp_path / "slack": "slack/Dockerfile"}) + + assert _files(tmp_path / "gmail") == {name: SERVERS[name] for name in ( + "BaseService/base.py", "__init__.py", "gmail/Dockerfile", "gmail/main.py", "gsuite/g.py", "wheels/x.whl")} + assert _files(tmp_path / "slack") == {name: SERVERS[name] for name in ( + "BaseService/base.py", "slack/Dockerfile", "slack/main.py")} + assert (tmp_path / "gmail" / "gmail" / "main.py").stat().st_mode & 0o111 + assert pruned[tmp_path / "slack"].files == 3 and pruned[tmp_path / "slack"].whole is None + assert pruned[tmp_path / "slack"].size == sum(len(SERVERS[name]) for name in ( + "BaseService/base.py", "slack/Dockerfile", "slack/main.py")) + + +def test_what_the_ignore_file_leaves_out_stays_out_and_a_dockerfiles_own_ignore_file_replaces_the_contexts(tmp_path): + files = { + "Dockerfile": "FROM python\nCOPY app/ /app/\n", ".dockerignore": "app/tests\n!app/tests/keep.py\n", + "app/main.py": "m", "app/tests/t.py": "t", "app/tests/keep.py": "k", + "sub/Dockerfile": "FROM python\nCOPY app/ /app/\n", "sub/Dockerfile.dockerignore": "# only notes left out\n**/*.md\n", + "app/notes.md": "n", + } + archive = _archive(tmp_path / "ctx.tar.gz", files) + + write_pruned(archive, {tmp_path / "root": "Dockerfile", tmp_path / "sub": "sub/Dockerfile"}) + + assert set(_files(tmp_path / "root")) == {"Dockerfile", ".dockerignore", "app/main.py", "app/tests/keep.py", + "app/notes.md"} + assert set(_files(tmp_path / "sub")) == {"sub/Dockerfile", "sub/Dockerfile.dockerignore", "app/main.py", + "app/tests/t.py", "app/tests/keep.py"} + + +def test_a_dockerfile_that_reads_all_of_the_context_gets_what_docker_would_send_and_says_why(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY . /app\n", ".dockerignore": "*.log\n", "a.py": "a", "b.log": "b"} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", ".dockerignore", "a.py"} + assert pruned[tmp_path / "out"].whole == "COPY . copies all of it" + + +def test_a_kept_link_keeps_what_it_points_at_in_the_context_and_a_link_out_of_it_is_left_out(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY current/ /app/\nCOPY out /out\n", "v2/app.py": "2", "v1/app.py": "1"} + archive = _archive(tmp_path / "ctx.tar.gz", files, links={"current": "v2", "out": "../../etc/passwd"}) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "v2/app.py"} + assert (tmp_path / "out" / "current").is_symlink() + assert not (tmp_path / "out" / "out").exists() + + +def test_wildcards_match_one_part_of_a_path_as_dockers_do(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY *.txt [^b]*.cfg /x/\n", "a.txt": "a", "sub/c.txt": "c", "a.cfg": "a", + "b.cfg": "b"} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "a.txt", "a.cfg"} + + +def test_what_the_os_or_python_leaves_behind_in_a_copied_folder_is_left_out(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY lib/ /lib/\n", "lib/mod.py": "m", "lib/__pycache__/mod.cpython-312.pyc": "c", + "lib/.DS_Store": "d"} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "lib/mod.py"} + + +def test_a_dockerfile_missing_from_the_archive_is_refused(tmp_path): + archive = _archive(tmp_path / "ctx.tar.gz", {"a.py": "a"}) + + with pytest.raises(ValueError, match="the build context has no gmail/Dockerfile"): + write_pruned(archive, {tmp_path / "out": "gmail/Dockerfile"}) + + +def test_a_member_no_context_holds_is_refused(tmp_path): + archive = tmp_path / "ctx.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + dockerfile = b"FROM python\nCOPY . /app\n" + info = tarfile.TarInfo("Dockerfile") + info.size = len(dockerfile) + tar.addfile(info, io.BytesIO(dockerfile)) + hard = tarfile.TarInfo("hard") + hard.type, hard.linkname = tarfile.LNKTYPE, "Dockerfile" + tar.addfile(hard) + + with pytest.raises(ValueError, match="neither a file, a folder nor a link"): + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) diff --git a/tst/unit/utils/dockerfile_test.py b/tst/unit/utils/dockerfile_test.py new file mode 100644 index 00000000..2dd60ea5 --- /dev/null +++ b/tst/unit/utils/dockerfile_test.py @@ -0,0 +1,71 @@ +"""A Dockerfile read into instructions as docker build reads it, and a COPY or ADD's arguments.""" + +import pytest + +from agent_env.utils.dockerfile import copy_arguments, instructions + +DOCKERFILE = """# syntax=docker/dockerfile:1 +FROM python:3.12-slim AS base +# a comment +RUN apt-get update \\ + # a comment among the continued lines + && apt-get install -y curl + +COPY --chown=1:1 a.txt \\ + b/ /app/ +RUN < Date: Sun, 18 Oct 2026 22:47:51 -0700 Subject: [PATCH 02/14] fix(build-context): read a here-string as the shell does, fail safe on a likely misreading, and find link targets in time docker build leaves a <<< here-string to the shell, so the reader no longer opens a heredoc on one and swallows the instructions after it. When the reading looks wrong, a heredoc running unclosed to the end or a source matching nothing in the context, the image keeps everything the context sends and says why, rather than lose a file it needs. Kept links find their targets in a sorted index instead of a scan of every name: 5,000 links among 100,000 files went from 28 s to 0.16 s. The Modal rewrite keeps the comment lines among a continued instruction's lines, above the one line it writes. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_image_build.py | 4 ++- src/agent_env/utils/build_context.py | 20 +++++++++++-- src/agent_env/utils/dockerfile.py | 12 +++++--- .../modal_context_build_test.py | 7 +++++ tst/unit/utils/build_context_prune_test.py | 29 +++++++++++++++++++ tst/unit/utils/dockerfile_test.py | 15 ++++++++++ 6 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_image_build.py b/src/agent_env/providers/sandbox_providers/modal_image_build.py index 280c2581..a6288c17 100644 --- a/src/agent_env/providers/sandbox_providers/modal_image_build.py +++ b/src/agent_env/providers/sandbox_providers/modal_image_build.py @@ -103,7 +103,9 @@ def _for_modal(instruction: Instruction, context: Path) -> str: or not all(_local_and_not_an_archive(source, context) for source in args.sources): return instruction.text keyword = "COPY" - return " ".join([keyword, *args.flags, *paths]) + "\n" + # The rewrite is one line, so comment lines among the continued ones go above it rather than be lost. + comments = [line for line in instruction.text.splitlines(keepends=True)[1:] if line.lstrip().startswith("#")] + return "".join(comments) + " ".join([keyword, *args.flags, *paths]) + "\n" def _local_and_not_an_archive(source: str, context: Path) -> bool: diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index d7f4f884..792b8bc6 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -3,6 +3,7 @@ from __future__ import annotations +import bisect import gzip import hashlib import json @@ -140,6 +141,8 @@ def context_reads(dockerfile: str) -> ContextReads: mount's source, but not a COPY --from, a URL or git source, or a heredoc.""" paths: list[str] = [] for instruction in instructions(dockerfile): + if not instruction.complete: + return ContextReads(whole=f"a heredoc in {instruction.keyword} never ends, so the rest can't be read") if instruction.keyword in ("COPY", "ADD"): whole = _copy_reads(instruction, paths) elif instruction.keyword == "RUN": @@ -176,6 +179,9 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned ignore = next((name for name in _ignore_files(dockerfile) if name in texts), None) matcher = FilePatternMatcher(*_patterns(texts[ignore])) if ignore else None reads = context_reads(texts[dockerfile]) + # A source the reading found that names nothing here is likelier a misreading than a build that fails. + if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): + reads = ContextReads(whole=f"{missing} matches nothing in the context") plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher), reads.whole) counts = {folder: [0, 0] for folder in images} with tarfile.open(archive, "r:gz") as tar: @@ -255,20 +261,30 @@ def sent(name: str) -> bool: else: kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} kept |= {name for name in (dockerfile, ignore) if name in links} + names = sorted(links) pending = [name for name in kept if links[name]] while pending: name = pending.pop() target = posixpath.normpath(posixpath.join(posixpath.dirname(name), links[name])) if links[name].startswith("/") or target == ".." or target.startswith("../"): continue - for other in links: - if other not in kept and (other == target or other.startswith(f"{target}/")) and sent(other): + for other in _at_or_under(names, target): + if other not in kept and sent(other): kept.add(other) if links[other]: pending.append(other) return kept | {str(parent) for name in kept for parent in PurePosixPath(name).parents if str(parent) in links} +def _at_or_under(names: list[str], path: str) -> list[str]: + """The names in ``names``, sorted, that are ``path`` or under it.""" + at = bisect.bisect_left(names, path) + found = [path] if at < len(names) and names[at] == path else [] + start = bisect.bisect_left(names, f"{path}/") + end = bisect.bisect_left(names, f"{path}0") # "0" sorts right after "/" + return found + names[start:end] + + def _matches(path: str, name: str) -> bool: """Whether ``name`` is ``path`` or under it, each part matched as Docker matches a COPY source's.""" want, have = path.split("/"), name.split("/") diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py index 77922a6c..7fa01b16 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/dockerfile.py @@ -7,7 +7,8 @@ from dataclasses import dataclass _DIRECTIVE = re.compile(r"#\s*([A-Za-z][A-Za-z0-9_-]*)\s*=\s*(\S+)\s*") -_HEREDOC = re.compile(r"<<(-?)([\"']?)([A-Za-z_][A-Za-z0-9_]*)\2") +# ``<<<`` is a shell here-string, which docker build leaves to the shell, not a heredoc. +_HEREDOC = re.compile(r"(? list[Instruction]: parts.append(body[:-len(escape)]) keyword, _, arguments = " ".join(part.strip() for part in parts).partition(" ") keyword, arguments = keyword.upper(), arguments.strip() - heredocs = [] + heredocs, complete = [], True if keyword in _HEREDOC_KEYWORDS: for strip_tabs, _, name in _HEREDOC.findall(arguments): - body = [] + body, closed = [], False while at < len(lines): line = lines[at] text.append(line) at += 1 if (line.rstrip("\r\n").lstrip("\t") if strip_tabs else line.rstrip("\r\n")) == name: + closed = True break body.append(line) heredocs.append("".join(body)) - found.append(Instruction(keyword, arguments, "".join(text), tuple(heredocs))) + complete = complete and closed + found.append(Instruction(keyword, arguments, "".join(text), tuple(heredocs), complete)) return found diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index f1e3ac15..a4d1f488 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -161,6 +161,13 @@ def test_what_modal_cant_take_is_written_as_its_docker_equivalent(tmp_path, line f"FROM python\n{built}\nRUN true\n" +def test_a_comment_among_the_lines_of_a_rewritten_instruction_is_kept_above_it(tmp_path): + (tmp_path / "sub").mkdir() + + assert modal_image_build.modal_dockerfile("FROM python\nADD sub \\\n # the vendored copy\n /added/\n", + tmp_path) == "FROM python\n # the vendored copy\nCOPY sub /added/\n" + + @pytest.mark.parametrize("line", [ 'COPY ["with space.txt", "/z/"]', "ADD archive.tar.gz /unpacked/", "ADD https://example.com/f /f", "ADD --checksum=sha256:abc a.txt /c", "ADD missing.txt /m", "COPY --from=build /out /out", "COPY a.txt /x/", diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index d64ccd73..528dc1d0 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -38,6 +38,15 @@ def test_a_build_that_reads_all_of_the_context_or_cant_be_told_says_why(dockerfi assert context_reads(f"FROM python\n{dockerfile}") == ContextReads(whole=why) +def test_a_shell_here_string_leaves_the_copies_after_it_read(): + assert context_reads("FROM x\nRUN cat << /out\nCOPY a.txt /a.txt\n") == ContextReads(paths=("a.txt",)) + + +def test_a_heredoc_left_open_reads_all_of_the_context_rather_than_miss_a_copy_it_swallowed(): + assert context_reads("FROM x\nRUN < Path: """A context tar.gz holding ``files`` and ``links``, everything in it as another tool might have written it.""" @@ -140,6 +149,26 @@ def test_what_the_os_or_python_leaves_behind_in_a_copied_folder_is_left_out(tmp_ assert set(_files(tmp_path / "out")) == {"Dockerfile", "lib/mod.py"} +def test_a_source_that_matches_nothing_in_the_context_keeps_all_of_it_and_says_which(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY app/ /app/\nCOPY gone.txt /gone\n", "app/main.py": "m", "other.py": "o"} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "app/main.py", "other.py"} + assert pruned[tmp_path / "out"].whole == "gone.txt matches nothing in the context" + + +def test_a_chain_of_kept_links_keeps_what_its_last_link_points_at(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY current /app\n", "releases/v3/app.py": "3", "releases/v2/app.py": "2"} + archive = _archive(tmp_path / "ctx.tar.gz", files, links={"current": "stable", "stable": "releases/v3"}) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "releases/v3/app.py"} + assert (tmp_path / "out" / "stable").is_symlink() + + def test_a_dockerfile_missing_from_the_archive_is_refused(tmp_path): archive = _archive(tmp_path / "ctx.tar.gz", {"a.py": "a"}) diff --git a/tst/unit/utils/dockerfile_test.py b/tst/unit/utils/dockerfile_test.py index 2dd60ea5..a34e7c84 100644 --- a/tst/unit/utils/dockerfile_test.py +++ b/tst/unit/utils/dockerfile_test.py @@ -48,6 +48,21 @@ def test_a_directive_after_the_first_instruction_is_a_comment(): assert [(i.keyword, i.arguments) for i in found if i.keyword] == [("FROM", "x"), ("COPY", "a `"), ("B", "")] +def test_a_shell_here_string_is_no_heredoc_and_the_instructions_after_it_stand(): + # docker build runs ``RUN cat << /out\nCOPY a.txt /a.txt\nword\n") + + assert [(i.keyword, i.heredocs) for i in found if i.keyword] == [("FROM", ()), ("RUN", ()), ("COPY", ()), + ("WORD", ())] + + +def test_a_heredoc_left_open_runs_to_the_end_and_says_so(): + found = [i for i in instructions("FROM x\nRUN < Date: Sun, 18 Oct 2026 22:54:58 -0700 Subject: [PATCH 03/14] fix(dockerfile): read a tab, a continued word and a quoted heredoc as docker build does An instruction's name is split from its arguments on any whitespace, so COPYapp is a COPY again. An escaped line break is removed with nothing put in its place, as docker build removes it, so a path broken across lines stays one path. A heredoc opens only outside quotes and only in shell form: docker build writes a quoted < --- src/agent_env/utils/dockerfile.py | 41 +++++++++++++++---- .../modal_context_build_test.py | 3 +- tst/unit/utils/build_context_prune_test.py | 7 ++++ tst/unit/utils/dockerfile_test.py | 23 +++++++++-- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py index 7fa01b16..d8f23e79 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/dockerfile.py @@ -19,7 +19,7 @@ class Instruction: ``text`` is what the Dockerfile holds for it, so the texts of all of them joined are the Dockerfile.""" keyword: str | None # upper case - arguments: str # on one line: escaped line breaks and the comment lines among them removed, heredoc bodies left out + arguments: str # on one line, escaped line breaks removed as docker build removes them, heredoc bodies left out text: str heredocs: tuple[str, ...] = () # each heredoc's body, in order complete: bool = True # False when a heredoc it opens runs to the end of the Dockerfile unclosed @@ -46,6 +46,8 @@ def instructions(dockerfile: str) -> list[Instruction]: if directive.group(1).lower() == "escape": escape = directive.group(2) at += 1 + # The escape character ending a line, spaces or tabs after it, joins the next line on as it is. + continued = re.compile(re.escape(escape) + r"[ \t]*$") found = [Instruction(None, "", line) for line in lines[:at]] while at < len(lines): line = lines[at] @@ -60,16 +62,16 @@ def instructions(dockerfile: str) -> list[Instruction]: at += 1 if parts and (not line.strip() or line.lstrip().startswith("#")): continue - body = line.rstrip("\r\n").rstrip() - if not body.endswith(escape): + body = line.rstrip("\r\n") + if not (end := continued.search(body)): parts.append(body) break - parts.append(body[:-len(escape)]) - keyword, _, arguments = " ".join(part.strip() for part in parts).partition(" ") - keyword, arguments = keyword.upper(), arguments.strip() + parts.append(body[:end.start()]) + keyword, *rest = "".join(parts).split(None, 1) + keyword, arguments = keyword.upper(), rest[0].strip() if rest else "" heredocs, complete = [], True if keyword in _HEREDOC_KEYWORDS: - for strip_tabs, _, name in _HEREDOC.findall(arguments): + for strip_tabs, name in _heredoc_names(arguments): body, closed = [], False while at < len(lines): line = lines[at] @@ -85,6 +87,29 @@ def instructions(dockerfile: str) -> list[Instruction]: return found +def _heredoc_names(arguments: str) -> list[tuple[str, str]]: + """Each heredoc ``arguments`` opens, as (its ``-`` if any, its name): a ``< CopyArguments | None: """A COPY or ADD's arguments; None for any other instruction, or one whose arguments it can't read: JSON that isn't a list of at least two strings, a quoted shell-form path, or fewer than a source and a destination.""" @@ -109,6 +134,6 @@ def copy_arguments(instruction: Instruction) -> CopyArguments | None: args = rest.split() if len(args) < 2: return None - sources = tuple(source for source in args[:-1] if not _HEREDOC.fullmatch(source)) + sources = tuple(source for source in args[:-1] if json_form or not _HEREDOC.fullmatch(source)) return CopyArguments(instruction.keyword, tuple(flags), sources, args[-1], json_form, heredoc=len(sources) < len(args) - 1) diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index a4d1f488..2de14cfc 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -146,13 +146,14 @@ async def test_a_failed_modal_build_keeps_modals_pointer_to_its_build_log(local_ @pytest.mark.parametrize("line, built", [ ('COPY ["a.txt", "/x/a.txt"]', "COPY a.txt /x/a.txt"), + ('COPY\t["a.txt", "/t/"]', "COPY a.txt /t/"), ('copy --chown=1:1 ["a.txt", "sub", "/y/"]', "COPY --chown=1:1 a.txt sub /y/"), ("ADD a.txt /added/", "COPY a.txt /added/"), ("ADD sub \\\n /added/sub/", "COPY sub /added/sub/"), ("ADD *.txt /globbed/", "COPY *.txt /globbed/"), ('ADD ["a.txt", "/j/"]', "COPY a.txt /j/"), ("ADD --chmod=755 a.txt /c", "COPY --chmod=755 a.txt /c"), -], ids=["json-copy", "json-copy-flags", "add-file", "add-folder-continued", "add-glob", "json-add", "add-chmod"]) +], ids=["json-copy", "json-copy-tab", "json-copy-flags", "add-file", "add-folder-continued", "add-glob", "json-add", "add-chmod"]) def test_what_modal_cant_take_is_written_as_its_docker_equivalent(tmp_path, line, built): (tmp_path / "a.txt").write_text("a") (tmp_path / "sub").mkdir() diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index 528dc1d0..2b1d5ca5 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -38,6 +38,13 @@ def test_a_build_that_reads_all_of_the_context_or_cant_be_told_says_why(dockerfi assert context_reads(f"FROM python\n{dockerfile}") == ContextReads(whole=why) +def test_a_tab_a_continued_path_and_a_quoted_heredoc_are_read_as_docker_reads_them(): + dockerfile = ('FROM x\nCOPY\ta.txt /tab.txt\nCOPY pack\\\nage.json /joined.json\nRUN echo "< /q\n' + 'RUN ["sh", "-c", "cat < /out\nCOPY a.txt /a.txt\n") == ContextReads(paths=("a.txt",)) diff --git a/tst/unit/utils/dockerfile_test.py b/tst/unit/utils/dockerfile_test.py index a34e7c84..f6efebfb 100644 --- a/tst/unit/utils/dockerfile_test.py +++ b/tst/unit/utils/dockerfile_test.py @@ -28,8 +28,8 @@ def test_the_instructions_joined_are_the_dockerfile_and_each_reads_as_docker_rea assert "".join(instruction.text for instruction in found) == DOCKERFILE assert [(i.keyword, i.arguments) for i in found if i.keyword] == [ ("FROM", "python:3.12-slim AS base"), - ("RUN", "apt-get update && apt-get install -y curl"), - ("COPY", "--chown=1:1 a.txt b/ /app/"), + ("RUN", "apt-get update && apt-get install -y curl"), + ("COPY", "--chown=1:1 a.txt b/ /app/"), ("RUN", "< /q\nRUN ["sh", "-c", "cat < /a\n' + 'COPY a.txt /a.txt\n') + + assert [(i.keyword, i.heredocs) for i in found if i.keyword] == [("FROM", ()), ("RUN", ()), ("RUN", ()), ("RUN", ()), + ("COPY", ())] + + def test_a_shell_here_string_is_no_heredoc_and_the_instructions_after_it_stand(): # docker build runs ``RUN cat << /out\nCOPY a.txt /a.txt\nword\n") From 64c48887489e668feb7c178fe1afc956a2974964 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 18 Oct 2026 23:29:33 -0700 Subject: [PATCH 04/14] fix(build-context): follow a link a source's path crosses, as docker build does A source whose leading parts are a link in the context (COPY current/app.py, current -> releases/v2) is followed through it, keeping the links it crosses and what they lead to, instead of keeping the whole context. A link out of the context or round in a loop still keeps all of it. The integration test copies through a link and checks the images built from the whole and the pruned context still hold the same files. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/build_context.py | 41 ++++++++++++++++--- .../utils/build_context_prune_test.py | 5 ++- tst/unit/utils/build_context_prune_test.py | 21 ++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index 792b8bc6..594d5cf6 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -178,11 +178,11 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned raise ValueError(f"the build context has no {dockerfile}") ignore = next((name for name in _ignore_files(dockerfile) if name in texts), None) matcher = FilePatternMatcher(*_patterns(texts[ignore])) if ignore else None - reads = context_reads(texts[dockerfile]) + reads, crossed = _through_links(context_reads(texts[dockerfile]), links) # A source the reading found that names nothing here is likelier a misreading than a build that fails. if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): - reads = ContextReads(whole=f"{missing} matches nothing in the context") - plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher), reads.whole) + reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() + plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole) counts = {folder: [0, 0] for folder in images} with tarfile.open(archive, "r:gz") as tar: for member in tar: @@ -249,8 +249,39 @@ def _context_path(source: str) -> str | None: return None if path == "." or path == ".." or path.startswith("../") else path +def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tuple[ContextReads, set[str]]: + """``reads`` with each source that names nothing itself followed through the links its leading parts name, as + docker build follows them, and the links it crossed. A wildcard source is left as it is.""" + if reads.whole is not None: + return reads, set() + paths, crossed = [], set() + for path in reads.paths: + if any(char in path for char in "*?[") or any(_matches(path, name) for name in links): + paths.append(path) + continue + resolved, hops = _resolve(path, links) + paths.append(resolved) + crossed |= hops + return ContextReads(paths=tuple(paths)), crossed + + +def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]]: + """``path`` with each part that is a link in the context replaced by where it leads, and the links that took; as it + is, crossing nothing, when a link leads out of the context or round in a loop.""" + resolved, crossed = "", set() + for part in path.split("/"): + resolved = posixpath.join(resolved, part) + while (target := links.get(resolved)) is not None: + following = posixpath.normpath(posixpath.join(posixpath.dirname(resolved), target)) + if target.startswith("/") or following == ".." or following.startswith("../") or resolved in crossed: + return path, set() + crossed.add(resolved) + resolved = following + return resolved, crossed + + def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, - matcher: FilePatternMatcher | None) -> set[str]: + matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: """The names in ``links`` (each name's link target, None when it's no link) that a build reads, as ``write_pruned`` keeps them, and the folders above them. What the OS or Python leaves behind is dropped, as ``BuildContext`` drops it.""" def sent(name: str) -> bool: @@ -260,7 +291,7 @@ def sent(name: str) -> bool: kept = {name for name in links if sent(name)} else: kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} - kept |= {name for name in (dockerfile, ignore) if name in links} + kept |= {name for name in (dockerfile, ignore) if name in links} | {name for name in crossed if sent(name)} names = sorted(links) pending = [name for name in kept if links[name]] while pending: diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/build_context_prune_test.py index 03e18d39..92297280 100644 --- a/tst/integration/utils/build_context_prune_test.py +++ b/tst/integration/utils/build_context_prune_test.py @@ -28,6 +28,7 @@ COPY conf/*.cfg /etc/svc/ ADD vendor.tar.gz /vendor/ COPY current/ /release/ +COPY current/app.txt /through-link.txt RUN --mount=type=bind,source=scripts,target=/scripts cp /scripts/run.sh /out/run.sh """ # What the build reads, beside a context ten times bigger it never does. @@ -81,7 +82,7 @@ def _image_files(tag: str) -> dict[str, tuple[int, str]]: found = {} with tarfile.open(fileobj=io.BytesIO(exported)) as tar: for member in tar: - if member.name.split("/", 1)[0] not in ("app", "out", "etc", "vendor", "release"): + if member.name.split("/", 1)[0] not in ("app", "out", "etc", "vendor", "release", "through-link.txt"): continue content = hashlib.sha256(tar.extractfile(member).read()).hexdigest() if member.isreg() else member.linkname found[member.name] = (member.mode, content) @@ -119,5 +120,5 @@ def test_an_image_built_from_the_pruned_context_holds_what_one_from_the_whole_co built = _image_files(docker[1]) assert built == _image_files(docker[0]) assert {"app/main.py", "out/built.txt", "out/run.sh", "etc/svc/a.cfg", "vendor/lib/vendored.py", - "release/app.txt"} <= set(built) + "release/app.txt", "through-link.txt"} <= set(built) assert not any(name.startswith("app/tests") or name.endswith(".pyc") for name in built) diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index 2b1d5ca5..1b7a49e4 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -136,6 +136,27 @@ def test_a_kept_link_keeps_what_it_points_at_in_the_context_and_a_link_out_of_it assert not (tmp_path / "out" / "out").exists() +def test_a_source_reached_through_a_link_keeps_the_link_and_what_it_leads_to(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY current/app.py /app.py\n", "releases/v2/app.py": "2", + "releases/v1/app.py": "1"} + archive = _archive(tmp_path / "ctx.tar.gz", files, links={"current": "stable", "stable": "releases/v2"}) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "releases/v2/app.py"} + assert (tmp_path / "out" / "current").is_symlink() and (tmp_path / "out" / "stable").is_symlink() + assert pruned[tmp_path / "out"].whole is None + + +def test_a_link_loop_or_one_out_of_the_context_in_a_source_keeps_all_of_it(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY loop/a.py /a.py\nCOPY out/b.py /b.py\n", "other.py": "o"} + archive = _archive(tmp_path / "ctx.tar.gz", files, links={"loop": "loop2", "loop2": "loop", "out": "../elsewhere"}) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert pruned[tmp_path / "out"].whole == "loop/a.py matches nothing in the context" + + def test_wildcards_match_one_part_of_a_path_as_dockers_do(tmp_path): files = {"Dockerfile": "FROM python\nCOPY *.txt [^b]*.cfg /x/\n", "a.txt": "a", "sub/c.txt": "c", "a.cfg": "a", "b.cfg": "b"} From fcd4745f94668cfda4fd85d40ae928fd617fd41e Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 00:00:50 -0700 Subject: [PATCH 05/14] fix(build-context): read a linked Dockerfile, Docker's wildcard classes, and every flag as docker build does A Dockerfile or ignore file stored as a link is read where it leads. Wildcards follow Go's filepath.Match, which docker build matches COPY sources with: ! in a class is a character and ^ negates it. Flags split on any whitespace outside quotes, and a bind mount's options are one CSV record, so a tab between flags or a quoted source with a space keeps the folder it mounts. Arguments are exec form only when they parse as a JSON list of strings, so a shell-form COPY [!a].txt is read as one. The Modal rewrite leaves a JSON path like < --- .../sandbox_providers/modal_image_build.py | 5 +- src/agent_env/utils/build_context.py | 83 +++++++++++++++++-- src/agent_env/utils/dockerfile.py | 58 +++++++++---- .../utils/build_context_prune_test.py | 22 +++-- .../modal_context_build_test.py | 4 +- tst/unit/utils/build_context_prune_test.py | 37 +++++++++ tst/unit/utils/dockerfile_test.py | 9 +- 7 files changed, 179 insertions(+), 39 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_image_build.py b/src/agent_env/providers/sandbox_providers/modal_image_build.py index a6288c17..b8a79229 100644 --- a/src/agent_env/providers/sandbox_providers/modal_image_build.py +++ b/src/agent_env/providers/sandbox_providers/modal_image_build.py @@ -16,7 +16,7 @@ from agent_env.config import get_config from agent_env.utils.build_context import extract, ignore_file -from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions +from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions, opens_heredoc if TYPE_CHECKING: from agent_env.artifact.artifacts.docker_image import DockerImageArtifact @@ -93,7 +93,8 @@ def _for_modal(instruction: Instruction, context: Path) -> str: return instruction.text paths = (*args.sources, args.destination) if args.json_form: - if not all(path and not any(c.isspace() for c in path) for path in paths): + # Written in shell form, a path with whitespace would split, and one like < dict[Path, Pruned links[name] = member.linkname if member.issym() else None if name in wanted and member.isreg(): texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + # A Dockerfile or ignore file stored as a link is read where it leads, as docker build reads it. + linked = {name: _resolve(name, links)[0] for name in wanted if links.get(name)} + if unread := {target for target in linked.values() if target not in texts}: + texts |= _texts(archive, unread) + texts |= {name: texts[target] for name, target in linked.items() if target in texts} plans: dict[Path, tuple[set[str], str | None]] = {} for folder, dockerfile in dockerfiles.items(): if dockerfile not in texts: @@ -202,6 +209,16 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned return {folder: Pruned(*counts[folder], plans[folder][1]) for folder in images} +def _texts(archive: Path, names: set[str]) -> dict[str, str]: + """The text of each regular file ``names`` names in the tar.gz at ``archive``.""" + texts = {} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + if member.isreg() and (name := _member_name(member)) in names: + texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + return texts + + def _copy_reads(instruction: Instruction, paths: list[str]) -> str | None: """Add the context paths a COPY or ADD reads to ``paths``; why it reads all of the context, when it does.""" args = copy_arguments(instruction) @@ -225,13 +242,14 @@ def _copy_reads(instruction: Instruction, paths: list[str]) -> str | None: def _mount_reads(instruction: Instruction, paths: list[str]) -> str | None: """Add the context paths a RUN's bind mounts read to ``paths``; why it reads all of the context, when it does. A bind mount names no other image or stage with ``from``, and its source defaults to the whole context.""" - rest = instruction.arguments - while rest.startswith("--"): - flag, _, rest = rest.partition(" ") - rest = rest.strip() + for flag in leading_flags(instruction.arguments)[0]: if not flag.startswith("--mount="): continue - options = dict(option.partition("=")[::2] for option in flag.removeprefix("--mount=").split(",")) + # A mount's options are one CSV record, as docker build reads them, so a quoted field holds a comma or space. + options = {} + for field in next(csv.reader([flag.removeprefix("--mount=")])): + key, _, value = field.partition("=") + options[key.strip()] = value.strip().strip("\"'") if options.get("type", "bind") != "bind" or "from" in options: continue source = options.get("source") or options.get("src") or "." @@ -256,7 +274,7 @@ def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tupl return reads, set() paths, crossed = [], set() for path in reads.paths: - if any(char in path for char in "*?[") or any(_matches(path, name) for name in links): + if _WILDCARD.search(path) or any(_matches(path, name) for name in links): paths.append(path) continue resolved, hops = _resolve(path, links) @@ -319,7 +337,54 @@ def _at_or_under(names: list[str], path: str) -> list[str]: def _matches(path: str, name: str) -> bool: """Whether ``name`` is ``path`` or under it, each part matched as Docker matches a COPY source's.""" want, have = path.split("/"), name.split("/") - return len(have) >= len(want) and all(fnmatchcase(part, glob.replace("[^", "[!")) for part, glob in zip(have, want)) + return len(have) >= len(want) and all( + part == glob if not _WILDCARD.search(glob) else bool((pattern := _part_pattern(glob)) and pattern.fullmatch(part)) + for part, glob in zip(have, want)) + + +_WILDCARD = re.compile(r"[*?\[\\]") + + +@functools.lru_cache(maxsize=1024) +def _part_pattern(glob: str) -> re.Pattern[str] | None: + """A path part's wildcard pattern as Go's ``filepath.Match``, which docker build matches COPY sources with, reads + it: ``*`` any characters and ``?`` any one, ``[...]`` a class that ``^`` negates (``!`` is a character like any + other), and ``\\`` the next character as it is. None when the pattern is malformed, which docker build refuses.""" + out, at = [], 0 + while at < len(glob): + char = glob[at] + if char == "*": + out.append("[^/]*") + elif char == "?": + out.append("[^/]") + elif char == "\\": + if at + 1 == len(glob): + return None + at += 1 + out.append(re.escape(glob[at])) + elif char == "[": + at += 1 + negated = at < len(glob) and glob[at] == "^" + at += negated + members = [] + while at < len(glob) and glob[at] != "]": + if glob[at] == "\\": + at += 1 + if at == len(glob): + return None + members.append(re.escape(glob[at])) + elif glob[at] == "-" and members and at + 1 < len(glob) and glob[at + 1] != "]": + members.append("-") + else: + members.append(re.escape(glob[at])) + at += 1 + if at == len(glob) or not members: + return None + out.append(f"[{'^' if negated else ''}{''.join(members)}]") + else: + out.append(re.escape(char)) + at += 1 + return re.compile("".join(out)) def _ignore_files(dockerfile: str) -> tuple[str, ...]: diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py index d8f23e79..c2ff74da 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/dockerfile.py @@ -87,10 +87,43 @@ def instructions(dockerfile: str) -> list[Instruction]: return found +def leading_flags(arguments: str) -> tuple[list[str], str]: + """The ``--flag`` words ``arguments`` begins with, split on any whitespace outside quotes, and the rest of it.""" + flags, at = [], 0 + while arguments.startswith("--", at): + end, quote = at, None + while end < len(arguments) and (quote or not arguments[end].isspace()): + char = arguments[end] + quote = (None if char == quote else quote) if quote else (char if char in "\"'" else None) + end += 1 + flags.append(arguments[at:end]) + at = end + while at < len(arguments) and arguments[at].isspace(): + at += 1 + return flags, arguments[at:] + + +def _json_strings(arguments: str) -> list[str] | None: + """``arguments`` as the JSON list of strings of an exec-form instruction; None when it's shell form, as docker + build reads anything that doesn't parse as one, a ``[`` it opens with included.""" + if not arguments.startswith("["): + return None + try: + parsed = json.loads(arguments) + except ValueError: + return None + return parsed if isinstance(parsed, list) and all(isinstance(arg, str) for arg in parsed) else None + + +def opens_heredoc(word: str) -> bool: + """Whether ``word``, written in a shell-form instruction, would open a heredoc.""" + return _HEREDOC.fullmatch(word) is not None + + def _heredoc_names(arguments: str) -> list[tuple[str, str]]: """Each heredoc ``arguments`` opens, as (its ``-`` if any, its name): a ``< list[tuple[str, str]]: def copy_arguments(instruction: Instruction) -> CopyArguments | None: - """A COPY or ADD's arguments; None for any other instruction, or one whose arguments it can't read: JSON that - isn't a list of at least two strings, a quoted shell-form path, or fewer than a source and a destination.""" + """A COPY or ADD's arguments; None for any other instruction, or one whose arguments it can't read: a quoted + shell-form path, or fewer than a source and a destination. Arguments that don't parse as a JSON list of strings are + shell form, as docker build reads them.""" if instruction.keyword not in ("COPY", "ADD"): return None - rest, flags = instruction.arguments, [] - while rest.startswith("--"): - flag, _, rest = rest.partition(" ") - flags.append(flag) - rest = rest.strip() - json_form = rest.startswith("[") - if json_form: - try: - args = json.loads(rest) - except ValueError: - return None - if not (isinstance(args, list) and all(isinstance(arg, str) for arg in args)): - return None - else: + flags, rest = leading_flags(instruction.arguments) + args = _json_strings(rest) + json_form = args is not None + if not json_form: if any(quote in rest for quote in "\"'"): return None args = rest.split() diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/build_context_prune_test.py index 92297280..fad79e2b 100644 --- a/tst/integration/utils/build_context_prune_test.py +++ b/tst/integration/utils/build_context_prune_test.py @@ -29,11 +29,14 @@ ADD vendor.tar.gz /vendor/ COPY current/ /release/ COPY current/app.txt /through-link.txt +COPY lit/[!a].txt /lit/ +RUN --network=none --mount=type=bind,"source=my dir",target=/m cp /m/f /out/spaced.txt RUN --mount=type=bind,source=scripts,target=/scripts cp /scripts/run.sh /out/run.sh """ # What the build reads, beside a context ten times bigger it never does. CONTEXT = { - "server/Dockerfile": DOCKERFILE, + "server/real.Dockerfile": DOCKERFILE, + "lit/!.txt": "bang\n", "lit/a.txt": "a\n", "lit/b.txt": "b\n", "my dir/f": "spaced\n", ".dockerignore": "app/tests\n**/*.pyc\n", "tools/gen.sh": "echo generated\n", "app/main.py": "print('main')\n", "app/util.py": "u = 1\n", "app/tests/test_main.py": "assert True\n", "app/cache.pyc": "compiled", "conf/a.cfg": "a=1\n", @@ -60,9 +63,10 @@ def _stored_context(path: Path) -> Path: for name, text in CONTEXT.items(): _add(tar, name, text.encode(), 0o755 if name.endswith(".sh") else 0o644) _add(tar, "vendor.tar.gz", vendor.getvalue()) - link = tarfile.TarInfo("current") - link.type, link.linkname = tarfile.SYMTYPE, "releases/v2" - tar.addfile(link) + for name, target in (("current", "releases/v2"), ("server/Dockerfile", "real.Dockerfile")): + link = tarfile.TarInfo(name) + link.type, link.linkname = tarfile.SYMTYPE, target + tar.addfile(link) return path @@ -82,7 +86,7 @@ def _image_files(tag: str) -> dict[str, tuple[int, str]]: found = {} with tarfile.open(fileobj=io.BytesIO(exported)) as tar: for member in tar: - if member.name.split("/", 1)[0] not in ("app", "out", "etc", "vendor", "release", "through-link.txt"): + if member.name.split("/", 1)[0] not in ("app", "out", "etc", "vendor", "release", "through-link.txt", "lit"): continue content = hashlib.sha256(tar.extractfile(member).read()).hexdigest() if member.isreg() else member.linkname found[member.name] = (member.mode, content) @@ -109,8 +113,9 @@ def test_an_image_built_from_the_pruned_context_holds_what_one_from_the_whole_co kept = {str(path.relative_to(tmp_path / "pruned")) for path in (tmp_path / "pruned").rglob("*") if path.is_file() or path.is_symlink()} - assert kept == {"server/Dockerfile", ".dockerignore", "tools/gen.sh", "app/main.py", "app/util.py", "conf/a.cfg", - "conf/b.cfg", "vendor.tar.gz", "current", "releases/v2/app.txt", "scripts/run.sh"} + assert kept == {"server/Dockerfile", "server/real.Dockerfile", ".dockerignore", "tools/gen.sh", "app/main.py", + "app/util.py", "conf/a.cfg", "conf/b.cfg", "vendor.tar.gz", "current", "releases/v2/app.txt", + "scripts/run.sh", "lit/!.txt", "lit/a.txt", "my dir/f"} assert pruned[tmp_path / "pruned"].whole is None suffix = uuid.uuid4().hex[:8] docker.extend([f"prune-full-{suffix}", f"prune-kept-{suffix}"]) @@ -120,5 +125,6 @@ def test_an_image_built_from_the_pruned_context_holds_what_one_from_the_whole_co built = _image_files(docker[1]) assert built == _image_files(docker[0]) assert {"app/main.py", "out/built.txt", "out/run.sh", "etc/svc/a.cfg", "vendor/lib/vendored.py", - "release/app.txt", "through-link.txt"} <= set(built) + "release/app.txt", "through-link.txt", "lit/!.txt", "lit/a.txt", "out/spaced.txt"} <= set(built) + assert "lit/b.txt" not in built assert not any(name.startswith("app/tests") or name.endswith(".pyc") for name in built) diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index 2de14cfc..310e653b 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -172,9 +172,9 @@ def test_a_comment_among_the_lines_of_a_rewritten_instruction_is_kept_above_it(t @pytest.mark.parametrize("line", [ 'COPY ["with space.txt", "/z/"]', "ADD archive.tar.gz /unpacked/", "ADD https://example.com/f /f", "ADD --checksum=sha256:abc a.txt /c", "ADD missing.txt /m", "COPY --from=build /out /out", "COPY a.txt /x/", - 'ADD "a.txt" /q/', "ADD < /out\nCOPY a.txt /a.txt\nword\n") From c92418e288d645214fe3af54b4c4bec9a1612a87 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 07:59:09 -0700 Subject: [PATCH 06/14] feat(build-context): find the Dockerfile an image names none for, and report each image's largest files An image given no Dockerfile path builds the context's Dockerfile, or, with none at its root, the only one in it; a context holding several or none is refused, naming them, and Pruned.dockerfile says which was used. Pruned.largest names each folder's five biggest files, so an import can show what it wrote before a folder is shared. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/build_context.py | 47 +++++++++++++++++----- tst/unit/utils/build_context_prune_test.py | 45 +++++++++++++++++++++ 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index ec256b1e..bb743fb5 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -7,6 +7,7 @@ import csv import functools import gzip +import heapq import hashlib import json import os @@ -126,12 +127,14 @@ class ContextReads: @dataclass(frozen=True) class Pruned: - """What ``write_pruned`` wrote into a folder: ``files`` files of ``size`` bytes, and why it wrote everything the - context sends when it did.""" + """What ``write_pruned`` wrote into a folder for the Dockerfile at ``dockerfile`` in it: ``files`` files of + ``size`` bytes, the biggest of them by bytes, and why it wrote everything the context sends when it did.""" + dockerfile: str files: int size: int whole: str | None + largest: tuple[tuple[str, int], ...] = () # The COPY and ADD flags that change how a source is written, not which one it is. @@ -156,14 +159,15 @@ def context_reads(dockerfile: str) -> ContextReads: return ContextReads(paths=tuple(dict.fromkeys(paths))) -def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned]: +def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, Pruned]: """Write into each folder ``images`` names the part of the build context tar.gz at ``archive`` that its Dockerfile, at the path ``images`` gives in the context, reads: what ``context_reads`` finds, less what the context's ignore file leaves out and what the OS or Python leaves behind, with the Dockerfile, the ignore file - and the targets of kept links. Reads the - archive twice and never unpacks the rest of it. Raises ValueError when a Dockerfile isn't in the archive, or on a - member ``extract`` refuses.""" - dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items()} + and the targets of kept links. An image with no path given builds the context's ``Dockerfile``, or, with none at + its root, the only one in it. Reads the archive twice, a third time for a Dockerfile found or reached through a + link, and never unpacks the rest of it. Raises ValueError when a Dockerfile isn't in the archive or can't be told + apart from others, or on a member ``extract`` refuses.""" + dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items() if dockerfile} wanted = {name for dockerfile in dockerfiles.values() for name in (dockerfile, *_ignore_files(dockerfile))} links: dict[str, str | None] = {} texts: dict[str, str] = {} @@ -174,6 +178,11 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned links[name] = member.linkname if member.issym() else None if name in wanted and member.isreg(): texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + for folder in images.keys() - dockerfiles.keys(): + dockerfiles[folder] = _found_dockerfile(links) + wanted |= {dockerfiles[folder], *_ignore_files(dockerfiles[folder])} + if unread := {name for name in wanted if name in links and not links[name] and name not in texts}: + texts |= _texts(archive, unread) # A Dockerfile or ignore file stored as a link is read where it leads, as docker build reads it. linked = {name: _resolve(name, links)[0] for name in wanted if links.get(name)} if unread := {target for target in linked.values() if target not in texts}: @@ -190,7 +199,7 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole) - counts = {folder: [0, 0] for folder in images} + written: dict[Path, list[tuple[str, int]]] = {folder: [] for folder in images} with tarfile.open(archive, "r:gz") as tar: for member in tar: name = _member_name(member) @@ -204,9 +213,25 @@ def write_pruned(archive: Path, images: Mapping[Path, str]) -> dict[Path, Pruned tar.extract(member, folder, filter=_context_member) first = folder / name if member.isreg() else None if member.isreg(): - counts[folder][0] += 1 - counts[folder][1] += member.size - return {folder: Pruned(*counts[folder], plans[folder][1]) for folder in images} + written[folder].append((name, member.size)) + return {folder: Pruned(dockerfiles[folder], len(files), sum(size for _, size in files), plans[folder][1], + tuple(heapq.nlargest(_LARGEST, files, key=lambda file: file[1]))) + for folder, files in written.items()} + + +# How many of an image's biggest files its Pruned names, for an import to show what it wrote. +_LARGEST = 5 + + +def _found_dockerfile(links: Mapping[str, str | None]) -> str: + """The Dockerfile an image that names none builds: the context's ``Dockerfile``, else the only one in it.""" + if "Dockerfile" in links: + return "Dockerfile" + found = sorted(name for name in links if posixpath.basename(name) == "Dockerfile") + if len(found) != 1: + raise ValueError(f"the build context names no Dockerfile and holds {len(found)}" + f"{': ' + ', '.join(found) if found else ''}; name the one it builds") + return found[0] def _texts(archive: Path, names: set[str]) -> dict[str, str]: diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index 2fe51213..1797cad8 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -234,6 +234,51 @@ def test_a_chain_of_kept_links_keeps_what_its_last_link_points_at(tmp_path): assert (tmp_path / "out" / "stable").is_symlink() +def test_an_image_naming_no_dockerfile_builds_the_contexts_or_the_only_one_in_it(tmp_path): + root = _archive(tmp_path / "root.tar.gz", {"Dockerfile": "FROM python\nCOPY a /a\n", "a": "a", + "sub/Dockerfile": "FROM python\n"}) + nested = tmp_path / "nested.tar.gz" + with tarfile.open(nested, "w:gz") as tar: # the only Dockerfile, written three times as a stored context had it + for text in ("FROM python\n", "FROM python\nCOPY b /b\n", "FROM python\nCOPY app/ /app/\n"): + info = tarfile.TarInfo("claude_code/Dockerfile") + info.size = len(text) + tar.addfile(info, io.BytesIO(text.encode())) + info = tarfile.TarInfo("app/main.py") + info.size = 1 + tar.addfile(info, io.BytesIO(b"m")) + + from_root = write_pruned(root, {tmp_path / "root": None})[tmp_path / "root"] + from_nested = write_pruned(nested, {tmp_path / "nested": None})[tmp_path / "nested"] + + assert from_root.dockerfile == "Dockerfile" and set(_files(tmp_path / "root")) == {"Dockerfile", "a"} + assert from_nested.dockerfile == "claude_code/Dockerfile" + assert _files(tmp_path / "nested") == {"claude_code/Dockerfile": "FROM python\nCOPY app/ /app/\n", + "app/main.py": "m"} + + +@pytest.mark.parametrize(("files", "problem"), [ + ({"a/Dockerfile": "FROM x\n", "b/Dockerfile": "FROM x\n"}, "holds 2: a/Dockerfile, b/Dockerfile; name the one"), + ({"a.py": "a"}, "names no Dockerfile and holds 0; name the one it builds"), +], ids=["two", "none"]) +def test_an_image_naming_no_dockerfile_is_refused_when_the_context_doesnt_settle_which(tmp_path, files, problem): + archive = _archive(tmp_path / "ctx.tar.gz", files) + + with pytest.raises(ValueError, match=problem): + write_pruned(archive, {tmp_path / "out": None}) + + +def test_each_image_names_its_largest_files_for_the_report(tmp_path): + files = {"Dockerfile": "FROM python\nCOPY data/ /data/\n", + **{f"data/f{n}.bin": "x" * (100 * n) for n in range(1, 8)}, "big-unread.bin": "x" * 10_000} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"})[tmp_path / "out"] + + assert pruned.largest == (("data/f7.bin", 700), ("data/f6.bin", 600), ("data/f5.bin", 500), ("data/f4.bin", 400), + ("data/f3.bin", 300)) + assert pruned.dockerfile == "Dockerfile" + + def test_a_dockerfile_missing_from_the_archive_is_refused(tmp_path): archive = _archive(tmp_path / "ctx.tar.gz", {"a.py": "a"}) From bd62a61d4f8b1889153fa7eef24a7e66e8444807 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 08:19:49 -0700 Subject: [PATCH 07/14] fix(build-context): count a name the archive holds twice once, and keep docstrings to one line A name an archive holds more than once is written over, so each image's file count, size and largest files now use the entry left in its folder rather than every entry. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/build_context.py | 56 +++++++------------ src/agent_env/utils/dockerfile.py | 21 +++---- .../utils/build_context_prune_test.py | 4 +- tst/unit/utils/build_context_prune_test.py | 19 ++++++- 4 files changed, 44 insertions(+), 56 deletions(-) diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index bb743fb5..db2e64f3 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -118,8 +118,7 @@ def extract(archive: Path, out: Path) -> None: @dataclass(frozen=True) class ContextReads: - """What a Dockerfile's build reads of its context: the context paths in ``paths``, each of which may hold Docker's - wildcards, or all of it when ``whole`` says why: it copies all of it, or reads what can't be told before a build.""" + """The context paths a build reads, each of which may hold Docker's wildcards, or why it reads all of it.""" paths: tuple[str, ...] = () whole: str | None = None @@ -127,8 +126,7 @@ class ContextReads: @dataclass(frozen=True) class Pruned: - """What ``write_pruned`` wrote into a folder for the Dockerfile at ``dockerfile`` in it: ``files`` files of - ``size`` bytes, the biggest of them by bytes, and why it wrote everything the context sends when it did.""" + """What ``write_pruned`` wrote into one folder, and why it wrote all the context sends when it did.""" dockerfile: str files: int @@ -139,11 +137,12 @@ class Pruned: # The COPY and ADD flags that change how a source is written, not which one it is. _SOURCE_KEEPING_FLAGS = ("--chown=", "--chmod=", "--link", "--checksum=", "--keep-git-dir", "--unpack") +_WILDCARD = re.compile(r"[*?\[\\]") +_LARGEST = 5 def context_reads(dockerfile: str) -> ContextReads: - """What a build of ``dockerfile`` reads of its context: each local COPY and ADD source of every stage and each bind - mount's source, but not a COPY --from, a URL or git source, or a heredoc.""" + """The context paths a build of ``dockerfile`` reads: its local COPY and ADD sources and bind-mount sources.""" paths: list[str] = [] for instruction in instructions(dockerfile): if not instruction.complete: @@ -160,13 +159,7 @@ def context_reads(dockerfile: str) -> ContextReads: def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, Pruned]: - """Write into each folder ``images`` names the part of the build context tar.gz at ``archive`` that its - Dockerfile, at the path ``images`` gives in the context, reads: what ``context_reads`` finds, less what the - context's ignore file leaves out and what the OS or Python leaves behind, with the Dockerfile, the ignore file - and the targets of kept links. An image with no path given builds the context's ``Dockerfile``, or, with none at - its root, the only one in it. Reads the archive twice, a third time for a Dockerfile found or reached through a - link, and never unpacks the rest of it. Raises ValueError when a Dockerfile isn't in the archive or can't be told - apart from others, or on a member ``extract`` refuses.""" + """Write into each folder what its Dockerfile's build reads of the context tar.gz (None: the one it finds).""" dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items() if dockerfile} wanted = {name for dockerfile in dockerfiles.values() for name in (dockerfile, *_ignore_files(dockerfile))} links: dict[str, str | None] = {} @@ -199,7 +192,8 @@ def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole) - written: dict[Path, list[tuple[str, int]]] = {folder: [] for folder in images} + # By name, since a name the archive holds twice is written over: what's left is its last entry. + written: dict[Path, dict[str, int]] = {folder: {} for folder in images} with tarfile.open(archive, "r:gz") as tar: for member in tar: name = _member_name(member) @@ -213,16 +207,14 @@ def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, tar.extract(member, folder, filter=_context_member) first = folder / name if member.isreg() else None if member.isreg(): - written[folder].append((name, member.size)) - return {folder: Pruned(dockerfiles[folder], len(files), sum(size for _, size in files), plans[folder][1], - tuple(heapq.nlargest(_LARGEST, files, key=lambda file: file[1]))) + written[folder][name] = member.size + else: + written[folder].pop(name, None) + return {folder: Pruned(dockerfiles[folder], len(files), sum(files.values()), plans[folder][1], + tuple(heapq.nlargest(_LARGEST, files.items(), key=lambda file: file[1]))) for folder, files in written.items()} -# How many of an image's biggest files its Pruned names, for an import to show what it wrote. -_LARGEST = 5 - - def _found_dockerfile(links: Mapping[str, str | None]) -> str: """The Dockerfile an image that names none builds: the context's ``Dockerfile``, else the only one in it.""" if "Dockerfile" in links: @@ -265,8 +257,7 @@ def _copy_reads(instruction: Instruction, paths: list[str]) -> str | None: def _mount_reads(instruction: Instruction, paths: list[str]) -> str | None: - """Add the context paths a RUN's bind mounts read to ``paths``; why it reads all of the context, when it does. A - bind mount names no other image or stage with ``from``, and its source defaults to the whole context.""" + """Add the context paths a RUN's bind mounts read to ``paths``; why it reads all of the context, when it does.""" for flag in leading_flags(instruction.arguments)[0]: if not flag.startswith("--mount="): continue @@ -293,8 +284,7 @@ def _context_path(source: str) -> str | None: def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tuple[ContextReads, set[str]]: - """``reads`` with each source that names nothing itself followed through the links its leading parts name, as - docker build follows them, and the links it crossed. A wildcard source is left as it is.""" + """``reads`` with each source that names nothing followed through the links on its path, and the links crossed.""" if reads.whole is not None: return reads, set() paths, crossed = [], set() @@ -309,8 +299,7 @@ def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tupl def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]]: - """``path`` with each part that is a link in the context replaced by where it leads, and the links that took; as it - is, crossing nothing, when a link leads out of the context or round in a loop.""" + """``path`` with its links followed, and the links crossed; as it is when a link leads out or loops.""" resolved, crossed = "", set() for part in path.split("/"): resolved = posixpath.join(resolved, part) @@ -325,8 +314,7 @@ def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]] def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: - """The names in ``links`` (each name's link target, None when it's no link) that a build reads, as ``write_pruned`` - keeps them, and the folders above them. What the OS or Python leaves behind is dropped, as ``BuildContext`` drops it.""" + """The names in ``links`` (name to link target) that ``write_pruned`` keeps for ``reads``, and their folders.""" def sent(name: str) -> bool: return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) @@ -367,14 +355,9 @@ def _matches(path: str, name: str) -> bool: for part, glob in zip(have, want)) -_WILDCARD = re.compile(r"[*?\[\\]") - - @functools.lru_cache(maxsize=1024) def _part_pattern(glob: str) -> re.Pattern[str] | None: - """A path part's wildcard pattern as Go's ``filepath.Match``, which docker build matches COPY sources with, reads - it: ``*`` any characters and ``?`` any one, ``[...]`` a class that ``^`` negates (``!`` is a character like any - other), and ``\\`` the next character as it is. None when the pattern is malformed, which docker build refuses.""" + """A path part's wildcard as Go's ``filepath.Match``, which docker build uses, reads it; None when malformed.""" out, at = [], 0 while at < len(glob): char = glob[at] @@ -413,8 +396,7 @@ def _part_pattern(glob: str) -> re.Pattern[str] | None: def _ignore_files(dockerfile: str) -> tuple[str, ...]: - """Where ``docker build`` looks for its ignore file, in order: ``.dockerignore`` beside the - Dockerfile, then the context's ``.dockerignore``.""" + """Where ``docker build`` looks for a Dockerfile's ignore file, in order.""" return f"{dockerfile}.dockerignore", ".dockerignore" diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py index c2ff74da..bdc356bf 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/dockerfile.py @@ -9,20 +9,18 @@ _DIRECTIVE = re.compile(r"#\s*([A-Za-z][A-Za-z0-9_-]*)\s*=\s*(\S+)\s*") # ``<<<`` is a shell here-string, which docker build leaves to the shell, not a heredoc. _HEREDOC = re.compile(r"(? list[Instruction]: - """``dockerfile``'s instructions and the lines between them, in order: an instruction's escaped line breaks - continue it, a comment line among them is dropped from its arguments, and a heredoc's body is part of it.""" + """``dockerfile``'s instructions and the lines between them, in order, as ``docker build`` splits them.""" lines = dockerfile.splitlines(keepends=True) escape, at = "\\", 0 while at < len(lines) and (directive := _DIRECTIVE.fullmatch(lines[at].strip())): @@ -104,8 +101,7 @@ def leading_flags(arguments: str) -> tuple[list[str], str]: def _json_strings(arguments: str) -> list[str] | None: - """``arguments`` as the JSON list of strings of an exec-form instruction; None when it's shell form, as docker - build reads anything that doesn't parse as one, a ``[`` it opens with included.""" + """``arguments`` as an exec-form JSON list of strings; None when ``docker build`` reads it as shell form.""" if not arguments.startswith("["): return None try: @@ -121,8 +117,7 @@ def opens_heredoc(word: str) -> bool: def _heredoc_names(arguments: str) -> list[tuple[str, str]]: - """Each heredoc ``arguments`` opens, as (its ``-`` if any, its name): a ``< list[tuple[str, str]]: def copy_arguments(instruction: Instruction) -> CopyArguments | None: - """A COPY or ADD's arguments; None for any other instruction, or one whose arguments it can't read: a quoted - shell-form path, or fewer than a source and a destination. Arguments that don't parse as a JSON list of strings are - shell form, as docker build reads them.""" + """A COPY or ADD's arguments; None for another instruction, a quoted shell-form path, or fewer than two paths.""" if instruction.keyword not in ("COPY", "ADD"): return None flags, rest = leading_flags(instruction.arguments) diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/build_context_prune_test.py index fad79e2b..a69cce03 100644 --- a/tst/integration/utils/build_context_prune_test.py +++ b/tst/integration/utils/build_context_prune_test.py @@ -1,6 +1,4 @@ -"""An image built from the part of a context ``write_pruned`` keeps holds exactly the files one built from the whole -context does: a multi-stage build with wildcards, a local archive ADD unpacks, a bind mount, an ignore file and a linked -folder, from a context full of what the build never reads. Needs Docker.""" +"""An image built from what ``write_pruned`` keeps holds what one built from the whole context does. Needs Docker.""" import hashlib import io diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index 1797cad8..dc0092e0 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -1,5 +1,4 @@ -"""What a Dockerfile's build reads of its context, and writing each of several images only that part of a context -they share, from its tar.gz.""" +"""What a Dockerfile's build reads of its context, and ``write_pruned`` writing only that part.""" import io import tarfile @@ -279,6 +278,22 @@ def test_each_image_names_its_largest_files_for_the_report(tmp_path): assert pruned.dockerfile == "Dockerfile" +def test_a_name_the_archive_holds_twice_counts_once_at_the_size_left_in_the_folder(tmp_path): + archive = tmp_path / "ctx.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + for name, text in (("Dockerfile", "FROM python\nCOPY data/ /data/\n"), ("data/a.bin", "x" * 5000), + ("data/b.bin", "x" * 100), ("data/a.bin", "x" * 10)): + info = tarfile.TarInfo(name) + info.size = len(text) + tar.addfile(info, io.BytesIO(text.encode())) + + pruned = write_pruned(archive, {tmp_path / "out": "Dockerfile"})[tmp_path / "out"] + + assert (tmp_path / "out" / "data" / "a.bin").stat().st_size == 10 + assert (pruned.files, pruned.size) == (3, 30 + 100 + 10) + assert pruned.largest == (("data/b.bin", 100), ("Dockerfile", 30), ("data/a.bin", 10)) + + def test_a_dockerfile_missing_from_the_archive_is_refused(tmp_path): archive = _archive(tmp_path / "ctx.tar.gz", {"a.py": "a"}) From d2679c893d6e6eaa4e88ae4f4a71778c38a43d01 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 08:38:30 -0700 Subject: [PATCH 08/14] fix(build-context): keep the whole context for another frontend or an unknown mount type, and read mount options in any case - A `# syntax=` directive naming a frontend other than Docker's own keeps the whole context, since that frontend may read the context its own way. - A RUN --mount type that isn't bind, cache, tmpfs, secret or ssh keeps the whole context instead of being skipped. - Mount option names and the type are read in any case, as docker build reads them: `type=BIND` was skipped as a mount that reads nothing. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/build_context.py | 15 +++++++++--- src/agent_env/utils/dockerfile.py | 21 +++++++++++++---- .../utils/build_context_prune_test.py | 2 +- tst/unit/utils/build_context_prune_test.py | 23 +++++++++++++++++++ tst/unit/utils/dockerfile_test.py | 7 +++++- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index db2e64f3..a37a3c3d 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -23,7 +23,7 @@ from modal import FilePatternMatcher -from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions, leading_flags +from agent_env.utils.dockerfile import Instruction, copy_arguments, directives, instructions, leading_flags from agent_env.utils.paths import LEAVINGS # Bumped when what source_digest covers changes, so a digest is never compared across meanings. @@ -137,12 +137,18 @@ class Pruned: # The COPY and ADD flags that change how a source is written, not which one it is. _SOURCE_KEEPING_FLAGS = ("--chown=", "--chmod=", "--link", "--checksum=", "--keep-git-dir", "--unpack") +# The RUN --mount types that never read the build context. +_OFF_CONTEXT_MOUNTS = frozenset({"cache", "tmpfs", "secret", "ssh"}) +# The Dockerfile frontend this reading follows; another may read the context its own way. +_DOCKERFILE_SYNTAX = re.compile(r"(?:docker\.io/)?docker/dockerfile(?:-upstream)?(?::[\w.-]+)?(?:@sha256:[0-9a-f]{64})?") _WILDCARD = re.compile(r"[*?\[\\]") _LARGEST = 5 def context_reads(dockerfile: str) -> ContextReads: """The context paths a build of ``dockerfile`` reads: its local COPY and ADD sources and bind-mount sources.""" + if (syntax := directives(dockerfile).get("syntax")) and not _DOCKERFILE_SYNTAX.fullmatch(syntax): + return ContextReads(whole=f"# syntax={syntax} isn't followed") paths: list[str] = [] for instruction in instructions(dockerfile): if not instruction.complete: @@ -265,9 +271,12 @@ def _mount_reads(instruction: Instruction, paths: list[str]) -> str | None: options = {} for field in next(csv.reader([flag.removeprefix("--mount=")])): key, _, value = field.partition("=") - options[key.strip()] = value.strip().strip("\"'") - if options.get("type", "bind") != "bind" or "from" in options: + options[key.strip().lower()] = value.strip().strip("\"'") + kind = options.get("type", "bind").lower() + if kind in _OFF_CONTEXT_MOUNTS or (kind == "bind" and "from" in options): continue + if kind != "bind": + return f"RUN --mount type={kind} isn't followed" source = options.get("source") or options.get("src") or "." if "$" in source: return f"RUN --mount source {source} names a variable" diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/dockerfile.py index bdc356bf..709bea8d 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/dockerfile.py @@ -38,11 +38,8 @@ class CopyArguments: def instructions(dockerfile: str) -> list[Instruction]: """``dockerfile``'s instructions and the lines between them, in order, as ``docker build`` splits them.""" lines = dockerfile.splitlines(keepends=True) - escape, at = "\\", 0 - while at < len(lines) and (directive := _DIRECTIVE.fullmatch(lines[at].strip())): - if directive.group(1).lower() == "escape": - escape = directive.group(2) - at += 1 + parsed, at = _directives(lines) + escape = parsed.get("escape", "\\") # The escape character ending a line, spaces or tabs after it, joins the next line on as it is. continued = re.compile(re.escape(escape) + r"[ \t]*$") found = [Instruction(None, "", line) for line in lines[:at]] @@ -84,6 +81,20 @@ def instructions(dockerfile: str) -> list[Instruction]: return found +def directives(dockerfile: str) -> dict[str, str]: + """The parser directives ``dockerfile`` opens with, by lower-case name.""" + return _directives(dockerfile.splitlines())[0] + + +def _directives(lines: list[str]) -> tuple[dict[str, str], int]: + """The parser directives ``lines`` open with, and how many lines they take.""" + found, at = {}, 0 + while at < len(lines) and (directive := _DIRECTIVE.fullmatch(lines[at].strip())): + found[directive.group(1).lower()] = directive.group(2) + at += 1 + return found, at + + def leading_flags(arguments: str) -> tuple[list[str], str]: """The ``--flag`` words ``arguments`` begins with, split on any whitespace outside quotes, and the rest of it.""" flags, at = [], 0 diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/build_context_prune_test.py index a69cce03..104a48dc 100644 --- a/tst/integration/utils/build_context_prune_test.py +++ b/tst/integration/utils/build_context_prune_test.py @@ -29,7 +29,7 @@ COPY current/app.txt /through-link.txt COPY lit/[!a].txt /lit/ RUN --network=none --mount=type=bind,"source=my dir",target=/m cp /m/f /out/spaced.txt -RUN --mount=type=bind,source=scripts,target=/scripts cp /scripts/run.sh /out/run.sh +RUN --mount=type=cache,target=/root/.cache --mount=type=BIND,source=scripts,target=/scripts cp /scripts/run.sh /out/run.sh """ # What the build reads, beside a context ten times bigger it never does. CONTEXT = { diff --git a/tst/unit/utils/build_context_prune_test.py b/tst/unit/utils/build_context_prune_test.py index dc0092e0..40825778 100644 --- a/tst/unit/utils/build_context_prune_test.py +++ b/tst/unit/utils/build_context_prune_test.py @@ -53,6 +53,29 @@ def test_a_heredoc_left_open_reads_all_of_the_context_rather_than_miss_a_copy_it whole="a heredoc in RUN never ends, so the rest can't be read") +@pytest.mark.parametrize("syntax", ["docker/dockerfile:1", "docker.io/docker/dockerfile:1.7-labs", + "docker/dockerfile-upstream:master", f"docker/dockerfile:1@sha256:{'0' * 64}"]) +def test_a_syntax_directive_naming_dockers_own_frontend_is_read(syntax): + assert context_reads(f"# syntax={syntax}\nFROM x\nCOPY a /a\n") == ContextReads(paths=("a",)) + + +def test_a_frontend_of_its_own_keeps_all_of_the_context(): + assert context_reads("# syntax=example.com/acme/frontend:1\nFROM x\nCOPY a /a\n") == ContextReads( + whole="# syntax=example.com/acme/frontend:1 isn't followed") + + +def test_mounts_that_never_read_the_context_add_nothing_and_options_read_in_any_case(): + dockerfile = ("FROM x\nRUN --mount=type=cache,target=/c --mount=type=secret,id=k --mount=type=ssh " + "--mount=type=tmpfs,target=/t ls\nRUN --mount=TYPE=BIND,Source=tools,target=/m ls\n") + + assert context_reads(dockerfile) == ContextReads(paths=("tools",)) + + +def test_a_mount_type_it_doesnt_know_keeps_all_of_the_context(): + assert context_reads("FROM x\nRUN --mount=type=future,source=a,target=/a ls\n") == ContextReads( + whole="RUN --mount type=future isn't followed") + + def _archive(path: Path, files: dict[str, str], links: dict[str, str] | None = None, executable: tuple[str, ...] = ()) -> Path: """A context tar.gz holding ``files`` and ``links``, everything in it as another tool might have written it.""" diff --git a/tst/unit/utils/dockerfile_test.py b/tst/unit/utils/dockerfile_test.py index 3adb620c..3ea40294 100644 --- a/tst/unit/utils/dockerfile_test.py +++ b/tst/unit/utils/dockerfile_test.py @@ -2,7 +2,7 @@ import pytest -from agent_env.utils.dockerfile import copy_arguments, instructions, leading_flags +from agent_env.utils.dockerfile import copy_arguments, directives, instructions, leading_flags DOCKERFILE = """# syntax=docker/dockerfile:1 FROM python:3.12-slim AS base @@ -42,6 +42,11 @@ def test_the_escape_directive_changes_what_continues_a_line(): assert [(i.keyword, i.arguments) for i in found if i.keyword] == [("FROM", "windows"), ("COPY", "a.txt C:\\app\\")] +def test_the_directives_a_dockerfile_opens_with_by_name(): + assert directives("# syntax=docker/dockerfile:1\n# Escape=`\nFROM x\n# check=skip=all\n") == { + "syntax": "docker/dockerfile:1", "escape": "`"} + + def test_a_directive_after_the_first_instruction_is_a_comment(): found = instructions("FROM x\n# escape=`\nCOPY a `\n b\n") From ad28e9165375c09e0ab6128fa4c99957299da879 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 08:56:50 -0700 Subject: [PATCH 09/14] refactor(docker): gather the Docker build helpers into utils/docker, and split the pruner into steps - utils/docker/ holds what was four utils modules: dockerfile (reading a Dockerfile, now with context_reads, what a build of it reads), context (BuildContext, extract and the ignore file, as on main), prune (write_pruned), build (build_image) and metadata (what a put records about a build). - write_pruned reads, plans and writes in three steps, and one helper finds where a link leads. - The packer and the pruner read an ignore file one way, as docker build reads it: its BOM dropped, and a line opening with # a comment. The packer used to read comment lines as patterns. - The COPY, ADD and bind-mount readers return what they read rather than add to a list. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- .../artifact/artifacts/docker_image.py | 4 +- src/agent_env/bundle/ledger.py | 2 +- src/agent_env/bundle/materialize.py | 2 +- src/agent_env/bundle/preflight.py | 2 +- src/agent_env/bundle/run.py | 2 +- src/agent_env/cli/__init__.py | 2 +- src/agent_env/cli/a2a_agent/put.py | 2 +- src/agent_env/cli/env/mcp_server.py | 2 +- src/agent_env/cli/env/website.py | 2 +- src/agent_env/cli/utils.py | 4 +- src/agent_env/env/bootstrap.py | 6 +- .../sandbox_providers/modal_image_build.py | 4 +- .../providers/sandbox_providers/sandbox.py | 2 +- .../sandbox_providers/smol_vm/sandbox.py | 2 +- src/agent_env/utils/build_context.py | 553 ------------------ src/agent_env/utils/docker/__init__.py | 0 .../{docker_build.py => docker/build.py} | 0 src/agent_env/utils/docker/context.py | 251 ++++++++ .../utils/{ => docker}/dockerfile.py | 91 ++- .../{build_metadata.py => docker/metadata.py} | 0 src/agent_env/utils/docker/prune.py | 249 ++++++++ .../env/envs/build_context_local_test.py | 2 +- tst/integration/utils/docker/__init__.py | 0 .../prune_test.py} | 2 +- .../test_docker_image_build_context.py | 2 +- tst/unit/bundle/materialize_test.py | 2 +- tst/unit/cli/build_platform_test.py | 2 +- tst/unit/cli/sandbox_url_relocation_test.py | 2 +- tst/unit/utils/docker/__init__.py | 0 .../build_test.py} | 6 +- .../context_test.py} | 12 +- .../utils/{ => docker}/dockerfile_test.py | 79 ++- .../prune_test.py} | 78 +-- 34 files changed, 709 insertions(+), 662 deletions(-) delete mode 100644 src/agent_env/utils/build_context.py create mode 100644 src/agent_env/utils/docker/__init__.py rename src/agent_env/utils/{docker_build.py => docker/build.py} (100%) create mode 100644 src/agent_env/utils/docker/context.py rename src/agent_env/utils/{ => docker}/dockerfile.py (60%) rename src/agent_env/utils/{build_metadata.py => docker/metadata.py} (100%) create mode 100644 src/agent_env/utils/docker/prune.py create mode 100644 tst/integration/utils/docker/__init__.py rename tst/integration/utils/{build_context_prune_test.py => docker/prune_test.py} (99%) create mode 100644 tst/unit/utils/docker/__init__.py rename tst/unit/utils/{docker_build_test.py => docker/build_test.py} (96%) rename tst/unit/utils/{build_context_test.py => docker/context_test.py} (94%) rename tst/unit/utils/{ => docker}/dockerfile_test.py (54%) rename tst/unit/utils/{build_context_prune_test.py => docker/prune_test.py} (75%) diff --git a/AGENTS.md b/AGENTS.md index 284ff365..f7d7415c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,7 +98,7 @@ and deserialized through a registry. | Package | Contents | |---|---| | `env/` | `Env` base class: a subclass declares `type`, implements `from_dict` and `async deploy(...) -> DeployedEnv`, and may override `reset` and `load_file_artifact_universe`. Built-ins in `env/envs/`: `MCPServerEnv` (`mcp_server`), `MultiEnv` (`multi`, MCP servers plus websites), `ServiceDBEnv` (`service_db`, PostgreSQL), `WebsiteEnv` (`website`), `GatewayEnv` (`gateway_server`). `env/gateway/gateway.py` is the env-side gateway server, container-only code. | -| `artifact/` | Immutable versioned artifacts; every `put()` creates a new version. The document goes to the document store, binary payloads to the object store. Built-ins: `CliArtifact` (`cli`), `FileArtifact` (`file`), `FileArtifactUniverse` (`file_artifact_universe`), `DockerImageArtifact` (`docker_image`), `EnvironmentArtifact` (`environment`), `EnvironmentUniverseArtifact` (`environment_universe`), `SkillArtifact` (`skill`); `[artifacts] type_aliases` maps another stored spelling of a type onto one of them. A `DockerImageArtifact` holds a tar.gz of its image, or, written by `put_ref` (`--image-ref` on `env mcp-server put` and `a2a-agent put`), only a registry reference with its tag pinned to a digest, which sandboxes pull, or, written by `put_context`, only its build context, which a VM sandbox builds before running it (`VmSandbox.load_docker_images`) and Modal builds before creating its container (`ModalSandboxProvider.prepare_image`, outside a fallback chain's deadline); anything else that runs images by name (a container on the local provider, a lone server on any provider but Modal) refuses such an image. A build context (`utils/build_context.py`) is what `docker build` would send, `.dockerignore` applied, written so equal files give equal bytes and an equal `source_digest`. | +| `artifact/` | Immutable versioned artifacts; every `put()` creates a new version. The document goes to the document store, binary payloads to the object store. Built-ins: `CliArtifact` (`cli`), `FileArtifact` (`file`), `FileArtifactUniverse` (`file_artifact_universe`), `DockerImageArtifact` (`docker_image`), `EnvironmentArtifact` (`environment`), `EnvironmentUniverseArtifact` (`environment_universe`), `SkillArtifact` (`skill`); `[artifacts] type_aliases` maps another stored spelling of a type onto one of them. A `DockerImageArtifact` holds a tar.gz of its image, or, written by `put_ref` (`--image-ref` on `env mcp-server put` and `a2a-agent put`), only a registry reference with its tag pinned to a digest, which sandboxes pull, or, written by `put_context`, only its build context, which a VM sandbox builds before running it (`VmSandbox.load_docker_images`) and Modal builds before creating its container (`ModalSandboxProvider.prepare_image`, outside a fallback chain's deadline); anything else that runs images by name (a container on the local provider, a lone server on any provider but Modal) refuses such an image. A build context (`utils/docker/context.py`) is what `docker build` would send, `.dockerignore` applied, written so equal files give equal bytes and an equal `source_digest`. | | `task/`, `task_step/` | A `Task` holds its `TaskStep`s inline; `Task.run()` executes them as a DAG. `depends_on` (None means all prior steps) gates a step, independent steps run concurrently, `fail_task_on_error` makes a failure fatal or tolerated, `retry_config` rolls a failed span back through the step journal and re-dispatches it. Built-in steps live in `task_step/task_steps/` (`deploy_env`, `deploy_agent`, `prompt_agent`, the verifiers under `verifiers/`, and more). `task/episode/` runs a task with some of its agents played from outside, as a training loop does (`EpisodeSpec`). | | `store/` | Four store ABCs with local and cloud implementations: `DocumentStore` (SQLite, MongoDB, DynamoDB, Firestore with MongoDB compatibility), `ObjectStore` (filesystem, S3, Cloud Storage), `ImageStore` (local OCI registry, ECR), `SecretStore` (env vars or file, AWS Secrets Manager, Google Cloud Secret Manager). `VersionedEntityStore` implements the shared versioned get/put logic, `QueryBuilder` is the immutable chained query API, `store/base.py` holds the error types. `store/routing.py` is the `@local` namespace: while the CLI runs, an `@local/…` id and what an `@local` task's run records go to local stores beside the configured ones, and that run's writes to the configured stores are refused. A new backend must pass the conformance kits in `tst/store/`. | | `config/` | The `Config` singleton (`get_config`, `configure`, `reset_config`) in `config/runtime.py`, file discovery in `config/loader.py`, and `load_impl`, which resolves `module:Class` pointers. `agent_env.store` re-exports the config names for compatibility. | diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index abf99242..00007c0b 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -23,8 +23,8 @@ from agent_env.store.image_store.local_registry_image_store import LocalRegistryImageStore from agent_env.store.image_store.oci_registry_credentials import is_loopback_host, names_registry, registry_host_from_ref from agent_env.store.image_store.registry_api import pin_digest, pinned_from -from agent_env.utils.build_context import BuildContext -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, normalize_platform +from agent_env.utils.docker.context import BuildContext +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, normalize_platform logger = logging.getLogger(__name__) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 3a58dc17..6fb6ba53 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -29,7 +29,7 @@ from agent_env.store.document_store import LocalSqliteDocumentStore from agent_env.store.local_state import holding_locks from agent_env.task.store import TASKS_COLLECTION -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM from .authoring import build_context_files from .parse import Bundle, BundleKind diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 8a11ab37..de8ee659 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -36,7 +36,7 @@ from agent_env.task import Task from agent_env.task_step.registry import get_task_step_registry from agent_env.task_step.task_step import TaskStep -from agent_env.utils.docker_build import DockerBuildError, build_image +from agent_env.utils.docker.build import DockerBuildError, build_image from ._fs import relative, with_article from .authoring import AuthoringContext, build_context_files diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index 13754bac..ab3ad3f8 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -71,7 +71,7 @@ from agent_env.task_step.task_steps.deploy_env import DeployEnvTaskStep from agent_env.task_step.task_steps.deploy_sandbox import DeploySandboxTaskStep from agent_env.task_step.task_steps.verifiers.rubrics_verifier import RubricsVerifierTaskStep -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, docker_unreachable +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, docker_unreachable from ._fs import relative from .parse import BundleError, BundleKind diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index 37dcc943..c2ab3f74 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -32,7 +32,7 @@ from agent_env.task.task import recorded_scores from agent_env.task.teardown import TeardownReport, teardown_run from agent_env.task_step.context import TaskStepContext -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM from ._fs import relative from .materialize import Materialization, Materialized, materialize diff --git a/src/agent_env/cli/__init__.py b/src/agent_env/cli/__init__.py index a2b4f10b..9356b217 100644 --- a/src/agent_env/cli/__init__.py +++ b/src/agent_env/cli/__init__.py @@ -7,7 +7,7 @@ from agent_env.plugins._cli import load_cli_plugins, load_cli_root_options from agent_env.store.base import NotFoundError from agent_env.store.routing import namespace_routing -from agent_env.utils.docker_build import DockerBuildError +from agent_env.utils.docker.build import DockerBuildError from .a2a_agent import a2a_agent from .artifact import artifact diff --git a/src/agent_env/cli/a2a_agent/put.py b/src/agent_env/cli/a2a_agent/put.py index d1a15678..407c411e 100644 --- a/src/agent_env/cli/a2a_agent/put.py +++ b/src/agent_env/cli/a2a_agent/put.py @@ -7,7 +7,7 @@ from agent_env.artifact import DockerImageArtifact from agent_env.cli.utils import build_platform_option, detect_env_metadata, refuse_unwritable_ids from agent_env.store.ids import derive_id, image_repository -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, build_image +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, build_image @click.command() diff --git a/src/agent_env/cli/env/mcp_server.py b/src/agent_env/cli/env/mcp_server.py index 82696df4..86fab3aa 100644 --- a/src/agent_env/cli/env/mcp_server.py +++ b/src/agent_env/cli/env/mcp_server.py @@ -16,7 +16,7 @@ resolve_environment_name, ) from agent_env.utils.card_naming import card_name_from_github, card_name_from_source -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, build_image +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, build_image from agent_env.env import Env, MCPServerEnv from agent_env.store.ids import derive_id, image_repository diff --git a/src/agent_env/cli/env/website.py b/src/agent_env/cli/env/website.py index f10930fd..9045413b 100644 --- a/src/agent_env/cli/env/website.py +++ b/src/agent_env/cli/env/website.py @@ -16,7 +16,7 @@ resolve_environment_name, ) from agent_env.utils.card_naming import card_name_from_github, card_name_from_source -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, build_image +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, build_image from agent_env.env.envs.website import WebsiteEnv from agent_env.store.ids import derive_id, image_repository diff --git a/src/agent_env/cli/utils.py b/src/agent_env/cli/utils.py index 21a0b72d..0489733a 100644 --- a/src/agent_env/cli/utils.py +++ b/src/agent_env/cli/utils.py @@ -10,8 +10,8 @@ from agent_env.providers.env_providers.env_provider import _env_provider_class from agent_env.providers.env_providers.env_server_provider import EnvironmentServerProvider from agent_env.store.base import NotFoundError -from agent_env.utils.build_metadata import detect_base_metadata, detect_env_metadata # noqa: F401 re-exported for the put commands -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM +from agent_env.utils.docker.metadata import detect_base_metadata, detect_env_metadata # noqa: F401 re-exported for the put commands +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM @contextmanager diff --git a/src/agent_env/env/bootstrap.py b/src/agent_env/env/bootstrap.py index 02263c5a..acd6eab1 100644 --- a/src/agent_env/env/bootstrap.py +++ b/src/agent_env/env/bootstrap.py @@ -27,9 +27,9 @@ from agent_env.store.local_state import holding_locks from agent_env.store.object_store import LocalFilesystemObjectStore from agent_env.store.routing import configured_store -from agent_env.utils.build_context import BuildContext -from agent_env.utils.build_metadata import detect_env_metadata -from agent_env.utils.docker_build import build_image, docker_unreachable +from agent_env.utils.docker.context import BuildContext +from agent_env.utils.docker.metadata import detect_env_metadata +from agent_env.utils.docker.build import build_image, docker_unreachable _ENV_PACKAGE = Path(__file__).parent GATEWAY_DOCKERFILE = _ENV_PACKAGE / "gateway" / "Dockerfile" diff --git a/src/agent_env/providers/sandbox_providers/modal_image_build.py b/src/agent_env/providers/sandbox_providers/modal_image_build.py index b8a79229..3fad4d3a 100644 --- a/src/agent_env/providers/sandbox_providers/modal_image_build.py +++ b/src/agent_env/providers/sandbox_providers/modal_image_build.py @@ -15,8 +15,8 @@ import modal from agent_env.config import get_config -from agent_env.utils.build_context import extract, ignore_file -from agent_env.utils.dockerfile import Instruction, copy_arguments, instructions, opens_heredoc +from agent_env.utils.docker.context import extract, ignore_file +from agent_env.utils.docker.dockerfile import Instruction, copy_arguments, instructions, opens_heredoc if TYPE_CHECKING: from agent_env.artifact.artifacts.docker_image import DockerImageArtifact diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index 81714474..4b4d6016 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -24,7 +24,7 @@ from agent_env.config import get_config from agent_env.store.image_store.oci_registry_credentials import registry_host_from_ref -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM from agent_env.utils.paths import validate_relative_filename if TYPE_CHECKING: diff --git a/src/agent_env/providers/sandbox_providers/smol_vm/sandbox.py b/src/agent_env/providers/sandbox_providers/smol_vm/sandbox.py index 92afdba2..e00ba42b 100644 --- a/src/agent_env/providers/sandbox_providers/smol_vm/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/smol_vm/sandbox.py @@ -15,7 +15,7 @@ from agent_env.providers.sandbox_providers.local_sandbox import LocalSandboxProvider from agent_env.store.object_store.local.store import LocalFilesystemObjectStore from agent_env.store.object_store.local.tls import local_ca -from agent_env.utils.docker_build import normalize_platform +from agent_env.utils.docker.build import normalize_platform from agent_env.providers.sandbox_providers.sandbox_provider import SANDBOX_MODE_VM _PORTS_FILE = "/storage/agentenv-ports.json" diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py deleted file mode 100644 index a37a3c3d..00000000 --- a/src/agent_env/utils/build_context.py +++ /dev/null @@ -1,553 +0,0 @@ -"""A Docker build context as a tar.gz that is the same, with the same digest, wherever the same files are written -from: what ``docker build`` would send from a folder, in a fixed order, with nothing of the machine in it.""" - -from __future__ import annotations - -import bisect -import csv -import functools -import gzip -import heapq -import hashlib -import json -import os -import posixpath -import re -import shutil -import stat -import tarfile -import unicodedata -from collections.abc import Mapping -from dataclasses import dataclass -from pathlib import Path, PurePosixPath - -from modal import FilePatternMatcher - -from agent_env.utils.dockerfile import Instruction, copy_arguments, directives, instructions, leading_flags -from agent_env.utils.paths import LEAVINGS - -# Bumped when what source_digest covers changes, so a digest is never compared across meanings. -_DIGEST_SCHEME = 1 -_READ_CHUNK_BYTES = 1 << 20 - - -@dataclass(frozen=True) -class _Entry: - name: str # POSIX, NFC, relative to the context's root - mode: int - path: Path | None = None # a file's, read for its content - sha256: str | None = None # a file's - link: str | None = None # a link's target, as written in it - - @property - def content(self) -> str | None: - """What the digest records for the entry beyond its name and mode: a file's sha256 or a link's target.""" - return self.sha256 if self.path is not None else (f"link:{self.link}" if self.link is not None else None) - - -@dataclass(frozen=True) -class BuildContext: - """The files ``docker build`` sends from a folder: all but what its ignore file excludes, the Dockerfile and the - ignore file always kept, and what the OS or Python leaves behind always dropped. A link is sent as a link, never - followed, as docker sends it. Modes are git's two: 0755 when the owner may execute, else 0644.""" - - dockerfile: str | None # its path in the context, None when it's outside it - entries: tuple[_Entry, ...] - - @classmethod - def of(cls, root: Path, dockerfile: Path | None = None) -> BuildContext: - """The context at ``root`` for ``dockerfile``. Raises ValueError naming every entry it can't take: a special - file, an unreadable one, two names equal once normalized.""" - if not root.is_dir(): - raise ValueError(f"build context {root} is not a folder") - top = Path(os.path.realpath(root)) - ignores = ignore_file(top, dockerfile) - # Sent whatever the ignore file says, and through any link to them, so a build finds them in the context. - sent = [path for path in (dockerfile, ignores) if path is not None] - kept = {name for path in sent for name in (_inside(top, path), _inside(top, Path(os.path.realpath(path)))) if name} - named = None - if dockerfile is not None and _inside(top, Path(os.path.realpath(dockerfile))): - named = _inside(top, dockerfile) - matcher = FilePatternMatcher.from_file(ignores) if ignores else None - walk = _Walk(top, matcher, kept) - walk.run() - if walk.problems: - raise ValueError(f"build context {root}: " + "; ".join(walk.problems)) - return cls(dockerfile=named, entries=walk.entries()) - - def source_digest(self, platform: str | None) -> str: - """sha256 over every entry's name, mode and content, the Dockerfile's path and ``platform``: equal for equal - sources, so an image built from one can stand for the other.""" - manifest = { - "scheme": _DIGEST_SCHEME, - "dockerfile": self.dockerfile, - "platform": platform, - "entries": [[entry.name, f"{entry.mode:o}", entry.content] for entry in self.entries], - } - canonical = json.dumps(manifest, separators=(",", ":"), ensure_ascii=False) - return f"sha256:{hashlib.sha256(canonical.encode()).hexdigest()}" - - def write(self, out: Path) -> None: - """Write the context to ``out`` as a tar.gz with sorted names, no times, owners or file name in it, and git's - modes. Raises RuntimeError when a file changed since the context was read.""" - with (out.open("wb") as raw, gzip.GzipFile(filename="", mode="wb", fileobj=raw, mtime=0) as gz, - tarfile.open(fileobj=gz, mode="w", format=tarfile.PAX_FORMAT) as tar): - for entry in self.entries: - info = tarfile.TarInfo(entry.name) - info.mode, info.mtime = entry.mode, 0 - if entry.path is None: - info.type = tarfile.SYMTYPE if entry.link is not None else tarfile.DIRTYPE - info.linkname = entry.link or "" - tar.addfile(info) - continue - with entry.path.open("rb") as f: - info.size = os.fstat(f.fileno()).st_size - reader = _Hashing(f) - tar.addfile(info, reader) - if reader.hexdigest() != entry.sha256: - raise RuntimeError(f"{entry.name} changed while its build context was being written") - - -def extract(archive: Path, out: Path) -> None: - """Unpack a build context's tar.gz into the folder ``out``: its files, folders and links. A link that leads out - of ``out`` is left out, as nothing there is the build's. Raises ValueError on anything else, which no context - written by ``BuildContext.write`` holds: a hard link, a device, a name that leads out of ``out``.""" - with tarfile.open(archive, "r:gz") as tar: - tar.extractall(out, filter=_context_member) - - -@dataclass(frozen=True) -class ContextReads: - """The context paths a build reads, each of which may hold Docker's wildcards, or why it reads all of it.""" - - paths: tuple[str, ...] = () - whole: str | None = None - - -@dataclass(frozen=True) -class Pruned: - """What ``write_pruned`` wrote into one folder, and why it wrote all the context sends when it did.""" - - dockerfile: str - files: int - size: int - whole: str | None - largest: tuple[tuple[str, int], ...] = () - - -# The COPY and ADD flags that change how a source is written, not which one it is. -_SOURCE_KEEPING_FLAGS = ("--chown=", "--chmod=", "--link", "--checksum=", "--keep-git-dir", "--unpack") -# The RUN --mount types that never read the build context. -_OFF_CONTEXT_MOUNTS = frozenset({"cache", "tmpfs", "secret", "ssh"}) -# The Dockerfile frontend this reading follows; another may read the context its own way. -_DOCKERFILE_SYNTAX = re.compile(r"(?:docker\.io/)?docker/dockerfile(?:-upstream)?(?::[\w.-]+)?(?:@sha256:[0-9a-f]{64})?") -_WILDCARD = re.compile(r"[*?\[\\]") -_LARGEST = 5 - - -def context_reads(dockerfile: str) -> ContextReads: - """The context paths a build of ``dockerfile`` reads: its local COPY and ADD sources and bind-mount sources.""" - if (syntax := directives(dockerfile).get("syntax")) and not _DOCKERFILE_SYNTAX.fullmatch(syntax): - return ContextReads(whole=f"# syntax={syntax} isn't followed") - paths: list[str] = [] - for instruction in instructions(dockerfile): - if not instruction.complete: - return ContextReads(whole=f"a heredoc in {instruction.keyword} never ends, so the rest can't be read") - if instruction.keyword in ("COPY", "ADD"): - whole = _copy_reads(instruction, paths) - elif instruction.keyword == "RUN": - whole = _mount_reads(instruction, paths) - else: - continue - if whole: - return ContextReads(whole=whole) - return ContextReads(paths=tuple(dict.fromkeys(paths))) - - -def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, Pruned]: - """Write into each folder what its Dockerfile's build reads of the context tar.gz (None: the one it finds).""" - dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items() if dockerfile} - wanted = {name for dockerfile in dockerfiles.values() for name in (dockerfile, *_ignore_files(dockerfile))} - links: dict[str, str | None] = {} - texts: dict[str, str] = {} - with tarfile.open(archive, "r:gz") as tar: - for member in tar: - if (name := _member_name(member)) is None: - continue - links[name] = member.linkname if member.issym() else None - if name in wanted and member.isreg(): - texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") - for folder in images.keys() - dockerfiles.keys(): - dockerfiles[folder] = _found_dockerfile(links) - wanted |= {dockerfiles[folder], *_ignore_files(dockerfiles[folder])} - if unread := {name for name in wanted if name in links and not links[name] and name not in texts}: - texts |= _texts(archive, unread) - # A Dockerfile or ignore file stored as a link is read where it leads, as docker build reads it. - linked = {name: _resolve(name, links)[0] for name in wanted if links.get(name)} - if unread := {target for target in linked.values() if target not in texts}: - texts |= _texts(archive, unread) - texts |= {name: texts[target] for name, target in linked.items() if target in texts} - plans: dict[Path, tuple[set[str], str | None]] = {} - for folder, dockerfile in dockerfiles.items(): - if dockerfile not in texts: - raise ValueError(f"the build context has no {dockerfile}") - ignore = next((name for name in _ignore_files(dockerfile) if name in texts), None) - matcher = FilePatternMatcher(*_patterns(texts[ignore])) if ignore else None - reads, crossed = _through_links(context_reads(texts[dockerfile]), links) - # A source the reading found that names nothing here is likelier a misreading than a build that fails. - if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): - reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() - plans[folder] = (_kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole) - # By name, since a name the archive holds twice is written over: what's left is its last entry. - written: dict[Path, dict[str, int]] = {folder: {} for folder in images} - with tarfile.open(archive, "r:gz") as tar: - for member in tar: - name = _member_name(member) - first: Path | None = None - for folder in (folder for folder, (kept, _) in plans.items() if name in kept): - if member.isreg() and first is not None: - (folder / name).parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(first, folder / name) - else: - folder.mkdir(parents=True, exist_ok=True) - tar.extract(member, folder, filter=_context_member) - first = folder / name if member.isreg() else None - if member.isreg(): - written[folder][name] = member.size - else: - written[folder].pop(name, None) - return {folder: Pruned(dockerfiles[folder], len(files), sum(files.values()), plans[folder][1], - tuple(heapq.nlargest(_LARGEST, files.items(), key=lambda file: file[1]))) - for folder, files in written.items()} - - -def _found_dockerfile(links: Mapping[str, str | None]) -> str: - """The Dockerfile an image that names none builds: the context's ``Dockerfile``, else the only one in it.""" - if "Dockerfile" in links: - return "Dockerfile" - found = sorted(name for name in links if posixpath.basename(name) == "Dockerfile") - if len(found) != 1: - raise ValueError(f"the build context names no Dockerfile and holds {len(found)}" - f"{': ' + ', '.join(found) if found else ''}; name the one it builds") - return found[0] - - -def _texts(archive: Path, names: set[str]) -> dict[str, str]: - """The text of each regular file ``names`` names in the tar.gz at ``archive``.""" - texts = {} - with tarfile.open(archive, "r:gz") as tar: - for member in tar: - if member.isreg() and (name := _member_name(member)) in names: - texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") - return texts - - -def _copy_reads(instruction: Instruction, paths: list[str]) -> str | None: - """Add the context paths a COPY or ADD reads to ``paths``; why it reads all of the context, when it does.""" - args = copy_arguments(instruction) - if args is None: - return f"{instruction.keyword} {instruction.arguments} can't be read" - if any(flag.startswith("--from=") for flag in args.flags): - return None - if unknown := next((flag for flag in args.flags if not flag.startswith(_SOURCE_KEEPING_FLAGS)), None): - return f"{args.keyword} {unknown} isn't followed" - for source in args.sources: - if args.keyword == "ADD" and ("://" in source or source.startswith("git@")): - continue - if "$" in source: - return f"{args.keyword} {source} names a variable" - if (path := _context_path(source)) is None: - return f"{args.keyword} {source} copies all of it" - paths.append(path) - return None - - -def _mount_reads(instruction: Instruction, paths: list[str]) -> str | None: - """Add the context paths a RUN's bind mounts read to ``paths``; why it reads all of the context, when it does.""" - for flag in leading_flags(instruction.arguments)[0]: - if not flag.startswith("--mount="): - continue - # A mount's options are one CSV record, as docker build reads them, so a quoted field holds a comma or space. - options = {} - for field in next(csv.reader([flag.removeprefix("--mount=")])): - key, _, value = field.partition("=") - options[key.strip().lower()] = value.strip().strip("\"'") - kind = options.get("type", "bind").lower() - if kind in _OFF_CONTEXT_MOUNTS or (kind == "bind" and "from" in options): - continue - if kind != "bind": - return f"RUN --mount type={kind} isn't followed" - source = options.get("source") or options.get("src") or "." - if "$" in source: - return f"RUN --mount source {source} names a variable" - if (path := _context_path(source)) is None: - return f"RUN --mount binds {source}, all of it" - paths.append(path) - return None - - -def _context_path(source: str) -> str | None: - """``source``'s path in the context; None for the context itself, or a path that would leave it.""" - path = posixpath.normpath(source.lstrip("/")) - return None if path == "." or path == ".." or path.startswith("../") else path - - -def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tuple[ContextReads, set[str]]: - """``reads`` with each source that names nothing followed through the links on its path, and the links crossed.""" - if reads.whole is not None: - return reads, set() - paths, crossed = [], set() - for path in reads.paths: - if _WILDCARD.search(path) or any(_matches(path, name) for name in links): - paths.append(path) - continue - resolved, hops = _resolve(path, links) - paths.append(resolved) - crossed |= hops - return ContextReads(paths=tuple(paths)), crossed - - -def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]]: - """``path`` with its links followed, and the links crossed; as it is when a link leads out or loops.""" - resolved, crossed = "", set() - for part in path.split("/"): - resolved = posixpath.join(resolved, part) - while (target := links.get(resolved)) is not None: - following = posixpath.normpath(posixpath.join(posixpath.dirname(resolved), target)) - if target.startswith("/") or following == ".." or following.startswith("../") or resolved in crossed: - return path, set() - crossed.add(resolved) - resolved = following - return resolved, crossed - - -def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, - matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: - """The names in ``links`` (name to link target) that ``write_pruned`` keeps for ``reads``, and their folders.""" - def sent(name: str) -> bool: - return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) - - if reads.whole is not None: - kept = {name for name in links if sent(name)} - else: - kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} - kept |= {name for name in (dockerfile, ignore) if name in links} | {name for name in crossed if sent(name)} - names = sorted(links) - pending = [name for name in kept if links[name]] - while pending: - name = pending.pop() - target = posixpath.normpath(posixpath.join(posixpath.dirname(name), links[name])) - if links[name].startswith("/") or target == ".." or target.startswith("../"): - continue - for other in _at_or_under(names, target): - if other not in kept and sent(other): - kept.add(other) - if links[other]: - pending.append(other) - return kept | {str(parent) for name in kept for parent in PurePosixPath(name).parents if str(parent) in links} - - -def _at_or_under(names: list[str], path: str) -> list[str]: - """The names in ``names``, sorted, that are ``path`` or under it.""" - at = bisect.bisect_left(names, path) - found = [path] if at < len(names) and names[at] == path else [] - start = bisect.bisect_left(names, f"{path}/") - end = bisect.bisect_left(names, f"{path}0") # "0" sorts right after "/" - return found + names[start:end] - - -def _matches(path: str, name: str) -> bool: - """Whether ``name`` is ``path`` or under it, each part matched as Docker matches a COPY source's.""" - want, have = path.split("/"), name.split("/") - return len(have) >= len(want) and all( - part == glob if not _WILDCARD.search(glob) else bool((pattern := _part_pattern(glob)) and pattern.fullmatch(part)) - for part, glob in zip(have, want)) - - -@functools.lru_cache(maxsize=1024) -def _part_pattern(glob: str) -> re.Pattern[str] | None: - """A path part's wildcard as Go's ``filepath.Match``, which docker build uses, reads it; None when malformed.""" - out, at = [], 0 - while at < len(glob): - char = glob[at] - if char == "*": - out.append("[^/]*") - elif char == "?": - out.append("[^/]") - elif char == "\\": - if at + 1 == len(glob): - return None - at += 1 - out.append(re.escape(glob[at])) - elif char == "[": - at += 1 - negated = at < len(glob) and glob[at] == "^" - at += negated - members = [] - while at < len(glob) and glob[at] != "]": - if glob[at] == "\\": - at += 1 - if at == len(glob): - return None - members.append(re.escape(glob[at])) - elif glob[at] == "-" and members and at + 1 < len(glob) and glob[at + 1] != "]": - members.append("-") - else: - members.append(re.escape(glob[at])) - at += 1 - if at == len(glob) or not members: - return None - out.append(f"[{'^' if negated else ''}{''.join(members)}]") - else: - out.append(re.escape(char)) - at += 1 - return re.compile("".join(out)) - - -def _ignore_files(dockerfile: str) -> tuple[str, ...]: - """Where ``docker build`` looks for a Dockerfile's ignore file, in order.""" - return f"{dockerfile}.dockerignore", ".dockerignore" - - -def _patterns(ignore_file: str) -> list[str]: - """An ignore file's patterns, its comment lines left out as Docker leaves them out.""" - return [line for line in ignore_file.splitlines() if line.strip() and not line.lstrip().startswith("#")] - - -def _member_name(member: tarfile.TarInfo) -> str | None: - """``member``'s path in the context; None for the context's own folder.""" - name = posixpath.normpath(member.name.lstrip("/")) - return None if name == "." else name - - -class _Walk: - """Lists a context folder in sorted order.""" - - def __init__(self, top: Path, matcher: FilePatternMatcher | None, kept: set[str]): - self.top = top - self.matcher = matcher - self.kept = kept - self.found: dict[str, _Entry] = {} - self.folders: set[str] = set() - self.names: set[str] = set() - self.problems: list[str] = [] - - def run(self) -> None: - pending = [(self.top, ())] - while pending: - folder, parts = pending.pop() - try: - with os.scandir(folder) as listing: - children = sorted(listing, key=lambda child: child.name) - except OSError as e: - self.problems.append(f"{_show(parts)}: {e.strerror or e}") - continue - for child in children: - if child.name in LEAVINGS: - continue - child_parts = (*parts, unicodedata.normalize("NFC", child.name)) - name = "/".join(child_parts) - excluded = name not in self.kept and self._excluded(name) - if child.is_dir(follow_symlinks=False): - if (excluded and self._prunable(name)) or not self._claim(name): - continue - if not excluded: - self.folders.add(name) - pending.append((Path(child.path), child_parts)) - elif excluded: - continue - elif child.is_symlink(): - self._add(_Entry(name, 0o777, link=os.readlink(child.path))) - elif child.is_file(follow_symlinks=False): - self._file(name, Path(child.path)) - else: - self.problems.append(f"{name}: neither a regular file, a folder nor a link") - - def entries(self) -> tuple[_Entry, ...]: - folders = set(self.folders) - for name in self.found: - folders.update(str(parent) for parent in PurePosixPath(name).parents if str(parent) != ".") - listed = [*self.found.values(), *(_Entry(name, 0o755) for name in folders - set(self.found))] - return tuple(sorted(listed, key=lambda entry: entry.name)) - - def _excluded(self, name: str) -> bool: - return self.matcher is not None and self.matcher(Path(name)) - - def _prunable(self, name: str) -> bool: - """Whether nothing under an excluded folder can be sent: no exception pattern brings a file back, and no file - the context always keeps is in it.""" - return self.matcher.can_prune_directories() and not any(kept.startswith(f"{name}/") for kept in self.kept) - - def _add(self, entry: _Entry) -> None: - if self._claim(entry.name): - self.found[entry.name] = entry - - def _claim(self, name: str) -> bool: - """Whether ``name`` is still free: two names on disk can be one once normalized to NFC.""" - if name in self.names: - self.problems.append(f"{name}: two entries have this name once normalized to NFC; rename one") - return False - self.names.add(name) - return True - - def _file(self, name: str, path: Path) -> None: - try: - with path.open("rb") as f: - digest = hashlib.sha256() - while chunk := f.read(_READ_CHUNK_BYTES): - digest.update(chunk) - executable = os.fstat(f.fileno()).st_mode & stat.S_IXUSR - except OSError as e: - self.problems.append(f"{name}: {e.strerror or e}") - return - self._add(_Entry(name, 0o755 if executable else 0o644, path=path, sha256=digest.hexdigest())) - - -class _Hashing: - """A file the tar reads through, hashing what it reads.""" - - def __init__(self, f): - self._f = f - self._digest = hashlib.sha256() - - def read(self, size: int = -1) -> bytes: - data = self._f.read(size) - self._digest.update(data) - return data - - def hexdigest(self) -> str: - return self._digest.hexdigest() - - -def _context_member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: - """``member`` as ``extract`` writes it, None to leave it out.""" - if not (member.isreg() or member.isdir() or member.issym()): - raise ValueError(f"{member.name!r} in the build context is neither a file, a folder nor a link") - try: - member = tarfile.tar_filter(member, out) - except tarfile.FilterError as e: - raise ValueError(f"{member.name!r} in the build context: {e}") from e - if member.issym(): - top = os.path.realpath(out) - target = os.path.realpath(os.path.join(top, os.path.dirname(member.name), member.linkname)) - if os.path.commonpath([top, target]) != top: - return None - return member - - -def ignore_file(top: Path, dockerfile: Path | None) -> Path | None: - """The ignore file ``docker build`` reads: ``.dockerignore`` beside the Dockerfile, else the - context's ``.dockerignore``.""" - candidates = [dockerfile.with_name(f"{dockerfile.name}.dockerignore")] if dockerfile is not None else [] - return next((path for path in (*candidates, top / ".dockerignore") if path.is_file()), None) - - -def _inside(top: Path, path: Path) -> str | None: - """``path``'s name in the context at ``top``, the folders above it resolved but not the file itself, so a link - keeps its own name; None when it's outside the context.""" - named = Path(os.path.realpath(path.parent)) / path.name - return unicodedata.normalize("NFC", named.relative_to(top).as_posix()) if named.is_relative_to(top) else None - - -def _show(parts: tuple[str, ...]) -> str: - return "/".join(parts) or "." diff --git a/src/agent_env/utils/docker/__init__.py b/src/agent_env/utils/docker/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/agent_env/utils/docker_build.py b/src/agent_env/utils/docker/build.py similarity index 100% rename from src/agent_env/utils/docker_build.py rename to src/agent_env/utils/docker/build.py diff --git a/src/agent_env/utils/docker/context.py b/src/agent_env/utils/docker/context.py new file mode 100644 index 00000000..8cc1da33 --- /dev/null +++ b/src/agent_env/utils/docker/context.py @@ -0,0 +1,251 @@ +"""A Docker build context as a tar.gz that is the same, with the same digest, wherever the same files are written +from: what ``docker build`` would send from a folder, in a fixed order, with nothing of the machine in it.""" + +from __future__ import annotations + +import gzip +import hashlib +import json +import os +import stat +import tarfile +import unicodedata +from dataclasses import dataclass +from pathlib import Path, PurePosixPath + +from modal import FilePatternMatcher + +from agent_env.utils.paths import LEAVINGS + +# Bumped when what source_digest covers changes, so a digest is never compared across meanings. +_DIGEST_SCHEME = 1 +_READ_CHUNK_BYTES = 1 << 20 + + +@dataclass(frozen=True) +class _Entry: + name: str # POSIX, NFC, relative to the context's root + mode: int + path: Path | None = None # a file's, read for its content + sha256: str | None = None # a file's + link: str | None = None # a link's target, as written in it + + @property + def content(self) -> str | None: + """What the digest records for the entry beyond its name and mode: a file's sha256 or a link's target.""" + return self.sha256 if self.path is not None else (f"link:{self.link}" if self.link is not None else None) + + +@dataclass(frozen=True) +class BuildContext: + """The files ``docker build`` sends from a folder: all but what its ignore file excludes, the Dockerfile and the + ignore file always kept, and what the OS or Python leaves behind always dropped. A link is sent as a link, never + followed, as docker sends it. Modes are git's two: 0755 when the owner may execute, else 0644.""" + + dockerfile: str | None # its path in the context, None when it's outside it + entries: tuple[_Entry, ...] + + @classmethod + def of(cls, root: Path, dockerfile: Path | None = None) -> BuildContext: + """The context at ``root`` for ``dockerfile``. Raises ValueError naming every entry it can't take: a special + file, an unreadable one, two names equal once normalized.""" + if not root.is_dir(): + raise ValueError(f"build context {root} is not a folder") + top = Path(os.path.realpath(root)) + ignores = ignore_file(top, dockerfile) + # Sent whatever the ignore file says, and through any link to them, so a build finds them in the context. + sent = [path for path in (dockerfile, ignores) if path is not None] + kept = {name for path in sent for name in (_inside(top, path), _inside(top, Path(os.path.realpath(path)))) if name} + named = None + if dockerfile is not None and _inside(top, Path(os.path.realpath(dockerfile))): + named = _inside(top, dockerfile) + matcher = ignore_matcher(ignores.read_text(encoding="utf-8")) if ignores else None + walk = _Walk(top, matcher, kept) + walk.run() + if walk.problems: + raise ValueError(f"build context {root}: " + "; ".join(walk.problems)) + return cls(dockerfile=named, entries=walk.entries()) + + def source_digest(self, platform: str | None) -> str: + """sha256 over every entry's name, mode and content, the Dockerfile's path and ``platform``: equal for equal + sources, so an image built from one can stand for the other.""" + manifest = { + "scheme": _DIGEST_SCHEME, + "dockerfile": self.dockerfile, + "platform": platform, + "entries": [[entry.name, f"{entry.mode:o}", entry.content] for entry in self.entries], + } + canonical = json.dumps(manifest, separators=(",", ":"), ensure_ascii=False) + return f"sha256:{hashlib.sha256(canonical.encode()).hexdigest()}" + + def write(self, out: Path) -> None: + """Write the context to ``out`` as a tar.gz with sorted names, no times, owners or file name in it, and git's + modes. Raises RuntimeError when a file changed since the context was read.""" + with (out.open("wb") as raw, gzip.GzipFile(filename="", mode="wb", fileobj=raw, mtime=0) as gz, + tarfile.open(fileobj=gz, mode="w", format=tarfile.PAX_FORMAT) as tar): + for entry in self.entries: + info = tarfile.TarInfo(entry.name) + info.mode, info.mtime = entry.mode, 0 + if entry.path is None: + info.type = tarfile.SYMTYPE if entry.link is not None else tarfile.DIRTYPE + info.linkname = entry.link or "" + tar.addfile(info) + continue + with entry.path.open("rb") as f: + info.size = os.fstat(f.fileno()).st_size + reader = _Hashing(f) + tar.addfile(info, reader) + if reader.hexdigest() != entry.sha256: + raise RuntimeError(f"{entry.name} changed while its build context was being written") + + +def extract(archive: Path, out: Path) -> None: + """Unpack a build context's tar.gz into the folder ``out``: its files, folders and links. A link that leads out + of ``out`` is left out, as nothing there is the build's. Raises ValueError on anything else, which no context + written by ``BuildContext.write`` holds: a hard link, a device, a name that leads out of ``out``.""" + with tarfile.open(archive, "r:gz") as tar: + tar.extractall(out, filter=context_member) + + +class _Walk: + """Lists a context folder in sorted order.""" + + def __init__(self, top: Path, matcher: FilePatternMatcher | None, kept: set[str]): + self.top = top + self.matcher = matcher + self.kept = kept + self.found: dict[str, _Entry] = {} + self.folders: set[str] = set() + self.names: set[str] = set() + self.problems: list[str] = [] + + def run(self) -> None: + pending = [(self.top, ())] + while pending: + folder, parts = pending.pop() + try: + with os.scandir(folder) as listing: + children = sorted(listing, key=lambda child: child.name) + except OSError as e: + self.problems.append(f"{_show(parts)}: {e.strerror or e}") + continue + for child in children: + if child.name in LEAVINGS: + continue + child_parts = (*parts, unicodedata.normalize("NFC", child.name)) + name = "/".join(child_parts) + excluded = name not in self.kept and self._excluded(name) + if child.is_dir(follow_symlinks=False): + if (excluded and self._prunable(name)) or not self._claim(name): + continue + if not excluded: + self.folders.add(name) + pending.append((Path(child.path), child_parts)) + elif excluded: + continue + elif child.is_symlink(): + self._add(_Entry(name, 0o777, link=os.readlink(child.path))) + elif child.is_file(follow_symlinks=False): + self._file(name, Path(child.path)) + else: + self.problems.append(f"{name}: neither a regular file, a folder nor a link") + + def entries(self) -> tuple[_Entry, ...]: + folders = set(self.folders) + for name in self.found: + folders.update(str(parent) for parent in PurePosixPath(name).parents if str(parent) != ".") + listed = [*self.found.values(), *(_Entry(name, 0o755) for name in folders - set(self.found))] + return tuple(sorted(listed, key=lambda entry: entry.name)) + + def _excluded(self, name: str) -> bool: + return self.matcher is not None and self.matcher(Path(name)) + + def _prunable(self, name: str) -> bool: + """Whether nothing under an excluded folder can be sent: no exception pattern brings a file back, and no file + the context always keeps is in it.""" + return self.matcher.can_prune_directories() and not any(kept.startswith(f"{name}/") for kept in self.kept) + + def _add(self, entry: _Entry) -> None: + if self._claim(entry.name): + self.found[entry.name] = entry + + def _claim(self, name: str) -> bool: + """Whether ``name`` is still free: two names on disk can be one once normalized to NFC.""" + if name in self.names: + self.problems.append(f"{name}: two entries have this name once normalized to NFC; rename one") + return False + self.names.add(name) + return True + + def _file(self, name: str, path: Path) -> None: + try: + with path.open("rb") as f: + digest = hashlib.sha256() + while chunk := f.read(_READ_CHUNK_BYTES): + digest.update(chunk) + executable = os.fstat(f.fileno()).st_mode & stat.S_IXUSR + except OSError as e: + self.problems.append(f"{name}: {e.strerror or e}") + return + self._add(_Entry(name, 0o755 if executable else 0o644, path=path, sha256=digest.hexdigest())) + + +class _Hashing: + """A file the tar reads through, hashing what it reads.""" + + def __init__(self, f): + self._f = f + self._digest = hashlib.sha256() + + def read(self, size: int = -1) -> bytes: + data = self._f.read(size) + self._digest.update(data) + return data + + def hexdigest(self) -> str: + return self._digest.hexdigest() + + +def context_member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: + """``member`` as ``extract`` writes it, None to leave it out.""" + if not (member.isreg() or member.isdir() or member.issym()): + raise ValueError(f"{member.name!r} in the build context is neither a file, a folder nor a link") + try: + member = tarfile.tar_filter(member, out) + except tarfile.FilterError as e: + raise ValueError(f"{member.name!r} in the build context: {e}") from e + if member.issym(): + top = os.path.realpath(out) + target = os.path.realpath(os.path.join(top, os.path.dirname(member.name), member.linkname)) + if os.path.commonpath([top, target]) != top: + return None + return member + + +def ignore_file(top: Path, dockerfile: Path | None) -> Path | None: + """The ignore file ``docker build`` reads: ``.dockerignore`` beside the Dockerfile, else the + context's ``.dockerignore``.""" + candidates = [dockerfile.with_name(f"{dockerfile.name}.dockerignore")] if dockerfile is not None else [] + return next((path for path in (*candidates, top / ".dockerignore") if path.is_file()), None) + + +def ignore_names(dockerfile: str) -> tuple[str, str]: + """``ignore_file``'s candidates, in order, for a Dockerfile at ``dockerfile`` in the context.""" + return f"{dockerfile}.dockerignore", ".dockerignore" + + +def ignore_matcher(ignore_file: str) -> FilePatternMatcher: + """A matcher for an ignore file's patterns, as docker build reads them: a line opening with ``#`` is a comment.""" + lines = ignore_file.removeprefix("\ufeff").splitlines() + return FilePatternMatcher(*(line for line in lines if not line.startswith("#"))) + + +def _inside(top: Path, path: Path) -> str | None: + """``path``'s name in the context at ``top``, the folders above it resolved but not the file itself, so a link + keeps its own name; None when it's outside the context.""" + named = Path(os.path.realpath(path.parent)) / path.name + return unicodedata.normalize("NFC", named.relative_to(top).as_posix()) if named.is_relative_to(top) else None + + +def _show(parts: tuple[str, ...]) -> str: + return "/".join(parts) or "." diff --git a/src/agent_env/utils/dockerfile.py b/src/agent_env/utils/docker/dockerfile.py similarity index 60% rename from src/agent_env/utils/dockerfile.py rename to src/agent_env/utils/docker/dockerfile.py index 709bea8d..2d75af70 100644 --- a/src/agent_env/utils/dockerfile.py +++ b/src/agent_env/utils/docker/dockerfile.py @@ -1,8 +1,11 @@ -"""A Dockerfile's instructions as ``docker build`` reads them, and the arguments of its COPY and ADD instructions.""" +"""A Dockerfile's instructions as ``docker build`` reads them, its COPY and ADD arguments, and what a build of it reads +of its context.""" from __future__ import annotations +import csv import json +import posixpath import re from dataclasses import dataclass @@ -10,6 +13,12 @@ # ``<<<`` is a shell here-string, which docker build leaves to the shell, not a heredoc. _HEREDOC = re.compile(r"(? CopyArguments | None: sources = tuple(source for source in args[:-1] if json_form or not _HEREDOC.fullmatch(source)) return CopyArguments(instruction.keyword, tuple(flags), sources, args[-1], json_form, heredoc=len(sources) < len(args) - 1) + + +def context_reads(dockerfile: str) -> ContextReads: + """The context paths a build of ``dockerfile`` reads: its local COPY and ADD sources and bind-mount sources.""" + if (syntax := directives(dockerfile).get("syntax")) and not _DOCKERFILE_SYNTAX.fullmatch(syntax): + return ContextReads(whole=f"# syntax={syntax} isn't followed") + paths: list[str] = [] + for instruction in instructions(dockerfile): + if not instruction.complete: + return ContextReads(whole=f"a heredoc in {instruction.keyword} never ends, so the rest can't be read") + if instruction.keyword in ("COPY", "ADD"): + reads = _copy_reads(instruction) + elif instruction.keyword == "RUN": + reads = _mount_reads(instruction) + else: + continue + if reads.whole: + return reads + paths.extend(reads.paths) + return ContextReads(paths=tuple(dict.fromkeys(paths))) + + +def _copy_reads(instruction: Instruction) -> ContextReads: + """The context paths a COPY or ADD reads.""" + args = copy_arguments(instruction) + if args is None: + return ContextReads(whole=f"{instruction.keyword} {instruction.arguments} can't be read") + if any(flag.startswith("--from=") for flag in args.flags): + return ContextReads() + if unknown := next((flag for flag in args.flags if not flag.startswith(_SOURCE_KEEPING_FLAGS)), None): + return ContextReads(whole=f"{args.keyword} {unknown} isn't followed") + paths = [] + for source in args.sources: + if args.keyword == "ADD" and ("://" in source or source.startswith("git@")): + continue + if "$" in source: + return ContextReads(whole=f"{args.keyword} {source} names a variable") + if (path := _context_path(source)) is None: + return ContextReads(whole=f"{args.keyword} {source} copies all of it") + paths.append(path) + return ContextReads(paths=tuple(paths)) + + +def _mount_reads(instruction: Instruction) -> ContextReads: + """The context paths a RUN's bind mounts read.""" + paths = [] + for flag in leading_flags(instruction.arguments)[0]: + if not flag.startswith("--mount="): + continue + # A mount's options are one CSV record, as docker build reads them, so a quoted field holds a comma or space. + options = {} + for field in next(csv.reader([flag.removeprefix("--mount=")])): + key, _, value = field.partition("=") + options[key.strip().lower()] = value.strip().strip("\"'") + kind = options.get("type", "bind").lower() + if kind in _OFF_CONTEXT_MOUNTS or (kind == "bind" and "from" in options): + continue + if kind != "bind": + return ContextReads(whole=f"RUN --mount type={kind} isn't followed") + source = options.get("source") or options.get("src") or "." + if "$" in source: + return ContextReads(whole=f"RUN --mount source {source} names a variable") + if (path := _context_path(source)) is None: + return ContextReads(whole=f"RUN --mount binds {source}, all of it") + paths.append(path) + return ContextReads(paths=tuple(paths)) + + +def _context_path(source: str) -> str | None: + """``source``'s path in the context; None for the context itself, or a path that would leave it.""" + path = posixpath.normpath(source.lstrip("/")) + return None if path == "." or path == ".." or path.startswith("../") else path diff --git a/src/agent_env/utils/build_metadata.py b/src/agent_env/utils/docker/metadata.py similarity index 100% rename from src/agent_env/utils/build_metadata.py rename to src/agent_env/utils/docker/metadata.py diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py new file mode 100644 index 00000000..70d13ca4 --- /dev/null +++ b/src/agent_env/utils/docker/prune.py @@ -0,0 +1,249 @@ +"""The part of a stored build context each image's build reads, written into a folder of its own.""" + +from __future__ import annotations + +import bisect +import functools +import heapq +import posixpath +import re +import shutil +import tarfile +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path, PurePosixPath + +from modal import FilePatternMatcher + +from agent_env.utils.docker.context import context_member, ignore_matcher, ignore_names +from agent_env.utils.docker.dockerfile import ContextReads, context_reads +from agent_env.utils.paths import LEAVINGS + +_WILDCARD = re.compile(r"[*?\[\\]") +_LARGEST = 5 + + +@dataclass(frozen=True) +class Pruned: + """What ``write_pruned`` wrote into one folder, and why it wrote all the context sends when it did.""" + + dockerfile: str + files: int + size: int + whole: str | None + largest: tuple[tuple[str, int], ...] = () + + +def write_pruned(archive: Path, images: Mapping[Path, str | None]) -> dict[Path, Pruned]: + """Write into each folder what its Dockerfile's build reads of the context tar.gz (None: the one it finds).""" + links, dockerfiles, texts = _read(archive, images) + plans = {folder: _plan(dockerfile, links, texts) for folder, dockerfile in dockerfiles.items()} + written = _write(archive, {folder: kept for folder, (kept, _) in plans.items()}) + return {folder: Pruned(dockerfiles[folder], len(files), sum(files.values()), plans[folder][1], + tuple(heapq.nlargest(_LARGEST, files.items(), key=lambda file: file[1]))) + for folder, files in written.items()} + + +def _read(archive: Path, + images: Mapping[Path, str | None]) -> tuple[dict[str, str | None], dict[Path, str], dict[str, str]]: + """The archive's names to their link targets, each image's Dockerfile, and its Dockerfile and ignore file texts.""" + dockerfiles = {folder: posixpath.normpath(dockerfile) for folder, dockerfile in images.items() if dockerfile} + wanted = {name for dockerfile in dockerfiles.values() for name in (dockerfile, *ignore_names(dockerfile))} + links: dict[str, str | None] = {} + texts: dict[str, str] = {} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + if (name := _member_name(member)) is None: + continue + links[name] = member.linkname if member.issym() else None + if name in wanted and member.isreg(): + texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + for folder in images.keys() - dockerfiles.keys(): + dockerfiles[folder] = _found_dockerfile(links) + wanted |= {dockerfiles[folder], *ignore_names(dockerfiles[folder])} + if unread := {name for name in wanted if name in links and not links[name] and name not in texts}: + texts |= _texts(archive, unread) + # A Dockerfile or ignore file stored as a link is read where it leads, as docker build reads it. + linked = {name: _resolve(name, links)[0] for name in wanted if links.get(name)} + if unread := {target for target in linked.values() if target not in texts}: + texts |= _texts(archive, unread) + return links, dockerfiles, texts | {name: texts[target] for name, target in linked.items() if target in texts} + + +def _plan(dockerfile: str, links: Mapping[str, str | None], texts: Mapping[str, str]) -> tuple[set[str], str | None]: + """The names a build of ``dockerfile`` keeps, and why it keeps all the context sends when it does.""" + if dockerfile not in texts: + raise ValueError(f"the build context has no {dockerfile}") + ignore = next((name for name in ignore_names(dockerfile) if name in texts), None) + matcher = ignore_matcher(texts[ignore]) if ignore else None + reads, crossed = _through_links(context_reads(texts[dockerfile]), links) + # A source the reading found that names nothing here is likelier a misreading than a build that fails. + if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): + reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() + return _kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole + + +def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, int]]: + """Write each folder's kept names from the archive, and each folder's files by name with their sizes.""" + # By name, since a name the archive holds twice is written over: what's left is its last entry. + written: dict[Path, dict[str, int]] = {folder: {} for folder in kept} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + name = _member_name(member) + first: Path | None = None + for folder in (folder for folder, names in kept.items() if name in names): + if member.isreg() and first is not None: + (folder / name).parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(first, folder / name) + else: + folder.mkdir(parents=True, exist_ok=True) + tar.extract(member, folder, filter=context_member) + first = folder / name if member.isreg() else None + if member.isreg(): + written[folder][name] = member.size + else: + written[folder].pop(name, None) + return written + + +def _found_dockerfile(links: Mapping[str, str | None]) -> str: + """The Dockerfile an image that names none builds: the context's ``Dockerfile``, else the only one in it.""" + if "Dockerfile" in links: + return "Dockerfile" + found = sorted(name for name in links if posixpath.basename(name) == "Dockerfile") + if len(found) != 1: + raise ValueError(f"the build context names no Dockerfile and holds {len(found)}" + f"{': ' + ', '.join(found) if found else ''}; name the one it builds") + return found[0] + + +def _texts(archive: Path, names: set[str]) -> dict[str, str]: + """The text of each regular file ``names`` names in the tar.gz at ``archive``.""" + texts = {} + with tarfile.open(archive, "r:gz") as tar: + for member in tar: + if member.isreg() and (name := _member_name(member)) in names: + texts[name] = tar.extractfile(member).read().decode("utf-8", "replace") + return texts + + +def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tuple[ContextReads, set[str]]: + """``reads`` with each source that names nothing followed through the links on its path, and the links crossed.""" + if reads.whole is not None: + return reads, set() + paths, crossed = [], set() + for path in reads.paths: + if _WILDCARD.search(path) or any(_matches(path, name) for name in links): + paths.append(path) + continue + resolved, hops = _resolve(path, links) + paths.append(resolved) + crossed |= hops + return ContextReads(paths=tuple(paths)), crossed + + +def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]]: + """``path`` with its links followed, and the links crossed; as it is when a link leads out or loops.""" + resolved, crossed = "", set() + for part in path.split("/"): + resolved = posixpath.join(resolved, part) + while links.get(resolved) is not None: + if (following := _link_target(resolved, links)) is None or resolved in crossed: + return path, set() + crossed.add(resolved) + resolved = following + return resolved, crossed + + +def _link_target(name: str, links: Mapping[str, str | None]) -> str | None: + """Where the link ``name`` leads in the context; None when it leads out of it.""" + target = links[name] + following = posixpath.normpath(posixpath.join(posixpath.dirname(name), target)) + return None if target.startswith("/") or following == ".." or following.startswith("../") else following + + +def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, + matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: + """The names in ``links`` that ``write_pruned`` keeps for ``reads``, and their folders.""" + def sent(name: str) -> bool: + return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) + + if reads.whole is not None: + kept = {name for name in links if sent(name)} + else: + kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} + kept |= {name for name in (dockerfile, ignore) if name in links} | {name for name in crossed if sent(name)} + names = sorted(links) + pending = [name for name in kept if links[name]] + while pending: + if (target := _link_target(pending.pop(), links)) is None: + continue + for other in _at_or_under(names, target): + if other not in kept and sent(other): + kept.add(other) + if links[other]: + pending.append(other) + return kept | {str(parent) for name in kept for parent in PurePosixPath(name).parents if str(parent) in links} + + +def _at_or_under(names: list[str], path: str) -> list[str]: + """The names in ``names``, sorted, that are ``path`` or under it.""" + at = bisect.bisect_left(names, path) + found = [path] if at < len(names) and names[at] == path else [] + start = bisect.bisect_left(names, f"{path}/") + end = bisect.bisect_left(names, f"{path}0") # "0" sorts right after "/" + return found + names[start:end] + + +def _matches(path: str, name: str) -> bool: + """Whether ``name`` is ``path`` or under it, each part matched as Docker matches a COPY source's.""" + want, have = path.split("/"), name.split("/") + return len(have) >= len(want) and all( + part == glob if not _WILDCARD.search(glob) else bool((pattern := _part_pattern(glob)) and pattern.fullmatch(part)) + for part, glob in zip(have, want)) + + +@functools.lru_cache(maxsize=1024) +def _part_pattern(glob: str) -> re.Pattern[str] | None: + """A path part's wildcard as Go's ``filepath.Match``, which docker build uses, reads it; None when malformed.""" + out, at = [], 0 + while at < len(glob): + char = glob[at] + if char == "*": + out.append("[^/]*") + elif char == "?": + out.append("[^/]") + elif char == "\\": + if at + 1 == len(glob): + return None + at += 1 + out.append(re.escape(glob[at])) + elif char == "[": + at += 1 + negated = at < len(glob) and glob[at] == "^" + at += negated + members = [] + while at < len(glob) and glob[at] != "]": + if glob[at] == "\\": + at += 1 + if at == len(glob): + return None + members.append(re.escape(glob[at])) + elif glob[at] == "-" and members and at + 1 < len(glob) and glob[at + 1] != "]": + members.append("-") + else: + members.append(re.escape(glob[at])) + at += 1 + if at == len(glob) or not members: + return None + out.append(f"[{'^' if negated else ''}{''.join(members)}]") + else: + out.append(re.escape(char)) + at += 1 + return re.compile("".join(out)) + + +def _member_name(member: tarfile.TarInfo) -> str | None: + """``member``'s path in the context; None for the context's own folder.""" + name = posixpath.normpath(member.name.lstrip("/")) + return None if name == "." else name diff --git a/tst/integration/env/envs/build_context_local_test.py b/tst/integration/env/envs/build_context_local_test.py index abd4cb35..d0b9ffc2 100644 --- a/tst/integration/env/envs/build_context_local_test.py +++ b/tst/integration/env/envs/build_context_local_test.py @@ -18,7 +18,7 @@ from agent_env.env.envs.mcp_server import MCPServerEnv from agent_env.providers.sandbox_providers import sandbox from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox -from agent_env.utils.docker_build import normalize_platform +from agent_env.utils.docker.build import normalize_platform from tst.integration.env.envs.server_env_local_test import local_stack as local_stack from tst.integration.store.local_parallel_deploy_test import _docker, _seed_bootstrap_envs diff --git a/tst/integration/utils/docker/__init__.py b/tst/integration/utils/docker/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tst/integration/utils/build_context_prune_test.py b/tst/integration/utils/docker/prune_test.py similarity index 99% rename from tst/integration/utils/build_context_prune_test.py rename to tst/integration/utils/docker/prune_test.py index 104a48dc..72e3a2c2 100644 --- a/tst/integration/utils/build_context_prune_test.py +++ b/tst/integration/utils/docker/prune_test.py @@ -10,7 +10,7 @@ import pytest -from agent_env.utils.build_context import write_pruned +from agent_env.utils.docker.prune import write_pruned from tst.util.capabilities import missing_capability_reason pytestmark = [pytest.mark.integration, pytest.mark.int_test_slow] diff --git a/tst/unit/artifact/test_docker_image_build_context.py b/tst/unit/artifact/test_docker_image_build_context.py index d6b8dc4d..3666f550 100644 --- a/tst/unit/artifact/test_docker_image_build_context.py +++ b/tst/unit/artifact/test_docker_image_build_context.py @@ -12,7 +12,7 @@ from agent_env.artifact.store import get_artifact_store from agent_env.store.image_store.local_registry_image_store import LocalRegistryImageStore from agent_env.store.image_store.oci_registry_credentials import names_registry -from agent_env.utils.build_context import BuildContext +from agent_env.utils.docker.context import BuildContext SERVER = {"Dockerfile": "FROM python:3.12\nCOPY . /app\n", "src/app.py": "print(1)\n", ".dockerignore": "*.log\n", "debug.log": "noise"} diff --git a/tst/unit/bundle/materialize_test.py b/tst/unit/bundle/materialize_test.py index 6e5b6826..9c4b3981 100644 --- a/tst/unit/bundle/materialize_test.py +++ b/tst/unit/bundle/materialize_test.py @@ -35,7 +35,7 @@ from agent_env.store.routing import disable_namespace_routing from agent_env.task import Task from agent_env.task_step.task_step import TaskStep -from agent_env.utils.build_context import BuildContext +from agent_env.utils.docker.context import BuildContext from tst.unit.bundle._support import DIGEST, RefusingStore, layout, local_store, plan_of ROOT = "@local/~/triage" diff --git a/tst/unit/cli/build_platform_test.py b/tst/unit/cli/build_platform_test.py index 8b365d18..f4a217d6 100644 --- a/tst/unit/cli/build_platform_test.py +++ b/tst/unit/cli/build_platform_test.py @@ -9,7 +9,7 @@ from agent_env.cli import cli from agent_env.env.envs.website_browser import WEBSITE_BROWSER_IMAGE_TAG -from agent_env.utils.docker_build import DockerBuildError +from agent_env.utils.docker.build import DockerBuildError gateway = importlib.import_module("agent_env.cli.env.gateway") # the packages export the click commands service_db = importlib.import_module("agent_env.cli.env.service_db") diff --git a/tst/unit/cli/sandbox_url_relocation_test.py b/tst/unit/cli/sandbox_url_relocation_test.py index 2b774333..7ab59047 100644 --- a/tst/unit/cli/sandbox_url_relocation_test.py +++ b/tst/unit/cli/sandbox_url_relocation_test.py @@ -11,7 +11,7 @@ from click.testing import CliRunner from unittest.mock import MagicMock, patch -from agent_env.utils.docker_build import DockerBuildError +from agent_env.utils.docker.build import DockerBuildError def test_old_cli_path_no_longer_re_exports_the_helper(): diff --git a/tst/unit/utils/docker/__init__.py b/tst/unit/utils/docker/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tst/unit/utils/docker_build_test.py b/tst/unit/utils/docker/build_test.py similarity index 96% rename from tst/unit/utils/docker_build_test.py rename to tst/unit/utils/docker/build_test.py index 9ea3e8e2..d0b434bb 100644 --- a/tst/unit/utils/docker_build_test.py +++ b/tst/unit/utils/docker/build_test.py @@ -9,8 +9,8 @@ import pytest -from agent_env.utils import docker_build -from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, DockerBuildError, build_image, normalize_platform +from agent_env.utils.docker import build as docker_build +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, DockerBuildError, build_image, normalize_platform @pytest.fixture @@ -78,7 +78,7 @@ def run(argv, **_): def test_importing_it_pulls_neither_cli_nor_providers(): code = ( - "import sys; import agent_env.utils.docker_build; " + "import sys; import agent_env.utils.docker.build; " "assert 'agent_env.cli' not in sys.modules, 'cli imported'; " "assert 'agent_env.providers' not in sys.modules, 'providers imported'" ) diff --git a/tst/unit/utils/build_context_test.py b/tst/unit/utils/docker/context_test.py similarity index 94% rename from tst/unit/utils/build_context_test.py rename to tst/unit/utils/docker/context_test.py index ea5b91e6..ffdb013a 100644 --- a/tst/unit/utils/build_context_test.py +++ b/tst/unit/utils/docker/context_test.py @@ -9,7 +9,7 @@ import pytest -from agent_env.utils.build_context import BuildContext, extract, ignore_file +from agent_env.utils.docker.context import BuildContext, extract, ignore_file def _tree(root: Path, files: dict[str, str], executable: tuple[str, ...] = ()) -> Path: @@ -81,6 +81,16 @@ def test_a_dockerfiles_own_ignore_file_replaces_the_contexts(tmp_path): assert not any(entry.name.startswith("data") for entry in context.entries) +def test_the_ignore_file_is_read_as_docker_reads_it_its_bom_dropped_and_a_line_opening_with_a_hash_a_comment(tmp_path): + # docker build sends "# notes.txt" and leaves out a.log and "#raw.txt" for this ignore file. + root = _tree(tmp_path / "ctx", {**SERVER, ".dockerignore": "*.log\n# notes.txt\n #raw.txt\n", "a.log": "", + "# notes.txt": "", "#raw.txt": ""}) + + names = {entry.name for entry in BuildContext.of(root, root / "Dockerfile").entries} + + assert "# notes.txt" in names and "a.log" not in names and "#raw.txt" not in names + + def test_what_the_os_and_python_leave_behind_is_dropped(tmp_path): root = _tree(tmp_path / "ctx", {**SERVER, ".DS_Store": "", "app/__pycache__/main.cpython-312.pyc": ""}) diff --git a/tst/unit/utils/dockerfile_test.py b/tst/unit/utils/docker/dockerfile_test.py similarity index 54% rename from tst/unit/utils/dockerfile_test.py rename to tst/unit/utils/docker/dockerfile_test.py index 3ea40294..dbb55b7b 100644 --- a/tst/unit/utils/dockerfile_test.py +++ b/tst/unit/utils/docker/dockerfile_test.py @@ -1,8 +1,9 @@ -"""A Dockerfile read into instructions as docker build reads it, and a COPY or ADD's arguments.""" +"""A Dockerfile read into instructions as docker build reads it, a COPY or ADD's arguments, and what it reads.""" import pytest -from agent_env.utils.dockerfile import copy_arguments, directives, instructions, leading_flags +from agent_env.utils.docker.dockerfile import (ContextReads, context_reads, copy_arguments, directives, instructions, + leading_flags) DOCKERFILE = """# syntax=docker/dockerfile:1 FROM python:3.12-slim AS base @@ -113,3 +114,77 @@ def test_arguments_it_cant_read_are_none(line): [instruction] = [i for i in instructions(f"{line}\n") if i.keyword] assert copy_arguments(instruction) is None + + +@pytest.mark.parametrize(("dockerfile", "paths"), [ + ("COPY gmail/requirements.txt .\nCOPY BaseService/ ./svc/BaseService/\n", ("gmail/requirements.txt", "BaseService")), + ("FROM node AS cli\nFROM python\nCOPY --from=cli /usr/bin/node /usr/bin/node\nCOPY app /app\n", ("app",)), + ("ADD https://example.com/f /f\nADD git@github.com:o/r.git /r\nADD local.tar.gz /x\n", ("local.tar.gz",)), + ("COPY < /q\n' + 'RUN ["sh", "-c", "cat < /out\nCOPY a.txt /a.txt\n") == ContextReads(paths=("a.txt",)) + + +def test_a_heredoc_left_open_reads_all_of_the_context_rather_than_miss_a_copy_it_swallowed(): + assert context_reads("FROM x\nRUN < /q\n' - 'RUN ["sh", "-c", "cat < /out\nCOPY a.txt /a.txt\n") == ContextReads(paths=("a.txt",)) - - -def test_a_heredoc_left_open_reads_all_of_the_context_rather_than_miss_a_copy_it_swallowed(): - assert context_reads("FROM x\nRUN < Date: Mon, 19 Oct 2026 09:49:47 -0700 Subject: [PATCH 10/14] fix(docker): follow links, read escapes and refuse wildcards as docker build does Found by building random contexts and Dockerfiles both from the whole context and from the pruned folder: - docker build resolves a link inside the context, `/` and any `..` above it standing for its root, so a source through `sub -> /conf` reads conf/. The pruner read such a link as leading out and left its target behind. - A link that leads out of the context is written into the folder as it is, since docker build copies it as a link; nothing is still written through one. - docker build unescapes a source before matching it, so `COPY \[x].txt` copies x.txt. A source or mount source holding an escape now keeps the whole context. - A wildcard docker build refuses (a reversed or malformed class) keeps the whole context instead of raising. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/docker/context.py | 6 +- src/agent_env/utils/docker/dockerfile.py | 17 ++++-- src/agent_env/utils/docker/prune.py | 71 ++++++++++++------------ tst/unit/utils/docker/dockerfile_test.py | 10 ++++ tst/unit/utils/docker/prune_test.py | 62 +++++++++++++++++---- 5 files changed, 115 insertions(+), 51 deletions(-) diff --git a/src/agent_env/utils/docker/context.py b/src/agent_env/utils/docker/context.py index 8cc1da33..714df6dc 100644 --- a/src/agent_env/utils/docker/context.py +++ b/src/agent_env/utils/docker/context.py @@ -206,15 +206,15 @@ def hexdigest(self) -> str: return self._digest.hexdigest() -def context_member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: - """``member`` as ``extract`` writes it, None to leave it out.""" +def context_member(member: tarfile.TarInfo, out: str, *, links_out: bool = False) -> tarfile.TarInfo | None: + """``member`` as ``extract`` writes it; None for a link leading out of ``out``, unless ``links_out``.""" if not (member.isreg() or member.isdir() or member.issym()): raise ValueError(f"{member.name!r} in the build context is neither a file, a folder nor a link") try: member = tarfile.tar_filter(member, out) except tarfile.FilterError as e: raise ValueError(f"{member.name!r} in the build context: {e}") from e - if member.issym(): + if member.issym() and not links_out: top = os.path.realpath(out) target = os.path.realpath(os.path.join(top, os.path.dirname(member.name), member.linkname)) if os.path.commonpath([top, target]) != top: diff --git a/src/agent_env/utils/docker/dockerfile.py b/src/agent_env/utils/docker/dockerfile.py index 2d75af70..39121b91 100644 --- a/src/agent_env/utils/docker/dockerfile.py +++ b/src/agent_env/utils/docker/dockerfile.py @@ -217,8 +217,8 @@ def _copy_reads(instruction: Instruction) -> ContextReads: for source in args.sources: if args.keyword == "ADD" and ("://" in source or source.startswith("git@")): continue - if "$" in source: - return ContextReads(whole=f"{args.keyword} {source} names a variable") + if why := _unread(source): + return ContextReads(whole=f"{args.keyword} {source} {why}") if (path := _context_path(source)) is None: return ContextReads(whole=f"{args.keyword} {source} copies all of it") paths.append(path) @@ -242,14 +242,23 @@ def _mount_reads(instruction: Instruction) -> ContextReads: if kind != "bind": return ContextReads(whole=f"RUN --mount type={kind} isn't followed") source = options.get("source") or options.get("src") or "." - if "$" in source: - return ContextReads(whole=f"RUN --mount source {source} names a variable") + if why := _unread(source): + return ContextReads(whole=f"RUN --mount source {source} {why}") if (path := _context_path(source)) is None: return ContextReads(whole=f"RUN --mount binds {source}, all of it") paths.append(path) return ContextReads(paths=tuple(paths)) +def _unread(source: str) -> str | None: + """Why ``source`` can't be read before a build, as docker build expands and unescapes it first; None when it can.""" + if "$" in source: + return "names a variable" + if "\\" in source or "`" in source: + return "holds an escape" + return None + + def _context_path(source: str) -> str | None: """``source``'s path in the context; None for the context itself, or a path that would leave it.""" path = posixpath.normpath(source.lstrip("/")) diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py index 70d13ca4..4ec86fd9 100644 --- a/src/agent_env/utils/docker/prune.py +++ b/src/agent_env/utils/docker/prune.py @@ -19,8 +19,10 @@ from agent_env.utils.docker.dockerfile import ContextReads, context_reads from agent_env.utils.paths import LEAVINGS -_WILDCARD = re.compile(r"[*?\[\\]") +_WILDCARD = re.compile(r"[*?\[]") _LARGEST = 5 +# A link is written as it is, as docker build sends one, wherever it leads; nothing is written through one leading out. +_MEMBER = functools.partial(context_member, links_out=True) @dataclass(frozen=True) @@ -97,7 +99,7 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, shutil.copy2(first, folder / name) else: folder.mkdir(parents=True, exist_ok=True) - tar.extract(member, folder, filter=context_member) + tar.extract(member, folder, filter=_MEMBER) first = folder / name if member.isreg() else None if member.isreg(): written[folder][name] = member.size @@ -143,23 +145,28 @@ def _through_links(reads: ContextReads, links: Mapping[str, str | None]) -> tupl def _resolve(path: str, links: Mapping[str, str | None]) -> tuple[str, set[str]]: - """``path`` with its links followed, and the links crossed; as it is when a link leads out or loops.""" + """``path`` with its links followed as docker build follows them, and the links crossed; as it is when they loop.""" resolved, crossed = "", set() for part in path.split("/"): resolved = posixpath.join(resolved, part) while links.get(resolved) is not None: - if (following := _link_target(resolved, links)) is None or resolved in crossed: + if resolved in crossed: return path, set() crossed.add(resolved) - resolved = following + resolved = _link_target(resolved, links) return resolved, crossed -def _link_target(name: str, links: Mapping[str, str | None]) -> str | None: - """Where the link ``name`` leads in the context; None when it leads out of it.""" +def _link_target(name: str, links: Mapping[str, str | None]) -> str: + """Where the link ``name`` leads, as docker build follows it in the context: ``/`` and ``..`` stop at its root.""" target = links[name] - following = posixpath.normpath(posixpath.join(posixpath.dirname(name), target)) - return None if target.startswith("/") or following == ".." or following.startswith("../") else following + parts = [] if target.startswith("/") or "/" not in name else name.split("/")[:-1] + for part in target.split("/"): + if part == "..": + parts = parts[:-1] + elif part not in ("", "."): + parts.append(part) + return "/".join(parts) def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, @@ -172,13 +179,14 @@ def sent(name: str) -> bool: kept = {name for name in links if sent(name)} else: kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} - kept |= {name for name in (dockerfile, ignore) if name in links} | {name for name in crossed if sent(name)} - names = sorted(links) + kept |= {name for name in (dockerfile, ignore) if name in links} + # A link a source only crosses is needed for its path, not for all it leads to. pending = [name for name in kept if links[name]] + kept |= {name for name in crossed if sent(name)} + names = sorted(links) while pending: - if (target := _link_target(pending.pop(), links)) is None: - continue - for other in _at_or_under(names, target): + target = _link_target(pending.pop(), links) + for other in _at_or_under(names, target) if target else names: if other not in kept and sent(other): kept.add(other) if links[other]: @@ -205,7 +213,7 @@ def _matches(path: str, name: str) -> bool: @functools.lru_cache(maxsize=1024) def _part_pattern(glob: str) -> re.Pattern[str] | None: - """A path part's wildcard as Go's ``filepath.Match``, which docker build uses, reads it; None when malformed.""" + """A path part's wildcard as docker build matches it; None when docker build refuses it as malformed.""" out, at = [], 0 while at < len(glob): char = glob[at] @@ -213,30 +221,25 @@ def _part_pattern(glob: str) -> re.Pattern[str] | None: out.append("[^/]*") elif char == "?": out.append("[^/]") - elif char == "\\": - if at + 1 == len(glob): - return None - at += 1 - out.append(re.escape(glob[at])) elif char == "[": at += 1 negated = at < len(glob) and glob[at] == "^" at += negated - members = [] - while at < len(glob) and glob[at] != "]": - if glob[at] == "\\": - at += 1 - if at == len(glob): - return None - members.append(re.escape(glob[at])) - elif glob[at] == "-" and members and at + 1 < len(glob) and glob[at + 1] != "]": - members.append("-") - else: - members.append(re.escape(glob[at])) + ranges = [] + while not (at < len(glob) and glob[at] == "]" and ranges): + # A class's characters as Go's filepath.Match reads them: a range's ends neither - nor ], low to high. + if at + 1 >= len(glob) or glob[at] in "-]": + return None + low = high = glob[at] at += 1 - if at == len(glob) or not members: - return None - out.append(f"[{'^' if negated else ''}{''.join(members)}]") + if glob[at] == "-": + if at + 2 >= len(glob) or glob[at + 1] in "-]": + return None + high, at = glob[at + 1], at + 2 + if low > high: + return None + ranges.append(re.escape(low) if low == high else f"{re.escape(low)}-{re.escape(high)}") + out.append(f"[{'^' if negated else ''}{''.join(ranges)}]") else: out.append(re.escape(char)) at += 1 diff --git a/tst/unit/utils/docker/dockerfile_test.py b/tst/unit/utils/docker/dockerfile_test.py index dbb55b7b..c682fbee 100644 --- a/tst/unit/utils/docker/dockerfile_test.py +++ b/tst/unit/utils/docker/dockerfile_test.py @@ -155,6 +155,16 @@ def test_a_shell_here_string_leaves_the_copies_after_it_read(): assert context_reads("FROM x\nRUN cat << /out\nCOPY a.txt /a.txt\n") == ContextReads(paths=("a.txt",)) +@pytest.mark.parametrize(("line", "why"), [ + ("COPY \\[x].txt /x/", "COPY \\[x].txt holds an escape"), + ('COPY ["a\\\\b", "/x/"]', "COPY a\\b holds an escape"), + ("RUN --mount=type=bind,source=a\\b,target=/m ls", "RUN --mount source a\\b holds an escape"), +], ids=["shell", "json", "mount"]) +def test_a_source_holding_an_escape_keeps_all_of_the_context(line, why): + # docker build unescapes a source before matching it: COPY \[x].txt copies x.txt, not [x].txt. + assert context_reads(f"FROM x\n{line}\n") == ContextReads(whole=why) + + def test_a_heredoc_left_open_reads_all_of_the_context_rather_than_miss_a_copy_it_swallowed(): assert context_reads("FROM x\nRUN < Date: Mon, 19 Oct 2026 10:02:17 -0700 Subject: [PATCH 11/14] fix(docker): write pruned files at the time of writing, not the stored context's A stored context's files carry fixed times (BuildContext writes 0, for its digest), and docker build skips sending a file whose path, size and time match one it was sent before. Two folders of one name written from two stored contexts, such as one env in two bundles, or one folder written over, then built the first one's files. Each file is now written at the time of writing. The integration test builds one env's folder from two bundles; with the stored times kept, both images hold the first bundle's file. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/docker/prune.py | 12 +++++++++--- tst/integration/utils/docker/prune_test.py | 22 ++++++++++++++++++++++ tst/unit/utils/docker/prune_test.py | 12 ++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py index 4ec86fd9..8a0e73b4 100644 --- a/src/agent_env/utils/docker/prune.py +++ b/src/agent_env/utils/docker/prune.py @@ -21,8 +21,6 @@ _WILDCARD = re.compile(r"[*?\[]") _LARGEST = 5 -# A link is written as it is, as docker build sends one, wherever it leads; nothing is written through one leading out. -_MEMBER = functools.partial(context_member, links_out=True) @dataclass(frozen=True) @@ -99,7 +97,7 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, shutil.copy2(first, folder / name) else: folder.mkdir(parents=True, exist_ok=True) - tar.extract(member, folder, filter=_MEMBER) + tar.extract(member, folder, filter=_member) first = folder / name if member.isreg() else None if member.isreg(): written[folder][name] = member.size @@ -108,6 +106,14 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, return written +def _member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: + """``member`` as ``write_pruned`` writes it: a link as it is, wherever it leads, and at the time of writing.""" + if (member := context_member(member, out, links_out=True)) is None: + return None + # A stored context's times are fixed, and docker build skips sending a file whose time and size it has seen before. + return member.replace(mtime=None, deep=False) + + def _found_dockerfile(links: Mapping[str, str | None]) -> str: """The Dockerfile an image that names none builds: the context's ``Dockerfile``, else the only one in it.""" if "Dockerfile" in links: diff --git a/tst/integration/utils/docker/prune_test.py b/tst/integration/utils/docker/prune_test.py index 72e3a2c2..862aa6e3 100644 --- a/tst/integration/utils/docker/prune_test.py +++ b/tst/integration/utils/docker/prune_test.py @@ -126,3 +126,25 @@ def test_an_image_built_from_the_pruned_context_holds_what_one_from_the_whole_co "release/app.txt", "through-link.txt", "lit/!.txt", "lit/a.txt", "out/spaced.txt"} <= set(built) assert "lit/b.txt" not in built assert not any(name.startswith("app/tests") or name.endswith(".pyc") for name in built) + + +def test_folders_of_one_name_written_from_two_stored_contexts_each_build_their_own_files(tmp_path, docker): + # docker build skips sending a file whose path, size and time match one it was sent before, as a stored context's + # fixed times would make them, and builds the old file. + name = f"gmail-{uuid.uuid4().hex[:8]}" + built = [] + for version in ("1", "2"): + archive = tmp_path / f"v{version}.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + _add(tar, "Dockerfile", f"FROM {BASE}\nCOPY version.txt /version.txt\n".encode()) + _add(tar, "version.txt", f"version {version}\n".encode()) + folder = tmp_path / f"bundle-v{version}" / "envs" / name + write_pruned(archive, {folder: "Dockerfile"}) + docker.append(f"prune-{name}-v{version}") + result = subprocess.run(["docker", "build", "-q", "--no-cache", "-t", docker[-1], str(folder)], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + built.append(subprocess.run(["docker", "run", "--rm", docker[-1], "cat", "/version.txt"], capture_output=True, + text=True, check=True).stdout) + + assert built == ["version 1\n", "version 2\n"] diff --git a/tst/unit/utils/docker/prune_test.py b/tst/unit/utils/docker/prune_test.py index d890bc0a..d078d073 100644 --- a/tst/unit/utils/docker/prune_test.py +++ b/tst/unit/utils/docker/prune_test.py @@ -2,6 +2,7 @@ import io import tarfile +import time from pathlib import Path import pytest @@ -285,6 +286,17 @@ def test_a_name_the_archive_holds_twice_counts_once_at_the_size_left_in_the_fold assert pruned.largest == (("data/b.bin", 100), ("Dockerfile", 30), ("data/a.bin", 10)) +def test_each_file_is_written_at_the_time_of_writing_not_the_stored_contexts(tmp_path): + # A stored context's files carry the time 0; docker build would read a rewritten one as unchanged. + archive = _archive(tmp_path / "ctx.tar.gz", SERVERS) + before = time.time() + + write_pruned(archive, {tmp_path / "gmail": "gmail/Dockerfile", tmp_path / "slack": "slack/Dockerfile"}) + + for name in ("gmail/BaseService/base.py", "slack/BaseService/base.py", "slack/slack/main.py"): + assert (tmp_path / name).stat().st_mtime >= before - 1 + + def test_a_dockerfile_missing_from_the_archive_is_refused(tmp_path): archive = _archive(tmp_path / "ctx.tar.gz", {"a.py": "a"}) From cfaf3c1a611be1359d560b6962a29b24b9cf5045 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 10:27:07 -0700 Subject: [PATCH 12/14] fix(docker): keep the Dockerfile's link target and folders the archive only implies - docker build reads the Dockerfile and its ignore file where they lead, whatever the ignore file says. A Dockerfile stored as a link whose target the ignore file leaves out lost that target. - A folder the archive holds no member for, only the files under it, is sent by docker build even when the ignore file leaves out all of them, so COPY . and COPY sub/ copy it empty. The pruned folder now has it. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/docker/prune.py | 21 ++++++++++++++++----- tst/unit/utils/docker/prune_test.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py index 8a0e73b4..c3cee6cf 100644 --- a/src/agent_env/utils/docker/prune.py +++ b/src/agent_env/utils/docker/prune.py @@ -5,6 +5,7 @@ import bisect import functools import heapq +import os import posixpath import re import shutil @@ -103,6 +104,11 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, written[folder][name] = member.size else: written[folder].pop(name, None) + # A folder the archive only implies, as the parent of what it holds, has no member of its own. + for folder, names in kept.items(): + for name in names: + if not os.path.lexists(folder / name): + (folder / name).mkdir(parents=True, exist_ok=True) return written @@ -177,17 +183,22 @@ def _link_target(name: str, links: Mapping[str, str | None]) -> str: def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: - """The names in ``links`` that ``write_pruned`` keeps for ``reads``, and their folders.""" + """The names in ``links``, and the folders they imply, that ``write_pruned`` keeps for ``reads``.""" def sent(name: str) -> bool: return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) + implied = {str(parent) for name in links for parent in PurePosixPath(name).parents if str(parent) != "."} + candidates = links.keys() | implied if reads.whole is not None: - kept = {name for name in links if sent(name)} + kept = {name for name in candidates if sent(name)} else: - kept = {name for name in links if sent(name) and any(_matches(path, name) for path in reads.paths)} - kept |= {name for name in (dockerfile, ignore) if name in links} + kept = {name for name in candidates if sent(name) and any(_matches(path, name) for path in reads.paths)} + # docker build reads the Dockerfile and ignore file where they lead, whatever the ignore file says. + for name in filter(None, (dockerfile, ignore)): + resolved, hops = _resolve(name, links) + kept |= {other for other in (name, resolved, *hops) if other in links} # A link a source only crosses is needed for its path, not for all it leads to. - pending = [name for name in kept if links[name]] + pending = [name for name in kept if links.get(name)] kept |= {name for name in crossed if sent(name)} names = sorted(links) while pending: diff --git a/tst/unit/utils/docker/prune_test.py b/tst/unit/utils/docker/prune_test.py index d078d073..4be50745 100644 --- a/tst/unit/utils/docker/prune_test.py +++ b/tst/unit/utils/docker/prune_test.py @@ -286,6 +286,29 @@ def test_a_name_the_archive_holds_twice_counts_once_at_the_size_left_in_the_fold assert pruned.largest == (("data/b.bin", 100), ("Dockerfile", 30), ("data/a.bin", 10)) +def test_the_dockerfiles_link_target_is_kept_even_when_the_ignore_file_leaves_it_out(tmp_path): + # docker build reads the Dockerfile and its ignore file where they lead, whatever the ignore file says. + files = {"docker/real.Dockerfile": "FROM python\nCOPY app/ /app/\n", "docker/Dockerfile.dockerignore": "docker\n", + "app/main.py": "m"} + archive = _archive(tmp_path / "ctx.tar.gz", files, links={"docker/Dockerfile": "real.Dockerfile"}) + + write_pruned(archive, {tmp_path / "out": "docker/Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"docker/real.Dockerfile", "docker/Dockerfile.dockerignore", "app/main.py"} + + +@pytest.mark.parametrize("dockerfile", ["FROM python\nCOPY . /app\n", "FROM python\nCOPY sub/ /app/\n"], + ids=["whole", "copied"]) +def test_a_folder_the_archive_only_implies_is_written_when_the_ignore_file_leaves_out_all_it_holds(tmp_path, dockerfile): + # docker build sends sub/ as an empty folder here, and COPY sub/ copies it. + archive = _archive(tmp_path / "ctx.tar.gz", {"Dockerfile": dockerfile, ".dockerignore": "sub/*.md\n", + "sub/notes.md": "n"}) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert (tmp_path / "out" / "sub").is_dir() and not any((tmp_path / "out" / "sub").iterdir()) + + def test_each_file_is_written_at_the_time_of_writing_not_the_stored_contexts(tmp_path): # A stored context's files carry the time 0; docker build would read a rewritten one as unchanged. archive = _archive(tmp_path / "ctx.tar.gz", SERVERS) From a506f97fb51543e47fada69127c84bac5996e1c9 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 10:52:34 -0700 Subject: [PATCH 13/14] fix(docker): keep both ignore files in each pruned folder docker build's own frontend reads an empty .dockerignore as leaving nothing out, and the docker/dockerfile:1 frontend reads the context's .dockerignore in its place. The pruned folder kept only the ignore file the reading followed, so with the other frontend a build of it could read files the whole context's leaves out. Both stay now; what is kept follows the file that leaves the most in. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/docker/prune.py | 11 ++++++----- tst/unit/utils/docker/prune_test.py | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py index c3cee6cf..01689e0a 100644 --- a/src/agent_env/utils/docker/prune.py +++ b/src/agent_env/utils/docker/prune.py @@ -81,7 +81,7 @@ def _plan(dockerfile: str, links: Mapping[str, str | None], texts: Mapping[str, # A source the reading found that names nothing here is likelier a misreading than a build that fails. if missing := next((path for path in reads.paths if not any(_matches(path, name) for name in links)), None): reads, crossed = ContextReads(whole=f"{missing} matches nothing in the context"), set() - return _kept(links, reads, dockerfile, ignore, matcher, crossed), reads.whole + return _kept(links, reads, dockerfile, matcher, crossed), reads.whole def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, int]]: @@ -181,8 +181,8 @@ def _link_target(name: str, links: Mapping[str, str | None]) -> str: return "/".join(parts) -def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, ignore: str | None, - matcher: FilePatternMatcher | None, crossed: set[str] = frozenset()) -> set[str]: +def _kept(links: Mapping[str, str | None], reads: ContextReads, dockerfile: str, matcher: FilePatternMatcher | None, + crossed: set[str] = frozenset()) -> set[str]: """The names in ``links``, and the folders they imply, that ``write_pruned`` keeps for ``reads``.""" def sent(name: str) -> bool: return not LEAVINGS.intersection(name.split("/")) and (matcher is None or not matcher(Path(name))) @@ -193,8 +193,9 @@ def sent(name: str) -> bool: kept = {name for name in candidates if sent(name)} else: kept = {name for name in candidates if sent(name) and any(_matches(path, name) for path in reads.paths)} - # docker build reads the Dockerfile and ignore file where they lead, whatever the ignore file says. - for name in filter(None, (dockerfile, ignore)): + # docker build reads the Dockerfile and ignore file where they lead, whatever the ignore file says. Both ignore files + # stay, since docker build's frontends differ on which one an empty .dockerignore leaves in effect. + for name in (dockerfile, *ignore_names(dockerfile)): resolved, hops = _resolve(name, links) kept |= {other for other in (name, resolved, *hops) if other in links} # A link a source only crosses is needed for its path, not for all it leads to. diff --git a/tst/unit/utils/docker/prune_test.py b/tst/unit/utils/docker/prune_test.py index 4be50745..3f1acfb5 100644 --- a/tst/unit/utils/docker/prune_test.py +++ b/tst/unit/utils/docker/prune_test.py @@ -67,8 +67,21 @@ def test_what_the_ignore_file_leaves_out_stays_out_and_a_dockerfiles_own_ignore_ assert set(_files(tmp_path / "root")) == {"Dockerfile", ".dockerignore", "app/main.py", "app/tests/keep.py", "app/notes.md"} - assert set(_files(tmp_path / "sub")) == {"sub/Dockerfile", "sub/Dockerfile.dockerignore", "app/main.py", - "app/tests/t.py", "app/tests/keep.py"} + assert set(_files(tmp_path / "sub")) == {"sub/Dockerfile", "sub/Dockerfile.dockerignore", ".dockerignore", + "app/main.py", "app/tests/t.py", "app/tests/keep.py"} + + +def test_an_empty_dockerfiles_own_ignore_file_leaves_everything_in_and_both_ignore_files_stay(tmp_path): + # docker build's own frontend reads an empty Dockerfile.dockerignore as leaving nothing out; the docker/dockerfile:1 + # frontend reads the context's .dockerignore instead. Both files stay, so either frontend reads what it would. + files = {"Dockerfile": "FROM python\nCOPY app/ /app/\n", "Dockerfile.dockerignore": "", ".dockerignore": "**/*.md\n", + "app/main.py": "m", "app/notes.md": "n"} + archive = _archive(tmp_path / "ctx.tar.gz", files) + + write_pruned(archive, {tmp_path / "out": "Dockerfile"}) + + assert set(_files(tmp_path / "out")) == {"Dockerfile", "Dockerfile.dockerignore", ".dockerignore", "app/main.py", + "app/notes.md"} def test_a_dockerfile_that_reads_all_of_the_context_gets_what_docker_would_send_and_says_why(tmp_path): From e836d1b5e39945ea0fd91de601ff13a756651915 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 11:26:04 -0700 Subject: [PATCH 14/14] fix(docker): write nothing outside a pruned folder, read heredocs as BuildKit does, follow link chains - A folder after the first that shares an archive gets its files copied, without the extraction's check, so a link leading into the first folder could lead a copy out of a later one. Every copy and every folder made for one is now checked to land inside its folder. - A member named out of the context (../) is refused before anything is written, as extract refuses it, so no folder it implies is made outside. - A heredoc is a word: a file descriptor, <<, a - and the rest of the word as its delimiter, as BuildKit reads it. RUN < releases/latest/app with releases/latest -> v2 kept nothing under the literal path. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/docker/dockerfile.py | 54 ++++++++++++++++++------ src/agent_env/utils/docker/prune.py | 17 +++++++- tst/unit/utils/docker/dockerfile_test.py | 19 +++++++++ tst/unit/utils/docker/prune_test.py | 44 +++++++++++++++++++ 4 files changed, 119 insertions(+), 15 deletions(-) diff --git a/src/agent_env/utils/docker/dockerfile.py b/src/agent_env/utils/docker/dockerfile.py index 39121b91..2c55b11c 100644 --- a/src/agent_env/utils/docker/dockerfile.py +++ b/src/agent_env/utils/docker/dockerfile.py @@ -10,8 +10,9 @@ from dataclasses import dataclass _DIRECTIVE = re.compile(r"#\s*([A-Za-z][A-Za-z0-9_-]*)\s*=\s*(\S+)\s*") -# ``<<<`` is a shell here-string, which docker build leaves to the shell, not a heredoc. -_HEREDOC = re.compile(r"(? list[str] | None: def opens_heredoc(word: str) -> bool: """Whether ``word``, written in a shell-form instruction, would open a heredoc.""" - return _HEREDOC.fullmatch(word) is not None + return _heredoc(word) is not None def _heredoc_names(arguments: str) -> list[tuple[str, str]]: - """Each heredoc ``arguments`` opens, as (its ``-``, its name): an unquoted ``< tuple[str, str] | None: + """The heredoc ``word`` opens, as (its ``-``, its delimiter, unquoted); None when it opens none.""" + if not (match := _HEREDOC.fullmatch(word)): + return None + delimiter, quote, at = [], None, 0 + while at < len(match.group(2)): + char = match.group(2)[at] + if char == "\\" and quote != "'" and at + 1 < len(match.group(2)): + at += 1 + delimiter.append(match.group(2)[at]) + elif char == quote: + quote = None + elif char in "\"'" and quote is None: + quote = char + else: + delimiter.append(char) + at += 1 + return (match.group(1), "".join(delimiter)) if delimiter else None + + +def _words(arguments: str) -> list[str]: + """``arguments`` split into words on whitespace outside quotes, each as written, quotes and escapes kept.""" + words, word, quote, at = [], [], None, 0 while at < len(arguments): char = arguments[at] - if char == "\\" and quote != "'": + if char == "\\" and quote != "'" and at + 1 < len(arguments): + word.append(arguments[at:at + 2]) at += 2 continue if quote: quote = None if char == quote else quote elif char in "\"'": quote = char - elif heredoc := _HEREDOC.match(arguments, at): - found.append((heredoc.group(1), heredoc.group(3))) - at = heredoc.end() + elif char.isspace(): + if word: + words.append("".join(word)) + word = [] + at += 1 continue + word.append(char) at += 1 - return found + return words + (["".join(word)] if word else []) def copy_arguments(instruction: Instruction) -> CopyArguments | None: @@ -179,7 +207,7 @@ def copy_arguments(instruction: Instruction) -> CopyArguments | None: args = rest.split() if len(args) < 2: return None - sources = tuple(source for source in args[:-1] if json_form or not _HEREDOC.fullmatch(source)) + sources = tuple(source for source in args[:-1] if json_form or _heredoc(source) is None) return CopyArguments(instruction.keyword, tuple(flags), sources, args[-1], json_form, heredoc=len(sources) < len(args) - 1) diff --git a/src/agent_env/utils/docker/prune.py b/src/agent_env/utils/docker/prune.py index 01689e0a..1cad5878 100644 --- a/src/agent_env/utils/docker/prune.py +++ b/src/agent_env/utils/docker/prune.py @@ -94,6 +94,7 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, first: Path | None = None for folder in (folder for folder, names in kept.items() if name in names): if member.isreg() and first is not None: + _check_inside(folder, name) (folder / name).parent.mkdir(parents=True, exist_ok=True) shutil.copy2(first, folder / name) else: @@ -108,10 +109,18 @@ def _write(archive: Path, kept: Mapping[Path, set[str]]) -> dict[Path, dict[str, for folder, names in kept.items(): for name in names: if not os.path.lexists(folder / name): + _check_inside(folder, name) (folder / name).mkdir(parents=True, exist_ok=True) return written +def _check_inside(folder: Path, name: str) -> None: + """Raise ValueError when ``name`` in ``folder``, through any link written there, would land outside it.""" + top = os.path.realpath(folder) + if os.path.commonpath([top, os.path.realpath(folder / name)]) != top: + raise ValueError(f"{name!r} in the build context leads out of {folder}") + + def _member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: """``member`` as ``write_pruned`` writes it: a link as it is, wherever it leads, and at the time of writing.""" if (member := context_member(member, out, links_out=True)) is None: @@ -203,7 +212,9 @@ def sent(name: str) -> bool: kept |= {name for name in crossed if sent(name)} names = sorted(links) while pending: - target = _link_target(pending.pop(), links) + # A target is followed through the links on its way, which stay for its path. + target, hops = _resolve(_link_target(pending.pop(), links), links) + kept |= {hop for hop in hops if sent(hop)} for other in _at_or_under(names, target) if target else names: if other not in kept and sent(other): kept.add(other) @@ -265,6 +276,8 @@ def _part_pattern(glob: str) -> re.Pattern[str] | None: def _member_name(member: tarfile.TarInfo) -> str | None: - """``member``'s path in the context; None for the context's own folder.""" + """``member``'s path in the context; None for the context's own folder. Raises ValueError for one leading out.""" name = posixpath.normpath(member.name.lstrip("/")) + if name == ".." or name.startswith("../"): + raise ValueError(f"{member.name!r} in the build context leads out of it") return None if name == "." else name diff --git a/tst/unit/utils/docker/dockerfile_test.py b/tst/unit/utils/docker/dockerfile_test.py index c682fbee..d5f4d613 100644 --- a/tst/unit/utils/docker/dockerfile_test.py +++ b/tst/unit/utils/docker/dockerfile_test.py @@ -71,6 +71,25 @@ def test_a_heredoc_is_only_ever_opened_outside_quotes_in_shell_form(): ("COPY", ())] +@pytest.mark.parametrize(("line", "heredocs"), [ + ("RUN < /x\n", ()), +], ids=["dotted-delimiter", "quoted-dotted", "file-descriptor", "no-space-before"]) +def test_a_heredoc_is_a_word_opening_with_lt_lt_and_its_delimiter_is_the_rest_of_that_word(line, heredocs): + # docker build closes <