X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/4d3a1c5346c99da360af58f1932b8f7a2ba87723..7c04fe3e0e93d22b9c884934dbbd80fed6072ee3:/services/api/app/models/commit.rb

diff --git a/services/api/app/models/commit.rb b/services/api/app/models/commit.rb
index 7979ff4408..49541248f2 100644
--- a/services/api/app/models/commit.rb
+++ b/services/api/app/models/commit.rb
@@ -1,55 +1,26 @@
 class Commit < ActiveRecord::Base
   require 'shellwords'
 
-  # Make sure the specified commit really exists, and return the full
-  # sha1 commit hash.
-  #
-  # Accepts anything "git rev-list" accepts, optionally (and
-  # preferably) preceded by "repo_name:".
-  #
-  # Examples: "1234567", "master", "apps:1234567", "apps:master",
-  # "apps:HEAD"
-
-  # def self.find_by_commit_ish(commit_ish)
-  #   if only_valid_chars.match(commit_ish)       
-  #     logger.warn "find_by_commit_ish called with string containing invalid characters: '#{commit_ish}'"
-  #     return nil
-  #   end
-
-  #   want_repo = nil
-  #   if commit_ish.index(':')
-  #     want_repo, commit_ish = commit_ish.split(':',2)
-  #   end
-  #   repositories.each do |repo_name, repo|
-  #     next if want_repo and want_repo != repo_name
-  #     ENV['GIT_DIR'] = repo[:git_dir]
-  #     # we're passing user input to a command line, this is a potential a security hole but I am reasonably confident that shellescape sanitizes the input adequately
-  #     IO.foreach("|git rev-list --max-count=1 --format=oneline 'origin/'#{commit_ish.shellescape} 2>/dev/null || git rev-list --max-count=1 --format=oneline ''#{commit_ish.shellescape}") do |line|
-  #       sha1, message = line.strip.split " ", 2
-  #       next if sha1.length != 40
-  #       begin
-  #         Commit.find_or_create_by_repository_name_and_sha1_and_message(repo_name, sha1, message[0..254])
-  #       rescue
-  #         logger.warn "find_or_create failed: repo_name #{repo_name} sha1 #{sha1} message #{message[0..254]}"
-  #         # Ignore cache failure. Commit is real. We should proceed.
-  #       end
-  #       return sha1
-  #     end
-  #   end
-  #   nil
-  # end
+  def self.git_check_ref_format(e)
+    if !e or e.empty? or e[0] == '-' or e[0] == '$'
+      # definitely not valid
+      false
+    else
+      `git check-ref-format --allow-onelevel #{e.shellescape}`
+      $?.success?
+    end
+  end
 
   def self.find_commit_range(current_user, repository, minimum, maximum, exclude)
-    only_valid_chars = /[^A-Za-z0-9_-]/
-    if only_valid_chars.match(minimum) || only_valid_chars.match(maximum) 
-      logger.warn "find_commit_range called with string containing invalid characters: '#{minimum}', '#{maximum}'"
+    if (minimum and !git_check_ref_format(minimum)) or !git_check_ref_format(maximum)
+      logger.warn "find_commit_range called with invalid minimum or maximum: '#{minimum}', '#{maximum}'"
       return nil
     end
 
     if minimum and minimum.empty?
         minimum = nil
     end
-    
+
     if !maximum
       maximum = "HEAD"
     end
@@ -65,56 +36,52 @@ class Commit < ActiveRecord::Base
       readable = readable.where(name: repository)
     end
 
-    #puts "min #{minimum}"
-    #puts "max #{maximum}"
-    #puts "rep #{repository}"
-
     commits = []
     readable.each do |r|
       if on_disk_repos[r.name]
         ENV['GIT_DIR'] = on_disk_repos[r.name][:git_dir]
 
-        #puts "dir #{on_disk_repos[r.name][:git_dir]}"
-
         # We've filtered for invalid characters, so we can pass the contents of
         # minimum and maximum safely on the command line
 
-        #puts "git rev-list --max-count=1 #{maximum}"
-
         # Get the commit hash for the upper bound
         max_hash = nil
-        IO.foreach("|git rev-list --max-count=1 #{maximum}") do |line|
+        IO.foreach("|git rev-list --max-count=1 #{maximum.shellescape}") do |line|
           max_hash = line.strip
         end
 
-        # If not found, nothing else to do
-        next if !max_hash
+        # If not found or string is invalid, nothing else to do
+        next if !max_hash or !git_check_ref_format(max_hash)
 
         resolved_exclude = nil
         if exclude
           resolved_exclude = []
           exclude.each do |e|
-            IO.foreach("|git rev-list --max-count=1 #{e}") do |line|
-              resolved_exclude.push(line.strip)
-            end  
+            if git_check_ref_format(e)
+              IO.foreach("|git rev-list --max-count=1 #{e.shellescape}") do |line|
+                resolved_exclude.push(line.strip)
+              end
+            else
+              logger.warn "find_commit_range called with invalid exclude invalid characters: '#{exclude}'"
+              return nil
+            end
           end
         end
 
-        if minimum          
+        if minimum
           # Get the commit hash for the lower bound
           min_hash = nil
-          IO.foreach("|git rev-list --max-count=1 #{minimum}") do |line|
+          IO.foreach("|git rev-list --max-count=1 #{minimum.shellescape}") do |line|
             min_hash = line.strip
           end
 
-          # If not found, nothing else to do
-          next if !min_hash
-          
+          # If not found or string is invalid, nothing else to do
+          next if !min_hash or !git_check_ref_format(min_hash)
+
           # Now find all commits between them
-          #puts "git rev-list #{min_hash}..#{max_hash}"
-          IO.foreach("|git rev-list #{min_hash}..#{max_hash}") do |line|
+          IO.foreach("|git rev-list #{min_hash.shellescape}..#{max_hash.shellescape}") do |line|
             hash = line.strip
-            commits.push(hash) if !resolved_exclude or !resolved_exclude.include? hash              
+            commits.push(hash) if !resolved_exclude or !resolved_exclude.include? hash
           end
 
           commits.push(min_hash) if !resolved_exclude or !resolved_exclude.include? min_hash