From a7a14e57d6a69d8ce94d7d22bdeea9d50a61eb9a Mon Sep 17 00:00:00 2001 From: aviavissar Date: Tue, 1 Sep 2026 10:13:28 +0300 Subject: [PATCH] test(CEL-PDP)--collision-precedence-rules-(stdlib-shadowing,-bag-namespace-vs-scalar)-lack-test-coverage-#3 Signed-off-by: aviavissar Co-authored-by: Cursor --- builtins/pdps/cel/src/activation.rs | 47 +++++++++++++++++++++++++++++ builtins/pdps/cel/src/resolver.rs | 37 +++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/builtins/pdps/cel/src/activation.rs b/builtins/pdps/cel/src/activation.rs index 9812e54..f1ec762 100644 --- a/builtins/pdps/cel/src/activation.rs +++ b/builtins/pdps/cel/src/activation.rs @@ -382,4 +382,51 @@ mod tests { bag.set("delegation.depth", 3_i64); assert!(truthy("delegation.depth == 3", &bag)); } + + /// The sibling of `namespace_wins_on_leaf_collision`: a scalar + /// arrives *after* the namespace already exists. `insert` keeps the + /// branch and drops the scalar (the warning at the terminal-segment + /// arm). Driven through `insert` directly because `AttributeBag` is + /// a `HashMap` — `set` order is not `iter` order, so a bag cannot + /// pin this arm. + #[test] + fn namespace_wins_when_scalar_arrives_after_branch() { + let mut root = BTreeMap::new(); + insert( + &mut root, + "delegation.depth", + &["delegation", "depth"], + Value::from(3_i64), + ); + insert( + &mut root, + "delegation", + &["delegation"], + Value::from("scalar-value".to_owned()), + ); + + let depth_kept = match root.get("delegation") { + Some(Node::Branch(children)) => { + matches!(children.get("depth"), Some(Node::Leaf(_))) + }, + _ => false, + }; + assert!( + depth_kept, + "namespace must win when a scalar arrives after the branch exists; \ + the scalar must be dropped so delegation.depth still lives under the map", + ); + + let mut ctx = Context::default(); + for (name, node) in root { + ctx.add_variable_from_value(name, node_to_value(node)); + } + assert!( + matches!( + run_cel("delegation.depth == 3", &ctx), + Ok(Value::Bool(true)) + ), + "CEL must still resolve delegation.depth after the colliding scalar is dropped", + ); + } } diff --git a/builtins/pdps/cel/src/resolver.rs b/builtins/pdps/cel/src/resolver.rs index 9dc034c..cb0fa83 100644 --- a/builtins/pdps/cel/src/resolver.rs +++ b/builtins/pdps/cel/src/resolver.rs @@ -145,6 +145,11 @@ impl CelResolver { /// /// Composes: calling `with_functions` more than once stacks the /// callbacks. Each runs in registration order on every context. + /// Later custom setups can shadow earlier custom setups. They do + /// **not** replace a CEL standard-library overload of the same + /// name (`size`, `has`, `matches`, `double`, …) as of `cel` 0.14 + /// — the stdlib wins and the colliding registration is ignored + /// for that signature. Pick names that cannot collide. /// /// # Example /// @@ -671,6 +676,38 @@ mod tests { ); } + /// Pins custom-vs-stdlib collision precedence. A host-registered + /// `size` that would return 777 must not replace CEL's built-in + /// `size` (5 for `"hello"`). As of `cel` 0.14 the stdlib overload + /// wins; this test fails if that flips. + #[tokio::test] + async fn custom_function_does_not_shadow_stdlib_size() { + let r = CelResolver::new().with_functions(|ctx| { + ctx.add_function("size", |_s: Arc| -> i64 { 777 }); + }); + let bag = bag_with(&[("subject.id", "alice")]); + + let stdlib = r + .evaluate(&cel_call("size('hello') == 5"), &bag) + .await + .unwrap(); + assert_eq!( + stdlib.decision, + Decision::Allow, + "stdlib size('hello') is 5; if this fails, a custom size shadowed the builtin", + ); + + let custom = r + .evaluate(&cel_call("size('hello') == 777"), &bag) + .await + .unwrap(); + assert!( + matches!(custom.decision, Decision::Deny { .. }), + "custom size returning 777 must not run in place of stdlib size; got {:?}", + custom.decision, + ); + } + /// The `regex` cel-feature is explicitly enabled in our Cargo.toml. /// Pin that `matches(s, pattern)` actually works through the /// resolver so a future feature-set churn breaks loudly here.