From 3d5188abf2930cabc66e8c6b9c9c561be67741aa Mon Sep 17 00:00:00 2001 From: Darwin Wu Date: Thu, 23 Jul 2026 19:00:43 -0700 Subject: [PATCH] feat(users): verify mention targets directly Slack IDs were accepted without checking whether they identify an active human. Add user resolve for IDs and emails. It uses direct Slack lookups and emits mentions only when the whole batch passes. Names and handles are rejected; no directory scan is used. --- README.md | 6 + skills/agent-slack/SKILL.md | 3 +- src/cli/user-command.ts | 56 +++++++++ src/slack/strict-user-resolution.ts | 122 ++++++++++++++++++ test/strict-user-resolution.test.ts | 173 ++++++++++++++++++++++++++ test/user-command.test.ts | 185 ++++++++++++++++++++++++++++ 6 files changed, 544 insertions(+), 1 deletion(-) create mode 100644 src/slack/strict-user-resolution.ts create mode 100644 test/strict-user-resolution.test.ts create mode 100644 test/user-command.test.ts diff --git a/README.md b/README.md index e081803..fe901b8 100644 --- a/README.md +++ b/README.md @@ -472,10 +472,16 @@ agent-slack user list --workspace "https://workspace.slack.com" --limit 200 | jq agent-slack user get U12345678 --workspace "https://workspace.slack.com" | jq . agent-slack user get "@alice" --workspace "https://workspace.slack.com" | jq . +# Verify active humans before constructing Slack mentions +agent-slack user resolve U12345678 bob@example.com \ + --workspace "https://workspace.slack.com" + # Open a DM or group DM with one to eight other users (the caller is implicit) agent-slack user dm-open "@alice" "@bob" --workspace "https://workspace.slack.com" | jq . ``` +`user resolve` accepts only canonical U/W user IDs and email addresses. It uses direct lookups and emits mentions only when every input resolves to an active human; otherwise it exits nonzero and emits none. + ### Unreads (inbox view) See all unread messages across channels, DMs, and threads in one place: diff --git a/skills/agent-slack/SKILL.md b/skills/agent-slack/SKILL.md index 7512344..1ff8d33 100644 --- a/skills/agent-slack/SKILL.md +++ b/skills/agent-slack/SKILL.md @@ -1,6 +1,6 @@ --- name: agent-slack -description: "Slack CLI for agents: read URLs/threads/history/unreads/later/canvases/workflows, create canvases from Markdown, search messages/files, download attachments, lookup users, list/create/invite channels, open DMs, compose messages, manage Slack-native drafts, schedule sends, and explicit sends/edits/deletes/reactions/mark-read/uploads." +description: "Slack CLI for agents: read URLs/threads/history/unreads/later/canvases/workflows, create canvases from Markdown, search messages/files, download attachments, lookup users, resolve verified human mentions, list/create/invite channels, open DMs, compose messages, manage Slack-native drafts, schedule sends, and explicit sends/edits/deletes/reactions/mark-read/uploads." --- # agent-slack @@ -20,6 +20,7 @@ If a capability named here is absent from installed help, report version skew in - Read and search freely. - Perform write actions only when explicitly requested: sends, edits, deletes, reactions, invitations, channel or canvas creation, mark-read operations, scheduling or canceling delivery, uploads, Later state/reminder changes, DM/group-DM creation, and `workflow run`. Workflow runs can execute downstream actions. +- Never scan the full user directory to resolve a mention. `user resolve` accepts canonical user IDs and emails; use its mentions only when `safe_to_mention` is true. - For compose- or review-only requests, return proposed text without invoking Slack, or use `message draft create` to add a Slack-native draft the user can review and send (nothing is posted). `message compose` is send-capable; use it only when the user explicitly asks to open the interactive editor. In CI or another noninteractive environment, do not invoke it without separate authorization to send immediately: CI skips the editor and sends supplied text. - With `AGENT_SLACK_SAFE_MODE=1` (or the global `--safe-mode` flag) set, safe mode is enforced at the tool level: `message send` is redirected to the draft editor and `message edit`/`message delete` are blocked. Use it when nothing should post without human review. diff --git a/src/cli/user-command.ts b/src/cli/user-command.ts index a47f71b..053c788 100644 --- a/src/cli/user-command.ts +++ b/src/cli/user-command.ts @@ -2,6 +2,14 @@ import type { Command } from "commander"; import type { CliContext } from "./context.ts"; import { pruneEmpty } from "../lib/compact-json.ts"; import { getDmChannelForUsers, getUser, listUsers } from "../slack/users.ts"; +import { + resolveStrictUserIdentities, + type UserResolution, +} from "../slack/strict-user-resolution.ts"; + +const USER_RESOLUTION_ERROR = "Unable to resolve users safely."; +const SLACK_WORKSPACE_HOST = + /^(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+(?:slack\.com|slack-gov\.com)$/; export function registerUserCommand(input: { program: Command; ctx: CliContext }): void { const userCmd = input.program.command("user").description("Workspace user directory"); @@ -41,6 +49,37 @@ export function registerUserCommand(input: { program: Command; ctx: CliContext } } }); + userCmd + .command("resolve") + .description("Verify active humans by Slack user ID or email") + .argument("", "Canonical U/W user IDs or email addresses") + .option( + "--workspace ", + "Workspace selector (full URL or unique substring; required if you have multiple workspaces)", + ) + .action(async (...args) => { + const [identities, options] = args as [string[], { workspace?: string }]; + try { + const workspaceUrl = input.ctx.effectiveWorkspaceUrl(options.workspace); + const output = await input.ctx.withAutoRefresh({ + workspaceUrl, + work: async () => { + const { client, workspace_url } = await input.ctx.getClientForWorkspace(workspaceUrl); + const workspace = requireSlackWorkspaceOrigin(workspace_url); + const resolution = await resolveStrictUserIdentities({ client, identities }); + return { workspace, resolution }; + }, + }); + printUserResolution(output.workspace, output.resolution); + if (!output.resolution.safe_to_mention) { + process.exitCode = 1; + } + } catch { + console.error(USER_RESOLUTION_ERROR); + process.exitCode = 1; + } + }); + userCmd .command("get") .description("Get a single workspace user") @@ -90,3 +129,20 @@ export function registerUserCommand(input: { program: Command; ctx: CliContext } } }); } + +function requireSlackWorkspaceOrigin(workspaceUrl: string | undefined): string { + const url = workspaceUrl && URL.canParse(workspaceUrl) ? new URL(workspaceUrl) : null; + if ( + !url || + url.protocol !== "https:" || + url.origin !== workspaceUrl || + !SLACK_WORKSPACE_HOST.test(url.hostname) + ) { + throw new Error("Resolved workspace is not a canonical Slack origin"); + } + return workspaceUrl; +} + +function printUserResolution(workspace: string, resolution: UserResolution): void { + console.log(JSON.stringify(pruneEmpty({ workspace, ...resolution }), null, 2)); +} diff --git a/src/slack/strict-user-resolution.ts b/src/slack/strict-user-resolution.ts new file mode 100644 index 0000000..052f24a --- /dev/null +++ b/src/slack/strict-user-resolution.ts @@ -0,0 +1,122 @@ +import { isRecord } from "../lib/object-type-guards.ts"; +import type { SlackApiClient } from "./client.ts"; +import { isUserId } from "./user-id.ts"; + +type Identity = { kind: "id"; value: string } | { kind: "email"; value: string }; + +type ResolutionResult = { + index: number; + status: "resolved" | "unresolved"; + mention?: `<@${string}>`; +}; + +type InternalResult = ResolutionResult & { userId?: string }; + +export type UserResolution = { + safe_to_mention: boolean; + results: ResolutionResult[]; +}; + +const EMAIL_PATTERN = /^[^@\s]+@[^@\s]+\.[^@\s]+$/; + +export async function resolveStrictUserIdentities(input: { + client: SlackApiClient; + identities: string[]; +}): Promise { + if (input.identities.length === 0) { + throw new Error("At least one user identity is required"); + } + + const identities = input.identities.map(parseIdentity); + const results: InternalResult[] = []; + + for (const [index, identity] of identities.entries()) { + let response: Record; + try { + response = + identity.kind === "id" + ? await input.client.api("users.info", { user: identity.value }) + : await input.client.api("users.lookupByEmail", { email: identity.value }); + } catch (error) { + if (isNotFoundError(error, identity.kind)) { + results.push({ index, status: "unresolved" }); + continue; + } + throw error; + } + + const userId = parseVerifiedUserId(response.user); + const matchesInput = userId && (identity.kind === "email" || userId === identity.value); + results.push( + matchesInput ? { index, status: "resolved", userId } : { index, status: "unresolved" }, + ); + } + + if (results.every((result) => result.status === "resolved")) { + return { + safe_to_mention: true, + results: results.map((result) => ({ + index: result.index, + status: "resolved", + mention: `<@${result.userId!}>`, + })), + }; + } + + return { + safe_to_mention: false, + results: results.map(({ index, status }) => ({ index, status })), + }; +} + +function parseIdentity(input: string, index: number): Identity { + const value = input.trim(); + if (isUserId(value)) { + return { kind: "id", value }; + } + if (EMAIL_PATTERN.test(value)) { + return { kind: "email", value: value.toLowerCase() }; + } + throw new Error(`User identity at index ${index} must be a canonical U/W ID or email`); +} + +function parseVerifiedUserId(value: unknown): string | null { + if (!isRecord(value) || Array.isArray(value)) { + return null; + } + const id = typeof value.id === "string" && isUserId(value.id) ? value.id : null; + const profile = isRecord(value.profile) && !Array.isArray(value.profile) ? value.profile : null; + if (!id || id === "USLACKBOT" || !profile || value.deleted !== false || value.is_bot !== false) { + return null; + } + + const inactiveOrBotSignals = [ + value.is_connector_bot, + value.is_workflow_bot, + value.is_agentforce_bot, + value.is_invited_user, + value.suspended, + value.is_forgotten, + profile.is_agentforce_bot, + profile.is_sidekick_bot, + ]; + if (inactiveOrBotSignals.some((signal) => signal != null && signal !== false)) { + return null; + } + if (profile.bot_id != null && profile.bot_id !== "") { + return null; + } + + return id; +} + +function isNotFoundError(error: unknown, kind: Identity["kind"]): boolean { + const expected = kind === "id" ? "user_not_found" : "users_not_found"; + if (error instanceof Error && error.message === expected) { + return true; + } + if (!isRecord(error) || !isRecord(error.data) || Array.isArray(error.data)) { + return false; + } + return error.data.error === expected; +} diff --git a/test/strict-user-resolution.test.ts b/test/strict-user-resolution.test.ts new file mode 100644 index 0000000..5839d11 --- /dev/null +++ b/test/strict-user-resolution.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, test } from "bun:test"; +import type { SlackApiClient } from "../src/slack/client.ts"; +import { resolveStrictUserIdentities } from "../src/slack/strict-user-resolution.ts"; + +type ApiCall = { method: string; params: Record }; + +function user(id: string, fields: Record = {}): Record { + const profile = + fields.profile && typeof fields.profile === "object" && !Array.isArray(fields.profile) + ? fields.profile + : {}; + return { + id, + deleted: false, + is_bot: false, + ...fields, + profile: { ...profile }, + }; +} + +function client( + handler: (method: string, params: Record) => Promise>, +): SlackApiClient { + return { api: handler } as unknown as SlackApiClient; +} + +describe("strict batch user resolution", () => { + test("uses direct ID and email lookups and preserves input order", async () => { + const calls: ApiCall[] = []; + const result = await resolveStrictUserIdentities({ + client: client(async (method, params) => { + calls.push({ method, params }); + if (method === "users.info") { + return { user: user(String(params.user)) }; + } + if (method === "users.lookupByEmail") { + return { user: user("U33333333") }; + } + throw new Error(`Unexpected method: ${method}`); + }), + identities: ["U11111111", "W22222222", "Alice@Example.com"], + }); + + expect(calls).toEqual([ + { method: "users.info", params: { user: "U11111111" } }, + { method: "users.info", params: { user: "W22222222" } }, + { method: "users.lookupByEmail", params: { email: "alice@example.com" } }, + ]); + expect(result).toEqual({ + safe_to_mention: true, + results: [ + { index: 0, status: "resolved", mention: "<@U11111111>" }, + { index: 1, status: "resolved", mention: "<@W22222222>" }, + { index: 2, status: "resolved", mention: "<@U33333333>" }, + ], + }); + }); + + test("withholds every mention when one direct lookup is not found", async () => { + const result = await resolveStrictUserIdentities({ + client: client(async (method, params) => { + if (method === "users.info") { + return { user: user(String(params.user)) }; + } + throw new Error("users_not_found"); + }), + identities: ["U11111111", "missing@example.com"], + }); + + expect(result).toEqual({ + safe_to_mention: false, + results: [ + { index: 0, status: "resolved" }, + { index: 1, status: "unresolved" }, + ], + }); + expect(JSON.stringify(result)).not.toContain("<@"); + }); + + test("recognizes standard-token not-found errors", async () => { + const error = Object.assign(new Error("An API error occurred: user_not_found"), { + data: { error: "user_not_found" }, + }); + const result = await resolveStrictUserIdentities({ + client: client(async () => { + throw error; + }), + identities: ["U11111111"], + }); + + expect(result).toEqual({ + safe_to_mention: false, + results: [{ index: 0, status: "unresolved" }], + }); + }); + + test("rejects inactive users and bot signals", async () => { + const unsafeUsers = [ + user("U40000001", { deleted: true }), + user("U40000002", { is_bot: true }), + user("USLACKBOT"), + user("U40000003", { profile: { bot_id: "B12345678" } }), + user("U40000004", { is_connector_bot: true }), + user("U40000005", { is_workflow_bot: true }), + user("U40000006", { is_agentforce_bot: true }), + user("U40000007", { is_invited_user: true }), + user("U40000008", { suspended: true }), + user("U40000009", { is_forgotten: true }), + user("U40000010", { profile: { is_agentforce_bot: true } }), + user("U40000011", { profile: { is_sidekick_bot: true } }), + user("U40000012", { suspended: "false" }), + ]; + let index = 0; + const result = await resolveStrictUserIdentities({ + client: client(async () => ({ user: unsafeUsers[index++] })), + identities: unsafeUsers.map((item) => String(item.id)), + }); + + expect(result.safe_to_mention).toBe(false); + expect(result.results.every((item) => item.status === "unresolved")).toBe(true); + expect(JSON.stringify(result)).not.toContain("<@"); + }); + + test("fails closed on malformed or mismatched users", async () => { + const cases: { identity: string; response: Record }[] = [ + { identity: "U11111111", response: {} }, + { identity: "U11111111", response: { user: [] } }, + { identity: "U11111111", response: { user: user("U22222222") } }, + { + identity: "U11111111", + response: { user: { id: "U11111111", deleted: false, is_bot: false } }, + }, + { identity: "alice@example.com", response: { user: user("not-a-user-id") } }, + ]; + + for (const { identity, response } of cases) { + await expect( + resolveStrictUserIdentities({ + client: client(async () => response), + identities: [identity], + }), + ).resolves.toEqual({ + safe_to_mention: false, + results: [{ index: 0, status: "unresolved" }], + }); + } + }); + + test("rejects unsupported identities before any API call", async () => { + let calls = 0; + const apiClient = client(async () => { + calls += 1; + return {}; + }); + for (const identity of ["@alice", "Alice Smith", "alice", "u12345678", "<@U12345678>"]) { + await expect( + resolveStrictUserIdentities({ client: apiClient, identities: [identity] }), + ).rejects.toThrow("canonical U/W ID or email"); + } + expect(calls).toBe(0); + }); + + test("propagates non-definitive request errors", async () => { + await expect( + resolveStrictUserIdentities({ + client: client(async () => { + throw new Error("rate_limited"); + }), + identities: ["U11111111"], + }), + ).rejects.toThrow("rate_limited"); + }); +}); diff --git a/test/user-command.test.ts b/test/user-command.test.ts new file mode 100644 index 0000000..5351045 --- /dev/null +++ b/test/user-command.test.ts @@ -0,0 +1,185 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { Command } from "commander"; +import type { CliContext } from "../src/cli/context.ts"; +import { registerUserCommand } from "../src/cli/user-command.ts"; +import type { SlackApiClient } from "../src/slack/client.ts"; + +const originalLog = console.log; +const originalError = console.error; +let logs: string[]; +let errors: string[]; + +beforeEach(() => { + logs = []; + errors = []; + process.exitCode = 0; + console.log = (...args: unknown[]) => logs.push(args.map(String).join(" ")); + console.error = (...args: unknown[]) => errors.push(args.map(String).join(" ")); +}); + +afterEach(() => { + console.log = originalLog; + console.error = originalError; + process.exitCode = 0; +}); + +type Response = Error | Record; + +function user(id: string, fields: Record = {}): Record { + const profile = + fields.profile && typeof fields.profile === "object" && !Array.isArray(fields.profile) + ? fields.profile + : {}; + return { + id, + deleted: false, + is_bot: false, + ...fields, + profile: { ...profile }, + }; +} + +function clientFor( + responses: Response[], + calls: { method: string; params: Record }[] = [], +): SlackApiClient { + return { + api: async (method: string, params: Record) => { + calls.push({ method, params }); + const response = responses.shift(); + if (!response || response instanceof Error) { + throw response ?? new Error("Missing response"); + } + return response; + }, + } as unknown as SlackApiClient; +} + +function context(client: SlackApiClient, overrides: Partial = {}): CliContext { + return { + effectiveWorkspaceUrl: (workspace) => workspace, + withAutoRefresh: async (input: { work: () => Promise }) => input.work(), + getClientForWorkspace: async () => ({ + client, + auth: { auth_type: "standard", token: "x" }, + workspace_url: "https://workspace.slack.com", + }), + ...overrides, + } as CliContext; +} + +async function runResolve(ctx: CliContext, ...args: string[]): Promise { + const program = new Command(); + registerUserCommand({ program, ctx }); + await program.parseAsync(["user", "resolve", ...args], { from: "user" }); +} + +describe("user resolve command", () => { + test("auth refresh restarts the entire direct batch", async () => { + const calls: { method: string; params: Record }[] = []; + const client = clientFor( + [ + { user: user("U11111111") }, + new Error("invalid_auth"), + { user: user("U33333333") }, + { user: user("W22222222") }, + ], + calls, + ); + const ctx = context(client, { + withAutoRefresh: async (input: { work: () => Promise }) => { + try { + return await input.work(); + } catch { + return await input.work(); + } + }, + getClientForWorkspace: async () => ({ + client, + auth: { auth_type: "standard", token: "x" }, + workspace_url: "https://agency.slack-gov.com", + }), + }); + + await runResolve(ctx, "alice@example.com", "W22222222"); + + expect(calls).toEqual([ + { method: "users.lookupByEmail", params: { email: "alice@example.com" } }, + { method: "users.info", params: { user: "W22222222" } }, + { method: "users.lookupByEmail", params: { email: "alice@example.com" } }, + { method: "users.info", params: { user: "W22222222" } }, + ]); + expect(JSON.parse(logs[0]!)).toMatchObject({ + workspace: "https://agency.slack-gov.com", + safe_to_mention: true, + results: [{ mention: "<@U33333333>" }, { mention: "<@W22222222>" }], + }); + expect(logs[0]).not.toContain("U11111111"); + }); + + test("uses a fixed generic error for request failures", async () => { + await runResolve(context(clientFor([new Error("timeout <@U99999999>")])), "U11111111"); + + expect(logs).toEqual([]); + expect(errors).toEqual(["Unable to resolve users safely."]); + expect(errors[0]).not.toContain("U99999999"); + expect(process.exitCode).toBe(1); + }); + + test("withholds mentions for an unsafe direct result", async () => { + await runResolve( + context(clientFor([{ user: user("U11111111", { deleted: true }) }])), + "U11111111", + ); + + expect(errors).toEqual([]); + expect(JSON.parse(logs[0]!)).toEqual({ + workspace: "https://workspace.slack.com", + safe_to_mention: false, + results: [{ index: 0, status: "unresolved" }], + }); + expect(logs[0]).not.toContain("<@"); + expect(process.exitCode).toBe(1); + }); + + test("validates the workspace before resolving and uses a fixed generic error", async () => { + let apiCalls = 0; + const client = { api: async () => apiCalls++ } as unknown as SlackApiClient; + const badWorkspaces = [ + undefined, + "http://workspace.slack.com", + "https://collector.example", + "https://workspace.slack.com/<@U99999999>", + ]; + + for (const workspace_url of badWorkspaces) { + logs = []; + errors = []; + await runResolve( + context(client, { + getClientForWorkspace: async () => ({ + client, + auth: { auth_type: "standard", token: "x" }, + workspace_url, + }), + }), + "U11111111", + ); + expect(logs).toEqual([]); + expect(errors).toEqual(["Unable to resolve users safely."]); + } + expect(apiCalls).toBe(0); + }); + + test("rejects names without calling Slack", async () => { + let apiCalls = 0; + const client = { api: async () => apiCalls++ } as unknown as SlackApiClient; + + await runResolve(context(client), "Alice Smith"); + + expect(apiCalls).toBe(0); + expect(logs).toEqual([]); + expect(errors).toEqual(["Unable to resolve users safely."]); + expect(process.exitCode).toBe(1); + }); +});