From 80e508ac543cb68b4d1496b59f5d4771d2d6b6b4 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:52:23 -0700 Subject: [PATCH 1/6] fix(kubernetes): bind bootstrap to runtime identity Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- .agents/skills/helm-dev-environment/SKILL.md | 4 +- architecture/compute-runtimes.md | 26 ++-- architecture/gateway.md | 12 +- crates/openshell-driver-docker/src/lib.rs | 4 +- crates/openshell-driver-kubernetes/README.md | 9 +- .../openshell-driver-kubernetes/src/driver.rs | 80 +++++++--- .../openshell-driver-kubernetes/src/grpc.rs | 22 +-- crates/openshell-driver-mxc/src/grpc.rs | 2 +- crates/openshell-driver-podman/src/grpc.rs | 4 +- crates/openshell-driver-vm/src/driver.rs | 4 +- .../src/auth/compute_driver.rs | 34 +++- crates/openshell-server/src/auth/principal.rs | 8 +- crates/openshell-server/src/compute/mod.rs | 146 +++++++++++++++--- crates/openshell-server/src/grpc/auth_rpc.rs | 64 +++++++- crates/openshell-server/src/test_support.rs | 4 +- docs/kubernetes/access-control.mdx | 2 +- docs/reference/gateway-auth.mdx | 2 +- docs/reference/gateway-config.mdx | 2 +- proto/compute_driver.proto | 16 +- skills/debug-openshell-cluster/SKILL.md | 5 + 20 files changed, 355 insertions(+), 95 deletions(-) diff --git a/.agents/skills/helm-dev-environment/SKILL.md b/.agents/skills/helm-dev-environment/SKILL.md index fddbe8f1e4..228e12988a 100644 --- a/.agents/skills/helm-dev-environment/SKILL.md +++ b/.agents/skills/helm-dev-environment/SKILL.md @@ -359,7 +359,9 @@ OpenShell mounts the SPIFFE CSI Workload API socket at grants. Supervisor-to-gateway authentication remains on the Kubernetes ServiceAccount bootstrap and gateway-minted sandbox JWT path; the selected Kubernetes compute driver validates the projected token before the gateway -mints its JWT. +mints its JWT. The driver also returns the runtime identity recorded during +provisioning, and the gateway rejects bootstrap when that identity does not +match the durable sandbox record. ### Vault Credential Driver diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 4fd27e2b9c..507350011b 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -506,19 +506,19 @@ The Kubernetes driver's `AuthenticateSandbox` implementation applies its named until the first watcher update. It validates the projected token with Kubernetes `TokenReview`, checks the live -pod UID, and verifies the pod's controlling Sandbox CR UID and sandbox ID before -returning the identity to the gateway. These checks rely on an ownership -invariant. In shared and managed modes, the Kubernetes driver and its trusted -Agent Sandbox controller exclusively administer the sandbox namespace, Sandbox -CRs, sandbox pods, and configured sandbox ServiceAccount. Other principals must -not create or mutate those resources or use that ServiceAccount. In operator -mode, the platform operator retains -namespace lifecycle ownership, but must preserve the same exclusive control of -Sandbox CRs and the pods and ServiceAccount used for sandbox token bootstrap. -An allowlisted namespace is therefore a trust grant, not a tenant isolation -boundary. Kubernetes owner references alone do not prove which controller -created a pod, so admitting principals that can fabricate that resource chain -would allow them to claim an existing sandbox identity. +pod UID, and verifies the pod's controlling Sandbox CR UID and sandbox ID. The +driver returns both the sandbox ID and an opaque runtime identity derived from +the namespace, immutable Sandbox CR UID, and authenticated supervisor Pod UID. The gateway records that runtime +identity when provisioning succeeds and requires an exact match before issuing +a sandbox JWT. This correlates credential authentication with the durable +runtime record rather than authorizing from the sandbox ID alone. + +Shared and managed modes still reserve the sandbox namespace, Sandbox CRs, +sandbox pods, and configured sandbox ServiceAccount for the Kubernetes driver +and trusted Agent Sandbox controller. In operator mode, the platform operator +retains namespace lifecycle ownership and must preserve the same control of +those resources. An allowlisted namespace is a trust grant, not a tenant +isolation boundary. ### Credential Driver Integration diff --git a/architecture/gateway.md b/architecture/gateway.md index f237fce0c8..4113420a63 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -317,11 +317,13 @@ Sandbox secrets are gateway-signed JWTs bound to a single sandbox ID. Docker, Podman, and VM drivers deliver the initial token through supervisor-only runtime material; Kubernetes supervisors exchange a projected ServiceAccount token through `IssueSandboxToken`. The gateway delegates that opaque credential -to the selected compute driver's `AuthenticateSandbox` RPC. A capable driver is -trusted to return the authenticated sandbox ID, while the gateway still requires -a matching durable sandbox record before minting a JWT. The Kubernetes driver -uses its own named configuration to run TokenReview and verify the live pod and -controlling Sandbox CR. The bootstrap path accepts +to the selected compute driver's `AuthenticateSandbox` RPC. A capable driver +returns the authenticated sandbox ID and an opaque runtime identity. The +gateway requires both a matching durable sandbox record and the exact +driver/runtime identity recorded at provisioning before minting a JWT. The +Kubernetes driver uses its own named configuration to run TokenReview and +verify the live pod and controlling Sandbox CR. Its runtime identity binds the +namespace, immutable Sandbox CR UID, and supervisor Pod UID. The bootstrap path accepts both `agents.x-k8s.io/v1beta1` ownerReferences from newer Agent Sandbox controllers and `agents.x-k8s.io/v1alpha1` ownerReferences from existing deployments. Supervisors renew gateway JWTs in memory before expiry only while diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 92d685cf41..bd19d7f8c3 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -3092,7 +3092,7 @@ impl ComputeDriver for DockerComputeDriver { .sandbox .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; self.create_sandbox_inner(&sandbox).await?; - span_status.finish(Ok(Response::new(CreateSandboxResponse {}))) + span_status.finish(Ok(Response::new(CreateSandboxResponse::default()))) } #[tracing::instrument( @@ -3152,7 +3152,7 @@ impl ComputeDriver for DockerComputeDriver { } self.publish_container_snapshot(&request.sandbox_id, &request.name) .await?; - Ok(Response::new(StartSandboxResponse {})) + Ok(Response::new(StartSandboxResponse::default())) } #[tracing::instrument( diff --git a/crates/openshell-driver-kubernetes/README.md b/crates/openshell-driver-kubernetes/README.md index c24cd9c9dd..0ca05b0806 100644 --- a/crates/openshell-driver-kubernetes/README.md +++ b/crates/openshell-driver-kubernetes/README.md @@ -140,9 +140,12 @@ mount attaches an existing PVC under `/sandbox`, which skips the default PVC. Both Pods set `automountServiceAccountToken: false`. The supervisor receives an explicit audience-bound projected token for the one-shot `IssueSandboxToken` -exchange. The driver verifies that token and the gateway returns the -sandbox-scoped JWT used by the supervisor session. The sandbox Pod receives -neither token. +exchange. The driver verifies that token and returns an opaque runtime identity +derived from the namespace, immutable Sandbox resource UID, and supervisor Pod +UID. The gateway +requires that identity to match the value recorded during provisioning before +returning the sandbox-scoped JWT used by the supervisor session. The sandbox +Pod receives neither token. The gateway uses the supervisor relay for connect, exec, logs, and file sync. Sandbox Pods do not need direct external ingress for SSH. diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index cec9b20c5d..493efa87d2 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -777,7 +777,10 @@ impl KubernetesComputeDriver { } /// Authenticate the projected `ServiceAccount` token used by a sandbox pod. - pub async fn authenticate_sandbox(&self, credential: &str) -> Result { + pub async fn authenticate_sandbox( + &self, + credential: &str, + ) -> Result<(String, String), tonic::Status> { let reviews: Api = Api::all(self.client.clone()); let review = TokenReview { metadata: ObjectMeta::default(), @@ -831,7 +834,15 @@ impl KubernetesComputeDriver { })?.ok_or_else(|| tonic::Status::permission_denied("sandbox owner not found"))?; validate_sandbox_owner_identity(&owner, &sandbox_id, &sandbox)?; require_proxy_control_authentication(via_proxy_control)?; - Ok(sandbox_id) + let resource_uid = sandbox + .metadata + .uid + .as_deref() + .ok_or_else(|| tonic::Status::permission_denied("sandbox owner has no UID"))?; + Ok(( + sandbox_id, + kubernetes_runtime_identity(&identity.namespace, resource_uid, &identity.pod_uid), + )) } #[allow(clippy::result_large_err)] @@ -1651,14 +1662,17 @@ impl KubernetesComputeDriver { sandbox.name = %sandbox.name, ) )] - pub async fn create_sandbox(&self, sandbox: &Sandbox) -> Result<(), KubernetesDriverError> { + pub async fn create_sandbox(&self, sandbox: &Sandbox) -> Result { let span_status = openshell_otel::ErrorStatusGuard::current(); let result = self.create_sandbox_inner(sandbox).await; span_status.finish(result) } #[allow(clippy::similar_names)] - async fn create_sandbox_inner(&self, sandbox: &Sandbox) -> Result<(), KubernetesDriverError> { + async fn create_sandbox_inner( + &self, + sandbox: &Sandbox, + ) -> Result { let gpu_requirements = sandbox .spec .as_ref() @@ -1840,7 +1854,7 @@ impl KubernetesComputeDriver { ))); } }; - if let Err(error) = self + let runtime_identity = match self .create_sandbox_runtime_companions( sandbox, &target_namespace, @@ -1856,14 +1870,17 @@ impl KubernetesComputeDriver { ) .await { - warn!(sandbox_id = %sandbox.id, %error, "sandbox-runtime provisioning failed; rolling back Sandbox CR"); - let _ = agent_sandbox_api - .api - .delete(&kube_name, &DeleteParams::default()) - .await; - return Err(error); - } - Ok(()) + Ok(runtime_identity) => runtime_identity, + Err(error) => { + warn!(sandbox_id = %sandbox.id, %error, "sandbox-runtime provisioning failed; rolling back Sandbox CR"); + let _ = agent_sandbox_api + .api + .delete(&kube_name, &DeleteParams::default()) + .await; + return Err(error); + } + }; + Ok(runtime_identity) } async fn create_sandbox_runtime_fence( @@ -2150,7 +2167,7 @@ impl KubernetesComputeDriver { agent_gid: u32, main_process_spec: &str, log_level: &str, - ) -> Result<(), KubernetesDriverError> { + ) -> Result { let cr_uid = sandbox_cr.metadata.uid.as_deref().ok_or_else(|| { KubernetesDriverError::Message("created Sandbox CR has no UID".to_string()) })?; @@ -2419,7 +2436,7 @@ impl KubernetesComputeDriver { api_version: "v1".to_string(), kind: "Pod".to_string(), name: names.supervisor_pod.clone(), - uid: supervisor_uid, + uid: supervisor_uid.clone(), controller: Some(false), block_owner_deletion: Some(false), }, @@ -2465,7 +2482,11 @@ impl KubernetesComputeDriver { // boundary PID 1 is running at this point; the agent process cannot // start until control attaches and confirms enforcement. Reconcile // removes the marker after the supervisor Pod becomes Ready. - Ok(()) + Ok(kubernetes_runtime_identity( + namespace, + cr_uid, + &supervisor_uid, + )) } #[allow(clippy::too_many_arguments, clippy::similar_names)] @@ -2804,7 +2825,7 @@ impl KubernetesComputeDriver { sandbox_id: &str, generation_id: &str, launch_authentication: &[u8], - ) -> Result<(), KubernetesDriverError> { + ) -> Result { let span_status = openshell_otel::ErrorStatusGuard::current(); let result = Box::pin(self.start_sandbox_runtime_generation( sandbox_id, @@ -2821,7 +2842,7 @@ impl KubernetesComputeDriver { sandbox_id: &str, encoded_generation: &str, encoded_authentication: &[u8], - ) -> Result<(), KubernetesDriverError> { + ) -> Result { let generation = openshell_core::sandbox_generation::SandboxGenerationId::parse( encoded_generation.to_string(), ) @@ -2864,7 +2885,18 @@ impl KubernetesComputeDriver { { self.complete_sandbox_runtime_bootstrap(&lookup_api, &object) .await; - return Ok(()); + let cr_uid = object.metadata.uid.as_deref().ok_or_else(|| { + KubernetesDriverError::Message("sandbox resource has no UID".to_string()) + })?; + let supervisor_uid = required_sandbox_annotation( + &object, + ANNOTATION_SANDBOX_RUNTIME_SUPERVISOR_UID, + )?; + return Ok(kubernetes_runtime_identity( + namespace, + cr_uid, + &supervisor_uid, + )); } } @@ -3057,7 +3089,11 @@ impl KubernetesComputeDriver { } return Err(error); } - Ok(()) + Ok(kubernetes_runtime_identity( + &namespace, + cr_uid, + &supervisor_uid, + )) } async fn prepare_sandbox_stop( @@ -4589,6 +4625,10 @@ fn sandbox_annotations(sandbox: &Sandbox) -> BTreeMap { annotations } +fn kubernetes_runtime_identity(namespace: &str, resource_uid: &str, pod_uid: &str) -> String { + format!("kubernetes://{namespace}/{resource_uid}/{pod_uid}") +} + fn sandbox_id_from_object(obj: &DynamicObject) -> Result { if let Some(annotations) = obj.metadata.annotations.as_ref() && let Some(id) = annotations.get(LABEL_SANDBOX_ID) diff --git a/crates/openshell-driver-kubernetes/src/grpc.rs b/crates/openshell-driver-kubernetes/src/grpc.rs index 3427b81f3b..ccb3a03357 100644 --- a/crates/openshell-driver-kubernetes/src/grpc.rs +++ b/crates/openshell-driver-kubernetes/src/grpc.rs @@ -62,8 +62,12 @@ impl ComputeDriver for ComputeDriverService { if credential.is_empty() { return Err(Status::invalid_argument("credential is required")); } - let sandbox_id = self.driver.authenticate_sandbox(&credential).await?; - Ok(Response::new(AuthenticateSandboxResponse { sandbox_id })) + let (sandbox_id, runtime_identity) = + self.driver.authenticate_sandbox(&credential).await?; + Ok(Response::new(AuthenticateSandboxResponse { + sandbox_id, + runtime_identity, + })) }) .await } @@ -153,11 +157,11 @@ impl ComputeDriver for ComputeDriverService { .into_inner() .sandbox .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; - self.driver - .create_sandbox(&sandbox) - .await - .map_err(|e| Status::from(openshell_core::ComputeDriverError::from(e)))?; - Ok(Response::new(CreateSandboxResponse {})) + let runtime_identity = + self.driver.create_sandbox(&sandbox).await.map_err(|e| { + Status::from(openshell_core::ComputeDriverError::from(e)) + })?; + Ok(Response::new(CreateSandboxResponse { runtime_identity })) }), ) .await @@ -195,7 +199,7 @@ impl ComputeDriver for ComputeDriverService { if request.sandbox_id.is_empty() { return Err(Status::invalid_argument("sandbox_id is required")); } - Box::pin(self.driver.start_sandbox( + let runtime_identity = Box::pin(self.driver.start_sandbox( &request.sandbox_id, &request.generation_id, &request.launch_authentication, @@ -204,7 +208,7 @@ impl ComputeDriver for ComputeDriverService { .map_err(|error| { Status::from(openshell_core::ComputeDriverError::from(error)) })?; - Ok(Response::new(StartSandboxResponse {})) + Ok(Response::new(StartSandboxResponse { runtime_identity })) }), ) .await diff --git a/crates/openshell-driver-mxc/src/grpc.rs b/crates/openshell-driver-mxc/src/grpc.rs index 7ddff4dfff..f82a8fd85f 100644 --- a/crates/openshell-driver-mxc/src/grpc.rs +++ b/crates/openshell-driver-mxc/src/grpc.rs @@ -110,7 +110,7 @@ impl ComputeDriver for ComputeDriverService { .sandbox .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; self.backend.create_sandbox(&sandbox).await?; - Ok(Response::new(CreateSandboxResponse {})) + Ok(Response::new(CreateSandboxResponse::default())) } async fn stop_sandbox( diff --git a/crates/openshell-driver-podman/src/grpc.rs b/crates/openshell-driver-podman/src/grpc.rs index a25b6b7e4d..a6d208f707 100644 --- a/crates/openshell-driver-podman/src/grpc.rs +++ b/crates/openshell-driver-podman/src/grpc.rs @@ -150,7 +150,7 @@ impl ComputeDriver for ComputeDriverService { Box::pin(self.driver.create_sandbox(&sandbox)) .await .map_err(Status::from)?; - Ok(Response::new(CreateSandboxResponse {})) + Ok(Response::new(CreateSandboxResponse::default())) }) .await } @@ -192,7 +192,7 @@ impl ComputeDriver for ComputeDriverService { ) .await .map_err(Status::from)?; - Ok(Response::new(StartSandboxResponse {})) + Ok(Response::new(StartSandboxResponse::default())) }) .await } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index fc337ea7ce..fbd4230dec 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -1155,7 +1155,7 @@ impl VmDriver { task.abort(); } - Ok(CreateSandboxResponse {}) + Ok(CreateSandboxResponse::default()) } async fn provision_sandbox( @@ -4368,7 +4368,7 @@ impl ComputeDriver for VmDriver { request.launch_authentication, ) .await?; - Ok(Response::new(StartSandboxResponse {})) + Ok(Response::new(StartSandboxResponse::default())) } async fn delete_sandbox( diff --git a/crates/openshell-server/src/auth/compute_driver.rs b/crates/openshell-server/src/auth/compute_driver.rs index 17f1f98c23..29c14c84c8 100644 --- a/crates/openshell-server/src/auth/compute_driver.rs +++ b/crates/openshell-server/src/auth/compute_driver.rs @@ -42,17 +42,24 @@ impl Authenticator for ComputeDriverAuthenticator { return Ok(None); }; - let sandbox_id = self.compute.authenticate_sandbox(credential).await?; + let authenticated = self.compute.authenticate_sandbox(credential).await?; + let sandbox_id = authenticated.sandbox_id; if sandbox_id.is_empty() { return Err(Status::permission_denied( "compute driver returned an empty sandbox identity", )); } + if authenticated.runtime_identity.is_empty() { + return Err(Status::permission_denied( + "compute driver returned an empty runtime identity", + )); + } Ok(Some(Principal::Sandbox(SandboxPrincipal { sandbox_id, source: SandboxIdentitySource::ComputeDriver { driver_name: self.compute.configured_driver_name().to_string(), + runtime_identity: authenticated.runtime_identity, }, trust_domain: Some("openshell".to_string()), }))) @@ -102,8 +109,12 @@ mod tests { assert_eq!(principal.sandbox_id, "sandbox-a"); assert!(matches!( principal.source, - SandboxIdentitySource::ComputeDriver { ref driver_name } + SandboxIdentitySource::ComputeDriver { + ref driver_name, + ref runtime_identity, + } if driver_name == "external-kubernetes" + && runtime_identity == "test-runtime" )); } @@ -153,6 +164,25 @@ mod tests { assert_eq!(error.code(), Code::PermissionDenied); } + #[tokio::test] + async fn empty_runtime_identity_is_rejected() { + let auth = authenticator(NoopTestDriver::authenticating_sandbox_with_runtime( + "sandbox-a", + "", + )) + .await; + + let error = auth + .authenticate( + &bearer_headers("driver-credential"), + ISSUE_SANDBOX_TOKEN_PATH, + ) + .await + .expect_err("empty runtime identity must fail closed"); + + assert_eq!(error.code(), Code::PermissionDenied); + } + #[tokio::test] async fn driver_authentication_error_propagates() { let auth = authenticator(NoopTestDriver::failing_sandbox_authentication( diff --git a/crates/openshell-server/src/auth/principal.rs b/crates/openshell-server/src/auth/principal.rs index 7d1cc00044..41520f3bb8 100644 --- a/crates/openshell-server/src/auth/principal.rs +++ b/crates/openshell-server/src/auth/principal.rs @@ -83,6 +83,10 @@ pub enum SandboxIdentitySource { BootstrapCert { fingerprint: String }, /// Driver-native credential used to bootstrap a gateway-minted JWT via /// `IssueSandboxToken`. The named compute driver authenticated only the - /// sandbox identity; the gateway still authorizes the exchange. - ComputeDriver { driver_name: String }, + /// sandbox identity and its concrete runtime binding; the gateway still + /// authorizes the exchange against the binding recorded at creation. + ComputeDriver { + driver_name: String, + runtime_identity: String, + }, } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index a0219ab1bb..85c05958df 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -58,6 +58,10 @@ use tonic::transport::Channel; #[cfg(unix)] use tonic::transport::Endpoint; use tonic::{Code, Request, Status}; + +pub const COMPUTE_DRIVER_ANNOTATION: &str = "internal.openshell.ai/compute-driver"; +pub const COMPUTE_RUNTIME_IDENTITY_ANNOTATION: &str = + "internal.openshell.ai/compute-runtime-identity"; #[cfg(unix)] use tower::service_fn; use tracing::{Instrument as _, debug, info, warn}; @@ -782,7 +786,10 @@ impl ComputeRuntime { self.driver_info.supports_sandbox_authentication } - pub(crate) async fn authenticate_sandbox(&self, credential: &str) -> Result { + pub(crate) async fn authenticate_sandbox( + &self, + credential: &str, + ) -> Result { if !self.supports_sandbox_authentication() { return Err(Status::unimplemented( "selected compute driver does not authenticate sandbox credentials", @@ -798,7 +805,7 @@ impl ComputeRuntime { |driver| async move { driver.authenticate_sandbox(Request::new(request)).await }, ) .await - .map(|response| response.into_inner().sandbox_id) + .map(tonic::Response::into_inner) } #[must_use] @@ -983,17 +990,46 @@ impl ComputeRuntime { ) .await { - Ok(_) => { + Ok(response) => { + let runtime_identity = response.into_inner().runtime_identity; + if self.supports_sandbox_authentication() && runtime_identity.is_empty() { + return Err(Status::internal( + "compute driver did not return a runtime identity", + )); + } // The driver now owns the staged archive and removes the // request directory once it has built the disk. Every other // arm lets the guard drop and clean up. if let Some(staged) = staged.as_mut() { staged.disarm(); } - self.sandbox_watch_bus.notify(sandbox.object_id()); - if let Some(metadata) = sandbox.metadata.as_mut() { + if self.supports_sandbox_authentication() { + let driver_name = self.configured_driver_name().to_string(); + sandbox = self + .store + .update_message_cas::(&sandbox_id, 0, move |sandbox| { + if let Some(metadata) = sandbox.metadata.as_mut() { + metadata.annotations.insert( + COMPUTE_DRIVER_ANNOTATION.to_string(), + driver_name.clone(), + ); + metadata.annotations.insert( + COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + runtime_identity.clone(), + ); + } + }) + .await + .map_err(|error| { + Status::internal(format!( + "persist compute runtime identity failed: {error}" + )) + })?; + } else if let Some(metadata) = sandbox.metadata.as_mut() { metadata.resource_version = result.resource_version; } + self.sandbox_index.update_from_sandbox(&sandbox); + self.sandbox_watch_bus.notify(sandbox.object_id()); Ok(sandbox) } Err(status) if status.code() == Code::AlreadyExists => { @@ -1461,9 +1497,13 @@ impl ComputeRuntime { sandbox: Some(driver_sandbox), })) .await - .map(|_| { + .map(|response| { tonic::Response::new( - openshell_core::proto::compute::v1::StartSandboxResponse {}, + openshell_core::proto::compute::v1::StartSandboxResponse { + runtime_identity: response + .into_inner() + .runtime_identity, + }, ) }) }, @@ -1473,14 +1513,44 @@ impl ComputeRuntime { } match result { - Ok(_) => { + Ok(response) => { let _global_guard = self.lock_global_for_lifecycle(&lifecycle_guard).await; - let latest = self - .store - .get_message::(&sandbox_id) - .await - .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? - .ok_or_else(|| Status::not_found("sandbox not found"))?; + let runtime_identity = response.into_inner().runtime_identity; + if self.supports_sandbox_authentication() && runtime_identity.is_empty() { + return Err(Status::internal( + "compute driver did not return a runtime identity", + )); + } + let latest = if self.supports_sandbox_authentication() { + let driver_name = self.configured_driver_name().to_string(); + self.store + .update_message_cas::(&sandbox_id, 0, move |sandbox| { + if let Some(metadata) = sandbox.metadata.as_mut() { + metadata.annotations.insert( + COMPUTE_DRIVER_ANNOTATION.to_string(), + driver_name.clone(), + ); + metadata.annotations.insert( + COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + runtime_identity.clone(), + ); + } + }) + .await + .map_err(|error| { + Status::internal(format!( + "persist compute runtime identity failed: {error}" + )) + })? + } else { + self.store + .get_message::(&sandbox_id) + .await + .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? + .ok_or_else(|| Status::not_found("sandbox not found"))? + }; + self.sandbox_index.update_from_sandbox(&latest); + self.sandbox_watch_bus.notify(&sandbox_id); Ok(latest) } Err(err) => { @@ -5355,7 +5425,7 @@ fn is_terminal_failure_reason(reason: &str) -> bool { #[derive(Debug)] pub struct NoopTestDriver { workspace_delete_failures: std::sync::atomic::AtomicUsize, - sandbox_authentication: Option>, + sandbox_authentication: Option>, } #[cfg(any(test, feature = "test-support"))] @@ -5372,7 +5442,18 @@ impl NoopTestDriver { pub fn authenticating_sandbox(sandbox_id: impl Into) -> Self { Self { workspace_delete_failures: std::sync::atomic::AtomicUsize::new(0), - sandbox_authentication: Some(Ok(sandbox_id.into())), + sandbox_authentication: Some(Ok((sandbox_id.into(), "test-runtime".to_string()))), + } + } + + #[cfg(test)] + pub fn authenticating_sandbox_with_runtime( + sandbox_id: impl Into, + runtime_identity: impl Into, + ) -> Self { + Self { + workspace_delete_failures: std::sync::atomic::AtomicUsize::new(0), + sandbox_authentication: Some(Ok((sandbox_id.into(), runtime_identity.into()))), } } @@ -5406,9 +5487,10 @@ impl ComputeDriver for NoopTestDriver { Status, > { match &self.sandbox_authentication { - Some(Ok(sandbox_id)) => Ok(tonic::Response::new( + Some(Ok((sandbox_id, runtime_identity))) => Ok(tonic::Response::new( openshell_core::proto::compute::v1::AuthenticateSandboxResponse { sandbox_id: sandbox_id.clone(), + runtime_identity: runtime_identity.clone(), }, )), Some(Err((code, message))) => Err(Status::new(*code, message.clone())), @@ -5484,7 +5566,15 @@ impl ComputeDriver for NoopTestDriver { ) -> Result, Status> { Ok(tonic::Response::new( - openshell_core::proto::compute::v1::CreateSandboxResponse {}, + openshell_core::proto::compute::v1::CreateSandboxResponse { + runtime_identity: self + .sandbox_authentication + .as_ref() + .and_then(|result| result.as_ref().ok()) + .map_or_else(String::new, |(_, runtime_identity)| { + runtime_identity.clone() + }), + }, )) } @@ -5504,7 +5594,15 @@ impl ComputeDriver for NoopTestDriver { ) -> Result, Status> { Ok(tonic::Response::new( - openshell_core::proto::compute::v1::StartSandboxResponse {}, + openshell_core::proto::compute::v1::StartSandboxResponse { + runtime_identity: self + .sandbox_authentication + .as_ref() + .and_then(|result| result.as_ref().ok()) + .map_or_else(String::new, |(_, runtime_identity)| { + runtime_identity.clone() + }), + }, )) } @@ -6077,7 +6175,7 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(CreateSandboxResponse {})) + Ok(tonic::Response::new(CreateSandboxResponse::default())) } async fn stop_sandbox( @@ -6091,7 +6189,7 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(StartSandboxResponse {})) + Ok(tonic::Response::new(StartSandboxResponse::default())) } async fn delete_sandbox( @@ -6425,7 +6523,7 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(CreateSandboxResponse {})) + Ok(tonic::Response::new(CreateSandboxResponse::default())) } async fn stop_sandbox( @@ -6488,7 +6586,9 @@ mod tests { .expect("start outcome lock poisoned") .clone(); match outcome { - ControlledLifecycleOutcome::Ok => Ok(tonic::Response::new(StartSandboxResponse {})), + ControlledLifecycleOutcome::Ok => { + Ok(tonic::Response::new(StartSandboxResponse::default())) + } ControlledLifecycleOutcome::NotFound => Err(Status::not_found("sandbox not found")), ControlledLifecycleOutcome::Error(message) => Err(Status::internal(message)), } diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index 8eca7276b9..60255dfd5d 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -75,7 +75,11 @@ pub async fn handle_issue_sandbox_token( // Only a selected compute driver may establish the bootstrap sandbox // identity. Sandboxes already holding a gateway JWT use refresh instead. - if !matches!(sandbox.source, SandboxIdentitySource::ComputeDriver { .. }) { + let SandboxIdentitySource::ComputeDriver { + driver_name, + runtime_identity, + } = &sandbox.source + else { debug!( sandbox_id = %sandbox.sandbox_id, "IssueSandboxToken rejected: non-bootstrap principal source" @@ -83,7 +87,7 @@ pub async fn handle_issue_sandbox_token( return Err(Status::permission_denied( "this principal cannot mint a sandbox token; use RefreshSandboxToken", )); - } + }; let issuer = state.sandbox_jwt_issuer.as_ref().ok_or_else(|| { warn!( @@ -93,7 +97,27 @@ pub async fn handle_issue_sandbox_token( Status::unavailable("sandbox JWT minting is not configured on this gateway") })?; - let _ = ensure_sandbox_exists(state, &sandbox.sandbox_id).await?; + let sandbox_record = ensure_sandbox_exists(state, &sandbox.sandbox_id).await?; + let metadata = sandbox_record + .metadata + .as_ref() + .ok_or_else(|| Status::permission_denied("sandbox runtime identity is unavailable"))?; + let expected_driver = metadata + .annotations + .get(crate::compute::COMPUTE_DRIVER_ANNOTATION); + let expected_runtime_identity = metadata + .annotations + .get(crate::compute::COMPUTE_RUNTIME_IDENTITY_ANNOTATION); + if expected_driver != Some(driver_name) || expected_runtime_identity != Some(runtime_identity) { + warn!( + sandbox_id = %sandbox.sandbox_id, + driver_name, + "IssueSandboxToken rejected: compute runtime identity mismatch" + ); + return Err(Status::permission_denied( + "compute runtime identity does not match the sandbox", + )); + } let minted = issuer.mint(&sandbox.sandbox_id)?; info!( @@ -484,6 +508,15 @@ mod tests { ..Default::default() }; identity.write(&mut sandbox.metadata.as_mut().expect("metadata").annotations); + let annotations = &mut sandbox.metadata.as_mut().expect("metadata").annotations; + annotations.insert( + crate::compute::COMPUTE_DRIVER_ANNOTATION.to_string(), + "kubernetes".to_string(), + ); + annotations.insert( + crate::compute::COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + "test-runtime".to_string(), + ); sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); } @@ -776,6 +809,7 @@ mod tests { sandbox_id: "sandbox-a".to_string(), source: SandboxIdentitySource::ComputeDriver { driver_name: "kubernetes".to_string(), + runtime_identity: "test-runtime".to_string(), }, trust_domain: Some("openshell".to_string()), })); @@ -787,6 +821,28 @@ mod tests { assert!(resp.expiration_time.is_some()); } + #[tokio::test] + async fn issue_rejects_mismatched_compute_runtime_identity() { + use crate::auth::principal::SandboxIdentitySource; + + let state = state_with_issuer().await; + let mut req = Request::new(IssueSandboxTokenRequest {}); + req.extensions_mut() + .insert(Principal::Sandbox(SandboxPrincipal { + sandbox_id: "sandbox-a".to_string(), + source: SandboxIdentitySource::ComputeDriver { + driver_name: "kubernetes".to_string(), + runtime_identity: "replacement-runtime".to_string(), + }, + trust_domain: Some("openshell".to_string()), + })); + + let err = handle_issue_sandbox_token(&state, req) + .await + .expect_err("mismatched runtime must not receive a token"); + assert_eq!(err.code(), tonic::Code::PermissionDenied); + } + #[tokio::test] async fn issue_rejects_missing_sandbox() { use crate::auth::principal::SandboxIdentitySource; @@ -798,6 +854,7 @@ mod tests { sandbox_id: "sandbox-deleted".to_string(), source: SandboxIdentitySource::ComputeDriver { driver_name: "kubernetes".to_string(), + runtime_identity: "test-runtime".to_string(), }, trust_domain: Some("openshell".to_string()), })); @@ -844,6 +901,7 @@ mod tests { sandbox_id: "sandbox-a".to_string(), source: SandboxIdentitySource::ComputeDriver { driver_name: "kubernetes".to_string(), + runtime_identity: "test-runtime".to_string(), }, trust_domain: Some("openshell".to_string()), })); diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index 9d9650c2f0..a6b8d99e0b 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -368,7 +368,7 @@ impl ComputeDriver for FakeComputeDriver { .calls .push(FakeComputeDriverCall::CreateSandbox { sandbox }); }); - Ok(Response::new(CreateSandboxResponse {})) + Ok(Response::new(CreateSandboxResponse::default())) } async fn stop_sandbox( @@ -398,7 +398,7 @@ impl ComputeDriver for FakeComputeDriver { sandbox_name: request.name, }); }); - Ok(Response::new(StartSandboxResponse {})) + Ok(Response::new(StartSandboxResponse::default())) } async fn delete_sandbox( diff --git a/docs/kubernetes/access-control.mdx b/docs/kubernetes/access-control.mdx index 251c67c054..1ff54a0e01 100644 --- a/docs/kubernetes/access-control.mdx +++ b/docs/kubernetes/access-control.mdx @@ -21,7 +21,7 @@ For how the CLI resolves gateways and stores credentials, refer to [Gateway Auth ## Sandbox Supervisor Identity -Kubernetes sandbox supervisors authenticate back to the gateway as sandbox workloads. By default, the Kubernetes compute driver validates each projected ServiceAccount token and returns the authenticated sandbox ID to the gateway. The gateway verifies the sandbox still exists and mints its own sandbox JWT. +Kubernetes sandbox supervisors authenticate back to the gateway as sandbox workloads. By default, the Kubernetes compute driver validates each projected ServiceAccount token and returns the authenticated sandbox ID plus a stable runtime identity to the gateway. The gateway requires that identity to match the namespace, immutable Sandbox resource UID, and supervisor Pod UID recorded during provisioning before it mints its own sandbox JWT. Dynamic provider token grants can use SPIFFE without changing supervisor-to-gateway authentication. Set `server.providerTokenGrants.spiffe.enabled=true` to mount the SPIFFE CSI Workload API socket into gateway and sandbox pods while keeping the projected ServiceAccount token bootstrap and gateway-minted sandbox JWT path. diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 4d9e0841ad..5dc8298c67 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -222,7 +222,7 @@ Common identity providers such as Keycloak (RS256), Microsoft Entra ID (RSA), an If `OPENSHELL_OIDC_SCOPES_CLAIM` is set, the gateway also enforces scopes. It accepts space-delimited scope strings such as `scope: "openid sandbox:read"` and JSON arrays such as `scp: ["sandbox:read"]`. Standard OIDC scopes such as `openid`, `profile`, `email`, and `offline_access` are ignored for authorization. `openshell:all` grants access to all scoped methods. -Supervisor-to-gateway RPCs do not use user OIDC tokens or mTLS user identity. Each sandbox supervisor presents a gateway-minted `Authorization: Bearer` token scoped to its sandbox ID. On Kubernetes, the Kubernetes compute driver validates the projected ServiceAccount token with TokenReview, verifies the live pod UID and controlling `Sandbox` ownerReference, and returns the authenticated sandbox ID to the gateway. The gateway verifies that sandbox still exists before minting its JWT. Log upload, policy status, provider environment lookup, and sandbox config sync run with sandbox-restricted scope, while CLI users authenticate with OIDC, edge auth, local mTLS user authentication, or an explicitly enabled unauthenticated local developer mode. Provider environment responses expose only the credentials and configuration attached to that sandbox, subject to endpoint binding and credential expiry checks. +Supervisor-to-gateway RPCs do not use user OIDC tokens or mTLS user identity. Each sandbox supervisor presents a gateway-minted `Authorization: Bearer` token scoped to its sandbox ID. On Kubernetes, the Kubernetes compute driver validates the projected ServiceAccount token with TokenReview, verifies the live pod UID and controlling `Sandbox` ownerReference, and returns the authenticated sandbox ID plus a stable runtime identity. The gateway requires that runtime identity to match the value recorded when it provisioned the sandbox before minting a JWT. Log upload, policy status, provider environment lookup, and sandbox config sync run with sandbox-restricted scope, while CLI users authenticate with OIDC, edge auth, local mTLS user authentication, or an explicitly enabled unauthenticated local developer mode. Provider environment responses expose only the credentials and configuration attached to that sandbox, subject to endpoint binding and credential expiry checks. Re-authenticate an OIDC gateway with: diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index a65d9b1cb5..5d2873bdfc 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -317,7 +317,7 @@ gateway under the worktree-specific k3d cluster name; select it with `openshell gateway select `. The local Podman, Docker, and VM gateway tasks export to the forwarded receiver automatically. -In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`; configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. The gateway trusts the returned sandbox ID only for `IssueSandboxToken`, verifies that the sandbox still exists, and then mints its own JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. +In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`; configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. It must return the same non-empty runtime identity from sandbox creation and credential authentication. For `IssueSandboxToken`, the gateway verifies the sandbox exists and the driver/runtime identity matches the durable provisioning record before minting its own JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. ### Tuning diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 36fa4eeb76..e7de47977b 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -110,6 +110,10 @@ message AuthenticateSandboxRequest { message AuthenticateSandboxResponse { // Stable gateway-assigned sandbox ID authenticated by the driver. string sandbox_id = 1; + // Opaque, stable identity of the compute resource presenting the credential. + // The gateway compares this with the identity recorded when the sandbox was + // created to authorize the bootstrap exchange. + string runtime_identity = 2; } // Static portable resource request forms supported by a compute driver. @@ -383,7 +387,11 @@ message CreateSandboxRequest { DriverSandbox sandbox = 1; } -message CreateSandboxResponse {} +message CreateSandboxResponse { + // Opaque, stable identity of the compute resource created for the sandbox. + // Required when the driver advertises sandbox authentication support. + string runtime_identity = 1; +} message StopSandboxRequest { // Stable sandbox ID stored by the gateway. @@ -407,7 +415,11 @@ message StartSandboxRequest { string generation_id = 4; } -message StartSandboxResponse {} +message StartSandboxResponse { + // Updated opaque runtime identity after a successful start. Required when + // the driver advertises sandbox authentication support. + string runtime_identity = 1; +} message DeleteSandboxRequest { // Stable sandbox ID stored by the gateway. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index f81416c5ab..2b3fc424ed 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -649,6 +649,11 @@ the selected Kubernetes compute driver rejects projected tokens from other service accounts. For an external driver, inspect its logs and confirm it advertises `supports_sandbox_authentication`; the gateway delegates the opaque credential over the driver socket and never interprets Kubernetes settings. +Drivers that advertise sandbox authentication must return the same non-empty +runtime identity from sandbox creation and credential authentication. A +gateway log reporting a compute runtime identity mismatch indicates stale or +re-created runtime resources; compare the live resource UID with the sandbox +that the gateway provisioned. ```bash helm -n openshell get values openshell | grep -A3 sandboxServiceAccount From 231339b165d91cd47f6020f0188ebe44fa1e28d4 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:04:20 -0700 Subject: [PATCH 2/6] fix(compute): compensate runtime binding failures Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- architecture/compute-runtimes.md | 10 +- crates/openshell-driver-docker/src/lib.rs | 1 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-mxc/src/driver.rs | 1 + crates/openshell-driver-podman/src/driver.rs | 1 + crates/openshell-driver-vm/src/driver.rs | 1 + crates/openshell-server/src/compute/mod.rs | 684 ++++++++++++++++-- crates/openshell-server/src/grpc/mod.rs | 26 + crates/openshell-server/src/grpc/sandbox.rs | 89 ++- crates/openshell-server/src/test_support.rs | 1 + proto/compute_driver.proto | 3 + 11 files changed, 746 insertions(+), 72 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 507350011b..ee35fbf892 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -508,9 +508,13 @@ The Kubernetes driver's `AuthenticateSandbox` implementation applies its named It validates the projected token with Kubernetes `TokenReview`, checks the live pod UID, and verifies the pod's controlling Sandbox CR UID and sandbox ID. The driver returns both the sandbox ID and an opaque runtime identity derived from -the namespace, immutable Sandbox CR UID, and authenticated supervisor Pod UID. The gateway records that runtime -identity when provisioning succeeds and requires an exact match before issuing -a sandbox JWT. This correlates credential authentication with the durable +the namespace, immutable Sandbox CR UID, and authenticated supervisor Pod UID. +Drivers that authenticate sandboxes must advertise this runtime-binding +contract; the gateway rejects incompatible drivers during initialization. The +gateway records the runtime identity when provisioning succeeds and requires +an exact match before issuing a sandbox JWT. If binding validation or storage +fails after a lifecycle call succeeds, the gateway compensates that call before +returning the error. This correlates credential authentication with the durable runtime record rather than authorizing from the sandbox ID alone. Shared and managed modes still reserve the sandbox namespace, Sandbox CRs, diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index bd19d7f8c3..ae0a545634 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -962,6 +962,7 @@ impl DockerComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 493efa87d2..ddb652782b 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -752,6 +752,7 @@ impl KubernetesComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: false, supports_sandbox_authentication: true, + supports_runtime_identity_binding: true, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 23491533bb..538aa4c53e 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -458,6 +458,7 @@ impl MxcComputeBackend { default_image: DEFAULT_IMAGE_SENTINEL.to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: true, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index de3bf3c953..3266d9c9ac 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -528,6 +528,7 @@ impl PodmanComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index fbd4230dec..22077a6508 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -986,6 +986,7 @@ impl VmDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 85c05958df..b9df4b24ea 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -231,7 +231,7 @@ impl LifecycleGateRegistry { /// Passing it to `lock_global_for_lifecycle` makes that ordering visible at /// every global-lock acquisition in a lifecycle path. #[derive(Debug)] -struct SandboxLifecycleGuard { +pub struct SandboxLifecycleGuard { _guard: tokio::sync::OwnedMutexGuard<()>, } @@ -662,6 +662,13 @@ impl ComputeRuntime { capabilities.extension.clone(), ) .map_err(|error| ComputeError::Message(error.to_string()))?; + if capabilities.supports_sandbox_authentication + && !capabilities.supports_runtime_identity_binding + { + return Err(ComputeError::Precondition(format!( + "compute driver '{driver_name}' authenticates sandboxes but does not support runtime identity binding" + ))); + } info!( configured_driver = %driver_name, advertised_driver = %capabilities.driver_name, @@ -720,6 +727,15 @@ impl ComputeRuntime { }) } + pub(crate) async fn sandbox_create_guards( + &self, + sandbox_id: &str, + ) -> crate::persistence::PersistenceResult<(SandboxLifecycleGuard, SandboxSyncGuard)> { + let lifecycle_guard = self.lifecycle_gates.lock_for(sandbox_id).await; + let global_guard = self.sandbox_sync_guard().await?; + Ok((lifecycle_guard, global_guard)) + } + /// Acquires the process-wide lock for code that already holds the /// sandbox-ID lifecycle gate. The guard parameter documents and enforces /// that callers acquire locks in lifecycle-gate -> global-lock order. @@ -895,12 +911,12 @@ impl ComputeRuntime { sandbox_token: Option, await_main_process_attachment: bool, ) -> Result { - self.create_sandbox_authenticated( + Box::pin(self.create_sandbox_authenticated( sandbox, sandbox_token, None, await_main_process_attachment, - ) + )) .await } @@ -910,6 +926,33 @@ impl ComputeRuntime { sandbox_token: Option, launch_authentication: Option>, await_main_process_attachment: bool, + ) -> Result { + let (lifecycle_guard, global_guard) = self + .sandbox_create_guards(sandbox.object_id()) + .await + .map_err(|error| { + crate::grpc::persistence_error_to_status(error, "acquire sandbox mutation lock") + })?; + Box::pin(self.create_sandbox_authenticated_with_guards( + sandbox, + sandbox_token, + launch_authentication, + await_main_process_attachment, + lifecycle_guard, + global_guard, + )) + .await + } + + #[allow(clippy::too_many_arguments)] + pub(crate) async fn create_sandbox_authenticated_with_guards( + &self, + sandbox: Sandbox, + sandbox_token: Option, + launch_authentication: Option>, + await_main_process_attachment: bool, + lifecycle_guard: SandboxLifecycleGuard, + global_guard: SandboxSyncGuard, ) -> Result { let sandbox_id = sandbox.object_id().to_string(); let mut sandbox = sandbox; @@ -965,6 +1008,10 @@ impl ComputeRuntime { Status::internal(format!("persist sandbox failed: {e}")) } })?; + if let Some(metadata) = sandbox.metadata.as_mut() { + metadata.resource_version = result.resource_version; + } + drop(global_guard); if let Some(token) = sandbox_token && let Some(spec) = driver_sandbox.spec.as_mut() @@ -992,20 +1039,28 @@ impl ComputeRuntime { { Ok(response) => { let runtime_identity = response.into_inner().runtime_identity; - if self.supports_sandbox_authentication() && runtime_identity.is_empty() { - return Err(Status::internal( - "compute driver did not return a runtime identity", - )); - } // The driver now owns the staged archive and removes the - // request directory once it has built the disk. Every other - // arm lets the guard drop and clean up. + // request directory once it has built the disk. if let Some(staged) = staged.as_mut() { staged.disarm(); } + let global_guard = self.lock_global_for_lifecycle(&lifecycle_guard).await; + if self.supports_sandbox_authentication() && runtime_identity.is_empty() { + let status = + Status::internal("compute driver did not return a runtime identity"); + return Err(self + .compensate_failed_create( + &sandbox_id, + sandbox.object_name(), + lifecycle_guard, + global_guard, + status, + ) + .await); + } if self.supports_sandbox_authentication() { let driver_name = self.configured_driver_name().to_string(); - sandbox = self + let persisted = self .store .update_message_cas::(&sandbox_id, 0, move |sandbox| { if let Some(metadata) = sandbox.metadata.as_mut() { @@ -1019,14 +1074,24 @@ impl ComputeRuntime { ); } }) - .await - .map_err(|error| { - Status::internal(format!( + .await; + sandbox = match persisted { + Ok(sandbox) => sandbox, + Err(error) => { + let status = Status::internal(format!( "persist compute runtime identity failed: {error}" - )) - })?; - } else if let Some(metadata) = sandbox.metadata.as_mut() { - metadata.resource_version = result.resource_version; + )); + return Err(self + .compensate_failed_create( + &sandbox_id, + sandbox.object_name(), + lifecycle_guard, + global_guard, + status, + ) + .await); + } + }; } self.sandbox_index.update_from_sandbox(&sandbox); self.sandbox_watch_bus.notify(sandbox.object_id()); @@ -1062,6 +1127,100 @@ impl ComputeRuntime { } } + async fn compensate_failed_create( + &self, + sandbox_id: &str, + sandbox_name: &str, + lifecycle_guard: SandboxLifecycleGuard, + global_guard: tokio::sync::OwnedMutexGuard<()>, + original: Status, + ) -> Status { + let transition = match self + .begin_sandbox_delete_with_initial_snapshot(sandbox_id, None) + .await + { + Ok(BeginDelete::Started(transition)) => *transition, + Ok(BeginDelete::AlreadyDeleting) => { + return Status::new( + original.code(), + format!( + "{}; cleanup after successful create was already claimed", + original.message() + ), + ); + } + Err(error) => { + return Status::new( + original.code(), + format!( + "{}; cleanup after successful create could not claim the sandbox record: {}", + original.message(), + error.message() + ), + ); + } + }; + self.sandbox_index.update_from_sandbox(&transition.deleting); + self.sandbox_watch_bus.notify(sandbox_id); + drop(global_guard); + + let delete_result = self + .driver + .call( + openshell_otel::rpc::DELETE_SANDBOX, + Some(sandbox_id), + |driver| { + let sandbox_id = sandbox_id.to_string(); + let sandbox_name = sandbox_name.to_string(); + async move { + driver + .delete_sandbox(Request::new(DeleteSandboxRequest { + sandbox_id, + name: sandbox_name, + })) + .await + } + }, + ) + .await; + match delete_result { + Ok(response) => { + if response.into_inner().deleted { + // The driver accepted an asynchronous deletion. Keep the + // durable Deleting record until the watch path confirms + // that the backend is absent, matching ordinary delete + // semantics. + original + } else if self + .remove_deleting_sandbox_record(&lifecycle_guard, sandbox_id) + .await + { + original + } else { + Status::new( + original.code(), + format!( + "{}; cleanup after successful create lost ownership of the sandbox record", + original.message() + ), + ) + } + } + Err(error) => { + self.recover_failed_delete(&lifecycle_guard, &transition) + .await; + Status::new( + original.code(), + format!( + "{}; cleanup after successful create failed: {}", + original.message(), + error.message() + ), + ) + } + } + } + pub(crate) async fn stop_sandbox( &self, workspace: &str, @@ -1514,34 +1673,53 @@ impl ComputeRuntime { match result { Ok(response) => { - let _global_guard = self.lock_global_for_lifecycle(&lifecycle_guard).await; let runtime_identity = response.into_inner().runtime_identity; if self.supports_sandbox_authentication() && runtime_identity.is_empty() { - return Err(Status::internal( - "compute driver did not return a runtime identity", - )); + let status = + Status::internal("compute driver did not return a runtime identity"); + return Err(self + .compensate_successful_start(&lifecycle_guard, &starting, &previous, status) + .await); } + let global_guard = self.lock_global_for_lifecycle(&lifecycle_guard).await; let latest = if self.supports_sandbox_authentication() { let driver_name = self.configured_driver_name().to_string(); - self.store - .update_message_cas::(&sandbox_id, 0, move |sandbox| { - if let Some(metadata) = sandbox.metadata.as_mut() { - metadata.annotations.insert( - COMPUTE_DRIVER_ANNOTATION.to_string(), - driver_name.clone(), - ); - metadata.annotations.insert( - COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), - runtime_identity.clone(), - ); - } - }) - .await - .map_err(|error| { - Status::internal(format!( + let persisted = self + .store + .update_message_cas::( + &sandbox_id, + sandbox_resource_version(&starting), + move |sandbox| { + if let Some(metadata) = sandbox.metadata.as_mut() { + metadata.annotations.insert( + COMPUTE_DRIVER_ANNOTATION.to_string(), + driver_name.clone(), + ); + metadata.annotations.insert( + COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + runtime_identity.clone(), + ); + } + }, + ) + .await; + match persisted { + Ok(sandbox) => sandbox, + Err(error) => { + drop(global_guard); + let status = Status::internal(format!( "persist compute runtime identity failed: {error}" - )) - })? + )); + return Err(self + .compensate_successful_start( + &lifecycle_guard, + &starting, + &previous, + status, + ) + .await); + } + } } else { self.store .get_message::(&sandbox_id) @@ -1564,6 +1742,59 @@ impl ComputeRuntime { } } + async fn compensate_successful_start( + &self, + lifecycle_guard: &SandboxLifecycleGuard, + starting: &Sandbox, + previous: &Sandbox, + original: Status, + ) -> Status { + let sandbox_id = starting.object_id(); + let sandbox_name = starting.object_name(); + let stop_result = self + .driver + .call( + openshell_otel::rpc::STOP_SANDBOX, + Some(sandbox_id), + |driver| { + let sandbox_id = sandbox_id.to_string(); + let sandbox_name = sandbox_name.to_string(); + async move { + driver + .stop_sandbox(Request::new(StopSandboxRequest { + sandbox_id, + name: sandbox_name, + })) + .await + } + }, + ) + .await; + if let Err(error) = stop_result { + return Status::new( + original.code(), + format!( + "{}; rollback after successful start failed: {}", + original.message(), + error.message() + ), + ); + } + + let _global_guard = self.lock_global_for_lifecycle(lifecycle_guard).await; + if self.restore_lifecycle_snapshot(starting, previous).await { + original + } else { + Status::new( + original.code(), + format!( + "{}; rollback after successful start lost ownership of the sandbox record", + original.message() + ), + ) + } + } + /// Reconcile an ambiguous lifecycle error against the driver's observed /// state before deciding whether the pre-operation snapshot is still true. /// @@ -1619,7 +1850,7 @@ impl ComputeRuntime { ); } } else { - self.restore_lifecycle_snapshot(transition, previous).await; + let _ = self.restore_lifecycle_snapshot(transition, previous).await; } } Ok(Some(_) | None) | Err(_) => { @@ -1700,7 +1931,7 @@ impl ComputeRuntime { .map_err(|e| crate::grpc::persistence_error_to_status(e, "update sandbox lifecycle")) } - async fn restore_lifecycle_snapshot(&self, owned: &Sandbox, previous: &Sandbox) { + async fn restore_lifecycle_snapshot(&self, owned: &Sandbox, previous: &Sandbox) -> bool { let sandbox_id = owned.object_id().to_string(); let previous = previous.clone(); match self @@ -1715,9 +1946,11 @@ impl ComputeRuntime { Ok(restored) => { self.sandbox_index.update_from_sandbox(&restored); self.sandbox_watch_bus.notify(&sandbox_id); + true } Err(err) => { debug!(sandbox_id, error = %err, "Skipped lifecycle rollback after concurrent change"); + false } } } @@ -5514,6 +5747,7 @@ impl ComputeDriver for NoopTestDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: self.sandbox_authentication.is_some(), + supports_runtime_identity_binding: self.sandbox_authentication.is_some(), driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -6105,6 +6339,7 @@ mod tests { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -6271,6 +6506,9 @@ mod tests { delete_calls: AtomicUsize, delete_requests: TestMutex>, delete_outcome: TestMutex, + create_started: Notify, + create_release: Semaphore, + create_blocked: AtomicBool, stop_started: Notify, stop_finished: Notify, stop_release: Semaphore, @@ -6286,6 +6524,9 @@ mod tests { start_requests: TestMutex>, start_authentications: TestMutex>>, start_outcome: TestMutex, + runtime_identity: TestMutex, + advertises_sandbox_authentication: AtomicBool, + advertises_runtime_identity_binding: AtomicBool, get_started: Notify, get_release: Semaphore, get_blocked: AtomicBool, @@ -6305,6 +6546,9 @@ mod tests { delete_calls: AtomicUsize::new(0), delete_requests: TestMutex::new(Vec::new()), delete_outcome: TestMutex::new(ControlledDeleteOutcome::Ok(true)), + create_started: Notify::new(), + create_release: Semaphore::new(0), + create_blocked: AtomicBool::new(false), stop_started: Notify::new(), stop_finished: Notify::new(), stop_release: Semaphore::new(0), @@ -6320,6 +6564,9 @@ mod tests { start_requests: TestMutex::new(Vec::new()), start_authentications: TestMutex::new(Vec::new()), start_outcome: TestMutex::new(ControlledLifecycleOutcome::Ok), + runtime_identity: TestMutex::new(String::new()), + advertises_sandbox_authentication: AtomicBool::new(false), + advertises_runtime_identity_binding: AtomicBool::new(false), get_started: Notify::new(), get_release: Semaphore::new(0), get_blocked: AtomicBool::new(false), @@ -6331,6 +6578,14 @@ mod tests { self.delete_blocked.store(true, Ordering::SeqCst); } + fn block_create(&self) { + self.create_blocked.store(true, Ordering::SeqCst); + } + + fn release_create(&self) { + self.create_release.add_permits(1); + } + fn release_delete(&self) { self.delete_release.add_permits(1); } @@ -6380,6 +6635,20 @@ mod tests { .expect("start outcome lock poisoned") = outcome; } + fn set_runtime_identity(&self, runtime_identity: impl Into) { + *self + .runtime_identity + .lock() + .expect("runtime identity lock poisoned") = runtime_identity.into(); + } + + fn set_binding_capabilities(&self, authentication: bool, binding: bool) { + self.advertises_sandbox_authentication + .store(authentication, Ordering::SeqCst); + self.advertises_runtime_identity_binding + .store(binding, Ordering::SeqCst); + } + fn set_get_outcome(&self, outcome: ControlledGetOutcome) { *self.get_outcome.lock().expect("get outcome lock poisoned") = outcome; } @@ -6456,7 +6725,12 @@ mod tests { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, - supports_sandbox_authentication: false, + supports_sandbox_authentication: self + .advertises_sandbox_authentication + .load(Ordering::SeqCst), + supports_runtime_identity_binding: self + .advertises_runtime_identity_binding + .load(Ordering::SeqCst), driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -6523,7 +6797,21 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(CreateSandboxResponse::default())) + self.create_started.notify_one(); + if self.create_blocked.load(Ordering::SeqCst) { + self.create_release + .acquire() + .await + .expect("create release semaphore closed") + .forget(); + } + Ok(tonic::Response::new(CreateSandboxResponse { + runtime_identity: self + .runtime_identity + .lock() + .expect("runtime identity lock poisoned") + .clone(), + })) } async fn stop_sandbox( @@ -6586,9 +6874,13 @@ mod tests { .expect("start outcome lock poisoned") .clone(); match outcome { - ControlledLifecycleOutcome::Ok => { - Ok(tonic::Response::new(StartSandboxResponse::default())) - } + ControlledLifecycleOutcome::Ok => Ok(tonic::Response::new(StartSandboxResponse { + runtime_identity: self + .runtime_identity + .lock() + .expect("runtime identity lock poisoned") + .clone(), + })), ControlledLifecycleOutcome::NotFound => Err(Status::not_found("sandbox not found")), ControlledLifecycleOutcome::Error(message) => Err(Status::internal(message)), } @@ -6703,6 +6995,304 @@ mod tests { runtime } + fn enable_runtime_identity_binding(runtime: &mut ComputeRuntime) { + runtime.driver_info.supports_sandbox_authentication = true; + } + + fn set_compute_runtime_binding(sandbox: &mut Sandbox, identity: &str) { + let annotations = &mut sandbox + .metadata + .as_mut() + .expect("sandbox metadata") + .annotations; + annotations.insert( + COMPUTE_DRIVER_ANNOTATION.to_string(), + "test-driver".to_string(), + ); + annotations.insert( + COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + identity.to_string(), + ); + } + + async fn runtime_with_binding_store_failure( + driver: Arc, + ) -> (tempfile::TempDir, ComputeRuntime) { + let directory = tempfile::tempdir().expect("temporary database directory"); + let database_url = format!("sqlite://{}", directory.path().join("gateway.db").display()); + let store = Arc::new(Store::connect(&database_url).await.expect("connect store")); + let pool = sqlx::SqlitePool::connect(&database_url) + .await + .expect("connect failure injector"); + sqlx::query( + "CREATE TRIGGER reject_test_runtime_identity \ + BEFORE UPDATE OF payload ON objects \ + WHEN instr(NEW.payload, CAST('new-runtime-identity' AS BLOB)) > 0 \ + BEGIN SELECT RAISE(ABORT, 'injected runtime identity persistence failure'); END", + ) + .execute(&pool) + .await + .expect("install persistence failure trigger"); + pool.close().await; + + let mut runtime = test_runtime(driver).await; + runtime.store = store; + enable_runtime_identity_binding(&mut runtime); + (directory, runtime) + } + + #[tokio::test] + async fn incompatible_authenticating_driver_is_rejected_during_initialization() { + let driver = ControlledDriver::new(); + driver.set_binding_capabilities(true, false); + let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); + + let error = ComputeRuntime::from_driver( + "legacy-driver".to_string(), + driver, + None, + store, + SandboxIndex::new(), + SandboxWatchBus::new(), + TracingLogBus::new(), + Arc::new(SupervisorSessionRegistry::new()), + ) + .await + .expect_err("driver without the binding contract must be rejected"); + + assert!(error.to_string().contains("runtime identity binding")); + } + + #[tokio::test] + async fn empty_create_runtime_identity_deletes_backend_and_record() { + let driver = ControlledDriver::new(); + driver.set_delete_outcome(ControlledDeleteOutcome::Ok(false)); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let sandbox = sandbox_record( + "sb-create-empty-identity", + "create-empty-identity", + SandboxPhase::Provisioning, + ); + + let error = runtime + .create_sandbox(sandbox.clone(), None, false) + .await + .expect_err("empty identity must fail create"); + + assert!( + error + .message() + .contains("did not return a runtime identity") + ); + assert_eq!(driver.delete_calls(), 1); + assert_eq!( + driver.delete_requests(), + vec![( + sandbox.object_id().to_string(), + sandbox.object_name().to_string() + )] + ); + assert!( + runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn create_compensation_retains_deleting_record_while_backend_delete_is_pending() { + let driver = ControlledDriver::new(); + driver.set_delete_outcome(ControlledDeleteOutcome::Ok(true)); + driver.set_get_outcome(ControlledGetOutcome::Sandbox(Box::new( + ready_driver_sandbox("sb-create-delete-pending", "create-delete-pending"), + ))); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let sandbox = sandbox_record( + "sb-create-delete-pending", + "create-delete-pending", + SandboxPhase::Provisioning, + ); + + runtime + .create_sandbox(sandbox.clone(), None, false) + .await + .expect_err("empty identity must fail create"); + + assert_eq!(driver.delete_calls(), 1); + let retained = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .expect("pending backend deletion must retain the durable record"); + assert_eq!(retained.phase(), SandboxPhase::Deleting as i32); + } + + #[tokio::test] + async fn create_binding_store_failure_deletes_backend_and_record() { + let driver = ControlledDriver::new(); + driver.set_delete_outcome(ControlledDeleteOutcome::Ok(false)); + driver.set_runtime_identity("new-runtime-identity"); + let (_directory, runtime) = runtime_with_binding_store_failure(driver.clone()).await; + let sandbox = sandbox_record( + "sb-create-store-failure", + "create-store-failure", + SandboxPhase::Provisioning, + ); + + let error = runtime + .create_sandbox(sandbox.clone(), None, false) + .await + .expect_err("binding persistence failure must fail create"); + + assert!( + error + .message() + .contains("persist compute runtime identity failed") + ); + assert_eq!(driver.delete_calls(), 1); + assert!( + runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn create_merges_runtime_binding_after_watch_update() { + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + driver.block_create(); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let sandbox = sandbox_record( + "sb-create-watch-race", + "create-watch-race", + SandboxPhase::Provisioning, + ); + + let create_runtime = runtime.clone(); + let create_sandbox = sandbox.clone(); + let create = tokio::spawn(async move { + create_runtime + .create_sandbox(create_sandbox, None, false) + .await + }); + tokio::time::timeout(Duration::from_secs(1), driver.create_started.notified()) + .await + .expect("create did not reach the driver"); + + let update_runtime = runtime.clone(); + let update = tokio::spawn(async move { + update_runtime + .apply_sandbox_update(ready_driver_sandbox( + "sb-create-watch-race", + "create-watch-race", + )) + .await + }); + update.await.unwrap().unwrap(); + + driver.release_create(); + create.await.unwrap().unwrap(); + + let stored = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.metadata.unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "new-runtime-identity" + ); + assert_eq!(driver.delete_calls(), 0); + } + + #[tokio::test] + async fn empty_start_runtime_identity_stops_backend_and_restores_record() { + let driver = ControlledDriver::new(); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let mut sandbox = sandbox_record( + "sb-start-empty-identity", + "start-empty-identity", + SandboxPhase::Stopped, + ); + set_compute_runtime_binding(&mut sandbox, "previous-runtime-identity"); + runtime.store.put_message(&sandbox).await.unwrap(); + + let error = runtime + .start_sandbox("default", sandbox.object_name()) + .await + .expect_err("empty identity must fail start"); + + assert!( + error + .message() + .contains("did not return a runtime identity") + ); + assert_eq!(driver.start_calls(), 1); + assert_eq!(driver.stop_calls(), 1); + let restored = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + assert_eq!(restored.phase(), SandboxPhase::Stopped as i32); + assert_eq!( + restored.metadata.unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "previous-runtime-identity" + ); + } + + #[tokio::test] + async fn start_binding_store_failure_stops_backend_and_restores_record() { + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + let (_directory, runtime) = runtime_with_binding_store_failure(driver.clone()).await; + let mut sandbox = sandbox_record( + "sb-start-store-failure", + "start-store-failure", + SandboxPhase::Stopped, + ); + set_compute_runtime_binding(&mut sandbox, "previous-runtime-identity"); + runtime.store.put_message(&sandbox).await.unwrap(); + + let error = runtime + .start_sandbox("default", sandbox.object_name()) + .await + .expect_err("binding persistence failure must fail start"); + + assert!( + error + .message() + .contains("persist compute runtime identity failed") + ); + assert_eq!(driver.start_calls(), 1); + assert_eq!(driver.stop_calls(), 1); + let restored = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + assert_eq!(restored.phase(), SandboxPhase::Stopped as i32); + assert_eq!( + restored.metadata.unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "previous-runtime-identity" + ); + } + fn register_test_supervisor_session(runtime: &ComputeRuntime, sandbox_id: &str) { let (tx, _rx) = mpsc::channel(1); let (shutdown_tx, _shutdown_rx) = oneshot::channel(); diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 9c7761be96..9a6f65198d 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -1026,6 +1026,32 @@ pub mod test_support { test_server_state_for_driver(driver_name, true).await } + pub async fn test_server_state_with_compute_driver( + driver_name: &str, + driver: Arc, + ) -> Arc { + let store = Arc::new( + Store::connect("sqlite::memory:?cache=shared") + .await + .unwrap(), + ); + crate::ensure_default_workspace(&store).await.unwrap(); + seed_example_provider_profiles(&store).await; + let compute = new_test_runtime_with_driver(store.clone(), driver_name, driver); + with_test_provider_profile_sources(Arc::new(ServerState::new( + Config::new(None) + .with_database_url("sqlite::memory:?cache=shared") + .with_credential_drivers(["test-static"]), + store, + compute, + SandboxIndex::new(), + SandboxWatchBus::new(), + TracingLogBus::new(), + Arc::new(SupervisorSessionRegistry::new()), + None, + ))) + } + async fn test_server_state_for_driver( driver_name: &str, seed_profiles: bool, diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index bffdc6ebbf..6170201db9 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -450,13 +450,17 @@ async fn handle_create_sandbox_inner( .authorize(&token, &workspace, &subject)?; } - let _sandbox_sync_guard = if spec.providers.is_empty() { - None + let id = uuid::Uuid::new_v4().to_string(); + let name = if request.name.is_empty() { + generate_routable_name() } else { - Some(state.compute.sandbox_sync_guard().await.map_err(|err| { - super::persistence_error_to_status(err, "acquire sandbox mutation lock") - })?) + request.name.clone() }; + let (sandbox_lifecycle_guard, sandbox_sync_guard) = state + .compute + .sandbox_create_guards(&id) + .await + .map_err(|err| super::persistence_error_to_status(err, "acquire sandbox mutation lock"))?; // Validate provider names exist (fail fast). for name in &spec.providers { @@ -516,13 +520,6 @@ async fn handle_create_sandbox_inner( ) .await?; - let id = uuid::Uuid::new_v4().to_string(); - let name = if request.name.is_empty() { - generate_routable_name() - } else { - request.name.clone() - }; - let now_ms = current_time_ms(); let mut sandbox = Sandbox { @@ -620,15 +617,15 @@ async fn handle_create_sandbox_inner( }) .transpose()?; - let sandbox = state - .compute - .create_sandbox_authenticated( - sandbox, - sandbox_token, - launch_authentication, - await_main_process_attachment, - ) - .await?; + let sandbox = Box::pin(state.compute.create_sandbox_authenticated_with_guards( + sandbox, + sandbox_token, + launch_authentication, + await_main_process_attachment, + sandbox_lifecycle_guard, + sandbox_sync_guard, + )) + .await?; let mut service_urls = HashMap::with_capacity(request.service_exposures.len()); for exposure in &request.service_exposures { @@ -3361,8 +3358,10 @@ async fn run_exec_with_russh( #[cfg(test)] mod tests { use super::*; + use crate::compute::NoopTestDriver; use crate::grpc::test_support::{ - authed_request, test_server_state, test_server_state_with_driver, + authed_request, test_server_state, test_server_state_with_compute_driver, + test_server_state_with_driver, }; use openshell_core::proto::datamodel::v1::ObjectMeta; use openshell_core::proto::{GpuResourceRequirements, SandboxServiceExposure, ServiceEndpoint}; @@ -4478,6 +4477,52 @@ mod tests { assert!(err.message().contains("provider-b")); } + #[tokio::test] + async fn provider_create_failure_releases_global_guard_before_compensation() { + let state = test_server_state_with_compute_driver( + "test", + Arc::new(NoopTestDriver::authenticating_sandbox_with_runtime( + "unused", "", + )), + ) + .await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + + let result = tokio::time::timeout( + std::time::Duration::from_secs(1), + handle_create_sandbox( + &state, + authed_request(CreateSandboxRequest { + name: "provider-fail".to_string(), + spec: Some(SandboxSpec { + providers: vec!["work-github".to_string()], + ..Default::default() + }), + workspace_scope: Some(openshell_core::proto::workspace_selector( + "default".to_string(), + )), + ..Default::default() + }), + ), + ) + .await + .expect("create compensation must not deadlock") + .expect_err("empty runtime identity must fail create"); + + assert_eq!(result.code(), tonic::Code::Internal, "{}", result.message()); + let retained = state + .store + .get_message_by_name::("default", "provider-fail") + .await + .unwrap() + .expect("accepted asynchronous cleanup must retain the sandbox record"); + assert_eq!(retained.phase(), SandboxPhase::Deleting as i32); + } + #[tokio::test] async fn create_sandbox_uses_configured_provider_profile_sources() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index a6b8d99e0b..cc6b69c4bb 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -150,6 +150,7 @@ impl FakeComputeDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, + supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index e7de47977b..2b83c6bc0e 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -85,6 +85,9 @@ message GetCapabilitiesResponse { bool gateway_manages_lifecycle = 6; // Whether AuthenticateSandbox is implemented by this driver. bool supports_sandbox_authentication = 7; + // Whether successful create, start, and authentication responses include a + // stable runtime identity suitable for gateway-side binding checks. + bool supports_runtime_identity_binding = 13; // Whether the driver reports runtime readiness itself. When false, the // gateway waits for the standard OpenShell supervisor session in addition // to the driver's platform-ready observation. From 85709c1c11ea0ba49aaa2906b9e1c28bb634042a Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:16:31 -0700 Subject: [PATCH 3/6] fix(compute): clean up backend on store failure Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- crates/openshell-server/src/compute/mod.rs | 125 +++++++++++++++++---- 1 file changed, 104 insertions(+), 21 deletions(-) diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index b9df4b24ea..8c1786aa38 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -1150,12 +1150,24 @@ impl ComputeRuntime { ); } Err(error) => { + drop(global_guard); + let delete_result = self + .delete_backend_after_failed_create(sandbox_id, sandbox_name) + .await; + let cleanup_detail = match delete_result { + Ok(_) => String::new(), + Err(delete_error) => format!( + "; best-effort backend cleanup also failed: {}", + delete_error.message() + ), + }; return Status::new( original.code(), format!( - "{}; cleanup after successful create could not claim the sandbox record: {}", + "{}; cleanup after successful create could not claim the sandbox record: {}{}", original.message(), - error.message() + error.message(), + cleanup_detail ), ); } @@ -1165,27 +1177,11 @@ impl ComputeRuntime { drop(global_guard); let delete_result = self - .driver - .call( - openshell_otel::rpc::DELETE_SANDBOX, - Some(sandbox_id), - |driver| { - let sandbox_id = sandbox_id.to_string(); - let sandbox_name = sandbox_name.to_string(); - async move { - driver - .delete_sandbox(Request::new(DeleteSandboxRequest { - sandbox_id, - name: sandbox_name, - })) - .await - } - }, - ) + .delete_backend_after_failed_create(sandbox_id, sandbox_name) .await; match delete_result { - Ok(response) => { - if response.into_inner().deleted { + Ok(deleted) => { + if deleted { // The driver accepted an asynchronous deletion. Keep the // durable Deleting record until the watch path confirms // that the backend is absent, matching ordinary delete @@ -1221,6 +1217,32 @@ impl ComputeRuntime { } } + async fn delete_backend_after_failed_create( + &self, + sandbox_id: &str, + sandbox_name: &str, + ) -> Result { + self.driver + .call( + openshell_otel::rpc::DELETE_SANDBOX, + Some(sandbox_id), + |driver| { + let sandbox_id = sandbox_id.to_string(); + let sandbox_name = sandbox_name.to_string(); + async move { + driver + .delete_sandbox(Request::new(DeleteSandboxRequest { + sandbox_id, + name: sandbox_name, + })) + .await + } + }, + ) + .await + .map(|response| response.into_inner().deleted) + } + pub(crate) async fn stop_sandbox( &self, workspace: &str, @@ -7166,6 +7188,67 @@ mod tests { ); } + #[tokio::test] + async fn create_compensation_deletes_backend_when_delete_transition_cannot_be_stored() { + let directory = tempfile::tempdir().expect("temporary database directory"); + let database_url = format!("sqlite://{}", directory.path().join("gateway.db").display()); + let store = Arc::new(Store::connect(&database_url).await.expect("connect store")); + let pool = sqlx::SqlitePool::connect(&database_url) + .await + .expect("connect failure injector"); + sqlx::query( + "CREATE TRIGGER reject_test_payload_updates \ + BEFORE UPDATE OF payload ON objects \ + BEGIN SELECT RAISE(ABORT, 'injected payload persistence failure'); END", + ) + .execute(&pool) + .await + .expect("install persistence failure trigger"); + pool.close().await; + + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + let mut runtime = test_runtime(driver.clone()).await; + runtime.store = store; + enable_runtime_identity_binding(&mut runtime); + let sandbox = sandbox_record( + "sb-create-transition-failure", + "create-transition-failure", + SandboxPhase::Provisioning, + ); + + let error = runtime + .create_sandbox(sandbox.clone(), None, false) + .await + .expect_err("binding and delete-transition persistence failures must fail create"); + + assert!( + error + .message() + .contains("persist compute runtime identity failed") + ); + assert!( + error + .message() + .contains("could not claim the sandbox record") + ); + assert_eq!(driver.delete_calls(), 1); + assert_eq!( + driver.delete_requests(), + vec![( + sandbox.object_id().to_string(), + sandbox.object_name().to_string() + )] + ); + let retained = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .expect("failed durable transition must retain the original record"); + assert_eq!(retained.phase(), SandboxPhase::Provisioning as i32); + } + #[tokio::test] async fn create_merges_runtime_binding_after_watch_update() { let driver = ControlledDriver::new(); From dcd5f1870c34a60a088d57311d00e8cf1f659ba4 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:58:52 -0700 Subject: [PATCH 4/6] fix(compute): merge runtime binding after start Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- crates/openshell-server/src/compute/mod.rs | 213 +++++++++++++++++++-- 1 file changed, 198 insertions(+), 15 deletions(-) diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 8c1786aa38..382278e49e 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -1707,22 +1707,11 @@ impl ComputeRuntime { let latest = if self.supports_sandbox_authentication() { let driver_name = self.configured_driver_name().to_string(); let persisted = self - .store - .update_message_cas::( + .persist_start_runtime_binding( &sandbox_id, - sandbox_resource_version(&starting), - move |sandbox| { - if let Some(metadata) = sandbox.metadata.as_mut() { - metadata.annotations.insert( - COMPUTE_DRIVER_ANNOTATION.to_string(), - driver_name.clone(), - ); - metadata.annotations.insert( - COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), - runtime_identity.clone(), - ); - } - }, + &starting, + &driver_name, + &runtime_identity, ) .await; match persisted { @@ -1764,6 +1753,80 @@ impl ComputeRuntime { } } + async fn persist_start_runtime_binding( + &self, + sandbox_id: &str, + starting: &Sandbox, + driver_name: &str, + runtime_identity: &str, + ) -> Result { + let expected_generation = sandbox_runtime_generation(starting)?; + let mut expected_resource_version = sandbox_resource_version(starting); + + for attempt in 1..=START_PHASE_CAS_RETRY_LIMIT { + let driver_name = driver_name.to_string(); + let runtime_identity = runtime_identity.to_string(); + match self + .store + .update_message_cas::( + sandbox_id, + expected_resource_version, + move |sandbox| { + if let Some(metadata) = sandbox.metadata.as_mut() { + metadata + .annotations + .insert(COMPUTE_DRIVER_ANNOTATION.to_string(), driver_name.clone()); + metadata.annotations.insert( + COMPUTE_RUNTIME_IDENTITY_ANNOTATION.to_string(), + runtime_identity.clone(), + ); + } + }, + ) + .await + { + Ok(sandbox) => return Ok(sandbox), + Err(crate::persistence::PersistenceError::Conflict { .. }) + if attempt < START_PHASE_CAS_RETRY_LIMIT => + { + let current = self + .store + .get_message::(sandbox_id) + .await + .map_err(|error| error.to_string())? + .ok_or_else(|| { + "sandbox was removed while persisting runtime identity".to_string() + })?; + let current_generation = sandbox_runtime_generation(¤t)?; + let phase = + SandboxPhase::try_from(current.phase()).unwrap_or(SandboxPhase::Unknown); + if current_generation != expected_generation + || !matches!( + phase, + SandboxPhase::Starting + | SandboxPhase::Provisioning + | SandboxPhase::Ready + ) + { + return Err(format!( + "sandbox changed lifecycle ownership while persisting runtime identity (phase: {phase:?})" + )); + } + expected_resource_version = sandbox_resource_version(¤t); + debug!( + sandbox_id, + attempt, + expected_resource_version, + "Retrying runtime identity persistence after concurrent start progress" + ); + } + Err(error) => return Err(error.to_string()), + } + } + + unreachable!("runtime identity persistence retry loop always returns") + } + async fn compensate_successful_start( &self, lifecycle_guard: &SandboxLifecycleGuard, @@ -7376,6 +7439,126 @@ mod tests { ); } + #[tokio::test] + async fn start_merges_runtime_binding_after_supervisor_connects() { + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + driver.block_start(); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let mut sandbox = sandbox_record( + "sb-start-supervisor-race", + "start-supervisor-race", + SandboxPhase::Stopped, + ); + set_compute_runtime_binding(&mut sandbox, "previous-runtime-identity"); + runtime.store.put_message(&sandbox).await.unwrap(); + + let start_runtime = runtime.clone(); + let sandbox_name = sandbox.object_name().to_string(); + let start = + tokio::spawn( + async move { start_runtime.start_sandbox("default", &sandbox_name).await }, + ); + tokio::time::timeout(Duration::from_secs(1), driver.start_started.notified()) + .await + .expect("start did not reach the driver"); + + runtime + .supervisor_session_connected(sandbox.object_id(), "replacement-instance") + .await + .expect("replacement supervisor must connect while start is in flight"); + driver.release_start(); + + let started = start + .await + .expect("start task must finish") + .expect("start must merge its binding with supervisor readiness"); + assert_eq!( + started.metadata.as_ref().unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "new-runtime-identity" + ); + assert_eq!( + started.status.as_ref().unwrap().main_process_instance_id, + "replacement-instance" + ); + assert_eq!(driver.stop_calls(), 0); + } + + #[tokio::test] + async fn start_does_not_bind_runtime_identity_after_generation_changes() { + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + driver.block_start(); + let mut runtime = test_runtime(driver.clone()).await; + enable_runtime_identity_binding(&mut runtime); + let mut sandbox = sandbox_record( + "sb-start-generation-race", + "start-generation-race", + SandboxPhase::Stopped, + ); + set_compute_runtime_binding(&mut sandbox, "previous-runtime-identity"); + runtime.store.put_message(&sandbox).await.unwrap(); + + let start_runtime = runtime.clone(); + let sandbox_name = sandbox.object_name().to_string(); + let start = + tokio::spawn( + async move { start_runtime.start_sandbox("default", &sandbox_name).await }, + ); + tokio::time::timeout(Duration::from_secs(1), driver.start_started.notified()) + .await + .expect("start did not reach the driver"); + + let starting = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .expect("starting record must exist"); + runtime + .store + .update_message_cas::( + sandbox.object_id(), + sandbox_resource_version(&starting), + |sandbox| { + sandbox.metadata.as_mut().unwrap().annotations.insert( + crate::auth::sandbox_session::RUNTIME_GENERATION_ANNOTATION.to_string(), + "replacement-generation".to_string(), + ); + }, + ) + .await + .expect("replace runtime generation while start is in flight"); + driver.release_start(); + + let error = start + .await + .expect("start task must finish") + .expect_err("a replaced generation must reject the prior runtime binding"); + assert!( + error + .message() + .contains("changed lifecycle ownership while persisting runtime identity") + ); + assert_eq!(driver.stop_calls(), 1); + let retained = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .expect("replacement record must be retained"); + assert_eq!( + retained.metadata.as_ref().unwrap().annotations + [crate::auth::sandbox_session::RUNTIME_GENERATION_ANNOTATION], + "replacement-generation" + ); + assert_eq!( + retained.metadata.as_ref().unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "previous-runtime-identity" + ); + } + fn register_test_supervisor_session(runtime: &ComputeRuntime, sandbox_id: &str) { let (tx, _rx) = mpsc::channel(1); let (shutdown_tx, _shutdown_rx) = oneshot::channel(); From 6a900c60ff3ab659cf6643279b9d8369af85d182 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:50:51 -0700 Subject: [PATCH 5/6] fix(auth): bind restarted sandbox sessions Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- .agents/skills/helm-dev-environment/SKILL.md | 8 +- architecture/compute-runtimes.md | 17 ++- architecture/gateway.md | 9 +- crates/openshell-driver-kubernetes/README.md | 9 +- .../openshell-driver-kubernetes/src/driver.rs | 144 +++++++++++++++++- .../openshell-driver-kubernetes/src/grpc.rs | 1 + crates/openshell-server/src/compute/mod.rs | 133 ++++++++++++++-- crates/openshell-server/src/grpc/auth_rpc.rs | 77 ++++++++-- crates/openshell-server/src/multiplex.rs | 13 +- docs/reference/gateway-config.mdx | 2 +- proto/compute_driver.proto | 5 + proto/openshell.proto | 3 +- sdk/go/proto/openshellv1/openshell.pb.go | 3 +- skills/debug-openshell-cluster/SKILL.md | 6 +- 14 files changed, 383 insertions(+), 47 deletions(-) diff --git a/.agents/skills/helm-dev-environment/SKILL.md b/.agents/skills/helm-dev-environment/SKILL.md index 228e12988a..b17831beb5 100644 --- a/.agents/skills/helm-dev-environment/SKILL.md +++ b/.agents/skills/helm-dev-environment/SKILL.md @@ -359,9 +359,11 @@ OpenShell mounts the SPIFFE CSI Workload API socket at grants. Supervisor-to-gateway authentication remains on the Kubernetes ServiceAccount bootstrap and gateway-minted sandbox JWT path; the selected Kubernetes compute driver validates the projected token before the gateway -mints its JWT. The driver also returns the runtime identity recorded during -provisioning, and the gateway rejects bootstrap when that identity does not -match the durable sandbox record. +returns the current generation-bound session JWT. The driver also returns the +runtime identity recorded during provisioning. Restart preserves its namespace +and Sandbox CR UID, rejects ambiguous label matches, and rotates only the +supervisor Pod UID; bootstrap fails when the live identity does not match the +durable sandbox record. ### Vault Credential Driver diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index ee35fbf892..20e8f37e72 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -226,11 +226,13 @@ conservative operator-managed behavior. Drivers that can verify a platform-native sandbox credential advertise `GetCapabilities.supports_sandbox_authentication`. On the path-scoped `IssueSandboxToken` exchange, the gateway forwards the opaque bearer credential -to that selected driver through `AuthenticateSandbox`. The driver returns only -the authenticated sandbox ID. The gateway then verifies that its durable -sandbox record exists and mints the gateway JWT. The driver socket is therefore -a sandbox-identity trust boundary, but it does not grant user or administrator -authority. +to that selected driver through `AuthenticateSandbox`. The driver returns the +authenticated sandbox ID and opaque runtime identity. The gateway verifies +that both match its durable sandbox record and returns a generation-bound +session JWT whose lineage is checked on every subsequent sandbox RPC. Legacy +unbound sandbox JWTs are not admitted when session authentication is enabled. +The driver socket is therefore a sandbox-identity trust boundary, but it does +not grant user or administrator authority. ## Deletion Lifecycle @@ -517,6 +519,11 @@ fails after a lifecycle call succeeds, the gateway compensates that call before returning the error. This correlates credential authentication with the durable runtime record rather than authorizing from the sandbox ID alone. +`StartSandbox` carries the previously recorded opaque identity. Kubernetes +requires exactly one label-selected Sandbox CR and verifies that its namespace +and immutable UID match that identity before replacing the supervisor Pod. The +new Pod UID becomes the updated binding only after the continuity check passes. + Shared and managed modes still reserve the sandbox namespace, Sandbox CRs, sandbox pods, and configured sandbox ServiceAccount for the Kubernetes driver and trusted Agent Sandbox controller. In operator mode, the platform operator diff --git a/architecture/gateway.md b/architecture/gateway.md index 4113420a63..0c221fb3a7 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -320,10 +320,15 @@ token through `IssueSandboxToken`. The gateway delegates that opaque credential to the selected compute driver's `AuthenticateSandbox` RPC. A capable driver returns the authenticated sandbox ID and an opaque runtime identity. The gateway requires both a matching durable sandbox record and the exact -driver/runtime identity recorded at provisioning before minting a JWT. The +driver/runtime identity recorded at provisioning before returning the current +generation-bound session JWT. Session authentication checks the durable runtime +generation and token lineage for every sandbox RPC, so a replaced runtime and +legacy unbound tokens cannot retain provider or control-plane access. The Kubernetes driver uses its own named configuration to run TokenReview and verify the live pod and controlling Sandbox CR. Its runtime identity binds the -namespace, immutable Sandbox CR UID, and supervisor Pod UID. The bootstrap path accepts +namespace, immutable Sandbox CR UID, and supervisor Pod UID. Restart preserves +the namespace and CR UID, rejects ambiguous label matches, and rotates only the +Pod-bound portion of the identity. The bootstrap path accepts both `agents.x-k8s.io/v1beta1` ownerReferences from newer Agent Sandbox controllers and `agents.x-k8s.io/v1alpha1` ownerReferences from existing deployments. Supervisors renew gateway JWTs in memory before expiry only while diff --git a/crates/openshell-driver-kubernetes/README.md b/crates/openshell-driver-kubernetes/README.md index 0ca05b0806..07f8a0ffe1 100644 --- a/crates/openshell-driver-kubernetes/README.md +++ b/crates/openshell-driver-kubernetes/README.md @@ -142,10 +142,11 @@ Both Pods set `automountServiceAccountToken: false`. The supervisor receives an explicit audience-bound projected token for the one-shot `IssueSandboxToken` exchange. The driver verifies that token and returns an opaque runtime identity derived from the namespace, immutable Sandbox resource UID, and supervisor Pod -UID. The gateway -requires that identity to match the value recorded during provisioning before -returning the sandbox-scoped JWT used by the supervisor session. The sandbox -Pod receives neither token. +UID. Restart requires exactly one matching Sandbox resource and preserves its +namespace and UID while rotating the supervisor Pod UID. The gateway requires +the authenticated identity to match the durable binding before returning the +generation-bound session JWT used by the supervisor. The sandbox Pod receives +neither token. The gateway uses the supervisor relay for connect, exec, logs, and file sync. Sandbox Pods do not need direct external ingress for SSH. diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index ddb652782b..26390c4b4e 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -2826,12 +2826,14 @@ impl KubernetesComputeDriver { sandbox_id: &str, generation_id: &str, launch_authentication: &[u8], + expected_runtime_identity: &str, ) -> Result { let span_status = openshell_otel::ErrorStatusGuard::current(); let result = Box::pin(self.start_sandbox_runtime_generation( sandbox_id, generation_id, launch_authentication, + expected_runtime_identity, )) .await; span_status.finish(result) @@ -2843,24 +2845,27 @@ impl KubernetesComputeDriver { sandbox_id: &str, encoded_generation: &str, encoded_authentication: &[u8], + encoded_expected_runtime_identity: &str, ) -> Result { let generation = openshell_core::sandbox_generation::SandboxGenerationId::parse( encoded_generation.to_string(), ) .map_err(|error| KubernetesDriverError::InvalidArgument(error.to_string()))?; let launch_authentication = decode_launch_authentication(encoded_authentication)?; + let expected_runtime_identity = + parse_kubernetes_runtime_identity(encoded_expected_runtime_identity)?; let lookup_api = self .supported_sandbox_api_for_lookup(self.client.clone()) .await .map_err(KubernetesDriverError::Message)?; let selector = self.sandbox_lookup_selector(sandbox_id); - let mut objects = lookup_api + let objects = lookup_api .api .list(&ListParams::default().labels(&selector)) .await .map_err(KubernetesDriverError::from_kube)? .items; - let mut object = objects.pop().ok_or(KubernetesDriverError::NotFound)?; + let mut object = select_expected_sandbox_runtime(objects, &expected_runtime_identity)?; if sandbox_runtime_bootstrap_in_progress(&object) { let phase = sandbox_runtime_bootstrap_phase(&object); if phase != Some(SandboxRuntimeBootstrapPhase::Suspending) @@ -2906,13 +2911,13 @@ impl KubernetesComputeDriver { // after gateway replacement because launch credentials stay in // memory and are supplied again by the caller. self.stop_sandbox_inner(sandbox_id).await?; - let mut refreshed = lookup_api + let refreshed = lookup_api .api .list(&ListParams::default().labels(&selector)) .await .map_err(KubernetesDriverError::from_kube)? .items; - object = refreshed.pop().ok_or(KubernetesDriverError::NotFound)?; + object = select_expected_sandbox_runtime(refreshed, &expected_runtime_identity)?; } let namespace = object .metadata @@ -2946,6 +2951,7 @@ impl KubernetesComputeDriver { sandbox_id, encoded_generation, encoded_authentication, + encoded_expected_runtime_identity, )) .await; } @@ -4630,6 +4636,72 @@ fn kubernetes_runtime_identity(namespace: &str, resource_uid: &str, pod_uid: &st format!("kubernetes://{namespace}/{resource_uid}/{pod_uid}") } +#[derive(Debug, PartialEq, Eq)] +struct KubernetesRuntimeIdentity { + namespace: String, + resource_uid: String, +} + +fn parse_kubernetes_runtime_identity( + encoded: &str, +) -> Result { + let value = encoded.strip_prefix("kubernetes://").ok_or_else(|| { + KubernetesDriverError::Precondition( + "persisted runtime identity is missing or is not a Kubernetes identity".to_string(), + ) + })?; + let mut components = value.split('/'); + let namespace = components.next().unwrap_or_default(); + let resource_uid = components.next().unwrap_or_default(); + let pod_uid = components.next().unwrap_or_default(); + if namespace.is_empty() + || resource_uid.is_empty() + || pod_uid.is_empty() + || components.next().is_some() + { + return Err(KubernetesDriverError::Precondition( + "persisted Kubernetes runtime identity is invalid".to_string(), + )); + } + Ok(KubernetesRuntimeIdentity { + namespace: namespace.to_string(), + resource_uid: resource_uid.to_string(), + }) +} + +fn select_expected_sandbox_runtime( + objects: Vec, + expected: &KubernetesRuntimeIdentity, +) -> Result { + let [object]: [DynamicObject; 1] = objects.try_into().map_err(|objects: Vec<_>| { + if objects.is_empty() { + KubernetesDriverError::NotFound + } else { + KubernetesDriverError::Precondition( + "multiple Kubernetes Sandbox resources match the persisted sandbox identity" + .to_string(), + ) + } + })?; + let namespace = object.metadata.namespace.as_deref().ok_or_else(|| { + KubernetesDriverError::Precondition( + "matched Kubernetes Sandbox resource has no namespace".to_string(), + ) + })?; + let resource_uid = object.metadata.uid.as_deref().ok_or_else(|| { + KubernetesDriverError::Precondition( + "matched Kubernetes Sandbox resource has no UID".to_string(), + ) + })?; + if namespace != expected.namespace || resource_uid != expected.resource_uid { + return Err(KubernetesDriverError::Precondition( + "matched Kubernetes Sandbox resource does not match the persisted runtime identity" + .to_string(), + )); + } + Ok(object) +} + fn sandbox_id_from_object(obj: &DynamicObject) -> Result { if let Some(annotations) = obj.metadata.annotations.as_ref() && let Some(id) = annotations.get(LABEL_SANDBOX_ID) @@ -7086,6 +7158,7 @@ mod tests { "sandbox-1", "invalid-generation", b"secret-launch-authentication", + "kubernetes://namespace/resource/pod", ) .with_subscriber(subscriber) .await @@ -7421,6 +7494,7 @@ mod tests { )); let mut sandbox = DynamicObject::new("sandbox-a", &resource); sandbox.metadata.uid = Some(uid.to_string()); + sandbox.metadata.namespace = Some("sandbox-namespace".to_string()); sandbox.metadata.labels = Some(BTreeMap::from([( LABEL_SANDBOX_ID.to_string(), sandbox_id.to_string(), @@ -7428,6 +7502,68 @@ mod tests { sandbox } + #[test] + fn restart_runtime_selection_preserves_namespace_and_resource_uid() { + let expected = parse_kubernetes_runtime_identity( + "kubernetes://sandbox-namespace/resource-uid/old-supervisor-uid", + ) + .expect("persisted identity"); + let sandbox = sandbox_object_for_test("resource-uid", "sandbox-id-a"); + let selected = select_expected_sandbox_runtime(vec![sandbox], &expected) + .expect("matching runtime must be selected"); + assert_eq!(selected.metadata.uid.as_deref(), Some("resource-uid")); + + let wrong_namespace = KubernetesRuntimeIdentity { + namespace: "other-namespace".to_string(), + resource_uid: "resource-uid".to_string(), + }; + assert!(matches!( + select_expected_sandbox_runtime( + vec![sandbox_object_for_test("resource-uid", "sandbox-id-a")], + &wrong_namespace, + ), + Err(KubernetesDriverError::Precondition(_)) + )); + + let wrong_uid = KubernetesRuntimeIdentity { + namespace: "sandbox-namespace".to_string(), + resource_uid: "replacement-resource-uid".to_string(), + }; + assert!(matches!( + select_expected_sandbox_runtime( + vec![sandbox_object_for_test("resource-uid", "sandbox-id-a")], + &wrong_uid, + ), + Err(KubernetesDriverError::Precondition(_)) + )); + } + + #[test] + fn restart_runtime_selection_rejects_ambiguous_and_invalid_bindings() { + let expected = parse_kubernetes_runtime_identity( + "kubernetes://sandbox-namespace/resource-uid/old-supervisor-uid", + ) + .expect("persisted identity"); + assert!(matches!( + select_expected_sandbox_runtime( + vec![ + sandbox_object_for_test("resource-uid", "sandbox-id-a"), + sandbox_object_for_test("resource-uid", "sandbox-id-a"), + ], + &expected, + ), + Err(KubernetesDriverError::Precondition(_)) + )); + assert!(matches!( + parse_kubernetes_runtime_identity(""), + Err(KubernetesDriverError::Precondition(_)) + )); + assert!(matches!( + parse_kubernetes_runtime_identity("kubernetes://namespace/resource-only"), + Err(KubernetesDriverError::Precondition(_)) + )); + } + #[test] fn pod_identity_requires_matching_uid_annotation_and_expected_owner_kind() { let owner = sandbox_owner_for_test("sandbox-a", "sandbox-uid-a"); diff --git a/crates/openshell-driver-kubernetes/src/grpc.rs b/crates/openshell-driver-kubernetes/src/grpc.rs index ccb3a03357..12cd0ebfff 100644 --- a/crates/openshell-driver-kubernetes/src/grpc.rs +++ b/crates/openshell-driver-kubernetes/src/grpc.rs @@ -203,6 +203,7 @@ impl ComputeDriver for ComputeDriverService { &request.sandbox_id, &request.generation_id, &request.launch_authentication, + &request.expected_runtime_identity, )) .await .map_err(|error| { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 382278e49e..27fd69d03e 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -1631,6 +1631,7 @@ impl ComputeRuntime { let generation_id = sandbox_runtime_generation(&starting) .map_err(Status::failed_precondition)? .into_string(); + let expected_runtime_identity = sandbox_compute_runtime_identity(&previous); let authentication_for_recreate = launch_authentication.clone(); let mut result = self .await_provisioning_operation( @@ -1648,6 +1649,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication, generation_id, + expected_runtime_identity, })) .await } @@ -1712,6 +1714,11 @@ impl ComputeRuntime { &starting, &driver_name, &runtime_identity, + &[ + SandboxPhase::Starting, + SandboxPhase::Provisioning, + SandboxPhase::Ready, + ], ) .await; match persisted { @@ -1759,6 +1766,7 @@ impl ComputeRuntime { starting: &Sandbox, driver_name: &str, runtime_identity: &str, + allowed_phases: &[SandboxPhase], ) -> Result { let expected_generation = sandbox_runtime_generation(starting)?; let mut expected_resource_version = sandbox_resource_version(starting); @@ -1800,13 +1808,7 @@ impl ComputeRuntime { let current_generation = sandbox_runtime_generation(¤t)?; let phase = SandboxPhase::try_from(current.phase()).unwrap_or(SandboxPhase::Unknown); - if current_generation != expected_generation - || !matches!( - phase, - SandboxPhase::Starting - | SandboxPhase::Provisioning - | SandboxPhase::Ready - ) + if current_generation != expected_generation || !allowed_phases.contains(&phase) { return Err(format!( "sandbox changed lifecycle ownership while persisting runtime identity (phase: {phase:?})" @@ -2934,6 +2936,7 @@ impl ComputeRuntime { continue; } }; + let expected_runtime_identity = sandbox_compute_runtime_identity(&sandbox); match self .await_provisioning_operation( &sandbox, @@ -2944,6 +2947,7 @@ impl ComputeRuntime { let sandbox_id = sandbox_id.clone(); let sandbox_name = sandbox_name.clone(); let launch_authentication = launch_authentication.clone(); + let expected_runtime_identity = expected_runtime_identity.clone(); async move { driver .start_sandbox(Request::new(StartSandboxRequest { @@ -2951,6 +2955,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication, generation_id, + expected_runtime_identity, })) .await } @@ -2959,7 +2964,49 @@ impl ComputeRuntime { ) .await { - Ok(_) => { + Ok(response) => { + let mut recovered_sandbox = sandbox.clone(); + if self.supports_sandbox_authentication() { + let runtime_identity = response.into_inner().runtime_identity; + if runtime_identity.is_empty() { + warn!( + sandbox_id = %sandbox.object_id(), + "Compute driver returned an empty runtime identity during startup recovery" + ); + authentication_failed(sandbox.object_id()); + failed += 1; + continue; + } + match self + .persist_start_runtime_binding( + &sandbox_id, + &sandbox, + self.configured_driver_name(), + &runtime_identity, + &[ + SandboxPhase::Starting, + SandboxPhase::Provisioning, + SandboxPhase::Ready, + SandboxPhase::Error, + SandboxPhase::Unspecified, + SandboxPhase::Unknown, + ], + ) + .await + { + Ok(updated) => recovered_sandbox = updated, + Err(error) => { + warn!( + sandbox_id = %sandbox.object_id(), + %error, + "Failed to persist runtime identity during startup recovery" + ); + authentication_failed(sandbox.object_id()); + failed += 1; + continue; + } + } + } if let Err(err) = authentication_committed(sandbox.object_id()).await { warn!( sandbox_id = %sandbox.object_id(), @@ -2968,7 +3015,7 @@ impl ComputeRuntime { ); } let did_recover = if recoverable_error { - self.clear_recoverable_error(&sandbox).await + self.clear_recoverable_error(&recovered_sandbox).await } else { false }; @@ -3130,6 +3177,7 @@ impl ComputeRuntime { continue; } }; + let expected_runtime_identity = sandbox_compute_runtime_identity(&sandbox); if let Err(err) = self .driver .call( @@ -3142,6 +3190,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication: Vec::new(), generation_id, + expected_runtime_identity, })) .await }, @@ -5099,6 +5148,19 @@ fn sandbox_resource_version(sandbox: &Sandbox) -> u64 { .map_or(0, |metadata| metadata.resource_version) } +fn sandbox_compute_runtime_identity(sandbox: &Sandbox) -> String { + sandbox + .metadata + .as_ref() + .and_then(|metadata| { + metadata + .annotations + .get(COMPUTE_RUNTIME_IDENTITY_ANNOTATION) + }) + .cloned() + .unwrap_or_default() +} + fn sandbox_runtime_generation( sandbox: &Sandbox, ) -> Result { @@ -6608,6 +6670,7 @@ mod tests { start_calls: AtomicUsize, start_requests: TestMutex>, start_authentications: TestMutex>>, + start_expected_runtime_identities: TestMutex>, start_outcome: TestMutex, runtime_identity: TestMutex, advertises_sandbox_authentication: AtomicBool, @@ -6648,6 +6711,7 @@ mod tests { start_calls: AtomicUsize::new(0), start_requests: TestMutex::new(Vec::new()), start_authentications: TestMutex::new(Vec::new()), + start_expected_runtime_identities: TestMutex::new(Vec::new()), start_outcome: TestMutex::new(ControlledLifecycleOutcome::Ok), runtime_identity: TestMutex::new(String::new()), advertises_sandbox_authentication: AtomicBool::new(false), @@ -6778,6 +6842,13 @@ mod tests { .clone() } + fn start_expected_runtime_identities(&self) -> Vec { + self.start_expected_runtime_identities + .lock() + .expect("start expected runtime identities lock poisoned") + .clone() + } + fn send_event(&self, event: WatchSandboxesEvent) { self.watch_tx .send(Ok(event)) @@ -6943,6 +7014,10 @@ mod tests { .lock() .expect("start authentications lock poisoned") .push(request.launch_authentication); + self.start_expected_runtime_identities + .lock() + .expect("start expected runtime identities lock poisoned") + .push(request.expected_runtime_identity); self.start_calls.fetch_add(1, Ordering::SeqCst); self.start_started.notify_one(); if self.start_blocked.load(Ordering::SeqCst) { @@ -7483,6 +7558,10 @@ mod tests { "replacement-instance" ); assert_eq!(driver.stop_calls(), 0); + assert_eq!( + driver.start_expected_runtime_identities(), + vec!["previous-runtime-identity".to_string()] + ); } #[tokio::test] @@ -12750,6 +12829,42 @@ mod tests { ); } + #[tokio::test] + async fn start_persisted_sandboxes_preserves_and_updates_runtime_binding() { + let driver = ControlledDriver::new(); + driver.set_runtime_identity("new-runtime-identity"); + let mut runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await; + enable_runtime_identity_binding(&mut runtime); + let mut sandbox = sandbox_record("sb-1", "sandbox", SandboxPhase::Ready); + set_compute_runtime_binding(&mut sandbox, "previous-runtime-identity"); + runtime.store.put_message(&sandbox).await.unwrap(); + + runtime + .start_persisted_sandboxes_with_authentication( + |_| async { Ok(b"authentication".to_vec()) }, + |_| async { Ok(()) }, + |_| {}, + ) + .await + .unwrap(); + + assert_eq!( + driver.start_expected_runtime_identities(), + vec!["previous-runtime-identity".to_string()] + ); + let restored = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .expect("sandbox must remain persisted"); + assert_eq!( + restored.metadata.unwrap().annotations[COMPUTE_RUNTIME_IDENTITY_ANNOTATION], + "new-runtime-identity" + ); + } + #[tokio::test] async fn startup_sweep_rechecks_intent_after_acquiring_gate() { let driver = ControlledDriver::new(); diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index 60255dfd5d..ec494e9434 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -89,13 +89,16 @@ pub async fn handle_issue_sandbox_token( )); }; - let issuer = state.sandbox_jwt_issuer.as_ref().ok_or_else(|| { - warn!( - sandbox_id = %sandbox.sandbox_id, - "IssueSandboxToken called but sandbox JWT issuer is not configured" - ); - Status::unavailable("sandbox JWT minting is not configured on this gateway") - })?; + let session_authority = state + .sandbox_session_jwt_authority + .as_ref() + .ok_or_else(|| { + warn!( + sandbox_id = %sandbox.sandbox_id, + "IssueSandboxToken called but sandbox session minting is not configured" + ); + Status::unavailable("sandbox session minting is not configured on this gateway") + })?; let sandbox_record = ensure_sandbox_exists(state, &sandbox.sandbox_id).await?; let metadata = sandbox_record @@ -119,15 +122,26 @@ pub async fn handle_issue_sandbox_token( )); } - let minted = issuer.mint(&sandbox.sandbox_id)?; + let identity = + crate::auth::sandbox_session::PersistedSandboxIdentity::read(&metadata.annotations) + .map_err(|_| Status::permission_denied("sandbox runtime identity is invalid"))?; + let authentication = session_authority.mint_persisted_launch(&sandbox.sandbox_id, &identity)?; + let token = authentication + .supervisor + .gateway_token + .expose_secret() + .to_string(); info!( sandbox_id = %sandbox.sandbox_id, - "issued gateway sandbox JWT" + "issued generation-bound gateway sandbox JWT" ); Ok(Response::new(IssueSandboxTokenResponse { - token: minted.token, + token, expiration_time: openshell_core::time::optional_timestamp_from_legacy_millis( - minted.expires_at_ms, + authentication + .supervisor + .gateway_expires_at + .saturating_mul(1000), ) .map_err(|error| Status::internal(error.to_string()))?, })) @@ -799,8 +813,10 @@ mod tests { } #[tokio::test] - async fn issue_returns_token_for_existing_sandbox() { + async fn issue_returns_generation_bound_token_and_rejects_it_after_runtime_replacement() { + use crate::auth::authenticator::Authenticator; use crate::auth::principal::SandboxIdentitySource; + use crate::auth::sandbox_jwt::SandboxSessionJwtAuthenticator; let state = state_with_issuer().await; let mut req = Request::new(IssueSandboxTokenRequest {}); @@ -819,6 +835,43 @@ mod tests { .into_inner(); assert!(!resp.token.is_empty()); assert!(resp.expiration_time.is_some()); + + let authenticator = SandboxSessionJwtAuthenticator::new( + state + .sandbox_session_jwt_authority + .clone() + .expect("session authority"), + state.store.clone(), + ); + let mut headers = http::HeaderMap::new(); + headers.insert( + "authorization", + format!("Bearer {}", resp.token) + .parse() + .expect("bearer header"), + ); + let provider_path = "/openshell.v1.OpenShell/GetSandboxProviderEnvironment"; + let principal = authenticator + .authenticate(&headers, provider_path) + .await + .expect("active runtime token must authenticate") + .expect("session authenticator must recognize its token"); + assert!(matches!(principal, Principal::Sandbox(_))); + + let replacement = crate::auth::sandbox_session::PersistedSandboxIdentity::new() + .expect("replacement identity"); + state + .store + .update_message_cas::("sandbox-a", 0, move |sandbox| { + replacement.write(&mut sandbox.metadata.as_mut().expect("metadata").annotations); + }) + .await + .expect("replace runtime identity"); + let error = authenticator + .authenticate(&headers, provider_path) + .await + .expect_err("obsolete runtime token must not reach provider access"); + assert_eq!(error.code(), tonic::Code::Unauthenticated); } #[tokio::test] diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 6ec78d7f6d..45630a05f7 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -866,9 +866,13 @@ where /// 2. `ComputeDriverAuthenticator` (path-scoped to `IssueSandboxToken`) /// — delegates a driver-native credential and receives a sandbox identity /// so the handler can mint a gateway JWT. No-op on every other path. -/// 3. `SandboxJwtAuthenticator` — validates gateway-minted JWTs. Recognized -/// via a distinctive `kid` so non-matching Bearer tokens fall through. -/// 4. `OidcAuthenticator` — validates user Bearer tokens against the +/// 3. `SandboxSessionJwtAuthenticator` — validates generation-bound gateway +/// JWTs against the durable sandbox identity. When configured, legacy +/// unbound sandbox JWTs are deliberately not admitted. +/// 4. `SandboxJwtAuthenticator` — legacy fallback used only when session JWT +/// authentication is unavailable. Recognized via a distinctive `kid` so +/// non-matching Bearer tokens fall through. +/// 5. `OidcAuthenticator` — validates user Bearer tokens against the /// configured OIDC issuer. Returns `Unauthenticated` for missing /// Bearer headers so non-OIDC clients can't sneak through. /// @@ -889,6 +893,7 @@ fn build_authenticator_chain(state: &ServerState) -> Option if let Some(driver) = state.compute_driver_authenticator.clone() { authenticators.push(driver); } + let session_authentication_enabled = state.sandbox_session_jwt_authority.is_some(); if let Some(authority) = state.sandbox_session_jwt_authority.clone() { authenticators.push(Arc::new( crate::auth::sandbox_jwt::SandboxSessionJwtAuthenticator::new( @@ -897,7 +902,7 @@ fn build_authenticator_chain(state: &ServerState) -> Option ), )); } - if let Some(jwt) = state.sandbox_jwt_authenticator.clone() { + if !session_authentication_enabled && let Some(jwt) = state.sandbox_jwt_authenticator.clone() { authenticators.push(jwt); } if let Some(cache) = state.oidc_cache.clone() { diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 5d2873bdfc..5999176975 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -317,7 +317,7 @@ gateway under the worktree-specific k3d cluster name; select it with `openshell gateway select `. The local Podman, Docker, and VM gateway tasks export to the forwarded receiver automatically. -In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`; configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. It must return the same non-empty runtime identity from sandbox creation and credential authentication. For `IssueSandboxToken`, the gateway verifies the sandbox exists and the driver/runtime identity matches the durable provisioning record before minting its own JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. +In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`; configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. It must return the same non-empty runtime identity from sandbox creation and credential authentication. Start requests include the previously recorded opaque identity so the driver can preserve the stable resource while rotating generation-specific compute. For `IssueSandboxToken`, the gateway verifies the sandbox exists and the driver/runtime identity matches the durable provisioning record before returning a generation-bound session JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. ### Tuning diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 2b83c6bc0e..9ea1fe263e 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -416,6 +416,11 @@ message StartSandboxRequest { // Stable identity of this gateway start transition. Retries of the same // transition carry the same generation ID; a later start uses a new ID. string generation_id = 4; + // Opaque runtime identity persisted from the prior successful create or + // start. Drivers that advertise runtime identity binding must preserve the + // stable resource represented by this identity while replacing only the + // generation-specific runtime component. + string expected_runtime_identity = 5; } message StartSandboxResponse { diff --git a/proto/openshell.proto b/proto/openshell.proto index 232314e855..0623943f87 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -827,7 +827,8 @@ message IssueSandboxTokenRequest {} message IssueSandboxTokenResponse { reserved 2; reserved "expires_at_ms"; - // Gateway-minted JWT bound to the calling sandbox's UUID. + // Gateway-minted session JWT bound to the calling sandbox's UUID, active + // runtime generation, authorization epoch, and durable token lineage. string token = 1 [(openshell.options.v1.secret) = true]; // Absolute expiry of the issued token. Absence means the token is non-expiring. google.protobuf.Timestamp expiration_time = 102; diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index cc6fc19a1c..8b3438eff2 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -1186,7 +1186,8 @@ func (*IssueSandboxTokenRequest) Descriptor() ([]byte, []int) { // gateway RPC. type IssueSandboxTokenResponse struct { state protoimpl.MessageState `protogen:"open.v1"` - // Gateway-minted JWT bound to the calling sandbox's UUID. + // Gateway-minted session JWT bound to the calling sandbox's UUID, active + // runtime generation, authorization epoch, and durable token lineage. Token string `protobuf:"bytes,1,opt,name=token,proto3" json:"token,omitempty"` // Absolute expiry of the issued token. Absence means the token is non-expiring. ExpirationTime *timestamppb.Timestamp `protobuf:"bytes,102,opt,name=expiration_time,json=expirationTime,proto3" json:"expiration_time,omitempty"` diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 2b3fc424ed..67fb2288af 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -653,7 +653,11 @@ Drivers that advertise sandbox authentication must return the same non-empty runtime identity from sandbox creation and credential authentication. A gateway log reporting a compute runtime identity mismatch indicates stale or re-created runtime resources; compare the live resource UID with the sandbox -that the gateway provisioned. +that the gateway provisioned. Restart also rejects multiple Sandbox resources +with the same sandbox label and requires the persisted namespace and CR UID to +remain unchanged. A generation-bound session-token rejection usually means the +supervisor is presenting credentials from a runtime that was replaced; inspect +the persisted generation before retrying bootstrap. ```bash helm -n openshell get values openshell | grep -A3 sandboxServiceAccount From 915620967a7c2b6834aa15d216858df818ae3ec7 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Mon, 21 Sep 2026 20:28:51 -0700 Subject: [PATCH 6/6] refactor(compute): fold runtime binding into authentication Signed-off-by: Drew Newberry --- architecture/compute-runtimes.md | 15 +++--- crates/openshell-driver-docker/src/lib.rs | 1 - .../openshell-driver-kubernetes/src/driver.rs | 1 - crates/openshell-driver-mxc/src/driver.rs | 1 - crates/openshell-driver-podman/src/driver.rs | 1 - crates/openshell-driver-vm/src/driver.rs | 1 - crates/openshell-server/src/compute/mod.rs | 49 +------------------ crates/openshell-server/src/test_support.rs | 1 - proto/compute_driver.proto | 7 ++- 9 files changed, 12 insertions(+), 65 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 20e8f37e72..6e4fabb353 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -511,13 +511,14 @@ It validates the projected token with Kubernetes `TokenReview`, checks the live pod UID, and verifies the pod's controlling Sandbox CR UID and sandbox ID. The driver returns both the sandbox ID and an opaque runtime identity derived from the namespace, immutable Sandbox CR UID, and authenticated supervisor Pod UID. -Drivers that authenticate sandboxes must advertise this runtime-binding -contract; the gateway rejects incompatible drivers during initialization. The -gateway records the runtime identity when provisioning succeeds and requires -an exact match before issuing a sandbox JWT. If binding validation or storage -fails after a lifecycle call succeeds, the gateway compensates that call before -returning the error. This correlates credential authentication with the durable -runtime record rather than authorizing from the sandbox ID alone. +Advertising sandbox authentication includes the runtime-binding contract. The +gateway requires non-empty runtime identities from successful create, start, +and authentication responses. It records the runtime identity when provisioning +succeeds and requires an exact match before issuing a sandbox JWT. If binding +validation or storage fails after a lifecycle call succeeds, the gateway +compensates that call before returning the error. This correlates credential +authentication with the durable runtime record rather than authorizing from the +sandbox ID alone. `StartSandbox` carries the previously recorded opaque identity. Kubernetes requires exactly one label-selected Sandbox CR and verifies that its namespace diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index ae0a545634..bd19d7f8c3 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -962,7 +962,6 @@ impl DockerComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 26390c4b4e..bd71dd3545 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -752,7 +752,6 @@ impl KubernetesComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: false, supports_sandbox_authentication: true, - supports_runtime_identity_binding: true, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 538aa4c53e..23491533bb 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -458,7 +458,6 @@ impl MxcComputeBackend { default_image: DEFAULT_IMAGE_SENTINEL.to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: true, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 3266d9c9ac..de3bf3c953 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -528,7 +528,6 @@ impl PodmanComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 22077a6508..fbd4230dec 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -986,7 +986,6 @@ impl VmDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: Some(ResourceCapabilities { cpu: Some(CpuResourceCapabilities { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 27fd69d03e..ca4a51e126 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -662,13 +662,6 @@ impl ComputeRuntime { capabilities.extension.clone(), ) .map_err(|error| ComputeError::Message(error.to_string()))?; - if capabilities.supports_sandbox_authentication - && !capabilities.supports_runtime_identity_binding - { - return Err(ComputeError::Precondition(format!( - "compute driver '{driver_name}' authenticates sandboxes but does not support runtime identity binding" - ))); - } info!( configured_driver = %driver_name, advertised_driver = %capabilities.driver_name, @@ -5894,7 +5887,6 @@ impl ComputeDriver for NoopTestDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: self.sandbox_authentication.is_some(), - supports_runtime_identity_binding: self.sandbox_authentication.is_some(), driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -6486,7 +6478,6 @@ mod tests { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -6673,8 +6664,6 @@ mod tests { start_expected_runtime_identities: TestMutex>, start_outcome: TestMutex, runtime_identity: TestMutex, - advertises_sandbox_authentication: AtomicBool, - advertises_runtime_identity_binding: AtomicBool, get_started: Notify, get_release: Semaphore, get_blocked: AtomicBool, @@ -6714,8 +6703,6 @@ mod tests { start_expected_runtime_identities: TestMutex::new(Vec::new()), start_outcome: TestMutex::new(ControlledLifecycleOutcome::Ok), runtime_identity: TestMutex::new(String::new()), - advertises_sandbox_authentication: AtomicBool::new(false), - advertises_runtime_identity_binding: AtomicBool::new(false), get_started: Notify::new(), get_release: Semaphore::new(0), get_blocked: AtomicBool::new(false), @@ -6791,13 +6778,6 @@ mod tests { .expect("runtime identity lock poisoned") = runtime_identity.into(); } - fn set_binding_capabilities(&self, authentication: bool, binding: bool) { - self.advertises_sandbox_authentication - .store(authentication, Ordering::SeqCst); - self.advertises_runtime_identity_binding - .store(binding, Ordering::SeqCst); - } - fn set_get_outcome(&self, outcome: ControlledGetOutcome) { *self.get_outcome.lock().expect("get outcome lock poisoned") = outcome; } @@ -6881,12 +6861,7 @@ mod tests { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, - supports_sandbox_authentication: self - .advertises_sandbox_authentication - .load(Ordering::SeqCst), - supports_runtime_identity_binding: self - .advertises_runtime_identity_binding - .load(Ordering::SeqCst), + supports_sandbox_authentication: false, driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), @@ -7201,28 +7176,6 @@ mod tests { (directory, runtime) } - #[tokio::test] - async fn incompatible_authenticating_driver_is_rejected_during_initialization() { - let driver = ControlledDriver::new(); - driver.set_binding_capabilities(true, false); - let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); - - let error = ComputeRuntime::from_driver( - "legacy-driver".to_string(), - driver, - None, - store, - SandboxIndex::new(), - SandboxWatchBus::new(), - TracingLogBus::new(), - Arc::new(SupervisorSessionRegistry::new()), - ) - .await - .expect_err("driver without the binding contract must be rejected"); - - assert!(error.to_string().contains("runtime identity binding")); - } - #[tokio::test] async fn empty_create_runtime_identity_deletes_backend_and_record() { let driver = ControlledDriver::new(); diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index cc6b69c4bb..a6b8d99e0b 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -150,7 +150,6 @@ impl FakeComputeDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, supports_sandbox_authentication: false, - supports_runtime_identity_binding: false, driver_reports_runtime_readiness: false, resource_capabilities: None, rootfs_tar_staging_dir: String::new(), diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 9ea1fe263e..d7875d67e6 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -83,11 +83,10 @@ message GetCapabilitiesResponse { // Whether the gateway should stop running sandbox compute during graceful // shutdown and restart the retained running intent on startup. bool gateway_manages_lifecycle = 6; - // Whether AuthenticateSandbox is implemented by this driver. + // Whether AuthenticateSandbox is implemented by this driver. Drivers that + // enable this capability must return a stable runtime identity from create, + // start, and authentication responses for gateway-side binding checks. bool supports_sandbox_authentication = 7; - // Whether successful create, start, and authentication responses include a - // stable runtime identity suitable for gateway-side binding checks. - bool supports_runtime_identity_binding = 13; // Whether the driver reports runtime readiness itself. When false, the // gateway waits for the standard OpenShell supervisor session in addition // to the driver's platform-ready observation.