fix(list_table): preventing sql injection attack on list_tables (#165)

* preventing sql injection attack on list_tables

* refactor: address review comments - use unknown[] and add transaction wrapper

- Replace z.any() with z.unknown() for better type safety (aligns with postgres-meta)
- Wrap query execution in transaction to prevent race conditions for future parallelization
- All 123 tests passing

* test: use division by zero payload from HackerOne report

- Changed SQL injection test to use actual payload from Linear issue AI-139
- Payload: "public') OR (SELECT 1)=1/0--" which causes division by zero error without parameterization
- With parameterized queries, returns empty array [] instead of error
- Addresses review feedback about using realistic example from original report

* chore: remove debug console.log statements

Author: Rodriguespn

.vscode/settings.json

@@ -5,4 +5,4 @@
   "[json]": {
     "editor.defaultFormatter": "biomejs.biome"
   }
-}
+}

packages/mcp-server-supabase/src/pg-meta/index.ts

@@ -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 };
 }
 
 /**

packages/mcp-server-supabase/src/platform/api-platform.ts

@@ -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',
@@ -179,6 +180,7 @@ export function createSupabaseApiPlatform(
           },
           body: {
             query,
+            parameters,
             read_only,
           },
         }

packages/mcp-server-supabase/src/platform/types.ts

@@ -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(),
 });
 

packages/mcp-server-supabase/src/server.test.ts

@@ -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();
 

packages/mcp-server-supabase/src/tools/database-operation-tools.ts

@@ -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

packages/mcp-server-supabase/src/util.ts

@@ -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]

packages/mcp-server-supabase/test/mocks.ts

@@ -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);
@@ -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);
     }
   ),