fix(sandbox): restore GPU procfs baseline by elezar · Pull Request #1522 · NVIDIA/OpenShell

elezar · 2026-05-22T13:47:40Z

Summary

Restore CUDA GPU startup compatibility by promoting /proc from
filesystem_policy.read_only to filesystem_policy.read_write when /proc
is part of the active GPU runtime baseline.

This keeps the change intentionally narrow. The existing baseline enrichment
already places /proc in the GPU read-write baseline because CUDA writes
/proc/<pid>/task/<tid>/comm during initialization. The missing behavior was
that an existing read-only /proc entry caused enrichment to skip the
read-write baseline path. This PR restores that promotion and emits an
informational log message when it happens.

Broader handling for user-supplied policy conflicts and explicit baseline
conflict controls is left to follow-up work such as #1629.

Related Issue

Fixes #1486

Related follow-up: #1629

Changes

Promote /proc from read_only to read_write when the GPU read-write
baseline requires it.
Preserve existing behavior for other read-only/read-write baseline conflicts.
Emit an informational log when /proc is promoted for GPU runtime
compatibility.
Add a regression test covering GPU baseline enrichment without network
policy.

Testing

mise exec -- cargo fmt --all
mise exec -- cargo test -p openshell-sandbox --lib baseline_tests -- --nocapture
mise run pre-commit completed Helm lint, Rust format, Rust check, Rust clippy, markdown lint, and license checks; python:proto failed in the parallel run because grpc_tools was missing after .venv recreation.
mise run python:proto

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture/docs updated (not applicable for this minimal runtime fix)

github-actions · 2026-05-22T13:48:05Z

🌿 Preview your docs: https://nvidia-preview-pr-1522.docs.buildwithfern.com/openshell

pimlock

LGTM with a few nits and questions.

pimlock · 2026-05-29T03:29:43Z

+and writable work paths needed by the proxy and shell environment. GPU runtimes
+add the NVIDIA or WSL2 device nodes exposed inside the sandbox and promote
+`/proc` to read-write for default-like policies because CUDA initialization


Is this sentence still accurate?

promote /proc to read-write for default-like policies because CUDA initialization

Right now, the write access comes from the allow_cuda_procfs_writes patch, right? But the policy doesn't get /proc as read-write?

No. This should have been removed. Let me rework this again for accuracy.

pimlock · 2026-05-29T03:30:52Z

+    fn allow_cuda_procfs_writes_allows_descendant_comm_write() {
+        if std::env::var_os(PROCFS_WRITE_HELPER_ENV).is_some() {
+            return;
+        }


From what I can tell the allow_cuda_procfs_writes_allows_descendant_comm_write launches the current test binary with PROCFS_WRITE_HELPER_ENV, and allow_cuda_procfs_writes_helper only runs real helper logic in that child process.

Could we add a short comment or rename the helper test to make that harness pattern clearer? I assume a subprocess is needed because Landlock restrictions are irreversible for the current process.

I'm going to remove the implicit broadening of the permissions from this PR and defer it to #1628. We can look at improving the readability of the tests there.

pimlock · 2026-05-29T03:36:40Z

+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum BaselinePolicySource {
+    DefaultLike,


Could be worth documenting what this means, from what I can tell it's "the source is either the hardcoded default policy or the policy baked into the sandbox image", is this right?

Yes, running through this again, I think the naming is not what it should be. The differences we are trying to capture is a policy that CAN be normalized (i.e. /proc elevated to read-write if GPUs are requested) such as gateway-provided baseline policies, and users-supplied policies where /proc is explicitly specified where we must not increas priviledges.

I will make a pass with some renames / clarifying comments.

pimlock · 2026-05-29T03:41:55Z

+                modified |= fs.read_only.len() != before;
+            }
+            continue;
+        }


Reading this out loud:

we are enriching with a read_write path and that path already exists in the policy

if the policy is default-like and the enrichment comes from the GPU baseline, then remove that path from read_only paths.

This is what happens here, right? What scenario would this happen in?

Default -> implies a policy that was not explicitly specified for this sandbox by the user (e.g. gateway imposed policy) or the default fallback. Since /proc is included in PROXY_BASELINE_READ_ONLY we need to ensure that -- if /proc was not specified explicitly (i.e. is "default-like" in the current set of changes) AND a gpu is requested for the sandbox, we need to remove it from the read_only set and then add it to the read_write set.

Let me see if I can come up with a more concrete scenario.

OK. I think I misunderstood some things here -- or made assumptions that didn't hold. The only case when we would allow a promotion for this PR was if NO other policy was specified by the user and the default defined in code was applied. (This was more clear in later iterations of this PR).

I don't know whether that's the way we want to go since allowing control of this requires an admin to remove /proc from the policies they specify OR set /proc to read-write for ALL sandboxes. I have created #1629 which would allow an admin to opt-out of promotion to read-write for a set of paths (or path patterns). That may be a better solution than this PR.

pimlock · 2026-05-29T03:52:07Z

+    } else {
+        BaselinePolicySource::Custom
+    }
+}


I found this part a bit confusing -> the policy coming from the sandbox image is considered DefaultLike based on https://github.com/NVIDIA/OpenShell/pull/1522/changes#diff-84c5d46e0c1e203a6a93f4a5e29986d5dc6787f11230c2e101d820fca482c1b4R2296

But it's considered Custom based on this check.
Also a DefaultLike policy that is enriched is also then considered Custom. Maybe there are some other names for this to make it clearer?

Looking at this again, I think that the policies coming from the sandbox image need more scrutiny. I'll make sure that it's consistent/clear here and we can create an internal issue to discuss.

pimlock · 2026-05-29T03:58:47Z

            // TLS handshakes.
            grpc_retry("Policy discovery sync", || {
                grpc_client::discover_and_sync_policy(endpoint, id, sandbox, &discovered)
            })
            .await?
        };

        // Ensure baseline filesystem paths are present for proxy-mode
        // sandboxes.  If the policy was enriched, sync the updated version
        // back to the gateway so users can see the effective policy.
-        let enriched = enrich_proto_baseline_paths(&mut proto_policy);
+        let enriched = enrich_proto_baseline_paths(&mut proto_policy)?;
        if enriched
            && let Some(sandbox_name) = sandbox.as_deref()
            && let Err(e) = grpc_client::sync_policy(endpoint, sandbox_name, &proto_policy).await
        {
            warn!(
                error = %e,
                "Failed to sync enriched policy back to gateway (non-fatal)"
            );
        }


This is outside of changes here, but the comment in the BaselinePolicySource got me here (with how the enrich_proto_baseline_paths uses Custom).

I think the flow in this function could be improved:

line 2278, where we got policy from the server: lines 2313-2325 should likely live there, because that part is only applicable to when we get the policy from the gateway, as if we got the policy as default/from the image, we already enriched it, so there is no need to do that again?

unless there is a situation in which this is still useful, even after the policy was initially enriched? Maybe safer to leave it, but maybe worth determining the source in this function, e.g. having the if in 2277 to capture both the policy and source?

pimlock · 2026-05-29T04:27:03Z

+    let result = ruleset
+        .add_rule(PathBeneath::new(
+            path_fd,
+            make_bitflags!(AccessFs::{WriteFile}),


How much narrower is this WriteFile from a standard read_write access? I'm wondering mostly because with /proc being in added with read_write to the policy, the user analyzing what's allowed sees it in the policy. It's more risky, but explicit.

With this, the /proc is in the policy as read-only, but some narrower scope is allowed, but without any mention in the policy.

So it's between "wider, but explicit" vs "narrower, but implicit". I'm not sure what are the risks of the former, but I think that may be a better option. WDYT?

The read-write permissions that OpenShell applies are taken from AccessFs::from_all: https://docs.rs/landlock/0.4.4/src/landlock/fs.rs.html#104

This includes the from_read: https://docs.rs/landlock/0.4.4/src/landlock/fs.rs.html#116-118
And from_write: https://docs.rs/landlock/0.4.4/src/landlock/fs.rs.html#130-143 (which is ABI version dependent).

Assuming V1 this means that the additional permissions that promoting to read-write provide are:

WriteFile

RemoveDir

RemoveFile

MakeChar

MakeDir

MakeReg

MakeSock

MakeFifo

MakeBlock

MakeSym

I think that in the case of OpenShell, where auditing what is allowed/accessed is important, explicit behaviour should be favoured over implicit behaviour. Does this maybe mean that we should keep the broader behaviour for now (the first commit) and then look at how we can better surface the permissions that we are adding? Perhaps adding a finer granularity for the policies makes sense?

I have created #1628 to track looking into this properly instead of adding this as a "quick" fix on top of this PR.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

elezar · 2026-06-01T19:29:39Z

Thanks for your initial review @pimlock. After the initial back and forth, I realised that there were a number of edge cases that I was not considering. I believe I was trying to detect user intent with insufficient signal and as such have updated this PR to ALWAYS promote /proc to read-write if GPUs are requested and instead capture explicit intent in #1629 as a follow-up. This PR would unblock the GPU-enabled tests, but I'm happy to continue iterating on it if required.

elezar requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners May 22, 2026 13:47

elezar mentioned this pull request May 22, 2026

fix(sandbox): decouple GPU baseline from network policy #1524

Merged

6 tasks

elezar changed the base branch from main to fix/1486-gpu-enrichment-no-network/elezar May 22, 2026 14:06

Base automatically changed from fix/1486-gpu-enrichment-no-network/elezar to main May 27, 2026 08:20

elezar force-pushed the fix/1486-gpu-sandbox-filesystem-policy/elezar branch from 96a1caa to 59e399a Compare May 27, 2026 09:02

elezar mentioned this pull request May 28, 2026

feat(gpu): derive sandbox access requirements from CDI specs #1606

Open

17 tasks

elezar force-pushed the fix/1486-gpu-sandbox-filesystem-policy/elezar branch from 12bde4d to d73e6de Compare May 28, 2026 19:22

pimlock reviewed May 29, 2026

View reviewed changes

This was referenced May 29, 2026

feat(sandbox): narrow GPU procfs permissions and surface runtime additions #1628

Open

feat(policy): add runtime baseline conflict controls #1629

Open

elezar force-pushed the fix/1486-gpu-sandbox-filesystem-policy/elezar branch from d73e6de to 2f3b5b2 Compare May 29, 2026 14:31

fix(sandbox): restore GPU procfs baseline

a0171ff

Signed-off-by: Evan Lezar <elezar@nvidia.com>

elezar force-pushed the fix/1486-gpu-sandbox-filesystem-policy/elezar branch from 2f3b5b2 to a0171ff Compare June 1, 2026 18:29

elezar changed the title ~~fix(sandbox): restore GPU filesystem baseline~~ fix(sandbox): restore GPU procfs baseline Jun 1, 2026

elezar requested a review from pimlock June 1, 2026 19:29

Conversation

elezar commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

pimlock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elezar commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

elezar commented May 22, 2026 •

edited

Loading