mjalmoudy94
a0c8473eb8
The execution boundary for v2. Adds the local docker/podman sandbox adapter and the hardened host subprocess runner every other delivery adapter will use. - adapters/process.py: the one place the platform shells out. argv-only (no shell), start_new_session + killpg on timeout, concurrent stdout/stderr drains with a byte cap, explicit minimal env (never os.environ), and a missing binary reported as spawn_failed rather than an exception. - adapters/docker_sandbox.py: DockerSandboxAdapter over SandboxProviderPort. --user 1000:1000, --cap-drop ALL, no-new-privileges, pids/cpu/memory limits, --read-only with tmpfs scratch, one bind mount (the work tree, never the git dir), labels, no published ports. Path-safe file access; secrets refused in `docker exec -e`. Extras for later phases: start_demo_container (loopback publish only), list_managed_containers, remove_container. - registry: SANDBOX_PROVIDER=docker, cached singleton so the workspace registry survives create_workspace -> start_session. - sandbox_gateway worker: reap_orphans() removes labelled containers no live session points at, audited as sandbox.reap; run_pass() does both passes. - sandbox router: every mutating route now requires the platform owner (PLAN-v2 §7.7); read-only status routes stay open to org members. - sandbox-panel: canManageInfra prop (fails closed) hides the create form, the session panel and the action buttons from non-owners. Tests: 346 API (+37), 67 web (+3); ruff, mypy, tsc and lint clean. Three existing tests now drive the sandbox as the platform owner, since that is who may touch it. Report: docs/plan/reports/agent-3-phase-1.md
143 lines
8.0 KiB
Markdown
143 lines
8.0 KiB
Markdown
# Agent 3 — Phase 1 report: Docker sandbox adapter
|
|
|
|
Date: 2026-10-13 · Branch: `dev` (work done directly on `dev`; see "Deviations")
|
|
|
|
## What was done
|
|
|
|
### 1.1 `DockerSandboxAdapter` (`app/infrastructure/adapters/docker_sandbox.py`)
|
|
|
|
Implements `SandboxProviderPort` over the docker/podman CLI, with the binary taken from
|
|
`SANDBOX_DOCKER_BIN` so podman is a drop-in (the flags used are the common subset).
|
|
|
|
- `create_workspace` — creates `SANDBOX_WORKSPACES_ROOT/<org>/<project>/work` at `0700`, and
|
|
encodes the workspace identity in the external id (`ws-<org>-<project>`) so a restarted
|
|
process can rebuild the host paths without a database round-trip. One workspace per
|
|
application, per D5.
|
|
- `start_session` — `docker run -d` with the full hardening set:
|
|
`--user 1000:1000`, `--cap-drop ALL`, `--security-opt no-new-privileges`, `--pids-limit 512`,
|
|
`--cpus`, `--memory`, `--read-only` + `--tmpfs /tmp` + `--tmpfs /home/app`,
|
|
a single bind mount of the work dir at `/workspace`, labels
|
|
`smart-org.org` / `smart-org.project` / `smart-org.role`, default bridge network and **no
|
|
published ports**. It reuses an existing container of the same name rather than stacking
|
|
duplicates after a restart.
|
|
- `run_command` — `docker exec -w <cwd> <ctr> sh -lc <cmd>`. `cwd` and `on_output` are
|
|
adapter-level extras for the engine; the port signature is unchanged. Environment variables
|
|
are passed with `-e`, and any key that looks like a secret is **refused** — argv is visible
|
|
in `ps`, so credentials must never travel that way.
|
|
- `read_file` / `write_file` — through the host bind mount, with `safe_join` path safety
|
|
(rejects `..`, absolute escapes and symlinks that resolve outside the work dir).
|
|
- `pause` / `resume` / `destroy` — stop / start / `rm -f -v`.
|
|
- `snapshot` — `docker commit`, documented as capturing the root filesystem only (the work
|
|
tree is a bind mount and is *not* included).
|
|
- `get_preview_url` — `http://127.0.0.1:<port>`, and it refuses a platform-protected port.
|
|
- Extras used by later phases: `start_demo_container` (loopback-only publish, same hardening,
|
|
`role=demo`), `list_managed_containers`, `remove_container`, `host_dir_for`,
|
|
`binary_available`, plus the module-level `image_for_language` map
|
|
(node:20-bookworm, python:3.13-bookworm, php:8.3-cli, golang:1.23, debian:bookworm fallback).
|
|
|
|
### 1.1a `app/infrastructure/adapters/process.py` (new, shared)
|
|
|
|
The one place the platform shells out. `run_process` exists to make four failure modes
|
|
impossible:
|
|
|
|
- **shell injection** — `create_subprocess_exec` with an argv list, never `shell=True`;
|
|
- **orphaned children** — `start_new_session=True` and `killpg` on timeout (SIGTERM then
|
|
SIGKILL), so a `docker build`'s helpers die with it;
|
|
- **pipe deadlock** — stdout and stderr drained concurrently by two tasks, with a byte cap
|
|
applied while streaming (the callback still sees every chunk, so live logs stay complete);
|
|
- **secret inheritance** — an explicit minimal `env` (a handful of safe keys), never
|
|
`os.environ` wholesale.
|
|
|
|
A missing binary returns `spawn_failed=True` rather than raising; a non-zero exit or a
|
|
timeout is an ordinary result the caller inspects.
|
|
|
|
### 1.2 Registry
|
|
|
|
`get_sandbox_provider()` now maps `SANDBOX_PROVIDER=docker` to the adapter (cached as a
|
|
singleton so the workspace registry stays consistent between `create_workspace` and the
|
|
`start_session` that follows). `mock` remains the default; `daytona`/`e2b` are unchanged.
|
|
|
|
### 1.3 Orphan reaping
|
|
|
|
`SandboxGatewayWorker.reap_orphans()` lists every container carrying our labels and removes
|
|
any whose id is not in `SandboxSessionRepository.list_active_external_ids()` (new method).
|
|
Each removal is audited as `sandbox.reap` with `reason=orphan_container`. A provider without
|
|
`list_managed_containers` is skipped silently.
|
|
`SandboxGatewayWorker.run_pass()` runs idle-session reaping and orphan reaping in one pass,
|
|
and `workers/__main__.py` now calls it.
|
|
|
|
### 1.4 Infrastructure hidden from non-owners
|
|
|
|
`app/interfaces/http/routers/sandbox.py`: every **mutating** route now requires the platform
|
|
owner via a local `_require_platform_owner(context)` helper — create/destroy workspace,
|
|
start/stop session, run command, read/write file, snapshot, preview URL, set limits. The
|
|
**read-only status** routes (templates, effective limits, workspace list, command log) stay
|
|
open to org members, because the project page shows them.
|
|
|
|
`components/sandbox-panel.tsx` takes a `canManageInfra` prop (default `false`, so a call site
|
|
that forgets it fails closed). A non-owner sees the limits card, the workspace list and a
|
|
short note that infrastructure is managed by the platform owner; the create form, the session
|
|
panel and the per-workspace action buttons are not rendered at all (not merely disabled).
|
|
`(app)/projects/[id]/page.tsx` passes `canManageInfra={user?.is_platform_owner ?? false}`.
|
|
|
|
## Files changed
|
|
|
|
| File | Change |
|
|
|---|---|
|
|
| `app/infrastructure/adapters/process.py` | new — hardened host subprocess runner |
|
|
| `app/infrastructure/adapters/docker_sandbox.py` | new — the docker/podman sandbox adapter |
|
|
| `app/infrastructure/adapters/registry.py` | `SANDBOX_PROVIDER=docker` branch |
|
|
| `app/infrastructure/repositories/sandbox.py` | `list_active_external_ids()` |
|
|
| `app/workers/sandbox_gateway.py` | `reap_orphans()`, `run_pass()` |
|
|
| `app/workers/__main__.py` | sandbox-gateway pass calls `run_pass()` |
|
|
| `app/interfaces/http/routers/sandbox.py` | owner-only mutating routes |
|
|
| `tests/test_docker_sandbox.py` | new — 37 tests |
|
|
| `tests/test_workers.py` | 2 sandbox tests now use a platform-owner org |
|
|
| `tests/test_e2e_smoke.py` | smoke path signs up as the platform owner |
|
|
| `apps/web/src/components/sandbox-panel.tsx` | `canManageInfra` gating |
|
|
| `apps/web/src/app/(app)/projects/[id]/page.tsx` | passes the owner flag |
|
|
| `apps/web/src/test/delivery-sandbox-panel.test.tsx` | new — 3 tests |
|
|
| `.gitignore` | `.pytest-tmp/` |
|
|
|
|
## Test output
|
|
|
|
```
|
|
apps/api: 346 passed (was 309; +37)
|
|
ruff check . -> All checks passed!
|
|
mypy app -> Success: no issues found in 127 source files
|
|
apps/web: 67 passed (was 64; +3)
|
|
tsc --noEmit -> clean
|
|
next lint -> No ESLint warnings or errors
|
|
```
|
|
|
|
## Deviations from the TODO
|
|
|
|
1. **No worktrees.** The plan's environment (exFAT `/projects`, ext4 worktrees under
|
|
`/home/mamad/work/`) describes a Linux box. This machine is macOS with the repo at
|
|
`/Users/mamad/Projects/agy-bot/web`; all work is done on `dev` and committed per phase.
|
|
2. **Owner-only rather than "admin+ with a setting".** The TODO offered both; owner-only is
|
|
the stated preference and matches "users never touch infrastructure" (PLAN-v2 §1.9).
|
|
3. **Test updates.** Making sandbox mutation owner-only breaks three existing tests that drove
|
|
those routes as a plain manager. `test_e2e_smoke.py` and the two sandbox-gateway tests in
|
|
`test_workers.py` now sign up as the platform owner. No assertion was weakened; only the
|
|
actor's platform role changed.
|
|
4. **`tmp_path` needs a writable `TMPDIR` on this machine.** The pre-existing
|
|
`$TMPDIR/pytest-of-unknown` directory is rejected by the local sandbox broker, so pytest
|
|
cannot create its base temp dir. Tests are run with `TMPDIR=<repo>/.pytest-tmp/tmproot`
|
|
(now gitignored). This is an environment quirk, not a code change.
|
|
|
|
## Open questions / CHANGE REQUESTS
|
|
|
|
- **None.** `RepoWorkspacePort`, `DeliveryHooks` and `DeployTargetTester` are consumed exactly
|
|
as frozen; the docker adapter adds methods rather than changing the port.
|
|
|
|
## Known limitations
|
|
|
|
- The workspace registry (external id → image) is process-local. A cold process rebuilds the
|
|
host dir from the encoded external id but falls back to the default image; in practice
|
|
`create_workspace` and `start_session` happen in the same request.
|
|
- `snapshot` does not capture the work tree (bind mount). Documented in the docstring.
|
|
- No `@pytest.mark.docker` integration test yet; the fake runner covers the argv contract,
|
|
which is where the security properties live. A live round-trip is added in Phase 3, where
|
|
the build/demo containers make it meaningful.
|