From cbe9e23806ba7802a8fe50de63a779810c89e325 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sat, 19 Sep 2026 14:27:32 -0700 Subject: [PATCH 1/9] fix(mxc): reject unsupported live policy updates (NVBug 6782891) Signed-off-by: Prekshi Vyas --- crates/openshell-cli/src/run.rs | 39 ++++++- .../sandbox_name_fallback_integration.rs | 88 ++++++++++++++- crates/openshell-driver-docker/src/lib.rs | 1 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-mxc/src/driver.rs | 13 +++ crates/openshell-driver-podman/src/driver.rs | 1 + crates/openshell-driver-vm/src/driver.rs | 1 + crates/openshell-server/src/compute/mod.rs | 25 ++++- crates/openshell-server/src/grpc/policy.rs | 103 +++++++++++++++++- crates/openshell-server/src/test_support.rs | 1 + proto/compute_driver.proto | 5 + 11 files changed, 268 insertions(+), 10 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 673d4a9da3..7b0db76078 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5171,6 +5171,32 @@ where } else { config.version }; + let (status, active_version) = if policy_source == PolicySource::Global { + // Global policy writes are validated and marked loaded synchronously. + // Do not query the global history endpoint here: policy get on a + // sandbox is workspace-readable, while global history is platform-admin + // scoped. + (PolicyStatus::Loaded, version) + } else { + let status_response = client + .get_sandbox_policy_status(GetSandboxPolicyStatusRequest { + name: name.to_string(), + version, + global: false, + workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)), + }) + .await + .into_diagnostic()? + .into_inner(); + let revision = status_response + .revision + .as_ref() + .ok_or_else(|| miette!("policy version {version} is missing from policy history"))?; + ( + PolicyStatus::try_from(revision.status).unwrap_or(PolicyStatus::Unspecified), + status_response.active_version, + ) + }; match output { "json" => { @@ -5178,9 +5204,15 @@ where obj.insert("scope".to_string(), serde_json::json!("sandbox")); obj.insert("sandbox".to_string(), serde_json::json!(name)); obj.insert("version".to_string(), serde_json::json!(version)); - obj.insert("active_version".to_string(), serde_json::json!(version)); + obj.insert( + "active_version".to_string(), + serde_json::json!(active_version), + ); obj.insert("hash".to_string(), serde_json::json!(config.policy_hash)); - obj.insert("status".to_string(), serde_json::json!("effective")); + obj.insert( + "status".to_string(), + serde_json::json!(policy_status_json_name(status)), + ); obj.insert( "config_revision".to_string(), serde_json::json!(config.config_revision), @@ -5211,8 +5243,9 @@ where } "table" => { writeln!(stdout, "Version: {version}").into_diagnostic()?; + writeln!(stdout, "Active: {active_version}").into_diagnostic()?; writeln!(stdout, "Hash: {}", config.policy_hash).into_diagnostic()?; - writeln!(stdout, "Status: Effective").into_diagnostic()?; + writeln!(stdout, "Status: {status:?}").into_diagnostic()?; writeln!(stdout, "Source: {policy_source_label}").into_diagnostic()?; writeln!(stdout, "Config rev: {}", config.config_revision).into_diagnostic()?; if config.global_policy_version > 0 { diff --git a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs index d040501590..6e18dec517 100644 --- a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs +++ b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs @@ -25,6 +25,7 @@ use openshell_core::proto::{ ServiceStatus, SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, }; use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; use tempfile::TempDir; use tokio::net::TcpListener; use tokio::sync::{Mutex, mpsc}; @@ -38,6 +39,7 @@ use tonic::{Response, Status}; #[derive(Clone, Default)] struct SandboxState { last_get_name: Arc>>, + global_policy: Arc, } #[derive(Clone, Default)] @@ -191,6 +193,7 @@ impl OpenShell for TestOpenShell { req.sandbox_id, "test-id", "GetSandboxConfig should pass the id from GetSandbox" ); + let global_policy = self.state.global_policy.load(Ordering::Relaxed); Ok(Response::new(GetSandboxConfigResponse { policy: Some(SandboxPolicy { version: 9, @@ -233,7 +236,12 @@ impl OpenShell for TestOpenShell { version: 9, policy_hash: "sha256:effective-policy".to_string(), config_revision: 42, - policy_source: openshell_core::proto::PolicySource::Sandbox.into(), + policy_source: if global_policy { + openshell_core::proto::PolicySource::Global.into() + } else { + openshell_core::proto::PolicySource::Sandbox.into() + }, + global_policy_version: if global_policy { 4 } else { 0 }, ..Default::default() })) } @@ -460,9 +468,22 @@ impl OpenShell for TestOpenShell { ) -> Result, Status> { let req = request.into_inner(); assert_eq!(req.name, "my-sandbox"); - assert_eq!(req.version, 3); assert!(!req.global); + if req.version == 9 { + return Ok(Response::new(GetSandboxPolicyStatusResponse { + revision: Some(SandboxPolicyRevision { + version: 9, + policy_hash: "sha256:effective-policy".to_string(), + status: PolicyStatus::Pending.into(), + ..Default::default() + }), + active_version: 7, + })); + } + + assert_eq!(req.version, 3); + let policy = SandboxPolicy { version: 7, network_policies: std::iter::once(( @@ -843,9 +864,9 @@ async fn policy_get_full_json_cli_prints_policy_payload() { assert_eq!(json["scope"], "sandbox"); assert_eq!(json["sandbox"], "my-sandbox"); assert_eq!(json["version"], 9); - assert_eq!(json["active_version"], 9); + assert_eq!(json["active_version"], 7); assert_eq!(json["hash"], "sha256:effective-policy"); - assert_eq!(json["status"], "effective"); + assert_eq!(json["status"], "pending"); assert_eq!(json["config_revision"], 42); assert_eq!(json["policy_source"], "sandbox"); assert_eq!( @@ -891,7 +912,7 @@ async fn policy_get_base_json_cli_prints_round_trippable_policy_payload() { serde_json::from_slice(&stdout).expect("stdout should be valid JSON"); assert_eq!(json["scope"], "sandbox"); assert_eq!(json["sandbox"], "my-sandbox"); - assert_eq!(json["status"], "effective"); + assert_eq!(json["status"], "pending"); assert!( json["policy"]["network_policies"] .get("_provider_api") @@ -904,6 +925,63 @@ async fn policy_get_base_json_cli_prints_round_trippable_policy_payload() { ); } +#[tokio::test] +async fn policy_get_latest_table_reports_persisted_status_and_active_version() { + let ts = run_server().await; + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + run::sandbox_policy_get_to_writer( + &ts.endpoint, + "my-sandbox", + 0, + run::PolicyGetView::Metadata, + "table", + "default", + &ts.tls, + (&mut stdout, &mut stderr), + ) + .await + .expect("policy get should succeed"); + + assert!(stderr.is_empty()); + let output = String::from_utf8(stdout).expect("table output should be UTF-8"); + assert!(output.contains("Version: 9"), "{output}"); + assert!(output.contains("Active: 7"), "{output}"); + assert!(output.contains("Status: Pending"), "{output}"); +} + +#[tokio::test] +async fn policy_get_global_effective_policy_remains_workspace_readable() { + let ts = run_server().await; + ts.openshell + .state + .global_policy + .store(true, Ordering::Relaxed); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + run::sandbox_policy_get_to_writer( + &ts.endpoint, + "my-sandbox", + 0, + run::PolicyGetView::Metadata, + "json", + "default", + &ts.tls, + (&mut stdout, &mut stderr), + ) + .await + .expect("workspace-readable global effective policy should not query admin-only history"); + + assert!(stderr.is_empty()); + let json: serde_json::Value = serde_json::from_slice(&stdout).expect("valid JSON"); + assert_eq!(json["version"], 4); + assert_eq!(json["active_version"], 4); + assert_eq!(json["status"], "loaded"); + assert_eq!(json["policy_source"], "global"); +} + #[tokio::test] async fn policy_get_explicit_revision_uses_stored_policy_status() { let ts = run_server().await; diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 62ba8c9f69..376617e306 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -668,6 +668,7 @@ impl DockerComputeDriver { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, } } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 7b33d4b963..c6b8f3d218 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -580,6 +580,7 @@ impl KubernetesComputeDriver { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }) } diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 072b1debf1..b3a5b53a98 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -813,6 +813,7 @@ impl MxcComputeBackend { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: self.config.backend == MxcBackend::ProcessContainer, + supports_live_policy_updates: Some(false), } } @@ -2388,12 +2389,24 @@ mod lifecycle_tests { fn ui_policy_capability_tracks_configured_backend() { let process_container = MxcComputeBackend::new_mocked(MxcComputeConfig::default()); assert!(process_container.capabilities().supports_ui_policy); + assert_eq!( + process_container + .capabilities() + .supports_live_policy_updates, + Some(false) + ); let isolation_session = MxcComputeBackend::new_mocked(MxcComputeConfig { backend: MxcBackend::IsolationSession, ..Default::default() }); assert!(!isolation_session.capabilities().supports_ui_policy); + assert_eq!( + isolation_session + .capabilities() + .supports_live_policy_updates, + Some(false) + ); } fn driver_sandbox_with_command(id: &str, cwd: &str, command: Vec) -> DriverSandbox { diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index ae564cd4ab..c92f4e8ced 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -529,6 +529,7 @@ impl PodmanComputeDriver { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index b1b3073958..ecd3fa68fa 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -801,6 +801,7 @@ impl VmDriver { .into_owned(), rootfs_tar_max_bytes: self.config.rootfs_tar_max_bytes(), supports_ui_policy: false, + supports_live_policy_updates: None, } } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index be57ca32dd..969d777ed3 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -382,6 +382,9 @@ pub struct ComputeDriverInfoSnapshot { /// Whether this configured driver instance completely enforces the portable /// UI policy contract. pub supports_ui_policy: bool, + /// Whether this driver can apply policy changes after sandbox creation. + /// `None` preserves the behavior of older external drivers. + pub supports_live_policy_updates: Option, } /// Interval between store-vs-backend reconciliation sweeps. @@ -743,6 +746,7 @@ impl ComputeRuntime { rootfs_tar_staging_dir: capabilities.rootfs_tar_staging_dir, rootfs_tar_max_bytes: capabilities.rootfs_tar_max_bytes, supports_ui_policy: capabilities.supports_ui_policy, + supports_live_policy_updates: capabilities.supports_live_policy_updates, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -924,6 +928,15 @@ impl ComputeRuntime { self.driver_info.supports_sandbox_authentication } + /// Whether operator-authored policy updates can reach an already-created + /// sandbox. Unspecified preserves compatibility with older drivers. + #[must_use] + pub(crate) fn supports_live_policy_updates(&self) -> bool { + self.driver_info + .supports_live_policy_updates + .unwrap_or(true) + } + pub(crate) async fn authenticate_sandbox(&self, credential: &str) -> Result { if !self.supports_sandbox_authentication() { return Err(Status::unimplemented( @@ -5001,6 +5014,7 @@ impl ComputeDriver for NoopTestDriver { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }, )) } @@ -5126,7 +5140,12 @@ pub async fn new_test_runtime(store: Arc) -> ComputeRuntime { #[cfg(test)] pub async fn new_test_runtime_for_driver(store: Arc, driver_name: &str) -> ComputeRuntime { - new_test_runtime_with_driver(store, driver_name, Arc::new(NoopTestDriver::default())).await + let mut runtime = + new_test_runtime_with_driver(store, driver_name, Arc::new(NoopTestDriver::default())).await; + if driver_name == "mxc" { + runtime.driver_info.supports_live_policy_updates = Some(false); + } + runtime } #[cfg(test)] @@ -5149,6 +5168,7 @@ pub async fn new_test_runtime_with_driver( rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }, telemetry_compute_driver: TelemetryComputeDriver::custom(), driver_process: None, @@ -5473,6 +5493,7 @@ mod tests { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, })) } @@ -5839,6 +5860,7 @@ mod tests { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, })) } @@ -6053,6 +6075,7 @@ mod tests { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }, telemetry_compute_driver: TelemetryComputeDriver::custom(), driver_process: None, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index cd71e8064f..692faefa14 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3844,6 +3844,16 @@ async fn handle_update_config_inner( let sandbox_id = sandbox.object_id().to_string(); let mut response_annotations = sandbox_metadata_annotations(&sandbox); + if !sandbox_caller + && (has_policy || has_merge_ops) + && !state.compute.supports_live_policy_updates() + { + return Err(Status::failed_precondition(format!( + "compute driver '{}' cannot apply policy updates to an existing sandbox; delete and recreate the sandbox with the requested policy", + state.compute.configured_driver_name() + ))); + } + if has_setting { let _settings_guard = state.settings_mutex.lock().await; @@ -7307,7 +7317,9 @@ mod tests { Principal, SandboxIdentitySource, SandboxPrincipal, UserPrincipal, }; use crate::grpc::provider::ProviderEnvironment; - use crate::grpc::test_support::{authed_request, test_server_state}; + use crate::grpc::test_support::{ + authed_request, test_server_state, test_server_state_with_driver, + }; use crate::persistence::test_store; use std::collections::HashMap; use std::sync::Arc; @@ -8447,6 +8459,95 @@ mod tests { ); } + #[tokio::test] + async fn mxc_rejects_operator_policy_update_before_persistence() { + use openshell_core::proto::FilesystemPolicy; + + let state = test_server_state_with_driver("mxc").await; + let sandbox_id = "mxc-live-policy"; + let mut baseline = openshell_policy::restrictive_default_policy(); + baseline.filesystem = Some(FilesystemPolicy { + read_only: vec![r"C:\Windows".to_string()], + ..Default::default() + }); + state + .store + .put_message(&test_sandbox( + sandbox_id, + sandbox_id, + baseline.clone(), + Vec::new(), + )) + .await + .expect("store MXC sandbox"); + + let mut additive = baseline; + additive + .filesystem + .as_mut() + .expect("filesystem policy") + .read_only + .push(r"C:\Program Files".to_string()); + let error = handle_update_config( + &state, + with_user(Request::new(UpdateConfigRequest { + name: sandbox_id.to_string(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + policy: Some(additive), + ..Default::default() + })), + ) + .await + .expect_err("MXC cannot apply an operator policy update to a live sandbox"); + + assert_eq!(error.code(), Code::FailedPrecondition); + assert!(error.message().contains("delete and recreate")); + assert!( + state + .store + .get_latest_policy(sandbox_id) + .await + .expect("policy history lookup") + .is_none(), + "a rejected update must not create a pending revision" + ); + } + + #[tokio::test] + async fn mxc_allows_sandbox_authored_policy_sync() { + let state = test_server_state_with_driver("mxc").await; + let sandbox_id = "mxc-policy-sync"; + let policy = openshell_policy::restrictive_default_policy(); + state + .store + .put_message(&test_sandbox( + sandbox_id, + sandbox_id, + policy.clone(), + Vec::new(), + )) + .await + .expect("store MXC sandbox"); + + let response = handle_update_config( + &state, + with_sandbox( + Request::new(UpdateConfigRequest { + name: sandbox_id.to_string(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + policy: Some(policy), + ..Default::default() + }), + sandbox_id, + ), + ) + .await + .expect("sandbox-authored startup sync remains supported") + .into_inner(); + + assert_eq!(response.version, 1); + } + #[tokio::test] async fn policy_record_identity_global_deduplicates_defaulted_mcp_history() { for (case, legacy_policy) in defaulted_mcp_policy_cases() { diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index 62487d6bdb..4261f8ab6b 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -101,6 +101,7 @@ impl FakeComputeDriver { rootfs_tar_staging_dir: String::new(), rootfs_tar_max_bytes: 0, supports_ui_policy: false, + supports_live_policy_updates: None, }, gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 1d781bd6ea..16dc25091f 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -104,6 +104,11 @@ message GetCapabilitiesResponse { // portable SandboxPolicy.ui contract. Partial support must report false so // the gateway rejects every explicit UI section before provisioning. bool supports_ui_policy = 12; + // Whether the driver can apply policy changes to an already-created sandbox. + // Omit this field to preserve the legacy gateway behavior. Drivers that + // cannot update a live sandbox must explicitly report false so the gateway + // rejects changes instead of persisting a policy that is not enforced. + optional bool supports_live_policy_updates = 13; } message AuthenticateSandboxRequest { From 9fb6fe92fb8c317999d24fe7bbd74f9b3e392eac Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sun, 20 Sep 2026 16:24:25 -0700 Subject: [PATCH 2/9] fix(mxc): gate all live policy mutations Signed-off-by: Prekshi Vyas --- crates/openshell-cli/src/run.rs | 10 +- crates/openshell-server/src/grpc/policy.rs | 373 +++++++++++++++++++-- 2 files changed, 357 insertions(+), 26 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 7b0db76078..c13eecbeeb 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5172,10 +5172,12 @@ where config.version }; let (status, active_version) = if policy_source == PolicySource::Global { - // Global policy writes are validated and marked loaded synchronously. - // Do not query the global history endpoint here: policy get on a - // sandbox is workspace-readable, while global history is platform-admin - // scoped. + // Drivers without live policy updates reject global policy mutations + // while any sandbox exists, so this sandbox necessarily received the + // reported global version at startup. Other drivers retain the + // synchronous global-policy contract. Do not query global history here: + // sandbox policy reads are workspace-readable, while that endpoint is + // platform-admin scoped. (PolicyStatus::Loaded, version) } else { let status_response = client diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 692faefa14..04e4aa3773 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1646,7 +1646,7 @@ async fn auto_approve_chunk( .await?; let credential_binding_context = merge_validation.credential_binding_context(); let merge_result = merge_chunk_into_policy_with_validation( - state.store.as_ref(), + state, sandbox_id, context.workspace, &chunk, @@ -2323,6 +2323,34 @@ async fn validate_provider_composition_for_existing_sandboxes( } } +fn require_live_policy_update_support(state: &ServerState) -> Result<(), Status> { + if state.compute.supports_live_policy_updates() { + return Ok(()); + } + + Err(Status::failed_precondition(format!( + "compute driver '{}' cannot apply policy updates to an existing sandbox; delete and recreate the sandbox with the requested policy", + state.compute.configured_driver_name() + ))) +} + +async fn require_global_policy_update_support(state: &ServerState) -> Result<(), Status> { + if state.compute.supports_live_policy_updates() { + return Ok(()); + } + + let page = state + .store + .list_message_page::(ObjectListQuery::AllWorkspaces, None, 1) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; + if page.messages.is_empty() { + return Ok(()); + } + + require_live_policy_update_support(state) +} + pub async fn validate_provider_composition_startup_preflight( state: &ServerState, ) -> Result<(), Status> { @@ -3677,6 +3705,7 @@ async fn handle_update_config_inner( // Serialize its writes after validation so a report cannot commit // evidence derived from the policy this update has replaced. let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; + require_global_policy_update_support(state).await?; let latest = state .store .get_latest_policy(GLOBAL_POLICY_SANDBOX_ID) @@ -3782,6 +3811,12 @@ async fn handle_update_config_inner( None }; let mut global_settings = load_global_settings(state.store.as_ref()).await?; + if key == POLICY_SETTING_KEY + && req.delete_setting + && global_settings.settings.contains_key(POLICY_SETTING_KEY) + { + require_global_policy_update_support(state).await?; + } let provider_composition_was_enabled = provider_policy_composition_enabled_in(&global_settings)?; let changed = if req.delete_setting { @@ -3844,14 +3879,8 @@ async fn handle_update_config_inner( let sandbox_id = sandbox.object_id().to_string(); let mut response_annotations = sandbox_metadata_annotations(&sandbox); - if !sandbox_caller - && (has_policy || has_merge_ops) - && !state.compute.supports_live_policy_updates() - { - return Err(Status::failed_precondition(format!( - "compute driver '{}' cannot apply policy updates to an existing sandbox; delete and recreate the sandbox with the requested policy", - state.compute.configured_driver_name() - ))); + if !sandbox_caller && (has_policy || has_merge_ops) { + require_live_policy_update_support(state)?; } if has_setting { @@ -3975,7 +4004,7 @@ async fn handle_update_config_inner( }; let baseline_policy = spec.policy.clone(); let (version, hash, updated_sandbox) = apply_merge_operations_with_retry( - state.store.as_ref(), + state, &sandbox_id, &workspace, baseline_policy.as_ref(), @@ -5257,7 +5286,7 @@ async fn handle_approve_draft_chunk_inner( sandbox_policy_merge_validation_data(state, &workspace, &sandbox, provider_names).await?; let credential_binding_context = merge_validation.credential_binding_context(); let merge_result = merge_chunk_into_policy_with_validation( - state.store.as_ref(), + state, &sandbox_id, &workspace, &chunk, @@ -5668,7 +5697,7 @@ async fn handle_approve_all_draft_chunks_inner( Status::failed_precondition("bulk approval has no reviewed policy snapshot") })?; match apply_merge_operations_with_retry( - state.store.as_ref(), + state, &sandbox_id, &workspace, Some(&final_base), @@ -6768,6 +6797,31 @@ fn stage_validated_merge_operation( #[allow(clippy::too_many_arguments)] async fn apply_merge_operations_with_retry( + state: &ServerState, + sandbox_id: &str, + workspace: &str, + baseline_policy: Option<&ProtoSandboxPolicy>, + operations: &[PolicyMergeOp], + validation_context: PolicyMergeValidationContext<'_>, + expected_current_effective_hash: Option<&str>, + atomic_context: Option<&AtomicPolicyWriteContext<'_>>, +) -> Result<(i64, String, Option), Status> { + require_live_policy_update_support(state)?; + apply_merge_operations_with_retry_in_store( + state.store.as_ref(), + sandbox_id, + workspace, + baseline_policy, + operations, + validation_context, + expected_current_effective_hash, + atomic_context, + ) + .await +} + +#[allow(clippy::too_many_arguments)] +async fn apply_merge_operations_with_retry_in_store( store: &Store, sandbox_id: &str, workspace: &str, @@ -6926,7 +6980,7 @@ async fn apply_merge_operations_with_retry( } async fn merge_chunk_into_policy_with_validation( - store: &Store, + state: &ServerState, sandbox_id: &str, workspace: &str, chunk: &DraftChunkRecord, @@ -6945,7 +6999,7 @@ async fn merge_chunk_into_policy_with_validation( clear_provider_credentialed_markers(policy); } apply_merge_operations_with_retry( - store, + state, sandbox_id, workspace, baseline_policy.as_ref(), @@ -6967,17 +7021,34 @@ async fn merge_chunk_into_policy( chunk: &DraftChunkRecord, provider_layers: &[ProviderPolicyLayer], ) -> Result<(i64, String), Status> { - merge_chunk_into_policy_with_validation( + let rule = NetworkPolicyRule::decode(chunk.proposed_rule.as_slice()) + .map_err(|e| Status::internal(format!("decode proposed_rule failed: {e}")))?; + let operations = [PolicyMergeOp::AddRule { + rule_name: chunk.rule_name.clone(), + rule, + }]; + validate_merge_operations_for_server(&operations)?; + let mut baseline_policy = chunk.current_effective_policy.clone(); + if let Some(policy) = &mut baseline_policy { + strip_provider_rule_names(policy); + clear_provider_credentialed_markers(policy); + } + apply_merge_operations_with_retry_in_store( store, sandbox_id, workspace, - chunk, + baseline_policy.as_ref(), + &operations, PolicyMergeValidationContext { provider_layers, credential_binding: None, }, + (!chunk.current_effective_policy_hash.is_empty()) + .then_some(chunk.current_effective_policy_hash.as_str()), + None, ) .await + .map(|(version, hash, _)| (version, hash)) } async fn remove_chunk_from_policy( @@ -6987,7 +7058,7 @@ async fn remove_chunk_from_policy( chunk: &DraftChunkRecord, ) -> Result<(i64, String), Status> { apply_merge_operations_with_retry( - state.store.as_ref(), + state, sandbox_id, workspace, None, @@ -8513,6 +8584,264 @@ mod tests { ); } + #[tokio::test] + async fn mxc_rejects_global_policy_changes_while_a_sandbox_exists() { + let state = test_server_state_with_driver("mxc").await; + let initial = test_policy_with_rule("initial", "initial.example.com"); + let created = handle_update_config( + &state, + with_user(Request::new(UpdateConfigRequest { + global: true, + policy: Some(initial.clone()), + ..Default::default() + })), + ) + .await + .expect("MXC may configure global policy before any sandbox exists") + .into_inner(); + assert_eq!(created.version, 1); + + state + .store + .put_message(&test_sandbox( + "mxc-global-policy", + "mxc-global-policy", + ProtoSandboxPolicy::default(), + Vec::new(), + )) + .await + .expect("store MXC sandbox"); + + let replace_error = handle_update_config( + &state, + with_user(Request::new(UpdateConfigRequest { + global: true, + policy: Some(test_policy_with_rule("replacement", "new.example.com")), + ..Default::default() + })), + ) + .await + .expect_err("MXC must reject a global policy replacement for an existing sandbox"); + assert_eq!(replace_error.code(), Code::FailedPrecondition); + + let delete_error = handle_update_config( + &state, + with_user(Request::new(UpdateConfigRequest { + global: true, + setting_key: POLICY_SETTING_KEY.to_string(), + delete_setting: true, + ..Default::default() + })), + ) + .await + .expect_err("MXC must reject deleting global policy for an existing sandbox"); + assert_eq!(delete_error.code(), Code::FailedPrecondition); + + let revisions = state + .store + .list_policies(GLOBAL_POLICY_SANDBOX_ID, 10, 0) + .await + .expect("list global policy revisions"); + assert_eq!(revisions.len(), 1); + assert_eq!(revisions[0].version, 1); + let settings = load_global_settings(state.store.as_ref()) + .await + .expect("load global settings"); + assert_eq!( + decode_policy_from_global_settings(&settings) + .expect("decode global policy") + .expect("global policy remains configured"), + initial + ); + } + + #[tokio::test] + async fn mxc_rejects_all_advisor_policy_mutation_paths_before_persistence() { + let state = test_server_state_with_driver("mxc").await; + let sandbox_id = "mxc-advisor-policy"; + let sandbox_name = "mxc-advisor-policy"; + state + .store + .put_message(&test_sandbox( + sandbox_id, + sandbox_name, + ProtoSandboxPolicy::default(), + Vec::new(), + )) + .await + .expect("store MXC sandbox"); + + let proposal = |name: &str, host: &str| PolicyChunk { + rule_name: name.to_string(), + proposed_rule: Some(NetworkPolicyRule { + name: name.to_string(), + endpoints: vec![NetworkEndpoint { + host: host.to_string(), + port: 443, + ..Default::default() + }], + binaries: vec![NetworkBinary { + path: r"C:\Windows\System32\curl.exe".to_string(), + }], + }), + ..Default::default() + }; + let submitted = handle_submit_policy_analysis( + &state, + with_user(Request::new(SubmitPolicyAnalysisRequest { + name: sandbox_name.to_string(), + analysis_mode: "agent_authored".to_string(), + proposed_chunks: vec![proposal("manual", "manual.example.com")], + ..Default::default() + })), + ) + .await + .expect("store pending proposal") + .into_inner(); + let chunk_id = &submitted.accepted_chunk_ids[0]; + let chunk = state + .store + .get_draft_chunk(chunk_id) + .await + .expect("fetch proposal") + .expect("stored proposal"); + assert_eq!(chunk.status, "pending"); + + let manual_error = handle_approve_draft_chunk( + &state, + with_user(Request::new(ApproveDraftChunkRequest { + name: sandbox_name.to_string(), + chunk_id: chunk.id.clone(), + review_token: chunk.review_token.clone(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + })), + ) + .await + .expect_err("manual approval must not mutate MXC policy"); + assert_eq!(manual_error.code(), Code::FailedPrecondition); + + let bulk_error = handle_approve_all_draft_chunks( + &state, + with_user(Request::new(ApproveAllDraftChunksRequest { + name: sandbox_name.to_string(), + include_security_flagged: true, + approvals: vec![openshell_core::proto::DraftChunkApproval { + chunk_id: chunk.id.clone(), + review_token: chunk.review_token.clone(), + }], + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + })), + ) + .await + .expect_err("bulk approval must not mutate MXC policy"); + assert_eq!(bulk_error.code(), Code::FailedPrecondition); + assert!( + state + .store + .get_latest_policy(sandbox_id) + .await + .expect("policy history lookup") + .is_none() + ); + + seed_sandbox_approval_mode(&state, sandbox_name, "auto").await; + let auto_submitted = handle_submit_policy_analysis( + &state, + with_user(Request::new(SubmitPolicyAnalysisRequest { + name: sandbox_name.to_string(), + analysis_mode: "agent_authored".to_string(), + proposed_chunks: vec![proposal("automatic", "automatic.example.com")], + ..Default::default() + })), + ) + .await + .expect("automatic approval failure leaves the proposal pending") + .into_inner(); + let automatic = state + .store + .get_draft_chunk(&auto_submitted.accepted_chunk_ids[0]) + .await + .expect("fetch automatic proposal") + .expect("stored automatic proposal"); + assert_eq!(automatic.status, "pending"); + assert!(automatic.application_error.contains("delete and recreate")); + assert!( + state + .store + .get_latest_policy(sandbox_id) + .await + .expect("policy history lookup") + .is_none() + ); + + let approved_rule = NetworkPolicyRule::decode(chunk.proposed_rule.as_slice()) + .expect("decode proposed rule"); + let mut approved_policy = ProtoSandboxPolicy::default(); + approved_policy + .network_policies + .insert(chunk.rule_name.clone(), approved_rule); + state + .store + .put_policy_revision( + "mxc-existing-policy", + sandbox_id, + "default", + 1, + &approved_policy.encode_to_vec(), + &deterministic_policy_hash(&approved_policy), + ) + .await + .expect("seed policy applied at sandbox startup"); + state + .store + .update_draft_chunk_status(chunk_id, "approved", Some(current_time_ms()), None) + .await + .expect("mark startup policy proposal approved"); + + let undo_error = handle_undo_draft_chunk( + &state, + with_user(Request::new(UndoDraftChunkRequest { + name: sandbox_name.to_string(), + chunk_id: chunk.id.clone(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + })), + ) + .await + .expect_err("undo must not mutate MXC policy"); + assert_eq!(undo_error.code(), Code::FailedPrecondition); + + let reject_error = handle_reject_draft_chunk( + &state, + with_user(Request::new(RejectDraftChunkRequest { + name: sandbox_name.to_string(), + chunk_id: chunk.id.clone(), + reason: "reject approved proposal".to_string(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + })), + ) + .await + .expect_err("rejecting an approved proposal must not mutate MXC policy"); + assert_eq!(reject_error.code(), Code::FailedPrecondition); + + let stored_chunk = state + .store + .get_draft_chunk(chunk_id) + .await + .expect("fetch approved proposal") + .expect("stored approved proposal"); + assert_eq!(stored_chunk.status, "approved"); + assert_eq!( + state + .store + .get_latest_policy(sandbox_id) + .await + .expect("policy history lookup") + .expect("startup policy revision") + .version, + 1 + ); + } + #[tokio::test] async fn mxc_allows_sandbox_authored_policy_sync() { let state = test_server_state_with_driver("mxc").await; @@ -8693,7 +9022,7 @@ mod tests { .await .expect("store legacy merge base"); - let (version, hash, _) = apply_merge_operations_with_retry( + let (version, hash, _) = apply_merge_operations_with_retry_in_store( &store, &sandbox_id, "default", @@ -11280,7 +11609,7 @@ mod tests { .collect::>(); let error = apply_merge_operations_with_retry( - state.store.as_ref(), + state.as_ref(), "sb-ambiguous-merge", "default", None, @@ -13662,7 +13991,7 @@ mod tests { }, ]; let error = apply_merge_operations_with_retry( - state.store.as_ref(), + state.as_ref(), sandbox_id, "default", Some(&reviewed_policy), @@ -18297,7 +18626,7 @@ mod tests { }]; let (left, right) = tokio::join!( - apply_merge_operations_with_retry( + apply_merge_operations_with_retry_in_store( &store, sandbox_id, "default", @@ -18310,7 +18639,7 @@ mod tests { None, None ), - apply_merge_operations_with_retry( + apply_merge_operations_with_retry_in_store( &store, sandbox_id, "default", From 5e1d3f0c8bb2eedfe97d19da6fa179e11c31650b Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Mon, 21 Sep 2026 23:50:25 -0700 Subject: [PATCH 3/9] fix(mxc): gate composed policy mutations Signed-off-by: Prekshi Vyas --- crates/openshell-server/src/grpc/policy.rs | 64 ++++++++- crates/openshell-server/src/grpc/provider.rs | 136 +++++++++++++++---- crates/openshell-server/src/grpc/sandbox.rs | 93 ++++++++++++- 3 files changed, 262 insertions(+), 31 deletions(-) diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 04e4aa3773..c1792dea3d 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -2323,13 +2323,13 @@ async fn validate_provider_composition_for_existing_sandboxes( } } -fn require_live_policy_update_support(state: &ServerState) -> Result<(), Status> { +pub(super) fn require_live_policy_update_support(state: &ServerState) -> Result<(), Status> { if state.compute.supports_live_policy_updates() { return Ok(()); } Err(Status::failed_precondition(format!( - "compute driver '{}' cannot apply policy updates to an existing sandbox; delete and recreate the sandbox with the requested policy", + "compute driver '{}' cannot apply effective policy changes to an existing sandbox; delete and recreate the sandbox with the requested policy", state.compute.configured_driver_name() ))) } @@ -8655,6 +8655,66 @@ mod tests { ); } + #[tokio::test] + async fn provider_free_create_waits_for_global_policy_transition() { + use openshell_core::proto::{CreateSandboxRequest, SandboxSpec}; + + let state = test_server_state().await; + let guard = state.compute.sandbox_sync_guard().await; + + let update_state = state.clone(); + let update = tokio::spawn(async move { + handle_update_config( + &update_state, + with_user(Request::new(UpdateConfigRequest { + global: true, + policy: Some(test_policy_with_rule("global", "global.example.com")), + ..Default::default() + })), + ) + .await + }); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + assert!( + !update.is_finished(), + "global update should wait for the guard" + ); + + let create_state = state.clone(); + let create = tokio::spawn(async move { + super::super::sandbox::handle_create_sandbox( + &create_state, + authed_request(CreateSandboxRequest { + name: "provider-free-create".to_string(), + spec: Some(SandboxSpec::default()), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + await_main_process_attachment: false, + workload_template_name: String::new(), + }), + ) + .await + }); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + assert!( + !create.is_finished(), + "provider-free create must not overlap a global policy transition" + ); + + drop(guard); + tokio::time::timeout(std::time::Duration::from_secs(5), update) + .await + .expect("global update should finish after guard release") + .expect("join global update") + .expect("global update should succeed"); + tokio::time::timeout(std::time::Duration::from_secs(5), create) + .await + .expect("create should finish after global update") + .expect("join create") + .expect("provider-free create should succeed"); + } + #[tokio::test] async fn mxc_rejects_all_advisor_policy_mutation_paths_before_persistence() { let state = test_server_state_with_driver("mxc").await; diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index e0a83890c4..30a9146756 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2729,16 +2729,15 @@ pub(super) async fn handle_import_provider_profiles( ); diagnostics.extend(validate_profile_set(&profiles)); if !has_errors(&diagnostics) { - diagnostics.extend( - profile_attached_sandbox_diagnostics( - state.store.as_ref(), - &catalog, - &workspace, - &profiles, - "import", - ) - .await?, - ); + let (attached_diagnostics, _) = profile_attached_sandbox_diagnostics( + state.store.as_ref(), + &catalog, + &workspace, + &profiles, + "import", + ) + .await?; + diagnostics.extend(attached_diagnostics); } if has_errors(&diagnostics) { @@ -2843,17 +2842,18 @@ pub(super) async fn handle_update_provider_profiles( severity: "error".to_string(), }); } + let mut affects_attached_sandbox = false; if !has_errors(&diagnostics) { - diagnostics.extend( - profile_attached_sandbox_diagnostics( - state.store.as_ref(), - &catalog, - &workspace, - &profiles, - "update", - ) - .await?, - ); + let (attached_diagnostics, affects_attached) = profile_attached_sandbox_diagnostics( + state.store.as_ref(), + &catalog, + &workspace, + &profiles, + "update", + ) + .await?; + diagnostics.extend(attached_diagnostics); + affects_attached_sandbox = affects_attached; } if has_errors(&diagnostics) { @@ -2864,6 +2864,10 @@ pub(super) async fn handle_update_provider_profiles( })); } + if affects_attached_sandbox { + super::policy::require_live_policy_update_support(state)?; + } + let expected_resource_version = expected_resource_version.unwrap_or_default(); let (_, profile) = profiles .into_iter() @@ -3489,7 +3493,7 @@ async fn profile_attached_sandbox_diagnostics( workspace: &str, profiles: &[(String, ProviderTypeProfile)], operation: &str, -) -> Result, Status> { +) -> Result<(Vec, bool), Status> { let mut candidate_profiles = HashMap::::new(); for (source, profile) in profiles { let Some(id) = normalize_profile_id(&profile.id) else { @@ -3498,7 +3502,7 @@ async fn profile_attached_sandbox_diagnostics( candidate_profiles.insert(id, (source.clone(), profile.clone())); } if candidate_profiles.is_empty() { - return Ok(Vec::new()); + return Ok((Vec::new(), false)); } let is_platform_scope = workspace.is_empty(); @@ -3523,6 +3527,7 @@ async fn profile_attached_sandbox_diagnostics( .await? }; let mut diagnostics = Vec::new(); + let mut affects_attached_sandbox = false; let validate_policy_composition = super::policy::provider_policy_composition_enabled(store).await?; for sandbox in sandboxes { @@ -3650,6 +3655,7 @@ async fn profile_attached_sandbox_diagnostics( if imported_profiles_used.is_empty() { continue; } + affects_attached_sandbox = true; if let Err(err) = validate_dynamic_token_grant_bindings_unambiguous(&bindings) { for (source, profile_id) in &imported_profiles_used { diagnostics.push(ProfileValidationDiagnostic { @@ -3684,7 +3690,7 @@ async fn profile_attached_sandbox_diagnostics( } } - Ok(diagnostics) + Ok((diagnostics, affects_attached_sandbox)) } fn stored_provider_profile_for_workspace( @@ -4966,7 +4972,9 @@ mod tests { use super::*; use crate::auth::identity::{Identity, IdentityProvider}; use crate::auth::principal::{Principal, UserPrincipal}; - use crate::grpc::test_support::{authed_request, test_server_state}; + use crate::grpc::test_support::{ + authed_request, test_server_state, test_server_state_with_driver, + }; use crate::grpc::{MAX_MAP_KEY_LEN, MAX_PROVIDER_TYPE_LEN}; use crate::persistence::test_store; use openshell_core::proto::{ @@ -5435,6 +5443,86 @@ mod tests { ); } + #[tokio::test] + async fn mxc_rejects_attached_provider_profile_update_before_persistence() { + let state = test_server_state_with_driver("mxc").await; + let mut original = custom_profile("custom-api"); + original.endpoints = vec![NetworkEndpoint { + host: "api.before.example".to_string(), + port: 443, + ..Default::default() + }]; + state + .store + .put_message(&stored_provider_profile(original)) + .await + .unwrap(); + state + .store + .put_message(&provider_with_values("work-custom", "custom-api")) + .await + .unwrap(); + state + .store + .put_message(&Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "mxc-profile-sandbox-id".to_string(), + name: "mxc-profile-sandbox".to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + spec: Some(SandboxSpec { + providers: vec!["work-custom".to_string()], + ..Default::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let stored_before = state + .store + .get_message_by_name::("default", "custom-api") + .await + .unwrap() + .unwrap(); + let mut updated = custom_profile("custom-api"); + updated.resource_version = stored_before.metadata.as_ref().unwrap().resource_version; + updated.endpoints = vec![NetworkEndpoint { + host: "api.after.example".to_string(), + port: 443, + ..Default::default() + }]; + + let error = handle_update_provider_profiles( + &state, + authed_request(UpdateProviderProfilesRequest { + profile: Some(ProviderProfileImportItem { + profile: Some(updated), + source: "custom-api.yaml".to_string(), + }), + expected_resource_version: 0, + id: "custom-api".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .expect_err("MXC cannot apply an attached provider profile update"); + assert_eq!(error.code(), Code::FailedPrecondition); + assert!(error.message().contains("delete and recreate")); + + let stored_after = state + .store + .get_message_by_name::("default", "custom-api") + .await + .unwrap() + .unwrap(); + assert_eq!( + stored_after.profile.unwrap().endpoints[0].host, + "api.before.example" + ); + } + #[tokio::test] async fn update_provider_profile_rejects_built_in_and_missing_profiles() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index cc16a141db..dbfcb6c91d 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -394,11 +394,10 @@ async fn handle_create_sandbox_inner( .authorize(&token, &workspace, &subject)?; } - let _sandbox_sync_guard = if spec.providers.is_empty() { - None - } else { - Some(state.compute.sandbox_sync_guard().await) - }; + // Serialize every create with global policy and provider mutations. A + // provider-free create still resolves global policy, so allowing it to + // overlap a global update could persist a sandbox against the old policy. + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; // Validate provider names exist (fail fast). for name in &spec.providers { @@ -1119,6 +1118,14 @@ pub(super) async fn handle_attach_sandbox_provider( .as_ref() .ok_or_else(|| Status::internal("sandbox spec is missing"))?; + if !spec + .providers + .iter() + .any(|name| name == &request.provider_name) + { + super::policy::require_live_policy_update_support(state)?; + } + // Pre-check: fail fast if already at MAX_PROVIDERS limit (avoid spurious CAS conflicts) // Note: This is an optimization; the CAS closure rechecks after dedupe in case of races if spec.providers.len() >= MAX_PROVIDERS @@ -1256,6 +1263,14 @@ pub(super) async fn handle_detach_sandbox_provider( .spec .as_ref() .ok_or_else(|| Status::internal("sandbox spec is missing"))?; + + if spec + .providers + .iter() + .any(|name| name == &request.provider_name) + { + super::policy::require_live_policy_update_support(state)?; + } let mut candidate_spec = spec.clone(); candidate_spec .providers @@ -3876,6 +3891,74 @@ mod tests { assert_eq!(spec.log_level, "debug"); } + #[tokio::test] + async fn mxc_rejects_provider_attachment_and_detachment_before_persistence() { + let state = test_server_state_with_driver("mxc").await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox("attach-target", Vec::new())) + .await + .unwrap(); + + let attach_error = handle_attach_sandbox_provider( + &state, + authed_request(AttachSandboxProviderRequest { + sandbox_name: "attach-target".to_string(), + provider_name: "work-github".to_string(), + expected_resource_version: 0, + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + }), + ) + .await + .expect_err("MXC cannot apply provider attachment to an existing sandbox"); + assert_eq!(attach_error.code(), tonic::Code::FailedPrecondition); + assert!(attach_error.message().contains("delete and recreate")); + let attach_target = state + .store + .get_message_by_name::("default", "attach-target") + .await + .unwrap() + .unwrap(); + assert!(attach_target.spec.unwrap().providers.is_empty()); + + state + .store + .put_message(&test_sandbox( + "detach-target", + vec!["work-github".to_string()], + )) + .await + .unwrap(); + let detach_error = handle_detach_sandbox_provider( + &state, + authed_request(DetachSandboxProviderRequest { + sandbox_name: "detach-target".to_string(), + provider_name: "work-github".to_string(), + expected_resource_version: 0, + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + }), + ) + .await + .expect_err("MXC cannot apply provider detachment to an existing sandbox"); + assert_eq!(detach_error.code(), tonic::Code::FailedPrecondition); + assert!(detach_error.message().contains("delete and recreate")); + let detach_target = state + .store + .get_message_by_name::("default", "detach-target") + .await + .unwrap() + .unwrap(); + assert_eq!( + detach_target.spec.unwrap().providers, + vec!["work-github".to_string()] + ); + } + #[tokio::test] async fn attach_sandbox_provider_uses_configured_provider_profile_sources() { let state = test_server_state_with_user_only_github_profile().await; From b91c58b3416d7318f47773b7558352b17f982e65 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 00:40:10 -0700 Subject: [PATCH 4/9] fix(ci): satisfy provider update lint --- crates/openshell-server/src/grpc/provider.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 30a9146756..248f237d21 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2842,8 +2842,7 @@ pub(super) async fn handle_update_provider_profiles( severity: "error".to_string(), }); } - let mut affects_attached_sandbox = false; - if !has_errors(&diagnostics) { + let affects_attached_sandbox = if !has_errors(&diagnostics) { let (attached_diagnostics, affects_attached) = profile_attached_sandbox_diagnostics( state.store.as_ref(), &catalog, @@ -2853,8 +2852,10 @@ pub(super) async fn handle_update_provider_profiles( ) .await?; diagnostics.extend(attached_diagnostics); - affects_attached_sandbox = affects_attached; - } + affects_attached + } else { + false + }; if has_errors(&diagnostics) { return Ok(Response::new(UpdateProviderProfilesResponse { From 95c64487adf0741dc2185dc091d46e03db5f518d Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 00:46:14 -0700 Subject: [PATCH 5/9] fix(ci): order provider validation branches --- crates/openshell-server/src/grpc/provider.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 248f237d21..a226680171 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2842,7 +2842,9 @@ pub(super) async fn handle_update_provider_profiles( severity: "error".to_string(), }); } - let affects_attached_sandbox = if !has_errors(&diagnostics) { + let affects_attached_sandbox = if has_errors(&diagnostics) { + false + } else { let (attached_diagnostics, affects_attached) = profile_attached_sandbox_diagnostics( state.store.as_ref(), &catalog, @@ -2853,8 +2855,6 @@ pub(super) async fn handle_update_provider_profiles( .await?; diagnostics.extend(attached_diagnostics); affects_attached - } else { - false }; if has_errors(&diagnostics) { From 2f7dd18272c4a2c2532784a145c23d4ab2873b84 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 01:03:55 -0700 Subject: [PATCH 6/9] test(mxc): make policy synchronization deterministic --- crates/openshell-server/src/grpc/policy.rs | 31 +-------------------- crates/openshell-server/src/grpc/sandbox.rs | 20 ++++--------- 2 files changed, 7 insertions(+), 44 deletions(-) diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index c1792dea3d..3164378c34 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -8656,9 +8656,7 @@ mod tests { } #[tokio::test] - async fn provider_free_create_waits_for_global_policy_transition() { - use openshell_core::proto::{CreateSandboxRequest, SandboxSpec}; - + async fn global_policy_transition_waits_for_sandbox_sync_guard() { let state = test_server_state().await; let guard = state.compute.sandbox_sync_guard().await; @@ -8680,39 +8678,12 @@ mod tests { "global update should wait for the guard" ); - let create_state = state.clone(); - let create = tokio::spawn(async move { - super::super::sandbox::handle_create_sandbox( - &create_state, - authed_request(CreateSandboxRequest { - name: "provider-free-create".to_string(), - spec: Some(SandboxSpec::default()), - labels: HashMap::new(), - annotations: HashMap::new(), - workspace_scope: Some(openshell_core::proto::workspace_selector("default")), - await_main_process_attachment: false, - workload_template_name: String::new(), - }), - ) - .await - }); - tokio::time::sleep(std::time::Duration::from_millis(20)).await; - assert!( - !create.is_finished(), - "provider-free create must not overlap a global policy transition" - ); - drop(guard); tokio::time::timeout(std::time::Duration::from_secs(5), update) .await .expect("global update should finish after guard release") .expect("join global update") .expect("global update should succeed"); - tokio::time::timeout(std::time::Duration::from_secs(5), create) - .await - .expect("create should finish after global update") - .expect("join create") - .expect("provider-free create should succeed"); } #[tokio::test] diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index dbfcb6c91d..f402bbb4b5 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -4980,13 +4980,8 @@ mod tests { } #[tokio::test] - async fn create_sandbox_with_providers_waits_for_sandbox_sync_guard() { + async fn provider_free_create_waits_for_sandbox_sync_guard() { let state = test_server_state().await; - state - .store - .put_message(&test_provider("work-github", "github")) - .await - .unwrap(); let guard = state.compute.sandbox_sync_guard().await; let task_state = state.clone(); @@ -4995,10 +4990,7 @@ mod tests { &task_state, authed_request(CreateSandboxRequest { name: "guarded-create".to_string(), - spec: Some(SandboxSpec { - providers: vec!["work-github".to_string()], - ..Default::default() - }), + spec: Some(SandboxSpec::default()), labels: HashMap::new(), annotations: HashMap::new(), workspace_scope: Some(openshell_core::proto::workspace_selector("default")), @@ -5012,7 +5004,7 @@ mod tests { tokio::time::sleep(std::time::Duration::from_millis(20)).await; assert!( !task.is_finished(), - "sandbox create with initial providers should wait for sandbox sync guard" + "provider-free sandbox create should wait for sandbox sync guard" ); drop(guard); @@ -5022,9 +5014,9 @@ mod tests { .expect("join create task") .expect("create should succeed") .into_inner(); - assert_eq!( - response.sandbox.unwrap().spec.unwrap().providers, - vec!["work-github".to_string()] + assert!( + response.sandbox.unwrap().spec.unwrap().providers.is_empty(), + "the synchronization test must exercise a provider-free create" ); } From dbf328befdfd502dd4d4307477b2ac50dfd3f932 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 13:07:22 -0700 Subject: [PATCH 7/9] fix(server): scope MXC policy synchronization --- architecture/compute-runtimes.md | 10 ++++ crates/openshell-server/src/grpc/sandbox.rs | 53 ++++++++++++++++++--- docs/sandboxes/policies.mdx | 2 +- skills/openshell-cli/SKILL.md | 4 +- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 8f7c5e5080..f970847da1 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -118,6 +118,16 @@ be supplied by a gateway-global policy because it is applied at startup. When a global dynamic policy is active, effective-policy reads retain the UI block from the sandbox's creation policy. +Live policy updates are also capability-negotiated. A driver that reports +`supports_live_policy_updates = true` must apply every gateway mutation that can +change an existing sandbox's effective dynamic policy. A driver that reports +`false` causes the gateway to reject those mutations before persistence and to +serialize sandbox creation with global policy and provider changes. Rejected +mutations include direct replacement or merge, global policy changes while a +sandbox exists, policy-advisor approval or undo, provider attach or detach, and +updates to a provider profile used by a running sandbox. An omitted capability +retains the legacy `true` behavior for compatibility with existing drivers. + The gateway records driver identity and version from the startup capability response. Elevated gateway info reports that initialized driver snapshot instead of re-querying drivers on each request. diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index f402bbb4b5..f5d0a83f59 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -394,10 +394,15 @@ async fn handle_create_sandbox_inner( .authorize(&token, &workspace, &subject)?; } - // Serialize every create with global policy and provider mutations. A - // provider-free create still resolves global policy, so allowing it to - // overlap a global update could persist a sandbox against the old policy. - let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; + // Drivers without live policy updates need an atomic boundary between + // create-time policy resolution and mutations that affect existing sandboxes. + // Capable drivers retain the existing concurrent-create behavior and do not + // need the MXC-specific serialization around their potentially slow RPCs. + let _sandbox_sync_guard = if state.compute.supports_live_policy_updates() { + None + } else { + Some(state.compute.sandbox_sync_guard().await) + }; // Validate provider names exist (fail fast). for name in &spec.providers { @@ -4980,8 +4985,8 @@ mod tests { } #[tokio::test] - async fn provider_free_create_waits_for_sandbox_sync_guard() { - let state = test_server_state().await; + async fn mxc_provider_free_create_waits_for_sandbox_sync_guard() { + let state = test_server_state_with_driver("mxc").await; let guard = state.compute.sandbox_sync_guard().await; let task_state = state.clone(); @@ -5020,6 +5025,42 @@ mod tests { ); } + #[tokio::test] + async fn live_update_driver_create_does_not_wait_for_sandbox_sync_guard() { + let state = test_server_state().await; + + let guard = state.compute.sandbox_sync_guard().await; + let task_state = state.clone(); + let task = tokio::spawn(async move { + handle_create_sandbox( + &task_state, + authed_request(CreateSandboxRequest { + name: "concurrent-create".to_string(), + spec: Some(SandboxSpec::default()), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + await_main_process_attachment: false, + workload_template_name: String::new(), + }), + ) + .await + }); + + let response = tokio::time::timeout(std::time::Duration::from_secs(5), task) + .await + .expect("live-update-capable create must not wait for the sync guard") + .expect("join create task") + .expect("create should succeed") + .into_inner(); + drop(guard); + + assert!( + response.sandbox.unwrap().spec.unwrap().providers.is_empty(), + "the concurrency test must exercise a provider-free create" + ); + } + #[tokio::test] async fn sandbox_template_handlers_create_get_list_and_delete_workspace_resource() { let state = test_server_state().await; diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 1d3685bf39..c83cb66f00 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -12,7 +12,7 @@ Use this page to apply and iterate policy changes on running sandboxes. For a fu ## Policy Structure -A policy has static sections `filesystem_policy`, `landlock`, `process`, and `ui` that are locked at sandbox creation. `network_policies` and `network_middlewares` are dynamic schema sections, but live updates work only on compute drivers that support runtime policy reload. MXC rejects live policy replacement and merge updates, so recreate the sandbox there. +A policy has static sections `filesystem_policy`, `landlock`, `process`, and `ui` that are locked at sandbox creation. `network_policies` and `network_middlewares` are dynamic schema sections, but live updates work only on compute drivers that support runtime policy reload. MXC rejects every operation that would change a running sandbox's effective policy: direct replacement or merge, global policy changes, policy-advisor approval or undo, provider attach or detach, and updates to an attached provider's profile. The gateway rejects these operations before saving anything; delete and recreate the MXC sandbox instead. ```yaml wordWrap showLineNumbers={false} version: 1 diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index b68cb3ca58..b44b748e50 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -443,7 +443,7 @@ the operation that removes retained state. This is the most important multi-step workflow. It enables a tight feedback cycle where sandbox policy is refined based on observed activity. -**Key concept**: Policies have static fields (immutable after creation: `filesystem_policy`, `landlock`, `process`, `ui`) and two dynamic fields: `network_policies` and `network_middlewares`. Both dynamic fields can be updated without recreating the sandbox when the selected compute driver supports live policy updates. MXC rejects live policy replacement and merge updates; delete and recreate an MXC sandbox instead. UI capabilities are enforced only by a configured driver/backend advertising complete UI-policy support. Today that is the MXC driver's OpenShell `process_container` backend, which emits MXC's `processcontainer` containment value. `isolation_session` and non-Windows drivers reject any explicit UI section before provisioning; omit it to preserve their existing behavior. +**Key concept**: Policies have static fields (immutable after creation: `filesystem_policy`, `landlock`, `process`, `ui`) and two dynamic fields: `network_policies` and `network_middlewares`. Both dynamic fields can be updated without recreating the sandbox when the selected compute driver supports live policy updates. MXC rejects every operation that would change a running sandbox's effective policy: direct replacement or merge, global policy changes, policy-advisor approval or undo, provider attach or detach, and updates to an attached provider's profile. The gateway rejects these operations before saving anything; delete and recreate the MXC sandbox instead. UI capabilities are enforced only by a configured driver/backend advertising complete UI-policy support. Today that is the MXC driver's OpenShell `process_container` backend, which emits MXC's `processcontainer` containment value. `isolation_session` and non-Windows drivers reject any explicit UI section before provisioning; omit it to preserve their existing behavior. An endpoint with omitted `protocol` retains explicit-proxy behavior. Explicit `protocol: tcp` requests policy DNS and transparent TCP and currently requires @@ -513,7 +513,7 @@ Edit `current-policy.yaml` to allow the blocked actions. **For policy content au - Binary matching patterns - Ordered `network_middlewares`, host selection, HTTP and WebSocket bindings, and `fail_open` or `fail_closed` behavior -`network_policies` and `network_middlewares` can be modified at runtime when the selected compute driver supports live policy updates. Use `--wait` to verify that the active runtime loaded the revision; do not infer enforcement from the gateway accepting the update. MXC rejects live policy replacement and merge updates; delete and recreate an MXC sandbox instead. If `filesystem_policy`, `landlock`, `process`, or `ui` need changes, the sandbox must be recreated. Built-in middleware such as `openshell/regex` needs no gateway registration. An operator-run middleware must already be registered under `[[openshell.supervisor.middleware]]`; changing that static registration requires a gateway restart. +`network_policies` and `network_middlewares` can be modified at runtime when the selected compute driver supports live policy updates. Use `--wait` to verify that the active runtime loaded the revision; do not infer enforcement from the gateway accepting the update. MXC rejects every operation that would change a running sandbox's effective policy: direct replacement or merge, global policy changes, policy-advisor approval or undo, provider attach or detach, and updates to an attached provider's profile. The gateway rejects these operations before saving anything; delete and recreate the MXC sandbox instead. If `filesystem_policy`, `landlock`, `process`, or `ui` need changes, the sandbox must be recreated. Built-in middleware such as `openshell/regex` needs no gateway registration. An operator-run middleware must already be registered under `[[openshell.supervisor.middleware]]`; changing that static registration requires a gateway restart. Middleware can inspect parsed HTTP request bodies and complete client-to-upstream WebSocket text messages over both `ws://` and `wss://` when the implementation advertises the matching binding. The built-in `openshell/regex` advertises both bindings and applies its fixed patterns to UTF-8 text. A host-matched HTTP-only attachment can inspect the upgrade GET but does not join the WebSocket chain; look for `binding_not_selected` coverage. Binary messages pass under both `on_error` modes and active stages emit `unsupported_message_type` coverage; upstream-to-client messages remain uninspected. A broken fail-open WebSocket stage is disabled for the rest of that connection; inspect sandbox OCSF logs for `openshell.middleware.websocket_stage_disabled`. From b17199ccdf81b9d37be6f349ba9207dce88bf3fc Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 13:42:53 -0700 Subject: [PATCH 8/9] fix(server): serialize provider-backed sandbox creation --- crates/openshell-server/src/grpc/sandbox.rs | 64 ++++++++++++++++++--- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index f5d0a83f59..bff7192f69 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -396,13 +396,14 @@ async fn handle_create_sandbox_inner( // Drivers without live policy updates need an atomic boundary between // create-time policy resolution and mutations that affect existing sandboxes. - // Capable drivers retain the existing concurrent-create behavior and do not - // need the MXC-specific serialization around their potentially slow RPCs. - let _sandbox_sync_guard = if state.compute.supports_live_policy_updates() { - None - } else { - Some(state.compute.sandbox_sync_guard().await) - }; + // Provider-backed creates also serialize with profile mutation so the initial + // policy snapshot cannot miss a concurrent profile update before persistence. + let _sandbox_sync_guard = + if !state.compute.supports_live_policy_updates() || !spec.providers.is_empty() { + Some(state.compute.sandbox_sync_guard().await) + } else { + None + }; // Validate provider names exist (fail fast). for name in &spec.providers { @@ -5061,6 +5062,55 @@ mod tests { ); } + #[tokio::test] + async fn create_sandbox_with_providers_waits_for_sandbox_sync_guard() { + let state = test_server_state().await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + + let guard = state.compute.sandbox_sync_guard().await; + let task_state = state.clone(); + let task = tokio::spawn(async move { + handle_create_sandbox( + &task_state, + authed_request(CreateSandboxRequest { + name: "provider-backed-create".to_string(), + spec: Some(SandboxSpec { + providers: vec!["work-github".to_string()], + ..Default::default() + }), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace_scope: Some(openshell_core::proto::workspace_selector("default")), + await_main_process_attachment: false, + workload_template_name: String::new(), + }), + ) + .await + }); + + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + assert!( + !task.is_finished(), + "sandbox create with initial providers should wait for sandbox sync guard" + ); + drop(guard); + + let response = tokio::time::timeout(std::time::Duration::from_secs(5), task) + .await + .expect("create should finish after guard release") + .expect("join create task") + .expect("create should succeed") + .into_inner(); + assert_eq!( + response.sandbox.unwrap().spec.unwrap().providers, + vec!["work-github".to_string()] + ); + } + #[tokio::test] async fn sandbox_template_handlers_create_get_list_and_delete_workspace_resource() { let state = test_server_state().await; From 1b4df5d83b4b936ccefbc76b5e2c9b1f8682c5f2 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 22 Sep 2026 13:59:50 -0700 Subject: [PATCH 9/9] test(server): use valid provider create fixture name --- crates/openshell-server/src/grpc/sandbox.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index bff7192f69..73bdd75299 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -5077,7 +5077,7 @@ mod tests { handle_create_sandbox( &task_state, authed_request(CreateSandboxRequest { - name: "provider-backed-create".to_string(), + name: "provider-create".to_string(), spec: Some(SandboxSpec { providers: vec!["work-github".to_string()], ..Default::default()