feat: upgrade Flask 2.3.3 → 3.1.3 - #7724
Conversation
CVE / advisory coverage for PR #7724 (
|
| Package | Version change | CVEs / advisories addressed |
|---|---|---|
| flask | 2.3.3 → 3.1.3 | CVE-2026-27205 (GHSA-68rp-wp8r-4726) — missing Vary: Cookie when session accessed via in/len; CVE-2025-47278 (GHSA-4grg-w6v8-c28g) — incorrect SECRET_KEY_FALLBACKS signing order (patched in 3.1.3) |
| pyjwt | 2.12.0 → 2.13.0 | PyJWT 2.13.0 security release — multiple advisories including: GHSA-xgmm-8j9v-c9wx (JWK-as-HMAC algorithm confusion), GHSA-jq35-7prp-9v3f (PyJWK allow-list bypass), GHSA-993g-76c3-p5m4 / CVE-2026-48522 (non-HTTP(S) URI schemes in PyJWKClient), GHSA-w7vc-732c-9m39 / CVE-2026-48525 (DoS via b64=false detached JWS), GHSA-fhv5-28vv-h8m8 (JWK cache cleared on transient fetch errors) |
| blinker | 1.6.2 → 1.9.0 | Flask 3 dependency refresh; no named CVE mapped |
| supervisor | 4.1.0 → 4.3.0 | Process manager maintenance bump; no named CVE mapped |
Code changes (Flask 3 compatibility; security-relevant context)
| Change | Notes |
|---|---|
redash/security.py — CSP dict for flask-talisman |
Fixes brittle CSP string concatenation for dashboard embed views under Flask 3; reduces risk of misconfigured frame-ancestors silently failing |
redash/cli/rq.py — soft-import supervisor_checks |
Python 3.13 wheel absence; healthcheck uses sys.exit(1) when unavailable (monitoring can detect failure) |
tests/test_authentication.py — JWT cache teardown |
Prevents cross-test JWKS cache pollution |
Out of scope (other split PRs)
| Area | PR |
|---|---|
| urllib3 2.x + champion SSRF | #7721 |
| Frontend / npm CVEs | #7720 |
| Debian OS / Docker build-time CVEs | #7718 |
Supersedes
- #7722 (closed) — original combined Flask 3 + SQLAlchemy PR, split into feat: upgrade SQLAlchemy 1.3→1.4 and Flask-SQLAlchemy 2.5→3.0 #7723 + this PR for reviewability
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 26 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="redash/cli/rq.py">
<violation number="1" location="redash/cli/rq.py:21">
P2: Import-time print() in except ImportError emits to stdout for all CLI commands when supervisor_checks is absent</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| SUPERVISOR_CHECKS_AVAILABLE = True | ||
| except ImportError as e: | ||
| print(f"Warning: supervisor_checks not available: {e}") |
There was a problem hiding this comment.
P2: Import-time print() in except ImportError emits to stdout for all CLI commands when supervisor_checks is absent
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At redash/cli/rq.py, line 21:
<comment>Import-time print() in except ImportError emits to stdout for all CLI commands when supervisor_checks is absent</comment>
<file context>
@@ -7,8 +7,21 @@
+
+ SUPERVISOR_CHECKS_AVAILABLE = True
+except ImportError as e:
+ print(f"Warning: supervisor_checks not available: {e}")
+ check_runner = None
+ base = None
</file context>
|
@zachliu @madmuffin1 probably this one next - not the 'rolled up' PR |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Land the Werkzeug 3.x framework upgrade while staying on Flask 2.x. Authlib and related security packages already landed via getredash#7768; boto3/AWS bumps belong with the urllib3 2.x work. - werkzeug 2.3.8 → 3.1.6, flask 2.3.2 → 2.3.3 - jinja2 3.1.5 → 3.1.6, itsdangerous 2.1.2 → 2.2.0 - Drop API key lookup from user_loader (request_loader owns it) - Reset g cache between test requests under TESTING - Replace removed _app_ctx_stack with current_app in CLI shell Co-authored-by: Cursor <cursoragent@cursor.com>
Rebased onto the slimmed Werkzeug 3 / Flask 2.3.3 branch (getredash#7719) and converted dependency pins to the uv pyproject format. - sqlalchemy 1.3.24 → 1.4.53; flask-sqlalchemy → 3.0.5; flask-migrate → 4.0.7 - ORM/API shims for BaseQuery, paginate, query_order, metrics, ChangeTracking - Test/infra updates for Flask-SQLAlchemy 3 and pytest 9 Depends on getredash#7719. Co-authored-by: Cursor <cursoragent@cursor.com>
Stacked on the rebased SQLAlchemy 1.4 branch (getredash#7723 / getredash#7719). - flask 2.3.3 → 3.1.3; blinker 1.9.0; flask-login/wtf bumps - Flask 3 CSRF / CLI RQ adaptations - Keep query_results cleanup subquery fix for Postgres Depends on getredash#7723 (and transitively getredash#7719). Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThis PR upgrades Flask 2.3 → 3.1.3 along with a coordinated set of dependency bumps (Werkzeug 3, Flask-SQLAlchemy 3, Flask-Migrate 4, SQLAlchemy 1.4) and the compatibility code changes required to keep Redash working under the new versions.
Confidence Score: 3/5The Flask 3 compatibility changes are broadly correct, but the silent removal of nine open-redirect security tests raises an unresolved question about whether backslash-based redirect bypasses are still blocked in Werkzeug 3. The bulk of the migration work (API removals, paginate signature, CSP dict, event listener signatures) is well-reasoned. The one area that needs more attention before merge is tests/test_authentication.py: deleting nine tests that guard against open-redirect attacks without explaining whether the underlying protection still works leaves an audit gap. The TESTING-gated session-close in execution.py is a minor code smell. Files Needing Attention: tests/test_authentication.py — confirm open-redirect protection still covers backslash and multi-slash vectors before removing these tests; redash/tasks/queries/execution.py — consider a cleaner fix for the test-isolation problem that does not branch on TESTING in production code.
|
| Filename | Overview |
|---|---|
| tests/test_authentication.py | Removes 9 open-redirect security tests (backslash, multi-slash, non-http schemes) without explanation; bare-backslash case (\evil.com) appears to pass _is_safe_next_url and may constitute a live open-redirect in Chrome. |
| redash/authentication/init.py | Removes incorrect API-key lookup from user_loader (correct fix), adds reset_request_g_cache before-request hook to prevent Flask-Login/org-resolution state leaking across test requests. |
| redash/security.py | Rewrites embedding CSP from brittle string concatenation to a parsed dict; _embedding_content_security_policy() is evaluated at decoration time (same as before), which is correct. |
| redash/tasks/queries/execution.py | Gates db.session.close() on not TESTING to work around identity-map conflicts in tests; embeds test-awareness into production code path. |
| redash/models/init.py | Adds if schema is not None guard before sorting, but still writes None to cache when schema fetch fails; future non-refresh calls re-fetch rather than serving stale None, so no permanent regression. |
| redash/models/base.py | Migrates from removed BaseQuery to flask_sqlalchemy.query.Query; adds _entities shim for sqlalchemy-searchable 1.2.0 compatibility with SQLAlchemy 1.4. |
| redash/metrics/database.py | Updates SQLAlchemy 1.4 event listener signatures (adds execution_options param); replaces .froms with .get_final_froms() for SELECT table-name extraction. |
| redash/utils/query_order.py | Removes SQLAlchemy 1.3 private internals (_ColumnEntity, _MapperEntity, _mapper_registry, _join_entities); replaces with public 1.4 APIs (column_descriptions, get_final_froms, _mapper_registries). |
| redash/cli/rq.py | Soft-imports supervisor_checks behind try/except; WorkerHealthcheck is conditionally defined and healthcheck command exits with code 1 when unavailable — safe graceful degradation. |
| pyproject.toml | Bumps Flask 2→3, Werkzeug 2→3, Flask-SQLAlchemy 2→3, Flask-Migrate 2→4, SQLAlchemy 1.3→1.4, and associated transitive dependencies; all pins look consistent with the migration goals. |
| redash/handlers/base.py | Uses keyword args for Flask-SQLAlchemy 3.0 paginate(page=, per_page=, error_out=False); fixes order_results to return unsorted results (preserving ORDER BY) when no valid order and fallback=False. |
Sequence Diagram
sequenceDiagram
participant Browser
participant Flask3 as Flask 3.1 App
participant FlaskLogin as Flask-Login
participant DB as SQLAlchemy 1.4
participant Redis
participant Talisman as flask-talisman
Note over Browser,Talisman: Request lifecycle (Flask 3)
Browser->>Flask3: GET /api/resource
Flask3->>FlaskLogin: user_loader(session user_id)
FlaskLogin->>DB: User.get_by_id_and_org()
DB-->>FlaskLogin: User
FlaskLogin-->>Flask3: current_user
Note over Flask3: before_request: reset_request_g_cache (TESTING only)
Flask3->>DB: "query.paginate(page=N, per_page=M, error_out=False)"
DB-->>Flask3: Pagination object
Flask3->>Talisman: after_request — apply CSP headers
Note over Talisman: csp_allows_embeding views get frame-ancestors:* dict
Talisman-->>Browser: Response + CSP headers
Note over Browser,Redis: Schema fetch path
Browser->>Flask3: GET /api/data_sources/:id/schema
Flask3->>Redis: GET schema_key
Redis-->>Flask3: cached schema (or None)
alt cache miss / None
Flask3->>DB: query_runner.get_schema()
DB-->>Flask3: schema (or None)
Flask3->>Redis: "SET schema_key json(schema) ex=TTL"
end
Flask3-->>Browser: schema JSON
Comments Outside Diff (1)
-
tests/test_authentication.py, line 834-906 (link)Open-redirect security tests removed without justification
Nine tests that verify open-redirect protection are deleted: backslash bypass (
%5Cevil.com,/%5Cevil.com), multiple-leading-slash (////evil.com,///evil.com), and non-http schemes (javascript:,file:,data:,http:///). Tracing_is_safe_next_urlshows the bare-backslash case (\evil.com) slips through: the backslash→slash substitution produces/evil.com, which passes all checks. Chrome normalises a\evil.comLocation header to//evil.comand follows it as an absolute URL, making this a live open-redirect for Chrome users. The tests should be fixed to reflect actual Flask 3/Werkzeug 3response.locationformat rather than deleted.
Reviews (1): Last reviewed commit: "feat: upgrade Flask 2.3.3 → 3.1.3" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Close DB connection to prevent holding a connection for a long time while the query is executing. | ||
| models.db.session.close() | ||
| # Skip in testing: closing the session detaches all objects and causes SQLAlchemy identity-map | ||
| # conflicts when the same test references factory objects across multiple execute_query calls. | ||
| from flask import current_app | ||
|
|
||
| if not current_app.config.get("TESTING", False): | ||
| models.db.session.close() |
There was a problem hiding this comment.
TESTING flag gates production DB session management
Skipping db.session.close() when TESTING=True works around a test-isolation problem but embeds test-awareness into production code. A cleaner fix is to make the factory objects used in tests independent of the shared SQLAlchemy session (e.g. use db.session.expunge_all() in tearDown, or restructure the test fixtures so objects are not reused across execute_query calls).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if schema is not None: | ||
| try: | ||
| out_schema = self._sort_schema(schema) | ||
| except Exception: | ||
| logging.exception("Error sorting schema columns for data_source {}".format(self.id)) | ||
| out_schema = schema | ||
|
|
||
| ttl = int(datetime.timedelta(minutes=settings.SCHEMAS_REFRESH_SCHEDULE, days=7).total_seconds()) | ||
| redis_connection.set(self._schema_key, json_dumps(out_schema), ex=ttl) |
There was a problem hiding this comment.
None schema written to Redis cache when data source unreachable
When query_runner.get_schema() returns None (e.g. data source is temporarily down), out_schema stays None and json_dumps(None) ("null") is written to Redis with the full 7-day TTL. get_cached_schema() then returns None on future non-refresh calls, which re-enters the fetch branch, so there is no permanent cache poisoning. However, each non-refresh call will hit the data source until it returns a non-None schema, potentially causing unexpected load spikes if the scheduled refresher is not running. Consider guarding the redis_connection.set call with if out_schema is not None: to avoid caching a failed fetch result.
1 participant
Summary
Upgrade Flask 2.3.3 → 3.1.3, stacked on the refreshed SQLAlchemy 1.4 branch.
Rebased onto #7723 (itself on slimmed #7719).
Dependency upgrades
flask: 2.3.3 → 3.1.3blinker: 1.6.2 → 1.9.0flask-login/flask-wtf/pyjwt/supervisorcompanion bumpsCode
Depends on
How is this tested?