Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-cdk/aws-iam: should check for invalid array in PolicyStatement conditions #25826

Open
cloventt opened this issue Jun 2, 2023 · 1 comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@cloventt
Copy link

cloventt commented Jun 2, 2023

Describe the bug

For some reason, typescript allows an array to be set on PolicyStatement.conditions, which is supposed to be an object of type {[key: string]: any}. This raises no compile errors either in Typescript or CDK and generates an invalid PolicyDocument in the CFN template.

Expected Behavior

The broken code should not synth.

Current Behavior

When accidentally passing an array to PolicyStatement.conditions, there are no type errors raised (so IDE shows no problem in syntax highlighting) and some invalid CFN is synthed.

Reproduction Steps

I had this code:

      this.bucket.addToResourcePolicy(new PolicyStatement({
        effect: Effect.ALLOW,
        principals: [
          new ServicePrincipal('s3.amazonaws.com'),
        ],
        actions: [
          's3:PutObject',
        ],
        resources: [
          this.bucket.arnForObjects('*'),
        ],
        conditions: [ // this array should not be here
          {
            StringEquals: {
              'aws:SourceAccount': '123456789',
              's3:x-amz-acl': 'bucket-owner-full-control',
            },
          },
        ],
      }));
    });

Which synthed fine and resulted in a CFN template that looks like this:

{
  "PolicyDocument": {
    "Statement": [
      {
        "Action": "s3:PutObject",
        "Condition": {
          "0": {
            "StringEquals": {
              "aws:SourceAccount": "123456789",
              "s3:x-amz-acl": "bucket-owner-full-control",
            }
          }
        },
      }
    ]
  }
}

When applying this template, CFN reports this error:

Invalid Condition type : 0 (Service: Amazon S3; Status Code: 400; Error Code: MalformedPolicy;)

Possible Solution

I'm really not sure why Typescript doesn't report this as a proper type error, but it might be worth adding something defensive to check that the PolicyStatement condition is actually an object and not an array, and failing the build if the developer puts the wrong thing in (like I did).

Additional Information/Context

No response

CDK CLI Version

2.79.1

Framework Version

No response

Node.js Version

19.1.0

OS

Linux

Language

Typescript

Language Version

4.9.5

Other information

No response

@cloventt cloventt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Jun 2, 2023
@peterwoodworth
Copy link
Contributor

PolicyStatement.conditions is of type any, so I don't think this is unexpected. If we change this to anything but any, we risk breaking customer code, so I think this will stay any. I suppose we could include a check which makes sure what's provided isn't an array, but I'm not confident that's necessary.

@peterwoodworth peterwoodworth added p2 feature-request A feature should be added or improved. effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 2, 2023
@peterwoodworth peterwoodworth changed the title aws-cdk/aws-iam: typescript allows invalid array in PolicyStatement conditions aws-cdk/aws-iam: typescript should check for invalid array in PolicyStatement conditions Jun 2, 2023
@peterwoodworth peterwoodworth changed the title aws-cdk/aws-iam: typescript should check for invalid array in PolicyStatement conditions aws-cdk/aws-iam: should check for invalid array in PolicyStatement conditions Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

2 participants