rusackas commented on code in PR #43391:
URL: https://github.com/apache/superset/pull/43391#discussion_r3994318374
##########
superset/security/manager.py:
##########
@@ -1615,6 +1662,146 @@ def _columns_metrics_modified(
return False
+def _annotation_layer_identity(layer: Any) -> Optional[tuple[str, str]]:
+ """
+ Identity of an annotation layer for tamper comparison: the source type and
+ the underlying source it reads (a native annotation-layer id or a chart
+ id). Cosmetic keys (``name``, styling, overrides) are not part of the
+ identity. Returns ``None`` for a malformed (non-dict) layer.
+ """
+ if not isinstance(layer, dict):
+ return None
+ return (
+ freeze_value(layer.get("sourceType")),
+ freeze_value(layer.get("value")),
+ )
+
+
+def _annotation_layers_modified(
+ query_context: "QueryContext",
+ form_data: dict[str, Any],
+ stored_chart: "Slice",
+ stored_query_context: Optional[dict[str, Any]],
+) -> bool:
+ """
+ Whether the request references annotation layers the stored chart does
+ not already carry.
+
+ ``annotation_layers`` is accepted on any query object, and native layers
+ resolve every annotation of each referenced layer id with no further
+ access check, so a guest injecting a layer the chart was not saved with
+ would read data that was never shared with them. Replaying the chart's
+ own stored layers is not tampering.
+ """
+ requested: set[Optional[tuple[str, str]]] = {
+ _annotation_layer_identity(layer)
+ for layer in form_data.get("annotation_layers") or []
+ }
+ requested.update(
+ _annotation_layer_identity(layer)
+ for query in query_context.queries
+ for layer in getattr(query, "annotation_layers", None) or []
+ )
+ if not requested:
+ return False
+ # A malformed (non-dict) layer is nothing the frontend produces from a
+ # stored chart; treat it as tampering rather than crashing on it later.
+ if None in requested:
+ return True
+
+ stored: set[Optional[tuple[str, str]]] = {
+ _annotation_layer_identity(layer)
+ for layer in stored_chart.params_dict.get("annotation_layers") or []
+ }
+ if stored_query_context:
Review Comment:
Good catch, fixed in f23dc32. Now checks the layer against the chart's
current `params` only, since a params-only update can leave a stale layer
sitting in the cached `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]
