From a505e8d9eb92235e60bdb5dc421ade54836cd6ed Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Mon, 17 Aug 2026 11:02:18 -0700 Subject: [PATCH 1/2] fix(providers): keep refresh credential handles stable Signed-off-by: Mrunal Patel --- .agents/skills/openshell-cli/SKILL.md | 8 + architecture/sandbox.md | 16 +- .../src/provider_credentials.rs | 273 +++++++++- crates/openshell-core/src/secrets.rs | 125 ++++- crates/openshell-server/src/grpc/policy.rs | 44 +- crates/openshell-server/src/grpc/provider.rs | 496 +++++++++++++++++- .../openshell-server/src/provider_refresh.rs | 70 ++- .../src/l7/relay.rs | 5 + .../src/l7/websocket.rs | 1 + .../openshell-supervisor-network/src/proxy.rs | 6 + docs/sandboxes/providers-v2.mdx | 25 +- e2e/python/test_sandbox_providers.py | 2 +- e2e/rust/Cargo.toml | 5 + e2e/rust/tests/provider_refresh_handles.rs | 356 +++++++++++++ proto/openshell.proto | 11 + sdk/go/proto/openshellv1/openshell.pb.go | 43 +- 16 files changed, 1407 insertions(+), 79 deletions(-) create mode 100644 e2e/rust/tests/provider_refresh_handles.rs diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index c1f180b1e2..f082a899f8 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -180,6 +180,14 @@ openshell provider refresh rotate my-outlook --credential-key MS_GRAPH_ACCESS_TO Prefer `--secret-material-env KEY[=ENVVAR]` for secret refresh material. `--material KEY=VALUE` is for non-secret material; `--secret-material-key` marks supplied material keys as secret. +Gateway-managed refresh credentials use an identity-stable workload handle. +Routine automatic refresh and `provider refresh rotate` update the access token +behind that handle, so long-running processes do not need to restart. Running +processes must be restarted once when upgrading from revision-scoped +placeholders. A later `provider refresh configure` call is an explicit +reauthorization boundary: it revokes the previous handle, and processes holding +that handle fail closed until restarted. + --- ## Workflow 3: Sandbox Lifecycle diff --git a/architecture/sandbox.md b/architecture/sandbox.md index e6ec528bd8..12dc940ec4 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -87,6 +87,16 @@ partially active or last-known-good static set. Invalid metadata preserves the supplied dynamic snapshot, while a fetch failure preserves the currently active dynamic snapshot. +Gateway-managed refresh credentials use an opaque workload handle derived from +the sandbox, provider identity, credential key, refresh authorization epoch, +and canonical endpoint boundary. The handle remains stable while the gateway +rotates the short-lived value, so an already-running process keeps one +placeholder and each request resolves against the current token. Explicit +refresh reconfiguration, provider replacement or detachment, and endpoint +boundary changes produce a new handle and revoke the old one. Supervisors do +not retain old values for these handles. Unmanaged static credentials retain +the bounded revision-generation behavior. + Route selection and policy evaluation use a syntax-only redacted request target; they do not materialize real credentials. Cross-endpoint placeholder use returns HTTP 403. After a WebSocket upgrade it closes the connection with policy @@ -258,8 +268,10 @@ when policy allows the target endpoint. For GCP providers, a loopback metadata server inside the network namespace serves placeholders to SDKs that bypass the proxy (e.g. Go's `cloud.google.com/go/compute/metadata`). Secrets must not be logged in OCSF or plain tracing output. The supervisor uses revision-scoped -placeholders for rotating provider credentials; provider environment keys -beginning with `v_` are reserved for that placeholder namespace. +placeholders for unmanaged rotating credentials and identity-stable opaque +handles for gateway-managed refresh credentials. Provider environment keys +beginning with `v_` or `s<64 lowercase hex characters>_` are reserved +for those placeholder namespaces. Provider profiles can also declare dynamic token grants. For matching HTTP endpoints, the supervisor obtains a SPIFFE JWT-SVID from the local Workload API, diff --git a/crates/openshell-core/src/provider_credentials.rs b/crates/openshell-core/src/provider_credentials.rs index cc71e1efbf..2b1537a21b 100644 --- a/crates/openshell-core/src/provider_credentials.rs +++ b/crates/openshell-core/src/provider_credentials.rs @@ -45,6 +45,7 @@ struct StaticCredentialIdentityEpoch { struct CompiledStaticCredentialBinding { endpoints: Vec, credential_identity: String, + workload_credential_handle: String, } #[derive(Debug, Clone)] @@ -122,25 +123,43 @@ impl ProviderCredentialState { static_credential_bindings, &non_secret_environment_keys, )?; - let state = - Self::from_environment(revision, env, credential_expires_at_ms, dynamic_credentials); - { - let mut inner = state - .inner - .write() - .expect("provider credential state poisoned"); - inner - .known_static_credential_keys - .extend(static_credential_bindings.keys().cloned()); - update_static_credential_identity_epochs( - &mut inner.static_credential_identity_epochs, + let stable_handles = static_credential_stable_handles(&static_credential_bindings); + let (child_env, generation_resolver, current_resolver) = + SecretResolver::from_provider_env_for_current_revision_with_stable_handles( + env, + credential_expires_at_ms, revision, - &static_credential_bindings, + &stable_handles, ); - inner.non_secret_environment_keys = non_secret_environment_keys.into_iter().collect(); - inner.static_credential_bindings = static_credential_bindings; - } - Ok(state) + let snapshot = Arc::new(ProviderCredentialSnapshot { + revision, + child_env, + dynamic_credentials, + }); + let generations = generation_resolver.map(Arc::new).into_iter().collect(); + let current_resolver = current_resolver.map(Arc::new); + let combined_resolver = merge_resolvers(&generations, current_resolver.as_ref()); + let known_static_credential_keys = static_credential_bindings.keys().cloned().collect(); + let mut static_credential_identity_epochs = HashMap::new(); + update_static_credential_identity_epochs( + &mut static_credential_identity_epochs, + revision, + &static_credential_bindings, + ); + + Ok(Self { + inner: Arc::new(RwLock::new(ProviderCredentialStateInner { + current: snapshot, + generations, + current_resolver, + combined_resolver, + suppressed_keys: HashSet::new(), + non_secret_environment_keys: non_secret_environment_keys.into_iter().collect(), + static_credential_bindings, + known_static_credential_keys, + static_credential_identity_epochs, + })), + }) } /// Build a static provider state from an already-prepared child @@ -505,11 +524,13 @@ impl ProviderCredentialState { } }; + let stable_handles = static_credential_stable_handles(&static_credential_bindings); let (mut child_env, generation_resolver, current_resolver) = - SecretResolver::from_provider_env_for_current_revision( + SecretResolver::from_provider_env_for_current_revision_with_stable_handles( env, credential_expires_at_ms, revision, + &stable_handles, ); let mut inner = self .inner @@ -633,6 +654,13 @@ fn compile_static_credential_bindings( "static credential binding has no authorized endpoints", )); } + if !binding.workload_credential_handle.is_empty() + && !is_valid_workload_credential_handle(&binding.workload_credential_handle) + { + return Err(binding_error( + "static credential binding has an invalid workload credential handle", + )); + } for endpoint in &binding.endpoints { if endpoint.port == 0 || endpoint.port > u32::from(u16::MAX) { return Err(binding_error( @@ -655,6 +683,7 @@ fn compile_static_credential_bindings( CompiledStaticCredentialBinding { endpoints, credential_identity: binding.credential_identity, + workload_credential_handle: binding.workload_credential_handle, }, )) }) @@ -683,13 +712,37 @@ fn static_credential_identities( .collect() } +fn static_credential_stable_handles( + bindings: &HashMap, +) -> HashMap { + bindings + .iter() + .filter(|(_, binding)| !binding.workload_credential_handle.is_empty()) + .map(|(key, binding)| (key.clone(), binding.workload_credential_handle.clone())) + .collect() +} + +fn is_valid_workload_credential_handle(handle: &str) -> bool { + handle.len() == 64 + && handle + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) +} + fn update_static_credential_identity_epochs( epochs: &mut HashMap, revision: u64, bindings: &HashMap, ) { - epochs.retain(|key, _| bindings.contains_key(key)); + epochs.retain(|key, _| { + bindings + .get(key) + .is_some_and(|binding| binding.workload_credential_handle.is_empty()) + }); for (key, binding) in bindings { + if !binding.workload_credential_handle.is_empty() { + continue; + } match epochs.get_mut(key) { Some(epoch) if epoch.identity == binding.credential_identity => { Arc::make_mut(&mut epoch.revisions).insert(revision); @@ -754,9 +807,17 @@ mod tests { path: path.to_string(), }], credential_identity: "provider-a:API_KEY".to_string(), + workload_credential_handle: String::new(), } } + fn stable_binding(host: &str, port: u32, path: &str, handle: &str) -> StaticCredentialBinding { + let mut binding = binding(host, port, path); + binding.credential_identity = format!("refresh:{handle}"); + binding.workload_credential_handle = handle.to_string(); + binding + } + fn assert_binding_validation_error( env: HashMap, bindings: HashMap, @@ -822,6 +883,15 @@ mod tests { "static credential binding has no authorized endpoints", ); + let mut invalid_handle = binding("api.example.com", 443, "/**"); + invalid_handle.workload_credential_handle = "not-an-opaque-handle".to_string(); + assert_binding_validation_error( + credential_env(), + HashMap::from([("API_KEY".to_string(), invalid_handle)]), + Vec::new(), + "static credential binding has an invalid workload credential handle", + ); + for (host, port) in [ ("api.example.com", 0), ("api.example.com", u32::from(u16::MAX) + 1), @@ -1218,6 +1288,171 @@ mod tests { ); } + #[test] + fn stable_handle_resolves_current_token_after_initial_token_expires() { + let handle = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + let now_ms = i64::try_from( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system time") + .as_millis(), + ) + .expect("current time fits i64"); + let binding = stable_binding("api.example.com", 443, "/**", handle); + let state = ProviderCredentialState::from_bound_environment( + 1, + HashMap::from([("API_KEY".to_string(), "expired".to_string())]), + HashMap::from([("API_KEY".to_string(), now_ms - 1)]), + HashMap::new(), + HashMap::from([("API_KEY".to_string(), binding.clone())]), + Vec::new(), + ) + .expect("initial stable binding"); + let workload_placeholder = state.snapshot().child_env["API_KEY"].clone(); + assert!(workload_placeholder.contains(handle)); + assert_eq!( + state + .resolver_for_endpoint("api.example.com", 443, "/v1") + .expect("resolver") + .resolve_placeholder(&workload_placeholder), + None, + "the expired access token must fail closed" + ); + + state + .install_bound_environment( + 2, + HashMap::from([("API_KEY".to_string(), "current".to_string())]), + HashMap::from([("API_KEY".to_string(), now_ms + 60_000)]), + HashMap::new(), + HashMap::from([("API_KEY".to_string(), binding)]), + Vec::new(), + ) + .expect("refreshed stable binding"); + + assert_eq!(state.snapshot().child_env["API_KEY"], workload_placeholder); + let resolver = state + .resolver_for_endpoint("api.example.com", 443, "/v1") + .expect("resolver"); + assert_eq!( + resolver.resolve_placeholder(&workload_placeholder), + Some("current") + ); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:v2_API_KEY"), + None, + "refresh-managed credentials must not expose revision aliases" + ); + } + + #[test] + fn stable_handle_survives_twelve_rotations_and_state_reconstruction() { + let handle = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + let binding = stable_binding("api.example.com", 443, "/**", handle); + let state = ProviderCredentialState::from_bound_environment( + 1, + HashMap::from([("API_KEY".to_string(), "token-1".to_string())]), + HashMap::new(), + HashMap::new(), + HashMap::from([("API_KEY".to_string(), binding.clone())]), + Vec::new(), + ) + .expect("initial stable binding"); + let workload_placeholder = state.snapshot().child_env["API_KEY"].clone(); + + for revision in 2..=13 { + let token = format!("token-{revision}"); + state + .install_bound_environment( + revision, + HashMap::from([("API_KEY".to_string(), token.clone())]), + HashMap::new(), + HashMap::new(), + HashMap::from([("API_KEY".to_string(), binding.clone())]), + Vec::new(), + ) + .expect("same-epoch rotation"); + assert_eq!(state.snapshot().child_env["API_KEY"], workload_placeholder); + assert_eq!( + state + .resolver_for_endpoint("api.example.com", 443, "/v1") + .expect("resolver") + .resolve_placeholder(&workload_placeholder), + Some(token.as_str()) + ); + } + + let reconstructed = ProviderCredentialState::from_bound_environment( + 14, + HashMap::from([("API_KEY".to_string(), "token-14".to_string())]), + HashMap::new(), + HashMap::new(), + HashMap::from([("API_KEY".to_string(), binding)]), + Vec::new(), + ) + .expect("reconstructed network supervisor state"); + assert_eq!( + reconstructed.snapshot().child_env["API_KEY"], + workload_placeholder + ); + assert_eq!( + reconstructed + .resolver_for_endpoint("api.example.com", 443, "/v1") + .expect("resolver") + .resolve_placeholder(&workload_placeholder), + Some("token-14") + ); + } + + #[test] + fn authorization_epoch_or_endpoint_change_revokes_stable_handle() { + let old_handle = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"; + let new_handle = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"; + let state = ProviderCredentialState::from_bound_environment( + 1, + HashMap::from([("API_KEY".to_string(), "old".to_string())]), + HashMap::new(), + HashMap::new(), + HashMap::from([( + "API_KEY".to_string(), + stable_binding("old.example.com", 443, "/v1/**", old_handle), + )]), + Vec::new(), + ) + .expect("old authorization"); + let old_placeholder = state.snapshot().child_env["API_KEY"].clone(); + + state + .install_bound_environment( + 2, + HashMap::from([("API_KEY".to_string(), "new".to_string())]), + HashMap::new(), + HashMap::new(), + HashMap::from([( + "API_KEY".to_string(), + stable_binding("new.example.com", 443, "/v2/**", new_handle), + )]), + Vec::new(), + ) + .expect("replacement authorization"); + + let current = state + .resolver_for_endpoint("new.example.com", 443, "/v2/messages") + .expect("current resolver"); + assert_eq!(current.resolve_placeholder(&old_placeholder), None); + assert_eq!( + current.resolve_placeholder(&state.snapshot().child_env["API_KEY"]), + Some("new") + ); + let wrong_endpoint = state + .resolver_for_endpoint("old.example.com", 443, "/v1/messages") + .expect("endpoint-scoped resolver"); + let error = wrong_endpoint + .rewrite_header_value(&state.snapshot().child_env["API_KEY"]) + .expect_err("new handle must not resolve at the old endpoint"); + assert!(error.is_endpoint_mismatch()); + } + #[test] fn aged_generation_falls_back_across_non_monotonic_same_identity_rotations() { let state = ProviderCredentialState::from_bound_environment( diff --git a/crates/openshell-core/src/secrets.rs b/crates/openshell-core/src/secrets.rs index 903581ae98..6450da4f61 100644 --- a/crates/openshell-core/src/secrets.rs +++ b/crates/openshell-core/src/secrets.rs @@ -173,32 +173,73 @@ impl SecretResolver { credential_expires_at_ms: HashMap, revision: u64, ) -> (HashMap, Option, Option) { - if revision == 0 { - let (child_env, current_resolver) = - Self::from_provider_env_for_revision_with_current_aliases( - provider_env, - credential_expires_at_ms, - 0, - true, + Self::from_provider_env_for_current_revision_with_stable_handles( + provider_env, + credential_expires_at_ms, + revision, + &HashMap::new(), + ) + } + + /// Build workload environment and resolver snapshots with gateway-issued + /// stable handles for selected refresh-managed credentials. + /// + /// Stable credentials are registered only in the current resolver. Their + /// previous values never enter the bounded revision-generation queue, so a + /// revoked or replaced handle cannot resolve an older access token. + pub(crate) fn from_provider_env_for_current_revision_with_stable_handles( + provider_env: HashMap, + credential_expires_at_ms: HashMap, + revision: u64, + stable_handles: &HashMap, + ) -> (HashMap, Option, Option) { + let mut child_env = HashMap::with_capacity(provider_env.len()); + let mut generation_values = HashMap::new(); + let mut current_values = HashMap::new(); + + for (key, value) in provider_env { + if uses_reserved_revision_namespace(&key) { + tracing::warn!( + provider_env_key = %key, + "skipping provider credential env var in reserved placeholder namespace" ); - return (child_env, None, current_resolver); - } - let provider_env_for_current = provider_env.clone(); - let credential_expires_at_ms_for_current = credential_expires_at_ms.clone(); - let (child_env, revision_resolver) = - Self::from_provider_env_for_revision_with_current_aliases( - provider_env, - credential_expires_at_ms, - revision, - false, + continue; + } + let secret = SecretValue { + value: Arc::from(value), + expires_at_ms: credential_expires_at_ms + .get(&key) + .copied() + .unwrap_or_default(), + }; + let placeholder = stable_handles.get(&key).map_or_else( + || { + let placeholder = placeholder_for_env_key_for_revision(&key, revision); + if revision != 0 { + generation_values.insert(placeholder.clone(), secret.clone()); + } + placeholder + }, + |handle| placeholder_for_env_key_for_stable_handle(&key, handle), ); - let (_, current_resolver) = Self::from_provider_env_for_revision_with_current_aliases( - provider_env_for_current, - credential_expires_at_ms_for_current, - revision, - true, - ); - (child_env, revision_resolver, current_resolver) + child_env.insert(key.clone(), placeholder.clone()); + current_values.insert(placeholder, secret.clone()); + current_values.insert(placeholder_for_env_key(&key), secret); + } + + let resolver = |by_placeholder: HashMap| { + (!by_placeholder.is_empty()).then_some(Self { + by_placeholder, + denied_env_keys: HashSet::new(), + identity_bound_env_keys: HashSet::new(), + revision_fallback_allowed_revisions: HashMap::new(), + }) + }; + ( + child_env, + resolver(generation_values), + resolver(current_values), + ) } fn from_provider_env_for_revision_with_current_aliases( @@ -330,6 +371,7 @@ impl SecretResolver { pub fn resolve_placeholder(&self, value: &str) -> Option<&str> { if placeholder_env_key(value).is_some_and(|key| self.identity_bound_env_keys.contains(key)) && revisioned_placeholder_parts(value).is_none() + && stable_placeholder_parts(value).is_none() { // Canonical placeholders and provider-shaped aliases carry no // credential identity. Endpoint-bound request input must use the @@ -673,14 +715,22 @@ fn revisioned_placeholder_parts(token: &str) -> Option<(u64, &str)> { Some((revision.parse().ok()?, key)) } +fn stable_placeholder_parts(token: &str) -> Option<(&str, &str)> { + let suffix = token.strip_prefix(PLACEHOLDER_PREFIX)?; + let (handle, key) = split_stable_env_key(suffix)?; + Some((handle, key)) +} + fn placeholder_env_key(token: &str) -> Option<&str> { - revisioned_placeholder_env_key(token) + stable_placeholder_parts(token) + .map(|(_, key)| key) + .or_else(|| revisioned_placeholder_env_key(token)) .or_else(|| token.strip_prefix(PLACEHOLDER_PREFIX)) .or_else(|| alias_env_key(token)) } pub fn uses_reserved_revision_namespace(key: &str) -> bool { - split_revisioned_env_key(key).is_some() + split_revisioned_env_key(key).is_some() || split_stable_env_key(key).is_some() } fn split_revisioned_env_key(key: &str) -> Option<(&str, &str)> { @@ -696,6 +746,21 @@ fn split_revisioned_env_key(key: &str) -> Option<(&str, &str)> { Some((revision, env_key)) } +fn split_stable_env_key(key: &str) -> Option<(&str, &str)> { + let suffix = key.strip_prefix('s')?; + let (handle, env_key) = suffix.split_once('_')?; + if handle.len() != 64 + || !handle + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + || env_key.is_empty() + || !env_key.bytes().all(is_env_key_char) + { + return None; + } + Some((handle, env_key)) +} + fn token_boundary_ok(text: &str, abs_start: usize, token_end: usize, token: &str) -> bool { if token.starts_with(PLACEHOLDER_PREFIX) { return token_end == text.len() @@ -719,6 +784,10 @@ pub fn placeholder_for_env_key_for_revision(key: &str, revision: u64) -> String } } +pub(crate) fn placeholder_for_env_key_for_stable_handle(key: &str, handle: &str) -> String { + format!("{PLACEHOLDER_PREFIX}s{handle}_{key}") +} + // --------------------------------------------------------------------------- // Secret validation (F1 — CWE-113) // --------------------------------------------------------------------------- @@ -1341,8 +1410,12 @@ mod tests { fn reserved_revision_namespace_requires_version_and_key() { assert!(uses_reserved_revision_namespace("v10_GITHUB_TOKEN")); assert!(uses_reserved_revision_namespace("v999999_very_unlikely")); + assert!(uses_reserved_revision_namespace( + "saaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_GITHUB_TOKEN" + )); assert!(!uses_reserved_revision_namespace("v_GITHUB_TOKEN")); assert!(!uses_reserved_revision_namespace("v10_")); + assert!(!uses_reserved_revision_namespace("sshort_GITHUB_TOKEN")); assert!(!uses_reserved_revision_namespace("very_unlikely")); assert!(!uses_reserved_revision_namespace("GITHUB_TOKEN")); } diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 3b841e66a5..a63121ae05 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1981,7 +1981,7 @@ async fn compute_provider_env_revision_with_catalog_and_policy_bindings( policy_bindings: &HashMap>, ) -> Result { let mut hasher = Sha256::new(); - hasher.update(b"openshell-provider-env-revision-v3"); + hasher.update(b"openshell-provider-env-revision-v4"); for provider_name in provider_names { hasher.update(provider_name.as_bytes()); @@ -1998,6 +1998,10 @@ async fn compute_provider_env_revision_with_catalog_and_policy_bindings( let provider = Provider::decode(record.payload.as_slice()).map_err(|e| { Status::internal(format!("decode provider '{provider_name}' failed: {e}")) })?; + let refresh_states = + crate::provider_refresh::list_refresh_states_for_provider(store, &record.id) + .await?; + hash_provider_refresh_states(&refresh_states, &mut hasher)?; hasher.update(provider.r#type.as_bytes()); hash_provider_profile_revision( catalog, @@ -2050,7 +2054,7 @@ fn compute_provider_env_revision_from_records_and_policy_bindings( policy_bindings: &HashMap>, ) -> Result { let mut hasher = Sha256::new(); - hasher.update(b"openshell-provider-env-revision-v3"); + hasher.update(b"openshell-provider-env-revision-v4"); for record in records { hasher.update(record.name.as_bytes()); @@ -2058,6 +2062,7 @@ fn compute_provider_env_revision_from_records_and_policy_bindings( hasher.update(record.resource_version.to_le_bytes()); let provider = &record.provider; + hash_provider_refresh_states(&record.refresh_states, &mut hasher)?; hasher.update(provider.r#type.as_bytes()); hash_provider_profile_revision( catalog, @@ -2087,6 +2092,40 @@ fn compute_provider_env_revision_from_records_and_policy_bindings( )?)) } +fn hash_provider_refresh_states( + states: &[openshell_core::proto::StoredProviderCredentialRefreshState], + hasher: &mut Sha256, +) -> Result<(), Status> { + let mut states = states.iter().collect::>(); + states.sort_by(|left, right| { + left.credential_key + .cmp(&right.credential_key) + .then_with(|| { + left.metadata + .as_ref() + .map(|metadata| metadata.id.as_str()) + .cmp(&right.metadata.as_ref().map(|metadata| metadata.id.as_str())) + }) + }); + for state in states { + hasher.update(b"provider-refresh-state"); + hasher.update(state.credential_key.as_bytes()); + hasher.update(state.strategy.to_le_bytes()); + hasher.update(crate::provider_refresh::effective_authorization_epoch(state)?.as_bytes()); + if let Some(metadata) = &state.metadata { + hasher.update(metadata.id.as_bytes()); + hasher.update(metadata.resource_version.to_le_bytes()); + } + let mut outputs = state.additional_output_keys.iter().collect::>(); + outputs.sort(); + for (output, key) in outputs { + hasher.update(output.as_bytes()); + hasher.update(key.as_bytes()); + } + } + Ok(()) +} + fn hash_policy_credential_bindings( bindings: &HashMap>, hasher: &mut Sha256, @@ -2252,6 +2291,7 @@ pub(super) async fn handle_get_sandbox_provider_environment( &provider_records, &policy_credential_bindings, &state.credentials, + Some(&sandbox_id), ) .await?; diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 4d67eee018..4dc90a5f8d 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -14,14 +14,17 @@ use crate::provider_profile_sources::{ }; use openshell_core::metadata::ObjectWorkspace; use openshell_core::proto::{ - CredentialHandle, Provider, ProviderCredentialTokenGrantAudienceOverride, ProviderProfile, - ProviderProfileCredential, Sandbox, StaticCredentialBinding, StaticCredentialEndpointBinding, + CredentialHandle, Provider, ProviderCredentialRefreshStrategy, + ProviderCredentialTokenGrantAudienceOverride, ProviderProfile, ProviderProfileCredential, + Sandbox, StaticCredentialBinding, StaticCredentialEndpointBinding, + StoredProviderCredentialRefreshState, }; use openshell_core::telemetry::{ LifecycleOperation, ProviderProfile as TelemetryProviderProfile, TelemetryOutcome, }; use openshell_policy::ProviderPolicyLayer; use prost::Message; +use sha2::{Digest, Sha256}; use std::collections::{HashMap, HashSet}; use tonic::Status; use tracing::warn; @@ -73,6 +76,7 @@ pub(super) struct ProviderEnvironmentRecord { pub object_id: String, pub resource_version: u64, pub provider: Provider, + pub refresh_states: Vec, } impl ProviderEnvironment { @@ -963,11 +967,14 @@ pub(super) async fn load_provider_environment_records( .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; let provider = Provider::decode(record.payload.as_slice()) .map_err(|e| Status::internal(format!("failed to decode provider '{name}': {e}")))?; + let refresh_states = + crate::provider_refresh::list_refresh_states_for_provider(store, &record.id).await?; records.push(ProviderEnvironmentRecord { name: name.clone(), object_id: record.id, resource_version: record.resource_version, provider, + refresh_states, }); } Ok(records) @@ -989,6 +996,7 @@ pub(super) async fn resolve_provider_environment_from_records( records, &HashMap::new(), &credentials, + None, ) .await } @@ -1006,6 +1014,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_credentials( records, &HashMap::new(), credentials, + None, ) .await } @@ -1027,6 +1036,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin records, policy_bindings, &credentials, + None, ) .await } @@ -1037,6 +1047,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin records: &[ProviderEnvironmentRecord], policy_bindings: &HashMap>, credentials: &crate::credentials::CredentialRuntime, + sandbox_id: Option<&str>, ) -> Result { if records.is_empty() { return Ok(ProviderEnvironment::default()); @@ -1096,6 +1107,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin (None, None) => None, }; let has_no_usable_endpoint = effective_endpoints.is_some_and(Vec::is_empty); + let refresh_epochs = refresh_authorization_epochs_by_key(record)?; for (key, value) in &provider.credentials { if is_non_injectable_provider_credential(provider, key) { @@ -1146,10 +1158,13 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin } static_credential_bindings.insert( key.clone(), - StaticCredentialBinding { - endpoints: endpoints.clone(), - credential_identity: format!("{}:{key}", record.object_id), - }, + static_credential_binding( + sandbox_id, + record, + key, + endpoints, + refresh_epochs.get(key).map(String::as_str), + ), ); } } else { @@ -1200,10 +1215,13 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin } static_credential_bindings.insert( key.clone(), - StaticCredentialBinding { - endpoints: endpoints.clone(), - credential_identity: format!("{}:{key}", record.object_id), - }, + static_credential_binding( + sandbox_id, + record, + &key, + endpoints, + refresh_epochs.get(&key).map(String::as_str), + ), ); } } else { @@ -1234,6 +1252,101 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin }) } +fn refresh_authorization_epochs_by_key( + record: &ProviderEnvironmentRecord, +) -> Result, Status> { + let mut epochs = HashMap::new(); + for state in &record.refresh_states { + if !crate::provider_refresh::is_gateway_mintable_strategy( + ProviderCredentialRefreshStrategy::try_from(state.strategy).unwrap_or_default(), + ) { + continue; + } + let epoch = crate::provider_refresh::effective_authorization_epoch(state)?.to_string(); + for key in + std::iter::once(&state.credential_key).chain(state.additional_output_keys.values()) + { + if let Some(previous) = epochs.insert(key.clone(), epoch.clone()) + && previous != epoch + { + return Err(Status::failed_precondition( + "multiple provider refresh authorizations claim one credential key", + )); + } + } + } + Ok(epochs) +} + +fn static_credential_binding( + sandbox_id: Option<&str>, + record: &ProviderEnvironmentRecord, + key: &str, + endpoints: &[StaticCredentialEndpointBinding], + authorization_epoch: Option<&str>, +) -> StaticCredentialBinding { + let workload_credential_handle = sandbox_id + .zip(authorization_epoch) + .map(|(sandbox_id, epoch)| { + derive_workload_credential_handle(sandbox_id, &record.object_id, key, epoch, endpoints) + }) + .unwrap_or_default(); + let credential_identity = if workload_credential_handle.is_empty() { + format!("{}:{key}", record.object_id) + } else { + format!("refresh:{workload_credential_handle}") + }; + StaticCredentialBinding { + endpoints: endpoints.to_vec(), + credential_identity, + workload_credential_handle, + } +} + +fn derive_workload_credential_handle( + sandbox_id: &str, + provider_id: &str, + credential_key: &str, + authorization_epoch: &str, + endpoints: &[StaticCredentialEndpointBinding], +) -> String { + let mut endpoints = endpoints + .iter() + .map(|endpoint| { + let host = endpoint + .host + .trim() + .trim_end_matches('.') + .to_ascii_lowercase(); + let path = match endpoint.path.trim() { + "" | "**" | "/**" => "/**".to_string(), + path => path.to_string(), + }; + (host, endpoint.port, path) + }) + .collect::>(); + endpoints.sort(); + endpoints.dedup(); + + let mut hasher = Sha256::new(); + hash_handle_component(&mut hasher, b"openshell-workload-credential-handle-v1"); + hash_handle_component(&mut hasher, sandbox_id.as_bytes()); + hash_handle_component(&mut hasher, provider_id.as_bytes()); + hash_handle_component(&mut hasher, credential_key.as_bytes()); + hash_handle_component(&mut hasher, authorization_epoch.as_bytes()); + for (host, port, path) in endpoints { + hash_handle_component(&mut hasher, host.as_bytes()); + hash_handle_component(&mut hasher, &port.to_le_bytes()); + hash_handle_component(&mut hasher, path.as_bytes()); + } + hex::encode(hasher.finalize()) +} + +fn hash_handle_component(hasher: &mut Sha256, value: &[u8]) { + hasher.update(value.len().to_le_bytes()); + hasher.update(value); +} + /// Resolve dynamic credentials (token grants) from the same records used for /// the provider-environment revision and static credential bindings. fn resolve_dynamic_credentials_from_records( @@ -2008,10 +2121,10 @@ use openshell_core::proto::{ GetProviderRequest, ImportProviderProfilesRequest, ImportProviderProfilesResponse, LintProviderProfilesRequest, LintProviderProfilesResponse, ListProviderProfilesRequest, ListProviderProfilesResponse, ListProvidersRequest, ListProvidersResponse, - ProviderCredentialRefreshStrategy, ProviderProfileDiagnostic, ProviderProfileImportItem, - ProviderProfileResponse, ProviderResponse, RotateProviderCredentialRequest, - RotateProviderCredentialResponse, StoredProviderProfile, UpdateProviderProfilesRequest, - UpdateProviderProfilesResponse, UpdateProviderRequest, + ProviderProfileDiagnostic, ProviderProfileImportItem, ProviderProfileResponse, + ProviderResponse, RotateProviderCredentialRequest, RotateProviderCredentialResponse, + StoredProviderProfile, UpdateProviderProfilesRequest, UpdateProviderProfilesResponse, + UpdateProviderRequest, }; use openshell_providers::{ CredentialRefreshProfile, ProfileValidationDiagnostic, ProviderTypeProfile, @@ -5569,6 +5682,48 @@ mod tests { Some(&expires_at_ms) ); + let first_refresh = crate::provider_refresh::get_refresh_state( + state.store.as_ref(), + "default", + provider.object_id(), + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap() + .expect("first refresh state"); + handle_configure_provider_refresh( + &state, + authed_request(ConfigureProviderRefreshRequest { + provider: "msgraph".to_string(), + credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(), + strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials as i32, + material: HashMap::from([ + ("tenant_id".to_string(), "tenant".to_string()), + ("client_id".to_string(), "client-id".to_string()), + ("client_secret".to_string(), "client-secret".to_string()), + ]), + secret_material_keys: vec!["client_secret".to_string()], + expires_at_ms: Some(expires_at_ms), + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + let second_refresh = crate::provider_refresh::get_refresh_state( + state.store.as_ref(), + "default", + provider.object_id(), + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap() + .expect("reconfigured refresh state"); + assert_eq!(first_refresh.object_id(), second_refresh.object_id()); + assert_ne!( + first_refresh.authorization_epoch, second_refresh.authorization_epoch, + "every explicit configuration starts a new authorization epoch" + ); + let deleted = handle_delete_provider_refresh( &state, authed_request(DeleteProviderRefreshRequest { @@ -7833,6 +7988,317 @@ mod tests { ); } + #[test] + fn workload_handle_derivation_is_order_independent_and_boundary_bound() { + let endpoints = vec![ + StaticCredentialEndpointBinding { + host: "API.EXAMPLE.COM.".to_string(), + port: 443, + path: "**".to_string(), + }, + StaticCredentialEndpointBinding { + host: "files.example.com".to_string(), + port: 8443, + path: "/v1/**".to_string(), + }, + ]; + let mut reordered = endpoints.clone(); + reordered.reverse(); + let first = derive_workload_credential_handle( + "sandbox-a", + "provider-a", + "ACCESS_TOKEN", + "epoch-a", + &endpoints, + ); + assert_eq!( + first, + derive_workload_credential_handle( + "sandbox-a", + "provider-a", + "ACCESS_TOKEN", + "epoch-a", + &reordered, + ) + ); + for changed in [ + derive_workload_credential_handle( + "sandbox-b", + "provider-a", + "ACCESS_TOKEN", + "epoch-a", + &endpoints, + ), + derive_workload_credential_handle( + "sandbox-a", + "provider-b", + "ACCESS_TOKEN", + "epoch-a", + &endpoints, + ), + derive_workload_credential_handle( + "sandbox-a", + "provider-a", + "OTHER_TOKEN", + "epoch-a", + &endpoints, + ), + derive_workload_credential_handle( + "sandbox-a", + "provider-a", + "ACCESS_TOKEN", + "epoch-b", + &endpoints, + ), + derive_workload_credential_handle( + "sandbox-a", + "provider-a", + "ACCESS_TOKEN", + "epoch-a", + &[StaticCredentialEndpointBinding { + host: "api.example.com".to_string(), + port: 443, + path: "/v2/**".to_string(), + }], + ), + ] { + assert_ne!(first, changed); + } + assert_eq!(first.len(), 64); + assert!( + first + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + ); + } + + #[tokio::test] + async fn refresh_managed_handle_survives_rotation_and_reconstruction_then_reconfigure_revokes() + { + let state = test_server_state().await; + let mut profile = custom_profile("stable-refresh-provider"); + profile.credentials = vec![refreshable_credential("access_token", "ACCESS_TOKEN")]; + profile.endpoints = vec![NetworkEndpoint { + host: "api.example.com".to_string(), + port: 443, + path: "/v1/**".to_string(), + protocol: "rest".to_string(), + access: "full".to_string(), + ..Default::default() + }]; + handle_import_provider_profiles( + &state, + authed_request(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(profile), + source: "stable-refresh-provider.yaml".to_string(), + }], + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + let provider = create_provider_record( + state.store.as_ref(), + "default", + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: "stable-refresh".to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + r#type: "stable-refresh-provider".to_string(), + credentials: HashMap::from([("ACCESS_TOKEN".to_string(), "token-1".to_string())]), + profile_workspace: "default".to_string(), + ..Default::default() + }, + ) + .await + .unwrap(); + let configure = || ConfigureProviderRefreshRequest { + provider: "stable-refresh".to_string(), + credential_key: "ACCESS_TOKEN".to_string(), + strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials as i32, + material: HashMap::from([ + ("client_id".to_string(), "client-id".to_string()), + ("client_secret".to_string(), "client-secret".to_string()), + ]), + secret_material_keys: vec!["client_secret".to_string()], + expires_at_ms: None, + workspace: "default".to_string(), + }; + handle_configure_provider_refresh(&state, authed_request(configure())) + .await + .unwrap(); + + let catalog = state + .provider_profile_sources + .snapshot_catalog(state.store.as_ref(), "default") + .await + .unwrap(); + let names = vec!["stable-refresh".to_string()]; + let records = load_provider_environment_records(state.store.as_ref(), "default", &names) + .await + .unwrap(); + let revision_1 = crate::grpc::policy::compute_provider_env_revision_with_catalog( + state.store.as_ref(), + &catalog, + "default", + &names, + ) + .await + .unwrap(); + let first = resolve_provider_environment_from_records_with_policy_bindings_and_credentials( + state.store.as_ref(), + &catalog, + &records, + &HashMap::new(), + &state.credentials, + Some("sandbox-id"), + ) + .await + .unwrap(); + let first_handle = first.static_credential_bindings["ACCESS_TOKEN"] + .workload_credential_handle + .clone(); + assert!(!first_handle.is_empty()); + let credential_state = + openshell_core::provider_credentials::ProviderCredentialState::from_bound_environment( + revision_1, + first.environment.clone(), + first.credential_expires_at_ms.clone(), + first.dynamic_credentials.clone(), + first.static_credential_bindings.clone(), + Vec::new(), + ) + .unwrap(); + let original_placeholder = credential_state.snapshot().child_env["ACCESS_TOKEN"].clone(); + + update_provider_record_with_catalog( + state.store.as_ref(), + &catalog, + "default", + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: "stable-refresh".to_string(), + ..Default::default() + }), + credentials: HashMap::from([("ACCESS_TOKEN".to_string(), "token-2".to_string())]), + ..Default::default() + }, + ) + .await + .unwrap(); + let records = load_provider_environment_records(state.store.as_ref(), "default", &names) + .await + .unwrap(); + let revision_2 = crate::grpc::policy::compute_provider_env_revision_with_catalog( + state.store.as_ref(), + &catalog, + "default", + &names, + ) + .await + .unwrap(); + let second = + resolve_provider_environment_from_records_with_policy_bindings_and_credentials( + state.store.as_ref(), + &catalog, + &records, + &HashMap::new(), + &state.credentials, + Some("sandbox-id"), + ) + .await + .unwrap(); + assert_eq!( + second.static_credential_bindings["ACCESS_TOKEN"].workload_credential_handle, + first_handle + ); + credential_state + .install_bound_environment( + revision_2, + second.environment.clone(), + second.credential_expires_at_ms.clone(), + second.dynamic_credentials.clone(), + second.static_credential_bindings.clone(), + Vec::new(), + ) + .unwrap(); + assert_eq!( + credential_state + .resolver_for_endpoint("api.example.com", 443, "/v1/messages") + .unwrap() + .resolve_placeholder(&original_placeholder), + Some("token-2") + ); + let reconstructed = + openshell_core::provider_credentials::ProviderCredentialState::from_bound_environment( + revision_2, + second.environment.clone(), + second.credential_expires_at_ms.clone(), + second.dynamic_credentials.clone(), + second.static_credential_bindings.clone(), + Vec::new(), + ) + .unwrap(); + assert_eq!( + reconstructed + .resolver_for_endpoint("api.example.com", 443, "/v1/messages") + .unwrap() + .resolve_placeholder(&original_placeholder), + Some("token-2") + ); + + handle_configure_provider_refresh(&state, authed_request(configure())) + .await + .unwrap(); + let revision_3 = crate::grpc::policy::compute_provider_env_revision_with_catalog( + state.store.as_ref(), + &catalog, + "default", + &names, + ) + .await + .unwrap(); + assert_ne!(revision_2, revision_3); + let records = load_provider_environment_records(state.store.as_ref(), "default", &names) + .await + .unwrap(); + let third = resolve_provider_environment_from_records_with_policy_bindings_and_credentials( + state.store.as_ref(), + &catalog, + &records, + &HashMap::new(), + &state.credentials, + Some("sandbox-id"), + ) + .await + .unwrap(); + assert_ne!( + third.static_credential_bindings["ACCESS_TOKEN"].workload_credential_handle, + first_handle + ); + credential_state + .install_bound_environment( + revision_3, + third.environment, + third.credential_expires_at_ms, + third.dynamic_credentials, + third.static_credential_bindings, + Vec::new(), + ) + .unwrap(); + assert_eq!( + credential_state + .resolver_for_endpoint("api.example.com", 443, "/v1/messages") + .unwrap() + .resolve_placeholder(&original_placeholder), + None + ); + assert_eq!(provider.object_name(), "stable-refresh"); + } + #[tokio::test] async fn resolve_provider_env_withholds_endpointless_profile_credentials_independently() { let store = test_store().await; @@ -8012,6 +8478,7 @@ mod tests { &records, &HashMap::new(), &credentials, + None, ) .await .unwrap(); @@ -8036,6 +8503,7 @@ mod tests { &records, &policy_bindings, &credentials, + None, ) .await .unwrap(); diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index dc039265f6..ff6e3df596 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -39,6 +39,28 @@ pub fn refresh_state_name(provider_id: &str, credential_key: &str) -> String { format!("provider-refresh-{provider_id}-{key}") } +/// Return the durable authorization epoch for one configured refresh grant. +/// +/// Records created before the explicit epoch field was introduced use their +/// gateway-generated object ID as a stable migration epoch. An explicit +/// reconfiguration writes a new random epoch while preserving object metadata, +/// so reauthorization still revokes handles derived from the legacy value. +pub fn effective_authorization_epoch( + state: &StoredProviderCredentialRefreshState, +) -> Result<&str, Status> { + if !state.authorization_epoch.is_empty() { + return Ok(&state.authorization_epoch); + } + state + .metadata + .as_ref() + .map(|metadata| metadata.id.as_str()) + .filter(|id| !id.is_empty()) + .ok_or_else(|| { + Status::failed_precondition("provider refresh state has no authorization epoch") + }) +} + pub async fn put_refresh_state( store: &Store, state: &StoredProviderCredentialRefreshState, @@ -270,6 +292,7 @@ pub fn new_refresh_state( refresh_before_seconds: config.refresh_before_seconds, max_lifetime_seconds: config.max_lifetime_seconds, additional_output_keys: config.additional_output_keys, + authorization_epoch: uuid::Uuid::new_v4().to_string(), }) } @@ -1234,9 +1257,9 @@ async fn run_refresh_worker_tick( #[cfg(test)] mod tests { use super::{ - NewRefreshStateConfig, delete_refresh_state, get_refresh_state, new_refresh_state, - put_refresh_state, refresh_provider_credential, refresh_state_name, refresh_strategy_name, - run_refresh_worker_tick, seconds_until_ms, + NewRefreshStateConfig, delete_refresh_state, effective_authorization_epoch, + get_refresh_state, new_refresh_state, put_refresh_state, refresh_provider_credential, + refresh_state_name, refresh_strategy_name, run_refresh_worker_tick, seconds_until_ms, }; use crate::credentials::CredentialRuntime; use crate::persistence::{current_time_ms, test_store}; @@ -1268,6 +1291,43 @@ mod tests { ); } + #[test] + fn new_refresh_configuration_rotates_authorization_epoch_and_legacy_state_is_stable() { + let provider = Provider { + metadata: Some(ObjectMeta { + id: "provider-id".to_string(), + name: "provider".to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + ..Default::default() + }; + let config = || NewRefreshStateConfig { + strategy: ProviderCredentialRefreshStrategy::Oauth2RefreshToken, + material: HashMap::new(), + secret_material_keys: Vec::new(), + expires_at_ms: 0, + token_url: "https://issuer.example/token".to_string(), + scopes: vec!["scope".to_string()], + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + additional_output_keys: HashMap::new(), + }; + let first = new_refresh_state(&provider, "default", "ACCESS_TOKEN", config()) + .expect("first refresh configuration"); + let second = new_refresh_state(&provider, "default", "ACCESS_TOKEN", config()) + .expect("second refresh configuration"); + assert!(!first.authorization_epoch.is_empty()); + assert_ne!(first.authorization_epoch, second.authorization_epoch); + + let mut legacy = first; + legacy.authorization_epoch.clear(); + assert_eq!( + effective_authorization_epoch(&legacy).expect("legacy migration epoch"), + legacy.metadata.as_ref().expect("metadata").id + ); + } + #[test] fn refresh_log_helpers_format_safe_operational_fields() { assert_eq!(seconds_until_ms(1_000, 61_000), 60); @@ -1335,6 +1395,7 @@ mod tests { ) .unwrap(); put_refresh_state(&store, &state).await.unwrap(); + let authorization_epoch = state.authorization_epoch.clone(); let refreshed = refresh_provider_credential( &store, @@ -1346,6 +1407,7 @@ mod tests { ) .await .unwrap(); + assert_eq!(refreshed.authorization_epoch, authorization_epoch); assert_eq!(refreshed.status, "refreshed"); assert!(refreshed.expires_at_ms > 0); assert!(refreshed.next_refresh_at_ms > 0); @@ -1404,6 +1466,7 @@ mod tests { ) .unwrap(); put_refresh_state(&store, &state).await.unwrap(); + let authorization_epoch = state.authorization_epoch.clone(); let config = Config::new(None).with_credential_drivers(["test-static"]); let credentials = CredentialRuntime::from_config(&config).unwrap(); @@ -1417,6 +1480,7 @@ mod tests { ) .await .unwrap(); + assert_eq!(refreshed.authorization_epoch, authorization_epoch); let stored = store .get_message_by_name::("default", "my-stored-graph") diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 6bd0a98476..c953a113a9 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2807,6 +2807,7 @@ mod tests { path: "/allowed/**".to_string(), }], credential_identity: identity.to_string(), + workload_credential_handle: String::new(), } } @@ -4374,6 +4375,7 @@ network_policies: path: "/v1/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -4477,6 +4479,7 @@ network_policies: path: "/v1/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -5125,6 +5128,7 @@ network_policies: path: "/allowed".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -7108,6 +7112,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), diff --git a/crates/openshell-supervisor-network/src/l7/websocket.rs b/crates/openshell-supervisor-network/src/l7/websocket.rs index 7ba286b103..db73104dd3 100644 --- a/crates/openshell-supervisor-network/src/l7/websocket.rs +++ b/crates/openshell-supervisor-network/src/l7/websocket.rs @@ -2639,6 +2639,7 @@ network_policies: path: "/socket".to_string(), }], credential_identity: "provider-a:DISCORD_BOT_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index b4da286bf8..94187abc30 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -6666,6 +6666,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -9396,6 +9397,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -9437,6 +9439,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -9482,6 +9485,7 @@ network_policies: path: "/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -10172,6 +10176,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: "provider-a:API_TOKEN".to_string(), + workload_credential_handle: String::new(), }, )]), Vec::new(), @@ -10259,6 +10264,7 @@ network_policies: path: "/allowed/**".to_string(), }], credential_identity: format!("provider-a:{key}"), + workload_credential_handle: String::new(), }, ) }) diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index f867b84bd5..695950f8ae 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -177,10 +177,17 @@ HTTP 101 upgrade has completed. OpenShell closes that WebSocket with policy violation code 1008 instead of returning an HTTP response. Binding updates apply to both current and retained placeholder generations. -Credential rotation keeps the current endpoint set. Updating a provider profile -changes the binding on the next sandbox provider-environment sync, and detaching -the provider revokes resolution for placeholders already held by running -processes. +For gateway-managed refresh credentials, automatic and manual token rotation +keep one opaque workload placeholder and replace only its current resolver +value. This lets a long-running process use each newly minted access token +without restarting. OpenShell does not retain older values behind this stable +placeholder. + +Explicitly configuring refresh again starts a new authorization epoch, even +when the provider and credential key are unchanged. Reconfiguration, provider +replacement or detachment, refresh deletion, and endpoint-boundary changes +revoke the old placeholder. Updating a provider profile changes the binding on +the next sandbox provider-environment sync. Static credentials require at least one usable binding. OpenShell withholds only @@ -196,6 +203,10 @@ credential material from supervisors that do not advertise binding support. Rotate attached static credentials after upgrading when an older sandbox may have received their real values. +When upgrading from revision-scoped refresh placeholders to stable refresh +handles, restart each existing workload once so it receives the new placeholder. +Later access-token rotations do not require workload restarts. + ## Roadmap The following Providers v2 design items are not part of the current behavior: @@ -608,6 +619,12 @@ The gateway sends a complete host, port, and path binding for every emitted stat Refresh configuration is stored separately from the current injectable credential value. The gateway refresh worker reads refresh state, mints a new short-lived token for supported strategies, writes the token back to the provider record, and updates credential expiry metadata. +Each explicit `refresh configure` call also starts a new gateway-owned +authorization epoch. Automatic refresh and `refresh rotate` preserve that epoch, +so running workloads keep the same opaque credential handle while the short-lived +token changes. Configuring refresh again is a revocation boundary and causes +running workloads that still hold the previous handle to fail closed. + For a complete Microsoft Graph OAuth2 refresh-token walkthrough, see [Refresh Microsoft Graph Credentials with Providers v2](/get-started/tutorials/microsoft-graph-provider-refresh). The profile YAML strategy values use underscores, while the CLI `--strategy` values use kebab-case: diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 7262a52910..40fd05a122 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -39,7 +39,7 @@ def _is_placeholder_for_env_key(value: str, key: str) -> bool: token = value.removeprefix(prefix) if token == value: return False - return token.startswith("v") and token.endswith(f"_{key}") + return token.startswith(("v", "s")) and token.endswith(f"_{key}") def _default_policy() -> sandbox_pb2.SandboxPolicy: diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 078dabfdf7..7f6c6467d9 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -86,6 +86,11 @@ name = "podman_userns" path = "tests/podman_userns.rs" required-features = ["e2e-podman"] +[[test]] +name = "provider_refresh_handles" +path = "tests/provider_refresh_handles.rs" +required-features = ["e2e-podman"] + [[test]] name = "vm_gateway_start" path = "tests/vm_gateway_start.rs" diff --git a/e2e/rust/tests/provider_refresh_handles.rs b/e2e/rust/tests/provider_refresh_handles.rs new file mode 100644 index 0000000000..a12281f3e4 --- /dev/null +++ b/e2e/rust/tests/provider_refresh_handles.rs @@ -0,0 +1,356 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-podman")] + +//! Podman E2E coverage for refresh-managed workload credential handles. +//! +//! A fake issuer invalidates every prior access token. One long-running shell +//! retains its original environment while the gateway rotates the provider 12 +//! times. The shell must continue reaching the resource with the newest token, +//! and explicit refresh reconfiguration must revoke its old handle. + +use std::io::Write; +use std::process::Stdio; +use std::time::Duration; + +use openshell_e2e::harness::binary::openshell_cmd; +use openshell_e2e::harness::container::HostSupportContainer; +use openshell_e2e::harness::sandbox::SandboxGuard; +use tempfile::{Builder as TempFileBuilder, NamedTempFile}; + +const PROVIDER_NAME: &str = "e2e-stable-refresh-handle"; +const PROFILE_ID: &str = "e2e-stable-refresh-handle"; +const TOKEN_ENV: &str = "REFRESH_E2E_ACCESS_TOKEN"; +const READY_MARKER: &str = "stable-refresh-parent-ready"; + +const FIXTURE_SCRIPT: &str = r#" +import json +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + +current_token = "bootstrap-token" +generation = 0 + +class Handler(BaseHTTPRequestHandler): + def do_POST(self): + global current_token, generation + if self.path != "/token": + self.send_response(404) + self.end_headers() + return + generation += 1 + current_token = f"access-token-{generation}" + body = json.dumps({ + "access_token": current_token, + "expires_in": 300, + "token_type": "Bearer", + }).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + if self.path == "/": + self.send_response(204) + elif self.path == "/probe" and self.headers.get("Authorization") == f"Bearer {current_token}": + self.send_response(204) + else: + self.send_response(401) + self.end_headers() + + def log_message(self, *_args): + pass + +ThreadingHTTPServer(("0.0.0.0", 8000), Handler).serve_forever() +"#; + +async fn run_cli(args: &[&str]) -> Result { + run_cli_with_env(args, &[]).await +} + +async fn run_cli_with_env(args: &[&str], env: &[(&str, &str)]) -> Result { + let mut command = openshell_cmd(); + command + .args(args) + .envs(env.iter().copied()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let output = command + .output() + .await + .map_err(|error| format!("run openshell command: {error}"))?; + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + if !output.status.success() { + return Err(format!( + "openshell command failed (exit {:?}):\n{combined}", + output.status.code() + )); + } + Ok(combined) +} + +async fn delete_provider_resources() { + let _ = run_cli(&["provider", "delete", PROVIDER_NAME]).await; + let _ = run_cli(&["provider", "profile", "delete", PROFILE_ID]).await; +} + +fn write_profile(resource_port: u16, token_port: u16) -> Result { + let mut file = TempFileBuilder::new() + .suffix(".yaml") + .tempfile() + .map_err(|error| format!("create profile: {error}"))?; + let profile = format!( + r#"id: {PROFILE_ID} +display_name: Stable refresh handle E2E +category: other +credentials: + - name: access_token + env_vars: [{TOKEN_ENV}] + required: true + auth_style: bearer + header_name: authorization + refresh: + strategy: oauth2_client_credentials + token_url: http://127.0.0.1:{token_port}/token + refresh_before_seconds: 30 + max_lifetime_seconds: 300 + material: + - name: client_id + required: true + - name: client_secret + required: true + secret: true +endpoints: + - host: host.openshell.internal + port: {resource_port} + path: /probe + protocol: rest + access: full + enforcement: enforce + allowed_ips: + - 10.0.0.0/8 + - 169.254.0.0/16 + - 172.0.0.0/8 + - 192.168.0.0/16 +binaries: + - /usr/bin/curl +"# + ); + file.write_all(profile.as_bytes()) + .map_err(|error| format!("write profile: {error}"))?; + file.flush() + .map_err(|error| format!("flush profile: {error}"))?; + Ok(file) +} + +fn write_policy(resource_port: u16) -> Result { + let mut file = TempFileBuilder::new() + .suffix(".yaml") + .tempfile() + .map_err(|error| format!("create policy: {error}"))?; + let policy = format!( + r#"version: 1 +filesystem_policy: + include_workdir: true + read_only: [/usr, /lib, /proc, /etc, /dev/urandom] + read_write: [/sandbox, /tmp, /dev/null] +landlock: + compatibility: best_effort +process: + run_as_user: sandbox + run_as_group: sandbox +network_policies: + refresh_probe: + name: refresh_probe + endpoints: + - host: host.openshell.internal + port: {resource_port} + path: /probe + protocol: rest + access: full + enforcement: enforce + allowed_ips: + - 10.0.0.0/8 + - 169.254.0.0/16 + - 172.0.0.0/8 + - 192.168.0.0/16 + binaries: + - path: /usr/bin/curl +"# + ); + file.write_all(policy.as_bytes()) + .map_err(|error| format!("write policy: {error}"))?; + file.flush() + .map_err(|error| format!("flush policy: {error}"))?; + Ok(file) +} + +async fn configure_refresh(profile: &NamedTempFile) -> Result<(), String> { + let profile_path = profile.path().to_string_lossy().into_owned(); + run_cli(&["provider", "profile", "import", "--file", &profile_path]).await?; + run_cli_with_env( + &[ + "provider", + "create", + "--name", + PROVIDER_NAME, + "--type", + PROFILE_ID, + "--credential", + TOKEN_ENV, + ], + &[(TOKEN_ENV, "bootstrap-token")], + ) + .await?; + reconfigure_refresh().await +} + +async fn reconfigure_refresh() -> Result<(), String> { + run_cli_with_env( + &[ + "provider", + "refresh", + "configure", + PROVIDER_NAME, + "--credential-key", + TOKEN_ENV, + "--strategy", + "oauth2-client-credentials", + "--material", + "client_id=e2e-client", + "--secret-material-env", + "client_secret=REFRESH_E2E_CLIENT_SECRET", + ], + &[("REFRESH_E2E_CLIENT_SECRET", "e2e-client-secret")], + ) + .await + .map(|_| ()) +} + +async fn rotate() -> Result<(), String> { + run_cli(&[ + "provider", + "refresh", + "rotate", + PROVIDER_NAME, + "--credential-key", + TOKEN_ENV, + ]) + .await + .map(|_| ()) +} + +async fn trigger_probe(sandbox: &SandboxGuard) -> Result { + sandbox + .exec(&[ + "sh", + "-c", + "rm -f /sandbox/probe-result; touch /sandbox/probe-trigger", + ]) + .await?; + let deadline = tokio::time::Instant::now() + Duration::from_secs(10); + loop { + if let Ok(result) = sandbox.exec(&["cat", "/sandbox/probe-result"]).await { + return Ok(result.trim().to_string()); + } + if tokio::time::Instant::now() >= deadline { + return Err("timed out waiting for long-running credential probe".to_string()); + } + tokio::time::sleep(Duration::from_millis(100)).await; + } +} + +async fn wait_for_probe_success(sandbox: &SandboxGuard) -> Result<(), String> { + let deadline = tokio::time::Instant::now() + Duration::from_secs(45); + loop { + if trigger_probe(sandbox).await? == "ok" { + return Ok(()); + } + if tokio::time::Instant::now() >= deadline { + return Err("long-running process never resolved the latest rotated token".to_string()); + } + tokio::time::sleep(Duration::from_millis(500)).await; + } +} + +async fn wait_for_probe_failure(sandbox: &SandboxGuard) -> Result<(), String> { + let deadline = tokio::time::Instant::now() + Duration::from_secs(45); + loop { + if trigger_probe(sandbox).await? == "failed" { + return Ok(()); + } + if tokio::time::Instant::now() >= deadline { + return Err("old workload handle survived explicit reconfiguration".to_string()); + } + tokio::time::sleep(Duration::from_millis(500)).await; + } +} + +#[tokio::test] +async fn long_running_process_survives_rotations_and_reconfigure_revokes() -> Result<(), String> { + delete_provider_resources().await; + let fixture = HostSupportContainer::start_python(FIXTURE_SCRIPT, 8000).await?; + let profile = write_profile(fixture.port, fixture.port)?; + let policy = write_policy(fixture.port)?; + configure_refresh(&profile).await?; + + let policy_path = policy.path().to_string_lossy().into_owned(); + let resource_url = format!("http://host.openshell.internal:{}/probe", fixture.port); + let parent_script = format!( + r#"case "$REFRESH_E2E_ACCESS_TOKEN" in + openshell:resolve:env:s*_REFRESH_E2E_ACCESS_TOKEN) ;; + *) exit 64 ;; +esac +echo {READY_MARKER} +while true; do + if [ -f /sandbox/probe-trigger ]; then + rm -f /sandbox/probe-trigger + if curl --fail --silent --output /dev/null \ + --header "Authorization: Bearer $REFRESH_E2E_ACCESS_TOKEN" \ + {resource_url}; then + echo ok > /sandbox/probe-result + else + echo failed > /sandbox/probe-result + fi + fi + sleep 0.1 +done"# + ); + let mut sandbox = SandboxGuard::create_keep_with_args( + &["--provider", PROVIDER_NAME, "--policy", &policy_path], + &["sh", "-c", &parent_script], + READY_MARKER, + ) + .await?; + + let result = async { + if trigger_probe(&sandbox).await? != "ok" { + return Err("initial long-running credential probe failed".to_string()); + } + + for _ in 0..12 { + rotate().await?; + } + wait_for_probe_success(&sandbox).await?; + + reconfigure_refresh().await?; + wait_for_probe_failure(&sandbox).await?; + + let fresh_probe = format!( + "curl --fail --silent --output /dev/null --header \"Authorization: Bearer $REFRESH_E2E_ACCESS_TOKEN\" {resource_url}" + ); + sandbox.exec(&["sh", "-c", &fresh_probe]).await?; + Ok(()) + } + .await; + + sandbox.cleanup().await; + delete_provider_resources().await; + result +} diff --git a/proto/openshell.proto b/proto/openshell.proto index a30852664c..6d756721f6 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -1594,6 +1594,11 @@ message StoredProviderCredentialRefreshState { // collision reservation, and env-key surfacing so later profile edits cannot // silently redirect writes. map additional_output_keys = 17; + // Opaque gateway-owned authorization epoch for the configured refresh + // grant. Explicit refresh configuration creates a new epoch; automatic and + // manual token rotation preserve it. It is never derived from or exposed + // with refresh material. + string authorization_epoch = 18; } message GetProviderRefreshStatusRequest { @@ -1790,6 +1795,12 @@ message StaticCredentialBinding { // Supervisors use it to retain old revision placeholders only across // rotations of the same provider credential. string credential_identity = 2; + // Opaque gateway-issued handle for a refresh-managed credential identity + // epoch. When non-empty, supervisors keep the workload placeholder stable + // across access-token rotations and replace only the resolver value. The + // handle changes when the sandbox, provider, credential key, refresh + // authorization epoch, or endpoint authorization boundary changes. + string workload_credential_handle = 3; } // Get sandbox provider environment response. diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index ecff3a1e17..acf7d6554e 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -5854,8 +5854,13 @@ type StoredProviderCredentialRefreshState struct { // collision reservation, and env-key surfacing so later profile edits cannot // silently redirect writes. AdditionalOutputKeys map[string]string `protobuf:"bytes,17,rep,name=additional_output_keys,json=additionalOutputKeys,proto3" json:"additional_output_keys,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Opaque gateway-owned authorization epoch for the configured refresh + // grant. Explicit refresh configuration creates a new epoch; automatic and + // manual token rotation preserve it. It is never derived from or exposed + // with refresh material. + AuthorizationEpoch string `protobuf:"bytes,18,opt,name=authorization_epoch,json=authorizationEpoch,proto3" json:"authorization_epoch,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *StoredProviderCredentialRefreshState) Reset() { @@ -6007,6 +6012,13 @@ func (x *StoredProviderCredentialRefreshState) GetAdditionalOutputKeys() map[str return nil } +func (x *StoredProviderCredentialRefreshState) GetAuthorizationEpoch() string { + if x != nil { + return x.AuthorizationEpoch + } + return "" +} + type GetProviderRefreshStatusRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Provider string `protobuf:"bytes,1,opt,name=provider,proto3" json:"provider,omitempty"` @@ -7383,8 +7395,14 @@ type StaticCredentialBinding struct { // Supervisors use it to retain old revision placeholders only across // rotations of the same provider credential. CredentialIdentity string `protobuf:"bytes,2,opt,name=credential_identity,json=credentialIdentity,proto3" json:"credential_identity,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Opaque gateway-issued handle for a refresh-managed credential identity + // epoch. When non-empty, supervisors keep the workload placeholder stable + // across access-token rotations and replace only the resolver value. The + // handle changes when the sandbox, provider, credential key, refresh + // authorization epoch, or endpoint authorization boundary changes. + WorkloadCredentialHandle string `protobuf:"bytes,3,opt,name=workload_credential_handle,json=workloadCredentialHandle,proto3" json:"workload_credential_handle,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *StaticCredentialBinding) Reset() { @@ -7431,6 +7449,13 @@ func (x *StaticCredentialBinding) GetCredentialIdentity() string { return "" } +func (x *StaticCredentialBinding) GetWorkloadCredentialHandle() string { + if x != nil { + return x.WorkloadCredentialHandle + } + return "" +} + // Get sandbox provider environment response. type GetSandboxProviderEnvironmentResponse struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -13481,7 +13506,7 @@ const file_openshell_proto_rawDesc = "" + "\n" + "last_error\x18\t \x01(\tR\tlastError\"<\n" + "\x18ProviderProfileDiscovery\x12 \n" + - "\vcredentials\x18\x01 \x03(\tR\vcredentials\"\x93\b\n" + + "\vcredentials\x18\x01 \x03(\tR\vcredentials\"\xc4\b\n" + "$StoredProviderCredentialRefreshState\x12>\n" + "\bmetadata\x18\x01 \x01(\v2\".openshell.datamodel.v1.ObjectMetaR\bmetadata\x12\x1f\n" + "\vprovider_id\x18\x02 \x01(\tR\n" + @@ -13502,7 +13527,8 @@ const file_openshell_proto_rawDesc = "" + "\x06scopes\x18\x0e \x03(\tR\x06scopes\x124\n" + "\x16refresh_before_seconds\x18\x0f \x01(\x03R\x14refreshBeforeSeconds\x120\n" + "\x14max_lifetime_seconds\x18\x10 \x01(\x03R\x12maxLifetimeSeconds\x12\x82\x01\n" + - "\x16additional_output_keys\x18\x11 \x03(\v2L.openshell.v1.StoredProviderCredentialRefreshState.AdditionalOutputKeysEntryR\x14additionalOutputKeys\x1a;\n" + + "\x16additional_output_keys\x18\x11 \x03(\v2L.openshell.v1.StoredProviderCredentialRefreshState.AdditionalOutputKeysEntryR\x14additionalOutputKeys\x12/\n" + + "\x13authorization_epoch\x18\x12 \x01(\tR\x12authorizationEpoch\x1a;\n" + "\rMaterialEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\x1aG\n" + @@ -13602,10 +13628,11 @@ const file_openshell_proto_rawDesc = "" + "\x1fStaticCredentialEndpointBinding\x12\x12\n" + "\x04host\x18\x01 \x01(\tR\x04host\x12\x12\n" + "\x04port\x18\x02 \x01(\rR\x04port\x12\x12\n" + - "\x04path\x18\x03 \x01(\tR\x04path\"\x97\x01\n" + + "\x04path\x18\x03 \x01(\tR\x04path\"\xd5\x01\n" + "\x17StaticCredentialBinding\x12K\n" + "\tendpoints\x18\x01 \x03(\v2-.openshell.v1.StaticCredentialEndpointBindingR\tendpoints\x12/\n" + - "\x13credential_identity\x18\x02 \x01(\tR\x12credentialIdentity\"\x90\b\n" + + "\x13credential_identity\x18\x02 \x01(\tR\x12credentialIdentity\x12<\n" + + "\x1aworkload_credential_handle\x18\x03 \x01(\tR\x18workloadCredentialHandle\"\x90\b\n" + "%GetSandboxProviderEnvironmentResponse\x12l\n" + "\venvironment\x18\x01 \x03(\v2D.openshell.v1.GetSandboxProviderEnvironmentResponse.EnvironmentEntryB\x04\x88\xb5\x18\x01R\venvironment\x122\n" + "\x15provider_env_revision\x18\x02 \x01(\x04R\x13providerEnvRevision\x12\x87\x01\n" + From 4bbd69b5312c7e84a109e59b8023e1df25ad170c Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Tue, 18 Aug 2026 12:10:40 -0700 Subject: [PATCH 2/2] fix(providers): protect refresh-owned credentials Signed-off-by: Mrunal Patel --- .agents/skills/openshell-cli/SKILL.md | 6 + architecture/sandbox.md | 6 +- crates/openshell-server/src/grpc/provider.rs | 238 +++++++++++++++++-- docs/sandboxes/providers-v2.mdx | 8 + 4 files changed, 240 insertions(+), 18 deletions(-) diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index f082a899f8..1cee3d5c37 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -188,6 +188,12 @@ placeholders. A later `provider refresh configure` call is an explicit reauthorization boundary: it revokes the previous handle, and processes holding that handle fail closed until restarted. +While gateway-managed refresh is configured, `provider update --credential` +cannot replace or delete the refresh-owned primary credential or any co-minted +output. Use `provider refresh rotate`, reconfigure refresh, or delete refresh +before returning those keys to manual management. Unrelated provider fields +remain updateable. + --- ## Workflow 3: Sandbox Lifecycle diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 12dc940ec4..698f88a80a 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -94,8 +94,10 @@ rotates the short-lived value, so an already-running process keeps one placeholder and each request resolves against the current token. Explicit refresh reconfiguration, provider replacement or detachment, and endpoint boundary changes produce a new handle and revoke the old one. Supervisors do -not retain old values for these handles. Unmanaged static credentials retain -the bounded revision-generation behavior. +not retain old values for these handles. Public provider updates cannot replace +or delete the refresh-owned primary credential or co-minted outputs; internal +CAS rotation and explicit refresh lifecycle operations own those values. +Unmanaged static credentials retain the bounded revision-generation behavior. Route selection and policy evaluation use a syntax-only redacted request target; they do not materialize real credentials. Cross-endpoint placeholder use returns diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 4dc90a5f8d..66d66be7fb 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -305,6 +305,46 @@ pub(super) async fn update_provider_record_with_catalog( update_provider_record_validating(store, workspace, catalog, provider, None).await } +async fn reject_refresh_owned_credential_updates( + store: &Store, + provider_id: &str, + credential_updates: &HashMap, +) -> Result<(), Status> { + if credential_updates.is_empty() { + return Ok(()); + } + + let mut refresh_owned_keys = HashSet::new(); + for refresh_state in + crate::provider_refresh::list_refresh_states_for_provider(store, provider_id).await? + { + let strategy = + ProviderCredentialRefreshStrategy::try_from(refresh_state.strategy).unwrap_or_default(); + if !crate::provider_refresh::is_gateway_mintable_strategy(strategy) { + continue; + } + + for key in std::iter::once(refresh_state.credential_key) + .chain(refresh_state.additional_output_keys.into_values()) + { + if credential_updates.contains_key(&key) { + refresh_owned_keys.insert(key); + } + } + } + + if refresh_owned_keys.is_empty() { + return Ok(()); + } + + let mut refresh_owned_keys: Vec<_> = refresh_owned_keys.into_iter().collect(); + refresh_owned_keys.sort(); + Err(Status::failed_precondition(format!( + "credentials managed by provider refresh cannot be updated or deleted with provider update: {}; use provider refresh rotate, configure, or delete", + refresh_owned_keys.join(", ") + ))) +} + async fn update_provider_record_validating( store: &Store, workspace: &str, @@ -352,6 +392,9 @@ async fn update_provider_record_validating( )); } + reject_refresh_owned_credential_updates(store, existing.object_id(), &provider.credentials) + .await?; + let current_version = existing.metadata.as_ref().map_or(0, |m| m.resource_version); let cas_version = if expected_resource_version == 0 { @@ -8073,8 +8116,8 @@ mod tests { } #[tokio::test] - async fn refresh_managed_handle_survives_rotation_and_reconstruction_then_reconfigure_revokes() - { + async fn refresh_managed_handle_rejects_manual_replacement_survives_rotation_and_reconstruction_then_reconfigure_revokes() + { let state = test_server_state().await; let mut profile = custom_profile("stable-refresh-provider"); profile.credentials = vec![refreshable_credential("access_token", "ACCESS_TOKEN")]; @@ -8174,21 +8217,84 @@ mod tests { .unwrap(); let original_placeholder = credential_state.snapshot().child_env["ACCESS_TOKEN"].clone(); - update_provider_record_with_catalog( - state.store.as_ref(), - &catalog, - "default", - Provider { - metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { - name: "stable-refresh".to_string(), - ..Default::default() + for value in ["token-2", ""] { + let err = handle_update_provider( + &state, + authed_request(UpdateProviderRequest { + provider: Some(Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: "stable-refresh".to_string(), + ..Default::default() + }), + credentials: HashMap::from([( + "ACCESS_TOKEN".to_string(), + value.to_string(), + )]), + ..Default::default() + }), + credential_expires_at_ms: HashMap::new(), + workspace: "default".to_string(), }), - credentials: HashMap::from([("ACCESS_TOKEN".to_string(), "token-2".to_string())]), - ..Default::default() - }, - ) - .await - .unwrap(); + ) + .await + .unwrap_err(); + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("provider refresh")); + assert!(err.message().contains("ACCESS_TOKEN")); + } + + let unchanged = state + .store + .get_message_by_name::("default", "stable-refresh") + .await + .unwrap() + .unwrap(); + assert_eq!(unchanged.credentials["ACCESS_TOKEN"], "token-1"); + let unchanged_records = + load_provider_environment_records(state.store.as_ref(), "default", &names) + .await + .unwrap(); + let unchanged_environment = + resolve_provider_environment_from_records_with_policy_bindings_and_credentials( + state.store.as_ref(), + &catalog, + &unchanged_records, + &HashMap::new(), + &state.credentials, + Some("sandbox-id"), + ) + .await + .unwrap(); + credential_state + .install_bound_environment( + revision_1, + unchanged_environment.environment, + unchanged_environment.credential_expires_at_ms, + unchanged_environment.dynamic_credentials, + unchanged_environment.static_credential_bindings, + Vec::new(), + ) + .unwrap(); + assert_eq!( + credential_state + .resolver_for_endpoint("api.example.com", 443, "/v1/messages") + .unwrap() + .resolve_placeholder(&original_placeholder), + Some("token-1") + ); + + // The gateway refresh path owns credential rotation and writes the + // minted value with an internal CAS, preserving the authorization + // epoch and therefore the workload handle. + state + .store + .update_message_cas::(provider.object_id(), 0, |current| { + current + .credentials + .insert("ACCESS_TOKEN".to_string(), "token-2".to_string()); + }) + .await + .unwrap(); let records = load_provider_environment_records(state.store.as_ref(), "default", &names) .await .unwrap(); @@ -10541,6 +10647,106 @@ mod tests { ); } + #[tokio::test] + async fn update_provider_rejects_gateway_refresh_primary_and_additional_output_keys() { + use crate::grpc::policy::set_global_bool_setting_for_test; + + let state = test_server_state().await; + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + + let original_credentials = HashMap::from([ + ( + "AWS_ACCESS_KEY_ID".to_string(), + "original-access-key".to_string(), + ), + ( + "AWS_SECRET_ACCESS_KEY".to_string(), + "original-secret-key".to_string(), + ), + ( + "AWS_SESSION_TOKEN".to_string(), + "original-session-token".to_string(), + ), + ]); + let provider = create_provider_record( + state.store.as_ref(), + "default", + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: "aws-update-guard".to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + r#type: "aws".to_string(), + credentials: original_credentials.clone(), + ..Default::default() + }, + ) + .await + .unwrap(); + + handle_configure_provider_refresh( + &state, + authed_request(ConfigureProviderRefreshRequest { + provider: "aws-update-guard".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + for key in [ + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + ] { + for value in ["replacement", ""] { + let err = handle_update_provider( + &state, + authed_request(UpdateProviderRequest { + provider: Some(Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: "aws-update-guard".to_string(), + ..Default::default() + }), + credentials: HashMap::from([(key.to_string(), value.to_string())]), + ..Default::default() + }), + credential_expires_at_ms: HashMap::new(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap_err(); + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("provider refresh")); + assert!(err.message().contains(key)); + } + } + + let unchanged = state + .store + .get_message::(provider.object_id()) + .await + .unwrap() + .unwrap(); + assert_eq!(unchanged.credentials, original_credentials); + } + #[tokio::test] async fn configure_aws_sts_requires_profile_declaring_the_refresh() { use crate::grpc::StoredSettingValue; diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index 695950f8ae..9395de3156 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -625,6 +625,14 @@ so running workloads keep the same opaque credential handle while the short-live token changes. Configuring refresh again is a revocation boundary and causes running workloads that still hold the previous handle to fail closed. +While gateway-managed refresh is configured, `provider update --credential` +cannot replace or delete its primary credential or any co-minted output. Use +`provider refresh rotate` to mint a new short-lived value. Re-run +`provider refresh configure` to start a new authorization, or use +`provider refresh delete` before returning those credential keys to manual +management. You can still update unrelated credentials, configuration, and +credential expiry metadata. + For a complete Microsoft Graph OAuth2 refresh-token walkthrough, see [Refresh Microsoft Graph Credentials with Providers v2](/get-started/tutorials/microsoft-graph-provider-refresh). The profile YAML strategy values use underscores, while the CLI `--strategy` values use kebab-case: