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

This commit is contained in:
Xiyuan Chen 2026-05-07 22:08:23 -07:00 committed by GitHub
parent 1b0d4637b3
commit 29f34848cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 24 additions and 7 deletions

View File

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

View File

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

View File

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