forked from EduCraft/curriculum-project-hub
204 lines
12 KiB
Markdown
204 lines
12 KiB
Markdown
# ADR 0018: The Agent Execution Surface Is Bounded By The Project Workspace
|
||
|
||
## Status
|
||
|
||
Accepted, and implemented. The mechanism is settled: the Claude Code SDK's
|
||
built-in sandbox (bubblewrap on Linux, seatbelt on macOS) confines the agent
|
||
process and its Bash subprocesses at the OS level. `bypassPermissions`
|
||
remains (headless server — no interactive prompts), but the sandbox is the
|
||
hard boundary, not the permission-prompt layer. Constitution rule 4
|
||
divergence (ADR-0017's unbounded tools) is resolved.
|
||
|
||
Builds on **ADR-0001** (project group as collaboration space), **ADR-0002**
|
||
(project lock scoped to `run_id`), **ADR-0004** (permission grants govern
|
||
`triggerAgent`), **ADR-0007** (the engineering file is a directory tree on
|
||
disk), and **ADR-0017** (Claude Code SDK as the agent runtime).
|
||
|
||
## Context
|
||
|
||
ADR-0001/0002/0004 together cover *collaboration governance* up to the
|
||
moment a teacher `@Claude`s: who may trigger an `AgentRun`, that the run
|
||
holds a project-scoped lock, that the group is a long-lived space. They stop
|
||
at `triggerAgent`. What the agent may do *after* the trigger — which files it
|
||
may read or write, what commands it may run — is not named in any ADR, not
|
||
modeled in `spec/`, and not enforced by any contract-level invariant.
|
||
|
||
The gap is not theoretical. ADR-0017 adopted the Claude Code SDK with
|
||
`allowedTools: ["Read", "Write", "Bash", "Glob", "Grep"]` and
|
||
`permissionMode: "bypassPermissions"`. Under that configuration the SDK's
|
||
built-in tools operate over the whole host filesystem with no confinement, and
|
||
`Bash` executes arbitrary shell with no approval gate. The Hub's
|
||
`hub/src/agent/workspace.ts` was *intended* to be "the agent's only file
|
||
surface" (its module doc says so) and ships a `confine()` path validator that
|
||
rejects `..` and absolute escapes. But ADR-0017's cutover left those custom
|
||
tools unwired — the SDK's built-in tools replaced them — so `confine()` is
|
||
dead code and the boundary it was meant to enforce does not exist at runtime.
|
||
Compounding this, the systemd unit's default `SERVICE_USER` is `root`
|
||
(`hub/deploy/install_service.sh`), so the unbounded agent runs as root.
|
||
|
||
This is a real divergence point by the constitution's test: "if unwritten,
|
||
will the developer and the agent make different assumptions?" They already
|
||
have. ADR-0017 assumed the boundary was an implementation concern; the
|
||
`workspace.ts` author assumed it was a contract. The next person to touch the
|
||
runner has nothing telling them the boundary must be enforced, and ADR-0017
|
||
silently overrode the one that existed.
|
||
|
||
## Decision
|
||
|
||
### The agent execution surface is bounded by the run's project workspace
|
||
|
||
During an `AgentRun`, every file operation the agent performs — read, write,
|
||
glob, grep target — must fall **within the run's project workspace directory**
|
||
(the on-disk engineering-file tree of ADR-0007). Operations whose target path
|
||
escapes that tree are refused. This is a platform core invariant, pinned in
|
||
`Spec.System.AgentSurface`.
|
||
|
||
The boundary is **per-run, anchored to the run's project**, the same scope as
|
||
the Lock (ADR-0002). This keeps the two invariants composable:
|
||
|
||
- `Lock` (ADR-0002) bounds **concurrency** — who may be modifying the project.
|
||
- `AgentSurface` (this ADR) bounds **blast radius** — what the agent may touch.
|
||
|
||
Both scope to `run × project`. Acquiring the lock and entering the surface
|
||
happen together at run start; releasing the lock and leaving the surface
|
||
happen together at run termination.
|
||
|
||
### Shell execution is subject to the same boundary, by effect
|
||
|
||
A shell command the agent runs executes with the workspace as its working
|
||
directory (already true for the `cph` subprocess, ADR-0016) and its **file
|
||
effects** — files read, written, created, or deleted — are subject to the same
|
||
workspace boundary. The agent may invoke **named external tools** whose
|
||
binaries live outside the workspace (notably `cph` via `CPH_BIN`); the
|
||
boundary governs *file effects*, not *binary location*. Arbitrary
|
||
`cat /etc/shadow` or `curl … > /etc/...` is out of bounds regardless of how it
|
||
is invoked.
|
||
|
||
### The Hub process runs as a non-privileged service user
|
||
|
||
The systemd unit must not run as `root`. The default in
|
||
`hub/deploy/install_service.sh` changes from `root` to a dedicated unprivileged
|
||
user. This is a deployment invariant, recorded here because the unbounded-agent
|
||
risk is compounded by root; it is enforced in the unit, not in `spec/` (it is
|
||
not a semantic divergence between developer and agent — it is operational
|
||
discipline).
|
||
|
||
### The mechanism: SDK sandbox (OS-level, per agent process)
|
||
|
||
The boundary is enforced by the Claude Code SDK's built-in sandbox
|
||
(bubblewrap plus `socat` on Linux, seatbelt on macOS), configured via `query()` options in
|
||
`hub/src/agent/runner.ts`:
|
||
|
||
- `sandbox.enabled: true` + `failIfUnavailable: true` — the agent process and
|
||
its Bash subprocesses run sandboxed; if the sandbox can't start, `query()`
|
||
emits an error and exits rather than running unsandboxed.
|
||
- `sandbox.allowUnsandboxedCommands: false` — a tool cannot opt out with the
|
||
SDK's `dangerouslyDisableSandbox` input. Claude Code 2.1.202 does not enforce
|
||
that option reliably, so a host-side `PreToolUse` hook also denies every
|
||
Bash request whose input explicitly sets `dangerouslyDisableSandbox: true`
|
||
before a process can start.
|
||
- `sandbox.filesystem.denyRead: ["/"]` with `allowRead` for the canonical
|
||
current workspace and a small named system-runtime set — normal reads stay
|
||
in the run's workspace while `/bin`, shared libraries, CA certificates,
|
||
fonts and the configured `cph` executable remain available as the external
|
||
tool exception described above.
|
||
- `sandbox.filesystem.allowWrite: [workspaceDir]` confines persistent host
|
||
effects to the canonical ADR-0007 workspace. Config/cache/home stay beneath
|
||
`.cph/agent-runtime/`; `TMPDIR`, `TMP`, `TEMP`, and `CLAUDE_CODE_TMPDIR` all
|
||
point at the workspace-local `.cph/t`. The workspace allocator uses stable
|
||
compact Organization/Project path segments, deployment requires a short
|
||
workspace root, and the canonical temp prefix fails fast above 56 bytes so
|
||
the SDK can append randomized `socat` bridge socket names without exceeding
|
||
Linux `sockaddr_un.sun_path`. Bubblewrap shadows non-allowlisted host trees
|
||
with disposable tmpfs mounts: a shell write there may succeed inside that
|
||
private namespace, but it cannot mutate the corresponding host path. The
|
||
Linux proof checks host state after the sandbox exits.
|
||
- The SDK subprocess environment replaces rather than spreads `process.env`.
|
||
Only provider protocol variables and non-secret runtime variables cross the
|
||
boundary; database, Feishu and Hub session credentials never enter it.
|
||
`sandbox.credentials` denies provider token variables inside Bash and denies
|
||
configured credential files. `CPH_SANDBOX_EXTRA_DENY_READ` names additional
|
||
deployment-specific secret paths on the Hub side only.
|
||
- `settingSources: []` and strict MCP configuration prevent an untrusted
|
||
workspace or service-user config from widening tools, hooks, MCP servers, or
|
||
sandbox paths.
|
||
- Agent skills are Organization-scoped runtime configuration, not Hub release
|
||
assets. A controlled host-console installer imports each version into a
|
||
content-addressed persistent store and records its digest in PostgreSQL. A
|
||
role selects enabled Organization skills alongside its model, system prompt
|
||
and tool allowlist. Each run copies only those selected immutable versions
|
||
into a run-scoped plugin outside the project workspace; the sandbox exposes
|
||
that snapshot read-only and deletes it after the run. SDK-bundled skills and
|
||
filesystem setting sources remain disabled, so project `.claude` content
|
||
cannot register skills or widen tools. Requested skill versions are recorded
|
||
on `run.created`; SDK initialization remains authoritative loading evidence.
|
||
- Network: open (see Open Questions).
|
||
|
||
`bypassPermissions` is kept (headless server — no interactive prompts); the
|
||
sandbox is the hard boundary, not the permission-prompt layer. This is
|
||
subprocess-level, not process-level: the Hub host process is not sandboxed
|
||
(it manages DB, Feishu ws, logs), only the Claude Code agent process and its
|
||
children. This keeps the sandbox config surface small and lets future
|
||
external CLIs (Gitea, Lark) run inside the sandbox by giving their binaries
|
||
read access, without widening the Hub's own filesystem surface.
|
||
|
||
The contract pins the **invariant** (operations stay in the workspace), not
|
||
the **mechanism**. The SDK sandbox is the current mechanism; switching to a
|
||
different one (tool wrappers, container) would not be a spec change as long
|
||
as it upholds `AgentFileOp.Authorized`.
|
||
|
||
## Consequences
|
||
|
||
- `Spec.System.AgentSurface` gains an `AgentFileOp` structure and an
|
||
`Authorized` predicate mirroring `Memory.McpReadRequest.Authorized`: the op
|
||
carries a `run` and a `path`; `Authorized` holds iff the path falls within
|
||
the run's workspace, via platform-provided `runWorkspace` and `pathWithin`
|
||
(representations `OPEN`).
|
||
- `hub/src/agent/workspace.ts`'s `confine()` intent is now contract-backed.
|
||
Re-wiring it (or replacing it with an OS sandbox) is **mandatory**, not a
|
||
preference. Its current dead-code status under ADR-0017 is a divergence to
|
||
be addressed, not silently retained.
|
||
- ADR-0017's `bypassPermissions` + unrestricted built-in tools is a
|
||
**divergence from this ADR** and is surfaced as such (constitution rule 4).
|
||
The reconciliation — restrict the SDK tool set, wrap Bash, or sandbox at the
|
||
OS level — is the open mechanism question above. Whichever is chosen must
|
||
uphold the `AgentSurface` invariant.
|
||
- Inbound file/image delivery (`hub/src/feishu/trigger.ts`) lands files under
|
||
`${workspaceDir}/.cph/inbox/`, which is inside the boundary — consistent, no
|
||
change to the delivery path.
|
||
- The `cph` subprocess (ADR-0016) already runs with `cwd = workspaceDir`
|
||
(`hub/src/agent/cph.ts`), so it is inside the boundary by construction.
|
||
|
||
## Open Questions / Deferred
|
||
|
||
- **Network egress.** Decided: network is **open** in the initial
|
||
production deployment. The sandbox confines filesystem writes to the
|
||
workspace and denies reads of sensitive host paths, but does not restrict
|
||
outbound network. Rationale: future Gitea/Lark CLI integration needs
|
||
unconstrained network egress; `cph build` may fetch typst packages. The
|
||
exfiltration risk (agent `curl`s workspace content out) is accepted as a
|
||
tradeoff for CLI integration simplicity. This is revisitable — a network
|
||
allowlist (`sandbox.network.allowedDomains`) can be added without a spec
|
||
change if the threat model tightens.
|
||
- **Named system-runtime read set.** The implementation allows only the
|
||
platform runtime directories/files needed for shell, dynamic libraries,
|
||
fonts, TLS/DNS and `cph` execution in addition to the project workspace.
|
||
Expanding that set requires an explicit reviewed code change. Deployments
|
||
name credential files with `CPH_SANDBOX_EXTRA_DENY_READ`; these paths are
|
||
protected through the SDK credential layer and are not passed to the Agent
|
||
environment. The exact runtime set remains plumbing and is not spec-pinned.
|
||
- **Shell command vocabulary.** Whether the agent's Bash is restricted to a
|
||
whitelist (e.g. `cph`, `ls`, `grep`) or allowed arbitrary commands whose
|
||
*effects* are then confined, is a policy choice left to implementation. The
|
||
invariant governs effects either way; current choice is arbitrary commands
|
||
(sandbox confines effects).
|
||
|
||
## Related Decisions
|
||
|
||
- **Rate limiting and token usage.** ADR-0022 requires multi-dimensional hard
|
||
request limits while keeping token thresholds as attributed soft alerts in
|
||
the initial release.
|
||
- **Inbound file limits.** ADR-0022 requires hard file, attachment, archive,
|
||
Project-storage, and Organization-storage limits with explicit failure and
|
||
partial-file cleanup.
|