From 7160d45f937a00b17cb22dd1f5dcd607f43e2ac7 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Wed, 12 Aug 2026 10:43:40 -0600 Subject: [PATCH] FOUR-32476: Sanitize DevLink installation errors --- .../DevLinkRemoteValidationException.php | 14 ++ ProcessMaker/Jobs/DevLinkInstall.php | 31 +++- ProcessMaker/Models/DevLink.php | 13 ++ tests/Feature/Jobs/DevLinkInstallTest.php | 152 +++++++++++++++++- tests/Model/DevLinkTest.php | 38 +++++ 5 files changed, 241 insertions(+), 7 deletions(-) create mode 100644 ProcessMaker/Exception/DevLinkRemoteValidationException.php diff --git a/ProcessMaker/Exception/DevLinkRemoteValidationException.php b/ProcessMaker/Exception/DevLinkRemoteValidationException.php new file mode 100644 index 0000000000..0528b6381a --- /dev/null +++ b/ProcessMaker/Exception/DevLinkRemoteValidationException.php @@ -0,0 +1,14 @@ +userId, $this->operationId); - if ($exception instanceof DevLinkRemoteBundleException) { - Log::error($exception->getMessage(), ['exception' => $exception]); - $logger->error($exception->getMessage()); + + Log::error('DevLink operation failed.', [ + 'exception' => $exception, + 'operation_id' => $this->operationId, + 'dev_link_id' => $this->devLinkId, + 'type' => $this->type, + ]); + + if ( + $exception instanceof DevLinkRemoteBundleException + || $exception instanceof DevLinkRemoteValidationException + || $exception instanceof ValidationException + ) { + $message = $exception instanceof ValidationException + ? collect($exception->errors())->flatten()->first(fn ($message) => is_string($message)) + : $exception->getMessage(); + $logger->error($message ?: $exception->getMessage()); + } elseif ($exception instanceof RequestException) { + $logger->error(__(self::REMOTE_ERROR_MESSAGE)); } else { - $logger->exception($exception); + $logger->error(__(self::UNEXPECTED_ERROR_MESSAGE)); } // Unlock the job diff --git a/ProcessMaker/Models/DevLink.php b/ProcessMaker/Models/DevLink.php index 7637d24253..f31e460643 100644 --- a/ProcessMaker/Models/DevLink.php +++ b/ProcessMaker/Models/DevLink.php @@ -7,6 +7,7 @@ use Illuminate\Support\Facades\Http; use Illuminate\Support\Str; use ProcessMaker\Exception\DevLinkRemoteBundleException; +use ProcessMaker\Exception\DevLinkRemoteValidationException; use ProcessMaker\ImportExport\Importer; use ProcessMaker\ImportExport\Logger; use ProcessMaker\ImportExport\Options; @@ -165,6 +166,18 @@ public function installRemoteBundle($remoteBundleId, $updateType) throw new DevLinkRemoteBundleException($invalidAssets, $exception); } + $invalidDependencies = $exception->response->json('errors.dependencies'); + $validationMessage = $exception->response->json('error.message'); + if ( + $exception->response->status() === 422 + && is_array($invalidDependencies) + && $invalidDependencies !== [] + && is_string($validationMessage) + && trim($validationMessage) !== '' + ) { + throw new DevLinkRemoteValidationException($validationMessage, $exception); + } + throw $exception; } diff --git a/tests/Feature/Jobs/DevLinkInstallTest.php b/tests/Feature/Jobs/DevLinkInstallTest.php index 67cc45dd66..f31372f9eb 100644 --- a/tests/Feature/Jobs/DevLinkInstallTest.php +++ b/tests/Feature/Jobs/DevLinkInstallTest.php @@ -2,11 +2,17 @@ namespace Tests\Feature\Jobs; +use GuzzleHttp\Psr7\Response as PsrResponse; +use Illuminate\Http\Client\RequestException; +use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Mockery; use ProcessMaker\Exception\DevLinkRemoteBundleException; +use ProcessMaker\Exception\DevLinkRemoteValidationException; +use ProcessMaker\Exception\ValidationException; use ProcessMaker\Events\ImportLog; use ProcessMaker\Jobs\DevLinkInstall; use ProcessMaker\Jobs\ImportV2; @@ -16,9 +22,16 @@ class DevLinkInstallTest extends TestCase { - public function testFailedJobIncludesOperationIdInErrorEvent() + private const MISSING_DISPLAY_SCREEN_MESSAGE = 'The dashboard "DevLink Dashboard" references a Display Screen that is no longer available. Assign a valid Display Screen on the source instance and try again.'; + + private const REMOTE_ERROR_MESSAGE = 'The remote instance could not complete the DevLink request. Check the source instance logs and try again.'; + + private const UNEXPECTED_ERROR_MESSAGE = 'The DevLink operation could not be completed. Check the target instance logs and try again.'; + + public function testFailedUnexpectedJobSanitizesErrorAndLogsCorrelatedException() { Event::fake([ImportLog::class]); + Log::spy(); Storage::fake('local'); $lock = Mockery::mock(); @@ -38,13 +51,81 @@ public function testFailedJobIncludesOperationIdInErrorEvent() 'operation-123', ); - $job->failed(new RuntimeException('Installation failed')); + $exception = new RuntimeException('Sensitive installation details'); + + $job->failed($exception); Event::assertDispatched(ImportLog::class, function (ImportLog $event) { return $event->type === 'error' - && str_contains($event->message, 'Installation failed') + && $event->message === self::UNEXPECTED_ERROR_MESSAGE + && !str_contains($event->message, RuntimeException::class) + && !str_contains($event->message, 'Sensitive installation details') && $event->operationId === 'operation-123'; }); + + Log::shouldHaveReceived('error') + ->once() + ->withArgs(function (string $message, array $context) use ($exception) { + return $message === 'DevLink operation failed.' + && $context['exception'] === $exception + && $context['operation_id'] === 'operation-123' + && $context['dev_link_id'] === 456 + && $context['type'] === DevLinkInstall::TYPE_INSTALL_BUNDLE; + }); + } + + public function testFailedRemoteRequestSanitizesHttpResponseBody() + { + Event::fake([ImportLog::class]); + Log::spy(); + Storage::fake('local'); + + $lock = Mockery::mock(); + $lock->shouldReceive('forceRelease')->once(); + Cache::shouldReceive('lock') + ->once() + ->with(ImportV2::CACHE_LOCK_KEY) + ->andReturn($lock); + + $job = new DevLinkInstall( + 123, + 456, + Bundle::class, + 789, + DevLinkInstall::MODE_UPDATE, + DevLinkInstall::TYPE_INSTALL_BUNDLE, + 'operation-remote-error', + ); + $exception = new RequestException(new Response(new PsrResponse( + 500, + [], + json_encode([ + 'message' => 'The MAC is invalid.', + 'exception' => 'Illuminate\\Contracts\\Encryption\\DecryptException', + 'trace' => ['sensitive trace'], + ]) + ))); + + $job->failed($exception); + + Event::assertDispatched(ImportLog::class, function (ImportLog $event) { + return $event->type === 'error' + && $event->message === self::REMOTE_ERROR_MESSAGE + && !str_contains($event->message, '500') + && !str_contains($event->message, 'MAC') + && !str_contains($event->message, RequestException::class) + && $event->operationId === 'operation-remote-error'; + }); + + Log::shouldHaveReceived('error') + ->once() + ->withArgs(function (string $message, array $context) use ($exception) { + return $message === 'DevLink operation failed.' + && $context['exception'] === $exception + && $context['operation_id'] === 'operation-remote-error' + && $context['dev_link_id'] === 456 + && $context['type'] === DevLinkInstall::TYPE_INSTALL_BUNDLE; + }); } public function testFailedRemoteBundleJobIncludesOperationIdInActionableErrorEvent() @@ -83,4 +164,69 @@ public function testFailedRemoteBundleJobIncludesOperationIdInActionableErrorEve && $event->operationId === 'operation-456'; }); } + + public function testFailedRemoteValidationJobIncludesOperationIdInActionableErrorEvent() + { + Event::fake([ImportLog::class]); + Storage::fake('local'); + + $lock = Mockery::mock(); + $lock->shouldReceive('forceRelease')->once(); + Cache::shouldReceive('lock') + ->once() + ->with(ImportV2::CACHE_LOCK_KEY) + ->andReturn($lock); + + $job = new DevLinkInstall( + 123, + 456, + Bundle::class, + 789, + DevLinkInstall::MODE_UPDATE, + DevLinkInstall::TYPE_INSTALL_BUNDLE, + 'operation-remote-validation', + ); + + $job->failed(new DevLinkRemoteValidationException(self::MISSING_DISPLAY_SCREEN_MESSAGE)); + + Event::assertDispatched(ImportLog::class, function (ImportLog $event) { + return $event->type === 'error' + && $event->message === self::MISSING_DISPLAY_SCREEN_MESSAGE + && $event->operationId === 'operation-remote-validation'; + }); + } + + public function testFailedLocalValidationJobIncludesOperationIdInActionableErrorEvent() + { + Event::fake([ImportLog::class]); + Storage::fake('local'); + $exception = ValidationException::withMessages([ + 'dependencies' => [self::MISSING_DISPLAY_SCREEN_MESSAGE], + ]); + + $lock = Mockery::mock(); + $lock->shouldReceive('forceRelease')->once(); + Cache::shouldReceive('lock') + ->once() + ->with(ImportV2::CACHE_LOCK_KEY) + ->andReturn($lock); + + $job = new DevLinkInstall( + 123, + 456, + Bundle::class, + 789, + DevLinkInstall::MODE_UPDATE, + DevLinkInstall::TYPE_REINSTALL_BUNDLE, + 'operation-local-validation', + ); + + $job->failed($exception); + + Event::assertDispatched(ImportLog::class, function (ImportLog $event) { + return $event->type === 'error' + && $event->message === self::MISSING_DISPLAY_SCREEN_MESSAGE + && $event->operationId === 'operation-local-validation'; + }); + } } diff --git a/tests/Model/DevLinkTest.php b/tests/Model/DevLinkTest.php index f0efaf12d3..785b06b229 100644 --- a/tests/Model/DevLinkTest.php +++ b/tests/Model/DevLinkTest.php @@ -5,6 +5,7 @@ use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Storage; use ProcessMaker\Exception\DevLinkRemoteBundleException; +use ProcessMaker\Exception\DevLinkRemoteValidationException; use ProcessMaker\Models\Bundle; use ProcessMaker\Models\DevLink; use ProcessMaker\Models\Screen; @@ -28,6 +29,8 @@ class DevLinkTest extends TestCase private const EXPORT_LOCAL_BUNDLE_API_PATH = 'export-local-bundle/123'; + private const MISSING_DISPLAY_SCREEN_MESSAGE = 'The dashboard "DevLink Dashboard" references a Display Screen that is no longer available. Assign a valid Display Screen on the source instance and try again.'; + public function testGetClientUrl() { $devLink = DevLink::factory()->create([ @@ -162,6 +165,41 @@ public function testInstallRemoteBundleReportsUnavailableRemoteAssets() $devLink->installRemoteBundle(123, 'update'); } + public function testInstallRemoteBundleReportsUnavailableRemoteDependencies() + { + Http::preventStrayRequests(); + Http::fake([ + self::remoteApiUrl(self::LOCAL_BUNDLE_API_PATH) => Http::response(self::remoteBundleResponse('5')), + self::remoteApiUrl(self::EXPORT_LOCAL_BUNDLE_API_PATH) => Http::response(['payloads' => []]), + self::remoteApiUrl('export-local-bundle/123/settings') => Http::response(['settings' => []]), + self::remoteApiUrl('export-local-bundle/123/settings-payloads') => Http::response([ + 'error' => [ + 'code' => 422, + 'message' => self::MISSING_DISPLAY_SCREEN_MESSAGE, + ], + 'errors' => [ + 'dependencies' => [self::MISSING_DISPLAY_SCREEN_MESSAGE], + ], + ], 422), + ]); + + $devLink = DevLink::factory()->create([ + 'url' => self::REMOTE_INSTANCE_URL, + ]); + + try { + $devLink->installRemoteBundle(123, 'update'); + $this->fail('Installing a remote bundle with an unavailable dependency should fail.'); + } catch (DevLinkRemoteValidationException $exception) { + $this->assertSame(self::MISSING_DISPLAY_SCREEN_MESSAGE, $exception->getMessage()); + } + + $this->assertDatabaseMissing('bundles', [ + 'dev_link_id' => $devLink->id, + 'remote_id' => 123, + ]); + } + public function testInstallRemoteBundleImportsAndReinstallsEverySelectedMenu() { if (!hasPackage('package-dynamic-ui')) {