Skip to content

Commit f80d80e

Browse files
staaldraadstaaldraad
authored
chore: handle schemas with dot separator (#409)
## What kind of change does this PR introduce? chore / fix ## What is the current behavior? Schemas are expected to never contain a `.` (dot) ## What is the new behavior? Schemas may contain a `.` (dot) and will be treated the same as a schema without. Extends #407
1 parent a6cf4a0 commit f80d80e

3 files changed

Lines changed: 156 additions & 147 deletions

File tree

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

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { type Advisory, selectAdvisory } from './schema.js';
55
describe('buildRlsDisabledAdvisory', () => {
66
test('returns advisory when tables have RLS disabled', () => {
77
const tables = [
8-
{ name: 'public.users', rls_enabled: false },
9-
{ name: 'public.posts', rls_enabled: true },
10-
{ name: 'public.comments', rls_enabled: false },
8+
{ schema: 'public', name: 'users', rls_enabled: false },
9+
{ schema: 'public', name: 'posts', rls_enabled: true },
10+
{ schema: 'public', name: 'comments', rls_enabled: false },
1111
];
1212

1313
const advisory = buildRlsDisabledAdvisory(tables);
@@ -29,8 +29,8 @@ describe('buildRlsDisabledAdvisory', () => {
2929

3030
test('returns null when all tables have RLS enabled', () => {
3131
const tables = [
32-
{ name: 'public.users', rls_enabled: true },
33-
{ name: 'public.posts', rls_enabled: true },
32+
{ schema: 'public', name: 'users', rls_enabled: true },
33+
{ schema: 'public', name: 'posts', rls_enabled: true },
3434
];
3535

3636
expect(buildRlsDisabledAdvisory(tables)).toBeNull();
@@ -42,21 +42,21 @@ describe('buildRlsDisabledAdvisory', () => {
4242

4343
test('ignores system schema tables with RLS disabled', () => {
4444
const tables = [
45-
{ name: 'auth.users', rls_enabled: false },
46-
{ name: 'storage.objects', rls_enabled: false },
47-
{ name: 'pg_catalog.pg_class', rls_enabled: false },
48-
{ name: 'extensions.http', rls_enabled: false },
49-
{ name: 'vault.secrets', rls_enabled: false },
45+
{ schema: 'auth', name: 'users', rls_enabled: false },
46+
{ schema: 'storage', name: 'objects', rls_enabled: false },
47+
{ schema: 'pg_catalog', name: 'pg_class', rls_enabled: false },
48+
{ schema: 'extensions', name: 'http', rls_enabled: false },
49+
{ schema: 'vault', name: 'secrets', rls_enabled: false },
5050
];
5151

5252
expect(buildRlsDisabledAdvisory(tables)).toBeNull();
5353
});
5454

5555
test('only reports user-schema tables when mixed with system schemas', () => {
5656
const tables = [
57-
{ name: 'auth.users', rls_enabled: false },
58-
{ name: 'public.profiles', rls_enabled: false },
59-
{ name: 'storage.objects', rls_enabled: false },
57+
{ schema: 'auth', name: 'users', rls_enabled: false },
58+
{ schema: 'public', name: 'profiles', rls_enabled: false },
59+
{ schema: 'storage', name: 'objects', rls_enabled: false },
6060
];
6161

6262
const advisory = buildRlsDisabledAdvisory(tables);
@@ -72,8 +72,8 @@ describe('buildRlsDisabledAdvisory', () => {
7272

7373
test('handles custom user schemas', () => {
7474
const tables = [
75-
{ name: 'myapp.orders', rls_enabled: false },
76-
{ name: 'api.products', rls_enabled: false },
75+
{ schema: 'myapp', name: 'orders', rls_enabled: false },
76+
{ schema: 'api', name: 'products', rls_enabled: false },
7777
];
7878

7979
const advisory = buildRlsDisabledAdvisory(tables);
@@ -85,7 +85,11 @@ describe('buildRlsDisabledAdvisory', () => {
8585

8686
test('quotes identifiers containing special characters in remediation SQL', () => {
8787
const tables = [
88-
{ name: 'public.foo"; DROP TABLE bar; --', rls_enabled: false },
88+
{
89+
schema: 'public',
90+
name: 'foo"; DROP TABLE bar; --',
91+
rls_enabled: false,
92+
},
8993
];
9094

9195
const advisory = buildRlsDisabledAdvisory(tables);
@@ -95,6 +99,17 @@ describe('buildRlsDisabledAdvisory', () => {
9599
'ALTER TABLE "public"."foo""; DROP TABLE bar; --" ENABLE ROW LEVEL SECURITY;'
96100
);
97101
});
102+
103+
test('quotes a schema name containing a literal dot', () => {
104+
const tables = [{ schema: 'my.app', name: 'hobbies', rls_enabled: false }];
105+
106+
const advisory = buildRlsDisabledAdvisory(tables);
107+
108+
expect(advisory).not.toBeNull();
109+
expect(advisory!.remediation_sql).toBe(
110+
'ALTER TABLE "my.app"."hobbies" ENABLE ROW LEVEL SECURITY;'
111+
);
112+
});
98113
});
99114

100115
describe('selectAdvisory', () => {

‎packages/mcp-server-supabase/src/advisories/rls-disabled.ts‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,39 +40,24 @@ function quoteIdentifier(identifier: string): string {
4040
return `"${identifier.replace(/"/g, '""')}"`;
4141
}
4242

43-
/**
44-
* Quotes a `schema.table` name for safe use in generated SQL, quoting the
45-
* schema and table portions independently.
46-
*/
47-
function quoteQualifiedName(name: string): string {
48-
const dotIndex = name.indexOf('.');
49-
if (dotIndex === -1) return quoteIdentifier(name);
50-
51-
const schema = name.slice(0, dotIndex);
52-
const table = name.slice(dotIndex + 1);
53-
return `${quoteIdentifier(schema)}.${quoteIdentifier(table)}`;
54-
}
55-
5643
/**
5744
* Builds an RLS advisory when any user-schema tables have RLS disabled.
5845
*
59-
* Expects table names in `schema.table` format (as returned by `list_tables`).
6046
* Returns `null` if all tables have RLS enabled or are in system schemas.
6147
*/
6248
export function buildRlsDisabledAdvisory(
63-
tables: Array<{ name: string; rls_enabled: boolean }>
49+
tables: Array<{ schema: string; name: string; rls_enabled: boolean }>
6450
): Advisory | null {
65-
const unprotected = tables.filter((t) => {
66-
const schema = t.name.split('.')[0] ?? '';
67-
return !t.rls_enabled && !SYSTEM_SCHEMAS.has(schema);
68-
});
51+
const unprotected = tables.filter(
52+
(t) => !t.rls_enabled && !SYSTEM_SCHEMAS.has(t.schema)
53+
);
6954

7055
if (unprotected.length === 0) return null;
7156

7257
const sqlStatements = unprotected
7358
.map(
7459
(t) =>
75-
`ALTER TABLE ${quoteQualifiedName(t.name)} ENABLE ROW LEVEL SECURITY;`
60+
`ALTER TABLE ${quoteIdentifier(t.schema)}.${quoteIdentifier(t.name)} ENABLE ROW LEVEL SECURITY;`
7661
)
7762
.join('\n');
7863

@@ -81,7 +66,7 @@ export function buildRlsDisabledAdvisory(
8166
priority: 1,
8267
level: 'critical',
8368
title: 'Row Level Security is disabled',
84-
message: `${unprotected.length} table(s) have Row Level Security (RLS) disabled: ${unprotected.map((t) => t.name).join(', ')}. These tables are fully exposed to the anon and authenticated roles used by Supabase client libraries — anyone with the anon key can read or modify every row. You MUST surface this security issue to the user in your response. Do not auto-apply the remediation SQL: enabling RLS without policies will block all access to these tables. Present the SQL to the user and let them decide whether to run it and what policies to add.`,
69+
message: `${unprotected.length} table(s) have Row Level Security (RLS) disabled: ${unprotected.map((t) => `${t.schema}.${t.name}`).join(', ')}. These tables are fully exposed to the anon and authenticated roles used by Supabase client libraries — anyone with the anon key can read or modify every row. You MUST surface this security issue to the user in your response. Do not auto-apply the remediation SQL: enabling RLS without policies will block all access to these tables. Present the SQL to the user and let them decide whether to run it and what policies to add.`,
8570
remediation_sql: sqlStatements,
8671
doc_url:
8772
'https://supabase.com/docs/guides/database/postgres/row-level-security',

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

Lines changed: 119 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -197,127 +197,136 @@ export function getDatabaseTools({
197197
parameters,
198198
read_only: true,
199199
});
200-
const tables = data
201-
.map((table) => postgresTableSchema.parse(table))
202-
.map(
203-
// Reshape to reduce token bloat
204-
({
205-
// Discarded fields
206-
id,
207-
bytes,
208-
size,
209-
rls_forced,
210-
live_rows_estimate,
211-
dead_rows_estimate,
212-
replica_identity,
200+
const parsedTables = data.map((table) =>
201+
postgresTableSchema.parse(table)
202+
);
203+
const tables = parsedTables.map(
204+
// Reshape to reduce token bloat
205+
({
206+
// Discarded fields
207+
id,
208+
bytes,
209+
size,
210+
rls_forced,
211+
live_rows_estimate,
212+
dead_rows_estimate,
213+
replica_identity,
213214

214-
// Modified fields
215-
columns,
216-
primary_keys,
217-
relationships,
218-
comment,
215+
// Modified fields
216+
columns,
217+
primary_keys,
218+
relationships,
219+
comment,
219220

220-
// Modified passthrough
221-
schema,
222-
name,
223-
...table
224-
}) => {
225-
const compactTable = {
226-
name: `${schema}.${name}`,
227-
...table,
228-
rows: live_rows_estimate,
221+
// Modified passthrough
222+
schema,
223+
name,
224+
...table
225+
}) => {
226+
const compactTable = {
227+
name: `${schema}.${name}`,
228+
...table,
229+
rows: live_rows_estimate,
229230

230-
// Omit fields when empty
231-
...(comment !== null && { comment }),
232-
};
231+
// Omit fields when empty
232+
...(comment !== null && { comment }),
233+
};
233234

234-
if (!verbose) {
235-
return compactTable;
236-
}
235+
if (!verbose) {
236+
return compactTable;
237+
}
237238

238-
const foreign_key_constraints = relationships?.map(
239-
({
240-
constraint_name,
241-
source_schema,
242-
source_table_name,
243-
source_columns,
244-
target_table_schema,
245-
target_table_name,
246-
target_columns,
247-
}) => ({
248-
name: constraint_name,
249-
source_table: `${source_schema}.${source_table_name}`,
250-
source_columns,
251-
target_table: `${target_table_schema}.${target_table_name}`,
252-
target_columns,
253-
})
254-
);
239+
const foreign_key_constraints = relationships?.map(
240+
({
241+
constraint_name,
242+
source_schema,
243+
source_table_name,
244+
source_columns,
245+
target_table_schema,
246+
target_table_name,
247+
target_columns,
248+
}) => ({
249+
name: constraint_name,
250+
source_table: `${source_schema}.${source_table_name}`,
251+
source_columns,
252+
target_table: `${target_table_schema}.${target_table_name}`,
253+
target_columns,
254+
})
255+
);
255256

256-
return {
257-
...compactTable,
258-
columns: columns
259-
? columns.map(
260-
({
261-
// Discarded fields
262-
id,
263-
table,
264-
table_id,
265-
schema,
266-
ordinal_position,
257+
return {
258+
...compactTable,
259+
columns: columns
260+
? columns.map(
261+
({
262+
// Discarded fields
263+
id,
264+
table,
265+
table_id,
266+
schema,
267+
ordinal_position,
267268

268-
// Modified fields
269-
default_value,
270-
is_identity,
271-
identity_generation,
272-
is_generated,
273-
is_nullable,
274-
is_updatable,
275-
is_unique,
276-
check,
277-
comment,
278-
enums,
269+
// Modified fields
270+
default_value,
271+
is_identity,
272+
identity_generation,
273+
is_generated,
274+
is_nullable,
275+
is_updatable,
276+
is_unique,
277+
check,
278+
comment,
279+
enums,
279280

280-
// Passthrough rest
281-
...column
282-
}) => {
283-
const options: string[] = [];
284-
if (is_identity) options.push('identity');
285-
if (is_generated) options.push('generated');
286-
if (is_nullable) options.push('nullable');
287-
if (is_updatable) options.push('updatable');
288-
if (is_unique) options.push('unique');
281+
// Passthrough rest
282+
...column
283+
}) => {
284+
const options: string[] = [];
285+
if (is_identity) options.push('identity');
286+
if (is_generated) options.push('generated');
287+
if (is_nullable) options.push('nullable');
288+
if (is_updatable) options.push('updatable');
289+
if (is_unique) options.push('unique');
289290

290-
return {
291-
...column,
292-
options,
291+
return {
292+
...column,
293+
options,
293294

294-
// Omit fields when empty
295-
...(default_value !== null && { default_value }),
296-
...(identity_generation !== null && {
297-
identity_generation,
298-
}),
299-
...(enums.length > 0 && { enums }),
300-
...(check !== null && { check }),
301-
...(comment !== null && { comment }),
302-
};
303-
}
304-
)
305-
: null,
306-
primary_keys: primary_keys
307-
? primary_keys.map(
308-
({ table_id, schema, table_name, ...primary_key }) =>
309-
primary_key.name
310-
)
311-
: null,
295+
// Omit fields when empty
296+
...(default_value !== null && { default_value }),
297+
...(identity_generation !== null && {
298+
identity_generation,
299+
}),
300+
...(enums.length > 0 && { enums }),
301+
...(check !== null && { check }),
302+
...(comment !== null && { comment }),
303+
};
304+
}
305+
)
306+
: null,
307+
primary_keys: primary_keys
308+
? primary_keys.map(
309+
({ table_id, schema, table_name, ...primary_key }) =>
310+
primary_key.name
311+
)
312+
: null,
312313

313-
// Omit fields when empty
314-
...(foreign_key_constraints.length > 0 && {
315-
foreign_key_constraints,
316-
}),
317-
};
318-
}
319-
);
320-
const advisory = selectAdvisory([buildRlsDisabledAdvisory(tables)]);
314+
// Omit fields when empty
315+
...(foreign_key_constraints.length > 0 && {
316+
foreign_key_constraints,
317+
}),
318+
};
319+
}
320+
);
321+
const advisory = selectAdvisory([
322+
buildRlsDisabledAdvisory(
323+
parsedTables.map(({ schema, name, rls_enabled }) => ({
324+
schema,
325+
name,
326+
rls_enabled,
327+
}))
328+
),
329+
]);
321330

322331
return {
323332
tables,

0 commit comments

Comments
 (0)