Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
"[json]": {
"editor.defaultFormatter": "biomejs.biome"
}
}
}
11 changes: 8 additions & 3 deletions packages/mcp-server-supabase/src/pg-meta/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,19 @@ export function listTablesSql(schemas: string[] = []) {
`;

sql += '\n';
let parameters: any[] = [];

if (schemas.length > 0) {
sql += `where schema in (${schemas.map((s) => `'${s}'`).join(',')})`;
const placeholders = schemas.map((_, i) => `$${i + 1}`).join(', ');
sql += `where schema in (${placeholders})`;
parameters = schemas;
} else {
sql += `where schema not in (${SYSTEM_SCHEMAS.map((s) => `'${s}'`).join(',')})`;
const placeholders = SYSTEM_SCHEMAS.map((_, i) => `$${i + 1}`).join(', ');
sql += `where schema not in (${placeholders})`;
parameters = SYSTEM_SCHEMAS;
}

return sql;
return { query: sql, parameters };
}

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/mcp-server-supabase/src/platform/api-platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ export function createSupabaseApiPlatform(

const database: DatabaseOperations = {
async executeSql<T>(projectId: string, options: ExecuteSqlOptions) {
const { query, read_only } = executeSqlOptionsSchema.parse(options);
const { query, parameters, read_only } =
executeSqlOptionsSchema.parse(options);

const response = await managementApiClient.POST(
'/v1/projects/{ref}/database/query',
Expand All @@ -179,6 +180,7 @@ export function createSupabaseApiPlatform(
},
body: {
query,
parameters,
read_only,
},
}
Expand Down
1 change: 1 addition & 0 deletions packages/mcp-server-supabase/src/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export const deployEdgeFunctionOptionsSchema = z.object({

export const executeSqlOptionsSchema = z.object({
query: z.string(),
parameters: z.array(z.unknown()).optional(),
read_only: z.boolean().optional(),
});

Expand Down
36 changes: 36 additions & 0 deletions packages/mcp-server-supabase/src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,42 @@ describe('tools', () => {
);
});

test('list_tables is not vulnerable to SQL injection via schemas parameter', async () => {
const { callTool } = await setup();

const org = await createOrganization({
name: 'SQLi Org',
plan: 'free',
allowed_release_channels: ['ga'],
});

const project = await createProject({
name: 'SQLi Project',
region: 'us-east-1',
organization_id: org.id,
});
project.status = 'ACTIVE_HEALTHY';

// Attempt SQL injection via schemas parameter using payload from HackerOne report
// This payload attempts to break out of the string and inject a division by zero expression
// Reference: https://linear.app/supabase/issue/AI-139
const maliciousSchema = "public') OR (SELECT 1)=1/0--";

// With proper parameterization, this should NOT throw "division by zero" error
// The literal schema name doesn't exist, so it should return empty array
// WITHOUT parameterization, this would throw: "division by zero" error
const maliciousResult = await callTool({
name: 'list_tables',
arguments: {
project_id: project.id,
schemas: [maliciousSchema],
},
});

// Should return empty array without errors, proving the SQL injection was prevented
expect(maliciousResult).toEqual([]);
});

test('list extensions', async () => {
const { callTool } = await setup();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ export function getDatabaseTools({
}),
inject: { project_id },
execute: async ({ project_id, schemas }) => {
const query = listTablesSql(schemas);
const { query, parameters } = listTablesSql(schemas);
const data = await database.executeSql(project_id, {
query,
parameters,
read_only: true,
});
const tables = data
Expand Down
15 changes: 8 additions & 7 deletions packages/mcp-server-supabase/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,19 @@ export type ValueOf<T> = T[keyof T];

// UnionToIntersection<A | B> = A & B
export type UnionToIntersection<U> = (
U extends unknown ? (arg: U) => 0 : never
U extends unknown
? (arg: U) => 0
: never
) extends (arg: infer I) => 0
? I
: never;

// LastInUnion<A | B> = B
export type LastInUnion<U> =
UnionToIntersection<U extends unknown ? (x: U) => 0 : never> extends (
x: infer L
) => 0
? L
: never;
export type LastInUnion<U> = UnionToIntersection<
U extends unknown ? (x: U) => 0 : never
> extends (x: infer L) => 0
? L
: never;

// UnionToTuple<A, B> = [A, B]
export type UnionToTuple<T, Last = LastInUnion<T>> = [T] extends [never]
Expand Down
63 changes: 41 additions & 22 deletions packages/mcp-server-supabase/test/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ export const mockManagementApi = [
/**
* Execute a SQL query on a project's database
*/
http.post<{ projectId: string }, { query: string; read_only?: boolean }>(
http.post<
{ projectId: string },
{ query: string; parameters?: unknown[]; read_only?: boolean }
>(
`${API_URL}/v1/projects/:projectId/database/query`,
async ({ params, request }) => {
const project = mockProjects.get(params.projectId);
Expand All @@ -269,30 +272,46 @@ export const mockManagementApi = [
);
}
const { db } = project;
const { query, read_only } = await request.json();
const { query, parameters, read_only } = await request.json();

// Not secure, but good enough for testing
const wrappedQuery = `
SET ROLE ${read_only ? 'supabase_read_only_role' : 'postgres'};
${query};
RESET ROLE;
`;

const statementResults = await db.exec(wrappedQuery);

// Remove last result, which is for the "RESET ROLE" statement
statementResults.pop();

const lastStatementResults = statementResults.at(-1);
try {
// Use transaction to prevent race conditions if tests are parallelized
const result = await db.transaction(async (tx) => {
// Set role before executing query
await tx.exec(
`SET ROLE ${read_only ? 'supabase_read_only_role' : 'postgres'};`
);

// Use query() method with parameters if provided, otherwise use exec()
const queryResult =
parameters && parameters.length > 0
? await tx.query(query, parameters)
: await tx.exec(query);

// Reset role
await tx.exec('RESET ROLE;');

return queryResult;
});

if (!lastStatementResults) {
return HttpResponse.json(
{ message: 'Failed to execute query' },
{ status: 500 }
);
// Handle different response formats
if (Array.isArray(result)) {
// exec() returns an array of results
const lastStatementResults = result.at(-1);
if (!lastStatementResults) {
return HttpResponse.json(
{ message: 'Failed to execute query' },
{ status: 500 }
);
}
return HttpResponse.json(lastStatementResults.rows);
} else {
// query() returns a single result object
return HttpResponse.json(result.rows);
}
} catch (error) {
throw error;
}

return HttpResponse.json(lastStatementResults.rows);
}
),

Expand Down