From b3bc1c0b489b813b13e600172b736d879d161b6c Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Thu, 30 Jul 2026 13:22:41 -0400 Subject: [PATCH 01/15] FOUR-32465: [Octane] CRITICAL Data Leaks Between Requests "$uid2id" --- .../Nayra/Repositories/EntityRepository.php | 10 +-- .../Repositories/EntityRepositoryTest.php | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php diff --git a/ProcessMaker/Nayra/Repositories/EntityRepository.php b/ProcessMaker/Nayra/Repositories/EntityRepository.php index 4b10af1a02..2f84e55b7e 100644 --- a/ProcessMaker/Nayra/Repositories/EntityRepository.php +++ b/ProcessMaker/Nayra/Repositories/EntityRepository.php @@ -11,7 +11,7 @@ abstract class EntityRepository { - private static $uid2id = ['requests' =>[], 'tokens' =>[]]; + private $uid2id = ['requests' =>[], 'tokens' =>[]]; abstract public function create(array $transaction): ? Model; @@ -41,16 +41,16 @@ public function resolveId(string $uid): int } // Get record if is not stored previously - if (!isset(self::$uid2id[$type][$uid])) { + if (!isset($this->uid2id[$type][$uid])) { $record = $instance->select('id')->where('uuid', $uid)->first(); if ($record) { - self::$uid2id[$type][$uid] = $record->getKey(); + $this->uid2id[$type][$uid] = $record->getKey(); } else { throw new Exception("The uid {$uid} does not exist in the database"); } } - return self::$uid2id[$type][$uid] ?? 0; + return $this->uid2id[$type][$uid] ?? 0; } /** @@ -71,6 +71,6 @@ public function storeUid(string $uid, int $id): void break; } - self::$uid2id[$type][$uid] = $id; + $this->uid2id[$type][$uid] = $id; } } diff --git a/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php b/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php new file mode 100644 index 0000000000..29986e2f77 --- /dev/null +++ b/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php @@ -0,0 +1,66 @@ +assertFalse( + $reflection->isStatic(), + 'uid2id must NOT be static to prevent data leaks between requests in Octane' + ); + } + + /** + * Test that $uid2id is a private property. + */ + public function test_uid2id_is_private(): void + { + $reflection = new ReflectionProperty(EntityRepository::class, 'uid2id'); + + $this->assertTrue( + $reflection->isPrivate(), + 'uid2id should be private' + ); + } +} From 214ac429b5ed38f593f8ccef33bb620dc2d6b6fe Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 09:36:14 -0400 Subject: [PATCH 02/15] feat: refactor redirect handling by introducing RedirectToEventService --- ProcessMaker/Jobs/BpmnAction.php | 6 +- .../Listeners/HandleRedirectListener.php | 35 ++------- .../Providers/ProcessMakerServiceProvider.php | 3 + .../Services/RedirectToEventService.php | 75 +++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 ProcessMaker/Services/RedirectToEventService.php diff --git a/ProcessMaker/Jobs/BpmnAction.php b/ProcessMaker/Jobs/BpmnAction.php index f78ddaf647..eefc44a149 100644 --- a/ProcessMaker/Jobs/BpmnAction.php +++ b/ProcessMaker/Jobs/BpmnAction.php @@ -14,10 +14,10 @@ use Illuminate\Support\Facades\Log; use ProcessMaker\BpmnEngine; use ProcessMaker\Exception\HttpABTestingException; -use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\Process as Definitions; use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Models\ProcessRequestLock; +use ProcessMaker\Services\RedirectToEventService; use Throwable; abstract class BpmnAction implements ShouldQueue @@ -60,6 +60,7 @@ abstract class BpmnAction implements ShouldQueue public function handle() { $response = null; + $redirectToEventService = app(RedirectToEventService::class); try { extract($this->loadContext()); $this->engine = $engine; @@ -74,7 +75,7 @@ public function handle() // (e.g. completed, assigned, process completed, etc) // excluding system process (non_persistent_process) if ($this->processId !== 'non_persistent_process') { - HandleRedirectListener::sendRedirectToEvent(); + $redirectToEventService->sendRedirectToEvent(); } } catch (HttpABTestingException $exception) { Log::error($exception->getMessage()); @@ -87,6 +88,7 @@ public function handle() $request->logError($exception, $element); } } finally { + $redirectToEventService->reset(); $this->unlock(); } diff --git a/ProcessMaker/Listeners/HandleRedirectListener.php b/ProcessMaker/Listeners/HandleRedirectListener.php index 78491809a4..490235af17 100644 --- a/ProcessMaker/Listeners/HandleRedirectListener.php +++ b/ProcessMaker/Listeners/HandleRedirectListener.php @@ -2,40 +2,19 @@ namespace ProcessMaker\Listeners; -use ProcessMaker\Events\RedirectToEvent; use ProcessMaker\Models\ProcessRequest; +use ProcessMaker\Services\RedirectToEventService; class HandleRedirectListener { - private static $processRequest = null; - - protected static $redirectionMethod = ''; - - private static $redirectionParams = []; - - protected function setRedirectTo(ProcessRequest $processRequest, string $method, ...$params): void - { - self::$processRequest = $processRequest; - self::$redirectionMethod = $method; - self::$redirectionParams = $params; + public function __construct( + private ?RedirectToEventService $redirectToEventService = null + ) { } - public static function sendRedirectToEvent() + protected function setRedirectTo(ProcessRequest $processRequest, string $method, ...$params): void { - $method = self::$redirectionMethod; - $params = self::$redirectionParams; - $processRequest = self::$processRequest; - - // Only get active tokens if there is a valid process request - if ($processRequest !== null) { - $params['activeTokens'] = ProcessRequest::getActiveTokens($processRequest); - $event = new RedirectToEvent($processRequest, $method, $params); - event($event); - - // Clean params to prevent sending the same redirect multiple times - self::$redirectionParams = []; - self::$redirectionMethod = ''; - self::$processRequest = null; - } + $this->redirectToEventService ??= app(RedirectToEventService::class); + $this->redirectToEventService->setRedirectTo($processRequest, $method, ...$params); } } diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..603b116c3d 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -53,6 +53,7 @@ use ProcessMaker\Providers\PermissionServiceProvider; use ProcessMaker\Repositories\SettingsConfigRepository; use ProcessMaker\Services\ConditionalRedirectService; +use ProcessMaker\Services\RedirectToEventService; use RuntimeException; use Spatie\Multitenancy\Events\MadeTenantCurrentEvent; use Spatie\Multitenancy\Events\TenantNotFoundForRequestEvent; @@ -243,6 +244,8 @@ public function register(): void $this->app->instance('tenant-resolved', false); + $this->app->scoped(RedirectToEventService::class); + /** * Conditional Redirect Service * This service is used to evaluate the conditional redirect property of a process request token. diff --git a/ProcessMaker/Services/RedirectToEventService.php b/ProcessMaker/Services/RedirectToEventService.php new file mode 100644 index 0000000000..4dd67eb8c8 --- /dev/null +++ b/ProcessMaker/Services/RedirectToEventService.php @@ -0,0 +1,75 @@ +processRequest = $processRequest; + $this->redirectionMethod = $method; + $this->redirectionParams = $params; + } + + /** + * Dispatch the pending redirect, including the request's active token IDs. + * + * This method is a no-op when no redirect is pending. Pending state is + * consumed before querying tokens or dispatching the event so an exception + * cannot cause stale request data to be retried or leaked into later work. + * + * @throws \Throwable If active-token retrieval or event dispatch fails + */ + public function sendRedirectToEvent(): void + { + if ($this->processRequest === null) { + return; + } + + $processRequest = $this->processRequest; + $method = $this->redirectionMethod; + $params = $this->redirectionParams; + + // Consume the pending redirect before doing work that may throw. + $this->reset(); + + $params['activeTokens'] = ProcessRequest::getActiveTokens($processRequest); + event(new RedirectToEvent($processRequest, $method, $params)); + } + + /** + * Discard all pending redirect state without dispatching an event. + */ + public function reset(): void + { + $this->processRequest = null; + $this->redirectionMethod = ''; + $this->redirectionParams = []; + } +} From 7f405b46f66a6be76b56f4c40268782b2ae1efff Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 10:01:09 -0400 Subject: [PATCH 03/15] test: add unit tests for BpmnAction and RedirectToEventService functionality --- .../Jobs/BpmnActionRedirectCleanupTest.php | 25 ++ .../Services/RedirectToEventServiceTest.php | 230 ++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php create mode 100644 tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php diff --git a/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php b/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php new file mode 100644 index 0000000000..ceb1b27262 --- /dev/null +++ b/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php @@ -0,0 +1,25 @@ +shouldReceive('sendRedirectToEvent')->never(); + $redirectToEventService->shouldReceive('reset')->once(); + app()->instance(RedirectToEventService::class, $redirectToEventService); + + $job = new class extends BpmnAction { + protected $definitionsId = -1; + }; + + $this->assertNull($job->handle()); + } +} diff --git a/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php b/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php new file mode 100644 index 0000000000..e24d3fa3c1 --- /dev/null +++ b/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php @@ -0,0 +1,230 @@ +create(); + $secondRequest = ProcessRequest::factory()->create(); + $service = app(RedirectToEventService::class); + + $service->setRedirectTo($firstRequest, 'firstRedirect', [ + 'requestId' => $firstRequest->id, + ]); + $service->setRedirectTo($secondRequest, 'secondRedirect', [ + 'requestId' => $secondRequest->id, + ]); + + $service->sendRedirectToEvent(); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($secondRequest) { + return $event->method === 'secondRedirect' + && $event->params[0]['requestId'] === $secondRequest->id + && $event->params['activeTokens'] === [] + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $secondRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); + } + + public function test_scoped_binding_does_not_leak_pending_redirect_between_operations(): void + { + Event::fake([RedirectToEvent::class]); + + $firstRequest = ProcessRequest::factory()->create(); + $firstScope = app(RedirectToEventService::class); + $this->assertSame($firstScope, app(RedirectToEventService::class)); + $firstScope->setRedirectTo($firstRequest, 'staleRedirect'); + + app()->forgetScopedInstances(); + + $secondScope = app(RedirectToEventService::class); + $this->assertNotSame($firstScope, $secondScope); + + $secondScope->sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); + + $secondRequest = ProcessRequest::factory()->create(); + $secondScope->setRedirectTo($secondRequest, 'currentRedirect', [ + 'requestId' => $secondRequest->id, + ]); + $secondScope->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($secondRequest) { + return $event->method === 'currentRedirect' + && $event->params[0]['requestId'] === $secondRequest->id + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $secondRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); + } + + public function test_reset_discards_pending_redirect(): void + { + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo(ProcessRequest::factory()->create(), 'discardedRedirect'); + + $service->reset(); + $service->sendRedirectToEvent(); + + Event::assertNotDispatched(RedirectToEvent::class); + } + + public function test_activity_completed_listener_and_dispatcher_share_the_same_scoped_state(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $activeToken = ProcessRequestToken::factory()->create([ + 'process_id' => $processRequest->process_id, + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $activeToken->setInstance($processRequest); + + app(HandleActivityCompletedRedirect::class)->handle(new ActivityCompleted($activeToken)); + app(RedirectToEventService::class)->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $activeToken, + $processRequest + ) { + return $event->method === 'processUpdated' + && $event->params[0]['tokenId'] === $activeToken->id + && $event->params[0]['requestStatus'] === $processRequest->status + && $event->params['activeTokens'] === [$activeToken->id]; + }); + } + + public function test_active_tokens_exclude_closed_and_unrelated_request_tokens(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $unrelatedRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $activeToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $closedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'CLOSED', + ]); + $unrelatedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $unrelatedRequest->id, + 'status' => 'ACTIVE', + ]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'isolatedRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $activeToken, + $closedToken, + $unrelatedToken + ) { + return $event->params['activeTokens'] === [$activeToken->id] + && !in_array($closedToken->id, $event->params['activeTokens'], true) + && !in_array($unrelatedToken->id, $event->params['activeTokens'], true); + }); + } + + public function test_active_tokens_include_all_active_tokens_in_the_same_collaboration(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create(); + $collaboratingRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => $processRequest->process_collaboration_id, + ]); + $unrelatedRequest = ProcessRequest::factory()->create(); + $firstToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $collaboratingToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $collaboratingRequest->id, + 'status' => 'ACTIVE', + ]); + $unrelatedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $unrelatedRequest->id, + 'status' => 'ACTIVE', + ]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'collaborationRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $firstToken, + $collaboratingToken, + $unrelatedToken + ) { + $activeTokens = $event->params['activeTokens']; + sort($activeTokens); + + $expectedTokens = [$firstToken->id, $collaboratingToken->id]; + sort($expectedTokens); + + return $activeTokens === $expectedTokens + && !in_array($unrelatedToken->id, $activeTokens, true); + }); + } + + public function test_pending_redirect_is_consumed_when_event_dispatch_throws(): void + { + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'failingRedirect'); + + $originalDispatcher = Event::getFacadeRoot(); + $failingDispatcher = Mockery::mock(Dispatcher::class); + $failingDispatcher->shouldReceive('dispatch') + ->once() + ->with(Mockery::type(RedirectToEvent::class)) + ->andThrow(new RuntimeException('Broadcast failed')); + Event::swap($failingDispatcher); + + try { + try { + $service->sendRedirectToEvent(); + $this->fail('The event dispatcher should have thrown an exception.'); + } catch (RuntimeException $exception) { + $this->assertSame('Broadcast failed', $exception->getMessage()); + } + + // A retry without a new redirect must not dispatch the failed event again. + $service->sendRedirectToEvent(); + } finally { + Event::swap($originalDispatcher); + } + } +} From 2ff685cd1be44a5ea7082c9e61c1402e9a9e682f Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Fri, 31 Jul 2026 10:55:30 -0400 Subject: [PATCH 04/15] feat(FOUR-32473): [Octane] CRITICAL Data Leaks Between Requests "$redirectionParams" --- .../Listeners/HandleRedirectListener.php | 4 +- .../Listeners/HandleRedirectListenerTest.php | 235 ++++++++++++++++++ 2 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php diff --git a/ProcessMaker/Listeners/HandleRedirectListener.php b/ProcessMaker/Listeners/HandleRedirectListener.php index 7679a73572..2d2850fad8 100644 --- a/ProcessMaker/Listeners/HandleRedirectListener.php +++ b/ProcessMaker/Listeners/HandleRedirectListener.php @@ -44,9 +44,7 @@ public static function sendRedirectToEvent() event($event); // Clean params to prevent sending the same redirect multiple times - self::$redirectionParams = []; - self::$redirectionMethod = ''; - self::$processRequest = null; + self::reset(); } } } diff --git a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php new file mode 100644 index 0000000000..0e3e86cf4e --- /dev/null +++ b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php @@ -0,0 +1,235 @@ +setRedirectTo($processRequest, $method, ...$params); + } + }; + } + + /** + * Read a private static property from HandleRedirectListener. + */ + private function readStaticProperty(string $property): mixed + { + $reflection = new ReflectionProperty(HandleRedirectListener::class, $property); + $reflection->setAccessible(true); + + return $reflection->getValue(); + } + + /** + * Assert that all 3 static properties are in their default/clean state. + */ + private function assertStateIsClean(): void + { + $this->assertNull($this->readStaticProperty('processRequest')); + $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([], $this->readStaticProperty('redirectionParams')); + } + + /** + * Test that reset() clears the static $processRequest property. + */ + public function test_reset_clears_process_request(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated'); + + HandleRedirectListener::reset(); + + $this->assertNull($this->readStaticProperty('processRequest')); + } + + /** + * Test that reset() clears the static $redirectionMethod property. + */ + public function test_reset_clears_redirection_method(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processCompletedRedirect'); + + HandleRedirectListener::reset(); + + $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + } + + /** + * Test that reset() clears the static $redirectionParams property. + */ + public function test_reset_clears_redirection_params(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['key' => 'value']); + + HandleRedirectListener::reset(); + + $this->assertSame([], $this->readStaticProperty('redirectionParams')); + } + + /** + * Critical test for Octane: verify that reset() prevents data leaks. + * After reset(), the stored redirect data should be gone. + */ + public function test_reset_prevents_stale_redirect_from_leaking(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['tokenId' => 123]); + + // Simulate Octane reset between requests + HandleRedirectListener::reset(); + + // sendRedirectToEvent should NOT dispatch RedirectToEvent after reset + $this->expectNotToPerformAssertions(); + HandleRedirectListener::sendRedirectToEvent(); + } + + /** + * Test that reset() can be called multiple times safely. + */ + public function test_reset_can_be_called_multiple_times(): void + { + HandleRedirectListener::reset(); + HandleRedirectListener::reset(); + HandleRedirectListener::reset(); + + // Should not throw any errors + $this->assertStateIsClean(); + } + + /** + * Test that sendRedirectToEvent dispatches the event and clears state. + */ + public function test_send_redirect_to_event_dispatches_and_clears_state(): void + { + \Illuminate\Support\Facades\Event::fake([RedirectToEvent::class]); + + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated'); + + HandleRedirectListener::sendRedirectToEvent(); + + // Assert the event was dispatched + \Illuminate\Support\Facades\Event::assertDispatched(RedirectToEvent::class); + + // After dispatch, the state should be cleared + $this->assertNull($this->readStaticProperty('processRequest')); + } + + /** + * CRITICAL: Simulate the full Octane request cycle to guarantee no data leak. + * + * Flow: + * 1. Request A stores data with different values + * 2. Reset (simulating Octane's RequestTerminated event) + * 3. Verify ALL 3 properties are clean + * 4. Request B stores NEW data with different values + * 5. Verify Request B's data is correct (not contaminated by Request A) + * 6. Reset again + * 7. Verify clean again + */ + public function test_full_octane_cycle_guarantees_no_data_leak(): void + { + // === Request A === + $requestA = ProcessRequest::factory()->create(); + $probeA = $this->createProbe(); + $probeA->queue($requestA, 'processCompletedRedirect', ['tokenA' => 111]); + + // Verify Request A data is stored (setRedirectTo uses ...$params, so it's nested) + $this->assertSame($requestA->getKey(), $this->readStaticProperty('processRequest')->getKey()); + $this->assertSame('processCompletedRedirect', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([['tokenA' => 111]], $this->readStaticProperty('redirectionParams')); + + // === Octane reset after Request A === + HandleRedirectListener::reset(); + + // === Verify ALL properties are clean after reset === + $this->assertStateIsClean(); + + // === Request B (simulating a DIFFERENT user/request) === + $requestB = ProcessRequest::factory()->create(); + $probeB = $this->createProbe(); + $probeB->queue($requestB, 'processUpdated', ['tokenB' => 222, 'userId' => 999]); + + // Verify Request B's data is correct (NOT contaminated by Request A) + $this->assertSame($requestB->getKey(), $this->readStaticProperty('processRequest')->getKey()); + $this->assertSame('processUpdated', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([['tokenB' => 222, 'userId' => 999]], $this->readStaticProperty('redirectionParams')); + + // Verify Request A's data is GONE (no leak) + $this->assertNotSame($requestA->getKey(), $this->readStaticProperty('processRequest')?->getKey()); + + // === Octane reset after Request B === + HandleRedirectListener::reset(); + + // === Verify clean again === + $this->assertStateIsClean(); + } + + /** + * CRITICAL: Verify that ResetRequestState orchestrator triggers the reset correctly. + */ + public function test_reset_request_state_triggers_handle_redirect_reset(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['tokenId' => 456]); + + // Verify state is dirty before reset + $this->assertNotNull($this->readStaticProperty('processRequest')); + + // Execute the orchestrator (same as Octane's RequestTerminated listener) + $resetState = new ResetRequestState(); + $resetState->handle(); + + // Verify orchestrator cleaned everything + $this->assertStateIsClean(); + } + + /** + * CRITICAL: Simulate the scenario where sendRedirectToEvent() fails, + * but reset() still cleans up (edge case in Octane). + */ + public function test_reset_cleans_up_even_when_send_redirect_fails(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['data' => 'sensitive']); + + // Simulate that sendRedirectToEvent is NEVER called (e.g., error in BPMN flow) + // But Octane's RequestTerminated event still fires and calls reset() + + // This should NOT be called in this scenario: + // HandleRedirectListener::sendRedirectToEvent(); + + // Octane reset still happens + HandleRedirectListener::reset(); + + // Verify no sensitive data leaked + $this->assertStateIsClean(); + } +} From f0ba6183b8e2452bd0309f083e4765a9e6542996 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 10:55:30 -0400 Subject: [PATCH 05/15] FOUR-32474 [Octane] CRITICAL Data Leaks Between Requests "$landlordValues" Description: Fix Octane data leak by storing landlord config in request-scoped Context Replace SwitchTenant static $landlordValues with Laravel Context to prevent tenant config snapshots from persisting across Octane requests. Remove unused duplicate property from ProcessMakerServiceProvider. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32474 --- ProcessMaker/Multitenancy/SwitchTenant.php | 10 ++-- .../Providers/ProcessMakerServiceProvider.php | 3 -- .../Multitenancy/SwitchTenantTest.php | 53 +++++++++++++++++++ 3 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php diff --git a/ProcessMaker/Multitenancy/SwitchTenant.php b/ProcessMaker/Multitenancy/SwitchTenant.php index e85c1421f7..34e7a5be60 100644 --- a/ProcessMaker/Multitenancy/SwitchTenant.php +++ b/ProcessMaker/Multitenancy/SwitchTenant.php @@ -6,6 +6,7 @@ use Illuminate\Contracts\Routing\UrlGenerator; use Illuminate\Support\Arr; use Illuminate\Support\Env; +use Illuminate\Support\Facades\Context; use Monolog\Handler\RotatingFileHandler; use ProcessMaker\Application; use ProcessMaker\Multitenancy\Broadcasting\TenantAwareBroadcastManager; @@ -17,7 +18,7 @@ class SwitchTenant implements SwitchTenantTask { use UsesMultitenancyConfig; - public static $landlordValues = null; + private const LANDLORD_VALUES_CONTEXT_KEY = 'multitenancy.landlord_values'; /** * Make the given tenant current. @@ -31,9 +32,8 @@ public function makeCurrent(IsTenant $tenant): void \Log::debug('SwitchTenant: ' . $tenant->id, ['domain' => request()->getHost()]); - // Save the landlord values for later use - if (!self::$landlordValues) { - self::$landlordValues = $app->make('config')->all(); + if (!Context::has(self::LANDLORD_VALUES_CONTEXT_KEY)) { + Context::add(self::LANDLORD_VALUES_CONTEXT_KEY, $app->make('config')->all()); } // Set the tenant's domain in the request headers. Used for things like the global url() helper. @@ -70,7 +70,7 @@ public function forgetCurrent(): void private function landlordConfig($key) { - return Arr::get(self::$landlordValues, $key); + return Arr::get(Context::get(self::LANDLORD_VALUES_CONTEXT_KEY), $key); } private function setConfig($key, $value) diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..3706dac585 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -75,9 +75,6 @@ class ProcessMakerServiceProvider extends ServiceProvider // Track the query time for each request private static $queryTime = 0; - // Track the landlord values for multitenancy - private static $landlordValues = null; - public function boot(): void { // Track the start time for service providers boot diff --git a/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php b/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php new file mode 100644 index 0000000000..e19db9daa0 --- /dev/null +++ b/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php @@ -0,0 +1,53 @@ + 'https://landlord.example.com']); + Context::add(self::LANDLORD_VALUES_KEY, config()->all()); + + config(['app.url' => 'https://tenant-modified.example.com']); + + $this->assertSame( + 'https://landlord.example.com', + $this->landlordConfig('app.url') + ); + } + + public function test_landlord_values_are_not_reused_across_requests(): void + { + Context::add(self::LANDLORD_VALUES_KEY, ['app' => ['url' => 'https://tenant-a.example.com']]); + Context::forget(self::LANDLORD_VALUES_KEY); + + config(['app.url' => 'https://tenant-b.example.com']); + Context::add(self::LANDLORD_VALUES_KEY, config()->all()); + + $this->assertSame( + 'https://tenant-b.example.com', + Context::get(self::LANDLORD_VALUES_KEY)['app']['url'] + ); + } + + private function landlordConfig(string $key): mixed + { + $method = new \ReflectionMethod(SwitchTenant::class, 'landlordConfig'); + + return $method->invoke(new SwitchTenant(), $key); + } +} From 6775f5eeab5ea5fcf397c645ba460cc61e0a12f5 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 11:43:04 -0400 Subject: [PATCH 06/15] FOUR-32475 [Octane] CRITICAL Data Leaks Between Requests "AnonymousUser::class" Description: Replace AnonymousUser singleton with a scoped binding and add resolve() to load the user from the database per request. Prevents stale anonymous user data from leaking across Octane requests while keeping the same behavior in PHP-FPM and queue workers. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32475 --- ProcessMaker/Models/AnonymousUser.php | 6 ++ .../Providers/ProcessMakerServiceProvider.php | 5 +- .../ProcessMaker/Models/AnonymousUserTest.php | 64 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/unit/ProcessMaker/Models/AnonymousUserTest.php diff --git a/ProcessMaker/Models/AnonymousUser.php b/ProcessMaker/Models/AnonymousUser.php index b78c65e1df..3e15c87e08 100644 --- a/ProcessMaker/Models/AnonymousUser.php +++ b/ProcessMaker/Models/AnonymousUser.php @@ -12,6 +12,12 @@ class AnonymousUser extends User protected $table = 'users'; + public static function resolve(): self + { + return static::where('username', '=', static::ANONYMOUS_USERNAME) + ->firstOrFail(); + } + public $isAnonymous = true; public function receivesBroadcastNotificationsOn($notification) diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..f11d725e55 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -188,9 +188,8 @@ public function register(): void return new Managers\GlobalScriptsManager(); }); - $this->app->singleton(Models\AnonymousUser::class, function ($app) { - return Models\AnonymousUser::where('username', '=', Models\AnonymousUser::ANONYMOUS_USERNAME) - ->firstOrFail(); + $this->app->scoped(Models\AnonymousUser::class, function ($app) { + return Models\AnonymousUser::resolve(); }); $this->app->singleton(PolicyExtension::class, function ($app) { diff --git a/tests/unit/ProcessMaker/Models/AnonymousUserTest.php b/tests/unit/ProcessMaker/Models/AnonymousUserTest.php new file mode 100644 index 0000000000..864e094c9a --- /dev/null +++ b/tests/unit/ProcessMaker/Models/AnonymousUserTest.php @@ -0,0 +1,64 @@ +app->forgetScopedInstances(); + + parent::tearDown(); + } + + public function test_resolve_returns_anonymous_user_from_database(): void + { + $user = AnonymousUser::resolve(); + + $this->assertInstanceOf(AnonymousUser::class, $user); + $this->assertSame(AnonymousUser::ANONYMOUS_USERNAME, $user->username); + } + + public function test_container_binding_returns_same_instance_within_request(): void + { + $first = app(AnonymousUser::class); + $second = app(AnonymousUser::class); + + $this->assertSame($first, $second); + } + + public function test_container_binding_is_not_reused_across_requests(): void + { + $first = app(AnonymousUser::class); + + $this->app->forgetScopedInstances(); + + $second = app(AnonymousUser::class); + + $this->assertNotSame($first, $second); + $this->assertSame($first->id, $second->id); + } + + public function test_container_binding_reflects_database_changes_after_flush(): void + { + $original = app(AnonymousUser::class); + $originalEmail = $original->email; + + User::where('username', AnonymousUser::ANONYMOUS_USERNAME) + ->update(['email' => 'updated-anon@example.com']); + + $this->app->forgetScopedInstances(); + + $refreshed = app(AnonymousUser::class); + + $this->assertSame('updated-anon@example.com', $refreshed->email); + $this->assertNotSame($originalEmail, $refreshed->email); + + User::where('username', AnonymousUser::ANONYMOUS_USERNAME) + ->update(['email' => $originalEmail]); + } +} From 9d0d012b476ddc865bbc913b1f015c9863ae448e Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 14:05:13 -0400 Subject: [PATCH 07/15] feat: refactor ResetRequestState to use RedirectToEventService for handling redirects --- ProcessMaker/Octane/ResetRequestState.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ProcessMaker/Octane/ResetRequestState.php b/ProcessMaker/Octane/ResetRequestState.php index 45071e97ad..8bde301049 100644 --- a/ProcessMaker/Octane/ResetRequestState.php +++ b/ProcessMaker/Octane/ResetRequestState.php @@ -4,14 +4,19 @@ namespace ProcessMaker\Octane; -use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\RedirectToEventService; final class ResetRequestState { + public function __construct( + private readonly RedirectToEventService $redirectToEventService + ) { + } + public function handle(): void { ProcessMakerServiceProvider::beginRequestTiming(); - HandleRedirectListener::reset(); + $this->redirectToEventService->reset(); } } From 51a44447a4482064473c01c0290c2d7c5502f583 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 14:05:22 -0400 Subject: [PATCH 08/15] test: update ResetRequestStateTest to utilize app() for dependency resolution and ensure proper redirect handling --- tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 7895d8519f..1ec4a124aa 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -13,6 +13,7 @@ use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Octane\ResetRequestState; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\RedirectToEventService; use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; @@ -24,7 +25,7 @@ public function test_it_clears_request_timing_before_the_next_request(): void $this->assertGreaterThan(0, ProcessMakerServiceProvider::getQueryTime()); - $listener = new ResetRequestState(); + $listener = app(ResetRequestState::class); $listener->handle(); $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); @@ -37,10 +38,10 @@ public function test_it_prevents_redirect_state_from_leaking_into_the_next_reque $redirectListener = new RedirectStateProbe(); $redirectListener->queue(ProcessRequest::factory()->create()); - $listener = new ResetRequestState(); + $listener = app(ResetRequestState::class); $listener->handle(); - HandleRedirectListener::sendRedirectToEvent(); + app(RedirectToEventService::class)->sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); } @@ -59,7 +60,7 @@ public function test_octane_request_termination_automatically_resets_request_sta new Response() )); - HandleRedirectListener::sendRedirectToEvent(); + app(RedirectToEventService::class)->sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); } From 0c58e630e3926e2fe8b88b40e09da3c6321ad753 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 15:28:33 -0400 Subject: [PATCH 09/15] test: enhance ResetRequestStateTest with query duration tracking and error response handling --- .../Octane/ResetRequestStateTest.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 7895d8519f..22e50f8959 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -4,6 +4,7 @@ namespace Tests\Unit\ProcessMaker\Octane; +use Illuminate\Database\Events\QueryExecuted; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; @@ -18,6 +19,20 @@ class ResetRequestStateTest extends TestCase { + protected function tearDown(): void + { + ProcessMakerServiceProvider::beginRequestTiming(); + + parent::tearDown(); + } + + private function recordQueryDuration(float $milliseconds): void + { + $connection = DB::connection(); + + event(new QueryExecuted('SELECT 1', [], $milliseconds, $connection)); + } + public function test_it_clears_request_timing_before_the_next_request(): void { DB::select('SELECT 1'); @@ -49,6 +64,13 @@ public function test_octane_request_termination_automatically_resets_request_sta { Event::fake([RedirectToEvent::class]); + ProcessMakerServiceProvider::beginRequestTiming(); + $this->recordQueryDuration(5000); + + $firstRequestQueryTime = ProcessMakerServiceProvider::getQueryTime(); + + $this->assertSame(5000.0, $firstRequestQueryTime); + $redirectListener = new RedirectStateProbe(); $redirectListener->queue(ProcessRequest::factory()->create()); @@ -59,9 +81,33 @@ public function test_octane_request_termination_automatically_resets_request_sta new Response() )); + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + HandleRedirectListener::sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); + + $this->recordQueryDuration(10); + + $nextRequestQueryTime = ProcessMakerServiceProvider::getQueryTime(); + + $this->assertSame(10.0, $nextRequestQueryTime); + } + + public function test_octane_request_termination_resets_timing_after_an_error_response(): void + { + DB::select('SELECT 1'); + + $this->assertGreaterThan(0, ProcessMakerServiceProvider::getQueryTime()); + + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/failed-request'), + new Response(status: 500) + )); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); } } From ee0318330474acba38a450dad8f546072a3b55b1 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 15:28:42 -0400 Subject: [PATCH 10/15] test: add query duration tracking and isolate metrics in ServerTimingMiddlewareTest --- tests/Feature/ServerTimingMiddlewareTest.php | 67 ++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/Feature/ServerTimingMiddlewareTest.php b/tests/Feature/ServerTimingMiddlewareTest.php index 040401d227..9654a44854 100644 --- a/tests/Feature/ServerTimingMiddlewareTest.php +++ b/tests/Feature/ServerTimingMiddlewareTest.php @@ -2,8 +2,11 @@ namespace Tests\Feature; +use Illuminate\Database\Events\QueryExecuted; +use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Route; +use Laravel\Octane\ApplicationGateway; use ProcessMaker\Http\Middleware\ServerTimingMiddleware; use ProcessMaker\Models\User; use ProcessMaker\Providers\ProcessMakerServiceProvider; @@ -15,6 +18,13 @@ class ServerTimingMiddlewareTest extends TestCase { use RequestHelper; + protected function tearDown(): void + { + ProcessMakerServiceProvider::beginRequestTiming(); + + parent::tearDown(); + } + private function getHeader($response, $header) { $headers = $response->headers->all(); @@ -22,6 +32,24 @@ private function getHeader($response, $header) return $headers[$header]; } + private function getMetricDuration($response, string $metric): float + { + $serverTiming = implode(',', $this->getHeader($response, 'server-timing')); + + preg_match('/(?:^|,)\\s*' . preg_quote($metric, '/') . ';dur=([\\d.]+)/', $serverTiming, $matches); + + $this->assertArrayHasKey(1, $matches, "The {$metric} metric was not present in the Server-Timing header."); + + return (float) $matches[1]; + } + + private function recordQueryDuration(float $milliseconds): void + { + $connection = DB::connection(); + + event(new QueryExecuted('SELECT 1', [], $milliseconds, $connection)); + } + public function testServerTimingHeaderIncludesAllMetrics() { Route::middleware(ServerTimingMiddleware::class)->get('/test', function () { @@ -85,6 +113,45 @@ public function testQueryTimeIsMeasured() $this->assertGreaterThanOrEqual(200, (float) $dbTime); } + public function testOctaneGatewayIsolatesQueryTimingAcrossConsecutiveRequests() + { + Route::middleware(ServerTimingMiddleware::class)->get('/octane-query/slow', function () { + $this->recordQueryDuration(5000); + + return response()->json(['request' => 'slow']); + }); + + Route::middleware(ServerTimingMiddleware::class)->get('/octane-query/fast', function () { + $this->recordQueryDuration(10); + + return response()->json(['request' => 'fast']); + }); + + $gateway = new ApplicationGateway($this->app, $this->app); + + $firstRequest = Request::create('/octane-query/slow'); + $firstResponse = $gateway->handle($firstRequest); + $firstRequestQueryTime = $this->getMetricDuration($firstResponse, 'db'); + + $this->assertGreaterThanOrEqual(5000, $firstRequestQueryTime); + + $gateway->terminate($firstRequest, $firstResponse); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + + $secondRequest = Request::create('/octane-query/fast'); + $secondResponse = $gateway->handle($secondRequest); + $secondRequestQueryTime = $this->getMetricDuration($secondResponse, 'db'); + + $this->assertGreaterThanOrEqual(10, $secondRequestQueryTime); + $this->assertLessThan(5000, $secondRequestQueryTime); + $this->assertLessThan($firstRequestQueryTime, $secondRequestQueryTime); + + $gateway->terminate($secondRequest, $secondResponse); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + } + public function testServiceProviderTimeIsMeasured() { // Mock a route From fde54105434211e654b837acfc9e25ad56d4625a Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 15:40:33 -0400 Subject: [PATCH 11/15] =?UTF-8?q?FOUR-32498=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"addons"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Fix Octane state leak by moving controller addons from static trait property to singleton registry Replace `HasControllerAddons` static `$addons` with `ControllerAddonsRegistry` to prevent addon accumulation across Octane requests while keeping the existing `registerAddon()` and `getPluginAddons()` API unchanged. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32498 --- .../Managers/ControllerAddonsRegistry.php | 45 ++++++ .../Providers/ProcessMakerServiceProvider.php | 4 + ProcessMaker/Traits/HasControllerAddons.php | 29 +--- .../Managers/ControllerAddonsRegistryTest.php | 136 ++++++++++++++++++ 4 files changed, 189 insertions(+), 25 deletions(-) create mode 100644 ProcessMaker/Managers/ControllerAddonsRegistry.php create mode 100644 tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php diff --git a/ProcessMaker/Managers/ControllerAddonsRegistry.php b/ProcessMaker/Managers/ControllerAddonsRegistry.php new file mode 100644 index 0000000000..2159b56523 --- /dev/null +++ b/ProcessMaker/Managers/ControllerAddonsRegistry.php @@ -0,0 +1,45 @@ +addons[] = $config; + } + + /** + * Get configured addons for a controller. + */ + public function getAddons(string $scope, string $method, array $data): array + { + $addons = []; + + foreach ($this->addons as $addon) { + if ($addon['method'] !== $method || $addon['scope'] !== $scope) { + continue; + } + + if (isset($addon['data']) && is_callable($addon['data'])) { + $data = call_user_func($addon['data'], $data); + } + + $addon['content'] = isset($addon['view']) && !isset($addon['content']) + ? view($addon['view'], $data)->render() + : (isset($addon['content']) ? $addon['content'] : ''); + $addon['script'] = isset($addon['script']) && is_string($addon['script']) + ? view($addon['script'], $data)->render() + : ''; + $addons[] = $addon; + } + + return $addons; + } +} diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 55ef2cfa55..04d81fbeda 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -136,6 +136,10 @@ public function register(): void return new Managers\LoginManager(); }); + $this->app->singleton(Managers\ControllerAddonsRegistry::class, function () { + return new Managers\ControllerAddonsRegistry(); + }); + /* * Maps our Index Manager as a singleton. The Index Manager is used * to manage customizations to the search indexer. diff --git a/ProcessMaker/Traits/HasControllerAddons.php b/ProcessMaker/Traits/HasControllerAddons.php index 0889ac1d0d..374bb9c36d 100644 --- a/ProcessMaker/Traits/HasControllerAddons.php +++ b/ProcessMaker/Traits/HasControllerAddons.php @@ -2,10 +2,10 @@ namespace ProcessMaker\Traits; +use ProcessMaker\Managers\ControllerAddonsRegistry; + trait HasControllerAddons { - private static $addons = []; - /** * Get configured addons for this controller * @@ -16,26 +16,7 @@ trait HasControllerAddons */ protected function getPluginAddons($method, array $data) { - if (!isset(static::$addons)) { - return; - } - - $addons = []; - foreach (static::$addons as $addon) { - // The addon must have the requested method and must be associated to the current controller - if ($addon['method'] === $method && $addon['scope'] === get_class($this)) { - if (isset($addon['data']) && is_callable($addon['data'])) { - $data = call_user_func($addon['data'], $data); - } - $addon['content'] = isset($addon['view']) && !isset($addon['content']) - ? view($addon['view'], $data)->render() : (isset($addon['content']) - ? $addon['content'] : ''); - $addon['script'] = isset($addon['script']) && is_string($addon['script']) ? view($addon['script'], $data)->render() : ''; - $addons[] = $addon; - } - } - - return $addons; + return app(ControllerAddonsRegistry::class)->getAddons(static::class, $method, $data); } /** @@ -47,8 +28,6 @@ protected function getPluginAddons($method, array $data) */ public static function registerAddon(array $config) { - // Add the controller to which the addon is attached - $config['scope'] = static::class; - static::$addons[] = $config; + app(ControllerAddonsRegistry::class)->register(static::class, $config); } } diff --git a/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php b/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php new file mode 100644 index 0000000000..bc5d018391 --- /dev/null +++ b/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php @@ -0,0 +1,136 @@ +registry = new ControllerAddonsRegistry(); + $this->bindRegistryInContainer(); + } + + protected function tearDown(): void + { + Container::setInstance($this->previousContainer); + + parent::tearDown(); + } + + public function test_register_addon_is_retrieved_for_matching_scope_and_method(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'test-addon', + 'method' => 'edit', + 'title' => 'Test Addon', + 'content' => 'addon-content', + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('test-addon', $addons[0]['id']); + $this->assertSame('addon-content', $addons[0]['content']); + } + + public function test_addons_from_other_controllers_are_not_returned(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'user-addon', + 'method' => 'edit', + 'content' => 'user-content', + ]); + $this->registry->register('Other\\Controller', [ + 'id' => 'other-addon', + 'method' => 'edit', + 'content' => 'other-content', + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('user-addon', $addons[0]['id']); + } + + public function test_get_plugin_addons_does_not_mutate_registered_addons(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'test-addon', + 'method' => 'edit', + 'content' => 'original-content', + ]); + + $this->registry->getAddons(UserController::class, 'edit', []); + $this->registry->getAddons(UserController::class, 'edit', []); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('original-content', $addons[0]['content']); + } + + public function test_register_addon_static_method_delegates_to_registry(): void + { + UserController::registerAddon([ + 'id' => 'static-addon', + 'method' => 'edit.settings', + 'content' => 'settings-content', + ]); + + $controller = new UserController(); + $addons = $this->invokeGetPluginAddons($controller, 'edit.settings', []); + + $this->assertCount(1, $addons); + $this->assertSame('static-addon', $addons[0]['id']); + } + + public function test_callable_data_modifier_is_applied_when_resolving_addons(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'callable-addon', + 'method' => 'edit', + 'content' => 'content', + 'data' => fn (array $data) => array_merge($data, ['extra' => 'value']), + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', ['base' => 'data']); + + $this->assertCount(1, $addons); + } + + private function bindRegistryInContainer(): void + { + $this->previousContainer = Container::getInstance(); + + $container = new Container(); + $container->singleton(ControllerAddonsRegistry::class, fn () => $this->registry); + Container::setInstance($container); + + if (!function_exists('app')) { + require_once dirname(__DIR__, 4) . '/vendor/laravel/framework/src/Illuminate/Foundation/helpers.php'; + } + } + + /** + * @return array> + */ + private function invokeGetPluginAddons(object $controller, string $method, array $data): array + { + $reflection = new \ReflectionMethod($controller, 'getPluginAddons'); + + return $reflection->invoke($controller, $method, $data); + } +} From 3392225cdb1573786efc66df371c4c200758c7d9 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 18:13:25 -0400 Subject: [PATCH 12/15] test: enhance HandleRedirectListenerTest to utilize RedirectToEventService for improved redirect handling and state management --- .../Listeners/HandleRedirectListenerTest.php | 285 ++++++++---------- 1 file changed, 131 insertions(+), 154 deletions(-) diff --git a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php index 0e3e86cf4e..9f6ff1ed13 100644 --- a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php +++ b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php @@ -4,21 +4,28 @@ namespace Tests\Unit\ProcessMaker\Listeners; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\Event; +use Laravel\Octane\Events\RequestTerminated; +use Mockery; use ProcessMaker\Events\RedirectToEvent; use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Octane\ResetRequestState; -use ReflectionProperty; +use ProcessMaker\Services\RedirectToEventService; +use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; class HandleRedirectListenerTest extends TestCase { /** - * Create a test subclass that exposes the protected setRedirectTo method. + * Create a test listener that exposes the protected setRedirectTo method. */ - private function createProbe(): HandleRedirectListener + private function createProbe(?RedirectToEventService $service = null): HandleRedirectListener { - return new class extends HandleRedirectListener { + $service ??= app(RedirectToEventService::class); + + return new class ($service) extends HandleRedirectListener { public function queue(ProcessRequest $processRequest, string $method, ...$params): void { $this->setRedirectTo($processRequest, $method, ...$params); @@ -26,210 +33,180 @@ public function queue(ProcessRequest $processRequest, string $method, ...$params }; } - /** - * Read a private static property from HandleRedirectListener. - */ - private function readStaticProperty(string $property): mixed - { - $reflection = new ReflectionProperty(HandleRedirectListener::class, $property); - $reflection->setAccessible(true); - - return $reflection->getValue(); - } - - /** - * Assert that all 3 static properties are in their default/clean state. - */ - private function assertStateIsClean(): void - { - $this->assertNull($this->readStaticProperty('processRequest')); - $this->assertSame('', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([], $this->readStaticProperty('redirectionParams')); - } - - /** - * Test that reset() clears the static $processRequest property. - */ public function test_reset_clears_process_request(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated'); - - HandleRedirectListener::reset(); - - $this->assertNull($this->readStaticProperty('processRequest')); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); + $staleRequest = ProcessRequest::factory()->create(); + $currentRequest = ProcessRequest::factory()->create(); + + $probe->queue($staleRequest, 'staleRedirect'); + $service->reset(); + $probe->queue($currentRequest, 'currentRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($currentRequest) { + return $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $currentRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * Test that reset() clears the static $redirectionMethod property. - */ public function test_reset_clears_redirection_method(): void { - $probe = $this->createProbe(); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processCompletedRedirect'); - HandleRedirectListener::reset(); + $probe->queue($request, 'staleRedirect'); + $service->reset(); + $probe->queue($request, 'currentRedirect'); + $service->sendRedirectToEvent(); - $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + Event::assertDispatched( + RedirectToEvent::class, + fn (RedirectToEvent $event) => $event->method === 'currentRedirect' + ); } - /** - * Test that reset() clears the static $redirectionParams property. - */ public function test_reset_clears_redirection_params(): void { - $probe = $this->createProbe(); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['key' => 'value']); - HandleRedirectListener::reset(); + $probe->queue($request, 'processUpdated', ['secret' => 'stale']); + $service->reset(); + $probe->queue($request, 'processUpdated', ['tokenId' => 222]); + $service->sendRedirectToEvent(); - $this->assertSame([], $this->readStaticProperty('redirectionParams')); + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) { + return $event->params[0] === ['tokenId' => 222] + && !array_key_exists('secret', $event->params[0]); + }); } - /** - * Critical test for Octane: verify that reset() prevents data leaks. - * After reset(), the stored redirect data should be gone. - */ public function test_reset_prevents_stale_redirect_from_leaking(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['tokenId' => 123]); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue( + ProcessRequest::factory()->create(), + 'processUpdated', + ['tokenId' => 123] + ); - // Simulate Octane reset between requests - HandleRedirectListener::reset(); + $service->reset(); + $service->sendRedirectToEvent(); - // sendRedirectToEvent should NOT dispatch RedirectToEvent after reset - $this->expectNotToPerformAssertions(); - HandleRedirectListener::sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); } - /** - * Test that reset() can be called multiple times safely. - */ public function test_reset_can_be_called_multiple_times(): void { - HandleRedirectListener::reset(); - HandleRedirectListener::reset(); - HandleRedirectListener::reset(); + Event::fake([RedirectToEvent::class]); - // Should not throw any errors - $this->assertStateIsClean(); + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue(ProcessRequest::factory()->create(), 'processUpdated'); + + $service->reset(); + $service->reset(); + $service->reset(); + $service->sendRedirectToEvent(); + + Event::assertNotDispatched(RedirectToEvent::class); } - /** - * Test that sendRedirectToEvent dispatches the event and clears state. - */ public function test_send_redirect_to_event_dispatches_and_clears_state(): void { - \Illuminate\Support\Facades\Event::fake([RedirectToEvent::class]); - - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated'); + Event::fake([RedirectToEvent::class]); - HandleRedirectListener::sendRedirectToEvent(); + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue(ProcessRequest::factory()->create(), 'processUpdated'); - // Assert the event was dispatched - \Illuminate\Support\Facades\Event::assertDispatched(RedirectToEvent::class); + $service->sendRedirectToEvent(); + $service->sendRedirectToEvent(); - // After dispatch, the state should be cleared - $this->assertNull($this->readStaticProperty('processRequest')); + Event::assertDispatched( + RedirectToEvent::class, + fn (RedirectToEvent $event) => $event->method === 'processUpdated' + ); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * CRITICAL: Simulate the full Octane request cycle to guarantee no data leak. - * - * Flow: - * 1. Request A stores data with different values - * 2. Reset (simulating Octane's RequestTerminated event) - * 3. Verify ALL 3 properties are clean - * 4. Request B stores NEW data with different values - * 5. Verify Request B's data is correct (not contaminated by Request A) - * 6. Reset again - * 7. Verify clean again - */ public function test_full_octane_cycle_guarantees_no_data_leak(): void { - // === Request A === + Event::fake([RedirectToEvent::class]); + $requestA = ProcessRequest::factory()->create(); - $probeA = $this->createProbe(); - $probeA->queue($requestA, 'processCompletedRedirect', ['tokenA' => 111]); + $scopeA = app(RedirectToEventService::class); + $this->createProbe($scopeA)->queue( + $requestA, + 'processCompletedRedirect', + ['tokenA' => 111] + ); - // Verify Request A data is stored (setRedirectTo uses ...$params, so it's nested) - $this->assertSame($requestA->getKey(), $this->readStaticProperty('processRequest')->getKey()); - $this->assertSame('processCompletedRedirect', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([['tokenA' => 111]], $this->readStaticProperty('redirectionParams')); + app(ResetRequestState::class)->handle(); + $scopeA->sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); - // === Octane reset after Request A === - HandleRedirectListener::reset(); + app()->forgetScopedInstances(); - // === Verify ALL properties are clean after reset === - $this->assertStateIsClean(); + $scopeB = app(RedirectToEventService::class); + $this->assertNotSame($scopeA, $scopeB); - // === Request B (simulating a DIFFERENT user/request) === $requestB = ProcessRequest::factory()->create(); - $probeB = $this->createProbe(); - $probeB->queue($requestB, 'processUpdated', ['tokenB' => 222, 'userId' => 999]); - - // Verify Request B's data is correct (NOT contaminated by Request A) - $this->assertSame($requestB->getKey(), $this->readStaticProperty('processRequest')->getKey()); - $this->assertSame('processUpdated', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([['tokenB' => 222, 'userId' => 999]], $this->readStaticProperty('redirectionParams')); - - // Verify Request A's data is GONE (no leak) - $this->assertNotSame($requestA->getKey(), $this->readStaticProperty('processRequest')?->getKey()); - - // === Octane reset after Request B === - HandleRedirectListener::reset(); - - // === Verify clean again === - $this->assertStateIsClean(); + $this->createProbe($scopeB)->queue( + $requestB, + 'processUpdated', + ['tokenB' => 222, 'userId' => 999] + ); + $scopeB->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($requestB) { + return $event->method === 'processUpdated' + && $event->params[0] === ['tokenB' => 222, 'userId' => 999] + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $requestB->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * CRITICAL: Verify that ResetRequestState orchestrator triggers the reset correctly. - */ - public function test_reset_request_state_triggers_handle_redirect_reset(): void + public function test_reset_request_state_triggers_redirect_service_reset(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['tokenId' => 456]); - - // Verify state is dirty before reset - $this->assertNotNull($this->readStaticProperty('processRequest')); - - // Execute the orchestrator (same as Octane's RequestTerminated listener) - $resetState = new ResetRequestState(); - $resetState->handle(); + $service = Mockery::mock(RedirectToEventService::class); + $service->shouldReceive('reset')->once(); - // Verify orchestrator cleaned everything - $this->assertStateIsClean(); + (new ResetRequestState($service))->handle(); } - /** - * CRITICAL: Simulate the scenario where sendRedirectToEvent() fails, - * but reset() still cleans up (edge case in Octane). - */ - public function test_reset_cleans_up_even_when_send_redirect_fails(): void + public function test_octane_termination_cleans_up_when_redirect_is_never_sent(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['data' => 'sensitive']); + Event::fake([RedirectToEvent::class]); - // Simulate that sendRedirectToEvent is NEVER called (e.g., error in BPMN flow) - // But Octane's RequestTerminated event still fires and calls reset() + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue( + ProcessRequest::factory()->create(), + 'processUpdated', + ['data' => 'sensitive'] + ); - // This should NOT be called in this scenario: - // HandleRedirectListener::sendRedirectToEvent(); + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/first-request'), + new Response() + )); - // Octane reset still happens - HandleRedirectListener::reset(); + app(RedirectToEventService::class)->sendRedirectToEvent(); - // Verify no sensitive data leaked - $this->assertStateIsClean(); + Event::assertNotDispatched(RedirectToEvent::class); } } From 743dde4fc51c6ef225ab3cb86e199be9ea4cb055 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 11:12:26 -0400 Subject: [PATCH 13/15] feat: update SettingObserver to use instance property for artisan cache refresh flag --- ProcessMaker/Observers/SettingObserver.php | 6 +++--- ProcessMaker/Providers/ProcessMakerServiceProvider.php | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Observers/SettingObserver.php b/ProcessMaker/Observers/SettingObserver.php index e4144ab3fb..1a5a6ed240 100644 --- a/ProcessMaker/Observers/SettingObserver.php +++ b/ProcessMaker/Observers/SettingObserver.php @@ -10,7 +10,7 @@ class SettingObserver { - private static $added_refresh_artisan_caches = false; + private bool $addedRefreshArtisanCaches = false; /** * Handle the setting "created" event. @@ -95,7 +95,7 @@ private function invalidateSettingCache(Setting $setting) // Check to see if we already added the refresh to the app's terminating queue. // This is important for install commands when multiple settings are being created/updated. - if (self::$added_refresh_artisan_caches) { + if ($this->addedRefreshArtisanCaches) { return; } @@ -106,6 +106,6 @@ private function invalidateSettingCache(Setting $setting) RefreshArtisanCaches::dispatchSync(); }); - self::$added_refresh_artisan_caches = true; + $this->addedRefreshArtisanCaches = true; } } diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index fe32f28b43..a67e07bb52 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -200,6 +200,8 @@ public function register(): void return Models\AnonymousUser::resolve(); }); + $this->app->scoped(Observers\SettingObserver::class); + $this->app->singleton(PolicyExtension::class, function ($app) { return new PolicyExtension(); }); From c6fee3d1aee54acf9ed5d8d0738f9688ca2cb0dd Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 11:44:36 -0400 Subject: [PATCH 14/15] test: add unit tests to validate callback scheduling and cache invalidation --- .../Observers/SettingObserverTest.php | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 tests/unit/ProcessMaker/Observers/SettingObserverTest.php diff --git a/tests/unit/ProcessMaker/Observers/SettingObserverTest.php b/tests/unit/ProcessMaker/Observers/SettingObserverTest.php new file mode 100644 index 0000000000..612d15c3b4 --- /dev/null +++ b/tests/unit/ProcessMaker/Observers/SettingObserverTest.php @@ -0,0 +1,145 @@ +app->forgetScopedInstances(); + + $observer = app(SettingObserver::class); + $callbackCount = $this->terminatingCallbackCount(); + + $observer->saving($this->setting('first-setting')); + $observer->saving($this->setting('second-setting')); + $observer->deleted($this->setting('third-setting')); + + $this->assertSame($observer, app(SettingObserver::class)); + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_octane_termination_allows_a_refresh_callback_in_the_next_scope(): void + { + $this->app->forgetScopedInstances(); + + $firstObserver = app(SettingObserver::class); + $firstObserver->saving($this->setting('first-request-setting')); + $callbackCount = $this->terminatingCallbackCount(); + + (new FlushTemporaryContainerInstances())->handle(new RequestTerminated( + $this->app, + $this->app, + Request::create('/first-request'), + new Response() + )); + + $secondObserver = app(SettingObserver::class); + $secondObserver->saving($this->setting('second-request-setting')); + + $this->assertNotSame($firstObserver, $secondObserver); + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_eloquent_saves_updates_and_deletes_share_the_scoped_observer(): void + { + $this->app->forgetScopedInstances(); + $callbackCount = $this->terminatingCallbackCount(); + + $firstSetting = Setting::factory()->create([ + 'key' => 'four-32505-first-setting', + 'config' => 'first value', + 'format' => 'text', + ]); + Setting::factory()->create([ + 'key' => 'four-32505-second-setting', + 'config' => 'second value', + 'format' => 'text', + ]); + + $firstSetting->config = 'updated value'; + $firstSetting->save(); + $firstSetting->delete(); + + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_it_invalidates_every_setting_cache_entry_while_debouncing_the_refresh(): void + { + $this->app->forgetScopedInstances(); + + $observer = app(SettingObserver::class); + $settingCache = SettingCacheFactory::getSettingsCache(); + $settings = [ + $this->setting('cached-first-setting'), + $this->setting('cached-second-setting'), + $this->setting('cached-deleted-setting'), + ]; + + foreach ($settings as $setting) { + $settingCache->set($settingCache->createKey(['key' => $setting->key]), 'cached value'); + } + + $callbackCount = $this->terminatingCallbackCount(); + $observer->saving($settings[0]); + $observer->saving($settings[1]); + $observer->deleted($settings[2]); + + foreach ($settings as $setting) { + $this->assertTrue($settingCache->missing( + $settingCache->createKey(['key' => $setting->key]) + )); + } + + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_the_terminating_callback_dispatches_the_refresh_job_synchronously(): void + { + Bus::fake([RefreshArtisanCaches::class]); + $this->app->forgetScopedInstances(); + + $callbackCount = $this->terminatingCallbackCount(); + app(SettingObserver::class)->saving($this->setting('refresh-job-setting')); + + Bus::assertNotDispatched(RefreshArtisanCaches::class); + + $callbacks = $this->terminatingCallbacks(); + $this->app->call($callbacks[$callbackCount]); + + Bus::assertDispatchedSyncTimes(RefreshArtisanCaches::class, 1); + } + + private function setting(string $key): Setting + { + return new Setting([ + 'key' => $key, + 'config' => 'value', + 'format' => 'text', + ]); + } + + private function terminatingCallbackCount(): int + { + return count($this->terminatingCallbacks()); + } + + private function terminatingCallbacks(): array + { + return (new ReflectionProperty(Application::class, 'terminatingCallbacks')) + ->getValue($this->app); + } +} From 96fbfd3d35cfc495ff47df746a1e860e61e4e6e7 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Mon, 3 Aug 2026 13:22:33 -0400 Subject: [PATCH 15/15] =?UTF-8?q?FOUR-32507=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"ServerTimingMiddleware"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Fix Octane state leak in ServerTimingMiddleware by reading min_package_time from config per request instead of a static property. Add tests for package timing threshold and config refresh between requests. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32507 --- .../Middleware/ServerTimingMiddleware.php | 13 ++--- tests/Feature/ServerTimingMiddlewareTest.php | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php index 2a15e84a6d..aab562465f 100644 --- a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php +++ b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php @@ -9,18 +9,10 @@ class ServerTimingMiddleware { - // Minimum time in ms to include a package in the Server-Timing header - private static $minPackageTime; - - public function __construct() - { - self::$minPackageTime = config('app.server_timing.min_package_time'); - } - /** * Handle an incoming request. * - * @param \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response) $next + * @param Closure(Request): (Response) $next */ public function handle(Request $request, Closure $next): Response { @@ -56,12 +48,13 @@ public function handle(Request $request, Closure $next): Response } $packageTimes = ProcessMakerServiceProvider::getPackageBootTiming(); + $minPackageTime = config('app.server_timing.min_package_time'); foreach ($packageTimes as $package => $timing) { $time = ($timing['end'] - $timing['start']) * 1000; // Only include packages that took more than MIN_PACKAGE_TIME ms - if ($time > self::$minPackageTime) { + if ($time > $minPackageTime) { $serverTiming[] = "{$package};dur={$time}"; } } diff --git a/tests/Feature/ServerTimingMiddlewareTest.php b/tests/Feature/ServerTimingMiddlewareTest.php index 040401d227..11ce958fd4 100644 --- a/tests/Feature/ServerTimingMiddlewareTest.php +++ b/tests/Feature/ServerTimingMiddlewareTest.php @@ -22,6 +22,11 @@ private function getHeader($response, $header) return $headers[$header]; } + private function getServerTimingHeaderValue($response): string + { + return implode(',', $this->getHeader($response, 'server-timing')); + } + public function testServerTimingHeaderIncludesAllMetrics() { Route::middleware(ServerTimingMiddleware::class)->get('/test', function () { @@ -162,6 +167,54 @@ public function testServerTimingOnLogin() $this->assertStringContainsString('db;dur=', $serverTiming[2]); } + public function testPackageTimingRespectsMinPackageTimeThreshold() + { + config([ + 'app.server_timing.enabled' => true, + 'app.server_timing.min_package_time' => 5, + ]); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-fast-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-fast-package', 0.002); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-slow-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-slow-package', 0.010); + + Route::middleware(ServerTimingMiddleware::class)->get('/package-threshold-test', function () { + return response()->json(['message' => 'Package threshold test']); + }); + + $response = $this->get('/package-threshold-test'); + $response->assertHeader('Server-Timing'); + + $serverTiming = $this->getServerTimingHeaderValue($response); + + $this->assertStringNotContainsString('foour32507-fast-package;dur=', $serverTiming); + $this->assertStringContainsString('foour32507-slow-package;dur=', $serverTiming); + } + + public function testMinPackageTimeReadsConfigPerRequest() + { + config(['app.server_timing.enabled' => true]); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-octane-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-octane-package', 0.008); + + Route::middleware(ServerTimingMiddleware::class)->get('/octane-min-package-test', function () { + return response()->json(['message' => 'Octane min package test']); + }); + + config(['app.server_timing.min_package_time' => 10]); + $responseAboveThreshold = $this->get('/octane-min-package-test'); + $serverTimingAboveThreshold = $this->getServerTimingHeaderValue($responseAboveThreshold); + $this->assertStringNotContainsString('foour32507-octane-package;dur=', $serverTimingAboveThreshold); + + config(['app.server_timing.min_package_time' => 5]); + $responseBelowThreshold = $this->get('/octane-min-package-test'); + $serverTimingBelowThreshold = $this->getServerTimingHeaderValue($responseBelowThreshold); + $this->assertStringContainsString('foour32507-octane-package;dur=', $serverTimingBelowThreshold); + } + public function testServerTimingIfIsDisabled() { config(['app.server_timing.enabled' => false]);