Skip to content

Commit b4d790d

Browse files
authored
Add rule W1051 to validate if dyn ref when arn (#3962)
1 parent 1a259af commit b4d790d

File tree

5 files changed

+188
-4
lines changed

5 files changed

+188
-4
lines changed

src/cfnlint/rules/functions/DynamicReference.py

+5-3
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ def __init__(self) -> None:
7777
super().__init__()
7878
self.child_rules = {
7979
"E1051": None,
80+
"W1051": None,
8081
"E1027": None,
8182
}
8283

@@ -125,9 +126,10 @@ def dynamicReference(
125126
evolved = validator.evolve(schema=_secrets_manager_arn)
126127
else: # this is secrets manager
127128
evolved = validator.evolve(schema=_secrets_manager)
128-
rule = self.child_rules["E1051"]
129-
if rule and hasattr(rule, "validate"):
130-
yield from rule.validate(validator, {}, v, schema)
129+
for rule_id in ["E1051", "W1051"]:
130+
rule = self.child_rules[rule_id]
131+
if rule and hasattr(rule, "validate"):
132+
yield from rule.validate(validator, {}, v, schema)
131133

132134
for err in evolved.iter_errors(parts):
133135
yield self._clean_errors(err)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
from typing import Any
7+
8+
import regex as re
9+
10+
from cfnlint.jsonschema import ValidationError, Validator
11+
from cfnlint.rules import CloudFormationLintRule
12+
13+
14+
class DynamicReferenceSecretsManagerArn(CloudFormationLintRule):
15+
id = "W1051"
16+
shortdesc = (
17+
"Validate dynamic references to secrets manager "
18+
"are not used when a secrets manager ARN was expected"
19+
)
20+
description = (
21+
"Certain properties expect a secret manager ARN. This rule "
22+
"validates if you may be accidently using a secret in place "
23+
"of the ARN"
24+
)
25+
source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/dynamic-references.html#dynamic-references-secretsmanager"
26+
tags = ["functions", "dynamic reference"]
27+
28+
def validate(self, validator: Validator, s: Any, instance: Any, schema: Any):
29+
30+
if "Fn::Sub" == validator.context.path.path[-1]:
31+
if not re.match(r"^(\\\")?{{resolve:secretsmanager:.*}}(\\\")?$", instance):
32+
return
33+
34+
if len(validator.context.path.path) < 3:
35+
return
36+
37+
if (
38+
validator.context.path.path[0] != "Resources"
39+
or validator.context.path.path[2] != "Properties"
40+
):
41+
return
42+
43+
fields = [
44+
"SecretArn",
45+
"SecretARN",
46+
"SecretsManagerSecretId",
47+
"SecretsManagerOracleAsmSecretId",
48+
"SecretsManagerSecurityDbEncryptionSecretId",
49+
"SecretsManagerConfiguration",
50+
]
51+
52+
for field in fields:
53+
if any(field == p for p in validator.context.path.path):
54+
yield ValidationError(
55+
(
56+
f"Dynamic reference {instance!r} to secrets manager when "
57+
f"the field {field!r} expects the ARN to the secret and "
58+
"not the secret"
59+
),
60+
rule=self,
61+
)

src/cfnlint/rules/functions/Sub.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ def __init__(self) -> None:
3131
{
3232
"W1019": None,
3333
"W1020": None,
34+
"W1051": None,
3435
}
3536
)
3637
self._functions = [
@@ -142,9 +143,14 @@ def fn_sub(
142143

143144
# we know the structure is valid at this point
144145
# so any child rule doesn't have to revalidate it
146+
value_validator = validator.evolve(
147+
context=validator.context.evolve(
148+
path=validator.context.path.descend(path=key)
149+
)
150+
)
145151
for _, rule in self.child_rules.items():
146152
if rule and hasattr(rule, "validate"):
147-
for err in rule.validate(validator, s, instance.get("Fn::Sub"), schema):
153+
for err in rule.validate(value_validator, s, value, schema):
148154
err.path.append("Fn::Sub")
149155
err.rule = rule
150156
yield err

test/unit/rules/functions/test_dynamic_reference.py

+13
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ def context(cfn):
4747
{
4848
"E1051": _TestRule(),
4949
"E1027": _TestRule(),
50+
"W1051": _TestRule(),
5051
},
5152
[],
5253
),
@@ -57,6 +58,7 @@ def context(cfn):
5758
{
5859
"E1051": None,
5960
"E1027": None,
61+
"W1051": None,
6062
},
6163
[],
6264
),
@@ -67,6 +69,7 @@ def context(cfn):
6769
{
6870
"E1051": _TestRule(),
6971
"E1027": _TestRule(),
72+
"W1051": _TestRule(),
7073
},
7174
[],
7275
),
@@ -77,6 +80,7 @@ def context(cfn):
7780
{
7881
"E1051": None,
7982
"E1027": None,
83+
"W1051": None,
8084
},
8185
[],
8286
),
@@ -87,6 +91,7 @@ def context(cfn):
8791
{
8892
"E1051": _TestRule(),
8993
"E1027": _TestRule(),
94+
"W1051": _TestRule(),
9095
},
9196
[],
9297
),
@@ -97,6 +102,7 @@ def context(cfn):
97102
{
98103
"E1051": _TestRule(),
99104
"E1027": _TestRule(),
105+
"W1051": _TestRule(),
100106
},
101107
[],
102108
),
@@ -107,6 +113,7 @@ def context(cfn):
107113
{
108114
"E1051": _TestRule(),
109115
"E1027": _TestRule(),
116+
"W1051": _TestRule(),
110117
},
111118
[
112119
ValidationError(
@@ -123,6 +130,7 @@ def context(cfn):
123130
{
124131
"E1051": _TestRule(),
125132
"E1027": _TestRule(),
133+
"W1051": _TestRule(),
126134
},
127135
[
128136
ValidationError(
@@ -139,6 +147,7 @@ def context(cfn):
139147
{
140148
"E1051": _TestRule(),
141149
"E1027": _TestRule(),
150+
"W1051": _TestRule(),
142151
},
143152
[],
144153
),
@@ -149,6 +158,7 @@ def context(cfn):
149158
{
150159
"E1051": None,
151160
"E1027": None,
161+
"W1051": None,
152162
},
153163
[],
154164
),
@@ -159,6 +169,7 @@ def context(cfn):
159169
{
160170
"E1051": _TestRule(),
161171
"E1027": _TestRule(),
172+
"W1051": _TestRule(),
162173
},
163174
[],
164175
),
@@ -169,6 +180,7 @@ def context(cfn):
169180
{
170181
"E1051": _TestRule(),
171182
"E1027": _TestRule(),
183+
"W1051": _TestRule(),
172184
},
173185
[],
174186
),
@@ -179,6 +191,7 @@ def context(cfn):
179191
{
180192
"E1051": _TestRule(),
181193
"E1027": _TestRule(),
194+
"W1051": _TestRule(),
182195
},
183196
[
184197
ValidationError(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
from collections import deque
7+
8+
import pytest
9+
10+
from cfnlint.jsonschema import ValidationError
11+
from cfnlint.rules.functions.DynamicReferenceSecretsManagerArn import (
12+
DynamicReferenceSecretsManagerArn,
13+
)
14+
15+
16+
@pytest.fixture(scope="module")
17+
def rule():
18+
rule = DynamicReferenceSecretsManagerArn()
19+
yield rule
20+
21+
22+
@pytest.mark.parametrize(
23+
"name,instance,path,expected",
24+
[
25+
(
26+
"Valid secrets manager",
27+
"{{resolve:secretsmanager:Parameter}}",
28+
{
29+
"path": deque(["Resources", "MyResource", "Properties", "Password"]),
30+
},
31+
[],
32+
),
33+
(
34+
"Valid sub not to secrets manaager",
35+
"secretsmanager",
36+
{
37+
"path": deque(
38+
["Resources", "MyResource", "Properties", "Password", "Fn::Sub"]
39+
),
40+
},
41+
[],
42+
),
43+
(
44+
"Valid secrets manager outside of Resources",
45+
"{{resolve:secretsmanager:Parameter}}",
46+
{
47+
"path": deque(["Parameters", "Type", "Default"]),
48+
},
49+
[],
50+
),
51+
(
52+
"Valid secrets manager outside of Resources",
53+
"{{resolve:secretsmanager:Parameter}}",
54+
{
55+
"path": deque(["Metadata", "Value"]),
56+
},
57+
[],
58+
),
59+
(
60+
"Invalid secrets manager when ARN was expected",
61+
"{{resolve:secretsmanager:secret}}",
62+
{
63+
"path": deque(["Resources", "MyResource", "Properties", "SecretArn"]),
64+
},
65+
[
66+
ValidationError(
67+
(
68+
"Dynamic reference '{{resolve:secretsmanager:secret}}'"
69+
" to secrets manager when the field 'SecretArn' expects "
70+
"the ARN to the secret and not the secret"
71+
),
72+
rule=DynamicReferenceSecretsManagerArn(),
73+
)
74+
],
75+
),
76+
(
77+
"Invalid secrets manager when ARN was expected",
78+
'\\"{{resolve:secretsmanager:${MyParameter}}}\\"',
79+
{
80+
"path": deque(
81+
["Resources", "MyResource", "Properties", "SecretArn", "Fn::Sub"]
82+
),
83+
},
84+
[
85+
ValidationError(
86+
(
87+
"Dynamic reference "
88+
"'\\\\\"{{resolve:secretsmanager:${MyParameter}}}\\\\\"'"
89+
" to secrets manager when the field 'SecretArn' "
90+
"expects the ARN to the secret and not the secret"
91+
),
92+
rule=DynamicReferenceSecretsManagerArn(),
93+
)
94+
],
95+
),
96+
],
97+
indirect=["path"],
98+
)
99+
def test_validate(name, instance, path, expected, validator, rule):
100+
errs = list(rule.validate(validator, {}, instance, {}))
101+
102+
assert errs == expected, f"Test {name!r} got {errs!r}"

0 commit comments

Comments
 (0)