diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb index 5283cf0b821..d10269f4438 100644 --- a/lib/gitlab/popen.rb +++ b/lib/gitlab/popen.rb @@ -1,8 +1,13 @@ 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 diff --git a/spec/lib/gitlab/popen_spec.rb b/spec/lib/gitlab/popen_spec.rb index 4791be41613..a4a0846b7b9 100644 --- a/spec/lib/gitlab/popen_spec.rb +++ b/spec/lib/gitlab/popen_spec.rb @@ -10,7 +10,7 @@ describe 'Gitlab::Popen', 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 'Gitlab::Popen', 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