feat: rootless docker-in-sandbox (DinD) behind a per-session flag - #252
feat: rootless docker-in-sandbox (DinD) behind a per-session flag#252yourbuddyconner wants to merge 19 commits into
Conversation
Adds repoDockerFlag() to source-service.ts: reads .valet/prebuild.yaml via the GitHub Contents API, returns override.docker === true, caches per owner/repo@ref for 10 minutes. Best-effort throughout — any failure (no token, network error, bad YAML, non-GitHub host, no repos) resolves false without blocking the session build. Wires resolveRepoDockerFlag() into EngineHost.buildSession(): ORs meta.docker with the repo flag before assembling sandboxOpts. Guards on githubTokenDeps/db presence (mirrors buildCredentialResolver pattern). Only github.com-hosted primary repos qualify.
resolveRepoDockerFlag in host.ts now races repoDockerFlag against a 5-second Promise.race; on timeout it console.errors the session id and "timed out" and returns false without caching (a timeout is not a repo answer). repoDockerCache clears() when size >= 1000 before each insert to prevent unbounded growth. Two new tests: a hanging-fetch seam test verifying the result is not cached on timeout, and a 1001-key cap test.
- Add packages/sandbox-docker/test/dind.e2e.test.ts: creates a docker:true sandbox, polls for daemon ready, then runs hello-world, builds a Dockerfile, and runs nginx:alpine with a published port. - Register sandbox-dind row in scripts/e2e/lib.ts (group: docker, needs: docker, 15 min timeout); update lib.test.ts counts from 34 to 35 rows. - Add Docker-in-sandbox subsection to docs/security-model.md matching the amended spec (CAP_SYS_ADMIN + CAP_NET_ADMIN, /dev/fuse + /dev/net/tun, seccomp/AppArmor/systempaths unconfined; not privileged, no host socket). - Fix pre-existing test fixture drift: add docker:false to SessionDetail fixtures in session-header.test.tsx and transcript.test.ts; add dockerSupport:true to capabilities() assertion in docker-sandbox.test.ts.
![valet-valet-turnkey-dev[bot]](https://avatars.githubusercontent.com/u/104513330?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR is well-structured and the implementation matches the stated intent. Two real issues worth flagging: a subuid/subgid range collision in the Dockerfile that could silently break rootless uid-mapping, and a subtle TOCTOU bug in the start-docker.sh idempotency check that can corrupt the symlink under concurrent invocations.
-
docker/Dockerfile.sandbox-k8s:lines 143-147 — The subuid/subgid range200000:65536fordockerdcollides with any existing user that already owns that range in the base image. The script removes thedockerdline first, then appends it — so if the base image already has another user mapped to200000+, those entries remain andnewuidmapwill refuse to write an overlapping range, causingdockerd-rootless.shto fail at startup with a cryptic uid-map error. The earlier plan doc shows100000:65536(the conventional subuid start for the first added user); the Dockerfile was changed to200000:65536without removing or auditing the existing base entries. At minimum, add aRUN awk -F: '$2 >= 200000 && $2 < 265536 {exit 1}' /etc/subuid /etc/subgid || { echo "subuid/subgid collision"; exit 1; }check, or switch to a range that is verifiably unoccupied in the base (e.g.,500000:65536). -
docker/start-docker.sh:lines 18-31 — The idempotency guard checks for a live socket, then conditionally launches the daemon — but the symlinkln -sf "$SOCK" /var/run/docker.sockonly runs when the socket appears after a fresh launch, not on the re-entry path (theexit 0at line 20 returns before the symlink). This means if the sandbox container restarts (PID 1 respawnsstart-docker.sh) while the daemon is already running,/var/run/docker.sockwill not be recreated, so any root-rundockerCLI invocation will fail with "no such file". Move theln -sfbefore theexit 0early return, or unconditionally run it after theforpoll loop regardless of how the daemon came up.- Secondary nit on the same file:
su -s /bin/bash dockerd -c "... >> '$LOG' 2>&1 &"— the single-quoted$RUNTIME_DIRand$LOGinside the double-quoted heredoc work because they're actually double-quoted in the outer shell, but only because the shell expands them before passing tosu. This is fine, but a path containing spaces (unlikely here, but/tmp/valet-dockeris fine) would still break. Not blocking, just worth noting if the path ever becomes configurable.
- Secondary nit on the same file:
Created on behalf of Xiangan He xiangan@turnkey.io
Task 9 acceptance: valet's docker-gated suites inside a
|
1 participant
What
Sessions can opt into a rootless docker daemon inside their sandbox, on both the docker and kubernetes providers. Spec:
docs/specs/2026-08-15-sandbox-docker-design.md; plan:docs/plans/2026-08-15-sandbox-docker.md.Opt-in (either switch):
docker: truein the repo's.valet/prebuild.yaml, ordocker: trueat session create. The flag threads session meta →SandboxCreateOpts.docker→ provider deltas. Sandboxes without the flag are byte-identical to today (unit-pinned on both providers).What an opted-in sandbox gets (never
--privileged, no host socket, all other caps dropped):CAP_SYS_ADMIN+CAP_NET_ADMIN/dev/fuse+/dev/net/tunVALET_SANDBOX_DOCKER=1→ the image's start scripts launchdockerd-rootlessas a dedicated non-rootdockerduserThe sandbox image bakes the toolchain dormant; sandboxes that do not opt in pay image size only (verified: no daemon process without the env flag).
Spec amendment made during implementation
The original design said "no added capabilities". Empirically false for rootless DinD in an unprivileged container:
newuidmapneeds SYS_ADMIN to writeuid_map, and rootlesskit's netns sysctl needs NET_ADMIN + unmasked/proc/sys. Verified by three live container experiments (spec-exact flags fail; caps alone fail; caps + unmasked proc works). The spec's decision 2 and security-model.md document the final posture: strictly weaker than privileged, confined to opted-in sandboxes.Validation
make e2e: 31 passed / 0 failed / 4 skipped (skips are missing external creds, unrelated). Includes the newsandbox-dindrow: live rootless DinD round-trip —docker run hello-world,docker build, published port + curl, all inside adocker: truesandbox.pnpm typecheckclean.Known limits / follow-ups
docker: truesandbox (make e2e E2E_ARGS="--only sandbox-docker,store-postgres,prebuilds-docker"in-sandbox) and record the scorecard here.procMount: Unmaskedon kubernetes requires the ProcMountType feature gate; where unavailable the k8s DinD path does not converge (noted in spec; check at k8s acceptance).