From 5ddea5e32a7ba6fd5c33bcca7c63336df06fbedc Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Mon, 16 Mar 2026 15:25:27 +0800 Subject: [PATCH 1/3] fix(tools): enforce agent tool profile restrictions correctly When an agent has an explicit alsoAllow list, global tool sections (tools.exec, tools.fs) no longer implicitly expose those tools. This fixes a security boundary violation where exec could be used despite being excluded from the agent's profile + alsoAllow. Agent-level tool sections still trigger implicit exposure as expected. Fixes #47487 --- src/agents/pi-tools.policy.test.ts | 72 ++++++++++++++++++++++++++++++ src/agents/pi-tools.policy.ts | 20 +++++++-- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/agents/pi-tools.policy.test.ts b/src/agents/pi-tools.policy.test.ts index 846044c41c0..805ab556d5a 100644 --- a/src/agents/pi-tools.policy.test.ts +++ b/src/agents/pi-tools.policy.test.ts @@ -271,4 +271,76 @@ describe("resolveEffectiveToolPolicy", () => { const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "coder" }); expect(result.profileAlsoAllow).toEqual(["read", "write", "edit"]); }); + + it("does not implicitly expose global exec when agent has explicit alsoAllow without it", () => { + // Regression test for #47487: tool profile restrictions not enforced + const cfg = { + tools: { + exec: { security: "full", ask: "off" }, + }, + agents: { + list: [ + { + id: "messenger", + tools: { + profile: "messaging", + alsoAllow: ["web_search"], + }, + }, + ], + }, + } as OpenClawConfig; + const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "messenger" }); + expect(result.profileAlsoAllow).toEqual(["web_search"]); + expect(result.profileAlsoAllow).not.toContain("exec"); + expect(result.profileAlsoAllow).not.toContain("process"); + }); + + it("does not implicitly expose global fs when agent has explicit empty alsoAllow", () => { + const cfg = { + tools: { + fs: { workspaceOnly: false }, + }, + agents: { + list: [ + { + id: "restricted", + tools: { + profile: "messaging", + alsoAllow: [], + }, + }, + ], + }, + } as OpenClawConfig; + const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "restricted" }); + // Empty array is returned (not undefined) when explicit alsoAllow is set + expect(result.profileAlsoAllow).toEqual([]); + expect(result.profileAlsoAllow).not.toContain("read"); + expect(result.profileAlsoAllow).not.toContain("write"); + expect(result.profileAlsoAllow).not.toContain("edit"); + }); + + it("still uses agent-level exec section for implicit exposure even with explicit alsoAllow", () => { + const cfg = { + tools: { + profile: "messaging", + }, + agents: { + list: [ + { + id: "coder", + tools: { + alsoAllow: ["web_search"], + exec: { host: "sandbox" }, + }, + }, + ], + }, + } as OpenClawConfig; + const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "coder" }); + expect(result.profileAlsoAllow).toContain("web_search"); + expect(result.profileAlsoAllow).toContain("exec"); + expect(result.profileAlsoAllow).toContain("process"); + }); }); diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index 4e7cea7c94e..be91fb9c2db 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -210,18 +210,25 @@ function hasExplicitToolSection(section: unknown): boolean { function resolveImplicitProfileAlsoAllow(params: { globalTools?: OpenClawConfig["tools"]; agentTools?: AgentToolsConfig; + /** When agent has explicit alsoAllow, skip global tool sections for implicit exposure. */ + agentHasExplicitAlsoAllow?: boolean; }): string[] | undefined { const implicit = new Set(); + // When agent has explicit alsoAllow set, only agent-level tool sections should + // trigger implicit exposure. Global tool sections should not override agent's + // explicit restriction. This prevents tools.exec at global level from bypassing + // an agent's profile + alsoAllow restriction. + const useGlobalSections = !params.agentHasExplicitAlsoAllow; if ( hasExplicitToolSection(params.agentTools?.exec) || - hasExplicitToolSection(params.globalTools?.exec) + (useGlobalSections && hasExplicitToolSection(params.globalTools?.exec)) ) { implicit.add("exec"); implicit.add("process"); } if ( hasExplicitToolSection(params.agentTools?.fs) || - hasExplicitToolSection(params.globalTools?.fs) + (useGlobalSections && hasExplicitToolSection(params.globalTools?.fs)) ) { implicit.add("read"); implicit.add("write"); @@ -260,9 +267,14 @@ export function resolveEffectiveToolPolicy(params: { modelProvider: params.modelProvider, modelId: params.modelId, }); + const agentExplicitAlsoAllow = resolveExplicitProfileAlsoAllow(agentTools); const explicitProfileAlsoAllow = - resolveExplicitProfileAlsoAllow(agentTools) ?? resolveExplicitProfileAlsoAllow(globalTools); - const implicitProfileAlsoAllow = resolveImplicitProfileAlsoAllow({ globalTools, agentTools }); + agentExplicitAlsoAllow ?? resolveExplicitProfileAlsoAllow(globalTools); + const implicitProfileAlsoAllow = resolveImplicitProfileAlsoAllow({ + globalTools, + agentTools, + agentHasExplicitAlsoAllow: agentExplicitAlsoAllow !== undefined, + }); const profileAlsoAllow = explicitProfileAlsoAllow || implicitProfileAlsoAllow ? Array.from( From 40e262874aede345bfd6f26bca07951a325b9b0d Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Mon, 16 Mar 2026 15:46:55 +0800 Subject: [PATCH 2/3] fix(cron): honor model override in isolated agentTurn payloads Add regression tests verifying that payload.model is correctly forwarded to runEmbeddedPiAgent for isolated cron runs. Tests cover: - OpenRouter nested provider paths (openrouter/anthropic/claude-haiku-4-5) - Model override with explicit allowlist - Priority over configured defaults and subagents.model Fixes #47592 --- ...lated-agent.payload-model-override.test.ts | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 src/cron/isolated-agent.payload-model-override.test.ts diff --git a/src/cron/isolated-agent.payload-model-override.test.ts b/src/cron/isolated-agent.payload-model-override.test.ts new file mode 100644 index 00000000000..62107b16a3e --- /dev/null +++ b/src/cron/isolated-agent.payload-model-override.test.ts @@ -0,0 +1,210 @@ +import "./isolated-agent.mocks.js"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { loadModelCatalog } from "../agents/model-catalog.js"; +import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; +import { logWarn } from "../logger.js"; +import { createCliDeps, mockAgentPayloads } from "./isolated-agent.delivery.test-helpers.js"; +import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; +import { + makeCfg, + makeJob, + withTempCronHome, + writeSessionStoreEntries, +} from "./isolated-agent.test-harness.js"; +import type { CronJob } from "./types.js"; + +vi.mock("../logger.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + logWarn: vi.fn(), + }; +}); + +const withTempHome = withTempCronHome; + +/** + * Extract the provider and model from the last runEmbeddedPiAgent call. + */ +function lastEmbeddedCall(): { provider?: string; model?: string } { + const calls = vi.mocked(runEmbeddedPiAgent).mock.calls; + expect(calls.length).toBeGreaterThan(0); + return calls.at(-1)?.[0] as { provider?: string; model?: string }; +} + +async function runCronWithModel( + home: string, + payloadModel: string, + cfgOverrides?: Partial[2]>, +) { + const storePath = await writeSessionStoreEntries(home, { + "agent:main:main": { + sessionId: "main-session", + updatedAt: Date.now(), + lastProvider: "webchat", + lastTo: "", + }, + }); + mockAgentPayloads([{ text: "ok" }]); + + const jobPayload: CronJob["payload"] = { + kind: "agentTurn", + message: "test message", + model: payloadModel, + deliver: false, + }; + + const res = await runCronIsolatedAgentTurn({ + cfg: makeCfg(home, storePath, cfgOverrides), + deps: createCliDeps(), + job: makeJob(jobPayload), + message: "test message", + sessionKey: "cron:job-1", + lane: "cron", + }); + + return { res, call: lastEmbeddedCall() }; +} + +describe("runCronIsolatedAgentTurn: payload.model override (#47592)", () => { + beforeEach(() => { + vi.mocked(runEmbeddedPiAgent).mockClear(); + vi.mocked(logWarn).mockClear(); + vi.mocked(loadModelCatalog).mockResolvedValue([]); + }); + + describe("OpenRouter nested provider paths", () => { + it("forwards openrouter/anthropic/claude-haiku-4-5 correctly", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "openrouter/anthropic/claude-haiku-4-5", + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("openrouter"); + expect(call.model).toBe("anthropic/claude-haiku-4-5"); + expect(vi.mocked(logWarn)).not.toHaveBeenCalled(); + }); + }); + + it("forwards openrouter/anthropic/claude-3-5-haiku-latest correctly", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "openrouter/anthropic/claude-3-5-haiku-latest", + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("openrouter"); + expect(call.model).toBe("anthropic/claude-3-5-haiku-latest"); + }); + }); + + it("forwards openrouter/meta-llama/llama-3.3-70b:free correctly", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "openrouter/meta-llama/llama-3.3-70b:free", + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("openrouter"); + expect(call.model).toBe("meta-llama/llama-3.3-70b:free"); + }); + }); + }); + + describe("model override with explicit allowlist", () => { + it("allows payload model when in allowlist", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "openai/gpt-4.1-mini", + { + agents: { + defaults: { + model: "anthropic/claude-opus-4-5", + models: { + "openai/gpt-4.1-mini": {}, + }, + }, + }, + }, + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("openai"); + expect(call.model).toBe("gpt-4.1-mini"); + expect(vi.mocked(logWarn)).not.toHaveBeenCalled(); + }); + }); + + it("logs warning and falls back when payload model is not in allowlist", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "openrouter/anthropic/claude-haiku-4-5", + { + agents: { + defaults: { + model: "anthropic/claude-opus-4-5", + models: { + "anthropic/claude-opus-4-5": {}, + // openrouter model NOT in allowlist + }, + }, + }, + }, + ); + expect(res.status).toBe("ok"); + // Should fall back to default model + expect(call.provider).toBe("anthropic"); + expect(call.model).toBe("claude-opus-4-5"); + // Should have logged a warning + expect(vi.mocked(logWarn)).toHaveBeenCalledWith( + expect.stringContaining("not allowed"), + ); + }); + }); + }); + + describe("model override priority", () => { + it("payload model overrides configured default", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "anthropic/claude-sonnet-4-5", + { + agents: { + defaults: { + model: "anthropic/claude-opus-4-5", // default is opus + }, + }, + }, + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("anthropic"); + expect(call.model).toBe("claude-sonnet-4-5"); // should use sonnet from payload + }); + }); + + it("payload model overrides subagents.model config", async () => { + await withTempHome(async (home) => { + const { res, call } = await runCronWithModel( + home, + "anthropic/claude-opus-4-5", + { + agents: { + defaults: { + model: "anthropic/claude-sonnet-4-5", + subagents: { + model: "openai/gpt-4.1-mini", // subagent override + }, + }, + }, + }, + ); + expect(res.status).toBe("ok"); + expect(call.provider).toBe("anthropic"); + expect(call.model).toBe("claude-opus-4-5"); // payload should win + }); + }); + }); +}); From b7c09cba44708ab67cbc5d5b4b9e7464cdfb848e Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Mon, 16 Mar 2026 16:09:23 +0800 Subject: [PATCH 3/3] fix(cron): improve test helper error reporting for lastEmbeddedCall Replace embedded expect assertion with an explicit throw so failures report at the call-site rather than inside the helper, making it easier to identify which test case triggered the problem. --- src/cron/isolated-agent.payload-model-override.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cron/isolated-agent.payload-model-override.test.ts b/src/cron/isolated-agent.payload-model-override.test.ts index 62107b16a3e..ae264324531 100644 --- a/src/cron/isolated-agent.payload-model-override.test.ts +++ b/src/cron/isolated-agent.payload-model-override.test.ts @@ -28,8 +28,9 @@ const withTempHome = withTempCronHome; */ function lastEmbeddedCall(): { provider?: string; model?: string } { const calls = vi.mocked(runEmbeddedPiAgent).mock.calls; - expect(calls.length).toBeGreaterThan(0); - return calls.at(-1)?.[0] as { provider?: string; model?: string }; + const last = calls.at(-1)?.[0] as { provider?: string; model?: string } | undefined; + if (!last) throw new Error("runEmbeddedPiAgent was never called"); + return last; } async function runCronWithModel(