Partial schema validation (#79)

Experimental implementation of partial schema validation as described in cedar-policy/rfcs#8.

Author: john-h-kastner-aws

cedar-policy-cli/Cargo.toml

@@ -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"

cedar-policy-cli/src/lib.rs

@@ -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)]
@@ -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) => {
@@ -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())

cedar-policy-cli/tests/sample.rs

@@ -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);

cedar-policy-core/src/entities.rs

@@ -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"),
@@ -1789,6 +1791,7 @@ mod schema_based_parsing_tests {
                     ]
                     .into_iter()
                     .collect(),
+                    open_attrs: false,
                 }),
                 _ => None,
             }
@@ -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"))]
@@ -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!(

cedar-policy-core/src/entities/conformance.rs

@@ -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

cedar-policy-core/src/entities/json/context.rs

@@ -33,6 +33,7 @@ impl ContextSchema for NullContextSchema {
     fn context_type(&self) -> SchemaType {
         SchemaType::Record {
             attrs: HashMap::new(),
+            open_attrs: false,
         }
     }
 }

cedar-policy-core/src/entities/json/entities.rs

@@ -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(),

cedar-policy-core/src/entities/json/schema.rs

@@ -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
@@ -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
+    }
 }

cedar-policy-core/src/entities/json/schema_types.rs

@@ -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 {
@@ -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) => {
@@ -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 attrs2 is
+                                // closed, incompatible.  otherwise fine
+                                !v.required || *open2
                             }
                         }
                     }) && attrs2.iter().all(|(k, v)| {
@@ -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
                             }
                         }
                     })
@@ -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
@@ -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, .. } => {
@@ -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(),

cedar-policy-core/src/entities/json/value.rs

@@ -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) => {
@@ -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