mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix: pass exposedSchemas to getLints in MCP advisor operations (#43790)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
89895a7286
commit
62426253c3
6 files changed
+151
-13
No files matched your search
@@ -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'
|
||||
|
||||
@@ -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'}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
Reference in new issue
Block a user