Compare commits

...

2 Commits

Author SHA1 Message Date
Rip&Tear
338662201f docs: document NL2SQL read-only enforcement and its limits
The page claimed read-only mode blocked "multi-statement queries containing
semicolons" and said nothing about CTEs, EXPLAIN ANALYZE, or the fact that a
SELECT can still reach the database server's filesystem. Both gaps matter now
that those routes are enforced.

- Replace the semicolon sentence with a table of every indirect write route
  blocked in read-only mode: multi-statement, writable CTEs (including
  AS MATERIALIZED), a write after a CTE, EXPLAIN ANALYZE, INTO OUTFILE,
  server-filesystem functions, and unparseable WITH statements.
- Explain that analysis masks literals and comments, so `SELECT 'DROP TABLE
  users'` is allowed while `EXPLAIN /*x*/ ANALYZE DELETE ...` is blocked, and
  that a semicolon inside a literal no longer splits statements.
- Document the new SET TRANSACTION READ ONLY backstop and which backends
  fall back without it.
- Add a warning that these checks are defence in depth and a least-privileged
  read-only database role is the only complete control.

Every example in the new table was verified against the implementation.
Applied to en/ar/ko/pt-BR under docs/edge; the ko and pt-BR pages carry the
warning inline as they have no Hardening Recommendations section.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 08:47:04 +08:00
Rip&Tear
7f6367a21c fix(tools): close NL2SQL read-only mode bypasses
NL2SQLTool's read-only mode could be bypassed three ways, all confirmed by
executing the validators directly.

1. `_AS_PAREN_RE` was `\bAS\s*\(`, which never matches PostgreSQL's
   `AS [NOT] MATERIALIZED (`. `WITH d AS MATERIALIZED (DELETE FROM users
   RETURNING *) SELECT * FROM d` therefore parsed as having no CTE body at
   all, and `_validate_statement` returned without running a single check.

2. `_resolve_explain_command` scanned raw text, so a comment between the
   keywords (`EXPLAIN /*x*/ ANALYZE DELETE FROM users`) stalled option
   parsing and the statement was treated as an inert EXPLAIN. EXPLAIN
   ANALYZE executes its argument.

3. The first-keyword allowlist admits statements that begin with SELECT but
   write, and those survive a transaction rollback: MySQL
   `SELECT ... INTO OUTFILE` writes a file on the DB server, and
   `pg_read_file` / `lo_import` / `dblink_exec` reach its filesystem or open
   a connection outside the transaction.

Changes:

- Analyse statements over a mask that blanks string literals, dollar-quoted
  strings, quoted identifiers and comments while preserving offsets, so a
  keyword in a literal is never matched and one behind a comment always is.
  MySQL executable comments (`/*! ... */`) are left visible because the
  server runs them.
- Match the `AS [NOT] MATERIALIZED (` spelling.
- Validate CTE bodies against an allowlist of read-only leading keywords
  instead of a write-command denylist, and fail closed: a WITH statement
  whose CTE bodies cannot be located, or which has no query after them, is
  now rejected rather than passed through.
- Block INTO OUTFILE/DUMPFILE and known server-filesystem functions.
- Mark the transaction `SET TRANSACTION READ ONLY` in read-only mode where
  the backend supports it, so enforcement no longer rests on parsing alone.
  Backends without the syntax log and fall back.
- Split statements on semicolons outside strings and comments, which also
  stops a semicolon in a literal from being rejected as multi-statement.
- Document that a least-privileged read-only DB role is the actual control
  and these checks are defence in depth.

Adds 30 regression tests. Full file: 111 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 08:38:07 +08:00
6 changed files with 644 additions and 126 deletions

View File

@@ -27,6 +27,10 @@ mode: "wide"
## توصيات التقوية
<Warning>
تُعد فحوصات القراءة فقط المدمجة طبقة دفاع إضافية، وليست حدًا كاملًا. فهي تفحص نص العبارة، ولغة SQL تختلف باختلاف نظام قواعد البيانات: إذ يمكن لعبارة تبدأ بـ `SELECT` أن تصل إلى نظام ملفات خادم قاعدة البيانات (`SELECT ... INTO OUTFILE`، `pg_read_file()`) أو أن تستدعي دالة ذات آثار جانبية. تُحظر المنافذ المعروفة صراحةً، لكن التحكم الكامل الوحيد هو الصلاحيات التي تمنحها في `db_uri`. **وجّه الأداة إلى دور قاعدة بيانات للقراءة فقط بأقل الصلاحيات الممكنة.**
</Warning>
استخدم جميع الإجراءات التالية في بيئة الإنتاج:
- استخدم مستخدم قاعدة بيانات للقراءة فقط كلما أمكن
@@ -47,7 +51,21 @@ mode: "wide"
أي محاولة لتنفيذ عملية كتابة (`INSERT`، `UPDATE`، `DELETE`، `DROP`، `CREATE`، `ALTER`، `TRUNCATE`، إلخ) ستُسبب خطأً ما لم يتم تفعيل DML صراحةً.
كما تُحظر الاستعلامات متعددة العبارات التي تحتوي على فاصلة منقوطة (مثل `SELECT 1; DROP TABLE users`) في وضع القراءة فقط لمنع هجمات الحقن.
كما يحظر وضع القراءة فقط الطرق غير المباشرة للكتابة:
| المحظور في وضع القراءة فقط | مثال |
| --- | --- |
| الاستعلامات متعددة العبارات | `SELECT 1; DROP TABLE users` |
| تعبيرات CTE الكاتبة، بما في ذلك صيغة `MATERIALIZED` | `WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d` |
| عملية كتابة تلي تعبير CTE | `WITH d AS (SELECT 1) DELETE FROM users` |
| `EXPLAIN ANALYZE`، الذي ينفّذ العبارة التابعة له فعليًا | `EXPLAIN ANALYZE DELETE FROM users` |
| الكتابة إلى نظام ملفات خادم قاعدة البيانات | `SELECT * FROM users INTO OUTFILE '/var/www/shell.php'` |
| الدوال التي تصل إلى نظام ملفات الخادم أو تفتح اتصالًا جديدًا | `SELECT pg_read_file('/etc/passwd')`، `dblink_exec(...)` |
| عبارة `WITH` يتعذّر تحليلها للتأكد من أنها للقراءة فقط | `WITH d AS DELETE FROM users` |
تُحلَّل العبارات بعد إخفاء النصوص الحرفية والتعليقات، لذا لا تُعامَل كلمة مفتاحية مخبأة داخل نص حرفي على أنها أمر (`SELECT 'DROP TABLE users'` مسموح)، ولا يُخفي تعليقٌ موضوع بين الكلمات المفتاحية أمرًا (`EXPLAIN /*x*/ ANALYZE DELETE ...` محظور). كما أن الفاصلة المنقوطة داخل نص حرفي ليست فاصلًا بين العبارات، لذا فإن `SELECT ';'` عبارة واحدة صحيحة.
في وضع القراءة فقط، تضع الأداة أيضًا المعاملة في حالة `SET TRANSACTION READ ONLY`، فترفض PostgreSQL وMySQL عمليات الكتابة على مستوى قاعدة البيانات مهما كانت صياغة العبارة. أما الأنظمة التي لا تدعم هذه الصيغة (SQLite، SQL Server، Snowflake) فتُسجّل رسالة تصحيح وتعود إلى الاعتماد على فحص العبارات وحده — وهذا سبب إضافي للاعتماد على دور للقراءة فقط بدلًا من التحليل النصي.
### تفعيل عمليات الكتابة

View File

@@ -29,6 +29,10 @@ If you route untrusted input to agents using this tool, treat it as a high-risk
## Hardening Recommendations
<Warning>
The built-in read-only checks are defence in depth, not a complete boundary. They inspect the statement text, and SQL is dialect-specific: a statement beginning with `SELECT` can still reach the database server's filesystem (`SELECT ... INTO OUTFILE`, `pg_read_file()`) or call a side-effecting function. The known sinks are blocked explicitly, but the only complete control is the privileges you grant in `db_uri`. **Point the tool at a least-privileged, read-only database role.**
</Warning>
Use all of the following in production:
- Use a read-only database user whenever possible
@@ -49,7 +53,21 @@ Use all of the following in production:
Any attempt to execute a write operation (`INSERT`, `UPDATE`, `DELETE`, `DROP`, `CREATE`, `ALTER`, `TRUNCATE`, etc.) will raise an error unless DML is explicitly enabled.
Multi-statement queries containing semicolons (e.g. `SELECT 1; DROP TABLE users`) are also blocked in read-only mode to prevent injection attacks.
Read-only mode also blocks the indirect routes to a write:
| Blocked in read-only mode | Example |
| --- | --- |
| Multi-statement queries | `SELECT 1; DROP TABLE users` |
| Writable CTEs, including the materialised spelling | `WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d` |
| A write following a CTE | `WITH d AS (SELECT 1) DELETE FROM users` |
| `EXPLAIN ANALYZE`, which executes its argument | `EXPLAIN ANALYZE DELETE FROM users` |
| Writes to the database server's filesystem | `SELECT * FROM users INTO OUTFILE '/var/www/shell.php'` |
| Functions reaching the server's filesystem or opening a new connection | `SELECT pg_read_file('/etc/passwd')`, `dblink_exec(...)` |
| A `WITH` statement that cannot be parsed as read-only | `WITH d AS DELETE FROM users` |
Statements are analysed with string literals and comments masked out, so a keyword hidden in a literal is not mistaken for a command (`SELECT 'DROP TABLE users'` is allowed) and a comment placed between keywords does not hide one (`EXPLAIN /*x*/ ANALYZE DELETE ...` is blocked). A semicolon inside a string literal does not count as a statement separator, so `SELECT ';'` is a single valid statement.
In read-only mode the tool additionally marks the transaction `SET TRANSACTION READ ONLY`, so PostgreSQL and MySQL reject writes at the database regardless of how the statement was spelled. Backends without that syntax (SQLite, SQL Server, Snowflake) log a debug message and fall back to statement validation alone — one more reason to rely on a read-only role rather than on parsing.
### Enabling Write Operations

View File

@@ -24,7 +24,25 @@ mode: "wide"
DML을 명시적으로 활성화하지 않으면 쓰기 작업(`INSERT`, `UPDATE`, `DELETE`, `DROP`, `CREATE`, `ALTER`, `TRUNCATE` 등)을 실행하려고 할 때 오류가 발생합니다.
읽기 전용 모드에서는 세미콜론이 포함된 다중 구문 쿼리(예: `SELECT 1; DROP TABLE users`)도 인젝션 공격을 방지하기 위해 차단니다.
읽기 전용 모드는 우회 경로를 통한 쓰기도 차단니다:
| 읽기 전용 모드에서 차단됨 | 예시 |
| --- | --- |
| 다중 구문 쿼리 | `SELECT 1; DROP TABLE users` |
| 쓰기 CTE(`MATERIALIZED` 표기 포함) | `WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d` |
| CTE 뒤에 오는 쓰기 작업 | `WITH d AS (SELECT 1) DELETE FROM users` |
| 인자를 실제로 실행하는 `EXPLAIN ANALYZE` | `EXPLAIN ANALYZE DELETE FROM users` |
| 데이터베이스 서버 파일시스템에 대한 쓰기 | `SELECT * FROM users INTO OUTFILE '/var/www/shell.php'` |
| 서버 파일시스템에 접근하거나 새 연결을 여는 함수 | `SELECT pg_read_file('/etc/passwd')`, `dblink_exec(...)` |
| 읽기 전용임을 확인할 수 없는 `WITH` 구문 | `WITH d AS DELETE FROM users` |
구문은 문자열 리터럴과 주석을 가린 상태에서 분석됩니다. 따라서 리터럴 안에 숨은 키워드는 명령으로 오인되지 않고(`SELECT 'DROP TABLE users'`는 허용), 키워드 사이에 삽입된 주석이 명령을 숨기지도 못합니다(`EXPLAIN /*x*/ ANALYZE DELETE ...`는 차단). 문자열 리터럴 안의 세미콜론은 구문 구분자로 세지 않으므로 `SELECT ';'`는 유효한 단일 구문입니다.
읽기 전용 모드에서는 트랜잭션에 `SET TRANSACTION READ ONLY`도 적용하므로, PostgreSQL과 MySQL은 구문 표기 방식과 무관하게 데이터베이스 차원에서 쓰기를 거부합니다. 해당 구문을 지원하지 않는 백엔드(SQLite, SQL Server, Snowflake)는 디버그 로그를 남기고 구문 검증에만 의존합니다. 파싱이 아니라 읽기 전용 역할에 의존해야 하는 이유가 하나 더 있는 셈입니다.
<Warning>
내장된 읽기 전용 검사는 완전한 경계가 아니라 심층 방어 수단입니다. 이 검사는 구문 텍스트를 검사하며 SQL은 데이터베이스마다 다릅니다. `SELECT`로 시작하는 구문도 데이터베이스 서버의 파일시스템에 접근하거나(`SELECT ... INTO OUTFILE`, `pg_read_file()`) 부수 효과가 있는 함수를 호출할 수 있습니다. 알려진 경로는 명시적으로 차단하지만, 완전한 통제 수단은 `db_uri`에 부여하는 권한뿐입니다. **최소 권한의 읽기 전용 데이터베이스 역할을 사용하십시오.**
</Warning>
### 쓰기 작업 활성화

View File

@@ -24,7 +24,25 @@ O `NL2SQLTool` opera em **modo somente leitura por padrão**. Apenas os seguinte
Qualquer tentativa de executar uma operação de escrita (`INSERT`, `UPDATE`, `DELETE`, `DROP`, `CREATE`, `ALTER`, `TRUNCATE`, etc.) resultará em erro, a menos que o DML seja habilitado explicitamente.
Consultas com múltiplas instruções contendo ponto e vírgula (ex.: `SELECT 1; DROP TABLE users`) também são bloqueadas no modo somente leitura para prevenir ataques de injeção.
O modo somente leitura também bloqueia os caminhos indiretos para uma escrita:
| Bloqueado no modo somente leitura | Exemplo |
| --- | --- |
| Consultas com múltiplas instruções | `SELECT 1; DROP TABLE users` |
| CTEs de escrita, incluindo a forma `MATERIALIZED` | `WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d` |
| Uma escrita após uma CTE | `WITH d AS (SELECT 1) DELETE FROM users` |
| `EXPLAIN ANALYZE`, que executa seu argumento | `EXPLAIN ANALYZE DELETE FROM users` |
| Escrita no sistema de arquivos do servidor de banco de dados | `SELECT * FROM users INTO OUTFILE '/var/www/shell.php'` |
| Funções que alcançam o sistema de arquivos do servidor ou abrem uma nova conexão | `SELECT pg_read_file('/etc/passwd')`, `dblink_exec(...)` |
| Uma instrução `WITH` que não pode ser confirmada como somente leitura | `WITH d AS DELETE FROM users` |
As instruções são analisadas com literais de texto e comentários mascarados, de modo que uma palavra-chave escondida em um literal não é confundida com um comando (`SELECT 'DROP TABLE users'` é permitido) e um comentário colocado entre palavras-chave não esconde nenhum (`EXPLAIN /*x*/ ANALYZE DELETE ...` é bloqueado). Um ponto e vírgula dentro de um literal não conta como separador de instruções, portanto `SELECT ';'` é uma única instrução válida.
No modo somente leitura, a ferramenta também marca a transação como `SET TRANSACTION READ ONLY`, de forma que PostgreSQL e MySQL rejeitam escritas no próprio banco de dados, independentemente de como a instrução foi escrita. Backends sem essa sintaxe (SQLite, SQL Server, Snowflake) registram uma mensagem de depuração e voltam a depender apenas da validação de instruções — mais um motivo para confiar em um papel somente leitura em vez da análise textual.
<Warning>
As verificações internas de somente leitura são defesa em profundidade, não uma fronteira completa. Elas inspecionam o texto da instrução, e SQL é específico de cada banco: uma instrução que começa com `SELECT` ainda pode alcançar o sistema de arquivos do servidor (`SELECT ... INTO OUTFILE`, `pg_read_file()`) ou chamar uma função com efeitos colaterais. Os caminhos conhecidos são bloqueados explicitamente, mas o único controle completo são os privilégios concedidos em `db_uri`. **Aponte a ferramenta para um papel de banco de dados somente leitura e com privilégio mínimo.**
</Warning>
### Habilitando Operações de Escrita

View File

@@ -60,64 +60,54 @@ _WRITE_COMMANDS = {
}
# Subset of write commands that can realistically appear *inside* a CTE body.
# Narrower than _WRITE_COMMANDS to avoid false positives on identifiers like
# ``comment``, ``set``, or ``reset`` which are common column/table names.
_CTE_WRITE_INDICATORS = {
"INSERT",
"UPDATE",
"DELETE",
"DROP",
"ALTER",
"CREATE",
"TRUNCATE",
"MERGE",
# Keywords that may legitimately open a CTE body in read-only mode. This is an
# allowlist rather than a write-command denylist: anything unrecognised at the
# head of a CTE body is treated as a write and blocked, so a dialect keyword we
# have not enumerated cannot slip through (see _validate_cte_statement).
_CTE_READ_ONLY_LEADS = _READ_ONLY_COMMANDS | {
"VALUES",
"TABLE",
"WITH",
"SEARCH",
"CYCLE",
}
_AS_PAREN_RE = re.compile(r"\bAS\s*\(", re.IGNORECASE)
# ``AS (`` optionally preceded by PostgreSQL's [NOT] MATERIALIZED modifier.
# Without the modifier branch, ``WITH d AS MATERIALIZED (DELETE …)`` parses as
# having no CTE body at all and skips validation entirely.
_AS_PAREN_RE = re.compile(
r"\bAS\s+(?:NOT\s+)?MATERIALIZED\s*\(|\bAS\s*\(", re.IGNORECASE
)
# MySQL runs the body of a version-gated comment (``/*!40001 … */``) as real
# SQL, so those must not be masked away as inert comment text.
_MYSQL_EXEC_COMMENT_PREFIX = "/*!"
# PostgreSQL dollar-quoted string delimiters: $$ … $$ or $tag$ … $tag$.
_DOLLAR_QUOTE_RE = re.compile(r"\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$")
# Server-side file writes reachable from a plain SELECT, which no amount of
# transaction-level read-only enforcement prevents.
_FILE_SINK_RE = re.compile(r"\bINTO\s+(?:OUTFILE|DUMPFILE)\b", re.IGNORECASE)
# Functions that read or write the database server's filesystem, or open a new
# connection that escapes the current (read-only) transaction. Callable from a
# SELECT, so the first-keyword check never sees them.
_SERVER_FILE_FUNC_RE = re.compile(
r"\b(?:pg_read_file|pg_read_binary_file|pg_ls_dir|pg_stat_file|pg_logdir_ls"
r"|lo_import|lo_export|load_file|dblink|dblink_exec|dblink_send_query)\s*\(",
re.IGNORECASE,
)
def _iter_as_paren_matches(stmt: str) -> Iterator[re.Match[str]]:
"""Yield regex matches for ``AS\\s*(`` outside of string literals."""
in_string: set[int] = set()
i = 0
while i < len(stmt):
if stmt[i] == "'":
start = i
end = _skip_string_literal(stmt, i)
in_string.update(range(start, end))
i = end
else:
i += 1
def _skip_quoted(stmt: str, pos: int) -> int:
"""Skip past the quoted run starting at *pos*.
for m in _AS_PAREN_RE.finditer(stmt):
if m.start() not in in_string:
yield m
def _detect_writable_cte(stmt: str) -> str | None:
"""Return the first write command inside a CTE body, or None.
Instead of tokenizing the whole statement (which falsely matches column
names like ``comment``), this walks through parenthesized CTE bodies and
checks only the *first keyword after* an opening ``AS (`` for a write
command. Uses a regex to handle any whitespace (spaces, tabs, newlines)
between ``AS`` and ``(``. Skips matches inside string literals.
"""
for m in _iter_as_paren_matches(stmt):
body = stmt[m.end() :].lstrip()
first_word = body.split()[0].upper().strip("()") if body.split() else ""
if first_word in _CTE_WRITE_INDICATORS:
return first_word
return None
def _skip_string_literal(stmt: str, pos: int) -> int:
"""Skip past a string literal starting at pos (single-quoted).
Handles escaped quotes ('') inside the literal.
Returns the index after the closing quote.
Handles single-quoted literals, double-quoted identifiers (or strings under
ANSI_QUOTES) and MySQL backtick identifiers, including the doubled-delimiter
escape (``''``). Returns the index just past the closing delimiter, or the
end of the string when the run is unterminated.
"""
quote_char = stmt[pos]
i = pos + 1
@@ -131,15 +121,142 @@ def _skip_string_literal(stmt: str, pos: int) -> int:
return i # Unterminated literal — return end
def _find_matching_close_paren(stmt: str, start: int) -> int:
"""Find the matching close paren, skipping string literals."""
# Kept as an alias because the single-quote case is the one callers reason about.
_skip_string_literal = _skip_quoted
def _mask_inert_spans(stmt: str) -> str:
"""Blank out quoted runs and comments, preserving every character offset.
Analysis runs over the mask so that keywords hidden inside strings are not
matched, and keywords hidden *behind* comments are. Offsets are preserved so
a match found in the mask can be sliced out of the original statement.
MySQL executable comments (``/*! … */``) are deliberately left visible: the
server executes their contents, so the validator must see them too.
Args:
stmt: The SQL statement to mask.
Returns:
A same-length copy of *stmt* with inert spans replaced by spaces.
"""
out = list(stmt)
n = len(stmt)
i = 0
def blank(start: int, end: int) -> None:
for k in range(start, min(end, n)):
if out[k] != "\n": # keep line structure for "--" comment scanning
out[k] = " "
while i < n:
ch = stmt[i]
if stmt.startswith("--", i):
end = stmt.find("\n", i)
end = n if end == -1 else end
blank(i, end)
i = end
elif stmt.startswith("/*", i) and not stmt.startswith(
_MYSQL_EXEC_COMMENT_PREFIX, i
):
depth = 1
j = i + 2
while j < n and depth > 0:
if stmt.startswith("/*", j):
depth += 1
j += 2
elif stmt.startswith("*/", j):
depth -= 1
j += 2
else:
j += 1
blank(i, j)
i = j
elif ch in ("'", '"', "`"):
end = _skip_quoted(stmt, i)
blank(i, end)
i = end
elif ch == "$" and (m := _DOLLAR_QUOTE_RE.match(stmt, i)):
tag = m.group(0)
close = stmt.find(tag, m.end())
end = n if close == -1 else close + len(tag)
blank(i, end)
i = end
else:
i += 1
return "".join(out)
def _split_statements(sql_query: str) -> list[str]:
"""Split *sql_query* on semicolons that are not inside a string or comment.
A naive ``str.split(";")`` both rejects legitimate queries containing a
semicolon in a literal and miscounts statements when one is hidden in a
comment.
"""
masked = _mask_inert_spans(sql_query)
statements: list[str] = []
start = 0
for i, ch in enumerate(masked):
if ch == ";":
chunk = sql_query[start:i].strip()
if chunk:
statements.append(chunk)
start = i + 1
tail = sql_query[start:].strip()
if tail:
statements.append(tail)
return statements
def _iter_as_paren_matches(masked: str) -> Iterator[re.Match[str]]:
"""Yield ``AS (`` matches over an already-masked statement."""
return _AS_PAREN_RE.finditer(masked)
def _first_keyword(text_: str) -> str:
"""Return the leading SQL keyword of *text_*, uppercased."""
tokens = text_.split()
if not tokens:
return ""
return tokens[0].upper().strip("()").rstrip(";")
def _iter_cte_bodies(masked: str) -> Iterator[str]:
"""Yield the leading keyword of each top-level CTE body in *masked*.
Matches nested inside a body already consumed are skipped, so a subquery
that itself contains ``AS (`` does not shift the outer parse.
"""
consumed_until = 0
for m in _iter_as_paren_matches(masked):
if m.start() < consumed_until:
continue
consumed_until = _find_matching_close_paren(masked, m.end())
yield _first_keyword(masked[m.end() :])
def _detect_writable_cte(stmt: str) -> str | None:
"""Return the first non-read-only keyword opening a CTE body, or None.
Kept for backwards compatibility with callers that only need a yes/no
answer; :func:`_validate_cte_statement` is the enforcing path.
"""
masked = _mask_inert_spans(stmt)
for lead in _iter_cte_bodies(masked):
if lead and lead not in _CTE_READ_ONLY_LEADS:
return lead
return None
def _find_matching_close_paren(masked: str, start: int) -> int:
"""Find the matching close paren in an already-masked statement."""
depth = 1
i = start
while i < len(stmt) and depth > 0:
ch = stmt[i]
if ch == "'":
i = _skip_string_literal(stmt, i)
continue
while i < len(masked) and depth > 0:
ch = masked[i]
if ch == "(":
depth += 1
elif ch == ")":
@@ -153,14 +270,21 @@ def _extract_main_query_after_cte(stmt: str) -> str | None:
For ``WITH cte AS (SELECT 1) DELETE FROM users``, returns ``DELETE FROM users``.
Returns None if no main query is found after the last CTE body.
Handles parentheses inside string literals (e.g., ``SELECT '(' FROM t``).
"""
masked = _mask_inert_spans(stmt)
return _extract_main_query_from_masked(masked)
def _extract_main_query_from_masked(masked: str) -> str | None:
"""Same as :func:`_extract_main_query_after_cte` for pre-masked input."""
last_cte_end = 0
for m in _iter_as_paren_matches(stmt):
last_cte_end = _find_matching_close_paren(stmt, m.end())
for m in _iter_as_paren_matches(masked):
if m.start() < last_cte_end:
continue
last_cte_end = _find_matching_close_paren(masked, m.end())
if last_cte_end > 0:
remainder = stmt[last_cte_end:].strip().lstrip(",").strip()
remainder = masked[last_cte_end:].strip().lstrip(",").strip()
if remainder:
return remainder
return None
@@ -170,9 +294,14 @@ def _resolve_explain_command(stmt: str) -> str | None:
"""Resolve the underlying command from an EXPLAIN [ANALYZE] [VERBOSE] statement.
Returns the real command (e.g., 'DELETE') if ANALYZE is present, else None.
Handles both space-separated and parenthesized syntax.
Handles both space-separated and parenthesized syntax. Comments are masked
first, so ``EXPLAIN /*x*/ ANALYZE DELETE …`` resolves to ``DELETE`` rather
than stalling on the comment token.
"""
rest = stmt.strip()[len("EXPLAIN") :].strip()
masked = _mask_inert_spans(stmt).strip()
if not masked.upper().startswith("EXPLAIN"):
return None
rest = masked[len("EXPLAIN") :].strip()
if not rest:
return None
@@ -188,16 +317,21 @@ def _resolve_explain_command(stmt: str) -> str | None:
)
rest = rest[close + 1 :].strip()
else:
while rest:
first_opt = rest.split()[0].upper().rstrip(";") if rest.split() else ""
if first_opt in ("ANALYZE", "ANALYSE"):
# Consume option tokens one at a time. Slicing by token *length* would
# desynchronise whenever the raw token differs from its normalised form.
tokens = rest.split()
consumed = 0
for token in tokens:
normalised = token.upper().rstrip(";")
if normalised in ("ANALYZE", "ANALYSE"):
analyze_found = True
if first_opt not in explain_opts:
if normalised not in explain_opts:
break
rest = rest[len(first_opt) :].strip()
consumed += 1
rest = " ".join(tokens[consumed:])
if analyze_found and rest:
return rest.split()[0].upper().rstrip(";")
return _first_keyword(rest)
return None
@@ -217,9 +351,25 @@ class NL2SQLTool(BaseTool):
blocked unless ``allow_dml=True`` is set explicitly or the environment
variable ``CREWAI_NL2SQL_ALLOW_DML=true`` is present.
Writable CTEs (``WITH d AS (DELETE …) SELECT …``) and
``EXPLAIN ANALYZE <write-stmt>`` are treated as write operations and are
blocked in read-only mode.
Writable CTEs (``WITH d AS (DELETE …) SELECT …``, including the
``AS [NOT] MATERIALIZED`` spelling) and ``EXPLAIN ANALYZE <write-stmt>`` are
treated as write operations and are blocked in read-only mode. Statements
are analysed with strings and comments masked out, so neither a keyword
hidden in a literal nor a comment inserted between keywords changes the
verdict, and a ``WITH`` statement that cannot be parsed is rejected rather
than allowed.
In read-only mode the transaction is additionally marked
``SET TRANSACTION READ ONLY`` where the backend supports it, so enforcement
does not rest on statement parsing alone.
.. warning::
Keyword validation cannot fully express "read-only": a SELECT can still
reach the database server's filesystem (``INTO OUTFILE``,
``pg_read_file``) or invoke a side-effecting function. The known sinks
are blocked explicitly, but the only complete control is to point
``db_uri`` at a **least-privileged, read-only database role**. Treat the
checks in this class as defence in depth, not as a substitute.
The ``_fetch_all_available_columns`` helper uses parameterised queries so
that table names coming from the database catalogue cannot be used as an
@@ -299,7 +449,7 @@ class NL2SQLTool(BaseTool):
style bypasses. When ``allow_dml=True`` every statement is checked and
a warning is emitted for write operations.
"""
statements = [s.strip() for s in sql_query.split(";") if s.strip()]
statements = _split_statements(sql_query)
if not statements:
raise ValueError("NL2SQLTool received an empty SQL query.")
@@ -315,13 +465,18 @@ class NL2SQLTool(BaseTool):
def _validate_statement(self, stmt: str) -> None:
"""Validate a single SQL statement (no semicolons)."""
masked = _mask_inert_spans(stmt)
command = self._extract_command(stmt)
# Some writes are reachable from a statement whose first keyword is
# SELECT, so they are invisible to the command check below and survive a
# transaction-level rollback. Check them before anything else.
self._reject_select_level_side_effects(masked)
# EXPLAIN ANALYZE / EXPLAIN ANALYSE actually *executes* the underlying
# query. Resolve the real command so write operations are caught.
# parenthesized ("EXPLAIN (ANALYZE) DELETE …", "EXPLAIN (ANALYZE, VERBOSE) DELETE …").
# EXPLAIN ANALYZE actually executes the underlying query — resolve the
# real command so write operations are caught.
# query, in both the space-separated and parenthesized spellings
# ("EXPLAIN (ANALYZE) DELETE …"). Resolve the real command so write
# operations are caught.
if command == "EXPLAIN":
resolved = _resolve_explain_command(stmt)
if resolved:
@@ -329,44 +484,7 @@ class NL2SQLTool(BaseTool):
# (e.g. WITH d AS (DELETE …) SELECT …) must be blocked in read-only mode.
if command == "WITH":
write_found = _detect_writable_cte(stmt)
if write_found:
found = write_found
if not self.allow_dml:
raise ValueError(
f"NL2SQLTool is configured in read-only mode and blocked a "
f"writable CTE containing a '{found}' statement. To allow "
f"write operations set allow_dml=True or "
f"CREWAI_NL2SQL_ALLOW_DML=true."
)
logger.warning(
"NL2SQLTool: executing writable CTE with '%s' because allow_dml=True.",
found,
)
return
main_query = _extract_main_query_after_cte(stmt)
if main_query:
main_cmd = main_query.split()[0].upper().rstrip(";")
if main_cmd in _WRITE_COMMANDS:
if not self.allow_dml:
raise ValueError(
f"NL2SQLTool is configured in read-only mode and blocked a "
f"'{main_cmd}' statement after a CTE. To allow write "
f"operations set allow_dml=True or "
f"CREWAI_NL2SQL_ALLOW_DML=true."
)
logger.warning(
"NL2SQLTool: executing '%s' after CTE because allow_dml=True.",
main_cmd,
)
elif main_cmd not in _READ_ONLY_COMMANDS:
if not self.allow_dml:
raise ValueError(
f"NL2SQLTool blocked an unrecognised SQL command '{main_cmd}' "
f"after a CTE. Only {sorted(_READ_ONLY_COMMANDS)} are allowed "
f"in read-only mode."
)
self._validate_cte_statement(masked)
return
if command in _WRITE_COMMANDS:
@@ -389,12 +507,121 @@ class NL2SQLTool(BaseTool):
f"mode."
)
def _reject_select_level_side_effects(self, masked: str) -> None:
"""Block writes and server-file access that a SELECT can reach.
``SELECT … INTO OUTFILE`` writes a file on the database server, and
functions like ``pg_read_file`` or ``dblink_exec`` read the server's
filesystem or open a connection outside the current transaction. None of
these are undone by a rollback, and all of them present as a read-only
first keyword, so they need an explicit check.
These checks are a backstop, not the primary control: only a
least-privileged database role can properly bound what the tool reaches.
Args:
masked: The statement with strings and comments already masked.
Raises:
ValueError: If a file sink or server-file function is present and
``allow_dml`` is False.
"""
if self.allow_dml:
return
if _FILE_SINK_RE.search(masked):
raise ValueError(
"NL2SQLTool is configured in read-only mode and blocked a query "
"writing to the database server's filesystem (INTO OUTFILE / "
"INTO DUMPFILE). Grant the tool a read-only database role rather "
"than enabling allow_dml."
)
if match := _SERVER_FILE_FUNC_RE.search(masked):
raise ValueError(
f"NL2SQLTool is configured in read-only mode and blocked a call to "
f"'{match.group(0).rstrip('( ')}', which reaches the database "
f"server's filesystem or opens a connection outside the current "
f"transaction. Grant the tool a read-only database role rather "
f"than enabling allow_dml."
)
def _validate_cte_statement(self, masked: str) -> None:
"""Validate a statement whose first keyword is ``WITH``.
Fails closed: a ``WITH`` statement whose CTE bodies cannot be located, or
which has no query after them, is rejected in read-only mode rather than
passed through unchecked.
Args:
masked: The statement with strings and comments already masked.
Raises:
ValueError: If the statement writes, or cannot be parsed, while
``allow_dml`` is False.
"""
leads = list(_iter_cte_bodies(masked))
if not leads:
if not self.allow_dml:
raise ValueError(
"NL2SQLTool blocked a WITH statement whose CTE definitions "
"could not be parsed, so it cannot be confirmed read-only. "
"To allow write operations set allow_dml=True or "
"CREWAI_NL2SQL_ALLOW_DML=true."
)
return
for lead in leads:
if lead and lead not in _CTE_READ_ONLY_LEADS:
if not self.allow_dml:
raise ValueError(
f"NL2SQLTool is configured in read-only mode and blocked a "
f"writable CTE containing a '{lead}' statement. To allow "
f"write operations set allow_dml=True or "
f"CREWAI_NL2SQL_ALLOW_DML=true."
)
logger.warning(
"NL2SQLTool: executing writable CTE with '%s' because allow_dml=True.",
lead,
)
return
main_query = _extract_main_query_from_masked(masked)
if main_query is None:
if not self.allow_dml:
raise ValueError(
"NL2SQLTool blocked a WITH statement with no query after its "
"CTE definitions, so it cannot be confirmed read-only. To "
"allow write operations set allow_dml=True or "
"CREWAI_NL2SQL_ALLOW_DML=true."
)
return
main_cmd = _first_keyword(main_query)
if main_cmd in _WRITE_COMMANDS:
if not self.allow_dml:
raise ValueError(
f"NL2SQLTool is configured in read-only mode and blocked a "
f"'{main_cmd}' statement after a CTE. To allow write "
f"operations set allow_dml=True or "
f"CREWAI_NL2SQL_ALLOW_DML=true."
)
logger.warning(
"NL2SQLTool: executing '%s' after CTE because allow_dml=True.",
main_cmd,
)
elif main_cmd not in _READ_ONLY_COMMANDS and not self.allow_dml:
raise ValueError(
f"NL2SQLTool blocked an unrecognised SQL command '{main_cmd}' "
f"after a CTE. Only {sorted(_READ_ONLY_COMMANDS)} are allowed "
f"in read-only mode."
)
@staticmethod
def _extract_command(sql_query: str) -> str:
"""Return the uppercased first keyword of *sql_query*."""
stripped = sql_query.strip().lstrip("(")
first_token = stripped.split()[0] if stripped.split() else ""
return first_token.upper().rstrip(";")
return _first_keyword(_mask_inert_spans(sql_query).strip())
# Schema introspection helpers
@@ -458,7 +685,7 @@ class NL2SQLTool(BaseTool):
# Check ALL statements so that e.g. "SELECT 1; DROP TABLE t" triggers a
# commit when allow_dml=True, regardless of statement order.
_stmts = [s.strip() for s in sql_query.split(";") if s.strip()]
_stmts = _split_statements(sql_query)
def _is_write_stmt(s: str) -> bool:
cmd = self._extract_command(s)
@@ -474,7 +701,7 @@ class NL2SQLTool(BaseTool):
return True
main_q = _extract_main_query_after_cte(s)
if main_q:
return main_q.split()[0].upper().rstrip(";") in _WRITE_COMMANDS
return _first_keyword(main_q) in _WRITE_COMMANDS
return False
is_write = any(_is_write_stmt(s) for s in _stmts)
@@ -483,6 +710,9 @@ class NL2SQLTool(BaseTool):
Session = sessionmaker(bind=engine) # noqa: N806
session = Session()
try:
if not self.allow_dml:
self._enforce_read_only_transaction(session)
result = session.execute(text(sql_query), params or {})
if self.allow_dml and is_write:
@@ -501,3 +731,32 @@ class NL2SQLTool(BaseTool):
finally:
session.close()
@staticmethod
def _enforce_read_only_transaction(session: Any) -> None:
"""Ask the backend to enforce read-only for this transaction.
Statement inspection alone cannot guarantee a query is read-only — SQL
is dialect-specific and the parser here is deliberately simple. Marking
the transaction read-only moves enforcement into the database, where
PostgreSQL and MySQL reject writes outright regardless of how the
statement was spelled.
Backends without the syntax (SQLite, SQL Server, Snowflake) raise, in
which case the transaction is rolled back to clear the error state and
keyword validation remains the only control. That is logged rather than
raised so those backends keep working.
Args:
session: The active SQLAlchemy session.
"""
try:
session.execute(text("SET TRANSACTION READ ONLY"))
except Exception as exc:
session.rollback()
logger.debug(
"NL2SQLTool: backend rejected 'SET TRANSACTION READ ONLY' (%s); "
"falling back to statement validation only. A read-only "
"database role is strongly recommended.",
exc,
)

View File

@@ -598,3 +598,190 @@ class TestCTEUnknownCommand:
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="unrecognised"):
tool._validate_query("WITH cte AS (SELECT 1) FOOBAR")
# Regression: read-only bypasses via CTE modifiers, comments and SELECT-level sinks
class TestMaterializedCTE:
"""`AS MATERIALIZED (` must be recognised as a CTE body.
The original `\\bAS\\s*\\(` pattern never matched PostgreSQL's materialisation
modifier, so the statement parsed as having no CTE at all and skipped
validation entirely.
"""
@pytest.mark.parametrize(
"stmt",
[
"WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d",
"WITH d AS NOT MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d",
"WITH d AS materialized (DROP TABLE users) SELECT 1",
"WITH d AS\nMATERIALIZED\n(UPDATE users SET a=1 RETURNING *) SELECT * FROM d",
],
)
def test_writable_materialized_cte_blocked(self, stmt: str):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="read-only mode"):
tool._validate_query(stmt)
def test_read_only_materialized_cte_allowed(self):
tool = _make_tool(allow_dml=False)
tool._validate_query(
"WITH d AS MATERIALIZED (SELECT 1 AS x) SELECT * FROM d"
)
def test_writable_materialized_cte_allowed_when_dml_enabled(self):
tool = _make_tool(allow_dml=True)
tool._validate_query(
"WITH d AS MATERIALIZED (DELETE FROM users RETURNING *) SELECT * FROM d"
)
class TestCTEFailsClosed:
"""A WITH statement that cannot be parsed must be rejected, not passed through."""
def test_with_and_no_parsable_cte_body_blocked(self):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="could not be parsed"):
tool._validate_query("WITH d AS DELETE FROM users")
def test_with_and_no_main_query_blocked(self):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="no query after"):
tool._validate_query("WITH d AS (SELECT 1)")
def test_unrecognised_cte_lead_blocked(self):
"""An unenumerated keyword opening a CTE body is treated as a write."""
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="read-only mode"):
tool._validate_query("WITH d AS (GRANT ALL ON t TO x) SELECT * FROM d")
class TestCommentEvasion:
"""Comments must not hide a write command from the validator."""
@pytest.mark.parametrize(
"stmt",
[
"EXPLAIN /*x*/ ANALYZE DELETE FROM users",
"EXPLAIN /* multi\nline */ ANALYZE DROP TABLE users",
"EXPLAIN --skip\nANALYZE DELETE FROM users",
],
)
def test_comment_before_analyze_still_blocked(self, stmt: str):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="read-only mode"):
tool._validate_query(stmt)
def test_comment_inside_cte_still_blocked(self):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="read-only mode"):
tool._validate_query(
"WITH d AS (/* note */ DELETE FROM users RETURNING *) SELECT * FROM d"
)
def test_mysql_executable_comment_is_not_masked(self):
"""MySQL runs /*! … */ bodies, so a semicolon inside one is a real split."""
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="multi-statement"):
tool._validate_query("SELECT 1 /*! ; DROP TABLE users */")
def test_plain_comment_with_semicolon_is_single_statement(self):
tool = _make_tool(allow_dml=False)
tool._validate_query("SELECT 1 /* ; not a split */ FROM t")
class TestSemicolonInLiteral:
"""Splitting must ignore semicolons inside strings rather than over-reject."""
def test_semicolon_in_string_literal_is_single_statement(self):
tool = _make_tool(allow_dml=False)
tool._validate_query("SELECT ';' AS punctuation")
def test_semicolon_in_dollar_quoted_string_is_single_statement(self):
tool = _make_tool(allow_dml=False)
tool._validate_query("SELECT $$a;b$$ AS x")
def test_write_keyword_inside_literal_is_not_a_write(self):
tool = _make_tool(allow_dml=False)
tool._validate_query("SELECT 'DROP TABLE users' AS harmless_text")
class TestSelectLevelSideEffects:
"""Writes reachable from a SELECT survive rollback, so they need blocking."""
@pytest.mark.parametrize(
"stmt",
[
"SELECT * FROM users INTO OUTFILE '/var/www/html/shell.php'",
"SELECT a FROM t INTO DUMPFILE '/tmp/x'",
],
)
def test_file_sink_blocked_in_read_only(self, stmt: str):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="filesystem"):
tool._validate_query(stmt)
@pytest.mark.parametrize(
"stmt",
[
"SELECT pg_read_file('/etc/passwd')",
"SELECT pg_ls_dir('/')",
"SELECT lo_import('/etc/shadow')",
"SELECT load_file('/etc/passwd')",
"SELECT dblink_exec('dbname=x', 'DELETE FROM users')",
],
)
def test_server_file_functions_blocked_in_read_only(self, stmt: str):
tool = _make_tool(allow_dml=False)
with pytest.raises(ValueError, match="read-only mode"):
tool._validate_query(stmt)
def test_file_sink_allowed_when_dml_enabled(self):
tool = _make_tool(allow_dml=True)
tool._validate_query("SELECT * FROM t INTO OUTFILE '/tmp/x'")
def test_similar_column_name_is_not_a_false_positive(self):
"""`load_files` / a column called `dblink` must not trip the check."""
tool = _make_tool(allow_dml=False)
tool._validate_query("SELECT dblink FROM connections")
tool._validate_query("SELECT into_outfile_count FROM stats")
class TestReadOnlyTransactionBackstop:
"""Read-only mode asks the backend to enforce read-only too."""
def test_read_only_mode_attempts_set_transaction_read_only(self):
tool = _make_tool(allow_dml=False)
original = NL2SQLTool._enforce_read_only_transaction
seen: list[bool] = []
def _spy(session):
seen.append(True)
return original(session)
with patch.object(
NL2SQLTool, "_enforce_read_only_transaction", staticmethod(_spy)
):
tool.execute_sql("SELECT 1 AS val")
assert seen, "read-only mode must mark the transaction read-only"
def test_unsupported_backend_falls_back_without_raising(self):
"""SQLite rejects SET TRANSACTION READ ONLY; the query must still run."""
tool = _make_tool(allow_dml=False)
result = tool.execute_sql("SELECT 1 AS val")
assert result == [{"val": 1}]
def test_dml_mode_does_not_mark_transaction_read_only(self):
tool = _make_tool(allow_dml=True)
seen: list[bool] = []
def _spy(session):
seen.append(True)
with patch.object(NL2SQLTool, "_enforce_read_only_transaction", staticmethod(_spy)):
tool.execute_sql("SELECT 1 AS val")
assert not seen