Fix problematic usage of ENV for Windows

As part of PUP-5726, Windows API calls for retrieving / setting environment variables were added. This was done to work around a Ruby ENV bug https://bugs.ruby-lang.org/issues/8822 that corrupts UTF8 characters set in the environment (as keys or values) that don't map cleanly to the local codepage. While this was allegedly fixed in Ruby 2.3.0, it has not yet been backported or scheduled for backport.

Therefore, the withenv helper was updated to not use Rubys ENV, and to instead use the API calls.

However, there are a number of places where ENV is accessed and modified that may also trigger corruption issues.

This ticket should cover:

• Finding other danger spots in the code - for instance access through ENV of PATH and HOME have risks of being problematic, but SYSTEMROOT, PATHEXT, windir and COMSPEC should be safe to ignore due to the low likelihood of these values containing Unicode characters. ENV usage inside tests can also generally be ignored, as problems running specs would likely only manifest on a non-English Windows install with a username that includes Unicode chars, that has a Unicode path as a home dir.
  • One way to synthetically trigger failures would be to insert a Unicode value into PATH like c:\ᚠ or similarly set HOME
• Potentially adding a cross-platform wrapper for setting / reading env vars (that delegates to ENV for POSIX and calls Windows APIs otherwise)
• Replace Windows specific ENV access where there is danger
• Finding danger spots in other modules by auditing the forge / notifying authors where appropriate (probably best done in another ticket)

I've grepped through all of the code, and have narrowed the problematic spots (covering the first point), down to:

C:\source\puppet\lib\puppet\defaults.rb:

  236:           ENV["PATH"] = "" if ENV["PATH"].nil?

  237:           ENV["PATH"] = value unless value == "none"

  238:           paths = ENV["PATH"].split(File::PATH_SEPARATOR)

  239            Puppet::Util::Platform.default_paths.each do |path|

  240:             ENV["PATH"] += File::PATH_SEPARATOR + path unless paths.include?(path)

C:\source\puppet\lib\puppet\util.rb:

  201:       ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|

  202          begin

  ...

  208:           if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV['HOME'] == "")

  209              # if we get here they have a tilde in their PATH.  We'll issue a single warning about this and then

C:\source\puppet\lib\puppet\file_system\uniquefile.rb:

  159:       for dir in [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp']

  160          if dir and stat = File.stat(dir) and stat.directory? and stat.writable?

C:\source\puppet\lib\puppet\node\environment.rb:

  451    def self.extralibs()

  452:     if ENV["PUPPETLIB"]

  453:       split_path(ENV["PUPPETLIB"])

C:\source\puppet\lib\puppet\test\test_helper.rb:

  124:       $old_env = ENV.to_hash

  125  

  ...

  176        # faster to use the compare-then-set model to avoid excessive work that it

  177        # justifies the complexity.  --daniel 2012-03-15

  178:       unless ENV.to_hash == $old_env

  179:         ENV.clear

  180:         $old_env.each {|k, v| ENV[k] = v }

C:\source\puppet\spec\unit\util\run_mode_spec.rb:

  276    def without_env(name, &block)

  277:     saved = ENV[name]

  278:     ENV.delete name

  279      yield

  280    ensure

  281:     ENV[name] = saved

  282    end