(superset) 04/05: fix(reports): scope execution-log API reads to editable schedules

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/tag-report-log-dataset-schema-checks
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 816b98f876455b9eb083a5a888ac21b9eac9e26d
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 00:48:26 2026 -0700

    fix(reports): scope execution-log API reads to editable schedules
    
    ReportExecutionLogRestApi declared no base_filters, so the only
    scoping on its list/item routes was the caller-chosen schedule pk
    folded into the rison filters -- any role with generic ReportSchedule
    read could iterate every schedule's logs, including alert result
    values and database error messages for schedules it doesn't own. Add
    ReportExecutionLogFilter, scoping directly on
    ReportExecutionLog.report_schedule_id (mirroring
    ReportScheduleFilter on the sibling schedule API), and apply it as a
    base filter on both routes.
---
 superset/reports/filters.py               | 22 +++++++++++++++-
 superset/reports/logs/api.py              |  8 ++++++
 tests/unit_tests/reports/filters_test.py  | 42 +++++++++++++++++++++++++++++++
 tests/unit_tests/reports/logs_api_test.py | 37 +++++++++++++++++++++++++++
 4 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/superset/reports/filters.py b/superset/reports/filters.py
index ad08a4a234a..21209312df6 100644
--- a/superset/reports/filters.py
+++ b/superset/reports/filters.py
@@ -22,7 +22,7 @@ from sqlalchemy.orm.query import Query
 
 from superset import db, security_manager
 from superset.daos.base import _escape_like
-from superset.reports.models import ReportSchedule
+from superset.reports.models import ReportExecutionLog, ReportSchedule
 from superset.subjects.filters import subject_relation_exists_for_current_user
 from superset.views.base import BaseFilter
 
@@ -43,6 +43,26 @@ class ReportScheduleFilter(BaseFilter):  # pylint: disable=too-few-public-method
         return query.filter(ReportSchedule.id.in_(editor_ids_query))
 
 
+class ReportExecutionLogFilter(BaseFilter):  # pylint: disable=too-few-public-methods
+    """
+    Scope execution logs to report schedules the user can edit, mirroring
+    ``ReportScheduleFilter`` on the schedule API. Logs carry evaluated alert
+    values and database error messages, so they must not be readable across
+    ownership boundaries via an attacker-chosen schedule id.
+    """
+
+    def apply(self, query: Query, value: Any) -> Query:
+        if security_manager.can_access_all_datasources():
+            return query
+
+        from superset.subjects.models import report_schedule_editors
+
+        editor_ids_query = db.session.query(
+            report_schedule_editors.c.report_schedule_id
+        ).filter(subject_relation_exists_for_current_user(report_schedule_editors))
+        return query.filter(ReportExecutionLog.report_schedule_id.in_(editor_ids_query))
+
+
 class ReportScheduleAllTextFilter(BaseFilter):  # pylint: disable=too-few-public-methods
     name = _("All Text")
     arg_name = "report_all_text"
diff --git a/superset/reports/logs/api.py b/superset/reports/logs/api.py
index 9a2cea964e3..96673d1ef3d 100644
--- a/superset/reports/logs/api.py
+++ b/superset/reports/logs/api.py
@@ -31,6 +31,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface
 
 from superset import is_feature_enabled
 from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.reports.filters import ReportExecutionLogFilter
 from superset.reports.logs.schemas import openapi_spec_methods_override
 from superset.reports.models import ReportExecutionLog
 from superset.views.base_api import BaseSupersetModelRestApi
@@ -53,6 +54,13 @@ class ReportExecutionLogRestApi(BaseSupersetModelRestApi):
     class_permission_name = "ReportSchedule"
     resource_name = "report"
     allow_browser_login = True
+    # Ownership scoping. Without a base filter the only scoping on these
+    # routes is the caller-controlled ``pk`` path parameter (folded into the
+    # rison filters below), so any role with ReportSchedule read could
+    # iterate every schedule's logs deployment-wide. Mirrors the
+    # ReportScheduleFilter applied by the schedule API
+    # (superset/reports/api.py).
+    base_filters = [["id", ReportExecutionLogFilter, lambda: []]]
 
     show_columns = [
         "id",
diff --git a/tests/unit_tests/reports/filters_test.py b/tests/unit_tests/reports/filters_test.py
index bdcb2c181fb..defc5fd11ce 100644
--- a/tests/unit_tests/reports/filters_test.py
+++ b/tests/unit_tests/reports/filters_test.py
@@ -106,3 +106,45 @@ def test_report_schedule_all_text_filter_coerces_non_string(
         mock_report_schedule.sql,
     ):
         column.ilike.assert_called_once_with(expected, escape="\\")
+
+
+@patch("superset.reports.filters.security_manager", new_callable=MagicMock)
+def test_report_execution_log_filter_admin_sees_all(mock_sm: MagicMock) -> None:
+    """
+    Regression test: ``ReportExecutionLogRestApi`` had no base filter at all,
+    so any role with generic ReportSchedule read could iterate every
+    schedule's logs by pk. An admin (can_access_all_datasources) must still
+    see everything unfiltered.
+    """
+    from superset.reports.filters import ReportExecutionLogFilter
+
+    mock_sm.can_access_all_datasources.return_value = True
+    query = MagicMock()
+    f = ReportExecutionLogFilter("id", MagicMock())
+    result = f.apply(query, None)
+    assert result is query
+    query.filter.assert_not_called()
+
+
+@patch("superset.reports.filters.security_manager", new_callable=MagicMock)
+@patch("superset.reports.filters.db")
+def test_report_execution_log_filter_non_admin_scoped_to_log_fk(
+    mock_db: MagicMock, mock_sm: MagicMock
+) -> None:
+    """
+    A non-admin must be scoped by ``ReportExecutionLog.report_schedule_id``
+    directly (not by an unjoined filter on ``ReportSchedule.id``, which would
+    pass for any log row as long as the caller edits at least one schedule).
+    """
+    from superset.reports.filters import ReportExecutionLogFilter
+    from superset.reports.models import ReportExecutionLog
+
+    mock_sm.can_access_all_datasources.return_value = False
+    query = MagicMock()
+    f = ReportExecutionLogFilter("id", MagicMock())
+    f.apply(query, None)
+
+    query.filter.assert_called_once()
+    (filter_expr,) = query.filter.call_args[0]
+    assert filter_expr.left.table is ReportExecutionLog.__table__
+    assert filter_expr.left.name == "report_schedule_id"
diff --git a/tests/unit_tests/reports/logs_api_test.py b/tests/unit_tests/reports/logs_api_test.py
new file mode 100644
index 00000000000..e081f54e58f
--- /dev/null
+++ b/tests/unit_tests/reports/logs_api_test.py
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from superset.reports.filters import ReportExecutionLogFilter
+from superset.reports.logs.api import ReportExecutionLogRestApi
+
+
+def test_execution_log_api_has_ownership_base_filter() -> None:
+    """
+    Regression test: the schedule id in the ``/log/`` and ``/log/<id>``
+    routes is caller-controlled (folded into the rison filters), so an
+    editor-scoped base filter must be applied to both the list and item
+    routes -- otherwise any role with generic ReportSchedule read can iterate
+    every schedule's logs.
+    """
+    assert ReportExecutionLogRestApi.base_filters, (
+        "ReportExecutionLogRestApi must apply an ownership base filter; "
+        "without one, logs are readable across schedules regardless of "
+        "ownership"
+    )
+    assert any(
+        filter_class is ReportExecutionLogFilter
+        for _, filter_class, _ in ReportExecutionLogRestApi.base_filters
+    )