diff --git a/lib/puppet-lint/plugins/check_unsafe_interpolations.rb b/lib/puppet-lint/plugins/check_unsafe_interpolations.rb index 2e63096..398bd57 100644 --- a/lib/puppet-lint/plugins/check_unsafe_interpolations.rb +++ b/lib/puppet-lint/plugins/check_unsafe_interpolations.rb @@ -1,5 +1,6 @@ PuppetLint.new_check(:check_unsafe_interpolations) do COMMANDS = Array['command', 'onlyif', 'unless'] + INTERPOLATED_STRINGS = Array[:DQPRE, :DQMID] def check # Gather any exec commands' resources into an array exec_resources = resource_indexes.map { |resource| @@ -7,13 +8,15 @@ def check resource if resource[:type].value == 'exec' && !(COMMANDS & resource_parameters).empty? }.compact - # Get title tokens with modified puppet lint function - title_tokens_list = get_title_tokens + # Filter the list of titles returned by get_title_tokens for those contained in an exec + exec_title_tokens = get_title_tokens.select { |title| + title if title[:resource_type].value == 'exec' + }.compact # Iterate over title tokens and raise a warning if any are variables - title_tokens_list[:tokens].each do |token| - if token.type == :VARIABLE - notify_warning(token) + unless exec_title_tokens.empty? + exec_title_tokens.each do |title| + check_unsafe_title(title) end end @@ -23,11 +26,18 @@ def check end end - # Iterates over tokens in a command and raises a warning if an interpolated variable is found + # Iterate over the tokens in a title and raise a warning if an interpolated variable is found + def check_unsafe_title(title) + title[:tokens].each do |token| + notify_warning(token.next_code_token) if interpolated?(token) + end + end + + # Iterates over an exec resource and if a command, onlyif or unless paramter is found, it is checked for unsafe interpolations def check_unsafe_interpolations(command_resources) command_resources[:tokens].each do |token| - # We are only interested in tokens from command onwards - next unless token.type == :NAME + # Skip iteration if token isn't a command of type :NAME + next unless COMMANDS.include?(token.value) && token.type == :NAME # Don't check the command if it is parameterised next if parameterised?(token) @@ -54,9 +64,7 @@ def check_command(token) # Iterate through tokens in command while current_token.type != :NEWLINE # Check if token is a varibale and if it is parameterised - if current_token.type == :VARIABLE - rule_violations.append(current_token) - end + rule_violations.append(current_token.next_code_token) if interpolated?(current_token) current_token = current_token.next_token end @@ -76,45 +84,48 @@ def parameterised?(token) # This function is a replacement for puppet_lint's title_tokens function which assumes titles have single quotes # This function adds a check for titles in double quotes where there could be interpolated variables def get_title_tokens - tokens_array = [] - result = {} + result = [] tokens.each_index do |token_idx| - if tokens[token_idx].type == :COLON - # Check if title is an array - if tokens[token_idx - 1].type == :RBRACK - array_start_idx = tokens.rindex do |r| - r.type == :LBRACK - end - title_array_tokens = tokens[(array_start_idx + 1)..(token_idx - 2)] - tokens_array.concat(title_array_tokens.select do |token| - { STRING: true, NAME: true }.include?(token.type) - end) - result = { - tokens: tokens_array, - resource_type: tokens[array_start_idx].prev_code_token.prev_code_token - } - # Check if title is double quotes string - elsif tokens[token_idx - 1].type == :DQPOST - # Find index of the start of the title - title_start_idx = tokens.rindex do |r| - r.type == :DQPRE - end - result = { - # Title is tokens from :DQPRE to the index before :COLON - tokens: tokens[title_start_idx..(token_idx - 1)], - resource_type: tokens[title_start_idx].prev_code_token.prev_code_token - } - # Title is in single quotes - else - next_token = tokens[token_idx].next_code_token - tokens_array.concat(tokens[..token_idx - 1]) unless next_token.type == :LBRACE - result = { - tokens: tokens_array, - resource_type: tokens[token_idx - 1].prev_code_token.prev_code_token - } + next unless tokens[token_idx].type == :COLON + tokens_array = [] + # Check if title is an array + if tokens[token_idx - 1].type == :RBRACK + array_start_idx = tokens.rindex do |r| + r.type == :LBRACK + end + title_array_tokens = tokens[(array_start_idx + 1)..(token_idx - 2)] + tokens_array.concat(title_array_tokens.select do |token| + { STRING: true, NAME: true }.include?(token.type) + end) + result << { + tokens: tokens_array, + resource_type: tokens[array_start_idx].prev_code_token.prev_code_token + } + # Check if title is double quotes string + elsif tokens[token_idx - 1].type == :DQPOST + # Find index of the start of the title + title_start_idx = tokens.rindex do |r| + r.type == :DQPRE end + result << { + # Title is tokens from :DQPRE to the index before :COLON + tokens: tokens[title_start_idx..(token_idx - 1)], + resource_type: tokens[title_start_idx].prev_code_token.prev_code_token + } + # Title is in single quotes + else + next_token = tokens[token_idx].next_code_token + tokens_array.concat([tokens[token_idx - 1]]) unless next_token.type == :LBRACE + result << { + tokens: tokens_array, + resource_type: tokens[token_idx - 1].prev_code_token.prev_code_token + } end end result end + + def interpolated?(token) + INTERPOLATED_STRINGS.include?(token.type) + end end diff --git a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb index 261cd27..da9dd37 100644 --- a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb +++ b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb @@ -120,5 +120,45 @@ class foo { expect(problems).to have(0).problems end end + + context 'file resource' do + let(:code) do + <<-PUPPET + class foo { + file { '/etc/bar': + ensure => file, + backup => false, + content => $baz, + } + } + PUPPET + end + + it 'detects zero problems' do + expect(problems).to have(0).problems + end + end + + context 'file resource and an exec with unsafe interpolation in command' do + let(:code) do + <<-PUPPET + class foo { + file { '/etc/bar': + ensure => file, + backup => false, + content => $baz, + } + + exec { 'qux': + command => "echo ${foo}", + } + } + PUPPET + end + + it 'detects one problem' do + expect(problems).to have(1).problems + end + end end end