From 89b4f8c4ece8468efceb3ac1a8e87909954abd90 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Fri, 17 Apr 2026 07:27:07 -0500 Subject: [PATCH 01/10] fix: ensure cookie attributes are correctly set for HttpOnly and Secure flags --- src/Session/Cookie.php | 4 ++-- tests/Feature/Session/SessionMiddlewareTest.php | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Session/Cookie.php b/src/Session/Cookie.php index fade3a81..266b9a59 100644 --- a/src/Session/Cookie.php +++ b/src/Session/Cookie.php @@ -24,11 +24,11 @@ public function build(): CookieAttributes ->withPath($this->config->path()); if ($this->config->httpOnly()) { - $cookieAttributes->withHttpOnly(); + $cookieAttributes = $cookieAttributes->withHttpOnly(); } if ($this->config->secure()) { - $cookieAttributes->withSecure(); + $cookieAttributes = $cookieAttributes->withSecure(); } return $cookieAttributes; diff --git a/tests/Feature/Session/SessionMiddlewareTest.php b/tests/Feature/Session/SessionMiddlewareTest.php index 870a3b52..36f04207 100644 --- a/tests/Feature/Session/SessionMiddlewareTest.php +++ b/tests/Feature/Session/SessionMiddlewareTest.php @@ -39,6 +39,9 @@ ->assertOk(); expect($response->getHeaders())->toHaveKey('set-cookie'); + expect($response->getHeader('set-cookie')) + ->toContain('HttpOnly') + ->toContain('SameSite=Lax'); $this->get(path: '/name', headers: ['Cookie' => $response->getHeader('set-cookie')]) ->assertOk(); @@ -61,6 +64,9 @@ ->assertOk(); expect($response->getHeaders())->toHaveKey('set-cookie'); + expect($response->getHeader('set-cookie')) + ->toContain('HttpOnly') + ->toContain('Secure'); }); it('initializes the session middleware with redis driver', function () { From bcb4d74b0681765461dd04a153bffeabc944856d Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Fri, 17 Apr 2026 07:34:04 -0500 Subject: [PATCH 02/10] fix: implement task restoration with validation for serialized payloads in DatabaseQueue and RedisQueue --- src/Queue/DatabaseQueue.php | 7 +++++- src/Queue/Queue.php | 29 ++++++++++++++++++++++ src/Queue/RedisQueue.php | 6 ++++- tests/Unit/Queue/DatabaseQueueTest.php | 34 ++++++++++++++++++++++++++ tests/Unit/Queue/RedisQueueTest.php | 24 ++++++++++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/Queue/DatabaseQueue.php b/src/Queue/DatabaseQueue.php index f98c6605..6ed7e2d8 100644 --- a/src/Queue/DatabaseQueue.php +++ b/src/Queue/DatabaseQueue.php @@ -83,7 +83,12 @@ public function pop(string|null $queueName = null): QueuableTask|null return null; } - $task = unserialize($queuedTask['payload']); + $task = $this->restoreTask($queuedTask['payload']); + + if ($task === null) { + return null; + } + $task->setTaskId($queuedTask['id']); $task->setAttempts($queuedTask['attempts']); diff --git a/src/Queue/Queue.php b/src/Queue/Queue.php index e4a43ea4..a7788a3e 100644 --- a/src/Queue/Queue.php +++ b/src/Queue/Queue.php @@ -7,6 +7,12 @@ use Phenix\Queue\Contracts\Queue as QueueContract; use Phenix\Queue\Contracts\TaskState; use Phenix\Tasks\QueuableTask; +use Throwable; + +use function class_exists; +use function is_subclass_of; +use function preg_match; +use function unserialize; abstract class Queue implements QueueContract { @@ -75,4 +81,27 @@ public function getStateManager(): TaskState { return $this->stateManager; } + + protected function restoreTask(string $payload): QueuableTask|null + { + if (preg_match('/^O:\d+:"([^"]+)":/', $payload, $matches) !== 1) { + return null; + } + + $class = $matches[1]; + + if (! class_exists($class) || ! is_subclass_of($class, QueuableTask::class)) { + return null; + } + + try { + $task = unserialize($payload, ['allowed_classes' => [$class]]); + + $queuableTask = $task instanceof QueuableTask ? $task : null; + } catch (Throwable) { + $queuableTask = null; + } + + return $queuableTask; + } } diff --git a/src/Queue/RedisQueue.php b/src/Queue/RedisQueue.php index 8fec38db..f356bed3 100644 --- a/src/Queue/RedisQueue.php +++ b/src/Queue/RedisQueue.php @@ -56,7 +56,11 @@ public function pop(string|null $queueName = null): QueuableTask|null return null; } - $task = unserialize($payload); + $task = $this->restoreTask($payload); + + if ($task === null) { + return null; + } $task->setQueueName($queueName ?? $this->queueName ?? 'default'); diff --git a/tests/Unit/Queue/DatabaseQueueTest.php b/tests/Unit/Queue/DatabaseQueueTest.php index fc3e99d8..d24d61bc 100644 --- a/tests/Unit/Queue/DatabaseQueueTest.php +++ b/tests/Unit/Queue/DatabaseQueueTest.php @@ -182,6 +182,40 @@ expect($task)->not->toBeNull(); }); +it('rejects unauthorized serialized payloads', function (): void { + $connection = $this->getMockBuilder(MysqlConnectionPool::class)->getMock(); + $transaction = $this->getMockBuilder(SqlTransaction::class)->getMock(); + + $databaseStatement = $this->getMockBuilder(Statement::class) + ->disableOriginalConstructor() + ->getMock(); + + $databaseStatement->expects($this->once()) + ->method('execute') + ->willReturn(new Result([ + [ + 'id' => 'unauthorized-task', + 'payload' => serialize(new stdClass()), + 'attempts' => 0, + 'reserved_at' => null, + 'available_at' => (new DateTime())->format('Y-m-d H:i:s'), + 'created_at' => (new DateTime())->format('Y-m-d H:i:s'), + ], + ])); + + $transaction->expects($this->once()) + ->method('prepare') + ->willReturn($databaseStatement); + + $connection->expects($this->once()) + ->method('beginTransaction') + ->willReturn($transaction); + + $this->app->swap(Connection::default(), $connection); + + expect(Queue::pop())->toBeNull(); +}); + it('returns null when no queued task exists', function (): void { $connection = $this->getMockBuilder(MysqlConnectionPool::class)->getMock(); $transaction = $this->getMockBuilder(SqlTransaction::class)->getMock(); diff --git a/tests/Unit/Queue/RedisQueueTest.php b/tests/Unit/Queue/RedisQueueTest.php index 886183c1..5bdf13fc 100644 --- a/tests/Unit/Queue/RedisQueueTest.php +++ b/tests/Unit/Queue/RedisQueueTest.php @@ -113,6 +113,30 @@ expect($task)->toBeInstanceOf(BasicQueuableTask::class); }); +it('rejects unauthorized serialized payloads', function (): void { + $clientMock = $this->getMockBuilder(ClientWrapper::class) + ->disableOriginalConstructor() + ->getMock(); + + $clientMock->expects($this->once()) + ->method('execute') + ->with( + $this->equalTo('EVAL'), + $this->isType('string'), + $this->equalTo(3), + $this->equalTo('queues:default'), + $this->equalTo('queues:failed'), + $this->equalTo('queues:delayed'), + $this->isType('int'), + $this->equalTo(60) + ) + ->willReturn(serialize(new stdClass())); + + $this->app->swap(Connection::redis('default'), $clientMock); + + expect(Queue::pop())->toBeNull(); +}); + it('returns the queue size', function (): void { $clientMock = $this->getMockBuilder(ClientWrapper::class) ->disableOriginalConstructor() From 5b54d69d3f7d430fc6ba87b1833b2b14e1312cd3 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Fri, 17 Apr 2026 07:36:46 -0500 Subject: [PATCH 03/10] fix: enhance security by validating cache payloads with MAC checks --- src/Cache/Stores/FileStore.php | 27 +++++++++++++++++++++++++-- tests/Unit/Cache/FileStoreTest.php | 23 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/Cache/Stores/FileStore.php b/src/Cache/Stores/FileStore.php index b033665e..e55b80fe 100644 --- a/src/Cache/Stores/FileStore.php +++ b/src/Cache/Stores/FileStore.php @@ -6,10 +6,12 @@ use Closure; use Phenix\Cache\CacheStore; +use Phenix\Facades\Config; use Phenix\Facades\File; use Phenix\Util\Arr; use Phenix\Util\Date; +use function hash_equals; use function is_array; class FileStore extends CacheStore @@ -31,7 +33,7 @@ public function get(string $key, Closure|null $callback = null): mixed $data = json_decode($raw, true); - if (! is_array($data) || ! Arr::has($data, ['expires_at', 'value'])) { + if (! is_array($data) || ! Arr::has($data, ['expires_at', 'value', 'mac']) || ! $this->hasValidMac($data)) { $this->delete($key); return $this->resolveCallback($key, $callback); @@ -58,6 +60,8 @@ public function set(string $key, mixed $value, Date|null $ttl = null): void 'value' => base64_encode(serialize($value)), ]; + $payload['mac'] = $this->mac($payload['value'], $expiresAt); + File::put($this->filename($key), json_encode($payload, JSON_THROW_ON_ERROR)); } @@ -68,6 +72,8 @@ public function forever(string $key, mixed $value): void 'value' => base64_encode(serialize($value)), ]; + $payload['mac'] = $this->mac($payload['value'], null); + File::put($this->filename($key), json_encode($payload, JSON_THROW_ON_ERROR)); } @@ -81,7 +87,7 @@ public function has(string $key): bool $data = json_decode($raw, true); - if (! is_array($data)) { + if (! is_array($data) || ! Arr::has($data, ['expires_at', 'value', 'mac']) || ! $this->hasValidMac($data)) { return false; } @@ -121,6 +127,23 @@ protected function filename(string $key): string return $this->path . DIRECTORY_SEPARATOR . sha1($this->prefix . $key) . '.cache'; } + protected function hasValidMac(array $data): bool + { + return hash_equals( + (string) $data['mac'], + $this->mac((string) $data['value'], $data['expires_at']) + ); + } + + protected function mac(string $value, int|string|null $expiresAt): string + { + return hash_hmac( + 'sha256', + $value . '|' . ($expiresAt ?? 'forever'), + (string) Config::get('app.key', Config::get('cache.prefix', '')) + ); + } + protected function resolveCallback(string $key, Closure|null $callback): mixed { if ($callback === null) { diff --git a/tests/Unit/Cache/FileStoreTest.php b/tests/Unit/Cache/FileStoreTest.php index af91bef7..bfa3301b 100644 --- a/tests/Unit/Cache/FileStoreTest.php +++ b/tests/Unit/Cache/FileStoreTest.php @@ -178,6 +178,29 @@ expect(Cache::has('corrupted'))->toBeTrue(); }); +it('rejects unsigned serialized cache payloads', function (): void { + $cachePath = Config::get('cache.stores.file.path'); + $prefix = Config::get('cache.prefix'); + + $filename = $cachePath . DIRECTORY_SEPARATOR . sha1("{$prefix}unsigned") . '.cache'; + + File::put($filename, json_encode([ + 'expires_at' => time() + 3600, + 'value' => base64_encode(serialize(new stdClass())), + ], JSON_THROW_ON_ERROR)); + + $callCount = 0; + + $value = Cache::get('unsigned', function () use (&$callCount): string { + $callCount++; + + return 'safe_value'; + }); + + expect($value)->toBe('safe_value'); + expect($callCount)->toBe(1); +}); + it('handles corrupted trying to check cache exists', function (): void { $cachePath = Config::get('cache.stores.file.path'); $prefix = Config::get('cache.prefix'); From 0463ae8f1cee044a0cfd030349275d37ef70902c Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Fri, 17 Apr 2026 08:20:46 -0500 Subject: [PATCH 04/10] fix: implement request body size limits and error handling for payloads --- src/Http/ExceptionHandler.php | 9 +++ src/Http/Request.php | 59 +++++++++++-------- src/Http/Requests/Concerns/HasTrailers.php | 25 ++++++++ src/Http/Requests/FormParser.php | 26 ++++++-- src/Http/Requests/JsonParser.php | 20 +++++-- src/Http/Requests/StreamParser.php | 27 +++++---- tests/Feature/RequestTest.php | 44 ++++++++++++++ tests/Feature/Requests/LimitedBodyRequest.php | 20 +++++++ .../Requests/LimitedStreamedRequest.php | 15 +++++ 9 files changed, 202 insertions(+), 43 deletions(-) create mode 100644 src/Http/Requests/Concerns/HasTrailers.php create mode 100644 tests/Feature/Requests/LimitedBodyRequest.php create mode 100644 tests/Feature/Requests/LimitedStreamedRequest.php diff --git a/src/Http/ExceptionHandler.php b/src/Http/ExceptionHandler.php index d8c2cc11..7d945d26 100644 --- a/src/Http/ExceptionHandler.php +++ b/src/Http/ExceptionHandler.php @@ -6,6 +6,7 @@ use Amp\Http\HttpStatus; use Amp\Http\Server\ExceptionHandler as ExceptionHandlerContract; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; use Amp\Http\Server\Response; use Throwable; @@ -25,6 +26,14 @@ public function handleException(Request $request, Throwable $exception): Respons 'client' => $request->getClient()->getRemoteAddress()->toString(), ]); + if ($exception instanceof HttpErrorException) { + return $this->errorHandler->handleError( + status: $exception->getStatus(), + reason: $exception->getReason(), + request: $request + ); + } + if (! $this->errorHandler->shouldExposeDebugDetails()) { return $this->errorHandler->handleError(status: HttpStatus::INTERNAL_SERVER_ERROR, request: $request); } diff --git a/src/Http/Request.php b/src/Http/Request.php index ef4c37b2..38664d10 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -11,7 +11,6 @@ use Amp\Http\Server\RequestBody; use Amp\Http\Server\Router; use Amp\Http\Server\Session\Session as ServerSession; -use Amp\Http\Server\Trailers; use League\Uri\Components\Query; use Phenix\Contracts\Arrayable; use Phenix\Http\Constants\ContentType; @@ -20,6 +19,7 @@ use Phenix\Http\Requests\Concerns\HasCookies; use Phenix\Http\Requests\Concerns\HasHeaders; use Phenix\Http\Requests\Concerns\HasQueryParameters; +use Phenix\Http\Requests\Concerns\HasTrailers; use Phenix\Http\Requests\Concerns\HasUser; use Phenix\Http\Requests\FormParser; use Phenix\Http\Requests\JsonParser; @@ -32,9 +32,12 @@ class Request implements Arrayable use HasUser; use HasHeaders; use HasCookies; + use HasTrailers; use HasQueryParameters; - protected readonly BodyParser $body; + protected const DEFAULT_BODY_SIZE_LIMIT = 120 * 1024 * 1024; + + protected BodyParser|null $body; protected readonly Query $query; @@ -60,7 +63,7 @@ public function __construct( $this->query = Query::fromUri($request->getUri()); $this->routeAttributes = new RouteAttributes($routeAttributes); - $this->body = $this->getParser(); + $this->body = null; } public function getClient(): Client @@ -83,21 +86,6 @@ public function setBody(ReadableStream|string $body): void $this->request->setBody($body); } - public function getTrailers(): Trailers|null - { - return $this->request->getTrailers(); - } - - public function setTrailers(Trailers $trailers): void - { - $this->request->setTrailers($trailers); - } - - public function removeTrailers(): void - { - $this->request->removeTrailers(); - } - public function getMethod(): string { return $this->request->getMethod(); @@ -133,11 +121,13 @@ public function query(string|null $key = null, array|string|int|null $default = public function body(string|null $key = null, array|string|int|null $default = null): BodyParser|BufferedFile|array|string|int|null { + $body = $this->bodyParser(); + if ($key) { - return $this->body->hasFile($key) ? $this->body->getFile($key) : $this->body->get($key, $default); + return $body->hasFile($key) ? $body->getFile($key) : $body->get($key, $default); } - return $this->body; + return $body; } public function session(string|null $key = null, array|string|int|null $default = null): Session|array|string|int|null @@ -156,7 +146,7 @@ public function ip(): Ip public function toArray(): array { - return $this->body->toArray(); + return $this->bodyParser()->toArray(); } protected function mode(): RequestMode @@ -164,20 +154,41 @@ protected function mode(): RequestMode return RequestMode::BUFFERED; } + protected function bodySizeLimit(): int + { + return self::DEFAULT_BODY_SIZE_LIMIT; + } + + protected function fieldCountLimit(): int|null + { + return null; + } + + protected function bodyParser(): BodyParser + { + return $this->body ??= $this->getParser(); + } + protected function getParser(): BodyParser { $contentType = ContentType::fromValue($this->request->getHeader('content-type')); if ($contentType === ContentType::JSON) { - return JsonParser::fromRequest($this->request); + return JsonParser::fromRequest($this->request, [ + 'body_size_limit' => $this->bodySizeLimit(), + ]); } if ($this->mode() === RequestMode::STREAMED) { return StreamParser::fromRequest($this->request, [ - 'body_size_limit' => 120 * 1024 * 1024, + 'body_size_limit' => $this->bodySizeLimit(), + 'field_count_limit' => $this->fieldCountLimit(), ]); } - return FormParser::fromRequest($this->request); + return FormParser::fromRequest($this->request, [ + 'body_size_limit' => $this->bodySizeLimit(), + 'field_count_limit' => $this->fieldCountLimit(), + ]); } } diff --git a/src/Http/Requests/Concerns/HasTrailers.php b/src/Http/Requests/Concerns/HasTrailers.php new file mode 100644 index 00000000..ba3d6861 --- /dev/null +++ b/src/Http/Requests/Concerns/HasTrailers.php @@ -0,0 +1,25 @@ +request->getTrailers(); + } + + public function setTrailers(Trailers $trailers): void + { + $this->request->setTrailers($trailers); + } + + public function removeTrailers(): void + { + $this->request->removeTrailers(); + } +} diff --git a/src/Http/Requests/FormParser.php b/src/Http/Requests/FormParser.php index 1722fd31..22f7ea3a 100644 --- a/src/Http/Requests/FormParser.php +++ b/src/Http/Requests/FormParser.php @@ -4,10 +4,15 @@ namespace Phenix\Http\Requests; +use Amp\ByteStream\BufferException; +use Amp\Http\HttpStatus; use Amp\Http\Server\FormParser\BufferedFile; use Amp\Http\Server\FormParser\Form; +use Amp\Http\Server\FormParser\FormParser as AmpFormParser; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; +use function Amp\Http\Server\FormParser\parseContentBoundary; use function is_array; use function is_null; use function is_numeric; @@ -16,14 +21,19 @@ class FormParser extends BodyParser { private Form|null $form; - public function __construct() - { + public function __construct( + private readonly int $bodySizeLimit = 120 * 1024 * 1024, + private readonly int|null $fieldCountLimit = null + ) { $this->form = null; } public static function fromRequest(Request $request, array $options = []): self { - $parser = new self(); + $parser = new self( + $options['body_size_limit'] ?? 120 * 1024 * 1024, + $options['field_count_limit'] ?? null + ); $parser->parse($request); return $parser; @@ -79,7 +89,15 @@ public function toArray(): array protected function parse(Request $request): self { - $this->form = Form::fromRequest($request); + $boundary = parseContentBoundary($request->getHeader('content-type') ?? ''); + + try { + $body = $boundary === null ? '' : $request->getBody()->buffer(limit: $this->bodySizeLimit); + } catch (BufferException $exception) { + throw new HttpErrorException(HttpStatus::PAYLOAD_TOO_LARGE, 'Request body is too large', $exception); + } + + $this->form = (new AmpFormParser($this->fieldCountLimit))->parseBody($body, $boundary); return $this; } diff --git a/src/Http/Requests/JsonParser.php b/src/Http/Requests/JsonParser.php index 63d2bb41..c894d672 100644 --- a/src/Http/Requests/JsonParser.php +++ b/src/Http/Requests/JsonParser.php @@ -4,7 +4,10 @@ namespace Phenix\Http\Requests; +use Amp\ByteStream\BufferException; +use Amp\Http\HttpStatus; use Amp\Http\Server\FormParser\BufferedFile; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; use function is_numeric; @@ -13,14 +16,15 @@ class JsonParser extends BodyParser { private array $body; - public function __construct() - { + public function __construct( + private readonly int $bodySizeLimit = 120 * 1024 * 1024 + ) { $this->body = []; } public static function fromRequest(Request $request, array $options = []): self { - return (new self())->parse($request); + return (new self($options['body_size_limit'] ?? 120 * 1024 * 1024))->parse($request); } public function get(string $key, array|string|int|null $default = null): array|string|int|null @@ -66,9 +70,15 @@ public function toArray(): array protected function parse(Request $request): self { - $body = json_decode($request->getBody()->read() ?? '', true); + try { + $raw = $request->getBody()->buffer(limit: $this->bodySizeLimit); + } catch (BufferException $exception) { + throw new HttpErrorException(HttpStatus::PAYLOAD_TOO_LARGE, 'Request body is too large', $exception); + } + + $body = json_decode($raw, true); - if (json_last_error() === JSON_ERROR_NONE) { + if (json_last_error() === JSON_ERROR_NONE && is_array($body)) { $this->body = $body; } diff --git a/src/Http/Requests/StreamParser.php b/src/Http/Requests/StreamParser.php index 3422b715..7b226d15 100644 --- a/src/Http/Requests/StreamParser.php +++ b/src/Http/Requests/StreamParser.php @@ -4,9 +4,12 @@ namespace Phenix\Http\Requests; +use Amp\ByteStream\BufferException; +use Amp\Http\HttpStatus; use Amp\Http\Server\FormParser\BufferedFile; use Amp\Http\Server\FormParser\StreamedField; use Amp\Http\Server\FormParser\StreamingFormParser; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; use Amp\Pipeline\ConcurrentIterator; @@ -84,21 +87,25 @@ public function toArray(): array protected function parse(Request $request): self { /** @var ConcurrentIterator $iterator */ - $iterator = $this->parser->parseForm($request); + $iterator = $this->parser->parseForm($request, $this->bodySizeLimit); while ($iterator->continue()) { /** @var StreamedField $field */ $field = $iterator->getValue(); - if ($field->isFile()) { - $this->files[$field->getName()] = new BufferedFile( - $field->getFilename(), - $field->buffer(), - $field->getMimeType(), - $field->getHeaderPairs() - ); - } else { - $this->data[$field->getName()] = $field->buffer(); + try { + if ($field->isFile()) { + $this->files[$field->getName()] = new BufferedFile( + $field->getFilename(), + $field->buffer(limit: $this->bodySizeLimit ?? PHP_INT_MAX), + $field->getMimeType(), + $field->getHeaderPairs() + ); + } else { + $this->data[$field->getName()] = $field->buffer(limit: $this->bodySizeLimit ?? PHP_INT_MAX); + } + } catch (BufferException $exception) { + throw new HttpErrorException(HttpStatus::PAYLOAD_TOO_LARGE, 'Request body is too large', $exception); } } diff --git a/tests/Feature/RequestTest.php b/tests/Feature/RequestTest.php index 125fef15..24e3a351 100644 --- a/tests/Feature/RequestTest.php +++ b/tests/Feature/RequestTest.php @@ -14,6 +14,8 @@ use Phenix\Http\Request; use Phenix\Http\Response; use Phenix\Testing\TestResponse; +use Tests\Feature\Requests\LimitedBodyRequest; +use Tests\Feature\Requests\LimitedStreamedRequest; use Tests\Unit\Routing\AcceptJsonResponses; beforeEach(function (): void { @@ -129,6 +131,48 @@ ->assertOk(); }); +it('rejects json bodies above the request body limit', function (): void { + Route::post('/limited-json', function (LimitedBodyRequest $request): Response { + return response()->json($request->toArray()); + }); + + $this->app->run(); + + $this->post( + path: '/limited-json', + body: ['payload' => str_repeat('x', 64)], + headers: ['content-type' => ContentType::JSON->value] + )->assertStatusCode(HttpStatus::PAYLOAD_TOO_LARGE); +}); + +it('rejects buffered form bodies above the request body limit', function (): void { + Route::post('/limited-form', function (LimitedBodyRequest $request): Response { + return response()->json($request->toArray()); + }); + + $this->app->run(); + + $body = new Form(); + $body->addField('payload', str_repeat('x', 64)); + + $this->post('/limited-form', $body) + ->assertStatusCode(HttpStatus::PAYLOAD_TOO_LARGE); +}); + +it('rejects streamed form bodies above the request body limit', function (): void { + Route::post('/limited-stream', function (LimitedStreamedRequest $request): Response { + return response()->json($request->toArray()); + }); + + $this->app->run(); + + $body = new Form(); + $body->addField('payload', str_repeat('x', 64)); + + $this->post('/limited-stream', $body) + ->assertStatusCode(HttpStatus::PAYLOAD_TOO_LARGE); +}); + it('responds with a view', function (): void { Route::get('/users', function (): Response { return response()->view('users.index', [ diff --git a/tests/Feature/Requests/LimitedBodyRequest.php b/tests/Feature/Requests/LimitedBodyRequest.php new file mode 100644 index 00000000..615f33a8 --- /dev/null +++ b/tests/Feature/Requests/LimitedBodyRequest.php @@ -0,0 +1,20 @@ + Date: Fri, 17 Apr 2026 08:21:07 -0500 Subject: [PATCH 05/10] tests: reject access when URL query is modified --- tests/Feature/Routing/ValidateSignatureTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Feature/Routing/ValidateSignatureTest.php b/tests/Feature/Routing/ValidateSignatureTest.php index 8387bb9b..99be4f30 100644 --- a/tests/Feature/Routing/ValidateSignatureTest.php +++ b/tests/Feature/Routing/ValidateSignatureTest.php @@ -102,3 +102,17 @@ $this->get(path: $modifiedUrl) ->assertStatusCode(HttpStatus::FORBIDDEN); }); + +it('rejects access when URL query is modified', function (): void { + Route::get('/signed/{user}', fn (): Response => response()->plain('Ok')) + ->name('signed.query') + ->middleware(ValidateSignature::class); + + $this->app->run(); + + $signedUrl = Url::signedRoute('signed.query', ['user' => 42, 'scope' => 'read']); + $modifiedUrl = str_replace('scope=read', 'scope=write', $signedUrl); + + $this->get(path: $modifiedUrl) + ->assertStatusCode(HttpStatus::FORBIDDEN); +}); From 4f9619ea9e2ee59b90ff4e9d910a8f5dce2b2e60 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 20 Apr 2026 13:21:23 +0000 Subject: [PATCH 06/10] fix: throw error on missing key --- src/Cache/Stores/FileStore.php | 11 +++++++++-- tests/Unit/Cache/FileStoreTest.php | 3 +++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Cache/Stores/FileStore.php b/src/Cache/Stores/FileStore.php index e55b80fe..6066c0c6 100644 --- a/src/Cache/Stores/FileStore.php +++ b/src/Cache/Stores/FileStore.php @@ -6,6 +6,7 @@ use Closure; use Phenix\Cache\CacheStore; +use Phenix\Crypto\Exceptions\MissingKeyException; use Phenix\Facades\Config; use Phenix\Facades\File; use Phenix\Util\Arr; @@ -137,10 +138,16 @@ protected function hasValidMac(array $data): bool protected function mac(string $value, int|string|null $expiresAt): string { + $key = Config::get('app.key'); + + if (empty($key)) { + throw new MissingKeyException('The application key is not set.'); + } + return hash_hmac( 'sha256', - $value . '|' . ($expiresAt ?? 'forever'), - (string) Config::get('app.key', Config::get('cache.prefix', '')) + "{$value}|" . ($expiresAt ?? 'forever'), + (string) $key ); } diff --git a/tests/Unit/Cache/FileStoreTest.php b/tests/Unit/Cache/FileStoreTest.php index bfa3301b..ddd19069 100644 --- a/tests/Unit/Cache/FileStoreTest.php +++ b/tests/Unit/Cache/FileStoreTest.php @@ -5,12 +5,15 @@ use Phenix\Cache\Constants\Store; use Phenix\Facades\Cache; use Phenix\Facades\Config; +use Phenix\Facades\Crypto; use Phenix\Facades\File; use Phenix\Util\Date; use function Amp\delay; beforeEach(function (): void { + Config::set('app.key', Crypto::generateEncodedKey()); + Config::set('cache.default', Store::FILE->value); Cache::clear(); From e586cf2beeeb6052b942417445bdd5640f2fde39 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 20 Apr 2026 13:25:27 +0000 Subject: [PATCH 07/10] fix: simplify return statement in pop method of DatabaseQueue --- src/Queue/DatabaseQueue.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Queue/DatabaseQueue.php b/src/Queue/DatabaseQueue.php index 6ed7e2d8..84e3cd13 100644 --- a/src/Queue/DatabaseQueue.php +++ b/src/Queue/DatabaseQueue.php @@ -92,11 +92,7 @@ public function pop(string|null $queueName = null): QueuableTask|null $task->setTaskId($queuedTask['id']); $task->setAttempts($queuedTask['attempts']); - if ($this->stateManager->reserve($task)) { - return $task; - } - - return null; + return $this->stateManager->reserve($task) ? $task : null; }); } From 3cc0603630b8e8e1d9e86b1fc66866be6a356082 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 20 Apr 2026 13:39:33 +0000 Subject: [PATCH 08/10] fix: streamline task retrieval logic in pop method of RedisQueue --- src/Queue/RedisQueue.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Queue/RedisQueue.php b/src/Queue/RedisQueue.php index f356bed3..74f77435 100644 --- a/src/Queue/RedisQueue.php +++ b/src/Queue/RedisQueue.php @@ -58,14 +58,10 @@ public function pop(string|null $queueName = null): QueuableTask|null $task = $this->restoreTask($payload); - if ($task === null) { - return null; - } - - $task->setQueueName($queueName ?? $this->queueName ?? 'default'); - - if ($this->stateManager->reserve($task)) { - return $task; + if ($task) { + $task->setQueueName($queueName ?? $this->queueName ?? 'default'); + + return $this->stateManager->reserve($task) ? $task : null; } $this->redis->execute('RPUSH', $queueKey, $payload); From 3af8467963aa816f0337b10cb39753fdb931fe00 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 20 Apr 2026 14:01:21 +0000 Subject: [PATCH 09/10] style: php cs --- src/Queue/RedisQueue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Queue/RedisQueue.php b/src/Queue/RedisQueue.php index 74f77435..244bab9b 100644 --- a/src/Queue/RedisQueue.php +++ b/src/Queue/RedisQueue.php @@ -60,7 +60,7 @@ public function pop(string|null $queueName = null): QueuableTask|null if ($task) { $task->setQueueName($queueName ?? $this->queueName ?? 'default'); - + return $this->stateManager->reserve($task) ? $task : null; } From f68112a7665cf0a5ffdf5eeb54163d3b1242cb3b Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 20 Apr 2026 14:52:31 +0000 Subject: [PATCH 10/10] fix: improve task reservation logic in pop method of RedisQueue --- src/Queue/RedisQueue.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Queue/RedisQueue.php b/src/Queue/RedisQueue.php index 244bab9b..f0738833 100644 --- a/src/Queue/RedisQueue.php +++ b/src/Queue/RedisQueue.php @@ -61,10 +61,12 @@ public function pop(string|null $queueName = null): QueuableTask|null if ($task) { $task->setQueueName($queueName ?? $this->queueName ?? 'default'); - return $this->stateManager->reserve($task) ? $task : null; - } + if ($this->stateManager->reserve($task)) { + return $task; + } - $this->redis->execute('RPUSH', $queueKey, $payload); + $this->redis->execute('RPUSH', $queueKey, $payload); + } return null; }