Merge pull request #9159 from joshcooper/rubocop_main_security

(PUP-11772) Resolve Security cops main

Author: mhashizume

.rubocop_todo.yml

@@ -918,23 +918,6 @@ Naming/VariableName:
 Naming/VariableNumber:
   Enabled: false
 
-Security/Eval:
-  Exclude:
-    - 'lib/puppet/interface/action.rb'
-    - 'lib/puppet/interface/action_builder.rb'
-    - 'lib/puppet/pops/loader/ruby_data_type_instantiator.rb'
-    - 'lib/puppet/pops/loader/ruby_function_instantiator.rb'
-    - 'lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb'
-
-Security/Open:
-  Exclude:
-    - 'lib/puppet/file_system/file_impl.rb'
-    - 'lib/puppet/file_system/posix.rb'
-    - 'lib/puppet/provider/package/appdmg.rb'
-    - 'lib/puppet/provider/package/windows/package.rb'
-    - 'lib/puppet/util/command_line/trollop.rb'
-    - 'lib/puppet/util/execution.rb'
-
 # Configuration parameters: EnforcedStyle, AllowModifiersOnSymbols.
 # SupportedStyles: inline, group
 Style/AccessModifierDeclarations:

examples/enc/regexp_nodes/regexp_nodes.rb

@@ -133,7 +133,7 @@ def matched_in_patternfile?(filepath, matchthis)
     patternlist = []
 
     begin
-      open(filepath).each do |l|
+      File.open(filepath).each do |l|
         l.chomp!
 
         next if l =~ /^$/

lib/puppet/file_system/file_impl.rb

@@ -151,7 +151,7 @@ def lstat(path)
   end
 
   def compare_stream(path, stream)
-    open(path, 0, 'rb') { |this| FileUtils.compare_stream(this, stream) }
+    ::File.open(path, 0, 'rb') { |this| FileUtils.compare_stream(this, stream) }
   end
 
   def chmod(mode, path)

lib/puppet/file_system/posix.rb

@@ -11,7 +11,7 @@ def binread(path)
   # issue this method reimplements the faster 2.0 version that will correctly
   # compare binary File and StringIO streams.
   def compare_stream(path, stream)
-    open(path, 0, 'rb') do |this|
+    ::File.open(path, 'rb') do |this|
       bsize = stream_blksize(this, stream)
       sa = String.new.force_encoding('ASCII-8BIT')
       sb = String.new.force_encoding('ASCII-8BIT')

lib/puppet/interface/action.rb

@@ -265,12 +265,14 @@ def #{@name}(#{decl.join(", ")})
 end
 WRAPPER
 
+    # It should be possible to rewrite this code to use `define_method`
+    # instead of `class/instance_eval` since Ruby 1.8 is long dead.
     if @face.is_a?(Class)
-      @face.class_eval do eval wrapper, nil, file, line end
+      @face.class_eval do eval wrapper, nil, file, line end # rubocop:disable Security/Eval
       @face.send(:define_method, internal_name, &block)
       @when_invoked = @face.instance_method(name)
     else
-      @face.instance_eval do eval wrapper, nil, file, line end
+      @face.instance_eval do eval wrapper, nil, file, line end # rubocop:disable Security/Eval
       @face.meta_def(internal_name, &block)
       @when_invoked = @face.method(name).unbind
     end

lib/puppet/interface/action_builder.rb

@@ -5,6 +5,8 @@
 # within the context of a new instance of this class.
 # @api public
 class Puppet::Interface::ActionBuilder
+  extend Forwardable
+
   # The action under construction
   # @return [Puppet::Interface::Action]
   # @api private
@@ -142,15 +144,8 @@ def render_as(value = nil)
     property = setter.to_s.chomp('=')
 
     unless method_defined? property
-      # Using eval because the argument handling semantics are less awful than
-      # when we use the define_method/block version.  The later warns on older
-      # Ruby versions if you pass the wrong number of arguments, but carries
-      # on, which is totally not what we want. --daniel 2011-04-18
-      eval <<-METHOD
-        def #{property}(value)
-          @action.#{property} = value
-        end
-      METHOD
+      # ActionBuilder#<property> delegates to Action#<setter>
+      def_delegator :@action, setter, property
     end
   end
 

lib/puppet/pops/loader/ruby_data_type_instantiator.rb

@@ -20,7 +20,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
     # make the private loader available in a binding to allow it to be passed on
     loader_for_type = loader.private_loader
     here = get_binding(loader_for_type)
-    created = eval(ruby_code_string, here, source_ref, 1)
+    created = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval
     unless created.is_a?(Puppet::Pops::Types::PAnyType)
       raise ArgumentError, _("The code loaded from %{source_ref} did not produce a data type when evaluated. Got '%{klass}'") % { source_ref: source_ref, klass: created.class }
     end

lib/puppet/pops/loader/ruby_function_instantiator.rb

@@ -20,7 +20,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
     # make the private loader available in a binding to allow it to be passed on
     loader_for_function = loader.private_loader
     here = get_binding(loader_for_function)
-    created = eval(ruby_code_string, here, source_ref, 1)
+    created = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval
     unless created.is_a?(Class)
       raise ArgumentError, _("The code loaded from %{source_ref} did not produce a Function class when evaluated. Got '%{klass}'") % { source_ref: source_ref, klass: created.class }
     end

lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb

@@ -38,7 +38,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
       # This will do the 3x loading and define the "function_<name>" and "real_function_<name>" methods
       # in the anonymous module used to hold function definitions.
       #
-      func_info = eval(ruby_code_string, here, source_ref, 1)
+      func_info = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval
 
       # Validate what was loaded
       unless func_info.is_a?(Hash)

lib/puppet/provider/package/appdmg.rb

@@ -67,7 +67,7 @@ def self.installpkgdmg(source, name)
         end
       end
 
-      open(cached_source) do |dmg|
+      File.open(cached_source) do |dmg|
         xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-mountrandom", "/tmp", dmg.path
           ptable = Puppet::Util::Plist::parse_plist(xml_str)
           # JJM Filter out all mount-paths into a single array, discard the rest.