Chef-13 freeze merged node attributelcg/node-freeze

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

5 files changed, 29 insertions, 4 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 676b658068..0219369b7b 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -82,3 +82,10 @@ n["foo"] << "buzz"
 before this would have mutated the original string in-place so that `node["foo"]` and `node.default["foo"]` would have changed to "fizzbuzz"
 while now they remain "fizz" and only the mutable `n["foo"]` copy is changed to "fizzbuzz".
 
+### Freezing immutable merged attributes
+
+Since Chef 11 merged node attributes have been intended to be immutable but the merged strings have not been frozen.  In Chef 13, in the
+process of merging the node attributes strings and other simple objects are dup'd and frozen.  In order to get a mutable copy, you can
+now correctly use the `node.dup` or `node.to_hash` methods, or you should mutate the object correctly through its precedence level like
+`node.default["some_string"] << "appending_this"`.
+
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index 13a8aefe97..be9285a755 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -22,8 +22,14 @@ require "chef/node/mixin/immutablize_hash"
 
 class Chef
   class Node
-
     module Immutablize
+      # For elements like Fixnums, true, nil...
+      def safe_dup(e)
+        e.dup
+      rescue TypeError
+        e
+      end
+
       def immutablize(value)
         case value
         when Hash
@@ -31,7 +37,7 @@ class Chef
         when Array
           ImmutableArray.new(value, __root__, __node__, __precedence__)
         else
-          value
+          safe_dup(value).freeze
         end
       end
     end
diff --git a/lib/chef/node/mixin/immutablize_array.rb b/lib/chef/node/mixin/immutablize_array.rb
index cfa7266b9a..bd330cf8a9 100644
--- a/lib/chef/node/mixin/immutablize_array.rb
+++ b/lib/chef/node/mixin/immutablize_array.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb
index f09e6944fc..f6b22ed7d7 100644
--- a/lib/chef/node/mixin/immutablize_hash.rb
+++ b/lib/chef/node/mixin/immutablize_hash.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index 0869b83e43..3dd0d0f650 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1244,4 +1244,16 @@ describe Chef::Node::Attribute do
       @attributes.default["foo"]["bar"]["baz"] = "quux"
     end
   end
+
+  describe "frozen immutable strings" do
+    it "strings in hashes should be frozen" do
+      @attributes.default["foo"]["bar"]["baz"] = "fizz"
+      expect { @attributes["foo"]["bar"]["baz"] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String")
+    end
+
+    it "strings in arrays should be frozen" do
+      @attributes.default["foo"]["bar"] = [ "fizz" ]
+      expect { @attributes["foo"]["bar"][0] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String")
+    end
+  end
 end