Fix: block dangerous administrative functions in readonly mode

[calghar]

While looking at the readonly implementation, I noticed that PostgreSQL administrative functions like pg_read_file(), pg_ls_dir(), and set_config() pass right through the keyword check — they are valid SELECT statements and do not contain any mutating keywords. PostgreSQL's default_transaction_read_only=on does not help here either since these functions are gated by role privileges, not transaction mode.

In practice, this means a user connecting with superuser (which is common in quick setups and demos) can read arbitrary files from the server filesystem even with readonly = true.

This is a different class of issue from #275 (CTEs) and #284 (conditional comments) — those hid DML inside creative syntax. This one uses perfectly normal SELECT statements that happen to call privileged functions.

The fix

A per-dialect denylist of known dangerous functions, checked after the existing keyword validation passes. The regex matches function names followed by ( to avoid false positives on identifiers or column names that happen to share a name with these functions.

Covered dialects:

• PostgreSQL: pg_read_file, pg_ls_dir, pg_stat_file, set_config, pg_terminate_backend, dblink_exec, lo_export, and others
• MySQL/MariaDB: load_file, INTO OUTFILE/DUMPFILE
• SQL Server: xp_cmdshell, xp_dirtree, openrowset, opendatasource

Normal functions (aggregates, string ops, current_setting, pg_catalog queries) are not affected.

Reproduction

[[sources]]
id = "default"
dsn = "postgres://postgres:password@localhost:5432/mydb"

[[tools]]
name = "execute_sql"
source = "default"
readonly = true

{"method":"tools/call","params":{"name":"execute_sql","arguments":{"sql":"SELECT pg_read_file('/etc/passwd', 0, 200)"}}}

Before this fix: returns file contents. After: rejected with the standard readonly error message.

Testing

• 29 new unit tests covering all blocked functions plus false-positive checks for safe functions
• All 689 existing unit tests pass unchanged
• Verified end-to-end against PostgreSQL 16

A note on scope

I understand the docs say readonly is not a security boundary, and using a restricted database role is the proper defence. That said, the keyword check already blocks INSERT/DELETE/DROP regardless of connection privileges, so it felt consistent to also block functions with equivalent impact.

[Copilot]

Pull request overview

This PR hardens readonly SQL validation by adding per-dialect checks for privileged functions and clauses that can have side effects despite appearing in allowed SELECT-style statements.

Changes:

• Adds dangerous-function denylist patterns for PostgreSQL, MySQL/MariaDB, and SQL Server.
• Applies the denylist after existing readonly keyword validation.
• Adds unit tests for blocked functions and selected safe-function false positives.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/utils/allowed-keywords.ts	Adds dangerous administrative function patterns and readonly rejection logic.
src/utils/__tests__/allowed-keywords.test.ts	Adds coverage for blocked PostgreSQL, MySQL/MariaDB, and SQL Server function usage.

Comments suppressed due to low confidence (3)

src/utils/allowed-keywords.ts:138

• This check runs after stripCommentsAndStrings, which removes PostgreSQL double-quoted identifiers. PostgreSQL still allows calls such as SELECT "pg_read_file"(...), so quoting a dangerous function name bypasses the denylist and leaves the readonly file-read issue unfixed.

  const dangerousPattern = dangerousFunctions[connectorType as ConnectorType];
  if (dangerousPattern && dangerousPattern.test(cleanedSQL)) {

src/utils/allowed-keywords.ts:25

• The PostgreSQL filesystem denylist omits other built-in directory listing functions such as pg_ls_replslotdir, pg_ls_logicalmapdir, and pg_ls_logicalsnapdir. These are also callable from SELECT and expose server-side filesystem metadata, so the same readonly bypass remains for those administrative functions.

  postgres: /\b(?:pg_read_file|pg_read_binary_file|pg_ls_dir|pg_ls_logdir|pg_ls_waldir|pg_ls_tmpdir|pg_ls_archive_statusdir|pg_stat_file|pg_terminate_backend|pg_cancel_backend|pg_reload_conf|pg_rotate_logfile|set_config|dblink|dblink_exec|dblink_connect|lo_export|lo_import|pg_file_write|pg_file_rename|pg_file_unlink)\s*\(/i,

src/utils/allowed-keywords.ts:29

• SQL Server also supports OPENQUERY(linked_server, '...'), which can execute a pass-through command on a linked server from a SELECT. Because the command text is stripped as a string literal, mutating SQL inside it would not be detected, so omitting openquery leaves a similar readonly bypass.

  sqlserver: /\b(?:xp_cmdshell|xp_fileexist|xp_dirtree|xp_subdirs|xp_fixeddrives|openrowset|opendatasource|bulk\s+insert)\s*\(/i,

[calghar]

Addressed the review feedback:

• MySQL load_file false positive: Now requires ( after load_file so column references aren't rejected.
• Missing dblink_send_query: Added to PostgreSQL pattern.
• Missing pg_ls_* variants: Added pg_ls_replslotdir, pg_ls_logicalmapdir, pg_ls_logicalsnapdir.
• Missing OPENQUERY: Added to SQL Server pattern.

On the double-quoted identifier bypass (SELECT "pg_read_file"(...)) — this is a pre-existing architectural limitation since stripCommentsAndStrings removes double-quoted blocks before any validation runs. The same issue affects the existing mutating keyword checks. Addressing it would require changes to the strip/validate pipeline and felt out of scope for this PR.