Compare commits

...

4 Commits

Author SHA1 Message Date
didericis-codex 6fea44067f fix(manifest): preserve declared boolean fields
tracker-policy-pr / check-pr (pull_request) Successful in 11s
test / integration-docker (pull_request) Successful in 18s
test / unit (pull_request) Successful in 42s
test / integration-firecracker (pull_request) Successful in 3m35s
test / coverage (pull_request) Successful in 26s
test / publish-infra (pull_request) Has been skipped
prd-number / assign-numbers (push) Failing after 29s
test / integration-docker (push) Successful in 44s
lint / lint (push) Successful in 1m0s
test / unit (push) Successful in 56s
Update Quality Badges / update-badges (push) Successful in 1m45s
test / integration-firecracker (push) Successful in 6m54s
test / coverage (push) Successful in 26s
test / publish-infra (push) Successful in 2m22s
2026-07-22 18:35:31 +00:00
didericis-claude bf8ff91b31 fix(manifest): presence-aware runtime merge for nested_containers
test / integration-docker (pull_request) Successful in 17s
test / unit (pull_request) Successful in 44s
lint / lint (push) Successful in 57s
test / integration-firecracker (pull_request) Successful in 3m31s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 13s
The runtime merge (merge_bottles_runtime / _fold_two_bottles) was using
OR semantics for nested_containers, which meant `nested_containers: false`
in a later bottle could never override an earlier `true`.

Fix: add `nested_containers_explicit: bool` to ManifestBottle, set to
True only when the key is present in the source dict. The runtime merge
functions now use the override's value only when it was explicitly
supplied; otherwise the base's value is preserved. This matches the
documented last-wins / presence-aware semantics used by supervise and
agent_provider, and aligns with _merge_bottles (the file-based extends
path) which already did presence checks via `"nested_containers" in
child_raw`.

Tests: rename existing test to clarify the preserved-when-omitted case,
add explicit-false-overrides-true and explicit-true-overrides-false cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-07-22 17:58:00 +00:00
didericis-claude 315ed04979 test: update unit tests to match podman-in-nested-containers-layer change
tracker-policy-pr / check-pr (pull_request) Successful in 8s
test / integration-docker (pull_request) Successful in 15s
test / unit (pull_request) Successful in 38s
lint / lint (push) Successful in 54s
test / integration-firecracker (pull_request) Successful in 4m21s
test / coverage (pull_request) Successful in 24s
test / publish-infra (pull_request) Has been skipped
- Replace test_all_install_podman with test_none_install_podman: agent
  images intentionally no longer include podman; the nested-containers
  derived layer is where it lives (see build_image in nested_containers.py).
- Add podman to the package assertions in test_macos_nested_containers so
  its presence in the derived Dockerfile is explicitly verified.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-07-22 17:36:51 +00:00
didericis-claude 9a04ab262b review: remove security-boundary note, move podman into nested-containers layer
test / integration-docker (pull_request) Successful in 19s
test / unit (pull_request) Failing after 39s
lint / lint (push) Successful in 56s
tracker-policy-pr / check-pr (pull_request) Failing after 13m51s
test / integration-firecracker (pull_request) Failing after 14m1s
test / coverage (pull_request) Has been skipped
test / publish-infra (pull_request) Has been skipped
- Drop the module-level paragraph emphasizing the single-UID security
  boundary (#issuecomment-5010): redundant given the safe environment.
- Move `podman` from each agent image (claude, codex, pi) into the
  `build_image` derived layer so bottles without `nested_containers`
  pay no image-size cost (#issuecomment-5013).
- Update `build_image` docstring and inline comments to reflect the move.
- Add TODO(#394) noting the planned docker-layer abstraction refactor.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-07-22 17:30:58 +00:00
10 changed files with 134 additions and 43 deletions
@@ -19,11 +19,6 @@ what would send podman down the `newuidmap` path that cannot work here.
The agent still talks to `docker` and `docker compose`; those speak to The agent still talks to `docker` and `docker compose`; those speak to
podman's Docker-compatible API socket, so nothing in the agent's habits podman's Docker-compatible API socket, so nothing in the agent's habits
changes. changes.
Nested containers run *within* the bottle boundary, not inside a new one: the
single-UID mapping means `root` in a nested container is the agent user
outside it. This is for build and test workloads, not for sandboxing
untrusted code.
""" """
from __future__ import annotations from __future__ import annotations
@@ -65,9 +60,11 @@ def build_image(
) -> str: ) -> str:
"""Layer the nested-container tooling onto an already-built agent image. """Layer the nested-container tooling onto an already-built agent image.
Only what the flag is meant to gate lands here. Podman itself is already Podman and its networking stack live here rather than in the base agent
in every built-in agent image (issue #451); the storage/network helpers, images so that bottles without the flag pay no image-size cost.
the Docker CLI, and the compose plugin are the ~100MB this flag buys.
# TODO(#394): replace this hand-rolled Dockerfile with a docker-layer
# abstraction once that infrastructure exists.
""" """
image = f"{base_image}{IMAGE_SUFFIX}" image = f"{base_image}{IMAGE_SUFFIX}"
init_script = Path(__file__).with_name("nested-containers-init.sh") init_script = Path(__file__).with_name("nested-containers-init.sh")
@@ -82,9 +79,11 @@ def build_image(
"COPY --from=docker_cli /usr/local/libexec/docker/cli-plugins/" "COPY --from=docker_cli /usr/local/libexec/docker/cli-plugins/"
"docker-compose /usr/local/libexec/docker/cli-plugins/docker-compose\n" "docker-compose /usr/local/libexec/docker/cli-plugins/docker-compose\n"
"RUN apt-get update \\\n" "RUN apt-get update \\\n"
# podman 5's networking stack, installed explicitly because the # podman 5's networking stack, installed explicitly because
# base image's `--no-install-recommends` podman does not pull it # --no-install-recommends omits it and each missing piece fails
# in and each piece fails at a different, misleading layer: # at a different, misleading layer:
# podman -> moved here from the base agent images so that
# bottles without nested_containers pay no cost
# passt -> `pasta`, the default rootless netns helper # passt -> `pasta`, the default rootless netns helper
# (podman 4 used slirp4netns); without it # (podman 4 used slirp4netns); without it
# nothing starts: "could not find pasta" # nothing starts: "could not find pasta"
@@ -95,7 +94,7 @@ def build_image(
# looks healthy # looks healthy
# slirp4netns stays as the documented fallback for pasta. # slirp4netns stays as the documented fallback for pasta.
" && apt-get install -y --no-install-recommends " " && apt-get install -y --no-install-recommends "
"aardvark-dns fuse-overlayfs netavark nftables passt " "aardvark-dns fuse-overlayfs netavark nftables passt podman "
"slirp4netns uidmap \\\n" "slirp4netns uidmap \\\n"
" && rm -rf /var/lib/apt/lists/* \\\n" " && rm -rf /var/lib/apt/lists/* \\\n"
# Deliberate: an empty subordinate range keeps podman on the # Deliberate: an empty subordinate range keeps podman on the
-1
View File
@@ -26,7 +26,6 @@ RUN apt-get update \
ca-certificates \ ca-certificates \
curl \ curl \
openssh-client \ openssh-client \
podman \
ripgrep \ ripgrep \
iproute2 \ iproute2 \
dnsutils \ dnsutils \
-1
View File
@@ -11,7 +11,6 @@ RUN apt-get update \
ca-certificates \ ca-certificates \
curl \ curl \
openssh-client \ openssh-client \
podman \
procps \ procps \
ripgrep \ ripgrep \
&& rm -rf /var/lib/apt/lists/* && rm -rf /var/lib/apt/lists/*
-1
View File
@@ -11,7 +11,6 @@ RUN apt-get update \
curl \ curl \
fd-find \ fd-find \
openssh-client \ openssh-client \
podman \
ripgrep \ ripgrep \
&& ln -s /usr/bin/fdfind /usr/local/bin/fd \ && ln -s /usr/bin/fdfind /usr/local/bin/fd \
&& rm -rf /var/lib/apt/lists/* && rm -rf /var/lib/apt/lists/*
+5
View File
@@ -49,6 +49,10 @@ class ManifestBottle:
# costs image weight, a resident service, and relaxed guest device modes # costs image weight, a resident service, and relaxed guest device modes
# that the majority of bottles never need. # that the majority of bottles never need.
nested_containers: bool = False nested_containers: bool = False
# Source fields retained across extends/runtime composition. Boolean
# defaults otherwise erase the distinction between "omitted" and an
# explicitly declared value (especially False).
declared_fields: frozenset[str] = frozenset()
@classmethod @classmethod
def from_dict(cls, name: str, raw: object) -> "ManifestBottle": def from_dict(cls, name: str, raw: object) -> "ManifestBottle":
@@ -139,4 +143,5 @@ class ManifestBottle:
env=env, agent_provider=agent_provider, git=git, env=env, agent_provider=agent_provider, git=git,
git_user=git_user, egress=egress, supervise=supervise_raw, git_user=git_user, egress=egress, supervise=supervise_raw,
nested_containers=nested_raw, nested_containers=nested_raw,
declared_fields=frozenset(d),
) )
+31 -20
View File
@@ -8,6 +8,17 @@ from .manifest_git import ManifestGitUser, parse_git_gate_config
from .manifest_util import ManifestError, as_json_object from .manifest_util import ManifestError, as_json_object
def _overlay_declared_bool(
base: ManifestBottle,
override: ManifestBottle,
field: str,
) -> bool:
"""Overlay a defaulted boolean only when override declared it."""
value = getattr(override if field in override.declared_fields else base, field)
assert isinstance(value, bool)
return value
def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle": def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
"""Merge an ordered list of pre-resolved ManifestBottle objects. """Merge an ordered list of pre-resolved ManifestBottle objects.
@@ -15,16 +26,13 @@ def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
the same field-merge rules as the file-based extends machinery: the same field-merge rules as the file-based extends machinery:
env: dict merge, later wins; git_user: per-field overlay, later env: dict merge, later wins; git_user: per-field overlay, later
wins on non-empty; git (repos): union by name, later wins; egress wins on non-empty; git (repos): union by name, later wins; egress
routes: concatenate; agent_provider, supervise: later routes: concatenate; agent_provider, supervise, nested_containers:
replaces; nested_containers: OR (see below). later replaces (presence-aware).
nested_containers is OR'd rather than replaced because these objects Defaulted booleans use presence-aware replacement: if the later bottle
are already resolved: a bottle that never mentions the key is was loaded from a source that explicitly declared the key, its value
indistinguishable from one that sets it false, so "later replaces" wins (so an explicit `false` can override an earlier `true`). If the
would let any bottle composed after a container-enabled one silently later bottle never mentioned the key, the earlier value is preserved.
drop the capability. The file-based `extends:` path still sees the
raw keys, so there an explicit `nested_containers: false` in a child
turns it back off.
""" """
if not bottles: if not bottles:
raise ValueError("merge_bottles_runtime requires at least one bottle") raise ValueError("merge_bottles_runtime requires at least one bottle")
@@ -62,8 +70,11 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle
git=merged_git, git=merged_git,
git_user=merged_git_user, git_user=merged_git_user,
egress=merged_egress, egress=merged_egress,
supervise=override.supervise, supervise=_overlay_declared_bool(base, override, "supervise"),
nested_containers=base.nested_containers or override.nested_containers, nested_containers=_overlay_declared_bool(
base, override, "nested_containers"
),
declared_fields=base.declared_fields | override.declared_fields,
) )
@@ -215,8 +226,11 @@ def _fold_two_bottles(
git=merged_git, git=merged_git,
git_user=merged_git_user, git_user=merged_git_user,
egress=merged_egress, egress=merged_egress,
supervise=later.supervise, supervise=_overlay_declared_bool(earlier, later, "supervise"),
nested_containers=earlier.nested_containers or later.nested_containers, nested_containers=_overlay_declared_bool(
earlier, later, "nested_containers"
),
declared_fields=earlier.declared_fields | later.declared_fields,
), merged_repos_raw ), merged_repos_raw
@@ -274,13 +288,9 @@ def _merge_bottles(
if "agent_provider" in child_raw if "agent_provider" in child_raw
else parent.agent_provider else parent.agent_provider
) )
merged_supervise = ( merged_supervise = _overlay_declared_bool(parent, child, "supervise")
child.supervise if "supervise" in child_raw else parent.supervise merged_nested_containers = _overlay_declared_bool(
) parent, child, "nested_containers"
merged_nested_containers = (
child.nested_containers
if "nested_containers" in child_raw
else parent.nested_containers
) )
validate_egress_routes(name, merged_egress.routes) validate_egress_routes(name, merged_egress.routes)
@@ -292,6 +302,7 @@ def _merge_bottles(
egress=merged_egress, egress=merged_egress,
supervise=merged_supervise, supervise=merged_supervise,
nested_containers=merged_nested_containers, nested_containers=merged_nested_containers,
declared_fields=parent.declared_fields | child.declared_fields,
) )
+4 -2
View File
@@ -21,10 +21,12 @@ class TestBuiltinAgentImages(unittest.TestCase):
r"(?m)^FROM node:22-trixie-slim\s*$", r"(?m)^FROM node:22-trixie-slim\s*$",
) )
def test_all_install_podman(self): def test_none_install_podman(self):
# podman lives in the nested-containers derived layer (nested_containers.py),
# not in the base agent images, so bottles without the flag pay no cost.
for dockerfile in _AGENT_DOCKERFILES: for dockerfile in _AGENT_DOCKERFILES:
with self.subTest(provider=dockerfile.parent.name): with self.subTest(provider=dockerfile.parent.name):
self.assertRegex( self.assertNotRegex(
dockerfile.read_text(), dockerfile.read_text(),
re.compile(r"(?m)^\s*podman(?:\s|\\|$)"), re.compile(r"(?m)^\s*podman(?:\s|\\|$)"),
) )
+2 -2
View File
@@ -108,7 +108,7 @@ class TestNestedContainersImage(unittest.TestCase):
calls.append((image, context, dockerfile)) calls.append((image, context, dockerfile))
text = Path(dockerfile).read_text(encoding="utf-8") text = Path(dockerfile).read_text(encoding="utf-8")
self.assertIn("FROM agent:base", text) self.assertIn("FROM agent:base", text)
self.assertIn("aardvark-dns fuse-overlayfs netavark nftables passt", text) self.assertIn("aardvark-dns fuse-overlayfs netavark nftables passt podman", text)
self.assertIn("USER node", text) self.assertIn("USER node", text)
self.assertTrue((Path(context) / "nested-containers-init.sh").is_file()) self.assertTrue((Path(context) / "nested-containers-init.sh").is_file())
@@ -128,7 +128,7 @@ class TestNestedContainersImage(unittest.TestCase):
seen.append(Path(dockerfile).read_text(encoding="utf-8")) seen.append(Path(dockerfile).read_text(encoding="utf-8"))
nested_containers.build_image("agent:base", build) nested_containers.build_image("agent:base", build)
for package in ("passt", "nftables", "aardvark-dns"): for package in ("podman", "passt", "nftables", "aardvark-dns"):
self.assertIn(package, seen[0]) self.assertIn(package, seen[0])
def test_strips_subordinate_ranges_so_podman_avoids_newuidmap(self) -> None: def test_strips_subordinate_ranges_so_podman_avoids_newuidmap(self) -> None:
+44 -4
View File
@@ -56,16 +56,56 @@ class TestMergeBottlesRuntime(unittest.TestCase):
result = merge_bottles_runtime([base, override]) result = merge_bottles_runtime([base, override])
self.assertFalse(result.supervise) self.assertFalse(result.supervise)
def test_nested_containers_survives_a_later_bottle(self): def test_supervise_survives_a_later_bottle_that_omits_the_key(self):
"""OR, not replace: a resolved bottle that never mentioned the key is disabled = _bottle(supervise=False)
indistinguishable from one that set it false, so `--bottle with-docker quiet = _bottle(env={"X": "1"})
--bottle claude-dev` must not silently drop the capability.""" self.assertFalse(merge_bottles_runtime([disabled, quiet]).supervise)
def test_supervise_explicit_true_overrides_earlier_false(self):
disabled = _bottle(supervise=False)
enabled = _bottle(supervise=True)
self.assertTrue(merge_bottles_runtime([disabled, enabled]).supervise)
def test_nested_containers_survives_a_later_bottle_that_omits_the_key(self):
"""A bottle that never mentions nested_containers must not silently
drop the capability: `--bottle with-docker --bottle claude-dev`."""
enabled = _bottle(nested_containers=True) enabled = _bottle(nested_containers=True)
quiet = _bottle(env={"X": "1"}) quiet = _bottle(env={"X": "1"})
self.assertTrue(merge_bottles_runtime([enabled, quiet]).nested_containers) self.assertTrue(merge_bottles_runtime([enabled, quiet]).nested_containers)
self.assertTrue(merge_bottles_runtime([quiet, enabled]).nested_containers) self.assertTrue(merge_bottles_runtime([quiet, enabled]).nested_containers)
self.assertFalse(merge_bottles_runtime([quiet, quiet]).nested_containers) self.assertFalse(merge_bottles_runtime([quiet, quiet]).nested_containers)
def test_nested_containers_explicit_false_overrides_earlier_true(self):
"""An explicit nested_containers: false in a later bottle must win
over an earlier true (last-wins, presence-aware)."""
enabled = _bottle(nested_containers=True)
disabled = _bottle(nested_containers=False)
self.assertFalse(merge_bottles_runtime([enabled, disabled]).nested_containers)
def test_nested_containers_explicit_true_overrides_earlier_false(self):
enabled = _bottle(nested_containers=True)
disabled = _bottle(nested_containers=False)
self.assertTrue(merge_bottles_runtime([disabled, enabled]).nested_containers)
def test_extended_boolean_values_remain_explicit_at_runtime(self):
idx = _index(
bottles={
"enabled": {"nested_containers": True, "supervise": True},
"parent": {},
"disabled_child": {
"extends": "parent",
"nested_containers": False,
"supervise": False,
},
},
agents={"impl": {"bottle": "enabled", "skills": [], "prompt": ""}},
)
result = idx.load_for_agent(
"impl", ("enabled", "disabled_child")
).bottle
self.assertFalse(result.nested_containers)
self.assertFalse(result.supervise)
def test_three_bottles_merged_left_to_right(self): def test_three_bottles_merged_left_to_right(self):
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"}) b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
b2 = _bottle(env={"B": "2", "C": "2"}) b2 = _bottle(env={"B": "2", "C": "2"})
+37
View File
@@ -68,6 +68,23 @@ class TestExtendsBasic(unittest.TestCase):
self.assertTrue(m.bottles["base"].supervise) self.assertTrue(m.bottles["base"].supervise)
self.assertFalse(m.bottles["off"].supervise) self.assertFalse(m.bottles["off"].supervise)
def test_child_overrides_nested_containers_scalar(self):
m = _build(
base={"nested_containers": True},
off={"extends": "base", "nested_containers": False},
)
self.assertTrue(m.bottles["base"].nested_containers)
self.assertFalse(m.bottles["off"].nested_containers)
def test_inherited_boolean_declaration_is_preserved(self):
m = _build(
base={"nested_containers": True, "supervise": False},
child={"extends": "base"},
)
child = m.bottles["child"]
self.assertIn("nested_containers", child.declared_fields)
self.assertIn("supervise", child.declared_fields)
def test_parent_resolved_once_for_multiple_children(self): def test_parent_resolved_once_for_multiple_children(self):
# Two children sharing one parent: both inherit; the parent # Two children sharing one parent: both inherit; the parent
# is resolved once + cached. (Cache behavior is internal; we # is resolved once + cached. (Cache behavior is internal; we
@@ -477,6 +494,26 @@ class TestExtendsMultiParent(unittest.TestCase):
) )
self.assertTrue(m.bottles["child"].supervise) self.assertTrue(m.bottles["child"].supervise)
def test_later_parent_omitting_boole_preserves_earlier_values(self):
m = _build(
p1={"nested_containers": True, "supervise": False},
p2={"env": {"FROM_P2": "1"}},
child={"extends": ["p1", "p2"]},
)
child = m.bottles["child"]
self.assertTrue(child.nested_containers)
self.assertFalse(child.supervise)
def test_later_parent_explicit_boole_override_earlier_values(self):
m = _build(
p1={"nested_containers": True, "supervise": False},
p2={"nested_containers": False, "supervise": True},
child={"extends": ["p1", "p2"]},
)
child = m.bottles["child"]
self.assertFalse(child.nested_containers)
self.assertTrue(child.supervise)
def test_child_supervise_overrides_all_parents(self): def test_child_supervise_overrides_all_parents(self):
m = _build( m = _build(
p1={"supervise": True}, p1={"supervise": True},