From c94ddc5ec8a823af52630b9baeccd9f95d13457e Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Wed, 24 Jun 2026 00:09:43 +0200 Subject: [PATCH] Fix to delete selected unit files safely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⓘ 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 --- systemd/save_unit.cgi | 8 +++++++- systemd/systemd-lib.pl | 22 +++++++++------------- systemd/t/run-tests.t | 29 +++++++++++++++++++++++------ 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/systemd/save_unit.cgi b/systemd/save_unit.cgi index a7d270fd6..b5ee8f0b7 100755 --- a/systemd/save_unit.cgi +++ b/systemd/save_unit.cgi @@ -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'}); } diff --git a/systemd/systemd-lib.pl b/systemd/systemd-lib.pl index 86c15ec1a..e1b7b4888 100644 --- a/systemd/systemd-lib.pl +++ b/systemd/systemd-lib.pl @@ -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() diff --git a/systemd/t/run-tests.t b/systemd/t/run-tests.t index c1d148734..ebcd140b8 100644 --- a/systemd/t/run-tests.t +++ b/systemd/t/run-tests.t @@ -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');