feat: migrate the run action's sandbox to runc with seccomp supp#175
Draft
dash14 wants to merge 21 commits into
Draft
feat: migrate the run action's sandbox to runc with seccomp supp#175dash14 wants to merge 21 commits into
dash14 wants to merge 21 commits into
Conversation
Adds docker cp extraction of runc and gen-seccomp-profile onto the runner host, runs gen-seccomp-profile natively to resolve the seccomp filter, and builds the full OCI Runtime Spec (config.json) in JS from runc's own `runc spec` baseline. run-isolated.sh itself isn't wired up to consume these yet.
…priv Replaces the hand-rolled unshare placeholder namespace, setpriv privilege drop, and mountinfo-walking read-only remount with runc run against the OCI bundle isolated-exec.js now builds. run-isolated.sh's own job shrinks to what runc can't do itself: wiring the sandbox netns's veth into the proxy's sandbox0 bridge, and bind-mounting the host's own / for runc's rootfs. Verified end-to-end (network isolation, capability/seccomp enforcement, uid/gid drop, read-only filesystem policy) by driving the built run/dist/main.cjs against a real proxy container as a non-root user.
Ties the sandboxed process's life to run-isolated.sh's via a two-hop setpriv --pdeathsig=KILL chain (run-isolated.sh -> runc run -> sandboxed command), since `runc run`'s own process sits between them. Closes the same SIGKILL-orphaning gap bwrap's --die-with-parent would have, without needing a new binary -- confirmed util-linux's setpriv already supports --pdeathsig. Verified empirically: killing run-isolated.sh directly tears down the whole chain.
…loop integration-test-seccomp.sh verifies unshare -U and io_uring_setup are actually blocked (the two CVE classes docs/security.md names) while ordinary syscalls still work. integration-test-die-with-parent.sh SIGKILLs run-isolated.sh mid-run and verifies the whole sandboxed process tree dies with it rather than surviving as an orphan. Both wired into test_sandbox_integration, so CI's existing test_sandbox job picks them up. Also updates the Mac-friendly dev loop (run/dev/Dockerfile, compose.sandbox-dev.yaml, Makefile's test_sandbox_mode) for run-isolated.sh's new runc-based interface: runc and gen-seccomp-profile are now built directly into the dev image (mirroring docker/Dockerfile), and a new build-test-bundle.sh stands in for isolated-exec.js's buildOciConfig so the dev loop can keep testing run-isolated.sh directly without needing the Docker socket mounted in. The dev-runner service also needs the same writable-/sys/fs/cgroup treatment the "builder" service already has, for the same reason (runc's own cgroup management) -- not needed on the real runner host this dev loop stands in for.
Rewrites docs/security.md's Isolation Mechanisms (seccomp filter, die-with-parent, capabilities/mounts now enforced by runc via config.json rather than setpriv/manual remounts) and removes the now-resolved 'No seccomp profile' limitation. Rewrites docs/development.md's Run Action Internals to match (runc/gen-seccomp-profile extraction, OCI bundle construction in isolated-exec.js, the two-hop pdeathsig chain) and updates the Sandbox Dev Loop section and directory tree listing. Flips docs/reference.md's seccomp NOTE from 'not applied' to 'applied'.
grep -E 'CapEff|NoNewPrivs' /proc/self/status only checked that those fields existed (always true for any process), not that CapEff was actually 0000000000000000 or NoNewPrivs was 1 -- a capability-drop regression would have passed silently. Switches to grep -q with the exact expected value, matching integration-test-defaults.sh's style. Also adds a basic read-only-filesystem-policy check (/tmp writable, /etc not), which wasn't exercised by this dev-loop smoke test at all before.
dash14
force-pushed
the
feat/run-action-runc-migration
branch
from
July 19, 2026 04:04
b9540cf to
6c20888
Compare
gen-seccomp-profile's execFileSync (and the subsequent generateBaseOciSpec/writeRunScript/buildOciConfig calls) weren't wrapped in a SandboxError like the adjacent docker-cp extraction, so a failure surfaced as an unclassified exception instead of a clean ::error:: with a diagnosable code. Verified with a proxy image whose gen-seccomp-profile deliberately fails: the run now fails with a clean OCI_CONFIG_BUILD_FAILED SandboxError and cleanup (docker compose down) still runs.
Extract EXTRA_MASKED_PROC_PATHS into run/scripts/extra-masked-proc-paths.json so isolated-exec.js and run/dev/build-test-bundle.sh (its bash/jq stand-in for the Mac dev loop) read the same list instead of hand-duplicating it, removing a silent-drift risk between the two. isolated-exec.js picks it up via a static JSON import (resolved relative to the source file, so it works both unbundled in tests and inlined by rollup in run/dist/main.cjs); build-test-bundle.sh reads it through a new read-only bind mount in compose.sandbox-dev.yaml. Also drops the now-redundant Set-based dedup of maskedPaths -- the doc comment already establishes the two lists don't overlap on runc v1.5.1.
…faults run-isolated.sh silently defaulted these to the values main.js already always passes explicitly, leaving two sources of truth for the same addressing scheme with no way to tell which one is actually in effect. Make them required arguments like --proxy-pid, and always pass them from isolated-exec.js's runIsolated (they were already always truthy in practice). Also fixes the Mac dev loop's Makefile invocation, which was relying on the now-removed --target-ip default.
run/dev/Dockerfile duplicates the same pinned ARG RUNC_VERSION/RUNC_SHA256_* block as run/docker/Dockerfile (see docs/development.md), but the customManager only watched the latter -- renovate could bump one Dockerfile's runc pin without the other, silently drifting the Mac dev loop's runc out of sync with production.
pgrep -f could in principle match more than one process (e.g. a leftover process from a previous failed run, or the pattern incidentally matching an unrelated process's argv), in which case BASH_PID became a multi-line string and kill -9 "$BASH_PID" failed with an invalid-argument error -- masking the real assertion behind a confusing, unrelated failure. Count the matches explicitly and fail with a clear diagnostic if it's not exactly 1.
Each `run:` step redid docker cp of runc/gen-seccomp-profile, ran runc spec, and re-resolved the seccomp profile from scratch, even though all three are identical for repeated steps against the same proxy image. Cache them under a digest-keyed directory in tmpdir(), reused across every `run:` step in the same job (and across jobs on a self-hosted runner). Concurrent steps racing to populate the same cache entry stage in a private directory and atomically rename it into place; a losing racer just discards its own staging dir and reuses the winner's. The runc binary itself is still copied into each run's own scratch dir before use, rather than executed directly from the shared cache: pointing run-isolated.sh's `mount --rbind /` at a binary that lives in a long-lived, shared location meant it stayed present (and busy) inside every concurrently running sandbox's rootfs snapshot, not just the one using it. A cheap local copy keeps the expensive work cached while keeping the actually-executed file private per run, same as before this cache existed. Also drops gen-seccomp-profile's JSON pretty-printing -- pure overhead for output that's only ever machine-parsed.
integration-test-concurrent.sh intermittently failed (reproduced independent of any other change on this branch, including on the pre-existing baseline) with EBUSY on rmSync's final rmdir of the rootfs bind-mount directory, even though /proc/self/mountinfo no longer listed it as a mountpoint at the time of the error. Root-caused via a debug build that dumped mountinfo and retried on failure: multiple `run:` steps running concurrently each recursively bind-mount "/" into their own scratch dir, and since they all live under the same shared /tmp, each step's snapshot unavoidably nests a copy of every other concurrently-running step's own rootfs bind-mount. Under that load, the kernel's teardown of an already lazily-detached mount can lag behind rmSync's rmdir by a short, bounded window -- every observed failure resolved on the very next retry after a brief wait. Retrying rmSync a bounded number of times on EBUSY specifically (not other errors) closes this: 6/6 concurrent runs passed in verification, versus a 50-75% failure rate before this change on the same environment.
run-isolated.sh's `mount --rbind /` staging previously ran directly in the one mount namespace shared by every concurrently running `run:` step on the host. Since each step's own rootfsBindDir lives under the same /tmp, each step's rbind snapshot unavoidably captured a nested copy of every other concurrently running step's own rootfs tree too -- whatever happened to exist under "/" at that instant. This didn't break sandbox isolation itself (runc still creates its own further-nested namespace for the sandboxed process regardless), but it was the root cause of the intermittent EBUSY race in withScratchDir's cleanup that the previous commit's retry papered over: concurrent steps' unmount/rmdir could race against each other's staging. Re-exec into `unshare --mount --propagation private` before doing anything else, the same staging pattern tools like bubblewrap use: every mount this script creates from that point on (the rootfs bind-mount, the netns file) is invisible to, and unaffected by, every other concurrent invocation from the moment it's created. Verified empirically -- the cross-instance mount nesting no longer occurs at all, and 8/8 concurrent runs passed. No `--fork` is used, so the re-exec replaces the current process in place; this script's PID (and /proc/self/cmdline, matched by integration-test-die-with-parent.sh's pgrep) stays the same throughout. The now-redundant `mount --make-rprivate "$ROOTFS_BIND_DIR"` is removed -- every mount created under an already-private parent is private by default. The previous commit's EBUSY retry stays in place as defense-in-depth.
Removed comparisons to the previous unshare-based implementation
("matching the previous X", "mirrors the previous Y", "verified
empirically", debugging-journal-style asides) that don't help a reader of
the current code, and shortened a few overly long rationale blocks down to
the load-bearing facts. Also fixed two comments that had gone stale from
later commits in this same branch: main.js's gateway/dns default-values
comment (the defaults were removed from run-isolated.sh in an earlier
commit) and gen-seccomp-profile/main.go's "once per run step" claim (no
longer true once getOrPopulateBootstrapCache started caching it). No
behavior change -- comment-only, confirmed by an unchanged run/dist/main.cjs
after rebuilding.
run-isolated.sh runs as root (via sudo -n), but this test's own process runs as the unprivileged CI user -- a plain kill -9 against it fails with EPERM, since only root or a same-UID process can signal it. This has failed in every real CI run since the test was added (caught locally only because verification always ran as root, masking the UID mismatch). Verified with a non-root user in a throwaway container: kill -9 now succeeds and the assertion passes across repeated runs.
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
1 participant
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
run's sandbox (run-isolated.sh) isolates the user's command viarunc(the OCI reference runtime) instead of hand-rolledunshare/nsenter/setprivnamespace/mount/capability handling. Namespaces, capabilities, mounts, uid/gid, and a seccomp filter are all declared in an OCIconfig.jsonand enforced by runc itself, including native support for joining the proxy container's pre-existing network namespace and uid/gid dropping without a user namespace. runc is also already indirectly trusted by buildcage's ownexplicit/transparentbuild engines viamoby/buildkit.How the sandbox is built
isolated-exec.jsbuildsconfig.jsonstarting fromrunc spec's own defaults and patches in:root.pathpointing at a bind-mounted copy of the host's/(read-only — every real host mount point is forced individually read-only outside workdir/home/tmp/writable, sinceroot.readonlyalone only covers the top-level rootfs mount), the network namespace entry pointed at the netnsrun-isolated.shwires a veth into, fully cleared capabilities plusnoNewPrivileges, the step's real environment, and a seccomp filter.moby/profiles), resolved into OCI form against an empty capability set (matching the sandbox) by a small Go tool,gen-seccomp-profile. It blocksunshare(CLONE_NEWUSER), theio_uring_*family, and the rest of Docker's default deny-list.runcandgen-seccomp-profileare distributed as part of the proxy image (runcas a checksum-pinned official release binary) and extracted onto the runner host atrunaction startup —gen-seccomp-profilespecifically must run natively on the host rather thandocker exec'd, since its output depends on the real host's kernel/architecture. Both extractions, plusrunc specand the resolved seccomp profile, are cached per proxy image (digest-keyed, under a shared tmp directory) and reused across everyrun:step in the same job; theruncbinary handed to each run is always a fresh copy into that run's own scratch directory, keeping the shared cache itself out of any single run's rootfs snapshot.run-isolated.shre-execs itself into a fresh, private mount namespace (unshare --mount --propagation private) before doing any of its own mount/netns work, so its rootfs bind-mount and netns file are invisible to, and unaffected by, every otherrun:step running concurrently on the same host.setpriv --pdeathsig=KILLchain (run-isolated.sh→runc run→ the sandboxed command) ties the sandboxed process's life torun-isolated.sh's own, sincerunc run's own process sits between the two — SIGKILL-ingrun-isolated.shtears down the whole tree instead of leaving the sandboxed command running as an orphan.run-isolated.sh's own trap unmounts it on exit, andisolated-exec.js'swithScratchDirforce-detaches (lazy unmount) anything still mounted under its scratch directory before deleting it, retrying on transientEBUSY.Tests and tooling
run:steps (run/test/integration-test-*.sh, wired intotest_sandbox_integration).run/dev/) buildsrunc/gen-seccomp-profiledirectly into its own image rather than extracting them via a mounted Docker socket, keeping it scoped to testingrun-isolated.shitself.renovate.jsontracks the pinnedRUNC_VERSIONin both the proxy image and the dev-loop image.docs/security.md/docs/development.md/docs/reference.mddescribe the runc-based implementation; the previously-documented "No seccomp profile" Known Limitation is resolved.