Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function for checking type name collssion with builtin in DRT (3.4.x) #1378

Merged
merged 7 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
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
12 changes: 3 additions & 9 deletions cedar-policy-validator/src/schema/namespace_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
john-h-kastner-aws marked this conversation as resolved.
Show resolved Hide resolved

/// The current schema format specification does not include multiple action entity
/// types. All action entities are required to use a single `Action` entity
Expand Down Expand Up @@ -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<Id, SchemaType>,
schema_namespace: Option<&Name>,
Expand All @@ -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 =
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",
]
Loading