Merge b7c09cba44708ab67cbc5d5b4b9e7464cdfb848e into 598f1826d8b2bc969aace2c6459824737667218c

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");
+  });
 });

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<string>();
+  // 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(

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<typeof import("../logger.js")>();
+  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<Parameters<typeof makeCfg>[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
+      });
+    });
+  });
+});