From 55d05fe52de880cd8497df8cea052351c594fad8 Mon Sep 17 00:00:00 2001 From: Tim Ren <137012659+xr843@users.noreply.github.com> Date: Thu, 14 May 2026 23:59:31 +0800 Subject: [PATCH] fix(security): enforce tenant scoping on app trace-config endpoints (GHSA-48xc-wmw8-3jr3) (#35793) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Ido Shani Co-authored-by: -LAN- --- api/controllers/console/app/app.py | 11 ++--- api/controllers/console/app/ops_trace.py | 27 +++++++----- .../controllers/console/app/test_app_apis.py | 43 +++++++++++++++++-- ...test_chat_conversation_status_count_api.py | 8 ++-- 4 files changed, 66 insertions(+), 23 deletions(-) diff --git a/api/controllers/console/app/app.py b/api/controllers/console/app/app.py index a73c202bad..045325f283 100644 --- a/api/controllers/console/app/app.py +++ b/api/controllers/console/app/app.py @@ -3,7 +3,6 @@ import re import uuid from datetime import datetime from typing import Any, Literal -from uuid import UUID from flask import request from flask_restx import Resource @@ -850,10 +849,11 @@ class AppTraceApi(Resource): @setup_required @login_required @account_initialization_required - def get(self, app_id: UUID): + @get_app_model + def get(self, app_model): """Get app trace""" with session_factory.create_session() as session: - app_trace_config = OpsTraceManager.get_app_tracing_config(str(app_id), session) + app_trace_config = OpsTraceManager.get_app_tracing_config(app_model.id, session) return app_trace_config @@ -867,12 +867,13 @@ class AppTraceApi(Resource): @login_required @account_initialization_required @edit_permission_required - def post(self, app_id: UUID): + @get_app_model + def post(self, app_model): # add app trace args = AppTracePayload.model_validate(console_ns.payload) OpsTraceManager.update_app_tracing_config( - app_id=str(app_id), + app_id=app_model.id, enabled=args.enabled, tracing_provider=args.tracing_provider, ) diff --git a/api/controllers/console/app/ops_trace.py b/api/controllers/console/app/ops_trace.py index 9227d00a21..41acf39541 100644 --- a/api/controllers/console/app/ops_trace.py +++ b/api/controllers/console/app/ops_trace.py @@ -1,5 +1,4 @@ from typing import Any -from uuid import UUID from flask import request from flask_restx import Resource, fields @@ -9,8 +8,10 @@ from werkzeug.exceptions import BadRequest from controllers.common.schema import register_schema_models from controllers.console import console_ns from controllers.console.app.error import TracingConfigCheckError, TracingConfigIsExist, TracingConfigNotExist +from controllers.console.app.wraps import get_app_model from controllers.console.wraps import account_initialization_required, setup_required from libs.login import login_required +from models import App from services.ops_service import OpsService @@ -43,11 +44,14 @@ class TraceAppConfigApi(Resource): @setup_required @login_required @account_initialization_required - def get(self, app_id: UUID): - args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True)) + @get_app_model + def get(self, app_model: App): + args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True)) # type: ignore try: - trace_config = OpsService.get_tracing_app_config(app_id=str(app_id), tracing_provider=args.tracing_provider) + trace_config = OpsService.get_tracing_app_config( + app_id=app_model.id, tracing_provider=args.tracing_provider + ) if not trace_config: return {"has_not_configured": True} return trace_config @@ -65,13 +69,14 @@ class TraceAppConfigApi(Resource): @setup_required @login_required @account_initialization_required - def post(self, app_id: UUID): + @get_app_model + def post(self, app_model: App): """Create a new trace app configuration""" args = TraceConfigPayload.model_validate(console_ns.payload) try: result = OpsService.create_tracing_app_config( - app_id=str(app_id), tracing_provider=args.tracing_provider, tracing_config=args.tracing_config + app_id=app_model.id, tracing_provider=args.tracing_provider, tracing_config=args.tracing_config ) if not result: raise TracingConfigIsExist() @@ -90,13 +95,14 @@ class TraceAppConfigApi(Resource): @setup_required @login_required @account_initialization_required - def patch(self, app_id: UUID): + @get_app_model + def patch(self, app_model: App): """Update an existing trace app configuration""" args = TraceConfigPayload.model_validate(console_ns.payload) try: result = OpsService.update_tracing_app_config( - app_id=str(app_id), tracing_provider=args.tracing_provider, tracing_config=args.tracing_config + app_id=app_model.id, tracing_provider=args.tracing_provider, tracing_config=args.tracing_config ) if not result: raise TracingConfigNotExist() @@ -113,12 +119,13 @@ class TraceAppConfigApi(Resource): @setup_required @login_required @account_initialization_required - def delete(self, app_id: UUID): + @get_app_model + def delete(self, app_model: App): """Delete an existing trace app configuration""" args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True)) try: - result = OpsService.delete_tracing_app_config(app_id=str(app_id), tracing_provider=args.tracing_provider) + result = OpsService.delete_tracing_app_config(app_id=app_model.id, tracing_provider=args.tracing_provider) if not result: raise TracingConfigNotExist() return {"result": "success"}, 204 diff --git a/api/tests/test_containers_integration_tests/controllers/console/app/test_app_apis.py b/api/tests/test_containers_integration_tests/controllers/console/app/test_app_apis.py index bb737754a1..b13bdba2bc 100644 --- a/api/tests/test_containers_integration_tests/controllers/console/app/test_app_apis.py +++ b/api/tests/test_containers_integration_tests/controllers/console/app/test_app_apis.py @@ -8,7 +8,9 @@ from unittest.mock import MagicMock, patch import pytest from flask import Flask +from flask.testing import FlaskClient from pydantic import ValidationError +from sqlalchemy.orm import Session from werkzeug.exceptions import BadRequest, NotFound from controllers.console import console_ns @@ -57,6 +59,12 @@ from controllers.console.app.workflow_app_log import WorkflowAppLogQuery from controllers.console.app.workflow_draft_variable import WorkflowDraftVariableUpdatePayload from controllers.console.app.workflow_statistic import WorkflowStatisticQuery from controllers.console.app.workflow_trigger import Parser, ParserEnable +from models.model import AppMode +from tests.test_containers_integration_tests.controllers.console.helpers import ( + authenticate_console_client, + create_console_account_and_tenant, + create_console_app, +) def _unwrap(func): @@ -270,6 +278,35 @@ class TestOpsTraceEndpoints: def app(self, flask_app_with_containers: Flask): return flask_app_with_containers + @pytest.mark.parametrize( + "path_template", + [ + "/console/api/apps/{app_id}/trace-config?tracing_provider=langfuse", + "/console/api/apps/{app_id}/trace", + ], + ) + def test_trace_endpoints_hide_apps_from_other_tenants( + self, + db_session_with_containers: Session, + test_client_with_containers: FlaskClient, + path_template: str, + ): + account, _tenant = create_console_account_and_tenant(db_session_with_containers) + foreign_account, foreign_tenant = create_console_account_and_tenant(db_session_with_containers) + foreign_app = create_console_app( + db_session_with_containers, + tenant_id=foreign_tenant.id, + account_id=foreign_account.id, + mode=AppMode.CHAT, + ) + + response = test_client_with_containers.get( + path_template.format(app_id=foreign_app.id), + headers=authenticate_console_client(test_client_with_containers, account), + ) + + assert response.status_code == 404 + def test_ops_trace_query_basic(self): query = TraceProviderQuery(tracing_provider="langfuse") assert query.tracing_provider == "langfuse" @@ -289,7 +326,7 @@ class TestOpsTraceEndpoints: ) with app.test_request_context("/?tracing_provider=langfuse"): - result = method(app_id="app-1") + result = method(app_model=MagicMock(id="app-1")) assert result == {"has_not_configured": True} @@ -308,7 +345,7 @@ class TestOpsTraceEndpoints: json={"tracing_provider": "langfuse", "tracing_config": {"api_key": "k"}}, ): with pytest.raises(BadRequest): - method(app_id="app-1") + method(app_model=MagicMock(id="app-1")) def test_trace_app_config_delete_not_found(self, app: Flask, monkeypatch: pytest.MonkeyPatch): api = ops_trace_module.TraceAppConfigApi() @@ -322,7 +359,7 @@ class TestOpsTraceEndpoints: with app.test_request_context("/?tracing_provider=langfuse"): with pytest.raises(BadRequest): - method(app_id="app-1") + method(app_model=MagicMock(id="app-1")) class TestSiteEndpoints: diff --git a/api/tests/test_containers_integration_tests/controllers/console/app/test_chat_conversation_status_count_api.py b/api/tests/test_containers_integration_tests/controllers/console/app/test_chat_conversation_status_count_api.py index 5a22f81a69..0efd77934e 100644 --- a/api/tests/test_containers_integration_tests/controllers/console/app/test_chat_conversation_status_count_api.py +++ b/api/tests/test_containers_integration_tests/controllers/console/app/test_chat_conversation_status_count_api.py @@ -6,17 +6,17 @@ import uuid from flask.testing import FlaskClient from sqlalchemy.orm import Session -from configs import dify_config from constants import HEADER_NAME_CSRF_TOKEN from graphon.enums import WorkflowExecutionStatus from libs.datetime_utils import naive_utc_now from libs.token import _real_cookie_name, generate_csrf_token -from models import Account, DifySetup, Tenant, TenantAccountJoin +from models import Account, Tenant, TenantAccountJoin from models.account import AccountStatus, TenantAccountRole, TenantStatus from models.enums import ConversationFromSource, CreatorUserRole from models.model import App, AppMode, Conversation, Message from models.workflow import WorkflowRun from services.account_service import AccountService +from tests.test_containers_integration_tests.controllers.console.helpers import ensure_dify_setup def _create_account_and_tenant(db_session: Session) -> tuple[Account, Tenant]: @@ -47,9 +47,7 @@ def _create_account_and_tenant(db_session: Session) -> tuple[Account, Tenant]: account.timezone = "UTC" db_session.commit() - dify_setup = DifySetup(version=dify_config.project.version) - db_session.add(dify_setup) - db_session.commit() + ensure_dify_setup(db_session) return account, tenant