Added sqlx based postgres implementation - #8
Merged
Merged
Conversation
BtXin
added a commit
that referenced
this pull request
Jul 22, 2026
…ser, tidy validator Second-round review follow-ups (#3–#10): - #4 Encode the trust boundary in the type. Ad-hoc /query now validates against a dedicated AdhocSqlPolicy (access modes + denied schemas); the trusted pipeline path keeps a bare SqlValidatorConfig. Reaching for validate_sql on untrusted input can no longer silently skip the allowlist + schema denial. - #5 Add coverage proving the schema denial descends into indirect relations (set ops, scalar/IN/EXISTS subqueries) via visit_relations; document the residual (operator-defined views/federated aliases, which ad-hoc SQL cannot create) and its structural fix. See follow-up issue. - #6 Parse via datafusion::sql::sqlparser (DataFusion enables the visitor feature); drop the standalone sqlparser dep entirely so validator and engine share one parser by construction — a DF bump is now a compile break, not silent parse divergence. - #8 Single AUTH_SCHEMA const used at both the register and deny sites. - #9 check_denied_schemas reports the table via extract_table_name (quote-stripped), matching WriteNotAllowed. - #10 statement_keyword maps the AST variant to a &'static str instead of Display-rendering the whole statement on each rejection. - #7 AppState::new derives the policy once; removes the copy-pasted Arc::new(validator_config_from_sources(..)) across ~9 sites. - #3 Document the startup-snapshot invariant (no runtime access_mode writer) on the field and the builder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BtXin
added a commit
that referenced
this pull request
Jul 23, 2026
* docs: design spec for POST /query ad-hoc SQL endpoint
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: implementation plan for POST /query endpoint
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(plan): extract shared require_session helper in Task 4
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(validator): block COPY, add single-statement validation with StatementKind
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(engine): add execute_with_limit pushing row cap into the plan
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(server): extract response helpers into response.rs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(server): add POST /query endpoint for ad-hoc SQL
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(server): record metrics on max_rows validation path, guard max_rows overflow
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: fmt/clippy fixes for query endpoint
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(validator): close EXPLAIN/SET statement-policy bypass in /query
EXPLAIN ANALYZE executed its wrapped statement, and SET mutated the
shared SessionContext, both bypassing DDL/COPY/write-access checks.
Recurse into EXPLAIN's inner statement and reject session-mutating SET.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(validator): reject PREPARE/EXECUTE in /query statement policy
PREPARE stored a write/DDL plan on the shared SessionContext and EXECUTE
ran it later, bypassing the per-source access-mode gate. Ad-hoc one-shot
queries have no use for prepared statements — reject both.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(server): address /query review — auth-schema denial, statement allowlist, scoped policy
- Forbid ad-hoc SQL from referencing the auth schema (auth.sessions holds
live bearer tokens); enforced via sqlparser visit_relations over every
relation (FROM/JOIN/subquery/CTE/DML target/EXPLAIN/DESCRIBE, incl.
3-part catalog.auth.table names).
- Invert the ad-hoc statement policy to a strict allowlist (Query,
access-checked DML, EXPLAIN-of-allowed, SHOW/DESCRIBE); unknown or
future sqlparser statement types (MERGE, GRANT, transactions, ...) are
rejected by default.
- Scope the new denials to /query: the pipeline-load path reverts to its
pre-PR policy (DDL blocked, DML access-checked, COPY/SET allowed), so
existing deployments keep booting.
- Skip {param} brace preprocessing for ad-hoc SQL — it rewrote string
literals and rejected valid queries.
- Match write-access checks on the fully qualified table name, honoring a
source's access mode for source.table references.
- Build the validator config once at startup into AppState instead of per
request (no runtime config writer exists).
- Log every ad-hoc statement before execution as an audit trail.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(deps): unify sqlparser at 0.59 to match DataFusion's parser
The validator previously parsed with a standalone sqlparser 0.53 while
DataFusion 52.5 executes with 0.59 — a version skew that could let the
two parsers disagree about a statement's shape. The workspace-root 0.55
entry had no users at all. Now a single workspace dep (0.59, visitor
feature) is shared with DataFusion, so validation and execution always
parse identically.
Migration notes:
- Insert targets are now TableObject; non-table-name targets (table
functions) have no registered name to access-check.
- ObjectName components are ObjectNamePart; identity comparisons use the
bare ident value, which also fixes quoted identifiers slipping past
access-mode and denied-schema checks (INSERT INTO "users",
"auth"."sessions") — covered by a new test.
- 'SELECT FROM users' (empty projection) parses in 0.59; dropped from
the parse-error test, it fails at DataFusion planning instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(validator): close default-qualifier write bypass; fail closed on non-table INSERT targets
Round-2 review fixes:
- check_write_access now strips DataFusion's default catalog/schema
qualifiers before lookup, so INSERT/UPDATE/DELETE against
public.<table> or datafusion.public.<table> honors the same access
mode as the bare name (they all resolve to the same table). A
datafusion.<source>.<table> reference still honors the source's mode.
- INSERT with a non-TableName target (ClickHouse INSERT INTO FUNCTION)
is rejected instead of skipping the access check — unreachable via
GenericDialect today, but the write path must fail closed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(validator): type-split ad-hoc policy, share DataFusion's parser, tidy validator
Second-round review follow-ups (#3–#10):
- #4 Encode the trust boundary in the type. Ad-hoc /query now validates
against a dedicated AdhocSqlPolicy (access modes + denied schemas);
the trusted pipeline path keeps a bare SqlValidatorConfig. Reaching for
validate_sql on untrusted input can no longer silently skip the
allowlist + schema denial.
- #5 Add coverage proving the schema denial descends into indirect
relations (set ops, scalar/IN/EXISTS subqueries) via visit_relations;
document the residual (operator-defined views/federated aliases, which
ad-hoc SQL cannot create) and its structural fix. See follow-up issue.
- #6 Parse via datafusion::sql::sqlparser (DataFusion enables the visitor
feature); drop the standalone sqlparser dep entirely so validator and
engine share one parser by construction — a DF bump is now a compile
break, not silent parse divergence.
- #8 Single AUTH_SCHEMA const used at both the register and deny sites.
- #9 check_denied_schemas reports the table via extract_table_name
(quote-stripped), matching WriteNotAllowed.
- #10 statement_keyword maps the AST variant to a &'static str instead of
Display-rendering the whole statement on each rejection.
- #7 AppState::new derives the policy once; removes the copy-pasted
Arc::new(validator_config_from_sources(..)) across ~9 sites.
- #3 Document the startup-snapshot invariant (no runtime access_mode
writer) on the field and the builder.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(query): fail closed on multi-target DELETE; stop leaking engine internals
Third-round defense-in-depth review:
- Reject DELETE with a non-empty `tables` list or a `USING` clause on the
ad-hoc /query path instead of relying on DataFusion to reject the form at
plan time (only `delete.from` was access-checked before).
- Stop returning raw engine/schema internals in /query error responses:
the execution-error and JSON-conversion-error bodies now carry a generic
message with no `details`, while the full DataFusion error and the
record-batch schema stay in the server-side log.
The auth-catalog isolation suggestion is tracked in #164 (the reviewer
asked for an issue, not a block).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added sqlx based postgres implementation to avoid wrapping around datafusion-table-provider and address the auto-increment column bug #5