From 448580bf21ea91e0435278de629d8e7160e86edf Mon Sep 17 00:00:00 2001 From: Miles Date: Sat, 7 Nov 2026 15:34:06 +0000 Subject: [PATCH 1/4] test(network): characterise which connections policy reloads should close Every policy reload advances one sandbox-wide generation, and every pinned relay closes when it moves, so approving an endpoint for one host drops every open proxied connection in the sandbox. Add tests, against the existing relay and engine entry points, for how established connections should respond to reloads. Each test's doc comment says whether it describes behaviour the following change alters ("Changed", failing on this commit) or behaviour it must keep ("Unchanged", passing before and after). Changed: connections stay open when a reload does not affect them: - raw TCP, HTTP passthrough and inspected (L7) relays, when an endpoint is approved for another host, an endpoint is inserted before theirs, or a gateway-delivered policy is restamped with a new policy hash; - relays without middleware, when middleware is added for another host or the middleware registry is replaced; a kept-alive passthrough connection that runs middleware, on its next request after an unrelated reload; - relays that sleep through several unrelated reloads, or are checked between any number of them; - HTTP relays, across provider credential changes they did not take part in: credentials never injected on the connection, other providers' rotations and attachments (including through install_prepared), identical reinstalls, suppression of a credential from child processes, and a credential first injected after its provider was attached or served through an aged-out placeholder, and a credential sent through its canonical or stable-handle placeholder, or through the previous revision's placeholder while unchanged, when another credential rotates; - a kept-alive MCP tunnel keeps reporting endpoint status after an unrelated reload, against the new policy once its inventory is published. Unchanged: connections still close when a reload does affect them: revoked endpoints, narrowed L7 rules (YAML and gateway-delivered), middleware added for their host, changed or made fail-closed, a replaced registry for connections that run middleware, reloads that revoked or changed them even if a later reload restored them, a long run of reloads they slept through, rotated or detached credentials they injected (including rotations that coalesced, and a rotation behind an unchanged stable-handle placeholder), a superseded value sent through an older retained revision, untracked credential handles, a reload between authorization and relay setup, and plain generation guards. Endpoint-status results of a kept-alive MCP tunnel are never attributed to an inventory of a policy it is not running under: not before the reloaded policy's inventory is published, not to an inventory published ahead of the runtime, and not after a rules change, which closes the tunnel. One test records that every reload restamps policy_hash on unchanged endpoints. Changed, in the supervisor: revoking provider credentials (an unavailable environment, bindings that fail validation, or a snapshot for another revision) publishes a policy generation, so open connections re-check the credentials they sent. Today revocation publishes nothing, and a connection authenticated once with a revoked credential, such as an upgraded WebSocket, stays open. Unchanged: a revocation that finds nothing left to revoke, as when a failed refresh repeats on every poll, publishes no generation. Existing relay tests use Arc, which the change requires. Co-Authored-By: Claude Opus 5.5 --- .../openshell-supervisor-network/src/opa.rs | 111 ++ .../src/proxy/relay.rs | 1717 ++++++++++++++++- .../src/config_runtime.rs | 140 ++ 3 files changed, 1962 insertions(+), 6 deletions(-) diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 257bd3ce2e..485d4407e0 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -12591,4 +12591,115 @@ 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 + } + + /// Unchanged: 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" + ); + } + + /// Unchanged: 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()); + } + + /// Unchanged: 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) + ); + } } diff --git a/crates/openshell-supervisor-network/src/proxy/relay.rs b/crates/openshell-supervisor-network/src/proxy/relay.rs index 495c22241b..505f683463 100644 --- a/crates/openshell-supervisor-network/src/proxy/relay.rs +++ b/crates/openshell-supervisor-network/src/proxy/relay.rs @@ -362,6 +362,7 @@ fn emit_stale_relay_close(request: &L7EvalContext, guard: &PolicyGenerationGuard mod tests { use super::super::{EgressIntent, EndpointDecision, ProcessIdentityEvidence}; use super::*; + use openshell_core::provider_credentials::ProviderCredentialState; const POLICY_REGO: &str = include_str!("../../data/sandbox-policy.rego"); const EMPTY_POLICY_DATA: &str = "network_policies: {}\n"; @@ -412,7 +413,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 +502,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 +520,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 +536,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 +574,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 +589,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 +625,1708 @@ 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 input = crate::opa::NetworkInput { + host: "example.com".to_string(), + port: 80, + binary_path: std::path::PathBuf::from("/usr/bin/curl"), + binary_sha256: String::new(), + ancestors: vec![], + cmdline_paths: vec![], + }; + let authorization = engine.authorize_egress(&input).unwrap(); + assert!( + matches!(authorization.action, NetworkAction::Allow { .. }), + "fixture must authorize example.com:80" + ); + EgressDecision { + intent: EgressIntent::connect("example.com".to_string(), 80), + action: authorization.action.clone(), + policy_generation: authorization.generation, + identity: ProcessIdentityEvidence::Available, + endpoint: EndpointDecision::from_authorization(&authorization), + binary: Some(input.binary_path), + binary_pid: None, + ancestors: vec![], + cmdline_paths: vec![], + } + } + + /// 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 + } + + /// Changed: approving an endpoint for another host leaves a raw relay open. + /// Today it closes. + #[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"); + } + + /// Changed: a raw relay keeps moving bytes after an unrelated reload, and + /// still closes when its own endpoint is revoked. Today the reload closes it. + #[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"); + } + + /// Unchanged: 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"); + } + + /// Unchanged: 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" + ); + } + + /// Changed: approving an endpoint for another host leaves an HTTP + /// passthrough relay current, re-pinned to the new generation. Today it goes stale. + #[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()); + } + + /// Changed: approving an endpoint for another host leaves an inspected (L7) + /// tunnel current. Today it goes stale. + #[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" + ); + } + + /// Unchanged: 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"); + } + + /// Unchanged: 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()); + } + + /// Changed: a connection that never had a credential injected cannot hold + /// a stale one, so it survives credential changes. Today it closes. + #[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" + ); + } + + /// Unchanged: 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"); + } + + /// Changed: rotating another provider's credential, or attaching a new + /// provider, leaves open a connection that never sent them. Today it closes. + #[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()); + } + + /// Changed: a connection that first injects a credential after its provider + /// was attached survives unrelated reloads, and closes when that credential + /// rotates. Today the attach closes it. + #[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()); + } + + /// Unchanged: 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 + } + + /// Changed: a kept-alive passthrough connection that runs middleware + /// serves its next request after an unrelated reload. Today it drops it. + #[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" + ); + } + + /// Unchanged: 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) + } + + /// Changed: an endpoint inserted before the connection's own, in the same + /// policy, shifts its index but not its rules. Today the relay closes. + #[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); + } + + /// Changed: in a gateway-delivered policy every reload restamps every + /// endpoint with the new policy hash. That stamp alone must not close an + /// inspected tunnel. Today it closes. + #[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()); + } + + /// Unchanged: 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()); + } + + /// Changed: middleware selected for another host does not affect this + /// connection. Today the relay closes. + #[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); + } + + /// Unchanged: 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); + } + + /// Changed: replacing the middleware registry cannot affect a connection + /// that runs no middleware. Today the relay closes. + #[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); + } + + /// Unchanged: 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()); + } + + /// Changed: several unrelated reloads can land before a connection next + /// checks. It re-pins through all of them. Today it goes stale. + #[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()); + } + + /// Changed: a connection checked between reloads stays open however many + /// unrelated reloads there are. Today it goes stale at the first. + #[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}"); + } + } + + /// Unchanged: 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()); + } + + /// Unchanged: 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()); + } + + /// Unchanged: 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()); + } + + /// Unchanged: 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 --- + + /// Changed: `install_prepared`, the path runtime reloads use, rotates a + /// credential this connection never sent. Today the relay closes. + #[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()); + } + + /// Unchanged: `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()); + } + + /// Changed: reinstalling identical credentials changes nothing a + /// connection holds. Today the relay closes. + #[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()); + } + + /// Changed: hiding a credential from future child processes does not + /// revoke it; the proxy still resolves it. Today the relay closes. + #[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()); + } + + /// Changed: 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. Today the relay closes. + #[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()); + } + + /// Unchanged: 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()); + } + + /// Unchanged: 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 + } + + /// Changed: 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. Today the reload + /// closes the tunnel, so nothing more is exchanged or reported. + #[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" + ); + } + + /// Unchanged: 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); + } + + /// Unchanged: 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); + } + + /// Unchanged: 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()); + } + + /// Changed: a connection that sent a credential through its stable-handle + /// placeholder survives a rotation of another credential, and keeps its + /// placeholder. Today the relay closes. + #[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()); + } + + /// Unchanged: 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 --- + + /// Changed: 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. Today + /// the relay closes. + #[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()); + } + + /// Unchanged: 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()); + } + + /// Changed: 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. Today the relay closes. + #[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..4bc22252b0 100644 --- a/crates/openshell-supervisor/src/config_runtime.rs +++ b/crates/openshell-supervisor/src/config_runtime.rs @@ -4201,4 +4201,144 @@ mod tests { assert!(runtime.middleware_services.is_empty()); assert!(ctx.extension_credentials.get("removed-guard").is_none()); } + + /// Changed: 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. Today credentials are + /// revoked without a new generation, so a connection that authenticated + /// once with a revoked credential, such as an upgraded WebSocket, stays + /// open. (Successful installs already publish one; 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" + ); + } + } + + /// Unchanged: 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); + } + } } From 96eb09b28b45e3be8de73940c00959ff548e14f2 Mon Sep 17 00:00:00 2001 From: Miles Date: Sat, 7 Nov 2026 15:34:06 +0000 Subject: [PATCH 2/4] fix(network): keep connections open across policy reloads that do not affect them Every policy reload, middleware registry change and provider credential change advanced one sandbox-wide generation, and every pinned relay closed when it moved. Approving an endpoint for one host dropped every open proxied connection in the sandbox. When the generation moves, a pinned guard now re-checks the connection it protects and closes only if the answer changed. Policy: a ConnectionPolicyContract captures what decides how a connection is relayed: the L4 action and matched policy, the ordered matched endpoints (policy name and endpoint object, ignoring the endpoint index and the per-reload policy_hash stamp), the exact-host flag, and the host's middleware chain plus the registry epoch when that chain is non-empty. Endpoint configs, which relays also read, are not compared separately: Rego derives them from the same endpoints, in the same order. The engine records its last 16 published generations, and a waking connection is re-checked against every generation since its pin, because the generation watch coalesces reloads. If every contract equals the one that authorized it, the guard re-pins; any difference, evaluation error, fail-closed generation, or pin that has left the history makes the guard permanently stale and the relay closes as before. Per-request Rego rules read only the matched endpoints, so a tunnel's cloned evaluator stays valid after a re-pin. Credentials: the provider credential state owns one version per credential (static environment key or token-grant provider:credential identity), recomputed lazily by comparing content whenever its snapshot changes. Each proxied connection gets a tracked handle and records the version of every credential it injects, at the two points credential material leaves the supervisor: SecretResolver handing out a secret, and inject_or_report adding a grant. HTTP-family relays close only when a credential they injected changes or is removed, or when they sent a value that was already superseded. A use counts as current when the value it handed out is its key's current value, whichever placeholder form named it: provider revisions are sandbox-wide, so an older revision's placeholder usually still names the current value. Untracked handles and unattributable uses close on the next check. Raw TCP and TLS passthrough relays never inject credentials and carry no pin. Endpoint status: a tunnel's endpoint config carries the hash of the policy it was selected from, so an MCP tunnel that re-pinned would have stopped reporting endpoint status. The revalidator now records the stamped policy hash of the generation its guard is pinned to, from authorization through every re-pin, and observations (and token-grant reports) use it. The inventory check is unchanged, so results are still dropped while the inventory describes a different policy. Revocation: the supervisor revokes static provider credentials when a provider refresh fails, its bindings fail validation, or it delivers the wrong revision, but it published no generation, so connections that had sent a revoked credential were never checked. Both revocation paths now call OpaEngine::publish_credential_change, which publishes a generation of the unchanged policy (keeping any quarantine), when the revocation changed any credential, judged by comparing ProviderCredentialState::installed_credential_versions before and after. A failed refresh that repeats on every poll therefore wakes connections once, not every time. Connections that sent the revoked credential close; the rest re-pin. A reload between authorization and relay setup still refuses the relay. OpaEngine::revalidating_generation_guard is the single entry point for pinning a revalidating guard, and it and with_revalidator are crate-private, so every guard another crate obtains from the engine is plain: checking it is a cheap comparison that never evaluates policy or takes the engine lock. The HTTP passthrough middleware path lets the guard re-pin before comparing raw generation numbers. The policy docs now describe which changes close a connection. Co-Authored-By: Claude Opus 5.5 --- .../src/provider_credentials.rs | 350 +++++++++++++- crates/openshell-core/src/secrets.rs | 184 ++++++- .../src/l7/mod.rs | 19 +- .../src/l7/relay.rs | 9 +- .../src/l7/token_grant_injection.rs | 17 + .../openshell-supervisor-network/src/opa.rs | 338 +++++++++++-- .../src/opa/revalidation.rs | 452 ++++++++++++++++++ .../openshell-supervisor-network/src/proxy.rs | 48 +- .../src/proxy/egress.rs | 19 + .../src/proxy/relay.rs | 71 ++- .../src/config_runtime.rs | 28 ++ .../how-it-works/policies/manage-policies.mdx | 27 +- docs/how-it-works/policies/network-rules.mdx | 9 +- 13 files changed, 1467 insertions(+), 104 deletions(-) create mode 100644 crates/openshell-supervisor-network/src/opa/revalidation.rs diff --git a/crates/openshell-core/src/provider_credentials.rs b/crates/openshell-core/src/provider_credentials.rs index ab9bd26195..5f69dc44f6 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) } 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..195abceec6 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(()); 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..19c26539e2 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, 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(), }) 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 505f683463..bdf6fe9e7e 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( diff --git a/crates/openshell-supervisor/src/config_runtime.rs b/crates/openshell-supervisor/src/config_runtime.rs index 4bc22252b0..1d220de069 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, 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 From d7a924a71e9cb79a1af1fa6b5a71f0f45dd55485 Mon Sep 17 00:00:00 2001 From: Miles Date: Sat, 7 Nov 2026 15:34:06 +0000 Subject: [PATCH 3/4] test(network): cover connection revalidation and credential versioning Unit tests for the code the previous commit introduces, which need its new APIs. Behaviour visible through the relays is covered by the earlier characterisation tests. - contracts: unrelated endpoints are ignored; endpoint, L7 rule, binary and overlapping-policy changes close the connection and keep it closed after the policy is restored; quarantine always closes; policy_hash restamping is not a change; denials capture no revalidator; guard clones share one pin; the wait ignores unrelated reloads; - middleware: any field of the selected chain changing closes the connection, an identical chain does not, and only reloads that replace the registry close connections that run middleware; - capture: baselines come from the recorded authorizing generation, and a generation that left the history cannot be captured; - credential versions: recording of key and version per secret sent, including SigV4-style lookups; unused credentials; versions shared across connections and stable across identical reinstalls; a removed and restored key gets a new version; an older revision records stale only if its value has since changed; child-env-only installs and revocation retire credentials; grant versions follow their configuration; untracked handles and unattributable uses; - installed credential versions: unchanged by identical reinstalls and by a revocation with nothing left to revoke; changed by a new value, an added or removed credential, and a grant configuration change; - credential change publication: a new generation of the unchanged policy makes plain guards stale and lets a connection without credentials re-pin; a quarantine stays in force; - pinned policy hash: the authorizing generation's from the start, then that of the generation the guard re-pinned to (not a newer one it has not checked); none for plain guards or policies without a stamp; - endpoint configs are exactly the matched endpoints that carry extended configuration, in the same order, which is why the contract does not compare them; - token grants record their identity; passthrough middleware runs on the next request after an unrelated reload. Co-Authored-By: Claude Opus 5.5 --- .../src/provider_credentials.rs | 500 +++++++++++++++ .../src/l7/relay.rs | 202 ++++++ .../src/l7/token_grant_injection.rs | 67 ++ .../openshell-supervisor-network/src/opa.rs | 594 ++++++++++++++++++ 4 files changed, 1363 insertions(+) diff --git a/crates/openshell-core/src/provider_credentials.rs b/crates/openshell-core/src/provider_credentials.rs index 5f69dc44f6..8fd7d95712 100644 --- a/crates/openshell-core/src/provider_credentials.rs +++ b/crates/openshell-core/src/provider_credentials.rs @@ -4226,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-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 195abceec6..0bc2166c7e 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -13589,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 19c26539e2..c5ac90abb3 100644 --- a/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs +++ b/crates/openshell-supervisor-network/src/l7/token_grant_injection.rs @@ -1372,6 +1372,73 @@ mod tests { assert_eq!(fixture.requested_ids(), [REQUEST_ID]); } + fn tracked_grant_context(fixture: &TokenGrantTestFixture) -> 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 3ac278df02..17c8ec42e6 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -12984,4 +12984,598 @@ network_policies: 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); + } } From 3d9687014d00a9bf788e981490ee34dac2d8af87 Mon Sep 17 00:00:00 2001 From: Miles Date: Sat, 7 Nov 2026 15:34:06 +0000 Subject: [PATCH 4/4] test(network): rewrite the characterisation tests for the new APIs Test-only. The first commit's tests were written against the APIs that existed before the change so that they could run, and fail, on their own. Now that the change has landed: - authorize test decisions through EgressDecision::policy_input, the input revalidation re-evaluates; - check that injecting a credential is recorded on the connection, and that a connection that never injected one records nothing; - drop the test module's own ProviderCredentialState import, which the module now inherits; - drop the "Changed"/"Unchanged" labels and the descriptions of the old behaviour from the doc comments. Co-Authored-By: Claude Opus 5.5 --- .../openshell-supervisor-network/src/opa.rs | 6 +- .../src/proxy/relay.rs | 180 +++++++++--------- .../src/config_runtime.rs | 11 +- 3 files changed, 99 insertions(+), 98 deletions(-) diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 17c8ec42e6..284073aa83 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -12900,7 +12900,7 @@ network_policies: proto } - /// Unchanged: a guard pinned with `generation_guard` stays strict and goes + /// 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() { @@ -12919,7 +12919,7 @@ network_policies: ); } - /// Unchanged: a tunnel evaluator from `clone_engine_for_tunnel` stays strict. + /// 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(); @@ -12934,7 +12934,7 @@ network_policies: assert!(tunnel.is_stale()); } - /// Unchanged: proto conversion stamps the hash of the complete policy into + /// 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] diff --git a/crates/openshell-supervisor-network/src/proxy/relay.rs b/crates/openshell-supervisor-network/src/proxy/relay.rs index bdf6fe9e7e..3ba29e8d61 100644 --- a/crates/openshell-supervisor-network/src/proxy/relay.rs +++ b/crates/openshell-supervisor-network/src/proxy/relay.rs @@ -399,8 +399,6 @@ fn emit_stale_relay_close(request: &L7EvalContext, guard: &PolicyGenerationGuard mod tests { use super::super::{EgressIntent, EndpointDecision, ProcessIdentityEvidence}; use super::*; - use openshell_core::provider_credentials::ProviderCredentialState; - const POLICY_REGO: &str = include_str!("../../data/sandbox-policy.rego"); const EMPTY_POLICY_DATA: &str = "network_policies: {}\n"; @@ -787,30 +785,29 @@ network_policies: /// Authorize curl to example.com:80 the way the CONNECT adapter does. fn authorized_decision(engine: &OpaEngine) -> EgressDecision { - let input = crate::opa::NetworkInput { - host: "example.com".to_string(), - port: 80, - binary_path: std::path::PathBuf::from("/usr/bin/curl"), - binary_sha256: String::new(), + 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![], }; - let authorization = engine.authorize_egress(&input).unwrap(); + // 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" ); - EgressDecision { - intent: EgressIntent::connect("example.com".to_string(), 80), - action: authorization.action.clone(), - policy_generation: authorization.generation, - identity: ProcessIdentityEvidence::Available, - endpoint: EndpointDecision::from_authorization(&authorization), - binary: Some(input.binary_path), - binary_pid: None, - ancestors: vec![], - cmdline_paths: vec![], - } + 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`. @@ -857,8 +854,7 @@ network_policies: true } - /// Changed: approving an endpoint for another host leaves a raw relay open. - /// Today it closes. + /// 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()); @@ -879,8 +875,8 @@ network_policies: assert!(!closed, "approving another host must not close this relay"); } - /// Changed: a raw relay keeps moving bytes after an unrelated reload, and - /// still closes when its own endpoint is revoked. Today the reload closes it. + /// 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}; @@ -918,7 +914,7 @@ network_policies: .expect("stale relay closure should be clean"); } - /// Unchanged: revoking the connection's endpoint closes a raw relay. + /// 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()); @@ -935,7 +931,7 @@ network_policies: assert!(closed, "revoking the endpoint must close the relay"); } - /// Unchanged: a raw relay that was let through a fail-open chain closes + /// 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() { @@ -962,8 +958,8 @@ network_policies: ); } - /// Changed: approving an endpoint for another host leaves an HTTP - /// passthrough relay current, re-pinned to the new generation. Today it goes stale. + /// 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()); @@ -979,8 +975,8 @@ network_policies: assert_eq!(generation_guard.captured_generation(), engine.current_generation()); } - /// Changed: approving an endpoint for another host leaves an inspected (L7) - /// tunnel current. Today it goes stale. + /// 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 = @@ -1003,7 +999,7 @@ network_policies: ); } - /// Unchanged: narrowing the L7 rules of the connection's endpoint makes its + /// Narrowing the L7 rules of the connection's endpoint makes its /// tunnel stale. #[test] fn inspected_relay_goes_stale_when_its_rules_change() { @@ -1024,7 +1020,7 @@ network_policies: assert!(evaluator.is_stale(), "narrowed rules must close the tunnel"); } - /// Unchanged: a relay whose credential handle cannot report use goes stale + /// A relay whose credential handle cannot report use goes stale /// when provider credentials change. #[test] fn http_relay_goes_stale_when_provider_credentials_change() { @@ -1124,10 +1120,18 @@ network_policies: .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" + ); } - /// Changed: a connection that never had a credential injected cannot hold - /// a stale one, so it survives credential changes. Today it closes. + /// 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()); @@ -1142,9 +1146,11 @@ network_policies: !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()); } - /// Unchanged: rotating a credential the connection sent closes it; an + /// 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() { @@ -1160,8 +1166,8 @@ network_policies: assert!(guard.is_stale(), "the upstream session may hold the old credential"); } - /// Changed: rotating another provider's credential, or attaching a new - /// provider, leaves open a connection that never sent them. Today it closes. + /// 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()); @@ -1188,9 +1194,9 @@ network_policies: assert!(!guard.is_stale()); } - /// Changed: a connection that first injects a credential after its provider + /// A connection that first injects a credential after its provider /// was attached survives unrelated reloads, and closes when that credential - /// rotates. Today the attach closes it. + /// 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()); @@ -1216,7 +1222,7 @@ network_policies: assert!(guard.is_stale()); } - /// Unchanged: a reload between authorization and relay setup refuses the + /// A reload between authorization and relay setup refuses the /// relay, whatever the reload changed. #[test] fn reload_between_authorization_and_pin_refuses_the_relay() { @@ -1340,8 +1346,8 @@ network_policies: forwarded } - /// Changed: a kept-alive passthrough connection that runs middleware - /// serves its next request after an unrelated reload. Today it drops it. + /// 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); @@ -1352,7 +1358,7 @@ network_policies: ); } - /// Unchanged: a changed middleware chain does not apply to a kept-alive + /// 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() { @@ -1543,8 +1549,8 @@ network_policies: proto_policy_with(inspected_proto_endpoint("mcp", "/mcp", allow), other_hosts) } - /// Changed: an endpoint inserted before the connection's own, in the same - /// policy, shifts its index but not its rules. Today the relay closes. + /// 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()); @@ -1565,9 +1571,9 @@ network_policies: assert!(!closed); } - /// Changed: in a gateway-delivered policy every reload restamps every + /// In a gateway-delivered policy every reload restamps every /// endpoint with the new policy hash. That stamp alone must not close an - /// inspected tunnel. Today it closes. + /// 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()); @@ -1582,7 +1588,7 @@ network_policies: assert!(!evaluator.is_stale()); } - /// Unchanged: narrowing the rules of a gateway-delivered endpoint closes + /// 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() { @@ -1598,8 +1604,8 @@ network_policies: assert!(evaluator.is_stale()); } - /// Changed: middleware selected for another host does not affect this - /// connection. Today the relay closes. + /// 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()); @@ -1617,7 +1623,7 @@ network_policies: assert!(!closed); } - /// Unchanged: middleware newly selected for this host closes the relay, + /// 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() { @@ -1636,8 +1642,8 @@ network_policies: assert!(closed); } - /// Changed: replacing the middleware registry cannot affect a connection - /// that runs no middleware. Today the relay closes. + /// 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()); @@ -1660,7 +1666,7 @@ network_policies: assert!(!closed); } - /// Unchanged: replacing the middleware registry closes a connection that + /// 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] @@ -1681,8 +1687,8 @@ network_policies: assert!(guard.is_stale()); } - /// Changed: several unrelated reloads can land before a connection next - /// checks. It re-pins through all of them. Today it goes 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()); @@ -1700,8 +1706,8 @@ network_policies: assert_eq!(guard.captured_generation(), engine.current_generation()); } - /// Changed: a connection checked between reloads stays open however many - /// unrelated reloads there are. Today it goes stale at the first. + /// 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()); @@ -1717,7 +1723,7 @@ network_policies: } } - /// Unchanged: a connection that sleeps through a very long run of reloads + /// 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() { @@ -1735,7 +1741,7 @@ network_policies: assert!(guard.is_stale()); } - /// Unchanged: a reload that revoked the connection closes it even if the + /// 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() { @@ -1750,7 +1756,7 @@ network_policies: assert!(guard.is_stale()); } - /// Unchanged: a reload that changed the connection's rules closes it even + /// 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() { @@ -1770,7 +1776,7 @@ network_policies: assert!(evaluator.is_stale()); } - /// Unchanged: once a connection has been found stale it stays stale, even + /// 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() { @@ -1791,8 +1797,8 @@ network_policies: // --- Credential changes: install paths and placeholder generations --- - /// Changed: `install_prepared`, the path runtime reloads use, rotates a - /// credential this connection never sent. Today the relay closes. + /// `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()); @@ -1811,7 +1817,7 @@ network_policies: assert!(!guard.is_stale()); } - /// Unchanged: `install_prepared` rotating a credential this connection + /// `install_prepared` rotating a credential this connection /// sent closes the relay. #[test] fn http_relay_closes_when_install_prepared_rotates_a_credential_it_injected() { @@ -1828,8 +1834,8 @@ network_policies: assert!(guard.is_stale()); } - /// Changed: reinstalling identical credentials changes nothing a - /// connection holds. Today the relay closes. + /// 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()); @@ -1844,8 +1850,8 @@ network_policies: assert!(!guard.is_stale()); } - /// Changed: hiding a credential from future child processes does not - /// revoke it; the proxy still resolves it. Today the relay closes. + /// 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()); @@ -1861,9 +1867,9 @@ network_policies: assert!(!guard.is_stale()); } - /// Changed: a long-running process may still hold a placeholder from a + /// 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. Today the relay closes. + /// 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()); @@ -1892,7 +1898,7 @@ network_policies: assert!(!guard.is_stale()); } - /// Unchanged: a connection that sent a credential closes when it rotates, + /// 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() { @@ -1909,7 +1915,7 @@ network_policies: assert!(guard.is_stale()); } - /// Unchanged: detaching the provider of a credential this connection sent + /// 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() { @@ -2097,10 +2103,9 @@ network_policies: outcome } - /// Changed: 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. Today the reload - /// closes the tunnel, so nothing more is exchanged or reported. + /// 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"]); @@ -2116,7 +2121,7 @@ network_policies: ); } - /// Unchanged: until the supervisor publishes the reloaded policy's + /// 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] @@ -2128,7 +2133,7 @@ network_policies: assert_eq!(outcome.second_reported, None); } - /// Unchanged: an inventory published ahead of the runtime, for a policy + /// 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() { @@ -2140,7 +2145,7 @@ network_policies: assert_eq!(outcome.second_reported, None); } - /// Unchanged: a reload that changes the MCP endpoint's rules closes the + /// 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() { @@ -2235,9 +2240,9 @@ network_policies: assert!(rewritten.is_some()); } - /// Changed: a connection that sent a credential through its stable-handle + /// A connection that sent a credential through its stable-handle /// placeholder survives a rotation of another credential, and keeps its - /// placeholder. Today the relay closes. + /// 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()); @@ -2254,7 +2259,7 @@ network_policies: assert!(!guard.is_stale()); } - /// Unchanged: a connection that sent a credential through its stable-handle + /// 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] @@ -2275,10 +2280,9 @@ network_policies: // --- Credentials injected through other placeholder forms --- - /// Changed: a connection that sent a credential through its canonical + /// 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. Today - /// the relay closes. + /// 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()); @@ -2309,7 +2313,7 @@ network_policies: assert!(!guard.is_stale()); } - /// Unchanged: a connection that sent a credential through a placeholder + /// 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] @@ -2334,11 +2338,11 @@ network_policies: assert!(guard.is_stale()); } - /// Changed: provider revisions are sandbox-wide, so rotating another + /// 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. Today the relay closes. + /// 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()); diff --git a/crates/openshell-supervisor/src/config_runtime.rs b/crates/openshell-supervisor/src/config_runtime.rs index 1d220de069..b5fe00d594 100644 --- a/crates/openshell-supervisor/src/config_runtime.rs +++ b/crates/openshell-supervisor/src/config_runtime.rs @@ -4230,14 +4230,11 @@ mod tests { assert!(ctx.extension_credentials.get("removed-guard").is_none()); } - /// Changed: revoking provider credentials publishes a policy generation, + /// 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. Today credentials are - /// revoked without a new generation, so a connection that authenticated - /// once with a revoked credential, such as an upgraded WebSocket, stays - /// open. (Successful installs already publish one; see - /// `stream_provider_rotation_rebinds_policy_generation`.) + /// 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}; @@ -4310,7 +4307,7 @@ mod tests { } } - /// Unchanged: a revocation that finds no credential left to revoke, as + /// 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]