diff --git a/README.md b/README.md index 2a54160..2c742b8 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,88 @@ On compatible macOS hosts, the default backend requires Apple's `container` CLI Use `BOT_BOTTLE_BACKEND=docker ./cli.py start ` on hosts where neither Apple Container nor KVM is available and Docker is the desired backend. +### Containers inside a bottle + +A bottle may set `nested_containers: true`. On the macOS backend this starts a +guest-local, rootless **podman** service after the bottle is registered and +exposes its Docker-compatible API socket, so the agent still runs `docker` and +`docker compose`. Nothing is mounted from the host: Docker Desktop's socket +stays out of the bottle and the guest gains no outer VM capabilities. Backends +that cannot do this (`docker`, `firecracker`) reject the flag rather than +silently ignore it. + +Rootless Docker was tried first and does not work here at all: Apple +Container's capability bounding set omits `CAP_SYS_ADMIN`, which the kernel +requires to write a multi-range `uid_map`. See +[`docs/research/rootless-docker-in-apple-container-spike.md`](docs/research/rootless-docker-in-apple-container-spike.md). + +The tradeoff to understand before enabling it: podman avoids that requirement +by falling back to a single-UID mapping, so nested containers provide **no +isolation from the agent itself** — `root` inside a nested container is the +agent user outside it. Nested containers are a build/test convenience, not a +security boundary. The bottle remains the boundary. + +Pulling images goes through the bottle's egress proxy like every other +request, so each registry needs a route — **and so does the CDN it redirects +layer blobs to**, which is a different host. Without the CDN route the pull +authenticates, fetches the manifest, then 403s partway through. + +Docker Hub and GHCR additionally need `preserve_auth: true`: their token dance +uses a client-fetched per-scope bearer token that the proxy would otherwise +strip. Turn DLP off on every registry and CDN route — the bodies are +compressed layer blobs that no detector can read, and buffering them is what +triggers the shared-proxy OOM in #455. + +```yaml +nested_containers: true +egress: + routes: + # Docker Hub: registry, token endpoint, blob CDN. + - host: registry-1.docker.io + preserve_auth: true + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: auth.docker.io + preserve_auth: true + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: production.cloudfront.docker.com + dlp: { outbound_detectors: false, inbound_detectors: false } + # GHCR: registry + blob CDN. + - host: ghcr.io + preserve_auth: true + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: pkg-containers.githubusercontent.com + dlp: { outbound_detectors: false, inbound_detectors: false } + # quay.io: registry + blob CDNs. No preserve_auth needed for public pulls. + - host: quay.io + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: cdn01.quay.io + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: cdn02.quay.io + dlp: { outbound_detectors: false, inbound_detectors: false } + - host: cdn03.quay.io + dlp: { outbound_detectors: false, inbound_detectors: false } +``` + +`mcr.microsoft.com` and `registry.k8s.io` follow the same shape and also +redirect blobs elsewhere (`*.data.mcr.microsoft.com` and +`us-*-docker.pkg.dev` respectively); route whichever host the 403 names. + +Inside a nested container the same allowlist applies: an allowlisted host +returns 200 and anything else gets a 403 straight from the proxy. The +gateway's CA bundle and proxy settings are wired in automatically, so +`docker run … curl https://…` works with no extra flags — no `--add-host`, +`-e`, or `-v`. + +Two things worth knowing when testing that: + +- Public DNS inside a nested container fails **by design**. Everything + egresses through the proxy, so `nslookup` failing is expected and is not + evidence of a problem. +- Alpine's BusyBox `wget` drops the connection after the proxy's TLS + interception and reports `error getting response`, even though the proxy + logs the decrypted request and returns a response. Use `curl` to test + egress; BusyBox `wget` will lie to you. + ### Firecracker on Linux On Linux, a KVM-capable host defaults to the Firecracker backend. It needs: diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 0c45852..ff5223e 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -302,6 +302,11 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): name: str + # Whether this backend can run a container engine *inside* the bottle. + # Backends that cannot must reject `nested_containers: true` rather than + # reach for a host daemon socket (issue #392). + supports_nested_containers: bool = False + def prepare(self, spec: BottleSpec, stage_dir: Path) -> PlanT: """Template method: run cross-backend host-side validation, then delegate to the subclass's `_resolve_plan` for the @@ -315,12 +320,16 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): prepare_egress, prepare_git_gate, prepare_supervise, + reject_nested_containers, resolve_manifest_dockerfile, write_launch_metadata, ) manifest = self._validate(spec) + if not self.supports_nested_containers: + reject_nested_containers(self.name, manifest) + self._preflight() from ..git_gate_host_key import preflight_host_keys diff --git a/bot_bottle/backend/macos_container/backend.py b/bot_bottle/backend/macos_container/backend.py index ee2cc05..2448c00 100644 --- a/bot_bottle/backend/macos_container/backend.py +++ b/bot_bottle/backend/macos_container/backend.py @@ -31,6 +31,7 @@ class MacosContainerBottleBackend( `--backend=macos-container`.""" name = "macos-container" + supports_nested_containers = True @classmethod def is_available(cls) -> bool: diff --git a/bot_bottle/backend/macos_container/bottle_plan.py b/bot_bottle/backend/macos_container/bottle_plan.py index f93a092..b167685 100644 --- a/bot_bottle/backend/macos_container/bottle_plan.py +++ b/bot_bottle/backend/macos_container/bottle_plan.py @@ -20,6 +20,9 @@ class MacosContainerBottlePlan(BottlePlan): # bottle is registered. See launch.py's stamp for why it lives here and not # only in the exec-time proxy env. identity_token: str = "" + # Guest-local container engine (issue #392). Gates the derived image, the + # device-mode relaxation, and the resident podman service. + nested_containers: bool = False @property def container_name(self) -> str: diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 9a5ddae..45b0a76 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -66,6 +66,7 @@ from .gateway_hosts import ( refresh_gateway_host, set_gateway_host, ) +from . import nested_containers as nested_containers_mod from .bottle_plan import MacosContainerBottlePlan from ...orchestrator.config_store import resolve_teardown_timeout from .consolidated_launch import ( @@ -82,10 +83,14 @@ _AGENT_SLEEP_SECONDS = "2147483647" def build_or_load_images(plan: MacosContainerBottlePlan) -> BottleImages: """Resolve the agent image ref for this plan. The gateway's own image is built by `ensure_gateway` — it belongs to the shared singleton.""" + return BottleImages(agent=_layer_nested_containers(plan, _agent_image(plan))) + + +def _agent_image(plan: MacosContainerBottlePlan) -> str: committed = read_committed_image(plan.slug) if committed and container_mod.image_exists(committed): info(f"using committed image {committed!r}") - return BottleImages(agent=committed) + return committed if plan.spec.image_policy == "cached": if not container_mod.image_exists(plan.image): die( @@ -93,9 +98,31 @@ def build_or_load_images(plan: MacosContainerBottlePlan) -> BottleImages: "run without --cached-images to build it" ) info(f"using cached agent image {plan.image!r}") - return BottleImages(agent=plan.image) + return plan.image container_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) - return BottleImages(agent=plan.image) + return plan.image + + +def _layer_nested_containers( + plan: MacosContainerBottlePlan, agent_image: str, +) -> str: + """Add the guest-local container tooling on top of the agent image. + + A separate derived tag, not the provider Dockerfile, so bottles that never + ask for nested containers carry none of its weight. + """ + if not plan.nested_containers: + return agent_image + derived = f"{agent_image}{nested_containers_mod.IMAGE_SUFFIX}" + if plan.spec.image_policy == "cached": + if not container_mod.image_exists(derived): + die( + f"cached nested-container image {derived!r} not found; " + "run without --cached-images to build it" + ) + info(f"using cached nested-container image {derived!r}") + return derived + return nested_containers_mod.build_image(agent_image, container_mod.build_image) @contextmanager @@ -196,6 +223,10 @@ def launch( # token above, so — unlike the run-time env — the plan CAN carry it. plan = dataclasses.replace(plan, identity_token=ctx.identity_token) + exec_env = { + **_identity_proxy_env(endpoint, ctx.identity_token), + **nested_containers_mod.guest_env(plan.nested_containers), + } bottle = MacosContainerBottle( plan.container_name, teardown, @@ -209,10 +240,16 @@ def launch( ), terminal_color=plan.spec.color, agent_workdir=plan.workspace_plan.workdir, - exec_env=_identity_proxy_env(endpoint, ctx.identity_token), + exec_env=exec_env, ) bottle.prompt_path = provision(plan, bottle) + if plan.nested_containers: + nested_containers_mod.prepare_guest_devices( + plan.container_name, container_mod.exec_container_as_root, + ) + nested_containers_mod.start(bottle) + yield bottle finally: teardown() diff --git a/bot_bottle/backend/macos_container/nested-containers-init.sh b/bot_bottle/backend/macos_container/nested-containers-init.sh new file mode 100755 index 0000000..e4c3425 --- /dev/null +++ b/bot_bottle/backend/macos_container/nested-containers-init.sh @@ -0,0 +1,210 @@ +#!/bin/sh +set -eu + +uid="$(id -u)" +if [ "$uid" -eq 0 ]; then + echo "refusing to run the guest container engine as root" >&2 + exit 1 +fi + +# Every piece of podman 5's networking stack is checked here, because each +# one fails at a different and misleading layer if it is absent: no pasta and +# nothing starts at all; no nft and netavark cannot build the bridge every +# compose file expects; no aardvark-dns and DNS inside nested containers fails +# while everything else looks healthy. +for command in podman docker fuse-overlayfs pasta nft slirp4netns; do + command -v "$command" >/dev/null 2>&1 || { + echo "missing nested-container prerequisite: $command" >&2 + exit 1 + } +done + +# The inverse of the rootless-Docker check, and the whole point of the podman +# variant: a subordinate range would push podman onto newuidmap, which cannot +# write a multi-range uid_map without CAP_SYS_ADMIN in this guest. An empty +# range keeps it on the single-UID self-mapping an unprivileged process may +# write itself. +if grep -q "^$(id -un):" /etc/subuid 2>/dev/null; then + echo "unexpected subordinate UID range for $(id -un): podman would" >&2 + echo "require CAP_SYS_ADMIN via newuidmap in this guest" >&2 + exit 1 +fi + +for device in /dev/fuse /dev/net/tun; do + [ -r "$device" ] && [ -w "$device" ] || { + echo "device $device is not readable/writable by $(id -un)" >&2 + exit 1 + } +done + +# Short by necessity, not by accident: conmon's attach socket lives under +# this directory and must fit in a 108-byte sun_path. See nested_containers.py. +# Must stay in step with AGENT_CA_BUNDLE in bot_bottle/backend/util.py; a unit +# test pins the two together. +CA_BUNDLE="/etc/ssl/certs/ca-certificates.crt" +[ -r "$CA_BUNDLE" ] || { + echo "gateway CA bundle $CA_BUNDLE is missing or unreadable" >&2 + exit 1 +} + +# The proxy URL the agent inherits names `bot-bottle-gateway`, which resolves +# only through this bottle's /etc/hosts. A nested container gets its own hosts +# file, so it cannot resolve the name and dies at "Could not resolve proxy". +# +# podman's containers.conf `hosts_file` would fix that, except the +# Docker-compatible API ignores it — it only takes effect for native +# `podman run`, and the agent types `docker`. So the name is resolved *here* +# and the address, not the name, goes into the proxy URL the nested container +# receives. Verified on macOS 26 / podman 5.4.2: with the address in place, +# https://quay.io returns 200 and a non-allowlisted host still gets 403, so +# the egress boundary applies inside nested containers too. +GATEWAY_NAME="bot-bottle-gateway" +gateway_ip="$( + awk -v name="$GATEWAY_NAME" '$2 == name { print $1; exit }' /etc/hosts +)" +[ -n "$gateway_ip" ] || { + echo "no /etc/hosts entry for $GATEWAY_NAME; the gateway address is" >&2 + echo "needed so nested containers can reach the egress proxy" >&2 + exit 1 +} + +export XDG_RUNTIME_DIR="${XDG_RUNTIME_DIR:-/tmp/bbp}" +config="$HOME/.config/containers" +mkdir -p "$XDG_RUNTIME_DIR" "$config" +chmod 700 "$XDG_RUNTIME_DIR" + +# ignore_chown_errors is required, not incidental: with a single-UID mapping +# there is no second UID for image layers to be chowned to, so layers that +# record other owners would otherwise fail to extract. +cat > "$config/storage.conf" <<'CONF' +[storage] +driver="overlay" +[storage.options.overlay] +mount_program="/usr/bin/fuse-overlayfs" +ignore_chown_errors="true" +CONF + +# No cgroup delegation reaches this guest, so asking podman to manage cgroups +# fails; events_logger=file avoids the journald socket that is equally absent. +# +# The rest of this config is what lets a nested container reach the network: +# +# hosts_file only takes effect for native `podman run` — the +# Docker-compatible API ignores it, and the agent types +# `docker`. Kept anyway because it costs nothing and makes +# podman-native use behave; the compat path is covered by the +# address-bearing proxy URL below. +# volumes/env the gateway TLS-intercepts, so a container that does not +# trust the bottle's CA bundle gets "unable to get local issuer +# certificate". Mounting the bundle read-only and pointing the +# usual env vars at it covers curl, wget, python, and node +# without distro-specific trust commands. +# +# The proxy URL carries the bottle's identity token. podman already forwards +# that same URL into every nested container from the agent's own environment, +# so writing it to a 0600 file inside this disposable VM hands it to nobody +# new. It is never echoed. +CA_BUNDLE="$CA_BUNDLE" GATEWAY_NAME="$GATEWAY_NAME" GATEWAY_IP="$gateway_ip" \ +CONTAINERS_CONF="$config/containers.conf" python3 - <<'PY' +import os +from pathlib import Path + +ca = os.environ["CA_BUNDLE"] +name = os.environ["GATEWAY_NAME"] +ip = os.environ["GATEWAY_IP"] + +entries = [ + f"SSL_CERT_FILE={ca}", + f"CURL_CA_BUNDLE={ca}", + f"REQUESTS_CA_BUNDLE={ca}", + f"NODE_EXTRA_CA_CERTS={ca}", +] +# The gateway name resolves only through the bottle's /etc/hosts, which a +# nested container does not inherit, so hand it the address instead. +for var in ("HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"): + value = os.environ.get(var) + if value: + entries.append(f"{var}={value.replace(name, ip)}") +# NO_PROXY keeps the name: it is matched against what a client asks for, and +# code inside a nested container still says "bot-bottle-gateway". +for var in ("NO_PROXY", "no_proxy"): + value = os.environ.get(var) + if value: + entries.append(f"{var}={value}") + +path = Path(os.environ["CONTAINERS_CONF"]) +path.write_text("\n".join([ + "[containers]", + 'cgroups="disabled"', + # podman copies the host's proxy vars into every container by default, + # and that copy *wins* over the env below — putting the unresolvable + # gateway name back. Turn it off so the address-bearing URLs stand. + "http_proxy=false", + 'hosts_file="/etc/hosts"', + f'volumes=["{ca}:{ca}:ro"]', + "env=[", + *[f' "{entry}",' for entry in entries], + "]", + "[engine]", + 'cgroup_manager="cgroupfs"', + 'events_logger="file"', + "", +]), encoding="utf-8") +path.chmod(0o600) +PY + +# Registry pulls egress through the bottle's proxy like everything else. The +# token-bearing proxy URL is already in the agent's environment; persisting it +# inside this disposable VM does not broaden its authority. +# +# This file is also what the Docker CLI copies into every container it starts, +# and being client-side it beats anything the podman service does — it is why +# containers.conf `env`, `http_proxy=false`, and the service's own environment +# all failed to change what a nested container saw. The address goes in here +# for the same reason it goes everywhere else: `bot-bottle-gateway` resolves +# in the bottle, never inside a nested container. +GATEWAY_NAME="$GATEWAY_NAME" GATEWAY_IP="$gateway_ip" python3 - <<'PY' +import json +import os +from pathlib import Path + +name = os.environ["GATEWAY_NAME"] +ip = os.environ["GATEWAY_IP"] + +proxy = os.environ.get("HTTPS_PROXY") or os.environ.get("https_proxy", "") +# NO_PROXY keeps the name: it is matched against what a client asks for, and +# code inside a nested container still says "bot-bottle-gateway". +no_proxy = os.environ.get("NO_PROXY") or os.environ.get("no_proxy", "") +config = {"proxies": {"default": { + "httpProxy": proxy.replace(name, ip), + "httpsProxy": proxy.replace(name, ip), + "noProxy": no_proxy, +}}} +path = Path.home() / ".docker" / "config.json" +path.parent.mkdir(parents=True, exist_ok=True) +path.write_text(json.dumps(config), encoding="utf-8") +path.chmod(0o600) +PY + +if docker info >/dev/null 2>&1; then + exit 0 +fi + +# Belt to the ~/.docker/config.json braces above, which is what actually +# decides this for `docker run`. The service environment is what podman falls +# back to for anything the CLI does not stamp — its own registry pulls, and +# containers created through the API by something other than the Docker CLI. +# Cheap, and it keeps the address consistent across both paths. +# +# Assigned via parameter expansion, never echoed: these carry the bottle's +# identity token. +for var in HTTP_PROXY HTTPS_PROXY http_proxy https_proxy; do + eval "value=\${$var:-}" + [ -n "$value" ] || continue + eval "export $var=\"\${value%%$GATEWAY_NAME*}$gateway_ip\${value#*$GATEWAY_NAME}\"" +done + +log=/tmp/bot-bottle-nested-containers.log +nohup podman system service --time=0 \ + "unix://$XDG_RUNTIME_DIR/podman.sock" \ + >"$log" 2>&1 /attach`, and a Unix socket +# path may not exceed 108 bytes (`sun_path`). The descriptive +# `/tmp/bot-bottle-podman-run` produced a 116-byte path — over the limit, so +# attach would have broken as soon as anything got far enough to attach. Do +# not lengthen this for readability; it buys 8 bytes of headroom. +_RUNTIME_DIR = "/tmp/bbp" +_SOCKET = f"{_RUNTIME_DIR}/podman.sock" +_LOG = "/tmp/bot-bottle-nested-containers.log" +IMAGE_SUFFIX = "-nested-containers" +READY_RETRIES = 30 + +# Apple Container creates both device nodes 0600 root:root, so the agent user +# cannot open them: /dev/fuse blocks the fuse-overlayfs storage driver and +# /dev/net/tun blocks slirp4netns, which rootless podman uses for the default +# bridge network that stock compose files expect. Relaxing the modes needs no +# capability the bottle does not already hold — unlike CAP_SYS_ADMIN, which is +# what killed the rootless-Docker approach. +_GUEST_DEVICES = ("/dev/fuse", "/dev/net/tun") + + +def build_image( + base_image: str, + build: Callable[..., None], +) -> str: + """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 + in every built-in agent image (issue #451); the storage/network helpers, + the Docker CLI, and the compose plugin are the ~100MB this flag buys. + """ + image = f"{base_image}{IMAGE_SUFFIX}" + init_script = Path(__file__).with_name("nested-containers-init.sh") + with tempfile.TemporaryDirectory(prefix="bot-bottle-nested-containers.") as tmp: + context = Path(tmp) + shutil.copy2(init_script, context / "nested-containers-init.sh") + (context / "Dockerfile").write_text( + "FROM docker:28-cli AS docker_cli\n" + f"FROM {base_image}\n" + "USER root\n" + "COPY --from=docker_cli /usr/local/bin/docker /usr/local/bin/docker\n" + "COPY --from=docker_cli /usr/local/libexec/docker/cli-plugins/" + "docker-compose /usr/local/libexec/docker/cli-plugins/docker-compose\n" + "RUN apt-get update \\\n" + # podman 5's networking stack, installed explicitly because the + # base image's `--no-install-recommends` podman does not pull it + # in and each piece fails at a different, misleading layer: + # passt -> `pasta`, the default rootless netns helper + # (podman 4 used slirp4netns); without it + # nothing starts: "could not find pasta" + # nftables -> `nft`, which netavark shells out to for the + # bridge network every compose file expects + # aardvark-dns -> name resolution *inside* nested containers; + # without it DNS fails while everything else + # looks healthy + # slirp4netns stays as the documented fallback for pasta. + " && apt-get install -y --no-install-recommends " + "aardvark-dns fuse-overlayfs netavark nftables passt " + "slirp4netns uidmap \\\n" + " && rm -rf /var/lib/apt/lists/* \\\n" + # Deliberate: an empty subordinate range keeps podman on the + # single-UID mapping that needs no CAP_SYS_ADMIN. Adding ranges + # here would reintroduce the newuidmap failure this design exists + # to route around. + " && sed -i '/^node:/d' /etc/subuid /etc/subgid\n" + "COPY nested-containers-init.sh " + "/usr/local/libexec/bot-bottle/nested-containers-init\n" + "RUN chmod 0755 /usr/local/libexec/bot-bottle/nested-containers-init\n" + "USER node\n", + encoding="utf-8", + ) + build(image, str(context), dockerfile=str(context / "Dockerfile")) + return image + + +def guest_env(enabled: bool) -> dict[str, str]: + """Environment consumed by the Docker CLI inside an enabled bottle.""" + if not enabled: + return {} + return { + "DOCKER_HOST": f"unix://{_SOCKET}", + "XDG_RUNTIME_DIR": _RUNTIME_DIR, + } + + +def prepare_guest_devices(container_name: str, exec_as_root: Callable[..., None]) -> None: + """Make /dev/fuse and /dev/net/tun openable by the agent user. + + Runs as root inside the bottle because the agent must not be able to + re-mode device nodes itself. No outer capability is involved. + """ + exec_as_root( + container_name, + ["sh", "-c", f"chmod 0666 {' '.join(_GUEST_DEVICES)}"], + ) + + +def start(bottle: object) -> None: + """Start and verify the unprivileged service through the bottle exec API.""" + info("starting guest-local container engine") + result = bottle.exec(shlex.quote(_INIT)) # type: ignore[attr-defined] + if result.returncode != 0: + detail = (result.stderr or result.stdout or "").strip() + die(f"nested-container bootstrap failed: {detail or ''}") + + for _ in range(READY_RETRIES): + result = bottle.exec("docker info >/dev/null 2>&1") # type: ignore[attr-defined] + if result.returncode == 0: + info("guest-local container engine is ready") + return + time.sleep(0.2) + + logs = bottle.exec( # type: ignore[attr-defined] + f"tail -n 80 {_LOG} 2>/dev/null || true" + ) + die( + "guest-local container engine did not become ready without additional " + f"outer privileges:\n{(logs.stdout or logs.stderr or '').strip()}" + ) + + +__all__ = ["build_image", "guest_env", "prepare_guest_devices", "start"] diff --git a/bot_bottle/backend/macos_container/resolve_plan.py b/bot_bottle/backend/macos_container/resolve_plan.py index 9a9eb28..4c9883e 100644 --- a/bot_bottle/backend/macos_container/resolve_plan.py +++ b/bot_bottle/backend/macos_container/resolve_plan.py @@ -44,4 +44,5 @@ def resolve_plan( egress_plan=egress_plan, supervise_plan=supervise_plan, agent_provision=agent_provision_plan, + nested_containers=manifest.bottle.nested_containers, ) diff --git a/bot_bottle/backend/resolve_common.py b/bot_bottle/backend/resolve_common.py index 9f55a61..88f94ff 100644 --- a/bot_bottle/backend/resolve_common.py +++ b/bot_bottle/backend/resolve_common.py @@ -26,6 +26,7 @@ from ..bottle_state import ( ) from ..egress import Egress, EgressPlan from ..git_gate import GitGate, GitGatePlan +from ..log import die from ..manifest import Manifest, ManifestBottle from ..supervise import Supervise, SupervisePlan from . import BottleSpec @@ -112,6 +113,22 @@ def merge_provision_env_vars(provision: AgentProvisionPlan) -> AgentProvisionPla return replace(provision, guest_env=merged) +def reject_nested_containers(backend: str, manifest: Manifest) -> None: + """Fail loudly when a backend cannot honor `nested_containers: true`. + + Silently ignoring it would hand the agent a bottle where `docker` is not + there — and the only sound alternatives on these backends (a host daemon + socket, a privileged container) are exactly what issue #392 rules out. + """ + if not manifest.bottle.nested_containers: + return + die( + f"nested_containers is not supported on the {backend} backend. " + "Only macos-container runs a guest-local container engine today; " + "mounting the host Docker socket is not an option bot-bottle offers." + ) + + def resolve_manifest_dockerfile(path_value: str, spec: BottleSpec) -> str: """Resolve a manifest-supplied dockerfile path relative to user_cwd.""" path = Path(os.path.expanduser(path_value)) @@ -122,6 +139,7 @@ def resolve_manifest_dockerfile(path_value: str, spec: BottleSpec) -> str: __all__ = [ "merge_provision_env_vars", + "reject_nested_containers", "mint_slug", "prepare_agent_state_dir", "prepare_egress", diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index b52e244..a584a20 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -524,6 +524,8 @@ def _manifest_to_yaml(manifest: Manifest) -> str: lines.append(f" scheme: {r.AuthScheme}") lines.append(f" supervise: {'true' if bottle.supervise else 'false'}") + if bottle.nested_containers: + lines.append(" nested_containers: true") return "\n".join(lines) diff --git a/bot_bottle/egress.py b/bot_bottle/egress.py index 663996e..22b3dfc 100644 --- a/bot_bottle/egress.py +++ b/bot_bottle/egress.py @@ -255,6 +255,8 @@ def _route_to_yaml_fields(r: Route) -> dict[str, object]: fields["matches"] = matches_data if r.git_fetch: fields["git"] = {"fetch": True} + if r.preserve_auth: + fields["preserve_auth"] = True if ( r.outbound_detectors is not None or r.inbound_detectors is not None @@ -335,6 +337,8 @@ def egress_render_routes( lines.append(" git:") if git_dict.get("fetch") is True: lines.append(" fetch: true") + if f.get("preserve_auth") is True: + lines.append(" preserve_auth: true") if "dlp" in f: dlp_dict: dict[str, object] = f["dlp"] # type: ignore lines.append(" dlp:") diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 68c362d..f8b6307 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -20,6 +20,7 @@ Bottle schema (frontmatter): egress: { routes: [ , ... ] } # route keys: host, matches, auth, role, dlp supervise: # optional (default true) + nested_containers: # optional (default false) Agent schema (frontmatter): bottle: # required diff --git a/bot_bottle/manifest_bottle.py b/bot_bottle/manifest_bottle.py index 8d47c6b..455a926 100644 --- a/bot_bottle/manifest_bottle.py +++ b/bot_bottle/manifest_bottle.py @@ -44,6 +44,11 @@ class ManifestBottle: # daemon that exposes egress MCP tools to the agent. Set # `supervise: false` to skip the gateway. supervise: bool = True + # Guest-local container engine (issue #392). Not a host-daemon grant: + # backends implement it inside the bottle or reject it. Gated because it + # costs image weight, a resident service, and relaxed guest device modes + # that the majority of bottles never need. + nested_containers: bool = False @classmethod def from_dict(cls, name: str, raw: object) -> "ManifestBottle": @@ -123,7 +128,15 @@ class ManifestBottle: f"(was {type(supervise_raw).__name__})" ) + nested_raw = d.get("nested_containers", False) + if not isinstance(nested_raw, bool): + raise ManifestError( + f"bottle '{name}' nested_containers must be a boolean " + f"(was {type(nested_raw).__name__})" + ) + return cls( env=env, agent_provider=agent_provider, git=git, git_user=git_user, egress=egress, supervise=supervise_raw, + nested_containers=nested_raw, ) diff --git a/bot_bottle/manifest_extends.py b/bot_bottle/manifest_extends.py index 28911e0..36fdd40 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -15,7 +15,16 @@ def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle": the same field-merge rules as the file-based extends machinery: env: dict merge, later wins; git_user: per-field overlay, later wins on non-empty; git (repos): union by name, later wins; egress - routes: concatenate; agent_provider, supervise: later replaces. + routes: concatenate; agent_provider, supervise: later + replaces; nested_containers: OR (see below). + + nested_containers is OR'd rather than replaced because these objects + are already resolved: a bottle that never mentions the key is + indistinguishable from one that sets it false, so "later replaces" + would let any bottle composed after a container-enabled one silently + 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: raise ValueError("merge_bottles_runtime requires at least one bottle") @@ -54,6 +63,7 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle git_user=merged_git_user, egress=merged_egress, supervise=override.supervise, + nested_containers=base.nested_containers or override.nested_containers, ) @@ -206,6 +216,7 @@ def _fold_two_bottles( git_user=merged_git_user, egress=merged_egress, supervise=later.supervise, + nested_containers=earlier.nested_containers or later.nested_containers, ), merged_repos_raw @@ -266,6 +277,11 @@ def _merge_bottles( merged_supervise = ( child.supervise if "supervise" in child_raw else parent.supervise ) + merged_nested_containers = ( + child.nested_containers + if "nested_containers" in child_raw + else parent.nested_containers + ) validate_egress_routes(name, merged_egress.routes) return ManifestBottle( @@ -275,6 +291,7 @@ def _merge_bottles( git_user=merged_git_user, egress=merged_egress, supervise=merged_supervise, + nested_containers=merged_nested_containers, ) diff --git a/bot_bottle/manifest_schema.py b/bot_bottle/manifest_schema.py index 3e292b2..165718a 100644 --- a/bot_bottle/manifest_schema.py +++ b/bot_bottle/manifest_schema.py @@ -16,7 +16,10 @@ _FILENAME_RX = re.compile(r"^[a-z][a-z0-9-]*$") # sets dies with a "did you mean" pointer: typos should not silently # ghost into an empty config. BOTTLE_KEYS = frozenset( - {"env", "extends", "agent_provider", "git-gate", "egress", "supervise"} + { + "env", "extends", "agent_provider", "git-gate", "egress", + "supervise", "nested_containers", + } ) AGENT_KEYS_REQUIRED: frozenset[str] = frozenset() AGENT_KEYS_OPTIONAL = frozenset({"bottle", "skills", "git-gate"}) diff --git a/docs/prds/prd-new-nested-containers.md b/docs/prds/prd-new-nested-containers.md new file mode 100644 index 0000000..a91b658 --- /dev/null +++ b/docs/prds/prd-new-nested-containers.md @@ -0,0 +1,153 @@ +# PRD prd-new: Containers inside a bottle + +- **Status:** Draft +- **Author:** Claude +- **Created:** 2026-07-21 +- **Issue:** #392 + +## Summary + +Let an agent run `docker` and `docker compose` *inside* its bottle by starting +a guest-local rootless podman service that exposes a Docker-compatible API +socket. Gated per bottle by `nested_containers: true`. No host daemon socket is +mounted and no capability is added to the guest, on any backend. + +## Problem + +Agent tasks routinely involve containers: `docker compose up` to verify a +scaffolded stack, a throwaway database for a test run, building an image to +check a Dockerfile works. Bottles have no way to do any of that, so those tasks +either fail or get done outside the bottle — which defeats the point of the +bottle. + +## Goals / success criteria + +- A bottle with `nested_containers: true` on the `macos-container` backend can + run `docker compose up -d --wait` against a stock compose file, pulling from + a routed registry and serving from the workspace. +- The agent's habits do not change: `docker` and `docker compose`, not + `podman`. +- No host Docker socket is mounted, on any backend. +- No capability is added to the Apple Container guest. +- Backends without a guest-local engine reject the flag with a clear error + rather than ignoring it. +- Bottles that do not set the flag carry none of its cost. + +## Non-goals + +- Nested containers as an isolation layer. The single-UID mapping means `root` + inside a nested container is the agent user outside it. This is for build and + test workloads; the bottle stays the security boundary. +- The `docker` and `firecracker` backends. Both reject the flag for now. +- Reaching registries that are not routed through the bottle's egress proxy. + +## Design + +### Why podman, not rootless Docker + +Apple Container's capability bounding set omits `CAP_SYS_ADMIN`, which the +kernel requires in order to write a multi-range `uid_map` via `newuidmap`. +Rootless Docker has no path that avoids that write, so it cannot run in a +bottle without granting `CAP_SYS_ADMIN` — which is close to root in practical +terms and gives up most of what the bottle is for. + +Podman does have a path: with **no** subordinate UID range configured it falls +back to a single-UID self-mapping, which an unprivileged process may write +itself. The image build therefore *removes* the agent user's `/etc/subuid` and +`/etc/subgid` entries rather than adding them — their presence is exactly what +would send podman down the `newuidmap` path. Full negative result in +[`docs/research/rootless-docker-in-apple-container-spike.md`](../research/rootless-docker-in-apple-container-spike.md). + +### The flag, and its name + +The flag is kept because it gates real costs, not because podman needs a +privilege grant: ~100MB of derived image, a resident service per bottle, and +relaxed modes on `/dev/fuse` and `/dev/net/tun` that the majority of bottles +should not get. + +It is named `nested_containers`, not `docker_access`. It implies no Docker and +grants access to nothing on the host — naming it after Docker access would +describe the one thing the design refuses to do. + +### Shape + +- `bot_bottle/backend/macos_container/nested_containers.py` — derived-image + build, guest env, device preparation, service start/readiness. +- `bot_bottle/backend/macos_container/nested-containers-init.sh` — the + unprivileged bootstrap that runs inside the bottle. Fails closed on every + prerequisite it needs (podman, the storage/network helpers, writable device + nodes, an *empty* subordinate range) rather than degrading. +- The derived image `…-nested-containers` layers the Docker CLI, the compose + plugin, and podman 5's networking stack — `passt` (pasta), `netavark`, + `nftables`, `aardvark-dns` — plus `fuse-overlayfs`, `slirp4netns`, and + `uidmap` onto the agent image. Podman itself already ships in every built-in + image (#451), but with `--no-install-recommends`, so none of the networking + pieces arrive with it. Each absence fails at a different and misleading + layer: no pasta and nothing starts; no `nft` and containers are created but + never start; no aardvark-dns and DNS inside a container fails while + everything else looks healthy. Image pulls keep working throughout, which is + what makes these read as compat-API bugs. +- `BottleBackend.supports_nested_containers` — false by default, so a backend + that cannot honor the flag fails in the shared `prepare` template. + +Guest configuration the single-UID mapping forces: +`ignore_chown_errors` (no second UID for layers to be chowned to), +`cgroups="disabled"` and `cgroup_manager="cgroupfs"` (no cgroup delegation +reaches the guest), `events_logger="file"` (no journald socket). + +### Registry reach + +Image pulls egress through the bottle's proxy like everything else, so each +registry needs a route — **and so does the CDN it redirects blobs to**, which +is a separate host: `production.cloudfront.docker.com` for Docker Hub, +`pkg-containers.githubusercontent.com` for GHCR, `cdn0*.quay.io` for quay. +Without those the pull authenticates, fetches the manifest, and then 403s +partway through. Docker Hub and GHCR need `preserve_auth: true` — their +token dance uses a client-fetched per-scope bearer token that the proxy would +otherwise strip. Layer pulls should also set `dlp.outbound_detectors: false` +and `dlp.inbound_detectors: false`: body scanning on a multi-hundred-MB layer +is what triggers #455, and a registry route's scanned bodies are compressed +layer blobs rather than anything a detector can read. + +### Reaching the network from a nested container + +Public DNS inside a nested container fails by design: everything egresses +through the proxy. What was actually broken was reaching the proxy at all, and +it took four attempts to fix because podman applies proxy settings at several +layers and the last writer wins: + +| Layer | Applies to | +|---|---| +| `~/.docker/config.json` proxies block | **every container the Docker CLI starts** — client-side, beats everything below | +| service environment | podman's own pulls, non-CLI API clients | +| `containers.conf` `env` | native `podman run` | +| `containers.conf` `hosts_file`, `http_proxy` | native `podman run` only — the compat API ignores both | + +The gateway is named `bot-bottle-gateway`, which resolves only through the +bottle's `/etc/hosts`; a nested container gets its own. Since no config key +reaches the compat path, the name is resolved in the bottle and the *address* +is substituted into the proxy URL at each layer above. `NO_PROXY` keeps the +name, which is matched against what a client asks for. + +The gateway TLS-intercepts, so the CA bundle is mounted read-only and +`SSL_CERT_FILE`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, and +`NODE_EXTRA_CA_CERTS` point at it — no distro-specific trust commands. + +## Verified on macOS 26 / Apple Container 1.0.0 / podman 5.4.2 + +With plain `docker`, no extra flags: image pulls from Docker Hub, GHCR, and +quay; `docker run` with correct exit-code propagation; `docker compose` up, +logs, down, and a published port curl'd from the bottle; `curl https://quay.io` +and `https://pypi.org/simple/` returning 200 from inside a nested container; +and `https://example.com` returning **403** — the egress allowlist applies to +nested containers, not just the bottle. + +Alpine's BusyBox `wget` drops the connection after TLS interception and +reports "error getting response". The proxy logs the decrypted request, and +`curl` on the same host succeeds, so this is a BusyBox client limitation +rather than anything in the egress path. + +## Open questions + +- Whether the `docker` and `firecracker` backends want an equivalent, or + whether guest-local containers stay macOS-only. diff --git a/docs/research/egress-proxy-oom-on-large-downloads.md b/docs/research/egress-proxy-oom-on-large-downloads.md new file mode 100644 index 0000000..1c0ae25 --- /dev/null +++ b/docs/research/egress-proxy-oom-on-large-downloads.md @@ -0,0 +1,86 @@ +# Egress proxy OOMs on large downloads + +Found on 2026-07-21 while running the rootless-podman spike +(`docs/research/rootless-docker-in-apple-container-spike.md`). Recorded +rather than fixed — the fix is a security-relevant decision, not a +mechanical patch. + +## Summary + +A single large HTTPS download through the gateway kills the egress +proxy. `mitmdump` buffers whole response bodies so the DLP detectors can +scan them, grows past the gateway container's memory limit, and is +OOM-killed by the cgroup. Nothing restarts it. + +Two properties make this worse than a failed download: + +- **The gateway is a per-host singleton.** Every bottle shares it, so + one bottle's download takes egress away from all of them. +- **There is no restart on death.** The gateway supervisor is + `while : ; do wait ; done`; a killed daemon stays dead until the infra + container is recreated. + +So ordinary agent activity — pulling a container image, downloading a +model or dataset, fetching a large tarball — is a denial of service +against every other bottle on the host. No malice required, though it is +trivially reachable on purpose. + +## Evidence + +Triggered by `docker compose up` pulling `quay.io/fedora/python-312` +(two layers, ~82MB and ~83MB) inside a bottle. The pull itself +succeeded; the *next* request failed: + +``` +initializing source docker://quay.io/fedora/python-312:latest: + pinging container registry quay.io: Get "https://quay.io/v2/": + proxyconnect tcp: dial tcp 192.168.128.39:9099: connect: connection refused +``` + +From the gateway's `dmesg`: + +``` +python3 invoked oom-killer: gfp_mask=0x100cca(GFP_HIGHUSER_MOVABLE), order=0 +oom-kill:constraint=CONSTRAINT_MEMCG, + oom_memcg=/container/bot-bottle-mac-infra, + task_memcg=/container/bot-bottle-mac-infra,task=mitmdump,pid=118 +Memory cgroup out of memory: Killed process 118 (mitmdump) + total-vm:1391936kB, anon-rss:997768kB +``` + +~1GB RSS against a 1024MB container. Note the amplification: ~165MB of +layers produced ~1GB of resident memory, so the buffering is several +copies deep (encoded body, decoded body, and the text conversion the +regex detectors scan). + +Afterwards the gateway container was still running and healthy-looking — +orchestrator, supervise, and git-http all alive — with no `mitmdump` +process at all, and it stayed that way until the container was +recreated. A liveness check on the container would not have caught this. + +## Reproduction + +1. Launch any bottle with an egress route to a host serving a large file. +2. Download >~150MB over HTTPS through the proxy. +3. `dmesg | grep -i oom` inside `bot-bottle-mac-infra`, and note that no + `mitmdump` process remains. + +Beware a false negative when checking: truncating the process listing +(`cut -c1-45`) cuts before the binary name, because `mitmdump` runs as +`/usr/local/bin/python3.12 /usr/local/bin/mitmdump …`. + +## Fix options, not yet chosen + +1. **Restart dead daemons.** Smallest change and strictly an + improvement: an OOM then degrades one download instead of removing + egress for every bottle. Does not stop the OOM. +2. **Cap the scanned body size.** Above a threshold, stop buffering — + either skip the scan or stream it. This is the root-cause fix and a + security decision: a size threshold is exactly the hole an exfiltrator + would aim for, so "skip above N" trades a DoS for a covert channel. + Streaming with a bounded window keeps coverage, at more complexity. +3. **Raise the gateway's memory limit.** Moves the threshold; does not + remove it. + +Worth noting that (1) and (2) are complementary — the restart gap is +worth closing regardless of how the memory behaviour is resolved. diff --git a/docs/research/rootless-docker-in-apple-container-spike.md b/docs/research/rootless-docker-in-apple-container-spike.md new file mode 100644 index 0000000..5a18b5a --- /dev/null +++ b/docs/research/rootless-docker-in-apple-container-spike.md @@ -0,0 +1,359 @@ +# Rootless Docker inside Apple Container bottles + +Spike branch: `spike/rootless-docker-macos` (`a4d8461`) + +**Outcome:** the podman recommendation below shipped as the +`nested_containers` bottle flag — see +[`docs/prds/prd-new-nested-containers.md`](../prds/prd-new-nested-containers.md). +The `docker_access` name used throughout the spike text was renamed on the +way in; it granted no access to anything on the host. + +## Summary + +**Negative result.** Rootless Docker cannot run inside an Apple +Container bottle without granting the bottle `CAP_SYS_ADMIN`. This is a +kernel constraint on writing multi-range `uid_map`, not a packaging gap +we can close with a better init script, a different base image, or more +careful `/etc/subuid` handling. + +The spike was built on the premise — stated in +`bot_bottle/backend/macos_container/rootless_docker.py` — that it would +*"deliberately refuse to compensate for missing prerequisites with outer +capabilities, a privileged container, or a host Docker socket."* That +premise is exactly what the experiment falsified. The two ways forward +are to abandon the premise (add `CAP_SYS_ADMIN` to the bottle, and with +it most of the isolation the bottle exists to provide) or to abandon +rootless Docker. + +Recommendation: abandon rootless Docker. Podman does not have this +problem — see [Podman is not blocked by +this](#podman-is-not-blocked-by-this) below. + +## Local environment + +Tested on 2026-07-21: + +```console +$ sw_vers +ProductName: macOS +ProductVersion: 26.5.1 +BuildVersion: 25F80 + +$ container --version +container CLI version 1.0.0 (build: release, commit: ee848e3) + +$ uname -a # inside the bottle +Linux ... 6.18.15 #1 SMP Tue Mar 17 01:36:53 UTC 2026 aarch64 GNU/Linux +``` + +## The failure + +`tests/integration/test_macos_rootless_docker_spike.py` builds the +image, launches the bottle, and dies in `rootless_docker.start`: + +``` ++ exec rootlesskit --net=slirp4netns --mtu=65520 ... dockerd-rootless.sh +[rootlesskit:parent] error: failed to setup UID/GID map: + newuidmap 1100 [0 1000 1 1 100000 65536] failed: + newuidmap: write to uid_map failed: Operation not permitted +``` + +## Why it fails + +Every prerequisite you would normally suspect is present and correct in +the guest: + +| Check | Result | +| --- | --- | +| `/usr/bin/newuidmap` | `-rwsr-xr-x root root` — setuid bit intact, survived the OCI export | +| `/` mount options | `rw,relatime` — **not** `nosuid` | +| `NoNewPrivs` | `0` | +| `Seccomp` | `0`, no filters | +| `/etc/subuid`, `/etc/subgid` | `node:100000:65536` in both | +| user namespace | `user:[4026531837]`, identical to pid 1 — the *initial* userns | +| `unshare -U -r true` | succeeds | +| `/proc/sys/user/max_user_namespaces` | `4505` | + +The one thing that is missing is in the capability bounding set that +Apple Container gives the container: + +``` +CapBnd: 00000000a80425fb += chown, dac_override, fowner, fsetid, kill, setgid, setuid, setpcap, + net_bind_service, net_raw, sys_chroot, mknod, audit_write, setfcap +``` + +No `CAP_SYS_ADMIN`. That is the whole story, and the chain is: + +1. The kernel's `map_write()` gates writing a `uid_map` on + `file_ns_capable(file, ns, CAP_SYS_ADMIN)` — capability over the + **new** user namespace, evaluated against the credentials that opened + `/proc//uid_map`. +2. `newuidmap` is setuid-root, so it runs with euid 0 — but its + capability sets are clamped by the bounding set, which has no + `CAP_SYS_ADMIN`. +3. `cap_capable()` has a shortcut that grants *all* capabilities when + the caller's userns is the new namespace's parent **and** + `ns->owner == cred->euid`. It does not apply: the namespace was + created by `node` (uid 1000) while `newuidmap` runs as euid 0. +4. So the check falls through to the effective-set test in the initial + userns, which fails. `EPERM`. + +Note that the single-line unprivileged path (`unshare -U -r`) works +precisely because it does not go through `newuidmap` and does not need +`CAP_SYS_ADMIN`. Only the multi-range subuid mapping that rootless +Docker requires does. + +This is the same constraint that makes upstream's `dind-rootless` image +require `--privileged`. It is not specific to Apple Container, except +that Apple Container gives us no bounding set that includes +`CAP_SYS_ADMIN` by default. + +## It does work with the capability — which is the point + +Adding the capability clears the failure immediately, and exposes one +further, much smaller blocker: `/dev/net/tun` exists (the kernel has +tun; `/proc/misc` lists `200 tun`) but Apple Container creates it +`crw------- root root`, so uid 1000 cannot open it and `slirp4netns` +fails with `open: Permission denied`. A `chmod 0666 /dev/net/tun` as +root inside the bottle fixes that, and needs no capability beyond what +the bottle already has. + +With both applied by hand, the daemon comes up completely: + +```console +$ container run --rm -u root --cap-add CAP_SYS_ADMIN \ + bot-bottle-claude:latest-rootless-docker sh -c '...' +Server Version: 20.10.24+dfsg1 +Storage Driver: fuse-overlayfs +Cgroup Driver: none +Cgroup Version: 2 +API listen on /tmp/rt/docker.sock +``` + +So `rootless-docker-init.sh` and `rootless_docker.py` are *correct*. +The spike did not fail on a bug. It failed on its own premise. + +Two secondary findings from that run, relevant if anyone revisits this: + +- Debian's `docker.io` package pins Docker **20.10** (EOL), not the 28.x + implied by the `docker:28-cli` compose plugin the image copies in. +- `Cgroup Driver: none` — no resource limits on nested containers. + +## Why we should not just add the capability + +`CAP_SYS_ADMIN` is close to a superset of "root" in practical terms — +mount, `pivot_root`, namespace manipulation, and a long tail of +subsystem-specific powers. Granting it to the agent bottle would +undercut the containment argument the rest of the backend is built +around, including the deliberately narrow choices immediately adjacent +to it in `launch.py` (`--cap-drop CAP_NET_RAW`, no `NET_ADMIN`, a +host-only agent network). Trading all of that for nested `docker +compose` is a bad exchange. + +## Podman is not blocked by this + +Sanity-checked on the same host, same kernel, same runtime, so the +comparison is apples to apples: + +| Scenario | Result | +| --- | --- | +| Podman rootless, `/etc/subuid` populated | **Fails identically** — `newuidmap: write to uid_map failed: Operation not permitted` | +| Podman rootless, no subuid ranges, `--network=host` | **Works**, no added capabilities | +| Podman rootless, no subuid ranges, default netns, `/dev/net/tun` at `0600` | Fails — `slirp4netns: open("/dev/net/tun"): Permission denied` | +| Podman rootless, no subuid ranges, default netns, `/dev/net/tun` at `0666` | **Works**, no added capabilities | + +The difference is that podman degrades gracefully when no subuid range +is available: it falls back to a single-UID self-mapping, which an +unprivileged process may write itself, so `newuidmap` is never invoked +and `CAP_SYS_ADMIN` is never needed. Docker's rootless mode has no +equivalent fallback. + +The cost of that fallback is real and should be weighed before building +on it: with a single-UID mapping, every UID inside a nested container +collapses onto the bottle's own uid 1000. There is no UID separation +between the agent and anything it runs — `root` in a nested container is +the agent user outside it. It also requires `ignore_chown_errors` on the +storage driver. Whether that is acceptable depends on whether the bottle +boundary (which is unchanged) or the nested-container boundary (which is +effectively nil) is the one we are relying on. + +## What the podman spike then needed + +The podman implementation that replaced the Docker one on this branch +turned up two more device-node blockers of the same shape as +`/dev/net/tun` — Apple Container creates the node, but 0600 root:root: + +- **`/dev/fuse`** — blocks the `fuse-overlayfs` storage driver + (`fuse: failed to open /dev/fuse: Permission denied`). Without it the + only working driver is `vfs`, which copies whole layers per container. +- **`/dev/net/tun`** — blocks `slirp4netns`, which rootless podman uses + for the default bridge network. + +Both are fixed by `chmod 0666` as root inside the bottle, which needs no +capability the bottle does not already hold. This is categorically +different from the `CAP_SYS_ADMIN` requirement: it is a permission on a +node that already exists, not an outer privilege grant. + +One design note worth recording: the agent-facing surface stays `docker` +and `docker compose`, pointed at podman's Docker-compatible API socket +via `DOCKER_HOST`. Setting `netns="host"` in `containers.conf` does *not* +propagate through that compat API — stock `docker run` and compose files +request bridge networking explicitly — so slirp4netns (and therefore the +`/dev/net/tun` chmod) is required for ordinary compose files to work at +all. Host networking remains available per-workload via +`--network=host`. + +Verified working in a bottle with zero added capabilities: fuse-overlayfs +storage, the compat API socket, `docker run` on both bridge and host +networking, and published ports. + +### Nested pulls collide with our own egress DLP + +The first live run got podman up and `docker compose` running, then +failed on the image pull: + +``` +web Pulling +initializing source docker://python:3.12-alpine: reading manifest ... +StatusCode: 403, egress DLP: Generic Bearer JWT found in body +``` + +This is bot-bottle's own egress scanner, not a podman problem. The +Docker registry auth flow carries a bearer JWT *by protocol*, and the +`token_patterns` detector's `Generic Bearer JWT` rule +(`Bearer\s+[A-Za-z0-9._\-]{50,}`) matches it on every pull. Any bottle +that pulls images will hit this. + +The fix is per-route detector scoping, which the egress config already +supports — drop `token_patterns` on the registry hosts and keep +`known_secrets`: + +```json +{"host": "registry-1.docker.io", + "dlp": {"outbound_detectors": ["known_secrets"]}} +``` + +That is the right trade rather than a grudging one: `known_secrets` +matches the bottle's *actual* credential values, so real exfil through a +registry host is still caught. `token_patterns` on a registry route only +ever produces protocol noise. + +Worth generalising later: any manifest enabling `nested_containers` needs +this on its registry routes, so it probably belongs in a shared +registry-route snippet rather than being copy-pasted per bottle. + +### And then registry auth collides with the Authorization strip + +With DLP scoped, the pull failed differently: `unauthorized: +authentication required`. This one is architectural. + +`egress_addon.py` strips agent-set `Authorization` unconditionally +before forwarding — deliberately, so an agent cannot smuggle a +credential out in a header the DLP detectors don't recognise. A route +may carry gateway-injected auth instead, but only from a *static* token +in an env var (`auth_scheme` + `token_env`). + +Docker registry auth doesn't fit that shape. The client fetches a +short-lived, per-repository-scope bearer token from `auth.docker.io` and +presents it to `registry-1.docker.io`. There is no static token to +inject, and the token the client legitimately obtained is stripped. + +Measured inside a bottle, by hand: + +| Step | Result | +| --- | --- | +| Fetch token from `auth.docker.io` | 200, 5409-byte token body | +| Manifest request **with** that valid token | 401 | +| Manifest request with **no** Authorization | 401 — identical | + +A valid token behaves exactly like sending none, which is direct +evidence the header never arrives. Any nested-container workflow that +pulls from a registry is blocked on this, so it is not a detail that can +be deferred: pulling base images is most of what nested containers are +for. + +### Registries that skip the token dance work today + +Not every registry needs the stripped header. Measured directly: + +| Registry | Manifest request with no `Authorization` | +| --- | --- | +| `quay.io` | 200 | +| `mcr.microsoft.com` | 200 | +| `registry.k8s.io` | 307 (redirect, no auth) | +| `ghcr.io` | 401 | +| `registry-1.docker.io` | 401 | + +So "just add the registry to the bottle config" genuinely works — for +quay, MCR, registry.k8s.io, or any unauthenticated internal registry. +Docker Hub and GHCR are the ones that need the strip resolved. The +acceptance test uses quay for exactly this reason. + +Resolving it for Docker Hub means picking one of: + +1. **Per-route opt-in to preserve client Authorization.** Smallest + change. Note the compounding effect on exactly these routes: the DLP + scoping above already removed `token_patterns` there, so a + preserved-auth registry route is one where the agent may send bearer + tokens that neither the strip nor the pattern detector inspects. + `known_secrets` still applies, so the bottle's real credentials are + still caught. +2. **A registry-aware gateway** that performs the token dance itself and + injects the result. Preserves the invariant fully; materially more + work, and it makes the gateway speak a specific registry protocol. +3. **Pre-seed images at provision time** (host-side `container image + save` into podman storage), so bottles never pull at runtime. + Preserves the invariant, and limits nested containers to + pre-approved images — which fits the custody positioning, at the cost + of no ad-hoc `docker pull`. +4. **Stop.** Nested containers are not supported on this backend. + +### Podman 4.3.1 silently swallows container exit codes + +Debian bookworm — which the current agent base image is built on — +ships podman 4.3.1. Through its Docker-compatible API, `docker run` +returns 0 no matter what the container did: + +| Command | podman 4.3.1 | podman 5.4.2 | +| --- | --- | --- | +| `docker run … sh -c 'exit 7'` (compat API) | **0** | 7 | +| `docker run … sh -c 'exit 0'` (compat API) | 0 | 0 | +| `podman run … sh -c 'exit 7'` (native) | 7 | 7 | + +This is worse than a broken feature: every failing command an agent runs +via `docker run` reports success. A test suite, a build step, or a CI +script inside a bottle would pass while failing. It also silently +defeated the acceptance test's egress-containment assertion, which is +why that assertion now checks an in-band marker rather than an exit +code. + +Podman 5.4.2 (Debian trixie) fixes it, but needs two packages that +bookworm's podman does not: `passt` (podman 5's default network tool) +and `nftables` (netavark shells out to `nft`; without it every run fails +with `unable to upgrade to tcp, received 500`). With both installed, +exit codes propagate correctly and the compat API behaves. + +The open question this leaves is where podman 5 comes from, since the +agent base is bookworm-based: + +1. **Move the agent images to Debian trixie.** Trixie is current stable. + Correct, and the blast radius is every bottle, not just this feature. +2. **Drop the compat socket and use podman natively** (`podman-docker` + provides a `docker` shim; compose comes from `podman-compose`). + Native podman propagates exit codes correctly even on 4.3.1. Contained + to this feature, at the cost of `docker compose` becoming + `docker-compose`/`podman-compose`. +3. **Ship bookworm's podman 4.3.1 with the compat socket** — not viable. + Silent false success is a correctness bug agents cannot see. + +## Recommendation + +1. Do not revive rootless Docker on this backend. This document is the + record of why. +2. Nested containers, if wanted, come from podman under the + single-mapping constraint — with the explicit understanding that the + nested-container boundary carries no security weight. `root` in a + nested container is the agent user outside it. +3. Nested containers are therefore a build/test convenience. The bottle + remains the security boundary, exactly as it was. diff --git a/tests/unit/test_backend_prepare.py b/tests/unit/test_backend_prepare.py index 6013229..921caaf 100644 --- a/tests/unit/test_backend_prepare.py +++ b/tests/unit/test_backend_prepare.py @@ -21,7 +21,7 @@ from bot_bottle.backend.firecracker import FirecrackerBottleBackend from bot_bottle.manifest import ManifestIndex -def _manifest() -> ManifestIndex: +def _manifest(*, nested_containers: bool = False) -> ManifestIndex: return ManifestIndex.from_json_obj({ "bottles": { "dev": { @@ -29,6 +29,7 @@ def _manifest() -> ManifestIndex: "LITERAL_ENV": "literal-value", "FORWARDED_ENV": "${HOST_SECRET_ENV}", }, + "nested_containers": nested_containers, }, }, "agents": { @@ -41,9 +42,11 @@ def _manifest() -> ManifestIndex: }) -def _spec(tmp: Path, *, identity: str) -> BottleSpec: +def _spec( + tmp: Path, *, identity: str, nested_containers: bool = False, +) -> BottleSpec: return BottleSpec( - manifest=_manifest(), + manifest=_manifest(nested_containers=nested_containers), agent_name="demo", copy_cwd=False, user_cwd=str(tmp), @@ -113,6 +116,42 @@ class TestFirecrackerPrepare(_FakeStateMixin, unittest.TestCase): self.assertEqual({"FORWARDED_ENV": "secret-value"}, plan.forwarded_env) +class TestNestedContainersRejection(_FakeStateMixin, unittest.TestCase): + """A backend with no guest-local engine must refuse the flag outright. + + Ignoring it would leave the agent without `docker`, and the only ways to + fake it here (a host daemon socket, a privileged container) are what + issue #392 rules out. + """ + + def test_docker_backend_refuses_the_flag(self) -> None: + backend = DockerBottleBackend() + spec = _spec( + Path(self.tmp.name), identity="demo-docker", nested_containers=True, + ) + with ( + patch( + "bot_bottle.backend.resolve_common.die", side_effect=RuntimeError, + ) as die, + self.assertRaises(RuntimeError), + ): + backend.prepare(spec, Path(self.tmp.name) / "stage") + self.assertIn("nested_containers", die.call_args.args[0]) + self.assertIn("docker", die.call_args.args[0]) + + def test_firecracker_backend_refuses_the_flag(self) -> None: + backend = FirecrackerBottleBackend() + spec = _spec(Path(self.tmp.name), identity="demo-fc", nested_containers=True) + with ( + patch( + "bot_bottle.backend.resolve_common.die", side_effect=RuntimeError, + ) as die, + self.assertRaises(RuntimeError), + ): + backend.prepare(spec, Path(self.tmp.name) / "stage") + self.assertIn("firecracker", die.call_args.args[0]) + + class TestMintSlug(unittest.TestCase): def _spec(self, *, label: str = "", identity: str = "") -> BottleSpec: manifest = _manifest() diff --git a/tests/unit/test_egress.py b/tests/unit/test_egress.py index 2db81b7..43cb73d 100644 --- a/tests/unit/test_egress.py +++ b/tests/unit/test_egress.py @@ -379,6 +379,25 @@ class TestRenderRoutes(unittest.TestCase): addon_routes = load_config(rendered).routes self.assertTrue(addon_routes[0].git_fetch) + def test_preserve_auth_round_trips_to_the_addon(self): + """Regression: the manifest parsed preserve_auth and the addon honored + it, but the renderer in between dropped it — so the flag never reached + the proxy and registry pulls kept failing with "unauthorized" while + the config looked correct everywhere it was inspected.""" + from bot_bottle.egress_addon_core import load_config + b = _bottle([{"host": "registry-1.docker.io", "preserve_auth": True}]) + routes = egress_routes_for_bottle(b) + rendered = egress_render_routes(routes) + self.assertIn("preserve_auth: true", rendered) + self.assertTrue(load_config(rendered).routes[0].preserve_auth) + + def test_preserve_auth_omitted_when_unset(self): + b = _bottle([{"host": "x.example"}]) + rendered = egress_render_routes(egress_routes_for_bottle(b)) + self.assertNotIn("preserve_auth", rendered) + from bot_bottle.egress_addon_core import load_config + self.assertFalse(load_config(rendered).routes[0].preserve_auth) + def test_log_zero_omitted_from_render(self): b = _bottle([{"host": "x.example"}]) routes = egress_routes_for_bottle(b) diff --git a/tests/unit/test_macos_container_launch_wiring.py b/tests/unit/test_macos_container_launch_wiring.py index 94256a3..5432565 100644 --- a/tests/unit/test_macos_container_launch_wiring.py +++ b/tests/unit/test_macos_container_launch_wiring.py @@ -49,6 +49,7 @@ def _plan( agent_git_gate_url: str = "", agent_supervise_url: str = "", image_policy: str = "fresh", + nested_containers: bool = False, ) -> MacosContainerBottlePlan: routes_path = stage_dir / "routes.yaml" routes_path.write_text("routes: []\n", encoding="utf-8") @@ -67,6 +68,7 @@ def _plan( manifest=_MANIFEST, stage_dir=stage_dir, slug="dev-abc", + nested_containers=nested_containers, container_name="bot-bottle-dev-abc", image="bot-bottle-agent:latest", dockerfile_path="/repo/Dockerfile", diff --git a/tests/unit/test_macos_nested_containers.py b/tests/unit/test_macos_nested_containers.py new file mode 100644 index 0000000..c771500 --- /dev/null +++ b/tests/unit/test_macos_nested_containers.py @@ -0,0 +1,333 @@ +"""Unit coverage for the fail-closed guest-local container engine (#392).""" + +from __future__ import annotations + +import unittest +from dataclasses import dataclass +from pathlib import Path +from types import SimpleNamespace +from typing import cast +from unittest.mock import patch + +from bot_bottle.backend.macos_container import nested_containers +from bot_bottle.backend.macos_container import launch as launch_mod +from bot_bottle.backend.macos_container.bottle_plan import MacosContainerBottlePlan + + +class _Bottle: + def __init__(self, results: list[SimpleNamespace]) -> None: + self.results = results + self.commands: list[str] = [] + + def exec(self, command: str) -> SimpleNamespace: + self.commands.append(command) + return self.results.pop(0) + + +def _result(returncode: int, *, stdout: str = "", stderr: str = "") -> SimpleNamespace: + return SimpleNamespace(returncode=returncode, stdout=stdout, stderr=stderr) + + +@dataclass(frozen=True) +class _AgentProvision: + image: str + + +@dataclass(frozen=True) +class _Spec: + image_policy: str = "build" + + +@dataclass(frozen=True) +class _Plan: + slug: str + image: str + dockerfile_path: str + nested_containers: bool + agent_provision: _AgentProvision + spec: _Spec = _Spec() + + +def _base_image_only(ref: str) -> bool: + """Only the un-derived agent image is cached on the host.""" + return not ref.endswith(nested_containers.IMAGE_SUFFIX) + + +def _plan(**kwargs: object) -> MacosContainerBottlePlan: + return cast(MacosContainerBottlePlan, cast(object, _Plan(**kwargs))) # type: ignore[arg-type] + + +class TestNestedContainersStart(unittest.TestCase): + def test_bootstraps_then_waits_for_guest_local_service(self) -> None: + bottle = _Bottle([_result(0), _result(1), _result(0)]) + with patch.object(nested_containers.time, "sleep"): + nested_containers.start(bottle) + self.assertIn("nested-containers-init", bottle.commands[0]) + self.assertEqual(2, bottle.commands.count("docker info >/dev/null 2>&1")) + + def test_bootstrap_failure_is_fatal_without_privilege_fallback(self) -> None: + bottle = _Bottle([_result(1, stderr="slirp4netns missing")]) + with patch.object(nested_containers, "die", side_effect=RuntimeError) as die: + with self.assertRaises(RuntimeError): + nested_containers.start(bottle) + self.assertIn("slirp4netns missing", die.call_args.args[0]) + self.assertEqual(1, len(bottle.commands)) + + def test_timeout_reports_guest_log(self) -> None: + bottle = _Bottle( + [_result(0)] + + [_result(1) for _ in range(nested_containers.READY_RETRIES)] + + [_result(0, stdout="operation not permitted")] + ) + with patch.object(nested_containers.time, "sleep"), \ + patch.object(nested_containers, "die", side_effect=RuntimeError) as die: + with self.assertRaises(RuntimeError): + nested_containers.start(bottle) + self.assertIn("operation not permitted", die.call_args.args[0]) + + +class TestNestedContainersDevices(unittest.TestCase): + def test_relaxes_only_the_two_blocked_device_nodes_as_root(self) -> None: + calls: list[tuple[str, list[str]]] = [] + + def record(name: str, argv: list[str]) -> None: + calls.append((name, argv)) + + nested_containers.prepare_guest_devices("bottle-1", record) + self.assertEqual(1, len(calls)) + name, argv = calls[0] + self.assertEqual("bottle-1", name) + self.assertIn("chmod 0666 /dev/fuse /dev/net/tun", argv[-1]) + + +class TestNestedContainersImage(unittest.TestCase): + def test_layers_tooling_without_changing_base_image(self) -> None: + calls: list[tuple[str, str, str]] = [] + + def build(image: str, context: str, *, dockerfile: str) -> None: + calls.append((image, context, dockerfile)) + text = Path(dockerfile).read_text(encoding="utf-8") + self.assertIn("FROM agent:base", text) + self.assertIn("aardvark-dns fuse-overlayfs netavark nftables passt", text) + self.assertIn("USER node", text) + self.assertTrue((Path(context) / "nested-containers-init.sh").is_file()) + + image = nested_containers.build_image("agent:base", build) + self.assertEqual("agent:base-nested-containers", image) + self.assertEqual("agent:base-nested-containers", calls[0][0]) + + def test_installs_the_whole_podman_5_networking_stack(self) -> None: + """Each missing piece fails at a different, misleading layer: no pasta + and nothing starts; no nft and netavark cannot build the bridge that + compose expects; no aardvark-dns and DNS inside nested containers + fails while everything else looks healthy. Image pulls keep working + throughout, which is what made these read as compat-API bugs.""" + seen: list[str] = [] + + def build(_image: str, _context: str, *, dockerfile: str) -> None: + seen.append(Path(dockerfile).read_text(encoding="utf-8")) + + nested_containers.build_image("agent:base", build) + for package in ("passt", "nftables", "aardvark-dns"): + self.assertIn(package, seen[0]) + + def test_strips_subordinate_ranges_so_podman_avoids_newuidmap(self) -> None: + """The single-UID fallback is the entire reason podman works here. + + A subordinate range would send podman down the newuidmap path, which + cannot write a multi-range uid_map without CAP_SYS_ADMIN in an Apple + Container guest — the failure that killed the rootless-Docker spike. + """ + seen: list[str] = [] + + def build(_image: str, _context: str, *, dockerfile: str) -> None: + seen.append(Path(dockerfile).read_text(encoding="utf-8")) + + nested_containers.build_image("agent:base", build) + text = seen[0] + self.assertIn("sed -i '/^node:/d' /etc/subuid /etc/subgid", text) + self.assertNotIn("subuid", text.replace( + "sed -i '/^node:/d' /etc/subuid /etc/subgid", "", + )) + + +class TestInitScript(unittest.TestCase): + """The bootstrap runs inside the bottle, so its wiring is only checkable + here or on a live macOS host.""" + + def setUp(self) -> None: + self.script = ( + Path(nested_containers.__file__).with_name("nested-containers-init.sh") + .read_text(encoding="utf-8") + ) + + def test_ca_bundle_path_matches_the_backend_constant(self) -> None: + from bot_bottle.backend.util import AGENT_CA_BUNDLE + self.assertIn(f'CA_BUNDLE="{AGENT_CA_BUNDLE}"', self.script) + + def test_resolves_the_gateway_address_for_nested_containers(self) -> None: + """podman's hosts_file only applies to native `podman run` — the + Docker-compat API ignores it, and the agent types `docker`. So the + gateway name is resolved here and the *address* goes into the proxy + URL; otherwise every nested container dies at "Could not resolve + proxy", which reads like broken DNS but is a missing hosts entry.""" + self.assertIn('$2 == name { print $1; exit }', self.script) + self.assertIn('value.replace(name, ip)', self.script) + + def test_docker_cli_proxy_config_uses_the_address(self) -> None: + """The Docker CLI copies ~/.docker/config.json's proxies block into + every container it starts. Being client-side it beats the podman + service, so this — not containers.conf — is what decides whether a + nested container can reach the proxy.""" + block = self.script[self.script.index('"proxies"'):] + self.assertIn('"httpProxy": proxy.replace(name, ip)', block) + self.assertIn('"httpsProxy": proxy.replace(name, ip)', block) + self.assertIn('"noProxy": no_proxy', block) + + def test_substitutes_the_address_into_the_service_environment(self) -> None: + """Covers what the Docker CLI does not stamp: podman's own registry + pulls, and containers created through the API by another client.""" + launch = self.script[self.script.index("podman system service"):] + self.assertNotIn("$GATEWAY_NAME", launch) # substitution precedes it + setup = self.script[:self.script.index("podman system service")] + self.assertIn("for var in HTTP_PROXY HTTPS_PROXY http_proxy https_proxy", + setup) + + def test_disables_podmans_own_proxy_passthrough(self) -> None: + """podman copies the host's proxy vars into every container by + default, and that copy overrides the env we set — putting the + unresolvable gateway name back and leaving nested containers at + "bad address 'bot-bottle-gateway'".""" + self.assertIn('"http_proxy=false"', self.script) + + def test_keeps_the_gateway_name_in_no_proxy(self) -> None: + """NO_PROXY is matched against what a client asks for, and code inside + a nested container still says bot-bottle-gateway.""" + start = self.script.index('for var in ("NO_PROXY"') + loop = self.script[start:self.script.index("path = Path(", start)] + self.assertIn('entries.append(f"{var}={value}")', loop) + self.assertNotIn("replace(name, ip)", loop) + self.assertIn('"noProxy": no_proxy', self.script) + + def test_fails_closed_without_a_gateway_address(self) -> None: + self.assertIn('[ -n "$gateway_ip" ] || {', self.script) + + def test_mounts_and_trusts_the_gateway_ca(self) -> None: + """The gateway TLS-intercepts, so without the bundle every HTTPS call + from a nested container fails with "unable to get local issuer + certificate".""" + self.assertIn('volumes=["{ca}:{ca}:ro"]', self.script) + for var in ( + "SSL_CERT_FILE", "CURL_CA_BUNDLE", "REQUESTS_CA_BUNDLE", + "NODE_EXTRA_CA_CERTS", + ): + self.assertIn(f'f"{var}={{ca}}"', self.script) + + def test_writes_the_token_bearing_config_unreadable_to_others(self) -> None: + """The proxy URL carries the bottle's identity token.""" + self.assertIn("path.chmod(0o600)", self.script) + + +class TestGuestEnvironment(unittest.TestCase): + def test_disabled_bottle_gets_no_docker_environment(self) -> None: + self.assertEqual({}, nested_containers.guest_env(False)) + + def test_enabled_bottle_uses_only_the_guest_local_socket(self) -> None: + env = nested_containers.guest_env(True) + self.assertEqual("unix:///tmp/bbp/podman.sock", env["DOCKER_HOST"]) + self.assertNotIn("/var/run/docker.sock", " ".join(env.values())) + + def test_runtime_dir_leaves_room_for_conmons_attach_socket(self) -> None: + """podman builds `$XDG_RUNTIME_DIR/libpod/tmp/socket/<64-hex>/attach`, + which must fit in sun_path (108 bytes). A descriptive runtime dir blew + past it and every attached `docker run` failed with "unable to upgrade + to tcp, received 500" while pulls and detached runs looked fine.""" + attach = ( + nested_containers.guest_env(True)["XDG_RUNTIME_DIR"] + + "/libpod/tmp/socket/" + "a" * 64 + "/attach" + ) + self.assertLessEqual(len(attach), 107, attach) + + def test_macos_backend_declares_support(self) -> None: + from bot_bottle.backend.macos_container.backend import ( + MacosContainerBottleBackend, + ) + self.assertTrue(MacosContainerBottleBackend.supports_nested_containers) + + +class TestBuildOrLoadImages(unittest.TestCase): + def test_disabled_bottle_keeps_the_plain_agent_image(self) -> None: + plan = _plan( + slug="dev-abc", image="agent:base", dockerfile_path="/repo/Dockerfile", + nested_containers=False, agent_provision=_AgentProvision("agent:base"), + ) + with patch.object(launch_mod, "read_committed_image", return_value=None), \ + patch.object(launch_mod.container_mod, "build_image"), \ + patch.object(launch_mod.nested_containers_mod, "build_image") as derived: + images = launch_mod.build_or_load_images(plan) + derived.assert_not_called() + self.assertEqual("agent:base", images.agent) + + def test_enabled_bottle_builds_base_then_derived_variant(self) -> None: + plan = _plan( + slug="dev-abc", image="agent:base", dockerfile_path="/repo/Dockerfile", + nested_containers=True, agent_provision=_AgentProvision("agent:base"), + ) + with patch.object(launch_mod, "read_committed_image", return_value=None), \ + patch.object(launch_mod.container_mod, "build_image") as build, \ + patch.object( + launch_mod.nested_containers_mod, + "build_image", + return_value="agent:base-nested-containers", + ) as derived: + images = launch_mod.build_or_load_images(plan) + + build.assert_called_once_with( + "agent:base", launch_mod._REPO_DIR, # pylint: disable=protected-access + dockerfile="/repo/Dockerfile", + ) + derived.assert_called_once_with("agent:base", build) + self.assertEqual("agent:base-nested-containers", images.agent) + + def test_derived_image_layers_onto_a_committed_image(self) -> None: + plan = _plan( + slug="dev-abc", image="agent:base", dockerfile_path="/repo/Dockerfile", + nested_containers=True, agent_provision=_AgentProvision("agent:base"), + ) + with patch.object( + launch_mod, "read_committed_image", return_value="agent:committed", + ), \ + patch.object(launch_mod.container_mod, "image_exists", return_value=True), \ + patch.object(launch_mod.container_mod, "build_image") as build, \ + patch.object( + launch_mod.nested_containers_mod, + "build_image", + return_value="agent:committed-nested-containers", + ) as derived: + images = launch_mod.build_or_load_images(plan) + + build.assert_not_called() + derived.assert_called_once_with("agent:committed", build) + self.assertEqual("agent:committed-nested-containers", images.agent) + + def test_cached_policy_refuses_to_build_the_derived_image(self) -> None: + plan = _plan( + slug="dev-abc", image="agent:base", dockerfile_path="/repo/Dockerfile", + nested_containers=True, agent_provision=_AgentProvision("agent:base"), + spec=_Spec(image_policy="cached"), + ) + with patch.object(launch_mod, "read_committed_image", return_value=None), \ + patch.object( + launch_mod.container_mod, "image_exists", + side_effect=_base_image_only, + ), \ + patch.object(launch_mod.nested_containers_mod, "build_image") as derived, \ + patch.object(launch_mod, "die", side_effect=RuntimeError) as die: + with self.assertRaises(RuntimeError): + launch_mod.build_or_load_images(plan) + derived.assert_not_called() + self.assertIn("agent:base-nested-containers", die.call_args.args[0]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_manifest_bottle_merge.py b/tests/unit/test_manifest_bottle_merge.py index a0b3795..9aad8cb 100644 --- a/tests/unit/test_manifest_bottle_merge.py +++ b/tests/unit/test_manifest_bottle_merge.py @@ -56,6 +56,16 @@ class TestMergeBottlesRuntime(unittest.TestCase): result = merge_bottles_runtime([base, override]) self.assertFalse(result.supervise) + def test_nested_containers_survives_a_later_bottle(self): + """OR, not replace: a resolved bottle that never mentioned the key is + indistinguishable from one that set it false, so `--bottle with-docker + --bottle claude-dev` must not silently drop the capability.""" + enabled = _bottle(nested_containers=True) + quiet = _bottle(env={"X": "1"}) + self.assertTrue(merge_bottles_runtime([enabled, quiet]).nested_containers) + self.assertTrue(merge_bottles_runtime([quiet, enabled]).nested_containers) + self.assertFalse(merge_bottles_runtime([quiet, quiet]).nested_containers) + def test_three_bottles_merged_left_to_right(self): b1 = _bottle(env={"A": "1", "B": "1", "C": "1"}) b2 = _bottle(env={"B": "2", "C": "2"}) diff --git a/tests/unit/test_manifest_validation.py b/tests/unit/test_manifest_validation.py index d23cc77..618717d 100644 --- a/tests/unit/test_manifest_validation.py +++ b/tests/unit/test_manifest_validation.py @@ -44,6 +44,17 @@ class TestBottleValidation(unittest.TestCase): with self.assertRaises(ManifestError): ManifestBottle.from_dict("b", {"supervise": "yes"}) + def test_nested_containers_not_bool(self) -> None: + with self.assertRaises(ManifestError): + ManifestBottle.from_dict("b", {"nested_containers": "yes"}) + + def test_nested_containers_defaults_off(self) -> None: + self.assertFalse(ManifestBottle.from_dict("b", {}).nested_containers) + self.assertTrue( + ManifestBottle.from_dict("b", {"nested_containers": True}) + .nested_containers + ) + def test_removed_runtime_field(self) -> None: with self.assertRaises(ManifestError): ManifestBottle.from_dict("b", {"runtime": "runsc"})