update immutable API blacklist and whitelist

adds the whitelist so that when we update to new ruby versions it forces
us to audit the new Array and Hash methods and figure out if they are
mutators or not.

this change closes the Hash#store loophole that was being abused by
some cookbook consumers to directly modify merged ImmutableHashes
(producing inherently undefined behavior that we 'broke' at one
point).

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 220 insertions, 8 deletions

diff --git a/lib/chef/node/mixin/immutablize_array.rb b/lib/chef/node/mixin/immutablize_array.rb
index bd330cf8a9..72273d5333 100644
--- a/lib/chef/node/mixin/immutablize_array.rb
+++ b/lib/chef/node/mixin/immutablize_array.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,15 +19,118 @@ class Chef
   class Node
     module Mixin
       module ImmutablizeArray
+        # Allowed methods that MUST NOT mutate the object
+        # (if any of these methods mutate the underlying object that is a bug that needs to be fixed)
+        ALLOWED_METHODS = [
+          :&,
+          :*,
+          :+,
+          :-,
+          :[],
+          :all?,
+          :any?,
+          :assoc,
+          :at,
+          :bsearch,
+          :bsearch_index,
+          :chunk,
+          :chunk_while,
+          :collect,
+          :collect_concat,
+          :combination,
+          :compact,
+          :count,
+          :cycle,
+          :detect,
+          :dig,
+          :drop,
+          :drop_while,
+          :each,
+          :each_cons,
+          :each_entry,
+          :each_index,
+          :each_slice,
+          :each_with_index,
+          :each_with_object,
+          :empty?,
+          :entries,
+          :fetch,
+          :find,
+          :find_all,
+          :find_index,
+          :first,
+          :flat_map,
+          :flatten,
+          :grep,
+          :grep_v,
+          :group_by,
+          :include?,
+          :index,
+          :inject,
+          :join,
+          :last,
+          :lazy,
+          :length,
+          :map,
+          :max,
+          :max_by,
+          :member?,
+          :min,
+          :min_by,
+          :minmax,
+          :minmax_by,
+          :none?,
+          :one?,
+          :pack,
+          :partition,
+          :permutation,
+          :product,
+          :rassoc,
+          :reduce,
+          :reject,
+          :repeated_combination,
+          :repeated_permutation,
+          :reverse,
+          :reverse_each,
+          :rindex,
+          :rotate,
+          :sample,
+          :save_plist,
+          :select,
+          :shelljoin,
+          :shuffle,
+          :size,
+          :slice,
+          :slice_after,
+          :slice_before,
+          :slice_when,
+          :sort,
+          :sort_by,
+          :sum,
+          :take,
+          :take_while,
+          :to_a,
+          :to_ary,
+          :to_h,
+          :to_plist,
+          :to_set,
+          :transpose,
+          :uniq,
+          :values_at,
+          :zip,
+          :|,
+        ]
         # A list of methods that mutate Array. Each of these is overridden to
         # raise an error, making this instances of this class more or less
         # immutable.
         DISALLOWED_MUTATOR_METHODS = [
           :<<,
           :[]=,
+          :append,
           :clear,
           :collect!,
           :compact!,
+          :concat,
           :default=,
           :default_proc=,
           :delete,
@@ -40,18 +143,21 @@ class Chef
           :map!,
           :merge!,
           :pop,
+          :prepend,
           :push,
-          :update,
           :reject!,
-          :reverse!,
           :replace,
+          :reverse!,
+          :rotate!,
           :select!,
           :shift,
+          :shuffle!,
           :slice!,
           :sort!,
           :sort_by!,
           :uniq!,
           :unshift,
+          :update,
         ]
 
         # Redefine all of the methods that mutate a Hash to raise an error when called.
diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb
index f6b22ed7d7..cd2bbf7bf6 100644
--- a/lib/chef/node/mixin/immutablize_hash.rb
+++ b/lib/chef/node/mixin/immutablize_hash.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,10 +19,112 @@ class Chef
   class Node
     module Mixin
       module ImmutablizeHash
+        # allowed methods that MUST NOT mutate the object
+        # (if any of these methods mutate the underlying object that is a bug that needs to be fixed)
+        ALLOWED_METHODS = [
+          :<,
+          :<=,
+          :>,
+          :>=,
+          :[],
+          :all?,
+          :any?,
+          :assoc,
+          :chunk,
+          :chunk_while,
+          :collect,
+          :collect_concat,
+          :compact,
+          :compare_by_identity,
+          :compare_by_identity?,
+          :count,
+          :cycle,
+          :default,
+          :default_proc,
+          :detect,
+          :dig,
+          :drop,
+          :drop_while,
+          :each,
+          :each_cons,
+          :each_entry,
+          :each_key,
+          :each_pair,
+          :each_slice,
+          :each_value,
+          :each_with_index,
+          :each_with_object,
+          :empty?,
+          :entries,
+          :fetch,
+          :fetch_values,
+          :find,
+          :find_all,
+          :find_index,
+          :first,
+          :flat_map,
+          :flatten,
+          :grep,
+          :grep_v,
+          :group_by,
+          :has_key?,
+          :has_value?,
+          :include?,
+          :index,
+          :inject,
+          :invert,
+          :key,
+          :key?,
+          :keys,
+          :lazy,
+          :length,
+          :map,
+          :max,
+          :max_by,
+          :member?,
+          :merge,
+          :min,
+          :min_by,
+          :minmax,
+          :minmax_by,
+          :none?,
+          :one?,
+          :partition,
+          :rassoc,
+          :reduce,
+          :reject,
+          :reverse_each,
+          :save_plist,
+          :select,
+          :size,
+          :slice,
+          :slice_after,
+          :slice_before,
+          :slice_when,
+          :sort,
+          :sort_by,
+          :sum,
+          :take,
+          :take_while,
+          :to_a,
+          :to_h,
+          :to_hash,
+          :to_plist,
+          :to_proc,
+          :to_set,
+          :transform_keys,
+          :transform_values,
+          :uniq,
+          :value?,
+          :values,
+          :values_at,
+          :zip,
+        ]
         DISALLOWED_MUTATOR_METHODS = [
           :[]=,
           :clear,
           :collect!,
+          :compact!,
           :default=,
           :default_proc=,
           :delete,
@@ -30,15 +132,19 @@ class Chef
           :keep_if,
           :map!,
           :merge!,
-          :update,
+          :rehash,
           :reject!,
           :replace,
           :select!,
           :shift,
-          :write,
-          :write!,
-          :unlink,
+          :store,
+          :transform_keys!,
+          :transform_values!,
           :unlink!,
+          :unlink,
+          :update,
+          :write!,
+          :write,
         ]
 
         # Redefine all of the methods that mutate a Hash to raise an error when called.