Skip to content

Commit

Permalink
Add function for checking type name collssion with builtin in DRT (3.…
Browse files Browse the repository at this point in the history
…4.x) (#1378)

Signed-off-by: John Kastner <[email protected]>
  • Loading branch information
john-h-kastner-aws authored Dec 17, 2024
1 parent 97f7939 commit 88d7993
Show file tree
Hide file tree
Showing 15 changed files with 34 additions and 38 deletions.
2 changes: 1 addition & 1 deletion cedar-policy-core/src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = Var> {
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-core/src/ast/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl Pattern {
}

#[cfg(test)]
pub mod test {
pub(crate) mod test {
use super::*;

impl std::ops::Add for Pattern {
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-core/src/ast/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = PrincipalOrResourceConstraint> {
Expand Down
10 changes: 5 additions & 5 deletions cedar-policy-core/src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ impl<'e> Evaluator<'e> {
}

#[cfg(test)]
pub fn interpret_inline_policy(&self, e: &Expr) -> Result<Value> {
pub(crate) fn interpret_inline_policy(&self, e: &Expr) -> Result<Value> {
match self.partial_interpret(e, &HashMap::new())? {
PartialValue::Value(v) => {
debug_assert!(e.source_loc().is_some() == v.source_loc().is_some());
Expand Down Expand Up @@ -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::*;
Expand All @@ -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),
Expand All @@ -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![
Expand All @@ -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"));
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-core/src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ pub enum ExtensionFunctionLookupError {
pub type Result<T> = std::result::Result<T, ExtensionFunctionLookupError>;

#[cfg(test)]
pub mod test {
pub(crate) mod test {
use super::*;
use std::collections::HashSet;

Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-validator/src/fuzzy_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl Validator {
&'a self,
p: &'a Policy,
mode: ValidationMode,
) -> Option<impl Iterator<Item = ValidationError> + 'a> {
) -> Option<impl Iterator<Item = ValidationError<'a>> + 'a> {
// Ignore static policies since they are already handled by `validate_policy`
if p.is_static() {
return None;
Expand Down
6 changes: 3 additions & 3 deletions cedar-policy-validator/src/rbac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl Validator {
pub(crate) fn validate_entity_types<'a>(
&'a self,
template: &'a Template,
) -> impl Iterator<Item = ValidationError> + 'a {
) -> impl Iterator<Item = ValidationError<'a>> + '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
Expand Down Expand Up @@ -89,7 +89,7 @@ impl Validator {
pub(crate) fn validate_action_ids<'a>(
&'a self,
template: &'a Template,
) -> impl Iterator<Item = ValidationError> + 'a {
) -> impl Iterator<Item = ValidationError<'a>> + 'a {
// Valid action id names that will be used to generate suggestions if an
// action id is not found
let known_action_ids = self
Expand Down Expand Up @@ -354,7 +354,7 @@ impl Validator {
pub(crate) fn get_apply_specs_for_action<'a>(
&'a self,
action_constraint: &'a ActionConstraint,
) -> impl Iterator<Item = &ValidatorApplySpec> + 'a {
) -> impl Iterator<Item = &'a ValidatorApplySpec> + 'a {
self.get_actions_satisfying_constraint(action_constraint)
// Get the action type if the id string exists, and then the
// applies_to list.
Expand Down
6 changes: 3 additions & 3 deletions cedar-policy-validator/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<impl Iterator<Item = &Name> + 'a> {
pub fn ancestors<'a>(&'a self, ty: &'a Name) -> Option<impl Iterator<Item = &'a Name> + 'a> {
if self.entity_types.contains_key(ty) {
Some(self.entity_types.values().filter_map(|ety| {
if ety.descendants.contains(ty) {
Expand Down Expand Up @@ -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
Expand All @@ -629,7 +629,7 @@ impl ValidatorSchema {
pub(crate) fn get_entity_types_in_set<'a>(
&'a self,
euids: impl IntoIterator<Item = &'a EntityUID> + 'a,
) -> impl Iterator<Item = &Name> {
) -> impl Iterator<Item = &'a Name> {
euids.into_iter().flat_map(|e| self.get_entity_types_in(e))
}

Expand Down
16 changes: 4 additions & 12 deletions cedar-policy-validator/src/schema/namespace_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@ 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},
is_builtin_type_name, schema_file_format,
types::{AttributeType, Attributes, OpenTag, Type},
ActionBehavior, ActionEntityUID, ActionType, NamespaceDefinition, SchemaType,
SchemaTypeVariant, TypeOfAttribute, SCHEMA_TYPE_VARIANT_TAGS,
SchemaTypeVariant, TypeOfAttribute,
};

/// 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
Expand Down Expand Up @@ -248,12 +246,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<Id, SchemaType>,
schema_namespace: Option<&Name>,
Expand All @@ -262,7 +254,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 =
Expand Down
6 changes: 5 additions & 1 deletion cedar-policy-validator/src/schema_file_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions cedar-policy-validator/src/typecheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
{
Expand Down Expand Up @@ -451,7 +451,7 @@ impl<'a> Typechecker<'a> {
&'b self,
env: RequestEnv<'b>,
t: &'b Template,
) -> Box<dyn Iterator<Item = RequestEnv> + 'b> {
) -> Box<dyn Iterator<Item = RequestEnv<'b>> + 'b> {
match env {
RequestEnv::UndeclaredAction => Box::new(std::iter::once(RequestEnv::UndeclaredAction)),
RequestEnv::DeclaredAction {
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ impl Schema {
pub fn ancestors<'a>(
&'a self,
ty: &'a EntityTypeName,
) -> Option<impl Iterator<Item = &EntityTypeName> + 'a> {
) -> Option<impl Iterator<Item = &'a EntityTypeName> + 'a> {
self.0
.ancestors(&ty.0)
.map(|iter| iter.map(RefCast::ref_cast))
Expand Down
3 changes: 1 addition & 2 deletions cedar-wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
3 changes: 2 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
unlicensed = "deny"
copyleft = "deny"
default = "deny"
unused-allowed-license = "deny"
unused-allowed-license = "allow"
confidence-threshold = 0.95
allow = [
"Apache-2.0",
"MIT",
"ISC",
"CC0-1.0",
"Unicode-DFS-2016",
"Unicode-3.0",
]

0 comments on commit 88d7993

Please sign in to comment.