diff --git a/CHANGELOG.md b/CHANGELOG.md index f0d3c95..404d88b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -834,6 +834,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Fixed +- **Delegators and elicitation handlers are no longer reported as narrowed when + their family-specific hooks are reached.** `delegate(...)` is credited to + `token.delegate`, and elicitation verbs to `elicit`. Unreached hooks declared by + those plugins are still reported. + +- **The bundled hyper transport selects `ring` explicitly.** It no longer depends + on rustls's process default, which is ambiguous when a host includes both `ring` + and `aws-lc-rs`. It neither reads nor installs that default, and TLS setup errors + now return `HttpTransportError::Connect` instead of panicking. + +- **Glob routes under `tool:`, `resource:`, `prompt:`, and `llm:` now evaluate + their policy bodies.** Annotation lookup now resolves a request name to the + configured pattern. Exact selectors still outrank globs, including when the + exact route has no policy body. + - **A bundle joined through both `meta.tags` and `groups:` no longer runs its `authentication:` steps twice.** Bundle membership is now deduplicated before authentication and assertion layers are resolved, keeping their inheritance diff --git a/Cargo.lock b/Cargo.lock index 6173dea..c3f5e4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2379,8 +2379,10 @@ dependencies = [ "praxis-policy-plugin-elicitation-ciba", "praxis-policy-plugin-identity-jwt", "praxis-policy-session-valkey", + "rustls", "tokio", "tower-service", + "webpki-roots", ] [[package]] @@ -3871,9 +3873,9 @@ checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" [[package]] name = "uuid" -version = "1.24.1" +version = "1.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" +checksum = "b5772d71c9be8a8a6ac2117d949c5b224c1b72241bb611d9a3012edcf8af7812" dependencies = [ "getrandom 0.4.3", "js-sys", diff --git a/crates/ppe-apl-runtime/src/dispatch_plan.rs b/crates/ppe-apl-runtime/src/dispatch_plan.rs index f5999fd..d9865b5 100644 --- a/crates/ppe-apl-runtime/src/dispatch_plan.rs +++ b/crates/ppe-apl-runtime/src/dispatch_plan.rs @@ -637,6 +637,29 @@ pub(crate) fn collect_plugin_names_by_half(route: &CompiledRoute) -> (Vec Vec<(String, &'static str)> { + let mut out: Vec<(String, &'static str)> = Vec::new(); + let mut visit = |e: &Effect| { + let pair = match e { + Effect::Delegate(ds) => (ds.plugin_name.clone(), HOOK_TOKEN_DELEGATE), + Effect::Elicit(es) => (es.plugin_name.clone(), HOOK_ELICIT), + _ => return, + }; + if !out.contains(&pair) { + out.push(pair); + } + }; + walk_effects(&route.pre_invocation, &mut visit); + walk_effects(&route.post_invocation, &mut visit); + out +} + /// Compute the union of capabilities declared by every plugin a /// `CompiledRoute` can dispatch to (with per-route overrides applied). /// diff --git a/crates/ppe-apl-runtime/src/visitor.rs b/crates/ppe-apl-runtime/src/visitor.rs index 4c545b3..6bc4fd8 100644 --- a/crates/ppe-apl-runtime/src/visitor.rs +++ b/crates/ppe-apl-runtime/src/visitor.rs @@ -518,13 +518,20 @@ impl AplConfigVisitor { /// routes below it. fn record_reached_plugins(&self, route: &CompiledRoute, hook_pre: &str, hook_post: &str) { let (pre, post) = crate::dispatch_plan::collect_plugin_names_by_half(route); + // Delegation and elicitation use family-specific hooks, recorded below. + let family_fixed = crate::dispatch_plan::collect_family_fixed_plugin_hooks(route); let mut state = self .state .write() .unwrap_or_else(std::sync::PoisonError::into_inner); + let family_fixed_names: std::collections::HashSet<&str> = + family_fixed.iter().map(|(name, _)| name.as_str()).collect(); for (names, hook) in [(pre, hook_pre), (post, hook_post)] { for name in names { state.reached_plugin_names.insert(name.clone()); + if family_fixed_names.contains(name.as_str()) { + continue; + } state .reached_plugin_hooks .entry(name) @@ -532,6 +539,14 @@ impl AplConfigVisitor { .insert(hook.to_owned()); } } + for (name, hook) in family_fixed { + state.reached_plugin_names.insert(name.clone()); + state + .reached_plugin_hooks + .entry(name) + .or_default() + .insert(hook.to_owned()); + } } /// Tally the plugins a layer's steps name, without a hook. @@ -2145,9 +2160,7 @@ routes: ); } - /// A glob route under one of the four MCP selectors. The annotation is - /// installed under the pattern as written, and the lookup is exact - /// equality, so a request named by a glob never reaches the body. + /// A glob route's annotation is keyed by its pattern, not the request name. const GLOB_TOOL_ROUTE: &str = r#" engine_settings: dispatch: policy @@ -2159,10 +2172,10 @@ routes: "#; #[tokio::test] - async fn a_glob_tool_route_still_does_not_evaluate_its_policy_body() { + async fn a_glob_tool_route_evaluates_its_policy_body() { let mgr = engine_with(GLOB_TOOL_ROUTE).await; - let (allowed, _bg) = mgr + let (denied, _bg) = mgr .invoke_named::( HOOK_CMF_TOOL_PRE_INVOKE, payload(), @@ -2171,25 +2184,23 @@ routes: ) .await; assert!( - allowed.continue_processing, - "a name the glob matches does not equal the pattern the handler is \ - installed under, so the body does not evaluate; violation = {:?}", - allowed.violation + !denied.continue_processing, + "the route denies and the name the glob covers is governed by it" ); - // The handler exists and its body denies, so the line above is the - // lookup and not a missing installation. - let (denied, _bg) = mgr + // A name outside the pattern reaches no route body. + let (allowed, _bg) = mgr .invoke_named::( HOOK_CMF_TOOL_PRE_INVOKE, payload(), - tool_request("hr-*"), + tool_request("finance-close"), None, ) .await; assert!( - !denied.continue_processing, - "the body is installed under the pattern as written" + allowed.continue_processing, + "a name outside the pattern reaches no body; violation = {:?}", + allowed.violation ); } diff --git a/crates/ppe-apl-runtime/tests/dispatch_mode_e2e.rs b/crates/ppe-apl-runtime/tests/dispatch_mode_e2e.rs index a52340f..c1c1c23 100644 --- a/crates/ppe-apl-runtime/tests/dispatch_mode_e2e.rs +++ b/crates/ppe-apl-runtime/tests/dispatch_mode_e2e.rs @@ -556,6 +556,73 @@ routes: ); } +/// A `delegate(...)` step reaches its plugin on `token.delegate`. +#[test] +fn a_delegator_reached_by_a_delegate_step_is_not_reported_as_narrowed() { + let alarms = alarms_raised_by_loading( + " +plugins: + - name: workday-oauth + kind: builtin + hooks: [token.delegate] +routes: + - tool: get_compensation + authorization: + pre_invocation: + - \"delegate(workday-oauth, target: workday-api, audience: workday-api)\" +", + ); + assert!( + !alarms.contains(&NARROWED.to_owned()), + "`token.delegate` is covered: {alarms:?}" + ); +} + +/// An elicitation verb reaches its handler on `elicit`. +#[test] +fn an_elicitation_handler_reached_by_a_verb_is_not_reported_as_narrowed() { + let alarms = alarms_raised_by_loading( + " +plugins: + - name: manager-approver + kind: builtin + hooks: [elicit] +routes: + - tool: adjust_compensation + authorization: + pre_invocation: + - \"require_approval(manager-approver, from: claim.manager, channel: \\\"ciba\\\")\" +", + ); + assert!( + !alarms.contains(&NARROWED.to_owned()), + "`elicit` is covered: {alarms:?}" + ); +} + +/// Family-specific reachability does not hide other uncovered hooks. +#[test] +fn a_delegator_declaring_an_unreached_cmf_hook_is_still_reported() { + let alarms = alarms_raised_by_loading( + " +plugins: + - name: workday-oauth + kind: builtin + hooks: [token.delegate, cmf.tool_post_invoke] +routes: + - tool: get_compensation + authorization: + pre_invocation: + - \"delegate(workday-oauth, target: workday-api, audience: workday-api)\" +", + ); + assert!( + alarms.contains(&NARROWED.to_owned()), + "`cmf.tool_post_invoke` is declared and no step reaches it there: \ + {alarms:?}" + ); +} + // ---- the core-side backstop ------------------------------------------- /// A host that registers no orchestrator gets the flipped default with no diff --git a/crates/ppe-apl-runtime/tests/http_route_e2e.rs b/crates/ppe-apl-runtime/tests/http_route_e2e.rs index 05d344d..a3f3b2a 100644 --- a/crates/ppe-apl-runtime/tests/http_route_e2e.rs +++ b/crates/ppe-apl-runtime/tests/http_route_e2e.rs @@ -781,12 +781,9 @@ routes: } } -/// A glob route under one of the entity selectors installs its annotation under -/// the pattern as written, and the lookup is exact, so its policy body never -/// evaluates. With the activation list gone there is no chain behind it either, -/// so the route reaches nothing. +/// A glob selector resolves request names to the annotation keyed by its pattern. #[tokio::test] -async fn a_glob_entity_routes_body_never_evaluates_and_nothing_replaces_it() { +async fn a_glob_entity_route_dispatches_its_body() { const YAML: &str = r#" engine_settings: dispatch: policy @@ -802,11 +799,166 @@ routes: "#; let (mgr, ledger) = engine_with(YAML).await; + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-get-salary")).await); + assert_eq!( + fired(&ledger), + vec!["body-audit".to_owned()], + "the request name must resolve to the glob annotation" + ); +} + +/// Every name matched by a glob shares its compiled body. +#[tokio::test] +async fn many_names_under_one_glob_share_its_body() { + const YAML: &str = r#" +engine_settings: + dispatch: policy +plugins: + - name: body-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] +routes: + - tool: "hr-*" + authorization: + pre_invocation: + - "run(body-audit)" +"#; + let (mgr, ledger) = engine_with(YAML).await; + + for name in ["hr-get-salary", "hr-adjust-comp"] { + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request(name)).await); + } + assert_eq!( + fired(&ledger), + vec!["body-audit".to_owned(), "body-audit".to_owned()], + "every name the pattern covers reaches the one body it wrote" + ); +} + +/// A bracket expression is a literal, not a character class. +/// +/// `wildmatch` reads `*` and `?` and nothing else, so `hr-[a-z]` matches the +/// eight characters `hr-[a-z]` and no other name. That makes it an exact +/// selector: the annotation key and the request name agree whenever it matches, +/// which is why the glob detector does not look for `[`. +#[tokio::test] +async fn a_bracket_selector_is_matched_literally() { + const YAML: &str = r#" +engine_settings: + dispatch: policy +plugins: + - name: body-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] +routes: + - tool: "hr-[a-z]" + authorization: + pre_invocation: + - "run(body-audit)" +"#; + let (mgr, ledger) = engine_with(YAML).await; + + // The name a character class would have covered. + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-x")).await); + assert!(fired(&ledger).is_empty(), "`hr-x` matches no route"); + + // The literal spelling is what the selector names, and the exact lookup + // finds its annotation with no glob resolution. + clear(&ledger); + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-[a-z]")).await); + assert_eq!( + fired(&ledger), + vec!["body-audit".to_owned()], + "the literal name reaches the body" + ); +} + +/// A name outside the glob reaches no body. +#[tokio::test] +async fn a_name_outside_the_glob_reaches_no_body() { + const YAML: &str = r#" +engine_settings: + dispatch: policy +plugins: + - name: body-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] +routes: + - tool: "hr-*" + authorization: + pre_invocation: + - "run(body-audit)" +"#; + let (mgr, ledger) = engine_with(YAML).await; + + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("finance-close")).await); + assert!(fired(&ledger).is_empty(), "the name matches no route"); +} + +/// An exact selector outranks a matching glob. +#[tokio::test] +async fn an_exact_entity_route_outranks_a_glob_that_also_matches() { + const YAML: &str = r#" +engine_settings: + dispatch: policy +plugins: + - name: glob-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] + - name: exact-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] +routes: + - tool: "hr-*" + authorization: + pre_invocation: + - "run(glob-audit)" + - tool: hr-get-salary + authorization: + pre_invocation: + - "run(exact-audit)" +"#; + let (mgr, ledger) = engine_with(YAML).await; + + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-get-salary")).await); + assert_eq!( + fired(&ledger), + vec!["exact-audit".to_owned()], + "the exact route must win" + ); + + clear(&ledger); + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-adjust-comp")).await); + assert_eq!( + fired(&ledger), + vec!["glob-audit".to_owned()], + "a name only the pattern covers still reaches the pattern's body" + ); +} + +/// An exact route without policy still shadows a matching glob. +#[tokio::test] +async fn an_exact_route_with_no_body_shadows_a_glob_that_has_one() { + const YAML: &str = r#" +engine_settings: + dispatch: policy +plugins: + - name: glob-audit + kind: test/record + hooks: [cmf.tool_pre_invoke] +routes: + - tool: "hr-*" + authorization: + pre_invocation: + - "run(glob-audit)" + - tool: hr-get-salary +"#; + let (mgr, ledger) = engine_with(YAML).await; + assert!(fire(&mgr, "cmf.tool_pre_invoke", tool_request("hr-get-salary")).await); assert!( fired(&ledger).is_empty(), - "the glob's annotation is keyed on the pattern, so an exact lookup for \ - `hr-get-salary` finds nothing and nothing else activates a plugin" + "the glob body must not replace the more specific route's empty body" ); } @@ -1517,9 +1669,12 @@ routes: tool_request("get_weather"), "tool-audit", ), - // The glob's annotation is keyed on the pattern as written and the - // lookup is exact, so nothing answers for a name the glob covers. - ("cmf.tool_pre_invoke", tool_request("hr-get-salary"), ""), + // Resolve the request name to the glob pattern used as the annotation key. + ( + "cmf.tool_pre_invoke", + tool_request("hr-get-salary"), + "glob-audit", + ), ( "cmf.resource_pre_fetch", entity_request("resource", "file:///data.csv"), diff --git a/crates/ppe-core/src/engine.rs b/crates/ppe-core/src/engine.rs index 8cacf64..ccfc4f1 100644 --- a/crates/ppe-core/src/engine.rs +++ b/crates/ppe-core/src/engine.rs @@ -160,6 +160,36 @@ fn declares_assertions(config: &PolicyConfig) -> bool { || config.routes.iter().any(|route| route.assertions.is_some()) } +/// Whether any named route uses a glob selector. +/// +/// This avoids resolving routes before annotation lookup in exact/list-only configs. +fn declares_glob_named_routes(config: &PolicyConfig) -> bool { + config.routes.iter().any(|route| { + [ + route.tool.as_ref(), + route.resource.as_ref(), + route.prompt.as_ref(), + route.llm.as_ref(), + ] + .into_iter() + .flatten() + .any(is_glob_selector) + }) +} + +/// Whether a selector can match a different name through wildcards. +/// +/// `*` and `?` are the whole set: `wildmatch` defines no others and no escapes, +/// so `hr-[a-z]` is the literal eight characters rather than a character class. +/// A literal selector needs nothing here, because its annotation key and the +/// request name agree whenever it matches. +fn is_glob_selector(selector: &config::StringOrList) -> bool { + match selector { + config::StringOrList::Single(pattern) => pattern.as_str().contains(['*', '?']), + config::StringOrList::List(_) => false, + } +} + /// Turn a refused assertion into a denial, keeping what the pipeline recorded. /// /// `PipelineResult::denied` constructs with no errors and no metadata, so a bare @@ -493,6 +523,9 @@ struct RuntimeSnapshot { /// under. Read on the generic-HTTP path when a request carries no readable /// path, for the same reason its authentication counterpart is. http_routes_declaring_assertions: Arc<[String]>, + + /// Whether any named route uses a glob selector. + declares_glob_named_routes: bool, } /// Composite key for route annotations. Includes the hook name so a single @@ -823,6 +856,7 @@ fn snapshot_from_config(registry: PluginRegistry, policy_config: PolicyConfig) - let http_routes_declaring_authentication = http_routes_declaring_authentication(&policy_config); let declares_assertions = declares_assertions(&policy_config); let http_routes_declaring_assertions = http_routes_declaring_assertions(&policy_config); + let declares_glob_named_routes = declares_glob_named_routes(&policy_config); RuntimeSnapshot { registry, executor, @@ -832,6 +866,7 @@ fn snapshot_from_config(registry: PluginRegistry, policy_config: PolicyConfig) - http_routes_declaring_authentication, declares_assertions, http_routes_declaring_assertions, + declares_glob_named_routes, } } @@ -848,6 +883,7 @@ impl PolicyEngine { http_routes_declaring_authentication: Arc::from(Vec::new()), declares_assertions: false, http_routes_declaring_assertions: Arc::from(Vec::new()), + declares_glob_named_routes: false, }; Self { runtime: arc_swap::ArcSwap::from_pointee(snapshot), @@ -2495,6 +2531,51 @@ impl PolicyEngine { hook_name: hook_name.to_owned(), }) }); + // Glob annotations are keyed by their pattern, not the request name. + // Resolve only when a glob exists, reusing assertion resolution when available. + let glob_matched = if candidate.is_none() && snapshot.declares_glob_named_routes { + early_named.as_ref().map_or_else( + || { + routing_config.and_then(|policy_config| { + config::resolve_route( + policy_config, + config::RouteQuery::named(et, en).with_scope(request_scope), + ) + }) + }, + |matched| { + Some(config::MatchedRoute { + route: matched.route, + name: matched.name.clone(), + }) + }, + ) + } else { + None + }; + // An equal name was already checked above. + let candidate = candidate.or_else(|| { + let matched = glob_matched.as_ref()?; + if matched.name == en { + return None; + } + let scoped = request_scope.and_then(|s| { + snapshot.route_annotations.get(&AnnotationKey { + entity_type: et.to_owned(), + entity_name: matched.name.clone(), + scope: Some(s.to_owned()), + hook_name: hook_name.to_owned(), + }) + }); + scoped.or_else(|| { + snapshot.route_annotations.get(&AnnotationKey { + entity_type: et.to_owned(), + entity_name: matched.name.clone(), + scope: None, + hook_name: hook_name.to_owned(), + }) + }) + }); if let Some(entry) = candidate { return Ok(( Arc::new(vec![entry.clone()]), diff --git a/crates/ppe/Cargo.toml b/crates/ppe/Cargo.toml index d528c09..7cde930 100644 --- a/crates/ppe/Cargo.toml +++ b/crates/ppe/Cargo.toml @@ -78,6 +78,8 @@ valkey = ["_builtin", "dep:praxis-policy-session-valkey"] http-hyper = [ "dep:hyper-util", "dep:hyper-rustls", + "dep:rustls", + "dep:webpki-roots", "dep:http-body-util", "dep:bytes", "dep:http", @@ -137,6 +139,13 @@ hyper-rustls = { version = "0.27", optional = true, default-features = false, fe "tls12", "webpki-roots", ] } +# Direct dependencies let the transport select `ring` without reading or setting +# rustls's process-wide default provider. +rustls = { version = "0.23", optional = true, default-features = false, features = [ + "ring", + "tls12", +] } +webpki-roots = { version = "1", optional = true } http-body-util = { version = "0.1", optional = true } bytes = { workspace = true, optional = true } http = { workspace = true, optional = true } diff --git a/crates/ppe/src/http_hyper.rs b/crates/ppe/src/http_hyper.rs index aa561cc..a32e162 100644 --- a/crates/ppe/src/http_hyper.rs +++ b/crates/ppe/src/http_hyper.rs @@ -217,57 +217,80 @@ impl HyperTransport { self } - /// The shared client, built on first call. - fn client(&self) -> &HyperClient { - self.client.get_or_init(|| { - let mut http = HttpConnector::new_with_resolver(EgressResolver { - allow_private: self.allow_private_destinations, - }); - // The HTTPS connector wraps this one, so it must accept the - // `https` scheme rather than rejecting it as non-HTTP. - http.enforce_http(false); - http.set_connect_timeout(Some(self.connect_timeout)); - // hyper-util defaults this to false; reqwest sets it true. - // Leaving Nagle on would let a small request body — a token - // exchange form is a couple of hundred bytes — sit waiting to - // coalesce with data that never comes, adding tens of - // milliseconds to a call on the request path. - http.set_nodelay(true); - http.set_keepalive(self.tcp_keepalive); - - let tls = hyper_rustls::HttpsConnectorBuilder::new() - // Webpki roots rather than the system store, matching - // what the reqwest path resolved to and keeping the - // trust set identical across every deployment. - .with_webpki_roots() - // `https_or_http`, not `https_only`: `identity-jwt` - // supports an explicit `insecure_http: true` for local - // development, and it already refuses plaintext by - // default one layer up. Enforcing here as well would - // make that setting silently ineffective. - .https_or_http(); - - // `enable_all_versions` advertises ALPN `h2, http/1.1`; - // `enable_http1` advertises none. Either way a peer that - // cannot do HTTP/2 gets HTTP/1.1, and plaintext gets it - // regardless since there is no ALPN without TLS. - let https = if self.http2 { - tls.enable_all_versions().wrap_connector(http) - } else { - tls.enable_http1().wrap_connector(http) - }; + /// Return the shared client, building it on first use. + fn client(&self) -> Result<&HyperClient, HttpTransportError> { + if let Some(client) = self.client.get() { + return Ok(client); + } + let built = Self::build_client(self)?; + // Another caller may win the race; use whichever equivalent client was stored. + let _ = self.client.set(built); + self.client + .get() + .ok_or_else(|| HttpTransportError::Connect("HTTP client init raced".to_owned())) + } + + /// Build a pooling client. + fn build_client(&self) -> Result { + let mut http = HttpConnector::new_with_resolver(EgressResolver { + allow_private: self.allow_private_destinations, + }); + // The HTTPS connector wraps this one, so it must accept the + // `https` scheme rather than rejecting it as non-HTTP. + http.enforce_http(false); + http.set_connect_timeout(Some(self.connect_timeout)); + // hyper-util defaults this to false; reqwest sets it true. + // Leaving Nagle on would let a small request body — a token + // exchange form is a couple of hundred bytes — sit waiting to + // coalesce with data that never comes, adding tens of + // milliseconds to a call on the request path. + http.set_nodelay(true); + http.set_keepalive(self.tcp_keepalive); + + // Select `ring` locally: a host may load both supported providers, + // leaving rustls without a process default. Do not install one here. + // Keep the webpki roots used by the previous connector configuration. + let mut roots = rustls::RootCertStore::empty(); + roots.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + let tls_config = rustls::ClientConfig::builder_with_provider(std::sync::Arc::new( + rustls::crypto::ring::default_provider(), + )) + .with_safe_default_protocol_versions() + .map_err(|e| HttpTransportError::Connect(format!("rustls client configuration: {e}")))? + .with_root_certificates(roots) + .with_no_client_auth(); + + let tls = hyper_rustls::HttpsConnectorBuilder::new() + .with_tls_config(tls_config) + // `https_or_http`, not `https_only`: `identity-jwt` + // supports an explicit `insecure_http: true` for local + // development, and it already refuses plaintext by + // default one layer up. Enforcing here as well would + // make that setting silently ineffective. + .https_or_http(); + + // `enable_all_versions` advertises ALPN `h2, http/1.1`; + // `enable_http1` advertises none. Either way a peer that + // cannot do HTTP/2 gets HTTP/1.1, and plaintext gets it + // regardless since there is no ALPN without TLS. + let https = if self.http2 { + tls.enable_all_versions().wrap_connector(http) + } else { + tls.enable_http1().wrap_connector(http) + }; - Client::builder(TokioExecutor::new()) - // Without a timer, `pool_idle_timeout` silently does - // nothing and idle connections are never evicted. With - // `pool_max_idle_per_host` defaulting to unlimited, that - // is unbounded socket growth against a busy `IdP`, which - // is a slow leak rather than an error anyone would see. - .pool_timer(TokioTimer::new()) - .pool_idle_timeout(self.pool_idle_timeout) - .pool_max_idle_per_host(self.pool_max_idle_per_host) - .build(https) - }) + let client = Client::builder(TokioExecutor::new()) + // Without a timer, `pool_idle_timeout` silently does + // nothing and idle connections are never evicted. With + // `pool_max_idle_per_host` defaulting to unlimited, that + // is unbounded socket growth against a busy `IdP`, which + // is a slow leak rather than an error anyone would see. + .pool_timer(TokioTimer::new()) + .pool_idle_timeout(self.pool_idle_timeout) + .pool_max_idle_per_host(self.pool_max_idle_per_host) + .build(https); + + Ok(client) } } @@ -380,7 +403,7 @@ impl HttpTransport for HyperTransport { // Build the pool even when the destination is later refused, so // a refused first request still lands the client on the runtime // that served it. - let client = self.client(); + let client = self.client()?; if !self.allow_private_destinations && let Some(host) = uri.host() @@ -676,6 +699,27 @@ mod tests { assert!(t.client.get().is_some(), "first use must build the pool"); } + #[tokio::test] + async fn the_pool_builds_with_no_process_default_crypto_provider() { + // Building against the named provider must neither require nor install a default. + assert!( + rustls::crypto::CryptoProvider::get_default().is_none(), + "the guard only means something while nothing has installed a default" + ); + let t = HyperTransport::new(); + // A closed loopback port builds the client, then fails to connect. + let _ = t.execute(HttpRequest::get("https://127.0.0.1:1/x")).await; + assert!( + t.client.get().is_some(), + "the pool must build against the named provider" + ); + assert!( + rustls::crypto::CryptoProvider::get_default().is_none(), + "naming a provider must not install one process-wide, which would \ + race a host installing its own" + ); + } + #[test] fn the_defaults_bound_the_pool_and_disable_nagle() { // These three are the difference between "works in a demo" and diff --git a/docs/upgrade-apl.md b/docs/upgrade-apl.md index 9f9d980..f9ee5fa 100644 --- a/docs/upgrade-apl.md +++ b/docs/upgrade-apl.md @@ -12,6 +12,9 @@ means every key in it does something. Work through the sections in order. The first two change the shape of the document, and the rest are local rewrites. +Section 10 also covers the `perform_http` capability required by plugins that +fetch JWKS, exchange tokens, or dispatch CIBA prompts. + --- ## 1. `engine_settings:`, and the dispatch mode @@ -428,6 +431,78 @@ For a host embedding the engine rather than only writing configuration. | `GlobalConfig.identity`, `PolicyGroup.identity`, `RouteEntry.identity` | `.authentication` | | `compile_config`, `ConfigYaml`, `CompiledConfig` | deleted; see below | | `ParseError::RenamedField`, `::ConflictingAuthorizationForms` | deleted | +| `cmf::constants::HOOK_CMF_HTTP_REQUEST` | `http_hook::HOOK_HTTP_REQUEST` | +| `cmf::constants::HOOK_CMF_HTTP_RESPONSE` | `http_hook::HOOK_HTTP_RESPONSE` | +| `invoke_named::` on either HTTP hook | `invoke_named::`, with `HttpPayload` | +| `HookHandler` for an HTTP hook | `HookHandler` | +| `HookFamily::for_entity -> HookFamily` | `-> Option` | +| `Subject::claims: HashMap` | `HashMap` | + +### The generic-HTTP hooks moved families + +The hook names are now `http.request` and `http.response`; their constants, +handler type, and empty payload live in `praxis_policy_core::http_hook`: + +```rust +// before +use praxis_policy_core::cmf::constants::HOOK_CMF_HTTP_REQUEST; +let payload = MessagePayload { message: Message::text(Role::User, "") }; +mgr.invoke_named::(HOOK_CMF_HTTP_REQUEST, payload, ext, None).await; +``` + +```rust +// after +use praxis_policy_core::http_hook::{HOOK_HTTP_REQUEST, HttpHook, HttpPayload}; +mgr.invoke_named::(HOOK_HTTP_REQUEST, HttpPayload, ext, None).await; +``` + +HTTP handlers no longer receive a fabricated placeholder message. YAML using +`cmf.http_request` or `cmf.http_response` now fails with the replacement name. + +### Subject claims keep their JSON shape + +`Subject::claims` changed from `HashMap` to +`HashMap`. To retain flat strings without quoting string values: + +```rust +let flat = value.as_str().map_or_else(|| value.to_string(), str::to_owned); +``` + +### A host must install an HTTP transport + +This requirement is checked at initialization, not compile time. + +`identity-jwt`, `delegator-oauth`, and `elicitation-ciba` use a transport supplied +by the host. Without one, HTTP-dependent plugins fail at +`PolicyEngine::initialize()`. + +To reuse the host's connection pool, trust store, and egress path: + +```rust +mgr.set_http_transport(Arc::new(MyTransport::new())); +``` + +Otherwise, enable `praxis-policy`'s non-default `http-hyper` feature and install +the bundled transport explicitly: + +```rust +praxis_policy::install_builtins(&mgr); +praxis_policy::install_default_http_transport(&mgr); +``` + +The bundled transport builds its pool on first use. Each plugin using `jwks_url`, +OAuth delegation, or CIBA must also declare `perform_http`: + +```yaml +plugins: + - name: jwt-user + kind: identity/jwt + capabilities: + - perform_http +``` + +`ServiceError::NotInstalled` indicates a missing host transport; +`ServiceError::NotPermitted` indicates a missing capability. `Phase` and `CompiledRoute` both derive `Serialize`, so **the serialized keys change too**: a phase serializes as `pre_invocation` / `post_invocation`, and a