Describe the bug

In bucket-deployment.ts, the BucketDeployment construct grants its execution role access to read/write the entire bucket. However, if the bucket was configured with a non-empty destinationKeyPrefix (as is recommended), it only needs to read or write objects with the given key prefix.

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Library Version

No response

Expected Behavior

The role that is configured by BucketDeployment should be configured with the least authority needed to do its job.

Current Behavior

The role that is configured by BucketDeployment has access to read/write/delete objects outside of the expected prefix. This permission is slightly broader than it needs.

Reproduction Steps

For example, if you have this snippet in a CDK stack:

    const jobAssetsPrefix = 'job-assets/';
    this.jobAssetsDeployment = new BucketDeployment(this, 'JobAssets', {
      sources: [],
      destinationBucket: this.generalAssetsBucket,
      destinationKeyPrefix: jobAssetsPrefix,
      prune: true,
    });

you will find that the cfn template includes a policy like this (trimmed for readability):

  "CustomCDKBucketDeployment8693BB64968944B69AAFB0CC9EB8756CServiceRoleDefaultPolicy88902FDF": {
   "Type": "AWS::IAM::Policy",
   "Properties": {
    "PolicyDocument": {
     "Statement": [
      {
       "Action": [
        "s3:GetObject*",
        "s3:GetBucket*",
        "s3:List*",
        "s3:DeleteObject*",
        "s3:PutObject",
        "s3:PutObjectLegalHold",
        "s3:PutObjectRetention",
        "s3:PutObjectTagging",
        "s3:PutObjectVersionTagging",
        "s3:Abort*"
       ],
       "Effect": "Allow",
       "Resource": [
         {"Fn::GetAtt": ["CommonAssetsBucket276E20FC","Arn"] },
         {"Fn::Join": ["",[{"Fn::GetAtt": ["CommonAssetsBucket276E20FC","Arn"]}, "/*"]] }
       ]
      },

That last resource entry would ideally look like

         {"Fn::Join": ["",[{"Fn::GetAtt": ["CommonAssetsBucket276E20FC","Arn"]}, "/job-assets/*"]] }

Possible Solution

Where currently the construct has (here):

    this.destinationBucket.grantReadWrite(handler);
    if (props.accessControl) {
      this.destinationBucket.grantPutAcl(handler);
    }

it would instead compute an object pattern based on props.destinationKeyPrefix, and pass in that pattern as the second argument to .grantReadWrite() and .grantPutAcl(). A missing or empty prefix would result in a pattern of '*', but a prefix of (for example) job-assets/ would result in a pattern of "job-assets/*".

I'm not 100% sure what the right behavior of a prefix of / or /some-path/ would be. Some users would expect the leading slash to be stripped, to avoid two consecutive slashes in the resulting policy ARN.

Additional Information/Context

This isn't a terribly critical excess permission, since it only applies to the deployment lambda and roles that can invoke that lambda are presumably already privileged enough to do bad things in other ways; but when auditing roles in a stack to ensure no over-broad permissions were granted, this sticks out.

AWS CDK Library version (aws-cdk-lib)

v2.217.0

AWS CDK CLI version

n/a

Node.js Version

20

OS

Amazon Linux 2

Language

TypeScript

Language Version

No response

Other information

No response