diff --git a/.agents/skills/helm-dev-environment/SKILL.md b/.agents/skills/helm-dev-environment/SKILL.md index fddbe8f1e4..b17831beb5 100644 --- a/.agents/skills/helm-dev-environment/SKILL.md +++ b/.agents/skills/helm-dev-environment/SKILL.md @@ -359,7 +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. +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 4fd27e2b9c..6e4fabb353 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 @@ -506,19 +508,29 @@ 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. +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 +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 +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..0c221fb3a7 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -317,11 +317,18 @@ 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 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. 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-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..07f8a0ffe1 100644 --- a/crates/openshell-driver-kubernetes/README.md +++ b/crates/openshell-driver-kubernetes/README.md @@ -140,8 +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 +exchange. The driver verifies that token and returns an opaque runtime identity +derived from the namespace, immutable Sandbox resource UID, and supervisor Pod +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. diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index cec9b20c5d..bd71dd3545 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,12 +2825,14 @@ impl KubernetesComputeDriver { sandbox_id: &str, generation_id: &str, launch_authentication: &[u8], - ) -> Result<(), KubernetesDriverError> { + 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) @@ -2821,24 +2844,27 @@ impl KubernetesComputeDriver { sandbox_id: &str, encoded_generation: &str, encoded_authentication: &[u8], - ) -> Result<(), KubernetesDriverError> { + 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) @@ -2864,7 +2890,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, + )); } } @@ -2873,13 +2910,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 @@ -2913,6 +2950,7 @@ impl KubernetesComputeDriver { sandbox_id, encoded_generation, encoded_authentication, + encoded_expected_runtime_identity, )) .await; } @@ -3057,7 +3095,11 @@ impl KubernetesComputeDriver { } return Err(error); } - Ok(()) + Ok(kubernetes_runtime_identity( + &namespace, + cr_uid, + &supervisor_uid, + )) } async fn prepare_sandbox_stop( @@ -4589,6 +4631,76 @@ 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}") +} + +#[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) @@ -7045,6 +7157,7 @@ mod tests { "sandbox-1", "invalid-generation", b"secret-launch-authentication", + "kubernetes://namespace/resource/pod", ) .with_subscriber(subscriber) .await @@ -7380,6 +7493,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(), @@ -7387,6 +7501,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 3427b81f3b..12cd0ebfff 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,16 +199,17 @@ 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, + &request.expected_runtime_identity, )) .await .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..ca4a51e126 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}; @@ -227,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<()>, } @@ -716,6 +720,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. @@ -782,7 +795,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 +814,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] @@ -888,12 +904,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 } @@ -903,6 +919,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; @@ -958,6 +1001,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() @@ -983,17 +1030,64 @@ impl ComputeRuntime { ) .await { - Ok(_) => { + Ok(response) => { + let runtime_identity = response.into_inner().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(); } - self.sandbox_watch_bus.notify(sandbox.object_id()); - if let Some(metadata) = sandbox.metadata.as_mut() { - metadata.resource_version = result.resource_version; + 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(); + let persisted = 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; + sandbox = match persisted { + Ok(sandbox) => sandbox, + Err(error) => { + let status = Status::internal(format!( + "persist compute runtime identity failed: {error}" + )); + 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()); Ok(sandbox) } Err(status) if status.code() == Code::AlreadyExists => { @@ -1026,6 +1120,122 @@ 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) => { + 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: {}{}", + original.message(), + error.message(), + cleanup_detail + ), + ); + } + }; + self.sandbox_index.update_from_sandbox(&transition.deleting); + self.sandbox_watch_bus.notify(sandbox_id); + drop(global_guard); + + let delete_result = self + .delete_backend_after_failed_create(sandbox_id, sandbox_name) + .await; + match delete_result { + 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 + // 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() + ), + ) + } + } + } + + 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, @@ -1414,6 +1624,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( @@ -1431,6 +1642,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication, generation_id, + expected_runtime_identity, })) .await } @@ -1461,9 +1673,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 +1689,57 @@ impl ComputeRuntime { } match result { - Ok(_) => { - 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"))?; + Ok(response) => { + let runtime_identity = response.into_inner().runtime_identity; + 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_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(); + let persisted = self + .persist_start_runtime_binding( + &sandbox_id, + &starting, + &driver_name, + &runtime_identity, + &[ + SandboxPhase::Starting, + SandboxPhase::Provisioning, + SandboxPhase::Ready, + ], + ) + .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) + .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) => { @@ -1494,6 +1753,128 @@ impl ComputeRuntime { } } + async fn persist_start_runtime_binding( + &self, + sandbox_id: &str, + 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); + + 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 || !allowed_phases.contains(&phase) + { + 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, + 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. /// @@ -1549,7 +1930,7 @@ impl ComputeRuntime { ); } } else { - self.restore_lifecycle_snapshot(transition, previous).await; + let _ = self.restore_lifecycle_snapshot(transition, previous).await; } } Ok(Some(_) | None) | Err(_) => { @@ -1630,7 +2011,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 @@ -1645,9 +2026,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 } } } @@ -2546,6 +2929,7 @@ impl ComputeRuntime { continue; } }; + let expected_runtime_identity = sandbox_compute_runtime_identity(&sandbox); match self .await_provisioning_operation( &sandbox, @@ -2556,6 +2940,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 { @@ -2563,6 +2948,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication, generation_id, + expected_runtime_identity, })) .await } @@ -2571,7 +2957,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(), @@ -2580,7 +3008,7 @@ impl ComputeRuntime { ); } let did_recover = if recoverable_error { - self.clear_recoverable_error(&sandbox).await + self.clear_recoverable_error(&recovered_sandbox).await } else { false }; @@ -2742,6 +3170,7 @@ impl ComputeRuntime { continue; } }; + let expected_runtime_identity = sandbox_compute_runtime_identity(&sandbox); if let Err(err) = self .driver .call( @@ -2754,6 +3183,7 @@ impl ComputeRuntime { name: sandbox_name, launch_authentication: Vec::new(), generation_id, + expected_runtime_identity, })) .await }, @@ -4711,6 +5141,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 { @@ -5355,7 +5798,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 +5815,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 +5860,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 +5939,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 +5967,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 +6548,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 +6562,7 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(StartSandboxResponse {})) + Ok(tonic::Response::new(StartSandboxResponse::default())) } async fn delete_sandbox( @@ -6173,6 +6644,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, @@ -6187,7 +6661,9 @@ mod tests { start_calls: AtomicUsize, start_requests: TestMutex>, start_authentications: TestMutex>>, + start_expected_runtime_identities: TestMutex>, start_outcome: TestMutex, + runtime_identity: TestMutex, get_started: Notify, get_release: Semaphore, get_blocked: AtomicBool, @@ -6207,6 +6683,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), @@ -6221,7 +6700,9 @@ 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()), get_started: Notify::new(), get_release: Semaphore::new(0), get_blocked: AtomicBool::new(false), @@ -6233,6 +6714,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); } @@ -6282,6 +6771,13 @@ 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_get_outcome(&self, outcome: ControlledGetOutcome) { *self.get_outcome.lock().expect("get outcome lock poisoned") = outcome; } @@ -6326,6 +6822,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)) @@ -6425,7 +6928,21 @@ mod tests { &self, _request: Request, ) -> Result, Status> { - Ok(tonic::Response::new(CreateSandboxResponse {})) + 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( @@ -6472,6 +6989,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) { @@ -6488,7 +7009,13 @@ mod tests { .expect("start outcome lock poisoned") .clone(); match outcome { - ControlledLifecycleOutcome::Ok => Ok(tonic::Response::new(StartSandboxResponse {})), + 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)), } @@ -6603,18 +7130,479 @@ mod tests { runtime } - fn register_test_supervisor_session(runtime: &ComputeRuntime, sandbox_id: &str) { - let (tx, _rx) = mpsc::channel(1); - let (shutdown_tx, _shutdown_rx) = oneshot::channel(); - runtime.supervisor_sessions.register( - sandbox_id.to_string(), - "session-1".to_string(), - tx, - shutdown_tx, - ); + fn enable_runtime_identity_binding(runtime: &mut ComputeRuntime) { + runtime.driver_info.supports_sandbox_authentication = true; } - fn sandbox_record(id: &str, name: &str, phase: SandboxPhase) -> Sandbox { + 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 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_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(); + 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" + ); + } + + #[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); + assert_eq!( + driver.start_expected_runtime_identities(), + vec!["previous-runtime-identity".to_string()] + ); + } + + #[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(); + runtime.supervisor_sessions.register( + sandbox_id.to_string(), + "session-1".to_string(), + tx, + shutdown_tx, + ); + } + + fn sandbox_record(id: &str, name: &str, phase: SandboxPhase) -> Sandbox { let mut annotations = HashMap::new(); crate::auth::sandbox_session::PersistedSandboxIdentity { runtime_generation: openshell_core::sandbox_generation::SandboxGenerationId::parse( @@ -11794,6 +12782,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 8eca7276b9..ec494e9434 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,27 +87,61 @@ 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(|| { + 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 + .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, - "IssueSandboxToken called but sandbox JWT issuer is not configured" + driver_name, + "IssueSandboxToken rejected: compute runtime identity mismatch" ); - Status::unavailable("sandbox JWT minting is not configured on this gateway") - })?; - - let _ = ensure_sandbox_exists(state, &sandbox.sandbox_id).await?; + return Err(Status::permission_denied( + "compute runtime identity does not match the sandbox", + )); + } - 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()))?, })) @@ -484,6 +522,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(); } @@ -766,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 {}); @@ -776,6 +825,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()), })); @@ -785,6 +835,65 @@ 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] + 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] @@ -798,6 +907,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 +954,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/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/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/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..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. 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. 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 36fa4eeb76..d7875d67e6 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -83,7 +83,9 @@ 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 the driver reports runtime readiness itself. When false, the // gateway waits for the standard OpenShell supervisor session in addition @@ -110,6 +112,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 +389,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. @@ -405,9 +415,18 @@ 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 {} +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/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 f81416c5ab..67fb2288af 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -649,6 +649,15 @@ 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. 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