Append upstream API exception messages in ApiCallTool

Services/Api/ApiCallQueryService.php

@@ -115,15 +115,15 @@ private function normalizeValue(mixed $value, string $context): mixed
 
     private function buildFailureMessage(CoreApiRequestException $e): string
     {
-        $detail = $this->extractSafeFailureDetail($e);
+        $detail = $this->extractFailureDetail($e);
         if ($detail === null) {
             return self::GENERIC_FAILURE_MESSAGE;
         }
 
         return self::DETAILED_FAILURE_PREFIX . $detail;
     }
 
-    private function extractSafeFailureDetail(CoreApiRequestException $e): ?string
+    private function extractFailureDetail(CoreApiRequestException $e): ?string
     {
         $previous = $e->getPrevious();
         if (!$previous instanceof \Throwable) {
@@ -140,73 +140,74 @@ private function extractSafeFailureDetail(CoreApiRequestException $e): ?string
             return null;
         }
 
-        if (!$this->isSafeFailureDetail($message)) {
+        if ($this->shouldSuppressFailureDetail($message)) {
             return null;
         }
 
         return $message . '.';
     }
 
-    private function isSafeFailureDetail(string $message): bool
+    private function shouldSuppressFailureDetail(string $message): bool
     {
-        if (strlen($message) > 160) {
-            return false;
-        }
-
         $normalized = strtolower($message);
+
         $unsafeFragments = [
+            // Secret/session-token wording. Access-denial phrasing is routed
+            // via NoAccessLikeErrorDetector before this denylist runs.
+            'token_auth',
+            'bearer ',
+            'session token',
+            'session id',
+            'phpsessid',
+            'force_api_session',
+
+            // SQL internals. Verb+clause SQL is handled by the regex below.
             'sqlstate',
-            'select ',
-            'insert ',
-            'update ',
-            'delete ',
             'create table',
             'drop table',
             'alter table',
-            'exception',
-            'stack trace',
-            ' in /',
-            ' at /',
-            '/var/www/',
-            '\\',
-            '<',
-            '>',
-            'token_auth',
-            'bearer ',
-            'session',
-            'permission denied',
-            'call to ',
-            'uncaught ',
+
+            // PHP runtime crash wording. "Uncaught"/"Stack trace" live in
+            // getTraceAsString(), not getMessage(), so they are not listed.
+            'call to undefined ',
+
+            // Internal invariant/class-name failures.
+            'sanity check:',
+            'unknown datatable type',
+            'unexpected datatable type',
+
+            // Filesystem path leakage.
+            "wasn't found in ",
+
+            // SegmentEditor wraps nested parser/validator exception text verbatim.
+            'the specified segment is invalid:',
         ];
 
         foreach ($unsafeFragments as $fragment) {
             if (str_contains($normalized, $fragment)) {
-                return false;
+                return true;
             }
         }
 
-        if (preg_match('/#\d+/', $message) === 1) {
-            return false;
+        // Raw SQL leakage: verb + matching clause keyword. The clause context
+        // avoids suppressing prose like "please select a value" or
+        // "failed to delete user".
+        if (
+            preg_match(
+                '/\bselect\b.{0,200}\bfrom\b|\binsert\s+into\b|\bupdate\b.{0,200}\bset\b|\bdelete\s+from\b/s',
+                $normalized,
+            ) === 1
+        ) {
+            return true;
         }
 
-        $safeSignals = [
-            'parameter',
-            'missing',
-            'invalid',
-            'must be',
-            'required',
-            'expected',
-            'unknown value',
-            'not supported',
-            'out of range',
-        ];
-
-        foreach ($safeSignals as $signal) {
-            if (str_contains($normalized, $signal)) {
-                return true;
-            }
+        // Namespaced class-like tokens ("Piwik\DataTable\Map"). Require two
+        // PascalCase segments so stray escapes like "\n" or "\d" do not match.
+        if (preg_match('/\b[A-Z][A-Za-z0-9_]+(?:\\\\[A-Z][A-Za-z0-9_]*){1,}/', $message) === 1) {
+            return true;
         }
 
-        return false;
+        // Absolute filesystem paths.
+        return preg_match('~(?:^|[\s(])/(?:[^/\s]+/)+[^/\s)]+~', $message) === 1;
     }
 }

Support/Errors/NoAccessLikeErrorDetector.php

@@ -24,8 +24,27 @@ public static function isDetected(\Throwable $e): bool
             return false;
         }
 
-        return str_contains($message, 'no access')
+        if (
+            str_contains($message, 'no access')
             || str_contains($message, 'checkuserhasviewaccess')
-            || str_contains($message, 'view access');
+            || str_contains($message, 'view access')
+            || str_contains($message, 'access denied')
+            || str_contains($message, 'permission denied')
+        ) {
+            return true;
+        }
+
+        if (preg_match('/\bnot authorized\b/', $message) === 1) {
+            return true;
+        }
+
+        // "unauthorized" needs access context so validation wording like
+        // "unauthorized character in input" does not get routed to the
+        // no-access path. Allow bare "Unauthorized" replies (end-of-string
+        // or trailing punctuation) and access-context nouns.
+        return preg_match(
+            '/\bunauthorized(?:\s+(?:access|request|user|client|api|to)\b|\s*(?=[.!?,:;)]|$))/',
+            $message,
+        ) === 1;
     }
 }

tests/Integration/McpTools/ApiCallTest.php

@@ -280,7 +280,7 @@ public function testFullModeRejectsBlockedProxyLikeMethodBySplitSelector(): void
         );
     }
 
-    public function testFullModeAttemptsMutatingMethodCall(): void
+    public function testFullModeReturnsDetailedValidationErrorForAddUser(): void
     {
         McpTestHelper::setRawApiAccessMode('full');
 
@@ -295,17 +295,33 @@ public function testFullModeAttemptsMutatingMethodCall(): void
         );
 
         McpTestHelper::assertToolError($result);
-        $content = $result->content[0] ?? null;
-        self::assertInstanceOf(\Matomo\Dependencies\McpServer\Mcp\Schema\Content\TextContent::class, $content);
-        $errorText = $content->text;
-        self::assertIsString($errorText);
-        self::assertTrue(
-            $errorText === 'Matomo API request failed.'
-            || str_starts_with($errorText, 'Matomo API request failed: '),
+        $this->assertIsDetailedMissingParameterError($result);
+    }
+
+    public function testFullModeSuppressesNestedSegmentValidationError(): void
+    {
+        McpTestHelper::setRawApiAccessMode('full');
+
+        $server = McpTestHelper::buildServer();
+        $sessionId = McpTestHelper::initializeSession($server);
+        McpTestHelper::callToolAndAssertError(
+            $server,
+            $sessionId,
+            ApiCallFull::TOOL_NAME,
+            [
+                'method' => 'SegmentEditor.add',
+                'parameters' => [
+                    'name' => 'Broken Segment',
+                    'definition' => 'invalidSegmentDefinition==value',
+                    'idSite' => $this->idSite,
+                ],
+            ],
+            'Matomo API request failed.',
+            __METHOD__,
         );
     }
 
-    public function testCreateModeAttemptsCreateMethodCall(): void
+    public function testCreateModeReturnsDetailedValidationErrorForAddUser(): void
     {
         McpTestHelper::setRawApiAccessMode('create');
 
@@ -320,11 +336,10 @@ public function testCreateModeAttemptsCreateMethodCall(): void
         );
 
         McpTestHelper::assertToolError($result);
-        $content = $result->content[0] ?? null;
-        self::assertInstanceOf(\Matomo\Dependencies\McpServer\Mcp\Schema\Content\TextContent::class, $content);
+        $this->assertIsDetailedMissingParameterError($result);
     }
 
-    public function testDeleteModeAttemptsDeleteMethodCall(): void
+    public function testDeleteModeReturnsDetailedValidationErrorForDeleteSite(): void
     {
         McpTestHelper::setRawApiAccessMode('delete');
 
@@ -339,17 +354,10 @@ public function testDeleteModeAttemptsDeleteMethodCall(): void
         );
 
         McpTestHelper::assertToolError($result);
-        $content = $result->content[0] ?? null;
-        self::assertInstanceOf(\Matomo\Dependencies\McpServer\Mcp\Schema\Content\TextContent::class, $content);
-        $errorText = $content->text;
-        self::assertIsString($errorText);
-        self::assertTrue(
-            $errorText === 'Matomo API request failed.'
-            || str_starts_with($errorText, 'Matomo API request failed: '),
-        );
+        $this->assertIsDetailedMissingParameterError($result);
     }
 
-    public function testUpdateModeAttemptsUpdateMethodCall(): void
+    public function testUpdateModeReturnsDetailedValidationErrorForUpdateUser(): void
     {
         McpTestHelper::setRawApiAccessMode('update');
 
@@ -364,14 +372,7 @@ public function testUpdateModeAttemptsUpdateMethodCall(): void
         );
 
         McpTestHelper::assertToolError($result);
-        $content = $result->content[0] ?? null;
-        self::assertInstanceOf(\Matomo\Dependencies\McpServer\Mcp\Schema\Content\TextContent::class, $content);
-        $errorText = $content->text;
-        self::assertIsString($errorText);
-        self::assertTrue(
-            $errorText === 'Matomo API request failed.'
-            || str_starts_with($errorText, 'Matomo API request failed: '),
-        );
+        $this->assertIsDetailedMissingParameterError($result);
     }
 
     public function testRejectsReservedParameterKeys(): void
@@ -530,4 +531,14 @@ private function listToolNamesForCurrentConfig(): array
 
         return array_values(array_map(static fn($tool) => $tool->name, $result->tools));
     }
+
+    private function assertIsDetailedMissingParameterError(
+        \Matomo\Dependencies\McpServer\Mcp\Schema\Result\CallToolResult $result,
+    ): void {
+        $content = $result->content[0] ?? null;
+        self::assertInstanceOf(\Matomo\Dependencies\McpServer\Mcp\Schema\Content\TextContent::class, $content);
+        self::assertIsString($content->text);
+        self::assertStringStartsWith('Matomo API request failed: ', $content->text);
+        self::assertStringContainsString('General_PleaseSpecifyValue', $content->text);
+    }
 }