Getting rid of unspecified (#366)

cedar-drt/create_corpus.sh

@@ -14,14 +14,21 @@
 # limitations under the License.
 
 FUZZ_TARGET="${FUZZ_TARGET:-abac}"
-TIMEOUT="${TIMEOUT:-15m}" # 15m = 900s
+TIMEOUT_MINS="${TIMEOUT:-15}" # 15m = 900s
 JOBS="${JOBS:-4}"
-DUMP_DIR="${DUMP_DIR:-corpus_tests}"
+DUMP_DIR="${DUMP_DIR:-corpus-tests}"
 
 # Assumes cedar-drt is already correctly built and configured
 rm -rf $DUMP_DIR
 source set_env_vars.sh
 export FUZZ_TARGET
 export DUMP_DIR
-timeout $TIMEOUT cargo fuzz run -s none $FUZZ_TARGET -j $JOBS -- -rss_limit_mb=8196
-./synthesize_tests.sh
+echo "Running fuzzer for $TIMEOUT_MINS minutes ..."
+TIMEOUT=$(( $TIMEOUT_MINS * 60 ))
+cargo fuzz run -s none $FUZZ_TARGET -j $JOBS -- -rss_limit_mb=8196 -max_total_time=$TIMEOUT
+echo "Fuzzer done!"
+# Minimize corpus to remove redundant seeds
+echo "Minimizing corpus..."
+cargo fuzz cmin "$FUZZ_TARGET" -s none
+echo "Minimization done"
+./synthesize_tests.sh

cedar-drt/fuzz/fuzz_targets/formatter.rs

@@ -18,7 +18,7 @@
 
 use cedar_drt::initialize_log;
 use cedar_drt_inner::fuzz_target;
-use cedar_policy_core::ast::{AnyId, EntityType, ExprKind, Literal, StaticPolicy, Template};
+use cedar_policy_core::ast::{AnyId, StaticPolicy, Template};
 use cedar_policy_core::parser::{self, parse_policy};
 use cedar_policy_formatter::token::{Comment, Token, WrappedToken};
 use cedar_policy_formatter::{policies_str_to_pretty, Config};
@@ -74,10 +74,6 @@ impl<'a> Arbitrary<'a> for FuzzTargetInput {
     }
 }
 
-fn contains_unspecified_entities(p: &StaticPolicy) -> bool {
-    p.condition().subexpressions().any(|e| matches!(e.expr_kind(), ExprKind::Lit(Literal::EntityUID(euid)) if matches!(*euid.entity_type(), EntityType::Unspecified)))
-}
-
 // Attach each token two uuids as leading comment and one uuid as trailing comment
 fn attach_comment(p: &str, uuids: &mut Vec<String>) -> String {
     Token::lexer(p)
@@ -141,14 +137,6 @@ fn round_trip(p: &StaticPolicy) -> Result<StaticPolicy, parser::err::ParseErrors
 fuzz_target!(|input: FuzzTargetInput| {
     initialize_log();
     let p: StaticPolicy = input.policy.into();
-    // Version 1.2 introduces an unspecified entity type.
-    // An entity of such type prints to an invalid string,
-    // which further causes round-tripping to fail.
-    // The fix is to add a filter to the generated policies,
-    // so that we don't fuzz any policies with unspecified entities.
-    if contains_unspecified_entities(&p) {
-        return;
-    }
     let (t, _) = Template::link_static_policy(p.clone());
 
     debug!("Starting policy: {:?}", p);

cedar-drt/fuzz/fuzz_targets/roundtrip.rs

@@ -18,7 +18,7 @@
 
 use cedar_drt::initialize_log;
 use cedar_drt_inner::{check_policy_equivalence, fuzz_target};
-use cedar_policy_core::ast::{self, EntityType, ExprKind, Literal, StaticPolicy, Template};
+use cedar_policy_core::ast::{self, StaticPolicy, Template};
 use cedar_policy_core::est;
 use cedar_policy_core::parser::{self, parse_policy};
 use cedar_policy_generators::{
@@ -69,10 +69,6 @@ impl<'a> Arbitrary<'a> for FuzzTargetInput {
     }
 }
 
-fn contains_unspecified_entities(p: &StaticPolicy) -> bool {
-    p.condition().subexpressions().any(|e| matches!(e.expr_kind(), ExprKind::Lit(Literal::EntityUID(euid)) if matches!(*euid.entity_type(), EntityType::Unspecified)))
-}
-
 // AST --> text --> CST --> AST
 // Print a policy to a string and parse it back using the standard AST parser.
 // Panics if parsing fails.
@@ -129,14 +125,6 @@ fn round_trip_est(p: &StaticPolicy) -> StaticPolicy {
 fuzz_target!(|input: FuzzTargetInput| {
     initialize_log();
     let p: StaticPolicy = input.policy.into();
-    // Version 1.2 introduces an unspecified entity type.
-    // An entity of such type prints to an invalid string,
-    // which further causes round-tripping to fail.
-    // The fix is to add a filter to the generated policies,
-    // so that we don't fuzz any policies with unspecified entities.
-    if contains_unspecified_entities(&p) {
-        return;
-    }
 
     debug!("Running on policy: {:?}", p);
 

cedar-drt/fuzz/fuzz_targets/validation-pbt-type-directed.rs

@@ -144,8 +144,7 @@ fuzz_target!(|input: FuzzTargetInput| {
                             AuthorizationError::PolicyEvaluationError { error, .. } => {
                                 match error {
                                     // Evaluation errors the validator should prevent.
-                                    EvaluationError::UnspecifiedEntityAccess(_)
-                                    | EvaluationError::RecordAttrDoesNotExist(_)
+                                    EvaluationError::RecordAttrDoesNotExist(_)
                                     | EvaluationError::EntityAttrDoesNotExist(_)
                                     | EvaluationError::FailedExtensionFunctionLookup(_)
                                     | EvaluationError::TypeError(_)

cedar-drt/fuzz/fuzz_targets/validation-pbt.rs

@@ -94,8 +94,6 @@ const LOG_FILENAME_ENTITIES_ERROR: &str = "./logs/err_entities.txt";
 const LOG_FILENAME_TOTAL_ENTITY_TYPES: &str = "./logs/schemastats/total_entity_types";
 const LOG_FILENAME_TOTAL_ACTIONS: &str = "./logs/schemastats/total_actions";
 const LOG_FILENAME_APPLIES_TO_NONE: &str = "./logs/schemastats/applies_to_none";
-const LOG_FILENAME_APPLIES_TO_PRINCIPAL_NONE: &str = "./logs/schemastats/applies_to_principal_none";
-const LOG_FILENAME_APPLIES_TO_RESOURCE_NONE: &str = "./logs/schemastats/applies_to_resource_none";
 const LOG_FILENAME_APPLIES_TO_PRINCIPAL_LEN: &str = "./logs/schemastats/applies_to_principal_len";
 const LOG_FILENAME_APPLIES_TO_RESOURCE_LEN: &str = "./logs/schemastats/applies_to_resource_len";
 const LOG_FILENAME_TOTAL_ENTITIES: &str = "./logs/hierarchystats/total_entities";
@@ -191,30 +189,20 @@ fn maybe_log_schemastats(schema: Option<&NamespaceDefinition>, suffix: &str) {
                     resource_types,
                     ..
                 }) => {
-                    match principal_types.as_ref() {
-                        None => checkpoint(
-                            LOG_FILENAME_APPLIES_TO_PRINCIPAL_NONE.to_string() + "_" + suffix,
-                        ),
-                        Some(tys) => checkpoint(
-                            LOG_FILENAME_APPLIES_TO_PRINCIPAL_LEN.to_string()
-                                + "_"
-                                + suffix
-                                + "_"
-                                + &format!("{:03}", tys.len()),
-                        ),
-                    }
-                    match resource_types.as_ref() {
-                        None => checkpoint(
-                            LOG_FILENAME_APPLIES_TO_RESOURCE_NONE.to_string() + "_" + suffix,
-                        ),
-                        Some(tys) => checkpoint(
-                            LOG_FILENAME_APPLIES_TO_RESOURCE_LEN.to_string()
-                                + "_"
-                                + suffix
-                                + "_"
-                                + &format!("{:03}", tys.len()),
-                        ),
-                    }
+                    checkpoint(
+                        LOG_FILENAME_APPLIES_TO_PRINCIPAL_LEN.to_string()
+                            + "_"
+                            + suffix
+                            + "_"
+                            + &format!("{:03}", principal_types.len()),
+                    );
+                    checkpoint(
+                        LOG_FILENAME_APPLIES_TO_RESOURCE_LEN.to_string()
+                            + "_"
+                            + suffix
+                            + "_"
+                            + &format!("{:03}", resource_types.len()),
+                    );
                 }
             }
         }
@@ -365,8 +353,7 @@ fuzz_target!(|input: FuzzTargetInput| {
                             AuthorizationError::PolicyEvaluationError { error, .. } => {
                                 match error {
                                     // Evaluation errors the validator should prevent.
-                                    EvaluationError::UnspecifiedEntityAccess(_)
-                                    | EvaluationError::RecordAttrDoesNotExist(_)
+                                    EvaluationError::RecordAttrDoesNotExist(_)
                                     | EvaluationError::EntityAttrDoesNotExist(_)
                                     | EvaluationError::FailedExtensionFunctionLookup(_)
                                     | EvaluationError::TypeError(_)

cedar-drt/fuzz/src/dump.rs

@@ -16,7 +16,7 @@
 
 use cedar_policy::{AuthorizationError, Policy};
 use cedar_policy_core::ast::{
-    Context, EntityType, EntityUID, EntityUIDEntry, PolicySet, Request, RestrictedExpr,
+    Context, EntityUID, EntityUIDEntry, PolicySet, Request, RestrictedExpr,
 };
 use cedar_policy_core::authorizer::Response;
 use cedar_policy_core::entities;
@@ -213,21 +213,18 @@ fn passes_validation(schema: SchemaFragment, policies: &PolicySet) -> bool {
     }
 }
 
-/// Dump the entity uid to a json value if it is specified, otherwise return `None`
-fn dump_request_var(var: &EntityUIDEntry) -> Option<JsonValueWithNoDuplicateKeys> {
+/// Dump the entity uid to a json value
+fn dump_request_var(var: &EntityUIDEntry) -> JsonValueWithNoDuplicateKeys {
     match var {
         EntityUIDEntry::Unknown { .. } => {
             panic!("`dump` does not support requests with unknown fields")
         }
-        EntityUIDEntry::Known { euid, .. } => match euid.entity_type() {
-            EntityType::Unspecified => None,
-            EntityType::Specified(_) => {
-                let json = serde_json::to_value(TypeAndId::from(euid as &EntityUID))
-                    .expect("failed to serialize euid")
-                    .into();
-                Some(json)
-            }
-        },
+        EntityUIDEntry::Known { euid, .. } => {
+            let json = serde_json::to_value(TypeAndId::from(euid as &EntityUID))
+                .expect("failed to serialize euid")
+                .into();
+            json
+        }
     }
 }
 

cedar-drt/fuzz/src/schemas.rs

@@ -104,7 +104,6 @@ fn action_type_equivalence(name: &str, lhs: ActionType, rhs: ActionType) -> Resu
             (None, None) => Ok(()),
             (Some(lhs), Some(rhs)) => {
                 // If either of them has at least one empty appliesTo list, the other must have the same attribute.
-                // Otherwise both of them must apply to unspecified entities or non-empty entity lists, which must be equal.
                 if (either_empty(&lhs) && either_empty(&rhs)) || rhs == lhs {
                     Ok(())
                 } else {
@@ -114,8 +113,6 @@ fn action_type_equivalence(name: &str, lhs: ActionType, rhs: ActionType) -> Resu
                     ))
                 }
             }
-            // if one of them has `appliesTo` being null, then the other must have both principal and resource types unspecified
-            (Some(spec), None) | (None, Some(spec)) if both_unspecified(&spec) => Ok(()),
             (Some(_), None) => Err(format!(
                 "Mismatched applies to in `{name}`, lhs was `Some`, `rhs` was `None`"
             )),
@@ -126,13 +123,8 @@ fn action_type_equivalence(name: &str, lhs: ActionType, rhs: ActionType) -> Resu
     }
 }
 
-fn both_unspecified(spec: &ApplySpec) -> bool {
-    spec.resource_types.is_none() && spec.principal_types.is_none()
-}
-
 fn either_empty(spec: &ApplySpec) -> bool {
-    matches!(spec.resource_types.as_ref(), Some(ts) if ts.is_empty())
-        || matches!(spec.principal_types.as_ref(), Some(ts) if ts.is_empty())
+    spec.principal_types.is_empty() || spec.resource_types.is_empty()
 }
 
 /// Just compare entity attribute types and context types are equivalent
@@ -141,7 +133,7 @@ pub fn validator_schema_attr_types_equivalent(
     schema2: &cedar_policy_validator::ValidatorSchema,
 ) -> bool {
     let entity_attr_tys1: HashMap<
-        &cedar_drt::ast::Name,
+        &cedar_drt::ast::EntityType,
         HashMap<&smol_str::SmolStr, &cedar_policy_validator::types::AttributeType>,
     > = HashMap::from_iter(
         schema1

cedar-drt/synthesize_tests.sh

@@ -22,8 +22,6 @@ source set_env_vars.sh
 
 set -u
 
-# Minimize corpus to remove redundant seeds
-cargo fuzz cmin "$FUZZ_TARGET" -s none
 
 run_single_test () {
     # We could use `cargo fuzz run "$FUZZ_TARGET" -s none "$1"`
@@ -43,4 +41,4 @@ for hash in $(ls "fuzz/corpus/$FUZZ_TARGET"); do
     DUMP_TEST_DIR=$DUMP_DIR \
     DUMP_TEST_NAME=$hash \
     run_single_test "fuzz/corpus/$FUZZ_TARGET/$hash"
-done
+done

cedar-lean/DiffTest/Parser.lean

@@ -46,13 +46,7 @@ def jsonToName (json : Lean.Json) : ParseResult Name := do
     }
 
 def jsonToEntityType (json : Lean.Json) : ParseResult EntityType := do
-  let (tag, body) ← unpackJsonSum json
-  match tag with
-  | "Specified" => jsonToName body
-  | "Unspecified" =>
-    -- "Unspecified" entities are treated as normal entities with a unique name
-    .ok { id := "<Unspecified>", path := [] }
-  | tag => .error s!"jsonToEntityType: unknown tag {tag}"
+  jsonToName json
 
 def jsonToEuid (json : Lean.Json) : ParseResult EntityUID := do
   let eid ← getJsonField json "eid" >>= jsonToString

cedar-policy-generators/src/expr.rs

@@ -566,7 +566,7 @@ impl<'a> ExprGenerator<'a> {
                                             // This does not use an explicit namespace because entity types
                                             // implicitly use the schema namespace if an explicit one is not
                                             // provided.
-                                            name: entity_name.clone().into(),
+                                            name: ast::Name::from(entity_name.clone()).into(),
                                         }
                                     ),
                                     max_depth - 1,
@@ -2156,7 +2156,7 @@ impl<'a> ExprGenerator<'a> {
     /// generate a UID with the given typename
     pub fn arbitrary_uid_with_type(
         &self,
-        ty: &ast::Name,
+        ty: &ast::EntityType,
         u: &mut Unstructured<'_>,
     ) -> Result<ast::EntityUID> {
         match self.hierarchy {