diff --git a/cedar-drt/fuzz/src/schemas.rs b/cedar-drt/fuzz/src/schemas.rs index c2ec7239..cbc68c1b 100644 --- a/cedar-drt/fuzz/src/schemas.rs +++ b/cedar-drt/fuzz/src/schemas.rs @@ -88,6 +88,31 @@ impl<'a, T: Equiv> Equiv for &'a T { } } +impl Equiv for cedar_policy_core::est::Annotations { + fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + Equiv::equiv(&lhs.0, &rhs.0) + } +} + +impl Equiv for Option { + fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + match (lhs, rhs) { + (Some(a), Some(b)) => { + if a == b { + return Ok(()); + } + } + (Some(a), None) | (None, Some(a)) => { + if a.val.is_empty() { + return Ok(()); + } + } + (None, None) => return Ok(()), + }; + Err(format!("{lhs:?} and {rhs:?} are not equivalent")) + } +} + impl Equiv for json_schema::NamespaceDefinition { @@ -95,6 +120,8 @@ impl Equiv lhs: &json_schema::NamespaceDefinition, rhs: &json_schema::NamespaceDefinition, ) -> Result<(), String> { + Equiv::equiv(&lhs.annotations, &rhs.annotations) + .map_err(|e| format!("mismatch in namespace annotations: {e}"))?; Equiv::equiv(&lhs.entity_types, &rhs.entity_types) .map_err(|e| format!("mismatch in entity type declarations: {e}"))?; Equiv::equiv(&lhs.common_types, &rhs.common_types) @@ -191,12 +218,16 @@ impl Equiv for BTreeMap { impl Equiv for json_schema::CommonType { fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + Equiv::equiv(&lhs.annotations, &rhs.annotations) + .map_err(|e| format!("mismatch in common type annotations: {e}"))?; Equiv::equiv(&lhs.ty, &rhs.ty) } } impl Equiv for json_schema::EntityType { fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + Equiv::equiv(&lhs.annotations, &rhs.annotations) + .map_err(|e| format!("mismatch in entity annotations: {e}"))?; Equiv::equiv( &lhs.member_of_types.iter().collect::>(), &rhs.member_of_types.iter().collect::>(), @@ -226,6 +257,8 @@ impl Equiv for cedar_policy_validator::ValidatorEntityType { impl Equiv for json_schema::TypeOfAttribute { fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + Equiv::equiv(&lhs.annotations, &rhs.annotations) + .map_err(|e| format!("mismatch in type of attribute annotations: {e}"))?; if lhs.required != rhs.required { return Err("attributes differ in required flag".into()); } @@ -434,6 +467,8 @@ impl TypeName for InternalName { impl Equiv for json_schema::ActionType { fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> { + Equiv::equiv(&lhs.annotations, &rhs.annotations) + .map_err(|e| format!("mismatch in action annotations: {e}"))?; if &lhs.attributes != &rhs.attributes && !(lhs.attributes.as_ref().is_none_or(HashMap::is_empty) && rhs.attributes.as_ref().is_none_or(HashMap::is_empty)) @@ -517,3 +552,70 @@ impl Equiv for cedar_policy_validator::ValidatorSchema { Ok(()) } } + +#[cfg(test)] +mod tests { + use cedar_drt::est::Annotations; + + use crate::schemas::Equiv; + + #[test] + fn annotations() { + // positive cases + let pairs: [(Annotations, Annotations); 5] = [ + // value being null is equivalent to an empty string + ( + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + serde_json::from_value(serde_json::json!({"a": ""})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": ""})).unwrap(), + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + ), + // both values being null is also equivalent + ( + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + ), + // otherwise compare non-null values + ( + serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(), + serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": "🥨", "b": "🥯🍩"})).unwrap(), + serde_json::from_value(serde_json::json!({"b": "🥯🍩", "a": "🥨"})).unwrap(), + ), + ]; + pairs + .iter() + .for_each(|(a, b)| assert!(Equiv::equiv(a, b).is_ok())); + + // negative cases + let pairs: [(Annotations, Annotations); 5] = [ + ( + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + serde_json::from_value(serde_json::json!({"a": "🍪"})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": ""})).unwrap(), + serde_json::from_value(serde_json::json!({"b": null})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": null})).unwrap(), + serde_json::from_value(serde_json::json!({"b": null})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(), + serde_json::from_value(serde_json::json!({"a": "🍪"})).unwrap(), + ), + ( + serde_json::from_value(serde_json::json!({"a": "🥨", "b": "🥯🍪"})).unwrap(), + serde_json::from_value(serde_json::json!({"b": "🥯🍩", "a": "🥨"})).unwrap(), + ), + ]; + pairs + .iter() + .for_each(|(a, b)| assert!(Equiv::equiv(a, b).is_err())); + } +} diff --git a/cedar-policy-generators/src/policy.rs b/cedar-policy-generators/src/policy.rs index d1e45418..11cd7741 100644 --- a/cedar-policy-generators/src/policy.rs +++ b/cedar-policy-generators/src/policy.rs @@ -20,12 +20,10 @@ use crate::hierarchy::Hierarchy; use crate::size_hint_utils::size_hint_for_ratio; use arbitrary::{Arbitrary, Unstructured}; use cedar_policy_core::ast::{ - Annotation, Annotations, AnyId, Effect, EntityUID, Expr, Policy, PolicyID, PolicySet, - StaticPolicy, Template, + Effect, EntityUID, Expr, Policy, PolicyID, PolicySet, StaticPolicy, Template, }; use cedar_policy_core::{ast, est}; use serde::Serialize; -use smol_str::SmolStr; use std::fmt::Display; use std::sync::Arc; @@ -39,7 +37,7 @@ use std::sync::Arc; #[serde(into = "est::Policy")] pub struct GeneratedPolicy { id: PolicyID, - annotations: HashMap, + annotations: cedar_policy_core::est::Annotations, effect: Effect, principal_constraint: PrincipalOrResourceConstraint, action_constraint: ActionConstraint, @@ -59,7 +57,7 @@ impl GeneratedPolicy { /// Create a new `GeneratedPolicy` with these fields pub fn new( id: PolicyID, - annotations: impl IntoIterator, + annotations: cedar_policy_core::est::Annotations, effect: Effect, principal_constraint: PrincipalOrResourceConstraint, action_constraint: ActionConstraint, @@ -68,7 +66,7 @@ impl GeneratedPolicy { ) -> Self { Self { id, - annotations: annotations.into_iter().collect(), + annotations, effect, principal_constraint, action_constraint, @@ -141,19 +139,12 @@ impl GeneratedPolicy { } } -fn convert_annotations(annotations: HashMap) -> Annotations { - annotations - .into_iter() - .map(|(k, v)| (k, Annotation { val: v, loc: None })) - .collect() -} - impl From for StaticPolicy { fn from(gen: GeneratedPolicy) -> StaticPolicy { StaticPolicy::new( gen.id, None, - convert_annotations(gen.annotations), + gen.annotations.into(), gen.effect, gen.principal_constraint.into(), gen.action_constraint.into(), @@ -169,7 +160,7 @@ impl From for Template { Template::new( gen.id, None, - convert_annotations(gen.annotations), + gen.annotations.into(), gen.effect, gen.principal_constraint.into(), gen.action_constraint.into(), diff --git a/cedar-policy-generators/src/schema.rs b/cedar-policy-generators/src/schema.rs index 30fae4a5..821a1e41 100644 --- a/cedar-policy-generators/src/schema.rs +++ b/cedar-policy-generators/src/schema.rs @@ -30,7 +30,6 @@ use crate::size_hint_utils::{size_hint_for_choose, size_hint_for_range, size_hin use crate::{accum, gen, gen_inner, uniform}; use arbitrary::{self, Arbitrary, MaxRecursionReached, Unstructured}; use cedar_policy_core::ast::{self, Effect, PolicyID, UnreservedId}; -use cedar_policy_core::est::Annotations; use cedar_policy_core::extensions::Extensions; use cedar_policy_validator::json_schema::{CommonType, CommonTypeId}; use cedar_policy_validator::{ @@ -153,15 +152,16 @@ fn arbitrary_typeofattribute_with_bounded_depth>( Ok(json_schema::TypeOfAttribute { ty: arbitrary_schematype_with_bounded_depth::(settings, entity_types, max_depth, u)?, required: u.arbitrary()?, - annotations: Annotations::new(), + annotations: u.arbitrary()?, }) } /// size hint for arbitrary_typeofattribute_with_bounded_depth fn arbitrary_typeofattribute_size_hint(depth: usize) -> (usize, Option) { - arbitrary::size_hint::and( + arbitrary::size_hint::and_all(&[ arbitrary_schematype_size_hint(depth), ::size_hint(depth), - ) + ::size_hint(depth), + ]) } /// internal helper function, an alternative to the `Arbitrary` impl for @@ -710,15 +710,15 @@ impl Schema { common_types: common_types .into_iter() .map(|(id, ty)| { - ( + Ok(( id, CommonType { ty, - annotations: Annotations::new(), + annotations: u.arbitrary()?, }, - ) + )) }) - .collect(), + .collect::>>()?, entity_types: entity_types.into(), actions: actions.into(), annotations: self.schema.annotations.clone(), @@ -931,7 +931,7 @@ impl Schema { u )?) ), - annotations: Annotations::new(), + annotations: u.arbitrary()?, }, )) }) @@ -1034,7 +1034,7 @@ impl Schema { }, //TODO: Fuzz arbitrary attribute names and values. attributes: None, - annotations: Annotations::new(), + annotations: u.arbitrary()?, }, )) }) @@ -1064,7 +1064,7 @@ impl Schema { common_types: BTreeMap::new().into(), entity_types: entity_types.into_iter().collect(), actions: actions.into_iter().collect(), - annotations: Annotations::new(), + annotations: u.arbitrary()?, }; let attrsorcontexts /* : impl Iterator */ = nsdef.entity_types.values().map(|et| attrs_from_attrs_or_context(&nsdef, &et.shape)) .chain(nsdef.actions.iter().filter_map(|(_, action)| action.applies_to.as_ref()).map(|a| attrs_from_attrs_or_context(&nsdef, &a.context))); @@ -1361,7 +1361,6 @@ impl Schema { u: &mut Unstructured<'_>, ) -> Result { let id = u.arbitrary()?; - let annotations: HashMap = u.arbitrary()?; let effect = u.arbitrary()?; let principal_constraint = self.arbitrary_principal_constraint(hierarchy, u)?; let action_constraint = self.arbitrary_action_constraint(u, Some(3))?; @@ -1386,7 +1385,7 @@ impl Schema { } Ok(ABACPolicy(GeneratedPolicy::new( id, - annotations, + u.arbitrary()?, effect, principal_constraint, action_constraint,