refactor: simplify remote process chain and harden ActivityMonitor (#9189)

CLAUDE.md

@@ -43,7 +43,7 @@ ### Backend Structure (app/)
 - **Models/** — Eloquent models extending `BaseModel` which provides auto-CUID2 UUID generation. Key models: `Server`, `Application`, `Service`, `Project`, `Environment`, `Team`, plus standalone database models (`StandalonePostgresql`, `StandaloneMysql`, etc.). Common traits: `HasConfiguration`, `HasMetrics`, `HasSafeStringAttribute`, `ClearsGlobalSearchCache`.
 - **Services/** — Business logic services (ConfigurationGenerator, DockerImageParser, ContainerStatusAggregator, HetznerService, etc.). Use Services for complex orchestration; use Actions for single-purpose domain operations.
 - **Helpers/** — Global helpers loaded via `bootstrap/includeHelpers.php` from `bootstrap/helpers/` — organized into `shared.php`, `constants.php`, `versions.php`, `subscriptions.php`, `domains.php`, `docker.php`, `services.php`, `github.php`, `proxy.php`, `notifications.php`.
-- **Data/** — Spatie Laravel Data DTOs (e.g., `CoolifyTaskArgs`, `ServerMetadata`).
+- **Data/** — Spatie Laravel Data DTOs (e.g., `ServerMetadata`).
 - **Enums/** — PHP enums (TitleCase keys). Key enums: `ProcessStatus`, `Role` (MEMBER/ADMIN/OWNER with rank comparison), `BuildPackTypes`, `ProxyTypes`, `ContainerStatusTypes`.
 - **Rules/** — Custom validation rules (`ValidGitRepositoryUrl`, `ValidServerIp`, `ValidHostname`, `DockerImageFormat`, etc.).
 

app/Actions/CoolifyTask/PrepareCoolifyTask.php

@@ -1,54 +0,0 @@
-<?php
-
-namespace App\Actions\CoolifyTask;
-
-use App\Data\CoolifyTaskArgs;
-use App\Jobs\CoolifyTask;
-use Spatie\Activitylog\Models\Activity;
-
-/**
- * The initial step to run a `CoolifyTask`: a remote SSH process
- * with monitoring/tracking/trace feature. Such thing is made
- * possible using an Activity model and some attributes.
- */
-class PrepareCoolifyTask
-{
-    protected Activity $activity;
-
-    protected CoolifyTaskArgs $remoteProcessArgs;
-
-    public function __construct(CoolifyTaskArgs $remoteProcessArgs)
-    {
-        $this->remoteProcessArgs = $remoteProcessArgs;
-
-        if ($remoteProcessArgs->model) {
-            $properties = $remoteProcessArgs->toArray();
-            unset($properties['model']);
-
-            $this->activity = activity()
-                ->withProperties($properties)
-                ->performedOn($remoteProcessArgs->model)
-                ->event($remoteProcessArgs->type)
-                ->log('[]');
-        } else {
-            $this->activity = activity()
-                ->withProperties($remoteProcessArgs->toArray())
-                ->event($remoteProcessArgs->type)
-                ->log('[]');
-        }
-    }
-
-    public function __invoke(): Activity
-    {
-        $job = new CoolifyTask(
-            activity: $this->activity,
-            ignore_errors: $this->remoteProcessArgs->ignore_errors,
-            call_event_on_finish: $this->remoteProcessArgs->call_event_on_finish,
-            call_event_data: $this->remoteProcessArgs->call_event_data,
-        );
-        dispatch($job);
-        $this->activity->refresh();
-
-        return $this->activity;
-    }
-}

app/Data/CoolifyTaskArgs.php

@@ -1,30 +0,0 @@
-<?php
-
-namespace App\Data;
-
-use App\Enums\ProcessStatus;
-use Illuminate\Database\Eloquent\Model;
-use Spatie\LaravelData\Data;
-
-/**
- * The parameters to execute a CoolifyTask, organized in a DTO.
- */
-class CoolifyTaskArgs extends Data
-{
-    public function __construct(
-        public string $server_uuid,
-        public string $command,
-        public string $type,
-        public ?string $type_uuid = null,
-        public ?int $process_id = null,
-        public ?Model $model = null,
-        public ?string $status = null,
-        public bool $ignore_errors = false,
-        public $call_event_on_finish = null,
-        public $call_event_data = null
-    ) {
-        if (is_null($status)) {
-            $this->status = ProcessStatus::QUEUED->value;
-        }
-    }
-}

app/Livewire/ActivityMonitor.php

@@ -2,7 +2,9 @@
 
 namespace App\Livewire;
 
+use App\Models\Server;
 use App\Models\User;
+use Livewire\Attributes\Locked;
 use Livewire\Component;
 use Spatie\Activitylog\Models\Activity;
 
@@ -10,6 +12,7 @@ class ActivityMonitor extends Component
 {
     public ?string $header = null;
 
+    #[Locked]
     public $activityId = null;
 
     public $eventToDispatch = 'activityFinished';
@@ -57,25 +60,47 @@ public function hydrateActivity()
 
         $activity = Activity::find($this->activityId);
 
-        if ($activity) {
+        if (! $activity) {
-            $teamId = data_get($activity, 'properties.team_id');
+            $this->activity = null;
-            if ($teamId && $teamId !== currentTeam()?->id) {
+
+            return;
+        }
+
+        $currentTeamId = currentTeam()?->id;
+
+        // Check team_id stored directly in activity properties
+        $activityTeamId = data_get($activity, 'properties.team_id');
+        if ($activityTeamId !== null) {
+            if ((int) $activityTeamId !== (int) $currentTeamId) {
                 $this->activity = null;
 
                 return;
             }
+
+            $this->activity = $activity;
+
+            return;
+        }
+
+        // Fallback: verify ownership via the server that ran the command
+        $serverUuid = data_get($activity, 'properties.server_uuid');
+        if ($serverUuid) {
+            $server = Server::where('uuid', $serverUuid)->first();
+            if ($server && (int) $server->team_id !== (int) $currentTeamId) {
+                $this->activity = null;
+
+                return;
+            }
+
+            if ($server) {
+                $this->activity = $activity;
+
+                return;
+            }
         }
 
-        $this->activity = $activity;
+        // Fail closed: no team_id and no server_uuid means we cannot verify ownership
-    }
+        $this->activity = null;
-
-    public function updatedActivityId($value)
-    {
-        if ($value) {
-            $this->hydrateActivity();
-            $this->isPollingActive = true;
-            self::$eventDispatched = false;
-        }
     }
 
     public function polling()

bootstrap/helpers/remoteProcess.php

@@ -1,9 +1,10 @@
 <?php
 
-use App\Actions\CoolifyTask\PrepareCoolifyTask;
-use App\Data\CoolifyTaskArgs;
 use App\Enums\ActivityTypes;
+use App\Enums\ProcessStatus;
 use App\Helpers\SshMultiplexingHelper;
+use App\Helpers\SshRetryHandler;
+use App\Jobs\CoolifyTask;
 use App\Models\Application;
 use App\Models\ApplicationDeploymentQueue;
 use App\Models\PrivateKey;
@@ -38,29 +39,46 @@ function remote_process(
     if (Auth::check()) {
         $teams = Auth::user()->teams->pluck('id');
         if (! $teams->contains($server->team_id) && ! $teams->contains(0)) {
-            throw new \Exception('User is not part of the team that owns this server');
+            throw new Exception('User is not part of the team that owns this server');
         }
     }
 
     SshMultiplexingHelper::ensureMultiplexedConnection($server);
 
-    return resolve(PrepareCoolifyTask::class, [
+    $properties = [
-        'remoteProcessArgs' => new CoolifyTaskArgs(
+        'server_uuid' => $server->uuid,
-            server_uuid: $server->uuid,
+        'command' => $command_string,
-            command: $command_string,
+        'type' => $type,
-            type: $type,
+        'type_uuid' => $type_uuid,
-            type_uuid: $type_uuid,
+        'status' => ProcessStatus::QUEUED->value,
-            model: $model,
+        'team_id' => $server->team_id,
-            ignore_errors: $ignore_errors,
+    ];
-            call_event_on_finish: $callEventOnFinish,
+
-            call_event_data: $callEventData,
+    $activityLog = activity()
-        ),
+        ->withProperties($properties)
-    ])();
+        ->event($type);
+
+    if ($model) {
+        $activityLog->performedOn($model);
+    }
+
+    $activity = $activityLog->log('[]');
+
+    dispatch(new CoolifyTask(
+        activity: $activity,
+        ignore_errors: $ignore_errors,
+        call_event_on_finish: $callEventOnFinish,
+        call_event_data: $callEventData,
+    ));
+
+    $activity->refresh();
+
+    return $activity;
 }
 
 function instant_scp(string $source, string $dest, Server $server, $throwError = true)
 {
-    return \App\Helpers\SshRetryHandler::retry(
+    return SshRetryHandler::retry(
         function () use ($source, $dest, $server) {
             $scp_command = SshMultiplexingHelper::generateScpCommand($server, $source, $dest);
             $process = Process::timeout(config('constants.ssh.command_timeout'))->run($scp_command);
@@ -92,7 +110,7 @@ function instant_remote_process_with_timeout(Collection|array $command, Server $
     }
     $command_string = implode("\n", $command);
 
-    return \App\Helpers\SshRetryHandler::retry(
+    return SshRetryHandler::retry(
         function () use ($server, $command_string) {
             $sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string);
             $process = Process::timeout(30)->run($sshCommand);
@@ -128,7 +146,7 @@ function instant_remote_process(Collection|array $command, Server $server, bool
     $command_string = implode("\n", $command);
     $effectiveTimeout = $timeout ?? config('constants.ssh.command_timeout');
 
-    return \App\Helpers\SshRetryHandler::retry(
+    return SshRetryHandler::retry(
         function () use ($server, $command_string, $effectiveTimeout, $disableMultiplexing) {
             $sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string, $disableMultiplexing);
             $process = Process::timeout($effectiveTimeout)->run($sshCommand);
@@ -170,9 +188,9 @@ function excludeCertainErrors(string $errorOutput, ?int $exitCode = null)
 
     if ($ignored) {
         // TODO: Create new exception and disable in sentry
-        throw new \RuntimeException($errorMessage, $exitCode);
+        throw new RuntimeException($errorMessage, $exitCode);
     }
-    throw new \RuntimeException($errorMessage, $exitCode);
+    throw new RuntimeException($errorMessage, $exitCode);
 }
 
 function decode_remote_command_output(?ApplicationDeploymentQueue $application_deployment_queue = null, bool $includeAll = false): Collection
@@ -194,7 +212,7 @@ function decode_remote_command_output(?ApplicationDeploymentQueue $application_d
             associative: true,
             flags: JSON_THROW_ON_ERROR
         );
-    } catch (\JsonException $e) {
+    } catch (JsonException $e) {
         // If JSON decoding fails, try to clean up the logs and retry
         try {
             // Ensure valid UTF-8 encoding
@@ -204,7 +222,7 @@ function decode_remote_command_output(?ApplicationDeploymentQueue $application_d
                 associative: true,
                 flags: JSON_THROW_ON_ERROR
             );
-        } catch (\JsonException $e) {
+        } catch (JsonException $e) {
             // If it still fails, return empty collection to prevent crashes
             return collect([]);
         }
@@ -353,7 +371,7 @@ function checkRequiredCommands(Server $server)
         }
         try {
             instant_remote_process(["docker run --rm --privileged --net=host --pid=host --ipc=host --volume /:/host busybox chroot /host bash -c 'apt update && apt install -y {$command}'"], $server);
-        } catch (\Throwable) {
+        } catch (Throwable) {
             break;
         }
         $commandFound = instant_remote_process(["docker run --rm --privileged --net=host --pid=host --ipc=host --volume /:/host busybox chroot /host bash -c 'command -v {$command}'"], $server, false);

tests/Feature/ActivityMonitorCrossTeamTest.php

@@ -1,9 +1,11 @@
 <?php
 
 use App\Livewire\ActivityMonitor;
+use App\Models\Server;
 use App\Models\Team;
 use App\Models\User;
 use Illuminate\Foundation\Testing\RefreshDatabase;
+use Livewire\Exceptions\CannotUpdateLockedPropertyException;
 use Livewire\Livewire;
 use Spatie\Activitylog\Models\Activity;
 
@@ -17,7 +19,7 @@
     $this->otherTeam = Team::factory()->create();
 });
 
-test('hydrateActivity blocks access to another teams activity', function () {
+test('hydrateActivity blocks access to another teams activity via team_id', function () {
     $otherActivity = Activity::create([
         'log_name' => 'default',
         'description' => 'test activity',
@@ -27,12 +29,12 @@
     $this->actingAs($this->user);
     session(['currentTeam' => ['id' => $this->team->id]]);
 
-    $component = Livewire::test(ActivityMonitor::class)
+    Livewire::test(ActivityMonitor::class)
-        ->set('activityId', $otherActivity->id)
+        ->call('newMonitorActivity', $otherActivity->id)
         ->assertSet('activity', null);
 });
 
-test('hydrateActivity allows access to own teams activity', function () {
+test('hydrateActivity allows access to own teams activity via team_id', function () {
     $ownActivity = Activity::create([
         'log_name' => 'default',
         'description' => 'test activity',
@@ -43,13 +45,13 @@
     session(['currentTeam' => ['id' => $this->team->id]]);
 
     $component = Livewire::test(ActivityMonitor::class)
-        ->set('activityId', $ownActivity->id);
+        ->call('newMonitorActivity', $ownActivity->id);
 
     expect($component->get('activity'))->not->toBeNull();
     expect($component->get('activity')->id)->toBe($ownActivity->id);
 });
 
-test('hydrateActivity allows access to activity without team_id in properties', function () {
+test('hydrateActivity blocks access to activity without team_id or server_uuid', function () {
     $legacyActivity = Activity::create([
         'log_name' => 'default',
         'description' => 'legacy activity',
@@ -59,9 +61,72 @@
     $this->actingAs($this->user);
     session(['currentTeam' => ['id' => $this->team->id]]);
 
+    Livewire::test(ActivityMonitor::class)
+        ->call('newMonitorActivity', $legacyActivity->id)
+        ->assertSet('activity', null);
+});
+
+test('hydrateActivity blocks access to activity from another teams server via server_uuid', function () {
+    $otherServer = Server::factory()->create([
+        'team_id' => $this->otherTeam->id,
+    ]);
+
+    $otherActivity = Activity::create([
+        'log_name' => 'default',
+        'description' => 'test activity',
+        'properties' => ['server_uuid' => $otherServer->uuid],
+    ]);
+
+    $this->actingAs($this->user);
+    session(['currentTeam' => ['id' => $this->team->id]]);
+
+    Livewire::test(ActivityMonitor::class)
+        ->call('newMonitorActivity', $otherActivity->id)
+        ->assertSet('activity', null);
+});
+
+test('hydrateActivity allows access to activity from own teams server via server_uuid', function () {
+    $ownServer = Server::factory()->create([
+        'team_id' => $this->team->id,
+    ]);
+
+    $ownActivity = Activity::create([
+        'log_name' => 'default',
+        'description' => 'test activity',
+        'properties' => ['server_uuid' => $ownServer->uuid],
+    ]);
+
+    $this->actingAs($this->user);
+    session(['currentTeam' => ['id' => $this->team->id]]);
+
     $component = Livewire::test(ActivityMonitor::class)
-        ->set('activityId', $legacyActivity->id);
+        ->call('newMonitorActivity', $ownActivity->id);
 
     expect($component->get('activity'))->not->toBeNull();
-    expect($component->get('activity')->id)->toBe($legacyActivity->id);
+    expect($component->get('activity')->id)->toBe($ownActivity->id);
 });
+
+test('hydrateActivity returns null for non-existent activity id', function () {
+    $this->actingAs($this->user);
+    session(['currentTeam' => ['id' => $this->team->id]]);
+
+    Livewire::test(ActivityMonitor::class)
+        ->call('newMonitorActivity', 99999)
+        ->assertSet('activity', null);
+});
+
+test('activityId property is locked and cannot be set from client', function () {
+    $otherActivity = Activity::create([
+        'log_name' => 'default',
+        'description' => 'test activity',
+        'properties' => ['team_id' => $this->otherTeam->id],
+    ]);
+
+    $this->actingAs($this->user);
+    session(['currentTeam' => ['id' => $this->team->id]]);
+
+    // Attempting to set a #[Locked] property from the client should throw
+    Livewire::test(ActivityMonitor::class)
+        ->set('activityId', $otherActivity->id)
+        ->assertStatus(500);
+})->throws(CannotUpdateLockedPropertyException::class);