Fix bad formatting in generic rules messages (#233)

* fix generic rules messages not well formatted * update changelog and version Co-authored-by: Ramon <[email protected]>
Skyscanner · Sep 23, 2022 · 02c9e69 · 02c9e69
1 parent b2cb2ca
commit 02c9e69
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.13.1]
+### Fixes
+- Fixes `GenericResourcePartialWildcardPrincipalRule` and `GenericCrossAccountTrustRule` message, since sometimes it was bad-formatted in markdown.
+
 ## [1.13.0]
 ### Additions
 - Default logging level from INFO to WARNING #230

diff --git a/cfripper/__version__.py b/cfripper/__version__.py
@@ -1,3 +1,3 @@
-VERSION = (1, 13, 0)
+VERSION = (1, 13, 1)
 
 __version__ = ".".join(map(str, VERSION))
diff --git a/cfripper/rules/cross_account_trust.py b/cfripper/rules/cross_account_trust.py
@@ -133,7 +133,7 @@ class GenericCrossAccountTrustRule(CrossAccountCheckingRule):
         |`account_id` | `str`       | Account ID found in the principal                              |
     """
 
-    REASON = "{} has forbidden cross-account with {}"
+    REASON = "{} has forbidden cross-account with `{}`"
 
     def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
         result = Result()

diff --git a/cfripper/rules/wildcard_principals.py b/cfripper/rules/wildcard_principals.py
@@ -275,7 +275,7 @@ class GenericResourcePartialWildcardPrincipalRule(GenericResourceWildcardPrincip
     """
 
     REASON_WILDCARD_PRINCIPAL = (
-        "{} should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at '{}'"
+        "{} should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `{}`"
     )
     RISK_VALUE = RuleRisk.MEDIUM
     FULL_REGEX = REGEX_PARTIAL_WILDCARD_PRINCIPAL

diff --git a/tests/rules/test_GenericCrossAccountTrustRule.py b/tests/rules/test_GenericCrossAccountTrustRule.py
@@ -114,7 +114,7 @@ def test_iam_role_is_checked_in_generic_rule(template_one_role):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="RootRole has forbidden cross-account with arn:aws:iam::999999999:role/[email protected]",
+                reason="RootRole has forbidden cross-account with `arn:aws:iam::999999999:role/[email protected]`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -136,7 +136,7 @@ def test_s3_bucket_cross_account_with_generic(s3_bucket_cross_account):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="S3BucketPolicyAccountAccess has forbidden cross-account with arn:aws:iam::987654321:root",
+                reason="S3BucketPolicyAccountAccess has forbidden cross-account with `arn:aws:iam::987654321:root`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -158,7 +158,7 @@ def test_s3_bucket_cross_account_and_normal_with_generic(s3_bucket_cross_account
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="S3BucketPolicyAccountAccess has forbidden cross-account with arn:aws:iam::666555444333:root",
+                reason="S3BucketPolicyAccountAccess has forbidden cross-account with `arn:aws:iam::666555444333:root`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -180,7 +180,7 @@ def test_s3_bucket_cross_account_and_normal_with_org_aws_account_with_generic(s3
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="S3BucketPolicyAccountAccess has forbidden cross-account with arn:aws:iam::666555444333:root",
+                reason="S3BucketPolicyAccountAccess has forbidden cross-account with `arn:aws:iam::666555444333:root`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -253,7 +253,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason=f"TestDomain has forbidden cross-account with {principal}",
+                reason=f"TestDomain has forbidden cross-account with `{principal}`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -276,7 +276,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="TestDomain has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="TestDomain has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -293,7 +293,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="KmsMasterKey has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="KmsMasterKey has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -311,7 +311,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="TestDomain has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="TestDomain has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -329,7 +329,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResource has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="NonexistentResource has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -345,7 +345,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResource has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="NonexistentResource has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -355,7 +355,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
                 ),
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResourceSecond has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="NonexistentResourceSecond has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -371,7 +371,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResourceSecond has forbidden cross-account with arn:aws:sts::999999999:assumed-role/test-role/session",
+                    reason="NonexistentResourceSecond has forbidden cross-account with `arn:aws:sts::999999999:assumed-role/test-role/session`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -389,7 +389,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResource has forbidden cross-account with arn:aws:iam::999999999:role/[email protected]",
+                    reason="NonexistentResource has forbidden cross-account with `arn:aws:iam::999999999:role/[email protected]`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -405,7 +405,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResource has forbidden cross-account with arn:aws:iam::999999999:role/[email protected]",
+                    reason="NonexistentResource has forbidden cross-account with `arn:aws:iam::999999999:role/[email protected]`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -415,7 +415,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
                 ),
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResourceTwo has forbidden cross-account with arn:aws:iam::999999999:role/[email protected]",
+                    reason="NonexistentResourceTwo has forbidden cross-account with `arn:aws:iam::999999999:role/[email protected]`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -431,7 +431,7 @@ def test_generic_cross_account_for_opensearch_domain_different_principals(princi
             [
                 Failure(
                     granularity=RuleGranularity.RESOURCE,
-                    reason="NonexistentResourceTwo has forbidden cross-account with arn:aws:iam::999999999:role/[email protected]",
+                    reason="NonexistentResourceTwo has forbidden cross-account with `arn:aws:iam::999999999:role/[email protected]`",
                     risk_value=RuleRisk.MEDIUM,
                     rule="GenericCrossAccountTrustRule",
                     rule_mode=RuleMode.BLOCKING,
@@ -487,7 +487,7 @@ def test_generic_cross_account_rule_es_domain_cross_account_failure(principal):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason=f"TestDomain has forbidden cross-account with {principal}",
+                reason=f"TestDomain has forbidden cross-account with `{principal}`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -532,7 +532,7 @@ def test_generic_cross_account_with_kms_key_failure(principal):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason=f"KMSKey has forbidden cross-account with {principal}",
+                reason=f"KMSKey has forbidden cross-account with `{principal}`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericCrossAccountTrustRule",
                 rule_mode=RuleMode.BLOCKING,

diff --git a/tests/rules/test_GenericResourcePartialWildcardPrincipal.py b/tests/rules/test_GenericResourcePartialWildcardPrincipal.py
@@ -47,7 +47,7 @@ def test_failures_are_raised(bad_template):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at 'arn:aws:iam::123445:12345*'",
+                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `arn:aws:iam::123445:12345*`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -57,7 +57,7 @@ def test_failures_are_raised(bad_template):
             ),
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at 'arn:aws:iam::123445:root'",
+                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `arn:aws:iam::123445:root`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -67,7 +67,7 @@ def test_failures_are_raised(bad_template):
             ),
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at '79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be'",
+                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -77,7 +77,7 @@ def test_failures_are_raised(bad_template):
             ),
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at 'eb2fe74dc7e8125d8f8fcae89d90e6dfdecabf896e1a69d55e949b009fd95a97'",
+                reason="PolicyA should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `eb2fe74dc7e8125d8f8fcae89d90e6dfdecabf896e1a69d55e949b009fd95a97`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -99,7 +99,7 @@ def test_failures_for_correct_account_ids(intra_account_root_access):
         [
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="AccLoadBalancerAccessLogBucketPolicy should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at 'arn:aws:iam::123456789012:root'",
+                reason="AccLoadBalancerAccessLogBucketPolicy should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `arn:aws:iam::123456789012:root`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,
@@ -109,7 +109,7 @@ def test_failures_for_correct_account_ids(intra_account_root_access):
             ),
             Failure(
                 granularity=RuleGranularity.RESOURCE,
-                reason="AccLoadBalancerAccessLogBucketPolicy should not allow wildcard, account-wide or root in resource-id like 'arn:aws:iam::12345:root' at '987654321012'",
+                reason="AccLoadBalancerAccessLogBucketPolicy should not allow wildcard, account-wide or root in resource-id like `arn:aws:iam::12345:root` at `987654321012`",
                 risk_value=RuleRisk.MEDIUM,
                 rule="GenericResourcePartialWildcardPrincipalRule",
                 rule_mode=RuleMode.BLOCKING,