From aace71ffa1c09dc15a5076b8c51f0c545987bcf0 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 17:47:08 +0000 Subject: [PATCH 1/2] feat(docker): scope managed resources by host UID Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-docker/README.md | 11 ++ crates/openshell-driver-docker/src/lib.rs | 57 +++++++++- crates/openshell-driver-docker/src/tests.rs | 109 +++++++++++++++++++ docs/how-it-works/gateways/configuration.mdx | 19 +++- skills/debug-openshell-cluster/SKILL.md | 7 ++ 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 7f2222bb34..f620c0bf8b 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -25,6 +25,17 @@ label resolver and are denied under enforcement. GPU devices are temporarily exempt. Inspect labels again before launch, restart, and during reconciliation. See [resource admission configuration](../../docs/how-it-works/gateways/configuration.mdx#external-resource-admission). +`scope_by_host_uid = true` under `[openshell.drivers.docker]` derives the +driver's existing sandbox namespace from the gateway process's effective host +UID and `sandbox_label` (`uid--`). The same namespace flows +through workload, supervisor, staging, and identity container labels; container +queries and startup cleanup; volume labels and names; and private host state. +The default remains unscoped for compatibility. Opt-in fails before driver +reconciliation when OpenShell containers or volumes still have the unscoped +label, avoiding an invisible legacy sandbox after a config change. Delete old +sandboxes through the original gateway before enabling the option. The scope +separates OS users through OpenShell, but Docker daemon access can bypass it. + The driver creates two containers for each sandbox: - `openshell-sandbox` is PID 1 in the workload container. It owns the workload diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 8bf5df4e22..efb66647cf 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -195,9 +195,13 @@ pub struct DockerComputeConfig { /// Image pull policy for sandbox images. pub image_pull_policy: ImagePullPolicy, - /// Value of the `openshell.sandbox_namespace` label applied to Docker sandboxes. + /// Value of the `openshell.ai/sandbox-namespace` label applied to Docker sandboxes. pub sandbox_label: String, + /// Scope Docker-managed resources to the gateway process's host UID. + /// Enable only after removing resources in the unscoped namespace. + pub scope_by_host_uid: bool, + /// Gateway gRPC endpoint the sandbox connects back to. pub grpc_endpoint: String, @@ -323,6 +327,7 @@ impl Default for DockerComputeConfig { default_image: openshell_core::image::default_sandbox_image(), image_pull_policy: ImagePullPolicy::default(), sandbox_label: "default".to_string(), + scope_by_host_uid: false, grpc_endpoint: String::new(), supervisor_otlp_endpoint: None, supervisor_network_mode: DockerSupervisorNetworkMode::Host, @@ -347,6 +352,47 @@ impl Default for DockerComputeConfig { } } +fn docker_sandbox_namespace(config: &DockerComputeConfig, host_uid: u32) -> String { + if config.scope_by_host_uid { + format!("uid-{host_uid}-{}", config.sandbox_label) + } else { + config.sandbox_label.clone() + } +} + +async fn reject_unscoped_docker_resources(docker: &Docker, namespace: &str) -> CoreResult<()> { + let filters = managed_resource_label_filters(namespace, []); + let containers = docker + .list_containers(Some( + ListContainersOptionsBuilder::default() + .all(true) + .filters(&filters) + .build(), + )) + .await + .map_err(|error| { + Error::execution(format!( + "failed to list unscoped Docker containers: {error}" + )) + })?; + let volumes = docker + .list_volumes(Some( + ListVolumesOptionsBuilder::default() + .filters(&filters) + .build(), + )) + .await + .map_err(|error| { + Error::execution(format!("failed to list unscoped Docker volumes: {error}")) + })?; + if !containers.is_empty() || volumes.volumes.is_some_and(|volumes| !volumes.is_empty()) { + return Err(Error::config(format!( + "scope_by_host_uid requires an empty unscoped Docker namespace '{namespace}'; remove its OpenShell containers and volumes before enabling it" + ))); + } + Ok(()) +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct DockerGuestTlsPaths { pub(crate) ca: PathBuf, @@ -914,6 +960,8 @@ impl DockerComputeDriver { network_trust: Option, ) -> CoreResult { docker_config.validate_configuration(gateway_bind_address)?; + let sandbox_namespace = + docker_sandbox_namespace(docker_config, rustix::process::geteuid().as_raw()); let socket_path = docker_config .socket_path .clone() @@ -933,6 +981,9 @@ impl DockerComputeDriver { docker.version().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon version: {err}")) })?; + if docker_config.scope_by_host_uid { + reject_unscoped_docker_resources(&docker, &docker_config.sandbox_label).await?; + } let info = docker.info().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon info: {err}")) })?; @@ -986,7 +1037,7 @@ impl DockerComputeDriver { matches!(endpoint.host(), Some(url::Host::Domain(host)) if host.eq_ignore_ascii_case(HOST_DOCKER_INTERNAL)) }) { - Some(resolve_docker_host_gateway(&docker, &supervisor_image_id, &docker_config.sandbox_label).await?) + Some(resolve_docker_host_gateway(&docker, &supervisor_image_id, &sandbox_namespace).await?) } else { docker_supervisor_host_address(&supervisor_grpc_endpoint) }; @@ -1021,7 +1072,7 @@ impl DockerComputeDriver { config: DockerDriverRuntimeConfig { default_image: docker_config.default_image.clone(), image_pull_policy: docker_config.image_pull_policy, - sandbox_namespace: docker_config.sandbox_label.clone(), + sandbox_namespace, stop_timeout_secs: DEFAULT_STOP_TIMEOUT_SECS, log_level: gateway_log_level.to_string(), sandbox_binary, diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 7e99f37e22..e2c702b60b 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2571,6 +2571,115 @@ fn managed_container_label_filters_include_gateway_namespace() { assert!(labels.contains(&format!("{LABEL_SANDBOX_ID}=sbx-123"))); } +#[test] +fn host_uid_scope_separates_docker_resources() { + let mut config = DockerComputeConfig::default(); + assert!(!config.scope_by_host_uid); + assert_eq!(docker_sandbox_namespace(&config, 1000), "default"); + + config.scope_by_host_uid = true; + let alice_namespace = docker_sandbox_namespace(&config, 1000); + let bob_namespace = docker_sandbox_namespace(&config, 1001); + assert_eq!(alice_namespace, "uid-1000-default"); + assert_eq!(bob_namespace, "uid-1001-default"); + + let sandbox = test_sandbox(); + let mut alice = runtime_config(); + alice.sandbox_namespace = alice_namespace.clone(); + let mut bob = runtime_config(); + bob.sandbox_namespace = bob_namespace.clone(); + + let workload = build_container_create_body(&sandbox, &alice).unwrap(); + assert_eq!( + workload.labels.unwrap().get(LABEL_SANDBOX_NAMESPACE), + Some(&alice_namespace) + ); + let supervisor = + docker_auxiliary_container_labels(&sandbox, &alice, LABEL_ISOLATION_ROLE_SUPERVISOR); + assert_eq!( + supervisor.get(LABEL_SANDBOX_NAMESPACE), + Some(&alice_namespace) + ); + let resolver = docker_host_gateway_resolver_body("supervisor:test", &alice_namespace); + assert_eq!( + resolver.labels.unwrap().get(LABEL_SANDBOX_NAMESPACE), + Some(&alice_namespace) + ); + + let alice_filters = managed_container_label_filters(&alice_namespace, []); + let bob_filters = managed_container_label_filters(&bob_namespace, []); + assert_ne!(alice_filters, bob_filters); + assert_ne!( + docker_channel_volume_name(&sandbox, &alice), + docker_channel_volume_name(&sandbox, &bob) + ); + assert_ne!( + docker_supervisor_volume_name(&sandbox, &alice), + docker_supervisor_volume_name(&sandbox, &bob) + ); +} + +#[test] +fn host_uid_scope_is_opt_in_through_docker_config() { + let config: DockerComputeConfig = toml::from_str("scope_by_host_uid = true").unwrap(); + assert!(config.scope_by_host_uid); + assert_eq!(docker_sandbox_namespace(&config, 1000), "uid-1000-default"); +} + +#[tokio::test] +async fn host_uid_scope_rejects_legacy_containers_and_volumes() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + + for (containers, volumes, allowed) in [ + ("[]", r#"{"Volumes":[]}"#, true), + (r#"[{"Id":"legacy"}]"#, r#"{"Volumes":[]}"#, false), + ( + "[]", + r#"{"Volumes":[{"Name":"legacy","Driver":"local","Mountpoint":"/tmp/legacy","Labels":{},"Options":{},"Scope":"local"}]}"#, + false, + ), + ] { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + for (path, body) in [("/containers/json", containers), ("/volumes", volumes)] { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut request = [0_u8; 4096]; + let length = socket.read(&mut request).await.unwrap(); + assert!( + String::from_utf8_lossy(&request[..length]).contains(path), + "unexpected Docker API request" + ); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + socket.write_all(response.as_bytes()).await.unwrap(); + } + }); + let docker = Docker::connect_with_http( + &format!("http://{address}"), + 2, + bollard::API_DEFAULT_VERSION, + ) + .unwrap(); + let result = reject_unscoped_docker_resources(&docker, "default").await; + tokio::time::timeout(Duration::from_secs(2), server) + .await + .unwrap() + .unwrap(); + assert_eq!(result.is_ok(), allowed); + if let Err(error) = result { + assert!( + error + .to_string() + .contains("empty unscoped Docker namespace"), + "{error}" + ); + } + } +} + #[test] fn build_container_create_body_replaces_inherited_cmd_with_sandbox_bootstrap() { let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 3d5680301d..6f051d44ea 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1320,8 +1320,11 @@ socket_path = "/var/run/docker.sock" default_image = "nvcr.io/nvidia/base/ubuntu:24.04" # Canonical values: always | if_not_present | never. `newer` is Podman-only. image_pull_policy = "if_not_present" -# Value assigned to the openshell.sandbox_namespace label on sandbox containers. +# Value assigned to the openshell.ai/sandbox-namespace label on sandbox containers. sandbox_label = "docker-dev" +# Optional: scope Docker containers, volumes, and private state by host UID. +# Enable on each per-user gateway before creating sandboxes. +# scope_by_host_uid = true # host (default) or bridge networking for the supervisor. # Operator-owned; sandbox driver config cannot override this choice. supervisor_network_mode = "host" @@ -1363,6 +1366,20 @@ provider_spiffe_workload_api_socket = "/run/spire/agent.sock" Use `sandbox_label` for Docker configurations. +When separate OS users run gateways against one Docker daemon, set +`scope_by_host_uid = true` under `[openshell.drivers.docker]` on each gateway. +The driver prefixes `sandbox_label` with the gateway process's effective host +UID. Container lookups, startup cleanup, named volumes, and private state then +use that scoped value. The default is `false`, preserving existing resource +names. The opt-in fails at startup if any OpenShell containers or volumes still +use the unscoped `sandbox_label`; delete those sandboxes with the old gateway +before enabling it. Keep the gateway service under the same host UID afterward. + +Each gateway still needs a distinct loopback bind port and its own client +registration. Package-managed port selection is not yet available. This +separates users through OpenShell's gateway and driver; a user who can access +the Docker daemon directly can inspect or alter other users' containers. + `supervisor_network_mode` accepts `host` (the default), `bridge`, or `auto`. `auto` also selects host; the driver does not infer a mode from the workload runtime. The workload always uses `network=none`. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 9e56f7117c..b781510108 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -154,6 +154,13 @@ when neither that CA nor the package-managed local CA is available. Kubernetes projects only the gateway CA from a Secret into supervisor Pods; supervisors authenticate gateway RPCs with sandbox bearer tokens. +For per-user Docker gateways sharing one daemon, check each user's +`[openshell.drivers.docker].scope_by_host_uid` setting and the UID running that +gateway. The option is off by default. If startup reports resources under the +unscoped `sandbox_label`, use the original gateway to delete those sandboxes +before enabling the option. Follow the Docker section of the published gateway +configuration guide for the port and Docker daemon trust limits. + Custom names use `[openshell.drivers.].socket_path`. A launch-time `--compute-driver-socket` override may also use `docker`, `podman`, `kubernetes`, or `vm`; the endpoint then takes precedence over built-in construction. First-party standalone drivers require the socket parent directory to be owned by the driver's effective UID, force its mode to `0700`, create the socket with mode `0600`, and accept only peers with that same UID. Check the parent and socket separately with `stat`; a gateway running under a different UID cannot connect even when filesystem permissions or group membership would otherwise allow it. Operator-supplied drivers must provide equivalent access control appropriate to their implementation. Check gateway logs for connection errors, `GetCapabilities` failures, missing peer metadata, protocol-major mismatch, unmet required capabilities, or an unexpected advertised driver name. `openshell gateway info` reports successful startup negotiations. The advertised name is diagnostic metadata; negotiated features control optional behavior. The gateway does not create or supervise operator-supplied driver processes or sockets. For the Kubernetes Secrets credential driver, every provider credential lives in From 06d162c1e642fe95f117ad02488605e93fa9fdcc Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 18:24:50 +0000 Subject: [PATCH 2/2] fix(docker): scope container names and verify pending deletes Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-docker/README.md | 13 +- crates/openshell-driver-docker/src/lib.rs | 153 ++++++++++----- crates/openshell-driver-docker/src/tests.rs | 193 ++++++++++++++++++- docs/how-it-works/gateways/configuration.mdx | 3 +- skills/debug-openshell-cluster/SKILL.md | 2 +- 5 files changed, 307 insertions(+), 57 deletions(-) diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index f620c0bf8b..db680daa03 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -29,12 +29,13 @@ See [resource admission configuration](../../docs/how-it-works/gateways/configur driver's existing sandbox namespace from the gateway process's effective host UID and `sandbox_label` (`uid--`). The same namespace flows through workload, supervisor, staging, and identity container labels; container -queries and startup cleanup; volume labels and names; and private host state. -The default remains unscoped for compatibility. Opt-in fails before driver -reconciliation when OpenShell containers or volumes still have the unscoped -label, avoiding an invisible legacy sandbox after a config change. Delete old -sandboxes through the original gateway before enabling the option. The scope -separates OS users through OpenShell, but Docker daemon access can bypass it. +queries and startup cleanup; container and volume names and labels; and +private host state. The default remains unscoped for compatibility. Opt-in +fails before driver reconciliation when OpenShell containers or volumes still +have the unscoped label, avoiding an invisible legacy sandbox after a config +change. Delete old sandboxes through the original gateway, then stop that +gateway before enabling the option. The scope separates OS users through +OpenShell, but Docker daemon access can bypass it. The driver creates two containers for each sandbox: diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index efb66647cf..f6e92ca365 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -393,11 +393,64 @@ async fn reject_unscoped_docker_resources(docker: &Docker, namespace: &str) -> C Ok(()) } +async fn remove_owned_pending_container( + docker: &Docker, + name: &str, + sandbox_id: &str, + namespace: &str, +) -> Result { + let inspected = match docker.inspect_container(name, None).await { + Ok(container) => container, + Err(error) if is_not_found_error(&error) => return Ok(false), + Err(error) => return Err(internal_status("inspect pending Docker container", error)), + }; + let labels = inspected + .config + .as_ref() + .and_then(|config| config.labels.as_ref()); + let owned = labels.is_some_and(|labels| { + labels.get(LABEL_MANAGED_BY).map(String::as_str) == Some(LABEL_MANAGED_BY_VALUE) + && labels.get(LABEL_SANDBOX_NAMESPACE).map(String::as_str) == Some(namespace) + && labels.get(LABEL_SANDBOX_ID).map(String::as_str) == Some(sandbox_id) + && labels.get(LABEL_ISOLATION_ROLE).map(String::as_str) + == Some(LABEL_ISOLATION_ROLE_SANDBOX) + && labels.get(LABEL_ISOLATION_BACKEND).map(String::as_str) + == Some(LABEL_ISOLATION_BACKEND_OPEN_SHELL) + }); + if !owned { + return Err(Status::failed_precondition(format!( + "Docker container '{name}' is not owned by this sandbox namespace; refusing to remove it" + ))); + } + let container_id = inspected + .id + .as_deref() + .filter(|id| !id.is_empty()) + .ok_or_else(|| Status::failed_precondition("pending Docker container has no ID"))?; + match docker + .remove_container( + container_id, + Some(RemoveContainerOptionsBuilder::default().force(true).build()), + ) + .await + { + Ok(()) => Ok(true), + Err(error) if is_not_found_error(&error) => Ok(false), + Err(error) => Err(internal_status("delete pending Docker container", error)), + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct DockerGuestTlsPaths { pub(crate) ca: PathBuf, } +#[derive(Debug, Clone, Copy)] +enum DockerContainerNaming { + Legacy, + ScopedByHostUid, +} + #[derive(Debug, Clone)] struct DockerDriverRuntimeConfig { allow_driver_config: bool, @@ -405,6 +458,7 @@ struct DockerDriverRuntimeConfig { default_image: String, image_pull_policy: ImagePullPolicy, sandbox_namespace: String, + container_naming: DockerContainerNaming, stop_timeout_secs: u32, log_level: String, sandbox_binary: Arc>, @@ -432,6 +486,17 @@ struct DockerDriverRuntimeConfig { network_trust: Option, } +impl DockerDriverRuntimeConfig { + fn container_name(&self, sandbox: &DriverSandbox) -> String { + if matches!(self.container_naming, DockerContainerNaming::Legacy) { + return container_name_for_sandbox(sandbox); + } + let digest = format!("{:x}", Sha256::digest(self.sandbox_namespace.as_bytes())); + let prefix = format!("{CONTAINER_NAME_PREFIX}{}-", &digest[..24]); + container_name_for_sandbox_with_prefix(sandbox, &prefix) + } +} + #[derive(Debug, Clone, Copy)] struct DockerGpuRuntimeCapabilities { cdi_supported: bool, @@ -1073,6 +1138,11 @@ impl DockerComputeDriver { default_image: docker_config.default_image.clone(), image_pull_policy: docker_config.image_pull_policy, sandbox_namespace, + container_naming: if docker_config.scope_by_host_uid { + DockerContainerNaming::ScopedByHostUid + } else { + DockerContainerNaming::Legacy + }, stop_timeout_secs: DEFAULT_STOP_TIMEOUT_SECS, log_level: gateway_log_level.to_string(), sandbox_binary, @@ -1770,7 +1840,7 @@ impl DockerComputeDriver { )); } - let container_name = container_name_for_sandbox(sandbox); + let container_name = self.config.container_name(sandbox); let gpu_devices = match self .resolve_gpu_cdi_devices( validated.gpu_requirements, @@ -2350,41 +2420,32 @@ impl DockerComputeDriver { .await? else { if let Some(record) = pending { - let container_name = container_name_for_sandbox(&record.sandbox); - match self - .docker - .remove_container( - &container_name, - Some(RemoveContainerOptionsBuilder::default().force(true).build()), + let container_name = self.config.container_name(&record.sandbox); + let removed = remove_owned_pending_container( + &self.docker, + &container_name, + &record.sandbox.id, + &self.config.sandbox_namespace, + ) + .await?; + self.clear_runtime_failure(&record.sandbox.id).await; + if removed { + remove_docker_channel_volume_by_id( + &self.docker, + &record.sandbox.id, + &self.config, ) - .await - { - Ok(()) => { - self.clear_runtime_failure(&record.sandbox.id).await; - remove_docker_channel_volume_by_id( - &self.docker, - &record.sandbox.id, - &self.config, - ) - .await?; - cleanup_docker_boundary_state(&record.sandbox, &self.config); - return Ok(true); - } - Err(err) if is_not_found_error(&err) => { - self.clear_runtime_failure(&record.sandbox.id).await; - let _ = remove_docker_channel_volume_by_id( - &self.docker, - &record.sandbox.id, - &self.config, - ) - .await; - cleanup_docker_boundary_state(&record.sandbox, &self.config); - return Ok(true); - } - Err(err) => { - return Err(internal_status("delete docker sandbox container", err)); - } + .await?; + } else { + let _ = remove_docker_channel_volume_by_id( + &self.docker, + &record.sandbox.id, + &self.config, + ) + .await; } + cleanup_docker_boundary_state(&record.sandbox, &self.config); + return Ok(true); } if !sandbox_id.is_empty() { self.stop_control_process(sandbox_id).await; @@ -5112,7 +5173,7 @@ async fn stage_docker_supervisor_bundle( config: &DockerDriverRuntimeConfig, archive: Vec, ) -> Result<(), Status> { - let stager_name = format!("{}-supervisor-stage", container_name_for_sandbox(sandbox)); + let stager_name = format!("{}-supervisor-stage", config.container_name(sandbox)); let _ = docker .remove_container( &stager_name, @@ -5422,7 +5483,7 @@ async fn spawn_docker_control_process( let workspace_root = tokio::fs::read_to_string(directory.join(WORKSPACE_ROOT_FILE)) .await .map_err(|error| Status::internal(format!("read Docker workspace root: {error}")))?; - let supervisor_name = format!("{}-supervisor", container_name_for_sandbox(sandbox)); + let supervisor_name = format!("{}-supervisor", config.container_name(sandbox)); let _ = docker .remove_container( &supervisor_name, @@ -6797,27 +6858,27 @@ const MAX_CONTAINER_NAME_LEN: usize = 200; const CONTAINER_NAME_PREFIX: &str = "openshell-"; fn container_name_for_sandbox(sandbox: &DriverSandbox) -> String { + container_name_for_sandbox_with_prefix(sandbox, CONTAINER_NAME_PREFIX) +} + +fn container_name_for_sandbox_with_prefix(sandbox: &DriverSandbox, prefix: &str) -> String { let id_suffix = sanitize_docker_name(&sandbox.id); let workspace = sanitize_docker_name(&sandbox.workspace); let name = sanitize_docker_name(&sandbox.name); - // Format: openshell-{workspace}--{name}-{id} - // The workspace and id are never truncated — they ensure uniqueness. - // Only the sandbox name portion is truncated when the total exceeds - // MAX_CONTAINER_NAME_LEN. - + // The sandbox name portion is truncated to fit Docker's name limit. if name.is_empty() { - let mut base = format!("{CONTAINER_NAME_PREFIX}{workspace}---{id_suffix}"); + let mut base = format!("{prefix}{workspace}---{id_suffix}"); if base.len() > MAX_CONTAINER_NAME_LEN { base.truncate(MAX_CONTAINER_NAME_LEN); } return trim_container_name_tail(base); } - // Reserve space for fixed parts: prefix + workspace + "--" + "-" + id - let reserved = CONTAINER_NAME_PREFIX.len() + workspace.len() + 2 + 1 + id_suffix.len(); + // Reserve space for fixed parts: prefix + workspace + "--" + "-" + id. + let reserved = prefix.len() + workspace.len() + 2 + 1 + id_suffix.len(); if reserved >= MAX_CONTAINER_NAME_LEN { - let mut base = format!("{CONTAINER_NAME_PREFIX}{workspace}---{id_suffix}"); + let mut base = format!("{prefix}{workspace}---{id_suffix}"); base.truncate(MAX_CONTAINER_NAME_LEN); return trim_container_name_tail(base); } @@ -6828,7 +6889,7 @@ fn container_name_for_sandbox(sandbox: &DriverSandbox) -> String { } else { name }; - format!("{CONTAINER_NAME_PREFIX}{workspace}--{truncated_name}-{id_suffix}") + format!("{prefix}{workspace}--{truncated_name}-{id_suffix}") } /// Normalize an image reference for a pull request by appending `:latest` diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index e2c702b60b..52fd2e64d5 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -319,6 +319,7 @@ fn runtime_config() -> DockerDriverRuntimeConfig { default_image: "image:latest".to_string(), image_pull_policy: ImagePullPolicy::IfNotPresent, sandbox_namespace: "default".to_string(), + container_naming: DockerContainerNaming::Legacy, stop_timeout_secs: DEFAULT_STOP_TIMEOUT_SECS, log_level: "info".to_string(), sandbox_binary: Arc::new(b"\x7fELFtest".to_vec()), @@ -2619,6 +2620,23 @@ fn host_uid_scope_separates_docker_resources() { ); } +#[test] +fn scoped_container_names_separate_identical_sandbox_ids() { + let sandbox = test_sandbox(); + let mut alice = runtime_config(); + alice.sandbox_namespace = "uid-1000-default".to_string(); + alice.container_naming = DockerContainerNaming::ScopedByHostUid; + let mut bob = runtime_config(); + bob.sandbox_namespace = "uid-1001-default".to_string(); + bob.container_naming = DockerContainerNaming::ScopedByHostUid; + + assert_ne!(alice.container_name(&sandbox), bob.container_name(&sandbox)); + assert_eq!( + runtime_config().container_name(&sandbox), + container_name_for_sandbox(&sandbox) + ); +} + #[test] fn host_uid_scope_is_opt_in_through_docker_config() { let config: DockerComputeConfig = toml::from_str("scope_by_host_uid = true").unwrap(); @@ -2646,9 +2664,30 @@ async fn host_uid_scope_rejects_legacy_containers_and_volumes() { let (mut socket, _) = listener.accept().await.unwrap(); let mut request = [0_u8; 4096]; let length = socket.read(&mut request).await.unwrap(); - assert!( - String::from_utf8_lossy(&request[..length]).contains(path), - "unexpected Docker API request" + let request = String::from_utf8_lossy(&request[..length]); + assert!(request.contains(path), "unexpected Docker API request"); + let request_path = request + .lines() + .next() + .unwrap() + .split_whitespace() + .nth(1) + .unwrap(); + let url = Url::parse(&format!("http://localhost{request_path}")).unwrap(); + let filters: HashMap> = serde_json::from_str( + url.query_pairs() + .find(|(key, _)| key == "filters") + .expect("Docker label filters") + .1 + .as_ref(), + ) + .unwrap(); + assert_eq!( + filters.get("label"), + Some(&vec![ + format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE}"), + format!("{LABEL_SANDBOX_NAMESPACE}=default"), + ]) ); let response = format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", @@ -2680,6 +2719,154 @@ async fn host_uid_scope_rejects_legacy_containers_and_volumes() { } } +#[tokio::test] +async fn pending_delete_uses_inspected_container_id() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + for (method, path, status, body) in [ + ( + "GET", + "/containers/pending-owned/json", + "200 OK", + r#"{"Id":"immutable-owned-id","Config":{"Labels":{"openshell.ai/managed-by":"openshell","openshell.ai/sandbox-id":"pending-owned","openshell.ai/sandbox-namespace":"uid-1000-default","openshell.ai/isolation-role":"sandbox","openshell.ai/isolation-backend":"openshell-sandbox"}}}"#, + ), + ( + "DELETE", + "/containers/immutable-owned-id?", + "204 No Content", + "", + ), + ] { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut buffer = [0_u8; 4096]; + let length = socket.read(&mut buffer).await.unwrap(); + let request = String::from_utf8_lossy(&buffer[..length]); + let first_line = request.lines().next().unwrap(); + assert!(first_line.starts_with(method), "{first_line}"); + assert!(first_line.contains(path), "{first_line}"); + let response = format!( + "HTTP/1.1 {status}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + socket.write_all(response.as_bytes()).await.unwrap(); + } + }); + let docker = Docker::connect_with_http( + &format!("http://{address}"), + 2, + bollard::API_DEFAULT_VERSION, + ) + .unwrap(); + assert!( + remove_owned_pending_container( + &docker, + "pending-owned", + "pending-owned", + "uid-1000-default" + ) + .await + .unwrap() + ); + tokio::time::timeout(Duration::from_secs(2), server) + .await + .unwrap() + .unwrap(); +} + +#[tokio::test] +async fn pending_delete_rejects_container_from_another_namespace() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let (stop_sender, mut stop_receiver) = oneshot::channel::<()>(); + let server = tokio::spawn(async move { + let mut requests = Vec::new(); + loop { + let accepted = tokio::select! { + _ = &mut stop_receiver => break, + accepted = listener.accept() => accepted.unwrap(), + }; + let (mut socket, _) = accepted; + let mut buffer = [0_u8; 4096]; + let length = socket.read(&mut buffer).await.unwrap(); + let request = String::from_utf8_lossy(&buffer[..length]); + let first_line = request.lines().next().unwrap().to_string(); + let (status, body) = if first_line.contains("/containers/json?") { + ("200 OK", "[]") + } else if first_line.starts_with("GET ") && first_line.contains("/json") { + ( + "200 OK", + r#"{"Id":"foreign-id","Config":{"Labels":{"openshell.ai/managed-by":"openshell","openshell.ai/sandbox-id":"pending-foreign","openshell.ai/sandbox-namespace":"uid-1001-default","openshell.ai/isolation-role":"sandbox","openshell.ai/isolation-backend":"openshell-sandbox"}}}"#, + ) + } else if first_line.starts_with("DELETE ") && first_line.contains("/containers/") { + ("204 No Content", "") + } else if first_line.starts_with("DELETE ") && first_line.contains("/volumes/") { + ("404 Not Found", r#"{"message":"No such volume"}"#) + } else { + panic!("unexpected Docker API request: {first_line}"); + }; + requests.push(first_line); + let response = format!( + "HTTP/1.1 {status}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + socket.write_all(response.as_bytes()).await.unwrap(); + } + requests + }); + + let mut config = runtime_config(); + config.sandbox_namespace = "uid-1000-default".to_string(); + config.container_naming = DockerContainerNaming::ScopedByHostUid; + let mut driver = test_driver_with_config(config); + driver.docker = Arc::new( + Docker::connect_with_http( + &format!("http://{address}"), + 2, + bollard::API_DEFAULT_VERSION, + ) + .unwrap(), + ); + let mut sandbox = test_sandbox(); + sandbox.id = "pending-foreign".to_string(); + driver.pending.lock().await.insert( + sandbox.id.clone(), + PendingSandboxRecord { + sandbox: sandbox.clone(), + task: None, + }, + ); + + let result = tokio::time::timeout( + Duration::from_secs(2), + driver.delete_sandbox_inner(&sandbox.id, &sandbox.name), + ) + .await + .unwrap(); + let _ = stop_sender.send(()); + let requests = server.await.unwrap(); + assert_eq!(result.unwrap_err().code(), tonic::Code::FailedPrecondition); + assert!( + requests + .iter() + .any(|request| request.contains("/containers/json?")) + ); + assert!( + requests + .iter() + .any(|request| request.contains("/containers/") && request.contains("/json")) + ); + assert!( + !requests + .iter() + .any(|request| request.starts_with("DELETE ") && request.contains("/containers/")) + ); +} + #[test] fn build_container_create_body_replaces_inherited_cmd_with_sandbox_bootstrap() { let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 6f051d44ea..cc0d916ef6 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1373,7 +1373,8 @@ UID. Container lookups, startup cleanup, named volumes, and private state then use that scoped value. The default is `false`, preserving existing resource names. The opt-in fails at startup if any OpenShell containers or volumes still use the unscoped `sandbox_label`; delete those sandboxes with the old gateway -before enabling it. Keep the gateway service under the same host UID afterward. +and stop it before enabling the option. Keep the gateway service under the same +host UID afterward. Each gateway still needs a distinct loopback bind port and its own client registration. Package-managed port selection is not yet available. This diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index b781510108..e1de546a89 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -158,7 +158,7 @@ For per-user Docker gateways sharing one daemon, check each user's `[openshell.drivers.docker].scope_by_host_uid` setting and the UID running that gateway. The option is off by default. If startup reports resources under the unscoped `sandbox_label`, use the original gateway to delete those sandboxes -before enabling the option. Follow the Docker section of the published gateway +and stop it before enabling the option. Follow the Docker section of the published gateway configuration guide for the port and Docker daemon trust limits. Custom names use `[openshell.drivers.].socket_path`. A launch-time `--compute-driver-socket` override may also use `docker`, `podman`, `kubernetes`, or `vm`; the endpoint then takes precedence over built-in construction. First-party standalone drivers require the socket parent directory to be owned by the driver's effective UID, force its mode to `0700`, create the socket with mode `0600`, and accept only peers with that same UID. Check the parent and socket separately with `stat`; a gateway running under a different UID cannot connect even when filesystem permissions or group membership would otherwise allow it. Operator-supplied drivers must provide equivalent access control appropriate to their implementation. Check gateway logs for connection errors, `GetCapabilities` failures, missing peer metadata, protocol-major mismatch, unmet required capabilities, or an unexpected advertised driver name. `openshell gateway info` reports successful startup negotiations. The advertised name is diagnostic metadata; negotiated features control optional behavior. The gateway does not create or supervise operator-supplied driver processes or sockets.