diff --git a/.env.example b/.env.example index 10b8a3105f..e264559aac 100644 --- a/.env.example +++ b/.env.example @@ -45,6 +45,8 @@ GITHUB_TOKEN={fill me in} # FORCE_SSL={optional, set to 1 to require SSL} +# ENV_WHITELIST={optional, comma separated list of env values that should be used when deploying } + ## JIRA_BASE_URL, if set, would enable the auto-detection of JIRA issue keys ## (e.g., KEY-123, SAMSON-456) in the titles and bodies of the pull requests ## associated with a deploy. The auto-detected JIRA issues will be displayed diff --git a/app/models/git_repository.rb b/app/models/git_repository.rb index 1c5e2fb81c..f93bfac643 100644 --- a/app/models/git_repository.rb +++ b/app/models/git_repository.rb @@ -102,8 +102,8 @@ def valid_url? def downstream_commit?(old_commit, new_commit) return true if old_commit == new_commit cmd = "git merge-base --is-ancestor #{old_commit} #{new_commit}" - status, output = run_single_command(cmd) { |line| line.strip } - $?.exitstatus == 1 + status, output = run_single_command(cmd) { |l| l } + !status && !output[0].to_s.include?("fatal") end def executor diff --git a/app/models/terminal_executor.rb b/app/models/terminal_executor.rb index 60f80f4970..ff062ca9b6 100644 --- a/app/models/terminal_executor.rb +++ b/app/models/terminal_executor.rb @@ -1,5 +1,4 @@ -require 'pty' -require 'shellwords' +require 'open3' # Executes commands in a fake terminal. The output will be streamed to a # specified IO-like object. @@ -34,26 +33,30 @@ def stop!(signal) private def execute_command!(command) - output, input, @pid = Bundler.with_clean_env do - PTY.spawn(command, in: '/dev/null') - end + Open3.popen2e(whitelisted_env, command, in: '/dev/null', unsetenv_others: true, pgroup: true) do |stdin, oe, wait_thr| + @pid = wait_thr.pid - @pgid = begin - Process.getpgid(pid) - rescue Errno::ESRCH - nil - end + @pgid = begin + Process.getpgid(@pid) + rescue Errno::ESRCH + nil + end - begin - output.each(256) {|line| @output.write(line) } - rescue Errno::EIO - # The IO has been closed. - end + oe.each(256) do |line| + @output.write line.gsub("\n", "\r\n") + end - _, status = Process.wait2(pid) - - input.close + wait_thr.value + end.success? + end - return status.success? + def whitelisted_env + whitelist = [ + 'PATH', 'HOME', 'TMPDIR', 'CACHE_DIR', 'TERM', 'SHELL', # general + 'RBENV_ROOT', 'RBENV_HOOK_PATH', 'RBENV_DIR', # ruby + 'DOCKER_URL', 'DOCKER_REGISTRY' # docker + ] + ENV['ENV_WHITELIST'].to_s.split(/, ?/) + ENV.to_h.slice(*whitelist) end end + diff --git a/test/models/git_repository_test.rb b/test/models/git_repository_test.rb index a07e80b0bc..e9a0e2f429 100644 --- a/test/models/git_repository_test.rb +++ b/test/models/git_repository_test.rb @@ -218,5 +218,4 @@ repository.clean! end end - end diff --git a/test/models/terminal_executor_test.rb b/test/models/terminal_executor_test.rb index e051833e3c..be7c5313c1 100644 --- a/test/models/terminal_executor_test.rb +++ b/test/models/terminal_executor_test.rb @@ -1,6 +1,18 @@ require_relative '../test_helper' describe TerminalExecutor do + def with_env(env) + old = env.map do |k, v| + k = k.to_s + o = ENV[k] + ENV[k] = v + [k, o] + end + yield + ensure + old.each { |k, v| ENV[k] = v } + end + let(:output) { StringIO.new } subject { TerminalExecutor.new(output) } @@ -28,6 +40,28 @@ subject.execute!('echo "hi"').must_equal(true) end + it 'does not expose env secrets' do + with_env MY_SECRET: 'XYZ' do + subject.execute!('env') + output.string.wont_include("SECRET") + end + end + + it "keeps CACHE_DIR" do + with_env CACHE_DIR: 'XYZ' do + subject.execute!('env') + output.string.must_include("CACHE_DIR=XYZ") + end + end + + it "keeps custom env vars from ENV_WHITELIST" do + with_env ENV_WHITELIST: 'ABC, XYZ,ZZZ', XYZ: 'FOO', ZZZ: 'FOO' do + subject.execute!('env') + output.string.must_include("XYZ=FOO") + output.string.must_include("ZZZ=FOO") + end + end + describe 'in verbose mode' do subject { TerminalExecutor.new(output, verbose: true) }