From 29f34848cd90f09af19d64ad155b149987992cb1 Mon Sep 17 00:00:00 2001 From: Xiyuan Chen <52963600+GareArc@users.noreply.github.com> Date: Thu, 7 May 2026 22:08:23 -0700 Subject: [PATCH] fix(tools): scope builtin tool default-credential clear to tenant (#35887) --- .../console/workspace/tool_providers.py | 4 ++-- .../tools/builtin_tools_manage_service.py | 5 ++--- .../test_builtin_tools_manage_service.py | 22 +++++++++++++++++-- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py index 34c9534de8..e653c9064c 100644 --- a/api/controllers/console/workspace/tool_providers.py +++ b/api/controllers/console/workspace/tool_providers.py @@ -876,10 +876,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 b8242ab3a5..20de1f4058 100644 --- a/api/services/tools/builtin_tools_manage_service.py +++ b/api/services/tools/builtin_tools_manage_service.py @@ -408,7 +408,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 """ @@ -422,12 +422,11 @@ 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.execute( update(BuiltinToolProvider) .where( BuiltinToolProvider.tenant_id == tenant_id, - BuiltinToolProvider.user_id == user_id, BuiltinToolProvider.provider == provider, BuiltinToolProvider.is_default.is_(True), ) 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 ce0d94398d..c210db580e 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 @@ -180,7 +180,7 @@ class TestSetDefaultProvider: session.scalar.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}.sessionmaker") @patch(f"{MODULE}.db") @@ -189,11 +189,29 @@ class TestSetDefaultProvider: target = MagicMock() session.scalar.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 + @patch(f"{MODULE}.sessionmaker") + @patch(f"{MODULE}.db") + def test_clear_default_is_tenant_scoped_not_user_scoped(self, mock_db, mock_sm_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_sessionmaker(mock_sm_cls) + session.scalar.return_value = MagicMock() + + BuiltinToolManageService.set_default_provider("tenant-1", "google", "cred-id") + + session.execute.assert_called_once() + update_stmt = session.execute.call_args.args[0] + compiled = str(update_stmt.compile(compile_kwargs={"literal_binds": True})) + assert "user_id" not in compiled + assert "tenant_id" in compiled + assert "provider" in compiled + class TestUpdateBuiltinToolProvider: @patch(f"{MODULE}.sessionmaker")