From 38f0315fe2443c2c45ca42171b1c0915a664c41e Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sun, 25 Oct 2026 12:30:28 +0300 Subject: [PATCH] fix(mcp): system.* collections stay hidden from find/count, and the schema resource is logged and redacted (#1774, #1775) --- lib/core/mcp/mcp_mongo_service.dart | 15 +++--- lib/core/mcp/querya_mcp_server.dart | 15 +++++- test/core/mcp/mcp_mongo_service_test.dart | 17 +++++++ test/core/mcp/mcp_security_test.dart | 61 ++++++++++++++++++++++- 4 files changed, 99 insertions(+), 9 deletions(-) diff --git a/lib/core/mcp/mcp_mongo_service.dart b/lib/core/mcp/mcp_mongo_service.dart index b8256d8c..f2937729 100644 --- a/lib/core/mcp/mcp_mongo_service.dart +++ b/lib/core/mcp/mcp_mongo_service.dart @@ -185,11 +185,7 @@ class McpMongoService { Future> listCollections(int connectionId) { return _withSession(connectionId, (session) async { - final names = await session.listCollections(); - return [ - for (final n in names) - if (!n.startsWith('system.')) n, - ]..sort(); + return (await _visibleCollections(session)).toList()..sort(); }); } @@ -243,9 +239,16 @@ class McpMongoService { } } + /// The collections the model may see and name; `system.*` ones (profile, + /// views) hold past queries and stay hidden from every tool. + Future> _visibleCollections(McpMongoSession session) async { + final names = await session.listCollections(); + return names.where((n) => !n.startsWith('system.')); + } + /// The collection must exist, so model input never names one unchecked. Future _knownCollection(McpMongoSession session, String name) async { - final names = await session.listCollections(); + final names = (await _visibleCollections(session)).toList(); if (names.contains(name)) return name; final lower = name.toLowerCase(); final loose = names.where((n) => n.toLowerCase() == lower).toList(); diff --git a/lib/core/mcp/querya_mcp_server.dart b/lib/core/mcp/querya_mcp_server.dart index 023f6912..7def6cb3 100644 --- a/lib/core/mcp/querya_mcp_server.dart +++ b/lib/core/mcp/querya_mcp_server.dart @@ -344,7 +344,20 @@ base class QueryaMcpServer extends MCPServer with ToolsSupport, ResourcesSupport if (!request.uri.startsWith(prefix)) return null; final id = int.tryParse(request.uri.substring(prefix.length)); if (id == null) return null; - final tables = await service.schemaOverview(id); + final started = DateTime.now(); + final sw = Stopwatch()..start(); + final List tables; + try { + tables = await service.schemaOverview(id); + } on McpToolException catch (e) { + _report('schema', started, sw.elapsed, id, null, null, e.message, e.rule); + rethrow; + } catch (e) { + final message = McpRedaction.redact('Internal error: $e'); + _report('schema', started, sw.elapsed, id, null, null, message, null); + throw McpToolException(message); + } + _report('schema', started, sw.elapsed, id, null, tables.length, null, null); return ReadResourceResult(contents: [ TextResourceContents( uri: request.uri, diff --git a/test/core/mcp/mcp_mongo_service_test.dart b/test/core/mcp/mcp_mongo_service_test.dart index c9f1e550..3284439a 100644 --- a/test/core/mcp/mcp_mongo_service_test.dart +++ b/test/core/mcp/mcp_mongo_service_test.dart @@ -113,6 +113,23 @@ void main() { opened = 0; }); + test('system.* collections are neither listed nor readable (#1774)', + () async { + final svc = service(); + expect(await svc.listCollections(1), ['orders', 'users']); + await expectLater( + svc.find(1, 'system.views'), + throwsA(isA() + .having((e) => e.message, 'message', contains('Collection not found'))), + ); + await expectLater( + svc.count(1, 'system.views'), + throwsA(isA() + .having((e) => e.message, 'message', contains('Collection not found'))), + ); + expect(session.calls, isEmpty); + }); + group('McpMongoGuard', () { test('plain filters pass', () { expect( diff --git a/test/core/mcp/mcp_security_test.dart b/test/core/mcp/mcp_security_test.dart index 03b60725..66e0242a 100644 --- a/test/core/mcp/mcp_security_test.dart +++ b/test/core/mcp/mcp_security_test.dart @@ -66,9 +66,17 @@ SqlExecutionResult _catalog(String sql) { } /// Client and server over an in-memory channel. -Future<(ServerConnection, QueryaMcpServer)> _connect(McpQueryService service) async { +Future<(ServerConnection, QueryaMcpServer)> _connect( + McpQueryService service, { + void Function(McpCallRecord)? onCall, +}) async { final pair = StreamChannelController(); - final server = QueryaMcpServer(pair.local, service: service, version: 't'); + final server = QueryaMcpServer( + pair.local, + service: service, + version: 't', + onCall: onCall, + ); final client = MCPClient(Implementation(name: 'attacker', version: '1')); final conn = client.connectServer(pair.foreign); await conn.initialize(InitializeRequest( @@ -371,6 +379,55 @@ void main() { }); }); + group('the schema resource (#1775)', () { + test('is logged like a tool call', () async { + final calls = []; + final (conn, server) = await _connect( + McpQueryService( + createDelegate: (row, dialect) => + FakeSqlExecutionDelegate()..onExecute = _catalog, + access: _Access({1}), + loadConnections: () async => [_row(1, 'postgresql')], + ), + onCall: calls.add, + ); + addTearDown(server.shutdown); + + await conn.readResource(ReadResourceRequest(uri: 'schema://1')); + + expect(calls.single.tool, 'schema'); + expect(calls.single.connectionId, 1); + expect(calls.single.error, isNull); + }); + + test('masks secrets in a failing driver error and logs it', () async { + final calls = []; + final (conn, server) = await _connect( + McpQueryService( + createDelegate: (row, dialect) => throw StateError( + 'connect postgresql://admin:$_pw@db.internal/shop failed'), + access: _Access({1}), + loadConnections: () async => [_row(1, 'postgresql')], + ), + onCall: calls.add, + ); + addTearDown(server.shutdown); + + Object? caught; + try { + await conn.readResource(ReadResourceRequest(uri: 'schema://1')); + } catch (e) { + caught = e; + } + + expect(caught, isNotNull); + expect('$caught', isNot(contains(_pw))); + expect(calls.single.tool, 'schema'); + expect(calls.single.error, isNotNull); + expect(calls.single.error, isNot(contains(_pw))); + }); + }); + test('a connection that is not shared is invisible to every tool', () async { var created = 0; final (conn, server) = await _connect(McpQueryService(