envato / cloudformation_rspec

Test your CloudFormation templates
MIT License
10 stars 0 forks source link

Unable to validate outputs when IAM policy document contains version #9

Closed HieronymusLex closed 4 years ago

HieronymusLex commented 5 years ago

When testing a YAML template, if I include a Version in an IAM trust policy document for a role, or an IAM policy document, I receive the following error message:

Failure/Error: expect(template_body).to have_output_including("Role")     
Psych::DisallowedClass:
     Tried to load unspecified class: Date
     ...

to reproduce use the following template.yaml:

Resources:
  Role:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Version: 2012-10-17
        Statement:
          - Sid: ''
            Effect: Allow
            Principal:
              Service:
                - firehose.amazonaws.com
            Action: 'sts:AssumeRole'
            Condition:
              StringEquals:
                'sts:ExternalId': !Ref 'AWS::AccountId'

and the following test:

require 'cloudformation_rspec'

describe 'template' do
  let(:template_body) { File.read('template.yaml') }

  it 'outputs stack resources' do
    expect(template_body).to have_output_including("Role")
  end
end
HieronymusLex commented 5 years ago

looks like if you quote the version it'll pass https://github.com/ruby/psych/issues/262

not sure there's anything this lib can do as it seems to be an upstream issue?

patrobinson commented 5 years ago

It might be possible to have a flag, say called unsafe, that tells us to perform a standard YAML.load. AFAICT Psych doesn't want to allow using Dates in safe_load because they aren't doing validation (and yaml can easily be used to inject arbitrary code). But that's not really a concern for us, since we generally trust our templates (unless you're sucking down someone else's templates and testing them).

https://github.com/envato/cloudformation_rspec/blob/967f8101dc1ea3605267ad2b55ece97c605d3059/lib/cloudformation_rspec/matchers/output.rb#L28 would be where we need to check the flag and do an "unsafe load"

HieronymusLex commented 5 years ago

yeah all the templates I'm testing are one's that I've wrote locally, but that being said I reckon it'd be better to default to safe behaviour and maybe have a documented flag to allow non safe-load. The workaround (i.e. quote the date-like string) is pretty easy