diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 19d8410fda..ce0b5cb581 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes to the **Prowler SDK** are documented in this file. ### 🐞 Fixed +- `s3_bucket_shadow_resource_vulnerability` no longer emits a tautological `PASS` finding for every bucket; a finding is now produced only when the bucket name matches one of the predictable service patterns (Glue, SageMaker, EMR, CodeStar) [(#11220)](https://github.com/prowler-cloud/prowler/pull/11220) - `sqlserver_tde_encrypted_with_cmk` check for Azure provider no longer reports a false `FAIL` for SQL Servers whose user databases are correctly encrypted with a customer-managed key, by excluding the system `master` database (always reports TDE `Disabled` and is not customer-controllable) from the TDE evaluation [(#11233)](https://github.com/prowler-cloud/prowler/pull/11233) --- diff --git a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py index eb509b1c85..ce14eca587 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py +++ b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py @@ -24,30 +24,30 @@ class s3_bucket_shadow_resource_vulnerability(Check): # First, check buckets in the current account for bucket in s3_client.buckets.values(): - report = Check_Report_AWS(self.metadata(), resource=bucket) - report.region = bucket.region - report.resource_id = bucket.name - report.resource_arn = bucket.arn - report.resource_tags = bucket.tags - report.status = "PASS" - report.status_extended = ( - f"S3 bucket {bucket.name} is not a known shadow resource." - ) - - # Check if this bucket matches any predictable pattern + # Only emit a finding when the bucket name actually matches one of + # the predictable service patterns. A bucket whose name does not + # match any pattern is, by definition, not a shadow resource, so a + # PASS finding for it would be tautological and add no signal. for service, pattern_format in predictable_patterns.items(): pattern = pattern_format.replace("", bucket.region) if re.match(pattern, bucket.name): + report = Check_Report_AWS(self.metadata(), resource=bucket) + report.region = bucket.region + report.resource_id = bucket.name + report.resource_arn = bucket.arn + report.resource_tags = bucket.tags + if bucket.owner_id != s3_client.audited_canonical_id: report.status = "FAIL" report.status_extended = f"S3 bucket {bucket.name} for service {service} is a known shadow resource and it is owned by another account ({bucket.owner_id})." else: report.status = "PASS" report.status_extended = f"S3 bucket {bucket.name} for service {service} is a known shadow resource but it is correctly owned by the audited account." + + findings.append(report) + reported_buckets.add(bucket.name) break - findings.append(report) - reported_buckets.add(bucket.name) # Now check for shadow resources in other accounts by testing predictable patterns # We'll test different regions to see if shadow resources exist diff --git a/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py b/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py index 9b80d6db35..4b435f0f59 100644 --- a/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py +++ b/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py @@ -93,6 +93,8 @@ class Test_s3_bucket_shadow_resource_vulnerability: @mock_aws def test_bucket_not_predictable(self): + # A bucket whose name does not match any predictable service pattern + # is not a shadow resource and must not produce any finding. aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" @@ -113,6 +115,55 @@ class Test_s3_bucket_shadow_resource_vulnerability: s3_client.provider = aws_provider s3_client._head_bucket = mock.MagicMock(return_value=False) + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.s3.s3_bucket_shadow_resource_vulnerability.s3_bucket_shadow_resource_vulnerability.s3_client", + new=s3_client, + ), + ): + from prowler.providers.aws.services.s3.s3_bucket_shadow_resource_vulnerability.s3_bucket_shadow_resource_vulnerability import ( + s3_bucket_shadow_resource_vulnerability, + ) + + check = s3_bucket_shadow_resource_vulnerability() + result = check.execute() + + assert len(result) == 0 + + @mock_aws + def test_only_predictable_bucket_reported_among_many(self): + # With a mix of buckets, only the one matching a predictable pattern + # must produce a finding; the rest must be silent. + aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) + aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" + + predictable_bucket = f"sagemaker-{AWS_REGION_US_EAST_1}-{AWS_ACCOUNT_NUMBER}" + plain_buckets = [ + "config-bucket-data", + "my-app-data-bucket", + "guardduty-findings-store", + ] + + s3_client = mock.MagicMock() + s3_client.audited_canonical_id = AWS_ACCOUNT_NUMBER + s3_client.audited_partition = "aws" + s3_client.buckets = { + name: Bucket( + name=name, + arn=f"arn:aws:s3:::{name}", + region=AWS_REGION_US_EAST_1, + owner_id=AWS_ACCOUNT_NUMBER, + tags=[], + ) + for name in [predictable_bucket, *plain_buckets] + } + s3_client.provider = aws_provider + s3_client._head_bucket = mock.MagicMock(return_value=False) + with ( mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", @@ -132,14 +183,10 @@ class Test_s3_bucket_shadow_resource_vulnerability: assert len(result) == 1 report = result[0] - - # Test all report attributes assert report.status == "PASS" - assert report.region == AWS_REGION_US_EAST_1 - assert report.resource_id == bucket_name - assert report.resource_arn == f"arn:aws:s3:::{bucket_name}" - assert report.resource_tags == [{"Key": "Project", "Value": "test-project"}] - assert "is not a known shadow resource" in report.status_extended + assert report.resource_id == predictable_bucket + assert "SageMaker" in report.status_extended + assert "is correctly owned by the audited account" in report.status_extended @mock_aws def test_shadow_resource_in_other_account(self):