Merge pull request #9276 from chrisroberts/e-scrub

Scrub sensitive information prior to message output

lib/vagrant.rb

@@ -1,7 +1,8 @@
 require "vagrant/shared_helpers"
 
-require 'rubygems'
-require 'log4r'
+require "rubygems"
+require "log4r"
+require "vagrant/util"
 
 # Enable logging if it is requested. We do this before
 # anything else so that we can setup the output before
@@ -41,12 +42,14 @@ if ENV["VAGRANT_LOG"] && ENV["VAGRANT_LOG"] != ""
     logger = Log4r::Logger.new("vagrant")
     logger.outputters = Log4r::Outputter.stderr
     logger.level = level
+    base_formatter = Log4r::BasicFormatter.new
     if ENV["VAGRANT_LOG_TIMESTAMP"]
-      Log4r::Outputter.stderr.formatter = Log4r::PatternFormatter.new(
+      base_formatter = Log4r::PatternFormatter.new(
         pattern: "%d [%5l] %m",
         date_pattern: "%F %T"
       )
     end
+    Log4r::Outputter.stderr.formatter = Vagrant::Util::LoggingFormatter.new(base_formatter)
     logger = nil
   end
 end
@@ -69,7 +72,8 @@ global_logger.info("Vagrant version: #{Vagrant::VERSION}")
 global_logger.info("Ruby version: #{RUBY_VERSION}")
 global_logger.info("RubyGems version: #{Gem::VERSION}")
 ENV.each do |k, v|
-  global_logger.info("#{k}=#{v.inspect}") if k =~ /^VAGRANT_/
+  next if k.start_with?("VAGRANT_OLD")
+  global_logger.info("#{k}=#{v.inspect}") if k.start_with?("VAGRANT_")
 end
 
 # We need these components always so instead of an autoload we

lib/vagrant/ui.rb

@@ -248,7 +248,7 @@ module Vagrant
       end
 
       def format_message(type, message, **opts)
-        message
+        Util::CredentialScrubber.desensitize(message)
       end
     end
 
@@ -320,6 +320,8 @@ module Vagrant
             type == :detail || type == :ask || opts[:prefix_spaces]
         end
 
+        message = Util::CredentialScrubber.desensitize(message)
+
         # Fast-path if there is no prefix
         return message if prefix.empty?
 

lib/vagrant/util.rb

@@ -8,6 +8,7 @@ module Vagrant
     autoload :Env,                       'vagrant/util/env'
     autoload :HashWithIndifferentAccess, 'vagrant/util/hash_with_indifferent_access'
     autoload :GuestInspection,           'vagrant/util/guest_inspection'
+    autoload :LoggingFormatter,          'vagrant/util/logging_formatter'
     autoload :Platform,                  'vagrant/util/platform'
     autoload :Retryable,                 'vagrant/util/retryable'
     autoload :SafeExec,                  'vagrant/util/safe_exec'

lib/vagrant/util/credential_scrubber.rb

@@ -24,6 +24,45 @@ module Vagrant
           uri.to_s
         end
       end
+
+      # Remove sensitive information from string
+      #
+      # @param [String] string
+      # @return [String]
+      def self.desensitize(string)
+        string = string.dup
+        sensitive_strings.each do |remove|
+          string.gsub!(remove, REPLACEMENT_TEXT)
+        end
+        string
+      end
+
+      # Register a sensitive string to be scrubbed
+      def self.sensitive(string)
+        sensitive_strings.push(string).uniq!
+        nil
+      end
+
+      # Deregister a sensitive string and allow output
+      def self.unsensitive(string)
+        sensitive_strings.delete(string)
+        nil
+      end
+
+      # @return [Array<string>]
+      def self.sensitive_strings
+        if !defined?(@_sensitive_strings)
+          @_sensitive_strings = []
+        end
+        @_sensitive_strings
+      end
+
+      # @private
+      # Reset the cached values for scrubber. This is not considered a public
+      # API and should only be used for testing.
+      def self.reset!
+        instance_variables.each(&method(:remove_instance_variable))
+      end
     end
   end
 end

lib/vagrant/util/logging_formatter.rb

@@ -0,0 +1,28 @@
+require "vagrant/util/credential_scrubber"
+require "log4r/formatter/formatter"
+
+module Vagrant
+  module Util
+    # Wrapper for logging formatting to provide
+    # information scrubbing prior to being written
+    # to output target
+    class LoggingFormatter < Log4r::BasicFormatter
+
+      # @return [Log4r::PatternFormatter]
+      attr_reader :formatter
+
+      # Creates a new formatter wrapper instance.
+      #
+      # @param [Log4r::Formatter]
+      def initialize(formatter)
+        @formatter = formatter
+      end
+
+      # Format event and scrub output
+      def format(event)
+        msg = formatter.format(event)
+        CredentialScrubber.desensitize(msg)
+      end
+    end
+  end
+end

test/unit/vagrant/ui_test.rb

@@ -96,6 +96,20 @@ describe Vagrant::UI::Basic do
       subject.detail("foo")
     end
   end
+
+  context "with sensitive data" do
+    let(:password){ "my-birthday" }
+    let(:output){ "You're password is: #{password}" }
+
+    before{ Vagrant::Util::CredentialScrubber.sensitive(password) }
+
+    it "should remove sensitive information from the output" do
+      expect(subject).to receive(:safe_puts).with(any_args) do |message, **opts|
+        expect(message).not_to include(password)
+      end
+      subject.detail(output)
+    end
+  end
 end
 
 describe Vagrant::UI::Colored do

test/unit/vagrant/util/credential_scrubber_test.rb

@@ -0,0 +1,88 @@
+require File.expand_path("../../../base", __FILE__)
+
+require "vagrant/util/credential_scrubber"
+
+describe Vagrant::Util::CredentialScrubber do
+  subject{ Vagrant::Util::CredentialScrubber }
+
+  after{ subject.reset! }
+
+  describe ".url_scrubber" do
+    let(:user){ "vagrant-user" }
+    let(:password){ "vagrant-pass" }
+    let(:url){ "http://#{user}:#{password}@example.com" }
+
+    it "should remove user credentials from URL" do
+      result = subject.url_scrubber(url)
+      expect(result).not_to include(user)
+      expect(result).not_to include(password)
+    end
+  end
+
+  describe ".sensitive" do
+    it "should return a nil value" do
+      expect(subject.sensitive("value")).to be_nil
+    end
+
+    it "should add value to list of strings" do
+      subject.sensitive("value")
+      expect(subject.sensitive_strings).to include("value")
+    end
+
+    it "should remove duplicates" do
+      subject.sensitive("value")
+      subject.sensitive("value")
+      expect(subject.sensitive_strings.count("value")).to eq(1)
+    end
+  end
+
+  describe ".unsensitive" do
+    it "should return a nil value" do
+      expect(subject.unsensitive("value")).to be_nil
+    end
+
+    it "should remove value from list" do
+      subject.sensitive("value")
+      expect(subject.sensitive_strings).to include("value")
+      subject.unsensitive("value")
+      expect(subject.sensitive_strings).not_to include("value")
+    end
+  end
+
+  describe ".sensitive_strings" do
+    it "should always return the same array" do
+      expect(subject.sensitive_strings).to be(subject.sensitive_strings)
+    end
+  end
+
+  describe ".desensitize" do
+    let(:to_scrub){ [] }
+    let(:string){ "a line of text with my-birthday and my-cats-birthday embedded" }
+    before{ to_scrub.each{|s| subject.sensitive(s) }}
+
+    context "with no sensitive strings registered" do
+      it "should not modify the string" do
+        expect(subject.desensitize(string)).to eq(string)
+      end
+    end
+
+    context "with single value registered" do
+      let(:to_scrub){ ["my-birthday"] }
+
+      it "should remove the registered value" do
+        expect(subject.desensitize(string)).not_to include(to_scrub.first)
+      end
+    end
+
+    context "with multiple values registered" do
+      let(:to_scrub){ ["my-birthday", "my-cats-birthday"] }
+
+      it "should remove all registered values" do
+        result = subject.desensitize(string)
+        to_scrub.each do |registered_value|
+          expect(result).not_to include(registered_value)
+        end
+      end
+    end
+  end
+end