fix(tools): scope builtin tool default-credential clear to tenant (#35888)

This commit is contained in:
Xiyuan Chen 2026-05-07 21:45:33 -07:00 committed by GitHub
parent cd771ed909
commit f3d4605dc7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 28 additions and 7 deletions

View File

@ -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
)

View File

@ -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

View File

@ -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")