Skip to content

Commit 8eaddd1

Browse files
authored
Merge pull request #142 from puppetlabs/CAT-1301-add_unsafe_interpolations_check
(CAT-1301) Add check unsafe interpolations check
2 parents e01caaa + d3c9d9f commit 8eaddd1

File tree

3 files changed

+340
-12
lines changed

3 files changed

+340
-12
lines changed

.rubocop_todo.yml

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# This configuration was generated by
22
# `rubocop --auto-gen-config`
3-
# on 2023-04-04 08:44:46 UTC using RuboCop version 1.48.1.
3+
# on 2023-08-25 10:33:19 UTC using RuboCop version 1.48.1.
44
# The point is for the user to remove these configuration records
55
# one by one as the offenses are removed from the code base.
66
# Note that changes in the inspected code, or installation of new
@@ -34,12 +34,12 @@ Lint/MissingCopEnableDirective:
3434
- 'lib/puppet-lint/tasks/puppet-lint.rb'
3535
- 'spec/unit/puppet-lint/puppet-lint_spec.rb'
3636

37-
# Offense count: 58
37+
# Offense count: 60
3838
# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
3939
Metrics/AbcSize:
4040
Max: 142
4141

42-
# Offense count: 34
42+
# Offense count: 36
4343
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode.
4444
# AllowedMethods: refine
4545
Metrics/BlockLength:
@@ -55,22 +55,27 @@ Metrics/BlockNesting:
5555
Metrics/ClassLength:
5656
Max: 387
5757

58-
# Offense count: 33
58+
# Offense count: 35
5959
# Configuration parameters: AllowedMethods, AllowedPatterns.
6060
Metrics/CyclomaticComplexity:
6161
Max: 33
6262

63-
# Offense count: 80
63+
# Offense count: 82
6464
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
6565
Metrics/MethodLength:
6666
Max: 108
6767

68-
# Offense count: 26
68+
# Offense count: 27
6969
# Configuration parameters: AllowedMethods, AllowedPatterns.
7070
Metrics/PerceivedComplexity:
7171
Max: 31
7272

73-
# Offense count: 182
73+
# Offense count: 1
74+
Naming/AccessorMethodName:
75+
Exclude:
76+
- 'lib/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations.rb'
77+
78+
# Offense count: 186
7479
# Configuration parameters: ForbiddenDelimiters.
7580
# ForbiddenDelimiters: (?i-mx:(^|\s)(EO[A-Z]{1}|END)(\s|$))
7681
Naming/HeredocDelimiterNaming:
@@ -92,13 +97,13 @@ Performance/CollectionLiteralInLoop:
9297
- 'lib/puppet-lint/plugins/check_resources/ensure_first_param.rb'
9398
- 'lib/puppet-lint/plugins/check_whitespace/trailing_whitespace.rb'
9499

95-
# Offense count: 406
100+
# Offense count: 417
96101
# Configuration parameters: Prefixes, AllowedPatterns.
97102
# Prefixes: when, with, without
98103
RSpec/ContextWording:
99104
Enabled: false
100105

101-
# Offense count: 41
106+
# Offense count: 42
102107
# Configuration parameters: IgnoredMetadata.
103108
RSpec/DescribeClass:
104109
Enabled: false
@@ -122,7 +127,7 @@ RSpec/FilePath:
122127
- 'spec/unit/puppet-lint/lexer_spec.rb'
123128
- 'spec/unit/puppet-lint/puppet-lint_spec.rb'
124129

125-
# Offense count: 138
130+
# Offense count: 139
126131
RSpec/MultipleExpectations:
127132
Max: 137
128133

@@ -136,14 +141,21 @@ RSpec/MultipleMemoizedHelpers:
136141
RSpec/NestedGroups:
137142
Max: 5
138143

139-
# Offense count: 8
144+
# Offense count: 10
140145
RSpec/RepeatedExampleGroupDescription:
141146
Exclude:
142147
- 'spec/unit/puppet-lint/plugins/check_resources/file_mode_spec.rb'
143148
- 'spec/unit/puppet-lint/plugins/check_resources/unquoted_file_mode_spec.rb'
149+
- 'spec/unit/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations_spec.rb'
144150
- 'spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb'
145151

146-
# Offense count: 106
152+
# Offense count: 1
153+
# This cop supports unsafe autocorrection (--autocorrect-all).
154+
Style/ConcatArrayLiterals:
155+
Exclude:
156+
- 'lib/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations.rb'
157+
158+
# Offense count: 108
147159
# This cop supports unsafe autocorrection (--autocorrect-all).
148160
# Configuration parameters: EnforcedStyle.
149161
# SupportedStyles: always, always_true, never
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
COMMANDS = Array['command', 'onlyif', 'unless']
2+
INTERPOLATED_STRINGS = Array[:DQPRE, :DQMID]
3+
USELESS_CHARS = Array[:WHITESPACE, :COMMA]
4+
5+
PuppetLint.new_check(:check_unsafe_interpolations) do
6+
def check
7+
# Gather any exec commands' resources into an array
8+
exec_resources = resource_indexes.filter_map do |resource|
9+
resource_parameters = resource[:param_tokens].map(&:value)
10+
resource if resource[:type].value == 'exec' && !(COMMANDS & resource_parameters).empty?
11+
end
12+
13+
# Iterate over title tokens and raise a warning if any are variables
14+
unless get_exec_titles.empty?
15+
get_exec_titles.each do |title|
16+
check_unsafe_title(title)
17+
end
18+
end
19+
20+
# Iterate over each command found in any exec
21+
exec_resources.each do |command_resources|
22+
check_unsafe_interpolations(command_resources)
23+
end
24+
end
25+
26+
# Iterate over the tokens in a title and raise a warning if an interpolated variable is found
27+
def check_unsafe_title(title)
28+
title.each do |token|
29+
notify_warning(token.next_code_token) if interpolated?(token)
30+
end
31+
end
32+
33+
# Iterates over an exec resource and if a command, onlyif or unless paramter is found, it is checked for unsafe interpolations
34+
def check_unsafe_interpolations(command_resources)
35+
command_resources[:tokens].each do |token|
36+
# Skip iteration if token isn't a command of type :NAME
37+
next unless COMMANDS.include?(token.value) && token.type == :NAME
38+
# Don't check the command if it is parameterised
39+
next if parameterised?(token)
40+
41+
check_command(token).each do |t|
42+
notify_warning(t)
43+
end
44+
end
45+
end
46+
47+
# Raises a warning given a token and message
48+
def notify_warning(token)
49+
notify :warning,
50+
message: "unsafe interpolation of variable '#{token.value}' in exec command",
51+
line: token.line,
52+
column: token.column
53+
end
54+
55+
# Iterates over the tokens in a command and adds it to an array of violations if it is an input variable
56+
def check_command(token)
57+
# Initialise variables needed in while loop
58+
rule_violations = []
59+
current_token = token
60+
61+
# Iterate through tokens in command
62+
while current_token.type != :NEWLINE
63+
# Check if token is a varibale and if it is parameterised
64+
rule_violations.append(current_token.next_code_token) if interpolated?(current_token)
65+
current_token = current_token.next_token
66+
end
67+
68+
rule_violations
69+
end
70+
71+
# A command is parameterised if its args are placed in an array
72+
# This function checks if the current token is a :FARROW and if so, if it is followed by an LBRACK
73+
def parameterised?(token)
74+
current_token = token
75+
while current_token.type != :NEWLINE
76+
return true if current_token.type == :FARROW && current_token.next_token.next_token.type == :LBRACK
77+
78+
current_token = current_token.next_token
79+
end
80+
end
81+
82+
# This function is a replacement for puppet_lint's title_tokens function which assumes titles have single quotes
83+
# This function adds a check for titles in double quotes where there could be interpolated variables
84+
def get_exec_titles
85+
result = []
86+
tokens.each_with_index do |_token, token_idx|
87+
next if tokens[token_idx].value != 'exec'
88+
89+
# We have a resource declaration. Now find the title
90+
tokens_array = []
91+
# Check if title is an array
92+
if tokens[token_idx]&.next_code_token&.next_code_token&.type == :LBRACK
93+
# Get the start and end indices of the array of titles
94+
array_start_idx = tokens.rindex { |r| r.type == :LBRACK }
95+
array_end_idx = tokens.rindex { |r| r.type == :RBRACK }
96+
97+
# Grab everything within the array
98+
title_array_tokens = tokens[(array_start_idx + 1)..(array_end_idx - 1)]
99+
tokens_array.concat(title_array_tokens.reject do |token|
100+
USELESS_CHARS.include?(token.type)
101+
end)
102+
result << tokens_array
103+
# Check if title is double quotes string
104+
elsif tokens[token_idx].next_code_token.next_code_token.type == :DQPRE
105+
# Find the start and end of the title
106+
title_start_idx = tokens.find_index(tokens[token_idx].next_code_token.next_code_token)
107+
title_end_idx = title_start_idx + index_offset_for(':', tokens[title_start_idx..tokens.length])
108+
109+
result << tokens[title_start_idx..title_end_idx]
110+
# Title is in single quotes
111+
else
112+
tokens_array.concat([tokens[token_idx].next_code_token.next_code_token])
113+
114+
result << tokens_array
115+
end
116+
end
117+
result
118+
end
119+
120+
def interpolated?(token)
121+
INTERPOLATED_STRINGS.include?(token.type)
122+
end
123+
124+
# Finds the index offset of the next instance of `value` in `tokens_slice` from the original index
125+
def index_offset_for(value, tokens_slice)
126+
tokens_slice.each_with_index do |token, i|
127+
return i if value.include?(token.value)
128+
end
129+
end
130+
end

0 commit comments

Comments
 (0)