From bc25f2e2d37ad93b201441496c22e4920b2a0ae1 Mon Sep 17 00:00:00 2001 From: Darwin Wu Date: Thu, 10 Sep 2026 13:15:20 -0700 Subject: [PATCH] fix(auth): restrict browser credentials to Slack origins --- README.md | 3 ++ src/slack/client.ts | 14 ++++++-- src/slack/workspace-url.ts | 41 +++++++++++++++++++++++ test/client.test.ts | 67 +++++++++++++++++++++++++++++++++----- test/workspace-url.test.ts | 29 +++++++++++++++++ 5 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 src/slack/workspace-url.ts create mode 100644 test/workspace-url.test.ts diff --git a/README.md b/README.md index 7fb9c07..0b81b53 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,9 @@ export SLACK_COOKIE_D="xoxd-..." # cookie d agent-slack auth test ``` +Browser authentication accepts only canonical HTTPS workspace origins under `slack.com`; paths, +credentials, ports, and lookalike domains are rejected before tokens or cookies are sent. + Or use a standard Slack token (xoxb/xoxp): ```bash diff --git a/src/slack/client.ts b/src/slack/client.ts index fad8cbd..453cf78 100644 --- a/src/slack/client.ts +++ b/src/slack/client.ts @@ -1,5 +1,6 @@ import { WebClient } from "@slack/web-api"; import { getUserAgent } from "../lib/version.ts"; +import { normalizeSlackWorkspaceUrl } from "./workspace-url.ts"; export type SlackAuth = | { auth_type: "standard"; token: string } @@ -76,7 +77,10 @@ export class SlackApiClient { constructor(auth: SlackAuth, options?: { workspaceUrl?: string }) { this.auth = auth; - this.workspaceUrl = options?.workspaceUrl; + this.workspaceUrl = + auth.auth_type === "browser" && options?.workspaceUrl + ? normalizeSlackWorkspaceUrl(options.workspaceUrl) + : options?.workspaceUrl; if (auth.auth_type === "standard") { this.web = new WebClient(auth.token, { timeout: getSlackApiTimeoutMs(), @@ -119,7 +123,8 @@ export class SlackApiClient { attempt?: number; }): Promise> { const attempt = input.attempt ?? 0; - const url = `${input.workspaceUrl.replace(/\/$/, "")}/api/${input.method}`; + const workspaceUrl = normalizeSlackWorkspaceUrl(input.workspaceUrl); + const url = `${workspaceUrl}/api/${input.method}`; const fd = new FormData(); fd.append("token", input.auth.xoxc_token); for (const [k, v] of Object.entries(input.params)) { @@ -132,6 +137,7 @@ export class SlackApiClient { try { response = await fetch(url, { method: "POST", + redirect: "error", headers: { Cookie: `d=${encodeURIComponent(input.auth.xoxd_cookie)}`, Origin: "https://app.slack.com", @@ -208,7 +214,8 @@ export class SlackApiClient { attempt?: number; }): Promise> { const attempt = input.attempt ?? 0; - const url = `${input.workspaceUrl.replace(/\/$/, "")}/api/${input.method}`; + const workspaceUrl = normalizeSlackWorkspaceUrl(input.workspaceUrl); + const url = `${workspaceUrl}/api/${input.method}`; const cleanedEntries = Object.entries(input.params) .filter(([, v]) => v !== undefined) .map(([k, v]) => [k, typeof v === "object" ? JSON.stringify(v) : String(v)]); @@ -221,6 +228,7 @@ export class SlackApiClient { try { response = await fetch(url, { method: "POST", + redirect: "error", headers: { Cookie: `d=${encodeURIComponent(input.auth.xoxd_cookie)}`, "Content-Type": "application/x-www-form-urlencoded", diff --git a/src/slack/workspace-url.ts b/src/slack/workspace-url.ts new file mode 100644 index 0000000..3898223 --- /dev/null +++ b/src/slack/workspace-url.ts @@ -0,0 +1,41 @@ +export const SLACK_WORKSPACE_ORIGIN_ERROR = + "Workspace URL must be a canonical HTTPS Slack workspace origin " + + "(https://.slack.com)."; + +function isSlackWorkspaceHostname(hostname: string): boolean { + const normalized = hostname.toLowerCase(); + const suffix = ".slack.com"; + if (!normalized.endsWith(suffix) || normalized.length > 253) { + return false; + } + + const workspace = normalized.slice(0, -suffix.length); + return ( + workspace.length > 0 && + workspace.split(".").every((label) => /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/.test(label)) + ); +} + +export function normalizeSlackWorkspaceUrl(input: string): string { + let url: URL; + try { + url = new URL(input); + } catch { + throw new Error(SLACK_WORKSPACE_ORIGIN_ERROR); + } + + if ( + url.protocol !== "https:" || + url.username !== "" || + url.password !== "" || + url.port !== "" || + (url.pathname !== "" && url.pathname !== "/") || + url.search !== "" || + url.hash !== "" || + !isSlackWorkspaceHostname(url.hostname) + ) { + throw new Error(SLACK_WORKSPACE_ORIGIN_ERROR); + } + + return url.origin; +} diff --git a/test/client.test.ts b/test/client.test.ts index c7e395d..b305f40 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -10,6 +10,61 @@ afterEach(() => { delete process.env.AGENT_SLACK_RATE_LIMIT_MAX_WAIT_MS; }); +function browserAuth() { + return { + auth_type: "browser" as const, + xoxc_token: "xoxc-test", + xoxd_cookie: "xoxd-test", + }; +} + +describe("SlackApiClient credential destinations", () => { + test("rejects unsafe workspace origins before browser credentials can be sent", () => { + const fetchMock = mock(async () => new Response(JSON.stringify({ ok: true }))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + expect( + () => new SlackApiClient(browserAuth(), { workspaceUrl: "https://collector.example" }), + ).toThrow("canonical HTTPS Slack workspace origin"); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + test("revalidates immediately before both browser transports", async () => { + const fetchMock = mock(async () => new Response(JSON.stringify({ ok: true }))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + const client = new SlackApiClient(browserAuth(), { + workspaceUrl: "https://workspace.slack.com", + }); + + (client as unknown as { workspaceUrl: string }).workspaceUrl = "https://collector.example"; + + await expect(client.api("auth.test")).rejects.toThrow("canonical HTTPS Slack workspace origin"); + await expect(client.apiMultipart("files.createCanvas")).rejects.toThrow( + "canonical HTTPS Slack workspace origin", + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + test("rejects redirects for both browser transports", async () => { + const fetchMock = mock( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response(JSON.stringify({ ok: true })), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + const client = new SlackApiClient(browserAuth(), { + workspaceUrl: "https://workspace.slack.com", + }); + + await expect(client.api("auth.test")).resolves.toEqual({ ok: true }); + await expect(client.apiMultipart("files.createCanvas")).resolves.toEqual({ ok: true }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + for (const call of fetchMock.mock.calls) { + expect(call[1]?.redirect).toBe("error"); + } + }); +}); + describe("SlackApiClient browser multipart transport", () => { test("retries HTTP 429 responses using Retry-After", async () => { // Fail-fast defaults to 0ms; opt in to waiting so the retry path runs. @@ -35,14 +90,9 @@ describe("SlackApiClient browser multipart transport", () => { return 0; }) as unknown as typeof setTimeout; - const client = new SlackApiClient( - { - auth_type: "browser", - xoxc_token: "xoxc-test", - xoxd_cookie: "xoxd-test", - }, - { workspaceUrl: "https://workspace.slack.com" }, - ); + const client = new SlackApiClient(browserAuth(), { + workspaceUrl: "https://workspace.slack.com", + }); await expect( client.apiMultipart("files.createCanvas", { @@ -55,6 +105,7 @@ describe("SlackApiClient browser multipart transport", () => { expect(delays).toEqual([2000]); for (const call of fetchMock.mock.calls) { expect(call[1]?.body).toBeInstanceOf(FormData); + expect(call[1]?.redirect).toBe("error"); } }); }); diff --git a/test/workspace-url.test.ts b/test/workspace-url.test.ts new file mode 100644 index 0000000..5083df5 --- /dev/null +++ b/test/workspace-url.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test"; +import { normalizeSlackWorkspaceUrl } from "../src/slack/workspace-url.ts"; + +describe("Slack workspace origins", () => { + test("canonicalizes workspace and Enterprise Grid origins", () => { + expect(normalizeSlackWorkspaceUrl("https://TEAM.slack.com/")).toBe("https://team.slack.com"); + expect(normalizeSlackWorkspaceUrl("https://acme.enterprise.slack.com")).toBe( + "https://acme.enterprise.slack.com", + ); + }); + + test("rejects origins outside the Slack credential boundary", () => { + for (const value of [ + "http://team.slack.com", + "https://example.com", + "https://team.slack.com.evil.test", + "https://slack.com", + "https://user:password@team.slack.com", + "https://team.slack.com:8443", + "https://team.slack.com/archives/C123", + "https://team.slack.com?token=secret", + "https://team.slack.com#fragment", + ]) { + expect(() => normalizeSlackWorkspaceUrl(value), value).toThrow( + "canonical HTTPS Slack workspace origin", + ); + } + }); +});