feat(slack): support alternate Web API roots (#97154)

* Allow Slack to target alternate Web API roots

* Protect Slack API URL routing

* Simplify Slack API URL routing

* Use env-only Slack API root routing

* Remove Slack client option alias

* Keep Slack API URL dotenv test inline

* Remove leftover Slack PR churn

* Block workspace Slack API URL dotenv

* Preserve Slack proxy HTTPS protocol

* Keep Slack option resolution explicit

* Remove Slack probe formatting diff

* fix(slack): preserve explicit API root precedence

* fix(slack): narrow cached write client options
This commit is contained in:
Dallin Romney
2026-06-28 18:52:37 -07:00
committed by GitHub
parent 8f9beb7766
commit 39e1be080c
8 changed files with 333 additions and 36 deletions

View File

@@ -30,12 +30,11 @@ export const SLACK_WRITE_RETRY_OPTIONS: RetryOptions = {
* Returns `undefined` when no proxy env var is configured or when Slack hosts
* are excluded by `NO_PROXY`.
*/
function resolveSlackProxyAgent(): Agent | undefined {
function resolveSlackProxyAgent(targetUrl: string): Agent | undefined {
try {
return createNodeProxyAgent({
mode: "env",
targetUrl: "https://slack.com/",
protocol: "https",
targetUrl,
});
} catch {
// Malformed proxy URL; degrade gracefully to direct connection.
@@ -43,19 +42,32 @@ function resolveSlackProxyAgent(): Agent | undefined {
}
}
function resolveSlackApiUrlFromEnv(): string | undefined {
return process.env.SLACK_API_URL?.trim() || undefined;
}
function applySlackApiUrlAndProxyOptions(options: WebClientOptions): void {
const slackApiUrl = options.slackApiUrl ?? resolveSlackApiUrlFromEnv();
const proxyTargetUrl = slackApiUrl ?? "https://slack.com/";
options.agent ??= resolveSlackProxyAgent(proxyTargetUrl);
if (slackApiUrl !== undefined) {
options.slackApiUrl = slackApiUrl;
} else {
delete options.slackApiUrl;
}
}
export function resolveSlackWebClientOptions(options: WebClientOptions = {}): WebClientOptions {
return {
...options,
agent: options.agent ?? resolveSlackProxyAgent(),
retryConfig: options.retryConfig ?? SLACK_DEFAULT_RETRY_OPTIONS,
};
const resolved: WebClientOptions = Object.assign({}, options);
applySlackApiUrlAndProxyOptions(resolved);
resolved.retryConfig ??= SLACK_DEFAULT_RETRY_OPTIONS;
return resolved;
}
export function resolveSlackWriteClientOptions(options: WebClientOptions = {}): WebClientOptions {
return {
...options,
agent: options.agent ?? resolveSlackProxyAgent(),
retryConfig: options.retryConfig ?? SLACK_WRITE_RETRY_OPTIONS,
maxRequestConcurrency: options.maxRequestConcurrency ?? 1,
};
const resolved: WebClientOptions = Object.assign({}, options);
applySlackApiUrlAndProxyOptions(resolved);
resolved.retryConfig ??= SLACK_WRITE_RETRY_OPTIONS;
resolved.maxRequestConcurrency ??= 1;
return resolved;
}

View File

@@ -2,7 +2,8 @@
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { WebClientOptions } from "@slack/web-api";
import { afterEach, beforeAll, beforeEach, describe, expect, expectTypeOf, it, vi } from "vitest";
vi.mock("@slack/web-api", () => {
const WebClient = vi.fn(function WebClientMock(
@@ -27,6 +28,7 @@ let SLACK_DEFAULT_RETRY_OPTIONS: typeof import("./client.js").SLACK_DEFAULT_RETR
let SLACK_WRITE_RETRY_OPTIONS: typeof import("./client.js").SLACK_WRITE_RETRY_OPTIONS;
let WebClient: ReturnType<typeof vi.fn>;
const SLACK_API_URL_KEYS = ["SLACK_API_URL", "OPENCLAW_SLACK_API_URL"] as const;
const PROXY_KEYS = [
"HTTPS_PROXY",
"HTTP_PROXY",
@@ -56,6 +58,22 @@ function restoreProxyEnvForTest() {
}
}
function clearSlackApiUrlEnvForTest() {
for (const key of SLACK_API_URL_KEYS) {
delete process.env[key];
}
}
function restoreSlackApiUrlEnvForTest() {
for (const key of SLACK_API_URL_KEYS) {
if (originalEnv[key] !== undefined) {
process.env[key] = originalEnv[key];
} else {
delete process.env[key];
}
}
}
function requireAgent<T extends { agent?: unknown }>(options: T): NonNullable<T["agent"]> {
if (!options.agent) {
throw new Error("expected proxy agent");
@@ -90,6 +108,11 @@ beforeAll(async () => {
beforeEach(() => {
WebClient.mockClear();
clearSlackWriteClientCacheForTest();
clearSlackApiUrlEnvForTest();
});
afterEach(() => {
restoreSlackApiUrlEnvForTest();
});
describe("slack web client config", () => {
@@ -106,6 +129,49 @@ describe("slack web client config", () => {
expect(options.retryConfig).toBe(customRetry);
});
it("uses SLACK_API_URL as the default Slack Web API root", () => {
process.env.SLACK_API_URL = " http://127.0.0.1:49152/api/ ";
expect(resolveSlackWebClientOptions().slackApiUrl).toBe("http://127.0.0.1:49152/api/");
expect(resolveSlackWriteClientOptions().slackApiUrl).toBe("http://127.0.0.1:49152/api/");
});
it("does not read OPENCLAW_SLACK_API_URL as a default Slack Web API root", () => {
process.env.OPENCLAW_SLACK_API_URL = "http://127.0.0.1:49152/api/";
expect(resolveSlackWebClientOptions().slackApiUrl).toBeUndefined();
expect(resolveSlackWriteClientOptions().slackApiUrl).toBeUndefined();
});
it("preserves Slack API URL client options over SLACK_API_URL", () => {
process.env.SLACK_API_URL = "http://127.0.0.1:49152/api/";
const explicitApiUrlOption = {
slackApiUrl: "http://127.0.0.1:49153/api/",
timeout: 1000,
};
expect(resolveSlackWebClientOptions(explicitApiUrlOption).slackApiUrl).toBe(
"http://127.0.0.1:49153/api/",
);
expect(resolveSlackWriteClientOptions(explicitApiUrlOption).slackApiUrl).toBe(
"http://127.0.0.1:49153/api/",
);
});
it("preserves Slack API URL client options when SLACK_API_URL is unset", () => {
const explicitApiUrlOption = {
slackApiUrl: "http://127.0.0.1:49153/api/",
timeout: 1000,
};
expect(resolveSlackWebClientOptions(explicitApiUrlOption).slackApiUrl).toBe(
"http://127.0.0.1:49153/api/",
);
expect(resolveSlackWriteClientOptions(explicitApiUrlOption).slackApiUrl).toBe(
"http://127.0.0.1:49153/api/",
);
});
it("passes merged options into WebClient", () => {
const customAgent = {} as never;
@@ -190,6 +256,46 @@ describe("slack web client config", () => {
expect(WebClient).toHaveBeenCalledTimes(2);
});
it("only exposes API-root options on cached write clients", () => {
expectTypeOf<NonNullable<Parameters<typeof getSlackWriteClient>[1]>>().toEqualTypeOf<
Pick<WebClientOptions, "slackApiUrl">
>();
});
it("keeps write clients separated by Slack API URL client options", () => {
clearProxyEnvForTest();
try {
const firstOptions = {
slackApiUrl: "http://127.0.0.1:49152/api/",
};
const secondOptions = {
slackApiUrl: "http://127.0.0.1:49153/api/",
};
const first = getSlackWriteClient("xoxb-test", firstOptions);
const second = getSlackWriteClient("xoxb-test", secondOptions);
expect(second).not.toBe(first);
expect(WebClient).toHaveBeenCalledTimes(2);
} finally {
restoreProxyEnvForTest();
}
});
it("keeps write clients separated by SLACK_API_URL", () => {
clearProxyEnvForTest();
try {
process.env.SLACK_API_URL = "http://127.0.0.1:49152/api/";
const first = getSlackWriteClient("xoxb-test");
process.env.SLACK_API_URL = "http://127.0.0.1:49153/api/";
const second = getSlackWriteClient("xoxb-test");
expect(second).not.toBe(first);
expect(WebClient).toHaveBeenCalledTimes(2);
} finally {
restoreProxyEnvForTest();
}
});
it("builds stable non-secret token cache keys", () => {
const token = "xoxb-sensitive-token";
const first = createSlackTokenCacheKey(token);

View File

@@ -6,6 +6,8 @@ import { resolveSlackWebClientOptions, resolveSlackWriteClientOptions } from "./
const SLACK_WRITE_CLIENT_CACHE_MAX = 32;
const slackWriteClientCache = new Map<string, WebClient>();
type SlackWriteClientCacheOptions = Pick<WebClientOptions, "slackApiUrl">;
export {
resolveSlackWebClientOptions,
resolveSlackWriteClientOptions,
@@ -25,15 +27,24 @@ export function createSlackTokenCacheKey(token: string): string {
return `sha256:${createHash("sha256").update(token).digest("base64url")}`;
}
export function getSlackWriteClient(token: string): WebClient {
function slackWriteClientCacheKey(token: string, options: SlackWriteClientCacheOptions): string {
const tokenKey = createSlackTokenCacheKey(token);
return options.slackApiUrl ? `${tokenKey}:api:${options.slackApiUrl}` : tokenKey;
}
export function getSlackWriteClient(
token: string,
options: SlackWriteClientCacheOptions = {},
): WebClient {
const resolvedOptions = resolveSlackWriteClientOptions(options);
const tokenKey = slackWriteClientCacheKey(token, resolvedOptions);
const cached = slackWriteClientCache.get(tokenKey);
if (cached) {
slackWriteClientCache.delete(tokenKey);
slackWriteClientCache.set(tokenKey, cached);
return cached;
}
const client = createSlackWriteClient(token);
const client = new WebClient(token, resolvedOptions);
if (slackWriteClientCache.size >= SLACK_WRITE_CLIENT_CACHE_MAX) {
const oldestTokenKey = slackWriteClientCache.keys().next().value;
if (oldestTokenKey) {

View File

@@ -0,0 +1,147 @@
// Slack tests cover real Web API routing behavior.
import { createServer, type Server } from "node:http";
import type { AddressInfo } from "node:net";
import { afterEach, describe, expect, it } from "vitest";
import { createSlackWebClient } from "./client.js";
const SLACK_API_URL_KEYS = ["SLACK_API_URL"] as const;
const PROXY_KEYS = [
"HTTPS_PROXY",
"HTTP_PROXY",
"https_proxy",
"http_proxy",
"NO_PROXY",
"no_proxy",
"OPENCLAW_PROXY_ACTIVE",
"OPENCLAW_PROXY_CA_FILE",
] as const;
const TEST_ENV_KEYS = [...SLACK_API_URL_KEYS, ...PROXY_KEYS] as const;
const originalEnv = { ...process.env };
type SlackApiRequest = {
authorization?: string;
method?: string;
url?: string;
};
function restoreTestEnv() {
for (const key of TEST_ENV_KEYS) {
if (originalEnv[key] !== undefined) {
process.env[key] = originalEnv[key];
} else {
delete process.env[key];
}
}
}
async function closeServer(server: Server): Promise<void> {
await new Promise<void>((resolve, reject) => {
server.close((error) => {
if (error) {
reject(error);
return;
}
resolve();
});
});
}
async function startSlackApiServer(requests: SlackApiRequest[]): Promise<{
baseUrl: string;
close(): Promise<void>;
}> {
const server = createServer((request, response) => {
requests.push({
authorization: request.headers.authorization,
method: request.method,
url: request.url,
});
request.resume();
response.writeHead(200, { "content-type": "application/json" });
response.end(
`${JSON.stringify({
ok: true,
team: "Mock Slack",
team_id: "TMOCK",
url: "https://mock.slack.test/",
user: "mock-bot",
user_id: "UMOCK",
})}\n`,
);
});
await new Promise<void>((resolve) => {
server.listen(0, "127.0.0.1", resolve);
});
const address = server.address() as AddressInfo;
return {
baseUrl: `http://127.0.0.1:${address.port}`,
close: () => closeServer(server),
};
}
afterEach(() => {
restoreTestEnv();
});
describe("Slack Web API routing", () => {
it("routes real WebClient requests to the SLACK_API_URL root", async () => {
for (const key of TEST_ENV_KEYS) {
delete process.env[key];
}
const requests: SlackApiRequest[] = [];
const server = await startSlackApiServer(requests);
try {
process.env.SLACK_API_URL = `${server.baseUrl}/api/`;
const client = createSlackWebClient("xoxb-route-proof", {
retryConfig: { retries: 0 },
timeout: 1000,
});
const result = await client.auth.test();
expect(result.ok).toBe(true);
expect(requests).toEqual([
{
authorization: "Bearer xoxb-route-proof",
method: "POST",
url: "/api/auth.test",
},
]);
} finally {
await server.close();
}
});
it("routes real WebClient requests to explicit Slack API URL options before SLACK_API_URL", async () => {
for (const key of TEST_ENV_KEYS) {
delete process.env[key];
}
const envRequests: SlackApiRequest[] = [];
const explicitRequests: SlackApiRequest[] = [];
const envServer = await startSlackApiServer(envRequests);
const explicitServer = await startSlackApiServer(explicitRequests);
try {
process.env.SLACK_API_URL = `${envServer.baseUrl}/api/`;
const client = createSlackWebClient("xoxb-route-proof", {
retryConfig: { retries: 0 },
slackApiUrl: `${explicitServer.baseUrl}/api/`,
timeout: 1000,
});
const result = await client.auth.test();
expect(result.ok).toBe(true);
expect(envRequests).toEqual([]);
expect(explicitRequests).toEqual([
{
authorization: "Bearer xoxb-route-proof",
method: "POST",
url: "/api/auth.test",
},
]);
} finally {
await explicitServer.close();
await envServer.close();
}
});
});

View File

@@ -38,6 +38,22 @@ describe("slack config schema", () => {
}
});
it("rejects Slack Web API URL config overrides", () => {
const res = SlackConfigSchema.safeParse({
apiUrl: "http://127.0.0.1:49152/api/",
accounts: { ops: { apiUrl: "http://127.0.0.1:49153/api/" } },
});
expect(res.success).toBe(false);
if (!res.success) {
expect(
res.error.issues.some(
(issue) => issue.code === "unrecognized_keys" && issue.keys.includes("apiUrl"),
),
).toBe(true);
}
});
it("accepts unfurl controls at root and account level", () => {
const res = SlackConfigSchema.safeParse({
unfurlLinks: false,

View File

@@ -50,9 +50,10 @@ type SlackAuthTestResponse = {
team?: string;
};
function resolveReadToken(params: DirectoryConfigParams): string | undefined {
function createSlackDirectoryClient(params: DirectoryConfigParams) {
const account = resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId });
return account.userToken ?? account.botToken?.trim();
const token = account.userToken ?? account.botToken?.trim();
return token ? createSlackWebClient(token) : null;
}
function normalizeQuery(value?: string | null): string {
@@ -101,11 +102,10 @@ function slackUserToDirectoryEntry(
export async function getSlackDirectorySelfLive(
params: DirectoryConfigParams,
): Promise<ChannelDirectoryEntry | null> {
const token = resolveReadToken(params);
if (!token) {
const client = createSlackDirectoryClient(params);
if (!client) {
return null;
}
const client = createSlackWebClient(token);
const auth = (await client.auth.test()) as SlackAuthTestResponse;
const userId = normalizeOptionalString(auth.user_id);
if (!userId) {
@@ -125,11 +125,10 @@ export async function getSlackDirectorySelfLive(
export async function listSlackDirectoryPeersLive(
params: DirectoryConfigParams,
): Promise<ChannelDirectoryEntry[]> {
const token = resolveReadToken(params);
if (!token) {
const client = createSlackDirectoryClient(params);
if (!client) {
return [];
}
const client = createSlackWebClient(token);
const query = normalizeQuery(params.query);
const members: SlackUser[] = [];
let cursor: string | undefined;
@@ -172,11 +171,10 @@ export async function listSlackDirectoryPeersLive(
export async function listSlackDirectoryGroupsLive(
params: DirectoryConfigParams,
): Promise<ChannelDirectoryEntry[]> {
const token = resolveReadToken(params);
if (!token) {
const client = createSlackDirectoryClient(params);
if (!client) {
return [];
}
const client = createSlackWebClient(token);
const query = normalizeQuery(params.query);
const channels: SlackChannel[] = [];
let cursor: string | undefined;

View File

@@ -286,6 +286,7 @@ describe("loadDotEnv", () => {
"CLOUDSDK_PYTHON=./attacker-python",
"EXAMPLE_API_HOST=https://evil-api.example.com",
"MINIMAX_API_HOST=https://evil.example.com",
"SLACK_API_URL=http://evil-slack.example.com/api/",
"HTTP_PROXY=http://evil-proxy:8080",
"HOMEBREW_BREW_FILE=./evil-brew/bin/brew",
"HOMEBREW_PREFIX=./evil-brew",
@@ -309,6 +310,7 @@ describe("loadDotEnv", () => {
delete process.env.CLOUDSDK_PYTHON;
delete process.env.EXAMPLE_API_HOST;
delete process.env.MINIMAX_API_HOST;
delete process.env.SLACK_API_URL;
delete process.env.HTTP_PROXY;
delete process.env.HOMEBREW_BREW_FILE;
delete process.env.HOMEBREW_PREFIX;
@@ -332,6 +334,7 @@ describe("loadDotEnv", () => {
expect(process.env.CLOUDSDK_PYTHON).toBeUndefined();
expect(process.env.EXAMPLE_API_HOST).toBeUndefined();
expect(process.env.MINIMAX_API_HOST).toBeUndefined();
expect(process.env.SLACK_API_URL).toBeUndefined();
expect(process.env.HTTP_PROXY).toBeUndefined();
expect(process.env.HOMEBREW_BREW_FILE).toBeUndefined();
expect(process.env.HOMEBREW_PREFIX).toBeUndefined();
@@ -433,6 +436,12 @@ describe("loadDotEnv", () => {
await withIsolatedEnvAndCwd(async () => {
await withDotEnvFixture(async ({ base, cwdDir }) => {
const bundledPluginsDir = path.join(base, "attacker-bundled");
const pathOverrideEnvKeys = [
"OPENCLAW_AGENT_DIR",
"OPENCLAW_BUNDLED_PLUGINS_DIR",
"OPENCLAW_OAUTH_DIR",
"PI_CODING_AGENT_DIR",
] as const;
await writeEnvFile(
path.join(cwdDir, ".env"),
[
@@ -443,17 +452,11 @@ describe("loadDotEnv", () => {
].join("\n"),
);
deleteTestEnvValue("OPENCLAW_AGENT_DIR");
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
delete process.env.OPENCLAW_OAUTH_DIR;
delete process.env.PI_CODING_AGENT_DIR;
clearEnv(pathOverrideEnvKeys);
loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true });
expect(process.env.OPENCLAW_AGENT_DIR).toBeUndefined();
expect(process.env.OPENCLAW_BUNDLED_PLUGINS_DIR).toBeUndefined();
expect(process.env.OPENCLAW_OAUTH_DIR).toBeUndefined();
expect(process.env.PI_CODING_AGENT_DIR).toBeUndefined();
expectEnvUndefined(pathOverrideEnvKeys);
});
});
});
@@ -557,6 +560,7 @@ describe("loadDotEnv", () => {
"HTTP_PROXY=http://proxy.test:8080",
"OPENCLAW_PINNED_PYTHON=/trusted/python",
"OPENCLAW_PINNED_WRITE_PYTHON=/trusted/write-python",
"SLACK_API_URL=http://trusted-slack.example.com/api/",
].join("\n"),
);
vi.spyOn(process, "cwd").mockReturnValue(cwdDir);
@@ -564,6 +568,7 @@ describe("loadDotEnv", () => {
delete process.env.HTTP_PROXY;
delete process.env.OPENCLAW_PINNED_PYTHON;
delete process.env.OPENCLAW_PINNED_WRITE_PYTHON;
delete process.env.SLACK_API_URL;
loadDotEnv({ quiet: true });
@@ -571,6 +576,7 @@ describe("loadDotEnv", () => {
expect(process.env.HTTP_PROXY).toBe("http://proxy.test:8080");
expect(process.env.OPENCLAW_PINNED_PYTHON).toBe("/trusted/python");
expect(process.env.OPENCLAW_PINNED_WRITE_PYTHON).toBe("/trusted/write-python");
expect(process.env.SLACK_API_URL).toBe("http://trusted-slack.example.com/api/");
});
});
});

View File

@@ -164,6 +164,7 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
"PROGRAMFILES(X86)",
"PROGRAMW6432",
"STATE_DIRECTORY",
"SLACK_API_URL",
"SYNOLOGY_CHAT_INCOMING_URL",
"SYNOLOGY_NAS_HOST",
"UV_PYTHON",