[core][sandbox] Support multi-uid user namespaces for network="public" sandboxes - #65823
Open
xyuzh wants to merge 1 commit into
Open
[core][sandbox] Support multi-uid user namespaces for network="public" sandboxes#65823xyuzh wants to merge 1 commit into
xyuzh wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental REST API service for Ray Sandbox, allowing users to manage sandboxes externally via a FastAPI application on Ray Serve. It also adds support for private network namespaces bridged by pasta (passt) and multi-uid user-namespace mapping to preserve file ownership within sandboxes. The review feedback highlights two critical issues: a missing json import in image_utils.py that will cause a runtime NameError, and a potential unhandled OSError in idmap.py if subprocess.Popen fails outside of the try block.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
xyuzh
force-pushed
the
sandbox-multi-uid
branch
from
September 1, 2026 07:00
d633a27 to
f2f0c1b
Compare
pcmoritz
pushed a commit
that referenced
this pull request
Sep 6, 2026
…ns via pasta (#65820) ## Description `network="public"` sandboxes currently run with runsc `--network=host` in the Ray worker's own network namespace: every sandbox on a node shares one port space, so concurrent workloads that bind a fixed port collide and can reach each other's listeners. The concrete failure is terminal-bench's QEMU tasks (`qemu-startup`, `qemu-alpine-ssh`), which start QEMU with `hostfwd=tcp::2222-:22` and then SSH to `localhost:2222` from inside the same sandbox. Under co-tenancy the second bind gets `EADDRINUSE`, and a verifier can connect to a *different* sandbox's guest. This PR gives each `public` sandbox a private user+network namespace pair bridged by pasta (passt) user-mode networking, the rootless-Podman topology: - a tiny holder process (`unshare --user --map-root-user --net`) pins the namespaces for the sandbox's lifetime; - `pasta` attaches from the pod side (`--netns/--userns /proc/$PID/ns/*`) and runs in the **foreground** inside the sandbox's process group, so teardown's `killpg` takes it with the rest of the tree. `-t/-u/-T/-U none --no-map-gw` make it egress-only: in-sandbox binds are never republished on the pod, pod-local services are unreachable from the sandbox loopback, and there is no inbound path; - `runsc run` executes inside via `nsenter` as mapped root. `--rootless` is dropped because nesting a second userns breaks the gofer's `/proc` magic-link derefs; since rootless mode is also what tolerated cgroup permission failures, the wrapper forces `--ignore-cgroups` for rootless configs. runsc still gets `--network=host`, but "host" is now private to the sandbox. Mount and pid namespaces stay shared, so the bundle and control sockets under `--root` keep working for pod-side `state`/`exec`/`kill`/`delete`. ### What `public` does and does not isolate `public` isolates sandboxes from each other and from the node's own services. It does **not** isolate them from the network the node sits on: pasta relays every outbound connection through the pod's own sockets and has no destination filter, so a `public` sandbox can reach other Ray nodes (including the head node's GCS and dashboard ports), other pods, and any internal service the node can reach. The docs now say this explicitly and keep `none` as the recommendation for untrusted code. Closing that gap needs egress policy outside pasta: a node-level netfilter rule set (which needs `CAP_NET_ADMIN` in the pod netns), or a second, intermediate user+network namespace we own and can firewall with nftables before handing traffic to the pod-side pasta. That is a follow-up, not part of this PR. ### Why not `pasta [flags] runsc ...` pasta can spawn a command in namespaces it creates itself, which would collapse the holder, pidfile, and nsenter into one wrapper. Prototyped in a privileged container (non-root, pasta from source, `pasta <flags> --foreground -- runsc ... run ...`): the command runs as uid 0 with a fixed `0 <uid> 1` map inside new user, net, **pid, mount, ipc, and uts** namespaces. runsc boots fine, but the pod side loses control of it: `runsc exec` fails with `waiting on pid 2: sandbox is not running` because the state file records the inner pid, and `runsc state` silently reports `running` whenever some unrelated pod process happens to have that pid. Every control call would have to be wrapped in `nsenter -U -n -p -m -t <child>` (that does work), and the single-uid map rules out the multi-uid mapping #65823 needs. The holder + attach shape keeps pid and mount namespaces shared for exactly that reason; with pasta in the foreground it costs one extra `sleep` process. Requires `pasta` and `nsenter` on nodes for `public` sandboxes. Docs updated (requirements, mode table with a warning admonition, install snippets, troubleshooting). Per-exec `user` and `write_file(append=)` moved to #65942 per review. ## Related issues Related to #65633. Per-exec user support split into #65942. ## Additional information Tested with `TEST_SANDBOX=1` in a privileged `rayproject/ray:nightly-py312` container on arm64 as the non-root `ray` user, with pasta built from source: two concurrent `public` sandboxes both bind `0.0.0.0:2222` and each reaches its own listener on `127.0.0.1:2222`; the worker namespace shows nothing on 2222; no address names one sandbox from another; egress and generated-resolv.conf DNS work; `delete_sandbox` and the create-failure path leave no pasta process behind (the tests diff the set of running pasta pids). The exact pasta flag list, the `--foreground`/pidfile gate, and the forced `--ignore-cgroups` are pinned by argv-level unit tests that run without runsc or pasta. ``` TEST_SANDBOX=1 pytest ray/experimental/sandbox/tests/test_gvisor_backend.py -k "netns or build_run_command or requires_pasta" 10 passed ``` --------- Signed-off-by: xyuzh <xinyzng@gmail.com>
xyuzh
force-pushed
the
sandbox-multi-uid
branch
from
September 6, 2026 05:23
0a1a4e5 to
a0cb869
Compare
Member
Author
|
Rebuilt on the current #65820 head (which now includes master with #65748) as a single commit, a0cb869. Two things changed beyond conflict resolution:
Verified in a privileged container with |
…" sandboxes Rootless-style sandboxes previously mapped the whole container to one host uid: every file read as root, chown to any other user failed with EINVAL, and ownership baked into image layers (postfix's 0700 uid-101 spool, mailman's uid-38 data) flattened away, failing workloads that spread ownership across users. network="public" sandboxes now map subordinate id ranges into their user namespace, the rootless-Podman model: the holder starts unmapped and the setuid newuidmap/newgidmap helpers write "0 <worker-id> 1" plus "1 <subbase> <count>" before pasta and runsc join as mapped root. detect_idmap() degrades to the single-uid mapping (warn-once) when the uidmap helpers, /etc/subuid ranges, or no_new_privs make the mapping impossible; RAY_SANDBOX_SINGLE_UID=1 forces that fallback. Image-baked ownership survives through the cache: extraction records each layer's non-root owners (whiteout-aware) into an .ownership.json sidecar next to the worker-owned rootfs, and multi-uid sandboxes mount a per-node "<image>.idmap" variant: a copy of that rootfs made inside an ephemeral mapped user namespace with the sidecar's owners applied (chown, then the mode restored so setuid/setgid bits survive). The .extracted marker is versioned (ownership-v2), so legacy caches re-pull once, keeping the pins of sandboxes already running on them. Cache eviction counts the variant toward the cap and removes it through a mapped namespace, and sandbox teardown removes subordinate-owned files the same way. Traversing another user's 0700 directory as root additionally needs CAP_DAC_OVERRIDE, which Docker's default capability set carries and the bare runsc-spec default does not; documented in troubleshooting. Signed-off-by: xyuzh <xinyzng@gmail.com> (cherry picked from commit a0cb869)
xyuzh
force-pushed
the
sandbox-multi-uid
branch
from
September 6, 2026 05:24
a0cb869 to
f8bb2b2
Compare
Member
Author
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit f8bb2b2. Configure here.
| and (member.uid or member.gid) | ||
| and not member.islnk() | ||
| ): | ||
| ownership[name] = (member.uid, member.gid) |
There was a problem hiding this comment.
Ownership map ignores later root owners
High Severity
A later layer that ships a path as root does not clear a prior non-root record, so apply_ownership still chowns that path. Replacing a directory with a file, or the reverse, also leaves child sidecar entries. Multi-uid sandboxes then get image-baked owners from an earlier layer instead of the final one.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f8bb2b2. Configure here.
| except (RuntimeError, subprocess.TimeoutExpired) as err: | ||
| raise SandboxCreationError( | ||
| f"idmapped rootfs build failed for '{image}': {err}" | ||
| ) from err |
There was a problem hiding this comment.
Idmap build leaks unreclaimable temp trees
High Severity
ensure_idmapped_rootfs removes any existing .idmap variant, then on TimeoutExpired or RuntimeError leaves the mapped cp -a temp tree in place. Eviction skips names containing .tmp., and the worker cannot delete subordinate-owned files, so each failed build leaks a full rootfs copy. The copy is also bounded by the sandbox creation timeout, which defaults to 30s.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f8bb2b2. Configure here.
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.
Description
Rootless-style sandboxes map the whole container to one host uid: every file reads as root,
chownto any other user fails withEINVAL, and ownership baked into image layers (postfix's0700uid-101 spool, mailman's uid-38 data) flattens away, so workloads that spread ownership across users fail.#65820 has merged; this is a single commit on top of master.
network="public"sandboxes now map subordinate id ranges into their user namespace, the rootless-Podman model:newuidmap/newgidmaphelpers write0 <worker-id> 1plus1 <subbase> <count>before pasta and runsc join as mapped root.detect_idmap()probes the node once per process and degrades to the single-uid mapping (warn-once) when theuidmaphelpers,/etc/subuidranges, orno_new_privsmake the mapping impossible; on nodes whose setuid bits are stripped it falls back to privileged map-file writes viasudo.RAY_SANDBOX_SINGLE_UID=1forces single-uid..ownership.jsonsidecar next to the worker-ownedrootfs/, which single-uid sandboxes keep using unchanged. Multi-uid sandboxes mount a per-node<image>.idmapvariant: acp -aof that rootfs made inside an ephemeral mapped user namespace with the sidecar's owners applied (chown, then the mode restored so setuid/setgid bits survive). No archive is written; the variant is rebuilt when the image is re-pulled or the mapping changes. The.extractedmarker is versioned (ownership-v2), so legacy caches re-pull once, and that re-pull keeps the pins of sandboxes already running on the old extraction..idmapvariant through a mapped namespace (and counts it towardRAY_SANDBOX_IMAGE_CACHE_MAX_BYTES), and sandbox teardown does the same for files a sandbox chowned under its workdir bind.Traversing another user's
0700directory as root additionally needsCAP_DAC_OVERRIDE, which Docker's default capability set carries and the barerunsc specdefault does not; documented in troubleshooting. The HTTP service's best-effort/etc/subuidseeding at boot moves to #65633 with the rest of that layer.Related issues
Follow-up to #65820. Related to #65633.
Additional information
Tested with
TEST_SANDBOX=1in a privilegedrayproject/ray:nightly-py312container on arm64 as the non-rootrayuser, withuidmapinstalled,ray:100000:65536in/etc/subuidand/etc/subgid, and pasta built from source: runtimechownto arbitrary uids inside the sandbox and host-visibly on a workdir bind atsubuid_base + uid; image-baked ownership (0700uid-101 dir,02710setgid dir) surviving into the sandbox from a local tar image; the mailman-shaped adduser +chown -R+ setgid inheritance flow; and anetwork="none"sandbox unaffected by an existing.idmapvariant. The netns suite (same-port binds, egress and DNS, teardown, failed create) still passes alongside. Argv-level unit tests pin the multi-uid holder script, and the cache tests cover ownership recording across layers, sidecar and marker writing, stale-marker re-pull, owner application with mode restore, and eviction.