FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(supervisor): drop sandbox child capability bounding set by alangou · Pull Request #2001 · NVIDIA/OpenShell · GitHub

/ OpenShell Public

fix(supervisor): drop sandbox child capability bounding set - #2001

Merged
alangou merged 1 commit into
mainfrom
alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and
Jun 29, 2026
ConversationCommits (1)ChecksFiles changed
Merged

fix(supervisor): drop sandbox child capability bounding set#2001
alangou merged 1 commit into
mainfrom
alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and

Conversation

alangou commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Drop the Linux capability bounding set in the common sandbox child privilege-drop path so workloads and openshell connect shells cannot regain container-granted capabilities after exec.

Related Issue

Closes #1452

Changes

  • Added Linux-only capability bounding set reduction in drop_privileges() after initgroups/setgid and before setuid.
  • Drops capabilities from 0..=cap_last_cap, with CAP_SETPCAP dropped last and a graceful fallback when CAP_SETPCAP is unavailable.
  • Added focused Linux-only regression coverage and documented the child-process invariant in architecture/sandbox.md.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E not applicable; no e2e files changed

Additional checks run:

  • mise run fmt && mise run e2e && mise run e2e:kubernetes && mise run e2e:docker
  • cargo test -p openshell-supervisor-process --lib process::tests -- --nocapture
  • cargo test -p openshell-supervisor-process --lib ssh::tests::pre_exec_always_calls_drop_privileges -- --nocapture

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

elezar reviewed Jun 25, 2026
View reviewed changes

elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A codex-assisted review:

I think this still fails closed incorrectly for the Podman path. drop_capability_bounding_set() returns Ok(()) when CAP_SETPCAP is
not effective, and also when the first PR_CAPBSET_DROP returns EPERM. But the Podman driver currently drops SETPCAP from the
supervisor container, so that path leaves the child bounding set unchanged while still spawning the workload/connect shell.

Could we either keep SETPCAP available to the supervisor until child setup, or fail the spawn when the bounding set cannot be cleared
and is still nonempty? This may also be a good place to use capctl rather than custom /proc parsing and raw prctl;
capctl::caps::bounding::clear() plus an explicit “EPERM is only OK if the bounding set is already empty” check would make the invariant
clearer.

The current regression test skips when CAP_SETPCAP is unavailable, so it would not catch the Podman-relevant failure mode.

johntmyers commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

A codex-assisted review:

I think this still fails closed incorrectly for the Podman path....

+1. Seems like different drivers can do different drops on their own which would impact this common code that runs driver-agnostically.

alangou force-pushed the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch 3 times, most recently from 0c0c87e to b629752 Compare June 26, 2026 13:11
alangou requested a review from elezar June 26, 2026 13:27

alangou commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@johntmyers @elezar I have implemented the requested changes

elezar reviewed Jun 26, 2026
View reviewed changes
Comment thread crates/openshell-server/src/compute/mod.rs Outdated
alangou force-pushed the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch from b629752 to 6acb0bc Compare June 26, 2026 15:14
alangou requested a review from elezar June 26, 2026 16:05
cjagwani mentioned this pull request Jun 26, 2026
Closed

johntmyers commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

looks good @alangou - will need to update the README for openshell-driver-podman as it still mentions SETPCAP is dropped

fix(supervisor): drop sandbox child capability bounding set
b0bb43b 
Reduce the Linux capability bounding set in the common privilege-drop path before executing sandbox workloads or connect shells and use capctl

Signed-off-by: Adrien Langou <alangou@nvidia.com>
alangou force-pushed the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch from 6acb0bc to b0bb43b Compare June 29, 2026 08:44

alangou commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@johntmyers the readme is updated (auto-merge is enable)

alangou enabled auto-merge (squash) June 29, 2026 09:30
elezar added the test:e2e Requires end-to-end coverage label Jun 29, 2026

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

Label test:e2e applied for b0bb43b. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

alangou merged commit afc06dd into main Jun 29, 2026
62 checks passed
alangou deleted the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch June 29, 2026 15:10
waynesun09 mentioned this pull request Jun 30, 2026
Merged
2 tasks
ralphbean mentioned this pull request Jun 30, 2026
Closed
ben-alkov pushed a commit to ben-alkov/fullsend that referenced this pull request Jun 30, 2026
fix: pin supervisor image to match openshell CLI version
facd02e 
The OpenShell gateway defaults to pulling
ghcr.io/nvidia/openshell/supervisor:latest for the sandbox supervisor
binary. When NVIDIA released v0.0.73 (2026-06-30 15:31 UTC), the
:latest tag was re-pointed to a supervisor that drops the Linux
capability bounding set (NVIDIA/OpenShell#2001), which crashes with
EINVAL in rootless Podman on GitHub Actions runners.

Write a gateway.toml that pins supervisor_image to the version from
openshell-version.sh so the supervisor always matches the installed
gateway and is immune to upstream :latest tag changes.

Fixes fullsend-ai#2792

Assisted-by: Claude (investigation, fix)
Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09 mentioned this pull request Jun 30, 2026
Closed
3 tasks
elezar mentioned this pull request Jul 1, 2026
Closed
7 tasks
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

elezar elezar approved these changes
mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
maxamillion Awaiting requested review from maxamillion
derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

Assignees

No one assigned

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(sandbox): support --cap-drop / capability scoping on sandbox create

3 participants

Back | FazBrowse Home | New Git URL