Repository: salesforce/policy_sentry
Pull request #353 (2021-04-07 21:40:36)

Fix Excluded Actions
SIDs can be empty due to excluded actions. This check will prevent those SIDs from being returned to the user.

## What does this PR do?
It checks if the current SID is empty of actions and excludes empty SIDs from the statements list
[//]: # (Required: Describe the effects of your pull request in detail. If
multiple changes are involved, a bulleted list is often useful.)
Skip evaluating a SID if the action list is empty after excluding actions.
## What gif best describes this PR or how it makes you feel?

[//]: # (Encouraged: Insert an appropriate gif/meme to brighten up your reviewer's day.)

## Completion checklist

- [x] Additions and changes have unit tests
- [x] [Unit tests, Pylint, security testing, and Integration tests are passing.](https://github.com/salesforce/policy_sentry/actions). GitHub actions does this automatically
- [x] The pull request has been appropriately labeled using the provided PR labels


salesforce-cla[bot]: Thanks for the contribution!  Unfortunately we can't verify the commit author(s): Keith Register <k***@a***.com>.  One possible solution is to [add that email to your GitHub account](https://github.com/settings/emails).  Alternatively you can [change your commits to another email](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user) and force push the change.  After getting your commits associated with your GitHub account, [sign the Salesforce.com Contributor License Agreement](https://cla.salesforce.com/sign-cla?prUrl=https%3A%2F%2Fgithub.com%2Fsalesforce%2Fpolicy_sentry%2Fpull%2F353) and this Pull Request will be revalidated.

Patch:
diff --git a/examples/yml/crud-with-exclude-actions-empty-sid.yml b/examples/yml/crud-with-exclude-actions-empty-sid.yml
new file mode 100644
index 00000000..8a6cd839
--- /dev/null
+++ b/examples/yml/crud-with-exclude-actions-empty-sid.yml
@@ -0,0 +1,5 @@
+mode: crud
+write:
+- arn:aws:s3:::test
+exclude-actions:
+- "iam:Pass*"
diff --git a/policy_sentry/writing/sid_group.py b/policy_sentry/writing/sid_group.py
index 73024974..3d5bbad8 100644
--- a/policy_sentry/writing/sid_group.py
+++ b/policy_sentry/writing/sid_group.py
@@ -174,6 +174,9 @@ class SidGroup:
             else:
                 actions = temp_actions
             # temp_actions.clear()
+            # Check if SID is empty of actions. Continue if yes.
+            if not actions:
+                continue
             match_found = False
             if minimize is not None and isinstance(minimize, int):
                 logger.debug("Minimizing statements...")


Test patch:
diff --git a/test/writing/test_sid_group_crud.py b/test/writing/test_sid_group_crud.py
index 74bf4aac..96781930 100644
--- a/test/writing/test_sid_group_crud.py
+++ b/test/writing/test_sid_group_crud.py
@@ -593,3 +593,69 @@ class SidGroupCrudTestCase(unittest.TestCase):
             ]
         }
         self.assertDictEqual(results, expected_result)
+
+
+    def test_exclude_actions_empty_sid_from_crud_output(self):
+        sid_group = SidGroup()
+        crud_with_exclude_actions_empty_sid = os.path.abspath(
+            os.path.join(
+                os.path.dirname(__file__),
+                os.path.pardir,
+                os.path.pardir,
+                "examples",
+                "yml",
+                "crud-with-exclude-actions-empty-sid.yml",
+            )
+        )
+
+        with open(crud_with_exclude_actions_empty_sid, "r") as this_yaml_file:
+            crud_with_exclude_actions_empty_sid_cfg = yaml.safe_load(this_yaml_file)
+        # crud_with_exclude_actions_empty_sid_cfg = {
+        #     "mode": "crud",
+        #     "write": [
+        #         "arn:aws:s3:::test"
+        #     ],
+        #     "exclude-actions": [
+        #         "iam:Pass*"
+        #     ]
+        # }
+
+        # print(json.dumps(crud_with_exclude_actions_empty_sid_cfg, indent=4))
+        sid_group.process_template(crud_with_exclude_actions_empty_sid_cfg)
+        result = sid_group.get_rendered_policy(crud_with_exclude_actions_empty_sid_cfg)
+        # print(json.dumps(result, indent=4))
+        expected_result = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "S3WriteBucket",
+                    "Effect": "Allow",
+                    "Action": [
+                        "s3:CreateBucket",
+                        "s3:DeleteBucket",
+                        "s3:DeleteBucketOwnershipControls",
+                        "s3:DeleteBucketWebsite",
+                        "s3:PutAccelerateConfiguration",
+                        "s3:PutAnalyticsConfiguration",
+                        "s3:PutBucketCORS",
+                        "s3:PutBucketLogging",
+                        "s3:PutBucketNotification",
+                        "s3:PutBucketObjectLockConfiguration",
+                        "s3:PutBucketOwnershipControls",
+                        "s3:PutBucketRequestPayment",
+                        "s3:PutBucketVersioning",
+                        "s3:PutBucketWebsite",
+                        "s3:PutEncryptionConfiguration",
+                        "s3:PutIntelligentTieringConfiguration",
+                        "s3:PutInventoryConfiguration",
+                        "s3:PutLifecycleConfiguration",
+                        "s3:PutMetricsConfiguration",
+                        "s3:PutReplicationConfiguration"
+                    ],
+                    "Resource": [
+                        "arn:aws:s3:::test"
+                    ]
+                }
+            ]
+        }
+        self.assertDictEqual(result, expected_result)