Skip to content

Commit 2e5078d

Browse files
committed
(PUP-9997) Remove call to Dir.chdir when managing symlinks
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
1 parent cd0f3ce commit 2e5078d

File tree

1 file changed

+9
-11
lines changed

1 file changed

+9
-11
lines changed

lib/puppet/type/file/target.rb

+9-11
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,20 @@ def mklink
4444

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

47-
Dir.chdir(File.dirname(@resource[:path])) do
48-
Puppet::Util::SUIDManager.asuser(@resource.asuser) do
49-
mode = @resource.should(:mode)
50-
if mode
51-
Puppet::Util.withumask(0o00) do
52-
Puppet::FileSystem.symlink(target, @resource[:path])
53-
end
54-
else
47+
Puppet::Util::SUIDManager.asuser(@resource.asuser) do
48+
mode = @resource.should(:mode)
49+
if mode
50+
Puppet::Util.withumask(0o00) do
5551
Puppet::FileSystem.symlink(target, @resource[:path])
5652
end
53+
else
54+
Puppet::FileSystem.symlink(target, @resource[:path])
5755
end
56+
end
5857

59-
@resource.send(:property_fix)
58+
@resource.send(:property_fix)
6059

61-
:link_created
62-
end
60+
:link_created
6361
end
6462

6563
def insync?(currentvalue)

0 commit comments

Comments
 (0)