From 4afdc1dbfa61fcb915ea38b420b66fbff0a2f879 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Tue, 30 Jun 2026 11:27:19 -0400 Subject: [PATCH 1/8] feat(http): add HTTP request-line attributes to HttpExtension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add optional method/path/host/scheme to HttpExtension and surface them in the APL bag as http.method/path/host/scheme. These let CEL/APL policies reason over the HTTP request line — needed by the Praxis AuthPolicy transpiler, where Kuadrant predicates over request.method/path/host map to http.* (Praxis spike Phase B / U1). The request line rides the existing read_headers capability: the `http` extension slot is gated as a whole in cpex-core's filter_extensions, so a base-tier split would require granular http sub-field filtering (deferred). The host field is documented to be populated from a validated authority (e.g. HTTP/2 :authority), never a raw client Host header, so host-based policy cannot be bypassed. Signed-off-by: Frederico Araujo --- crates/apl-cmf/src/capability_namespaces.rs | 5 +++ crates/apl-cmf/src/constants.rs | 7 ++++ crates/apl-cmf/src/http.rs | 43 +++++++++++++++++++++ crates/apl-cmf/src/lib.rs | 3 +- crates/cpex-core/src/extensions/http.rs | 21 ++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/crates/apl-cmf/src/capability_namespaces.rs b/crates/apl-cmf/src/capability_namespaces.rs index ac50f98f..7bbdbe80 100644 --- a/crates/apl-cmf/src/capability_namespaces.rs +++ b/crates/apl-cmf/src/capability_namespaces.rs @@ -146,6 +146,11 @@ const TABLE: &[CapabilityEntry] = &[ prefixes: &[ BAG_HTTP_REQUEST_HEADERS_PREFIX, BAG_HTTP_RESPONSE_HEADERS_PREFIX, + // The request line rides the same capability as headers. + BAG_HTTP_METHOD, + BAG_HTTP_PATH, + BAG_HTTP_HOST, + BAG_HTTP_SCHEME, ], }, CapabilityEntry { diff --git a/crates/apl-cmf/src/constants.rs b/crates/apl-cmf/src/constants.rs index 276fb4a6..85e0f096 100644 --- a/crates/apl-cmf/src/constants.rs +++ b/crates/apl-cmf/src/constants.rs @@ -105,6 +105,13 @@ pub const BAG_META_PREFIX: &str = "meta."; pub const BAG_REQUEST_PREFIX: &str = "request."; pub const BAG_HTTP_REQUEST_HEADERS_PREFIX: &str = "http.request_headers."; pub const BAG_HTTP_RESPONSE_HEADERS_PREFIX: &str = "http.response_headers."; +// HTTP request line — exact keys. These ride the same `read_headers` +// capability as headers (the whole `http` slot is gated together in +// `cpex-core::extensions::filter`). +pub const BAG_HTTP_METHOD: &str = "http.method"; +pub const BAG_HTTP_PATH: &str = "http.path"; +pub const BAG_HTTP_HOST: &str = "http.host"; +pub const BAG_HTTP_SCHEME: &str = "http.scheme"; pub const BAG_LLM_PREFIX: &str = "llm."; pub const BAG_MCP_PREFIX: &str = "mcp."; pub const BAG_COMPLETION_PREFIX: &str = "completion."; diff --git a/crates/apl-cmf/src/http.rs b/crates/apl-cmf/src/http.rs index a28fdf9e..b0950bb4 100644 --- a/crates/apl-cmf/src/http.rs +++ b/crates/apl-cmf/src/http.rs @@ -10,13 +10,31 @@ // to remember the original case. // // Namespace: +// http.method : String (request line) +// http.path : String +// http.host : String +// http.scheme : String // http.request_headers. : String (lowercased name) // http.response_headers. : String (lowercased name) use apl_core::AttributeBag; use cpex_core::extensions::HttpExtension; +use crate::constants::{BAG_HTTP_HOST, BAG_HTTP_METHOD, BAG_HTTP_PATH, BAG_HTTP_SCHEME}; + pub fn extract_http(http: &HttpExtension, bag: &mut AttributeBag) { + if let Some(method) = &http.method { + bag.set(BAG_HTTP_METHOD.to_string(), method.clone()); + } + if let Some(path) = &http.path { + bag.set(BAG_HTTP_PATH.to_string(), path.clone()); + } + if let Some(host) = &http.host { + bag.set(BAG_HTTP_HOST.to_string(), host.clone()); + } + if let Some(scheme) = &http.scheme { + bag.set(BAG_HTTP_SCHEME.to_string(), scheme.clone()); + } for (k, v) in &http.request_headers { bag.set( format!("http.request_headers.{}", k.to_lowercase()), @@ -35,6 +53,31 @@ pub fn extract_http(http: &HttpExtension, bag: &mut AttributeBag) { mod tests { use super::*; + #[test] + fn request_line_surfaced_in_bag() { + let http = HttpExtension { + method: Some("POST".to_string()), + path: Some("/api/widgets".to_string()), + host: Some("api.example.com".to_string()), + scheme: Some("https".to_string()), + ..Default::default() + }; + let mut bag = AttributeBag::new(); + extract_http(&http, &mut bag); + assert_eq!(bag.get_string("http.method"), Some("POST")); + assert_eq!(bag.get_string("http.path"), Some("/api/widgets")); + assert_eq!(bag.get_string("http.host"), Some("api.example.com")); + assert_eq!(bag.get_string("http.scheme"), Some("https")); + } + + #[test] + fn request_line_absent_when_unset() { + let http = HttpExtension::default(); + let mut bag = AttributeBag::new(); + extract_http(&http, &mut bag); + assert_eq!(bag.get_string("http.method"), None); + } + #[test] fn headers_lowercased_in_bag() { let mut http = HttpExtension::default(); diff --git a/crates/apl-cmf/src/lib.rs b/crates/apl-cmf/src/lib.rs index 47c63a48..cb71f3dc 100644 --- a/crates/apl-cmf/src/lib.rs +++ b/crates/apl-cmf/src/lib.rs @@ -29,7 +29,8 @@ // AgentExtension → agent.* (session, conversation, lineage) // MetaExtension → meta.* // RequestExtension → request.* -// HttpExtension → http.request_headers.*, http.response_headers.* +// HttpExtension → http.method, http.path, http.host, http.scheme, +// http.request_headers.*, http.response_headers.* // LLMExtension → llm.* // MCPExtension → mcp.tool.*, mcp.resource.*, mcp.prompt.* // CompletionExtension → completion.* diff --git a/crates/cpex-core/src/extensions/http.rs b/crates/cpex-core/src/extensions/http.rs index 3fa1157a..464f0889 100644 --- a/crates/cpex-core/src/extensions/http.rs +++ b/crates/cpex-core/src/extensions/http.rs @@ -28,6 +28,27 @@ pub struct HttpExtension { /// HTTP response headers (from upstream, populated post-invoke). #[serde(default)] pub response_headers: HashMap, + + /// HTTP request method (e.g. `GET`, `POST`). Set by the host when + /// the request is HTTP; `None` for non-HTTP transports. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub method: Option, + + /// HTTP request path (e.g. `/api/v1/widgets`). Excludes the query + /// string unless the host chooses to include it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub path: Option, + + /// HTTP request authority/host. The host MUST populate this from a + /// validated authority (e.g. the HTTP/2 `:authority` pseudo-header), + /// never a raw client-supplied `Host` header, so host-based policy + /// is not bypassable. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub host: Option, + + /// HTTP request scheme (`http` / `https`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub scheme: Option, } impl HttpExtension { From e725334ec1b4917baffe6975c18de8734875f4d4 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Tue, 30 Jun 2026 16:13:08 -0400 Subject: [PATCH 2/8] feat(apl): carry custom denial response via PluginViolation.details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a per-route `response:` block (the transpiled form of a Kuadrant AuthPolicy `denyWith`) that lets a route declare a custom HTTP status, body, and headers for its denials (Praxis spike Phase B / U2). - New optional DenyResponse on CompiledRoute (additive; most-specific layer wins in apply_layer). Read out-of-band from the route YAML by the apl-cpex visitor, like the `policy:` block — cpex-core tolerates the key. - On Decision::Deny, route_handler stashes status/body/headers into the existing PluginViolation.details map under http.status / http.body / http.headers. No new fields on PluginViolation and no new APL grammar — the violation type stays stable and reason-only denies are unchanged. A host (e.g. the Praxis policy filter) reads details to render a custom denial response; absent → host default behavior. Signed-off-by: Frederico Araujo --- crates/apl-core/src/lib.rs | 3 +- crates/apl-core/src/rules.rs | 55 ++++++++++++++++++++++++++++ crates/apl-cpex/src/route_handler.rs | 19 ++++++++++ crates/apl-cpex/src/visitor.rs | 49 ++++++++++++++++++++++++- 4 files changed, 123 insertions(+), 3 deletions(-) diff --git a/crates/apl-core/src/lib.rs b/crates/apl-core/src/lib.rs index 48d12e7c..e4ed02e5 100644 --- a/crates/apl-core/src/lib.rs +++ b/crates/apl-core/src/lib.rs @@ -36,7 +36,8 @@ pub use plugin_decl::{ }; pub use route::{evaluate_post, evaluate_pre, evaluate_route, RouteDecision, RoutePayload}; pub use rules::{ - CompareOp, CompiledRoute, Condition, Effect, Expression, Literal, Phase, PhaseSet, Rule, + CompareOp, CompiledRoute, Condition, DenyResponse, Effect, Expression, Literal, Phase, + PhaseSet, Rule, }; pub use step::{ delegation_bag_keys, DelegateStep, DelegationError, DelegationInvoker, DelegationOutcome, diff --git a/crates/apl-core/src/rules.rs b/crates/apl-core/src/rules.rs index 557765fa..50247bdc 100644 --- a/crates/apl-core/src/rules.rs +++ b/crates/apl-core/src/rules.rs @@ -405,6 +405,25 @@ impl PhaseSet { } } +/// Custom response to attach when a route's policy denies — the +/// transpiled form of a Kuadrant `AuthPolicy` `response.unauthorized` +/// `denyWith`. Carried on the route and surfaced on the deny outcome's +/// `details` map by the host (apl-cpex), so a host can render a custom +/// HTTP response. All fields optional; an absent block leaves the host's +/// default denial response unchanged. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +pub struct DenyResponse { + /// HTTP status to use for the denial (e.g. 403, 302). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub status: Option, + /// Response body. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub body: Option, + /// Response headers (e.g. `Location` for a redirect, `WWW-Authenticate`). + #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")] + pub headers: std::collections::BTreeMap, +} + /// Compiler output for a single route. /// /// One `CompiledRoute` per route_key. The compiler merges global / default / @@ -434,6 +453,11 @@ pub struct CompiledRoute { /// hooks/kind/source always come from the global declaration. #[serde(default, skip_serializing_if = "std::collections::HashMap::is_empty")] pub plugin_overrides: std::collections::HashMap, + + /// Custom denial response (transpiled `denyWith`). Most-specific layer + /// wins on collision. `None` leaves the host's default denial behavior. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub response: Option, } impl CompiledRoute { @@ -515,6 +539,11 @@ impl CompiledRoute { // plugin_overrides: HashMap::extend overwrites on key collision, // which is exactly the more_specific-wins semantic. self.plugin_overrides.extend(more_specific.plugin_overrides); + + // response: most-specific declared block wins; absent leaves self's. + if more_specific.response.is_some() { + self.response = more_specific.response; + } } } @@ -522,6 +551,32 @@ impl CompiledRoute { mod tests { use super::*; + #[test] + fn apply_layer_response_most_specific_wins() { + let mut base = CompiledRoute::new("tool:x"); + base.response = Some(DenyResponse { + status: Some(401), + ..Default::default() + }); + + let mut layer = CompiledRoute::new("tool:x"); + layer.response = Some(DenyResponse { + status: Some(403), + body: Some("forbidden".to_string()), + ..Default::default() + }); + base.apply_layer(layer); + assert_eq!(base.response.as_ref().unwrap().status, Some(403)); + assert_eq!( + base.response.as_ref().unwrap().body.as_deref(), + Some("forbidden") + ); + + // A layer without a response leaves the existing one intact. + base.apply_layer(CompiledRoute::new("tool:x")); + assert_eq!(base.response.as_ref().unwrap().status, Some(403)); + } + #[test] fn phase_set_basic() { let mut set = PhaseSet::new(); diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index 16cf4cf6..9371bda8 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -424,6 +424,25 @@ impl AnyHookHandler for AplRouteHandler { }, }; + // Attach the route's transpiled `denyWith` (status/body/headers) to + // the violation's `details` map so the host can render a custom HTTP + // denial response. Carried via `details` (not new violation fields) + // to keep the violation type stable. Absent → host default response. + if let (Some(v), Some(resp)) = (violation.as_mut(), self.route.response.as_ref()) { + if let Some(status) = resp.status { + v.details + .insert("http.status".to_string(), serde_json::json!(status)); + } + if let Some(body) = &resp.body { + v.details + .insert("http.body".to_string(), serde_json::json!(body)); + } + if !resp.headers.is_empty() { + v.details + .insert("http.headers".to_string(), serde_json::json!(resp.headers)); + } + } + // Append fail-closed (R18) with merge precedence: // - decision Allow + append Err → flip to Deny with a // distinguished `session.persist_failed` violation. diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index ccb07b17..d8b25ce8 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -63,7 +63,7 @@ use cpex_core::visitor::{ConfigVisitor, VisitorError}; use apl_core::parser::compile_policy_block_value; use apl_core::plugin_decl::{PluginDeclaration, PluginRegistry}; -use apl_core::rules::CompiledRoute; +use apl_core::rules::{CompiledRoute, DenyResponse}; use apl_core::step::{PdpFactory, PdpResolver}; use crate::dispatch_plan::DispatchCache; @@ -518,6 +518,13 @@ impl ConfigVisitor for AplConfigVisitor { effective.apply_layer(route_layer); } + // Route-level denial response (transpiled `denyWith`). Read from + // the route YAML alongside the APL block; cpex-core tolerates the + // out-of-band key. Route scope is most-specific, so set directly. + if let Some(resp) = response_subblock(yaml, &route_key) { + effective.response = Some(resp); + } + // Load-time lint, once per route: flag any APL `plugins:` // override declared for a plugin that no policy / delegate step // references. Checked on the fully-stacked `effective` route so @@ -880,14 +887,52 @@ fn apl_subblock(yaml: &serde_yaml::Value) -> Option { } } +/// Extract a route-level `response:` block — the transpiled `denyWith`. +/// cpex-core tolerates this out-of-band key on the route; here we +/// deserialize it into a [`DenyResponse`]. A malformed block is logged +/// and skipped (best-effort) rather than failing the whole config. +fn response_subblock(yaml: &serde_yaml::Value, route_key: &str) -> Option { + let block = yaml.get("response")?; + if block.is_null() { + return None; + } + match serde_yaml::from_value::(block.clone()) { + Ok(resp) => Some(resp), + Err(e) => { + tracing::warn!(route = route_key, error = %e, "APL visitor: ignoring malformed route `response:` block"); + None + }, + } +} + #[cfg(test)] mod tests { - use super::apl_subblock; + use super::{apl_subblock, response_subblock}; fn yaml(s: &str) -> serde_yaml::Value { serde_yaml::from_str(s).expect("valid yaml") } + #[test] + fn response_subblock_parses_denywith() { + let v = yaml( + "tool: \"*\"\nresponse:\n status: 403\n body: \"{\\\"error\\\":\\\"forbidden\\\"}\"\n headers:\n WWW-Authenticate: \"Bearer\"\n", + ); + let resp = response_subblock(&v, "tool:*").expect("response present"); + assert_eq!(resp.status, Some(403)); + assert_eq!(resp.body.as_deref(), Some("{\"error\":\"forbidden\"}")); + assert_eq!( + resp.headers.get("WWW-Authenticate").map(String::as_str), + Some("Bearer") + ); + } + + #[test] + fn response_subblock_absent_is_none() { + let v = yaml("tool: \"*\"\npolicy:\n - \"deny\"\n"); + assert!(response_subblock(&v, "tool:*").is_none()); + } + #[test] fn apl_wrapper_is_returned_as_is() { let v = yaml("apl:\n policy:\n - \"deny\"\n"); From a3334e2c4110328c38fa771b590328744422cc90 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Tue, 30 Jun 2026 17:13:52 -0400 Subject: [PATCH 3/8] feat(apl): evaluate the global policy for entity-less HTTP requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the catch-all `global` policy enforce on generic (non-MCP/A2A) HTTP requests, which carry no entity (Praxis spike Phase B / U3). - New reserved coordinates: ENTITY_HTTP ("http") / ENTITY_NAME_GLOBAL ("*") and the HOOK_CMF_HTTP_REQUEST ("cmf.http_request") hook. - The visitor installs a Pre-phase AplRouteHandler bound to the compiled global policy under those coordinates, granted read_headers so the policy can read the request line/headers. Entity routes still stack `global` via apply_layer; this adds the entity-less evaluation path. - A global-scope `response:` block (transpiled denyWith) is carried onto the global handler and surfaced on deny via PluginViolation.details (U2). A host fires invoke_named::("cmf.http_request", ...) with meta.entity_type/name set to the reserved coordinates. End-to-end tests cover allow, deny, and custom-denyWith — exercising U1 + U2 + U3 together. Signed-off-by: Frederico Araujo --- crates/apl-cpex/src/visitor.rs | 54 ++++++++- crates/apl-cpex/tests/global_http_authz.rs | 131 +++++++++++++++++++++ crates/cpex-core/src/cmf/constants.rs | 15 +++ 3 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 crates/apl-cpex/tests/global_http_authz.rs diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index d8b25ce8..8c61db6b 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -51,10 +51,10 @@ use std::collections::HashMap; use std::sync::{Arc, RwLock, Weak}; use cpex_core::cmf::constants::{ - ENTITY_LLM, ENTITY_PROMPT, ENTITY_RESOURCE, ENTITY_TOOL, HOOK_CMF_LLM_INPUT, - HOOK_CMF_LLM_OUTPUT, HOOK_CMF_PROMPT_POST_INVOKE, HOOK_CMF_PROMPT_PRE_INVOKE, - HOOK_CMF_RESOURCE_POST_FETCH, HOOK_CMF_RESOURCE_PRE_FETCH, HOOK_CMF_TOOL_POST_INVOKE, - HOOK_CMF_TOOL_PRE_INVOKE, + ENTITY_HTTP, ENTITY_LLM, ENTITY_NAME_GLOBAL, ENTITY_PROMPT, ENTITY_RESOURCE, ENTITY_TOOL, + HOOK_CMF_HTTP_REQUEST, HOOK_CMF_LLM_INPUT, HOOK_CMF_LLM_OUTPUT, HOOK_CMF_PROMPT_POST_INVOKE, + HOOK_CMF_PROMPT_PRE_INVOKE, HOOK_CMF_RESOURCE_POST_FETCH, HOOK_CMF_RESOURCE_PRE_FETCH, + HOOK_CMF_TOOL_POST_INVOKE, HOOK_CMF_TOOL_PRE_INVOKE, }; use cpex_core::config::RouteEntry; use cpex_core::manager::PluginManager; @@ -357,7 +357,7 @@ impl ConfigVisitor for AplConfigVisitor { fn visit_global( &self, - _mgr: &Arc, + mgr: &Arc, yaml: &serde_yaml::Value, ) -> Result<(), VisitorError> { let Some(apl_block) = apl_subblock(yaml) else { @@ -386,8 +386,50 @@ impl ConfigVisitor for AplConfigVisitor { // `post_policy:` / `args:` / `result:` / `plugins:` (and inert // fields it ignores), so a shallow strip on a clone is enough. let policy_only = strip_non_dsl_keys(&apl_block); - let compiled = compile_policy_block_value("global.apl", &policy_only) + let mut compiled = compile_policy_block_value("global.apl", &policy_only) .map_err(|e| Box::new(e) as VisitorError)?; + // A `response:` block at the global scope is the catch-all denyWith. + compiled.response = response_subblock(yaml, "global"); + + // Install a catch-all handler so the global policy also evaluates for + // generic (non-MCP/A2A) HTTP requests, which carry no entity (U3). + // Entity routes still stack `global` via apply_layer in visit_route; + // this is the *entity-less* evaluation path. Pre-phase only — + // authorization is an admission check, so there is no post handler. + if !compiled.policy.is_empty() { + let (plugin_registry, pdp_router_arc) = { + let state = self.state.read().unwrap_or_else(|p| p.into_inner()); + ( + Arc::new(state.plugin_registry.clone()), + Arc::new(state.pdp_router.clone()) as Arc, + ) + }; + let session_store = self + .session_store + .read() + .unwrap_or_else(|p| p.into_inner()) + .clone(); + // The global HTTP policy reads the request line / headers, so + // grant `read_headers` on top of the visitor baseline. + let mut caps = self.base_capabilities.clone(); + caps.insert("read_headers".to_string()); + install_handler( + mgr, + ENTITY_HTTP, + ENTITY_NAME_GLOBAL, + None, + HOOK_CMF_HTTP_REQUEST, + Phase::Pre, + Arc::new(compiled.clone()), + &plugin_registry, + &self.dispatch_cache, + &session_store, + &self.manager, + Some(pdp_router_arc), + &caps, + ); + } + self.state .write() .unwrap_or_else(|p| p.into_inner()) diff --git a/crates/apl-cpex/tests/global_http_authz.rs b/crates/apl-cpex/tests/global_http_authz.rs new file mode 100644 index 00000000..90909c85 --- /dev/null +++ b/crates/apl-cpex/tests/global_http_authz.rs @@ -0,0 +1,131 @@ +// Location: ./crates/apl-cpex/tests/global_http_authz.rs +// Copyright 2026 +// SPDX-License-Identifier: Apache-2.0 +// Authors: Fred Araujo +// +// End-to-end: a `global` APL policy is evaluated for a generic +// (non-MCP/A2A) HTTP request that carries no entity. The visitor installs +// a catch-all handler under (ENTITY_HTTP, ENTITY_NAME_GLOBAL, +// HOOK_CMF_HTTP_REQUEST); the host fires that hook with `meta` set to the +// reserved coordinates and an `http` extension carrying the request line. +// This is the entity-less authorization path the Praxis AuthPolicy +// transpiler targets (spike Phase B / U3). It also exercises U1 +// (http.method in the bag) and U2 (custom denyWith via the route +// `response:` block surfaced on the violation details). + +use std::sync::Arc; + +use cpex_core::cmf::constants::{ENTITY_HTTP, ENTITY_NAME_GLOBAL, HOOK_CMF_HTTP_REQUEST}; +use cpex_core::cmf::enums::Role; +use cpex_core::cmf::{CmfHook, Message, MessagePayload}; +use cpex_core::extensions::{Extensions, HttpExtension, MetaExtension}; +use cpex_core::manager::PluginManager; + +use apl_cpex::{register_apl, AplOptions}; + +async fn manager_with(yaml: &str) -> Arc { + let mgr = Arc::new(PluginManager::default()); + register_apl(&mgr, AplOptions::in_process()); + mgr.load_config_yaml(yaml).expect("load_config_yaml"); + mgr.initialize().await.expect("initialize"); + mgr +} + +/// A generic-HTTP request: reserved entity coordinates + an `http` +/// extension carrying the request method. +fn http_request(method: &str) -> Extensions { + let mut meta = MetaExtension::default(); + meta.entity_type = Some(ENTITY_HTTP.to_string()); + meta.entity_name = Some(ENTITY_NAME_GLOBAL.to_string()); + let http = HttpExtension { + method: Some(method.to_string()), + ..Default::default() + }; + Extensions { + meta: Some(Arc::new(meta)), + http: Some(Arc::new(http)), + ..Default::default() + } +} + +fn payload() -> MessagePayload { + MessagePayload { + message: Message::text(Role::User, "hi"), + } +} + +// APL predicate:action form: deny when the method is not GET. (Comparisons +// use this form; `require(...)` is truthiness-only.) +const GET_ONLY: &str = r#" +plugin_settings: + routing_enabled: true +global: + apl: + policy: + - "http.method != 'GET': deny" +"#; + +#[tokio::test] +async fn global_policy_allows_matching_http_request() { + let mgr = manager_with(GET_ONLY).await; + let (res, _bg) = mgr + .invoke_named::(HOOK_CMF_HTTP_REQUEST, payload(), http_request("GET"), None) + .await; + assert!( + res.continue_processing, + "GET must be allowed by the global policy; violation = {:?}", + res.violation + ); +} + +#[tokio::test] +async fn global_policy_denies_nonmatching_http_request() { + let mgr = manager_with(GET_ONLY).await; + let (res, _bg) = mgr + .invoke_named::(HOOK_CMF_HTTP_REQUEST, payload(), http_request("POST"), None) + .await; + assert!( + !res.continue_processing, + "POST must be denied by the global policy" + ); +} + +/// A route-level `response:` block (transpiled `denyWith`) surfaces custom +/// status/body/headers on the violation `details` map (U2) when the global +/// policy denies. +#[tokio::test] +async fn global_policy_deny_carries_custom_response() { + const YAML: &str = r#" +plugin_settings: + routing_enabled: true +global: + apl: + policy: + - "http.method != 'GET': deny" + response: + status: 403 + body: "{\"error\":\"forbidden\"}" + headers: + X-Reason: "method-not-allowed" +"#; + let mgr = manager_with(YAML).await; + let (res, _bg) = mgr + .invoke_named::( + HOOK_CMF_HTTP_REQUEST, + payload(), + http_request("DELETE"), + None, + ) + .await; + assert!(!res.continue_processing, "DELETE must be denied"); + let v = res.violation.expect("deny must surface a violation"); + assert_eq!(v.details.get("http.status"), Some(&serde_json::json!(403))); + assert_eq!( + v.details.get("http.body"), + Some(&serde_json::json!("{\"error\":\"forbidden\"}")) + ); + assert_eq!( + v.details.get("http.headers"), + Some(&serde_json::json!({ "X-Reason": "method-not-allowed" })) + ); +} diff --git a/crates/cpex-core/src/cmf/constants.rs b/crates/cpex-core/src/cmf/constants.rs index 454ec7b2..1be8e581 100644 --- a/crates/cpex-core/src/cmf/constants.rs +++ b/crates/cpex-core/src/cmf/constants.rs @@ -76,6 +76,15 @@ pub const ENTITY_LLM: &str = "llm"; pub const ENTITY_PROMPT: &str = "prompt"; pub const ENTITY_RESOURCE: &str = "resource"; +/// Reserved entity type for generic (non-MCP/A2A) HTTP requests. The +/// catch-all `global` policy is dispatched under this entity so an +/// entity-less request can be authorized; hosts set `meta.entity_type` to +/// this and `meta.entity_name` to [`ENTITY_NAME_GLOBAL`]. +pub const ENTITY_HTTP: &str = "http"; + +/// Reserved entity name for the global catch-all policy annotation. +pub const ENTITY_NAME_GLOBAL: &str = "*"; + // --------------------------------------------------------------------------- // CMF hook names — the canonical names plugins register under and hosts // pass to `PluginManager::invoke_named::(...)`. Two per entity @@ -94,3 +103,9 @@ pub const HOOK_CMF_PROMPT_PRE_INVOKE: &str = "cmf.prompt_pre_invoke"; pub const HOOK_CMF_PROMPT_POST_INVOKE: &str = "cmf.prompt_post_invoke"; pub const HOOK_CMF_RESOURCE_PRE_FETCH: &str = "cmf.resource_pre_fetch"; pub const HOOK_CMF_RESOURCE_POST_FETCH: &str = "cmf.resource_post_fetch"; + +/// Generic HTTP request hook. Hosts fire this for non-MCP/A2A HTTP +/// requests; the catch-all `global` policy (if any) is annotated under +/// it via [`ENTITY_HTTP`] / [`ENTITY_NAME_GLOBAL`]. Pre-invocation only — +/// authorization is an admission check, so there is no post counterpart. +pub const HOOK_CMF_HTTP_REQUEST: &str = "cmf.http_request"; From 4f03fdb76497ce75854eef6a0d7174cbf91d0b1b Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 4 Jul 2026 10:56:49 -0300 Subject: [PATCH 4/8] fix(apl): scope denyWith response to its owning scope Address review feedback on the generic-HTTP authorization PR: - Stop apply_layer from propagating `response`, so a `global` catch-all denyWith no longer leaks onto inherited entity (tool/llm/prompt/resource) denials with no opt-out. - Decorate only genuine denials via a shared decorate_denial_response helper, and apply it at the session load/persist fail-closed sites too (previously they rendered the default shape). - Warn when `response:` sits at default/policy-bundle scope, where it is inert, instead of dropping it silently. - Parse the route `response:` once above the per-entity loop. - Extract snapshot_dispatch_state to share the registry/router/store read between the global and per-route handler installs. - Promote the http.status/body/headers detail keys to DETAIL_HTTP_* constants shared by producer and consumer. Signed-off-by: Frederico Araujo --- crates/apl-cmf/src/constants.rs | 7 ++ crates/apl-core/src/rules.rs | 36 ++++--- crates/apl-cpex/src/route_handler.rs | 78 ++++++++++----- crates/apl-cpex/src/visitor.rs | 109 +++++++++++++-------- crates/apl-cpex/tests/global_http_authz.rs | 86 +++++++++++++++- 5 files changed, 229 insertions(+), 87 deletions(-) diff --git a/crates/apl-cmf/src/constants.rs b/crates/apl-cmf/src/constants.rs index 85e0f096..d1e30619 100644 --- a/crates/apl-cmf/src/constants.rs +++ b/crates/apl-cmf/src/constants.rs @@ -112,6 +112,13 @@ pub const BAG_HTTP_METHOD: &str = "http.method"; pub const BAG_HTTP_PATH: &str = "http.path"; pub const BAG_HTTP_HOST: &str = "http.host"; pub const BAG_HTTP_SCHEME: &str = "http.scheme"; +// Violation `details` keys carrying a transpiled `denyWith` (custom HTTP +// denial response). Shared between the producer (apl-cpex route handler) +// and any consumer (host renderer / tests) so the stringly-typed contract +// stays coupled to one definition. +pub const DETAIL_HTTP_STATUS: &str = "http.status"; +pub const DETAIL_HTTP_BODY: &str = "http.body"; +pub const DETAIL_HTTP_HEADERS: &str = "http.headers"; pub const BAG_LLM_PREFIX: &str = "llm."; pub const BAG_MCP_PREFIX: &str = "mcp."; pub const BAG_COMPLETION_PREFIX: &str = "completion."; diff --git a/crates/apl-core/src/rules.rs b/crates/apl-core/src/rules.rs index 50247bdc..44313ee3 100644 --- a/crates/apl-core/src/rules.rs +++ b/crates/apl-core/src/rules.rs @@ -540,10 +540,13 @@ impl CompiledRoute { // which is exactly the more_specific-wins semantic. self.plugin_overrides.extend(more_specific.plugin_overrides); - // response: most-specific declared block wins; absent leaves self's. - if more_specific.response.is_some() { - self.response = more_specific.response; - } + // response: deliberately NOT layered. A custom denial response is + // scope-local — the entity-less HTTP handler carries the `global` + // block directly, and an entity route carries only its own + // `response:`. Propagating it here let a `global` catch-all + // `denyWith` leak onto every inherited entity (MCP tool / llm / + // prompt / resource) denial with no way to opt back out. Callers + // set `response` explicitly at the scope that owns it. } } @@ -552,7 +555,9 @@ mod tests { use super::*; #[test] - fn apply_layer_response_most_specific_wins() { + fn apply_layer_does_not_propagate_response() { + // `response` is scope-local and must never cross a layer boundary — + // a `global` catch-all denyWith must not leak onto entity routes. let mut base = CompiledRoute::new("tool:x"); base.response = Some(DenyResponse { status: Some(401), @@ -566,15 +571,18 @@ mod tests { ..Default::default() }); base.apply_layer(layer); - assert_eq!(base.response.as_ref().unwrap().status, Some(403)); - assert_eq!( - base.response.as_ref().unwrap().body.as_deref(), - Some("forbidden") - ); - - // A layer without a response leaves the existing one intact. - base.apply_layer(CompiledRoute::new("tool:x")); - assert_eq!(base.response.as_ref().unwrap().status, Some(403)); + // base keeps its own response; the layer's is dropped. + assert_eq!(base.response.as_ref().unwrap().status, Some(401)); + + // A layer's response never populates an empty base either. + let mut empty = CompiledRoute::new("tool:x"); + let mut with_resp = CompiledRoute::new("tool:x"); + with_resp.response = Some(DenyResponse { + status: Some(418), + ..Default::default() + }); + empty.apply_layer(with_resp); + assert!(empty.response.is_none()); } #[test] diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index 9371bda8..8d007769 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -40,11 +40,12 @@ use cpex_core::manager::PluginManager; use cpex_core::plugin::{Plugin, PluginConfig}; use cpex_core::registry::AnyHookHandler; +use apl_cmf::constants::{DETAIL_HTTP_BODY, DETAIL_HTTP_HEADERS, DETAIL_HTTP_STATUS}; use apl_cmf::{extract_args, extract_result, BagBuilder}; use apl_core::evaluator::Decision; use apl_core::plugin_decl::PluginRegistry; use apl_core::route::{evaluate_post, evaluate_pre, RoutePayload}; -use apl_core::rules::CompiledRoute; +use apl_core::rules::{CompiledRoute, DenyResponse}; use apl_core::step::PdpResolver; use crate::cmf_invoker::CmfPluginInvoker; @@ -220,14 +221,16 @@ impl AnyHookHandler for AplRouteHandler { error = %e, "session label load failed; failing request closed" ); + let mut v = PluginViolation::new( + "session.load_failed", + "session state could not be loaded", + ); + decorate_denial_response(&mut v, self.route.response.as_ref()); return Ok(Box::new(ErasedResultFields { continue_processing: false, modified_payload: None, modified_extensions: None, - violation: Some(PluginViolation::new( - "session.load_failed", - "session state could not be loaded", - )), + violation: Some(v), })); }, }; @@ -408,6 +411,12 @@ impl AnyHookHandler for AplRouteHandler { None }; + // Attach the route's transpiled `denyWith` to a violation at each + // genuine-denial site (below) via `decorate_denial_response`, rather + // than blanket-decorating whatever `violation` is set. This keeps the + // custom response off any future non-denial signal (e.g. an + // elicitation/retry/confirm violation) that must reach the host with + // its own wire shape intact. let (mut continue_processing, mut violation) = match decision.decision { Decision::Allow => (true, None), Decision::Deny { @@ -420,29 +429,12 @@ impl AnyHookHandler for AplRouteHandler { rule_source }; let reason = reason.unwrap_or_else(|| "access denied".to_string()); - (false, Some(PluginViolation::new(code, reason))) + let mut v = PluginViolation::new(code, reason); + decorate_denial_response(&mut v, self.route.response.as_ref()); + (false, Some(v)) }, }; - // Attach the route's transpiled `denyWith` (status/body/headers) to - // the violation's `details` map so the host can render a custom HTTP - // denial response. Carried via `details` (not new violation fields) - // to keep the violation type stable. Absent → host default response. - if let (Some(v), Some(resp)) = (violation.as_mut(), self.route.response.as_ref()) { - if let Some(status) = resp.status { - v.details - .insert("http.status".to_string(), serde_json::json!(status)); - } - if let Some(body) = &resp.body { - v.details - .insert("http.body".to_string(), serde_json::json!(body)); - } - if !resp.headers.is_empty() { - v.details - .insert("http.headers".to_string(), serde_json::json!(resp.headers)); - } - } - // Append fail-closed (R18) with merge precedence: // - decision Allow + append Err → flip to Deny with a // distinguished `session.persist_failed` violation. @@ -463,10 +455,12 @@ impl AnyHookHandler for AplRouteHandler { ); if continue_processing { continue_processing = false; - violation = Some(PluginViolation::new( + let mut v = PluginViolation::new( "session.persist_failed", "session state could not be persisted", - )); + ); + decorate_denial_response(&mut v, self.route.response.as_ref()); + violation = Some(v); } } @@ -489,6 +483,36 @@ impl AnyHookHandler for AplRouteHandler { // Helpers // ===================================================================== +/// Attach a route's transpiled `denyWith` (status/body/headers) to a +/// denial `violation`'s `details` map so the host can render a custom HTTP +/// denial response. Carried via `details` (not new violation fields) to +/// keep the violation type stable. `None` response leaves the host default. +/// +/// Call this only from genuine-denial sites — never blanket-apply it to +/// whatever violation happens to be set, or a non-denial signal (e.g. an +/// elicitation/retry/confirm) would get stamped with a `403`-shaped +/// response the host would render instead of the intended wire signal. +fn decorate_denial_response(violation: &mut PluginViolation, response: Option<&DenyResponse>) { + let Some(resp) = response else { + return; + }; + if let Some(status) = resp.status { + violation + .details + .insert(DETAIL_HTTP_STATUS.to_string(), serde_json::json!(status)); + } + if let Some(body) = &resp.body { + violation + .details + .insert(DETAIL_HTTP_BODY.to_string(), serde_json::json!(body)); + } + if !resp.headers.is_empty() { + violation + .details + .insert(DETAIL_HTTP_HEADERS.to_string(), serde_json::json!(resp.headers)); + } +} + /// Rewrite the first text part of `msg` with `new_text`. If there is no /// text part, append one. Mirrors what `MessagePayload`'s normal /// modify-path does for single-view v0. diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index 8c61db6b..2b7478b7 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -286,6 +286,34 @@ impl AplConfigVisitor { state.pdp_router.register(resolver); Ok(()) } + + /// Snapshot the request-time dispatch state — plugin registry, PDP + /// router, and active session store — each `Arc`-wrapped for a handler + /// to capture. Reads the visitor's `RwLock`s once through a single + /// poison-recovery path shared by both handler-install sites + /// (`visit_global`'s entity-less HTTP handler and `visit_route`'s + /// per-entity handlers) so the policy can't diverge between them. + fn snapshot_dispatch_state( + &self, + ) -> ( + Arc, + Arc, + Arc, + ) { + let (plugin_registry, pdp_router_arc) = { + let state = self.state.read().unwrap_or_else(|p| p.into_inner()); + ( + Arc::new(state.plugin_registry.clone()), + Arc::new(state.pdp_router.clone()) as Arc, + ) + }; + let session_store = self + .session_store + .read() + .unwrap_or_else(|p| p.into_inner()) + .clone(); + (plugin_registry, pdp_router_arc, session_store) + } } /// Read-only baseline for APL predicates: enough to make @@ -397,18 +425,7 @@ impl ConfigVisitor for AplConfigVisitor { // this is the *entity-less* evaluation path. Pre-phase only — // authorization is an admission check, so there is no post handler. if !compiled.policy.is_empty() { - let (plugin_registry, pdp_router_arc) = { - let state = self.state.read().unwrap_or_else(|p| p.into_inner()); - ( - Arc::new(state.plugin_registry.clone()), - Arc::new(state.pdp_router.clone()) as Arc, - ) - }; - let session_store = self - .session_store - .read() - .unwrap_or_else(|p| p.into_inner()) - .clone(); + let (plugin_registry, pdp_router_arc, session_store) = self.snapshot_dispatch_state(); // The global HTTP policy reads the request line / headers, so // grant `read_headers` on top of the visitor baseline. let mut caps = self.base_capabilities.clone(); @@ -443,6 +460,7 @@ impl ConfigVisitor for AplConfigVisitor { entity_type: &str, yaml: &serde_yaml::Value, ) -> Result<(), VisitorError> { + warn_if_response_at_unsupported_scope(yaml, &format!("global.defaults.{entity_type}")); let Some(apl_block) = apl_subblock(yaml) else { return Ok(()); }; @@ -464,6 +482,7 @@ impl ConfigVisitor for AplConfigVisitor { tag: &str, yaml: &serde_yaml::Value, ) -> Result<(), VisitorError> { + warn_if_response_at_unsupported_scope(yaml, &format!("global.policies.{tag}")); let Some(apl_block) = apl_subblock(yaml) else { return Ok(()); }; @@ -508,21 +527,21 @@ impl ConfigVisitor for AplConfigVisitor { .map(|m| m.tags.clone()) .unwrap_or_default(); - // Snapshot the plugin registry + PDP router once outside the - // per-entity loop. `visit_plugins` populated the registry - // before any `visit_route` call; the router has been populated - // by code-supplied `register_pdp` calls + `visit_global` - // factory dispatch. Routes share both, so cloning each into an - // `Arc` once and handing clones to each handler is cheaper than - // re-reading the RwLock per entity. Cloning `PdpRouter` is - // refcount bumps on each inner resolver — cheap. - let (plugin_registry, pdp_router_arc) = { - let state = self.state.read().unwrap_or_else(|p| p.into_inner()); - ( - Arc::new(state.plugin_registry.clone()), - Arc::new(state.pdp_router.clone()) as Arc, - ) - }; + // Snapshot the dispatch state once outside the per-entity loop. + // `visit_plugins` populated the registry before any `visit_route` + // call; the router + session store were finalized in `visit_global`. + // Routes share all three, so cloning each into an `Arc` once and + // handing clones to each handler is cheaper than re-reading the + // RwLocks per entity. Cloning `PdpRouter` is refcount bumps on each + // inner resolver — cheap. + let (plugin_registry, pdp_router_arc, session_store) = self.snapshot_dispatch_state(); + + // Route-level denial response (transpiled `denyWith`) — parsed once; + // its input (`yaml`) is loop-invariant across the entity names this + // route matches, so hoisting avoids re-deserializing (and + // re-warning) once per entity. `response` is scope-local: an entity + // route carries only its own block, never an inherited `global` one. + let route_response = response_subblock(yaml, &format!("routes.{entity_type}")); for (idx, entity_name) in entity_names.iter().enumerate() { // route_key is what `DispatchCache` keys on, so it must @@ -560,12 +579,12 @@ impl ConfigVisitor for AplConfigVisitor { effective.apply_layer(route_layer); } - // Route-level denial response (transpiled `denyWith`). Read from - // the route YAML alongside the APL block; cpex-core tolerates the - // out-of-band key. Route scope is most-specific, so set directly. - if let Some(resp) = response_subblock(yaml, &route_key) { - effective.response = Some(resp); - } + // Route-level denial response (transpiled `denyWith`), parsed + // above the loop. Route scope is most-specific and inheritance + // was removed, so this is the only source of `response` for an + // entity route — a malformed or absent block leaves it `None` + // (host default denial), never a leaked `global` response. + effective.response = route_response.clone(); // Load-time lint, once per route: flag any APL `plugins:` // override declared for a plugin that no policy / delegate step @@ -621,16 +640,6 @@ impl ConfigVisitor for AplConfigVisitor { }, }; - // Snapshot the active session store (a `global.apl.session_store` - // block in `visit_global` may have swapped it). Each handler - // captures its own clone, so request-time dispatch never touches - // the visitor's lock. - let session_store = self - .session_store - .read() - .unwrap_or_else(|p| p.into_inner()) - .clone(); - // Install Pre + Post handlers. Each handler instance is bound to // ONE phase so the executor can pick the right entry-point off // the (entity_type, entity_name, scope, hook_name) key. @@ -933,6 +942,20 @@ fn apl_subblock(yaml: &serde_yaml::Value) -> Option { /// cpex-core tolerates this out-of-band key on the route; here we /// deserialize it into a [`DenyResponse`]. A malformed block is logged /// and skipped (best-effort) rather than failing the whole config. +/// Warn when a `response:` block appears at a scope that never renders it. +/// A custom denial response is honored only at `global` (the entity-less +/// HTTP path) or on a route; at `default` / policy-bundle scope it is inert +/// — there is no propagation path to a handler. Mirrors the existing +/// global-only-key lint so a misplaced `response:` fails loud, not silent. +fn warn_if_response_at_unsupported_scope(yaml: &serde_yaml::Value, scope: &str) { + if yaml.get("response").is_some_and(|v| !v.is_null()) { + tracing::warn!( + scope, + "APL visitor: `response:` is honored only at `global` or route scope; ignoring here", + ); + } +} + fn response_subblock(yaml: &serde_yaml::Value, route_key: &str) -> Option { let block = yaml.get("response")?; if block.is_null() { diff --git a/crates/apl-cpex/tests/global_http_authz.rs b/crates/apl-cpex/tests/global_http_authz.rs index 90909c85..24b482e6 100644 --- a/crates/apl-cpex/tests/global_http_authz.rs +++ b/crates/apl-cpex/tests/global_http_authz.rs @@ -21,6 +21,7 @@ use cpex_core::cmf::{CmfHook, Message, MessagePayload}; use cpex_core::extensions::{Extensions, HttpExtension, MetaExtension}; use cpex_core::manager::PluginManager; +use apl_cmf::constants::{DETAIL_HTTP_BODY, DETAIL_HTTP_HEADERS, DETAIL_HTTP_STATUS}; use apl_cpex::{register_apl, AplOptions}; async fn manager_with(yaml: &str) -> Arc { @@ -54,6 +55,17 @@ fn payload() -> MessagePayload { } } +/// An MCP tool-call request: `meta` naming a `tool` entity, no `http` ext. +fn tool_request(name: &str) -> Extensions { + let mut meta = MetaExtension::default(); + meta.entity_type = Some("tool".to_string()); + meta.entity_name = Some(name.to_string()); + Extensions { + meta: Some(Arc::new(meta)), + ..Default::default() + } +} + // APL predicate:action form: deny when the method is not GET. (Comparisons // use this form; `require(...)` is truthiness-only.) const GET_ONLY: &str = r#" @@ -119,13 +131,81 @@ global: .await; assert!(!res.continue_processing, "DELETE must be denied"); let v = res.violation.expect("deny must surface a violation"); - assert_eq!(v.details.get("http.status"), Some(&serde_json::json!(403))); assert_eq!( - v.details.get("http.body"), + v.details.get(DETAIL_HTTP_STATUS), + Some(&serde_json::json!(403)) + ); + assert_eq!( + v.details.get(DETAIL_HTTP_BODY), Some(&serde_json::json!("{\"error\":\"forbidden\"}")) ); assert_eq!( - v.details.get("http.headers"), + v.details.get(DETAIL_HTTP_HEADERS), Some(&serde_json::json!({ "X-Reason": "method-not-allowed" })) ); } + +/// A `global` `response:` (the entity-less HTTP catch-all denyWith) must NOT +/// be inherited by an entity route. A denied MCP tool call gets the plain +/// violation shape — no `http.*` details leaked from the global block. +#[tokio::test] +async fn global_response_does_not_leak_onto_entity_denial() { + const YAML: &str = r#" +plugin_settings: + routing_enabled: true +global: + apl: + policy: + - "require(authenticated)" + response: + status: 403 + body: "{\"error\":\"global\"}" +routes: + - tool: locked + apl: + policy: + - "require(authenticated)" +"#; + let mgr = manager_with(YAML).await; + let (res, _bg) = mgr + .invoke_named::("cmf.tool_pre_invoke", payload(), tool_request("locked"), None) + .await; + assert!(!res.continue_processing, "tool policy must deny"); + let v = res.violation.expect("deny must surface a violation"); + assert!( + v.details.get(DETAIL_HTTP_STATUS).is_none() + && v.details.get(DETAIL_HTTP_BODY).is_none() + && v.details.get(DETAIL_HTTP_HEADERS).is_none(), + "global response leaked onto entity denial: {:?}", + v.details + ); +} + +/// A route-scoped `response:` still decorates that route's own denial — the +/// feature works per-route; only silent inheritance was removed. +#[tokio::test] +async fn route_scoped_response_still_decorates_entity_denial() { + const YAML: &str = r#" +plugin_settings: + routing_enabled: true +routes: + - tool: locked + apl: + policy: + - "require(authenticated)" + response: + status: 401 + body: "route" +"#; + let mgr = manager_with(YAML).await; + let (res, _bg) = mgr + .invoke_named::("cmf.tool_pre_invoke", payload(), tool_request("locked"), None) + .await; + assert!(!res.continue_processing, "tool policy must deny"); + let v = res.violation.expect("deny must surface a violation"); + assert_eq!( + v.details.get(DETAIL_HTTP_STATUS), + Some(&serde_json::json!(401)) + ); + assert_eq!(v.details.get(DETAIL_HTTP_BODY), Some(&serde_json::json!("route"))); +} From 36820f02ec442b6626c64e1ab1c7bd17e1e61afc Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 4 Jul 2026 20:31:41 -0300 Subject: [PATCH 5/8] style(apl): rustfmt + clippy fixes for denyWith tests Signed-off-by: Frederico Araujo --- crates/apl-cpex/src/route_handler.rs | 7 +++--- crates/apl-cpex/tests/global_http_authz.rs | 25 ++++++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index 8d007769..e267d07a 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -507,9 +507,10 @@ fn decorate_denial_response(violation: &mut PluginViolation, response: Option<&D .insert(DETAIL_HTTP_BODY.to_string(), serde_json::json!(body)); } if !resp.headers.is_empty() { - violation - .details - .insert(DETAIL_HTTP_HEADERS.to_string(), serde_json::json!(resp.headers)); + violation.details.insert( + DETAIL_HTTP_HEADERS.to_string(), + serde_json::json!(resp.headers), + ); } } diff --git a/crates/apl-cpex/tests/global_http_authz.rs b/crates/apl-cpex/tests/global_http_authz.rs index 24b482e6..96235c59 100644 --- a/crates/apl-cpex/tests/global_http_authz.rs +++ b/crates/apl-cpex/tests/global_http_authz.rs @@ -168,14 +168,19 @@ routes: "#; let mgr = manager_with(YAML).await; let (res, _bg) = mgr - .invoke_named::("cmf.tool_pre_invoke", payload(), tool_request("locked"), None) + .invoke_named::( + "cmf.tool_pre_invoke", + payload(), + tool_request("locked"), + None, + ) .await; assert!(!res.continue_processing, "tool policy must deny"); let v = res.violation.expect("deny must surface a violation"); assert!( - v.details.get(DETAIL_HTTP_STATUS).is_none() - && v.details.get(DETAIL_HTTP_BODY).is_none() - && v.details.get(DETAIL_HTTP_HEADERS).is_none(), + !v.details.contains_key(DETAIL_HTTP_STATUS) + && !v.details.contains_key(DETAIL_HTTP_BODY) + && !v.details.contains_key(DETAIL_HTTP_HEADERS), "global response leaked onto entity denial: {:?}", v.details ); @@ -199,7 +204,12 @@ routes: "#; let mgr = manager_with(YAML).await; let (res, _bg) = mgr - .invoke_named::("cmf.tool_pre_invoke", payload(), tool_request("locked"), None) + .invoke_named::( + "cmf.tool_pre_invoke", + payload(), + tool_request("locked"), + None, + ) .await; assert!(!res.continue_processing, "tool policy must deny"); let v = res.violation.expect("deny must surface a violation"); @@ -207,5 +217,8 @@ routes: v.details.get(DETAIL_HTTP_STATUS), Some(&serde_json::json!(401)) ); - assert_eq!(v.details.get(DETAIL_HTTP_BODY), Some(&serde_json::json!("route"))); + assert_eq!( + v.details.get(DETAIL_HTTP_BODY), + Some(&serde_json::json!("route")) + ); } From 4608522f1deb00a68af372edfecae46ae418d7e8 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 4 Jul 2026 22:01:47 -0300 Subject: [PATCH 6/8] fix(apl): close fail-open gaps in entity-less HTTP catch-all Gate the catch-all handler install on args OR policy (not policy alone), so an args-only global.apl still authorizes entity-less HTTP traffic. Warn when a global response: is configured but no installable policy exists, including the bare response-only block that hit visit_global's early return. Accept response: nested under apl: as well as top-level, with top-level taking precedence (documented as deliberate). Cover the fail-closed session-store denials and the new paths with tests. Signed-off-by: Frederico Araujo --- crates/apl-cpex/src/visitor.rs | 177 +++++++++++++++++++++- crates/apl-cpex/tests/end_to_end_route.rs | 105 +++++++++++++ crates/apl-cpex/tests/visitor_e2e.rs | 36 +++++ 3 files changed, 310 insertions(+), 8 deletions(-) diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index 2b7478b7..db71f3ed 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -389,6 +389,19 @@ impl ConfigVisitor for AplConfigVisitor { yaml: &serde_yaml::Value, ) -> Result<(), VisitorError> { let Some(apl_block) = apl_subblock(yaml) else { + // No `apl:` wrapper and no flat DSL keys — there is nothing to + // compile or install. But a bare `global: { response: {...} }` + // (a denyWith with no accompanying policy) would otherwise be + // dropped here silently, before the `response_subblock` read + // below ever runs. Warn so this fail-open-by-omission case gets + // the same signal as the args/policy-empty case handled further + // down, rather than vanishing without a trace. + if response_yaml_block(yaml).is_some_and(|v| !v.is_null()) { + tracing::warn!( + "APL visitor: global.response is set but global.apl has no policy/args block \ + — the entity-less HTTP catch-all handler will not install, so this response can never fire", + ); + } return Ok(()); }; @@ -424,7 +437,14 @@ impl ConfigVisitor for AplConfigVisitor { // Entity routes still stack `global` via apply_layer in visit_route; // this is the *entity-less* evaluation path. Pre-phase only — // authorization is an admission check, so there is no post handler. - if !compiled.policy.is_empty() { + let installs_pre_handler = http_catchall_should_install(&compiled); + if !installs_pre_handler && compiled.response.is_some() { + tracing::warn!( + "APL visitor: global.response is set but global.apl has no `args:`/`policy:` steps \ + — the entity-less HTTP catch-all handler will not install, so this response can never fire", + ); + } + if installs_pre_handler { let (plugin_registry, pdp_router_arc, session_store) = self.snapshot_dispatch_state(); // The global HTTP policy reads the request line / headers, so // grant `read_headers` on top of the visitor baseline. @@ -938,17 +958,48 @@ fn apl_subblock(yaml: &serde_yaml::Value) -> Option { } } -/// Extract a route-level `response:` block — the transpiled `denyWith`. -/// cpex-core tolerates this out-of-band key on the route; here we -/// deserialize it into a [`DenyResponse`]. A malformed block is logged -/// and skipped (best-effort) rather than failing the whole config. +/// Whether the entity-less HTTP catch-all handler (Pre-phase only) should +/// install for a compiled `global` layer. Gate on both Pre-phase steps +/// (`args` + `policy`, via [`CompiledRoute::declared_phases`]), not +/// `policy` alone — an operator whose `global.apl` has only an `args:` +/// admission block (no `policy:`) must still get the catch-all installed, +/// or entity-less HTTP traffic silently bypasses it entirely (fail-open by +/// omission). +fn http_catchall_should_install(compiled: &CompiledRoute) -> bool { + let declared = compiled.declared_phases(); + declared.contains(apl_core::rules::Phase::Args) + || declared.contains(apl_core::rules::Phase::Policy) +} + +/// `response:` is not an APL DSL term (it never enters [`apl_subblock`]'s +/// [`FLAT_APL_KEYS`]) — it is documented and tested as a sibling of `apl:` +/// (`global: { apl: {...}, response: {...} }`). But an operator who mirrors +/// the `pdp:` / `session_store:` convention (which *do* work identically +/// whether flat or nested under `apl:`) may reasonably nest `response:` +/// inside `apl:` too. Accept both spellings so that mistake degrades to +/// "the other spelling wins," not "silently dropped." +/// +/// PRECEDENCE — deliberately the INVERSE of [`apl_subblock`]. `apl_subblock` +/// makes an explicit `apl:` wrapper win *entirely* over flat top-level keys +/// (for `policy:`/`pdp:`/`session_store:`); here the top-level sibling +/// `response:` wins over an `apl:`-nested one. This is intentional, not an +/// oversight: the top-level sibling is the documented, already-shipped, +/// tested form, so preferring it preserves backward compatibility, and the +/// choice can only affect the *rendered denial shape* (status/body/headers) +/// — never an Allow/Deny outcome. Do NOT "align" this with `apl_subblock`'s +/// wrapper-wins rule without a deliberate compatibility decision. +fn response_yaml_block(yaml: &serde_yaml::Value) -> Option<&serde_yaml::Value> { + yaml.get("response") + .or_else(|| yaml.get("apl").and_then(|apl| apl.get("response"))) +} + /// Warn when a `response:` block appears at a scope that never renders it. /// A custom denial response is honored only at `global` (the entity-less /// HTTP path) or on a route; at `default` / policy-bundle scope it is inert /// — there is no propagation path to a handler. Mirrors the existing /// global-only-key lint so a misplaced `response:` fails loud, not silent. fn warn_if_response_at_unsupported_scope(yaml: &serde_yaml::Value, scope: &str) { - if yaml.get("response").is_some_and(|v| !v.is_null()) { + if response_yaml_block(yaml).is_some_and(|v| !v.is_null()) { tracing::warn!( scope, "APL visitor: `response:` is honored only at `global` or route scope; ignoring here", @@ -956,8 +1007,12 @@ fn warn_if_response_at_unsupported_scope(yaml: &serde_yaml::Value, scope: &str) } } +/// Extract a route-level `response:` block — the transpiled `denyWith`. +/// cpex-core tolerates this out-of-band key on the route; here we +/// deserialize it into a [`DenyResponse`]. A malformed block is logged +/// and skipped (best-effort) rather than failing the whole config. fn response_subblock(yaml: &serde_yaml::Value, route_key: &str) -> Option { - let block = yaml.get("response")?; + let block = response_yaml_block(yaml)?; if block.is_null() { return None; } @@ -972,12 +1027,70 @@ fn response_subblock(yaml: &serde_yaml::Value, route_key: &str) -> Option serde_yaml::Value { serde_yaml::from_str(s).expect("valid yaml") } + fn deny_effect() -> Effect { + Effect::Deny { + reason: None, + code: None, + } + } + + fn field_rule(field: &str) -> FieldRule { + FieldRule { + field: field.to_string(), + pipeline: Pipeline { + stages: vec![Stage::Type(TypeCheck::Str)], + }, + source: "test".to_string(), + } + } + + #[test] + fn http_catchall_installs_for_args_only_global_block() { + // Regression for the fail-open-by-omission gap: a `global.apl` with + // only `args:` (no `policy:`) must still get the entity-less HTTP + // catch-all installed. Before the fix this gated on + // `!compiled.policy.is_empty()` alone, so an args-only admission + // block silently disabled authorization for all entity-less HTTP + // traffic. + let mut route = CompiledRoute::new("global"); + route.args.push(field_rule("http.method")); + assert!( + http_catchall_should_install(&route), + "an args-only global block must still install the catch-all handler" + ); + } + + #[test] + fn http_catchall_installs_for_policy_only_global_block() { + let mut route = CompiledRoute::new("global"); + route.policy.push(deny_effect()); + assert!(http_catchall_should_install(&route)); + } + + #[test] + fn http_catchall_does_not_install_for_empty_or_post_only_global_block() { + let empty = CompiledRoute::new("global"); + assert!( + !http_catchall_should_install(&empty), + "an empty global block has nothing to evaluate; installing would be a no-op handler" + ); + + let mut post_only = CompiledRoute::new("global"); + post_only.post_policy.push(deny_effect()); + assert!( + !http_catchall_should_install(&post_only), + "post_policy never runs on the Pre-phase-only catch-all, so it must not gate installation" + ); + } + #[test] fn response_subblock_parses_denywith() { let v = yaml( @@ -998,6 +1111,54 @@ mod tests { assert!(response_subblock(&v, "tool:*").is_none()); } + #[test] + fn response_subblock_nested_under_apl_wrapper_is_read() { + // An operator mirroring the pdp:/session_store: convention (which + // work identically flat or nested under `apl:`) may nest `response:` + // under `apl:` too. It must not be silently absorbed. + let v = + yaml("tool: \"*\"\napl:\n policy:\n - \"deny\"\n response:\n status: 401\n"); + let resp = response_subblock(&v, "tool:*").expect("nested response present"); + assert_eq!(resp.status, Some(401)); + } + + #[test] + fn response_subblock_top_level_wins_over_nested_apl_form() { + let v = yaml( + "tool: \"*\"\napl:\n policy:\n - \"deny\"\n response:\n status: 401\nresponse:\n status: 403\n", + ); + let resp = response_subblock(&v, "tool:*").expect("response present"); + assert_eq!( + resp.status, + Some(403), + "top-level sibling response takes precedence over the nested apl: form" + ); + } + + #[test] + fn response_subblock_malformed_is_none_not_propagated() { + // `status` must deserialize as a u16; a string value fails to parse. + // A malformed block must be dropped (warn-only), never bubble up an + // error that fails the whole config load. + let v = yaml("tool: \"*\"\nresponse:\n status: \"not-a-number\"\n"); + assert!( + response_subblock(&v, "tool:*").is_none(), + "malformed response: block must be ignored, not panic or propagate an error" + ); + } + + #[test] + fn warn_if_response_at_unsupported_scope_is_a_safe_noop() { + use super::warn_if_response_at_unsupported_scope; + // The helper only emits a tracing event; it must never panic whether + // `response:` is present or absent at a scope that can't render it. + let with_response = yaml("policy:\n - \"deny\"\nresponse:\n status: 403\n"); + let without = yaml("policy:\n - \"deny\"\n"); + warn_if_response_at_unsupported_scope(&with_response, "global.defaults.tool"); + warn_if_response_at_unsupported_scope(&with_response, "global.policies.some-tag"); + warn_if_response_at_unsupported_scope(&without, "global.defaults.tool"); + } + #[test] fn apl_wrapper_is_returned_as_is() { let v = yaml("apl:\n policy:\n - \"deny\"\n"); diff --git a/crates/apl-cpex/tests/end_to_end_route.rs b/crates/apl-cpex/tests/end_to_end_route.rs index a3cbf019..b2637564 100644 --- a/crates/apl-cpex/tests/end_to_end_route.rs +++ b/crates/apl-cpex/tests/end_to_end_route.rs @@ -752,6 +752,111 @@ async fn append_failure_fails_request_closed() { ); } +// Same tagger route as `TAGGER_ROUTE_YAML`, but with a route-level +// `response:` block — proves the fail-closed session-store denials +// (`session.load_failed` / `session.persist_failed`) decorate their +// violation with the route's custom denyWith too, not just an ordinary +// `Decision::Deny`. +const TAGGER_ROUTE_WITH_RESPONSE_YAML: &str = r#" +plugins: + - name: tagger + kind: tagger + hooks: [cmf.tool_pre_invoke] + capabilities: [append_labels, read_labels] +routes: + - tool: get_weather + apl: + policy: + - "plugin(tagger)" + response: + status: 503 + body: "session unavailable" +"#; + +async fn tagger_manager_with_store_and_yaml( + store: Arc, + yaml: &str, +) -> Arc { + let mgr = Arc::new(PluginManager::default()); + mgr.register_factory("tagger", Box::new(TaintingPluginFactory)); + register_apl( + &mgr, + AplOptions { + dispatch_cache: Arc::new(DispatchCache::new()), + session_store: store, + pdps: Vec::new(), + pdp_factories: Vec::new(), + session_store_factories: Vec::new(), + base_capabilities: None, + }, + ); + mgr.load_config_yaml(yaml).expect("load_config_yaml"); + mgr.initialize().await.expect("initialize"); + mgr +} + +/// A `session.load_failed` denial (AE1) still carries the route's custom +/// `response:` (denyWith) on its `details` map — the fix that closed prior +/// review gap #3 must hold for the load-failure fail-closed path, not just +/// `Decision::Deny`. +#[tokio::test] +async fn load_failure_carries_route_response() { + let store: Arc = Arc::new(ErrorSessionStore { + fail_load: true, + fail_append: false, + }); + let mgr = tagger_manager_with_store_and_yaml(store, TAGGER_ROUTE_WITH_RESPONSE_YAML).await; + let (mut ext, _key) = session_ext_and_key("sess-load-fail-resp", "alice"); + set_tool_meta(&mut ext, "get_weather"); + + let (result, _bg) = mgr + .invoke_named::("cmf.tool_pre_invoke", cmf_payload(), ext, None) + .await; + + assert!(!result.continue_processing); + let violation = result + .violation + .expect("load failure must surface a violation"); + assert_eq!(violation.code, "session.load_failed"); + assert_eq!( + violation + .details + .get(apl_cmf::constants::DETAIL_HTTP_STATUS), + Some(&serde_json::json!(503)), + "load_failed denial must carry the route's custom response status" + ); +} + +/// A `session.persist_failed` denial (AE6, R18) still carries the route's +/// custom `response:` (denyWith) on its `details` map. +#[tokio::test] +async fn persist_failure_carries_route_response() { + let store: Arc = Arc::new(ErrorSessionStore { + fail_load: false, + fail_append: true, + }); + let mgr = tagger_manager_with_store_and_yaml(store, TAGGER_ROUTE_WITH_RESPONSE_YAML).await; + let (mut ext, _key) = session_ext_and_key("sess-append-fail-resp", "alice"); + set_tool_meta(&mut ext, "get_weather"); + + let (result, _bg) = mgr + .invoke_named::("cmf.tool_pre_invoke", cmf_payload(), ext, None) + .await; + + assert!(!result.continue_processing); + let violation = result + .violation + .expect("append failure must flip to a Deny with a violation"); + assert_eq!(violation.code, "session.persist_failed"); + assert_eq!( + violation + .details + .get(apl_cmf::constants::DETAIL_HTTP_STATUS), + Some(&serde_json::json!(503)), + "persist_failed denial must carry the route's custom response status" + ); +} + /// R18 merge precedence: when the policy already Denies AND the append /// fails, the original policy violation is preserved (not overwritten by /// `session.persist_failed`) — the request is already denied, so the diff --git a/crates/apl-cpex/tests/visitor_e2e.rs b/crates/apl-cpex/tests/visitor_e2e.rs index a0d23ee5..d977174a 100644 --- a/crates/apl-cpex/tests/visitor_e2e.rs +++ b/crates/apl-cpex/tests/visitor_e2e.rs @@ -435,6 +435,42 @@ routes: assert!(result.violation.is_none()); } +/// A bare `global: { response: {...} }` — a denyWith with no accompanying +/// `apl:` policy/args block — must load cleanly (the visitor warns and moves +/// on) rather than panicking or erroring. `visit_global` returns early when +/// `apl_subblock` finds no APL terms; this guards that the stranded +/// `response:` on that early-return path is handled, not silently exploded. +#[tokio::test] +async fn global_response_without_apl_block_loads_without_error() { + const YAML: &str = r#" +plugins: + - name: allow-gate + kind: allow-gate + hooks: [cmf.tool_pre_invoke] +global: + response: + status: 403 + body: "forbidden" +routes: + - tool: anything +"#; + // The load must not panic or return Err despite the response-only global + // block having no installable policy. A request still flows through the + // legacy chain (no catch-all handler was installed for the entity-less + // path, which is the documented behavior this warns about). + let mgr = build_manager_with_visitor(YAML).await; + + let ext = Extensions { + meta: Some(Arc::new(meta_for_tool("anything"))), + ..Default::default() + }; + let (result, _bg) = mgr + .invoke_named::("cmf.tool_pre_invoke", cmf_payload("hi"), ext, None) + .await; + assert!(result.continue_processing); + assert!(result.violation.is_none()); +} + /// Smoke test that the visitor surfaces a compile error from a malformed /// APL block as a `PluginError::Config` out of `load_config_yaml`. Catches /// regressions where visitor errors swallow into Ok(_) or panic. From 92853f6da21b8ed200f4ccd4447a991f69f2315f Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 4 Jul 2026 22:09:07 -0300 Subject: [PATCH 7/8] docs(apl): document HTTP request-line attrs, response: block, and entity-less HTTP authz Add http.method/path/host/scheme to the extensions and read_headers tables. Document the route/global response: (denyWith) block and the global-policy path that authorizes generic HTTP requests carrying no MCP/A2A entity. Signed-off-by: Frederico Araujo --- docs/content/docs/apl/_index.md | 36 +++++++++++++++++++++++++++++++++ docs/content/docs/extensions.md | 4 ++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/docs/content/docs/apl/_index.md b/docs/content/docs/apl/_index.md index c5bd3aec..f33c6796 100644 --- a/docs/content/docs/apl/_index.md +++ b/docs/content/docs/apl/_index.md @@ -94,6 +94,42 @@ For richer conditionals, use the `when` / `do` form, where `do` is a single effe - "plugin(audit-log)" ``` +## Custom denial response + +By default a deny surfaces a reason and code, and the host renders its own denial. A route can instead attach a custom HTTP response — status, body, headers — through a `response:` block, a sibling of the route's `apl:` block: + +```yaml +routes: + - tool: locked + apl: + policy: + - "require(authenticated)" + response: + status: 403 + body: "{\"error\":\"forbidden\"}" + headers: + WWW-Authenticate: "Bearer" +``` + +All three fields are optional; an absent block leaves the host's default denial unchanged. When the route denies, the status/body/headers are carried on the violation for the host to render on the wire. `response:` is honored at route scope and at `global` scope (below); it is inert — and warns at load time — under `defaults` or a policy bundle. It is scope-local: a `global` `response:` is not inherited by entity routes. + +## Authorizing HTTP requests without an entity + +Routes key on an MCP / A2A entity — a tool, prompt, resource, or LLM. A generic HTTP request that carries no such entity is authorized by the `global` policy instead: when `global.apl` declares an `args:` or `policy:` block, CPEX evaluates it for these requests, reading the request line (`http.method`, `http.path`, `http.host`, `http.scheme`) and headers. Pair it with a `global` `response:` to return a custom denial. + +```yaml +global: + apl: + policy: + - "http.method != 'GET': deny" + response: + status: 405 + headers: + Allow: "GET" +``` + +The host must populate `http.host` from a validated request authority, never a raw client `Host` header, so host-based predicates cannot be spoofed by the caller. + ## Field pipelines `args:` and `result:` map a field to a pipeline of stages separated by `|`. Stages run left to right; a failed validator denies the phase. diff --git a/docs/content/docs/extensions.md b/docs/content/docs/extensions.md index 106a5e32..e311a637 100644 --- a/docs/content/docs/extensions.md +++ b/docs/content/docs/extensions.md @@ -23,7 +23,7 @@ Each extension flattens into bag attributes under its namespace, gated by a read | Agent | session, conversation, turn, and lineage context | `agent.*` | `read_agent` | | Meta | entity metadata: type, name, tags, scope, properties | `meta.*` | `read_meta` | | Request | environment, request id, timestamp, trace and span ids | `request.*` | `read_request` | -| HTTP | request and response headers (lowercased) | `http.request_headers.*`, `http.response_headers.*` | `read_headers`, `write_headers` | +| HTTP | request line (method, path, host, scheme) and request/response headers (lowercased) | `http.method`, `http.path`, `http.host`, `http.scheme`, `http.request_headers.*`, `http.response_headers.*` | `read_headers`, `write_headers` | | LLM | model id, provider, capabilities | `llm.*` | `read_llm` | | MCP | tool, resource, or prompt metadata | `mcp.*` (`mcp.tool.*`, `mcp.resource.*`, `mcp.prompt.*`) | `read_mcp` | | Completion | stop reason, token counts, model, latency | `completion.*` | `read_completion` | @@ -64,7 +64,7 @@ plugins: | `read_agent` | `agent.*` | | `read_meta` | `meta.*` | | `read_request` | `request.*` | -| `read_headers` | `http.request_headers.*`, `http.response_headers.*` | +| `read_headers` | `http.method`, `http.path`, `http.host`, `http.scheme`, `http.request_headers.*`, `http.response_headers.*` | | `read_llm` | `llm.*` | | `read_mcp` | `mcp.*` | | `read_completion` | `completion.*` | From ea4586e44d77049327385940cea30842621eb0c4 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sun, 5 Jul 2026 10:35:42 -0300 Subject: [PATCH 8/8] docs(apl): drop internal design-doc cross-references from comments Remove requirement/unit/spike identifiers (R5/R7/R8/R9/R10/R11/R14/R15/R16/R17/R18, U1/U2/U3, AE1-AE6, spike phase, Praxis transpiler) from doc comments and test descriptions across apl-core, apl-cpex, and the valkey session store. The comments now describe behavior in their own terms rather than pointing at private design docs the public repo doesn't carry. Signed-off-by: Frederico Araujo --- builtins/session/valkey/src/config.rs | 14 +++++++------- builtins/session/valkey/src/connection.rs | 2 +- builtins/session/valkey/src/error.rs | 2 +- builtins/session/valkey/src/store.rs | 16 ++++++++-------- .../valkey/tests/valkey_store_integration.rs | 10 +++++----- crates/apl-core/src/rules.rs | 6 +++--- crates/apl-cpex/src/cmf_invoker.rs | 2 +- crates/apl-cpex/src/route_handler.rs | 6 +++--- crates/apl-cpex/src/visitor.rs | 2 +- crates/apl-cpex/tests/end_to_end_route.rs | 16 ++++++++-------- crates/apl-cpex/tests/global_http_authz.rs | 9 ++++----- 11 files changed, 42 insertions(+), 43 deletions(-) diff --git a/builtins/session/valkey/src/config.rs b/builtins/session/valkey/src/config.rs index fc9c0245..064a0432 100644 --- a/builtins/session/valkey/src/config.rs +++ b/builtins/session/valkey/src/config.rs @@ -4,7 +4,7 @@ // Authors: Fred Araujo // // Parses and validates the `global.apl.session_store` block for the -// Valkey backend. Deliberately minimal (R11): a single endpoint, TLS, +// Valkey backend. Deliberately minimal: a single endpoint, TLS, // auth, key prefix, optional sliding TTL, and fail-closed timeout/retry // knobs with committed safe defaults. Sentinel/Cluster fields are NOT // present — they are out of scope and would be dead config surface. @@ -52,17 +52,17 @@ pub struct ValkeyConfig { #[serde(default)] pub password: Option, - /// Key prefix/namespace for label keys (R9). + /// Key prefix/namespace for label keys. #[serde(default = "default_key_prefix")] pub key_prefix: String, /// Sliding TTL in seconds, refreshed on load and append. `None` - /// (default) means no expiry (R7). + /// (default) means no expiry. #[serde(default)] pub ttl_seconds: Option, /// Declared maximum session-identity lifetime, used only to emit the - /// TTL-soundness warning (R17) when `ttl_seconds` is shorter. + /// TTL-soundness warning when `ttl_seconds` is shorter. #[serde(default)] pub max_session_lifetime_seconds: Option, @@ -90,9 +90,9 @@ impl ValkeyConfig { } /// Enforce the non-negotiable invariants. TLS is mandatory off - /// localhost (R10); a `tls: true` + plaintext `redis://` scheme is a + /// localhost; a `tls: true` + plaintext `redis://` scheme is a /// contradiction (would connect in cleartext); the connection URL - /// must build; the TTL-soundness warning (R17) is emitted here. + /// must build; the TTL-soundness warning is emitted here. /// /// All error text routes the endpoint through [`redact_endpoint`] so /// embedded credentials never leak into errors or logs. @@ -152,7 +152,7 @@ impl ValkeyConfig { ttl_seconds = ttl, max_session_lifetime_seconds = life, "valkey session_store TTL is shorter than the declared max session lifetime; \ - accumulated taint can silently expire (downgrade-by-waiting) — see R8" + accumulated taint can silently expire (downgrade-by-waiting)" ); } } diff --git a/builtins/session/valkey/src/connection.rs b/builtins/session/valkey/src/connection.rs index 28c0d0c7..c748a307 100644 --- a/builtins/session/valkey/src/connection.rs +++ b/builtins/session/valkey/src/connection.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Fred Araujo // -// Internal connection layer (R14): builds and holds the deadpool-redis +// Internal connection layer: builds and holds the deadpool-redis // pool for the Valkey backend. Kept private to this crate — it is NOT a // public reusable API. When a second consumer (the planned OAuth token // cache) is actually scheduled, extract a shared layer then diff --git a/builtins/session/valkey/src/error.rs b/builtins/session/valkey/src/error.rs index fd6b791d..fef053c9 100644 --- a/builtins/session/valkey/src/error.rs +++ b/builtins/session/valkey/src/error.rs @@ -17,7 +17,7 @@ pub enum BuildError { #[error("invalid valkey session_store config: {0}")] Config(String), - /// TLS is mandatory for any non-localhost endpoint (R10): session + /// TLS is mandatory for any non-localhost endpoint: session /// security labels must not transit a network segment in plaintext. #[error( "valkey session_store requires TLS for non-localhost endpoint '{0}' \ diff --git a/builtins/session/valkey/src/store.rs b/builtins/session/valkey/src/store.rs index 0c847abc..c9906bdc 100644 --- a/builtins/session/valkey/src/store.rs +++ b/builtins/session/valkey/src/store.rs @@ -6,16 +6,16 @@ // `ValkeySessionStore` — the Valkey-backed `SessionStore`. Labels live in // a Redis SET per session so `append_labels` is a single atomic // server-side union (`SADD`), never a client-side read-modify-write that -// would lose labels under concurrent cross-node appends (R16). +// would lose labels under concurrent cross-node appends. // -// # Fail-closed mapping (R5/R15) +// # Fail-closed mapping // // - `SMEMBERS` on a missing key returns an empty set → `Ok(empty)` -// (unknown session, R15). It is NOT an error. +// (unknown session). It is NOT an error. // - connection/timeout/protocol/decode failures → `Err(Backend)` so the // caller fails the request closed. // -// # Sliding TTL (R7) +// # Sliding TTL // // `append_labels` issues `SADD` + `EXPIRE` in one atomic pipeline. // `load_labels` refreshes the TTL fail-open: the read already succeeded, @@ -97,8 +97,8 @@ impl SessionStore for ValkeySessionStore { let mut conn = self.conn().await?; // SMEMBERS on a missing key returns an empty set (Ok), so an - // unknown session naturally maps to Ok(empty) (R15). Only a real - // backend failure becomes Err (R5). + // unknown session naturally maps to Ok(empty). Only a real + // backend failure becomes Err. let labels: Vec = match tokio::time::timeout(self.command_timeout, conn.smembers(&key)).await { Ok(res) => res.map_err(backend)?, @@ -111,7 +111,7 @@ impl SessionStore for ValkeySessionStore { // Sliding-TTL refresh is fail-open for the read: the labels were // read successfully, so a refresh failure is alarmed, not failed - // closed (R7). A persistently-failing refresh risks silent key + // closed. A persistently-failing refresh risks silent key // expiry across requests — see the operator runbook. if let Some(ttl) = self.ttl_seconds { let refresh: Result = @@ -146,7 +146,7 @@ impl SessionStore for ValkeySessionStore { // Atomic server-side union + optional TTL refresh in one round // trip (MULTI/EXEC). SADD is a commutative merge, so concurrent - // cross-node appends never lose labels (R16). + // cross-node appends never lose labels. let mut pipe = redis::pipe(); pipe.atomic(); pipe.sadd(&key, labels).ignore(); diff --git a/builtins/session/valkey/tests/valkey_store_integration.rs b/builtins/session/valkey/tests/valkey_store_integration.rs index 744de6e6..798d7c9c 100644 --- a/builtins/session/valkey/tests/valkey_store_integration.rs +++ b/builtins/session/valkey/tests/valkey_store_integration.rs @@ -94,7 +94,7 @@ fn store_key(session_id: &str) -> String { format!("taint:v1:{hex}") } -/// AE4 / R16: concurrent appends from two "nodes" (separate store +/// Concurrent appends from two "nodes" (separate store /// instances against one Valkey) union without loss; a third reader sees /// the full set. #[tokio::test] @@ -122,7 +122,7 @@ async fn cross_node_concurrent_append_unions() { assert_eq!(labels, vec!["INTERNAL".to_string(), "PII".to_string()]); } -/// R15: an unknown session is a confirmed key-miss → Ok(empty), not Err. +/// An unknown session is a confirmed key-miss → Ok(empty), not Err. #[tokio::test] #[ignore] async fn unknown_session_returns_empty_ok() { @@ -137,7 +137,7 @@ async fn unknown_session_returns_empty_ok() { assert!(labels.is_empty()); } -/// R5: a reachable but undecodable reply (key holds a string, not a SET) +/// A reachable but undecodable reply (key holds a string, not a SET) /// fails closed (Err) rather than returning Ok(empty). #[tokio::test] #[ignore] @@ -164,7 +164,7 @@ async fn wrongtype_reply_fails_closed() { ); } -/// R5: an unreachable endpoint fails closed quickly (bounded by the +/// An unreachable endpoint fails closed quickly (bounded by the /// command timeout). No container needed, but kept with the suite. #[tokio::test] #[ignore] @@ -183,7 +183,7 @@ async fn unreachable_endpoint_fails_closed() { ); } -/// AE2 / R7: a configured TTL is set on append and refreshed on load. +/// A configured TTL is set on append and refreshed on load. #[tokio::test] #[ignore] async fn ttl_set_on_append_and_refreshed_on_load() { diff --git a/crates/apl-core/src/rules.rs b/crates/apl-core/src/rules.rs index 44313ee3..ea008b97 100644 --- a/crates/apl-core/src/rules.rs +++ b/crates/apl-core/src/rules.rs @@ -405,9 +405,9 @@ impl PhaseSet { } } -/// Custom response to attach when a route's policy denies — the -/// transpiled form of a Kuadrant `AuthPolicy` `response.unauthorized` -/// `denyWith`. Carried on the route and surfaced on the deny outcome's +/// Custom response to attach when a route's policy denies (e.g., equivalent +/// to a Kuadrant `AuthPolicy` `response.unauthorized` `denyWith`). +/// Carried on the route and surfaced on the deny outcome's /// `details` map by the host (apl-cpex), so a host can render a custom /// HTTP response. All fields optional; an absent block leaves the host's /// default denial response unchanged. diff --git a/crates/apl-cpex/src/cmf_invoker.rs b/crates/apl-cpex/src/cmf_invoker.rs index 34f400ef..3419f0ef 100644 --- a/crates/apl-cpex/src/cmf_invoker.rs +++ b/crates/apl-cpex/src/cmf_invoker.rs @@ -225,7 +225,7 @@ impl CmfPluginInvoker { /// this exactly once after route evaluation completes. /// /// An append error is returned so the caller can fail the request - /// closed (R18). Because this runs after the policy decision is + /// closed. Because this runs after the policy decision is /// computed, the route handler converts an append error into a Deny /// outcome rather than dropping the accumulated taint silently. pub async fn persist_session(&self) -> Result<(), SessionStoreError> { diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index e267d07a..a54598fc 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -199,7 +199,7 @@ impl AnyHookHandler for AplRouteHandler { // (e.g. `extensions_arc`, `persist_session`) deref through the Arc. // Hydration loads accumulated session labels. A store failure // here happens *before* any policy decision, so we fail the - // request closed immediately (R5/R18, F2): deny with a + // request closed immediately: deny with a // distinguished violation rather than proceeding as if the // session carried no taint. Sessionless traffic never reaches // the store, so this only denies session-bearing requests. @@ -348,7 +348,7 @@ impl AnyHookHandler for AplRouteHandler { // Commit any session-scoped labels accumulated during this // request. No-op when there was no session id. The result is - // folded into the decision below (R18) — captured here because + // folded into the decision below — captured here because // `continue_processing`/`violation` are computed after persist. let persist_result = invoker.persist_session().await; @@ -435,7 +435,7 @@ impl AnyHookHandler for AplRouteHandler { }, }; - // Append fail-closed (R18) with merge precedence: + // Append fail-closed with merge precedence: // - decision Allow + append Err → flip to Deny with a // distinguished `session.persist_failed` violation. // - decision Deny + append Err → keep the original policy diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index db71f3ed..cf1d2539 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -433,7 +433,7 @@ impl ConfigVisitor for AplConfigVisitor { compiled.response = response_subblock(yaml, "global"); // Install a catch-all handler so the global policy also evaluates for - // generic (non-MCP/A2A) HTTP requests, which carry no entity (U3). + // generic (non-MCP/A2A) HTTP requests, which carry no entity. // Entity routes still stack `global` via apply_layer in visit_route; // this is the *entity-less* evaluation path. Pre-phase only — // authorization is an admission check, so there is no post handler. diff --git a/crates/apl-cpex/tests/end_to_end_route.rs b/crates/apl-cpex/tests/end_to_end_route.rs index b2637564..9702eed9 100644 --- a/crates/apl-cpex/tests/end_to_end_route.rs +++ b/crates/apl-cpex/tests/end_to_end_route.rs @@ -615,7 +615,7 @@ routes: } // --------------------------------------------------------------------- -// Fail-closed semantics (U2 / R4, R5, R18; AE1, AE6). +// Fail-closed semantics. // // A distributed SessionStore can fail. These tests use an erroring // test-double to prove the request fails *closed* — a store error @@ -697,7 +697,7 @@ async fn tagger_manager_with_store(store: Arc) -> Arc