Skip to content

fix: harden sql validator traversal#179

Merged
brendanjryan merged 2 commits intomainfrom
codex/sql-validator-hardening
May 4, 2026
Merged

fix: harden sql validator traversal#179
brendanjryan merged 2 commits intomainfrom
codex/sql-validator-hardening

Conversation

@brendanjryan
Copy link
Copy Markdown
Contributor

@brendanjryan brendanjryan commented May 3, 2026

Summary

Harden SQL query validation by traversing CTEs, set operations, table modifiers, window clauses, and function arguments more completely. Add regression coverage and a changelog entry.

@brendanjryan brendanjryan marked this pull request as ready for review May 3, 2026 20:53
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ecea935ba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/query/validator.rs
Comment on lines +29 to +33
if regex_lite::Regex::new(r"(?i)\blimit\s+all\b")
.expect("valid LIMIT ALL regex")
.is_match(sql)
{
return Err(anyhow!("LIMIT ALL is not allowed"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove pre-parse LIMIT ALL regex check

The new raw regex gate rejects any SQL text containing the phrase limit all before parsing, which causes false positives in otherwise safe queries (for example, string literals or comments like SELECT 'limit all' FROM blocks). This is a regression in validator correctness because these queries do not use a LIMIT ALL clause, but now fail validation solely due to text matching.

Useful? React with 👍 / 👎.

@brendanjryan brendanjryan force-pushed the codex/sql-validator-hardening branch from 3431dd4 to 0e7efe5 Compare May 4, 2026 00:35
@brendanjryan brendanjryan merged commit daf3f52 into main May 4, 2026
7 checks passed
@brendanjryan brendanjryan deleted the codex/sql-validator-hardening branch May 4, 2026 01:13
brendanjryan added a commit that referenced this pull request May 4, 2026
This PR was opened by the Changelogs release workflow.

When you're ready to release, merge this PR and the packages will be
published.

---

## `tidx@0.5.5`

### Patch Changes

- Harden PostgreSQL SQL validation by fixing CTE scope handling,
schema-qualified table checks, recursive depth accounting, LIMIT ALL
rejection, and traversal of previously unchecked AST clauses. (by
@brendanryan, [#179](#179))
- Validate public ClickHouse queries, block system catalogs and
dangerous table functions, enforce ClickHouse request timeouts, and
validate view SELECT SQL before execution. (by @brendanryan,
[#180](#180))
- Bound PostgreSQL query result processing by streaming rows with a hard
request limit and appending automatic LIMIT clauses on a separate line.
(by @brendanryan, [#181](#181))
- Hardened view administration by failing closed for trusted CIDR
checks, rejecting malformed CIDR configuration, hot-reloading active
trusted CIDRs, and requiring an explicit admin mutation header. (by
@brendanryan, [#182](#182))

Co-authored-by: brendanjryan <1572504+brendanjryan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant