aws-cloudformation / cloudformation-guard

Guard offers a policy-as-code domain-specific language (DSL) to write rules and validate JSON- and YAML-formatted data such as CloudFormation Templates, K8s configurations, and Terraform JSON plans/configurations against those rules. Take this survey to provide feedback about cfn-guard: https://amazonmr.au1.qualtrics.com/jfe/form/SV_bpyzpfoYGGuuUl0
Apache License 2.0
1.29k stars 180 forks source link

[BUG] Error message includes details of unrelated resource #202

Closed alexpulver closed 2 years ago

alexpulver commented 3 years ago

Describe the bug When adding a custom error message, cfn-guard also prints details of unrelated resource from the data.

To Reproduce Please supply:

  1. Example rules and template that results in the error

LandingPageFrontend.guard

let aws_s3_bucket_resources = Resources.*[ Type == 'AWS::S3::Bucket' ]

rule aws_s3_bucket_versioning_defined {
  when %aws_s3_bucket_resources not empty {
    %aws_s3_bucket_resources {
      Properties {
        VersioningConfiguration exists <<VersioningConfiguration is not defined>>
      }
    }
  }
}

LandingPageFrontend.template.json

{
  "Resources": {
    "WebsiteBucket4326D7C2": {
      "Type": "AWS::S3::Bucket",
      "Properties": {
      },
      "Metadata": {
        "aws:cdk:path": "LandingPageFrontend/Website/Bucket/Resource"
      }
    }
  }
}
  1. The commands you used to invoke the tool
    cfn-guard validate -d LandingPageFrontend.template.json -r LandingPageFrontend.guard
  2. The output of a -v log level if it's not related to cfn-guard-lambda, or the relevant CloudWatch log messages if it is related to the cfn-guard-lambda
    LandingPageFrontend.template.json Status = FAIL
    FAILED rules
    LandingPageFrontend.guard/aws_s3_bucket_versioning_defined    FAIL
    ---
    Evaluation of rules LandingPageFrontend.guard against data LandingPageFrontend.template.json
    --
    Property [/Resources/WebsiteBucket4326D7C2/Properties] in data [LandingPageFrontend.template.json] is not compliant with [LandingPageFrontend.guard/aws_s3_bucket_versioning_defined] because needed value at [{}] did not exist. Error Message [VersioningConfiguration is not defined;Metadata: [String((Path("/Resources/WebsiteBucket4326D7C2/Metadata/aws:cdk:path"), "LandingPageFrontend/Website/Bucket/Resource"))]]
    --
    Evaluation Tree
    Rule(aws_s3_bucket_versioning_defined, FAIL)
    |  Message: DEFAULT MESSAGE(FAIL)
    Condition(, PASS)
        |  Message: DEFAULT MESSAGE(PASS)
        Clause(Clause(Location[file:LandingPageFrontend.guard, line:4, column:8], Check: %aws_s3_bucket_resources NOT EMPTY ), PASS)
            |  From: Map((Path("/Resources/WebsiteBucket4326D7C2"), MapValue { keys: [String((Path("/Resources/WebsiteBucket4326D7C2/Type"), "Type")), String((Path("/Resources/WebsiteBucket4326D7C2/Properties"), "Properties")), String((Path("/Resources/WebsiteBucket4326D7C2/Metadata"), "Metadata"))], values: {"Type": String((Path("/Resources/WebsiteBucket4326D7C2/Type"), "AWS::S3::Bucket")), "Properties": Map((Path("/Resources/WebsiteBucket4326D7C2/Properties"), MapValue { keys: [], values: {} })), "Metadata": Map((Path("/Resources/WebsiteBucket4326D7C2/Metadata"), MapValue { keys: [String((Path("/Resources/WebsiteBucket4326D7C2/Metadata/aws:cdk:path"), "aws:cdk:path"))], values: {"aws:cdk:path": String((Path("/Resources/WebsiteBucket4326D7C2/Metadata/aws:cdk:path"), "LandingPageFrontend/Website/Bucket/Resource"))} }))} }))
            |  Message: DEFAULT MESSAGE(PASS)
    ConditionBlock(, FAIL)
        |  Message: DEFAULT MESSAGE(FAIL)
        Conjunction(cfn_guard::rules::exprs::GuardClause, FAIL)
            |  Message: DEFAULT MESSAGE(FAIL)
            BlockClause(Block[Location[file:LandingPageFrontend.guard, line:5, column:5]], FAIL)
                |  Message: DEFAULT MESSAGE(FAIL)
                Conjunction(cfn_guard::rules::exprs::GuardClause, FAIL)
                    |  Message: DEFAULT MESSAGE(FAIL)
                    BlockClause(Block[Location[file:LandingPageFrontend.guard, line:6, column:7]], FAIL)
                        |  Message: DEFAULT MESSAGE(FAIL)
                        Conjunction(cfn_guard::rules::exprs::GuardClause, FAIL)
                            |  Message: DEFAULT MESSAGE(FAIL)
                            Clause(Clause(Location[file:LandingPageFrontend.guard, line:7, column:9], Check: VersioningConfiguration  EXISTS ), FAIL)
                                |  From: Map((Path("/Resources/WebsiteBucket4326D7C2/Properties"), MapValue { keys: [], values: {} }))
                                |  Message: VersioningConfiguration is not defined
    Metadata: [String((Path("/Resources/WebsiteBucket4326D7C2/Metadata/aws:cdk:path"), "LandingPageFrontend/Website/Bucket/Resource"))]

NOTE: Please be sure that the templates, rules and logs you provide as part of your bug report do not contain any sensitive information.

Expected behavior A clear and concise description of what you expected to happen.

If I remove the Metadata resource from the template, I get a clean failure message, without the additional details. That is the expected behavior if Metadata would be in the template.

Property [/Resources/WebsiteBucket4326D7C2/Properties] in data [LandingPageFrontend.template.json] is not compliant with [LandingPageFrontend.guard/aws_s3_bucket_versioning_defined] because needed value at [{}] did not exist. Error Message [VersioningConfiguration is not defined]

Operating System: [eg, MacOS, Windows, Ubuntu, etc]

macOS

OS Version [eg Catalina, 10, 18.04, etc]

Catalina 10.15.7

Additional context Add any other context about the problem here.

dchakrav-github commented 3 years ago

Thanks for the bug report @alexpulver. The metadata is just appended information for CDK. The error message still displays

Clause(Clause(Location[file:LandingPageFrontend.guard, line:7, column:9], Check: VersioningConfiguration  EXISTS ), FAIL)
                                |  From: Map((Path("/Resources/WebsiteBucket4326D7C2/Properties"), MapValue { keys: [], values: {} }))
                                |  Message: VersioningConfiguration is not defined

VersioningConfiguration is not defined, but also appends CDK data. Would you prefer that be optional?

alexpulver commented 3 years ago

Thanks for clarifying @dchakrav-github! I wasn't sure if Guard appended the AWS CDK metadata purposefully or accidentally. The message formatting is a bit too verbose - contains Guard internal structure instead of a string. Also, having that metadata not as part of the error message would be helpful. That's what mainly caused me to think it's a bug, as I included nothing in the error message to have this appear. I saw your comment on #204 - if you think that approach makes sense; I think it would be preferable to the current behavior.

Error Message [VersioningConfiguration is not defined;Metadata: [String((Path("/Resources/WebsiteBucket4326D7C2/Metadata/aws:cdk:path"), "LandingPageFrontend/Website/Bucket/Resource"))]]
razcloud commented 2 years ago

Please check out the Guard 2.1.0 Release that should resolve this issue.

alexpulver commented 2 years ago

Thanks @razcloud! It looks great now.

P.S.: There seems to be a space missing after the "Message" string, but it's a minor thing 😃 .

$ cfn-guard validate -d template.json -r rules.guard
template.json Status = FAIL
FAILED rules
rules.guard/aws_s3_bucket_versioning_defined    FAIL
---
Evaluating data template.json against rules rules.guard
Number of non-compliant resources 1
Resource = WebsiteBucket4326D7C2 {
  Type      = AWS::S3::Bucket
  CDK-Path  = LandingPageFrontend/Website/Bucket/Resource
  Rule = aws_s3_bucket_versioning_defined {
    ALL {
      Check =  VersioningConfiguration EXISTS   {
        Message= VersioningConfiguration is not defined
        RequiredPropertyError {
          PropertyPath = /Resources/WebsiteBucket4326D7C2/Properties[L:5,C:20]
          MissingProperty = VersioningConfiguration
          Reason = Could not find key VersioningConfiguration inside struct at path /Resources/WebsiteBucket4326D7C2/Properties[L:5,C:20]
          Code:
                3.    "WebsiteBucket4326D7C2": {
                4.      "Type": "AWS::S3::Bucket",
                5.      "Properties": {
                6.        "AccessControl": "PublicRead"
                7.      },
                8.      "Metadata": {
        }
      }
    }
  }
}
razcloud commented 2 years ago

@alexpulver Thank you for letting us know about that! Will have that corrected.