fix(bridges): scope agent bridge-identity provisioning to the owning tenant (CHOO-2687)

[wojtyniakAQ]

Summary

Registering an agent created its platform identity on every running bridge on the instance, including bridges belonging to other tenants — a Mattermost bot and a Slack user group carrying the agent's name and description, a Discord role carrying its name. Deleting an agent did the same in reverse.

Row-level security cannot catch this and never could: no query happens. The fan-out iterates an in-memory dict that deliberately spans every tenant (the server has to run every tenant's bridges) and makes outbound platform calls from it.

The removal side is the worse half, and was not in the ticket. Creating an identity in a foreign tenant leaks a name and a description. Removing one is destructive: deleting an agent in tenant A would delete the platform bot, user group or role of a same-named agent in tenant B. Both are fixed here, because shipping one without the other is a fix that reads as complete and is not.

Both halves of the join already existed and were simply never compared — the registering request has its tenant bound, and every running bridge already knows its own. So this adds a public tenant_id on BridgeCore, a bridges_for_tenant() on the lifecycle service, and uses them at the two call sites. all_bridges() is untouched; it has no other production callers.

An unbound tenant raises rather than falling back to every bridge. Both call paths bind one before they get here, so unbound is a bug upstream — and the fallback would be precisely the leak being fixed.

Test plan

• New test_bridge_identity_tenant_scope.py: an agent registered under tenant A does not reach tenant B's bridge, and does reach tenant A's; an agent deleted under tenant A does not remove tenant B's identity for the same name.
• Both tests proved to bite. Reverting the creation fix: assert [('cross-tenant-bot', 'cross-tenant-bot desc')] == []. Reverting the removal fix: assert ['shared-name'] == [] — tenant B's identity removed by tenant A's deletion.
• Full suite: 3096 passed, 6 skipped, 1 xfailed. No existing test asserted the old fan-out, so nothing had to be weakened to accommodate this.
• just check and just typecheck pass.

Note

Teams and Telegram are unaffected — both implement identity creation as a no-op under a single-bot model — and Discord leaks only the name. The ticket implies uniform behaviour across platforms; it is Slack and Mattermost that leak name and description.

[petr-sandbox]

Very nice.

Things I checked that are correct and non-obvious, so they don't get lost:

• register_agent_with_token wraps register_agent in with tenant_scope(tenant_id) using the token's tenant, deliberately overriding whatever the caller had. So current_tenant_id() inside _create_bridge_identities reads the authoritative value, not the requesting operator's — which is what makes the POST /connectors path safe.
• The third create_agent_identity call site, BridgeCore._create_agent_identities (bridge startup), already loads its agents through tenant_session(self._session_factory, self._bridge_tenant_id). The startup fan-out was never cross-tenant.
• bridge_tenant_id: str is required on BridgeCore, so the new tenant_id property can't be None.

1. The precondition fires halfway through a mutation

In delete_agent, the order is: read agent → client_lifecycle.stop() → event_buffer.remove() → _remove_bridge_identities() → DB delete → cache invalidate → client_lifecycle.remove().

The new RuntimeError lands in the middle. An unbound caller leaves the agent stopped but still present: row in the DB, buffer cleared, client stopped but never removed. register_agent is the same shape — the agent row is committed before _create_bridge_identities, so the raise leaves a registered agent whose caller never gets the RegistrationResult with its API key.

The guard is right; its position isn't. Check current_tenant_id() at the top of both public methods, before anything is touched.

2. ConnectorCore.delete_agents() is the one method there that doesn't bind its own tenant

_register_agent (line 148) and the poll loop (line 206) both wrap in tenant_scope(self._connector_tenant_id), and the constructor comment states the contract: "each poll and each event it produces binds it for that piece of work and releases it again." delete_agents relies on ambient instead.

It works today only because its single caller chain (gateway/connectors.py → ServerConnectorLifecycle.remove) is a request. This PR makes that accidental dependency load-bearing — it's now the difference between working and RuntimeError. One with tenant_scope(self._connector_tenant_id): makes the class consistent and decouples it. Worth doing in this PR since it's the path the PR body names as already bound.

3. Neither RuntimeError is tested

They carry the "raise rather than fall back" decision, which is the interesting half of the design, and they're what will catch the next unbound caller. Two small tests.

4. all_bridges() now has no production callers

Only the five test fakes. Its own new sibling's docstring says "a caller that fans out to it without filtering would act on other tenants' bridges too" — deleting it is how you make that unrepeatable, rather than leaving a documented footgun with a warning next to it. The fakes' copies go with it. (stop_all iterates self._bridges directly and is correctly instance-wide, so nothing else needs it.)

[petr-sandbox]

Looks good.