diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 319031b1d..62512792c 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -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 `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 effective global-or-sandbox policy UID, primary GID, or supplementary groups must already be able to traverse every parent and write and enter the directory. Podman checks the original image in a networkless temporary probe before attaching the workspace volume, so inspect the probe failure in gateway logs. +- If Podman reports probe cleanup or timeout failures, inspect temporary containers with `podman ps -a --filter name=openshell-workdir-probe` and gateway logs. The driver force-removes the exact attempt-unique probe on every normal success or failure path. A gateway process crash can leave a stopped probe; verify its generated name and image before removing it manually. The driver deliberately does not sweep containers by a forgeable label or name prefix. - 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 866e57008..1b51187dd 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -446,6 +446,13 @@ 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 gateways also honor the image's OCI `WORKDIR`. An empty +value, `/`, or `/sandbox` uses the compatibility workspace at `/sandbox`. +Any other value must be an absolute, normalized path that already exists in +the image, contains no symlink components, and is traversable and writable by +the sandbox UID, GID, and supplementary groups. Podman validates that access +against the pinned image before its managed workspace volume covers the path. + ### Forward ports ```bash diff --git a/Cargo.lock b/Cargo.lock index 88c7dc0b7..e6224b335 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3855,6 +3855,7 @@ dependencies = [ "tracing", "tracing-subscriber", "url", + "uuid", ] [[package]] diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 646b6320b..846df471a 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`. They also 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,7 +199,7 @@ 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 @@ -210,12 +210,19 @@ 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 -`filesystem.include_workdir` is enabled, it becomes the automatic writable -policy path. Podman, Kubernetes/OpenShift, and VM retain their existing -`/sandbox` workspace behavior. +Both drivers reject image `VOLUME` declarations that would mask the workdir +ancestry. Docker performs the check in the final container before workload +launch. Podman performs it in a minimal networkless container from the same +pinned image ID before its managed workspace volume covers the path. The probe +adopts the identity source from the effective global-or-sandbox policy, or +discovers the image policy when neither exists, and emits a normalized +attestation that the final supervisor must match. This internal Podman-only +contract is required because the managed volume hides the original image tree +before the final supervisor starts. The resolved workspace is the child cwd and `HOME`; +when `filesystem.include_workdir` is enabled, it +becomes the automatic writable policy path. Kubernetes/OpenShift keep their +`/sandbox` PVC and `fsGroup` behavior, and VM keeps its `/sandbox` guest +initialization path. 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/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 1549258fa..22a82e110 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -126,6 +126,13 @@ pub const SANDBOX_GID: &str = "OPENSHELL_SANDBOX_GID"; /// OCI only for the former contract. pub const OCI_IMAGE_USER: &str = "OPENSHELL_OCI_IMAGE_USER"; +/// Normalized UID/GID/supplementary-group identity attested by the Podman +/// immutable-image workspace probe. +/// +/// A non-empty value also asserts that the +/// original workspace was validated before Podman's managed volume covered it. +pub const OCI_WORKSPACE_IDENTITY: &str = "OPENSHELL_OCI_WORKSPACE_IDENTITY"; + // The corporate upstream-proxy configuration deliberately has no reserved // environment variables: it travels on the supervisor's argv // (`--upstream-proxy` and friends), which a sandbox image cannot forge the diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index dd4d9ef0f..0d139ebf2 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2195,6 +2195,7 @@ fn build_environment_for_oci_user( user_env.extend(template.environment.clone()); } user_env.extend(spec.environment.clone()); + user_env.remove(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY); environment.extend(user_env.clone()); if !user_env.is_empty() && let Ok(json) = serde_json::to_string(&user_env) @@ -2250,6 +2251,11 @@ fn build_environment_for_oci_user( environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + // Ignore image-provided Podman probe attestations. + environment.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.to_string(), + String::new(), + ); environment.insert( openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), oci_user.to_string(), diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index ac525c705..c7b83b0ce 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -45,6 +45,7 @@ fn test_sandbox() -> DriverSandbox { }), resource_requirements: None, sandbox_token: String::new(), + workspace_validation_identity: None, }), status: None, workspace: String::new(), @@ -578,6 +579,10 @@ fn build_environment_protects_oci_identity_metadata() { (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), (openshell_core::sandbox_env::SANDBOX_UID, "9999"), (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, + "9999:9999:", + ), ] { spec.environment.insert(key.to_string(), value.to_string()); } @@ -590,6 +595,10 @@ fn build_environment_protects_oci_identity_metadata() { ))); assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_UID))); assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_GID))); + assert!(env.contains(&format!( + "{}=", + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY + ))); assert!(!env.iter().any(|entry| entry.ends_with("=spoofed"))); assert!(!env.iter().any(|entry| entry.ends_with("=9999"))); } diff --git a/crates/openshell-driver-podman/Cargo.toml b/crates/openshell-driver-podman/Cargo.toml index e46d2eed8..0397f8623 100644 --- a/crates/openshell-driver-podman/Cargo.toml +++ b/crates/openshell-driver-podman/Cargo.toml @@ -35,6 +35,7 @@ tracing-subscriber = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } url = { workspace = true } +uuid = { workspace = true } [dev-dependencies] prost-types = { workspace = true } diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 965a295d1..3b9b9ecf1 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -8,14 +8,27 @@ isolation enforcement to the `openshell-sandbox` supervisor binary, which is sideloaded into each container via an OCI image volume mount. Before creating the container, the driver inspects the final sandbox image and -captures its immutable image ID and raw OCI `Config.User`. Container creation -uses that image ID with pulling disabled, preventing a mutable tag from changing -between inspection and launch. The supervisor runs as root, resolves omitted -policy identity fields from the image declaration, and drops only agent -children to the completed identity. Named OCI components remain names after -validation; a missing group is filled with the user's numeric primary GID. Explicit -`process.run_as_user` and `process.run_as_group` values take precedence -independently. +captures its immutable image ID, raw OCI `Config.User`, and OCI +`Config.WorkingDir`. Container creation uses that image ID with pulling +disabled, preventing a mutable tag from changing between inspection and launch. +The supervisor runs as root, resolves omitted policy identity fields from the +image declaration, and drops only agent children to the completed identity. +Named OCI components remain names after validation; a missing group is filled +with the user's numeric primary GID. Explicit `process.run_as_user` and +`process.run_as_group` values take precedence independently. + +An absolute OCI working directory becomes the agent workspace. An empty, +root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell +creates and owns as a compatibility workspace. For any other workdir, a +resource-limited, networkless probe verifies the original pinned image before +Podman covers the path with the managed workspace volume. The completed process +identity must already be able to traverse every parent and write and enter the +directory, without symlink components or OpenShell control-path collisions. +The gateway supplies the identity source from the effective global-or-sandbox +policy, or requests image-policy discovery when neither exists. The final +supervisor must match the probe's normalized identity before preparing the +volume. See [Compute runtimes](../../architecture/compute-runtimes.md#process-identity) +for the invariant and probe lifecycle. For a rootless networking deep dive, see [NETWORKING.md](NETWORKING.md). @@ -87,9 +100,11 @@ optional `selinux_label` of `shared` (applies `:z`) or `private` (applies 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, -`/etc/openshell`, `/etc/openshell-tls`, or `/run/netns`. +Mount targets must be absolute container paths and must not replace the +resolved workspace root or any of its parents. Nested workspace mounts remain +valid. Mounts also must not contain or be contained by concrete OpenShell +control targets such as the supervisor mount, TLS and token files, runtime +socket, or `/run/netns`. Example named-volume usage: @@ -291,6 +306,13 @@ sequenceDiagram D->>P: pull_image(supervisor, "missing") D->>P: pull_image(sandbox_image, policy) + D->>P: inspect_image(sandbox_image) + + opt Non-default OCI workdir + D->>P: create + start validation probe + D->>P: wait + read bounded logs + D->>P: force-remove exact probe name + end D->>P: create_volume(workspace) Note over D: On failure below, rollback volume diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 9fe39cf7e..c1d2d4ee8 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -25,6 +25,7 @@ const API_TIMEOUT: Duration = Duration::from_secs(30); /// Maximum allowed size for the event stream line buffer (1 MB). const MAX_EVENT_BUFFER: usize = 1_048_576; +const MAX_CONTAINER_LOG_BYTES: usize = 16 * 1024; #[derive(Debug, thiserror::Error)] pub enum PodmanApiError { @@ -117,6 +118,16 @@ pub struct ContainerState { pub finished_at: Option, } +#[derive(Debug, serde::Deserialize)] +#[serde(untagged)] +enum ContainerWaitResponse { + ExitCode(i64), + Status { + #[serde(rename = "StatusCode")] + status_code: i64, + }, +} + #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] pub struct HealthState { @@ -177,6 +188,10 @@ pub struct ImageInspect { pub struct ImageConfig { #[serde(default)] pub user: String, + #[serde(default)] + pub working_dir: String, + #[serde(default)] + pub volumes: Option>, } /// A container summary returned by the list API. @@ -332,17 +347,26 @@ impl PodmanClient { builder.body(body).expect("valid request") } - /// Send a pre-built HTTP request and return status + body bytes. - async fn send_request( + /// Execute a pre-built HTTP request and return the streaming response. + async fn execute_request( &self, req: Request>, timeout: Duration, - ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + ) -> Result, PodmanApiError> { let mut sender = self.connect().await?; - let response = tokio::time::timeout(timeout, sender.send_request(req)) + tokio::time::timeout(timeout, sender.send_request(req)) .await .map_err(|_| PodmanApiError::Timeout(timeout))? - .map_err(|e| PodmanApiError::Connection(e.to_string()))?; + .map_err(|error| PodmanApiError::Connection(error.to_string())) + } + + /// Send a pre-built HTTP request and return status + body bytes. + async fn send_request( + &self, + req: Request>, + timeout: Duration, + ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + let response = self.execute_request(req, timeout).await?; let status = response.status(); let bytes = tokio::time::timeout(timeout, response.into_body().collect()) .await @@ -352,6 +376,34 @@ impl PodmanClient { Ok((status, bytes)) } + /// Send a request while retaining at most `max_bytes` of the response. + async fn send_request_bounded( + &self, + req: Request>, + timeout: Duration, + max_bytes: usize, + ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + let response = self.execute_request(req, timeout).await?; + let status = response.status(); + let deadline = tokio::time::Instant::now() + timeout; + let mut body = response.into_body(); + let mut bytes = Vec::with_capacity(max_bytes.min(4096)); + while bytes.len() < max_bytes { + let frame = tokio::time::timeout_at(deadline, body.frame()) + .await + .map_err(|_| PodmanApiError::Timeout(timeout))?; + let Some(frame) = frame else { + break; + }; + let frame = frame.map_err(|error| PodmanApiError::Connection(error.to_string()))?; + if let Some(data) = frame.data_ref() { + let remaining = max_bytes - bytes.len(); + bytes.extend_from_slice(&data[..data.len().min(remaining)]); + } + } + Ok((status, Bytes::from(bytes))) + } + /// Perform a versioned HTTP request and return status + body bytes. async fn request( &self, @@ -457,6 +509,22 @@ impl PodmanClient { .await } + /// Wait for a container to exit and return its exit code. + pub async fn wait_container(&self, name: &str) -> Result { + validate_name(name)?; + let response: ContainerWaitResponse = self + .request_json( + hyper::Method::POST, + &format!("/libpod/containers/{name}/wait?condition=exited"), + None, + ) + .await?; + Ok(match response { + ContainerWaitResponse::ExitCode(code) => code, + ContainerWaitResponse::Status { status_code } => status_code, + }) + } + /// Stop a container with a grace period in seconds. pub async fn stop_container( &self, @@ -524,6 +592,27 @@ impl PodmanClient { .await } + /// Fetch a bounded tail of stdout and stderr from a container. + pub async fn container_logs(&self, name: &str) -> Result { + validate_name(name)?; + let req = Self::build_request( + hyper::Method::GET, + &format!( + "/{API_VERSION}/libpod/containers/{name}/logs?stdout=true&stderr=true&tail=20×tamps=false" + ), + Full::new(Bytes::new()), + None, + ); + let (status, bytes) = self + .send_request_bounded(req, API_TIMEOUT, MAX_CONTAINER_LOG_BYTES) + .await?; + if status.is_success() { + Ok(String::from_utf8_lossy(&bytes).into_owned()) + } else { + Err(error_from_response(status.as_u16(), &bytes)) + } + } + /// List containers matching label filters (e.g. `&["openshell.managed=true"]`). pub async fn list_containers( &self, @@ -973,12 +1062,12 @@ mod tests { } #[tokio::test] - async fn inspect_image_reads_immutable_id_and_oci_user() { + async fn inspect_image_reads_immutable_id_and_oci_config() { 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 +1082,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 @@ -1055,4 +1151,34 @@ mod tests { handle.await.expect("stub task should finish"); let _ = std::fs::remove_file(socket_path); } + + #[tokio::test] + async fn container_logs_fetches_bounded_stdout_and_stderr_tail() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "container-logs", + vec![StubResponse::new( + StatusCode::OK, + "workspace validation failed\n", + )], + ); + let client = PodmanClient::new(socket_path.clone()); + + let logs = client + .container_logs("workdir-probe") + .await + .expect("container logs should be returned"); + + assert_eq!(logs, "workspace validation failed\n"); + handle.await.expect("stub task should finish"); + assert_eq!( + request_log + .lock() + .expect("request log lock should not be poisoned") + .as_slice(), + [ + "GET /v5.0.0/libpod/containers/workdir-probe/logs?stdout=true&stderr=true&tail=20×tamps=false" + ] + ); + let _ = std::fs::remove_file(socket_path); + } } diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 005f688a1..2850ae546 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -3,12 +3,15 @@ //! 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; #[cfg(test)] use openshell_core::gpu::{driver_gpu_requirements, validate_specific_gpu_device_request}; -use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate}; +use openshell_core::proto::compute::v1::{ + DriverSandbox, DriverSandboxTemplate, workspace_validation_identity, +}; use openshell_core::proto_struct::deserialize_optional_non_empty_string_list; use openshell_core::{driver_mounts, proto_struct}; use serde::Serialize; @@ -181,6 +184,57 @@ pub fn short_id(id: &str) -> String { // Typed container spec structs for the Podman libpod create API. // --------------------------------------------------------------------------- +/// Immutable image metadata normalized once for both probe and 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)?; + if let Some(volumes) = image_config.and_then(|config| config.volumes.as_ref()) { + for volume in volumes.keys() { + driver_mounts::validate_container_mount_target(volume).map_err(|error| { + ComputeDriverError::Precondition(format!( + "invalid image-declared volume '{volume}': {error}" + )) + })?; + driver_mounts::validate_workspace_mount_target(volume, &workspace_root).map_err( + |_| { + ComputeDriverError::Precondition(format!( + "image-declared volume '{volume}' masks OCI WorkingDir '{workspace_root}' before workspace validation" + )) + }, + )?; + driver_mounts::validate_mount_control_path(volume, &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, + }) + } +} + #[derive(Serialize)] struct ContainerSpec { name: String, @@ -190,6 +244,9 @@ struct ContainerSpec { volumes: Vec, image_volumes: Vec, hostname: String, + /// Start the supervisor independently of the image workspace. The + /// supervisor validates and prepares that workspace before child launch. + 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, @@ -231,6 +288,43 @@ struct ContainerSpec { portmappings: Vec, } +/// Minimal Podman spec for an OpenShell-owned auxiliary container. +/// +/// Keep this typed separately from [`ContainerSpec`] so probes cannot +/// accidentally inherit sandbox networking, secrets, or workspace mounts, +/// while still sharing resource-limit construction. +#[derive(Serialize)] +pub struct WorkspaceProbeSpec { + name: String, + image: String, + entrypoint: Vec, + command: Vec, + user: String, + work_dir: String, + image_volumes: Vec, + image_volume_mode: String, + netns: NetNS, + no_new_privileges: bool, + cap_drop: Vec, + cap_add: Vec, + image_pull_policy: String, + resource_limits: ResourceLimits, +} + +impl WorkspaceProbeSpec { + pub fn name(&self) -> &str { + &self.name + } + + pub fn image_id(&self) -> &str { + &self.image + } + + pub fn to_value(&self) -> Value { + serde_json::to_value(self).expect("WorkspaceProbeSpec serialization cannot fail") + } +} + /// A port mapping entry for the libpod `SpecGenerator`. #[derive(Serialize)] struct PortMapping { @@ -423,6 +517,7 @@ fn build_env( user_env.insert(k.clone(), v.clone()); } } + user_env.remove(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY); env.extend(user_env.clone()); if !user_env.is_empty() && let Ok(json) = serde_json::to_string(&user_env) @@ -483,6 +578,13 @@ fn build_env( env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + // The final spec overwrites this only after a successful immutable-image + // probe. An explicit empty value prevents image ENV from forging the + // attestation contract on the /sandbox compatibility path. + env.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.into(), + String::new(), + ); env.insert( openshell_core::sandbox_env::OCI_IMAGE_USER.into(), oci_user.to_string(), @@ -495,7 +597,6 @@ fn build_env( openshell_core::sandbox_env::SANDBOX_GID.into(), String::new(), ); - // 4. Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from a driver-owned bind mount. if let Some(s) = spec @@ -533,6 +634,10 @@ fn build_labels(sandbox: &DriverSandbox) -> BTreeMap { labels } +fn workspace_probe_name() -> String { + format!("openshell-workdir-probe-{}", uuid::Uuid::new_v4().simple()) +} + /// Parse resource limits from the sandbox template, falling back to defaults. fn build_resource_limits(sandbox: &DriverSandbox, config: &PodmanComputeConfig) -> ResourceLimits { let resources = sandbox @@ -612,6 +717,7 @@ pub fn podman_driver_image_mount_sources( fn podman_user_mounts( sandbox: &DriverSandbox, enable_bind_mounts: bool, + workspace_root: &str, ) -> Result { let template = sandbox .spec @@ -623,6 +729,13 @@ 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)?; match mount { PodmanDriverMountConfig::Bind { source, @@ -897,14 +1010,21 @@ 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, + None, ) } @@ -914,16 +1034,16 @@ 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, + workspace_identity: Option<&str>, ) -> 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 mut 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) + let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts, &image.workspace_root) .map_err(ComputeDriverError::InvalidArgument)?; if sandbox .spec @@ -952,7 +1072,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 +1083,19 @@ 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)); + if let Some(identity) = workspace_identity { + env.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.into(), + identity.into(), + ); + } let container_spec = ContainerSpec { name, - image: image_id.to_string(), + image: image.id.clone(), labels, env, volumes, @@ -982,6 +1106,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,10 +1114,9 @@ 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 - // corporate proxy flags follow it; the workload command comes from - // the reserved environment variable. + // Operator-owned corporate proxy flags. The workload command is not + // part of argv (the supervisor takes it from the reserved command + // env var), so these flags are the whole command list. command, // Force the supervisor to run as root (UID 0). Sandbox images may // set a non-root USER directive (e.g. `USER sandbox`), but the @@ -1188,6 +1312,82 @@ pub fn build_container_spec_for_image( Ok(serde_json::to_value(container_spec).expect("ContainerSpec serialization cannot fail")) } +/// Build a minimal one-shot container that validates the image's original +/// workdir before the final Podman workspace volume covers it. +pub fn build_workspace_probe_spec( + sandbox: &DriverSandbox, + config: &PodmanComputeConfig, + image: &ResolvedPodmanImage, +) -> Result, ComputeDriverError> { + if image.workspace_root == driver_mounts::DEFAULT_WORKSPACE_ROOT { + return Ok(None); + } + + let spec = sandbox + .spec + .as_ref() + .ok_or_else(|| ComputeDriverError::Precondition("sandbox.spec is required".into()))?; + let mut command = vec![ + "probe-workspace".to_string(), + "--workdir".to_string(), + image.workspace_root.clone(), + "--oci-user".to_string(), + image.oci_user.clone(), + ]; + let identity = spec.workspace_validation_identity.as_ref().ok_or_else(|| { + ComputeDriverError::Precondition( + "workspace_validation_identity is required for a non-default OCI workdir".into(), + ) + })?; + match identity.source.as_ref().ok_or_else(|| { + ComputeDriverError::Precondition("workspace_validation_identity.source is required".into()) + })? { + workspace_validation_identity::Source::Policy(policy) => { + for (flag, value) in [ + ("--run-as-user", policy.run_as_user.as_str()), + ("--run-as-group", policy.run_as_group.as_str()), + ] { + if !value.is_empty() { + command.push(flag.to_string()); + command.push(value.to_string()); + } + } + } + workspace_validation_identity::Source::Image(_) => { + command.push("--discover-policy-identity".to_string()); + } + } + + let probe_spec = WorkspaceProbeSpec { + // An attempt-unique name prevents an indeterminate create response + // from making a retry conflict with the first probe. + name: workspace_probe_name(), + image: image.id.clone(), + entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], + command, + user: "0:0".into(), + work_dir: "/".into(), + image_volumes: vec![ImageVolume { + source: config.supervisor_image.clone(), + destination: SUPERVISOR_MOUNT_DIR.into(), + rw: false, + }], + // Do not materialize OCI VOLUME declarations: the probe must inspect + // the immutable image layer rather than a fresh anonymous volume. + image_volume_mode: "ignore".into(), + netns: NetNS { + nsmode: "none".into(), + }, + no_new_privileges: true, + cap_drop: vec!["ALL".into()], + cap_add: vec!["SETUID".into(), "SETGID".into()], + image_pull_policy: "never".into(), + resource_limits: build_resource_limits(sandbox, config), + }; + + Ok(Some(probe_spec)) +} + fn hostadd_entries(config: &PodmanComputeConfig) -> Vec { let host_gateway_ip = config.host_gateway_ip.trim(); if host_gateway_ip.is_empty() { @@ -1260,7 +1460,10 @@ fn parse_memory_to_bytes(quantity: &str) -> Option { #[cfg(test)] mod tests { use super::*; - use openshell_core::proto::compute::v1::{GpuResourceRequirements, ResourceRequirements}; + use crate::client::ImageConfig; + use openshell_core::proto::compute::v1::{ + DriverSandboxSpec, GpuResourceRequirements, ResourceRequirements, + }; static ENV_LOCK: std::sync::LazyLock> = std::sync::LazyLock::new(|| std::sync::Mutex::new(())); @@ -1279,6 +1482,22 @@ 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(), + volumes: None, + }), + } + } + + 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)); @@ -1373,6 +1592,10 @@ mod tests { (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), (openshell_core::sandbox_env::SANDBOX_UID, "9999"), (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, + "9999:9999:", + ), ] { spec.environment.insert(key.to_string(), value.to_string()); } @@ -1383,8 +1606,8 @@ mod tests { None, None, "registry.example/app:latest", - "sha256:immutable", - "app:staff", + &resolved_image("sha256:immutable", "app:staff", "/workspace/project"), + Some("1000:1000:"), ) .unwrap(); @@ -1395,6 +1618,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") @@ -1408,11 +1643,260 @@ mod tests { Some("") ); assert_eq!( - container["command"], - serde_json::json!(["--workdir", "/sandbox"]) + container["env"][openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY].as_str(), + Some("1000:1000:") ); } + #[test] + fn compatibility_workspace_emits_empty_attestation() { + let container = build_container_spec_for_image( + &test_sandbox("test-id", "test-name"), + &test_config(), + None, + None, + "registry.example/app:latest", + &resolved_image("sha256:immutable", "app:staff", ""), + None, + ) + .unwrap(); + + assert_eq!( + container["env"][openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY].as_str(), + Some("") + ); + } + + #[test] + fn container_spec_rejects_invalid_oci_working_dir() { + let err = ResolvedPodmanImage::from_inspect( + &image_inspect("sha256:immutable", "app:staff", "relative/workspace"), + &test_config(), + ) + .unwrap_err(); + + assert!( + err.to_string() + .contains("must be an absolute container path") + ); + } + + #[test] + fn container_spec_rejects_openshell_control_path_working_dir() { + let err = ResolvedPodmanImage::from_inspect( + &image_inspect( + "sha256:immutable", + "app:staff", + "/opt/openshell/bin/project", + ), + &test_config(), + ) + .unwrap_err(); + + assert!(err.to_string().contains("OpenShell control path")); + } + + #[test] + fn container_spec_rejects_image_volume_masking_workdir() { + let mut image = image_inspect("sha256:immutable", "app:staff", "/home/app/project"); + image + .config + .as_mut() + .unwrap() + .volumes + .get_or_insert_default() + .insert("/home".into(), serde_json::json!({})); + + let error = ResolvedPodmanImage::from_inspect(&image, &test_config()).unwrap_err(); + assert!(error.to_string().contains("masks OCI WorkingDir")); + } + + #[test] + fn workspace_probe_uses_pinned_image_without_workspace_or_network() { + let mut sandbox = test_sandbox("test-id", "test-name"); + let spec = sandbox.spec.get_or_insert_default(); + spec.template.get_or_insert_default().resources = Some( + openshell_core::proto::compute::v1::DriverResourceRequirements { + cpu_limit: "750m".into(), + memory_limit: "1Gi".into(), + ..Default::default() + }, + ); + spec.workspace_validation_identity = Some( + openshell_core::proto::compute::v1::WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Policy( + openshell_core::proto::compute::v1::PolicyProcessIdentity { + run_as_user: "policy-user".into(), + run_as_group: "policy-group".into(), + }, + )), + }, + ); + let mut config = test_config(); + config.sandbox_pids_limit = 37; + let probe = build_workspace_probe_spec( + &sandbox, + &config, + &resolved_image("sha256:immutable", "app:staff", "/workspace/project"), + ) + .unwrap() + .unwrap() + .to_value(); + + assert_eq!(probe["image"], "sha256:immutable"); + assert_eq!(probe["netns"]["nsmode"], "none"); + assert_eq!(probe["image_volume_mode"], "ignore"); + assert_eq!(probe["cap_drop"], serde_json::json!(["ALL"])); + assert_eq!(probe["cap_add"], serde_json::json!(["SETUID", "SETGID"])); + assert!(probe.get("labels").is_none()); + assert_eq!(probe["resource_limits"]["cpu"]["quota"], 75_000); + assert_eq!( + probe["resource_limits"]["memory"]["limit"], + 1024 * 1024 * 1024_u64 + ); + assert_eq!(probe["resource_limits"]["PidsLimit"], 37); + assert!(probe.get("volumes").is_none()); + assert!(probe.get("secrets").is_none()); + assert!(probe.get("env").is_none()); + assert_eq!( + probe["command"], + serde_json::json!([ + "probe-workspace", + "--workdir", + "/workspace/project", + "--oci-user", + "app:staff", + "--run-as-user", + "policy-user", + "--run-as-group", + "policy-group" + ]) + ); + } + + #[test] + fn workspace_probe_names_are_attempt_unique() { + let first = workspace_probe_name(); + let second = workspace_probe_name(); + + assert!(first.starts_with("openshell-workdir-probe-")); + assert_ne!(first, second); + } + + #[test] + fn workspace_probe_discovers_image_policy_identity_when_requested() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox + .spec + .get_or_insert_default() + .workspace_validation_identity = Some( + openshell_core::proto::compute::v1::WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Image( + openshell_core::proto::compute::v1::ImagePolicyDiscovery {}, + )), + }, + ); + + let probe = build_workspace_probe_spec( + &sandbox, + &test_config(), + &resolved_image("sha256:immutable", "app:staff", "/home/app/project"), + ) + .unwrap() + .unwrap() + .to_value(); + + assert_eq!( + probe["command"], + serde_json::json!([ + "probe-workspace", + "--workdir", + "/home/app/project", + "--oci-user", + "app:staff", + "--discover-policy-identity" + ]) + ); + } + + #[test] + fn workspace_probe_requires_an_explicit_identity_source() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec::default()); + + let Err(error) = build_workspace_probe_spec( + &sandbox, + &test_config(), + &resolved_image("sha256:immutable", "app:staff", "/home/app/project"), + ) else { + panic!("non-default workdir should require an identity source"); + }; + + assert!(error.to_string().contains("workspace_validation_identity")); + } + + #[test] + fn workspace_probe_skips_sandbox_compatibility_fallback() { + let probe = build_workspace_probe_spec( + &test_sandbox("test-id", "test-name"), + &test_config(), + &resolved_image("sha256:immutable", "sandbox:sandbox", "/"), + ) + .unwrap(); + assert!(probe.is_none()); + } + + #[test] + fn container_spec_reserves_resolved_workspace_root_but_allows_nested_mounts() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate::default()), + ..Default::default() + }); + let set_tmpfs_target = |sandbox: &mut DriverSandbox, target: &str| { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{"type": "tmpfs", "target": target}] + }))); + }; + + for workspace in ["/workspace", "/workspace/project"] { + set_tmpfs_target(&mut sandbox, "/workspace"); + let err = build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &resolved_image("sha256:immutable", "app:staff", workspace), + None, + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("reserved for the OpenShell workspace") + ); + } + + set_tmpfs_target(&mut sandbox, "/workspace/cache"); + build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &resolved_image("sha256:immutable", "app:staff", "/workspace"), + None, + ) + .expect("nested workspace mounts remain supported"); + } + #[test] fn volume_name_uses_id() { assert_eq!( diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 51c689fb2..bcb27ce2a 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -32,6 +32,8 @@ use std::time::Duration; use tracing::{debug, info, warn}; use url::Url; +const WORKSPACE_PROBE_TIMEOUT: Duration = Duration::from_secs(30); + impl From for ComputeDriverError { fn from(value: PodmanApiError) -> Self { match value { @@ -54,6 +56,10 @@ pub struct PodmanComputeDriver { rootless: bool, /// Rootless network helper reported by Podman, such as `pasta`. rootless_network_cmd: String, + /// Serializes container image-volume attachment and removal. Rootless + /// Podman can otherwise detach a shared type=image mount while another + /// sandbox container is starting. + container_lifecycle_lock: Arc>, gpu_selector: Arc, gpu_inventory_refresh: Arc (CdiGpuInventory, bool) + Send + Sync>, } @@ -222,6 +228,118 @@ fn podman_gpu_selection_error(err: CdiGpuSelectionError) -> ComputeDriverError { ComputeDriverError::Precondition(err.to_string()) } +const WORKSPACE_IDENTITY_MARKER: &str = "OPENSHELL_WORKSPACE_IDENTITY="; + +fn parse_workspace_identity(logs: &str) -> Result { + logs.match_indices(WORKSPACE_IDENTITY_MARKER) + .filter_map(|(start, _)| { + logs[start + WORKSPACE_IDENTITY_MARKER.len()..] + .split(|character: char| character.is_control()) + .next() + }) + .find_map(|identity| serde_json::from_str::(identity).ok()) + .ok_or_else(|| { + ComputeDriverError::Precondition( + "OCI WorkingDir validation probe did not report its process identity".to_string(), + ) + }) +} + +fn sanitized_probe_diagnostic(logs: &str) -> String { + let sanitized = logs + .chars() + .filter(|character| !character.is_control() || matches!(character, '\n' | '\t')) + .collect::(); + let sanitized = sanitized.trim(); + if sanitized.is_empty() { + "no probe diagnostic was emitted".to_string() + } else { + sanitized.chars().take(2048).collect() + } +} + +async fn run_workspace_probe( + client: PodmanClient, + lifecycle_lock: Arc>, + stop_timeout_secs: u32, + probe: container::WorkspaceProbeSpec, + timeout: Duration, +) -> Result { + let probe_name = probe.name().to_string(); + let image_id = probe.image_id().to_string(); + let create_body = probe.to_value(); + // Set before awaiting create because a timeout or transport error can + // leave us unable to tell whether Podman created the container. + let mut cleanup_required = false; + let validation = tokio::time::timeout(timeout, async { + { + let _lifecycle_guard = lifecycle_lock.lock().await; + cleanup_required = true; + if let Err(error) = client.create_container(&create_body).await { + if matches!(error, PodmanApiError::Conflict(_)) { + // Do not remove a container that this attempt did not + // create, even though UUID name collisions are unlikely. + cleanup_required = false; + } + return Err(ComputeDriverError::from(error)); + } + client + .start_container(&probe_name) + .await + .map_err(ComputeDriverError::from)?; + } + + let exit_code = client + .wait_container(&probe_name) + .await + .map_err(ComputeDriverError::from)?; + let logs = client.container_logs(&probe_name).await; + if exit_code == 0 { + let logs = logs.map_err(ComputeDriverError::from)?; + parse_workspace_identity(&logs) + } else { + let diagnostic = logs.map_or_else( + |error| format!("unable to read probe diagnostic: {error}"), + |logs| sanitized_probe_diagnostic(&logs), + ); + Err(ComputeDriverError::Precondition(format!( + "OCI WorkingDir validation failed for image '{image_id}' (probe exited with code {exit_code}): {diagnostic}", + ))) + } + }) + .await + .unwrap_or_else(|_| { + Err(ComputeDriverError::Precondition(format!( + "OCI WorkingDir validation timed out for image '{image_id}' after {} seconds", + timeout.as_secs() + ))) + }); + + if !cleanup_required { + return validation; + } + + let cleanup = { + let _lifecycle_guard = lifecycle_lock.lock().await; + client + .remove_container(&probe_name, stop_timeout_secs) + .await + }; + if let Err(cleanup_error) = cleanup + && !matches!(cleanup_error, PodmanApiError::NotFound(_)) + { + if validation.is_ok() { + return Err(ComputeDriverError::from(cleanup_error)); + } + warn!( + probe = %probe_name, + %cleanup_error, + "Failed to remove workspace validation probe" + ); + } + validation +} + /// Resolve the socket to connect to: explicit configuration wins, otherwise /// fall back to `detect`. Returns an error if neither resolves. /// @@ -243,6 +361,33 @@ fn resolve_socket_path( } impl PodmanComputeDriver { + async fn validate_image_workspace( + &self, + sandbox: &DriverSandbox, + image: &container::ResolvedPodmanImage, + ) -> Result, ComputeDriverError> { + let Some(probe) = container::build_workspace_probe_spec(sandbox, &self.config, image)? + else { + return Ok(None); + }; + crate::client::validate_name(probe.name()) + .map_err(|error| ComputeDriverError::Precondition(error.to_string()))?; + + let task = tokio::spawn(run_workspace_probe( + self.client.clone(), + Arc::clone(&self.container_lifecycle_lock), + self.config.stop_timeout_secs, + probe, + WORKSPACE_PROBE_TIMEOUT, + )); + let identity = task.await.map_err(|error| { + ComputeDriverError::Message(format!( + "OCI WorkingDir validation probe task failed: {error}" + )) + })??; + Ok(Some(identity)) + } + /// Create a new driver, verifying the Podman socket is reachable. pub async fn new(mut config: PodmanComputeConfig) -> Result { const MAX_PING_RETRIES: u32 = 5; @@ -391,6 +536,7 @@ impl PodmanComputeDriver { network_gateway_ip, rootless, rootless_network_cmd, + container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -692,11 +838,11 @@ 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)?; + let workspace_identity = self + .validate_image_workspace(sandbox, &resolved_image) + .await?; for image in container::podman_driver_image_mount_sources(sandbox, self.config.enable_bind_mounts) .map_err(ComputeDriverError::Precondition)? @@ -761,8 +907,8 @@ impl PodmanComputeDriver { token_secret_name.as_deref(), gpu_devices.as_deref(), image, - &inspected_image.id, - image_user, + &resolved_image, + workspace_identity.as_deref(), ) { Ok(spec) => spec, Err(e) => { @@ -770,8 +916,31 @@ impl PodmanComputeDriver { return Err(e); } }; - match self.client.create_container(&spec).await { - Ok(_) => {} + // Podman implements the supervisor as a shared type=image mount. + // Keep attach/start atomic with respect to another sandbox's removal. + let lifecycle_result = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + match self.client.create_container(&spec).await { + Ok(_) => match self.client.start_container(&name).await { + Ok(()) => Ok(()), + Err(e) => { + warn!( + sandbox_name = %sandbox.name, + error = %e, + "Failed to start container; cleaning up" + ); + let _ = self + .client + .remove_container(&name, self.config.stop_timeout_secs) + .await; + Err(e) + } + }, + Err(e) => Err(e), + } + }; + match lifecycle_result { + Ok(()) => {} Err(PodmanApiError::Conflict(_)) => { // Clean up the volume we just created. It is keyed by *this* // sandbox's ID, not the conflicting container's ID (which @@ -786,21 +955,6 @@ impl PodmanComputeDriver { } } - // 5. Start container. - if let Err(e) = self.client.start_container(&name).await { - warn!( - sandbox_name = %sandbox.name, - error = %e, - "Failed to start container; cleaning up" - ); - let _ = self - .client - .remove_container(&name, self.config.stop_timeout_secs) - .await; - cleanup_created().await; - return Err(ComputeDriverError::from(e)); - } - info!( sandbox_id = %sandbox.id, sandbox_name = %sandbox.name, @@ -862,17 +1016,20 @@ impl PodmanComputeDriver { }; info!(sandbox_id = %sandbox_id, container = %container_id, "Deleting sandbox container"); - // Keep stop, timeout, and removal in one Podman operation. Splitting - // stop and remove can race with another container starting an image - // mount when the stop reaches its timeout. - let container_existed = match self - .client - .remove_container(&container_id, self.config.stop_timeout_secs) - .await - { - Ok(()) => true, - Err(PodmanApiError::NotFound(_)) => false, - Err(e) => return Err(ComputeDriverError::from(e)), + // Keep stop, timeout, and removal in one Podman operation. Removing a + // container detaches its type=image mounts, so do not overlap it with + // another sandbox container's create/start window. + let container_existed = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + match self + .client + .remove_container(&container_id, self.config.stop_timeout_secs) + .await + { + Ok(()) => true, + Err(PodmanApiError::NotFound(_)) => false, + Err(e) => return Err(ComputeDriverError::from(e)), + } }; // Remove workspace volume. @@ -1009,6 +1166,7 @@ impl PodmanComputeDriver { network_gateway_ip: None, rootless: false, rootless_network_cmd: String::new(), + container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -1098,7 +1256,8 @@ mod tests { use crate::test_utils::{StubResponse, spawn_podman_stub}; use hyper::StatusCode; use openshell_core::proto::compute::v1::{ - DriverSandboxSpec, DriverSandboxTemplate, ResourceRequirements, + DriverSandboxSpec, DriverSandboxTemplate, ImagePolicyDiscovery, ResourceRequirements, + WorkspaceValidationIdentity, workspace_validation_identity, }; use std::collections::HashMap; use std::fs; @@ -1494,11 +1653,11 @@ mod tests { assert!(driver.network_gateway_ip().is_none()); assert!(driver.gateway_listener_requirements().unwrap().is_empty()); handle.await.expect("stub task should finish"); + let requests = request_log + .lock() + .expect("request log lock should not be poisoned"); assert_eq!( - request_log - .lock() - .expect("request log lock should not be poisoned") - .as_slice(), + requests.as_slice(), [ "GET /_ping".to_string(), format!("GET {}", api_path("/libpod/info")), @@ -2058,6 +2217,228 @@ mod tests { } } + fn workspace_probe_sandbox(id: &str) -> DriverSandbox { + let mut sandbox = plain_sandbox(id, "demo"); + sandbox.spec = Some(DriverSandboxSpec { + workspace_validation_identity: Some(WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Image( + ImagePolicyDiscovery {}, + )), + }), + ..Default::default() + }); + sandbox + } + + fn workspace_probe_image(config: &PodmanComputeConfig) -> container::ResolvedPodmanImage { + container::ResolvedPodmanImage::from_inspect( + &crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + volumes: None, + }), + }, + config, + ) + .unwrap() + } + + #[tokio::test] + async fn workspace_probe_waits_for_success_and_always_removes_container() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-success", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#), + StubResponse::new( + StatusCode::OK, + "OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe"); + let image = workspace_probe_image(&driver.config); + + let identity = driver + .validate_image_workspace(&sandbox, &image) + .await + .expect("successful probe should pass"); + assert_eq!(identity.as_deref(), Some("1234:1235:")); + + handle.await.expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert_eq!(requests.len(), 5); + assert!(requests[0].contains("/libpod/containers/create")); + assert!(requests[1].contains("/start")); + assert!(requests[2].contains("/wait?condition=exited")); + assert!(requests[3].contains("/logs?")); + assert!(requests[4].starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_removes_container_after_validation_failure() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-failure", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":1}"#), + StubResponse::new( + StatusCode::OK, + "image workspace path component '/workspace' is not writable and traversable\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe-fail"); + let image = workspace_probe_image(&driver.config); + + let error = driver + .validate_image_workspace(&sandbox, &image) + .await + .unwrap_err(); + assert!(error.to_string().contains("exited with code 1")); + assert!(error.to_string().contains("not writable and traversable")); + + handle.await.expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_cleanup_survives_caller_cancellation() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-cancelled-caller", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#) + .with_delay(Duration::from_millis(100)), + StubResponse::new( + StatusCode::OK, + "OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe-cancel"); + let image = workspace_probe_image(&driver.config); + + let caller = + tokio::spawn(async move { driver.validate_image_workspace(&sandbox, &image).await }); + tokio::time::timeout(Duration::from_secs(2), async { + loop { + if request_log + .lock() + .unwrap() + .iter() + .any(|request| request.contains("/wait?condition=exited")) + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("probe should reach wait before cancellation"); + caller.abort(); + let _ = caller.await; + + tokio::time::timeout(Duration::from_secs(2), handle) + .await + .expect("owned probe task should finish after caller cancellation") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_timeout_forces_container_cleanup() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-timeout", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#) + .with_delay(Duration::from_millis(100)), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe-timeout"); + let image = workspace_probe_image(&driver.config); + let probe = container::build_workspace_probe_spec(&sandbox, &driver.config, &image) + .unwrap() + .unwrap(); + + let error = run_workspace_probe( + driver.client.clone(), + Arc::clone(&driver.container_lifecycle_lock), + driver.config.stop_timeout_secs, + probe, + Duration::from_millis(25), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("timed out")); + + tokio::time::timeout(Duration::from_secs(10), handle) + .await + .expect("timed-out probe should be force-removed") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_create_timeout_still_attempts_cleanup() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-create-timeout", + vec![ + StubResponse::new(StatusCode::CREATED, "{}").with_delay(Duration::from_millis(100)), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe-create-timeout"); + let image = workspace_probe_image(&driver.config); + let probe = container::build_workspace_probe_spec(&sandbox, &driver.config, &image) + .unwrap() + .unwrap(); + + let error = run_workspace_probe( + driver.client.clone(), + Arc::clone(&driver.container_lifecycle_lock), + driver.config.stop_timeout_secs, + probe, + Duration::from_millis(25), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("timed out")); + + tokio::time::timeout(Duration::from_secs(2), handle) + .await + .expect("ambiguous create should be reconciled") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert!(requests[0].contains("/libpod/containers/create")); + assert!(requests[1].starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + fn secret_delete_request(sandbox_id: &str) -> String { format!( "DELETE {}", @@ -2245,4 +2626,29 @@ mod tests { ); let _ = fs::remove_file(socket_path); } + + #[test] + fn workspace_identity_parser_accepts_plain_and_multiplexed_logs() { + assert_eq!( + parse_workspace_identity( + "probe startup\nOPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:7,8\"\n" + ) + .unwrap(), + "1234:1235:7,8" + ); + assert_eq!( + parse_workspace_identity( + "\u{1}\0\0\0\0\0\0.OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n" + ) + .unwrap(), + "1234:1235:" + ); + assert!( + parse_workspace_identity( + "OPENSHELL_WORKSPACE_IDENTITY=not-json\n\ + OPENSHELL_WORKSPACE_IDENTITY={\"uid\":1234}\n" + ) + .is_err() + ); + } } diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 956fed927..80377ee60 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()?; @@ -202,18 +202,47 @@ pub async fn run_sandbox( &mut policy, &driver_identity, )?; + let workspace_attestation = if matches!( + &driver_identity, + openshell_supervisor_process::identity::DriverIdentity::OciUser { .. } + ) { + match std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) { + Some(value) if !value.is_empty() => Some(value.into_string().map_err(|_| { + miette::miette!("Podman workspace identity attestation is not valid UTF-8") + })?), + _ => None, + } + } else { + None + }; + if let Some(expected) = workspace_attestation.as_deref() { + let actual = + openshell_supervisor_process::process::resolved_workspace_identity_attestation( + &policy, resolved, + )?; + if expected != actual { + return Err(miette::miette!( + "process identity changed after OCI workspace validation" + )); + } + } ( resolved, openshell_supervisor_process::process::ResolvedWorkspace::new( workdir.clone(), use_workdir_as_home, + workspace_attestation.is_some(), ), ) }; #[cfg(not(unix))] let (resolved_process_identity, workspace) = ( openshell_supervisor_process::process::ResolvedProcessIdentity::default(), - openshell_supervisor_process::process::ResolvedWorkspace::new(workdir.clone(), false), + openshell_supervisor_process::process::ResolvedWorkspace::new( + workdir.clone(), + false, + false, + ), ); #[cfg_attr(not(target_os = "linux"), allow(unused_mut))] @@ -2156,6 +2185,18 @@ fn discover_policy_from_disk_or_default() -> openshell_core::proto::SandboxPolic discover_policy_from_path(primary) } +/// Discover only the process identity portion of the immutable image policy. +/// +/// The Podman workspace probe uses this when no policy existed at create time, +/// matching the final supervisor's disk-policy discovery before the gateway +/// backfills that policy. +pub fn discover_process_policy_from_disk_or_default() -> openshell_core::policy::ProcessPolicy { + discover_policy_from_disk_or_default() + .process + .map(Into::into) + .unwrap_or_default() +} + /// Try to read a sandbox policy YAML from `path`, falling back to the /// hardcoded restrictive default if the file is missing or invalid. fn discover_policy_from_path(path: &std::path::Path) -> openshell_core::proto::SandboxPolicy { diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 98af7f9ea..8297c8145 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -33,6 +33,7 @@ const COPY_SELF_SUBCOMMAND: &str = "copy-self"; /// to confirm the cross-sandbox IDOR guard fires. const DEBUG_RPC_SUBCOMMAND: &str = "debug-rpc"; const VALIDATE_WORKSPACE_SUBCOMMAND: &str = "validate-workspace"; +const PROBE_WORKSPACE_SUBCOMMAND: &str = "probe-workspace"; /// Default `--mode` value: run both supervisor leaves in a single binary. const DEFAULT_MODE: &str = "network,process"; @@ -272,7 +273,76 @@ fn validate_workspace(args: &[String]) -> Result<()> { #[cfg(not(target_os = "linux"))] fn validate_workspace(_args: &[String]) -> Result<()> { Err(miette::miette!( - "workspace validation is only supported on Unix" + "workspace validation is only supported on Linux" + )) +} + +/// Internal one-shot command used by Podman to validate the immutable image +/// before the final workspace volume covers its OCI workdir. +#[derive(Parser, Debug)] +#[command(name = "probe-workspace", hide = true)] +struct ProbeWorkspaceArgs { + #[arg(long)] + workdir: String, + #[arg(long)] + oci_user: String, + #[arg(long)] + run_as_user: Option, + #[arg(long)] + run_as_group: Option, + #[arg(long)] + discover_policy_identity: bool, +} + +#[cfg(unix)] +fn probe_workspace(args: &[String]) -> Result<()> { + use openshell_core::policy::{ + FilesystemPolicy, LandlockPolicy, NetworkPolicy, ProcessPolicy, SandboxPolicy, + }; + use openshell_supervisor_process::identity::{DriverIdentity, resolve_process_identity}; + + let args = ProbeWorkspaceArgs::try_parse_from( + std::iter::once(PROBE_WORKSPACE_SUBCOMMAND.to_string()).chain(args.iter().cloned()), + ) + .into_diagnostic()?; + let process = if args.discover_policy_identity { + openshell_sandbox::discover_process_policy_from_disk_or_default() + } else { + ProcessPolicy { + run_as_user: args.run_as_user, + run_as_group: args.run_as_group, + } + }; + let mut policy = SandboxPolicy { + version: 0, + filesystem: FilesystemPolicy::default(), + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process, + }; + let resolved_identity = resolve_process_identity( + &mut policy, + &DriverIdentity::OciUser { + declaration: args.oci_user, + }, + )?; + let identity = + openshell_supervisor_process::process::validate_oci_workspace_as_process_identity( + &policy, + resolved_identity, + Path::new(&args.workdir), + )?; + println!( + "OPENSHELL_WORKSPACE_IDENTITY={}", + serde_json::to_string(&identity).into_diagnostic()? + ); + Ok(()) +} + +#[cfg(not(unix))] +fn probe_workspace(_args: &[String]) -> Result<()> { + Err(miette::miette!( + "workspace probing is only supported on Unix" )) } @@ -527,6 +597,9 @@ fn main() -> Result<()> { if raw_args.get(1).map(String::as_str) == Some(VALIDATE_WORKSPACE_SUBCOMMAND) { return validate_workspace(&raw_args[2..]); } + if raw_args.get(1).map(String::as_str) == Some(PROBE_WORKSPACE_SUBCOMMAND) { + return probe_workspace(&raw_args[2..]); + } let args = Args::parse(); @@ -721,6 +794,36 @@ mod tests { validate_workspace(&args).expect("current identity should retain workspace authority"); } + #[cfg(unix)] + #[test] + fn workspace_probe_subcommand_resolves_final_policy_identity() { + let uid = nix::unistd::geteuid().as_raw(); + let gid = nix::unistd::getegid().as_raw(); + if uid < 1000 || gid < 1000 { + // Process policies intentionally reject system identities. Local + // macOS users commonly have IDs below 1000, so this test cannot + // exercise the credential transition for those accounts. + 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(0o777)).unwrap(); + let args = vec![ + "--workdir".to_string(), + root.display().to_string(), + "--oci-user".to_string(), + "unused".to_string(), + "--run-as-user".to_string(), + uid.to_string(), + "--run-as-group".to_string(), + gid.to_string(), + ]; + + probe_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<()> { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index a1c33e49f..5d7ec2473 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -33,9 +33,9 @@ use openshell_core::proto::compute::v1::{ GetGatewayListenerRequirementsResponse, GetSandboxRequest, GpuResourceRequirements as DriverGpuResourceRequirements, ListSandboxesRequest, ResourceRequirements as DriverSandboxResourceRequirements, ValidateSandboxCreateRequest, - WatchSandboxesEvent, WatchSandboxesRequest, compute_driver_client::ComputeDriverClient, - compute_driver_server::ComputeDriver, gateway_listener_requirement::Selector, - watch_sandboxes_event, + WatchSandboxesEvent, WatchSandboxesRequest, WorkspaceValidationIdentity, + compute_driver_client::ComputeDriverClient, compute_driver_server::ComputeDriver, + gateway_listener_requirement::Selector, watch_sandboxes_event, }; use openshell_core::proto::{ PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxSpec, SandboxStatus, @@ -840,9 +840,17 @@ impl ComputeRuntime { &self.gateway_listener_requirements } - pub async fn validate_sandbox_create(&self, sandbox: &Sandbox) -> Result<(), Status> { - let driver_sandbox = driver_sandbox_from_public(sandbox, &self.driver_info.name) - .map_err(|status| *status)?; + pub async fn validate_sandbox_create( + &self, + sandbox: &Sandbox, + workspace_validation_identity: Option, + ) -> Result<(), Status> { + let driver_sandbox = driver_sandbox_from_public( + sandbox, + &self.driver_info.name, + workspace_validation_identity, + ) + .map_err(|status| *status)?; self.driver .call( "driver.validate_sandbox_create", @@ -863,10 +871,15 @@ impl ComputeRuntime { &self, sandbox: Sandbox, sandbox_token: Option, + workspace_validation_identity: Option, ) -> Result { let sandbox_id = sandbox.object_id().to_string(); - let mut driver_sandbox = driver_sandbox_from_public(&sandbox, &self.driver_info.name) - .map_err(|status| *status)?; + let mut driver_sandbox = driver_sandbox_from_public( + &sandbox, + &self.driver_info.name, + workspace_validation_identity, + ) + .map_err(|status| *status)?; // Create with MustCreate condition to prevent duplicate creation race self.sandbox_index.update_from_sandbox(&sandbox); @@ -2496,6 +2509,7 @@ pub async fn connect_remote_compute_driver( fn driver_sandbox_from_public( sandbox: &Sandbox, driver_name: &str, + workspace_validation_identity: Option, ) -> Result> { Ok(DriverSandbox { id: sandbox.object_id().to_string(), @@ -2504,7 +2518,9 @@ fn driver_sandbox_from_public( spec: sandbox .spec .as_ref() - .map(|spec| driver_sandbox_spec_from_public(spec, driver_name)) + .map(|spec| { + driver_sandbox_spec_from_public(spec, driver_name, workspace_validation_identity) + }) .transpose()?, status: sandbox.status.as_ref().map(driver_status_from_public), workspace: sandbox.object_workspace().to_string(), @@ -2514,6 +2530,7 @@ fn driver_sandbox_from_public( fn driver_sandbox_spec_from_public( spec: &SandboxSpec, driver_name: &str, + workspace_validation_identity: Option, ) -> Result> { Ok(DriverSandboxSpec { log_level: spec.log_level.clone(), @@ -2532,6 +2549,11 @@ fn driver_sandbox_spec_from_public( } }), sandbox_token: String::new(), + workspace_validation_identity: if driver_name == "podman" { + workspace_validation_identity + } else { + None + }, }) } @@ -3272,7 +3294,7 @@ mod tests { ..Default::default() }; - let driver = driver_sandbox_spec_from_public(&public, "test-driver") + let driver = driver_sandbox_spec_from_public(&public, "test-driver", None) .expect("driver spec should map"); let gpu = driver @@ -3283,6 +3305,30 @@ mod tests { assert_eq!(gpu.count, Some(2)); } + #[test] + fn driver_sandbox_spec_forwards_workspace_identity_only_to_podman() { + use openshell_core::proto::compute::v1::{ + PolicyProcessIdentity, workspace_validation_identity, + }; + + let public = SandboxSpec::default(); + let identity = WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Policy( + PolicyProcessIdentity { + run_as_user: "app".into(), + run_as_group: "staff".into(), + }, + )), + }; + + let podman = driver_sandbox_spec_from_public(&public, "podman", Some(identity.clone())) + .expect("driver spec should map"); + assert_eq!(podman.workspace_validation_identity, Some(identity.clone())); + + let docker = driver_sandbox_spec_from_public(&public, "docker", Some(identity)).unwrap(); + assert!(docker.workspace_validation_identity.is_none()); + } + #[test] fn select_driver_config_forwards_only_matching_driver_block() { let config = prost_types::Struct { @@ -4319,7 +4365,7 @@ mod tests { let traced = test_exporter::install_traced(); async { runtime - .create_sandbox(sandbox, None) + .create_sandbox(sandbox, None, None) .await .expect("create succeeds"); } @@ -4443,7 +4489,7 @@ mod tests { let traced = test_exporter::install_traced(); async { runtime - .create_sandbox(sandbox, None) + .create_sandbox(sandbox, None, None) .await .expect_err("driver refuses the create"); } @@ -4658,7 +4704,7 @@ mod tests { ..Default::default() }); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); runtime .apply_sandbox_update(ready_driver_sandbox("sb-1", "sandbox-a")) .await @@ -6958,8 +7004,11 @@ mod tests { ..Default::default() }); - runtime.validate_sandbox_create(&sandbox).await.unwrap(); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime + .validate_sandbox_create(&sandbox, None) + .await + .unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); assert!( runtime .delete_sandbox("default", "uds-sandbox") @@ -7068,7 +7117,7 @@ mod tests { deletion_timestamp_ms: 0, }); - let created = runtime.create_sandbox(sandbox, None).await.unwrap(); + let created = runtime.create_sandbox(sandbox, None, None).await.unwrap(); assert_eq!( created.metadata.as_ref().unwrap().resource_version, @@ -7103,7 +7152,7 @@ mod tests { .labels .insert("env".to_string(), "prod".to_string()); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); let matching = runtime .store @@ -7127,11 +7176,13 @@ mod tests { // Spawn two concurrent creation attempts for the same sandbox let runtime1 = runtime.clone(); let sandbox1 = sandbox.clone(); - let handle1 = tokio::spawn(async move { runtime1.create_sandbox(sandbox1, None).await }); + let handle1 = + tokio::spawn(async move { runtime1.create_sandbox(sandbox1, None, None).await }); let runtime2 = runtime.clone(); let sandbox2 = sandbox.clone(); - let handle2 = tokio::spawn(async move { runtime2.create_sandbox(sandbox2, None).await }); + let handle2 = + tokio::spawn(async move { runtime2.create_sandbox(sandbox2, None, None).await }); // Wait for both to complete let result1 = handle1.await.unwrap(); @@ -7189,7 +7240,7 @@ mod tests { }), ..Default::default() }; - let driver_sb = driver_sandbox_from_public(&sandbox, "kubernetes").unwrap(); + let driver_sb = driver_sandbox_from_public(&sandbox, "kubernetes", None).unwrap(); assert_eq!(driver_sb.workspace, "alpha"); assert_eq!(driver_sb.name, "work"); assert_eq!(driver_sb.id, "sb-1"); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index e3a8c2b0d..b70206cbd 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -23,6 +23,10 @@ use crate::provider_profile_sources::EffectiveProviderProfileCatalog; #[cfg(test)] use crate::provider_profile_sources::ProviderProfileSources; use openshell_core::net::{is_always_blocked_ip, is_internal_ip}; +use openshell_core::proto::compute::v1::{ + ImagePolicyDiscovery, PolicyProcessIdentity, WorkspaceValidationIdentity, + workspace_validation_identity, +}; use openshell_core::proto::policy_merge_operation; use openshell_core::proto::setting_value; use openshell_core::proto::{ @@ -4827,6 +4831,37 @@ fn decode_policy_from_global_settings( Ok(Some(policy)) } +fn workspace_validation_identity_from_policy( + policy: Option<&ProtoSandboxPolicy>, +) -> WorkspaceValidationIdentity { + let source = policy.map_or( + workspace_validation_identity::Source::Image(ImagePolicyDiscovery {}), + |policy| { + let process = policy.process.as_ref(); + workspace_validation_identity::Source::Policy(PolicyProcessIdentity { + run_as_user: process.map_or_else(String::new, |p| p.run_as_user.clone()), + run_as_group: process.map_or_else(String::new, |p| p.run_as_group.clone()), + }) + }, + ); + WorkspaceValidationIdentity { + source: Some(source), + } +} + +/// Resolve the process-identity source used by a new Podman workspace probe. +/// Global policy has the same precedence here as it does in `GetSandboxConfig`. +pub(super) async fn workspace_validation_identity_for_create( + state: &ServerState, + sandbox_policy: Option<&ProtoSandboxPolicy>, +) -> Result { + let global_settings = load_global_settings(state.store.as_ref()).await?; + let global_policy = decode_policy_from_global_settings(&global_settings)?; + Ok(workspace_validation_identity_from_policy( + global_policy.as_ref().or(sandbox_policy), + )) +} + fn merge_effective_settings( global: &StoredSettings, sandbox: &StoredSettings, @@ -11811,6 +11846,56 @@ mod tests { assert_eq!(decoded.version, 7); } + #[test] + fn workspace_validation_uses_image_identity_without_an_effective_policy() { + let identity = workspace_validation_identity_from_policy(None); + assert!(matches!( + identity.source, + Some(workspace_validation_identity::Source::Image(_)) + )); + } + + #[tokio::test] + async fn workspace_validation_uses_global_policy_identity() { + let state = test_server_state().await; + let global_policy = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "2000".into(), + run_as_group: "2000".into(), + }), + ..Default::default() + }; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + POLICY_SETTING_KEY.to_string(), + StoredSettingValue::Bytes(hex::encode(global_policy.encode_to_vec())), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + let sandbox_policy = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "1000".into(), + run_as_group: "1000".into(), + }), + ..Default::default() + }; + let identity = workspace_validation_identity_for_create(&state, Some(&sandbox_policy)) + .await + .unwrap(); + + let Some(workspace_validation_identity::Source::Policy(process)) = identity.source else { + panic!("global policy should supply the workspace-validation identity"); + }; + assert_eq!(process.run_as_user, "2000"); + assert_eq!(process.run_as_group, "2000"); + } + #[test] fn config_revision_changes_when_effective_setting_changes() { let policy = ProtoSandboxPolicy::default(); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 4925c9eae..5c4ce1d48 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -31,7 +31,7 @@ use openshell_core::telemetry::{ LifecycleOperation, LifecycleResource, SandboxTemplateSource, TelemetryComputeDriver, TelemetryOutcome, }; -use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; +use openshell_core::{ComputeDriverKind, ObjectId, ObjectName, ObjectWorkspace}; use prost::Message; use std::collections::HashMap; use std::net::IpAddr; @@ -204,9 +204,7 @@ fn emit_sandbox_create_telemetry( ); } -fn telemetry_compute_driver( - driver_kind: Option, -) -> TelemetryComputeDriver { +fn telemetry_compute_driver(driver_kind: Option) -> TelemetryComputeDriver { TelemetryComputeDriver::from_driver_kind(driver_kind) } @@ -276,6 +274,19 @@ async fn handle_create_sandbox_inner( crate::middleware::validate_policy(state.middleware_registry.as_ref(), policy).await?; } + let workspace_validation_identity = + if matches!(state.compute.driver_kind(), Some(ComputeDriverKind::Podman)) { + Some( + super::policy::workspace_validation_identity_for_create( + state, + spec.policy.as_ref(), + ) + .await?, + ) + } else { + None + }; + let id = uuid::Uuid::new_v4().to_string(); let name = if request.name.is_empty() { generate_routable_name() @@ -306,7 +317,7 @@ async fn handle_create_sandbox_inner( state .compute - .validate_sandbox_create(&sandbox) + .validate_sandbox_create(&sandbox, workspace_validation_identity.clone()) .await .map_err(|status| { warn!(error = %status, "Rejecting sandbox create request"); @@ -333,7 +344,10 @@ async fn handle_create_sandbox_inner( None => None, }; - let sandbox = state.compute.create_sandbox(sandbox, sandbox_token).await?; + let sandbox = state + .compute + .create_sandbox(sandbox, sandbox_token, workspace_validation_identity) + .await?; info!( sandbox_id = %id, @@ -2292,19 +2306,19 @@ mod tests { #[test] fn telemetry_compute_driver_uses_resolved_driver_kind() { assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Docker)), + telemetry_compute_driver(Some(ComputeDriverKind::Docker)), TelemetryComputeDriver::Docker ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Kubernetes)), + telemetry_compute_driver(Some(ComputeDriverKind::Kubernetes)), TelemetryComputeDriver::Kubernetes ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Podman)), + telemetry_compute_driver(Some(ComputeDriverKind::Podman)), TelemetryComputeDriver::Podman ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Vm)), + telemetry_compute_driver(Some(ComputeDriverKind::Vm)), TelemetryComputeDriver::Vm ); assert_eq!( @@ -3195,8 +3209,7 @@ mod tests { #[tokio::test] async fn create_and_get_preserve_partial_process_identity() { - let state = - test_server_state_with_driver(openshell_core::ComputeDriverKind::Docker.as_str()).await; + let state = test_server_state_with_driver(ComputeDriverKind::Docker.as_str()).await; let policy = openshell_core::proto::SandboxPolicy { version: 1, process: Some(openshell_core::proto::ProcessPolicy { @@ -3259,9 +3272,7 @@ mod tests { #[tokio::test] async fn create_and_get_restore_legacy_identity_defaults_for_non_local_driver() { - let state = - test_server_state_with_driver(openshell_core::ComputeDriverKind::Kubernetes.as_str()) - .await; + let state = test_server_state_with_driver(ComputeDriverKind::Kubernetes.as_str()).await; let policy = openshell_core::proto::SandboxPolicy { version: 1, process: Some(openshell_core::proto::ProcessPolicy { diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 659fe3dc0..90d70bd82 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -86,12 +86,17 @@ impl ResolvedProcessIdentity { pub struct ResolvedWorkspace { root: Option, use_as_home: bool, + prevalidated: bool, } impl ResolvedWorkspace { #[must_use] - pub fn new(root: Option, use_as_home: bool) -> Self { - Self { root, use_as_home } + pub fn new(root: Option, use_as_home: bool, prevalidated: bool) -> Self { + Self { + root, + use_as_home, + prevalidated, + } } #[must_use] @@ -108,6 +113,11 @@ impl ResolvedWorkspace { pub fn home(&self) -> Option<&str> { self.use_as_home.then(|| self.root()).flatten() } + + #[must_use] + pub const fn prevalidated(&self) -> bool { + self.prevalidated + } } impl ProcessEnforcementMode { @@ -142,6 +152,7 @@ pub(crate) fn prepare_child_sandbox( const SUPERVISOR_ONLY_ENV_VARS: &[&str] = &[ openshell_core::sandbox_env::OCI_IMAGE_USER, + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, openshell_core::sandbox_env::SANDBOX_UID, openshell_core::sandbox_env::SANDBOX_GID, openshell_core::sandbox_env::SANDBOX_TOKEN, @@ -1394,6 +1405,101 @@ fn validate_oci_workspace_in_subprocess( )) } +/// Drop a one-shot workspace probe to the completed sandbox identity, validate +/// using the kernel's real path-access checks, and return a normalized identity +/// attestation for the final supervisor. +#[cfg(unix)] +pub fn validate_oci_workspace_as_process_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, + workdir: &Path, +) -> Result { + validate_sandbox_user_with_identity(policy, resolved_identity)?; + validate_sandbox_group_with_identity(policy, resolved_identity)?; + let (uid, gid, mut supplementary_gids) = + resolve_filesystem_identity(policy, resolved_identity)?; + let uid = uid.ok_or_else(|| miette::miette!("workspace probe UID is unresolved"))?; + let gid = gid.ok_or_else(|| miette::miette!("workspace probe GID is unresolved"))?; + supplementary_gids.sort_unstable_by_key(|group| group.as_raw()); + supplementary_gids.dedup(); + + if nix::unistd::geteuid().is_root() { + #[cfg(not(any( + target_os = "macos", + target_os = "ios", + target_os = "haiku", + target_os = "redox" + )))] + nix::unistd::setgroups(&supplementary_gids).into_diagnostic()?; + nix::unistd::setgid(gid).into_diagnostic()?; + nix::unistd::setuid(uid).into_diagnostic()?; + } + + let effective_identity = (nix::unistd::geteuid(), nix::unistd::getegid()); + if effective_identity != (uid, gid) { + return Err(miette::miette!( + "workspace probe privilege drop failed: expected {uid}:{gid}, got {}:{}", + effective_identity.0, + effective_identity.1 + )); + } + #[cfg(target_os = "linux")] + validate_oci_workspace_as_effective_identity(workdir)?; + #[cfg(not(target_os = "linux"))] + validate_oci_workspace( + workdir, + Some(effective_identity.0), + Some(effective_identity.1), + &supplementary_gids, + )?; + Ok(workspace_identity_attestation( + Some(uid), + Some(gid), + &supplementary_gids, + )) +} + +/// Normalize a completed process identity for comparison with the immutable +/// image probe. +#[cfg(unix)] +pub fn workspace_identity_attestation( + uid: Option, + gid: Option, + supplementary_gids: &[Gid], +) -> String { + let mut groups = supplementary_gids + .iter() + .map(|group| group.as_raw()) + .collect::>(); + groups.sort_unstable(); + groups.dedup(); + format!( + "{}:{}:{}", + uid.map_or_else(String::new, |value| value.as_raw().to_string()), + gid.map_or_else(String::new, |value| value.as_raw().to_string()), + groups + .iter() + .map(u32::to_string) + .collect::>() + .join(",") + ) +} + +/// Resolve and normalize the completed policy/driver identity without changing +/// process credentials. +#[cfg(unix)] +pub fn resolved_workspace_identity_attestation( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result { + let (uid, gid, supplementary_gids) = resolve_filesystem_identity(policy, resolved_identity)?; + Ok(workspace_identity_attestation( + uid, + gid, + &supplementary_gids, + )) +} + #[cfg(unix)] fn validate_workspace_component( path: &Path, @@ -1427,6 +1533,7 @@ fn validate_workspace_component( path.display() )); } + reject_special_workspace_filesystem(path)?; let required = if is_workspace { 0o3 } else { 0o1 }; if !identity_has_permissions(&metadata, uid, gid, supplementary_gids, required) { let requirement = if is_workspace { @@ -1450,6 +1557,7 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { 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()?; + reject_special_workspace_filesystem_fd(¤t_fd, ¤t_path)?; rustix::fs::accessat( ¤t_fd, ".", @@ -1464,6 +1572,7 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { })?; let last_component = components.len().saturating_sub(1); + for (index, component) in components.into_iter().enumerate() { current_path.push(&component); let stat = rustix::fs::statat(¤t_fd, &component, AtFlags::SYMLINK_NOFOLLOW).map_err( @@ -1509,22 +1618,71 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { ) })?; - let next_fd = rustix::fs::openat(¤t_fd, &component, open_flags, Mode::empty()) + current_fd = rustix::fs::openat(¤t_fd, &component, open_flags, Mode::empty()) .map_err(|error| { miette::miette!( "failed to open image workspace path component '{}': {error}", current_path.display() ) })?; + reject_special_workspace_filesystem_fd(¤t_fd, ¤t_path)?; if is_workspace { - validate_effective_workspace_write(&next_fd, ¤t_path)?; + validate_effective_workspace_write(¤t_fd, ¤t_path)?; } - current_fd = next_fd; } Ok(()) } +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem_fd(fd: &impl std::os::fd::AsFd, path: &Path) -> Result<()> { + let fs = rustix::fs::fstatfs(fd).into_diagnostic()?; + #[allow(clippy::cast_sign_loss)] + reject_special_workspace_filesystem_type(path, fs.f_type as u64) +} + +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem(path: &Path) -> Result<()> { + // Linux filesystem magic values for virtual/kernel-managed filesystems. + // The decision is based on the mounted filesystem, not its conventional + // distro path, so renamed or unusually mounted control filesystems remain + // protected without rejecting ordinary application directories. + let fs = rustix::fs::statfs(path).into_diagnostic()?; + #[allow(clippy::cast_sign_loss)] + let filesystem_type = fs.f_type as u64; + reject_special_workspace_filesystem_type(path, filesystem_type) +} + +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem_type(path: &Path, filesystem_type: u64) -> Result<()> { + const SPECIAL_FILESYSTEMS: &[u64] = &[ + 0x0000_1cd1, // devpts + 0x0000_9fa0, // proc + 0x0102_1994, // tmpfs (including the container /dev tree) + 0x1980_0202, // mqueue + 0x0027_e0eb, // cgroup + 0x4249_4e4d, // bpf + 0x6265_6572, // sysfs + 0x6367_7270, // cgroup2 + 0x6462_6720, // debugfs + 0x7363_6673, // securityfs + 0x7472_6163, // tracefs + ]; + if SPECIAL_FILESYSTEMS.contains(&filesystem_type) { + return Err(miette::miette!( + "workspace path component '{}' is on a kernel-managed filesystem (type {filesystem_type:#x})", + path.display() + )); + } + Ok(()) +} + +#[cfg(not(target_os = "linux"))] +#[allow(clippy::unnecessary_wraps)] +fn reject_special_workspace_filesystem(_path: &Path) -> Result<()> { + 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}; @@ -1802,7 +1960,13 @@ fn chown_recursive( /// UIDs/GIDs (passed directly to `chown` without a passwd lookup). #[cfg(unix)] pub fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { - prepare_filesystem_with_identity(policy, ResolvedProcessIdentity::default(), None, false) + prepare_filesystem_with_identity( + policy, + ResolvedProcessIdentity::default(), + None, + false, + false, + ) } #[cfg(unix)] @@ -1811,6 +1975,7 @@ pub fn prepare_filesystem_with_identity( resolved_identity: ResolvedProcessIdentity, workdir: Option<&str>, prepare_workspace: bool, + workspace_prevalidated: bool, ) -> Result<()> { use nix::unistd::chown; @@ -1831,17 +1996,19 @@ 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. + // Docker and Podman own 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. if prepare_workspace { let workspace = workdir.ok_or_else(|| { miette::miette!("local container driver did not supply a workspace workdir") })?; let workspace = Path::new(workspace); - if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) { + if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) + || workspace_prevalidated + { info!(path = %workspace.display(), ?uid, ?gid, "Preparing managed workspace"); prepare_oci_workspace(workspace, uid, gid, &supplementary_gids)?; } else { @@ -1867,8 +2034,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() { @@ -3008,6 +3175,23 @@ mod tests { .expect("supplementary group already has write and traverse authority"); } + #[cfg(unix)] + #[test] + fn workspace_identity_attestation_sorts_and_deduplicates_groups() { + assert_eq!( + workspace_identity_attestation( + Some(Uid::from_raw(1000)), + Some(Gid::from_raw(1001)), + &[ + Gid::from_raw(1003), + Gid::from_raw(1002), + Gid::from_raw(1003), + ], + ), + "1000:1001:1002,1003" + ); + } + #[cfg(unix)] #[test] fn validate_oci_workspace_rejects_unwritable_directory() { @@ -3157,6 +3341,25 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + fn effective_identity_validation_uses_kernel_access_checks() { + if nix::unistd::geteuid().is_root() { + return; + } + + 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(); + validate_oci_workspace_as_effective_identity(&root) + .expect("current identity can write and enter its directory"); + + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o500)).unwrap(); + let error = validate_oci_workspace_as_effective_identity(&root).unwrap_err(); + assert!(error.to_string().contains("not writable")); + } + #[cfg(unix)] #[test] fn validate_oci_workspace_rejects_restrictive_parent() { diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index 91e56b7ec..9e375a336 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -101,6 +101,7 @@ pub async fn run_process( resolved_process_identity, workspace.root(), workspace.home().is_some(), + workspace.prevalidated(), )?; } diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index be1b67953..127fc4944 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -704,8 +704,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")`. Docker and Podman replace that default with their +/// 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 2132f3360..17e13d2cf 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -251,9 +251,10 @@ Podman mount schema: Podman `volume` and `image` mounts do not support `subpath` in OpenShell driver config, and OpenShell rejects `subpath` for those mount types. OpenShell rejects mount `source` and `target` values with surrounding whitespace. OpenShell also -rejects mount targets that replace the workspace root, container root, supervisor -files, `/etc/openshell`, `/etc/openshell-tls`, authentication material, or -network namespace paths. These checks do not make host bind mounts safe. +rejects mount targets that replace or contain the workspace root, target the +container root, or contain or are contained by concrete OpenShell control +targets such as the supervisor mount, TLS and token files, runtime socket, or +network namespace mount. These checks do not make host bind mounts safe. ## MicroVM Driver @@ -444,7 +445,7 @@ 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 @@ -454,13 +455,22 @@ 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. +`/dev`, and rejects overlap with actual OpenShell control paths. Both drivers +reject image `VOLUME` declarations that would mask the workdir or one of its +parents before validation. Docker checks the original image filesystem in the +final supervisor. Podman checks the same pinned image ID in a temporary +networkless probe without the workspace volume, token, or TLS secrets. The +probe retains only `SETUID` and `SETGID`, drops to +the identity selected by the effective global-or-sandbox policy, and uses the +kernel to validate access. When neither policy exists, it discovers the image +policy identity. The final supervisor must match the probe's normalized +identity attestation. Podman captures a bounded, sanitized failure diagnostic, +removes the exact probe it created, and only then creates the final +volume-backed sandbox. Probe names are unique to each attempt, so an ambiguous +create response cannot poison a retry. 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 @@ -491,9 +501,9 @@ 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. -Images with no working directory, `WORKDIR /`, or `WORKDIR /sandbox` use -OpenShell's managed `/sandbox` compatibility workspace. For any other Docker -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`. +does not. 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 path, create the directory in the image and grant the final +process identity write and execute permission in the Dockerfile. +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 5652a0011..7b2a3bd75 100644 --- a/e2e/rust/tests/custom_image.rs +++ b/e2e/rust/tests/custom_image.rs @@ -56,6 +56,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ && useradd -m -u 3234 -g appstaff app WORKDIR /workspace/project +# Image metadata must not be able to forge the driver's successful-validation +# attestation. +ENV OPENSHELL_OCI_WORKSPACE_IDENTITY=3234:3235: USER app CMD ["sleep", "infinity"] "#; diff --git a/e2e/rust/tests/driver_config_volume.rs b/e2e/rust/tests/driver_config_volume.rs index 0702a4637..2a91f9ce1 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 e30516bf0..ff492bbb0 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -3,14 +3,14 @@ #![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. +//! This exercises the Podman API inspect → protected metadata → create path +//! rather than only its unit serialization boundaries. use std::process::Stdio; @@ -22,7 +22,7 @@ const BASE_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:late const READY_MARKER: &str = "podman-oci-identity-ready"; const OCI_UID: &str = "2345"; const OCI_GID: &str = "2346"; -const OCI_FALLBACK_POLICY: &str = r#"version: 1 +const IMAGE_POLICY: &str = r#"version: 1 filesystem_policy: include_workdir: true @@ -32,6 +32,10 @@ landlock: compatibility: best_effort network_policies: {} + +process: + run_as_user: "2345" + run_as_group: "2346" "#; struct ImageGuard { @@ -52,9 +56,21 @@ impl ImageGuard { let context = tempfile::tempdir().map_err(|err| format!("create build context: {err}"))?; let containerfile = context.path().join("Containerfile"); + std::fs::write(context.path().join("policy.yaml"), IMAGE_POLICY) + .map_err(|err| format!("write image policy: {err}"))?; 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\ + COPY policy.yaml /etc/openshell/policy.yaml\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,31 +180,29 @@ 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_and_inspected_image_id() { if !is_e2e_driver("podman") { eprintln!("Skipping Podman OCI identity test: e2e driver is not podman"); return; } let image = ImageGuard::build().expect("build Podman OCI identity image"); - // The community base image contains a baked default policy with an - // explicit `sandbox` process identity. Supply a complete policy that - // intentionally omits `process` so this test exercises OCI fallback. - let policy = tempfile::NamedTempFile::new().expect("create OCI fallback policy"); - 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"); + // Do not supply a policy at create time. The probe must discover the + // image's policy before the workspace volume hides the immutable layer. let mut sandbox = SandboxGuard::create_keep_with_args( - &[ - "--from", - &image.tag, - "--policy", - policy_path, - "--no-tty", - ], + &["--from", &image.tag, "--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 +219,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"); diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index e3f18af19..6aaf34180 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -132,8 +132,29 @@ message DriverSandboxSpec { // ServiceAccount token bootstrap instead). Never echoed to the public // Sandbox proto. string sandbox_token = 11 [(openshell.options.v1.secret) = true]; + // Effective process-identity source needed by local container drivers when + // they validate an OCI image workspace before launching the supervisor. + WorkspaceValidationIdentity workspace_validation_identity = 12; } +message WorkspaceValidationIdentity { + oneof source { + // A sandbox or global policy is authoritative. Empty fields retain the + // OCI USER component for that field. + PolicyProcessIdentity policy = 1; + // No effective policy exists, so discover an image-provided policy before + // resolving the final process identity. + ImagePolicyDiscovery image = 2; + } +} + +message PolicyProcessIdentity { + string run_as_user = 1; + string run_as_group = 2; +} + +message ImagePolicyDiscovery {} + message ResourceRequirements { // GPU requirements for the sandbox. Presence indicates a GPU request. GpuResourceRequirements gpu = 1;