-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(s3): add missing safe actions to grantWrite
, grantReadWrite
and grantPut
methods
#18494
fix(s3): add missing safe actions to grantWrite
, grantReadWrite
and grantPut
methods
#18494
Conversation
…VersionAcl to BUCKET_PUT_ACL_ACTIONS
7c25c6d
to
f0da7e9
Compare
Thanks, @flavioleggio. We got approval from AppSec for this. Can you just resolve the conflicts please? |
0e61d96
to
17b926d
Compare
@otaviomacedo done! |
Pull request has been modified.
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…nd `grantPut` methods (aws#18494) In this pull request I try to fix a big behavioral change in the bucket grant methods for write actions. This change was introduced due to security issues as stated in [this pull request](aws#12391) and substituted the `s3:PutObject*` action glob pattern with the simple `s3:PutObject` to exclude the dangerous `s3:PutObjectAcl` and `s3:PutObjectVersionAcl` from the equation. While I agree about the security issues, I think that other useful actions - if safe, obviously - should be added when calling the `grantWrite`, `grantReadWrite` and `grantPut` methods. This pull request adds the following actions: * s3:PutObjectLegalHold * s3:PutObjectRetention * s3:PutObjectTagging * s3:PutObjectVersionTagging I also added the `s3:PutObjectVersionAcl` action to the `grantPutAcl` method, along with the existing `s3:PutObjectAcl`. I adapted existing unit and integ tests to accept these new actions. Fixes aws#13616 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…nd `grantPut` methods (aws#18494) In this pull request I try to fix a big behavioral change in the bucket grant methods for write actions. This change was introduced due to security issues as stated in [this pull request](aws#12391) and substituted the `s3:PutObject*` action glob pattern with the simple `s3:PutObject` to exclude the dangerous `s3:PutObjectAcl` and `s3:PutObjectVersionAcl` from the equation. While I agree about the security issues, I think that other useful actions - if safe, obviously - should be added when calling the `grantWrite`, `grantReadWrite` and `grantPut` methods. This pull request adds the following actions: * s3:PutObjectLegalHold * s3:PutObjectRetention * s3:PutObjectTagging * s3:PutObjectVersionTagging I also added the `s3:PutObjectVersionAcl` action to the `grantPutAcl` method, along with the existing `s3:PutObjectAcl`. I adapted existing unit and integ tests to accept these new actions. Fixes aws#13616 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Shouldn't the api docs reflect this change ? https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.Bucket.html#grantwbrputidentity-objectskeypattern |
In this pull request I try to fix a big behavioral change in the bucket grant methods for write actions. This change was introduced due to security issues as stated in this pull request and substituted the
s3:PutObject*
action glob pattern with the simples3:PutObject
to exclude the dangerouss3:PutObjectAcl
ands3:PutObjectVersionAcl
from the equation.While I agree about the security issues, I think that other useful actions - if safe, obviously - should be added when calling the
grantWrite
,grantReadWrite
andgrantPut
methods. This pull request adds the following actions:I also added the
s3:PutObjectVersionAcl
action to thegrantPutAcl
method, along with the existings3:PutObjectAcl
.I adapted existing unit and integ tests to accept these new actions.
Fixes #13616
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license