From b5b5681605bff371219d46e9be3d93878a38f200 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Mon, 26 Aug 2024 12:14:29 -0700 Subject: [PATCH] update for cedar#1147 (#425) Signed-off-by: Craig Disselkoen --- cedar-drt/fuzz/fuzz_targets/validation-pbt.rs | 4 ++ cedar-policy-generators/src/err.rs | 22 +++++++++- cedar-policy-generators/src/schema.rs | 44 ++++++------------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/cedar-drt/fuzz/fuzz_targets/validation-pbt.rs b/cedar-drt/fuzz/fuzz_targets/validation-pbt.rs index e60e387a8..299a15374 100644 --- a/cedar-drt/fuzz/fuzz_targets/validation-pbt.rs +++ b/cedar-drt/fuzz/fuzz_targets/validation-pbt.rs @@ -86,6 +86,7 @@ const LOG_FILENAME_ERR_CONTEXT: &str = "./logs/err_context.txt"; const LOG_FILENAME_ERR_INCORRECT_FORMAT: &str = "./logs/err_incorrect_format.txt"; const LOG_FILENAME_ERR_OTHER: &str = "./logs/err_other.txt"; const LOG_FILENAME_ENTITIES_ERROR: &str = "./logs/err_entities.txt"; +const LOG_FILENAME_SCHEMA_ERROR: &str = "./logs/err_schema.txt"; // In the below, "vyes" means the schema passed validation, while "vno" means we // got to the point of running the validator but validation failed @@ -117,6 +118,9 @@ fn log_err(res: Result, doing_what: &str) -> Result { Err(Error::EntitiesError(_)) => { checkpoint(LOG_FILENAME_ENTITIES_ERROR.to_string() + "_" + doing_what) } + Err(Error::SchemaError(_)) => { + checkpoint(LOG_FILENAME_SCHEMA_ERROR.to_string() + "_" + doing_what) + } Err(Error::NotEnoughData) => { checkpoint(LOG_FILENAME_ERR_NOT_ENOUGH_DATA.to_string() + "_" + doing_what) } diff --git a/cedar-policy-generators/src/err.rs b/cedar-policy-generators/src/err.rs index 87654ba5f..ad5669ca3 100644 --- a/cedar-policy-generators/src/err.rs +++ b/cedar-policy-generators/src/err.rs @@ -15,49 +15,66 @@ */ use cedar_policy_core::ast; +use thiserror::Error; /// Our Error type, which carries somewhat more information than /// `arbitrary::Error`, for the purposes of debugging and metrics -#[derive(Debug)] +#[derive(Debug, Error)] pub enum Error { /// Like `NotEnoughData` in `arbitrary::Error`, this represents that we ran /// out of bytes in the `Unstructured` + #[error("not enough data")] NotEnoughData, /// Like `EmptyChoose` in `arbitrary::Error`, this represents that we tried /// to `.choose()` (or equivalent) on an empty slice + #[error("while {doing_what}: `.choose()` (or equivalent) on an empty slice")] EmptyChoose { /// short string describing what we were doing when this happened doing_what: String, }, /// Tried to generate an expression deeper than allowed, and couldn't /// recover by putting some depth-0 expression in the leaf + #[error("recursion deeper than allowed, and we couldn't recover")] TooDeep, /// Generated schema ended up with no valid principal types, no valid /// resource types, or both + #[error("no valid principal types, or, no valid resource types")] NoValidPrincipalOrResourceTypes, /// Tried to generate something with an extension type, but extension types /// were disabled in settings + #[error( + "tried to generate something with an extension type, but extension types were disabled" + )] ExtensionsDisabled, /// Tried to generate something using the `like` operator, but the `like` /// operator was disabled in settings + #[error("tried to generate something using the `like` operator, but the `like` operator was disabled")] LikeDisabled, /// `IncorrectFormat` error that was generated by the `arbitrary` crate directly. /// We try to maintain the invariant that we don't generate these ourselves, /// preferring the more specific errors above + #[error("while {doing_what}: incorrect format")] IncorrectFormat { /// short string describing what we were doing when this happened doing_what: String, }, /// An error that occurs attempting to convert a hierarchy into an entities structure, /// or more generally an Entity as represented in this crate, into an `ast::Entity` + #[error("{0}")] EntitiesError(String), /// An error that occurs building an `ast::Context`, possibly due to errors /// in extension constructors. - ContextError(ast::ContextCreationError), + #[error(transparent)] + ContextError(#[from] ast::ContextCreationError), + /// Error thrown by the `cedar_policy_validator` crate while processing a + /// user-provided schema. + #[error(transparent)] + SchemaError(#[from] cedar_policy_validator::SchemaError), /// Error generated by the `arbitrary` crate, other than the three in /// `arbitrary::Error` as of this writing. This is necessary because /// `arbitrary::Error` is marked non-exhaustive. We don't generate these /// ourselves. + #[error(transparent)] OtherArbitrary(arbitrary::Error), } @@ -76,6 +93,7 @@ impl From for arbitrary::Error { Error::EntitiesError(_) => arbitrary::Error::IncorrectFormat, Error::IncorrectFormat { .. } => arbitrary::Error::IncorrectFormat, Error::ContextError(_) => arbitrary::Error::IncorrectFormat, + Error::SchemaError(_) => arbitrary::Error::IncorrectFormat, Error::OtherArbitrary(e) => e, } } diff --git a/cedar-policy-generators/src/schema.rs b/cedar-policy-generators/src/schema.rs index 47ba70ce2..4f4a8d716 100644 --- a/cedar-policy-generators/src/schema.rs +++ b/cedar-policy-generators/src/schema.rs @@ -31,7 +31,10 @@ use crate::{accum, gen, gen_inner, uniform}; use arbitrary::{self, Arbitrary, Unstructured}; use cedar_policy_core::ast::{self, Effect, PolicyID, UnreservedId}; use cedar_policy_core::extensions::Extensions; -use cedar_policy_validator::{json_schema, RawName, SchemaError, ValidatorSchema}; +use cedar_policy_validator::{ + json_schema, ActionBehavior, AllDefs, RawName, SchemaError, ValidatorNamespaceDef, + ValidatorSchema, ValidatorSchemaFragment, +}; use smol_str::{SmolStr, ToSmolStr}; use std::collections::BTreeMap; @@ -947,37 +950,18 @@ impl Schema { u: &mut Unstructured<'_>, ) -> Result { let namespace_internal: Option<&ast::InternalName> = namespace.as_ref().map(AsRef::as_ref); - let nsdef = nsdef.conditionally_qualify_type_references(namespace_internal); - let unreserved_to_def = |unreserved: UnreservedId| -> ast::InternalName { - RawName::new_from_unreserved(unreserved).qualify_with(namespace_internal) - }; - let action_id_to_def = |id: SmolStr| -> ast::EntityUID { - json_schema::ActionEntityUID::default_type(id) - .qualify_with(namespace_internal) - .try_into() - .unwrap() - }; - let all_common_defs = nsdef - .common_types - .keys() - .cloned() - .map(unreserved_to_def) - .collect(); - let all_entity_defs = nsdef - .entity_types - .keys() - .cloned() - .map(unreserved_to_def) - .collect(); - let all_action_defs = nsdef - .actions - .keys() - .cloned() - .map(action_id_to_def) - .collect(); + let all_defs = AllDefs::single_fragment(&ValidatorSchemaFragment::from_namespaces([ + ValidatorNamespaceDef::from_namespace_definition( + namespace.clone().map(Into::into), + nsdef.clone(), + ActionBehavior::PermitAttributes, + Extensions::all_available(), + )?, + ])); Self::from_nsdef( nsdef - .fully_qualify_type_references(&all_common_defs, &all_entity_defs, &all_action_defs) + .conditionally_qualify_type_references(namespace_internal) + .fully_qualify_type_references(&all_defs) .unwrap(), namespace, settings,