Pull request overview

This PR adds S3 bucket policy update support intended to safely append log-delivery statements (CloudTrail/ALB/ELB) without overwriting existing bucket policies.

Changes:

• Introduces a shared _apply_bucket_policy helper to append policy statements by Sid with basic retry handling.
• Refactors ALB/ELB/VPC log-enablement flows to call the shared bucket-policy helper.
• Adds a new CloudFormation custom resource Custom::AddBucketPolicy to manage bucket policy statements by service type.

sumologic-app-utils/src/awsresource.py:1406

• _add_policy assumes existing_policy["Statement"] is always a list, but IAM-style policies can legally have a single statement object. Normalizing to a list avoids iterating over dict keys and generating incorrect existing_sids.

            existing_sids = {s.get("Sid") for s in existing_policy["Statement"] if s.get("Sid")}

sumologic-app-utils/src/awsresource.py:1438

• Like _add_policy, _remove_policy assumes Statement is always a list; normalize to a list before filtering to avoid iterating a dict (and to ensure the resulting policy JSON is valid).

        existing_policy["Statement"] = [
            s for s in existing_policy["Statement"] if s.get("Sid") not in our_sids
        ]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.

sumologic-app-utils/src/awsresource.py:532

• existing_policy["Statement"] is assumed to be a list of dicts, but IAM/S3 policies allow Statement to be either a single dict or a list. If a bucket has a single-statement policy, this comprehension/append path will raise (e.g., iterating a dict yields keys, and strings don’t have .get). Normalize Statement to a list before iterating/appending.

            existing_sids = {s.get("Sid") for s in existing_policy["Statement"] if s.get("Sid")}
            for stmt in statements:
                if stmt.get("Sid") not in existing_sids:
                    existing_policy["Statement"].append(stmt)
            try:

sumologic-app-utils/src/awsresource.py:1411

• Same Statement shape issue here: _add_policy assumes existing_policy["Statement"] is a list of dicts, but it can be a single dict. This will break SID detection and append when Statement is not a list. Normalize before iterating/appending.

            existing_sids = {s.get("Sid") for s in existing_policy["Statement"] if s.get("Sid")}
            added = []
            for stmt in expected_stmts:
                if stmt["Sid"] not in existing_sids:
                    existing_policy["Statement"].append(stmt)
                    added.append(stmt["Sid"])

sumologic-app-utils/src/awsresource.py:1427

• The post-write verification also assumes verify["Statement"] is a list; if it is a dict, {s.get("Sid") for s in verify["Statement"]} will raise. Normalize the verification Statement value too.

                verify = json.loads(s3.get_bucket_policy(Bucket=bucket_name)["Policy"])
                current_sids = {s.get("Sid") for s in verify["Statement"]}
                if not expected_sids.issubset(current_sids):

sumologic-app-utils/src/awsresource.py:1447

• _remove_policy assumes existing_policy["Statement"] is a list of dicts, but it can be a single dict. In that case the filter comprehension will iterate keys and fail on .get. Normalize Statement to a list before filtering.

        existing_policy["Statement"] = [
            s for s in existing_policy["Statement"] if s.get("Sid") not in our_sids
        ]

LGTM