Skip to content

Commit 474eccd

Browse files
committed
fix: skip chown when not running as root in rootless containers
Guard all three FileUtils.chown call sites in file_system.rb with the existing running_as_root? check so that openvoxserver-ca no longer crashes when running inside rootless containers (e.g. podman rootless, OpenShift with arbitrary UIDs) where the process lacks CAP_CHOWN. Affected methods: forcibly_symlink, write_file, ensure_dir. In these environments file ownership is typically managed through SGID bits and g=u permission patterns instead of explicit chown calls. Inspired by the approach in #30. Signed-off-by: Simon Lauger <simon@lauger.de>
1 parent bada32e commit 474eccd

File tree

2 files changed

+117
-10
lines changed

2 files changed

+117
-10
lines changed

lib/puppetserver/ca/utils/file_system.rb

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,17 @@ def self.check_for_existing_files(one_or_more_paths)
5353
def self.forcibly_symlink(source, link_target)
5454
FileUtils.remove_dir(link_target, true)
5555
FileUtils.symlink(source, link_target)
56-
# Ensure the symlink has the same ownership as the source.
57-
# This requires using `FileUtils.chown` rather than `File.chown`, as
58-
# the latter will update the ownership of the source rather than the
59-
# link itself.
60-
# Symlink permissions are ignored in favor of the source's permissions,
61-
# so we don't have to change those.
62-
source_info = File.stat(source)
63-
FileUtils.chown(source_info.uid, source_info.gid, link_target)
56+
# Ensure the symlink has the same ownership as the source when running
57+
# with privileges to change ownership.
58+
if instance.running_as_root?
59+
# This requires using `FileUtils.chown` rather than `File.chown`, as
60+
# the latter will update the ownership of the source rather than the
61+
# link itself.
62+
# Symlink permissions are ignored in favor of the source's permissions,
63+
# so we don't have to change those.
64+
source_info = File.stat(source)
65+
FileUtils.chown(source_info.uid, source_info.gid, link_target)
66+
end
6467
end
6568

6669
def initialize
@@ -93,14 +96,14 @@ def write_file(path, one_or_more_objects, mode)
9396
f.puts object.to_s
9497
end
9598
end
96-
FileUtils.chown(@user, @group, path)
99+
FileUtils.chown(@user, @group, path) if running_as_root?
97100
end
98101

99102
# Warning: directory mode should be specified in DIR_MODES above
100103
def ensure_dir(directory)
101104
if !File.exist?(directory)
102105
FileUtils.mkdir_p(directory, mode: DIR_MODES[directory])
103-
FileUtils.chown(@user, @group, directory)
106+
FileUtils.chown(@user, @group, directory) if running_as_root?
104107
end
105108
end
106109
end
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
require 'spec_helper'
2+
require 'tmpdir'
3+
require 'fileutils'
4+
5+
require 'puppetserver/ca/utils/file_system'
6+
7+
RSpec.describe Puppetserver::Ca::Utils::FileSystem do
8+
describe '.forcibly_symlink' do
9+
it 'creates a symlink without calling chown when not running as root' do
10+
Dir.mktmpdir do |tmpdir|
11+
source = File.join(tmpdir, 'new_cadir')
12+
link_target = File.join(tmpdir, 'old_cadir')
13+
FileUtils.mkdir_p(source)
14+
FileUtils.mkdir_p(link_target)
15+
16+
allow(described_class.instance).to receive(:running_as_root?).and_return(false)
17+
expect(FileUtils).not_to receive(:chown)
18+
19+
described_class.forcibly_symlink(source, link_target)
20+
21+
expect(File.symlink?(link_target)).to be(true)
22+
expect(File.readlink(link_target)).to eq(source)
23+
end
24+
end
25+
26+
it 'creates a symlink and calls chown when running as root' do
27+
Dir.mktmpdir do |tmpdir|
28+
source = File.join(tmpdir, 'new_cadir')
29+
link_target = File.join(tmpdir, 'old_cadir')
30+
FileUtils.mkdir_p(source)
31+
FileUtils.mkdir_p(link_target)
32+
33+
allow(described_class.instance).to receive(:running_as_root?).and_return(true)
34+
allow(File).to receive(:stat).and_call_original
35+
36+
source_info = instance_double(File::Stat, uid: 123, gid: 456)
37+
allow(File).to receive(:stat).with(source).and_return(source_info)
38+
expect(FileUtils).to receive(:chown).with(123, 456, link_target)
39+
40+
described_class.forcibly_symlink(source, link_target)
41+
42+
expect(File.symlink?(link_target)).to be(true)
43+
end
44+
end
45+
end
46+
47+
describe '#write_file' do
48+
it 'writes the file without calling chown when not running as root' do
49+
Dir.mktmpdir do |tmpdir|
50+
path = File.join(tmpdir, 'test.pem')
51+
52+
instance = described_class.instance
53+
allow(instance).to receive(:running_as_root?).and_return(false)
54+
expect(FileUtils).not_to receive(:chown)
55+
56+
instance.write_file(path, 'test content', 0644)
57+
58+
expect(File.read(path)).to include('test content')
59+
end
60+
end
61+
62+
it 'writes the file and calls chown when running as root' do
63+
Dir.mktmpdir do |tmpdir|
64+
path = File.join(tmpdir, 'test.pem')
65+
66+
instance = described_class.instance
67+
allow(instance).to receive(:running_as_root?).and_return(true)
68+
expect(FileUtils).to receive(:chown)
69+
70+
instance.write_file(path, 'test content', 0644)
71+
end
72+
end
73+
end
74+
75+
describe '#ensure_dir' do
76+
it 'creates the directory without calling chown when not running as root' do
77+
Dir.mktmpdir do |tmpdir|
78+
directory = File.join(tmpdir, 'newdir')
79+
80+
instance = described_class.instance
81+
allow(instance).to receive(:running_as_root?).and_return(false)
82+
expect(FileUtils).not_to receive(:chown)
83+
84+
instance.ensure_dir(directory)
85+
86+
expect(File.directory?(directory)).to be(true)
87+
end
88+
end
89+
90+
it 'creates the directory and calls chown when running as root' do
91+
Dir.mktmpdir do |tmpdir|
92+
directory = File.join(tmpdir, 'newdir')
93+
94+
instance = described_class.instance
95+
allow(instance).to receive(:running_as_root?).and_return(true)
96+
expect(FileUtils).to receive(:chown)
97+
98+
instance.ensure_dir(directory)
99+
100+
expect(File.directory?(directory)).to be(true)
101+
end
102+
end
103+
end
104+
end

0 commit comments

Comments
 (0)