diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 3669ede1d..b1038615e 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -54,9 +54,9 @@ use openshell_core::proto::{ use openshell_core::settings::{self, SettingValueKind}; use openshell_core::{ObjectId, ObjectName}; use openshell_providers::{ - ProviderRegistry, ProviderTypeProfile, detect_provider_from_command, normalize_provider_type, - parse_profile_json, parse_profile_yaml, profile_to_json, profile_to_yaml, profiles_to_json, - profiles_to_yaml, + ProviderRegistry, ProviderTypeProfile, RealDiscoveryContext, detect_provider_from_command, + discover_from_profile, normalize_provider_type, parse_profile_json, parse_profile_yaml, + profile_to_json, profile_to_yaml, profiles_to_json, profiles_to_yaml, }; use owo_colors::OwoColorize; use std::collections::{HashMap, HashSet}; @@ -1709,7 +1709,12 @@ pub async fn sandbox_create( }; let requested_gpu = gpu || image.as_deref().is_some_and(image_requests_gpu); - let inferred_types: Vec = inferred_provider_type(command).into_iter().collect(); + let providers_v2_enabled = gateway_providers_v2_enabled(&mut client).await?; + let inferred_types: Vec = if providers_v2_enabled { + Vec::new() + } else { + inferred_provider_type(command).into_iter().collect() + }; let configured_providers = ensure_required_providers( &mut client, providers, @@ -3631,9 +3636,8 @@ async fn auto_create_provider( return Ok(()); } - let registry = ProviderRegistry::new(); - let discovered = registry - .discover_existing(provider_type) + let discovered = discover_existing_provider_data(client, provider_type) + .await .map_err(|err| miette::miette!("failed to discover provider '{provider_type}': {err}"))?; let Some(discovered) = discovered else { eprintln!( @@ -4094,6 +4098,68 @@ fn service_url_for_gateway(service_url: &str, gateway_endpoint: &str) -> String service_url.to_string() } +async fn gateway_providers_v2_enabled(client: &mut crate::tls::GrpcClient) -> Result { + let response = client + .get_gateway_config(GetGatewayConfigRequest {}) + .await + .into_diagnostic()? + .into_inner(); + let Some(setting) = response.settings.get(settings::PROVIDERS_V2_ENABLED_KEY) else { + return Ok(false); + }; + match setting.value.as_ref() { + Some(setting_value::Value::BoolValue(enabled)) => Ok(*enabled), + None => Ok(false), + Some(_) => Err(miette::miette!( + "gateway setting '{}' has invalid value type; expected bool", + settings::PROVIDERS_V2_ENABLED_KEY + )), + } +} + +async fn fetch_provider_profile( + client: &mut crate::tls::GrpcClient, + provider_type: &str, +) -> Result { + let response = client + .get_provider_profile(GetProviderProfileRequest { + id: provider_type.to_string(), + }) + .await + .map_err(|status| { + if status.code() == Code::NotFound { + miette::miette!( + "provider profile '{provider_type}' not found; providers v2 discovery requires a provider profile" + ) + } else { + miette::miette!(status.to_string()) + } + })?; + + response + .into_inner() + .profile + .ok_or_else(|| miette::miette!("provider profile '{provider_type}' missing from response")) +} + +async fn discover_existing_provider_data( + client: &mut crate::tls::GrpcClient, + provider_type: &str, +) -> Result> { + if gateway_providers_v2_enabled(client).await? { + let profile = fetch_provider_profile(client, provider_type).await?; + let profile = ProviderTypeProfile::from_proto(&profile); + discover_from_profile(&profile, &RealDiscoveryContext).map_err(|err| { + miette::miette!("failed to discover existing provider data from profile: {err}") + }) + } else { + let registry = ProviderRegistry::new(); + registry + .discover_existing(provider_type) + .map_err(|err| miette::miette!("failed to discover existing provider data: {err}")) + } +} + pub async fn provider_create( server: &str, name: &str, @@ -4143,10 +4209,7 @@ pub async fn provider_create( let mut config_map = parse_key_value_pairs(config, "--config")?; if from_existing { - let registry = ProviderRegistry::new(); - let discovered = registry - .discover_existing(&provider_type) - .map_err(|err| miette::miette!("failed to discover existing provider data: {err}"))?; + let discovered = discover_existing_provider_data(&mut client, &provider_type).await?; let Some(discovered) = discovered else { return Err(miette::miette!( "no existing local credentials/config found for provider type '{provider_type}'" @@ -4162,13 +4225,9 @@ pub async fn provider_create( } if credential_map.is_empty() { - let allows_refresh_bootstrap = client - .get_provider_profile(GetProviderProfileRequest { - id: provider_type.clone(), - }) + let allows_refresh_bootstrap = fetch_provider_profile(&mut client, &provider_type) .await .ok() - .and_then(|response| response.into_inner().profile) .is_some_and(|profile| provider_profile_allows_refresh_bootstrap(&profile)); if !allows_refresh_bootstrap { return Err(miette::miette!( @@ -4911,10 +4970,7 @@ pub async fn provider_update( .ok_or_else(|| miette::miette!("provider '{name}' not found"))?; let provider_type = existing.r#type; - let registry = ProviderRegistry::new(); - let discovered = registry - .discover_existing(&provider_type) - .map_err(|err| miette::miette!("failed to discover existing provider data: {err}"))?; + let discovered = discover_existing_provider_data(&mut client, &provider_type).await?; let Some(discovered) = discovered else { return Err(miette::miette!( "no existing local credentials/config found for provider type '{provider_type}'" diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index b0e3b99a1..090097a20 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -23,10 +23,10 @@ use openshell_core::proto::{ ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, Provider, ProviderCredentialRefresh, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileCredential, - ProviderResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, + ProviderProfileDiscovery, ProviderResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, RotateProviderCredentialRequest, RotateProviderCredentialResponse, Sandbox, SandboxResponse, - SandboxStreamEvent, ServiceStatus, SupervisorMessage, UpdateProviderRequest, - WatchSandboxRequest, + SandboxStreamEvent, ServiceStatus, SettingValue, SupervisorMessage, UpdateProviderRequest, + WatchSandboxRequest, setting_value, }; use openshell_core::{ObjectId, ObjectName}; use std::collections::HashMap; @@ -46,6 +46,7 @@ struct ProviderState { refresh_requests: Arc>>, sandbox_providers: Arc>>>, sandbox_provider_requests: Arc>>, + global_settings: Arc>>, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -270,7 +271,10 @@ impl OpenShell for TestOpenShell { &self, _request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(GetGatewayConfigResponse::default())) + Ok(Response::new(GetGatewayConfigResponse { + settings: self.state.global_settings.lock().await.clone(), + settings_revision: 1, + })) } async fn get_sandbox_provider_environment( @@ -901,6 +905,15 @@ async fn run_server() -> TestServer { } } +async fn enable_providers_v2(ts: &TestServer) { + ts.state.global_settings.lock().await.insert( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + SettingValue { + value: Some(setting_value::Value::BoolValue(true)), + }, + ); +} + #[tokio::test] async fn provider_cli_run_functions_support_full_crud_flow() { let ts = run_server().await; @@ -1159,6 +1172,8 @@ credentials: env_vars: [CUSTOM_API_KEY] auth_style: bearer header_name: authorization +discovery: + credentials: [api_key] endpoints: - host: api.custom.example port: 443 @@ -1209,6 +1224,209 @@ binaries: [/usr/bin/custom] .expect("profile delete"); } +#[tokio::test] +async fn provider_create_from_existing_uses_profile_discovery_when_v2_enabled() { + let ts = run_server().await; + enable_providers_v2(&ts).await; + ts.state.profiles.lock().await.insert( + "custom-discovery".to_string(), + ProviderProfile { + id: "custom-discovery".to_string(), + display_name: "Custom Discovery".to_string(), + credentials: vec![ProviderProfileCredential { + name: "api_key".to_string(), + env_vars: vec!["CUSTOM_DISCOVERY_API_KEY".to_string()], + required: true, + ..Default::default() + }], + discovery: Some(ProviderProfileDiscovery { + credentials: vec!["api_key".to_string()], + }), + ..Default::default() + }, + ); + let _env = EnvVarGuard::set(&[("CUSTOM_DISCOVERY_API_KEY", "profile-secret")]); + + run::provider_create( + &ts.endpoint, + "custom-discovered", + "custom-discovery", + true, + &[], + &[], + &ts.tls, + ) + .await + .expect("profile-backed provider create --from-existing"); + + let provider = ts + .state + .providers + .lock() + .await + .get("custom-discovered") + .cloned() + .expect("custom provider should be stored"); + assert_eq!(provider.r#type, "custom-discovery"); + assert_eq!( + provider.credentials.get("CUSTOM_DISCOVERY_API_KEY"), + Some(&"profile-secret".to_string()) + ); +} + +#[tokio::test] +async fn provider_create_from_existing_uses_registry_discovery_when_v2_disabled() { + let ts = run_server().await; + let _env = EnvVarGuard::set(&[("OPENAI_API_KEY", "legacy-openai-secret")]); + + run::provider_create( + &ts.endpoint, + "legacy-openai", + "openai", + true, + &[], + &[], + &ts.tls, + ) + .await + .expect("legacy provider create --from-existing"); + + let provider = ts + .state + .providers + .lock() + .await + .get("legacy-openai") + .cloned() + .expect("legacy provider should be stored"); + assert_eq!(provider.r#type, "openai"); + assert_eq!( + provider.credentials.get("OPENAI_API_KEY"), + Some(&"legacy-openai-secret".to_string()) + ); +} + +#[tokio::test] +async fn provider_create_from_existing_requires_profile_when_v2_enabled() { + let ts = run_server().await; + enable_providers_v2(&ts).await; + let _env = EnvVarGuard::set(&[("OPENAI_API_KEY", "legacy-openai-secret")]); + + let err = run::provider_create(&ts.endpoint, "v2-openai", "openai", true, &[], &[], &ts.tls) + .await + .expect_err("v2 discovery without a profile should fail"); + + assert!( + err.to_string() + .contains("providers v2 discovery requires a provider profile"), + "unexpected error: {err}" + ); + assert!(!ts.state.providers.lock().await.contains_key("v2-openai")); +} + +#[tokio::test] +async fn provider_create_from_existing_fails_when_profile_discovery_finds_nothing() { + let ts = run_server().await; + enable_providers_v2(&ts).await; + ts.state.profiles.lock().await.insert( + "empty-discovery".to_string(), + ProviderProfile { + id: "empty-discovery".to_string(), + display_name: "Empty Discovery".to_string(), + credentials: vec![ProviderProfileCredential { + name: "api_key".to_string(), + env_vars: vec!["CUSTOM_DISCOVERY_TOKEN_NOT_SET_1460".to_string()], + required: false, + ..Default::default() + }], + discovery: Some(ProviderProfileDiscovery { + credentials: vec!["api_key".to_string()], + }), + ..Default::default() + }, + ); + + let err = run::provider_create( + &ts.endpoint, + "empty-discovered", + "empty-discovery", + true, + &[], + &[], + &ts.tls, + ) + .await + .expect_err("empty profile-backed discovery should fail"); + + assert!( + err.to_string() + .contains("no existing local credentials/config found"), + "unexpected error: {err}" + ); + assert!( + !ts.state + .providers + .lock() + .await + .contains_key("empty-discovered") + ); +} + +#[tokio::test] +async fn provider_update_from_existing_uses_profile_discovery_when_v2_enabled() { + let ts = run_server().await; + enable_providers_v2(&ts).await; + ts.state.profiles.lock().await.insert( + "custom-update-discovery".to_string(), + ProviderProfile { + id: "custom-update-discovery".to_string(), + display_name: "Custom Update Discovery".to_string(), + credentials: vec![ProviderProfileCredential { + name: "api_key".to_string(), + env_vars: vec!["CUSTOM_UPDATE_DISCOVERY_API_KEY".to_string()], + required: true, + ..Default::default() + }], + discovery: Some(ProviderProfileDiscovery { + credentials: vec!["api_key".to_string()], + }), + ..Default::default() + }, + ); + ts.state.providers.lock().await.insert( + "custom-update".to_string(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "id-custom-update".to_string(), + name: "custom-update".to_string(), + ..Default::default() + }), + r#type: "custom-update-discovery".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ); + let _env = EnvVarGuard::set(&[("CUSTOM_UPDATE_DISCOVERY_API_KEY", "updated-profile-secret")]); + + run::provider_update(&ts.endpoint, "custom-update", true, &[], &[], &[], &ts.tls) + .await + .expect("profile-backed provider update --from-existing"); + + let provider = ts + .state + .providers + .lock() + .await + .get("custom-update") + .cloned() + .expect("custom provider should still be stored"); + assert_eq!( + provider.credentials.get("CUSTOM_UPDATE_DISCOVERY_API_KEY"), + Some(&"updated-profile-secret".to_string()) + ); +} + #[tokio::test] async fn provider_profile_import_from_directory_imports_supported_profile_files() { let ts = run_server().await; diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 2ce409413..3ed43b2fc 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -23,8 +23,8 @@ use openshell_core::proto::{ ListSandboxProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, PlatformEvent, ProviderResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, Sandbox, SandboxCondition, SandboxLogLine, SandboxPhase, SandboxResponse, SandboxStatus, SandboxStreamEvent, - ServiceStatus, SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, - sandbox_stream_event, + ServiceStatus, SettingValue, SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, + sandbox_stream_event, setting_value, }; use std::collections::HashMap; use std::fs; @@ -46,6 +46,7 @@ struct SandboxState { vm_error_after_started: Arc, vm_slow_progress_before_ready: Arc, vm_log_churn_before_ready: Arc, + global_settings: Arc>>, } #[derive(Clone, Default)] @@ -165,7 +166,10 @@ impl OpenShell for TestOpenShell { &self, _request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(GetGatewayConfigResponse::default())) + Ok(Response::new(GetGatewayConfigResponse { + settings: self.state.global_settings.lock().await.clone(), + settings_revision: 1, + })) } async fn get_sandbox_provider_environment( @@ -751,6 +755,15 @@ async fn create_requests(server: &TestServer) -> Vec { server.openshell.state.create_requests.lock().await.clone() } +async fn enable_providers_v2(server: &TestServer) { + server.openshell.state.global_settings.lock().await.insert( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + SettingValue { + value: Some(setting_value::Value::BoolValue(true)), + }, + ); +} + fn test_tls(server: &TestServer) -> TlsOptions { server.tls.with_gateway_name("openshell") } @@ -871,6 +884,53 @@ async fn sandbox_create_sends_cpu_and_memory_limits_only() { assert!(!resources.fields.contains_key("requests")); } +#[tokio::test] +async fn sandbox_create_does_not_infer_command_providers_when_v2_enabled() { + let server = run_server().await; + enable_providers_v2(&server).await; + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + run::sandbox_create( + &server.endpoint, + Some("v2-no-inferred-provider"), + None, + "openshell", + None, + true, + false, + None, + None, + None, + None, + &[], + None, + None, + &["claude".to_string(), "--version".to_string()], + Some(true), + Some(false), + &HashMap::new(), + &tls, + ) + .await + .expect("sandbox create should succeed without inferred provider"); + + let requests = create_requests(&server).await; + let providers = requests[0] + .spec + .as_ref() + .expect("sandbox spec should be sent") + .providers + .clone(); + assert!( + providers.is_empty(), + "providers v2 should not infer command providers, got {providers:?}" + ); +} + #[tokio::test] async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() { let server = run_server().await; diff --git a/crates/openshell-providers/src/discovery.rs b/crates/openshell-providers/src/discovery.rs index 8c10bbf7e..79d6fb091 100644 --- a/crates/openshell-providers/src/discovery.rs +++ b/crates/openshell-providers/src/discovery.rs @@ -1,7 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use crate::{DiscoveredProvider, DiscoveryContext, ProviderDiscoverySpec, ProviderError}; +use crate::{ + DiscoveredProvider, DiscoveryContext, ProviderDiscoverySpec, ProviderError, ProviderTypeProfile, +}; +use std::collections::HashSet; pub fn discover_with_spec( spec: &ProviderDiscoverySpec, @@ -26,3 +29,132 @@ pub fn discover_with_spec( Ok(Some(discovered)) } } + +pub fn discover_from_profile( + profile: &ProviderTypeProfile, + context: &dyn DiscoveryContext, +) -> Result, ProviderError> { + let mut discovered = DiscoveredProvider::default(); + let mut scanned_env_vars = HashSet::new(); + + for credential_name in &profile.discovery.credentials { + let credential_name = credential_name.trim(); + let Some(credential) = profile + .credentials + .iter() + .find(|credential| credential.name.trim() == credential_name) + else { + return Err(ProviderError::UnknownDiscoveryCredential { + profile_id: profile.id.clone(), + credential_name: credential_name.to_string(), + }); + }; + + for env_var in &credential.env_vars { + let env_var = env_var.trim(); + if env_var.is_empty() || !scanned_env_vars.insert(env_var.to_string()) { + continue; + } + if let Some(value) = context.env_var(env_var) + && !value.trim().is_empty() + { + discovered + .credentials + .entry(env_var.to_string()) + .or_insert(value); + } + } + } + + if discovered.is_empty() { + Ok(None) + } else { + Ok(Some(discovered)) + } +} + +#[cfg(test)] +mod tests { + use super::discover_from_profile; + use crate::profiles::{CredentialProfile, DiscoveryProfile}; + use crate::test_helpers::MockDiscoveryContext; + use crate::{ProviderError, ProviderTypeProfile}; + + fn profile() -> ProviderTypeProfile { + ProviderTypeProfile { + id: "custom".to_string(), + display_name: "Custom".to_string(), + description: String::new(), + category: openshell_core::proto::ProviderProfileCategory::Other, + credentials: vec![ + CredentialProfile { + name: "api_key".to_string(), + env_vars: vec!["CUSTOM_API_KEY".to_string(), "CUSTOM_API_TOKEN".to_string()], + required: true, + description: String::new(), + auth_style: String::new(), + header_name: String::new(), + query_param: String::new(), + refresh: None, + }, + CredentialProfile { + name: "secondary".to_string(), + env_vars: vec!["CUSTOM_API_KEY".to_string()], + required: false, + description: String::new(), + auth_style: String::new(), + header_name: String::new(), + query_param: String::new(), + refresh: None, + }, + ], + endpoints: Vec::new(), + binaries: Vec::new(), + inference_capable: false, + discovery: DiscoveryProfile { + credentials: vec!["api_key".to_string(), "secondary".to_string()], + }, + } + } + + #[test] + fn profile_discovery_scans_referenced_credential_env_vars() { + let ctx = MockDiscoveryContext::new().with_env("CUSTOM_API_TOKEN", "secret-token"); + + let discovered = discover_from_profile(&profile(), &ctx) + .expect("discovery should succeed") + .expect("provider should be discovered"); + + assert_eq!( + discovered.credentials.get("CUSTOM_API_TOKEN"), + Some(&"secret-token".to_string()) + ); + assert!(!discovered.credentials.contains_key("CUSTOM_API_KEY")); + } + + #[test] + fn profile_discovery_ignores_empty_values_and_returns_none_when_empty() { + let ctx = MockDiscoveryContext::new().with_env("CUSTOM_API_KEY", " "); + + let discovered = discover_from_profile(&profile(), &ctx).expect("discovery should succeed"); + + assert!(discovered.is_none()); + } + + #[test] + fn profile_discovery_rejects_unknown_credential_references() { + let mut profile = profile(); + profile.discovery.credentials = vec!["missing".to_string()]; + + let err = discover_from_profile(&profile, &MockDiscoveryContext::new()) + .expect_err("unknown discovery credential should fail"); + + assert!(matches!( + err, + ProviderError::UnknownDiscoveryCredential { + profile_id, + credential_name + } if profile_id == "custom" && credential_name == "missing" + )); + } +} diff --git a/crates/openshell-providers/src/lib.rs b/crates/openshell-providers/src/lib.rs index afdf796e5..21a1750ab 100644 --- a/crates/openshell-providers/src/lib.rs +++ b/crates/openshell-providers/src/lib.rs @@ -16,7 +16,7 @@ use std::path::Path; pub use openshell_core::proto::Provider; pub use context::{DiscoveryContext, RealDiscoveryContext}; -pub use discovery::discover_with_spec; +pub use discovery::{discover_from_profile, discover_with_spec}; pub use profiles::{ CredentialRefreshProfile, ProfileError, ProfileValidationDiagnostic, ProviderTypeProfile, default_profiles, get_default_profile, normalize_profile_id, parse_profile_json, @@ -28,6 +28,13 @@ pub use profiles::{ pub enum ProviderError { #[error("unsupported provider type: {0}")] UnsupportedProvider(String), + #[error( + "provider profile '{profile_id}' discovery references unknown credential '{credential_name}'" + )] + UnknownDiscoveryCredential { + profile_id: String, + credential_name: String, + }, } #[derive(Debug, Clone, Default, PartialEq, Eq)] diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 65dd33bea..25c750e63 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -9,7 +9,7 @@ use openshell_core::proto::{ GraphqlOperation, L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, ProviderCredentialRefresh, ProviderCredentialRefreshMaterial, ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileCategory, - ProviderProfileCredential, + ProviderProfileCredential, ProviderProfileDiscovery, }; use serde::ser::SerializeStruct; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; @@ -114,6 +114,12 @@ pub struct CredentialRefreshMaterialProfile { pub secret: bool, } +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] +pub struct DiscoveryProfile { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub credentials: Vec, +} + // These YAML/JSON DTOs mirror the network policy protos intentionally. Keep // every lossless conversion below in sync with proto/sandbox.proto. If a field // is added to NetworkEndpoint, L7Rule, L7Allow, L7DenyRule, L7QueryMatcher, @@ -242,6 +248,8 @@ pub struct ProviderTypeProfile { pub binaries: Vec, #[serde(default)] pub inference_capable: bool, + #[serde(default, skip_serializing_if = "discovery_is_empty")] + pub discovery: DiscoveryProfile, } // Provider profile import/export is expected to be lossless for the network @@ -277,6 +285,11 @@ impl ProviderTypeProfile { endpoints: profile.endpoints.iter().map(endpoint_from_proto).collect(), binaries: profile.binaries.iter().map(binary_from_proto).collect(), inference_capable: profile.inference_capable, + discovery: profile + .discovery + .as_ref() + .map(discovery_from_proto) + .unwrap_or_default(), } } @@ -317,6 +330,8 @@ impl ProviderTypeProfile { endpoints: self.endpoints.iter().map(endpoint_to_proto).collect(), binaries: self.binaries.iter().map(binary_to_proto).collect(), inference_capable: self.inference_capable, + discovery: (!discovery_is_empty(&self.discovery)) + .then(|| discovery_to_proto(&self.discovery)), } } @@ -330,6 +345,10 @@ impl ProviderTypeProfile { } } +fn discovery_is_empty(discovery: &DiscoveryProfile) -> bool { + discovery.credentials.is_empty() +} + impl Serialize for BinaryProfile { fn serialize(&self, serializer: S) -> Result where @@ -541,6 +560,18 @@ fn credential_refresh_to_proto(refresh: &CredentialRefreshProfile) -> ProviderCr } } +fn discovery_from_proto(discovery: &ProviderProfileDiscovery) -> DiscoveryProfile { + DiscoveryProfile { + credentials: discovery.credentials.clone(), + } +} + +fn discovery_to_proto(discovery: &DiscoveryProfile) -> ProviderProfileDiscovery { + ProviderProfileDiscovery { + credentials: discovery.credentials.clone(), + } +} + fn endpoint_to_proto(endpoint: &EndpointProfile) -> NetworkEndpoint { NetworkEndpoint { host: endpoint.host.clone(), @@ -875,6 +906,33 @@ pub fn validate_profile_set( } } + let mut discovery_credentials = HashSet::new(); + for (index, credential_name) in profile.discovery.credentials.iter().enumerate() { + let credential_name = credential_name.trim(); + if credential_name.is_empty() { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + format!("discovery.credentials[{index}]"), + "discovery credential name must not be empty", + )); + } else if !discovery_credentials.insert(credential_name.to_string()) { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + format!("discovery.credentials[{index}]"), + format!("duplicate discovery credential: {credential_name}"), + )); + } else if !credential_names.contains(credential_name) { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + format!("discovery.credentials[{index}]"), + format!("unknown discovery credential: {credential_name}"), + )); + } + } + let mut env_vars = HashSet::new(); for credential in &profile.credentials { for env_var in &credential.env_vars { @@ -1035,7 +1093,7 @@ mod tests { use openshell_core::proto::ProviderProfileCategory; use super::{ - ProfileError, ProviderTypeProfile, default_profiles, get_default_profile, + DiscoveryProfile, ProfileError, ProviderTypeProfile, default_profiles, get_default_profile, normalize_profile_id, parse_profile_catalog_yamls, parse_profile_json, parse_profile_yaml, profile_to_json, profile_to_yaml, validate_profile_set, }; @@ -1061,7 +1119,23 @@ mod tests { proto.category, ProviderProfileCategory::SourceControl as i32 ); - assert_eq!(proto.endpoints.len(), 2); + assert_eq!(proto.endpoints.len(), 3); + assert!( + proto.endpoints.iter().any(|endpoint| { + endpoint.host == "api.github.com" + && endpoint.protocol == "graphql" + && endpoint.path == "/graphql" + && endpoint.access == "read-only" + }), + "github profile should include read-only GraphQL endpoint" + ); + assert!( + proto + .endpoints + .iter() + .all(|endpoint| endpoint.access == "read-only"), + "github profile endpoints should all be read-only" + ); assert_eq!(proto.binaries.len(), 4); } @@ -1092,6 +1166,29 @@ credentials: assert_eq!(profile.credential_env_vars(), vec!["EXAMPLE_API_KEY"]); } + #[test] + fn profile_discovery_metadata_round_trips_through_proto_and_yaml() { + let profile = parse_profile_yaml( + r" +id: example +display_name: Example +credentials: + - name: api_key + env_vars: [EXAMPLE_API_KEY] +discovery: + credentials: [api_key] +", + ) + .expect("profile should parse"); + + assert_eq!(profile.discovery.credentials, vec!["api_key"]); + let from_proto = ProviderTypeProfile::from_proto(&profile.to_proto()); + assert_eq!(from_proto.discovery.credentials, vec!["api_key"]); + let exported = profile_to_yaml(&from_proto).expect("yaml"); + assert!(exported.contains("discovery:")); + assert!(exported.contains("api_key")); + } + #[test] fn profile_refresh_metadata_round_trips_through_proto_and_yaml() { let profile = parse_profile_yaml( @@ -1241,6 +1338,8 @@ credentials: - name: api_key env_vars: [BROKEN_TOKEN, ""] auth_style: unknown +discovery: + credentials: [api_key, missing_key] endpoints: - host: "" port: 0 @@ -1260,6 +1359,7 @@ binaries: ["", /usr/bin/broken] assert!(messages.contains(&"credential env var must not be empty")); assert!(messages.contains(&"query_param is required for query auth")); assert!(messages.contains(&"unsupported auth_style: unknown")); + assert!(messages.contains(&"unknown discovery credential: missing_key")); assert!( messages .iter() @@ -1282,6 +1382,7 @@ binaries: ["", /usr/bin/broken] endpoints: Vec::new(), binaries: Vec::new(), inference_capable: false, + discovery: DiscoveryProfile::default(), }, ), ( @@ -1295,6 +1396,7 @@ binaries: ["", /usr/bin/broken] endpoints: Vec::new(), binaries: Vec::new(), inference_capable: false, + discovery: DiscoveryProfile::default(), }, ), ( @@ -1308,6 +1410,7 @@ binaries: ["", /usr/bin/broken] endpoints: Vec::new(), binaries: Vec::new(), inference_capable: false, + discovery: DiscoveryProfile::default(), }, ), ]; diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 482e9e0f8..bdc96d862 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3451,6 +3451,7 @@ mod tests { }], binaries: Vec::new(), inference_capable: false, + discovery: None, }), }) .await @@ -3507,6 +3508,7 @@ mod tests { harness: true, }], inference_capable: false, + discovery: None, }), }) .await @@ -3556,6 +3558,7 @@ mod tests { }], binaries: Vec::new(), inference_capable: false, + discovery: None, }), }) .await @@ -3583,7 +3586,7 @@ mod tests { assert_eq!(layers.len(), 1); assert_eq!(layers[0].rule_name, "_provider_work_github"); - assert_eq!(layers[0].rule.endpoints.len(), 2); + assert_eq!(layers[0].rule.endpoints.len(), 3); assert!( layers[0] .rule @@ -3591,6 +3594,23 @@ mod tests { .iter() .any(|endpoint| endpoint.host == "api.github.com") ); + assert!( + layers[0].rule.endpoints.iter().any(|endpoint| { + endpoint.host == "api.github.com" + && endpoint.protocol == "graphql" + && endpoint.path == "/graphql" + && endpoint.access == "read-only" + }), + "github provider policy should include read-only GraphQL endpoint" + ); + assert!( + layers[0] + .rule + .endpoints + .iter() + .all(|endpoint| endpoint.access == "read-only"), + "github provider policy should be read-only by default" + ); } #[test] @@ -4085,6 +4105,7 @@ mod tests { harness: true, }], inference_capable: false, + discovery: None, }), }], }), diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 6e760eb37..3ddaae037 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1515,6 +1515,7 @@ mod tests { endpoints: Vec::new(), binaries: Vec::new(), inference_capable: false, + discovery: None, } } @@ -1925,6 +1926,7 @@ mod tests { harness: true, }], inference_capable: false, + discovery: None, }), source: "advanced-api.yaml".to_string(), }], @@ -2951,6 +2953,7 @@ mod tests { endpoints: vec![], binaries: vec![], inference_capable: false, + discovery: None, }), source: "delegated-refresh-api.yaml".to_string(), }], diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 8328e8a42..7bc1f977f 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -67,6 +67,12 @@ openshell settings set --global --key providers_v2_enabled --value true Without `providers_v2_enabled=true`, provider behavior remains credential-only. +When `providers_v2_enabled=true`, `--from-existing` uses profile-backed +discovery instead of the legacy provider registry. The requested `--type` must +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. + ## Manage Providers List, inspect, update, and delete providers from the active gateway. @@ -102,9 +108,10 @@ Use `0` as the timestamp to clear expiry for a credential key. ## Credential Refresh Provider refresh stores non-injectable refresh material separately from the -provider's current credential values. The gateway can mint OAuth2 client -credentials tokens and Google service account JWT tokens, then write the current -access token back to the provider record for sandbox injection. +provider's current credential values. The gateway can mint OAuth2 refresh-token +tokens, OAuth2 client credentials tokens, and Google service account JWT tokens, +then write the current access token back to the provider record for sandbox +injection. Configure refresh metadata for one injectable credential key: @@ -164,17 +171,20 @@ also contribute provider-generated network policy entries when providers keep the previous behavior and only provide credentials. -Providers cannot be added to a running sandbox. If you need to attach an -additional provider, delete the sandbox and recreate it with all required -providers specified. +Legacy provider attachment is fixed at sandbox creation time. Providers v2 adds +`openshell sandbox provider attach` and `openshell sandbox provider detach` for +running sandboxes. See [Providers v2](/sandboxes/providers-v2#attach-and-detach-providers) +for runtime attach and detach behavior. ### Auto-Discovery Shortcut -When the trailing command in `openshell sandbox create` is a recognized tool name (`claude`, `codex`, or `opencode`), the CLI auto-creates the required -provider from your local credentials if one does not already exist. You do not -need to create the provider separately: +When `providers_v2_enabled=false` and the trailing command in +`openshell sandbox create` is a recognized tool name (`claude`, `codex`, or +`opencode`), the CLI auto-creates the required provider from your local +credentials if one does not already exist. You do not need to create the +provider separately: ```shell openshell sandbox create -- claude @@ -183,6 +193,10 @@ openshell sandbox create -- claude This detects `claude` as a known tool, finds your `ANTHROPIC_API_KEY`, creates a provider, attaches it to the sandbox, and launches Claude Code. +Providers v2 disables command-derived provider inference. When +`providers_v2_enabled=true`, create or import the provider profile, create the +provider instance, and pass `--provider ` explicitly. + ## How Credential Injection Works The agent process inside the sandbox never sees real credential values. At startup, the proxy replaces each credential with an opaque placeholder token in the agent's environment. When the agent sends an HTTP request containing a placeholder, the proxy resolves it to the real credential before forwarding upstream. diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index fed501e03..9a4a9dc35 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -55,6 +55,7 @@ Providers v2 currently includes these user-facing features: - `openshell provider list-profiles` with table, YAML, and JSON output. - `openshell provider profile export`, `import`, `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. - Just-in-time effective policy composition from sandbox policy plus attached provider profiles. - Runtime sandbox provider lifecycle commands under `openshell sandbox provider list|attach|detach`. - Credential refresh configuration with `openshell provider refresh status|configure|rotate|delete`. @@ -87,6 +88,14 @@ List available profiles: openshell provider list-profiles ``` +Built-in Providers v2 profiles currently include: + +| Profile ID | Category | Credential environment variables | +|---|---|---| +| `claude-code` | `agent` | `ANTHROPIC_API_KEY`, `CLAUDE_API_KEY` | +| `github` | `source_control` | `GITHUB_TOKEN`, `GH_TOKEN` | +| `nvidia` | `inference` | `NVIDIA_API_KEY` | + Export a built-in profile as YAML: ```shell @@ -170,6 +179,9 @@ credentials: required: true secret: true +discovery: + credentials: [api_token] + endpoints: - host: api.example.com port: 443 @@ -224,6 +236,12 @@ binaries: `credentials` declares the credential names, environment variables, auth metadata, and optional refresh metadata for the provider type. The current runtime still exposes configured credential keys as placeholder environment variables and resolves placeholders in outbound HTTP requests. +`discovery` controls what `--from-existing` scans when +`providers_v2_enabled=true`. Each entry in `discovery.credentials` must name a +credential declared under `credentials`. OpenShell scans the referenced +credential's `env_vars` in order and stores the first non-empty local +environment value under the actual environment variable key. + `endpoints` contains the same endpoint object shape as sandbox network policy. A profile can use access presets, protocol-specific allow rules, deny rules, WebSocket credential rewriting, request body credential rewriting, GraphQL fields, and SSRF IP allowlists. `binaries` contains the executable paths allowed to reach the profile endpoints when the profile contributes policy to a sandbox. @@ -269,15 +287,25 @@ openshell provider create \ --credential GITHUB_TOKEN ``` -Create a provider from local credentials discovered by the provider implementation: +Create a provider from local credentials discovered through the provider profile: ```shell openshell provider create \ --name work-claude \ - --type claude \ + --type claude-code \ --from-existing ``` +When `providers_v2_enabled=true`, `--from-existing` uses the provider profile's +`discovery` section. If no profile exists for the requested type, the command +fails instead of falling back to the legacy provider registry. When +`providers_v2_enabled=false`, `--from-existing` uses the legacy provider +registry and ignores profile `discovery` metadata. + +For example, with Providers v2 enabled, `--type openai --from-existing` +requires an imported `openai` profile with a `discovery` section. Setting +`OPENAI_API_KEY` alone is not enough for v2 profile discovery. + Create a provider from an imported custom profile: ```shell @@ -330,10 +358,14 @@ Create the provider instance first: ```shell openshell provider create \ --name my-graph \ - --type outlook \ + --type microsoft-graph-mail \ --credential MS_GRAPH_ACCESS_TOKEN ``` +This example assumes you imported a custom profile with +`id: microsoft-graph-mail`. Provider refresh can be configured only for provider +types whose profile declares compatible credential refresh metadata. + Configure OAuth2 client credentials refresh: ```shell @@ -376,6 +408,8 @@ openshell provider refresh configure drive-work \ --secret-material-key private_key ``` +This example assumes you imported a custom profile with `id: google-drive`. + `--secret-material-key` takes the name of a `--material` key, not the secret value. For example, use `--material client_secret="$MS_CLIENT_SECRET"` with `--secret-material-key client_secret`. The key should match a material entry so OpenShell can record that material field as sensitive when it stores refresh state. The gateway uses the material values to mint future access tokens, but only the `--credential-key` value, such as `MS_GRAPH_ACCESS_TOKEN`, becomes the injectable provider credential. If a refresh response rotates an OAuth refresh token, OpenShell stores the new `refresh_token` material and marks `refresh_token` as secret automatically. @@ -447,6 +481,10 @@ 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. +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. + List providers attached to a sandbox: ```shell @@ -475,7 +513,13 @@ endpoints: - host: api.github.com port: 443 protocol: rest - access: read-write + access: read-only + enforcement: enforce + - host: api.github.com + port: 443 + path: /graphql + protocol: graphql + access: read-only enforcement: enforce - host: github.com port: 443 @@ -506,7 +550,13 @@ network_policies: - host: api.github.com port: 443 protocol: rest - access: read-write + access: read-only + enforcement: enforce + - host: api.github.com + port: 443 + path: /graphql + protocol: graphql + access: read-only enforcement: enforce - host: github.com port: 443 diff --git a/proto/openshell.proto b/proto/openshell.proto index 10c69f414..90d1594f7 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -926,6 +926,12 @@ message ProviderCredentialRefreshStatus { string last_error = 9; } +// Provider profile local discovery declaration. +message ProviderProfileDiscovery { + // Credential names from ProviderProfile.credentials eligible for local discovery. + repeated string credentials = 1; +} + message StoredProviderCredentialRefreshState { openshell.datamodel.v1.ObjectMeta metadata = 1; string provider_id = 2; @@ -1007,6 +1013,7 @@ message ProviderProfile { repeated openshell.sandbox.v1.NetworkEndpoint endpoints = 6; repeated openshell.sandbox.v1.NetworkBinary binaries = 7; bool inference_capable = 8; + ProviderProfileDiscovery discovery = 9; } // Stored custom provider profile object. diff --git a/providers/claude-code.yaml b/providers/claude-code.yaml index b835f3d45..341853252 100644 --- a/providers/claude-code.yaml +++ b/providers/claude-code.yaml @@ -13,6 +13,8 @@ credentials: required: true auth_style: header header_name: x-api-key +discovery: + credentials: [api_key] endpoints: - host: api.anthropic.com port: 443 diff --git a/providers/github.yaml b/providers/github.yaml index cc24ae922..2be9fb2de 100644 --- a/providers/github.yaml +++ b/providers/github.yaml @@ -12,11 +12,19 @@ credentials: required: true auth_style: bearer header_name: authorization +discovery: + credentials: [api_token] endpoints: - host: api.github.com port: 443 protocol: rest - access: read-write + access: read-only + enforcement: enforce + - host: api.github.com + port: 443 + path: /graphql + protocol: graphql + access: read-only enforcement: enforce - host: github.com port: 443 diff --git a/providers/nvidia.yaml b/providers/nvidia.yaml index 42ea7f7df..d056a08de 100644 --- a/providers/nvidia.yaml +++ b/providers/nvidia.yaml @@ -13,6 +13,8 @@ credentials: required: true auth_style: bearer header_name: authorization +discovery: + credentials: [api_key] endpoints: - host: integrate.api.nvidia.com port: 443