michael/webmin
1
0
Fork 0
mirror of https://github.com/webmin/webmin.git synced 2026-10-21 03:30:40 +01:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Fix to remove Cron env check when moving jobs

Browse Source 
c23b6b5b48 (commitcomment-204659847)
This commit is contained in:
Ilia Ross
2026-10-13 19:02:54 +02:00
parent b5ce991eb3
commit 01e454414b
4 changed files with 12 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
cron/cron-lib.pl
View File

@@ -1357,27 +1357,6 @@ elsif ($direction eq "bottom") {
return;
}
=head2 cron_move_affects_env(\@jobs, &job, &target)
Returns 1 if moving a Cron entry to the target crosses an environment
variable, or 0 otherwise.
=cut
sub cron_move_affects_env
{
my ($jobs, $job, $target) = @_;
return 0 if (!$job || !$target);
my ($first, $last) = sort { $a <=> $b }
($job->{'index'}, $target->{'index'});
for(my $i=$first; $i<=$last; $i++) {
my $entry = $jobs->[$i];
return 1 if ($entry &&
$entry->{'file'} eq $job->{'file'} &&
$entry->{'name'});
}
return 0;
}
=head2 swap_cron_jobs(&job1, &job2)
Swaps two Cron jobs, which must be in the same file, identified by their

47
cron/index.cgi
View File

@@ -193,44 +193,15 @@ foreach $u (@ulist) {
}
# Show mover buttons
local $prv = $i > 0 ? $plist[$i-1]->[0] : undef;
local $nxt = $i != $#plist ? $plist[$i+1]->[0] : undef;
if ($access{'move'}) {
# Hide moves that would cross a protected variable
local $upswap = &get_cron_move_target(\@jobs, $idx, "up");
local $downswap = &get_cron_move_target(\@jobs, $idx, "down");
local $topswap = &get_cron_move_target(\@jobs, $idx, "top");
local $bottomswap = &get_cron_move_target(\@jobs, $idx, "bottom");
local $movable = $job->{'name'} ||
$job->{'type'} == 0 ||
$job->{'type'} == 3;
local $canup = $upswap && $movable &&
$upswap->{'index'} != $idx &&
$upswap->{'file'} eq $job->{'file'} &&
($upswap->{'name'} || $upswap->{'type'} == 0 ||
$upswap->{'type'} == 3) &&
($access{'env'} ||
!&cron_move_affects_env(\@jobs, $job, $upswap));
local $candown = $downswap && $movable &&
$downswap->{'index'} != $idx &&
$downswap->{'file'} eq $job->{'file'} &&
($downswap->{'name'} || $downswap->{'type'} == 0 ||
$downswap->{'type'} == 3) &&
($access{'env'} ||
!&cron_move_affects_env(\@jobs, $job, $downswap));
local $cantop = $topswap && $movable &&
$topswap->{'index'} != $idx &&
$topswap->{'file'} eq $job->{'file'} &&
($topswap->{'name'} || $topswap->{'type'} == 0 ||
$topswap->{'type'} == 3) &&
($access{'env'} ||
!&cron_move_affects_env(\@jobs, $job, $topswap));
local $canbottom = $bottomswap && $movable &&
$bottomswap->{'index'} != $idx &&
$bottomswap->{'file'} eq $job->{'file'} &&
($bottomswap->{'name'} ||
$bottomswap->{'type'} == 0 ||
$bottomswap->{'type'} == 3) &&
($access{'env'} ||
!&cron_move_affects_env(\@jobs, $job, $bottomswap));
local $canup = $prv &&
$prv->{'file'} eq $job->{'file'} &&
($job->{'type'} == 0 || $job->{'type'} == 3);
local $candown = $nxt &&
$nxt->{'file'} eq $job->{'file'} &&
($job->{'type'} == 0 || $job->{'type'} == 3);
local $mover = "move.cgi?search=".
&urlize($in{'search'})."&idx=$idx";
push(@cols, &ui_up_down_arrows(
@@ -241,7 +212,7 @@ foreach $u (@ulist) {
&ui_up_down_arrows(
"$mover&top=1",
"$mover&bottom=1",
$cantop, $canbottom,
$canup, $candown,
"images/top.gif",
"images/bottom.gif",
));

3
cron/move.cgi
View File

@@ -34,9 +34,6 @@ $job->{'file'} eq $swap->{'file'} &&
($job->{'name'} || $job->{'type'} == 0 || $job->{'type'} == 3) &&
($swap->{'name'} || $swap->{'type'} == 0 || $swap->{'type'} == 3) ||
&error($text{'move_etype'});
# Crossing a variable changes which jobs it applies to
$access{'env'} || !&cron_move_affects_env(\@jobs, $job, $swap) ||
&error($text{'env_ecannot'});
&lock_file($job->{'file'});
&swap_cron_jobs($job, $swap);
&unlock_file($job->{'file'});

23
cron/t/run-tests.t
View File

@@ -67,17 +67,6 @@ is(get_cron_move_target(\@jobs, 0, "top"), undef,
is(get_cron_move_target(\@jobs, 5, "bottom"), undef,
"bottom has no target for the last entry in a file");
ok(cron_move_affects_env(\@jobs, $jobs[2], $jobs[3]),
"an adjacent variable affects a move");
ok(cron_move_affects_env(\@jobs, $jobs[5], $jobs[1]),
"an intermediate variable affects a top move");
ok(cron_move_affects_env(\@jobs, $jobs[1], $jobs[5]),
"an intermediate variable affects a bottom move");
ok(!cron_move_affects_env(\@jobs, $jobs[1], $jobs[2]),
"ordinary jobs can move without a variable");
ok(!cron_move_affects_env(\@jobs, $jobs[0], $jobs[6]),
"variables in another file do not affect a move");
my $cronfile = "$vardir/crontab";
my $crondir = "$vardir/crontabs";
make_path($crondir);
@@ -204,13 +193,7 @@ my %full_acl = (
write_crontab();
my $original = read_crontab_fixture();
seed_acl(%full_acl, 'env' => 0);
my $result = run_cgi("move.cgi", { 'idx' => 4, 'top' => 1 });
like($result->{'out'}, qr/not allowed to edit environment variables/i,
"top move cannot cross a protected variable");
is(read_crontab_fixture(), $original,
"denied top move leaves the crontab unchanged");
$result = run_cgi("delete_cron.cgi", { 'idx' => 2 });
my $result = run_cgi("delete_cron.cgi", { 'idx' => 2 });
like($result->{'out'}, qr/job does not exist/i,
"job delete endpoint rejects a variable row");
$result = run_cgi("save_cron.cgi", { 'idx' => 2 });
@@ -287,10 +270,10 @@ foreach my $case (
"rejected $desc leaves the crontab unchanged");
}
seed_acl(%full_acl);
seed_acl(%full_acl, 'env' => 0);
$result = run_cgi("move.cgi", { 'idx' => 4, 'top' => 1 });
like($result->{'out'}, qr/^Location:/m,
"a user with env permission can move a job across a variable");
"a job can move across a variable without env permission");
isnt(read_crontab_fixture(), $original,
"the permitted top move changes the crontab");