execute_sql runs MERGE ... WHEN MATCHED THEN DELETE without the destructive confirmation

Open
  • Is something wrong?
#532 4 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 1 day

@barryroodt is already working on this.

Since Nov 9, 2026.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
69/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
typescript
Domain
database, security

Research direction

Start with packages/mcp-server-supabase/src/tools/destructive-sql.ts, especially isDestructiveSql and the EXPLAIN recheck, then review the confirmation path in database-operation-tools.ts. Reproduce the listed MERGE cases and compare them with DELETE and UPDATE. Done means destructive MERGE actions trigger confirmation while safe statements retain their current behavior.

Written by the indexing model from the issue text.

Description

MCP

Hey folks, I think execute_sql lets a MERGE ... WHEN MATCHED THEN DELETE (or THEN UPDATE) run without the destructive SQL confirmation, even when it touches every row.

What the confirmation is for

When execute_sql is in confirmation.enabledTools (opt in, and the client needs elicitation support), the server asks before running SQL it considers destructive. The prompt text in database-operation-tools.ts:502 says it covers "DROP, DELETE, TRUNCATE or UPDATE without WHERE". A MERGE that deletes or updates every matched row seems to fit that intent pretty squarely.

Current behavior
  1. Create a table with two rows: create table films (id int, title text); insert into films values (1, 'a'), (2, 'b');
  2. Run the server not read only, with execute_sql in confirmation.enabledTools, using a client that supports elicitation.
  3. Call execute_sql with merge into films f using films s on true when matched then delete
  4. No confirmation is shown (the decision is recorded as bypass with reason not_destructive).
  5. Both rows are gone.
Expected behavior

The same confirmation you get for delete from films should show up before the MERGE runs.

Minimal repro

Against main at 7e5037b:

import { isDestructiveSql } from './packages/mcp-server-supabase/src/tools/destructive-sql.js';

isDestructiveSql('merge into films f using (select 1) s on true when matched then delete');                  // false
isDestructiveSql('merge into films f using films s on true when matched then delete');                       // false
isDestructiveSql('explain analyze merge into films f using films s on true when matched then delete');       // false
isDestructiveSql('merge into films f using films s on true when matched then update set title = null');      // false

isDestructiveSql('delete from films');                // true
isDestructiveSql('update films set title = null');    // true

From reading the code, I think this is why:

  • destructive-sql.ts:70 only flags statements that start with drop, delete or truncate.
  • isUpdateWithoutWhereFromScan (destructive-sql.ts:191 to 197) needs the statement to start with update.
  • The explain analyze recheck (destructive-sql.ts:272 to 279) runs those same checks on the body, so it misses MERGE too.
  • database-operation-tools.ts:78 then skips the confirmation with not_destructive.
flowchart LR
  A[execute_sql with MERGE] --> B{starts with drop, delete, truncate or update?}
  B -- no --> C[not_destructive]
  C --> D[runs with no confirmation]
Related work

Huge thanks to @barryroodt for #408, which added this whole confirmation flow, and for #426, which moves the check onto a real parser. The data modifying CTE forms (with d as (delete ... returning) select ..., and explain analyze of them) are already listed as a known trade off in #408 and handled in #426, so I'm only raising MERGE here. From what I can see, classifyStatement in #426 doesn't have a MergeStmt branch yet, so MERGE would still slip through after it lands.

Question

Would you rather have MERGE added as a small check on the current scanner, or handled as a MergeStmt case in #426? Totally happy either way, and I'm glad to send a PR for whichever you prefer (or leave it to Barry if it fits better in #426). Let me know if I've misread anything!

Dominant language
TypeScript
Stars
3k
Forks
432
Avg merge
1d 7h
Merged PRs (30d)
52

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from supabase/mcp

All issues in supabase/mcp

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.