Check for unsupported element combination (#3145)

aws-cloudformation · Apr 15, 2024 · 1201614 · 1201614
1 parent 62c4ad8
commit 1201614
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 2 deletions.
diff --git a/src/cfnlint/rules/resources/iam/Policy.py b/src/cfnlint/rules/resources/iam/Policy.py
@@ -151,6 +151,10 @@ def _check_policy_statement(
         if "Action" not in statement and "NotAction" not in statement:
             message = "IAM Policy statement missing Action or NotAction"
             matches.append(RuleMatch(branch[:], message))
+        if "Action" in statement and "NotAction" in statement:
+            message = "IAM Policy statement should have only one of Action or NotAction"
+            matches.append(RuleMatch(branch[:] + ["Action"], message))
+            matches.append(RuleMatch(branch[:] + ["NotAction"], message))
         if is_identity_policy:
             if "Principal" in statement or "NotPrincipal" in statement:
                 message = "IAM Resource Policy statement shouldn't have Principal or NotPrincipal"
@@ -159,10 +163,18 @@ def _check_policy_statement(
             if "Principal" not in statement and "NotPrincipal" not in statement:
                 message = "IAM Resource Policy statement should have Principal or NotPrincipal"
                 matches.append(RuleMatch(branch[:] + ["Principal"], message))
+            if "Principal" in statement and "NotPrincipal" in statement:
+                message = "IAM Resource Policy statement should have only one of Principal or NotPrincipal"
+                matches.append(RuleMatch(branch[:] + ["Principal"], message))
+                matches.append(RuleMatch(branch[:] + ["NotPrincipal"], message))
         if not resource_exceptions:
             if "Resource" not in statement and "NotResource" not in statement:
                 message = "IAM Policy statement missing Resource or NotResource"
                 matches.append(RuleMatch(branch[:], message))
+            if "Resource" in statement and "NotResource" in statement:
+                message = "IAM Resource Policy statement should have only one of Resource or NotResource"
+                matches.append(RuleMatch(branch[:] + ["Resource"], message))
+                matches.append(RuleMatch(branch[:] + ["NotResource"], message))
 
         resources = statement.get("Resource", [])
         if isinstance(resources, str):

diff --git a/test/fixtures/templates/bad/resources/iam/iam_policy.yaml b/test/fixtures/templates/bad/resources/iam/iam_policy.yaml
@@ -40,3 +40,22 @@ Resources:
         - cCondition
         - Statement: {}
         - Statement: []
+  rIamPolicyExtraThings:
+    Type: AWS::IAM::Policy
+    Properties:
+      PolicyName: test
+      Roles:
+      - MyRole
+      PolicyDocument:
+        Version: '2012-10-17'
+        Statement:
+        - Effect: Allow
+
+          Action:
+          - cloudwatch:*
+          NotAction:
+          - ec2:*
+          Resource:
+          - '*'
+          NotResource:
+          - 'arn'
diff --git a/test/fixtures/templates/bad/resources/iam/resource_policy.yaml b/test/fixtures/templates/bad/resources/iam/resource_policy.yaml
@@ -16,6 +16,20 @@ Resources:
                 aws:Referer:
                   - "http://www.example.com/*"
                   - "http://example.com/*"
+  SampleBucketPolicy2:
+    Type: AWS::S3::BucketPolicy
+    Properties:
+      Bucket: "myExampleBucket"
+      PolicyDocument:
+        Statement:
+          -
+            Action:
+              - "s3:GetObject"
+            Effect: "Allow"
+            Resource:
+            - "*"
+            Principal: test
+            NotPrincipal: test
   mysnspolicy:
     Type: AWS::SNS::TopicPolicy
     Properties:

diff --git a/test/unit/rules/resources/iam/test_iam_policy.py b/test/unit/rules/resources/iam/test_iam_policy.py
@@ -27,11 +27,11 @@ def test_file_positive(self):
     def test_file_negative(self):
         """Test failure"""
         self.helper_file_negative(
-            "test/fixtures/templates/bad/resources/iam/iam_policy.yaml", 12
+            "test/fixtures/templates/bad/resources/iam/iam_policy.yaml", 16
         )
 
     def test_file_resource_negative(self):
         """Test failure"""
         self.helper_file_negative(
-            "test/fixtures/templates/bad/resources/iam/resource_policy.yaml", 4
+            "test/fixtures/templates/bad/resources/iam/resource_policy.yaml", 6
         )