Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix]: cfn-guard no longer errors out when rule files are empty, or contain only comments #402

Merged
merged 2 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions guard/resources/validate/comments.guard
joshfried-aws marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# this is a comment

3 changes: 2 additions & 1 deletion guard/src/commands/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn validate_and_return_json(

let rules_file_name = rules.file_name;
return match crate::rules::parser::rules_file(span) {
Ok(rules) => {
Ok(Some(rules)) => {
let mut write_output = BufWriter::new(Vec::new());
let root = input_data.path_value;
let traversal = Traversal::from(&root);
Expand Down Expand Up @@ -79,6 +79,7 @@ pub fn validate_and_return_json(
Err(e) => Err(Error::ParseError(e.to_string())),
}
}
Ok(None) => Ok(String::default()),
Err(e) => Err(Error::ParseError(e.to_string())),
};
}
6 changes: 4 additions & 2 deletions guard/src/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ or failure testing.
writeln!(writer, "Parse Error on ruleset file {e}",)?;
exit_code = 1;
}
Ok(rules) => {
Ok(Some(rules)) => {
let data_test_files = each_rule_file
.test_files
.iter()
Expand All @@ -232,6 +232,7 @@ or failure testing.
exit_code
}
}
Ok(None) => {}
}
writeln!(writer, "---")?;
}
Expand Down Expand Up @@ -283,13 +284,14 @@ or failure testing.
writeln!(writer, "Parse Error on ruleset file {e}")?;
exit_code = 1;
}
Ok(rules) => {
Ok(Some(rules)) => {
let curr_exit_code =
test_with_data(&data_test_files, &rules, verbose, writer)?;
if curr_exit_code != 0 {
exit_code = curr_exit_code;
}
}
Ok(None) => {}
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions guard/src/commands/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ fn evaluate_rule(
return Ok(5);
}

Ok(rule) => {
Ok(Some(rule)) => {
let status = evaluate_against_data_input(
data_type,
output,
Expand All @@ -558,6 +558,7 @@ fn evaluate_rule(
return Ok(19);
}
}
Ok(None) => return Ok(0),
}

Ok(0)
Expand All @@ -577,7 +578,10 @@ fn deserialize_payload(payload: &str) -> Result<Payload> {
}
}

fn parse_rules<'r>(rules_file_content: &'r str, rules_file_name: &'r str) -> Result<RulesFile<'r>> {
fn parse_rules<'r>(
rules_file_content: &'r str,
rules_file_name: &'r str,
) -> Result<Option<RulesFile<'r>>> {
let span = crate::rules::parser::Span::new_extra(rules_file_content, rules_file_name);
crate::rules::parser::rules_file(span)
}
Expand Down
3 changes: 2 additions & 1 deletion guard/src/commands/validate/structured.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ impl<'eval> StructuredEvaluator<'eval> {
))?;
self.exit_code = 5;
}
Ok(rule) => rules.push(rule),
Ok(Some(rule)) => rules.push(rule),
Ok(None) => {}
}
Ok(rules)
},
Expand Down
2 changes: 1 addition & 1 deletion guard/src/rules/evaluate_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ rule iam_role_lambda_compliance when iam_role_exists {

fn parse_rules<'c>(rules: &'c str, name: &'c str) -> Result<RulesFile<'c>> {
let span = Span::new_extra(rules, name);
rules_file(span)
Ok(rules_file(span)?.unwrap())
}

fn read_data(file: File) -> Result<Value> {
Expand Down
19 changes: 15 additions & 4 deletions guard/src/rules/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,18 @@ pub(crate) fn get_rule_name<'b>(rule_file_name: &str, rule_name: &'b str) -> &'b
//
// Rules File
//
pub(crate) fn rules_file(input: Span) -> Result<RulesFile, Error> {
pub(crate) fn rules_file(input: Span) -> Result<Option<RulesFile>, Error> {
let input = match zero_or_more_ws_or_comment(input) {
Ok(input) => {
if input.0.is_empty() {
return Ok(None);
}

input.0
}
Err(_) => input,
};

let exprs = all_consuming(fold_many1(
remove_whitespace_comments(alt((
map(assignment, Exprs::Assignment),
Expand Down Expand Up @@ -1866,11 +1877,11 @@ pub(crate) fn rules_file(input: Span) -> Result<RulesFile, Error> {
named_rules.insert(0, default_rule);
}

Ok(RulesFile {
Ok(Some(RulesFile {
assignments: global_assignments,
guard_rules: named_rules,
parameterized_rules,
})
}))
}

//
Expand Down Expand Up @@ -1986,7 +1997,7 @@ impl<'a> TryFrom<&'a str> for RulesFile<'a> {

fn try_from(value: &'a str) -> Result<Self, Self::Error> {
let span = from_str2(value);
rules_file(span)
Ok(rules_file(span)?.unwrap())
}
}

Expand Down
4 changes: 2 additions & 2 deletions guard/src/rules/parser_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3643,11 +3643,11 @@ fn test_rules_file_default_rules() -> Result<(), Error> {
let rules_file = rules_file(from_str2(s))?;
assert_eq!(
rules_file,
RulesFile {
Some(RulesFile {
assignments: vec![],
guard_rules: vec![default_rule],
parameterized_rules: vec![],
}
})
);
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions guard/tests/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

pub(crate) mod utils;

#[cfg(test)]
mod validate_tests {
use std::io::{stderr, stdout, Cursor};
Expand Down Expand Up @@ -187,11 +186,11 @@ mod validate_tests {
)]
#[case(vec!["s3-server-side-encryption-template-non-compliant-2.yaml"], vec!["malformed-rule.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(vec!["malformed-template.yaml"], vec!["s3_bucket_server_side_encryption_enabled_2.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(vec!["s3-server-side-encryption-template-non-compliant-2.yaml"], vec!["blank-rule.guard"], StatusCode::PARSING_ERROR)]
#[case(vec!["s3-server-side-encryption-template-non-compliant-2.yaml"], vec!["blank-rule.guard"], StatusCode::SUCCESS)]
#[case(
vec!["s3-server-side-encryption-template-non-compliant-2.yaml"],
vec!["s3_bucket_server_side_encryption_enabled_2.guard", "blank-rule.guard"],
StatusCode::PARSING_ERROR
StatusCode::VALIDATION_ERROR
)]
#[case(vec!["blank-template.yaml"], vec!["s3_bucket_server_side_encryption_enabled_2.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(
Expand All @@ -202,6 +201,7 @@ mod validate_tests {
#[case(vec!["dne.yaml"], vec!["rules-dir/s3_bucket_public_read_prohibited.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(vec!["data-dir/s3-public-read-prohibited-template-non-compliant.yaml"], vec!["dne.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(vec!["blank.yaml"], vec!["rules-dir/s3_bucket_public_read_prohibited.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(vec!["s3-server-side-encryption-template-non-compliant-2.yaml"], vec!["comments.guard"], StatusCode::SUCCESS)]
fn test_single_data_file_single_rules_file_status(
#[case] data_arg: Vec<&str>,
#[case] rules_arg: Vec<&str>,
Expand Down