Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 5 additions & 0 deletions cedar-policy-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ serde_json = "1.0"
miette = { version = "5.9.0", features = ["fancy"] }
thiserror = "1.0"

[features]
default = []
experimental = ["partial-validate"]
partial-validate = ["cedar-policy/partial-validate"]

[dev-dependencies]
assert_cmd = "2.0"
tempfile = "3"
Expand Down
19 changes: 18 additions & 1 deletion cedar-policy-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ pub struct ValidateArgs {
/// Report a validation failure for non-fatal warnings
#[arg(long)]
pub deny_warnings: bool,
/// Validate the policy using partial schema validation. This option is
/// experimental and will cause the CLI to exit if it was not built with the
/// experimental `partial-validate` feature enabled.
#[arg(long = "partial-validate")]
pub partial_validate: bool,
}

#[derive(Args, Debug)]
Expand Down Expand Up @@ -432,6 +437,18 @@ pub fn check_parse(args: &CheckParseArgs) -> CedarExitCode {
}

pub fn validate(args: &ValidateArgs) -> CedarExitCode {
let mode = if args.partial_validate {
#[cfg(not(feature = "partial-validate"))]
{
println!("Error: arguments include the experimental option `--partial-validate`, but this executable was not built with `partial-validate` experimental feature enabled");
return CedarExitCode::Failure;
}
#[cfg(feature = "partial-validate")]
ValidationMode::Partial
} else {
ValidationMode::default()
};

let pset = match read_policy_set(Some(&args.policies_file)) {
Ok(pset) => pset,
Err(e) => {
Expand All @@ -449,7 +466,7 @@ pub fn validate(args: &ValidateArgs) -> CedarExitCode {
};

let validator = Validator::new(schema);
let result = validator.validate(&pset, ValidationMode::default());
let result = validator.validate(&pset, mode);

let exit_code = if !result.validation_passed()
|| (args.deny_warnings && !result.validation_passed_without_warnings())
Expand Down
1 change: 1 addition & 0 deletions cedar-policy-cli/tests/sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ fn run_validate_test(policies_file: &str, schema_file: &str, exit_code: CedarExi
schema_file: schema_file.into(),
policies_file: policies_file.into(),
deny_warnings: false,
partial_validate: false,
};
let output = validate(&cmd);
assert_eq!(exit_code, output, "{:#?}", cmd);
Expand Down
11 changes: 11 additions & 0 deletions cedar-policy-core/src/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1767,11 +1767,13 @@ mod schema_based_parsing_tests {
)]
.into_iter()
.collect(),
open_attrs: false,
}),
),
]
.into_iter()
.collect(),
open_attrs: false,
}),
"home_ip" => Some(SchemaType::Extension {
name: Name::parse_unqualified_name("ipaddr").expect("valid"),
Expand All @@ -1789,6 +1791,7 @@ mod schema_based_parsing_tests {
]
.into_iter()
.collect(),
open_attrs: false,
}),
_ => None,
}
Expand All @@ -1815,6 +1818,10 @@ mod schema_based_parsing_tests {
fn allowed_parent_types(&self) -> Arc<HashSet<EntityType>> {
Arc::new(HashSet::new())
}

fn open_attributes(&self) -> bool {
false
}
}

#[cfg(all(feature = "decimal", feature = "ipaddr"))]
Expand Down Expand Up @@ -2878,6 +2885,10 @@ mod schema_based_parsing_tests {
fn allowed_parent_types(&self) -> Arc<HashSet<EntityType>> {
Arc::new(HashSet::new())
}

fn open_attributes(&self) -> bool {
false
}
}

let entitiesjson = json!(
Expand Down
10 changes: 6 additions & 4 deletions cedar-policy-core/src/entities/conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,12 @@ impl<'a, S: Schema> EntitySchemaConformanceChecker<'a, S> {
None => {
// `None` indicates the attribute shouldn't exist -- see
// docs on the `attr_type()` trait method
return Err(EntitySchemaConformanceError::UnexpectedEntityAttr {
uid: uid.clone(),
attr: attr.clone(),
});
if !schema_etype.open_attributes() {
return Err(EntitySchemaConformanceError::UnexpectedEntityAttr {
uid: uid.clone(),
attr: attr.clone(),
});
}
}
Some(expected_ty) => {
// typecheck: ensure that the entity attribute value matches
Expand Down
1 change: 1 addition & 0 deletions cedar-policy-core/src/entities/json/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ impl ContextSchema for NullContextSchema {
fn context_type(&self) -> SchemaType {
SchemaType::Record {
attrs: HashMap::new(),
open_attrs: false,
}
}
}
Expand Down
21 changes: 15 additions & 6 deletions cedar-policy-core/src/entities/json/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,21 @@ impl<'e, 's, S: Schema> EntityJsonParser<'e, 's, S> {
// `None` indicates the attribute shouldn't exist -- see
// docs on the `attr_type()` trait method
None => {
return Err(JsonDeserializationError::EntitySchemaConformance(
EntitySchemaConformanceError::UnexpectedEntityAttr {
uid: uid.clone(),
attr: k,
},
))
if desc.open_attributes() {
vparser.val_into_restricted_expr(v.into(), None, || {
JsonDeserializationErrorContext::EntityAttribute {
uid: uid.clone(),
attr: k.clone(),
}
})?
} else {
return Err(JsonDeserializationError::EntitySchemaConformance(
EntitySchemaConformanceError::UnexpectedEntityAttr {
uid: uid.clone(),
attr: k,
},
));
}
}
Some(expected_ty) => vparser.val_into_restricted_expr(
v.into(),
Expand Down
7 changes: 7 additions & 0 deletions cedar-policy-core/src/entities/json/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ pub trait EntityTypeDescription {

/// Get the entity types which are allowed to be parents of this entity type.
fn allowed_parent_types(&self) -> Arc<HashSet<EntityType>>;

/// May entities with this type have attributes other than those specified
/// in the schema
fn open_attributes(&self) -> bool;
}

/// Simple type that implements `EntityTypeDescription` by expecting no
Expand All @@ -132,4 +136,7 @@ impl EntityTypeDescription for NullEntityTypeDescription {
fn allowed_parent_types(&self) -> Arc<HashSet<EntityType>> {
Arc::new(HashSet::new())
}
fn open_attributes(&self) -> bool {
false
}
}
41 changes: 30 additions & 11 deletions cedar-policy-core/src/entities/json/schema_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub enum SchemaType {
Record {
/// Attributes and their types
attrs: HashMap<SmolStr, AttributeType>,
/// Can a record with this type have attributes other than those specified in `attrs`
open_attrs: bool,
},
/// Entity
Entity {
Expand Down Expand Up @@ -119,7 +121,16 @@ impl SchemaType {
(Set { element_ty: elty1 }, Set { element_ty: elty2 }) => {
elty1.is_consistent_with(elty2)
}
(Record { attrs: attrs1 }, Record { attrs: attrs2 }) => {
(
Record {
attrs: attrs1,
open_attrs: open1,
},
Record {
attrs: attrs2,
open_attrs: open2,
},
) => {
attrs1.iter().all(|(k, v)| {
match attrs2.get(k) {
Some(ty) => {
Expand All @@ -129,9 +140,9 @@ impl SchemaType {
}
None => {
// attrs1 has the attribute, attrs2 does not.
// if required in attrs1, incompatible.
// otherwise fine
!v.required
// if required in attrs1 and and attrs2 is
// closed, incompatible. otherwise fine
!v.required || *open2
}
}
}) && attrs2.iter().all(|(k, v)| {
Expand All @@ -143,9 +154,9 @@ impl SchemaType {
}
None => {
// attrs2 has the attribute, attrs1 does not.
// if required in attrs2, incompatible.
// otherwise fine
!v.required
// if required in attrs2 and attrs1 is closed,
// incompatible. otherwise fine
!v.required || *open1
}
}
})
Expand Down Expand Up @@ -192,11 +203,17 @@ impl std::fmt::Display for SchemaType {
Self::String => write!(f, "string"),
Self::Set { element_ty } => write!(f, "(set of {})", &element_ty),
Self::EmptySet => write!(f, "empty-set"),
Self::Record { attrs } => {
if attrs.is_empty() {
Self::Record { attrs, open_attrs } => {
if attrs.is_empty() && *open_attrs {
write!(f, "any record")
} else if attrs.is_empty() {
write!(f, "empty record")
} else {
write!(f, "record with attributes: {{")?;
if *open_attrs {
write!(f, "record with at least attributes: {{")?;
} else {
write!(f, "record with attributes: {{")?;
}
// sorting attributes ensures that there is a single, deterministic
// Display output for each `SchemaType`, which is important for
// tests that check equality of error messages
Expand Down Expand Up @@ -324,7 +341,8 @@ pub fn schematype_of_restricted_expr(
// but marking it optional is more flexible -- allows the
// attribute type to `is_consistent_with()` more types
Ok((k.clone(), AttributeType::optional(attr_type)))
}).collect::<Result<HashMap<_,_>, GetSchemaTypeError>>()?
}).collect::<Result<HashMap<_,_>, GetSchemaTypeError>>()?,
open_attrs: false,
})
}
ExprKind::ExtensionFunctionApp { fn_name, .. } => {
Expand Down Expand Up @@ -370,6 +388,7 @@ pub fn schematype_of_value(value: &Value) -> Result<SchemaType, HeterogeneousSet
.iter()
.map(|(k, v)| Ok((k.clone(), AttributeType::required(schematype_of_value(v)?))))
.collect::<Result<_, HeterogeneousSetError>>()?,
open_attrs: false,
}),
Value::ExtensionValue(ev) => Ok(SchemaType::Extension {
name: ev.typename(),
Expand Down
19 changes: 12 additions & 7 deletions cedar-policy-core/src/entities/json/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ impl<'e> ValueParser<'e> {
Some(
expected_ty @ SchemaType::Record {
attrs: expected_attrs,
open_attrs,
},
) => match val {
serde_json::Value::Object(mut actual_attrs) => {
Expand All @@ -537,14 +538,18 @@ impl<'e> ValueParser<'e> {
}
})
.collect::<Result<Vec<(SmolStr, RestrictedExpr)>, JsonDeserializationError>>()?;
// we've now checked that all expected attrs exist, and removed them from `actual_attrs`.
// we still need to verify that we didn't have any unexpected attrs.
if let Some((record_attr, _)) = actual_attrs.into_iter().next() {
return Err(JsonDeserializationError::UnexpectedRecordAttr {
ctx: Box::new(ctx2()),
record_attr: record_attr.into(),
});

if !open_attrs {
// we've now checked that all expected attrs exist, and removed them from `actual_attrs`.
// we still need to verify that we didn't have any unexpected attrs.
if let Some((record_attr, _)) = actual_attrs.into_iter().next() {
return Err(JsonDeserializationError::UnexpectedRecordAttr {
ctx: Box::new(ctx2()),
record_attr: record_attr.into(),
});
}
}

// having duplicate keys should be impossible here (because
// neither `actual_attrs` nor `expected_attrs` can have
// duplicate keys; they're both maps), but we can still throw
Expand Down
3 changes: 3 additions & 0 deletions cedar-policy-validator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,8 @@ decimal = ["cedar-policy-core/decimal"]
# Enables `Arbitrary` implementations for several types in this crate
arbitrary = ["dep:arbitrary"]

# Experimental features.
partial-validate = []

[dev-dependencies]
cool_asserts = "2.0"
4 changes: 4 additions & 0 deletions cedar-policy-validator/src/coreschema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ impl entities::EntityTypeDescription for EntityTypeDescription {
fn allowed_parent_types(&self) -> Arc<HashSet<ast::EntityType>> {
Arc::clone(&self.allowed_parent_types)
}

fn open_attributes(&self) -> bool {
self.validator_type.open_attributes.is_open()
}
}

impl ast::RequestSchema for ValidatorSchema {
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-validator/src/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl std::fmt::Display for UnsupportedFeature {
match self {
Self::OpenRecordsAndEntities => write!(
f,
"records and entities with additional attributes are not yet implemented"
"records and entities with `additionalAttributes` are experimental, but the experimental `partial-validate` feature is not enabled"
),
Self::ActionAttributes(attrs) => write!(
f,
Expand Down
Loading