rebenitez1802 commented on PR #40662:
URL: https://github.com/apache/superset/pull/40662#issuecomment-4669458477
My reviewer agent flagged this 2 blockers:
🔴 High — Probe Query pollutes db.session, 500s every RLS estimate
In _apply_sql_security (superset/commands/sql_lab/estimate.py), the RLS
branch builds probe_query = Query(database=self._database, ...) without
client_id and without expunging. client_id is nullable=False with no default
(models/sql_lab.py:130), and database cascades the transient into the session
via backref("queries", cascade="all, delete-orphan") (sql_lab.py:190-193). The
next call — apply_rls → get_predicates_for_table → db.session.query(...)
(utils/rls.py:112) — triggers autoflush, which INSERTs the pending probe and
raises IntegrityError, so any RLS-enabled estimate referencing a table fails.
The unit tests miss it because self._database is a MagicMock and apply_rls is
patched. Fix: mirror security/manager.py:2933-2941 — set
client_id=shortid()[:10], use database_id=self._database.id instead of the
database= relationship, and db.session.expunge(probe_query); cover with an
integration test using a real session and real apply_rls.
🟡 Medium — search_path gate bypassed when an explicit schema is supplied
schema = self._schema or
self._database.get_default_schema_for_query(probe_query, ...) short-circuits,
so get_default_schema_for_query is skipped whenever the caller passes a
non-empty schema — but that method (db_engine_specs/postgres.py:604-630) is
where the per-query gate rejecting SET search_path = ... lives, and the
executor calls it unconditionally (sql_lab.py:449-452). An estimate with
explicit schema + a SET search_path statement bypasses the gate the executor
enforces, contradicting the PR’s own docstring;
test_apply_sql_security_respects_explicit_catalog_schema even asserts
get_default_schema_for_query.assert_not_called(), locking the bypass in. Fix:
always call get_default_schema_for_query (run the gate), then pick self._schema
or resolved or "".
--
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]
