From 82aa02d4c49d26678b46fa0de9896444e6356c9a Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Wed, 19 Aug 2026 13:53:57 +0200 Subject: [PATCH 1/8] Bound MCP log queries by the built-in _id index instead of the unindexed datetime sort --- src/Helper/MongoHelper.php | 38 ++++++++++++++ src/Log/Repository/AbstractLogRepository.php | 13 ++++- src/Mcp/Log/McpLogRepository.php | 7 ++- tests/Helper/MongoHelperTest.php | 53 ++++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 src/Helper/MongoHelper.php create mode 100644 tests/Helper/MongoHelperTest.php diff --git a/src/Helper/MongoHelper.php b/src/Helper/MongoHelper.php new file mode 100644 index 0000000..91a2a55 --- /dev/null +++ b/src/Helper/MongoHelper.php @@ -0,0 +1,38 @@ +getTimestamp()))), + self::TIMESTAMP_HEX_LENGTH, + self::TIMESTAMP_HEX_PAD, + STR_PAD_LEFT, + ); + } +} diff --git a/src/Log/Repository/AbstractLogRepository.php b/src/Log/Repository/AbstractLogRepository.php index 9a26886..2b0c327 100644 --- a/src/Log/Repository/AbstractLogRepository.php +++ b/src/Log/Repository/AbstractLogRepository.php @@ -5,6 +5,7 @@ namespace AnzuSystems\CommonBundle\Log\Repository; use AnzuSystems\CommonBundle\Document\Log; +use AnzuSystems\CommonBundle\Helper\MongoHelper; use AnzuSystems\CommonBundle\Repository\Mongo\AbstractAnzuMongoRepository; use DateTimeImmutable; use MongoDB\BSON\UTCDateTime; @@ -14,6 +15,7 @@ */ abstract class AbstractLogRepository extends AbstractAnzuMongoRepository { + protected const string FIELD_ID = '_id'; protected const string FIELD_DATETIME = 'datetime'; protected const string FIELD_CONTEXT_CONTEXT_ID = 'context.contextId'; protected const string REGEX_FLAG_CASE_INSENSITIVE = 'i'; @@ -25,6 +27,8 @@ abstract class AbstractLogRepository extends AbstractAnzuMongoRepository private const int LIMIT_MIN = 1; private const int SORT_DESC = -1; + private const string ID_LOWER_BOUND_SLACK = '-1 minute'; + private const string ID_UPPER_BOUND_SLACK = '+1 day'; private const array RAW_ARRAY_TYPE_MAP = [ 'root' => 'array', 'document' => 'array', @@ -41,6 +45,9 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], + self::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), + ], ], $limit); } @@ -58,7 +65,7 @@ protected function findLatestRawDocuments(array $match, int $limit): array { $documents = $this->collection->find($match, [ 'sort' => [ - self::FIELD_DATETIME => self::SORT_DESC, + self::FIELD_ID => self::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, @@ -78,6 +85,10 @@ protected function createDatetimeWindowMatch(DateTimeImmutable $from, DateTimeIm self::MONGO_GTE => new UTCDateTime($from), self::MONGO_LTE => new UTCDateTime($until), ], + self::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), + self::MONGO_LTE => MongoHelper::maxObjectIdFor($until->modify(self::ID_UPPER_BOUND_SLACK)), + ], ]; } } diff --git a/src/Mcp/Log/McpLogRepository.php b/src/Mcp/Log/McpLogRepository.php index 0870797..2a038fd 100644 --- a/src/Mcp/Log/McpLogRepository.php +++ b/src/Mcp/Log/McpLogRepository.php @@ -5,12 +5,14 @@ namespace AnzuSystems\CommonBundle\Mcp\Log; use AnzuSystems\CommonBundle\ApiFilter\ApiQueryMongo; +use AnzuSystems\CommonBundle\Helper\MongoHelper; use DateTimeImmutable; use MongoDB\BSON\UTCDateTime; use MongoDB\Collection; final readonly class McpLogRepository { + private const string FIELD_ID = '_id'; private const string FIELD_DATETIME = 'datetime'; private const string FIELD_CONTEXT_ID = 'contextId'; private const string MONGO_GTE = '$gte'; @@ -38,9 +40,12 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], + self::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from), + ], ], [ 'sort' => [ - self::FIELD_DATETIME => self::SORT_DESC, + self::FIELD_ID => self::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, diff --git a/tests/Helper/MongoHelperTest.php b/tests/Helper/MongoHelperTest.php new file mode 100644 index 0000000..c653b77 --- /dev/null +++ b/tests/Helper/MongoHelperTest.php @@ -0,0 +1,53 @@ + + */ + public function minObjectIdProvider(): array + { + return [ + ['6a7f7a000000000000000000', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], + ['0000e1000000000000000000', new DateTimeImmutable('1970-01-01 16:00:00+00:00')], + ['000000000000000000000000', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], + ['ffffffff0000000000000000', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], + ]; + } + + /** + * @return list + */ + public function maxObjectIdProvider(): array + { + return [ + ['6a7f7a00ffffffffffffffff', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], + ['00000000ffffffffffffffff', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], + ['ffffffffffffffffffffffff', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], + ]; + } +} From 69f53a34bc0f154c3449643a5be16f1aaad32649 Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 13:51:03 +0200 Subject: [PATCH 2/8] Add MongoHelper newest-first sort and id constants for the bounded log queries --- src/Helper/MongoHelper.php | 44 +++++++++++++--- src/Log/Repository/AbstractLogRepository.php | 10 ++-- src/Mcp/Log/McpLogRepository.php | 11 ++-- src/Resources/doc/helpers.md | 1 + tests/Helper/MongoHelperTest.php | 52 +++++++++---------- tests/Mcp/McpLogRepositoryTest.php | 53 ++++++++++++++++++++ 6 files changed, 125 insertions(+), 46 deletions(-) create mode 100644 tests/Mcp/McpLogRepositoryTest.php diff --git a/src/Helper/MongoHelper.php b/src/Helper/MongoHelper.php index 91a2a55..ad7424f 100644 --- a/src/Helper/MongoHelper.php +++ b/src/Helper/MongoHelper.php @@ -6,15 +6,19 @@ use DateTimeImmutable; use MongoDB\BSON\ObjectId; +use MongoDB\BSON\UTCDateTime; final class MongoHelper { + public const string FIELD_ID = '_id'; + public const int SORT_DESC = -1; + private const int TIMESTAMP_MIN = 0; private const int TIMESTAMP_MAX = 0xFFFFFFFF; - private const int TIMESTAMP_HEX_LENGTH = 8; - private const string TIMESTAMP_HEX_PAD = '0'; + private const string TIMESTAMP_HEX_FORMAT = '%08x'; private const string OBJECT_ID_MIN_SUFFIX = '0000000000000000'; private const string OBJECT_ID_MAX_SUFFIX = 'ffffffffffffffff'; + private const int DATETIME_MILLIS_FALLBACK = 0; public static function minObjectIdFor(DateTimeImmutable $datetime): ObjectId { @@ -26,13 +30,39 @@ public static function maxObjectIdFor(DateTimeImmutable $datetime): ObjectId return new ObjectId(self::timestampHex($datetime) . self::OBJECT_ID_MAX_SUFFIX); } + /** + * @param list> $documents + * + * @return list> + */ + public static function sortNewestFirst(array $documents, string $datetimeField): array + { + usort( + $documents, + static fn (array $a, array $b): int => self::datetimeMillis($b, $datetimeField) <=> self::datetimeMillis($a, $datetimeField), + ); + + return $documents; + } + private static function timestampHex(DateTimeImmutable $datetime): string { - return str_pad( - dechex(min(self::TIMESTAMP_MAX, max(self::TIMESTAMP_MIN, $datetime->getTimestamp()))), - self::TIMESTAMP_HEX_LENGTH, - self::TIMESTAMP_HEX_PAD, - STR_PAD_LEFT, + return sprintf( + self::TIMESTAMP_HEX_FORMAT, + min(self::TIMESTAMP_MAX, max(self::TIMESTAMP_MIN, $datetime->getTimestamp())), ); } + + /** + * @param array $document + */ + private static function datetimeMillis(array $document, string $datetimeField): int + { + $datetime = $document[$datetimeField] ?? null; + if ($datetime instanceof UTCDateTime) { + return (int) (string) $datetime; + } + + return self::DATETIME_MILLIS_FALLBACK; + } } diff --git a/src/Log/Repository/AbstractLogRepository.php b/src/Log/Repository/AbstractLogRepository.php index 2b0c327..8e8149d 100644 --- a/src/Log/Repository/AbstractLogRepository.php +++ b/src/Log/Repository/AbstractLogRepository.php @@ -15,7 +15,6 @@ */ abstract class AbstractLogRepository extends AbstractAnzuMongoRepository { - protected const string FIELD_ID = '_id'; protected const string FIELD_DATETIME = 'datetime'; protected const string FIELD_CONTEXT_CONTEXT_ID = 'context.contextId'; protected const string REGEX_FLAG_CASE_INSENSITIVE = 'i'; @@ -26,7 +25,6 @@ abstract class AbstractLogRepository extends AbstractAnzuMongoRepository protected const string MONGO_EXISTS = '$exists'; private const int LIMIT_MIN = 1; - private const int SORT_DESC = -1; private const string ID_LOWER_BOUND_SLACK = '-1 minute'; private const string ID_UPPER_BOUND_SLACK = '+1 day'; private const array RAW_ARRAY_TYPE_MAP = [ @@ -45,7 +43,7 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], - self::FIELD_ID => [ + MongoHelper::FIELD_ID => [ self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), ], ], $limit); @@ -65,14 +63,14 @@ protected function findLatestRawDocuments(array $match, int $limit): array { $documents = $this->collection->find($match, [ 'sort' => [ - self::FIELD_ID => self::SORT_DESC, + MongoHelper::FIELD_ID => MongoHelper::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, 'typeMap' => self::RAW_ARRAY_TYPE_MAP, ]); - return array_values($documents->toArray()); + return MongoHelper::sortNewestFirst(array_values($documents->toArray()), self::FIELD_DATETIME); } /** @@ -85,7 +83,7 @@ protected function createDatetimeWindowMatch(DateTimeImmutable $from, DateTimeIm self::MONGO_GTE => new UTCDateTime($from), self::MONGO_LTE => new UTCDateTime($until), ], - self::FIELD_ID => [ + MongoHelper::FIELD_ID => [ self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), self::MONGO_LTE => MongoHelper::maxObjectIdFor($until->modify(self::ID_UPPER_BOUND_SLACK)), ], diff --git a/src/Mcp/Log/McpLogRepository.php b/src/Mcp/Log/McpLogRepository.php index 2a038fd..cc33112 100644 --- a/src/Mcp/Log/McpLogRepository.php +++ b/src/Mcp/Log/McpLogRepository.php @@ -12,12 +12,11 @@ final readonly class McpLogRepository { - private const string FIELD_ID = '_id'; private const string FIELD_DATETIME = 'datetime'; private const string FIELD_CONTEXT_ID = 'contextId'; private const string MONGO_GTE = '$gte'; private const int LIMIT_MIN = 1; - private const int SORT_DESC = -1; + private const string ID_LOWER_BOUND_SLACK = '-1 minute'; private const array RAW_ARRAY_TYPE_MAP = [ 'root' => 'array', 'document' => 'array', @@ -40,18 +39,18 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], - self::FIELD_ID => [ - self::MONGO_GTE => MongoHelper::minObjectIdFor($from), + MongoHelper::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), ], ], [ 'sort' => [ - self::FIELD_ID => self::SORT_DESC, + MongoHelper::FIELD_ID => MongoHelper::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, 'typeMap' => self::RAW_ARRAY_TYPE_MAP, ]); - return array_values($documents->toArray()); + return MongoHelper::sortNewestFirst(array_values($documents->toArray()), self::FIELD_DATETIME); } } diff --git a/src/Resources/doc/helpers.md b/src/Resources/doc/helpers.md index 980e70b..542e749 100644 --- a/src/Resources/doc/helpers.md +++ b/src/Resources/doc/helpers.md @@ -4,6 +4,7 @@ Helpers Common Bundle has some helper classes which should not be used as services and their methods can be called statically. * [CollectionHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/CollectionHelper.php) +* [MongoHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/MongoHelper.php) * [EmailHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/EmailHelper.php) * [PasswordHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/PasswordHelper.php) * [UuidHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/UuidHelper.php) diff --git a/tests/Helper/MongoHelperTest.php b/tests/Helper/MongoHelperTest.php index c653b77..cc36e29 100644 --- a/tests/Helper/MongoHelperTest.php +++ b/tests/Helper/MongoHelperTest.php @@ -5,49 +5,47 @@ namespace AnzuSystems\CommonBundle\Tests\Helper; use AnzuSystems\CommonBundle\Helper\MongoHelper; -use AnzuSystems\CommonBundle\Tests\AnzuKernelTestCase; use DateTimeImmutable; +use MongoDB\BSON\UTCDateTime; +use PHPUnit\Framework\TestCase; -final class MongoHelperTest extends AnzuKernelTestCase +final class MongoHelperTest extends TestCase { - /** - * @dataProvider minObjectIdProvider - */ - public function testMinObjectIdFor(string $expectedResult, DateTimeImmutable $datetime): void - { - self::assertSame($expectedResult, (string) MongoHelper::minObjectIdFor($datetime)); - } + private const string DATETIME_FIELD = 'datetime'; /** - * @dataProvider maxObjectIdProvider + * @dataProvider objectIdBoundsProvider */ - public function testMaxObjectIdFor(string $expectedResult, DateTimeImmutable $datetime): void + public function testObjectIdBounds(string $expectedTimestampHex, DateTimeImmutable $datetime): void { - self::assertSame($expectedResult, (string) MongoHelper::maxObjectIdFor($datetime)); + self::assertSame($expectedTimestampHex . '0000000000000000', (string) MongoHelper::minObjectIdFor($datetime)); + self::assertSame($expectedTimestampHex . 'ffffffffffffffff', (string) MongoHelper::maxObjectIdFor($datetime)); } /** - * @return list + * @return array */ - public function minObjectIdProvider(): array + public static function objectIdBoundsProvider(): array { return [ - ['6a7f7a000000000000000000', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], - ['0000e1000000000000000000', new DateTimeImmutable('1970-01-01 16:00:00+00:00')], - ['000000000000000000000000', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], - ['ffffffff0000000000000000', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], + 'utc' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], + 'sub-second precision is dropped' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 20:26:40.999+00:00')], + 'offset is normalised to utc' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 22:26:40+02:00')], + 'small timestamp is left padded' => ['0000e100', new DateTimeImmutable('1970-01-01 16:00:00+00:00')], + 'negative timestamp is clamped to zero' => ['00000000', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], + 'post 2106 timestamp is clamped to max' => ['ffffffff', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], ]; } - /** - * @return list - */ - public function maxObjectIdProvider(): array + public function testSortNewestFirst(): void { - return [ - ['6a7f7a00ffffffffffffffff', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], - ['00000000ffffffffffffffff', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], - ['ffffffffffffffffffffffff', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], - ]; + $oldest = [self::DATETIME_FIELD => new UTCDateTime(1_000), 'id' => 'oldest']; + $middle = [self::DATETIME_FIELD => new UTCDateTime(2_000), 'id' => 'middle']; + $newest = [self::DATETIME_FIELD => new UTCDateTime(3_000), 'id' => 'newest']; + $missing = ['id' => 'missing']; + + $sorted = MongoHelper::sortNewestFirst([$middle, $missing, $newest, $oldest], self::DATETIME_FIELD); + + self::assertSame(['newest', 'middle', 'oldest', 'missing'], array_column($sorted, 'id')); } } diff --git a/tests/Mcp/McpLogRepositoryTest.php b/tests/Mcp/McpLogRepositoryTest.php new file mode 100644 index 0000000..6e5d7ac --- /dev/null +++ b/tests/Mcp/McpLogRepositoryTest.php @@ -0,0 +1,53 @@ + new UTCDateTime(1_000), + 'id' => 'older', + ]; + $newer = [ + 'datetime' => new UTCDateTime(2_000), + 'id' => 'newer', + ]; + + $cursor = $this->createMock(CursorInterface::class); + $cursor->method('toArray') + ->willReturn([$older, $newer]); + + $collection = $this->createMock(Collection::class); + $collection + ->expects(self::once()) + ->method('find') + ->with( + self::callback(static fn (array $filter): bool => self::CONTEXT_ID === $filter['contextId'] + && (string) new UTCDateTime($from) === (string) $filter['datetime']['$gte'] + && (string) MongoHelper::minObjectIdFor($from->modify('-1 minute')) === (string) $filter[MongoHelper::FIELD_ID]['$gte']), + self::callback(static fn (array $options): bool => [MongoHelper::FIELD_ID => MongoHelper::SORT_DESC] === $options['sort'] + && self::LIMIT === $options['limit']), + ) + ->willReturn($cursor); + + $documents = (new McpLogRepository($collection))->findLatestByContextId(self::CONTEXT_ID, $from, self::LIMIT); + + self::assertSame(['newer', 'older'], array_column($documents, 'id')); + } +} From 2a430a290bda66bc6d3291065f1d57e7eca09f16 Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 13:51:03 +0200 Subject: [PATCH 3/8] Add per-tool MCP permissions and a per-caller rate limit override --- CHANGELOG.md | 7 ++ .../AnzuSystemsCommonExtension.php | 31 ++++-- .../Handler/FilterToolsListRequestHandler.php | 53 +++++++++ src/Mcp/McpRateLimiter.php | 38 ++++++- src/Mcp/McpToolExecutor.php | 11 ++ src/Mcp/Security/McpToolPermission.php | 34 ++++++ src/Mcp/Security/McpToolPermissionConfig.php | 52 +++++++++ .../Security/Voter/McpToolPermissionVoter.php | 24 +++++ src/Resources/config/mcp.php | 19 +++- src/Resources/doc/mcp.md | 70 +++++++++++- ...stemsCommonExtensionMcpPermissionsTest.php | 102 ++++++++++++++++++ .../FilterToolsListRequestHandlerTest.php | 63 +++++++++++ tests/Mcp/McpRateLimiterTest.php | 79 ++++++++++---- tests/Mcp/McpToolExecutorTest.php | 21 +++- .../Security/McpToolPermissionVoterTest.php | 59 ++++++++++ 15 files changed, 629 insertions(+), 34 deletions(-) create mode 100644 src/Mcp/Handler/FilterToolsListRequestHandler.php create mode 100644 src/Mcp/Security/McpToolPermission.php create mode 100644 src/Mcp/Security/McpToolPermissionConfig.php create mode 100644 src/Mcp/Security/Voter/McpToolPermissionVoter.php create mode 100644 tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php create mode 100644 tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php create mode 100644 tests/Mcp/Security/McpToolPermissionVoterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b1e0afe..76bee15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## [Unreleased] + +### Features +* Per-tool MCP permissions: every tool requires `mcp_tool_` (`McpToolPermission::forTool()`), voted by `McpToolPermissionVoter` through the standard permission model (no implicit access, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `Security`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. `McpToolPermission::SEARCH_APP_LOGS|SEARCH_AUDIT_LOGS|GET_LOGS_BY_CONTEXT` name the bundle tool permissions. The bundle prepends the `mcp_tool` permission subject with its log tools into the `permissions` config; projects add their own tool actions (see "Tool permissions" in `src/Resources/doc/mcp.md`). +* `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the rate limiter config array + storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). + ## [12.0.0](https://github.com/anzusystems/common-bundle/compare/11.3.0...12.0.0) (2026-07-22) ### Features @@ -11,6 +17,7 @@ ### Changes * `JournalLogRepository` and `AuditLogRepository` now extend the new `AbstractLogRepository` (public API unchanged). * New `conflict` with `symfony/mcp-bundle >=0.11` — the MCP integration compiles against the 0.10 SDK internals. +* Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()`. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. ## [11.3.0](https://github.com/anzusystems/common-bundle/compare/11.2.0...11.3.0) (2026-07-13) diff --git a/src/DependencyInjection/AnzuSystemsCommonExtension.php b/src/DependencyInjection/AnzuSystemsCommonExtension.php index c258d13..23c8fc0 100644 --- a/src/DependencyInjection/AnzuSystemsCommonExtension.php +++ b/src/DependencyInjection/AnzuSystemsCommonExtension.php @@ -71,7 +71,12 @@ use AnzuSystems\CommonBundle\Log\Repository\AuditLogRepository; use AnzuSystems\CommonBundle\Log\Repository\JournalLogRepository; use AnzuSystems\CommonBundle\Mcp\Controller\McpController; +use AnzuSystems\CommonBundle\Mcp\McpRateLimiter; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermissionConfig; +use AnzuSystems\CommonBundle\Mcp\Tool\GetLogsByContextTool; +use AnzuSystems\CommonBundle\Mcp\Tool\SearchAppLogsTool; +use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; use AnzuSystems\CommonBundle\Messenger\Message\AuditLogMessage; use AnzuSystems\CommonBundle\Messenger\Message\JournalLogMessage; use AnzuSystems\CommonBundle\Request\ParamConverter\ApiFilterParamConverter; @@ -117,6 +122,11 @@ final class AnzuSystemsCommonExtension extends Extension implements PrependExtensionInterface { private const string MCP_TOOL_SCAN_DIR = 'vendor/anzusystems/common-bundle/src/Mcp/Tool'; + private const array MCP_TOOL_NAMES = [ + SearchAppLogsTool::NAME, + SearchAuditLogsTool::NAME, + GetLogsByContextTool::NAME, + ]; private array $processedConfig; @@ -212,6 +222,7 @@ public function prepend(ContainerBuilder $container): void */ public function load(array $configs, ContainerBuilder $container): void { + $this->processedConfig = $this->processConfiguration(new Configuration(), $configs); $loader = new PhpFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.php'); @@ -571,6 +582,9 @@ private function prependMcp(ContainerBuilder $container): void 'scan_dirs' => [self::MCP_TOOL_SCAN_DIR], ], ]); + $container->prependExtensionConfig($this->getAlias(), [ + 'permissions' => McpToolPermissionConfig::forTools(self::MCP_TOOL_NAMES), + ]); } private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): void @@ -624,15 +638,14 @@ private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): $rateLimiterStorageDefinition->setArgument('$pool', new Reference($mcp['rate_limiter']['cache_pool'])); $container->setDefinition('anzu_systems_common.mcp.rate_limiter_storage', $rateLimiterStorageDefinition); - $rateLimiterFactoryDefinition = new Definition(RateLimiterFactory::class); - $rateLimiterFactoryDefinition->setArgument('$config', [ - 'id' => 'mcp', - 'policy' => 'sliding_window', - 'limit' => $mcp['rate_limiter']['limit'], - 'interval' => $mcp['rate_limiter']['interval'], - ]); - $rateLimiterFactoryDefinition->setArgument('$storage', new Reference('anzu_systems_common.mcp.rate_limiter_storage')); - $container->setDefinition('anzu_systems_common.mcp.rate_limiter_factory', $rateLimiterFactoryDefinition); + $container + ->getDefinition(McpRateLimiter::class) + ->replaceArgument('$rateLimiterConfig', [ + 'id' => 'mcp', + 'policy' => 'sliding_window', + 'limit' => $mcp['rate_limiter']['limit'], + 'interval' => $mcp['rate_limiter']['interval'], + ]); $container ->getDefinition(McpController::class) diff --git a/src/Mcp/Handler/FilterToolsListRequestHandler.php b/src/Mcp/Handler/FilterToolsListRequestHandler.php new file mode 100644 index 0000000..bcbe505 --- /dev/null +++ b/src/Mcp/Handler/FilterToolsListRequestHandler.php @@ -0,0 +1,53 @@ + + */ +final readonly class FilterToolsListRequestHandler implements RequestHandlerInterface +{ + public function __construct( + private RegistryInterface $registry, + private Security $security, + private int $pageSize, + ) { + } + + public function supports(Request $request): bool + { + return $request instanceof ListToolsRequest; + } + + public function handle(Request $request, SessionInterface $session): Response|Error + { + if (false === $request instanceof ListToolsRequest) { + return Error::forInternalError('Unsupported request.', $request->getId()); + } + + $page = $this->registry->getTools($this->pageSize, $request->cursor); + $grantedTools = array_values(array_filter( + $page->references, + fn (Tool $tool): bool => $this->security->isGranted(McpToolPermission::forTool($tool->name)), + )); + + return new Response( + $request->getId(), + new ListToolsResult($grantedTools, $page->nextCursor), + ); + } +} diff --git a/src/Mcp/McpRateLimiter.php b/src/Mcp/McpRateLimiter.php index 7749851..0643597 100644 --- a/src/Mcp/McpRateLimiter.php +++ b/src/Mcp/McpRateLimiter.php @@ -6,15 +6,28 @@ use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; use AnzuSystems\Contracts\AnzuApp; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; +use Symfony\Component\RateLimiter\LimiterInterface; use Symfony\Component\RateLimiter\RateLimiterFactory; +use Symfony\Component\RateLimiter\Storage\StorageInterface; final readonly class McpRateLimiter { + public const string TOKEN_ATTRIBUTE_KEY = 'mcp_rate_limit_key'; + public const string TOKEN_ATTRIBUTE_LIMIT = 'mcp_rate_limit'; + + private const string CONFIG_LIMIT = 'limit'; + + /** + * @param array $rateLimiterConfig + */ public function __construct( - private RateLimiterFactory $mcpLimiter, + private array $rateLimiterConfig, + private StorageInterface $storage, private CurrentAnzuUserProvider $currentUserProvider, + private Security $security, ) { } @@ -30,8 +43,11 @@ public function checkRateLimit(): void throw new AccessDeniedHttpException('Anonymous access to the MCP endpoint is not allowed.'); } - $limiter = $this->mcpLimiter->create((string) $userId); - $limit = $limiter->consume(); + $key = $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_KEY); + $limit = $this->createLimiter( + is_string($key) ? $key : (string) $userId, + $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_LIMIT), + )->consume(); if ($limit->isAccepted()) { return; } @@ -49,4 +65,20 @@ public function checkRateLimit(): void ], ); } + + private function resolveTokenAttribute(string $name): mixed + { + return $this->security->getToken()?->getAttributes()[$name] ?? null; + } + + private function createLimiter(string $key, mixed $limitOverride): LimiterInterface + { + $config = $this->rateLimiterConfig; + if (is_int($limitOverride) && $limitOverride > 0) { + $config[self::CONFIG_LIMIT] = $limitOverride; + } + + return new RateLimiterFactory($config, $this->storage) + ->create($key); + } } diff --git a/src/Mcp/McpToolExecutor.php b/src/Mcp/McpToolExecutor.php index 403ec05..11bf497 100644 --- a/src/Mcp/McpToolExecutor.php +++ b/src/Mcp/McpToolExecutor.php @@ -7,9 +7,11 @@ use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; use Closure; use Monolog\Attribute\WithMonologChannel; use Psr\Log\LoggerInterface; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Throwable; @@ -18,6 +20,8 @@ { public const string ERROR_KEY = 'error'; + private const string TOOL_ACCESS_DENIED_MESSAGE = 'Access denied — the current MCP user is not allowed to use the tool "%s".'; + /** * @param array, string> $toolErrorExceptions */ @@ -25,6 +29,7 @@ public function __construct( private CurrentAnzuUserProvider $currentUserProvider, private LoggerInterface $logger, private McpLogger $mcpLogger, + private Security $security, private array $toolErrorExceptions = [], ) { } @@ -41,6 +46,12 @@ public function execute(string $toolName, array $params, Closure $callback): arr $error = null; try { + if (false === $this->security->isGranted(McpToolPermission::forTool($toolName))) { + $error = sprintf(self::TOOL_ACCESS_DENIED_MESSAGE, $toolName); + + return [self::ERROR_KEY => $error]; + } + return $callback(); } catch (McpToolInputException $exception) { $error = $exception->getMessage(); diff --git a/src/Mcp/Security/McpToolPermission.php b/src/Mcp/Security/McpToolPermission.php new file mode 100644 index 0000000..aae1421 --- /dev/null +++ b/src/Mcp/Security/McpToolPermission.php @@ -0,0 +1,34 @@ +camel() + ->toString(); + } + + public static function isToolPermission(string $permission): bool + { + return str_starts_with($permission, self::PREFIX); + } +} diff --git a/src/Mcp/Security/McpToolPermissionConfig.php b/src/Mcp/Security/McpToolPermissionConfig.php new file mode 100644 index 0000000..879c356 --- /dev/null +++ b/src/Mcp/Security/McpToolPermissionConfig.php @@ -0,0 +1,52 @@ + $toolNames + * + * @return array> + */ + public static function forTools(array $toolNames): array + { + $actions = []; + $actionTranslations = []; + foreach ($toolNames as $toolName) { + $action = McpToolPermission::toAction($toolName); + $actions[$action] = []; + $actionTranslations[$action] = [self::LOCALE_EN => self::toLabel($toolName)]; + } + + return [ + PermissionConfig::PRM_CONFIG => [ + McpToolPermission::SUBJECT => $actions, + ], + PermissionConfig::PRM_TRANSLATION => [ + 'subjects' => [ + McpToolPermission::SUBJECT => [self::LOCALE_EN => self::SUBJECT_TITLE], + ], + 'actions' => $actionTranslations, + ], + ]; + } + + private static function toLabel(string $toolName): string + { + return new UnicodeString($toolName) + ->replace(self::TOOL_NAME_WORD_SEPARATOR, self::LABEL_WORD_SEPARATOR) + ->title() + ->toString(); + } +} diff --git a/src/Mcp/Security/Voter/McpToolPermissionVoter.php b/src/Mcp/Security/Voter/McpToolPermissionVoter.php new file mode 100644 index 0000000..b216adb --- /dev/null +++ b/src/Mcp/Security/Voter/McpToolPermissionVoter.php @@ -0,0 +1,24 @@ + + */ +final class McpToolPermissionVoter extends AbstractVoter +{ + protected function supports(string $attribute, mixed $subject): bool + { + return McpToolPermission::isToolPermission($attribute); + } + + protected function getSupportedPermissions(): array + { + return []; + } +} diff --git a/src/Resources/config/mcp.php b/src/Resources/config/mcp.php index 9f1a171..268ea80 100644 --- a/src/Resources/config/mcp.php +++ b/src/Resources/config/mcp.php @@ -9,6 +9,7 @@ use AnzuSystems\CommonBundle\Log\Repository\AuditLogRepository; use AnzuSystems\CommonBundle\Log\Repository\JournalLogRepository; use AnzuSystems\CommonBundle\Mcp\Controller\McpController; +use AnzuSystems\CommonBundle\Mcp\Handler\FilterToolsListRequestHandler; use AnzuSystems\CommonBundle\Mcp\Handler\StrictToolArgumentsRequestHandler; use AnzuSystems\CommonBundle\Mcp\Log\McpLogFinder; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; @@ -17,6 +18,7 @@ use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; use AnzuSystems\CommonBundle\Mcp\Resolver\McpContextIdResolver; use AnzuSystems\CommonBundle\Mcp\Resolver\McpDateWindowResolver; +use AnzuSystems\CommonBundle\Mcp\Security\Voter\McpToolPermissionVoter; use AnzuSystems\CommonBundle\Mcp\Tool\GetLogsByContextTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAppLogsTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; @@ -41,6 +43,18 @@ ->tag('monolog.logger', ['channel' => 'mcp']) ; + $services->set(FilterToolsListRequestHandler::class) + ->arg('$registry', service('mcp.registry')) + ->arg('$security', service('security.helper')) + ->arg('$pageSize', param('mcp.pagination_limit')) + ->tag('mcp.request_handler') + ; + + $services->set(McpToolPermissionVoter::class) + ->call('setSecurity', [service('security.helper')]) + ->tag('security.voter') + ; + $services->set(McpLogger::class) ->arg('$mcpLogCollection', service('anzu_mongo_mcp_log_collection')) ; @@ -61,13 +75,16 @@ ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) ->arg('$logger', service('logger')) ->arg('$mcpLogger', service(McpLogger::class)) + ->arg('$security', service('security.helper')) ->arg('$toolErrorExceptions', null) ->tag('monolog.logger', ['channel' => 'mcp']) ; $services->set(McpRateLimiter::class) - ->arg('$mcpLimiter', service('anzu_systems_common.mcp.rate_limiter_factory')) + ->arg('$rateLimiterConfig', null) + ->arg('$storage', service('anzu_systems_common.mcp.rate_limiter_storage')) ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) + ->arg('$security', service('security.helper')) ; $services->set(McpController::class) diff --git a/src/Resources/doc/mcp.md b/src/Resources/doc/mcp.md index d1f9a55..b91f87b 100644 --- a/src/Resources/doc/mcp.md +++ b/src/Resources/doc/mcp.md @@ -62,10 +62,78 @@ infrastructure after a bundle upgrade. ## Provided services * `McpController` (alias `mcp.server.controller`) — streamable HTTP transport endpoint with DNS-rebinding protection - (`allowed_hosts`) and a per-user sliding-window rate limit. + (`allowed_hosts`) and a sliding-window rate limit (per user, or per caller when the security token carries the + rate-limit attributes — see below). * `McpToolExecutor` — wraps tool callbacks: converts `McpToolInputException`, `AccessDeniedException` and configured `tool_error_exceptions` into tool error results, logs every call to the monolog `mcp` channel and to the `mcpLogs` capped collection. * `StrictToolArgumentsRequestHandler` — rejects tool calls with unknown arguments. +* `McpToolPermissionVoter` + `FilterToolsListRequestHandler` — per-tool permissions (see below); the call-side check + lives in `McpToolExecutor`. * `SearchAppLogsTool`, `SearchAuditLogsTool`, `GetLogsByContextTool` — diagnostic tools over the shared log collections, correlated by `contextId`. + +## Rate limit override per caller + +`McpRateLimiter` keys the sliding window by the current user id and applies the configured `rate_limiter.limit`. +An authenticator can override both by setting attributes on the security token: + +```php +$token->setAttribute(McpRateLimiter::TOKEN_ATTRIBUTE_KEY, 'pat_' . $personalAccessToken->getId()); +$token->setAttribute(McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT, $personalAccessToken->getRateLimit()); +``` + +`TOKEN_ATTRIBUTE_KEY` (string) selects the bucket (e.g. one bucket per personal access token), `TOKEN_ATTRIBUTE_LIMIT` +(positive `int`) replaces the configured limit for that bucket — `null`, a missing attribute or a non-positive value +keeps the configured default. Tokens without the attributes fall back to the per-user bucket. The configured +`rate_limiter.interval` applies to every bucket. The personal access token authenticator from +[anzusystems/auth-bundle](https://github.com/anzusystems/auth-bundle) sets both attributes. + +## Tool permissions + +Every registered tool requires the permission `mcp_tool_` (`McpToolPermission::forTool()`), e.g. +`search_app_logs` → `mcp_tool_searchAppLogs`. The permission is resolved through the standard permission model +(`AnzuUser::getResolvedPermissions()` from the user's permissions + permission groups, `Grant::ALLOW`/`Grant::DENY`, +super admin bypass) by `McpToolPermissionVoter`; a permission that is not granted denies the tool — there is no +implicit access. + +* `tools/call` of a tool without the grant returns a tool error result (`{"error": "Access denied — …"}`) from + `McpToolExecutor::execute()` before the tool callback runs, and the denied call is written to the `mcpLogs` collection + with the error (so every tool must route through the executor, as the bundle tools do). +* `tools/list` returns only the tools the current user may call — the filter runs after registry pagination, so a + page may come back with fewer tools (even none) while `nextCursor` is still set; clients follow the cursor as usual. + +The bundle prepends the `mcp_tool` subject with its own tool actions (`searchAppLogs`, `searchAuditLogs`, +`getLogsByContext`) and their translations into the `permissions` config, so they show up in the admin permission +editor. Projects add their own tools to the same subject (`McpToolPermission::toAction()` gives the action name): + +```yaml +anzu_systems_common: + permissions: + config: + mcp_tool: + listPublishedArticles: + getArticles: + translation: + actions: + listPublishedArticles: { en: List published articles, sk: Zoznam publikovaných článkov } + getArticles: { en: Get articles, sk: Detail článkov } +``` + +## Log query bounds + +The log repositories behind the diagnostic tools (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, +`findLatestByContextId()` and `McpLogRepository::findLatestByContextId()`) have no index on `datetime`; they bound and sort +every query by the built-in `_id` index instead, deriving the ObjectId range from the `datetime` window via +`MongoHelper::minObjectIdFor()` / `maxObjectIdFor()`. The `datetime` filter stays in place, so the `_id` range only +narrows the scan. Assumptions behind the bounds: + +* Journal and audit records are written asynchronously (Messenger consumer), so the `_id` timestamp is the insertion time, + not the record `datetime`. The range is widened by `-1 minute` below and `+1 day` above the window; a record inserted more + than one day after its `datetime` (consumer outage, failed-message replay) falls outside a historical window. +* Records are selected in insertion (`_id`) order and the returned page is re-sorted by `datetime`; with a `limit`, the + selected newest rows are those inserted last, which can differ from a strict `datetime` ordering when consumers run in + parallel. +* For selective filters (`contextId`, `onlyErrors`) over large windows the scan still fetches every document in the `_id` + range until `limit` is filled, guarded only by `mongo_query_max_time_ms`; a compound `{ 'context.contextId': 1, _id: -1 }` + index on `appLogs` / `auditLogs` is the real fix for by-context lookups on busy collections. diff --git a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php new file mode 100644 index 0000000..c2c5448 --- /dev/null +++ b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php @@ -0,0 +1,102 @@ + 'mongodb://localhost', + 'username' => 'user', + 'password' => 'password', + 'database' => 'logs', + ]; + + public function testBundleToolPermissionsAreMergedWithProjectPermissions(): void + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.project_dir', sys_get_temp_dir()); + $container->registerExtension($this->createMcpExtensionStub()); + $extension = new AnzuSystemsCommonExtension(); + $container->registerExtension($extension); + $container->loadFromExtension(self::EXTENSION_ALIAS, $this->createConfig()); + + $extension->prepend($container); + $extension->load($container->getExtensionConfig(self::EXTENSION_ALIAS), $container); + + $permissions = $container->getDefinition(PermissionConfig::class) + ->getArgument('$config'); + self::assertIsArray($permissions); + $actions = array_keys($permissions[PermissionConfig::PRM_CONFIG][McpToolPermission::SUBJECT]); + self::assertContains(McpToolPermission::toAction(SearchAppLogsTool::NAME), $actions); + self::assertContains(McpToolPermission::toAction(SearchAuditLogsTool::NAME), $actions); + self::assertContains(McpToolPermission::toAction(GetLogsByContextTool::NAME), $actions); + self::assertContains(self::PROJECT_ACTION, $actions); + self::assertArrayHasKey( + McpToolPermission::SUBJECT, + $permissions[PermissionConfig::PRM_TRANSLATION]['subjects'], + ); + } + + /** + * @return array + */ + private function createConfig(): array + { + return [ + 'settings' => [ + 'app_redis' => 'redis://localhost', + ], + 'logs' => [ + 'messenger_transport' => [ + 'name' => 'logs', + 'dsn' => 'in-memory://', + ], + 'journal' => [ + 'mongo' => self::MONGO, + ], + 'audit' => [ + 'mongo' => self::MONGO, + ], + ], + 'mcp' => [ + 'enabled' => true, + 'allowed_hosts' => ['localhost'], + ], + 'permissions' => [ + PermissionConfig::PRM_CONFIG => [ + McpToolPermission::SUBJECT => [ + self::PROJECT_ACTION => [], + ], + ], + ], + ]; + } + + private function createMcpExtensionStub(): Extension + { + return new class() extends Extension { + public function load(array $configs, ContainerBuilder $container): void + { + } + + public function getAlias(): string + { + return 'mcp'; + } + }; + } +} diff --git a/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php new file mode 100644 index 0000000..eb6a374 --- /dev/null +++ b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php @@ -0,0 +1,63 @@ +createHandler(); + + self::assertTrue($handler->supports(new ListToolsRequest()->withId(self::REQUEST_ID))); + self::assertFalse($handler->supports(new CallToolRequest(self::GRANTED_TOOL_NAME, [])->withId(self::REQUEST_ID))); + } + + public function testHandleReturnsOnlyGrantedTools(): void + { + $response = $this->createHandler() + ->handle(new ListToolsRequest()->withId(self::REQUEST_ID), $this->createStub(SessionInterface::class)); + + self::assertInstanceOf(Response::class, $response); + self::assertInstanceOf(ListToolsResult::class, $response->result); + self::assertCount(1, $response->result->tools); + self::assertSame(self::GRANTED_TOOL_NAME, $response->result->tools[0]->name); + } + + private function createHandler(): FilterToolsListRequestHandler + { + $tools = []; + foreach ([self::GRANTED_TOOL_NAME, self::DENIED_TOOL_NAME] as $toolName) { + $tools[$toolName] = new Tool($toolName, null, ['type' => 'object'], null, null); + } + $registry = $this->createMock(RegistryInterface::class); + $registry->method('getTools') + ->with(self::PAGE_SIZE, null) + ->willReturn(new Page($tools, null)); + + $security = $this->createMock(Security::class); + $security->method('isGranted') + ->willReturnCallback(static fn (mixed $attribute): bool => McpToolPermission::forTool(self::GRANTED_TOOL_NAME) === $attribute); + + return new FilterToolsListRequestHandler($registry, $security, self::PAGE_SIZE); + } +} diff --git a/tests/Mcp/McpRateLimiterTest.php b/tests/Mcp/McpRateLimiterTest.php index bea3cfb..5fc677a 100644 --- a/tests/Mcp/McpRateLimiterTest.php +++ b/tests/Mcp/McpRateLimiterTest.php @@ -9,33 +9,43 @@ use AnzuSystems\Contracts\AnzuApp; use AnzuSystems\Contracts\Entity\AnzuUser; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; -use Symfony\Component\RateLimiter\RateLimiterFactory; use Symfony\Component\RateLimiter\Storage\InMemoryStorage; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\InMemoryUser; final class McpRateLimiterTest extends TestCase { private const int LIMIT = 1; + private const int LIMIT_OVERRIDE = 2; private const int INTERVAL_SECONDS = 60; private const int USER_ID = 42; + private const string TOKEN_KEY = 'pat_7'; + private const string FIREWALL_NAME = 'mcp'; + private const string USER_IDENTIFIER = 'mcp-user'; + private const array RATE_LIMITER_CONFIG = [ + 'id' => 'mcp_test', + 'policy' => 'sliding_window', + 'limit' => self::LIMIT, + 'interval' => '1 minute', + ]; public function testAnonymousUserIsRejected(): void { - $rateLimiter = new McpRateLimiter( - $this->createLimiterFactory(), - $this->createCurrentUserProvider(AnzuApp::getUserIdAnonymous()), - ); + $rateLimiter = $this->createRateLimiter(AnzuApp::getUserIdAnonymous()); $this->expectException(AccessDeniedHttpException::class); $rateLimiter->checkRateLimit(); } - public function testThrowsWhenLimitExceeded(): void + public function testTokenWithoutAttributesThrowsWhenDefaultLimitExceeded(): void { - $rateLimiter = new McpRateLimiter($this->createLimiterFactory(), $this->createCurrentUserProvider()); + $rateLimiter = $this->createRateLimiter(token: $this->createToken([])); $rateLimiter->checkRateLimit(); try { @@ -53,26 +63,57 @@ public function testThrowsWhenLimitExceeded(): void } } - private function createLimiterFactory(): RateLimiterFactory + public function testTokenAttributesOverrideLimitAndKey(): void { - return new RateLimiterFactory( - [ - 'id' => 'mcp_test', - 'policy' => 'sliding_window', - 'limit' => self::LIMIT, - 'interval' => '1 minute', - ], - new InMemoryStorage(), - ); + $rateLimiter = $this->createRateLimiter(token: $this->createToken([ + McpRateLimiter::TOKEN_ATTRIBUTE_KEY => self::TOKEN_KEY, + McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT => self::LIMIT_OVERRIDE, + ])); + $rateLimiter->checkRateLimit(); + $rateLimiter->checkRateLimit(); + + try { + $rateLimiter->checkRateLimit(); + self::fail('Expected ' . TooManyRequestsHttpException::class); + } catch (TooManyRequestsHttpException $exception) { + self::assertSame((string) self::LIMIT_OVERRIDE, $exception->getHeaders()['X-RateLimit-Limit']); + } } - private function createCurrentUserProvider(int $userId = self::USER_ID): CurrentAnzuUserProvider + public function testNullLimitAttributeFallsBackToDefaultLimit(): void + { + $rateLimiter = $this->createRateLimiter(token: $this->createToken([ + McpRateLimiter::TOKEN_ATTRIBUTE_KEY => self::TOKEN_KEY, + McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT => null, + ])); + $rateLimiter->checkRateLimit(); + + $this->expectException(TooManyRequestsHttpException::class); + + $rateLimiter->checkRateLimit(); + } + + private function createRateLimiter(int $userId = self::USER_ID, ?TokenInterface $token = null): McpRateLimiter { $user = $this->createConfiguredMock(AnzuUser::class, ['getId' => $userId]); $currentUserProvider = $this->createMock(CurrentAnzuUserProvider::class); $currentUserProvider->method('getCurrentUser') ->willReturn($user); + $security = $this->createMock(Security::class); + $security->method('getToken') + ->willReturn($token); + + return new McpRateLimiter(self::RATE_LIMITER_CONFIG, new InMemoryStorage(), $currentUserProvider, $security); + } + + /** + * @param array $attributes + */ + private function createToken(array $attributes): TokenInterface + { + $token = new UsernamePasswordToken(new InMemoryUser(self::USER_IDENTIFIER, null), self::FIREWALL_NAME); + $token->setAttributes($attributes); - return $currentUserProvider; + return $token; } } diff --git a/tests/Mcp/McpToolExecutorTest.php b/tests/Mcp/McpToolExecutorTest.php index 950fba8..b9a59fb 100644 --- a/tests/Mcp/McpToolExecutorTest.php +++ b/tests/Mcp/McpToolExecutorTest.php @@ -8,12 +8,14 @@ use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; use AnzuSystems\Contracts\Entity\AnzuUser; use MongoDB\Collection; use MongoDB\InsertOneResult; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use RuntimeException; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Security\Core\Exception\AccessDeniedException; final class McpToolExecutorTest extends TestCase @@ -53,6 +55,16 @@ public function testInputExceptionIsReturnedAsToolError(): void self::assertSame('Invalid input.', $this->insertedDocuments[0]['error']); } + public function testMissingToolPermissionIsReturnedAsToolErrorWithoutRunningCallback(): void + { + $result = $this->createExecutor(toolGranted: false) + ->execute(self::TOOL_NAME, [], static fn (): array => throw new RuntimeException('must not run')); + + self::assertStringContainsString('Access denied', $result[McpToolExecutor::ERROR_KEY]); + self::assertStringContainsString(self::TOOL_NAME, $result[McpToolExecutor::ERROR_KEY]); + self::assertSame(McpLogger::LEVEL_NAME_ERROR, $this->insertedDocuments[0]['levelName']); + } + public function testAccessDeniedIsReturnedAsToolError(): void { $result = $this->createExecutor() @@ -98,7 +110,7 @@ public function testUnknownExceptionIsRethrownAndLoggedAsError(): void /** * @param array $toolErrorExceptions */ - private function createExecutor(array $toolErrorExceptions = []): McpToolExecutor + private function createExecutor(array $toolErrorExceptions = [], bool $toolGranted = true): McpToolExecutor { $this->insertedDocuments = []; @@ -116,10 +128,17 @@ private function createExecutor(array $toolErrorExceptions = []): McpToolExecuto $currentUserProvider->method('getCurrentUser') ->willReturn($user); + $security = $this->createMock(Security::class); + $security->method('isGranted') + ->willReturnCallback( + static fn (mixed $attribute): bool => $toolGranted && McpToolPermission::forTool(self::TOOL_NAME) === $attribute + ); + return new McpToolExecutor( $currentUserProvider, new NullLogger(), new McpLogger($collection), + $security, $toolErrorExceptions, ); } diff --git a/tests/Mcp/Security/McpToolPermissionVoterTest.php b/tests/Mcp/Security/McpToolPermissionVoterTest.php new file mode 100644 index 0000000..567308a --- /dev/null +++ b/tests/Mcp/Security/McpToolPermissionVoterTest.php @@ -0,0 +1,59 @@ + $resolvedPermissions + */ + #[DataProvider('voteProvider')] + public function testVote(array $resolvedPermissions, string $attribute, int $expectedVote): void + { + $user = $this->createConfiguredMock(AnzuUser::class, ['getResolvedPermissions' => $resolvedPermissions]); + $token = $this->createConfiguredMock(TokenInterface::class, ['getUser' => $user]); + $voter = new McpToolPermissionVoter(); + $voter->setSecurity($this->createMock(Security::class)); + + self::assertSame($expectedVote, $voter->vote($token, null, [$attribute])); + } + + /** + * @return iterable, 1: string, 2: int}> + */ + public static function voteProvider(): iterable + { + $permission = McpToolPermission::forTool(self::TOOL_NAME); + + yield 'allow grant' => [[$permission => Grant::ALLOW], $permission, VoterInterface::ACCESS_GRANTED]; + yield 'missing permission denies by default' => [[], $permission, VoterInterface::ACCESS_DENIED]; + yield 'foreign permission abstains' => [[], 'cms_article_read', VoterInterface::ACCESS_ABSTAIN]; + } +} From 75ad926b0bbefe39a1cfa0cbb287378f7104268d Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 14:12:12 +0200 Subject: [PATCH 4/8] Narrow tools list filtering to Tool references and keep the MCP test set minimal --- src/Mcp/Handler/FilterToolsListRequestHandler.php | 10 ++++++---- ...AnzuSystemsCommonExtensionMcpPermissionsTest.php | 3 +++ tests/Mcp/McpRateLimiterTest.php | 13 ------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/Mcp/Handler/FilterToolsListRequestHandler.php b/src/Mcp/Handler/FilterToolsListRequestHandler.php index bcbe505..892b964 100644 --- a/src/Mcp/Handler/FilterToolsListRequestHandler.php +++ b/src/Mcp/Handler/FilterToolsListRequestHandler.php @@ -40,10 +40,12 @@ public function handle(Request $request, SessionInterface $session): Response|Er } $page = $this->registry->getTools($this->pageSize, $request->cursor); - $grantedTools = array_values(array_filter( - $page->references, - fn (Tool $tool): bool => $this->security->isGranted(McpToolPermission::forTool($tool->name)), - )); + $grantedTools = []; + foreach ($page->references as $reference) { + if ($reference instanceof Tool && $this->security->isGranted(McpToolPermission::forTool($reference->name))) { + $grantedTools[] = $reference; + } + } return new Response( $request->getId(), diff --git a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php index c2c5448..bddd951 100644 --- a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php +++ b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php @@ -18,6 +18,7 @@ final class AnzuSystemsCommonExtensionMcpPermissionsTest extends TestCase { private const string EXTENSION_ALIAS = 'anzu_systems_common'; private const string PROJECT_ACTION = 'listSites'; + private const string KERNEL_ENVIRONMENT = 'test'; private const array MONGO = [ 'uri' => 'mongodb://localhost', 'username' => 'user', @@ -29,6 +30,8 @@ public function testBundleToolPermissionsAreMergedWithProjectPermissions(): void { $container = new ContainerBuilder(); $container->setParameter('kernel.project_dir', sys_get_temp_dir()); + $container->setParameter('kernel.environment', self::KERNEL_ENVIRONMENT); + $container->setParameter('kernel.debug', false); $container->registerExtension($this->createMcpExtensionStub()); $extension = new AnzuSystemsCommonExtension(); $container->registerExtension($extension); diff --git a/tests/Mcp/McpRateLimiterTest.php b/tests/Mcp/McpRateLimiterTest.php index 5fc677a..fa36553 100644 --- a/tests/Mcp/McpRateLimiterTest.php +++ b/tests/Mcp/McpRateLimiterTest.php @@ -80,19 +80,6 @@ public function testTokenAttributesOverrideLimitAndKey(): void } } - public function testNullLimitAttributeFallsBackToDefaultLimit(): void - { - $rateLimiter = $this->createRateLimiter(token: $this->createToken([ - McpRateLimiter::TOKEN_ATTRIBUTE_KEY => self::TOKEN_KEY, - McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT => null, - ])); - $rateLimiter->checkRateLimit(); - - $this->expectException(TooManyRequestsHttpException::class); - - $rateLimiter->checkRateLimit(); - } - private function createRateLimiter(int $userId = self::USER_ID, ?TokenInterface $token = null): McpRateLimiter { $user = $this->createConfiguredMock(AnzuUser::class, ['getId' => $userId]); From 689e88693e0ef8609a9e6f0044f256327ca449f4 Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 14:17:17 +0200 Subject: [PATCH 5/8] Use the annotation data provider supported by the bundle test runner --- tests/Mcp/Security/McpToolPermissionVoterTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Mcp/Security/McpToolPermissionVoterTest.php b/tests/Mcp/Security/McpToolPermissionVoterTest.php index 567308a..b05af04 100644 --- a/tests/Mcp/Security/McpToolPermissionVoterTest.php +++ b/tests/Mcp/Security/McpToolPermissionVoterTest.php @@ -11,7 +11,6 @@ use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; use AnzuSystems\Contracts\Entity\AnzuUser; use AnzuSystems\Contracts\Security\Grant; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -33,8 +32,9 @@ public function testForToolBuildsCamelCasePermission(): void /** * @param array $resolvedPermissions + * + * @dataProvider voteProvider */ - #[DataProvider('voteProvider')] public function testVote(array $resolvedPermissions, string $attribute, int $expectedVote): void { $user = $this->createConfiguredMock(AnzuUser::class, ['getResolvedPermissions' => $resolvedPermissions]); From c13549b0336fa66efb95dc5649324bb0bf863a79 Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 14:48:07 +0200 Subject: [PATCH 6/8] Round two review fixes: empty rate limit key fallback, bucket key assertion, changelog placement --- CHANGELOG.md | 5 ++++- src/Mcp/McpRateLimiter.php | 3 ++- src/Resources/doc/helpers.md | 2 +- ...stemsCommonExtensionMcpPermissionsTest.php | 2 -- tests/Mcp/McpRateLimiterTest.php | 20 +++++++++++++++---- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76bee15..07d8a43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,12 @@ ### Features * Per-tool MCP permissions: every tool requires `mcp_tool_` (`McpToolPermission::forTool()`), voted by `McpToolPermissionVoter` through the standard permission model (no implicit access, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `Security`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. `McpToolPermission::SEARCH_APP_LOGS|SEARCH_AUDIT_LOGS|GET_LOGS_BY_CONTEXT` name the bundle tool permissions. The bundle prepends the `mcp_tool` permission subject with its log tools into the `permissions` config; projects add their own tool actions (see "Tool permissions" in `src/Resources/doc/mcp.md`). +* Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()` and `FIELD_ID` / `SORT_DESC` constants. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. * `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the rate limiter config array + storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). +### Changes +* BC change: `AbstractLogRepository::FIELD_ID` (protected) moved to `MongoHelper::FIELD_ID`; `McpToolExecutor` requires `Security` (its tool-permission check). + ## [12.0.0](https://github.com/anzusystems/common-bundle/compare/11.3.0...12.0.0) (2026-07-22) ### Features @@ -17,7 +21,6 @@ ### Changes * `JournalLogRepository` and `AuditLogRepository` now extend the new `AbstractLogRepository` (public API unchanged). * New `conflict` with `symfony/mcp-bundle >=0.11` — the MCP integration compiles against the 0.10 SDK internals. -* Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()`. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. ## [11.3.0](https://github.com/anzusystems/common-bundle/compare/11.2.0...11.3.0) (2026-07-13) diff --git a/src/Mcp/McpRateLimiter.php b/src/Mcp/McpRateLimiter.php index 0643597..8288f28 100644 --- a/src/Mcp/McpRateLimiter.php +++ b/src/Mcp/McpRateLimiter.php @@ -5,6 +5,7 @@ namespace AnzuSystems\CommonBundle\Mcp; use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; +use AnzuSystems\CommonBundle\Helper\StringHelper; use AnzuSystems\Contracts\AnzuApp; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; @@ -45,7 +46,7 @@ public function checkRateLimit(): void $key = $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_KEY); $limit = $this->createLimiter( - is_string($key) ? $key : (string) $userId, + is_string($key) && StringHelper::isNotEmpty($key) ? $key : (string) $userId, $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_LIMIT), )->consume(); if ($limit->isAccepted()) { diff --git a/src/Resources/doc/helpers.md b/src/Resources/doc/helpers.md index 542e749..d1bbc17 100644 --- a/src/Resources/doc/helpers.md +++ b/src/Resources/doc/helpers.md @@ -4,7 +4,7 @@ Helpers Common Bundle has some helper classes which should not be used as services and their methods can be called statically. * [CollectionHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/CollectionHelper.php) -* [MongoHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/MongoHelper.php) * [EmailHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/EmailHelper.php) +* [MongoHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/MongoHelper.php) * [PasswordHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/PasswordHelper.php) * [UuidHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/UuidHelper.php) diff --git a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php index bddd951..d7547aa 100644 --- a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php +++ b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php @@ -29,9 +29,7 @@ final class AnzuSystemsCommonExtensionMcpPermissionsTest extends TestCase public function testBundleToolPermissionsAreMergedWithProjectPermissions(): void { $container = new ContainerBuilder(); - $container->setParameter('kernel.project_dir', sys_get_temp_dir()); $container->setParameter('kernel.environment', self::KERNEL_ENVIRONMENT); - $container->setParameter('kernel.debug', false); $container->registerExtension($this->createMcpExtensionStub()); $extension = new AnzuSystemsCommonExtension(); $container->registerExtension($extension); diff --git a/tests/Mcp/McpRateLimiterTest.php b/tests/Mcp/McpRateLimiterTest.php index fa36553..28faff2 100644 --- a/tests/Mcp/McpRateLimiterTest.php +++ b/tests/Mcp/McpRateLimiterTest.php @@ -65,10 +65,11 @@ public function testTokenWithoutAttributesThrowsWhenDefaultLimitExceeded(): void public function testTokenAttributesOverrideLimitAndKey(): void { + $storage = new InMemoryStorage(); $rateLimiter = $this->createRateLimiter(token: $this->createToken([ McpRateLimiter::TOKEN_ATTRIBUTE_KEY => self::TOKEN_KEY, McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT => self::LIMIT_OVERRIDE, - ])); + ]), storage: $storage); $rateLimiter->checkRateLimit(); $rateLimiter->checkRateLimit(); @@ -78,10 +79,16 @@ public function testTokenAttributesOverrideLimitAndKey(): void } catch (TooManyRequestsHttpException $exception) { self::assertSame((string) self::LIMIT_OVERRIDE, $exception->getHeaders()['X-RateLimit-Limit']); } + + $this->createRateLimiter(token: $this->createToken([]), storage: $storage) + ->checkRateLimit(); } - private function createRateLimiter(int $userId = self::USER_ID, ?TokenInterface $token = null): McpRateLimiter - { + private function createRateLimiter( + int $userId = self::USER_ID, + ?TokenInterface $token = null, + ?InMemoryStorage $storage = null, + ): McpRateLimiter { $user = $this->createConfiguredMock(AnzuUser::class, ['getId' => $userId]); $currentUserProvider = $this->createMock(CurrentAnzuUserProvider::class); $currentUserProvider->method('getCurrentUser') @@ -90,7 +97,12 @@ private function createRateLimiter(int $userId = self::USER_ID, ?TokenInterface $security->method('getToken') ->willReturn($token); - return new McpRateLimiter(self::RATE_LIMITER_CONFIG, new InMemoryStorage(), $currentUserProvider, $security); + return new McpRateLimiter( + self::RATE_LIMITER_CONFIG, + $storage ?? new InMemoryStorage(), + $currentUserProvider, + $security, + ); } /** From fae8d0f2a6960913632ff95b6d990c02cd9abe88 Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 15:25:36 +0200 Subject: [PATCH 7/8] Pass the configured limit and interval to McpRateLimiter as scalars --- CHANGELOG.md | 2 +- .../AnzuSystemsCommonExtension.php | 8 ++---- src/Mcp/McpRateLimiter.php | 25 +++++++++++-------- src/Resources/config/mcp.php | 3 ++- tests/Mcp/McpRateLimiterTest.php | 10 +++----- 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07d8a43..02cb726 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ### Features * Per-tool MCP permissions: every tool requires `mcp_tool_` (`McpToolPermission::forTool()`), voted by `McpToolPermissionVoter` through the standard permission model (no implicit access, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `Security`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. `McpToolPermission::SEARCH_APP_LOGS|SEARCH_AUDIT_LOGS|GET_LOGS_BY_CONTEXT` name the bundle tool permissions. The bundle prepends the `mcp_tool` permission subject with its log tools into the `permissions` config; projects add their own tool actions (see "Tool permissions" in `src/Resources/doc/mcp.md`). * Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()` and `FIELD_ID` / `SORT_DESC` constants. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. -* `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the rate limiter config array + storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). +* `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the configured `limit` + `interval` and the storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). ### Changes * BC change: `AbstractLogRepository::FIELD_ID` (protected) moved to `MongoHelper::FIELD_ID`; `McpToolExecutor` requires `Security` (its tool-permission check). diff --git a/src/DependencyInjection/AnzuSystemsCommonExtension.php b/src/DependencyInjection/AnzuSystemsCommonExtension.php index 23c8fc0..af2c4cc 100644 --- a/src/DependencyInjection/AnzuSystemsCommonExtension.php +++ b/src/DependencyInjection/AnzuSystemsCommonExtension.php @@ -640,12 +640,8 @@ private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): $container ->getDefinition(McpRateLimiter::class) - ->replaceArgument('$rateLimiterConfig', [ - 'id' => 'mcp', - 'policy' => 'sliding_window', - 'limit' => $mcp['rate_limiter']['limit'], - 'interval' => $mcp['rate_limiter']['interval'], - ]); + ->replaceArgument('$limit', $mcp['rate_limiter']['limit']) + ->replaceArgument('$interval', $mcp['rate_limiter']['interval']); $container ->getDefinition(McpController::class) diff --git a/src/Mcp/McpRateLimiter.php b/src/Mcp/McpRateLimiter.php index 8288f28..4e46d20 100644 --- a/src/Mcp/McpRateLimiter.php +++ b/src/Mcp/McpRateLimiter.php @@ -19,13 +19,12 @@ public const string TOKEN_ATTRIBUTE_KEY = 'mcp_rate_limit_key'; public const string TOKEN_ATTRIBUTE_LIMIT = 'mcp_rate_limit'; - private const string CONFIG_LIMIT = 'limit'; + private const string LIMITER_ID = 'mcp'; + private const string LIMITER_POLICY = 'sliding_window'; - /** - * @param array $rateLimiterConfig - */ public function __construct( - private array $rateLimiterConfig, + private int $limit, + private string $interval, private StorageInterface $storage, private CurrentAnzuUserProvider $currentUserProvider, private Security $security, @@ -74,12 +73,16 @@ private function resolveTokenAttribute(string $name): mixed private function createLimiter(string $key, mixed $limitOverride): LimiterInterface { - $config = $this->rateLimiterConfig; - if (is_int($limitOverride) && $limitOverride > 0) { - $config[self::CONFIG_LIMIT] = $limitOverride; - } + $limit = is_int($limitOverride) && $limitOverride > 0 ? $limitOverride : $this->limit; - return new RateLimiterFactory($config, $this->storage) - ->create($key); + return new RateLimiterFactory( + [ + 'id' => self::LIMITER_ID, + 'policy' => self::LIMITER_POLICY, + 'limit' => $limit, + 'interval' => $this->interval, + ], + $this->storage, + )->create($key); } } diff --git a/src/Resources/config/mcp.php b/src/Resources/config/mcp.php index 268ea80..c1efad9 100644 --- a/src/Resources/config/mcp.php +++ b/src/Resources/config/mcp.php @@ -81,7 +81,8 @@ ; $services->set(McpRateLimiter::class) - ->arg('$rateLimiterConfig', null) + ->arg('$limit', null) + ->arg('$interval', null) ->arg('$storage', service('anzu_systems_common.mcp.rate_limiter_storage')) ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) ->arg('$security', service('security.helper')) diff --git a/tests/Mcp/McpRateLimiterTest.php b/tests/Mcp/McpRateLimiterTest.php index 28faff2..bbe7519 100644 --- a/tests/Mcp/McpRateLimiterTest.php +++ b/tests/Mcp/McpRateLimiterTest.php @@ -27,12 +27,7 @@ final class McpRateLimiterTest extends TestCase private const string TOKEN_KEY = 'pat_7'; private const string FIREWALL_NAME = 'mcp'; private const string USER_IDENTIFIER = 'mcp-user'; - private const array RATE_LIMITER_CONFIG = [ - 'id' => 'mcp_test', - 'policy' => 'sliding_window', - 'limit' => self::LIMIT, - 'interval' => '1 minute', - ]; + private const string INTERVAL = '1 minute'; public function testAnonymousUserIsRejected(): void { @@ -98,7 +93,8 @@ private function createRateLimiter( ->willReturn($token); return new McpRateLimiter( - self::RATE_LIMITER_CONFIG, + self::LIMIT, + self::INTERVAL, $storage ?? new InMemoryStorage(), $currentUserProvider, $security, From 8e649e5a4a4de69ba4de06f5fb1e6457eb75810b Mon Sep 17 00:00:00 2001 From: gabrielzigo Date: Thu, 20 Aug 2026 16:30:08 +0200 Subject: [PATCH 8/8] Map MCP tools to host permissions through McpToolAccessChecker instead of a dedicated permission set --- CHANGELOG.md | 4 +- .../AnzuSystemsCommonExtension.php | 18 +-- src/DependencyInjection/Configuration.php | 4 + .../Handler/FilterToolsListRequestHandler.php | 7 +- src/Mcp/McpToolExecutor.php | 7 +- src/Mcp/Security/McpToolAccessChecker.php | 29 +++++ src/Mcp/Security/McpToolPermission.php | 34 ------ src/Mcp/Security/McpToolPermissionConfig.php | 52 --------- .../Security/Voter/McpToolPermissionVoter.php | 24 ---- src/Resources/config/mcp.php | 16 +-- src/Resources/doc/mcp.md | 40 +++---- ...stemsCommonExtensionMcpPermissionsTest.php | 103 ------------------ .../FilterToolsListRequestHandlerTest.php | 12 +- tests/Mcp/McpToolExecutorTest.php | 9 +- .../Mcp/Security/McpToolAccessCheckerTest.php | 36 ++++++ .../Security/McpToolPermissionVoterTest.php | 59 ---------- 16 files changed, 119 insertions(+), 335 deletions(-) create mode 100644 src/Mcp/Security/McpToolAccessChecker.php delete mode 100644 src/Mcp/Security/McpToolPermission.php delete mode 100644 src/Mcp/Security/McpToolPermissionConfig.php delete mode 100644 src/Mcp/Security/Voter/McpToolPermissionVoter.php delete mode 100644 tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php create mode 100644 tests/Mcp/Security/McpToolAccessCheckerTest.php delete mode 100644 tests/Mcp/Security/McpToolPermissionVoterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 02cb726..0c100b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,12 @@ ## [Unreleased] ### Features -* Per-tool MCP permissions: every tool requires `mcp_tool_` (`McpToolPermission::forTool()`), voted by `McpToolPermissionVoter` through the standard permission model (no implicit access, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `Security`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. `McpToolPermission::SEARCH_APP_LOGS|SEARCH_AUDIT_LOGS|GET_LOGS_BY_CONTEXT` name the bundle tool permissions. The bundle prepends the `mcp_tool` permission subject with its log tools into the `permissions` config; projects add their own tool actions (see "Tool permissions" in `src/Resources/doc/mcp.md`). +* Per-tool MCP permissions: `mcp.tool_permissions` maps every tool name to an existing host permission, resolved by `McpToolAccessChecker` through the host's standard permission model (unmapped tool = denied, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `McpToolAccessChecker`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. * Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()` and `FIELD_ID` / `SORT_DESC` constants. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. * `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the configured `limit` + `interval` and the storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). ### Changes -* BC change: `AbstractLogRepository::FIELD_ID` (protected) moved to `MongoHelper::FIELD_ID`; `McpToolExecutor` requires `Security` (its tool-permission check). +* BC change: `AbstractLogRepository::FIELD_ID` (protected) moved to `MongoHelper::FIELD_ID`; `McpToolExecutor` requires `McpToolAccessChecker` (its tool-permission check). ## [12.0.0](https://github.com/anzusystems/common-bundle/compare/11.3.0...12.0.0) (2026-07-22) diff --git a/src/DependencyInjection/AnzuSystemsCommonExtension.php b/src/DependencyInjection/AnzuSystemsCommonExtension.php index af2c4cc..636820a 100644 --- a/src/DependencyInjection/AnzuSystemsCommonExtension.php +++ b/src/DependencyInjection/AnzuSystemsCommonExtension.php @@ -73,10 +73,7 @@ use AnzuSystems\CommonBundle\Mcp\Controller\McpController; use AnzuSystems\CommonBundle\Mcp\McpRateLimiter; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; -use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermissionConfig; -use AnzuSystems\CommonBundle\Mcp\Tool\GetLogsByContextTool; -use AnzuSystems\CommonBundle\Mcp\Tool\SearchAppLogsTool; -use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\CommonBundle\Messenger\Message\AuditLogMessage; use AnzuSystems\CommonBundle\Messenger\Message\JournalLogMessage; use AnzuSystems\CommonBundle\Request\ParamConverter\ApiFilterParamConverter; @@ -122,11 +119,6 @@ final class AnzuSystemsCommonExtension extends Extension implements PrependExtensionInterface { private const string MCP_TOOL_SCAN_DIR = 'vendor/anzusystems/common-bundle/src/Mcp/Tool'; - private const array MCP_TOOL_NAMES = [ - SearchAppLogsTool::NAME, - SearchAuditLogsTool::NAME, - GetLogsByContextTool::NAME, - ]; private array $processedConfig; @@ -222,7 +214,6 @@ public function prepend(ContainerBuilder $container): void */ public function load(array $configs, ContainerBuilder $container): void { - $this->processedConfig = $this->processConfiguration(new Configuration(), $configs); $loader = new PhpFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.php'); @@ -582,9 +573,6 @@ private function prependMcp(ContainerBuilder $container): void 'scan_dirs' => [self::MCP_TOOL_SCAN_DIR], ], ]); - $container->prependExtensionConfig($this->getAlias(), [ - 'permissions' => McpToolPermissionConfig::forTools(self::MCP_TOOL_NAMES), - ]); } private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): void @@ -651,6 +639,10 @@ private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): ->getDefinition(McpToolExecutor::class) ->replaceArgument('$toolErrorExceptions', $mcp['tool_error_exceptions']); + $container + ->getDefinition(McpToolAccessChecker::class) + ->replaceArgument('$toolPermissions', $mcp['tool_permissions']); + $container ->getDefinition(CreateMcpLogCollectionCommand::class) ->replaceArgument('$mcpLogCollectionName', $mongo['collection']) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 0f55572..4a28642 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -361,6 +361,10 @@ private function addMcpSection(): NodeDefinition ->end() ->scalarPrototype()->end() ->end() + ->arrayNode('tool_permissions') + ->useAttributeAsKey('tool') + ->scalarPrototype()->end() + ->end() ->arrayNode('rate_limiter') ->addDefaultsIfNotSet() ->children() diff --git a/src/Mcp/Handler/FilterToolsListRequestHandler.php b/src/Mcp/Handler/FilterToolsListRequestHandler.php index 892b964..975d928 100644 --- a/src/Mcp/Handler/FilterToolsListRequestHandler.php +++ b/src/Mcp/Handler/FilterToolsListRequestHandler.php @@ -4,7 +4,7 @@ namespace AnzuSystems\CommonBundle\Mcp\Handler; -use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use Mcp\Capability\RegistryInterface; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\Request; @@ -14,7 +14,6 @@ use Mcp\Schema\Tool; use Mcp\Server\Handler\Request\RequestHandlerInterface; use Mcp\Server\Session\SessionInterface; -use Symfony\Bundle\SecurityBundle\Security; /** * @implements RequestHandlerInterface @@ -23,7 +22,7 @@ { public function __construct( private RegistryInterface $registry, - private Security $security, + private McpToolAccessChecker $toolAccessChecker, private int $pageSize, ) { } @@ -42,7 +41,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er $page = $this->registry->getTools($this->pageSize, $request->cursor); $grantedTools = []; foreach ($page->references as $reference) { - if ($reference instanceof Tool && $this->security->isGranted(McpToolPermission::forTool($reference->name))) { + if ($reference instanceof Tool && $this->toolAccessChecker->isToolGranted($reference->name)) { $grantedTools[] = $reference; } } diff --git a/src/Mcp/McpToolExecutor.php b/src/Mcp/McpToolExecutor.php index 11bf497..283ea51 100644 --- a/src/Mcp/McpToolExecutor.php +++ b/src/Mcp/McpToolExecutor.php @@ -7,11 +7,10 @@ use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; -use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use Closure; use Monolog\Attribute\WithMonologChannel; use Psr\Log\LoggerInterface; -use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Throwable; @@ -29,7 +28,7 @@ public function __construct( private CurrentAnzuUserProvider $currentUserProvider, private LoggerInterface $logger, private McpLogger $mcpLogger, - private Security $security, + private McpToolAccessChecker $toolAccessChecker, private array $toolErrorExceptions = [], ) { } @@ -46,7 +45,7 @@ public function execute(string $toolName, array $params, Closure $callback): arr $error = null; try { - if (false === $this->security->isGranted(McpToolPermission::forTool($toolName))) { + if (false === $this->toolAccessChecker->isToolGranted($toolName)) { $error = sprintf(self::TOOL_ACCESS_DENIED_MESSAGE, $toolName); return [self::ERROR_KEY => $error]; diff --git a/src/Mcp/Security/McpToolAccessChecker.php b/src/Mcp/Security/McpToolAccessChecker.php new file mode 100644 index 0000000..3125efe --- /dev/null +++ b/src/Mcp/Security/McpToolAccessChecker.php @@ -0,0 +1,29 @@ + $toolPermissions + */ + public function __construct( + private array $toolPermissions, + private Security $security, + ) { + } + + public function isToolGranted(string $toolName): bool + { + $permission = $this->toolPermissions[$toolName] ?? null; + if (null === $permission) { + return false; + } + + return $this->security->isGranted($permission); + } +} diff --git a/src/Mcp/Security/McpToolPermission.php b/src/Mcp/Security/McpToolPermission.php deleted file mode 100644 index aae1421..0000000 --- a/src/Mcp/Security/McpToolPermission.php +++ /dev/null @@ -1,34 +0,0 @@ -camel() - ->toString(); - } - - public static function isToolPermission(string $permission): bool - { - return str_starts_with($permission, self::PREFIX); - } -} diff --git a/src/Mcp/Security/McpToolPermissionConfig.php b/src/Mcp/Security/McpToolPermissionConfig.php deleted file mode 100644 index 879c356..0000000 --- a/src/Mcp/Security/McpToolPermissionConfig.php +++ /dev/null @@ -1,52 +0,0 @@ - $toolNames - * - * @return array> - */ - public static function forTools(array $toolNames): array - { - $actions = []; - $actionTranslations = []; - foreach ($toolNames as $toolName) { - $action = McpToolPermission::toAction($toolName); - $actions[$action] = []; - $actionTranslations[$action] = [self::LOCALE_EN => self::toLabel($toolName)]; - } - - return [ - PermissionConfig::PRM_CONFIG => [ - McpToolPermission::SUBJECT => $actions, - ], - PermissionConfig::PRM_TRANSLATION => [ - 'subjects' => [ - McpToolPermission::SUBJECT => [self::LOCALE_EN => self::SUBJECT_TITLE], - ], - 'actions' => $actionTranslations, - ], - ]; - } - - private static function toLabel(string $toolName): string - { - return new UnicodeString($toolName) - ->replace(self::TOOL_NAME_WORD_SEPARATOR, self::LABEL_WORD_SEPARATOR) - ->title() - ->toString(); - } -} diff --git a/src/Mcp/Security/Voter/McpToolPermissionVoter.php b/src/Mcp/Security/Voter/McpToolPermissionVoter.php deleted file mode 100644 index b216adb..0000000 --- a/src/Mcp/Security/Voter/McpToolPermissionVoter.php +++ /dev/null @@ -1,24 +0,0 @@ - - */ -final class McpToolPermissionVoter extends AbstractVoter -{ - protected function supports(string $attribute, mixed $subject): bool - { - return McpToolPermission::isToolPermission($attribute); - } - - protected function getSupportedPermissions(): array - { - return []; - } -} diff --git a/src/Resources/config/mcp.php b/src/Resources/config/mcp.php index c1efad9..54b4795 100644 --- a/src/Resources/config/mcp.php +++ b/src/Resources/config/mcp.php @@ -18,7 +18,7 @@ use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; use AnzuSystems\CommonBundle\Mcp\Resolver\McpContextIdResolver; use AnzuSystems\CommonBundle\Mcp\Resolver\McpDateWindowResolver; -use AnzuSystems\CommonBundle\Mcp\Security\Voter\McpToolPermissionVoter; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\CommonBundle\Mcp\Tool\GetLogsByContextTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAppLogsTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; @@ -43,18 +43,18 @@ ->tag('monolog.logger', ['channel' => 'mcp']) ; + $services->set(McpToolAccessChecker::class) + ->arg('$toolPermissions', null) + ->arg('$security', service('security.helper')) + ; + $services->set(FilterToolsListRequestHandler::class) ->arg('$registry', service('mcp.registry')) - ->arg('$security', service('security.helper')) + ->arg('$toolAccessChecker', service(McpToolAccessChecker::class)) ->arg('$pageSize', param('mcp.pagination_limit')) ->tag('mcp.request_handler') ; - $services->set(McpToolPermissionVoter::class) - ->call('setSecurity', [service('security.helper')]) - ->tag('security.voter') - ; - $services->set(McpLogger::class) ->arg('$mcpLogCollection', service('anzu_mongo_mcp_log_collection')) ; @@ -75,7 +75,7 @@ ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) ->arg('$logger', service('logger')) ->arg('$mcpLogger', service(McpLogger::class)) - ->arg('$security', service('security.helper')) + ->arg('$toolAccessChecker', service(McpToolAccessChecker::class)) ->arg('$toolErrorExceptions', null) ->tag('monolog.logger', ['channel' => 'mcp']) ; diff --git a/src/Resources/doc/mcp.md b/src/Resources/doc/mcp.md index b91f87b..960423f 100644 --- a/src/Resources/doc/mcp.md +++ b/src/Resources/doc/mcp.md @@ -68,8 +68,8 @@ infrastructure after a bundle upgrade. `tool_error_exceptions` into tool error results, logs every call to the monolog `mcp` channel and to the `mcpLogs` capped collection. * `StrictToolArgumentsRequestHandler` — rejects tool calls with unknown arguments. -* `McpToolPermissionVoter` + `FilterToolsListRequestHandler` — per-tool permissions (see below); the call-side check - lives in `McpToolExecutor`. +* `McpToolAccessChecker` + `FilterToolsListRequestHandler` — per-tool permissions mapped to host permissions (see + below); the call-side check lives in `McpToolExecutor`. * `SearchAppLogsTool`, `SearchAuditLogsTool`, `GetLogsByContextTool` — diagnostic tools over the shared log collections, correlated by `contextId`. @@ -91,35 +91,27 @@ keeps the configured default. Tokens without the attributes fall back to the per ## Tool permissions -Every registered tool requires the permission `mcp_tool_` (`McpToolPermission::forTool()`), e.g. -`search_app_logs` → `mcp_tool_searchAppLogs`. The permission is resolved through the standard permission model -(`AnzuUser::getResolvedPermissions()` from the user's permissions + permission groups, `Grant::ALLOW`/`Grant::DENY`, -super admin bypass) by `McpToolPermissionVoter`; a permission that is not granted denies the tool — there is no +Every tool is mapped to an existing permission of the host application in `mcp.tool_permissions` (tool name → +permission name); `McpToolAccessChecker` resolves it through the host's standard permission model +(`Security::isGranted()` → the host voters, super admin bypass). A tool that is not mapped is denied — there is no implicit access. -* `tools/call` of a tool without the grant returns a tool error result (`{"error": "Access denied — …"}`) from +```yaml +anzu_systems_common: + mcp: + tool_permissions: + search_app_logs: cms_log_read + search_audit_logs: cms_log_read + get_logs_by_context: cms_log_read + list_sites: cms_site_read +``` + +* `tools/call` of a tool the current user may not use returns a tool error result (`{"error": "Access denied — …"}`) from `McpToolExecutor::execute()` before the tool callback runs, and the denied call is written to the `mcpLogs` collection with the error (so every tool must route through the executor, as the bundle tools do). * `tools/list` returns only the tools the current user may call — the filter runs after registry pagination, so a page may come back with fewer tools (even none) while `nextCursor` is still set; clients follow the cursor as usual. -The bundle prepends the `mcp_tool` subject with its own tool actions (`searchAppLogs`, `searchAuditLogs`, -`getLogsByContext`) and their translations into the `permissions` config, so they show up in the admin permission -editor. Projects add their own tools to the same subject (`McpToolPermission::toAction()` gives the action name): - -```yaml -anzu_systems_common: - permissions: - config: - mcp_tool: - listPublishedArticles: - getArticles: - translation: - actions: - listPublishedArticles: { en: List published articles, sk: Zoznam publikovaných článkov } - getArticles: { en: Get articles, sk: Detail článkov } -``` - ## Log query bounds The log repositories behind the diagnostic tools (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, diff --git a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php b/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php deleted file mode 100644 index d7547aa..0000000 --- a/tests/DependencyInjection/AnzuSystemsCommonExtensionMcpPermissionsTest.php +++ /dev/null @@ -1,103 +0,0 @@ - 'mongodb://localhost', - 'username' => 'user', - 'password' => 'password', - 'database' => 'logs', - ]; - - public function testBundleToolPermissionsAreMergedWithProjectPermissions(): void - { - $container = new ContainerBuilder(); - $container->setParameter('kernel.environment', self::KERNEL_ENVIRONMENT); - $container->registerExtension($this->createMcpExtensionStub()); - $extension = new AnzuSystemsCommonExtension(); - $container->registerExtension($extension); - $container->loadFromExtension(self::EXTENSION_ALIAS, $this->createConfig()); - - $extension->prepend($container); - $extension->load($container->getExtensionConfig(self::EXTENSION_ALIAS), $container); - - $permissions = $container->getDefinition(PermissionConfig::class) - ->getArgument('$config'); - self::assertIsArray($permissions); - $actions = array_keys($permissions[PermissionConfig::PRM_CONFIG][McpToolPermission::SUBJECT]); - self::assertContains(McpToolPermission::toAction(SearchAppLogsTool::NAME), $actions); - self::assertContains(McpToolPermission::toAction(SearchAuditLogsTool::NAME), $actions); - self::assertContains(McpToolPermission::toAction(GetLogsByContextTool::NAME), $actions); - self::assertContains(self::PROJECT_ACTION, $actions); - self::assertArrayHasKey( - McpToolPermission::SUBJECT, - $permissions[PermissionConfig::PRM_TRANSLATION]['subjects'], - ); - } - - /** - * @return array - */ - private function createConfig(): array - { - return [ - 'settings' => [ - 'app_redis' => 'redis://localhost', - ], - 'logs' => [ - 'messenger_transport' => [ - 'name' => 'logs', - 'dsn' => 'in-memory://', - ], - 'journal' => [ - 'mongo' => self::MONGO, - ], - 'audit' => [ - 'mongo' => self::MONGO, - ], - ], - 'mcp' => [ - 'enabled' => true, - 'allowed_hosts' => ['localhost'], - ], - 'permissions' => [ - PermissionConfig::PRM_CONFIG => [ - McpToolPermission::SUBJECT => [ - self::PROJECT_ACTION => [], - ], - ], - ], - ]; - } - - private function createMcpExtensionStub(): Extension - { - return new class() extends Extension { - public function load(array $configs, ContainerBuilder $container): void - { - } - - public function getAlias(): string - { - return 'mcp'; - } - }; - } -} diff --git a/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php index eb6a374..c4102ac 100644 --- a/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php +++ b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php @@ -5,7 +5,7 @@ namespace AnzuSystems\CommonBundle\Tests\Mcp\Handler; use AnzuSystems\CommonBundle\Mcp\Handler\FilterToolsListRequestHandler; -use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use Mcp\Capability\RegistryInterface; use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Page; @@ -21,6 +21,8 @@ final class FilterToolsListRequestHandlerTest extends TestCase { private const string GRANTED_TOOL_NAME = 'list_sites'; private const string DENIED_TOOL_NAME = 'search_app_logs'; + private const string GRANTED_PERMISSION = 'cms_site_read'; + private const string DENIED_PERMISSION = 'cms_log_read'; private const int PAGE_SIZE = 20; private const int REQUEST_ID = 1; @@ -56,8 +58,12 @@ private function createHandler(): FilterToolsListRequestHandler $security = $this->createMock(Security::class); $security->method('isGranted') - ->willReturnCallback(static fn (mixed $attribute): bool => McpToolPermission::forTool(self::GRANTED_TOOL_NAME) === $attribute); + ->willReturnCallback(static fn (mixed $attribute): bool => self::GRANTED_PERMISSION === $attribute); + $toolAccessChecker = new McpToolAccessChecker( + [self::GRANTED_TOOL_NAME => self::GRANTED_PERMISSION, self::DENIED_TOOL_NAME => self::DENIED_PERMISSION], + $security, + ); - return new FilterToolsListRequestHandler($registry, $security, self::PAGE_SIZE); + return new FilterToolsListRequestHandler($registry, $toolAccessChecker, self::PAGE_SIZE); } } diff --git a/tests/Mcp/McpToolExecutorTest.php b/tests/Mcp/McpToolExecutorTest.php index b9a59fb..223e5a7 100644 --- a/tests/Mcp/McpToolExecutorTest.php +++ b/tests/Mcp/McpToolExecutorTest.php @@ -8,7 +8,7 @@ use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; -use AnzuSystems\CommonBundle\Mcp\Security\McpToolPermission; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\Contracts\Entity\AnzuUser; use MongoDB\Collection; use MongoDB\InsertOneResult; @@ -22,6 +22,7 @@ final class McpToolExecutorTest extends TestCase { private const int USER_ID = 42; private const string TOOL_NAME = 'test_tool'; + private const string TOOL_PERMISSION = 'cms_test_read'; private const string BACKEND_ERROR_MESSAGE = 'Backend is temporarily unavailable, retry the call.'; private array $insertedDocuments = []; @@ -130,15 +131,13 @@ private function createExecutor(array $toolErrorExceptions = [], bool $toolGrant $security = $this->createMock(Security::class); $security->method('isGranted') - ->willReturnCallback( - static fn (mixed $attribute): bool => $toolGranted && McpToolPermission::forTool(self::TOOL_NAME) === $attribute - ); + ->willReturn($toolGranted); return new McpToolExecutor( $currentUserProvider, new NullLogger(), new McpLogger($collection), - $security, + new McpToolAccessChecker([self::TOOL_NAME => self::TOOL_PERMISSION], $security), $toolErrorExceptions, ); } diff --git a/tests/Mcp/Security/McpToolAccessCheckerTest.php b/tests/Mcp/Security/McpToolAccessCheckerTest.php new file mode 100644 index 0000000..83ffb24 --- /dev/null +++ b/tests/Mcp/Security/McpToolAccessCheckerTest.php @@ -0,0 +1,36 @@ +createMock(Security::class); + $security->method('isGranted') + ->willReturnCallback(static fn (mixed $attribute): bool => self::GRANTED_PERMISSION === $attribute); + $checker = new McpToolAccessChecker( + [ + self::GRANTED_TOOL => self::GRANTED_PERMISSION, + self::DENIED_TOOL => self::DENIED_PERMISSION, + ], + $security, + ); + + self::assertTrue($checker->isToolGranted(self::GRANTED_TOOL)); + self::assertFalse($checker->isToolGranted(self::DENIED_TOOL)); + self::assertFalse($checker->isToolGranted(self::UNMAPPED_TOOL)); + } +} diff --git a/tests/Mcp/Security/McpToolPermissionVoterTest.php b/tests/Mcp/Security/McpToolPermissionVoterTest.php deleted file mode 100644 index b05af04..0000000 --- a/tests/Mcp/Security/McpToolPermissionVoterTest.php +++ /dev/null @@ -1,59 +0,0 @@ - $resolvedPermissions - * - * @dataProvider voteProvider - */ - public function testVote(array $resolvedPermissions, string $attribute, int $expectedVote): void - { - $user = $this->createConfiguredMock(AnzuUser::class, ['getResolvedPermissions' => $resolvedPermissions]); - $token = $this->createConfiguredMock(TokenInterface::class, ['getUser' => $user]); - $voter = new McpToolPermissionVoter(); - $voter->setSecurity($this->createMock(Security::class)); - - self::assertSame($expectedVote, $voter->vote($token, null, [$attribute])); - } - - /** - * @return iterable, 1: string, 2: int}> - */ - public static function voteProvider(): iterable - { - $permission = McpToolPermission::forTool(self::TOOL_NAME); - - yield 'allow grant' => [[$permission => Grant::ALLOW], $permission, VoterInterface::ACCESS_GRANTED]; - yield 'missing permission denies by default' => [[], $permission, VoterInterface::ACCESS_DENIED]; - yield 'foreign permission abstains' => [[], 'cms_article_read', VoterInterface::ACCESS_ABSTAIN]; - } -}