Error messages need to be set for each check

[corymhall]

What is the problem?

When writing a rule, any error message needs to be applied to every check.

So for example the s3_bucket_versioning_enabled rule is written like this:

rule S3_BUCKET_VERSIONING_ENABLED when %s3_buckets_versioning_enabled !empty {
  %s3_buckets_versioning_enabled.Properties.VersioningConfiguration exists
  %s3_buckets_versioning_enabled.Properties.VersioningConfiguration.Status == 'Enabled'
  <<
    Violation: S3 Bucket Versioning must be enabled.
    Fix: Set the S3 Bucket property VersioningConfiguration.Status to 'Enabled' .
  >>
}

The custom message there will only be shown in the JSON response for the %s3_buckets_versioning_enabled.Properties.VersioningConfiguration.Status == 'Enabled' check. If it fails at the exists check then the error message won't be shown.

Reproduction Steps

template

{
 "Resources": {
  "MyBucket": {
   "Type": "AWS::S3::Bucket",
   "Properties": {}
  }
}

rule s3_bucket_versioning_enabled

rule S3_BUCKET_VERSIONING_ENABLED when %s3_buckets_versioning_enabled !empty {
  %s3_buckets_versioning_enabled.Properties.VersioningConfiguration exists
  %s3_buckets_versioning_enabled.Properties.VersioningConfiguration.Status == 'Enabled'
  <<
    Violation: S3 Bucket Versioning must be enabled.
    Fix: Set the S3 Bucket property VersioningConfiguration.Status to 'Enabled' .
  >>
}

cfn-guard validate --data path/to/template --rules /path/to/rule

What did you expect to happen?

I would expect the output for each check to contain the custom error message.

What actually happened?

{
  "name": "",
  "metadata": {},
  "status": "FAIL",
  "not_compliant": [
    {
      "Rule": {
        "name": "S3_BUCKET_VERSIONING_ENABLED",
        "metadata": {},
        "messages": {
          "custom_message": null,
          "error_message": null
        },
        "checks": [
          {
            "Clause": {
              "Unary": {
                "context": " %s3_buckets_versioning_enabled[*].Properties.VersioningConfiguration EXISTS  ",
                "messages": {
                  "custom_message": "",
                  "error_message": "Check was not compliant as property [VersioningConfiguration] is missing. Value traversed to [Path=/Resources/MyConstructBucketA5944A03/Properties[L:4,C:17] Value={\"PublicAccessBlockConfiguration\":{\"BlockPublicAcls\":false,\"BlockPublicPolicy\":false,\"IgnorePublicAcls\":false,\"RestrictPublicBuckets\":false}}]."
                },
                "check": {
                  "UnResolved": {
                    "value": {
                      "traversed_to": {
                        "path": "/Resources/MyConstructBucketA5944A03/Properties",
                        "value": {
                          "PublicAccessBlockConfiguration": {
                            "BlockPublicAcls": false,
                            "BlockPublicPolicy": false,
                            "IgnorePublicAcls": false,
                            "RestrictPublicBuckets": false
                          }
                        }
                      },
                      "remaining_query": "VersioningConfiguration",
                      "reason": "Could not find key VersioningConfiguration inside struct at path /Resources/MyConstructBucketA5944A03/Properties[L:4,C:17]"
                    },
                    "comparison": [
                      "Exists",
                      false
                    ]
                  }
                }
              }
            }
          },
          {
            "Clause": {
              "Binary": {
                "context": " %s3_buckets_versioning_enabled[*].Properties.VersioningConfiguration.Status EQUALS  \"Enabled\"",
                "messages": {
                  "custom_message": ";    Violation: S3 Bucket Versioning must be enabled.;    Fix: Set the S3 Bucket property VersioningConfiguration.Status to 'Enabled' .;  ",
                  "error_message": "Check was not compliant as property [VersioningConfiguration.Status] to compare from is missing. Value traversed to [Path=/Resources/MyConstructBucketA5944A03/Properties[L:4,C:17] Value={\"PublicAccessBlockConfiguration\":{\"BlockPublicAcls\":false,\"BlockPublicPolicy\":false,\"IgnorePublicAcls\":false,\"RestrictPublicBuckets\":false}}]."
                },
                "check": {
                  "UnResolved": {
                    "value": {
                      "traversed_to": {
                        "path": "/Resources/MyConstructBucketA5944A03/Properties",
                        "value": {
                          "PublicAccessBlockConfiguration": {
                            "BlockPublicAcls": false,
                            "BlockPublicPolicy": false,
                            "IgnorePublicAcls": false,
                            "RestrictPublicBuckets": false
                          }
                        }
                      },
                      "remaining_query": "VersioningConfiguration.Status",
                      "reason": "Could not find key VersioningConfiguration inside struct at path /Resources/MyConstructBucketA5944A03/Properties[L:4,C:17]"
                    },
                    "comparison": [
                      "Eq",
                      false
                    ]
                  }
                }
              }
            }
          }
        ]
      }
    }
  ],
  "not_applicable": [],
  "compliant": []
}

CloudFormation Guard Version

2.1.3

OS

Ubuntu

OS Version

No response

Other information

One solution is to wrap all the checks inside a rule check (example from Control Tower rules)

rule s3_version_lifecycle_policy_check %s3_buckets not empty {
    check(%s3_buckets.Properties)
        <<
        [CT.S3.PR.3]: Require an Amazon S3 buckets to have versioning configured and a lifecycle policy
        [FIX]: Configure versioning-enabled buckets with at least one active lifecycle rule.
        >>
}

rule check(s3_bucket) {
    %s3_bucket {
        VersioningConfiguration exists
        VersioningConfiguration is_struct

        VersioningConfiguration {
            Status exists
            Status == "Enabled"
        }
    }
}

[grolston]

Thank you for submitting this issue. The check is using a basic AND operation which was put in there for backward compatibility at the time for older versions of cfn-guard. We are working with the cfn-guard team to make this work for custom error messages.

With the current version a single property value check will complete the exists and value check in one operation. What we can do is remove the exists check to eliminate this issue. There are a few other rules this is done and we can update those as well.

[grolston]

@corymhall the issue you are seeing is something being worked on within cfn-guard. Due to that we have transferred the issue to the repo.

[joshfried-aws]

Hi @corymhall, your workaround is currently the best way to achieve your wishes is to encapsulate all the checks inside of a parametrized rule and then use that message.

I am going to go ahead and change the label from bug to enhancement. The reason for this is because this behaviour is as expected.