Fix to delete selected unit files safely

ⓘ Make system unit deletion operate on the selected unit file, preserve vendor deletion policy, and reject invalid/stale delete targets before any stop or disable side effects.

https://github.com/webmin/webmin/actions/runs/28058126464/job/83065504870
This commit is contained in:
Ilia Ross
2026-06-24 00:09:43 +02:00
parent 0290ec16a5
commit c94ddc5ec8
3 changed files with 39 additions and 20 deletions

View File

@@ -205,10 +205,16 @@ elsif ($in{'delete'}) {
{ 'user' => $unituser });
}
else {
my $delete_file = $u->{'file'};
system_unit_file_deletable($u) ||
error($text{'systemd_elocaldelete'});
(-e $delete_file || -l $delete_file) ||
error($text{'systemd_egone'});
# Stop and disable are best-effort, but deletion must be reported.
disable_unit($in{'name'});
stop_unit($in{'name'});
my ($ok, $out) = delete_system_unit($in{'name'});
my ($ok, $out) = delete_system_unit($in{'name'}, $delete_file);
$ok || error($out);
webmin_log("delete", "systemd", $in{'name'});
}

View File

@@ -3465,25 +3465,21 @@ delete_user_unit_file($user, $file) ||
return reload_user_manager($user);
}
=head2 delete_system_unit(name)
=head2 delete_system_unit(name, file)
Delete a permitted systemd unit file.
Delete the selected permitted systemd unit file.
=cut
sub delete_system_unit
{
my ($name) = @_;
my ($name, $file) = @_;
return (0, $text{'systemd_ename'}) if (!valid_unit_name($name));
foreach my $root (get_system_unit_file_root_candidates()) {
my $file = $root."/".$name;
next if (!-e $file && !-l $file);
return (0, $text{'systemd_elocaldelete'})
if (!system_unit_root_delete_allowed($root));
unlink_logged($file);
reload_manager();
return (1, "");
}
return (0, $text{'systemd_egone'});
return (0, $text{'systemd_elocaldelete'})
if (!system_unit_file_delete_allowed($file, $name));
return (0, $text{'systemd_egone'}) if (!-e $file && !-l $file);
unlink_logged($file);
reload_manager();
return (1, "");
}
=head2 get_unit_types()

View File

@@ -991,20 +991,21 @@ like(get_unit_root(), qr{^/(etc|usr/lib|lib)/systemd/system$},
write_test_file("$root/demo", "bare");
write_test_file("$root/demo.service", "typed");
($ok) = delete_system_unit('demo');
($ok) = delete_system_unit('demo', "$root/demo");
ok(!$ok, 'delete_system_unit rejects bare service name');
ok(-e "$root/demo", 'delete_system_unit leaves suffix-less file alone');
ok(-e "$root/demo.service", 'delete_system_unit leaves typed file after bare rejection');
($ok) = delete_system_unit('demo.service');
($ok) = delete_system_unit('demo.service', "$root/demo.service");
ok($ok, 'delete_system_unit accepts typed service name');
ok(!-e "$root/demo.service", 'delete_system_unit removes service file');
my $out;
($ok, $out) = delete_system_unit('demo.service');
($ok, $out) = delete_system_unit('demo.service', "$root/demo.service");
ok(!$ok, 'delete_system_unit rejects already-missing unit');
is($out, $text{'systemd_egone'},
'delete_system_unit reports stale missing unit');
write_test_file("$vendor_root/vendor.service", "packaged");
($ok, $out) = delete_system_unit('vendor.service');
($ok, $out) = delete_system_unit('vendor.service',
"$vendor_root/vendor.service");
ok(!$ok, 'delete_system_unit rejects packaged system unit files');
is($out, $text{'systemd_elocaldelete'},
'delete_system_unit reports local-only delete policy');
@@ -1012,11 +1013,24 @@ like(get_unit_root(), qr{^/(etc|usr/lib|lib)/systemd/system$},
'delete_system_unit leaves packaged system unit file alone');
{
local $config{'delete_vendor_units'} = 1;
($ok, $out) = delete_system_unit('vendor.service');
($ok, $out) = delete_system_unit('vendor.service',
"$vendor_root/vendor.service");
ok($ok, 'delete_system_unit can delete packaged unit files when configured');
ok(!-e "$vendor_root/vendor.service",
'delete_system_unit removes configured packaged unit file');
}
write_test_file("$root/shadow.service", "local");
write_test_file("$vendor_root/shadow.service", "packaged");
{
local $config{'delete_vendor_units'} = 1;
($ok, $out) = delete_system_unit('shadow.service',
"$vendor_root/shadow.service");
ok($ok, 'delete_system_unit deletes the selected packaged unit file');
ok(-e "$root/shadow.service",
'delete_system_unit leaves same-named local unit file alone');
ok(!-e "$vendor_root/shadow.service",
'delete_system_unit removes selected packaged unit file');
}
}
{
@@ -2286,7 +2300,10 @@ like($save_source, qr/delete_user_dropin_file/,
like($save_source, qr/delete_system_dropin_file/,
'save page can delete system drop-in overrides');
like($save_source,
qr/my \(\$ok, \$out\) = delete_system_unit\(\$in\{'name'\}\);\s*\$ok \|\| error\(\$out\);/s,
qr/system_unit_file_deletable\(\$u\).*?disable_unit\(\$in\{'name'\}\)/s,
'save page validates system unit delete target before side effects');
like($save_source,
qr/my \(\$ok, \$out\) = delete_system_unit\(\$in\{'name'\}, \$delete_file\);\s*\$ok \|\| error\(\$out\);/s,
'save page reports failed system unit deletes');
like($save_source, qr/stock_unit/,
'save page can return from override edits without saving');