From b90fe73c9650fdaf6de58539e07557571675f555 Mon Sep 17 00:00:00 2001 From: -LAN- Date: Fri, 10 Apr 2026 11:23:32 +0800 Subject: [PATCH] fix(api): prevent cross-tenant external API use-check disclosure (#34744) --- api/controllers/console/datasets/external.py | 3 +- api/services/external_knowledge_service.py | 15 ++++++--- .../console/datasets/test_external.py | 33 ++++++++++++++++--- .../services/external_dataset_service.py | 5 +-- .../services/test_external_dataset_service.py | 10 ++++-- 5 files changed, 51 insertions(+), 15 deletions(-) diff --git a/api/controllers/console/datasets/external.py b/api/controllers/console/datasets/external.py index f3866f6aef..e513e8c8f9 100644 --- a/api/controllers/console/datasets/external.py +++ b/api/controllers/console/datasets/external.py @@ -227,10 +227,11 @@ class ExternalApiUseCheckApi(Resource): @login_required @account_initialization_required def get(self, external_knowledge_api_id): + _, current_tenant_id = current_account_with_tenant() external_knowledge_api_id = str(external_knowledge_api_id) external_knowledge_api_is_using, count = ExternalDatasetService.external_knowledge_api_use_check( - external_knowledge_api_id + external_knowledge_api_id, current_tenant_id ) return {"is_using": external_knowledge_api_is_using, "count": count}, 200 diff --git a/api/services/external_knowledge_service.py b/api/services/external_knowledge_service.py index d30ec940f5..2bf1afba3e 100644 --- a/api/services/external_knowledge_service.py +++ b/api/services/external_knowledge_service.py @@ -148,18 +148,23 @@ class ExternalDatasetService: db.session.commit() @staticmethod - def external_knowledge_api_use_check(external_knowledge_api_id: str) -> tuple[bool, int]: + def external_knowledge_api_use_check(external_knowledge_api_id: str, tenant_id: str) -> tuple[bool, int]: + """ + Return usage for an external knowledge API within a single tenant. + + The caller already scopes access by tenant, so this query must do the + same; otherwise the endpoint becomes a cross-tenant UUID oracle. + """ count = ( db.session.scalar( select(func.count(ExternalKnowledgeBindings.id)).where( - ExternalKnowledgeBindings.external_knowledge_api_id == external_knowledge_api_id + ExternalKnowledgeBindings.external_knowledge_api_id == external_knowledge_api_id, + ExternalKnowledgeBindings.tenant_id == tenant_id, ) ) or 0 ) - if count > 0: - return True, count - return False, 0 + return count > 0, count @staticmethod def get_external_knowledge_binding_with_dataset_id(tenant_id: str, dataset_id: str) -> ExternalKnowledgeBindings: diff --git a/api/tests/unit_tests/controllers/console/datasets/test_external.py b/api/tests/unit_tests/controllers/console/datasets/test_external.py index 161d0c41e8..514bbbe040 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_external.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_external.py @@ -1,3 +1,4 @@ +from importlib import import_module from unittest.mock import MagicMock, PropertyMock, patch import pytest @@ -11,6 +12,7 @@ from controllers.console.datasets.external import ( BedrockRetrievalApi, ExternalApiTemplateApi, ExternalApiTemplateListApi, + ExternalApiUseCheckApi, ExternalDatasetCreateApi, ExternalKnowledgeHitTestingApi, ) @@ -19,6 +21,8 @@ from services.external_knowledge_service import ExternalDatasetService from services.hit_testing_service import HitTestingService from services.knowledge_service import ExternalDatasetTestService +external_controller = import_module("controllers.console.datasets.external") + def unwrap(func): while hasattr(func, "__wrapped__"): @@ -44,10 +48,11 @@ def current_user(): @pytest.fixture(autouse=True) -def mock_auth(mocker, current_user): - mocker.patch( - "controllers.console.datasets.external.current_account_with_tenant", - return_value=(current_user, "tenant-1"), +def mock_auth(monkeypatch, current_user): + monkeypatch.setattr( + external_controller, + "current_account_with_tenant", + lambda: (current_user, "tenant-1"), ) @@ -136,6 +141,26 @@ class TestExternalApiTemplateApi: method(api, "api-id") +class TestExternalApiUseCheckApi: + def test_get_scopes_usage_check_to_current_tenant(self, app): + api = ExternalApiUseCheckApi() + method = unwrap(api.get) + + with ( + app.test_request_context("/"), + patch.object( + ExternalDatasetService, + "external_knowledge_api_use_check", + return_value=(True, 2), + ) as mock_use_check, + ): + response, status = method(api, "api-id") + + assert status == 200 + assert response == {"is_using": True, "count": 2} + mock_use_check.assert_called_once_with("api-id", "tenant-1") + + class TestExternalDatasetCreateApi: def test_create_success(self, app): api = ExternalDatasetCreateApi() diff --git a/api/tests/unit_tests/services/external_dataset_service.py b/api/tests/unit_tests/services/external_dataset_service.py index 5848603ab8..dd41c0c97e 100644 --- a/api/tests/unit_tests/services/external_dataset_service.py +++ b/api/tests/unit_tests/services/external_dataset_service.py @@ -396,10 +396,11 @@ class TestExternalDatasetServiceUsageAndBindings: mock_db_session.scalar.return_value = 3 - in_use, count = ExternalDatasetService.external_knowledge_api_use_check("api-1") + in_use, count = ExternalDatasetService.external_knowledge_api_use_check("api-1", "tenant-1") assert in_use is True assert count == 3 + assert "tenant_id" in str(mock_db_session.scalar.call_args.args[0]) def test_external_knowledge_api_use_check_not_in_use(self, mock_db_session: MagicMock): """ @@ -408,7 +409,7 @@ class TestExternalDatasetServiceUsageAndBindings: mock_db_session.scalar.return_value = 0 - in_use, count = ExternalDatasetService.external_knowledge_api_use_check("api-1") + in_use, count = ExternalDatasetService.external_knowledge_api_use_check("api-1", "tenant-1") assert in_use is False assert count == 0 diff --git a/api/tests/unit_tests/services/test_external_dataset_service.py b/api/tests/unit_tests/services/test_external_dataset_service.py index 7c8dab5029..0777e6a8a4 100644 --- a/api/tests/unit_tests/services/test_external_dataset_service.py +++ b/api/tests/unit_tests/services/test_external_dataset_service.py @@ -974,26 +974,29 @@ class TestExternalDatasetServiceAPIUseCheck: """Test API use check when API has one binding.""" # Arrange api_id = "api-123" + tenant_id = "tenant-123" mock_db.session.scalar.return_value = 1 # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id, tenant_id) # Assert assert in_use is True assert count == 1 + assert "tenant_id" in str(mock_db.session.scalar.call_args.args[0]) @patch("services.external_knowledge_service.db") def test_external_knowledge_api_use_check_in_use_multiple(self, mock_db, factory): """Test API use check with multiple bindings.""" # Arrange api_id = "api-123" + tenant_id = "tenant-123" mock_db.session.scalar.return_value = 10 # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id, tenant_id) # Assert assert in_use is True @@ -1004,11 +1007,12 @@ class TestExternalDatasetServiceAPIUseCheck: """Test API use check when API is not in use.""" # Arrange api_id = "api-123" + tenant_id = "tenant-123" mock_db.session.scalar.return_value = 0 # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(api_id, tenant_id) # Assert assert in_use is False