Merge pull request #5912 from mitchellh/sethvargo/subprocess_escape

Automatically restore the original environment if we are runnings something outside of the embedded installer

lib/vagrant/util/env.rb

@@ -1,7 +1,17 @@
+require "bundler"
+
 module Vagrant
   module Util
     class Env
-      #
+      def self.with_original_env
+        original_env = ENV.to_hash
+        ENV.replace(::Bundler::ORIGINAL_ENV) if defined?(::Bundler::ORIGINAL_ENV)
+        ENV.update(Vagrant.original_env)
+        yield
+      ensure
+        ENV.replace(original_env.to_hash)
+      end
+
       # Execute the given command, removing any Ruby-specific environment
       # variables. This is an "enhanced" version of `Bundler.with_clean_env`,
       # which only removes Bundler-specific values. We need to remove all
@@ -25,18 +35,16 @@ module Vagrant
       #
       # @param [Proc] block
       #   the block to execute with the cleaned environment
-      #
+      def self.with_clean_env
-      def self.with_clean_env(&block)
+        with_original_env do
-        original = ENV.to_hash
+          ENV["MANPATH"] = ENV["BUNDLE_ORIG_MANPATH"]
-
+          ENV.delete_if { |k,_| k[0,7] == "BUNDLE_" }
-        ENV.delete('_ORIGINAL_GEM_PATH')
+          if ENV.has_key? "RUBYOPT"
-        ENV.delete_if { |k,_| k.start_with?('BUNDLE_') }
+            ENV["RUBYOPT"] = ENV["RUBYOPT"].sub("-rbundler/setup", "")
-        ENV.delete_if { |k,_| k.start_with?('GEM_') }
+            ENV["RUBYOPT"] = ENV["RUBYOPT"].sub("-I#{File.expand_path('..', __FILE__)}", "")
-        ENV.delete_if { |k,_| k.start_with?('RUBY') }
+          end
-
+          yield
-        yield
+        end
-      ensure
-        ENV.replace(original.to_hash)
       end
     end
   end

lib/vagrant/util/subprocess.rb

@@ -72,32 +72,39 @@ module Vagrant
         process.io.stderr = stderr_writer
         process.duplex = true
 
-        # If we're in an installer on Mac and we're executing a command
+        # Special installer-related things
-        # in the installer context, then force DYLD_LIBRARY_PATH to look
+        if Vagrant.in_installer?
-        # at our libs first.
-        if Vagrant.in_installer? && Platform.darwin?
           installer_dir = ENV["VAGRANT_INSTALLER_EMBEDDED_DIR"].to_s.downcase
-          if @command[0].downcase.include?(installer_dir)
+
-            @logger.info("Command in the installer. Specifying DYLD_LIBRARY_PATH...")
+          # If we're in an installer on Mac and we're executing a command
-            process.environment["DYLD_LIBRARY_PATH"] =
+          # in the installer context, then force DYLD_LIBRARY_PATH to look
-              "#{installer_dir}/lib:#{ENV["DYLD_LIBRARY_PATH"]}"
+          # at our libs first.
-          else
+          if Platform.darwin?
-            @logger.debug("Command not in installer, not touching env vars.")
+            if @command[0].downcase.include?(installer_dir)
+              @logger.info("Command in the installer. Specifying DYLD_LIBRARY_PATH...")
+              process.environment["DYLD_LIBRARY_PATH"] =
+                "#{installer_dir}/lib:#{ENV["DYLD_LIBRARY_PATH"]}"
+            else
+              @logger.debug("Command not in installer, not touching env vars.")
+            end
+
+            if File.setuid?(@command[0]) || File.setgid?(@command[0])
+              @logger.info("Command is setuid/setgid, clearing DYLD_LIBRARY_PATH")
+              process.environment["DYLD_LIBRARY_PATH"] = ""
+            end
           end
 
-          if File.setuid?(@command[0]) || File.setgid?(@command[0])
+          # If the command that is being run is not inside the installer, reset
-            @logger.info("Command is setuid/setgid, clearing DYLD_LIBRARY_PATH")
+          # the original environment - this is required for shelling out to
-            process.environment["DYLD_LIBRARY_PATH"] = ""
+          # other subprocesses that depend on environment variables (like Ruby
-          end
+          # and $GEM_PATH for example)
-        end
+          if !@command[0].downcase.include?(installer_dir)
-
+            @logger.info("Command not in installer, restoring original environment...")
-        # Reset the Bundler environment back - this is required for anyone who
+            jailbreak(process.environment)
-        # is not using the official Vagrant installers and is running Vagrant
-        # via bundler
-        if defined?(Bundler::ORIGINAL_ENV)
-          Bundler::ORIGINAL_ENV.each do |k, v|
-            process.environment[k] = v
           end
+        else
+          @logger.info("Vagrant not running in installer, restoring original environment...")
+          jailbreak(process.environment)
         end
 
         # Set the environment on the process if we must
@@ -248,6 +255,58 @@ module Vagrant
           @stderr    = stderr
         end
       end
+
+      private
+
+      # This is, quite possibly, the saddest function in all of Vagrant.
+      #
+      # If a user is running Vagrant via Bundler (but not via the official
+      # installer), we want to reset to the "original" environment so that when
+      # shelling out to other Ruby processes (specifically), the original
+      # environment is restored. This is super important for things like
+      # rbenv and chruby, who rely on environment variables to locate gems, but
+      # Bundler stomps on those environment variables like an angry T-Rex after
+      # watching Jurassic Park 2 and realizing they replaced you with CGI.
+      #
+      # If a user is running in Vagrant via the official installer, BUT trying
+      # to execute a subprocess *outside* of the installer, we want to reset to
+      # the "original" environment. In this case, the Vagrant installer actually
+      # knows what the original environment was and replaces it completely.
+      #
+      # Finally, we reset any Bundler-specific environment variables, since the
+      # subprocess being called could, itself, be Bundler. And Bundler does not
+      # behave very nicely in these circumstances.
+      #
+      # This function was added in Vagrant 1.7.3, but there is a failsafe
+      # because the author doesn't trust himself that this functionality won't
+      # break existing assumptions, so users can specify
+      # `VAGRANT_SKIP_SUBPROCESS_JAILBREAK` and none of the above will happen.
+      #
+      # This function modifies the given hash in place!
+      #
+      # @return [nil]
+      def jailbreak(env = {})
+        return if ENV.key?("VAGRANT_SKIP_SUBPROCESS_JAILBREAK")
+
+        env.replace(::Bundler::ORIGINAL_ENV) if defined?(::Bundler::ORIGINAL_ENV)
+        env.merge!(Vagrant.original_env)
+
+        # Bundler does this, so I guess we should as well, since I think it
+        # other subprocesses that use Bundler will reload it
+        env["MANPATH"] = ENV["BUNDLE_ORIG_MANPATH"]
+
+        # Replace all current environment BUNDLE_ variables to nil
+        ENV.each do |k,_|
+          env[k] = nil if k[0,7] == "BUNDLE_"
+        end
+
+        # If RUBYOPT was set, unset it with Bundler
+        if ENV.key?("RUBYOPT")
+          env["RUBYOPT"] = ENV["RUBYOPT"].sub("-rbundler/setup", "")
+        end
+
+        nil
+      end
     end
   end
 end

plugins/provisioners/chef/provisioner/chef_client.rb

@@ -114,16 +114,14 @@ module VagrantPlugins
           # Knife is not part of the current Vagrant bundle, so it needs to run
           # in the context of the system.
           Vagrant.global_lock do
-            Vagrant::Util::Env.with_clean_env do
+            command = ["knife", deletable, "delete", "--yes", node_name]
-              command = ["knife", deletable, "delete", "--yes", node_name]
+            r = Vagrant::Util::Subprocess.execute(*command)
-              r = Vagrant::Util::Subprocess.execute(*command)
+            if r.exit_code != 0
-              if r.exit_code != 0
+              @machine.ui.error(I18n.t(
-                @machine.ui.error(I18n.t(
+                "vagrant.chef_client_cleanup_failed",
-                  "vagrant.chef_client_cleanup_failed",
+                deletable: deletable,
-                  deletable: deletable,
+                stdout: r.stdout,
-                  stdout: r.stdout,
+                stderr: r.stderr))
-                  stderr: r.stderr))
-              end
             end
           end
         end

website/docs/source/v2/other/environmental-variables.html.md

@@ -90,6 +90,19 @@ Note that any `vagrant plugin` commands automatically don't load any
 plugins, so if you do install any unstable plugins, you can always use
 the `vagrant plugin` commands without having to worry.
 
+## VAGRANT\_SKIP\_SUBPROCESS\_JAILBREAK
+
+As of Vagrant 1.7.3, Vagrant tries to intelligently detect if it is running in
+the installer or running via Bundler. Although not officially supported, Vagrant
+tries its best to work when executed via Bundler. When Vagrant detects that you
+have spawned a subprocess that lives outside of Vagrant's installer, Vagrant
+will do its best to reset the preserved environment dring the subprocess
+execution.
+
+If Vagrant detects it is running outside of the officially installer, the
+original environment will always be restored. You can disable this automatic
+jailbreak by setting the `VAGRANT_SKIP_SUBPROCES_JAILBREAK`.
+
 ## VAGRANT\_VAGRANTFILE
 
 This specifies the filename of the Vagrantfile that Vagrant searches for.