diff --git a/extensions/slack/src/client-options.ts b/extensions/slack/src/client-options.ts index 5660fd8305d..e24f0bf2abe 100644 --- a/extensions/slack/src/client-options.ts +++ b/extensions/slack/src/client-options.ts @@ -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; } diff --git a/extensions/slack/src/client.test.ts b/extensions/slack/src/client.test.ts index 02c7a333c41..564a4773937 100644 --- a/extensions/slack/src/client.test.ts +++ b/extensions/slack/src/client.test.ts @@ -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; +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(options: T): NonNullable { 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[1]>>().toEqualTypeOf< + Pick + >(); + }); + + 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); diff --git a/extensions/slack/src/client.ts b/extensions/slack/src/client.ts index 16e70e1f922..c09e2cae9a9 100644 --- a/extensions/slack/src/client.ts +++ b/extensions/slack/src/client.ts @@ -6,6 +6,8 @@ import { resolveSlackWebClientOptions, resolveSlackWriteClientOptions } from "./ const SLACK_WRITE_CLIENT_CACHE_MAX = 32; const slackWriteClientCache = new Map(); +type SlackWriteClientCacheOptions = Pick; + 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) { diff --git a/extensions/slack/src/client.web-api.test.ts b/extensions/slack/src/client.web-api.test.ts new file mode 100644 index 00000000000..5674105c960 --- /dev/null +++ b/extensions/slack/src/client.web-api.test.ts @@ -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 { + await new Promise((resolve, reject) => { + server.close((error) => { + if (error) { + reject(error); + return; + } + resolve(); + }); + }); +} + +async function startSlackApiServer(requests: SlackApiRequest[]): Promise<{ + baseUrl: string; + close(): Promise; +}> { + 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((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(); + } + }); +}); diff --git a/extensions/slack/src/config-schema.test.ts b/extensions/slack/src/config-schema.test.ts index a591d4b045a..82590576525 100644 --- a/extensions/slack/src/config-schema.test.ts +++ b/extensions/slack/src/config-schema.test.ts @@ -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, diff --git a/extensions/slack/src/directory-live.ts b/extensions/slack/src/directory-live.ts index 906dc1804f0..ca8ac48e6dd 100644 --- a/extensions/slack/src/directory-live.ts +++ b/extensions/slack/src/directory-live.ts @@ -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 { - 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 { - 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 { - 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; diff --git a/src/infra/dotenv.test.ts b/src/infra/dotenv.test.ts index 448cc2f982e..742507f80cf 100644 --- a/src/infra/dotenv.test.ts +++ b/src/infra/dotenv.test.ts @@ -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/"); }); }); }); diff --git a/src/infra/dotenv.ts b/src/infra/dotenv.ts index cfa022af226..5463fdaf7de 100644 --- a/src/infra/dotenv.ts +++ b/src/infra/dotenv.ts @@ -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",