michael/webmin
1
0
Fork 0
mirror of https://github.com/webmin/webmin.git synced 2026-08-23 07:20:28 +01:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Fix logrotate vendor override write handling

Browse Source
This commit is contained in:
Ilia Ross
2026-08-11 20:24:38 +02:00
parent 07f0ddfb6d
commit 5969c33e06
3 changed files with 192 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
logrotate/logrotate-lib.pl
View File

@@ -435,8 +435,7 @@ return undef;
}
# save_directive(&parent, &old|name, &new, [indent])
# Updates one entry identified by either its name or parsed object. Vendor
# entries are transparently copied and rebound to their writable overrides.
# Updates one entry identified by either its name or parsed object
sub save_directive
{
my ($parent, $oldv, $newv, $indent) = @_;
@@ -446,10 +445,22 @@ my $new = !defined($newv) ? undef : ref($newv) ? $newv :
{ 'name' => $old ? $old->{'name'} : $oldv,
'value' => $newv };
# Find the vendor file behind this write. Existing directives use their own
# file, while additions use the parent section or the effective main config.
# Deleting an entry that is already absent is a true no-op. In particular,
# do not put a missing local main config into the writable line cache.
return if (!$old && !$new);
# Find the file behind this write. Existing directives use their own file,
# new sections may name a separate file, and other additions use the parent
# section or effective main config.
my $vendor_file;
my $write_file = $old ? $old->{'file'} : $parent->{'file'};
my $write_file = $parent->{'file'};
if ($old) {
$write_file = $old->{'file'};
}
elsif ($new && $new->{'file'} &&
!($parent->{'global'} && !$new->{'members'})) {
$write_file = $new->{'file'};
}
if ($write_file) {
my $shadowed_vendor = &get_vendor_config_file($write_file);
if (&is_vendor_main_config($write_file) ||
@@ -457,91 +468,24 @@ if ($write_file) {
$vendor_file = $write_file;
}
elsif ($shadowed_vendor &&
&same_file($write_file, $shadowed_vendor)) {
(!-f $write_file || -l $write_file ||
&same_file($write_file, $shadowed_vendor))) {
$vendor_file = $shadowed_vendor;
}
}
if (!$vendor_file && !$old && $new && !$new->{'members'} &&
$parent->{'global'} &&
if (!$vendor_file && !$old && $parent->{'global'} &&
&same_file($write_file, $parent->{'file'}) &&
&is_vendor_main_config(&get_main_config_file())) {
$vendor_file = &get_main_config_file();
}
# Materialize a writable override, then replace stale parsed references with
# their identical positions in the newly-parsed local configuration. Exact
# copying keeps both top-level and member indexes stable across this reparse.
if ($vendor_file) {
my $parent_global = $parent->{'global'};
my $parent_index = $parent->{'index'};
my $old_index = $old ? $old->{'index'} : undef;
my $old_line = $old ? $old->{'line'} : undef;
my $old_ref = ref($oldv) ? $oldv : undef;
my $local_file = &is_vendor_main_config($vendor_file) ?
&ensure_local_main_config() :
&ensure_local_config_override($vendor_file);
if (!$local_file || &same_file($local_file, $vendor_file)) {
&error(&text('save_evendorwrite',
"<tt>".&html_escape($vendor_file)."</tt>"));
}
my $fresh_root = &get_config_parent();
my $fresh_parent = $parent_global ? $fresh_root :
$fresh_root->{'members'}->[$parent_index];
# Copying changes which file owns the parsed objects, so callers must create
# and reload a local override before editing. Never write through a stale
# object that still points at the vendor tree.
&error(&text('save_evendorwrite',
"<tt>".&html_escape($vendor_file)."</tt>")) if ($vendor_file);
# Fail closed if concurrent configuration changes made the saved indexes
# unsafe to reuse. This must never fall through to the vendor line cache.
if (!$fresh_parent ||
(!$parent_global &&
(!&same_file($fresh_parent->{'file'}, $local_file) ||
!$fresh_parent->{'members'}))) {
&error(&text('save_evendorwrite',
"<tt>".&html_escape($vendor_file)."</tt>"));
}
# Preserve the caller's parent object identity so later saves, explicit
# flushes, and unlocks all refer to the new writable local file.
%$parent = %$fresh_parent;
if ($parent_global) {
$get_config_parent_cache = $parent;
$fresh_root = $parent;
}
else {
$fresh_root->{'members'}->[$parent_index] = $parent;
}
$conf = $parent->{'members'};
# A referenced old object may also contain the caller's pending edits, as
# when Virtualmin changes a section name and passes the same object twice.
# Keep its data and identity, but rebase all of its file ownership locally.
if ($old) {
my $fresh_old = $conf->[$old_index];
if (!$fresh_old || $fresh_old->{'line'} != $old_line ||
!&same_file($fresh_old->{'file'}, $local_file)) {
&error(&text('save_evendorwrite',
"<tt>".&html_escape($vendor_file)."</tt>"));
}
if ($old_ref) {
$old_ref->{'line'} = $fresh_old->{'line'};
$old_ref->{'eline'} = $fresh_old->{'eline'};
$old_ref->{'index'} = $fresh_old->{'index'};
my @objects = ($old_ref);
while (@objects) {
my $object = shift(@objects);
$object->{'file'} = $local_file
if ($object->{'file'} &&
&same_file($object->{'file'}, $vendor_file));
push(@objects, @{$object->{'members'}})
if ($object->{'members'});
}
$conf->[$old_index] = $old_ref;
$old = $old_ref;
}
else {
$old = $fresh_old;
}
}
}
my $lref = &read_file_lines($old ? $old->{'file'} : $parent->{'file'});
my $lref = &read_file_lines($write_file);
my @lines = &directive_lines($new, $indent) if ($new);
my $gparent = &get_config_parent();
if ($old && $new) {

35
logrotate/save_log.cgi
View File

@@ -5,6 +5,32 @@
require './logrotate-lib.pl';
&ReadParse();
# Resolve a new section's destination before loading any parsed objects. If
# its relative name already exists in the vendor tree, materialize the whole
# local override before appending the new section.
@files = split(/\s+/, $in{'file'});
if ($in{'new'} ||
(!$in{'global'} && !$in{'delete'} && !$in{'now'})) {
&error_setup($text{'save_err'});
foreach $f (@files) {
$f =~ /^\/\S+$/ || &error($text{'save_efile'});
}
@files || &error($text{'save_enofiles'});
$in{'file'} =~ s/\r//g;
}
if ($in{'new'}) {
$cfilename = $files[0] =~ /\/([^\/]+)$/ ? $1 : undef;
$new_config_file = &get_add_file($cfilename);
$vendor_file = &get_vendor_config_file($new_config_file);
if ($vendor_file) {
&ensure_local_config_override($vendor_file);
}
elsif (&same_file($new_config_file, $config{'logrotate_conf'}) &&
&is_vendor_main_config(&get_main_config_file())) {
&ensure_local_main_config();
}
}
# On systems with vendor configuration below /usr, create the writable local
# main config before changing global options. The parent object intentionally
# keeps this local path as its write destination.
@@ -20,16 +46,14 @@ if (!$in{'global'} && !$in{'new'} && !$in{'now'} &&
$parent = &get_config_parent();
$conf = $parent->{'members'};
}
@files = split(/\s+/, $in{'file'});
if ($in{'global'}) {
# Editing the global options
$log = $parent;
}
elsif ($in{'new'}) {
# Adding a new section
$cfilename = $files[0] =~ /\/([^\/]+)$/ ? $1 : undef;
$log = { 'members' => [ ],
'file' => &get_add_file($cfilename) };
'file' => $new_config_file };
$logfile = $in{'file'};
}
else {
@@ -66,11 +90,6 @@ else {
&lock_file($log->{'file'});
&error_setup($text{'save_err'});
if (!$in{'global'}) {
foreach $f (@files) {
$f =~ /^\/\S+$/ || &error($text{'save_efile'});
}
@files || &error($text{'save_enofiles'});
$in{'file'} =~ s/\r//g;
$log->{'name'} = [ split(/\n/, $in{'file'}) ];
}

176
logrotate/t/run-tests.t
View File

@@ -144,24 +144,96 @@ is_deeply(log_names($config), [ '/var/log/vendor-main.log' ],
is_deeply($files, [ $vendor_main_file ],
'file cache excludes external directories when scanning is disabled');
# The public writer performs copy-on-write itself, so API consumers do not
# need to know whether the effective main configuration came from /usr/etc.
my $vendor_parent = main::get_config_parent();
main::save_directive($vendor_parent, 'weekly', '');
main::flush_file_lines($local_main_file);
is(read_text($local_main_file), $vendor_main_text,
'direct main-config writes automatically create a local copy');
is(read_text($vendor_main_file), $vendor_main_text,
'automatic main-config copying leaves the vendor file unchanged');
is(main::find('weekly', $vendor_parent->{'members'})->{'file'},
$local_main_file,
'the caller parent is rebound to the writable main config');
# The low-level writer must fail closed if a caller skips copy-on-write.
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval {
main::save_directive(main::get_config_parent(), 'weekly', '');
};
like($@, qr/Refusing to modify vendor configuration/,
'direct writes to the vendor main configuration are rejected');
}
# Repeated preparation is harmless once the local main override exists.
# Deleting an already-absent option is a no-op and must not cache an empty
# local main file that a later unscoped flush could accidentally create.
main::save_directive(main::get_config_parent(),
'missing-vendor-option', undef);
main::flush_file_lines();
ok(!-e $local_main_file,
'missing global option deletion leaves the local main config absent');
# A new section with an explicit vendor destination must also fail closed.
my $vendor_target = "$vendor_add_dir/one";
my $vendor_target_text = read_text($vendor_target);
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval {
main::save_directive(main::get_config_parent(), undef,
{ 'file' => $vendor_target,
'name' => [ '/var/log/unsafe-vendor-write.log' ],
'members' => [ ] });
};
like($@, qr/Refusing to modify vendor configuration/,
'new sections cannot target a vendor drop-in directly');
}
is(read_text($vendor_target), $vendor_target_text,
'rejecting a new vendor section leaves its destination unchanged');
# A section without its own file would create an incomplete local main config.
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval {
main::save_directive(main::get_config_parent(), undef,
{ 'name' => [ '/var/log/unsafe-main-write.log' ],
'members' => [ ] });
};
like($@, qr/Refusing to modify vendor configuration/,
'new sections cannot replace the vendor main config implicitly');
}
ok(!-e $local_main_file,
'rejecting an implicit main write does not create a partial override');
# Adding a fresh local drop-in must not put the absent local main in the line
# cache, because the normal unscoped flush would then create it as an empty
# file and hide the complete vendor main configuration.
my $new_local_dropin = "$local_add_dir/new-local";
main::save_directive(main::get_config_parent(), undef,
{ 'file' => $new_local_dropin,
'name' => [ '/var/log/new-local.log' ],
'members' => [ { 'name' => 'weekly' } ] });
main::flush_file_lines();
ok(-f $new_local_dropin,
'new sections are written to their explicit local drop-in');
ok(!-e $local_main_file,
'adding a local drop-in does not create an empty local main config');
is(read_text($vendor_main_file), $vendor_main_text,
'adding a local drop-in leaves the vendor main config unchanged');
# A missing local file cannot safely replace a whole same-named vendor file.
my $missing_local_override = "$local_add_dir/one";
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval {
main::save_directive(main::get_config_parent(), undef,
{ 'file' => $missing_local_override,
'name' => [ '/var/log/incomplete-override.log' ],
'members' => [ ] });
};
like($@, qr/Refusing to modify vendor configuration/,
'new sections cannot create incomplete vendor overrides');
}
ok(!-e $missing_local_override,
'rejecting an incomplete override leaves its local path absent');
# Editing global options materializes an exact local copy before parsing.
$main::config{'scan_add_file'} = 1;
clear_config_cache();
is(main::ensure_local_main_config(), $local_main_file,
'an existing local main config remains the write target');
'editing the vendor main config creates a local main config');
is(read_text($local_main_file), $vendor_main_text,
'local main config starts as an exact vendor copy');
is(read_text($vendor_main_file), $vendor_main_text,
@@ -169,18 +241,40 @@ is(read_text($vendor_main_file), $vendor_main_text,
is(main::get_main_config_file(), $local_main_file,
'local main config takes precedence after it is created');
# A nested save through the public API copies the whole vendor drop-in to the
# same relative local path and immediately switches the caller's ownership.
# A new section may be appended after the same-named vendor file has been
# copied in full, which is the preflight performed by save_log.cgi.
is(main::ensure_local_config_override($vendor_target),
$missing_local_override,
'new-section preflight creates the complete local override');
my $prepared_parent = main::get_config_parent();
main::save_directive($prepared_parent, undef,
{ 'file' => $missing_local_override,
'name' => [ '/var/log/appended-local.log' ],
'members' => [ { 'name' => 'weekly' } ] });
main::flush_file_lines($missing_local_override);
like(read_text($missing_local_override), qr{/var/log/vendor-one\.log},
'prepared override retains the original vendor section');
like(read_text($missing_local_override), qr{/var/log/appended-local\.log},
'prepared override receives the new local section');
is(read_text($vendor_target), $vendor_target_text,
'appending locally leaves the same-named vendor file unchanged');
# Editing a vendor drop-in must also be prepared before parsed objects change.
my $vendor_dropin = "$vendor_add_dir/deep/vendor";
my $local_dropin = "$local_add_dir/deep/vendor";
($config, undef, $files) = main::get_config();
my ($deep_log) = grep { $_->{'members'} &&
$_->{'name'}->[0] eq '/var/log/deep-vendor.log' }
@$config;
main::save_directive($deep_log, 'monthly', '', "\t");
main::flush_file_lines($local_dropin);
is($deep_log->{'file'}, $local_dropin,
'nested vendor writes transparently rebind the caller locally');
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval { main::save_directive($deep_log, 'monthly', '', "\t"); };
like($@, qr/Refusing to modify vendor configuration/,
'direct writes to a vendor drop-in are rejected');
}
is(main::ensure_local_config_override($vendor_dropin), $local_dropin,
'editing a vendor drop-in creates its matching local override');
is(read_text($local_dropin), read_text($vendor_dropin),
'local drop-in starts as an exact copy of the whole vendor file');
is(main::get_local_override_file($vendor_dropin), $local_dropin,
@@ -194,23 +288,12 @@ is(main::get_vendor_config_file($local_dropin), $vendor_dropin,
@$config;
is($deep_log->{'file'}, $local_dropin,
'parser switches to the local copy after an override is created');
# Whole-section callers may mutate and pass the same parsed object as both
# old and new. Preserve those edits while changing its file ownership.
my $vendor_one = "$vendor_add_dir/one";
my $local_one = "$local_add_dir/one";
my ($one_log) = grep { $_->{'members'} &&
$_->{'name'}->[0] eq '/var/log/vendor-one.log' }
@$config;
push(@{$one_log->{'name'}}, '/var/log/vendor-one-extra.log');
main::save_directive(main::get_config_parent(), $one_log, $one_log);
main::flush_file_lines($local_one);
is($one_log->{'file'}, $local_one,
'whole-section saves preserve the caller object and move it locally');
like(read_text($local_one), qr{/var/log/vendor-one-extra\.log},
'whole-section saves preserve pending caller edits');
unlike(read_text($vendor_one), qr{/var/log/vendor-one-extra\.log},
'whole-section saves never change the vendor source');
main::save_directive($deep_log, 'monthly', undef, "\t");
main::flush_file_lines($local_dropin);
unlike(read_text($local_dropin), qr/^\s*monthly\s*$/m,
'prepared drop-in can be changed through its local override');
like(read_text($vendor_dropin), qr/^\s*monthly\s*$/m,
'changing the local override leaves the vendor drop-in unchanged');
# An empty local file must remain both effective and backup-visible because
# its existence is what prevents the vendor file from becoming active again.
@@ -255,6 +338,25 @@ local $main::config{'add_file'} = $edge_local_dir;
local $main::config{'vendor_add_file'} = $edge_vendor_dir;
is_deeply([ main::get_add_file_configs() ], [ "$edge_local_dir/linked" ],
'local existing path overrides the matching vendor file');
# Discovery follows the wrapper's existence rule, but editing must not follow
# a local symlink when it shadows a same-named vendor configuration.
{
no warnings qw(once redefine);
local *main::error = sub { die $_[0]; };
eval {
main::save_directive(
{ 'members' => [ ], 'file' => "$edge_dir/parent" },
undef,
{ 'file' => "$edge_local_dir/linked",
'name' => [ '/var/log/symlink-write.log' ],
'members' => [ ] });
};
like($@, qr/Refusing to modify vendor configuration/,
'local symlink overrides are rejected for editing');
}
is(read_text($edge_target), "local\n",
'rejecting a symlink override leaves its target unchanged');
}
# A regular local path must still be rejected when it is a hard link to its