Skip to content

Commit

Permalink
(PUP-9997) Remove call to Dir.chdir when managing symlinks
Browse files Browse the repository at this point in the history
Symlinks can refer to their target using a relative or absolute path. If a
relative path is used, then it's assumed to be relative to the symlink's parent
directory. For example, /tmp/link refers to /tmp/target using a relative path:

    $ ls -l /tmp/link
    lrwxrwxrwx 1 josh 6 Jun 27 23:26 /tmp/link -> target

In commit 02f91fc, symlinks were merged into the file (aka pfile) type. The
`ensure` property was modified to validate whether the target existed or was a
directory[1] In order for relative symlinks to be validated, it was necessary
to call `Dir.chdir`.

Later the call to `Dir.chdir` was moved to `target.rb`[2], and then the
`FileTest.exists?(target)` check was deleted[3].

This commit removes the call to `Dir.chdir`. This means we no longer change our
cwd prior to dropping privileges and creating the symlink. This should be ok
because none of the code within the Dir.chdir block is sensitive to cwd. Also
when we reach this block of code, we're committed to creating the symlink given
an absolute `@resource[:path]`, so we don't need to be concerned about TOCTOU
race conditions.

[1] https://github.com/puppetlabs/puppet/blob/02f91fcd55aefe63a11a29c5608c0738867b8130/lib/puppet/type/pfile/ensure.rb#L76-L86
[2] https://github.com/puppetlabs/puppet/blob/2cd67ad843409a49747748a1278e5ef788dd97bd/lib/puppet/type/pfile/target.rb#L41-L45
[3] 662fcaf#diff-03f0464c0f42c4b979f6150ec2f7e8043ec964fe4d401cd0cd57a82f43a856ba
  • Loading branch information
joshcooper committed Jun 28, 2024
1 parent cd0f3ce commit 66c4a25
Showing 1 changed file with 9 additions and 11 deletions.
20 changes: 9 additions & 11 deletions lib/puppet/type/file/target.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,20 @@ def mklink

raise Puppet::Error, "Could not remove existing file" if Puppet::FileSystem.exist?(@resource[:path])

Dir.chdir(File.dirname(@resource[:path])) do
Puppet::Util::SUIDManager.asuser(@resource.asuser) do
mode = @resource.should(:mode)
if mode
Puppet::Util.withumask(0o00) do
Puppet::FileSystem.symlink(target, @resource[:path])
end
else
Puppet::Util::SUIDManager.asuser(@resource.asuser) do
mode = @resource.should(:mode)
if mode
Puppet::Util.withumask(0o00) do
Puppet::FileSystem.symlink(target, @resource[:path])
end
else
Puppet::FileSystem.symlink(target, @resource[:path])
end
end

@resource.send(:property_fix)
@resource.send(:property_fix)

:link_created
end
:link_created
end

def insync?(currentvalue)
Expand Down

0 comments on commit 66c4a25

Please sign in to comment.