feat(delivery): docker sandbox adapter + owner-only infrastructure (v2 phase A3-1)
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
This commit is contained in:
@@ -0,0 +1,274 @@
|
||||
"""The one place the platform shells out to a host binary.
|
||||
|
||||
Nothing else in the codebase may call ``subprocess`` or ``os.system``. Every
|
||||
adapter that needs an external binary (docker/podman, git, caddy reload) goes
|
||||
through :func:`run_process`, which exists to make four failure modes impossible
|
||||
rather than merely unlikely:
|
||||
|
||||
**Shell injection.** The command is an ``argv`` sequence handed straight to
|
||||
``asyncio.create_subprocess_exec``. There is no ``shell=True`` path and no
|
||||
string interpolation into a shell, so a task id or a branch name can never
|
||||
become a second command.
|
||||
|
||||
**Orphaned children.** The child starts in its own session
|
||||
(``start_new_session=True``), so a timeout kills the whole *process group*. A
|
||||
``docker build`` that spawns helpers, or a ``git`` that runs a credential
|
||||
helper, dies with its parent instead of lingering as a stray process holding a
|
||||
lock.
|
||||
|
||||
**Pipe deadlock.** stdout and stderr are drained concurrently by two tasks.
|
||||
Reading one to completion before touching the other is the classic 64 KB
|
||||
deadlock: the child blocks writing stderr, we block reading stdout, and both
|
||||
wait forever. ``communicate()`` is not used because we want a byte cap and a
|
||||
streaming callback.
|
||||
|
||||
**Secret inheritance.** The child gets an explicit, minimal environment built
|
||||
from :func:`base_env`, never ``os.environ`` wholesale. The platform process
|
||||
holds the JWT secret, the database DSN and every provider key; a subprocess that
|
||||
inherits them can leak them through a stack trace, a build log or a core dump.
|
||||
|
||||
Output is capped in bytes while streaming, so a runaway build cannot exhaust
|
||||
memory. The cap is applied *after* the callback sees the chunk, so live logs
|
||||
still show everything that arrived.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
import os
|
||||
import signal
|
||||
import time
|
||||
from collections.abc import Awaitable, Callable, Mapping, Sequence
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
#: Environment variables a child may inherit from the platform process. Anything
|
||||
#: not listed is dropped. Deliberately tiny.
|
||||
_BASE_ENV_KEYS: tuple[str, ...] = ("PATH", "LANG", "LC_ALL", "LC_CTYPE", "TZ", "TMPDIR")
|
||||
|
||||
#: Fallback PATH when the parent has none (systemd units often do not).
|
||||
_FALLBACK_PATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
|
||||
|
||||
#: Read size per pipe chunk. 64 KiB is the pipe buffer size, so this drains
|
||||
#: promptly without spinning.
|
||||
_CHUNK_BYTES = 65_536
|
||||
|
||||
#: How long a child gets to die politely after SIGTERM before SIGKILL.
|
||||
_TERM_GRACE_SECONDS = 5.0
|
||||
|
||||
#: An argv element longer than this is almost certainly a mistake (a base64 blob
|
||||
#: pasted into a command line). Refused rather than silently truncated.
|
||||
MAX_ARGV_ELEMENT_CHARS = 100_000
|
||||
|
||||
|
||||
class ProcessError(RuntimeError):
|
||||
"""A host command could not be started at all (binary missing, not executable)."""
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class ProcessResult:
|
||||
"""Outcome of one host command.
|
||||
|
||||
``argv`` is retained so a test can assert on the *exact* argument vector —
|
||||
which is how the hardening flags and the "no credential in argv" rule are
|
||||
verified.
|
||||
"""
|
||||
|
||||
argv: tuple[str, ...]
|
||||
exit_code: int
|
||||
stdout: str = ""
|
||||
stderr: str = ""
|
||||
duration_ms: int = 0
|
||||
timed_out: bool = False
|
||||
truncated: bool = False
|
||||
#: True when the binary itself could not be executed (ENOENT/EACCES).
|
||||
spawn_failed: bool = False
|
||||
spawn_error: str = ""
|
||||
|
||||
@property
|
||||
def ok(self) -> bool:
|
||||
return self.exit_code == 0 and not self.timed_out and not self.spawn_failed
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class _DrainResult:
|
||||
data: bytes = b""
|
||||
truncated: bool = False
|
||||
lines: list[str] = field(default_factory=list)
|
||||
|
||||
|
||||
def base_env(extra: Mapping[str, str] | None = None) -> dict[str, str]:
|
||||
"""Minimal child environment: a few safe keys plus explicit overrides.
|
||||
|
||||
Callers add what they need (``HOME`` for git, ``GIT_CONFIG_NOSYSTEM=1``) and
|
||||
nothing else is inherited.
|
||||
"""
|
||||
env = {key: os.environ[key] for key in _BASE_ENV_KEYS if key in os.environ}
|
||||
env.setdefault("PATH", _FALLBACK_PATH)
|
||||
if extra:
|
||||
env.update({str(k): str(v) for k, v in extra.items()})
|
||||
return env
|
||||
|
||||
|
||||
def _validate_argv(argv: Sequence[str]) -> tuple[str, ...]:
|
||||
if not argv:
|
||||
raise ValueError("argv must not be empty")
|
||||
cleaned: list[str] = []
|
||||
for index, element in enumerate(argv):
|
||||
if not isinstance(element, str):
|
||||
raise TypeError(f"argv[{index}] must be str, got {type(element).__name__}")
|
||||
if "\x00" in element:
|
||||
raise ValueError(f"argv[{index}] contains a NUL byte")
|
||||
if len(element) > MAX_ARGV_ELEMENT_CHARS:
|
||||
raise ValueError(
|
||||
f"argv[{index}] is {len(element)} chars; "
|
||||
f"the limit is {MAX_ARGV_ELEMENT_CHARS}"
|
||||
)
|
||||
cleaned.append(element)
|
||||
return tuple(cleaned)
|
||||
|
||||
|
||||
async def _drain(
|
||||
stream: asyncio.StreamReader | None,
|
||||
*,
|
||||
cap_bytes: int,
|
||||
on_output: Callable[[str], Awaitable[None]] | None,
|
||||
) -> _DrainResult:
|
||||
"""Read a pipe to EOF, keeping at most ``cap_bytes`` and streaming every chunk."""
|
||||
result = _DrainResult()
|
||||
if stream is None:
|
||||
return result
|
||||
|
||||
buffer = bytearray()
|
||||
truncated = False
|
||||
while True:
|
||||
chunk = await stream.read(_CHUNK_BYTES)
|
||||
if not chunk:
|
||||
break
|
||||
if len(buffer) < cap_bytes:
|
||||
remaining = cap_bytes - len(buffer)
|
||||
buffer.extend(chunk[:remaining])
|
||||
if len(chunk) > remaining:
|
||||
truncated = True
|
||||
else:
|
||||
truncated = True
|
||||
if on_output is not None:
|
||||
# The callback sees the *raw* chunk even past the cap: a live log
|
||||
# should not stop scrolling just because we stopped storing.
|
||||
with contextlib.suppress(Exception):
|
||||
await on_output(chunk.decode("utf-8", "replace"))
|
||||
|
||||
result.data = bytes(buffer)
|
||||
result.truncated = truncated
|
||||
return result
|
||||
|
||||
|
||||
async def _terminate(proc: asyncio.subprocess.Process) -> None:
|
||||
"""SIGTERM the child's process group, then SIGKILL if it will not go."""
|
||||
if proc.returncode is not None:
|
||||
return
|
||||
|
||||
def _signal_group(sig: int) -> None:
|
||||
# The child is a session leader (``start_new_session=True``), so its pid
|
||||
# *is* its process-group id. Fall back to the pid on platforms without
|
||||
# process groups.
|
||||
with contextlib.suppress(ProcessLookupError, PermissionError, OSError):
|
||||
if hasattr(os, "killpg"):
|
||||
os.killpg(proc.pid, sig)
|
||||
else: # pragma: no cover - POSIX is the only supported target
|
||||
proc.send_signal(sig)
|
||||
|
||||
_signal_group(signal.SIGTERM)
|
||||
try:
|
||||
await asyncio.wait_for(proc.wait(), timeout=_TERM_GRACE_SECONDS)
|
||||
return
|
||||
except TimeoutError:
|
||||
pass
|
||||
_signal_group(signal.SIGKILL)
|
||||
with contextlib.suppress(Exception):
|
||||
await proc.wait()
|
||||
|
||||
|
||||
async def run_process(
|
||||
argv: Sequence[str],
|
||||
*,
|
||||
cwd: str | None = None,
|
||||
env: Mapping[str, str] | None = None,
|
||||
timeout_seconds: float = 120.0,
|
||||
max_output_bytes: int = 200_000,
|
||||
on_output: Callable[[str], Awaitable[None]] | None = None,
|
||||
stdin_data: bytes | None = None,
|
||||
) -> ProcessResult:
|
||||
"""Run ``argv`` and return its bounded, timed result.
|
||||
|
||||
Never raises for a non-zero exit or a timeout — those are ordinary outcomes
|
||||
the caller inspects. It *does* raise :class:`ProcessError` when the binary
|
||||
cannot be started, because that is a deployment fault, not a command result.
|
||||
|
||||
``stdin_data`` is written and closed immediately; the platform never hands a
|
||||
child an interactive terminal.
|
||||
"""
|
||||
cleaned = _validate_argv(argv)
|
||||
started = time.perf_counter()
|
||||
|
||||
try:
|
||||
proc = await asyncio.create_subprocess_exec(
|
||||
*cleaned,
|
||||
cwd=cwd,
|
||||
env=base_env(env),
|
||||
stdin=asyncio.subprocess.PIPE if stdin_data is not None else asyncio.subprocess.DEVNULL,
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
start_new_session=True,
|
||||
)
|
||||
except (FileNotFoundError, NotADirectoryError, PermissionError, OSError) as exc:
|
||||
return ProcessResult(
|
||||
argv=cleaned,
|
||||
exit_code=127,
|
||||
stderr=str(exc),
|
||||
duration_ms=int((time.perf_counter() - started) * 1000),
|
||||
spawn_failed=True,
|
||||
spawn_error=type(exc).__name__,
|
||||
)
|
||||
|
||||
if stdin_data is not None and proc.stdin is not None:
|
||||
with contextlib.suppress(BrokenPipeError, ConnectionResetError):
|
||||
proc.stdin.write(stdin_data)
|
||||
await proc.stdin.drain()
|
||||
with contextlib.suppress(Exception):
|
||||
proc.stdin.close()
|
||||
|
||||
stdout_task = asyncio.create_task(
|
||||
_drain(proc.stdout, cap_bytes=max_output_bytes, on_output=on_output)
|
||||
)
|
||||
stderr_task = asyncio.create_task(
|
||||
_drain(proc.stderr, cap_bytes=max_output_bytes, on_output=None)
|
||||
)
|
||||
|
||||
timed_out = False
|
||||
try:
|
||||
await asyncio.wait_for(proc.wait(), timeout=max(1.0, float(timeout_seconds)))
|
||||
except TimeoutError:
|
||||
timed_out = True
|
||||
await _terminate(proc)
|
||||
|
||||
stdout_result, stderr_result = await asyncio.gather(stdout_task, stderr_task)
|
||||
|
||||
return ProcessResult(
|
||||
argv=cleaned,
|
||||
exit_code=proc.returncode if proc.returncode is not None else -1,
|
||||
stdout=stdout_result.data.decode("utf-8", "replace"),
|
||||
stderr=stderr_result.data.decode("utf-8", "replace"),
|
||||
duration_ms=int((time.perf_counter() - started) * 1000),
|
||||
timed_out=timed_out,
|
||||
truncated=stdout_result.truncated or stderr_result.truncated,
|
||||
)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"MAX_ARGV_ELEMENT_CHARS",
|
||||
"ProcessError",
|
||||
"ProcessResult",
|
||||
"base_env",
|
||||
"run_process",
|
||||
]
|
||||
Reference in New Issue
Block a user