[ARVADOS] created: 780b29bc2d8cc7cc990303a62c23e8a629170f67

[git at public.curoverse.com]

        at  780b29bc2d8cc7cc990303a62c23e8a629170f67 (commit)


commit 780b29bc2d8cc7cc990303a62c23e8a629170f67
Author: Tim Pierce <twp at curoverse.com>
Date:   Thu Dec 4 17:12:57 2014 -0500

    4269: added job validation forbidding collection uuids
    
    Added Job validation no_collection_uuids, searching recursively through
    script_parameters for any field matching a collection uuid pattern.

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 4a464f5..bc64cf5 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -15,6 +15,7 @@ class Job < ArvadosModel
   validate :find_docker_image_locator
   validate :validate_status
   validate :validate_state_change
+  validate :no_collection_uuids
   before_save :update_timestamps_when_state_changes
 
   has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
@@ -374,4 +375,34 @@ class Job < ArvadosModel
     end
     ok
   end
+
+  def no_collection_uuids
+    # recursive_hash_search searches recursively through hashes and
+    # arrays in 'thing' for string fields matching regular expression
+    # 'pattern'.  Returns true if pattern is found, false otherwise.
+    def recursive_hash_search thing, pattern
+      if thing.is_a? Hash
+        thing.each do |k, v|
+          return true if recursive_hash_search v, pattern
+        end
+      elsif thing.is_a? Array
+        thing.each do |k|
+          return true if recursive_hash_search k, pattern
+        end
+      elsif thing.is_a? String
+        return true if thing.match pattern
+      end
+      false
+    end
+
+    # Fail validation if any script_parameters field includes a string containing a
+    # collection uuid pattern.
+    if self.script_parameters_changed?
+      if recursive_hash_search(self.script_parameters, /[a-z0-9]{5}-4zz18-[a-z0-9]{15}/)
+        self.errors.add :script_parameters, "must use portable_data_hash instead of collection uuid"
+        return false
+      end
+    end
+    true
+  end
 end
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index e885b69..24bc260 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -371,4 +371,33 @@ class JobTest < ActiveSupport::TestCase
   test "can't modify job to assign SDK version directly" do
     check_modification_prohibited(arvados_sdk_version: SDK_MASTER)
   end
+
+  test "job validation fails when collection uuid found in script_parameters" do
+    bad_params = {
+      script_parameters: {
+        'input' => {
+          'param1' => 'the collection uuid zzzzz-4zz18-012345678901234'
+        }
+      }
+    }
+    assert_raises(ActiveRecord::RecordInvalid,
+                  "created job with a collection uuid in script_parameters") do
+      job = Job.create!(job_attrs(bad_params))
+    end
+  end
+
+  test "job validation succeeds when no collection uuid in script_parameters" do
+    good_params = {
+      script_parameters: {
+        'arg1' => 'foo',
+        'arg2' => [ 'bar', 'baz' ],
+        'arg3' => {
+          'a' => 1,
+          'b' => [2, 3, 4],
+        }
+      }
+    }
+    job = Job.create!(job_attrs(good_params))
+    assert job.valid?
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
--