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]: Loader can panic when handling nested maps #415

Merged
merged 3 commits into from
Nov 17, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ Resources:
- ServerSideEncryptionByDefault:
SSEAlgorithm: AES256
VersioningConfiguration:
Status: Enabled
Status: Enabled
19 changes: 19 additions & 0 deletions guard/resources/validate/nested_crash.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# ---
# AWSTemplateFormatVersion: 2010-09-09
# Description: CloudFormation - Server Side Encryption

Resources:
MyBucket:
Type: AWS::S3::Bucket
Properties:
PublicAccessBlockConfiguration:
BlockPublicAcls: true
BlockPublicPolicy: true
IgnorePublicAcls: true
RestrictPublicBuckets: true
BucketEncryption:
ServerSideEncryptionConfiguration:
- ServerSideEncryptionByDefault:
SSEAlgorithm: { { CRASH } }
VersioningConfiguration:
Status: Enabled
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ rule S3_BUCKET_SERVER_SIDE_ENCRYPTION_ENABLED when %s3_buckets_server_side_encry
Violation: S3 Bucket must enable server-side encryption.
Fix: Set the S3 Bucket property BucketEncryption.ServerSideEncryptionConfiguration.ServerSideEncryptionByDefault.SSEAlgorithm to either "aws:kms" or "AES256"
>>
}
}
19 changes: 10 additions & 9 deletions guard/src/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ or failure testing.
.parent()
.map_or("".to_string(), |p| format!("{}", p.display())),
)
.or_insert(vec![])
.or_default()
.push(GuardFile {
prefix,
file,
Expand Down Expand Up @@ -356,14 +356,15 @@ fn test_with_data(
eval_rules_file(rules, &mut root_scope, None)?; // we never use data file name in the output
let top = root_scope.reset_recorder().extract();

let by_rules = top.children.iter().fold(HashMap::new(), |mut acc, rule| {
if let Some(RecordType::RuleCheck(NamedStatus { name, .. })) =
rule.container
{
acc.entry(name).or_insert(vec![]).push(&rule.container)
}
acc
});
let by_rules: HashMap<&str, Vec<&Option<RecordType<'_>>>> =
top.children.iter().fold(HashMap::new(), |mut acc, rule| {
if let Some(RecordType::RuleCheck(NamedStatus { name, .. })) =
rule.container
{
acc.entry(name).or_default().push(&rule.container)
}
acc
});

for (rule_name, rule) in by_rules {
let expected = match each.expectations.rules.get(rule_name) {
Expand Down
4 changes: 2 additions & 2 deletions guard/src/commands/validate/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,12 +709,12 @@ pub(super) fn insert_into_trees<'report, 'value: 'report>(

if let Some(from) = clause.value_from() {
let path = from.self_path().0.to_string();
path_tree.entry(path).or_insert(vec![]).push(node.clone());
path_tree.entry(path).or_default().push(node.clone());
}

if let Some(from) = clause.value_to() {
let path = from.self_path().0.to_string();
path_tree.entry(path).or_insert(vec![]).push(node);
path_tree.entry(path).or_default().push(node);
}
}

Expand Down
2 changes: 0 additions & 2 deletions guard/src/rules/functions/converters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ pub(crate) fn parse_float(
}
PathAwareValue::Char((path, val)) => {
aggr.push(Some(PathAwareValue::Float((path.clone(), {
let path = path;
val.to_digit(10).ok_or(crate::Error::ParseError(format!(
"failed to convert a character: {val} into a float at {path}"
)))
Expand Down Expand Up @@ -69,7 +68,6 @@ pub(crate) fn parse_int(args: &[QueryResult]) -> crate::rules::Result<Vec<Option
}
PathAwareValue::Char((path, val)) => {
aggr.push(Some(PathAwareValue::Int((path.clone(), {
let path = path;
val.to_digit(10).ok_or(crate::Error::ParseError(format!(
"failed to convert a character: {val} into an integer at {path}"
)))
Expand Down
2 changes: 2 additions & 0 deletions guard/src/rules/libyaml/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,10 @@ impl Loader {
let value = key_values.remove(0);
let key_str = match key {
MarkedValue::String(val, loc) => (val, loc),
MarkedValue::Map(..) => continue,
_ => unreachable!(),
};

map.insert(key_str, value);
}
}
Expand Down
2 changes: 1 addition & 1 deletion guard/src/rules/path_value_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ fn merge_values_test() -> Result<(), Error> {
_ => unreachable!(),
};
assert_eq!(resources_map.values.len(), 2);
assert!(matches!(resources_map.values.get("PARAMETERS"), Some(_)),);
assert!(resources_map.values.get("PARAMETERS").is_some());

let parameters = PathAwareValue::try_from(serde_yaml::from_str::<serde_yaml::Value>(
r#"
Expand Down
2 changes: 0 additions & 2 deletions guard/tests/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ mod test_command_tests {
args.push(String::from(self.rules_and_test_file.unwrap()));
}

if self.directory_only {}

if self.alphabetical {
args.push(format!("-{}", ALPHABETICAL.1));
}
Expand Down
6 changes: 3 additions & 3 deletions guard/tests/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ mod validate_tests {
#[case(vec!["blank-template.yaml"], vec!["s3_bucket_server_side_encryption_enabled_2.guard"], StatusCode::INTERNAL_FAILURE)]
#[case(
vec!["blank-template.yaml", "s3-server-side-encryption-template-non-compliant-2.yaml"],
vec!["s3_bucket_server_side_encryption_enabled_2.guard"],
StatusCode::INTERNAL_FAILURE
)]
vec!["s3_bucket_server_side_encryption_enabled_2.guard"], StatusCode::INTERNAL_FAILURE)]
#[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)]
#[case(vec!["s3-server-side-encryption-template-non-compliant-2.yaml"], vec!["comments.guard"], StatusCode::SUCCESS)]
#[case(vec!["nested_crash.yaml"], vec!["s3_bucket_server_side_encryption_enabled_2.guard"], StatusCode::VALIDATION_ERROR)]
fn test_single_data_file_single_rules_file_status(
#[case] data_arg: Vec<&str>,
#[case] rules_arg: Vec<&str>,
Expand Down
Loading