From f3d4605dc7fcee8f0edced05fc3f846fc873091d Mon Sep 17 00:00:00 2001 From: Xiyuan Chen <52963600+GareArc@users.noreply.github.com> Date: Thu, 7 May 2026 21:45:33 -0700 Subject: [PATCH] fix(tools): scope builtin tool default-credential clear to tenant (#35888) --- .../console/workspace/tool_providers.py | 4 +-- .../tools/builtin_tools_manage_service.py | 6 ++--- .../test_builtin_tools_manage_service.py | 25 +++++++++++++++++-- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py index b38f05795a..e07063dd0d 100644 --- a/api/controllers/console/workspace/tool_providers.py +++ b/api/controllers/console/workspace/tool_providers.py @@ -875,10 +875,10 @@ class ToolBuiltinProviderSetDefaultApi(Resource): @login_required @account_initialization_required def post(self, provider): - current_user, current_tenant_id = current_account_with_tenant() + _, current_tenant_id = current_account_with_tenant() payload = BuiltinProviderDefaultCredentialPayload.model_validate(console_ns.payload or {}) return BuiltinToolManageService.set_default_provider( - tenant_id=current_tenant_id, user_id=current_user.id, provider=provider, id=payload.id + tenant_id=current_tenant_id, provider=provider, id=payload.id ) diff --git a/api/services/tools/builtin_tools_manage_service.py b/api/services/tools/builtin_tools_manage_service.py index 6797a67dde..41241538a9 100644 --- a/api/services/tools/builtin_tools_manage_service.py +++ b/api/services/tools/builtin_tools_manage_service.py @@ -406,7 +406,7 @@ class BuiltinToolManageService: return {"result": "success"} @staticmethod - def set_default_provider(tenant_id: str, user_id: str, provider: str, id: str): + def set_default_provider(tenant_id: str, provider: str, id: str): """ set default provider """ @@ -416,9 +416,9 @@ class BuiltinToolManageService: if target_provider is None: raise ValueError("provider not found") - # clear default provider + # clear default provider (tenant-scoped: only one default per provider per workspace) session.query(BuiltinToolProvider).filter_by( - tenant_id=tenant_id, user_id=user_id, provider=provider, is_default=True + tenant_id=tenant_id, provider=provider, is_default=True ).update({"is_default": False}) # set new default provider diff --git a/api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py b/api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py index 439d203c58..9fc2068498 100644 --- a/api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py +++ b/api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py @@ -174,7 +174,7 @@ class TestSetDefaultProvider: session.query.return_value.filter_by.return_value.first.return_value = None with pytest.raises(ValueError, match="provider not found"): - BuiltinToolManageService.set_default_provider("t", "u", "p", "id") + BuiltinToolManageService.set_default_provider("t", "p", "id") @patch(f"{MODULE}.Session") @patch(f"{MODULE}.db") @@ -183,12 +183,33 @@ class TestSetDefaultProvider: target = MagicMock() session.query.return_value.filter_by.return_value.first.return_value = target - result = BuiltinToolManageService.set_default_provider("t", "u", "p", "id") + result = BuiltinToolManageService.set_default_provider("t", "p", "id") assert result == {"result": "success"} assert target.is_default is True session.commit.assert_called_once() + @patch(f"{MODULE}.Session") + @patch(f"{MODULE}.db") + def test_clear_default_is_tenant_scoped_not_user_scoped(self, mock_db, mock_session_cls): + # Regression: clearing prior defaults must NOT filter by user_id, otherwise + # two workspace members can each leave their own credential as default at + # the same time (the default flag is tenant-scoped, not per-user). + session = _mock_session(mock_session_cls) + session.query.return_value.filter_by.return_value.first.return_value = MagicMock() + + BuiltinToolManageService.set_default_provider("tenant-1", "google", "cred-id") + + clear_calls = [ + call + for call in session.query.return_value.filter_by.call_args_list + if call.kwargs.get("is_default") is True + ] + assert len(clear_calls) == 1 + assert "user_id" not in clear_calls[0].kwargs + assert clear_calls[0].kwargs["tenant_id"] == "tenant-1" + assert clear_calls[0].kwargs["provider"] == "google" + class TestUpdateBuiltinToolProvider: @patch(f"{MODULE}.Session")