Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .agents/skills/debug-openshell-cluster/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ Common findings:
- Gateway process stopped: inspect exit status and logs.
- Sandbox image missing or pull denied: verify image reference and registry credentials.
- Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected.
- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must be an absolute normalized directory with no symlink components; the final policy UID, primary GID, and supplementary groups must pass the kernel's effective traverse/write checks, including POSIX ACL and LSM decisions. OpenShell does not create, chown, or chmod a non-default image workdir.
- Docker also rejects an image `VOLUME` that covers the workdir or one of its parents because the runtime would mask the immutable path before validation. Move the `VOLUME` below the workspace or remove the declaration.
- A workdir rejected as a special filesystem or OpenShell control-path collision cannot be made valid with permissions. Move the image workdir away from kernel-backed mounts and the concrete supervisor, TLS, token, runtime, and socket paths named in the error.
- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must already exist as an absolute normalized directory with no symlink components. OpenShell does not test identity permissions or create, chown, or chmod a non-default image workdir; diagnose later `chdir` or write failures from the image's final user and permissions.
- Docker rejects an image `VOLUME` that covers the workdir, one of its parents, or an OpenShell control path. Move the `VOLUME` below the workspace or remove the declaration.
- A workdir rejected as a forbidden workspace root or OpenShell control-path collision cannot be made valid with permissions. Move it away from the protected kernel-managed roots, the supervisor's executable/library roots, and the concrete supervisor, TLS, token, runtime, and socket paths named in the error. This is a mount-placement guardrail, not a general custom-image integrity check.
- Docker driver cannot initialize because it cannot find `openshell-sandbox`: verify `OPENSHELL_DOCKER_SUPERVISOR_BIN`, the sibling binary next to `openshell-gateway`, or the configured supervisor image contains `/openshell-sandbox`.
- Sandbox never registers: check gateway logs and supervisor callback endpoint.
- Supervisor image exits before printing `openshell-sandbox --version`: the image should be the scratch supervisor image from `deploy/docker/Dockerfile.supervisor` and must contain a static executable at `/openshell-sandbox`.
Expand All @@ -202,6 +202,8 @@ Common findings:
- Rootless networking unavailable: inspect Podman network configuration.
- Sandbox image missing or pull denied: verify image reference and registry credentials.
- Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected.
- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's OCI `WorkingDir`. Empty, `/`, and `/sandbox` use managed `/sandbox`; other paths must be normalized and, after Podman initializes the workspace volume, contain only existing directories with no symlinks or protected runtime/control/system roots. OpenShell does not test identity permissions or create, chown, or chmod a non-default workdir; diagnose later `chdir` or write failures from the image's final user and permissions.
- Podman mounts the persistent named workspace volume at OCI `WorkingDir` and validates after normal copy-up. Inspect the volume inside the failed container with `podman inspect` and `podman unshare` as appropriate. Ownership and SELinux behavior can differ between rootless and rootful deployments; fix the image or runtime configuration rather than expecting OpenShell to repair permissions.
- Supervisor cannot call back: check callback endpoint and gateway logs.
- Gateway exits before becoming healthy with a callback-listener discovery
error: inspect `podman info --debug`, the configured Podman network, and the
Expand Down
11 changes: 11 additions & 0 deletions .agents/skills/openshell-cli/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,17 @@ field wins independently; omitted fields fall back to the image declaration.
An image with no `USER` fails before readiness unless policy supplies both
fields.

Docker and Podman custom images may declare an absolute OCI `WORKDIR`. Empty,
`/`, and `/sandbox` use the managed `/sandbox` compatibility workspace. For any
other path, the image author must make the final OCI/policy identity able to
traverse and write it. OpenShell validates the path's structure before
initialization but does not test identity permissions or create, chown, or chmod
it. An unusable image fails naturally when its workload changes directory or
writes. Podman mounts its persistent named volume at that path and performs
normal initial copy-up. Workdirs cannot overlap kernel-managed OCI mounts or the
supervisor's minimal executable and library roots; this protects workspace
mount placement, not custom-image integrity.

### Forward ports

```bash
Expand Down
44 changes: 27 additions & 17 deletions architecture/compute-runtimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ The gateway preserves whether each policy process field was omitted. The active
driver then supplies one authoritative identity input to the supervisor:

- Docker and Podman inspect the final sandbox image, pin container creation to
its immutable image ID, and pass its raw OCI `Config.User`. Docker also
resolves the workspace from OCI `Config.WorkingDir` during that inspection.
its immutable image ID, and pass its raw OCI `Config.User`. Both resolve the
workspace from OCI `Config.WorkingDir` during that inspection.
- Kubernetes passes its platform-resolved numeric UID/GID, including OpenShift
SCC-derived values.
- VM keeps its existing guest identity behavior.
Expand All @@ -199,23 +199,33 @@ and uses the same privilege-drop path for direct and SSH children. When a
declaration omits the group, the supervisor fills it with the user's numeric
primary GID. It does not rewrite the account files.

Docker uses an absolute OCI working directory as the workspace. An
Docker and Podman use an absolute OCI working directory as the workspace. An
empty, root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which
OpenShell creates and owns as a compatibility workspace. Any other workdir must already
exist in the immutable image without symlink components. The completed
identity, including supplementary groups, must already be able to traverse
every parent and write and enter the workdir; OpenShell does not change that
directory's ownership or mode. A one-shot validator drops to that identity and
uses kernel effective-access checks so POSIX ACL and LSM decisions are honored.
Path checks reserve the standard OCI runtime namespaces under `/proc`, `/sys`,
and `/dev`, while separate collision checks are derived from actual OpenShell
control paths.
Docker performs the check in the final container before workload launch and
rejects image `VOLUME` declarations that would mask the workdir ancestry. The
resolved workspace is the child cwd and `HOME`; when
OpenShell creates and owns as a compatibility workspace. OpenShell does not
create, chown, or chmod any other workdir. Image authors are responsible for
making the final OCI/policy identity able to traverse and write that workdir.
An unusable image fails naturally when its workload changes directory or writes.

Before policy, credential, TLS, or networking initialization, the final
supervisor performs a no-follow structural walk of every non-default path. It
rejects missing components, symlinks, non-directories, and OpenShell control
paths. The drivers also reject overlap in
either direction with `/proc`, `/sys`, `/dev`, `/bin`, `/sbin`, `/usr/bin`,
`/usr/sbin`, `/lib`, `/lib64`, `/usr/lib`, or `/usr/lib64`. The first three are
kernel-managed OCI mounts; the others protect executable and library roots used
by the supervisor. This is a mount-placement guardrail, not an image-integrity
or permission guarantee. Explicit driver-config mounts may not cover the
resolved OCI `WORKDIR` or an OpenShell control path. Docker rejects
image-declared `VOLUME` entries that cover the workdir or control paths.
Podman leaves image-declared volume behavior to the runtime.

Docker checks the image directory directly. Podman mounts the persistent named
workspace volume at the resolved workdir and validates it after Podman's normal
initial copy-up. OpenShell does not repair ownership or permissions after that
copy-up. The resolved workspace is the child cwd and `HOME`; when
`filesystem.include_workdir` is enabled, it becomes the automatic writable
policy path. Podman, Kubernetes/OpenShift, and VM retain their existing
`/sandbox` workspace behavior.
policy path. Kubernetes/OpenShift and VM retain their existing `/sandbox`
workspace behavior.

Sandbox creation fails before the workload becomes ready when a required image
identity is absent, malformed, unknown, ambiguous, or resolves to UID/GID 0.
Expand Down
35 changes: 26 additions & 9 deletions crates/openshell-core/src/container_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,31 @@ pub const SIDECAR_RUN_ROOT: &str = "/run/openshell-sidecar";
pub const NETNS_MOUNT_ROOT: &str = "/run/netns";
pub const NETNS_IPROUTE2_ROOT: &str = "/var/run/netns";

/// Standard Linux container namespaces that an image-selected workspace must
/// not contain or enter.
/// Container roots that an image-selected workspace must not overlap.
///
/// These roots cover the default filesystems and devices defined by the OCI
/// Runtime Specification: procfs, sysfs, cgroups, device nodes, devpts, shared
/// memory, and POSIX message queues.
/// <https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#default-filesystems>
pub const OCI_RUNTIME_MOUNT_ROOTS: &[&str] = &["/proc", "/sys", "/dev"];
/// The first group contains kernel-managed mounts from the OCI runtime. The
/// second protects executable and library roots used by the in-container
/// supervisor. This is a narrow mount-placement guardrail, not an image
/// integrity check; application paths such as `/usr/src/app` remain valid.
///
/// TODO: Make the supervisor's helper functionality self-contained so it no
/// longer depends on executable and library paths shared with the image.
/// <https://github.com/NVIDIA/OpenShell/pull/2715>
pub const FORBIDDEN_WORKSPACE_ROOTS: &[&str] = &[

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outcome of a review session with agent:

The OCI WORKDIR support and structural validation look well thought out, but I think the workspace-root protection is incomplete in a security-relevant way.

This PR allows /usr/local/bin as an OCI workspace while the privileged supervisor executes dmesg through PATH. Docker’s fixed supervisor PATH includes /usr/local/bin; Podman additionally permits the image/user environment to control PATH. Therefore an image can make /usr/local/bin writable by its non-root workload, have the workload place a dmesg executable there, and have a later privileged supervisor invocation execute it as root.

The same general concern applies to writable library locations such as /usr/local/lib, although the exact loader behavior depends on the image configuration.

I don’t think this PR needs to solve the entire workload-image trust problem tracked by #2750. However, it should avoid making any path used to resolve or load privileged supervisor code into a persistent writable workspace.

Before merging, I suggest:

  1. Add every supervisor executable-search location to the forbidden workspace roots, particularly /usr/local/bin and /usr/local/sbin. Because overlap is rejected in both directions, this should also prevent mounting a workspace at /usr/local and masking those directories.
  2. Review /usr/local/lib, /usr/local/lib64, and other configured loader paths under the same rule.
  3. Align Podman with Docker by assigning a supervisor-owned PATH, rather than passing an image/user-controlled PATH to the root supervisor.
  4. Run privileged helper commands with a clean, allowlisted environment so variables such as LD_PRELOAD and LD_LIBRARY_PATH cannot influence them.
  5. Add a regression test using a hostile writable WORKDIR containing a fake dmesg.

#2750 can then provide the stronger long-term fix: ensuring the privileged supervisor never executes or dynamically loads anything from the workload image.

// Kernel-managed OCI runtime mounts.
"/proc",
"/sys",
"/dev",
// Executable and library roots needed by the supervisor.
"/bin",
"/sbin",
"/lib",
"/lib64",
"/usr/bin",
"/usr/sbin",
"/usr/lib",
"/usr/lib64",
];
Comment thread
matthewgrossman marked this conversation as resolved.

/// High-level namespaces mounted or created by `OpenShell` inside sandboxes.
///
Expand Down Expand Up @@ -118,7 +135,7 @@ mod tests {
}

#[test]
fn runtime_roots_cover_standard_oci_mount_destinations() {
fn forbidden_workspace_roots_cover_standard_oci_mount_destinations() {
for path in [
"/proc",
"/dev",
Expand All @@ -129,7 +146,7 @@ mod tests {
"/sys/fs/cgroup",
] {
assert!(
OCI_RUNTIME_MOUNT_ROOTS
FORBIDDEN_WORKSPACE_ROOTS
.iter()
.any(|root| Path::new(path).starts_with(root)),
"OCI runtime mount {path} is outside the reserved roots"
Expand Down
34 changes: 28 additions & 6 deletions crates/openshell-core/src/driver_mounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use std::path::Path;

use crate::container_paths::{CONTROL_ROOTS, OCI_RUNTIME_MOUNT_ROOTS};
use crate::container_paths::{CONTROL_ROOTS, FORBIDDEN_WORKSPACE_ROOTS};

/// `SELinux` relabelling mode for bind mounts.
///
Expand Down Expand Up @@ -110,13 +110,16 @@ pub fn resolve_oci_workspace_root(working_dir: &str) -> Result<String, String> {
return Ok(DEFAULT_WORKSPACE_ROOT.to_string());
}
let workspace_root = normalize_absolute_container_path(working_dir, "OCI WorkingDir")?;
for runtime_path in OCI_RUNTIME_MOUNT_ROOTS {
validate_workspace_reserved_path(&workspace_root, runtime_path, "OCI runtime mount")?;
for forbidden_root in FORBIDDEN_WORKSPACE_ROOTS {
validate_workspace_reserved_path(
&workspace_root,
forbidden_root,
"forbidden workspace root",
)?;
}
for control_path in CONTROL_ROOTS {
validate_workspace_control_path(&workspace_root, control_path)?;
}

Ok(workspace_root)
}

Expand Down Expand Up @@ -278,7 +281,7 @@ mod tests {
}

#[test]
fn oci_workspace_root_rejects_runtime_and_openshell_control_path_collisions() {
fn oci_workspace_root_rejects_forbidden_and_openshell_control_path_collisions() {
for invalid in [
"/proc",
"/proc/self",
Expand All @@ -299,6 +302,17 @@ mod tests {
"/run/openshell-sidecar/control.sock",
"/run/netns/project",
"/var/run/netns/project",
"/bin",
"/bin/project",
"/sbin",
"/lib",
"/lib/project",
"/lib64",
"/usr",
"/usr/bin",
"/usr/bin/project",
"/usr/lib",
"/usr/lib64",
] {
assert!(
resolve_oci_workspace_root(invalid).is_err(),
Expand All @@ -310,9 +324,17 @@ mod tests {
"/app",
"/etc/project",
"/home/app",
"/lib32/app",
"/libx32/app",
"/opt/app",
"/usr/bin/project",
"/usr/lib32/app",
"/usr/libexec/app",
"/usr/libx32/app",
"/usr/src/app",
"/usr/local",
"/usr/local/app",
"/usr/local/bin",
"/usr/local/lib",
"/var/lib/app",
"/var/app/current",
"/var/task",
Expand Down
27 changes: 12 additions & 15 deletions crates/openshell-driver-docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,23 @@ numeric primary GID. Explicit `process.run_as_user` and
An absolute OCI working directory becomes the agent workspace. An empty,
root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell
creates when necessary and owns as a compatibility workspace. Any other image
workdir must already exist without symlink components. The completed identity,
including supplementary groups, must already be able to traverse every parent
and write and enter the workdir. OpenShell does not change its ownership or
mode.

OpenShell deliberately asks the Linux kernel to make this access decision
under the completed sandbox identity instead of reproducing permission rules
from ownership and mode bits. Mode-bit inspection alone can reject authority
granted by a POSIX ACL or overlook a denial imposed by a Linux Security Module
such as SELinux or AppArmor. OpenShell does not configure or otherwise manage
ACLs or LSM policy here; the one-shot validator only observes the kernel's
effective decision. This keeps the no-authority-expansion invariant aligned
with the access the eventual workload will receive without adding a separate,
incomplete permission model to OpenShell.
workdir must already exist without symlink components. OpenShell does not
create it or change its ownership or mode. Image authors are responsible for
making the final OCI/policy identity able to traverse and write it; an unusable
image fails naturally when its workload changes directory or writes.

Before loading policy, credentials, TLS, or networking state, the supervisor
performs a no-follow structural walk. It rejects missing components, symlinks,
non-directories, and OpenShell control paths. This check protects workspace
mount placement; it does not validate custom-image
integrity or workdir permissions.

Image `VOLUME` declarations must not cover the workdir or one of its parents
because Docker would mount the volume before the supervisor could validate the
immutable image path.
Workdirs under the standard OCI runtime namespaces `/proc`, `/sys`, and `/dev`
are rejected, as are paths that overlap concrete OpenShell control resources.
are rejected, as are paths that overlap protected executable and library roots
or concrete OpenShell control resources.
The workspace is the child cwd and `HOME`. The supervisor starts from `/`, then
reports an invalid workdir as a readiness failure.

Expand Down
34 changes: 33 additions & 1 deletion crates/openshell-driver-docker/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,38 @@ fn container_creation_rejects_openshell_control_path_working_dir() {
assert!(err.message().contains("OpenShell control path"));
}

#[test]
fn container_creation_protects_forbidden_roots_but_allows_usr_application_paths() {
let rejected = DockerImageMetadata {
id: "sha256:immutable".to_string(),
user: "1234:1235".to_string(),
working_dir: "/usr".to_string(),
volumes: Vec::new(),
};
let error = build_container_create_body_for_image(
&test_sandbox(),
&runtime_config(),
&DockerSandboxDriverConfig::default(),
None,
&rejected,
)
.unwrap_err();
assert!(error.message().contains("forbidden workspace root"));

let allowed = DockerImageMetadata {
working_dir: "/usr/src/app".to_string(),
..rejected
};
build_container_create_body_for_image(
&test_sandbox(),
&runtime_config(),
&DockerSandboxDriverConfig::default(),
None,
&allowed,
)
.expect("application workdirs below /usr remain valid");
}

#[test]
fn container_creation_rejects_image_volume_that_masks_working_dir() {
let sandbox = test_sandbox();
Expand Down Expand Up @@ -1460,7 +1492,7 @@ fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() {
);
assert_eq!(
create_body.cmd,
Some(vec!["--workdir".to_string(), "/sandbox".to_string()])
Some(vec!["--workdir".to_string(), "/sandbox".to_string(),])
);
assert_eq!(
create_body
Expand Down
Loading
Loading