From 7b22d813ed14282e078aa4749dda4a183ae4827b Mon Sep 17 00:00:00 2001 From: GareArc Date: Thu, 7 May 2026 02:30:27 -0700 Subject: [PATCH] fix(tools): scope builtin tool default-credential clear to tenant When two workspace members each set their own credential as the default for the same provider, both rows ended up with is_default=True for the same (tenant_id, provider). The clear UPDATE was filtered by user_id, so it only reset the caller's own defaults and left other members' defaults intact. The consumer (`get_builtin_provider`) is tenant-scoped and picks an arbitrary winner via the created_at tiebreak when multiple defaults exist, producing user-visible inconsistencies. Drop user_id from the clear filter so default selection becomes properly tenant-scoped, matching `DatasourceProviderService.set_default_datasource_provider`. Adds a regression test that asserts the compiled UPDATE WHERE clause contains tenant_id/provider but not user_id. --- .../tools/builtin_tools_manage_service.py | 3 +-- .../tools/test_builtin_tools_manage_service.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/api/services/tools/builtin_tools_manage_service.py b/api/services/tools/builtin_tools_manage_service.py index b8242ab3a5..9641c414c5 100644 --- a/api/services/tools/builtin_tools_manage_service.py +++ b/api/services/tools/builtin_tools_manage_service.py @@ -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..a05e9ccd02 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 @@ -194,6 +194,24 @@ class TestSetDefaultProvider: 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", "user-A", "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")