Skip to content

Commit dcc6011

Browse files
tianzhouclaude
andauthored
fix: scope search_objects to configured database on MySQL/MariaDB (#332)
* Fix: enable query plans on SQL Server via EXPLAIN SQL Server has no native EXPLAIN statement, and the bogus `explain`/ `showplan` entries in the read-only allow-list did nothing. The actual mechanism, SET SHOWPLAN_XML ON, is session scoped, must be the only statement in its batch, and is suppressed inside a transaction — so it could not be used through the pooled, stateless connector. Translate a leading `EXPLAIN <query>` into a SHOWPLAN_XML request run on a short-lived single-connection pool, giving SQL Server a Postgres/MySQL -like EXPLAIN. SHOWPLAN_XML compiles without executing, so it is read-only safe; the isolated session keeps SHOWPLAN state off the shared pool. Drop the dead `showplan` keyword. Closes #310 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Harden SQL Server EXPLAIN translation (PR review) Address Copilot review feedback on the EXPLAIN/SHOWPLAN flow: - Skip leading whitespace and SQL comments when detecting EXPLAIN, so a comment-prefixed EXPLAIN that passes the (comment-stripping) read-only validator is also translated instead of reaching the server untranslated. - Trim the extracted inner query. - Reject empty EXPLAIN and any SET SHOWPLAN inside the explained statement. SQL Server already blocks SET SHOWPLAN alongside other statements in a batch (verified: the DELETE does not execute), but this keeps the read-only guarantee self-contained rather than relying on server behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Treat comment-only EXPLAIN as empty (PR review) Validate the explained statement against comment/string-stripped SQL so `EXPLAIN /* comment */` raises the "requires a statement" error instead of running an empty batch. Reuses the strip already done for the SET SHOWPLAN guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: scope search_objects to configured database on MySQL/MariaDB search_objects without an explicit schema fanned out over connector.getSchemas(), which on MySQL/MariaDB returns every database on the server (INFORMATION_SCHEMA.SCHEMATA). This leaked tables, views, columns, procedures, and indexes from databases the user never configured, and made object_type="schema" list the whole instance. Introduce an optional Connector.getDefaultSchema() returning the schema a search should default to (the DSN database via DATABASE() on MySQL/MariaDB, or null when none is configured). The tool layer now resolves the search scope as: explicit schema -> connector default -> full getSchemas() list. Validation of an explicit schema still uses the full list, so deliberate cross-database access is preserved. Also exclude system databases (information_schema, performance_schema, mysql, sys) from getSchemas() on MySQL/MariaDB, matching the PostgreSQL connector which already hides pg_catalog et al. Connectors whose getSchemas() is already scoped to the connected database (PostgreSQL, SQL Server, SQLite) need no change. Fixes #323 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a3ab94d commit dcc6011

7 files changed

Lines changed: 186 additions & 11 deletions

File tree

src/connectors/__tests__/mariadb.integration.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class MariaDBTestContainer implements TestContainer {
1919
class MariaDBIntegrationTest extends IntegrationTestBase<MariaDBTestContainer> {
2020
constructor() {
2121
const config: DatabaseTestConfig = {
22-
expectedSchemas: ['testdb', 'information_schema'],
22+
expectedSchemas: ['testdb'],
2323
expectedTables: ['users', 'orders', 'products'],
2424
supportsStoredProcedures: false, // Disabled due to container privilege restrictions
2525
supportsComments: true,
@@ -182,6 +182,19 @@ describe('MariaDB Connector Integration Tests', () => {
182182
mariadbTest.createSSLTests();
183183

184184
describe('MariaDB-specific Features', () => {
185+
it('should exclude server-level system databases from getSchemas()', async () => {
186+
const schemas = await mariadbTest.connector.getSchemas();
187+
expect(schemas).toContain('testdb');
188+
expect(schemas).not.toContain('information_schema');
189+
expect(schemas).not.toContain('performance_schema');
190+
expect(schemas).not.toContain('mysql');
191+
expect(schemas).not.toContain('sys');
192+
});
193+
194+
it('should report the DSN-configured database as the default schema', async () => {
195+
expect(await mariadbTest.connector.getDefaultSchema!()).toBe('testdb');
196+
});
197+
185198
it('should execute multiple statements with native support', async () => {
186199
// First insert the test data
187200
await mariadbTest.connector.executeSQL(`

src/connectors/__tests__/mysql.integration.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class MySQLTestContainer implements TestContainer {
1919
class MySQLIntegrationTest extends IntegrationTestBase<MySQLTestContainer> {
2020
constructor() {
2121
const config: DatabaseTestConfig = {
22-
expectedSchemas: ['testdb', 'information_schema'],
22+
expectedSchemas: ['testdb'],
2323
expectedTables: ['users', 'orders', 'products'],
2424
supportsStoredProcedures: false, // Disabled due to container privilege restrictions
2525
supportsComments: true,
@@ -176,6 +176,19 @@ describe('MySQL Connector Integration Tests', () => {
176176
mysqlTest.createSSLTests();
177177

178178
describe('MySQL-specific Features', () => {
179+
it('should exclude server-level system databases from getSchemas()', async () => {
180+
const schemas = await mysqlTest.connector.getSchemas();
181+
expect(schemas).toContain('testdb');
182+
expect(schemas).not.toContain('information_schema');
183+
expect(schemas).not.toContain('performance_schema');
184+
expect(schemas).not.toContain('mysql');
185+
expect(schemas).not.toContain('sys');
186+
});
187+
188+
it('should report the DSN-configured database as the default schema', async () => {
189+
expect(await mysqlTest.connector.getDefaultSchema!()).toBe('testdb');
190+
});
191+
179192
it('should execute multiple statements with native support', async () => {
180193
// First insert the test data
181194
await mysqlTest.connector.executeSQL(`

src/connectors/interface.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,19 @@ export interface Connector {
156156
*/
157157
getSchemas(): Promise<string[]>;
158158

159+
/**
160+
* Get the schema that searches should default to when no schema is specified.
161+
*
162+
* Returns a single schema name when the connection is scoped to one (e.g. the
163+
* database named in a MySQL/MariaDB DSN), or null when there is no configured
164+
* scope and callers should fall back to getSchemas() (the full list).
165+
*
166+
* Connectors whose getSchemas() is already scoped to the connected database
167+
* (PostgreSQL, SQL Server, SQLite) may omit this or return null.
168+
* @returns Promise with the default schema name, or null for the full list
169+
*/
170+
getDefaultSchema?(): Promise<string | null>;
171+
159172
/**
160173
* Get all tables in the database or in a specific schema
161174
* @param schema Optional schema name. If not provided, implementation should use the default schema:

src/connectors/mariadb/index.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,13 @@ export class MariaDBConnector implements Connector {
156156
}
157157

158158
try {
159-
// In MariaDB, schemas are equivalent to databases
159+
// In MariaDB, schemas are equivalent to databases. Exclude server-level
160+
// system databases so the list matches the user-facing schemas only
161+
// (parity with the PostgreSQL connector, which hides pg_catalog et al.).
160162
const rows = await this.pool.query(`
161-
SELECT SCHEMA_NAME
163+
SELECT SCHEMA_NAME
162164
FROM INFORMATION_SCHEMA.SCHEMATA
165+
WHERE SCHEMA_NAME NOT IN ('information_schema', 'performance_schema', 'mysql', 'sys')
163166
ORDER BY SCHEMA_NAME
164167
`) as any[];
165168

@@ -574,6 +577,19 @@ export class MariaDBConnector implements Connector {
574577
return rows[0].DB;
575578
}
576579

580+
/**
581+
* Default search scope = the database named in the DSN. DATABASE() returns
582+
* null when the connection was opened without a database, in which case
583+
* callers fall back to the full server-wide schema list.
584+
*/
585+
async getDefaultSchema(): Promise<string | null> {
586+
if (!this.pool) {
587+
throw new Error("Not connected to database");
588+
}
589+
const rows = await this.pool.query("SELECT DATABASE() AS DB") as any[];
590+
return rows[0]?.DB ?? null;
591+
}
592+
577593
async executeSQL(sql: string, options: ExecuteOptions, parameters?: any[]): Promise<SQLResult> {
578594
if (!this.pool) {
579595
throw new Error("Not connected to database");

src/connectors/mysql/index.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,13 @@ export class MySQLConnector implements Connector {
169169
}
170170

171171
try {
172-
// In MySQL, schemas are equivalent to databases
172+
// In MySQL, schemas are equivalent to databases. Exclude server-level
173+
// system databases so the list matches the user-facing schemas only
174+
// (parity with the PostgreSQL connector, which hides pg_catalog et al.).
173175
const [rows] = (await this.pool.query(`
174-
SELECT SCHEMA_NAME
176+
SELECT SCHEMA_NAME
175177
FROM INFORMATION_SCHEMA.SCHEMATA
178+
WHERE SCHEMA_NAME NOT IN ('information_schema', 'performance_schema', 'mysql', 'sys')
176179
ORDER BY SCHEMA_NAME
177180
`)) as [any[], any];
178181

@@ -587,6 +590,19 @@ export class MySQLConnector implements Connector {
587590
return rows[0].DB;
588591
}
589592

593+
/**
594+
* Default search scope = the database named in the DSN. DATABASE() returns
595+
* null when the connection was opened without a database, in which case
596+
* callers fall back to the full server-wide schema list.
597+
*/
598+
async getDefaultSchema(): Promise<string | null> {
599+
if (!this.pool) {
600+
throw new Error("Not connected to database");
601+
}
602+
const [rows] = (await this.pool.query("SELECT DATABASE() AS DB")) as [any[], any];
603+
return rows[0]?.DB ?? null;
604+
}
605+
590606
async executeSQL(sql: string, options: ExecuteOptions, parameters?: any[]): Promise<SQLResult> {
591607
if (!this.pool) {
592608
throw new Error("Not connected to database");

src/tools/__tests__/search-objects.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,4 +1305,88 @@ describe('search_database_objects tool', () => {
13051305
expect(asteriskParsed.data.results.map((r: any) => r.name)).toEqual(['user*data']);
13061306
});
13071307
});
1308+
1309+
describe('default schema scoping', () => {
1310+
// Simulates a MySQL/MariaDB connector whose getSchemas() lists every
1311+
// database on the server, but getDefaultSchema() reports the DSN-configured
1312+
// database. Searches without an explicit schema must stay within the default.
1313+
beforeEach(() => {
1314+
mockConnector = createMockConnector('mysql');
1315+
(mockConnector as any).getDefaultSchema = vi.fn();
1316+
mockGetCurrentConnector.mockReturnValue(mockConnector);
1317+
vi.mocked(mockConnector.getSchemas).mockResolvedValue([
1318+
'configured_db',
1319+
'other_db',
1320+
'third_db',
1321+
]);
1322+
vi.mocked(mockConnector.getTables).mockImplementation(async (schema?: string) => {
1323+
if (schema === 'configured_db') return ['orders'];
1324+
if (schema === 'other_db') return ['secrets'];
1325+
return ['misc'];
1326+
});
1327+
});
1328+
1329+
it('scopes table search to the default schema and never fans out to other databases', async () => {
1330+
vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db');
1331+
1332+
const handler = createSearchDatabaseObjectsToolHandler();
1333+
const result = await handler(
1334+
{ object_type: 'table', pattern: '%', detail_level: 'names' },
1335+
null
1336+
);
1337+
1338+
const parsed = parseToolResponse(result);
1339+
expect(parsed.data.results.map((r: any) => r.schema)).toEqual(['configured_db']);
1340+
expect(parsed.data.results.map((r: any) => r.name)).toEqual(['orders']);
1341+
// Only the configured database should have been inspected.
1342+
expect(mockConnector.getTables).toHaveBeenCalledTimes(1);
1343+
expect(mockConnector.getTables).toHaveBeenCalledWith('configured_db');
1344+
});
1345+
1346+
it('scopes schema listing to the default schema', async () => {
1347+
vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db');
1348+
1349+
const handler = createSearchDatabaseObjectsToolHandler();
1350+
const result = await handler(
1351+
{ object_type: 'schema', pattern: '%', detail_level: 'names' },
1352+
null
1353+
);
1354+
1355+
const parsed = parseToolResponse(result);
1356+
expect(parsed.data.results.map((r: any) => r.name)).toEqual(['configured_db']);
1357+
});
1358+
1359+
it('falls back to the full schema list when no default is configured (null)', async () => {
1360+
vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue(null);
1361+
1362+
const handler = createSearchDatabaseObjectsToolHandler();
1363+
const result = await handler(
1364+
{ object_type: 'table', pattern: '%', detail_level: 'names' },
1365+
null
1366+
);
1367+
1368+
const parsed = parseToolResponse(result);
1369+
expect(parsed.data.results.map((r: any) => r.schema)).toEqual([
1370+
'configured_db',
1371+
'other_db',
1372+
'third_db',
1373+
]);
1374+
});
1375+
1376+
it('honors an explicit schema filter targeting a non-default database', async () => {
1377+
vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db');
1378+
1379+
const handler = createSearchDatabaseObjectsToolHandler();
1380+
const result = await handler(
1381+
{ object_type: 'table', pattern: '%', schema: 'other_db', detail_level: 'names' },
1382+
null
1383+
);
1384+
1385+
const parsed = parseToolResponse(result);
1386+
expect(parsed.data.results.map((r: any) => r.schema)).toEqual(['other_db']);
1387+
expect(parsed.data.results.map((r: any) => r.name)).toEqual(['secrets']);
1388+
// getDefaultSchema must not override an explicit caller-provided schema.
1389+
expect((mockConnector as any).getDefaultSchema).not.toHaveBeenCalled();
1390+
});
1391+
});
13081392
});

src/tools/search-objects.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,26 @@ function likePatternToRegex(pattern: string): RegExp {
6666
return new RegExp(`^${escaped}$`, "i");
6767
}
6868

69+
/**
70+
* Resolve the set of schemas a search should scope to when the caller did not
71+
* pass an explicit `schema` filter.
72+
*
73+
* Prefers the connector's default schema (e.g. the database named in a
74+
* MySQL/MariaDB DSN) so searches don't leak across every database on the
75+
* server. When the connector reports no default (null) — or doesn't implement
76+
* getDefaultSchema at all — it falls back to the full getSchemas() list, which
77+
* for PostgreSQL/SQL Server/SQLite is already scoped to the connected database.
78+
*/
79+
async function resolveDefaultSchemas(connector: Connector): Promise<string[]> {
80+
if (connector.getDefaultSchema) {
81+
const defaultSchema = await connector.getDefaultSchema();
82+
if (defaultSchema) {
83+
return [defaultSchema];
84+
}
85+
}
86+
return connector.getSchemas();
87+
}
88+
6989
/**
7090
* Get row count estimate for a table.
7191
* Prefers the connector's native statistics-based method (e.g. pg_class.reltuples)
@@ -123,7 +143,7 @@ async function searchSchemas(
123143
detailLevel: DetailLevel,
124144
limit: number
125145
): Promise<any[]> {
126-
const schemas = await connector.getSchemas();
146+
const schemas = await resolveDefaultSchemas(connector);
127147
const regex = likePatternToRegex(pattern);
128148
const matched = schemas.filter((schema: string) => regex.test(schema)).slice(0, limit);
129149

@@ -170,7 +190,7 @@ async function searchTables(
170190
if (schemaFilter) {
171191
schemasToSearch = [schemaFilter];
172192
} else {
173-
schemasToSearch = await connector.getSchemas();
193+
schemasToSearch = await resolveDefaultSchemas(connector);
174194
}
175195

176196
// Search tables in each schema
@@ -374,7 +394,7 @@ async function searchColumns(
374394
if (schemaFilter) {
375395
schemasToSearch = [schemaFilter];
376396
} else {
377-
schemasToSearch = await connector.getSchemas();
397+
schemasToSearch = await resolveDefaultSchemas(connector);
378398
}
379399

380400
// Search columns in tables and views across schemas
@@ -463,7 +483,7 @@ async function searchProcedures(
463483
if (schemaFilter) {
464484
schemasToSearch = [schemaFilter];
465485
} else {
466-
schemasToSearch = await connector.getSchemas();
486+
schemasToSearch = await resolveDefaultSchemas(connector);
467487
}
468488

469489
// Search procedures/functions in each schema
@@ -532,7 +552,7 @@ async function searchIndexes(
532552
if (schemaFilter) {
533553
schemasToSearch = [schemaFilter];
534554
} else {
535-
schemasToSearch = await connector.getSchemas();
555+
schemasToSearch = await resolveDefaultSchemas(connector);
536556
}
537557

538558
// Search indexes in tables across schemas

0 commit comments

Comments
 (0)