Skip to content

Adds examples and integration tests for cedar-lean-cli's analysis capabilities.#638

Merged
chaluli merged 2 commits into
cedar-policy:mainfrom
chaluli:main
Jun 23, 2025
Merged

Adds examples and integration tests for cedar-lean-cli's analysis capabilities.#638
chaluli merged 2 commits into
cedar-policy:mainfrom
chaluli:main

Conversation

@chaluli

@chaluli chaluli commented Jun 13, 2025

Copy link
Copy Markdown

Adds examples to cedar-lean-cli for analysis. Uses examples to write integration tests. Add running integration tests to CI.

Updates analysis to output tabular results in a deterministic order (to make tests pass).

Updates cedar-lean-cli to change how vacuous policy findings are printed to make the text easier to understand. (applies to all requests instead of (permits) allows all / (forbids) denies all and applies to no requests instead of (permits) denies all / (forbids) allows all).

…abilities.

Signed-off-by: Charlie Murphy <mutmoth@amazon.com>
Signed-off-by: Charlie Murphy <mutmoth@amazon.com>

@adpaco adpaco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

I like output files to end with newlines and it looked like you were trimming in check_output so adding them shouldn't be an issue, right?

Comment on lines +196 to +201
match &self.principal_type {
Some(req_principal_type) if req_principal_type != principal_type => {
continue
}
_ => (),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
match &self.principal_type {
Some(req_principal_type) if req_principal_type != principal_type => {
continue
}
_ => (),
};
if let Some(req_principal_type) = &self.principal_type && req_principal_type != principal_type {
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we check and extract the value in &self.principal_type before going into this loop? Then you wouldn't need to have an if let Some(...) check in each iteration.

Comment on lines +202 to 207
if let Some(resource_types) = schema.resources_for_action(&action_id) {
for resource_type in resource_types {
match &self.resource_type {
Some(req_resource_type) if req_resource_type != resource_type => {
continue
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approximately same comments apply here.

Comment on lines +28 to +34
let output = Command::new("cedar-lean-cli")
.arg("analyze")
.arg("policies")
.arg(base_path.join("policies1.cedar"))
.arg(base_path.join("policies.cedarschema"))
.output()
.expect("Failed to run cedar-lean-cli");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have a helper to build these commands so you only pass policies1.cedar and policies.cedarschema to it here, for example.

use std::path::{Path, PathBuf};
use std::process::{Command, Output};

fn check_output<P: AsRef<Path>>(output: Output, expected_output_file: P, should_error: bool) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have any test with should_error set to true, right?

@chaluli chaluli merged commit 32f8dc7 into cedar-policy:main Jun 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants