fix: Guards laufen auch im permission_callback vor permission() und im try des execute_callback – Lese-Tools antworten bei gesperrter Fähigkeit mit der Ablehnung statt internal_error, werfende Guards ergeben internal_error

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/Abilities/AbilityRegistrar.php

@@ -63,7 +63,7 @@ final class AbilityRegistrar
             'description' => $ability->description(),
             'category' => self::CATEGORY,
             'input_schema' => $ability->inputSchema(),
-            'permission_callback' => static function (mixed $input = null) use ($ability): bool|WP_Error {
+            'permission_callback' => static function (mixed $input = null) use ($ability, $guards): bool|WP_Error {
                 if (! Access::currentUserMayUse()) {
                     return Errors::forbidden('Dein Benutzerkonto ist für LIT MCP nicht freigegeben.');
                 }
@@ -73,7 +73,12 @@ final class AbilityRegistrar
                 }
 
                 try {
-                    return $ability->permission(is_array($input) ? $input : []);
+                    $input = is_array($input) ? $input : [];
+                    // Before permission(): a read tool whose loader breaks must answer with the guard's refusal
+                    // (e.g. the Divi release lock), not with internal_error from its own permission check.
+                    $refusal = self::guardRefusal($guards, $ability, $input);
+
+                    return $refusal ?? $ability->permission($input);
                 } catch (Throwable $e) {
                     Log::failure($ability->name(), $e);
                     return Errors::internal();
@@ -81,14 +86,15 @@ final class AbilityRegistrar
             },
             'execute_callback' => static function (mixed $input = null) use ($ability, $guards): mixed {
                 $input = is_array($input) ? $input : [];
-                foreach ($guards as $guard) {
-                    $refusal = $guard->refusal($ability, $input);
+
+                try {
+                    // Second line of defence: the permission callback already asked the guards. A refusal there
+                    // stops the call, so a write is logged as refused only once.
+                    $refusal = self::guardRefusal($guards, $ability, $input);
                     if ($refusal !== null) {
                         return $refusal;
                     }
-                }
 
-                try {
                     $result = $ability->execute($input);
                     foreach ($guards as $guard) {
                         $result = $guard->annotate($ability, $result);
@@ -125,6 +131,22 @@ final class AbilityRegistrar
         return $args;
     }
 
+    /**
+     * @param list<AbilityGuard>   $guards
+     * @param array<string, mixed> $input
+     */
+    private static function guardRefusal(array $guards, Ability $ability, array $input): ?WP_Error
+    {
+        foreach ($guards as $guard) {
+            $refusal = $guard->refusal($ability, $input);
+            if ($refusal !== null) {
+                return $refusal;
+            }
+        }
+
+        return null;
+    }
+
     /**
      * A partial write stays partial_write; otherwise the first ToolRefusal anywhere in the cause chain is the answer
      * (e.g. the Divi release lock, spec §5.5); a rolled-back write is write_failed; anything else internal_error.

tests/Unit/Abilities/AbilityRegistrarTest.php

@@ -393,6 +393,119 @@ final class AbilityRegistrarTest extends TestCase
         self::assertSame([['x' => 1]], $seen);
     }
 
+    private function throwingGuard(): AbilityGuard
+    {
+        return new class implements AbilityGuard {
+            public function refusal(Ability $ability, array $input): ?WP_Error
+            {
+                throw new RuntimeException('secret path /var/www/x.php');
+            }
+
+            public function annotate(Ability $ability, mixed $result): mixed
+            {
+                return $result;
+            }
+
+            public function outputSchema(Ability $ability, ?array $schema): ?array
+            {
+                return $schema;
+            }
+        };
+    }
+
+    /** @param callable(): mixed $call */
+    private function withErrorLog(callable $call): mixed
+    {
+        $log = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log');
+        $previous = ini_set('error_log', $log);
+
+        try {
+            return $call();
+        } finally {
+            ini_set('error_log', (string) $previous);
+            unlink($log);
+        }
+    }
+
+    public function testAGuardRefusalInThePermissionCallbackSkipsPermissionAndExecute(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        $seen = [];
+        $ability = $this->ability(false);
+        $ability->shouldNotReceive('permission');
+        $ability->expects('execute')->never();
+        $registrar = $this->guardedRegistrar($ability, [$this->guard('a', Errors::forbidden('Nein.'), $seen)]);
+
+        $result = ($registrar->argsFor($ability)['permission_callback'])(['x' => 1]);
+
+        self::assertToolError($result, 'forbidden');
+        self::assertSame([['x' => 1]], $seen);
+    }
+
+    public function testAReadToolWhosePermissionWouldThrowGetsTheGuardRefusalInstead(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        $ability = $this->ability(true);
+        $ability->shouldNotReceive('permission');
+        $registrar = $this->guardedRegistrar($ability, [$this->guard('a', Errors::forbidden('Gesperrt.'))]);
+
+        self::assertToolError(($registrar->argsFor($ability)['permission_callback'])([]), 'forbidden');
+    }
+
+    public function testAPassingGuardLeavesThePermissionToTheAbility(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        $ability = $this->ability();
+        $ability->expects('permission')->with(['x' => 1])->andReturn(true);
+        $registrar = $this->guardedRegistrar($ability, [$this->guard('a')]);
+
+        self::assertTrue(($registrar->argsFor($ability)['permission_callback'])(['x' => 1]));
+    }
+
+    public function testAThrowingGuardIsAnInternalErrorInThePermissionCallback(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        $ability = $this->ability(false);
+        $ability->shouldNotReceive('permission');
+        $ability->expects('execute')->never();
+        $registrar = $this->guardedRegistrar($ability, [$this->throwingGuard()]);
+
+        $result = $this->withErrorLog(
+            static fn (): mixed => ($registrar->argsFor($ability)['permission_callback'])([])
+        );
+
+        $payload = self::assertToolError($result, 'internal_error');
+        self::assertStringNotContainsString('/var/www', $result->get_error_message());
+        self::assertIsArray($payload);
+    }
+
+    public function testAThrowingGuardIsAnInternalErrorInTheExecuteCallbackAndNothingRuns(): void
+    {
+        $ability = $this->ability(false);
+        $ability->expects('execute')->never();
+        $registrar = $this->guardedRegistrar($ability, [$this->throwingGuard()]);
+
+        $result = $this->withErrorLog(
+            static fn (): mixed => ($registrar->argsFor($ability)['execute_callback'])([])
+        );
+
+        self::assertToolError($result, 'internal_error');
+    }
+
+    public function testARealFeatureGateRefusesAReadToolInThePermissionCallback(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        DiviFeatureFixtures::lock(DiviFeature::PageRead);
+        $ability = $this->ability(true, [DiviFeature::PageRead]);
+        $ability->shouldNotReceive('permission');
+        $ability->expects('execute')->never();
+
+        $result = ($this->diviRegistrar($ability)->argsFor($ability)['permission_callback'])(['post_id' => 7]);
+
+        $payload = self::assertToolError($result, 'divi_untested');
+        self::assertSame(['page-read'], array_column($payload['features'], 'id'));
+    }
+
     public function testTheFirstRefusalWinsAndLaterGuardsAreNotAsked(): void
     {
         $seenSecond = [];