(superset) branch fix/log-allowlist-denials created (now 7d9b918f4db)

[ru...@apache.org]

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

rusackas pushed a change to branch fix/log-allowlist-denials
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 7d9b918f4db feat(api): log rejected related/distinct field access as security events

This branch includes the following new commits:

     new 7d9b918f4db feat(api): log rejected related/distinct field access as security events

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(superset) 01/01: feat(api): log rejected related/distinct field access as security events

[ru...@apache.org]

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

rusackas pushed a commit to branch fix/log-allowlist-denials
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 7d9b918f4dbb55605c92a9d1c9b1977b2b3e182a
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Mon Jun 22 17:59:04 2026 -0700

    feat(api): log rejected related/distinct field access as security events
    
    When a user-supplied column_name fails the related/distinct allowlist check,
    the API incremented a statsd counter and returned 404 but emitted no structured
    log event, so rejected field-access attempts were absent from the security
    audit trail (no user identity, endpoint, or attempted value).
    
    Emit a sanitized security log event (user id, endpoint, attempted column) on
    these denials, alongside the existing statsd counter. The column name is
    sanitized to a single bounded token to prevent log injection. The allowlist
    control itself is unchanged.
    
    Extends the related-field failure integration test to assert the event is
    logged with the rejected column name.
    
    Co-Authored-By: Claude Fable 5 <no...@anthropic.com>
---
 superset/views/base_api.py                | 23 +++++++++++++++++++++++
 tests/integration_tests/base_api_tests.py | 10 +++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/superset/views/base_api.py b/superset/views/base_api.py
index a71a0bd9c75..2017ad5891d 100644
--- a/superset/views/base_api.py
+++ b/superset/views/base_api.py
@@ -223,6 +223,27 @@ class BaseSupersetApiMixin:
             f"{self.__class__.__name__}.{func_name}.{action}"
         )
 
+    def log_rejected_field_access(self, func_name: str, column_name: str) -> None:
+        """Emit a security log event when a related/distinct field is rejected.
+
+        The allowlist check itself blocks the request; this records the attempt
+        in the structured log (alongside the existing statsd counter) so that
+        rejected field access is visible to security monitoring and forensics,
+        with the caller's identity, the endpoint, and the attempted value.
+        """
+        # Sanitize the user-supplied column name to a single, bounded token so
+        # it cannot inject newlines or forge extra log lines.
+        sanitized_column = "".join(
+            ch for ch in str(column_name) if ch.isprintable() and ch not in "\r\n"
+        )[:200]
+        logger.warning(
+            "Rejected disallowed field access: user_id=%s endpoint=%s.%s column=%s",
+            get_user_id(),
+            self.__class__.__name__,
+            func_name,
+            sanitized_column,
+        )
+
     def timing_stats(self, action: str, func_name: str, value: float) -> None:
         """
         Proxy function for statsd.incr to impose a key structure for REST API's
@@ -619,6 +640,7 @@ class BaseSupersetModelRestApi(BaseSupersetApiMixin, ModelRestApi):
             return response
         if column_name not in self.allowed_rel_fields:
             self.incr_stats("error", self.related.__name__)
+            self.log_rejected_field_access(self.related.__name__, column_name)
             return self.response_404()
         args = kwargs.get("rison", {})
 
@@ -698,6 +720,7 @@ class BaseSupersetModelRestApi(BaseSupersetApiMixin, ModelRestApi):
         """
         if column_name not in self.allowed_distinct_fields:
             self.incr_stats("error", self.related.__name__)
+            self.log_rejected_field_access(self.distinct.__name__, column_name)
             return self.response_404()
         args = kwargs.get("rison", {})
         # handle pagination
diff --git a/tests/integration_tests/base_api_tests.py b/tests/integration_tests/base_api_tests.py
index f694603fea5..a1bb0fb1832 100644
--- a/tests/integration_tests/base_api_tests.py
+++ b/tests/integration_tests/base_api_tests.py
@@ -423,5 +423,13 @@ class ApiOwnersTestCaseMixin:
         self.login(ADMIN_USERNAME)
         uri = f"api/v1/{self.resource_name}/related/owner"
 
-        rv = self.client.get(uri)
+        with patch("superset.views.base_api.logger") as mock_logger:
+            rv = self.client.get(uri)
         assert rv.status_code == 404
+        # A disallowed related field is recorded as a security log event,
+        # including the rejected column name, in addition to the 404.
+        mock_logger.warning.assert_called_once()
+        assert "column=owner" in (
+            mock_logger.warning.call_args.args[0]
+            % mock_logger.warning.call_args.args[1:]
+        )