Skip to content

Commit 7a1ad16

Browse files
author
Matt Luttrell
committed
The Condition intrinsic function should only be evaluated if its in the "Conditions" section.
Fixes #33
1 parent 77b68b6 commit 7a1ad16

24 files changed

Lines changed: 208 additions & 157 deletions

‎cfn_policy_validator/cfn_tools/cfn_object.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def __init__(self, value={}):
1818

1919
def add_ancestors(self, parent_ancestors: list, parent_key: str):
2020
self.ancestors = list(parent_ancestors)
21-
if parent_key is not None and parent_key != 'Resources':
21+
if parent_key is not None:
2222
self.ancestors.append(parent_key)
2323

2424
def set_node_evaluator(self, node_evaluator):
@@ -32,6 +32,10 @@ def parent(self):
3232

3333
return parent[0]
3434

35+
@property
36+
def top_level_ancestor(self):
37+
return self.ancestors[0] if len(self.ancestors) > 0 else None
38+
3539
def eval(self, expected_schema: Any, visited_values: list = None):
3640
path = self.ancestors_as_string()
3741

‎cfn_policy_validator/parsers/utils/node_evaluator.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ def eval_with_validation(self, value: Any, expected_schema: Any, resource_proper
9494

9595
@prune_references_to_no_value
9696
@evaluate_dynamic_references
97-
def eval(self, value: Any, resource_properties_to_eval=None, visited_nodes=None, is_evaluating_conditions=False):
97+
def eval(self, value: Any, resource_properties_to_eval=None, visited_nodes=None):
9898
""" Evaluates the value of a CloudFormation key/value pair by evaluating intrinsic functions and pseudo
9999
parameters in the template and returns the evaluated value.
100100
@@ -114,7 +114,15 @@ def eval(self, value: Any, resource_properties_to_eval=None, visited_nodes=None,
114114
elif isinstance(value, CfnObject):
115115
# intrinsic functions must be the only key, so we only need to look at the first key
116116
first_key = next(iter(value), None)
117-
evaluator = self.evaluators.get(first_key)
117+
118+
# the condition function is a special case since 'condition' can appear in other places like IAM policies
119+
# ignore it if we're not in the "Conditions" section of the template. longer term we should only evaluate
120+
# intrinsic functions in valid sections of the template
121+
if first_key == 'Condition' and value.top_level_ancestor != 'Conditions':
122+
evaluator = None
123+
else:
124+
evaluator = self.evaluators.get(first_key)
125+
118126
if evaluator is not None:
119127
intrinsic_function_value = value.get(first_key)
120128
return evaluator.evaluate(intrinsic_function_value, visited_nodes=visited_nodes)
@@ -133,7 +141,7 @@ def eval(self, value: Any, resource_properties_to_eval=None, visited_nodes=None,
133141
# when passing the list of visited nodes to child evals, create a separate copy for each so
134142
# that they don't share the same visited references
135143
copy_of_visited_nodes = copy.deepcopy(visited_nodes)
136-
value[key] = self.eval(value[key], resource_properties_to_eval, copy_of_visited_nodes, is_evaluating_conditions)
144+
value[key] = self.eval(value[key], resource_properties_to_eval, copy_of_visited_nodes)
137145

138146
return dict(value)
139147

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_kms.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def test_with_no_properties(self):
6060
with self.assertRaises(ApplicationError) as cm:
6161
ResourceParser.parse(template, account_config)
6262

63-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
63+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
6464

6565
@mock_node_evaluator_setup()
6666
def test_with_no_key_policy(self):
@@ -74,7 +74,7 @@ def test_with_no_key_policy(self):
7474
with self.assertRaises(ApplicationError) as cm:
7575
ResourceParser.parse(template, account_config)
7676

77-
self.assertEqual(required_property_error('KeyPolicy', 'ResourceA.Properties'), str(cm.exception))
77+
self.assertEqual(required_property_error('KeyPolicy', 'Resources.ResourceA.Properties'), str(cm.exception))
7878

7979
@mock_node_evaluator_setup()
8080
def test_with_invalid_key_policy(self):
@@ -90,7 +90,7 @@ def test_with_invalid_key_policy(self):
9090
with self.assertRaises(ApplicationError) as cm:
9191
ResourceParser.parse(template, account_config)
9292

93-
self.assertEqual(expected_type_error('ResourceA.Properties.KeyPolicy', 'object', "['Invalid']"), str(cm.exception))
93+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.KeyPolicy', 'object', "['Invalid']"), str(cm.exception))
9494

9595
@mock_node_evaluator_setup()
9696
def test_with_unsupported_function_in_unused_property(self):

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_lambda_aws.py‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def test_with_no_properties(self):
2525
with self.assertRaises(ApplicationError) as cm:
2626
ResourceParser.parse(template, account_config)
2727

28-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
28+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
2929

3030
@mock_node_evaluator_setup()
3131
def test_with_no_action(self):
@@ -42,7 +42,7 @@ def test_with_no_action(self):
4242
with self.assertRaises(ApplicationError) as cm:
4343
ResourceParser.parse(template, account_config)
4444

45-
self.assertEqual(required_property_error('Action', 'ResourceA.Properties'), str(cm.exception))
45+
self.assertEqual(required_property_error('Action', 'Resources.ResourceA.Properties'), str(cm.exception))
4646

4747
@mock_node_evaluator_setup()
4848
def test_with_no_function_name(self):
@@ -59,7 +59,7 @@ def test_with_no_function_name(self):
5959
with self.assertRaises(ApplicationError) as cm:
6060
ResourceParser.parse(template, account_config)
6161

62-
self.assertEqual(required_property_error('FunctionName', 'ResourceA.Properties'), str(cm.exception))
62+
self.assertEqual(required_property_error('FunctionName', 'Resources.ResourceA.Properties'), str(cm.exception))
6363

6464
@mock_node_evaluator_setup()
6565
def test_with_no_principal(self):
@@ -76,7 +76,7 @@ def test_with_no_principal(self):
7676
with self.assertRaises(ApplicationError) as cm:
7777
ResourceParser.parse(template, account_config)
7878

79-
self.assertEqual(required_property_error('Principal', 'ResourceA.Properties'), str(cm.exception))
79+
self.assertEqual(required_property_error('Principal', 'Resources.ResourceA.Properties'), str(cm.exception))
8080

8181
@mock_node_evaluator_setup()
8282
def test_with_unsupported_function_in_unused_property(self):
@@ -148,30 +148,30 @@ def assert_invalid_type(self, template, path, expected_type, invalid_value):
148148
def test_invalid_action_type(self):
149149
invalid_value = ['Invalid']
150150
template = _build_lambda_permissions_policy(action=invalid_value)
151-
self.assert_invalid_type(template, "ResourceA.Properties.Action", 'string', invalid_value)
151+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.Action", 'string', invalid_value)
152152

153153
@mock_node_evaluator_setup()
154154
def test_invalid_function_name_type(self):
155155
invalid_value = ['Invalid']
156156
template = _build_lambda_permissions_policy(function_name=invalid_value)
157-
self.assert_invalid_type(template, "ResourceA.Properties.FunctionName", 'string', invalid_value)
157+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.FunctionName", 'string', invalid_value)
158158

159159
def test_invalid_principal_type(self):
160160
invalid_value = ['Invalid']
161161
template = _build_lambda_permissions_policy(principal=invalid_value)
162-
self.assert_invalid_type(template, "ResourceA.Properties.Principal", 'string', invalid_value)
162+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.Principal", 'string', invalid_value)
163163

164164
@mock_node_evaluator_setup()
165165
def test_invalid_source_arn_type(self):
166166
invalid_value = ['Invalid']
167167
template = _build_lambda_permissions_policy(source_arn=invalid_value)
168-
self.assert_invalid_type(template, "ResourceA.Properties.SourceArn", 'string', invalid_value)
168+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.SourceArn", 'string', invalid_value)
169169

170170
@mock_node_evaluator_setup()
171171
def test_invalid_source_account_type(self):
172172
invalid_value = ['Invalid']
173173
template = _build_lambda_permissions_policy(source_account=invalid_value)
174-
self.assert_invalid_type(template, "ResourceA.Properties.SourceAccount", 'string', invalid_value)
174+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.SourceAccount", 'string', invalid_value)
175175

176176

177177
class WhenParsingALambdaPermissionsPolicy(unittest.TestCase):
@@ -526,7 +526,7 @@ def test_with_no_properties(self):
526526
with self.assertRaises(ApplicationError) as cm:
527527
ResourceParser.parse(template, account_config)
528528

529-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
529+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
530530

531531
@mock_node_evaluator_setup()
532532
def test_with_no_action(self):
@@ -543,7 +543,7 @@ def test_with_no_action(self):
543543
with self.assertRaises(ApplicationError) as cm:
544544
ResourceParser.parse(template, account_config)
545545

546-
self.assertEqual(required_property_error('Action', 'ResourceA.Properties'), str(cm.exception))
546+
self.assertEqual(required_property_error('Action', 'Resources.ResourceA.Properties'), str(cm.exception))
547547

548548
@mock_node_evaluator_setup()
549549
def test_with_no_layer_version_arn(self):
@@ -560,7 +560,7 @@ def test_with_no_layer_version_arn(self):
560560
with self.assertRaises(ApplicationError) as cm:
561561
ResourceParser.parse(template, account_config)
562562

563-
self.assertEqual(required_property_error('LayerVersionArn', 'ResourceA.Properties'), str(cm.exception))
563+
self.assertEqual(required_property_error('LayerVersionArn', 'Resources.ResourceA.Properties'), str(cm.exception))
564564

565565
@mock_node_evaluator_setup()
566566
def test_with_no_principal(self):
@@ -577,7 +577,7 @@ def test_with_no_principal(self):
577577
with self.assertRaises(ApplicationError) as cm:
578578
ResourceParser.parse(template, account_config)
579579

580-
self.assertEqual(required_property_error('Principal', 'ResourceA.Properties'), str(cm.exception))
580+
self.assertEqual(required_property_error('Principal', 'Resources.ResourceA.Properties'), str(cm.exception))
581581

582582

583583
def _build_lambda_layer_version_permissions_policy(action='lambda:GetLayerVersion',
@@ -612,25 +612,25 @@ def assert_invalid_type(self, template, path, expected_type, invalid_value):
612612
def test_invalid_action_type(self):
613613
invalid_value = ['Invalid']
614614
template = _build_lambda_layer_version_permissions_policy(action=invalid_value)
615-
self.assert_invalid_type(template, "ResourceA.Properties.Action", "string", invalid_value)
615+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.Action", "string", invalid_value)
616616

617617
@mock_node_evaluator_setup()
618618
def test_invalid_layer_version_arn_type(self):
619619
invalid_value = ['Invalid']
620620
template = _build_lambda_layer_version_permissions_policy(layer_version_arn=invalid_value)
621-
self.assert_invalid_type(template, "ResourceA.Properties.LayerVersionArn", "string", invalid_value)
621+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.LayerVersionArn", "string", invalid_value)
622622

623623
@mock_node_evaluator_setup()
624624
def test_invalid_principal_type(self):
625625
invalid_value = ['Invalid']
626626
template = _build_lambda_layer_version_permissions_policy(principal=invalid_value)
627-
self.assert_invalid_type(template, "ResourceA.Properties.Principal", "string", invalid_value)
627+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.Principal", "string", invalid_value)
628628

629629
@mock_node_evaluator_setup()
630630
def test_invalid_organization_id_type(self):
631631
invalid_value = ['Invalid']
632632
template = _build_lambda_layer_version_permissions_policy(organization_id=invalid_value)
633-
self.assert_invalid_type(template, "ResourceA.Properties.OrganizationId", "string", invalid_value)
633+
self.assert_invalid_type(template, "Resources.ResourceA.Properties.OrganizationId", "string", invalid_value)
634634

635635
@mock_node_evaluator_setup()
636636
def test_with_unsupported_function_in_unused_property(self):

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_s3_access_point_policy.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def test_with_no_properties(self):
7373
with self.assertRaises(ApplicationError) as cm:
7474
ResourceParser.parse(template, account_config)
7575

76-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
76+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
7777

7878
@mock_node_evaluator_setup()
7979
def test_with_invalid_name_type(self):
@@ -92,7 +92,7 @@ def test_with_invalid_name_type(self):
9292
with self.assertRaises(ApplicationError) as cm:
9393
ResourceParser.parse(template, account_config)
9494

95-
self.assertEqual(expected_type_error('ResourceA.Properties.Name', 'string', "['MyAccessPoint']"), str(cm.exception))
95+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.Name', 'string', "['MyAccessPoint']"), str(cm.exception))
9696

9797
@mock_node_evaluator_setup()
9898
def test_with_invalid_policy_type(self):
@@ -111,7 +111,7 @@ def test_with_invalid_policy_type(self):
111111
with self.assertRaises(ApplicationError) as cm:
112112
ResourceParser.parse(template, account_config)
113113

114-
self.assertEqual(expected_type_error('ResourceA.Properties.Policy', 'object', "['Invalid']"),
114+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.Policy', 'object', "['Invalid']"),
115115
str(cm.exception))
116116

117117
@mock_node_evaluator_setup()
@@ -131,7 +131,7 @@ def test_with_invalid_vpc_configuration_type(self):
131131
with self.assertRaises(ApplicationError) as cm:
132132
ResourceParser.parse(template, account_config)
133133

134-
self.assertEqual(expected_type_error('ResourceA.Properties.VpcConfiguration', 'object', "['Invalid']"),
134+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.VpcConfiguration', 'object', "['Invalid']"),
135135
str(cm.exception))
136136

137137
@mock_node_evaluator_setup()

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_s3_bucket_acls.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def test_with_invalid_bucket_name_type(self):
4646
with self.assertRaises(ApplicationError) as cm:
4747
ResourceParser.parse(template, account_config)
4848

49-
self.assertEqual(expected_type_error('ResourceA.Properties.BucketName', 'string', "['MyBucket']"), str(cm.exception))
49+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.BucketName', 'string', "['MyBucket']"), str(cm.exception))
5050

5151
@mock_node_evaluator_setup()
5252
def test_with_invalid_access_control_type(self):
@@ -62,7 +62,7 @@ def test_with_invalid_access_control_type(self):
6262
with self.assertRaises(ApplicationError) as cm:
6363
ResourceParser.parse(template, account_config)
6464

65-
self.assertEqual(expected_type_error('ResourceA.Properties.AccessControl', 'string', "['Invalid']"),
65+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.AccessControl', 'string', "['Invalid']"),
6666
str(cm.exception))
6767

6868
@mock_node_evaluator_setup()

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_s3_bucket_policy.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def test_with_no_properties(self):
6868
with self.assertRaises(ApplicationError) as cm:
6969
ResourceParser.parse(template, account_config)
7070

71-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
71+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
7272

7373
@mock_node_evaluator_setup()
7474
def test_with_no_bucket(self):
@@ -86,7 +86,7 @@ def test_with_no_bucket(self):
8686
with self.assertRaises(ApplicationError) as cm:
8787
ResourceParser.parse(template, account_config)
8888

89-
self.assertEqual(required_property_error('Bucket', 'ResourceA.Properties'), str(cm.exception))
89+
self.assertEqual(required_property_error('Bucket', 'Resources.ResourceA.Properties'), str(cm.exception))
9090

9191
@mock_node_evaluator_setup()
9292
def test_with_invalid_bucket_type(self):
@@ -103,7 +103,7 @@ def test_with_invalid_bucket_type(self):
103103
with self.assertRaises(ApplicationError) as cm:
104104
ResourceParser.parse(template, account_config)
105105

106-
self.assertEqual(expected_type_error('ResourceA.Properties.Bucket', 'string', "['MyBucket']"), str(cm.exception))
106+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.Bucket', 'string', "['MyBucket']"), str(cm.exception))
107107

108108
@mock_node_evaluator_setup()
109109
def test_with_no_policy_document(self):
@@ -119,7 +119,7 @@ def test_with_no_policy_document(self):
119119
with self.assertRaises(ApplicationError) as cm:
120120
ResourceParser.parse(template, account_config)
121121

122-
self.assertEqual(required_property_error('PolicyDocument', 'ResourceA.Properties'), str(cm.exception))
122+
self.assertEqual(required_property_error('PolicyDocument', 'Resources.ResourceA.Properties'), str(cm.exception))
123123

124124
@mock_node_evaluator_setup()
125125
def test_with_invalid_policy_document_type(self):
@@ -136,7 +136,7 @@ def test_with_invalid_policy_document_type(self):
136136
with self.assertRaises(ApplicationError) as cm:
137137
ResourceParser.parse(template, account_config)
138138

139-
self.assertEqual(expected_type_error('ResourceA.Properties.PolicyDocument', 'object', "['Invalid']"),
139+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.PolicyDocument', 'object', "['Invalid']"),
140140
str(cm.exception))
141141

142142
@mock_node_evaluator_setup()

‎cfn_policy_validator/tests/parsers_tests/resource_tests/test_s3_multi_region_access_point_policy.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def test_with_no_properties(self):
7171
with self.assertRaises(ApplicationError) as cm:
7272
ResourceParser.parse(template, account_config)
7373

74-
self.assertEqual(required_property_error('Properties', 'ResourceA'), str(cm.exception))
74+
self.assertEqual(required_property_error('Properties', 'Resources.ResourceA'), str(cm.exception))
7575

7676
@mock_node_evaluator_setup()
7777
def test_with_no_name(self):
@@ -87,7 +87,7 @@ def test_with_no_name(self):
8787
with self.assertRaises(ApplicationError) as cm:
8888
ResourceParser.parse(template, account_config)
8989

90-
self.assertEqual(required_property_error('MrapName', 'ResourceA.Properties'),
90+
self.assertEqual(required_property_error('MrapName', 'Resources.ResourceA.Properties'),
9191
str(cm.exception))
9292

9393
@mock_node_evaluator_setup()
@@ -105,7 +105,7 @@ def test_with_invalid_name_type(self):
105105
with self.assertRaises(ApplicationError) as cm:
106106
ResourceParser.parse(template, account_config)
107107

108-
self.assertEqual(expected_type_error('ResourceA.Properties.MrapName', 'string', "['MyMultiRegionAccessPoint']"), str(cm.exception))
108+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.MrapName', 'string', "['MyMultiRegionAccessPoint']"), str(cm.exception))
109109

110110
@mock_node_evaluator_setup()
111111
def test_with_no_policy(self):
@@ -121,7 +121,7 @@ def test_with_no_policy(self):
121121
with self.assertRaises(ApplicationError) as cm:
122122
ResourceParser.parse(template, account_config)
123123

124-
self.assertEqual(required_property_error('Policy', 'ResourceA.Properties'), str(cm.exception))
124+
self.assertEqual(required_property_error('Policy', 'Resources.ResourceA.Properties'), str(cm.exception))
125125

126126
@mock_node_evaluator_setup()
127127
def test_with_invalid_policy_type(self):
@@ -138,7 +138,7 @@ def test_with_invalid_policy_type(self):
138138
with self.assertRaises(ApplicationError) as cm:
139139
ResourceParser.parse(template, account_config)
140140

141-
self.assertEqual(expected_type_error('ResourceA.Properties.Policy', 'object', "['Invalid']"),
141+
self.assertEqual(expected_type_error('Resources.ResourceA.Properties.Policy', 'object', "['Invalid']"),
142142
str(cm.exception))
143143

144144
@mock_node_evaluator_setup()

0 commit comments

Comments
 (0)