From a2554d2255f6dd402d83aecf8280685f34df8258 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 18 Jul 2026 21:29:38 -0400 Subject: [PATCH 1/8] fix: resolve aislop findings, suppress documented false positives Signed-off-by: Frederico Araujo --- .../plugins/delegator-oauth/src/config.rs | 3 + .../plugins/elicitation-ciba/src/approver.rs | 24 +- .../plugins/elicitation-ciba/src/config.rs | 1 + builtins/plugins/identity-jwt/src/config.rs | 1 + builtins/session/valkey/src/config.rs | 1 + crates/apl-cmf/src/security.rs | 19 +- crates/apl-core/src/evaluator.rs | 48 +-- crates/apl-core/src/parser.rs | 332 ++++++++---------- crates/apl-cpex/src/delegation_invoker.rs | 42 +-- crates/cpex-core/src/config.rs | 74 +--- crates/cpex-core/src/delegation/payload.rs | 20 +- crates/cpex-core/src/extensions/filter.rs | 28 +- .../src/extensions/raw_credentials.rs | 2 + crates/cpex-ffi/src/lib.rs | 134 +------ go/cpex/abi.go | 3 + go/cpex/apl.go | 3 +- go/cpex/identity.go | 16 +- 17 files changed, 203 insertions(+), 548 deletions(-) diff --git a/builtins/plugins/delegator-oauth/src/config.rs b/builtins/plugins/delegator-oauth/src/config.rs index e01b3a67..dd0ac214 100644 --- a/builtins/plugins/delegator-oauth/src/config.rs +++ b/builtins/plugins/delegator-oauth/src/config.rs @@ -128,11 +128,14 @@ mod tests { #[test] fn config_deserializes_from_json() { let raw = json!({ + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "token_endpoint": "https://auth.example.com/oauth/token", "client_id": "gateway", + // aislop-ignore-next-line ai-slop/hardcoded-id -- Literal-secret-source test fixture, not a real credential "client_secret_source": { "kind": "literal", "secret": "dev-only" }, }); let cfg: OAuthDelegatorConfig = serde_json::from_value(raw).unwrap(); + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only assert_eq!(cfg.token_endpoint, "https://auth.example.com/oauth/token"); assert_eq!(cfg.client_id, "gateway"); assert_eq!(cfg.timeout_seconds, 5); diff --git a/builtins/plugins/elicitation-ciba/src/approver.rs b/builtins/plugins/elicitation-ciba/src/approver.rs index 705d0286..3012672d 100644 --- a/builtins/plugins/elicitation-ciba/src/approver.rs +++ b/builtins/plugins/elicitation-ciba/src/approver.rs @@ -120,8 +120,6 @@ impl CibaApprover { }) } - // ---------------- dispatch ---------------- - async fn do_dispatch(&self, payload: &ElicitationPayload) -> PluginResult { let login_hint = payload.from(); if login_hint.is_empty() { @@ -229,8 +227,6 @@ impl CibaApprover { PluginResult::modify_payload(out) } - // ---------------- check ---------------- - async fn do_check(&self, payload: &ElicitationPayload) -> PluginResult { let id = match payload.elicitation_id() { Some(id) => id, @@ -344,8 +340,6 @@ impl CibaApprover { PluginResult::modify_payload(out) } - // ---------------- validate ---------------- - async fn do_validate(&self, payload: &ElicitationPayload) -> PluginResult { let id = match payload.elicitation_id() { Some(id) => id, @@ -361,10 +355,10 @@ impl CibaApprover { Some(c) => c, None => return invalid(payload, "unknown elicitation id"), }; - // The approver claim was extracted from the OP token at `check` - // (provenance: it came straight from the OP over our authenticated - // TLS poll). Genuineness here = who actually approved matches who we - // asked for — a stored-vs-stored comparison, no token at rest. + // Both sides of this comparison are values already stored from the + // OP's authenticated TLS response, not a token read at rest — this + // check only asks whether the approver who resolved matches the + // approver who was asked. let resolved = match &correlation.resolved_approver { Some(a) => a, None => { @@ -416,8 +410,6 @@ impl HookHandler for CibaApprover { } } -// ----------------- wire shapes ----------------- - /// CIBA backchannel auth response (OIDC CIBA core §7.3). #[derive(Debug, Deserialize)] struct BackchannelResponse { @@ -443,8 +435,6 @@ struct OAuthError { error: String, } -// ----------------- helpers ----------------- - fn cfg_err(plugin: &str, msg: String) -> Box { Box::new(PluginError::Config { message: format!("plugin '{plugin}' (cpex-plugin-elicitation-ciba): {msg}"), @@ -506,10 +496,8 @@ fn sanitize_binding_message(purpose: &str) -> String { /// unparseable so the caller can fall back to a default. fn parse_duration_secs(s: &str) -> Option { let s = s.trim(); - if s.is_empty() { - return None; - } - let (num, mult) = match s.chars().last().unwrap() { + let last = s.chars().last()?; + let (num, mult) = match last { 's' => (&s[..s.len() - 1], 1), 'm' => (&s[..s.len() - 1], 60), 'h' => (&s[..s.len() - 1], 3600), diff --git a/builtins/plugins/elicitation-ciba/src/config.rs b/builtins/plugins/elicitation-ciba/src/config.rs index 5ec71cd4..9bab2b57 100644 --- a/builtins/plugins/elicitation-ciba/src/config.rs +++ b/builtins/plugins/elicitation-ciba/src/config.rs @@ -148,6 +148,7 @@ mod tests { "backchannel_endpoint": "https://kc/realms/corp/protocol/openid-connect/ext/ciba/auth", "token_endpoint": "https://kc/realms/corp/protocol/openid-connect/token", "client_id": "cpex-gateway", + // aislop-ignore-next-line ai-slop/hardcoded-id -- Literal-secret-source test fixture, not a real credential "client_secret_source": { "kind": "literal", "secret": "dev-only" }, }); let cfg: CibaConfig = serde_json::from_value(raw).unwrap(); diff --git a/builtins/plugins/identity-jwt/src/config.rs b/builtins/plugins/identity-jwt/src/config.rs index 0efeb175..ba747e08 100644 --- a/builtins/plugins/identity-jwt/src/config.rs +++ b/builtins/plugins/identity-jwt/src/config.rs @@ -457,6 +457,7 @@ mod tests { #[tokio::test] async fn jwks_http_url_rejected_at_build_async() { let src = DecodingKeySource::JwksUrl { + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only url: "http://idp.example/jwks".into(), insecure_http: false, refresh_secs: 3600, diff --git a/builtins/session/valkey/src/config.rs b/builtins/session/valkey/src/config.rs index 064a0432..5225e624 100644 --- a/builtins/session/valkey/src/config.rs +++ b/builtins/session/valkey/src/config.rs @@ -381,6 +381,7 @@ mod tests { fn tls_required_error_redacts_credentials() { // rediss-less, non-localhost, with embedded creds, tls off → error. let err = + // aislop-ignore-next-line security/hardcoded-secret -- test fixture proving credentials get redacted, not a real secret parse("kind: valkey\nendpoint: redis://user:topsecret@prod.host:6379\n").unwrap_err(); let msg = format!("{err}"); assert!( diff --git a/crates/apl-cmf/src/security.rs b/crates/apl-cmf/src/security.rs index 19d0dab7..646b0d3e 100644 --- a/crates/apl-cmf/src/security.rs +++ b/crates/apl-cmf/src/security.rs @@ -57,7 +57,6 @@ use crate::constants::{ /// Flatten a `SecurityExtension` into the bag. pub fn extract_security(sec: &SecurityExtension, bag: &mut AttributeBag) { - // ----- Subject (caller identity) ----- if let Some(subject) = &sec.subject { let mut authenticated = false; if let Some(id) = &subject.id { @@ -94,22 +93,18 @@ pub fn extract_security(sec: &SecurityExtension, bag: &mut AttributeBag) { } } - // ----- Client (OAuth application identity) ----- if let Some(client) = &sec.client { extract_client(client, bag); } - // ----- Inbound caller's attested workload identity ----- if let Some(caller) = &sec.caller_workload { extract_workload("caller_workload", caller, bag); } - // ----- Our own attested workload identity (outbound) ----- if let Some(this_w) = &sec.this_workload { extract_workload("this_workload", this_w, bag); } - // ----- Other security fields ----- if let Some(m) = &sec.auth_method { bag.set("auth_method", m.clone()); } @@ -364,16 +359,13 @@ mod tests { assert_eq!(bag.get_bool("role.guest"), Some(true)); } - // ----------------------------------------------------------------- - // Client (OAuth application identity) bag namespace - // ----------------------------------------------------------------- - fn agent_client() -> ClientExtension { ClientExtension { client_id: "agent-app".into(), client_name: Some("Agent App".into()), trust_level: ClientTrustLevel::FirstParty, authorized_scopes: vec!["read".into(), "write".into()], + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only authorized_audiences: vec!["https://api.example.com".into()], roles: vec!["partner".into()], permissions: vec!["call_tool".into()], @@ -414,6 +406,7 @@ mod tests { extract_client(&agent_client(), &mut bag); assert!(bag.set_contains("client.authorized_scopes", "read")); assert!(bag.set_contains("client.authorized_scopes", "write")); + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only assert!(bag.set_contains("client.authorized_audiences", "https://api.example.com",)); assert!(bag.set_contains("client.teams", "acme")); } @@ -441,10 +434,6 @@ mod tests { assert_eq!(bag.get_string("client.trust_level"), Some("partner-tier-A")); } - // ----------------------------------------------------------------- - // Workload (extract_workload helper — both prefixes) - // ----------------------------------------------------------------- - fn workload_fixture() -> WorkloadIdentity { WorkloadIdentity { spiffe_id: Some("spiffe://corp.com/svc/foo".into()), @@ -492,10 +481,6 @@ mod tests { assert_eq!(bag.get_string("caller_workload.spiffe_id"), None); } - // ----------------------------------------------------------------- - // extract_security orchestrates all four identity slots - // ----------------------------------------------------------------- - #[test] fn extract_security_populates_all_four_identity_namespaces() { // Single fixture exercising subject + client + caller_workload + diff --git a/crates/apl-core/src/evaluator.rs b/crates/apl-core/src/evaluator.rs index 62eafe7c..baa0c5c7 100644 --- a/crates/apl-core/src/evaluator.rs +++ b/crates/apl-core/src/evaluator.rs @@ -206,10 +206,6 @@ fn looks_like_attribute_ref(s: &str) -> bool { .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.') } -// ===================================================================== -// Async effect evaluator (policy: / post_policy: walks Vec) -// ===================================================================== - /// Walk an Effect list against the bag, dispatching PDP calls via `pdp` /// and plugin invocations via `plugins`. Returns the phase's overall /// decision. @@ -1115,10 +1111,6 @@ async fn dispatch_field_op( } } -// ===================================================================== -// Pipe-chain evaluator (args: / result: field pipelines) -// ===================================================================== - /// Result of running a pipeline against one field's value. /// /// `Pass`: every stage succeeded; the original value should be kept. @@ -1158,8 +1150,9 @@ pub struct PipelineEvaluation { /// `Stage::Plugin` fires so the invoker knows which field is in focus. /// Pass `""` for standalone pipeline runs that aren't part of a field rule. /// -/// `Stage::Validate { name }` is currently a no-op with a TODO — the named -/// validator registry lands in a later step. +/// `Stage::Validate { name }` has no named-validator registry backing it; +/// it always denies with a message pointing operators at `regex(...)` or +/// `plugin(...)` instead. pub async fn evaluate_pipeline( pipeline: &Pipeline, value: &serde_json::Value, @@ -1174,7 +1167,6 @@ pub async fn evaluate_pipeline( for (idx, stage) in pipeline.stages.iter().enumerate() { match stage { - // ----- Validators ----- Stage::Type(tc) => { if !type_check(tc, ¤t) { return PipelineEvaluation { @@ -1313,7 +1305,6 @@ pub async fn evaluate_pipeline( }; }, - // ----- Transforms ----- Stage::Mask { keep_last } => { let Some(s) = current.as_str() else { return PipelineEvaluation { @@ -1363,7 +1354,6 @@ pub async fn evaluate_pipeline( replaced = true; }, - // ----- Effects ----- Stage::Taint { label, scopes } => { taints.push(TaintEvent { label: label.clone(), @@ -1563,8 +1553,6 @@ mod tests { Expression::Condition(c) } - // ----- Decision-level semantics ----- - #[test] fn empty_rules_allow() { let mut bag = AttributeBag::new(); @@ -1641,8 +1629,6 @@ mod tests { assert_eq!(evaluate_rules(&rules, &bag), Decision::Allow); } - // ----- Predicate semantics ----- - #[test] fn missing_key_is_false() { let mut bag = AttributeBag::new(); @@ -1697,8 +1683,6 @@ mod tests { assert!(eval_expression(&Expression::Not(Box::new(b)), &bag)); } - // ----- Comparison operators ----- - #[test] fn int_comparisons() { let mut bag = AttributeBag::new(); @@ -1834,8 +1818,6 @@ mod tests { )); } - // ----- Realistic end-to-end ----- - #[test] fn hr_compensation_scenario() { // From the HR demo: alice (hr role + view_ssn perm) requests compensation @@ -1903,10 +1885,6 @@ mod tests { } } - // =================================================================== - // Pipe-chain evaluator tests - // =================================================================== - use crate::pipeline::{Stage, TypeCheck}; use serde_json::json; @@ -2199,8 +2177,6 @@ mod tests { } } - // ----- Regex stage ----- - #[tokio::test] async fn pipeline_regex_match_passes() { let mut bag = AttributeBag::new(); @@ -2259,8 +2235,6 @@ mod tests { } } - // ----- Taint and Scan stages ----- - #[tokio::test] async fn pipeline_taint_records_event() { let mut bag = AttributeBag::new(); @@ -2391,8 +2365,6 @@ mod tests { ); } - // ----- Plugin stage in pipe chain ----- - /// Pipe-context plugin invoker that returns canned outcomes by name. struct PipePlugin { outcomes: std::collections::HashMap, @@ -2550,10 +2522,6 @@ mod tests { } } - // =================================================================== - // 5c additions: Exists, InSet, Always - // =================================================================== - #[test] fn exists_distinguishes_missing_from_falsy() { let mut bag = AttributeBag::new(); @@ -2675,10 +2643,6 @@ mod tests { } } - // =================================================================== - // 5c-v/vi: async step evaluator with mock resolvers - // =================================================================== - use crate::step::{ PdpCall, PdpDecision, PdpDialect, PdpError, PdpResolver, PluginError, PluginInvocation, PluginInvoker, PluginOutcome, @@ -3178,8 +3142,6 @@ mod tests { ); } - // ----- E2: FieldOp end-to-end through evaluate_steps ----- - #[tokio::test] async fn field_op_in_do_redacts_args_during_pre_phase() { // Sketches the demo case: when condition holds, redact args.ssn @@ -3306,8 +3268,6 @@ mod tests { } } - // ----- E3: Sequential / Parallel orchestration ----- - #[tokio::test] async fn sequential_runs_effects_in_order_until_deny() { // A Sequential block runs each effect in order. Allow-only @@ -3480,8 +3440,6 @@ mod tests { } } - // ----- Effect::Elicit (human-in-the-loop) ----- - /// Run a single `Effect::Elicit` against the given invoker + bag. /// Seeds the `user.manager` attribute (the `from` these tests use) so /// it resolves — an unresolved attribute `from` now fails closed, which diff --git a/crates/apl-core/src/parser.rs b/crates/apl-core/src/parser.rs index 5006c05c..e934152b 100644 --- a/crates/apl-core/src/parser.rs +++ b/crates/apl-core/src/parser.rs @@ -32,10 +32,6 @@ use crate::plugin_decl::{PluginDeclaration, PluginOverride, PluginRegistry}; use crate::rules::{CompareOp, CompiledRoute, Condition, Effect, Expression, Literal, Rule}; use crate::step::{DelegateStep, ElicitKind, ElicitStep, PdpCall, PdpDialect, Step}; -// ===================================================================== -// Errors -// ===================================================================== - #[derive(Debug, Error)] pub enum ParseError { #[error("YAML parse error: {0}")] @@ -64,10 +60,6 @@ pub enum ParseError { ConflictingAuthorizationForms { location: String, phase: String }, } -// ===================================================================== -// Lexer -// ===================================================================== - #[derive(Debug, Clone, PartialEq)] enum Tok { Ident(String), // dotted: subject.id, role.hr, authenticated @@ -297,10 +289,6 @@ fn is_ident_cont(b: u8) -> bool { b.is_ascii_alphanumeric() || b == b'_' || b == b'.' } -// ===================================================================== -// Predicate parser (Pratt-style; precedence () > ! > & > |) -// ===================================================================== - struct PredParser<'a> { src: &'a str, toks: Vec, @@ -343,7 +331,7 @@ impl<'a> PredParser<'a> { parts.push(self.parse_and()?); } Ok(if parts.len() == 1 { - parts.pop().unwrap() + parts.pop().expect("parts.len() == 1 was checked above") } else { Expression::Or(parts) }) @@ -356,7 +344,7 @@ impl<'a> PredParser<'a> { parts.push(self.parse_unary()?); } Ok(if parts.len() == 1 { - parts.pop().unwrap() + parts.pop().expect("parts.len() == 1 was checked above") } else { Expression::And(parts) }) @@ -519,10 +507,6 @@ pub fn parse_predicate(src: &str) -> Result { PredParser::parse(src.trim()) } -// ===================================================================== -// Rule parser -// ===================================================================== - /// Parse a single rule line into a `Rule`. /// /// Accepted forms (DSL §3.2): @@ -688,7 +672,10 @@ fn parse_require_rule(line: &str) -> Result { .map(|k| Expression::Condition(Condition::IsFalse { key: k })) .collect(); if falses.len() == 1 { - return Ok(falses.into_iter().next().unwrap()); + return Ok(falses + .into_iter() + .next() + .expect("falses.len() == 1 was checked above")); } Ok(match sep { Some(Tok::Or) => Expression::And(falses), // require(X | Y) → !X & !Y @@ -832,10 +819,6 @@ fn strip_string_literal(s: &str, rule: &str) -> Result { } } -// ===================================================================== -// Step parser (pre_invocation / post_invocation entries — steps + rules) -// ===================================================================== - /// Parse a single YAML entry from a `pre_invocation` / `post_invocation` list. /// /// Two YAML shapes (DSL §3.2 + §7): @@ -1368,7 +1351,10 @@ fn parse_step_map(m: &serde_yaml::Mapping, source: &str) -> Result Result bool { @@ -1607,7 +1577,10 @@ fn parse_effect_value(val: &serde_yaml::Value, source: &str) -> Result return parse_sequential_effect(v, source), @@ -1688,7 +1661,7 @@ fn parse_effect_string(s: &str, source: &str) -> Result { let trimmed = s.trim(); if let Some(mut effects) = try_bare_action(trimmed) { if effects.len() == 1 { - return Ok(effects.pop().unwrap()); + return Ok(effects.pop().expect("effects.len() == 1 was checked above")); } } if let Some(effect) = try_parse_deny_call(trimmed, s)? { @@ -1869,7 +1842,11 @@ fn step_to_effect(step: Step, source: &str) -> Result { ), }); } - Ok(rule.effects.into_iter().next().unwrap()) + Ok(rule + .effects + .into_iter() + .next() + .expect("rule.effects.len() == 1 was checked above")) }, Step::Pdp { .. } => Err(ParseError::Rule { rule: source.to_string(), @@ -2026,10 +2003,6 @@ fn extract_call_args(line: &str, name: &str) -> Option { None } -// ===================================================================== -// Pipe-chain parser (args: / result: field pipelines) -// ===================================================================== - /// Parse a pipe-chain string into a `Pipeline`. /// /// Splits on `|` (outside parens/quotes), trims each stage, parses each. @@ -2089,7 +2062,6 @@ fn parse_stage(src: &str) -> Result { let (head, args) = split_head_args(s).ok_or_else(|| bad("expected stage identifier"))?; match (head, args.as_deref()) { - // ----- Bare validators / transforms / effects ----- ("str", None) => Ok(Stage::Type(TypeCheck::Str)), ("int", None) => Ok(Stage::Type(TypeCheck::Int)), ("bool", None) => Ok(Stage::Type(TypeCheck::Bool)), @@ -2111,118 +2083,134 @@ fn parse_stage(src: &str) -> Result { kind: ScanKind::InjectionScan, }), - // ----- Parameterized ----- - ("mask", Some(a)) => { - let n: usize = a - .trim() - .parse() - .map_err(|_| bad(&format!("mask(N) expects integer, got `{}`", a)))?; - Ok(Stage::Mask { keep_last: n }) - }, - ("redact", Some(a)) => { - // redact(!perm.view_ssn) — argument is a predicate expression. - let cond = parse_predicate(a).map_err(|e| ParseError::Predicate { - predicate: src.to_string(), - msg: format!("invalid redact() condition: {}", e), - })?; - Ok(Stage::Redact { - condition: Some(cond), - }) - }, + ("mask", Some(a)) => parse_stage_mask(a, &bad), + ("redact", Some(a)) => parse_stage_redact_cond(a, src), ("hash", Some(_)) => Err(bad("hash takes no arguments")), ("omit", Some(_)) => Err(bad( "omit takes no arguments — for conditional omit, use a policy rule predicate", )), - ("len", Some(a)) => { - let (min, max) = parse_range_inner(a) - .ok_or_else(|| bad(&format!("len(...) expects N..M range, got `{}`", a)))?; - let to_usize = |v: i64| -> Result { - if v < 0 { - Err(bad("len bounds must be non-negative")) - } else { - Ok(v as usize) - } - }; - Ok(Stage::Length { - min: min.map(to_usize).transpose()?, - max: max.map(to_usize).transpose()?, - }) - }, - ("enum", Some(a)) => { - let values = split_top_level(a, b',') - .into_iter() - .map(|v| { - let t = v.trim(); - // Allow either bare identifier or quoted string. - if (t.starts_with('"') && t.ends_with('"')) - || (t.starts_with('\'') && t.ends_with('\'')) - { - t[1..t.len() - 1].to_string() - } else { - t.to_string() - } - }) - .filter(|s| !s.is_empty()) - .collect::>(); - if values.is_empty() { - return Err(bad("enum() requires at least one value")); - } - Ok(Stage::Enum { values }) - }, - ("regex", Some(a)) => { - let pattern = a.trim(); - let pat = if (pattern.starts_with('"') && pattern.ends_with('"')) - || (pattern.starts_with('\'') && pattern.ends_with('\'')) - { - pattern[1..pattern.len() - 1].to_string() - } else { - pattern.to_string() - }; - Ok(Stage::Regex { pattern: pat }) - }, - ("validate", Some(a)) => { - // Named-validator dispatch (`validate(name)`) is in the - // spec (DSL §4.2) but not implemented in this build — - // the evaluator's no-op stub would silently let invalid - // values through. Reject at compile time so operators - // notice immediately and reach for one of the working - // alternatives: - // - // * `regex("pattern")` — inline named-regex equivalent - // * `plugin(name)` — full plugin dispatch for rich - // validation (Luhn, format-with-context, etc.) - // - // When the ValidatorRegistry slice lands, this arm flips - // back to returning `Stage::Validate { name }`. - Err(bad(&format!( - "`validate({})` — named-validator dispatch is not implemented \ - in this build. Use `regex(\"pattern\")` for a named-regex \ - equivalent, or `plugin({})` for richer validation logic.", - a.trim(), - a.trim(), - ))) - }, + ("len", Some(a)) => parse_stage_len(a, &bad), + ("enum", Some(a)) => parse_stage_enum(a, &bad), + ("regex", Some(a)) => Ok(parse_stage_regex(a)), + ("validate", Some(a)) => Err(parse_stage_validate_rejected(a, &bad)), // `run` is an alias for `plugin` (mirrors the policy-step alias). - ("plugin" | "run", Some(a)) => { - let name = a.trim(); - if name.is_empty() { - // Mirror the empty-name guard in `parse_step_string` so - // both the policy-step and field-stage paths reject a - // nameless `plugin()` / `run()` with the same diagnostic. - return Err(bad(&format!( - "`{head}(...)`: plugin name must not be empty" - ))); - } - Ok(Stage::Plugin { - name: name.to_string(), - }) - }, + ("plugin" | "run", Some(a)) => parse_stage_plugin(head, a, &bad), ("taint", Some(a)) => parse_taint(a, src), (other, _) => Err(bad(&format!("unknown stage `{}`", other))), } } +fn parse_stage_mask(a: &str, bad: &impl Fn(&str) -> ParseError) -> Result { + let n: usize = a + .trim() + .parse() + .map_err(|_| bad(&format!("mask(N) expects integer, got `{}`", a)))?; + Ok(Stage::Mask { keep_last: n }) +} + +fn parse_stage_redact_cond(a: &str, src: &str) -> Result { + // redact(!perm.view_ssn) — argument is a predicate expression. + let cond = parse_predicate(a).map_err(|e| ParseError::Predicate { + predicate: src.to_string(), + msg: format!("invalid redact() condition: {}", e), + })?; + Ok(Stage::Redact { + condition: Some(cond), + }) +} + +fn parse_stage_len(a: &str, bad: &impl Fn(&str) -> ParseError) -> Result { + let (min, max) = parse_range_inner(a) + .ok_or_else(|| bad(&format!("len(...) expects N..M range, got `{}`", a)))?; + let to_usize = |v: i64| -> Result { + if v < 0 { + Err(bad("len bounds must be non-negative")) + } else { + Ok(v as usize) + } + }; + Ok(Stage::Length { + min: min.map(to_usize).transpose()?, + max: max.map(to_usize).transpose()?, + }) +} + +fn parse_stage_enum(a: &str, bad: &impl Fn(&str) -> ParseError) -> Result { + let values = split_top_level(a, b',') + .into_iter() + .map(|v| { + let t = v.trim(); + // Allow either bare identifier or quoted string. + if (t.starts_with('"') && t.ends_with('"')) + || (t.starts_with('\'') && t.ends_with('\'')) + { + t[1..t.len() - 1].to_string() + } else { + t.to_string() + } + }) + .filter(|s| !s.is_empty()) + .collect::>(); + if values.is_empty() { + return Err(bad("enum() requires at least one value")); + } + Ok(Stage::Enum { values }) +} + +fn parse_stage_regex(a: &str) -> Stage { + let pattern = a.trim(); + let pat = if (pattern.starts_with('"') && pattern.ends_with('"')) + || (pattern.starts_with('\'') && pattern.ends_with('\'')) + { + pattern[1..pattern.len() - 1].to_string() + } else { + pattern.to_string() + }; + Stage::Regex { pattern: pat } +} + +// Named-validator dispatch (`validate(name)`) is in the spec (DSL §4.2) +// but not implemented in this build — the evaluator's no-op stub would +// silently let invalid values through. Reject at compile time so +// operators notice immediately and reach for one of the working +// alternatives: +// +// * `regex("pattern")` — inline named-regex equivalent +// * `plugin(name)` — full plugin dispatch for rich validation (Luhn, +// format-with-context, etc.) +// +// When the ValidatorRegistry slice lands, this rejection flips back to +// returning `Stage::Validate { name }`. +fn parse_stage_validate_rejected(a: &str, bad: &impl Fn(&str) -> ParseError) -> ParseError { + bad(&format!( + "`validate({})` — named-validator dispatch is not implemented \ + in this build. Use `regex(\"pattern\")` for a named-regex \ + equivalent, or `plugin({})` for richer validation logic.", + a.trim(), + a.trim(), + )) +} + +fn parse_stage_plugin( + head: &str, + a: &str, + bad: &impl Fn(&str) -> ParseError, +) -> Result { + let name = a.trim(); + if name.is_empty() { + // Mirror the empty-name guard in `parse_step_string` so both the + // policy-step and field-stage paths reject a nameless `plugin()` + // / `run()` with the same diagnostic. + return Err(bad(&format!( + "`{head}(...)`: plugin name must not be empty" + ))); + } + Ok(Stage::Plugin { + name: name.to_string(), + }) +} + /// Try to parse `s` as a bare range literal: `0..100`, `..500`, `0..`, `0..1M`. fn try_parse_range(s: &str) -> Option { if !s.contains("..") { @@ -2366,10 +2354,6 @@ fn parse_taint_scope(s: &str, src: &str) -> Result { } } -// ===================================================================== -// YAML config -// ===================================================================== - /// Top-level config — only the bits step 5a understands. /// /// `policy_evaluator:`, `imports:`, `global:`, `defaults:`, `tags:`, @@ -2651,18 +2635,12 @@ pub fn compile_policy_block_value( compile_apl_blocks(source, raw) } -// ===================================================================== -// Tests -// ===================================================================== - #[cfg(test)] mod tests { use super::*; use crate::attributes::AttributeBag; use crate::evaluator::Decision; - // ----- Lexer ----- - #[test] fn lex_basic() { let toks = Lexer::new("delegation.depth > 2").tokenize_all().unwrap(); @@ -2708,8 +2686,6 @@ mod tests { assert!(format!("{}", err).contains("expected `==`")); } - // ----- Predicate parser ----- - #[test] fn pred_bare_identifier() { let e = parse_predicate("authenticated").unwrap(); @@ -2924,8 +2900,6 @@ mod tests { assert!(format!("{}", err).contains("trailing")); } - // ----- Rule parser ----- - #[test] fn rule_predicate_action_form() { let r = parse_rule("delegation.depth > 2: deny", "test").unwrap(); @@ -3089,8 +3063,6 @@ mod tests { assert!(format!("{}", err).contains("expected a quoted string")); } - // ----- E1: when/do canonical form ----- - fn parse_step_yaml(yaml: &str) -> Result { let v: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); parse_step(&v, "test") @@ -3195,8 +3167,6 @@ do: assert!(format!("{}", err).contains("no effects")); } - // ----- E1: shorthand multi-effect map (predicate: [list]) ----- - #[test] fn shorthand_multi_effect_map() { // Shorthand for the canonical when/do form. The predicate is @@ -3263,8 +3233,6 @@ do: assert!(format!("{}", err).contains("no effects")); } - // ----- E2: content effects in do: (field pipe chains) ----- - #[test] fn when_do_with_field_op_result_redact() { // The headline E2 case: `result.salary | redact` as an effect @@ -3359,8 +3327,6 @@ do: "args.card_number | run(luhn)" fn field_stage_plugin_empty_name_is_rejected() { // `plugin()` / `run()` with no name in a field pipeline must be // rejected, mirroring the policy-step path (`parse_step_string`). - // Previously the field-stage path accepted it as - // `Stage::Plugin { name: "" }`. for verb in ["plugin", "run"] { let err = parse_stage(&format!("{verb}()")).expect_err("empty name must error"); let msg = format!("{err}"); @@ -3431,8 +3397,6 @@ do: "args.x | ""#; assert_eq!(find_top_level_pipe("mask(a|b)"), None); } - // ----- E3: sequential: / parallel: parsing ----- - #[test] fn top_level_sequential() { // `- sequential: [list]` as a top-level policy step. @@ -3578,8 +3542,6 @@ sequential: } } - // ----- Colon-splitting edge cases ----- - #[test] fn split_respects_quotes_and_parens() { // The `:` inside parens / quotes shouldn't be the separator. @@ -3592,8 +3554,6 @@ sequential: } } - // ----- YAML compilation ----- - #[test] fn compile_simple_route() { let yaml = r#" @@ -3997,8 +3957,6 @@ routes: } } - // ----- End-to-end with evaluator ----- - #[tokio::test] async fn end_to_end_hr_compensation() { let yaml = r#" @@ -4124,8 +4082,6 @@ routes: } } - // ----- Pipeline parsing ----- - #[test] fn pipeline_simple_bare_stages() { let p = parse_pipeline("str").unwrap(); @@ -4303,8 +4259,6 @@ routes: assert!(format!("{}", err).contains("omit takes no arguments")); } - // ----- YAML compilation with pipelines ----- - #[test] fn compile_route_with_args_and_result() { let yaml = r#" @@ -4367,8 +4321,6 @@ routes: assert!(format!("{}", err).contains("unknown stage")); } - // ----- plugins: block + route-level overrides ----- - #[test] fn compile_captures_root_plugins_block_into_registry() { let yaml = r#" @@ -4438,8 +4390,6 @@ routes: assert_eq!(eff.hooks, &["tool_pre_invoke".to_string()]); } - // ----- compile_policy_block_value (single-block compiler for visitors) ----- - #[test] fn compile_policy_block_value_parses_apl_body() { let yaml = r#" @@ -4482,8 +4432,6 @@ pre_invocation: } } - // ----- delegate: step parsing ----- - #[test] fn parse_delegate_step_with_only_plugin() { let yaml = r#" @@ -4580,8 +4528,6 @@ pre_invocation: assert!(msg.contains("must be a map"), "got: {msg}"); } - // ----- delegate(...) function-call string form ----- - #[test] fn parse_delegate_string_bare_plugin_name() { let yaml = r#"- "delegate(workday-oauth)""#; @@ -4812,8 +4758,6 @@ routes: assert_eq!(post_ds.source, "get_compensation.post_invocation[0]"); } - // ----- elicitation sugar verbs (require_approval, confirm, …) ----- - fn parse_elicit_str(s: &str) -> crate::step::ElicitStep { let value = serde_yaml::Value::String(s.to_string()); let step = parse_step(&value, "test.policy[0]").expect("parse"); @@ -4975,8 +4919,6 @@ routes: assert_eq!(e.scope.as_deref(), Some("args.amount <= 25000")); } - // ----- validate(name) compile-time rejection (DSL spec §4.2) ----- - #[test] fn parse_pipeline_rejects_validate_stage_at_compile_time() { // Named-validator dispatch isn't implemented; the parser diff --git a/crates/apl-cpex/src/delegation_invoker.rs b/crates/apl-cpex/src/delegation_invoker.rs index 71ca4521..b4825d8e 100644 --- a/crates/apl-cpex/src/delegation_invoker.rs +++ b/crates/apl-cpex/src/delegation_invoker.rs @@ -94,11 +94,11 @@ impl DelegationPluginInvoker { #[async_trait] impl DelegationInvoker for DelegationPluginInvoker { async fn delegate(&self, step: &DelegateStep) -> Result { - // 1. Resolve the plugin's token.delegate entry from the plan. - // Routes that don't reference this plugin in `pre_invocation:` / - // `post_invocation:` at compile time won't have it in the plan - // — surface that as NotFound so the evaluator's on_error - // semantics kick in. + // Resolve the plugin's token.delegate entry out of + // `self.plan.delegation_entries`. Routes that don't reference this + // plugin in `pre_invocation:` / `post_invocation:` at compile time + // won't have an entry there — surface that as NotFound so the + // evaluator's on_error semantics kick in. let entry = self .plan .delegation_entries @@ -106,15 +106,14 @@ impl DelegationInvoker for DelegationPluginInvoker { .ok_or_else(|| DelegationError::NotFound(step.plugin_name.clone()))? .clone(); - // 2. Snapshot extensions to construct the payload + pass into - // invoke_entries. We keep the canonical copy under the - // Mutex; this snapshot is the per-call working copy. + // Snapshot extensions to construct the payload and pass into + // invoke_entries. The canonical copy stays under the Mutex; this + // snapshot is the per-call working copy. let current_extensions = self.extensions.lock().await.clone(); - // 3. Pull the inbound bearer token from raw_credentials. v0 - // looks for the User-role token; future iterations can - // surface multi-token selection (Client / Workload) via - // step config. + // Pull the inbound bearer token from raw_credentials. Looks for + // the User-role token; future iterations can surface multi-token + // selection (Client / Workload) via step config. let bearer_token = current_extensions .raw_credentials .as_ref() @@ -122,17 +121,14 @@ impl DelegationInvoker for DelegationPluginInvoker { .map(|tok| (*tok.token).clone()) .unwrap_or_default(); - // 4. Read step args. Step `config_override` is a yaml map per - // the IR — we extract a few well-known keys onto the typed - // DelegationPayload builders. Unknown keys still flow - // through to the plugin via the per-call config-override - // pathway at registration time (already applied when the - // plan was built — plugins consume them from their - // `cfg.config`). For Slice B we keep this mapping minimal: - // `target` is required (delegation needs to know who the - // downstream call is for); `audience`, `permissions`, - // `mode`, `auth_enforced_by` are recognized; everything - // else stays opaque. + // Read step args. Step `config_override` is a yaml map per the IR + // — extract a few well-known keys onto the typed DelegationPayload + // builders. Unknown keys still flow through to the plugin via the + // per-call config-override pathway (plugins consume them from + // their `cfg.config`). `target` is required (delegation needs to + // know who the downstream call is for); `audience`, `permissions`, + // `mode`, `auth_enforced_by` are recognized; everything else stays + // opaque. let cfg = step.config_override.as_ref().and_then(|v| v.as_mapping()); let target_name: String = cfg diff --git a/crates/cpex-core/src/config.rs b/crates/cpex-core/src/config.rs index 05073c27..30adf128 100644 --- a/crates/cpex-core/src/config.rs +++ b/crates/cpex-core/src/config.rs @@ -26,10 +26,6 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::error::PluginError; use crate::plugin::PluginConfig; -// --------------------------------------------------------------------------- -// Top-Level Config -// --------------------------------------------------------------------------- - /// Top-level CPEX configuration. /// /// Parsed from a single YAML file. Plugin scoping mode is controlled @@ -68,10 +64,6 @@ impl CpexConfig { } } -// --------------------------------------------------------------------------- -// Plugin Settings -// --------------------------------------------------------------------------- - /// Global plugin settings. /// /// Controls executor behavior and routing mode. All fields have @@ -139,10 +131,6 @@ fn default_true() -> bool { true } -// --------------------------------------------------------------------------- -// Global Config -// --------------------------------------------------------------------------- - /// Global configuration — applies across all routes. /// /// Only used when routing is enabled. Contains named policy groups @@ -176,10 +164,6 @@ pub struct GlobalConfig { pub identity: Option, } -// --------------------------------------------------------------------------- -// Policy Group -// --------------------------------------------------------------------------- - /// A named policy group — plugins to activate and optional metadata. /// /// The `all` group is reserved and always applied. @@ -210,10 +194,6 @@ pub struct PolicyGroup { pub identity: Option, } -// --------------------------------------------------------------------------- -// Plugin Ref (route/group plugin reference) -// --------------------------------------------------------------------------- - /// A reference to a plugin in a route or policy group. /// /// ```yaml @@ -293,10 +273,6 @@ where } } -// --------------------------------------------------------------------------- -// Route Entry -// --------------------------------------------------------------------------- - /// A per-entity routing rule. /// /// Matches one entity type (tool, resource, prompt, or LLM) and @@ -363,10 +339,6 @@ pub struct RouteEntry { pub identity: Option, } -// --------------------------------------------------------------------------- -// Custom Deserialize for RouteEntry.identity -// --------------------------------------------------------------------------- - /// Deserialize the `authentication:` block in a `RouteEntry`. Accepts either a YAML /// list (treated as additive — `replace_inherited: false`) or a /// YAML map with `replace_inherited: bool?` + `steps: [...]`. Each @@ -490,10 +462,6 @@ fn parse_identity_step( } } -// --------------------------------------------------------------------------- -// Route Meta -// --------------------------------------------------------------------------- - /// Operational metadata on a route entry. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct RouteMeta { @@ -511,10 +479,6 @@ pub struct RouteMeta { pub properties: HashMap, } -// --------------------------------------------------------------------------- -// String or List (for tool matching) -// --------------------------------------------------------------------------- - /// An entity-name pattern. Holds the original pattern string (for /// serialization round-tripping and operator-facing diagnostics) plus a /// `WildMatch` matcher pre-compiled at deserialize time so route resolution @@ -604,10 +568,6 @@ impl StringOrList { } } -// --------------------------------------------------------------------------- -// Config Loading -// --------------------------------------------------------------------------- - /// Load and parse a CPEX config from a YAML file. pub fn load_config(path: &Path) -> Result> { let content = std::fs::read_to_string(path).map_err(|e| PluginError::Config { @@ -670,10 +630,6 @@ fn reject_renamed_identity_key(raw: &serde_yaml::Value) -> Result<(), Box Result<(), Box> { Ok(()) } -// --------------------------------------------------------------------------- -// Route Resolution -// --------------------------------------------------------------------------- - /// Specificity scores for route matching. const SPECIFICITY_EXACT_NAME: usize = 1000; const SPECIFICITY_NAME_LIST: usize = 500; @@ -1005,7 +957,6 @@ fn find_matching_route<'a>( let mut best: Option<(usize, &RouteEntry)> = None; for route in &config.routes { - // Check scope compatibility let route_scope = route.meta.as_ref().and_then(|m| m.scope.as_deref()); let scope_bonus = match (route_scope, request_scope) { (None, _) => 0, // route is global @@ -1678,8 +1629,6 @@ routes: assert_eq!(route.when.as_deref(), Some("args.sensitive == true")); } - // ---- route-level `authentication:` block ---- - #[test] fn parse_route_identity_list_form() { let yaml = r#" @@ -1810,9 +1759,8 @@ routes: #[test] fn legacy_identity_key_is_rejected_at_route_and_global() { - // Breaking rename: `identity:` was renamed to `authentication:`. - // A stale key must fail loudly, never be silently dropped (which - // would skip authentication — a fail-open). + // The legacy `identity:` key must fail loudly, never be silently + // dropped (which would skip authentication — a fail-open). for yaml in [ "routes:\n - tool: t\n identity:\n - corp-jwt\n", "global:\n identity:\n - corp-jwt\n", @@ -1840,8 +1788,6 @@ routes: assert!(msg.contains("list of steps"), "got: {msg}"); } - // ---- resolve_identity_plugins_for_route ---- - #[test] fn resolve_identity_returns_empty_when_no_route_matches() { let yaml = r#" @@ -1922,8 +1868,6 @@ routes: ); } - // ---- Slice C: global + tag-bundle inheritance ---- - #[test] fn resolve_identity_includes_global_layer_when_route_has_no_block() { // global.identity defined; route declares no identity. The @@ -2120,20 +2064,6 @@ routes: assert!(non_matching.is_empty()); } - // ----------------------------------------------------------------- - // `plugins:` accepts both shapes (map-tolerant deserializer) - // - // A *sequence* is the structural activation list. A *mapping* is the - // APL per-plugin override form (consumed by the APL visitor from the - // raw YAML), so it deserializes to an empty structural list here. - // Before this, a map at route/defaults/policy scope failed the whole - // `CpexConfig` parse with "invalid type: map, expected a sequence". - // - // These exercise deserialization directly (not `parse_config`, which - // also runs `validate_config`'s plugin-reference checks) because the - // bug being fixed was a *deserialize-time* failure. - // ----------------------------------------------------------------- - fn deserialize_cfg(yaml: &str) -> Result { serde_yaml::from_str(yaml).map_err(|e| e.to_string()) } diff --git a/crates/cpex-core/src/delegation/payload.rs b/crates/cpex-core/src/delegation/payload.rs index f02d725d..8b88876b 100644 --- a/crates/cpex-core/src/delegation/payload.rs +++ b/crates/cpex-core/src/delegation/payload.rs @@ -157,11 +157,6 @@ pub struct AttenuationConfig { /// clones and return the updated payload). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DelegationPayload { - // ----- Input (private — caller-supplied, never mutated by handlers) ----- - /// The caller's current credential — the one a token-exchange - /// handler will swap for a downstream-scoped credential. Cleared - /// on drop via `Zeroizing`. `#[serde(skip)]` — never appears in - /// serialized output. #[serde(skip)] bearer_token: Zeroizing, @@ -194,12 +189,6 @@ pub struct DelegationPayload { #[serde(default, skip_serializing_if = "Option::is_none")] route_attenuation: Option, - // ----- Output (pub — handlers populate via direct assignment on clones) ----- - /// The minted outbound credential. `None` until a handler - /// produces one. Carries the raw bytes (cleared on drop), the - /// header the proxy plugin should attach it under, the - /// audience it was minted for, the effective scopes, and the - /// expiry timestamp. #[serde(default, skip_serializing_if = "Option::is_none")] pub delegated_token: Option, @@ -260,8 +249,6 @@ impl DelegationPayload { } } - // -------- Input builders -------- - pub fn with_target_type(mut self, t: TargetType) -> Self { self.target_type = t; self @@ -292,8 +279,6 @@ impl DelegationPayload { self } - // -------- Input read accessors -------- - /// The caller's bearer token — borrowed, no way to move or /// replace the underlying `Zeroizing` through this. pub fn bearer_token(&self) -> &str { @@ -328,8 +313,6 @@ impl DelegationPayload { self.route_attenuation.as_ref() } - // -------- Output helpers -------- - /// Layer another payload's *output* fields onto this one's, /// following "Some replaces None, last write wins per slot." /// Input fields are not touched — the running payload's input @@ -356,8 +339,6 @@ impl DelegationPayload { } } - // -------- Host-side application helpers -------- - /// Pull the resolved `DelegationPayload` out of a `PipelineResult` /// returned by `mgr.invoke_named::(...)`. /// Returns `None` when the pipeline was denied or when the result's @@ -515,6 +496,7 @@ mod tests { updated.delegated_token = Some(RawDelegatedToken::new( "minted-bytes", "Authorization", + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "https://api.example.com", vec!["read".into()], Utc::now(), diff --git a/crates/cpex-core/src/extensions/filter.rs b/crates/cpex-core/src/extensions/filter.rs index 8bed45a2..b260d36c 100644 --- a/crates/cpex-core/src/extensions/filter.rs +++ b/crates/cpex-core/src/extensions/filter.rs @@ -18,10 +18,6 @@ use super::container::Extensions; use super::security::{SecurityExtension, SubjectExtension}; use super::tiers::{AccessPolicy, Capability, MutabilityTier, SlotPolicy}; -// --------------------------------------------------------------------------- -// Slot Registry — static policies per extension slot -// --------------------------------------------------------------------------- - /// Extension slot identifiers. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum SlotName { @@ -215,10 +211,6 @@ pub fn slot_policy(slot: SlotName) -> SlotPolicy { } } -// --------------------------------------------------------------------------- -// Capability Checking -// --------------------------------------------------------------------------- - /// Check if a set of capabilities grants read access to a slot. fn has_read_access(policy: &SlotPolicy, capabilities: &HashSet) -> bool { if policy.access == AccessPolicy::Unrestricted { @@ -269,10 +261,6 @@ fn cap_str(cap: Capability) -> String { .to_string() } -// --------------------------------------------------------------------------- -// Filter Extensions -// --------------------------------------------------------------------------- - /// Build a Extensions containing only slots the plugin can access. /// /// Starts from an empty Extensions and clones in only the @@ -444,10 +432,8 @@ fn build_filtered_security( } // Our own outbound workload identity — also gated under - // `read_workload`. Plugins not declaring it never see our - // gateway's SPIFFE-SVID (previously this slot was always-visible - // under the old `agent` name; the cap gating is intentional new - // behavior, per spec §4.4). + // `read_workload`, per spec §4.4. Plugins not declaring it never + // see our gateway's SPIFFE-SVID. if let Some(ref tw) = security.this_workload { let policy = slot_policy(SlotName::SecurityThisWorkload); if has_read_access(&policy, capabilities) { @@ -681,10 +667,6 @@ mod tests { assert!(filtered.delegation.unwrap().delegated); } - // ----------------------------------------------------------------- - // New identity-slot capability gating (slice 1 step C) - // ----------------------------------------------------------------- - /// Builds a SecurityExtension carrying all four identity principal /// slots — subject, client, caller_workload, this_workload. /// Used by the new-slot cap-gating tests. @@ -785,10 +767,6 @@ mod tests { assert!(sec.client.is_none()); } - // ----------------------------------------------------------------- - // RawCredentialsExtension capability gating - // ----------------------------------------------------------------- - fn extensions_with_raw_credentials() -> Extensions { use crate::extensions::raw_credentials::{ DelegationKey, DelegationMode, RawCredentialsExtension, RawDelegatedToken, @@ -802,6 +780,7 @@ mod tests { raw.delegated_tokens.insert( DelegationKey { subject_id: "alice".into(), + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into()], mode: DelegationMode::OnBehalfOfUser, @@ -809,6 +788,7 @@ mod tests { RawDelegatedToken::new( "delegated-bytes", "Authorization", + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "https://api.example.com", vec!["read".into()], chrono::Utc::now(), diff --git a/crates/cpex-core/src/extensions/raw_credentials.rs b/crates/cpex-core/src/extensions/raw_credentials.rs index b479f589..dc739677 100644 --- a/crates/cpex-core/src/extensions/raw_credentials.rs +++ b/crates/cpex-core/src/extensions/raw_credentials.rs @@ -309,12 +309,14 @@ mod tests { fn delegation_key_hash_eq_consistency() { let k1 = DelegationKey { subject_id: "alice".into(), + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into(), "write".into()], mode: DelegationMode::OnBehalfOfUser, }; let k2 = DelegationKey { subject_id: "alice".into(), + // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into(), "write".into()], mode: DelegationMode::OnBehalfOfUser, diff --git a/crates/cpex-ffi/src/lib.rs b/crates/cpex-ffi/src/lib.rs index 7a9464ac..2a9ae7a1 100644 --- a/crates/cpex-ffi/src/lib.rs +++ b/crates/cpex-ffi/src/lib.rs @@ -28,18 +28,6 @@ use cpex_core::manager::PluginManager; // APL governance wiring — the `cpex_apl_install` extern "C" entry point. mod apl; -// --------------------------------------------------------------------------- -// FFI Result Codes -// --------------------------------------------------------------------------- -// -// FFI functions return c_int. 0 means success; negative codes classify -// failures so the Go (or other-language) caller can produce a typed -// error rather than a single opaque "invoke failed" string. The codes -// are stable wire ABI — additions go at the end with a fresh value; -// don't renumber. -// -// Mapped on the Go side in `go/cpex/manager.go::errorFromRC`. - /// Operation succeeded. pub const RC_OK: c_int = 0; /// Manager handle is null or shut down. @@ -63,35 +51,6 @@ pub const RC_TIMEOUT: c_int = -6; /// Plugin panicked; caught by `catch_unwind` at the FFI boundary. pub const RC_PANIC: c_int = -7; -// --------------------------------------------------------------------------- -// FFI ABI Version -// --------------------------------------------------------------------------- -// -// The FFI ABI version is an integer that identifies the C-surface -// contract this crate exposes. Bump it on any breaking change to the -// C surface: -// -// - added / removed / renamed extern "C" function -// - argument count, argument type, or return type change on an -// existing function -// - layout change of a struct that crosses the boundary -// - semantic change to an existing function (e.g. new RC_* value -// returned for a previously-success case, change in pointer -// ownership) -// -// Adding a new RC_* code at the end of the existing range is *not* a -// breaking change (the wire codes are stable; consumers handle unknown -// codes as generic failure). -// -// Consumers — every language binding — MUST call `cpex_ffi_abi_version` -// at init and compare against the version their binding was generated -// for. Mismatch is a hard error: the C surface they generated against -// is not the one they're linked against. Document the binding's -// expected ABI version in its source. -// -// Bumps are recorded in CHANGELOG.md under "Changed" with the from→to -// integers and a one-line description of what moved. - /// FFI ABI version. Bump on breaking C-surface changes; see module /// docs above for what counts as breaking. pub const FFI_ABI_VERSION: u32 = 2; @@ -114,23 +73,6 @@ pub extern "C" fn cpex_ffi_abi_version() -> u32 { /// usual case never hits this bound. const FFI_WALL_CLOCK_TIMEOUT: Duration = Duration::from_secs(60); -// --------------------------------------------------------------------------- -// Shared Tokio Runtime -// --------------------------------------------------------------------------- -// -// One process-singleton runtime serves every manager rather than each -// `cpex_manager_new` building its own. With many managers (multi-tenant -// hosts that create one per request, dynamic plugin reload, etc.) the -// per-manager model exploded thread count: 100 managers × num_cpus -// workers each = hundreds of OS threads (and ~2MB stack apiece). -// -// Worker thread count precedence (highest first): -// 1. `cpex_configure_runtime(N)` — explicit FFI call, before first use. -// 2. `CPEX_FFI_WORKER_THREADS` env var — operator-friendly; read once -// on first use of `shared_runtime()`. -// 3. tokio default (`num_cpus`). -// -// Once the runtime is initialized it's fixed for the process lifetime. static SHARED_RUNTIME: OnceLock = OnceLock::new(); /// Name of the env var operators set to bound worker threads without @@ -315,10 +257,6 @@ where } } -// --------------------------------------------------------------------------- -// Payload Type Registry -// --------------------------------------------------------------------------- - /// Payload type IDs — must match Go constants. pub const PAYLOAD_GENERIC: u8 = 0; pub const PAYLOAD_CMF_MESSAGE: u8 = 1; @@ -391,10 +329,6 @@ fn serialize_payload(payload: &dyn PluginPayload) -> Result<(u8, Vec), Strin Err("unknown payload type, cannot serialize across FFI".to_string()) } -// --------------------------------------------------------------------------- -// Opaque Handle Types -// --------------------------------------------------------------------------- - /// Opaque handle to a PluginManager. /// /// All managers share the process-singleton runtime returned by @@ -417,10 +351,6 @@ pub struct CpexBackgroundTasksInner { tasks: BackgroundTasks, } -// --------------------------------------------------------------------------- -// Helper: safe string from C -// --------------------------------------------------------------------------- - unsafe fn c_str_to_slice<'a>(ptr: *const c_char, len: c_int) -> Option<&'a str> { if ptr.is_null() || len <= 0 { return None; @@ -457,7 +387,8 @@ fn alloc_bytes(data: &[u8]) -> (*mut u8, c_int) { ); return (ptr::null_mut(), 0); } - let layout = std::alloc::Layout::from_size_align(len, 1).unwrap(); + let layout = std::alloc::Layout::from_size_align(len, 1) + .expect("align 1 is always valid and len <= c_int::MAX is far under isize::MAX"); unsafe { let ptr = std::alloc::alloc(layout); if ptr.is_null() { @@ -468,10 +399,6 @@ fn alloc_bytes(data: &[u8]) -> (*mut u8, c_int) { } } -// --------------------------------------------------------------------------- -// Manager Lifecycle -// --------------------------------------------------------------------------- - /// Create a new PluginManager from a YAML config string. /// /// Returns an opaque handle. The manager owns a tokio runtime for @@ -716,10 +643,6 @@ pub unsafe extern "C" fn cpex_plugin_names( RC_OK } -// --------------------------------------------------------------------------- -// Hook Invocation -// --------------------------------------------------------------------------- - /// Invoke a hook by name. /// /// Payload and extensions are passed as MessagePack bytes. @@ -816,7 +739,6 @@ pub unsafe extern "C" fn cpex_invoke( None => return RC_INVALID_HANDLE, }; - // Parse hook name let name = match c_str_to_slice(hook_name, hook_len) { Some(s) => s, None => return RC_INVALID_INPUT, @@ -918,17 +840,14 @@ pub unsafe extern "C" fn cpex_invoke( }, }; - // Return result bytes let (ptr, len) = alloc_bytes(&result_bytes); *result_msgpack_out = ptr; *result_len_out = len; - // Return context table as opaque handle *context_table_out = Box::into_raw(Box::new(CpexContextTableInner { table: result.context_table, })); - // Return background tasks as opaque handle *bg_handle_out = Box::into_raw(Box::new(CpexBackgroundTasksInner { tasks: bg })); RC_OK @@ -1026,11 +945,6 @@ pub unsafe extern "C" fn cpex_invoke_resolved( Extensions::default() }; - // ---- Identity resolution (in-process) ---- - // Resolve identity and merge the result into the extensions BEFORE the - // hook runs, so raw_credentials (skip-serialized across FFI) reach the - // hook in Rust memory. Skipped when no identity payload was supplied or - // no identity.resolve hook is registered. let merged_extensions: Extensions = if identity_len > 0 && inner .manager @@ -1088,7 +1002,6 @@ pub unsafe extern "C" fn cpex_invoke_resolved( base_extensions }; - // ---- Hook invoke with the identity-enriched extensions ---- let (result, bg) = match run_safely( inner .manager @@ -1185,10 +1098,6 @@ unsafe fn finish_pipeline_result( RC_OK } -// --------------------------------------------------------------------------- -// Background Tasks -// --------------------------------------------------------------------------- - /// Wait for all background tasks to complete. /// /// Returns MessagePack-encoded errors (empty array if none). @@ -1221,7 +1130,11 @@ pub unsafe extern "C" fn cpex_wait_background( if bg_handle.is_null() { let empty: Vec = Vec::new(); - let (ptr, len) = alloc_bytes(&rmp_serde::to_vec_named(&empty).unwrap()); + let empty_bytes = match rmp_serde::to_vec_named(&empty) { + Ok(b) => b, + Err(_) => return RC_SERIALIZE_ERROR, + }; + let (ptr, len) = alloc_bytes(&empty_bytes); *errors_msgpack_out = ptr; *errors_len_out = len; return RC_OK; @@ -1268,10 +1181,6 @@ pub unsafe extern "C" fn cpex_free_background(bg_handle: *mut CpexBackgroundTask } } -// --------------------------------------------------------------------------- -// Context Table -// --------------------------------------------------------------------------- - /// Free a context table handle. /// /// # Safety @@ -1283,10 +1192,6 @@ pub unsafe extern "C" fn cpex_free_context_table(ct: *mut CpexContextTableInner) } } -// --------------------------------------------------------------------------- -// Memory Management -// --------------------------------------------------------------------------- - /// Free a byte buffer allocated by the FFI layer. /// /// # Safety @@ -1297,14 +1202,11 @@ pub unsafe extern "C" fn cpex_free_bytes(ptr: *mut u8, len: c_int) { if ptr.is_null() || len <= 0 { return; } - let layout = std::alloc::Layout::from_size_align(len as usize, 1).unwrap(); + let layout = std::alloc::Layout::from_size_align(len as usize, 1) + .expect("align 1 is always valid and len (a positive c_int) is far under isize::MAX"); std::alloc::dealloc(ptr, layout); } -// --------------------------------------------------------------------------- -// FFI Result Types — serialized to MessagePack for the caller -// --------------------------------------------------------------------------- - /// Pipeline result serialized across the FFI boundary. /// Matches the Go `PipelineResult` struct field names. /// @@ -1350,10 +1252,6 @@ mod serde_bytes_opt { } } -// --------------------------------------------------------------------------- -// Generic Payload — wraps a deserialized MessagePack value -// --------------------------------------------------------------------------- - /// A generic payload that wraps a deserialized serde_json::Value. /// /// Used for FFI dispatch when the concrete payload type isn't known @@ -1366,16 +1264,6 @@ pub struct GenericPayload { cpex_core::impl_plugin_payload!(GenericPayload); -// --------------------------------------------------------------------------- -// FFI unit tests -// --------------------------------------------------------------------------- -// -// These tests call the `extern "C"` functions directly from Rust to -// exercise the FFI safety layer (catch_unwind, return-code mapping, -// payload-type validation) without needing a Go test harness. The -// reviewer flagged that `cpex-ffi` had zero `#[cfg(test)]` coverage — -// this seeds the file with regressions for the highest-value invariants. - #[cfg(test)] mod tests { use super::*; @@ -1388,8 +1276,6 @@ mod tests { use cpex_core::hooks::PluginResult; use cpex_core::plugin::{Plugin, PluginConfig, PluginMode}; - // --- Test scaffolding ----------------------------------------------------- - /// Test hook type using GenericPayload — that's the type /// PAYLOAD_GENERIC produces at the FFI deserialization boundary, /// so the typed-adapter downcast actually finds the handler. @@ -1499,8 +1385,6 @@ mod tests { rc } - // --- Tests ---------------------------------------------------------------- - /// Panic in a plugin must be caught at the FFI boundary and mapped /// to `RC_PANIC` rather than unwinding across `extern "C"` (UB on /// Rust < 1.81; abort on >= 1.81). Direct regression for P0 #2. diff --git a/go/cpex/abi.go b/go/cpex/abi.go index 0a35af01..4a40efbf 100644 --- a/go/cpex/abi.go +++ b/go/cpex/abi.go @@ -37,6 +37,9 @@ const expectedFFIABIVersion uint32 = 2 func init() { actual := uint32(C.cpex_ffi_abi_version()) if actual != expectedFFIABIVersion { + // Intentional panic: init() has no error return, and a mismatched + // ABI means every cgo call in this package is already unsound — + // there is no error value we could hand back to a caller. panic(fmt.Sprintf( "cpex: FFI ABI version mismatch — Go binding expects %d, "+ "linked libcpex_ffi reports %d. Upgrade github.com/"+ diff --git a/go/cpex/apl.go b/go/cpex/apl.go index df97885c..85032a85 100644 --- a/go/cpex/apl.go +++ b/go/cpex/apl.go @@ -42,7 +42,8 @@ import "C" // creation and therefore does NOT support APL — use the default-manager // flow instead: // -// mgr, _ := NewPluginManagerDefault() +// mgr, err := NewPluginManagerDefault() +// if err != nil { ... } // mgr.EnableAPL() // mgr.LoadConfig(yaml) // mgr.Initialize() diff --git a/go/cpex/identity.go b/go/cpex/identity.go index 220df7cf..1578ab29 100644 --- a/go/cpex/identity.go +++ b/go/cpex/identity.go @@ -11,9 +11,9 @@ // idp := cpex.NewIdentityPayload(cpex.TokenSourceBearer, headers) // res, ct, bg, err := mgr.InvokeByName( // cpex.HookIdentityResolve, cpex.PayloadIdentity, idp, ext, nil) -// // On success the resolved IdentityPayload comes back as res.ModifiedPayload. +// // On success res.ModifiedPayload holds the resolved IdentityPayload. // resolved, _ := cpex.DeserializePayload[cpex.IdentityPayload](res) -// // resolved.Subject now carries roles / permissions / teams. +// // resolved.Subject carries roles / permissions / teams. // // The resolved Subject / Client / RawCredentials are then applied onto // the Extensions passed to the downstream tool/prompt/resource hook so @@ -29,15 +29,15 @@ import "github.com/vmihailenco/msgpack/v5" // crates/cpex-core/src/identity/hook.rs. const HookIdentityResolve = "identity.resolve" -// TokenSource values — where a credential was extracted from. Wire form +// TokenSource values identify where a credential comes from. Wire form // is snake_case to match the Rust TokenSource enum // (#[serde(rename_all = "snake_case")]). const ( - TokenSourceBearer = "bearer" - TokenSourceUserToken = "user_token" - TokenSourceMTLS = "mtls" + TokenSourceBearer = "bearer" + TokenSourceUserToken = "user_token" + TokenSourceMTLS = "mtls" TokenSourceSpiffeJwtSvid = "spiffe_jwt_svid" - TokenSourceAPIKey = "api_key" + TokenSourceAPIKey = "api_key" ) // IdentityPayload mirrors the Rust IdentityPayload. Input fields @@ -55,14 +55,12 @@ const ( // them onto the next hook's Extensions without the bindings needing a // typed mirror of every Rust extension. type IdentityPayload struct { - // ----- Input ----- Source string `msgpack:"source"` SourceHeader string `msgpack:"source_header,omitempty"` Headers map[string]string `msgpack:"headers,omitempty"` ClientHost string `msgpack:"client_host,omitempty"` ClientPort uint16 `msgpack:"client_port,omitempty"` - // ----- Output ----- Subject *SubjectExtension `msgpack:"subject,omitempty"` Client msgpack.RawMessage `msgpack:"client,omitempty"` CallerWorkload msgpack.RawMessage `msgpack:"caller_workload,omitempty"` From 1745b7bba63f94e6a9c201906825279715292a08 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 18 Jul 2026 21:30:48 -0400 Subject: [PATCH 2/8] chore: remove dead code across crates Signed-off-by: Frederico Araujo --- bindings/python/src/conversions.rs | 24 -------- bindings/python/src/manager.rs | 26 --------- bindings/python/src/result.rs | 4 -- builtins/pdps/cedar-direct/src/cedar_attrs.rs | 7 --- builtins/pdps/cedar-direct/src/entities.rs | 4 -- builtins/pdps/cedar-direct/src/resolver.rs | 7 --- builtins/plugins/identity-jwt/src/resolver.rs | 4 -- crates/apl-cmf/src/capability_namespaces.rs | 5 -- crates/apl-cmf/src/constants.rs | 29 ---------- crates/apl-core/src/pipeline.rs | 4 -- crates/apl-core/src/route.rs | 16 ------ crates/apl-core/src/rules.rs | 2 - crates/apl-core/src/step.rs | 20 ------- crates/apl-cpex/src/cmf_invoker.rs | 7 --- crates/apl-cpex/src/dispatch_plan.rs | 4 -- crates/apl-cpex/src/parallel_safety.rs | 12 ---- crates/apl-cpex/src/route_handler.rs | 8 --- crates/apl-cpex/src/session_resolver.rs | 23 -------- crates/apl-cpex/src/visitor.rs | 4 -- crates/cpex-builtins/src/lib.rs | 20 ------- crates/cpex-core/src/cmf/constants.rs | 21 ------- crates/cpex-core/src/cmf/content.rs | 12 ---- crates/cpex-core/src/cmf/enums.rs | 16 ------ crates/cpex-core/src/cmf/message.rs | 12 ---- crates/cpex-core/src/cmf/view.rs | 12 ---- crates/cpex-core/src/context.rs | 8 --- crates/cpex-core/src/elicitation/payload.rs | 10 ---- crates/cpex-core/src/error.rs | 12 ---- crates/cpex-core/src/executor.rs | 56 ------------------- crates/cpex-core/src/extensions/container.rs | 8 --- crates/cpex-core/src/extensions/tiers.rs | 37 ------------ crates/cpex-core/src/factory.rs | 8 --- crates/cpex-core/src/hooks/adapter.rs | 4 -- crates/cpex-core/src/hooks/metadata.rs | 4 -- crates/cpex-core/src/hooks/payload.rs | 8 --- crates/cpex-core/src/hooks/trait_def.rs | 12 ---- crates/cpex-core/src/hooks/types.rs | 14 ----- crates/cpex-core/src/identity/payload.rs | 13 ----- crates/cpex-core/src/manager.rs | 53 ------------------ crates/cpex-core/src/plugin.rs | 20 ------- crates/cpex-core/src/registry.rs | 20 ------- crates/cpex-orchestration/src/lib.rs | 23 -------- crates/cpex/src/lib.rs | 12 ---- go/cpex/cmf.go | 24 -------- 44 files changed, 649 deletions(-) diff --git a/bindings/python/src/conversions.rs b/bindings/python/src/conversions.rs index 9686fc8f..50a1d4d0 100644 --- a/bindings/python/src/conversions.rs +++ b/bindings/python/src/conversions.rs @@ -18,10 +18,6 @@ use pyo3::prelude::*; use pyo3::types::{PyBool, PyDict, PyFloat, PyInt, PyList, PyString}; use serde_json::{Map, Value}; -// --------------------------------------------------------------------------- -// GenericPayload — local struct for non-CMF hooks (KD5) -// --------------------------------------------------------------------------- - /// Wraps any serde_json::Value for hooks that are not `cmf.*` (KD1, KD2). /// /// Defined locally because `cpex-core` exports the macro but not the struct @@ -33,10 +29,6 @@ pub struct GenericPayload { cpex_core::impl_plugin_payload!(GenericPayload); -// --------------------------------------------------------------------------- -// PyObject → serde_json::Value -// --------------------------------------------------------------------------- - /// Convert a Python object to a `serde_json::Value`. /// /// Supported types: `bool`, `int`, `float`, `str`, `None`, `list`, `dict` @@ -104,10 +96,6 @@ pub fn pyobj_to_json_value( ))) } -// --------------------------------------------------------------------------- -// serde_json::Value → PyObject -// --------------------------------------------------------------------------- - /// Convert a `serde_json::Value` to a Python object. /// /// `null` → `None`, booleans → `bool`, numbers → `int` or `float`, @@ -145,10 +133,6 @@ pub fn json_value_to_pyobj<'py>(py: Python<'py>, v: &Value) -> PyResult` for a hook. /// /// `cmf.*` hooks → `MessagePayload` (serde-constructed from the value). @@ -170,10 +154,6 @@ pub fn resolve_payload(hook_name: &str, value: Value) -> PyResult Option { None } -// --------------------------------------------------------------------------- -// Extensions / PluginContextTable helpers -// --------------------------------------------------------------------------- - /// Deserialize Python dict → `Extensions` via serde. /// /// An empty dict yields `Extensions::default()` (all fields `#[serde(default)]`). diff --git a/bindings/python/src/manager.rs b/bindings/python/src/manager.rs index 7d3f841d..8e1e80c8 100644 --- a/bindings/python/src/manager.rs +++ b/bindings/python/src/manager.rs @@ -142,7 +142,6 @@ impl PyPluginManager { extensions: Option<&Bound<'_, PyAny>>, context_table: Option<&Bound<'_, PyAny>>, ) -> PyResult> { - // --- GIL held: convert all arguments --- let payload_value = pyobj_to_json_value(py, payload, 0)?; let rust_payload = resolve_payload(hook_name, payload_value)?; @@ -161,14 +160,6 @@ impl PyPluginManager { let manager = Arc::clone(&self.inner); let hook_name = hook_name.to_string(); - // --- GIL released: async execution with wall-clock timeout (KD7) --- - // future_into_py catches panics via tokio's JoinHandle and converts them - // to pyo3_async_runtimes.RustPanic (a PyException subclass). To keep the - // documented interface consistent — the docstring promises RuntimeError — - // we spawn invoke_by_name as an isolated tokio task and intercept the - // JoinError::is_panic() case ourselves, converting it to RuntimeError. - // This mirrors cpex-ffi's run_safely/catch_unwind pattern at the async - // boundary. future_into_py( py, invoke_with_timeout( @@ -183,10 +174,6 @@ impl PyPluginManager { } } -// --------------------------------------------------------------------------- -// Async helper — extracted so tests can inject a short timeout -// --------------------------------------------------------------------------- - /// Drives a single `invoke_by_name` call with an isolated tokio task (panic /// capture) and a wall-clock timeout, returning a Python-compatible result. /// @@ -239,19 +226,6 @@ pub(crate) async fn invoke_with_timeout( } } -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- -// -// These tests exercise the tokio::spawn + JoinError::is_panic() panic-catching -// path without requiring a live Python interpreter. They run under plain -// `cargo test -p cpex-python` (cdylibs produce a separate test binary that -// doesn't link against libpython). -// -// The test mirrors the FFI crate's `cpex_invoke_returns_rc_panic_when_plugin_panics` -// pattern: register a plugin that unconditionally panics, invoke it, verify the -// panic is caught and surfaced as a JoinError rather than aborting the process. - #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/bindings/python/src/result.rs b/bindings/python/src/result.rs index 7a96fc33..1011d2b9 100644 --- a/bindings/python/src/result.rs +++ b/bindings/python/src/result.rs @@ -132,10 +132,6 @@ impl PyPipelineResult { } } -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - /// Convert a `PipelineResult` from the Rust runtime into `PyPipelineResult`. /// /// If `modified_payload` is present but cannot be serialised, a synthetic diff --git a/builtins/pdps/cedar-direct/src/cedar_attrs.rs b/builtins/pdps/cedar-direct/src/cedar_attrs.rs index b0a24b21..0733e548 100644 --- a/builtins/pdps/cedar-direct/src/cedar_attrs.rs +++ b/builtins/pdps/cedar-direct/src/cedar_attrs.rs @@ -44,13 +44,6 @@ pub const ATTR_TEAMS: &str = "teams"; /// `apl-cmf`'s `claim.*` bag keys. pub const ATTR_CLAIMS: &str = "claims"; -// ----- JSON wrapping keys (Cedar's entity-from-JSON shape) --------- -// -// These aren't entity attributes per se — they're the top-level -// keys of the JSON shape Cedar expects when reading an entity from -// `Entity::from_json_value`. Kept here so the entity-builder code -// stays free of magic strings. - /// `uid` — the {type, id} envelope at the top of an entity JSON. pub const KEY_UID: &str = "uid"; diff --git a/builtins/pdps/cedar-direct/src/entities.rs b/builtins/pdps/cedar-direct/src/entities.rs index c6d30628..616b99cc 100644 --- a/builtins/pdps/cedar-direct/src/entities.rs +++ b/builtins/pdps/cedar-direct/src/entities.rs @@ -193,10 +193,6 @@ pub fn build_resource( }) } -// ===================================================================== -// Helpers -// ===================================================================== - /// Apply the optional namespace to a bare entity type. `Some("Acme")` + /// `"User"` → `"Acme::User"`. `None` → `"User"`. Lets operators with /// namespaced schemas (`Acme::User`, `Acme::Document`) work without diff --git a/builtins/pdps/cedar-direct/src/resolver.rs b/builtins/pdps/cedar-direct/src/resolver.rs index 2a26e713..f761cacb 100644 --- a/builtins/pdps/cedar-direct/src/resolver.rs +++ b/builtins/pdps/cedar-direct/src/resolver.rs @@ -120,7 +120,6 @@ impl CedarDirectResolver { .as_mapping() .ok_or_else(|| BuildError::ConfigShape("Cedar PDP config must be a mapping".into()))?; - // ----- policy source ----- let policy_text = read_yaml_string(map, "policy_text"); let policy_file = read_yaml_string(map, "policy_file"); let policies = match (policy_text, policy_file) { @@ -141,7 +140,6 @@ impl CedarDirectResolver { .parse() .map_err(|e: cedar_policy::ParseErrors| BuildError::PolicyParse(e.to_string()))?; - // ----- optional schema ----- let schema_text = read_yaml_string(map, "schema_text"); let schema_file = read_yaml_string(map, "schema_file"); let schema = match (schema_text, schema_file) { @@ -157,7 +155,6 @@ impl CedarDirectResolver { (None, None) => None, }; - // ----- optional dialect override ----- let dialect = match read_yaml_string(map, "dialect").as_deref() { None | Some("cedar") => PdpDialect::Cedar, Some(other) => PdpDialect::Custom(other.to_string()), @@ -256,10 +253,6 @@ impl PdpResolver for CedarDirectResolver { } } -// ===================================================================== -// Helpers -// ===================================================================== - fn parse_schema(text: &str) -> Result { Schema::from_cedarschema_str(text) .map(|(schema, _warnings)| schema) diff --git a/builtins/plugins/identity-jwt/src/resolver.rs b/builtins/plugins/identity-jwt/src/resolver.rs index 00234a50..a9434f6a 100644 --- a/builtins/plugins/identity-jwt/src/resolver.rs +++ b/builtins/plugins/identity-jwt/src/resolver.rs @@ -583,10 +583,6 @@ impl HookHandler for JwtIdentityResolver { } } -// ===================================================================== -// Internal helpers -// ===================================================================== - /// Pull the `iss` claim out of a JWT *without* verifying the /// signature. Used purely to look up which trusted issuer config /// to validate against next. diff --git a/crates/apl-cmf/src/capability_namespaces.rs b/crates/apl-cmf/src/capability_namespaces.rs index 7bbdbe80..5c3cb4f2 100644 --- a/crates/apl-cmf/src/capability_namespaces.rs +++ b/crates/apl-cmf/src/capability_namespaces.rs @@ -48,7 +48,6 @@ struct CapabilityEntry { /// `filter_extensions` rules and the per-extension extractor /// modules (`security.rs`, `delegation.rs`, etc.). const TABLE: &[CapabilityEntry] = &[ - // ----- Subject identity ----- CapabilityEntry { name: CAP_READ_SUBJECT, // `read_subject` exposes id + type only; `authenticated` is @@ -92,7 +91,6 @@ const TABLE: &[CapabilityEntry] = &[ BAG_AUTHENTICATED, ], }, - // ----- Security extension (non-subject) ----- CapabilityEntry { // Labels are not extracted into discrete bag keys today — // they live on `Extensions.security.labels` and plugins @@ -111,7 +109,6 @@ const TABLE: &[CapabilityEntry] = &[ // Exposes both inbound caller workload AND this-host workload. prefixes: &[BAG_WORKLOAD_PREFIX, BAG_CALLER_WORKLOAD_PREFIX], }, - // ----- Credential material — payload-only, no bag prefixes ----- CapabilityEntry { // Gates `Extensions.raw_credentials.inbound_tokens` — those // tokens flow through plugin payloads (IdentityPayload, @@ -123,12 +120,10 @@ const TABLE: &[CapabilityEntry] = &[ name: CAP_READ_DELEGATED_TOKENS, prefixes: &[], }, - // ----- Delegation chain ----- CapabilityEntry { name: CAP_READ_DELEGATION, prefixes: &[BAG_DELEGATION_PREFIX, BAG_DELEGATED], }, - // ----- Other extensions ----- CapabilityEntry { name: CAP_READ_AGENT, prefixes: &[BAG_AGENT_PREFIX], diff --git a/crates/apl-cmf/src/constants.rs b/crates/apl-cmf/src/constants.rs index d1e30619..5e41adc7 100644 --- a/crates/apl-cmf/src/constants.rs +++ b/crates/apl-cmf/src/constants.rs @@ -23,27 +23,19 @@ // prevent drift. Tests in `capability_namespaces` flag the // contract. -// ===================================================================== -// Capability names — must match cpex-core's vocabulary -// ===================================================================== - -// ----- Subject identity (read) ----- pub const CAP_READ_SUBJECT: &str = "read_subject"; pub const CAP_READ_ROLES: &str = "read_roles"; pub const CAP_READ_PERMISSIONS: &str = "read_permissions"; pub const CAP_READ_TEAMS: &str = "read_teams"; pub const CAP_READ_CLAIMS: &str = "read_claims"; -// ----- Security extension (non-subject) ----- pub const CAP_READ_LABELS: &str = "read_labels"; pub const CAP_READ_CLIENT: &str = "read_client"; pub const CAP_READ_WORKLOAD: &str = "read_workload"; -// ----- Credential material — payload-only, no bag prefixes ----- pub const CAP_READ_INBOUND_CREDENTIALS: &str = "read_inbound_credentials"; pub const CAP_READ_DELEGATED_TOKENS: &str = "read_delegated_tokens"; -// ----- Per-extension reads ----- pub const CAP_READ_DELEGATION: &str = "read_delegation"; pub const CAP_READ_AGENT: &str = "read_agent"; pub const CAP_READ_META: &str = "read_meta"; @@ -56,22 +48,10 @@ pub const CAP_READ_PROVENANCE: &str = "read_provenance"; pub const CAP_READ_FRAMEWORK: &str = "read_framework"; pub const CAP_READ_CUSTOM: &str = "read_custom"; -// ----- Write tokens — don't unlock bag attributes ----- pub const CAP_APPEND_LABELS: &str = "append_labels"; pub const CAP_APPEND_DELEGATION: &str = "append_delegation"; pub const CAP_WRITE_HEADERS: &str = "write_headers"; -// ===================================================================== -// Bag-attribute prefixes (and exact-match keys) — must match what -// the apl-cmf extractor modules write. -// -// Prefixes ending in `.` match any key starting with them -// (e.g. `BAG_ROLE_PREFIX` matches `role.hr`, `role.admin`). -// Prefixes WITHOUT a trailing `.` match the exact bag key -// (e.g. `BAG_AUTHENTICATED` matches only `authenticated`). -// ===================================================================== - -// ----- Subject ----- pub const BAG_SUBJECT_ID: &str = "subject.id"; pub const BAG_SUBJECT_TYPE: &str = "subject.type"; pub const BAG_SUBJECT_TEAMS: &str = "subject.teams"; @@ -81,25 +61,16 @@ pub const BAG_PERM_PREFIX: &str = "perm."; pub const BAG_TEAM_PREFIX: &str = "team."; pub const BAG_CLAIM_PREFIX: &str = "claim."; -// ----- Payload (args / result) ----- -// -// These are the dotted-prefix forms used when apl-cmf::payload flattens -// the request's args object and the upstream's result object into the -// bag. APL predicates / Cedar `${args.X}` substitutions / OPA `input.X` -// paths all resolve through these. pub const BAG_ARGS_PREFIX: &str = "args."; pub const BAG_RESULT_PREFIX: &str = "result."; -// ----- Client + workload ----- pub const BAG_CLIENT_PREFIX: &str = "client."; pub const BAG_WORKLOAD_PREFIX: &str = "workload."; pub const BAG_CALLER_WORKLOAD_PREFIX: &str = "caller_workload."; -// ----- Delegation ----- pub const BAG_DELEGATION_PREFIX: &str = "delegation."; pub const BAG_DELEGATED: &str = "delegated"; -// ----- Other extensions ----- pub const BAG_AGENT_PREFIX: &str = "agent."; pub const BAG_META_PREFIX: &str = "meta."; pub const BAG_REQUEST_PREFIX: &str = "request."; diff --git a/crates/apl-core/src/pipeline.rs b/crates/apl-core/src/pipeline.rs index 2514da83..0c544218 100644 --- a/crates/apl-core/src/pipeline.rs +++ b/crates/apl-core/src/pipeline.rs @@ -64,7 +64,6 @@ pub enum ScanKind { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Stage { - // ----- Validators (halt with deny on failure) ----- Type(TypeCheck), /// `regex("pattern")` — parser captures the pattern; evaluator stubbed /// until we add the `regex` crate dependency. @@ -91,8 +90,6 @@ pub enum Stage { values: Vec, }, - // ----- Transforms (produce a new value) ----- - /// `mask(N)` — replace all but last N chars with `*`. Mask { keep_last: usize, }, @@ -108,7 +105,6 @@ pub enum Stage { /// `hash` — replace value with a hash digest. Hash, - // ----- Effects (deferred to step 5c — IR captured, eval stubbed) ----- Taint { label: String, scopes: Vec, diff --git a/crates/apl-core/src/route.rs b/crates/apl-core/src/route.rs index 3ede63a0..292f43e3 100644 --- a/crates/apl-core/src/route.rs +++ b/crates/apl-core/src/route.rs @@ -96,7 +96,6 @@ pub async fn evaluate_pre( let mut taints: Vec = Vec::new(); let mut args_modified = false; - // ----- args ----- for rule in &route.args { let Some(current) = get_dotted(&payload.args, &rule.field).cloned() else { continue; // missing field → no pipeline to run @@ -141,7 +140,6 @@ pub async fn evaluate_pre( } } - // ----- policy ----- let policy_eval = evaluate_effects( &route.policy, bag, @@ -185,7 +183,6 @@ pub async fn evaluate_post( let mut taints: Vec = Vec::new(); let mut result_modified = false; - // ----- result (only when a response payload is present) ----- if let Some(result) = payload.result.as_mut() { for rule in &route.result { let Some(current) = get_dotted(result, &rule.field).cloned() else { @@ -232,7 +229,6 @@ pub async fn evaluate_post( } } - // ----- post_policy ----- let post_eval = evaluate_effects( &route.post_policy, bag, @@ -297,10 +293,6 @@ pub async fn evaluate_route( } } -// ===================================================================== -// Dotted-path JSON helpers -// ===================================================================== - /// Read `root.a.b.c` from a JSON value via dot-separated path. Returns /// `None` if any segment is missing or the path crosses a non-object. pub(crate) fn get_dotted<'a>( @@ -381,8 +373,6 @@ mod tests { use async_trait::async_trait; use serde_json::json; - // ----- Fixtures ----- - struct AllowPdp; #[async_trait] impl PdpResolver for AllowPdp { @@ -482,8 +472,6 @@ mod tests { ) } - // ----- Tests ----- - #[tokio::test] async fn empty_route_allows() { let route = CompiledRoute::new("noop"); @@ -871,8 +859,6 @@ mod tests { assert_eq!(payload.result.as_ref().unwrap()["ssn"], json!("[REDACTED]")); } - // ----- Helper unit tests ----- - #[test] fn dotted_get_simple_and_nested() { let v = json!({ "a": { "b": { "c": 7 } } }); @@ -906,8 +892,6 @@ mod tests { assert!(!remove_dotted(&mut v, "a.b")); } - // ----- evaluate_pre / evaluate_post (phase split) ----- - #[tokio::test] async fn evaluate_pre_runs_args_and_policy_only() { // Route with both args validators + result transforms. evaluate_pre diff --git a/crates/apl-core/src/rules.rs b/crates/apl-core/src/rules.rs index 8e3b0de7..48b9ba9c 100644 --- a/crates/apl-core/src/rules.rs +++ b/crates/apl-core/src/rules.rs @@ -889,8 +889,6 @@ mod tests { ); } - // ----- E3: parallel-purity validation ----- - #[test] fn validate_parallel_pure_block_passes() { // A parallel block of read-only effects validates clean. diff --git a/crates/apl-core/src/step.rs b/crates/apl-core/src/step.rs index 5cfa7094..b1cfe501 100644 --- a/crates/apl-core/src/step.rs +++ b/crates/apl-core/src/step.rs @@ -332,10 +332,6 @@ impl PdpDialect { } } -// ===================================================================== -// Resolver traits -// ===================================================================== - /// External policy-decision dispatch. Implemented by Cedar, OPA HTTP /// clients, AuthZen clients, NeMo Guardrails — anything that can answer /// "given this call, allow or deny?" against a request context. @@ -540,10 +536,6 @@ impl DelegationInvoker for NoopDelegationInvoker { } } -// ===================================================================== -// Elicitation dispatch -// ===================================================================== - /// Elicitation dispatch — drives a human-in-the-loop step (approval, /// confirmation, step-up, …) through a channel plugin. apl-cpex /// implements this against the named `ElicitationHandler` plugin @@ -817,10 +809,6 @@ impl ElicitationInvoker for AutoApprovingElicitor { } } -// ===================================================================== -// Resolver results -// ===================================================================== - /// What a PDP returned. #[derive(Debug, Clone, PartialEq)] pub struct PdpDecision { @@ -860,10 +848,6 @@ impl PluginOutcome { } } -// ===================================================================== -// Errors -// ===================================================================== - #[derive(Debug, Error)] pub enum PdpError { #[error("no PDP resolver registered for dialect {0:?}")] @@ -882,10 +866,6 @@ pub enum PluginError { Dispatch(String), } -// ===================================================================== -// Convenience -// ===================================================================== - impl Step { /// Wrap a `Rule` as a `Step`. Saves typing in tests and parser code. pub fn rule(r: Rule) -> Self { diff --git a/crates/apl-cpex/src/cmf_invoker.rs b/crates/apl-cpex/src/cmf_invoker.rs index 3419f0ef..4677523f 100644 --- a/crates/apl-cpex/src/cmf_invoker.rs +++ b/crates/apl-cpex/src/cmf_invoker.rs @@ -271,9 +271,6 @@ impl PluginInvoker for CmfPluginInvoker { ext.meta.as_ref().and_then(|m| m.entity_type.clone()) }; - // Pick the entry whose registered hook matches the current - // dispatch context via cpex-core's hook metadata table. - // Replaces the prior naming heuristic. let dispatch_phase = match invocation.phase() { DispatchPhase::Pre => HookPhase::Pre, DispatchPhase::Post => HookPhase::Post, @@ -386,10 +383,6 @@ impl PluginInvoker for CmfPluginInvoker { } } -// ===================================================================== -// Helpers -// ===================================================================== - /// Snapshot `extensions.security.labels` as an owned `HashSet`. /// Empty when security is absent. fn snapshot_labels(extensions: &Extensions) -> HashSet { diff --git a/crates/apl-cpex/src/dispatch_plan.rs b/crates/apl-cpex/src/dispatch_plan.rs index 8023c268..30437c59 100644 --- a/crates/apl-cpex/src/dispatch_plan.rs +++ b/crates/apl-cpex/src/dispatch_plan.rs @@ -199,10 +199,6 @@ impl RouteDispatchPlan { continue; } - // Store every (hook_name, HookEntry) pair the plugin - // registered. Dispatch-time entry selection (pick_entry) - // consults cpex-core's hook routing table per hook name. - // Replaces the prior naming heuristic. let mut entries_by_hook: HashMap = HashMap::new(); for (hook_name, entry) in entries { entries_by_hook.insert(hook_name, entry); diff --git a/crates/apl-cpex/src/parallel_safety.rs b/crates/apl-cpex/src/parallel_safety.rs index d5018730..1482e712 100644 --- a/crates/apl-cpex/src/parallel_safety.rs +++ b/crates/apl-cpex/src/parallel_safety.rs @@ -187,10 +187,6 @@ fn is_safe_in_parallel(mode: PluginMode) -> bool { ) } -// ===================================================================== -// Tests -// ===================================================================== - #[cfg(test)] mod tests { use super::*; @@ -236,8 +232,6 @@ mod tests { Effect::Parallel(vec![Effect::Plugin { name: name.into() }]) } - // --- Allowed modes --- - #[test] fn audit_plugin_in_parallel_is_accepted() { let reg = MockLookup::new().with("audit_logger", PluginMode::Audit); @@ -259,8 +253,6 @@ mod tests { assert!(validate_parallel_plugin_modes(&route, ®).is_ok()); } - // --- Rejected modes --- - #[test] fn sequential_plugin_in_parallel_is_rejected() { let reg = MockLookup::new().with("mutator", PluginMode::Sequential); @@ -288,8 +280,6 @@ mod tests { assert!(err.contains("ghost")); } - // --- Scoping: only mismatches INSIDE a parallel block are caught --- - #[test] fn sequential_plugin_outside_parallel_is_allowed() { // The same Sequential-mode plugin is fine at the top level — @@ -316,8 +306,6 @@ mod tests { assert!(err.contains("mutator")); } - // --- Diagnostics: every violation, both phases --- - #[test] fn multiple_violations_all_reported() { // Surface every violation in one pass so the operator can fix diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index 8c8457bf..2128aeaa 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -567,10 +567,6 @@ 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 @@ -754,10 +750,6 @@ fn extensions_changed(before: &Extensions, after: &Extensions) -> bool { security_changed || delegation_changed || raw_creds_changed } -// --------------------------------------------------------------------- -// Phase 5: pending elicitation ↔ wire (`-32120`) -// --------------------------------------------------------------------- - /// Extract the elicitation id an agent echoes on retry from the /// `X-Policy-Elicitation-Id` request header. `None` when absent/empty. /// Pure so it's unit-testable without the full handler path. diff --git a/crates/apl-cpex/src/session_resolver.rs b/crates/apl-cpex/src/session_resolver.rs index ec444ac9..27b097b0 100644 --- a/crates/apl-cpex/src/session_resolver.rs +++ b/crates/apl-cpex/src/session_resolver.rs @@ -189,10 +189,6 @@ pub fn resolve_session(ext: &Extensions) -> Option<(String, SessionSource)> { None } -// ===================================================================== -// Tests — one scenario per tier, plus tier-priority assertions. -// ===================================================================== - #[cfg(test)] mod tests { use super::*; @@ -234,8 +230,6 @@ mod tests { } } - // --- Tier 0: agent (pre-resolved) --- - #[test] fn tier0_agent_session_id_is_subject_bound() { // A pre-resolved (client-supplied) session id is hashed together @@ -361,8 +355,6 @@ mod tests { assert_eq!(sid, subject_scoped(Some("alice"), "from-agent").unwrap()); } - // --- Tier 1: token_claim --- - #[test] fn tier1_token_claim_hits_when_session_id_claim_present() { let sec = SecurityExtension { @@ -498,15 +490,6 @@ mod tests { assert_eq!(sid, subject_scoped(Some("alice"), "from-claim").unwrap()); } - // --- Tier 2 (`X-CPEX-Session-Id` header) is intentionally absent --- - // - // The Python `SessionResolver` included a header tier; cpex Rust - // does not. See the module-level doc comment for the threat model. - // A spoofing-regression guard lives below in - // `header_x_cpex_session_id_is_ignored`. - - // --- Tier 2: identity --- - #[test] fn tier2_identity_derived_when_no_claim() { let sec = SecurityExtension { @@ -591,8 +574,6 @@ mod tests { assert_ne!(sid1, sid2); } - // --- Tier 3: none --- - #[test] fn tier3_no_session_when_no_data() { let ext = Extensions::default(); @@ -611,8 +592,6 @@ mod tests { assert!(resolve_session(&ext).is_none()); } - // --- Wire-format documentation --- - #[test] fn separator_format_collides_when_subject_contains_colon() { // Document — but do not silently ignore — the colon-separator @@ -639,8 +618,6 @@ mod tests { ); } - // --- Spoofing guard (regression test for P0-2) --- - #[test] fn header_x_cpex_session_id_is_ignored() { // The Python apl-plugins resolver honored an `X-CPEX-Session-Id` diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index 7164fed6..f166e13b 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -702,10 +702,6 @@ impl ConfigVisitor for AplConfigVisitor { } } -// ===================================================================== -// Helpers -// ===================================================================== - #[allow(clippy::too_many_arguments)] fn install_handler( mgr: &Arc, diff --git a/crates/cpex-builtins/src/lib.rs b/crates/cpex-builtins/src/lib.rs index 46276552..296a3ced 100644 --- a/crates/cpex-builtins/src/lib.rs +++ b/crates/cpex-builtins/src/lib.rs @@ -34,10 +34,6 @@ use apl_core::step::PdpFactory; use apl_cpex::{register_apl, AplOptions, SessionStoreFactory}; use cpex_core::manager::PluginManager; -// ----------------------------------------------------------------------------- -// Feature-gated re-exports of each builtin's factory + KIND -// ----------------------------------------------------------------------------- - #[cfg(feature = "cedar-direct")] pub use cpex_pdp_cedar_direct::CedarDirectPdpFactory; #[cfg(feature = "cel")] @@ -55,10 +51,6 @@ pub use cpex_plugin_pii_scanner::{PiiScannerFactory, KIND as PII_KIND}; #[cfg(feature = "valkey")] pub use cpex_session_valkey::{ValkeyConfig, ValkeySessionStoreFactory, KIND as VALKEY_KIND}; -// ----------------------------------------------------------------------------- -// Plugin-factory registration (by-kind axis) -// ----------------------------------------------------------------------------- - /// Generate [`register_builtins`] from a feature → factory table. Each entry /// expands to a `#[cfg(feature = ...)]`-gated, **explicit** /// `register_factory(KIND, Box::new(Factory))` call keyed off the builtin @@ -97,10 +89,6 @@ register_builtins! { feature "audit-logger" => cpex_plugin_audit_logger::AuditLoggerFactory, } -// ----------------------------------------------------------------------------- -// PDP-factory and session-store axes -// ----------------------------------------------------------------------------- - /// The enabled PDP factories, ready to drop into /// [`AplOptions::pdp_factories`]. A route's `cedar:` or `cel:` step selects /// which one runs. @@ -130,10 +118,6 @@ pub fn builtin_session_store_factories() -> Vec> { factories } -// ----------------------------------------------------------------------------- -// One-call install -// ----------------------------------------------------------------------------- - /// Register every enabled plugin factory and install the APL config visitor /// on `mgr` with in-process defaults (a `MemorySessionStore` and the default /// baseline capabilities). The enabled PDP and session-store factories are @@ -152,10 +136,6 @@ pub fn install_builtins(mgr: &Arc) { let _visitor = register_apl(mgr, opts); } -// ----------------------------------------------------------------------------- -// Tests -// ----------------------------------------------------------------------------- - #[cfg(test)] mod tests { use super::*; diff --git a/crates/cpex-core/src/cmf/constants.rs b/crates/cpex-core/src/cmf/constants.rs index 1be8e581..b8fb68b8 100644 --- a/crates/cpex-core/src/cmf/constants.rs +++ b/crates/cpex-core/src/cmf/constants.rs @@ -8,10 +8,6 @@ /// Current CMF message schema version. pub const SCHEMA_VERSION: &str = "2.0"; -// --------------------------------------------------------------------------- -// Serialization field names for MessageView::to_dict() / to_opa_input() -// --------------------------------------------------------------------------- - // Core view fields pub const FIELD_KIND: &str = "kind"; pub const FIELD_ROLE: &str = "role"; @@ -64,13 +60,6 @@ pub const FIELD_TAGS: &str = "tags"; // OPA envelope pub const FIELD_OPA_INPUT: &str = "input"; -// --------------------------------------------------------------------------- -// Entity type identifiers — used in MetaExtension.entity_type and as the -// keys for `global.defaults` per-entity-type policy groups. These are the -// MCP entity taxonomy: tools (callable functions), LLMs (model -// invocations), prompts (template fills), resources (URI fetches). -// --------------------------------------------------------------------------- - pub const ENTITY_TOOL: &str = "tool"; pub const ENTITY_LLM: &str = "llm"; pub const ENTITY_PROMPT: &str = "prompt"; @@ -85,16 +74,6 @@ 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 -// type — pre-invocation (called from APL's policy / args phase) and -// post-invocation (called from APL's post_policy / result phase). -// -// Used as keys in `hooks::metadata`'s routing table and from plugin -// declarations. -// --------------------------------------------------------------------------- - pub const HOOK_CMF_TOOL_PRE_INVOKE: &str = "cmf.tool_pre_invoke"; pub const HOOK_CMF_TOOL_POST_INVOKE: &str = "cmf.tool_post_invoke"; pub const HOOK_CMF_LLM_INPUT: &str = "cmf.llm_input"; diff --git a/crates/cpex-core/src/cmf/content.rs b/crates/cpex-core/src/cmf/content.rs index 6c5e6f86..938862de 100644 --- a/crates/cpex-core/src/cmf/content.rs +++ b/crates/cpex-core/src/cmf/content.rs @@ -18,10 +18,6 @@ use serde::{Deserialize, Serialize}; use super::enums::ResourceType; use super::message::Message; -// --------------------------------------------------------------------------- -// Domain Objects -// --------------------------------------------------------------------------- - /// Normalized tool/function invocation request. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolCall { @@ -158,10 +154,6 @@ pub struct PromptResult { pub error_message: Option, } -// --------------------------------------------------------------------------- -// Media Source Types -// --------------------------------------------------------------------------- - /// Image source data. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ImageSource { @@ -223,10 +215,6 @@ pub struct DocumentSource { pub title: Option, } -// --------------------------------------------------------------------------- -// ContentPart — Tagged Enum -// --------------------------------------------------------------------------- - /// A typed content part in a CMF message. /// /// Discriminated by the `content_type` field. Each variant wraps diff --git a/crates/cpex-core/src/cmf/enums.rs b/crates/cpex-core/src/cmf/enums.rs index 97f1dcc2..e976e582 100644 --- a/crates/cpex-core/src/cmf/enums.rs +++ b/crates/cpex-core/src/cmf/enums.rs @@ -10,10 +10,6 @@ use serde::{Deserialize, Serialize}; -// --------------------------------------------------------------------------- -// Role -// --------------------------------------------------------------------------- - /// Identifies WHO is speaking in a conversation turn. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -30,10 +26,6 @@ pub enum Role { Tool, } -// --------------------------------------------------------------------------- -// Channel -// --------------------------------------------------------------------------- - /// Classifies the kind of output a message represents. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -46,10 +38,6 @@ pub enum Channel { Final, } -// --------------------------------------------------------------------------- -// ContentType -// --------------------------------------------------------------------------- - /// Discriminator for the typed ContentPart hierarchy. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -80,10 +68,6 @@ pub enum ContentType { Document, } -// --------------------------------------------------------------------------- -// ResourceType -// --------------------------------------------------------------------------- - /// Type of resource being referenced. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] diff --git a/crates/cpex-core/src/cmf/message.rs b/crates/cpex-core/src/cmf/message.rs index 6a13a2ec..4eb7678c 100644 --- a/crates/cpex-core/src/cmf/message.rs +++ b/crates/cpex-core/src/cmf/message.rs @@ -22,10 +22,6 @@ use super::content::*; use super::enums::{Channel, Role}; use crate::hooks::trait_def::PluginResult; -// --------------------------------------------------------------------------- -// Message -// --------------------------------------------------------------------------- - /// Canonical CMF message representing a single turn in a conversation. /// /// All content is carried as typed ContentPart variants. Extensions @@ -211,10 +207,6 @@ impl Message { } } -// --------------------------------------------------------------------------- -// MessagePayload — PluginPayload wrapper -// --------------------------------------------------------------------------- - /// CMF Message wrapped as a PluginPayload for hook dispatch. /// /// This is the payload type for all `cmf.*` hooks. Plugins that @@ -228,10 +220,6 @@ pub struct MessagePayload { crate::impl_plugin_payload!(MessagePayload); -// --------------------------------------------------------------------------- -// CmfHook — Hook Type Definition -// --------------------------------------------------------------------------- - crate::define_hook! { /// CMF message evaluation hook. /// diff --git a/crates/cpex-core/src/cmf/view.rs b/crates/cpex-core/src/cmf/view.rs index 23d5457c..da320a22 100644 --- a/crates/cpex-core/src/cmf/view.rs +++ b/crates/cpex-core/src/cmf/view.rs @@ -19,10 +19,6 @@ use super::enums::{ContentType, Role}; use super::message::Message; use crate::hooks::payload::Extensions; -// --------------------------------------------------------------------------- -// Enums -// --------------------------------------------------------------------------- - /// Type of content a view represents. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -125,10 +121,6 @@ impl ViewKind { } } -// --------------------------------------------------------------------------- -// MessageView -// --------------------------------------------------------------------------- - /// Read-only, zero-copy view over a single content part. /// /// Provides a uniform interface for policy evaluation regardless @@ -532,10 +524,6 @@ impl<'a> std::fmt::Debug for MessageView<'a> { } } -// --------------------------------------------------------------------------- -// iter_views — decompose a Message into views -// --------------------------------------------------------------------------- - /// Decompose a Message into individually addressable MessageViews. /// /// Yields one view per content part. Each view provides a uniform diff --git a/crates/cpex-core/src/context.rs b/crates/cpex-core/src/context.rs index 2176c7bc..27105f8c 100644 --- a/crates/cpex-core/src/context.rs +++ b/crates/cpex-core/src/context.rs @@ -25,10 +25,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use uuid::Uuid; -// --------------------------------------------------------------------------- -// Plugin Context -// --------------------------------------------------------------------------- - /// Per-plugin, per-invocation execution context. /// /// Each plugin receives its own `PluginContext` with: @@ -104,10 +100,6 @@ impl Default for PluginContext { } } -// --------------------------------------------------------------------------- -// Plugin Context Table -// --------------------------------------------------------------------------- - /// Threaded execution state carried from one hook invocation to the next /// within a single request lifecycle (e.g., `pre_invoke` → `post_invoke`). /// diff --git a/crates/cpex-core/src/elicitation/payload.rs b/crates/cpex-core/src/elicitation/payload.rs index 7cfeaf23..1beeea0f 100644 --- a/crates/cpex-core/src/elicitation/payload.rs +++ b/crates/cpex-core/src/elicitation/payload.rs @@ -101,8 +101,6 @@ pub enum ElicitationOutcomeKind { /// updated payload). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ElicitationPayload { - // ----- Input (private — bridge-supplied, never mutated by handlers) ----- - /// Which touch-point this is. operation: ElicitationOp, /// Correlation id from a prior `Dispatch`. `None` on dispatch (the @@ -138,8 +136,6 @@ pub struct ElicitationPayload { /// selected by name). channel: Option, - // ----- Output (pub — handlers populate via direct assignment on clones) ----- - /// Correlation id minted on `Dispatch`. The agent echoes it on retry. #[serde(default, skip_serializing_if = "Option::is_none")] pub id: Option, @@ -210,8 +206,6 @@ impl ElicitationPayload { } } - // -------- Input builders -------- - /// Set the correlation id (check / validate operations). pub fn with_elicitation_id(mut self, id: impl Into) -> Self { self.elicitation_id = Some(id.into()); @@ -238,8 +232,6 @@ impl ElicitationPayload { self } - // -------- Input read accessors -------- - pub fn operation(&self) -> ElicitationOp { self.operation } @@ -272,8 +264,6 @@ impl ElicitationPayload { self.channel.as_deref() } - // -------- Host-side application helper -------- - /// Pull the resolved `ElicitationPayload` out of a `PipelineResult` /// returned by `mgr.invoke_entries::(...)`. Returns /// `None` when the pipeline was denied or the result's payload wasn't diff --git a/crates/cpex-core/src/error.rs b/crates/cpex-core/src/error.rs index 576dc5bc..fca9b51e 100644 --- a/crates/cpex-core/src/error.rs +++ b/crates/cpex-core/src/error.rs @@ -15,10 +15,6 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use thiserror::Error; -// --------------------------------------------------------------------------- -// Plugin Errors -// --------------------------------------------------------------------------- - /// Top-level error type for the CPEX framework. /// /// Covers plugin execution failures, policy violations, timeouts, @@ -94,10 +90,6 @@ impl PluginError { } } -// --------------------------------------------------------------------------- -// Plugin Error Record -// --------------------------------------------------------------------------- - /// A `Clone`-able, serialization-friendly snapshot of a `PluginError`. /// /// Used in `PipelineResult.errors` to surface execution failures from @@ -193,10 +185,6 @@ impl From<&PluginError> for PluginErrorRecord { } } -// --------------------------------------------------------------------------- -// Plugin Violations -// --------------------------------------------------------------------------- - /// Structured policy violation returned by a plugin that denies execution. /// /// Carries a machine-readable code, human-readable reason, and optional diff --git a/crates/cpex-core/src/executor.rs b/crates/cpex-core/src/executor.rs index 3d1f6934..b9e24e70 100644 --- a/crates/cpex-core/src/executor.rs +++ b/crates/cpex-core/src/executor.rs @@ -39,10 +39,6 @@ use crate::hooks::payload::{Extensions, PluginPayload, WriteToken}; use crate::plugin::OnError; use crate::registry::{group_by_mode, HookEntry}; -// --------------------------------------------------------------------------- -// Executor Configuration -// --------------------------------------------------------------------------- - /// Configuration for the executor. #[derive(Debug, Clone)] pub struct ExecutorConfig { @@ -62,10 +58,6 @@ impl Default for ExecutorConfig { } } -// --------------------------------------------------------------------------- -// Pipeline Result -// --------------------------------------------------------------------------- - /// Aggregate result from a full hook invocation across all phases. /// /// Wraps the final payload, extensions, any violation, and the @@ -161,10 +153,6 @@ impl PipelineResult { } } -// --------------------------------------------------------------------------- -// Background Tasks -// --------------------------------------------------------------------------- - /// Handles to fire-and-forget background tasks spawned by the executor. /// /// Returned separately from [`PipelineResult`] so that the policy @@ -233,10 +221,6 @@ impl fmt::Debug for BackgroundTasks { } } -// --------------------------------------------------------------------------- -// Executor -// --------------------------------------------------------------------------- - /// 5-phase plugin execution engine. /// /// Dispatches hooks through the phase pipeline: @@ -305,7 +289,6 @@ impl Executor { // become the violation directly. let mut errors: Vec = Vec::new(); - // Phase 1: SEQUENTIAL — serial, chained, can block + modify if let Some(v) = self .run_serial_phase( &sequential, @@ -325,8 +308,6 @@ impl Executor { ); } - // Phase 2: TRANSFORM — serial, chained, can modify, cannot block - // can_block=false means denials are suppressed (returns None) self.run_serial_phase( &transform, &mut current_payload, @@ -339,7 +320,6 @@ impl Executor { ) .await; - // Phase 3: AUDIT — serial, read-only, discard results self.run_ref_phase( &audit, &*current_payload, @@ -350,7 +330,6 @@ impl Executor { ) .await; - // Phase 4: CONCURRENT — parallel, can block, cannot modify if let Some(violation) = self .run_concurrent_phase( &concurrent, @@ -368,9 +347,6 @@ impl Executor { ); } - // Phase 5: FIRE_AND_FORGET — background, read-only, ignore results. - // FAF errors don't go in PipelineResult.errors — they're delivered - // via BackgroundTasks::wait_for_background_tasks() instead. let bg_handles = self.spawn_fire_and_forget( &fire_and_forget, &*current_payload, @@ -386,10 +362,6 @@ impl Executor { ) } - // ----------------------------------------------------------------------- - // Phase 1 & 2: Serial execution (SEQUENTIAL / TRANSFORM) - // ----------------------------------------------------------------------- - /// Run a serial phase — plugins execute one at a time, each seeing /// the (possibly modified) payload from the previous. /// @@ -421,10 +393,6 @@ impl Executor { let plugin_id = entry.plugin_ref.id(); let on_error = entry.plugin_ref.trusted_config().on_error; - // Take this plugin's context out of the table — pulls its stored - // local_state and seeds global_state from the canonical store. - // Replaces the previous values().last() seed, which was - // non-deterministic across HashMap iteration orders. let mut ctx = ctx_table.take_context(plugin_id); // Filter extensions per plugin based on declared capabilities. @@ -589,20 +557,12 @@ impl Executor { }, } - // Commit this plugin's context back to the table — replaces the - // canonical global_state with its (possibly modified) copy and - // stores the local_state for the next hook invocation. The - // global_state move is free; only the local_state insert allocates. ctx_table.store_context(plugin_id, ctx); } None // no denial } - // ----------------------------------------------------------------------- - // Phase 3 & 5: Read-only execution (AUDIT / FIRE_AND_FORGET) - // ----------------------------------------------------------------------- - /// Run a read-only phase — plugins receive &payload, results discarded. async fn run_ref_phase( &self, @@ -682,10 +642,6 @@ impl Executor { } } - // ----------------------------------------------------------------------- - // Phase 4: Concurrent (parallel, fail-fast) - // ----------------------------------------------------------------------- - /// Run the concurrent phase — plugins execute truly in parallel. /// Returns the first violation if any plugin denies. /// @@ -923,10 +879,6 @@ impl Executor { first_violation } - // ----------------------------------------------------------------------- - // Phase 5: Fire-and-Forget (background, no await) - // ----------------------------------------------------------------------- - /// Spawn fire-and-forget handlers as background tasks. /// /// Each handler runs in its own `tokio::spawn` — the pipeline does @@ -1011,16 +963,8 @@ impl Default for Executor { } } -// --------------------------------------------------------------------------- -// Internal types -// --------------------------------------------------------------------------- - // SerialResult removed — run_serial_phase now returns Option directly. -// --------------------------------------------------------------------------- -// Erased Result Extraction -// --------------------------------------------------------------------------- - /// Common fields extracted from a type-erased PluginResult. /// /// Handlers return `Box` which wraps this struct. The diff --git a/crates/cpex-core/src/extensions/container.rs b/crates/cpex-core/src/extensions/container.rs index 51da6a81..d9bb99f3 100644 --- a/crates/cpex-core/src/extensions/container.rs +++ b/crates/cpex-core/src/extensions/container.rs @@ -29,10 +29,6 @@ use super::raw_credentials::RawCredentialsExtension; use super::request::RequestExtension; use super::security::SecurityExtension; -// --------------------------------------------------------------------------- -// Extensions — all Arc, fully immutable, zero-copy shareable -// --------------------------------------------------------------------------- - /// Typed container for all message extensions. /// /// All slots are `Arc` — fully immutable, zero-copy shareable. @@ -258,10 +254,6 @@ impl Extensions { } } -// --------------------------------------------------------------------------- -// OwnedExtensions — plugin's writeable workspace -// --------------------------------------------------------------------------- - /// Owned copy of extensions for plugin modification. /// /// Returned by `Extensions::cow_copy()`. Immutable slots share diff --git a/crates/cpex-core/src/extensions/tiers.rs b/crates/cpex-core/src/extensions/tiers.rs index cc3592d1..7dde854b 100644 --- a/crates/cpex-core/src/extensions/tiers.rs +++ b/crates/cpex-core/src/extensions/tiers.rs @@ -39,10 +39,6 @@ pub enum MutabilityTier { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Capability { - // ----- Subject (user identity) ----- - /// Read the authenticated subject identity (`security.subject`). - /// Unlocks the slot but not its sub-fields — roles / teams / - /// claims / permissions each have their own cap below. ReadSubject, /// Read subject roles (`security.subject.roles`). ReadRoles, @@ -53,57 +49,24 @@ pub enum Capability { /// Read subject permissions (`security.subject.permissions`). ReadPermissions, - // ----- Client (OAuth application identity) ----- - /// Read the OAuth client / gateway-access identity - /// (`security.client`). Distinct from the user identity - /// (`subject`) — a single user can connect through different - /// clients (first-party browser, third-party agent) and policies - /// sometimes want to gate on the client. ReadClient, - // ----- Workload (attested SPIFFE / mTLS identity) ----- - /// Read either workload-identity slot — both - /// `security.caller_workload` (the inbound attested peer) and - /// `security.this_workload` (our own outbound identity). One - /// capability covers both: a plugin either has access to - /// attested-workload identity or it doesn't. Distinct from - /// `read_agent` which governs session / conversation context, - /// **NOT** identity. ReadWorkload, - // ----- Agent execution context (session / conversation) ----- - /// Read the agent execution context (`AgentExtension`). - /// **NOT a credential** — this carries session / conversation / - /// lineage state, not identity. Identity reads use - /// `read_subject` / `read_client` / `read_workload`. ReadAgent, - // ----- HTTP wire layer ----- - /// Read HTTP headers. ReadHeaders, /// Write (modify) HTTP headers. WriteHeaders, - // ----- Security labels (taint flow) ----- - /// Read security labels. ReadLabels, /// Append security labels (monotonic add-only). AppendLabels, - // ----- Delegation chain (validated) ----- - /// Read the delegation chain. ReadDelegation, /// Append to the delegation chain (monotonic). AppendDelegation, - // ----- Raw credentials (Layer 3) ----- - /// Read raw inbound tokens - /// (`raw_credentials.inbound_tokens`) — the bearer-token - /// strings captured at the wire layer before validation. - /// Narrowly scoped: only IdentityResolve handlers, forwarding - /// plugins, and a small set of audit plugins should declare it. - /// Out-of-process plugins can't see these tokens regardless of - /// capability — token fields are `#[serde(skip)]`. ReadInboundCredentials, /// Read minted outbound delegated tokens /// (`raw_credentials.delegated_tokens`) — the credentials a diff --git a/crates/cpex-core/src/factory.rs b/crates/cpex-core/src/factory.rs index f5c6bb5d..fdef3059 100644 --- a/crates/cpex-core/src/factory.rs +++ b/crates/cpex-core/src/factory.rs @@ -24,10 +24,6 @@ use crate::error::PluginError; use crate::plugin::{Plugin, PluginConfig}; use crate::registry::AnyHookHandler; -// --------------------------------------------------------------------------- -// Plugin Factory Trait -// --------------------------------------------------------------------------- - /// Factory for creating plugin instances from config. /// /// The host registers factories by `kind` name before loading @@ -79,10 +75,6 @@ pub struct PluginInstance { pub handlers: Vec<(&'static str, Arc)>, } -// --------------------------------------------------------------------------- -// Plugin Factory Registry -// --------------------------------------------------------------------------- - /// Registry of plugin factories keyed by `kind` name. /// /// The host populates this before calling `PluginManager::from_config()`. diff --git a/crates/cpex-core/src/hooks/adapter.rs b/crates/cpex-core/src/hooks/adapter.rs index 985108ca..c2834725 100644 --- a/crates/cpex-core/src/hooks/adapter.rs +++ b/crates/cpex-core/src/hooks/adapter.rs @@ -29,10 +29,6 @@ use crate::hooks::trait_def::{HookHandler, HookTypeDef, PluginResult}; use crate::plugin::Plugin; use crate::registry::AnyHookHandler; -// --------------------------------------------------------------------------- -// Typed Handler Adapter -// --------------------------------------------------------------------------- - /// Adapts a typed `HookHandler` into the type-erased `AnyHookHandler` /// interface used by the executor. /// diff --git a/crates/cpex-core/src/hooks/metadata.rs b/crates/cpex-core/src/hooks/metadata.rs index e0921f32..19ace802 100644 --- a/crates/cpex-core/src/hooks/metadata.rs +++ b/crates/cpex-core/src/hooks/metadata.rs @@ -162,10 +162,6 @@ impl HookMetadata { } } -// ===================================================================== -// Built-in registry -// ===================================================================== - /// Built-in hook metadata. Plugin authors and hosts can register /// additional entries via [`register_hook_metadata`]. The 8 CMF step /// hooks (entity × pre/post) are the complete CMF-routable surface diff --git a/crates/cpex-core/src/hooks/payload.rs b/crates/cpex-core/src/hooks/payload.rs index d284bf4c..ef3e6950 100644 --- a/crates/cpex-core/src/hooks/payload.rs +++ b/crates/cpex-core/src/hooks/payload.rs @@ -29,10 +29,6 @@ use std::fmt; // compatibility with existing code that imports from hooks::payload. pub use crate::extensions::{Extensions, Guarded, MetaExtension, OwnedExtensions, WriteToken}; -// --------------------------------------------------------------------------- -// PluginPayload Trait -// --------------------------------------------------------------------------- - /// Base trait for all hook payloads. /// /// Mirrors Python's `PluginPayload(BaseModel, frozen=True)`. Every @@ -98,10 +94,6 @@ impl fmt::Debug for dyn PluginPayload { } } -// --------------------------------------------------------------------------- -// Blanket helper macro for implementing PluginPayload -// --------------------------------------------------------------------------- - /// Implements `PluginPayload` for a type that is `Clone + Send + Sync + 'static`. /// /// Saves boilerplate — instead of writing the three methods manually, diff --git a/crates/cpex-core/src/hooks/trait_def.rs b/crates/cpex-core/src/hooks/trait_def.rs index b75d2b8a..cdd2cefc 100644 --- a/crates/cpex-core/src/hooks/trait_def.rs +++ b/crates/cpex-core/src/hooks/trait_def.rs @@ -24,10 +24,6 @@ use crate::error::PluginViolation; use crate::hooks::payload::{Extensions, PluginPayload}; use crate::plugin::Plugin; -// --------------------------------------------------------------------------- -// HookTypeDef Trait -// --------------------------------------------------------------------------- - /// Defines a hook's contract: what goes in and what comes out. /// /// Each hook type is a zero-sized marker struct that implements this @@ -69,10 +65,6 @@ pub trait HookTypeDef: Send + Sync + 'static { const NAME: &'static str; } -// --------------------------------------------------------------------------- -// Hook Handler Trait -// --------------------------------------------------------------------------- - /// Typed handler for a specific hook type. /// /// Plugin authors implement this trait (alongside [`Plugin`]) to handle @@ -168,10 +160,6 @@ pub trait HookHandler: Plugin + Send + Sync { ) -> impl std::future::Future + Send; } -// --------------------------------------------------------------------------- -// Plugin Result -// --------------------------------------------------------------------------- - /// Result returned by a hook handler. /// /// Payload and extension modifications are **separate** — this is a diff --git a/crates/cpex-core/src/hooks/types.rs b/crates/cpex-core/src/hooks/types.rs index 3295d1a6..8bc2b3e1 100644 --- a/crates/cpex-core/src/hooks/types.rs +++ b/crates/cpex-core/src/hooks/types.rs @@ -18,10 +18,6 @@ use std::fmt; use serde::{Deserialize, Serialize}; -// --------------------------------------------------------------------------- -// Hook Type -// --------------------------------------------------------------------------- - /// A named hook point in the host's execution lifecycle. /// /// Wraps a string identifier. Hook types are open — hosts register @@ -75,12 +71,6 @@ impl From for HookType { } } -// --------------------------------------------------------------------------- -// Built-in Hook String Constants -// --------------------------------------------------------------------------- -// Canonical string names for built-in hooks. Use these with -// HookType::new() or pass them directly to APIs that accept &str. - /// Legacy hook names — typed payloads (ToolPreInvokePayload, etc.). pub mod hook_names { // Tool lifecycle @@ -121,10 +111,6 @@ pub mod cmf_hook_names { pub const RESOURCE_POST_FETCH: &str = "cmf.resource_post_fetch"; } -// --------------------------------------------------------------------------- -// Built-in hook type helpers -// --------------------------------------------------------------------------- - /// Returns all built-in hook types with their canonical string values. /// /// Called once during PluginManager initialization to populate the diff --git a/crates/cpex-core/src/identity/payload.rs b/crates/cpex-core/src/identity/payload.rs index 42c95221..f1626ade 100644 --- a/crates/cpex-core/src/identity/payload.rs +++ b/crates/cpex-core/src/identity/payload.rs @@ -113,9 +113,6 @@ impl Default for TokenSource { /// existing Sequential-phase machinery — no bespoke plumbing. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct IdentityPayload { - // ----- Input (private — host-supplied, never mutated by handlers) ----- - /// Raw credential bytes. Cleared on drop via `Zeroizing`. - /// `#[serde(skip)]` — never appears in serialized output. #[serde(skip)] raw_token: Zeroizing, @@ -138,8 +135,6 @@ pub struct IdentityPayload { #[serde(default, skip_serializing_if = "Option::is_none")] client_port: Option, - // ----- Output (pub — handlers populate via direct assignment on clones) ----- - /// Resolved user identity. `None` until a handler populates it. #[serde(default, skip_serializing_if = "Option::is_none")] pub subject: Option, @@ -197,8 +192,6 @@ impl IdentityPayload { } } - // -------- Input builders -------- - pub fn with_source_header(mut self, h: impl Into) -> Self { self.source_header = Some(h.into()); self @@ -219,8 +212,6 @@ impl IdentityPayload { self } - // -------- Input read accessors (no mutable variants) -------- - /// The raw credential bytes. Borrowed — handlers cannot move /// or replace the underlying `Zeroizing` through this /// accessor. @@ -248,8 +239,6 @@ impl IdentityPayload { self.client_port } - // -------- Output helpers -------- - /// Layer another payload's *output* fields onto this one's, /// following "Some replaces None, last write wins per slot." /// Input fields are not touched — the running payload's input @@ -283,8 +272,6 @@ impl IdentityPayload { } } - // -------- Host-side application helpers -------- - /// Pull the resolved `IdentityPayload` out of a `PipelineResult` /// returned by `mgr.invoke_named::(...)`. Returns /// `None` when the pipeline was denied (no `modified_payload`) diff --git a/crates/cpex-core/src/manager.rs b/crates/cpex-core/src/manager.rs index 9f94209f..d14f75d8 100644 --- a/crates/cpex-core/src/manager.rs +++ b/crates/cpex-core/src/manager.rs @@ -44,10 +44,6 @@ use crate::hooks::HookType; use crate::plugin::{Plugin, PluginConfig}; use crate::registry::{AnyHookHandler, PluginRef, PluginRegistry}; -// --------------------------------------------------------------------------- -// Manager Configuration -// --------------------------------------------------------------------------- - /// Default upper bound on the routing cache. Caps memory growth from /// attacker-controlled entity names without forcing operators to tune. pub const DEFAULT_ROUTE_CACHE_MAX_ENTRIES: usize = 10_000; @@ -74,10 +70,6 @@ impl Default for ManagerConfig { } } -// --------------------------------------------------------------------------- -// Plugin Manager -// --------------------------------------------------------------------------- - /// Central plugin lifecycle and dispatch manager. /// /// Owns the plugin registry and executor. Provides the public API @@ -436,10 +428,6 @@ impl PluginManager { self.generation.load(Ordering::Acquire) } - // ----------------------------------------------------------------------- - // Factory Registration - // ----------------------------------------------------------------------- - /// Register a plugin factory for a given `kind` name. /// /// The host calls this to tell the manager how to create plugins @@ -464,10 +452,6 @@ impl PluginManager { .register(kind, factory); } - // ----------------------------------------------------------------------- - // Config Loading - // ----------------------------------------------------------------------- - /// Load plugins from a YAML config file. /// /// Parses the config, looks up each plugin's `kind` in the @@ -705,10 +689,6 @@ impl PluginManager { Ok(manager) } - // ----------------------------------------------------------------------- - // Registration - // ----------------------------------------------------------------------- - /// Register a plugin handler for its primary hook name. /// /// This is the preferred registration method. The framework creates @@ -805,10 +785,6 @@ impl PluginManager { Ok(()) } - // ----------------------------------------------------------------------- - // Lifecycle - // ----------------------------------------------------------------------- - /// Initialize all registered plugins. /// /// Calls `plugin.initialize()` on each registered plugin. Must be @@ -910,10 +886,6 @@ impl PluginManager { info!("PluginManager shutdown complete"); } - // ----------------------------------------------------------------------- - // Hook Invocation — Dynamic (invoke_by_name) - // ----------------------------------------------------------------------- - /// Invoke a hook by name with a type-erased payload. /// /// This is the dynamic dispatch path used by Python/Go/WASM @@ -990,11 +962,6 @@ impl PluginManager { .await } - // ----------------------------------------------------------------------- - // Hook Invocation — Typed (invoke::) - - // ----------------------------------------------------------------------- - /// Invoke a typed hook. /// /// This is the compile-time dispatch path used by Rust callers. @@ -1209,10 +1176,6 @@ impl PluginManager { .await } - // ----------------------------------------------------------------------- - // Route Annotation - // ----------------------------------------------------------------------- - /// Override the resolved plugin list for one `(entity_type, entity_name)` /// pair on the listed hooks with a single synthetic handler. The handler /// takes responsibility for any further plugin dispatch within itself @@ -1287,10 +1250,6 @@ impl PluginManager { }); } - // ----------------------------------------------------------------------- - // Route Filtering - // ----------------------------------------------------------------------- - /// Filter hook entries based on route resolution, with caching. /// /// When routing is enabled and extensions.meta provides entity @@ -1793,10 +1752,6 @@ impl PluginManager { .len() } - // ----------------------------------------------------------------------- - // Query Methods - // ----------------------------------------------------------------------- - /// Whether anything would run for the given hook name — either a /// registered plugin handler OR a route annotation targeting that hook. /// @@ -5468,14 +5423,6 @@ routes: // So saw_security will be true, but the content is filtered } - // ----------------------------------------------------------------------- - // Awaiting handler tests - // - // `HookHandler` is async by design. These tests cover handlers - // that genuinely `.await` inside the body — sleeps, yields, and - // co-registration with handlers whose body has no `.await` at all. - // ----------------------------------------------------------------------- - /// Plugin that genuinely awaits inside its handler. Increments a /// shared counter after the await resolves so the test can verify /// the handler ran end-to-end and observed its async point. diff --git a/crates/cpex-core/src/plugin.rs b/crates/cpex-core/src/plugin.rs index dd743a24..21282dc2 100644 --- a/crates/cpex-core/src/plugin.rs +++ b/crates/cpex-core/src/plugin.rs @@ -27,10 +27,6 @@ use serde::{Deserialize, Serialize}; use crate::error::PluginError; -// --------------------------------------------------------------------------- -// Plugin Trait -// --------------------------------------------------------------------------- - /// Core plugin interface — lifecycle management only. /// /// Every plugin in the CPEX framework — regardless of language or @@ -104,10 +100,6 @@ pub trait Plugin: Send + Sync { } } -// --------------------------------------------------------------------------- -// Plugin Configuration -// --------------------------------------------------------------------------- - /// Declared plugin configuration from the unified YAML config. /// /// Controls how the framework loads, schedules, and gates the plugin. @@ -288,10 +280,6 @@ fn default_priority() -> i32 { 100 } -// --------------------------------------------------------------------------- -// Plugin Condition (legacy scoping) -// --------------------------------------------------------------------------- - /// Condition for when a plugin should execute. /// /// Narrows plugin scope to specific servers, tenants, tools, prompts, @@ -439,10 +427,6 @@ impl PluginCondition { } } -// --------------------------------------------------------------------------- -// Plugin Mode -// --------------------------------------------------------------------------- - /// Execution mode — determines a plugin's scheduling behavior and authority. /// /// The 5-phase model defines both what a plugin *can do* (block, modify) @@ -518,10 +502,6 @@ impl fmt::Display for PluginMode { } } -// --------------------------------------------------------------------------- -// Error Handling Mode -// --------------------------------------------------------------------------- - /// Error handling behavior when a plugin fails. /// /// Independent of [`PluginMode`] — any mode can use any error behavior. diff --git a/crates/cpex-core/src/registry.rs b/crates/cpex-core/src/registry.rs index f7da2c37..d3a178cd 100644 --- a/crates/cpex-core/src/registry.rs +++ b/crates/cpex-core/src/registry.rs @@ -41,10 +41,6 @@ use crate::hooks::trait_def::HookTypeDef; use crate::hooks::HookType; use crate::plugin::{Plugin, PluginConfig, PluginMode}; -// --------------------------------------------------------------------------- -// Plugin Ref — trusted wrapper -// --------------------------------------------------------------------------- - /// Manager-owned wrapper that pairs a plugin with its authoritative config. /// /// The `trusted_config` comes from the config loader / manager — never @@ -158,10 +154,6 @@ impl PluginRef { } } -// --------------------------------------------------------------------------- -// Type-Erased Hook Handler -// --------------------------------------------------------------------------- - /// Type-erased interface for calling a hook handler. /// /// The executor uses this to dispatch hooks without knowing the @@ -196,10 +188,6 @@ pub trait AnyHookHandler: Send + Sync { fn hook_type_name(&self) -> &'static str; } -// --------------------------------------------------------------------------- -// Hook Entry — PluginRef + handler paired together -// --------------------------------------------------------------------------- - /// A registered hook handler paired with its PluginRef. /// /// The executor uses `plugin_ref` for scheduling decisions (mode, @@ -218,10 +206,6 @@ pub struct HookEntry { pub handler: Arc, } -// --------------------------------------------------------------------------- -// Plugin Registry -// --------------------------------------------------------------------------- - /// Manages registered plugin instances and hook handler mappings. /// /// Stores `PluginRef` wrappers by name and `HookEntry` (PluginRef + @@ -485,10 +469,6 @@ impl Default for PluginRegistry { } } -// --------------------------------------------------------------------------- -// Group HookEntries by mode (used by the executor) -// --------------------------------------------------------------------------- - /// Groups a list of HookEntries by their execution mode. /// /// Reads mode from `plugin_ref.trusted_config` — never from the plugin. diff --git a/crates/cpex-orchestration/src/lib.rs b/crates/cpex-orchestration/src/lib.rs index c4649177..d46da935 100644 --- a/crates/cpex-orchestration/src/lib.rs +++ b/crates/cpex-orchestration/src/lib.rs @@ -32,10 +32,6 @@ use futures::future::BoxFuture; use tokio::task::{Id, JoinSet}; use tokio::time::timeout; -// ===================================================================== -// Public API -// ===================================================================== - /// Configuration knobs for [`run_branches`]. #[derive(Debug, Clone, Copy)] pub struct BranchConfig { @@ -217,31 +213,12 @@ where .collect() } -// ===================================================================== -// Implementation note on the generic signature -// ===================================================================== -// -// `P` is the closure type for `is_deny`. We declare it as a generic -// type parameter rather than `impl Fn(...)` so the function works -// uniformly across async runtimes and callers that need to use -// boxed predicates (`Box`) for runtime polymorphism. -// -// The `BoxFuture` import isn't strictly needed for the public API -// but is re-exported below for callers that want to build -// homogeneous branch vectors out of differently-typed futures (the -// common case in apl-core's `Effect::Parallel` dispatch, where each -// effect's future has a unique inferred type). - /// Convenience alias re-exported from `futures` for callers building /// type-erased branch vectors. `apl-core`'s `Effect::Parallel` /// dispatch uses this because the per-effect futures have different /// inferred types and need erasure to fit in a single `Vec`. pub type ErasedBranch = BoxFuture<'static, T>; -// ===================================================================== -// Tests -// ===================================================================== - #[cfg(test)] mod tests { use super::*; diff --git a/crates/cpex/src/lib.rs b/crates/cpex/src/lib.rs index 4e711466..8730d3fc 100644 --- a/crates/cpex/src/lib.rs +++ b/crates/cpex/src/lib.rs @@ -62,10 +62,6 @@ //! registration helpers and the concrete factory types are re-exported here //! from [`cpex-builtins`](cpex_builtins). -// ----------------------------------------------------------------------------- -// Host runtime re-exports (always available) -// ----------------------------------------------------------------------------- - // Whole-crate re-exports for advanced use (types not surfaced below). pub use {apl_cmf, apl_core, apl_cpex, cpex_core}; @@ -75,10 +71,6 @@ pub use apl_cpex::{ }; pub use cpex_core::manager::PluginManager; -// ----------------------------------------------------------------------------- -// Bundled extensions (only when a builtins feature pulls in cpex-builtins) -// ----------------------------------------------------------------------------- - // The whole aggregator, for advanced use. #[cfg(feature = "cpex-builtins")] pub use cpex_builtins; @@ -108,10 +100,6 @@ pub use cpex_builtins::{PiiScannerFactory, PII_KIND}; #[cfg(feature = "valkey")] pub use cpex_builtins::{ValkeyConfig, ValkeySessionStoreFactory, VALKEY_KIND}; -// ----------------------------------------------------------------------------- -// Tests -// ----------------------------------------------------------------------------- - #[cfg(all(test, feature = "cpex-builtins"))] mod tests { use super::*; diff --git a/go/cpex/cmf.go b/go/cpex/cmf.go index cfe82ead..2b3438c2 100644 --- a/go/cpex/cmf.go +++ b/go/cpex/cmf.go @@ -17,10 +17,6 @@ package cpex import "github.com/vmihailenco/msgpack/v5" -// --------------------------------------------------------------------------- -// CMF Message Types -// --------------------------------------------------------------------------- - // MessagePayload wraps a Message for FFI transport. // Matches Rust's cpex_core::cmf::MessagePayload. type MessagePayload struct { @@ -45,10 +41,6 @@ func NewMessage(role string, content ...ContentPart) Message { } } -// --------------------------------------------------------------------------- -// Content Parts — tagged union via content_type discriminator -// --------------------------------------------------------------------------- - // ContentPart represents one element in a Message's content list. // Uses custom msgpack marshaling to produce the tagged-union wire format: // @@ -187,10 +179,6 @@ func (cp *ContentPart) DecodeMsgpack(dec *msgpack.Decoder) error { return nil } -// --------------------------------------------------------------------------- -// Content Part Constructors -// --------------------------------------------------------------------------- - // Constructor functions are named `NewXPart` to avoid shadowing the // matching `XContent` field on ContentPart. Previously a constructor // like `ToolCallContent(tc)` had the same name as the field @@ -258,10 +246,6 @@ func NewDocumentPart(doc DocumentSource) ContentPart { return ContentPart{ContentType: ContentTypeDocument, DocumentContent: &doc} } -// --------------------------------------------------------------------------- -// Domain Objects -// --------------------------------------------------------------------------- - // ToolCall represents a tool invocation request. type ToolCall struct { ToolCallID string `msgpack:"tool_call_id"` @@ -322,10 +306,6 @@ type PromptResult struct { ErrorMessage string `msgpack:"error_message,omitempty"` } -// --------------------------------------------------------------------------- -// Media Source Types -// --------------------------------------------------------------------------- - // ImageSource holds image data (URL or base64). type ImageSource struct { SourceType string `msgpack:"type"` @@ -357,10 +337,6 @@ type DocumentSource struct { Title string `msgpack:"title,omitempty"` } -// --------------------------------------------------------------------------- -// Decode helpers — extract typed domain objects from a decoded `any` value. -// --------------------------------------------------------------------------- - // decodeAs re-encodes a decoded msgpack value and unmarshals it into a // typed struct, letting the struct's msgpack tags drive field selection. // Replaces 11 hand-rolled decoders that each had to enumerate fields From 958d601ac6501194de9ea5fe7e47aa8796f81607 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 18 Jul 2026 21:32:04 -0400 Subject: [PATCH 3/8] chore: additional code quality cleanup Signed-off-by: Frederico Araujo --- builtins/plugins/delegator-oauth/src/delegator.rs | 1 - crates/apl-cmf/src/capability_namespaces.rs | 1 - crates/apl-core/src/route.rs | 1 - crates/cpex-core/src/executor.rs | 2 -- crates/cpex-core/src/extensions/container.rs | 3 --- crates/cpex-core/src/extensions/guarded.rs | 1 - crates/cpex-core/src/manager.rs | 8 -------- crates/cpex-core/src/registry.rs | 2 -- 8 files changed, 19 deletions(-) diff --git a/builtins/plugins/delegator-oauth/src/delegator.rs b/builtins/plugins/delegator-oauth/src/delegator.rs index 3dd99e83..89173065 100644 --- a/builtins/plugins/delegator-oauth/src/delegator.rs +++ b/builtins/plugins/delegator-oauth/src/delegator.rs @@ -243,7 +243,6 @@ impl HookHandler for OAuthDelegator { let scope = Self::requested_scopes(payload); - // Build the form-encoded body. RFC 8693 §2.1. let mut form: Vec<(&str, &str)> = vec![ ("grant_type", GRANT_TYPE_TOKEN_EXCHANGE), ("subject_token", bearer), diff --git a/crates/apl-cmf/src/capability_namespaces.rs b/crates/apl-cmf/src/capability_namespaces.rs index 5c3cb4f2..8c00da9d 100644 --- a/crates/apl-cmf/src/capability_namespaces.rs +++ b/crates/apl-cmf/src/capability_namespaces.rs @@ -253,7 +253,6 @@ mod tests { #[test] fn write_capability_returns_empty() { - // Write caps don't expose bag-readable state. assert!(capability_namespaces(CAP_APPEND_LABELS).is_empty()); assert!(capability_namespaces(CAP_APPEND_DELEGATION).is_empty()); assert!(capability_namespaces(CAP_WRITE_HEADERS).is_empty()); diff --git a/crates/apl-core/src/route.rs b/crates/apl-core/src/route.rs index 292f43e3..ee686889 100644 --- a/crates/apl-core/src/route.rs +++ b/crates/apl-core/src/route.rs @@ -888,7 +888,6 @@ mod tests { let mut v = json!({ "a": { "b": 1, "c": 2 } }); assert!(remove_dotted(&mut v, "a.b")); assert_eq!(v, json!({ "a": { "c": 2 } })); - // Removing a missing leaf returns false. assert!(!remove_dotted(&mut v, "a.b")); } diff --git a/crates/cpex-core/src/executor.rs b/crates/cpex-core/src/executor.rs index b9e24e70..5699eee6 100644 --- a/crates/cpex-core/src/executor.rs +++ b/crates/cpex-core/src/executor.rs @@ -429,7 +429,6 @@ impl Executor { match result { Ok(Ok(result_box)) => { if let Some(erased) = extract_erased(result_box) { - // Check deny if !erased.continue_processing && can_block { if let Some(mut v) = erased.violation { v.plugin_name = Some(plugin_name.to_string()); @@ -443,7 +442,6 @@ impl Executor { *payload = mp; } if let Some(owned) = erased.modified_extensions { - // Validate tier constraints before accepting let valid = extensions.validate_immutable(&owned); if !valid { warn!( diff --git a/crates/cpex-core/src/extensions/container.rs b/crates/cpex-core/src/extensions/container.rs index d9bb99f3..a89c7e0f 100644 --- a/crates/cpex-core/src/extensions/container.rs +++ b/crates/cpex-core/src/extensions/container.rs @@ -177,7 +177,6 @@ impl Extensions { delegation: self.delegation.as_ref().map(|arc| (**arc).clone()), custom: self.custom.as_ref().map(|arc| (**arc).clone()), - // Write tokens — propagated from the original http_write_token: if self.http_write_token.is_some() { Some(WriteToken::new()) } else { @@ -290,7 +289,6 @@ pub struct OwnedExtensions { pub delegation: Option, pub custom: Option>, - // Write tokens — propagated from executor pub http_write_token: Option, pub labels_write_token: Option, pub delegation_write_token: Option, @@ -481,7 +479,6 @@ mod tests { use crate::extensions::delegation::DelegationHop; use crate::extensions::DelegationExtension; - // Build extensions with security, http, delegation, custom let mut security = SecurityExtension::default(); security.add_label("PII"); diff --git a/crates/cpex-core/src/extensions/guarded.rs b/crates/cpex-core/src/extensions/guarded.rs index fb369a16..9596dc94 100644 --- a/crates/cpex-core/src/extensions/guarded.rs +++ b/crates/cpex-core/src/extensions/guarded.rs @@ -134,7 +134,6 @@ mod tests { // Read — no token needed assert!(guarded.read().map.is_empty()); - // Write — token required guarded .write(&token) .map diff --git a/crates/cpex-core/src/manager.rs b/crates/cpex-core/src/manager.rs index d14f75d8..cb24dbd1 100644 --- a/crates/cpex-core/src/manager.rs +++ b/crates/cpex-core/src/manager.rs @@ -814,7 +814,6 @@ impl PluginManager { if let Err(e) = plugin.initialize().await { error!("Failed to initialize plugin '{}': {}", plugin_name, e); - // Clean up already-initialized plugins for init_name in initialized_plugins.iter().rev() { if let Some(pr) = snapshot.registry.get(init_name) { if let Err(shutdown_err) = pr.plugin().shutdown().await { @@ -1322,7 +1321,6 @@ impl PluginManager { }, }; - // Extract entity info from meta extension let meta = match &extensions.meta { Some(m) => m, None => return Arc::new(entries.to_vec()), @@ -2004,7 +2002,6 @@ mod tests { cfg: config.clone(), }); - // Clean registration — no AnyHookHandler needed mgr.register_handler::(plugin, config).unwrap(); mgr.initialize().await.unwrap(); @@ -3159,7 +3156,6 @@ mod tests { #[tokio::test] async fn test_timeout_fires_on_slow_handler() { - // Create a manager with a very short timeout let config = ManagerConfig { executor: crate::executor::ExecutorConfig { timeout_seconds: 1, @@ -3425,7 +3421,6 @@ mod tests { _extensions: &Extensions, ctx: &mut PluginContext, ) -> Result, Box> { - // Increment a counter in local_state let count = ctx .get_local("call_count") .and_then(|v| v.as_u64()) @@ -5292,7 +5287,6 @@ routes: .unwrap(); mgr.initialize().await.unwrap(); - // Build extensions with a security label let mut security = crate::extensions::SecurityExtension::default(); security.add_label("ORIGINAL"); @@ -5326,7 +5320,6 @@ routes: .unwrap(); mgr.initialize().await.unwrap(); - // Build extensions with a request extension let ext = Extensions { request: Some(std::sync::Arc::new(crate::extensions::RequestExtension { request_id: Some("original-req-id".into()), @@ -5396,7 +5389,6 @@ routes: .unwrap(); mgr.initialize().await.unwrap(); - // Build extensions WITH security data let mut security = crate::extensions::SecurityExtension::default(); security.add_label("SECRET"); security.subject = Some(crate::extensions::security::SubjectExtension { diff --git a/crates/cpex-core/src/registry.rs b/crates/cpex-core/src/registry.rs index d3a178cd..63eb2cf4 100644 --- a/crates/cpex-core/src/registry.rs +++ b/crates/cpex-core/src/registry.rs @@ -396,12 +396,10 @@ impl PluginRegistry { pub fn unregister(&mut self, name: &str) -> Option> { let plugin_ref = self.plugins.remove(name)?; - // Remove from hook index for entries in self.hook_index.values_mut() { entries.retain(|e| e.plugin_ref.name() != name); } - // Clean up empty hook entries self.hook_index.retain(|_, entries| !entries.is_empty()); Some(plugin_ref) From 2fb6a5d9c872bf248add504d76c9cb8d752be382 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 18 Jul 2026 22:29:23 -0400 Subject: [PATCH 4/8] docs: restore high-value doc comments dropped by aislop cleanup Puts back the ~25 substantive doc/rationale blocks the comment cleanup over-removed (FFI ABI contract, capability security docs, session-spoofing note, wire-contract refs, bug rationale). Keeps the code improvements and banner/blank-line removals. No code changes; workspace builds and tests pass. Signed-off-by: Frederico Araujo --- bindings/python/src/manager.rs | 16 ++++++ builtins/pdps/cedar-direct/src/cedar_attrs.rs | 7 +++ .../plugins/delegator-oauth/src/delegator.rs | 1 + crates/apl-cmf/src/constants.rs | 13 +++++ crates/apl-core/src/pipeline.rs | 1 + crates/apl-cpex/src/cmf_invoker.rs | 3 + crates/apl-cpex/src/dispatch_plan.rs | 4 ++ crates/apl-cpex/src/session_resolver.rs | 7 +++ crates/cpex-core/src/cmf/constants.rs | 11 ++++ crates/cpex-core/src/delegation/payload.rs | 9 +++ crates/cpex-core/src/elicitation/payload.rs | 2 + crates/cpex-core/src/executor.rs | 13 +++++ crates/cpex-core/src/extensions/tiers.rs | 29 ++++++++++ crates/cpex-core/src/identity/payload.rs | 3 + crates/cpex-ffi/src/lib.rs | 56 +++++++++++++++++++ crates/cpex-orchestration/src/lib.rs | 13 +++++ 16 files changed, 188 insertions(+) diff --git a/bindings/python/src/manager.rs b/bindings/python/src/manager.rs index 8e1e80c8..3766570b 100644 --- a/bindings/python/src/manager.rs +++ b/bindings/python/src/manager.rs @@ -160,6 +160,14 @@ impl PyPluginManager { let manager = Arc::clone(&self.inner); let hook_name = hook_name.to_string(); + // GIL released: async execution with wall-clock timeout (KD7). + // future_into_py catches panics via tokio's JoinHandle and converts them + // to pyo3_async_runtimes.RustPanic (a PyException subclass). To keep the + // documented interface consistent — the docstring promises RuntimeError — + // we spawn invoke_by_name as an isolated tokio task and intercept the + // JoinError::is_panic() case ourselves, converting it to RuntimeError. + // This mirrors cpex-ffi's run_safely/catch_unwind pattern at the async + // boundary. future_into_py( py, invoke_with_timeout( @@ -226,6 +234,14 @@ pub(crate) async fn invoke_with_timeout( } } +// These tests exercise the tokio::spawn + JoinError::is_panic() panic-catching +// path without requiring a live Python interpreter. They run under plain +// `cargo test -p cpex-python` (cdylibs produce a separate test binary that +// doesn't link against libpython). +// +// The test mirrors the FFI crate's `cpex_invoke_returns_rc_panic_when_plugin_panics` +// pattern: register a plugin that unconditionally panics, invoke it, verify the +// panic is caught and surfaced as a JoinError rather than aborting the process. #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/builtins/pdps/cedar-direct/src/cedar_attrs.rs b/builtins/pdps/cedar-direct/src/cedar_attrs.rs index 0733e548..354c1873 100644 --- a/builtins/pdps/cedar-direct/src/cedar_attrs.rs +++ b/builtins/pdps/cedar-direct/src/cedar_attrs.rs @@ -44,6 +44,13 @@ pub const ATTR_TEAMS: &str = "teams"; /// `apl-cmf`'s `claim.*` bag keys. pub const ATTR_CLAIMS: &str = "claims"; +// JSON wrapping keys (Cedar's entity-from-JSON shape). +// +// These aren't entity attributes per se — they're the top-level +// keys of the JSON shape Cedar expects when reading an entity from +// `Entity::from_json_value`. Kept here so the entity-builder code +// stays free of magic strings. + /// `uid` — the {type, id} envelope at the top of an entity JSON. pub const KEY_UID: &str = "uid"; diff --git a/builtins/plugins/delegator-oauth/src/delegator.rs b/builtins/plugins/delegator-oauth/src/delegator.rs index 89173065..3dd99e83 100644 --- a/builtins/plugins/delegator-oauth/src/delegator.rs +++ b/builtins/plugins/delegator-oauth/src/delegator.rs @@ -243,6 +243,7 @@ impl HookHandler for OAuthDelegator { let scope = Self::requested_scopes(payload); + // Build the form-encoded body. RFC 8693 §2.1. let mut form: Vec<(&str, &str)> = vec![ ("grant_type", GRANT_TYPE_TOKEN_EXCHANGE), ("subject_token", bearer), diff --git a/crates/apl-cmf/src/constants.rs b/crates/apl-cmf/src/constants.rs index 5e41adc7..aba127a3 100644 --- a/crates/apl-cmf/src/constants.rs +++ b/crates/apl-cmf/src/constants.rs @@ -52,6 +52,13 @@ pub const CAP_APPEND_LABELS: &str = "append_labels"; pub const CAP_APPEND_DELEGATION: &str = "append_delegation"; pub const CAP_WRITE_HEADERS: &str = "write_headers"; +// Bag-attribute prefixes (and exact-match keys) — must match what +// the apl-cmf extractor modules write. +// +// Prefixes ending in `.` match any key starting with them +// (e.g. `BAG_ROLE_PREFIX` matches `role.hr`, `role.admin`). +// Prefixes WITHOUT a trailing `.` match the exact bag key +// (e.g. `BAG_AUTHENTICATED` matches only `authenticated`). pub const BAG_SUBJECT_ID: &str = "subject.id"; pub const BAG_SUBJECT_TYPE: &str = "subject.type"; pub const BAG_SUBJECT_TEAMS: &str = "subject.teams"; @@ -61,6 +68,12 @@ pub const BAG_PERM_PREFIX: &str = "perm."; pub const BAG_TEAM_PREFIX: &str = "team."; pub const BAG_CLAIM_PREFIX: &str = "claim."; +// Payload (args / result). +// +// These are the dotted-prefix forms used when apl-cmf::payload flattens +// the request's args object and the upstream's result object into the +// bag. APL predicates / Cedar `${args.X}` substitutions / OPA `input.X` +// paths all resolve through these. pub const BAG_ARGS_PREFIX: &str = "args."; pub const BAG_RESULT_PREFIX: &str = "result."; diff --git a/crates/apl-core/src/pipeline.rs b/crates/apl-core/src/pipeline.rs index 0c544218..8a7117a0 100644 --- a/crates/apl-core/src/pipeline.rs +++ b/crates/apl-core/src/pipeline.rs @@ -90,6 +90,7 @@ pub enum Stage { values: Vec, }, + /// `mask(N)` — replace all but last N chars with `*`. Mask { keep_last: usize, }, diff --git a/crates/apl-cpex/src/cmf_invoker.rs b/crates/apl-cpex/src/cmf_invoker.rs index 4677523f..1dc91dc5 100644 --- a/crates/apl-cpex/src/cmf_invoker.rs +++ b/crates/apl-cpex/src/cmf_invoker.rs @@ -271,6 +271,9 @@ impl PluginInvoker for CmfPluginInvoker { ext.meta.as_ref().and_then(|m| m.entity_type.clone()) }; + // Pick the entry whose registered hook matches the current + // dispatch context via cpex-core's hook metadata table. + // Replaces the prior naming heuristic. let dispatch_phase = match invocation.phase() { DispatchPhase::Pre => HookPhase::Pre, DispatchPhase::Post => HookPhase::Post, diff --git a/crates/apl-cpex/src/dispatch_plan.rs b/crates/apl-cpex/src/dispatch_plan.rs index 30437c59..8023c268 100644 --- a/crates/apl-cpex/src/dispatch_plan.rs +++ b/crates/apl-cpex/src/dispatch_plan.rs @@ -199,6 +199,10 @@ impl RouteDispatchPlan { continue; } + // Store every (hook_name, HookEntry) pair the plugin + // registered. Dispatch-time entry selection (pick_entry) + // consults cpex-core's hook routing table per hook name. + // Replaces the prior naming heuristic. let mut entries_by_hook: HashMap = HashMap::new(); for (hook_name, entry) in entries { entries_by_hook.insert(hook_name, entry); diff --git a/crates/apl-cpex/src/session_resolver.rs b/crates/apl-cpex/src/session_resolver.rs index 27b097b0..338f74b3 100644 --- a/crates/apl-cpex/src/session_resolver.rs +++ b/crates/apl-cpex/src/session_resolver.rs @@ -490,6 +490,13 @@ mod tests { assert_eq!(sid, subject_scoped(Some("alice"), "from-claim").unwrap()); } + // Tier 2 (`X-CPEX-Session-Id` header) is intentionally absent. + // + // The Python `SessionResolver` included a header tier; cpex Rust + // does not. See the module-level doc comment for the threat model. + // A spoofing-regression guard lives below in + // `header_x_cpex_session_id_is_ignored`. + #[test] fn tier2_identity_derived_when_no_claim() { let sec = SecurityExtension { diff --git a/crates/cpex-core/src/cmf/constants.rs b/crates/cpex-core/src/cmf/constants.rs index b8fb68b8..185b85c1 100644 --- a/crates/cpex-core/src/cmf/constants.rs +++ b/crates/cpex-core/src/cmf/constants.rs @@ -60,6 +60,10 @@ pub const FIELD_TAGS: &str = "tags"; // OPA envelope pub const FIELD_OPA_INPUT: &str = "input"; +// Entity type identifiers — used in MetaExtension.entity_type and as the +// keys for `global.defaults` per-entity-type policy groups. These are the +// MCP entity taxonomy: tools (callable functions), LLMs (model +// invocations), prompts (template fills), resources (URI fetches). pub const ENTITY_TOOL: &str = "tool"; pub const ENTITY_LLM: &str = "llm"; pub const ENTITY_PROMPT: &str = "prompt"; @@ -74,6 +78,13 @@ 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 +// type — pre-invocation (called from APL's policy / args phase) and +// post-invocation (called from APL's post_policy / result phase). +// +// Used as keys in `hooks::metadata`'s routing table and from plugin +// declarations. pub const HOOK_CMF_TOOL_PRE_INVOKE: &str = "cmf.tool_pre_invoke"; pub const HOOK_CMF_TOOL_POST_INVOKE: &str = "cmf.tool_post_invoke"; pub const HOOK_CMF_LLM_INPUT: &str = "cmf.llm_input"; diff --git a/crates/cpex-core/src/delegation/payload.rs b/crates/cpex-core/src/delegation/payload.rs index 8b88876b..9b2df7f3 100644 --- a/crates/cpex-core/src/delegation/payload.rs +++ b/crates/cpex-core/src/delegation/payload.rs @@ -157,6 +157,10 @@ pub struct AttenuationConfig { /// clones and return the updated payload). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DelegationPayload { + /// The caller's current credential — the one a token-exchange + /// handler will swap for a downstream-scoped credential. Cleared + /// on drop via `Zeroizing`. `#[serde(skip)]` — never appears in + /// serialized output. #[serde(skip)] bearer_token: Zeroizing, @@ -189,6 +193,11 @@ pub struct DelegationPayload { #[serde(default, skip_serializing_if = "Option::is_none")] route_attenuation: Option, + /// The minted outbound credential. `None` until a handler + /// produces one. Carries the raw bytes (cleared on drop), the + /// header the proxy plugin should attach it under, the + /// audience it was minted for, the effective scopes, and the + /// expiry timestamp. #[serde(default, skip_serializing_if = "Option::is_none")] pub delegated_token: Option, diff --git a/crates/cpex-core/src/elicitation/payload.rs b/crates/cpex-core/src/elicitation/payload.rs index 1beeea0f..5c7f00d3 100644 --- a/crates/cpex-core/src/elicitation/payload.rs +++ b/crates/cpex-core/src/elicitation/payload.rs @@ -101,6 +101,7 @@ pub enum ElicitationOutcomeKind { /// updated payload). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ElicitationPayload { + /// Which touch-point this is. operation: ElicitationOp, /// Correlation id from a prior `Dispatch`. `None` on dispatch (the @@ -136,6 +137,7 @@ pub struct ElicitationPayload { /// selected by name). channel: Option, + /// Correlation id minted on `Dispatch`. The agent echoes it on retry. #[serde(default, skip_serializing_if = "Option::is_none")] pub id: Option, diff --git a/crates/cpex-core/src/executor.rs b/crates/cpex-core/src/executor.rs index 5699eee6..0f4ca420 100644 --- a/crates/cpex-core/src/executor.rs +++ b/crates/cpex-core/src/executor.rs @@ -308,6 +308,8 @@ impl Executor { ); } + // Phase 2: TRANSFORM — serial, chained, can modify, cannot block. + // can_block=false means denials are suppressed (returns None). self.run_serial_phase( &transform, &mut current_payload, @@ -347,6 +349,9 @@ impl Executor { ); } + // Phase 5: FIRE_AND_FORGET — background, read-only, ignore results. + // FAF errors don't go in PipelineResult.errors — they're delivered + // via BackgroundTasks::wait_for_background_tasks() instead. let bg_handles = self.spawn_fire_and_forget( &fire_and_forget, &*current_payload, @@ -393,6 +398,10 @@ impl Executor { let plugin_id = entry.plugin_ref.id(); let on_error = entry.plugin_ref.trusted_config().on_error; + // Take this plugin's context out of the table — pulls its stored + // local_state and seeds global_state from the canonical store. + // Replaces the previous values().last() seed, which was + // non-deterministic across HashMap iteration orders. let mut ctx = ctx_table.take_context(plugin_id); // Filter extensions per plugin based on declared capabilities. @@ -555,6 +564,10 @@ impl Executor { }, } + // Commit this plugin's context back to the table — replaces the + // canonical global_state with its (possibly modified) copy and + // stores the local_state for the next hook invocation. The + // global_state move is free; only the local_state insert allocates. ctx_table.store_context(plugin_id, ctx); } diff --git a/crates/cpex-core/src/extensions/tiers.rs b/crates/cpex-core/src/extensions/tiers.rs index 7dde854b..e3e76f50 100644 --- a/crates/cpex-core/src/extensions/tiers.rs +++ b/crates/cpex-core/src/extensions/tiers.rs @@ -39,6 +39,9 @@ pub enum MutabilityTier { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Capability { + /// Read the authenticated subject identity (`security.subject`). + /// Unlocks the slot but not its sub-fields — roles / teams / + /// claims / permissions each have their own cap below. ReadSubject, /// Read subject roles (`security.subject.roles`). ReadRoles, @@ -49,24 +52,50 @@ pub enum Capability { /// Read subject permissions (`security.subject.permissions`). ReadPermissions, + /// Read the OAuth client / gateway-access identity + /// (`security.client`). Distinct from the user identity + /// (`subject`) — a single user can connect through different + /// clients (first-party browser, third-party agent) and policies + /// sometimes want to gate on the client. ReadClient, + /// Read either workload-identity slot — both + /// `security.caller_workload` (the inbound attested peer) and + /// `security.this_workload` (our own outbound identity). One + /// capability covers both: a plugin either has access to + /// attested-workload identity or it doesn't. Distinct from + /// `read_agent` which governs session / conversation context, + /// **NOT** identity. ReadWorkload, + /// Read the agent execution context (`AgentExtension`). + /// **NOT a credential** — this carries session / conversation / + /// lineage state, not identity. Identity reads use + /// `read_subject` / `read_client` / `read_workload`. ReadAgent, + /// Read HTTP headers. ReadHeaders, /// Write (modify) HTTP headers. WriteHeaders, + /// Read security labels. ReadLabels, /// Append security labels (monotonic add-only). AppendLabels, + /// Read the delegation chain. ReadDelegation, /// Append to the delegation chain (monotonic). AppendDelegation, + /// Read raw inbound tokens + /// (`raw_credentials.inbound_tokens`) — the bearer-token + /// strings captured at the wire layer before validation. + /// Narrowly scoped: only IdentityResolve handlers, forwarding + /// plugins, and a small set of audit plugins should declare it. + /// Out-of-process plugins can't see these tokens regardless of + /// capability — token fields are `#[serde(skip)]`. ReadInboundCredentials, /// Read minted outbound delegated tokens /// (`raw_credentials.delegated_tokens`) — the credentials a diff --git a/crates/cpex-core/src/identity/payload.rs b/crates/cpex-core/src/identity/payload.rs index f1626ade..4048f1d4 100644 --- a/crates/cpex-core/src/identity/payload.rs +++ b/crates/cpex-core/src/identity/payload.rs @@ -113,6 +113,8 @@ impl Default for TokenSource { /// existing Sequential-phase machinery — no bespoke plumbing. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct IdentityPayload { + /// Raw credential bytes. Cleared on drop via `Zeroizing`. + /// `#[serde(skip)]` — never appears in serialized output. #[serde(skip)] raw_token: Zeroizing, @@ -135,6 +137,7 @@ pub struct IdentityPayload { #[serde(default, skip_serializing_if = "Option::is_none")] client_port: Option, + /// Resolved user identity. `None` until a handler populates it. #[serde(default, skip_serializing_if = "Option::is_none")] pub subject: Option, diff --git a/crates/cpex-ffi/src/lib.rs b/crates/cpex-ffi/src/lib.rs index 2a9ae7a1..1ddb0a67 100644 --- a/crates/cpex-ffi/src/lib.rs +++ b/crates/cpex-ffi/src/lib.rs @@ -28,6 +28,14 @@ use cpex_core::manager::PluginManager; // APL governance wiring — the `cpex_apl_install` extern "C" entry point. mod apl; +// FFI functions return c_int. 0 means success; negative codes classify +// failures so the Go (or other-language) caller can produce a typed +// error rather than a single opaque "invoke failed" string. The codes +// are stable wire ABI — additions go at the end with a fresh value; +// don't renumber. +// +// Mapped on the Go side in `go/cpex/manager.go::errorFromRC`. + /// Operation succeeded. pub const RC_OK: c_int = 0; /// Manager handle is null or shut down. @@ -51,6 +59,31 @@ pub const RC_TIMEOUT: c_int = -6; /// Plugin panicked; caught by `catch_unwind` at the FFI boundary. pub const RC_PANIC: c_int = -7; +// The FFI ABI version is an integer that identifies the C-surface +// contract this crate exposes. Bump it on any breaking change to the +// C surface: +// +// - added / removed / renamed extern "C" function +// - argument count, argument type, or return type change on an +// existing function +// - layout change of a struct that crosses the boundary +// - semantic change to an existing function (e.g. new RC_* value +// returned for a previously-success case, change in pointer +// ownership) +// +// Adding a new RC_* code at the end of the existing range is *not* a +// breaking change (the wire codes are stable; consumers handle unknown +// codes as generic failure). +// +// Consumers — every language binding — MUST call `cpex_ffi_abi_version` +// at init and compare against the version their binding was generated +// for. Mismatch is a hard error: the C surface they generated against +// is not the one they're linked against. Document the binding's +// expected ABI version in its source. +// +// Bumps are recorded in CHANGELOG.md under "Changed" with the from→to +// integers and a one-line description of what moved. + /// FFI ABI version. Bump on breaking C-surface changes; see module /// docs above for what counts as breaking. pub const FFI_ABI_VERSION: u32 = 2; @@ -73,6 +106,19 @@ pub extern "C" fn cpex_ffi_abi_version() -> u32 { /// usual case never hits this bound. const FFI_WALL_CLOCK_TIMEOUT: Duration = Duration::from_secs(60); +// One process-singleton runtime serves every manager rather than each +// `cpex_manager_new` building its own. With many managers (multi-tenant +// hosts that create one per request, dynamic plugin reload, etc.) the +// per-manager model exploded thread count: 100 managers × num_cpus +// workers each = hundreds of OS threads (and ~2MB stack apiece). +// +// Worker thread count precedence (highest first): +// 1. `cpex_configure_runtime(N)` — explicit FFI call, before first use. +// 2. `CPEX_FFI_WORKER_THREADS` env var — operator-friendly; read once +// on first use of `shared_runtime()`. +// 3. tokio default (`num_cpus`). +// +// Once the runtime is initialized it's fixed for the process lifetime. static SHARED_RUNTIME: OnceLock = OnceLock::new(); /// Name of the env var operators set to bound worker threads without @@ -945,6 +991,11 @@ pub unsafe extern "C" fn cpex_invoke_resolved( Extensions::default() }; + // Identity resolution (in-process): resolve identity and merge the + // result into the extensions BEFORE the hook runs, so raw_credentials + // (skip-serialized across FFI) reach the hook in Rust memory. Skipped + // when no identity payload was supplied or no identity.resolve hook is + // registered. let merged_extensions: Extensions = if identity_len > 0 && inner .manager @@ -1264,6 +1315,11 @@ pub struct GenericPayload { cpex_core::impl_plugin_payload!(GenericPayload); +// These tests call the `extern "C"` functions directly from Rust to +// exercise the FFI safety layer (catch_unwind, return-code mapping, +// payload-type validation) without needing a Go test harness. The +// reviewer flagged that `cpex-ffi` had zero `#[cfg(test)]` coverage — +// this seeds the file with regressions for the highest-value invariants. #[cfg(test)] mod tests { use super::*; diff --git a/crates/cpex-orchestration/src/lib.rs b/crates/cpex-orchestration/src/lib.rs index d46da935..108647b9 100644 --- a/crates/cpex-orchestration/src/lib.rs +++ b/crates/cpex-orchestration/src/lib.rs @@ -213,6 +213,19 @@ where .collect() } +// Implementation note on the generic signature: +// +// `P` is the closure type for `is_deny`. We declare it as a generic +// type parameter rather than `impl Fn(...)` so the function works +// uniformly across async runtimes and callers that need to use +// boxed predicates (`Box`) for runtime polymorphism. +// +// The `BoxFuture` import isn't strictly needed for the public API +// but is re-exported below for callers that want to build +// homogeneous branch vectors out of differently-typed futures (the +// common case in apl-core's `Effect::Parallel` dispatch, where each +// effect's future has a unique inferred type). + /// Convenience alias re-exported from `futures` for callers building /// type-erased branch vectors. `apl-core`'s `Effect::Parallel` /// dispatch uses this because the per-effect futures have different From 26553ce316b684501e1509397035194a8373e91f Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sat, 18 Jul 2026 22:32:52 -0400 Subject: [PATCH 5/8] docs: reattach delegate: step doc to parse_delegate_step Signed-off-by: Frederico Araujo --- crates/apl-core/src/parser.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/apl-core/src/parser.rs b/crates/apl-core/src/parser.rs index e934152b..f6d22466 100644 --- a/crates/apl-core/src/parser.rs +++ b/crates/apl-core/src/parser.rs @@ -1857,6 +1857,18 @@ fn step_to_effect(step: Step, source: &str) -> Result { } } +/// Parse a `delegate:` step body into a `Step::Delegate`. Accepted +/// YAML shape: +/// +/// ```yaml +/// - delegate: +/// plugin: workday-oauth # required — TokenDelegateHook plugin name +/// config: # optional — per-call config override +/// target: workday-api +/// permissions: [read_compensation] +/// on_error: deny # optional — deny | continue (default deny) +/// ``` +/// /// `config:` is opaque — the framework hands it to the named plugin /// via the existing per-call config-override pathway. The plugin /// owns the typed schema (target / audience / permissions / mode / From 1c873812fe8db491f12ff169c3f166d2f1990af0 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Sun, 19 Jul 2026 22:42:00 -0400 Subject: [PATCH 6/8] docs: remove internal plan/spec cross-references from comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strips R#/KD#/C# requirement-plan IDs, E#/Slice/step/sub-step impl-plan labels, P0-#/Finding review IDs, internal spec § refs, and internal plan/spec/design/runbook doc-filename pointers from comments and doc-comments. Reworded prose to read cleanly. Kept T0/T1/T2 session-tier labels and external-standard citations (RFC/OIDC/OAuth §). No behavior changes; workspace builds and 939 tests pass. Signed-off-by: Frederico Araujo --- bindings/python/build.rs | 2 +- bindings/python/src/conversions.rs | 12 +-- bindings/python/src/error.rs | 6 +- bindings/python/src/lib.rs | 4 +- bindings/python/src/manager.rs | 12 +-- bindings/python/src/result.rs | 6 +- builtins/pdps/cedar-direct/src/decision.rs | 3 +- builtins/pdps/cedar-direct/src/lib.rs | 3 +- builtins/pdps/cedar-direct/src/request.rs | 2 +- builtins/pdps/cel/src/lib.rs | 2 +- builtins/plugins/delegator-biscuit/src/lib.rs | 4 +- builtins/plugins/delegator-oauth/src/lib.rs | 5 +- .../plugins/elicitation-ciba/src/approver.rs | 6 +- .../elicitation-ciba/tests/live_keycloak.rs | 11 +- builtins/plugins/identity-jwt/src/config.rs | 8 +- builtins/plugins/identity-jwt/src/lib.rs | 5 +- builtins/plugins/identity-jwt/src/resolver.rs | 4 +- .../identity-jwt/src/trusted_issuer.rs | 8 +- .../identity-jwt/tests/jwks_url_e2e.rs | 4 +- crates/apl-cmf/src/lib.rs | 2 +- crates/apl-cmf/tests/end_to_end.rs | 4 +- crates/apl-core/src/attributes.rs | 2 +- crates/apl-core/src/evaluator.rs | 34 +++--- crates/apl-core/src/lib.rs | 4 +- crates/apl-core/src/parser.rs | 100 +++++++++--------- crates/apl-core/src/pipeline.rs | 8 +- crates/apl-core/src/plugin_decl.rs | 8 +- crates/apl-core/src/route.rs | 2 +- crates/apl-core/src/rules.rs | 29 +++-- crates/apl-core/src/step.rs | 27 ++--- crates/apl-core/tests/yaml_end_to_end.rs | 4 +- crates/apl-cpex/src/cmf_invoker.rs | 2 +- crates/apl-cpex/src/dispatch_plan.rs | 14 +-- crates/apl-cpex/src/lib.rs | 2 +- crates/apl-cpex/src/route_handler.rs | 6 +- crates/apl-cpex/src/session_resolver.rs | 2 +- crates/apl-cpex/src/visitor.rs | 2 +- crates/apl-cpex/tests/cmf_invoker_dispatch.rs | 6 +- crates/apl-cpex/tests/delegate_step_e2e.rs | 4 +- crates/apl-cpex/tests/end_to_end_route.rs | 2 +- crates/apl-cpex/tests/visitor_e2e.rs | 8 +- crates/cpex-core/src/context.rs | 2 +- crates/cpex-core/src/delegation/mod.rs | 4 +- crates/cpex-core/src/delegation/payload.rs | 12 +-- .../cpex-core/src/extensions/authorization.rs | 2 +- crates/cpex-core/src/extensions/container.rs | 4 +- crates/cpex-core/src/extensions/delegation.rs | 4 +- crates/cpex-core/src/extensions/filter.rs | 2 +- crates/cpex-core/src/extensions/guarded.rs | 2 +- crates/cpex-core/src/extensions/monotonic.rs | 2 +- .../src/extensions/raw_credentials.rs | 6 +- crates/cpex-core/src/extensions/security.rs | 3 +- crates/cpex-core/src/hooks/metadata.rs | 2 +- crates/cpex-core/src/identity/hook.rs | 2 +- crates/cpex-core/src/identity/mod.rs | 6 +- crates/cpex-core/src/identity/route_config.rs | 8 +- crates/cpex-core/tests/delegation_e2e.rs | 5 +- crates/cpex-core/tests/identity_e2e.rs | 3 +- crates/cpex-core/tests/identity_route_e2e.rs | 6 +- crates/cpex-ffi/src/lib.rs | 6 +- 60 files changed, 215 insertions(+), 245 deletions(-) diff --git a/bindings/python/build.rs b/bindings/python/build.rs index 550acfe3..dad68702 100644 --- a/bindings/python/build.rs +++ b/bindings/python/build.rs @@ -12,7 +12,7 @@ // the approach used by maturin itself. // // Under maturin `CARGO_FEATURE_EXTENSION_MODULE` is set, so we skip the flag -// to avoid a duplicate that maturin already injects (KD3). +// to avoid a duplicate that maturin already injects. fn main() { #[cfg(target_os = "macos")] diff --git a/bindings/python/src/conversions.rs b/bindings/python/src/conversions.rs index 50a1d4d0..a3ff9bdd 100644 --- a/bindings/python/src/conversions.rs +++ b/bindings/python/src/conversions.rs @@ -4,10 +4,10 @@ // Authors: Ted Habeck // // PyObject ↔ serde_json::Value traversal, payload resolution, and -// modified-payload serialization (R6, R3, KD1, KD2, KD5). +// modified-payload serialization. // // Never calls Python's `json` module from Rust — all conversion is direct -// PyObject inspection / construction (#2 / R6). +// PyObject inspection / construction (#2). use cpex_core::cmf::MessagePayload; use cpex_core::context::PluginContextTable; @@ -18,7 +18,7 @@ use pyo3::prelude::*; use pyo3::types::{PyBool, PyDict, PyFloat, PyInt, PyList, PyString}; use serde_json::{Map, Value}; -/// Wraps any serde_json::Value for hooks that are not `cmf.*` (KD1, KD2). +/// Wraps any serde_json::Value for hooks that are not `cmf.*`. /// /// Defined locally because `cpex-core` exports the macro but not the struct /// itself (the FFI crate defines its own copy too). @@ -34,7 +34,7 @@ cpex_core::impl_plugin_payload!(GenericPayload); /// Supported types: `bool`, `int`, `float`, `str`, `None`, `list`, `dict` /// (with `str` keys). Any other type raises `ValueError` naming the type. /// -/// Recursion is capped at 128 levels (R3). `depth` starts at 0. +/// Recursion is capped at 128 levels. `depth` starts at 0. pub fn pyobj_to_json_value( _py: Python<'_>, obj: &Bound<'_, PyAny>, @@ -136,7 +136,7 @@ pub fn json_value_to_pyobj<'py>(py: Python<'py>, v: &Value) -> PyResult` for a hook. /// /// `cmf.*` hooks → `MessagePayload` (serde-constructed from the value). -/// All other hook names → `GenericPayload { value }` (KD1, KD2). +/// All other hook names → `GenericPayload { value }`. /// /// A `from_value` failure on a CMF payload raises `ValueError` rather than /// silently falling through to GenericPayload — the caller sent a cmf hook @@ -158,7 +158,7 @@ pub fn resolve_payload(hook_name: &str, value: Value) -> PyResult` into the appropriate Python exception. /// -/// Mapping (per C2, KD9): +/// Mapping: /// Config / UnknownHook → `ValueError` (config/conversion failures) /// Timeout → `TimeoutError` /// Execution / other → `RuntimeError` @@ -36,7 +36,7 @@ pub fn plugin_error_to_pyerr(e: Box) -> PyErr { } => PyTimeoutError::new_err(format!( "cpex plugin '{plugin_name}' timed out after {timeout_ms}ms" )), - // Violation is dead on the invoke_by_name path (KD9); treat defensively. + // Violation is dead on the invoke_by_name path; treat defensively. PluginError::Violation { plugin_name, violation, diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index d51693a5..4b888179 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -7,7 +7,7 @@ // // Module registration and tokio runtime initialization. // The runtime is initialized once with a multi-thread builder that honours -// the `CPEX_PY_WORKER_THREADS` environment variable (KD8), mirroring the +// the `CPEX_PY_WORKER_THREADS` environment variable, mirroring the // `CPEX_FFI_WORKER_THREADS` knob in cpex-ffi. use pyo3::prelude::*; @@ -53,7 +53,7 @@ fn _lib(m: &Bound<'_, PyModule>) -> PyResult<()> { // Initialize the pyo3-async-runtimes tokio runtime with a multi-thread // builder so async methods are dispatched onto a real thread pool rather // than a single-threaded executor. This must run before any `future_into_py` - // call — doing it here at module import time is the correct hook (KD8). + // call — doing it here at module import time is the correct hook. // // This is a separate runtime from cpex-ffi's SHARED_RUNTIME; the // shared-budget philosophy is mirrored, not the runtime instance. diff --git a/bindings/python/src/manager.rs b/bindings/python/src/manager.rs index 3766570b..917b803a 100644 --- a/bindings/python/src/manager.rs +++ b/bindings/python/src/manager.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Ted Habeck // -// `PyPluginManager` — PyO3 wrapper around `cpex_core::PluginManager` (R1, R3, KD4). +// `PyPluginManager` — PyO3 wrapper around `cpex_core::PluginManager`. // // Construction is synchronous; lifecycle methods (`initialize`, `shutdown`, // `invoke_hook`) are returned as Python awaitables via `future_into_py`. @@ -14,7 +14,7 @@ // [GIL re-acq.] pipeline_result_to_py // // BackgroundTasks are dropped (not awaited per call); fire-and-forget tasks -// run on the manager's TaskTracker and are drained by `shutdown()` (KD4). +// run on the manager's TaskTracker and are drained by `shutdown()`. use std::sync::Arc; use std::time::Duration; @@ -34,7 +34,7 @@ use crate::error::plugin_error_to_pyerr; use crate::result::pipeline_result_to_py; /// Wall-clock timeout for every async call through the PyO3 boundary. -/// Mirrors `FFI_WALL_CLOCK_TIMEOUT` in cpex-ffi (KD7). +/// Mirrors `FFI_WALL_CLOCK_TIMEOUT` in cpex-ffi. const PY_WALL_CLOCK_TIMEOUT: Duration = Duration::from_secs(60); #[pyclass(name = "PluginManager")] @@ -95,7 +95,7 @@ impl PyPluginManager { }) } - /// Shut down all registered plugins and drain fire-and-forget tasks (KD4). + /// Shut down all registered plugins and drain fire-and-forget tasks. /// /// Returns an awaitable (coroutine). fn shutdown<'py>(&self, py: Python<'py>) -> PyResult> { @@ -132,7 +132,7 @@ impl PyPluginManager { /// `ValueError` — payload/extensions/context conversion failure, /// or depth > 128. /// `RuntimeError` — plugin execution error or panic at the boundary. - /// `TimeoutError` — wall-clock timeout exceeded (KD7). + /// `TimeoutError` — wall-clock timeout exceeded. #[pyo3(signature = (hook_name, payload, extensions=None, context_table=None))] fn invoke_hook<'py>( &self, @@ -160,7 +160,7 @@ impl PyPluginManager { let manager = Arc::clone(&self.inner); let hook_name = hook_name.to_string(); - // GIL released: async execution with wall-clock timeout (KD7). + // GIL released: async execution with wall-clock timeout. // future_into_py catches panics via tokio's JoinHandle and converts them // to pyo3_async_runtimes.RustPanic (a PyException subclass). To keep the // documented interface consistent — the docstring promises RuntimeError — diff --git a/bindings/python/src/result.rs b/bindings/python/src/result.rs index 1011d2b9..07c6d16a 100644 --- a/bindings/python/src/result.rs +++ b/bindings/python/src/result.rs @@ -3,13 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Ted Habeck // -// `PyPipelineResult` — read-only Python view of `cpex_core::PipelineResult` (R1, R2, R3). +// `PyPipelineResult` — read-only Python view of `cpex_core::PipelineResult`. // // Mirrors the field set of `PipelineResult` exactly. All fields are read-only // getters; no setters are exposed. // // If the modified_payload could not be serialised back to a Python dict the -// caller appends a synthetic `PluginErrorRecord` to `errors` (R2, #8); +// caller appends a synthetic `PluginErrorRecord` to `errors` (#8); // `modified_payload` is exposed as `None` in that case — mirrors the pattern // at `crates/cpex-ffi/src/lib.rs:877`. @@ -136,7 +136,7 @@ impl PyPipelineResult { /// /// If `modified_payload` is present but cannot be serialised, a synthetic /// `PluginErrorRecord` is appended to `errors` and `modified_payload` is -/// exposed as `None` — mirrors cpex-ffi's behaviour at lib.rs:877 (R2, #8). +/// exposed as `None` — mirrors cpex-ffi's behaviour at lib.rs:877 (#8). pub fn pipeline_result_to_py(mut result: PipelineResult) -> PyResult { // Serialise modified_payload; on failure emit a synthetic error record. let modified_payload_value: Option = match result.modified_payload.take() { diff --git a/builtins/pdps/cedar-direct/src/decision.rs b/builtins/pdps/cedar-direct/src/decision.rs index 46b8bf03..3735b34b 100644 --- a/builtins/pdps/cedar-direct/src/decision.rs +++ b/builtins/pdps/cedar-direct/src/decision.rs @@ -23,8 +23,7 @@ // // - Obligations — Cedar 4.10 doesn't have first-class obligations. // Policy annotations could carry them (`@obligation(...)`) but -// wiring the annotation vocabulary is deferred — see -// `docs/specs/cedar-context-contract.md`. +// wiring the annotation vocabulary is deferred. // // # Fail-closed on evaluation errors // diff --git a/builtins/pdps/cedar-direct/src/lib.rs b/builtins/pdps/cedar-direct/src/lib.rs index c38c1e8a..864820e8 100644 --- a/builtins/pdps/cedar-direct/src/lib.rs +++ b/builtins/pdps/cedar-direct/src/lib.rs @@ -71,8 +71,7 @@ // - `context.security` — `{ labels: [...], classification }`. // // Operators document this layout in their Cedar schema; policy authors -// rely on it. See `docs/specs/cedar-context-contract.md` for the -// authoritative shape. +// rely on it. // // # Schema (optional) // diff --git a/builtins/pdps/cedar-direct/src/request.rs b/builtins/pdps/cedar-direct/src/request.rs index f0c75c22..1cb89124 100644 --- a/builtins/pdps/cedar-direct/src/request.rs +++ b/builtins/pdps/cedar-direct/src/request.rs @@ -125,7 +125,7 @@ pub fn parse<'a>( /// Build the CPEX-provided context block (everything under /// `context.delegation`, `context.meta`, `context.security`) from the /// `AttributeBag`. Operators reason about these in Cedar policies via -/// the well-known paths documented in `docs/specs/cedar-context-contract.md`. +/// the well-known paths. fn build_cpex_context(bag: &AttributeBag) -> Value { let mut root = Map::new(); diff --git a/builtins/pdps/cel/src/lib.rs b/builtins/pdps/cel/src/lib.rs index 5c697e63..46113c91 100644 --- a/builtins/pdps/cel/src/lib.rs +++ b/builtins/pdps/cel/src/lib.rs @@ -68,7 +68,7 @@ // `false → Deny`. A non-boolean result, an undeclared-variable reference, // a compile error, or any other evaluation error is **fail-closed → Deny** // with the cause in `PdpDecision.diagnostics` (matches APL's PDP -// fail-closed default; DSL §8.9). Operators can flip a *runtime* error +// fail-closed default). Operators can flip a *runtime* error // (undeclared variable, type error, non-boolean) to allow-through via // `on_error: allow` in the PDP config block, but the default is `deny`. // Compile errors are never flippable — see `resolver::OnError`. diff --git a/builtins/plugins/delegator-biscuit/src/lib.rs b/builtins/plugins/delegator-biscuit/src/lib.rs index ab1529e7..c1750643 100644 --- a/builtins/plugins/delegator-biscuit/src/lib.rs +++ b/builtins/plugins/delegator-biscuit/src/lib.rs @@ -23,8 +23,8 @@ // Subsequent hops can each append further blocks. Completion blocks // (post-execution audit) are a future hook family. // -// Sub-step A scope: module structure only. Real implementation in -// sub-step B; integration tests in sub-step C. +// Scope: module structure only. Real implementation and integration +// tests land later. pub mod config; pub mod delegator; diff --git a/builtins/plugins/delegator-oauth/src/lib.rs b/builtins/plugins/delegator-oauth/src/lib.rs index 246011eb..80cb9c7b 100644 --- a/builtins/plugins/delegator-oauth/src/lib.rs +++ b/builtins/plugins/delegator-oauth/src/lib.rs @@ -16,9 +16,8 @@ // `access_token` becomes the `RawDelegatedToken` the framework // stashes under `Extensions.raw_credentials.delegated_tokens`. // -// Sub-step A scope: data shapes + module structure only. Actual -// HTTP exchange logic in sub-step B; mock-IdP integration tests in -// sub-step C. +// Scope: data shapes + module structure only. Actual HTTP exchange +// logic and mock-IdP integration tests land later. pub mod config; pub mod delegator; diff --git a/builtins/plugins/elicitation-ciba/src/approver.rs b/builtins/plugins/elicitation-ciba/src/approver.rs index 3012672d..27545309 100644 --- a/builtins/plugins/elicitation-ciba/src/approver.rs +++ b/builtins/plugins/elicitation-ciba/src/approver.rs @@ -145,7 +145,7 @@ impl CibaApprover { // anti-phishing correlation code shown on both devices, not the // full transaction text — the canonical, human-readable `purpose` // is recorded upstream (apl-core audit). So derive a valid code - // from it. See docs/keycloak-ciba-phase0-runbook.md. + // from it. let binding_message = payload.purpose().map(sanitize_binding_message); let mut form: Vec<(&str, &str)> = vec![ @@ -465,9 +465,7 @@ fn invalid(payload: &ElicitationPayload, reason: &str) -> PluginResult Option { } #[tokio::test] -#[ignore = "requires a live Keycloak configured per docs/keycloak-ciba-phase0-runbook.md"] +#[ignore = "requires a live Keycloak"] async fn live_dispatch_then_pending() { let (Some(backchannel), Some(token), Some(client_id), Some(secret), Some(login_hint)) = ( env("CIBA_BACKCHANNEL_ENDPOINT"), @@ -78,7 +77,7 @@ async fn live_dispatch_then_pending() { let approver = CibaApprover::new(cfg).expect("construct approver"); let ext = Extensions::default(); - // 1. dispatch → backchannel auth request (runbook §5.1). + // 1. dispatch → backchannel auth request. let dispatch = ElicitationPayload::new(ElicitationOp::Dispatch, "approval", &login_hint) .with_purpose("Phase 0 verification — please ignore"); let mut ctx = PluginContext::new(); @@ -97,7 +96,7 @@ async fn live_dispatch_then_pending() { assert_eq!(dispatched.approver.as_deref(), Some(login_hint.as_str())); eprintln!("dispatch OK — auth_req_id = {id}"); - // 2. check → token poll before approval (runbook §5.2). Without a + // 2. check → token poll before approval. Without a // completed decoupled approval this must report Pending. let check = ElicitationPayload::new(ElicitationOp::Check, "approval", "").with_elicitation_id(&id); diff --git a/builtins/plugins/identity-jwt/src/config.rs b/builtins/plugins/identity-jwt/src/config.rs index ba747e08..4e4b82de 100644 --- a/builtins/plugins/identity-jwt/src/config.rs +++ b/builtins/plugins/identity-jwt/src/config.rs @@ -227,13 +227,13 @@ impl DecodingKeySource { /// The fetch is bounded by `JWKS_FETCH_TIMEOUT` to prevent a /// slow or hostile JWKS endpoint from hanging gateway startup /// indefinitely. A timed-out fetch surfaces as an error string - /// the caller can soft-fail on (Slice B). + /// the caller can soft-fail on. /// - /// **v0 caveat (still open after Slice A):** + /// **v0 caveat:** /// /// * No automatic rotation — the store is bound at initialize - /// time. Slice B adds a background refresh task so IdP key - /// rolls don't require a gateway restart. + /// time. A background refresh task keeps IdP key + /// rolls from requiring a gateway restart. pub async fn build_async(&self) -> Result { match self { Self::JwksUrl { diff --git a/builtins/plugins/identity-jwt/src/lib.rs b/builtins/plugins/identity-jwt/src/lib.rs index a0a43538..8147153c 100644 --- a/builtins/plugins/identity-jwt/src/lib.rs +++ b/builtins/plugins/identity-jwt/src/lib.rs @@ -11,9 +11,8 @@ // identity path: validate a Bearer token and extract identity, // independent of any PDP step that runs later in the route. // -// Sub-step A scope: data shapes + module structure only. Actual -// validation logic in sub-step B; multi-issuer + key rotation in -// sub-step C; integration tests in sub-step D. +// Scope: data shapes + module structure only. Actual validation +// logic, multi-issuer + key rotation, and integration tests land later. // // # Error handling // diff --git a/builtins/plugins/identity-jwt/src/resolver.rs b/builtins/plugins/identity-jwt/src/resolver.rs index a9434f6a..8f4793d2 100644 --- a/builtins/plugins/identity-jwt/src/resolver.rs +++ b/builtins/plugins/identity-jwt/src/resolver.rs @@ -269,7 +269,7 @@ impl Plugin for JwtIdentityResolver { /// the KeyStore on a periodic schedule (default 10 min, /// configurable per-issuer via `refresh_secs`). /// - /// **Soft-fail semantics (Slice B):** an unreachable / slow / + /// **Soft-fail semantics:** an unreachable / slow / /// malformed JWKS at startup logs a warning and leaves the /// issuer's KeyStore *empty*. The plugin still loads, the /// gateway still boots, and the background refresh task gets @@ -605,7 +605,7 @@ fn peek_issuer(token: &str) -> Option { /// Reason `validate_token` couldn't verify the JWT. Wraps the /// usual `jsonwebtoken::errors::Error` plus the kid-selection -/// and JWKS-availability cases introduced by Slice A / B. +/// and JWKS-availability cases. enum ValidateError { /// The JWT's header `kid` didn't match any key the issuer's /// KeyStore knows about. Distinct from `InvalidSignature` so diff --git a/builtins/plugins/identity-jwt/src/trusted_issuer.rs b/builtins/plugins/identity-jwt/src/trusted_issuer.rs index 935c8e83..2f5bdc95 100644 --- a/builtins/plugins/identity-jwt/src/trusted_issuer.rs +++ b/builtins/plugins/identity-jwt/src/trusted_issuer.rs @@ -30,11 +30,11 @@ use jsonwebtoken::{Algorithm, DecodingKey}; /// /// A KeyStore with no entries at all (`by_kid.is_empty() && fallback.is_none()`) /// is a valid runtime state — it represents "JWKS fetch failed, -/// retry pending" in the soft-fail design (Slice B). Today every +/// retry pending" in the soft-fail design. Today every /// construction path populates at least one slot before the store /// is reachable from the resolver. /// -/// # Update discipline (Slice B refresh) +/// # Update discipline (refresh) /// /// When the periodic refresh task lands, the intended pattern is /// **whole-store replacement** — the refresh fetches a fresh JWKS, @@ -55,8 +55,8 @@ pub struct KeyStore { } impl KeyStore { - /// Empty store. Only useful for the soft-fail placeholder path - /// (Slice B); current code always populates before exposing. + /// Empty store. Only useful for the soft-fail placeholder path; + /// current code always populates before exposing. pub fn empty() -> Self { Self { by_kid: HashMap::new(), diff --git a/builtins/plugins/identity-jwt/tests/jwks_url_e2e.rs b/builtins/plugins/identity-jwt/tests/jwks_url_e2e.rs index c03162b1..bbd52a7d 100644 --- a/builtins/plugins/identity-jwt/tests/jwks_url_e2e.rs +++ b/builtins/plugins/identity-jwt/tests/jwks_url_e2e.rs @@ -258,7 +258,7 @@ async fn skipping_initialize_rejects_with_untrusted_issuer() { } // ===================================================================== -// P0-5 Slice A: kid-based key selection + JWKS fetch timeout +// kid-based key selection + JWKS fetch timeout // ===================================================================== /// Build a JWKS containing two RSA keys with distinct `kid`s. Used by @@ -506,7 +506,7 @@ async fn jwks_fetch_times_out_when_endpoint_stalls() { } // ===================================================================== -// P0-5 Slice B: soft-fail at boot + periodic JWKS refresh +// soft-fail at boot + periodic JWKS refresh // ===================================================================== /// JWKS endpoint is unreachable at gateway boot. The plugin must diff --git a/crates/apl-cmf/src/lib.rs b/crates/apl-cmf/src/lib.rs index cb71f3dc..a237ee8e 100644 --- a/crates/apl-cmf/src/lib.rs +++ b/crates/apl-cmf/src/lib.rs @@ -7,7 +7,7 @@ // AttributeBag. This is where the *attribute vocabulary* APL policy // authors write against gets defined. // -// Layering (see docs/specs/apl-design.md §4): +// Layering: // // cpex-core : typed extension data (SecurityExtension, …) // apl-cmf : ←── this crate, flat-key bridge diff --git a/crates/apl-cmf/tests/end_to_end.rs b/crates/apl-cmf/tests/end_to_end.rs index ca2c4cdf..faf07ab7 100644 --- a/crates/apl-cmf/tests/end_to_end.rs +++ b/crates/apl-cmf/tests/end_to_end.rs @@ -25,7 +25,7 @@ use serde_json::json; // `evaluate_route` takes `&Arc` / `&Arc` // so the call paths inside apl-core can `Arc::clone` an owned, 'static reference -// into each spawned branch (E3.2). All tests pass the same no-op stubs; wrap once. +// into each spawned branch. All tests pass the same no-op stubs; wrap once. fn pdp() -> Arc { Arc::new(AllowPdp) } @@ -39,7 +39,7 @@ fn elicitations() -> Arc { Arc::new(NoopElicitationInvoker) } -// HR route from unified-config-proposal.md §Example 1. +// HR route. const HR_ROUTE_YAML: &str = r#" routes: get_employee: diff --git a/crates/apl-core/src/attributes.rs b/crates/apl-core/src/attributes.rs index c7fcca40..9fd3573b 100644 --- a/crates/apl-core/src/attributes.rs +++ b/crates/apl-core/src/attributes.rs @@ -16,7 +16,7 @@ // them and the evaluator picks them up unchanged. // // Mapping from cpex-core extensions into the bag lives in `apl-cmf`, not -// here. See docs/specs/apl-design.md §4 for the module layering. +// here. use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; diff --git a/crates/apl-core/src/evaluator.rs b/crates/apl-core/src/evaluator.rs index baa0c5c7..4cff5cb6 100644 --- a/crates/apl-core/src/evaluator.rs +++ b/crates/apl-core/src/evaluator.rs @@ -6,13 +6,13 @@ // APL evaluator — walks the IR against an AttributeBag and returns a Decision. // // The evaluator is sync and infallible by design. Missing attributes resolve -// to `false` (DSL spec §2.6); operator type mismatches resolve to `false`. +// to `false`; operator type mismatches resolve to `false`. // The host drives the four phases separately by calling `evaluate_rules` once // per declared phase — phase orchestration lives in `apl-cpex`. // -// Semantics anchored in: -// - DSL spec apl-dsl-spec.md §2 (operators), §3 (actions), §8.1 (require) -// - apl-design.md §7 (native fast-path, sync inside async outer) +// Semantics: +// - operators, actions, require +// - native fast-path, sync inside async outer use std::sync::Arc; @@ -36,7 +36,7 @@ pub enum Decision { /// Evaluate a phase's rules against the bag. /// -/// Spec §3 semantics: +/// Spec semantics: /// - First `deny` halts; subsequent rules / effects don't run. /// - `allow` effects *do not* short-circuit — evaluation continues to /// the next effect (then to the next rule). @@ -113,7 +113,7 @@ fn eval_condition(cond: &Condition, bag: &AttributeBag) -> bool { fn eval_comparison(key: &str, op: CompareOp, lit: &Literal, bag: &AttributeBag) -> bool { let attr = match bag.get(key) { Some(v) => v, - None => return false, // missing → false (spec §2.6) + None => return false, // missing → false }; match op { @@ -148,7 +148,7 @@ fn numeric_compare(attr: &AttributeValue, lit: &Literal, op: CompareOp) -> bool // (e.g. `"amount": "25000"`), and a policy author writing // `args.amount > 10000` plainly means a numeric comparison. A string // that doesn't parse as a number is genuinely non-numeric → false - // (order operators don't apply, spec §2.3). + // (order operators don't apply). let a = match coerce_f64_attr(attr) { Some(a) => a, None => return false, @@ -210,7 +210,7 @@ fn looks_like_attribute_ref(s: &str) -> bool { /// and plugin invocations via `plugins`. Returns the phase's overall /// decision. /// -/// Semantics (DSL §3, §7.5): +/// Semantics: /// - `Effect::When` — evaluate the condition; if true, run the body in /// order with the same first-deny-wins logic. /// - `Effect::Pdp` — call resolver; on Allow run `on_allow` reactions and @@ -230,7 +230,7 @@ fn looks_like_attribute_ref(s: &str) -> bool { /// - `Effect::Deny` — halt with the supplied reason/code. /// /// PDP / plugin errors map to a Deny with the error in the reason, per -/// the design's fail-closed default (DSL §8.9). Pre-E4 `evaluate_steps` +/// the design's fail-closed default. `evaluate_steps` /// is preserved as a deprecated alias that forwards here. #[allow(clippy::too_many_arguments)] pub async fn evaluate_effects( @@ -677,7 +677,7 @@ async fn dispatch_effect( /// Drive one [`Effect::Elicit`] step: dispatch on first arrival, check /// status on every pass, and on an approved-and-genuine response apply /// the runtime's `scope`-over-args sufficiency check before allowing the -/// phase to continue. See `docs/apl-manager-approval-ciba-design.md`. +/// phase to continue. /// /// Failure handling follows the step's `on_error` (default `deny`, /// fail-closed). An explicit human *denial* always halts regardless of @@ -865,7 +865,7 @@ async fn dispatch_elicitation( /// [`Effect::validate_parallel_purity`], so at runtime we trust the /// IR not to contain mutation effects. /// -/// # Concurrency model (E3.2) +/// # Concurrency model /// /// Built on [`cpex_orchestration::run_branches`] — the same JoinSet /// + abort-on-deny primitive `cpex-core`'s executor uses for its @@ -1405,7 +1405,7 @@ pub async fn evaluate_pipeline( } }, Stage::Scan { kind } => { - // Spec mapping (apl-dsl-spec §4): scan stages are taint + // Spec mapping: scan stages are taint // emitters. The actual PII detection / injection signal // lives in plugin(...) variants of the same scanners; this // stage just records the label so downstream policies can @@ -1507,7 +1507,7 @@ mod tests { // Wrap stateless test invokers in `Arc` once per call. The // public evaluator API takes `&Arc` so internal // dispatch (notably `Effect::Parallel`) can `Arc::clone` an owned, - // 'static reference into each spawned branch (slice E3.2). + // 'static reference into each spawned branch. fn null_pipe_plugins() -> Arc { Arc::new(NullPipelinePlugins) } @@ -1592,7 +1592,7 @@ mod tests { #[test] fn allow_does_not_short_circuit() { - // Spec §3: explicit allow continues evaluation. A later deny still fires. + // Explicit allow continues evaluation. A later deny still fires. let mut bag = AttributeBag::new(); bag.set("ok", true); bag.set("bad", true); @@ -1644,7 +1644,7 @@ mod tests { }, &bag )); - // Comparison on missing → false (spec §2.6). + // Comparison on missing → false. assert!(!eval_condition( &Condition::Comparison { key: "missing".into(), @@ -1771,7 +1771,7 @@ mod tests { }, &bag, )); - // Order operators on strings → false (spec §2.3). + // Order operators on strings → false. assert!(!eval_condition( &Condition::Comparison { key: "subject.id".into(), @@ -2600,7 +2600,7 @@ mod tests { #[test] fn in_set_missing_keys_resolve_to_false() { let mut bag = AttributeBag::new(); - // Both missing → in = false → not in = true (spec §2.6 missing→false + // Both missing → in = false → not in = true (missing→false // applies to the underlying `in` lookup; negate flips it). assert!(!eval_condition( &Condition::InSet { diff --git a/crates/apl-core/src/lib.rs b/crates/apl-core/src/lib.rs index d084d38c..7744e543 100644 --- a/crates/apl-core/src/lib.rs +++ b/crates/apl-core/src/lib.rs @@ -8,10 +8,8 @@ // This crate is the language nucleus. It does not depend on CPEX directly; // the bridge from cpex-core extensions into the AttributeBag lives in // `apl-cmf`, and the `PolicyEvaluator` implementation lives in `apl-cpex`. -// -// See docs/specs/apl-design.md for the full design. -#![doc = "APL — Authorization Policy Language. See docs/specs/apl-design.md."] +#![doc = "APL — Authorization Policy Language."] pub mod attributes; pub mod evaluator; diff --git a/crates/apl-core/src/parser.rs b/crates/apl-core/src/parser.rs index f6d22466..c7566437 100644 --- a/crates/apl-core/src/parser.rs +++ b/crates/apl-core/src/parser.rs @@ -8,10 +8,10 @@ // Runs once at config load. The IR it produces is what the evaluator walks // at request time; the parser is never on the hot path. // -// Grammar anchored in apl-dsl-spec.md §2 (predicates) / §3 (rules) / §8 (EBNF). -// YAML shape anchored in apl-design.md §5 (`routes:` as map keyed by route_key). +// Grammar: predicates, rules, EBNF. YAML shape: `routes:` as a map +// keyed by route_key. // -// Step 5a scope: +// Scope: // ✓ Predicate grammar: identifiers, literals, comparisons, contains, // & | ! parens, require(...) // ✓ Actions: deny / allow / (default deny on missing) @@ -40,7 +40,7 @@ pub enum ParseError { #[error("rule '{rule}': {msg}")] Rule { rule: String, msg: String }, - #[error("unsupported step `{kind}` in rule '{rule}' — defer to step 5b")] + #[error("unsupported step `{kind}` in rule '{rule}'")] UnsupportedStep { rule: String, kind: String }, #[error("predicate '{predicate}': {msg}")] @@ -369,7 +369,7 @@ impl<'a> PredParser<'a> { _ => Err(self.err("expected `)`")), } }, - // `require(...)` is a rule-level shorthand per DSL §8 grammar + // `require(...)` is a rule-level shorthand per the DSL grammar // (`rule = require_call | predicate ...`), not a sub-predicate. // Trying to nest it inside `&` / `|` is a grammar error. Some(Tok::Require) => Err(self.err( @@ -382,7 +382,7 @@ impl<'a> PredParser<'a> { } } - /// `exists()` — DSL §2.2. Returns true if the key is present + /// `exists()`. Returns true if the key is present /// in the AttributeBag, regardless of value (distinct from truthiness). fn parse_exists(&mut self) -> Result { self.bump(); // exists @@ -423,7 +423,7 @@ impl<'a> PredParser<'a> { _ => unreachable!("parse_atom dispatched here"), }; - // `in` and `not in` — two-key set membership (DSL §2.4). + // `in` and `not in` — two-key set membership. if matches!(self.peek(), Some(Tok::In)) { self.bump(); return self.finish_in_set(key, false); @@ -502,17 +502,16 @@ impl<'a> PredParser<'a> { } } -/// Parse a predicate string into the IR. Public for tests + step-5b use. +/// Parse a predicate string into the IR. Public for tests. pub fn parse_predicate(src: &str) -> Result { PredParser::parse(src.trim()) } /// Parse a single rule line into a `Rule`. /// -/// Accepted forms (DSL §3.2): +/// Accepted forms: /// 1. `"require(...)"` → rule-level shorthand, desugars to /// `when: do: deny` -/// per DSL §8.1 /// 2. `": "` → Rule { condition, action } /// 3. `""` → Rule { condition, action: Deny } (default) /// 4. `""` (action only) → treated as form 3 (always-true predicate) @@ -525,7 +524,7 @@ pub fn parse_rule(line: &str, source: &str) -> Result { let trimmed = line.trim(); // require(...) shorthand — special-cased because it desugars to a - // negated predicate + Deny action, and the spec grammar (§8) puts it + // negated predicate + Deny action, and the spec grammar puts it // as a top-level rule alternative, not a sub-predicate. if is_require_call(trimmed) { let condition = parse_require_rule(trimmed)?; @@ -572,7 +571,7 @@ pub fn parse_rule(line: &str, source: &str) -> Result { source: source.to_string(), }); } - // DSL §2 default: bare predicate denies. + // Default: bare predicate denies. ( trimmed, vec![Effect::Deny { @@ -600,7 +599,7 @@ fn is_require_call(s: &str) -> bool { } /// Parse `require(a)` / `require(a, b, ...)` / `require(a | b | ...)` and -/// return the desugared "when" expression per DSL §8.1: +/// return the desugared "when" expression: /// /// require(X) → IsFalse(X) /// require(X, Y, ...) → Or([IsFalse(X), IsFalse(Y), ...]) (deny if any falsy) @@ -729,8 +728,8 @@ fn split_predicate_action(s: &str) -> Option<(&str, &str)> { } /// Parse the *right* side of a shorthand `predicate: action` rule into a -/// single-element effects vec. Recognized forms (DSL §3 + the `code` -/// extension we added in E1): +/// single-element effects vec. Recognized forms (the `code` +/// extension we added): /// /// * `deny` → `vec![Effect::Deny { reason: None, code: None }]` /// * `deny('reason')` → `vec![Effect::Deny { reason: Some, code: None }]` @@ -779,7 +778,7 @@ fn try_parse_deny_call(s: &str, rule: &str) -> Result, ParseError msg: "malformed `deny(...)`".into(), })?; // Two positional args max. Spec precedent: `deny('reason')` (1 arg); - // E1 extension: `deny('reason', 'code')` (2 args). Both quoted. + // Extension: `deny('reason', 'code')` (2 args). Both quoted. let parts = split_top_level_commas(&inside).map_err(|e| ParseError::Rule { rule: rule.to_string(), msg: format!("deny(...): {}", e), @@ -821,7 +820,7 @@ fn strip_string_literal(s: &str, rule: &str) -> Result { /// Parse a single YAML entry from a `pre_invocation` / `post_invocation` list. /// -/// Two YAML shapes (DSL §3.2 + §7): +/// Two YAML shapes: /// - **String entry** — a rule line, taint effect, or plugin call. /// - `"require(authenticated)"` → `Step::Rule` /// - `"delegation.depth > 2: deny"` → `Step::Rule` @@ -1061,8 +1060,7 @@ fn parse_delegate_call_args(inside: &str, source: &str) -> Result &str { } fn parse_step_map(m: &serde_yaml::Mapping, source: &str) -> Result { - // Canonical structured rule: `- when: X\n do: Y` (DSL §3.2). + // Canonical structured rule: `- when: X\n do: Y`. // Detected by the presence of *both* `when` and `do` keys — order // doesn't matter, and the map can carry extra keys for future // extensions (e.g. `id:` for rule identifiers). @@ -1360,8 +1358,8 @@ fn parse_step_map(m: &serde_yaml::Mapping, source: &str) -> Result Result stays uniform. match key.trim() { @@ -1461,7 +1459,7 @@ fn is_known_pdp_dialect(key: &str) -> bool { matches!(base.trim(), "cedar" | "opa" | "authzen" | "nemo" | "cel") } -/// Parse the canonical `- when: X` `do: Y` rule form (DSL §3.2). `Y` +/// Parse the canonical `- when: X` `do: Y` rule form. `Y` /// may be a single effect string (`do: deny`) or a list of effect /// entries (`do: [plugin(audit), taint(X), deny('msg')]`). Map-form /// effects (like a nested `delegate:` block) are allowed inside `do:` @@ -1514,8 +1512,8 @@ fn parse_when_do_rule(m: &serde_yaml::Mapping, source: &str) -> Result Result parse_effect_string(s, source), serde_yaml::Value::Mapping(m) => { - // E3: `sequential:` / `parallel:` map forms — a single-key + // `sequential:` / `parallel:` map forms — a single-key // map whose key is `sequential` / `parallel` and whose // value is a list of effects. if m.len() == 1 { @@ -1776,9 +1774,9 @@ fn is_valid_field_path(s: &str) -> bool { /// `Effect`. The legitimate inputs are `Plugin`, `Delegate`, `Taint`, /// and `Rule` (when a control action like `deny`/`allow` was parsed). /// Anything else (`Pdp`) is rejected — nested PDP calls inside `do:` -/// are out of scope for E1. +/// are out of scope. /// Recursively map a top-level `Step` (as produced by `parse_step`) into -/// an `Effect`. Used at compile_apl_blocks during E4 — keeps `parse_step`'s +/// an `Effect`. Used at compile_apl_blocks — keeps `parse_step`'s /// internal shape for the moment while the public IR collapses to Effect. /// All five Step variants map cleanly: Rule → When, Pdp → Pdp (recursive /// on reactions), Plugin/Delegate/Taint pass-through. @@ -1822,12 +1820,12 @@ fn step_to_effect(step: Step, source: &str) -> Result { Step::Elicit(e) => Ok(Effect::Elicit(e)), Step::Taint { label, scopes } => Ok(Effect::Taint { label, scopes }), Step::Rule(rule) => { - // Nested when/do inside a do: list isn't supported in E1 + // Nested when/do inside a do: list isn't supported // — only control effects (allow/deny) flatten cleanly. if !matches!(rule.condition, Expression::Always) { return Err(ParseError::Rule { rule: source.to_string(), - msg: "conditional rules nested inside `do:` are not supported in E1 \ + msg: "conditional rules nested inside `do:` are not supported \ (use a sibling `when:`/`do:` rule instead)" .into(), }); @@ -1850,7 +1848,7 @@ fn step_to_effect(step: Step, source: &str) -> Result { }, Step::Pdp { .. } => Err(ParseError::Rule { rule: source.to_string(), - msg: "PDP calls inside `do:` are not supported in E1 (use a sibling \ + msg: "PDP calls inside `do:` are not supported (use a sibling \ step instead)" .into(), }), @@ -2084,7 +2082,7 @@ fn parse_stage(src: &str) -> Result { ("redact", None) => Ok(Stage::Redact { condition: None }), ("omit", None) => Ok(Stage::Omit), ("hash", None) => Ok(Stage::Hash), - // Scan placeholders parse as bare identifiers (DSL §4.5). + // Scan placeholders parse as bare identifiers. ("pii.redact", None) => Ok(Stage::Scan { kind: ScanKind::PiiRedact, }), @@ -2182,7 +2180,7 @@ fn parse_stage_regex(a: &str) -> Stage { Stage::Regex { pattern: pat } } -// Named-validator dispatch (`validate(name)`) is in the spec (DSL §4.2) +// Named-validator dispatch (`validate(name)`) is in the spec // but not implemented in this build — the evaluator's no-op stub would // silently let invalid values through. Reject at compile time so // operators notice immediately and reach for one of the working @@ -2335,7 +2333,7 @@ fn parse_taint(args: &str, src: &str) -> Result { } let scopes = if parts.len() == 1 { - vec![TaintScope::Session] // default per DSL §4.6 + vec![TaintScope::Session] // default } else { let scope_arg = parts[1..].join(","); let scope_arg = scope_arg.trim(); @@ -2366,7 +2364,7 @@ fn parse_taint_scope(s: &str, src: &str) -> Result { } } -/// Top-level config — only the bits step 5a understands. +/// Top-level config — only the bits the parser understands. /// /// `policy_evaluator:`, `imports:`, `global:`, `defaults:`, `tags:`, /// `plugin_dirs:`, `plugin_settings:`, `version:` are all accepted and @@ -2468,7 +2466,7 @@ pub struct CompiledConfig { /// /// Routes with no APL fields populated (no `authorization:` / /// `pre_invocation:` / `post_invocation:` / `args:` / `result:`) are -/// **omitted from `routes`**, per apl-design §5 +/// **omitted from `routes`**, per apl-design /// "Routes without APL blocks fall back to legacy plugin-chain execution." /// A route-level `plugins:` override block alone is not enough — overrides /// only have meaning when the route actually dispatches plugins via APL @@ -2753,7 +2751,7 @@ mod tests { #[test] fn pred_parens_override_precedence() { - // `(role.finance | role.admin) & !delegated` from DSL §2.5. + // `(role.finance | role.admin) & !delegated`. let e = parse_predicate("(role.finance | role.admin) & !delegated").unwrap(); match e { Expression::And(parts) => { @@ -2767,7 +2765,7 @@ mod tests { #[test] fn pred_require_rejected_as_predicate() { - // require() is a rule-level shorthand per DSL §8, not a sub-predicate. + // require() is a rule-level shorthand, not a sub-predicate. // Trying to use it inside a predicate expression must fail clearly. let err = parse_predicate("require(authenticated)").unwrap_err(); assert!(format!("{}", err).contains("rule-level shorthand")); @@ -2775,7 +2773,7 @@ mod tests { #[test] fn rule_require_single_arg_desugars_to_isfalse_and_deny() { - // require(X) → Rule { condition: IsFalse(X), action: Deny } (DSL §8.1) + // require(X) → Rule { condition: IsFalse(X), action: Deny } let r = parse_rule("require(authenticated)", "test").unwrap(); assert!(matches!( r.effects.as_slice(), @@ -2794,7 +2792,7 @@ mod tests { #[test] fn rule_require_comma_is_and_desugars_to_or_of_isfalse() { - // require(X, Y) → Or([IsFalse(X), IsFalse(Y)]) + Deny (DSL §8.1) + // require(X, Y) → Or([IsFalse(X), IsFalse(Y)]) + Deny // i.e., "deny if any are falsy" = "any are falsy → deny" let r = parse_rule("require(role.hr, perm.view_ssn)", "test").unwrap(); assert_eq!( @@ -2812,7 +2810,7 @@ mod tests { #[test] fn rule_require_pipe_is_or_desugars_to_and_of_isfalse() { - // require(X | Y) → And([IsFalse(X), IsFalse(Y)]) + Deny (DSL §8.1) + // require(X | Y) → And([IsFalse(X), IsFalse(Y)]) + Deny // i.e., "deny only if all are falsy" = "all are falsy → deny" let r = parse_rule("require(role.finance | role.admin)", "test").unwrap(); assert_eq!( @@ -2927,7 +2925,7 @@ mod tests { #[test] fn rule_predicate_only_defaults_to_deny() { - // DSL §2: missing action defaults to deny. + // Missing action defaults to deny. let r = parse_rule("!authenticated", "test").unwrap(); assert!(matches!(r.effects.as_slice(), [Effect::Deny { .. }])); } @@ -2941,7 +2939,7 @@ mod tests { #[test] fn rule_bare_action_unconditional() { // Bare `- deny` and `- allow` are unconditional rules with - // Expression::Always as the predicate (DSL §3.1). + // Expression::Always as the predicate. let r = parse_rule("deny", "test").unwrap(); assert_eq!(r.condition, Expression::Always); assert!(matches!( @@ -3023,8 +3021,8 @@ mod tests { fn rule_deny_with_unquoted_arg_rejected() { // `deny "reason"` (space-separated, no parens) is not a valid // form. The supported reason-carrying shape is - // `deny('reason')` / `deny('reason', 'code')` per DSL §3 and - // the E1 `code` extension. + // `deny('reason')` / `deny('reason', 'code')` and + // the `code` extension. let err = parse_rule(r#"authenticated: deny "go away""#, "test").unwrap_err(); assert!(format!("{}", err).contains("unsupported action")); } @@ -3042,7 +3040,7 @@ mod tests { #[test] fn rule_deny_with_reason_and_code_accepted() { - // `deny('reason', 'code')` — E1 extension. Both reason and + // `deny('reason', 'code')` — extension. Both reason and // author-supplied code surface in the violation. let r = parse_rule( r#"delegation.depth > 2: deny('too deep', 'delegation.depth_exceeded')"#, @@ -3104,7 +3102,7 @@ mod tests { #[test] fn when_do_single_effect_deny_with_reason_and_code() { - // The E1 `deny('reason', 'code')` extension works inside `do:` too. + // The `deny('reason', 'code')` extension works inside `do:` too. let step = parse_step_yaml( "when: delegation.depth > 2\ndo: deny('too deep', 'delegation.depth_exceeded')", ) @@ -3247,7 +3245,7 @@ do: #[test] fn when_do_with_field_op_result_redact() { - // The headline E2 case: `result.salary | redact` as an effect + // The headline case: `result.salary | redact` as an effect // inside a do: list, alongside other effect kinds. let yaml = r#" when: "!perm.view_ssn" @@ -3728,7 +3726,7 @@ routes: fn compile_omits_routes_without_apl_blocks() { // A route with no APL blocks (no authorization / pre_invocation / // post_invocation / args / result) is a "legacy" route per - // apl-design §5 and must be + // apl-design and must be // omitted from the compiled output. Unknown route keys (e.g. // legacy CPEX `priority`) are stashed in `other`, not errored. let yaml = r#" @@ -4266,7 +4264,7 @@ routes: #[test] fn pipeline_omit_with_args_rejected() { - // omit has no conditional form per DSL §4.1. + // omit has no conditional form. let err = parse_pipeline("omit(!perm.x)").unwrap_err(); assert!(format!("{}", err).contains("omit takes no arguments")); } diff --git a/crates/apl-core/src/pipeline.rs b/crates/apl-core/src/pipeline.rs index 8a7117a0..2a84b5e5 100644 --- a/crates/apl-core/src/pipeline.rs +++ b/crates/apl-core/src/pipeline.rs @@ -10,9 +10,7 @@ // the request; transforms (mask/redact/omit/hash) modify the value; effects // (taint) record side information. // -// Grounded in apl-dsl-spec.md §4. -// -// Stages whose evaluator behavior is deferred to step 5c (taint dispatch, +// Stages whose evaluator behavior is deferred (taint dispatch, // plugin invocation, regex/named validators, scan placeholders) are still // represented in the IR so the parser can produce them — the evaluator // recognizes them and returns a clear "deferred" signal rather than crashing. @@ -80,7 +78,7 @@ pub enum Stage { max: Option, }, /// Bare range literal `N..M`, `..M`, `N..`, with optional `k`/`K`/`m`/`M` - /// numeric suffixes. Integer-only per DSL §4.3. + /// numeric suffixes. Integer-only. Range { min: Option, max: Option, @@ -101,7 +99,7 @@ pub enum Stage { condition: Option, }, /// `omit` — drop the field from output entirely. No conditional form - /// per DSL §4.1 — use a policy rule for conditional omit. + /// — use a policy rule for conditional omit. Omit, /// `hash` — replace value with a hash digest. Hash, diff --git a/crates/apl-core/src/plugin_decl.rs b/crates/apl-core/src/plugin_decl.rs index 1b64fc88..d5a536f2 100644 --- a/crates/apl-core/src/plugin_decl.rs +++ b/crates/apl-core/src/plugin_decl.rs @@ -7,11 +7,7 @@ // unified-config YAML document, plus the per-route override block and // the 2-layer resolver that merges them. // -// Spec: `contextforge-plugins-framework-apl/docs/specs/unified-config-proposal.md`, -// §"Plugin Declaration" (lines 173+) -// §"Route-Level Plugin Config Overrides" (lines 360+) -// -// Layering, per spec: +// Layering: // - Global declaration (root `plugins:`) — full shape // - Route-level override (`routes..plugins.

:`) — `config`, // `capabilities`, `on_error` only; hooks/kind/source NOT overridable @@ -50,7 +46,7 @@ pub struct PluginDeclaration { /// future versions will choose by invocation context (policy vs /// post_policy vs pipe-chain). /// - /// Per spec §"Hook dispatch": NOT overridable per-route. + /// NOT overridable per-route. #[serde(default)] pub hooks: Vec, diff --git a/crates/apl-core/src/route.rs b/crates/apl-core/src/route.rs index ee686889..1b817f70 100644 --- a/crates/apl-core/src/route.rs +++ b/crates/apl-core/src/route.rs @@ -12,7 +12,7 @@ // the right order with the right transitions (apply field mutations, halt // on deny, thread taints across phases). // -// Phase semantics (anchored in apl-dsl-spec.md §3): +// Phase semantics: // - args: walk field rules; Replace/Omit mutate `payload.args`; Deny halts // - policy: walk steps; Deny halts // - result: only runs if `payload.result.is_some()`; same as args diff --git a/crates/apl-core/src/rules.rs b/crates/apl-core/src/rules.rs index 48b9ba9c..58e300ad 100644 --- a/crates/apl-core/src/rules.rs +++ b/crates/apl-core/src/rules.rs @@ -10,13 +10,13 @@ // consumes the IR plus an AttributeBag and returns a decision. // // IR types are kept small and pure-data — no dependencies on cpex-core -// extensions, no evaluation logic. See docs/specs/apl-design.md §7. +// extensions, no evaluation logic. use serde::{Deserialize, Serialize}; /// Comparison operators in DSL predicates. /// -/// `In` / `NotIn` are intentionally absent: the DSL spec §2.4 has them as +/// `In` / `NotIn` are intentionally absent: the DSL spec has them as /// `value_key in set_key` — both sides are attribute references, not a /// key-vs-literal shape. They'll land as a dedicated `Condition` variant /// when the parser arrives. @@ -79,7 +79,7 @@ impl From for Literal { /// The DSL's `require(...)` keyword is **not** represented here — it's a /// rule-level shorthand for "deny when the condition fails," and the parser /// desugars it into `Not` / `And` / `Or` over `IsFalse` expressions plus -/// an `Action::Deny`. See DSL spec §8.1 desugarings. +/// an `Action::Deny`. See the DSL spec desugarings. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Condition { Comparison { @@ -95,14 +95,14 @@ pub enum Condition { }, /// DSL `exists(key)` — true iff the key is present in the /// AttributeBag, regardless of its value. Distinct from `IsTrue` - /// (which only succeeds for truthy values). Per DSL §2.2. + /// (which only succeeds for truthy values). Exists { key: String, }, /// DSL `value_key in set_key` (negate=false) / `value_key not in set_key` /// (negate=true). Both operands are attribute keys, not literals — the /// scalar at `value_key` is checked for membership in the StringSet at - /// `set_key`. Per DSL §2.4. Returns `false` if either key is missing or + /// `set_key`. Returns `false` if either key is missing or /// the types don't match (scalar must resolve to a string). InSet { value_key: String, @@ -113,8 +113,8 @@ pub enum Condition { /// Compound predicate. /// -/// `Always` is the implicit-true predicate for bare-effect rules -/// (DSL §3.1): `- plugin(rate_limiter)` / `- taint(audit)` / unconditional +/// `Always` is the implicit-true predicate for bare-effect rules: +/// `- plugin(rate_limiter)` / `- taint(audit)` / unconditional /// `- deny` / `- allow`. It's never produced by predicate-string parsing /// — only by rule-level forms where no `when:` is supplied. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -126,17 +126,17 @@ pub enum Expression { Always, } -/// One thing a matching rule does. Mirrors DSL spec §3 effect classes: +/// One thing a matching rule does. Mirrors the DSL effect classes: /// /// * Control — `Allow`, `Deny` /// * Label — `Taint` /// * Host — `Plugin`, `Delegate` /// /// Content effects (`redact`, `mask`, `omit`, `hash`) and orchestration -/// (`Sequential`, `Parallel`) land in later slices (E2 / E3). +/// (`Sequential`, `Parallel`) land in later work. /// /// PDP calls (`cedar:(…)`, `opa(…)`, …) remain top-level [`Step`] -/// variants for now; folding them into `Effect` is an E4 cleanup. +/// variants for now; folding them into `Effect` is a cleanup. /// /// # Inside a `Vec` (a rule's `effects` body) /// @@ -179,14 +179,13 @@ pub enum Effect { /// confirmation, step-up, …) through a channel plugin, hold pending /// state across the agent's retries, validate the response, resume. /// The elicitation analogue of `Delegate`. See - /// `docs/apl-manager-approval-ciba-design.md` and /// [`crate::step::ElicitStep`]. Elicit(crate::step::ElicitStep), Taint { label: String, scopes: Vec, }, - /// Content effect (DSL §3) — apply a pipe chain (`redact`, `mask`, + /// Content effect — apply a pipe chain (`redact`, `mask`, /// `omit`, `hash`, validators, transforms) to a field in the /// route's args or result. The author writes /// `result.salary | redact` inside a `do:` body; the parser @@ -238,7 +237,7 @@ pub enum Effect { source: String, }, /// External PDP call. `on_allow` / `on_deny` are reaction effect - /// lists fired against the PDP's decision (DSL §7.5). Replaces + /// lists fired against the PDP's decision. Replaces /// `Step::Pdp { ... }` — `args`-shape stays identical. Pdp { call: crate::step::PdpCall, @@ -372,7 +371,7 @@ impl From for Effect { /// One of the four lifecycle phases the evaluator runs per route. /// -/// See docs/specs/apl-design.md §3 — the `PolicyEvaluator` trait has one +/// The `PolicyEvaluator` trait has one /// async method per phase. `declared_phases()` lets the host skip phases /// the route doesn't use. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -444,7 +443,7 @@ pub struct DenyResponse { /// `args` and `result` are per-field pipelines (validators + transforms). /// `policy` and `post_policy` are step lists — predicate-and-action rules /// plus PDP calls, plugin invocations, and taint effects. See -/// apl-dsl-spec §1.2 / §4 / §7. +/// apl-dsl-spec. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct CompiledRoute { pub route_key: String, diff --git a/crates/apl-core/src/step.rs b/crates/apl-core/src/step.rs index b1cfe501..84cf5ec7 100644 --- a/crates/apl-core/src/step.rs +++ b/crates/apl-core/src/step.rs @@ -19,8 +19,7 @@ // recognized but no-op in apl-core — actual SessionStore writes happen in // `apl-cpex`, which has access to that machinery. // -// Grounded in apl-dsl-spec.md §3 (effects) / §7 (PDP integration) and -// apl-design.md §8.1 (PdpResolver seam). +// Covers effects, PDP integration, and the PdpResolver seam. use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -38,16 +37,16 @@ use crate::rules::Rule; /// `Step` exists only because `parse_step` builds its nodes /// incrementally and the conversion to `Effect::When` / /// `Effect::Pdp` happens at the top of `compile_apl_blocks` once -/// the source position is known. Not part of the public API as of -/// E4 — external code dispatches on `Effect` everywhere. +/// the source position is known. Not part of the public API — +/// external code dispatches on `Effect` everywhere. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub(crate) enum Step { - /// Predicate-and-action rule (the existing 5a/5b/5c case). + /// Predicate-and-action rule. Rule(Rule), /// External PDP call. `on_deny` / `on_allow` are reaction Step lists - /// that fire based on the PDP's decision (DSL §7.5). + /// that fire based on the PDP's decision. Pdp { call: PdpCall, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -63,8 +62,7 @@ pub(crate) enum Step { /// `delegate: { plugin: ..., ... }` — mint a downstream delegation /// token via a TokenDelegateHook plugin. Populates /// `delegation.granted_*` attributes in the bag so subsequent - /// rules in the same step list can read them. See - /// `docs/apl-identity-delegation-design.md`. + /// rules in the same step list can read them. Delegate(DelegateStep), /// `taint(label[, scope])` — apply a taint label. Always succeeds; @@ -77,8 +75,7 @@ pub(crate) enum Step { /// `require_approval(...)` / `confirm(...)` / … — dispatch an /// elicitation to a human and resume once resolved. The elicitation /// analogue of `Delegate`; resolution is dispatched to an - /// `ElicitationHandler` plugin via apl-cpex. See - /// `docs/apl-manager-approval-ciba-design.md`. + /// `ElicitationHandler` plugin via apl-cpex. Elicit(ElicitStep), } @@ -148,8 +145,7 @@ pub struct DelegateStep { /// runtime applies to the human's response. A single AST node /// ([`Step::Elicit`]) covers every kind; the DSL exposes each via a /// sugar verb (`require_approval` → `Approval`, `confirm` → `Confirm`, -/// …) that all parse to the same node. See -/// `docs/apl-elicitation-hook-design.md` for the per-kind contracts. +/// …) that all parse to the same node. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ElicitKind { @@ -200,7 +196,7 @@ impl ElicitKind { /// request that hits this step) and *resolution* (a later retry). That /// gap is owned by the channel (e.g. Keycloak CIBA), never by a plugin /// call: each of dispatch/check/validate is short and synchronous to the -/// request it runs in. See `docs/apl-manager-approval-ciba-design.md`. +/// request it runs in. /// /// # First arrival vs. retry /// @@ -890,8 +886,7 @@ impl Step { /// chain attributes (`delegation.depth`, `delegation.origin`, /// `delegation.chain`, ...) populated by identity resolver plugins /// via `IdentityPayload.delegation` + apply-to-extensions, then -/// surfaced through apl-cmf's BagBuilder. See -/// `docs/specs/delegation-hooks-rust-spec.md` §6.3 for that mapping. +/// surfaced through apl-cmf's BagBuilder. /// /// The `delegation.granted.*` sub-namespace defined here is for /// OUTBOUND results — what came back from a `delegate(...)` step @@ -922,7 +917,7 @@ pub mod delegation_bag_keys { /// On *dispatch* the runtime writes `id` + `status = "pending"` (plus /// `approver` / `intent_id` when known). On *resolution* it updates /// `status` and sets `outcome`. A phase with a pending elicitation does -/// not forward (see `docs/apl-manager-approval-ciba-design.md`). +/// not forward. pub mod elicitation_bag_keys { /// `String` — the elicitation id the agent echoes on retry. Server-side /// key into `{requester, args, scope, original_request_id}`. diff --git a/crates/apl-core/tests/yaml_end_to_end.rs b/crates/apl-core/tests/yaml_end_to_end.rs index 8ef30593..53aa5a83 100644 --- a/crates/apl-core/tests/yaml_end_to_end.rs +++ b/crates/apl-core/tests/yaml_end_to_end.rs @@ -8,8 +8,8 @@ // only (`compile_config` + `evaluate_route` + traits) and serves as the // authoritative "if this passes, apl-core works as a unit" check. // -// The fixture follows Example 1 from unified-config-proposal.md, adapted to -// the map-keyed `routes:` shape that the parser actually accepts (the spec's +// The fixture is a representative HR route, adapted to +// the map-keyed `routes:` shape that the parser actually accepts (the // list-with-matchers form is a deferred shape). use std::sync::Arc; diff --git a/crates/apl-cpex/src/cmf_invoker.rs b/crates/apl-cpex/src/cmf_invoker.rs index 1dc91dc5..568f3b16 100644 --- a/crates/apl-cpex/src/cmf_invoker.rs +++ b/crates/apl-cpex/src/cmf_invoker.rs @@ -186,7 +186,7 @@ impl CmfPluginInvoker { /// ([`persist_session`] below) picks them up. Filters by /// `TaintScope::Session` — Message-scoped taints (and any future /// scope) are deliberately ignored here; they have their own - /// destination (TBD: TS2 — a labels slot on `MessagePayload`). + /// destination (TBD: a labels slot on `MessagePayload`). /// /// Host (`AplRouteHandler`) calls this once per request after /// `evaluate_pre` / `evaluate_post` returns, with the diff --git a/crates/apl-cpex/src/dispatch_plan.rs b/crates/apl-cpex/src/dispatch_plan.rs index 8023c268..9ecfdaea 100644 --- a/crates/apl-cpex/src/dispatch_plan.rs +++ b/crates/apl-cpex/src/dispatch_plan.rs @@ -25,7 +25,7 @@ // When APL declares a route-level `plugins.:` block that narrows // `capabilities` or changes `on_error`, the plan creates a derived // `PluginRef` wrapping the same plugin `Arc` with a merged -// `TrustedConfig`. Per `feedback_override_isolation.md`: each derived +// `TrustedConfig`. Each derived // PluginRef gets a fresh `AtomicBool` circuit breaker — failures in the // override-context plugin don't disable the base, and vice versa. // @@ -222,10 +222,10 @@ impl RouteDispatchPlan { // step/field classification — they're a separate hook family. // We still apply per-call config overrides via the existing // `build_override_entries` pathway, threading the step's - // `config_override` as the only override surface (Slice B - // doesn't expose per-step caps or on_error overrides on - // delegation entries — the on_error lives in the IR step - // itself and is honored by the evaluator). + // `config_override` as the only override surface (per-step + // caps and on_error overrides aren't exposed on delegation + // entries — the on_error lives in the IR step itself and is + // honored by the evaluator). for name in collect_delegate_plugin_names(route) { let entries = manager .build_override_entries(&name, None, None, None) @@ -317,8 +317,8 @@ fn parse_on_error(s: &str) -> Option { /// `visit` on each. Used by `collect_*_names` below to find Plugin / /// Delegate references that may be nested inside `Effect::When`, /// `Effect::Sequential`, `Effect::Parallel`, or `Effect::Pdp` reaction -/// lists. Pre-E4 these were flat — Step::Plugin lived directly under -/// policy: — so a simple iter() was enough; after E4 the IR is tree- +/// lists. Previously these were flat — Step::Plugin lived directly under +/// policy: — so a simple iter() was enough; now the IR is tree- /// shaped and the same scan needs recursion. fn walk_effects(effects: &[Effect], visit: &mut F) { for e in effects { diff --git a/crates/apl-cpex/src/lib.rs b/crates/apl-cpex/src/lib.rs index 4479f75d..df27803f 100644 --- a/crates/apl-cpex/src/lib.rs +++ b/crates/apl-cpex/src/lib.rs @@ -16,7 +16,7 @@ // // # v0 simplification — single-view-per-Message // -// CMF spec §4.2 distinguishes two messaging patterns: +// CMF distinguishes two messaging patterns: // - LLM wire format — bundled multi-part Messages (thinking + text + // tool_call(s)) — many MessageViews per Message. // - Framework/protocol format (MCP, A2A, LangGraph) — single diff --git a/crates/apl-cpex/src/route_handler.rs b/crates/apl-cpex/src/route_handler.rs index 2128aeaa..94329341 100644 --- a/crates/apl-cpex/src/route_handler.rs +++ b/crates/apl-cpex/src/route_handler.rs @@ -329,7 +329,7 @@ impl AnyHookHandler for AplRouteHandler { } } - // Slice B: real delegation invoker, sharing the CMF invoker's + // Real delegation invoker, sharing the CMF invoker's // extensions Mutex so a `delegate(...)` step's writes to // raw_credentials / delegation are visible to downstream CMF // plugins and to the post phase. Routes that don't declare @@ -394,8 +394,8 @@ impl AnyHookHandler for AplRouteHandler { // pipeline `Stage::Taint`) into `extensions.security.labels` // so the existing label-diff flow inside `persist_session` // picks them up. Message-scoped taints are filtered out by - // `apply_session_taints` — they need their own destination - // (see TS2). No-op when no taints emitted. + // `apply_session_taints` — they need their own destination. + // No-op when no taints emitted. invoker.apply_session_taints(&decision.taints).await; // Commit any session-scoped labels accumulated during this diff --git a/crates/apl-cpex/src/session_resolver.rs b/crates/apl-cpex/src/session_resolver.rs index 338f74b3..c06a7a50 100644 --- a/crates/apl-cpex/src/session_resolver.rs +++ b/crates/apl-cpex/src/session_resolver.rs @@ -394,7 +394,7 @@ mod tests { #[test] fn tier1_same_session_id_claim_different_subjects_are_distinct() { - // The Finding 2 guarantee for T1. An issuer that reuses a + // The guarantee for T1. An issuer that reuses a // session_id value across multiple principals (multi-tenant // naming conventions, counters that don't carry the subject, // etc.) must NOT let one principal land in another's session diff --git a/crates/apl-cpex/src/visitor.rs b/crates/apl-cpex/src/visitor.rs index f166e13b..3e1320fa 100644 --- a/crates/apl-cpex/src/visitor.rs +++ b/crates/apl-cpex/src/visitor.rs @@ -626,7 +626,7 @@ impl ConfigVisitor for AplConfigVisitor { continue; } - // E3.1 — plugin-mode validation for `parallel:` blocks. + // Plugin-mode validation for `parallel:` blocks. // `apl-core::Effect::validate_parallel_purity` already rejected // FieldOp / Delegate at parse time; this pass checks that every // `plugin(X)` inside a `parallel:` references a plugin whose diff --git a/crates/apl-cpex/tests/cmf_invoker_dispatch.rs b/crates/apl-cpex/tests/cmf_invoker_dispatch.rs index 90a260c2..766b8ea4 100644 --- a/crates/apl-cpex/tests/cmf_invoker_dispatch.rs +++ b/crates/apl-cpex/tests/cmf_invoker_dispatch.rs @@ -390,7 +390,7 @@ async fn current_payload_reflects_accumulated_mutations() { // TrustedConfig. cpex-core's executor then enforces the narrower caps // in its single per-entry `filter_extensions` pass — no double filter, // no second clone of security. The base plugin's circuit breaker stays -// isolated per `feedback_override_isolation.md`. +// isolated. // --------------------------------------------------------------------- /// Capture-plugin fixture — records the Extensions it actually receives @@ -584,12 +584,12 @@ async fn route_override_caps_narrow_what_plugin_sees() { } // --------------------------------------------------------------------- -// Slice 101 — hook routing table regression +// Hook routing table regression // --------------------------------------------------------------------- // // Multi-hook plugin selection bug regression: a plugin registered // under BOTH `cmf.tool_pre_invoke` and `cmf.tool_post_invoke` must -// dispatch to the right entry per phase. Before Slice 101 the +// dispatch to the right entry per phase. Previously the // dispatch plan classified both as "step" and arbitrary "first // non-field wins" picked one for every dispatch — silent wrong // routing when policy and post_policy needed different handlers. diff --git a/crates/apl-cpex/tests/delegate_step_e2e.rs b/crates/apl-cpex/tests/delegate_step_e2e.rs index 2808185a..fc5b4006 100644 --- a/crates/apl-cpex/tests/delegate_step_e2e.rs +++ b/crates/apl-cpex/tests/delegate_step_e2e.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Teryl Taylor // -// End-to-end test for `Step::Delegate` dispatch (Slice B). +// End-to-end test for `Step::Delegate` dispatch. // // Verifies the full flow: // * APL parser produces a `Step::Delegate` from policy YAML. @@ -286,7 +286,7 @@ async fn delegate_step_grants_visible_to_downstream_require() { }); // APL semantics: `allow` rules don't short-circuit — only `deny` - // halts (spec §3). So the assertion shape is "deny if NOT granted", + // halts. So the assertion shape is "deny if NOT granted", // which falls through to the implicit allow at end-of-steps when // the delegate succeeded. let yaml = r#" diff --git a/crates/apl-cpex/tests/end_to_end_route.rs b/crates/apl-cpex/tests/end_to_end_route.rs index a7ff63cb..3a676049 100644 --- a/crates/apl-cpex/tests/end_to_end_route.rs +++ b/crates/apl-cpex/tests/end_to_end_route.rs @@ -540,7 +540,7 @@ routes: assert_eq!(stored, vec!["PII".to_string(), "PRIOR".to_string()]); } -/// Slice TS1 proof: an APL `taint(audit, session)` step lands the +/// Proof: an APL `taint(audit, session)` step lands the /// label in `security.labels` (via `apply_session_taints`) AND the /// SessionStore (via `persist_session`). No plugin is involved — the /// taint comes from the YAML, not from any handler's modify_extensions. diff --git a/crates/apl-cpex/tests/visitor_e2e.rs b/crates/apl-cpex/tests/visitor_e2e.rs index a8659f56..edf2a51f 100644 --- a/crates/apl-cpex/tests/visitor_e2e.rs +++ b/crates/apl-cpex/tests/visitor_e2e.rs @@ -475,10 +475,10 @@ routes: /// APL block as a `PluginError::Config` out of `load_config_yaml`. Catches /// regressions where visitor errors swallow into Ok(_) or panic. // --------------------------------------------------------------------- -// Slice 102 — multi-entity-type route support (llm / prompt / resource) +// Multi-entity-type route support (llm / prompt / resource) // --------------------------------------------------------------------- // -// Pre-Slice-102, the visitor hardcoded annotation on +// Previously, the visitor hardcoded annotation on // `cmf.tool_pre_invoke` / `cmf.tool_post_invoke` regardless of route // entity_type — so an `llm:` route would silently bind to the tool // hooks and never fire when the host called `invoke_named::("cmf.llm_input", ...)`. @@ -525,7 +525,7 @@ routes: } /// Same llm route but post — annotation lands on `cmf.llm_output`. -/// Pre-Slice-102, this would have annotated on `cmf.tool_post_invoke` +/// Previously, this would have annotated on `cmf.tool_post_invoke` /// and never matched. #[tokio::test] async fn llm_route_annotates_on_llm_output_hook_for_post_phase() { @@ -623,7 +623,7 @@ routes: } /// Cross-check: an llm route's APL annotation MUST NOT install on -/// `cmf.tool_pre_invoke`. Pre-Slice-102, the visitor would have +/// `cmf.tool_pre_invoke`. Previously, the visitor would have /// annotated llm routes on the tool hook by mistake; this test pins /// that the bug is gone. /// diff --git a/crates/cpex-core/src/context.rs b/crates/cpex-core/src/context.rs index 27105f8c..474776ec 100644 --- a/crates/cpex-core/src/context.rs +++ b/crates/cpex-core/src/context.rs @@ -17,7 +17,7 @@ // Identity, request metadata, tenant scope, etc. live in extensions // (MetaExtension, SecurityExtension), not in the context. // -// Mirrors the spec's PluginContext in plugin-framework-spec-v2.md §8.1. +// Mirrors the spec's PluginContext. use std::collections::HashMap; diff --git a/crates/cpex-core/src/delegation/mod.rs b/crates/cpex-core/src/delegation/mod.rs index af86a83c..61c344fe 100644 --- a/crates/cpex-core/src/delegation/mod.rs +++ b/crates/cpex-core/src/delegation/mod.rs @@ -9,9 +9,9 @@ // trait machinery (provided by cpex-core's generic hooks layer) // plus the hook-specific payload + result types. // -// Sub-step A scope: data shapes + host helpers — no executor +// Scope: data shapes + host helpers — no executor // wiring (that's free via `mgr.invoke_named::`), -// no TokenCacheControl trait (that lands in a follow-up slice with +// no TokenCacheControl trait (that lands in a follow-up with // the cache infrastructure). pub mod hook; diff --git a/crates/cpex-core/src/delegation/payload.rs b/crates/cpex-core/src/delegation/payload.rs index 9b2df7f3..dc752063 100644 --- a/crates/cpex-core/src/delegation/payload.rs +++ b/crates/cpex-core/src/delegation/payload.rs @@ -5,7 +5,7 @@ // // `DelegationPayload` — the unified state struct threaded through the // TokenDelegate hook chain. Same input/output split pattern as -// `IdentityPayload` (slice 2): +// `IdentityPayload`: // // * **Input** (private — host-supplied, never mutated by handlers) — // `bearer_token`, `target_name`, `target_type`, `target_audience`, @@ -21,7 +21,7 @@ // // # Where this hook fits // -// IdentityResolve (slice 2) is *inbound* — validates the caller's +// IdentityResolve is *inbound* — validates the caller's // credentials at request entry, populates `security.subject` / // `security.client` / `security.caller_workload`. TokenDelegate is // *outbound* — when a plugin (typically a forwarding proxy) needs to @@ -34,11 +34,11 @@ // // # Caching // -// Not in this slice. The spec describes a `TokenCacheControl` trait -// at §9.8 that wraps this hook with `get_or_mint(audience, scopes)` +// Not implemented yet. The spec describes a `TokenCacheControl` trait +// that wraps this hook with `get_or_mint(audience, scopes)` // semantics — outbound callers ask the trait for a token; the trait // hits the cache first and only dispatches through the hook on cache -// miss. That layer lives one slice later. For now, every +// miss. That layer comes later. For now, every // `mgr.invoke_named::(...)` re-runs the chain. // // # Rejection @@ -401,7 +401,7 @@ impl DelegationPayload { // Default to OnBehalfOfUser when the handler didn't // populate `delegation_mode`. Backward-compatible with - // handlers from sub-step B; future handlers should + // earlier handlers; future handlers should // populate the field explicitly. let mode = self .delegation_mode diff --git a/crates/cpex-core/src/extensions/authorization.rs b/crates/cpex-core/src/extensions/authorization.rs index fe95c15a..e9a42d4d 100644 --- a/crates/cpex-core/src/extensions/authorization.rs +++ b/crates/cpex-core/src/extensions/authorization.rs @@ -8,7 +8,7 @@ // Carried on DelegationHop alongside `scopes_granted`. Each hop can narrow // the details structurally (drop entries, remove actions, add constraints). // The narrowing-check helper lives elsewhere (framework enforcement at the -// TokenDelegate boundary, per docs/specs/delegation-hooks-rust-spec.md §9.6). +// TokenDelegate boundary). use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; diff --git a/crates/cpex-core/src/extensions/container.rs b/crates/cpex-core/src/extensions/container.rs index a89c7e0f..e1d37827 100644 --- a/crates/cpex-core/src/extensions/container.rs +++ b/crates/cpex-core/src/extensions/container.rs @@ -278,8 +278,8 @@ pub struct OwnedExtensions { pub framework: Option>, pub meta: Option>, /// Raw credentials are shared by Arc here too — write tokens for - /// `inbound_tokens` and `delegated_tokens` mutation paths land in - /// slice 2 (IdentityResolve) and slice 3 (TokenDelegate). Until + /// `inbound_tokens` and `delegated_tokens` mutation paths land with + /// the IdentityResolve and TokenDelegate hooks. Until /// then, no plugin writes through `OwnedExtensions.raw_credentials`. pub raw_credentials: Option>, diff --git a/crates/cpex-core/src/extensions/delegation.rs b/crates/cpex-core/src/extensions/delegation.rs index a8f085fa..2f5f9f05 100644 --- a/crates/cpex-core/src/extensions/delegation.rs +++ b/crates/cpex-core/src/extensions/delegation.rs @@ -14,8 +14,8 @@ use super::security::SubjectType; /// Delegation strategy used to mint the credential at this hop. /// -/// The known variants cover the reference implementations in -/// docs/specs/delegation-hooks-rust-spec.md §9.5. `Custom(String)` is the +/// The known variants cover the reference implementations. +/// `Custom(String)` is the /// escape hatch for host-defined strategies (UCAN variants, in-house mints). /// Marked `#[non_exhaustive]` so new known variants can be added without a /// breaking change to host code that exhaustively matches. diff --git a/crates/cpex-core/src/extensions/filter.rs b/crates/cpex-core/src/extensions/filter.rs index b260d36c..91903929 100644 --- a/crates/cpex-core/src/extensions/filter.rs +++ b/crates/cpex-core/src/extensions/filter.rs @@ -432,7 +432,7 @@ fn build_filtered_security( } // Our own outbound workload identity — also gated under - // `read_workload`, per spec §4.4. Plugins not declaring it never + // `read_workload`. Plugins not declaring it never // see our gateway's SPIFFE-SVID. if let Some(ref tw) = security.this_workload { let policy = slot_policy(SlotName::SecurityThisWorkload); diff --git a/crates/cpex-core/src/extensions/guarded.rs b/crates/cpex-core/src/extensions/guarded.rs index 9596dc94..5b095987 100644 --- a/crates/cpex-core/src/extensions/guarded.rs +++ b/crates/cpex-core/src/extensions/guarded.rs @@ -10,7 +10,7 @@ // Write access requires the framework to issue a WriteToken based on // the plugin's declared capabilities. // -// Mirrors the spec in rust-implementation-spec.md §2.3. +// Mirrors the reference implementation spec. use serde::{Deserialize, Serialize}; diff --git a/crates/cpex-core/src/extensions/monotonic.rs b/crates/cpex-core/src/extensions/monotonic.rs index 82530199..371740a5 100644 --- a/crates/cpex-core/src/extensions/monotonic.rs +++ b/crates/cpex-core/src/extensions/monotonic.rs @@ -9,7 +9,7 @@ // remove(). Declassification requires a DeclassifierToken that only // the security subsystem can construct. // -// Mirrors the spec in rust-implementation-spec.md §2.2. +// Mirrors the reference implementation spec. use std::collections::HashSet; use std::hash::Hash; diff --git a/crates/cpex-core/src/extensions/raw_credentials.rs b/crates/cpex-core/src/extensions/raw_credentials.rs index dc739677..0929c8a8 100644 --- a/crates/cpex-core/src/extensions/raw_credentials.rs +++ b/crates/cpex-core/src/extensions/raw_credentials.rs @@ -4,7 +4,7 @@ // Authors: Teryl Taylor // // `RawCredentialsExtension` — Layer 3 of the three-layer credential -// storage model (docs/specs/delegation-hooks-rust-spec.md §4.2). +// storage model. // Carries the *raw* token material — bearer JWTs, opaque session // strings, SPIFFE-JWT-SVIDs, UCAN tokens, transaction tokens — that // IdentityResolve and TokenDelegate handlers need to do their jobs. @@ -39,8 +39,6 @@ // security audit story is much simpler when "raw credentials never // leave the host process" is an invariant rather than a per-plugin // trust decision. Handlers that need raw material must run in-process. -// See the slice plan and the architecture discussion in -// `docs/raw-credentials-slice-plan.md` for the reasoning. // // # Memory hygiene // @@ -227,7 +225,7 @@ impl RawDelegatedToken { /// by `DelegationKey`. Populated by TokenDelegate handlers and /// read by forwarding / proxy plugins. /// -/// `plugin_credentials` (spec §10.7) is intentionally absent until +/// `plugin_credentials` is intentionally absent until /// a plugin-credential consumer exists. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct RawCredentialsExtension { diff --git a/crates/cpex-core/src/extensions/security.rs b/crates/cpex-core/src/extensions/security.rs index 34ceac86..6c9896ef 100644 --- a/crates/cpex-core/src/extensions/security.rs +++ b/crates/cpex-core/src/extensions/security.rs @@ -267,8 +267,7 @@ pub struct WorkloadIdentity { /// /// Carries security labels (monotonic add-only), classification, /// up to four distinct identity principals, and data-policy metadata. -/// The four principal slots map to the identity sources documented in -/// `docs/specs/delegation-hooks-rust-spec.md` §4.1: +/// The four principal slots map to the identity sources: /// /// - `subject` — the *user* (or service-as-user) initiating the request /// - `client` — the *OAuth client / application* brokering the request diff --git a/crates/cpex-core/src/hooks/metadata.rs b/crates/cpex-core/src/hooks/metadata.rs index 19ace802..952ef1b8 100644 --- a/crates/cpex-core/src/hooks/metadata.rs +++ b/crates/cpex-core/src/hooks/metadata.rs @@ -45,7 +45,7 @@ // this to pick the right entry for the current phase context. // // Lookup is the foundation for `apl-cpex::dispatch_plan`'s entry -// selection. See `docs/apl-hook-family-expansion.md` Layer 1. +// selection. // // # Phase semantics // diff --git a/crates/cpex-core/src/identity/hook.rs b/crates/cpex-core/src/identity/hook.rs index a2a77576..a1f4e94b 100644 --- a/crates/cpex-core/src/identity/hook.rs +++ b/crates/cpex-core/src/identity/hook.rs @@ -25,7 +25,7 @@ // // This file defines the *types*. Lifecycle wiring — when the // framework calls `invoke_named::(...)`, how results -// merge back into `Extensions` — lands in sub-step B / C of slice 2. +// merge back into `Extensions` — lands elsewhere. use crate::hooks::trait_def::PluginResult; diff --git a/crates/cpex-core/src/identity/mod.rs b/crates/cpex-core/src/identity/mod.rs index 28fca362..aeba5308 100644 --- a/crates/cpex-core/src/identity/mod.rs +++ b/crates/cpex-core/src/identity/mod.rs @@ -8,13 +8,13 @@ // Mirrors the cmf/ module layout: the hook marker + handler trait // machinery (provided by cpex-core's generic hooks layer) plus the // hook-specific payload + result types. Token-delegation lives in -// its own sibling module (slice 3); the two hook families share +// its own sibling module; the two hook families share // nothing in terms of payloads so they get separate `HookTypeDef` // markers. // -// Sub-step A scope: data shapes only — no executor wiring, no +// Scope: data shapes only — no executor wiring, no // framework merge-into-Extensions logic, no APL integration. Those -// land in sub-steps B / C / D. +// land later. pub mod hook; pub mod payload; diff --git a/crates/cpex-core/src/identity/route_config.rs b/crates/cpex-core/src/identity/route_config.rs index 1c9aa357..b4b2839f 100644 --- a/crates/cpex-core/src/identity/route_config.rs +++ b/crates/cpex-core/src/identity/route_config.rs @@ -6,8 +6,6 @@ // Route-level identity configuration — the parsed shape of a // route's `identity:` block in unified-config YAML. // -// See `docs/apl-identity-delegation-design.md` for the full design. -// // # Semantic note // // Identity binding is **hook-specific**: the `identity:` block @@ -61,7 +59,7 @@ use serde::{Deserialize, Serialize}; /// entry list when filtered for this route, and the host's /// `IdentityPayload` flows through unchanged (no resolvers fire). /// -/// Inheritance (Slice C, deferred) walks `global → tags → route` +/// Inheritance (deferred) walks `global → tags → route` /// and merges each layer's `RouteIdentityConfig` based on /// `replace_inherited`: when `false` (the default), the new layer's /// steps append after the inherited ones; when `true`, the new @@ -78,8 +76,8 @@ pub struct RouteIdentityConfig { /// (`identity: { replace_inherited: true, steps: [...] }`). /// The list-form YAML always produces `false`. /// - /// Honored by the inheritance merge once Slice C lands. Slice A - /// stores the flag without exercising its merge semantics (no + /// Honored by the inheritance merge once that lands. Today the + /// flag is stored without exercising its merge semantics (no /// inheritance to override yet at route level). #[serde(default, skip_serializing_if = "is_false")] pub replace_inherited: bool, diff --git a/crates/cpex-core/tests/delegation_e2e.rs b/crates/cpex-core/tests/delegation_e2e.rs index 4307b6f5..f0a821af 100644 --- a/crates/cpex-core/tests/delegation_e2e.rs +++ b/crates/cpex-core/tests/delegation_e2e.rs @@ -3,8 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Teryl Taylor // -// End-to-end test for the TokenDelegate hook family — sub-step B of -// slice 3. +// End-to-end test for the TokenDelegate hook family. // // Verifies the host-explicit dispatch model: an outbound caller // (typically a forwarding-proxy plugin) constructs a @@ -478,7 +477,7 @@ async fn apply_to_extensions_writes_delegated_token_keyed_by_subject() { /// delegation through downstream CMF dispatch correctly cap-gates /// the `delegated_tokens` slot. /// -/// Mirrors the slice 2 `cap_gating_post_apply_through_cmf_dispatch` +/// Mirrors the `cap_gating_post_apply_through_cmf_dispatch` /// test but for the *outbound* leg: /// 1. TokenDelegate handler mints a downstream credential. /// 2. Host applies the resolved payload back to `Extensions` via diff --git a/crates/cpex-core/tests/identity_e2e.rs b/crates/cpex-core/tests/identity_e2e.rs index 269e0906..440fa6d3 100644 --- a/crates/cpex-core/tests/identity_e2e.rs +++ b/crates/cpex-core/tests/identity_e2e.rs @@ -3,8 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Teryl Taylor // -// End-to-end test for the IdentityResolve hook family — sub-step B -// of slice 2. +// End-to-end test for the IdentityResolve hook family. // // Verifies the host-explicit dispatch model: the host constructs an // `IdentityPayload`, calls `mgr.invoke_named::(...)`, diff --git a/crates/cpex-core/tests/identity_route_e2e.rs b/crates/cpex-core/tests/identity_route_e2e.rs index 4ed024ba..03f27242 100644 --- a/crates/cpex-core/tests/identity_route_e2e.rs +++ b/crates/cpex-core/tests/identity_route_e2e.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // Authors: Teryl Taylor // -// End-to-end tests for the route-level `authentication:` block (Slice A). +// End-to-end tests for the route-level `authentication:` block. // // Verifies the hook-specific binding semantics: // * A route's `authentication:` block is the authoritative dispatch list @@ -478,7 +478,7 @@ routes: ); } -/// Slice C — end-to-end inheritance: global.identity contributes to +/// End-to-end inheritance: global.identity contributes to /// the dispatch lineup for routes that declare no identity block of /// their own. Verifies the dispatch path picks up the global layer. #[tokio::test] @@ -723,7 +723,7 @@ routes: // hook family — meaning `filter_extensions(&ext, &caps)` runs before // each handler invoke and narrows what the plugin sees to its // declared capabilities. These tests pin that behavior for the -// route-level identity dispatch path (Slice A). +// route-level identity dispatch path. // // Identity is unusual in that resolvers typically WRITE state (subject, // chain) rather than read it — but they still need read capabilities diff --git a/crates/cpex-ffi/src/lib.rs b/crates/cpex-ffi/src/lib.rs index 1ddb0a67..ad62bdfb 100644 --- a/crates/cpex-ffi/src/lib.rs +++ b/crates/cpex-ffi/src/lib.rs @@ -713,7 +713,7 @@ pub unsafe extern "C" fn cpex_plugin_names( /// (`result_msgpack_out`, `result_len_out`, `bg_handle_out`) follow /// the same discipline: null/zero on error, populated on success. /// -/// Pre-P0-1 the function consumed the input only after validation +/// Previously the function consumed the input only after validation /// passed but before `run_safely`. On `RC_TIMEOUT` / `RC_PANIC` the /// input had been consumed but `*context_table_out` was never written, /// so the Go wrapper kept its stale handle and a subsequent @@ -765,7 +765,7 @@ pub unsafe extern "C" fn cpex_invoke( // model) and post-validation errors left it consumed without // writing `*context_table_out` (a *different* ownership model). // Two contracts in one function is exactly what produced the - // P0-1 UAF. + // use-after-free. let input_ctx_table: Option = if context_table.is_null() { None } else { @@ -1443,7 +1443,7 @@ mod tests { /// Panic in a plugin must be caught at the FFI boundary and mapped /// to `RC_PANIC` rather than unwinding across `extern "C"` (UB on - /// Rust < 1.81; abort on >= 1.81). Direct regression for P0 #2. + /// Rust < 1.81; abort on >= 1.81). #[test] fn cpex_invoke_returns_rc_panic_when_plugin_panics() { let mgr = build_test_manager(); From 948b19ea75257a90fff689ffa37fbc2277a8f085 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Tue, 21 Jul 2026 15:06:43 -0400 Subject: [PATCH 7/8] chore: remove aislop-ignore comments Signed-off-by: Frederico Araujo --- builtins/plugins/delegator-oauth/src/config.rs | 3 --- builtins/plugins/elicitation-ciba/src/config.rs | 1 - builtins/plugins/identity-jwt/src/config.rs | 1 - builtins/session/valkey/src/config.rs | 1 - crates/apl-cmf/src/security.rs | 2 -- crates/cpex-core/src/delegation/payload.rs | 1 - crates/cpex-core/src/extensions/filter.rs | 2 -- crates/cpex-core/src/extensions/raw_credentials.rs | 2 -- 8 files changed, 13 deletions(-) diff --git a/builtins/plugins/delegator-oauth/src/config.rs b/builtins/plugins/delegator-oauth/src/config.rs index dd0ac214..e01b3a67 100644 --- a/builtins/plugins/delegator-oauth/src/config.rs +++ b/builtins/plugins/delegator-oauth/src/config.rs @@ -128,14 +128,11 @@ mod tests { #[test] fn config_deserializes_from_json() { let raw = json!({ - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "token_endpoint": "https://auth.example.com/oauth/token", "client_id": "gateway", - // aislop-ignore-next-line ai-slop/hardcoded-id -- Literal-secret-source test fixture, not a real credential "client_secret_source": { "kind": "literal", "secret": "dev-only" }, }); let cfg: OAuthDelegatorConfig = serde_json::from_value(raw).unwrap(); - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only assert_eq!(cfg.token_endpoint, "https://auth.example.com/oauth/token"); assert_eq!(cfg.client_id, "gateway"); assert_eq!(cfg.timeout_seconds, 5); diff --git a/builtins/plugins/elicitation-ciba/src/config.rs b/builtins/plugins/elicitation-ciba/src/config.rs index 9bab2b57..5ec71cd4 100644 --- a/builtins/plugins/elicitation-ciba/src/config.rs +++ b/builtins/plugins/elicitation-ciba/src/config.rs @@ -148,7 +148,6 @@ mod tests { "backchannel_endpoint": "https://kc/realms/corp/protocol/openid-connect/ext/ciba/auth", "token_endpoint": "https://kc/realms/corp/protocol/openid-connect/token", "client_id": "cpex-gateway", - // aislop-ignore-next-line ai-slop/hardcoded-id -- Literal-secret-source test fixture, not a real credential "client_secret_source": { "kind": "literal", "secret": "dev-only" }, }); let cfg: CibaConfig = serde_json::from_value(raw).unwrap(); diff --git a/builtins/plugins/identity-jwt/src/config.rs b/builtins/plugins/identity-jwt/src/config.rs index 4e4b82de..dd181f61 100644 --- a/builtins/plugins/identity-jwt/src/config.rs +++ b/builtins/plugins/identity-jwt/src/config.rs @@ -457,7 +457,6 @@ mod tests { #[tokio::test] async fn jwks_http_url_rejected_at_build_async() { let src = DecodingKeySource::JwksUrl { - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only url: "http://idp.example/jwks".into(), insecure_http: false, refresh_secs: 3600, diff --git a/builtins/session/valkey/src/config.rs b/builtins/session/valkey/src/config.rs index 5225e624..064a0432 100644 --- a/builtins/session/valkey/src/config.rs +++ b/builtins/session/valkey/src/config.rs @@ -381,7 +381,6 @@ mod tests { fn tls_required_error_redacts_credentials() { // rediss-less, non-localhost, with embedded creds, tls off → error. let err = - // aislop-ignore-next-line security/hardcoded-secret -- test fixture proving credentials get redacted, not a real secret parse("kind: valkey\nendpoint: redis://user:topsecret@prod.host:6379\n").unwrap_err(); let msg = format!("{err}"); assert!( diff --git a/crates/apl-cmf/src/security.rs b/crates/apl-cmf/src/security.rs index 646b0d3e..39de6938 100644 --- a/crates/apl-cmf/src/security.rs +++ b/crates/apl-cmf/src/security.rs @@ -365,7 +365,6 @@ mod tests { client_name: Some("Agent App".into()), trust_level: ClientTrustLevel::FirstParty, authorized_scopes: vec!["read".into(), "write".into()], - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only authorized_audiences: vec!["https://api.example.com".into()], roles: vec!["partner".into()], permissions: vec!["call_tool".into()], @@ -406,7 +405,6 @@ mod tests { extract_client(&agent_client(), &mut bag); assert!(bag.set_contains("client.authorized_scopes", "read")); assert!(bag.set_contains("client.authorized_scopes", "write")); - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only assert!(bag.set_contains("client.authorized_audiences", "https://api.example.com",)); assert!(bag.set_contains("client.teams", "acme")); } diff --git a/crates/cpex-core/src/delegation/payload.rs b/crates/cpex-core/src/delegation/payload.rs index dc752063..f2a9bc44 100644 --- a/crates/cpex-core/src/delegation/payload.rs +++ b/crates/cpex-core/src/delegation/payload.rs @@ -505,7 +505,6 @@ mod tests { updated.delegated_token = Some(RawDelegatedToken::new( "minted-bytes", "Authorization", - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "https://api.example.com", vec!["read".into()], Utc::now(), diff --git a/crates/cpex-core/src/extensions/filter.rs b/crates/cpex-core/src/extensions/filter.rs index 91903929..2b8722f4 100644 --- a/crates/cpex-core/src/extensions/filter.rs +++ b/crates/cpex-core/src/extensions/filter.rs @@ -780,7 +780,6 @@ mod tests { raw.delegated_tokens.insert( DelegationKey { subject_id: "alice".into(), - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into()], mode: DelegationMode::OnBehalfOfUser, @@ -788,7 +787,6 @@ mod tests { RawDelegatedToken::new( "delegated-bytes", "Authorization", - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only "https://api.example.com", vec!["read".into()], chrono::Utc::now(), diff --git a/crates/cpex-core/src/extensions/raw_credentials.rs b/crates/cpex-core/src/extensions/raw_credentials.rs index 0929c8a8..bc11b454 100644 --- a/crates/cpex-core/src/extensions/raw_credentials.rs +++ b/crates/cpex-core/src/extensions/raw_credentials.rs @@ -307,14 +307,12 @@ mod tests { fn delegation_key_hash_eq_consistency() { let k1 = DelegationKey { subject_id: "alice".into(), - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into(), "write".into()], mode: DelegationMode::OnBehalfOfUser, }; let k2 = DelegationKey { subject_id: "alice".into(), - // aislop-ignore-next-line ai-slop/hardcoded-url -- RFC 2606 example domain, test fixture only audience: "https://api.example.com".into(), scopes: vec!["read".into(), "write".into()], mode: DelegationMode::OnBehalfOfUser, From 33ca5ffb6ceea6baf5eac9c68a6bc0eaee5dd9e1 Mon Sep 17 00:00:00 2001 From: Frederico Araujo Date: Tue, 21 Jul 2026 20:44:30 -0400 Subject: [PATCH 8/8] style: apply rustfmt and gofmt Signed-off-by: Frederico Araujo --- crates/apl-core/src/parser.rs | 23 +++++++++++------------ go/cpex/manager_test.go | 14 +++++++------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/apl-core/src/parser.rs b/crates/apl-core/src/parser.rs index c7566437..195f19ba 100644 --- a/crates/apl-core/src/parser.rs +++ b/crates/apl-core/src/parser.rs @@ -2082,17 +2082,6 @@ fn parse_stage(src: &str) -> Result { ("redact", None) => Ok(Stage::Redact { condition: None }), ("omit", None) => Ok(Stage::Omit), ("hash", None) => Ok(Stage::Hash), - // Scan placeholders parse as bare identifiers. - ("pii.redact", None) => Ok(Stage::Scan { - kind: ScanKind::PiiRedact, - }), - ("pii.detect", None) => Ok(Stage::Scan { - kind: ScanKind::PiiDetect, - }), - ("injection.scan", None) => Ok(Stage::Scan { - kind: ScanKind::InjectionScan, - }), - ("mask", Some(a)) => parse_stage_mask(a, &bad), ("redact", Some(a)) => parse_stage_redact_cond(a, src), ("hash", Some(_)) => Err(bad("hash takes no arguments")), @@ -2103,10 +2092,20 @@ fn parse_stage(src: &str) -> Result { ("enum", Some(a)) => parse_stage_enum(a, &bad), ("regex", Some(a)) => Ok(parse_stage_regex(a)), ("validate", Some(a)) => Err(parse_stage_validate_rejected(a, &bad)), - // `run` is an alias for `plugin` (mirrors the policy-step alias). ("plugin" | "run", Some(a)) => parse_stage_plugin(head, a, &bad), ("taint", Some(a)) => parse_taint(a, src), + // Scan placeholders parse as bare identifiers. + ("pii.redact", None) => Ok(Stage::Scan { + kind: ScanKind::PiiRedact, + }), + ("pii.detect", None) => Ok(Stage::Scan { + kind: ScanKind::PiiDetect, + }), + ("injection.scan", None) => Ok(Stage::Scan { + kind: ScanKind::InjectionScan, + }), + (other, _) => Err(bad(&format!("unknown stage `{}`", other))), } } diff --git a/go/cpex/manager_test.go b/go/cpex/manager_test.go index 7dff38aa..747ad8a3 100644 --- a/go/cpex/manager_test.go +++ b/go/cpex/manager_test.go @@ -1187,13 +1187,13 @@ func TestLoadConfigInvalidYAML(t *testing.T) { // handle survives. // // This test: -// 1. Performs a successful invoke to get a real ContextTable. -// 2. Calls InvokeByName again with that ContextTable PLUS an -// invalid payload_type that forces Rust to return RC_PARSE_ERROR -// AFTER the consumption point. -// 3. Confirms the second call errored (sanity). -// 4. Calls Close() on the original ContextTable — must NOT crash. -// Pre-fix this was a UAF (free of already-freed memory). +// 1. Performs a successful invoke to get a real ContextTable. +// 2. Calls InvokeByName again with that ContextTable PLUS an +// invalid payload_type that forces Rust to return RC_PARSE_ERROR +// AFTER the consumption point. +// 3. Confirms the second call errored (sanity). +// 4. Calls Close() on the original ContextTable — must NOT crash. +// Pre-fix this was a UAF (free of already-freed memory). func TestInvokeByNameErrorDoesNotUAFContextTable(t *testing.T) { mgr, err := NewPluginManagerDefault() if err != nil {