Prep for v2.4.2 (#373)

.github/workflows/ci.yml

@@ -31,19 +31,7 @@ jobs:
       - run: cd cedar-policy-core ; cargo test --no-default-features --verbose
       - run: cd cedar-policy-formatter ; cargo test --no-default-features --verbose
       - run: cd cedar-policy-validator ; cargo test --no-default-features --verbose
-
-  cargo_audit:
-    name: Cargo Audit
-    runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        toolchain:
-          - stable
-    steps:
-      - uses: actions/checkout@v3
-      - run: rustup update ${{ matrix.toolchain }} && rustup default ${{ matrix.toolchain }}
-      - run: cargo install cargo-audit
-      - run: cargo audit --deny warnings
+      - run: cargo audit --deny warnings # For some reason this hangs if you don't cargo build first
 
   cargo_semver_checks:
     name: Cargo SemVer Checks

CONTRIBUTING.md

@@ -94,7 +94,7 @@ The following examples assume the *next major* release on main is 3.0, then *nex
 
 * **Add a new feature to release in next minor:** Add a changelog entry to `[Unreleased 2.x]` on main, then backport to 2.x (including the changelog entry).
 * **Introduce a breaking API change to release in next major:** Add a changelog entry to `[Unreleased 3.0]` on main, do not backport.
-* **Upgrade a dependency to fix a CVE:** Add a changelog entry to `[Unreleased 2.x]` on main, then backport to 2.x (including the changelog entry), then backport to 2.4 and ensure the changelog entry is added to `[Unreleased 2.4.1]`.
+* **Upgrade a dependency to fix a CVE:** Add a changelog entry to `[Unreleased 2.x]` on main, then backport to 2.x (including the changelog entry), then backport to 2.4 and ensure the changelog entry is added to `[Unreleased 2.4.2]`.
 
 ## Review Process
 

README.md

@@ -103,6 +103,24 @@ To build, simply run `cargo build` (or `cargo build --release`).
 We maintain changelogs for our public-facing crates: [cedar-policy](./cedar-policy/CHANGELOG.md) and [cedar-policy-cli](./cedar-policy-cli/CHANGELOG.md).
 For a list of the current and past releases, see [crates.io](https://crates.io/crates/cedar-policy) or [Releases](https://github.com/cedar-policy/cedar/releases).
 
+## Backward Compatibility Considerations
+
+Cedar is written in Rust and you will typically depend on Cedar via Cargo. Cargo makes sane choices for the majority of project, but your needs may differ. If you don't want automatic updates to Cedar replace
+
+```
+[dependencies]
+cedar-policy = "2.3.3"
+```
+
+with
+
+```
+[dependencies]
+cedar-policy = "=2.3.3"
+```
+
+in your `Cargo.toml` file.
+
 ## Security
 
 See [SECURITY](SECURITY.md) for more information.

cedar-policy-cli/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Changelog
 
+## 2.4.2
+
 ## 2.4.1
 
 ## 2.4.0

cedar-policy-cli/Cargo.toml

@@ -2,7 +2,7 @@
 name = "cedar-policy-cli"
 edition = "2021"
 
-version = "2.4.1"
+version = "2.4.2"
 license = "Apache-2.0"
 categories = ["compilers", "config"]
 description = "CLI interface for the Cedar Policy language."
@@ -11,8 +11,8 @@ homepage = "https://cedarpolicy.com"
 repository = "https://github.com/cedar-policy/cedar"
 
 [dependencies]
-cedar-policy = { version = "=2.4.1", path = "../cedar-policy" }
-cedar-policy-formatter = { version = "=2.4.1", path = "../cedar-policy-formatter" }
+cedar-policy = { version = "=2.4.2", path = "../cedar-policy" }
+cedar-policy-formatter = { version = "=2.4.2", path = "../cedar-policy-formatter" }
 clap = { version = "4", features = ["derive", "env"] }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"

cedar-policy-core/Cargo.toml

@@ -3,7 +3,7 @@ name = "cedar-policy-core"
 edition = "2021"
 build = "build.rs"
 
-version = "2.4.1"
+version = "2.4.2"
 license = "Apache-2.0"
 categories = ["compilers", "config"]
 description = "Core implemenation of the Cedar Policy language."

cedar-policy-core/src/ast/request.rs

@@ -164,11 +164,16 @@ impl Context {
         Self::from_pairs([])
     }
 
-    /// Create a `Context` from a `RestrictedExpr`, which must be a `Record`
-    /// INVARIANT: `from_expr` must only be called with an `expr` that is a `Record`
-    pub fn from_expr(expr: RestrictedExpr) -> Self {
-        debug_assert!(matches!(expr.expr_kind(), ExprKind::Record { .. }));
-        Self { context: expr }
+    /// Create a `Context` from a `RestrictedExpr`, which must be a `Record`.
+    /// If it is not a `Record`, then this function returns `Err` (returning
+    /// ownership of the non-record expression), otherwise it returns `Ok` of
+    /// a context for that record.
+    pub fn from_expr(expr: RestrictedExpr) -> Result<Self, RestrictedExpr> {
+        match expr.expr_kind() {
+            // INVARIANT: `context` must be a `Record`, which is guaranteed by the match case.
+            ExprKind::Record { .. } => Ok(Self { context: expr }),
+            _ => Err(expr),
+        }
     }
 
     /// Create a `Context` from a map of key to `RestrictedExpr`, or a Vec of

cedar-policy-core/src/authorizer.rs

@@ -523,7 +523,8 @@ mod test {
         let context = Context::from_expr(RestrictedExpr::record([(
             "test".into(),
             RestrictedExpr::new(Expr::unknown("name")).unwrap(),
-        )]));
+        )]))
+        .unwrap();
         let a = Authorizer::new();
         let q = Request::new(
             EntityUID::with_eid("p"),

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

@@ -15,7 +15,7 @@
  */
 
 use super::{JsonDeserializationError, JsonDeserializationErrorContext, SchemaType, ValueParser};
-use crate::ast::{Context, ExprKind};
+use crate::ast::Context;
 use crate::extensions::Extensions;
 use std::collections::HashMap;
 
@@ -80,14 +80,11 @@ impl<'e, 's, S: ContextSchema> ContextJsonParser<'e, 's, S> {
         let rexpr = vparser.val_into_rexpr(json, expected_ty.as_ref(), || {
             JsonDeserializationErrorContext::Context
         })?;
-        // INVARIANT `Context::from_exprs` requires that `rexpr` is a `Record`.
-        // This is checked by the `match` expression
-        match rexpr.expr_kind() {
-            ExprKind::Record { .. } => Ok(Context::from_expr(rexpr)),
-            _ => Err(JsonDeserializationError::ExpectedContextToBeRecord {
+        Context::from_expr(rexpr).map_err(|rexpr| {
+            JsonDeserializationError::ExpectedContextToBeRecord {
                 got: Box::new(rexpr),
-            }),
-        }
+            }
+        })
     }
 
     /// Parse context JSON (in `std::io::Read` form) into a `Context` object

cedar-policy-core/src/evaluator.rs

@@ -4040,7 +4040,7 @@ pub mod test {
         let euid: EntityUID = r#"Test::"test""#.parse().unwrap();
         let rexpr = RestrictedExpr::new(context_expr)
             .expect("Context Expression was not a restricted expression");
-        let context = Context::from_expr(rexpr);
+        let context = Context::from_expr(rexpr).unwrap();
         let q = Request::new(euid.clone(), euid.clone(), euid, context);
         let es = Entities::new();
         let exts = Extensions::none();
@@ -4163,7 +4163,8 @@ pub mod test {
         let context = Context::from_expr(RestrictedExpr::new_unchecked(Expr::record([
             ("a".into(), Expr::val(3)),
             ("b".into(), Expr::unknown("b".to_string())),
-        ])));
+        ])))
+        .unwrap();
         let euid: EntityUID = r#"Test::"test""#.parse().unwrap();
         let q = Request::new(euid.clone(), euid.clone(), euid, context);
         let es = Entities::new();
@@ -4202,7 +4203,7 @@ pub mod test {
             Expr::unknown("cell".to_string()),
         )]))
         .expect("should qualify as restricted");
-        let context = Context::from_expr(c_expr);
+        let context = Context::from_expr(c_expr).unwrap();
 
         let q = Request::new(p, a, r, context);
         let exts = Extensions::none();
@@ -4275,7 +4276,8 @@ pub mod test {
             Context::from_expr(RestrictedExpr::new_unchecked(Expr::record([(
                 "condition".into(),
                 Expr::unknown("unknown_condition"),
-            )]))),
+            )])))
+            .unwrap(),
         );
         let eval = Evaluator::new(&q, &es, &exts).unwrap();
 

cedar-policy-formatter/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Changelog
 
+## 2.4.2
+
 ## 2.4.1
 
 - Bumped `pretty`, `logos`, and `regex` dependencies

cedar-policy-formatter/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cedar-policy-formatter"
-version = "2.4.1"
+version = "2.4.2"
 edition = "2021"
 license = "Apache-2.0"
 categories = ["compilers", "config"]
@@ -10,7 +10,7 @@ homepage = "https://cedarpolicy.com"
 repository = "https://github.com/cedar-policy/cedar"
 
 [dependencies]
-cedar-policy-core = { version = "=2.4.1", path = "../cedar-policy-core" }
+cedar-policy-core = { version = "=2.4.2", path = "../cedar-policy-core" }
 pretty = "0.12.1"
 logos = "0.13.0"
 itertools = "0.10"

cedar-policy-validator/Cargo.toml

@@ -2,7 +2,7 @@
 name = "cedar-policy-validator"
 edition = "2021"
 
-version = "2.4.1"
+version = "2.4.2"
 license = "Apache-2.0"
 categories = ["compilers", "config"]
 description = "Validator for the Cedar Policy language."
@@ -11,7 +11,7 @@ homepage = "https://cedarpolicy.com"
 repository = "https://github.com/cedar-policy/cedar"
 
 [dependencies]
-cedar-policy-core = { version = "=2.4.1", path = "../cedar-policy-core" }
+cedar-policy-core = { version = "=2.4.2", path = "../cedar-policy-core" }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0", features = ["preserve_order"] }
 serde_with = "3.0"

cedar-policy-validator/src/lib.rs

@@ -19,7 +19,7 @@
 
 use std::collections::HashSet;
 
-use cedar_policy_core::ast::{PolicySet, Template};
+use cedar_policy_core::ast::{Policy, PolicySet, Template};
 
 mod err;
 mod str_checks;
@@ -76,40 +76,71 @@ impl Validator {
         Self { schema }
     }
 
-    /// Validate all templates in a policy set (which includes static policies) and
-    /// return an iterator of policy notes associated with each policy id.
+    /// Validate all templates, links, and static policies in a policy set.
+    /// Return an iterator of policy notes associated with each policy id.
     pub fn validate<'a>(
         &'a self,
         policies: &'a PolicySet,
         mode: ValidationMode,
     ) -> ValidationResult<'a> {
-        let template_errs = policies
+        let template_and_static_policy_errs = policies
             .all_templates()
             .flat_map(|p| self.validate_policy(p, mode));
-        let instantiation_errs = policies.policies().flat_map(|p| {
-            self.validate_slots(p.env())
-                .map(move |note| ValidationError::with_policy_id(p.id(), None, note))
-        });
-        ValidationResult::new(template_errs.chain(instantiation_errs))
+        let link_errs = policies
+            .policies()
+            .filter_map(|p| self.validate_slots(p))
+            .flatten();
+        ValidationResult::new(template_and_static_policy_errs.chain(link_errs))
     }
 
-    /// Run all validations against a single policy, gathering all validation
-    /// notes from together in the returned iterator.
+    /// Run all validations against a single static policy or template (note
+    /// that Core `Template` includes static policies as well), gathering all
+    /// validation notes together in the returned iterator.
     fn validate_policy<'a>(
         &'a self,
         p: &'a Template,
         mode: ValidationMode,
     ) -> impl Iterator<Item = ValidationError> + 'a {
         self.validate_entity_types(p)
             .chain(self.validate_action_ids(p))
-            .chain(self.validate_action_application(p))
+            .chain(self.validate_action_application(
+                p.principal_constraint(),
+                p.action_constraint(),
+                p.resource_constraint(),
+            ))
             .map(move |note| ValidationError::with_policy_id(p.id(), None, note))
             .chain(self.typecheck_policy(p, mode))
     }
 
+    /// Run relevant validations against a single template-linked policy,
+    /// gathering all validation notes together in the returned iterator.
+    fn validate_slots<'a>(
+        &'a self,
+        p: &'a Policy,
+    ) -> Option<impl Iterator<Item = ValidationError> + 'a> {
+        // Ignore static policies since they are already handled by `validate_policy`
+        if p.is_static() {
+            return None;
+        }
+        // For template-linked policies `Policy::principal_constraint()` and
+        // `Policy::resource_constraint()` return a copy of the constraint with
+        // the slot filled by the appropriate value.
+        Some(
+            self.validate_entity_types_in_slots(p.env())
+                .chain(self.validate_action_application(
+                    &p.principal_constraint(),
+                    p.action_constraint(),
+                    &p.resource_constraint(),
+                ))
+                .map(move |note| ValidationError::with_policy_id(p.id(), None, note)),
+        )
+    }
+
     /// Construct a Typechecker instance and use it to detect any type errors in
-    /// the argument policy in the context of the schema for this validator. Any
-    /// detected type errors are wrapped and returned as `ValidationErrorKind`s.
+    /// the argument static policy or template (note that Core `Template`
+    /// includes static policies as well) in the context of the schema for this
+    /// validator. Any detected type errors are wrapped and returned as
+    /// `ValidationErrorKind`s.
     fn typecheck_policy<'a>(
         &'a self,
         t: &'a Template,
@@ -313,19 +344,51 @@ mod test {
         )
         .expect("Linking failed!");
         let result = validator.validate(&set, ValidationMode::default());
-
-        let pid = ast::PolicyID::from_string("link2");
-        let resource_err = ValidationError::with_policy_id(
-            &pid,
+        assert!(!result.validation_passed());
+        assert_eq!(result.validation_errors().count(), 2);
+        let id = ast::PolicyID::from_string("link2");
+        let undefined_err = ValidationError::with_policy_id(
+            &id,
             None,
             ValidationErrorKind::unrecognized_entity_type(
                 "some_namespace::Undefined".to_string(),
                 Some("some_namespace::User".to_string()),
             ),
         );
+        let invalid_action_err = ValidationError::with_policy_id(
+            &id,
+            None,
+            ValidationErrorKind::invalid_action_application(false, false),
+        );
+        assert!(result.validation_errors().any(|x| x == &undefined_err));
+        assert!(result.validation_errors().any(|x| x == &invalid_action_err));
+
+        // this is also an invalid instantiation (not a valid resource type for any action in the schema)
+        let mut values = HashMap::new();
+        values.insert(
+            ast::SlotId::resource(),
+            ast::EntityUID::from_components(
+                "some_namespace::User".parse().unwrap(),
+                ast::Eid::new("foo"),
+            ),
+        );
+        set.link(
+            ast::PolicyID::from_string("template"),
+            ast::PolicyID::from_string("link3"),
+            values,
+        )
+        .expect("Linking failed!");
+        let result = validator.validate(&set, ValidationMode::default());
         assert!(!result.validation_passed());
-        println!("{:?}", result.validation_errors().collect::<Vec<_>>());
-        assert!(result.validation_errors().any(|x| x == &resource_err));
+        // `result` contains the two prior error messages plus one new one
+        assert_eq!(result.validation_errors().count(), 3);
+        let id = ast::PolicyID::from_string("link3");
+        let invalid_action_err = ValidationError::with_policy_id(
+            &id,
+            None,
+            ValidationErrorKind::invalid_action_application(false, false),
+        );
+        assert!(result.validation_errors().any(|x| x == &invalid_action_err));
 
         Ok(())
     }