GHSA-q85w-c766-h5g8 - Advisory for four unprotected routes in price_data_follower, check_proxy and language handling that missing. Reported-by: Mayssare Amakhtari (@cy3erm) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: dgtlmoon

changedetectionio/blueprint/check_proxies/__init__.py

@@ -7,7 +7,7 @@
 from functools import wraps
 
 from flask import Blueprint
-from flask_login import login_required
+from changedetectionio.auth_decorator import login_optionally_required
 
 STATUS_CHECKING = 0
 STATUS_FAILED = 1
@@ -94,14 +94,14 @@ def _recalc_check_status(uuid):
 
         return results
 
-    @login_required
     @check_proxies_blueprint.route("/<uuid_str:uuid>/status", methods=['GET'])
+    @login_optionally_required
     def get_recheck_status(uuid):
         results = _recalc_check_status(uuid=uuid)
         return results
 
-    @login_required
     @check_proxies_blueprint.route("/<uuid_str:uuid>/start", methods=['GET'])
+    @login_optionally_required
     def start_check(uuid):
 
         if not datastore.proxy_list:

changedetectionio/blueprint/price_data_follower/__init__.py

@@ -1,7 +1,7 @@
 
 from changedetectionio.strtobool import strtobool
 from flask import Blueprint, flash, redirect, url_for
-from flask_login import login_required
+from changedetectionio.auth_decorator import login_optionally_required
 from changedetectionio.store import ChangeDetectionStore
 from changedetectionio import queuedWatchMetaData
 from changedetectionio import worker_pool
@@ -14,8 +14,8 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q: PriorityQueue
 
     price_data_follower_blueprint = Blueprint('price_data_follower', __name__)
 
-    @login_required
     @price_data_follower_blueprint.route("/<uuid_str:uuid>/accept", methods=['GET'])
+    @login_optionally_required
     def accept(uuid):
         datastore.data['watching'][uuid]['track_ldjson_price_data'] = PRICE_DATA_TRACK_ACCEPT
         datastore.data['watching'][uuid]['processor'] = 'restock_diff'
@@ -24,8 +24,8 @@ def accept(uuid):
         worker_pool.queue_item_async_safe(update_q, queuedWatchMetaData.PrioritizedItem(priority=1, item={'uuid': uuid}))
         return redirect(url_for("watchlist.index"))
 
-    @login_required
     @price_data_follower_blueprint.route("/<uuid_str:uuid>/reject", methods=['GET'])
+    @login_optionally_required
     def reject(uuid):
         datastore.data['watching'][uuid]['track_ldjson_price_data'] = PRICE_DATA_TRACK_REJECT
         datastore.data['watching'][uuid].commit()

changedetectionio/flask_app.py

@@ -624,8 +624,13 @@ def check_authentication():
             # Permitted - static flag icons need to load on login page
             elif request.endpoint and request.endpoint == 'static_flags':
                 return None
-            # Permitted - language selection should work on login page
-            elif request.endpoint and request.endpoint == 'set_language':
+            # Permitted - language selection should work on login page.
+            # Both halves of the language modal must be exempt: it renders for anonymous
+            # users (base.html deliberately leaves it outside the is_authenticated guard),
+            # so exempting only set_language let you pick a language but bounced
+            # "Auto-detect from browser" to /login without clearing the session locale.
+            elif request.endpoint and request.endpoint in ('set_language',
+                                                           'ui.delete_locale_language_session_var_if_it_exists'):
                 return None
             # Permitted
             elif request.endpoint and 'login' in request.endpoint:

changedetectionio/tests/test_language_selector_anonymous.py

@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+"""
+The language modal renders for anonymous users on the login page (base.html keeps it
+outside the `current_user.is_authenticated or not has_password` guard that wraps the
+search modal). Both of its actions must therefore work while logged out.
+
+Regression: only `set_language` was exempted in check_authentication(), so an anonymous
+user at the login screen could pick a specific language but clicking "Auto-detect from
+browser" right below it 302'd to /login without clearing the session locale.
+"""
+
+from flask import url_for
+from .util import live_server_setup, wait_for_all_checks
+
+
+def test_language_endpoints_work_for_anonymous_users(client, live_server, measure_memory_usage, datastore_path):
+    # Enable password protection so the global auth wall in check_authentication() is active
+    res = client.post(
+        url_for("settings.settings_page"),
+        data={
+            "application-password": "hunter2",
+            "requests-time_between_check-minutes": 180,
+            "application-fetch_backend": "html_requests",
+        },
+        follow_redirects=True)
+    assert res.status_code == 200
+
+    client.get(url_for("logout"), follow_redirects=True)
+
+    # Both language links are rendered on the login page, so both must be reachable
+    res = client.get(url_for("login"))
+    assert res.status_code == 200
+    assert b'language-selector' in res.data, "Language modal trigger should render for anonymous users"
+
+    # Picking a specific language must not redirect to the login page
+    res = client.get(url_for("set_language", locale="de"), follow_redirects=False)
+    assert res.status_code == 302
+    assert '/login' not in res.headers.get("Location", ""), \
+        "set_language must not bounce anonymous users to /login"
+
+    # ...and neither must clearing it back to auto-detect
+    res = client.get(url_for("ui.delete_locale_language_session_var_if_it_exists"), follow_redirects=False)
+    assert res.status_code == 302
+    assert '/login' not in res.headers.get("Location", ""), \
+        "Auto-detect must not bounce anonymous users to /login (it renders on the login page)"

changedetectionio/tests/unit/test_auth_decorator_order.py

@@ -1,21 +1,33 @@
 """
-Static analysis test: verify @login_optionally_required is always applied
-AFTER (inner to) @blueprint.route(), not before it.
+Static analysis test: verify @blueprint.route() is always the outermost
+decorator on a view, so nothing sits above it.
 
-In Flask, @route() must be the outermost decorator because it registers
-whatever function it receives. If @login_optionally_required is placed
-above @route(), the raw unprotected function gets registered and auth is
-silently bypassed (GHSA-jmrh-xmgh-x9j4).
+In Flask, @route() must be outermost because it registers whatever function
+it receives and then returns that function unchanged. Any decorator placed
+above @route() is applied only to the module-level name, never to the view
+the blueprint actually dispatches to — so it is silently dead code. When the
+dead decorator is an auth wrapper, the route is left unprotected
+(GHSA-jmrh-xmgh-x9j4).
 
 Correct order (route outermost, auth inner):
     @blueprint.route('/path')
     @login_optionally_required
     def view(): ...
 
 Wrong order (auth never called):
-    @login_optionally_required   ← registered by route, then discarded
+    @login_optionally_required   ← discarded; route registered the raw fn
     @blueprint.route('/path')
     def view(): ...
+
+This check is deliberately name-agnostic. An earlier version matched only
+the literal name `login_optionally_required`, which missed four routes using
+plain flask_login `login_required` (GHSA-q85w-c766-h5g8) — an allowlist of
+decorator names only ever catches the names someone remembered to add. We
+now flag *any* non-route decorator above @route, which also covers attribute
+forms (@flask_login.login_required), aliased imports, and non-auth
+decorators that are equally dead up there.
+
+Stacked @route decorators are a legitimate Flask idiom and are exempt.
 """
 
 import ast
@@ -35,11 +47,6 @@ def _is_route_decorator(node: ast.expr) -> bool:
     )
 
 
-def _is_auth_decorator(node: ast.expr) -> bool:
-    """Return True if the decorator is @login_optionally_required."""
-    return isinstance(node, ast.Name) and node.id == "login_optionally_required"
-
-
 def collect_violations() -> list[str]:
     violations = []
 
@@ -54,20 +61,22 @@ def collect_violations() -> list[str]:
                 continue
 
             decorators = node.decorator_list
-            auth_indices = [i for i, d in enumerate(decorators) if _is_auth_decorator(d)]
             route_indices = [i for i, d in enumerate(decorators) if _is_route_decorator(d)]
+            if not route_indices:
+                continue
 
-            # Bad order: auth decorator appears at a lower index (higher up) than a route decorator
-            for auth_idx in auth_indices:
-                for route_idx in route_indices:
-                    if auth_idx < route_idx:
-                        rel = path.relative_to(REPO_ROOT)
-                        violations.append(
-                            f"{rel}:{node.lineno} — `{node.name}`: "
-                            f"@login_optionally_required (line {decorators[auth_idx].lineno}) "
-                            f"is above @route (line {decorators[route_idx].lineno}); "
-                            f"auth wrapper will never be called"
-                        )
+            # Everything above the last @route is discarded by the registration.
+            # Other @route decorators up there are fine — stacking routes is normal.
+            last_route = max(route_indices)
+            for i, decorator in enumerate(decorators):
+                if i < last_route and not _is_route_decorator(decorator):
+                    rel = path.relative_to(REPO_ROOT)
+                    violations.append(
+                        f"{rel}:{node.lineno} — `{node.name}`: "
+                        f"@{ast.unparse(decorator)} (line {decorator.lineno}) is above @route "
+                        f"(line {decorators[last_route].lineno}); it will never be applied "
+                        f"to the registered view"
+                    )
 
     return violations
 
@@ -76,9 +85,10 @@ def test_auth_decorator_order():
     violations = collect_violations()
     if violations:
         msg = (
-            "\n\nFound routes where @login_optionally_required is placed ABOVE @blueprint.route().\n"
-            "This silently disables authentication — @route() registers the raw function\n"
-            "and the auth wrapper is never called.\n\n"
+            "\n\nFound decorators placed ABOVE @blueprint.route().\n"
+            "@route() registers the raw function and returns it unchanged, so anything\n"
+            "above it is never applied to the view Flask dispatches to. If the decorator\n"
+            "is an auth wrapper, the route is left completely unauthenticated.\n\n"
             "Fix: move @blueprint.route() to be the outermost (topmost) decorator.\n\n"
             + "\n".join(f"  • {v}" for v in violations)
         )