channels: add Slack HTTP receiver for distributed-app mode#93
Conversation
Phantom Cloud tenants flip SLACK_TRANSPORT=http to receive Slack events via the central phantom-slack-events gateway instead of opening a Socket Mode WebSocket out to Slack themselves. Self-hosters keep the Socket Mode flow at slack.ts unchanged. The hybrid metadata pattern is locked: identity (team_id, installer_user_id, team_name) flows through /v1/identity.slack on the phantomd metadata gateway; secrets (slack_bot_token, slack_gateway_signing_secret) flow through /v1/secrets/<name> via the Phase C path. The new MetadataIdentityFetcher mirrors the existing MetadataSecretFetcher and parses the optional slack subfield. SlackHttpChannel reuses Bolt's App with an ExpressReceiver and signatureVerification: false. A custom Express middleware verifies the gateway's HMAC over <forwardedAt>:<eventId>:<rawBody> with a 5-minute replay window, then enforces a defense-in-depth team_id match against this.teamId before Bolt sees the body. Failures return 401 (HMAC) or 403 (team_id) before any handler runs. The existing slack-actions.ts, slack-formatter.ts, feedback.ts, progress-stream.ts, and status-reactions.ts are receiver-agnostic and reused via the same app.action()/app.event() registrations. Owner access control redesigns for HTTP mode per plan section 6.4: "any user from this.teamId" replaces the strict OWNER_SLACK_USER_ID check. The Socket Mode rejection DM is intentionally not replicated; HTTP-mode tenants are multi-user. Test count: +88 new tests (1922 vs 1834 baseline), zero plaintext bot token in any log line, error message, or test assertion.
The new factory at slack-channel-factory.ts picks SlackChannel or SlackHttpChannel based on SLACK_TRANSPORT (default "socket", "http" opts a tenant into the distributed-app flow). The factory is the one place that depends on both classes; everywhere else takes the structural SlackTransport union from slack-transport.ts. The 5-line constructor guard in slack.ts rejects transport: "http" up front so a misconfigured tenant fails loudly at construction instead of silently running Socket Mode. Scheduler delivery, /trigger endpoint, and the onboarding flow widen their type signatures from SlackChannel to SlackTransport. Both classes share the same method surface (sendDm, postToChannel, isConnected, getClient, etc.) so the call sites are byte-identical. Default SLACK_TRANSPORT=socket path is byte-identical to today's behaviour: the existing 30 SlackChannel tests pass unchanged.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 454efd42b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| teamId: identity.slack.teamId, | ||
| installerUserId: identity.slack.installerUserId, | ||
| teamName: identity.slack.teamName, | ||
| listenPort: input.port, |
There was a problem hiding this comment.
The HTTP transport path passes input.port into listenPort, but that port is already bound by the main Bun.serve server before channels connect; when SlackHttpChannel.connect() calls receiver.start(listenPort), it attempts a second bind and the Slack receiver fails to start, leaving HTTP-mode Slack ingress non-functional. This is reproducible whenever SLACK_TRANSPORT=http is enabled with the default boot flow.
Useful? React with 👍 / 👎.
| expressApp.use(`${this.listenPath}/events`, guard); | ||
| expressApp.use(`${this.listenPath}/interactivity`, guard); | ||
| expressApp.use(`${this.listenPath}/commands`, guard); |
There was a problem hiding this comment.
These app.use(...) registrations are appended after ExpressReceiver has already mounted its endpoint route handlers, so in normal Express ordering the request can be fully handled before this guard runs. In that case the gateway HMAC/team checks are bypassed while signatureVerification: false is active, which removes the intended in-process authenticity check for forwarded requests.
Useful? React with 👍 / 👎.
…w findings Closes the team_id defense-in-depth gap (Critical findings 1-2 plus Major 7) by rejecting events without parseable team_id at all three layers (middleware, per-event app_mention/message handlers, and the previously-unguarded reaction_added handler), with the single allowed exception being Slack's url_verification challenge ping. Splits Slack API egress methods into slack-egress.ts and Express helpers into slack-http-utils.ts so slack-http-receiver.ts lands at 295 lines (under the 300-line cap), and delegates the same egress helpers from slack.ts to DRY the duplicate methods. Adds redactTokens on the auth.test() failure log path so a future Bolt SDK change cannot leak a token via err.message. Aligns the identity fetcher's tenant_slug to phantomd's omitempty wire shape and documents the schema-drift policy for the hand-rolled guards. Plus four Minor cleanups: adds the slack.ts constructor-guard test (M-4), drops the dead hex-decode try/catch in the verifier (m-9), uses the SlackTransport alias in index.ts (m-12), and pins the redaction contract with a token-bearing auth.test() failure test (m-16).
1 participant
Summary
SlackHttpChannelthat accepts forwarded Slack events from the centralphantom-slack-eventsgateway over HTTPS, gated bySLACK_TRANSPORT=http. Self-hosters runningSLACK_TRANSPORT=socket(default) see no behaviour change.MetadataIdentityFetcherparallel to Phase C [codex] Fix dynamic handler test env cleanup #5'sMetadataSecretFetcher. Identity (team_id, installer_user_id, team_name) flows through/v1/identity.slack; secrets (slack_bot_token,slack_gateway_signing_secret) flow through/v1/secrets/<name>. The hybrid metadata pattern is locked.<forwardedAt>:<eventId>:<rawBody>with a 5-minute replay window, then enforces a defense-in-depthteam_idmatch againstthis.teamIdbefore Bolt sees the body. Failures return 401 (HMAC) or 403 (team_id) before any handler runs.slack-actions.ts,slack-formatter.ts,feedback.ts,progress-stream.ts, andstatus-reactions.tsunchanged. The 5-line constructor guard inslack.tsis the only edit to the Socket Mode class.this.teamIdcan talk to Phantom; the strictOWNER_SLACK_USER_IDcheck stays Socket Mode only.SlackChanneltests pass unchanged.Test plan
bun testgreen: 1922 pass, 0 fail (was 1834)bun run typecheckgreen: zero errors, noany, no@ts-ignorebun run lintgreenSLACK_TRANSPORT=socket(default) byte-identical to today's behaviourSLACK_TRANSPORT=httpwith no/v1/identity.slackinstall fails loudlyArchitecture refs
local/2026-04-24-slack-distributed-research/slack-distributed-implementation-plan.mdlines 1660-2155Phase 5a dependency
Depends on the
/v1/identity.slacksubfield shipped in:The unit test path mocks the metadata response shape, so this PR is reviewable and mergeable without waiting on a live phantomd deployment.