Repository navigation
fix: drop supplementary groups for sandboxed launches - #154
Conversation
Unprivileged bubblewrap keeps the caller's supplementary groups, and groups such as docker and lxd are root-equivalent: a network-only sandbox could reach /var/run/docker.sock. When the owner has any, sandboxed launches start through unshare with maps from the setuid newuidmap/newgidmap, drop every supplementary group and exec bubblewrap as the same uid without capabilities. Missing uidmap or subordinate ranges refuse the launch with a fix-it message. runtime_isolation_smoke.py now fails when a sandboxed app keeps any supplementary group; it fails on 0.4.0 and passes here.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: caa1806a-63f5-4354-9ff2-65d43c416561) |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
![revuto-review[bot]](https://avatars.githubusercontent.com/u/55848801?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is an auto review done by revuto.
Two installation compatibility issues can prevent sandboxed apps from launching.
The launch environment (LD_PRELOAD, PATH, ...) was applied to bwrap itself, so a caller under a permission ceiling could run code outside the sandbox. The launcher and bwrap now start from absolute paths with an empty environment; the app environment and sandbox options reach bwrap through --args on a private memfd, so they also stay out of the world-readable command line. Apps receive the same variables as before. Group dropping is best effort: hosts without uidmap, a subordinate range, util-linux 2.40, unprivileged user namespaces or with no_new_privs keep the previous behavior; doctor reports why under sandbox_group_drop. Sandboxed tmux clients drop groups too.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 330a69b9-32c4-4dc8-96ad-e4ce7d3e5254) |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
There was a problem hiding this comment.
This is an auto review done by revuto.
The NUL-delimited argument transport allows a crafted cwd to inject bubblewrap options and bypass mount restrictions.
Reject NUL bytes in launch variables, sandbox arguments and profile mount paths, drop --clearenv so bubblewrap older than 0.5 keeps working, run the group-drop preflight through bubblewrap and cache it per process, and keep the argument memfd off the standard streams. The isolation smoke expects dropped groups only on hosts that report the capability.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: e7cc9849-de8f-48cf-8277-4ae9a1a3871d) |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: cd7b18ff-2e32-428a-98f1-261cb44d6451) |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
There was a problem hiding this comment.
This is an auto review done by revuto.
Revuto completed the review and found no evidence-backed concerns.
|
Self-review at 59ebdab. What it does: sandboxed launches run bubblewrap under Checked:
Local validation (this head's code): cargo fmt, clippy -D warnings, 256 tests in both TMPDIR modes, doctor, runtime isolation smoke on x11 and i3 (groups dropped), terminal lifecycle, integration smoke: all pass. Review threads: three, all answered and resolved. |
Sandboxed launches drop the owner's supplementary groups where the host allows it, and launch variables no longer reach the sandbox launcher.
Groups. Unprivileged bubblewrap keeps the caller's supplementary groups;
dockerandlxdare root-equivalent. When the owner has supplementary groups, sandboxed launches (and sandboxed tmux clients) start throughunshare --userwith maps from the setuidnewuidmap/newgidmap, drop every supplementary group and exec bubblewrap as the same uid with no capabilities. This is best effort: hosts withoutuidmap, a subordinate range, util-linux 2.40, unprivileged user namespaces (Ubuntu's AppArmor restriction) or withno_new_privskeep the previous behavior. A cached preflight decides;doctorreports the outcome undersandbox_group_drop.Launcher environment. The launch environment (
LD_PRELOAD,PATH, ...) used to apply tobwrapitself, so a caller under a permission ceiling could run code outside the sandbox. The launcher and bwrap now start from absolute paths with an empty environment; the app environment and sandbox options reach bwrap through--argson a private close-on-exec memfd that only the child inherits, so they also stay out of the world-readable command line. Apps receive the same variables as before.Validation on the rig (groups include docker, lxd, adm):
runtime_isolation_smoke.pynow fails when a sandboxed app keeps a supplementary group: installed 0.4.0 fails it; this branch passes on x11 and i3.no_new_privsstage, which keeps groups as before), full integration smoke with a sandboxed browser: pass.Note
High Risk
Changes core sandbox spawn semantics (user namespaces, group membership, and how env reaches bubblewrap); misconfiguration or preflight bugs could break launches or weaken isolation on hosts that previously relied on group-based access.
Overview
Hardens sandboxed bubblewrap launches so untrusted workspace apps no longer inherit the owner’s supplementary groups (e.g.
docker/lxd) when the host can support it, and so caller-controlled launch environment cannot influence the sandbox launcher.When the user has extra groups, sandboxed app and tmux-client paths can prepend an
unshare --userwrapper that maps IDs vianewuidmap/newgidmap, drops all supplementary groups, then execsbwrapwith a cleared environment. Capability is cached and preflight-tested once per process; unsupported hosts keep the old behavior.doctornow reports this undersandbox_group_drop, docs adduidmapand a README limitations section, andruntime_isolation_smoke.pyfails if a sandboxed probe still sees supplementary groups when dropping is expected.Sandboxed apps are built through
sandboxed_command: launcher and bwrap use absolute paths andenv_clear(), while workspace env and bwrap flags go through--argson a close-on-exec memfd (not the world-visible argv). Profile mount paths reject NUL bytes to match that encoding.Reviewed by Cursor Bugbot for commit 59ebdab. Configure here.