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( 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..ae264324531 --- /dev/null +++ b/src/cron/isolated-agent.payload-model-override.test.ts @@ -0,0 +1,211 @@ +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; + 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( + 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 + }); + }); + }); +});