gabotorresruiz commented on code in PR #45552:
URL: https://github.com/apache/superset/pull/45552#discussion_r4338680387
##########
superset/connectors/sqla/models.py:
##########
@@ -708,6 +911,32 @@ def data_for_slices( # pylint: disable=too-many-locals #
noqa: C901
]
column_names.update(_columns)
+ # Full-access metadata retains hidden controls and query-only
+ # dependencies; only active saved controls authorize chart labels.
+ display_columns, display_metrics = self._form_display_references(
+ form_data,
+ slc.viz_type or
self._saved_display_form_data(slc).get("viz_type"),
+ )
+ query_columns, query_metrics = self._query_output_references(
+ slc.query_context
+ )
+ display_columns.extend(query_columns)
+ display_metrics.extend(query_metrics)
+ for references, names in (
+ (display_columns, column_names),
+ (display_metrics, metric_names),
+ ):
+ for reference in references:
+ if isinstance(reference, str):
+ names.add(reference)
+ elif label := _authored_display_label(reference):
+ authored_labels.setdefault(label, label)
Review Comment:
`display_columns` and `display_metrics` get the query context references
appended just above, so an adhoc label authored in a chart's stored
`query_context` becomes a `verbose_map` key for every full access caller. That
stored context is writable by a permitted non-editor through the relaxed
editorship branch in `UpdateChartCommand.validate`, which your own
`..._ignore_viewer_query_context_update` test exercises, so those keys are
caller chosen strings with no bound on how many. Before this change
`query_context` could only contribute names that matched real dataset columns.
It does look inert: real `verbose_map` entries win the merge on line 976,
and the restricted map is unaffected because `_slice_display_references` never
reads the context. But do the context's authored labels need to be in
`verbose_map` at all? The restricted side only needs the ones from the saved
form data, which `_form_display_references` already supplies, so collecting
`authored_labels` before the two `extend` calls would keep the feature and drop
the writable input. Or is there a case where the context is the only source of
that label?
--
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]
