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); + }); +});