From d0acd85743ca052f19da9eecb4556032ed112cdf Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 17 Dec 2024 16:00:00 +0000 Subject: [PATCH 1/7] Expose function to check for type name collision in DRT Signed-off-by: John Kastner --- cedar-policy-validator/src/schema/namespace_def.rs | 12 +++--------- cedar-policy-validator/src/schema_file_format.rs | 6 +++++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cedar-policy-validator/src/schema/namespace_def.rs b/cedar-policy-validator/src/schema/namespace_def.rs index 453c9f1da..dedfb29b0 100644 --- a/cedar-policy-validator/src/schema/namespace_def.rs +++ b/cedar-policy-validator/src/schema/namespace_def.rs @@ -32,14 +32,14 @@ use cedar_policy_core::{ use smol_str::{SmolStr, ToSmolStr}; use super::ValidatorApplySpec; -use crate::types::OpenTag; use crate::{ err::*, schema_file_format, types::{AttributeType, Attributes, Type}, ActionBehavior, ActionEntityUID, ActionType, NamespaceDefinition, SchemaType, - SchemaTypeVariant, TypeOfAttribute, SCHEMA_TYPE_VARIANT_TAGS, + SchemaTypeVariant, TypeOfAttribute, }; +use crate::{is_builtin_type_name, types::OpenTag}; /// The current schema format specification does not include multiple action entity /// types. All action entities are required to use a single `Action` entity @@ -248,12 +248,6 @@ impl ValidatorNamespaceDef { }) } - fn is_builtin_type_name(name: &SmolStr) -> bool { - SCHEMA_TYPE_VARIANT_TAGS - .iter() - .any(|type_name| name == type_name) - } - fn build_type_defs( schema_file_type_def: HashMap, schema_namespace: Option<&Name>, @@ -262,7 +256,7 @@ impl ValidatorNamespaceDef { .into_iter() .map(|(name, schema_ty)| -> Result<_> { let name_str = name.clone().into_smolstr(); - if Self::is_builtin_type_name(&name_str) { + if is_builtin_type_name(&name_str) { return Err(SchemaError::DuplicateCommonType(name_str.to_string())); } let name = diff --git a/cedar-policy-validator/src/schema_file_format.rs b/cedar-policy-validator/src/schema_file_format.rs index b5e8655ad..06ca6f66f 100644 --- a/cedar-policy-validator/src/schema_file_format.rs +++ b/cedar-policy-validator/src/schema_file_format.rs @@ -688,7 +688,7 @@ fn is_partial_schema_default(b: &bool) -> bool { // do this automatically, but it returns an empty slice for the variants names // of `SchemaTypeVariant`. // https://docs.rs/serde-aux/latest/serde_aux/serde_introspection/fn.serde_introspect.html -pub(crate) static SCHEMA_TYPE_VARIANT_TAGS: &[&str] = &[ +static SCHEMA_TYPE_VARIANT_TAGS: &[&str] = &[ "String", "Long", "Boolean", @@ -698,6 +698,10 @@ pub(crate) static SCHEMA_TYPE_VARIANT_TAGS: &[&str] = &[ "Extension", ]; +pub fn is_builtin_type_name(name: &str) -> bool { + SCHEMA_TYPE_VARIANT_TAGS.contains(&name) +} + impl SchemaType { /// Is this `SchemaType` an extension type, or does it contain one /// (recursively)? Returns `None` if this is a `TypeDef` because we can't From e21befad213bb972bfc9f24e3d50c9f670746f6c Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 17 Dec 2024 19:42:49 +0000 Subject: [PATCH 2/7] add elided lifetimes Signed-off-by: John Kastner --- cedar-policy-validator/src/lib.rs | 2 +- cedar-policy-validator/src/rbac.rs | 6 +++--- cedar-policy-validator/src/schema.rs | 6 +++--- cedar-policy-validator/src/typecheck.rs | 8 ++++---- cedar-policy/src/api.rs | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cedar-policy-validator/src/lib.rs b/cedar-policy-validator/src/lib.rs index 5c1d52001..0787122bc 100644 --- a/cedar-policy-validator/src/lib.rs +++ b/cedar-policy-validator/src/lib.rs @@ -154,7 +154,7 @@ impl Validator { &'a self, p: &'a Policy, mode: ValidationMode, - ) -> Option + 'a> { + ) -> Option> + 'a> { // Ignore static policies since they are already handled by `validate_policy` if p.is_static() { return None; diff --git a/cedar-policy-validator/src/rbac.rs b/cedar-policy-validator/src/rbac.rs index 34e0365c3..ae452a178 100644 --- a/cedar-policy-validator/src/rbac.rs +++ b/cedar-policy-validator/src/rbac.rs @@ -38,7 +38,7 @@ impl Validator { pub(crate) fn validate_entity_types<'a>( &'a self, template: &'a Template, - ) -> impl Iterator + 'a { + ) -> impl Iterator> + 'a { // All valid entity types in the schema. These will be used to generate // suggestion when an entity type is not found. let known_entity_types = self @@ -89,7 +89,7 @@ impl Validator { pub(crate) fn validate_action_ids<'a>( &'a self, template: &'a Template, - ) -> impl Iterator + 'a { + ) -> impl Iterator> + 'a { // Valid action id names that will be used to generate suggestions if an // action id is not found let known_action_ids = self @@ -354,7 +354,7 @@ impl Validator { pub(crate) fn get_apply_specs_for_action<'a>( &'a self, action_constraint: &'a ActionConstraint, - ) -> impl Iterator + 'a { + ) -> impl Iterator + 'a { self.get_actions_satisfying_constraint(action_constraint) // Get the action type if the id string exists, and then the // applies_to list. diff --git a/cedar-policy-validator/src/schema.rs b/cedar-policy-validator/src/schema.rs index dd9371218..be88697f6 100644 --- a/cedar-policy-validator/src/schema.rs +++ b/cedar-policy-validator/src/schema.rs @@ -196,7 +196,7 @@ impl ValidatorSchema { /// # Errors /// /// Returns [`None`] if the `ty` is not found in the schema - pub fn ancestors<'a>(&'a self, ty: &'a Name) -> Option + 'a> { + pub fn ancestors<'a>(&'a self, ty: &'a Name) -> Option + 'a> { if self.entity_types.contains_key(ty) { Some(self.entity_types.values().filter_map(|ety| { if ety.descendants.contains(ty) { @@ -609,7 +609,7 @@ impl ValidatorSchema { /// includes all entity types that are descendants of the type of `entity` /// according to the schema, and the type of `entity` itself because /// `entity in entity` evaluates to `true`. - pub(crate) fn get_entity_types_in<'a>(&'a self, entity: &'a EntityUID) -> Vec<&Name> { + pub(crate) fn get_entity_types_in<'a>(&'a self, entity: &'a EntityUID) -> Vec<&'a Name> { match entity.entity_type() { EntityType::Specified(ety) => { let mut descendants = self @@ -629,7 +629,7 @@ impl ValidatorSchema { pub(crate) fn get_entity_types_in_set<'a>( &'a self, euids: impl IntoIterator + 'a, - ) -> impl Iterator { + ) -> impl Iterator { euids.into_iter().flat_map(|e| self.get_entity_types_in(e)) } diff --git a/cedar-policy-validator/src/typecheck.rs b/cedar-policy-validator/src/typecheck.rs index 82c979fc6..62a36f250 100644 --- a/cedar-policy-validator/src/typecheck.rs +++ b/cedar-policy-validator/src/typecheck.rs @@ -316,7 +316,7 @@ impl<'a> Typechecker<'a> { pub fn typecheck_by_request_env<'b>( &'b self, t: &'b Template, - ) -> Vec<(RequestEnv, PolicyCheck)> { + ) -> Vec<(RequestEnv<'b>, PolicyCheck)> { self.apply_typecheck_fn_by_request_env(t, |request, expr| { let mut type_errors = Vec::new(); let empty_prior_eff = EffectSet::new(); @@ -345,7 +345,7 @@ impl<'a> Typechecker<'a> { &'b self, t: &'b Template, typecheck_fn: F, - ) -> Vec<(RequestEnv, C)> + ) -> Vec<(RequestEnv<'b>, C)> where F: Fn(&RequestEnv, &Expr) -> C, { @@ -451,7 +451,7 @@ impl<'a> Typechecker<'a> { &'b self, env: RequestEnv<'b>, t: &'b Template, - ) -> Box + 'b> { + ) -> Box> + 'b> { match env { RequestEnv::UndeclaredAction => Box::new(std::iter::once(RequestEnv::UndeclaredAction)), RequestEnv::DeclaredAction { @@ -2450,7 +2450,7 @@ impl<'a> Typechecker<'a> { fn lookup_extension_function<'b>( &'b self, f: &'b Name, - ) -> Result<&ExtensionFunctionType, impl FnOnce(Expr) -> TypeError + 'b> { + ) -> Result<&'b ExtensionFunctionType, impl FnOnce(Expr) -> TypeError + 'b> { let extension_funcs: Vec<&ExtensionFunctionType> = self .extensions .iter() diff --git a/cedar-policy/src/api.rs b/cedar-policy/src/api.rs index e5cee016f..8f3a72489 100644 --- a/cedar-policy/src/api.rs +++ b/cedar-policy/src/api.rs @@ -1710,7 +1710,7 @@ impl Schema { pub fn ancestors<'a>( &'a self, ty: &'a EntityTypeName, - ) -> Option + 'a> { + ) -> Option + 'a> { self.0 .ancestors(&ty.0) .map(|iter| iter.map(RefCast::ref_cast)) From 3c9f2ebae42a64c363c0f9503d801f720fc93ef5 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 17 Dec 2024 17:48:59 +0000 Subject: [PATCH 3/7] fix warnings due to `pub` functions in `#[cfg(test)]` Signed-off-by: John Kastner --- cedar-policy-core/src/ast/expr.rs | 2 +- cedar-policy-core/src/ast/pattern.rs | 2 +- cedar-policy-core/src/ast/policy.rs | 2 +- cedar-policy-core/src/evaluator.rs | 10 +++++----- cedar-policy-core/src/extensions.rs | 2 +- cedar-policy-validator/src/fuzzy_match.rs | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cedar-policy-core/src/ast/expr.rs b/cedar-policy-core/src/ast/expr.rs index 2d2483f46..b3e2db287 100644 --- a/cedar-policy-core/src/ast/expr.rs +++ b/cedar-policy-core/src/ast/expr.rs @@ -1426,7 +1426,7 @@ pub enum Var { } #[cfg(test)] -pub mod var_generator { +pub(crate) mod var_generator { use super::Var; #[cfg(test)] pub fn all_vars() -> impl Iterator { diff --git a/cedar-policy-core/src/ast/pattern.rs b/cedar-policy-core/src/ast/pattern.rs index 0b93fefd3..56fc2324f 100644 --- a/cedar-policy-core/src/ast/pattern.rs +++ b/cedar-policy-core/src/ast/pattern.rs @@ -163,7 +163,7 @@ impl Pattern { } #[cfg(test)] -pub mod test { +pub(crate) mod test { use super::*; impl std::ops::Add for Pattern { diff --git a/cedar-policy-core/src/ast/policy.rs b/cedar-policy-core/src/ast/policy.rs index ac1e2c593..f0148cbfa 100644 --- a/cedar-policy-core/src/ast/policy.rs +++ b/cedar-policy-core/src/ast/policy.rs @@ -1810,7 +1810,7 @@ impl<'a> Iterator for EntityIterator<'a> { } #[cfg(test)] -pub mod test_generators { +pub(crate) mod test_generators { use super::*; pub fn all_por_constraints() -> impl Iterator { diff --git a/cedar-policy-core/src/evaluator.rs b/cedar-policy-core/src/evaluator.rs index bb1345655..f0e5dc801 100644 --- a/cedar-policy-core/src/evaluator.rs +++ b/cedar-policy-core/src/evaluator.rs @@ -785,7 +785,7 @@ impl<'e> Evaluator<'e> { } #[cfg(test)] - pub fn interpret_inline_policy(&self, e: &Expr) -> Result { + pub(crate) fn interpret_inline_policy(&self, e: &Expr) -> Result { match self.partial_interpret(e, &HashMap::new())? { PartialValue::Value(v) => { debug_assert!(e.source_loc().is_some() == v.source_loc().is_some()); @@ -887,7 +887,7 @@ fn stack_size_check() -> Result<()> { // PANIC SAFETY: Unit Test Code #[allow(clippy::panic)] #[cfg(test)] -pub mod test { +pub(crate) mod test { use std::str::FromStr; use super::*; @@ -899,7 +899,7 @@ pub mod test { use cool_asserts::assert_matches; - // Many of these tests use this Request + /// Many of these tests use this Request pub fn basic_request() -> Request { Request::new( (EntityUID::with_eid("test_principal"), None), @@ -926,7 +926,7 @@ pub mod test { .unwrap() } - // Many of these tests use this basic `Entities` + /// Many of these tests use this basic `Entities` pub fn basic_entities() -> Entities { Entities::from_entities( vec![ @@ -942,7 +942,7 @@ pub mod test { .expect("failed to create basic entities") } - // This `Entities` has richer Entities + /// This `Entities` has richer Entities pub fn rich_entities() -> Entities { let entity_no_attrs_no_parents = Entity::with_uid(EntityUID::with_eid("entity_no_attrs_no_parents")); diff --git a/cedar-policy-core/src/extensions.rs b/cedar-policy-core/src/extensions.rs index 2789cb3a3..2248f5ec7 100644 --- a/cedar-policy-core/src/extensions.rs +++ b/cedar-policy-core/src/extensions.rs @@ -178,7 +178,7 @@ pub enum ExtensionFunctionLookupError { pub type Result = std::result::Result; #[cfg(test)] -pub mod test { +pub(crate) mod test { use super::*; use std::collections::HashSet; diff --git a/cedar-policy-validator/src/fuzzy_match.rs b/cedar-policy-validator/src/fuzzy_match.rs index 0adf0e390..3f9b09f2a 100644 --- a/cedar-policy-validator/src/fuzzy_match.rs +++ b/cedar-policy-validator/src/fuzzy_match.rs @@ -75,7 +75,7 @@ pub fn levenshtein_distance(word1: &str, word2: &str) -> usize { } #[cfg(test)] -pub mod test { +mod test { use super::*; ///the key differs by 1 letter from a word in words From 0aab5fa9db917ace63b3879f59ca66f3e6304b5f Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 17 Dec 2024 20:02:30 +0000 Subject: [PATCH 4/7] Update url dependecy to match `main` Signed-off-by: John Kastner --- cedar-wasm/Cargo.toml | 3 +-- deny.toml | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cedar-wasm/Cargo.toml b/cedar-wasm/Cargo.toml index cf6d61e79..ca645fe81 100644 --- a/cedar-wasm/Cargo.toml +++ b/cedar-wasm/Cargo.toml @@ -39,6 +39,5 @@ cool_asserts = "2.0" [build-dependencies] cargo-lock = "9.0.0" -# Lock `url` (dependencies of cargo-lock) to 2.5.0 until we know if Unicode 3.0 license is ok. -url = "=2.5.0" +url = "2.5.4" itertools = "0.13.0" diff --git a/deny.toml b/deny.toml index a96ed2c9a..b7683d635 100644 --- a/deny.toml +++ b/deny.toml @@ -2,7 +2,7 @@ unlicensed = "deny" copyleft = "deny" default = "deny" -unused-allowed-license = "deny" +unused-allowed-license = "allow" confidence-threshold = 0.95 allow = [ "Apache-2.0", @@ -10,4 +10,5 @@ allow = [ "ISC", "CC0-1.0", "Unicode-DFS-2016", + "Unicode-3.0", ] From 61b3fa9b5e8fd7727713b4f1fb265d0b2ee9a065 Mon Sep 17 00:00:00 2001 From: John Kastner <130772734+john-h-kastner-aws@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:45:40 -0500 Subject: [PATCH 5/7] Update namespace_def.rs --- cedar-policy-validator/src/schema/namespace_def.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cedar-policy-validator/src/schema/namespace_def.rs b/cedar-policy-validator/src/schema/namespace_def.rs index dedfb29b0..d31f5f482 100644 --- a/cedar-policy-validator/src/schema/namespace_def.rs +++ b/cedar-policy-validator/src/schema/namespace_def.rs @@ -38,9 +38,8 @@ use crate::{ types::{AttributeType, Attributes, Type}, ActionBehavior, ActionEntityUID, ActionType, NamespaceDefinition, SchemaType, SchemaTypeVariant, TypeOfAttribute, + is_builtin_type_name, types::OpenTag, }; -use crate::{is_builtin_type_name, types::OpenTag}; - /// The current schema format specification does not include multiple action entity /// types. All action entities are required to use a single `Action` entity /// type. However, the action entity type may be namespaced, so an action entity From 17581d80377d04a83eec695e318b71689f8de80a Mon Sep 17 00:00:00 2001 From: John Kastner <130772734+john-h-kastner-aws@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:46:01 -0500 Subject: [PATCH 6/7] Update namespace_def.rs --- cedar-policy-validator/src/schema/namespace_def.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cedar-policy-validator/src/schema/namespace_def.rs b/cedar-policy-validator/src/schema/namespace_def.rs index d31f5f482..ef0b1a705 100644 --- a/cedar-policy-validator/src/schema/namespace_def.rs +++ b/cedar-policy-validator/src/schema/namespace_def.rs @@ -35,10 +35,10 @@ use super::ValidatorApplySpec; use crate::{ err::*, schema_file_format, - types::{AttributeType, Attributes, Type}, + types::{AttributeType, Attributes, Type, OpenTag}, ActionBehavior, ActionEntityUID, ActionType, NamespaceDefinition, SchemaType, SchemaTypeVariant, TypeOfAttribute, - is_builtin_type_name, types::OpenTag, + is_builtin_type_name, }; /// The current schema format specification does not include multiple action entity /// types. All action entities are required to use a single `Action` entity From 83f7dd9adeb71f9f0db172675d0c04555390eece Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 17 Dec 2024 20:46:56 +0000 Subject: [PATCH 7/7] fmt Signed-off-by: John Kastner --- cedar-policy-validator/src/schema/namespace_def.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cedar-policy-validator/src/schema/namespace_def.rs b/cedar-policy-validator/src/schema/namespace_def.rs index ef0b1a705..915649dc6 100644 --- a/cedar-policy-validator/src/schema/namespace_def.rs +++ b/cedar-policy-validator/src/schema/namespace_def.rs @@ -34,11 +34,10 @@ use smol_str::{SmolStr, ToSmolStr}; use super::ValidatorApplySpec; use crate::{ err::*, - schema_file_format, - types::{AttributeType, Attributes, Type, OpenTag}, + is_builtin_type_name, schema_file_format, + types::{AttributeType, Attributes, OpenTag, Type}, ActionBehavior, ActionEntityUID, ActionType, NamespaceDefinition, SchemaType, SchemaTypeVariant, TypeOfAttribute, - is_builtin_type_name, }; /// The current schema format specification does not include multiple action entity /// types. All action entities are required to use a single `Action` entity