(PUP-11772) Resolve Security/Eval

Both actions and functions/data types already define arbitrary code and are
loaded from trusted locations, so using eval isn't any worse.

I updated the ActionBuilder to delegate specific methods to the action. For
example, if an action calls the DSL method `summary "something"`, then the
ActionBuilder will call the corresponding setter on the Action, e.g.
Action#summary = "something".

The Action code is bit more complicated because the arity of the block passed to
`when_invoked=` may be 0, positive or negative, depending on whether it accepts
optional arguments. Since we don't support Ruby 1.8 - 2.6, it could be improved
in the future to not call `eval`, but I didn't feel like bothering.

(cherry picked from commit 1e4316b)

Author: joshcooper

.rubocop_todo.yml

@@ -927,14 +927,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'
-
 # Configuration parameters: EnforcedStyle, AllowModifiersOnSymbols.
 # SupportedStyles: inline, group
 Style/AccessModifierDeclarations:

lib/puppet/interface/action.rb

@@ -264,12 +264,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

@@ -4,6 +4,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
@@ -141,15 +143,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

@@ -19,7 +19,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

@@ -19,7 +19,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

@@ -37,7 +37,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)