eugeneo17 opened a new pull request, #43090:
URL: https://github.com/apache/superset/pull/43090
SUMMARY
Addresses #33346.
Row-Level Security is enforced on the SQL Lab query path but bypassed when a
chart is created from that same ad-hoc SQL, silently rendering unfiltered data
(the issue reproduces with 3 filtered rows in SQL Lab vs. 125 unfiltered in the
resulting chart). In embedded / multi-tenant deployments, where RLS is the
tenant boundary, this is a cross-tenant disclosure, and it is silent — neither
the restricted analyst nor an admin can detect the over-exposure from the UI.
The root cause is path divergence: charts built from SQL Lab use a Query
datasource whose is_rls_supported is False and whose
get_sqla_row_level_filters() was a stub returning []
(superset/models/helpers.py), and the RLS cache key was skipped for Query
datasources (superset/security/manager.py). Superset already has two separate
RLS mechanisms (SQL-rewriting in SQL Lab; WHERE-injection for charts) and the
chart-from-SQL path fell between them.
This PR converges enforcement to a single, fail-closed, server-side gate
(superset/security/rls_enforcement.py) that every ad-hoc-SQL datasource render
is routed through. It rewrites the ad-hoc SQL over the referenced RLS-governed
tables at query-build time (superset/utils/rls.py), so enforcement is
path-independent; fails closed with a fixed, non-disclosive message when RLS
cannot be safely resolved (the sensitive denial reason stays server-side);
fixes the RLS cache-key identity so filtered and unfiltered results can no
longer collide; records queryable enforcement evidence (new model + REST API +
DB migration + CLI); and surfaces enforcement in the UI (an "RLS active" badge
and a dedicated RLS error message). Behaviour is unchanged when no RLS applies.
WORTH NOTING
- Column-level security is out of scope by design. The gate is CLS-ready,
but the masking seam intentionally raises NotImplementedError("Column masking
is not yet supported") — row-level enforcement is complete; column masking is a
documented future seam.
- The evidence tamper-evidence hash chain is opt-in / off by default
(RLS_EVIDENCE_HASH_CHAIN_ENABLED = False). Evidence rows are always recorded;
the cross-row integrity chain is only populated when enabled.
- The evidence migration's down_revision targets the branch base. Before
merge it must be rebased onto current master and re-pointed to the current
single migration head, or check-db-migration-confict will flag it.
- I couldn't exercise the full suite / a live DB across engines here —
upgrade & downgrade of the new table and the integration tests need a CI run.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A for the enforcement change (backend security fix). The UI adds an "RLS
active" badge on charts backed by ad-hoc SQL over governed tables.
TESTING INSTRUCTIONS
pytest tests/unit_tests/security/test_rls_enforcement.py \
tests/unit_tests/security/test_fail_closed_decision.py \
tests/unit_tests/security/test_rls_cache_key_identity.py \
tests/unit_tests/models/test_helpers_rls_gate.py
pytest tests/integration_tests/security/rls_cache_isolation_tests.py \
tests/integration_tests/security/row_level_security_tests.py
New tests cover the guest/embedded variant, cache isolation, the fail-closed
decision, non-disclosive denial, and a zero-impact baseline. Before merge:
reproduce #33346 end-to-end (RLS rule → restricted user → SQL Lab filtered →
Create chart → chart shows the same filtered rows, not the full set).
--
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]
