From 94d146f6d9f2f55749b850cb7c3e06671a170bec Mon Sep 17 00:00:00 2001 From: michalsn Date: Fri, 5 Jun 2026 20:17:46 +0200 Subject: [PATCH 1/2] fix: nested transformer request scope leakage --- system/API/BaseTransformer.php | 46 +++-- tests/_support/API/ChildTransformer.php | 31 ++++ tests/_support/API/ParentTransformer.php | 67 ++++++++ tests/system/API/TransformerTest.php | 158 ++++++++++++++++++ user_guide_src/source/changelogs/v4.7.4.rst | 1 + .../source/outgoing/api_transformers.rst | 15 ++ 6 files changed, 304 insertions(+), 14 deletions(-) create mode 100644 tests/_support/API/ChildTransformer.php create mode 100644 tests/_support/API/ParentTransformer.php diff --git a/system/API/BaseTransformer.php b/system/API/BaseTransformer.php index 540d443d1cdc..4badb8267139 100644 --- a/system/API/BaseTransformer.php +++ b/system/API/BaseTransformer.php @@ -54,6 +54,11 @@ */ abstract class BaseTransformer implements TransformerInterface { + /** + * Nesting depth of the transformation currently in progress (0 = root). + */ + private static int $depth = 0; + /** * @var list|null */ @@ -69,17 +74,24 @@ abstract class BaseTransformer implements TransformerInterface public function __construct( private ?IncomingRequest $request = null, ) { + // An explicitly provided request is always honored - its scope is + // intentional. Only the implicit global fallback is suppressed for + // nested transformers, which is the actual leak vector. + $explicitRequest = $request instanceof IncomingRequest; + $this->request = $request ?? request(); - $fields = $this->request->getGet('fields'); - $this->fields = is_string($fields) - ? array_map(trim(...), explode(',', $fields)) - : $fields; + if ($explicitRequest || self::$depth === 0) { + $fields = $this->request->getGet('fields'); + $this->fields = is_string($fields) + ? array_map(trim(...), explode(',', $fields)) + : $fields; - $includes = $this->request->getGet('include'); - $this->includes = is_string($includes) - ? array_map(trim(...), explode(',', $includes)) - : $includes; + $includes = $this->request->getGet('include'); + $this->includes = is_string($includes) + ? array_map(trim(...), explode(',', $includes)) + : $includes; + } } /** @@ -207,13 +219,19 @@ private function insertIncludes(array $data): array } } - foreach ($this->includes as $include) { - $method = 'include' . ucfirst($include); - if (method_exists($this, $method)) { - $data[$include] = $this->{$method}(); - } else { - throw ApiException::forMissingInclude($include); + self::$depth++; + + try { + foreach ($this->includes as $include) { + $method = 'include' . ucfirst($include); + if (method_exists($this, $method)) { + $data[$include] = $this->{$method}(); + } else { + throw ApiException::forMissingInclude($include); + } } + } finally { + self::$depth--; } return $data; diff --git a/tests/_support/API/ChildTransformer.php b/tests/_support/API/ChildTransformer.php new file mode 100644 index 000000000000..a226dad97608 --- /dev/null +++ b/tests/_support/API/ChildTransformer.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\API; + +use CodeIgniter\API\BaseTransformer; + +/** + * Nested transformer used to verify that the root request's scope + * (fields/includes) does not leak into related resources. + */ +class ChildTransformer extends BaseTransformer +{ + public function toArray(mixed $resource): array + { + return [ + 'child_id' => $resource['id'] ?? null, + 'status' => 'transformed', + ]; + } +} diff --git a/tests/_support/API/ParentTransformer.php b/tests/_support/API/ParentTransformer.php new file mode 100644 index 000000000000..803b4dbb353f --- /dev/null +++ b/tests/_support/API/ParentTransformer.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\API; + +use CodeIgniter\API\BaseTransformer; + +/** + * Root transformer used to verify that nested transformers do not inherit + * the root request's `fields`/`include` query state. + */ +class ParentTransformer extends BaseTransformer +{ + public function toArray(mixed $resource): array + { + return [ + 'parent_id' => $resource['id'] ?? null, + ]; + } + + /** + * Includes a single related child resource. + * + * @return array + */ + protected function includeChildren(): array + { + return (new ChildTransformer())->transform(['id' => 99]); + } + + /** + * Includes a collection of related child resources. + * + * @return list> + */ + protected function includeChildrenCollection(): array + { + return (new ChildTransformer())->transformMany([ + ['id' => 77], + ['id' => 88], + ]); + } + + /** + * Includes a single child while explicitly forwarding the request, opting + * the child into the request-derived scope even though it is nested. + * + * @return array + */ + protected function includeExplicitChild(): array + { + $childRequest = clone request(); + $childRequest->setGlobal('get', ['fields' => 'child_id']); + + return (new ChildTransformer($childRequest))->transform(['id' => 99]); + } +} diff --git a/tests/system/API/TransformerTest.php b/tests/system/API/TransformerTest.php index 8c6f50857d23..6605d47e2d6d 100644 --- a/tests/system/API/TransformerTest.php +++ b/tests/system/API/TransformerTest.php @@ -22,6 +22,8 @@ use Config\Services; use PHPUnit\Framework\Attributes\Group; use stdClass; +use Tests\Support\API\ChildTransformer; +use Tests\Support\API\ParentTransformer; /** * @internal @@ -641,4 +643,160 @@ protected function includePosts(): array $this->assertArrayHasKey('posts', $result); $this->assertSame([['id' => 1, 'title' => 'Post 1']], $result['posts']); } + + public function testNestedTransformerDoesNotInheritIncludeState(): void + { + // The child transformer has no includeChildren() method. If the root + // request's `include=children` leaked into it, transforming the child + // would raise an ApiException for the missing include method. + $request = $this->createMockRequest('include=children'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedTransformerDoesNotInheritFieldFilter(): void + { + // `fields=parent_id` applies to the root only. If it leaked into the + // child, array_intersect_key would strip every child field, leaving []. + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedCollectionTransformerDoesNotInheritState(): void + { + $request = $this->createMockRequest('include=childrenCollection&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'childrenCollection' => [ + ['child_id' => 77, 'status' => 'transformed'], + ['child_id' => 88, 'status' => 'transformed'], + ], + ], $result); + } + + public function testBareNestedInstantiationDoesNotInheritState(): void + { + // Reproduces the exact leak vector: a child created with a bare + // `new ChildTransformer()` (no request passed) inside an include + // method must not pick up the root request's scope from request(). + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $root = new ParentTransformer(); + + $result = $root->transform(['id' => 5]); + + $this->assertSame([ + 'parent_id' => 5, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedTransformerHonorsExplicitRequest(): void + { + // A child created with an explicitly passed request must honor that + // request's scope even while nested - the isolation only suppresses + // the implicit global fallback, not deliberate developer intent. + $request = $this->createMockRequest('include=explicitChild&fields=child_id,parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'explicitChild' => ['child_id' => 99], + ], $result); + } + + public function testRootScopeStillAppliesAfterNesting(): void + { + // Sanity check that the root transformer keeps applying its own scope + // while nested children are isolated. + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1, 'secret' => 'hidden']); + + // Root keeps only parent_id (plus the include key), the child is intact. + $this->assertArrayHasKey('parent_id', $result); + $this->assertArrayNotHasKey('secret', $result); + $this->assertSame(['child_id' => 99, 'status' => 'transformed'], $result['children']); + } + + public function testDepthIsRestoredAfterIncludeThrows(): void + { + $request = $this->createMockRequest('include=nonexistent'); + Services::injectMock('request', $request); + + $throwingRoot = new class ($request) extends BaseTransformer { + public function toArray(mixed $resource): array + { + return $resource; + } + }; + + try { + $throwingRoot->transform(['id' => 1, 'name' => 'Test']); + $this->fail('Expected ApiException was not thrown.'); + } catch (ApiException) { + // expected + } + + // The nesting depth must be balanced after the exception, so a fresh + // root transformer still applies the request scope (depth back to 0). + $fieldsRequest = $this->createMockRequest('fields=id'); + Services::injectMock('request', $fieldsRequest); + + $nextRoot = new class ($fieldsRequest) extends BaseTransformer { + public function toArray(mixed $resource): array + { + return $resource; + } + }; + + $result = $nextRoot->transform(['id' => 1, 'name' => 'Test']); + + $this->assertSame(['id' => 1], $result); + } + + public function testBareNestedTransformerStillUsedByChildTransformerDirectly(): void + { + // When ChildTransformer is itself the root (no parent), it must apply + // request scope as usual - the isolation only affects nesting. + $request = $this->createMockRequest('fields=child_id'); + Services::injectMock('request', $request); + + $transformer = new ChildTransformer($request); + + $result = $transformer->transform(['id' => 42]); + + $this->assertSame(['child_id' => 42], $result); + } } diff --git a/user_guide_src/source/changelogs/v4.7.4.rst b/user_guide_src/source/changelogs/v4.7.4.rst index daf64bbda631..7a38176717c0 100644 --- a/user_guide_src/source/changelogs/v4.7.4.rst +++ b/user_guide_src/source/changelogs/v4.7.4.rst @@ -30,6 +30,7 @@ Deprecations Bugs Fixed ********** +- **API:** Fixed a bug in Transformers where the root request's ``fields`` and ``include`` query parameters leaked into nested transformers created inside ``include*()`` methods, causing incorrect field filtering, unexpected includes, or infinite recursion. - **Database:** Fixed a bug where ``updateBatch()`` could be called after Query Builder ``where()`` conditions, even though it's not supported. In this situation, now the ``DatabaseException`` is thrown. - **HTTP:** Fixed a bug where the User Agent library reported Safari's WebKit version instead of the browser version from the ``Version`` token. diff --git a/user_guide_src/source/outgoing/api_transformers.rst b/user_guide_src/source/outgoing/api_transformers.rst index 5e2ab90ca5cb..38ce1de1fc14 100644 --- a/user_guide_src/source/outgoing/api_transformers.rst +++ b/user_guide_src/source/outgoing/api_transformers.rst @@ -174,6 +174,14 @@ The response would include: ] } +.. note:: The ``fields`` and ``include`` query parameters describe the **root** resource only. + Transformers instantiated inside an ``include*()`` method (such as ``PostTransformer`` above) + are treated as nested resources: when created **without an explicit request** they do **not** + inherit the root request's ``fields``/``include`` state. This prevents the parent's query + parameters from leaking into related resources, which could otherwise cause incorrect field + filtering or unexpected/recursive includes. If you deliberately pass a request to a nested + transformer, that request's scope is honored. + Restricting Available Includes =============================== @@ -262,6 +270,13 @@ Class Reference Initializes the transformer and extracts the ``fields`` and ``include`` query parameters from the request. + .. note:: When no request is explicitly passed, the global request's ``fields`` and ``include`` + are read **only for the root transformer**. A transformer constructed during a nested + transformation (i.e. inside an ``include*()`` method) without an explicit request still uses + the global request object, but does not read ``fields``/``include`` from it, to avoid leaking + the root's scope. Passing an explicit request always applies that request's scope, regardless + of nesting. + .. php:method:: toArray(mixed $resource) :param mixed $resource: The resource being transformed (Entity, array, object, or null) From f32114e454e696811b314649a2d08593ee04511f Mon Sep 17 00:00:00 2001 From: michalsn Date: Fri, 5 Jun 2026 20:32:08 +0200 Subject: [PATCH 2/2] fix random tests execution --- tests/system/API/TransformerTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/system/API/TransformerTest.php b/tests/system/API/TransformerTest.php index 6605d47e2d6d..11f37f0fd859 100644 --- a/tests/system/API/TransformerTest.php +++ b/tests/system/API/TransformerTest.php @@ -35,12 +35,14 @@ protected function setUp(): void { parent::setUp(); + Services::resetSingle('request'); Services::superglobals()->setGetArray([]); } protected function tearDown(): void { Services::superglobals()->setGetArray([]); + Services::resetSingle('request'); parent::tearDown(); }