fix(cloudtrail): do not attach s3 bucket permission when orgId is not set for organization trail#30490
fix(cloudtrail): do not attach s3 bucket permission when orgId is not set for organization trail#30490
Conversation
github-actions
bot
added
the
beginning-contributor
aws-cdk-automation
left a comment
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
|
Exemption Request: deploying this requires AWS organization management account |
aws-cdk-automation
added
the
pr-linter/exemption-requested
aws-cdk-automation
added
the
pr/needs-community-review
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
…specified for organization trail
sarisia
force-pushed
the
cloudtrail-missing-orgid
branch
from
e70c347 to
5b8134d
Compare
|
Could someone please review this? Thanks |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing ✅ A exemption request has been requested. Please wait for a maintainer's review. |
|
|
… set for organization trail (#30778) Recreate #30490 ### Issue # (if applicable) no open issue ### Reason for this change Organization trail without `orgId` attaches improper s3 bucket policy which allows cloudtrail to write to `/AWSLogs/undefined/*` prefix. ```json { "Action": "s3:PutObject", "Condition": { "StringEquals": { "s3:x-amz-acl": "bucket-owner-full-control", "aws:SourceArn": { "Fn::Join": [ "", [ "arn:", { "Ref": "AWS::Partition" }, ":cloudtrail:us-east-1:123456789012:trail/undefined" ] ] } } }, "Effect": "Allow", "Principal": { "Service": "cloudtrail.amazonaws.com" }, "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "TrailS30071F172", "Arn" ] }, "/AWSLogs/undefined/*" ] ] } } ``` ### Description of changes - do not attach s3 bucket policy if `isOrganizationTrail` is set and `orgId` is not set - generate warning if attaching s3 bucket policy is skipped ### Description of how you validated changes - added unit test ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
… set for organization trail (aws#30778) Recreate aws#30490 ### Issue # (if applicable) no open issue ### Reason for this change Organization trail without `orgId` attaches improper s3 bucket policy which allows cloudtrail to write to `/AWSLogs/undefined/*` prefix. ```json { "Action": "s3:PutObject", "Condition": { "StringEquals": { "s3:x-amz-acl": "bucket-owner-full-control", "aws:SourceArn": { "Fn::Join": [ "", [ "arn:", { "Ref": "AWS::Partition" }, ":cloudtrail:us-east-1:123456789012:trail/undefined" ] ] } } }, "Effect": "Allow", "Principal": { "Service": "cloudtrail.amazonaws.com" }, "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "TrailS30071F172", "Arn" ] }, "/AWSLogs/undefined/*" ] ] } } ``` ### Description of changes - do not attach s3 bucket policy if `isOrganizationTrail` is set and `orgId` is not set - generate warning if attaching s3 bucket policy is skipped ### Description of how you validated changes - added unit test ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
2 participants
Issue # (if applicable)
no open issue
Reason for this change
Organization trail without
orgIdattaches improper s3 bucket policywhich allows cloudtrail to write to
/AWSLogs/undefined/*prefix.{ "Action": "s3:PutObject", "Condition": { "StringEquals": { "s3:x-amz-acl": "bucket-owner-full-control", "aws:SourceArn": { "Fn::Join": [ "", [ "arn:", { "Ref": "AWS::Partition" }, ":cloudtrail:us-east-1:123456789012:trail/undefined" ] ] } } }, "Effect": "Allow", "Principal": { "Service": "cloudtrail.amazonaws.com" }, "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "TrailS30071F172", "Arn" ] }, "/AWSLogs/undefined/*" ] ] } }(there is also another bug which trail name becomes
undefined, which is addressed in #30489 based on this PR)Description of changes
isOrganizationTrailis set andorgIdis not setDescription of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license