This is an automated email from the ASF dual-hosted git repository. aminghadersohi pushed a commit to branch work-pr-39604 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3a632e1d829dd51d9e57a9397e67a5bd09c236d7 Author: Amin Ghadersohi <[email protected]> AuthorDate: Thu May 14 23:37:10 2026 +0000 fix(mcp): address CodeQL security warnings and add ApiKey RBAC regression test - Remove JWT-extracted username from ValueError message in auth.py to avoid CodeQL py/clear-text-logging-sensitive-data; log at DEBUG instead - Log count of invalid FAB_API_KEY_PREFIXES entries rather than values to avoid the same CodeQL rule in composite_token_verifier.py - Add regression test asserting "ApiKey" in ADMIN_ONLY_VIEW_MENUS so a future rename cannot silently re-open the FAB ApiKeyApi to non-Admin roles --- superset/mcp_service/auth.py | 7 +++++-- superset/mcp_service/composite_token_verifier.py | 6 +++++- tests/unit_tests/security/test_granular_export_permissions.py | 11 +++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py index 22e82e038fe..61361657000 100644 --- a/superset/mcp_service/auth.py +++ b/superset/mcp_service/auth.py @@ -305,9 +305,12 @@ def _resolve_user_from_jwt_context(app: Any) -> User | None: if not user: # Fail closed: JWT says this user should exist but they don't. # Do NOT fall through to MCP_DEV_USERNAME or stale g.user. + # Avoid echoing the JWT-extracted username in the exception message + # (CodeQL py/clear-text-logging-sensitive-data). + logger.debug("JWT-authenticated user not found in database (identity from JWT)") raise ValueError( - f"JWT authenticated user '{username}' not found in Superset database. " - f"Ensure the user exists before granting MCP access." + "JWT authenticated user not found in Superset database. " + "Ensure the user exists before granting MCP access." ) return user diff --git a/superset/mcp_service/composite_token_verifier.py b/superset/mcp_service/composite_token_verifier.py index 8889d66af42..72b22df60aa 100644 --- a/superset/mcp_service/composite_token_verifier.py +++ b/superset/mcp_service/composite_token_verifier.py @@ -73,8 +73,12 @@ class CompositeTokenVerifier(TokenVerifier): ] invalid = [p for p in api_key_prefixes if p not in valid] if invalid: + # Log count only — actual values may be config secrets + # (CodeQL py/clear-text-logging-sensitive-data). logger.warning( - "FAB_API_KEY_PREFIXES contains invalid entries (ignored): %r", invalid + "FAB_API_KEY_PREFIXES has %d invalid entries (empty/non-string)" + " — ignored", + len(invalid), ) self._api_key_prefixes = tuple(valid) diff --git a/tests/unit_tests/security/test_granular_export_permissions.py b/tests/unit_tests/security/test_granular_export_permissions.py index 6db5e9712b2..af703debc64 100644 --- a/tests/unit_tests/security/test_granular_export_permissions.py +++ b/tests/unit_tests/security/test_granular_export_permissions.py @@ -91,6 +91,17 @@ def test_is_gamma_pvm_excludes_export_image(app_context: None) -> None: assert sm._is_gamma_pvm(pvm) is False +def test_api_key_view_menu_is_admin_only() -> None: + """Regression test: 'ApiKey' must be in ADMIN_ONLY_VIEW_MENUS. + + FAB registers an ApiKeyApi blueprint when FAB_API_KEY_ENABLED=True. + Without this guard any Gamma user could reach the API key management + endpoints. A rename or removal of the entry would silently re-open + that access hole. + """ + assert "ApiKey" in SupersetSecurityManager.ADMIN_ONLY_VIEW_MENUS + + def test_is_gamma_pvm_allows_copy_clipboard(app_context: None) -> None: """Verify _is_gamma_pvm returns True for can_copy_clipboard.""" from superset.extensions import appbuilder
