Change Gitlab::Popen to use arrays for commands

Jacob Vosmaer
1 parent 9f20580e
Showing 2 changed files with 16 additions and 4 deletions Show diff stats
lib/gitlab/popen.rb
spec/lib/gitlab/popen_spec.rb
 require 'fileutils'
+require 'open3'
 module Gitlab
   module Popen
     def popen(cmd, path)
+      unless cmd.is_a?(Array)
+        raise "System commands must be given as an array of strings"
+      end
+
       vars = { "PWD" => path }
       options = { chdir: path }
@@ -12,10 +17,10 @@ module Gitlab
       @cmd_output = ""
       @cmd_status = 0
-      Open3.popen3(vars, cmd, options) do |stdin, stdout, stderr, wait_thr|
-        @cmd_status = wait_thr.value.exitstatus
+      Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
         @cmd_output << stdout.read
         @cmd_output << stderr.read
+        @cmd_status = wait_thr.value.exitstatus
       end
       return @cmd_output, @cmd_status
@@ -10,7 +10,7 @@ describe &#39;Gitlab::Popen&#39;, no_db: true do
   context 'zero status' do
     before do
-      @output, @status = @klass.new.popen('ls', path)
+      @output, @status = @klass.new.popen(%W(ls), path)
     end
     it { @status.should be_zero }
@@ -19,11 +19,18 @@ describe &#39;Gitlab::Popen&#39;, no_db: true do
   context 'non-zero status' do
     before do
-      @output, @status = @klass.new.popen('cat NOTHING', path)
+      @output, @status = @klass.new.popen(%W(cat NOTHING), path)
     end
     it { @status.should == 1 }
     it { @output.should include('No such file or directory') }
   end
+
+  context 'unsafe string command' do
+    it 'raises an error when it gets called with a string argument' do
+      expect { @klass.new.popen('ls', path) }.to raise_error
+    end
+  end
+
 end