{"record":{"id":"b71ee92fd4caf2b9","repo":"puppetlabs/puppet","slug":"illegal-method-definition-of-method-method-name","errorCode":null,"errorMessage":"Illegal method definition of method '%{method_name}' in source %{source_ref} on line %{line} in legacy function. See %{url} for more information","messagePattern":"Illegal method definition of method '%(.+?)' in source %(.+?) on line %(.+?) in legacy function\\. See %(.+?) for more information","errorType":"exception","errorClass":"SecurityError","httpStatus":null,"severity":"error","filePath":"lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb","lineNumber":98,"sourceCode":"    ripped.each { |x| walk(x, source_ref, result) }\n    true\n  end\n  private_class_method :assert_code\n\n  def self.walk(x, source_ref, result)\n    return unless x.is_a?(Array)\n\n    first = x[0]\n    case first\n    when :fcall, :call\n      # Ripper returns a :fcall for a function call in a module (want to know there is a call to newfunction()).\n      # And it returns :call for a qualified named call\n      identity_part = find_identity(x)\n      result << :found_newfunction if identity_part.is_a?(Array) && identity_part[1] == 'newfunction'\n    when :def, :defs\n      # There should not be any calls to def in a 3x function\n      mname, mline = extract_name_line(find_identity(x))\n      raise SecurityError, _(\"Illegal method definition of method '%{method_name}' in source %{source_ref} on line %{line} in legacy function. See %{url} for more information\") % {\n        method_name: mname,\n        source_ref: source_ref,\n        line: mline,\n        url: \"https://puppet.com/docs/puppet/latest/functions_refactor_legacy.html\"\n      }\n    end\n    x.each { |v| walk(v, source_ref, result) }\n  end\n  private_class_method :walk\n\n  def self.find_identity(rast)\n    rast.find { |x| x.is_a?(Array) && x[0] == :@ident }\n  end\n  private_class_method :find_identity\n\n  # Extracts the method name and line number from the Ripper Rast for an id entry.\n  # The expected input (a result from Ripper :@ident entry) is an array with:\n  # [0] == :def (or :defs for self.def)","sourceCodeStart":80,"sourceCodeEnd":116,"githubUrl":"https://github.com/puppetlabs/puppet/blob/e227c27540975c25aa22d533a52424a9d2fc886a/lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb#L80-L116","documentation":"Raised as a SecurityError by RubyLegacyFunctionInstantiator when Puppet parses the Ruby source of a legacy 3.x function (a file under lib/puppet/parser/functions/ that calls newfunction) and finds a `def` or `defs` node. The instantiator walks the Ripper AST of the function body; method definitions are forbidden because legacy function bodies are eval'ed into a shared scope and a def would leak methods into the Puppet::Parser::Functions namespace or the compiler process. The only permitted Ruby 'call' shape is the newfunction() invocation itself (:fcall/:call).","triggerScenarios":"Loading a 3.x function whose block contains `def helper(...)` or `def self.helper(...)` — e.g. lib/puppet/parser/functions/myfunc.rb with `newfunction(:myfunc) do |args| def split_it(x) ... end ... end`. The walk() finds the :def node, extracts method name and line via find_identity/extract_name_line, and raises before the function is ever usable.","commonSituations":"Copying a 3.x function from an old module or a Puppet 3 cookbook into a modern module; refactoring a 4.x function back to the legacy API; vendors shipping functions with helper methods written as defs; upgrading Puppet versions where this security check got stricter.","solutions":["Refactor the function to the modern 4.x API: Puppet::Functions.create_function(:mymodule::myfunc) — method definitions on the function class are legal there.","If staying on 3.x, replace the `def` with a lambda assigned to a local variable (helper = ->(x) { ... }) inside the newfunction block.","Move the helper methods into a separate Ruby class/module under lib/puppet_x/<org>/ and require + call it from the function body instead of defining it inline.","Remove dead code: sometimes the def is a leftover from a copy-paste and is never called — delete it."],"exampleFix":"// before (lib/puppet/parser/functions/myfunc.rb)\nnewfunction(:myfunc) do |args|\n  def split_it(str)\n    str.split(',')\n  end\n  split_it(args[0])\nend\n\n// after (lib/puppet/functions/mymodule/myfunc.rb)\nPuppet::Functions.create_function(:'mymodule::myfunc') do\n  dispatch :myfunc do\n    required_param 'String', :str\n    return_type 'Array'\n  end\n  def myfunc(str)\n    str.split(',')\n  end\nend","handlingStrategy":"validation","validationCode":"# Scan legacy 3.x function sources for method definitions before Puppet loads them\nrequire 'ripper'\n\ndef contains_method_def?(ruby_source)\n  ast = Ripper.sexp(ruby_source)\n  return false unless ast\n  walk = lambda do |node|\n    case node\n    when Array\n      return true if node[0].is_a?(Symbol) && %i[def defs].include?(node[0])\n      node.any? { |child| walk.call(child) }\n    else\n      false\n    end\n  end\n  walk.call(ast)\nend\n\nDir['lib/puppet/parser/functions/*.rb'].each do |f|\n  abort \"#{f}: illegal def in legacy function\" if contains_method_def?(File.read(f))\nend","typeGuard":null,"tryCatchPattern":null,"preventionTips":["Prefer the 4.x function API (Puppet::Functions.create_function) for all new Ruby functions — method definitions are legal there.","In legacy functions, express helpers as lambdas (helper = ->(x) { ... }) instead of def.","Keep shared helpers in lib/puppet_x/<org>/<util>.rb and require them from function files.","Run a CI grep for /^\\s*def\\s/ under lib/puppet/parser/functions/ to catch regressions."],"tags":["puppet","legacy-functions","ruby","ripper","securityerror","static-analysis"],"backgroundTag":"puppet-legacy-function-illegal-method","analyzedSha":"e227c27540975c25aa22d533a52424a9d2fc886a","analyzedAt":"2026-08-21T20:49:46.650Z","schemaVersion":2},"datasetVersion":"2026-08-21T23:17:16.201Z"}