feat(messaging): install protocol and the tables behind it (CHOO-2626) - #474
Open
petr-sandbox wants to merge 2 commits into
Open
petr-sandbox wants to merge 2 commits into
petr-sandbox wants to merge 2 commits into
Conversation
…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>
petr-sandbox
marked this pull request as ready for review
September 15, 2026 14:11
petr-sandbox
requested review from
amaudruz and
christian-mcdermott
as code owners
September 15, 2026 14:11
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.
First of three stacked PRs splitting #435, which grew past the size anyone can review in one sitting. This one is the schema and the protocol: the tables an install lives in, the interface a platform's installer implements, and Slack's implementation of it. Nothing calls any of it yet — the routes and the service that drive it are PR B.
What a distributed app needs that the current bridge does not
Today a Slack bridge is an operator pasting their own app's tokens into a registration form. A Slack app listed in the Marketplace is the other shape: one app, owned by us, installed by a customer into their own workspace, and it forbids Socket Mode — events arrive as signed HTTP posts to URLs declared once in the app manifest, the same URLs for every customer. So an inbound event names a workspace and nothing else, and the deployment has to answer "whose is this?" before it can do anything with it.
That answer is what
messaging_installsis for.What is here
messaging_installs— one row per workspace a tenant has connected, holding the encrypted bot token, the granted scopes, and a pointer at the bridge serving it.(platform, external_workspace_id)is unique deployment-wide: the claim is the insert, so a second tenant installing into a workspace someone already holds fails in the database rather than in a check above it that cannot be made atomic with the write.messaging_install_states— one row per install attempt, burnt on redemption. The gateway and the public callback are different origins with no shared session, so the OAuthstateis a signed token naming the tenant; this table is what stops a captured one being replayed.tenant_of_messaging_install— a newSECURITY DEFINERlookup indb/tenant_lookup.py, because an inbound webhook has authenticated to nothing and every table that could name its tenant is scoped. It joins the existing closed list, andtest_tenant_exemption_allowlist.pypins who may import it.MessagingAppInstaller— the per-platform interface: build an authorize URL, redeem a code, verify a webhook signature, read a workspace id out of a payload, render a connection config. Slack's implementation is the only one, and Teams and Discord will differ enough in each of those that an interface with one implementation is still worth having.event_deliveryonSlackConnectionConfig—"socket_mode" | "webhook", defaulting to"socket_mode", so every bridge that exists today keeps the transport it has.Config —
SLACK_APP_CLIENT_ID/_CLIENT_SECRET/_SIGNING_SECRET, all nullable and cross-validated only when one is set.This is inert
A deployment that sets none of the new config has no installer registry, no public routes, and no behaviour change. The two new tables are new; the altered one is empty everywhere. Registration is the feature flag — there is no
installs_enabledsetting, because an installer exists exactly when that platform's app credentials are configured.Checks
just typecheckclean (249 files),just checkclean, single alembic headd3f6b0c95a17, full suite 3314 passed / 6 skipped / 1 xfailed.Part of CHOO-2626. Stacked: A (this) → B (the flow) → C (ending an install).
🤖 Generated with Claude Code