From 197e94302ba02c2ec6988030a6a228ce433fd3ab Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 24 Dec 2025 16:35:57 +0000 Subject: [PATCH] Patches (#2219) * feat: implement URL validation to prevent SSRF * feat: add zip extraction security * ruff fixes * fix: standardize error messages across API responses --- application/api/answer/routes/answer.py | 2 +- application/api/connector/routes.py | 120 ++++++++++++++++++------ application/api/user/sources/routes.py | 2 +- application/api/user/tools/mcp.py | 10 +- application/auth.py | 4 +- application/retriever/classic_rag.py | 4 +- frontend/.env.development | 4 +- 7 files changed, 102 insertions(+), 44 deletions(-) diff --git a/application/api/answer/routes/answer.py b/application/api/answer/routes/answer.py index e79ea378..c24ebffc 100644 --- a/application/api/answer/routes/answer.py +++ b/application/api/answer/routes/answer.py @@ -137,5 +137,5 @@ class AnswerResource(Resource, BaseAnswerResource): f"/api/answer - error: {str(e)} - traceback: {traceback.format_exc()}", extra={"error": str(e), "traceback": traceback.format_exc()}, ) - return make_response({"error": str(e)}, 500) + return make_response({"error": "An error occurred processing your request"}, 500) return make_response(result, 200) diff --git a/application/api/connector/routes.py b/application/api/connector/routes.py index d8efc1d3..1a4e80bf 100644 --- a/application/api/connector/routes.py +++ b/application/api/connector/routes.py @@ -1,7 +1,9 @@ import base64 import datetime +import html import json import uuid +from urllib.parse import urlencode from bson.objectid import ObjectId @@ -35,6 +37,18 @@ connector = Blueprint("connector", __name__) connectors_ns = Namespace("connectors", description="Connector operations", path="/") api.add_namespace(connectors_ns) +# Fixed callback status path to prevent open redirect +CALLBACK_STATUS_PATH = "/api/connectors/callback-status" + + +def build_callback_redirect(params: dict) -> str: + """Build a safe redirect URL to the callback status page. + + Uses a fixed path and properly URL-encodes all parameters + to prevent URL injection and open redirect vulnerabilities. + """ + return f"{CALLBACK_STATUS_PATH}?{urlencode(params)}" + @connectors_ns.route("/api/connectors/auth") @@ -75,8 +89,8 @@ class ConnectorAuth(Resource): "state": state }), 200) except Exception as e: - current_app.logger.error(f"Error generating connector auth URL: {e}") - return make_response(jsonify({"success": False, "error": str(e)}), 500) + current_app.logger.error(f"Error generating connector auth URL: {e}", exc_info=True) + return make_response(jsonify({"success": False, "error": "Failed to generate authorization URL"}), 500) @connectors_ns.route("/api/connectors/callback") @@ -93,18 +107,37 @@ class ConnectorsCallback(Resource): error = request.args.get('error') state_dict = json.loads(base64.urlsafe_b64decode(state.encode()).decode()) - provider = state_dict["provider"] - state_object_id = state_dict["object_id"] + provider = state_dict.get("provider") + state_object_id = state_dict.get("object_id") + + # Validate provider + if not provider or not isinstance(provider, str) or not ConnectorCreator.is_supported(provider): + return redirect(build_callback_redirect({ + "status": "error", + "message": "Invalid provider" + })) if error: if error == "access_denied": - return redirect(f"/api/connectors/callback-status?status=cancelled&message=Authentication+was+cancelled.+You+can+try+again+if+you'd+like+to+connect+your+account.&provider={provider}") + return redirect(build_callback_redirect({ + "status": "cancelled", + "message": "Authentication was cancelled. You can try again if you'd like to connect your account.", + "provider": provider + })) else: current_app.logger.warning(f"OAuth error in callback: {error}") - return redirect(f"/api/connectors/callback-status?status=error&message=Authentication+failed.+Please+try+again+and+make+sure+to+grant+all+requested+permissions.&provider={provider}") + return redirect(build_callback_redirect({ + "status": "error", + "message": "Authentication failed. Please try again and make sure to grant all requested permissions.", + "provider": provider + })) if not authorization_code: - return redirect(f"/api/connectors/callback-status?status=error&message=Authentication+failed.+Please+try+again+and+make+sure+to+grant+all+requested+permissions.&provider={provider}") + return redirect(build_callback_redirect({ + "status": "error", + "message": "Authentication failed. Please try again and make sure to grant all requested permissions.", + "provider": provider + })) try: auth = ConnectorCreator.create_auth(provider) @@ -141,15 +174,28 @@ class ConnectorsCallback(Resource): ) # Redirect to success page with session token and user email - return redirect(f"/api/connectors/callback-status?status=success&message=Authentication+successful&provider={provider}&session_token={session_token}&user_email={user_email}") + return redirect(build_callback_redirect({ + "status": "success", + "message": "Authentication successful", + "provider": provider, + "session_token": session_token, + "user_email": user_email + })) except Exception as e: current_app.logger.error(f"Error exchanging code for tokens: {str(e)}", exc_info=True) - return redirect(f"/api/connectors/callback-status?status=error&message=Authentication+failed.+Please+try+again+and+make+sure+to+grant+all+requested+permissions.&provider={provider}") + return redirect(build_callback_redirect({ + "status": "error", + "message": "Authentication failed. Please try again and make sure to grant all requested permissions.", + "provider": provider + })) except Exception as e: current_app.logger.error(f"Error handling connector callback: {e}") - return redirect("/api/connectors/callback-status?status=error&message=Authentication+failed.+Please+try+again+and+make+sure+to+grant+all+requested+permissions.") + return redirect(build_callback_redirect({ + "status": "error", + "message": "Authentication failed. Please try again and make sure to grant all requested permissions." + })) @connectors_ns.route("/api/connectors/files") @@ -228,8 +274,8 @@ class ConnectorFiles(Resource): "has_more": has_more }), 200) except Exception as e: - current_app.logger.error(f"Error loading connector files: {e}") - return make_response(jsonify({"success": False, "error": f"Failed to load files: {str(e)}"}), 500) + current_app.logger.error(f"Error loading connector files: {e}", exc_info=True) + return make_response(jsonify({"success": False, "error": "Failed to load files"}), 500) @connectors_ns.route("/api/connectors/validate-session") @@ -289,8 +335,8 @@ class ConnectorValidateSession(Resource): "access_token": token_info.get('access_token') }), 200) except Exception as e: - current_app.logger.error(f"Error validating connector session: {e}") - return make_response(jsonify({"success": False, "error": str(e)}), 500) + current_app.logger.error(f"Error validating connector session: {e}", exc_info=True) + return make_response(jsonify({"success": False, "error": "Failed to validate session"}), 500) @connectors_ns.route("/api/connectors/disconnect") @@ -311,8 +357,8 @@ class ConnectorDisconnect(Resource): return make_response(jsonify({"success": True}), 200) except Exception as e: - current_app.logger.error(f"Error disconnecting connector session: {e}") - return make_response(jsonify({"success": False, "error": str(e)}), 500) + current_app.logger.error(f"Error disconnecting connector session: {e}", exc_info=True) + return make_response(jsonify({"success": False, "error": "Failed to disconnect session"}), 500) @connectors_ns.route("/api/connectors/sync") @@ -418,8 +464,8 @@ class ConnectorSync(Resource): return make_response( jsonify({ "success": False, - "error": str(err) - }), + "error": "Failed to sync connector source" + }), 400 ) @@ -430,17 +476,28 @@ class ConnectorCallbackStatus(Resource): def get(self): """Return HTML page with connector authentication status""" try: - status = request.args.get('status', 'error') - message = request.args.get('message', '') - provider = request.args.get('provider', 'connector') + # Validate and sanitize status to a known value + status_raw = request.args.get('status', 'error') + status = status_raw if status_raw in ('success', 'error', 'cancelled') else 'error' + + # Escape all user-controlled values for HTML context + message = html.escape(request.args.get('message', '')) + provider_raw = request.args.get('provider', 'connector') + provider = html.escape(provider_raw.replace('_', ' ').title()) session_token = request.args.get('session_token', '') - user_email = request.args.get('user_email', '') - + user_email = html.escape(request.args.get('user_email', '')) + + # Use json.dumps for safe JavaScript string embedding + js_status = json.dumps(status) + js_session_token = json.dumps(session_token) + js_user_email = json.dumps(user_email) + js_provider_type = json.dumps(provider_raw) + html_content = f""" - {provider.replace('_', ' ').title()} Authentication + {provider} Authentication