diff --git a/lib/puppetserver/ca/utils/file_system.rb b/lib/puppetserver/ca/utils/file_system.rb index c8282da9..a48ffda3 100644 --- a/lib/puppetserver/ca/utils/file_system.rb +++ b/lib/puppetserver/ca/utils/file_system.rb @@ -53,14 +53,18 @@ def self.check_for_existing_files(one_or_more_paths) def self.forcibly_symlink(source, link_target) FileUtils.remove_dir(link_target, true) FileUtils.symlink(source, link_target) - # Ensure the symlink has the same ownership as the source. - # This requires using `FileUtils.chown` rather than `File.chown`, as - # the latter will update the ownership of the source rather than the - # link itself. - # Symlink permissions are ignored in favor of the source's permissions, - # so we don't have to change those. - source_info = File.stat(source) - FileUtils.chown(source_info.uid, source_info.gid, link_target) + + # Ensure the symlink has the same ownership as the source when running + # with privileges to change ownership. + if instance.running_as_root? + # This requires using `FileUtils.chown` rather than `File.chown`, as + # the latter will update the ownership of the source rather than the + # link itself. + # Symlink permissions are ignored in favor of the source's permissions, + # so we don't have to change those. + source_info = File.stat(source) + FileUtils.chown(source_info.uid, source_info.gid, link_target) + end end def initialize diff --git a/spec/puppetserver/ca/utils/file_system_spec.rb b/spec/puppetserver/ca/utils/file_system_spec.rb new file mode 100644 index 00000000..362f4846 --- /dev/null +++ b/spec/puppetserver/ca/utils/file_system_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' +require 'tmpdir' +require 'fileutils' + +require 'puppetserver/ca/utils/file_system' + +RSpec.describe Puppetserver::Ca::Utils::FileSystem do + describe '.forcibly_symlink' do + it 'creates a symlink without changing ownership when not running as root' do + Dir.mktmpdir do |tmpdir| + source = File.join(tmpdir, 'new_cadir') + link_target = File.join(tmpdir, 'old_cadir') + FileUtils.mkdir_p(source) + FileUtils.mkdir_p(link_target) + + allow(described_class.instance).to receive(:running_as_root?).and_return(false) + + expect(FileUtils).not_to receive(:chown) + + described_class.forcibly_symlink(source, link_target) + + expect(File.symlink?(link_target)).to be(true) + expect(File.readlink(link_target)).to eq(source) + end + end + + it 'changes symlink ownership to match the source when running as root' do + Dir.mktmpdir do |tmpdir| + source = File.join(tmpdir, 'new_cadir') + link_target = File.join(tmpdir, 'old_cadir') + FileUtils.mkdir_p(source) + FileUtils.mkdir_p(link_target) + + allow(described_class.instance).to receive(:running_as_root?).and_return(true) + allow(File).to receive(:stat).and_call_original + + source_info = instance_double(File::Stat, uid: 123, gid: 456) + allow(File).to receive(:stat).with(source).and_return(source_info) + + expect(FileUtils).to receive(:chown).with(123, 456, link_target) + + described_class.forcibly_symlink(source, link_target) + end + end + end +end