🐛 fix `aws.s3.bucket.policy` resource #5228

afiune · 2025-02-15T06:28:41Z

This change allows running aws.s3.bucket.policy when in discovery mode.

cnspec shell aws --discover s3-buckets

Also, fixes the issue discovered at #5218
where we noticed that the policy resource doesn't implement the id() func properly.

Closes #5169

This change allows running `aws.s3.bucket.policy` when in discovery mode. ``` cnspec shell aws --discover s3-buckets ``` Also, fixes the issue discovered at #5218 where we noticed that the policy resource doesn't implement the `id()` func properly. Closes #5169 Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

afiune · 2025-02-15T06:31:08Z

providers/aws/resources/aws.lr

-  // Name for the policy
-  name string
+  // Bucket name that this policy belongs
+  bucketName string


This might be a breaking change, but a bucket policy doesn't have a name. It belongs to a bucket and, to calculate the bucket policy ID we need the bucket name, so I re purpose this field.

If this is not acceptable, I am ok creating an internal struct to pass the bucket name but, we need to do something about this namefield that says it is the name of the policy, but it's not.

we don't know whether this is used by someone or not. Perhaps keep both for a while and add a comment to name that it's deprecated. We can remove it in next major release

If someone is using it, they are using it incorrectly but I agree, I'll put it back with a deprecation.

afiune · 2025-02-15T06:31:36Z

providers/aws/resources/aws.lr

  // Document for the policy
  document string
  // Version of the policy
  version() string
  // List of statements for the policy
  statements() []dict
+  // Whether the bucket policy exists
+  exists bool


Buckets can have a policy, but not all of them has it. This is very helpful, I think.

I don't understand why this is needed. If there is no bucket policy then that resource shouldn't be created, right? Do we have some specific requirement to have the MQL object even when such resource in AWS doesn't exist?

we should be able to use null comparisons if we need to check that a policy exists or does not exist on a bucket. like aws.s3.bucket.policy != null -- does that not work?

It sounds it is possible! 🙌🏽 Thank you both for the correction 💯

github-actions · 2025-02-15T06:34:24Z

Test Results

3 335 tests ±0 3 331 ✅ ±0 1m 39s ⏱️ -15s
385 suites ±0 4 💤 ±0
29 files ±0 0 ❌ ±0

Results for commit 958b932. ± Comparison against base commit 0162b01.

♻️ This comment has been updated with latest results.

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

vjeffrey · 2025-02-20T03:02:43Z

i'm hitting a panic when i try to test this

cnquery> aws.s3.buckets { policy != null }
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x1071ee7d0]

goroutine 176 [running]:
go.mondoo.com/cnquery/v11/providers/aws/resources.(*mqlAwsS3BucketPolicy).MqlID(0x10cd66390?)
	/Users/vj/go/src/go.mondoo.io/cnquery/providers/aws/resources/aws.lr.go:20888

vjeffrey · 2025-02-20T03:32:24Z

providers/aws/resources/aws_s3.go

-		if isNotFoundForS3(err) {
-			return emptyAwsS3BucketPolicy(a.MqlRuntime)
-		}
+	if err != nil && !isNotFoundForS3(err) {


perhaps it would be useful to set the plugin state here? plugin.StateIsNull | plugin.StateIsSet

Nice, I learned something new today. Thanks @vjeffrey 👍🏽

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

vjeffrey · 2025-02-20T23:46:22Z

hmm. the other panic is gone:

cnquery> aws.s3.buckets { policy != null }
aws.s3.buckets: [
  0: {
    policy != null: false
  }

but i'm seeing this:

cnquery> aws.s3.buckets { public }
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x98 pc=0x10bb6fbb8]

goroutine 83 [running]:
go.mondoo.com/cnquery/v11/providers/aws/resources.(*mqlAwsS3BucketPolicy).parsePolicyDocument(0x14000faa008?)
	/Users/vj/go/src/go.mondoo.io/cnquery/providers/aws/resources/aws_s3.go:714 +0x18
go.mondoo.com/cnquery/v11/providers/aws/resources.(*mqlAwsS3Bucket).public(0x14000faa008)
	/Users/vj/go/src/go.mondoo.io/cnquery/providers/aws/resources/aws_s3.go:462 +0x23c

mm-weber · 2025-02-21T13:11:33Z

@afiune

Tested this PR on the proposed solution, something seems to be off, still:

This does throw an error, too:

Tested this against via cnspec shell aws --discover s3-buckets using a s3 bucket with those properties:
via aws.s3.bucket {*} .

afiune · 2025-02-21T16:55:12Z

@vjeffrey @mm-weber ugh, I think that there are a few moving parts that I missed, specially since we merged #5218 and that is where it's throwing a panic.

Will have something to review soon.

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

afiune · 2025-02-21T17:29:19Z

providers/aws/resources/aws_s3.go

@@ -133,6 +158,9 @@ func initAwsS3Bucket(runtime *plugin.Runtime, args map[string]*llx.RawData) (map
 	var arn string
 	if args["arn"] != nil {
 		arn = args["arn"].Value.(string)
+		if !strings.HasPrefix(arn, "arn:aws:s3:") {
+			return nil, nil, errors.Newf("not a valid bucket ARN '%s'", arn)


During testing I detected that, if you are NOT in discovery and, you try to access aws.s3.bucket then we try to "create a bucket resource" of the AWS account, which is incorrect.

Now, we will exit with helpful error message:

cnquery> aws.s3.bucket not a valid bucket ARN 'arn:aws:sts::123456789012' aws.s3.bucket: no data available

vjeffrey · 2025-02-21T17:43:17Z

providers/aws/resources/aws.lr

  // Unique ID for the policy
  id string
-  // Name for the policy
+  // Deprecated


nit: can you add "use bucketName instead" so that the deprecation reason is clear?

providers/aws/resources/aws.lr

afiune commented Feb 15, 2025

View reviewed changes

🔄 do not create bucket policy if not exists

01b8dd5

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

afiune requested review from vjeffrey and imilchev February 20, 2025 00:46

vjeffrey reviewed Feb 20, 2025

View reviewed changes

🐛 set policy to plugin.StateIsNull when not found

99d3c83

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

afiune requested a review from vjeffrey February 20, 2025 22:59

🐛 fix accessing aws.s3.bucket.policy

958b932

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>

afiune commented Feb 21, 2025

View reviewed changes

vjeffrey reviewed Feb 21, 2025

View reviewed changes

afiune commented Feb 21, 2025

View reviewed changes

providers/aws/resources/aws.lr Outdated Show resolved Hide resolved

🧹 update aws.lr

d4dfcef

vjeffrey approved these changes Feb 21, 2025

View reviewed changes

afiune merged commit f89614d into main Feb 21, 2025
8 checks passed

afiune deleted the afiune/aws/bucket-policy branch February 21, 2025 18:01

github-actions bot locked and limited conversation to collaborators Feb 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 fix `aws.s3.bucket.policy` resource #5228

🐛 fix `aws.s3.bucket.policy` resource #5228

afiune commented Feb 15, 2025

afiune Feb 15, 2025

imilchev Feb 18, 2025

afiune Feb 19, 2025

afiune Feb 15, 2025

imilchev Feb 18, 2025

vjeffrey Feb 19, 2025

afiune Feb 20, 2025

github-actions bot commented Feb 15, 2025 •

edited

Loading

vjeffrey commented Feb 20, 2025

vjeffrey Feb 20, 2025

afiune Feb 20, 2025

vjeffrey commented Feb 20, 2025

mm-weber commented Feb 21, 2025

afiune commented Feb 21, 2025

afiune Feb 21, 2025

vjeffrey Feb 21, 2025

afiune Feb 21, 2025

🐛 fix aws.s3.bucket.policy resource #5228

🐛 fix aws.s3.bucket.policy resource #5228

Conversation

afiune commented Feb 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 15, 2025 • edited Loading

Test Results

vjeffrey commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjeffrey commented Feb 20, 2025

mm-weber commented Feb 21, 2025

afiune commented Feb 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

🐛 fix `aws.s3.bucket.policy` resource #5228

🐛 fix `aws.s3.bucket.policy` resource #5228

github-actions bot commented Feb 15, 2025 •

edited

Loading