diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index d2a35ca80..7eda661b4 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -1240,6 +1240,16 @@ pub fn validate_profile_set( message, )); } + if credential.token_grant.is_none() + && let Err(message) = validate_static_credential_header_name(credential) + { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.header_name", + message, + )); + } } for (index, endpoint) in profile.endpoints.iter().enumerate() { @@ -1542,6 +1552,19 @@ fn validate_token_grant_header_name(credential: &CredentialProfile) -> Result<() "" | "bearer" | "header" => credential.header_name.trim(), _ => return Ok(()), }; + validate_credential_header_name(header_name, "token_grant") +} + +fn validate_static_credential_header_name(credential: &CredentialProfile) -> Result<(), String> { + let header_name = match credential.auth_style.trim().to_ascii_lowercase().as_str() { + "bearer" if credential.header_name.trim().is_empty() => "Authorization", + "bearer" | "header" => credential.header_name.trim(), + _ => return Ok(()), + }; + validate_credential_header_name(header_name, "credential") +} + +fn validate_credential_header_name(header_name: &str, label: &str) -> Result<(), String> { if header_name.is_empty() { return Ok(()); } @@ -1566,13 +1589,14 @@ fn validate_token_grant_header_name(credential: &CredentialProfile) -> Result<() ) }); if !valid { - return Err("token_grant header_name is not a valid HTTP header name".to_string()); + return Err(format!( + "{label} header_name is not a valid HTTP header name" + )); } match header_name.to_ascii_lowercase().as_str() { - "host" | "content-length" | "transfer-encoding" | "connection" => Err( - "token_grant header_name may not override HTTP framing or connection headers" - .to_string(), - ), + "host" | "content-length" | "transfer-encoding" | "connection" => Err(format!( + "{label} header_name may not override HTTP framing or connection headers" + )), _ => Ok(()), } } @@ -2166,6 +2190,93 @@ credentials: ); } + #[test] + fn validate_profile_set_rejects_static_credential_framing_header_name() { + let profile = parse_profile_yaml( + r" +id: framing-header-static +display_name: Framing Header Static +credentials: + - name: api_token + env_vars: [API_TOKEN] + auth_style: header + header_name: Host +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("framing-static.yaml".to_string(), profile)]); + let diagnostic = diagnostics + .iter() + .find(|diagnostic| { + diagnostic.field == "credentials.header_name" + && diagnostic.message.contains("HTTP framing") + }) + .expect("framing header diagnostic should be reported for static credentials"); + + assert_eq!( + diagnostic.message, + "credential header_name may not override HTTP framing or connection headers" + ); + } + + #[test] + fn validate_profile_set_rejects_static_credential_invalid_header_name() { + let profile = parse_profile_yaml( + r" +id: invalid-header-static +display_name: Invalid Header Static +credentials: + - name: api_token + env_vars: [API_TOKEN] + auth_style: header + header_name: 'Invalid Header' +", + ) + .expect("profile should parse"); + + let diagnostics = + validate_profile_set(&[("invalid-header-static.yaml".to_string(), profile)]); + let diagnostic = diagnostics + .iter() + .find(|diagnostic| { + diagnostic.field == "credentials.header_name" + && diagnostic.message.contains("not a valid HTTP header name") + }) + .expect("invalid header name diagnostic should be reported for static credentials"); + + assert_eq!( + diagnostic.message, + "credential header_name is not a valid HTTP header name" + ); + } + + #[test] + fn validate_profile_set_accepts_static_credential_valid_header_name() { + let profile = parse_profile_yaml( + r" +id: valid-header-static +display_name: Valid Header Static +credentials: + - name: api_token + env_vars: [API_TOKEN] + auth_style: header + header_name: X-Api-Key +endpoints: + - host: api.example.com + port: 443 +", + ) + .expect("profile should parse"); + + let diagnostics = + validate_profile_set(&[("valid-header-static.yaml".to_string(), profile)]); + assert!( + diagnostics.is_empty(), + "valid static credential header should produce no diagnostics, got: {diagnostics:?}" + ); + } + #[test] fn validate_profile_set_rejects_ambiguous_same_credential_audience_overrides() { let profile = parse_profile_yaml( diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 2e2210f44..a192e87d1 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1219,8 +1219,12 @@ pub(super) async fn handle_get_sandbox_config( let settings = merge_effective_settings(&global_settings, &sandbox_settings)?; let config_revision = compute_config_revision(policy.as_ref(), &settings, policy_source); - let provider_env_revision = - compute_provider_env_revision(state.store.as_ref(), &sandbox_provider_names).await?; + let provider_env_revision = compute_provider_env_revision( + state.store.as_ref(), + &sandbox_provider_names, + providers_v2_enabled, + ) + .await?; Ok(Response::new(GetSandboxConfigResponse { policy, @@ -1237,9 +1241,11 @@ pub(super) async fn handle_get_sandbox_config( pub(super) async fn compute_provider_env_revision( store: &Store, provider_names: &[String], + providers_v2_enabled: bool, ) -> Result { let mut hasher = Sha256::new(); hasher.update(b"openshell-provider-env-revision-v1"); + hasher.update([u8::from(providers_v2_enabled)]); for provider_name in provider_names { hasher.update(provider_name.as_bytes()); @@ -1366,6 +1372,16 @@ async fn profile_provider_policy_layers( Ok(layers) } +pub async fn is_providers_v2_enabled(store: &Store) -> bool { + load_global_settings(store) + .await + .and_then(|s| bool_setting_enabled(&s, settings::PROVIDERS_V2_ENABLED_KEY)) + .unwrap_or_else(|e| { + warn!("failed to read providers_v2_enabled setting, defaulting to false: {e}"); + false + }) +} + fn bool_setting_enabled(settings: &StoredSettings, key: &str) -> Result { match settings.settings.get(key) { None => Ok(false), @@ -1407,13 +1423,20 @@ pub(super) async fn handle_get_sandbox_provider_environment( .spec .ok_or_else(|| Status::internal("sandbox has no spec"))?; + let providers_v2_enabled = is_providers_v2_enabled(state.store.as_ref()).await; + let provider_names = spec.providers; let provider_env_revision = - compute_provider_env_revision(state.store.as_ref(), &provider_names).await?; - let provider_environment = - super::provider::resolve_provider_environment(state.store.as_ref(), &provider_names) + compute_provider_env_revision(state.store.as_ref(), &provider_names, providers_v2_enabled) .await?; + let provider_environment = super::provider::resolve_provider_environment( + state.store.as_ref(), + &provider_names, + providers_v2_enabled, + ) + .await?; + info!( sandbox_id = %sandbox_id, provider_count = provider_names.len(), @@ -5001,10 +5024,13 @@ mod tests { .await .unwrap(); - let first = - compute_provider_env_revision(state.store.as_ref(), &["work-custom-token".to_string()]) - .await - .unwrap(); + let first = compute_provider_env_revision( + state.store.as_ref(), + &["work-custom-token".to_string()], + false, + ) + .await + .unwrap(); tokio::time::sleep(Duration::from_millis(2)).await; state @@ -5015,10 +5041,13 @@ mod tests { .await .unwrap(); - let second = - compute_provider_env_revision(state.store.as_ref(), &["work-custom-token".to_string()]) - .await - .unwrap(); + let second = compute_provider_env_revision( + state.store.as_ref(), + &["work-custom-token".to_string()], + false, + ) + .await + .unwrap(); assert_ne!( first, second, @@ -5026,6 +5055,29 @@ mod tests { ); } + #[tokio::test] + async fn provider_env_revision_changes_when_providers_v2_enabled_toggles() { + let state = test_server_state().await; + let store = state.store.as_ref(); + let provider = test_provider("work-github", "github"); + store.put_message(&provider).await.unwrap(); + + let revision_v2_off = + compute_provider_env_revision(store, &["work-github".to_string()], false) + .await + .unwrap(); + + let revision_v2_on = + compute_provider_env_revision(store, &["work-github".to_string()], true) + .await + .unwrap(); + + assert_ne!( + revision_v2_off, revision_v2_on, + "toggling providers_v2_enabled must change provider_env_revision so running sandboxes refresh" + ); + } + #[tokio::test] async fn sandbox_config_and_provider_env_follow_attached_provider_lifecycle() { use crate::grpc::sandbox::{ diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 641118206..661dfd606 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -5,6 +5,7 @@ #![allow(clippy::result_large_err)] // gRPC handlers return Result, Status> +use crate::grpc::policy::is_providers_v2_enabled; use crate::persistence::{ ObjectId, ObjectLabels, ObjectName, ObjectType, Store, WriteCondition, generate_name, }; @@ -222,6 +223,9 @@ pub(super) async fn update_provider_record( provider.credential_expires_at_ms, ); + // check the providers_v2_enabled flag, which affects provider validation + let providers_v2_enabled = is_providers_v2_enabled(store).await; + // Validate BEFORE writing to prevent persisting invalid state. // Validate only the mutable fields (credentials/config) plus metadata and // attached-sandbox invariants. The immutable name/type are carried forward @@ -230,7 +234,8 @@ pub(super) async fn update_provider_record( // #1347. super::validation::validate_object_metadata(candidate.metadata.as_ref(), "provider")?; validate_provider_mutable_fields(&candidate)?; - validate_provider_update_against_attached_sandboxes(store, &candidate).await?; + validate_provider_update_against_attached_sandboxes(store, &candidate, providers_v2_enabled) + .await?; // Serialize labels for storage let labels_map = candidate.object_labels(); @@ -434,6 +439,7 @@ fn merge_i64_map( pub(super) async fn resolve_provider_environment( store: &Store, provider_names: &[String], + providers_v2_enabled: bool, ) -> Result { if provider_names.is_empty() { return Ok(ProviderEnvironment::default()); @@ -442,7 +448,14 @@ pub(super) async fn resolve_provider_environment( let mut env = std::collections::HashMap::new(); let mut expires = std::collections::HashMap::new(); let now_ms = crate::persistence::current_time_ms(); - validate_provider_environment_keys_unique_at(store, provider_names, None, now_ms).await?; + validate_provider_environment_keys_unique_at( + store, + provider_names, + None, + now_ms, + providers_v2_enabled, + ) + .await?; for name in provider_names { let provider = store @@ -539,7 +552,12 @@ pub(super) async fn resolve_provider_environment( Ok(ProviderEnvironment { environment: env, credential_expires_at_ms: expires, - dynamic_credentials: resolve_dynamic_credentials(store, provider_names).await?, + dynamic_credentials: resolve_dynamic_credentials( + store, + provider_names, + providers_v2_enabled, + ) + .await?, }) } @@ -551,6 +569,7 @@ pub(super) async fn resolve_provider_environment( pub(super) async fn resolve_dynamic_credentials( store: &Store, provider_names: &[String], + providers_v2_enabled: bool, ) -> Result, Status> { if provider_names.is_empty() { return Ok(std::collections::HashMap::new()); @@ -579,6 +598,7 @@ pub(super) async fn resolve_dynamic_credentials( &mut dynamic_creds, &profile.to_proto(), provider_name, + providers_v2_enabled, ); } @@ -589,9 +609,10 @@ fn insert_dynamic_credentials_for_profile( dynamic_creds: &mut std::collections::HashMap, profile: &ProviderProfile, provider_name: &str, + providers_v2_enabled: bool, ) { for credential in &profile.credentials { - if credential.token_grant.is_none() { + if !can_inject_credential(credential, providers_v2_enabled) { continue; } for endpoint in &profile.endpoints { @@ -861,12 +882,14 @@ fn endpoint_path_matches(pattern: &str, path: &str) -> bool { pub async fn validate_provider_environment_keys_unique( store: &Store, provider_names: &[String], + providers_v2_enabled: bool, ) -> Result<(), Status> { validate_provider_environment_keys_unique_at( store, provider_names, None, crate::persistence::current_time_ms(), + providers_v2_enabled, ) .await } @@ -875,6 +898,7 @@ pub async fn validate_provider_credential_key_available_for_attached_sandboxes( store: &Store, provider: &Provider, credential_key: &str, + providers_v2_enabled: bool, ) -> Result<(), Status> { let mut candidate = provider.clone(); candidate @@ -882,12 +906,14 @@ pub async fn validate_provider_credential_key_available_for_attached_sandboxes( .entry(credential_key.to_string()) .or_insert_with(|| "pending".to_string()); candidate.credential_expires_at_ms.remove(credential_key); - validate_provider_update_against_attached_sandboxes(store, &candidate).await + validate_provider_update_against_attached_sandboxes(store, &candidate, providers_v2_enabled) + .await } pub async fn validate_provider_update_against_attached_sandboxes( store: &Store, provider: &Provider, + providers_v2_enabled: bool, ) -> Result<(), Status> { let provider_name = provider.object_name().to_string(); for sandbox in sandboxes_using_provider_records(store, &provider_name).await? { @@ -900,6 +926,7 @@ pub async fn validate_provider_update_against_attached_sandboxes( &spec.providers, Some(provider), crate::persistence::current_time_ms(), + providers_v2_enabled, ) .await .map_err(|err| { @@ -917,6 +944,7 @@ async fn validate_provider_environment_keys_unique_at( provider_names: &[String], candidate_provider: Option<&Provider>, now_ms: i64, + providers_v2_enabled: bool, ) -> Result<(), Status> { let mut seen = std::collections::HashMap::::new(); let mut dynamic_bindings = Vec::new(); @@ -943,14 +971,16 @@ async fn validate_provider_environment_keys_unique_at( seen.insert(key, provider_name.clone()); } } - dynamic_bindings.extend(dynamic_token_grant_bindings_for_provider(store, &provider).await?); + dynamic_bindings.extend( + credential_bindings_for_provider(store, &provider, providers_v2_enabled).await?, + ); } - validate_dynamic_token_grant_bindings_unambiguous(&dynamic_bindings)?; + validate_credential_bindings_unambiguous(&dynamic_bindings)?; Ok(()) } #[derive(Debug, Clone, PartialEq, Eq)] -struct DynamicTokenGrantBinding { +struct CredentialBinding { provider_name: String, credential_name: String, host: String, @@ -959,33 +989,36 @@ struct DynamicTokenGrantBinding { score: u32, } -async fn dynamic_token_grant_bindings_for_provider( +async fn credential_bindings_for_provider( store: &Store, provider: &Provider, -) -> Result, Status> { + providers_v2_enabled: bool, +) -> Result, Status> { let provider_name = provider.object_name().to_string(); let profile_id = normalize_provider_type(&provider.r#type).unwrap_or(provider.r#type.as_str()); let Some(profile) = get_provider_type_profile(store, profile_id).await? else { return Ok(Vec::new()); }; - Ok(dynamic_token_grant_bindings_for_profile( + Ok(credential_bindings_for_profile( &provider_name, &profile.to_proto(), + providers_v2_enabled, )) } -fn dynamic_token_grant_bindings_for_profile( +fn credential_bindings_for_profile( provider_name: &str, profile: &ProviderProfile, -) -> Vec { + providers_v2_enabled: bool, +) -> Vec { let mut bindings = Vec::new(); for credential in &profile.credentials { - if credential.token_grant.is_none() { + if !can_inject_credential(credential, providers_v2_enabled) { continue; } for endpoint in &profile.endpoints { for port in endpoint_ports(endpoint.port, &endpoint.ports) { - push_dynamic_token_grant_bindings_for_endpoint( + push_credential_bindings_for_endpoint( &mut bindings, provider_name, credential, @@ -999,15 +1032,15 @@ fn dynamic_token_grant_bindings_for_profile( bindings } -fn push_dynamic_token_grant_bindings_for_endpoint( - bindings: &mut Vec, +fn push_credential_bindings_for_endpoint( + bindings: &mut Vec, provider_name: &str, credential: &ProviderProfileCredential, endpoint_host: &str, endpoint_port: u32, endpoint_path: &str, ) { - push_dynamic_token_grant_binding( + push_credential_binding( bindings, provider_name, &credential.name, @@ -1039,7 +1072,7 @@ fn push_dynamic_token_grant_bindings_for_endpoint( } else { override_config.path.as_str() }; - push_dynamic_token_grant_binding( + push_credential_binding( bindings, provider_name, &credential.name, @@ -1050,15 +1083,15 @@ fn push_dynamic_token_grant_bindings_for_endpoint( } } -fn push_dynamic_token_grant_binding( - bindings: &mut Vec, +fn push_credential_binding( + bindings: &mut Vec, provider_name: &str, credential_name: &str, host: &str, port: u32, path: &str, ) { - let candidate = DynamicTokenGrantBinding { + let candidate = CredentialBinding { provider_name: provider_name.to_string(), credential_name: credential_name.to_string(), host: host.to_ascii_lowercase(), @@ -1071,9 +1104,7 @@ fn push_dynamic_token_grant_binding( } } -fn validate_dynamic_token_grant_bindings_unambiguous( - bindings: &[DynamicTokenGrantBinding], -) -> Result<(), Status> { +fn validate_credential_bindings_unambiguous(bindings: &[CredentialBinding]) -> Result<(), Status> { for (index, first) in bindings.iter().enumerate() { for second in bindings.iter().skip(index + 1) { if first.provider_name == second.provider_name @@ -1087,7 +1118,7 @@ fn validate_dynamic_token_grant_bindings_unambiguous( && path_patterns_can_overlap(&first.path, &second.path) { return Err(Status::failed_precondition(format!( - "dynamic token grants for '{}:{}' and '{}:{}' are ambiguous for {}:{} path selectors '{}' and '{}'; make one host/path selector more specific or attach only one matching provider", + "credentials for '{}:{}' and '{}:{}' are ambiguous for {}:{} path selectors '{}' and '{}'; make one host/path selector more specific or attach only one matching provider", first.provider_name, first.credential_name, second.provider_name, @@ -1156,6 +1187,14 @@ pub(super) fn is_valid_env_key(key: &str) -> bool { bytes.all(|byte| byte == b'_' || byte.is_ascii_alphanumeric()) } +// Which static cred auth styles we currently support injecting into the sandbox +const INJECTABLE_STATIC_AUTH_STYLES: &[&str] = &["bearer", "header"]; + +fn can_inject_credential(cred: &ProviderProfileCredential, providers_v2_enabled: bool) -> bool { + cred.token_grant.is_some() + || (providers_v2_enabled && INJECTABLE_STATIC_AUTH_STYLES.contains(&cred.auth_style.trim())) +} + // --------------------------------------------------------------------------- // Trait impls for persistence // --------------------------------------------------------------------------- @@ -1599,6 +1638,9 @@ async fn profile_import_attached_sandbox_diagnostics( .then_some(sandbox) }) .await?; + + let providers_v2_enabled = is_providers_v2_enabled(store).await; + let mut diagnostics = Vec::new(); for sandbox in sandboxes { let sandbox_name = sandbox.object_name().to_string(); @@ -1617,23 +1659,27 @@ async fn profile_import_attached_sandbox_diagnostics( let profile_id = normalize_provider_type(&provider.r#type).unwrap_or(provider.r#type.as_str()); if let Some((source, profile)) = candidate_profiles.get(profile_id) { - bindings.extend(dynamic_token_grant_bindings_for_profile( + bindings.extend(credential_bindings_for_profile( provider.object_name(), profile, + providers_v2_enabled, )); let used = (source.clone(), profile_id.to_string()); if !imported_profiles_used.contains(&used) { imported_profiles_used.push(used); } } else { - bindings.extend(dynamic_token_grant_bindings_for_provider(store, &provider).await?); + bindings.extend( + credential_bindings_for_provider(store, &provider, providers_v2_enabled) + .await?, + ); } } if imported_profiles_used.is_empty() { continue; } - if let Err(err) = validate_dynamic_token_grant_bindings_unambiguous(&bindings) { + if let Err(err) = validate_credential_bindings_unambiguous(&bindings) { for (source, profile_id) in &imported_profiles_used { diagnostics.push(ProfileValidationDiagnostic { source: source.clone(), @@ -1888,10 +1934,13 @@ pub(super) async fn handle_configure_provider_refresh( .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found"))?; + + let providers_v2_enabled = is_providers_v2_enabled(state.store.as_ref()).await; validate_provider_credential_key_available_for_attached_sandboxes( state.store.as_ref(), &provider, credential_key, + providers_v2_enabled, ) .await?; let refresh_defaults = @@ -2298,7 +2347,7 @@ mod tests { }; let mut dynamic_creds = HashMap::new(); - insert_dynamic_credentials_for_profile(&mut dynamic_creds, &profile, "keycloak"); + insert_dynamic_credentials_for_profile(&mut dynamic_creds, &profile, "keycloak", false); assert_eq!(dynamic_creds.len(), 4); for (host, audience) in service_audiences { @@ -2310,6 +2359,189 @@ mod tests { } } + #[test] + fn static_credentials_included_when_flag_enabled() { + let credential = ProviderProfileCredential { + name: "api_token".to_string(), + env_vars: vec!["GITHUB_TOKEN".to_string(), "GH_TOKEN".to_string()], + auth_style: "bearer".to_string(), + header_name: "Authorization".to_string(), + token_grant: None, + ..Default::default() + }; + let profile = ProviderProfile { + id: "github".to_string(), + display_name: "GitHub".to_string(), + description: String::new(), + category: ProviderProfileCategory::SourceControl as i32, + credentials: vec![credential], + endpoints: vec![ + NetworkEndpoint { + host: "api.github.com".to_string(), + port: 443, + ..Default::default() + }, + NetworkEndpoint { + host: "github.com".to_string(), + port: 443, + ..Default::default() + }, + ], + binaries: Vec::new(), + inference_capable: false, + discovery: None, + }; + + // With flag off, no static credentials emitted. + let mut creds_off = HashMap::new(); + insert_dynamic_credentials_for_profile(&mut creds_off, &profile, "my-github", false); + assert!( + creds_off.is_empty(), + "static credentials should be skipped when flag is false" + ); + + // With flag on, static credentials are emitted for each endpoint. + let mut creds_on = HashMap::new(); + insert_dynamic_credentials_for_profile(&mut creds_on, &profile, "my-github", true); + assert_eq!(creds_on.len(), 2, "one entry per endpoint expected"); + + let api_key = dynamic_credential_key("api.github.com", 443, "", "my-github", "api_token"); + let web_key = dynamic_credential_key("github.com", 443, "", "my-github", "api_token"); + let api_cred = &creds_on[&api_key]; + assert_eq!(api_cred.auth_style, "bearer"); + assert_eq!(api_cred.env_vars, vec!["GITHUB_TOKEN", "GH_TOKEN"]); + assert!(api_cred.token_grant.is_none()); + assert!(creds_on.contains_key(&web_key)); + } + + #[test] + fn static_credentials_flag_does_not_affect_token_grants() { + let credential = ProviderProfileCredential { + name: "access_token".to_string(), + env_vars: Vec::new(), + auth_style: "bearer".to_string(), + header_name: "Authorization".to_string(), + token_grant: Some(ProviderCredentialTokenGrant { + token_endpoint: "https://auth.example.com/token".to_string(), + audience: "api://default".to_string(), + jwt_svid_audience: "https://auth.example.com".to_string(), + client_assertion_type: "urn:ietf:params:oauth:client-assertion-type:jwt-bearer" + .to_string(), + scopes: vec!["read".to_string()], + cache_ttl_seconds: 300, + audience_overrides: Vec::new(), + }), + ..Default::default() + }; + let profile = ProviderProfile { + id: "example".to_string(), + display_name: "Example".to_string(), + description: String::new(), + category: ProviderProfileCategory::Other as i32, + credentials: vec![credential], + endpoints: vec![NetworkEndpoint { + host: "api.example.com".to_string(), + port: 443, + ..Default::default() + }], + binaries: Vec::new(), + inference_capable: false, + discovery: None, + }; + + let mut creds_off = HashMap::new(); + insert_dynamic_credentials_for_profile(&mut creds_off, &profile, "example", false); + let mut creds_on = HashMap::new(); + insert_dynamic_credentials_for_profile(&mut creds_on, &profile, "example", true); + + assert_eq!( + creds_off.len(), + 1, + "token grant should be emitted regardless of flag" + ); + assert_eq!(creds_off.len(), creds_on.len()); + let key = dynamic_credential_key("api.example.com", 443, "", "example", "access_token"); + assert!(creds_off[&key].token_grant.is_some()); + assert!(creds_on[&key].token_grant.is_some()); + } + + #[test] + fn multi_credential_profile_without_auth_style_not_injected() { + // Simulates a Codex-like profile: multiple credentials, none with auth_style. + let credentials = vec![ + ProviderProfileCredential { + name: "access_token".to_string(), + env_vars: vec!["CODEX_AUTH_ACCESS_TOKEN".to_string()], + ..Default::default() + }, + ProviderProfileCredential { + name: "refresh_token".to_string(), + env_vars: vec!["CODEX_AUTH_REFRESH_TOKEN".to_string()], + ..Default::default() + }, + ProviderProfileCredential { + name: "account_id".to_string(), + env_vars: vec!["CODEX_AUTH_ACCOUNT_ID".to_string()], + ..Default::default() + }, + ]; + let profile = ProviderProfile { + id: "codex-like".to_string(), + display_name: "Codex-like".to_string(), + credentials, + endpoints: vec![ + NetworkEndpoint { + host: "api.openai.com".to_string(), + port: 443, + ..Default::default() + }, + NetworkEndpoint { + host: "auth.openai.com".to_string(), + port: 443, + ..Default::default() + }, + ], + ..Default::default() + }; + + let mut creds = HashMap::new(); + insert_dynamic_credentials_for_profile(&mut creds, &profile, "codex-like", true); + + assert!( + creds.is_empty(), + "credentials without explicit auth_style should not be injected even with v2 enabled" + ); + } + + async fn import_static_credential_profile( + state: &Arc, + id: &str, + host: &str, + port: u32, + path: &str, + ) { + let mut profile = custom_profile(id); + profile.credentials = vec![static_credential("api_token", "API_TOKEN", true)]; + profile.endpoints = vec![NetworkEndpoint { + host: host.to_string(), + port, + path: path.to_string(), + protocol: "rest".to_string(), + ..Default::default() + }]; + handle_import_provider_profiles( + state, + Request::new(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(profile), + source: format!("{id}.yaml"), + }], + }), + ) + .await + .unwrap(); + } + async fn import_token_grant_profile( state: &Arc, id: &str, @@ -2364,6 +2596,34 @@ mod tests { .unwrap() } + async fn create_static_credential_provider( + store: &Store, + name: &str, + provider_type: &str, + ) -> Provider { + // Use a provider-unique credential key so the env key uniqueness check + // does not fire before we reach the binding ambiguity check. + let credential_key = format!("{}_TOKEN", name.to_ascii_uppercase().replace('-', "_")); + create_provider_record( + store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: name.to_string(), + created_at_ms: 1_000_000, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: provider_type.to_string(), + credentials: std::iter::once((credential_key, "secret".to_string())).collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap() + } + #[tokio::test] async fn dynamic_token_grants_reject_equal_specificity_overlap() { let state = test_server_state().await; @@ -2376,12 +2636,13 @@ mod tests { let err = validate_provider_environment_keys_unique( store, &["provider-a".to_string(), "provider-b".to_string()], + false, ) .await .expect_err("equal-specificity dynamic grants should be ambiguous"); assert_eq!(err.code(), Code::FailedPrecondition); - assert!(err.message().contains("dynamic token grants")); + assert!(err.message().contains("credential")); assert!(err.message().contains("ambiguous")); } @@ -2404,11 +2665,83 @@ mod tests { validate_provider_environment_keys_unique( store, &["provider-default".to_string(), "provider-admin".to_string()], + false, ) .await .expect("more-specific path should make dynamic grants deterministic"); } + #[tokio::test] + async fn static_credentials_reject_equal_specificity_overlap() { + let state = test_server_state().await; + let store = state.store.as_ref(); + import_static_credential_profile(&state, "static-a", "api.example.com", 443, "").await; + import_static_credential_profile(&state, "static-b", "api.example.com", 443, "").await; + create_static_credential_provider(store, "provider-static-a", "static-a").await; + create_static_credential_provider(store, "provider-static-b", "static-b").await; + + let err = validate_provider_environment_keys_unique( + store, + &[ + "provider-static-a".to_string(), + "provider-static-b".to_string(), + ], + true, + ) + .await + .expect_err("equal-specificity static credentials should be ambiguous"); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("credential")); + assert!(err.message().contains("ambiguous")); + } + + #[tokio::test] + async fn static_credentials_not_checked_when_v2_disabled() { + let state = test_server_state().await; + let store = state.store.as_ref(); + import_static_credential_profile(&state, "static-c", "api.example.com", 443, "").await; + import_static_credential_profile(&state, "static-d", "api.example.com", 443, "").await; + create_static_credential_provider(store, "provider-static-c", "static-c").await; + create_static_credential_provider(store, "provider-static-d", "static-d").await; + + validate_provider_environment_keys_unique( + store, + &[ + "provider-static-c".to_string(), + "provider-static-d".to_string(), + ], + false, + ) + .await + .expect("static credential overlap should not be checked when v2 is disabled"); + } + + #[tokio::test] + async fn static_vs_token_grant_reject_equal_specificity_overlap() { + let state = test_server_state().await; + let store = state.store.as_ref(); + import_static_credential_profile(&state, "static-mixed", "api.example.com", 443, "/v1/**") + .await; + import_token_grant_profile(&state, "grant-mixed", "api.example.com", 443, "/v1/**").await; + create_static_credential_provider(store, "provider-static-mixed", "static-mixed").await; + create_empty_token_grant_provider(store, "provider-grant-mixed", "grant-mixed").await; + + let err = validate_provider_environment_keys_unique( + store, + &[ + "provider-static-mixed".to_string(), + "provider-grant-mixed".to_string(), + ], + true, + ) + .await + .expect_err("static vs token grant at equal specificity should be ambiguous"); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("ambiguous")); + } + #[tokio::test] async fn import_provider_profile_rejects_attached_dynamic_binding_ambiguity() { let state = test_server_state().await; @@ -4436,7 +4769,9 @@ mod tests { #[tokio::test] async fn resolve_provider_env_empty_list_returns_empty() { let store = test_store().await; - let result = resolve_provider_environment(&store, &[]).await.unwrap(); + let result = resolve_provider_environment(&store, &[], false) + .await + .unwrap(); assert!(result.is_empty()); } @@ -4467,7 +4802,7 @@ mod tests { }; create_provider_record(&store, provider).await.unwrap(); - let result = resolve_provider_environment(&store, &["claude-local".to_string()]) + let result = resolve_provider_environment(&store, &["claude-local".to_string()], false) .await .unwrap(); assert_eq!(result.get("ANTHROPIC_API_KEY"), Some(&"sk-abc".to_string())); @@ -4485,7 +4820,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["static-provider".to_string()]) + let result = resolve_provider_environment(&store, &["static-provider".to_string()], false) .await .unwrap(); @@ -4522,9 +4857,10 @@ mod tests { }; create_provider_record(&store, provider).await.unwrap(); - let result = resolve_provider_environment(&store, &["expiring-provider".to_string()]) - .await - .unwrap(); + let result = + resolve_provider_environment(&store, &["expiring-provider".to_string()], false) + .await + .unwrap(); assert_eq!(result.get("FRESH_TOKEN"), Some(&"fresh".to_string())); assert!(!result.contains_key("STALE_TOKEN")); assert_eq!( @@ -4536,7 +4872,7 @@ mod tests { #[tokio::test] async fn resolve_provider_env_unknown_name_returns_error() { let store = test_store().await; - let err = resolve_provider_environment(&store, &["nonexistent".to_string()]) + let err = resolve_provider_environment(&store, &["nonexistent".to_string()], false) .await .unwrap_err(); assert_eq!(err.code(), Code::FailedPrecondition); @@ -4567,7 +4903,7 @@ mod tests { }; create_provider_record(&store, provider).await.unwrap(); - let result = resolve_provider_environment(&store, &["test-provider".to_string()]) + let result = resolve_provider_environment(&store, &["test-provider".to_string()], false) .await .unwrap(); assert_eq!(result.get("VALID_KEY"), Some(&"value".to_string())); @@ -4623,6 +4959,7 @@ mod tests { let result = resolve_provider_environment( &store, &["claude-local".to_string(), "gitlab-local".to_string()], + false, ) .await .unwrap(); @@ -4678,6 +5015,7 @@ mod tests { let err = resolve_provider_environment( &store, &["provider-a".to_string(), "provider-b".to_string()], + false, ) .await .unwrap_err(); @@ -4721,7 +5059,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-local".to_string()]) + let result = resolve_provider_environment(&store, &["vertex-local".to_string()], false) .await .unwrap(); @@ -4794,7 +5132,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-bootstrap".to_string()]) + let result = resolve_provider_environment(&store, &["vertex-bootstrap".to_string()], false) .await .unwrap(); @@ -4831,7 +5169,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-no-config".to_string()]) + let result = resolve_provider_environment(&store, &["vertex-no-config".to_string()], false) .await .unwrap(); @@ -4889,7 +5227,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-collision".to_string()]) + let result = resolve_provider_environment(&store, &["vertex-collision".to_string()], false) .await .unwrap(); @@ -4923,7 +5261,7 @@ mod tests { .await .unwrap(); - let result = resolve_provider_environment(&store, &["openai-local".to_string()]) + let result = resolve_provider_environment(&store, &["openai-local".to_string()], false) .await .unwrap(); @@ -5081,7 +5419,7 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let env = resolve_provider_environment(&store, &spec.providers, false) .await .unwrap(); @@ -5114,7 +5452,7 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let env = resolve_provider_environment(&store, &spec.providers, false) .await .unwrap(); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index e60ce3995..39ca1c9a8 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -10,6 +10,7 @@ #![allow(clippy::cast_possible_wrap)] // Intentional u32->i32 conversions for proto compat use crate::ServerState; +use crate::grpc::policy::is_providers_v2_enabled; use crate::persistence::{ObjectType, WriteCondition, generate_name}; use futures::future; use openshell_core::proto::{ @@ -141,7 +142,13 @@ 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(), &spec.providers).await?; + let providers_v2_enabled = is_providers_v2_enabled(state.store.as_ref()).await; + validate_provider_environment_keys_unique( + state.store.as_ref(), + &spec.providers, + providers_v2_enabled, + ) + .await?; // Ensure the template always carries the resolved image. let mut spec = spec; @@ -347,8 +354,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(state.store.as_ref(), &candidate_spec.providers) - .await?; + let providers_v2_enabled = is_providers_v2_enabled(state.store.as_ref()).await; + validate_provider_environment_keys_unique( + state.store.as_ref(), + &candidate_spec.providers, + providers_v2_enabled, + ) + .await?; let provider_name = request.provider_name.clone(); let attached = Arc::new(AtomicBool::new(false)); diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index b0b9a927c..bea7bdc41 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -414,8 +414,13 @@ async fn apply_minted_credential( } else { updated.credential_expires_at_ms.remove(credential_key); } - crate::grpc::provider::validate_provider_update_against_attached_sandboxes(store, &updated) - .await?; + let providers_v2_enabled = crate::grpc::policy::is_providers_v2_enabled(store).await; + crate::grpc::provider::validate_provider_update_against_attached_sandboxes( + store, + &updated, + providers_v2_enabled, + ) + .await?; store .update_message_cas::(provider.object_id(), 0, |current| { current diff --git a/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs b/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs index 0d7c18e99..eb931d7cc 100644 --- a/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs +++ b/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs @@ -65,95 +65,192 @@ pub fn default_resolver() -> Arc { /// Checks for endpoint-bound token grant credentials and injects an /// Authorization header before forwarding the request upstream. pub async fn inject_if_needed(req: L7Request, ctx: &L7EvalContext) -> Result { - let request_path = req.target.split('?').next().unwrap_or(req.target.as_str()); - let token_grant_credential = ctx.dynamic_credentials.as_ref().and_then(|dyn_creds| { + let request_path = req + .target + .split('?') + .next() + .unwrap_or(req.target.as_str()) + .to_string(); + let matched_credential = ctx.dynamic_credentials.as_ref().and_then(|dyn_creds| { dyn_creds.read().map_or(None, |creds_guard| { creds_guard .iter() .filter_map(|(key, cred)| { - let score = - dynamic_credential_key_match_score(key, &ctx.host, ctx.port, request_path)?; - cred.token_grant - .is_some() - .then(|| (score, key.clone(), cred.clone())) + let score = dynamic_credential_key_match_score( + key, + &ctx.host, + ctx.port, + &request_path, + )?; + Some((score, key.clone(), cred.clone())) }) .max_by_key(|(score, key, _)| (*score, key.clone())) .map(|(_, key, cred)| (key, cred)) }) }); - if let Some((provider_key, cred)) = token_grant_credential - && let Some(ref token_grant) = cred.token_grant - { - let resolver = ctx - .token_grant_resolver - .as_ref() - .ok_or_else(|| miette!("token grant resolver unavailable"))?; - let request = token_grant_request(&provider_key, token_grant); - - match resolver.obtain(request).await { - Ok(access_token) => { - let modified_raw_header = - inject_token_grant_header(&req.raw_header, &cred, &access_token)?; - let provider_key = ocsf_message_field(&provider_key); - ocsf_emit!( - HttpActivityBuilder::new(ocsf_ctx()) - .activity(ActivityId::Other) - .action(ActionId::Allowed) - .disposition(DispositionId::Allowed) - .severity(SeverityId::Informational) - .http_request(HttpRequest::new( - &req.action, - OcsfUrl::new("http", &ctx.host, request_path, ctx.port), - )) - .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) - .message(format!( - "Token grant successful for {} to {}:{}", - provider_key, ctx.host, ctx.port - )) - .build() - ); - return Ok(L7Request { - action: req.action, - target: req.target, - query_params: req.query_params, - raw_header: modified_raw_header, - body_length: req.body_length, - }); - } - Err(e) => { - warn!( - host = %ctx.host, - port = ctx.port, - provider = %provider_key, - error = %e, - "Token grant failed" - ); - let provider_key = ocsf_message_field(&provider_key); - ocsf_emit!( - HttpActivityBuilder::new(ocsf_ctx()) - .activity(ActivityId::Fail) - .action(ActionId::Denied) - .disposition(DispositionId::Blocked) - .severity(SeverityId::Medium) - .status(StatusId::Failure) - .http_request(HttpRequest::new( - &req.action, - OcsfUrl::new("http", &ctx.host, request_path, ctx.port), - )) - .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) - .message(format!( - "Token grant failed for {} to {}:{}: {}", - provider_key, ctx.host, ctx.port, e - )) - .build() - ); - return Err(miette!("Token grant failed: {}", e)); - } + if let Some((provider_key, cred)) = matched_credential { + if let Some(ref token_grant) = cred.token_grant { + inject_token_grant(req, ctx, &request_path, &provider_key, token_grant, &cred).await + } else { + inject_static_credential(req, ctx, &request_path, &provider_key, &cred) } + } else { + Ok(req) } +} - Ok(req) +async fn inject_token_grant( + req: L7Request, + ctx: &L7EvalContext, + request_path: &str, + provider_key: &str, + token_grant: &ProviderCredentialTokenGrant, + cred: &ProviderProfileCredential, +) -> Result { + let resolver = ctx + .token_grant_resolver + .as_ref() + .ok_or_else(|| miette!("token grant resolver unavailable"))?; + let request = token_grant_request(provider_key, token_grant); + + match resolver.obtain(request).await { + Ok(access_token) => { + let modified_raw_header = + inject_token_grant_header(&req.raw_header, cred, &access_token)?; + let provider_key = ocsf_message_field(provider_key); + ocsf_emit!( + HttpActivityBuilder::new(ocsf_ctx()) + .activity(ActivityId::Other) + .action(ActionId::Allowed) + .disposition(DispositionId::Allowed) + .severity(SeverityId::Informational) + .http_request(HttpRequest::new( + &req.action, + OcsfUrl::new("http", &ctx.host, request_path, ctx.port), + )) + .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) + .message(format!( + "Token grant successful for {} to {}:{}", + provider_key, ctx.host, ctx.port + )) + .build() + ); + Ok(L7Request { + action: req.action, + target: req.target, + query_params: req.query_params, + raw_header: modified_raw_header, + body_length: req.body_length, + }) + } + Err(e) => { + warn!( + host = %ctx.host, + port = ctx.port, + provider = %provider_key, + error = %e, + "Token grant failed" + ); + let provider_key = ocsf_message_field(provider_key); + ocsf_emit!( + HttpActivityBuilder::new(ocsf_ctx()) + .activity(ActivityId::Fail) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .http_request(HttpRequest::new( + &req.action, + OcsfUrl::new("http", &ctx.host, request_path, ctx.port), + )) + .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) + .message(format!( + "Token grant failed for {} to {}:{}: {}", + provider_key, ctx.host, ctx.port, e + )) + .build() + ); + Err(miette!("Token grant failed: {}", e)) + } + } +} + +fn inject_static_credential( + req: L7Request, + ctx: &L7EvalContext, + request_path: &str, + provider_key: &str, + cred: &ProviderProfileCredential, +) -> Result { + let Some(value) = cred.env_vars.iter().find_map(|env_var| { + let placeholder = format!( + "{}{env_var}", + openshell_core::secrets::PLACEHOLDER_PREFIX_PUBLIC + ); + ctx.secret_resolver + .as_ref() + .and_then(|r| r.resolve_placeholder(&placeholder)) + }) else { + let provider_key = ocsf_message_field(provider_key); + ocsf_emit!( + HttpActivityBuilder::new(ocsf_ctx()) + .activity(ActivityId::Fail) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .http_request(HttpRequest::new( + &req.action, + OcsfUrl::new("http", &ctx.host, request_path, ctx.port), + )) + .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) + .message(format!( + "No credential found for {} on provider {}", + cred.name, provider_key + )) + .build() + ); + return Err(miette!( + "no credential value found for credential '{}' on provider {}", + cred.name, + provider_key, + )); + }; + match cred.auth_style.trim().to_ascii_lowercase().as_str() { + "bearer" | "header" => { + let modified_raw_header = inject_token_grant_header(&req.raw_header, cred, value)?; + let provider_key = ocsf_message_field(provider_key); + ocsf_emit!( + HttpActivityBuilder::new(ocsf_ctx()) + .activity(ActivityId::Other) + .action(ActionId::Allowed) + .disposition(DispositionId::Allowed) + .severity(SeverityId::Informational) + .http_request(HttpRequest::new( + &req.action, + OcsfUrl::new("http", &ctx.host, request_path, ctx.port), + )) + .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) + .message(format!( + "Provider credential injected for {} on {}:{}", + provider_key, ctx.host, ctx.port + )) + .build() + ); + Ok(L7Request { + action: req.action, + target: req.target, + query_params: req.query_params, + raw_header: modified_raw_header, + body_length: req.body_length, + }) + } + other => Err(miette!( + "credential injection for auth_style '{}' is not yet supported", + other + )), + } } fn ocsf_message_field(value: &str) -> String { @@ -791,4 +888,213 @@ mod tests { ); fixture.assert_one_request("api.example.com\t443\t/v1/**\tprovider:access_token"); } + + fn static_credential_ctx( + env_var: &str, + secret_value: &str, + auth_style: &str, + header_name: &str, + ) -> (L7EvalContext, String) { + let (_, resolver) = openshell_core::secrets::SecretResolver::from_provider_env( + std::iter::once((env_var.to_string(), secret_value.to_string())).collect(), + ); + + let key = format!("api.example.com\t443\t\tmy-provider:{env_var}"); + let mut dynamic_credentials = std::collections::HashMap::new(); + dynamic_credentials.insert( + key.clone(), + ProviderProfileCredential { + name: "api_token".to_string(), + env_vars: vec![env_var.to_string()], + auth_style: auth_style.to_string(), + header_name: header_name.to_string(), + token_grant: None, + ..Default::default() + }, + ); + + let ctx = L7EvalContext { + host: "api.example.com".into(), + port: 443, + policy_name: "api".into(), + binary_path: "/usr/bin/curl".into(), + ancestors: vec![], + cmdline_paths: vec![], + secret_resolver: resolver.map(Arc::new), + activity_tx: None, + dynamic_credentials: Some(Arc::new(std::sync::RwLock::new(dynamic_credentials))), + token_grant_resolver: None, + }; + (ctx, key) + } + + #[tokio::test] + async fn inject_static_credential_injects_bearer_token() { + let (ctx, _) = + static_credential_ctx("GITHUB_TOKEN", "ghp_secret123", "bearer", "Authorization"); + let req = L7Request { + action: "GET".to_string(), + target: "/repos/owner/repo".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /repos/owner/repo HTTP/1.1\r\nHost: api.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + let rewritten = inject_if_needed(req, &ctx) + .await + .expect("static credential injection should succeed"); + let rewritten = + String::from_utf8(rewritten.raw_header).expect("rewritten request should be UTF-8"); + + assert!(rewritten.contains("Authorization: Bearer ghp_secret123\r\n")); + } + + #[tokio::test] + async fn inject_static_credential_injects_custom_header() { + let (ctx, _) = + static_credential_ctx("ANTHROPIC_API_KEY", "sk-ant-secret", "header", "x-api-key"); + let req = L7Request { + action: "GET".to_string(), + target: "/v1/messages".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /v1/messages HTTP/1.1\r\nHost: api.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + let rewritten = inject_if_needed(req, &ctx) + .await + .expect("static credential injection should succeed"); + let rewritten = + String::from_utf8(rewritten.raw_header).expect("rewritten request should be UTF-8"); + + assert!(rewritten.contains("x-api-key: sk-ant-secret\r\n")); + assert!(!rewritten.contains("Bearer")); + } + + #[tokio::test] + async fn inject_static_credential_fails_when_credential_missing() { + // Set up a credential entry pointing to an env var that is NOT in the resolver. + let mut dynamic_credentials = std::collections::HashMap::new(); + dynamic_credentials.insert( + "api.example.com\t443\t\tmy-provider:MISSING_TOKEN".to_string(), + ProviderProfileCredential { + name: "api_token".to_string(), + env_vars: vec!["MISSING_TOKEN".to_string()], + auth_style: "bearer".to_string(), + header_name: "Authorization".to_string(), + token_grant: None, + ..Default::default() + }, + ); + let ctx = L7EvalContext { + host: "api.example.com".into(), + port: 443, + policy_name: "api".into(), + binary_path: "/usr/bin/curl".into(), + ancestors: vec![], + cmdline_paths: vec![], + secret_resolver: None, + activity_tx: None, + dynamic_credentials: Some(Arc::new(std::sync::RwLock::new(dynamic_credentials))), + token_grant_resolver: None, + }; + let req = L7Request { + action: "GET".to_string(), + target: "/v1/data".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /v1/data HTTP/1.1\r\nHost: api.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + let err = inject_if_needed(req, &ctx) + .await + .expect_err("missing credential should fail closed"); + assert!(err.to_string().contains("no credential value found")); + } + + #[tokio::test] + async fn inject_static_credential_resolves_fallback_env_var() { + // Credential declares two env vars, only the second is in the resolver. + let (_, resolver) = openshell_core::secrets::SecretResolver::from_provider_env( + std::iter::once(("GH_TOKEN".to_string(), "ghp_fallback".to_string())).collect(), + ); + let mut dynamic_credentials = std::collections::HashMap::new(); + dynamic_credentials.insert( + "api.example.com\t443\t\tmy-provider:api_token".to_string(), + ProviderProfileCredential { + name: "api_token".to_string(), + env_vars: vec!["GITHUB_TOKEN".to_string(), "GH_TOKEN".to_string()], + auth_style: "bearer".to_string(), + header_name: "Authorization".to_string(), + token_grant: None, + ..Default::default() + }, + ); + let ctx = L7EvalContext { + host: "api.example.com".into(), + port: 443, + policy_name: "api".into(), + binary_path: "/usr/bin/curl".into(), + ancestors: vec![], + cmdline_paths: vec![], + secret_resolver: resolver.map(Arc::new), + activity_tx: None, + dynamic_credentials: Some(Arc::new(std::sync::RwLock::new(dynamic_credentials))), + token_grant_resolver: None, + }; + let req = L7Request { + action: "GET".to_string(), + target: "/repos".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /repos HTTP/1.1\r\nHost: api.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + let rewritten = inject_if_needed(req, &ctx) + .await + .expect("should resolve fallback env var"); + let rewritten = + String::from_utf8(rewritten.raw_header).expect("rewritten request should be UTF-8"); + + assert!(rewritten.contains("Authorization: Bearer ghp_fallback\r\n")); + } + + #[tokio::test] + async fn inject_static_credential_rejects_unsupported_auth_style() { + let (ctx, _) = static_credential_ctx("API_KEY", "secret123", "query", ""); + let req = L7Request { + action: "GET".to_string(), + target: "/v1/data".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /v1/data HTTP/1.1\r\nHost: api.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + let err = inject_if_needed(req, &ctx) + .await + .expect_err("unsupported auth_style should fail"); + assert!(err.to_string().contains("not yet supported")); + } + + #[tokio::test] + async fn inject_no_match_passes_request_through() { + let (ctx, _) = static_credential_ctx("TOKEN", "secret", "bearer", "Authorization"); + let req = L7Request { + action: "GET".to_string(), + target: "/data".to_string(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /data HTTP/1.1\r\nHost: other.example.com\r\n\r\n".to_vec(), + body_length: BodyLength::None, + }; + + // Host doesn't match, so request should pass through unmodified. + let mut ctx_wrong_host = ctx; + ctx_wrong_host.host = "other.example.com".into(); + + let result = inject_if_needed(req, &ctx_wrong_host) + .await + .expect("unmatched request should pass through"); + let raw = String::from_utf8(result.raw_header).expect("UTF-8"); + assert!(!raw.contains("Authorization")); + } } diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index 1456c5cfa..69941a50d 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -68,8 +68,8 @@ The following Providers v2 design items are not part of the current behavior: | Roadmap item | Current behavior | |---|---| -| General profile-driven credential placement | Static `auth_style`, `header_name`, `query_param`, and `path_template` placement metadata is stored and validated, but static credential injection still depends on environment placeholders generated from provider credentials. Dynamic `token_grant` credentials support `bearer` and `header` placement for matching HTTP endpoints. | -| Endpoint and binary scoped credential injection | Provider profile endpoints and binaries affect policy composition. Dynamic token grants are endpoint-scoped. Static placeholder injection is not yet restricted by profile endpoint or binary metadata. | +| General profile-driven credential placement | Static credentials with `auth_style: bearer` or `auth_style: header` are proxy-injected for matching profile endpoints, alongside dynamic `token_grant` credentials. Other placement modes (`query`, `path`) are stored and validated but not yet injected at runtime. | +| Binary scoped credential injection | Provider profile binaries affect policy composition but do not yet restrict which processes receive credential injection. | | Credential verification on create | `openshell provider create` does not yet probe provider verification endpoints or expose `--no-verify`. | | Automatic credential scope extraction | OpenShell does not yet inspect upstream provider responses to discover credential scopes. | | Inference mounting from attached providers | `inference_capable` is profile metadata. Attaching an inference-capable provider does not yet create `inference.local` routes. | @@ -259,7 +259,7 @@ binaries: `category` groups profiles in `openshell provider list-profiles`. Use one of the values in the category enum. -`credentials` declares the credential names, environment variables, auth metadata, optional refresh metadata, and optional dynamic token grant metadata for the provider type. The `auth_style` field accepts `basic`, `bearer`, `header`, `query`, or `path`. When `auth_style` is `path`, set `path_template` to a URL path containing the `{credential}` placeholder exactly once (for example, `/v1/{credential}/resources`). Static credentials are exposed as placeholder environment variables and resolved in outbound HTTP requests. Dynamic token grants are resolved by the sandbox proxy on demand for matching profile endpoints and support `bearer` or `header` placement. +`credentials` declares the credential names, environment variables, auth metadata, optional refresh metadata, and optional dynamic token grant metadata for the provider type. The `auth_style` field accepts `basic`, `bearer`, `header`, `query`, or `path`. When `auth_style` is `path`, set `path_template` to a URL path containing the `{credential}` placeholder exactly once (for example, `/v1/{credential}/resources`). Static credentials are exposed as placeholder environment variables and resolved in outbound HTTP requests. When `providers_v2_enabled` is set and a static credential declares `auth_style: bearer` or `auth_style: header`, the sandbox proxy also injects the credential value into matching profile endpoint traffic, using the same header placement rules as dynamic token grants. Credentials without an explicit `auth_style` are not proxy-injected. Dynamic token grants are resolved by the sandbox proxy on demand for matching profile endpoints and support `bearer` or `header` placement. `discovery` controls what `--from-existing` scans when `providers_v2_enabled=true`. Each entry in `discovery.credentials` must name a @@ -664,7 +664,7 @@ Already-running processes keep the environment they started with. OpenShell does Detaching a provider removes its provider policy layer from future effective policy reads and removes its credential placeholders from future process environments. It does not remove environment variables from already-running processes. -OpenShell rejects provider updates and refresh configuration when they would make two providers attached to the same sandbox expose the same active credential environment key. It also rejects attached provider sets with ambiguous dynamic token grants at equal host/path specificity. Use provider-specific credential names and make one dynamic grant selector more specific when one sandbox needs multiple providers with overlapping upstream concepts. +OpenShell rejects provider updates and refresh configuration when they would make two providers attached to the same sandbox expose the same active credential environment key. It also rejects attached provider sets with ambiguous credential bindings — both static and dynamic — at equal host/path specificity. Use provider-specific credential names and make one credential selector more specific when one sandbox needs multiple providers with overlapping upstream endpoints. ## Next Steps diff --git a/proto/openshell.proto b/proto/openshell.proto index d701956d3..3456393d8 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -1145,9 +1145,10 @@ message GetSandboxProviderEnvironmentResponse { uint64 provider_env_revision = 2; // Expiration timestamps for returned environment variables. map credential_expires_at_ms = 3; - // Dynamic credentials that require token grants or other runtime injection. - // Maps endpoint-bound provider metadata to credential metadata. - // Supervisor uses this to inject Authorization headers for token grant credentials. + // Endpoint-bound provider credentials for proxy-side injection. + // Maps endpoint keys to credential metadata. Entries with a token_grant + // use SPIFFE-backed runtime exchange; entries without resolve credential + // values from the environment map above. map dynamic_credentials = 4; }