fix(rest): idempotent, per-controller derived permission_callback

lib/Strategies/RestStrategy.php

@@ -45,11 +45,20 @@ class RestStrategy implements CoreRestStrategy
 
     /**
      * Requests whose middleware chain already ran in the permission
-     * callback, mapped to the middleware outcome (true, or the caught
-     * non-auth RestException to be re-thrown by the route callback so
-     * clients keep the rich legacy error shape).
+     * callback, mapped per controller class to the middleware outcome
+     * (true, the caught non-auth RestException to be re-thrown by the
+     * route callback so clients keep the rich legacy error shape, or
+     * the WP_Error returned for auth rejections).
      *
-     * @var WeakMap<WP_REST_Request, true|RestException>
+     * Keyed by controller class because WordPress invokes the permission
+     * callback of EVERY method registered on a matched route for the same
+     * WP_REST_Request (rest_send_allow_header does this to build the
+     * Allow header), and one controller's outcome must not clobber
+     * another's. Memoized so the chain runs at most once per controller
+     * per request — middleware is not required to be idempotent (e.g.
+     * CSV conversion mutates request params).
+     *
+     * @var WeakMap<WP_REST_Request, array<class-string, true|RestException|WP_Error>>
      */
     private WeakMap $middlewareOutcomes;
 
@@ -89,27 +98,52 @@ private function checkPermissions(Controller $controller, WP_REST_Request $wpReq
             return true;
         }
 
+        $outcomes = $this->middlewareOutcomes[$wpRequest] ?? [];
+        $controllerClass = get_class($controller);
+
+        // Already ran for this controller — return the same answer without
+        // re-running the chain. WordPress calls this more than once per
+        // request (dispatch, then rest_send_allow_header for every method
+        // on the route), and middleware may not be idempotent.
+        if (array_key_exists($controllerClass, $outcomes)) {
+            $stored = $outcomes[$controllerClass];
+
+            return $stored instanceof WP_Error ? $stored : true;
+        }
+
         $request = $this->resolveRequest($wpRequest);
 
         try {
             Arr::each($controller->getMiddleware($request), fn(Middleware $middleware) => $middleware->process($request));
-            $this->middlewareOutcomes[$wpRequest] = true;
+            $outcomes[$controllerClass] = true;
         } catch (RestException $e) {
             if (in_array($e->getCode(), [401, 403], true)) {
-                return new WP_Error(
+                $error = new WP_Error(
                     $e->getCode() === 401 ? 'rest_not_logged_in' : 'rest_forbidden',
                     $e->getMessage(),
                     ['status' => $e->getCode()]
                 );
+                $outcomes[$controllerClass] = $error;
+                $this->middlewareOutcomes[$wpRequest] = $outcomes;
+
+                return $error;
             }
 
-            $this->middlewareOutcomes[$wpRequest] = $e;
-        } catch (Exception $e) {
-            $this->logger->logException($e);
+            $outcomes[$controllerClass] = $e;
+        } catch (\Throwable $e) {
+            // \Throwable, not Exception: a TypeError thrown by middleware
+            // must surface as a 500, not fatal the whole request before
+            // WordPress can serve a body. logException() takes Exception,
+            // so non-Exception throwables are wrapped.
+            $this->logger->logException(
+                $e instanceof Exception ? $e : new Exception($e->getMessage(), (int) $e->getCode(), $e)
+            );
 
             return new WP_Error('rest_unexpected_error', 'An unexpected error occurred.', ['status' => 500]);
         }
 
+        $this->middlewareOutcomes[$wpRequest] = $outcomes;
+
         return true;
     }
 
@@ -121,7 +155,8 @@ private function checkPermissions(Controller $controller, WP_REST_Request $wpReq
      */
     private function wrapCallback(Controller $controller, Request $request, ?WP_REST_Request $wpRequest = null): Response
     {
-        $outcome = $wpRequest !== null ? ($this->middlewareOutcomes[$wpRequest] ?? null) : null;
+        $outcomes = $wpRequest !== null ? ($this->middlewareOutcomes[$wpRequest] ?? []) : [];
+        $outcome = $outcomes[get_class($controller)] ?? null;
 
         if ($outcome instanceof RestException) {
             // Middleware already ran in the permission callback and failed

tests/Unit/Strategies/RestStrategyPermissionsTest.php

@@ -138,4 +138,113 @@ public function process(Request $request): void
 
         $this->assertSame(1, $middleware->runs);
     }
+
+    public function testRepeatedPermissionChecksRunMiddlewareOnlyOnce(): void
+    {
+        // WordPress invokes permission_callback again per method on the
+        // matched route while building the Allow header
+        // (rest_send_allow_header), so the chain must be memoized —
+        // middleware is not required to be idempotent.
+        $middleware = new class implements Middleware {
+            public int $runs = 0;
+
+            public function process(Request $request): void
+            {
+                $this->runs++;
+
+                if ($this->runs > 1) {
+                    throw new \TypeError('non-idempotent middleware ran twice');
+                }
+            }
+        };
+
+        $controller = $this->makeControllerWithMiddleware($middleware);
+        $strategy = $this->makeStrategy();
+        $wpRequest = new WP_REST_Request();
+
+        $first = $this->checkPermissions($strategy, $controller, $wpRequest);
+        $second = $this->checkPermissions($strategy, $controller, $wpRequest);
+
+        $this->assertTrue($first);
+        $this->assertTrue($second);
+        $this->assertSame(1, $middleware->runs);
+    }
+
+    public function testRepeatedAuthRejectionReturnsSameWpErrorWithoutRerun(): void
+    {
+        $middleware = new class implements Middleware {
+            public int $runs = 0;
+
+            public function process(Request $request): void
+            {
+                $this->runs++;
+                throw new RestException('Forbidden.', [], 403);
+            }
+        };
+
+        $controller = $this->makeControllerWithMiddleware($middleware);
+        $strategy = $this->makeStrategy();
+        $wpRequest = new WP_REST_Request();
+
+        $first = $this->checkPermissions($strategy, $controller, $wpRequest);
+        $second = $this->checkPermissions($strategy, $controller, $wpRequest);
+
+        $this->assertInstanceOf(WP_Error::class, $first);
+        $this->assertInstanceOf(WP_Error::class, $second);
+        $this->assertSame(1, $middleware->runs);
+    }
+
+    public function testOutcomeIsKeyedPerControllerNotPerRequest(): void
+    {
+        // rest_send_allow_header checks EVERY method registered on the
+        // route against the same WP_REST_Request; one controller's failed
+        // middleware must not clobber another controller's passing outcome.
+        $passing = new class implements Middleware {
+            public function process(Request $request): void
+            {
+            }
+        };
+        $failing = new class implements Middleware {
+            public function process(Request $request): void
+            {
+                throw new RestException('Validation failed.', [], 422);
+            }
+        };
+
+        $getController = $this->makeControllerWithMiddleware($passing);
+        $postController = $this->makeControllerWithMiddleware($failing);
+        $strategy = $this->makeStrategy();
+        $wpRequest = new WP_REST_Request();
+
+        $this->assertTrue($this->checkPermissions($strategy, $getController, $wpRequest));
+        $this->assertTrue($this->checkPermissions($strategy, $postController, $wpRequest));
+
+        // The GET controller's stored outcome must still be a clean pass:
+        // wrapCallback must NOT throw the POST controller's 422.
+        $wrap = new ReflectionMethod($strategy, 'wrapCallback');
+        $resolve = new ReflectionMethod($strategy, 'resolveRequest');
+
+        try {
+            $wrap->invoke($strategy, $getController, $resolve->invoke($strategy, $wpRequest), $wpRequest);
+            $this->fail('Expected getResponse() sentinel exception.');
+        } catch (RestException $e) {
+            $this->fail('GET controller inherited the POST controller\'s middleware failure.');
+        } catch (Exception $e) {
+            $this->assertSame('not used', $e->getMessage());
+        }
+    }
+
+    public function testMiddlewareTypeErrorBecomesWpErrorInsteadOfFatal(): void
+    {
+        $middleware = new class implements Middleware {
+            public function process(Request $request): void
+            {
+                throw new \TypeError('explode(): Argument #2 ($string) must be of type string, array given');
+            }
+        };
+
+        $result = $this->checkPermissions($this->makeStrategy(), $this->makeControllerWithMiddleware($middleware), new WP_REST_Request());
+
+        $this->assertInstanceOf(WP_Error::class, $result);
+    }
 }