fix(security): guard is_guest_user against NoAuthorizationError on unauthenticated error paths

[eschutho]

SUMMARY

Fixes SUPERSET-PYTHON-15J6 — NoAuthorizationError: Missing JWT in cookies or headers, 28,783 events since firstSeen 2026-08-21, still firing daily.

The Sentry-captured exception is not the original problem — it is a secondary exception raised while Flask is already handling an unrelated, benign exception (in the sampled events, a 405 MethodNotAllowed from a request hitting the wrong HTTP method on a route, before any view/auth code runs). The same crash can happen for any early-in-request-lifecycle HTTPException.

Root-cause chain

1. Flask raises MethodNotAllowed (405) during routing, before any view/auth code runs.
2. The global @app.errorhandler(HTTPException) def show_http_exception (superset/views/error_handling.py) catches it and calls json_error_response(...).
3. json_error_response calls sanitize_superset_errors() (superset/utils/error_sanitization.py).
4. That calls is_sanitization_required() → security_manager.is_guest_user().
5. is_guest_user() with no user argument calls get_current_user() (superset/tasks/utils.py), whose g.user access forces resolution of flask_login's current_user LocalProxy.
6. Resolving the proxy invokes the app's registered request loader. For a request with no guest token, no JWT, and not a public-workspace / MCP-OAuth path, the loader calls verify_jwt_in_request() and lets it raise (by design — a global flask-jwt-extended handler is meant to turn that into a 401 for a real unauthenticated view request).
7. But here the raise happens inside show_http_exception, the error handler for the original 405. This second exception is not caught by anything and propagates out unhandled — captured by Sentry, and turning a benign 405 into an actual crash/500 for any embedded-enabled deployment, for any unauthenticated request that trips an HTTPException path before auth ever runs.

The fix

Wrap the current-user resolution inside SupersetSecurityManager.is_guest_user() in try/except NoAuthorizationError: return False.

A request that carries no JWT and no guest token definitionally cannot be an embedded guest viewer, so returning False is the semantically correct answer, not merely crash avoidance. is_guest_user is the shared choke point behind many call sites across the codebase; only the error-sanitization path is normally reachable before auth runs, and this fix protects all of them without changing behavior for any already-authenticated caller.

NoAuthorizationError is imported from flask_jwt_extended.exceptions (the top-level package does not re-export it). The fix lives entirely in superset/security/manager.py; error_sanitization.py, error_handling.py, and the private request-loader are intentionally untouched — their behavior is correct for real view requests.

TESTING INSTRUCTIONS

Added tests/unit_tests/security/manager_test.py::test_is_guest_user_no_jwt_returns_false_without_raising: with EMBEDDED_SUPERSET enabled and the current-user resolution raising NoAuthorizationError, is_guest_user() must return False rather than propagate.

Verified the test fails before the fix (raises NoAuthorizationError) and passes after:

$ pytest tests/unit_tests/security/manager_test.py tests/unit_tests/utils/test_error_sanitization.py -q
136 passed

$ ruff check superset/security/manager.py tests/unit_tests/security/manager_test.py
All checks passed!
$ ruff format --check superset/security/manager.py tests/unit_tests/security/manager_test.py
2 files already formatted

pre-commit (mypy, ruff, ruff-format, pylint) clean on the changed files.

Tradeoffs

None as a failure-mode change. This makes is_guest_user() never raise where it previously could sometimes crash the request, which is strictly more correct:

• For an unauthenticated request with no JWT/guest token, False is the semantically correct return value (such a request cannot be an embedded guest), so no legitimate guest is misclassified.
• For any already-authenticated caller (real guest token or logged-in user), the proxy resolves without raising, so NoAuthorizationError is never hit and behavior is byte-for-byte unchanged.
• No security-boundary change: it never grants guest treatment where it wasn't already granted — it only stops a benign error path from escalating into an unhandled 500. Nothing here is undisclosed.

ADDITIONAL INFORMATION

• Sentry: SUPERSET-PYTHON-15J6 (28,783 events, auto-resolves on merge via Fixes SUPERSET-PYTHON-15J6)
• Shortcut: SC-119741
• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration
• Introduces new feature or API
• Removes existing feature or API

[bito-code-review]

Code Review Agent Run #8128

Actionable Suggestions - 0

Additional Suggestions - 1

• superset/security/manager.py - 1

  • Dead except branch · Line 5501-5506
    `get_current_user()` (imported from `superset.tasks.utils`) reads `g.user` with a `hasattr` guard and never evaluates flask_login's `current_user` proxy nor calls `verify_jwt_in_request`, so it cannot raise `NoAuthorizationError`. The `except` branch is unreachable in production; the test only passes because it mocks `get_current_user` to raise. If the intent is to guard the request-loader raise, evaluate the `current_user` proxy inside the `try`; otherwise remove the dead `try/except` and correct the comment.

Review Details

• Files reviewed - 2 · Commit Range: 0b65d05..0b65d05

  • superset/security/manager.py
  • tests/unit_tests/security/manager_test.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.41%. Comparing base (669a0ce) to head (0b65d05).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #43826   +/-   ##
=======================================
  Coverage   79.41%   79.41%           
=======================================
  Files        2894     2894           
  Lines      167820   167824    +4     
  Branches    38863    38863           
=======================================
+ Hits       133269   133273    +4     
  Misses      32051    32051           
  Partials     2500     2500           

Flag	Coverage Δ
hive	37.78% <16.66%> (-0.01%)	⬇️
mysql	57.50% <83.33%> (-0.01%)	⬇️
postgres	57.53% <83.33%> (-0.01%)	⬇️
presto	39.67% <16.66%> (-0.01%)	⬇️
python	83.86% <100.00%> (+<0.01%)	⬆️
sqlite	57.23% <83.33%> (-0.01%)	⬇️
unit	74.34% <66.66%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.