From 036acbc112d3fcbce9de2e4b18246ffd1988a2c7 Mon Sep 17 00:00:00 2001 From: Shiju Date: Thu, 1 Oct 2026 14:48:37 +0530 Subject: [PATCH 1/5] fix(vm): enforce the configured workload identity Reject conflicting policy users and groups before VM image preparation and before guest attach or process startup changes state. Validate supervisor policy updates against the protected VM workload identity. Preserve the gateway CA transport and capability-free sandbox launcher. Signed-off-by: Shiju --- crates/openshell-driver-vm/src/driver.rs | 170 ++++++++++++- .../openshell-sandbox/src/boundary_server.rs | 224 ++++++++++++++++++ crates/openshell-server/src/compute/mod.rs | 59 ++++- .../openshell-server/src/grpc/validation.rs | 25 ++ crates/openshell-supervisor/src/lib.rs | 222 +++++++++++++++++ docs/how-it-works/policies/schema.mdx | 8 +- docs/how-it-works/sandboxes/runtimes.mdx | 4 +- e2e/rust/tests/vm_overlay.rs | 157 +++++++++++- 8 files changed, 851 insertions(+), 18 deletions(-) diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 30eec2cd5e..a03982038d 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -52,11 +52,12 @@ use openshell_core::proto::compute::v1::{ DriverSandboxTemplate as SandboxTemplate, EnsureWorkspaceRequest, EnsureWorkspaceResponse, GetCapabilitiesRequest, GetCapabilitiesResponse, GetSandboxRequest, GetSandboxResponse, GpuResourceCapabilities, ListSandboxesRequest, ListSandboxesResponse, - MemoryResourceCapabilities, ResourceCapabilities, StartSandboxRequest, StartSandboxResponse, - StopSandboxRequest, StopSandboxResponse, ValidateSandboxCreateRequest, - ValidateSandboxCreateResponse, WatchSandboxesDeletedEvent, WatchSandboxesEvent, - WatchSandboxesPlatformEvent, WatchSandboxesRequest, WatchSandboxesSandboxEvent, - compute_driver_server::ComputeDriver, watch_sandboxes_event, + MemoryResourceCapabilities, ResolvedWorkloadIdentity, ResourceCapabilities, + StartSandboxRequest, StartSandboxResponse, StopSandboxRequest, StopSandboxResponse, + ValidateSandboxCreateRequest, ValidateSandboxCreateResponse, WatchSandboxesDeletedEvent, + WatchSandboxesEvent, WatchSandboxesPlatformEvent, WatchSandboxesRequest, + WatchSandboxesSandboxEvent, WorkloadIdentityRequest, compute_driver_server::ComputeDriver, + watch_sandboxes_event, }; use openshell_core::proto_struct::{ deserialize_optional_non_empty_string_list, struct_to_json_value, @@ -1391,6 +1392,10 @@ impl VmDriver { &overlay_disk, &owner_source_disk, overlay_preparation, + sandbox + .spec + .as_ref() + .and_then(|spec| spec.workload_identity.as_ref()), ) .await .map_err(|err| Status::internal(format!("prepare guest overlay disk failed: {err}")))?; @@ -1723,6 +1728,18 @@ impl VmDriver { Some(record) if !record.deleting => { record.process = Some(process.clone()); record.gpu_bdf.clone_from(&gpu_bdf); + let identity = &runtime_descriptor.workload_identity; + record + .snapshot + .status + .get_or_insert_with(SandboxStatus::default) + .resolved_identity = Some(ResolvedWorkloadIdentity { + uid: identity.uid, + gid: identity.gid, + supplementary_gids: identity.supplementary_gids.clone(), + source: identity.source.clone(), + resource_digest: identity.resource_digest.clone(), + }); snapshot_to_publish = Some(record.snapshot.clone()); } _ => { @@ -2765,6 +2782,7 @@ impl VmDriver { overlay_disk: &Path, owner_source_disk: &Path, preparation: OverlayPreparation, + requested_identity: Option<&WorkloadIdentityRequest>, ) -> Result { let span_status = openshell_otel::ErrorStatusGuard::current(); let overlay_disk = overlay_disk.to_path_buf(); @@ -2786,6 +2804,9 @@ impl VmDriver { preparation, ) .await?; + // Validate before creating or recovering an overlay. A conflicting + // request must never change the persisted owner or its files. + validate_vm_workload_identity(requested_identity, owner_state)?; let owner_state_written_before_prepare = write_owner_state && preparation == OverlayPreparation::Fresh; if owner_state_written_before_prepare { @@ -6788,8 +6809,33 @@ fn status_with_condition( sandbox_fd: String::new(), conditions: vec![condition], deleting, - ..Default::default() + ..snapshot.status.clone().unwrap_or_default() + } +} + +/// VM init reconciles the named `sandbox` account to this overlay's owner. +/// Numeric selectors assert that same immutable identity; they cannot select +/// a new owner. Each empty selector independently accepts the driver default. +fn validate_vm_workload_identity( + request: Option<&WorkloadIdentityRequest>, + owner: SandboxOwnerIdentity, +) -> Result<(), String> { + let Some(request) = request else { + return Ok(()); + }; + for (field, selector, expected) in [ + ("run_as_user", request.user.as_str(), owner.uid), + ("run_as_group", request.group.as_str(), owner.gid), + ] { + if selector.is_empty() || selector == "sandbox" || selector.parse::() == Ok(expected) { + continue; + } + return Err(format!( + "VM {field} '{selector}' conflicts with the resolved workload identity {}:{}; omit the selector or request the driver-owned identity", + owner.uid, owner.gid + )); } + Ok(()) } fn provisioning_condition() -> SandboxCondition { @@ -7591,6 +7637,7 @@ mod tests { Path::new("/unused"), Path::new("/unused"), OverlayPreparation::Fresh, + None, ) .instrument(parent) .await; @@ -8177,6 +8224,117 @@ mod tests { } } + #[tokio::test] + async fn conflicting_vm_identity_preserves_overlay_and_owner() { + let directory = tempfile::tempdir().unwrap(); + let owner = SandboxOwnerIdentity { + uid: 1000, + gid: 1001, + }; + write_sandbox_owner_state(directory.path(), owner) + .await + .unwrap(); + let overlay = directory.path().join(SANDBOX_OVERLAY_IMAGE); + std::fs::write(&overlay, b"existing overlay must not be touched").unwrap(); + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.sandbox_uid = Some(10000); + driver.config.sandbox_gid = Some(10001); + let request = WorkloadIdentityRequest { + user: "10000".into(), + group: "10001".into(), + }; + let error = driver + .prepare_runtime_overlay( + directory.path(), + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + Some(&request), + ) + .await + .unwrap_err(); + assert!(error.contains("run_as_user '10000'"), "{error}"); + assert!(error.contains("1000:1001"), "{error}"); + assert_eq!( + std::fs::read(&overlay).unwrap(), + b"existing overlay must not be touched" + ); + assert_eq!( + std::fs::read_to_string(directory.path().join(SANDBOX_OWNER_STATE_FILE)).unwrap(), + owner.marker_contents() + ); + } + + #[test] + fn vm_workload_identity_checks_independent_numeric_and_symbolic_selectors() { + let owner = SandboxOwnerIdentity { + uid: 1000, + gid: 1001, + }; + validate_vm_workload_identity(None, owner).unwrap(); + for (user, group) in [ + ("", ""), + ("1000", ""), + ("", "1001"), + ("sandbox", "1001"), + ("1000", "sandbox"), + ("sandbox", "sandbox"), + ] { + validate_vm_workload_identity( + Some(&WorkloadIdentityRequest { + user: user.into(), + group: group.into(), + }), + owner, + ) + .unwrap(); + } + for (user, group, field) in [ + ("10000", "", "run_as_user"), + ("", "10001", "run_as_group"), + ("sandbox", "10001", "run_as_group"), + ("nobody", "", "run_as_user"), + ] { + let error = validate_vm_workload_identity( + Some(&WorkloadIdentityRequest { + user: user.into(), + group: group.into(), + }), + owner, + ) + .unwrap_err(); + assert!(error.contains(field), "{error}"); + assert!(error.contains("1000:1001"), "{error}"); + } + } + + #[test] + fn vm_lifecycle_status_retains_resolved_workload_identity() { + let identity = ResolvedWorkloadIdentity { + uid: 1000, + gid: 1001, + source: "vm-config".into(), + resource_digest: "sha256:image".into(), + ..Default::default() + }; + let snapshot = Sandbox { + status: Some(SandboxStatus { + resolved_identity: Some(identity.clone()), + ..Default::default() + }), + ..Default::default() + }; + for condition in [ + provisioning_condition(), + stopped_condition(), + deleting_condition(), + error_condition("ProcessExited", "exited"), + ] { + let status = status_with_condition(&snapshot, condition, false); + assert_eq!(status.resolved_identity.as_ref(), Some(&identity)); + } + } + #[tokio::test] async fn unmarked_overlay_uses_current_image_instead_of_blind_legacy_identity() { let dir = unique_temp_dir(); diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index f84f98d424..a592387a01 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -357,6 +357,34 @@ mod linux { Ok(()) } + /// VM selectors assert the protected overlay owner; they cannot choose a + /// replacement identity. Guest init maps `sandbox` to that owner before + /// launching this boundary, so no account lookup or privilege change belongs here. + fn validate_vm_policy_identity( + config: &BoundaryConfig, + policy: &SandboxPolicyWire, + ) -> Result<(), String> { + if !config.resource_claims.contains_key("vm.generation") { + return Ok(()); + } + let identity = &config.workload_identity; + for (field, selector, expected) in [ + ("run_as_user", policy.run_as_user.as_deref(), identity.uid), + ("run_as_group", policy.run_as_group.as_deref(), identity.gid), + ] { + let Some(selector) = selector.filter(|value| !value.is_empty()) else { + continue; + }; + if selector != "sandbox" && selector.parse::() != Ok(expected) { + return Err(format!( + "VM {field} '{selector}' conflicts with the resolved workload identity {}:{}; omit the selector or request the driver-owned identity", + identity.uid, identity.gid + )); + } + } + Ok(()) + } + fn tls_paths_are_absolute( tls: &openshell_sandbox_backend::boundary_protocol::SandboxTlsServerConfig, ) -> bool { @@ -2247,6 +2275,11 @@ mod linux { } fn attach(&self, policy: SandboxPolicyWire) -> Response { + // Reject a conflicting request before establishing the boundary + // or retaining the caller's policy for later replay. + if let Err(error) = validate_vm_policy_identity(&self.config, &policy) { + return guest_error(BoundaryErrorKind::Denied, error); + } let mut state = lock(&self.state); let accepted = match &*state { RuntimeState::AwaitingAttach => { @@ -2439,6 +2472,11 @@ mod linux { provider_env: std::collections::HashMap, provider_files: std::collections::HashMap, ) -> Response { + // Check the supplied launch policy before installing materials or + // replacing its selectors with the measured driver's numeric pair. + if let Err(error) = validate_vm_policy_identity(&self.config, &policy) { + return guest_error(BoundaryErrorKind::Denied, error); + } let spec = match resolve_agent_spec(spec) { Ok(spec) => spec, Err(error) => return guest_error(BoundaryErrorKind::Process, error), @@ -4643,6 +4681,192 @@ mod linux { validate_config(&config).unwrap(); validate_running_identity(&config.workload_identity, false).unwrap(); + let mut wrong_uid = config.workload_identity.clone(); + wrong_uid.uid = if wrong_uid.uid == 10000 { 10001 } else { 10000 }; + assert!(validate_running_identity(&wrong_uid, false).is_err()); + let mut wrong_gid = config.workload_identity.clone(); + wrong_gid.gid = if wrong_gid.gid == 10000 { 10001 } else { 10000 }; + assert!(validate_running_identity(&wrong_gid, false).is_err()); + } + + fn vm_identity_test_runtime() -> (tokio::runtime::Runtime, Arc) { + let process_runtime = tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .enable_all() + .build() + .expect("test process runtime"); + let mut boundary = { + let _entered = process_runtime.enter(); + availability_test_runtime().0 + }; + let config = &mut Arc::get_mut(&mut boundary) + .expect("test boundary has one owner") + .config; + config + .resource_claims + .insert("vm.generation".to_string(), config.generation.clone()); + (process_runtime, boundary) + } + + fn vm_identity_test_policy(user: Option<&str>, group: Option<&str>) -> SandboxPolicyWire { + SandboxPolicyWire::from(openshell_core::policy::SandboxPolicy { + version: 1, + filesystem: openshell_core::policy::FilesystemPolicy::default(), + network: openshell_core::policy::NetworkPolicy::default(), + landlock: openshell_core::policy::LandlockPolicy::default(), + process: openshell_core::policy::ProcessPolicy { + run_as_user: user.map(str::to_string), + run_as_group: group.map(str::to_string), + }, + }) + } + + #[test] + fn vm_policy_identity_checks_independent_selectors() { + let (_runtime, mut boundary) = vm_identity_test_runtime(); + let uid = boundary.config.workload_identity.uid.to_string(); + let gid = boundary.config.workload_identity.gid.to_string(); + for (user, group) in [ + (None, None), + (Some(""), Some("")), + (Some(uid.as_str()), None), + (None, Some(gid.as_str())), + (Some(uid.as_str()), Some(gid.as_str())), + (Some("sandbox"), Some(gid.as_str())), + (Some(uid.as_str()), Some("sandbox")), + (Some("sandbox"), Some("sandbox")), + ] { + validate_vm_policy_identity( + &boundary.config, + &vm_identity_test_policy(user, group), + ) + .expect("matching or omitted VM selectors"); + } + let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; + let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + for (user, group, field) in [ + (Some(wrong_uid), None, "run_as_user"), + (None, Some(wrong_gid), "run_as_group"), + (Some(uid.as_str()), Some(wrong_gid), "run_as_group"), + (Some(wrong_uid), Some(gid.as_str()), "run_as_user"), + (Some(wrong_uid), Some(wrong_gid), "run_as_user"), + ] { + let error = validate_vm_policy_identity( + &boundary.config, + &vm_identity_test_policy(user, group), + ) + .expect_err("either mismatched selector must fail"); + assert!(error.contains(field), "{error}"); + assert!(error.contains(&format!("{uid}:{gid}")), "{error}"); + } + for malformed in [ + "root", + "-1", + "4294967296", + "1000:1000", + " sandbox", + "sandbox\n", + ] { + for (user, group) in [(Some(malformed), None), (None, Some(malformed))] { + assert!( + validate_vm_policy_identity( + &boundary.config, + &vm_identity_test_policy(user, group), + ) + .is_err(), + "invalid selector {malformed:?} must fail" + ); + } + } + Arc::get_mut(&mut boundary) + .expect("test boundary has one owner") + .config + .resource_claims + .remove("vm.generation"); + validate_vm_policy_identity( + &boundary.config, + &vm_identity_test_policy(Some(wrong_uid), Some("image-user")), + ) + .expect("non-VM identity behavior is unchanged"); + } + + #[test] + fn vm_attach_rejects_conflicting_identity_without_binding() { + let (_runtime, boundary) = vm_identity_test_runtime(); + let identity = &boundary.config.workload_identity; + let uid = identity.uid.to_string(); + let gid = identity.gid.to_string(); + let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; + let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + for (user, group, field, requested) in [ + (Some(wrong_uid), None, "run_as_user", wrong_uid), + (None, Some(wrong_gid), "run_as_group", wrong_gid), + ] { + let response = boundary.attach(vm_identity_test_policy(user, group)); + let Response::Error { kind, message } = response else { + panic!("conflicting identity was attached: {response:?}"); + }; + assert_eq!(kind, BoundaryErrorKind::Denied); + assert!( + message.contains(&format!("{field} '{requested}'")), + "{message}" + ); + assert!(message.contains(&format!("{uid}:{gid}")), "{message}"); + assert!(matches!( + *lock(&boundary.state), + RuntimeState::AwaitingAttach + )); + assert!(lock(&boundary.attached_policy).is_none()); + } + assert!(matches!( + boundary.attach(vm_identity_test_policy(Some("sandbox"), Some("sandbox"))), + Response::Attached { .. } + )); + } + + #[test] + fn vm_start_rejects_conflicting_identity_without_launch() { + let (_runtime, boundary) = vm_identity_test_runtime(); + let identity = &boundary.config.workload_identity; + let uid = identity.uid.to_string(); + let gid = identity.gid.to_string(); + let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; + let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + *lock(&boundary.state) = RuntimeState::Ready(PreparedBoundary { + network_broker: boundary.network_broker.clone(), + }); + for (user, group, field, requested) in [ + (Some(wrong_uid), None, "run_as_user", wrong_uid), + (None, Some(wrong_gid), "run_as_group", wrong_gid), + ] { + let response = boundary.start_agent( + boundary.config.boundary_id.clone(), + AgentSpecWire { + program: "/bin/true".to_string(), + args: Vec::new(), + workdir: None, + timeout_secs: 5, + interactive: false, + }, + vm_identity_test_policy(user, group), + None, + None, + 0, + std::collections::HashMap::new(), + std::collections::HashMap::new(), + ); + let Response::Error { kind, message } = response else { + panic!("conflicting identity reached process start: {response:?}"); + }; + assert_eq!(kind, BoundaryErrorKind::Denied); + assert!( + message.contains(&format!("{field} '{requested}'")), + "{message}" + ); + assert!(message.contains(&format!("{uid}:{gid}")), "{message}"); + assert!(matches!(*lock(&boundary.state), RuntimeState::Ready(_))); + assert!(lock(&boundary.started_agent).is_none()); + } } #[test] diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 264cd77bbb..088231f6e1 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -5992,15 +5992,31 @@ fn public_status_from_driver( phase: SandboxPhase, current_policy_version: u32, ) -> SandboxStatus { + let mut conditions = status + .conditions + .iter() + .map(public_condition_from_driver) + .collect::>(); + if let Some(identity) = &status.resolved_identity { + // Keep the requested policy intact. This condition reports the + // immutable runtime identity through existing CLI/API status views. + conditions.retain(|condition| condition.r#type != "WorkloadIdentity"); + conditions.push(SandboxCondition { + r#type: "WorkloadIdentity".to_string(), + status: "True".to_string(), + reason: "DriverResolved".to_string(), + message: format!( + "Resolved workload UID:GID is {}:{}", + identity.uid, identity.gid + ), + transition_time: None, + }); + } SandboxStatus { agent_pod: status.instance_id.clone(), agent_fd: status.agent_fd.clone(), sandbox_fd: status.sandbox_fd.clone(), - conditions: status - .conditions - .iter() - .map(public_condition_from_driver) - .collect(), + conditions, phase: phase as i32, current_policy_version, main_process_instance_id: String::new(), @@ -9764,6 +9780,39 @@ mod tests { } } + #[test] + fn public_status_reports_driver_identity_without_rewriting_policy_or_readiness() { + let mut driver_status = + make_driver_status(make_driver_condition("Starting", "VM is starting")); + let unresolved = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0); + assert!( + !unresolved + .conditions + .iter() + .any(|condition| condition.r#type == "WorkloadIdentity") + ); + driver_status.resolved_identity = Some( + openshell_core::proto::compute::v1::ResolvedWorkloadIdentity { + uid: 1000, + gid: 1001, + source: "vm-config".into(), + resource_digest: "sha256:image".into(), + ..Default::default() + }, + ); + let status = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0); + assert_eq!(status.phase, SandboxPhase::Provisioning as i32); + assert_eq!(status.current_policy_version, 0); + assert_eq!(status.conditions[0], unresolved.conditions[0]); + let identity = status + .conditions + .iter() + .find(|condition| condition.r#type == "WorkloadIdentity") + .unwrap(); + assert_eq!(identity.reason, "DriverResolved"); + assert_eq!(identity.message, "Resolved workload UID:GID is 1000:1001"); + } + fn ready_driver_sandbox(id: &str, name: &str) -> DriverSandbox { DriverSandbox { id: id.to_string(), diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index f72c571ba5..abdfccda10 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -2403,6 +2403,31 @@ mod tests { // ---- Exec validation ---- + #[test] + fn validate_static_fields_rejects_independent_process_identity_changes() { + let baseline = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "sandbox".into(), + run_as_group: "1001".into(), + }), + ..Default::default() + }; + for (user, group) in [ + ("10000", "1001"), + ("sandbox", "10001"), + ("", "1001"), + ("sandbox", ""), + ] { + let mut changed = baseline.clone(); + changed.process = Some(openshell_core::proto::ProcessPolicy { + run_as_user: user.into(), + run_as_group: group.into(), + }); + let error = validate_static_fields_unchanged(&baseline, &changed).unwrap_err(); + assert!(error.message().contains("process policy cannot be changed")); + } + } + #[test] fn reject_control_chars_allows_normal_values() { assert!(reject_control_chars("hello world", "test").is_ok()); diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 99607dbedb..fa5613d416 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -701,6 +701,14 @@ pub async fn run_sandbox( ImagePolicyDiscovery::Missing }; + let vm_policy_identity = runtime_descriptor + .resource_claims + .contains_key("vm.generation") + .then_some(VmPolicyIdentity { + uid: runtime_descriptor.workload_identity.uid, + gid: runtime_descriptor.workload_identity.gid, + }); + // Load policy and initialize OPA engine let openshell_endpoint_for_proxy = openshell_endpoint.clone(); let sandbox_name_for_agg = sandbox.clone(); @@ -721,6 +729,7 @@ pub async fn run_sandbox( policy_data, &extension_credentials, LocalPolicyIdentity::Required, + vm_policy_identity, Some(image_discovery), &RemoteStartupGateway { endpoint: openshell_endpoint.clone().unwrap_or_default(), @@ -1092,6 +1101,7 @@ pub async fn run_sandbox( sandbox: poll_sandbox, opa_engine: poll_engine, loaded_policy_origin, + vm_identity: vm_policy_identity, entrypoint_pid: poll_pid, interval_secs: poll_interval_secs, ocsf_enabled: poll_ocsf_enabled, @@ -2044,6 +2054,39 @@ enum LocalPolicyIdentity { EndpointOnly, } +/// The VM driver fixes overlay ownership before this supervisor starts. Guest +/// init maps the `sandbox` account to this pair; the host must not resolve +/// guest selectors through its own account database. +#[derive(Clone, Copy)] +struct VmPolicyIdentity { + uid: u32, + gid: u32, +} + +impl VmPolicyIdentity { + fn validate(self, policy: &openshell_core::proto::SandboxPolicy) -> Result<()> { + let Some(process) = policy.process.as_ref() else { + return Ok(()); + }; + for (field, selector, expected) in [ + ("run_as_user", process.run_as_user.as_str(), self.uid), + ("run_as_group", process.run_as_group.as_str(), self.gid), + ] { + if !selector.is_empty() + && selector != "sandbox" + && selector.parse::() != Ok(expected) + { + return Err(miette::miette!( + "VM {field} '{selector}' conflicts with the resolved workload identity {}:{}; omit the selector or request the driver-owned identity", + self.uid, + self.gid + )); + } + } + Ok(()) + } +} + struct CapturedProviderEnvironment { credentials: ProviderCredentialState, expires_at_ms: Option, @@ -2100,6 +2143,7 @@ async fn load_policy( extension_credentials, local_policy_identity, None, + None, &RemoteStartupGateway { endpoint: openshell_endpoint.unwrap_or_default(), }, @@ -2119,6 +2163,7 @@ async fn load_policy_with_gateway( policy_data: Option, extension_credentials: &openshell_extension_core::ExtensionCredentialStore, local_policy_identity: LocalPolicyIdentity, + vm_identity: Option, image_discovery: Option, gateway: &impl StartupGateway, ) -> Result<( @@ -2408,6 +2453,23 @@ async fn load_policy_with_gateway( reconciliation_attempts = 0; continue; } + // Admission must reject incompatible image-discovered or repaired + // selectors before reporting this policy effective. + if let Some(identity) = vm_identity + && let Err(error) = identity.validate(&proto_policy) + { + reject_startup_configuration( + gateway, + &mut rejection_log, + id, + &instance_id, + &snapshot, + &error.to_string(), + ) + .await?; + reconciliation_attempts = 0; + continue; + } let provider = grpc_retry("Startup provider environment", || gateway.provider(id)).await?; if provider.provider_env_revision != snapshot.provider_env_revision { @@ -2862,6 +2924,7 @@ async fn reload_gateway_policy_runtime( entrypoint_pid, middleware, transparent_tcp, + None, || {}, ) .await @@ -2873,8 +2936,16 @@ async fn reload_gateway_configuration_runtime( entrypoint_pid: u32, middleware: MiddlewareReloadContext<'_>, transparent_tcp: TransparentTcpReloadState, + vm_identity: Option, commit_credentials: impl FnOnce(), ) -> std::result::Result { + if let (Some(identity), Some(policy)) = (vm_identity, policy) { + // Global policy changes also reach this path. Validate before any + // policy generation, middleware or provider credentials are committed. + identity + .validate(policy) + .map_err(GatewayRuntimeReloadError::PolicyValidation)?; + } if let Some(policy) = policy && policy_contains_explicit_tcp(policy) { @@ -3540,6 +3611,8 @@ struct PolicyPollLoopContext { /// explicit local-file override from an unbound gateway revision so the /// former is never replaced by policy polling. loaded_policy_origin: LoadedPolicyOrigin, + /// Immutable VM overlay identity, also enforced for global policy updates. + vm_identity: Option, entrypoint_pid: Arc, interval_secs: u64, ocsf_enabled: Arc, @@ -4448,6 +4521,7 @@ async fn run_policy_poll_loop_with_client( connector: &ctx.middleware_connector, }, ctx.transparent_tcp, + ctx.vm_identity, || { if let Some(prepared) = prepared_provider.as_ref() { ctx.provider_credentials.install_prepared(prepared); @@ -5500,6 +5574,150 @@ network_policies: assert!(log.changed(&snapshot, "invalid policy")); } + #[tokio::test(start_paused = true)] + async fn startup_rejects_vm_identity_until_matching_policy_is_available() { + use openshell_core::proto::{ConfigurationAdmissionState, PolicySource}; + let mut policy = proto_policy_fixture(); + enrich_proto_baseline_paths(&mut policy); + policy.process = Some(openshell_core::proto::ProcessPolicy { + run_as_user: "10000".into(), + run_as_group: "1001".into(), + }); + let (reports, mut reported) = tokio::sync::mpsc::unbounded_channel(); + let gateway = TestStartupGateway { + desired: Arc::new(std::sync::Mutex::new(settings_poll_result( + Some(policy), + 1, + PolicySource::Sandbox, + ))), + reports, + reject_next_accept: Arc::new(AtomicBool::new(false)), + snapshot_error: None, + report_error: None, + pending_snapshot: false, + pending_acceptance: false, + }; + let active_gateway = gateway.clone(); + let handle = tokio::spawn(async move { + load_policy_with_gateway( + Some("sandbox-id".into()), + Some("sandbox".into()), + Some("http://unused.invalid".into()), + None, + None, + &openshell_extension_core::ExtensionCredentialStore::new(), + LocalPolicyIdentity::Required, + Some(VmPolicyIdentity { + uid: 1000, + gid: 1001, + }), + Some(ImagePolicyDiscovery::Missing), + &active_gateway, + ) + .await + }); + assert_eq!( + reported.recv().await, + Some(ConfigurationAdmissionState::Pending) + ); + assert_eq!( + reported.recv().await, + Some(ConfigurationAdmissionState::Rejected) + ); + assert!( + !handle.is_finished(), + "mismatched policy must never be returned as effective" + ); + { + let mut desired = gateway.desired.lock().unwrap(); + desired + .policy + .as_mut() + .unwrap() + .process + .as_mut() + .unwrap() + .run_as_user = "sandbox".into(); + desired.config_revision += 1; + } + assert_eq!( + reported.recv().await, + Some(ConfigurationAdmissionState::Accepted) + ); + handle + .await + .unwrap() + .expect("matching repair should permit startup"); + } + + #[test] + fn vm_startup_identity_validates_selectors_independently() { + let identity = VmPolicyIdentity { + uid: 1000, + gid: 1001, + }; + for (user, group, accepted) in [ + ("", "", true), + ("1000", "", true), + ("", "1001", true), + ("sandbox", "sandbox", true), + ("10000", "", false), + ("", "10001", false), + ] { + let policy = openshell_core::proto::SandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: user.into(), + run_as_group: group.into(), + }), + ..Default::default() + }; + assert_eq!( + identity.validate(&policy).is_ok(), + accepted, + "{user}:{group}" + ); + } + } + + #[tokio::test] + async fn vm_identity_conflict_prevents_runtime_policy_and_credential_commit() { + let mut policy = proto_policy_fixture(); + let engine = OpaEngine::from_proto(&policy).unwrap(); + let before = engine.current_generation(); + policy.process = Some(openshell_core::proto::ProcessPolicy { + run_as_user: "sandbox".into(), + run_as_group: "10000".into(), + }); + let committed = AtomicBool::new(false); + let result = reload_gateway_configuration_runtime( + &engine, + Some(&policy), + 0, + MiddlewareReloadContext { + desired_services: &[], + authentication: &MiddlewareAuthentication::default(), + registry_changed: false, + connector: &default_middleware_connector(), + }, + TransparentTcpReloadState::default(), + Some(VmPolicyIdentity { + uid: 1000, + gid: 1001, + }), + || { + committed.store(true, Ordering::SeqCst); + }, + ) + .await; + let Err(GatewayRuntimeReloadError::PolicyValidation(error)) = result else { + panic!("conflicting VM group should fail policy validation"); + }; + assert!(error.to_string().contains("run_as_group '10000'")); + assert!(error.to_string().contains("1000:1001")); + assert_eq!(engine.current_generation(), before); + assert!(!committed.load(Ordering::SeqCst)); + } + #[tokio::test(start_paused = true)] async fn startup_pending_gateway_calls_exhaust_their_budgets() { for pending_snapshot in [true, false] { @@ -5529,6 +5747,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(ImagePolicyDiscovery::Missing), &gateway, ), @@ -5605,6 +5824,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(ImagePolicyDiscovery::Missing), &gateway, ), @@ -5650,6 +5870,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(ImagePolicyDiscovery::Missing), &active_gateway, ) @@ -7539,6 +7760,7 @@ network_policies: sandbox: "sandbox-test-name".to_string(), opa_engine, loaded_policy_origin, + vm_identity: None, entrypoint_pid: Arc::new(AtomicU32::new(0)), interval_secs: 0, ocsf_enabled: Arc::new(AtomicBool::new(false)), diff --git a/docs/how-it-works/policies/schema.mdx b/docs/how-it-works/policies/schema.mdx index a190cd5a91..db85e3a19d 100644 --- a/docs/how-it-works/policies/schema.mdx +++ b/docs/how-it-works/policies/schema.mdx @@ -89,11 +89,9 @@ and skipped. | `run_as_group` | string | Driver default | `sandbox` or a numeric GID for the workload. | A numeric ID must be from `1` through `4294967294`, so OpenShell rejects root. -Each field is independent, so you can set one and let the compute driver choose -the other. Only Docker and Podman apply these fields, and only from the policy -that you pass when you create the sandbox. Without them, Docker and Podman use -the image's `USER`. Kubernetes and VM sandboxes run as the identity configured -for their driver. +Each field is independent, so you can set one and let the compute driver choose the other. Docker and Podman use these fields to select the workload identity from the policy passed at creation; omitted fields use the image's `USER`. Kubernetes uses the identity configured for its driver. + +MicroVM preserves the driver's resolved overlay owner. An explicit numeric selector must match that owner; `sandbox` names the guest account reconciled to that owner. A conflicting selector fails before the workload starts, and omitted fields accept the driver default. `openshell sandbox get ` reports the resolved UID:GID in its `WorkloadIdentity` condition. This condition reports identity selection, not workload readiness. Sandbox policy updates cannot change process selectors; global policy updates must still match the resolved identity. Ordinary exec uses the same identity as the canonical process. ```yaml showLineNumbers={false} process: diff --git a/docs/how-it-works/sandboxes/runtimes.mdx b/docs/how-it-works/sandboxes/runtimes.mdx index c1090d1450..50e980e857 100644 --- a/docs/how-it-works/sandboxes/runtimes.mdx +++ b/docs/how-it-works/sandboxes/runtimes.mdx @@ -292,7 +292,7 @@ openshell sandbox create \ ## Sandbox User Identity -Set `process.run_as_user` and `process.run_as_group` in the sandbox policy to choose the sandbox user. Any non-root UID or GID is allowed. When a field is unset, the driver supplies it: +On Docker and Podman, set `process.run_as_user` and `process.run_as_group` in the creation policy to choose the sandbox user. Selectors accept `sandbox` or numeric IDs from `1` through `4294967294`. Kubernetes and MicroVM select the identity through driver configuration. When a field is unset, the driver supplies it: | Driver | Default identity | |---|---| @@ -300,4 +300,6 @@ Set `process.run_as_user` and `process.run_as_group` in the sandbox policy to ch | Kubernetes | OpenShift SCC namespace annotations, otherwise `1000`. Override with `sandbox_uid` and `sandbox_gid`. | | MicroVM | The image's `sandbox` account, otherwise `1000`. Override with `sandbox_uid` and `sandbox_gid`. | +MicroVM persists the resolved UID:GID with the writable overlay and retains it across restarts, even if driver defaults change. Explicit policy selectors must match that identity. For example, `run_as_user: "10000"` fails when the overlay owner is `1000:1000`; OpenShell reports both values without changing file ownership. The symbolic selector `sandbox` refers to the guest account for that owner, and each omitted selector accepts its driver default. The `WorkloadIdentity` condition in `openshell sandbox get ` shows the resolved pair separately from workload readiness. Canonical and exec processes use this same pair; live policy updates cannot change it. + On Docker, the image's `WORKDIR` becomes the workspace. Images with no `WORKDIR`, `/`, or `/sandbox` use `/sandbox`. Any other `WORKDIR` must exist in the image and be writable by the sandbox user. Podman, Kubernetes, and MicroVM always use `/sandbox`. diff --git a/e2e/rust/tests/vm_overlay.rs b/e2e/rust/tests/vm_overlay.rs index 8d1797ef27..6d85d9c727 100644 --- a/e2e/rust/tests/vm_overlay.rs +++ b/e2e/rust/tests/vm_overlay.rs @@ -4,10 +4,12 @@ //! VM-driver-specific assertions for the sandbox root filesystem. use std::process::Stdio; +use std::time::Duration; use openshell_e2e::harness::binary::openshell_cmd; +use openshell_e2e::harness::cli::{run_cli, wait_for_sandbox_phase}; use openshell_e2e::harness::output::strip_ansi; -use openshell_e2e::harness::sandbox::SandboxGuard; +use openshell_e2e::harness::sandbox::{SandboxGuard, unique_sandbox_name}; #[tokio::test] async fn vm_overlay() { @@ -55,3 +57,156 @@ async fn vm_overlay() { sandbox.cleanup().await; } + +const IDENTITY_MAIN: &str = "set -eu; if ! test -f /sandbox/canonical-identity; then printf '%s:%s\\n' \"$(id -u)\" \"$(id -g)\" > /sandbox/canonical-identity; fi; echo vm-identity-ready; exec sleep infinity"; + +fn identity_policy(user: &str, group: &str) -> tempfile::NamedTempFile { + let file = tempfile::NamedTempFile::new().expect("temporary identity policy"); + std::fs::write( + file.path(), + format!( + r#"version: 1 +filesystem_policy: + include_workdir: true + read_only: [/usr, /lib, /lib64, /proc, /etc, /dev/urandom] + read_write: [/sandbox, /tmp, /dev/null] +landlock: + compatibility: hard_requirement +process: + run_as_user: "{user}" + run_as_group: "{group}" +network_policies: {{}} +"# + ), + ) + .expect("write identity policy"); + file +} + +async fn assert_workload_identity(sandbox: &SandboxGuard) -> (String, String) { + let output = sandbox.exec(&[ + "sh", "-c", + "set -eu; actual=$(id -u):$(id -g); test \"$(cat /sandbox/canonical-identity)\" = \"$actual\"; test \"$(stat -c %u:%g /sandbox/canonical-identity)\" = \"$actual\"; printf 'identity=%s\\n' \"$actual\"", + ]).await.expect("canonical and exec identities and file ownership agree"); + let clean = strip_ansi(&output); + let pair = clean + .lines() + .find_map(|line| line.strip_prefix("identity=")) + .expect("observed workload identity"); + let (uid, gid) = pair.split_once(':').expect("UID:GID pair"); + let (status, code) = run_cli(&["sandbox", "get", &sandbox.name, "--output", "json"]).await; + assert_eq!(code, 0, "{status}"); + let status: serde_json::Value = + serde_json::from_str(&strip_ansi(&status)).expect("sandbox status JSON"); + assert!( + status["conditions"] + .as_array() + .expect("conditions") + .iter() + .any(|condition| { + condition["type"] == "WorkloadIdentity" + && condition["message"] == format!("Resolved workload UID:GID is {pair}") + }), + "status must report observed workload identity: {status}" + ); + (uid.to_string(), gid.to_string()) +} + +#[tokio::test] +async fn vm_identity_matches_status_and_rejects_conflicts() { + // Use observed defaults so the test also works with configured driver IDs. + // The canonical process writes a file before signaling readiness; exec + // verifies its content and ownership independently of the status report. + let default_policy = identity_policy("", ""); + let mut sandbox = SandboxGuard::create_keep_with_args( + &[ + "--policy", + default_policy.path().to_str().unwrap(), + "--no-tty", + ], + &["sh", "-c", IDENTITY_MAIN], + "vm-identity-ready", + ) + .await + .expect("omitted selectors use driver identity"); + let (uid, gid) = assert_workload_identity(&sandbox).await; + let original = sandbox + .exec(&["stat", "-c", "%u:%g", "/sandbox/canonical-identity"]) + .await + .unwrap(); + let (output, code) = run_cli(&["sandbox", "stop", &sandbox.name]).await; + assert_eq!(code, 0, "{output}"); + wait_for_sandbox_phase(&sandbox.name, "Stopped", Duration::from_secs(60)) + .await + .unwrap(); + let (output, code) = run_cli(&["sandbox", "start", &sandbox.name]).await; + assert_eq!(code, 0, "{output}"); + wait_for_sandbox_phase(&sandbox.name, "Ready", Duration::from_secs(120)) + .await + .unwrap(); + assert_eq!( + assert_workload_identity(&sandbox).await, + (uid.clone(), gid.clone()) + ); + let restored = sandbox + .exec(&["stat", "-c", "%u:%g", "/sandbox/canonical-identity"]) + .await + .unwrap(); + assert_eq!( + strip_ansi(&restored), + strip_ansi(&original), + "restart must retain the owned overlay file" + ); + sandbox.cleanup().await; + + for (user, group) in [ + (uid.as_str(), ""), + ("", gid.as_str()), + ("sandbox", "sandbox"), + ] { + let policy = identity_policy(user, group); + let mut matching = SandboxGuard::create_keep_with_args( + &["--policy", policy.path().to_str().unwrap(), "--no-tty"], + &["sh", "-c", IDENTITY_MAIN], + "vm-identity-ready", + ) + .await + .expect("matching numeric or symbolic selector"); + assert_eq!( + assert_workload_identity(&matching).await, + (uid.clone(), gid.clone()) + ); + matching.cleanup().await; + } + + let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; + let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + for (user, group, field) in [ + (wrong_uid, "", "run_as_user"), + ("", wrong_gid, "run_as_group"), + ] { + let policy = identity_policy(user, group); + let name = unique_sandbox_name(); + let mut cleanup = SandboxGuard::manage_existing(name.clone()); + let result = SandboxGuard::create(&[ + "--name", + &name, + "--policy", + policy.path().to_str().unwrap(), + "--no-tty", + ]) + .await; + let error = match result { + Ok(mut launched) => { + launched.cleanup().await; + panic!("conflicting VM identity reached Ready"); + } + Err(error) => error, + }; + assert!( + error.contains(field) && error.contains(&format!("{uid}:{gid}")), + "rejection must name selector and resolved identity: {error}" + ); + cleanup.cleanup().await; + } +} From 40e8e7d7a8d0099fbf0f95932ffb7a21195e465c Mon Sep 17 00:00:00 2001 From: Shiju Date: Thu, 1 Oct 2026 15:02:25 +0530 Subject: [PATCH 2/5] test(sandbox): clarify VM identity rejection fixtures Name invalid user and group fixtures distinctly and move the final workload identity into its group mismatch test. Signed-off-by: Shiju --- .../openshell-sandbox/src/boundary_server.rs | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index a592387a01..bc486061f6 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -4681,12 +4681,20 @@ mod linux { validate_config(&config).unwrap(); validate_running_identity(&config.workload_identity, false).unwrap(); - let mut wrong_uid = config.workload_identity.clone(); - wrong_uid.uid = if wrong_uid.uid == 10000 { 10001 } else { 10000 }; - assert!(validate_running_identity(&wrong_uid, false).is_err()); - let mut wrong_gid = config.workload_identity.clone(); - wrong_gid.gid = if wrong_gid.gid == 10000 { 10001 } else { 10000 }; - assert!(validate_running_identity(&wrong_gid, false).is_err()); + let mut wrong_user = config.workload_identity.clone(); + wrong_user.uid = if wrong_user.uid == 10000 { + 10001 + } else { + 10000 + }; + assert!(validate_running_identity(&wrong_user, false).is_err()); + let mut wrong_group = config.workload_identity; + wrong_group.gid = if wrong_group.gid == 10000 { + 10001 + } else { + 10000 + }; + assert!(validate_running_identity(&wrong_group, false).is_err()); } fn vm_identity_test_runtime() -> (tokio::runtime::Runtime, Arc) { @@ -4742,14 +4750,14 @@ mod linux { ) .expect("matching or omitted VM selectors"); } - let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; - let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + let wrong_user = if uid == "10000" { "10001" } else { "10000" }; + let wrong_group = if gid == "10000" { "10001" } else { "10000" }; for (user, group, field) in [ - (Some(wrong_uid), None, "run_as_user"), - (None, Some(wrong_gid), "run_as_group"), - (Some(uid.as_str()), Some(wrong_gid), "run_as_group"), - (Some(wrong_uid), Some(gid.as_str()), "run_as_user"), - (Some(wrong_uid), Some(wrong_gid), "run_as_user"), + (Some(wrong_user), None, "run_as_user"), + (None, Some(wrong_group), "run_as_group"), + (Some(uid.as_str()), Some(wrong_group), "run_as_group"), + (Some(wrong_user), Some(gid.as_str()), "run_as_user"), + (Some(wrong_user), Some(wrong_group), "run_as_user"), ] { let error = validate_vm_policy_identity( &boundary.config, @@ -4785,7 +4793,7 @@ mod linux { .remove("vm.generation"); validate_vm_policy_identity( &boundary.config, - &vm_identity_test_policy(Some(wrong_uid), Some("image-user")), + &vm_identity_test_policy(Some(wrong_user), Some("image-user")), ) .expect("non-VM identity behavior is unchanged"); } @@ -4796,11 +4804,11 @@ mod linux { let identity = &boundary.config.workload_identity; let uid = identity.uid.to_string(); let gid = identity.gid.to_string(); - let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; - let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + let wrong_user = if uid == "10000" { "10001" } else { "10000" }; + let wrong_group = if gid == "10000" { "10001" } else { "10000" }; for (user, group, field, requested) in [ - (Some(wrong_uid), None, "run_as_user", wrong_uid), - (None, Some(wrong_gid), "run_as_group", wrong_gid), + (Some(wrong_user), None, "run_as_user", wrong_user), + (None, Some(wrong_group), "run_as_group", wrong_group), ] { let response = boundary.attach(vm_identity_test_policy(user, group)); let Response::Error { kind, message } = response else { @@ -4830,14 +4838,14 @@ mod linux { let identity = &boundary.config.workload_identity; let uid = identity.uid.to_string(); let gid = identity.gid.to_string(); - let wrong_uid = if uid == "10000" { "10001" } else { "10000" }; - let wrong_gid = if gid == "10000" { "10001" } else { "10000" }; + let wrong_user = if uid == "10000" { "10001" } else { "10000" }; + let wrong_group = if gid == "10000" { "10001" } else { "10000" }; *lock(&boundary.state) = RuntimeState::Ready(PreparedBoundary { network_broker: boundary.network_broker.clone(), }); for (user, group, field, requested) in [ - (Some(wrong_uid), None, "run_as_user", wrong_uid), - (None, Some(wrong_gid), "run_as_group", wrong_gid), + (Some(wrong_user), None, "run_as_user", wrong_user), + (None, Some(wrong_group), "run_as_group", wrong_group), ] { let response = boundary.start_agent( boundary.config.boundary_id.clone(), From 042bc23704c66cd8ed527ee50c509da3bdb6c531 Mon Sep 17 00:00:00 2001 From: Shiju Date: Sat, 3 Oct 2026 03:50:17 +0530 Subject: [PATCH 3/5] fix(supervisor): align VM identity startup with current APIs Pass the optional rejection-log key for VM identity failures and keep generic startup-write regressions free of VM identity constraints. Repair the call sites after the branch rebase so the identity and cleanup proposals compile against the current startup helpers. Signed-off-by: Shiju --- crates/openshell-supervisor/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index fa5613d416..5f2c95bd1f 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -2465,6 +2465,7 @@ async fn load_policy_with_gateway( &instance_id, &snapshot, &error.to_string(), + None, ) .await?; reconciliation_attempts = 0; @@ -6204,6 +6205,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(discovery), &startup_gateway, ) @@ -6352,6 +6354,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(discovery), &gateway, ), @@ -6677,6 +6680,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(ImagePolicyDiscovery::Missing), &gateway, ), @@ -6771,6 +6775,7 @@ network_policies: None, &openshell_extension_core::ExtensionCredentialStore::new(), LocalPolicyIdentity::Required, + None, Some(discovery.clone()), &gateway, ), From fd1a801cfe025bb41c3a493797808baf674cc59e Mon Sep 17 00:00:00 2001 From: Shiju Date: Sat, 3 Oct 2026 07:21:56 +0530 Subject: [PATCH 4/5] fix(vm): restore inactive sandbox workload identity Recover the persisted overlay owner before publishing stopped and terminal sandboxes. Keep resources manageable when identity metadata is invalid. Clarify fixed MicroVM ownership in policy-generation guidance. Signed-off-by: Shiju --- crates/openshell-driver-vm/src/driver.rs | 256 ++++++++++++++++++++--- skills/generate-sandbox-policy/SKILL.md | 2 + 2 files changed, 231 insertions(+), 27 deletions(-) diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index a03982038d..f361a3fdb0 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -2170,38 +2170,41 @@ impl VmDriver { continue; } - if tokio::fs::metadata(state_dir.join(SANDBOX_STOPPED_FILE)) + let inactive_condition = if tokio::fs::metadata(state_dir.join(SANDBOX_STOPPED_FILE)) .await .is_ok() { - let snapshot = sandbox_snapshot(&sandbox, stopped_condition(), false); - let mut registry = self.registry.lock().await; - registry.entry(sandbox.id.clone()).or_insert(SandboxRecord { - snapshot: snapshot.clone(), - state_dir: state_dir.clone(), - process: None, - provisioning_task: None, - gpu_bdf: None, - deleting: false, - }); - drop(registry); - self.publish_snapshot(snapshot); - info!(sandbox_id = %sandbox.id, "vm driver: restored stopped sandbox without launching compute"); - continue; - } - - if tokio::fs::try_exists(state_dir.join(MAIN_PROCESS_EXITED_FILE)) + Some(stopped_condition()) + } else if tokio::fs::try_exists(state_dir.join(MAIN_PROCESS_EXITED_FILE)) .await .unwrap_or(false) { - let snapshot = sandbox_snapshot( - &sandbox, - error_condition( - "ProcessExited", - "Canonical main process exited before VM driver restart", - ), - false, - ); + Some(error_condition( + "ProcessExited", + "Canonical main process exited before VM driver restart", + )) + } else { + None + }; + if let Some(condition) = inactive_condition { + // These sandboxes do not run the launch path that publishes + // identity. Recover it from the same validated owner marker + // before either GetSandbox or WatchSandboxes can observe them. + let identity = match read_persisted_workload_identity(&state_dir).await { + Ok(identity) => identity, + Err(error) => { + warn!(sandbox_id = %sandbox.id, %error, "vm driver: ignoring invalid persisted workload identity"); + // Keep inactive resources manageable even when their + // identity cannot be trusted. Explicit deletion still + // needs this registry entry to remove persisted state. + None + } + }; + let mut snapshot = sandbox_snapshot(&sandbox, condition, false); + snapshot + .status + .get_or_insert_with(SandboxStatus::default) + .resolved_identity = identity; let mut registry = self.registry.lock().await; registry.entry(sandbox.id.clone()).or_insert(SandboxRecord { snapshot: snapshot.clone(), @@ -2215,7 +2218,7 @@ impl VmDriver { self.publish_snapshot(snapshot); info!( sandbox_id = %sandbox.id, - "vm driver: preserved terminal sandbox without restarting canonical process" + "vm driver: restored inactive sandbox without launching compute" ); continue; } @@ -5790,6 +5793,35 @@ async fn persisted_sandbox_owner_identity( Ok(None) } +/// Reconstruct status from the owner chosen during preparation, never from a +/// new driver default or a status embedded in the persisted create request. +/// Missing or v1 markers do not contain an identity to report. Invalid owner +/// data must not be published as a resolved identity. +async fn read_persisted_workload_identity( + state_dir: &Path, +) -> Result, String> { + let contents = match tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE)).await { + Ok(contents) if contents.trim() == SANDBOX_OWNER_STATE_V1 => return Ok(None), + Ok(contents) => contents, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(format!("read sandbox owner state: {error}")), + }; + let owner = parse_sandbox_owner_state(&contents) + .map_err(|error| format!("invalid sandbox owner state: {error}"))?; + let resource_digest = match read_persisted_image_identity(state_dir).await { + Ok(identity) => identity, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(error) => return Err(format!("read persisted VM image identity: {error}")), + }; + Ok(Some(ResolvedWorkloadIdentity { + uid: owner.uid, + gid: owner.gid, + supplementary_gids: Vec::new(), + source: "vm-config".into(), + resource_digest, + })) +} + async fn sandbox_owner_identity_from_image( image_path: &Path, ) -> Result { @@ -7504,6 +7536,172 @@ mod tests { } } + async fn assert_inactive_restore_retains_identity(marker: &str, reason: &str) { + let temp = tempfile::tempdir().unwrap(); + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = temp.path().to_path_buf(); + // Current driver defaults and serialized status are not evidence of + // the identity that owns an already prepared overlay. + driver.config.sandbox_uid = Some(9000); + driver.config.sandbox_gid = Some(9001); + let sandbox = Sandbox { + id: "sb-inactive-identity".into(), + name: "inactive-identity".into(), + status: Some(SandboxStatus { + resolved_identity: Some(ResolvedWorkloadIdentity { + uid: 8000, + gid: 8001, + ..Default::default() + }), + ..Default::default() + }), + ..Default::default() + }; + let state_dir = sandboxes_root_dir(temp.path()).join(&sandbox.id); + tokio::fs::create_dir_all(&state_dir).await.unwrap(); + write_sandbox_request(&state_dir, &sandbox).await.unwrap(); + write_sandbox_owner_state( + &state_dir, + SandboxOwnerIdentity { + uid: 4242, + gid: 4343, + }, + ) + .await + .unwrap(); + write_sandbox_image_metadata(&state_dir, "unused-image", "sha256:original-image") + .await + .unwrap(); + tokio::fs::write(state_dir.join(marker), b"inactive\n") + .await + .unwrap(); + let mut events = driver.events.subscribe(); + + driver.restore_persisted_sandboxes().await; + + let restored = driver + .get_sandbox(&sandbox.id, &sandbox.name) + .await + .unwrap() + .unwrap(); + let status = restored.status.as_ref().unwrap(); + assert_eq!(status.conditions[0].reason, reason); + let identity = status + .resolved_identity + .as_ref() + .expect("restored overlay owner"); + assert_eq!((identity.uid, identity.gid), (4242, 4343)); + assert!(identity.supplementary_gids.is_empty()); + assert_eq!(identity.source, "vm-config"); + assert_eq!(identity.resource_digest, "sha256:original-image"); + let registry = driver.registry.lock().await; + let record = registry.get(&sandbox.id).unwrap(); + assert!(record.process.is_none()); + assert!(record.provisioning_task.is_none()); + drop(registry); + let event = events.try_recv().expect("restored snapshot event"); + let Some(watch_sandboxes_event::Payload::Sandbox(event)) = event.payload else { + panic!("expected sandbox snapshot event"); + }; + assert_eq!(event.sandbox.as_ref(), Some(&restored)); + assert_eq!( + tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE)) + .await + .unwrap(), + "sandbox-owner-v2:4242:4343\n" + ); + assert!(state_dir.join(marker).exists()); + } + + #[tokio::test] + async fn stopped_restore_retains_persisted_workload_identity() { + assert_inactive_restore_retains_identity(SANDBOX_STOPPED_FILE, "ComputeStopped").await; + } + + #[tokio::test] + async fn terminal_restore_retains_persisted_workload_identity() { + assert_inactive_restore_retains_identity(MAIN_PROCESS_EXITED_FILE, "ProcessExited").await; + } + + #[tokio::test] + async fn inactive_restore_keeps_invalid_metadata_manageable() { + for (marker, reason) in [ + (SANDBOX_STOPPED_FILE, "ComputeStopped"), + (MAIN_PROCESS_EXITED_FILE, "ProcessExited"), + ] { + for invalid_owner in [true, false] { + let temp = tempfile::tempdir().unwrap(); + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = temp.path().to_path_buf(); + let sandbox = Sandbox { + id: "sb-invalid-owner".into(), + name: "invalid-owner".into(), + ..Default::default() + }; + let state_dir = sandboxes_root_dir(temp.path()).join(&sandbox.id); + tokio::fs::create_dir_all(&state_dir).await.unwrap(); + write_sandbox_request(&state_dir, &sandbox).await.unwrap(); + tokio::fs::write(state_dir.join(marker), b"inactive\n") + .await + .unwrap(); + let owner = if invalid_owner { + "sandbox-owner-v2:0:4343\n" + } else { + "sandbox-owner-v2:4242:4343\n" + }; + tokio::fs::write(state_dir.join(SANDBOX_OWNER_STATE_FILE), owner) + .await + .unwrap(); + if !invalid_owner { + // A directory deterministically fails read_to_string on every test host. + tokio::fs::create_dir(state_dir.join(IMAGE_IDENTITY_FILE)) + .await + .unwrap(); + } + let mut events = driver.events.subscribe(); + + driver.restore_persisted_sandboxes().await; + + let restored = driver + .get_sandbox(&sandbox.id, &sandbox.name) + .await + .unwrap() + .expect("inactive sandbox remains manageable"); + let status = restored.status.as_ref().unwrap(); + assert_eq!(status.conditions[0].reason, reason); + assert!(status.resolved_identity.is_none()); + let registry = driver.registry.lock().await; + let record = registry.get(&sandbox.id).unwrap(); + assert!(record.process.is_none()); + assert!(record.provisioning_task.is_none()); + drop(registry); + let event = events.try_recv().expect("inactive snapshot event"); + let Some(watch_sandboxes_event::Payload::Sandbox(event)) = event.payload else { + panic!("expected sandbox snapshot"); + }; + assert_eq!(event.sandbox.as_ref(), Some(&restored)); + assert!(state_dir.join(marker).exists()); + assert_eq!( + tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE)) + .await + .unwrap(), + owner + ); + assert!( + driver + .delete_sandbox(&sandbox.id, &sandbox.name) + .await + .unwrap() + .deleted + ); + assert!( + !state_dir.exists(), + "explicit delete removes retained state" + ); + } + } + } + #[tokio::test] async fn startup_does_not_restore_terminal_canonical_process() { let temp = tempfile::tempdir().unwrap(); @@ -7538,6 +7736,10 @@ mod tests { assert!(record.process.is_none()); assert!(record.provisioning_task.is_none()); let status = record.snapshot.status.as_ref().expect("terminal status"); + assert!( + status.resolved_identity.is_none(), + "an absent owner marker must not invent an identity" + ); assert!(status.conditions.iter().any(|condition| { condition.reason == "ProcessExited" && condition.status == "False" })); diff --git a/skills/generate-sandbox-policy/SKILL.md b/skills/generate-sandbox-policy/SKILL.md index ac03c8576b..d626aaf1b3 100644 --- a/skills/generate-sandbox-policy/SKILL.md +++ b/skills/generate-sandbox-policy/SKILL.md @@ -514,6 +514,8 @@ When the user explicitly requests `process.run_as_user` or (`4294967295`). Warn that a low numeric identity inherits permissions granted to the same ID on image files, mounted volumes, or devices. +For MicroVM, numeric selectors must match the resolved owner of the sandbox's writable overlay. User and group are checked independently; either mismatch prevents startup. If the owner UID:GID is unknown, omit the selectors or use `sandbox` so the driver retains that identity. Do not choose another numeric identity or suggest that the policy can change an existing overlay's owner. For example, with an owner of `1000:1000`, a request for UID `10000` must be rejected with an explanation and the omission/`sandbox` alternatives, rather than generating a policy that the VM cannot start. + If the user provides a file path, write to it. Otherwise, ask where to place it. A common convention is a project-local policy file (e.g., `sandbox-policy.yaml`) passed to `openshell sandbox create --policy ` or set via the `OPENSHELL_SANDBOX_POLICY` env var. ### Mode C: Present Only (no file write) From e62095dbd577c5eb8db06dba8785044ccf576aa1 Mon Sep 17 00:00:00 2001 From: Shiju Date: Sat, 3 Oct 2026 07:49:17 +0530 Subject: [PATCH 5/5] test(vm): flush identity fixture before restart Persist the canonical identity file before readiness and report the observed exec, canonical and file-owner identities before comparing them. Signed-off-by: Shiju --- e2e/rust/tests/vm_overlay.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/rust/tests/vm_overlay.rs b/e2e/rust/tests/vm_overlay.rs index 6d85d9c727..16a509376d 100644 --- a/e2e/rust/tests/vm_overlay.rs +++ b/e2e/rust/tests/vm_overlay.rs @@ -58,7 +58,9 @@ async fn vm_overlay() { sandbox.cleanup().await; } -const IDENTITY_MAIN: &str = "set -eu; if ! test -f /sandbox/canonical-identity; then printf '%s:%s\\n' \"$(id -u)\" \"$(id -g)\" > /sandbox/canonical-identity; fi; echo vm-identity-ready; exec sleep infinity"; +// VM stop terminates the guest without flushing its page cache. Flush this +// fixture before readiness so restart checks durable file content and ownership. +const IDENTITY_MAIN: &str = "set -eu; if ! test -f /sandbox/canonical-identity; then printf '%s:%s\\n' \"$(id -u)\" \"$(id -g)\" > /sandbox/canonical-identity; sync; fi; echo vm-identity-ready; exec sleep infinity"; fn identity_policy(user: &str, group: &str) -> tempfile::NamedTempFile { let file = tempfile::NamedTempFile::new().expect("temporary identity policy"); @@ -86,7 +88,7 @@ network_policies: {{}} async fn assert_workload_identity(sandbox: &SandboxGuard) -> (String, String) { let output = sandbox.exec(&[ "sh", "-c", - "set -eu; actual=$(id -u):$(id -g); test \"$(cat /sandbox/canonical-identity)\" = \"$actual\"; test \"$(stat -c %u:%g /sandbox/canonical-identity)\" = \"$actual\"; printf 'identity=%s\\n' \"$actual\"", + "set -eu; actual=$(id -u):$(id -g); canonical=$(cat /sandbox/canonical-identity); owner=$(stat -c %u:%g /sandbox/canonical-identity); printf 'exec=%s canonical=%s owner=%s\\n' \"$actual\" \"$canonical\" \"$owner\"; test \"$canonical\" = \"$actual\"; test \"$owner\" = \"$actual\"; printf 'identity=%s\\n' \"$actual\"", ]).await.expect("canonical and exec identities and file ownership agree"); let clean = strip_ansi(&output); let pair = clean