fix: validate clickhouse queries#180
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 467443a381
ℹ️ 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".
| /// ClickHouse query errors (syntax, missing table, etc.) are returned | ||
| /// immediately. | ||
| pub async fn query(&self, sql: &str, signatures: &[&str]) -> Result<QueryResult> { | ||
| self.query_with_timeout(sql, signatures, 30_000).await |
There was a problem hiding this comment.
Changing ClickHouseEngine::query to always call query_with_timeout(..., 30_000) applies a 30s cap to every existing internal caller, not just public /query traffic. In this repo I checked src/api/views.rs, where create_view performs INSERT INTO ... SELECT ... backfills via clickhouse.query(...); those backfills can legitimately run longer than 30s and will now fail with timeouts, breaking view creation on non-trivial datasets. The timeout should be enforced only on the public user-query path (query_user) or be configurable per call site.
Useful? React with 👍 / 👎.
brendanjryan
force-pushed
the
codex/clickhouse-query-safety
branch
from
364bddc to
22cadb9
Compare
brendanjryan
force-pushed
the
codex/clickhouse-query-safety
branch
from
63aad55 to
0eab9fa
Compare
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>
1 participant
Summary
Validate ClickHouse-backed public query and view SQL before execution, block unsafe ClickHouse tables and functions, apply request and execution timeouts only on public user-query paths, and include a changelog entry.