From 349106065a47b1b9241fc9420e2ad4343da703cc Mon Sep 17 00:00:00 2001 From: Alex Knight Date: Sun, 3 May 2026 23:42:32 +1000 Subject: [PATCH] fix(tools): allow no-tool llm-task runs (#76686) Summary: - The branch marks runtime `toolsAllow` sources as enforceable during disabled-tool runs, filters inherited allowlist sources in the guard, adds focused guard coverage, and updates the changelog. - Reproducibility: yes. The linked report gives concrete config and invocation steps, and source inspection on ... sables tools while the guard still treats inherited allowlist sources plus zero callable tools as an error. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 9c3e5f773e66b71eb163b47558f6624a6701c1e3. - Required merge gates passed before the squash merge. Prepared head SHA: 9c3e5f773e66b71eb163b47558f6624a6701c1e3 Review: https://github.com/openclaw/openclaw/pull/76686#issuecomment-4366216196 Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com> --- CHANGELOG.md | 1 + src/agents/pi-embedded-runner/run/attempt.ts | 2 +- src/agents/tool-allowlist-guard.test.ts | 27 +++++++++++++++++--- src/agents/tool-allowlist-guard.ts | 22 +++++++++++++--- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed4b2bc8a6a..6d6d407b2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ Docs: https://docs.openclaw.ai - Active Memory: preserve the target agent context when building embedded recall plugin tools so `memory_search` and `memory_get` stay available for explicit recall sessions. Fixes #76343. Thanks @Countermarch. - Plugins/externalization: repair missing configured plugin installs from npm by default, reserve ClawHub downloads for explicit `clawhubSpec` metadata, and cover agent-runtime/env-selected plugin repair. Thanks @vincentkoc. - Plugins/install: allow official catalog-matched npm channel plugins such as Feishu to pass the trusted install scanner path while keeping spoofed package names blocked. Thanks @vincentkoc. +- Tools/llm-task: keep JSON-only embedded model runs from tripping inherited tool allowlists when tools are intentionally disabled, while preserving runtime `toolsAllow` failures. Fixes #74019. Thanks @amknight. - Tools/profiles: make `tools.profile: "full"` grant all tools including optional plugin tools such as browser, so the full profile no longer silently drops plugin-provided tools that require an explicit allowlist entry. Fixes #76507. Thanks @amknight. - Feishu: keep timeout env parsing separate from the HTTP client wrapper so package security scans no longer report a false env-harvesting hit during install. Thanks @vincentkoc. - Upgrade/config: validate configured web-search providers and statically suppressed model/provider pairs against the active plugin set at config load, so stale plugin state fails loud before runtime fallback. diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 63b15928073..f7a22ab71ed 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -697,7 +697,7 @@ function collectAttemptExplicitToolAllowlistSources(params: { { label: "group tools.allow", allow: groupPolicy?.allow }, { label: "sandbox tools.allow", allow: params.sandboxToolPolicy?.allow }, { label: "subagent tools.allow", allow: subagentPolicy?.allow }, - { label: "runtime toolsAllow", allow: params.toolsAllow }, + { label: "runtime toolsAllow", allow: params.toolsAllow, enforceWhenToolsDisabled: true }, ]); } diff --git a/src/agents/tool-allowlist-guard.test.ts b/src/agents/tool-allowlist-guard.test.ts index 89e5f60e463..4a205d1c456 100644 --- a/src/agents/tool-allowlist-guard.test.ts +++ b/src/agents/tool-allowlist-guard.test.ts @@ -19,7 +19,9 @@ describe("tool allowlist guard", () => { it("fails closed for runtime toolsAllow when tools are disabled", () => { const error = buildEmptyExplicitToolAllowlistError({ - sources: [{ label: "runtime toolsAllow", entries: ["query_db"] }], + sources: [ + { label: "runtime toolsAllow", entries: ["query_db"], enforceWhenToolsDisabled: true }, + ], callableToolNames: [], toolsEnabled: true, disableTools: true, @@ -29,6 +31,17 @@ describe("tool allowlist guard", () => { expect(error?.message).toContain("tools are disabled for this run"); }); + it("allows inherited config allowlists when a run intentionally disables tools", () => { + expect( + buildEmptyExplicitToolAllowlistError({ + sources: [{ label: "tools.allow", entries: ["lobster", "llm-task"] }], + callableToolNames: [], + toolsEnabled: true, + disableTools: true, + }), + ).toBeNull(); + }); + it("fails closed when the selected model cannot use requested tools", () => { const error = buildEmptyExplicitToolAllowlistError({ sources: [{ label: "agents.db.tools.allow", entries: ["query_db"] }], @@ -63,13 +76,21 @@ describe("tool allowlist guard", () => { it("keeps source labels for config and runtime allowlists", () => { const sources = collectExplicitToolAllowlistSources([ { label: "tools.allow", allow: [" read ", ""] }, - { label: "runtime toolsAllow", allow: ["query_db"] }, + { + label: "runtime toolsAllow", + allow: ["query_db"], + enforceWhenToolsDisabled: true, + }, { label: "tools.byProvider.allow" }, ]); expect(sources).toEqual([ { label: "tools.allow", entries: ["read"] }, - { label: "runtime toolsAllow", entries: ["query_db"] }, + { + label: "runtime toolsAllow", + entries: ["query_db"], + enforceWhenToolsDisabled: true, + }, ]); }); }); diff --git a/src/agents/tool-allowlist-guard.ts b/src/agents/tool-allowlist-guard.ts index fb230a4558f..1239dcd3875 100644 --- a/src/agents/tool-allowlist-guard.ts +++ b/src/agents/tool-allowlist-guard.ts @@ -3,14 +3,24 @@ import { normalizeToolName } from "./tool-policy.js"; type ExplicitToolAllowlistSource = { label: string; entries: string[]; + enforceWhenToolsDisabled?: boolean; }; export function collectExplicitToolAllowlistSources( - sources: Array<{ label: string; allow?: string[] }>, + sources: Array<{ label: string; allow?: string[]; enforceWhenToolsDisabled?: boolean }>, ): ExplicitToolAllowlistSource[] { return sources.flatMap((source) => { const entries = (source.allow ?? []).map((entry) => entry.trim()).filter(Boolean); - return entries.length ? [{ label: source.label, entries }] : []; + if (entries.length === 0) { + return []; + } + return [ + { + label: source.label, + entries, + ...(source.enforceWhenToolsDisabled === true ? { enforceWhenToolsDisabled: true } : {}), + }, + ]; }); } @@ -20,11 +30,15 @@ export function buildEmptyExplicitToolAllowlistError(params: { toolsEnabled: boolean; disableTools?: boolean; }): Error | null { + const sources = + params.disableTools === true + ? params.sources.filter((source) => source.enforceWhenToolsDisabled === true) + : params.sources; const callableToolNames = params.callableToolNames.map(normalizeToolName).filter(Boolean); - if (params.sources.length === 0 || callableToolNames.length > 0) { + if (sources.length === 0 || callableToolNames.length > 0) { return null; } - const requested = params.sources + const requested = sources .map((source) => `${source.label}: ${source.entries.map(normalizeToolName).join(", ")}`) .join("; "); const reason =