diff --git a/changedetectionio/blueprint/settings/llm.py b/changedetectionio/blueprint/settings/llm.py index 35392d202..af3ae5869 100644 --- a/changedetectionio/blueprint/settings/llm.py +++ b/changedetectionio/blueprint/settings/llm.py @@ -56,6 +56,7 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): @login_optionally_required def llm_get_models(): from flask import request + from changedetectionio.validate_url import is_llm_api_base_safe provider = request.args.get('provider', '').strip() api_key = request.args.get('api_key', '').strip() api_base = request.args.get('api_base', '').strip() @@ -66,10 +67,29 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): logger.debug("LLM model list: no provider specified, returning 400") return jsonify({'models': [], 'error': 'No provider specified'}), 400 - # Fall back to the stored key if the user hasn't typed one yet + ok, reason = is_llm_api_base_safe(api_base) + if not ok: + logger.warning(f"LLM model list refused: api_base failed SSRF check ({reason})") + return jsonify({'models': [], 'error': reason}), 400 + + # Credential-exfiltration guard (GHSA-g36r-fm2p-87xm). + # Only substitute the stored api_key when api_base matches the stored + # api_base. If the caller pointed at a different destination, refuse — + # otherwise a CSRF / unauthenticated request can ship the operator's + # long-lived provider key (sent as Authorization: Bearer …) to an + # attacker-controlled URL. + stored_llm = datastore.data['settings']['application'].get('llm') or {} + stored_api_base = (stored_llm.get('api_base') or '').strip() if not api_key: - api_key = (datastore.data['settings']['application'].get('llm') or {}).get('api_key', '') - logger.debug("LLM model list: no api_key in request, using stored key") + if api_base == stored_api_base: + api_key = (stored_llm.get('api_key') or '') + logger.debug("LLM model list: no api_key in request, using stored key (api_base matches saved)") + elif api_base: + logger.warning("LLM model list refused: api_base differs from saved config but no api_key supplied") + return jsonify({'models': [], 'error': gettext( + "api_key is required when api_base differs from the saved configuration. " + "Refusing to send the stored API key to a different endpoint." + )}), 400 _PREFIXES = {'gemini': 'gemini/', 'ollama': 'ollama/', 'openrouter': 'openrouter/', 'openai_compatible': 'openai/'} @@ -115,15 +135,21 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): def llm_test(): from flask import request from changedetectionio.llm.client import completion + from changedetectionio.validate_url import is_llm_api_base_safe # Pull stored config as the fallback, then override with anything the # form-driven JS sent as query params. Lets users test config changes # without first hitting Save (matching how /settings/llm/models works). stored = datastore.data['settings']['application'].get('llm') or {} + # Keep the raw request-supplied values around so we can detect whether + # the caller explicitly steered api_base / api_key (credential-exfil guard below). + req_api_key = (request.args.get('api_key') or '').strip() + req_api_base = (request.args.get('api_base') or '').strip() + stored_api_base = (stored.get('api_base') or '').strip() llm_cfg = { 'model': (request.args.get('model') or stored.get('model', '')).strip(), - 'api_key': (request.args.get('api_key') or stored.get('api_key', '')).strip(), - 'api_base': (request.args.get('api_base') or stored.get('api_base', '')).strip(), + 'api_key': (req_api_key or stored.get('api_key', '')).strip(), + 'api_base': (req_api_base or stored_api_base).strip(), 'provider_kind': (request.args.get('provider_kind') or stored.get('provider_kind', '')).strip(), 'local_token_multiplier': request.args.get('local_token_multiplier') or stored.get('local_token_multiplier'), } @@ -140,6 +166,23 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): logger.error("LLM connection test failed: no model configured") return jsonify({'ok': False, 'error': 'No model configured.'}), 400 + ok, reason = is_llm_api_base_safe(api_base) + if not ok: + logger.warning(f"LLM connection test refused: api_base failed SSRF check ({reason})") + return jsonify({'ok': False, 'error': reason}), 400 + + # Credential-exfiltration guard (GHSA-g36r-fm2p-87xm). + # If the caller specified an api_base that differs from the saved one but + # did NOT supply a matching api_key, refuse to substitute the stored key. + # Otherwise a CSRF / unauthenticated request can route the operator's + # long-lived provider key to an attacker-controlled endpoint. + if req_api_base and req_api_base != stored_api_base and not req_api_key: + logger.warning("LLM connection test refused: api_base differs from saved config but no api_key supplied") + return jsonify({'ok': False, 'error': gettext( + "api_key is required when api_base differs from the saved configuration. " + "Refusing to send the stored API key to a different endpoint." + )}), 400 + try: logger.debug(f"LLM connection test: sending test prompt to model={model!r}") # Reuse the same multiplier path the production calls use, so cloud providers @@ -181,7 +224,12 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): logger.exception("LLM connection test full traceback:") return jsonify({'ok': False, 'error': str(e)}), 400 - @llm_blueprint.route("/clear", methods=['GET']) + # Both clear endpoints accept POST only — GET would let an attacker fire them via + # , wiping LLM configuration / cached summaries on a logged-in + # operator's browser (GHSA-g36r-fm2p-87xm). Flask-WTF CSRFProtect enforces a + # CSRF token on POST automatically; the template renders csrf_token() inside the + # surrounding
. + @llm_blueprint.route("/clear", methods=['POST']) @login_optionally_required def llm_clear(): logger.debug("LLM configuration cleared by user") @@ -190,7 +238,7 @@ def construct_llm_blueprint(datastore: ChangeDetectionStore): flash(gettext("AI / LLM configuration removed."), 'notice') return redirect(url_for('settings.settings_page') + '#ai') - @llm_blueprint.route("/clear-summary-cache", methods=['GET']) + @llm_blueprint.route("/clear-summary-cache", methods=['POST']) @login_optionally_required def llm_clear_summary_cache(): import glob diff --git a/changedetectionio/blueprint/settings/templates/settings_llm_tab.html b/changedetectionio/blueprint/settings/templates/settings_llm_tab.html index 38074337e..b391b6316 100644 --- a/changedetectionio/blueprint/settings/templates/settings_llm_tab.html +++ b/changedetectionio/blueprint/settings/templates/settings_llm_tab.html @@ -163,9 +163,14 @@ ✓ {{ _('AI / LLM configured:') }} {{ llm_config.get('model') }}   + {# data-method="POST" tells modal.js to POST with the CSRF token instead of + navigating — GET previously allowed -based CSRF wipe (GHSA-g36r-fm2p-87xm). + Stays as because we're inside the outer settings — nested forms are + invalid HTML, so modal.js builds a body-level hidden form for the POST. #} + {# See comment above on data-method="POST"+modal.js (GHSA-g36r-fm2p-87xm). #}