From b3bc1c0b489b813b13e600172b736d879d161b6c Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Thu, 30 Jul 2026 13:22:41 -0400 Subject: [PATCH 1/2] 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 2ff685cd1be44a5ea7082c9e61c1402e9a9e682f Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Fri, 31 Jul 2026 10:55:30 -0400 Subject: [PATCH 2/2] 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(); + } +}