From 592e709f0d97d3d5abc6f783cf80caaf5f7db02b Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:28:37 -0700 Subject: [PATCH 1/3] feat(providers): support profile updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- crates/openshell-cli/src/main.rs | 40 +++ crates/openshell-cli/src/run.rs | 45 ++- .../tests/ensure_providers_integration.rs | 7 + .../openshell-cli/tests/mtls_integration.rs | 7 + .../tests/provider_commands_integration.rs | 76 +++++ .../sandbox_create_lifecycle_integration.rs | 7 + .../sandbox_name_fallback_integration.rs | 7 + crates/openshell-server/src/grpc/mod.rs | 11 +- crates/openshell-server/src/grpc/policy.rs | 146 ++++++++- crates/openshell-server/src/grpc/provider.rs | 281 +++++++++++++++++- crates/openshell-server/tests/common/mod.rs | 7 + .../tests/supervisor_relay_integration.rs | 7 + docs/sandboxes/manage-providers.mdx | 12 + docs/sandboxes/providers-v2.mdx | 22 +- proto/openshell.proto | 16 + 15 files changed, 674 insertions(+), 17 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 9b80f1914..6c2c4c365 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -930,6 +930,18 @@ enum ProviderProfileCommands { from: Option, }, + /// Update existing custom provider profiles from a file or directory. + #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + Update { + /// Profile file to update. + #[arg(short = 'f', long = "file", value_hint = ValueHint::FilePath)] + file: Option, + + /// Directory containing profile files to update. + #[arg(long = "from", value_hint = ValueHint::DirPath)] + from: Option, + }, + /// Validate provider profile files without registering them. #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Lint { @@ -2927,6 +2939,15 @@ async fn main() -> Result<()> { ) .await?; } + ProviderProfileCommands::Update { file, from } => { + run::provider_profile_update( + endpoint, + file.as_deref(), + from.as_deref(), + &tls, + ) + .await?; + } ProviderProfileCommands::Lint { file, from } => { run::provider_profile_lint( endpoint, @@ -3765,6 +3786,25 @@ mod tests { }) )); + let update = Cli::try_parse_from([ + "openshell", + "provider", + "profile", + "update", + "-f", + "./profiles/custom-api.yaml", + ]) + .expect("provider profile update should parse"); + assert!(matches!( + update.command, + Some(Commands::Provider { + command: Some(ProviderCommands::Profile(ProviderProfileCommands::Update { + file: Some(_), + .. + })) + }) + )); + let delete = Cli::try_parse_from(["openshell", "provider", "profile", "delete", "custom-api"]) .expect("provider profile delete should parse"); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 6f5520755..4f3f1e2f8 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -51,8 +51,8 @@ use openshell_core::proto::{ RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, ServiceEndpointResponse, SetClusterInferenceRequest, SettingScope, SettingValue, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, - UpdateConfigRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, - setting_value, tcp_forward_init, + UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, + exec_sandbox_event, setting_value, tcp_forward_init, }; use openshell_core::settings::{self, SettingValueKind}; use openshell_core::{ObjectId, ObjectName}; @@ -5019,6 +5019,47 @@ pub async fn provider_profile_import( Err(miette!("provider profile import failed")) } +pub async fn provider_profile_update( + server: &str, + file: Option<&Path>, + from: Option<&Path>, + tls: &TlsOptions, +) -> Result<()> { + let (items, mut diagnostics) = load_profile_import_items(file, from)?; + if items.is_empty() && diagnostics.is_empty() { + return Err(miette!("no provider profile files found")); + } + if profile_diagnostics_have_errors(&diagnostics) { + print_profile_diagnostics(&diagnostics); + return Err(miette!("provider profile update failed")); + } + + let mut client = grpc_client(server, tls).await?; + if !items.is_empty() { + let response = client + .update_provider_profiles(UpdateProviderProfilesRequest { profiles: items }) + .await + .into_diagnostic()? + .into_inner(); + diagnostics.extend(response.diagnostics); + if response.updated { + println!( + "Updated {} provider profile{}.", + response.profiles.len(), + if response.profiles.len() == 1 { + "" + } else { + "s" + } + ); + return Ok(()); + } + } + + print_profile_diagnostics(&diagnostics); + Err(miette!("provider profile update failed")) +} + pub async fn provider_profile_lint( server: &str, file: Option<&Path>, diff --git a/crates/openshell-cli/tests/ensure_providers_integration.rs b/crates/openshell-cli/tests/ensure_providers_integration.rs index ea2d5a465..7bf8612b4 100644 --- a/crates/openshell-cli/tests/ensure_providers_integration.rs +++ b/crates/openshell-cli/tests/ensure_providers_integration.rs @@ -276,6 +276,13 @@ impl OpenShell for TestOpenShell { Err(Status::unimplemented("not implemented in test")) } + async fn update_provider_profiles( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/mtls_integration.rs b/crates/openshell-cli/tests/mtls_integration.rs index 7cb9e1e76..28a4e6c9c 100644 --- a/crates/openshell-cli/tests/mtls_integration.rs +++ b/crates/openshell-cli/tests/mtls_integration.rs @@ -226,6 +226,13 @@ impl OpenShell for TestOpenShell { Err(Status::unimplemented("not implemented in test")) } + async fn update_provider_profiles( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 1450b99d4..920bed6d9 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -468,6 +468,49 @@ impl OpenShell for TestOpenShell { )) } + async fn update_provider_profiles( + &self, + request: tonic::Request, + ) -> Result, Status> { + let mut profiles = self.state.profiles.lock().await; + let updates = request + .into_inner() + .profiles + .into_iter() + .filter_map(|item| item.profile) + .collect::>(); + for profile in &updates { + if !profiles.contains_key(&profile.id) { + return Ok(Response::new( + openshell_core::proto::UpdateProviderProfilesResponse { + diagnostics: vec![openshell_core::proto::ProviderProfileDiagnostic { + source: profile.id.clone(), + profile_id: profile.id.clone(), + field: "id".to_string(), + message: format!( + "custom provider profile '{}' does not exist", + profile.id + ), + severity: "error".to_string(), + }], + profiles: Vec::new(), + updated: false, + }, + )); + } + } + for profile in &updates { + profiles.insert(profile.id.clone(), profile.clone()); + } + Ok(Response::new( + openshell_core::proto::UpdateProviderProfilesResponse { + diagnostics: Vec::new(), + profiles: updates, + updated: true, + }, + )) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, @@ -1366,6 +1409,39 @@ binaries: [/usr/bin/custom] run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, &ts.tls) .await .expect("profile import"); + std::fs::write( + &profile_path, + r" +id: custom-api +display_name: Custom API Updated +category: other +credentials: + - name: api_key + env_vars: [CUSTOM_API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.updated.example + port: 443 +binaries: [/usr/bin/custom] +", + ) + .unwrap(); + run::provider_profile_update(&ts.endpoint, Some(&profile_path), None, &ts.tls) + .await + .expect("profile update"); + assert_eq!( + ts.state + .profiles + .lock() + .await + .get("custom-api") + .and_then(|profile| profile.endpoints.first()) + .map(|endpoint| endpoint.host.as_str()), + Some("api.updated.example") + ); run::provider_profile_export(&ts.endpoint, "custom-api", "yaml", &ts.tls) .await .expect("profile export"); diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 7061614cb..254f1fd31 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -277,6 +277,13 @@ impl OpenShell for TestOpenShell { Err(Status::unimplemented("not implemented in test")) } + async fn update_provider_profiles( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs index 5e753eff9..d4052ff68 100644 --- a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs +++ b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs @@ -262,6 +262,13 @@ impl OpenShell for TestOpenShell { Err(Status::unimplemented("not implemented in test")) } + async fn update_provider_profiles( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 5947bb334..fe2eb331c 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -40,7 +40,8 @@ use openshell_core::proto::{ RotateProviderCredentialRequest, RotateProviderCredentialResponse, SandboxResponse, SandboxStreamEvent, ServiceEndpointResponse, ServiceStatus, SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, SupervisorMessage, TcpForwardFrame, UndoDraftChunkRequest, - UndoDraftChunkResponse, UpdateConfigRequest, UpdateConfigResponse, UpdateProviderRequest, + UndoDraftChunkResponse, UpdateConfigRequest, UpdateConfigResponse, + UpdateProviderProfilesRequest, UpdateProviderProfilesResponse, UpdateProviderRequest, WatchSandboxRequest, open_shell_server::OpenShell, }; use serde::{Deserialize, Serialize}; @@ -404,6 +405,14 @@ impl OpenShell for OpenShellService { provider::handle_import_provider_profiles(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] + async fn update_provider_profiles( + &self, + request: Request, + ) -> Result, Status> { + provider::handle_update_provider_profiles(&self.state, request).await + } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn lint_provider_profiles( &self, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 2e2210f44..455619531 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -4798,6 +4798,129 @@ mod tests { ); } + #[tokio::test] + async fn sandbox_config_uses_updated_custom_provider_profile_without_rewriting_provider() { + use crate::grpc::provider::handle_update_provider_profiles; + use openshell_core::proto::{ + ProviderProfile, ProviderProfileCategory, ProviderProfileImportItem, + StoredProviderProfile, UpdateProviderProfilesRequest, + }; + + fn stored_profile(host: &str) -> StoredProviderProfile { + StoredProviderProfile { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "profile-custom-policy".to_string(), + name: "custom-policy".to_string(), + created_at_ms: 1_000_000, + labels: HashMap::new(), + resource_version: 0, + }), + profile: Some(ProviderProfile { + id: "custom-policy".to_string(), + display_name: "Custom Policy".to_string(), + description: String::new(), + category: ProviderProfileCategory::Other as i32, + credentials: Vec::new(), + endpoints: vec![NetworkEndpoint { + host: host.to_string(), + port: 443, + ..Default::default() + }], + binaries: Vec::new(), + inference_capable: false, + discovery: None, + }), + } + } + + let state = test_server_state().await; + enable_providers_v2(&state).await; + state + .store + .put_message(&stored_profile("api.before.example")) + .await + .unwrap(); + let provider = test_provider("work-custom", "custom-policy"); + state.store.put_message(&provider).await.unwrap(); + state + .store + .put_message(&test_sandbox( + "sb-custom-policy-update", + "custom-policy-update", + test_policy_with_rule("sandbox_only", "sandbox.example.com"), + vec!["work-custom".to_string()], + )) + .await + .unwrap(); + + let before_policy = get_sandbox_policy(&state, "sb-custom-policy-update").await; + assert!( + before_policy.network_policies["_provider_work_custom"] + .endpoints + .iter() + .any(|endpoint| endpoint.host == "api.before.example") + ); + + let updated_profile = stored_profile("api.after.example").profile.unwrap(); + let response = handle_update_provider_profiles( + &state, + with_user(Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(updated_profile), + source: "custom-policy.yaml".to_string(), + }], + })), + ) + .await + .unwrap() + .into_inner(); + assert!(response.updated); + + let after_policy = get_sandbox_policy(&state, "sb-custom-policy-update").await; + let provider_rule = &after_policy.network_policies["_provider_work_custom"]; + assert!( + provider_rule + .endpoints + .iter() + .any(|endpoint| endpoint.host == "api.after.example") + ); + assert!( + !provider_rule + .endpoints + .iter() + .any(|endpoint| endpoint.host == "api.before.example") + ); + + let persisted_provider: Provider = state + .store + .get_message_by_name("work-custom") + .await + .unwrap() + .unwrap(); + assert_eq!(persisted_provider.r#type, provider.r#type); + assert_eq!(persisted_provider.credentials, provider.credentials); + + let persisted_policy = state + .store + .get_latest_policy("sb-custom-policy-update") + .await + .unwrap() + .expect("sandbox policy should be lazily backfilled"); + let persisted_policy = + ProtoSandboxPolicy::decode(persisted_policy.policy_payload.as_slice()) + .expect("persisted sandbox policy should decode"); + assert!( + persisted_policy + .network_policies + .contains_key("sandbox_only") + ); + assert!( + !persisted_policy + .network_policies + .contains_key("_provider_work_custom") + ); + } + #[tokio::test] async fn sandbox_config_preserves_overlapping_user_and_provider_rules() { let state = test_server_state().await; @@ -4946,9 +5069,11 @@ mod tests { #[tokio::test] async fn provider_env_revision_changes_when_custom_profile_token_grant_changes() { + use crate::grpc::provider::handle_update_provider_profiles; use openshell_core::proto::{ ProviderCredentialTokenGrant, ProviderProfile, ProviderProfileCategory, - ProviderProfileCredential, StoredProviderProfile, + ProviderProfileCredential, ProviderProfileImportItem, StoredProviderProfile, + UpdateProviderProfilesRequest, }; use std::time::Duration; @@ -5007,13 +5132,20 @@ mod tests { .unwrap(); tokio::time::sleep(Duration::from_millis(2)).await; - state - .store - .put_message(&token_grant_profile( - "https://auth.example.com/rotated-token", - )) - .await + let rotated_profile = token_grant_profile("https://auth.example.com/rotated-token") + .profile .unwrap(); + handle_update_provider_profiles( + &state, + with_user(Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(rotated_profile), + source: "custom-token.yaml".to_string(), + }], + })), + ) + .await + .unwrap(); let second = compute_provider_env_revision(state.store.as_ref(), &["work-custom-token".to_string()]) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 641118206..81c6a100e 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1181,7 +1181,8 @@ use openshell_core::proto::{ ListProviderProfilesResponse, ListProvidersRequest, ListProvidersResponse, ProviderCredentialRefreshStrategy, ProviderProfileDiagnostic, ProviderProfileImportItem, ProviderProfileResponse, ProviderResponse, RotateProviderCredentialRequest, - RotateProviderCredentialResponse, StoredProviderProfile, UpdateProviderRequest, + RotateProviderCredentialResponse, StoredProviderProfile, UpdateProviderProfilesRequest, + UpdateProviderProfilesResponse, UpdateProviderRequest, }; use openshell_providers::{ CredentialRefreshProfile, ProfileValidationDiagnostic, ProviderTypeProfile, default_profiles, @@ -1302,7 +1303,7 @@ pub(super) async fn handle_import_provider_profiles( diagnostics.extend(validate_profile_set(&profiles)); if !has_errors(&diagnostics) { diagnostics.extend( - profile_import_attached_sandbox_diagnostics(state.store.as_ref(), &profiles).await?, + profile_attached_sandbox_diagnostics(state.store.as_ref(), &profiles, "import").await?, ); } @@ -1339,6 +1340,77 @@ pub(super) async fn handle_import_provider_profiles( })) } +pub(super) async fn handle_update_provider_profiles( + state: &Arc, + request: Request, +) -> Result, Status> { + let request = request.into_inner(); + let (profiles, mut diagnostics) = profiles_from_import_items(&request.profiles); + add_empty_profile_set_diagnostic(&profiles, &mut diagnostics); + diagnostics.extend(profile_update_target_diagnostics(state.store.as_ref(), &profiles).await?); + diagnostics.extend(validate_profile_set(&profiles)); + if !has_errors(&diagnostics) { + diagnostics.extend( + profile_attached_sandbox_diagnostics(state.store.as_ref(), &profiles, "update").await?, + ); + } + + if has_errors(&diagnostics) { + return Ok(Response::new(UpdateProviderProfilesResponse { + diagnostics: diagnostics.into_iter().map(proto_diagnostic).collect(), + profiles: Vec::new(), + updated: false, + })); + } + + let mut updated = Vec::with_capacity(profiles.len()); + for (_, profile) in profiles { + let id = normalize_profile_id(&profile.id) + .ok_or_else(|| Status::internal("validated provider profile id is invalid"))?; + let mut stored = state + .store + .get_message_by_name::(&id) + .await + .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? + .ok_or_else(|| Status::not_found("provider profile not found"))?; + let current_version = stored + .metadata + .as_ref() + .map_or(0, |metadata| metadata.resource_version); + stored.profile = Some(profile.to_proto()); + let labels_json = stored + .object_labels() + .filter(|labels| !labels.is_empty()) + .map(|labels| { + serde_json::to_string(&labels) + .map_err(|e| Status::internal(format!("serialize labels failed: {e}"))) + }) + .transpose()?; + let result = state + .store + .put_if( + StoredProviderProfile::object_type(), + stored.object_id(), + stored.object_name(), + &stored.encode_to_vec(), + labels_json.as_deref(), + WriteCondition::MatchResourceVersion(current_version), + ) + .await + .map_err(|e| super::persistence_error_to_status(e, "update provider profile"))?; + if let Some(metadata) = stored.metadata.as_mut() { + metadata.resource_version = result.resource_version; + } + updated.push(stored.profile.unwrap_or_default()); + } + + Ok(Response::new(UpdateProviderProfilesResponse { + diagnostics: Vec::new(), + profiles: updated, + updated: true, + })) +} + pub(super) async fn handle_lint_provider_profiles( state: &Arc, request: Request, @@ -1575,9 +1647,47 @@ async fn profile_conflict_diagnostics( Ok(diagnostics) } -async fn profile_import_attached_sandbox_diagnostics( +async fn profile_update_target_diagnostics( store: &Store, profiles: &[(String, ProviderTypeProfile)], +) -> Result, Status> { + let mut diagnostics = Vec::new(); + for (source, profile) in profiles { + let Some(id) = normalize_profile_id(&profile.id) else { + continue; + }; + if get_default_profile(&id).is_some() { + diagnostics.push(ProfileValidationDiagnostic { + source: source.clone(), + profile_id: id.clone(), + field: "id".to_string(), + message: format!("provider profile '{id}' is built-in and cannot be updated"), + severity: "error".to_string(), + }); + continue; + } + if store + .get_message_by_name::(&id) + .await + .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? + .is_none() + { + diagnostics.push(ProfileValidationDiagnostic { + source: source.clone(), + profile_id: id.clone(), + field: "id".to_string(), + message: format!("custom provider profile '{id}' does not exist"), + severity: "error".to_string(), + }); + } + } + Ok(diagnostics) +} + +async fn profile_attached_sandbox_diagnostics( + store: &Store, + profiles: &[(String, ProviderTypeProfile)], + operation: &str, ) -> Result, Status> { let mut candidate_profiles = std::collections::HashMap::::new(); @@ -1640,7 +1750,7 @@ async fn profile_import_attached_sandbox_diagnostics( profile_id: profile_id.clone(), field: "credentials.token_grant.audience_overrides".to_string(), message: format!( - "import would create ambiguous dynamic token grants on sandbox '{sandbox_name}': {}", + "{operation} would create ambiguous dynamic token grants on sandbox '{sandbox_name}': {}", err.message() ), severity: "error".to_string(), @@ -2174,7 +2284,8 @@ mod tests { NetworkEndpoint, ProviderCredentialRefresh, ProviderCredentialRefreshMaterial, ProviderCredentialTokenGrant, ProviderCredentialTokenGrantAudienceOverride, ProviderProfile, ProviderProfileCategory, ProviderProfileCredential, - ProviderProfileImportItem, Sandbox, SandboxSpec, + ProviderProfileImportItem, Sandbox, SandboxSpec, StoredProviderProfile, + UpdateProviderProfilesRequest, }; use openshell_core::{ObjectId, ObjectName}; use std::collections::HashMap; @@ -2473,6 +2584,166 @@ mod tests { })); } + #[tokio::test] + async fn update_provider_profile_replaces_custom_profile_and_preserves_metadata() { + let state = test_server_state().await; + let mut original = custom_profile("custom-api"); + original.display_name = "Original API".to_string(); + let mut stored = stored_provider_profile(original); + stored.metadata.as_mut().unwrap().labels = + HashMap::from([("team".to_string(), "platform".to_string())]); + state.store.put_message(&stored).await.unwrap(); + let before: StoredProviderProfile = state + .store + .get_message_by_name("custom-api") + .await + .unwrap() + .unwrap(); + + let mut updated_profile = custom_profile("custom-api"); + updated_profile.display_name = "Updated API".to_string(); + updated_profile.endpoints = vec![NetworkEndpoint { + host: "api.updated.example".to_string(), + port: 443, + ..Default::default() + }]; + let response = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(updated_profile.clone()), + source: "custom-api.yaml".to_string(), + }], + }), + ) + .await + .unwrap() + .into_inner(); + + assert!(response.updated); + assert_eq!(response.profiles, vec![updated_profile]); + let after: StoredProviderProfile = state + .store + .get_message_by_name("custom-api") + .await + .unwrap() + .unwrap(); + let before_meta = before.metadata.unwrap(); + let after_meta = after.metadata.unwrap(); + assert_eq!(after_meta.id, before_meta.id); + assert_eq!(after_meta.name, before_meta.name); + assert_eq!(after_meta.created_at_ms, before_meta.created_at_ms); + assert_eq!(after_meta.labels, before_meta.labels); + assert!(after_meta.resource_version > before_meta.resource_version); + assert_eq!( + after.profile.unwrap().endpoints[0].host, + "api.updated.example" + ); + } + + #[tokio::test] + async fn update_provider_profile_rejects_built_in_and_missing_profiles() { + let state = test_server_state().await; + + let built_in = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("github")), + source: "github.yaml".to_string(), + }], + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!built_in.updated); + assert!(built_in.diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("built-in and cannot be updated") + })); + + let missing = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("missing-custom")), + source: "missing-custom.yaml".to_string(), + }], + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!missing.updated); + assert!(missing.diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("custom provider profile 'missing-custom' does not exist") + })); + } + + #[tokio::test] + async fn update_provider_profile_rejects_attached_dynamic_binding_ambiguity() { + let state = test_server_state().await; + let store = state.store.as_ref(); + import_token_grant_profile(&state, "grant-existing", "api.example.com", 443, "/v1/**") + .await; + import_token_grant_profile(&state, "grant-updated", "api.example.com", 443, "/v2/**").await; + create_empty_token_grant_provider(store, "provider-existing", "grant-existing").await; + create_empty_token_grant_provider(store, "provider-updated", "grant-updated").await; + store + .put_message(&Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "sandbox-update-ambiguity-id".to_string(), + name: "sandbox-update-ambiguity".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + spec: Some(SandboxSpec { + providers: vec![ + "provider-existing".to_string(), + "provider-updated".to_string(), + ], + ..Default::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let mut profile = custom_profile("grant-updated"); + profile.credentials = vec![token_grant_credential("access_token")]; + profile.endpoints = vec![NetworkEndpoint { + host: "api.example.com".to_string(), + port: 443, + path: "/v1/**".to_string(), + protocol: "rest".to_string(), + ..Default::default() + }]; + let response = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(profile), + source: "grant-updated.yaml".to_string(), + }], + }), + ) + .await + .unwrap() + .into_inner(); + + assert!(!response.updated); + assert!(response.diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("update would create ambiguous dynamic token grants") + })); + } + fn provider_with_values(name: &str, provider_type: &str) -> Provider { Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { diff --git a/crates/openshell-server/tests/common/mod.rs b/crates/openshell-server/tests/common/mod.rs index 00228b043..0f01a0503 100644 --- a/crates/openshell-server/tests/common/mod.rs +++ b/crates/openshell-server/tests/common/mod.rs @@ -232,6 +232,13 @@ impl OpenShell for TestOpenShell { Err(Status::unimplemented("not implemented in test")) } + async fn update_provider_profiles( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + async fn lint_provider_profiles( &self, _request: tonic::Request, diff --git a/crates/openshell-server/tests/supervisor_relay_integration.rs b/crates/openshell-server/tests/supervisor_relay_integration.rs index dadb8b384..bd94d151e 100644 --- a/crates/openshell-server/tests/supervisor_relay_integration.rs +++ b/crates/openshell-server/tests/supervisor_relay_integration.rs @@ -257,6 +257,13 @@ impl OpenShell for RelayGateway { Err(Status::unimplemented("unused")) } + async fn update_provider_profiles( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn lint_provider_profiles( &self, _: tonic::Request, diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 67fa8f23a..25c631b32 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -75,6 +75,18 @@ have a built-in or imported provider profile with a `discovery` section. If no matching profile exists, the CLI returns an error instead of falling back to legacy discovery. +Update a custom provider profile after editing its endpoints, binaries, or +credential metadata: + +```shell +openshell provider profile update -f my-api-profile.yaml +``` + +Import remains create-only and fails if the profile ID already exists. Use +`provider profile update` for existing custom profiles. Built-in profiles are +read-only. When `providers_v2_enabled=true`, updated profile policy applies to +all provider instances of that type on the next sandbox config sync. + ## Manage Providers List, inspect, update, and delete providers from the active gateway. diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index 1456c5cfa..d00512bbc 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -53,7 +53,7 @@ Providers v2 currently includes these user-facing features: - Built-in provider profiles stored in the `providers/` directory of the GitHub repository. - `openshell provider list-profiles` with table, YAML, and JSON output. -- `openshell provider profile export`, `import`, `lint`, and `delete` for custom profiles. +- `openshell provider profile export`, `import`, `update`, `lint`, and `delete` for custom profiles. - Provider instances created from built-in or imported profile IDs with `openshell provider create --type `. - Profile-backed credential discovery for explicit `openshell provider create --from-existing` and `openshell provider update --from-existing` flows. The built-in `google-vertex-ai` profile also supplements discovery with Vertex config env vars such as `VERTEX_AI_PROJECT_ID` and `VERTEX_AI_REGION`. - Just-in-time effective policy composition from sandbox policy plus attached provider profiles. @@ -126,7 +126,23 @@ Import all non-recursive `*.yaml`, `*.yml`, and `*.json` files from a directory: openshell provider profile import --from ./provider-profiles ``` -Custom profile IDs must use lowercase kebab-case with `a-z`, `0-9`, and `-`. Built-in profile IDs and legacy provider aliases are reserved. Built-in profiles are read-only, and OpenShell rejects deleting a custom profile while a sandbox-attached provider uses it. +Import is create-only. It fails if a custom profile with the same ID already exists. + +Update an existing custom profile: + +```shell +openshell provider profile update -f github-profile.yaml +``` + +Update all non-recursive `*.yaml`, `*.yml`, and `*.json` files from a directory: + +```shell +openshell provider profile update --from ./provider-profiles +``` + +OpenShell validates every file in an update batch before writing profiles. If an update would make dynamic token grants ambiguous for an attached sandbox, OpenShell rejects the batch before changing any profile. If a concurrent write or storage error interrupts a valid batch, retry the command after reviewing which profiles were updated. + +Custom profile IDs must use lowercase kebab-case with `a-z`, `0-9`, and `-`. Built-in profile IDs and legacy provider aliases are reserved. Built-in profiles are read-only, and OpenShell rejects updating or deleting a built-in profile. OpenShell also rejects deleting a custom profile while a sandbox-attached provider uses it. ### Category Enum @@ -535,6 +551,8 @@ openshell sandbox create \ When `providers_v2_enabled=true`, each attached provider with a matching profile contributes a provider policy layer to the sandbox effective policy. When the setting is disabled, the sandbox receives provider credentials but not provider-derived policy entries. +Updating a custom provider profile affects every provider instance whose `type` matches that profile ID. Provider instances are not rewritten, and sandbox-authored policies are not modified. Running sandboxes observe the updated provider-derived policy on their next config sync. If a gateway-global policy is active, provider-derived policy layers remain suppressed. + Providers v2 does not infer or auto-attach providers from the sandbox command. Attach providers explicitly with `--provider` during sandbox creation, or use `openshell sandbox provider attach` after creation. diff --git a/proto/openshell.proto b/proto/openshell.proto index d701956d3..e1f29b05c 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -95,6 +95,10 @@ service OpenShell { rpc ImportProviderProfiles(ImportProviderProfilesRequest) returns (ImportProviderProfilesResponse); + // Update existing custom provider type profiles. + rpc UpdateProviderProfiles(UpdateProviderProfilesRequest) + returns (UpdateProviderProfilesResponse); + // Validate provider type profiles without registering them. rpc LintProviderProfiles(LintProviderProfilesRequest) returns (LintProviderProfilesResponse); @@ -1105,6 +1109,18 @@ message ImportProviderProfilesResponse { bool imported = 3; } +// Update custom provider profiles request. +message UpdateProviderProfilesRequest { + repeated ProviderProfileImportItem profiles = 1; +} + +// Update custom provider profiles response. +message UpdateProviderProfilesResponse { + repeated ProviderProfileDiagnostic diagnostics = 1; + repeated ProviderProfile profiles = 2; + bool updated = 3; +} + // Lint provider profiles request. message LintProviderProfilesRequest { repeated ProviderProfileImportItem profiles = 1; From 4cc163e73390bac5bd36c8d9f0b8458578a6b74b Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:12:25 -0700 Subject: [PATCH 2/3] fix(providers): require CAS for profile updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- architecture/gateway.md | 10 +- crates/openshell-cli/src/main.rs | 23 +- crates/openshell-cli/src/run.rs | 30 +-- .../tests/provider_commands_integration.rs | 83 +++--- crates/openshell-providers/src/discovery.rs | 1 + crates/openshell-providers/src/profiles.rs | 12 + crates/openshell-server/src/grpc/policy.rs | 41 ++- crates/openshell-server/src/grpc/provider.rs | 254 ++++++++++++++---- docs/sandboxes/manage-providers.mdx | 10 +- docs/sandboxes/providers-v2.mdx | 13 +- proto/openshell.proto | 19 +- 11 files changed, 345 insertions(+), 151 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index 7afec0767..9081ad40b 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -216,7 +216,8 @@ modes: mutation. If they diverge it returns `Conflict` without attempting the write. Client-facing operations that carry an `expected_resource_version` field use this mode: `AttachSandboxProvider`, `DetachSandboxProvider`, - `UpdateProvider`, and `UpdateConfig` (policy backfill path). + `UpdateProvider`, `UpdateProviderProfiles`, and `UpdateConfig` (policy + backfill path). **Lists.** The `list_messages` and `list_messages_with_selector` helpers decode protobuf payloads from list results and hydrate `resource_version` from the @@ -235,7 +236,7 @@ coverage: |---|---|---|---| | Sandbox | `MustCreate` | `update_message_cas` | `list_messages` | | Provider | `MustCreate` | `update_message_cas` | `list_messages` | -| ProviderProfile | `MustCreate` | (immutable) | `list_messages` | +| ProviderProfile | `MustCreate` | `MatchResourceVersion` | `list_messages` | | InferenceRoute | `MustCreate` | `update_message_cas` | `list_messages` | | SandboxPolicy | scoped versioning | scoped versioning | scoped query | | Settings | `Mutex`-guarded | `Mutex`-guarded | single-row | @@ -247,7 +248,10 @@ gateways, the Mutex alone would be insufficient. Sandbox-scoped settings rely entirely on CAS without a Mutex. The `resource_version` is surfaced to clients through `ObjectMeta` in proto -responses. Database migrations backfill existing rows with version 1. +responses. Provider profiles are the exception: custom profile get/list/export +responses copy the stored version onto the profile payload so exported YAML can +carry the expected version for safe single-profile updates. Database migrations +backfill existing rows with version 1. Policy and runtime settings are delivered together through the effective sandbox config path. A gateway-global policy can override sandbox-scoped policy. The diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 6c2c4c365..5c3a8fc72 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -930,16 +930,12 @@ enum ProviderProfileCommands { from: Option, }, - /// Update existing custom provider profiles from a file or directory. - #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + /// Update an existing custom provider profile from a file. + #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Update { /// Profile file to update. #[arg(short = 'f', long = "file", value_hint = ValueHint::FilePath)] - file: Option, - - /// Directory containing profile files to update. - #[arg(long = "from", value_hint = ValueHint::DirPath)] - from: Option, + file: PathBuf, }, /// Validate provider profile files without registering them. @@ -2939,14 +2935,8 @@ async fn main() -> Result<()> { ) .await?; } - ProviderProfileCommands::Update { file, from } => { - run::provider_profile_update( - endpoint, - file.as_deref(), - from.as_deref(), - &tls, - ) - .await?; + ProviderProfileCommands::Update { file } => { + run::provider_profile_update(endpoint, &file, &tls).await?; } ProviderProfileCommands::Lint { file, from } => { run::provider_profile_lint( @@ -3799,8 +3789,7 @@ mod tests { update.command, Some(Commands::Provider { command: Some(ProviderCommands::Profile(ProviderProfileCommands::Update { - file: Some(_), - .. + file: _ })) }) )); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4f3f1e2f8..8f03a68c9 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5019,13 +5019,8 @@ pub async fn provider_profile_import( Err(miette!("provider profile import failed")) } -pub async fn provider_profile_update( - server: &str, - file: Option<&Path>, - from: Option<&Path>, - tls: &TlsOptions, -) -> Result<()> { - let (items, mut diagnostics) = load_profile_import_items(file, from)?; +pub async fn provider_profile_update(server: &str, file: &Path, tls: &TlsOptions) -> Result<()> { + let (mut items, mut diagnostics) = load_profile_import_items(Some(file), None)?; if items.is_empty() && diagnostics.is_empty() { return Err(miette!("no provider profile files found")); } @@ -5035,23 +5030,22 @@ pub async fn provider_profile_update( } let mut client = grpc_client(server, tls).await?; - if !items.is_empty() { + if let Some(item) = items.pop() { + let expected_resource_version = item + .profile + .as_ref() + .map_or(0, |profile| profile.resource_version); let response = client - .update_provider_profiles(UpdateProviderProfilesRequest { profiles: items }) + .update_provider_profiles(UpdateProviderProfilesRequest { + profile: Some(item), + expected_resource_version, + }) .await .into_diagnostic()? .into_inner(); diagnostics.extend(response.diagnostics); if response.updated { - println!( - "Updated {} provider profile{}.", - response.profiles.len(), - if response.profiles.len() == 1 { - "" - } else { - "s" - } - ); + println!("Updated provider profile."); return Ok(()); } } diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 920bed6d9..e8aeb48e4 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -455,6 +455,10 @@ impl OpenShell for TestOpenShell { .profiles .into_iter() .filter_map(|item| item.profile) + .map(|mut profile| { + profile.resource_version = 1; + profile + }) .inspect(|profile| { profiles.insert(profile.id.clone(), profile.clone()); }) @@ -473,39 +477,43 @@ impl OpenShell for TestOpenShell { request: tonic::Request, ) -> Result, Status> { let mut profiles = self.state.profiles.lock().await; - let updates = request - .into_inner() - .profiles - .into_iter() - .filter_map(|item| item.profile) - .collect::>(); - for profile in &updates { - if !profiles.contains_key(&profile.id) { - return Ok(Response::new( - openshell_core::proto::UpdateProviderProfilesResponse { - diagnostics: vec![openshell_core::proto::ProviderProfileDiagnostic { - source: profile.id.clone(), - profile_id: profile.id.clone(), - field: "id".to_string(), - message: format!( - "custom provider profile '{}' does not exist", - profile.id - ), - severity: "error".to_string(), - }], - profiles: Vec::new(), - updated: false, - }, - )); - } - } - for profile in &updates { - profiles.insert(profile.id.clone(), profile.clone()); + let request = request.into_inner(); + let mut profile = request + .profile + .and_then(|item| item.profile) + .ok_or_else(|| Status::invalid_argument("provider profile is required"))?; + let Some(current) = profiles.get(&profile.id) else { + return Ok(Response::new( + openshell_core::proto::UpdateProviderProfilesResponse { + diagnostics: vec![openshell_core::proto::ProviderProfileDiagnostic { + source: profile.id.clone(), + profile_id: profile.id.clone(), + field: "id".to_string(), + message: format!("custom provider profile '{}' does not exist", profile.id), + severity: "error".to_string(), + }], + profile: None, + updated: false, + }, + )); + }; + let expected_resource_version = if request.expected_resource_version != 0 { + request.expected_resource_version + } else { + profile.resource_version + }; + if expected_resource_version == 0 || expected_resource_version != current.resource_version { + return Err(Status::aborted(format!( + "provider profile was modified concurrently (current resource_version: {})", + current.resource_version + ))); } + profile.resource_version = current.resource_version + 1; + profiles.insert(profile.id.clone(), profile.clone()); Ok(Response::new( openshell_core::proto::UpdateProviderProfilesResponse { diagnostics: Vec::new(), - profiles: updates, + profile: Some(profile), updated: true, }, )) @@ -1409,10 +1417,20 @@ binaries: [/usr/bin/custom] run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, &ts.tls) .await .expect("profile import"); + let resource_version = ts + .state + .profiles + .lock() + .await + .get("custom-api") + .expect("custom profile") + .resource_version; std::fs::write( &profile_path, - r" + format!( + r" id: custom-api +resource_version: {resource_version} display_name: Custom API Updated category: other credentials: @@ -1426,10 +1444,11 @@ endpoints: - host: api.updated.example port: 443 binaries: [/usr/bin/custom] -", +" + ), ) .unwrap(); - run::provider_profile_update(&ts.endpoint, Some(&profile_path), None, &ts.tls) + run::provider_profile_update(&ts.endpoint, &profile_path, &ts.tls) .await .expect("profile update"); assert_eq!( diff --git a/crates/openshell-providers/src/discovery.rs b/crates/openshell-providers/src/discovery.rs index 97a6a911b..ebe75e434 100644 --- a/crates/openshell-providers/src/discovery.rs +++ b/crates/openshell-providers/src/discovery.rs @@ -83,6 +83,7 @@ mod tests { fn profile() -> ProviderTypeProfile { ProviderTypeProfile { id: "custom".to_string(), + resource_version: 0, display_name: "Custom".to_string(), description: String::new(), category: openshell_core::proto::ProviderProfileCategory::Other, diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index d2a35ca80..f66168708 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -273,6 +273,8 @@ pub struct BinaryProfile { #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] pub struct ProviderTypeProfile { pub id: String, + #[serde(default, skip_serializing_if = "is_u64_zero")] + pub resource_version: u64, pub display_name: String, #[serde(default)] pub description: String, @@ -303,6 +305,7 @@ impl ProviderTypeProfile { pub fn from_proto(profile: &ProviderProfile) -> Self { Self { id: profile.id.clone(), + resource_version: profile.resource_version, display_name: profile.display_name.clone(), description: profile.description.clone(), category: ProviderProfileCategory::try_from(profile.category) @@ -373,6 +376,7 @@ impl ProviderTypeProfile { pub fn to_proto(&self) -> ProviderProfile { ProviderProfile { id: self.id.clone(), + resource_version: self.resource_version, display_name: self.display_name.clone(), description: self.description.clone(), category: self.category as i32, @@ -410,6 +414,11 @@ impl ProviderTypeProfile { } } +#[allow(clippy::trivially_copy_pass_by_ref)] +fn is_u64_zero(value: &u64) -> bool { + *value == 0 +} + impl CredentialProfile { #[must_use] pub fn is_runtime_resolvable(&self) -> bool { @@ -2395,6 +2404,7 @@ binaries: ["", /usr/bin/broken] "space.yaml".to_string(), ProviderTypeProfile { id: " alex-api ".to_string(), + resource_version: 0, display_name: "Space".to_string(), description: String::new(), category: ProviderProfileCategory::Other, @@ -2409,6 +2419,7 @@ binaries: ["", /usr/bin/broken] "underscore.yaml".to_string(), ProviderTypeProfile { id: "alex_api".to_string(), + resource_version: 0, display_name: "Underscore".to_string(), description: String::new(), category: ProviderProfileCategory::Other, @@ -2423,6 +2434,7 @@ binaries: ["", /usr/bin/broken] "case.yaml".to_string(), ProviderTypeProfile { id: "Alex-API".to_string(), + resource_version: 0, display_name: "Case".to_string(), description: String::new(), category: ProviderProfileCategory::Other, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 455619531..1443b6276 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -4445,6 +4445,7 @@ mod tests { }), profile: Some(openshell_core::proto::ProviderProfile { id: "generic".to_string(), + resource_version: 0, display_name: "Generic Override".to_string(), description: String::new(), category: openshell_core::proto::ProviderProfileCategory::Other as i32, @@ -4488,6 +4489,7 @@ mod tests { }), profile: Some(openshell_core::proto::ProviderProfile { id: "custom-api".to_string(), + resource_version: 0, display_name: "Custom API".to_string(), description: String::new(), category: openshell_core::proto::ProviderProfileCategory::Other as i32, @@ -4552,6 +4554,7 @@ mod tests { }), profile: Some(openshell_core::proto::ProviderProfile { id: "custom-api".to_string(), + resource_version: 0, display_name: "Custom API".to_string(), description: String::new(), category: openshell_core::proto::ProviderProfileCategory::Other as i32, @@ -4817,6 +4820,7 @@ mod tests { }), profile: Some(ProviderProfile { id: "custom-policy".to_string(), + resource_version: 0, display_name: "Custom Policy".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -4861,14 +4865,25 @@ mod tests { .any(|endpoint| endpoint.host == "api.before.example") ); - let updated_profile = stored_profile("api.after.example").profile.unwrap(); + let mut updated_profile = stored_profile("api.after.example").profile.unwrap(); + updated_profile.resource_version = state + .store + .get_message_by_name::("custom-policy") + .await + .unwrap() + .unwrap() + .metadata + .as_ref() + .unwrap() + .resource_version; let response = handle_update_provider_profiles( &state, with_user(Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(updated_profile), source: "custom-policy.yaml".to_string(), - }], + }), + expected_resource_version: 0, })), ) .await @@ -5088,6 +5103,7 @@ mod tests { }), profile: Some(ProviderProfile { id: "custom-token".to_string(), + resource_version: 0, display_name: "Custom Token".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -5132,16 +5148,27 @@ mod tests { .unwrap(); tokio::time::sleep(Duration::from_millis(2)).await; - let rotated_profile = token_grant_profile("https://auth.example.com/rotated-token") + let mut rotated_profile = token_grant_profile("https://auth.example.com/rotated-token") .profile .unwrap(); + rotated_profile.resource_version = state + .store + .get_message_by_name::("custom-token") + .await + .unwrap() + .unwrap() + .metadata + .as_ref() + .unwrap() + .resource_version; handle_update_provider_profiles( &state, with_user(Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(rotated_profile), source: "custom-token.yaml".to_string(), - }], + }), + expected_resource_version: 0, })), ) .await @@ -5295,6 +5322,7 @@ mod tests { source: "custom-api.yaml".to_string(), profile: Some(ProviderProfile { id: "custom-api".to_string(), + resource_version: 0, display_name: "Custom API".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -7304,6 +7332,7 @@ mod tests { }), profile: Some(ProviderProfile { id: "custom-api".to_string(), + resource_version: 0, display_name: "Custom API".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 81c6a100e..1b48d1f1b 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1317,8 +1317,8 @@ pub(super) async fn handle_import_provider_profiles( let mut imported = Vec::with_capacity(profiles.len()); for (_, profile) in profiles { - let stored = stored_provider_profile(profile.to_proto()); - state + let mut stored = stored_provider_profile(profile.to_proto()); + let result = state .store .put_if( StoredProviderProfile::object_type(), @@ -1330,7 +1330,14 @@ pub(super) async fn handle_import_provider_profiles( ) .await .map_err(|e| Status::internal(format!("persist provider profile failed: {e}")))?; - imported.push(stored.profile.unwrap_or_default()); + if let Some(metadata) = stored.metadata.as_mut() { + metadata.resource_version = result.resource_version; + } + let resource_version = stored_profile_resource_version(&stored); + imported.push(profile_response_payload( + stored.profile.unwrap_or_default(), + resource_version, + )); } Ok(Response::new(ImportProviderProfilesResponse { @@ -1345,10 +1352,29 @@ pub(super) async fn handle_update_provider_profiles( request: Request, ) -> Result, Status> { let request = request.into_inner(); - let (profiles, mut diagnostics) = profiles_from_import_items(&request.profiles); + let items = request.profile.into_iter().collect::>(); + let (profiles, mut diagnostics) = profiles_from_import_items(&items); add_empty_profile_set_diagnostic(&profiles, &mut diagnostics); diagnostics.extend(profile_update_target_diagnostics(state.store.as_ref(), &profiles).await?); diagnostics.extend(validate_profile_set(&profiles)); + let expected_resource_version = if request.expected_resource_version != 0 { + Some(request.expected_resource_version) + } else { + profiles + .first() + .map(|(_, profile)| profile.resource_version) + .filter(|version| *version != 0) + }; + if expected_resource_version.is_none() && !profiles.is_empty() { + let (source, profile) = &profiles[0]; + diagnostics.push(ProfileValidationDiagnostic { + source: source.clone(), + profile_id: profile.id.clone(), + field: "resource_version".to_string(), + message: "custom provider profile update requires a non-zero resource_version; export the current profile before editing it".to_string(), + severity: "error".to_string(), + }); + } if !has_errors(&diagnostics) { diagnostics.extend( profile_attached_sandbox_diagnostics(state.store.as_ref(), &profiles, "update").await?, @@ -1358,55 +1384,64 @@ pub(super) async fn handle_update_provider_profiles( if has_errors(&diagnostics) { return Ok(Response::new(UpdateProviderProfilesResponse { diagnostics: diagnostics.into_iter().map(proto_diagnostic).collect(), - profiles: Vec::new(), + profile: None, updated: false, })); } - let mut updated = Vec::with_capacity(profiles.len()); - for (_, profile) in profiles { - let id = normalize_profile_id(&profile.id) - .ok_or_else(|| Status::internal("validated provider profile id is invalid"))?; - let mut stored = state - .store - .get_message_by_name::(&id) - .await - .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? - .ok_or_else(|| Status::not_found("provider profile not found"))?; - let current_version = stored - .metadata - .as_ref() - .map_or(0, |metadata| metadata.resource_version); - stored.profile = Some(profile.to_proto()); - let labels_json = stored - .object_labels() - .filter(|labels| !labels.is_empty()) - .map(|labels| { - serde_json::to_string(&labels) - .map_err(|e| Status::internal(format!("serialize labels failed: {e}"))) - }) - .transpose()?; - let result = state - .store - .put_if( - StoredProviderProfile::object_type(), - stored.object_id(), - stored.object_name(), - &stored.encode_to_vec(), - labels_json.as_deref(), - WriteCondition::MatchResourceVersion(current_version), - ) - .await - .map_err(|e| super::persistence_error_to_status(e, "update provider profile"))?; - if let Some(metadata) = stored.metadata.as_mut() { - metadata.resource_version = result.resource_version; - } - updated.push(stored.profile.unwrap_or_default()); + let expected_resource_version = expected_resource_version.unwrap_or_default(); + let (_, profile) = profiles + .into_iter() + .next() + .ok_or_else(|| Status::internal("validated provider profile update is missing"))?; + let id = normalize_profile_id(&profile.id) + .ok_or_else(|| Status::internal("validated provider profile id is invalid"))?; + let mut stored = state + .store + .get_message_by_name::(&id) + .await + .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? + .ok_or_else(|| Status::not_found("provider profile not found"))?; + let current_version = stored + .metadata + .as_ref() + .map_or(0, |metadata| metadata.resource_version); + if current_version != expected_resource_version { + return Err(Status::aborted(format!( + "provider profile was modified concurrently (current resource_version: {current_version})" + ))); } + stored.profile = Some(profile_storage_payload(profile.to_proto())); + let labels_json = stored + .object_labels() + .filter(|labels| !labels.is_empty()) + .map(|labels| { + serde_json::to_string(&labels) + .map_err(|e| Status::internal(format!("serialize labels failed: {e}"))) + }) + .transpose()?; + let result = state + .store + .put_if( + StoredProviderProfile::object_type(), + stored.object_id(), + stored.object_name(), + &stored.encode_to_vec(), + labels_json.as_deref(), + WriteCondition::MatchResourceVersion(expected_resource_version), + ) + .await + .map_err(|e| super::persistence_error_to_status(e, "update provider profile"))?; + if let Some(metadata) = stored.metadata.as_mut() { + metadata.resource_version = result.resource_version; + } + let resource_version = stored_profile_resource_version(&stored); + let profile = profile_response_payload(stored.profile.unwrap_or_default(), resource_version); + Ok(Response::new(UpdateProviderProfilesResponse { diagnostics: Vec::new(), - profiles: updated, + profile: Some(profile), updated: true, })) } @@ -1480,8 +1515,15 @@ pub(super) async fn get_provider_type_profile( .get_message_by_name::(&id) .await .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? - .and_then(|stored| stored.profile) - .map(|profile| ProviderTypeProfile::from_proto(&profile)); + .and_then(|stored| { + let resource_version = stored_profile_resource_version(&stored); + stored.profile.map(|profile| { + ProviderTypeProfile::from_proto(&profile_response_payload( + profile, + resource_version, + )) + }) + }); Ok(profile) } @@ -1547,8 +1589,15 @@ async fn merged_provider_profiles(store: &Store) -> Result StoredProviderProfile { use crate::persistence::current_time_ms; let now_ms = current_time_ms(); + let profile = profile_storage_payload(profile); StoredProviderProfile { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: uuid::Uuid::new_v4().to_string(), @@ -1777,6 +1827,26 @@ fn stored_provider_profile(profile: ProviderProfile) -> StoredProviderProfile { } } +fn profile_storage_payload(mut profile: ProviderProfile) -> ProviderProfile { + profile.resource_version = 0; + profile +} + +fn profile_response_payload( + mut profile: ProviderProfile, + resource_version: u64, +) -> ProviderProfile { + profile.resource_version = resource_version; + profile +} + +fn stored_profile_resource_version(stored: &StoredProviderProfile) -> u64 { + stored + .metadata + .as_ref() + .map_or(0, |metadata| metadata.resource_version) +} + fn proto_diagnostic(diagnostic: ProfileValidationDiagnostic) -> ProviderProfileDiagnostic { ProviderProfileDiagnostic { source: diagnostic.source, @@ -2391,6 +2461,7 @@ mod tests { }; let profile = ProviderProfile { id: "keycloak-sso".to_string(), + resource_version: 0, display_name: "Keycloak SSO".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -2601,6 +2672,7 @@ mod tests { .unwrap(); let mut updated_profile = custom_profile("custom-api"); + updated_profile.resource_version = before.metadata.as_ref().unwrap().resource_version; updated_profile.display_name = "Updated API".to_string(); updated_profile.endpoints = vec![NetworkEndpoint { host: "api.updated.example".to_string(), @@ -2610,10 +2682,11 @@ mod tests { let response = handle_update_provider_profiles( &state, Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(updated_profile.clone()), source: "custom-api.yaml".to_string(), - }], + }), + expected_resource_version: 0, }), ) .await @@ -2621,7 +2694,11 @@ mod tests { .into_inner(); assert!(response.updated); - assert_eq!(response.profiles, vec![updated_profile]); + assert_eq!(response.profile.as_ref().unwrap().id, updated_profile.id); + assert_eq!( + response.profile.as_ref().unwrap().display_name, + updated_profile.display_name + ); let after: StoredProviderProfile = state .store .get_message_by_name("custom-api") @@ -2648,10 +2725,11 @@ mod tests { let built_in = handle_update_provider_profiles( &state, Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(custom_profile("github")), source: "github.yaml".to_string(), - }], + }), + expected_resource_version: 0, }), ) .await @@ -2667,10 +2745,11 @@ mod tests { let missing = handle_update_provider_profiles( &state, Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(custom_profile("missing-custom")), source: "missing-custom.yaml".to_string(), - }], + }), + expected_resource_version: 0, }), ) .await @@ -2684,6 +2763,52 @@ mod tests { })); } + #[tokio::test] + async fn update_provider_profile_requires_current_resource_version() { + let state = test_server_state().await; + state + .store + .put_message(&stored_provider_profile(custom_profile("custom-api"))) + .await + .unwrap(); + + let missing_version = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profile: Some(ProviderProfileImportItem { + profile: Some(custom_profile("custom-api")), + source: "custom-api.yaml".to_string(), + }), + expected_resource_version: 0, + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!missing_version.updated); + assert!(missing_version.diagnostics.iter().any(|diagnostic| { + diagnostic.field == "resource_version" + && diagnostic.message.contains("non-zero resource_version") + })); + + let mut stale_profile = custom_profile("custom-api"); + stale_profile.resource_version = 99; + let stale_error = handle_update_provider_profiles( + &state, + Request::new(UpdateProviderProfilesRequest { + profile: Some(ProviderProfileImportItem { + profile: Some(stale_profile), + source: "custom-api.yaml".to_string(), + }), + expected_resource_version: 0, + }), + ) + .await + .unwrap_err(); + assert_eq!(stale_error.code(), Code::Aborted); + assert!(stale_error.message().contains("resource_version")); + } + #[tokio::test] async fn update_provider_profile_rejects_attached_dynamic_binding_ambiguity() { let state = test_server_state().await; @@ -2715,6 +2840,15 @@ mod tests { .unwrap(); let mut profile = custom_profile("grant-updated"); + profile.resource_version = store + .get_message_by_name::("grant-updated") + .await + .unwrap() + .unwrap() + .metadata + .as_ref() + .unwrap() + .resource_version; profile.credentials = vec![token_grant_credential("access_token")]; profile.endpoints = vec![NetworkEndpoint { host: "api.example.com".to_string(), @@ -2726,10 +2860,11 @@ mod tests { let response = handle_update_provider_profiles( &state, Request::new(UpdateProviderProfilesRequest { - profiles: vec![ProviderProfileImportItem { + profile: Some(ProviderProfileImportItem { profile: Some(profile), source: "grant-updated.yaml".to_string(), - }], + }), + expected_resource_version: 0, }), ) .await @@ -2773,6 +2908,7 @@ mod tests { fn custom_profile(id: &str) -> ProviderProfile { ProviderProfile { id: id.to_string(), + resource_version: 0, display_name: format!("{id} Profile"), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -3208,6 +3344,7 @@ mod tests { profiles: vec![ProviderProfileImportItem { profile: Some(ProviderProfile { id: "advanced-api".to_string(), + resource_version: 0, display_name: "Advanced API".to_string(), description: String::new(), category: ProviderProfileCategory::Other as i32, @@ -4285,6 +4422,7 @@ mod tests { profiles: vec![ProviderProfileImportItem { profile: Some(ProviderProfile { id: "delegated-refresh-api".to_string(), + resource_version: 0, display_name: "Delegated Refresh API".to_string(), description: String::new(), category: ProviderProfileCategory::Messaging as i32, diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 25c631b32..74418571e 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -75,17 +75,19 @@ have a built-in or imported provider profile with a `discovery` section. If no matching profile exists, the CLI returns an error instead of falling back to legacy discovery. -Update a custom provider profile after editing its endpoints, binaries, or -credential metadata: +Update a custom provider profile after exporting it, editing its endpoints, +binaries, or credential metadata, and preserving the exported `resource_version`: ```shell +openshell provider profile export my-api -o yaml > my-api-profile.yaml openshell provider profile update -f my-api-profile.yaml ``` Import remains create-only and fails if the profile ID already exists. Use `provider profile update` for existing custom profiles. Built-in profiles are -read-only. When `providers_v2_enabled=true`, updated profile policy applies to -all provider instances of that type on the next sandbox config sync. +read-only. Update accepts one file at a time and rejects stale resource versions. +When `providers_v2_enabled=true`, updated profile policy applies to all provider +instances of that type on the next sandbox config sync. ## Manage Providers diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index d00512bbc..be4e0c3bc 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -128,19 +128,14 @@ openshell provider profile import --from ./provider-profiles Import is create-only. It fails if a custom profile with the same ID already exists. -Update an existing custom profile: +Update an existing custom profile by exporting the current custom profile, editing the file, and submitting the edited file back: ```shell +openshell provider profile export github-profile -o yaml > github-profile.yaml openshell provider profile update -f github-profile.yaml ``` -Update all non-recursive `*.yaml`, `*.yml`, and `*.json` files from a directory: - -```shell -openshell provider profile update --from ./provider-profiles -``` - -OpenShell validates every file in an update batch before writing profiles. If an update would make dynamic token grants ambiguous for an attached sandbox, OpenShell rejects the batch before changing any profile. If a concurrent write or storage error interrupts a valid batch, retry the command after reviewing which profiles were updated. +Exported custom profiles include `resource_version`. OpenShell requires that version during update so stale files cannot silently overwrite newer profile definitions. Update accepts one file at a time. If an update would make dynamic token grants ambiguous for an attached sandbox, OpenShell rejects it before changing the profile. Custom profile IDs must use lowercase kebab-case with `a-z`, `0-9`, and `-`. Built-in profile IDs and legacy provider aliases are reserved. Built-in profiles are read-only, and OpenShell rejects updating or deleting a built-in profile. OpenShell also rejects deleting a custom profile while a sandbox-attached provider uses it. @@ -164,6 +159,8 @@ Provider profile YAML and JSON use this shape. Treat this as a field map, not a ```yaml wordWrap showLineNumbers={false} id: custom-api +# Present on exported custom profiles; preserve it when updating. +resource_version: 1 display_name: Custom API description: Custom API access for sandbox agents category: data diff --git a/proto/openshell.proto b/proto/openshell.proto index e1f29b05c..df3702c6f 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -95,7 +95,7 @@ service OpenShell { rpc ImportProviderProfiles(ImportProviderProfilesRequest) returns (ImportProviderProfilesResponse); - // Update existing custom provider type profiles. + // Update an existing custom provider type profile. rpc UpdateProviderProfiles(UpdateProviderProfilesRequest) returns (UpdateProviderProfilesResponse); @@ -1079,6 +1079,10 @@ message ProviderProfile { repeated openshell.sandbox.v1.NetworkBinary binaries = 7; bool inference_capable = 8; ProviderProfileDiscovery discovery = 9; + // Storage resource version for custom profiles. Built-in profiles and new + // profile files use 0. Gateway responses set this for stored custom profiles. + // Update calls use this for optimistic concurrency. + uint64 resource_version = 10; } // Stored custom provider profile object. @@ -1109,15 +1113,20 @@ message ImportProviderProfilesResponse { bool imported = 3; } -// Update custom provider profiles request. +// Update one custom provider profile request. message UpdateProviderProfilesRequest { - repeated ProviderProfileImportItem profiles = 1; + ProviderProfileImportItem profile = 1; + // Expected storage resource version for optimistic concurrency control. + // If 0, the server uses the resource_version embedded in profile.profile. + // Updates without a non-zero version are rejected to prevent stale files from + // silently overwriting newer profile definitions. + uint64 expected_resource_version = 2; } -// Update custom provider profiles response. +// Update one custom provider profile response. message UpdateProviderProfilesResponse { repeated ProviderProfileDiagnostic diagnostics = 1; - repeated ProviderProfile profiles = 2; + ProviderProfile profile = 2; bool updated = 3; } From 8ee094b8f0acb8fbd114ad27cff6b723eb65b02b Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:29:08 -0700 Subject: [PATCH 3/3] fix(providers): serialize profile update validation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- architecture/gateway.md | 6 +++ crates/openshell-cli/src/run.rs | 29 +++++++++---- .../tests/provider_commands_integration.rs | 43 ++++++------------- crates/openshell-server/src/grpc/provider.rs | 5 +++ 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index 9081ad40b..d424cbaac 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -253,6 +253,12 @@ responses copy the stored version onto the profile payload so exported YAML can carry the expected version for safe single-profile updates. Database migrations backfill existing rows with version 1. +Provider profile updates also hold the sandbox synchronization guard while +checking attached-sandbox dynamic token grant ambiguity and writing the updated +profile. Sandbox provider attach/detach uses the same guard, so one gateway +process cannot interleave an attach with a profile update that would leave an +ambiguous final dynamic-token state. + Policy and runtime settings are delivered together through the effective sandbox config path. A gateway-global policy can override sandbox-scoped policy. The sandbox supervisor polls for config revisions and hot-reloads dynamic policy diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 8f03a68c9..6bb8632da 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4955,6 +4955,21 @@ pub async fn provider_profile_export( output: &str, tls: &TlsOptions, ) -> Result<()> { + let rendered = provider_profile_export_text(server, id, output, tls).await?; + if output == "json" { + println!("{rendered}"); + } else { + print!("{rendered}"); + } + Ok(()) +} + +pub async fn provider_profile_export_text( + server: &str, + id: &str, + output: &str, + tls: &TlsOptions, +) -> Result { let mut client = grpc_client(server, tls).await?; let response = client .get_provider_profile(GetProviderProfileRequest { id: id.to_string() }) @@ -4966,16 +4981,14 @@ pub async fn provider_profile_export( .ok_or_else(|| miette!("provider profile '{id}' not found"))?; let profile = ProviderTypeProfile::from_proto(&profile); - if !crate::output::print_output_direct( - output, - || profile_to_json(&profile).into_diagnostic(), - || profile_to_yaml(&profile).into_diagnostic(), - )? { - return Err(miette!( + match output { + "json" => profile_to_json(&profile).into_diagnostic(), + "yaml" => profile_to_yaml(&profile).into_diagnostic(), + "table" => Err(miette!( "profile export supports '-o yaml' and '-o json'; table output is not supported" - )); + )), + _ => Err(miette!("unsupported output format: {output}")), } - Ok(()) } pub async fn provider_profile_import( diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index e8aeb48e4..183f551e5 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -1417,37 +1417,18 @@ binaries: [/usr/bin/custom] run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, &ts.tls) .await .expect("profile import"); - let resource_version = ts - .state - .profiles - .lock() - .await - .get("custom-api") - .expect("custom profile") - .resource_version; - std::fs::write( - &profile_path, - format!( - r" -id: custom-api -resource_version: {resource_version} -display_name: Custom API Updated -category: other -credentials: - - name: api_key - env_vars: [CUSTOM_API_KEY] - auth_style: bearer - header_name: authorization -discovery: - credentials: [api_key] -endpoints: - - host: api.updated.example - port: 443 -binaries: [/usr/bin/custom] -" - ), - ) - .unwrap(); + let exported_yaml = + run::provider_profile_export_text(&ts.endpoint, "custom-api", "yaml", &ts.tls) + .await + .expect("profile export text"); + assert!(exported_yaml.contains("resource_version: 1")); + let updated_yaml = exported_yaml + .replace( + "display_name: Custom API", + "display_name: Custom API Updated", + ) + .replace("host: api.custom.example", "host: api.updated.example"); + std::fs::write(&profile_path, updated_yaml).unwrap(); run::provider_profile_update(&ts.endpoint, &profile_path, &ts.tls) .await .expect("profile update"); diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 1b48d1f1b..7c3ae4bf8 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1375,6 +1375,11 @@ pub(super) async fn handle_update_provider_profiles( severity: "error".to_string(), }); } + let _sandbox_sync_guard = if has_errors(&diagnostics) { + None + } else { + Some(state.compute.sandbox_sync_guard().await) + }; if !has_errors(&diagnostics) { diagnostics.extend( profile_attached_sandbox_diagnostics(state.store.as_ref(), &profiles, "update").await?,