From 8e70faeeffe62daa4d88ecb12be998d129474768 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Thu, 13 Aug 2026 21:00:17 -0700 Subject: [PATCH 1/2] feat(podman): honor OCI image working directories Signed-off-by: Matthew Grossman --- .../skills/debug-openshell-cluster/SKILL.md | 8 +- .agents/skills/openshell-cli/SKILL.md | 11 + architecture/compute-runtimes.md | 44 +- crates/openshell-core/src/container_paths.rs | 34 +- crates/openshell-core/src/driver_mounts.rs | 34 +- crates/openshell-driver-docker/README.md | 27 +- crates/openshell-driver-docker/src/tests.rs | 34 +- crates/openshell-driver-podman/README.md | 37 +- crates/openshell-driver-podman/src/client.rs | 13 +- .../openshell-driver-podman/src/container.rs | 156 +++++- crates/openshell-driver-podman/src/driver.rs | 11 +- crates/openshell-sandbox/src/lib.rs | 4 +- crates/openshell-sandbox/src/main.rs | 103 +--- .../src/bypass_monitor/mod.rs | 62 ++- .../src/process.rs | 515 +++--------------- .../openshell-supervisor-process/src/ssh.rs | 4 +- docs/reference/sandbox-compute-drivers.mdx | 53 +- e2e/rust/tests/custom_image.rs | 33 +- e2e/rust/tests/driver_config_volume.rs | 12 +- e2e/rust/tests/podman_oci_identity.rs | 49 +- 20 files changed, 574 insertions(+), 670 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index cc77d771b2..49472554c8 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -173,9 +173,9 @@ Common findings: - Gateway process stopped: inspect exit status and logs. - Sandbox image missing or pull denied: verify image reference and registry credentials. - Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. -- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must be an absolute normalized directory with no symlink components; the final policy UID, primary GID, and supplementary groups must pass the kernel's effective traverse/write checks, including POSIX ACL and LSM decisions. OpenShell does not create, chown, or chmod a non-default image workdir. -- Docker also rejects an image `VOLUME` that covers the workdir or one of its parents because the runtime would mask the immutable path before validation. Move the `VOLUME` below the workspace or remove the declaration. -- A workdir rejected as a special filesystem or OpenShell control-path collision cannot be made valid with permissions. Move the image workdir away from kernel-backed mounts and the concrete supervisor, TLS, token, runtime, and socket paths named in the error. +- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must already exist as an absolute normalized directory with no symlink components. OpenShell does not test identity permissions or create, chown, or chmod a non-default image workdir; diagnose later `chdir` or write failures from the image's final user and permissions. +- Docker rejects an image `VOLUME` that covers the workdir, one of its parents, or an OpenShell control path. Move the `VOLUME` below the workspace or remove the declaration. +- A workdir rejected as a forbidden workspace root or OpenShell control-path collision cannot be made valid with permissions. Move it away from the protected kernel-managed roots, the supervisor's executable/library roots, and the concrete supervisor, TLS, token, runtime, and socket paths named in the error. This is a mount-placement guardrail, not a general custom-image integrity check. - Docker driver cannot initialize because it cannot find `openshell-sandbox`: verify `OPENSHELL_DOCKER_SUPERVISOR_BIN`, the sibling binary next to `openshell-gateway`, or the configured supervisor image contains `/openshell-sandbox`. - Sandbox never registers: check gateway logs and supervisor callback endpoint. - Supervisor image exits before printing `openshell-sandbox --version`: the image should be the scratch supervisor image from `deploy/docker/Dockerfile.supervisor` and must contain a static executable at `/openshell-sandbox`. @@ -202,6 +202,8 @@ Common findings: - Rootless networking unavailable: inspect Podman network configuration. - Sandbox image missing or pull denied: verify image reference and registry credentials. - Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. +- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's OCI `WorkingDir`. Empty, `/`, and `/sandbox` use managed `/sandbox`; other paths must be normalized and, after Podman initializes the workspace volume, contain only existing directories with no symlinks or protected runtime/control/system roots. OpenShell does not test identity permissions or create, chown, or chmod a non-default workdir; diagnose later `chdir` or write failures from the image's final user and permissions. +- Podman mounts the persistent named workspace volume at OCI `WorkingDir` and validates after normal copy-up. Inspect the volume inside the failed container with `podman inspect` and `podman unshare` as appropriate. Ownership and SELinux behavior can differ between rootless and rootful deployments; fix the image or runtime configuration rather than expecting OpenShell to repair permissions. - Supervisor cannot call back: check callback endpoint and gateway logs. - Gateway exits before becoming healthy with a callback-listener discovery error: inspect `podman info --debug`, the configured Podman network, and the diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index 462e27f3f2..87eac6e700 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -464,6 +464,17 @@ field wins independently; omitted fields fall back to the image declaration. An image with no `USER` fails before readiness unless policy supplies both fields. +Docker and Podman custom images may declare an absolute OCI `WORKDIR`. Empty, +`/`, and `/sandbox` use the managed `/sandbox` compatibility workspace. For any +other path, the image author must make the final OCI/policy identity able to +traverse and write it. OpenShell validates the path's structure before +initialization but does not test identity permissions or create, chown, or chmod +it. An unusable image fails naturally when its workload changes directory or +writes. Podman mounts its persistent named volume at that path and performs +normal initial copy-up. Workdirs cannot overlap kernel-managed OCI mounts or the +supervisor's minimal executable and library roots; this protects workspace +mount placement, not custom-image integrity. + ### Forward ports ```bash diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 646b6320bd..f372e9f4ce 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -185,8 +185,8 @@ The gateway preserves whether each policy process field was omitted. The active driver then supplies one authoritative identity input to the supervisor: - Docker and Podman inspect the final sandbox image, pin container creation to - its immutable image ID, and pass its raw OCI `Config.User`. Docker also - resolves the workspace from OCI `Config.WorkingDir` during that inspection. + its immutable image ID, and pass its raw OCI `Config.User`. Both resolve the + workspace from OCI `Config.WorkingDir` during that inspection. - Kubernetes passes its platform-resolved numeric UID/GID, including OpenShift SCC-derived values. - VM keeps its existing guest identity behavior. @@ -199,23 +199,33 @@ and uses the same privilege-drop path for direct and SSH children. When a declaration omits the group, the supervisor fills it with the user's numeric primary GID. It does not rewrite the account files. -Docker uses an absolute OCI working directory as the workspace. An +Docker and Podman use an absolute OCI working directory as the workspace. An empty, root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which -OpenShell creates and owns as a compatibility workspace. Any other workdir must already -exist in the immutable image without symlink components. The completed -identity, including supplementary groups, must already be able to traverse -every parent and write and enter the workdir; OpenShell does not change that -directory's ownership or mode. A one-shot validator drops to that identity and -uses kernel effective-access checks so POSIX ACL and LSM decisions are honored. -Path checks reserve the standard OCI runtime namespaces under `/proc`, `/sys`, -and `/dev`, while separate collision checks are derived from actual OpenShell -control paths. -Docker performs the check in the final container before workload launch and -rejects image `VOLUME` declarations that would mask the workdir ancestry. The -resolved workspace is the child cwd and `HOME`; when +OpenShell creates and owns as a compatibility workspace. OpenShell does not +create, chown, or chmod any other workdir. Image authors are responsible for +making the final OCI/policy identity able to traverse and write that workdir. +An unusable image fails naturally when its workload changes directory or writes. + +Before policy, credential, TLS, or networking initialization, the final +supervisor performs a no-follow structural walk of every non-default path. It +rejects missing components, symlinks, non-directories, and OpenShell control +paths. The drivers also reject overlap in +either direction with `/proc`, `/sys`, `/dev`, `/bin`, `/sbin`, `/usr/bin`, +`/usr/sbin`, `/lib`, `/lib64`, `/usr/lib`, or `/usr/lib64`. The first three are +kernel-managed OCI mounts; the others protect executable and library roots used +by the supervisor. This is a mount-placement guardrail, not an image-integrity +or permission guarantee. Explicit driver-config mounts may not cover the +resolved OCI `WORKDIR` or an OpenShell control path. Docker rejects +image-declared `VOLUME` entries that cover the workdir or control paths. +Podman leaves image-declared volume behavior to the runtime. + +Docker checks the image directory directly. Podman mounts the persistent named +workspace volume at the resolved workdir and validates it after Podman's normal +initial copy-up. OpenShell does not repair ownership or permissions after that +copy-up. The resolved workspace is the child cwd and `HOME`; when `filesystem.include_workdir` is enabled, it becomes the automatic writable -policy path. Podman, Kubernetes/OpenShift, and VM retain their existing -`/sandbox` workspace behavior. +policy path. Kubernetes/OpenShift and VM retain their existing `/sandbox` +workspace behavior. Sandbox creation fails before the workload becomes ready when a required image identity is absent, malformed, unknown, ambiguous, or resolves to UID/GID 0. diff --git a/crates/openshell-core/src/container_paths.rs b/crates/openshell-core/src/container_paths.rs index c63e4bcdd8..6e41c30603 100644 --- a/crates/openshell-core/src/container_paths.rs +++ b/crates/openshell-core/src/container_paths.rs @@ -16,14 +16,30 @@ pub const SIDECAR_RUN_ROOT: &str = "/run/openshell-sidecar"; pub const NETNS_MOUNT_ROOT: &str = "/run/netns"; pub const NETNS_IPROUTE2_ROOT: &str = "/var/run/netns"; -/// Standard Linux container namespaces that an image-selected workspace must -/// not contain or enter. +/// Container roots that an image-selected workspace must not overlap. /// -/// These roots cover the default filesystems and devices defined by the OCI -/// Runtime Specification: procfs, sysfs, cgroups, device nodes, devpts, shared -/// memory, and POSIX message queues. -/// -pub const OCI_RUNTIME_MOUNT_ROOTS: &[&str] = &["/proc", "/sys", "/dev"]; +/// The first group contains kernel-managed mounts from the OCI runtime. The +/// second protects executable and library roots used by the in-container +/// supervisor. This is a narrow mount-placement guardrail, not an image +/// integrity check; application paths such as `/usr/src/app` remain valid. +/// +/// TODO: Make the supervisor's helper functionality self-contained so it no +/// longer depends on executable and library paths shared with the image. +pub const FORBIDDEN_WORKSPACE_ROOTS: &[&str] = &[ + // Kernel-managed OCI runtime mounts. + "/proc", + "/sys", + "/dev", + // Executable and library roots needed by the supervisor. + "/bin", + "/sbin", + "/lib", + "/lib64", + "/usr/bin", + "/usr/sbin", + "/usr/lib", + "/usr/lib64", +]; /// High-level namespaces mounted or created by `OpenShell` inside sandboxes. /// @@ -118,7 +134,7 @@ mod tests { } #[test] - fn runtime_roots_cover_standard_oci_mount_destinations() { + fn forbidden_workspace_roots_cover_standard_oci_mount_destinations() { for path in [ "/proc", "/dev", @@ -129,7 +145,7 @@ mod tests { "/sys/fs/cgroup", ] { assert!( - OCI_RUNTIME_MOUNT_ROOTS + FORBIDDEN_WORKSPACE_ROOTS .iter() .any(|root| Path::new(path).starts_with(root)), "OCI runtime mount {path} is outside the reserved roots" diff --git a/crates/openshell-core/src/driver_mounts.rs b/crates/openshell-core/src/driver_mounts.rs index b1a3049882..0a26c580aa 100644 --- a/crates/openshell-core/src/driver_mounts.rs +++ b/crates/openshell-core/src/driver_mounts.rs @@ -5,7 +5,7 @@ use std::path::Path; -use crate::container_paths::{CONTROL_ROOTS, OCI_RUNTIME_MOUNT_ROOTS}; +use crate::container_paths::{CONTROL_ROOTS, FORBIDDEN_WORKSPACE_ROOTS}; /// `SELinux` relabelling mode for bind mounts. /// @@ -110,13 +110,16 @@ pub fn resolve_oci_workspace_root(working_dir: &str) -> Result { return Ok(DEFAULT_WORKSPACE_ROOT.to_string()); } let workspace_root = normalize_absolute_container_path(working_dir, "OCI WorkingDir")?; - for runtime_path in OCI_RUNTIME_MOUNT_ROOTS { - validate_workspace_reserved_path(&workspace_root, runtime_path, "OCI runtime mount")?; + for forbidden_root in FORBIDDEN_WORKSPACE_ROOTS { + validate_workspace_reserved_path( + &workspace_root, + forbidden_root, + "forbidden workspace root", + )?; } for control_path in CONTROL_ROOTS { validate_workspace_control_path(&workspace_root, control_path)?; } - Ok(workspace_root) } @@ -278,7 +281,7 @@ mod tests { } #[test] - fn oci_workspace_root_rejects_runtime_and_openshell_control_path_collisions() { + fn oci_workspace_root_rejects_forbidden_and_openshell_control_path_collisions() { for invalid in [ "/proc", "/proc/self", @@ -299,6 +302,17 @@ mod tests { "/run/openshell-sidecar/control.sock", "/run/netns/project", "/var/run/netns/project", + "/bin", + "/bin/project", + "/sbin", + "/lib", + "/lib/project", + "/lib64", + "/usr", + "/usr/bin", + "/usr/bin/project", + "/usr/lib", + "/usr/lib64", ] { assert!( resolve_oci_workspace_root(invalid).is_err(), @@ -310,9 +324,17 @@ mod tests { "/app", "/etc/project", "/home/app", + "/lib32/app", + "/libx32/app", "/opt/app", - "/usr/bin/project", + "/usr/lib32/app", + "/usr/libexec/app", + "/usr/libx32/app", "/usr/src/app", + "/usr/local", + "/usr/local/app", + "/usr/local/bin", + "/usr/local/lib", "/var/lib/app", "/var/app/current", "/var/task", diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 05faf53c5c..cac75562b5 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -31,26 +31,23 @@ numeric primary GID. Explicit `process.run_as_user` and An absolute OCI working directory becomes the agent workspace. An empty, root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell creates when necessary and owns as a compatibility workspace. Any other image -workdir must already exist without symlink components. The completed identity, -including supplementary groups, must already be able to traverse every parent -and write and enter the workdir. OpenShell does not change its ownership or -mode. - -OpenShell deliberately asks the Linux kernel to make this access decision -under the completed sandbox identity instead of reproducing permission rules -from ownership and mode bits. Mode-bit inspection alone can reject authority -granted by a POSIX ACL or overlook a denial imposed by a Linux Security Module -such as SELinux or AppArmor. OpenShell does not configure or otherwise manage -ACLs or LSM policy here; the one-shot validator only observes the kernel's -effective decision. This keeps the no-authority-expansion invariant aligned -with the access the eventual workload will receive without adding a separate, -incomplete permission model to OpenShell. +workdir must already exist without symlink components. OpenShell does not +create it or change its ownership or mode. Image authors are responsible for +making the final OCI/policy identity able to traverse and write it; an unusable +image fails naturally when its workload changes directory or writes. + +Before loading policy, credentials, TLS, or networking state, the supervisor +performs a no-follow structural walk. It rejects missing components, symlinks, +non-directories, and OpenShell control paths. This check protects workspace +mount placement; it does not validate custom-image +integrity or workdir permissions. Image `VOLUME` declarations must not cover the workdir or one of its parents because Docker would mount the volume before the supervisor could validate the immutable image path. Workdirs under the standard OCI runtime namespaces `/proc`, `/sys`, and `/dev` -are rejected, as are paths that overlap concrete OpenShell control resources. +are rejected, as are paths that overlap protected executable and library roots +or concrete OpenShell control resources. The workspace is the child cwd and `HOME`. The supervisor starts from `/`, then reports an invalid workdir as a readiness failure. diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index ac525c705c..15a84247ef 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -667,6 +667,38 @@ fn container_creation_rejects_openshell_control_path_working_dir() { assert!(err.message().contains("OpenShell control path")); } +#[test] +fn container_creation_protects_forbidden_roots_but_allows_usr_application_paths() { + let rejected = DockerImageMetadata { + id: "sha256:immutable".to_string(), + user: "1234:1235".to_string(), + working_dir: "/usr".to_string(), + volumes: Vec::new(), + }; + let error = build_container_create_body_for_image( + &test_sandbox(), + &runtime_config(), + &DockerSandboxDriverConfig::default(), + None, + &rejected, + ) + .unwrap_err(); + assert!(error.message().contains("forbidden workspace root")); + + let allowed = DockerImageMetadata { + working_dir: "/usr/src/app".to_string(), + ..rejected + }; + build_container_create_body_for_image( + &test_sandbox(), + &runtime_config(), + &DockerSandboxDriverConfig::default(), + None, + &allowed, + ) + .expect("application workdirs below /usr remain valid"); +} + #[test] fn container_creation_rejects_image_volume_that_masks_working_dir() { let sandbox = test_sandbox(); @@ -1460,7 +1492,7 @@ fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() { ); assert_eq!( create_body.cmd, - Some(vec!["--workdir".to_string(), "/sandbox".to_string()]) + Some(vec!["--workdir".to_string(), "/sandbox".to_string(),]) ); assert_eq!( create_body diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 965a295d19..2678ffdc00 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -88,7 +88,7 @@ read-only by default; set `read_only: false` to make them writable. Podman image and volume mounts do not support `subpath` in OpenShell driver config. Mount `source` and `target` values must not contain surrounding whitespace. Mount targets must be absolute container paths and must not replace -the workspace root (`/sandbox`) or overlap OpenShell supervisor files, +the resolved workspace root or overlap OpenShell supervisor files, `/etc/openshell`, `/etc/openshell-tls`, or `/run/netns`. Example named-volume usage: @@ -120,6 +120,35 @@ exec. It drops unneeded defaults such as `DAC_OVERRIDE`, `FSETID`, `KILL`, `NET_BIND_SERVICE`, `NET_RAW`, `SETFCAP`, and `SYS_CHROOT`. +## OCI Working Directory + +The driver inspects and pins the sandbox image, then resolves its OCI +`Config.WorkingDir`. An empty value, `/`, or `/sandbox` uses the managed +`/sandbox` compatibility workspace. Otherwise, the persistent Podman named +volume is mounted at the normalized absolute workdir and Podman performs its +normal initial copy-up. + +Podman applies its normal runtime semantics for image-declared `VOLUME` +metadata. OpenShell validates the resulting visible workdir. + +The final supervisor rejects missing or symlink components, non-directories, +and OpenShell control paths. The driver also +rejects a workdir that overlaps `/proc`, `/sys`, `/dev`, or the supervisor's +minimal executable and library roots in either direction. This prevents +OpenShell from placing its persistent workspace over those paths; it does not +attempt to establish the integrity of a custom image or validate workdir +permissions. The resolved path becomes both cwd and `HOME` for direct and SSH +children. + +OpenShell does not create, chown, or chmod a non-default workdir. Podman may +initialize or adjust a named-volume mountpoint as part of its own volume +semantics, but OpenShell does not repair the result. Image authors must make +the declared `USER` able to use the declared `WORKDIR`; unusable images fail +naturally when the workload changes directory or writes. Rootless, rootful, +user-namespace, and SELinux configurations can differ in volume initialization +behavior, so validate custom images in the deployment's actual Podman +configuration. + ## Supervisor Sideloading The supervisor binary is delivered to sandbox containers via Podman's OCI image @@ -291,11 +320,13 @@ sequenceDiagram D->>P: pull_image(supervisor, "missing") D->>P: pull_image(sandbox_image, policy) + D->>P: inspect_image(sandbox_image) + Note over D: Pin image ID and resolve OCI USER + WORKDIR D->>P: create_volume(workspace) Note over D: On failure below, rollback volume - D->>P: create_container(spec) + D->>P: create_container(spec with volume at WORKDIR) alt Conflict (409) D->>P: remove_volume D-->>GW: AlreadyExists @@ -303,6 +334,8 @@ sequenceDiagram Note over D: On failure below, rollback container + volume D->>P: start_container + Note over P: Podman performs named-volume copy-up + Note over P: Supervisor validates the copied-up workspace structure D-->>GW: Ok ``` diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 9fe39cf7e2..246e1cde9c 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -177,6 +177,8 @@ pub struct ImageInspect { pub struct ImageConfig { #[serde(default)] pub user: String, + #[serde(default)] + pub working_dir: String, } /// A container summary returned by the list API. @@ -973,12 +975,12 @@ mod tests { } #[tokio::test] - async fn inspect_image_reads_immutable_id_and_oci_user() { + async fn inspect_image_reads_required_oci_metadata() { let (socket_path, request_log, handle) = spawn_podman_stub( "inspect-image", vec![StubResponse::new( StatusCode::OK, - r#"{"Id":"sha256:immutable","Config":{"User":"app:staff"}}"#, + r#"{"Id":"sha256:immutable","Config":{"User":"app:staff","WorkingDir":"/workspace/project"}}"#, )], ); let client = PodmanClient::new(socket_path.clone()); @@ -993,6 +995,13 @@ mod tests { image.config.as_ref().map(|config| config.user.as_str()), Some("app:staff") ); + assert_eq!( + image + .config + .as_ref() + .map(|config| config.working_dir.as_str()), + Some("/workspace/project") + ); handle.await.expect("stub task should finish"); assert_eq!( request_log diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 005f688a19..15b7cc7aea 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -3,6 +3,7 @@ //! Container spec construction for the Podman driver. +use crate::client::ImageInspect; use crate::config::PodmanComputeConfig; use openshell_core::ComputeDriverError; use openshell_core::driver_mounts::SelinuxLabel; @@ -177,6 +178,40 @@ pub fn short_id(id: &str) -> String { id.chars().take(12).collect() } +/// Immutable OCI image metadata normalized once for final launch. +#[derive(Debug, Clone)] +pub struct ResolvedPodmanImage { + id: String, + oci_user: String, + workspace_root: String, +} + +impl ResolvedPodmanImage { + pub fn from_inspect( + inspected: &ImageInspect, + config: &PodmanComputeConfig, + ) -> Result { + let image_config = inspected.config.as_ref(); + let workspace_root = driver_mounts::resolve_oci_workspace_root( + image_config.map_or("", |config| config.working_dir.as_str()), + ) + .map_err(ComputeDriverError::Precondition)?; + driver_mounts::validate_workspace_control_path( + &workspace_root, + &config.sandbox_ssh_socket_path, + ) + .map_err(ComputeDriverError::Precondition)?; + + Ok(Self { + id: inspected.id.clone(), + oci_user: image_config + .map_or("", |config| config.user.as_str()) + .to_string(), + workspace_root, + }) + } +} + // --------------------------------------------------------------------------- // Typed container spec structs for the Podman libpod create API. // --------------------------------------------------------------------------- @@ -190,6 +225,8 @@ struct ContainerSpec { volumes: Vec, image_volumes: Vec, hostname: String, + /// Start the supervisor independently of the image-selected workspace. + work_dir: String, /// Overrides the image's ENTRYPOINT. In Podman's libpod API, `command` /// only overrides CMD (appended as args to the entrypoint). We must set /// `entrypoint` explicitly so the supervisor binary runs directly, @@ -612,6 +649,8 @@ pub fn podman_driver_image_mount_sources( fn podman_user_mounts( sandbox: &DriverSandbox, enable_bind_mounts: bool, + workspace_root: &str, + control_path: &str, ) -> Result { let template = sandbox .spec @@ -623,6 +662,14 @@ fn podman_user_mounts( let config = podman_driver_config(template, enable_bind_mounts)?; let mut result = PodmanUserMounts::default(); for mount in config.mounts { + let target = match &mount { + PodmanDriverMountConfig::Bind { target, .. } + | PodmanDriverMountConfig::Volume { target, .. } + | PodmanDriverMountConfig::Tmpfs { target, .. } + | PodmanDriverMountConfig::Image { target, .. } => target, + }; + driver_mounts::validate_workspace_mount_target(target, workspace_root)?; + driver_mounts::validate_mount_control_path(target, control_path)?; match mount { PodmanDriverMountConfig::Bind { source, @@ -897,14 +944,20 @@ pub fn build_container_spec_with_token_and_gpu_devices( gpu_device_ids: Option<&[String]>, ) -> Result { let image = resolve_image(sandbox, config); + let resolved_image = ResolvedPodmanImage::from_inspect( + &ImageInspect { + id: image.to_string(), + config: None, + }, + config, + )?; build_container_spec_for_image( sandbox, config, token_secret_name, gpu_device_ids, image, - image, - "", + &resolved_image, ) } @@ -914,17 +967,21 @@ pub fn build_container_spec_for_image( token_secret_name: Option<&str>, gpu_device_ids: Option<&[String]>, requested_image: &str, - image_id: &str, - oci_user: &str, + image: &ResolvedPodmanImage, ) -> Result { let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); - let env = build_env(sandbox, config, requested_image, oci_user); + let env = build_env(sandbox, config, requested_image, &image.oci_user); let labels = build_labels(sandbox); let resource_limits = build_resource_limits(sandbox, config); - let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts) - .map_err(ComputeDriverError::InvalidArgument)?; + let user_mounts = podman_user_mounts( + sandbox, + config.enable_bind_mounts, + &image.workspace_root, + &config.sandbox_ssh_socket_path, + ) + .map_err(ComputeDriverError::InvalidArgument)?; if sandbox .spec .as_ref() @@ -952,7 +1009,7 @@ pub fn build_container_spec_for_image( let mut volumes = vec![NamedVolume { name: vol, - dest: "/sandbox".into(), + dest: image.workspace_root.clone(), options: vec!["rw".into()], }]; volumes.extend(user_mounts.volumes); @@ -963,15 +1020,12 @@ pub fn build_container_spec_for_image( rw: false, }]; image_volumes.extend(user_mounts.image_volumes); - let mut command = vec![ - "--workdir".to_string(), - driver_mounts::DEFAULT_WORKSPACE_ROOT.to_string(), - ]; + let mut command = vec!["--workdir".to_string(), image.workspace_root.clone()]; command.extend(upstream_proxy_cli_args(config)); let container_spec = ContainerSpec { name, - image: image_id.to_string(), + image: image.id.clone(), labels, env, volumes, @@ -982,6 +1036,7 @@ pub fn build_container_spec_for_image( // /openshell-sandbox, so it appears at /opt/openshell/bin/openshell-sandbox. image_volumes, hostname: format!("sandbox-{}", sandbox.name), + work_dir: "/".to_string(), // Override the image's ENTRYPOINT so the supervisor binary runs // directly. Sandbox images (e.g. the community base image) set // ENTRYPOINT ["/bin/bash"], and Podman's `command` field only @@ -989,8 +1044,7 @@ pub fn build_container_spec_for_image( // Without this, the container would run the entrypoint binary with // the supervisor path as an argument instead of executing it directly. entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], - // Keep Podman's existing /sandbox workspace contract explicit while - // the supervisor supports driver-selected workdirs. Operator-owned + // Pass the resolved image workspace explicitly. Operator-owned // corporate proxy flags follow it; the workload command comes from // the reserved environment variable. command, @@ -1260,6 +1314,7 @@ fn parse_memory_to_bytes(quantity: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::client::ImageConfig; use openshell_core::proto::compute::v1::{GpuResourceRequirements, ResourceRequirements}; static ENV_LOCK: std::sync::LazyLock> = @@ -1279,6 +1334,21 @@ mod tests { } } + fn image_inspect(id: &str, user: &str, working_dir: &str) -> ImageInspect { + ImageInspect { + id: id.to_string(), + config: Some(ImageConfig { + user: user.to_string(), + working_dir: working_dir.to_string(), + }), + } + } + + fn resolved_image(id: &str, user: &str, working_dir: &str) -> ResolvedPodmanImage { + ResolvedPodmanImage::from_inspect(&image_inspect(id, user, working_dir), &test_config()) + .unwrap() + } + #[test] fn parse_cpu_millicore() { assert_eq!(parse_cpu_to_microseconds("500m"), Some(50_000)); @@ -1383,8 +1453,7 @@ mod tests { None, None, "registry.example/app:latest", - "sha256:immutable", - "app:staff", + &resolved_image("sha256:immutable", "app:staff", "/workspace/project"), ) .unwrap(); @@ -1395,6 +1464,18 @@ mod tests { ); assert_eq!(container["user"].as_str(), Some("0:0")); assert_eq!(container["image_pull_policy"].as_str(), Some("never")); + assert_eq!( + container["command"], + serde_json::json!(["--workdir", "/workspace/project"]) + ); + assert_eq!(container["work_dir"].as_str(), Some("/")); + assert!(container["volumes"].as_array().is_some_and(|volumes| { + volumes.iter().any(|volume| { + volume["name"].as_str() == Some("openshell-sandbox-test-id-workspace") + && volume["dest"].as_str() == Some("/workspace/project") + && volume["options"] == serde_json::json!(["rw"]) + }) + })); assert_eq!( container["env"][openshell_core::sandbox_env::OCI_IMAGE_USER].as_str(), Some("app:staff") @@ -1407,10 +1488,43 @@ mod tests { container["env"][openshell_core::sandbox_env::SANDBOX_GID].as_str(), Some("") ); - assert_eq!( - container["command"], - serde_json::json!(["--workdir", "/sandbox"]) - ); + } + + #[test] + fn container_spec_rejects_invalid_or_protected_oci_working_dir() { + for working_dir in [ + "relative/workspace", + "/usr", + "/usr/bin", + "/usr/bin/project", + "/opt/openshell", + ] { + let error = ResolvedPodmanImage::from_inspect( + &image_inspect("sha256:immutable", "app:staff", working_dir), + &test_config(), + ) + .unwrap_err(); + assert!( + matches!(error, ComputeDriverError::Precondition(_)), + "unexpected error for {working_dir}: {error}" + ); + } + } + + #[test] + fn container_spec_allows_application_path_below_usr() { + let image = resolved_image("sha256:immutable", "app:staff", "/usr/src/app"); + let container = build_container_spec_for_image( + &test_sandbox("test-id", "test-name"), + &test_config(), + None, + None, + "registry.example/app:latest", + &image, + ) + .unwrap(); + + assert_eq!(container["volumes"][0]["dest"], "/usr/src/app"); } #[test] diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 51c689fb29..7f1eb6a7a6 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -692,10 +692,8 @@ impl PodmanComputeDriver { "podman image '{image}' inspection did not return an immutable image ID" ))); } - let image_user = inspected_image - .config - .as_ref() - .map_or("", |config| config.user.as_str()); + let resolved_image = + container::ResolvedPodmanImage::from_inspect(&inspected_image, &self.config)?; for image in container::podman_driver_image_mount_sources(sandbox, self.config.enable_bind_mounts) @@ -761,8 +759,7 @@ impl PodmanComputeDriver { token_secret_name.as_deref(), gpu_devices.as_deref(), image, - &inspected_image.id, - image_user, + &resolved_image, ) { Ok(spec) => spec, Err(e) => { @@ -786,7 +783,7 @@ impl PodmanComputeDriver { } } - // 5. Start container. + // 4. Start container. if let Err(e) = self.client.start_container(&name).await { warn!( sandbox_name = %sandbox.name, diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 0d26067f6b..9e2783213f 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -189,8 +189,8 @@ pub async fn run_sandbox( // Normalize the active driver's identity contract once, while both the // policy and launched image filesystem are available. Kubernetes and - // OpenShift retain their authoritative numeric pair; Docker fills only - // omitted policy fields from OCI Config.User. + // OpenShift retain their authoritative numeric pair; Docker and Podman + // fill only omitted policy fields from OCI Config.User. #[cfg(unix)] let (resolved_process_identity, workspace) = { let driver_identity = openshell_supervisor_process::identity::DriverIdentity::from_env()?; diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 98af7f9ea9..be6fc28597 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -32,7 +32,6 @@ const COPY_SELF_SUBCOMMAND: &str = "copy-self"; /// run `openshell-sandbox debug-rpc get-sandbox-config --sandbox-id ` /// to confirm the cross-sandbox IDOR guard fires. const DEBUG_RPC_SUBCOMMAND: &str = "debug-rpc"; -const VALIDATE_WORKSPACE_SUBCOMMAND: &str = "validate-workspace"; /// Default `--mode` value: run both supervisor leaves in a single binary. const DEFAULT_MODE: &str = "network,process"; @@ -232,50 +231,6 @@ struct Args { upstream_proxy_connect_by_hostname: bool, } -/// Internal one-shot command used by the privileged supervisor to validate an -/// image-provided workdir as the final sandbox identity. -#[derive(Parser, Debug)] -#[command(name = "validate-workspace", hide = true)] -struct ValidateWorkspaceArgs { - #[arg(long)] - workdir: String, - #[arg(long)] - expected_uid: u32, - #[arg(long)] - expected_gid: u32, -} - -#[cfg(target_os = "linux")] -fn validate_workspace(args: &[String]) -> Result<()> { - let args = ValidateWorkspaceArgs::try_parse_from( - std::iter::once(VALIDATE_WORKSPACE_SUBCOMMAND.to_string()).chain(args.iter().cloned()), - ) - .into_diagnostic()?; - let actual = ( - nix::unistd::geteuid().as_raw(), - nix::unistd::getegid().as_raw(), - ); - if actual != (args.expected_uid, args.expected_gid) { - return Err(miette::miette!( - "workspace validator privilege drop failed: expected {}:{}, got {}:{}", - args.expected_uid, - args.expected_gid, - actual.0, - actual.1 - )); - } - openshell_supervisor_process::process::validate_oci_workspace_as_effective_identity(Path::new( - &args.workdir, - )) -} - -#[cfg(not(target_os = "linux"))] -fn validate_workspace(_args: &[String]) -> Result<()> { - Err(miette::miette!( - "workspace validation is only supported on Unix" - )) -} - /// Copy the running executable to `dest`, creating parent directories as /// needed and ensuring the result is executable (mode `0755`). /// @@ -316,6 +271,17 @@ fn copy_self(dest: &str) -> Result<()> { Ok(()) } +fn validate_workspace_structure(workdir: Option<&str>) -> Result<()> { + let Some(workdir) = workdir else { + return Ok(()); + }; + if workdir == openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT { + return Ok(()); + } + openshell_supervisor_process::process::validate_oci_workspace_structure(Path::new(workdir))?; + Ok(()) +} + #[cfg(target_os = "linux")] fn prepare_sidecar_directory(path: &Path, uid: u32, gid: u32, mode: u32) -> Result<()> { use miette::Context as _; @@ -524,10 +490,6 @@ fn main() -> Result<()> { std::process::exit(exit); }); } - if raw_args.get(1).map(String::as_str) == Some(VALIDATE_WORKSPACE_SUBCOMMAND) { - return validate_workspace(&raw_args[2..]); - } - let args = Args::parse(); if args.mode.network_init { @@ -540,6 +502,10 @@ fn main() -> Result<()> { ); } + // Validate any non-default workspace independently of OCI/policy identity + // so explicit run_as fields cannot bypass the structural check. + validate_workspace_structure(args.workdir.as_deref())?; + // Try to open a rolling log file; fall back to stderr-only logging if it fails // (e.g., /var/log is not writable in custom workload images). // Rotates daily, keeps the 3 most recent files to bound disk usage. @@ -696,31 +662,6 @@ mod tests { use super::*; use std::os::unix::fs::PermissionsExt; - #[cfg(target_os = "linux")] - #[test] - fn workspace_validation_subcommand_uses_final_policy_identity() { - let uid = nix::unistd::geteuid().as_raw(); - let gid = nix::unistd::getegid().as_raw(); - if uid < 1000 || gid < 1000 { - return; - } - let dir = tempfile::tempdir_in("/tmp").unwrap(); - std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o711)).unwrap(); - let root = dir.path().canonicalize().unwrap().join("workspace"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).unwrap(); - let args = vec![ - "--workdir".to_string(), - root.display().to_string(), - "--expected-uid".to_string(), - uid.to_string(), - "--expected-gid".to_string(), - gid.to_string(), - ]; - - validate_workspace(&args).expect("current identity should retain workspace authority"); - } - /// Drives `copy_self`'s file-copy logic against an arbitrary source path /// so tests don't depend on `current_exe()`. fn copy_executable(src: &Path, dest: &Path) -> Result<()> { @@ -759,6 +700,20 @@ mod tests { assert_eq!(copied, b"#!/bin/false\n"); } + #[test] + fn workspace_structure_validation_skips_absent_and_compatibility_workdirs() { + validate_workspace_structure(None).expect("standalone invocations need no workdir"); + validate_workspace_structure(Some(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT)) + .expect("the managed compatibility workspace needs no structural walk"); + + let directory = tempfile::tempdir().unwrap(); + let directory = directory.path().canonicalize().unwrap(); + validate_workspace_structure(directory.to_str()) + .expect("a non-default existing directory should be checked"); + let missing = directory.join("missing"); + assert!(validate_workspace_structure(missing.to_str()).is_err()); + } + #[test] fn copy_self_into_existing_directory_uses_source_filename() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs b/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs index 44847b0d13..afb11fccad 100644 --- a/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs +++ b/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Bypass detection monitor — reads kernel log messages from `/dev/kmsg` to +//! Bypass detection monitor — follows kernel log messages with `dmesg` to //! detect and report direct connection attempts that bypass the HTTP CONNECT //! proxy. //! @@ -12,9 +12,9 @@ //! //! ## Graceful degradation //! -//! If `/dev/kmsg` cannot be opened (e.g., restricted container environment), -//! the monitor logs a one-time warning and returns. The nftables reject rules -//! still provide fast-fail UX — the monitor only adds diagnostic visibility. +//! If an approved `dmesg` binary is unavailable, the monitor logs a one-time +//! warning and returns. The nftables reject rules still provide fast-fail UX — +//! the monitor only adds diagnostic visibility. mod procfs; @@ -29,7 +29,21 @@ use std::sync::atomic::{AtomicU32, Ordering}; use tokio::sync::mpsc; use tracing::debug; -/// A parsed nftables log entry from `/dev/kmsg`. +const DMESG_PATHS: &[&str] = &[ + "/usr/bin/dmesg", + "/bin/dmesg", + "/usr/sbin/dmesg", + "/sbin/dmesg", +]; + +fn find_dmesg_binary<'a>(candidates: &'a [&'a str]) -> Option<&'a str> { + candidates.iter().copied().find(|candidate| { + let path = std::path::Path::new(candidate); + path.is_absolute() && path.is_file() + }) +} + +/// A parsed nftables kernel log entry. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BypassEvent { /// Destination IP address. @@ -181,8 +195,23 @@ pub fn spawn( use std::io::BufRead; use std::process::{Command, Stdio}; - // Verify dmesg is available before spawning the monitor. - let dmesg_check = Command::new("dmesg") + // Use only known system locations. The sandbox image's PATH is controlled + // by the image author and may include the writable workspace. + let Some(dmesg_path) = find_dmesg_binary(DMESG_PATHS) else { + let event = NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx()) + .activity(ActivityId::Other) + .severity(SeverityId::Low) + .message( + "dmesg not available at an approved system path; bypass detection monitor will not run. \ + Bypass REJECT rules still provide fast-fail behavior.", + ) + .build(); + ocsf_emit!(event); + return None; + }; + + // Verify dmesg works before spawning the monitor. + let dmesg_check = Command::new(dmesg_path) .arg("--version") .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -209,7 +238,7 @@ pub fn spawn( let handle = tokio::task::spawn_blocking(move || { // Start dmesg in follow mode to tail new kernel messages. - let mut child = match Command::new("dmesg") + let mut child = match Command::new(dmesg_path) .args(["--follow", "--notime"]) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -382,6 +411,23 @@ fn resolve_process_identity(entrypoint_pid: u32, src_port: u16) -> (String, Stri mod tests { use super::*; + #[test] + fn dmesg_lookup_uses_only_existing_absolute_candidates() { + let dir = tempfile::tempdir().unwrap(); + let binary = dir.path().join("dmesg"); + std::fs::write(&binary, "test").unwrap(); + let binary = binary.to_str().unwrap(); + + assert_eq!( + find_dmesg_binary(&["relative/dmesg", "/definitely/missing/dmesg", binary]), + Some(binary) + ); + assert_eq!( + find_dmesg_binary(&["relative/dmesg", "/definitely/missing/dmesg"]), + None + ); + } + #[test] fn parse_kmsg_line_tcp_bypass() { let line = "6,1234,5678,-;openshell:bypass:sandbox-abcd1234:IN= OUT=veth-s-abcd1234 \ diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 659fe3dc06..b90bf7c877 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -1299,172 +1299,19 @@ fn prepare_oci_workspace( prepare_oci_workspace_with(root, uid, gid, supplementary_gids, &nix::unistd::chown) } -/// Validate that selecting an image-provided OCI workdir does not grant the -/// sandbox identity any filesystem authority it lacked in the immutable image. -/// -/// Every path component must be a real directory (never a symlink), every -/// parent must already be traversable, and the final directory must already be -/// writable and traversable. No ownership or mode bits are changed. -#[cfg(unix)] -pub fn validate_oci_workspace( - root: &Path, - uid: Option, - gid: Option, - supplementary_gids: &[Gid], -) -> Result<()> { - let components = validated_workspace_components(root, false)?; - let mut current = PathBuf::from("/"); - validate_workspace_component(¤t, uid, gid, supplementary_gids, false)?; - let last_component = components.len().saturating_sub(1); - for (index, component) in components.into_iter().enumerate() { - current.push(component); - validate_workspace_component( - ¤t, - uid, - gid, - supplementary_gids, - index == last_component, - )?; - } - Ok(()) -} - -/// Validate an image-provided workdir in a clean copy of the supervisor so the -/// main process retains the root authority needed for subsequent setup. +/// Validate an OCI workspace's visible structure without testing identity +/// permissions. Local container supervisors run this before loading policy, +/// credentials, or networking state. #[cfg(target_os = "linux")] -fn validate_oci_workspace_in_subprocess( - policy: &SandboxPolicy, - resolved_identity: ResolvedProcessIdentity, - workdir: &Path, -) -> Result<()> { - use std::os::unix::process::CommandExt; - - let (uid, gid, supplementary_gids) = resolve_filesystem_identity(policy, resolved_identity)?; - let uid = uid.ok_or_else(|| miette::miette!("workspace validator UID is unresolved"))?; - let gid = gid.ok_or_else(|| miette::miette!("workspace validator GID is unresolved"))?; - let groups = supplementary_gids - .iter() - .map(|group| group.as_raw()) - .collect::>(); - let executable = std::env::current_exe().into_diagnostic()?; - let mut command = std::process::Command::new(executable); - command - .arg("validate-workspace") - .arg("--workdir") - .arg(workdir) - .arg("--expected-uid") - .arg(uid.to_string()) - .arg("--expected-gid") - .arg(gid.to_string()) - .env_clear() - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()); - - // `pre_exec` runs after fork and before exec. These direct credential - // syscalls are async-signal-safe and affect only the one-shot child. - #[allow(unsafe_code)] - unsafe { - command.pre_exec(move || { - if libc::setgroups(groups.len(), groups.as_ptr()) != 0 - || libc::setgid(gid.as_raw()) != 0 - || libc::setuid(uid.as_raw()) != 0 - { - return Err(std::io::Error::last_os_error()); - } - Ok(()) - }); - } - - let output = command.output().into_diagnostic()?; - if output.status.success() { - return Ok(()); - } - - let diagnostic = String::from_utf8_lossy(&output.stderr); - let diagnostic = diagnostic.trim(); - if diagnostic.is_empty() { - return Err(miette::miette!( - "image workspace validation failed with status {}", - output.status - )); - } - Err(miette::miette!( - "image workspace validation failed: {diagnostic}" - )) -} - -#[cfg(unix)] -fn validate_workspace_component( - path: &Path, - uid: Option, - gid: Option, - supplementary_gids: &[Gid], - is_workspace: bool, -) -> Result<()> { - let metadata = std::fs::symlink_metadata(path).map_err(|error| { - if error.kind() == std::io::ErrorKind::NotFound { - miette::miette!( - "image workspace path component '{}' does not exist", - path.display() - ) - } else { - miette::miette!( - "failed to inspect image workspace path component '{}': {error}", - path.display() - ) - } - })?; - if metadata.file_type().is_symlink() { - return Err(miette::miette!( - "workspace path component '{}' is a symlink — refusing to follow it", - path.display() - )); - } - if !metadata.is_dir() { - return Err(miette::miette!( - "workspace path component '{}' is not a directory", - path.display() - )); - } - let required = if is_workspace { 0o3 } else { 0o1 }; - if !identity_has_permissions(&metadata, uid, gid, supplementary_gids, required) { - let requirement = if is_workspace { - "writable and traversable" - } else { - "traversable" - }; - return Err(miette::miette!( - "workspace path component '{}' is not {requirement} by the sandbox identity in the image", - path.display() - )); - } - Ok(()) -} - -#[cfg(target_os = "linux")] -pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { - use rustix::fs::{Access, AtFlags, FileType, Mode, OFlags}; +pub fn validate_oci_workspace_structure(root: &Path) -> Result<()> { + use rustix::fs::{AtFlags, FileType, Mode, OFlags}; let components = validated_workspace_components(root, false)?; let open_flags = OFlags::PATH | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; let mut current_path = PathBuf::from("/"); let mut current_fd = rustix::fs::open("/", open_flags, Mode::empty()).into_diagnostic()?; - rustix::fs::accessat( - ¤t_fd, - ".", - Access::EXEC_OK, - AtFlags::EACCESS | AtFlags::SYMLINK_NOFOLLOW, - ) - .map_err(|error| { - miette::miette!( - "workspace path component '{}' is not traversable by the sandbox identity in the image: {error}", - current_path.display() - ) - })?; - let last_component = components.len().saturating_sub(1); - for (index, component) in components.into_iter().enumerate() { + for component in components { current_path.push(&component); let stat = rustix::fs::statat(¤t_fd, &component, AtFlags::SYMLINK_NOFOLLOW).map_err( |error| { @@ -1494,21 +1341,6 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { current_path.display() )); } - - let is_workspace = index == last_component; - rustix::fs::accessat( - ¤t_fd, - &component, - Access::EXEC_OK, - AtFlags::EACCESS | AtFlags::SYMLINK_NOFOLLOW, - ) - .map_err(|error| { - miette::miette!( - "workspace path component '{}' is not traversable by the sandbox identity in the image: {error}", - current_path.display() - ) - })?; - let next_fd = rustix::fs::openat(¤t_fd, &component, open_flags, Mode::empty()) .map_err(|error| { miette::miette!( @@ -1516,62 +1348,45 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { current_path.display() ) })?; - if is_workspace { - validate_effective_workspace_write(&next_fd, ¤t_path)?; - } current_fd = next_fd; } Ok(()) } -#[cfg(target_os = "linux")] -fn validate_effective_workspace_write(fd: &impl std::os::fd::AsFd, path: &Path) -> Result<()> { - use rustix::fs::{AtFlags, Mode, OFlags}; - - let mode = Mode::RUSR | Mode::WUSR; - let tmpfile_flags = OFlags::TMPFILE | OFlags::WRONLY | OFlags::CLOEXEC; - match rustix::fs::openat(fd, ".", tmpfile_flags, mode) { - Ok(_probe) => return Ok(()), - Err(rustix::io::Errno::INVAL | rustix::io::Errno::ISDIR | rustix::io::Errno::NOTSUP) => {} - Err(error) => { +#[cfg(not(target_os = "linux"))] +pub fn validate_oci_workspace_structure(root: &Path) -> Result<()> { + let components = validated_workspace_components(root, false)?; + let mut current = PathBuf::from("/"); + for component in components { + current.push(component); + let metadata = std::fs::symlink_metadata(¤t).map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + miette::miette!( + "image workspace path component '{}' does not exist", + current.display() + ) + } else { + miette::miette!( + "failed to inspect image workspace path component '{}': {error}", + current.display() + ) + } + })?; + if metadata.file_type().is_symlink() { return Err(miette::miette!( - "workspace path component '{}' is not writable by the sandbox identity in the image: {error}", - path.display() + "workspace path component '{}' is a symlink — refusing to follow it", + current.display() )); } - } - - // Some filesystems do not implement O_TMPFILE. Fall back to a short-lived, - // no-follow entry. A collision fails closed after bounded retries. - let create_flags = - OFlags::CREATE | OFlags::EXCL | OFlags::WRONLY | OFlags::NOFOLLOW | OFlags::CLOEXEC; - for attempt in 0..16 { - let name = format!(".openshell-workdir-probe-{}-{attempt}", std::process::id()); - match rustix::fs::openat(fd, &name, create_flags, mode) { - Ok(_probe) => { - rustix::fs::unlinkat(fd, &name, AtFlags::empty()).map_err(|error| { - miette::miette!( - "workspace write probe cleanup failed for '{}': {error}", - path.display() - ) - })?; - return Ok(()); - } - Err(rustix::io::Errno::EXIST) => {} - Err(error) => { - return Err(miette::miette!( - "workspace path component '{}' is not writable by the sandbox identity in the image: {error}", - path.display() - )); - } + if !metadata.is_dir() { + return Err(miette::miette!( + "workspace path component '{}' is not a directory", + current.display() + )); } } - - Err(miette::miette!( - "workspace write probe could not allocate a unique entry in '{}'", - path.display() - )) + Ok(()) } /// Prepare only the resolved `OpenShell` workspace directory itself. @@ -1831,11 +1646,9 @@ pub fn prepare_filesystem_with_identity( let (uid, gid, supplementary_gids) = resolve_filesystem_identity(policy, resolved_identity)?; - // Docker owns workspace resolution and must make the selected root usable - // by the final effective identity, including when both policy identity - // fields were explicit. Validate it before processing any user-authored - // read-write paths so an unsafe image path fails first. Other drivers - // retain their preparation. + // Local OCI drivers structurally validate image-derived workdirs during + // supervisor startup. Only the /sandbox compatibility fallback is managed + // here; image authors own permissions on every other OCI workdir. if prepare_workspace { let workspace = workdir.ok_or_else(|| { miette::miette!("local container driver did not supply a workspace workdir") @@ -1844,12 +1657,6 @@ pub fn prepare_filesystem_with_identity( if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) { info!(path = %workspace.display(), ?uid, ?gid, "Preparing managed workspace"); prepare_oci_workspace(workspace, uid, gid, &supplementary_gids)?; - } else { - info!(path = %workspace.display(), ?uid, ?gid, "Validating image workspace authority"); - #[cfg(target_os = "linux")] - validate_oci_workspace_in_subprocess(policy, resolved_identity, workspace)?; - #[cfg(not(target_os = "linux"))] - validate_oci_workspace(workspace, uid, gid, &supplementary_gids)?; } } @@ -1867,8 +1674,8 @@ pub fn prepare_filesystem_with_identity( } // Retain the existing Kubernetes/OpenShift behavior for driver-injected - // numeric identities. Docker clears this variable and does not receive - // identity-specific workspace preparation. + // numeric identities. Docker and Podman clear this variable and do not + // receive identity-specific workspace preparation. if std::env::var(openshell_core::sandbox_env::SANDBOX_UID).is_ok_and(|uid| !uid.is_empty()) { let sandbox_home = Path::new("/sandbox"); if sandbox_home.exists() { @@ -2972,232 +2779,60 @@ mod tests { assert!(child.exists(), "image-provided child should be untouched"); } - #[cfg(unix)] #[test] - fn validate_oci_workspace_accepts_existing_owner_writable_directory() { - let dir = tempfile::tempdir().unwrap(); - let root = dir.path().canonicalize().unwrap().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).unwrap(); + fn structural_workspace_validation_rejects_symlink_components_without_testing_writes() { + use std::os::unix::fs::{PermissionsExt, symlink}; - validate_oci_workspace( - &root, - Some(nix::unistd::geteuid()), - Some(nix::unistd::getegid()), - &[], - ) - .expect("image owner already has write and traverse authority"); - } + let temp_root = std::env::temp_dir().canonicalize().unwrap(); + let dir = tempfile::tempdir_in(temp_root).unwrap(); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + let real = dir.path().join("real"); + std::fs::create_dir(&real).unwrap(); + std::fs::set_permissions(&real, std::fs::Permissions::from_mode(0o555)).unwrap(); - #[cfg(unix)] - #[test] - fn validate_oci_workspace_accepts_supplementary_group_write_authority() { - let dir = tempfile::tempdir_in("/tmp").unwrap(); - std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o711)).unwrap(); - let root = dir.path().canonicalize().unwrap().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o070)).unwrap(); - let metadata = std::fs::symlink_metadata(&root).unwrap(); + validate_oci_workspace_structure(&real) + .expect("structure-only validation must not require write access"); - validate_oci_workspace( - &root, - Some(Uid::from_raw(metadata.uid().wrapping_add(1))), - Some(Gid::from_raw(metadata.gid().wrapping_add(1))), - &[Gid::from_raw(metadata.gid())], - ) - .expect("supplementary group already has write and traverse authority"); - } - - #[cfg(unix)] - #[test] - fn validate_oci_workspace_rejects_unwritable_directory() { - let dir = tempfile::tempdir_in("/tmp").unwrap(); - std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o711)).unwrap(); - let root = dir.path().canonicalize().unwrap().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o755)).unwrap(); - let metadata = std::fs::symlink_metadata(&root).unwrap(); - - let error = validate_oci_workspace( - &root, - Some(Uid::from_raw(metadata.uid().wrapping_add(1))), - Some(Gid::from_raw(metadata.gid().wrapping_add(1))), - &[], - ) - .unwrap_err(); - assert!(error.to_string().contains("not writable and traversable")); + let alias = dir.path().join("alias"); + symlink(&real, &alias).unwrap(); + let error = validate_oci_workspace_structure(&alias).unwrap_err(); + assert!(error.to_string().contains("symlink")); } - #[cfg(unix)] #[test] - fn validate_oci_workspace_rejects_missing_path() { + fn structural_workspace_validation_rejects_missing_components() { let dir = tempfile::tempdir().unwrap(); - let root = dir.path().canonicalize().unwrap().join("missing"); + let missing = dir.path().canonicalize().unwrap().join("missing"); - let error = validate_oci_workspace( - &root, - Some(nix::unistd::geteuid()), - Some(nix::unistd::getegid()), - &[], - ) - .unwrap_err(); + let error = validate_oci_workspace_structure(&missing).unwrap_err(); assert!(error.to_string().contains("does not exist")); } - #[cfg(target_os = "linux")] - #[test] - #[allow(unsafe_code)] - fn effective_identity_validation_honors_named_user_acl() { - const TEST_UID: u32 = 42_234; - const TEST_GID: u32 = 42_235; - const ACL_XATTR_VERSION: u32 = 2; - const ACL_USER_OBJ: u16 = 0x01; - const ACL_USER: u16 = 0x02; - const ACL_GROUP_OBJ: u16 = 0x04; - const ACL_MASK: u16 = 0x10; - const ACL_OTHER: u16 = 0x20; - const ACL_UNDEFINED_ID: u32 = u32::MAX; - - if !nix::unistd::geteuid().is_root() { - return; - } - - let dir = tempfile::tempdir_in("/tmp").unwrap(); - std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o711)).unwrap(); - let root = dir.path().canonicalize().unwrap().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).unwrap(); - - let mut acl = ACL_XATTR_VERSION.to_ne_bytes().to_vec(); - for (tag, permissions, id) in [ - (ACL_USER_OBJ, 0o7_u16, ACL_UNDEFINED_ID), - (ACL_USER, 0o7_u16, TEST_UID), - (ACL_GROUP_OBJ, 0o0_u16, ACL_UNDEFINED_ID), - (ACL_MASK, 0o7_u16, ACL_UNDEFINED_ID), - (ACL_OTHER, 0o0_u16, ACL_UNDEFINED_ID), - ] { - acl.extend_from_slice(&tag.to_ne_bytes()); - acl.extend_from_slice(&permissions.to_ne_bytes()); - acl.extend_from_slice(&id.to_ne_bytes()); - } - let path = CString::new(root.as_os_str().as_encoded_bytes()).unwrap(); - let name = c"system.posix_acl_access"; - let result = unsafe { - libc::setxattr( - path.as_ptr(), - name.as_ptr(), - acl.as_ptr().cast(), - acl.len(), - 0, - ) - }; - assert_eq!( - result, - 0, - "setxattr failed: {}", - std::io::Error::last_os_error() - ); - - match unsafe { fork() }.expect("fork should succeed") { - ForkResult::Child => { - let credentials_dropped = unsafe { - libc::setgroups(0, std::ptr::null()) == 0 - && libc::setgid(TEST_GID) == 0 - && libc::setuid(TEST_UID) == 0 - }; - let valid = credentials_dropped - && validate_oci_workspace_as_effective_identity(&root).is_ok(); - unsafe { libc::_exit(i32::from(!valid)) }; - } - ForkResult::Parent { child } => { - assert_eq!( - waitpid(child, None).expect("waitpid should succeed"), - WaitStatus::Exited(child, 0), - "named ACL user should retain workspace authority" - ); - } - } - } - - #[cfg(target_os = "linux")] - #[test] - #[allow(unsafe_code)] - fn effective_identity_validation_honors_landlock_denial() { - let dir = tempfile::tempdir_in("/tmp").unwrap(); - let root = dir.path().canonicalize().unwrap().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).unwrap(); - - let mut policy = policy_with_process(ProcessPolicy::default()); - policy.filesystem = FilesystemPolicy { - read_only: vec![root.clone()], - read_write: Vec::new(), - include_workdir: false, - }; - policy.landlock = LandlockPolicy { - compatibility: openshell_core::policy::LandlockCompatibility::HardRequirement, - }; - let Ok(prepared) = sandbox::linux::prepare_current_user(&policy, None) else { - return; - }; - - match unsafe { fork() }.expect("fork should succeed") { - ForkResult::Child => { - let denied = sandbox::linux::enforce(prepared).is_ok() - && validate_oci_workspace_as_effective_identity(&root).is_err(); - unsafe { libc::_exit(i32::from(!denied)) }; - } - ForkResult::Parent { child } => { - assert_eq!( - waitpid(child, None).expect("waitpid should succeed"), - WaitStatus::Exited(child, 0), - "kernel-effective validation should honor an enforced LSM denial" - ); - } - } - } - #[cfg(unix)] #[test] - fn validate_oci_workspace_rejects_restrictive_parent() { + fn filesystem_preparation_does_not_repair_nondefault_oci_workspace() { let dir = tempfile::tempdir().unwrap(); - let parent = dir.path().canonicalize().unwrap().join("private"); - let root = parent.join("project"); - std::fs::create_dir_all(&root).unwrap(); - std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o700)).unwrap(); - std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o777)).unwrap(); - let metadata = std::fs::symlink_metadata(&parent).unwrap(); + let root = dir.path().canonicalize().unwrap().join("project"); + std::fs::create_dir(&root).unwrap(); + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o555)).unwrap(); + let policy = policy_with_process(ProcessPolicy { + run_as_user: Some(nix::unistd::geteuid().as_raw().to_string()), + run_as_group: Some(nix::unistd::getegid().as_raw().to_string()), + }); - let error = validate_oci_workspace( - &root, - Some(Uid::from_raw(metadata.uid().wrapping_add(1))), - Some(Gid::from_raw(metadata.gid().wrapping_add(1))), - &[], + prepare_filesystem_with_identity( + &policy, + ResolvedProcessIdentity::default(), + root.to_str(), + true, ) - .unwrap_err(); - assert!(error.to_string().contains("not traversable")); - } - - #[cfg(unix)] - #[test] - fn validate_oci_workspace_rejects_symlink_component() { - use std::os::unix::fs::symlink; - - let dir = tempfile::tempdir().unwrap(); - let base = dir.path().canonicalize().unwrap(); - let target = base.join("target"); - let link = base.join("link"); - std::fs::create_dir(&target).unwrap(); - symlink(&target, &link).unwrap(); + .expect("non-default workspace permissions are image-owned"); - let error = validate_oci_workspace( - &link, - Some(nix::unistd::geteuid()), - Some(nix::unistd::getegid()), - &[], - ) - .unwrap_err(); - assert!(error.to_string().contains("symlink")); + let mode = std::fs::symlink_metadata(&root) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o555); } #[cfg(unix)] diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 07302da953..1f584891c7 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -717,8 +717,8 @@ impl Default for PtyRequest { /// (or defaults to `/home/{user}`). /// /// For numeric UIDs, there is no passwd entry, so the default remains -/// `("{uid}", "/sandbox")`. Docker replaces that default with its resolved -/// image workspace. +/// `("{uid}", "/sandbox")`. Local OCI drivers replace that default with the +/// resolved image workspace. fn session_user_and_home(policy: &SandboxPolicy, workdir_home: Option<&str>) -> (String, String) { let (user, default_home) = match policy.process.run_as_user.as_deref() { Some(user) if !user.is_empty() => { diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index ea4c1a37b0..f29586a2fd 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -446,23 +446,39 @@ declared name or numeric components for both direct and SSH children. When `USER` omits the group, the supervisor uses the user's numeric primary GID. It does not modify `/etc/passwd` or `/etc/group`. -Docker also inspects OCI `WorkingDir`. An absolute value becomes the +Docker and Podman also inspect OCI `WorkingDir`. An absolute value becomes the agent workspace; an empty, root (`/`), or explicit `/sandbox` value uses the managed `/sandbox` compatibility workspace. OpenShell creates and owns that compatibility workspace. Any other workdir must -already exist in the immutable image without symlink components. The completed -UID/GID and supplementary groups must already be able to traverse every parent -and write and enter the workdir. OpenShell does not change that directory's -ownership or mode. A one-shot validator drops to that identity and uses kernel -effective-access checks, including POSIX ACL grants and LSM denials. It rejects -workdirs that overlap the OCI runtime namespaces under `/proc`, `/sys`, or -`/dev`, and rejects overlap with actual OpenShell control paths. Docker checks -the original image filesystem in the final supervisor and rejects image -`VOLUME` declarations that would mask the workdir or one of its parents before -validation. The resolved workspace is the cwd and `HOME` for direct and SSH -children. The supervisor itself starts from `/`, so a missing or invalid -workspace is handled during readiness instead of preventing the container -runtime from starting it. +have directory-only, non-symlink components when the supervisor validates it; +OpenShell does not create image-derived components or change their ownership or +mode. The image author is responsible for making the final OCI/policy identity +able to traverse and write it. An unusable image fails naturally when its workload +changes directory or writes. + +Before loading policy, credentials, TLS, or networking state, the final +supervisor rejects missing or symlink components, non-directories, and +OpenShell control paths. The drivers reject overlap in either +direction with `/proc`, `/sys`, `/dev`, `/bin`, `/sbin`, `/usr/bin`, +`/usr/sbin`, `/lib`, `/lib64`, `/usr/lib`, and `/usr/lib64`. This narrow +guardrail prevents OpenShell from mounting its persistent workspace over +kernel-managed paths or executable and library roots used by the supervisor; +it does not validate custom-image integrity or workdir permissions. Explicit +driver-config mounts cannot cover the resolved OCI `WORKDIR` or an OpenShell +control path. Docker rejects image-declared `VOLUME` entries that cover the +workdir or control paths. Podman leaves image-declared volume behavior to the +runtime. + +Docker validates the image directory directly. Podman mounts its persistent +named workspace volume at the resolved workdir and validates the result after +Podman's normal initial copy-up. OpenShell does not repair Podman volume +ownership or permissions. Image authors are responsible for making `USER` able +to use `WORKDIR`; failures surface naturally when the workload changes directory +or writes. Rootless, rootful, user-namespace, and SELinux configurations may +initialize volumes differently. +The resolved workspace is the cwd and `HOME` for direct and SSH children. The +supervisor itself starts from `/`, so a missing or invalid workspace is handled +during readiness instead of preventing the container runtime from starting it. Sandbox creation fails before readiness if a required `USER` component is missing, malformed, unknown, ambiguous, or resolves to UID/GID 0. An image @@ -493,9 +509,10 @@ The VM driver injects the sandbox UID into the rootfs guest's `/etc/passwd`, `/e Docker and Podman custom images do not need a baked-in `"sandbox"` user. Declare a non-root OCI `USER`, or set both process identity fields explicitly in policy. Named image users require matching account entries; a numeric `UID:GID` pair -does not. For Docker, declare an absolute OCI `WORKDIR` to select the workspace. +does not. For Docker or Podman, declare an absolute OCI `WORKDIR` to select the workspace. Images with no working directory, `WORKDIR /`, or `WORKDIR /sandbox` use -OpenShell's managed `/sandbox` compatibility workspace. For any other Docker +OpenShell's managed `/sandbox` compatibility workspace. For any other local-container path, create the directory in the image and grant the final process identity -write and execute permission in the Dockerfile. Podman, Kubernetes/OpenShift, -and VM sandboxes continue to use `/sandbox`. +write and execute permission in the Dockerfile. OpenShell does not repair an +unusable workdir. Kubernetes/OpenShift and VM sandboxes continue to use +`/sandbox`. diff --git a/e2e/rust/tests/custom_image.rs b/e2e/rust/tests/custom_image.rs index 5652a0011e..fd145eee16 100644 --- a/e2e/rust/tests/custom_image.rs +++ b/e2e/rust/tests/custom_image.rs @@ -114,7 +114,7 @@ async fn sandbox_from_custom_dockerfile() { touch ssh-oci-user-write; echo ssh-write-ok", ]) .await - .expect("SSH child should write to prepared workspace"); + .expect("SSH child should write to the image-owned workspace"); assert!( ssh_output.contains("ssh-write-ok"), "expected SSH write marker:\n{ssh_output}" @@ -218,30 +218,23 @@ async fn sandbox_from_passwd_less_numeric_oci_user() { #[tokio::test] #[serial(custom_image)] -async fn sandbox_rejects_image_workdir_that_would_require_new_authority() { +async fn sandbox_does_not_repair_unwritable_image_workdir() { let tmpdir = tempfile::tempdir().expect("create tmpdir"); let dockerfile_path = tmpdir.path().join("Dockerfile"); fs::write(&dockerfile_path, UNWRITABLE_WORKDIR_DOCKERFILE_CONTENT).expect("write Dockerfile"); let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); - let result = SandboxGuard::create_keep_with_args( + let mut guard = SandboxGuard::create_keep_with_args( &["--from", dockerfile_str, "--no-tty"], - &["sh", "-c", "echo should-not-run"], - "should-not-run", + &[ + "sh", + "-c", + "set -eu; test ! -w .; test \"$(stat -c %a .)\" = 755; echo Ready; sleep infinity", + ], + "Ready", ) - .await; - let error = match result { - Ok(mut guard) => { - guard.cleanup().await; - panic!("root-owned workdir must not be made writable for the image user"); - } - Err(error) => error, - }; - let message = error.to_string(); - assert!( - message.contains("WorkingDir") - || message.contains("workspace") - || message.contains("readiness"), - "expected workspace authority failure, got: {message}" - ); + .await + .expect("structurally safe workdir should not be rejected or repaired"); + + guard.cleanup().await; } diff --git a/e2e/rust/tests/driver_config_volume.rs b/e2e/rust/tests/driver_config_volume.rs index 0702a4637d..2a91f9ce1d 100644 --- a/e2e/rust/tests/driver_config_volume.rs +++ b/e2e/rust/tests/driver_config_volume.rs @@ -25,9 +25,9 @@ use serde_json::{Map, Value}; const TEST_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest"; const VOLUME_TARGET: &str = "/sandbox/e2e-volume"; const BIND_TARGET: &str = "/sandbox/e2e-bind"; -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] const OCI_VOLUME_TARGET: &str = "/workspace/project/e2e-volume"; -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] const OCI_USER_DOCKERFILE: &str = r#"FROM public.ecr.aws/docker/library/python:3.13-slim RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ @@ -169,12 +169,12 @@ async fn sandbox_mounts_existing_driver_config_volume() { } #[tokio::test] -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] async fn oci_workspace_preparation_skips_nested_volume_ownership() { let driver = e2e_driver().expect("OPENSHELL_E2E_DRIVER must be set by the e2e wrapper"); assert!( - driver == "docker", - "OCI workspace mount e2e requires docker, got {driver}" + matches!(driver.as_str(), "docker" | "podman"), + "OCI workspace mount e2e requires docker or podman, got {driver}" ); let volume = VolumeGuard::create(&driver) @@ -342,7 +342,7 @@ async fn verify_volume(volume: &VolumeGuard) -> Result<(), String> { Ok(()) } -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] async fn verify_volume_ownership(volume: &VolumeGuard) -> Result<(), String> { let output = run_volume_container( volume, diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs index e30516bf09..1b6b243a14 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -3,14 +3,12 @@ #![cfg(feature = "e2e-podman")] -//! Podman-specific E2E coverage for OCI identity inspection and immutable-image -//! launch. +//! Podman-specific E2E coverage for OCI identity/workspace inspection, +//! workspace-volume copy-up, and immutable-image launch. //! //! The test builds an image through the selected Podman engine, creates a -//! sandbox from its mutable tag, and verifies both the child identity and the -//! image ID recorded on the real sandbox container. This exercises the Podman -//! API inspect → protected metadata → create path rather than only its unit -//! serialization boundaries. +//! sandbox from its mutable tag, and verifies the child identity, workspace, +//! copied image content, and image ID recorded on the real sandbox container. use std::process::Stdio; @@ -54,7 +52,16 @@ impl ImageGuard { let containerfile = context.path().join("Containerfile"); std::fs::write( &containerfile, - format!("FROM {BASE_IMAGE}\nUSER {OCI_UID}:{OCI_GID}\n"), + format!( + "FROM {BASE_IMAGE}\n\ + USER 0:0\n\ + RUN mkdir -p /home/app/project && \ + chown {OCI_UID}:{OCI_GID} /home/app /home/app/project && \ + chmod 0700 /home/app\n\ + WORKDIR /home/app/project\n\ + RUN printf root-owned > root-owned.txt && chown {OCI_UID}:{OCI_GID} .\n\ + USER {OCI_UID}:{OCI_GID}\n" + ), ) .map_err(|err| format!("write Containerfile: {err}"))?; @@ -164,7 +171,7 @@ fn normalized_image_id(image_id: &str) -> &str { } #[tokio::test] -async fn podman_uses_oci_identity_and_inspected_image_id() { +async fn podman_uses_oci_identity_workspace_copy_up_and_inspected_image_id() { if !is_e2e_driver("podman") { eprintln!("Skipping Podman OCI identity test: e2e driver is not podman"); return; @@ -178,17 +185,19 @@ async fn podman_uses_oci_identity_and_inspected_image_id() { std::fs::write(policy.path(), OCI_FALLBACK_POLICY).expect("write OCI fallback policy"); let policy_path = policy.path().to_str().expect("policy path is UTF-8"); let mut sandbox = SandboxGuard::create_keep_with_args( - &[ - "--from", - &image.tag, - "--policy", - policy_path, - "--no-tty", - ], + &["--from", &image.tag, "--policy", policy_path, "--no-tty"], &[ "sh", "-c", - "set -eu; printf 'direct-identity=%s:%s\n' \"$(id -u)\" \"$(id -g)\"; echo podman-oci-identity-ready; sleep infinity", + "set -eu; \ + test \"$(pwd -P)\" = /home/app/project; \ + test \"$HOME\" = /home/app/project; \ + test \"$(cat root-owned.txt)\" = root-owned; \ + test \"$(stat -c %u:%g .)\" = 2345:2346; \ + test \"$(stat -c %u:%g root-owned.txt)\" = 0:0; \ + touch direct-workspace-write; \ + printf 'direct-identity=%s:%s\n' \"$(id -u)\" \"$(id -g)\"; \ + echo podman-oci-identity-ready; sleep infinity", ], READY_MARKER, ) @@ -205,7 +214,13 @@ async fn podman_uses_oci_identity_and_inspected_image_id() { .exec(&[ "sh", "-c", - "test \"$(id -u):$(id -g)\" = 2345:2346; echo podman-ssh-identity-ok", + "set -eu; \ + test \"$(id -u):$(id -g)\" = 2345:2346; \ + test \"$(pwd -P)\" = /home/app/project; \ + test \"$HOME\" = /home/app/project; \ + test -f direct-workspace-write; \ + touch ssh-workspace-write; \ + echo podman-ssh-identity-ok", ]) .await .expect("SSH child should use Podman OCI identity"); From feb15b4dfed12c721d200eaece5d7bfc498da584 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Fri, 14 Aug 2026 09:50:33 -0700 Subject: [PATCH 2/2] refactor(sandbox): defer dmesg hardening Signed-off-by: Matthew Grossman --- crates/openshell-core/src/container_paths.rs | 1 + .../openshell-driver-podman/src/container.rs | 5 ++ .../src/bypass_monitor/mod.rs | 62 +++---------------- 3 files changed, 14 insertions(+), 54 deletions(-) diff --git a/crates/openshell-core/src/container_paths.rs b/crates/openshell-core/src/container_paths.rs index 6e41c30603..24a8a5880d 100644 --- a/crates/openshell-core/src/container_paths.rs +++ b/crates/openshell-core/src/container_paths.rs @@ -25,6 +25,7 @@ pub const NETNS_IPROUTE2_ROOT: &str = "/var/run/netns"; /// /// TODO: Make the supervisor's helper functionality self-contained so it no /// longer depends on executable and library paths shared with the image. +/// pub const FORBIDDEN_WORKSPACE_ROOTS: &[&str] = &[ // Kernel-managed OCI runtime mounts. "/proc", diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 15b7cc7aea..4c18d4a2ab 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1036,6 +1036,11 @@ pub fn build_container_spec_for_image( // /openshell-sandbox, so it appears at /opt/openshell/bin/openshell-sandbox. image_volumes, hostname: format!("sandbox-{}", sandbox.name), + // This is the supervisor's startup cwd, not the workload cwd. Starting + // at the image-selected workspace would let a malformed or inaccessible + // WORKDIR prevent the OCI runtime from launching the supervisor before + // it can validate the mounted workspace and report readiness. Direct and + // SSH children use the resolved workspace passed through `--workdir`. work_dir: "/".to_string(), // Override the image's ENTRYPOINT so the supervisor binary runs // directly. Sandbox images (e.g. the community base image) set diff --git a/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs b/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs index afb11fccad..44847b0d13 100644 --- a/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs +++ b/crates/openshell-supervisor-process/src/bypass_monitor/mod.rs @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Bypass detection monitor — follows kernel log messages with `dmesg` to +//! Bypass detection monitor — reads kernel log messages from `/dev/kmsg` to //! detect and report direct connection attempts that bypass the HTTP CONNECT //! proxy. //! @@ -12,9 +12,9 @@ //! //! ## Graceful degradation //! -//! If an approved `dmesg` binary is unavailable, the monitor logs a one-time -//! warning and returns. The nftables reject rules still provide fast-fail UX — -//! the monitor only adds diagnostic visibility. +//! If `/dev/kmsg` cannot be opened (e.g., restricted container environment), +//! the monitor logs a one-time warning and returns. The nftables reject rules +//! still provide fast-fail UX — the monitor only adds diagnostic visibility. mod procfs; @@ -29,21 +29,7 @@ use std::sync::atomic::{AtomicU32, Ordering}; use tokio::sync::mpsc; use tracing::debug; -const DMESG_PATHS: &[&str] = &[ - "/usr/bin/dmesg", - "/bin/dmesg", - "/usr/sbin/dmesg", - "/sbin/dmesg", -]; - -fn find_dmesg_binary<'a>(candidates: &'a [&'a str]) -> Option<&'a str> { - candidates.iter().copied().find(|candidate| { - let path = std::path::Path::new(candidate); - path.is_absolute() && path.is_file() - }) -} - -/// A parsed nftables kernel log entry. +/// A parsed nftables log entry from `/dev/kmsg`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BypassEvent { /// Destination IP address. @@ -195,23 +181,8 @@ pub fn spawn( use std::io::BufRead; use std::process::{Command, Stdio}; - // Use only known system locations. The sandbox image's PATH is controlled - // by the image author and may include the writable workspace. - let Some(dmesg_path) = find_dmesg_binary(DMESG_PATHS) else { - let event = NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx()) - .activity(ActivityId::Other) - .severity(SeverityId::Low) - .message( - "dmesg not available at an approved system path; bypass detection monitor will not run. \ - Bypass REJECT rules still provide fast-fail behavior.", - ) - .build(); - ocsf_emit!(event); - return None; - }; - - // Verify dmesg works before spawning the monitor. - let dmesg_check = Command::new(dmesg_path) + // Verify dmesg is available before spawning the monitor. + let dmesg_check = Command::new("dmesg") .arg("--version") .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -238,7 +209,7 @@ pub fn spawn( let handle = tokio::task::spawn_blocking(move || { // Start dmesg in follow mode to tail new kernel messages. - let mut child = match Command::new(dmesg_path) + let mut child = match Command::new("dmesg") .args(["--follow", "--notime"]) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -411,23 +382,6 @@ fn resolve_process_identity(entrypoint_pid: u32, src_port: u16) -> (String, Stri mod tests { use super::*; - #[test] - fn dmesg_lookup_uses_only_existing_absolute_candidates() { - let dir = tempfile::tempdir().unwrap(); - let binary = dir.path().join("dmesg"); - std::fs::write(&binary, "test").unwrap(); - let binary = binary.to_str().unwrap(); - - assert_eq!( - find_dmesg_binary(&["relative/dmesg", "/definitely/missing/dmesg", binary]), - Some(binary) - ); - assert_eq!( - find_dmesg_binary(&["relative/dmesg", "/definitely/missing/dmesg"]), - None - ); - } - #[test] fn parse_kmsg_line_tcp_bypass() { let line = "6,1234,5678,-;openshell:bypass:sandbox-abcd1234:IN= OUT=veth-s-abcd1234 \