Skip to content

feat: add validate-sql command to compile query SQL against the data lake - #808

Open
arielkr256 wants to merge 3 commits into
mainfrom
feat/explain-sql-validation
Open

feat: add validate-sql command to compile query SQL against the data lake#808
arielkr256 wants to merge 3 commits into
mainfrom
feat/explain-sql-validation

Conversation

@arielkr256

Copy link
Copy Markdown
Contributor

Summary

Adds a validate-sql command that validates the SQL of scheduled queries, saved queries, and SQL-based lookup tables by compiling them against the Panther data lake. Each query is executed with an EXPLAIN prefix via the executeDataLakeQuery / dataLakeQuery public API operations — Snowflake compiles the query without scanning any data (0 bytes scanned), surfacing syntax errors and invalid table/column references that static analysis (sqlfluff) misses.

Closes #639

panther_analysis_tool validate-sql --path . --api-token ... --api-host ...

Behavior

  • Validated: scheduled_query / saved_query specs (Query or SnowflakeQuery) and lookup_table specs with a Query field. Supports the standard --filter / --ignore-files options.
  • Missing tables (log source not onboarded in the instance):
    • Enabled queries fail — a scheduled query that can't resolve its table is broken in that instance.
    • Disabled queries are skipped (syntax errors still fail).
    • --skip-missing-tables extends the skip to enabled queries, for repos that ship content for log sources the validating instance doesn't have.
  • Skipped: Jinja macro library files ({% macro %}-only SQL, which expands to nothing standalone; queries that use macros validate fine since the backend expands templates server-side) and investigation templates with <PLACEHOLDER> markers.

Live testing

Ran against panther-analysis (106 queries) on the threat-research instance (~3.5 min):

Implementation notes

  • New GraphQL operations follow the existing pattern: .graphql files + PublicAPIRequests loaders + typed params/response dataclasses + Client abstract methods. LambdaClient raises (unsupported), consistent with other public-API-only features.
  • Validation polls dataLakeQuery until a terminal status with a 60s per-query timeout.
  • Also excludes venv from black/isort so make fmt doesn't reformat an in-project venv (separate commit).

🤖 Generated with Claude Code

arielkr256 and others added 2 commits June 12, 2026 14:09
`make fmt` runs black/isort on the whole repo and was reformatting an
in-project venv's site-packages.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lake

Adds a `validate-sql` command that validates the SQL of scheduled
queries, saved queries, and SQL-based lookup tables by executing them
with an EXPLAIN prefix via the executeDataLakeQuery / dataLakeQuery
public API operations. EXPLAIN compiles the query in Snowflake without
scanning any data, surfacing syntax errors and invalid table/column
references that static analysis misses.

Queries referencing tables that don't exist in the instance (e.g. log
sources that aren't onboarded) fail validation when the query is
enabled, are skipped when the query is disabled, and can be skipped
unconditionally with --skip-missing-tables. Jinja macro library files
and investigation templates with <PLACEHOLDER> markers are skipped.

Closes #639

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arielkr256
arielkr256 requested review from a team as code owners June 12, 2026 20:10
@cursor

cursor Bot commented Jun 12, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
New remote SQL execution path via API (mitigated by EXPLAIN-only validation and API-token gating); behavior depends on instance data lake support and table availability.

Overview
Adds a validate-sql CLI that compile-checks SQL in scheduled/saved queries and SQL lookup tables against the connected Panther data lake (API token only), using EXPLAIN so Snowflake validates syntax and references without scanning data.

The backend gains executeDataLakeQuery / dataLakeQuery GraphQL support on PublicAPIClient (with typed params/responses on Client; Lambda client reports unsupported). The command walks local analysis specs with existing --filter / --ignore-files, skips macro-only files, PantherFlow pragmas, and <PLACEHOLDER> templates, and polls each job (60s timeout). Missing-table errors fail enabled queries by default; disabled ones are skipped, with --skip-missing-tables to relax that for enabled queries.

Also excludes venv from black/isort. Unit tests cover target collection, EXPLAIN wrapping, polling, and skip/fail behavior.

Reviewed by Cursor Bugbot for commit 9c43e3f. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c84be6a. Configure here.

try:
status_res = backend.get_data_lake_query(GetDataLakeQueryParams(id=execute_res.data.id))
except BaseException as err: # pylint: disable=broad-except
return str(err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Interrupt signals treated as SQL errors

Medium Severity

_validate_target catches BaseException around API calls, so KeyboardInterrupt and similar signals are converted into a per-query error string instead of aborting the command. A long validate-sql run may keep going after Ctrl+C and report a bogus FAIL for the current query.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c84be6a. Configure here.

status_code=200,
data=GetDataLakeQueryResponse(
status=data.get("status", ""),
message=data.get("message", ""),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Null GraphQL fields crash parsing

Medium Severity

New data-lake helpers use res.data.get("executeDataLakeQuery", {}) and res.data.get("dataLakeQuery", {}). When GraphQL returns those fields as JSON null, .get yields None and the following .get("id") / .get("status") calls raise AttributeError instead of a controlled validation failure.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c84be6a. Configure here.

The backend routes queries with a `-- pragma: pantherflow` comment to
the PantherFlow engine, which ignores the EXPLAIN prefix and executes
the query for real. Skip them until the API exposes a compile-only
validation path for PantherFlow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@dekatzenel dekatzenel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor comments look worth fixing, plus a couple of other notes. Can you add screenshots of the success and failure output to the top-level comment?


if error is None:
print(f" {cli_output.success('PASS')} {target.analysis_id}")
elif _MISSING_TABLE_ERROR in error and (args.skip_missing_tables or not target.enabled):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The missing-table heuristic over-matches and silently hides real bugs.
_MISSING_TABLE_ERROR = "does not exist or not authorized" is a substring match, but Snowflake uses that exact phrasing for functions, views, stages, schemas — not just tables (e.g. Function 'FOO' does not exist or not authorized, Object 'DB.SCHEMA' does not exist or not authorized). So a query with a typo'd UDF name, a dropped view, or a wrong schema gets classified as a "missing table" and SKIPPED whenever the query is disabled (default) or --skip-missing-tables is set — instead of FAILing.

continue

placeholder = _PLACEHOLDER_PATTERN.search(sql)
if placeholder:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these be counted in the summary?

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.

Add SQL Validation for Scheduled Queries Using Panther Backend

2 participants