aws-cloudformation / aws-cloudformation/cloudformation-guard
[BUG] Capitalized YAML booleans ('True', 'TRUE', 'Yes', etc.) are parsed as strings, causing false-positive rule failures
- 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
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