sha174n commented on code in PR #40409:
URL: https://github.com/apache/superset/pull/40409#discussion_r3327371871
##########
tests/unit_tests/sql/parse_tests.py:
##########
@@ -1164,6 +1164,29 @@ def test_has_mutation(engine: str, sql: str, expected:
bool) -> None:
assert SQLScript(sql, engine).has_mutation() == expected
[email protected](
+ "engine, sql, expected",
+ [
+ # Plain SELECT parses to a proper AST node.
+ ("postgresql", "SELECT * FROM foo", False),
+ # CALL parses to ``exp.Command`` on Postgres.
+ ("postgresql", "CALL my_proc(1);", True),
+ # A script that mixes a parseable statement with an unparseable one
+ # is still flagged so strict scoping can refuse the whole script.
+ ("postgresql", "SELECT 1; CALL my_proc();", True),
+ # Kusto KQL statements aren't ``SQLStatement`` instances and must be
+ # skipped, not falsely flagged.
+ ("kustokql", "print 1", False),
Review Comment:
Fixed in 6714ebfdf1: `has_unparseable_statement` now returns True for
non-SQLStatement instances (Kusto KQL et al.), and this test expects True for
the kustokql case so strict scoping fails closed.
##########
superset/security/manager.py:
##########
@@ -2928,35 +2938,82 @@ def raise_for_access( # noqa: C901
cast(Query, query),
template_params,
)
+ parse_result = process_jinja_sql(query.sql, database,
template_params)
+ # Under strict scoping, refuse any statement the parser
+ # could not fully model (sqlglot exp.Command). Such
+ # statements may reference tables via dynamic SQL or
+ # vendor syntax that extract_tables_from_statement cannot
+ # see, so the dataset-match check below would be blind to
+ # them. Fail closed.
+ if (
+ force_dataset_match
+ and parse_result.script.has_unparseable_statement
+ ):
+ raise SupersetSecurityException(
+ SupersetError(
+
error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
+ message=_(
+ "SQL Lab cannot authorise a statement that "
+ "could not be fully parsed. Qualify tables "
+ "explicitly and avoid dynamic SQL inside "
+ "stored-procedure or vendor-specific calls."
+ ),
+ level=ErrorLevel.ERROR,
+ )
+ )
Review Comment:
Fixed in 6714ebfdf1: `has_unparseable_statement` now flags non-sqlglot
engines (e.g. Kusto KQL) as unparseable, so the strict-mode guard at this site
denies them. Added a KQL regression test covering this path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
