From 6afdf10266b449aee43a05d38db22cfa67038e91 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Apr 2026 07:24:10 +0100 Subject: [PATCH] fix: honor exec approval security from approvals (#60310) --- CHANGELOG.md | 1 + .../bash-tools.exec-host-shared.test.ts | 73 +++++++++++++++++++ src/agents/bash-tools.exec-host-shared.ts | 9 ++- 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b155339427..34e3708c970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Agents/exec approvals: let `exec-approvals.json` agent security override stricter gateway tool defaults so approved subagents can use `security: "full"` without falling back to allowlist enforcement again. (#60310) Thanks @lml2468. - Providers/OpenAI: preserve native `reasoning.effort: "none"` and strict tool schemas on direct OpenAI-family endpoints, keep compat routes on compat shaping, fix Responses WebSocket warm-up behavior, keep stable session and turn metadata, and fall back more gracefully after early WebSocket failures. - Providers/OpenAI Codex: split native `contextWindow` from runtime `contextTokens`, keep the default effective cap at `272000`, and expose a per-model `contextTokens` override on `models.providers.*.models[]`. - Providers/compat: stop forcing OpenAI-only defaults on proxy and custom OpenAI-compatible routes, preserve native vendor-specific reasoning/tool/streaming behavior across Anthropic-compatible, Moonshot, Mistral, ModelStudio, OpenRouter, xAI, and Z.ai endpoints, and route GitHub Copilot Claude models through Anthropic Messages instead of OpenAI Responses. diff --git a/src/agents/bash-tools.exec-host-shared.test.ts b/src/agents/bash-tools.exec-host-shared.test.ts index 33c1069fb0a..991d12d7454 100644 --- a/src/agents/bash-tools.exec-host-shared.test.ts +++ b/src/agents/bash-tools.exec-host-shared.test.ts @@ -3,6 +3,22 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ sendExecApprovalFollowup: vi.fn(), logWarn: vi.fn(), + resolveExecApprovals: vi.fn(() => ({ + defaults: { + security: "allowlist", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + agent: { + security: "allowlist", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + allowlist: [], + file: { version: 1, agents: {} }, + })), })); vi.mock("./bash-tools.exec-approval-followup.js", () => ({ @@ -13,8 +29,17 @@ vi.mock("../logger.js", () => ({ logWarn: mocks.logWarn, })); +vi.mock("../infra/exec-approvals.js", async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + resolveExecApprovals: mocks.resolveExecApprovals, + }; +}); + let sendExecApprovalFollowupResult: typeof import("./bash-tools.exec-host-shared.js").sendExecApprovalFollowupResult; let maxExecApprovalFollowupFailureLogKeys: typeof import("./bash-tools.exec-host-shared.js").MAX_EXEC_APPROVAL_FOLLOWUP_FAILURE_LOG_KEYS; +let resolveExecHostApprovalContext: typeof import("./bash-tools.exec-host-shared.js").resolveExecHostApprovalContext; let sendExecApprovalFollowup: typeof import("./bash-tools.exec-approval-followup.js").sendExecApprovalFollowup; let logWarn: typeof import("../logger.js").logWarn; @@ -23,6 +48,7 @@ describe("sendExecApprovalFollowupResult", () => { ({ sendExecApprovalFollowupResult, MAX_EXEC_APPROVAL_FOLLOWUP_FAILURE_LOG_KEYS: maxExecApprovalFollowupFailureLogKeys, + resolveExecHostApprovalContext, } = await import("./bash-tools.exec-host-shared.js")); ({ sendExecApprovalFollowup } = await import("./bash-tools.exec-approval-followup.js")); ({ logWarn } = await import("../logger.js")); @@ -31,6 +57,23 @@ describe("sendExecApprovalFollowupResult", () => { beforeEach(() => { vi.mocked(sendExecApprovalFollowup).mockReset(); vi.mocked(logWarn).mockReset(); + mocks.resolveExecApprovals.mockReset(); + mocks.resolveExecApprovals.mockReturnValue({ + defaults: { + security: "allowlist", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + agent: { + security: "allowlist", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + allowlist: [], + file: { version: 1, agents: {} }, + }); }); it("logs repeated followup dispatch failures once per approval id and error message", async () => { @@ -75,3 +118,33 @@ describe("sendExecApprovalFollowupResult", () => { ); }); }); + +describe("resolveExecHostApprovalContext", () => { + it("uses exec-approvals.json agent security even when it is broader than the tool default", () => { + mocks.resolveExecApprovals.mockReturnValue({ + defaults: { + security: "allowlist", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + agent: { + security: "full", + ask: "off", + askFallback: "deny", + autoAllowSkills: false, + }, + allowlist: [], + file: { version: 1, agents: {} }, + }); + + const result = resolveExecHostApprovalContext({ + agentId: "agent-main", + security: "allowlist", + ask: "off", + host: "gateway", + }); + + expect(result.hostSecurity).toBe("full"); + }); +}); diff --git a/src/agents/bash-tools.exec-host-shared.ts b/src/agents/bash-tools.exec-host-shared.ts index e063774cdc0..6db67504f50 100644 --- a/src/agents/bash-tools.exec-host-shared.ts +++ b/src/agents/bash-tools.exec-host-shared.ts @@ -9,7 +9,6 @@ import { } from "../infra/exec-approval-surface.js"; import { maxAsk, - minSecurity, resolveExecApprovalAllowedDecisions, resolveExecApprovals, type ExecAsk, @@ -203,7 +202,13 @@ export function resolveExecHostApprovalContext(params: { security: params.security, ask: params.ask, }); - const hostSecurity = minSecurity(params.security, approvals.agent.security); + // exec-approvals.json is the authoritative security policy and must be able to grant + // a less-restrictive level (e.g. "full") even when tool/runtime defaults are stricter + // (e.g. "allowlist"). This matches node-host behavior and mirrors the ask=off special + // case: exec-approvals.json can suppress prompts AND grant broader execution rights. + // When exec-approvals.json has no explicit agent or defaults entry, approvals.agent.security + // falls back to params.security, so this is backward-compatible. + const hostSecurity = approvals.agent.security; // An explicit ask=off policy in exec-approvals.json must be able to suppress // prompts even when tool/runtime defaults are stricter (for example on-miss). const hostAsk = approvals.agent.ask === "off" ? "off" : maxAsk(params.ask, approvals.agent.ask);