Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-7077

Puppet::Util::Windows::File.replace_file fails on UNC paths

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: PUP 4.7.0, PUP 4.8.1
    • Fix Version/s: PUP 4.10.0
    • Component/s: Windows
    • Labels:
    • Template:
    • Acceptance Criteria:
      • Files on non-NTFS volumes without ACL support can be managed with Puppet
      • Spec tests are added if necessary
    • Team:
      Agent
    • Story Points:
      2
    • Sprint:
      Agent 2017-04-05
    • Release Notes:
      Bug Fix
    • Release Notes Summary:
      Puppet can now manage files on UNC shares when the user has permission to create/modify the file, but the share permissions are not Full Control.
    • QA Risk Assessment:
      No Action

      Description

      A user report was received that Puppet::Util::Windows::File.replace_file was traced as a problem in file writes to UNC paths. The original code lives at https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/windows/file.rb#L51-L69

      Note that the current code calls ReplaceFile and specifies REPLACEFILE_WRITE_THROUGH (0x01) as the dwReplaceFlags despite MSDN documentation listing that value as unsupported.

      The original code dates back to 2012 and probably wasn't sufficiently tested in UNC path scenarios - https://github.com/puppetlabs/puppet/commit/73e302bbf5ab4a6c5513a39070e2c907542775ae#diff-a91b066f285a8fe7ccfa4162399d458fR11

      From the user report, given a manifest:

       file { 'puppettestfile':
          path    => '\\\\server\path\Puppet_Test_File.txt',
          ensure  => 'file',
          content => 'This is a puppet test file with content loaded from manifest',
      }
      

      Different errors propagated depending on different circumstances:

      When the file does not exist

      Error: ReplaceFile(//server/path/Puppet_Test_File.txt, //server/path/Puppet_Test_File.txt20170106-6416-1fqt5cr):  Access is denied.
      Error: /Stage[main]/Acurityconfig/File[puppettestfile]/ensure: change from absent to file failed: ReplaceFile(//sacuts16/TS16/TS16/Puppet_Test_File.txt, //sacuts16/TS16/TS16/Puppet_Test_File.txt20170106-6416-1fqt5cr):  Access is denied.
      

      NOTE: This results in a 0 byte file being created

      When the content of the file does not match

      Error: ReplaceFile(//server/path/Puppet_Test_File.txt, //server/path/Puppet_Test_File.txt20170106-11576-wsevbb):  Access is denied.
      Notice: /Stage[main]/ModuleName/File[puppettestfile]/content:
       
      Error: /Stage[main]/ModuleName/File[puppettestfile]/content: change from {md5}058755754b4ac17664d
      7a8eba995a8d1 to {md5}3568af3a5598f2998d60a5e92af55fd6 failed: ReplaceFile(//server/path/Puppet_Test_File.txt, //server/path/Puppet_Test_File.txt20170106-11576-wsevbb):  Access is denied
      

      The user was able to use an exec coupled with cmd.exe /c echo to successfully write files to the destination UNC path - so clearly the account Puppet running under has appropriate permission to do so.

      The user ultimately was able to work around the issue by passing REPLACEFILE_IGNORE_MERGE_ERRORS (0x02) as the value for dwReplaceFlags. It's possible the Puppet code can have that change made right away given the current value being passed is allegedly unsupported. Some additional tests may need to be written.

      The only concern here is whether or not the ACL can be set as desired. It may be necessary to add some documentation that managing files on UNC paths may not result in the expected mode being set.

      Callers of replace_file should also be audited to understand its current use in the Puppet codebase - https://github.com/puppetlabs/puppet/search?p=1&q=replace_file&utf8=%E2%9C%93

        Attachments

          Issue Links

            Activity

              jsd-sla-details-panel

                People

                • Assignee:
                  moses Moses Mendoza
                  Reporter:
                  ethan Ethan Brown
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  4 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: