fix(agents): canonicalize provider aliases in byProvider tool policy lookup [AI] (#72917)

* fix: address issue

* fix: address PR review feedback

* fix: address PR review feedback

* fix: address PR review feedback

* fix: address PR review feedback

* docs: add changelog entry for PR merge
This commit is contained in:
Pavan Kumar Gondhi
2026-04-28 18:14:59 +05:30
committed by GitHub
parent ccb3af556f
commit 037f197684
7 changed files with 239 additions and 12 deletions

View File

@@ -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.

View File

@@ -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")],

View File

@@ -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,

View File

@@ -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"] } },

View File

@@ -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: {

View File

@@ -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<string, ToolPolicyConfig> {
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<string, ToolPolicyConfig>();
for (const [key, entry] of lookup) {
resolved.set(key, entry.value);
}
return resolved;
}
function collectUniqueStrings(values: Array<string | null | undefined>): string[] {
@@ -251,19 +294,14 @@ function resolveProviderToolPolicy(params: {
return undefined;
}
const lookup = new Map<string, ToolPolicyConfig>();
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];

View File

@@ -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;