From 950b28f3e24b6769147f0f42658df203484c4ab9 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 9 Jun 2026 02:16:01 -0700 Subject: [PATCH 1/4] Fix: enable query plans on SQL Server via EXPLAIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ` 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) --- .../__tests__/sqlserver.integration.test.ts | 34 +++++++++++ src/connectors/sqlserver/index.ts | 56 +++++++++++++++++++ src/utils/__tests__/allowed-keywords.test.ts | 14 +++++ src/utils/allowed-keywords.ts | 4 +- 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/connectors/__tests__/sqlserver.integration.test.ts b/src/connectors/__tests__/sqlserver.integration.test.ts index f2da24be..555c65a3 100644 --- a/src/connectors/__tests__/sqlserver.integration.test.ts +++ b/src/connectors/__tests__/sqlserver.integration.test.ts @@ -264,6 +264,40 @@ describe('SQL Server Connector Integration Tests', () => { }); describe('SQL Server-specific Features', () => { + it('should return an execution plan for EXPLAIN (SHOWPLAN_XML)', async () => { + const result = await sqlServerTest.connector.executeSQL( + 'EXPLAIN SELECT * FROM users WHERE age > 30', + {} + ); + + expect(result.rows).toHaveLength(1); + const planXml = result.rows[0].plan as string; + expect(typeof planXml).toBe('string'); + // SHOWPLAN_XML output is a ShowPlanXML document referencing the query. + expect(planXml).toContain('ShowPlanXML'); + expect(planXml).toContain('users'); + }); + + it('should not execute the statement under EXPLAIN', async () => { + const before = await sqlServerTest.connector.executeSQL( + "SELECT COUNT(*) as count FROM users WHERE email = 'explain-noexec@example.com'", + {} + ); + expect(Number(before.rows[0].count)).toBe(0); + + // SHOWPLAN_XML compiles without executing, so this INSERT must not run. + await sqlServerTest.connector.executeSQL( + "EXPLAIN INSERT INTO users (name, email, age) VALUES ('NoExec', 'explain-noexec@example.com', 99)", + {} + ); + + const after = await sqlServerTest.connector.executeSQL( + "SELECT COUNT(*) as count FROM users WHERE email = 'explain-noexec@example.com'", + {} + ); + expect(Number(after.rows[0].count)).toBe(0); + }); + it('should handle SQL Server IDENTITY columns', async () => { await sqlServerTest.connector.executeSQL(` CREATE TABLE identity_test ( diff --git a/src/connectors/sqlserver/index.ts b/src/connectors/sqlserver/index.ts index b8cdc1e2..50fd76d1 100644 --- a/src/connectors/sqlserver/index.ts +++ b/src/connectors/sqlserver/index.ts @@ -171,6 +171,9 @@ export class SQLServerConnector implements Connector { // Source ID is set by ConnectorManager after cloning private sourceId: string = "default"; + /** Leading `EXPLAIN ` keyword, translated to a SHOWPLAN_XML request. */ + private static readonly EXPLAIN_PREFIX = /^\s*explain\s+/i; + getId(): string { return this.sourceId; } @@ -574,6 +577,15 @@ export class SQLServerConnector implements Connector { throw new Error("Not connected to SQL Server database"); } + // SQL Server has no native EXPLAIN statement. Translate a leading `EXPLAIN` + // into a SHOWPLAN_XML request so callers get a Postgres/MySQL-like + // experience. SHOWPLAN_XML compiles the statement without executing it, so + // this is always read-only safe. + const explainMatch = sqlQuery.match(SQLServerConnector.EXPLAIN_PREFIX); + if (explainMatch) { + return this.explainQuery(sqlQuery.slice(explainMatch[0].length)); + } + try { // Apply maxRows limit to SELECT queries if specified let processedSQL = sqlQuery; @@ -630,6 +642,50 @@ export class SQLServerConnector implements Connector { throw new Error(`Failed to execute query: ${(error as Error).message}`); } } + + /** + * Return the estimated execution plan for a query using SHOWPLAN_XML. + * + * SHOWPLAN_XML compiles the statement and returns its plan without executing + * it, but it has two constraints: `SET SHOWPLAN_XML ON` must be the only + * statement in its batch, and the setting is session scoped. The shared pool + * hands out a fresh connection per request() and an open transaction + * suppresses SHOWPLAN, so neither can carry the setting to a follow-up query. + * + * We therefore run the SET / query pair on a short-lived, single-connection + * pool built from the same config. The dedicated session keeps SHOWPLAN state + * off the shared pool, so a concurrent query can never land on a connection + * with SHOWPLAN enabled (which would return a plan instead of its results). + */ + private async explainQuery(innerQuery: string): Promise { + if (!this.config) { + throw new Error("Not connected to SQL Server database"); + } + + const explainPool = new sql.ConnectionPool({ + ...this.config, + pool: { ...this.config.pool, max: 1, min: 1 }, + }); + + try { + await explainPool.connect(); + // max:1 + sequential awaits guarantee both batches hit the same session. + await explainPool.request().batch("SET SHOWPLAN_XML ON"); + const planResult = await explainPool.request().batch(innerQuery); + + // The plan is returned as the single column of the first row. + const planRow = planResult.recordset?.[0]; + const planXml = planRow ? Object.values(planRow)[0] : null; + return { + rows: planXml != null ? [{ plan: planXml }] : [], + rowCount: planXml != null ? 1 : 0, + }; + } catch (error) { + throw new Error(`Failed to explain query: ${(error as Error).message}`); + } finally { + await explainPool.close(); + } + } } // Create and register the connector diff --git a/src/utils/__tests__/allowed-keywords.test.ts b/src/utils/__tests__/allowed-keywords.test.ts index 4416877c..889951c7 100644 --- a/src/utils/__tests__/allowed-keywords.test.ts +++ b/src/utils/__tests__/allowed-keywords.test.ts @@ -220,6 +220,20 @@ describe("isReadOnlySQL", () => { }); }); + describe("SQL Server keywords", () => { + it("should allow EXPLAIN (translated to SHOWPLAN_XML by the connector)", () => { + expect(isReadOnlySQL("EXPLAIN SELECT * FROM users", "sqlserver")).toBe(true); + }); + + it("should reject SHOWPLAN (not a real T-SQL statement)", () => { + expect(isReadOnlySQL("SHOWPLAN SELECT * FROM users", "sqlserver")).toBe(false); + }); + + it("should reject bare SET SHOWPLAN_XML (session-scoped, handled via EXPLAIN)", () => { + expect(isReadOnlySQL("SET SHOWPLAN_XML ON", "sqlserver")).toBe(false); + }); + }); + describe("edge cases", () => { it("should treat empty SQL after comment stripping as not read-only", () => { expect(isReadOnlySQL("-- just a comment", "postgres")).toBe(false); diff --git a/src/utils/allowed-keywords.ts b/src/utils/allowed-keywords.ts index 82de797f..e86ea578 100644 --- a/src/utils/allowed-keywords.ts +++ b/src/utils/allowed-keywords.ts @@ -11,7 +11,9 @@ export const allowedKeywords: Record = { mysql: ["select", "with", "explain", "show", "describe", "desc"], mariadb: ["select", "with", "explain", "show", "describe", "desc"], sqlite: ["select", "with", "explain", "pragma"], - sqlserver: ["select", "with", "explain", "showplan"], + // SQL Server has no native EXPLAIN statement; the connector translates a + // leading `EXPLAIN` into a SET SHOWPLAN_XML request (see SQLServerConnector). + sqlserver: ["select", "with", "explain"], }; /** From f3341fe804b1b9aaff8e0093e40e8b5b83208d47 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 9 Jun 2026 02:38:33 -0700 Subject: [PATCH 2/4] 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) --- .../__tests__/sqlserver.integration.test.ts | 35 +++++++++++++++++++ src/connectors/sqlserver/index.ts | 32 +++++++++++++---- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/connectors/__tests__/sqlserver.integration.test.ts b/src/connectors/__tests__/sqlserver.integration.test.ts index 555c65a3..878b1117 100644 --- a/src/connectors/__tests__/sqlserver.integration.test.ts +++ b/src/connectors/__tests__/sqlserver.integration.test.ts @@ -298,6 +298,41 @@ describe('SQL Server Connector Integration Tests', () => { expect(Number(after.rows[0].count)).toBe(0); }); + it('should translate EXPLAIN even when preceded by a comment', async () => { + const result = await sqlServerTest.connector.executeSQL( + '/* inspect plan */ EXPLAIN SELECT * FROM users', + {} + ); + expect(result.rows).toHaveLength(1); + expect(result.rows[0].plan as string).toContain('ShowPlanXML'); + }); + + it('should reject SET SHOWPLAN smuggled into an EXPLAIN query', async () => { + const before = await sqlServerTest.connector.executeSQL( + 'SELECT COUNT(*) as count FROM users', + {} + ); + + await expect( + sqlServerTest.connector.executeSQL( + 'EXPLAIN SET SHOWPLAN_XML OFF DELETE FROM users', + {} + ) + ).rejects.toThrow(/SET SHOWPLAN/i); + + const after = await sqlServerTest.connector.executeSQL( + 'SELECT COUNT(*) as count FROM users', + {} + ); + expect(Number(after.rows[0].count)).toBe(Number(before.rows[0].count)); + }); + + it('should reject an empty EXPLAIN', async () => { + await expect( + sqlServerTest.connector.executeSQL('EXPLAIN ', {}) + ).rejects.toThrow(/requires a statement/i); + }); + it('should handle SQL Server IDENTITY columns', async () => { await sqlServerTest.connector.executeSQL(` CREATE TABLE identity_test ( diff --git a/src/connectors/sqlserver/index.ts b/src/connectors/sqlserver/index.ts index 50fd76d1..0f40c059 100644 --- a/src/connectors/sqlserver/index.ts +++ b/src/connectors/sqlserver/index.ts @@ -15,6 +15,7 @@ import { isDriverNotInstalled } from "../../utils/module-loader.js"; import { SafeURL } from "../../utils/safe-url.js"; import { obfuscateDSNPassword } from "../../utils/dsn-obfuscate.js"; import { SQLRowLimiter } from "../../utils/sql-row-limiter.js"; +import { stripCommentsAndStrings } from "../../utils/sql-parser.js"; /** * SQL Server DSN parser @@ -171,8 +172,13 @@ export class SQLServerConnector implements Connector { // Source ID is set by ConnectorManager after cloning private sourceId: string = "default"; - /** Leading `EXPLAIN ` keyword, translated to a SHOWPLAN_XML request. */ - private static readonly EXPLAIN_PREFIX = /^\s*explain\s+/i; + /** + * Leading whitespace and SQL comments to skip before looking for a keyword. + * The read-only validator strips comments before checking the first keyword, + * so the connector must skip them too; otherwise an EXPLAIN preceded by a + * comment passes validation but reaches the server untranslated. + */ + private static readonly LEADING_NOISE = /^(?:\s+|--[^\n]*(?:\n|$)|\/\*[\s\S]*?\*\/)*/; getId(): string { return this.sourceId; @@ -580,10 +586,12 @@ export class SQLServerConnector implements Connector { // SQL Server has no native EXPLAIN statement. Translate a leading `EXPLAIN` // into a SHOWPLAN_XML request so callers get a Postgres/MySQL-like // experience. SHOWPLAN_XML compiles the statement without executing it, so - // this is always read-only safe. - const explainMatch = sqlQuery.match(SQLServerConnector.EXPLAIN_PREFIX); - if (explainMatch) { - return this.explainQuery(sqlQuery.slice(explainMatch[0].length)); + // this is read-only safe (further enforced in explainQuery). + const afterNoise = sqlQuery.slice( + sqlQuery.match(SQLServerConnector.LEADING_NOISE)![0].length + ); + if (/^explain\b/i.test(afterNoise)) { + return this.explainQuery(afterNoise.slice("explain".length).trim()); } try { @@ -658,6 +666,18 @@ export class SQLServerConnector implements Connector { * with SHOWPLAN enabled (which would return a plan instead of its results). */ private async explainQuery(innerQuery: string): Promise { + if (!innerQuery) { + throw new Error("EXPLAIN requires a statement to analyze"); + } + + // Defense in depth: the SET SHOWPLAN session toggle is what makes EXPLAIN + // non-executing, so the explained statement must not disable it. SQL Server + // already rejects `SET SHOWPLAN_* OFF` alongside other statements in a + // batch, but enforcing it here keeps the read-only guarantee self-contained. + if (/\bset\s+showplan/i.test(stripCommentsAndStrings(innerQuery, "sqlserver"))) { + throw new Error("EXPLAIN does not support SET SHOWPLAN statements"); + } + if (!this.config) { throw new Error("Not connected to SQL Server database"); } From 0b672eafba60a18817327f39abd74bbb268825d3 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 9 Jun 2026 02:50:24 -0700 Subject: [PATCH 3/4] 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) --- src/connectors/__tests__/sqlserver.integration.test.ts | 6 ++++++ src/connectors/sqlserver/index.ts | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/connectors/__tests__/sqlserver.integration.test.ts b/src/connectors/__tests__/sqlserver.integration.test.ts index 878b1117..6155c8b8 100644 --- a/src/connectors/__tests__/sqlserver.integration.test.ts +++ b/src/connectors/__tests__/sqlserver.integration.test.ts @@ -333,6 +333,12 @@ describe('SQL Server Connector Integration Tests', () => { ).rejects.toThrow(/requires a statement/i); }); + it('should reject a comment-only EXPLAIN', async () => { + await expect( + sqlServerTest.connector.executeSQL('EXPLAIN /* just a comment */', {}) + ).rejects.toThrow(/requires a statement/i); + }); + it('should handle SQL Server IDENTITY columns', async () => { await sqlServerTest.connector.executeSQL(` CREATE TABLE identity_test ( diff --git a/src/connectors/sqlserver/index.ts b/src/connectors/sqlserver/index.ts index 0f40c059..3cede0c0 100644 --- a/src/connectors/sqlserver/index.ts +++ b/src/connectors/sqlserver/index.ts @@ -666,7 +666,10 @@ export class SQLServerConnector implements Connector { * with SHOWPLAN enabled (which would return a plan instead of its results). */ private async explainQuery(innerQuery: string): Promise { - if (!innerQuery) { + // Validate against comment/string-stripped SQL so comment-only input counts + // as empty and a SET SHOWPLAN can't hide behind comments. + const cleaned = stripCommentsAndStrings(innerQuery, "sqlserver").trim(); + if (!cleaned) { throw new Error("EXPLAIN requires a statement to analyze"); } @@ -674,7 +677,7 @@ export class SQLServerConnector implements Connector { // non-executing, so the explained statement must not disable it. SQL Server // already rejects `SET SHOWPLAN_* OFF` alongside other statements in a // batch, but enforcing it here keeps the read-only guarantee self-contained. - if (/\bset\s+showplan/i.test(stripCommentsAndStrings(innerQuery, "sqlserver"))) { + if (/\bset\s+showplan/i.test(cleaned)) { throw new Error("EXPLAIN does not support SET SHOWPLAN statements"); } From 22b48077b4312b8f06232b45bf051ebe2458ba0e Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 9 Jun 2026 03:22:35 -0700 Subject: [PATCH 4/4] 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) --- .../__tests__/mariadb.integration.test.ts | 15 +++- .../__tests__/mysql.integration.test.ts | 15 +++- src/connectors/interface.ts | 13 +++ src/connectors/mariadb/index.ts | 20 ++++- src/connectors/mysql/index.ts | 20 ++++- src/tools/__tests__/search-objects.test.ts | 84 +++++++++++++++++++ src/tools/search-objects.ts | 30 +++++-- 7 files changed, 186 insertions(+), 11 deletions(-) diff --git a/src/connectors/__tests__/mariadb.integration.test.ts b/src/connectors/__tests__/mariadb.integration.test.ts index 84872dda..cdec9df7 100644 --- a/src/connectors/__tests__/mariadb.integration.test.ts +++ b/src/connectors/__tests__/mariadb.integration.test.ts @@ -19,7 +19,7 @@ class MariaDBTestContainer implements TestContainer { class MariaDBIntegrationTest extends IntegrationTestBase { constructor() { const config: DatabaseTestConfig = { - expectedSchemas: ['testdb', 'information_schema'], + expectedSchemas: ['testdb'], expectedTables: ['users', 'orders', 'products'], supportsStoredProcedures: false, // Disabled due to container privilege restrictions supportsComments: true, @@ -182,6 +182,19 @@ describe('MariaDB Connector Integration Tests', () => { mariadbTest.createSSLTests(); describe('MariaDB-specific Features', () => { + it('should exclude server-level system databases from getSchemas()', async () => { + const schemas = await mariadbTest.connector.getSchemas(); + expect(schemas).toContain('testdb'); + expect(schemas).not.toContain('information_schema'); + expect(schemas).not.toContain('performance_schema'); + expect(schemas).not.toContain('mysql'); + expect(schemas).not.toContain('sys'); + }); + + it('should report the DSN-configured database as the default schema', async () => { + expect(await mariadbTest.connector.getDefaultSchema!()).toBe('testdb'); + }); + it('should execute multiple statements with native support', async () => { // First insert the test data await mariadbTest.connector.executeSQL(` diff --git a/src/connectors/__tests__/mysql.integration.test.ts b/src/connectors/__tests__/mysql.integration.test.ts index 933d1b21..8e5d097d 100644 --- a/src/connectors/__tests__/mysql.integration.test.ts +++ b/src/connectors/__tests__/mysql.integration.test.ts @@ -19,7 +19,7 @@ class MySQLTestContainer implements TestContainer { class MySQLIntegrationTest extends IntegrationTestBase { constructor() { const config: DatabaseTestConfig = { - expectedSchemas: ['testdb', 'information_schema'], + expectedSchemas: ['testdb'], expectedTables: ['users', 'orders', 'products'], supportsStoredProcedures: false, // Disabled due to container privilege restrictions supportsComments: true, @@ -176,6 +176,19 @@ describe('MySQL Connector Integration Tests', () => { mysqlTest.createSSLTests(); describe('MySQL-specific Features', () => { + it('should exclude server-level system databases from getSchemas()', async () => { + const schemas = await mysqlTest.connector.getSchemas(); + expect(schemas).toContain('testdb'); + expect(schemas).not.toContain('information_schema'); + expect(schemas).not.toContain('performance_schema'); + expect(schemas).not.toContain('mysql'); + expect(schemas).not.toContain('sys'); + }); + + it('should report the DSN-configured database as the default schema', async () => { + expect(await mysqlTest.connector.getDefaultSchema!()).toBe('testdb'); + }); + it('should execute multiple statements with native support', async () => { // First insert the test data await mysqlTest.connector.executeSQL(` diff --git a/src/connectors/interface.ts b/src/connectors/interface.ts index 2daaefd4..3755794d 100644 --- a/src/connectors/interface.ts +++ b/src/connectors/interface.ts @@ -131,6 +131,19 @@ export interface Connector { */ getSchemas(): Promise; + /** + * Get the schema that searches should default to when no schema is specified. + * + * Returns a single schema name when the connection is scoped to one (e.g. the + * database named in a MySQL/MariaDB DSN), or null when there is no configured + * scope and callers should fall back to getSchemas() (the full list). + * + * Connectors whose getSchemas() is already scoped to the connected database + * (PostgreSQL, SQL Server, SQLite) may omit this or return null. + * @returns Promise with the default schema name, or null for the full list + */ + getDefaultSchema?(): Promise; + /** * Get all tables in the database or in a specific schema * @param schema Optional schema name. If not provided, implementation should use the default schema: diff --git a/src/connectors/mariadb/index.ts b/src/connectors/mariadb/index.ts index d0624ef8..4e4d2731 100644 --- a/src/connectors/mariadb/index.ts +++ b/src/connectors/mariadb/index.ts @@ -152,10 +152,13 @@ export class MariaDBConnector implements Connector { } try { - // In MariaDB, schemas are equivalent to databases + // In MariaDB, schemas are equivalent to databases. Exclude server-level + // system databases so the list matches the user-facing schemas only + // (parity with the PostgreSQL connector, which hides pg_catalog et al.). const rows = await this.pool.query(` - SELECT SCHEMA_NAME + SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA + WHERE SCHEMA_NAME NOT IN ('information_schema', 'performance_schema', 'mysql', 'sys') ORDER BY SCHEMA_NAME `) as any[]; @@ -542,6 +545,19 @@ export class MariaDBConnector implements Connector { return rows[0].DB; } + /** + * Default search scope = the database named in the DSN. DATABASE() returns + * null when the connection was opened without a database, in which case + * callers fall back to the full server-wide schema list. + */ + async getDefaultSchema(): Promise { + if (!this.pool) { + throw new Error("Not connected to database"); + } + const rows = await this.pool.query("SELECT DATABASE() AS DB") as any[]; + return rows[0]?.DB ?? null; + } + async executeSQL(sql: string, options: ExecuteOptions, parameters?: any[]): Promise { if (!this.pool) { throw new Error("Not connected to database"); diff --git a/src/connectors/mysql/index.ts b/src/connectors/mysql/index.ts index 46f7ed0e..4a4063fa 100644 --- a/src/connectors/mysql/index.ts +++ b/src/connectors/mysql/index.ts @@ -160,10 +160,13 @@ export class MySQLConnector implements Connector { } try { - // In MySQL, schemas are equivalent to databases + // In MySQL, schemas are equivalent to databases. Exclude server-level + // system databases so the list matches the user-facing schemas only + // (parity with the PostgreSQL connector, which hides pg_catalog et al.). const [rows] = (await this.pool.query(` - SELECT SCHEMA_NAME + SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA + WHERE SCHEMA_NAME NOT IN ('information_schema', 'performance_schema', 'mysql', 'sys') ORDER BY SCHEMA_NAME `)) as [any[], any]; @@ -550,6 +553,19 @@ export class MySQLConnector implements Connector { return rows[0].DB; } + /** + * Default search scope = the database named in the DSN. DATABASE() returns + * null when the connection was opened without a database, in which case + * callers fall back to the full server-wide schema list. + */ + async getDefaultSchema(): Promise { + if (!this.pool) { + throw new Error("Not connected to database"); + } + const [rows] = (await this.pool.query("SELECT DATABASE() AS DB")) as [any[], any]; + return rows[0]?.DB ?? null; + } + async executeSQL(sql: string, options: ExecuteOptions, parameters?: any[]): Promise { if (!this.pool) { throw new Error("Not connected to database"); diff --git a/src/tools/__tests__/search-objects.test.ts b/src/tools/__tests__/search-objects.test.ts index 85321a3f..21a22ec4 100644 --- a/src/tools/__tests__/search-objects.test.ts +++ b/src/tools/__tests__/search-objects.test.ts @@ -1106,4 +1106,88 @@ describe('search_database_objects tool', () => { expect(asteriskParsed.data.results.map((r: any) => r.name)).toEqual(['user*data']); }); }); + + describe('default schema scoping', () => { + // Simulates a MySQL/MariaDB connector whose getSchemas() lists every + // database on the server, but getDefaultSchema() reports the DSN-configured + // database. Searches without an explicit schema must stay within the default. + beforeEach(() => { + mockConnector = createMockConnector('mysql'); + (mockConnector as any).getDefaultSchema = vi.fn(); + mockGetCurrentConnector.mockReturnValue(mockConnector); + vi.mocked(mockConnector.getSchemas).mockResolvedValue([ + 'configured_db', + 'other_db', + 'third_db', + ]); + vi.mocked(mockConnector.getTables).mockImplementation(async (schema?: string) => { + if (schema === 'configured_db') return ['orders']; + if (schema === 'other_db') return ['secrets']; + return ['misc']; + }); + }); + + it('scopes table search to the default schema and never fans out to other databases', async () => { + vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db'); + + const handler = createSearchDatabaseObjectsToolHandler(); + const result = await handler( + { object_type: 'table', pattern: '%', detail_level: 'names' }, + null + ); + + const parsed = parseToolResponse(result); + expect(parsed.data.results.map((r: any) => r.schema)).toEqual(['configured_db']); + expect(parsed.data.results.map((r: any) => r.name)).toEqual(['orders']); + // Only the configured database should have been inspected. + expect(mockConnector.getTables).toHaveBeenCalledTimes(1); + expect(mockConnector.getTables).toHaveBeenCalledWith('configured_db'); + }); + + it('scopes schema listing to the default schema', async () => { + vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db'); + + const handler = createSearchDatabaseObjectsToolHandler(); + const result = await handler( + { object_type: 'schema', pattern: '%', detail_level: 'names' }, + null + ); + + const parsed = parseToolResponse(result); + expect(parsed.data.results.map((r: any) => r.name)).toEqual(['configured_db']); + }); + + it('falls back to the full schema list when no default is configured (null)', async () => { + vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue(null); + + const handler = createSearchDatabaseObjectsToolHandler(); + const result = await handler( + { object_type: 'table', pattern: '%', detail_level: 'names' }, + null + ); + + const parsed = parseToolResponse(result); + expect(parsed.data.results.map((r: any) => r.schema)).toEqual([ + 'configured_db', + 'other_db', + 'third_db', + ]); + }); + + it('honors an explicit schema filter targeting a non-default database', async () => { + vi.mocked((mockConnector as any).getDefaultSchema).mockResolvedValue('configured_db'); + + const handler = createSearchDatabaseObjectsToolHandler(); + const result = await handler( + { object_type: 'table', pattern: '%', schema: 'other_db', detail_level: 'names' }, + null + ); + + const parsed = parseToolResponse(result); + expect(parsed.data.results.map((r: any) => r.schema)).toEqual(['other_db']); + expect(parsed.data.results.map((r: any) => r.name)).toEqual(['secrets']); + // getDefaultSchema must not override an explicit caller-provided schema. + expect((mockConnector as any).getDefaultSchema).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/tools/search-objects.ts b/src/tools/search-objects.ts index 8077ffef..890f3542 100644 --- a/src/tools/search-objects.ts +++ b/src/tools/search-objects.ts @@ -66,6 +66,26 @@ function likePatternToRegex(pattern: string): RegExp { return new RegExp(`^${escaped}$`, "i"); } +/** + * Resolve the set of schemas a search should scope to when the caller did not + * pass an explicit `schema` filter. + * + * Prefers the connector's default schema (e.g. the database named in a + * MySQL/MariaDB DSN) so searches don't leak across every database on the + * server. When the connector reports no default (null) — or doesn't implement + * getDefaultSchema at all — it falls back to the full getSchemas() list, which + * for PostgreSQL/SQL Server/SQLite is already scoped to the connected database. + */ +async function resolveDefaultSchemas(connector: Connector): Promise { + if (connector.getDefaultSchema) { + const defaultSchema = await connector.getDefaultSchema(); + if (defaultSchema) { + return [defaultSchema]; + } + } + return connector.getSchemas(); +} + /** * Get row count estimate for a table. * Prefers the connector's native statistics-based method (e.g. pg_class.reltuples) @@ -123,7 +143,7 @@ async function searchSchemas( detailLevel: DetailLevel, limit: number ): Promise { - const schemas = await connector.getSchemas(); + const schemas = await resolveDefaultSchemas(connector); const regex = likePatternToRegex(pattern); const matched = schemas.filter((schema: string) => regex.test(schema)).slice(0, limit); @@ -170,7 +190,7 @@ async function searchTables( if (schemaFilter) { schemasToSearch = [schemaFilter]; } else { - schemasToSearch = await connector.getSchemas(); + schemasToSearch = await resolveDefaultSchemas(connector); } // Search tables in each schema @@ -276,7 +296,7 @@ async function searchColumns( if (schemaFilter) { schemasToSearch = [schemaFilter]; } else { - schemasToSearch = await connector.getSchemas(); + schemasToSearch = await resolveDefaultSchemas(connector); } // Search columns in tables across schemas @@ -358,7 +378,7 @@ async function searchProcedures( if (schemaFilter) { schemasToSearch = [schemaFilter]; } else { - schemasToSearch = await connector.getSchemas(); + schemasToSearch = await resolveDefaultSchemas(connector); } // Search procedures/functions in each schema @@ -427,7 +447,7 @@ async function searchIndexes( if (schemaFilter) { schemasToSearch = [schemaFilter]; } else { - schemasToSearch = await connector.getSchemas(); + schemasToSearch = await resolveDefaultSchemas(connector); } // Search indexes in tables across schemas