aws-cloudformation / aws-cloudformation/cloudformation-guard

[BUG] Capitalized YAML booleans ('True', 'TRUE', 'Yes', etc.) are parsed as strings, causing false-positive rule failures

Open Beginner friendly
#744 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
1.4k
Forks
196
Avg merge
3d 6h
Merged PRs (30d)
5

Description

### Describe the bug
In YAML templates, capitalized boolean values such as `True`, `TRUE`, `Yes`, `YES`, `False`, `FALSE`, `No`, and `Off` are parsed as `MarkedValue::String` rather than `MarkedValue::Bool`.

Consequently, any rule performing boolean equality (`== true` or `== false`) against these fields fails due to a type mismatch between `String` and `Bool`.

### Root Cause
PR #633 introduced `is_bool_true` and `is_bool_false` in `guard/src/rules/libyaml/loader.rs`:

https://github.com/aws-cloudformation/cloudformation-guard/blob/3e265bb6ad26090412b189ba7145719e7e7a8585/guard/src/rules/libyaml/loader.rs#L103-L119

```rust
fn is_bool_true(&self, s: &str) -> bool {
matches!(s, "true" | "yes" | "on" | "y")
}

fn is_bool_false(&self, s: &str) -> bool {
matches!(s, "false" | "no" | "off" | "n")
}
```

`s` is compared case-sensitively without being converted to lowercase. As a result, `"True"`, `"TRUE"`, `"Yes"`, etc., fail the match and fall through to `MarkedValue::String(val, location)` at line 94.

#### Why existing tests in `loader_tests.rs` did not catch this:
In `guard/src/rules/libyaml/loader_tests.rs`:

https://github.com/aws-cloudformation/cloudformation-guard/blob/3e265bb6ad26090412b189ba7145719e7e7a8585/guard/src/rules/libyaml/loader_tests.rs#L54-L68

```rust
#[rstest::rstest]
#[case::standard_lowercase_true("true", true)]
#[case::standard_capitalized_true("True", true)]
#[case::standard_uppercase_true("TRUE", true)]
...
fn test_handle_bool_happy_path(#[case] arg: &str, #[case] expected: bool) -> Result<()> {
let docs = format!("check: {arg}");
let mut loader = Loader::new();
match loader.load(String::from(docs))? {
MarkedValue::Map(map, ..) => {
assert!(map.len() == 1);
let (.., result) = map.first().unwrap();

if let MarkedValue::Bool(result, ..) = *result {
assert_eq!(result, expected);
}
}
_ => unreachable!("this isn't possible"),
}
Ok(())
}
```
Because the assertion is wrapped inside `if let MarkedValue::Bool(result, ..) = *result`, when `arg` is `"True"` or `"TRUE"`, `*result` is `MarkedValue::String("True")`. The `if let` condition silently fails to match, the assertion is skipped, and the test exits `Ok(())`.

### To Reproduce
**Template (`template.yaml`):**
```yaml
Resources:
MyBucket:
Type: AWS::S3::Bucket
Properties:
BucketEncryption:
ServerSideEncryptionConfiguration:
- ServerSideEncryptionByDefault:
Enabled: True
```

**Rules (`rule.guard`):**
```guard
let s3 = Resources.*[ Type == 'AWS::S3::Bucket' ]

rule check_bucket_encryption when %s3 !empty {
%s3.Properties.BucketEncryption.ServerSideEncryptionConfiguration[*].ServerSideEncryptionByDefault.Enabled == true
}
```

**Command:**
```bash
cfn-guard validate --data template.yaml --rules rule.guard
```

**Result:**
The check fails with a comparison failure between `String("True")` and `Bool(true)`.

### Expected Behavior
`True`, `TRUE`, `Yes`, `On`, `False`, `FALSE`, `No`, `Off` should parse as `MarkedValue::Bool`.

### Proposed Fix

1. In `guard/src/rules/libyaml/loader.rs`:
```rust
fn is_bool_true(&self, s: &str) -> bool {
matches!(s.to_ascii_lowercase().as_str(), "true" | "yes" | "on" | "y")
}

fn is_bool_false(&self, s: &str) -> bool {
matches!(s.to_ascii_lowercase().as_str(), "false" | "no" | "off" | "n")
}
```

2. In `guard/src/rules/libyaml/loader_tests.rs`:
Replace the vacuous `if let` with an explicit match asserting `MarkedValue::Bool`:
```rust
match *result {
MarkedValue::Bool(result, ..) => assert_eq!(result, expected),
ref other => panic!("expected MarkedValue::Bool, got {:?}", other),
}
```

Contributor guide

Open the contributing guide

Research direction

Start with guard/src/rules/libyaml/loader.rs and the boolean cases in guard/src/rules/libyaml/loader_tests.rs. Run the loader tests first, then verify the listed capitalized YAML values become MarkedValue::Bool and that the tests fail explicitly for any other value type. Done means boolean equality rules validate these values without a String/Bool mismatch.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.