fix: propagate ClickClack toolsAllow through replies

Propagate ClickClack account-level runtime tool allowlists through inbound reply dispatch so restricted ClickClack accounts keep their tool policy when model/agent replies are generated.

This threads `toolsAllow` through shared dispatch, provider wrappers, embedded agent execution, and ACP hook events. ACP-bound sessions now fail closed for restrictive runtime allowlists because ACPX cannot enforce per-turn tool allowlists on reused persistent sessions.

Verification:
- Live ClickClack E2E on Crabbox AWS `run_6a0472ed7e71`, provider `aws`, id `cbx_dace25addcaa`.
- `node scripts/run-vitest.mjs run src/auto-reply/reply/dispatch-acp.test.ts src/plugin-sdk/acp-runtime.test.ts src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts src/auto-reply/dispatch.test.ts src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/provider-dispatcher.test.ts extensions/clickclack/src/inbound.test.ts --reporter=verbose`
- Crabbox changed gate `run_d32af37fb265`, provider `aws`, id `cbx_8236876017c9`: `corepack pnpm check:changed`
- Autoreview clean: `.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main`

Supersedes #89500.

Co-authored-by: Michael Appel <mappel@nvidia.com>
This commit is contained in:
Peter Steinberger
2026-06-05 08:40:35 -07:00
committed by GitHub
parent 5a0f9cb03c
commit 797bcd5bdb
19 changed files with 256 additions and 3 deletions

View File

@@ -219,6 +219,38 @@ describe("handleClickClackInbound", () => {
expect(dispatchReply.mock.calls[0]?.[0].ctxPayload.CommandAuthorized).toBe(true);
});
it("propagates account toolsAllow into agent reply dispatch", async () => {
const runtime = createRuntime();
setClickClackRuntime(runtime);
const cfg = {
agents: {
defaults: {
model: "openai/gpt-5.4-mini",
},
},
tools: {
allow: ["*"],
},
} satisfies CoreConfig;
await handleClickClackInbound({
account: createAgentAccount({
toolsAllow: ["message"],
}),
config: cfg,
message: createMessage(),
});
const dispatchReply = vi.mocked(runtime.channel.inbound.dispatchReply);
expect(dispatchReply).toHaveBeenCalledTimes(1);
const dispatchParams = dispatchReply.mock.calls[0]?.[0] as
| (Record<string, unknown> & {
toolsAllow?: unknown;
})
| undefined;
expect(dispatchParams?.toolsAllow).toEqual(["message"]);
});
it("accepts ClickClack DM target syntax in allowFrom", async () => {
const runtime = createRuntime();
vi.mocked(runtime.channel.commands.shouldComputeCommandAuthorized).mockReturnValue(true);

View File

@@ -184,6 +184,7 @@ export async function handleClickClackInbound(params: {
recordInboundSession: runtime.channel.session.recordInboundSession,
dispatchReplyWithBufferedBlockDispatcher:
runtime.channel.reply.dispatchReplyWithBufferedBlockDispatcher,
toolsAllow: params.account.toolsAllow,
delivery: {
deliver: async (payload) => {
const text =

View File

@@ -270,6 +270,31 @@ describe("withReplyDispatcher", () => {
expect(typing.markDispatchIdle).toHaveBeenCalledTimes(1);
});
it("passes runtime toolsAllow from buffered dispatch into reply resolution", async () => {
hoisted.createReplyDispatcherWithTypingMock.mockReturnValueOnce({
dispatcher: createDispatcher([]),
replyOptions: {},
markDispatchIdle: vi.fn(),
markRunComplete: vi.fn(),
});
hoisted.dispatchReplyFromConfigMock.mockResolvedValueOnce({
queuedFinal: false,
counts: { tool: 0, block: 0, final: 0 },
});
await dispatchInboundMessageWithBufferedDispatcher({
ctx: buildTestCtx(),
cfg: {} as OpenClawConfig,
toolsAllow: ["message"],
dispatcherOptions: {
deliver: async () => undefined,
},
});
const params = hoisted.dispatchReplyFromConfigMock.mock.calls[0]?.[0];
expect(params?.replyOptions?.toolsAllow).toEqual(["message"]);
});
it("runs message_sending hooks before inbound dispatcher delivery", async () => {
const runMessageSending = vi.fn(async () => ({ content: "sanitized reply" }));
hoisted.getGlobalHookRunnerMock.mockReturnValue({

View File

@@ -52,6 +52,19 @@ type ForegroundReplyFenceSnapshot = {
const foregroundReplyFenceByKey = new Map<string, ForegroundReplyFenceState>();
const replyPayloadSendingDispatchers = new WeakSet<ReplyDispatcher>();
function applyRuntimeToolsAllow(
replyOptions: Omit<GetReplyOptions, "onBlockReply"> | undefined,
toolsAllow: string[] | undefined,
): Omit<GetReplyOptions, "onBlockReply"> | undefined {
if (toolsAllow === undefined) {
return replyOptions;
}
return {
...replyOptions,
toolsAllow,
};
}
function normalizeForegroundReplyFencePart(value: unknown): string | undefined {
if (typeof value !== "string") {
return undefined;
@@ -461,9 +474,11 @@ export async function dispatchInboundMessage(params: {
ctx: MsgContext | FinalizedMsgContext;
cfg: OpenClawConfig;
dispatcher: ReplyDispatcher;
toolsAllow?: string[];
replyOptions?: Omit<GetReplyOptions, "onBlockReply">;
replyResolver?: GetReplyFromConfig;
}): Promise<DispatchInboundResult> {
const replyOptions = applyRuntimeToolsAllow(params.replyOptions, params.toolsAllow);
const finalized = measureDiagnosticsTimelineSpanSync(
"auto_reply.finalize_context",
() => finalizeInboundContext(params.ctx),
@@ -483,7 +498,7 @@ export async function dispatchInboundMessage(params: {
});
}
installReplyPayloadSendingBeforeDeliver(params.dispatcher, finalized, {
runId: params.replyOptions?.runId,
runId: replyOptions?.runId,
});
const result = await withReplyDispatcher({
dispatcher: params.dispatcher,
@@ -495,7 +510,7 @@ export async function dispatchInboundMessage(params: {
ctx: finalized,
cfg: params.cfg,
dispatcher: params.dispatcher,
replyOptions: params.replyOptions,
replyOptions,
replyResolver: params.replyResolver,
}),
{
@@ -513,6 +528,7 @@ export async function dispatchInboundMessageWithBufferedDispatcher(params: {
ctx: MsgContext | FinalizedMsgContext;
cfg: OpenClawConfig;
dispatcherOptions: ReplyDispatcherWithTypingOptions;
toolsAllow?: string[];
replyOptions?: Omit<GetReplyOptions, "onBlockReply">;
replyResolver?: GetReplyFromConfig;
}): Promise<DispatchInboundResult> {
@@ -575,6 +591,7 @@ export async function dispatchInboundMessageWithBufferedDispatcher(params: {
ctx: finalized,
cfg: params.cfg,
dispatcher,
toolsAllow: params.toolsAllow,
replyResolver: params.replyResolver,
replyOptions: {
...params.replyOptions,
@@ -606,6 +623,7 @@ export async function dispatchInboundMessageWithDispatcher(params: {
ctx: MsgContext | FinalizedMsgContext;
cfg: OpenClawConfig;
dispatcherOptions: ReplyDispatcherOptions;
toolsAllow?: string[];
replyOptions?: Omit<GetReplyOptions, "onBlockReply">;
replyResolver?: GetReplyFromConfig;
}): Promise<DispatchInboundResult> {
@@ -630,6 +648,7 @@ export async function dispatchInboundMessageWithDispatcher(params: {
ctx: params.ctx,
cfg: params.cfg,
dispatcher,
toolsAllow: params.toolsAllow,
replyResolver: params.replyResolver,
replyOptions: params.replyOptions,
});

View File

@@ -91,6 +91,8 @@ export type GetReplyOptions = {
shouldSuppressToolErrorWarnings?: () => boolean | undefined;
/** If true, run the model without OpenClaw tools for this turn. */
disableTools?: boolean;
/** Runtime tool allow-list for this turn. Empty means no tools. */
toolsAllow?: string[];
/** If true, include the heartbeat response tool for structured heartbeat outcomes. */
enableHeartbeatTool?: boolean;
/** If true, keep the heartbeat response tool available even under narrow tool profiles. */

View File

@@ -1183,6 +1183,26 @@ describe("runAgentTurnWithFallback", () => {
expect(embeddedCall.abortSignal).toBe(replyOperation.abortSignal);
});
it("passes runtime toolsAllow to embedded agent runs", async () => {
state.runEmbeddedAgentMock.mockResolvedValueOnce({
payloads: [{ text: "ok" }],
meta: {},
});
const runAgentTurnWithFallback = await getRunAgentTurnWithFallback();
await runAgentTurnWithFallback(
createMinimalRunAgentTurnParams({
opts: {
toolsAllow: ["message"],
},
}),
);
expectMockCallArgFields(state.runEmbeddedAgentMock, 0, "embedded run params", {
toolsAllow: ["message"],
});
});
it("rechecks queued auto fallback primary probes before running", async () => {
const { markAutoFallbackPrimaryProbe } = await import("../../agents/agent-scope.js");
const probe = {

View File

@@ -2212,6 +2212,7 @@ export async function runAgentTurnWithFallback(params: {
currentInboundAudio: hasInboundAudio(params.sessionCtx),
agentAccountId: params.followupRun.run.agentAccountId,
senderIsOwner: params.followupRun.run.senderIsOwner,
toolsAllow: params.opts?.toolsAllow,
disableTools: params.opts?.disableTools,
abortSignal: runAbortSignal,
replyOperation: params.replyOperation,
@@ -2329,6 +2330,7 @@ export async function runAgentTurnWithFallback(params: {
suppressToolErrorWarnings:
params.opts?.shouldSuppressToolErrorWarnings ??
params.opts?.suppressToolErrorWarnings,
toolsAllow: params.opts?.toolsAllow,
disableTools: params.opts?.disableTools,
enableHeartbeatTool: params.opts?.enableHeartbeatTool,
forceHeartbeatTool: params.opts?.forceHeartbeatTool,

View File

@@ -305,6 +305,7 @@ async function runDispatch(params: {
suppressUserDelivery?: boolean;
suppressReplyLifecycle?: boolean;
sourceReplyDeliveryMode?: "automatic" | "message_tool_only";
toolsAllow?: string[];
}) {
const targetSessionKey = params.sessionKeyOverride ?? sessionKey;
return tryDispatchAcpReply({
@@ -332,6 +333,7 @@ async function runDispatch(params: {
: {}),
shouldSendToolSummaries: true,
bypassForCommand: false,
toolsAllow: params.toolsAllow,
...(params.onReplyStart ? { onReplyStart: params.onReplyStart } : {}),
recordProcessed: vi.fn(),
markIdle: vi.fn(),
@@ -1388,6 +1390,35 @@ describe("tryDispatchAcpReply", () => {
expect(bindingServiceMocks.unbind).not.toHaveBeenCalled();
});
it("fails closed when ACP dispatch cannot enforce restrictive runtime toolsAllow", async () => {
setReadyAcpResolution();
const { dispatcher } = createDispatcher();
await runDispatch({
bodyForAgent: "test",
dispatcher,
toolsAllow: ["message"],
});
expect(managerMocks.runTurn).not.toHaveBeenCalled();
expect(dispatcherCall(dispatcher.sendFinalReply).isError).toBe(true);
expect(dispatcherCall(dispatcher.sendFinalReply).text).toContain("runtime toolsAllow");
});
it("allows wildcard runtime toolsAllow through ACP dispatch", async () => {
setReadyAcpResolution();
const { dispatcher } = createDispatcher();
await runDispatch({
bodyForAgent: "test",
dispatcher,
toolsAllow: ["*"],
});
expect(managerMocks.runTurn).toHaveBeenCalledOnce();
expect(runTurnCall().text).toBe("test");
});
it("does not unbind stale bindings when ACP dispatch is disabled by policy", async () => {
managerMocks.resolveSession.mockReturnValue({
kind: "stale",

View File

@@ -11,7 +11,7 @@ import {
normalizeOptionalString,
} from "@openclaw/normalization-core/string-coerce";
import { resolveAcpAgentPolicyError, resolveAcpDispatchPolicyError } from "../../acp/policy.js";
import { type AcpRuntimeError, toAcpRuntimeError } from "../../acp/runtime/errors.js";
import { AcpRuntimeError, toAcpRuntimeError } from "../../acp/runtime/errors.js";
import { resolveAgentDir, resolveAgentWorkspaceDir } from "../../agents/agent-scope.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import type { TtsAutoMode } from "../../config/types.tts.js";
@@ -122,6 +122,13 @@ function resolveAcpTurnText(params: {
return params.promptText ? `${guidance}\n\n${params.promptText}` : guidance;
}
function isRestrictiveRuntimeToolsAllow(toolsAllow: string[] | undefined): boolean {
if (toolsAllow === undefined) {
return false;
}
return !toolsAllow.some((entry) => normalizeLowercaseStringOrEmpty(entry) === "*");
}
async function hasBoundConversationForSession(params: {
cfg: OpenClawConfig;
sessionKey: string;
@@ -361,6 +368,7 @@ export async function tryDispatchAcpReply(params: {
dispatcher: ReplyDispatcher;
runId?: string;
sessionKey?: string;
toolsAllow?: string[];
images?: Array<{ data: string; mimeType: string }>;
abortSignal?: AbortSignal;
inboundAudio: boolean;
@@ -505,6 +513,12 @@ export async function tryDispatchAcpReply(params: {
if (dispatchPolicyError) {
throw dispatchPolicyError;
}
if (isRestrictiveRuntimeToolsAllow(params.toolsAllow)) {
throw new AcpRuntimeError(
"ACP_DISPATCH_DISABLED",
"ACP dispatch cannot enforce runtime toolsAllow for this session; use an embedded runtime for restricted tool policy.",
);
}
if (acpResolution.kind === "stale") {
await maybeUnbindStaleBoundConversations({
targetSessionKey: canonicalSessionKey,

View File

@@ -35,6 +35,7 @@ function firstReplyDispatchCall() {
| [
{
sessionKey?: string;
toolsAllow?: string[];
sendPolicy?: string;
inboundAudio?: boolean;
},
@@ -128,6 +129,7 @@ describe("dispatchReplyFromConfig reply_dispatch hook", () => {
dispatcher: createDispatcher(),
fastAbortResolver: async () => ({ handled: false, aborted: false }),
formatAbortReplyTextResolver: () => "⚙️ Agent was aborted.",
replyOptions: { toolsAllow: ["message"] },
replyResolver: async () => ({ text: "model reply" }),
});
@@ -140,6 +142,7 @@ describe("dispatchReplyFromConfig reply_dispatch hook", () => {
expect(hookMocks.runner.runReplyDispatch).toHaveBeenCalledOnce();
const [replyDispatchEvent, replyDispatchRuntime] = firstReplyDispatchCall() ?? [];
expect(replyDispatchEvent?.sessionKey).toBe("agent:test:session");
expect(replyDispatchEvent?.toolsAllow).toEqual(["message"]);
expect(replyDispatchEvent?.sendPolicy).toBe("allow");
expect(replyDispatchEvent?.inboundAudio).toBe(false);
expect(replyDispatchRuntime?.cfg).toBe(emptyConfig);

View File

@@ -2138,6 +2138,7 @@ export async function dispatchReplyFromConfig(
ctx,
runId: params.replyOptions?.runId,
sessionKey: acpDispatchSessionKey,
toolsAllow: params.replyOptions?.toolsAllow,
images: params.replyOptions?.images,
inboundAudio,
sessionTtsAuto,
@@ -2777,6 +2778,7 @@ export async function dispatchReplyFromConfig(
ctx,
runId: params.replyOptions?.runId,
sessionKey: acpDispatchSessionKey,
toolsAllow: params.replyOptions?.toolsAllow,
images: params.replyOptions?.images,
inboundAudio,
sessionTtsAuto,

View File

@@ -0,0 +1,80 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import type {
ReplyDispatcherOptions,
ReplyDispatcherWithTypingOptions,
} from "./reply-dispatcher.js";
type BufferedDispatchFn =
typeof import("../dispatch.js").dispatchInboundMessageWithBufferedDispatcher;
type PlainDispatchFn = typeof import("../dispatch.js").dispatchInboundMessageWithDispatcher;
const hoisted = vi.hoisted(() => ({
bufferedDispatchMock: vi.fn(),
plainDispatchMock: vi.fn(),
}));
vi.mock("../dispatch.js", () => ({
dispatchInboundMessageWithBufferedDispatcher: (...args: Parameters<BufferedDispatchFn>) =>
hoisted.bufferedDispatchMock(...args),
dispatchInboundMessageWithDispatcher: (...args: Parameters<PlainDispatchFn>) =>
hoisted.plainDispatchMock(...args),
}));
const { dispatchReplyWithBufferedBlockDispatcher, dispatchReplyWithDispatcher } =
await import("./provider-dispatcher.js");
const dispatchResult = {
queuedFinal: false,
counts: { tool: 0, block: 0, final: 0 },
};
describe("provider dispatcher wrappers", () => {
beforeEach(() => {
vi.clearAllMocks();
hoisted.bufferedDispatchMock.mockResolvedValue(dispatchResult);
hoisted.plainDispatchMock.mockResolvedValue(dispatchResult);
});
it("forwards runtime toolsAllow through the buffered wrapper", async () => {
const dispatcherOptions = {
deliver: async () => ({ visibleReplySent: false }),
} satisfies ReplyDispatcherWithTypingOptions;
await dispatchReplyWithBufferedBlockDispatcher({
ctx: { Body: "hello" },
cfg: {} as OpenClawConfig,
dispatcherOptions,
toolsAllow: ["message"],
});
expect(hoisted.bufferedDispatchMock).toHaveBeenCalledTimes(1);
expect(hoisted.bufferedDispatchMock.mock.calls[0]?.[0]).toEqual(
expect.objectContaining({
dispatcherOptions,
toolsAllow: ["message"],
}),
);
});
it("forwards runtime toolsAllow through the plain wrapper", async () => {
const dispatcherOptions = {
deliver: async () => ({ visibleReplySent: false }),
} satisfies ReplyDispatcherOptions;
await dispatchReplyWithDispatcher({
ctx: { Body: "hello" },
cfg: {} as OpenClawConfig,
dispatcherOptions,
toolsAllow: ["message"],
});
expect(hoisted.plainDispatchMock).toHaveBeenCalledTimes(1);
expect(hoisted.plainDispatchMock.mock.calls[0]?.[0]).toEqual(
expect.objectContaining({
dispatcherOptions,
toolsAllow: ["message"],
}),
);
});
});

View File

@@ -20,6 +20,7 @@ export const dispatchReplyWithBufferedBlockDispatcher: DispatchReplyWithBuffered
ctx: params.ctx,
cfg: params.cfg,
dispatcherOptions: params.dispatcherOptions,
toolsAllow: params.toolsAllow,
replyResolver: params.replyResolver,
replyOptions: params.replyOptions,
});
@@ -31,6 +32,7 @@ export const dispatchReplyWithDispatcher: DispatchReplyWithDispatcher = async (p
ctx: params.ctx,
cfg: params.cfg,
dispatcherOptions: params.dispatcherOptions,
toolsAllow: params.toolsAllow,
replyResolver: params.replyResolver,
replyOptions: params.replyOptions,
});

View File

@@ -17,6 +17,7 @@ export type DispatchReplyWithBufferedBlockDispatcher = (params: {
ctx: DispatchReplyContext;
cfg: OpenClawConfig;
dispatcherOptions: ReplyDispatcherWithTypingOptions;
toolsAllow?: string[];
replyOptions?: DispatchReplyOptions;
replyResolver?: GetReplyFromConfig;
}) => Promise<DispatchFromConfigResult>;
@@ -26,6 +27,7 @@ export type DispatchReplyWithDispatcher = (params: {
ctx: DispatchReplyContext;
cfg: OpenClawConfig;
dispatcherOptions: ReplyDispatcherOptions;
toolsAllow?: string[];
replyOptions?: DispatchReplyOptions;
replyResolver?: GetReplyFromConfig;
}) => Promise<DispatchFromConfigResult>;

View File

@@ -425,6 +425,7 @@ export async function dispatchAssembledChannelTurn(
},
onError: params.delivery.onError,
},
toolsAllow: params.toolsAllow,
replyOptions: replyPipeline.replyOptions,
replyResolver: params.replyResolver,
}),

View File

@@ -381,6 +381,7 @@ export type AssembledChannelTurn = {
delivery: ChannelEventDeliveryAdapter;
replyPipeline?: ChannelTurnReplyPipelineOptions;
dispatcherOptions?: ChannelTurnDispatcherOptions;
toolsAllow?: string[];
replyOptions?: Omit<GetReplyOptions, "onBlockReply">;
replyResolver?: GetReplyFromConfig;
record?: ChannelTurnRecordOptions;

View File

@@ -79,6 +79,7 @@ export async function tryDispatchAcpReplyHook(
dispatcher: ctx.dispatcher,
runId: event.runId,
sessionKey: event.sessionKey,
toolsAllow: event.toolsAllow,
images: event.images,
abortSignal: ctx.abortSignal,
inboundAudio: event.inboundAudio,

View File

@@ -198,6 +198,20 @@ describe("tryDispatchAcpReplyHook", () => {
expect(livePredicate?.()).toBe(false);
});
it("passes runtime toolsAllow through to ACP dispatch", async () => {
bypassMock.mockResolvedValue(false);
dispatchMock.mockResolvedValue({
queuedFinal: false,
counts: { tool: 0, block: 0, final: 0 },
});
await tryDispatchAcpReplyHook({ ...event, toolsAllow: ["message"] }, ctx);
expect(dispatchMock).toHaveBeenCalledOnce();
const [payload] = dispatchMock.mock.calls[0] ?? [];
expect((payload as { toolsAllow?: string[] }).toolsAllow).toStrictEqual(["message"]);
});
it("returns unhandled when ACP dispatcher declines the turn", async () => {
bypassMock.mockResolvedValue(false);
dispatchMock.mockResolvedValue(undefined);

View File

@@ -440,6 +440,7 @@ export type PluginHookReplyDispatchEvent = {
ctx: FinalizedMsgContext;
runId?: string;
sessionKey?: string;
toolsAllow?: string[];
images?: Array<{ data: string; mimeType: string }>;
inboundAudio: boolean;
sessionTtsAuto?: TtsAutoMode;