Repository: cedar-policy/cedar
Pull request #1535 (2025-03-13 14:01:45)

a few more tweaks to protobuf format
## Description of changes

Makes two more changes to the protobuf format:
1. changes the request `context` from `Expr` to `map<string, Expr>`, since it's required to be a record anyway.  This aligns with the format for entity attributes which is also `map<string, Expr>`.
2. in `PolicySet`, changes both `templates` and `links` from a (map from id to object) to (just a list of object), since the objects in question already contain the ids.  This makes the encoding faster and more compact.

## Issue #, if available

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [x] A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

- [x] Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that [`cedar-spec`](https://github.com/cedar-policy/cedar-spec) (choose one, and delete the other options):

- [x] Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in `cedar-spec`, and how you have tested that your updates are correct.)

I confirm that [`docs.cedarpolicy.com`](https://docs.cedarpolicy.com/) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar language specification.


github-actions[bot]: 
# Coverage Report
Head Commit: ```8dede318393789b65d2a41c9b96bfd1dac481d77```

Base Commit: ```d2ab99af3825f233991e8e68e4434c8996f4bcd0```

[Download the full coverage report.](https://github.com/cedar-policy/cedar/actions/runs/13839011488/artifacts/2746834236)

## Coverage of Added or Modified Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 84.78%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy/src/proto/ast.rs | :yellow_circle: | 25/32 | 78.12% | 658-660, 667-670
cedar-policy/src/proto/policy.rs | :green_circle: | 14/14 | 100.00% | 
</details>

## Coverage of All Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 81.49%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3052/4395 | 69.44% | 69.60%
cedar-policy-cli | :red_circle: | 538/927 | 58.04% | 58.04%
cedar-policy-core | :green_circle: | 11639/13881 | 83.85% | 83.85%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8242/9650 | 85.41% | 85.41%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>

github-actions[bot]: 
# Coverage Report
Head Commit: ```6ada363f3255c42a165f9a5ef8845cf37f9f8a66```

Base Commit: ```543a7331e2c10fc20c494ee9ec7177d1bfa86a08```

[Download the full coverage report.](https://github.com/cedar-policy/cedar/actions/runs/14066152476/artifacts/2817841731)

## Coverage of Added or Modified Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 84.78%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy/src/proto/ast.rs | :yellow_circle: | 25/32 | 78.12% | 658-660, 667-670
cedar-policy/src/proto/policy.rs | :green_circle: | 14/14 | 100.00% | 
</details>

## Coverage of All Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 81.49%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3055/4405 | 69.35% | 69.51%
cedar-policy-cli | :red_circle: | 538/927 | 58.04% | 58.04%
cedar-policy-core | :green_circle: | 11657/13899 | 83.87% | 83.87%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8305/9724 | 85.41% | 85.41%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>


Patch:
diff --git a/cedar-policy-cli/CHANGELOG.md b/cedar-policy-cli/CHANGELOG.md
index 90c48b04..fa17cd34 100644
--- a/cedar-policy-cli/CHANGELOG.md
+++ b/cedar-policy-cli/CHANGELOG.md
@@ -8,8 +8,10 @@ Changes to the Cedar language, which are likely to affect users of the CLI, are
 ### Added
 
 - Added `json-to-cedar` direction to `translate-policy` command. (#1510, resolving #461)
-- Add `--level` option to the `validate` command, exposing the `level-validate`
+- Added `--level` option to the `validate` command, exposing the `level-validate`
   experimental feature through the CLI. (#1508, resolving #1501)
+- Improved the `check-parse` command, which now checks the parse of policies, schema,
+  and/or entities (whatever is passed). (#1548)
 
 ## 4.3.3
 
diff --git a/cedar-policy-cli/src/lib.rs b/cedar-policy-cli/src/lib.rs
index a05acf0c..8f37da8a 100644
--- a/cedar-policy-cli/src/lib.rs
+++ b/cedar-policy-cli/src/lib.rs
@@ -89,7 +89,10 @@ pub enum Commands {
     Evaluate(EvaluateArgs),
     /// Validate a policy set against a schema
     Validate(ValidateArgs),
-    /// Check that policies successfully parse
+    /// Check that policies, schema, and/or entities successfully parse.
+    /// (All arguments are optional; this checks that whatever is provided parses)
+    ///
+    /// If no arguments are provided, reads policies from stdin and checks that they parse.
     CheckParse(CheckParseArgs),
     /// Link a template
     Link(LinkArgs),
@@ -197,7 +200,13 @@ pub struct ValidateArgs {
 pub struct CheckParseArgs {
     /// Policies args (incorporated by reference)
     #[command(flatten)]
-    pub policies: PoliciesArgs,
+    pub policies: OptionalPoliciesArgs,
+    /// Schema args (incorporated by reference)
+    #[command(flatten)]
+    pub schema: OptionalSchemaArgs,
+    /// File containing JSON representation of a Cedar entity hierarchy
+    #[arg(long = "entities", value_name = "FILE")]
+    pub entities_file: Option<PathBuf>,
 }
 
 /// This struct contains the arguments that together specify a request.
@@ -477,6 +486,40 @@ impl PoliciesArgs {
     }
 }
 
+/// This struct contains the arguments that together specify an input policy or policy set,
+/// for commands where policies are optional.
+#[derive(Args, Debug)]
+pub struct OptionalPoliciesArgs {
+    /// File containing static Cedar policies and/or templates
+    #[arg(short, long = "policies", value_name = "FILE")]
+    pub policies_file: Option<String>,
+    /// Format of policies in the `--policies` file
+    #[arg(long = "policy-format", default_value_t, value_enum)]
+    pub policy_format: PolicyFormat,
+    /// File containing template-linked policies. Ignored if `--policies` is not
+    /// present (because in that case there are no templates to link against)
+    #[arg(short = 'k', long = "template-linked", value_name = "FILE")]
+    pub template_linked_file: Option<String>,
+}
+
+impl OptionalPoliciesArgs {
+    /// Turn this `OptionalPoliciesArgs` into the appropriate `PolicySet`
+    /// object, or `None` if no policies were provided
+    fn get_policy_set(&self) -> Result<Option<PolicySet>> {
+        match &self.policies_file {
+            None => Ok(None),
+            Some(policies_file) => {
+                let pargs = PoliciesArgs {
+                    policies_file: Some(policies_file.clone()),
+                    policy_format: self.policy_format,
+                    template_linked_file: self.template_linked_file.clone(),
+                };
+                pargs.get_policy_set().map(Some)
+            }
+        }
+    }
+}
+
 /// This struct contains the arguments that together specify an input schema.
 #[derive(Args, Debug)]
 pub struct SchemaArgs {
@@ -759,13 +802,55 @@ impl Termination for CedarExitCode {
 }
 
 pub fn check_parse(args: &CheckParseArgs) -> CedarExitCode {
+    // for backwards compatibility: if no policies/schema/entities are provided,
+    // read policies from stdin and check that they parse
+    if (
+        &args.policies.policies_file,
+        &args.schema.schema_file,
+        &args.entities_file,
+    ) == (&None, &None, &None)
+    {
+        let pargs = PoliciesArgs {
+            policies_file: None, // read from stdin
+            policy_format: args.policies.policy_format,
+            template_linked_file: args.policies.template_linked_file.clone(),
+        };
+        match pargs.get_policy_set() {
+            Ok(_) => return CedarExitCode::Success,
+            Err(e) => {
+                println!("{e:?}");
+                return CedarExitCode::Failure;
+            }
+        }
+    }
+
+    let mut exit_code = CedarExitCode::Success;
     match args.policies.get_policy_set() {
-        Ok(_) => CedarExitCode::Success,
+        Ok(_) => (),
         Err(e) => {
             println!("{e:?}");
-            CedarExitCode::Failure
+            exit_code = CedarExitCode::Failure;
+        }
+    }
+    let schema = match args.schema.get_schema() {
+        Ok(schema) => schema,
+        Err(e) => {
+            println!("{e:?}");
+            exit_code = CedarExitCode::Failure;
+            None
         }
+    };
+    match &args.entities_file {
+        None => (),
+        Some(efile) => match load_entities(efile, schema.as_ref()) {
+            Ok(_) => (),
+            Err(e) => {
+                println!("{e:?}");
+                exit_code = CedarExitCode::Failure;
+            }
+        },
     }
+    exit_code
 }
 
 pub fn validate(args: &ValidateArgs) -> CedarExitCode {
diff --git a/cedar-policy-validator/src/cedar_schema/ast.rs b/cedar-policy-validator/src/cedar_schema/ast.rs
index e9218a3c..290ba465 100644
--- a/cedar-policy-validator/src/cedar_schema/ast.rs
+++ b/cedar-policy-validator/src/cedar_schema/ast.rs
@@ -359,8 +359,8 @@ impl std::fmt::Display for PR {
 pub struct PRAppDecl {
     /// Is this constraining the `principal` or the `resource`
     pub kind: Node<PR>,
-    /// What entity types are allowed?
-    pub entity_tys: NonEmpty<Path>,
+    /// What entity types are allowed? `None` means none
+    pub entity_tys: Option<NonEmpty<Path>>,
 }
 
 /// A declaration of constraints on an action type
diff --git a/cedar-policy-validator/src/cedar_schema/err.rs b/cedar-policy-validator/src/cedar_schema/err.rs
index 2fe4d2f7..c2f3ce96 100644
--- a/cedar-policy-validator/src/cedar_schema/err.rs
+++ b/cedar-policy-validator/src/cedar_schema/err.rs
@@ -485,19 +485,39 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn no_principal(name: &impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_principal(name: &impl ToSmolStr, name_loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Principal,
             name: name.to_smolstr(),
-            loc,
+            missing_or_empty: MissingOrEmpty::Missing,
+            name_loc,
         })
     }
 
-    pub(crate) fn no_resource(name: &impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_resource(name: &impl ToSmolStr, name_loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Resource,
             name: name.to_smolstr(),
-            loc,
+            missing_or_empty: MissingOrEmpty::Missing,
+            name_loc,
+        })
+    }
+
+    pub(crate) fn empty_principal(name: &impl ToSmolStr, name_loc: Loc, loc: Loc) -> Self {
+        Self::NoPrincipalOrResource(NoPrincipalOrResource {
+            kind: PR::Principal,
+            name: name.to_smolstr(),
+            missing_or_empty: MissingOrEmpty::Empty { loc },
+            name_loc,
+        })
+    }
+
+    pub(crate) fn empty_resource(name: &impl ToSmolStr, name_loc: Loc, loc: Loc) -> Self {
+        Self::NoPrincipalOrResource(NoPrincipalOrResource {
+            kind: PR::Resource,
+            name: name.to_smolstr(),
+            missing_or_empty: MissingOrEmpty::Empty { loc },
+            name_loc,
         })
     }
 
@@ -615,18 +635,57 @@ impl Diagnostic for DuplicateDeclarations {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Error)]
-#[error("missing `{kind}` declaration for `{name}`")]
+#[error("{}", match .missing_or_empty {
+    MissingOrEmpty::Missing => format!("missing `{kind}` declaration for `{name}`"),
+    MissingOrEmpty::Empty { .. } => format!("for action `{name}`, `{kind}` is `[]`, which is invalid")
+})]
 pub struct NoPrincipalOrResource {
     kind: PR,
     name: SmolStr,
-    loc: Loc,
+    missing_or_empty: MissingOrEmpty,
+    /// Loc of the action name
+    name_loc: Loc,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+enum MissingOrEmpty {
+    /// The declaration was entirely missing
+    Missing,
+    /// The declaration was present but defined as `[]`
+    Empty {
+        /// `Loc` of the declaration
+        loc: Loc,
+    },
 }
 
 pub const NO_PR_HELP_MSG: &str =
-    "Every action must define both `principal` and `resource` targets.";
+    "Every action must define both `principal` and `resource` targets, and the `principal` and `resource` lists must not be `[]`.";
 
 impl Diagnostic for NoPrincipalOrResource {
-    impl_diagnostic_from_source_loc_field!(loc);
+    fn source_code(&self) -> Option<&dyn miette::SourceCode> {
+        Some(&self.name_loc.src as &dyn miette::SourceCode)
+    }
+
+    fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
+        match &self.missing_or_empty {
+            MissingOrEmpty::Missing => {
+                // just the action name
+                Some(Box::new(std::iter::once(miette::LabeledSpan::underline(
+                    self.name_loc.span,
+                ))))
+            }
+            MissingOrEmpty::Empty { loc } => {
+                // also underline the bad declaration
+                let action_name = miette::LabeledSpan::new_with_span(
+                    Some("for this action".into()),
+                    self.name_loc.span,
+                );
+                let decl =
+                    miette::LabeledSpan::new_with_span(Some("must not be `[]`".into()), loc.span);
+                Some(Box::new([action_name, decl].into_iter()))
+            }
+        }
+    }
 
     fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
         Some(Box::new(NO_PR_HELP_MSG))
diff --git a/cedar-policy-validator/src/cedar_schema/grammar.lalrpop b/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
index 8d56b616..b7e2220e 100644
--- a/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
+++ b/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
@@ -157,24 +157,20 @@ TypeDecl: Node<Declaration> = {
 // AppDecls := ('principal' | 'resource') ':' EntTypes [',' | ',' AppDecls]
 //          | 'context' ':' (Path | RecType) [',' | ',' AppDecls]
 AppDecls: Node<NonEmpty<Node<AppDecl>>> = {
-    <l:@L> <pr: PrincipalOrResource> ":" <ets:EntTypes> ","? <r:@R>
-        =>?
-                NonEmpty::collect(ets.into_iter()).ok_or(ParseError::User {
-                    error: UserError::EmptyList(Node::with_source_loc((), Loc::new(l..r, Arc::clone(src))))})
-                    .map(|ets|
-                        Node::with_source_loc(
-                            nonempty![Node::with_source_loc(AppDecl::PR(PRAppDecl { kind:pr, entity_tys: ets}), Loc::new(l..r, Arc::clone(src)))],
-                            Loc::new(l..r, Arc::clone(src)))),
-    <l:@L> <pr: PrincipalOrResource> ":" <ets:EntTypes> "," <r:@R> <mut ds: AppDecls>
-        =>?
-                NonEmpty::collect(ets.into_iter()).ok_or(ParseError::User {
-                    error: UserError::EmptyList(Node::with_source_loc((), Loc::new(l..r, Arc::clone(src))))})
-                    .map(|ets|
-                        {
-                            let (mut ds, _) = ds.into_inner();
-                            ds.insert(0, Node::with_source_loc(AppDecl::PR(PRAppDecl { kind:pr, entity_tys: ets}), Loc::new(l..r, Arc::clone(src))));
-                            Node::with_source_loc(ds, Loc::new(l..r, Arc::clone(src)))
-                        }),
+    <l:@L> <pr: PrincipalOrResource> ":" <ets:EntTypes> <r:@R> ","?
+        => {
+            let entity_tys: Option<NonEmpty<Path>> = NonEmpty::collect(ets.into_iter());
+            Node::with_source_loc(
+                nonempty![Node::with_source_loc(AppDecl::PR(PRAppDecl { kind:pr, entity_tys }), Loc::new(l..r, Arc::clone(src)))],
+                Loc::new(l..r, Arc::clone(src)))
+        },
+    <l:@L> <pr: PrincipalOrResource> ":" <ets:EntTypes> <r:@R> "," <mut ds: AppDecls>
+        => {
+            let (mut ds, _) = ds.into_inner();
+            let entity_tys: Option<NonEmpty<Path>> = NonEmpty::collect(ets.into_iter());
+            ds.insert(0, Node::with_source_loc(AppDecl::PR(PRAppDecl { kind:pr, entity_tys }), Loc::new(l..r, Arc::clone(src))));
+            Node::with_source_loc(ds, Loc::new(l..r, Arc::clone(src)))
+        },
     <l:@L> CONTEXT ":" <p:Path> ","? <r:@R>
         =>  Node::with_source_loc(
                 nonempty![Node::with_source_loc(AppDecl::Context(Either::Left(p)), Loc::new(l..r, Arc::clone(src)))],
diff --git a/cedar-policy-validator/src/cedar_schema/to_json_schema.rs b/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
index e20b09dd..0008551b 100644
--- a/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
+++ b/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
@@ -311,12 +311,19 @@ fn convert_app_decls(
                     )
                     .into());
                 }
-                None => {
-                    principal_types = Some(Node::with_source_loc(
-                        entity_tys.iter().map(|n| n.clone().into()).collect(),
-                        loc,
-                    ))
-                }
+                None => match entity_tys {
+                    None => {
+                        return Err(
+                            ToJsonSchemaError::empty_principal(name, name_loc.clone(), loc).into(),
+                        )
+                    }
+                    Some(entity_tys) => {
+                        principal_types = Some(Node::with_source_loc(
+                            entity_tys.iter().map(|n| n.clone().into()).collect(),
+                            loc,
+                        ))
+                    }
+                },
             },
             Node {
                 node:
@@ -334,12 +341,19 @@ fn convert_app_decls(
                         ToJsonSchemaError::duplicate_resource(name, existing_tys.loc, loc).into(),
                     );
                 }
-                None => {
-                    resource_types = Some(Node::with_source_loc(
-                        entity_tys.iter().map(|n| n.clone().into()).collect(),
-                        loc,
-                    ))
-                }
+                None => match entity_tys {
+                    None => {
+                        return Err(
+                            ToJsonSchemaError::empty_resource(name, name_loc.clone(), loc).into(),
+                        )
+                    }
+                    Some(entity_tys) => {
+                        resource_types = Some(Node::with_source_loc(
+                            entity_tys.iter().map(|n| n.clone().into()).collect(),
+                            loc,
+                        ))
+                    }
+                },
             },
         }
     }
diff --git a/cedar-policy/protobuf_schema/core.proto b/cedar-policy/protobuf_schema/core.proto
index e89de63e..eca2d807 100644
--- a/cedar-policy/protobuf_schema/core.proto
+++ b/cedar-policy/protobuf_schema/core.proto
@@ -21,23 +21,19 @@ message Request {
     EntityUid principal = 1;
     EntityUid action = 2;
     EntityUid resource = 3;
-    Expr context = 4;
+    map<string, Expr> context = 4;
 }
 
 // the protobuf PolicySet message describes a complete policy set, including
 // templates, static policies, and/or template-linked policies.
 message PolicySet {
-    // Key is PolicyID as a string.
-    // Value is a `TemplateBody`.
-    // Both templates and static policies are included in this map, with static
+    // Both templates and static policies are included here, with static
     // policies represented as templates with zero slots.
-    map<string, TemplateBody> templates = 1;
-    // Key is PolicyID as a string.
-    // Value is a `Policy`.
-    // All static policies and template-linked policies are included in this map.
-    // Static policies must have exactly one entry in this map, and the PolicyID
-    // of the static policy must be the same in this map and the above map.
-    map<string, Policy> links = 2;
+    repeated TemplateBody templates = 1;
+    // All static policies and template-linked policies are included here.
+    // Static policies must appear exactly once, and the PolicyID of the static
+    // policy must be the same in this list and the above list.
+    repeated Policy links = 2;
 }
 
 message Entities {
diff --git a/cedar-policy/src/proto/ast.rs b/cedar-policy/src/proto/ast.rs
index ca2fa5b5..175532b4 100644
--- a/cedar-policy/src/proto/ast.rs
+++ b/cedar-policy/src/proto/ast.rs
@@ -20,6 +20,7 @@ use super::models;
 use cedar_policy_core::{
     ast, evaluator::RestrictedEvaluator, extensions::Extensions, FromNormalizedStr,
 };
+use smol_str::ToSmolStr;
 use std::{collections::HashSet, sync::Arc};
 
 // PANIC SAFETY: experimental feature
@@ -436,6 +437,12 @@ impl From<&ast::Expr> for models::Expr {
     }
 }
 
+impl From<&ast::Value> for models::Expr {
+    fn from(v: &ast::Value) -> Self {
+        (&ast::Expr::from(v.clone())).into()
+    }
+}
+
 impl From<&models::expr::Var> for ast::Var {
     fn from(v: &models::expr::Var) -> Self {
         match v {
@@ -620,18 +627,49 @@ impl From<&models::Request> for ast::Request {
             ast::EntityUIDEntry::from(v.principal.as_ref().expect("principal.as_ref()")),
             ast::EntityUIDEntry::from(v.action.as_ref().expect("action.as_ref()")),
             ast::EntityUIDEntry::from(v.resource.as_ref().expect("resource.as_ref()")),
-            v.context.as_ref().map(ast::Context::from),
+            Some(
+                ast::Context::from_pairs(
+                    v.context.iter().map(|(k, v)| {
+                        (
+                            k.to_smolstr(),
+                            ast::RestrictedExpr::new(ast::Expr::from(v))
+                                .expect("encoded context should be a valid RestrictedExpr"),
+                        )
+                    }),
+                    Extensions::all_available(),
+                )
+                .expect("encoded context should be valid"),
+            ),
         )
     }
 }
 
 impl From<&ast::Request> for models::Request {
+    // PANIC SAFETY: experimental feature
+    #[allow(clippy::unimplemented)]
     fn from(v: &ast::Request) -> Self {
         Self {
             principal: Some(models::EntityUid::from(v.principal())),
             action: Some(models::EntityUid::from(v.action())),
             resource: Some(models::EntityUid::from(v.resource())),
-            context: v.context().map(models::Expr::from),
+            context: {
+                let ctx = match v.context() {
+                    Some(ctx) => ctx,
+                    None => unimplemented!(
+                        "Requests with unknown context currently cannot be modeled in protobuf"
+                    ),
+                };
+                match ctx {
+                    ast::Context::Value(map) => map
+                        .iter()
+                        .map(|(k, v)| (k.to_string(), models::Expr::from(v)))
+                        .collect(),
+                    ast::Context::RestrictedResidual(map) => map
+                        .iter()
+                        .map(|(k, v)| (k.to_string(), models::Expr::from(v)))
+                        .collect(),
+                }
+            },
         }
     }
 }
@@ -642,10 +680,10 @@ impl From<&models::Expr> for ast::Context {
         #[allow(clippy::expect_used)]
         ast::Context::from_expr(
             ast::BorrowedRestrictedExpr::new(&ast::Expr::from(v))
-                .expect("context should be valid restricted expr"),
+                .expect("encoded context should be valid restricted expr"),
             Extensions::none(),
         )
-        .expect("Context::from_expr")
+        .expect("encoded context should be valid")
     }
 }
 
diff --git a/cedar-policy/src/proto/policy.rs b/cedar-policy/src/proto/policy.rs
index 9501266e..c37fc891 100644
--- a/cedar-policy/src/proto/policy.rs
+++ b/cedar-policy/src/proto/policy.rs
@@ -427,19 +427,27 @@ impl From<&ast::Effect> for models::Effect {
 }
 
 impl From<&models::PolicySet> for ast::LiteralPolicySet {
+    // PANIC SAFETY: experimental feature
+    #[allow(clippy::expect_used)]
     fn from(v: &models::PolicySet) -> Self {
-        let templates = v.templates.iter().map(|(key, value)| {
+        let templates = v.templates.iter().map(|tb| {
             (
-                ast::PolicyID::from_string(key),
-                ast::Template::from(ast::TemplateBody::from(value)),
+                ast::PolicyID::from_string(&tb.id),
+                ast::Template::from(ast::TemplateBody::from(tb)),
             )
         });
 
-        let links = v.links.iter().map(|(key, value)| {
-            (
-                ast::PolicyID::from_string(key),
-                ast::LiteralPolicy::from(value),
-            )
+        let links = v.links.iter().map(|p| {
+            // per docs in core.proto, for static policies, `link_id` is omitted/ignored,
+            // and the ID of the policy is the `template_id`.
+            let id = if p.is_template_link {
+                p.link_id
+                    .as_ref()
+                    .expect("template link should have a link_id")
+            } else {
+                &p.template_id
+            };
+            (ast::PolicyID::from_string(id), ast::LiteralPolicy::from(p))
         });
 
         Self::new(templates, links)
@@ -448,45 +456,16 @@ impl From<&models::PolicySet> for ast::LiteralPolicySet {
 
 impl From<&ast::LiteralPolicySet> for models::PolicySet {
     fn from(v: &ast::LiteralPolicySet) -> Self {
-        let templates = v
-            .templates()
-            .map(|template| {
-                (
-                    String::from(template.id().as_ref()),
-                    models::TemplateBody::from(template),
-                )
-            })
-            .collect();
-        let links = v
-            .policies()
-            .map(|policy| {
-                (
-                    String::from(policy.id().as_ref()),
-                    models::Policy::from(policy),
-                )
-            })
-            .collect();
-
+        let templates = v.templates().map(models::TemplateBody::from).collect();
+        let links = v.policies().map(models::Policy::from).collect();
         Self { templates, links }
     }
 }
 
 impl From<&ast::PolicySet> for models::PolicySet {
     fn from(v: &ast::PolicySet) -> Self {
-        let templates: HashMap<String, models::TemplateBody> = v
-            .all_templates()
-            .map(|t| (String::from(t.id().as_ref()), models::TemplateBody::from(t)))
-            .collect();
-        let links: HashMap<String, models::Policy> = v
-            .policies()
-            .map(|policy| {
-                (
-                    String::from(policy.id().as_ref()),
-                    models::Policy::from(policy),
-                )
-            })
-            .collect();
-
+        let templates = v.all_templates().map(models::TemplateBody::from).collect();
+        let links = v.policies().map(models::Policy::from).collect();
         Self { templates, links }
     }
 }
@@ -504,6 +483,39 @@ mod test {
 
     use super::*;
 
+    // We add `PartialOrd` and `Ord` implementations for both `models::Policy` and
+    // `models::TemplateBody`, so that these can be sorted for testing purposes
+    impl Eq for models::Policy {}
+    impl PartialOrd for models::Policy {
+        fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+            Some(self.cmp(other))
+        }
+    }
+    impl Ord for models::Policy {
+        fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+            // assumes that (link-id, template-id) pair is unique, otherwise we're
+            // technically violating `Ord` contract because there could exist two
+            // policies that return `Ordering::Equal` but are not equal with `Eq`
+            self.link_id()
+                .cmp(other.link_id())
+                .then_with(|| self.template_id.cmp(&other.template_id))
+        }
+    }
+    impl Eq for models::TemplateBody {}
+    impl PartialOrd for models::TemplateBody {
+        fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+            Some(self.cmp(other))
+        }
+    }
+    impl Ord for models::TemplateBody {
+        fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+            // assumes that IDs are unique, otherwise we're technically violating
+            // `Ord` contract because there could exist two template-bodies that
+            // return `Ordering::Equal` but are not equal with `Eq`
+            self.id.cmp(&other.id)
+        }
+    }
+
     #[test]
     #[allow(clippy::too_many_lines)]
     fn policy_roundtrip() {
@@ -739,10 +751,17 @@ mod test {
             )]),
         )
         .unwrap();
-        let mps = models::PolicySet::from(&ps);
-        let mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
+        let mut mps = models::PolicySet::from(&ps);
+        let mut mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
+
+        // we accept permutations as equivalent, so before comparison, we sort
+        // both `.templates` and `.links`
+        mps.templates.sort();
+        mps_roundtrip.templates.sort();
+        mps.links.sort();
+        mps_roundtrip.links.sort();
 
-        // Can't compare LiteralPolicySets directly, so we compare their fields
+        // Can't compare `models::PolicySet` directly, so we compare their fields
         assert_eq!(mps.templates, mps_roundtrip.templates);
         assert_eq!(mps.links, mps_roundtrip.links);
     }
@@ -802,8 +821,15 @@ mod test {
             )]),
         )
         .unwrap();
-        let mps = models::PolicySet::from(&ps);
-        let mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
+        let mut mps = models::PolicySet::from(&ps);
+        let mut mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
+
+        // we accept permutations as equivalent, so before comparison, we sort
+        // both `.templates` and `.links`
+        mps.templates.sort();
+        mps_roundtrip.templates.sort();
+        mps.links.sort();
+        mps_roundtrip.links.sort();
 
         // Can't compare `models::PolicySet` directly, so we compare their fields
         assert_eq!(mps.templates, mps_roundtrip.templates);


Test patch:
diff --git a/cedar-policy-cli/tests/sample.rs b/cedar-policy-cli/tests/sample.rs
index 690fe23a..c7c4b455 100644
--- a/cedar-policy-cli/tests/sample.rs
+++ b/cedar-policy-cli/tests/sample.rs
@@ -23,30 +23,39 @@ use std::path::{Path, PathBuf};
 
 use cedar_policy::EvalResult;
 use cedar_policy::SlotId;
-use cedar_policy_cli::check_parse;
-use cedar_policy_cli::OptionalSchemaArgs;
-use cedar_policy_cli::SchemaArgs;
-use cedar_policy_cli::SchemaFormat;
 use cedar_policy_cli::{
-    authorize, evaluate, link, validate, Arguments, AuthorizeArgs, CedarExitCode, CheckParseArgs,
-    EvaluateArgs, LinkArgs, PoliciesArgs, PolicyFormat, RequestArgs, ValidateArgs,
+    authorize, check_parse, evaluate, link, validate, Arguments, AuthorizeArgs, CedarExitCode,
+    CheckParseArgs, EvaluateArgs, LinkArgs, OptionalPoliciesArgs, OptionalSchemaArgs, PoliciesArgs,
+    PolicyFormat, RequestArgs, SchemaArgs, SchemaFormat, ValidateArgs,
 };
 
 use predicates::prelude::*;
 use rstest::rstest;
 
-fn run_check_parse_test(policies_file: impl Into<String>, expected_exit_code: CedarExitCode) {
+#[track_caller]
+fn run_check_parse_test(
+    policies_file: impl Into<String>,
+    schema_file: impl Into<PathBuf>,
+    entities_file: Option<impl Into<PathBuf>>,
+    expected_exit_code: CedarExitCode,
+) {
     let cmd = CheckParseArgs {
-        policies: PoliciesArgs {
+        policies: OptionalPoliciesArgs {
             policies_file: Some(policies_file.into()),
             policy_format: PolicyFormat::Cedar,
             template_linked_file: None,
         },
+        schema: OptionalSchemaArgs {
+            schema_file: Some(schema_file.into()),
+            schema_format: SchemaFormat::Cedar,
+        },
+        entities_file: entities_file.map(Into::into),
     };
     let output = check_parse(&cmd);
     assert_eq!(output, expected_exit_code, "{:#?}", cmd);
 }
 
+#[track_caller]
 fn run_authorize_test(
     policies_file: impl Into<String>,
     entities_file: impl Into<String>,
@@ -66,6 +75,7 @@ fn run_authorize_test(
     );
 }
 
+#[track_caller]
 fn run_authorize_test_with_linked_policies(
     policies_file: impl Into<String>,
     entities_file: impl Into<String>,
@@ -101,6 +111,7 @@ fn run_authorize_test_with_linked_policies(
     assert_eq!(exit_code, output, "{:#?}", cmd,);
 }
 
+#[track_caller]
 fn run_link_test(
     policies_file: impl Into<String>,
     links_file: impl Into<String>,
@@ -125,6 +136,7 @@ fn run_link_test(
     assert_eq!(output, expected);
 }
 
+#[track_caller]
 fn run_authorize_test_context(
     policies_file: impl Into<String>,
     entities_file: impl Into<String>,
@@ -160,6 +172,7 @@ fn run_authorize_test_context(
     assert_eq!(exit_code, output, "{:#?}", cmd,);
 }
 
+#[track_caller]
 fn run_authorize_test_json(
     policies_file: impl Into<String>,
     entities_file: impl Into<String>,
@@ -196,6 +209,8 @@ fn run_authorize_test_json(
 fn test_authorize_samples() {
     run_check_parse_test(
         "sample-data/sandbox_a/policies_1.cedar",
+        "sample-data/sandbox_a/schema.cedarschema",
+        None::<PathBuf>,
         CedarExitCode::Success,
     );
     run_authorize_test(
@@ -208,6 +223,8 @@ fn test_authorize_samples() {
     );
     run_check_parse_test(
         "sample-data/sandbox_a/policies_2.cedar",
+        "sample-data/sandbox_a/schema.cedarschema",
+        None::<PathBuf>,
         CedarExitCode::Success,
     );
     run_authorize_test(
@@ -276,6 +293,8 @@ fn test_authorize_samples() {
     );
     run_check_parse_test(
         "sample-data/sandbox_a/policies_3.cedar",
+        "sample-data/sandbox_a/schema.cedarschema",
+        None::<PathBuf>,
         CedarExitCode::Success,
     );
     run_authorize_test(
@@ -329,6 +348,8 @@ fn test_authorize_samples() {
 
     run_check_parse_test(
         "sample-data/sandbox_b/policies_4.cedar",
+        "sample-data/sandbox_b/schema.cedarschema",
+        None::<PathBuf>,
         CedarExitCode::Success,
     );
     run_authorize_test(
@@ -365,6 +386,8 @@ fn test_authorize_samples() {
     );
     run_check_parse_test(
         "sample-data/sandbox_b/policies_5.cedar",
+        "sample-data/sandbox_b/schema.cedarschema",
+        None::<PathBuf>,
         CedarExitCode::Success,
     );
     run_authorize_test(
diff --git a/cedar-policy-core/src/test_utils.rs b/cedar-policy-core/src/test_utils.rs
index 710a7532..c18798be 100644
--- a/cedar-policy-core/src/test_utils.rs
+++ b/cedar-policy-core/src/test_utils.rs
@@ -132,6 +132,20 @@ impl<'a> ExpectedErrorMessageBuilder<'a> {
         }
     }
 
+    /// Add expected underlined text. The error message will be expected to have
+    /// exactly this many miette labels, and for each label (a, b), the underlined
+    /// portion should be `a` and the label text should be `b` (or `None` for no
+    /// label text).
+    pub fn with_underlines_or_labels(
+        self,
+        labels: impl IntoIterator<Item = (&'a str, Option<&'a str>)>,
+    ) -> Self {
+        Self {
+            underlines: labels.into_iter().collect(),
+            ..self
+        }
+    }
+
     /// Add expected contents of `source()`, or expected prefix of `source()` if
     /// this builder was originally constructed with `error_starts_with()`
     pub fn source(self, msg: &'a str) -> Self {
diff --git a/cedar-policy-validator/src/cedar_schema/test.rs b/cedar-policy-validator/src/cedar_schema/test.rs
index f502c277..115f7419 100644
--- a/cedar-policy-validator/src/cedar_schema/test.rs
+++ b/cedar-policy-validator/src/cedar_schema/test.rs
@@ -199,6 +199,50 @@ mod demo_tests {
         });
     }
 
+    #[test]
+    fn empty_principal() {
+        let src = r#"
+            entity a;
+            entity b;
+            action Foo appliesTo {
+                principal: [],
+                resource: [a, b]
+            };
+        "#;
+        assert_matches!(collect_warnings(json_schema::Fragment::from_cedarschema_str(src, Extensions::all_available())), Err(e) => {
+            expect_err(
+                src,
+                &miette::Report::new(e),
+                &ExpectedErrorMessageBuilder::error("error parsing schema: for action `Foo`, `principal` is `[]`, which is invalid")
+                    .with_underlines_or_labels([("Foo", Some("for this action")), ("principal: []", Some("must not be `[]`"))])
+                    .help(NO_PR_HELP_MSG)
+                    .build(),
+            );
+        });
+    }
+
+    #[test]
+    fn empty_resource() {
+        let src = r#"
+            entity a;
+            entity b;
+            action Foo appliesTo {
+                principal: [a, b],
+                resource: []
+            };
+        "#;
+        assert_matches!(collect_warnings(json_schema::Fragment::from_cedarschema_str(src, Extensions::all_available())), Err(e) => {
+            expect_err(
+                src,
+                &miette::Report::new(e),
+                &ExpectedErrorMessageBuilder::error("error parsing schema: for action `Foo`, `resource` is `[]`, which is invalid")
+                    .with_underlines_or_labels([("Foo", Some("for this action")), ("resource: []", Some("must not be `[]`"))])
+                    .help(NO_PR_HELP_MSG)
+                    .build(),
+            );
+        });
+    }
+
     #[test]
     fn both_targets() {
         let src = r#"
@@ -278,7 +322,7 @@ mod demo_tests {
                 r#"error parsing schema: duplicate `principal` declaration in action `Foo`"#
             ).help(
                 "Actions may only have a single principal declaration, but a principal declaration may specify a list of entity types like `principal: [X, Y, Z]`"
-            ).exactly_two_underlines("principal: [a, b],", "principal : [c]").build());
+            ).exactly_two_underlines("principal: [a, b]", "principal : [c]").build());
         });
     }
 
@@ -299,7 +343,7 @@ mod demo_tests {
                 r#"error parsing schema: duplicate `resource` declaration in action `Foo`"#
             ).help(
                 "Actions may only have a single resource declaration, but a resource declaration may specify a list of entity types like `resource: [X, Y, Z]`"
-            ).exactly_two_underlines("resource: [a, b],", "resource: [c]").build());
+            ).exactly_two_underlines("resource: [a, b]", "resource: [c]").build());
         });
     }
 
@@ -1040,13 +1084,13 @@ mod parser_tests {
     action A in [B, C] appliesTo { context: {}};
 "#,
         );
-        assert_matches!(res, Ok(_));
+        assert_matches!(res, Ok(_)); // becomes an error in later processing
         let res = parse_schema(
             r#"
     action A in [B, C] appliesTo { principal: []};
 "#,
         );
-        assert_matches!(res, Err(_));
+        assert_matches!(res, Ok(_)); // becomes an error in later processing
         let res = parse_schema(
             r#"
     action A in [B, C] appliesTo { principal: X, resource: [Y]};