Fix s3_get_bucket_policy_status #7926
Conversation
| for (const statement of policy.Statement) { | ||
| if (statement.Effect === 'Allow' && statement.Principal) { | ||
| const statement_principal = statement.Principal.AWS ? statement.Principal.AWS : statement.Principal; | ||
| for (const principal of _.flatten([statement_principal])) { |
There was a problem hiding this comment.
is this something possible? that we will have more than one principle, and one of them be '*'? (seems redundant no?) Anyway, if it does, add a comment about what you checking here
There was a problem hiding this comment.
it is redundant but possible both in our code and AWS. when checking on AWS if at least one of the principles is * it returned true for public policy. so we might as well do the same
| principal_wildcard = true; | ||
| } | ||
| } | ||
| for (const resource of statement.resource) { |
There was a problem hiding this comment.
is this something you checked with AWS? because in all the examples they have the resource is *
There was a problem hiding this comment.
yes, I checked it. when adding a policy with specific principles and resources with a wildcard it returns 'false' for this function. it did return true for specific resource and wildcard principals. Also looking at the definition of public it doesn't seem to list resources as one of the things evaluated for public policy. I am not sure why in their examples the resource is always *
nadavMiz
force-pushed
the
fix-bucket-poilcy-status
branch
from
2607524 to
427fc5a
Compare
Signed-off-by: nadav mizrahi <nadav.mizrahi16@gmail.com>
nadavMiz
force-pushed
the
fix-bucket-poilcy-status
branch
from
427fc5a to
45b34f4
Compare
Explain the changes
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
make run-single-test-postgres testname=test_s3_bucket_policy.js