diff --git a/docs/api-overview.md b/docs/api-overview.md index 383df6a3b..0eabb5aa6 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -161,6 +161,7 @@ Exported from `@loushy/build-ai-agent/testing` (see [Testing agents](testing.md) | `ToolInput`, `ToolOutput` | Argument and result types of a defined tool. | | `ToolRegistry` | Holds the tools an agent config refers to (advanced: `register(tool)` or `register(name, descriptor)`). | | `httpTool`, `currentDateTool`, `dayNameTool` | Built-in tools. | +| `createTodoTools({ store?, onChange? })` | `todo_write` / `todo_read` tools (plus `getTodos()`) so agents can plan and track multi-step work; see [Todo tools](#todo-tools). | | `loadMcpTools(client, connectionName)` | Load a connected MCP server's tools as `ToolDescriptor`s. Available from the package root, `@loushy/build-ai-agent/tools`, and `@loushy/build-ai-agent/mcp`. | ```ts @@ -232,6 +233,31 @@ non-text parts the result is `{ structuredContent, text, content }` with `content` holding only the non-text parts, so images, audio and resources are never dropped. +### Todo tools + +`createTodoTools(options?)` gives long-running agents a plan to track: +`todo_write` replaces the whole list (`{ id?, content, status }` items, status +`pending` | `in_progress` | `completed`, at most one `in_progress`) and returns +the list plus counts; `todo_read` returns it. Ids are assigned automatically and +stay stable when a later write repeats an item's content. Invalid lists (for +example two `in_progress`) reach the model as a structured tool error so it can +retry. The list lives in memory per call; pass `store` (`{ get, set }`) to +persist it, and `onChange` to update a UI. + +```ts +import { createAgent, createTodoTools, type TodoStore } from '@loushy/build-ai-agent'; + +const todos = createTodoTools({ onChange: (list) => console.log(list.length, 'todos') }); +const planner = createAgent({ prompt: 'Plan multi-step work, then do it.', provider, tools: todos.tools }); + +await planner.send('Migrate the repo to ESM'); +console.log(await todos.getTodos()); // [{ id: 'todo_1', content: '...', status: 'completed' }, ...] + +// Persist the list somewhere else: +let saved: Awaited> = []; +createTodoTools({ store: { get: () => saved, set: (next) => void (saved = next) } }); +``` + ### Tool errors A tool call can fail in two ways. In both, the run continues and the model @@ -442,6 +468,54 @@ failed - and the reason (never a secret or signature) is logged at `warn` level. Serve webhooks over HTTPS (terminate TLS in front of the adapter) so tokens and payloads are not sent in clear text. +### Slack request signatures + +`SlackTriggerAdapter.handleRequest({ headers, rawBody })` handles a raw Slack +Events API request and returns the `{ status, body }` to send back. **Set +`signingSecret` for any endpoint reachable from outside your machine**: without +it, anyone who can reach the endpoint can run your agent, and `listen()` logs a +one-time warning through `options.logger`. With it, every request is verified +as [Slack documents](https://docs.slack.dev/authentication/verifying-requests-from-slack) +before the body is parsed: HMAC-SHA256 over `v0:{X-Slack-Request-Timestamp}:{raw body}`, +compared in constant time with `X-Slack-Signature` (`v0=`), and requests +more than five minutes old are rejected. Failures get a generic +`401 {"error":"Unauthorized"}`; the reason (never a secret or signature) is +logged at `warn` level. The signed `url_verification` handshake is answered +after verification. + +```ts +import { createAgent, createMockProvider } from '@loushy/build-ai-agent'; +import { SlackTriggerAdapter, verifySlackSignature } from '@loushy/build-ai-agent/triggers'; +import * as http from 'node:http'; + +const agent = createAgent({ prompt: 'You are helpful.', provider: createMockProvider() }); +const slack = new SlackTriggerAdapter({ signingSecret: process.env.SLACK_SIGNING_SECRET }); +slack.listen(agent, (input) => agent.send(input)); + +http + .createServer((req, res) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', async () => { + const { status, body } = await slack.handleRequest({ headers: req.headers, rawBody: Buffer.concat(chunks) }); + res.writeHead(status, { 'Content-Type': 'application/json' }).end(JSON.stringify(body)); + }); + }) + .listen(3000); + +// Your own handler (slash commands, interactivity)? Verify the RAW body yourself: +const authentic = verifySlackSignature({ + signingSecret: process.env.SLACK_SIGNING_SECRET ?? '', + timestamp: '1700000000', // the X-Slack-Request-Timestamp header + signature: 'v0=...', // the X-Slack-Signature header + rawBody: '{"type":"url_verification"}', +}); +``` + +`handleRequest` answers Slack after the agent finishes; Slack expects a reply +within three seconds, so for slow agents acknowledge first and run the agent in +the background. + ### Cron schedules `CronTriggerAdapter` takes either a fixed `intervalMs` or a real cron diff --git a/examples/ops-pipeline/README.md b/examples/ops-pipeline/README.md index e40c09774..b453a255a 100644 --- a/examples/ops-pipeline/README.md +++ b/examples/ops-pipeline/README.md @@ -90,6 +90,10 @@ required for the default demo: - `SLACK_WEBHOOK_URL` - real Slack Incoming Webhook (see `src/tools/built-in/slack.ts`) +- `SLACK_SIGNING_SECRET` - your Slack app's signing secret. When set, + `POST /slack/interactions` verifies every request's `X-Slack-Signature` + (via the SDK's `verifySlackSignature`) and answers 401 otherwise. Set it + whenever the endpoint is reachable from the internet. - `OPENAI_API_KEY` / `ANTHROPIC_API_KEY` - real LLM provider (pass a real `LLMProvider` to `startOpsPipeline({ provider })` instead of the default `demoProvider`) diff --git a/examples/ops-pipeline/index.test.ts b/examples/ops-pipeline/index.test.ts index 44dff59c3..9b0339558 100644 --- a/examples/ops-pipeline/index.test.ts +++ b/examples/ops-pipeline/index.test.ts @@ -1,3 +1,4 @@ +import { createHmac } from 'node:crypto'; import { describe, it, expect, afterEach } from 'vitest'; import { startOpsPipeline, OpsPipelineHandle } from './index'; import { createDemoProvider } from './demoProvider'; @@ -68,3 +69,31 @@ describe('ops-pipeline end-to-end (LOU-J8, against mocks - zero external network expect(github.createdPullRequests[0].body).toContain('OrderService.java'); }); }); + +describe('ops-pipeline Slack signature verification (LOU-D18)', () => { + let handle: OpsPipelineHandle | undefined; + + afterEach(async () => { + await handle?.close(); + handle = undefined; + }); + + it('rejects unsigned interaction callbacks and accepts correctly signed ones when a signing secret is set', async () => { + const secret = 'ops-signing-secret'; + handle = await startOpsPipeline({ slackSigningSecret: secret, monitorPort: 0, slackPort: 0 }); + const url = `http://127.0.0.1:${handle.slack.port}/slack/interactions`; + const body = JSON.stringify({ type: 'block_actions', actions: [{ action_id: 'other', value: 'x' }] }); + + const unsigned = await fetch(url, { method: 'POST', body }); + expect(unsigned.status).toBe(401); + + const timestamp = String(Math.floor(Date.now() / 1000)); + const signature = `v0=${createHmac('sha256', secret).update(`v0:${timestamp}:${body}`).digest('hex')}`; + const signed = await fetch(url, { + method: 'POST', + headers: { 'x-slack-request-timestamp': timestamp, 'x-slack-signature': signature }, + body, + }); + expect(signed.status).toBe(200); + }); +}); diff --git a/examples/ops-pipeline/index.ts b/examples/ops-pipeline/index.ts index 13a921bb4..62a72764a 100644 --- a/examples/ops-pipeline/index.ts +++ b/examples/ops-pipeline/index.ts @@ -28,6 +28,7 @@ import * as http from 'node:http'; import { AgentType, ToolDescriptor } from '../../src/types'; import { ToolRegistry } from '../../src/tools'; +import { verifySlackSignature } from '../../src/triggers'; import { LLMProvider } from '../../src/providers/llm'; import { startMonitorServer, MonitorServerHandle, StartMonitorServerOptions } from './monitor'; import { createFixerDelegateTool, buildFixerAgent } from './fixer'; @@ -84,6 +85,11 @@ export interface OpsPipelineDeps { slackHost?: string; slackPort?: number; channel?: string; + /** + * Slack signing secret. When set (default: `SLACK_SIGNING_SECRET`), requests to + * POST /slack/interactions must carry a valid Slack signature or get a 401. + */ + slackSigningSecret?: string; /** * Guardrails to run before considering the GitHub PR call. Defaults to * handleFixerPatch()'s own defaults (real secretScanGuardrail + a @@ -135,9 +141,22 @@ interface SlackRouteContext { githubCreatePrTool: ToolDescriptor; slackTool: ToolDescriptor; channel: string; + signingSecret?: string; guardrails?: Guardrail[]; } +/** True when no signing secret is configured, or the request carries a valid Slack signature over the raw body. */ +function isAuthenticSlackRequest(req: http.IncomingMessage, raw: string, signingSecret: string | undefined): boolean { + if (!signingSecret) return true; + const header = (name: string) => (typeof req.headers[name] === 'string' ? (req.headers[name] as string) : undefined); + return verifySlackSignature({ + signingSecret, + timestamp: header('x-slack-request-timestamp'), + signature: header('x-slack-signature'), + rawBody: raw, + }); +} + /** * Handles one POST /slack/interactions request: resumes the paused run, then - * if the fixer produced a patch - sends it through the guardrail-gated PR path. @@ -149,6 +168,10 @@ async function handleSlackInteractionRequest( ): Promise { try { const raw = await readBody(req); + if (!isAuthenticSlackRequest(req, raw, ctx.signingSecret)) { + sendJson(res, 401, { error: 'Unauthorized' }); + return; + } const payload = parseSlackInteractionBody(raw); const approvalId = extractApprovalIdFromInteraction(payload); @@ -254,6 +277,7 @@ export async function startOpsPipeline(deps: OpsPipelineDeps = {}): Promise): Array> { + const last = model.calls[model.calls.length - 1]; + return last.messages.filter((m) => m.role === 'tool').map((m) => JSON.parse(String(m.content))); +} + +describe('createTodoTools', () => { + it('lets a run write, update and read the list, keeping ids stable', async () => { + const todos = createTodoTools(); + const model = mockModel([ + { + toolCalls: [ + { + name: 'todo_write', + args: { + todos: [ + { content: 'Write tests', status: 'in_progress' }, + { content: 'Ship it', status: 'pending' }, + ], + }, + }, + ], + }, + { + toolCalls: [ + { + name: 'todo_write', + args: { + todos: [ + { content: 'Write tests', status: 'completed' }, + { content: 'Ship it', status: 'in_progress' }, + { content: 'Celebrate', status: 'pending' }, + ], + }, + }, + { name: 'todo_read' }, + ], + }, + 'All done.', + ]); + const agent = createAgent({ prompt: 'Plan then act.', provider: model, tools: todos.tools }); + + const result = await agent.send('Do the thing'); + + expect(result.text).toBe('All done.'); + const [, , read] = toolResults(model); + const final = await todos.getTodos(); + expect(final.map((t) => [t.id, t.content, t.status])).toEqual([ + ['todo_1', 'Write tests', 'completed'], + ['todo_2', 'Ship it', 'in_progress'], + ['todo_3', 'Celebrate', 'pending'], + ]); + expect(read).toEqual({ + todos: final, + counts: { pending: 1, in_progress: 1, completed: 1, total: 3 }, + }); + }); + + it('assigns ids once, keeps explicit ids and avoids collisions', async () => { + const { todoWrite, getTodos } = createTodoTools(); + const exec = todoWrite.tool.execute as (args: unknown, ctx: unknown) => Promise; + await exec({ todos: [{ id: 'todo_1', content: 'a', status: 'pending' }, { content: 'b', status: 'pending' }] }, {}); + expect((await getTodos()).map((t) => t.id)).toEqual(['todo_1', 'todo_2']); + await exec({ todos: [{ content: 'b', status: 'completed' }, { content: 'c', status: 'pending' }] }, {}); + expect((await getTodos()).map((t) => t.id)).toEqual(['todo_2', 'todo_3']); + }); + + it('returns validation failures (two in_progress, duplicate ids, empty content) to the model as structured errors', async () => { + const todos = createTodoTools(); + const model = mockModel([ + { + toolCalls: [ + { + name: 'todo_write', + args: { todos: [{ content: 'a', status: 'in_progress' }, { content: 'b', status: 'in_progress' }] }, + }, + { name: 'todo_write', args: { todos: [{ id: 'x', content: 'a', status: 'pending' }, { id: 'x', content: 'b', status: 'pending' }] } }, + { name: 'todo_write', args: { todos: [{ content: ' ', status: 'pending' }] } }, + ], + }, + 'gave up', + ]); + const agent = createAgent({ prompt: 'p', provider: model, tools: todos.tools }); + + await agent.send('go'); + + const [two, dup, empty] = toolResults(model); + expect(two).toMatchObject({ error: 'ToolArgumentsValidationError', toolName: 'todo_write' }); + expect(String(two.message)).toMatch(/at most one todo may be in_progress/i); + expect(String(dup.message)).toMatch(/unique/); + expect(empty).toMatchObject({ error: 'ToolArgumentsValidationError' }); + expect(await todos.getTodos()).toEqual([]); + }); + + it('calls onChange with the new list after each write', async () => { + const onChange = vi.fn(); + const todos = createTodoTools({ onChange }); + const model = mockModel([ + { toolCalls: [{ name: 'todo_write', args: { todos: [{ content: 'a', status: 'pending' }] } }] }, + { toolCalls: [{ name: 'todo_read' }] }, + 'ok', + ]); + + await createAgent({ prompt: 'p', provider: model, tools: todos.tools }).send('go'); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith([{ id: 'todo_1', content: 'a', status: 'pending' }]); + }); + + it('reads and writes through a custom (async) store', async () => { + let saved: Todo[] = [{ id: 'todo_7', content: 'restored', status: 'pending' }]; + const store: TodoStore = { + get: async () => saved, + set: async (next) => { + saved = next; + }, + }; + const todos = createTodoTools({ store }); + const model = mockModel([ + { toolCalls: [{ name: 'todo_read' }] }, + { toolCalls: [{ name: 'todo_write', args: { todos: [{ content: 'restored', status: 'completed' }, { content: 'new', status: 'pending' }] } }] }, + 'ok', + ]); + + await createAgent({ prompt: 'p', provider: model, tools: todos.tools }).send('go'); + + expect(saved).toEqual([ + { id: 'todo_7', content: 'restored', status: 'completed' }, + { id: 'todo_8', content: 'new', status: 'pending' }, + ]); + expect(await todos.getTodos()).toBe(saved); + }); + + it('keeps separate in-memory lists per createTodoTools() call', async () => { + const a = createTodoTools(); + const b = createTodoTools(); + await (a.todoWrite.tool.execute as (args: unknown, ctx: unknown) => Promise)( + { todos: [{ content: 'only a', status: 'pending' }] }, + {} + ); + expect(await a.getTodos()).toHaveLength(1); + expect(await b.getTodos()).toHaveLength(0); + }); + + it('describes when to use each tool', () => { + const { todoWrite, todoRead } = createTodoTools(); + expect(todoWrite.description).toMatch(/multi-step|three or more steps/); + expect(todoRead.name).toBe('todo_read'); + }); +}); diff --git a/src/tools/built-in/todo.ts b/src/tools/built-in/todo.ts new file mode 100644 index 000000000..2b66e872f --- /dev/null +++ b/src/tools/built-in/todo.ts @@ -0,0 +1,182 @@ +/** + * Built-in todo tools (LOU-X7). + * + * Lets a long-running agent plan and track multi-step work. `todo_write` + * replaces the whole list, `todo_read` returns it, and both share one store. + */ +import { z } from 'zod'; +import { defineTool } from '../defineTool'; + +/** Progress of a {@link Todo}. */ +export type TodoStatus = 'pending' | 'in_progress' | 'completed'; + +/** One tracked task. */ +export interface Todo { + /** Stable identifier (assigned automatically when omitted). */ + id: string; + /** What needs doing, in imperative form. */ + content: string; + status: TodoStatus; +} + +/** + * Where the todo list lives. Implement it to persist the list (database, + * file, session state). Both methods may be async. + * + * @example + * ```ts + * const saved: Todo[] = []; + * const store: TodoStore = { get: () => saved, set: (todos) => { saved.splice(0, saved.length, ...todos); } }; + * ``` + */ +export interface TodoStore { + get(): Todo[] | Promise; + set(todos: Todo[]): void | Promise; +} + +/** Options for {@link createTodoTools}. */ +export interface TodoToolsOptions { + /** Backing store. Defaults to a fresh in-memory store per `createTodoTools()` call. */ + store?: TodoStore; + /** Called with the new list after every `todo_write` (e.g. to re-render a UI). */ + onChange?: (todos: readonly Todo[]) => void | Promise; +} + +/** What `todo_write` and `todo_read` return to the model. */ +export interface TodoListResult { + todos: Todo[]; + counts: Record & { total: number }; +} + +/** The tools and host accessor returned by {@link createTodoTools}. */ +export interface TodoTools { + /** `[todo_write, todo_read]`, ready for `createAgent({ tools })`. */ + tools: [TodoWriteTool, TodoReadTool]; + todoWrite: TodoWriteTool; + todoRead: TodoReadTool; + /** The current list, for the host application. */ + getTodos(): Promise; +} + +const todoItem = z.object({ + id: z.string().min(1).optional().describe('Keep the id of an existing item to update it; omit for new items.'), + content: z.string().trim().min(1).describe('Imperative description of the task.'), + status: z.enum(['pending', 'in_progress', 'completed']), +}); + +type TodoItemInput = z.output; + +function validateList(items: TodoItemInput[], ctx: z.RefinementCtx): void { + const inProgress = items.filter((item) => item.status === 'in_progress').length; + if (inProgress > 1) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `At most one todo may be in_progress, got ${inProgress}. Mark the others pending or completed.`, + }); + } + const ids = items.flatMap((item) => (item.id === undefined ? [] : [item.id])); + if (new Set(ids).size !== ids.length) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'Todo ids must be unique.' }); + } +} + +const writeInput = z.object({ + todos: z.array(todoItem).superRefine(validateList).describe('The complete list; replaces the previous one.'), +}); + +function summarize(todos: Todo[]): TodoListResult { + const counts = { pending: 0, in_progress: 0, completed: 0, total: todos.length }; + for (const todo of todos) counts[todo.status] += 1; + return { todos, counts }; +} + +/** Gives every item an id: explicit ones are kept, matching content reuses the existing id, the rest get a new one. */ +function assignIds(items: TodoItemInput[], existing: Todo[]): Todo[] { + const used = new Set([ + ...items.flatMap((item) => (item.id === undefined ? [] : [item.id])), + ]); + const reusable = new Map(); + for (const todo of existing) { + if (used.has(todo.id)) continue; + reusable.set(todo.content, [...(reusable.get(todo.content) ?? []), todo.id]); + } + const taken = new Set([...used, ...existing.map((todo) => todo.id)]); + let counter = Math.max(0, ...[...taken].map((id) => Number(/^todo_(\d+)$/.exec(id)?.[1] ?? 0))); + const fresh = (): string => { + do counter += 1; + while (taken.has(`todo_${counter}`)); + taken.add(`todo_${counter}`); + return `todo_${counter}`; + }; + return items.map((item) => ({ + id: item.id ?? reusable.get(item.content)?.shift() ?? fresh(), + content: item.content, + status: item.status, + })); +} + +type TodoWriteTool = ReturnType; +type TodoReadTool = ReturnType; + +function memoryStore(): TodoStore { + let todos: Todo[] = []; + return { + get: () => todos, + set: (next) => { + todos = next; + }, + }; +} + +function buildWriteTool(store: TodoStore, onChange: TodoToolsOptions['onChange']) { + return defineTool({ + name: 'todo_write', + description: + 'Plan and track multi-step work. Use it at the start of any task with three or more steps, and update it as you go: ' + + 'send the COMPLETE list each time (it replaces the previous one), keep at most one item in_progress, and mark items ' + + 'completed as soon as they are done. Skip it for simple one-step requests.', + input: writeInput, + async execute({ todos }): Promise { + const next = assignIds(todos, await store.get()); + await store.set(next); + await onChange?.(next); + return summarize(next); + }, + }); +} + +function buildReadTool(store: TodoStore) { + return defineTool({ + name: 'todo_read', + description: 'Return the current todo list with counts per status. Use it to re-check what is left before continuing or finishing.', + input: z.object({}), + async execute(): Promise { + return summarize(await store.get()); + }, + }); +} + +/** + * Create the `todo_write` / `todo_read` tools, sharing one store. + * + * @example + * ```ts + * const todos = createTodoTools({ onChange: (list) => render(list) }); + * const agent = createAgent({ prompt: 'Plan, then do.', provider, tools: todos.tools }); + * await agent.send('Migrate the repo to ESM'); + * console.log(await todos.getTodos()); + * ``` + */ +export function createTodoTools(options: TodoToolsOptions = {}): TodoTools { + const store = options.store ?? memoryStore(); + const todoWrite = buildWriteTool(store, options.onChange); + const todoRead = buildReadTool(store); + return { + tools: [todoWrite, todoRead], + todoWrite, + todoRead, + async getTodos() { + return store.get(); + }, + }; +} diff --git a/src/triggers/adapters/SlackTriggerAdapter.signature.test.ts b/src/triggers/adapters/SlackTriggerAdapter.signature.test.ts new file mode 100644 index 000000000..0848720bb --- /dev/null +++ b/src/triggers/adapters/SlackTriggerAdapter.signature.test.ts @@ -0,0 +1,136 @@ +import { createHmac } from 'node:crypto'; +import { afterEach, describe, it, expect, vi } from 'vitest'; +import { SlackTriggerAdapter } from './SlackTriggerAdapter'; +import { verifySlackSignature } from '../webhookAuth'; +import type { ExecutionResult } from '../../execution/AgentExecutor'; +import type { Logger } from '../../execution/logger'; +import type { RunnableAgent } from '../types'; + +const SECRET = 'slack-signing-secret'; +const NOW_SECONDS = 1_700_000_000; +const noopAgent: RunnableAgent = { send: vi.fn() }; + +function sign(body: string, timestamp: string | number = NOW_SECONDS, secret = SECRET): string { + return `v0=${createHmac('sha256', secret).update(`v0:${timestamp}:${body}`).digest('hex')}`; +} + +function signedHeaders(body: string, overrides: Record = {}) { + return { + 'x-slack-request-timestamp': String(NOW_SECONDS), + 'x-slack-signature': sign(body), + ...overrides, + }; +} + +function makeLogger(): Logger & { warn: ReturnType } { + return { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; +} + +const messageBody = JSON.stringify({ + type: 'event_callback', + event: { type: 'message', channel: 'C1', text: 'hello agent' }, +}); +const challengeBody = JSON.stringify({ type: 'url_verification', challenge: 'abc123' }); + +function setup(options: { signingSecret?: string; logger?: Logger } = {}) { + vi.useFakeTimers({ now: NOW_SECONDS * 1000, toFake: ['Date'] }); + const fetchImpl = vi.fn().mockResolvedValue({ ok: true, status: 200, text: async () => '' } as Response); + const adapter = new SlackTriggerAdapter({ webhookUrl: 'https://hooks.slack.test/x', fetchImpl, ...options }); + const onEvent = vi.fn().mockResolvedValue({ text: 'reply' } as ExecutionResult); + adapter.listen(noopAgent, onEvent); + return { adapter, onEvent }; +} + +type Built = { headers: Record; rawBody: string }; + +describe('SlackTriggerAdapter signature verification', () => { + afterEach(() => vi.useRealTimers()); + + it('accepts a correctly signed request and runs the agent', async () => { + const { adapter, onEvent } = setup({ signingSecret: SECRET }); + const res = await adapter.handleRequest({ headers: signedHeaders(messageBody), rawBody: Buffer.from(messageBody) }); + expect(res.status).toBe(200); + expect(onEvent).toHaveBeenCalledWith('hello agent', { channel: 'C1' }); + }); + + const staleHeaders = (b: string, offset: number) => ({ + 'x-slack-request-timestamp': String(NOW_SECONDS + offset), + 'x-slack-signature': sign(b, NOW_SECONDS + offset), + }); + const rejected: Array<[string, (b: string) => Built]> = [ + ['a wrong secret', (b) => ({ headers: { 'x-slack-request-timestamp': String(NOW_SECONDS), 'x-slack-signature': sign(b, NOW_SECONDS, 'other') }, rawBody: b })], + ['a tampered body', (b) => ({ headers: signedHeaders(b), rawBody: b.replace('hello', 'HELLO') })], + ['a stale timestamp', (b) => ({ headers: staleHeaders(b, -301), rawBody: b })], + ['a future timestamp', (b) => ({ headers: staleHeaders(b, 301), rawBody: b })], + ['a missing signature header', (b) => ({ headers: signedHeaders(b, { 'x-slack-signature': undefined }), rawBody: b })], + ['a missing timestamp header', (b) => ({ headers: signedHeaders(b, { 'x-slack-request-timestamp': undefined }), rawBody: b })], + ['a signature of the wrong length', (b) => ({ headers: signedHeaders(b, { 'x-slack-signature': sign(b).slice(0, -2) }), rawBody: b })], + ['a non-hex signature', (b) => ({ headers: signedHeaders(b, { 'x-slack-signature': `v0=${'z'.repeat(64)}` }), rawBody: b })], + ['a signature without the v0= prefix', (b) => ({ headers: signedHeaders(b, { 'x-slack-signature': sign(b).slice(3) }), rawBody: b })], + ]; + + it.each(rejected)('rejects %s with a generic 401 and never runs the agent', async (_name, build) => { + const logger = makeLogger(); + const { adapter, onEvent } = setup({ signingSecret: SECRET, logger }); + const res = await adapter.handleRequest(build(messageBody)); + expect(res).toEqual({ status: 401, body: { error: 'Unauthorized' } }); + expect(onEvent).not.toHaveBeenCalled(); + const logged = JSON.stringify(logger.warn.mock.calls); + expect(logged).not.toContain(SECRET); + expect(logged).not.toContain('v0='); + }); + + it('answers a signed url_verification challenge', async () => { + const { adapter } = setup({ signingSecret: SECRET }); + const res = await adapter.handleRequest({ headers: signedHeaders(challengeBody), rawBody: challengeBody }); + expect(res).toEqual({ status: 200, body: { challenge: 'abc123' } }); + }); + + it('rejects an unsigned url_verification challenge when a signing secret is set', async () => { + const { adapter } = setup({ signingSecret: SECRET }); + const res = await adapter.handleRequest({ headers: {}, rawBody: challengeBody }); + expect(res.status).toBe(401); + }); + + it('still answers an unsigned challenge when no signing secret is set (backwards compatible)', async () => { + const { adapter } = setup(); + const res = await adapter.handleRequest({ headers: {}, rawBody: challengeBody }); + expect(res).toEqual({ status: 200, body: { challenge: 'abc123' } }); + }); + + it('returns 400 for a signed non-JSON body and ignores bot messages', async () => { + const { adapter, onEvent } = setup({ signingSecret: SECRET }); + expect((await adapter.handleRequest({ headers: signedHeaders('nope'), rawBody: 'nope' })).status).toBe(400); + const bot = JSON.stringify({ type: 'event_callback', event: { type: 'message', channel: 'C1', text: 'x', bot_id: 'B1' } }); + expect((await adapter.handleRequest({ headers: signedHeaders(bot), rawBody: bot })).status).toBe(200); + expect(onEvent).not.toHaveBeenCalled(); + }); + + it('warns once at listen() when no signingSecret is set, and not when it is', () => { + const logger = makeLogger(); + const adapter = new SlackTriggerAdapter({ logger }); + adapter.listen(noopAgent, vi.fn()); + adapter.listen(noopAgent, vi.fn()); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn.mock.calls[0][0]).toMatch(/signingSecret.*api-overview/s); + + const quiet = makeLogger(); + new SlackTriggerAdapter({ logger: quiet, signingSecret: SECRET }).listen(noopAgent, vi.fn()); + expect(quiet.warn).not.toHaveBeenCalled(); + }); +}); + +describe('verifySlackSignature', () => { + const base = { signingSecret: SECRET, timestamp: String(NOW_SECONDS), rawBody: messageBody, now: NOW_SECONDS * 1000 }; + + it('accepts a valid signature (string or Buffer body) and rejects bad input without throwing', () => { + const signature = sign(messageBody); + expect(verifySlackSignature({ ...base, signature })).toBe(true); + expect(verifySlackSignature({ ...base, signature, rawBody: Buffer.from(messageBody) })).toBe(true); + expect(verifySlackSignature({ ...base, signature: undefined })).toBe(false); + expect(verifySlackSignature({ ...base, signature, timestamp: 'abc' })).toBe(false); + expect(verifySlackSignature({ ...base, signature, signingSecret: '' })).toBe(false); + expect(verifySlackSignature({ ...base, signature, now: (NOW_SECONDS + 301) * 1000 })).toBe(false); + expect(verifySlackSignature({ ...base, signature, now: (NOW_SECONDS + 299) * 1000 })).toBe(true); + }); +}); diff --git a/src/triggers/adapters/SlackTriggerAdapter.ts b/src/triggers/adapters/SlackTriggerAdapter.ts index affc60927..80f1da6f9 100644 --- a/src/triggers/adapters/SlackTriggerAdapter.ts +++ b/src/triggers/adapters/SlackTriggerAdapter.ts @@ -25,7 +25,9 @@ */ import type { ExecutionResult } from '../../execution/AgentExecutor'; import { SLACK_WEBHOOK_URL_ENV_KEY } from '../../tools/built-in/slack'; +import { Logger, noopLogger } from '../../execution/logger'; import { RunnableAgent, TriggerAdapter, TriggerContext, TriggerHandle } from '../types'; +import { checkSlackSignature } from '../webhookAuth'; export interface SlackMessageEvent { channel: string; @@ -37,6 +39,58 @@ export interface SlackTriggerAdapterOptions { webhookUrl?: string; /** Overrides the fetch implementation used to POST replies (testing). */ fetchImpl?: typeof fetch; + /** + * Your Slack app's signing secret. When set, `handleRequest()` verifies every + * inbound request (HMAC-SHA256 over `v0:{timestamp}:{raw body}`, five-minute + * replay window) and answers a generic 401 otherwise. **Set it for any + * endpoint reachable from outside your machine**: without it anyone who can + * reach the endpoint can run your agent. + * + * @example + * ```ts + * new SlackTriggerAdapter({ signingSecret: process.env.SLACK_SIGNING_SECRET }); + * ``` + */ + signingSecret?: string; + /** Receives signature failures and the "no signingSecret" warning. Defaults to a no-op logger. */ + logger?: Logger; +} + +/** A raw inbound Slack HTTP request, as handed to {@link SlackTriggerAdapter.handleRequest}. */ +export interface SlackInboundRequest { + /** Request headers (Node's `req.headers` works as is). */ + headers: Record; + /** The exact body bytes received. Do not re-serialize a parsed body: the signature covers the raw bytes. */ + rawBody: Buffer | string; +} + +/** What your HTTP handler should send back to Slack. */ +export interface SlackHttpResponse { + status: number; + body: Record; +} + +function headerValue(headers: SlackInboundRequest['headers'], name: string): string | undefined { + const value = headers[name]; + return typeof value === 'string' ? value : undefined; +} + +function parseJsonObject(raw: Buffer | string): Record | undefined { + try { + const parsed: unknown = JSON.parse(raw.toString()); + return typeof parsed === 'object' && parsed !== null ? (parsed as Record) : undefined; + } catch { + return undefined; + } +} + +/** The `{ channel, text }` of a plain user message in an Events API `event_callback`, if the payload is one. */ +function extractMessageEvent(payload: Record): SlackMessageEvent | undefined { + const event = payload.event as Record | undefined; + if (payload.type !== 'event_callback' || event?.type !== 'message') return undefined; + if (event.subtype !== undefined || event.bot_id !== undefined) return undefined; + if (typeof event.channel !== 'string' || typeof event.text !== 'string') return undefined; + return { channel: event.channel, text: event.text }; } export class SlackTriggerAdapter implements TriggerAdapter { @@ -44,6 +98,7 @@ export class SlackTriggerAdapter implements TriggerAdapter { private onEventHandler?: (input: string, context: TriggerContext) => Promise; private listening = false; + private warnedUnauthenticated = false; constructor(private readonly options: SlackTriggerAdapterOptions = {}) {} @@ -54,6 +109,7 @@ export class SlackTriggerAdapter implements TriggerAdapter { void agent; this.onEventHandler = onEvent; this.listening = true; + this.warnIfUnauthenticated(); return { stop: () => { @@ -63,6 +119,50 @@ export class SlackTriggerAdapter implements TriggerAdapter { }; } + private warnIfUnauthenticated(): void { + if (this.options.signingSecret || this.warnedUnauthenticated) return; + this.warnedUnauthenticated = true; + (this.options.logger ?? noopLogger).warn( + 'SlackTriggerAdapter has no `signingSecret`: Slack request signatures are not verified, so anyone who can reach ' + + 'your endpoint can run your agent. Pass options.signingSecret; see docs/api-overview.md#slack-request-signatures.' + ); + } + + /** + * Handles a raw Slack Events API HTTP request and returns the response to send. + * With `signingSecret` set, the signature is verified against the raw body + * before anything is parsed (so the signed `url_verification` handshake works + * too); failures get a generic 401 and are logged without secrets or signatures. + * + * @example + * ```ts + * const { status, body } = await adapter.handleRequest({ headers: req.headers, rawBody }); + * res.writeHead(status, { 'Content-Type': 'application/json' }).end(JSON.stringify(body)); + * ``` + */ + public async handleRequest(request: SlackInboundRequest): Promise { + const { signingSecret } = this.options; + if (signingSecret) { + // Scheme: https://docs.slack.dev/authentication/verifying-requests-from-slack + const failure = checkSlackSignature({ + signingSecret, + timestamp: headerValue(request.headers, 'x-slack-request-timestamp'), + signature: headerValue(request.headers, 'x-slack-signature'), + rawBody: request.rawBody, + }); + if (failure !== undefined) { + (this.options.logger ?? noopLogger).warn('slack request rejected: signature verification failed', { reason: failure }); + return { status: 401, body: { error: 'Unauthorized' } }; + } + } + const payload = parseJsonObject(request.rawBody); + if (!payload) return { status: 400, body: { error: 'Invalid JSON body' } }; + if (payload.type === 'url_verification') return { status: 200, body: { challenge: payload.challenge } }; + const event = extractMessageEvent(payload); + if (event) await this.handleEvent(event); + return { status: 200, body: { ok: true } }; + } + /** * Feeds a Slack message event into the adapter (called by whatever is * actually subscribed to Slack - an Events API HTTP endpoint, a Socket diff --git a/src/triggers/webhookAuth.ts b/src/triggers/webhookAuth.ts index 78dabdac5..665a525ac 100644 --- a/src/triggers/webhookAuth.ts +++ b/src/triggers/webhookAuth.ts @@ -164,3 +164,60 @@ export async function checkWebhookAuth( if (auth.type === 'bearer') return checkBearer(auth, req); return checkCustom(auth, req, rawBody); } + +/** Slack's documented replay window: reject requests more than five minutes from local time. */ +const SLACK_TOLERANCE_SECONDS = 300; +const SLACK_SIGNATURE_PATTERN = /^v0=[0-9a-f]{64}$/i; + +/** Inputs of {@link verifySlackSignature}. */ +export interface SlackSignatureInput { + /** Your Slack app's signing secret (Basic Information > App Credentials). */ + signingSecret: string; + /** The `X-Slack-Request-Timestamp` header value (Unix seconds). */ + timestamp: string | undefined; + /** The `X-Slack-Signature` header value (`v0=`). */ + signature: string | undefined; + /** The exact request body bytes, before any parsing. */ + rawBody: Buffer | string; + /** Current time in milliseconds. Defaults to `Date.now()`; for tests. */ + now?: number; +} + +/** + * Internal check behind {@link verifySlackSignature}: resolves to a short + * reason (safe to log - it contains no secret or signature) when the request + * is not authentic, or `undefined` when it is. + * + * Scheme: https://docs.slack.dev/authentication/verifying-requests-from-slack + * (formerly https://api.slack.com/authentication/verifying-requests-from-slack): + * HMAC-SHA256 over `v0:{timestamp}:{raw body}`, sent as `v0=` in + * `X-Slack-Signature`; requests more than five minutes old are rejected. + */ +export function checkSlackSignature(input: SlackSignatureInput): string | undefined { + const { signingSecret, timestamp, signature, rawBody, now = Date.now() } = input; + if (!signingSecret) return 'no signing secret configured'; + if (timestamp === undefined || signature === undefined) return 'missing signature headers'; + if (!SLACK_SIGNATURE_PATTERN.test(signature)) return 'malformed signature'; + if (!/^\d+$/.test(timestamp) || !isFresh(timestamp, SLACK_TOLERANCE_SECONDS, now)) return 'stale or invalid timestamp'; + const expected = createHmac('sha256', signingSecret).update(`v0:${timestamp}:`).update(rawBody).digest('hex'); + return safeEqual(signature.slice(3).toLowerCase(), expected) ? undefined : 'signature mismatch'; +} + +/** + * Verify a request really came from Slack. Use it in your own HTTP handler + * (slash commands, interactivity, Events API) with the RAW body, before parsing. + * Returns `false` for any invalid, missing, stale or tampered input; never throws. + * + * @example + * ```ts + * const ok = verifySlackSignature({ + * signingSecret: process.env.SLACK_SIGNING_SECRET ?? '', + * timestamp: req.headers['x-slack-request-timestamp'] as string | undefined, + * signature: req.headers['x-slack-signature'] as string | undefined, + * rawBody, + * }); + * ``` + */ +export function verifySlackSignature(input: SlackSignatureInput): boolean { + return checkSlackSignature(input) === undefined; +}