diff --git a/crates/openshell-core/src/provider_credentials.rs b/crates/openshell-core/src/provider_credentials.rs index ab9bd26195..8fd7d95712 100644 --- a/crates/openshell-core/src/provider_credentials.rs +++ b/crates/openshell-core/src/provider_credentials.rs @@ -3,7 +3,10 @@ //! Runtime provider credential snapshots. -use crate::secrets::{CredentialPlacement, SecretResolver}; +use crate::secrets::{ + CredentialPlacement, CredentialUseMarker, CredentialUses, STALE_CREDENTIAL_VERSION, + SecretResolver, +}; use crate::{ endpoint_path::EndpointPathPattern, host_pattern::HostPattern, @@ -57,6 +60,158 @@ struct ProviderCredentialStateInner { // Revocation must retain this requirement so retrying against a downgraded // gateway cannot silently publish revision-scoped replacements. requires_stable_placeholders: bool, + /// Per-credential versions, refreshed lazily from `current`; see + /// [`CredentialVersions`]. Locked only while the state lock is held. + credential_versions: std::sync::Mutex, +} + +/// One version per credential, owned by the credential state. +/// +/// Versions come from a single counter that only increases, and a +/// credential's version changes exactly when its content does: a static +/// credential's current-generation value, or a token grant's configuration. +/// A removed credential loses its version, and one added again later gets a +/// new one, so a connection can never see an old version become current +/// again. +/// +/// Every change to credential material replaces `current` with a new +/// snapshot, so versions are recomputed on the first read after `current` +/// changes, not by each install path. The previous snapshot and resolver are +/// held to compare against, which also keeps their allocations alive so a +/// later snapshot can never reuse their addresses. +#[derive(Default)] +struct CredentialVersions { + observed: Option>, + observed_resolver: Option>, + last_version: u64, + statics: Arc>, + grants: HashMap, +} + +impl fmt::Debug for CredentialVersions { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CredentialVersions") + .field("last_version", &self.last_version) + .field("statics", &self.statics.len()) + .field("grants", &self.grants.len()) + .finish_non_exhaustive() + } +} + +/// The version of every credential installed at one moment; see +/// [`ProviderCredentialState::installed_credential_versions`]. +#[derive(Debug, Clone)] +pub struct InstalledCredentialVersions { + statics: Arc>, + grants: HashMap, +} + +impl InstalledCredentialVersions { + /// Whether both describe the same credentials at the same versions. A + /// version changes only when its credential's content does, and a removed + /// credential added back gets a new one, so this is false exactly when a + /// credential was added, changed or removed in between. + #[must_use] + pub fn same_credentials(&self, other: &Self) -> bool { + let Self { statics, grants } = self; + **statics == *other.statics && *grants == other.grants + } +} + + +/// Token-grant dynamic credentials sharing one `provider:credential` +/// identity, keyed by their raw dynamic credential key and sorted. +type GrantConfigs = Vec<(String, crate::proto::ProviderProfileCredential)>; + +/// Group a snapshot's token-grant credentials by identity. +fn grants_by_identity(snapshot: &ProviderCredentialSnapshot) -> HashMap<&str, GrantConfigs> { + let mut grants: HashMap<&str, GrantConfigs> = HashMap::new(); + for (key, credential) in &snapshot.dynamic_credentials { + if let Some(identity) = crate::dynamic_credential_key::credential_identity(key) { + grants + .entry(identity) + .or_default() + .push((key.clone(), credential.clone())); + } + } + for configs in grants.values_mut() { + configs.sort_by(|left, right| left.0.cmp(&right.0)); + } + grants +} + +impl CredentialVersions { + /// Bring versions up to date with `inner.current`. + fn refresh(&mut self, inner: &ProviderCredentialStateInner) { + if self + .observed + .as_ref() + .is_some_and(|observed| Arc::ptr_eq(observed, &inner.current)) + { + return; + } + let previous_resolver = self.observed_resolver.take(); + let previous_snapshot = self.observed.take(); + let previous_statics = std::mem::take(&mut self.statics); + let previous_grants = std::mem::take(&mut self.grants); + let mut last_version = self.last_version; + let mut next_version = || { + last_version += 1; + last_version + }; + + let mut statics = HashMap::new(); + for (key, value) in inner + .current_resolver + .iter() + .flat_map(|resolver| resolver.canonical_values()) + { + let unchanged = previous_resolver + .as_deref() + .and_then(|previous| previous.canonical_value(key)) + .is_some_and(|previous| previous == value); + let version = match previous_statics.get(key) { + Some(version) if unchanged => *version, + _ => next_version(), + }; + statics.insert(key.to_owned(), version); + } + + let previous_configs = previous_snapshot + .as_deref() + .map(grants_by_identity) + .unwrap_or_default(); + let mut grants = HashMap::new(); + for (identity, configs) in grants_by_identity(&inner.current) { + let unchanged = previous_configs.get(identity) == Some(&configs); + let version = match previous_grants.get(identity) { + Some(version) if unchanged => *version, + _ => next_version(), + }; + grants.insert(identity.to_owned(), version); + } + + self.last_version = last_version; + self.statics = Arc::new(statics); + self.grants = grants; + self.observed = Some(Arc::clone(&inner.current)); + self.observed_resolver = inner.current_resolver.clone(); + } +} + +impl ProviderCredentialStateInner { + /// Credential versions for `self.current`, recomputed if it changed. + /// + /// Callers hold the state lock (read or write), which excludes installs + /// for the duration, so the versions describe exactly `self.current`. + fn refreshed_versions(&self) -> std::sync::MutexGuard<'_, CredentialVersions> { + let mut versions = self + .credential_versions + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + versions.refresh(self); + versions + } } #[derive(Debug)] @@ -83,6 +238,9 @@ struct CompiledStaticCredentialEndpointBinding { #[derive(Debug, Clone)] pub struct ProviderCredentialState { inner: Arc>, + /// Per-connection record of credential use; see + /// [`Self::with_use_tracking`]. Clones of one connection's handle share it. + use_marker: Option>, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -160,7 +318,9 @@ impl ProviderCredentialState { known_static_credential_keys: HashSet::new(), static_credential_identity_epochs: HashMap::new(), requires_stable_placeholders: false, + credential_versions: std::sync::Mutex::default(), })), + use_marker: None, } } @@ -247,7 +407,9 @@ impl ProviderCredentialState { known_static_credential_keys, static_credential_identity_epochs, requires_stable_placeholders: !stable_placeholder_environment_keys.is_empty(), + credential_versions: std::sync::Mutex::default(), })), + use_marker: None, }) } @@ -282,7 +444,9 @@ impl ProviderCredentialState { known_static_credential_keys: HashSet::new(), static_credential_identity_epochs: HashMap::new(), requires_stable_placeholders: false, + credential_versions: std::sync::Mutex::default(), })), + use_marker: None, } } @@ -331,6 +495,141 @@ impl ProviderCredentialState { .clone() } + /// A handle for one proxied connection that records when credential + /// material derived from it is used. + /// + /// Idempotent: a handle that already tracks use keeps its marker, so + /// adapters can call this at every entry point without splitting one + /// connection's record. The returned handle shares credential state with + /// `self`. + #[must_use] + pub fn with_use_tracking(&self) -> Self { + let mut tracked = self.clone(); + tracked + .use_marker + .get_or_insert_with(|| Arc::new(CredentialUseMarker::default())); + tracked + } + + /// Whether this handle records credential use (see [`Self::with_use_tracking`]). + #[must_use] + pub fn tracks_use(&self) -> bool { + self.use_marker.is_some() + } + + /// Record that this connection injected the token grant whose dynamic + /// credential key is `key` (raw or revision-scoped), at its current + /// version. A key without a `provider:credential` identity is recorded + /// as stale under the key itself. A no-op for untracked handles. + pub fn record_grant_use(&self, key: &str) { + let Some(marker) = &self.use_marker else { + return; + }; + let Some(identity) = crate::dynamic_credential_key::credential_identity(key) else { + marker.record_grant(key, STALE_CREDENTIAL_VERSION); + return; + }; + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + // The live version matches the one the request pinned unless + // credentials changed in between; then the credential generation guard + // refuses the request before its upstream write. + let version = inner + .refreshed_versions() + .grants + .get(identity) + .copied() + .unwrap_or(STALE_CREDENTIAL_VERSION); + marker.record_grant(identity, version); + } + + /// Which credentials this connection has injected, and at which + /// versions, or `None` for an untracked handle. + pub fn credential_uses(&self) -> Option { + self.use_marker.as_ref().map(|marker| marker.uses()) + } + + /// The current version of the static credential `env_key`, if configured. + pub fn static_credential_version(&self, env_key: &str) -> Option { + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + let versions = inner.refreshed_versions(); + versions.statics.get(env_key).copied() + } + + /// The current version of the token grant `identity`, if configured. + pub fn grant_credential_version(&self, identity: &str) -> Option { + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + let versions = inner.refreshed_versions(); + versions.grants.get(identity).copied() + } + + /// The version of every installed credential, to compare with + /// [`InstalledCredentialVersions::same_credentials`] across an operation + /// that may have changed them. + pub fn installed_credential_versions(&self) -> InstalledCredentialVersions { + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + let versions = inner.refreshed_versions(); + InstalledCredentialVersions { + statics: Arc::clone(&versions.statics), + grants: versions.grants.clone(), + } + } + + + /// Whether every credential this connection injected is still at the + /// version it injected. True for a connection that injected nothing; + /// false for an untracked handle, which cannot tell. + pub fn credential_uses_current(&self) -> bool { + let Some(marker) = &self.use_marker else { + return false; + }; + let uses = marker.uses(); + if uses.is_empty() { + return true; + } + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + let versions = inner.refreshed_versions(); + let at = |current: &HashMap, key: &String, version: &u64| { + current.get(key) == Some(version) + }; + uses.static_versions + .iter() + .all(|(key, version)| at(versions.statics.as_ref(), key, version)) + && uses + .grant_versions + .iter() + .all(|(key, version)| at(&versions.grants, key, version)) + } + + /// Link a resolver built from `inner.current` to this handle's use marker. + fn track_resolver( + &self, + resolver: SecretResolver, + inner: &ProviderCredentialStateInner, + ) -> SecretResolver { + match &self.use_marker { + Some(marker) => resolver.with_use_marker( + Arc::clone(marker), + Arc::clone(&inner.refreshed_versions().statics), + ), + None => resolver, + } + } + pub fn resolver(&self) -> Option> { let inner = self .inner @@ -338,12 +637,15 @@ impl ProviderCredentialState { .expect("provider credential state poisoned"); inner.combined_resolver.as_ref().map(|resolver| { Arc::new( - resolver.as_ref().clone().with_placements( - inner - .static_credential_bindings - .iter() - .map(|(key, binding)| (key.clone(), binding.placement.clone())) - .collect(), + self.track_resolver( + resolver.as_ref().clone().with_placements( + inner + .static_credential_bindings + .iter() + .map(|(key, binding)| (key.clone(), binding.placement.clone())) + .collect(), + ), + &inner, ), ) }) @@ -452,23 +754,23 @@ impl ProviderCredentialState { }) .map(|(key, epoch)| (key.clone(), epoch.revisions.clone())) .collect(); - Arc::new( - resolver - .as_ref() - .clone() - .with_placements( - inner - .static_credential_bindings - .iter() - .map(|(key, binding)| (key.clone(), binding.placement.clone())) - .collect(), - ) - .scoped_to_env_keys( - &inner.known_static_credential_keys, - &allowed, - revision_fallback_allowed_revisions, - ), - ) + let scoped = resolver + .as_ref() + .clone() + .with_placements( + inner + .static_credential_bindings + .iter() + .map(|(key, binding)| (key.clone(), binding.placement.clone())) + .collect(), + ) + .scoped_to_env_keys( + &inner.known_static_credential_keys, + &allowed, + revision_fallback_allowed_revisions, + ); + // Attributed to the same snapshot as `revision` above. + Arc::new(self.track_resolver(scoped, &inner)) }); (resolver, classifier, revision) } @@ -3924,4 +4226,504 @@ mod tests { None ); } + + // --- Credential use tracking --- + + fn token_state(revision: u64, tokens: &[(&str, &str)]) -> ProviderCredentialState { + ProviderCredentialState::from_environment( + revision, + tokens + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(), + HashMap::new(), + HashMap::new(), + ) + } + + fn reinstall_tokens(state: &ProviderCredentialState, revision: u64, tokens: &[(&str, &str)]) { + state.install_environment( + revision, + tokens + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(), + HashMap::new(), + HashMap::new(), + ); + } + + /// Resolve `env_key` through a resolver built from `state`, as a request + /// rewrite does, and return the secret it handed out. + fn inject_static(state: &ProviderCredentialState, env_key: &str) -> String { + let placeholder = state.snapshot().child_env[env_key].clone(); + state + .resolver_for_endpoint("api.github.com", 443, "/") + .expect("unbound credentials resolve on any endpoint") + .resolve_placeholder(&placeholder) + .expect("configured secret") + .to_string() + } + + #[test] + fn tracked_resolver_records_the_key_and_version_of_each_secret_sent() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "secret")]).with_use_tracking(); + assert!(tracked.credential_uses().unwrap().is_empty(), "nothing injected yet"); + + assert_eq!(inject_static(&tracked, "GITHUB_TOKEN"), "secret"); + + let uses = tracked.credential_uses().unwrap(); + assert_eq!( + uses.static_versions.get("GITHUB_TOKEN").copied(), + tracked.static_credential_version("GITHUB_TOKEN") + ); + assert!(uses.grant_versions.is_empty()); + } + + #[test] + fn current_env_key_lookups_record_use() { + // SigV4 and similar transforms select keys directly. + let tracked = token_state(10, &[("GITHUB_TOKEN", "secret")]).with_use_tracking(); + let resolver = tracked + .resolver_for_endpoint("api.github.com", 443, "/") + .unwrap(); + + assert_eq!( + resolver + .resolve_current_env_key_checked("GITHUB_TOKEN", "test") + .unwrap(), + Some("secret") + ); + assert!( + tracked + .credential_uses() + .unwrap() + .static_versions + .contains_key("GITHUB_TOKEN") + ); + } + + #[test] + fn uses_stay_current_until_a_credential_the_connection_used_changes() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b")]) + .with_use_tracking(); + inject_static(&tracked, "GITHUB_TOKEN"); + assert!(tracked.credential_uses_current()); + + // Another provider's credential rotates; this connection never sent it. + reinstall_tokens(&tracked, 11, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b2")]); + assert!(tracked.credential_uses_current(), "unused credentials don't matter"); + + // The credential it sent rotates. + reinstall_tokens(&tracked, 12, &[("GITHUB_TOKEN", "a2"), ("OTHER_TOKEN", "b2")]); + assert!(!tracked.credential_uses_current()); + } + + #[test] + fn removing_a_used_credential_is_a_change() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b")]) + .with_use_tracking(); + inject_static(&tracked, "GITHUB_TOKEN"); + + // Its provider is detached. + reinstall_tokens(&tracked, 11, &[("OTHER_TOKEN", "b")]); + + assert!(!tracked.credential_uses_current()); + } + + #[test] + fn a_connection_that_injected_nothing_is_always_current() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a")]).with_use_tracking(); + + reinstall_tokens(&tracked, 11, &[("GITHUB_TOKEN", "a2")]); + + assert!(tracked.credential_uses_current()); + } + + #[test] + fn an_unattributable_use_is_recorded_as_stale() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a")]).with_use_tracking(); + // A grant key with too many fields has no provider:credential identity. + let malformed = "a\tb\tc\td\te\tf\tg\th"; + + tracked.record_grant_use(malformed); + + assert_eq!( + tracked.credential_uses().unwrap().grant_versions.get(malformed), + Some(&STALE_CREDENTIAL_VERSION) + ); + assert!( + !tracked.credential_uses_current(), + "a use that cannot be attributed closes rather than staying open" + ); + } + + fn grant_credential(token_endpoint: &str) -> crate::proto::ProviderProfileCredential { + crate::proto::ProviderProfileCredential { + name: "access_token".to_string(), + auth_style: "bearer".to_string(), + header_name: "Authorization".to_string(), + token_grant: Some(crate::proto::ProviderCredentialTokenGrant { + token_endpoint: token_endpoint.to_string(), + ..Default::default() + }), + ..Default::default() + } + } + + fn grant_state(credentials: &[(&str, &str)]) -> ProviderCredentialState { + ProviderCredentialState::from_environment( + 10, + HashMap::new(), + HashMap::new(), + credentials + .iter() + .map(|(key, endpoint)| ((*key).to_string(), grant_credential(endpoint))) + .collect(), + ) + } + + const API_GRANT: &str = "api.example.com\t443\t/v1/**\tapi:access_token"; + const OTHER_GRANT: &str = "other.example.com\t443\t/**\tother:access_token"; + + #[test] + fn grant_uses_stay_current_until_that_grant_changes() { + let tracked = grant_state(&[ + (API_GRANT, "https://idp.example.com/token"), + (OTHER_GRANT, "https://idp.example.com/token"), + ]) + .with_use_tracking(); + + tracked.record_grant_use(API_GRANT); + let uses = tracked.credential_uses().unwrap(); + assert_eq!( + uses.grant_versions.keys().collect::>(), + ["api:access_token"] + ); + assert!(tracked.credential_uses_current()); + + // The other provider's grant changes. + tracked.install_environment( + 11, + HashMap::new(), + HashMap::new(), + HashMap::from([ + (API_GRANT.to_string(), grant_credential("https://idp.example.com/token")), + (OTHER_GRANT.to_string(), grant_credential("https://other-idp.example.com/token")), + ]), + ); + assert!(tracked.credential_uses_current(), "only the used grant matters"); + + // The used grant changes. + tracked.install_environment( + 12, + HashMap::new(), + HashMap::new(), + HashMap::from([( + API_GRANT.to_string(), + grant_credential("https://new-idp.example.com/token"), + )]), + ); + assert!(!tracked.credential_uses_current()); + } + + #[test] + fn untracked_handles_and_failed_lookups_record_nothing() { + let shared = token_state(10, &[("GITHUB_TOKEN", "secret")]); + let tracked = shared.with_use_tracking(); + assert!(tracked.tracks_use()); + assert!(!shared.tracks_use()); + + // Another handle on the same state reports to no connection. + inject_static(&shared, "GITHUB_TOKEN"); + assert_eq!(shared.credential_uses().map(|uses| uses.is_empty()), None); + assert!( + !shared.credential_uses_current(), + "an untracked handle cannot vouch for its connection" + ); + + // A lookup that hands out no secret is not a use. + let resolver = tracked + .resolver_for_endpoint("api.github.com", 443, "/") + .unwrap(); + assert!( + resolver + .resolve_placeholder("openshell:resolve:env:v10_NOT_CONFIGURED") + .is_none() + ); + + assert!(tracked.credential_uses().unwrap().is_empty()); + } + + #[test] + fn use_tracking_is_idempotent_and_shared_by_clones() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "secret")]).with_use_tracking(); + let retracked = tracked.with_use_tracking(); + let cloned = tracked.clone(); + + inject_static(&retracked, "GITHUB_TOKEN"); + + for handle in [&tracked, &cloned] { + assert!( + handle + .credential_uses() + .unwrap() + .static_versions + .contains_key("GITHUB_TOKEN") + ); + } + } + + // --- Central credential versions --- + + #[test] + fn a_credential_keeps_its_version_until_its_content_changes() { + let state = token_state(10, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let github = state.static_credential_version("GITHUB_TOKEN").unwrap(); + let other = state.static_credential_version("OTHER_TOKEN").unwrap(); + assert_ne!(github, other, "versions come from one counter"); + + reinstall_tokens(&state, 11, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b2")]); + assert_eq!(state.static_credential_version("GITHUB_TOKEN"), Some(github)); + let other_rotated = state.static_credential_version("OTHER_TOKEN").unwrap(); + assert!(other_rotated > other, "a changed value gets a newer version"); + + // Reinstalling identical content changes no version. + reinstall_tokens(&state, 12, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b2")]); + assert_eq!(state.static_credential_version("GITHUB_TOKEN"), Some(github)); + assert_eq!(state.static_credential_version("OTHER_TOKEN"), Some(other_rotated)); + } + + #[test] + fn a_removed_credential_added_back_gets_a_new_version() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a")]).with_use_tracking(); + let original = tracked.static_credential_version("GITHUB_TOKEN").unwrap(); + inject_static(&tracked, "GITHUB_TOKEN"); + + reinstall_tokens(&tracked, 11, &[]); + assert_eq!(tracked.static_credential_version("GITHUB_TOKEN"), None); + // The same value comes back; versions are never reused. + reinstall_tokens(&tracked, 12, &[("GITHUB_TOKEN", "a")]); + let restored = tracked.static_credential_version("GITHUB_TOKEN").unwrap(); + + assert!(restored > original); + assert!( + !tracked.credential_uses_current(), + "a connection cannot tell the gap apart from a rotation" + ); + } + + #[test] + fn a_value_from_an_older_generation_is_recorded_as_stale() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "old")]).with_use_tracking(); + let old_placeholder = tracked.snapshot().child_env["GITHUB_TOKEN"].clone(); + reinstall_tokens(&tracked, 11, &[("GITHUB_TOKEN", "new")]); + + // A long-running process still holds the revision-10 placeholder. + let resolver = tracked + .resolver_for_endpoint("api.github.com", 443, "/") + .unwrap(); + assert_eq!(resolver.resolve_placeholder(&old_placeholder), Some("old")); + + assert_eq!( + tracked + .credential_uses() + .unwrap() + .static_versions + .get("GITHUB_TOKEN"), + Some(&STALE_CREDENTIAL_VERSION) + ); + assert!(!tracked.credential_uses_current()); + } + + #[test] + fn an_older_revision_with_the_current_value_is_recorded_current() { + // Revisions are sandbox-wide: another credential changing moves every + // placeholder to the new revision, but this one still sends the + // current value. + let tracked = token_state(10, &[("GITHUB_TOKEN", "same"), ("OTHER_TOKEN", "a")]) + .with_use_tracking(); + let old_placeholder = tracked.snapshot().child_env["GITHUB_TOKEN"].clone(); + reinstall_tokens(&tracked, 11, &[("GITHUB_TOKEN", "same"), ("OTHER_TOKEN", "b")]); + assert_ne!(tracked.snapshot().child_env["GITHUB_TOKEN"], old_placeholder); + + let resolver = tracked + .resolver_for_endpoint("api.github.com", 443, "/") + .unwrap(); + assert_eq!(resolver.resolve_placeholder(&old_placeholder), Some("same")); + + assert_eq!( + tracked + .credential_uses() + .unwrap() + .static_versions + .get("GITHUB_TOKEN") + .copied(), + tracked.static_credential_version("GITHUB_TOKEN") + ); + assert!(tracked.credential_uses_current()); + } + + #[test] + fn connections_record_the_shared_version_of_a_credential() { + let shared = token_state(10, &[("GITHUB_TOKEN", "a")]); + let first = shared.with_use_tracking(); + let second = shared.with_use_tracking(); + + inject_static(&first, "GITHUB_TOKEN"); + inject_static(&second, "GITHUB_TOKEN"); + + let version = shared.static_credential_version("GITHUB_TOKEN"); + for connection in [&first, &second] { + assert_eq!( + connection + .credential_uses() + .unwrap() + .static_versions + .get("GITHUB_TOKEN") + .copied(), + version + ); + } + } + + // --- Versions across every install path --- + // + // Versions are recomputed whenever the snapshot changes, so each install + // path must replace the snapshot and its resolver together. The relay + // tests cover install_prepared, remove_env_key and the aged-out fallback. + + #[test] + fn child_env_only_installs_retire_resolvable_credentials() { + // These installs drop resolver material, so nothing a connection sent + // can still be current. + let installs: [fn(&ProviderCredentialState); 2] = [ + |state| { + state.install_child_env_snapshot(11, HashMap::new()); + }, + |state| { + state + .compare_and_install_child_env_snapshot(10, 11, HashMap::new()) + .unwrap(); + }, + ]; + for install in installs { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a")]).with_use_tracking(); + inject_static(&tracked, "GITHUB_TOKEN"); + + install(&tracked); + + assert_eq!(tracked.static_credential_version("GITHUB_TOKEN"), None); + assert!(!tracked.credential_uses_current()); + } + } + + #[test] + fn revoking_static_credentials_closes_connections_that_used_them() { + let tracked = token_state(10, &[("GITHUB_TOKEN", "a")]).with_use_tracking(); + inject_static(&tracked, "GITHUB_TOKEN"); + + tracked.revoke_static_provider_environment(11); + + assert!(!tracked.credential_uses_current()); + } + + #[test] + fn grant_versions_follow_their_configuration() { + let state = grant_state(&[(API_GRANT, "https://idp.example.com/token")]); + let version = state.grant_credential_version("api:access_token").unwrap(); + + state.install_environment( + 11, + HashMap::new(), + HashMap::new(), + HashMap::from([( + API_GRANT.to_string(), + grant_credential("https://idp.example.com/token"), + )]), + ); + assert_eq!(state.grant_credential_version("api:access_token"), Some(version)); + + state.install_environment( + 12, + HashMap::new(), + HashMap::new(), + HashMap::from([( + API_GRANT.to_string(), + grant_credential("https://new-idp.example.com/token"), + )]), + ); + assert_ne!(state.grant_credential_version("api:access_token"), Some(version)); + } + + // --- Installed credential versions --- + + #[test] + fn installed_versions_change_only_when_a_credential_does() { + let state = token_state(10, &[("GITHUB_TOKEN", "a")]); + let installed = state.installed_credential_versions(); + + reinstall_tokens(&state, 11, &[("GITHUB_TOKEN", "a")]); + assert!( + state + .installed_credential_versions() + .same_credentials(&installed), + "an identical reinstall changes nothing" + ); + + reinstall_tokens(&state, 12, &[("GITHUB_TOKEN", "b")]); + assert!( + !state + .installed_credential_versions() + .same_credentials(&installed) + ); + } + + #[test] + fn installed_versions_change_when_a_credential_is_added_or_removed() { + let state = token_state(10, &[("GITHUB_TOKEN", "a")]); + let installed = state.installed_credential_versions(); + + reinstall_tokens(&state, 11, &[("GITHUB_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let added = state.installed_credential_versions(); + assert!(!added.same_credentials(&installed)); + + reinstall_tokens(&state, 12, &[("GITHUB_TOKEN", "a")]); + assert!(!state.installed_credential_versions().same_credentials(&added)); + } + + #[test] + fn a_revocation_with_nothing_left_to_revoke_changes_nothing() { + let state = token_state(10, &[("GITHUB_TOKEN", "a")]); + let installed = state.installed_credential_versions(); + + state.revoke_static_provider_environment(11); + let revoked = state.installed_credential_versions(); + assert!(!revoked.same_credentials(&installed)); + + state.revoke_static_provider_environment(12); + assert!(state.installed_credential_versions().same_credentials(&revoked)); + } + + #[test] + fn installed_versions_follow_grant_configuration() { + let state = grant_state(&[(API_GRANT, "https://idp.example.com/token")]); + let installed = state.installed_credential_versions(); + + state.install_environment( + 11, + HashMap::new(), + HashMap::new(), + HashMap::from([( + API_GRANT.to_string(), + grant_credential("https://new-idp.example.com/token"), + )]), + ); + + assert!( + !state + .installed_credential_versions() + .same_credentials(&installed) + ); + } } diff --git a/crates/openshell-core/src/secrets.rs b/crates/openshell-core/src/secrets.rs index 6f4856be4c..b47b605c8f 100644 --- a/crates/openshell-core/src/secrets.rs +++ b/crates/openshell-core/src/secrets.rs @@ -218,6 +218,102 @@ pub struct SecretResolver { denied_env_keys: HashSet, identity_bound_env_keys: HashSet, revision_fallback_allowed_revisions: HashMap>>, + /// Where to report that this resolver handed out a real secret. + use_record: Option, +} + +/// Version that never matches a live credential version. +/// +/// Recorded for a value served from an older generation, which by definition +/// is no longer current, so the connection closes at its next check. +pub const STALE_CREDENTIAL_VERSION: u64 = 0; + +/// Records which provider credentials a connection injected, by version. +/// +/// The supervisor attaches one marker per proxied connection. Every resolver +/// derived for that connection reports each real secret it hands out, and +/// token-grant injection reports each grant before adding it. An empty marker +/// means the connection has never carried a provider credential upstream, so +/// no credential change can leave it holding stale material. +/// +/// The provider credential state owns one version per credential, assigned +/// by comparing content whenever its snapshot changes; a connection keeps only +/// the version it used: +/// +/// - static credentials by environment key; +/// - token grants by `provider:credential` identity. +/// +/// A use that cannot be attributed to a credential is recorded under its raw +/// placeholder or key at [`STALE_CREDENTIAL_VERSION`], so the connection +/// closes on the next credential check rather than never. +/// +/// Each key keeps its first recorded version. Recording is a superset by +/// design: some callers resolve to validate rather than to send, which can +/// only make a connection close more often. +#[derive(Debug, Default)] +pub struct CredentialUseMarker { + uses: std::sync::Mutex, +} + +/// The credentials one connection has injected; see [`CredentialUseMarker`]. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CredentialUses { + /// Static credential environment key to the version sent. + pub static_versions: std::collections::BTreeMap, + /// Token grant `provider:credential` identity to the version injected. + pub grant_versions: std::collections::BTreeMap, +} + +impl CredentialUses { + /// Whether nothing has been injected. + pub fn is_empty(&self) -> bool { + self.static_versions.is_empty() && self.grant_versions.is_empty() + } +} + +impl CredentialUseMarker { + /// Record that version `version` of the static credential `env_key` was sent. + pub fn record_static(&self, env_key: &str, version: u64) { + self.with_uses(|uses| { + uses.static_versions + .entry(env_key.to_owned()) + .or_insert(version); + }); + } + + /// Record that version `version` of the token grant `identity` was used. + pub fn record_grant(&self, identity: &str, version: u64) { + self.with_uses(|uses| { + uses.grant_versions + .entry(identity.to_owned()) + .or_insert(version); + }); + } + + /// A copy of everything recorded so far. + pub fn uses(&self) -> CredentialUses { + self.uses + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } + + fn with_uses(&self, update: impl FnOnce(&mut CredentialUses)) { + // A poisoned lock still holds a valid record; never lose a use. + let mut uses = self + .uses + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + update(&mut uses); + } +} + +/// A resolver's link to its connection's [`CredentialUseMarker`]. +#[derive(Clone)] +struct ResolverUseRecord { + marker: Arc, + /// Static credential versions of that installation. + static_versions: Arc>, } #[derive(Clone)] @@ -242,6 +338,76 @@ impl fmt::Debug for SecretResolver { } impl SecretResolver { + /// Report every real secret this resolver hands out to `marker`. + /// + /// `static_versions` is the version of each static credential in the + /// installation the resolver was built from. + #[must_use] + pub fn with_use_marker( + mut self, + marker: Arc, + static_versions: Arc>, + ) -> Self { + self.use_record = Some(ResolverUseRecord { + marker, + static_versions, + }); + self + } + + /// Each environment key's current-generation value. + /// + /// Only canonical entries are read; revision-scoped and stable-handle + /// placeholders alias them or carry older generations. + pub(crate) fn canonical_values(&self) -> impl Iterator { + self.by_placeholder + .iter() + .filter_map(|(placeholder, secret)| { + let key = placeholder.strip_prefix(PLACEHOLDER_PREFIX)?; + (placeholder_env_key(placeholder) == Some(key)) + .then_some((key, secret.value.as_ref())) + }) + } + + /// The current-generation value of `env_key`, if configured. + pub(crate) fn canonical_value(&self, env_key: &str) -> Option<&str> { + self.by_placeholder + .get(&placeholder_for_env_key(env_key)) + .map(|secret| secret.value.as_ref()) + } + + /// Report that this resolver handed out `served` for `placeholder`. + /// + /// The use is current when `served` is its key's current value, whichever + /// placeholder form named it: canonical, stable handle, the current + /// revision, a retained revision whose value has not changed since, or an + /// aged-out revision that fell back. Revisions are sandbox-wide, so an older + /// revision usually means another provider changed, not this credential. + /// A retained revision whose value has changed, or a key with no current + /// value, records [`STALE_CREDENTIAL_VERSION`]; so does a placeholder with + /// no recognisable key, under the placeholder itself. + fn note_use(&self, placeholder: &str, served: &str) { + let Some(record) = &self.use_record else { + return; + }; + let Some(env_key) = placeholder_env_key(placeholder) else { + record + .marker + .record_static(placeholder, STALE_CREDENTIAL_VERSION); + return; + }; + let version = if self.canonical_value(env_key) == Some(served) { + record + .static_versions + .get(env_key) + .copied() + .unwrap_or(STALE_CREDENTIAL_VERSION) + } else { + STALE_CREDENTIAL_VERSION + }; + record.marker.record_static(env_key, version); + } + #[cfg_attr(not(test), allow(dead_code))] pub fn from_provider_env( provider_env: HashMap, @@ -330,6 +496,7 @@ impl SecretResolver { denied_env_keys: HashSet::new(), identity_bound_env_keys: HashSet::new(), revision_fallback_allowed_revisions: HashMap::new(), + use_record: None, }) }; ( @@ -388,6 +555,7 @@ impl SecretResolver { denied_env_keys: HashSet::new(), identity_bound_env_keys: HashSet::new(), revision_fallback_allowed_revisions: HashMap::new(), + use_record: None, }), ) } @@ -418,6 +586,7 @@ impl SecretResolver { denied_env_keys, identity_bound_env_keys, revision_fallback_allowed_revisions, + use_record: None, }) } } @@ -454,6 +623,7 @@ impl SecretResolver { denied_env_keys, identity_bound_env_keys: bound_keys.clone(), revision_fallback_allowed_revisions, + use_record: self.use_record.clone(), } } @@ -605,7 +775,11 @@ impl SecretResolver { let canonical = placeholder_for_env_key(key); self.by_placeholder.get(&canonical)? }; - resolve_secret_value(secret) + let resolved = resolve_secret_value(secret); + if let Some(served) = resolved { + self.note_use(value, served); + } + resolved } /// Resolve the current value for an environment key selected by trusted @@ -628,10 +802,14 @@ impl SecretResolver { }); } let placeholder = placeholder_for_env_key(key); - Ok(self + let resolved = self .by_placeholder .get(&placeholder) - .and_then(resolve_secret_value)) + .and_then(resolve_secret_value); + if let Some(served) = resolved { + self.note_use(&placeholder, served); + } + Ok(resolved) } /// Resolve a placeholder while preserving endpoint-denial information. diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index c24777826e..13ceb8cf7f 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -214,6 +214,22 @@ pub(crate) struct EndpointObserver { policy_generation: Option, } +/// The policy hash endpoint-status results for `config` belong to. +/// +/// The policy a tunnel's guard is pinned to, which a revalidating guard +/// records from the moment it authorizes the tunnel through every re-pin. A +/// plain guard never moves from the policy the endpoint config was selected +/// from, so that config's stamped hash is the same answer. The inventory check +/// still rejects a result whenever the inventory describes a different policy. +pub(crate) fn observation_policy_hash( + config: &L7EndpointConfig, + guard: Option<&PolicyGenerationGuard>, +) -> String { + guard + .and_then(PolicyGenerationGuard::pinned_policy_hash) + .unwrap_or_else(|| config.policy_hash.clone()) +} + impl EndpointObserver { /// Bind a selected endpoint to authority captured before request-state lookup. /// @@ -239,11 +255,12 @@ impl EndpointObserver { // prevents an earlier selection from becoming current again. return None; } + let policy_hash = observation_policy_hash(config, guard); let sender = sender?.clone(); let observation = sender.begin_captured( context?, config.endpoint_id.clone(), - &config.policy_hash, + &policy_hash, provider_revision, )?; Some(Self { diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 61c342bc17..0bc2166c7e 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -219,7 +219,9 @@ pub(crate) async fn prepare_inspected_request( &owners, request_id, observation_context.filter(|context| { - context.matches_config(&config.policy_hash, Some(snapshot.revision)) + let policy_hash = + crate::l7::observation_policy_hash(config, Some(engine.generation_guard())); + context.matches_config(&policy_hash, Some(snapshot.revision)) }), ) .await? @@ -3629,6 +3631,11 @@ where let mut response_selection = None; let req = if let Some(engine) = middleware_engine { let input = middleware_network_input(ctx); + // Give a revalidating guard the chance to re-pin through an + // unrelated reload before comparing raw generation numbers. + if generation_guard.is_stale() { + return Ok(()); + } let (chain, generation) = engine.query_middleware_chain_with_generation(&input)?; if generation != generation_guard.captured_generation() { return Ok(()); @@ -13582,4 +13589,206 @@ network_policies: .unwrap() .unwrap(); } + + // --- Passthrough middleware across policy reloads --- + + const PASSTHROUGH_RELOAD_POLICY: &str = r#" +network_middlewares: + scheme: + middleware: test/scheme + on_error: fail_closed + endpoints: + include: ["api.example.test"] +network_policies: + passthrough_api: + name: passthrough_api + endpoints: + - host: api.example.test + port: 443 + binaries: + - { path: /usr/bin/curl } +"#; + + /// [`PASSTHROUGH_RELOAD_POLICY`] after approving an endpoint for another host. + const PASSTHROUGH_RELOAD_AND_OTHER_POLICY: &str = r#" +network_middlewares: + scheme: + middleware: test/scheme + on_error: fail_closed + endpoints: + include: ["api.example.test"] +network_policies: + passthrough_api: + name: passthrough_api + endpoints: + - host: api.example.test + port: 443 + binaries: + - { path: /usr/bin/curl } + other: + name: other + endpoints: + - host: other.example.test + port: 443 + binaries: + - { path: /usr/bin/curl } +"#; + + /// [`PASSTHROUGH_RELOAD_POLICY`] with this host's chain made fail-open. + const PASSTHROUGH_RELOAD_CHAIN_CHANGED_POLICY: &str = r#" +network_middlewares: + scheme: + middleware: test/scheme + on_error: fail_open + endpoints: + include: ["api.example.test"] +network_policies: + passthrough_api: + name: passthrough_api + endpoints: + - host: api.example.test + port: 443 + binaries: + - { path: /usr/bin/curl } +"#; + + fn passthrough_reload_input() -> NetworkInput { + NetworkInput { + host: "api.example.test".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: String::new(), + ancestors: vec![], + cmdline_paths: vec![], + } + } + + /// The guard the proxy pins for an established passthrough connection: + /// it re-pins through reloads that leave the connection's contract intact. + fn established_passthrough_guard( + engine: &Arc, + input: &NetworkInput, + ) -> PolicyGenerationGuard { + let generation = engine.authorize_egress(input).unwrap().generation; + assert!( + crate::opa::PolicyRevalidator::capture(engine, input.clone(), generation, None) + .is_some(), + "fixture must authorize the connection" + ); + engine + .revalidating_generation_guard(input.clone(), generation, None) + .unwrap() + } + + async fn read_http_head(reader: &mut R) -> Vec { + let mut head = Vec::new(); + let mut byte = [0u8; 1]; + while !head.ends_with(b"\r\n\r\n") { + let n = reader.read(&mut byte).await.unwrap(); + assert!(n > 0, "peer closed before a complete HTTP head"); + head.push(byte[0]); + } + head + } + + /// Send one request on a kept-alive passthrough connection with + /// middleware, reload the policy to `reload_data`, then send a second. + /// + /// Returns how many requests reached middleware and whether the second + /// request reached upstream. + async fn second_passthrough_request_after_reload(reload_data: &str) -> (usize, bool) { + use openshell_supervisor_middleware::{ChainRunner, MiddlewareRegistry}; + + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + let engine = + Arc::new(OpaEngine::from_strings(TEST_POLICY, PASSTHROUGH_RELOAD_POLICY).unwrap()); + let registry = MiddlewareRegistry::connect_services( + vec![Arc::new(SchemeRecorder(seen.clone()))], + Vec::new(), + ) + .await + .expect("connect named middleware services"); + engine.set_middleware_runner_for_tests(ChainRunner::from_registry(registry)); + let generation_guard = established_passthrough_guard(&engine, &passthrough_reload_input()); + let ctx = L7EvalContext { + host: "api.example.test".into(), + port: 443, + request_default_port: Some(443), + policy_name: "passthrough_api".into(), + binary_path: "/usr/bin/curl".into(), + ..Default::default() + }; + + let (mut app, mut relay_client) = tokio::io::duplex(8192); + let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192); + let relay_engine = Arc::clone(&engine); + let relay = tokio::spawn(async move { + relay_passthrough_with_credentials( + &mut relay_client, + &mut relay_upstream, + &ctx, + &generation_guard, + Some(relay_engine.as_ref()), + ) + .await + }); + let timeout = std::time::Duration::from_secs(1); + let request = b"GET /v1/messages HTTP/1.1\r\nHost: api.example.test\r\n\r\n"; + + app.write_all(request).await.unwrap(); + tokio::time::timeout(timeout, read_http_head(&mut upstream)) + .await + .expect("first request should reach upstream"); + upstream + .write_all(b"HTTP/1.1 204 No Content\r\n\r\n") + .await + .unwrap(); + let response = tokio::time::timeout(timeout, read_http_head(&mut app)) + .await + .expect("first response should reach the client"); + assert!(String::from_utf8_lossy(&response).contains("204 No Content")); + + engine.reload(TEST_POLICY, reload_data).unwrap(); + + app.write_all(request).await.unwrap(); + let mut second = [0u8; 1024]; + let forwarded = matches!( + tokio::time::timeout(std::time::Duration::from_millis(500), upstream.read(&mut second)) + .await, + Ok(Ok(n)) if n > 0 + ); + let middleware_calls = seen.lock().unwrap().len(); + + drop(app); + drop(upstream); + let _ = tokio::time::timeout(timeout, relay).await; + (middleware_calls, forwarded) + } + + #[tokio::test] + async fn passthrough_middleware_connection_serves_next_request_after_unrelated_reload() { + let (middleware_calls, forwarded) = + second_passthrough_request_after_reload(PASSTHROUGH_RELOAD_AND_OTHER_POLICY).await; + + assert_eq!(middleware_calls, 2, "both requests ran through middleware"); + assert!( + forwarded, + "approving another host must not end the connection" + ); + } + + #[tokio::test] + async fn passthrough_middleware_connection_drops_next_request_after_its_chain_changes() { + let (middleware_calls, forwarded) = + second_passthrough_request_after_reload(PASSTHROUGH_RELOAD_CHAIN_CHANGED_POLICY).await; + + assert_eq!( + middleware_calls, 1, + "only the first request reached middleware" + ); + assert!( + !forwarded, + "a changed chain must not apply to the old connection" + ); + } } 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 f868575097..c5ac90abb3 100644 --- a/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs +++ b/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs @@ -165,6 +165,17 @@ pub fn pin_live_credentials(ctx: &mut L7EvalContext) -> Option L7EvalContext { + L7EvalContext { + host: "api.example.com".into(), + port: 443, + policy_name: "api".into(), + binary_path: "/usr/bin/curl".into(), + provider_credentials: Some(fixture.provider_credentials().with_use_tracking()), + token_grant_resolver: Some(fixture.resolver()), + ..Default::default() + } + } + + fn grant_request_for(target: &str) -> L7Request { + L7Request { + action: "GET".to_string(), + target: target.to_string(), + query_params: HashMap::new(), + raw_header: format!("GET {target} HTTP/1.1\r\nHost: api.example.com\r\n\r\n") + .into_bytes(), + body_length: BodyLength::None, + } + } + + #[tokio::test] + async fn injecting_a_grant_records_credential_use_on_the_connection() { + let fixture = TokenGrantTestFixture::success( + "api.example.com\t443\t/v1/**\tprovider:access_token", + "grant-token", + ); + let ctx = tracked_grant_context(&fixture); + let credentials = ctx.provider_credentials.clone().unwrap(); + assert!(credentials.credential_uses().unwrap().is_empty()); + + inject_if_needed(grant_request_for("/v1/projects"), &ctx, REQUEST_ID) + .await + .expect("fake token grant should inject"); + + let uses = credentials.credential_uses().unwrap(); + assert_eq!( + uses.grant_versions.keys().collect::>(), + ["provider:access_token"], + "the connection must record which grant it carried" + ); + assert!(credentials.credential_uses_current()); + } + + #[tokio::test] + async fn requests_that_select_no_grant_record_no_credential_use() { + let fixture = TokenGrantTestFixture::success( + "api.example.com\t443\t/v1/**\tprovider:access_token", + "grant-token", + ); + let ctx = tracked_grant_context(&fixture); + + inject_if_needed(grant_request_for("/public/status"), &ctx, REQUEST_ID) + .await + .expect("no grant applies to this path"); + + assert!( + ctx.provider_credentials + .unwrap() + .credential_uses() + .unwrap() + .is_empty() + ); + } + #[tokio::test] async fn inject_if_needed_passes_token_exchange_grant_to_resolver() { let fixture = TokenGrantTestFixture::success_token_exchange( diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 257bd3ce2e..284073aa83 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -24,6 +24,7 @@ use openshell_policy_schema::{ ProcessPolicy as AuthoredProcessPolicy, }; use openshell_supervisor_middleware::{ChainEntry, ChainRunner, MiddlewareRegistry}; +use std::collections::VecDeque; use std::path::{Path, PathBuf}; use std::sync::{ Arc, Mutex, RwLock, @@ -33,6 +34,9 @@ use tokio::sync::watch; use tracing::info; mod raw_schema; +mod revalidation; + +pub use revalidation::{ConnectionPolicyContract, PolicyRevalidator}; /// Baked-in rego rules for OPA policy evaluation. /// These rules define the network access decision logic and static config @@ -115,6 +119,7 @@ pub struct EgressAuthorization { } /// Input for a network access policy evaluation. +#[derive(Debug, Clone)] pub struct NetworkInput { pub host: String, pub port: u16, @@ -183,6 +188,28 @@ pub struct OpaEngine { websocket_assembly_budget: crate::l7::websocket::WebSocketAssemblyBudget, generation_tx: watch::Sender, fail_closed_reason: RwLock>, + /// Incremented whenever the middleware runner's registry is replaced. + /// Written under the engine lock. + middleware_registry_epoch: AtomicU64, + /// The most recent published generations, oldest first; see + /// [`PublishedGeneration`]. Locked only while the engine lock is held. + history: Mutex>, +} + +/// How many recent generations revalidation can check a connection against. +/// +/// A connection that sleeps through more reloads than this closes, because it +/// can no longer be checked against every policy that was live meanwhile. +const GENERATION_HISTORY: usize = 16; + +/// The policy state one generation published, kept so that a connection +/// waking after several reloads is checked against each of them, not only the +/// latest. Engine clones share compiled policy and are cheap. +struct PublishedGeneration { + generation: u64, + engine: regorus::Engine, + fail_closed_reason: Option, + middleware_registry_epoch: u64, } #[cfg(test)] @@ -204,24 +231,82 @@ pub(crate) fn test_opa_query_count() -> u64 { } /// Generation guard captured when an HTTP tunnel or request path starts. +/// +/// A plain guard is stale as soon as the policy generation moves, and checking +/// it is a cheap comparison. A guard with a [`PolicyRevalidator`] instead +/// re-checks the connection it protects and re-pins to the new generation +/// when the policy change does not affect that connection; checking it may +/// evaluate policy under the engine lock. Only this crate's relays create +/// revalidating guards (`OpaEngine::revalidating_generation_guard`), so +/// every guard another crate obtains from the engine is plain. +/// +/// Clones share one pin, so every clone held by a relay observes a re-pin +/// performed through any of them. #[derive(Clone, Debug)] pub struct PolicyGenerationGuard { - captured_generation: u64, + captured_generation: Arc, current_generation: Arc, generation_rx: watch::Receiver, + revalidator: Option>, } impl PolicyGenerationGuard { + fn pinned( + generation: u64, + current_generation: &Arc, + generation_tx: &watch::Sender, + ) -> Self { + Self { + captured_generation: Arc::new(AtomicU64::new(generation)), + current_generation: Arc::clone(current_generation), + generation_rx: generation_tx.subscribe(), + revalidator: None, + } + } + + /// Let this guard follow policy changes that leave its connection's + /// contract unchanged. `None` keeps the strict close-on-change behavior. + #[must_use] + pub(crate) fn with_revalidator(mut self, revalidator: Option>) -> Self { + self.revalidator = revalidator; + self + } + + /// The stamped policy hash of the generation a revalidating guard is pinned + /// to: the one that authorized its connection, or a later one it re-pinned + /// to. Endpoint-status observations of the connection belong to that + /// policy. `None` for a plain guard, which never moves from the generation + /// its endpoint config was selected from, and for policies without a hash. + pub fn pinned_policy_hash(&self) -> Option { + self.revalidator + .as_ref()? + .pinned_policy_hash(self.captured_generation()) + } + pub fn captured_generation(&self) -> u64 { - self.captured_generation + self.captured_generation.load(Ordering::Acquire) } pub fn current_generation(&self) -> u64 { self.current_generation.load(Ordering::Acquire) } + /// Whether the policy that authorized this connection no longer applies. + /// + /// With a revalidator, a generation change triggers one re-evaluation of + /// the connection (a short, synchronous OPA query under the engine lock). + /// Callers must not hold the OPA engine lock. pub fn is_stale(&self) -> bool { - self.current_generation() != self.captured_generation + let Some(revalidator) = &self.revalidator else { + return self.current_generation() != self.captured_generation(); + }; + if revalidator.is_invalidated() { + return true; + } + if self.current_generation() == self.captured_generation() { + return false; + } + !revalidator.follow(&self.captured_generation, &self.current_generation) } pub fn ensure_current(&self) -> Result<()> { @@ -235,10 +320,12 @@ impl PolicyGenerationGuard { Ok(()) } - /// Wait until the policy generation changes. + /// Wait until the policy that authorized this connection no longer applies. /// /// Relay boundaries use this to close even an idle or raw stream as soon - /// as a new generation (including fail-closed quarantine) is published. + /// as a relevant generation (always including fail-closed quarantine) is + /// published. A revalidating guard re-pins through unrelated changes and + /// keeps waiting. pub async fn wait_until_stale(&self) { let mut receiver = self.generation_rx.clone(); while !self.is_stale() { @@ -259,6 +346,18 @@ pub struct TunnelPolicyEngine { } impl TunnelPolicyEngine { + /// Let this tunnel survive policy changes that leave its connection's + /// contract unchanged. + /// + /// The cloned evaluator is kept as-is after a re-pin. That is sound + /// because the contract includes every endpoint per-request rules read; + /// see [`ConnectionPolicyContract`]. + #[must_use] + pub(crate) fn with_revalidator(mut self, revalidator: Option>) -> Self { + self.generation_guard = self.generation_guard.with_revalidator(revalidator); + self + } + pub fn captured_generation(&self) -> u64 { self.generation_guard.captured_generation() } @@ -309,6 +408,12 @@ impl OpaEngine { fn with_engine(engine: regorus::Engine, binary_identity_required: bool) -> Self { let generation = Arc::new(AtomicU64::new(0)); let (generation_tx, _) = watch::channel(0); + let initial = PublishedGeneration { + generation: 0, + engine: engine.clone(), + fail_closed_reason: None, + middleware_registry_epoch: 0, + }; Self { engine: Mutex::new(engine), binary_identity_required, @@ -317,6 +422,33 @@ impl OpaEngine { websocket_assembly_budget: crate::l7::websocket::WebSocketAssemblyBudget::default(), generation_tx, fail_closed_reason: RwLock::new(None), + middleware_registry_epoch: AtomicU64::new(0), + history: Mutex::new(VecDeque::from([initial])), + } + } + + /// Record the policy state `generation` published. Callers hold the + /// engine lock, so revalidation never sees a generation without its entry. + /// A generation that is somehow not recorded fails closed: connections + /// that need it cannot be checked and close. + fn record_generation( + &self, + generation: u64, + engine: ®orus::Engine, + fail_closed_reason: Option, + ) { + let mut history = self + .history + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + history.push_back(PublishedGeneration { + generation, + engine: engine.clone(), + fail_closed_reason, + middleware_registry_epoch: self.middleware_registry_epoch.load(Ordering::Acquire), + }); + while history.len() > GENERATION_HISTORY { + history.pop_front(); } } @@ -607,19 +739,138 @@ impl OpaEngine { #[cfg(test)] record_test_opa_query(); - let input_json = network_input_json(input); - let mut engine = self .engine .lock() .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; - let generation = self.current_generation(); + self.authorize_egress_locked(&mut engine, input) + } + + /// The contract of a connection with `input` under the current generation. + pub fn connection_policy_contract( + &self, + input: &NetworkInput, + ) -> Result<(ConnectionPolicyContract, u64)> { + let _publication = self + .engine + .lock() + .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + let current = self.current_generation(); + let contract = self + .contract_at_locked(input, current)? + .ok_or_else(|| miette::miette!("current policy generation is not recorded"))?; + Ok((contract, current)) + } + + /// The contract of a connection with `input` under `generation`, or `None` + /// once that generation has left the history. + pub(crate) fn connection_contract_at( + &self, + input: &NetworkInput, + generation: u64, + ) -> Result> { + let _publication = self + .engine + .lock() + .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + self.contract_at_locked(input, generation) + } + + /// The contract under every generation published after `captured`, up to + /// and including the current one, and that current generation. + /// + /// `None` when any of those generations has left the history: the + /// connection then cannot be shown to have been allowed throughout. + /// Holding the engine lock excludes publication while this runs. + pub(crate) fn connection_contracts_since( + &self, + input: &NetworkInput, + captured: u64, + ) -> Result, u64)>> { + let _publication = self + .engine + .lock() + .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + let current = self.current_generation(); + let mut history = self + .history + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut contracts = Vec::new(); + for generation in captured.saturating_add(1)..=current { + let Some(published) = history + .iter_mut() + .find(|published| published.generation == generation) + else { + return Ok(None); + }; + contracts.push(Self::contract_of(published, input)?); + } + Ok(Some((contracts, current))) + } + fn contract_at_locked( + &self, + input: &NetworkInput, + generation: u64, + ) -> Result> { + let mut history = self + .history + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + history + .iter_mut() + .find(|published| published.generation == generation) + .map(|published| Self::contract_of(published, input)) + .transpose() + } + + /// Authorization and middleware selection for `input` under one published + /// generation, as a [`ConnectionPolicyContract`]. + fn contract_of( + published: &mut PublishedGeneration, + input: &NetworkInput, + ) -> Result { + let authorization = Self::authorize_on( + &mut published.engine, + input, + published.fail_closed_reason.clone(), + published.generation, + )?; + let middleware_chain = if matches!(authorization.action, NetworkAction::Allow { .. }) { + query_middleware_chain_locked(&mut published.engine, input)? + } else { + Vec::new() + }; + Ok(ConnectionPolicyContract::new( + &authorization, + middleware_chain, + published.middleware_registry_epoch, + )) + } + + fn authorize_egress_locked( + &self, + engine: &mut regorus::Engine, + input: &NetworkInput, + ) -> Result { let fail_closed_reason = self .fail_closed_reason .read() .map_err(|_| miette::miette!("OPA fail-closed state lock poisoned"))? .clone(); + Self::authorize_on(engine, input, fail_closed_reason, self.current_generation()) + } + + /// Evaluate egress authorization on `engine`, as published for + /// `generation` with `fail_closed_reason`. + fn authorize_on( + engine: &mut regorus::Engine, + input: &NetworkInput, + fail_closed_reason: Option, + generation: u64, + ) -> Result { + let input_json = network_input_json(input); if let Some(reason) = fail_closed_reason { return Ok(EgressAuthorization { action: NetworkAction::Deny { reason }, @@ -630,7 +881,7 @@ impl OpaEngine { }); } - set_regorus_input(&mut engine, input_json)?; + set_regorus_input(engine, input_json)?; let result = engine .eval_rule("data.openshell.sandbox.egress_authorization".into()) @@ -734,7 +985,8 @@ impl OpaEngine { .fail_closed_reason .write() .map_err(|_| miette::miette!("OPA fail-closed state lock poisoned"))? = None; - self.advance_generation(); + let generation = self.advance_generation(); + self.record_generation(generation, &engine, None); Ok(()) } @@ -810,8 +1062,11 @@ impl OpaEngine { *engine = new_engine; if let Some(new_runner) = new_runner { *runner = new_runner; + self.middleware_registry_epoch + .fetch_add(1, Ordering::AcqRel); } *fail_closed_reason = None; + self.record_generation(generation, &engine, None); self.generation_guard(generation) } @@ -821,18 +1076,23 @@ impl OpaEngine { /// The existing compiled engine remains available for an explicit /// `retain_last_valid` posture or a later valid reload, but all new network /// decisions deny with `reason` while the quarantine is active. Advancing - /// the generation invalidates and wakes every pinned relay. + /// the generation invalidates and wakes every pinned relay, including + /// revalidating relays: a quarantine is never re-pinned through, even if + /// the previous policy is restored before a relay observes it. pub fn enter_fail_closed(&self, reason: impl Into) -> Result { - let _engine = self + let engine = self .engine .lock() .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + let reason = reason.into(); *self .fail_closed_reason .write() .map_err(|_| miette::miette!("OPA fail-closed state lock poisoned"))? = - Some(reason.into()); - Ok(self.advance_generation()) + Some(reason.clone()); + let generation = self.advance_generation(); + self.record_generation(generation, &engine, Some(reason)); + Ok(generation) } pub fn fail_closed_reason(&self) -> Option { @@ -845,7 +1105,7 @@ impl OpaEngine { /// Reactivate the compiled last-known-good engine after an operator /// explicitly selects the availability-oriented retention posture. pub fn exit_fail_closed(&self) -> Result { - let _engine = self + let engine = self .engine .lock() .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; @@ -856,7 +1116,9 @@ impl OpaEngine { .take() .is_some(); if was_fail_closed { - Ok(self.advance_generation()) + let generation = self.advance_generation(); + self.record_generation(generation, &engine, None); + Ok(generation) } else { Ok(self.current_generation()) } @@ -895,7 +1157,7 @@ impl OpaEngine { pub fn replace_middleware_registry(&self, registry: MiddlewareRegistry) -> Result<()> { // Generation changes serialize through the engine lock so guarded // publication cannot overlap any runtime generation transition. - let _engine = self + let engine = self .engine .lock() .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; @@ -904,10 +1166,30 @@ impl OpaEngine { .write() .map_err(|_| miette::miette!("middleware runner lock poisoned"))?; *runner = runner.with_replacement_registry(registry); - self.advance_generation(); + self.middleware_registry_epoch + .fetch_add(1, Ordering::AcqRel); + let generation = self.advance_generation(); + self.record_generation(generation, &engine, self.fail_closed_reason()); Ok(()) } + /// Publish a new generation of the unchanged policy, so pinned relays wake + /// and re-check what they hold. For provider credential changes made + /// outside a policy reload, such as a revocation: a revalidating relay + /// re-pins unless it injected a credential that changed, and any other + /// guard goes stale as on a reload. A quarantine stays in force. + pub fn publish_credential_change(&self) -> Result { + // Generation changes serialize through the engine lock, as in + // `replace_middleware_registry`. + let engine = self + .engine + .lock() + .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + let generation = self.advance_generation(); + self.record_generation(generation, &engine, self.fail_closed_reason()); + Ok(generation) + } + pub(crate) fn middleware_runner(&self) -> Result { self.middleware_runner .read() @@ -934,11 +1216,11 @@ impl OpaEngine { "policy changed before HTTP relay started [expected_generation:{expected_generation} current_generation:{generation}]" )); } - Ok(PolicyGenerationGuard { - captured_generation: generation, - current_generation: Arc::clone(&self.generation), - generation_rx: self.generation_tx.subscribe(), - }) + Ok(PolicyGenerationGuard::pinned( + generation, + &self.generation, + &self.generation_tx, + )) } /// Query static sandbox configuration from the OPA data module. @@ -1130,11 +1412,11 @@ impl OpaEngine { } Ok(TunnelPolicyEngine { engine: Mutex::new(engine.clone()), - generation_guard: PolicyGenerationGuard { - captured_generation: generation, - current_generation: Arc::clone(&self.generation), - generation_rx: self.generation_tx.subscribe(), - }, + generation_guard: PolicyGenerationGuard::pinned( + generation, + &self.generation, + &self.generation_tx, + ), middleware_runner: self.middleware_runner()?, websocket_assembly_budget: self.websocket_assembly_budget(), }) @@ -12591,4 +12873,709 @@ network_policies: cmdline_paths: vec![], } } + + // --- Generation guards across policy reloads --- + + fn proto_endpoint(host: &str) -> NetworkEndpoint { + NetworkEndpoint { + host: host.to_string(), + port: 443, + ..Default::default() + } + } + + fn policy_mut<'a>(proto: &'a mut ProtoSandboxPolicy, name: &str) -> &'a mut NetworkPolicyRule { + proto + .network_policies + .get_mut(name) + .expect("fixture policy") + } + + /// `test_proto` after approving an endpoint that no claude connection uses. + fn proto_with_unrelated_endpoint() -> ProtoSandboxPolicy { + let mut proto = test_proto(); + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + proto + } + + /// A guard pinned with `generation_guard` stays strict and goes + /// stale on any reload; only relay guards re-check their connection. + #[test] + fn generation_guard_goes_stale_on_unrelated_reload() { + let engine = OpaEngine::from_proto(&test_proto()).unwrap(); + let guard = engine + .generation_guard(engine.current_generation()) + .unwrap(); + + engine + .reload_from_proto(&proto_with_unrelated_endpoint()) + .unwrap(); + + assert!( + guard.is_stale(), + "a guard pinned with generation_guard closes on any reload" + ); + } + + /// A tunnel evaluator from `clone_engine_for_tunnel` stays strict. + #[test] + fn tunnel_engine_goes_stale_on_unrelated_reload() { + let engine = OpaEngine::from_proto(&test_proto()).unwrap(); + let tunnel = engine + .clone_engine_for_tunnel(engine.current_generation()) + .unwrap(); + + engine + .reload_from_proto(&proto_with_unrelated_endpoint()) + .unwrap(); + + assert!(tunnel.is_stale()); + } + + /// Proto conversion stamps the hash of the complete policy into + /// every endpoint, so endpoints differ after any reload. Revalidation has to + /// ignore that stamp. + #[test] + fn every_reload_restamps_policy_hash_on_unchanged_endpoints() { + let mut proto = test_proto(); + policy_mut(&mut proto, "claude_code").endpoints[0] = NetworkEndpoint { + host: "api.anthropic.com".to_string(), + port: 443, + protocol: "rest".to_string(), + enforcement: openshell_core::proto::NetworkEnforcementMode::Enforce as i32, + rules: vec![L7Rule { + allow: Some(L7Allow { + method: "GET".to_string(), + path: "/v1/messages".to_string(), + ..Default::default() + }), + }], + ..Default::default() + }; + let before = OpaEngine::from_proto(&proto).unwrap(); + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + let after = OpaEngine::from_proto(&proto).unwrap(); + let input = NetworkInput { + host: "api.anthropic.com".into(), + port: 443, + binary_path: PathBuf::from("/usr/local/bin/claude"), + binary_sha256: String::new(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + let before = before.authorize_egress(&input).unwrap(); + let after = after.authorize_egress(&input).unwrap(); + + assert_eq!(before.action, after.action); + assert_ne!(before.endpoint_configs, after.endpoint_configs); + let hash = |configs: &[regorus::Value]| { + configs + .iter() + .map(|config| config["policy_hash"].clone()) + .collect::>() + }; + assert_ne!( + hash(&before.endpoint_configs), + hash(&after.endpoint_configs) + ); + } + + // --- Connection revalidation across policy generations --- + + fn claude_input(host: &str) -> NetworkInput { + NetworkInput { + host: host.into(), + port: 443, + binary_path: PathBuf::from("/usr/local/bin/claude"), + binary_sha256: String::new(), + ancestors: vec![], + cmdline_paths: vec![], + } + } + + /// Pin a revalidating guard the way the proxy does after authorization. + fn revalidating_guard(engine: &Arc, input: &NetworkInput) -> PolicyGenerationGuard { + let authorization = engine.authorize_egress(input).unwrap(); + assert!( + matches!(authorization.action, NetworkAction::Allow { .. }), + "fixture must authorize {}", + input.host + ); + let generation = authorization.generation; + assert!( + PolicyRevalidator::capture(engine, input.clone(), generation, None).is_some(), + "an allow must capture a contract" + ); + engine + .revalidating_generation_guard(input.clone(), generation, None) + .unwrap() + } + + fn rest_endpoint(host: &str, path: &str) -> NetworkEndpoint { + NetworkEndpoint { + host: host.to_string(), + port: 443, + protocol: "rest".to_string(), + enforcement: openshell_core::proto::NetworkEnforcementMode::Enforce as i32, + rules: vec![L7Rule { + allow: Some(L7Allow { + method: "GET".to_string(), + path: path.to_string(), + ..Default::default() + }), + }], + ..Default::default() + } + } + + #[test] + fn revalidating_guard_follows_reload_that_adds_an_unrelated_endpoint() { + let mut proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + let strict = engine + .generation_guard(guard.captured_generation()) + .unwrap(); + + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + engine.reload_from_proto(&proto).unwrap(); + + assert!( + strict.is_stale(), + "a plain guard still closes on any change" + ); + assert!( + !guard.is_stale(), + "an unrelated endpoint must not close the connection" + ); + assert_eq!(guard.captured_generation(), engine.current_generation()); + assert!(guard.ensure_current().is_ok()); + } + + #[test] + fn revalidating_guard_closes_when_its_endpoint_changes_and_stays_closed() { + let original = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&original).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + let mut narrowed = original.clone(); + policy_mut(&mut narrowed, "claude_code").endpoints[0] = + rest_endpoint("api.anthropic.com", "/v1/messages"); + engine.reload_from_proto(&narrowed).unwrap(); + assert!( + guard.is_stale(), + "an L7 rule on this endpoint changes the contract" + ); + + engine.reload_from_proto(&original).unwrap(); + assert!( + guard.is_stale(), + "restoring the policy must not revive a connection already found stale" + ); + } + + #[test] + fn revalidating_guard_closes_when_l7_rules_of_its_endpoint_change() { + let mut proto = test_proto(); + policy_mut(&mut proto, "claude_code").endpoints[0] = + rest_endpoint("api.anthropic.com", "/v1/messages"); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + + // Every reload restamps policy_hash on every endpoint; that alone + // must not count as a change. + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + engine.reload_from_proto(&proto).unwrap(); + assert!( + !guard.is_stale(), + "policy_hash restamping is not a contract change" + ); + + policy_mut(&mut proto, "claude_code").endpoints[0] = + rest_endpoint("api.anthropic.com", "/v1/**"); + engine.reload_from_proto(&proto).unwrap(); + assert!( + guard.is_stale(), + "per-request rules for this endpoint changed" + ); + } + + #[test] + fn revalidating_guard_closes_when_its_binary_loses_access() { + let mut proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + policy_mut(&mut proto, "claude_code").binaries = vec![NetworkBinary { + path: "/usr/bin/other".to_string(), + }]; + engine.reload_from_proto(&proto).unwrap(); + + assert!(guard.is_stale()); + } + + #[test] + fn revalidating_guard_closes_when_a_new_policy_matches_its_connection() { + let mut proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + // A second policy admitting the same binary and host changes the set + // of endpoints per-request rules consult, even if it only widens. + proto.network_policies.insert( + "another".to_string(), + NetworkPolicyRule { + name: "another".to_string(), + endpoints: vec![proto_endpoint("api.anthropic.com")], + binaries: vec![NetworkBinary { + path: "/usr/local/bin/claude".to_string(), + }], + }, + ); + engine.reload_from_proto(&proto).unwrap(); + + assert!(guard.is_stale()); + } + + #[test] + fn revalidating_guard_never_follows_a_fail_closed_quarantine() { + let proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + engine.enter_fail_closed("candidate rejected").unwrap(); + engine.exit_fail_closed().unwrap(); + + assert!( + guard.is_stale(), + "quarantine is a hard stop even when the previous policy returns" + ); + } + + #[test] + fn revalidating_guard_clones_share_one_pin() { + let mut proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + let clone = guard.clone(); + + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + engine.reload_from_proto(&proto).unwrap(); + + assert!(!guard.is_stale()); + assert_eq!(clone.captured_generation(), engine.current_generation()); + } + + #[tokio::test] + async fn revalidating_guard_waits_through_unrelated_reloads() { + let mut proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + let waiter = tokio::spawn({ + let guard = guard.clone(); + async move { guard.wait_until_stale().await } + }); + tokio::task::yield_now().await; + + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + engine.reload_from_proto(&proto).unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + assert!( + !waiter.is_finished(), + "an unrelated reload must not wake the relay closed" + ); + + policy_mut(&mut proto, "claude_code").binaries.clear(); + engine.reload_from_proto(&proto).unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(1), waiter) + .await + .expect("a relevant reload must end the wait") + .expect("waiter must not panic"); + } + + #[test] + fn connection_contract_excludes_policy_hash_metadata() { + let mut proto = test_proto(); + policy_mut(&mut proto, "claude_code").endpoints[0] = + rest_endpoint("api.anthropic.com", "/v1/messages"); + let before = OpaEngine::from_proto(&proto).unwrap(); + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + let after = OpaEngine::from_proto(&proto).unwrap(); + let input = claude_input("api.anthropic.com"); + + let before_configs = before.authorize_egress(&input).unwrap().endpoint_configs; + let after_configs = after.authorize_egress(&input).unwrap().endpoint_configs; + assert_ne!( + before_configs, after_configs, + "fixture must exercise the per-reload policy_hash stamp" + ); + let (before_contract, _) = before.connection_policy_contract(&input).unwrap(); + let (after_contract, _) = after.connection_policy_contract(&input).unwrap(); + assert!( + before_contract.relays_identically_to(&after_contract), + "policy_hash must not distinguish contracts:\n{before_contract:#?}\n{after_contract:#?}" + ); + } + + #[test] + fn denied_connection_captures_no_revalidator() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("not-allowed.example.com"); + let authorization = engine.authorize_egress(&input).unwrap(); + assert!(matches!(authorization.action, NetworkAction::Deny { .. })); + + let revalidator = + PolicyRevalidator::capture(&engine, input, authorization.generation, None); + assert!(revalidator.is_none()); + } + + #[test] + fn credential_change_publication_wakes_guards_without_changing_the_policy() { + let proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + let strict = engine + .generation_guard(guard.captured_generation()) + .unwrap(); + let before = engine.current_generation(); + + let published = engine.publish_credential_change().unwrap(); + + assert_eq!(published, before + 1); + assert_eq!(engine.current_generation(), published); + assert!(strict.is_stale(), "a plain guard closes on any generation"); + assert!( + !guard.is_stale(), + "a connection that sent no credential re-pins through an unchanged policy" + ); + assert_eq!(guard.captured_generation(), published); + } + + #[test] + fn credential_change_publication_keeps_a_quarantine() { + let proto = test_proto(); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let input = claude_input("api.anthropic.com"); + engine.enter_fail_closed("candidate rejected").unwrap(); + + engine.publish_credential_change().unwrap(); + + assert_eq!( + engine.fail_closed_reason().as_deref(), + Some("candidate rejected") + ); + assert!(!matches!( + engine.authorize_egress(&input).unwrap().action, + NetworkAction::Allow { .. } + )); + } + + /// The contract does not compare `endpoint_configs` because Rego derives + /// them from the matched endpoints. Pin that: the configs are exactly the + /// matched endpoints that carry extended configuration, in the same order. + #[test] + fn endpoint_configs_are_the_matched_endpoints_with_extended_config() { + let mut proto = test_proto(); + for (name, endpoint) in [ + ("inspected", rest_endpoint("api.anthropic.com", "/v1/**")), + ("another", proto_endpoint("api.anthropic.com")), + ] { + proto.network_policies.insert( + name.to_string(), + NetworkPolicyRule { + name: name.to_string(), + endpoints: vec![endpoint], + binaries: vec![NetworkBinary { + path: "/usr/local/bin/claude".to_string(), + }], + }, + ); + } + let engine = OpaEngine::from_proto(&proto).unwrap(); + + let authorization = engine + .authorize_egress(&claude_input("api.anthropic.com")) + .unwrap(); + + assert_eq!(authorization.matched_endpoints.len(), 3); + let extended: Vec<®orus::Value> = authorization + .matched_endpoints + .iter() + .map(|matched| &matched.endpoint) + .filter(|endpoint| { + matches!( + &endpoint["protocol"], + regorus::Value::String(protocol) if protocol.as_ref() == "rest" + ) + }) + .collect(); + assert_eq!( + authorization.endpoint_configs.iter().collect::>(), + extended, + "only the inspected endpoint carries extended configuration" + ); + } + + // --- Middleware parts of the connection contract --- + + /// Policy data with `middlewares` (a YAML `network_middlewares` body, or + /// empty for none) and fixed claude/gitlab network policies. + fn middleware_data(middlewares: &str) -> String { + let middlewares = if middlewares.is_empty() { + String::new() + } else { + format!("network_middlewares:\n{middlewares}") + }; + format!( + r"{middlewares} +network_policies: + claude_code: + name: claude_code + endpoints: + - {{ host: api.anthropic.com, port: 443 }} + binaries: + - {{ path: /usr/local/bin/claude }} + gitlab: + name: gitlab + endpoints: + - {{ host: gitlab.com, port: 443 }} + binaries: + - {{ path: /usr/bin/glab }} +" + ) + } + + /// One middleware stage selecting `host`. + fn middleware_stage(host: &str, on_error: &str, order: i32, mode: &str) -> String { + format!( + r#" scan: + middleware: test/scan + on_error: {on_error} + order: {order} + config: {{ mode: {mode} }} + endpoints: + include: ["{host}"] +"# + ) + } + + fn middleware_engine(middlewares: &str) -> Arc { + Arc::new(OpaEngine::from_strings(TEST_POLICY, &middleware_data(middlewares)).unwrap()) + } + + #[test] + fn revalidating_guard_closes_when_any_field_of_its_chain_changes() { + let original = middleware_stage("api.anthropic.com", "fail_closed", 10, "redact"); + for changed in [ + middleware_stage("api.anthropic.com", "fail_open", 10, "redact"), + middleware_stage("api.anthropic.com", "fail_closed", 20, "redact"), + middleware_stage("api.anthropic.com", "fail_closed", 10, "mask"), + ] { + let engine = middleware_engine(&original); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + engine + .reload(TEST_POLICY, &middleware_data(&changed)) + .unwrap(); + + assert!(guard.is_stale(), "chain change must close: {changed}"); + } + } + + #[test] + fn revalidating_guard_survives_reload_with_an_identical_chain() { + let stage = middleware_stage("api.anthropic.com", "fail_closed", 10, "redact"); + let engine = middleware_engine(&stage); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + // Middleware orders must be unique across the policy. + let other = middleware_stage("gitlab.com", "fail_closed", 20, "redact") + .replace(" scan:", " other_scan:"); + engine + .reload(TEST_POLICY, &middleware_data(&format!("{stage}{other}"))) + .unwrap(); + + assert!( + !guard.is_stale(), + "an unchanged chain for this host must survive" + ); + } + + fn regex_middleware_for(host: &str) -> NetworkMiddlewareConfig { + NetworkMiddlewareConfig { + middleware: openshell_supervisor_middleware_builtins::BUILTIN_REGEX.into(), + on_error: "fail_closed".into(), + endpoints: Some(openshell_core::proto::MiddlewareEndpointSelector { + include: vec![host.into()], + exclude: Vec::new(), + }), + ..Default::default() + } + } + + #[tokio::test] + async fn only_reloads_that_replace_the_registry_close_middleware_connections() { + let mut proto = test_proto(); + proto + .network_middlewares + .insert("redactor".into(), regex_middleware_for("api.anthropic.com")); + let engine = Arc::new(OpaEngine::from_proto(&proto).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("registry.gitlab.com")); + engine.reload_from_proto(&proto).unwrap(); + assert!( + !guard.is_stale(), + "a policy-only reload keeps the installed registry" + ); + + let builtins = MiddlewareRegistry::connect_services( + openshell_supervisor_middleware_builtins::services(), + Vec::new(), + ) + .await + .expect("built-in registry"); + engine + .reload_policy_and_middleware_from_proto_with_pid(&proto, 0, builtins) + .unwrap(); + assert!( + guard.is_stale(), + "a combined reload installs a new registry" + ); + } + + #[test] + fn capture_uses_the_authorizing_generation_from_the_history() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("api.anthropic.com"); + let authorized = engine.authorize_egress(&input).unwrap().generation; + + // Revoke the connection's access; its authorizing generation is still + // recorded, so its baseline is that generation's contract. + let mut revoked = test_proto(); + policy_mut(&mut revoked, "claude_code").binaries.clear(); + engine.reload_from_proto(&revoked).unwrap(); + + assert!(PolicyRevalidator::capture(&engine, input, authorized, None).is_some()); + } + + #[test] + fn capture_refuses_a_generation_that_left_the_history() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("api.anthropic.com"); + let authorized = engine.authorize_egress(&input).unwrap().generation; + + let proto = proto_with_unrelated_endpoint(); + for _ in 0..=GENERATION_HISTORY { + engine.reload_from_proto(&proto).unwrap(); + } + + assert!(PolicyRevalidator::capture(&engine, input, authorized, None).is_none()); + } + + // --- Policy hash for endpoint status after a re-pin --- + + fn stamped_hash_for(engine: &OpaEngine, input: &NetworkInput) -> String { + let authorization = engine.authorize_egress(input).unwrap(); + match &authorization.matched_endpoints[0].endpoint["policy_hash"] { + regorus::Value::String(hash) => hash.to_string(), + other => panic!("proto endpoints carry a policy hash, got {other:?}"), + } + } + + #[test] + fn pinned_policy_hash_starts_at_the_authorizing_generation() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + + assert_eq!( + guard.pinned_policy_hash(), + Some(stamped_hash_for(&engine, &input)) + ); + } + + #[test] + fn plain_guards_report_no_pinned_policy_hash() { + // A plain guard never moves, so its endpoint config's hash applies. + let engine = OpaEngine::from_proto(&test_proto()).unwrap(); + let guard = engine + .generation_guard(engine.current_generation()) + .unwrap(); + + assert_eq!(guard.pinned_policy_hash(), None); + } + + #[test] + fn pinned_policy_hash_follows_the_generation_the_guard_re_pinned_to() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + let original = stamped_hash_for(&engine, &input); + + engine + .reload_from_proto(&proto_with_unrelated_endpoint()) + .unwrap(); + assert!(!guard.is_stale()); + + let reloaded = stamped_hash_for(&engine, &input); + assert_ne!(reloaded, original); + assert_eq!(guard.pinned_policy_hash(), Some(reloaded)); + } + + #[test] + fn pinned_policy_hash_waits_for_the_guard_to_re_pin() { + let engine = Arc::new(OpaEngine::from_proto(&test_proto()).unwrap()); + let input = claude_input("api.anthropic.com"); + let guard = revalidating_guard(&engine, &input); + engine + .reload_from_proto(&proto_with_unrelated_endpoint()) + .unwrap(); + assert!(!guard.is_stale()); + + // A newer generation is live but the guard has not checked it yet: + // the hash it reports must still be that of the generation it holds. + let mut proto = proto_with_unrelated_endpoint(); + policy_mut(&mut proto, "gitlab") + .endpoints + .push(proto_endpoint("pages.gitlab.com")); + let held = stamped_hash_for(&engine, &input); + engine.reload_from_proto(&proto).unwrap(); + + assert_eq!(guard.pinned_policy_hash(), Some(held)); + } + + #[test] + fn pinned_policy_hash_is_none_for_policies_without_a_stamp() { + let engine = Arc::new(OpaEngine::from_strings(TEST_POLICY, &middleware_data("")).unwrap()); + let guard = revalidating_guard(&engine, &claude_input("api.anthropic.com")); + + let other_host = middleware_stage("gitlab.com", "fail_closed", 10, "redact"); + engine + .reload(TEST_POLICY, &middleware_data(&other_host)) + .unwrap(); + assert!(!guard.is_stale()); + + assert_eq!(guard.pinned_policy_hash(), None); + } } diff --git a/crates/openshell-supervisor-network/src/opa/revalidation.rs b/crates/openshell-supervisor-network/src/opa/revalidation.rs new file mode 100644 index 0000000000..fa17018757 --- /dev/null +++ b/crates/openshell-supervisor-network/src/opa/revalidation.rs @@ -0,0 +1,452 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Re-check established connections when the policy generation advances. +//! +//! Every policy reload, middleware registry replacement, and fail-closed +//! transition advances one sandbox-wide generation. Relays pin the generation +//! that authorized them. Without a revalidator, a pinned guard becomes stale as +//! soon as the generation moves, so approving an endpoint for one host closes +//! every open connection in the sandbox. +//! +//! A [`PolicyRevalidator`] lets a pinned guard follow the generation instead. +//! When the generation moves, the guard re-evaluates the connection's original +//! policy input against **every** generation published since its pin, not +//! only the latest. A connection can sleep through several reloads, since the +//! generation watch only delivers the newest value, and one of those reloads +//! may have excluded it even if a later one restored its rules. The engine +//! keeps a bounded history of recent generations for this; a connection whose +//! pin has left the history closes. +//! +//! If the [`ConnectionPolicyContract`] under each of those generations equals +//! the contract that authorized the connection, the guard re-pins to the +//! newest and the relay keeps running. Any difference, an evaluation error, or +//! a fail-closed quarantine among them (which denies everything) makes the +//! guard stale permanently, and the relay closes exactly as it did before. +//! +//! # What the contract must cover +//! +//! The contract must contain every policy input a relay consults after setup, +//! otherwise a connection could outlive a change that should have narrowed it: +//! +//! - The L4 decision and matched policy name. +//! - `matched_endpoints`, in order: the complete endpoint objects of every +//! policy whose binaries admit the caller and whose endpoints match the host +//! and port. Per-request Rego rules (`allow_request`, `deny_request`, +//! `request_deny_reason`, `allowed_token_grant_owners`, and the protocol +//! helpers they call) read only these endpoints, so an unchanged list means +//! every per-request decision is unchanged. This is why a tunnel's cloned +//! evaluator remains valid after it re-pins. +//! - `exact_declared_endpoint_host`, which selects SSRF handling. +//! - The middleware chain selected for the host and, when it is non-empty, the +//! middleware registry epoch, because tunnels hold a clone of the runner. +//! +//! Relays also read `endpoint_configs`: its first entry selects the TLS mode +//! and `allowed_ips`, and the list feeds L7 route selection. It is not compared +//! separately because Rego derives it from the same endpoints, in the same +//! order, as `matched_endpoints`: it is the matched endpoints that carry +//! extended configuration. An unchanged matched list therefore means unchanged +//! configs. +//! +//! Endpoint objects carry `policy_hash`, the hash of the complete canonical +//! policy. It changes on every reload and only binds endpoint-status +//! observations to an inventory, so it is excluded from the comparison. The +//! revalidator instead records the hash of the generation the connection is +//! pinned to, which is the policy its endpoint-status observations belong to. +//! +//! # Provider credentials +//! +//! Relays resolve provider credentials from the live provider state for every +//! request and WebSocket message, under their own revision guard. What a later +//! request cannot undo is an upstream session that an earlier request already +//! authenticated, such as an upgraded WebSocket. +//! +//! HTTP-family relays therefore carry a credential pin on a connection handle +//! that records which credentials the connection injected, and at which +//! version (see `CredentialUseMarker` in `openshell-core`). The credential +//! state owns one version per credential and changes it exactly when that +//! credential's content changes. Every static secret goes through +//! `SecretResolver`, which reports the environment key and version of each +//! value it hands out, and every token grant goes through `inject_or_report`, +//! which reports the grant's identity before adding the header. Until the +//! first use the pin is inert: the connection has never carried a credential +//! upstream, so a credential change cannot leave it holding a stale one. +//! +//! After that, the connection closes only when a credential it used is no +//! longer at the version it used: rotated, reconfigured, removed, or already +//! superseded when it was sent. Changes to other providers' credentials leave +//! it open. A use that cannot be attributed to one credential is recorded as +//! stale and closes the connection on the next check. Raw TCP and TLS +//! passthrough relays never inject credentials and carry no pin. + +use super::{EgressAuthorization, NetworkAction, NetworkInput, OpaEngine, PolicyGenerationGuard}; +use openshell_core::provider_credentials::ProviderCredentialState; +use openshell_supervisor_middleware::ChainEntry; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Weak}; +use tracing::debug; + +/// Endpoint keys that are policy metadata rather than policy semantics. +/// +/// `policy_hash` hashes the complete canonical policy, so it differs after +/// every reload even when this endpoint did not change. +const NON_SEMANTIC_ENDPOINT_KEYS: &[&str] = &["policy_hash"]; + +/// Upper bound on back-to-back reloads a single staleness check will follow. +/// +/// Each round re-evaluates against the live engine. If reloads keep landing +/// while a check runs, the guard gives up and reports stale, which is the +/// pre-revalidation behavior. +const MAX_REVALIDATION_ROUNDS: usize = 4; + +/// Policy facts that decide how an established connection is relayed. +/// +/// Compare contracts with [`Self::relays_identically_to`]. The type +/// deliberately has no `==`: its equality is a policy judgement, not +/// structural identity, and the method is written so that adding a field +/// fails to compile until the comparison handles it. See the module +/// documentation for why each field is present. +#[derive(Debug, Clone)] +pub struct ConnectionPolicyContract { + action: NetworkAction, + /// `(policy_name, endpoint)` pairs, in order. `endpoint_configs` is derived + /// from these, so it is not kept; see the module documentation. The + /// endpoint index is deliberately omitted: inserting an unrelated endpoint + /// earlier in the same policy shifts indexes without changing what this + /// connection may do. + matched_endpoints: Vec<(String, regorus::Value)>, + exact_declared_endpoint_host: bool, + middleware_chain: Vec, + /// Present only when the chain is non-empty; a registry swap cannot affect + /// a connection that runs no middleware. + middleware_registry_epoch: Option, + /// The complete-policy hash stamped into this generation's endpoints, or + /// empty for policies that carry none. Metadata, not compared: it is what + /// endpoint-status observations of a re-pinned connection belong to. + policy_hash: String, +} + +impl ConnectionPolicyContract { + /// Built only by the engine, from one generation's authorization and + /// middleware selection, so a baseline and its re-checks share one path. + pub(super) fn new( + authorization: &EgressAuthorization, + middleware_chain: Vec, + middleware_registry_epoch: u64, + ) -> Self { + let middleware_registry_epoch = + (!middleware_chain.is_empty()).then_some(middleware_registry_epoch); + Self { + action: authorization.action.clone(), + matched_endpoints: authorization + .matched_endpoints + .iter() + .map(|endpoint| { + ( + endpoint.policy_name.clone(), + semantic_endpoint(&endpoint.endpoint), + ) + }) + .collect(), + exact_declared_endpoint_host: authorization.exact_declared_endpoint_host, + middleware_chain, + middleware_registry_epoch, + policy_hash: stamped_policy_hash(authorization), + } + } + + fn allows(&self) -> bool { + matches!(self.action, NetworkAction::Allow { .. }) + } + + /// Whether a connection authorized under `other` is authorized and + /// relayed identically under `self`. + /// + /// The destructuring patterns name every field without `..`, so a new + /// field in the contract must be added here before the crate compiles. + /// Forgetting one would let connections survive changes to it. + pub fn relays_identically_to(&self, other: &Self) -> bool { + let Self { + action, + matched_endpoints, + exact_declared_endpoint_host, + middleware_chain, + middleware_registry_epoch, + // Changes on every reload; see `NON_SEMANTIC_ENDPOINT_KEYS`. + policy_hash: _, + } = self; + let Self { + action: other_action, + matched_endpoints: other_matched_endpoints, + exact_declared_endpoint_host: other_exact_declared_endpoint_host, + middleware_chain: other_middleware_chain, + middleware_registry_epoch: other_middleware_registry_epoch, + policy_hash: _, + } = other; + action == other_action + && matched_endpoints == other_matched_endpoints + && exact_declared_endpoint_host == other_exact_declared_endpoint_host + && same_middleware_chain(middleware_chain, other_middleware_chain) + && middleware_registry_epoch == other_middleware_registry_epoch + } +} + +/// Whether two selected chains run the same stages in the same order with +/// the same settings. +/// +/// Compares every [`ChainEntry`] field explicitly instead of relying on a +/// derived `==` on the shared type, for the same exhaustiveness reason as +/// [`ConnectionPolicyContract::relays_identically_to`]. +fn same_middleware_chain(left: &[ChainEntry], right: &[ChainEntry]) -> bool { + left.len() == right.len() + && left.iter().zip(right).all(|(left, right)| { + let ChainEntry { + name, + implementation, + order, + config, + on_error, + } = left; + let ChainEntry { + name: other_name, + implementation: other_implementation, + order: other_order, + config: other_config, + on_error: other_on_error, + } = right; + name == other_name + && implementation == other_implementation + && order == other_order + && config == other_config + && on_error == other_on_error + }) +} + +/// The complete-policy hash proto conversion stamps into every endpoint of a +/// generation, taken from any endpoint this authorization matched. Empty when +/// the policy carries none, as YAML policies do. +fn stamped_policy_hash(authorization: &EgressAuthorization) -> String { + authorization + .matched_endpoints + .iter() + .map(|endpoint| &endpoint.endpoint) + .find_map(|endpoint| match &endpoint["policy_hash"] { + regorus::Value::String(hash) if !hash.is_empty() => Some(hash.to_string()), + _ => None, + }) + .unwrap_or_default() +} + +/// Copy an endpoint object without keys that change on every reload. +fn semantic_endpoint(value: ®orus::Value) -> regorus::Value { + let mut value = value.clone(); + let has_metadata = matches!( + &value, + regorus::Value::Object(fields) + if NON_SEMANTIC_ENDPOINT_KEYS + .iter() + .any(|key| fields.contains_key(®orus::Value::from(*key))) + ); + if has_metadata && let Ok(fields) = value.as_object_mut() { + for key in NON_SEMANTIC_ENDPOINT_KEYS { + fields.remove(®orus::Value::from(*key)); + } + } + value +} + +/// Re-evaluates one connection's policy contract after a generation change. +/// +/// Shared by every clone of the guards pinned for that connection. Once a +/// re-evaluation finds a different contract the revalidator is invalidated for +/// good, so restoring the earlier policy cannot revive a connection that was +/// already observed under a policy that excluded it. +#[derive(Debug)] +pub struct PolicyRevalidator { + engine: Weak, + input: NetworkInput, + baseline: ConnectionPolicyContract, + credentials: Option, + invalidated: AtomicBool, + /// The newest generation this connection is pinned to, starting with the + /// one that authorized it, and that generation's stamped policy hash. + pinned_policy: std::sync::Mutex<(u64, String)>, +} + +/// What a relay's credentials must still match for it to stay open. +/// +/// Only tracked handles (see `ProviderCredentialState::with_use_tracking`) +/// can say which credentials a connection used. Every production relay gets +/// one; a pin on an untracked handle reports stale, so a caller that forgets +/// to track closes on the next reload rather than never. +#[derive(Debug)] +struct CredentialPin(ProviderCredentialState); + +impl CredentialPin { + /// False once a credential this connection injected is no longer at the + /// version it injected. + fn is_current(&self) -> bool { + self.0.credential_uses_current() + } +} + +impl PolicyRevalidator { + /// Capture the contract that authorized a connection with `input` at + /// `authorized_generation`. + /// + /// The baseline is evaluated by the same function as every re-check, from + /// that generation's recorded policy. Pass the relay's provider credential + /// handle when the relay can inject credentials; see the module + /// documentation. + /// + /// Returns `None` for denials, and when the generation has left the + /// history. A guard without a revalidator keeps the strict + /// close-on-change behavior. + pub(crate) fn capture( + engine: &Arc, + input: NetworkInput, + authorized_generation: u64, + credentials: Option<&ProviderCredentialState>, + ) -> Option> { + let baseline = engine + .connection_contract_at(&input, authorized_generation) + .ok() + .flatten() + .filter(ConnectionPolicyContract::allows)?; + let pinned_policy = + std::sync::Mutex::new((authorized_generation, baseline.policy_hash.clone())); + Some(Arc::new(Self { + engine: Arc::downgrade(engine), + baseline, + input, + credentials: credentials.map(|state| CredentialPin(state.clone())), + invalidated: AtomicBool::new(false), + pinned_policy, + })) + } + + pub(super) fn is_invalidated(&self) -> bool { + self.invalidated.load(Ordering::Acquire) + } + + /// Re-evaluate against every generation published since + /// `captured_generation`. + /// + /// Returns the generation the connection was checked up to, or `None` + /// (permanently) when its contract did not hold throughout. + fn revalidate(&self, captured_generation: u64) -> Option { + if self.is_invalidated() { + return None; + } + let outcome = self.evaluate(captured_generation); + if outcome.is_none() { + self.invalidated.store(true, Ordering::Release); + } + outcome + } + + fn evaluate(&self, captured_generation: u64) -> Option { + if !self + .credentials + .as_ref() + .is_none_or(CredentialPin::is_current) + { + return None; + } + let engine = self.engine.upgrade()?; + // Every generation since the pin, not only the latest: a reload that + // would have closed this connection must close it even if a later one + // restored its rules before it woke. That includes fail-closed + // quarantines, whose generations deny everything. Evaluation errors + // and generations no longer in the history fail closed. + let (contracts, current) = engine + .connection_contracts_since(&self.input, captured_generation) + .ok() + .flatten()?; + if !contracts + .iter() + .all(|contract| contract.relays_identically_to(&self.baseline)) + { + return None; + } + if let Some(newest) = contracts.last() { + self.record_repin(current, &newest.policy_hash); + } + Some(current) + } + + fn record_repin(&self, generation: u64, policy_hash: &str) { + let mut pinned = self + .pinned_policy + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if pinned.0 < generation { + *pinned = (generation, policy_hash.to_owned()); + } + } + + /// The stamped policy hash of `pinned_generation`, the generation that + /// authorized this connection or one it re-pinned to, if its policy carries + /// one. Endpoint-status observations of the connection belong to that + /// policy: it authorized the connection, or its contract was shown + /// identical there. + pub(super) fn pinned_policy_hash(&self, pinned_generation: u64) -> Option { + let pinned = self + .pinned_policy + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let (generation, hash) = &*pinned; + (*generation == pinned_generation && !hash.is_empty()).then(|| hash.clone()) + } + + /// Advance a shared pin while the contract still holds. + /// + /// Each round checks only the generations published since the previous + /// one. Returns `true` once `pin` equals the live generation. + pub(super) fn follow(&self, pin: &AtomicU64, live: &AtomicU64) -> bool { + for _ in 0..MAX_REVALIDATION_ROUNDS { + let captured = pin.load(Ordering::Acquire); + if captured == live.load(Ordering::Acquire) { + return !self.is_invalidated(); + } + let Some(generation) = self.revalidate(captured) else { + debug!( + host = %self.input.host, + port = self.input.port, + captured_generation = captured, + "Policy change affects established connection; closing" + ); + return false; + }; + // Clones of a guard share the pin; never move it backwards. + let pinned = pin.fetch_max(generation, Ordering::AcqRel).max(generation); + debug!( + host = %self.input.host, + port = self.input.port, + previous_generation = captured, + pinned_generation = pinned, + "Policy change does not affect established connection; re-pinned" + ); + } + pin.load(Ordering::Acquire) == live.load(Ordering::Acquire) && !self.is_invalidated() + } +} + +impl OpaEngine { + /// Pin `generation` for an established connection with `input`, re-pinning + /// through later generations that leave its contract intact. + /// + /// The single entry point for relays and their tests. Fails if + /// `generation` is no longer current. `credentials` is the connection's + /// tracked provider handle when the relay can inject credentials. + pub(crate) fn revalidating_generation_guard( + self: &Arc, + input: NetworkInput, + generation: u64, + credentials: Option<&ProviderCredentialState>, + ) -> miette::Result { + let guard = self.generation_guard(generation)?; + let revalidator = PolicyRevalidator::capture(self, input, generation, credentials); + Ok(guard.with_revalidator(revalidator)) + } +} diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index 96c0b8f50a..02b13ad324 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -1288,9 +1288,7 @@ async fn handle_transparent_tcp_connection( // generation before correlating endpoint identity or constructing a // connector. This prevents combining an old DNS answer with a newer // policy decision (or vice versa). - let Ok(generation_guard) = - relay::pin_policy_generation(&opa_engine, decision.policy_generation) - else { + let Ok(generation_guard) = relay::pin_policy_generation(&opa_engine, &decision, None) else { emit_transparent_mapping_denial( workload_addr, original, @@ -2929,21 +2927,17 @@ async fn handle_mediated_connection( return Ok(()); } - let connect_generation_guard = - match relay::pin_policy_generation(&opa_engine, decision.policy_generation) { - Ok(guard) => guard, - Err(error) => { - reject_stale_connect_policy( - &mut client, - &host_lc, - port, - activity_tx.as_ref(), - error, - ) + // Setup only: the relay chosen below pins its own guard, and pins provider + // credentials if it can inject them. + let connect_pin = relay::pin_policy_generation(&opa_engine, &decision, None); + let connect_generation_guard = match connect_pin { + Ok(guard) => guard, + Err(error) => { + reject_stale_connect_policy(&mut client, &host_lc, port, activity_tx.as_ref(), error) .await?; - return Ok(()); - } - }; + return Ok(()); + } + }; // Resolve the route's TLS treatment up front. `query_tls_mode` reads only // the policy decision + host/port (no peeked bytes), so it is valid before @@ -5763,10 +5757,14 @@ async fn handle_forward_proxy( "Forward proxy L4 policy decision" ); let sandbox_entrypoint_pid = entrypoint_pid.load(Ordering::Acquire); - let forward_generation_guard = match relay::pin_policy_generation( - &opa_engine, - decision.policy_generation, - ) { + // Track before pinning so the guard and the request context share one + // credential use record (`http_context` keeps an existing marker). + let provider_credentials = provider_credentials + .as_ref() + .map(ProviderCredentialState::with_use_tracking); + let forward_credentials = provider_credentials.as_ref(); + let forward_pin = relay::pin_policy_generation(&opa_engine, &decision, forward_credentials); + let forward_generation_guard = match forward_pin { Ok(guard) => guard, Err(e) => { warn!( @@ -5909,7 +5907,13 @@ async fn handle_forward_proxy( .await?; return Ok(()); } - let tunnel_engine = match relay::pin_l7_evaluator(&opa_engine, route.l7_policy_generation) { + let tunnel_pin = relay::pin_l7_evaluator( + &opa_engine, + &decision, + route.l7_policy_generation, + l7_ctx.provider_credentials.as_ref(), + ); + let tunnel_engine = match tunnel_pin { Ok(engine) => engine, Err(e) => { warn!( diff --git a/crates/openshell-supervisor-network/src/proxy/egress.rs b/crates/openshell-supervisor-network/src/proxy/egress.rs index 77b89a9b7e..78b4976f47 100644 --- a/crates/openshell-supervisor-network/src/proxy/egress.rs +++ b/crates/openshell-supervisor-network/src/proxy/egress.rs @@ -170,6 +170,25 @@ pub(super) struct EgressDecision { pub(super) cmdline_paths: Vec, } +impl EgressDecision { + /// The policy input this decision was evaluated with. + /// + /// Every adapter builds its OPA input from the intent destination and the + /// identity fields stored here (an absent binary is the empty path used by + /// endpoint-only mode). The binary digest is omitted because Rego never + /// reads it; integrity is enforced in Rust before authorization. + pub(super) fn policy_input(&self) -> crate::opa::NetworkInput { + crate::opa::NetworkInput { + host: self.intent.destination.host.clone(), + port: self.intent.destination.port, + binary_path: self.binary.clone().unwrap_or_default(), + binary_sha256: String::new(), + ancestors: self.ancestors.clone(), + cmdline_paths: self.cmdline_paths.clone(), + } + } +} + pub(super) fn connect_span(intent: &EgressIntent) -> tracing::Span { tracing::debug_span!( "supervisor.egress.connect", diff --git a/crates/openshell-supervisor-network/src/proxy/relay.rs b/crates/openshell-supervisor-network/src/proxy/relay.rs index 495c22241b..3ba29e8d61 100644 --- a/crates/openshell-supervisor-network/src/proxy/relay.rs +++ b/crates/openshell-supervisor-network/src/proxy/relay.rs @@ -5,10 +5,13 @@ use super::{EgressDecision, L7RouteSnapshot, emit_l7_tunnel_close_after_policy_change}; use crate::l7::relay::L7EvalContext; -use crate::opa::{NetworkAction, OpaEngine, PolicyGenerationGuard, TunnelPolicyEngine}; +use crate::opa::{ + NetworkAction, OpaEngine, PolicyGenerationGuard, PolicyRevalidator, TunnelPolicyEngine, +}; use miette::{IntoDiagnostic, Result}; use openshell_core::activity::ActivitySender; use openshell_core::endpoint_status::EndpointObservationSender; +use openshell_core::provider_credentials::ProviderCredentialState; use openshell_core::secrets::SecretResolver; use std::sync::Arc; use tokio::io::{AsyncRead, AsyncWrite}; @@ -45,12 +48,18 @@ pub(super) struct RelaySignals { /// Build the request-processing context shared by CONNECT and forward HTTP. pub(super) fn http_context( decision: &EgressDecision, - provider_credentials: Option, + provider_credentials: Option, secret_resolver: Option>, policy_local: Option<&crate::policy_local::PolicyLocalContext>, agent_proposals: openshell_core::proposals::AgentProposals, signals: RelaySignals, ) -> L7EvalContext { + // One use record per connection: every resolver and grant derived from + // this context reports to it, and the relay's credential pin reads it. + // Idempotent, so an adapter that already tracked the handle keeps it. + let provider_credentials = provider_credentials + .as_ref() + .map(ProviderCredentialState::with_use_tracking); // Provider-backed credentials must be acquired from the live state for // each request after middleware/token-grant awaits. Keep only the legacy // resolver fallback when no live provider state exists. @@ -104,20 +113,43 @@ pub(super) fn http_context( } } -/// Pin a generation for a relay or the forward HTTP single-request path. +/// Pin the generation that authorized `decision` for a relay, CONNECT setup, +/// or the forward HTTP single-request path. +/// +/// The pin fails if the policy changed after authorization. Once pinned, the +/// guard follows later changes that leave the connection's contract intact; +/// see [`crate::opa::PolicyRevalidator`]. `credentials` is the connection's +/// tracked provider handle when the guarded path may inject credentials, or +/// `None` when it never does. pub(super) fn pin_policy_generation( - opa_engine: &OpaEngine, - expected_generation: u64, + opa_engine: &Arc, + decision: &EgressDecision, + credentials: Option<&ProviderCredentialState>, ) -> Result { - opa_engine.generation_guard(expected_generation) + opa_engine.revalidating_generation_guard( + decision.policy_input(), + decision.policy_generation, + credentials, + ) } /// Clone an L7 evaluator for a relay or the forward HTTP single-request path. pub(super) fn pin_l7_evaluator( - opa_engine: &OpaEngine, + opa_engine: &Arc, + decision: &EgressDecision, expected_generation: u64, + credentials: Option<&ProviderCredentialState>, ) -> Result { - opa_engine.clone_engine_for_tunnel(expected_generation) + let evaluator = opa_engine.clone_engine_for_tunnel(expected_generation)?; + // The baseline must come from the generation the evaluator is pinned to, + // so that every later generation is checked. + let revalidator = PolicyRevalidator::capture( + opa_engine, + decision.policy_input(), + expected_generation, + credentials, + ); + Ok(evaluator.with_revalidator(revalidator)) } pub(super) fn validate_route_generation( @@ -144,7 +176,7 @@ pub(super) fn validate_route_generation( /// attempting to write an HTTP response into it. pub(super) fn prepare_http_relay<'a>( route: Option<&L7RouteSnapshot>, - opa_engine: &'a OpaEngine, + opa_engine: &'a Arc, decision: &EgressDecision, request: &'a L7EvalContext, ) -> Option> { @@ -157,8 +189,11 @@ pub(super) fn prepare_http_relay<'a>( return None; } + // HTTP relays can inject provider credentials, so they also pin them. + let credentials = request.provider_credentials.as_ref(); let policy = if let Some(route) = route.filter(|route| !route.configs.is_empty()) { - let evaluator = match pin_l7_evaluator(opa_engine, decision.policy_generation) { + let generation = decision.policy_generation; + let evaluator = match pin_l7_evaluator(opa_engine, decision, generation, credentials) { Ok(evaluator) => evaluator, Err(error) => { emit_l7_tunnel_close_after_policy_change( @@ -179,7 +214,7 @@ pub(super) fn prepare_http_relay<'a>( evaluator: Box::new(evaluator), } } else { - let generation_guard = match pin_policy_generation(opa_engine, decision.policy_generation) { + let generation_guard = match pin_policy_generation(opa_engine, decision, credentials) { Ok(guard) => guard, Err(error) => { emit_l7_tunnel_close_after_policy_change( @@ -196,16 +231,18 @@ pub(super) fn prepare_http_relay<'a>( Some(RelayContext { request, policy, - middleware_engine: opa_engine, + middleware_engine: opa_engine.as_ref(), }) } -/// Pin the generation used by a raw relay so policy activation or quarantine -/// closes streams that otherwise have no request boundary at which to notice -/// a stale decision. +/// Pin the generation used by a raw relay so a policy change that affects the +/// connection, or any quarantine, closes streams that otherwise have no +/// request boundary at which to notice a stale decision. +/// +/// Raw relays never inject provider credentials, so they do not pin them. pub(super) fn prepare_raw_relay( route: Option<&L7RouteSnapshot>, - opa_engine: &OpaEngine, + opa_engine: &Arc, decision: &EgressDecision, ) -> Option { if let Err(error) = validate_route_generation(route, decision.policy_generation) { @@ -217,7 +254,7 @@ pub(super) fn prepare_raw_relay( return None; } - match pin_policy_generation(opa_engine, decision.policy_generation) { + match pin_policy_generation(opa_engine, decision, None) { Ok(guard) => Some(guard), Err(error) => { emit_l7_tunnel_close_after_policy_change( @@ -362,7 +399,6 @@ fn emit_stale_relay_close(request: &L7EvalContext, guard: &PolicyGenerationGuard mod tests { use super::super::{EgressIntent, EndpointDecision, ProcessIdentityEvidence}; use super::*; - const POLICY_REGO: &str = include_str!("../../data/sandbox-policy.rego"); const EMPTY_POLICY_DATA: &str = "network_policies: {}\n"; @@ -412,7 +448,7 @@ mod tests { use tokio::io::{AsyncReadExt, AsyncWriteExt}; let (mut upstream, mut server) = tokio::io::duplex(1024); - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(engine.current_generation()); let request = request_context(); let context = prepare_http_relay(None, &engine, &decision, &request).unwrap(); @@ -501,7 +537,7 @@ mod tests { #[test] fn relay_without_route_pins_l4_decision_generation() { - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(engine.current_generation()); let request = request_context(); @@ -519,7 +555,7 @@ mod tests { #[test] fn empty_hydrated_route_cannot_replace_stale_l4_generation() { - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(u64::MAX); let route = L7RouteSnapshot { configs: vec![], @@ -535,7 +571,7 @@ mod tests { #[test] fn inspected_route_cannot_replace_stale_l4_generation() { - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(u64::MAX); let route = L7RouteSnapshot { configs: vec![super::super::L7ConfigSnapshot { @@ -573,7 +609,7 @@ mod tests { #[test] fn raw_route_cannot_replace_stale_l4_generation() { - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(u64::MAX); let route = L7RouteSnapshot { configs: vec![], @@ -588,7 +624,7 @@ mod tests { #[test] fn stale_generation_fails_before_relay_context_is_created() { - let engine = OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, EMPTY_POLICY_DATA).unwrap()); let decision = decision(engine.current_generation()); let request = request_context(); engine.reload(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); @@ -624,4 +660,1714 @@ mod tests { .expect("relay task should not panic") .expect("stale relay closure should be clean"); } + + // --- Established relays across policy reloads --- + // + // A reload advances one sandbox-wide policy generation. These tests pin + // down which reloads close an established relay. Reloads that change the + // connection's own rules, credentials, or middleware must close it. + + /// Policy allowing curl to reach example.com:80 at L4. + const CURL_EXAMPLE_POLICY: &str = r" +network_policies: + web: + name: web + endpoints: + - { host: example.com, port: 80 } + binaries: + - { path: /usr/bin/curl } +"; + + /// [`CURL_EXAMPLE_POLICY`] after approving an endpoint for another host. + const CURL_EXAMPLE_AND_OTHER_POLICY: &str = r" +network_policies: + web: + name: web + endpoints: + - { host: example.com, port: 80 } + binaries: + - { path: /usr/bin/curl } + other: + name: other + endpoints: + - { host: other.example.net, port: 443 } + binaries: + - { path: /usr/bin/curl } +"; + + /// example.com:80 inspected as REST. + const CURL_EXAMPLE_REST_POLICY: &str = r#" +network_policies: + web: + name: web + endpoints: + - host: example.com + port: 80 + protocol: rest + enforcement: enforce + rules: + - allow: { method: GET, path: "/**" } + binaries: + - { path: /usr/bin/curl } +"#; + + /// [`CURL_EXAMPLE_REST_POLICY`] after approving an endpoint for another host. + const CURL_EXAMPLE_REST_AND_OTHER_POLICY: &str = r#" +network_policies: + web: + name: web + endpoints: + - host: example.com + port: 80 + protocol: rest + enforcement: enforce + rules: + - allow: { method: GET, path: "/**" } + binaries: + - { path: /usr/bin/curl } + other: + name: other + endpoints: + - { host: other.example.net, port: 443 } + binaries: + - { path: /usr/bin/curl } +"#; + + /// [`CURL_EXAMPLE_REST_POLICY`] with narrower rules for example.com. + const CURL_EXAMPLE_REST_NARROWED_POLICY: &str = r#" +network_policies: + web: + name: web + endpoints: + - host: example.com + port: 80 + protocol: rest + enforcement: enforce + rules: + - allow: { method: GET, path: "/public/**" } + binaries: + - { path: /usr/bin/curl } +"#; + + /// example.com:80 with a fail-open middleware, then the same chain made + /// fail-closed. Raw relays decide the uninspectable-traffic gate at setup. + const CURL_EXAMPLE_FAIL_OPEN_MIDDLEWARE_POLICY: &str = r#" +network_middlewares: + scan: + middleware: test/scan + on_error: fail_open + endpoints: + include: ["example.com"] +network_policies: + web: + name: web + endpoints: + - { host: example.com, port: 80 } + binaries: + - { path: /usr/bin/curl } +"#; + + const CURL_EXAMPLE_FAIL_CLOSED_MIDDLEWARE_POLICY: &str = r#" +network_middlewares: + scan: + middleware: test/scan + on_error: fail_closed + endpoints: + include: ["example.com"] +network_policies: + web: + name: web + endpoints: + - { host: example.com, port: 80 } + binaries: + - { path: /usr/bin/curl } +"#; + + /// Authorize curl to example.com:80 the way the CONNECT adapter does. + fn authorized_decision(engine: &OpaEngine) -> EgressDecision { + let mut decision = EgressDecision { + intent: EgressIntent::connect("example.com".to_string(), 80), + action: NetworkAction::Deny { + reason: String::new(), + }, + policy_generation: 0, + identity: ProcessIdentityEvidence::Available, + endpoint: EndpointDecision::default(), + binary: Some(std::path::PathBuf::from("/usr/bin/curl")), + binary_pid: None, + ancestors: vec![], + cmdline_paths: vec![], + }; + // Authorize with the input revalidation re-evaluates. + let authorization = engine.authorize_egress(&decision.policy_input()).unwrap(); + assert!( + matches!(authorization.action, NetworkAction::Allow { .. }), + "fixture must authorize example.com:80" + ); + decision.action = authorization.action.clone(); + decision.policy_generation = authorization.generation; + decision.endpoint = EndpointDecision::from_authorization(&authorization); + decision + } + + /// The L7 route the CONNECT adapter hydrates from `decision`. + fn inspected_route(decision: &EgressDecision) -> L7RouteSnapshot { + let configs: Vec<_> = decision + .endpoint + .policy_configs + .iter() + .filter_map(crate::l7::parse_l7_config) + .map(|config| super::super::L7ConfigSnapshot { config }) + .collect(); + assert!(!configs.is_empty(), "fixture must select an L7 config"); + L7RouteSnapshot { + configs, + l7_policy_generation: decision.policy_generation, + } + } + + /// Run a raw relay for `decision` and report whether it closed after + /// `reload` (within `wait`). + async fn raw_relay_closes_after( + engine: &Arc, + decision: &EgressDecision, + reload: impl FnOnce(&OpaEngine), + wait: std::time::Duration, + ) -> bool { + let guard = prepare_raw_relay(None, engine, decision).expect("current decision pins"); + let request = request_context(); + let (_client_peer, mut proxy_client) = tokio::io::duplex(64); + let (_upstream_peer, mut proxy_upstream) = tokio::io::duplex(64); + let relay = tokio::spawn(async move { + relay_tcp(&mut proxy_client, &mut proxy_upstream, &guard, &request).await + }); + tokio::task::yield_now().await; + + reload(engine.as_ref()); + + let Ok(joined) = tokio::time::timeout(wait, relay).await else { + return false; + }; + joined + .expect("relay task should not panic") + .expect("closure should be clean"); + true + } + + /// Approving an endpoint for another host leaves a raw relay open. + #[tokio::test] + async fn raw_relay_survives_when_an_unrelated_endpoint_is_approved() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| { + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + }, + std::time::Duration::from_millis(200), + ) + .await; + + assert!(!closed, "approving another host must not close this relay"); + } + + /// A raw relay keeps moving bytes after an unrelated reload, and + /// still closes when its own endpoint is revoked. + #[tokio::test] + async fn raw_relay_keeps_relaying_bytes_after_an_unrelated_reload() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let guard = prepare_raw_relay(None, &engine, &decision).expect("current decision pins"); + let request = request_context(); + let (mut client_peer, mut proxy_client) = tokio::io::duplex(64); + let (mut upstream_peer, mut proxy_upstream) = tokio::io::duplex(64); + let relay = tokio::spawn(async move { + relay_tcp(&mut proxy_client, &mut proxy_upstream, &guard, &request).await + }); + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + client_peer.write_all(b"ping").await.unwrap(); + let mut received = [0u8; 4]; + tokio::time::timeout( + std::time::Duration::from_secs(1), + upstream_peer.read_exact(&mut received), + ) + .await + .expect("bytes must keep flowing after the reload") + .unwrap(); + assert_eq!(&received, b"ping"); + + // The same relay still closes once its own endpoint is revoked. + engine.reload(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(1), relay) + .await + .expect("revoking this endpoint must close the relay") + .expect("relay task should not panic") + .expect("stale relay closure should be clean"); + } + + /// Revoking the connection's endpoint closes a raw relay. + #[tokio::test] + async fn raw_relay_closes_when_its_endpoint_is_revoked() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| engine.reload(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(), + std::time::Duration::from_secs(1), + ) + .await; + + assert!(closed, "revoking the endpoint must close the relay"); + } + + /// A raw relay that was let through a fail-open chain closes + /// when the chain becomes fail-closed, which would now refuse it. + #[tokio::test] + async fn raw_relay_closes_when_its_middleware_becomes_fail_closed() { + let engine = Arc::new( + OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_FAIL_OPEN_MIDDLEWARE_POLICY).unwrap(), + ); + let decision = authorized_decision(&engine); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| { + engine + .reload(POLICY_REGO, CURL_EXAMPLE_FAIL_CLOSED_MIDDLEWARE_POLICY) + .unwrap(); + }, + std::time::Duration::from_secs(1), + ) + .await; + + assert!( + closed, + "a fail-closed chain would now refuse this uninspectable stream" + ); + } + + /// Approving an endpoint for another host leaves an HTTP + /// passthrough relay current, re-pinned to the new generation. + #[test] + fn http_passthrough_relay_stays_current_when_an_unrelated_endpoint_is_approved() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let generation_guard = http_relay_guard(&engine, &decision, &request); + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + + assert!(!generation_guard.is_stale(), "approving another host must not close this relay"); + assert_eq!(generation_guard.captured_generation(), engine.current_generation()); + } + + /// Approving an endpoint for another host leaves an inspected (L7) + /// tunnel current. + #[test] + fn inspected_relay_stays_current_when_an_unrelated_endpoint_is_approved() { + let engine = + Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_REST_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let route = inspected_route(&decision); + let request = request_context(); + let context = prepare_http_relay(Some(&route), &engine, &decision, &request).unwrap(); + let PreparedHttpPolicy::Inspect { evaluator, .. } = context.policy else { + panic!("an L7 route should use a tunnel evaluator"); + }; + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_REST_AND_OTHER_POLICY) + .unwrap(); + + assert!( + !evaluator.is_stale(), + "approving another host must not close this tunnel" + ); + } + + /// Narrowing the L7 rules of the connection's endpoint makes its + /// tunnel stale. + #[test] + fn inspected_relay_goes_stale_when_its_rules_change() { + let engine = + Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_REST_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let route = inspected_route(&decision); + let request = request_context(); + let context = prepare_http_relay(Some(&route), &engine, &decision, &request).unwrap(); + let PreparedHttpPolicy::Inspect { evaluator, .. } = context.policy else { + panic!("an L7 route should use a tunnel evaluator"); + }; + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_REST_NARROWED_POLICY) + .unwrap(); + + assert!(evaluator.is_stale(), "narrowed rules must close the tunnel"); + } + + /// A relay whose credential handle cannot report use goes stale + /// when provider credentials change. + #[test] + fn http_relay_goes_stale_when_provider_credentials_change() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = empty_provider_credentials(); + let mut request = request_context(); + request.provider_credentials = Some(credentials.clone()); + let context = prepare_http_relay(None, &engine, &decision, &request).unwrap(); + let PreparedHttpPolicy::Passthrough { generation_guard } = context.policy else { + panic!("route-less relay should use a generation guard"); + }; + + // Provider credentials change together with a runtime reload; the + // policy itself is identical. An upstream session (for example an + // upgraded WebSocket) may hold the previous credential. This handle + // does not track use, so it cannot show which credentials it carried + // and the relay closes on the next reload. + credentials.install_child_env_snapshot(2, std::collections::HashMap::new()); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(generation_guard.is_stale()); + } + + /// The request context the CONNECT adapter builds for `decision`, with + /// the sandbox's provider credentials. + fn connection_context( + decision: &EgressDecision, + credentials: &ProviderCredentialState, + ) -> L7EvalContext { + let mut context = http_context( + decision, + Some(credentials.clone()), + None, + None, + openshell_core::proposals::AgentProposals::default(), + RelaySignals { + activity: None, + endpoint_observation: None, + }, + ); + context.request_default_port = Some(80); + context + } + + /// The HTTP relay guard the CONNECT adapter pins for `request`. + fn http_relay_guard( + engine: &Arc, + decision: &EgressDecision, + request: &L7EvalContext, + ) -> PolicyGenerationGuard { + let context = prepare_http_relay(None, engine, decision, request).unwrap(); + let PreparedHttpPolicy::Passthrough { generation_guard } = context.policy else { + panic!("route-less relay should use a generation guard"); + }; + generation_guard + } + + fn token_credentials(revision: u64, tokens: &[(&str, &str)]) -> ProviderCredentialState { + ProviderCredentialState::from_environment( + revision, + tokens + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(), + std::collections::HashMap::new(), + std::collections::HashMap::new(), + ) + } + + /// Change provider credentials the way a runtime reload does: install the + /// new material, then publish a policy generation. + fn change_provider_credentials( + engine: &OpaEngine, + credentials: &ProviderCredentialState, + revision: u64, + tokens: &[(&str, &str)], + ) { + credentials.install_environment( + revision, + tokens + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(), + std::collections::HashMap::new(), + std::collections::HashMap::new(), + ); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + } + + /// Inject `env_key` on the connection with context `request`, the way a + /// request rewrite does: through a resolver built from its credentials. + fn inject_static_credential(request: &L7EvalContext, env_key: &str) { + let credentials = request.provider_credentials.as_ref().unwrap(); + let placeholder = credentials.snapshot().child_env[env_key].clone(); + let resolver = credentials + .resolver_for_endpoint("example.com", 80, "/") + .expect("unbound credentials resolve on any endpoint"); + assert!(resolver.resolve_placeholder(&placeholder).is_some()); + assert!( + credentials + .credential_uses() + .expect("http_context tracks credential use") + .static_versions + .contains_key(env_key), + "the injection must be recorded on the connection" + ); + } + + /// A connection that never had a credential injected cannot hold + /// a stale one, so it survives credential changes. + #[test] + fn http_relay_that_never_injected_survives_a_credential_change() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "a2")]); + + assert!( + !guard.is_stale(), + "a connection that never carried credentials cannot hold a stale one" + ); + let uses = request.provider_credentials.as_ref().unwrap().credential_uses(); + assert!(uses.expect("http_context tracks credential use").is_empty()); + } + + /// Rotating a credential the connection sent closes it; an + /// upstream session may hold the old credential. + #[test] + fn http_relay_closes_when_a_credential_it_injected_rotates() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + + inject_static_credential(&request, "API_TOKEN"); + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "a2")]); + + assert!(guard.is_stale(), "the upstream session may hold the old credential"); + } + + /// Rotating another provider's credential, or attaching a new + /// provider, leaves open a connection that never sent them. + #[test] + fn http_relay_survives_changes_to_credentials_it_did_not_inject() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + // Another provider rotates, then a third provider is attached. + change_provider_credentials( + &engine, + &credentials, + 2, + &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b2")], + ); + assert!(!guard.is_stale(), "OTHER_TOKEN was never sent on this connection"); + change_provider_credentials( + &engine, + &credentials, + 3, + &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b2"), ("NEW_TOKEN", "c")], + ); + assert!(!guard.is_stale()); + } + + /// A connection that first injects a credential after its provider + /// was attached survives unrelated reloads, and closes when that credential + /// rotates. + #[test] + fn http_relay_pins_credentials_it_first_injects_after_an_attach() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + + // A provider is attached while the connection is open, and the + // connection then injects its credential. + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "a")]); + assert!(!guard.is_stale()); + inject_static_credential(&request, "API_TOKEN"); + + // Unrelated reloads keep it open: it holds the current credential. + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + assert!(!guard.is_stale()); + + // Rotating that credential closes it. + change_provider_credentials(&engine, &credentials, 3, &[("API_TOKEN", "a2")]); + assert!(guard.is_stale()); + } + + /// A reload between authorization and relay setup refuses the + /// relay, whatever the reload changed. + #[test] + fn reload_between_authorization_and_pin_refuses_the_relay() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + + assert!(prepare_raw_relay(None, &engine, &decision).is_none()); + let request = request_context(); + assert!(prepare_http_relay(None, &engine, &decision, &request).is_none()); + } + + // --- HTTP passthrough with middleware across reloads --- + + /// Policy data for example.com:80 with the built-in regex middleware on + /// the host (`on_error` as given), plus `extra` policies. + fn regex_middleware_policy(on_error: &str, extra: &str) -> String { + format!( + r#" +network_middlewares: + redact: + middleware: openshell/regex + on_error: {on_error} + endpoints: + include: ["example.com"] +network_policies: + web: + name: web + endpoints: + - {{ host: example.com, port: 80 }} + binaries: + - {{ path: /usr/bin/curl }} +{extra}"# + ) + } + + const OTHER_POLICY: &str = r" other: + name: other + endpoints: + - { host: other.example.net, port: 443 } + binaries: + - { path: /usr/bin/curl } +"; + + async fn read_http_head(reader: &mut R) -> Vec { + use tokio::io::AsyncReadExt; + + let mut head = Vec::new(); + let mut byte = [0u8; 1]; + while !head.ends_with(b"\r\n\r\n") { + let n = reader.read(&mut byte).await.unwrap(); + assert!(n > 0, "peer closed before a complete HTTP head"); + head.push(byte[0]); + } + head + } + + /// Send one request on a kept-alive HTTP passthrough connection that runs + /// middleware, reload the policy to `reload_data`, and send a second. + /// Returns whether the second request reached upstream. + async fn second_passthrough_request_reaches_upstream(reload_data: &str) -> bool { + use openshell_supervisor_middleware::{ChainRunner, MiddlewareRegistry}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let initial = regex_middleware_policy("fail_closed", ""); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, &initial).unwrap()); + let registry = MiddlewareRegistry::connect_services( + openshell_supervisor_middleware_builtins::services(), + Vec::new(), + ) + .await + .expect("built-in registry"); + engine.set_middleware_runner_for_tests(ChainRunner::from_registry(registry)); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + let (mut app, mut relay_client) = tokio::io::duplex(8192); + let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192); + let relay_engine = Arc::clone(&engine); + let relay = tokio::spawn(async move { + crate::l7::relay::relay_passthrough_with_credentials( + &mut relay_client, + &mut relay_upstream, + &request, + &guard, + Some(relay_engine.as_ref()), + ) + .await + }); + let timeout = std::time::Duration::from_secs(1); + let request_bytes = b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n"; + + app.write_all(request_bytes).await.unwrap(); + tokio::time::timeout(timeout, read_http_head(&mut upstream)) + .await + .expect("first request should reach upstream"); + upstream + .write_all(b"HTTP/1.1 204 No Content\r\n\r\n") + .await + .unwrap(); + tokio::time::timeout(timeout, read_http_head(&mut app)) + .await + .expect("first response should reach the client"); + + engine.reload(POLICY_REGO, reload_data).unwrap(); + + app.write_all(request_bytes).await.unwrap(); + let mut second = [0u8; 1024]; + let forwarded = matches!( + tokio::time::timeout(std::time::Duration::from_millis(500), upstream.read(&mut second)) + .await, + Ok(Ok(n)) if n > 0 + ); + + drop(app); + drop(upstream); + let _ = tokio::time::timeout(timeout, relay).await; + forwarded + } + + /// A kept-alive passthrough connection that runs middleware + /// serves its next request after an unrelated reload. + #[tokio::test] + async fn passthrough_middleware_connection_serves_next_request_after_unrelated_reload() { + let reload = regex_middleware_policy("fail_closed", OTHER_POLICY); + + assert!( + second_passthrough_request_reaches_upstream(&reload).await, + "approving another host must not end the connection" + ); + } + + /// A changed middleware chain does not apply to a kept-alive + /// connection; its next request is dropped. + #[tokio::test] + async fn passthrough_middleware_connection_drops_next_request_after_its_chain_changes() { + let reload = regex_middleware_policy("fail_open", ""); + + assert!( + !second_passthrough_request_reaches_upstream(&reload).await, + "a changed chain must not apply to the old connection" + ); + } + + fn empty_provider_credentials() -> ProviderCredentialState { + ProviderCredentialState::from_child_env_snapshot(1, std::collections::HashMap::new()) + } + + // --- Policy reloads: endpoints, proto policies, middleware, coalescing --- + + /// [`CURL_EXAMPLE_POLICY`] with an endpoint inserted before example.com in + /// the same policy, which shifts example.com's endpoint index. + const CURL_EXAMPLE_AFTER_EARLIER_ENDPOINT_POLICY: &str = r" +network_policies: + web: + name: web + endpoints: + - { host: earlier.example.com, port: 80 } + - { host: example.com, port: 80 } + binaries: + - { path: /usr/bin/curl } +"; + + /// [`CURL_EXAMPLE_POLICY`] plus an endpoint for another host, numbered so + /// consecutive reloads differ. + fn curl_example_and_other_policy(n: usize) -> String { + format!( + r" +network_policies: + web: + name: web + endpoints: + - {{ host: example.com, port: 80 }} + binaries: + - {{ path: /usr/bin/curl }} + other: + name: other + endpoints: + - {{ host: other-{n}.example.net, port: 443 }} + binaries: + - {{ path: /usr/bin/curl }} +" + ) + } + + /// Middleware selecting `host`, for [`CURL_EXAMPLE_POLICY`]'s connection. + fn curl_example_with_middleware_for(host: &str) -> String { + format!( + r#" +network_middlewares: + scan: + middleware: test/scan + on_error: fail_closed + endpoints: + include: ["{host}"] +network_policies: + web: + name: web + endpoints: + - {{ host: example.com, port: 80 }} + binaries: + - {{ path: /usr/bin/curl }} +"# + ) + } + + /// The tunnel evaluator the CONNECT adapter pins for an inspected route. + fn inspected_relay_evaluator( + engine: &Arc, + decision: &EgressDecision, + request: &L7EvalContext, + ) -> Box { + let route = inspected_route(decision); + let context = prepare_http_relay(Some(&route), engine, decision, request).unwrap(); + let PreparedHttpPolicy::Inspect { evaluator, .. } = context.policy else { + panic!("an L7 route should use a tunnel evaluator"); + }; + evaluator + } + + /// A gateway-delivered policy: `endpoint` (example.com:80) for curl, plus + /// `other_hosts` in a second policy. Proto conversion stamps the hash of + /// the complete policy into every endpoint. + fn proto_policy_with( + endpoint: openshell_core::proto::NetworkEndpoint, + other_hosts: &[&str], + ) -> openshell_core::proto::SandboxPolicy { + use openshell_core::proto::{ + FilesystemPolicy, LandlockPolicy, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, + ProcessPolicy, SandboxPolicy, + }; + + let curl = || NetworkBinary { + path: "/usr/bin/curl".to_string(), + }; + let mut network_policies = std::collections::HashMap::new(); + network_policies.insert( + "web".to_string(), + NetworkPolicyRule { + name: "web".to_string(), + endpoints: vec![endpoint], + binaries: vec![curl()], + }, + ); + if !other_hosts.is_empty() { + network_policies.insert( + "other".to_string(), + NetworkPolicyRule { + name: "other".to_string(), + endpoints: other_hosts + .iter() + .map(|host| NetworkEndpoint { + host: (*host).to_string(), + port: 443, + ..Default::default() + }) + .collect(), + binaries: vec![curl()], + }, + ); + } + SandboxPolicy { + version: 1, + filesystem: Some(FilesystemPolicy { + include_workdir: true, + read_only: vec!["/usr".to_string(), "/lib".to_string()], + read_write: vec!["/sandbox".to_string(), "/tmp".to_string()], + }), + landlock: Some(LandlockPolicy { + compatibility: "best_effort".to_string(), + }), + process: Some(ProcessPolicy { + run_as_user: "sandbox".to_string(), + run_as_group: "sandbox".to_string(), + }), + network_policies, + network_middlewares: std::collections::HashMap::default(), + } + } + + /// example.com:80, at `path` (empty for any path), inspected as `protocol` + /// with one `allow` rule. + fn inspected_proto_endpoint( + protocol: &str, + path: &str, + allow: openshell_core::proto::L7Allow, + ) -> openshell_core::proto::NetworkEndpoint { + openshell_core::proto::NetworkEndpoint { + host: "example.com".to_string(), + port: 80, + path: path.to_string(), + protocol: protocol.to_string(), + enforcement: openshell_core::proto::NetworkEnforcementMode::Enforce as i32, + rules: vec![openshell_core::proto::L7Rule { allow: Some(allow) }], + ..Default::default() + } + } + + /// example.com:80 inspected as REST, allowing GET on `path`. + fn example_proto_policy( + path: &str, + other_hosts: &[&str], + ) -> openshell_core::proto::SandboxPolicy { + let allow = openshell_core::proto::L7Allow { + method: "GET".to_string(), + path: path.to_string(), + ..Default::default() + }; + proto_policy_with(inspected_proto_endpoint("rest", "", allow), other_hosts) + } + + /// example.com:80/mcp inspected as MCP, allowing the JSON-RPC `method`. + fn example_mcp_proto_policy( + method: &str, + other_hosts: &[&str], + ) -> openshell_core::proto::SandboxPolicy { + let allow = openshell_core::proto::L7Allow { + method: method.to_string(), + ..Default::default() + }; + proto_policy_with(inspected_proto_endpoint("mcp", "/mcp", allow), other_hosts) + } + + /// An endpoint inserted before the connection's own, in the same + /// policy, shifts its index but not its rules. + #[tokio::test] + async fn raw_relay_survives_an_endpoint_inserted_before_its_own() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| { + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AFTER_EARLIER_ENDPOINT_POLICY) + .unwrap(); + }, + std::time::Duration::from_millis(200), + ) + .await; + + assert!(!closed); + } + + /// In a gateway-delivered policy every reload restamps every + /// endpoint with the new policy hash. That stamp alone must not close an + /// inspected tunnel. + #[test] + fn inspected_relay_from_a_proto_policy_stays_current_when_another_policy_changes() { + let engine = Arc::new(OpaEngine::from_proto(&example_proto_policy("/**", &[])).unwrap()); + let decision = authorized_decision(&engine); + let request = request_context(); + let evaluator = inspected_relay_evaluator(&engine, &decision, &request); + + engine + .reload_from_proto(&example_proto_policy("/**", &["other.example.net"])) + .unwrap(); + + assert!(!evaluator.is_stale()); + } + + /// Narrowing the rules of a gateway-delivered endpoint closes + /// its inspected tunnel. + #[test] + fn inspected_relay_from_a_proto_policy_goes_stale_when_its_rules_change() { + let engine = Arc::new(OpaEngine::from_proto(&example_proto_policy("/**", &[])).unwrap()); + let decision = authorized_decision(&engine); + let request = request_context(); + let evaluator = inspected_relay_evaluator(&engine, &decision, &request); + + engine + .reload_from_proto(&example_proto_policy("/public/**", &[])) + .unwrap(); + + assert!(evaluator.is_stale()); + } + + /// Middleware selected for another host does not affect this + /// connection. + #[tokio::test] + async fn raw_relay_survives_middleware_added_for_another_host() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let data = curl_example_with_middleware_for("other.example.net"); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| engine.reload(POLICY_REGO, &data).unwrap(), + std::time::Duration::from_millis(200), + ) + .await; + + assert!(!closed); + } + + /// Middleware newly selected for this host closes the relay, + /// which cannot be inspected retroactively. + #[tokio::test] + async fn raw_relay_closes_when_middleware_is_added_for_its_host() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let data = curl_example_with_middleware_for("example.com"); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| engine.reload(POLICY_REGO, &data).unwrap(), + std::time::Duration::from_secs(1), + ) + .await; + + assert!(closed); + } + + /// Replacing the middleware registry cannot affect a connection + /// that runs no middleware. + #[tokio::test] + async fn raw_relay_without_middleware_survives_a_registry_replacement() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + + let closed = raw_relay_closes_after( + &engine, + &decision, + |engine| { + engine + .replace_middleware_registry( + openshell_supervisor_middleware::MiddlewareRegistry::default(), + ) + .unwrap(); + }, + std::time::Duration::from_millis(200), + ) + .await; + + assert!(!closed); + } + + /// Replacing the middleware registry closes a connection that + /// runs middleware, because its tunnel holds the old runner. A reload + /// lands before the connection checks, as reloads often coalesce. + #[test] + fn http_relay_with_middleware_goes_stale_when_the_registry_is_replaced() { + let data = curl_example_with_middleware_for("example.com"); + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, &data).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + engine + .replace_middleware_registry( + openshell_supervisor_middleware::MiddlewareRegistry::default(), + ) + .unwrap(); + engine.reload(POLICY_REGO, &data).unwrap(); + + assert!(guard.is_stale()); + } + + /// Several unrelated reloads can land before a connection next + /// checks. It re-pins through all of them. + #[test] + fn http_relay_stays_current_through_several_unchecked_unrelated_reloads() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + for n in 0..3 { + engine + .reload(POLICY_REGO, &curl_example_and_other_policy(n)) + .unwrap(); + } + + assert!(!guard.is_stale()); + assert_eq!(guard.captured_generation(), engine.current_generation()); + } + + /// A connection checked between reloads stays open however many + /// unrelated reloads there are. + #[test] + fn http_relay_checked_between_unrelated_reloads_stays_current_indefinitely() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + for n in 0..40 { + engine + .reload(POLICY_REGO, &curl_example_and_other_policy(n)) + .unwrap(); + assert!(!guard.is_stale(), "reload {n}"); + } + } + + /// A connection that sleeps through a very long run of reloads + /// closes, because it can no longer be checked against each of them. + #[test] + fn http_relay_goes_stale_after_a_long_run_of_unchecked_reloads() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + for n in 0..40 { + engine + .reload(POLICY_REGO, &curl_example_and_other_policy(n)) + .unwrap(); + } + + assert!(guard.is_stale()); + } + + /// A reload that revoked the connection closes it even if the + /// next reload restored access before the connection checked. + #[test] + fn http_relay_goes_stale_when_a_coalesced_reload_revoked_it() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &empty_provider_credentials()); + let guard = http_relay_guard(&engine, &decision, &request); + + engine.reload(POLICY_REGO, EMPTY_POLICY_DATA).unwrap(); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(guard.is_stale()); + } + + /// A reload that changed the connection's rules closes it even + /// if an unrelated reload followed before the connection checked. + #[test] + fn inspected_relay_goes_stale_when_a_coalesced_reload_changed_its_rules() { + let engine = + Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_REST_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = request_context(); + let evaluator = inspected_relay_evaluator(&engine, &decision, &request); + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_REST_NARROWED_POLICY) + .unwrap(); + engine + .reload(POLICY_REGO, CURL_EXAMPLE_REST_AND_OTHER_POLICY) + .unwrap(); + + assert!(evaluator.is_stale()); + } + + /// Once a connection has been found stale it stays stale, even + /// after its policy is restored. + #[test] + fn inspected_relay_stays_stale_after_its_policy_is_restored() { + let engine = + Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_REST_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let request = request_context(); + let evaluator = inspected_relay_evaluator(&engine, &decision, &request); + + engine + .reload(POLICY_REGO, CURL_EXAMPLE_REST_NARROWED_POLICY) + .unwrap(); + assert!(evaluator.is_stale()); + engine.reload(POLICY_REGO, CURL_EXAMPLE_REST_POLICY).unwrap(); + + assert!(evaluator.is_stale()); + } + + // --- Credential changes: install paths and placeholder generations --- + + /// `install_prepared`, the path runtime reloads use, rotates a + /// credential this connection never sent. + #[test] + fn http_relay_survives_install_prepared_rotating_a_credential_it_did_not_inject() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + credentials.install_prepared(&token_credentials( + 2, + &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b2")], + )); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(!guard.is_stale()); + } + + /// `install_prepared` rotating a credential this connection + /// sent closes the relay. + #[test] + fn http_relay_closes_when_install_prepared_rotates_a_credential_it_injected() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + credentials.install_prepared(&token_credentials(2, &[("API_TOKEN", "a2")])); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(guard.is_stale()); + } + + /// Reinstalling identical credentials changes nothing a + /// connection holds. + #[test] + fn http_relay_survives_a_reinstall_of_identical_credentials() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "a")]); + + assert!(!guard.is_stale()); + } + + /// Hiding a credential from future child processes does not + /// revoke it; the proxy still resolves it. + #[test] + fn http_relay_survives_suppression_of_a_credential_it_injected() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + credentials.remove_env_key("API_TOKEN"); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(!guard.is_stale()); + } + + /// A long-running process may still hold a placeholder from a + /// generation that has aged out; it falls back to the current value, so + /// the connection holds the current credential. + #[test] + fn http_relay_survives_after_an_aged_out_placeholder_falls_back_to_the_current_value() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let first_placeholder = credentials.snapshot().child_env["API_TOKEN"].clone(); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + + // More identical reinstalls than the retained generations. + for revision in 2..=10 { + change_provider_credentials(&engine, &credentials, revision, &[("API_TOKEN", "a")]); + assert!(!guard.is_stale()); + } + let resolver = request + .provider_credentials + .as_ref() + .unwrap() + .resolver_for_endpoint("example.com", 80, "/") + .unwrap(); + assert_eq!(resolver.resolve_placeholder(&first_placeholder), Some("a")); + engine + .reload(POLICY_REGO, CURL_EXAMPLE_AND_OTHER_POLICY) + .unwrap(); + + assert!(!guard.is_stale()); + } + + /// A connection that sent a credential closes when it rotates, + /// even if it rotated twice before the connection checked. + #[test] + fn http_relay_closes_after_coalesced_rotations_of_a_credential_it_injected() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "b")]); + change_provider_credentials(&engine, &credentials, 3, &[("API_TOKEN", "c")]); + + assert!(guard.is_stale()); + } + + /// Detaching the provider of a credential this connection sent + /// closes the relay. + #[test] + fn http_relay_closes_when_the_provider_of_a_credential_it_injected_is_detached() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_static_credential(&request, "API_TOKEN"); + + change_provider_credentials(&engine, &credentials, 2, &[("OTHER_TOKEN", "b")]); + + assert!(guard.is_stale()); + } + + // --- Endpoint status of a kept-alive MCP tunnel across reloads --- + + /// The endpoint config a connection to example.com:80/mcp selects under + /// `policy`, carrying the policy's stamped hash and the endpoint's id. + fn mcp_endpoint_config( + policy: &openshell_core::proto::SandboxPolicy, + ) -> crate::l7::L7EndpointConfig { + let engine = OpaEngine::from_proto(policy).unwrap(); + let decision = authorized_decision(&engine); + inspected_route(&decision).configs.remove(0).config + } + + /// Publish the endpoint-status inventory of the policy `config` came from, + /// as the supervisor does after installing a configuration. + async fn publish_mcp_inventory( + sender: &openshell_core::endpoint_status::EndpointObservationSender, + receiver: &mut openshell_core::endpoint_status::EndpointStatusReceiver, + config: &crate::l7::L7EndpointConfig, + ) { + use openshell_core::endpoint_status::{ + EndpointConfigVersion, EndpointInventoryEntry, EndpointStatusCommand, + }; + + sender + .reset( + EndpointConfigVersion { + policy_hash: config.policy_hash.clone(), + provider_env_revision: 1, + }, + vec![EndpointInventoryEntry { + endpoint_id: config.endpoint_id.clone(), + uses_provider_credentials: false, + }], + ) + .await + .expect("publish endpoint inventory"); + assert!(matches!( + receiver.recv().await, + Some(EndpointStatusCommand::Reset { .. }) + )); + } + + /// Send one MCP `initialize` on the connection; returns whether it reached + /// upstream, answering it there if so. + async fn mcp_exchange(app: &mut A, server: &mut S) -> bool + where + A: AsyncRead + AsyncWrite + Unpin, + S: AsyncRead + AsyncWrite + Unpin, + { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1"}}}"#; + let head = format!( + "POST /mcp HTTP/1.1\r\nHost: example.com\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n", + body.len() + ); + app.write_all(head.as_bytes()).await.unwrap(); + app.write_all(body).await.unwrap(); + + let received = tokio::time::timeout(std::time::Duration::from_millis(500), async { + let mut forwarded = Vec::new(); + while !forwarded.ends_with(body) { + let mut buffer = [0u8; 1024]; + let n = server.read(&mut buffer).await.ok()?; + if n == 0 { + return None; + } + forwarded.extend_from_slice(&buffer[..n]); + } + Some(()) + }) + .await; + if !matches!(received, Ok(Some(()))) { + return false; + } + server + .write_all(b"HTTP/1.1 204 No Content\r\n\r\n") + .await + .unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(1), read_http_head(app)) + .await + .expect("the response should reach the client"); + true + } + + /// The policy hash of the next MCP result reported, if one is reported. + async fn next_mcp_report( + receiver: &mut openshell_core::endpoint_status::EndpointStatusReceiver, + ) -> Option { + use openshell_core::endpoint_status::{EndpointResult, EndpointStatusCommand}; + + match tokio::time::timeout(std::time::Duration::from_millis(500), receiver.recv()).await { + Ok(Some(EndpointStatusCommand::Observe { + config_version, + result: EndpointResult::HttpResponseReceived, + .. + })) => Some(config_version.policy_hash), + _ => None, + } + } + + /// What a kept-alive MCP tunnel reported across a policy change. + struct McpStatusAcrossReload { + /// The policy hash the exchange before the change was reported against. + first_reported: Option, + /// Whether the exchange after the change reached upstream. + second_forwarded: bool, + /// The policy hash the exchange after the change was reported against. + second_reported: Option, + } + + /// Open an MCP tunnel under [`example_mcp_proto_policy`] with its endpoint + /// inventory published, complete one exchange, then reload the runtime to + /// `reload_to` (if any), publish the inventory of `publish_for` (if any), + /// and attempt a second exchange on the same connection. + async fn mcp_status_across_reload( + reload_to: Option, + publish_for: Option, + ) -> McpStatusAcrossReload { + let initial = example_mcp_proto_policy("initialize", &[]); + let engine = Arc::new(OpaEngine::from_proto(&initial).unwrap()); + let decision = authorized_decision(&engine); + let route = inspected_route(&decision); + let (sender, mut receiver) = openshell_core::endpoint_status::endpoint_status_channel(); + publish_mcp_inventory(&sender, &mut receiver, &route.configs[0].config).await; + let mut request = http_context( + &decision, + None, + None, + None, + openshell_core::proposals::AgentProposals::default(), + RelaySignals { + activity: None, + endpoint_observation: Some(sender.clone()), + }, + ); + request.request_default_port = Some(80); + let context = prepare_http_relay(Some(&route), &engine, &decision, &request).unwrap(); + + let (mut app, mut client) = tokio::io::duplex(8192); + let (mut upstream, mut server) = tokio::io::duplex(8192); + let relay = relay_http_stream(&mut client, &mut upstream, context); + let drive = async { + assert!(mcp_exchange(&mut app, &mut server).await, "first exchange"); + let first_reported = next_mcp_report(&mut receiver).await; + + if let Some(policy) = &reload_to { + engine.reload_from_proto(policy).unwrap(); + } + if let Some(policy) = &publish_for { + publish_mcp_inventory(&sender, &mut receiver, &mcp_endpoint_config(policy)).await; + } + + let second_forwarded = mcp_exchange(&mut app, &mut server).await; + let second_reported = next_mcp_report(&mut receiver).await; + drop(app); + drop(server); + McpStatusAcrossReload { + first_reported, + second_forwarded, + second_reported, + } + }; + let (_, outcome) = tokio::join!(relay, drive); + assert_eq!( + outcome.first_reported.as_deref(), + Some(route.configs[0].config.policy_hash.as_str()), + "the exchange before any change is reported against the policy in force" + ); + outcome + } + + /// After an unrelated reload, and once the supervisor publishes the new + /// policy's endpoint inventory, a kept-alive MCP tunnel keeps reporting its + /// exchanges, against the new policy. + #[tokio::test] + async fn mcp_tunnel_keeps_reporting_endpoint_status_after_an_unrelated_reload() { + let reloaded = example_mcp_proto_policy("initialize", &["other.example.net"]); + let reloaded_hash = mcp_endpoint_config(&reloaded).policy_hash; + + let outcome = mcp_status_across_reload(Some(reloaded.clone()), Some(reloaded)).await; + + assert!(outcome.second_forwarded, "the tunnel must stay open"); + assert_eq!( + outcome.second_reported, + Some(reloaded_hash), + "the exchange must be reported against the policy now in force" + ); + } + + /// Until the supervisor publishes the reloaded policy's + /// inventory, results cannot be attributed to the inventory of the policy + /// that was replaced, so the exchange after the reload is not reported. + #[tokio::test] + async fn mcp_tunnel_reports_nothing_until_the_reloaded_policy_inventory_is_published() { + let reloaded = example_mcp_proto_policy("initialize", &["other.example.net"]); + + let outcome = mcp_status_across_reload(Some(reloaded), None).await; + + assert_eq!(outcome.second_reported, None); + } + + /// An inventory published ahead of the runtime, for a policy + /// the tunnel is not running under, receives none of its results. + #[tokio::test] + async fn mcp_tunnel_reports_nothing_against_an_inventory_published_ahead_of_its_policy() { + let upcoming = example_mcp_proto_policy("initialize", &["other.example.net"]); + + let outcome = mcp_status_across_reload(None, Some(upcoming)).await; + + assert!(outcome.second_forwarded, "nothing changed for the running tunnel"); + assert_eq!(outcome.second_reported, None); + } + + /// A reload that changes the MCP endpoint's rules closes the + /// tunnel, so nothing more is exchanged or reported on it. + #[tokio::test] + async fn mcp_tunnel_closes_and_reports_nothing_when_its_rules_change() { + let reloaded = example_mcp_proto_policy("tools/list", &[]); + + let outcome = mcp_status_across_reload(Some(reloaded.clone()), Some(reloaded)).await; + + assert!(!outcome.second_forwarded); + assert_eq!(outcome.second_reported, None); + } + + // --- Credentials injected through a stable-handle placeholder --- + + /// `API_KEY` bound to example.com:80 with a stable workload handle, so its + /// placeholder survives rotations, plus an unbound `OTHER_TOKEN`. + fn stable_handle_bindings() + -> std::collections::HashMap { + std::collections::HashMap::from([( + "API_KEY".to_string(), + openshell_core::proto::StaticCredentialBinding { + endpoints: vec![openshell_core::proto::StaticCredentialEndpointBinding { + host: "example.com".to_string(), + port: 80, + path: "/**".to_string(), + }], + credential_identity: "provider:API_KEY".to_string(), + workload_credential_handle: "a".repeat(64), + }, + )]) + } + + fn stable_handle_tokens( + api_key: &str, + other: &str, + ) -> std::collections::HashMap { + std::collections::HashMap::from([ + ("API_KEY".to_string(), api_key.to_string()), + ("OTHER_TOKEN".to_string(), other.to_string()), + ]) + } + + fn stable_handle_credentials(api_key: &str, other: &str) -> ProviderCredentialState { + ProviderCredentialState::from_bound_environment_with_stable_placeholders( + 1, + stable_handle_tokens(api_key, other), + std::collections::HashMap::new(), + std::collections::HashMap::new(), + stable_handle_bindings(), + vec![], + vec!["API_KEY".to_string()], + ) + .expect("valid stable-handle binding") + } + + /// Install new values the way a runtime reload does, keeping the handle. + fn reinstall_stable_handle_credentials( + engine: &OpaEngine, + credentials: &ProviderCredentialState, + revision: u64, + api_key: &str, + other: &str, + ) { + credentials + .install_bound_environment_with_stable_placeholders( + revision, + stable_handle_tokens(api_key, other), + std::collections::HashMap::new(), + std::collections::HashMap::new(), + stable_handle_bindings(), + vec![], + vec!["API_KEY".to_string()], + ) + .expect("valid stable-handle binding"); + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + } + + /// Send `API_KEY` on the connection with context `request` through its + /// stable-handle placeholder, as an Authorization header rewrite does. + fn inject_through_stable_handle(request: &L7EvalContext) { + let credentials = request.provider_credentials.as_ref().unwrap(); + let placeholder = credentials.snapshot().child_env["API_KEY"].clone(); + assert!( + placeholder.starts_with("openshell:resolve:env:s"), + "fixture must issue a stable-handle placeholder, got {placeholder}" + ); + let resolver = credentials + .resolver_for_endpoint("example.com", 80, "/") + .expect("bound credential resolves on its endpoint"); + let rewritten = resolver + .rewrite_header_value_for_name("Authorization", &placeholder) + .expect("stable-handle placeholder resolves"); + assert!(rewritten.is_some()); + } + + /// A connection that sent a credential through its stable-handle + /// placeholder survives a rotation of another credential, and keeps its + /// placeholder. + #[test] + fn http_relay_that_injected_through_a_stable_handle_survives_an_unrelated_rotation() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = stable_handle_credentials("key-1", "other-1"); + let placeholder = credentials.snapshot().child_env["API_KEY"].clone(); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_through_stable_handle(&request); + + reinstall_stable_handle_credentials(&engine, &credentials, 2, "key-1", "other-2"); + + assert_eq!(credentials.snapshot().child_env["API_KEY"], placeholder); + assert!(!guard.is_stale()); + } + + /// A connection that sent a credential through its stable-handle + /// placeholder closes when the value behind the handle rotates, even + /// though the placeholder itself is unchanged. + #[test] + fn http_relay_closes_when_the_credential_behind_its_stable_handle_rotates() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = stable_handle_credentials("key-1", "other-1"); + let placeholder = credentials.snapshot().child_env["API_KEY"].clone(); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + inject_through_stable_handle(&request); + + reinstall_stable_handle_credentials(&engine, &credentials, 2, "key-2", "other-1"); + + assert_eq!(credentials.snapshot().child_env["API_KEY"], placeholder); + assert!(guard.is_stale()); + } + + // --- Credentials injected through other placeholder forms --- + + /// A connection that sent a credential through its canonical + /// placeholder (`openshell:resolve:env:KEY`, the documented form) sent the + /// current value, so it survives a rotation of another credential. + #[test] + fn http_relay_that_injected_through_a_canonical_placeholder_survives_an_unrelated_rotation() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let decision = authorized_decision(&engine); + let credentials = token_credentials(1, &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + let resolver = request + .provider_credentials + .as_ref() + .unwrap() + .resolver_for_endpoint("example.com", 80, "/") + .expect("unbound credentials resolve on any endpoint"); + assert_eq!( + resolver.resolve_placeholder(&openshell_core::secrets::placeholder_for_env_key( + "API_TOKEN" + )), + Some("a") + ); + + change_provider_credentials( + &engine, + &credentials, + 2, + &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b2")], + ); + + assert!(!guard.is_stale()); + } + + /// A connection that sent a credential through a placeholder + /// from an older, still-retained revision sent a value that is no longer + /// current, so it closes on the next reload. + #[test] + fn http_relay_closes_after_injecting_through_an_older_retained_revision() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let credentials = token_credentials(1, &[("API_TOKEN", "a")]); + let old_placeholder = credentials.snapshot().child_env["API_TOKEN"].clone(); + change_provider_credentials(&engine, &credentials, 2, &[("API_TOKEN", "a2")]); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + let resolver = request + .provider_credentials + .as_ref() + .unwrap() + .resolver_for_endpoint("example.com", 80, "/") + .expect("unbound credentials resolve on any endpoint"); + assert_eq!(resolver.resolve_placeholder(&old_placeholder), Some("a")); + + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(guard.is_stale()); + } + + /// Provider revisions are sandbox-wide, so rotating another + /// credential reissues every placeholder at the new revision. A process + /// still holding the previous revision's placeholder for an unchanged + /// credential sends the current value, so its connection survives the next + /// reload. + #[test] + fn http_relay_survives_after_injecting_an_unchanged_credential_through_an_older_revision() { + let engine = Arc::new(OpaEngine::from_strings(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap()); + let credentials = token_credentials(1, &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b")]); + let old_placeholder = credentials.snapshot().child_env["API_TOKEN"].clone(); + change_provider_credentials( + &engine, + &credentials, + 2, + &[("API_TOKEN", "a"), ("OTHER_TOKEN", "b2")], + ); + assert_ne!(credentials.snapshot().child_env["API_TOKEN"], old_placeholder); + let decision = authorized_decision(&engine); + let request = connection_context(&decision, &credentials); + let guard = http_relay_guard(&engine, &decision, &request); + let resolver = request + .provider_credentials + .as_ref() + .unwrap() + .resolver_for_endpoint("example.com", 80, "/") + .expect("unbound credentials resolve on any endpoint"); + assert_eq!(resolver.resolve_placeholder(&old_placeholder), Some("a")); + + engine.reload(POLICY_REGO, CURL_EXAMPLE_POLICY).unwrap(); + + assert!(!guard.is_stale()); + } } diff --git a/crates/openshell-supervisor/src/config_runtime.rs b/crates/openshell-supervisor/src/config_runtime.rs index 02736ad184..b5fe00d594 100644 --- a/crates/openshell-supervisor/src/config_runtime.rs +++ b/crates/openshell-supervisor/src/config_runtime.rs @@ -797,6 +797,7 @@ impl ConfigRuntime { reason: ProviderReadinessReason, revision: u64, ) { + let before = ctx.provider_credentials.installed_credential_versions(); ctx.provider_readiness.credentials_failed(identity, reason); ocsf_emit!(ConfigStateChangeBuilder::new(ocsf_ctx()) .severity(SeverityId::High) @@ -806,6 +807,7 @@ impl ConfigRuntime { .build()); ctx.provider_credentials .revoke_static_provider_environment(revision); + publish_credential_change(ctx, &before); endpoint_status::reset( ctx.endpoint_observation_tx.as_ref(), self.endpoint_policy.as_ref(), @@ -831,6 +833,7 @@ impl ConfigRuntime { .build()); ctx.provider_readiness .credentials_failed(identity, ProviderReadinessReason::CredentialInstallFailed); + let before = ctx.provider_credentials.installed_credential_versions(); let revision = provider.provider_env_revision; // Repeat the rejected binding validation on the live state to // revoke static material and retain the fetched dynamic grants. @@ -845,6 +848,7 @@ impl ConfigRuntime { provider.non_secret_environment_keys, provider.stable_placeholder_environment_keys, ); + publish_credential_change(ctx, &before); endpoint_status::reset( ctx.endpoint_observation_tx.as_ref(), self.endpoint_policy.as_ref(), @@ -1181,6 +1185,30 @@ pub async fn run_policy_poll_loop(ctx: PolicyPollLoopContext) -> Result<()> { run_policy_poll_loop_with_client(ctx, client).await } +/// Provider credentials may have changed outside a policy reload. If any did +/// since `before`, publish a generation of the unchanged policy, so open +/// connections re-check the credentials they sent and those that sent a +/// changed one close. A revocation that finds nothing left to revoke, as when +/// a failed refresh repeats on every poll, publishes nothing. +fn publish_credential_change( + ctx: &PolicyPollLoopContext, + before: &openshell_core::provider_credentials::InstalledCredentialVersions, +) { + if ctx + .provider_credentials + .installed_credential_versions() + .same_credentials(before) + { + return; + } + if let Err(error) = ctx.opa_engine.publish_credential_change() { + warn!( + error = %error, + "Provider credentials changed but open connections were not notified" + ); + } +} + async fn report_runtime_configuration( ctx: &PolicyPollLoopContext, snapshot: &openshell_core::grpc_client::SettingsPollResult, @@ -4201,4 +4229,141 @@ mod tests { assert!(runtime.middleware_services.is_empty()); assert!(ctx.extension_credentials.get("removed-guard").is_none()); } + + /// Revoking provider credentials publishes a policy generation, + /// so open connections wake and re-check the credentials they sent. Covers + /// each revocation path: an unavailable environment, bindings that fail + /// validation, and a snapshot for another revision. Successful installs + /// publish one too; see `stream_provider_rotation_rebinds_policy_generation`. + #[tokio::test] + async fn provider_credential_revocation_publishes_a_policy_generation() { + use openshell_core::proto::{ConfigApplyOutcome, PolicySource}; + + let initial = settings_poll_result(Some(proto_policy_fixture()), 1, PolicySource::Sandbox); + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + let mut ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: Some(LoadedPolicyRevision::from_snapshot(&initial)), + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + let (client, _polls, _reports) = scripted_policy_gateway(); + let mut runtime = started_runtime(&ctx, &initial); + for (revision, snapshot_revision, readiness_reason, classification, expected_code) in [ + ( + 2, + 2, + ProviderReadinessReason::UnsupportedSupervisor, + openshell_core::proto::ProviderEnvironmentValueClassification::NonSecret, + "provider_environment_unavailable", + ), + ( + 3, + 3, + ProviderReadinessReason::Unspecified, + openshell_core::proto::ProviderEnvironmentValueClassification::Unspecified, + "invalid_provider_environment", + ), + ( + 4, + 5, + ProviderReadinessReason::Unspecified, + openshell_core::proto::ProviderEnvironmentValueClassification::NonSecret, + "provider_environment_mismatch", + ), + ] { + ctx.provider_credentials = initial_provider_credentials( + static_provider_environment(1, Some("initial")), + &ctx.provider_readiness, + ); + let mut desired = initial.clone(); + desired.provider_env_revision = revision; + let mut snapshot = provider_snapshot_for(&desired, "REGION", "west"); + snapshot.provider_env_revision = snapshot_revision; + snapshot.readiness_reason = readiness_reason.into(); + snapshot.values[0].classification = classification.into(); + let before = ctx.opa_engine.current_generation(); + + let result = runtime + .apply_delivered_sandbox(&ctx, &client, desired, snapshot) + .await; + + assert_eq!( + result.outcome, + ConfigApplyOutcome::FailedRetainedLastKnownGood + ); + assert_eq!(result.failure.as_ref().unwrap().code, expected_code); + assert!( + ctx.provider_credentials.resolver().is_none(), + "{expected_code}: static credentials must be revoked" + ); + assert!( + ctx.opa_engine.current_generation() > before, + "{expected_code}: revocation must publish a generation" + ); + } + } + + /// A revocation that finds no credential left to revoke, as + /// when the same failed provider refresh repeats on every poll, publishes + /// no generation, so open connections are not re-checked each time. + #[tokio::test] + async fn repeated_provider_credential_revocation_publishes_no_further_generation() { + use openshell_core::proto::{ConfigApplyOutcome, PolicySource}; + + let initial = settings_poll_result(Some(proto_policy_fixture()), 1, PolicySource::Sandbox); + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + let mut ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: Some(LoadedPolicyRevision::from_snapshot(&initial)), + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + ctx.provider_credentials = initial_provider_credentials( + static_provider_environment(1, Some("initial")), + &ctx.provider_readiness, + ); + let (client, _polls, _reports) = scripted_policy_gateway(); + let mut runtime = started_runtime(&ctx, &initial); + let unavailable = |revision| { + let mut desired = initial.clone(); + desired.provider_env_revision = revision; + let mut snapshot = provider_snapshot_for(&desired, "REGION", "west"); + snapshot.readiness_reason = ProviderReadinessReason::UnsupportedSupervisor.into(); + (desired, snapshot) + }; + let (desired, snapshot) = unavailable(2); + let first = runtime + .apply_delivered_sandbox(&ctx, &client, desired, snapshot) + .await; + assert_eq!( + first.outcome, + ConfigApplyOutcome::FailedRetainedLastKnownGood + ); + assert!(ctx.provider_credentials.resolver().is_none()); + let after_first = ctx.opa_engine.current_generation(); + + for revision in [3, 4] { + let (desired, snapshot) = unavailable(revision); + let result = runtime + .apply_delivered_sandbox(&ctx, &client, desired, snapshot) + .await; + + assert_eq!( + result.outcome, + ConfigApplyOutcome::FailedRetainedLastKnownGood + ); + assert_eq!( + result.failure.as_ref().unwrap().code, + "provider_environment_unavailable" + ); + assert_eq!(ctx.opa_engine.current_generation(), after_first); + } + } } diff --git a/docs/how-it-works/policies/manage-policies.mdx b/docs/how-it-works/policies/manage-policies.mdx index c20af7b67b..6ebdf0f350 100644 --- a/docs/how-it-works/policies/manage-policies.mdx +++ b/docs/how-it-works/policies/manage-policies.mdx @@ -188,17 +188,34 @@ running sandbox. The startup sections do not: | Change | Effect on a running sandbox | Required action | |---|---|---| -| Network rules | The sandbox loads the new rules. Connections opened under the previous rules close. | Apply the change, then retry the request. | +| Network rules | The sandbox loads the new rules. Open connections whose rules changed close; other connections stay open. | Apply the change, then retry the request. | | Middleware in `network_middlewares` | You can add, remove, or reconfigure built-in middleware and external services already registered with the gateway. | Apply the change with `openshell policy set`. | | External middleware registration | Policy changes cannot register a service or change its gateway connection settings. | Update the gateway configuration and restart the gateway. | | Added filesystem paths | The saved policy can change, but the running workload keeps its existing filesystem permissions. | Recreate the sandbox. | | Removed filesystem paths, or changed `include_workdir`, Landlock, or process settings | OpenShell rejects the change after the workload starts. | Recreate the sandbox. | -When new network rules take effect, OpenShell closes connections that were -opened under the previous rules, including HTTP keep-alive connections, tunnels, -WebSocket connections, and long-lived streams. Clients must reconnect, and their +When new network rules take effect, OpenShell re-checks every open connection, +including HTTP keep-alive connections, tunnels, WebSocket connections, and +long-lived streams, against the new rules. A connection stays open only if the +new rules treat it exactly as the old ones did: the same allow decision, the +same matching policies and endpoints (including their L7 rules), and the same +middleware. Otherwise OpenShell closes it, the client must reconnect, and its next requests are checked against the new rules. +For example, approving an endpoint for one host leaves connections to other +hosts open, while changing the rules for a host closes that host's +connections. Some changes always close connections: + +- A fail-closed quarantine after an invalid policy, even if the previous policy + is restored before the connection is checked. +- A change to the middleware services registered with the gateway, for + connections that run middleware. +- A change to a provider credential that a connection has already had + injected, such as rotating it, detaching its provider, or the sandbox + revoking it after a failed provider refresh. Connections that never carried + that credential stay open, including when other providers are attached, + detached, or rotated. + OpenShell validates every change twice before it takes effect: ```mermaid @@ -207,7 +224,7 @@ flowchart TD B -->|Invalid| C["Change rejected
Current policy stays active"] B -->|Valid| D["Gateway saves a new revision"] D --> E{"Sandbox validates
and loads the revision"} - E -->|Loads| F["New rules active
Old connections close"] + E -->|Loads| F["New rules active
Affected connections close"] E -->|Fails| G["Failure mode applies
Block traffic or keep the last valid policy"] ``` diff --git a/docs/how-it-works/policies/network-rules.mdx b/docs/how-it-works/policies/network-rules.mdx index d7dec792a6..131efcecae 100644 --- a/docs/how-it-works/policies/network-rules.mdx +++ b/docs/how-it-works/policies/network-rules.mdx @@ -452,9 +452,12 @@ Set `allow_uninspected_credentials: true` to relay binary frames anyway. Set placeholders that must be resolved. This explicitly authorizes WebSocket text rewriting for ordinary static credentials, including credentials with HTTP placement metadata. -When new network rules take effect, OpenShell closes open WebSocket connections, -so the client must reconnect. The client might receive close code `1012`, or the -connection might close without a close frame. +When new network rules change how a WebSocket connection is treated, OpenShell +closes it, so the client must reconnect. The client might receive close code +`1012`, or the connection might close without a close frame. Connections the +change does not affect stay open; see +[How changes take effect](/how-it-works/policies/manage-policies#how-changes-take-effect) +for which changes close a connection. ### Allow GraphQL Operations