diff --git a/.gitignore b/.gitignore index 47fc1078..c5b0bbc2 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ .env .env.* .stashbase/* -.serena/* \ No newline at end of file +.serena/* +docs/superpowers/ \ No newline at end of file diff --git a/README.md b/README.md index e918effc..9692d71f 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ Stashbase is an open-source access layer that gives coding agents the access the - [How the Agent Proxy Works](#how-the-agent-proxy-works) - [Profile Syntax and Configuration](#profile-syntax-and-configuration) - [Filesystem and Network Containment](#filesystem-and-network-containment) + - [Docker Sandbox Backend](#docker-sandbox-backend) - [Remote Agent Sessions](#remote-agent-sessions) - [MCP Tools Authorization](#mcp-tools-authorization) - [Audit Logs and Session Revocation](#audit-logs-and-session-revocation) @@ -231,7 +232,7 @@ deny_write = [".git", "~/.ssh", "~/.aws"] On macOS, Stashbase wraps the agent in Seatbelt, which enforces filesystem rules. On Linux and WSL2, it uses `systemd-run --user` with cgroup IP rules, or falls back to `bubblewrap` for namespace isolation. Windows native is not implemented; use WSL2 instead. -Denied reads return `/dev/null`; denied writes go to an empty overlay. Existing file descriptors and data already in memory are not affected. These are policy-only; these profiles do not require secrets. +Denied reads see empty content (a genuine empty regular file, not `/dev/null` — that's a character device, which confuses tooling that expects a normal file at that path); denied writes go to an empty overlay. Existing file descriptors and data already in memory are not affected. These are policy-only; these profiles do not require secrets. #### Network containment @@ -241,6 +242,27 @@ On macOS, this uses the deprecated `sandbox-exec` utility. On Linux and WSL2, it This is network containment only, not filesystem, process-memory, or kernel isolation. +**If Docker is available, prefer the Docker sandbox backend below over the native one** — it's meaningfully stronger: filesystem access is allow-list rather than deny-list (nothing outside the working directory is visible at all, instead of specific paths being blocked), network egress is enforced at the network layer rather than relying on the agent to honor its proxy environment variables, and it works identically across macOS, Linux, and Windows (via Docker Desktop) instead of needing platform-specific mechanisms with a Windows gap. The native backend remains the default for now since it needs nothing beyond the CLI itself, but Docker is the recommended choice whenever it's an option. + +### Docker Sandbox Backend + +The recommended backend when Docker is available: the agent runs inside a Docker container instead of a same-host sandboxed process, with allow-list filesystem access and a network-layer firewall (enforced even against an agent that deliberately ignores its proxy env vars). + +```toml +[sandbox] +backend = "docker" +``` + +```bash +stashbase agent run --profile coding -- claude +``` + +Or override the profile's choice for one invocation without editing the file: `--docker-sandbox true|false`, per-run image overrides with `--docker-image ` / `--docker-dockerfile `, and resource caps with `--docker-memory ` / `--docker-cpus ` (also settable per profile via `[sandbox] memory`/`cpus`; no cap by default). + +Claude Code and Codex are pre-installed in the default sandbox image; a profile can also run its own image or Dockerfile instead (`[sandbox] image`/`dockerfile`) to add other tools, without loosening any of the sandbox constraints themselves. + +See **[docs/sandboxing.md](docs/sandboxing.md)** for the full picture: how the network firewall is enforced, custom images, git identity forwarding, login persistence across images, Codex/Claude Code OAuth quirks, and current limitations. + ### Remote Agent Sessions Use `--remote` to run with credentials managed entirely in the Stashbase control plane. Profiles can use either application secrets with `[secrets]` (requires `project` and `environment`) or user-specific `[personal_credentials]` (no Stashbase API key required): diff --git a/docker/agent-sandbox/Dockerfile b/docker/agent-sandbox/Dockerfile new file mode 100644 index 00000000..b432d540 --- /dev/null +++ b/docker/agent-sandbox/Dockerfile @@ -0,0 +1,78 @@ +# Node.js is still needed here even though Claude Code and Codex are +# installed as native binaries below (neither invokes Node itself at +# runtime): MCP servers agents commonly connect to are typically run via +# `npx`, so `npm`/`node` need to be on PATH for those to work. This is a +# Debian base underneath (bookworm-slim variant), so apt-get below works +# the same as it would on `debian:bookworm-slim`. +FROM node:22-bookworm-slim + +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + ca-certificates \ + curl \ + git \ + gh \ + bubblewrap \ + iptables \ + jq \ + dnsutils \ + unzip \ + less \ + procps \ + python3 \ + python3-pip \ + python3-venv \ + && rm -rf /var/lib/apt/lists/* + +# Debian's system pip refuses a bare `pip install` (PEP 668, +# "externally-managed-environment") to protect the OS's own Python install +# from being clobbered by unrelated packages. That protection matters far +# less for an ephemeral sandbox container than it does on a real host — the +# base image itself is never mutated at runtime, so there's nothing lasting +# to corrupt — so it's relaxed here for a working `pip install` out of the +# box. `python3-venv` is still included for anyone who wants an isolated +# environment anyway. +ENV PIP_BREAK_SYSTEM_PACKAGES=1 + +# Claude Code and Codex are installed via their own official native +# installers rather than `npm install -g` — both projects document this as +# the recommended method: it installs a self-contained platform binary +# (no Node runtime involved for the tool itself, unlike the npm package +# which just wraps the same binary), and pulls from Anthropic's/OpenAI's +# own signed release infrastructure rather than the npm registry. +# +# Both installers default to installing under $HOME (`/root` during this +# build) and only add `~/.local/bin` to *root's* shell profile — neither +# knows this container will later run as an arbitrary uid (see the --user +# flag set on Linux in docker_run_command). So: make the installed trees +# world-readable/executable (`chmod -R a+rX`, never +w — nothing here +# should be writable by the sandboxed process) and symlink both into +# `/usr/local/bin`, which is on PATH for every user by default. Verified +# directly that both `claude --version` and `codex --version` work when +# invoked as a non-root uid with this layout, not just as root. +# +# Codex ships helper binaries (a code-mode host, voice tools) alongside +# its main executable, so its install tree is kept intact and symlinked +# rather than copying a single binary out the way Claude Code's simpler, +# single-file layout would allow. +RUN curl -fsSL https://claude.ai/install.sh | bash \ + && curl -fsSL https://chatgpt.com/codex/install.sh | sh \ + && chmod 755 /root \ + && chmod -R a+rX /root/.local /root/.codex \ + && ln -s /root/.local/bin/claude /usr/local/bin/claude \ + && ln -s /root/.local/bin/codex /usr/local/bin/codex + +# A persistent volume is mounted here (see docker_sandbox.rs) so login +# state (e.g. Claude Code's ~/.claude, ~/.claude.json) survives across +# runs instead of vanishing with each --rm'd container. World-writable +# because the container may run as an arbitrary host uid (see the --user +# flag set on Linux in docker_run_command) with no matching passwd entry, +# so a named volume freshly created by Docker would otherwise be +# root-owned and unwritable to that uid. +RUN mkdir -p /home/agent && chmod 777 /home/agent + +# `iptables` above is used by the short-lived network-namespace-holder +# container (see start_netns_holder in docker_sandbox.rs), which uses this +# same image — the actual agent container never runs iptables itself and +# holds no networking capabilities at all. +WORKDIR /workspace diff --git a/docs/agent-profiles.md b/docs/agent-profiles.md index 72d8a0ab..8a47b93c 100644 --- a/docs/agent-profiles.md +++ b/docs/agent-profiles.md @@ -53,7 +53,7 @@ paths = ["/user", "/repos/*"] The child process receives `GH_TOKEN` as a placeholder. When it makes a matching HTTP request (GET to `/user` or `/repos/*` on `api.github.com`), the proxy injects the real token. Any unmatched request is blocked. -## Filesystem and Network Restrictions +## Filesystem, Network Restrictions, and Sandbox Backends Restrict what the child can read or write: @@ -63,12 +63,11 @@ deny_read = ["~/.ssh", "~/.aws"] deny_write = ["~/.git"] ``` -Paths use explicit prefixes: `~` for home, relative paths for the current directory. Enforcement uses platform-native mechanisms: -- **macOS**: Seatbelt sandbox -- **Linux**: `systemd-run` or `bubblewrap` (automatic fallback) -- **Unsupported platforms**: Validation fails closed; the run does not proceed +Paths use explicit prefixes: `~` for home, relative paths for the current directory. -Existing file descriptors and data already in process memory remain unrestricted. +By default, enforcement uses the platform-native mechanism (Seatbelt on macOS, `systemd-run`/`bubblewrap` on Linux). **If Docker is available, prefer `[sandbox] backend = "docker"` instead** — it's meaningfully stronger (allow-list filesystem access, a real network-layer firewall, and it works on Windows too, unlike the native backend). + +See **[Sandboxing](sandboxing.md)** for the full picture: both backends, how the Docker backend's network firewall is enforced, custom images, git identity forwarding, login persistence, and Codex/Claude Code OAuth quirks. ## Network Access and HTTP Rules @@ -205,7 +204,7 @@ The proxy is HTTP/HTTPS only and designed for standard developer tools. It does - Request-body or query-parameter injection (credentials are header-only) - Process-level isolation (same-user processes can still access broader system credentials) -For complete network isolation, use a container or VM. +For stronger filesystem and network isolation than the native backend provides, see [Sandboxing](sandboxing.md). ## Full Reference diff --git a/docs/agent-profiles/integrations.md b/docs/agent-profiles/integrations.md index afd5b0e4..3c834755 100644 --- a/docs/agent-profiles/integrations.md +++ b/docs/agent-profiles/integrations.md @@ -8,7 +8,7 @@ Claude Code can access external tools and APIs through agent profiles. For examp ```toml # .stashbase/agents/claude.toml -egress_hosts = ["api.anthropic.com", "mcp.linear.app"] +egress_hosts = ["api.anthropic.com", "platform.claude.com", "mcp.linear.app"] [secrets] project = "my-project" @@ -37,13 +37,15 @@ stashbase agent run --profile claude -- claude Claude Code receives only the Linear credential placeholder and can use allowed MCP tools through the proxy. +`platform.claude.com` is required alongside `api.anthropic.com` if you're logged in via OAuth (`/login` in Claude Code) rather than an API key — Claude Code's OAuth login and silent token refresh both go through `platform.claude.com`, a different host than the one used for actual model requests. Without it, login itself fails with "OAuth error: proxy refused the connection", or — if you were already logged in before restricting egress — the session works until the access token's next refresh is silently blocked, then fails hours later with "OAuth access token has expired." + ## Codex Codex needs GitHub access for repository operations and OpenAI for completions: ```toml # .stashbase/agents/codex.toml -egress_hosts = ["api.openai.com", "chatgpt.com", "api.github.com"] +egress_hosts = ["api.openai.com", "chatgpt.com", "auth.openai.com", "api.github.com"] allow_hooks = ["dependency_check"] [secrets] @@ -81,6 +83,8 @@ Run: stashbase agent run --profile codex -- codex ``` +`auth.openai.com` is required for `codex login --device-auth` (and its silent token refresh) — it's a different host than `api.openai.com`, which only serves completions. Without it, device-code login fails with an error like "failed to request device code: error sending request for url (https://auth.openai.com/api/accounts/deviceauth/usercode)". + ## HTTP MCP Server MCP servers over HTTP (like Linear) use a separate credential binding and tool allowlist: diff --git a/docs/sandboxing.md b/docs/sandboxing.md new file mode 100644 index 00000000..da37b148 --- /dev/null +++ b/docs/sandboxing.md @@ -0,0 +1,180 @@ +# Sandboxing + +`stashbase agent run` isolates the child process's filesystem and network access using one of two backends. Both are configured per profile under `[sandbox]` and `[filesystem]` — see [Agent Profiles](agent-profiles.md) for the rest of the profile schema (secrets, egress rules, MCP tool restrictions, etc.). + +**Recommendation: use the Docker backend whenever Docker is available.** It's meaningfully stronger on every axis that matters for running an agent you don't fully trust: + +- **Filesystem**: allow-list (only the working directory is visible at all) instead of deny-list (specific paths blocked, everything else still reachable). +- **Network**: enforced by a real firewall inside the container's network namespace, not by the agent choosing to honor `HTTPS_PROXY`/`HTTP_PROXY` — a process that deliberately opens a raw socket is blocked the same as one that respects the proxy. +- **Platform coverage**: works identically on macOS, Linux, and Windows (via Docker Desktop), rather than the native backend's platform-specific mechanisms that don't exist on Windows at all. (Windows support here has been implemented and reasoned through carefully — same Docker Desktop VM-boundary handling as macOS — but not yet run end-to-end on a real Windows machine.) +- **Extensibility**: `sandbox.image`/`sandbox.dockerfile` let a profile add exactly the tools it needs (Python, a compiler, whatever) without weakening the sandbox itself. + +The native backend stays the default because it needs nothing beyond the CLI itself — no Docker install, no daemon, no image to build — which matters for a quick first run. But once Docker is available, there's no real reason to prefer the weaker guarantees of the native backend over it. + +## Native backend (default) + +No configuration needed — this is what every profile gets unless `[sandbox] backend = "docker"` is set. Filesystem restrictions are opt-in: + +```toml +[filesystem] +deny_read = ["~/.ssh", "~/.aws"] +deny_write = ["~/.git"] +``` + +Paths use explicit prefixes: `~` for home, relative paths for the current directory. Enforcement uses platform-native mechanisms: +- **macOS**: Seatbelt sandbox +- **Linux**: `systemd-run` or `bubblewrap` (automatic fallback) +- **Windows and other unsupported platforms**: Validation fails closed; the run does not proceed with the native backend + +Existing file descriptors and data already in process memory remain unrestricted. Network egress is still enforced the same way it is under the Docker backend — through the loopback credential proxy and `egress_hosts`/`deny_hosts` — but there is no network-layer firewall backing that up the way there is for Docker; a process that ignores `HTTPS_PROXY`/`HTTP_PROXY` entirely and opens a raw connection can reach the network directly under the native backend. + +**Windows users**: the native backend doesn't support Windows at all, but the Docker backend does — it only needs Docker Desktop, not any platform-native sandboxing primitive. `agent validate` correctly checks Docker readiness instead of the native mechanisms for a Docker-backend profile, so it won't falsely report Windows as unsupported for a profile that sets `backend = "docker"`. + +## Docker backend + +Opt into container-based isolation instead: + +```toml +[sandbox] +backend = "docker" +``` + +Or override the profile's choice for a single invocation without editing the file, with `--docker-sandbox`: + +```bash +stashbase agent run --profile coding --docker-sandbox true -- claude # force Docker for this run +stashbase agent run --profile coding --docker-sandbox false -- claude # force native for this run +``` + +`--docker-sandbox` only overrides which backend enforces the run; it does not change any other profile setting (secrets, egress rules, `deny_read`/`deny_write`, etc.). Omit it to use whatever the profile declares. + +With `backend = "docker"`, the agent process runs inside a container on a fresh, isolated Docker network created for that single `agent run` invocation. Compared to the native backend: + +- The container has its own network namespace and can reach the host's credential proxy but nothing else — no LAN, no other local processes, no host-only services. This is enforced at the network layer inside the container (an `iptables` rule described below), not merely by the agent choosing to honor `HTTPS_PROXY`/`HTTP_PROXY`: a process that deliberately ignores those env vars and opens a raw connection is blocked the same as one that respects them, on both macOS and Linux. +- Filesystem access is allow-list, not deny-list: only the current working directory is visible inside the container. `deny_read`/`deny_write` paths outside the working directory are already invisible; paths inside it are additionally shadow-mounted (empty for `deny_read`, read-only for `deny_write`) so the same guarantee holds. +- Requires Docker installed and the daemon running. If Docker isn't available, the run fails closed with an error — it does not fall back to running unsandboxed or to the native backend. +- On Docker Desktop (macOS/Windows), the proxy binds to loopback and the container reaches it via `host.docker.internal`, since Desktop containers run inside a VM and cannot reach the host's bridge-network gateway directly. On native Linux Docker, the proxy binds to the per-run network's gateway address instead. Both platforms additionally get the network-layer firewall rule described below, which is what actually blocks a bypass attempt — the platform difference here only affects how the container reaches the proxy, not whether egress is enforced. +- The container always runs with `--init` (a real PID 1 that reaps zombie processes and forwards signals correctly) and a `--pids-limit` of 2048 — a generous cap no real workload comes close to, existing purely to contain a fork bomb to the container's own cgroup instead of the host. Unlike the memory/CPU limits below, these are never configurable and always on, since there's no legitimate workload either could break. + +### How network egress is enforced + +Enforcement is split across two containers per run, not built into the agent container itself: + +1. A short-lived **network namespace holder** is started first, attached to the run's isolated network, holding the `NET_ADMIN` Linux capability (`--cap-drop ALL --cap-add NET_ADMIN`). It installs one `iptables` rule — default-DROP all outbound traffic, with exceptions only for loopback and the credential proxy's specific resolved address and port — then verifies the rule actually took effect (a known-arbitrary host must be unreachable, and the proxy itself must still be reachable; either check failing means setup fails closed rather than continuing with a possibly-ineffective firewall) before blocking forever, keeping that network namespace alive. + + DNS gets no exception at all: Docker's embedded resolver (127.0.0.11) forwards unresolved lookups via the *host's* own DNS stack, entirely outside the container's network namespace — no `iptables` rule inside the container can see or block that traffic, since it never traverses the container's own OUTPUT chain. Left unaddressed, that's a live exfiltration channel (an agent can encode data in a query name to a domain it controls and have Docker itself relay it out). The holder's own DNS is instead pointed at a blackhole address (`--dns 0.0.0.0`) at creation time, which the agent inherits by sharing its network namespace. Local lookups like `host.docker.internal` still work — those resolve from `/etc/hosts`, never touching the (now-disabled) upstream forwarder — and the agent doesn't need real DNS for anything else, since the proxy address it's given is already a resolved raw IP. +2. The **agent container** joins that exact network namespace (`--network container:`) but holds no added capabilities of its own at all (`--cap-drop ALL`, nothing re-added). Namespace rules — including the firewall — are shared by anything attached to the namespace; the *capability* to change them is not. The agent container can use the firewall but can never modify it. + +This two-container split exists because the more obvious approach — start the agent container as root with `NET_ADMIN`, set up the firewall, then drop privileges and capabilities before running the real command — turned out not to work on Docker Desktop: dropping capabilities from inside a container requires the `CAP_SETPCAP` capability, and Docker Desktop silently zeroes out a container's entire capability set the moment `CAP_SETPCAP` is requested (confirmed directly, not assumed). Splitting the privileged setup into a separate container that never runs the actual agent sidesteps this entirely — the agent container never needs `CAP_SETPCAP`, `NET_ADMIN`, or root, on any platform. + +This was verified directly, including trying to defeat it from inside a real sandboxed session: a deliberate bypass attempt (unsetting all proxy env vars and issuing a raw `curl` to an arbitrary host) is blocked with a connection failure, identical to what the credential proxy itself returns for a denied host — and an attempt to run `iptables -F OUTPUT` from inside the agent container to erase the rule and reopen egress fails outright with "Permission denied," since the agent process holds zero capabilities. + +### Supported agents and the sandbox image + +Claude Code and Codex are pre-installed in the default sandbox image and are the only agents validated against this backend so far. Other tools that don't need anything beyond what the image provides should also run. + +The default image is built from `node:22-bookworm-slim` (Debian underneath) with `git`, `curl`, `ca-certificates`, `bubblewrap`, `iptables`, `jq`, `gh`, `dnsutils`, `unzip`, `less`, `procps`, and Python 3 (`python3`, `python3-pip`, `python3-venv`) installed via `apt`, plus Claude Code and Codex installed via their own official native installer scripts (not `npm install -g` — see the Dockerfile for why). `pip install` works out of the box without needing a virtualenv first — Debian's system pip normally refuses this (PEP 668), but that protection matters less for an ephemeral sandbox container than a real host, so it's relaxed here. It is not published to a registry — the Dockerfile is embedded in the `stashbase` binary itself, so a plain installed copy of the CLI can build it locally without needing this source repository. The first `agent run` that selects the Docker backend detects the image is missing and offers to build it (interactively; `--silent` runs fail closed instead of prompting). The build streams Docker's own progress live rather than running silently. + +### Custom images + +A profile can run a different image instead of the built-in default, either your own pre-built image or a custom Dockerfile — for example to add a language toolchain, a package manager, or other tools your agent needs. The default image stays the maintained baseline for everyone; this is the escape hatch for when it isn't enough. + +```toml +[sandbox] +backend = "docker" +image = "myorg/my-agent-image:latest" # a pre-built image; `docker run` pulls it if missing locally +``` + +```toml +[sandbox] +backend = "docker" +dockerfile = "./sandbox.Dockerfile" # path relative to the current working directory; stashbase builds and tags it locally +``` + +`image` and `dockerfile` are mutually exclusive — set at most one. Neither weakens the sandbox itself: regardless of which image runs, the agent container always gets `--cap-drop ALL`, `no-new-privileges`, and the same network-namespace-holder firewall described above — a custom image can add tools, but it cannot request more capabilities or opt out of network/filesystem enforcement. The network-namespace holder itself (the one privileged container, holding `NET_ADMIN`) always uses the built-in default image regardless of this setting, never a custom one. + +A `dockerfile` build is tagged deterministically from its path and only rebuilt when that tag doesn't already exist locally — edit the Dockerfile and remove the old image (`docker image rm`) to force a rebuild. + +A minimal base image needs `ca-certificates` installed for TLS-intercepted HTTPS requests to work — without it, the image's HTTP client can't validate the proxy's injected CA and HTTPS calls fail with a certificate error even though the request itself was allowed by policy. Verified with a plain `alpine:latest` image: unencrypted HTTP calls to allowed and denied hosts behave correctly out of the box, but HTTPS needs `apk add ca-certificates` (or the base image's equivalent) first. A musl-based image like Alpine also needs a glibc compatibility shim (e.g. `apk add gcompat libstdc++`) to run Claude Code's or Codex's native installer binaries, which are built against glibc — verified working with this combination. + +`--docker-image ` and `--docker-dockerfile ` override `sandbox.image`/`sandbox.dockerfile` for a single invocation, the same way `--docker-sandbox` overrides `sandbox.backend` — useful for trying a different image without editing the profile file. Either flag implies the Docker backend for that run even if the profile declares `backend = "native"` (or doesn't set `[sandbox]` at all), and the two are mutually exclusive with each other: + +```bash +stashbase agent run --profile coding --docker-image node:22-alpine -- claude +``` + +### Git identity + +Your global `git config user.name` and `user.email` (if configured on the host) are forwarded into the container as `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME`, and `GIT_COMMITTER_EMAIL`. This is the one piece of host configuration deliberately forwarded despite the filesystem allow-list, since it's authorship metadata, not a credential — without it, `git commit` inside the sandbox fails with no identity configured. It does not grant push access: `git push` (or any other authenticated git operation) still needs a real credential, wired through `[secrets]` like `GITHUB_TOKEN`, or run from outside the sandbox. Raw SSH keys are never forwarded. A profile that explicitly sets one of these four env vars itself takes precedence over the forwarded host value. + +### Login persistence + +Agent login/config state (e.g. Claude Code's `~/.claude`, Codex's `~/.codex`) is kept in a Docker-managed named volume, not a bind mount of your real home directory, so it survives across `agent run` invocations without exposing anything else on the host. This volume is shared across every profile, project, *and image* using the Docker backend on this machine — logging in once covers all of them, even after switching to a completely different custom image or Dockerfile, since the volume is mounted at the same container path (`/home/agent`) regardless of which image runs. + +### Codex and subscription login + +Codex's normal OAuth login flow opens a browser that redirects to a local HTTP callback server. That callback listens inside the container's own network namespace, which the host browser cannot reach — the container's `localhost` is not your machine's `localhost`. Use Codex's device-code flow instead, which doesn't depend on a local callback at all: + +```bash +stashbase agent run --profile coding -- codex login --device-auth +``` + +`auth.openai.com` must be in `egress_hosts` for device-code login (and its silent token refresh) to work — it's a different host than `api.openai.com`, which only serves completions. + +Claude Code has the same kind of gap: `platform.claude.com` must be in `egress_hosts` alongside `api.anthropic.com` for OAuth login (`/login`) and silent token refresh to work. Without it, login fails with "OAuth error: proxy refused the connection," or — if you were already logged in before restricting egress — the session works until the access token's next refresh is silently blocked, then fails hours later with "OAuth access token has expired." + +### Resource limits + +No CPU or memory cap is applied by default — an automatic one could silently break a legitimately heavy task with no warning. Opt in per profile: + +```toml +[sandbox] +backend = "docker" +memory = "2g" # docker run --memory +cpus = "1.5" # docker run --cpus +``` + +Or per invocation, without editing the file: `--docker-memory ` / `--docker-cpus ` (same override precedence as `--docker-image`/`--docker-dockerfile`, but these don't imply the Docker backend on their own — they're only meaningful once Docker is already selected). `agent validate` checks the value looks like something Docker would accept before you ever try to run it. + +### Cleaning up after a crash + +Every per-run Docker network (and its two containers) is named after that run's own session id — the same `ags_...` id shown in "Agent session"/"Audit session" and used for the audit log filename — specifically so leftovers can be traced back to the run that created them. Normal exit paths, including Ctrl+C, tear both containers and the network down as part of `agent run` itself; only a crash or a forceful `SIGKILL` of the `stashbase` process can leave them behind. + +```bash +stashbase agent docker cleanup +``` + +Lists any `stashbase-agent-run-*` networks still present, skips ones tied to a session this machine still has a live local record for, and asks for confirmation before removing the rest (`--yes` skips the prompt). A `--remote` run has no local record to check against at all, so a listed network could in principle still belong to a remote session genuinely in progress — the command shows each one's creation time so you can judge, rather than guessing on your behalf. + +To just look without removing anything: + +```bash +stashbase agent docker status +``` + +Lists the same networks (name, session id, creation time, and whether it's tied to a live local session) — pass `--json` for machine-readable output. + +### Managing the default image + +```bash +stashbase agent docker build [--force] +``` + +Builds the default sandbox image ahead of time instead of waiting to be prompted on first `agent run`, or rebuilds it with `--force` (e.g. after the embedded Dockerfile picks up new apt packages or a security patch) without needing to `docker rmi` it by hand first. + +Add `--profile ` to target that profile's own `sandbox.image`/`sandbox.dockerfile` instead of the default — useful for pre-building or force-refreshing a custom image the same way, without needing to trigger a real `agent run` first. `--profile-source auto|global|directory` controls where `--profile` is loaded from, same as `agent run`/`agent validate`. A profile using a plain `image` reference has nothing to build (`docker run` pulls it automatically), so this reports that and does nothing rather than erroring. + +### Checking readiness + +```bash +stashbase agent docker doctor +``` + +Checks whether the Docker sandbox backend can actually run here — the `docker` CLI on PATH, the daemon reachable (and its version), and whether the default image is already built — without starting a real sandboxed run to find out. Exits non-zero if anything's not ready; pass `--json` for machine-readable output. Useful for onboarding or CI setup scripts that want to fail fast with a clear reason, rather than discovering a missing Docker install only when a real `agent run` fails. + +### Docker backend limitations + +- Two containers run per invocation (the network namespace holder plus the agent container itself), not one — slightly more setup overhead per run than a single-container approach, in exchange for the firewall being enforced by capability separation rather than a privilege drop inside the agent container. +- The persistent home volume is shared across every profile and project — chat history and config from one profile's sandboxed sessions are visible to another profile's sandboxed sessions on the same machine. This is a privacy boundary, not a security one: it never grants access beyond what each run's own profile allows, since egress/credential policy is enforced per-run regardless of what's in the shared volume. + +This backend is opt-in only and does not change the default behavior of existing profiles. diff --git a/src/cmd/agent.rs b/src/cmd/agent.rs index dafc5286..a894d05e 100644 --- a/src/cmd/agent.rs +++ b/src/cmd/agent.rs @@ -43,6 +43,8 @@ pub enum AgentSubcommand { McpCheck(AgentMcpCheckCommand), /// View local metadata-only proxy audit logs Logs(AgentLogsCommand), + /// Manage Docker sandbox backend resources + Docker(AgentDockerCommand), } #[derive(Debug, Subcommand)] @@ -53,6 +55,52 @@ pub enum AgentSessionsSubcommand { Revoke(AgentRevokeCommand), } +#[derive(Debug, Args)] +pub struct AgentDockerCommand { + #[command(subcommand)] + pub subcommand: AgentDockerSubcommand, +} + +#[derive(Debug, Subcommand)] +pub enum AgentDockerSubcommand { + /// Find and remove Docker sandbox networks/containers left behind by a run that didn't tear down cleanly (e.g. `stashbase` was killed with SIGKILL mid-run) + Cleanup(AgentDockerCleanupCommand), + /// List Docker sandbox networks/containers currently present on this machine + Status(AgentDockerStatusCommand), + /// Build (or rebuild) the default Docker sandbox image + Build(AgentDockerBuildCommand), + /// Check whether the Docker sandbox backend can run on this machine + Doctor(AgentDockerDoctorCommand), +} + +#[derive(Debug, Args)] +pub struct AgentDockerDoctorCommand {} + +#[derive(Debug, Args)] +pub struct AgentDockerCleanupCommand { + /// Remove every leftover resource without prompting for confirmation + #[arg(long)] + pub yes: bool, +} + +#[derive(Debug, Args)] +pub struct AgentDockerStatusCommand {} + +#[derive(Debug, Args)] +pub struct AgentDockerBuildCommand { + /// Rebuild even if the image already exists locally + #[arg(long)] + pub force: bool, + + /// Build the given profile's `sandbox.image`/`sandbox.dockerfile` instead of the built-in default image + #[arg(long)] + pub profile: Option, + + /// Where to load --profile from + #[arg(long, value_enum, default_value = "auto")] + pub profile_source: AgentProfileSource, +} + #[derive(Debug, Args)] #[command(override_usage = "agent sessions list [--local | --remote]")] pub struct AgentSessionsCommand { @@ -178,6 +226,38 @@ pub struct AgentRunCommand { #[arg(long)] pub remote: bool, + /// Override the profile's `[sandbox] backend` for this run only: `true` + /// forces the Docker backend, `false` forces the native backend. + /// Omit to use whatever the profile declares. + #[arg(long, value_parser = clap::builder::BoolishValueParser::new())] + pub docker_sandbox: Option, + + /// Override the profile's `[sandbox] image` for this run only: run this + /// image instead of the profile's configured one (or the built-in + /// default). Implies the Docker backend even if the profile or + /// `--docker-sandbox` says otherwise. Mutually exclusive with + /// `--docker-dockerfile`. + #[arg(long, conflicts_with = "docker_dockerfile")] + pub docker_image: Option, + + /// Override the profile's `[sandbox] dockerfile` for this run only: + /// build and run this Dockerfile instead of the profile's configured + /// one (or the built-in default). Implies the Docker backend even if + /// the profile or `--docker-sandbox` says otherwise. Mutually + /// exclusive with `--docker-image`. + #[arg(long, conflicts_with = "docker_image")] + pub docker_dockerfile: Option, + + /// Override the profile's `[sandbox] memory` for this run only: + /// `docker run --memory` value, e.g. "2g". No cap by default. + #[arg(long)] + pub docker_memory: Option, + + /// Override the profile's `[sandbox] cpus` for this run only: `docker + /// run --cpus` value, e.g. "1.5". No cap by default. + #[arg(long)] + pub docker_cpus: Option, + /// Store metadata-only proxy audit events locally #[arg( long, diff --git a/src/config/config.rs b/src/config/config.rs index 7326a854..8d320e63 100644 --- a/src/config/config.rs +++ b/src/config/config.rs @@ -439,7 +439,20 @@ mod tests { let loaded = get_directory_agent_profile_from_dir(&directory, "codex") .unwrap() .unwrap(); - assert_eq!(loaded.source, "./.stashbase/agents/codex.toml"); + // Built the same way production does (join + `Display`) rather + // than a hardcoded forward-slash literal: `Path`'s `Display` uses + // the OS-native separator, so a literal like this only matches on + // Unix — on Windows the real value has backslashes in the nested + // part (`./.stashbase/agents\codex.toml`), which isn't a bug in + // the production code, just something a hardcoded literal can't + // account for. + let expected_source = format!( + "./{}", + std::path::PathBuf::from(DIRECTORY_AGENT_PROFILES_DIR) + .join("codex.toml") + .display() + ); + assert_eq!(loaded.source, expected_source); assert_eq!( loaded.profile.secrets.bindings["GITHUB_TOKEN"].hosts, ["api.github.com"] diff --git a/src/handlers/agent/docker.rs b/src/handlers/agent/docker.rs new file mode 100644 index 00000000..23f92ecf --- /dev/null +++ b/src/handlers/agent/docker.rs @@ -0,0 +1,504 @@ +use std::collections::HashSet; + +use anyhow::Result; + +use crate::cmd::agent::{ + AgentDockerBuildCommand, AgentDockerCleanupCommand, AgentDockerDoctorCommand, + AgentDockerStatusCommand, +}; +use crate::handlers::run::docker_sandbox::ExistingRunNetwork; +use crate::utils::output::{get_formatted_json_string, ColorizeIfColoredOutput}; + +/// Whether `network` should be skipped as a cleanup candidate because it's +/// tied to a session this machine still has a live local record for. Pure +/// and Docker-free so it's directly unit-testable; the Docker-backed +/// listing and the local-session lookup both happen in the caller. +fn belongs_to_a_live_local_session( + network: &ExistingRunNetwork, + local_sessions: &HashSet, +) -> bool { + network + .session_id + .as_deref() + .is_some_and(|id| local_sessions.contains(id)) +} + +/// Every per-run Docker network still present on this machine, paired with +/// whether it's tied to a session this machine still has a live local +/// record for. Shared by `status` (which reports both) and `cleanup` +/// (which only ever acts on the ones that aren't). +fn list_networks_with_liveness() -> Result> { + let networks = crate::handlers::run::docker_sandbox::list_run_networks() + .map_err(|error| anyhow::anyhow!("failed to list Docker sandbox networks: {error}"))?; + let local_sessions = super::sessions::list_local_sessions() + .unwrap_or_default() + .into_iter() + .map(|session| session.session_id) + .collect::>(); + Ok(networks + .into_iter() + .map(|network| { + let live = belongs_to_a_live_local_session(&network, &local_sessions); + (network, live) + }) + .collect()) +} + +/// Lists every Docker sandbox network currently present on this machine, +/// regardless of whether it belongs to a live session — a read-only view of +/// what `cleanup` would consider, without removing anything. +pub async fn handle_docker_status_command( + _command: AgentDockerStatusCommand, + raw_output: bool, +) -> Result<()> { + let entries = list_networks_with_liveness()?; + if raw_output { + let json = entries + .iter() + .map(|(network, live)| { + serde_json::json!({ + "name": network.name, + "session_id": network.session_id, + "created_at": network.created_at, + "live_local_session": live, + }) + }) + .collect::>(); + println!("{}", get_formatted_json_string(&json, true)?); + return Ok(()); + } + if entries.is_empty() { + println!("No Docker sandbox networks currently present."); + return Ok(()); + } + for (network, live) in &entries { + let live_label = if *live { + format!(" {}", "(live local session)".blue_if_tty()) + } else { + String::new() + }; + println!( + "{} created {}{live_label}", + network.name, + if network.created_at.is_empty() { + "unknown" + } else { + network.created_at.as_str() + }, + ); + } + Ok(()) +} + +/// Builds (or rebuilds, with `--force`) either the default Docker sandbox +/// image or, with `--profile`, that profile's own `sandbox.image` / +/// `sandbox.dockerfile` — mutually exclusive targets, never both. A +/// profile's custom image is otherwise built automatically the first time +/// that profile actually runs (see `ensure_docker_sandbox_image_available`); +/// this command exists to do that ahead of time, or to force a refresh +/// (new apt packages, a security patch) without having to `docker rmi` it +/// by hand first. +pub async fn handle_docker_build_command( + command: AgentDockerBuildCommand, + global_config: &crate::models::config::Config, + raw_output: bool, + silent: bool, +) -> Result<()> { + if let Some(error) = crate::handlers::run::docker_sandbox::docker_enforcement_error() { + anyhow::bail!("Docker sandbox backend unavailable: {error}"); + } + + let source = match &command.profile { + None => crate::handlers::run::docker_sandbox::AgentImageSource::Default, + Some(profile_name) => { + let global_profile = || { + global_config + .agent_profiles + .as_ref() + .and_then(|profiles| profiles.get(profile_name)) + .cloned() + }; + let profile = match command.profile_source { + crate::cmd::agent::AgentProfileSource::Global => global_profile(), + crate::cmd::agent::AgentProfileSource::Directory => { + crate::config::config::get_directory_agent_profile(profile_name)? + .map(|loaded| loaded.profile) + } + crate::cmd::agent::AgentProfileSource::Auto => { + crate::config::config::get_directory_agent_profile(profile_name)? + .map(|loaded| loaded.profile) + .or_else(global_profile) + } + }; + let Some(profile) = profile else { + anyhow::bail!("Agent profile '{profile_name}' was not found."); + }; + crate::handlers::run::docker_sandbox::AgentImageSource::from_profile( + profile.sandbox.image.as_deref(), + profile.sandbox.dockerfile.as_deref(), + ) + } + }; + + if matches!( + source, + crate::handlers::run::docker_sandbox::AgentImageSource::Image(_) + ) { + if raw_output { + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ + "image": source.image_tag(), + "built": false, + "reason": "pre-built image reference; nothing to build", + }), + true, + )? + ); + } else if !silent { + println!( + "Profile '{}' uses a pre-built image reference ({}); nothing to build — `docker run` pulls it automatically if it isn't already present locally.", + command.profile.as_deref().unwrap_or_default(), + source.image_tag() + ); + } + return Ok(()); + } + + if !command.force && crate::handlers::run::docker_sandbox::sandbox_image_exists(&source) { + if raw_output { + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ + "image": source.image_tag(), + "built": false, + "reason": "already exists; pass --force to rebuild", + }), + true, + )? + ); + } else if !silent { + println!( + "Docker sandbox image ({}) already exists. Use --force to rebuild it.", + source.image_tag() + ); + } + return Ok(()); + } + if !silent && !raw_output { + println!("Building Docker sandbox image ({})...", source.image_tag()); + } + // `docker build`'s own progress streams straight to stdout regardless + // of --json (see build_sandbox_image) — there's no clean way to + // suppress it while still showing build progress, the same tradeoff + // `docker build` itself has. The JSON result below is still the last + // thing printed, so it remains the thing to parse. + crate::handlers::run::docker_sandbox::build_sandbox_image(&source) + .map_err(|error| anyhow::anyhow!("failed to build the Docker sandbox image: {error}"))?; + if raw_output { + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ "image": source.image_tag(), "built": true }), + true, + )? + ); + } else if !silent { + println!("{}", "Docker sandbox image built.".green_if_tty()); + } + Ok(()) +} + +/// Finds Docker sandbox networks (and their paired containers) left behind +/// by a run that didn't tear down cleanly, and removes them after +/// confirmation. +/// +/// A network still existing when nothing is actively using it is always a +/// leftover from a crash or `SIGKILL` — every normal exit path, including +/// Ctrl+C, tears its network down as part of `agent run` itself (see +/// `remove_run_network`). What this command *cannot* know on its own is +/// whether a network belongs to a run that's still genuinely in progress — +/// a local run in progress is cross-checked against this machine's tracked +/// sessions and skipped automatically, but a remote run has no local +/// artifact to check against at all. For that reason this command lists +/// what it found (including how long ago each network was created) and +/// asks for confirmation rather than deleting automatically — the person +/// running it is expected to recognize whether they have a run legitimately +/// in progress right now. +pub async fn handle_docker_cleanup_command( + command: AgentDockerCleanupCommand, + raw_output: bool, + silent: bool, +) -> Result<()> { + if let Some(error) = crate::handlers::run::docker_sandbox::docker_enforcement_error() { + anyhow::bail!("Docker sandbox backend unavailable: {error}"); + } + + let candidates: Vec<_> = list_networks_with_liveness()? + .into_iter() + .filter(|(_, live)| !live) + .map(|(network, _)| network) + .collect(); + + if candidates.is_empty() { + if raw_output { + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ "candidates": [], "removed": [] }), + true, + )? + ); + } else if !silent { + println!("No leftover Docker sandbox resources found."); + } + return Ok(()); + } + + // Human-readable prose only — JSON mode reports the same information + // structurally instead, so stdout stays parseable as a single object. + if !silent && !raw_output { + println!( + "Found {} leftover Docker sandbox network(s) not tied to a live local session:\n", + candidates.len() + ); + for network in &candidates { + println!( + " {} (created {})", + network.name, + if network.created_at.is_empty() { + "unknown" + } else { + network.created_at.as_str() + } + ); + } + println!( + "\nA remote run has no local record to check against, so a network here could \ + still belong to a remote session genuinely in progress right now — remove only \ + what you recognize as no longer running." + ); + } + + let should_remove = if command.yes { + true + } else if silent { + anyhow::bail!( + "{} leftover Docker sandbox network(s) found; re-run with --yes to remove them, or without --silent to be prompted", + candidates.len() + ); + } else { + let confirmed = crate::utils::interaction::confirm_opt(&format!( + "Remove all {} network(s) listed above?", + candidates.len() + )) + .unwrap_or(false); + let _ = dialoguer::console::Term::stdout().show_cursor(); + confirmed + }; + + if !should_remove { + if raw_output { + let candidate_names: Vec<_> = candidates.iter().map(|network| &network.name).collect(); + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ "candidates": candidate_names, "removed": [] }), + true, + )? + ); + } else if !silent { + println!("Nothing removed."); + } + return Ok(()); + } + + let mut removed = Vec::new(); + let mut already_removed = Vec::new(); + let mut failures = Vec::new(); + for network in &candidates { + let run_network = crate::handlers::run::docker_sandbox::DockerRunNetwork { + name: network.name.clone(), + gateway_ip: String::new(), + }; + match crate::handlers::run::docker_sandbox::remove_run_network(&run_network) { + Ok(()) => { + if !silent && !raw_output { + println!("{} {}", "Removed".green_if_tty(), network.name); + } + removed.push(&network.name); + } + // Benign race: something else (a concurrently finishing + // legitimate run, or a second `cleanup` invocation) already + // removed it between our listing and this removal attempt — + // the end state we wanted is already true. + Err(error) if error.contains("not found") => { + if !silent && !raw_output { + println!("Already removed: {}", network.name); + } + already_removed.push(&network.name); + } + Err(error) => failures.push(format!("{}: {error}", network.name)), + } + } + + if raw_output { + println!( + "{}", + get_formatted_json_string( + &serde_json::json!({ + "removed": removed, + "already_removed": already_removed, + "failed": failures, + }), + true, + )? + ); + } + + if !failures.is_empty() { + anyhow::bail!( + "failed to remove {} network(s):\n{}", + failures.len(), + failures.join("\n") + ); + } + + Ok(()) +} + +/// Checks whether the Docker sandbox backend can actually run on this +/// machine, without starting a real sandboxed run to find out. Reports +/// each check independently (the `docker` CLI on PATH, the daemon +/// reachable, its version, and whether the default image is already +/// built) rather than the single combined error message +/// `docker_enforcement_error` gives a real `agent run` failing closed, +/// since a standalone diagnostic is exactly where naming which part +/// failed is most useful. +pub async fn handle_docker_doctor_command( + _command: AgentDockerDoctorCommand, + raw_output: bool, +) -> Result { + let binary_available = crate::handlers::run::docker_sandbox::docker_binary_available(); + let daemon_version = if binary_available { + crate::handlers::run::docker_sandbox::docker_daemon_version() + } else { + Err("skipped: `docker` CLI not found".to_owned()) + }; + let image_built = crate::handlers::run::docker_sandbox::sandbox_image_exists( + &crate::handlers::run::docker_sandbox::AgentImageSource::Default, + ); + let all_ok = binary_available && daemon_version.is_ok(); + + if raw_output { + let json = serde_json::json!({ + "docker_cli_available": binary_available, + "daemon_reachable": daemon_version.is_ok(), + "daemon_version": daemon_version.as_ref().ok(), + "daemon_error": daemon_version.as_ref().err(), + "default_image_built": image_built, + "ready": all_ok, + }); + println!("{}", get_formatted_json_string(&json, true)?); + return Ok(!all_ok); + } + + println!( + "{} Docker CLI on PATH", + if binary_available { + "✓".green_if_tty() + } else { + "✗".red_if_tty() + } + ); + match &daemon_version { + Ok(version) => println!( + "{} Docker daemon reachable (server version {version})", + "✓".green_if_tty() + ), + Err(error) => println!("{} Docker daemon reachable: {error}", "✗".red_if_tty()), + } + println!( + "{} Default sandbox image built", + if image_built { + "✓".green_if_tty() + } else { + "○".yellow_if_tty() + } + ); + if !image_built { + println!( + " (not required — `agent run` builds it on first use, or run `agent docker build`)" + ); + } + println!(); + if all_ok { + println!( + "{}", + "Docker sandbox backend is ready to use.".green_if_tty() + ); + } else { + println!( + "{}", + "Docker sandbox backend is not ready — see the failed check(s) above.".red_if_tty() + ); + } + + Ok(!all_ok) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn network(session_id: Option<&str>) -> ExistingRunNetwork { + ExistingRunNetwork { + name: format!( + "stashbase-agent-run-{}", + session_id.unwrap_or("00000000-0000-0000-0000-000000000000") + ), + session_id: session_id.map(str::to_owned), + created_at: "2026-01-01T00:00:00Z".to_owned(), + } + } + + #[test] + fn skips_a_network_whose_session_is_still_tracked_locally() { + let local_sessions = HashSet::from(["ags_live".to_owned()]); + assert!(belongs_to_a_live_local_session( + &network(Some("ags_live")), + &local_sessions + )); + } + + #[test] + fn does_not_skip_a_network_with_no_matching_local_session() { + let local_sessions = HashSet::from(["ags_live".to_owned()]); + assert!(!belongs_to_a_live_local_session( + &network(Some("ags_dead")), + &local_sessions + )); + } + + #[test] + fn does_not_skip_a_network_with_no_session_id_at_all() { + // The pre-session-naming fallback (a bare UUID) never matches a + // local session id, so it's always treated as a candidate. + let local_sessions = HashSet::from(["ags_live".to_owned()]); + assert!(!belongs_to_a_live_local_session( + &network(None), + &local_sessions + )); + } + + #[test] + fn does_not_skip_anything_when_there_are_no_local_sessions_at_all() { + let local_sessions = HashSet::new(); + assert!(!belongs_to_a_live_local_session( + &network(Some("ags_anything")), + &local_sessions + )); + } +} diff --git a/src/handlers/agent_doctor.rs b/src/handlers/agent/doctor.rs similarity index 100% rename from src/handlers/agent_doctor.rs rename to src/handlers/agent/doctor.rs diff --git a/src/handlers/agent_explain.rs b/src/handlers/agent/explain.rs similarity index 99% rename from src/handlers/agent_explain.rs rename to src/handlers/agent/explain.rs index e427c514..cf9194de 100644 --- a/src/handlers/agent_explain.rs +++ b/src/handlers/agent/explain.rs @@ -9,11 +9,11 @@ use crate::{ cmd::agent::{AgentExplainCommand, AgentProfileSource}, config::config, handlers::{ - agent_policy::{ + agent::policy::{ configured_host_matches, evaluate_secret_authorization, matching_rule_indices, normalize_request_path, SecretAuthorizationDecision, SecretHttpPolicy, }, - agent_validate::ensure_profile_is_valid_for_run, + agent::validate::ensure_profile_is_valid_for_run, run::subprocess::filesystem_backend_for_policy, }, models::config::Config, diff --git a/src/handlers/agent_init.rs b/src/handlers/agent/init.rs similarity index 100% rename from src/handlers/agent_init.rs rename to src/handlers/agent/init.rs diff --git a/src/handlers/agent_mcp.rs b/src/handlers/agent/mcp.rs similarity index 98% rename from src/handlers/agent_mcp.rs rename to src/handlers/agent/mcp.rs index 1d7bb092..1bd0172b 100644 --- a/src/handlers/agent_mcp.rs +++ b/src/handlers/agent/mcp.rs @@ -26,8 +26,8 @@ use crate::{ }, config::config, handlers::{ - agent_policy::SecretHttpPolicy, - agent_profiles::{ + agent::policy::SecretHttpPolicy, + agent::profiles::{ profile_not_found_error, profile_not_found_error_with_output, source_label, }, entry::root::{provision_remote_session_ca, remote_bindings, remote_session_state}, @@ -676,6 +676,11 @@ async fn proxied_client( egress_hosts_configured: profile.egress_hosts.is_some(), strict_deny: true, mcp_rules: mcp_rules.clone(), + backend: profile.sandbox.backend, + sandbox_image: profile.sandbox.image.clone(), + sandbox_dockerfile: profile.sandbox.dockerfile.clone(), + sandbox_memory: profile.sandbox.memory.clone(), + sandbox_cpus: profile.sandbox.cpus.clone(), }; let proxy = Proxy::start_with_port(secrets, policy, None, None).await?; let proxy_url = proxy.child_env()["HTTPS_PROXY"].clone(); @@ -824,6 +829,11 @@ async fn remote_proxied_client( tools: rule.tools.clone(), }) .collect(), + backend: profile.sandbox.backend, + sandbox_image: profile.sandbox.image.clone(), + sandbox_dockerfile: profile.sandbox.dockerfile.clone(), + sandbox_memory: profile.sandbox.memory.clone(), + sandbox_cpus: profile.sandbox.cpus.clone(), }; let proxy = Proxy::start_remote_with_port( RemoteProxyConfig { diff --git a/src/handlers/agent/mod.rs b/src/handlers/agent/mod.rs new file mode 100644 index 00000000..9011f86a --- /dev/null +++ b/src/handlers/agent/mod.rs @@ -0,0 +1,10 @@ +pub mod docker; +pub mod doctor; +pub mod explain; +pub mod init; +pub mod mcp; +pub mod policy; +pub mod policy_test; +pub mod profiles; +pub mod sessions; +pub mod validate; diff --git a/src/handlers/agent_policy.rs b/src/handlers/agent/policy.rs similarity index 100% rename from src/handlers/agent_policy.rs rename to src/handlers/agent/policy.rs diff --git a/src/handlers/agent_policy_test.rs b/src/handlers/agent/policy_test.rs similarity index 99% rename from src/handlers/agent_policy_test.rs rename to src/handlers/agent/policy_test.rs index 7fdfaacf..ea24b6ae 100644 --- a/src/handlers/agent_policy_test.rs +++ b/src/handlers/agent/policy_test.rs @@ -9,11 +9,11 @@ use crate::{ cmd::agent::{AgentPolicyTestCommand, AgentProfileSource}, config::config, handlers::{ - agent_policy::{ + agent::policy::{ configured_host_matches, evaluate_secret_authorization, SecretAuthorizationDecision, SecretHttpPolicy, }, - agent_validate::ensure_profile_is_valid_for_run, + agent::validate::ensure_profile_is_valid_for_run, }, models::{ agent::{AgentPolicyTestCase, AgentPolicyTestExpectation, AgentProfile}, @@ -329,6 +329,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::from([( "GITHUB_TOKEN".to_owned(), diff --git a/src/handlers/agent_profiles.rs b/src/handlers/agent/profiles.rs similarity index 98% rename from src/handlers/agent_profiles.rs rename to src/handlers/agent/profiles.rs index 0af7c039..f2c323e3 100644 --- a/src/handlers/agent_profiles.rs +++ b/src/handlers/agent/profiles.rs @@ -12,7 +12,7 @@ use crate::{ AgentProfilesShowCommand, AgentProfilesSubcommand, }, config::config, - handlers::agent_policy::{normalize_secret_http_policy, SecretHttpPolicy}, + handlers::agent::policy::{normalize_secret_http_policy, SecretHttpPolicy}, models::{ agent::AgentProfile, config::Config, @@ -347,6 +347,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::from([( "GITHUB_TOKEN".to_owned(), @@ -390,6 +391,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::from([( "API_KEY".to_owned(), @@ -431,6 +433,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::from([("GITHUB_TOKEN".to_owned(), binding.clone())]).into(), personal_credentials: HashMap::from([("LINEAR_API_KEY".to_owned(), binding)]), diff --git a/src/handlers/agent_sessions.rs b/src/handlers/agent/sessions.rs similarity index 97% rename from src/handlers/agent_sessions.rs rename to src/handlers/agent/sessions.rs index 0dfe2053..10bd7f68 100644 --- a/src/handlers/agent_sessions.rs +++ b/src/handlers/agent/sessions.rs @@ -56,6 +56,16 @@ impl LocalAgentSessionGuard { pub fn path(&self) -> PathBuf { self.0.clone() } + + /// The session id this guard was started with, recovered from its + /// backing file's name (`.json`) rather than stored + /// separately, since the two must always agree. + pub fn session_id(&self) -> &str { + self.0 + .file_stem() + .and_then(|stem| stem.to_str()) + .unwrap_or_default() + } } impl Drop for LocalAgentSessionGuard { diff --git a/src/handlers/agent_validate.rs b/src/handlers/agent/validate.rs similarity index 83% rename from src/handlers/agent_validate.rs rename to src/handlers/agent/validate.rs index 8e1da9a1..8c4f6843 100644 --- a/src/handlers/agent_validate.rs +++ b/src/handlers/agent/validate.rs @@ -153,6 +153,27 @@ pub fn ensure_profile_is_valid_for_run(profile: &AgentProfile) -> Result<()> { fn validate_runtime_requirements(profile: &AgentProfile) -> Vec { let mut checks = Vec::new(); + + // A Docker-backend profile never touches the native + // Seatbelt/systemd-run/bubblewrap mechanisms the checks below test — + // it's contained by the Docker sandbox instead, which is checked + // separately. Reporting a native-backend failure for a profile that + // will never use the native backend is actively misleading: it's the + // difference between "this profile can't run here" (true) and "this + // profile can't run *natively* here" (irrelevant to a Docker-backend + // profile, and would wrongly report Windows as unsupported even though + // Docker Desktop makes this backend work there too). + if profile.sandbox.backend == crate::models::agent::SandboxBackend::Docker { + match crate::handlers::run::docker_sandbox::docker_enforcement_error() { + Some(error) => checks.push(fail("Docker sandbox backend", error)), + None => checks.push(ok( + "Docker sandbox backend", + "Docker is installed and the daemon is reachable.".to_owned(), + )), + } + return checks; + } + match crate::handlers::run::subprocess::network_enforcement_error() { Some(error) => checks.push(fail("Network enforcement", error)), None => checks.push(ok( @@ -363,6 +384,56 @@ fn validate_profile(profile: &AgentProfile) -> Vec { } } + if profile.sandbox.image.is_some() && profile.sandbox.dockerfile.is_some() { + checks.push(fail( + "Sandbox image", + "'sandbox.image' and 'sandbox.dockerfile' are mutually exclusive; set at most one." + .to_owned(), + )); + } + if let Some(image) = &profile.sandbox.image { + if image.trim().is_empty() { + checks.push(fail( + "Sandbox image", + "'sandbox.image' must not be empty or whitespace.".to_owned(), + )); + } + } + if let Some(dockerfile) = &profile.sandbox.dockerfile { + let path = Path::new(dockerfile); + if !path.is_file() { + checks.push(fail( + "Sandbox Dockerfile", + format!("File not found: {}", path.display()), + )); + } else { + checks.push(ok( + "Sandbox Dockerfile", + format!("Readable: {}", path.display()), + )); + } + } + if let Some(memory) = &profile.sandbox.memory { + if !valid_docker_memory_value(memory) { + checks.push(fail( + "Sandbox memory limit", + format!( + "'{memory}' is not a valid `docker run --memory` value (expected a positive number optionally suffixed with b/k/m/g, e.g. \"2g\")." + ), + )); + } + } + if let Some(cpus) = &profile.sandbox.cpus { + if !valid_docker_cpus_value(cpus) { + checks.push(fail( + "Sandbox CPU limit", + format!( + "'{cpus}' is not a valid `docker run --cpus` value (expected a positive number, e.g. \"1.5\")." + ), + )); + } + } + let mut bindings: HashMap<&str, Vec<&str>> = HashMap::new(); let mut child_envs: HashMap<&str, Vec<&str>> = HashMap::new(); let mut placeholders: HashMap<&str, Vec<&str>> = HashMap::new(); @@ -970,6 +1041,30 @@ fn fail(name: impl Into, message: String) -> Check { } } +/// A `docker run --memory` value: a positive number optionally suffixed +/// with a case-insensitive `b`/`k`/`m`/`g` unit (Docker's own accepted +/// format). Not an exhaustive re-implementation of Docker's own parser — +/// just enough to catch an obviously malformed value before it reaches +/// `docker run` and fails there instead. +fn valid_docker_memory_value(value: &str) -> bool { + let value = value.trim(); + let number_part = match value.chars().last() { + Some(suffix) if suffix.is_ascii_alphabetic() => { + if !matches!(suffix.to_ascii_lowercase(), 'b' | 'k' | 'm' | 'g') { + return false; + } + &value[..value.len() - 1] + } + _ => value, + }; + number_part.parse::().is_ok_and(|number| number > 0.0) +} + +/// A `docker run --cpus` value: a positive decimal number. +fn valid_docker_cpus_value(value: &str) -> bool { + value.trim().parse::().is_ok_and(|number| number > 0.0) +} + #[cfg(test)] mod tests { use super::*; @@ -982,6 +1077,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::new().into(), personal_credentials: HashMap::new(), @@ -998,6 +1094,145 @@ mod tests { .contains("Unsupported hook capability 'anything_else'")); } + #[test] + fn docker_backend_profile_gets_a_docker_runtime_check_not_native_ones() { + // A Docker-backend profile never touches Seatbelt/systemd-run/ + // bubblewrap, so it must not be reported as unsupported on a + // platform where only those native mechanisms are unavailable + // (e.g. Windows) — it should get a Docker-specific check instead. + let mut profile = AgentProfile { + file: None, + egress_hosts: None, + allow_network_listeners: false, + deny_hosts: None, + filesystem: Default::default(), + sandbox: Default::default(), + mcp_servers: HashMap::new(), + secrets: HashMap::new().into(), + personal_credentials: HashMap::new(), + policy_tests: Vec::new(), + allow_hooks: Vec::new(), + }; + profile.sandbox.backend = crate::models::agent::SandboxBackend::Docker; + + let checks = validate_runtime_requirements(&profile); + assert!(checks + .iter() + .any(|check| check.name == "Docker sandbox backend")); + assert!(!checks + .iter() + .any(|check| check.name == "Network enforcement")); + assert!(!checks + .iter() + .any(|check| check.name == "Filesystem enforcement")); + } + + #[test] + fn rejects_sandbox_image_and_dockerfile_set_together() { + let mut profile = AgentProfile { + file: None, + egress_hosts: None, + allow_network_listeners: false, + deny_hosts: None, + filesystem: Default::default(), + sandbox: Default::default(), + mcp_servers: HashMap::new(), + secrets: HashMap::new().into(), + personal_credentials: HashMap::new(), + policy_tests: Vec::new(), + allow_hooks: Vec::new(), + }; + profile.sandbox.image = Some("myorg/img:tag".to_owned()); + profile.sandbox.dockerfile = Some("./Cargo.toml".to_owned()); + + assert!(validate_profile(&profile) + .iter() + .any(|check| check.status == Status::Fail + && check.name == "Sandbox image" + && check.message.contains("mutually exclusive"))); + } + + #[test] + fn rejects_a_missing_sandbox_dockerfile() { + let mut profile = AgentProfile { + file: None, + egress_hosts: None, + allow_network_listeners: false, + deny_hosts: None, + filesystem: Default::default(), + sandbox: Default::default(), + mcp_servers: HashMap::new(), + secrets: HashMap::new().into(), + personal_credentials: HashMap::new(), + policy_tests: Vec::new(), + allow_hooks: Vec::new(), + }; + profile.sandbox.dockerfile = Some("./does-not-exist.Dockerfile".to_owned()); + + assert!(validate_profile(&profile) + .iter() + .any(|check| check.status == Status::Fail && check.name == "Sandbox Dockerfile")); + } + + #[test] + fn accepts_valid_docker_memory_and_cpu_values() { + for value in ["2g", "512m", "1024k", "1", "1.5"] { + assert!( + valid_docker_memory_value(value), + "expected '{value}' to be a valid memory value" + ); + } + for value in ["1", "1.5", "0.5", "4"] { + assert!( + valid_docker_cpus_value(value), + "expected '{value}' to be a valid cpus value" + ); + } + } + + #[test] + fn rejects_invalid_docker_memory_and_cpu_values() { + for value in ["", "abc", "2x", "-1g", "0g"] { + assert!( + !valid_docker_memory_value(value), + "expected '{value}' to be rejected as a memory value" + ); + } + for value in ["", "abc", "-1", "0"] { + assert!( + !valid_docker_cpus_value(value), + "expected '{value}' to be rejected as a cpus value" + ); + } + } + + #[test] + fn rejects_a_malformed_sandbox_memory_or_cpus_value() { + let mut profile = AgentProfile { + file: None, + egress_hosts: None, + allow_network_listeners: false, + deny_hosts: None, + filesystem: Default::default(), + sandbox: Default::default(), + mcp_servers: HashMap::new(), + secrets: HashMap::new().into(), + personal_credentials: HashMap::new(), + policy_tests: Vec::new(), + allow_hooks: Vec::new(), + }; + profile.sandbox.memory = Some("not-a-memory-value".to_owned()); + profile.sandbox.cpus = Some("not-a-number".to_owned()); + + let checks = validate_profile(&profile); + assert!(checks + .iter() + .any(|check| check.status == Status::Fail && check.name == "Sandbox memory limit")); + assert!(checks + .iter() + .any(|check| check.status == Status::Fail && check.name == "Sandbox CPU limit")); + } + #[test] fn accepts_exact_hosts_and_subdomain_wildcards() { assert!(validate_host("api.github.com", false).is_ok()); @@ -1075,6 +1310,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: crate::models::agent::AgentSecretsProfile { project: Some("project".to_owned()), @@ -1109,6 +1345,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: crate::models::agent::AgentSecretsProfile { project: Some("project".to_owned()), @@ -1144,6 +1381,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::new().into(), personal_credentials: HashMap::new(), @@ -1164,6 +1402,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: HashMap::new().into(), personal_credentials: HashMap::from([( @@ -1198,6 +1437,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: crate::models::agent::AgentSecretsProfile { project: Some("project".to_owned()), @@ -1257,6 +1497,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: crate::models::agent::AgentSecretsProfile { project: Some("project".to_owned()), diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index bb16ca1b..cf3ab153 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -24,14 +24,16 @@ use crate::{ }, config::{config, secure_store}, handlers::{ - agent_doctor::handle_agent_doctor_command, - agent_explain::handle_agent_explain_command, - agent_init::handle_agent_init_command, - agent_mcp::{handle_agent_mcp_configure_command, handle_agent_mcp_tools_command}, - agent_policy::SecretHttpPolicy, - agent_policy_test::handle_agent_policy_test_command, - agent_profiles::handle_agent_profiles_command, - agent_validate::handle_agent_validate_command, + agent::{ + doctor::handle_agent_doctor_command, + explain::handle_agent_explain_command, + init::handle_agent_init_command, + mcp::{handle_agent_mcp_configure_command, handle_agent_mcp_tools_command}, + policy::SecretHttpPolicy, + policy_test::handle_agent_policy_test_command, + profiles::handle_agent_profiles_command, + validate::handle_agent_validate_command, + }, doctor::handle_doctor_command, entry::{ auth::{handle_whoami_command, GetCurrentAuthDetailsRequestArgs}, @@ -564,7 +566,7 @@ pub async fn handle_cli(args: Cli) { AgentSubcommand::Sessions { command: crate::cmd::agent::AgentSessionsSubcommand::List(command), } => { - crate::handlers::agent_sessions::handle_sessions( + crate::handlers::agent::sessions::handle_sessions( command, &api_key, raw_output, silent, ) .await @@ -572,11 +574,42 @@ pub async fn handle_cli(args: Cli) { AgentSubcommand::Sessions { command: crate::cmd::agent::AgentSessionsSubcommand::Revoke(command), } => { - crate::handlers::agent_sessions::handle_revoke( + crate::handlers::agent::sessions::handle_revoke( command, &api_key, raw_output, silent, ) .await } + AgentSubcommand::Docker(agent_docker) => match agent_docker.subcommand { + crate::cmd::agent::AgentDockerSubcommand::Cleanup(command) => { + crate::handlers::agent::docker::handle_docker_cleanup_command( + command, raw_output, silent, + ) + .await + } + crate::cmd::agent::AgentDockerSubcommand::Status(command) => { + crate::handlers::agent::docker::handle_docker_status_command( + command, raw_output, + ) + .await + } + crate::cmd::agent::AgentDockerSubcommand::Build(command) => { + crate::handlers::agent::docker::handle_docker_build_command( + command, &config, raw_output, silent, + ) + .await + } + crate::cmd::agent::AgentDockerSubcommand::Doctor(command) => { + match crate::handlers::agent::docker::handle_docker_doctor_command( + command, raw_output, + ) + .await + { + Ok(true) => std::process::exit(1), + Ok(false) => Ok(()), + Err(error) => Err(error), + } + } + }, AgentSubcommand::Logs(mut agent_logs) => match agent_logs.subcommand.take() { Some(AgentLogsSubcommand::List(list)) => { handle_agent_logs(list.into(), raw_output).await @@ -601,7 +634,7 @@ pub async fn handle_cli(args: Cli) { handle_agent_mcp_configure_command(command, &config, Some(api_key.as_str()), silent).await } crate::cmd::agent::AgentMcpSubcommand::Check(command) => { - match crate::handlers::agent_mcp::handle_agent_mcp_check_command( + match crate::handlers::agent::mcp::handle_agent_mcp_check_command( command, &config, raw_output, @@ -612,7 +645,7 @@ pub async fn handle_cli(args: Cli) { } } crate::cmd::agent::AgentMcpSubcommand::Verify(command) => { - match crate::handlers::agent_mcp::handle_agent_mcp_verify_command( + match crate::handlers::agent::mcp::handle_agent_mcp_verify_command( command, &config, raw_output, @@ -631,7 +664,7 @@ pub async fn handle_cli(args: Cli) { handle_agent_mcp_tools_command(agent_mcp, &config, Some(api_key.as_str()), raw_output, silent).await } AgentSubcommand::McpCheck(agent_mcp) => { - match crate::handlers::agent_mcp::handle_agent_mcp_check_command( + match crate::handlers::agent::mcp::handle_agent_mcp_check_command( agent_mcp, &config, raw_output, @@ -711,7 +744,7 @@ pub async fn handle_cli(args: Cli) { }}; let loaded_from_directory = directory_source.is_some(); - let Some(profile) = profile else { + let Some(mut profile) = profile else { let source = match agent_run.profile_source { AgentProfileSource::Global => "global", AgentProfileSource::Directory => "directory", @@ -724,7 +757,24 @@ pub async fn handle_cli(args: Cli) { return Ok(()); }; - crate::handlers::agent_validate::ensure_profile_is_valid_for_run(&profile)?; + profile.sandbox.backend = + profile.sandbox.backend.with_cli_override(agent_run.docker_sandbox); + if let Some(image) = &agent_run.docker_image { + profile.sandbox.image = Some(image.clone()); + profile.sandbox.dockerfile = None; + profile.sandbox.backend = crate::models::agent::SandboxBackend::Docker; + } else if let Some(dockerfile) = &agent_run.docker_dockerfile { + profile.sandbox.dockerfile = Some(dockerfile.clone()); + profile.sandbox.image = None; + profile.sandbox.backend = crate::models::agent::SandboxBackend::Docker; + } + if let Some(memory) = &agent_run.docker_memory { + profile.sandbox.memory = Some(memory.clone()); + } + if let Some(cpus) = &agent_run.docker_cpus { + profile.sandbox.cpus = Some(cpus.clone()); + } + crate::handlers::agent::validate::ensure_profile_is_valid_for_run(&profile)?; // Egress policy is meaningful only when the child cannot opt out of // its proxy environment. Contain every session to the loopback // proxy, including remote sessions, so `env -u HTTPS_PROXY …` is @@ -761,6 +811,11 @@ pub async fn handle_cli(args: Cli) { let dependency_hooks = dependency_hooks_enabled(&profile, &api_key); if !silent { eprintln!("Network sandbox: enabled"); + if profile.sandbox.backend + == crate::models::agent::SandboxBackend::Docker + { + eprintln!("Sandbox backend: Docker (container-isolated)"); + } print_agent_egress_warnings(&profile); eprintln!( "API hook broker: {}", @@ -913,6 +968,11 @@ pub async fn handle_cli(args: Cli) { egress_hosts_configured: profile.egress_hosts.is_some(), strict_deny: true, mcp_rules: compiled_mcp_rules(&profile), + backend: profile.sandbox.backend, + sandbox_image: profile.sandbox.image.clone(), + sandbox_dockerfile: profile.sandbox.dockerfile.clone(), + sandbox_memory: profile.sandbox.memory.clone(), + sandbox_cpus: profile.sandbox.cpus.clone(), }; let policy_fingerprint = policy.fingerprint(); let profile_source = directory_source @@ -1128,7 +1188,7 @@ pub async fn handle_cli(args: Cli) { let local_session = if agent_run.remote { None } else { - Some(crate::handlers::agent_sessions::LocalAgentSessionGuard::start( + Some(crate::handlers::agent::sessions::LocalAgentSessionGuard::start( local_session_id.clone(), )?) }; @@ -2225,6 +2285,7 @@ mod tests { allow_network_listeners: false, deny_hosts: None, filesystem: Default::default(), + sandbox: Default::default(), mcp_servers: HashMap::new(), secrets: AgentSecretsProfile { project: Some("project".to_owned()), diff --git a/src/handlers/mod.rs b/src/handlers/mod.rs index 60ffdbdc..f47b37d6 100644 --- a/src/handlers/mod.rs +++ b/src/handlers/mod.rs @@ -1,12 +1,4 @@ -pub mod agent_doctor; -pub mod agent_explain; -pub mod agent_init; -pub mod agent_mcp; -pub mod agent_policy; -pub mod agent_policy_test; -pub mod agent_profiles; -pub mod agent_sessions; -pub mod agent_validate; +pub mod agent; pub mod config; pub mod doctor; pub mod entry; diff --git a/src/handlers/run/docker_sandbox.rs b/src/handlers/run/docker_sandbox.rs new file mode 100644 index 00000000..dd936c94 --- /dev/null +++ b/src/handlers/run/docker_sandbox.rs @@ -0,0 +1,2129 @@ +use std::path::PathBuf; + +pub(crate) fn docker_binary_available() -> bool { + std::env::var_os("PATH") + .is_some_and(|path| std::env::split_paths(&path).any(|dir| dir.join("docker").is_file())) +} + +/// Checks whether the Docker daemon is reachable, returning its reported +/// server version on success (used by `agent docker doctor` to show what +/// version is actually running, not just that a check passed). +pub(crate) fn docker_daemon_version() -> Result { + let output = std::process::Command::new("docker") + .args(["info", "--format", "{{.ServerVersion}}"]) + .output() + .map_err(|error| format!("failed to run `docker info`: {error}"))?; + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).trim().to_owned()) + } else { + Err(String::from_utf8_lossy(&output.stderr).trim().to_owned()) + } +} + +/// Checks whether the Docker sandbox backend can run here. Returns `None` +/// when Docker is installed and the daemon is reachable, `Some(message)` +/// otherwise. Callers must fail the run closed on `Some` — never fall back +/// to an unsandboxed execution path. +pub(crate) fn docker_enforcement_error() -> Option { + if !docker_binary_available() { + return Some( + "the Docker sandbox backend requires the `docker` CLI to be installed and on PATH" + .to_owned(), + ); + } + match docker_daemon_version() { + Ok(_) => None, + Err(detail) => Some(format!( + "the Docker sandbox backend requires a reachable Docker daemon: {detail}" + )), + } +} + +/// Named after the run's own session id (the same `ags_...` id shown in +/// "Agent session"/"Audit session" and used for the audit log filename) so +/// a stray container or network left behind after a crash can be traced +/// back to the session that created it, instead of an unrelated random +/// UUID. Falls back to a fresh UUID only when no session id is available +/// (e.g. audit logging disabled and no local session guard, which +/// shouldn't happen in practice for either the local or remote run paths). +fn generate_run_network_name(session_id: Option<&str>) -> String { + let suffix = session_id + .filter(|id| !id.is_empty()) + .map(str::to_owned) + .unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); + format!("stashbase-agent-run-{suffix}") +} + +#[derive(Debug, Clone)] +pub(crate) struct DockerRunNetwork { + pub name: String, + pub gateway_ip: String, +} + +fn extract_gateway_from_inspect(json: &str) -> Result { + let parsed: serde_json::Value = serde_json::from_str(json) + .map_err(|error| format!("invalid `docker network inspect` output: {error}"))?; + parsed + .get(0) + .and_then(|network| network.get("IPAM")) + .and_then(|ipam| ipam.get("Config")) + .and_then(|config| config.get(0)) + .and_then(|entry| entry.get("Gateway")) + .and_then(|gateway| gateway.as_str()) + .map(|gateway| gateway.to_owned()) + .ok_or_else(|| "`docker network inspect` did not report a gateway address".to_owned()) +} + +/// Creates a fresh, isolated Docker bridge network for one `agent run` +/// invocation. The credential proxy binds to the returned gateway address +/// so only the container attached to this network can reach it. +/// +/// On native Linux this network is created `--internal`: Docker drops the +/// outbound NAT/masquerade rule that would otherwise let the container +/// reach the wider internet or LAN directly, while containers can still +/// reach the network's own gateway address (a directly-attached bridge +/// interface, not a routed hop) — so the proxy stays reachable. A plain +/// `--driver bridge` network without this flag gives the container full +/// internet/LAN egress no different from running on the host's own +/// network, which would defeat the point of a Docker-specific backend. +/// +/// `--internal` is Linux-only here because it also blocks the +/// `host.docker.internal` route Docker Desktop (macOS/Windows) uses to let +/// a container reach the host proxy at all (see `proxy_bind_host`) — on +/// Desktop this backend currently cannot offer kernel-enforced network +/// containment beyond what `HTTPS_PROXY`/`HTTP_PROXY` convention already +/// gives the native backend, and `docs/agent-profiles.md` says so. +fn network_create_args(name: &str) -> Vec { + let mut args = vec![ + "network".to_owned(), + "create".to_owned(), + "--driver".to_owned(), + "bridge".to_owned(), + ]; + if cfg!(target_os = "linux") { + args.push("--internal".to_owned()); + } + args.push(name.to_owned()); + args +} + +pub(crate) fn create_run_network(session_id: Option<&str>) -> Result { + let name = generate_run_network_name(session_id); + let create = std::process::Command::new("docker") + .args(network_create_args(&name)) + .output() + .map_err(|error| format!("failed to run `docker network create`: {error}"))?; + if !create.status.success() { + return Err(String::from_utf8_lossy(&create.stderr).trim().to_owned()); + } + let inspect = std::process::Command::new("docker") + .args(["network", "inspect", &name]) + .output() + .map_err(|error| format!("failed to run `docker network inspect`: {error}"))?; + if !inspect.status.success() { + let _ = remove_run_network(&DockerRunNetwork { + name: name.clone(), + gateway_ip: String::new(), + }); + return Err(String::from_utf8_lossy(&inspect.stderr).trim().to_owned()); + } + let gateway_ip = extract_gateway_from_inspect(&String::from_utf8_lossy(&inspect.stdout)) + .inspect_err(|_| { + let _ = remove_run_network(&DockerRunNetwork { + name: name.clone(), + gateway_ip: String::new(), + }); + })?; + Ok(DockerRunNetwork { name, gateway_ip }) +} + +/// Name of the short-lived helper container that holds the network +/// namespace the agent container joins (see `start_netns_holder`). +/// Deterministic from the network name so no extra state needs to be +/// threaded through the run — every caller that needs it (starting it, +/// joining it, tearing it down) can derive it the same way. +fn netns_holder_name(network: &DockerRunNetwork) -> String { + format!("{}-netns-holder", network.name) +} + +fn parse_proxy_host_port( + env_vars: &std::collections::HashMap, +) -> Option<(String, String)> { + let proxy_url = env_vars + .get("HTTPS_PROXY") + .or_else(|| env_vars.get("HTTP_PROXY"))?; + let hostport = proxy_url.split("://").nth(1)?; + let host = hostport.split(&[':', '/'][..]).next()?; + let port = hostport.split(':').nth(1)?.split('/').next()?; + Some((host.to_owned(), port.to_owned())) +} + +/// Starts a short-lived helper container attached to `network` that holds +/// `CAP_NET_ADMIN` just long enough to install one `iptables` rule +/// (default-DROP outbound, exceptions only for loopback and the +/// credential proxy's specific address/port), then blocks forever holding +/// the network namespace open. The actual agent container later joins this +/// exact namespace via `--network container:` (see +/// `docker_run_command`) with zero added capabilities of its own — network +/// namespace rules are shared by anything attached to that namespace, but +/// the *capability* to modify them is not, so the agent process can use +/// the firewall but never touch it. +/// +/// This exists because `setpriv`/capability-bounding-set tricks to strip +/// `NET_ADMIN` from the agent container itself after setup turned out not +/// to work on Docker Desktop: granting the container `CAP_SETPCAP` (needed +/// to modify its own bounding set at all) is silently zeroed out there — +/// confirmed directly, not assumed. Splitting privileged setup into a +/// separate container sidesteps that limitation entirely: the agent +/// container never needs `CAP_SETPCAP`, `CAP_NET_ADMIN`, or root, on any +/// platform. +/// +/// Returns the proxy's resolved IP address when a proxy was configured, so +/// the caller can point the agent container directly at that IP instead of +/// a hostname — the agent joins this namespace via +/// `--network container:`, which is incompatible with `--add-host` +/// (Docker rejects the combination outright), so the agent container has +/// no way to resolve `host.docker.internal` itself. Using the +/// already-resolved IP sidesteps needing DNS/hosts resolution in the agent +/// container at all. +pub(crate) fn start_netns_holder( + network: &DockerRunNetwork, + proxy_env_vars: &std::collections::HashMap, +) -> Result, String> { + let holder_name = netns_holder_name(network); + let start = std::process::Command::new("docker") + .args([ + "run", + "-d", + "--rm", + "--name", + &holder_name, + "--network", + &network.name, + "--add-host", + "host.docker.internal:host-gateway", + // Docker's embedded DNS resolver (127.0.0.11) forwards + // unresolved lookups to a real upstream server on the host + // side — outside this container's own network namespace + // entirely, so no iptables OUTPUT rule inside the container can + // ever see or block that traffic (confirmed directly: even + // with a default-DROP policy and no ACCEPT rule for port 53 at + // all, a lookup for an arbitrary external hostname still + // succeeded). That's a live data-exfiltration channel — an + // agent can encode secrets in a query name to a + // domain it controls and have Docker itself relay it out. + // Pointing the resolver's upstream at a blackhole address + // closes it: local lookups (`host.docker.internal` via the + // `--add-host` above) still work since those resolve from + // `/etc/hosts`, never touching the upstream forwarder at all. + // The agent joins this container's network namespace and + // inherits this same DNS config — it doesn't need real DNS + // either, since its proxy address is already a resolved raw IP + // (see `rewrite_proxy_urls_for_container`). + "--dns", + "0.0.0.0", + "--cap-drop", + "ALL", + "--cap-add", + "NET_ADMIN", + "--security-opt", + "no-new-privileges", + DEFAULT_SANDBOX_IMAGE, + "sleep", + "infinity", + ]) + .output() + .map_err(|error| { + format!("failed to run `docker run` for the network namespace holder: {error}") + })?; + if !start.status.success() { + return Err(String::from_utf8_lossy(&start.stderr).trim().to_owned()); + } + + let Some((proxy_host, proxy_port)) = parse_proxy_host_port(proxy_env_vars) else { + // No proxy configured for this run — leave the holder's network + // namespace at Docker's default (unrestricted) rather than + // guessing at a policy. The Docker backend always runs with the + // proxy in practice; this is a defensive fallback, not the normal + // path. + return Ok(None); + }; + let setup_script = format!( + "set -e\n\ + proxy_ip=$(getent hosts '{proxy_host}' 2>/dev/null | awk '{{print $1}}' | head -1)\n\ + if [ -z \"$proxy_ip\" ]; then proxy_ip='{proxy_host}'; fi\n\ + iptables -P OUTPUT DROP\n\ + iptables -A OUTPUT -o lo -j ACCEPT\n\ + iptables -A OUTPUT -d \"$proxy_ip\" -p tcp --dport '{proxy_port}' -j ACCEPT\n\ + \n\ + # Verify the rule actually took effect before trusting it, rather\n\ + # than assuming `iptables` exiting 0 means the running kernel\n\ + # honored it (inspired by Anthropic's own Claude Code devcontainer\n\ + # firewall script, which does the same kind of self-check). A\n\ + # known-arbitrary host must be unreachable, and the proxy itself\n\ + # must still be reachable — either failing means this run is not\n\ + # actually contained and must not proceed.\n\ + set +e\n\ + # 192.0.2.1 is TEST-NET-1 (RFC 5737) — reserved for documentation\n\ + # and testing, never routed on the real internet. Used here purely\n\ + # as an arbitrary destination the firewall must never let through,\n\ + # deliberately not a live third party's real IP (e.g. a public DNS\n\ + # resolver), so this check's correctness never depends on what\n\ + # that company's infrastructure happens to be doing right now.\n\ + #\n\ + # DROP (vs. REJECT) means a blocked connection gets no response at\n\ + # all, so this waits out its own timeout on the success path (the\n\ + # firewall is working) — that wait is pure per-run startup latency,\n\ + # so keep it as short as still reliably catches a real leak. A SYN\n\ + # to a genuinely reachable host resolves in low tens of ms even over\n\ + # the public internet, let alone from a container's own network\n\ + # stack, so 300ms leaves ample margin above any real response time\n\ + # while capping the wasted wait on the (expected) blocked outcome.\n\ + curl -s -m 0.3 -o /dev/null 'http://192.0.2.1/'\n\ + arbitrary_reachable=$?\n\ + curl -s -m 3 -o /dev/null \"http://$proxy_ip:{proxy_port}/\"\n\ + proxy_reachable=$?\n\ + set -e\n\ + \n\ + if [ \"$arbitrary_reachable\" -eq 0 ]; then\n\ + echo 'firewall verification failed: an arbitrary external host was reachable' >&2\n\ + exit 1\n\ + fi\n\ + # curl exit 7 = couldn't connect, 28 = timeout — both mean the\n\ + # proxy's own ACCEPT rule didn't take effect. Any other non-zero\n\ + # exit (e.g. a protocol complaint about a non-HTTP response from\n\ + # the forward-proxy port) still proves the TCP connection itself\n\ + # succeeded, which is all this check needs.\n\ + if [ \"$proxy_reachable\" -eq 7 ] || [ \"$proxy_reachable\" -eq 28 ]; then\n\ + echo \"firewall verification failed: proxy unreachable (curl exit $proxy_reachable)\" >&2\n\ + exit 1\n\ + fi\n\ + \n\ + echo \"$proxy_ip\"\n" + ); + // `docker exec` immediately after `docker run -d` can race the + // container's own network setup (DNS in particular isn't always ready + // the instant the process starts) — retry briefly rather than treat a + // transient race as a hard failure. + let mut last_error = String::new(); + for attempt in 0..5 { + if attempt > 0 { + std::thread::sleep(std::time::Duration::from_millis(200)); + } + let setup = std::process::Command::new("docker") + .args(["exec", &holder_name, "sh", "-c", &setup_script]) + .output() + .map_err(|error| { + format!("failed to run the network namespace holder's firewall setup: {error}") + })?; + if setup.status.success() { + let resolved_ip = String::from_utf8_lossy(&setup.stdout).trim().to_owned(); + return Ok(Some(resolved_ip)); + } + last_error = String::from_utf8_lossy(&setup.stderr).trim().to_owned(); + } + stop_container_if_running(&holder_name); + Err(last_error) +} + +/// Stops and removes the container for this run, if it's still around. The +/// agent container shares its name with the network (`docker_run_command` +/// passes `--name network.name`), so no separate identifier needs to be +/// tracked; the holder's name is derived the same deterministic way (see +/// `netns_holder_name`). Best-effort: a container that already exited and +/// self-removed (the common case — both containers run with `--rm`) has +/// nothing to stop, which is not an error. +/// +/// Uses `docker rm -f` rather than `docker stop`: both containers run with +/// `--rm`, so `stop` alone only *starts* the daemon's asynchronous +/// self-removal — it does not wait for the container to actually be gone, +/// which left a real race where `docker network rm` (called right after) +/// could still see the container's endpoint as attached and fail with +/// "has active endpoints". `rm -f` stops and removes synchronously in one +/// call, so by the time this returns the container and its network +/// attachment are actually gone. +fn stop_container_if_running(name: &str) { + let _ = std::process::Command::new("docker") + .args(["rm", "-f", name]) + .output(); +} + +/// Force-disconnects a container from a network, ignoring errors (the +/// common case is the container is already gone, in which case there's +/// nothing to disconnect). +fn force_disconnect(network_name: &str, container_name: &str) { + let _ = std::process::Command::new("docker") + .args(["network", "disconnect", "-f", network_name, container_name]) + .output(); +} + +/// Removes a per-run network created by `create_run_network`. Best-effort +/// overall: failure here should not mask the underlying run's exit status +/// — callers should log and continue. +/// +/// Both the agent and holder containers are stopped/removed first (see +/// `stop_container_if_running`), and the disconnect+remove sequence is +/// retried a few times with a short delay — confirmed necessary, not just +/// defensive: this Docker setup was observed leaving a network's own +/// bookkeeping pointing at a holder container's endpoint as still "active" +/// immediately after that container was already fully removed (`docker +/// inspect` on it returned "no such object"), causing both a bare `docker +/// network rm` *and* an immediate `disconnect` + `rm` attempt right after +/// removal to fail with "has active endpoints" — the same disconnect+rm +/// sequence reliably succeeds once retried a moment later, once the +/// daemon's own bookkeeping has caught up with the removal it already +/// performed. +pub(crate) fn remove_run_network(network: &DockerRunNetwork) -> Result<(), String> { + stop_container_if_running(&network.name); + stop_container_if_running(&netns_holder_name(network)); + + let mut last_error = String::new(); + for attempt in 0..10 { + if attempt > 0 { + std::thread::sleep(std::time::Duration::from_millis(300)); + } + force_disconnect(&network.name, &network.name); + force_disconnect(&network.name, &netns_holder_name(network)); + let output = std::process::Command::new("docker") + .args(["network", "rm", &network.name]) + .output() + .map_err(|error| format!("failed to run `docker network rm`: {error}"))?; + if output.status.success() { + return Ok(()); + } + last_error = String::from_utf8_lossy(&output.stderr).trim().to_owned(); + } + Err(last_error) +} + +/// Prefix shared by every per-run Docker network this backend creates — +/// used both to name new networks and to find existing ones left behind by +/// a run that didn't tear down cleanly (a crash or `SIGKILL` of the +/// `stashbase` process itself, which no normal exit path — including +/// Ctrl+C — leaves behind). +const RUN_NETWORK_NAME_PREFIX: &str = "stashbase-agent-run-"; + +/// A per-run Docker network still present on this machine, found by name +/// rather than tracked in memory (this process may not be the one that +/// created it) — see `list_run_networks`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ExistingRunNetwork { + pub name: String, + /// The session id embedded in the network's name (`ags_...`), or + /// `None` for the pre-session-naming fallback (a bare UUID) — see + /// `generate_run_network_name`. + pub session_id: Option, + /// RFC 3339 creation timestamp, straight from `docker network inspect`. + pub created_at: String, +} + +/// Lists every Docker network this backend has ever created that still +/// exists, regardless of which process (or machine session) created it. +/// Used by `agent docker cleanup` to find networks a crashed run left +/// behind; deliberately does not attempt to guess which ones are still +/// legitimately in use by a live run elsewhere — that judgment is left to +/// the caller (comparing against locally tracked sessions, prompting the +/// user, etc.), since this function has no way to know about a live +/// *remote* run's session at all. +pub(crate) fn list_run_networks() -> Result, String> { + let list = std::process::Command::new("docker") + .args([ + "network", + "ls", + "--filter", + &format!("name={RUN_NETWORK_NAME_PREFIX}"), + "--format", + "{{.Name}}", + ]) + .output() + .map_err(|error| format!("failed to run `docker network ls`: {error}"))?; + if !list.status.success() { + return Err(String::from_utf8_lossy(&list.stderr).trim().to_owned()); + } + String::from_utf8_lossy(&list.stdout) + .lines() + .map(str::trim) + .filter(|name| !name.is_empty()) + // `docker network ls --filter name=X` matches X anywhere in the + // name, not just as a prefix — filter again to be exact. + .filter(|name| name.starts_with(RUN_NETWORK_NAME_PREFIX)) + .map(|name| { + let inspect = std::process::Command::new("docker") + .args(["network", "inspect", name, "--format", "{{.Created}}"]) + .output() + .map_err(|error| format!("failed to run `docker network inspect`: {error}"))?; + let created_at = if inspect.status.success() { + String::from_utf8_lossy(&inspect.stdout).trim().to_owned() + } else { + // The network could have been removed between the `ls` and + // this `inspect` (e.g. a concurrent run finishing normally) + // — report it as unknown rather than failing the whole + // listing over a race that isn't this caller's problem. + String::new() + }; + let session_id = name + .strip_prefix(RUN_NETWORK_NAME_PREFIX) + .filter(|id| !id.is_empty()) + .map(str::to_owned); + Ok(ExistingRunNetwork { + name: name.to_owned(), + session_id, + created_at, + }) + }) + .collect() +} + +/// The host address the credential proxy should bind to for this Docker +/// run. On Docker Desktop (macOS/Windows), containers run inside a VM and +/// cannot reach a per-run bridge network's gateway address from the host +/// side — the host process cannot even bind to it (`docker network +/// inspect`'s gateway is only routable inside the Desktop VM). Loopback +/// plus the `host.docker.internal` hostname (which Docker Desktop resolves +/// back to the host) is the supported bridge there. On native Linux +/// Docker, the bridge network's gateway is a real host interface, so +/// binding to it directly keeps the proxy reachable only from this run's +/// isolated network rather than every interface on the host. +pub(crate) fn proxy_bind_host(network: &DockerRunNetwork) -> String { + if cfg!(target_os = "macos") || cfg!(target_os = "windows") { + "127.0.0.1".to_owned() + } else { + network.gateway_ip.clone() + } +} + +/// The host the *container* should use to reach the proxy bound via +/// `proxy_bind_host`. See that function's doc comment for why this differs +/// by platform. +pub(crate) fn proxy_container_host(network: &DockerRunNetwork) -> String { + if cfg!(target_os = "macos") || cfg!(target_os = "windows") { + "host.docker.internal".to_owned() + } else { + network.gateway_ip.clone() + } +} + +/// Env vars whose value is a `http://:[/path]` proxy URL +/// that the container needs to reach at a different host than the proxy +/// actually bound to (see `proxy_bind_host`'s doc comment). +const PROXY_URL_ENV_KEYS: &[&str] = &[ + "HTTP_PROXY", + "HTTPS_PROXY", + "http_proxy", + "https_proxy", + crate::api::dependencies::HOOK_BROKER_URL_ENV, +]; + +/// Rewrites the proxy's child-process env vars so the container reaches the +/// proxy at `container_host` instead of whatever host the proxy actually +/// bound to (`bind_host`) — the two differ on Docker Desktop. Only known +/// proxy-URL keys are rewritten; opaque secret placeholders are left +/// untouched even if they happen to contain the bind host as a substring. +pub(crate) fn rewrite_proxy_urls_for_container( + env_vars: &std::collections::HashMap, + bind_host: &str, + container_host: &str, +) -> std::collections::HashMap { + if bind_host == container_host { + return env_vars.clone(); + } + env_vars + .iter() + .map(|(key, value)| { + if PROXY_URL_ENV_KEYS.contains(&key.as_str()) { + (key.clone(), value.replacen(bind_host, container_host, 1)) + } else { + (key.clone(), value.clone()) + } + }) + .collect() +} + +pub(crate) const DEFAULT_SANDBOX_IMAGE: &str = "stashbase/agent-sandbox:latest"; + +/// Named Docker volume holding the sandboxed agent's persistent home +/// directory (login state, config) across runs. Shared by every profile +/// and every run on this machine — see `docker_run_command`'s doc comment. +const PERSISTENT_HOME_VOLUME: &str = "stashbase-agent-home"; + +/// The container-side path `PERSISTENT_HOME_VOLUME` is mounted at, and the +/// `HOME` the sandboxed process runs with. Fixed rather than derived from +/// the host's own home directory: on Linux the container may run under an +/// arbitrary `--user uid:gid` with no passwd entry, so there's no +/// meaningful host-equivalent path to mirror. +const CONTAINER_HOME: &str = "/home/agent"; + +/// The default sandbox image's Dockerfile, embedded at compile time so an +/// installed `stashbase` binary can build the image itself without needing +/// this source repository on disk or a registry to pull from (neither +/// exists yet for this image). +const SANDBOX_DOCKERFILE: &str = include_str!("../../../docker/agent-sandbox/Dockerfile"); + +/// Where the agent container's image comes from for a given run. The +/// network-namespace holder (privileged, holds `NET_ADMIN`) always uses +/// `DEFAULT_SANDBOX_IMAGE` regardless of this choice — a custom image must +/// never run with elevated capabilities, only the unprivileged agent +/// container it's paired with. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum AgentImageSource { + /// The built-in image, built from the embedded Dockerfile. + Default, + /// A pre-built image reference the caller is responsible for (`docker + /// run` pulls it automatically if not already present locally). + Image(String), + /// A local Dockerfile to build. Tagged deterministically from its + /// canonicalized path so repeated runs reuse the same build instead of + /// rebuilding every time. + Dockerfile(PathBuf), +} + +impl AgentImageSource { + /// Resolves an `AgentSandboxProfile`'s `image`/`dockerfile` fields (the + /// two are mutually exclusive — enforced separately at profile + /// validation time) into a concrete image source. + pub(crate) fn from_profile(image: Option<&str>, dockerfile: Option<&str>) -> AgentImageSource { + if let Some(image) = image { + AgentImageSource::Image(image.to_owned()) + } else if let Some(dockerfile) = dockerfile { + AgentImageSource::Dockerfile(PathBuf::from(dockerfile)) + } else { + AgentImageSource::Default + } + } + + /// The tag this source's image is built/referenced under. Only + /// meaningful for `Default`/`Dockerfile` (build targets); `Image` + /// already names its own reference directly. + fn build_tag(&self) -> Option { + match self { + AgentImageSource::Default => Some(DEFAULT_SANDBOX_IMAGE.to_owned()), + AgentImageSource::Dockerfile(path) => { + use sha2::{Digest, Sha256}; + let canonical = std::fs::canonicalize(path).unwrap_or_else(|_| path.clone()); + let digest = Sha256::digest(canonical.to_string_lossy().as_bytes()); + Some(format!( + "stashbase/agent-sandbox-custom:{}", + &hex::encode(digest)[..16] + )) + } + AgentImageSource::Image(_) => None, + } + } + + /// The image reference `docker run` should use for the agent container. + pub(crate) fn image_tag(&self) -> String { + match self { + AgentImageSource::Image(reference) => reference.clone(), + AgentImageSource::Default | AgentImageSource::Dockerfile(_) => self + .build_tag() + .expect("build_tag is Some for Default and Dockerfile variants"), + } + } +} + +/// Whether this image source's image already exists locally. A plain +/// `Image` reference is always reported as "available" — `docker run` pulls +/// it automatically if missing, the same as any ordinary `docker run +/// ` invocation, so there's nothing for `stashbase` itself to build. +pub(crate) fn sandbox_image_exists(source: &AgentImageSource) -> bool { + if matches!(source, AgentImageSource::Image(_)) { + return true; + } + std::process::Command::new("docker") + .args(["image", "inspect", &source.image_tag()]) + .output() + .map(|output| output.status.success()) + .unwrap_or(false) +} + +/// Builds the image for `source` (`Default` or `Dockerfile` only — `Image` +/// has nothing to build and is rejected). Writes the Dockerfile to a +/// temporary build context directory (Docker needs a real directory to +/// build from, not stdin, so the CA-mount-style "just pass a string" +/// approach doesn't apply here) and cleans that directory up afterward +/// regardless of build outcome. +/// +/// `docker build`'s own output (BuildKit's per-step progress, including +/// download/install progress for the apt and npm layers) is inherited +/// straight through to this process's stdout/stderr rather than captured — +/// the build can take a minute or more on first run (Node.js, npm +/// packages), and a silent hang would look broken. This does mean a +/// failure's error message comes from the already-visible build output, +/// not a captured string. +pub(crate) fn build_sandbox_image(source: &AgentImageSource) -> Result<(), String> { + let tag = source + .build_tag() + .ok_or_else(|| "a custom image reference has nothing to build".to_owned())?; + let build_dir = + std::env::temp_dir().join(format!("stashbase-agent-sandbox-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&build_dir) + .map_err(|error| format!("failed to create a temporary build directory: {error}"))?; + let build_result = match source { + AgentImageSource::Default => { + std::fs::write(build_dir.join("Dockerfile"), SANDBOX_DOCKERFILE) + .map_err(|error| format!("failed to write the embedded Dockerfile: {error}")) + .map(|()| build_dir.clone()) + } + AgentImageSource::Dockerfile(path) => { + let contents = std::fs::read_to_string(path) + .map_err(|error| format!("failed to read {}: {error}", path.display())); + contents.and_then(|contents| { + std::fs::write(build_dir.join("Dockerfile"), contents) + .map_err(|error| { + format!("failed to copy the Dockerfile into the build context: {error}") + }) + .map(|()| build_dir.clone()) + }) + } + AgentImageSource::Image(_) => unreachable!("checked by build_tag above"), + } + .and_then(|build_dir| { + std::process::Command::new("docker") + .args(["build", "-t", &tag]) + .arg(&build_dir) + .status() + .map_err(|error| format!("failed to run `docker build`: {error}")) + }) + .and_then(|status| { + if status.success() { + Ok(()) + } else { + Err("`docker build` failed; see the build output above for details".to_owned()) + } + }); + let _ = std::fs::remove_dir_all(&build_dir); + build_result +} + +/// `--user uid:gid` args for the agent container, Linux only: Docker +/// Desktop already maps container-root writes on a bind mount back to the +/// host user transparently; native Linux does not, so without this every +/// file the agent creates in the mounted project directory would end up +/// root-owned. +/// +/// Split into two `#[cfg]`-gated functions rather than one function with a +/// runtime `if cfg!(target_os = "linux")` check: `cfg!()` is a runtime +/// boolean, not conditional compilation, so code behind it still has to +/// *compile* on every target — and `libc::getuid`/`getgid` don't exist on +/// Windows at all (there's no POSIX uid/gid concept there), which broke the +/// Windows CI build under the old approach. An actual `#[cfg(...)]` +/// attribute excludes the Linux-only body from non-Linux compilation +/// entirely, not just from running. +#[cfg(target_os = "linux")] +fn docker_run_user_flag_args() -> Vec { + vec![ + "--user".to_owned(), + format!("{}:{}", unsafe { libc::getuid() }, unsafe { + libc::getgid() + }), + ] +} + +#[cfg(not(target_os = "linux"))] +fn docker_run_user_flag_args() -> Vec { + Vec::new() +} + +/// Builds a `docker run` invocation that mounts only the current working +/// directory (read-write), attaches the container to `network` so it can +/// reach the credential proxy at `network.gateway_ip`, and passes `env_vars` +/// explicitly via `-e` (never relying on inherited process environment, +/// since `docker run -e VAR` with no value pulls from the *calling* +/// process's environment, which would leak host env vars into the +/// container unintentionally). +/// +/// Errs (fail closed) rather than building an invocation that would mount +/// an unsafe path — see `append_ca_bundle_mount`. +#[allow(clippy::too_many_arguments)] +pub(crate) fn docker_run_command( + command: &str, + network: &DockerRunNetwork, + denied_read_paths: &[String], + denied_write_paths: &[String], + env_vars: &std::collections::HashMap, + stdin_is_terminal: bool, + agent_image: &str, + memory_limit: Option<&str>, + cpus_limit: Option<&str>, +) -> Result<(String, Vec), String> { + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/")); + let cwd_str = cwd.to_string_lossy().into_owned(); + + let mut args = vec![ + "run".to_owned(), + "--rm".to_owned(), + // Reusing the per-run network's name as the container's own name + // gives the caller a deterministic handle to explicitly `docker + // stop` this exact container during teardown, rather than relying + // solely on `docker run`'s own SIGINT-forwarding behavior (which + // doesn't apply to every termination path — e.g. this process + // being killed non-gracefully) to have already stopped it. + "--name".to_owned(), + network.name.clone(), + // Interactive agents (Claude Code, Cursor, Codex) need a stdin + // stream; without `-i` the container's stdin is /dev/null and + // every interactive TUI breaks. `-t` is only safe to add when the + // caller's own stdin is a real terminal. + "-i".to_owned(), + ]; + if stdin_is_terminal { + args.push("-t".to_owned()); + } + args.extend([ + // The agent container itself never holds NET_ADMIN or any other + // added capability — the network-layer egress firewall is set up + // by a separate, short-lived helper container this one joins the + // network namespace of (see `start_netns_holder` and the + // `--network container:` below). Namespace rules are + // shared by anything attached to that namespace; the capability to + // change them is not, so this container can use the firewall but + // never touch it. + "--cap-drop".to_owned(), + "ALL".to_owned(), + "--security-opt".to_owned(), + "no-new-privileges".to_owned(), + // A real init process as PID 1 (Docker bundles tini for this) + // reaps zombie processes and forwards signals correctly — without + // it, an agent that spawns and orphans subprocesses (build tools, + // language servers) can leak zombies for the life of the + // container. Always on: there's no legitimate workload this could + // break, unlike the opt-in memory/cpus limits above. + "--init".to_owned(), + // A generous but finite cap on the container's process count. Not + // meant to constrain any real workload — a coding agent spawning + // build tools, test runners, and language servers comes nowhere + // close to this — it exists purely to contain a fork bomb (bug or + // malicious) to the container's own cgroup instead of letting it + // exhaust the host's PID table. Always on for the same reason + // `--init` is: no real cost, meaningful downside blocked. + "--pids-limit".to_owned(), + "2048".to_owned(), + ]); + args.extend(docker_run_user_flag_args()); + args.extend([ + "--network".to_owned(), + format!("container:{}", netns_holder_name(network)), + ]); + // Opt-in only — see `AgentSandboxProfile::memory`/`cpus`. No default + // cap: an automatic one could silently break a legitimately + // memory/CPU-hungry task with no warning, so this only applies when a + // profile explicitly asks for it. + if let Some(memory) = memory_limit { + args.extend(["--memory".to_owned(), memory.to_owned()]); + } + if let Some(cpus) = cpus_limit { + args.extend(["--cpus".to_owned(), cpus.to_owned()]); + } + + append_filesystem_mounts(&mut args, &cwd_str, denied_read_paths, denied_write_paths)?; + append_ca_bundle_mount(&mut args, &cwd_str, env_vars)?; + + // A named Docker volume, not a bind mount of the real host home + // directory, persists login/config state (e.g. Claude Code's + // ~/.claude) across runs. Named volumes are Docker-managed storage — + // they don't expose any other host path to the container — so this + // doesn't reopen the filesystem allow-list `append_filesystem_mounts` + // exists to provide. Shared across all profiles/runs by design: log + // in once, every docker-backend run on this machine reuses it. + args.extend([ + "-v".to_owned(), + format!("{PERSISTENT_HOME_VOLUME}:{CONTAINER_HOME}"), + "-e".to_owned(), + format!("HOME={CONTAINER_HOME}"), + ]); + + args.extend(["-w".to_owned(), cwd_str]); + + // Git identity (name/email) is not sensitive the way SSH keys or + // credentials are, so unlike everything else outside the working + // directory it's worth forwarding — without it, `git commit` inside + // the container fails outright with no identity configured, since the + // container never sees the host's real ~/.gitconfig. Env vars only + // (not the .gitconfig file itself), so unrelated host git config + // (aliases, signing setup pointing at host paths, etc.) doesn't leak + // in. Caller-provided env vars win if a profile already sets one of + // these explicitly. + for (key, value) in host_git_identity_env_vars() { + if !env_vars.contains_key(&key) { + args.push("-e".to_owned()); + args.push(format!("{key}={value}")); + } + } + + for (key, value) in env_vars { + args.push("-e".to_owned()); + args.push(format!("{key}={value}")); + } + // FORCE_COLOR is normally set on the outer `docker` process by + // run_built_command, which has no effect on the container's own + // environment — set it explicitly here so colored output survives. + args.push("-e".to_owned()); + args.push("FORCE_COLOR=true".to_owned()); + + // TERM/COLORTERM drive terminal-capability detection (truecolor + // support, theme selection) in TUIs like Codex's — FORCE_COLOR alone + // only covers basic on/off color, not that. Forwarded from the host + // since the container has no controlling terminal of its own to + // detect these from; caller-provided env vars still win. + for key in ["TERM", "COLORTERM"] { + if !env_vars.contains_key(key) { + if let Ok(value) = std::env::var(key) { + args.push("-e".to_owned()); + args.push(format!("{key}={value}")); + } + } + } + + args.push(agent_image.to_owned()); + args.push(command.to_owned()); + Ok(("docker".to_owned(), args)) +} + +fn append_filesystem_mounts( + args: &mut Vec, + cwd: &str, + denied_read_paths: &[String], + denied_write_paths: &[String], +) -> Result<(), String> { + let read_paths = super::subprocess::resolve_policy_paths(denied_read_paths); + let write_paths = super::subprocess::resolve_policy_paths(denied_write_paths); + + let cwd_is_denied_write = write_paths.iter().any(|path| path == cwd); + if cwd_is_denied_write { + args.extend(["-v".to_owned(), format!("{cwd}:{cwd}:ro")]); + } else { + args.extend(["-v".to_owned(), format!("{cwd}:{cwd}")]); + } + + for path in &read_paths { + if !is_nested_under(path, cwd) { + continue; + } + // `--tmpfs` only accepts a directory target; a file target fails + // container creation outright ("not a directory"). For a denied + // file, bind-mount a genuine empty regular file over it read-only + // instead — not `/dev/null`, which works for hiding content (reads + // return nothing, writes are discarded) but is a character device, + // not a regular file: `ls -l` shows `crw-rw-rw-`, `stat()`/ + // `os.path.isfile()` disagree with a normal file, and it's + // confusing enough in practice that a coding agent inspecting it + // has been observed flagging it as broken and offering to "fix" + // it. An empty regular file gives the identical security effect + // (empty, read-only) without that anomaly. + if PathBuf::from(path).is_dir() { + args.extend(["--tmpfs".to_owned(), path.clone()]); + } else { + match empty_shadow_file_path() { + Ok(shadow_file) => { + args.extend(["-v".to_owned(), format!("{shadow_file}:{path}:ro")]); + } + Err(error) => return Err(error), + } + } + } + + for path in &write_paths { + if path == cwd || !is_nested_under(path, cwd) { + continue; + } + if read_paths + .iter() + .any(|read| read == path || is_nested_under(path, read)) + { + continue; + } + args.extend(["-v".to_owned(), format!("{path}:{path}:ro")]); + } + Ok(()) +} + +/// The host-side path of a genuine, empty, world-readable regular file used +/// as the shadow-mount source for a denied-read file (see +/// `append_filesystem_mounts`) — created once and reused across runs, since +/// it never needs to change. Kept in the system temp directory rather than +/// bundled into the image because a bind mount's source must be a host +/// path; it can't reference a path from inside the image itself. +/// +/// Shared with the native backend's bubblewrap invocation (see +/// `subprocess.rs`), which has the exact same need for the exact same +/// reason: `--ro-bind`/`-v` both preserve the *source's* file type at the +/// mount target, and `/dev/null` is a character device, not a regular +/// file — confusing enough in practice (`ls -l` shows `crw-rw-rw-`, +/// `stat()`/`os.path.isfile()` disagree with a normal file) that a coding +/// agent inspecting one has been observed flagging it as broken. +pub(crate) fn empty_shadow_file_path() -> Result { + let path = std::env::temp_dir().join("stashbase-docker-empty-shadow-file"); + if !path.exists() { + std::fs::write(&path, []).map_err(|error| { + format!( + "failed to create the empty shadow file at {}: {error}", + path.display() + ) + })?; + } + Ok(path.to_string_lossy().into_owned()) +} + +/// Env vars whose value is a filesystem path to the proxy's temporary CA +/// certificate (see `Proxy::start_inner`'s `child_env` construction in +/// `proxy.rs`). The container only sees the working directory by default, +/// so these paths must be bind-mounted read-only or TLS interception +/// breaks for every tool that reads one of them to trust the proxy. +const CA_BUNDLE_ENV_KEYS: &[&str] = &[ + "SSL_CERT_FILE", + "CURL_CA_BUNDLE", + "GIT_SSL_CAINFO", + "NODE_EXTRA_CA_CERTS", + "CODEX_CA_CERTIFICATE", +]; + +fn host_git_config(key: &str) -> Option { + let output = std::process::Command::new("git") + .args(["config", "--global", key]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let value = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + if value.is_empty() { + None + } else { + Some(value) + } +} + +/// Reads the host's global git identity (`user.name`/`user.email`) and +/// maps it to the env vars git itself honors for both authoring and +/// committing. Returns an empty map if the host has neither configured — +/// this is a convenience, not a requirement, and the container works +/// fine without it (git commands that don't need an identity still run). +fn host_git_identity_env_vars() -> std::collections::HashMap { + let mut vars = std::collections::HashMap::new(); + if let Some(name) = host_git_config("user.name") { + vars.insert("GIT_AUTHOR_NAME".to_owned(), name.clone()); + vars.insert("GIT_COMMITTER_NAME".to_owned(), name); + } + if let Some(email) = host_git_config("user.email") { + vars.insert("GIT_AUTHOR_EMAIL".to_owned(), email.clone()); + vars.insert("GIT_COMMITTER_EMAIL".to_owned(), email); + } + vars +} + +/// Mounts each distinct CA-bundle path found in `env_vars` into the +/// container read-only, as the single file — never its parent directory, +/// which on a typical system temp path (`/tmp/stashbase-proxy-ca-*.pem`) +/// would otherwise expose every other process's and every other agent +/// run's temp files to the container. Refuses (fails closed, per this +/// project's sandboxing policy) rather than mounting a path that is not +/// absolute or that resolves to the filesystem root — both would defeat +/// the filesystem allow-list this backend exists to provide. +fn append_ca_bundle_mount( + args: &mut Vec, + cwd: &str, + env_vars: &std::collections::HashMap, +) -> Result<(), String> { + let mut mounted_paths: Vec = Vec::new(); + for key in CA_BUNDLE_ENV_KEYS { + let Some(path) = env_vars.get(*key) else { + continue; + }; + if path.is_empty() || is_nested_under(path, cwd) || path == cwd { + // Already visible through the cwd mount. + continue; + } + if !PathBuf::from(path).is_absolute() { + return Err(format!( + "refusing to mount non-absolute CA bundle path into the Docker sandbox: {path}" + )); + } + if path == "/" { + return Err( + "refusing to mount the filesystem root into the Docker sandbox as a CA bundle path" + .to_owned(), + ); + } + if mounted_paths.iter().any(|mounted| mounted == path) { + continue; + } + args.extend(["-v".to_owned(), format!("{path}:{path}:ro")]); + mounted_paths.push(path.clone()); + } + Ok(()) +} + +fn is_nested_under(path: &str, ancestor: &str) -> bool { + PathBuf::from(path) != PathBuf::from(ancestor) + && PathBuf::from(path).starts_with(PathBuf::from(ancestor)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Mutex, OnceLock}; + + /// Serializes the tests that touch the real Docker daemon (image + /// build/rebuild, container run) — `cargo test`'s default parallelism + /// otherwise lets e.g. an image rebuild race a concurrent `docker run` + /// of that same image, causing an intermittent "image not found" or + /// similar transient failure that has nothing to do with the code + /// under test. + fn docker_daemon_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + + /// Builds the default sandbox image once per test binary run, for any + /// test that actually executes `docker run`/`exec` against it (as + /// opposed to the many tests that only build a `docker_run_command` + /// argv list without ever invoking Docker for real). `cargo test`'s + /// default order is not "this file's declaration order" and isn't + /// guaranteed at all — on a fresh machine with no image already built + /// (a clean CI runner, unlike a developer's machine that's likely + /// built it before), a test needing the image can and did run before + /// `sandbox_image_lifecycle_when_docker_available` (the only test that + /// used to build it), failing with "image not found" since Docker then + /// tries to pull a nonexistent, unpublished image instead. Call this + /// after acquiring `docker_daemon_lock()` and confirming Docker is + /// reachable, in every test that runs a real container. + fn ensure_default_sandbox_image_for_tests() { + static BUILD_ONCE: OnceLock<()> = OnceLock::new(); + BUILD_ONCE.get_or_init(|| { + if !sandbox_image_exists(&AgentImageSource::Default) { + build_sandbox_image(&AgentImageSource::Default) + .expect("building the embedded Dockerfile should succeed for tests"); + } + }); + } + + #[test] + fn sandbox_dockerfile_is_embedded_and_non_empty() { + assert!(SANDBOX_DOCKERFILE.contains("FROM")); + } + + #[test] + fn agent_image_source_defaults_when_neither_field_is_set() { + assert_eq!( + AgentImageSource::from_profile(None, None), + AgentImageSource::Default + ); + } + + #[test] + fn agent_image_source_prefers_image_over_dockerfile() { + // Profile validation rejects setting both, but resolution still + // needs a defined priority for defense in depth. + assert_eq!( + AgentImageSource::from_profile(Some("myorg/img:tag"), Some("./custom.Dockerfile")), + AgentImageSource::Image("myorg/img:tag".to_owned()) + ); + } + + #[test] + fn agent_image_source_image_tag_uses_the_reference_directly() { + let source = AgentImageSource::from_profile(Some("myorg/img:tag"), None); + assert_eq!(source.image_tag(), "myorg/img:tag"); + } + + #[test] + fn agent_image_source_default_tag_is_the_builtin_image() { + assert_eq!(AgentImageSource::Default.image_tag(), DEFAULT_SANDBOX_IMAGE); + } + + #[test] + fn agent_image_source_dockerfile_tag_is_deterministic_for_the_same_path() { + let a = AgentImageSource::from_profile(None, Some("./docker/agent-sandbox/Dockerfile")); + let b = AgentImageSource::from_profile(None, Some("./docker/agent-sandbox/Dockerfile")); + assert_eq!(a.image_tag(), b.image_tag()); + assert!(a.image_tag().starts_with("stashbase/agent-sandbox-custom:")); + } + + #[test] + fn agent_image_source_dockerfile_tag_differs_for_different_paths() { + let a = AgentImageSource::from_profile(None, Some("./docker/agent-sandbox/Dockerfile")); + let b = AgentImageSource::from_profile(None, Some("./Cargo.toml")); + assert_ne!(a.image_tag(), b.image_tag()); + } + + #[test] + fn a_plain_image_reference_is_always_reported_as_already_available() { + // No local build is possible for an image reference — `docker run` + // pulls it automatically, same as any ordinary invocation. + let source = AgentImageSource::Image("myorg/img:tag".to_owned()); + assert!(sandbox_image_exists(&source)); + } + + #[test] + fn building_a_plain_image_reference_is_rejected() { + let source = AgentImageSource::Image("myorg/img:tag".to_owned()); + let error = build_sandbox_image(&source).expect_err("nothing to build for an image ref"); + assert!(error.contains("nothing to build")); + } + + #[test] + fn sandbox_image_lifecycle_when_docker_available() { + let _guard = docker_daemon_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if docker_enforcement_error().is_some() { + eprintln!("skipping: Docker not available in this environment"); + return; + } + // Don't assert on the starting state — a prior test run or the + // developer's own machine may already have the image built. + // Just prove building it results in it existing. + build_sandbox_image(&AgentImageSource::Default) + .expect("building the embedded Dockerfile should succeed"); + assert!(sandbox_image_exists(&AgentImageSource::Default)); + } + + #[test] + fn docker_binary_lookup_matches_which_docker() { + let expected = std::env::var_os("PATH").is_some_and(|path| { + std::env::split_paths(&path).any(|dir| dir.join("docker").is_file()) + }); + assert_eq!(docker_binary_available(), expected); + } + + #[test] + fn proxy_bind_and_container_host_differ_on_docker_desktop_platforms() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let bind_host = proxy_bind_host(&network); + let container_host = proxy_container_host(&network); + // Docker Desktop (macOS and Windows) runs containers inside a VM, + // so the host process can't bind to the bridge network's gateway + // address at all — only native Linux Docker can. + if cfg!(target_os = "macos") || cfg!(target_os = "windows") { + assert_eq!(bind_host, "127.0.0.1"); + assert_eq!(container_host, "host.docker.internal"); + } else { + assert_eq!(bind_host, "172.30.0.1"); + assert_eq!(container_host, "172.30.0.1"); + } + } + + #[test] + fn rewrite_proxy_urls_replaces_only_known_proxy_keys() { + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("HTTPS_PROXY".to_owned(), "http://127.0.0.1:9999".to_owned()); + env_vars.insert("STASHBASE_GH_TOKEN".to_owned(), "127.0.0.1".to_owned()); + + let rewritten = + rewrite_proxy_urls_for_container(&env_vars, "127.0.0.1", "host.docker.internal"); + + assert_eq!(rewritten["HTTPS_PROXY"], "http://host.docker.internal:9999"); + // A placeholder that happens to contain the bind host as a + // substring must not be rewritten — only known proxy-URL keys are. + assert_eq!(rewritten["STASHBASE_GH_TOKEN"], "127.0.0.1"); + } + + #[test] + fn rewrite_proxy_urls_is_a_no_op_when_hosts_match() { + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert( + "HTTPS_PROXY".to_owned(), + "http://172.30.0.1:9999".to_owned(), + ); + let rewritten = rewrite_proxy_urls_for_container(&env_vars, "172.30.0.1", "172.30.0.1"); + assert_eq!(rewritten, env_vars); + } + + #[test] + fn run_network_names_are_unique_per_call_without_a_session_id() { + let first = generate_run_network_name(None); + let second = generate_run_network_name(None); + assert_ne!(first, second); + assert!(first.starts_with("stashbase-agent-run-")); + } + + #[test] + fn run_network_name_uses_the_session_id_when_given() { + // Naming the network after the run's own session id (rather than an + // unrelated random UUID) lets a stray container/network left behind + // after a crash be traced back to the "Agent session"/"Audit + // session" id already shown for that run. + let name = generate_run_network_name(Some("ags_test123")); + assert_eq!(name, "stashbase-agent-run-ags_test123"); + } + + #[test] + fn run_network_name_falls_back_to_a_uuid_for_an_empty_session_id() { + let first = generate_run_network_name(Some("")); + let second = generate_run_network_name(Some("")); + assert_ne!(first, second); + } + + #[test] + fn extract_gateway_parses_docker_network_inspect_output() { + let inspect_json = + r#"[{"IPAM":{"Config":[{"Subnet":"172.30.0.0/16","Gateway":"172.30.0.1"}]}}]"#; + let gateway = extract_gateway_from_inspect(inspect_json).unwrap(); + assert_eq!(gateway, "172.30.0.1"); + } + + #[test] + fn extract_gateway_errors_on_missing_gateway() { + let inspect_json = r#"[{"IPAM":{"Config":[{"Subnet":"172.30.0.0/16"}]}}]"#; + assert!(extract_gateway_from_inspect(inspect_json).is_err()); + } + + #[test] + fn create_and_remove_run_network_round_trips_when_docker_available() { + let _guard = docker_daemon_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if docker_enforcement_error().is_some() { + eprintln!("skipping: Docker not available in this environment"); + return; + } + let network = create_run_network(None).expect("network should be created"); + assert!(!network.gateway_ip.is_empty()); + remove_run_network(&network).expect("network should be removed"); + } + + #[test] + fn list_run_networks_finds_a_network_and_extracts_its_session_id_when_docker_available() { + let _guard = docker_daemon_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if docker_enforcement_error().is_some() { + eprintln!("skipping: Docker not available in this environment"); + return; + } + let session_id = format!("ags_listtest{}", uuid::Uuid::new_v4().simple()); + let network = create_run_network(Some(&session_id)).expect("network should be created"); + + let found = list_run_networks() + .expect("listing should succeed") + .into_iter() + .find(|entry| entry.name == network.name); + let entry = found.expect("the just-created network should be in the listing"); + assert_eq!(entry.session_id.as_deref(), Some(session_id.as_str())); + assert!(!entry.created_at.is_empty()); + + remove_run_network(&network).expect("network should be removed"); + let still_listed = list_run_networks() + .expect("listing should succeed") + .into_iter() + .any(|entry| entry.name == network.name); + assert!(!still_listed, "removed network should no longer be listed"); + } + + #[test] + fn remove_run_network_succeeds_even_with_a_still_running_container() { + let _guard = docker_daemon_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if docker_enforcement_error().is_some() { + eprintln!("skipping: Docker not available in this environment"); + return; + } + ensure_default_sandbox_image_for_tests(); + let network = create_run_network(None).expect("network should be created"); + // Start a long-running container on this network with the same + // name `docker_run_command` would give it, without `--rm`, so it + // is still attached when teardown runs — reproducing the "network + // has active endpoints" failure this function exists to avoid. + let start = std::process::Command::new("docker") + .args([ + "run", + "-d", + "--rm", + "--name", + &network.name, + "--network", + &network.name, + DEFAULT_SANDBOX_IMAGE, + "sleep", + "300", + ]) + .output() + .expect("docker run should execute"); + assert!( + start.status.success(), + "failed to start test container: {}", + String::from_utf8_lossy(&start.stderr) + ); + remove_run_network(&network) + .expect("network removal should succeed by stopping the still-running container first"); + } + + #[test] + fn netns_holder_name_is_derived_from_network_name() { + let network = DockerRunNetwork { + name: "stashbase-agent-run-abc123".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + assert_eq!( + netns_holder_name(&network), + "stashbase-agent-run-abc123-netns-holder" + ); + } + + #[test] + fn parse_proxy_host_port_reads_https_proxy() { + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert( + "HTTPS_PROXY".to_owned(), + "http://host.docker.internal:54321".to_owned(), + ); + let (host, port) = parse_proxy_host_port(&env_vars).unwrap(); + assert_eq!(host, "host.docker.internal"); + assert_eq!(port, "54321"); + } + + #[test] + fn parse_proxy_host_port_falls_back_to_http_proxy() { + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("HTTP_PROXY".to_owned(), "http://172.17.0.1:9999".to_owned()); + let (host, port) = parse_proxy_host_port(&env_vars).unwrap(); + assert_eq!(host, "172.17.0.1"); + assert_eq!(port, "9999"); + } + + #[test] + fn parse_proxy_host_port_returns_none_when_absent() { + let env_vars = std::collections::HashMap::new(); + assert!(parse_proxy_host_port(&env_vars).is_none()); + } + + #[test] + fn netns_holder_blocks_direct_egress_but_allows_proxy_when_docker_available() { + let _guard = docker_daemon_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if docker_enforcement_error().is_some() { + eprintln!("skipping: Docker not available in this environment"); + return; + } + ensure_default_sandbox_image_for_tests(); + let network = create_run_network(None).expect("network should be created"); + + // A tiny host-side listener the holder's firewall rule should allow + // through (simulating the credential proxy). + // Short name: DNS labels cap out at 63 characters, and + // `network.name` (already `stashbase-agent-run-`) plus a + // suffix would exceed that, making it unresolvable — a test + // artifact only, real usage never resolves container names. + let listener_container = format!("listener-{}", uuid::Uuid::new_v4().simple()); + let listen = std::process::Command::new("docker") + .args([ + "run", + "-d", + "--rm", + "--name", + &listener_container, + "--network", + &network.name, + DEFAULT_SANDBOX_IMAGE, + "node", + "-e", + "require('http').createServer((_, response) => response.end('ok')).listen(18234)", + ]) + .output() + .expect("docker run should execute"); + assert!(listen.status.success()); + // Give the listener a moment to bind before anything tries to + // reach it. + std::thread::sleep(std::time::Duration::from_millis(500)); + + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert( + "HTTPS_PROXY".to_owned(), + format!("http://{listener_container}:18234"), + ); + let holder_result = start_netns_holder(&network, &env_vars); + let cleanup = || { + stop_container_if_running(&listener_container); + let _ = remove_run_network(&network); + }; + if let Err(error) = holder_result { + cleanup(); + panic!("start_netns_holder failed: {error}"); + } + + let holder_name = netns_holder_name(&network); + let allowed = std::process::Command::new("docker") + .args([ + "exec", + &holder_name, + "curl", + "-s", + "-m", + "5", + "-o", + "/dev/null", + "-w", + "%{http_code}", + &format!("http://{listener_container}:18234/"), + ]) + .output(); + let blocked = std::process::Command::new("docker") + .args([ + "exec", + &holder_name, + "curl", + "-s", + "-m", + "5", + "-o", + "/dev/null", + "-w", + "%{http_code}", + "https://example.com", + ]) + .output(); + // DNS must not be a data-exfiltration side channel: Docker's + // embedded resolver forwards unresolved lookups via the host's own + // DNS stack, entirely outside this container's network namespace + // — no iptables rule inside the container can see, let alone + // block, that traffic. The only real fix is disabling upstream + // forwarding at the source (`--dns 0.0.0.0` on the holder, which + // the agent inherits) — assert that's actually working, not just + // that the firewall rule exists. + let dns_lookup = std::process::Command::new("docker") + .args(["exec", &holder_name, "getent", "hosts", "example.com"]) + .output(); + + cleanup(); + + let allowed = allowed.expect("docker exec should run"); + let blocked = blocked.expect("docker exec should run"); + let dns_lookup = dns_lookup.expect("docker exec should run"); + assert_eq!( + String::from_utf8_lossy(&allowed.stdout), + "200", + "the proxy-equivalent listener should be reachable through the firewall" + ); + assert_eq!( + String::from_utf8_lossy(&blocked.stdout), + "000", + "a direct request to an arbitrary host should be blocked by the firewall" + ); + assert!( + !dns_lookup.status.success(), + "an external hostname must not resolve at all — a successful lookup here means \ + Docker's embedded DNS resolver is still forwarding queries upstream, which is a \ + data-exfiltration channel no container-level firewall rule can block" + ); + } + + #[test] + fn docker_run_command_mounts_cwd_and_sets_env_with_no_denied_paths() { + let network = DockerRunNetwork { + name: "test-network".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert( + "HTTPS_PROXY".to_owned(), + "https://172.30.0.1:9999".to_owned(), + ); + + let (program, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + + assert_eq!(program, "docker"); + assert!(args.contains(&"run".to_owned())); + assert!(args.contains(&"--rm".to_owned())); + assert!(args.contains(&"--network".to_owned())); + assert!(args.contains(&"test-network".to_owned())); + assert!(args.contains(&"-e".to_owned())); + assert!(args.contains(&"HTTPS_PROXY=https://172.30.0.1:9999".to_owned())); + assert!(args.contains(&DEFAULT_SANDBOX_IMAGE.to_owned())); + assert_eq!(args[args.len() - 2], DEFAULT_SANDBOX_IMAGE); + assert_eq!(args[args.len() - 1], "claude"); + } + + #[test] + fn docker_run_command_mounts_cwd_readwrite_when_no_deny_paths() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let cwd = std::env::current_dir() + .unwrap() + .to_string_lossy() + .into_owned(); + assert!(args.contains(&"-v".to_owned())); + let mount_index = args.iter().position(|arg| arg == "-v").unwrap(); + assert_eq!(args[mount_index + 1], format!("{cwd}:{cwd}")); + } + + #[test] + fn docker_run_command_shadow_mounts_nested_deny_read_path_as_tmpfs() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let cwd = std::env::current_dir().unwrap(); + let nested = cwd.join(".git").to_string_lossy().into_owned(); + let (_, args) = docker_run_command( + "claude", + &network, + std::slice::from_ref(&nested), + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&"--tmpfs".to_owned())); + let tmpfs_index = args.iter().position(|arg| arg == "--tmpfs").unwrap(); + assert_eq!(args[tmpfs_index + 1], nested); + } + + #[test] + fn docker_run_command_shadow_mounts_nested_deny_read_file_as_empty_regular_file_bind() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let cwd = std::env::current_dir().unwrap(); + // Cargo.toml is a real file (not a directory) inside this repo's + // cwd — `--tmpfs` on a file target fails container creation + // outright ("not a directory"), so a denied file must be shadowed + // with a read-only bind mount of an empty regular file instead — + // not `/dev/null`: that hides content fine, but as a character + // device it confuses tooling (and agents) that expect a regular + // file at that path (`ls -l` shows `crw-rw-rw-`, `stat()` disagrees + // with a normal file). + let nested = cwd.join("Cargo.toml").to_string_lossy().into_owned(); + let (_, args) = docker_run_command( + "claude", + &network, + std::slice::from_ref(&nested), + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(!args.contains(&"--tmpfs".to_owned())); + let expected_source = empty_shadow_file_path().unwrap(); + let mount = args + .windows(2) + .find(|pair| pair[0] == "-v" && pair[1] == format!("{expected_source}:{nested}:ro")) + .unwrap_or_else(|| { + panic!("expected an empty-regular-file bind mount for the denied file") + }); + assert_eq!(mount[1], format!("{expected_source}:{nested}:ro")); + // And it really is a regular file, not /dev/null or any other + // special device — this is the whole point of the fix. + assert!(PathBuf::from(&expected_source).is_file()); + assert!(!PathBuf::from(&expected_source).metadata().unwrap().is_dir()); + #[cfg(unix)] + { + use std::os::unix::fs::FileTypeExt; + let file_type = PathBuf::from(&expected_source) + .metadata() + .unwrap() + .file_type(); + assert!(!file_type.is_char_device()); + assert!(!file_type.is_block_device()); + } + } + + #[test] + fn docker_run_command_shadow_mounts_nested_deny_write_path_readonly() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let cwd = std::env::current_dir().unwrap(); + let nested = cwd.join("Cargo.lock").to_string_lossy().into_owned(); + let (_, args) = docker_run_command( + "claude", + &network, + &[], + std::slice::from_ref(&nested), + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let nested_mount = args + .windows(2) + .find(|pair| pair[0] == "-v" && pair[1].starts_with(&format!("{nested}:"))) + .unwrap_or_else(|| panic!("no -v mount found for nested path {nested}")); + assert_eq!(nested_mount[1], format!("{nested}:{nested}:ro")); + } + + #[test] + fn docker_run_command_mounts_cwd_readonly_when_cwd_itself_is_denied_write() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let cwd = std::env::current_dir() + .unwrap() + .to_string_lossy() + .into_owned(); + let (_, args) = docker_run_command( + "claude", + &network, + &[], + std::slice::from_ref(&cwd), + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let mount_index = args.iter().position(|arg| arg == "-v").unwrap(); + assert_eq!(args[mount_index + 1], format!("{cwd}:{cwd}:ro")); + // The cwd mount plus the persistent home volume mount — no extra + // shadow mount, since the cwd mount itself is already read-only. + assert_eq!(args.iter().filter(|arg| *arg == "-v").count(), 2); + } + + #[test] + fn docker_run_command_forwards_host_git_identity_when_configured() { + let Some(name) = host_git_config("user.name") else { + eprintln!("skipping: host has no global git user.name configured"); + return; + }; + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&format!("GIT_AUTHOR_NAME={name}"))); + assert!(args.contains(&format!("GIT_COMMITTER_NAME={name}"))); + } + + #[test] + fn docker_run_command_lets_caller_env_vars_override_host_git_identity() { + if host_git_config("user.name").is_none() { + eprintln!("skipping: host has no global git user.name configured"); + return; + } + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("GIT_AUTHOR_NAME".to_owned(), "Explicit Override".to_owned()); + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&"GIT_AUTHOR_NAME=Explicit Override".to_owned())); + assert_eq!( + args.iter() + .filter(|arg| arg.starts_with("GIT_AUTHOR_NAME=")) + .count(), + 1 + ); + } + + #[test] + fn docker_run_command_mounts_ca_bundle_file_not_its_directory() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + // A hardcoded Unix-style literal here (e.g. `/tmp/...`) is not + // actually absolute under Windows path semantics — `PathBuf:: + // is_absolute()` is platform-aware, and the check this test + // exercises is real production logic, not something to special- + // case for tests. Build a path that's genuinely absolute on + // whatever platform this test runs on instead. + let ca_dir = std::env::temp_dir().join("stashbase-ca-test"); + let ca_path = ca_dir.join("ca.pem").to_string_lossy().into_owned(); + let ca_dir = ca_dir.to_string_lossy().into_owned(); + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("SSL_CERT_FILE".to_owned(), ca_path.clone()); + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + // Must mount only the exact file — mounting its parent directory + // would expose every other file in it (other processes' temp + // files, other agent runs' audit/revocation state) to the + // container, defeating the filesystem allow-list. + assert!(args.contains(&format!("{ca_path}:{ca_path}:ro"))); + assert!(!args + .iter() + .any(|arg| *arg == format!("{ca_dir}:{ca_dir}:ro"))); + } + + #[test] + fn docker_run_command_refuses_ca_bundle_path_at_filesystem_root() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("SSL_CERT_FILE".to_owned(), "/".to_owned()); + let result = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ); + assert!(result.is_err()); + } + + #[test] + fn docker_run_command_refuses_non_absolute_ca_bundle_path() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("SSL_CERT_FILE".to_owned(), "ca.pem".to_owned()); + let result = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ); + assert!(result.is_err()); + } + + #[test] + fn docker_run_command_names_the_container_after_the_network() { + let network = DockerRunNetwork { + name: "stashbase-agent-run-some-uuid".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let name_index = args.iter().position(|arg| arg == "--name").unwrap(); + assert_eq!(args[name_index + 1], network.name); + } + + #[test] + fn docker_run_command_always_passes_interactive_stdin_flag() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&"-i".to_owned())); + assert!(!args.contains(&"-t".to_owned())); + } + + #[test] + fn docker_run_command_adds_pty_flag_only_when_stdin_is_a_terminal() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + true, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&"-t".to_owned())); + } + + #[test] + fn docker_run_command_mounts_persistent_home_volume_and_sets_home() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&format!("{PERSISTENT_HOME_VOLUME}:{CONTAINER_HOME}"))); + assert!(args.contains(&format!("HOME={CONTAINER_HOME}"))); + } + + #[test] + fn docker_run_command_drops_capabilities_and_denies_new_privileges() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let cap_drop_index = args.iter().position(|arg| arg == "--cap-drop").unwrap(); + assert_eq!(args[cap_drop_index + 1], "ALL"); + // No --cap-add: the agent container never holds any added + // capability. The network-layer firewall is set up by a separate + // helper container (see start_netns_holder) whose namespace this + // one joins via `--network container:`. + assert!(!args.contains(&"--cap-add".to_owned())); + assert!(args.contains(&"--security-opt".to_owned())); + assert!(args.contains(&"no-new-privileges".to_owned())); + } + + #[test] + fn docker_run_command_always_adds_init_and_a_pids_limit() { + // Unlike memory/cpus, these are never configurable per profile and + // always applied — there's no legitimate workload either could + // break, only a fork bomb or zombie-process leak they exist to + // contain. + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(args.contains(&"--init".to_owned())); + let pids_limit_index = args.iter().position(|arg| arg == "--pids-limit").unwrap(); + assert_eq!(args[pids_limit_index + 1], "2048"); + } + + #[test] + fn docker_run_command_omits_resource_limits_by_default() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + assert!(!args.contains(&"--memory".to_owned())); + assert!(!args.contains(&"--cpus".to_owned())); + } + + #[test] + fn docker_run_command_adds_memory_and_cpu_limits_when_configured() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + Some("2g"), + Some("1.5"), + ) + .unwrap(); + let memory_index = args.iter().position(|arg| arg == "--memory").unwrap(); + assert_eq!(args[memory_index + 1], "2g"); + let cpus_index = args.iter().position(|arg| arg == "--cpus").unwrap(); + assert_eq!(args[cpus_index + 1], "1.5"); + } + + #[test] + fn docker_run_command_joins_the_netns_holders_network() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + let network_index = args.iter().position(|arg| arg == "--network").unwrap(); + assert_eq!(args[network_index + 1], "container:n-netns-holder"); + } + + #[test] + fn docker_run_command_uses_user_flag_only_on_linux() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &std::collections::HashMap::new(), + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + // Restoring `--user` is safe here: the agent container never runs + // any privileged setup itself, so it can start as the target + // uid/gid immediately, unlike the earlier setpriv-based approach. + if cfg!(target_os = "linux") { + assert!(args.contains(&"--user".to_owned())); + } + } + + #[test] + fn network_create_args_add_internal_flag_only_on_linux() { + let args = network_create_args("n"); + if cfg!(target_os = "linux") { + assert!(args.iter().any(|arg| arg == "--internal")); + } else { + assert!(!args.iter().any(|arg| arg == "--internal")); + } + } + + #[test] + fn docker_run_command_skips_ca_bundle_mount_when_already_under_cwd() { + let network = DockerRunNetwork { + name: "n".to_owned(), + gateway_ip: "172.30.0.1".to_owned(), + }; + let cwd = std::env::current_dir().unwrap(); + let ca_path = cwd.join("ca.pem").to_string_lossy().into_owned(); + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert("SSL_CERT_FILE".to_owned(), ca_path); + let (_, args) = docker_run_command( + "claude", + &network, + &[], + &[], + &env_vars, + false, + DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .unwrap(); + // Only the cwd mount and the persistent home volume mount should + // exist; no extra mount for a CA path that's already inside the + // working directory. + assert_eq!(args.iter().filter(|arg| *arg == "-v").count(), 2); + } +} diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index bc39f3d2..a78b877e 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -38,6 +38,80 @@ use crate::{ use super::format::format_env_variable_value; +/// Ensures the Docker sandbox backend's agent image exists locally, +/// building it (the built-in embedded Dockerfile, or a profile-supplied +/// custom one) on first use. There is no registry to `docker pull` from for +/// either of those, so the only way an installed `stashbase` binary can get +/// them is to build them itself; a plain `sandbox.image` reference, by +/// contrast, needs nothing built here — `docker run` pulls it automatically. +/// +/// Returns the image tag `docker run` should use for the agent container. +/// +/// In an interactive session, asks before building (an implicit multi- +/// minute `docker build` on first use would otherwise be a surprising side +/// effect of `agent run`). In `--silent` mode there is no one to ask, so +/// this fails closed with instructions rather than silently building or +/// silently running unsandboxed. +fn ensure_docker_sandbox_image_available( + source: &super::docker_sandbox::AgentImageSource, + silent: bool, +) -> anyhow::Result { + let tag = source.image_tag(); + if super::docker_sandbox::sandbox_image_exists(source) { + return Ok(tag); + } + if silent { + anyhow::bail!( + "the Docker sandbox image ({tag}) is not built yet; build it once with `docker build -t {tag} ` or re-run without --silent to be prompted", + ); + } + eprintln!(); + let should_build = crate::utils::interaction::confirm_opt(&format!( + "The Docker sandbox image ({tag}) isn't built yet. Build it now?" + )) + .unwrap_or(false); + // dialoguer can leave the terminal cursor hidden if the prompt is + // dismissed via Ctrl+C rather than answered normally (a known + // dialoguer/raw-mode interaction — see the Ctrl+C handler in main.rs + // for the same workaround applied to other prompts). Restore it + // unconditionally before deciding what the prompt's outcome was. + let _ = dialoguer::console::Term::stdout().show_cursor(); + if !should_build { + anyhow::bail!("Docker sandbox backend selected, but its image was not built"); + } + eprintln!("Building Docker sandbox image ({tag})..."); + super::docker_sandbox::build_sandbox_image(source) + .map_err(|error| anyhow::anyhow!("failed to build the Docker sandbox image: {error}"))?; + eprintln!("Docker sandbox image built."); + Ok(tag) +} + +/// Ensures every image a Docker-backend run will actually need is +/// available, and returns the tag the agent container should use. +/// +/// The network-namespace holder (see `start_netns_holder`) always uses the +/// built-in default image, deliberately never a profile's custom +/// `sandbox.image`/`sandbox.dockerfile` — but `ensure_docker_sandbox_image_available` +/// on its own only ensures whichever source the *agent* container resolves +/// to. For a profile using a custom image, that leaves the default image +/// unchecked: if it was never built (a user who only ever runs custom +/// images has no reason to have it), the holder's own `docker run` fails +/// outright since there's no registry to auto-pull it from. This ensures +/// both, skipping the duplicate prompt/build when the agent's own source +/// already *is* the default. +fn ensure_docker_images_available( + agent_image_source: &super::docker_sandbox::AgentImageSource, + silent: bool, +) -> anyhow::Result { + if *agent_image_source != super::docker_sandbox::AgentImageSource::Default { + ensure_docker_sandbox_image_available( + &super::docker_sandbox::AgentImageSource::Default, + silent, + )?; + } + ensure_docker_sandbox_image_available(agent_image_source, silent) +} + /// Runs an agent through the localhost relay while credentials stay in the /// control-plane's short-lived remote agent-proxy session. pub async fn handle_remote_agent_run( @@ -58,16 +132,79 @@ pub async fn handle_remote_agent_run( let denied_read_paths = policy.denied_read_paths.clone(); let denied_write_paths = policy.denied_write_paths.clone(); let allow_network_listeners = policy.allow_network_listeners; + let backend = policy.backend; + let agent_image_source = super::docker_sandbox::AgentImageSource::from_profile( + policy.sandbox_image.as_deref(), + policy.sandbox_dockerfile.as_deref(), + ); + let sandbox_memory = policy.sandbox_memory.clone(); + let sandbox_cpus = policy.sandbox_cpus.clone(); let command_audit_log = audit_log.clone(); - let proxy = super::proxy::Proxy::start_remote_with_hook( - remote, - policy, - audit_log, - proxy_port, - hooks_enabled.then_some(api_key), - ) - .await?; + let mut setup_spinner: Option = None; + let (docker_network, agent_image) = if backend == crate::models::agent::SandboxBackend::Docker { + // Resolved before the spinner starts: this can print its own + // interactive "build the image now?" prompt on first use, which + // must never race a concurrently animating spinner writing to the + // same stream (see the same reasoning for the proxy-started + // message below). + let agent_image = ensure_docker_images_available(&agent_image_source, silent)?; + setup_spinner = (!silent).then(|| { + crate::utils::spinner::new_spinner("Preparing sandbox network...", Streams::Stderr) + }); + ( + Some( + super::docker_sandbox::create_run_network( + audit_log.as_ref().map(|log| log.session_id()), + ) + .map_err(|error| { + anyhow::anyhow!("failed to create Docker sandbox network: {error}") + })?, + ), + agent_image, + ) + } else { + (None, String::new()) + }; + let proxy_start_result = if let Some(network) = &docker_network { + super::proxy::Proxy::start_remote_with_hook_and_bind_host( + remote, + policy, + audit_log, + proxy_port, + hooks_enabled.then_some(api_key), + &super::docker_sandbox::proxy_bind_host(network), + ) + .await + } else { + super::proxy::Proxy::start_remote_with_hook( + remote, + policy, + audit_log, + proxy_port, + hooks_enabled.then_some(api_key), + ) + .await + }; + let proxy = match proxy_start_result { + Ok(proxy) => proxy, + Err(error) => { + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + if let Some(network) = &docker_network { + let _ = super::docker_sandbox::remove_run_network(network); + } + return Err(error); + } + }; let _trusted_ca = trust_proxy_ca.then(|| proxy.trust_ca()).transpose()?; + // Stop the spinner before any plain `eprintln!` — spinoff redraws its + // line from a background thread, and interleaving that with ordinary + // stderr writes garbles both. Re-created below to cover the remaining + // netns-holder setup phase. + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } if !silent { let address = proxy.child_env()["HTTP_PROXY"].trim_start_matches("http://"); eprintln!( @@ -76,10 +213,56 @@ pub async fn handle_remote_agent_run( ); eprintln!("Remote agent proxy session active"); } - let result = subprocess::run_command_with_filesystem_policy( + let mut child_env = if let Some(network) = &docker_network { + super::docker_sandbox::rewrite_proxy_urls_for_container( + proxy.child_env(), + &super::docker_sandbox::proxy_bind_host(network), + &super::docker_sandbox::proxy_container_host(network), + ) + } else { + proxy.child_env().clone() + }; + let mut setup_spinner = (!silent && docker_network.is_some()).then(|| { + crate::utils::spinner::new_spinner( + "Starting network namespace holder and firewall...", + Streams::Stderr, + ) + }); + if let Some(network) = &docker_network { + match super::docker_sandbox::start_netns_holder(network, &child_env) { + Ok(Some(resolved_proxy_ip)) => { + // The agent container joins the holder's network namespace + // via `--network container:`, which Docker refuses + // to combine with `--add-host` — so the agent has no way to + // resolve `host.docker.internal` itself. Point it straight + // at the already-resolved IP instead, sidestepping the + // need for any DNS/hosts lookup in the agent container. + child_env = super::docker_sandbox::rewrite_proxy_urls_for_container( + &child_env, + &super::docker_sandbox::proxy_container_host(network), + &resolved_proxy_ip, + ); + } + Ok(None) => {} + Err(error) => { + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + let _ = super::docker_sandbox::remove_run_network(network); + proxy.stop().await; + return Err(anyhow::anyhow!( + "failed to start Docker sandbox network namespace holder: {error}" + )); + } + } + } + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + let result = subprocess::run_command_with_filesystem_policy_and_network( &cmd, args, - proxy.child_env().clone(), + child_env, source_env_names, sandbox, allow_network_listeners, @@ -88,9 +271,22 @@ pub async fn handle_remote_agent_run( &denied_read_paths, &denied_write_paths, command_audit_log, + backend, + docker_network.as_ref(), + &agent_image, + sandbox_memory.as_deref(), + sandbox_cpus.as_deref(), ) .await; proxy.stop().await; + if let Some(network) = &docker_network { + if let Err(error) = super::docker_sandbox::remove_run_network(network) { + eprintln!( + "warning: failed to remove Docker sandbox network {}: {error}", + network.name + ); + } + } if !silent { eprintln!("Remote agent proxy relay stopped"); } @@ -130,7 +326,7 @@ pub struct HandleRunArgs { pub silent: bool, pub scope: Option, pub dependency_hooks: bool, - pub local_session: Option, + pub local_session: Option, } pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { @@ -1014,7 +1210,7 @@ async fn handle_run( json_format: bool, dependency_hooks: bool, hook_api_key: Option, - local_session: Option, + local_session: Option, ) -> anyhow::Result<()> { apply_secret_bindings(&mut secrets, secret_bindings); let secrets_hash_map = env::expand_and_inject_env(&mut secrets); @@ -1137,23 +1333,100 @@ async fn handle_run( let allow_network_listeners = proxy_policy .as_ref() .is_some_and(|policy| policy.allow_network_listeners); + let backend = proxy_policy + .as_ref() + .map(|policy| policy.backend) + .unwrap_or_default(); + let agent_image_source = super::docker_sandbox::AgentImageSource::from_profile( + proxy_policy + .as_ref() + .and_then(|policy| policy.sandbox_image.as_deref()), + proxy_policy + .as_ref() + .and_then(|policy| policy.sandbox_dockerfile.as_deref()), + ); + let sandbox_memory = proxy_policy + .as_ref() + .and_then(|policy| policy.sandbox_memory.clone()); + let sandbox_cpus = proxy_policy + .as_ref() + .and_then(|policy| policy.sandbox_cpus.clone()); // Proxy mode gives the child placeholders, never the loaded secret values. // The temporary proxy owns the placeholder-to-secret mapping until the command exits. let command_result = if proxy { let command_audit_log = audit_log.clone(); - let proxy = super::proxy::Proxy::start_with_hook( - secrets_hash_map, - proxy_policy.unwrap_or_else(super::proxy::ProxyPolicy::permissive), - audit_log, - proxy_port, - dependency_hooks.then_some(hook_api_key).flatten(), - ) - .await?; + let mut setup_spinner: Option = None; + let (docker_network, agent_image) = if backend + == crate::models::agent::SandboxBackend::Docker + { + // Resolved before the spinner starts: this can print its own + // interactive "build the image now?" prompt on first use, which + // must never race a concurrently animating spinner writing to + // the same stream (see the same reasoning for the + // proxy-started message below). + let agent_image = ensure_docker_images_available(&agent_image_source, silent)?; + setup_spinner = (!silent).then(|| { + crate::utils::spinner::new_spinner("Preparing sandbox network...", Streams::Stderr) + }); + let run_session_id = local_session + .as_ref() + .map(|session| session.session_id()) + .or_else(|| audit_log.as_ref().map(|log| log.session_id())); + ( + Some( + super::docker_sandbox::create_run_network(run_session_id).map_err(|error| { + anyhow::anyhow!("failed to create Docker sandbox network: {error}") + })?, + ), + agent_image, + ) + } else { + (None, String::new()) + }; + let proxy_start_result = if let Some(network) = &docker_network { + super::proxy::Proxy::start_with_hook_and_bind_host( + secrets_hash_map, + proxy_policy.unwrap_or_else(super::proxy::ProxyPolicy::permissive), + audit_log, + proxy_port, + dependency_hooks.then_some(hook_api_key).flatten(), + &super::docker_sandbox::proxy_bind_host(network), + ) + .await + } else { + super::proxy::Proxy::start_with_hook( + secrets_hash_map, + proxy_policy.unwrap_or_else(super::proxy::ProxyPolicy::permissive), + audit_log, + proxy_port, + dependency_hooks.then_some(hook_api_key).flatten(), + ) + .await + }; + let proxy = match proxy_start_result { + Ok(proxy) => proxy, + Err(error) => { + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + if let Some(network) = &docker_network { + let _ = super::docker_sandbox::remove_run_network(network); + } + return Err(error); + } + }; if let Some(session) = &local_session { proxy.set_revocation_path(session.path()); } let _trusted_ca = trust_proxy_ca.then(|| proxy.trust_ca()).transpose()?; + // Stop the spinner before any plain `eprintln!` — spinoff redraws + // its line from a background thread, and interleaving that with + // ordinary stderr writes garbles both. Re-created below to cover + // the remaining netns-holder setup phase. + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } if !silent { let address = proxy.child_env()["HTTP_PROXY"].trim_start_matches("http://"); eprintln!( @@ -1161,8 +1434,52 @@ async fn handle_run( address.rsplit(':').next().unwrap_or_default() ); } - let child_env = proxy.child_env().clone(); - let command = Box::pin(subprocess::run_command_with_filesystem_policy( + let mut child_env = if let Some(network) = &docker_network { + super::docker_sandbox::rewrite_proxy_urls_for_container( + proxy.child_env(), + &super::docker_sandbox::proxy_bind_host(network), + &super::docker_sandbox::proxy_container_host(network), + ) + } else { + proxy.child_env().clone() + }; + let mut setup_spinner = (!silent && docker_network.is_some()).then(|| { + crate::utils::spinner::new_spinner( + "Starting network namespace holder and firewall...", + Streams::Stderr, + ) + }); + if let Some(network) = &docker_network { + match super::docker_sandbox::start_netns_holder(network, &child_env) { + Ok(Some(resolved_proxy_ip)) => { + // See the matching comment in handle_remote_agent_run: + // the agent container cannot resolve + // `host.docker.internal` itself once it joins the + // holder's network namespace, so point it at the + // already-resolved IP instead. + child_env = super::docker_sandbox::rewrite_proxy_urls_for_container( + &child_env, + &super::docker_sandbox::proxy_container_host(network), + &resolved_proxy_ip, + ); + } + Ok(None) => {} + Err(error) => { + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + let _ = super::docker_sandbox::remove_run_network(network); + proxy.stop().await; + return Err(anyhow::anyhow!( + "failed to start Docker sandbox network namespace holder: {error}" + )); + } + } + } + if let Some(mut spinner) = setup_spinner.take() { + spinner.clear(); + } + let command = Box::pin(subprocess::run_command_with_filesystem_policy_and_network( &cmd, args, child_env, @@ -1174,13 +1491,33 @@ async fn handle_run( &denied_read_paths, &denied_write_paths, command_audit_log, + backend, + docker_network.as_ref(), + &agent_image, + sandbox_memory.as_deref(), + sandbox_cpus.as_deref(), )); let result = command.await; proxy.stop().await; + if let Some(network) = &docker_network { + if let Err(error) = super::docker_sandbox::remove_run_network(network) { + eprintln!( + "warning: failed to remove Docker sandbox network {}: {error}", + network.name + ); + } + } if !silent { eprintln!("Agent proxy stopped"); } result + } else if backend != crate::models::agent::SandboxBackend::Native { + // The non-proxy path has no proxy/network to attach a Docker + // sandbox to. Fail closed rather than silently downgrading a + // profile's requested backend to Native. + Err(anyhow::anyhow!( + "the selected sandbox backend requires the agent proxy; re-run with the proxy enabled" + )) } else { // TODO: errors: no such file or directory subprocess::run_command( diff --git a/src/handlers/run/mod.rs b/src/handlers/run/mod.rs index decfda80..4b3bbe30 100644 --- a/src/handlers/run/mod.rs +++ b/src/handlers/run/mod.rs @@ -1,3 +1,4 @@ +pub mod docker_sandbox; pub mod entry; pub mod format; pub mod proxy; diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index f0c156c8..76368e9c 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -59,11 +59,11 @@ use tokio_rustls::{TlsAcceptor, TlsConnector}; use uuid::Uuid; use crate::{ - handlers::agent_policy::{ + handlers::agent::policy::{ evaluate_secret_authorization, host_matches, normalize_secret_http_policy, SecretAuthorizationDecision, SecretHttpPolicy, }, - models::agent::{AgentHttpRuleEffect, AgentMcpRule}, + models::agent::{AgentHttpRuleEffect, AgentMcpRule, SandboxBackend}, REQUEST_TIMEOUT_SECS, }; @@ -664,6 +664,21 @@ pub struct ProxyPolicy { pub egress_hosts_configured: bool, pub strict_deny: bool, pub mcp_rules: Vec, + /// Selects which enforcement backend the sandboxed child runs under. + pub backend: SandboxBackend, + /// Docker backend only: a custom image reference to run instead of the + /// built-in default. Takes priority over `sandbox_dockerfile` if both + /// are somehow set (profile loading rejects setting both). + pub sandbox_image: Option, + /// Docker backend only: path to a custom Dockerfile to build and run + /// instead of the built-in default image. + pub sandbox_dockerfile: Option, + /// Docker backend only: `docker run --memory` value, e.g. "2g". No cap + /// when unset. + pub sandbox_memory: Option, + /// Docker backend only: `docker run --cpus` value, e.g. "1.5". No cap + /// when unset. + pub sandbox_cpus: Option, } /// How a placeholder is represented in a child request and rewritten by the proxy. @@ -770,6 +785,11 @@ impl ProxyPolicy { egress_hosts_configured: false, strict_deny: false, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, } } @@ -780,6 +800,29 @@ impl ProxyPolicy { format!("egress_configured={}", self.egress_hosts_configured), format!("strict_deny={}", self.strict_deny), format!("allow_network_listeners={}", self.allow_network_listeners), + // The sandbox backend and its Docker-specific settings are part + // of the effective enforcement, not just the egress/secret + // policy — switching a profile from native to Docker, or + // swapping its custom image, must change the fingerprint, or + // an audit record can't tell those materially different runs + // apart. + format!("backend={:?}", self.backend), + format!( + "sandbox_image={}", + self.sandbox_image.as_deref().unwrap_or("") + ), + format!( + "sandbox_dockerfile={}", + self.sandbox_dockerfile.as_deref().unwrap_or("") + ), + format!( + "sandbox_memory={}", + self.sandbox_memory.as_deref().unwrap_or("") + ), + format!( + "sandbox_cpus={}", + self.sandbox_cpus.as_deref().unwrap_or("") + ), ]; let mut egress = normalize_hosts(self.allowed_egress_hosts.clone()) .into_iter() @@ -955,7 +998,17 @@ impl Proxy { audit_log: Option, proxy_port: Option, ) -> Result { - Self::start_inner(secrets, policy, audit_log, proxy_port, None, None, false).await + Self::start_inner( + secrets, + policy, + audit_log, + proxy_port, + None, + None, + false, + "127.0.0.1", + ) + .await } pub async fn start_with_hook( @@ -965,7 +1018,35 @@ impl Proxy { proxy_port: Option, api_key: Option, ) -> Result { - Self::start_inner(secrets, policy, audit_log, proxy_port, None, api_key, true).await + Self::start_inner( + secrets, + policy, + audit_log, + proxy_port, + None, + api_key, + true, + "127.0.0.1", + ) + .await + } + + /// Like `start_with_hook`, but binds the proxy to `bind_host` instead of + /// loopback. Used by the Docker sandbox backend, which binds the proxy + /// to a per-run Docker network's gateway address so only the sandboxed + /// container (not the whole LAN) can reach it. + pub async fn start_with_hook_and_bind_host( + secrets: HashMap, + policy: ProxyPolicy, + audit_log: Option, + proxy_port: Option, + api_key: Option, + bind_host: &str, + ) -> Result { + Self::start_inner( + secrets, policy, audit_log, proxy_port, None, api_key, true, bind_host, + ) + .await } pub async fn start_remote_with_port( @@ -983,6 +1064,7 @@ impl Proxy { Some(remote), None, false, + "127.0.0.1", ) .await } @@ -1003,6 +1085,31 @@ impl Proxy { Some(remote), api_key, true, + "127.0.0.1", + ) + .await + } + + /// Like `start_remote_with_hook`, but binds the proxy to `bind_host` + /// instead of loopback. See `start_with_hook_and_bind_host`. + pub async fn start_remote_with_hook_and_bind_host( + remote: RemoteProxyConfig, + policy: ProxyPolicy, + audit_log: Option, + proxy_port: Option, + api_key: Option, + bind_host: &str, + ) -> Result { + let placeholders = remote.placeholders.clone(); + Self::start_inner( + placeholders, + policy, + audit_log, + proxy_port, + Some(remote), + api_key, + true, + bind_host, ) .await } @@ -1015,6 +1122,7 @@ impl Proxy { remote: Option, hook_api_key: Option, hook_mode_set: bool, + bind_host: &str, ) -> Result { if proxy_port == Some(0) { anyhow::bail!("--proxy-port must be between 1 and 65535"); @@ -1034,7 +1142,7 @@ impl Proxy { ca_file = remote_ca; remove_ca_file = false; } - let bind_address = format!("127.0.0.1:{}", proxy_port.unwrap_or(0)); + let bind_address = format!("{bind_host}:{}", proxy_port.unwrap_or(0)); let listener = TcpListener::bind(&bind_address) .await .with_context(|| format!("failed to bind credential proxy to {bind_address}"))?; @@ -2808,7 +2916,7 @@ impl ProxyState { .read() .ok() .and_then(|path| path.clone()) - .is_some_and(|path| crate::handlers::agent_sessions::is_local_session_revoked(&path)) + .is_some_and(|path| crate::handlers::agent::sessions::is_local_session_revoked(&path)) } fn host_is_denied(&self, host: Option<&str>) -> bool { @@ -3440,6 +3548,11 @@ mod tests { tools: vec!["list_projects".to_owned()], }, ], + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, } } @@ -3819,6 +3932,11 @@ mod tests { egress_hosts_configured: true, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; let proxy = Proxy::start_remote_with_port(remote, policy, None, None) .await @@ -3945,6 +4063,11 @@ mod tests { egress_hosts_configured: true, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, } } @@ -4178,6 +4301,25 @@ mod tests { proxy.stop().await; } + #[tokio::test] + async fn proxy_binds_to_provided_host_instead_of_loopback() { + // 0.0.0.0 is used here only to prove the parameter is honored + // without depending on a real Docker network gateway in CI. + let proxy = Proxy::start_with_hook_and_bind_host( + HashMap::new(), + ProxyPolicy::permissive(), + None, + None, + None, + "0.0.0.0", + ) + .await + .unwrap(); + + assert!(proxy.child_env()["HTTP_PROXY"].starts_with("http://0.0.0.0:")); + proxy.stop().await; + } + #[tokio::test] async fn proxy_rejects_port_zero_override() { let result = @@ -4438,6 +4580,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; assert!(policy_allows_connect(&policy, "api.github.com")); @@ -4479,6 +4626,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, } } @@ -4528,6 +4680,38 @@ mod tests { assert_ne!(left.fingerprint(), different.fingerprint()); } + #[test] + fn policy_fingerprint_changes_with_sandbox_backend_and_settings() { + // The sandbox backend and its Docker-specific settings are part of + // the effective enforcement, not just the egress/secret policy — + // an audit record must be able to distinguish a native run from a + // Docker one, or one custom image from another, by fingerprint + // alone. + let native = rule_policy(Vec::new()); + let mut docker = native.clone(); + docker.backend = SandboxBackend::Docker; + assert_ne!(native.fingerprint(), docker.fingerprint()); + + let mut docker_image_a = docker.clone(); + docker_image_a.sandbox_image = Some("myorg/a:latest".to_owned()); + let mut docker_image_b = docker.clone(); + docker_image_b.sandbox_image = Some("myorg/b:latest".to_owned()); + assert_ne!(docker.fingerprint(), docker_image_a.fingerprint()); + assert_ne!(docker_image_a.fingerprint(), docker_image_b.fingerprint()); + + let mut docker_dockerfile = docker.clone(); + docker_dockerfile.sandbox_dockerfile = Some("./custom.Dockerfile".to_owned()); + assert_ne!(docker.fingerprint(), docker_dockerfile.fingerprint()); + + let mut docker_memory = docker.clone(); + docker_memory.sandbox_memory = Some("2g".to_owned()); + assert_ne!(docker.fingerprint(), docker_memory.fingerprint()); + + let mut docker_cpus = docker.clone(); + docker_cpus.sandbox_cpus = Some("1.5".to_owned()); + assert_ne!(docker.fingerprint(), docker_cpus.fingerprint()); + } + #[test] fn http_rules_allow_a_matching_request() { let policy = rule_policy(vec![rule( @@ -4611,6 +4795,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; assert!(secret_allows_request( &policy, @@ -4733,6 +4922,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; let proxy = Proxy::start( HashMap::from([("GITHUB_TOKEN".to_owned(), "real-token".to_owned())]), @@ -4773,6 +4967,11 @@ mod tests { egress_hosts_configured: true, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; assert!(policy_allows_egress(&policy, "example.com")); @@ -4799,6 +4998,11 @@ mod tests { egress_hosts_configured: true, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; let state = ProxyState { secrets: Arc::new(HashMap::new()), @@ -4864,6 +5068,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }; let proxy = Proxy::start( HashMap::from([("GH_TOKEN".to_owned(), "real-token".to_owned())]), @@ -5226,6 +5435,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }, None, ) @@ -5263,6 +5477,11 @@ mod tests { egress_hosts_configured: false, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }, None, ) @@ -5301,6 +5520,11 @@ mod tests { egress_hosts_configured: true, strict_deny: true, mcp_rules: Vec::new(), + backend: SandboxBackend::Native, + sandbox_image: None, + sandbox_dockerfile: None, + sandbox_memory: None, + sandbox_cpus: None, }, None, ) diff --git a/src/handlers/run/subprocess.rs b/src/handlers/run/subprocess.rs index 49e233ae..706a6c6f 100644 --- a/src/handlers/run/subprocess.rs +++ b/src/handlers/run/subprocess.rs @@ -1,13 +1,14 @@ use std::collections::HashMap; use std::env; +use std::io::IsTerminal; #[cfg(unix)] -use std::io::{IsTerminal, Read, Write}; +use std::io::{Read, Write}; #[cfg(unix)] use std::os::unix::io::{AsRawFd, FromRawFd}; use std::path::PathBuf; use std::process::ExitStatus; -use anyhow::Result; +use anyhow::{Context, Result}; use duct::{cmd, Expression}; use thiserror::Error; @@ -65,6 +66,7 @@ pub async fn run_command( &[], &[], None, + crate::models::agent::SandboxBackend::Native, ) .await } @@ -81,8 +83,91 @@ pub async fn run_command_with_filesystem_policy( denied_read_paths: &[String], denied_write_paths: &[String], audit_log: Option, + backend: crate::models::agent::SandboxBackend, +) -> Result { + run_command_with_filesystem_policy_and_network( + command, + args, + env_vars, + env_removals, + sandbox, + allow_network_listeners, + proxy_mode, + restrict_stashbase_credentials, + denied_read_paths, + denied_write_paths, + audit_log, + backend, + None, + super::docker_sandbox::DEFAULT_SANDBOX_IMAGE, + None, + None, + ) + .await +} + +/// Like `run_command_with_filesystem_policy`, but for the Docker backend +/// takes an already-created per-run network instead of creating its own — +/// the caller creates it once and binds the credential proxy to the same +/// network's gateway, so the proxy and the container agree on which +/// network they share. `docker_network` must be `Some` when `backend` is +/// `SandboxBackend::Docker`; it is ignored for the native backend. +#[allow(clippy::too_many_arguments)] +pub async fn run_command_with_filesystem_policy_and_network( + command: &str, + args: Vec, + env_vars: HashMap, + env_removals: Vec, + sandbox: bool, + allow_network_listeners: bool, + proxy_mode: bool, + restrict_stashbase_credentials: bool, + denied_read_paths: &[String], + denied_write_paths: &[String], + audit_log: Option, + backend: crate::models::agent::SandboxBackend, + docker_network: Option<&super::docker_sandbox::DockerRunNetwork>, + agent_image: &str, + sandbox_memory: Option<&str>, + sandbox_cpus: Option<&str>, ) -> Result { let current_dir = env::current_dir()?; + + if backend == crate::models::agent::SandboxBackend::Docker { + if let Some(error) = super::docker_sandbox::docker_enforcement_error() { + anyhow::bail!("Docker sandbox backend unavailable: {error}"); + } + let network = + docker_network.context("Docker sandbox backend selected without a per-run network")?; + let args = codex_args_forcing_full_access(command, args); + let (program, launcher_args) = super::docker_sandbox::docker_run_command( + command, + network, + denied_read_paths, + denied_write_paths, + &env_vars, + std::io::stdin().is_terminal(), + agent_image, + sandbox_memory, + sandbox_cpus, + ) + .map_err(|error| anyhow::anyhow!("failed to build Docker sandbox invocation: {error}"))?; + return run_built_command( + program, + launcher_args, + args, + env_vars, + env_removals, + restrict_stashbase_credentials, + proxy_mode, + denied_read_paths, + denied_write_paths, + audit_log, + current_dir, + ) + .await; + } + #[cfg(target_os = "macos")] let (args, codex_boundary) = codex_args_with_outer_sandbox( command, @@ -100,6 +185,44 @@ pub async fn run_command_with_filesystem_policy( denied_write_paths, codex_boundary, )?; + run_built_command( + program, + launcher_args, + args, + env_vars, + env_removals, + restrict_stashbase_credentials, + proxy_mode, + denied_read_paths, + denied_write_paths, + audit_log, + current_dir, + ) + .await +} + +#[allow(clippy::too_many_arguments)] +async fn run_built_command( + program: String, + launcher_args: Vec, + args: Vec, + env_vars: HashMap, + env_removals: Vec, + restrict_stashbase_credentials: bool, + proxy_mode: bool, + denied_read_paths: &[String], + denied_write_paths: &[String], + audit_log: Option, + current_dir: PathBuf, +) -> Result { + // NOTE: for the Docker backend, `program` is `docker` — every + // `cmd.env_remove`/`cmd.env` call below acts on the `docker` CLI + // process's own environment, not the container's. The container only + // ever sees what `docker_run_command` explicitly passed via `-e`, so + // these removals are inert (harmlessly so, since nothing is inherited + // into the container to begin with) for that path. Do not rely on this + // block to enforce a Docker-path env restriction — enforce it in + // `docker_run_command` instead. let cmd: Expression = cmd(program, launcher_args) .before_spawn(move |cmd| { // Agent profiles may rename a project secret for the child process. @@ -317,6 +440,60 @@ fn codex_boundary_for_mode(mode: &str) -> CodexSandboxBoundary { } } +/// Rewrites Codex's own `--sandbox ` argument to `danger-full-access` +/// (or injects it if absent), the same way `codex_args_with_outer_sandbox` +/// does for the native macOS backend when an outer Seatbelt profile is +/// already active. Unlike that macOS-only helper, this one runs +/// unconditionally for the Docker backend on every host platform: the +/// Docker container is *always* an outer sandbox once selected, and +/// Codex's own inner sandbox (bubblewrap on Linux, Seatbelt on macOS) is +/// both redundant — the container is already the enforcement boundary — +/// and, for bubblewrap specifically, non-functional inside a container +/// that has already dropped all capabilities (`bwrap` needs to create a +/// new user/mount namespace, which `--cap-drop ALL` blocks outright). +/// Codex's approval policy is left untouched; only its own filesystem +/// sandboxing is disabled. +fn codex_args_forcing_full_access(command: &str, mut args: Vec) -> Vec { + let is_codex = PathBuf::from(command) + .file_stem() + .is_some_and(|name| name.eq_ignore_ascii_case("codex")); + if !is_codex { + return args; + } + if args + .iter() + .any(|arg| arg == "--dangerously-bypass-approvals-and-sandbox") + { + return args; + } + let mut found_sandbox = false; + let mut index = 0; + while index < args.len() { + if args[index] == "--sandbox" { + if let Some(mode) = args.get_mut(index + 1) { + *mode = "danger-full-access".to_owned(); + } else { + args.push("danger-full-access".to_owned()); + } + found_sandbox = true; + index += 2; + } else if args[index].starts_with("--sandbox=") { + args[index] = "--sandbox=danger-full-access".to_owned(); + found_sandbox = true; + index += 1; + } else { + index += 1; + } + } + if !found_sandbox { + args.splice( + 0..0, + ["--sandbox".to_owned(), "danger-full-access".to_owned()], + ); + } + args +} + fn should_inherit_terminal_streams(stdin_is_terminal: bool, stderr_is_terminal: bool) -> bool { stdin_is_terminal && stderr_is_terminal } @@ -364,11 +541,8 @@ fn sandbox_command_with_filesystem_policy( if let Some(error) = bubblewrap_enforcement_error() { anyhow::bail!(error); } - return Ok(bubblewrap_command( - command, - denied_read_paths, - denied_write_paths, - )); + return bubblewrap_command(command, denied_read_paths, denied_write_paths) + .map_err(|error| anyhow::anyhow!(error)); } } #[cfg(not(any(target_os = "macos", target_os = "linux")))] @@ -445,11 +619,8 @@ fn sandbox_command_with_filesystem_policy( if let Some(error) = bubblewrap_enforcement_error() { anyhow::bail!(error); } - return Ok(bubblewrap_command( - command, - denied_read_paths, - denied_write_paths, - )); + return bubblewrap_command(command, denied_read_paths, denied_write_paths) + .map_err(|error| anyhow::anyhow!(error)); } if !sandbox && denied_read_paths.is_empty() && denied_write_paths.is_empty() { return Ok((command.to_owned(), Vec::new())); @@ -530,7 +701,7 @@ fn denied_file_rules(deny_read: &[String], deny_write: &[String]) -> String { } } -fn resolve_policy_paths(paths: &[String]) -> Vec { +pub(super) fn resolve_policy_paths(paths: &[String]) -> Vec { let home = env::var_os("HOME").map(PathBuf::from); let current_dir = env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let mut resolved = paths @@ -812,7 +983,7 @@ fn bubblewrap_command( command: &str, denied_read_paths: &[String], denied_write_paths: &[String], -) -> (String, Vec) { +) -> Result<(String, Vec), String> { let executable = if command_in_path("bwrap") { "bwrap" } else { @@ -839,7 +1010,12 @@ fn bubblewrap_command( if PathBuf::from(path).is_dir() { args.extend(["--tmpfs".to_owned(), path.clone()]); } else { - args.extend(["--ro-bind".to_owned(), "/dev/null".to_owned(), path.clone()]); + // Not `/dev/null`: `--ro-bind` preserves the *source's* file + // type at the target, and `/dev/null` is a character device, + // not a regular file — see `empty_shadow_file_path`'s doc + // comment for why that matters in practice. + let shadow_file = super::docker_sandbox::empty_shadow_file_path()?; + args.extend(["--ro-bind".to_owned(), shadow_file, path.clone()]); } } for path in resolve_policy_paths(denied_write_paths) { @@ -853,7 +1029,7 @@ fn bubblewrap_command( args.extend(["--ro-bind".to_owned(), path.clone(), path]); } args.extend(["--".to_owned(), command.to_owned()]); - (executable.to_owned(), args) + Ok((executable.to_owned(), args)) } #[cfg(target_os = "linux")] @@ -931,15 +1107,15 @@ pub(crate) fn filesystem_enforcement_error() -> Option { #[cfg(all(test, unix))] mod tests { + use super::{ + codex_args_forcing_full_access, filesystem_backend_for_policy, filesystem_denial_from_line, + run_command, sandbox_command, should_inherit_terminal_streams, + }; #[cfg(target_os = "macos")] use super::{ codex_args_with_outer_sandbox, codex_workspace_rules, escape_sbpl_path, has_outer_macos_sandbox, sandbox_command_with_filesystem_policy, CodexSandboxBoundary, }; - use super::{ - filesystem_backend_for_policy, filesystem_denial_from_line, run_command, sandbox_command, - should_inherit_terminal_streams, - }; use std::collections::HashMap; use std::sync::{Mutex, OnceLock}; @@ -948,6 +1124,76 @@ mod tests { LOCK.get_or_init(|| Mutex::new(())) } + #[test] + fn codex_args_forcing_full_access_leaves_non_codex_commands_untouched() { + let args = vec!["--sandbox".to_owned(), "workspace-write".to_owned()]; + let result = codex_args_forcing_full_access("claude", args.clone()); + assert_eq!(result, args); + } + + #[test] + fn codex_args_forcing_full_access_rewrites_existing_sandbox_flag() { + let args = vec![ + "exec".to_owned(), + "--sandbox".to_owned(), + "workspace-write".to_owned(), + ]; + let result = codex_args_forcing_full_access("codex", args); + assert_eq!( + result, + vec![ + "exec".to_owned(), + "--sandbox".to_owned(), + "danger-full-access".to_owned(), + ] + ); + } + + #[test] + fn codex_args_forcing_full_access_rewrites_equals_form() { + let args = vec!["exec".to_owned(), "--sandbox=read-only".to_owned()]; + let result = codex_args_forcing_full_access("codex", args); + assert_eq!( + result, + vec!["exec".to_owned(), "--sandbox=danger-full-access".to_owned()] + ); + } + + #[test] + fn codex_args_forcing_full_access_injects_flag_when_absent() { + let args = vec!["exec".to_owned(), "echo hi".to_owned()]; + let result = codex_args_forcing_full_access("codex", args); + assert_eq!( + result, + vec![ + "--sandbox".to_owned(), + "danger-full-access".to_owned(), + "exec".to_owned(), + "echo hi".to_owned(), + ] + ); + } + + #[test] + fn codex_args_forcing_full_access_respects_explicit_bypass_flag() { + let args = vec![ + "exec".to_owned(), + "--dangerously-bypass-approvals-and-sandbox".to_owned(), + ]; + let result = codex_args_forcing_full_access("codex", args.clone()); + assert_eq!(result, args); + } + + #[test] + fn codex_args_forcing_full_access_matches_codex_regardless_of_path() { + let args = vec!["--sandbox".to_owned(), "workspace-write".to_owned()]; + let result = codex_args_forcing_full_access("/usr/local/bin/codex", args); + assert_eq!( + result, + vec!["--sandbox".to_owned(), "danger-full-access".to_owned()] + ); + } + #[cfg(target_os = "linux")] #[test] fn detects_wsl_kernel_releases() { @@ -1434,7 +1680,8 @@ mod tests { "sh", &[private_dir.clone(), private_file.clone()], &[readonly_dir, readonly_file.clone()], - ); + ) + .unwrap(); assert!(program == "bwrap" || program == "bubblewrap"); assert!(args.windows(2).any(|window| { window @@ -1442,12 +1689,25 @@ mod tests { .map(String::as_str) .eq(["--tmpfs", private_dir.as_str()]) })); + // Not `/dev/null`: `--ro-bind` preserves the *source's* file type + // at the target, and `/dev/null` is a character device, not a + // regular file — see `empty_shadow_file_path`'s doc comment. + let expected_source = + crate::handlers::run::docker_sandbox::empty_shadow_file_path().unwrap(); assert!(args.windows(3).any(|window| { - window - .iter() - .map(String::as_str) - .eq(["--ro-bind", "/dev/null", private_file.as_str()]) + window.iter().map(String::as_str).eq([ + "--ro-bind", + expected_source.as_str(), + private_file.as_str(), + ]) })); + assert!(std::path::Path::new(&expected_source).is_file()); + { + use std::os::unix::fs::FileTypeExt; + let file_type = std::fs::metadata(&expected_source).unwrap().file_type(); + assert!(!file_type.is_char_device()); + assert!(!file_type.is_block_device()); + } assert!(args.windows(3).any(|window| { window.iter().map(String::as_str).eq([ "--ro-bind", diff --git a/src/models/agent.rs b/src/models/agent.rs index 8e47b2c8..f1a8c7bd 100644 --- a/src/models/agent.rs +++ b/src/models/agent.rs @@ -16,6 +16,10 @@ pub struct AgentProfile { /// Filesystem paths denied to the agent process tree. #[serde(default)] pub filesystem: AgentFilesystemProfile, + /// Selects the sandbox enforcement backend. Defaults to the platform's + /// native mechanism (Seatbelt/systemd-run/bubblewrap). + #[serde(default)] + pub sandbox: AgentSandboxProfile, /// Named HTTP MCP servers and their tool policies. #[serde(default)] pub mcp_servers: HashMap, @@ -81,6 +85,58 @@ pub struct AgentFilesystemProfile { pub deny_write: Vec, } +/// Selects which mechanism enforces filesystem/network isolation for the +/// agent process tree. `Native` (the default) preserves today's behavior: +/// Seatbelt on macOS, `systemd-run`/bubblewrap on Linux. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum SandboxBackend { + #[default] + Native, + Docker, +} + +impl SandboxBackend { + /// Applies a `--docker-sandbox` CLI override on top of this profile's + /// declared backend: `Some(true)` forces `Docker`, `Some(false)` forces + /// `Native`, `None` (the flag wasn't passed) leaves the profile's own + /// setting untouched. + pub fn with_cli_override(self, docker_sandbox_flag: Option) -> Self { + match docker_sandbox_flag { + Some(true) => Self::Docker, + Some(false) => Self::Native, + None => self, + } + } +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct AgentSandboxProfile { + #[serde(default)] + pub backend: SandboxBackend, + /// Docker backend only: run this image instead of the built-in default. + /// Mutually exclusive with `dockerfile` — see `ensure_profile_is_valid_for_run`. + #[serde(default)] + pub image: Option, + /// Docker backend only: build and run this Dockerfile (path relative to + /// the current working directory) instead of the built-in default image. + /// Mutually exclusive with `image`. + #[serde(default)] + pub dockerfile: Option, + /// Docker backend only: cap the agent container's memory, passed + /// straight through to `docker run --memory` (e.g. "2g", "512m"). No + /// cap by default — this is opt-in, since an automatic default could + /// silently break a legitimately memory-hungry task with no warning. + #[serde(default)] + pub memory: Option, + /// Docker backend only: cap the agent container's CPU allocation, + /// passed straight through to `docker run --cpus` (e.g. "1.5", "2"). + /// No cap by default, for the same reason as `memory`. + #[serde(default)] + pub cpus: Option, +} + /// Project/environment-backed secret bindings. Personal credentials deliberately /// live outside this table because they are owned by the authenticated account. #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -167,3 +223,77 @@ pub enum AgentHttpRuleEffect { Allow, Deny, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cli_override_forces_docker_regardless_of_profile() { + assert_eq!( + SandboxBackend::Native.with_cli_override(Some(true)), + SandboxBackend::Docker + ); + assert_eq!( + SandboxBackend::Docker.with_cli_override(Some(true)), + SandboxBackend::Docker + ); + } + + #[test] + fn cli_override_forces_native_regardless_of_profile() { + assert_eq!( + SandboxBackend::Docker.with_cli_override(Some(false)), + SandboxBackend::Native + ); + assert_eq!( + SandboxBackend::Native.with_cli_override(Some(false)), + SandboxBackend::Native + ); + } + + #[test] + fn cli_override_absent_keeps_profile_setting() { + assert_eq!( + SandboxBackend::Docker.with_cli_override(None), + SandboxBackend::Docker + ); + assert_eq!( + SandboxBackend::Native.with_cli_override(None), + SandboxBackend::Native + ); + } + + #[test] + fn sandbox_defaults_to_native_when_omitted() { + let toml = r#" + egress_hosts = ["api.github.com"] + "#; + let profile: AgentProfile = toml::from_str(toml).unwrap(); + assert_eq!(profile.sandbox.backend, SandboxBackend::Native); + } + + #[test] + fn sandbox_backend_docker_parses() { + let toml = r#" + egress_hosts = ["api.github.com"] + + [sandbox] + backend = "docker" + "#; + let profile: AgentProfile = toml::from_str(toml).unwrap(); + assert_eq!(profile.sandbox.backend, SandboxBackend::Docker); + } + + #[test] + fn sandbox_rejects_unknown_backend() { + let toml = r#" + egress_hosts = ["api.github.com"] + + [sandbox] + backend = "vm" + "#; + let result: Result = toml::from_str(toml); + assert!(result.is_err()); + } +}