msyavuz commented on code in PR #43111:
URL: https://github.com/apache/superset/pull/43111#discussion_r3805703810
##########
superset/security/manager.py:
##########
@@ -1237,7 +1393,15 @@ def query_context_modified(query_context:
"QueryContext") -> bool:
# native filter is allowed to target; other chartless paths keep prior
# behavior (see _native_filter_request_modified).
if stored_chart is None:
- return _native_filter_request_modified(query_context)
+ if _native_filter_request_modified(query_context):
+ return True
+ # Chartless non-native-filter requests (drill-to-detail, drill-by,
+ # samples) must not carry SQL extras; there is no stored chart to
+ # validate them against. Only the empty-filter sentinel is allowed.
+ sentinel_only: set[str] = {_EMPTY_FILTER_SENTINEL}
Review Comment:
The chartless branch now allows only the sentinel, which over-blocks two
real guest paths:
- Native Select filters put their "Pre-filter available values" adhoc
filters into `extras.where` (`filters/components/Select/buildQuery.ts` →
`processFilters`), so a dashboard whose native filter pre-filters with Custom
SQL 403s.
- Drill-to-detail is chartless too (`/datasource/samples` → `get_samples`
builds the context with `form_data={"dashboardId": ...}` and calls
`raise_for_access`), and `getDrillPayload` sends the chart's `extras.where`
plus drill `filters` whose `col` is the raw dimension — an adhoc dict whenever
that dimension is custom SQL.
What should these validate against when there is no stored chart — the
filter's own config, the dashboard's charts?
Nits while here: `_native_filter_request_modified`'s docstring still says
drill-to-detail/samples "are not treated as modified here" and that
`filter`/`extras` are not constrained, both now untrue; `_collect_allowed_sql`
calls `flt.get` on stored `adhoc_filters` without the `isinstance(flt, dict)`
guard used further down; and `sanitizeClause` appends `\n` when an expression
contains `--`, so `(expr\n)` won't match the stored raw `expr` on charts with
no saved `query_context`.
--
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]
