Multi-tenancy Phase 4: the distributed Slack app (CHOO-2626) - #435
Draft
petr-sandbox wants to merge 15 commits into
Draft
petr-sandbox wants to merge 15 commits into
petr-sandbox wants to merge 15 commits into
Conversation
This was referenced Sep 11, 2026
petr-sandbox
force-pushed
the
work/multi-tenancy-phase4
branch
from
September 14, 2026 21:51
671b79d to
f024427
Compare
…o a tenant The table behind the official messaging app: one row per external workspace a tenant has installed us into, holding the token that install granted. Slack is the first platform to use it; the shape is deliberately platform-agnostic because Teams, Discord and Telegram each need the same row. (platform, external_workspace_id) is unique across the deployment rather than per tenant. Inbound events arrive over one public endpoint carrying a workspace id and no tenant, so a workspace claimed twice is an event with two possible destinations and no way to choose. The database decides it, because a read-then-insert in application code cannot be made atomic. The cost is that a tenant claiming a claimed workspace learns it is claimed; the alternative is a silent second claim, discovered when a customer's messages arrive in somebody else's rooms. tenant_of_messaging_install is an addition to the closed set of SECURITY DEFINER functions that are exempt from row-level security, and is meant to be argued with rather than waved through. The argument: the webhook is unauthenticated by nature and holds nothing but a workspace id, what comes back is a tenant id and never a row, and without it there is no way to bind a tenant before touching the payload — which is the only order in which the payload may be touched. It is the first lookup to take two arguments, so TenantLookup carries a tuple of them and each bind is named after the argument it fills. A lookup on the workspace id alone would answer twice the first time two platforms minted the same string, and the caller refuses an ambiguous answer rather than picking — so one customer's traffic would start failing for a reason in another platform's namespace. bridge_id is nullable: the install row is written before anything is built on it, and removing a bridge should not force the credential to be re-granted. Without the column the webhook would have to find its bridge by string-matching a workspace id inside JSON. encrypted_bot_token uses the same key as every other credential this schema stores, so it is protected against a stolen dump and not against a compromised process. A per-tenant key is a stronger boundary and a later decision. Two frozen-copy comparisons needed the other direction added. 9c41a7b0e5d8 froze the lookups and 265ed188ad6f froze the scoped-table list, and both tests compared their copy against the live module exactly; a lookup or a table added afterwards is legitimately absent from them. _ADDED_SINCE and _POLICIED_SINCE name what was added and the revision that installs it, and each is paired with a test that the named revision really renders the same DDL — loosening either comparison to a subset check would pass just as happily for a function or a policy that exists in create_all and in no migration at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…626) Everything an app install needs that does not depend on the public hostname, which is still undecided. The host appears only as `GATEWAY_PUBLIC_URL` joined to a path computed at runtime, and as the literal placeholder `HOST` in the walkthrough. The install layer is generic and the Slack implementation is the only one: `MessagingAppInstaller` is per deployment and holds *our* app's credentials, where an adapter is per bridge and holds a customer's. The seam between them is `connection_config` — an installer's last act renders the grant into the dict the adapter already takes, so the lifecycle service registers, validates and starts an installed bridge exactly as it does one an operator typed in. Registration is the feature flag: an installer exists when its credentials are configured, and the endpoints refuse when it does not. `messaging_install_states` records one in-flight install. The row is not what carries the tenant across — the signed state is — so the flow needs no ninth `SECURITY DEFINER` lookup and the closed list stays as short as it is. What the row adds is single use, which a signature cannot give: without it, replaying a captured state installs an attacker's workspace against the victim's tenant and injects messages into their rooms. Slack takes events two incompatible ways, so `SlackConnectionConfig` gains a hidden `event_delivery` discriminator and a validator refusing both half-states. A Socket Mode bridge with no app token would send fine and receive nothing — the shape of failure that reads as "Slack is quiet today" for a week. Two guards were tripped on purpose and updated deliberately: - `/messaging` joins the unauthenticated allowlist. Its callers are Slack and a browser mid-redirect, neither of which holds a credential of ours; nothing under it discloses a version, and every route proves the platform's signature before it acts. - `app_token` is no longer required by the Slack config schema, because a webhook bridge has none. The model validator, not the form, is what enforces it under Socket Mode. The walkthrough is checked against the code rather than trusted: a test parses the manifest out of the markdown and compares scopes, redirect, request URLs and every slash command against what the installer actually asks for and the paths this application actually serves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g bridge (CHOO-2626) Both legs of the OAuth flow and everything between them. Still host-agnostic: the redirect is `GATEWAY_PUBLIC_URL` joined to a path, never derived from the incoming request, which is also the only way the two legs agree on a string the platform compares byte for byte. The state token is what makes the round trip work at all. The gateway and the public callback are different origins by deployment, so a cookie is not sent to the second leg; and the callback runs with no tenant bound, so it cannot read its way to one without a ninth SECURITY DEFINER lookup. Instead the state is signed under a key derived from JWT_SECRET_KEY — derived, not reused, so an install state can never be mistaken for an agent's JWT — and names the tenant. The callback verifies it, binds that tenant, and from there is an ordinary scoped request that RLS checks like any other. The finishing order is the part worth reviewing: verify the signature, bind the tenant, burn the state and commit, exchange the code, claim the workspace, register the bridge, point the install at it. Burning before the network call, in its own transaction, costs a restarted ten-second flow when the platform is down and buys a captured state being worth nothing. Burning inside the rest would hold a row lock across a call to someone else's API. The claim is the insert, so a workspace another tenant already holds fails in the database rather than in a check racing above it — and no bridge is built for it. Two seams were wrong and are fixed here: - The rendered connection config omitted the webhook delivery mode, so it validated as a Socket Mode bridge with no app token and registration would have failed. The test had been helping it along by adding the key; it now validates exactly what `register` is handed. - A grant carried no workspace name, leaving the bridge to be named after an opaque platform id in the operator's list. Registration stays the feature flag: the installer exists when the app credentials are configured, the gateway endpoint answers 501 when it does not, and the public routes are not mounted at all. The callback answers with a page rather than a redirect. Sending the browser to the gateway works only while the person installing is an operator who can reach a private hostname, and this flow is meant to outgrow that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tenant A distributed Slack app cannot use Socket Mode, so events arrive as signed HTTP posts at one public URL shared by every tenant on the deployment. The request carries no credential of ours and no tenant: what identifies the customer is the workspace id inside a body the platform signed. The path is three steps, deliberately separate. Verify the signature over the raw bytes, before anything reads the body — parsing first is how an unauthenticated body chooses which code runs. Parse it into the two arguments Socket Mode's own listener takes, so an event over HTTP and the same event over a socket reach `dispatch_event` indistinguishable from one another, including with no tenant bound. Then resolve the workspace to a tenant through the existing `tenant_of_messaging_install`, and re-read the install row scoped to that tenant — so a wrong answer above is a miss rather than a cross-tenant read, rather than adding a ninth exemption from row-level security. Status codes are chosen for what the platform does with them, not for a reader: 401 unverified, 400 unreadable, 404 for a workspace nobody here has installed, and 503 when the bridge is not running, because a retry while a bridge restarts is better than a 200 that discards a real message and reports it handled. The event is acknowledged before it is handled, since a turn can take minutes and the platform's deadline is seconds. `url_verification` is answered inline: it names no workspace, because it arrives before anyone has installed anything, so a handshake that had to resolve a tenant could never be answered and the app could never be configured. Tests drive the real router over the real installer and real Postgres, with two tenants installed into two workspaces, asserting each event reaches one bridge and no part of it reaches the other's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t exist The install and webhook endpoints live on the agent-bridge app, so a managed ingress has to send their prefix there rather than to the gateway SPA. Also states in the distributed-app page what the flow already enforces: an install connects a workspace to a tenant that exists already, and creating one is not reachable from Slack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…c origin The install redirect and the three Slack event URLs were built from GATEWAY_PUBLIC_URL, which is the host a person lands on following an "Open in Switch Console" deeplink. On a deployment whose gateway sits on a private network that host is exactly the wrong one: Slack has to dial it from the internet over TLS. Repointing the gateway URL to satisfy Slack would have moved every deeplink with it. So the origin Slack sees is its own setting, required whenever the distributed app is configured, validated as scheme-and-host with no path and refused if it is not https — Slack will not register an http URL, and accepting one here only defers the failure to a place with no logs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An operator could configure the app in the application and had no way to say so through the chart. Adds switchCore.slackApp — off by default, since a self-hosted install wants the per-tenant Socket Mode bridge instead — with the two credentials going through the Secret like the OIDC one and the client id and public origin as plain env. Enabling it without either is a render-time failure rather than a pod that starts and cannot complete an install. Also names, in the values comments and the sample manifest, the timeout that cuts an idle MCP stream on a cloud load balancer: the chart's streaming annotations are ingress-nginx's, and an ALB idles a connection out after 60 seconds regardless of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
petr-sandbox
force-pushed
the
work/multi-tenancy-phase4
branch
from
September 15, 2026 13:29
436f01c to
c8af4d8
Compare
…O-2626) `messaging_installs` made `(platform, external_workspace_id)` unique across the deployment so that an inbound event from a workspace has exactly one tenant to go to. That is still the guarantee. The constraint enforcing it was too strong: nothing deletes an install row, so the claim outlived the install and a workspace could be connected once, ever. The error a customer got told them to remove the existing install, through a path that did not exist. What has to be unique is the set of installs that are serving. The constraint becomes a unique index over `status = 'active'`, under the same name — the store reads that name out of the integrity error to tell a claimed workspace from any other failed write, and Postgres reports a unique index by the name a constraint would have carried. `tenant_of_messaging_install` is redefined against the same predicate, and that part is load-bearing rather than tidy. It resolves a workspace to a tenant for traffic nobody has authenticated, and it was written relying on the old constraint to answer at most once; the caller refuses an ambiguous answer. Left alone, the first workspace to be installed, released and installed again would make it answer twice, and the customer's live install would stop receiving events because of one they had themselves ended. `get_for_workspace` takes the same predicate for the same reason. `ended_at` records when, alongside the `status` that says which of the two ways it ended — a decision here, or news from the platform. An operator looking at a bridge that stopped working needs to know which. `encrypted_bot_token` becomes nullable so an install that has ended can keep its record without keeping its secret. Nothing yet writes any of this: no store method ends an install and no route calls one. This is the schema that makes those possible. `_REDEFINED_SINCE` is new bookkeeping in the lookup test. Creating and dropping a function both show up as a function that is there or is not; a redefinition leaves one of the right name and signature answering a different question, which nothing about the shape of the schema reveals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An install could be made and never unmade. Disconnecting one now revokes the credential at the platform, removes the bridge, and marks the row ended so the workspace is free to be installed again — and the two events by which a platform says an install is over are read the same way. The order is deliberate. The platform is told first, so a refusal nobody understands leaves the install exactly as it was and the operator can try again rather than finding the bridge gone and the token still live. The row is ended next, which also releases its pointer at the bridge — the foreign key has no ON DELETE, so nothing could delete the bridge while the install still named it. The bridge goes last: if that fails, what is left is a credential-less bridge an operator can see and remove, not an install still claiming a workspace it has been thrown out of. Ending twice is success on both paths. Slack redelivers its uninstall event, and an operator can click disconnect on a row a redelivery ended a second earlier; neither is a fault to report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The operator's two: list this organisation's installs, ended ones included, and disconnect one. The list keeps ended rows deliberately — somebody looking at it is usually looking because something stopped working, and "nothing here" is the wrong answer to "what happened to the bridge that was here yesterday". Neither carries a token or anything derived from one. On the webhook side, an uninstall is now read before the event is routed rather than after. It has to be: resolving insists on a running bridge, and this is the one event that arrives as the bridge goes away, so the ordinary path would drop it exactly when it mattered. An event for a workspace nobody holds now answers 200 instead of 404. That is the single place this endpoint says something other than what happened, and it is deliberate: an app left in a workspace whose install ended posts for as long as someone leaves it there, the platform cannot act on the refusal, and it counts refusals against the app as a whole — so the honest answer would be paid for by every other customer's delivery. The drop is in the log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The desktop app ships a pinned copy of deploy/local/standalone-docker-compose.yml and a check fails when the two drift. Adding the distributed Slack app's four variables to the source left the copy behind. All four default to empty, so a managed server that configures none of them starts exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
Split into three stacked PRs for review — #474 (schema and protocol) → #475 (the install flow) → #476 (ending an install). Leaving this one open as the umbrella until the stack lands; close it whenever you prefer. 🤖 Generated with Claude Code |
The bundled compose now interpolates MESSAGING_PUBLIC_URL and the three
SLACK_APP_* secrets, and env-file.test.ts requires every interpolated
variable to be set in the generated .env — a `${VAR:-}` default included,
deliberately, since that is how GATEWAY_PUBLIC_URL came to be silently
omitted and the deeplink redirect disabled on every managed stack.
These four are genuinely unset rather than forgotten. A managed stack
binds to loopback, so no platform can reach its callback or event URLs,
and the credentials belong to whoever registered the app rather than to
the machine running the console. switch-core registers no installer
without them and the operator UI says as much, so the omission is
visible rather than a button that fails at Slack.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Slack allows three seconds to acknowledge an event and re-sends whatever it does not get an answer to. The payload of a retry is byte-identical to the original, so nothing below the route could tell it from a second person saying the same words — and the visible failure is an agent replying twice to one question in a customer's channel. A distributed app cannot ship without this: it is the ordinary consequence of one slow room, not an edge case. `messaging_event_receipts` is the record of what has been taken. A row is written *before* the work and the unique index arbitrates: two retries in flight together both reach the insert and exactly one survives. Recording afterwards would order the two the wrong way round — both would dispatch and the duplicate would be noticed once it no longer mattered. The claim is committed before the dispatch, because an uncommitted index entry makes a concurrent retry block for the length of an agent's turn rather than lose immediately. That ordering chooses at-most-once, which is worth stating plainly: an event claimed by a process that then dies is not retried. It is not a new loss — the route has acknowledged before handling since it was written, because the deadline is shorter than a turn — but `handled_at` is what makes it visible, so a claimed receipt that never completed can be found. Uniqueness is `(tenant_id, platform, external_event_id)` rather than deployment-wide. The two protect equally, since a Slack event id is unique in its own namespace and a workspace belongs to one tenant; the tenant-local index keeps one customer's ids out of another's namespace and makes every conflict a row the inserting tenant can see. Deduplication sits after `resolve` and not before it, which keeps the table ordinary RLS-scoped and adds no `SECURITY DEFINER` exemption. It also composes with the 503 a restarting bridge already answers: that path writes no receipt, so the retry it asks for is handled rather than dropped. Only numbered envelopes are claimed. Slack numbers Events API deliveries and retries only those; a slash command and an interaction arrive once with no id, so they dispatch unclaimed. A missing id means "the platform does not retry this", never "this was not checked". `X-Slack-Retry-Num` is read as a hint — it is outside the signature, so a forged value can do no more than put a wrong number in a log line. Its one use is a warning: a run of retries is the only signal this deployment gets that its own acknowledgements are arriving too late. Pruning is opportunistic, on the traffic that creates the rows, following the `role_leases` precedent. There is no row-deleting janitor anywhere in this backend and inventing one for a single table would be the larger change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`DELETE /gateway/collaborations/{id}` deletes every room on the bridge and
then removes the bridge. `messaging_installs.bridge_id` is a real foreign key
with no `ON DELETE`, so on an install-created bridge that ordering played out
as: rooms irreversibly deleted, Postgres refuses the bridge deletion, operator
gets a 500 — and the Slack app is still installed with a token nobody revoked.
Refuse with a 409 before any room is touched, naming the workspace and
pointing at Disconnect, which revokes the token at the platform first.
`MessagingInstallStore.get_for_bridge` asks the question from the bridge's
side. It is reached through a new `get_install_store` dependency rather than
`get_install_service`, which is None on a deployment that registered no app of
its own — install rows outlive those credentials, and a bridge built by an
install has to stay protected after they are taken away.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rd (CHOO-2626) The install flow had no operator surface: the endpoints existed and nothing called them. Adds an "Installed apps" section to the Messaging Apps page — one button per platform this deployment can install, the organisation's installs with their status and scopes, and Disconnect. The section is absent entirely when the deployment registered no app of its own and has no installs on record, which is most of them. Disconnect carries the warning that has nowhere else to live: rooms on the connection survive but become internal-only, and installing again creates a new connection rather than reattaching them. Its 502 — the platform refused to revoke, nothing was destroyed — is shown in the dialog so the operator can retry, rather than closing on it. `deleteBridge` now throws instead of returning false. It could only report "it didn't work", and a connection an install created is refused with a 409 saying to disconnect the app instead — advice the operator needs to see rather than click Delete again. Webhook-versus-socket delivery is explained in the section's copy rather than offered as a field. It is not the operator's to choose: the install sets it, and the bridge-registration form hides it for that reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. The mechanism is built and tested; the Slack app itself does not exist yet and the public hostname is not stood up. Nothing here can be turned on until both land — see What is missing below.
What this is
Today a Slack bridge is an operator pasting a bot token from an app they registered themselves. That does not scale past us: a customer cannot register our app, and a distributed app on the Slack Marketplace cannot use Socket Mode. So this adds the other way in — a customer clicks Add to Slack, and a bridge appears that is indistinguishable from one an operator typed in by hand.
The seam that makes that true is
connection_config: an installer's last act is to render the grant into exactly the dict the platform's adapter already takes, so every line of lifecycle, validation and start-up code is shared. An installed bridge and a self-registered one differ in where the token came from, and in nothing else.An install connects a workspace to a tenant that already exists. Creating a tenant is Switch Console's job and is not reachable from Slack: the flow begins with an authenticated admin inside the tenant they are installing into.
The commits
messaging_installsand the lookup. One row per installed workspace, tenant-scoped like everything else, with a deployment-wide unique constraint on(platform, external_workspace_id)— one workspace's events have exactly one destination. The bot token is encrypted at rest with the existing bridge-config key.tenant_of_messaging_installjoins the closed list ofSECURITY DEFINERlookups, because a workspace id is the only thing an inbound event carries and it has to become a tenant before anything can be scoped.The install protocol.
MessagingAppInstaller— per deployment, holding the credentials of the app we registered, as against an adapter which is per bridge and per customer. Registration is the feature flag: an installer exists when a platform's app credentials are configured, and the endpoints refuse when they are not, so a deployment cannot half-offer installs.The round trip.
beginmints a signed, single-use state naming the tenant;completeburns it before redeeming the code, exchanges the grant, writes the install and registers the bridge. The state is signed rather than a cookie because the gateway and the public callback are different origins — a cookie set on the first leg is not sent on the second. Admin-initiated: the tenant comes from the caller's session.The inbound webhook. Verify the signature over the raw bytes before anything reads the body; parse per endpoint; resolve the workspace to a tenant; re-read the install row scoped to that tenant, so a wrong answer above is a miss rather than a cross-tenant read. Events over HTTP reach
dispatch_eventin exactly the shape Socket Mode delivers, with no tenant bound, so there is one path from there down rather than two that drift.MESSAGING_PUBLIC_URL. The origin Slack dials, separate fromGATEWAY_PUBLIC_URL. The latter is the host a person lands on following an "Open in Switch Console" deeplink and is routinely private; reusing it would have meant repointing every deeplink in order to satisfy Slack. Required whenever the app is configured, refused if it carries a path or is not https.Chart wiring.
switchCore.slackApp, off by default, plus/messagingin the routed path list and in the sample Ingress. Enabling it without the credentials or the origin fails at render rather than producing a pod that starts and cannot complete an install.Two decisions worth arguing with
Status codes are for Slack, not for a reader. Slack retries a 5xx, gives up on a 4xx, and counts failures against the app as a whole. So: 401 unverified, 400 unreadable, 404 for a workspace nobody here has installed, 503 when the bridge is not running. That last one is the interesting one — a 200 there would discard a real message and report it handled, which is the failure that reads as "Slack lost a message" and is never found.
The callback replies with a page, not a redirect. Sending the browser to the gateway works today, when the installer is an operator who can reach a private hostname, and breaks the moment a customer does it. When there is somewhere to send people this becomes a redirect and the page becomes its fallback.
Tests
Real Postgres under the restricted role throughout, because row-level security is half the argument and a mock has no policies. The webhook tests drive the real router over the real Slack installer — including the real signature check — with two tenants installed into two workspaces, asserting each event reaches one bridge and no part of it reaches the other's. A single-tenant test would pass against a router that ignored the payload entirely.
What is missing
config.messaging_public_urljoined to a computed path, and as the literal placeholderHOSTindocs/old/bridges/SLACK_DISTRIBUTED_APP.md. Substituting it is a documentation change and a Slack app manifest, not a code change./messagingneeds one, and the ingress work is in a separate repo.event_deliverydiscriminator on the Slack connection config; the operator-facing side of that coexistence is not finished.🤖 Generated with Claude Code