Merge pull request #10298 from chef/lcg/fix-bootstrapping-path-issues

6 files changed, 335 insertions, 142 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index ee25df6954..a767d072ef 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -74,42 +74,66 @@ module ChefConfig
       path
     end
 
-    # On *nix, /etc/chef
-    def self.etc_chef_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : PathHelper.join("/etc", ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+    # On *nix, /etc/chef, on Windows C:\chef
+    #
+    # @param windows [Boolean] optional flag to force to windows or unix-style
+    # @return [String] the platform-specific path
+    #
+    def self.etc_chef_dir(windows: ChefUtils.windows?)
+      path = windows ? c_chef_dir : PathHelper.join("/etc", ChefConfig::Dist::DIR_SUFFIX, windows: windows)
+      PathHelper.cleanpath(path, windows: windows)
     end
 
-    # On *nix, /var/chef
-    def self.var_chef_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : PathHelper.join("/var", ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+    # On *nix, /var/chef, on Windows C:\chef
+    #
+    # @param windows [Boolean] optional flag to force to windows or unix-style
+    # @return [String] the platform-specific path
+    #
+    def self.var_chef_dir(windows: ChefUtils.windows?)
+      path = windows ? c_chef_dir : PathHelper.join("/var", ChefConfig::Dist::DIR_SUFFIX, windows: windows)
+      PathHelper.cleanpath(path, windows: windows)
     end
 
-    # On *nix, the root of /var/, used to test if we can create and write in /var/chef
-    def self.var_root_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : "/var"
-      PathHelper.cleanpath(path)
+    # On *nix, /var, on Windows C:\
+    #
+    # @param windows [Boolean] optional flag to force to windows or unix-style
+    # @return [String] the platform-specific path
+    #
+    def self.var_root_dir(windows: ChefUtils.windows?)
+      path = windows ? "C:\\" : "/var"
+      PathHelper.cleanpath(path, windows: windows)
     end
 
     # On windows, C:/chef/
-    def self.c_chef_dir
+    #
+    # (should only be called in a windows-context)
+    #
+    # @return [String] the platform-specific path
+    #
+    def self.c_chef_dir(windows: ChefUtils.windows?)
       drive = windows_installation_drive || "C:"
-      path = PathHelper.join(drive, ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+      PathHelper.join(drive, ChefConfig::Dist::DIR_SUFFIX, windows: windows)
     end
 
-    def self.c_opscode_dir
+    # On windows, C:/opscode
+    #
+    # (should only be called in a windows-context)
+    #
+    # @return [String] the platform-specific path
+    #
+    def self.c_opscode_dir(windows: ChefUtils.windows?)
       drive = windows_installation_drive || "C:"
-      path = PathHelper.join(drive, ChefConfig::Dist::LEGACY_CONF_DIR, ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+      PathHelper.join(drive, ChefConfig::Dist::LEGACY_CONF_DIR, ChefConfig::Dist::DIR_SUFFIX, windows: windows)
     end
 
     # the drive where Chef is installed on a windows host. This is determined
     # either by the drive containing the current file or by the SYSTEMDRIVE ENV
     # variable
     #
+    # (should only be called in a windows-context)
+    #
     # @return [String] the drive letter
+    #
     def self.windows_installation_drive
       if ChefUtils.windows?
         drive = File.expand_path(__FILE__).split("/", 2)[0]
@@ -342,11 +366,11 @@ module ChefConfig
         # the cache path.
         unless path_accessible?(primary_cache_path) || path_accessible?(primary_cache_root)
           secondary_cache_path = PathHelper.join(user_home, ChefConfig::Dist::USER_CONF_DIR)
-          secondary_cache_path = target_mode? ? "#{secondary_cache_path}/#{target_mode.host}" : secondary_cache_path
+          secondary_cache_path = target_mode? ? PathHelper.join(secondary_cache_path, target_mode.host) : secondary_cache_path
           ChefConfig.logger.trace("Unable to access cache at #{primary_cache_path}. Switching cache to #{secondary_cache_path}")
           secondary_cache_path
         else
-          target_mode? ? "#{primary_cache_path}/#{target_mode.host}" : primary_cache_path
+          target_mode? ? PathHelper.join(primary_cache_path, target_mode.host) : primary_cache_path
         end
       end
     end
diff --git a/chef-config/lib/chef-config/path_helper.rb b/chef-config/lib/chef-config/path_helper.rb
index 19e7cce26a..8fe45febfc 100644
--- a/chef-config/lib/chef-config/path_helper.rb
+++ b/chef-config/lib/chef-config/path_helper.rb
@@ -26,14 +26,14 @@ module ChefConfig
     # Maximum characters in a standard Windows path (260 including drive letter and NUL)
     WIN_MAX_PATH = 259
 
-    def self.dirname(path)
-      if ChefUtils.windows?
+    def self.dirname(path, windows: ChefUtils.windows?)
+      if windows
         # Find the first slash, not counting trailing slashes
         end_slash = path.size
         loop do
-          slash = path.rindex(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator)}]/, end_slash - 1)
+          slash = path.rindex(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator(windows: windows))}]/, end_slash - 1)
           if !slash
-            return end_slash == path.size ? "." : path_separator
+            return end_slash == path.size ? "." : path_separator(windows: windows)
           elsif slash == end_slash - 1
             end_slash = slash
           else
@@ -47,29 +47,28 @@ module ChefConfig
 
     BACKSLASH = '\\'.freeze
 
-    def self.path_separator
-      if ChefUtils.windows?
-        File::ALT_SEPARATOR || BACKSLASH
+    def self.path_separator(windows: ChefUtils.windows?)
+      if windows
+        BACKSLASH
       else
         File::SEPARATOR
       end
     end
 
-    path_separator_regex = [Regexp.escape(File::SEPARATOR), Regexp.escape(path_separator)].uniq.join
+    def self.join(*args, windows: ChefUtils.windows?)
+      path_separator_regex = Regexp.escape(windows ? "#{File::SEPARATOR}#{BACKSLASH}" : File::SEPARATOR)
+      trailing_slashes_regex = /[#{path_separator_regex}]+$/.freeze
+      leading_slashes_regex = /^[#{path_separator_regex}]+/.freeze
 
-    TRAILING_SLASHES_REGEX = /[#{path_separator_regex}]+$/.freeze
-    LEADING_SLASHES_REGEX = /^[#{path_separator_regex}]+/.freeze
-
-    def self.join(*args)
       args.flatten.inject do |joined_path, component|
-        joined_path = joined_path.sub(TRAILING_SLASHES_REGEX, "")
-        component = component.sub(LEADING_SLASHES_REGEX, "")
-        joined_path + "#{path_separator}#{component}"
+        joined_path = joined_path.sub(trailing_slashes_regex, "")
+        component = component.sub(leading_slashes_regex, "")
+        joined_path + "#{path_separator(windows: windows)}#{component}"
       end
     end
 
-    def self.validate_path(path)
-      if ChefUtils.windows?
+    def self.validate_path(path, windows: ChefUtils.windows?)
+      if windows
         unless printable?(path)
           msg = "Path '#{path}' contains non-printable characters. Check that backslashes are escaped with another backslash (e.g. C:\\\\Windows) in double-quoted strings."
           ChefConfig.logger.error(msg)
@@ -108,14 +107,14 @@ module ChefConfig
     end
 
     # Produces a comparable path.
-    def self.canonical_path(path, add_prefix = true)
+    def self.canonical_path(path, add_prefix = true, windows: ChefUtils.windows?)
       # First remove extra separators and resolve any relative paths
       abs_path = File.absolute_path(path)
 
-      if ChefUtils.windows?
+      if windows
         # Add the \\?\ API prefix on Windows unless add_prefix is false
         # Downcase on Windows where paths are still case-insensitive
-        abs_path.gsub!(::File::SEPARATOR, path_separator)
+        abs_path.gsub!(::File::SEPARATOR, path_separator(windows: windows))
         if add_prefix && abs_path !~ /^\\\\?\\/
           abs_path.insert(0, "\\\\?\\")
         end
@@ -126,36 +125,67 @@ module ChefConfig
       abs_path
     end
 
-    # This is the INVERSE of Pathname#cleanpath, it converts forward
-    # slashes to backslashes for Windows.  Since the Ruby API and the
-    # Windows APIs all consume forward slashes, this helper function
-    # should only be used for *DISPLAY* logic to send strings back
-    # to the user with backslashes.  Internally, filename paths should
-    # generally be stored with forward slashes for consistency.  It is
-    # not necessary or desired to blindly convert pathnames to have
-    # backslashes on Windows.
+    # The built in ruby Pathname#cleanpath method does not clean up forward slashes and
+    # backslashes.  This is a wrapper around that which does.  In general this is NOT
+    # recommended for internal use within ruby/chef since ruby does not care about forward slashes
+    # vs. backslashes, even on Windows.  Where this generally matters is when being rendered
+    # to the user, or being rendered into things like the windows PATH or to commands that
+    # are being executed.  In some cases it may be easier on windows to render paths to
+    # unix-style for being eventually eval'd by ruby in the future (templates being rendered
+    # with code to be consumed by ruby) where forcing unix-style forward slashes avoids the
+    # issue of needing to escape the backslashes in rendered strings.  This has a boolean
+    # operator to force windows-style or non-windows style operation, where the default is
+    # determined by the underlying node['platform'] value.
+    #
+    # In general if you don't know if you need this routine, do not use it, best practice
+    # within chef/ruby itself is not to care.  Only use it to force windows or unix style
+    # when it really matters.
     #
-    # Generally, if the user isn't going to be seeing it, you should be
-    # using Pathname#cleanpath instead of this function.
-    def self.cleanpath(path)
+    # @param path [String] the path to clean
+    # @param windows [Boolean] optional flag to force to windows or unix-style
+    # @return [String] cleaned path
+    #
+    def self.cleanpath(path, windows: ChefUtils.windows?)
       path = Pathname.new(path).cleanpath.to_s
-      # ensure all forward slashes are backslashes
-      if ChefUtils.windows?
-        path = path.gsub(File::SEPARATOR, path_separator)
+      if windows
+        # ensure all forward slashes are backslashes
+        path.gsub(File::SEPARATOR, path_separator(windows: windows))
+      else
+        # ensure all backslashes are forward slashes
+        path.gsub(BACKSLASH, File::SEPARATOR)
       end
-      path
     end
 
-    def self.paths_eql?(path1, path2)
-      canonical_path(path1) == canonical_path(path2)
+    # This is not just escaping for something like use in Regexps, or in globs.  For the former
+    # just use Regexp.escape.  For the latter, use escape_glob_dir below.
+    #
+    # This is escaping where the path to be rendered is being put into a ruby file which will
+    # later be read back by ruby (or something similar) so we need quadruple backslashes.
+    #
+    # In order to print:
+    #
+    #   file_cache_path "C:\\chef"
+    #
+    # We need to convert "C:\chef" to "C:\\\\chef" to interpolate into a string which is rendered
+    # into the output file with that line in it.
+    #
+    # @param path [String] the path to escape
+    # @return [String] the escaped path
+    #
+    def self.escapepath(path)
+      path.gsub(BACKSLASH, BACKSLASH * 4)
+    end
+
+    def self.paths_eql?(path1, path2, windows: ChefUtils.windows?)
+      canonical_path(path1, windows: windows) == canonical_path(path2, windows: windows)
     end
 
     # @deprecated this method is deprecated. Please use escape_glob_dirs
     # Paths which may contain glob-reserved characters need
     # to be escaped before globbing can be done.
     # http://stackoverflow.com/questions/14127343
-    def self.escape_glob(*parts)
-      path = cleanpath(join(*parts))
+    def self.escape_glob(*parts, windows: ChefUtils.windows?)
+      path = cleanpath(join(*parts, windows: windows), windows: windows)
       path.gsub(/[\\\{\}\[\]\*\?]/) { |x| "\\" + x }
     end
 
@@ -166,8 +196,8 @@ module ChefConfig
       path.gsub(/[\\\{\}\[\]\*\?]/) { |x| "\\" + x }
     end
 
-    def self.relative_path_from(from, to)
-      Pathname.new(cleanpath(to)).relative_path_from(Pathname.new(cleanpath(from)))
+    def self.relative_path_from(from, to, windows: ChefUtils.windows?)
+      Pathname.new(cleanpath(to, windows: windows)).relative_path_from(Pathname.new(cleanpath(from, windows: windows)))
     end
 
     # Set the project-specific home directory environment variable.
@@ -213,11 +243,11 @@ module ChefConfig
     #
     # The return is a list of all the returned values from each block invocation or a list of paths
     # if no block is provided.
-    def self.all_homes(*args)
+    def self.all_homes(*args, windows: ChefUtils.windows?)
       paths = []
       paths << ENV[@@per_tool_home_environment] if defined?(@@per_tool_home_environment) && @@per_tool_home_environment && ENV[@@per_tool_home_environment]
       paths << ENV["CHEF_HOME"] if ENV["CHEF_HOME"]
-      if ChefUtils.windows?
+      if windows
         # By default, Ruby uses the the following environment variables to determine Dir.home:
         # HOME
         # HOMEDRIVE HOMEPATH
@@ -246,7 +276,7 @@ module ChefConfig
       # Note: Maybe this is a bad idea on some unixy systems where \ might be a valid character depending on
       # the particular brand of kool-aid you consume.  This code assumes that \ and / are both
       # path separators on any system being used.
-      paths = paths.map { |home_path| home_path.gsub(path_separator, ::File::SEPARATOR) if home_path }
+      paths = paths.map { |home_path| home_path.gsub(path_separator(windows: windows), ::File::SEPARATOR) if home_path }
 
       # Filter out duplicate paths and paths that don't exist.
       valid_paths = paths.select { |home_path| home_path && Dir.exist?(home_path.force_encoding("utf-8")) }
diff --git a/chef-config/spec/unit/config_spec.rb b/chef-config/spec/unit/config_spec.rb
index ccfe57c57c..8daee3fa19 100644
--- a/chef-config/spec/unit/config_spec.rb
+++ b/chef-config/spec/unit/config_spec.rb
@@ -222,13 +222,70 @@ RSpec.describe ChefConfig::Config do
       ChefConfig::Config.add_formatter(:doc, "/var/log/formatter.log")
       expect(ChefConfig::Config.formatters).to eq([[:doc, "/var/log/formatter.log"]])
     end
+  end
+
+  describe "#var_chef_path" do
+    let (:dirname) { ChefConfig::Dist::DIR_SUFFIX }
+
+    context "on unix", :unix_only do
+      it "var_chef_dir is /var/chef" do
+        expect(ChefConfig::Config.var_chef_dir).to eql("/var/#{dirname}")
+      end
+
+      it "var_root_dir is /var" do
+        expect(ChefConfig::Config.var_root_dir).to eql("/var")
+      end
+
+      it "etc_chef_dir is /etc/chef" do
+        expect(ChefConfig::Config.etc_chef_dir).to eql("/etc/#{dirname}")
+      end
+    end
+
+    context "on windows", :windows_only do
+      it "var_chef_dir is C:\\chef" do
+        expect(ChefConfig::Config.var_chef_dir).to eql("C:\\#{dirname}")
+      end
+
+      it "var_root_dir is C:\\" do
+        expect(ChefConfig::Config.var_root_dir).to eql("C:\\")
+      end
+
+      it "etc_chef_dir is C:\\chef" do
+        expect(ChefConfig::Config.etc_chef_dir).to eql("C:\\#{dirname}")
+      end
+    end
 
+    context "when forced to unix" do
+      it "var_chef_dir is /var/chef" do
+        expect(ChefConfig::Config.var_chef_dir(windows: false)).to eql("/var/#{dirname}")
+      end
+
+      it "var_root_dir is /var" do
+        expect(ChefConfig::Config.var_root_dir(windows: false)).to eql("/var")
+      end
+
+      it "etc_chef_dir is /etc/chef" do
+        expect(ChefConfig::Config.etc_chef_dir(windows: false)).to eql("/etc/#{dirname}")
+      end
+    end
+
+    context "when forced to windows" do
+      it "var_chef_dir is C:\\chef" do
+        expect(ChefConfig::Config.var_chef_dir(windows: true)).to eql("C:\\#{dirname}")
+      end
+
+      it "var_root_dir is C:\\" do
+        expect(ChefConfig::Config.var_root_dir(windows: true)).to eql("C:\\")
+      end
+
+      it "etc_chef_dir is C:\\chef" do
+        expect(ChefConfig::Config.etc_chef_dir(windows: true)).to eql("C:\\#{dirname}")
+      end
+    end
   end
 
   [ false, true ].each do |is_windows|
-
     context "On #{is_windows ? "Windows" : "Unix"}" do
-
       before :each do
         allow(ChefUtils).to receive(:windows?).and_return(is_windows)
       end
@@ -430,8 +487,8 @@ RSpec.describe ChefConfig::Config do
 
         describe "ChefConfig::Config[:cache_path]" do
           let(:target_mode_host) { "fluffy.kittens.org" }
-          let(:target_mode_primary_cache_path) { "#{primary_cache_path}/#{target_mode_host}" }
-          let(:target_mode_secondary_cache_path) { "#{secondary_cache_path}/#{target_mode_host}" }
+          let(:target_mode_primary_cache_path) { ChefUtils.windows? ? "#{primary_cache_path}\\#{target_mode_host}" : "#{primary_cache_path}/#{target_mode_host}" }
+          let(:target_mode_secondary_cache_path) { ChefUtils.windows? ? "#{secondary_cache_path}\\#{target_mode_host}" : "#{secondary_cache_path}/#{target_mode_host}" }
 
           before do
             if is_windows
diff --git a/chef-config/spec/unit/path_helper_spec.rb b/chef-config/spec/unit/path_helper_spec.rb
index f416bb1826..3eedb8bbf5 100644
--- a/chef-config/spec/unit/path_helper_spec.rb
+++ b/chef-config/spec/unit/path_helper_spec.rb
@@ -23,9 +23,8 @@ RSpec.describe ChefConfig::PathHelper do
 
   let(:path_helper) { described_class }
 
-  shared_examples_for "common_functionality" do
-    describe "join" do
-
+  context "common functionality" do
+    context "join" do
       it "joins starting with '' resolve to absolute paths" do
         expect(path_helper.join("", "a", "b")).to eq("#{path_helper.path_separator}a#{path_helper.path_separator}b")
       end
@@ -33,10 +32,9 @@ RSpec.describe ChefConfig::PathHelper do
       it "joins ending with '' add a / to the end" do
         expect(path_helper.join("a", "b", "")).to eq("a#{path_helper.path_separator}b#{path_helper.path_separator}")
       end
-
     end
 
-    describe "dirname" do
+    context "dirname" do
       it "dirname('abc') is '.'" do
         expect(path_helper.dirname("abc")).to eq(".")
       end
@@ -55,42 +53,109 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
+  context "forcing windows/non-windows" do
+    context "forcing windows" do
+      it "path_separator is \\" do
+        expect(path_helper.path_separator(windows: true)).to eq('\\')
+      end
+
+      context "platform-specific #join behavior" do
+        it "joins components on Windows when some end with unix separators" do
+          expected = "C:\\foo\\bar\\baz"
+          expect(path_helper.join('C:\\foo/', "bar", "baz", windows: true)).to eq(expected)
+        end
+
+        it "joins components when some end with separators" do
+          expected = "C:\\foo\\bar\\baz"
+          expect(path_helper.join('C:\\foo\\', "bar", "baz", windows: true)).to eq(expected)
+        end
+
+        it "joins components when some end and start with separators" do
+          expected = "C:\\foo\\bar\\baz"
+          expect(path_helper.join('C:\\foo\\', "bar/", "/baz", windows: true)).to eq(expected)
+        end
+
+        it "joins components that don't end in separators" do
+          expected = "C:\\foo\\bar\\baz"
+          expect(path_helper.join('C:\\foo', "bar", "baz", windows: true)).to eq(expected)
+        end
+      end
+
+      it "cleanpath changes slashes into backslashes and leaves backslashes alone" do
+        expect(path_helper.cleanpath('/a/b\\c/d/', windows: true)).to eq('\\a\\b\\c\\d')
+      end
+
+      it "cleanpath does not remove leading double backslash" do
+        expect(path_helper.cleanpath('\\\\a/b\\c/d/', windows: true)).to eq('\\\\a\\b\\c\\d')
+      end
+    end
+
+    context "forcing unix" do
+      it "path_separator is /" do
+        expect(path_helper.path_separator(windows: false)).to eq("/")
+      end
+
+      it "cleanpath removes extra slashes alone" do
+        expect(path_helper.cleanpath("/a///b/c/d/", windows: false)).to eq("/a/b/c/d")
+      end
+
+      context "platform-specific #join behavior" do
+        it "joins components when some end with separators" do
+          expected = "/foo/bar/baz"
+          expect(path_helper.join("/foo/", "bar", "baz", windows: false)).to eq(expected)
+        end
+
+        it "joins components when some end and start with separators" do
+          expected = "/foo/bar/baz"
+          expect(path_helper.join("/foo/", "bar/", "/baz", windows: false)).to eq(expected)
+        end
+
+        it "joins components that don't end in separators" do
+          expected = "/foo/bar/baz"
+          expect(path_helper.join("/foo", "bar", "baz", windows: false)).to eq(expected)
+        end
+      end
+
+      it "cleanpath changes backslashes into slashes and leaves slashes alone" do
+        expect(path_helper.cleanpath('/a/b\\c/d/', windows: false)).to eq("/a/b/c/d")
+      end
+
+      it "cleanpath does not remove leading double backslash" do
+        expect(path_helper.cleanpath('\\\\a/b\\c/d/', windows: false)).to eq("//a/b/c/d")
+      end
+    end
+  end
+
   context "on windows", :windows_only do
 
     before(:each) do
       allow(ChefUtils).to receive(:windows?).and_return(true)
     end
 
-    include_examples("common_functionality")
-
     it "path_separator is \\" do
       expect(path_helper.path_separator).to eq('\\')
     end
 
-    describe "platform-specific #join behavior" do
-
+    context "platform-specific #join behavior" do
       it "joins components on Windows when some end with unix separators" do
-        expect(path_helper.join('C:\\foo/', "bar", "baz")).to eq('C:\\foo\\bar\\baz')
+        expected = "C:\\foo\\bar\\baz"
+        expect(path_helper.join('C:\\foo/', "bar", "baz")).to eq(expected)
       end
 
       it "joins components when some end with separators" do
-        expected = path_helper.cleanpath("/foo/bar/baz")
-        expected = "C:#{expected}"
+        expected = "C:\\foo\\bar\\baz"
         expect(path_helper.join('C:\\foo\\', "bar", "baz")).to eq(expected)
       end
 
       it "joins components when some end and start with separators" do
-        expected = path_helper.cleanpath("/foo/bar/baz")
-        expected = "C:#{expected}"
+        expected = "C:\\foo\\bar\\baz"
         expect(path_helper.join('C:\\foo\\', "bar/", "/baz")).to eq(expected)
       end
 
       it "joins components that don't end in separators" do
-        expected = path_helper.cleanpath("/foo/bar/baz")
-        expected = "C:#{expected}"
+        expected = "C:\\foo\\bar\\baz"
         expect(path_helper.join('C:\\foo', "bar", "baz")).to eq(expected)
       end
-
     end
 
     it "cleanpath changes slashes into backslashes and leaves backslashes alone" do
@@ -100,17 +165,13 @@ RSpec.describe ChefConfig::PathHelper do
     it "cleanpath does not remove leading double backslash" do
       expect(path_helper.cleanpath('\\\\a/b\\c/d/')).to eq('\\\\a\\b\\c\\d')
     end
-
   end
 
   context "on unix", :unix_only do
-
     before(:each) do
       allow(ChefUtils).to receive(:windows?).and_return(false)
     end
 
-    include_examples("common_functionality")
-
     it "path_separator is /" do
       expect(path_helper.path_separator).to eq("/")
     end
@@ -119,8 +180,7 @@ RSpec.describe ChefConfig::PathHelper do
       expect(path_helper.cleanpath("/a///b/c/d/")).to eq("/a/b/c/d")
     end
 
-    describe "platform-specific #join behavior" do
-
+    context "platform-specific #join behavior" do
       it "joins components when some end with separators" do
         expected = path_helper.cleanpath("/foo/bar/baz")
         expect(path_helper.join("/foo/", "bar", "baz")).to eq(expected)
@@ -135,12 +195,19 @@ RSpec.describe ChefConfig::PathHelper do
         expected = path_helper.cleanpath("/foo/bar/baz")
         expect(path_helper.join("/foo", "bar", "baz")).to eq(expected)
       end
+    end
 
+    it "cleanpath changes backslashes into slashes and leaves slashes alone" do
+      expect(path_helper.cleanpath('/a/b\\c/d/', windows: false)).to eq("/a/b/c/d")
     end
 
+    # NOTE: this seems a bit weird to me, but this is just the way Pathname#cleanpath works
+    it "cleanpath does not remove leading double backslash" do
+      expect(path_helper.cleanpath('\\\\a/b\\c/d/')).to eq("//a/b/c/d")
+    end
   end
 
-  describe "validate_path" do
+  context "validate_path" do
     context "on windows" do
       before(:each) do
         # pass by default
@@ -171,7 +238,7 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
-  describe "windows_max_length_exceeded?" do
+  context "windows_max_length_exceeded?" do
     it "returns true if the path is too long (259 + NUL) for the API" do
       expect(path_helper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 6)).to be_truthy
     end
@@ -185,7 +252,7 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
-  describe "printable?" do
+  context "printable?" do
     it "returns true if the string contains no non-printable characters" do
       expect(path_helper.printable?("C:\\Program Files (x86)\\Microsoft Office\\Files.lst")).to be_truthy
     end
@@ -208,7 +275,7 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
-  describe "canonical_path" do
+  context "canonical_path" do
     context "on windows", :windows_only do
       it "returns an absolute path with backslashes instead of slashes" do
         expect(path_helper.canonical_path("\\\\?\\C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
@@ -230,25 +297,25 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
-  describe "paths_eql?" do
+  context "paths_eql?" do
     it "returns true if the paths are the same" do
-      allow(path_helper).to receive(:canonical_path).with("bandit").and_return("c:/bandit/bandit")
-      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
       expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_truthy
     end
 
     it "returns false if the paths are different" do
-      allow(path_helper).to receive(:canonical_path).with("bandit").and_return("c:/Bo/Bandit")
-      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("bandit", windows: ChefUtils.windows?).and_return("c:/Bo/Bandit")
+      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
       expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_falsey
     end
   end
 
-  describe "escape_glob" do
+  context "escape_glob" do
     it "escapes characters reserved by glob" do
       path = "C:\\this\\*path\\[needs]\\escaping?"
       escaped_path = "C:\\\\this\\\\\\*path\\\\\\[needs\\]\\\\escaping\\?"
-      expect(path_helper.escape_glob(path)).to eq(escaped_path)
+      expect(path_helper.escape_glob(path, windows: true)).to eq(escaped_path)
     end
 
     context "when given more than one argument" do
@@ -259,14 +326,12 @@ RSpec.describe ChefConfig::PathHelper do
                        else
                          "this/\\*path/\\[needs\\]/escaping\\?"
                        end
-        expect(path_helper).to receive(:join).with(*args).and_call_original
-        expect(path_helper).to receive(:cleanpath).and_call_original
         expect(path_helper.escape_glob(*args)).to eq(escaped_path)
       end
     end
   end
 
-  describe "escape_glob_dir" do
+  context "escape_glob_dir" do
     it "escapes characters reserved by glob without using backslashes for path separators" do
       path = "C:/this/*path/[needs]/escaping?"
       escaped_path = "C:/this/\\*path/\\[needs\\]/escaping\\?"
@@ -283,7 +348,7 @@ RSpec.describe ChefConfig::PathHelper do
     end
   end
 
-  describe "all_homes" do
+  context "all_homes" do
     before do
       stub_const("ENV", env)
       allow(ChefUtils).to receive(:windows?).and_return(is_windows)
diff --git a/lib/chef/knife/core/windows_bootstrap_context.rb b/lib/chef/knife/core/windows_bootstrap_context.rb
index 8469879022..7b4d517237 100644
--- a/lib/chef/knife/core/windows_bootstrap_context.rb
+++ b/lib/chef/knife/core/windows_bootstrap_context.rb
@@ -41,20 +41,6 @@ class Chef
           super(config, run_list, chef_config, secret)
         end
 
-        # This is a duplicate of ChefConfig::PathHelper.cleanpath, however
-        # this presumes Windows so we can avoid changing the method definitions
-        # across Chef, ChefConfig, and ChefUtils for the circumstance where
-        # the methods are being run for a system other than the one Ruby is
-        # executing on.
-        #
-        # We only need to cleanpath the paths that we are passing to cmd.exe,
-        # anything written to a configuration file or passed as an argument
-        # will be interpreted by ruby later and do the right thing.
-        def cleanpath(path)
-          path = Pathname.new(path).cleanpath.to_s
-          path.gsub(File::SEPARATOR, '\\')
-        end
-
         def validation_key
           if File.exist?(File.expand_path(chef_config[:validation_key]))
             IO.read(File.expand_path(chef_config[:validation_key]))
@@ -72,12 +58,21 @@ class Chef
         end
 
         def config_content
+          # The windows: true / windows: false in the block that follows is more than a bit weird.  The way to read this is that we need
+          # the e.g. var_chef_dir to be rendered for the windows value ("C:\chef"), but then we are rendering into a file to be read by
+          # ruby, so we don't actually care about forward-vs-backslashes and by rendering into unix we avoid having to deal with the
+          # double-backwhacking of everything.  So we expect to see:
+          #
+          # file_cache_path "C:/chef"
+          #
+          # Which is mildly odd, but should be entirely correct as far as ruby cares.
+          #
           client_rb = <<~CONFIG
             chef_server_url  "#{chef_config[:chef_server_url]}"
             validation_client_name "#{chef_config[:validation_client_name]}"
-            file_cache_path   "#{ChefConfig::Config.var_chef_dir(true)}/cache"
-            file_backup_path  "#{ChefConfig::Config.var_chef_dir(true)}/backup"
-            cache_options     ({:path => "#{ChefConfig::Config.etc_chef_dir(true)}/cache/checksums", :skip_expires => true})
+            file_cache_path   "#{ChefConfig::PathHelper.escapepath(ChefConfig::Config.var_chef_dir(windows: true))}\\\\cache"
+            file_backup_path  "#{ChefConfig::PathHelper.escapepath(ChefConfig::Config.var_chef_dir(windows: true))}\\\\backup"
+            cache_options     ({:path => "#{ChefConfig::PathHelper.escapepath(ChefConfig::Config.etc_chef_dir(windows: true))}\\\\cache\\\\checksums", :skip_expires => true})
           CONFIG
 
           unless chef_config[:chef_license].nil?
@@ -90,8 +85,8 @@ class Chef
             client_rb << "# Using default node name (fqdn)\n"
           end
 
-          if chef_config[:config_log_level]
-            client_rb << %Q{log_level :#{chef_config[:config_log_level]}\n}
+          if config[:config_log_level]
+            client_rb << %Q{log_level :#{config[:config_log_level]}\n}
           else
             client_rb << "log_level        :auto\n"
           end
@@ -140,11 +135,11 @@ class Chef
           end
 
           if config[:secret]
-            client_rb << %Q{encrypted_data_bag_secret "#{ChefConfig::Config.etc_chef_dir(true)}/encrypted_data_bag_secret"\n}
+            client_rb << %Q{encrypted_data_bag_secret "#{ChefConfig::Config.etc_chef_dir(windows: true)}/encrypted_data_bag_secret"\n}
           end
 
           unless trusted_certs_script.empty?
-            client_rb << %Q{trusted_certs_dir "#{ChefConfig::Config.etc_chef_dir(true)}/trusted_certs"\n}
+            client_rb << %Q{trusted_certs_dir "#{ChefConfig::Config.etc_chef_dir(windows: true)}/trusted_certs"\n}
           end
 
           if chef_config[:fips]
@@ -173,9 +168,14 @@ class Chef
         end
 
         def start_chef
+          c_opscode_dir = ChefConfig::PathHelper.cleanpath(ChefConfig::Config.c_opscode_dir, windows: true)
+          client_rb = clean_etc_chef_file("client.rb")
+          first_boot = clean_etc_chef_file("first-boot.json")
+
           bootstrap_environment_option = bootstrap_environment.nil? ? "" : " -E #{bootstrap_environment}"
-          start_chef = "SET \"PATH=%SystemRoot%\\system32;%SystemRoot%;%SystemRoot%\\System32\\Wbem;%SYSTEMROOT%\\System32\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;#{ChefConfig::Config.c_opscode_dir}\\bin;#{ChefConfig::Config.c_opscode_dir}\\embedded\\bin\;%PATH%\"\n"
-          start_chef << "#{Chef::Dist::CLIENT} -c #{ChefConfig::Config.etc_chef_dir(true)}/client.rb -j #{ChefConfig::Config.etc_chef_dir(true)}/first-boot.json#{bootstrap_environment_option}\n"
+
+          start_chef = "SET \"PATH=%SYSTEM32%;%SystemRoot%;%SYSTEM32%\\Wbem;%SYSTEM32%\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;#{c_opscode_dir}\\bin;#{c_opscode_dir}\\embedded\\bin\;%PATH%\"\n"
+          start_chef << "#{Chef::Dist::CLIENT} -c #{client_rb} -j #{first_boot}#{bootstrap_environment_option}\n"
         end
 
         def win_wget
@@ -275,8 +275,16 @@ class Chef
           install_command('"') + "\n" + fallback_install_task_command
         end
 
+        def clean_etc_chef_file(path)
+          ChefConfig::PathHelper.cleanpath(etc_chef_file(path), windows: true)
+        end
+
+        def etc_chef_file(path)
+          "#{bootstrap_directory}/#{path}"
+        end
+
         def bootstrap_directory
-          cleanpath(ChefConfig::Config.etc_chef_dir(true))
+          ChefConfig::Config.etc_chef_dir(windows: true)
         end
 
         def local_download_path
diff --git a/spec/unit/knife/core/windows_bootstrap_context_spec.rb b/spec/unit/knife/core/windows_bootstrap_context_spec.rb
index 6b041c5311..76b90c955e 100644
--- a/spec/unit/knife/core/windows_bootstrap_context_spec.rb
+++ b/spec/unit/knife/core/windows_bootstrap_context_spec.rb
@@ -148,24 +148,28 @@ describe Chef::Knife::Core::WindowsBootstrapContext do
 
   describe "#config_content" do
     before do
-      bootstrap_context.instance_variable_set(:@chef_config, Mash.new(config_log_level: :info,
-        config_log_location: STDOUT,
-        chef_server_url: "http://chef.example.com:4444",
-        validation_client_name: "chef-validator-testing",
-        file_cache_path: "c:/chef/cache",
-        file_backup_path: "c:/chef/backup",
-        cache_options: ({ path: "c:/chef/cache/checksums", skip_expires: true })))
+      bootstrap_context.instance_variable_set(
+        :@chef_config, Mash.new(
+          config_log_level: :info,
+          config_log_location: STDOUT,
+          chef_server_url: "http://chef.example.com:4444",
+          validation_client_name: "chef-validator-testing",
+          file_cache_path: "c:/chef/cache",
+          file_backup_path: "c:/chef/backup",
+          cache_options: ({ path: "c:/chef/cache/checksums", skip_expires: true })
+        )
+      )
     end
 
     it "generates the config file data" do
       expected = <<~EXPECTED
         echo.chef_server_url  "http://chef.example.com:4444"
         echo.validation_client_name "chef-validator-testing"
-        echo.file_cache_path   "C:/chef/cache"
-        echo.file_backup_path  "C:/chef/backup"
-        echo.cache_options     ^({:path =^> "C:/chef/cache/checksums", :skip_expires =^> true}^)
+        echo.file_cache_path   "C:\\\\chef\\\\cache"
+        echo.file_backup_path  "C:\\\\chef\\\\backup"
+        echo.cache_options     ^({:path =^> "C:\\\\chef\\\\cache\\\\checksums", :skip_expires =^> true}^)
         echo.# Using default node name ^(fqdn^)
-        echo.log_level :info
+        echo.log_level        :auto
         echo.log_location       STDOUT
       EXPECTED
       expect(bootstrap_context.config_content).to eq expected
@@ -183,7 +187,12 @@ describe Chef::Knife::Core::WindowsBootstrapContext do
 
   describe "#start_chef" do
     it "returns the expected string" do
-      expect(bootstrap_context.start_chef).to match(%r{SET \"PATH=%SystemRoot%\\system32;%SystemRoot%;%SystemRoot%\\System32\\Wbem;%SYSTEMROOT%\\System32\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;C:\/opscode\/chef\\bin;C:\/opscode\/chef\\embedded\\bin\;%PATH%\"\n})
+      expect(bootstrap_context.start_chef).to eq(
+        <<~EOH
+          SET "PATH=%SYSTEM32%;%SystemRoot%;%SYSTEM32%\\Wbem;%SYSTEM32%\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin;%PATH%"
+          chef-client -c C:\\chef\\client.rb -j C:\\chef\\first-boot.json
+        EOH
+      )
     end
   end