From b671171091c32d12113f97945b6ef505eabaa7c6 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Wed, 23 Sep 2026 03:36:56 +0000 Subject: [PATCH 1/4] fix(sandbox): qualify task memory against workload child (#3574) Signed-off-by: Drew Newberry --- architecture/sandbox.md | 7 + .../src/linux/seccomp_notify.rs | 127 +++++++++++++----- .../src/linux/task_memory.rs | 37 +++++ crates/openshell-sandbox/src/main.rs | 11 +- docs/reference/support-matrix.mdx | 5 + skills/debug-openshell-cluster/SKILL.md | 2 +- 6 files changed, 148 insertions(+), 41 deletions(-) diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 4e7ddd0385..2f9d88a1fb 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -205,6 +205,13 @@ The sandbox reserves `SIGUSR2` with a non-restarting no-op handler for these broker threads; startup rejects a conflicting handler. This signal disposition is process-global kernel state, while registrations and cancellation state are owned by the broker. Workload exec resets the caught handler to its default. +The broker copies pointer-bearing syscall arguments from a same-UID workload +child with `process_vm_readv` / `process_vm_writev`, falling back to +`/proc//mem` when those system calls are unavailable or blocked. Runtime +qualification keeps the trusted broker non-dumpable and proves read/write +access against a dumpable child, matching the production process topology. +It never treats access to the broker's own memory as workload evidence. + This sandbox runtime requires Landlock ABI v3 (Linux 6.2, or an equivalent vendor backport). The seccomp listener is installed in one of two cancellation modes, and the launch confirmation enforces the invariant diff --git a/crates/openshell-isolation-interface/src/linux/seccomp_notify.rs b/crates/openshell-isolation-interface/src/linux/seccomp_notify.rs index 4f86844d10..c9bbb0fafa 100644 --- a/crates/openshell-isolation-interface/src/linux/seccomp_notify.rs +++ b/crates/openshell-isolation-interface/src/linux/seccomp_notify.rs @@ -162,18 +162,11 @@ impl NotificationProbeReport { self.features.0 & 2 != 0 } - /// Whether process-VM read and write syscalls are admitted for same-process - /// memory, before the stronger child-credential probe runs in a driver. - #[must_use] - pub fn task_memory_copy(self) -> bool { - self.features.0 & 4 != 0 - } - /// Whether connected null-destination `sendto` bypassed notification while /// destination-bearing and unsafe-flag variants remained mediated. #[must_use] pub fn connected_send_fast_path(self) -> bool { - self.features.0 & 8 != 0 + self.features.0 & 4 != 0 } } @@ -465,11 +458,10 @@ pub fn install_workload_listener() -> io::Result { pub fn probe_notification_api() -> io::Result { let wait_killable_recv = probe_scalar_round_trip()?; probe_addfd_send()?; - probe_task_memory_copy()?; probe_connected_sendto_fast_path()?; Ok(NotificationProbeReport { wait_killable_recv, - features: NotificationProbeFeatures(1 | 2 | 4 | 8), + features: NotificationProbeFeatures(1 | 2 | 4), }) } @@ -585,32 +577,6 @@ fn probe_addfd_send() -> io::Result<()> { Ok(()) } -fn probe_task_memory_copy() -> io::Result<()> { - let source = 0x1122_3344_5566_7788_u64; - let tid = std::process::id(); - let mut source_bytes = [0_u8; size_of::()]; - crate::linux::task_memory::read_exact( - tid, - std::ptr::addr_of!(source) as u64, - &mut source_bytes, - )?; - let mut copied = u64::from_ne_bytes(source_bytes); - if copied != source { - return Err(io::Error::other("task-memory probe read wrong value")); - } - - let replacement = 0xaabb_ccdd_eeff_0011_u64; - crate::linux::task_memory::write_exact( - tid, - std::ptr::addr_of_mut!(copied) as u64, - &replacement.to_ne_bytes(), - )?; - if copied != replacement { - return Err(io::Error::other("task-memory probe wrote wrong value")); - } - Ok(()) -} - fn probe_connected_sendto_fast_path() -> io::Result<()> { let mut pair = [-1; 2]; // SAFETY: `pair` points to storage for exactly two returned descriptors. @@ -991,8 +957,12 @@ fn ioctl_ptr(fd: RawFd, request: libc::c_ulong, argument: *mut libc::c_void) -> #[cfg(test)] mod tests { + use std::process::Command; + use super::*; + const NONDUMPABLE_PROBE_CHILD: &str = "OPENSHELL_NONDUMPABLE_PROBE_CHILD"; + #[test] fn filter_rejects_empty_syscall_set() { let error = install_listener(&[]).err().expect("empty filter must fail"); @@ -1004,10 +974,93 @@ mod tests { let report = probe_notification_api().expect("active notification probe"); assert!(report.notification_round_trip()); assert!(report.addfd_send()); - assert!(report.task_memory_copy()); assert!(report.connected_send_fast_path()); } + #[test] + fn notification_probe_ignores_unavailable_self_task_memory() { + if std::env::var_os(NONDUMPABLE_PROBE_CHILD).is_some() { + // Match the production broker posture and Kata kernels that omit + // CONFIG_CROSS_MEMORY_ATTACH. The notification API probe must not + // inspect this trusted process through /proc/self/mem: a + // nondumpable non-root process cannot open that file, while the + // separately qualified dumpable workload child remains readable. + if unsafe { libc::geteuid() } == 0 { + // SAFETY: this disposable subprocess permanently drops its + // supplementary groups and root identity before probing. + assert_eq!(unsafe { libc::setgroups(0, std::ptr::null()) }, 0); + assert_eq!(unsafe { libc::setgid(65_534) }, 0); + assert_eq!(unsafe { libc::setuid(65_534) }, 0); + } + // SAFETY: PR_SET_DUMPABLE accepts one scalar flag and only + // tightens this disposable test process. + assert_eq!(unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) }, 0); + let self_mem_error = std::fs::File::open("/proc/self/mem") + .expect_err("nondumpable unprivileged self memory must be inaccessible"); + assert_eq!(self_mem_error.kind(), io::ErrorKind::PermissionDenied); + install_process_vm_enosys_filter().expect("install process-VM ENOSYS filter"); + probe_notification_api().expect("probe notification API without self task memory"); + return; + } + + let executable = std::env::current_exe().expect("resolve test executable"); + let status = Command::new(executable) + .arg("--exact") + .arg("linux::seccomp_notify::tests::notification_probe_ignores_unavailable_self_task_memory") + .arg("--nocapture") + .env(NONDUMPABLE_PROBE_CHILD, "1") + .status() + .expect("run disposable nondumpable probe process"); + assert!(status.success(), "nondumpable probe child failed: {status}"); + } + + fn install_process_vm_enosys_filter() -> io::Result<()> { + const SECCOMP_RET_ERRNO: u32 = 0x0005_0000; + let syscall_number = |number: libc::c_long| { + u32::try_from(number).map_err(|_| io::Error::other("syscall number does not fit u32")) + }; + let mut instructions = [ + stmt(BPF_LD_W_ABS, SECCOMP_DATA_NR_OFFSET), + jump( + BPF_JMP_JEQ_K, + syscall_number(libc::SYS_process_vm_readv)?, + 0, + 1, + ), + stmt(BPF_RET_K, SECCOMP_RET_ERRNO | libc::ENOSYS.unsigned_abs()), + jump( + BPF_JMP_JEQ_K, + syscall_number(libc::SYS_process_vm_writev)?, + 0, + 1, + ), + stmt(BPF_RET_K, SECCOMP_RET_ERRNO | libc::ENOSYS.unsigned_abs()), + stmt(BPF_RET_K, SECCOMP_RET_ALLOW), + ]; + let length = u16::try_from(instructions.len()) + .map_err(|_| io::Error::other("test seccomp filter is too large"))?; + let mut program = libc::sock_fprog { + len: length, + filter: instructions.as_mut_ptr(), + }; + set_no_new_privileges()?; + // SAFETY: program references the complete live test filter. No flags + // are required because the disposable process has one calling thread. + let result = unsafe { + libc::syscall( + libc::SYS_seccomp, + SECCOMP_SET_MODE_FILTER, + 0, + std::ptr::addr_of_mut!(program), + ) + }; + if result < 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } + #[test] fn errno_response_rejects_nonpositive_values() { // The input validation occurs before the listener FD is used. diff --git a/crates/openshell-isolation-interface/src/linux/task_memory.rs b/crates/openshell-isolation-interface/src/linux/task_memory.rs index 4a0422a006..1b37685b3b 100644 --- a/crates/openshell-isolation-interface/src/linux/task_memory.rs +++ b/crates/openshell-isolation-interface/src/linux/task_memory.rs @@ -58,6 +58,7 @@ pub fn read_exact(tid: u32, address: u64, destination: &mut [u8]) -> io::Result< Ok(copied) => require_exact(copied, destination.len(), "task-memory read"), Err(error) if syscall_profile_denied(&error) => { read_exact_from_proc_mem(tid, address, destination) + .map_err(|fallback| fallback_error("read", &error, fallback)) } Err(error) => Err(error), } @@ -104,6 +105,7 @@ pub fn write_exact(tid: u32, address: u64, source: &[u8]) -> io::Result<()> { Ok(copied) => require_exact(copied, source.len(), "task-memory write"), Err(error) if syscall_profile_denied(&error) => { write_exact_to_proc_mem(tid, address, source) + .map_err(|fallback| fallback_error("write", &error, fallback)) } Err(error) => Err(error), } @@ -116,6 +118,27 @@ fn syscall_profile_denied(error: &io::Error) -> bool { ) } +fn fallback_error(operation: &str, process_vm: &io::Error, proc_mem: io::Error) -> io::Error { + let process_vm = describe_error(process_vm); + let proc_mem_description = describe_error(&proc_mem); + io::Error::new( + proc_mem.kind(), + format!( + "task-memory {operation} unavailable: process_vm_{operation}v failed: {process_vm}; /proc//mem failed: {proc_mem_description}" + ), + ) +} + +fn describe_error(error: &io::Error) -> String { + let name = match error.raw_os_error() { + Some(libc::EACCES) => Some("EACCES"), + Some(libc::ENOSYS) => Some("ENOSYS"), + Some(libc::EPERM) => Some("EPERM"), + _ => None, + }; + name.map_or_else(|| error.to_string(), |name| format!("{name} ({error})")) +} + fn read_exact_from_proc_mem(tid: u32, address: u64, destination: &mut [u8]) -> io::Result<()> { let file = std::fs::File::open(format!("/proc/{tid}/mem"))?; let copied = file.read_at(destination, address)?; @@ -376,6 +399,20 @@ mod tests { assert_eq!(destination, source); } + #[test] + fn fallback_error_reports_both_memory_backends() { + let error = fallback_error( + "read", + &io::Error::from_raw_os_error(libc::ENOSYS), + io::Error::from_raw_os_error(libc::EACCES), + ); + let message = error.to_string(); + assert!(message.contains("process_vm_readv failed")); + assert!(message.contains("ENOSYS")); + assert!(message.contains("/proc//mem failed")); + assert!(message.contains("EACCES")); + } + #[test] fn rejects_invalid_ranges_before_syscall() { let mut byte = [0_u8; 1]; diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index d6d2389e77..4dad330771 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -197,6 +197,11 @@ fn qualify_runtime() -> Result<(openshell_sandbox::RuntimeQualification, Qualifi openshell_isolation_interface::linux::task_memory::probe_child_access() .into_diagnostic() .wrap_err("same-UID task-memory probe")?; + // The successful production-shaped parent-to-child round trip above is + // the task-memory evidence. Do not test /proc/self/mem here: this trusted + // broker is intentionally nondumpable, so that fallback is inaccessible + // even though /proc//mem remains available to mediation. + let task_memory_copy = true; probe_landlock_allow_deny().wrap_err("Landlock allow/deny probe")?; let notification = openshell_isolation_interface::linux::seccomp_notify::probe_notification_api() @@ -233,7 +238,7 @@ fn qualify_runtime() -> Result<(openshell_sandbox::RuntimeQualification, Qualifi landlock_allow_deny: true, seccomp_notification: notification.notification_round_trip(), seccomp_addfd_send: notification.addfd_send(), - task_memory_copy: notification.task_memory_copy(), + task_memory_copy, connected_send_fast_path: notification.connected_send_fast_path(), socket_virtualization: true, dns_relay_bind: true, @@ -257,8 +262,8 @@ fn qualify_runtime() -> Result<(openshell_sandbox::RuntimeQualification, Qualifi addfd_send: notification.addfd_send(), retained_socket_operation: true, proc_fd_identity: true, - task_memory_read: notification.task_memory_copy(), - task_memory_write: notification.task_memory_copy(), + task_memory_read: task_memory_copy, + task_memory_write: task_memory_copy, cancellation: notification.wait_killable_recv, // Legacy plain listener (< 5.19) disables broker output writes; // satisfies the `cancellation || writes_disabled` launch invariant. diff --git a/docs/reference/support-matrix.mdx b/docs/reference/support-matrix.mdx index 8f49dcc945..575058e4b9 100644 --- a/docs/reference/support-matrix.mdx +++ b/docs/reference/support-matrix.mdx @@ -103,12 +103,17 @@ when it runs inside a container or microVM: | -------------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | [Landlock LSM](https://docs.kernel.org/security/landlock.html) | Required | ABI 3 or newer, introduced in Linux 6.2, with Landlock enabled. The mandatory baseline protects private channel and bootstrap files, including against truncation. A filesystem policy's `best_effort` setting never disables this baseline. | | seccomp | Required | Nested user-notification filters and atomic `SECCOMP_IOCTL_NOTIF_ADDFD` with `SECCOMP_ADDFD_FLAG_SEND`, usable under the runtime's existing seccomp profile without added capabilities. The sandbox actively probes these operations before admitting the workload. | +| Task-memory access | Required | The non-dumpable broker must be able to read and write a same-UID, dumpable workload child's memory through `process_vm_readv` / `process_vm_writev` or `/proc//mem`. The sandbox actively probes the production parent-to-child topology before admitting the workload. | | seccomp `WAIT_KILLABLE_RECV` | Recommended (Linux 5.19+) | Keeps a notified workload thread in a kill-only wait so the broker can safely write mediated results into workload memory. Without it (kernels < 5.19, for example RHEL 9.x / RHCOS 5.14) the sandbox still starts, in a reduced **legacy read-only** mode described below. | A kernel version alone does not establish support. A disabled Landlock LSM or a runtime profile that blocks the required seccomp operations causes launch to fail closed. An upstream Linux 6.2 or newer kernel provides the required Landlock ABI; distribution backports must pass the same active qualification. +The broker remains non-dumpable during qualification. A runtime may satisfy +task-memory access through `/proc//mem` even when its kernel omits the +`process_vm_readv` and `process_vm_writev` system calls; OpenShell qualifies the +same parent-to-executed-child access shape used by mediated workloads. ### Legacy read-only mode (kernels before Linux 5.19) diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index aea5c02607..f33743e03f 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -276,7 +276,7 @@ Common findings: - Sandbox runtime image exits before printing `openshell-sandbox --version`: verify the configured image contains a static executable at `/openshell-sandbox`. - A sandbox with explicit `protocol: tcp` endpoints fails before workload readiness: confirm the selected isolation backend advertises TCP mediation, then inspect the sandbox and supervisor logs for protected-channel setup or listener failures. A driver that cannot supply the required outer egress fence and authenticated runtime channel must reject the policy before starting the agent. - Supervisor runtime validation fails: verify `supervisor_image` contains an `/openshell-supervisor` executable from the same release as the sandbox runtime, and that the dynamic loader and shared libraries it links against are available inside that image. `docker run --rm --network none --entrypoint /openshell-supervisor --version` should print that release; a `no such file or directory` error for a binary that exists means the loader or a library is missing. The supervisor runs from its own image and does not need to be static; only `/openshell-sandbox` must be. -- The sandbox fails its enforcement probe: inspect the sandbox log for the exact nested seccomp user-notification, task-memory, Landlock, loopback DNS, or socket-injection check that failed. Do not add capabilities or switch to an unconfined seccomp profile; use a runtime whose default profile permits the unprivileged probe. +- The sandbox fails its enforcement probe: inspect the sandbox log for the exact nested seccomp user-notification, task-memory, Landlock, loopback DNS, or socket-injection check that failed. A runtime may return `ENOSYS` for `process_vm_readv` and `process_vm_writev` while satisfying the production parent-to-workload-child task-memory probe through `/proc//mem`; only failure of both backends is fatal. Do not add capabilities or switch to an unconfined seccomp profile; use a runtime whose default profile permits the unprivileged probe. - A GPU sandbox fails because Docker reports no discovered NVIDIA CDI devices: verify `.DiscoveredDevices` contains entries such as `nvidia.com/gpu=all`, verify `/etc/cdi` or `/var/run/cdi` contains a generated NVIDIA spec, and check that `nvidia-cdi-refresh.service` and `nvidia-cdi-refresh.path` from NVIDIA Container Toolkit are enabled and healthy. The service is a one-shot unit, so `inactive (dead)` can be normal after a successful run; use `systemctl status` and `journalctl` to distinguish success from a skipped or failed refresh. Restart `nvidia-cdi-refresh.service` to regenerate missing or stale CDI specs, then restart or reload Docker and re-check `docker info`. During a graceful gateway restart, Docker, Podman, and VM sandboxes with From c8b20bf0a29f1aa9493592e98f1b9a122d050016 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Tue, 22 Sep 2026 21:12:05 -0700 Subject: [PATCH 2/4] ci(windows): seed caches on windows branch (#3576) Signed-off-by: Drew Newberry --- .../skills/build-openshell-mxc-windows/SKILL.md | 10 ++++++---- .../build-openshell-mxc-windows/reference.md | 2 +- .github/workflows/windows-msvc.yml | 16 ++++++++-------- architecture/windows-msvc-build.md | 14 ++++++++------ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/.agents/skills/build-openshell-mxc-windows/SKILL.md b/.agents/skills/build-openshell-mxc-windows/SKILL.md index 2bf8858a9b..21b49f1a5d 100644 --- a/.agents/skills/build-openshell-mxc-windows/SKILL.md +++ b/.agents/skills/build-openshell-mxc-windows/SKILL.md @@ -30,7 +30,7 @@ The Windows build lane is implemented by these tracked files: | `tasks/windows.toml` | Mise task entry points for `windows:*` commands. | | `tasks/rust.toml`, `tasks/test.toml`, and `tasks/markdown.toml` | Windows routing for compiler-bearing checks, explicit Unix-only test skips, and Markdown dependency setup. | | `tasks/scripts/windows-msvc.ps1` | PowerShell wrapper that enters the Visual Studio developer environment and invokes Cargo. | -| `.github/workflows/windows-msvc.yml` | Opt-in PR lint and test plus advisory main/manual cache seeding and dependent binary builds on native x64 and ARM64 runners. | +| `.github/workflows/windows-msvc.yml` | Opt-in PR lint and test plus advisory `windows` branch cache seeding and dependent binary builds on native x64 and ARM64 runners. | | `architecture/windows-msvc-build.md` | Design notes and validation contract. | | `.agents/skills/build-openshell-mxc-windows/` | This skill and companion reference material. | @@ -194,10 +194,12 @@ The GitHub Actions jobs layer architecture-specific `Swatinem/rust-cache` entries for Cargo registry and dependency target artifacts with sccache's GHA backend for cacheable Rust compiler outputs. Failed runs also save their usable dependency artifacts. Pull-request mirrors labeled `test:windows` run Clippy for the -Windows-supported workspace and e2e crates plus Rust tests. Pushes to `main` and -manual dispatches run the same lint and test commands in a cache-seed job, +Windows-supported workspace and e2e crates plus Rust tests. Pushes to `windows` and +manual dispatches on that branch run the same lint and test commands in a cache-seed job, followed by a dependent release-binary build job. The seed and PR jobs use the -same cache namespaces. Merge queues do not run this workflow. Main/manual seed +same cache namespaces, but pull-request mirrors cannot restore the +`windows` branch cache because GitHub scopes caches by branch. Merge queues do +not run this workflow. Release-branch seed and build jobs use job-level `continue-on-error: true`; opt-in PR jobs report failures normally. Applying the label alone does not start a run: re-run all jobs in the current mirror push run, or push a new mirrored commit. The binaries are not uploaded or published. diff --git a/.agents/skills/build-openshell-mxc-windows/reference.md b/.agents/skills/build-openshell-mxc-windows/reference.md index 7f800ae0f7..5ee638e1a9 100644 --- a/.agents/skills/build-openshell-mxc-windows/reference.md +++ b/.agents/skills/build-openshell-mxc-windows/reference.md @@ -9,7 +9,7 @@ maintaining the existing build-only Windows MSVC lane. |---|---| | `tasks/windows.toml` | Mise task definitions for `windows:*`. | | `tasks/scripts/windows-msvc.ps1` | Visual Studio environment discovery, rustup target setup, Cargo invocation, logs, artifact report. | -| `.github/workflows/windows-msvc.yml` | PR/merge-queue lint and test plus main/manual cache seeding and dependent binary builds on native x64 and ARM64 runners. | +| `.github/workflows/windows-msvc.yml` | Opt-in PR lint and test plus `windows` branch cache seeding and dependent binary builds on native x64 and ARM64 runners. | | `architecture/windows-msvc-build.md` | Human-readable design contract. | ## Commands diff --git a/.github/workflows/windows-msvc.yml b/.github/workflows/windows-msvc.yml index 00c41d9ca9..a268629a22 100644 --- a/.github/workflows/windows-msvc.yml +++ b/.github/workflows/windows-msvc.yml @@ -3,7 +3,7 @@ name: Windows MSVC on: push: branches: - - main + - windows - "pull-request/[0-9]+" workflow_dispatch: @@ -27,12 +27,12 @@ jobs: contents: read pull-requests: read outputs: - should_run: ${{ github.ref == 'refs/heads/main' || steps.gate.outputs.should_run == 'true' }} + should_run: ${{ github.ref == 'refs/heads/windows' || steps.gate.outputs.should_run == 'true' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - id: gate - if: github.ref != 'refs/heads/main' + if: github.ref != 'refs/heads/windows' uses: ./.github/actions/pr-gate with: required_label: test:windows @@ -42,7 +42,7 @@ jobs: needs: pr_metadata if: >- needs.pr_metadata.outputs.should_run == 'true' && - github.ref != 'refs/heads/main' && + github.ref != 'refs/heads/windows' && github.event_name != 'workflow_dispatch' strategy: fail-fast: false @@ -104,12 +104,12 @@ jobs: cache-seed: name: Seed cache (${{ matrix.arch }}) - # Keep main/manual Windows validation advisory while the lane matures. + # Keep release-branch Windows validation advisory while the lane matures. continue-on-error: true needs: pr_metadata if: >- needs.pr_metadata.outputs.should_run == 'true' && - (github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch') + github.ref == 'refs/heads/windows' strategy: fail-fast: false matrix: @@ -155,8 +155,8 @@ jobs: cache-on-failure: "true" cache-bin: "false" - name: Lint Windows-supported crates - # Keep the seed workload identical to the pull-request workload so its - # target and compiler caches are reusable by subsequent pull requests. + # Keep the seed workload identical to the pull-request workload so the + # windows-branch cache reflects the same lint and test commands. run: mise run --skip-tools windows:lint:${{ matrix.arch }} - name: Test run: mise run --skip-tools windows:test:${{ matrix.arch }} diff --git a/architecture/windows-msvc-build.md b/architecture/windows-msvc-build.md index aaf8b1a8fe..198ecaade3 100644 --- a/architecture/windows-msvc-build.md +++ b/architecture/windows-msvc-build.md @@ -69,16 +69,18 @@ creating misleading Windows driver artifacts. The GitHub Actions workflow runs Clippy for the Windows-supported workspace and e2e crates plus Rust tests for pull-request mirror branches labeled `test:windows`. Merge queues do not run this workflow. On -pushes to `main`, a cache-seed job runs the same lint and test commands before a -dependent job builds the release binaries. Manual dispatches exercise the same +pushes to `windows`, a cache-seed job runs the same lint and test commands before a +dependent job builds the release binaries. Manual dispatches on `windows` exercise the same seed-then-build path. The binaries remain CI validation artifacts and are not uploaded or published. Each job restores and saves a dedicated Rust cache containing the Cargo registry and dependency build artifacts, including artifacts from failed runs. The seed job and pull-request job use the same Cargo target and sccache -namespaces. The release build waits for the seed job, then restores its newly -warmed cache rather than compiling concurrently from a cold cache. +namespaces, but GitHub scopes caches by branch. Pull-request mirror branches +cannot restore the `windows` branch cache. The release build waits for the seed +job, then restores its newly warmed cache rather than compiling concurrently +from a cold cache. Windows validation is exposed through `tasks/windows.toml`: @@ -173,9 +175,9 @@ mise run --skip-tools windows:lint: mise run --skip-tools windows:test: ``` -Pushes to `main` and manual dispatches first seed the shared caches with those +Pushes to `windows` and manual dispatches on that branch first seed the shared caches with those same lint and test commands. Both seed and build jobs use job-level -`continue-on-error: true`, so Windows job failures do not fail the main/manual +`continue-on-error: true`, so Windows job failures do not fail the release-branch workflow. Opt-in PR jobs still report failures normally. After the seed job finishes, a separate job executes: From d02ebe2c4b22068960acefc28a2515be97d24c17 Mon Sep 17 00:00:00 2001 From: Divesh Chowdary <47188680+FrostGod@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:57:33 -0700 Subject: [PATCH 3/4] fix(kubernetes): prevent false sandbox suspension (#3567) Signed-off-by: divesh --- .../openshell-driver-kubernetes/src/driver.rs | 148 ++++++++++++++++-- 1 file changed, 131 insertions(+), 17 deletions(-) diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 5d072a0f21..5239f72898 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -102,8 +102,8 @@ const ANNOTATION_SANDBOX_RUNTIME_MAIN_PROCESS_SPEC: &str = const ANNOTATION_SANDBOX_RUNTIME_LOG_LEVEL: &str = "openshell.ai/sandbox-runtime-log-level"; const ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID: &str = "openshell.ai/sandbox-runtime-network-policy-uid"; -const ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION: &str = - "openshell.ai/sandbox-runtime-network-policy-version"; +const ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION: &str = + "openshell.ai/sandbox-runtime-network-policy-generation"; #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum SandboxRuntimeBootstrapPhase { @@ -2480,6 +2480,9 @@ impl KubernetesComputeDriver { let fence_uid = fence.metadata.uid.ok_or_else(|| { KubernetesDriverError::Message("workload NetworkPolicy has no UID".to_string()) })?; + let fence_generation = fence.metadata.generation.ok_or_else(|| { + KubernetesDriverError::Message("workload NetworkPolicy has no generation".to_string()) + })?; let fence_resource_version = fence.metadata.resource_version.ok_or_else(|| { KubernetesDriverError::Message( "workload NetworkPolicy has no resourceVersion".to_string(), @@ -2529,7 +2532,7 @@ impl KubernetesComputeDriver { ANNOTATION_SANDBOX_RUNTIME_WORKLOAD_UID: workload_pod_uid.clone(), ANNOTATION_SANDBOX_RUNTIME_SUPERVISOR_UID: supervisor_uid.clone(), ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID: fence_uid.clone(), - ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION: fence_resource_version.clone(), + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION: fence_generation.to_string(), } } }) @@ -2767,6 +2770,9 @@ impl KubernetesComputeDriver { let fence_uid = fence.metadata.uid.ok_or_else(|| { KubernetesDriverError::Message("workload NetworkPolicy has no UID".to_string()) })?; + let fence_generation = fence.metadata.generation.ok_or_else(|| { + KubernetesDriverError::Message("workload NetworkPolicy has no generation".to_string()) + })?; let fence_resource_version = fence.metadata.resource_version.ok_or_else(|| { KubernetesDriverError::Message( "workload NetworkPolicy has no resourceVersion".to_string(), @@ -2808,7 +2814,7 @@ impl KubernetesComputeDriver { ANNOTATION_SANDBOX_RUNTIME_WORKLOAD_UID: workload_pod_uid.clone(), ANNOTATION_SANDBOX_RUNTIME_SUPERVISOR_UID: supervisor_uid, ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID: fence_uid.clone(), - ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION: fence_resource_version.clone(), + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION: fence_generation.to_string(), } } }) @@ -5489,9 +5495,12 @@ async fn sandbox_from_object_with_sandbox_runtime_readiness( )); let (dependencies, workload_generation, supervisor_generation) = tokio::join!(dependencies, workload_generation, supervisor_generation); - if dependencies != SandboxRuntimeControlAvailability::Available - || workload_generation != SandboxRuntimeControlAvailability::Available - || supervisor_generation != SandboxRuntimeControlAvailability::Available + // A transient API read failure is not evidence that the running + // boundary disappeared. Preserve the CR's published readiness for + // Unknown and let periodic reconciliation retry. + if dependencies == SandboxRuntimeControlAvailability::Unavailable + || workload_generation == SandboxRuntimeControlAvailability::Unavailable + || supervisor_generation == SandboxRuntimeControlAvailability::Unavailable { mark_sandbox_runtime_control_unavailable(&mut sandbox); } @@ -6730,6 +6739,22 @@ fn required_sandbox_annotation( }) } +fn condition_observes_current_generation( + object: &DynamicObject, + condition: &serde_json::Value, +) -> bool { + match ( + object.metadata.generation, + condition + .get("observedGeneration") + .and_then(serde_json::Value::as_i64), + ) { + (Some(generation), Some(observed_generation)) => generation == observed_generation, + // Older Agent Sandbox API versions can omit observedGeneration. + _ => true, + } +} + fn status_from_object(obj: &DynamicObject) -> Option { let status = obj.data.get("status")?; let status_obj = status.as_object()?; @@ -6740,6 +6765,7 @@ fn status_from_object(obj: &DynamicObject) -> Option { .map(|items| { items .iter() + .filter(|condition| condition_observes_current_generation(obj, condition)) .filter_map(condition_from_value) .collect::>() }) @@ -6877,10 +6903,10 @@ fn sandbox_runtime_namespace_fence_generation_matches( == annotations .get(ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID) .map(String::as_str) - && policy.metadata.resource_version.as_deref() - == annotations - .get(ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION) - .map(String::as_str) + && annotations + .get(ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION) + .and_then(|generation| generation.parse::().ok()) + == policy.metadata.generation } fn kubernetes_sandbox_has_stopped_condition(obj: &DynamicObject) -> bool { @@ -6890,8 +6916,9 @@ fn kubernetes_sandbox_has_stopped_condition(obj: &DynamicObject) -> bool { .and_then(serde_json::Value::as_array) .is_some_and(|conditions| { conditions.iter().any(|condition| { - condition.get("type").and_then(serde_json::Value::as_str) - == Some(SANDBOX_SUSPENDED_CONDITION) + condition_observes_current_generation(obj, condition) + && condition.get("type").and_then(serde_json::Value::as_str) + == Some(SANDBOX_SUSPENDED_CONDITION) && condition .get("status") .and_then(serde_json::Value::as_str) @@ -6920,8 +6947,9 @@ fn kubernetes_sandbox_stop_failure(obj: &DynamicObject) -> Option { .as_array()? .iter() .find_map(|condition| { - let is_terminal = condition.get("type").and_then(serde_json::Value::as_str) - == Some(SANDBOX_SUSPENDED_CONDITION) + let is_terminal = condition_observes_current_generation(obj, condition) + && condition.get("type").and_then(serde_json::Value::as_str) + == Some(SANDBOX_SUSPENDED_CONDITION) && condition .get("status") .and_then(serde_json::Value::as_str) @@ -7074,7 +7102,7 @@ fn sandbox_runtime_suspension_completion_patch(resource_version: &str) -> serde_ ANNOTATION_SANDBOX_RUNTIME_WORKLOAD_UID: serde_json::Value::Null, ANNOTATION_SANDBOX_RUNTIME_SUPERVISOR_UID: serde_json::Value::Null, ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID: serde_json::Value::Null, - ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION: serde_json::Value::Null, + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION: serde_json::Value::Null, ANNOTATION_SANDBOX_RUNTIME_READINESS: "unavailable", }, } @@ -8240,6 +8268,58 @@ mod tests { )); } + fn sandbox_runtime_fence_pair_for_test() -> (NetworkPolicy, DynamicObject) { + let policy = NetworkPolicy { + metadata: ObjectMeta { + uid: Some("policy-uid".to_string()), + generation: Some(7), + resource_version: Some("200".to_string()), + ..Default::default() + }, + ..Default::default() + }; + let resource = ApiResource::from_gvk(&GroupVersionKind::gvk( + SANDBOX_GROUP, + SANDBOX_VERSION_V1BETA1, + SANDBOX_KIND, + )); + let mut sandbox = DynamicObject::new("sandbox", &resource); + sandbox.metadata.annotations = Some(BTreeMap::from([ + ( + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID.to_string(), + "policy-uid".to_string(), + ), + ( + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION.to_string(), + "7".to_string(), + ), + ])); + (policy, sandbox) + } + + #[test] + fn sandbox_runtime_fence_reconciliation_uses_generation_not_resource_version() { + let (mut policy, sandbox) = sandbox_runtime_fence_pair_for_test(); + + assert!( + sandbox_runtime_namespace_fence_generation_matches(&policy, &sandbox), + "metadata-only writes must not invalidate an unchanged fence" + ); + + policy.metadata.generation = Some(8); + assert!( + !sandbox_runtime_namespace_fence_generation_matches(&policy, &sandbox), + "a policy spec change must invalidate the recorded fence generation" + ); + + policy.metadata.generation = Some(7); + policy.metadata.uid = Some("replacement-policy-uid".to_string()); + assert!( + !sandbox_runtime_namespace_fence_generation_matches(&policy, &sandbox), + "a replacement policy must invalidate the recorded fence identity" + ); + } + #[test] fn sandbox_runtime_bootstrap_marker_and_age_gate_rollback() { let started = Duration::from_hours(490_896); @@ -8660,6 +8740,40 @@ mod tests { assert!(kubernetes_sandbox_has_stopped_condition(&sandbox)); } + #[test] + fn stale_generation_conditions_do_not_change_current_status() { + let resource = ApiResource::from_gvk(&GroupVersionKind::gvk( + SANDBOX_GROUP, + SANDBOX_VERSION_V1BETA1, + SANDBOX_KIND, + )); + let mut sandbox = DynamicObject::new("sandbox", &resource); + sandbox.metadata.generation = Some(2); + sandbox.data = serde_json::json!({ + "status": { + "conditions": [ + { + "type": "Suspended", + "status": "True", + "reason": "PodTerminated", + "observedGeneration": 1 + }, + { + "type": "Ready", + "status": "True", + "reason": "DependenciesReady", + "observedGeneration": 2 + } + ] + } + }); + + assert!(!kubernetes_sandbox_has_stopped_condition(&sandbox)); + let status = status_from_object(&sandbox).expect("sandbox status"); + assert_eq!(status.conditions.len(), 1); + assert_eq!(status.conditions[0].r#type, "Ready"); + } + #[test] fn beta_stop_requires_suspended_condition_and_deleted_pod() { let resource = ApiResource::from_gvk(&GroupVersionKind::gvk( @@ -11314,7 +11428,7 @@ mod tests { ANNOTATION_SANDBOX_RUNTIME_WORKLOAD_UID, ANNOTATION_SANDBOX_RUNTIME_SUPERVISOR_UID, ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_UID, - ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_VERSION, + ANNOTATION_SANDBOX_RUNTIME_NETWORK_POLICY_GENERATION, ] { assert_eq!( complete["metadata"]["annotations"][annotation], From 069ae6bd966c4ced02b5cff0a26aee0a7f23a82f Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Wed, 23 Sep 2026 07:14:19 +0000 Subject: [PATCH 4/4] fix(vm): scope GPU filesystem enrichment to assigned workloads (#3580) * fix(vm): scope GPU filesystem enrichment to assigned workloads Signed-off-by: Drew Newberry * refactor(supervisor): clarify proxy baseline enrichment Signed-off-by: Drew Newberry --------- Signed-off-by: Drew Newberry --- architecture/sandbox.md | 7 +- crates/openshell-driver-vm/src/driver.rs | 1 + .../openshell-driver-vm/src/isolation/mod.rs | 22 +- .../openshell-sandbox/src/boundary_server.rs | 9 +- crates/openshell-supervisor/src/lib.rs | 295 +++--------------- docs/reference/sandbox-compute-drivers.mdx | 2 + skills/debug-openshell-cluster/SKILL.md | 1 + 7 files changed, 80 insertions(+), 257 deletions(-) diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 2f9d88a1fb..e6f31436a4 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -135,8 +135,11 @@ OpenShell uses overlapping controls rather than a single sandbox primitive: | Outer network fence | The component that owns network enforcement prevents any missed or unsupported kernel path from escaping. Current examples are Docker `network_mode=none`, a NIC-less VM, and Kubernetes NetworkPolicy. | | Policy proxy | Evaluates destination, binary identity, TLS/L7 rules, SSRF checks, and inference interception. | -The supervisor may enrich baseline filesystem allowances for runtime-required -paths, such as proxy support files or GPU device paths when a GPU is present. +The supervisor may enrich baseline filesystem allowances for proxy support +files. GPU allowances are added by the workload-side sandbox only when the +immutable driver resource claims request a GPU and GPU devices are visible +inside the workload. Host supervisor device discovery must not influence these +allowances; a CPU-only VM preserves read-only `/proc` even on a GPU host. These internal allowances must stay sandbox-scoped and avoid exposing host secrets. For example, MXC governed egress grants the generated public CA bundle while the ephemeral CA private key remains in the host proxy's memory. diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 20a9c03bef..243a0ba0ca 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -1557,6 +1557,7 @@ impl VmDriver { agent_uid: sandbox_owner_state.uid, agent_gid: sandbox_owner_state.gid, child_env: merged_environment(&sandbox), + gpu_requested: is_gpu, } .provision() .map_err(|error| Status::failed_precondition(error.to_string()))?; diff --git a/crates/openshell-driver-vm/src/isolation/mod.rs b/crates/openshell-driver-vm/src/isolation/mod.rs index 841323ab23..a660ad98c7 100644 --- a/crates/openshell-driver-vm/src/isolation/mod.rs +++ b/crates/openshell-driver-vm/src/isolation/mod.rs @@ -11,6 +11,7 @@ use openshell_isolation_interface::contract::{ BackendError, OuterFenceGuarantee, OuterFenceGuarantees, ResolvedWorkloadIdentity, }; +use openshell_sandbox_backend::GPU_RESOURCE_CLAIM; use openshell_sandbox_backend::boundary_protocol::{ BoundaryConfig, BoundaryListener, GatewayVerificationKey, SandboxRuntimeDescriptor, SandboxTlsClientConfig, SandboxTlsServerConfig, SandboxTransport, @@ -70,6 +71,7 @@ pub struct VmBoundarySpec { pub agent_uid: u32, pub agent_gid: u32, pub child_env: HashMap, + pub gpu_requested: bool, } /// The protected guest config and matching host descriptor for one VM. @@ -89,10 +91,13 @@ impl VmBoundarySpec { "vm-config".to_string(), self.image_identity.clone(), )?; - let resource_claims = BTreeMap::from([ + let mut resource_claims = BTreeMap::from([ ("vm.generation".to_string(), self.generation.clone()), ("vm.image_identity".to_string(), self.image_identity), ]); + if self.gpu_requested { + resource_claims.insert(GPU_RESOURCE_CLAIM.to_string(), "true".to_string()); + } let outer_fence = VmOuterFenceEvidence { generation: &self.generation, network_device_count: 0, @@ -165,6 +170,12 @@ mod tests { #[test] fn provisioning_binds_identical_resource_claims() { + for gpu_requested in [false, true] { + assert_provisioning_claims(gpu_requested); + } + } + + fn assert_provisioning_claims(gpu_requested: bool) { let session_id = openshell_core::SandboxSessionId::new(); let material = generate_sandbox_tls_material(session_id).unwrap(); let provisioned = VmBoundarySpec { @@ -195,6 +206,7 @@ mod tests { agent_uid: 1000, agent_gid: 1000, child_env: HashMap::new(), + gpu_requested, } .provision() .unwrap(); @@ -203,6 +215,14 @@ mod tests { provisioned.boundary_config.resource_claims, provisioned.runtime_descriptor.resource_claims ); + assert_eq!( + provisioned + .runtime_descriptor + .resource_claims + .get(GPU_RESOURCE_CLAIM) + .map(String::as_str), + gpu_requested.then_some("true") + ); assert_eq!( provisioned.runtime_descriptor.resource_claims["vm.generation"], "generation-1" diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 8e33c316c2..9b10d1b728 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -82,7 +82,12 @@ mod linux { const MAX_REPLAY_LEDGER_ENTRIES: usize = 4096; const MAX_RETAINED_EXEC_PROCESSES: usize = 64; + // NVML may traverse the persistenced socket directory during initialization; + // WSL2 supplies GPU libraries under /usr/lib/wsl and the /dev/dxg device. const GPU_BASELINE_READ_ONLY: &[&str] = &["/run/nvidia-persistenced", "/usr/lib/wsl"]; + // CUDA opens device nodes read-write and writes thread names through + // /proc//task//comm during cuInit(). A /proc/self rule would bind + // to the launcher's inodes, not those of its workload children. const GPU_BASELINE_READ_WRITE: &[&str] = &[ "/dev/nvidiactl", "/dev/nvidia-uvm", @@ -97,8 +102,8 @@ mod linux { } /// Add the filesystem paths required by GPU devices visible inside the - /// workload container. The companion supervisor intentionally has no GPU - /// devices, so it cannot discover these paths on the sandbox's behalf. + /// workload. The supervisor's device namespace can differ from the + /// workload's, so discovery must happen here, gated by the resource claim. fn enrich_gpu_filesystem_paths( policy: &mut openshell_core::policy::SandboxPolicy, gpu_requested: bool, diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index a6e029725b..fd5ed08a48 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -1428,139 +1428,17 @@ const PROXY_BASELINE_READ_ONLY: &[&str] = &[ // policy. const PROXY_BASELINE_READ_WRITE: &[&str] = &["/tmp", "/dev/null"]; -/// GPU read-only paths. -/// -/// `/run/nvidia-persistenced`: NVML tries to connect to the persistenced -/// socket at init time. If the directory exists but Landlock denies traversal -/// (EACCES vs ECONNREFUSED), NVML returns `NVML_ERROR_INSUFFICIENT_PERMISSIONS` -/// even though the daemon is optional. Only read/traversal access is needed. -/// -/// `/usr/lib/wsl`: On WSL2, CDI bind-mounts GPU libraries (libdxcore.so, -/// libcuda.so.1.1, etc.) into paths under `/usr/lib/wsl/`. Although `/usr` -/// is already in `PROXY_BASELINE_READ_ONLY`, individual file bind-mounts may -/// not be covered by the parent-directory Landlock rule when the mount crosses -/// a filesystem boundary. Listing `/usr/lib/wsl` explicitly ensures traversal -/// is permitted regardless of Landlock's cross-mount behaviour. -const GPU_BASELINE_READ_ONLY: &[&str] = &[ - "/run/nvidia-persistenced", - "/usr/lib/wsl", // WSL2: CDI-injected GPU library directory -]; - -/// GPU read-write paths (static). -/// -/// `/dev/nvidiactl`, `/dev/nvidia-uvm`, `/dev/nvidia-uvm-tools`, -/// `/dev/nvidia-modeset`: control and UVM devices injected by CDI on native -/// Linux. Landlock restricts `open(2)` on device files even when DAC allows -/// it; these need read-write because NVML/CUDA opens them with `O_RDWR`. -/// These devices do not exist on WSL2 and will be skipped by the existence -/// check in `enrich_proto_baseline_paths()`. -/// -/// `/dev/dxg`: On WSL2, NVIDIA GPUs are exposed through the DXG kernel driver -/// (DirectX Graphics) rather than the native nvidia* devices. CDI injects -/// `/dev/dxg` as the sole GPU device node; it does not exist on native Linux -/// and will be skipped there by the existence check. -/// -/// `/proc`: CUDA writes to `/proc//task//comm` during `cuInit()` -/// to set thread names. Without write access, `cuInit()` returns error 304. -/// Must use `/proc` (not `/proc/self/task`) because Landlock rules bind to -/// inodes and child processes have different procfs inodes than the parent. -/// -/// Per-GPU device files (`/dev/nvidia0`, …) are enumerated at runtime by -/// `enumerate_gpu_device_nodes()` since the count varies. -const GPU_BASELINE_READ_WRITE: &[&str] = &[ - "/dev/nvidiactl", - "/dev/nvidia-uvm", - "/dev/nvidia-uvm-tools", - "/dev/nvidia-modeset", - "/dev/dxg", // WSL2: DXG device (GPU via DirectX kernel driver, injected by CDI) - "/proc", -]; - -/// Returns true if GPU devices are present in the container. -/// -/// Checks both the native Linux NVIDIA control device (`/dev/nvidiactl`) and -/// the WSL2 DXG device (`/dev/dxg`). CDI injects exactly one of these -/// depending on the host kernel; the other will not exist. -fn has_gpu_devices() -> bool { - std::path::Path::new("/dev/nvidiactl").exists() || std::path::Path::new("/dev/dxg").exists() -} - -/// Enumerate per-GPU device nodes (`/dev/nvidia0`, `/dev/nvidia1`, …). -fn enumerate_gpu_device_nodes() -> Vec { - let mut paths = Vec::new(); - if let Ok(entries) = std::fs::read_dir("/dev") { - for entry in entries.flatten() { - let name = entry.file_name(); - let name = name.to_string_lossy(); - if let Some(suffix) = name.strip_prefix("nvidia") { - if suffix.is_empty() || !suffix.chars().all(|c| c.is_ascii_digit()) { - continue; - } - paths.push(entry.path().to_string_lossy().into_owned()); - } - } - } - paths -} - -fn push_unique(paths: &mut Vec, path: String) { - if !paths.iter().any(|p| p == &path) { - paths.push(path); - } -} - -fn collect_baseline_enrichment_paths( - include_proxy: bool, - include_gpu: bool, - gpu_device_nodes: Vec, -) -> (Vec, Vec) { - let mut ro = Vec::new(); - let mut rw = Vec::new(); - - if include_proxy { - for &path in PROXY_BASELINE_READ_ONLY { - push_unique(&mut ro, path.to_string()); - } - for &path in PROXY_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); - } - } - - if include_gpu { - for &path in GPU_BASELINE_READ_ONLY { - push_unique(&mut ro, path.to_string()); - } - for &path in GPU_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); - } - for path in gpu_device_nodes { - push_unique(&mut rw, path); - } - } - - // A path promoted to read_write (e.g. /proc for GPU) should not also - // appear in read_only — Landlock handles the overlap correctly but the - // duplicate is confusing when inspecting the effective policy. - ro.retain(|p| !rw.contains(p)); - - (ro, rw) -} - -fn active_baseline_enrichment_paths(include_proxy: bool) -> (Vec, Vec) { - let include_gpu = has_gpu_devices(); - let gpu_device_nodes = if include_gpu { - enumerate_gpu_device_nodes() - } else { - Vec::new() - }; - collect_baseline_enrichment_paths(include_proxy, include_gpu, gpu_device_nodes) -} - -/// Collect all active baseline paths for tests and diagnostics. -/// Returns `(read_only, read_write)` as owned `String` vecs. -#[cfg(test)] -fn baseline_enrichment_paths() -> (Vec, Vec) { - active_baseline_enrichment_paths(true) +fn proxy_baseline_paths() -> (Vec, Vec) { + ( + PROXY_BASELINE_READ_ONLY + .iter() + .map(|path| (*path).to_string()) + .collect(), + PROXY_BASELINE_READ_WRITE + .iter() + .map(|path| (*path).to_string()) + .collect(), + ) } fn enrich_proto_baseline_paths_with( @@ -1609,15 +1487,6 @@ where continue; } if fs.read_only.iter().any(|p| p == path) { - if path == "/proc" { - info!( - path, - "Promoting /proc from read-only to read-write for GPU runtime compatibility" - ); - fs.read_only.retain(|p| p != path); - fs.read_write.push(path.clone()); - modified = true; - } continue; } fs.read_write.push(path.clone()); @@ -1628,12 +1497,15 @@ where } /// Ensure a proto `SandboxPolicy` includes the baseline filesystem paths -/// required by proxy-mode sandboxes and GPU runtimes. Paths are only added if +/// required by proxy-mode sandboxes. Paths are only added if /// missing; user-specified paths are never removed. /// /// Returns `true` if the policy was modified (caller may want to sync back). fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { - let (ro, rw) = active_baseline_enrichment_paths(!proto.network_policies.is_empty()); + if proto.network_policies.is_empty() { + return false; + } + let (ro, rw) = proxy_baseline_paths(); // Baseline paths are system-injected, not user-specified. Skip paths // that do not exist in this container image to avoid noisy warnings from @@ -1676,14 +1548,13 @@ fn proto_sync_payload_for_enriched_policy( } /// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem -/// paths required by proxy-mode sandboxes and GPU runtimes. Used for the +/// paths required by proxy-mode sandboxes. Used for the /// local-file code path where no proto is available. fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) { - let (ro, rw) = - active_baseline_enrichment_paths(matches!(policy.network.mode, NetworkMode::Proxy)); - if ro.is_empty() && rw.is_empty() { + if !matches!(policy.network.mode, NetworkMode::Proxy) { return; } + let (ro, rw) = proxy_baseline_paths(); let mut modified = false; for path in &ro { @@ -1743,61 +1614,23 @@ mod baseline_tests { use std::path::PathBuf; #[test] - fn proc_not_in_both_read_only_and_read_write_when_gpu_present() { - // When GPU devices are present, /proc is promoted to read_write - // (CUDA needs to write /proc//task//comm). It should - // NOT also appear in read_only. - if !has_gpu_devices() { - // Can't test GPU dedup without GPU devices; skip silently. - return; - } - let (ro, rw) = baseline_enrichment_paths(); - assert!( - rw.contains(&"/proc".to_string()), - "/proc should be in read_write when GPU is present" - ); - assert!( - !ro.contains(&"/proc".to_string()), - "/proc should NOT be in read_only when it is already in read_write" - ); - } - - #[test] - fn proc_in_read_only_without_gpu() { - if has_gpu_devices() { - // On a GPU host we can't test the non-GPU path; skip silently. - return; - } - let (ro, _rw) = baseline_enrichment_paths(); - assert!( - ro.contains(&"/proc".to_string()), - "/proc should be in read_only when GPU is not present" - ); + fn proxy_baseline_keeps_proc_read_only_on_every_host() { + let (ro, rw) = proxy_baseline_paths(); + assert!(ro.contains(&"/proc".to_string())); + assert!(!rw.contains(&"/proc".to_string())); } #[test] fn baseline_read_write_does_not_hardcode_sandbox() { - let (_ro, rw) = baseline_enrichment_paths(); + let (_ro, rw) = proxy_baseline_paths(); assert!(rw.contains(&"/tmp".to_string())); assert!(rw.contains(&"/dev/null".to_string())); assert!(!rw.contains(&"/sandbox".to_string())); } - #[test] - fn enumerate_gpu_device_nodes_skips_bare_nvidia() { - // "nvidia" (without a trailing digit) is a valid /dev entry on some - // systems but is not a per-GPU device node. The enumerator must - // not match it. - let nodes = enumerate_gpu_device_nodes(); - assert!( - !nodes.contains(&"/dev/nvidia".to_string()), - "bare /dev/nvidia should not be enumerated: {nodes:?}" - ); - } - #[test] fn no_duplicate_paths_in_baseline() { - let (ro, rw) = baseline_enrichment_paths(); + let (ro, rw) = proxy_baseline_paths(); // No path should appear in both lists. for path in &ro { assert!( @@ -1811,7 +1644,7 @@ mod baseline_tests { fn proto_enrichment_preserves_explicit_read_only_for_baseline_read_write_paths() { let mut policy = openshell_policy::restrictive_default_policy(); policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { - read_only: vec!["/tmp".to_string()], + read_only: vec!["/tmp".to_string(), "/proc".to_string()], read_write: vec![], include_workdir: false, }); @@ -1831,6 +1664,8 @@ mod baseline_tests { enrich_proto_baseline_paths(&mut policy); let filesystem = policy.filesystem.expect("filesystem policy"); + assert!(filesystem.read_only.contains(&"/proc".to_string())); + assert!(!filesystem.read_write.contains(&"/proc".to_string())); assert!( filesystem.read_only.contains(&"/tmp".to_string()), "explicit read_only baseline path should be preserved" @@ -1925,47 +1760,26 @@ mod baseline_tests { } #[test] - fn proto_gpu_enrichment_promotes_proc_without_network_policy() { + fn no_network_policy_is_unchanged_by_supervisor_baseline() { + // A CPU-only workload must start with this policy even when the + // supervisor's host has GPUs. No GPU hardware is needed by this test. let mut policy = openshell_policy::restrictive_default_policy(); + assert!(policy.network_policies.is_empty()); assert!( - policy.network_policies.is_empty(), - "regression setup must exercise the no-network default path" + policy + .filesystem + .as_ref() + .unwrap() + .read_only + .contains(&"/proc".to_string()) ); - let (ro, rw) = - collect_baseline_enrichment_paths(false, true, vec!["/dev/nvidia0".to_string()]); + let original = policy.clone(); - let enriched = enrich_proto_baseline_paths_with(&mut policy, &ro, &rw, |path| { - matches!(path, "/proc" | "/dev/nvidia0") - }); + let enriched = enrich_proto_baseline_paths(&mut policy); - let filesystem = policy.filesystem.expect("filesystem policy"); - assert!( - enriched, - "GPU enrichment should not require network policies" - ); - assert!( - filesystem.read_write.contains(&"/dev/nvidia0".to_string()), - "GPU enrichment should add enumerated device nodes without network policies" - ); - assert!( - !filesystem.read_only.contains(&"/proc".to_string()), - "GPU enrichment should remove /proc from read_only" - ); - assert!( - filesystem.read_write.contains(&"/proc".to_string()), - "GPU enrichment should promote /proc to read_write" - ); - } - - #[test] - fn gpu_baseline_read_write_contains_dxg() { - // /dev/dxg must be present so WSL2 sandboxes get the Landlock - // read-write rule for the CDI-injected DXG device. The existence - // check in enrich_proto_baseline_paths() skips it on native Linux. - assert!( - GPU_BASELINE_READ_WRITE.contains(&"/dev/dxg"), - "/dev/dxg must be in GPU_BASELINE_READ_WRITE for WSL2 support" - ); + assert!(!enriched); + assert_eq!(policy, original); + assert!(proto_sync_payload_for_enriched_policy(&policy, enriched).is_none()); } #[test] @@ -1999,29 +1813,6 @@ mod baseline_tests { "baseline enrichment must not promote explicit read_only /tmp to read_write" ); } - - #[test] - fn gpu_baseline_read_only_contains_usr_lib_wsl() { - // /usr/lib/wsl must be present so CDI-injected WSL2 GPU library - // bind-mounts are accessible under Landlock. Skipped on native Linux. - assert!( - GPU_BASELINE_READ_ONLY.contains(&"/usr/lib/wsl"), - "/usr/lib/wsl must be in GPU_BASELINE_READ_ONLY for WSL2 CDI library paths" - ); - } - - #[test] - fn has_gpu_devices_reflects_dxg_or_nvidiactl() { - // Verify the OR logic: result must match the manual disjunction of - // the two path checks. Passes in all environments. - let nvidiactl = std::path::Path::new("/dev/nvidiactl").exists(); - let dxg = std::path::Path::new("/dev/dxg").exists(); - assert_eq!( - has_gpu_devices(), - nvidiactl || dxg, - "has_gpu_devices() should be true iff /dev/nvidiactl or /dev/dxg exists" - ); - } } /// Returns `true` if the error is transient and worth retrying. diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 5bca34af16..2eb116902d 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -380,6 +380,8 @@ Stopped VM state directories contain a marker that prevents startup from launching the VM. The driver retains `sandbox.pb`, `overlay.ext4`, and extension state, then removes the marker and restores the same overlay on start. +CPU-only VMs do not inherit GPU filesystem allowances from the host. A policy can keep `/proc` read-only on a GPU-equipped host. For GPU-assigned VMs, the sandbox discovers GPU paths inside the guest. + For maintainer-level implementation details, refer to the [VM driver README](https://github.com/NVIDIA/OpenShell/blob/main/crates/openshell-driver-vm/README.md). ### Enable the VM Driver diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index f33743e03f..16c298ddd1 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -884,6 +884,7 @@ credential failures. | `BatchSpanProcessor.ExportError` repeatedly reports connection refused on `127.0.0.1:4317` | The local gateway started with OTLP configured but the collector forwarding task later stopped, or the config was created manually | Restart `gateway:docker`, `gateway:podman`, or `gateway:vm` so it re-detects the listener; inspect the generated `gateway.toml` for `[openshell.gateway.otlp]` | | Gateway starts but sandbox create fails | Compute driver cannot reach runtime | Docker/Podman/Kubernetes/VM driver logs | | Docker or Podman sandbox never registers | Wrong gateway endpoint, unavailable host networking, or supervisor startup failure | Gateway logs and supervisor container logs | +| CPU-only VM startup reports that read-only `/proc` cannot be removed | Older supervisors can infer GPU requirements from host devices | Check gateway, VM driver, and bundled supervisor versions; use matching updated artifacts. CPU-only VMs do not require read-write `/proc`. | | Docker GPU sandbox fails before startup | NVIDIA CDI specs are missing or Docker has not discovered them | `docker info --format '{{json .DiscoveredDevices}}'`, `/etc/cdi`, `/var/run/cdi`, `nvidia-cdi-refresh.service` | | Kubernetes gateway pod pending | PVC unbound, taint, selector, or insufficient resources | `kubectl -n openshell describe pod ` | | Kubernetes sandbox pod stuck pending, workspace PVC unbound | Cluster has no default `StorageClass` and OpenShell does not set `storageClassName` on the workspace PVC (clusters with a default `StorageClass` bind fine without it) | `kubectl -n openshell describe pvc`; set `server.workspaceStorageClass` (gateway config `workspace_storage_class`) to a valid `StorageClass` |