diff --git a/CHANGELOG.md b/CHANGELOG.md index e45644e8058..be0519399c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(agents): canonicalize provider aliases in byProvider tool policy lookup [AI]. (#72917) Thanks @pgondhi987. - fix(security): block npm_execpath injection from workspace .env [AI-assisted]. (#73262) Thanks @pgondhi987. - Tools/web_fetch: decode response bodies from raw bytes using declared HTTP, XML, or HTML meta charsets before extraction, so Shift_JIS and other legacy-charset pages no longer return mojibake. Fixes #72916. Thanks @amknight. - Channels/Discord: bound message read/search REST calls, route those actions through Gateway execution, and fall back to `CommandTargetSessionKey` for inbound hook session keys so Discord reads do not hang and hooks still fire when `SessionKey` is empty. Fixes #73431. (#73521) Thanks @amknight. diff --git a/src/agents/pi-embedded-runner/effective-tool-policy.test.ts b/src/agents/pi-embedded-runner/effective-tool-policy.test.ts index 941614bcd08..ad56b0ffb06 100644 --- a/src/agents/pi-embedded-runner/effective-tool-policy.test.ts +++ b/src/agents/pi-embedded-runner/effective-tool-policy.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { setPluginToolMeta } from "../../plugins/tools.js"; +import { providerAliasCases } from "../test-helpers/provider-alias-cases.js"; import type { AnyAgentTool } from "../tools/common.js"; import { applyFinalEffectiveToolPolicy } from "./effective-tool-policy.js"; @@ -15,6 +16,26 @@ function makeTool(name: string, ownerOnly = false): AnyAgentTool { } describe("applyFinalEffectiveToolPolicy", () => { + it.each(providerAliasCases)( + "applies canonical tools.byProvider deny policy to bundled tools for alias %s", + (alias, canonical) => { + const filtered = applyFinalEffectiveToolPolicy({ + bundledTools: [makeTool("mcp__bundle__exec"), makeTool("mcp__bundle__read")], + config: { + tools: { + byProvider: { + [canonical]: { deny: ["mcp__bundle__exec"] }, + }, + }, + }, + modelProvider: alias, + warn: () => {}, + }); + + expect(filtered.map((tool) => tool.name)).toEqual(["mcp__bundle__read"]); + }, + ); + it("filters bundled tools through the configured allowlist", () => { const filtered = applyFinalEffectiveToolPolicy({ bundledTools: [makeTool("mcp__bundle__fs_delete"), makeTool("mcp__bundle__fs_read")], diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 9576989e1b7..e471bb1a85b 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -730,7 +730,7 @@ export async function runEmbeddedAttempt( }), config: params.config, abortSignal: runAbortController.signal, - modelProvider: params.model.provider, + modelProvider: params.provider, modelId: params.modelId, modelCompat: extractModelCompat(params.model), modelApi: params.model.api, diff --git a/src/agents/pi-tools.create-openclaw-coding-tools.test.ts b/src/agents/pi-tools.create-openclaw-coding-tools.test.ts index 1b4193ce1c3..9c711934fe9 100644 --- a/src/agents/pi-tools.create-openclaw-coding-tools.test.ts +++ b/src/agents/pi-tools.create-openclaw-coding-tools.test.ts @@ -16,6 +16,7 @@ import { createOpenClawCodingTools } from "./pi-tools.js"; import { createHostSandboxFsBridge } from "./test-helpers/host-sandbox-fs-bridge.js"; import { expectReadWriteEditTools } from "./test-helpers/pi-tools-fs-helpers.js"; import { createPiToolsSandboxContext } from "./test-helpers/pi-tools-sandbox-context.js"; +import { providerAliasCases } from "./test-helpers/provider-alias-cases.js"; const tinyPngBuffer = Buffer.from( "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO2f7z8AAAAASUVORK5CYII=", @@ -437,6 +438,26 @@ describe("createOpenClawCodingTools", () => { expect(cronTools.some((tool) => tool.name === "message")).toBe(true); }); + it.each(providerAliasCases)( + "applies canonical tools.byProvider deny policy to core tools for alias %s", + (alias, canonical) => { + const tools = createOpenClawCodingTools({ + config: { + tools: { + byProvider: { + [canonical]: { deny: ["read"] }, + }, + }, + } as OpenClawConfig, + modelProvider: alias, + }); + const names = new Set(tools.map((tool) => tool.name)); + + expect(names.has("read")).toBe(false); + expect(names.has("write")).toBe(true); + }, + ); + it("expands group shorthands in global tool policy", () => { const tools = createOpenClawCodingTools({ config: { tools: { allow: ["group:fs"] } }, diff --git a/src/agents/pi-tools.policy.test.ts b/src/agents/pi-tools.policy.test.ts index 85f3a7913e8..28e83bb5ac8 100644 --- a/src/agents/pi-tools.policy.test.ts +++ b/src/agents/pi-tools.policy.test.ts @@ -11,6 +11,7 @@ import { resolveSubagentToolPolicyForSession, } from "./pi-tools.policy.js"; import { createStubTool } from "./test-helpers/pi-tool-stubs.js"; +import { providerAliasCases } from "./test-helpers/provider-alias-cases.js"; describe("pi-tools.policy", () => { it("treats * in allow as allow-all", () => { @@ -240,6 +241,136 @@ describe("resolveSubagentToolPolicy depth awareness", () => { }); describe("resolveEffectiveToolPolicy", () => { + it.each(providerAliasCases)( + "matches provider alias %s to canonical tools.byProvider key %s", + (alias, canonical) => { + const cfg = { + tools: { + byProvider: { + [canonical]: { deny: ["exec"] }, + }, + }, + } as unknown as OpenClawConfig; + + const result = resolveEffectiveToolPolicy({ config: cfg, modelProvider: alias }); + + expect(result.globalProviderPolicy).toEqual({ deny: ["exec"] }); + }, + ); + + it.each(providerAliasCases)( + "matches provider alias %s to canonical model-scoped tools.byProvider key %s", + (alias, canonical) => { + const cfg = { + tools: { + byProvider: { + [`${canonical}/claude-sonnet`]: { deny: ["exec"] }, + }, + }, + } as unknown as OpenClawConfig; + + const result = resolveEffectiveToolPolicy({ + config: cfg, + modelProvider: alias, + modelId: "claude-sonnet", + }); + + expect(result.globalProviderPolicy).toEqual({ deny: ["exec"] }); + }, + ); + + it("prefers canonical tools.byProvider policy when alias keys collide after normalization", () => { + const aliasFirst = { + tools: { + byProvider: { + bedrock: { deny: ["read"] }, + "amazon-bedrock": { deny: ["exec"] }, + }, + }, + } as unknown as OpenClawConfig; + const canonicalFirst = { + tools: { + byProvider: { + "amazon-bedrock": { deny: ["exec"] }, + bedrock: { deny: ["read"] }, + }, + }, + } as unknown as OpenClawConfig; + + expect( + resolveEffectiveToolPolicy({ config: aliasFirst, modelProvider: "bedrock" }) + .globalProviderPolicy, + ).toEqual({ deny: ["exec"] }); + expect( + resolveEffectiveToolPolicy({ config: canonicalFirst, modelProvider: "bedrock" }) + .globalProviderPolicy, + ).toEqual({ deny: ["exec"] }); + }); + + it("prefers canonical model-scoped tools.byProvider policy when alias keys collide", () => { + const aliasFirst = { + tools: { + byProvider: { + "bedrock/claude-sonnet": { deny: ["read"] }, + "amazon-bedrock/claude-sonnet": { deny: ["exec"] }, + }, + }, + } as unknown as OpenClawConfig; + const canonicalFirst = { + tools: { + byProvider: { + "amazon-bedrock/claude-sonnet": { deny: ["exec"] }, + "bedrock/claude-sonnet": { deny: ["read"] }, + }, + }, + } as unknown as OpenClawConfig; + const params = { modelProvider: "bedrock", modelId: "claude-sonnet" }; + + expect( + resolveEffectiveToolPolicy({ config: aliasFirst, ...params }).globalProviderPolicy, + ).toEqual({ deny: ["exec"] }); + expect( + resolveEffectiveToolPolicy({ config: canonicalFirst, ...params }).globalProviderPolicy, + ).toEqual({ deny: ["exec"] }); + }); + + it("keeps slash-containing modelId scoped to the selected provider", () => { + const cfg = { + tools: { + byProvider: { + "anthropic/claude-sonnet": { deny: ["exec"] }, + "openrouter/anthropic/claude-sonnet": { deny: ["read"] }, + }, + }, + } as unknown as OpenClawConfig; + + expect( + resolveEffectiveToolPolicy({ + config: cfg, + modelProvider: "openrouter", + modelId: "anthropic/claude-sonnet", + }).globalProviderPolicy, + ).toEqual({ deny: ["read"] }); + }); + + it("does not let slash-containing modelId select another provider policy", () => { + const cfg = { + tools: { + byProvider: { + "anthropic/claude-sonnet": { deny: ["exec"] }, + }, + }, + } as unknown as OpenClawConfig; + + expect( + resolveEffectiveToolPolicy({ + config: cfg, + modelProvider: "openrouter", + modelId: "anthropic/claude-sonnet", + }).globalProviderPolicy, + ).toBeUndefined(); + }); + it("implicitly re-exposes exec and process when tools.exec is configured", () => { const cfg = { tools: { diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index fc4a55b9fd5..647e8401628 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -16,6 +16,7 @@ import { import { normalizeMessageChannel } from "../utils/message-channel.js"; import { resolveAgentConfig, resolveAgentIdFromSessionKey } from "./agent-scope.js"; import type { AnyAgentTool } from "./pi-tools.types.js"; +import { normalizeProviderId } from "./provider-id.js"; import { pickSandboxToolPolicy } from "./sandbox-tool-policy.js"; import type { SandboxToolPolicy } from "./sandbox.js"; import { @@ -143,7 +144,49 @@ type ToolPolicyConfig = { }; function normalizeProviderKey(value: string): string { - return normalizeLowercaseStringOrEmpty(value); + const normalized = normalizeLowercaseStringOrEmpty(value); + const slashIndex = normalized.indexOf("/"); + if (slashIndex <= 0) { + return normalizeProviderId(normalized); + } + const provider = normalizeProviderId(normalized.slice(0, slashIndex)); + const modelId = normalized.slice(slashIndex + 1); + return modelId ? `${provider}/${modelId}` : provider; +} + +function isCanonicalProviderKey(value: string): boolean { + return normalizeLowercaseStringOrEmpty(value) === normalizeProviderKey(value); +} + +function buildProviderToolPolicyLookup( + entries: Array<[string, ToolPolicyConfig]>, +): Map { + const lookup = new Map< + string, + { + canonical: boolean; + value: ToolPolicyConfig; + } + >(); + for (const [key, value] of entries) { + const normalized = normalizeProviderKey(key); + if (!normalized) { + continue; + } + const canonical = isCanonicalProviderKey(key); + const existing = lookup.get(normalized); + // Alias and canonical keys can normalize to the same provider. Prefer the + // canonical entry so mixed legacy/canonical configs do not depend on + // Object.entries insertion order. + if (!existing || (canonical && !existing.canonical)) { + lookup.set(normalized, { canonical, value }); + } + } + const resolved = new Map(); + for (const [key, entry] of lookup) { + resolved.set(key, entry.value); + } + return resolved; } function collectUniqueStrings(values: Array): string[] { @@ -251,19 +294,14 @@ function resolveProviderToolPolicy(params: { return undefined; } - const lookup = new Map(); - for (const [key, value] of entries) { - const normalized = normalizeProviderKey(key); - if (!normalized) { - continue; - } - lookup.set(normalized, value); - } + const lookup = buildProviderToolPolicyLookup(entries); const normalizedProvider = normalizeProviderKey(provider); const rawModelId = normalizeOptionalLowercaseString(params.modelId); - const fullModelId = - rawModelId && !rawModelId.includes("/") ? `${normalizedProvider}/${rawModelId}` : rawModelId; + // Model IDs can contain provider-like prefixes (for example OpenRouter refs); + // keep them inside the selected provider scope instead of treating them as a + // byProvider override. + const fullModelId = rawModelId ? `${normalizedProvider}/${rawModelId}` : undefined; const candidates = [...(fullModelId ? [fullModelId] : []), normalizedProvider]; diff --git a/src/agents/test-helpers/provider-alias-cases.ts b/src/agents/test-helpers/provider-alias-cases.ts new file mode 100644 index 00000000000..1562e868b7d --- /dev/null +++ b/src/agents/test-helpers/provider-alias-cases.ts @@ -0,0 +1,15 @@ +export const providerAliasCases = [ + ["bedrock", "amazon-bedrock"], + ["aws-bedrock", "amazon-bedrock"], + ["modelstudio", "qwen"], + ["qwencloud", "qwen"], + ["z.ai", "zai"], + ["z-ai", "zai"], + ["kimi", "kimi"], + ["kimi-code", "kimi"], + ["kimi-coding", "kimi"], + ["bytedance", "volcengine"], + ["doubao", "volcengine"], + ["opencode-zen", "opencode"], + ["opencode-go-auth", "opencode-go"], +] as const;