diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index e1e731a0ce..06433fcc29 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -391,6 +391,21 @@ identity is absent, malformed, unknown, ambiguous, or resolves to UID/GID 0. The supervisor itself remains root so it can establish isolation before starting unprivileged children. +When the Kubernetes driver seeds a fresh workspace PVC from the image, the +seeded tree is written as the resolved sandbox UID/GID. The init container runs +as root so it can read every source path, and rewrites ownership while building +the transfer archive rather than chowning the workspace afterwards. Ownership +must be established during seeding: `fsGroup` is applied by kubelet when the +volume is mounted, which is before the init container writes anything, and the +sidecar topology does not perform the supervisor's privileged workspace +reconciliation. Modes and timestamps are not restored, so a nested read-only +mount under the workspace is never chmod'ed during seeding. + +The ownership rewrite uses GNU tar extensions, and the init container runs the +sandbox image itself. Images whose tar lacks them — a minimal base such as +Alpine, which provides BusyBox tar — seed with the previous flags instead, so +they keep the behavior they have today rather than failing to start. + Kubernetes can run the supervisor in the default combined topology or in a sidecar topology. Combined mode keeps network and process supervision in the agent container. Sidecar mode runs network enforcement, the proxy, and gateway diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index bb1b75e8a9..46e9c738c8 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -3408,11 +3408,15 @@ fn apply_supervisor_sidecar_topology( /// The init container mounts the PVC at a temporary path so it can still see /// the image's `/sandbox` directory. It checks for a sentinel file and skips /// the copy if the PVC was already initialised. +/// +/// Seeded content is written as `sandbox_uid`/`sandbox_gid` so the workload can +/// use it. See the extended note on the tar invocation below. #[allow(clippy::similar_names)] fn apply_workspace_persistence( pod_template: &mut serde_json::Value, image: &str, image_pull_policy: &str, + sandbox_uid: u32, sandbox_gid: u32, ) { let Some(spec) = pod_template.get_mut("spec").and_then(|v| v.as_object_mut()) else { @@ -3470,8 +3474,32 @@ fn apply_workspace_persistence( // is valid. `tar` copies the tree without dereferencing those links. // Archive only the contents, not the `/sandbox` directory entry // itself, so extraction never tries to chmod the PVC mount root. - // Extract without restoring owner, mode, or timestamps so the - // non-root init container can seed kubelet-owned PVCs. + // + // Ownership is rewritten to the resolved sandbox identity while + // *building* the archive (`--owner`/`--group`/`--numeric-owner`), and + // extraction then restores it. Seeding as root with `--no-same-owner` + // used to leave every seeded path owned by uid 0; a mode-0700 home + // directory from the image (`~/.config`, `~/.cache`) was then + // unreachable for the workload. `fsGroup` does not compensate: kubelet + // applies it when the volume is mounted, which is before this init + // container writes anything. + // + // Rewriting at archive time — rather than extracting as the sandbox + // user, or chowning the tree afterwards — keeps root's ability to read + // every source path, so images that ship root-owned private content + // still seed. It also avoids a recursive chown over the workspace, the + // pattern that broke on read-only submounts in #2294. + // + // Modes and timestamps are still not restored, so a nested read-only + // mount under the PVC is never chmod'ed during seeding. + // + // `--owner`/`--group` at archive time are GNU extensions, and this init + // container runs the sandbox image itself — a minimal base such as + // Alpine seeds with BusyBox tar, which rejects them and would fail the + // init container, leaving the pod unable to start. The script probes + // tar once and falls back to the previous flags when the extensions are + // unavailable: such images keep seeding exactly as they do today rather + // than regressing into a startup failure. // // The inner `[ -d ... ]` guard handles custom images that don't have // a /sandbox directory — the copy is skipped but the sentinel is @@ -3479,10 +3507,14 @@ fn apply_workspace_persistence( let copy_cmd = format!( "if [ ! -f {WORKSPACE_INIT_MOUNT_PATH}/{WORKSPACE_SENTINEL} ]; then \ if [ -d {WORKSPACE_MOUNT_PATH} ]; then \ + own=\"--owner={sandbox_uid} --group={sandbox_gid} --numeric-owner\"; \ + ext=\"--numeric-owner\"; \ + tar $own -cf /dev/null -T /dev/null 2>/dev/null || \ + {{ own=\"\"; ext=\"--no-same-owner\"; }}; \ tmp=$(mktemp) && rm -f \"$tmp\" && \ - (cd {WORKSPACE_MOUNT_PATH} && find . -mindepth 1 -maxdepth 1 -exec tar -cf \"$tmp\" {{}} +) && \ + (cd {WORKSPACE_MOUNT_PATH} && find . -mindepth 1 -maxdepth 1 -exec tar $own -cf \"$tmp\" {{}} +) && \ if [ -f \"$tmp\" ]; then \ - tar -C {WORKSPACE_INIT_MOUNT_PATH} --no-same-owner --no-same-permissions --touch -xf \"$tmp\" && \ + tar -C {WORKSPACE_INIT_MOUNT_PATH} $ext --no-same-permissions --touch -xf \"$tmp\" && \ rm -f \"$tmp\"; \ fi; \ fi && \ @@ -4130,6 +4162,7 @@ fn sandbox_template_to_k8s_with_validated_config( &mut result, image, params.image_pull_policy, + params.sandbox_uid, params.sandbox_gid, ); } @@ -7355,6 +7388,7 @@ mod tests { &mut pod_template, "openshell/sandbox:latest", "IfNotPresent", + 1000, // sandbox_uid 1000, // sandbox_gid ); @@ -7415,6 +7449,7 @@ mod tests { "my-custom-image:v2", "IfNotPresent", 1000, + 1000, ); let init_image = pod_template["spec"]["initContainers"][0]["image"] @@ -7437,7 +7472,7 @@ mod tests { } }); - apply_workspace_persistence(&mut pod_template, "img:latest", "Always", 1000); + apply_workspace_persistence(&mut pod_template, "img:latest", "Always", 1000, 1000); let cmd = pod_template["spec"]["initContainers"][0]["command"] .as_array() @@ -7456,10 +7491,134 @@ mod tests { "init script must archive sandbox contents without the mount root entry" ); assert!( - script.contains("--no-same-owner") - && script.contains("--no-same-permissions") - && script.contains("--touch"), - "init script must avoid restoring metadata onto the PVC root" + script.contains("--no-same-permissions") && script.contains("--touch"), + "init script must not restore modes or timestamps onto the PVC" + ); + } + + /// Regression: seeding the PVC as root with `--no-same-owner` left every + /// seeded path owned by uid 0, so a mode-0700 home directory from the + /// image (`~/.config`, `~/.cache`) was unreachable for the workload. + /// Ownership must be rewritten to the resolved sandbox identity instead. + #[test] + fn workspace_init_seeds_content_owned_by_the_sandbox_identity() { + let mut pod_template = serde_json::json!({ + "spec": { + "containers": [{ + "name": "agent", + "image": "img:latest" + }] + } + }); + + apply_workspace_persistence(&mut pod_template, "img:latest", "Always", 1234, 5678); + + let script = pod_template["spec"]["initContainers"][0]["command"][2] + .as_str() + .expect("init script should be the third command element") + .to_string(); + + assert!( + script.contains("--owner=1234") && script.contains("--group=5678"), + "init script must rewrite seeded ownership to the resolved sandbox identity, got: {script}" + ); + assert!( + script.contains("--numeric-owner"), + "ownership rewrite must be numeric so it does not depend on image account files" + ); + assert!( + script.contains("ext=\"--numeric-owner\""), + "extraction must restore the rewritten ownership, not discard it" + ); + assert!( + !script.contains(&format!( + "tar -C {WORKSPACE_INIT_MOUNT_PATH} --no-same-owner" + )), + "extraction must not discard ownership unconditionally — that is the \ + regression being fixed" + ); + assert!( + !script.contains("chown"), + "seeding must not chown the workspace tree — that pattern broke on \ + read-only submounts (#2294)" + ); + } + + /// `--owner`/`--group` are GNU extensions and this init container runs the + /// sandbox image itself, so a minimal base such as Alpine seeds with + /// `BusyBox` tar. Rejecting those options fails the init container and the + /// pod never starts, so the script probes tar once and degrades to the + /// previous flags instead of hard-failing. + #[test] + fn workspace_init_falls_back_when_tar_lacks_ownership_extensions() { + let mut pod_template = serde_json::json!({ + "spec": { + "containers": [{ + "name": "agent", + "image": "img:latest" + }] + } + }); + + apply_workspace_persistence(&mut pod_template, "img:latest", "Always", 1234, 5678); + + let script = pod_template["spec"]["initContainers"][0]["command"][2] + .as_str() + .expect("init script should be the third command element") + .to_string(); + + assert!( + script.contains("tar $own -cf /dev/null -T /dev/null 2>/dev/null"), + "the script must probe tar for the ownership extensions, got: {script}" + ); + assert!( + script.contains("own=\"\"; ext=\"--no-same-owner\""), + "the probe must fall back to the previous flags, got: {script}" + ); + assert!( + script.contains("tar $own -cf \"$tmp\"") + && script.contains("$ext --no-same-permissions --touch -xf"), + "both tar invocations must use the probed flags, got: {script}" + ); + } + + /// The seeded identity must track the driver-resolved UID/GID rather than + /// a hardcoded 1000, so images built for a different UID range (or an + /// OpenShift-allocated one) still produce a usable workspace. + #[test] + fn workspace_init_ownership_tracks_resolved_identity() { + let params = SandboxPodParams { + sandbox_uid: 1_000_660_000, + sandbox_gid: 1_000_660_000, + ..SandboxPodParams::default() + }; + + let pod_template = sandbox_template_to_k8s( + &SandboxTemplate { + image: "img:latest".to_string(), + ..SandboxTemplate::default() + }, + false, + &std::collections::HashMap::new(), + true, + ¶ms, + ); + + let init_containers = pod_template["spec"]["initContainers"] + .as_array() + .expect("initContainers should exist"); + let workspace_init = init_containers + .iter() + .find(|c| c["name"] == WORKSPACE_INIT_CONTAINER_NAME) + .expect("workspace init container should exist"); + + let script = workspace_init["command"][2] + .as_str() + .expect("init script should be the third command element"); + + assert!( + script.contains("--owner=1000660000") && script.contains("--group=1000660000"), + "seeded ownership must follow the resolved sandbox identity, got: {script}" ); } diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 987e66b0d9..9d1eed4f0e 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -590,7 +590,8 @@ You can override autodetection with explicit `sandbox_uid` / `sandbox_gid` confi The resolved UID/GID appear in: - Supervisor container environment variables (`OPENSHELL_SANDBOX_UID`, `OPENSHELL_SANDBOX_GID`) for direct kernel-level privilege dropping without `/etc/passwd` lookups. -- PVC init container `securityContext.runAsUser/runAsGroup/fsGroup` for workspace ownership operations. +- Agent container `securityContext.runAsUser/runAsGroup`, and the pod-level `securityContext.fsGroup` that kubelet applies to the workspace volume at mount time. +- Ownership of the content the `workspace-init` container seeds into a fresh workspace PVC. That container runs as root so it can read every path the image ships, and writes the seeded tree as the resolved UID/GID. Images whose `tar` lacks the GNU ownership extensions — a minimal base such as Alpine, which provides BusyBox `tar` — seed as root instead, so a mode-`0700` directory from such an image stays unreadable to the workload. ### VM Driver