Merge branch 'mk/avoid-credential-leak-qa' into 'master'

QA: Redact credentials from URI in git output

Closes gitlab-qa#258

See merge request gitlab-org/gitlab-ce!19492

qa/qa/git/repository.rb

@@ -7,7 +7,7 @@ module QA
     class Repository
       include Scenario::Actable
 
-      attr_reader :push_error
+      attr_reader :push_output
 
       def self.perform(*args)
         Dir.mktmpdir do |dir|
@@ -35,7 +35,7 @@ module QA
       end
 
       def clone(opts = '')
-        `git clone #{opts} #{@uri.to_s} ./ #{suppress_output}`
+        run_and_redact_credentials("git clone #{opts} #{@uri} ./")
       end
 
       def checkout(branch_name)
@@ -71,8 +71,7 @@ module QA
       end
 
       def push_changes(branch = 'master')
-        # capture3 returns stdout, stderr and status.
-        _, @push_error, _ = Open3.capture3("git push #{@uri} #{branch} #{suppress_output}")
+        @push_output, _ = run_and_redact_credentials("git push #{@uri} #{branch}")
       end
 
       def commits
@@ -81,12 +80,10 @@ module QA
 
       private
 
-      def suppress_output
-        # If we're running as the default user, it's probably a temporary
-        # instance and output can be useful for debugging
-        return if @username == Runtime::User.default_name
-
-        "&> #{File::NULL}"
+      # Since the remote URL contains the credentials, and git occasionally
+      # outputs the URL. Note that stderr is redirected to stdout.
+      def run_and_redact_credentials(command)
+        Open3.capture2("#{command} 2>&1 | sed -E 's#://[^@]+@#://****@#g'")
       end
     end
   end

qa/qa/specs/features/repository/protected_branches_spec.rb

@@ -60,9 +60,9 @@ module QA
           push_changes('protected-branch')
         end
 
-        expect(repository.push_error)
+        expect(repository.push_output)
           .to match(/remote\: GitLab\: You are not allowed to push code to protected branches on this project/)
-        expect(repository.push_error)
+        expect(repository.push_output)
           .to match(/\[remote rejected\] #{branch_name} -> #{branch_name} \(pre-receive hook declined\)/)
       end
     end

qa/spec/git/repository_spec.rb

+describe QA::Git::Repository do
+  let(:repository) { described_class.new }
+
+  before do
+    cd_empty_temp_directory
+    set_bad_uri
+    repository.use_default_credentials
+  end
+
+  describe '#clone' do
+    it 'redacts credentials from the URI in output' do
+      output, _ = repository.clone
+
+      expect(output).to include("fatal: unable to access 'http://****@foo/bar.git/'")
+    end
+  end
+
+  describe '#push_changes' do
+    before do
+      `git init` # need a repo to push from
+    end
+
+    it 'redacts credentials from the URI in output' do
+      output, _ = repository.push_changes
+
+      expect(output).to include("error: failed to push some refs to 'http://****@foo/bar.git'")
+    end
+  end
+
+  def cd_empty_temp_directory
+    tmp_dir = 'tmp/git-repository-spec/'
+    FileUtils.rm_r(tmp_dir) if File.exist?(tmp_dir)
+    FileUtils.mkdir_p tmp_dir
+    FileUtils.cd tmp_dir
+  end
+
+  def set_bad_uri
+    repository.uri = 'http://foo/bar.git'
+  end
+end