refactor(sandbox): remove unreachable root-side identity and workspace code - #3979
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
…e code RFC 0012 moved the workload into its own capability-free container that starts as the final sandbox identity. The sandbox no longer runs a root supervisor that prepares the filesystem, rewrites account files, resolves OCI USER entries, or drops privileges before launching the workload, so that code had no production callers. Remove the unreachable paths and their tests: - prepare_filesystem / prepare_filesystem_with_identity, the /sandbox and OCI workspace chown preparation, and the root-side workspace validation (validate_oci_workspace and its privilege-dropped subprocess) - the hidden validate-workspace subcommand - drop_privileges / drop_privileges_with_identity, capability bounding set clearing, validate_sandbox_user/group, and /etc/passwd and /etc/group rewriting - the sandbox-side OCI USER resolver (identity.rs) and ResolvedProcessIdentity; the boundary now writes the driver-resolved UID/GID into the policy directly The workspace check that still runs inside the capability-free boundary (validate_oci_workspace_as_effective_identity) is unchanged. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…ock comments Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
3d75d44 to
0124680
Compare
|
Label |
PR Review StatusThis maintainer-authored cleanup is project-valid, and the initial code review found no blocking issues. The required E2E coverage has not started because Action required: a maintainer must open Branch E2E Checks run 36768946391 and choose Re-run all jobs so the standard E2E suite runs for this head. Blocking findings:
Carried findings:
Non-blocking suggestions:
Gator metadata
|
drew
added
gator:blocked
drew
added
gator:in-review
Monitoring CompleteMonitoring is complete because this PR has merged. Final status: Gator found no blocking code issues, required checks including E2E passed, and maintainer approval was present before merge. I removed the active Gator metadata
|
2 participants
Summary
Remove sandbox code left over from the architecture before RFC 0012. These paths no longer run, but still show up in searches and reviews as if they handle image identity and workspace setup.
Why RFC 0012 made this code obsolete
Before RFC 0012, OpenShell's supervisor and the agent shared the image's filesystem. The supervisor started as root and prepared the environment before launching the agent: it resolved the image's
USER, prepared workspace ownership, updated account files, and dropped privileges. Those operations also needed checks to prevent an unusual image layout—such as symlinks—from causing root to change the wrong files.RFC 0012 separated the supervisor from the workload. The compute driver now resolves the workload identity, and the sandbox runs in the workload environment as that identity, without capabilities. Where a privileged bootstrap is necessary, a separate, limited bootstrap path prepares the environment and drops privileges before the sandbox accepts a supervisor connection.
The old root-side preparation code therefore has no production callers. Keeping it does not provide additional protection; it only makes the old architecture appear to still be in use.
This PR removes those unused paths. It keeps the checks that run today, including workload identity verification, capability checks, Landlock enforcement, and workspace validation performed as the workload user. The one live identity-wrapper call is replaced with the same UID/GID assignments it already performed.
Why this comes first in the stack
This is the first of four stacked PRs split out of #3801:
WORKDIRon Podman.USER/WORKDIRe2e tests across Docker and Podman.Removing the obsolete code first makes those changes easier to reason about: reviewers can see which component actually resolves identity, prepares the workspace, and enforces access. The later fixes target those active paths, and the shared e2e tests run with the cleanup underneath them—not alongside unused code that looks like it still handles workspace setup.
Related Issue
No issue required: this removes unused implementation paths without changing supported sandbox runtime behavior. It was found while working on #2526 / #3801.
Changes
process.rs: removeprepare_filesystem*(the/sandboxand OCI workspace chown step), the root-side OCI workspace validation and its privilege-dropped subprocess,drop_privileges*, capability bounding-set clearing,validate_sandbox_user/group*,/etc/passwdand/etc/grouprewriting,ResolvedProcessIdentity, and their tests.main.rs: remove the hiddenvalidate-workspacesubcommand, which only that unused subprocess path invoked.identity.rs: delete the sandbox-side imageUSERresolver. The drivers already resolve the identity.boundary_server.rs: write the driver-resolvedworkload_identityUID/GID intopolicy.process.run_as_user/groupdirectly instead of going through the removed resolver.validate_oci_workspace_as_effective_identity, the workspace check that still runs inside the capability-free boundary (called fromdelegated.rs).About 3,200 lines removed across five files in
openshell-sandbox. This removes Rust exports from the runtime crate and the hidden validation command; no in-repository consumer depends on the removed exports, and no driver, script, or workflow invokes that command.Testing
mise run pre-commitpassescargo clippy -p openshell-sandbox --all-targets -D warningsis clean foraarch64-unknown-linux-gnu(cross-built withcargo-zigbuild) and on the macOS host.oci-imagesuite at the top of this stack passes 4/4 on a local rootful Podman gateway with this change underneath.Checklist