diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d38fd047..28619402c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Docs: https://docs.clawd.bot - Web search: infer Perplexity base URL from API key source (direct vs OpenRouter). - TUI: keep thinking blocks ordered before content during streaming and isolate per-run assembly. (#1202) — thanks @aaronveklabs. - CLI: avoid duplicating --profile/--dev flags when formatting commands. +- Agents: enforce tool policy/sandbox rules for tool-dispatched skill commands and forward tool media outputs. (#1235) — thanks @dougvk. ## 2026.1.19-3 diff --git a/docs/tools/skills.md b/docs/tools/skills.md index 2a5838af5..4b3bdec79 100644 --- a/docs/tools/skills.md +++ b/docs/tools/skills.md @@ -88,6 +88,8 @@ Notes: The tool is invoked with params: `{ command: "", commandName: "", skillName: "" }`. + Tool-dispatch commands still respect tool policies/sandbox rules (same as normal model tools). + Replies are derived from tool results; include `MEDIA:` tokens or media URLs in tool output to send attachments. ## Gating (load-time filters) diff --git a/docs/tools/slash-commands.md b/docs/tools/slash-commands.md index 7ddc2f4e7..d5dbab12d 100644 --- a/docs/tools/slash-commands.md +++ b/docs/tools/slash-commands.md @@ -104,6 +104,7 @@ Notes: - **Skill commands:** `user-invocable` skills are exposed as slash commands. Names are sanitized to `a-z0-9_` (max 32 chars); collisions get numeric suffixes (e.g. `_2`). - By default, skill commands are forwarded to the model as a normal request. - Skills may optionally declare `command-dispatch: tool` to route the command directly to a tool (deterministic, no model). + - Tool-dispatch still respects tool policies/sandbox rules; replies use the tool result text/media (including `MEDIA:` tokens). - **Native command arguments:** Discord uses autocomplete for dynamic options (and button menus when you omit required args). Telegram and Slack show a button menu when a command supports choices and you omit the arg. ## Usage surfaces (what shows where) diff --git a/src/agents/session-tool-result-guard.tool-result-persist-hook.test.ts b/src/agents/session-tool-result-guard.tool-result-persist-hook.test.ts index ac6aea396..163742376 100644 --- a/src/agents/session-tool-result-guard.tool-result-persist-hook.test.ts +++ b/src/agents/session-tool-result-guard.tool-result-persist-hook.test.ts @@ -8,6 +8,8 @@ import { describe, expect, it, afterEach } from "vitest"; import { loadClawdbotPlugins } from "../plugins/loader.js"; import { resetGlobalHookRunner } from "../plugins/hook-runner-global.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createTestRegistry } from "../test-utils/channel-plugins.js"; import { guardSessionManager } from "./session-tool-result-guard-wrapper.js"; const EMPTY_CONFIG_SCHEMA = `configSchema: { @@ -22,7 +24,15 @@ function writeTempPlugin(params: { dir: string; id: string; body: string }): str return file; } +const ORIGINAL_BUNDLED_PLUGINS_DIR = process.env.CLAWDBOT_BUNDLED_PLUGINS_DIR; + afterEach(() => { + if (ORIGINAL_BUNDLED_PLUGINS_DIR === undefined) { + delete process.env.CLAWDBOT_BUNDLED_PLUGINS_DIR; + } else { + process.env.CLAWDBOT_BUNDLED_PLUGINS_DIR = ORIGINAL_BUNDLED_PLUGINS_DIR; + } + setActivePluginRegistry(createTestRegistry([])); resetGlobalHookRunner(); }); diff --git a/src/auto-reply/reply/get-reply-inline-actions.tool-dispatch.test.ts b/src/auto-reply/reply/get-reply-inline-actions.tool-dispatch.test.ts new file mode 100644 index 000000000..2508bef51 --- /dev/null +++ b/src/auto-reply/reply/get-reply-inline-actions.tool-dispatch.test.ts @@ -0,0 +1,177 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import type { SkillCommandSpec } from "../../agents/skills.js"; +import type { AnyAgentTool } from "../../agents/pi-tools.types.js"; +import type { ClawdbotConfig } from "../../config/config.js"; +import type { MsgContext, TemplateContext } from "../templating.js"; +import type { InlineDirectives } from "./directive-handling.js"; +import { parseInlineDirectives } from "./directive-handling.js"; +import type { TypingController } from "./typing.js"; +import { handleInlineActions } from "./get-reply-inline-actions.js"; + +vi.mock("../../agents/clawdbot-tools.js", () => ({ + createClawdbotTools: vi.fn(), +})); + +import { createClawdbotTools } from "../../agents/clawdbot-tools.js"; + +const mockedCreateClawdbotTools = vi.mocked(createClawdbotTools); + +const createTypingController = (): TypingController => ({ + onReplyStart: vi.fn(), + startTypingLoop: vi.fn(), + startTypingOnText: vi.fn(), + refreshTypingTtl: vi.fn(), + isActive: vi.fn(() => false), + markRunComplete: vi.fn(), + markDispatchIdle: vi.fn(), + cleanup: vi.fn(), +}); + +const baseCommand = { + surface: "slack", + channel: "slack", + ownerList: [], + isAuthorizedSender: true, + rawBodyNormalized: "/dispatch", + commandBodyNormalized: "/dispatch", + senderId: "user-1", +}; + +const baseDirectives = parseInlineDirectives("/dispatch") as InlineDirectives; + +function createParams(overrides: Partial[0]> = {}) { + const cfg = (overrides.cfg ?? + ({ + tools: { + allow: ["tool_allowed"], + }, + } as ClawdbotConfig)) as ClawdbotConfig; + return { + ctx: { + Surface: "slack", + Provider: "slack", + AccountId: "default", + } satisfies MsgContext as MsgContext, + sessionCtx: { + Body: "", + BodyForAgent: "", + BodyStripped: "", + } satisfies TemplateContext as TemplateContext, + cfg, + agentId: "main", + agentDir: "/tmp", + sessionEntry: undefined, + previousSessionEntry: undefined, + sessionStore: undefined, + sessionKey: "main", + storePath: undefined, + sessionScope: "per-sender", + workspaceDir: "/tmp", + isGroup: false, + opts: undefined, + typing: createTypingController(), + allowTextCommands: true, + inlineStatusRequested: false, + command: baseCommand, + skillCommands: [], + directives: baseDirectives, + cleanedBody: "/dispatch", + elevatedEnabled: false, + elevatedAllowed: false, + elevatedFailures: [], + defaultActivation: () => "always", + resolvedThinkLevel: "off", + resolvedVerboseLevel: "off", + resolvedReasoningLevel: "off", + resolvedElevatedLevel: "off", + resolveDefaultThinkingLevel: async () => undefined, + provider: "openai", + model: "gpt-4o-mini", + contextTokens: 0, + directiveAck: undefined, + abortedLastRun: false, + skillFilter: undefined, + ...overrides, + }; +} + +function createTool(name: string, execute: AnyAgentTool["execute"]): AnyAgentTool { + return { + name, + label: name, + description: name, + parameters: {}, + execute, + } as AnyAgentTool; +} + +describe("handleInlineActions tool-dispatch", () => { + beforeEach(() => { + mockedCreateClawdbotTools.mockReset(); + }); + + it("returns media payloads from tool results", async () => { + const tool = createTool("tool_allowed", async () => ({ + content: [{ type: "text", text: "Done\nMEDIA:/tmp/photo.jpg" }], + })); + mockedCreateClawdbotTools.mockReturnValue([tool]); + + const skillCommands: SkillCommandSpec[] = [ + { + name: "dispatch", + skillName: "dispatch", + description: "Dispatch", + dispatch: { kind: "tool", toolName: "tool_allowed", argMode: "raw" }, + }, + ]; + + const result = await handleInlineActions( + createParams({ + command: { ...baseCommand, commandBodyNormalized: "/dispatch hi" }, + skillCommands, + cleanedBody: "/dispatch hi", + }), + ); + + expect(result.kind).toBe("reply"); + const reply = (result as { reply?: unknown }).reply as { text?: string; mediaUrl?: string }; + expect(reply.text).toBe("Done"); + expect(reply.mediaUrl).toBe("file:///tmp/photo.jpg"); + }); + + it("blocks tool dispatch when policy disallows the tool", async () => { + const allowed = createTool("tool_allowed", async () => ({ content: "ok" })); + const blocked = createTool("tool_blocked", async () => ({ content: "nope" })); + mockedCreateClawdbotTools.mockReturnValue([allowed, blocked]); + + const cfg = { + tools: { + allow: ["tool_allowed"], + }, + } as ClawdbotConfig; + + const skillCommands: SkillCommandSpec[] = [ + { + name: "dispatch", + skillName: "dispatch", + description: "Dispatch", + dispatch: { kind: "tool", toolName: "tool_blocked", argMode: "raw" }, + }, + ]; + + const result = await handleInlineActions( + createParams({ + cfg, + skillCommands, + command: { ...baseCommand, commandBodyNormalized: "/dispatch arg" }, + cleanedBody: "/dispatch arg", + }), + ); + + expect(result.kind).toBe("reply"); + const reply = (result as { reply?: { text?: string; isError?: boolean } }).reply; + expect(reply?.text).toContain("Tool blocked by policy"); + expect(reply?.isError).toBe(true); + }); +}); diff --git a/src/auto-reply/reply/get-reply-inline-actions.ts b/src/auto-reply/reply/get-reply-inline-actions.ts index 14bc024c0..6c7bca4ef 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.ts @@ -1,21 +1,47 @@ +import { pathToFileURL } from "node:url"; + import { getChannelDock } from "../../channels/dock.js"; import type { SkillCommandSpec } from "../../agents/skills.js"; +import type { AnyAgentTool } from "../../agents/pi-tools.types.js"; import type { ClawdbotConfig } from "../../config/config.js"; import type { SessionEntry } from "../../config/sessions.js"; import type { MsgContext, TemplateContext } from "../templating.js"; import type { ElevatedLevel, ReasoningLevel, ThinkLevel, VerboseLevel } from "../thinking.js"; import type { GetReplyOptions, ReplyPayload } from "../types.js"; +import { + resolveSubagentToolPolicy, + resolveEffectiveToolPolicy, + filterToolsByPolicy, +} from "../../agents/pi-tools.policy.js"; +import { + buildPluginToolGroups, + collectExplicitAllowlist, + expandPolicyWithPluginGroups, + normalizeToolName, + resolveToolProfilePolicy, +} from "../../agents/tool-policy.js"; +import { + resolveSandboxRuntimeStatus, + formatSandboxToolPolicyBlockedMessage, +} from "../../agents/sandbox/runtime-status.js"; +import { isSubagentSessionKey } from "../../routing/session-key.js"; +import { resolveUserPath } from "../../utils.js"; import { getAbortMemory } from "./abort.js"; import { buildStatusReply, handleCommands } from "./commands.js"; import type { InlineDirectives } from "./directive-handling.js"; import { isDirectiveOnly } from "./directive-handling.js"; import type { createModelSelectionState } from "./model-selection.js"; import { extractInlineSimpleCommand } from "./reply-inline.js"; +import { parseReplyDirectives } from "./reply-directives.js"; import type { TypingController } from "./typing.js"; import { listSkillCommandsForWorkspace, resolveSkillCommandInvocation } from "../skill-commands.js"; import { logVerbose } from "../../globals.js"; import { createClawdbotTools } from "../../agents/clawdbot-tools.js"; -import { resolveGatewayMessageChannel } from "../../utils/message-channel.js"; +import { + resolveGatewayMessageChannel, + type GatewayMessageChannel, +} from "../../utils/message-channel.js"; +import { getPluginToolMeta } from "../../plugins/tools.js"; export type InlineActionResult = | { kind: "reply"; reply: ReplyPayload | ReplyPayload[] | undefined } @@ -25,9 +51,20 @@ export type InlineActionResult = abortedLastRun: boolean; }; -function extractTextFromToolResult(result: any): string | null { - if (!result || typeof result !== "object") return null; - const content = (result as { content?: unknown }).content; +function normalizeMediaUrlCandidate(raw: string): string | null { + const trimmed = raw.trim(); + if (!trimmed) return null; + if (/^https?:\/\//i.test(trimmed)) return trimmed; + if (trimmed.startsWith("file://")) return trimmed; + const resolved = trimmed.startsWith("~") ? resolveUserPath(trimmed) : trimmed; + if (resolved.startsWith("/")) { + return pathToFileURL(resolved).toString(); + } + if (resolved.startsWith("./") || resolved.startsWith("../")) return resolved; + return trimmed; +} + +function extractTextFromToolResultContent(content: unknown): string | null { if (typeof content === "string") { const trimmed = content.trim(); return trimmed ? trimmed : null; @@ -42,9 +79,202 @@ function extractTextFromToolResult(result: any): string | null { parts.push(rec.text); } } - const out = parts.join(""); - const trimmed = out.trim(); - return trimmed ? trimmed : null; + const out = parts.join("\n").trim(); + return out ? out : null; +} + +function extractMediaUrlsFromDetails(details: unknown): string[] { + if (!details || typeof details !== "object") return []; + const record = details as Record; + const candidates: string[] = []; + const mediaUrls = record.mediaUrls; + if (Array.isArray(mediaUrls)) { + for (const entry of mediaUrls) { + if (typeof entry === "string") candidates.push(entry); + } + } + const mediaUrl = record.mediaUrl; + if (typeof mediaUrl === "string") candidates.push(mediaUrl); + const media = record.media; + if (typeof media === "string") candidates.push(media); + const path = record.path; + if (typeof path === "string") candidates.push(path); + return candidates + .map((entry) => normalizeMediaUrlCandidate(entry)) + .filter((entry): entry is string => Boolean(entry)); +} + +function extractReplyPayloadFromToolResult(result: unknown): ReplyPayload | null { + if (!result || typeof result !== "object") return null; + + const maybePayload = result as ReplyPayload & { content?: unknown; details?: unknown }; + if ( + typeof maybePayload.text === "string" || + typeof maybePayload.mediaUrl === "string" || + Array.isArray(maybePayload.mediaUrls) + ) { + return { + text: maybePayload.text?.trim() ? maybePayload.text.trim() : undefined, + mediaUrl: maybePayload.mediaUrl, + mediaUrls: maybePayload.mediaUrls, + replyToId: maybePayload.replyToId, + replyToTag: maybePayload.replyToTag, + replyToCurrent: maybePayload.replyToCurrent, + audioAsVoice: maybePayload.audioAsVoice, + isError: maybePayload.isError, + }; + } + + const content = maybePayload.content; + const text = extractTextFromToolResultContent(content); + const parsed = text + ? parseReplyDirectives(text) + : { + text: "", + mediaUrls: undefined, + mediaUrl: undefined, + replyToId: undefined, + replyToCurrent: false, + replyToTag: false, + audioAsVoice: undefined, + isSilent: false, + }; + + if (parsed.isSilent) return null; + + const mediaFromText = parsed.mediaUrls ?? (parsed.mediaUrl ? [parsed.mediaUrl] : []); + const mediaFromDetails = extractMediaUrlsFromDetails(maybePayload.details); + const mediaUrls = Array.from(new Set([...mediaFromText, ...mediaFromDetails])) + .map((entry) => normalizeMediaUrlCandidate(entry)) + .filter((entry): entry is string => Boolean(entry)); + + const cleanedText = parsed.text?.trim() ? parsed.text.trim() : undefined; + if (!cleanedText && mediaUrls.length === 0) return null; + + return { + text: cleanedText, + mediaUrls: mediaUrls.length ? mediaUrls : undefined, + mediaUrl: mediaUrls[0], + replyToId: parsed.replyToId, + replyToTag: parsed.replyToTag, + replyToCurrent: parsed.replyToCurrent, + audioAsVoice: parsed.audioAsVoice, + isError: (result as { isError?: unknown }).isError === true, + }; +} + +function resolveToolDispatchTools(params: { + cfg: ClawdbotConfig; + sessionKey: string; + provider: string; + model: string; + agentDir?: string; + agentChannel?: GatewayMessageChannel; + agentAccountId?: string; + workspaceDir: string; +}): { + allTools: AnyAgentTool[]; + allowedTools: AnyAgentTool[]; + sandboxed: boolean; +} { + const sandboxRuntime = resolveSandboxRuntimeStatus({ + cfg: params.cfg, + sessionKey: params.sessionKey, + }); + const { + profile, + providerProfile, + globalPolicy, + globalProviderPolicy, + agentPolicy, + agentProviderPolicy, + } = resolveEffectiveToolPolicy({ + config: params.cfg, + sessionKey: params.sessionKey, + modelProvider: params.provider, + modelId: params.model, + }); + const profilePolicy = resolveToolProfilePolicy(profile); + const providerProfilePolicy = resolveToolProfilePolicy(providerProfile); + const sandboxPolicy = sandboxRuntime.sandboxed ? sandboxRuntime.toolPolicy : undefined; + const subagentPolicy = + isSubagentSessionKey(params.sessionKey) && params.sessionKey + ? resolveSubagentToolPolicy(params.cfg) + : undefined; + const pluginToolAllowlist = collectExplicitAllowlist([ + profilePolicy, + providerProfilePolicy, + globalPolicy, + globalProviderPolicy, + agentPolicy, + agentProviderPolicy, + sandboxPolicy, + subagentPolicy, + ]); + + const allTools = createClawdbotTools({ + agentSessionKey: params.sessionKey, + agentChannel: params.agentChannel, + agentAccountId: params.agentAccountId, + agentDir: params.agentDir, + workspaceDir: params.workspaceDir, + config: params.cfg, + sandboxed: sandboxRuntime.sandboxed, + pluginToolAllowlist, + }) as AnyAgentTool[]; + + const pluginGroups = buildPluginToolGroups({ + tools: allTools, + toolMeta: (tool) => getPluginToolMeta(tool), + }); + const profilePolicyExpanded = expandPolicyWithPluginGroups(profilePolicy, pluginGroups); + const providerProfilePolicyExpanded = expandPolicyWithPluginGroups( + providerProfilePolicy, + pluginGroups, + ); + const globalPolicyExpanded = expandPolicyWithPluginGroups(globalPolicy, pluginGroups); + const globalProviderPolicyExpanded = expandPolicyWithPluginGroups( + globalProviderPolicy, + pluginGroups, + ); + const agentPolicyExpanded = expandPolicyWithPluginGroups(agentPolicy, pluginGroups); + const agentProviderPolicyExpanded = expandPolicyWithPluginGroups( + agentProviderPolicy, + pluginGroups, + ); + const sandboxPolicyExpanded = expandPolicyWithPluginGroups(sandboxPolicy, pluginGroups); + const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups); + + const toolsFiltered = profilePolicyExpanded + ? filterToolsByPolicy(allTools, profilePolicyExpanded) + : allTools; + const providerProfileFiltered = providerProfilePolicyExpanded + ? filterToolsByPolicy(toolsFiltered, providerProfilePolicyExpanded) + : toolsFiltered; + const globalFiltered = globalPolicyExpanded + ? filterToolsByPolicy(providerProfileFiltered, globalPolicyExpanded) + : providerProfileFiltered; + const globalProviderFiltered = globalProviderPolicyExpanded + ? filterToolsByPolicy(globalFiltered, globalProviderPolicyExpanded) + : globalFiltered; + const agentFiltered = agentPolicyExpanded + ? filterToolsByPolicy(globalProviderFiltered, agentPolicyExpanded) + : globalProviderFiltered; + const agentProviderFiltered = agentProviderPolicyExpanded + ? filterToolsByPolicy(agentFiltered, agentProviderPolicyExpanded) + : agentFiltered; + const sandboxed = sandboxPolicyExpanded + ? filterToolsByPolicy(agentProviderFiltered, sandboxPolicyExpanded) + : agentProviderFiltered; + const subagentFiltered = subagentPolicyExpanded + ? filterToolsByPolicy(sandboxed, subagentPolicyExpanded) + : sandboxed; + + return { + allTools, + allowedTools: subagentFiltered, + sandboxed: sandboxRuntime.sandboxed, + }; } export async function handleInlineActions(params: { @@ -163,18 +393,34 @@ export async function handleInlineActions(params: { resolveGatewayMessageChannel(ctx.Surface) ?? resolveGatewayMessageChannel(ctx.Provider) ?? undefined; - - const tools = createClawdbotTools({ - agentSessionKey: sessionKey, + const { allTools, allowedTools } = resolveToolDispatchTools({ + cfg, + sessionKey, + provider, + model, + agentDir, agentChannel: channel, agentAccountId: (ctx as { AccountId?: string }).AccountId, - agentDir, workspaceDir, - config: cfg, }); - - const tool = tools.find((candidate) => candidate.name === dispatch.toolName); + const requestedName = normalizeToolName(dispatch.toolName); + const findTool = (tools: AnyAgentTool[]) => + tools.find((candidate) => normalizeToolName(candidate.name) === requestedName); + const tool = findTool(allowedTools); if (!tool) { + const allTool = findTool(allTools); + if (allTool) { + const sandboxReason = formatSandboxToolPolicyBlockedMessage({ + cfg, + sessionKey, + toolName: requestedName, + }); + const message = sandboxReason + ? `❌ Tool blocked by policy: ${dispatch.toolName}\n${sandboxReason}` + : `❌ Tool blocked by policy: ${dispatch.toolName}`; + typing.cleanup(); + return { kind: "reply", reply: { text: message, isError: true } }; + } typing.cleanup(); return { kind: "reply", reply: { text: `❌ Tool not available: ${dispatch.toolName}` } }; } @@ -186,13 +432,13 @@ export async function handleInlineActions(params: { commandName: skillInvocation.command.name, skillName: skillInvocation.command.skillName, } as any); - const text = extractTextFromToolResult(result) ?? "✅ Done."; + const reply = extractReplyPayloadFromToolResult(result) ?? { text: "✅ Done." }; typing.cleanup(); - return { kind: "reply", reply: { text } }; + return { kind: "reply", reply }; } catch (err) { const message = err instanceof Error ? err.message : String(err); typing.cleanup(); - return { kind: "reply", reply: { text: `❌ ${message}` } }; + return { kind: "reply", reply: { text: `❌ ${message}`, isError: true } }; } }