Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class TestStack extends Stack {

const bucket = new s3.Bucket(this, 'Bucket', {
removalPolicy: RemovalPolicy.DESTROY,
autoDeleteObjects: true,
autoDeleteObjects: true, // Note: Fix for #37257 adds s3:GetBucketTagging directly to the Lambda role
});

// Put objects in the bucket to ensure auto delete works as expected
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,24 @@ async function onDelete(bucketName?: string) {
* If the Custom Resource is ever deleted before the bucket, it must be because
* `autoDeleteObjects` has been switched to false, in which case the tag would have
* been removed before we get to this Delete event.
*
* If we get an AccessDenied error, it means the bucket policy (which grants s3:GetBucket*)
* has already been removed during the CFN update phase. This only happens when
* `autoDeleteObjects` was disabled, confirming the bucket should not be emptied.
*/
async function isBucketTaggedForDeletion(bucketName: string) {
const response = await s3.getBucketTagging({ Bucket: bucketName });
return response.TagSet?.some(tag => tag.Key === AUTO_DELETE_OBJECTS_TAG && tag.Value === 'true');
try {
const response = await s3.getBucketTagging({ Bucket: bucketName });
return response.TagSet?.some(tag => tag.Key === AUTO_DELETE_OBJECTS_TAG && tag.Value === 'true');
} catch (error: any) {
// If we cannot read the tags due to a permissions error, the bucket policy
// (which grants s3:GetBucket*) has already been removed. This only happens
// when autoDeleteObjects was disabled – meaning the tag was already removed too.
// Treat as "not tagged for deletion" to skip the bucket empty operation.
if (error.name === 'AccessDenied' || error.name === 'AccessControlListNotSupported') {
console.log(`Could not check tags for bucket '${bucketName}' due to ${error.name}, assuming not tagged for deletion.`);
return false;
}
throw error;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,33 @@ test('deletes all objects on delete event even when bucket policy cannot be read
});
});

test('does not empty bucket when getBucketTagging returns AccessDenied', async () => {
// GIVEN
mockS3Client.getBucketTagging.mockImplementation(async () => {
const { S3ServiceException } = jest.requireActual('@aws-sdk/client-s3');
return Promise.reject(new S3ServiceException({
name: 'AccessDenied',
$fault: 'client',
$metadata: {},
}));
});

// WHEN
const event: Partial<AWSLambda.CloudFormationCustomResourceDeleteEvent> = {
RequestType: 'Delete',
ResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
};
await invokeHandler(event);

// THEN
expect(mockS3Client.putBucketPolicy).not.toHaveBeenCalled();
expect(mockS3Client.listObjectVersions).not.toHaveBeenCalled();
expect(mockS3Client.deleteObjects).not.toHaveBeenCalled();
});

test('does not empty bucket if it is not tagged', async () => {
// GIVEN
givenNotTaggedForDeletion();
Expand Down
9 changes: 9 additions & 0 deletions packages/aws-cdk-lib/aws-s3/lib/mixins/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ export class BucketAutoDeleteObjects extends Mixin {
description: `Lambda function for auto-deleting objects in ${bucketRef.bucketName} S3 bucket.`,
});

// Grant s3:GetBucketTagging directly on the IAM execution role so it works
// even if the bucket policy is removed first (e.g., when disabling autoDeleteObjects).
// Without this, the Lambda would lose permission to call getBucketTagging() during
// the CFN CLEANUP phase when the bucket policy is removed in the UPDATE phase.
provider.addToRolePolicy(new iam.PolicyStatement({
actions: ['s3:GetBucketTagging'],
resources: [bucketRef.bucketArn],
}));

// Use a bucket policy to allow the custom resource to delete
// objects in the bucket
const policyResult = iam.ResourceWithPolicies.of(construct)?.addToResourcePolicy(new iam.PolicyStatement({
Expand Down
23 changes: 23 additions & 0 deletions packages/aws-cdk-lib/aws-s3/test/bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3820,6 +3820,29 @@ describe('bucket', () => {
},
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 's3:GetBucketTagging',
Effect: 'Allow',
Resource: {
'Fn::GetAtt': [
'MyBucketF68F3FF0',
'Arn',
],
},
},
],
Version: '2012-10-17',
},
Roles: [
{
Ref: 'CustomS3AutoDeleteObjectsCustomResourceProviderRole3B1BD092',
},
],
});

Template.fromStack(stack).hasResource('Custom::S3AutoDeleteObjects', {
'Properties': {
'ServiceToken': {
Expand Down
Loading