From fa6f77409627c0a7598fe28619245592688227c5 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 21 Aug 2026 09:53:32 -0700 Subject: [PATCH] fix(providers): honor configured profile sources in sandboxes Closes #2877 Use the gateway's configured provider profile catalog for sandbox creation and provider attachment validation. Signed-off-by: Drew Newberry --- crates/openshell-server/src/grpc/provider.rs | 1 + crates/openshell-server/src/grpc/sandbox.rs | 104 +++++++++++++++++- .../src/provider_profile_sources.rs | 2 +- 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 25bfbb045d..add09ea7ca 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1670,6 +1670,7 @@ fn endpoint_path_matches(pattern: &str, path: &str) -> bool { openshell_core::endpoint_path::matches(pattern, path) } +#[cfg(test)] pub async fn validate_provider_environment_keys_unique( store: &Store, workspace: &str, diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 40476d43dc..89f8c942ea 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -50,7 +50,7 @@ use russh::ChannelMsg; use russh::client::AuthResult; use super::provider::{ - get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique, + get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique_with_catalog, }; use super::validation::{ level_matches, source_matches, validate_exec_request_fields, @@ -265,8 +265,17 @@ async fn handle_create_sandbox_inner( .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; } - validate_provider_environment_keys_unique(state.store.as_ref(), &workspace, &spec.providers) + let provider_profile_catalog = state + .provider_profile_sources + .snapshot_catalog(state.store.as_ref(), &workspace) .await?; + validate_provider_environment_keys_unique_with_catalog( + state.store.as_ref(), + &provider_profile_catalog, + &workspace, + &spec.providers, + ) + .await?; // Ensure the template always carries the resolved image. let template = spec.template.get_or_insert_with(SandboxTemplate::default); @@ -571,8 +580,13 @@ pub(super) async fn handle_attach_sandbox_provider( candidate_spec.providers.push(request.provider_name.clone()); } validate_sandbox_spec(&request.sandbox_name, &candidate_spec)?; - validate_provider_environment_keys_unique( + let provider_profile_catalog = state + .provider_profile_sources + .snapshot_catalog(state.store.as_ref(), &workspace) + .await?; + validate_provider_environment_keys_unique_with_catalog( state.store.as_ref(), + &provider_profile_catalog, &workspace, &candidate_spec.providers, ) @@ -2415,8 +2429,33 @@ mod tests { use crate::grpc::test_support::{ authed_request, test_server_state, test_server_state_with_driver, }; + use crate::provider_profile_sources::ProviderProfileSources; + use openshell_core::GatewayProviderProfileSourceConfig; use openshell_core::proto::datamodel::v1::ObjectMeta; + async fn test_server_state_with_user_only_github_profile() -> Arc { + let mut state = test_server_state().await; + Arc::get_mut(&mut state) + .expect("test server state should be uniquely owned") + .provider_profile_sources = + ProviderProfileSources::from_config(&[GatewayProviderProfileSourceConfig::User], None) + .expect("user-only provider profile source configuration should be valid"); + + let github_profile = openshell_providers::builtin_profiles() + .iter() + .find(|profile| profile.id == "github") + .expect("github builtin profile") + .to_proto(); + state + .store + .put_message(&crate::provider_profile_sources::stored_provider_profile( + github_profile, + )) + .await + .expect("store user-managed github profile"); + state + } + // ---- shell_escape ---- #[test] @@ -2906,6 +2945,41 @@ mod tests { assert_eq!(spec.log_level, "debug"); } + #[tokio::test] + async fn attach_sandbox_provider_uses_configured_provider_profile_sources() { + let state = test_server_state_with_user_only_github_profile().await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox("work", Vec::new())) + .await + .unwrap(); + + let response = handle_attach_sandbox_provider( + &state, + authed_request(AttachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "work-github".to_string(), + expected_resource_version: 0, + workspace: String::new(), + }), + ) + .await + .expect("user-only profile catalog should not include the builtin github profile") + .into_inner(); + + assert!(response.attached); + let sandbox = response.sandbox.expect("updated sandbox"); + assert_eq!( + sandbox.spec.expect("sandbox spec").providers, + vec!["work-github"] + ); + } + #[tokio::test] async fn attach_sandbox_provider_is_idempotent_and_avoids_duplicates() { let state = test_server_state().await; @@ -3298,6 +3372,30 @@ mod tests { assert!(err.message().contains("provider-b")); } + #[tokio::test] + async fn create_sandbox_uses_configured_provider_profile_sources() { + let state = test_server_state_with_user_only_github_profile().await; + + let response = handle_create_sandbox( + &state, + authed_request(CreateSandboxRequest { + name: "user-catalog".to_string(), + spec: Some(openshell_core::proto::SandboxSpec::default()), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace: String::new(), + }), + ) + .await + .expect("user-only profile catalog should not include the builtin github profile") + .into_inner(); + + assert_eq!( + response.sandbox.expect("created sandbox").object_name(), + "user-catalog" + ); + } + #[tokio::test] async fn create_sandbox_rejects_reserved_provider_policy_key() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/provider_profile_sources.rs b/crates/openshell-server/src/provider_profile_sources.rs index 346a1f3979..cbe5ff892c 100644 --- a/crates/openshell-server/src/provider_profile_sources.rs +++ b/crates/openshell-server/src/provider_profile_sources.rs @@ -708,7 +708,7 @@ fn profile_snapshot_revision(profiles: &[ProviderProfile]) -> String { format!("sha256:{:x}", hasher.finalize()) } -#[expect(dead_code)] +#[cfg(test)] pub fn stored_provider_profile(profile: ProviderProfile) -> StoredProviderProfile { use crate::persistence::current_time_ms; let now_ms = current_time_ms();