feat(#576): durable per-scope sandbox — P1 interface + Docker backend - #776
Merged
Conversation
Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing.
This was referenced Aug 20, 2026
…e-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision).
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
Add/add conflicts in the three harness-sandbox files P3 evolved: main carries P1's squash (#776), this branch carries P1 plus P3's deliberate changes. Verified via git log that only the #776 squash ever touched these paths on main, so the branch side is the correct resolution. The lockfile conflict is resolved by taking main's and reconciling via npm install.
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #576 (qm competitive analysis) — Phase 1 of 3 (interface + Docker
backend; execute tool + policy gate is P2, scope durability + RO-layer is
P3, admin view is P4-if-time). This PR touches only a new workspace
package; it does not touch the orchestrator,
src/index.ts,agentBuilder.ts,or the credential/skill namespaces.
Introduces
@omadia/sandbox(middleware/packages/harness-sandbox):Sandboxcontract (provision/run/read/write/list/teardown), modelledon qm's
src/sandbox/sandbox.tsshape per the issue. Optional capabilities(
process-sessions,backup,blob-staging) are exposed via typeguards (
hasProcessSessions,hasBackup,hasBlobStaging) rather thaninterface fields — a backend that doesn't implement one simply fails the
guard.
DockerSandboxBackendv1 implements none of them; the guards returnfalsefor every sandbox it provisions.AgentComputerProfile— declarespersistent/egress/processSessions/maxRunSeconds/maxOutputBytes. Every field's doc comment names the exactcall site required to enforce it (this repo's most-repeated defect is a
declared field nobody wires — see
harness-lib-generalization-plan1andpr736-command-policypostmortems in project memory).DockerSandboxBackend— local Docker, built on the sameinjectable-
execpattern assrc/plugins/builder/buildSandbox.ts'sexecuteBuildseam (dockerExec.ts'sexecDockerinjection point), so thefull backend logic runs deterministically with zero real Docker.
Wiring proof, not declaration
egress: false→docker run --network none. Proven at two levels:a stub-level argv assertion (always runs, no Docker needed) AND — behind
the opt-in
SANDBOX_DOCKER_TEST=1gate — a real container attempting anoutbound
wgettoexample.comand observing it fail. Both were runlocally against an actual Docker daemon and pass (~6.2s including the
wget timeout).
pathGuard.ts'sclampSandboxPathPosix), samediscipline as the feat(#578): credential broker — the egress-stamping layer (phase 2/4) #772 broker and
zipExtractor.ts's zip-slip guard:absolute paths, NUL bytes, and any
../resolution outside the sandboxroot are rejected before a single
docker execis issued — asserteddirectly (the traversal tests check the stub recorded zero calls for a
rejected path).
deterministic function of the scope key
(
omadia-sbx-<sha256(scopeKey)[0:24]>), soprovision()re-attaches to analready-running container across backend-instance restarts. Docker itself
is the durable store for "which container belongs to this scope" — what
P3's registry adds on top is bookkeeping Docker doesn't give for free
(reaper timestamps, RO-layer content-hash tracking, multi-backend
routing), not a prerequisite for this backend to be correct.
Docker-gated vs stub split
Per the plan: CI has no guaranteed privileged Docker, so real-Docker tests
are gated (
SANDBOX_DOCKER_TEST=1, opt-in, off by default) and stub testscover 100% of the logic branches. 30 stub-tier tests always run in
npm test; 2 real-Docker tests are additional and were run manually againsta live daemon (both green — see commit message for the exact run).
Mutation-check evidence
With a full
npm run build -w @omadia/sandboxrebuild between each mutationand its revert:
!profile.egresscondition indockerSandbox.ts→egress:true does NOT pass --network nonetest failed as expected(asserted the exact wrong argv in the failure output).
pathGuard.ts'sclampSandboxPathPosix→both
.. escapetests inpathGuard.test.tsfailed as expected.Both reverted; full suite green again after rebuild.
Blast radius
middleware/packages/harness-sandbox), zeronew runtime dependencies (shells out to the
dockerCLI vianode:child_process, same constraint asdev-runner-shim).middleware/package.json: registered@omadia/sandboxin thebuild,typecheck,lint,lint:fixworkspace chains (built/typechecked first —no other package depends on it yet).
middleware/package-lock.json: workspace registration only (+12 lines).middleware/test/sandbox/(this repo's actualCI-covered test convention — the root
npm testglob istest/**/*.test.tsrelative tomiddleware/, so package-localtest/dirs like
dev-runner-shim's are not picked up by CI; confirmed thisbefore placing the new tests).
Verification run locally (this branch, after merging origin/main)
npm run build(full workspace): greennpm run typecheck(full workspace incl. golden/adversarial): greennpm run lint: cleannpm run test: 7049 tests, 7037 pass, 0 fail, 12 skipped (pre-existing),0 cancelled
Base / stacking
Branched from
origin/mainat merge time (29c2c678, includes #578 P1credentials). No stacking — this is the first #576 PR. P2 (execute tool +
command-policy gate) will stack on this branch.
Open questions for Marcel
(process-sessions/backup/blob-staging) — the type-guard seam exists per
the issue's qm reference but nothing needs it yet. Confirm that's the
right amount of yet-unused surface for this phase (YAGNI vs. "the seam
exists so P2/P3 plug straight in" — I read the issue as wanting the seam
now, implementations later).
alpine:3.20(small, busybox coreutils, shipswgetfor the egress proof). If agent work is expected to need a fullertoolchain (git, node, python) sooner than P3/P4, that's a v1 default worth
revisiting explicitly rather than inheriting silently.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.