Fix #[Security] on ExtendType fields (source-injection array_combine crash)

.github/workflows/continuous_integration.yml

@@ -22,7 +22,7 @@ jobs:
     strategy:
       matrix:
         install-args: ['', '--prefer-lowest']
-        php-version: ['8.1', '8.2', '8.3', '8.4']
+        php-version: ['8.2', '8.3', '8.4', '8.5']
       fail-fast: false
 
     steps:
@@ -73,7 +73,7 @@ jobs:
         run: "composer phpstan"
 
       - name: "Run coding standard checks with squizlabs/php_codesniffer on minimum supported PHP version"
-        if: matrix.php-version == '8.1'
+        if: matrix.php-version == '8.2'
         run: composer cs-check
 
       - name: "Archive code coverage results"

composer.json

@@ -10,7 +10,7 @@
         }
     ],
     "require": {
-        "php": ">=8.1",
+        "php": ">=8.2",
         "ext-json": "*",
         "composer/package-versions-deprecated": "^1.8",
         "phpdocumentor/reflection-docblock": "^5.4",
@@ -28,14 +28,14 @@
     },
     "require-dev": {
         "beberlei/porpaginas": "^2.0",
-        "doctrine/coding-standard": "^12.0 || ^13.0",
+        "doctrine/coding-standard": "^13.0",
         "ecodev/graphql-upload": "^7.0",
         "laminas/laminas-diactoros": "^3.5",
         "myclabs/php-enum": "^1.6.6",
         "php-coveralls/php-coveralls": "^2.7",
         "phpstan/extension-installer": "^1.4",
         "phpstan/phpstan": "^2.0",
-        "phpunit/phpunit": "^10.5 || ^11.0 || ^12.0",
+        "phpunit/phpunit": "^11.0",
         "symfony/var-dumper": "^6.4"
     },
     "suggest": {
@@ -68,6 +68,11 @@
             "composer/package-versions-deprecated": true,
             "dealerdirect/phpcodesniffer-composer-installer": true,
             "phpstan/extension-installer": true
+        },
+        "audit": {
+            "ignore": {
+                "PKSA-5jz8-6tcw-pbk4": "PHPUnit argument-injection via newline in INI values forwarded to child PHP processes (GHSA-qrr6-mg7r-m243). phpunit is a require-dev dependency; the attack surface is INI values in phpunit config or CLI args, which are authored by maintainers/CI and carry the same trust as any other committed code. No fix available for PHPUnit 10.x or 11.x yet — revisit when backport ships or when we bump min PHP to 8.3 and can move to ^12.5.22."
+            }
         }
     }
 }

phpcs.xml.dist

@@ -29,6 +29,9 @@
         <exclude name="Squiz.Commenting.FunctionComment.InvalidNoReturn" />
         <exclude name="Generic.Formatting.MultipleStatementAlignment" />
         <exclude name="Squiz.Functions.MultiLineFunctionDeclaration.NewlineBeforeOpenBrace" />
+        <!-- Requires PHP 8.3 typed class constants; our minimum supported PHP is 8.2 so this
+             rule can't be satisfied without a runtime bump. Re-enable when min PHP >= 8.3. -->
+        <exclude name="SlevomatCodingStandard.TypeHints.ClassConstantTypeHint" />
     </rule>
 
     <!-- Do not align assignments -->

src/FieldsBuilder.php

@@ -878,7 +878,13 @@ private function mapParameters(
 
         $docBlockTypes = [];
         foreach ($paramTags as $paramTag) {
-            $docBlockTypes[$paramTag->getVariableName()] = $paramTag->getType();
+            $variableName = $paramTag->getVariableName();
+            // Skip malformed @param tags with no variable name (phpdocumentor returns null for
+            // those). PHPStan 8.5 rejects the implicit null-to-empty-string coercion.
+            if ($variableName === null) {
+                continue;
+            }
+            $docBlockTypes[$variableName] = $paramTag->getType();
         }
 
         $parameterAnnotationsPerParameter = $this->annotationReader->getParameterAnnotationsPerParameter($refParameters);

src/Mappers/GlobTypeMapperCache.php

@@ -7,6 +7,7 @@
 use ReflectionClass;
 
 use function array_keys;
+use function assert;
 
 /**
  * The cached results of a GlobTypeMapper
@@ -48,7 +49,11 @@ public function registerAnnotations(ReflectionClass|string $sourceClass, GlobAnn
                 $this->mapClassToTypeArray[$objectClassName] = $className;
             }
 
+            // GlobAnnotationsCache::withType() sets typeClassName and typeName together, so
+            // inside this branch typeName is guaranteed non-null. Assert the invariant for PHPStan
+            // 8.5+, which is stricter about nullable array offsets.
             $typeName = $globAnnotationsCache->getTypeName();
+            assert($typeName !== null);
             $this->mapNameToType[$typeName] = $className;
         }
 

src/Mappers/Root/IteratorTypeMapper.php

@@ -206,8 +206,6 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $
      */
     private function splitIteratorFromOtherTypes(array &$types): Type|null
     {
-        $iteratorType = null;
-        $key = null;
         foreach ($types as $key => $singleDocBlockType) {
             if (! ($singleDocBlockType instanceof Object_)) {
                 continue;
@@ -216,21 +214,17 @@ private function splitIteratorFromOtherTypes(array &$types): Type|null
             /** @var class-string<object> $fqcn */
             $fqcn     = (string) $singleDocBlockType->getFqsen();
             $refClass = new ReflectionClass($fqcn);
-            // Note : $refClass->isIterable() is only accessible in PHP 7.2
             if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) {
                 continue;
             }
-            $iteratorType = $singleDocBlockType;
-            break;
-        }
 
-        if ($iteratorType === null) {
-            return null;
-        }
+            // One of the classes in the compound is an iterator. Remove it and let the caller
+            // test the remaining values as potential subTypes.
+            unset($types[$key]);
 
-        // One of the classes in the compound is an iterator. Let's remove it from the list and let's test all other values as potential subTypes.
-        unset($types[$key]);
+            return $singleDocBlockType;
+        }
 
-        return $iteratorType;
+        return null;
     }
 }

src/Middlewares/SecurityFieldMiddleware.php

@@ -18,7 +18,9 @@
 
 use function array_combine;
 use function array_keys;
+use function array_slice;
 use function assert;
+use function count;
 
 /**
  * A field middleware that reads "Security" Symfony annotations.
@@ -73,9 +75,15 @@ public function process(
         $originalResolver = $queryFieldDescriptor->getOriginalResolver();
 
         $parameters = $queryFieldDescriptor->getParameters();
+        // QueryField::fromFieldDescriptor prepends a SourceParameter to the parameters list when
+        // `injectSource` is true, but that injection happens AFTER this middleware runs. At resolver
+        // invocation time the runtime $args therefore includes the source as its first element while
+        // $parameters captured here does not. Track the flag so getVariables() can strip the leading
+        // source arg before zipping args to parameter names.
+        $injectSource = $queryFieldDescriptor->isInjectSource();
 
-        $queryFieldDescriptor = $queryFieldDescriptor->withResolver(function (object|null $source, ...$args) use ($originalResolver, $securityAnnotations, $resolver, $failWith, $parameters, $queryFieldDescriptor) {
-            $variables = $this->getVariables($args, $parameters, $originalResolver->executionSource($source));
+        $queryFieldDescriptor = $queryFieldDescriptor->withResolver(function (object|null $source, ...$args) use ($originalResolver, $securityAnnotations, $resolver, $failWith, $parameters, $queryFieldDescriptor, $injectSource) {
+            $variables = $this->getVariables($args, $parameters, $originalResolver->executionSource($source), $injectSource);
 
             foreach ($securityAnnotations as $annotation) {
                 try {
@@ -108,7 +116,7 @@ public function process(
      *
      * @return array<string, mixed>
      */
-    private function getVariables(array $args, array $parameters, object|null $source): array
+    private function getVariables(array $args, array $parameters, object|null $source, bool $injectSource = false): array
     {
         $variables = [
             // If a user is not logged, we provide an empty user object to make usage easier
@@ -118,8 +126,15 @@ private function getVariables(array $args, array $parameters, object|null $sourc
             'this' => $source,
         ];
 
+        // Strip the source arg prepended by QueryField::fromFieldDescriptor so the remaining
+        // user-supplied args line up positionally with the captured parameter names. The source
+        // is always exposed via `this`, so there's no loss of information for the expression.
+        if ($injectSource && count($args) > count($parameters)) {
+            $args = array_slice($args, 1);
+        }
+
         $argsName = array_keys($parameters);
-        $argsByName = array_combine($argsName, $args);
+        $argsByName = $argsName ? array_combine($argsName, $args) : [];
 
         return $variables + $argsByName;
     }

src/Middlewares/SecurityInputFieldMiddleware.php

@@ -15,6 +15,8 @@
 
 use function array_combine;
 use function array_keys;
+use function array_slice;
+use function count;
 
 /**
  * A field input middleware that reads "Security" Symfony annotations.
@@ -43,9 +45,13 @@ public function process(InputFieldDescriptor $inputFieldDescriptor, InputFieldHa
         $originalResolver = $inputFieldDescriptor->getOriginalResolver();
 
         $parameters = $inputFieldDescriptor->getParameters();
+        // InputField::fromFieldDescriptor prepends a SourceParameter to the parameters list when
+        // `injectSource` is true, but that injection happens AFTER this middleware runs. See the
+        // sibling SecurityFieldMiddleware for the detailed comment.
+        $injectSource = $inputFieldDescriptor->isInjectSource();
 
-        $inputFieldDescriptor = $inputFieldDescriptor->withResolver(function (object|null $source, ...$args) use ($originalResolver, $securityAnnotations, $resolver, $parameters, $inputFieldDescriptor) {
-            $variables = $this->getVariables($args, $parameters, $originalResolver->executionSource($source));
+        $inputFieldDescriptor = $inputFieldDescriptor->withResolver(function (object|null $source, ...$args) use ($originalResolver, $securityAnnotations, $resolver, $parameters, $inputFieldDescriptor, $injectSource) {
+            $variables = $this->getVariables($args, $parameters, $originalResolver->executionSource($source), $injectSource);
 
             foreach ($securityAnnotations as $annotation) {
                 try {
@@ -71,7 +77,7 @@ public function process(InputFieldDescriptor $inputFieldDescriptor, InputFieldHa
      *
      * @return array<string, mixed>
      */
-    private function getVariables(array $args, array $parameters, object|null $source): array
+    private function getVariables(array $args, array $parameters, object|null $source, bool $injectSource = false): array
     {
         $variables = [
             // If a user is not logged, we provide an empty user object to make usage easier
@@ -81,8 +87,12 @@ private function getVariables(array $args, array $parameters, object|null $sourc
             'this' => $source,
         ];
 
+        if ($injectSource && count($args) > count($parameters)) {
+            $args = array_slice($args, 1);
+        }
+
         $argsName = array_keys($parameters);
-        $argsByName = array_combine($argsName, $args);
+        $argsByName = $argsName ? array_combine($argsName, $args) : [];
 
         return $variables + $argsByName;
     }

tests/Fixtures/Integration/Types/ExtendedContactType.php

@@ -6,6 +6,7 @@
 
 use TheCodingMachine\GraphQLite\Annotations\ExtendType;
 use TheCodingMachine\GraphQLite\Annotations\Field;
+use TheCodingMachine\GraphQLite\Annotations\Security;
 use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Contact;
 
 use function strtoupper;
@@ -34,4 +35,17 @@ public function company(Contact $contact): string
     {
             return $contact->getName() . ' Ltd';
     }
+
+    /**
+     * Regression: #[Security] on an ExtendType field used to blow up with
+     * "array_combine(): ... must have the same number of elements" because
+     * SecurityFieldMiddleware didn't mirror the source-injection that
+     * QueryField::fromFieldDescriptor performs.
+     */
+    #[Field]
+    #[Security("user && user.bar == 42", failWith: null)]
+    public function extendedSecretName(Contact $contact): string|null
+    {
+        return $contact->getName();
+    }
 }

tests/Integration/EndToEndTest.php

@@ -1338,6 +1338,63 @@ public function testEndToEndSecurityInField(): void
         $this->assertSame('Access denied.', $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors'][0]['message']);
     }
 
+    /**
+     * Regression test for #[Security] on an #[ExtendType] field method.
+     *
+     * SecurityFieldMiddleware captures $parameters at process() time, but
+     * QueryField::fromFieldDescriptor prepends an implicit SourceParameter
+     * AFTER the middleware runs when isInjectSource() is true. At runtime the
+     * resolver receives N+1 args for N captured parameter names, which used
+     * to blow array_combine() up with "Argument #1 ($keys) and argument #2
+     * ($values) must have the same number of elements".
+     */
+    public function testEndToEndSecurityOnExtendTypeField(): void
+    {
+        // No logged-in user → the Security expression `user.bar == 42` fails, failWith returns null.
+        $schema = $this->mainContainer->get(Schema::class);
+        assert($schema instanceof Schema);
+
+        $queryString = '
+        query {
+            contacts {
+                name
+                extendedSecretName
+            }
+        }
+        ';
+
+        $result = GraphQL::executeQuery($schema, $queryString);
+        $data = $this->getSuccessResult($result);
+        $this->assertSame('Joe', $data['contacts'][0]['name']);
+        $this->assertNull($data['contacts'][0]['extendedSecretName']);
+
+        // Logged-in user with bar=42 → the Security expression passes and the field returns the name.
+        $container = $this->createContainer([
+            AuthenticationServiceInterface::class => static function () {
+                return new class implements AuthenticationServiceInterface {
+                    public function isLogged(): bool
+                    {
+                        return true;
+                    }
+
+                    public function getUser(): object|null
+                    {
+                        $user = new stdClass();
+                        $user->bar = 42;
+                        return $user;
+                    }
+                };
+            },
+        ]);
+
+        $schema = $container->get(Schema::class);
+        assert($schema instanceof Schema);
+
+        $result = GraphQL::executeQuery($schema, $queryString);
+        $data = $this->getSuccessResult($result);
+        $this->assertSame('Joe', $data['contacts'][0]['extendedSecretName']);
+    }
+
     public function testEndToEndUnions(): void
     {
         $schema = $this->mainContainer->get(Schema::class);

website/package.json

@@ -16,6 +16,9 @@
     "react": "^17.0.1",
     "react-dom": "^17.0.1"
   },
+  "resolutions": {
+    "webpack": "5.88.2"
+  },
   "browserslist": {
     "production": [
       ">0.5%",