Skip to content

Commit 0c6f0c5

Browse files
authored
channels: add Slack HTTP receiver for distributed-app mode (#93)
* channels: add Slack HTTP receiver for distributed-app mode 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. * channels: wire SLACK_TRANSPORT dispatch into Phantom boot 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. * channels: tighten Slack 5b team_id defense-in-depth and address review 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 parent 47137eb commit 0c6f0c5

20 files changed

Lines changed: 2572 additions & 141 deletions
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
2+
import type { ChannelsConfig } from "../../config/schemas.ts";
3+
4+
// Mock the Slack Bolt SDK before importing the factory so the underlying
5+
// channel constructors don't reach real network. The receiver test file
6+
// already mocks @slack/bolt, but module mocks are scoped per file under bun.
7+
const mockApp = mock(() => ({
8+
event: () => {},
9+
action: () => {},
10+
client: {
11+
auth: { test: () => Promise.resolve({ user_id: "U_BOT" }) },
12+
chat: { postMessage: () => Promise.resolve({ ts: "1.0" }), update: () => Promise.resolve({ ok: true }) },
13+
conversations: { open: () => Promise.resolve({ channel: { id: "D1" } }) },
14+
reactions: { add: () => Promise.resolve({ ok: true }), remove: () => Promise.resolve({ ok: true }) },
15+
},
16+
}));
17+
18+
const mockReceiver = {
19+
app: { use: () => {} },
20+
start: () => Promise.resolve({}),
21+
stop: () => Promise.resolve(),
22+
};
23+
24+
mock.module("@slack/bolt", () => ({
25+
App: mockApp,
26+
ExpressReceiver: mock(() => mockReceiver),
27+
}));
28+
29+
const { createSlackChannel, readSlackTransportFromEnv } = await import("../slack-channel-factory.ts");
30+
const { SlackChannel } = await import("../slack.ts");
31+
const { SlackHttpChannel } = await import("../slack-http-receiver.ts");
32+
33+
const SOCKET_CONFIG: ChannelsConfig = {
34+
slack: {
35+
enabled: true,
36+
bot_token: "xoxb-1",
37+
app_token: "xapp-1",
38+
owner_user_id: "U_OWNER",
39+
},
40+
};
41+
42+
const HTTP_IDENTITY = {
43+
slack: {
44+
teamId: "T9TK3CUKW",
45+
installerUserId: "U_INSTALLER",
46+
teamName: "Acme Corp",
47+
installedAt: "2026-04-25T00:00:00Z",
48+
},
49+
};
50+
51+
const SECRET_RESPONSES: Record<string, string> = {
52+
slack_bot_token: "xoxb-from-metadata",
53+
slack_gateway_signing_secret: "0123456789abcdef".repeat(4),
54+
};
55+
56+
describe("readSlackTransportFromEnv", () => {
57+
test("returns 'socket' when SLACK_TRANSPORT is unset", () => {
58+
expect(readSlackTransportFromEnv({} as NodeJS.ProcessEnv)).toBe("socket");
59+
});
60+
61+
test("returns 'socket' when SLACK_TRANSPORT='socket'", () => {
62+
expect(readSlackTransportFromEnv({ SLACK_TRANSPORT: "socket" } as NodeJS.ProcessEnv)).toBe("socket");
63+
});
64+
65+
test("returns 'http' when SLACK_TRANSPORT='http'", () => {
66+
expect(readSlackTransportFromEnv({ SLACK_TRANSPORT: "http" } as NodeJS.ProcessEnv)).toBe("http");
67+
});
68+
69+
test("throws on an unknown value", () => {
70+
expect(() => readSlackTransportFromEnv({ SLACK_TRANSPORT: "garbage" } as NodeJS.ProcessEnv)).toThrow(
71+
/Unknown SLACK_TRANSPORT/,
72+
);
73+
});
74+
75+
test("ignores leading/trailing whitespace via trim()", () => {
76+
expect(readSlackTransportFromEnv({ SLACK_TRANSPORT: " http " } as NodeJS.ProcessEnv)).toBe("http");
77+
});
78+
});
79+
80+
describe("createSlackChannel", () => {
81+
beforeEach(() => {
82+
mockApp.mockClear();
83+
});
84+
85+
afterEach(() => {
86+
// Nothing to reset; module mock is sticky for the file's lifetime.
87+
});
88+
89+
test("transport=socket with no Slack creds returns null", async () => {
90+
const ch = await createSlackChannel({
91+
transport: "socket",
92+
channelsConfig: null,
93+
port: 3100,
94+
});
95+
expect(ch).toBeNull();
96+
});
97+
98+
test("transport=socket with disabled Slack creds returns null", async () => {
99+
const disabled: ChannelsConfig = {
100+
slack: { enabled: false, bot_token: "x", app_token: "y" },
101+
};
102+
const ch = await createSlackChannel({
103+
transport: "socket",
104+
channelsConfig: disabled,
105+
port: 3100,
106+
});
107+
expect(ch).toBeNull();
108+
});
109+
110+
test("transport=socket with valid creds returns a SlackChannel instance", async () => {
111+
const ch = await createSlackChannel({
112+
transport: "socket",
113+
channelsConfig: SOCKET_CONFIG,
114+
port: 3100,
115+
});
116+
expect(ch).toBeInstanceOf(SlackChannel);
117+
});
118+
119+
test("transport=http with no slack subfield in identity throws a clear error", async () => {
120+
const idFetcher = { get: () => Promise.resolve({}) };
121+
const secFetcher = { get: () => Promise.resolve("unused") };
122+
await expect(
123+
createSlackChannel({
124+
transport: "http",
125+
channelsConfig: null,
126+
port: 3100,
127+
identityFetcher: idFetcher,
128+
secretsFetcher: secFetcher,
129+
}),
130+
).rejects.toThrow(/SLACK_TRANSPORT=http requires a Slack install/);
131+
});
132+
133+
test("transport=http with slack identity and metadata secrets returns a SlackHttpChannel", async () => {
134+
const idFetcher = { get: () => Promise.resolve(HTTP_IDENTITY) };
135+
const secFetcher = { get: (name: string) => Promise.resolve(SECRET_RESPONSES[name] ?? "") };
136+
const ch = await createSlackChannel({
137+
transport: "http",
138+
channelsConfig: null,
139+
port: 3100,
140+
identityFetcher: idFetcher,
141+
secretsFetcher: secFetcher,
142+
});
143+
expect(ch).toBeInstanceOf(SlackHttpChannel);
144+
// Cast to the concrete type to inspect the wired identity.
145+
const httpCh = ch as InstanceType<typeof SlackHttpChannel>;
146+
expect(httpCh.getTeamId()).toBe("T9TK3CUKW");
147+
expect(httpCh.getInstallerUserId()).toBe("U_INSTALLER");
148+
expect(httpCh.getTeamName()).toBe("Acme Corp");
149+
});
150+
151+
test("transport=http fetches both required secrets in parallel", async () => {
152+
const requested: string[] = [];
153+
const idFetcher = { get: () => Promise.resolve(HTTP_IDENTITY) };
154+
const secFetcher = {
155+
get: (name: string) => {
156+
requested.push(name);
157+
return Promise.resolve(SECRET_RESPONSES[name] ?? "");
158+
},
159+
};
160+
await createSlackChannel({
161+
transport: "http",
162+
channelsConfig: null,
163+
port: 3100,
164+
identityFetcher: idFetcher,
165+
secretsFetcher: secFetcher,
166+
});
167+
expect(requested).toContain("slack_bot_token");
168+
expect(requested).toContain("slack_gateway_signing_secret");
169+
});
170+
171+
test("transport=http never reads bot_token or app_token from channels.yaml", async () => {
172+
// Even when channels.yaml has socket creds, the http path uses metadata.
173+
const idFetcher = { get: () => Promise.resolve(HTTP_IDENTITY) };
174+
const secretCalls: string[] = [];
175+
const secFetcher = {
176+
get: (name: string) => {
177+
secretCalls.push(name);
178+
return Promise.resolve(SECRET_RESPONSES[name] ?? "");
179+
},
180+
};
181+
const ch = await createSlackChannel({
182+
transport: "http",
183+
channelsConfig: SOCKET_CONFIG,
184+
port: 3100,
185+
identityFetcher: idFetcher,
186+
secretsFetcher: secFetcher,
187+
});
188+
expect(ch).toBeInstanceOf(SlackHttpChannel);
189+
// The http path always pulls from metadata; SOCKET_CONFIG.slack.bot_token
190+
// is irrelevant. Pin this so a future refactor cannot accidentally
191+
// fall back to channels.yaml on a half-provisioned tenant.
192+
expect(secretCalls.length).toBe(2);
193+
});
194+
195+
test("default identity fetcher uses the link-local URL", async () => {
196+
// We don't need to run the full http path here, just assert that the
197+
// factory wires the documented default base URL when no custom URL is
198+
// passed. The fetcher class is responsible for the URL it constructs.
199+
const idFetcher = { get: () => Promise.resolve(HTTP_IDENTITY) };
200+
const secFetcher = { get: (name: string) => Promise.resolve(SECRET_RESPONSES[name] ?? "") };
201+
// Pin the contract: passing through metadataBaseUrl propagates.
202+
const ch = await createSlackChannel({
203+
transport: "http",
204+
channelsConfig: null,
205+
port: 3100,
206+
metadataBaseUrl: "http://gateway.test",
207+
identityFetcher: idFetcher,
208+
secretsFetcher: secFetcher,
209+
});
210+
expect(ch).toBeInstanceOf(SlackHttpChannel);
211+
});
212+
});

0 commit comments

Comments
 (0)