Merge pull request #6291 from xu-cheng/safe-curl

Redact token in the log

Library/Homebrew/exceptions.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "shellwords"
+require "utils"
 
 class UsageError < RuntimeError
   attr_reader :reason
@@ -520,7 +521,7 @@ class ErrorDuringExecution < RuntimeError
   attr_reader :status
   attr_reader :output
 
-  def initialize(cmd, status:, output: nil)
+  def initialize(cmd, status:, output: nil, secrets: [])
     @cmd = cmd
     @status = status
     @output = output
@@ -531,7 +532,8 @@ class ErrorDuringExecution < RuntimeError
       status
     end
 
-    s = +"Failure while executing; `#{cmd.shelljoin.gsub(/\\=/, "=")}` exited with #{exitstatus}."
+    redacted_cmd = redact_secrets(cmd.shelljoin.gsub('\=', "="), secrets)
+    s = +"Failure while executing; `#{redacted_cmd}` exited with #{exitstatus}."
 
     unless [*output].empty?
       format_output_line = lambda do |type_line|

Library/Homebrew/extend/ENV.rb

@@ -29,12 +29,16 @@ module EnvActivation
     replace(old_env)
   end
 
-  def clear_sensitive_environment!
+  def sensitive?(key)
-    each_key do |key|
+    /(cookie|key|token|password)/i =~ key
-      next unless /(cookie|key|token|password)/i =~ key
-
-      delete key
   end
+
+  def sensitive_environment
+    select { |key, _| sensitive?(key) }
+  end
+
+  def clear_sensitive_environment!
+    each_key { |key| delete key if sensitive?(key) }
   end
 end
 

Library/Homebrew/system_command.rb

@@ -34,7 +34,7 @@ class SystemCommand
   end
 
   def run!
-    puts command.shelljoin.gsub(/\\=/, "=") if verbose? || ARGV.debug?
+    puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? || ARGV.debug?
 
     @output = []
 
@@ -54,8 +54,9 @@ class SystemCommand
   end
 
   def initialize(executable, args: [], sudo: false, env: {}, input: [], must_succeed: false,
-                 print_stdout: false, print_stderr: true, verbose: false, **options)
+                 print_stdout: false, print_stderr: true, verbose: false, secrets: [], **options)
 
+    require "extend/ENV"
     @executable = executable
     @args = args
     @sudo = sudo
@@ -63,6 +64,7 @@ class SystemCommand
     @print_stdout = print_stdout
     @print_stderr = print_stderr
     @verbose = verbose
+    @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq
     @must_succeed = must_succeed
     options.assert_valid_keys!(:chdir)
     @options = options
@@ -106,9 +108,7 @@ class SystemCommand
   def assert_success
     return if @status.success?
 
-    raise ErrorDuringExecution.new(command,
+    raise ErrorDuringExecution.new(command, status: @status, output: @output, secrets: @secrets)
-                                   status: @status,
-                                   output: @output)
   end
 
   def expanded_args

Library/Homebrew/system_config.rb

@@ -4,6 +4,7 @@ require "hardware"
 require "software_spec"
 require "rexml/document"
 require "development_tools"
+require "extend/ENV"
 
 class SystemConfig
   class << self
@@ -173,7 +174,7 @@ class SystemConfig
           next if boring_keys.include?(key)
           next if defaults_hash[key.to_sym]
 
-          value = "set" if key =~ /(cookie|key|token|password)/i
+          value = "set" if ENV.sensitive?(key)
           f.puts "#{key}: #{value}"
         end
       end

Library/Homebrew/test/ENV_spec.rb

@@ -143,6 +143,13 @@ shared_examples EnvActivation do
     expect(subject["MAKEFLAGS"]).to eq("-j4")
   end
 
+  describe "#sensitive_environment" do
+    it "list sensitive environment" do
+      subject["SECRET_TOKEN"] = "password"
+      expect(subject.sensitive_environment).to include("SECRET_TOKEN")
+    end
+  end
+
   describe "#clear_sensitive_environment!" do
     it "removes sensitive environment variables" do
       subject["SECRET_TOKEN"] = "password"

Library/Homebrew/test/system_command_spec.rb

@@ -252,5 +252,31 @@ describe SystemCommand do
         expect(system_command(executable)).to be_a_success
       end
     end
+
+    context "when given arguments with secrets" do
+      it "does not leak the secrets" do
+        redacted_msg = /#{Regexp.escape("username:******")}/
+        expect do
+          described_class.run! "curl",
+                               args:    %w[--user username:hunter2],
+                               verbose: true,
+                               secrets: %w[hunter2]
+        end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout
+      end
+
+      it "does not leak the secrets set by environment" do
+        redacted_msg = /#{Regexp.escape("username:******")}/
+        expect do
+          begin
+            ENV["PASSWORD"] = "hunter2"
+            described_class.run! "curl",
+                                 args:    %w[--user username:hunter2],
+                                 verbose: true
+          ensure
+            ENV.delete "PASSWORD"
+          end
+        end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout
+      end
+    end
   end
 end

Library/Homebrew/utils.rb

@@ -500,3 +500,7 @@ end
 def command_help_lines(path)
   path.read.lines.grep(/^#:/).map { |line| line.slice(2..-1) }
 end
+
+def redact_secrets(input, secrets)
+  secrets.reduce(input) { |str, secret| str.gsub secret, "******" }.freeze
+end

Library/Homebrew/utils/curl.rb

@@ -42,13 +42,14 @@ def curl_args(*extra_args, show_output: false, user_agent: :default)
   args + extra_args
 end
 
-def curl(*args)
+def curl(*args, secrets: [], **options)
   # SSL_CERT_FILE can be incorrectly set by users or portable-ruby and screw
   # with SSL downloads so unset it here.
   system_command! curl_executable,
-                  args:         curl_args(*args),
+                  args:         curl_args(*args, **options),
                   print_stdout: true,
-                  env:          { "SSL_CERT_FILE" => nil }
+                  env:          { "SSL_CERT_FILE" => nil },
+                  secrets:      secrets
 end
 
 def curl_download(*args, to: nil, **options)
@@ -77,10 +78,11 @@ def curl_download(*args, to: nil, **options)
   curl("--location", "--remote-time", "--continue-at", continue_at.to_s, "--output", destination, *args, **options)
 end
 
-def curl_output(*args, **options)
+def curl_output(*args, secrets: [], **options)
-  system_command(curl_executable,
+  system_command curl_executable,
                  args:         curl_args(*args, show_output: true, **options),
-                 print_stderr: false)
+                 print_stderr: false,
+                 secrets:      secrets
 end
 
 def curl_check_http_content(url, user_agents: [:default], check_content: false, strict: false)

Library/Homebrew/utils/github.rb

@@ -195,7 +195,7 @@ module GitHub
 
       args += ["--dump-header", headers_tmpfile.path]
 
-      output, errors, status = curl_output("--location", url.to_s, *args)
+      output, errors, status = curl_output("--location", url.to_s, *args, secrets: [token])
       output, _, http_code = output.rpartition("\n")
       output, _, http_code = output.rpartition("\n") if http_code == "000"
       headers = headers_tmpfile.read