Prevent modification of attribute defaults

lib/artifactory/resources/base.rb

@@ -44,6 +44,16 @@ class << self
       #
       def attribute(key, default = nil)
         key = key.to_sym unless key.is_a?(Symbol)
+        case default
+        when NilClass, Integer, Float, Symbol, TrueClass, FalseClass, Proc
+          # Immutable primitive data types OK
+        when String
+          raise "Mutable String must not be used as a default attribute value. (Frozen OK)" unless default.frozen?
+        when Hash, Array
+          raise "Mutable or non-empty #{default.class} must not be used as a default attribute value. (Frozen empty #{default.class} OK)" unless default.frozen? && default.empty?
+        else
+          raise "Mutable type #{default.class} must not be used as a default attribute value"
+        end
 
         # Set this attribute in the top-level hash
         attributes[key] = nil

lib/artifactory/resources/build.rb

@@ -95,24 +95,24 @@ def from_hash(hash, options = {})
     end
 
     # Based on https://github.com/JFrogDev/build-info/blob/master/README.md#build-info-json-format
-    attribute :properties, {}
+    attribute :properties, {}.freeze
     attribute :version, BUILD_SCHEMA_VERSION
     attribute :name, -> { raise "Build component missing!" }
     attribute :number, -> { raise "Build number missing!" }
-    attribute :type, "GENERIC"
-    attribute :build_agent, {}
-    attribute :agent, {}
-    attribute :started, Time.now.utc.iso8601(3)
+    attribute :type, "GENERIC".freeze
+    attribute :build_agent, {}.freeze
+    attribute :agent, {}.freeze
+    attribute :started, Time.now.utc.iso8601(3).freeze
     attribute :duration_millis
     attribute :artifactory_principal
     attribute :url
     attribute :vcs_revision
     attribute :vcs_url
-    attribute :license_control, {}
-    attribute :build_retention, {}
-    attribute :modules, []
+    attribute :license_control, {}.freeze
+    attribute :build_retention, {}.freeze
+    attribute :modules, [].freeze
     attribute :governance
-    attribute :statuses, []
+    attribute :statuses, [].freeze
 
     #
     # Compare a build artifacts/dependencies/environment with an older

lib/artifactory/resources/ldap_setting.rb

@@ -119,6 +119,6 @@ def find_from_config(xpath, config, options = {})
     attribute :manager_dn
     attribute :manager_password
     attribute :auto_create_user
-    attribute :email_attribute, "mail"
+    attribute :email_attribute, "mail".freeze
   end
 end

lib/artifactory/resources/permission_target.rb

@@ -134,10 +134,10 @@ def abbreviate_principal(principal_hash)
     end
 
     attribute :name, -> { raise "Name missing!" }
-    attribute :includes_pattern, "**"
-    attribute :excludes_pattern, ""
+    attribute :includes_pattern, "**".freeze
+    attribute :excludes_pattern, "".freeze
     attribute :repositories
-    attribute :principals, { "users" => {}, "groups" => {} }
+    attribute :principals, -> { { "users" => {}, "groups" => {} } }
 
     def client_principal
       @client_principal ||= Principal.new(principals["users"], principals["groups"])

lib/artifactory/resources/repository.rb

@@ -68,26 +68,26 @@ def find(name, options = {})
 
     attribute :blacked_out, false
     attribute :description
-    attribute :checksum_policy_type, "client-checksums"
-    attribute :excludes_pattern, ""
+    attribute :checksum_policy_type, "client-checksums".freeze
+    attribute :excludes_pattern, "".freeze
     attribute :handle_releases, true
     attribute :handle_snapshots, true
-    attribute :includes_pattern, "**/*"
+    attribute :includes_pattern, "**/*".freeze
     attribute :key, -> { raise "Key is missing!" }
     attribute :max_unique_snapshots, 0
     attribute :notes
-    attribute :package_type, "generic"
-    attribute :property_sets, []
-    attribute :repo_layout_ref, "simple-default"
-    attribute :rclass, "local"
-    attribute :snapshot_version_behavior, "non-unique"
+    attribute :package_type, "generic".freeze
+    attribute :property_sets, [].freeze
+    attribute :repo_layout_ref, "simple-default".freeze
+    attribute :rclass, "local".freeze
+    attribute :snapshot_version_behavior, "non-unique".freeze
     attribute :suppress_pom_consistency_checks, false
-    attribute :url, ""
+    attribute :url, "".freeze
     attribute :yum_root_depth, 0
     attribute :calculate_yum_metadata, false
-    attribute :repositories, []
+    attribute :repositories, [].freeze
     attribute :external_dependencies_enabled, false
-    attribute :client_tls_certificate, ""
+    attribute :client_tls_certificate, "".freeze
 
     #
     # Creates or updates a repository configuration depending on if the

lib/artifactory/resources/user.rb

@@ -68,7 +68,7 @@ def find(name, options = {})
 
     attribute :admin, false
     attribute :email
-    attribute :groups, []
+    attribute :groups, [].freeze
     attribute :internal_password_disabled, false
     attribute :last_logged_in
     attribute :name, -> { raise "Name missing" }

spec/unit/resources/build_spec.rb

@@ -343,5 +343,43 @@ module Artifactory
         end
       end
     end
+    describe "attribute defaults" do
+      it "#properties modifying the default return value produces an error" do
+        expect { subject.properties["key"] = "value" }.to raise_error(FrozenError)
+      end
+
+      it "#type modifying the default return value produces an error" do
+        expect { subject.type.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#build_agent modifying the default return value produces an error" do
+        expect { subject.build_agent["key"] = "value" }.to raise_error(FrozenError)
+      end
+
+      it "#agent modifying the default return value produces an error" do
+        expect { subject.agent["key"] = "value" }.to raise_error(FrozenError)
+      end
+
+      it "#started modifying the default return value produces an error" do
+        expect { subject.started.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#license_control modifying the default return value produces an error" do
+        expect { subject.license_control["key"] = "value" }.to raise_error(FrozenError)
+      end
+
+      it "#build_retention modifying the default return value produces an error" do
+        expect { subject.build_retention["key"] = "value" }.to raise_error(FrozenError)
+      end
+
+      it "#modules modifying the default return value produces an error" do
+        expect { subject.modules << "value" }.to raise_error(FrozenError)
+      end
+
+      it "#statuses modifying the default return value produces an error" do
+        expect { subject.statuses << "value" }.to raise_error(FrozenError)
+      end
+
+    end
   end
 end

spec/unit/resources/ldap_setting_spec.rb

@@ -61,5 +61,10 @@ module Artifactory
         expect(described_class.find("viridian-ldap").key).to eq("viridian-ldap")
       end
     end
+    describe "attribute defaults" do
+      it "#email_attribute modifying the default return value produces an error" do
+        expect { subject.email_attribute.downcase! }.to raise_error(FrozenError)
+      end
+    end
   end
 end

spec/unit/resources/permission_target_spec.rb

@@ -185,5 +185,24 @@ module Artifactory
         expect(subject.groups).to eq({ "beatles" => %w{delete deploy} })
       end
     end
+    describe "attribute defaults" do
+      it "#includes_pattern modifying the default return value produces an error" do
+        expect { subject.includes_pattern.gsub!(/^\*\*$/, "path/**") }.to raise_error(FrozenError)
+      end
+
+      it "#excludes_pattern modifying the default return value produces an error" do
+        expect { subject.excludes_pattern.replace("path/**") }.to raise_error(FrozenError)
+      end
+
+      it "#users modifying the return value doesn't modify the default" do
+        subject.users["a_user"] = ["read"]
+        expect(described_class.new.users).to eq({})
+      end
+
+      it "#groups modifying the return value doesn't modify the default" do
+        subject.groups["a_group"] = ["read"]
+        expect(described_class.new.groups).to eq({})
+      end
+    end
   end
 end

spec/unit/resources/repository_spec.rb

@@ -215,5 +215,51 @@ module Artifactory
         subject.upload_from_archive("/local/path", "/remote/path", { properties: :foobar })
       end
     end
+
+    describe "attribute defaults" do
+      it "#checksum_policy_type modifying the default return value produces an error" do
+        expect { subject.checksum_policy_type.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#excludes_pattern modifying the default return value produces an error" do
+        expect { subject.excludes_pattern.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#includes_pattern modifying the default return value produces an error" do
+        expect { subject.includes_pattern.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#package_type modifying the default return value produces an error" do
+        expect { subject.package_type.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#property_sets modifying the default return value produces an error" do
+        expect { subject.property_sets << "value" }.to raise_error(FrozenError)
+      end
+
+      it "#repo_layout_ref modifying the default return value produces an error" do
+        expect { subject.repo_layout_ref.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#rclass modifying the default return value produces an error" do
+        expect { subject.rclass.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#snapshot_version_behavior modifying the default return value produces an error" do
+        expect { subject.snapshot_version_behavior.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#url modifying the default return value produces an error" do
+        expect { subject.url.downcase! }.to raise_error(FrozenError)
+      end
+
+      it "#repositories modifying the default return value produces an error" do
+        expect { subject.repositories << "value" }.to raise_error(FrozenError)
+      end
+
+      it "#client_tls_certificate modifying the default return value produces an error" do
+        expect { subject.client_tls_certificate.downcase! }.to raise_error(FrozenError)
+      end
+    end
   end
 end

spec/unit/resources/user_spec.rb

@@ -111,5 +111,11 @@ module Artifactory
         end
       end
     end
+
+    describe "attribute defaults" do
+      it "#groups modifying the default return value produces an error" do
+        expect { subject.groups << "value" }.to raise_error(FrozenError)
+      end
+    end
   end
 end