From 62426253c3fa1dc80951e78ee030270e9dec5000 Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues <44656907+Rodriguespn@users.noreply.github.com> Date: Tue, 17 Mar 2026 09:48:34 +0000 Subject: [PATCH] fix: pass exposedSchemas to getLints in MCP advisor operations (#43790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - MCP `getSecurityAdvisors` and `getPerformanceAdvisors` now pass `exposedSchemas` to `getLints`, fixing empty advisor results in local/self-hosted environments - Extracts `DEFAULT_EXPOSED_SCHEMAS` constant shared between the MCP handler and the `run-lints` API route (cc @joshenlim related https://github.com/supabase/supabase/pull/40043) - Adds unit tests for `enrichLintsQuery` and the MCP advisor operations ## The bug The MCP advisor tools (`get_advisors`) return empty arrays (`[]`) for **all** scenarios when running locally via `supabase start`. No security or performance advisors are surfaced, even when the database has clear issues (e.g., tables with no RLS). ### Root cause In `lib/api/self-hosted/mcp.ts`, both `getSecurityAdvisors` and `getPerformanceAdvisors` call `getLints({ headers })` **without passing `exposedSchemas`**: ```typescript // Before (mcp.ts:131) const { data, error } = await getLints({ headers }) ``` When `exposedSchemas` is `undefined`, `enrichLintsQuery` in `lints.ts` skips the `SET LOCAL pgrst.db_schemas = '...'` SQL statement: ```typescript // lints.ts:23 ${!!exposedSchemas ? `set local pgrst.db_schemas = '${exposedSchemas}';` : ''} ``` Without this GUC being set, the splinter SQL queries filter results using `current_setting('pgrst.db_schemas', 't')` — which returns an empty string in local environments. Every schema-filtered lint matches no schemas and returns zero rows. ### Why this only affects local/self-hosted environments In **hosted Supabase**, PostgREST sets the `pgrst.db_schemas` GUC on its own database connections based on the project's API configuration. The Studio MCP server in production reads the same project configuration, so the GUC is already available. **Locally**, PostgREST runs in a separate Docker container and only sets this GUC on _its own_ connections. Studio connects directly to PostgreSQL (bypassing PostgREST), so `current_setting('pgrst.db_schemas', 't')` returns `''`. The HTTP API endpoint (`/api/platform/.../run-lints`) already worked because `run-lints.ts` passes `exposedSchemas: 'public, storage'` — this parameter was simply never added to the MCP code path. ## How we verified the fix ### 1. Tests written to fail against the previous code We wrote two test files that target the exact bug: **`tests/unit/lints/enrichLintsQuery.test.ts`** — validates the SQL generation: - Confirms `SET LOCAL pgrst.db_schemas` is included when `exposedSchemas` is provided - Confirms it's omitted when `undefined` or empty (documenting current behavior) **`tests/unit/lints/mcp-advisors.test.ts`** — validates the MCP operations: - Asserts `getSecurityAdvisors` passes `exposedSchemas` to `getLints` - Asserts `getPerformanceAdvisors` passes `exposedSchemas` to `getLints` - Asserts the value matches `DEFAULT_EXPOSED_SCHEMAS` - Verifies SECURITY/PERFORMANCE category filtering still works Before the fix, the two `exposedSchemas` assertions failed: ``` FAIL getSecurityAdvisors should pass exposedSchemas to getLints → expected { Object (headers) } to have property "exposedSchemas" FAIL getPerformanceAdvisors should pass exposedSchemas to getLints → expected { Object (headers) } to have property "exposedSchemas" ``` ### 2. Fix applied, all tests pass After adding `exposedSchemas: DEFAULT_EXPOSED_SCHEMAS` to both MCP operations, all 14 tests pass (9 new + 5 existing MCP tests). ## Test plan run `supabase start`, create a table without RLS, call `get_advisors` via MCP — should return `rls_disabled_in_public` lint --------- Co-authored-by: Claude Opus 4.6 --- apps/studio/lib/api/self-hosted/constants.ts | 5 + apps/studio/lib/api/self-hosted/lints.ts | 8 +- apps/studio/lib/api/self-hosted/mcp.ts | 11 +- .../api/platform/projects/[ref]/run-lints.ts | 12 +-- .../tests/unit/lints/enrichLintsQuery.test.ts | 26 +++++ .../tests/unit/lints/mcp-advisors.test.ts | 102 ++++++++++++++++++ 6 files changed, 151 insertions(+), 13 deletions(-) create mode 100644 apps/studio/tests/unit/lints/enrichLintsQuery.test.ts create mode 100644 apps/studio/tests/unit/lints/mcp-advisors.test.ts diff --git a/apps/studio/lib/api/self-hosted/constants.ts b/apps/studio/lib/api/self-hosted/constants.ts index 0727ec3b0a3..d12f7ab9748 100644 --- a/apps/studio/lib/api/self-hosted/constants.ts +++ b/apps/studio/lib/api/self-hosted/constants.ts @@ -1,5 +1,10 @@ // Constants specific to self-hosted environments +// Schemas exposed via PostgREST Data API, read from the PGRST_DB_SCHEMAS env var +// that is passed to the Studio container via docker-compose / CLI. +export const DEFAULT_EXPOSED_SCHEMAS = + process.env.PGRST_DB_SCHEMAS ?? 'public,storage,graphql_public' + export const ENCRYPTION_KEY = process.env.PG_META_CRYPTO_KEY || 'SAMPLE_KEY' export const POSTGRES_PORT = parseInt(process.env.POSTGRES_PORT || '5432', 10) export const POSTGRES_HOST = process.env.POSTGRES_HOST || 'db' diff --git a/apps/studio/lib/api/self-hosted/lints.ts b/apps/studio/lib/api/self-hosted/lints.ts index 5b62974c09b..b650c3d3f35 100644 --- a/apps/studio/lib/api/self-hosted/lints.ts +++ b/apps/studio/lib/api/self-hosted/lints.ts @@ -19,7 +19,13 @@ export type ResponseData = export const enrichLintsQuery = (query: string, exposedSchemas?: string) => { return ` -set pg_stat_statements.track = none; +do $$ +begin + set pg_stat_statements.track = none; +exception when others then + -- not a superuser or extension not installed, skip silently +end +$$; ${!!exposedSchemas ? `set local pgrst.db_schemas = '${exposedSchemas}';` : ''} -- source: dashboard -- user: ${'self host'} diff --git a/apps/studio/lib/api/self-hosted/mcp.ts b/apps/studio/lib/api/self-hosted/mcp.ts index 41140208e3d..6828abc6fb7 100644 --- a/apps/studio/lib/api/self-hosted/mcp.ts +++ b/apps/studio/lib/api/self-hosted/mcp.ts @@ -9,6 +9,7 @@ import { GetLogsOptions, } from '@supabase/mcp-server-supabase/platform' import { ResponseError } from 'types' +import { DEFAULT_EXPOSED_SCHEMAS } from './constants' import { generateTypescriptTypes } from './generate-types' import { getLints } from './lints' import { getLogQuery, retrieveAnalyticsData } from './logs' @@ -128,7 +129,10 @@ export function getDebuggingOperations({ return data }, async getSecurityAdvisors(_projectRef) { - const { data, error } = await getLints({ headers }) + const { data, error } = await getLints({ + headers, + exposedSchemas: DEFAULT_EXPOSED_SCHEMAS, + }) if (error) { throw error @@ -137,7 +141,10 @@ export function getDebuggingOperations({ return data.filter((lint) => lint.categories.includes('SECURITY')) }, async getPerformanceAdvisors(_projectRef) { - const { data, error } = await getLints({ headers }) + const { data, error } = await getLints({ + headers, + exposedSchemas: DEFAULT_EXPOSED_SCHEMAS, + }) if (error) { throw error diff --git a/apps/studio/pages/api/platform/projects/[ref]/run-lints.ts b/apps/studio/pages/api/platform/projects/[ref]/run-lints.ts index 51fe4a29c31..08f5b6dab45 100644 --- a/apps/studio/pages/api/platform/projects/[ref]/run-lints.ts +++ b/apps/studio/pages/api/platform/projects/[ref]/run-lints.ts @@ -2,6 +2,7 @@ import { NextApiRequest, NextApiResponse } from 'next' import { constructHeaders } from 'lib/api/apiHelpers' import apiWrapper from 'lib/api/apiWrapper' +import { DEFAULT_EXPOSED_SCHEMAS } from 'lib/api/self-hosted/constants' import { getLints } from 'lib/api/self-hosted/lints' export default (req: NextApiRequest, res: NextApiResponse) => apiWrapper(req, res, handler) @@ -11,18 +12,9 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { switch (method) { case 'GET': - /** - * [Joshen] JFYI technically the exposed schemas is being set here via docker-compose.yml - * https://github.com/supabase/supabase/blob/master/docker/docker-compose.yml#L183 - * https://github.com/supabase/supabase/blob/474a78721e510301d15ca9dbd41f05ce10fa29e5/docker/.env.example#L55 - * - * But i noticed that the local API route on config/postgrest.ts has currently hardcoded db_schema to `public, storage` - * As such, this is only just a temporary patch here that we're hardcoding the exposed schemas but we will need to figure - * out how to get the dashboard to retrieve the values from docker-compose - */ const { data, error } = await getLints({ headers: constructHeaders(req.headers), - exposedSchemas: 'public, storage', + exposedSchemas: DEFAULT_EXPOSED_SCHEMAS, }) if (error) { diff --git a/apps/studio/tests/unit/lints/enrichLintsQuery.test.ts b/apps/studio/tests/unit/lints/enrichLintsQuery.test.ts new file mode 100644 index 00000000000..09558c2154a --- /dev/null +++ b/apps/studio/tests/unit/lints/enrichLintsQuery.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from 'vitest' +import { enrichLintsQuery } from 'lib/api/self-hosted/lints' + +describe('enrichLintsQuery', () => { + const dummyQuery = 'SELECT 1' + + it('should include SET LOCAL pgrst.db_schemas when exposedSchemas is provided', () => { + const result = enrichLintsQuery(dummyQuery, 'public, storage') + expect(result).toContain("set local pgrst.db_schemas = 'public, storage';") + }) + + it('should NOT include SET LOCAL pgrst.db_schemas when exposedSchemas is undefined', () => { + const result = enrichLintsQuery(dummyQuery, undefined) + expect(result).not.toContain('pgrst.db_schemas') + }) + + it('should NOT include SET LOCAL pgrst.db_schemas when exposedSchemas is empty string', () => { + const result = enrichLintsQuery(dummyQuery, '') + expect(result).not.toContain('pgrst.db_schemas') + }) + + it('should always include the query', () => { + const result = enrichLintsQuery(dummyQuery) + expect(result).toContain(dummyQuery) + }) +}) diff --git a/apps/studio/tests/unit/lints/mcp-advisors.test.ts b/apps/studio/tests/unit/lints/mcp-advisors.test.ts new file mode 100644 index 00000000000..3e822f7ea3e --- /dev/null +++ b/apps/studio/tests/unit/lints/mcp-advisors.test.ts @@ -0,0 +1,102 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { DEFAULT_EXPOSED_SCHEMAS } from 'lib/api/self-hosted/constants' + +// Mock getLints to capture what arguments the MCP operations pass +const mockGetLints = vi.fn() +vi.mock('lib/api/self-hosted/lints', () => ({ + getLints: (...args: unknown[]) => mockGetLints(...args), +})) + +// Mock getProjectSettings to avoid assertSelfHosted() check +vi.mock('lib/api/self-hosted/settings', () => ({ + getProjectSettings: () => ({ + app_config: { + db_schema: 'public', + endpoint: 'localhost', + protocol: 'http', + }, + service_api_keys: [{ api_key: 'test', name: 'anon key', tags: 'anon' }], + }), +})) + +// Must import after mock setup +import { getDebuggingOperations } from 'lib/api/self-hosted/mcp' + +describe('MCP advisor operations pass exposedSchemas to getLints', () => { + const headers = { Authorization: 'Bearer test' } + + beforeEach(() => { + vi.clearAllMocks() + mockGetLints.mockResolvedValue({ + data: [ + { + name: 'rls_disabled_in_public', + title: 'RLS Disabled in Public', + level: 'ERROR', + categories: ['SECURITY'], + description: 'test', + detail: 'test', + remediation: 'test', + metadata: {}, + cache_key: 'test', + }, + { + name: 'unindexed_foreign_keys', + title: 'Unindexed foreign keys', + level: 'INFO', + categories: ['PERFORMANCE'], + description: 'test', + detail: 'test', + remediation: 'test', + metadata: {}, + cache_key: 'test', + }, + ], + error: undefined, + }) + }) + + it('getSecurityAdvisors should pass exposedSchemas to getLints', async () => { + const ops = getDebuggingOperations({ headers }) + await ops.getSecurityAdvisors('test-project') + + expect(mockGetLints).toHaveBeenCalledOnce() + const callArgs = mockGetLints.mock.calls[0][0] + expect(callArgs).toHaveProperty('exposedSchemas') + expect(callArgs.exposedSchemas).toBeTruthy() + }) + + it('getPerformanceAdvisors should pass exposedSchemas to getLints', async () => { + const ops = getDebuggingOperations({ headers }) + await ops.getPerformanceAdvisors('test-project') + + expect(mockGetLints).toHaveBeenCalledOnce() + const callArgs = mockGetLints.mock.calls[0][0] + expect(callArgs).toHaveProperty('exposedSchemas') + expect(callArgs.exposedSchemas).toBeTruthy() + }) + + it('should use DEFAULT_EXPOSED_SCHEMAS constant', async () => { + const ops = getDebuggingOperations({ headers }) + await ops.getSecurityAdvisors('test-project') + + const callArgs = mockGetLints.mock.calls[0][0] + expect(callArgs.exposedSchemas).toBe(DEFAULT_EXPOSED_SCHEMAS) + }) + + it('getSecurityAdvisors should filter to SECURITY category', async () => { + const ops = getDebuggingOperations({ headers }) + const result = await ops.getSecurityAdvisors('test-project') + + expect(result).toHaveLength(1) + expect((result as Array<{ name: string }>)[0].name).toBe('rls_disabled_in_public') + }) + + it('getPerformanceAdvisors should filter to PERFORMANCE category', async () => { + const ops = getDebuggingOperations({ headers }) + const result = await ops.getPerformanceAdvisors('test-project') + + expect(result).toHaveLength(1) + expect((result as Array<{ name: string }>)[0].name).toBe('unindexed_foreign_keys') + }) +})