[Fix] Cron job tooling

app/Actions/CronJob/SyncCronJobs.php

@@ -99,7 +99,7 @@ private function syncUserCronJobs(Server $server, string $user): void
 
             // Check if this matches any Vito-managed cronjob (including site-level ones)
             $matchingCronJob = $vitoCronJobs->first(function ($cronJob) use ($frequency, $command) {
-                return $this->normalizeFrequency($cronJob->frequency) === $frequency && $this->normalizeCommand($cronJob->command) === $command;
+                return $this->normalizeFrequency($cronJob->frequency) === $frequency && $this->matchesCommand($cronJob->command, $command);
             });
 
             if ($matchingCronJob) {
@@ -126,7 +126,7 @@ private function syncUserCronJobs(Server $server, string $user): void
         // Create new cronjobs for manually created ones (not in Vito)
         foreach ($serverCronJobs as $cronJobData) {
             $isVitoManaged = $vitoCronJobs->contains(function ($cronJob) use ($cronJobData) {
-                return $this->normalizeFrequency($cronJob->frequency) === $cronJobData['frequency'] && $this->normalizeCommand($cronJob->command) === $cronJobData['command'];
+                return $this->normalizeFrequency($cronJob->frequency) === $cronJobData['frequency'] && $this->matchesCommand($cronJob->command, $cronJobData['command']);
             });
 
             if (! $isVitoManaged) {
@@ -154,6 +154,18 @@ private function normalizeCommand(string $command): string
         return preg_replace('/\s+/', ' ', trim($command));
     }
 
+    private function matchesCommand(string $cronJobCommand, string $serverCommand): bool
+    {
+        $normalizedCronJob = $this->normalizeCommand($cronJobCommand);
+        if ($normalizedCronJob === $serverCommand) {
+            return true;
+        }
+
+        $unwrapped = CronJob::unwrapCommand($serverCommand);
+
+        return $unwrapped !== null && $this->normalizeCommand($unwrapped) === $normalizedCronJob;
+    }
+
     /**
      * @throws SSHError
      */

app/Http/Controllers/CronJobController.php

@@ -120,6 +120,7 @@ public function site(Server $server, Site $site): Response
             ),
             'site' => $site,
             'sites' => $server->sites()->select('id', 'domain')->get(),
+            'ssh_users' => $site->getSshUsers(),
         ]);
     }
 

app/Models/CronJob.php

@@ -84,7 +84,8 @@ public static function crontab(Server $server, string $user): string
             ->get();
         /** @var CronJob $cronJob */
         foreach ($cronJobs as $key => $cronJob) {
-            $data .= $cronJob->frequency.' '.$cronJob->command;
+            $command = $user === 'root' ? $cronJob->command : self::wrapCommand($cronJob->command);
+            $data .= $cronJob->frequency.' '.$command;
             if ($key != count($cronJobs) - 1) {
                 $data .= "\n";
             }
@@ -93,6 +94,26 @@ public static function crontab(Server $server, string $user): string
         return $data;
     }
 
+    public static function wrapCommand(string $command): string
+    {
+        return 'bash -lc '.escapeshellarg($command);
+    }
+
+    public static function unwrapCommand(string $command): ?string
+    {
+        $prefix = 'bash -lc ';
+        if (! str_starts_with($command, $prefix)) {
+            return null;
+        }
+
+        $argument = trim(substr($command, strlen($prefix)));
+        if (strlen($argument) < 2 || ! str_starts_with($argument, "'") || ! str_ends_with($argument, "'")) {
+            return null;
+        }
+
+        return str_replace("'\\''", "'", substr($argument, 1, -1));
+    }
+
     public function frequencyLabel(): string
     {
         $labels = [

app/SSH/OS/Cron.php

@@ -16,7 +16,7 @@ public function update(string $user, string $cron): void
     {
         $this->server->ssh()->exec(
             view('ssh.cron.update', [
-                'cron' => $cron,
+                'cron' => str_replace("'", "'\\''", $cron),
                 'user' => $user,
             ]),
             'update-cron'

resources/js/pages/cronjobs/components/form.tsx

@@ -8,7 +8,7 @@ import {
   DialogTitle,
   DialogTrigger,
 } from '@/components/ui/dialog';
-import React, { FormEvent, ReactNode, useState } from 'react';
+import React, { FormEvent, ReactNode, useMemo, useState } from 'react';
 import { Form, FormField, FormFields } from '@/components/ui/form';
 import { Button } from '@/components/ui/button';
 import { useForm, usePage } from '@inertiajs/react';
@@ -34,9 +34,17 @@ export default function CronJobForm({
   cronJob?: CronJob;
   children: ReactNode;
 }) {
-  const page = usePage<SharedData & { server: Server; sites?: Array<{ id: number; domain: string }> }>();
+  const page = usePage<SharedData & { server: Server; sites?: Array<{ id: number; domain: string }>; ssh_users?: string[] }>();
   const configs = useConfigs()!;
   const [open, setOpen] = useState(false);
+
+  const sshUsers = useMemo(() => {
+    const base = site ? (page.props.ssh_users ?? []) : page.props.server.ssh_users;
+    if (cronJob?.user && !base.includes(cronJob.user)) {
+      return [...base, cronJob.user];
+    }
+    return base;
+  }, [site, page.props.ssh_users, page.props.server.ssh_users, cronJob?.user]);
   const form = useForm<{
     name: string;
     command: string;
@@ -47,7 +55,7 @@ export default function CronJobForm({
   }>({
     name: cronJob?.name || '',
     command: cronJob?.command || '',
-    user: cronJob?.user || '',
+    user: cronJob?.user || (site?.isolated_user_id ? site.user : ''),
     frequency: cronJob ? (configs.cronjob_intervals[cronJob.frequency] ? cronJob.frequency : 'custom') : '',
     custom: cronJob?.frequency || '',
     site_id: cronJob?.site_id?.toString() || '0',
@@ -175,7 +183,7 @@ export default function CronJobForm({
                 </SelectTrigger>
                 <SelectContent>
                   <SelectGroup>
-                    {page.props.server.ssh_users.map((user) => (
+                    {sshUsers.map((user) => (
                       <SelectItem key={`user-${user}`} value={user}>
                         {user}
                       </SelectItem>

tests/Feature/CronjobTest.php

@@ -2,6 +2,7 @@
 
 namespace Tests\Feature;
 
+use App\Actions\CronJob\SyncCronJobs;
 use App\Enums\CronjobStatus;
 use App\Facades\SSH;
 use App\Models\CronJob;
@@ -76,7 +77,7 @@ public function test_create_cronjob(): void
             'name' => 'My Cronjob',
         ]);
 
-        SSH::assertExecutedContains("echo '* * * * * ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * * * bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 
@@ -100,7 +101,7 @@ public function test_create_cronjob_for_isolated_user(): void
             'user' => 'example',
         ]);
 
-        SSH::assertExecutedContains("echo '* * * * * ls -la' | sudo -u example crontab -");
+        SSH::assertExecutedContains("echo '* * * * * bash -lc '\\''ls -la'\\''' | sudo -u example crontab -");
         SSH::assertExecutedContains('sudo -u example crontab -l');
     }
 
@@ -166,10 +167,53 @@ public function test_create_custom_cronjob(): void
             'status' => CronjobStatus::READY,
         ]);
 
-        SSH::assertExecutedContains("echo '* * * 1 1 ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * 1 1 bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 
+    public function test_cronjob_is_wrapped_in_login_shell_and_survives_sync(): void
+    {
+        SSH::fake();
+        $this->actingAs($this->user);
+
+        /** @var CronJob $cronjob */
+        $cronjob = CronJob::factory()->create([
+            'server_id' => $this->server->id,
+            'user' => 'vito',
+            'command' => "echo 'hi'",
+            'frequency' => '* * * * *',
+            'status' => CronjobStatus::READY,
+            'site_id' => null,
+        ]);
+
+        $crontab = CronJob::crontab($this->server, 'vito');
+        $this->assertSame("* * * * * bash -lc 'echo '\\''hi'\\'''", $crontab);
+        $this->assertSame("echo 'hi'", CronJob::unwrapCommand("bash -lc 'echo '\\''hi'\\'''"));
+
+        SSH::fake($crontab);
+        app(SyncCronJobs::class)->sync($this->server);
+
+        $cronjob->refresh();
+        $this->assertSame(CronjobStatus::READY, $cronjob->status);
+        $this->assertSame(1, CronJob::query()->where('user', 'vito')->where('command', "echo 'hi'")->count());
+    }
+
+    public function test_root_cronjob_is_not_wrapped(): void
+    {
+        $this->actingAs($this->user);
+
+        CronJob::factory()->create([
+            'server_id' => $this->server->id,
+            'user' => 'root',
+            'command' => '/usr/bin/backup.sh',
+            'frequency' => '0 2 * * *',
+            'status' => CronjobStatus::READY,
+            'site_id' => null,
+        ]);
+
+        $this->assertSame('0 2 * * * /usr/bin/backup.sh', CronJob::crontab($this->server, 'root'));
+    }
+
     public function test_enable_cronjob(): void
     {
         SSH::fake();
@@ -195,7 +239,7 @@ public function test_enable_cronjob(): void
 
         $this->assertEquals(CronjobStatus::READY, $cronjob->status);
 
-        SSH::assertExecutedContains("echo '* * * 1 1 ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * 1 1 bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 

tests/Feature/SiteCronjobTest.php

@@ -82,7 +82,7 @@ public function test_create_site_cronjob(): void
             'status' => CronjobStatus::READY,
         ]);
 
-        SSH::assertExecutedContains("echo '* * * * * ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * * * bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 
@@ -110,7 +110,7 @@ public function test_create_site_cronjob_for_isolated_user(): void
             'user' => 'example',
         ]);
 
-        SSH::assertExecutedContains("echo '* * * * * ls -la' | sudo -u example crontab -");
+        SSH::assertExecutedContains("echo '* * * * * bash -lc '\\''ls -la'\\''' | sudo -u example crontab -");
         SSH::assertExecutedContains('sudo -u example crontab -l');
     }
 
@@ -162,7 +162,7 @@ public function test_create_custom_site_cronjob(): void
             'status' => CronjobStatus::READY,
         ]);
 
-        SSH::assertExecutedContains("echo '* * * 1 1 ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * 1 1 bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 
@@ -193,7 +193,7 @@ public function test_enable_site_cronjob(): void
 
         $this->assertEquals(CronjobStatus::READY, $cronjob->status);
 
-        SSH::assertExecutedContains("echo '* * * 1 1 ls -la' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '* * * 1 1 bash -lc '\\''ls -la'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 
@@ -260,7 +260,7 @@ public function test_update_site_cronjob(): void
         $this->assertEquals('php artisan schedule:run', $cronjob->command);
         $this->assertEquals('0 * * * *', $cronjob->frequency);
 
-        SSH::assertExecutedContains("echo '0 * * * * php artisan schedule:run' | sudo -u vito crontab -");
+        SSH::assertExecutedContains("echo '0 * * * * bash -lc '\\''php artisan schedule:run'\\''' | sudo -u vito crontab -");
         SSH::assertExecutedContains('sudo -u vito crontab -l');
     }
 }