Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5aef60b
fix(cel): keep whole-valued floats as doubles
shaneutt Sep 1, 2026
57857fc
fix(opa): reject inline modules sharing a global package subtree
shaneutt Sep 1, 2026
3b3fee3
fix(identity-jwt): redact secrets in Debug, accept any Bearer casing
shaneutt Sep 1, 2026
b7ad8ff
fix(valkey): alarm on a timed-out TTL refresh
shaneutt Sep 1, 2026
fc5bba0
fix(core): compare every authority field in chain_extends
shaneutt Sep 1, 2026
bca0e8a
fix(core): synthesize a deny when a blocking plugin attaches no viola…
shaneutt Sep 1, 2026
1589b1b
fix(apl-runtime): track http and custom writes in extensions_changed
shaneutt Sep 1, 2026
205469b
fix(core): reject unknown keys in attenuation config
shaneutt Sep 1, 2026
c989441
fix(core): serialize delegated_tokens with a non-string key
shaneutt Sep 1, 2026
9a9c3ca
fix(core): stop merge_security dropping an append-only plugin's labels
shaneutt Sep 1, 2026
f6e06e9
fix(apl-core): fail closed when a parallel branch panics
shaneutt Sep 1, 2026
74f9f4d
fix(core): validate authentication step names at load
araujof Sep 1, 2026
9af88c4
test(apl-core): pin duplicate field-pipeline key rejection
araujof Sep 1, 2026
cc68b66
fix(apl-core): fan field pipelines out over arrays on the path
araujof Sep 1, 2026
22de41f
fix(apl): reject more than one elicitation per phase
araujof Sep 1, 2026
74d41c1
docs: tighten comments added by the #54 port
araujof Sep 2, 2026
a9aee3b
test: declare the plugins that authentication steps name
araujof Sep 2, 2026
c760cc6
style: wrap an over-width const in the authentication fixture
araujof Sep 2, 2026
688ca15
docs: condense comments across the #54 port
araujof Sep 2, 2026
08b76e6
docs: keep the invariants the condensing dropped
araujof Sep 2, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 43 additions & 44 deletions builtins/pdps/cel/src/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,18 +161,13 @@ fn node_to_value(node: Node) -> Value {

/// Convert one `AttributeValue` to a `cel::Value`.
///
/// CEL's type model distinguishes `int` and `double` strictly:
/// `delegation.depth <= 2` errors if `delegation.depth` is a double
/// and `2` is an int (the literal). To shield authors from that
/// asymmetry, an `f64` whose value is a whole number and fits a `i64`
/// is yielded as `Value::Int`. The same logic applies to the
/// author-supplied yaml args (see `yaml_to_value`) — both surfaces
/// now agree.
/// An `f64` stays `Value::Float`; narrowing whole-valued floats breaks CEL
/// arithmetic such as `confidence * 100.0`.
fn attr_to_value(attr: &AttributeValue) -> Value {
match attr {
AttributeValue::Bool(b) => Value::from(*b),
AttributeValue::Int(i) => Value::from(*i),
AttributeValue::Float(f) => float_to_value(*f),
AttributeValue::Float(f) => Value::from(*f),
AttributeValue::String(s) => Value::from(s.clone()),
// StringSet → list(string). Sort before yielding so authors
// who reach for `session.labels[0]` (or any other
Expand All @@ -189,34 +184,8 @@ fn attr_to_value(attr: &AttributeValue) -> Value {
}
}

/// Yield an `f64` as `Value::Int` when it represents a whole number
/// in `i64` range, otherwise `Value::Float`. Used by both
/// `attr_to_value` (bag scalars) and `yaml_to_value` (author args) so
/// `delegation.depth: 2` works against the literal `2` regardless of
/// whether the bag populated it as `Int(2)` or `Float(2.0)`.
#[allow(
clippy::cast_possible_truncation,
clippy::cast_precision_loss,
reason = "the conversion is guarded to finite, integral, in-range values; the \
bound casts are deliberate and explained below"
)]
fn float_to_value(f: f64) -> Value {
// The upper bound is strict on purpose. `i64::MAX as f64` cannot represent
// 2^63 - 1 and rounds up to exactly 2^63, so `<=` against it would admit
// 2^63, which is one past the last i64 and saturates on conversion. `<` is
// then exactly the right test. `i64::MIN as f64` is exact at -2^63, so the
// lower bound stays inclusive.
if f.is_finite() && f.fract() == 0.0 && f >= i64::MIN as f64 && f < i64::MAX as f64 {
Value::from(f as i64)
} else {
Value::from(f)
}
}

/// Convert a `serde_yaml::Value` (author-supplied `cel:` args) to a
/// `cel::Value`. Numbers without a fractional part map to `Int`, otherwise
/// `Float`. Non-string mapping keys are skipped (CEL map keys here are
/// always strings for author ergonomics).
/// Convert an author-supplied `cel:` argument to a `cel::Value`. Integers map
/// to `Int`, floats to `Float`, and non-string mapping keys are skipped.
fn yaml_to_value(v: &serde_yaml::Value) -> Value {
match v {
serde_yaml::Value::Null => Value::Null,
Expand All @@ -225,7 +194,7 @@ fn yaml_to_value(v: &serde_yaml::Value) -> Value {
if let Some(i) = n.as_i64() {
Value::from(i)
} else {
float_to_value(n.as_f64().unwrap_or(f64::NAN))
Value::from(n.as_f64().unwrap_or(f64::NAN))
}
},
serde_yaml::Value::String(s) => Value::from(s.clone()),
Expand Down Expand Up @@ -304,23 +273,53 @@ mod tests {
assert!(truthy("session.labels.exists(l, l == 'PII')", &bag));
}

/// An `f64` whose value is a whole number is yielded as an int so
/// authors can compare against integer literals without CEL's
/// strict int-vs-double type rules blowing up. A genuinely
/// fractional `f64` still arrives as a float (so `confidence > 0.9`
/// behaves correctly).
#[test]
fn whole_number_float_arrives_as_int_for_literal_compare() {
fn double_scalar_compares_against_int_literal() {
let mut bag = AttributeBag::new();
bag.set("delegation.depth", 2.0_f64);
bag.set("intent.confidence", 0.92_f64);
// Compare-with-int-literal: requires the bag value to be int.
assert!(truthy("delegation.depth == 2", &bag));
assert!(truthy("delegation.depth <= 2", &bag));
// Genuine doubles still compare to double literals.
assert!(truthy("intent.confidence > 0.9", &bag));
}

#[test]
fn whole_valued_float_keeps_arithmetic() {
let mut bag = AttributeBag::new();
bag.set("intent.confidence", 1.0_f64);
assert!(truthy("intent.confidence * 100.0 >= 90.0", &bag));
bag.set("intent.confidence", 0.92_f64);
assert!(truthy("intent.confidence * 100.0 >= 90.0", &bag));
}

#[test]
fn mixed_int_float_comparison_is_order_independent() {
let mut bag = AttributeBag::new();
bag.set("delegation.depth", 2.0_f64);
assert!(truthy("2 == delegation.depth", &bag));
assert!(truthy("2 <= delegation.depth", &bag));
assert!(truthy("2 >= delegation.depth", &bag));
assert!(truthy("3 > delegation.depth", &bag));
assert!(truthy("1 < delegation.depth", &bag));
assert!(truthy("3 != delegation.depth", &bag));
}

#[test]
fn whole_valued_float_in_int_list_matches() {
let mut bag = AttributeBag::new();
bag.set("delegation.depth", 2.0_f64);
assert!(
truthy("delegation.depth in [1, 2, 3]", &bag),
"float 2.0 in int list"
);
bag.set("intent.confidence", 1.0_f64);
assert!(
truthy("intent.confidence in [0.5, 1.0]", &bag),
"float in float list"
);
}

/// `StringSet` is yielded in sorted order so indexing returns a
/// stable value across runs. `"compensation" < "PII"` (ASCII;
/// uppercase letters sort before lowercase, but both labels here
Expand Down
69 changes: 65 additions & 4 deletions builtins/pdps/opa/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,12 @@ impl OpaResolver {
.add_policy(INLINE_MODULE_NAME.to_owned(), src.to_owned())
.map_err(|e| EngineError::Compile(e.to_string()))?;

// Reject an inline module that lands in a global module's package — it
// would merge into (and could override) operator policy. Fail-closed:
// inline modules may add new packages, never redefine a global one.
if self.global_packages.contains(&package) {
// Inline modules may add packages but may not share a global package subtree.
if self
.global_packages
.iter()
.any(|g| packages_share_subtree(&package, g))
{
return Err(EngineError::PackageCollision(package));
}

Expand Down Expand Up @@ -385,6 +387,15 @@ impl OpaResolver {
}
}

/// Whether two dotted Rego package paths are equal or one contains the other.
/// Path-separator boundaries keep siblings such as `data.authz` and
/// `data.authznext` distinct.
fn packages_share_subtree(a: &str, b: &str) -> bool {
a == b
|| a.strip_prefix(b).is_some_and(|rest| rest.starts_with('.'))
|| b.strip_prefix(a).is_some_and(|rest| rest.starts_with('.'))
}

/// Internal — failure shapes from preparing a per-step engine. All three
/// always deny regardless of `on_error`: a compile error is an author bug, a
/// package collision is a trust-boundary violation, and a cache-full condition
Expand Down Expand Up @@ -965,6 +976,23 @@ msg := "not a decision"
}
}

#[tokio::test]
async fn inline_module_cannot_override_global_subpackage() {
let global = "package authz\ndefault allow := false\nallow if data.authz.exceptions[input.subject.id]\n";
let r = resolver(&[global], OnError::Allow);
let inline = "package authz.exceptions\nexceptions := {\"eve\": true}\n";
let out = r
.evaluate(&call("data.authz.allow", Some(inline)), &bag("eve"))
.await
.unwrap();
match out.decision {
Decision::Deny { reason, .. } => {
assert!(reason.unwrap_or_default().contains("collides"));
},
other => panic!("sub-package collision must deny, got {other:?}"),
}
}

/// An inline module in a fresh package (no global collision) is accepted and
/// evaluates — inline modules remain a usable feature for additive policy.
#[tokio::test]
Expand All @@ -978,6 +1006,39 @@ msg := "not a decision"
assert_eq!(out.decision, Decision::Allow);
}

#[tokio::test]
async fn inline_module_in_prefix_sibling_package_is_allowed() {
let global = "package authz\nallow if input.subject.id == \"alice\"\n";
let r = resolver(&[global], OnError::Deny);
let inline = "package authznext\nallow if input.subject.id == \"alice\"\n";
let out = r
.evaluate(&call("data.authznext.allow", Some(inline)), &bag("alice"))
.await
.unwrap();
assert_eq!(
out.decision,
Decision::Allow,
"a prefix-sibling package must not collide with a global package"
);
}

#[tokio::test]
async fn inline_module_that_is_parent_of_global_collides() {
let global = "package authz.sub\nallow if input.subject.id == \"alice\"\n";
let r = resolver(&[global], OnError::Allow);
let inline = "package authz\nallow := true\n";
let out = r
.evaluate(&call("data.authz.allow", Some(inline)), &bag("alice"))
.await
.unwrap();
match out.decision {
Decision::Deny { reason, .. } => {
assert!(reason.unwrap_or_default().contains("collides"));
},
other => panic!("parent-package collision must deny, got {other:?}"),
}
}

#[tokio::test]
async fn missing_query_is_dispatch_error() {
let r = resolver(&[ALLOW_WITH_DEFAULT], OnError::Deny);
Expand Down
42 changes: 40 additions & 2 deletions builtins/plugins/identity-jwt/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ const DEFAULT_LEEWAY_SECONDS: u64 = 60;
/// `PluginFactory::create` trait surface across the workspace) while
/// putting the network I/O on the natural async hook the host
/// already drives via `PolicyEngine::initialize().await`.
#[derive(Debug)]
pub struct JwtIdentityResolver {
cfg: PluginConfig,
/// Each issuer behind its own `Arc` so the verify path can clone
Expand Down Expand Up @@ -130,6 +129,21 @@ pub struct JwtIdentityResolver {
header: String,
}

// Implement `Debug` manually because `cfg` and `pending_jwks` may contain HMAC
// signing secrets or inline PEM keys.
impl std::fmt::Debug for JwtIdentityResolver {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("JwtIdentityResolver")
.field("name", &self.cfg.name)
.field("role", &self.role)
.field("header", &self.header)
.field("pending_jwks_count", &self.pending_jwks.len())
.field("cfg", &"<redacted>")
.field("pending_jwks", &"<redacted>")
.finish_non_exhaustive()
}
}

impl JwtIdentityResolver {
/// Build a resolver from a `PluginConfig`. Reads `cfg.config`
/// (the plugin-specific config field — `Option<JsonValue>`),
Expand Down Expand Up @@ -677,7 +691,7 @@ impl HookHandler<IdentityHook> for JwtIdentityResolver {
let header_lc = self.header.to_ascii_lowercase();
let header_value = payload.headers().get(header_lc.as_str());
let raw_token: String = match header_value {
Some(v) => v.strip_prefix("Bearer ").unwrap_or(v).to_owned(),
Some(v) => strip_bearer_prefix(v).to_owned(),
None if !payload.raw_token().is_empty() => payload.raw_token().to_owned(),
None => {
return PluginResult::deny(PluginViolation::new(
Expand Down Expand Up @@ -934,6 +948,19 @@ fn peek_issuer(token: &str) -> Option<String> {
value.get("iss")?.as_str().map(String::from)
}

/// Strip a leading `Bearer` auth-scheme from a header value, if present.
///
/// The scheme is case-insensitive per RFC 9110 §11.1. Bare tokens are returned
/// unchanged for hosts that strip the scheme themselves.
fn strip_bearer_prefix(value: &str) -> &str {
match value.split_once(' ') {
Some((scheme, after)) if scheme.eq_ignore_ascii_case("bearer") => {
after.trim_start_matches(' ')
},
_ => value,
}
}

/// Reason `validate_token` couldn't verify the JWT. Wraps the
/// usual `jsonwebtoken::errors::Error` plus the kid-selection
/// and JWKS-availability cases.
Expand Down Expand Up @@ -1124,6 +1151,17 @@ mod tests {
);
}

#[test]
fn strip_bearer_prefix_is_case_insensitive() {
assert_eq!(strip_bearer_prefix("Bearer abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("bearer abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("BEARER abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("BeArEr abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("bearer abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("abc.def.ghi"), "abc.def.ghi");
assert_eq!(strip_bearer_prefix("bearerish"), "bearerish");
}

#[test]
fn new_rejects_missing_config_block() {
let cfg = PluginConfig {
Expand Down
18 changes: 9 additions & 9 deletions builtins/session/valkey/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,16 @@ impl SessionStore for ValkeySessionStore {
// closed. A persistently-failing refresh risks silent key
// expiry across requests — see the operator runbook.
if let Some(ttl) = self.ttl_seconds {
let refresh: Result<bool, _> = match tokio::time::timeout(
self.command_timeout,
conn.expire(&key, ttl_for_expire(ttl)),
)
.await
{
Ok(res) => res,
Err(_) => Ok(false), // treat timeout as a failed refresh
// Timeouts raise the same refresh-failure alarm as backend errors.
let refresh: Result<Result<bool, _>, _> =
tokio::time::timeout(self.command_timeout, conn.expire(&key, ttl_for_expire(ttl)))
.await;
let refresh_error: Option<String> = match refresh {
Ok(Ok(_)) => None,
Ok(Err(e)) => Some(e.to_string()),
Err(_) => Some("EXPIRE timed out".to_owned()),
};
if let Err(e) = refresh {
if let Some(e) = refresh_error {
tracing::warn!(
alarm = "session_store_ttl_refresh_failed",
error = %e,
Expand Down
Loading