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.3k stars 180 forks source link

[BUG] cfn-guard-lambda breaks on parsing templates with `verbose: false` #536

Closed boonew2 closed 3 months ago

boonew2 commented 4 months ago

Describe the bug When invoking the cfnGuard lambda with "verbose": false in the payload it seems to break parsing of the template in a hard to pin down way.. Error from the lambda is:

{"errorType":"\u0026alloc::boxed::Box\u003cdyn core::error::Error + core::marker::Send + core::marker::Sync\u003e","errorMessage":"trailing characters at line 1 column 19"}

To Reproduce Please supply:

---
AWSTemplateFormatVersion: 2010-09-09
Transform:
  - AWS::Serverless-2016-10-31

Resources:
  ApiGateway:
    Type: AWS::Serverless::Api
    Properties:
      EndpointConfiguration: REGIONAL
      FailOnWarnings: false
      StageName: dev
      DefinitionBody:
        swagger: "2.0"
        info:
          title: !Sub "${AWS::StackName}_Api"
        definitions:
          Empty:
            type: "object"
            title: "Empty Schema"
        securityDefinitions:
          sigv4:
            type: apiKey
            name: Authorization
            in: header
            x-amazon-apigateway-authtype: awsSigv4
          schemes:
            - "https"
        paths:
          /public/{proxy+}:
            x-amazon-apigateway-any-method:
              produces:
              - "application/json"
              parameters:
              - name: "proxy"
                in: "path"
                required: true
                type: "string"
              responses: {}
              x-amazon-apigateway-integration:
                uri: !Sub "https://${LoadBalancerDns}/public/{proxy}"
                responses:
                  default:
                    statusCode: "200"
                requestParameters:
                  integration.request.path.proxy: "method.request.path.proxy"
                passthroughBehavior: "when_no_match"
                connectionType: "VPC_LINK"
                connectionId: !Ref ApiVpcLink
                httpMethod: "ANY"
                type: "http_proxy"
          /healthcheck:
            get:
              produces:
              - "application/json"
              responses: {}
              x-amazon-apigateway-integration:
                uri: !Sub "https://${LoadBalancerDns}/HealthCheck"
                responses:
                  default:
                    statusCode: "200"
                requestParameters:
                  integration.request.header.userArn: "'healthcheck'"
                passthroughBehavior: "when_no_match"
                connectionType: "VPC_LINK"
                httpMethod: "GET"
                connectionId: !Ref ApiVpcLink
                type: "http_proxy"
          /private/{proxy+}:
            x-amazon-apigateway-any-method:
              produces:
              - "application/json"
              parameters:
              - name: "proxy"
                in: "path"
                required: true
                type: "string"
              responses: {}
              security:
              - sigv4: []
              x-amazon-apigateway-integration:
                uri: !Sub "https://${LoadBalancerDns}/private/{proxy}"
                responses:
                  default:
                    statusCode: "200"
                requestParameters:
                  integration.request.path.proxy: "method.request.path.proxy"
                  integration.request.header.userArn: "context.identity.userArn"
                passthroughBehavior: "when_no_match"
                connectionType: "VPC_LINK"
                connectionId: !Ref ApiVpcLink
                httpMethod: "ANY"
                type: "http_proxy"

Sample ruleset

let apigateway_api = Resources.*[ Type IN ['AWS::ApiGateway::RestApi','AWS::ApiGatewayV2::Api','AWS::Serverless::Api']
  Metadata.guard.SuppressedRules not exists or
  Metadata.guard.SuppressedRules.* != "APIGATEWAY_FAIL_ON_WARNINGS"
]

rule APIGATEWAY_FAIL_ON_WARNINGS when %apigateway_api !empty {
  let violations = %apigateway_api[
    Properties.FailOnWarnings !exists
    OR
    Properties.FailOnWarnings == false
    OR
    Properties.FailOnWarnings == 'False'
  ]

  %violations empty
  <<
    Violation: Apigateway API must set FailOnWarnings to True
    Fix: Set FailOnWarnings property to True
  >>
}

I'm invoking the lambda through a small powershell wrapper while testing, but i'm pretty sure another language would have the same results:

function Invoke-CfnGuardLambda([string]$TemplatePath, [string]$RulesetPath, [switch]$Verbose){
    $res = Invoke-LMFunction -FunctionName cfnGuard -InvocationType RequestResponse -Payload (@{ data=(gc -raw "$TemplatePath" | out-string); rules=@(gc -raw "$RulesetPath" | Out-String); verbose = $Verbose.IsPresent} | ConvertTo-Json) -Region us-west-2
    [System.Text.Encoding]::UTF8.GetString($res.Payload.ToArray()) | convertFrom-Json
}
# this throws the error
Invoke-CfnGuardLambda -TemplatePath C:\temp\template.yml -RulesetPath C:\temp\rules.ruleset

errorType                                                                            errorMessage                           
---------                                                                            ------------                           
&alloc::boxed::Box<dyn core::error::Error + core::marker::Send + core::marker::Sync> trailing characters at line 1 column 19

# this returns the verbose response as expected
Invoke-CfnGuardLambda -Verbose | ConvertTo-Json -Compress

{"message":[{"context":"File(rules=1)","container":"@{FileCheck=}","children":""}]}

Logs: cloudwatch logs dump the template that i already pasted, the ruleset, and then the error message returned in the response. I don't want to spam this to much so i'll just include the ruleset/error logs:


2024-07-16T22:46:51.737Z INFO [cfn_guard_lambda] Rules are: [["#############################################\r\n# Rule Identifier:\r\n# APIGATEWAY_FAIL_ON_WARNINGS\r\n#\r\n# Description:\r\n# Checks if the FailOnWarnings property is set to true on apigateway apis\r\n#\r\n# Reports on:\r\n# AWS::ApiGateway::RestApi\r\n# AWS::ApiGatewayV2::Api\r\n# AWS::Serverless::Api\r\n#\r\n# Evaluates:\r\n# AWS CloudFormation\r\n#\r\n# Rule Parameters:\r\n# NA\r\n#\r\n# Scenarios:\r\n# a) SKIP: when there are no apigateway api resources present\r\n# b) PASS: when all apigateway apis have FailOnWarnings set to true\r\n# c) FAIL: when all apigateway apis do not have FailOnWarnings set to true\r\n# d) SKIP: when metadata includes the suppression for rule APIGATEWAY_FAIL_ON_WARNINGS\r\n\r\nlet apigateway_api = Resources.*[ Type IN ['AWS::ApiGateway::RestApi','AWS::ApiGatewayV2::Api','AWS::Serverless::Api']\r\n Metadata.guard.SuppressedRules not exists or\r\n Metadata.guard.SuppressedRules.* != \"APIGATEWAY_FAIL_ON_WARNINGS\"\r\n]\r\n\r\nrule APIGATEWAY_FAIL_ON_WARNINGS when %apigateway_api !empty {\r\n let violations = %apigateway_api[\r\n Properties.FailOnWarnings !exists\r\n OR\r\n Properties.FailOnWarnings == false\r\n OR\r\n Properties.FailOnWarnings == 'False'\r\n ]\r\n\r\n %violations empty\r\n <<\r\n Violation: Apigateway API must set FailOnWarnings to True\r\n Fix: Set FailOnWarnings property to True\r\n >>\r\n}\r\n#############################################\r\n"]]
2024-07-16T22:46:51.738Z ERROR [lambda_runtime] expected ident at line 1 column 2

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 Specifying verbose: false in the payload doesn't break the lambda

Operating System: Windows

OS Version 11

Additional context The exact thing in the provided template that causes the error is really weird to pin down. If i comment out some of the paths in the api resource it will stop failing with verbose: false. If i comment out specific lines it will also start to work as expected, but i can't see anything in those lines that would be remotely problematic. The template is very trimmed down so it has some Refs to things that aren't provided in it, but i don't think that should matter to Guard (and the full template has the same behavior)

dannyvassallo commented 4 months ago

Hello! Thanks for the report - we're looking into this.

dannyvassallo commented 3 months ago

@boonew2 Can you please provide your Invoke-LMFunction wrapper function? I'd like to reproduce this exactly as you have it and that's one missing piece here.

In your examples:

# this throws the error
Invoke-CfnGuardLambda -TemplatePath C:\temp\template.yml -RulesetPath C:\temp\rules.ruleset

errorType                                                                            errorMessage                           
---------                                                                            ------------                           
&alloc::boxed::Box<dyn core::error::Error + core::marker::Send + core::marker::Sync> trailing characters at line 1 column 19

# this returns the verbose response as expected
Invoke-CfnGuardLambda -Verbose | ConvertTo-Json -Compress

{"message":[{"context":"File(rules=1)","container":"@{FileCheck=}","children":""}]}

Regarding this: Invoke-CfnGuardLambda -Verbose | ConvertTo-Json -Compress

How does that work with the missing parameters? Do you have a default set in your lambda invoker function for rules/files? I'm not sure the verbose flag is having an impact here as the example commands appear to be doing different things entirely.

Additionally, It doesn't look like template files are getting passed in based on the output FileCheck= and since you mention that you're on Windows, using pwsh, and commenting paths in and out with success I'd wager it's something to do with paths on windows or just illegal characters from windows when reading the files in using Get-Content and trying to create valid JSON to pass as a payload.

By default Windows paths aren't valid JSON since they use backslash as a separator and most parsers are going to treat that as an escape. If the filenames are at all included in the payload and you are trying to convert back and forth to JSON using powershell, that may actually be the problem but I'd like to narrow down the exact root cause of your pain.

Can you also send over the template as it's getting logged in CloudWatch? I'd be interested to see how the paths are rendered after being sent through to the lambda.

joshfried-aws commented 3 months ago

HI @boonew2, checking in here. Please provide my colleague @dannyvassallo with the information he requested when you have a moment.

joshfried-aws commented 3 months ago

Hi, I am going to close this issue out. Feel free to reopen if you feel the need.

Thanks

boonew2 commented 1 month ago

@dannyvassallo @joshfried-aws sorry for the delay in response. Apparently updates on this got caught in an email filter on my end. Invoke-LMFunction is the default function in the powershell sdk; nothing i've implemented on my end.

Regarding this: Invoke-CfnGuardLambda -Verbose | ConvertTo-Json -Compress

It doesn't. That is definitely a typo on my end and should have been Invoke-CfnGuardLambda -Verbose -TemplatePath C:\temp\template.yml -RulesetPath C:\temp\rules.ruleset | ConvertTo-Json -Compress

By default Windows paths aren't valid JSON since they use backslash as a separator and most parsers are going to treat that as an escape. If the filenames are at all included in the payload and you are trying to convert back and forth to JSON using powershell, that may actually be the problem but I'd like to narrow down the exact root cause of your pain.

Invoke-CfnGuardLambda dumps the file output into the payload; the file paths themselves aren't preserved in it.

Can you also send over the template as it's getting logged in CloudWatch? I'd be interested to see how the paths are rendered after being sent through to the lambda.

Attaching the cloudwatch stream; I don't recommend looking at the csv in excel.. the formatting is messed up log-events-viewer-result (2).csv

Feel free to reopen if you feel the need.

I don't think i can re-open from my end. ~I haven't tested this for awhile though, i'll pull latest bits and confirm that it is still an issue~ 3.1.1 is still latest it looks like so there aren't newer bits to pull