(CAT-1301) Add check unsafe interpolations check

GSPatton · GSPatton · commit 0b3812c2b4bd · 2023-08-18T11:30:07.000+01:00
This commit adds the check unsafe interpolations check to core puppet-lint insted of having it as a standalone gem
diff --git a/lib/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations.rb b/lib/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations.rb
@@ -0,0 +1,130 @@
+PuppetLint.new_check(:check_unsafe_interpolations) do
+  COMMANDS = Array['command', 'onlyif', 'unless']
+  INTERPOLATED_STRINGS = Array[:DQPRE, :DQMID]
+  USELESS_CHARS = Array[:WHITESPACE, :COMMA]
+  def check
+      # Gather any exec commands' resources into an array
+      exec_resources = resource_indexes.map { |resource|
+      resource_parameters = resource[:param_tokens].map(&:value)
+      resource if resource[:type].value == 'exec' && !(COMMANDS & resource_parameters).empty?
+      }.compact
+
+      # Iterate over title tokens and raise a warning if any are variables
+      unless get_exec_titles.empty?
+      get_exec_titles.each do |title|
+          check_unsafe_title(title)
+      end
+      end
+
+      # Iterate over each command found in any exec
+      exec_resources.each do |command_resources|
+      check_unsafe_interpolations(command_resources)
+      end
+  end
+
+  # Iterate over the tokens in a title and raise a warning if an interpolated variable is found
+  def check_unsafe_title(title)
+      title.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|
+      # 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)
+
+      check_command(token).each do |t|
+          notify_warning(t)
+      end
+      end
+  end
+
+  # Raises a warning given a token and message
+  def notify_warning(token)
+      notify :warning,
+              message: "unsafe interpolation of variable '#{token.value}' in exec command",
+              line: token.line,
+              column: token.column
+  end
+
+  # Iterates over the tokens in a command and adds it to an array of violations if it is an input variable
+  def check_command(token)
+      # Initialise variables needed in while loop
+      rule_violations = []
+      current_token = token
+
+      # Iterate through tokens in command
+      while current_token.type != :NEWLINE
+      # Check if token is a varibale and if it is parameterised
+      rule_violations.append(current_token.next_code_token) if interpolated?(current_token)
+      current_token = current_token.next_token
+      end
+
+      rule_violations
+  end
+
+  # A command is parameterised if its args are placed in an array
+  # This function checks if the current token is a :FARROW and if so, if it is followed by an LBRACK
+  def parameterised?(token)
+      current_token = token
+      while current_token.type != :NEWLINE
+      return true if current_token.type == :FARROW && current_token.next_token.next_token.type == :LBRACK
+      current_token = current_token.next_token
+      end
+  end
+
+  # 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_exec_titles
+      result = []
+      tokens.each_index do |token_idx|
+      next unless ['exec'].include?(tokens[token_idx].value)
+      # We have a resource declaration. Now find the title
+      tokens_array = []
+      # Check if title is an array
+      if tokens[token_idx].next_code_token.next_code_token.type == :LBRACK
+          # Get the start and end indices of the array of titles
+          array_start_idx = tokens.rindex { |r| r.type == :LBRACK }
+          array_end_idx = tokens.rindex { |r| r.type == :RBRACK }
+
+          # Grab everything within the array
+          title_array_tokens = tokens[(array_start_idx + 1)..(array_end_idx - 1)]
+          tokens_array.concat(title_array_tokens.reject do |token|
+          USELESS_CHARS.include?(token.type)
+          end)
+          result << tokens_array
+      # Check if title is double quotes string
+      elsif tokens[token_idx].next_code_token.next_code_token.type == :DQPRE
+          # Find the start and end of the title
+          title_start_idx = tokens.find_index(tokens[token_idx].next_code_token.next_code_token)
+          title_end_idx = title_start_idx + index_offset_for(':', tokens[title_start_idx..tokens.length])
+
+          result << tokens[title_start_idx..title_end_idx]
+      # Title is in single quotes
+      else
+          tokens_array.concat([tokens[token_idx].next_code_token.next_code_token])
+
+          result << tokens_array
+      end
+      end
+      result
+  end
+
+  def interpolated?(token)
+      INTERPOLATED_STRINGS.include?(token.type)
+  end
+
+  # Finds the index offset of the next instance of `value` in `tokens_slice` from the original index
+  def index_offset_for(value, tokens_slice)
+      i = 0
+      while i < tokens_slice.length
+      return i if value.include?(tokens_slice[i].value)
+      i += 1
+      end
+  end
+end
+  
diff --git a/spec/unit/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations_spec.rb b/spec/unit/puppet-lint/plugins/check_unsafe_interpolations/check_unsafe_interpolations_spec.rb
@@ -0,0 +1,186 @@
+require 'spec_helper'
+
+describe 'check_unsafe_interpolations' do
+  let(:msg) { "unsafe interpolation of variable 'foo' in exec command" }
+
+  context 'with fix disabled' do
+    context 'exec with unsafe interpolation in command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { 'bar':
+            command => "echo ${foo}",
+          }
+
+        }
+        PUPPET
+      end
+
+      it 'detects an unsafe exec command argument' do
+        expect(problems).to have(1).problems
+      end
+
+      it 'creates one warning' do
+        expect(problems).to contain_warning(msg)
+      end
+    end
+
+    context 'exec with multiple unsafe interpolations in command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { 'bar':
+            command => "echo ${foo} ${bar}",
+          }
+
+        }
+        PUPPET
+      end
+
+      it 'detects multiple unsafe exec command arguments' do
+        expect(problems).to have(2).problems
+      end
+
+      it 'creates two warnings' do
+        expect(problems).to contain_warning(msg)
+        expect(problems).to contain_warning(msg)
+      end
+    end
+
+    context 'code that uses title with unsafe string as command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { "echo ${foo}": }
+
+        }
+        PUPPET
+      end
+
+      it 'detects one problem' do
+        expect(problems).to have(1).problems
+      end
+
+      it 'creates one warning' do
+        expect(problems).to contain_warning(msg)
+      end
+    end
+
+    context 'exec with a safe string in command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { 'bar':
+            command => "echo foo",
+          }
+
+        }
+        PUPPET
+      end
+
+      it 'detects zero problems' do
+        expect(problems).to have(0).problems
+      end
+    end
+
+    context 'exec that has an array of args in command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { 'bar':
+            command => ['echo', $foo],
+          }
+        }
+        PUPPET
+      end
+
+      it 'detects zero problems' do
+        expect(problems).to have(0).problems
+      end
+    end
+
+    context 'exec that has an array of args in command' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+
+          exec { ["foo", "bar", "baz"]:
+            command => echo qux,
+          }
+        }
+        PUPPET
+      end
+
+      it 'detects zero problems' do
+        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
+
+    context 'case statement and an exec' do
+      let(:code) do
+        <<-PUPPET
+        class foo {
+          case bar {
+            baz : {
+              echo qux
+            }
+          }
+
+          exec { 'foo':
+            command => "echo bar",
+          }
+        }
+        PUPPET
+      end
+
+      it 'detects zero problems' do
+        expect(problems).to have(0).problems
+      end
+    end
+  end
+end
