From 235ad7ec956513c632c20eedd68bfbedc8cc3ea8 Mon Sep 17 00:00:00 2001 From: Bryan Pearson Date: Sat, 2 May 2026 21:17:53 -0700 Subject: [PATCH] fix(exec): keep configured security authoritative --- CHANGELOG.md | 1 + .../bash-tools.exec.security-floor.test.ts | 83 +++++++++++++++++++ src/agents/bash-tools.exec.ts | 5 +- 3 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 src/agents/bash-tools.exec.security-floor.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4849018638e..472d1e75e14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1296,6 +1296,7 @@ Docs: https://docs.openclaw.ai - Plugins/npm: build package-local runtime dist files for publishable plugins and stop listing root-package-excluded plugin sidecars in the core package metadata, so npm plugin installs such as `@openclaw/diffs` and `@openclaw/discord` no longer publish source-only runtime payloads. Fixes #76426. Thanks @PrinceOfEgypt. - Channels/secrets: resolve SecretRef-backed channel credentials through external plugin secret contracts after the plugin split, covering runtime startup, target discovery, webhook auth, disabled-account enumeration, and late-bound web_search config. Fixes #76371. (#76449) Thanks @joshavant and @neeravmakwana. - Docker/Gateway: pass Docker setup `.env` values into gateway and CLI containers and preserve exec SecretRef `passEnv` keys in managed service plans, so 1Password Connect-backed Discord tokens keep resolving after doctor or plugin repair. Thanks @vincentkoc. +- Exec/security: treat configured `tools.exec.security` as authoritative for normal tool calls so model-supplied `security` arguments cannot downgrade or tighten the operator policy, while preserving explicitly granted elevated-full overrides. (#65933) Thanks @bryanpearson. - Control UI/WebChat: explain compaction boundaries in chat history and link directly to session checkpoint controls so pre-compaction turns no longer look silently lost after refresh. Fixes #76415. Thanks @BunsDev. - Agents/compaction: add an optional bundled compaction notifier hook and retry once from the compacted transcript when automatic compaction leaves a turn without a final visible reply. (#76651) Thanks @simplyclever914. - Agents/incomplete-turn: detect and surface a warning when the agent's final text after a tool-call chain is silently dropped because the post-tool assistant response was never produced, instead of completing the turn with only the pre-tool analysis text. Fixes #76477. Thanks @amknight. diff --git a/src/agents/bash-tools.exec.security-floor.test.ts b/src/agents/bash-tools.exec.security-floor.test.ts new file mode 100644 index 00000000000..72a7b7e1982 --- /dev/null +++ b/src/agents/bash-tools.exec.security-floor.test.ts @@ -0,0 +1,83 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; +import { resetProcessRegistryForTests } from "./bash-process-registry.js"; +import { createExecTool } from "./bash-tools.exec.js"; + +describe("exec security floor", () => { + let envSnapshot: ReturnType; + + beforeEach(() => { + envSnapshot = captureEnv(["SHELL"]); + resetProcessRegistryForTests(); + }); + + afterEach(() => { + envSnapshot.restore(); + }); + + it("ignores model-supplied allowlist security when configured security is full", async () => { + const tool = createExecTool({ + security: "full", + ask: "off", + }); + + const result = await tool.execute("call-1", { + command: "echo hello", + security: "allowlist", + ask: "off", + }); + + expect(result.content[0]).toMatchObject({ type: "text" }); + const text = (result.content[0] as { text?: string }).text ?? ""; + expect(text).not.toMatch(/exec denied/i); + expect(text).not.toMatch(/allowlist miss/i); + expect(text.trim()).toContain("hello"); + }); + + it("enforces configured allowlist security when model also passes allowlist", async () => { + const tool = createExecTool({ + security: "allowlist", + ask: "off", + safeBins: [], + }); + + await expect( + tool.execute("call-2", { + command: "echo hello", + security: "allowlist", + ask: "off", + }), + ).rejects.toThrow(/exec denied: allowlist miss/i); + }); + + it("ignores model-supplied deny security when configured security is allowlist", async () => { + const tool = createExecTool({ + security: "allowlist", + ask: "off", + safeBins: [], + }); + + await expect( + tool.execute("call-3", { + command: "echo hello", + security: "deny", + ask: "off", + }), + ).rejects.toThrow(/exec denied: allowlist miss/i); + }); + + it("ignores model-supplied full security when configured security is deny", async () => { + const tool = createExecTool({ + security: "deny", + ask: "off", + }); + + await expect( + tool.execute("call-4", { + command: "echo hello", + security: "full", + ask: "off", + }), + ).rejects.toThrow(/exec denied/i); + }); +}); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 5beb58f5573..c80a702b5a8 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -8,7 +8,6 @@ import { type ExecSecurity, loadExecApprovals, maxAsk, - minSecurity, requireValidExecTarget, } from "../infra/exec-approvals.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; @@ -40,7 +39,6 @@ import { applyPathPrepend, applyShellPath, normalizeExecAsk, - normalizeExecSecurity, normalizePathPrepend, resolveExecTarget, resolveApprovalRunningNoticeMs, @@ -1346,8 +1344,7 @@ export function createExecTool( const approvalDefaults = loadExecApprovals().defaults; const configuredSecurity = defaults?.security ?? approvalDefaults?.security ?? (host === "sandbox" ? "deny" : "full"); - const requestedSecurity = normalizeExecSecurity(params.security); - let security = minSecurity(configuredSecurity, requestedSecurity ?? configuredSecurity); + let security = configuredSecurity; if (elevatedRequested && elevatedMode === "full") { security = "full"; }