mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-18 08:37:28 +00:00
fix(exec): inherit ask from exec-approvals.json when tools.exec.ask unset
Landed from contributor PR #29187 by @Bartok9. Co-authored-by: Bartok9 <259807879+Bartok9@users.noreply.github.com>
This commit is contained in:
@@ -313,6 +313,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Discord/exec approvals gateway auth: pass resolved shared gateway credentials into the Discord exec-approvals gateway client so token-auth installs stop failing approvals with `gateway token mismatch`. Related to #38179. Thanks @0riginal-claw for the adjacent PR #35147 investigation.
|
||||
- Subagents/workspace inheritance: propagate parent workspace directory to spawned subagent runs so child sessions reliably inherit workspace-scoped instructions (`AGENTS.md`, `SOUL.md`, etc.) without exposing workspace override through tool-call arguments. (#39247) Thanks @jasonQin6.
|
||||
- Exec approvals/gateway-node policy: honor explicit `ask=off` from `exec-approvals.json` even when runtime defaults are stricter, so trusted full/off setups stop re-prompting on gateway and node exec paths. Landed from contributor PR #26789 by @pandego. Thanks @pandego.
|
||||
- Exec approvals/config fallback: inherit `ask` from `exec-approvals.json` when `tools.exec.ask` is unset, so local full/off defaults no longer fall back to `on-miss` for exec tool and `nodes run`. Landed from contributor PR #29187 by @Bartok9. Thanks @Bartok9.
|
||||
|
||||
## 2026.3.2
|
||||
|
||||
|
||||
@@ -224,6 +224,40 @@ describe("exec approvals", () => {
|
||||
expect(calls).not.toContain("exec.approval.waitDecision");
|
||||
});
|
||||
|
||||
it("inherits ask=off from exec-approvals defaults when tool ask is unset", async () => {
|
||||
const approvalsPath = path.join(process.env.HOME ?? "", ".openclaw", "exec-approvals.json");
|
||||
await fs.mkdir(path.dirname(approvalsPath), { recursive: true });
|
||||
await fs.writeFile(
|
||||
approvalsPath,
|
||||
JSON.stringify(
|
||||
{
|
||||
version: 1,
|
||||
defaults: { security: "full", ask: "off", askFallback: "full" },
|
||||
agents: {},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
const calls: string[] = [];
|
||||
vi.mocked(callGatewayTool).mockImplementation(async (method) => {
|
||||
calls.push(method);
|
||||
return { ok: true };
|
||||
});
|
||||
|
||||
const tool = createExecTool({
|
||||
host: "gateway",
|
||||
security: "full",
|
||||
approvalRunningNoticeMs: 0,
|
||||
});
|
||||
|
||||
const result = await tool.execute("call3c", { command: "echo ok" });
|
||||
expect(result.details.status).toBe("completed");
|
||||
expect(calls).not.toContain("exec.approval.request");
|
||||
expect(calls).not.toContain("exec.approval.waitDecision");
|
||||
});
|
||||
|
||||
it("requires approval for elevated ask when allowlist misses", async () => {
|
||||
const calls: string[] = [];
|
||||
let resolveApproval: (() => void) | undefined;
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import type { AgentTool, AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { type ExecHost, maxAsk, minSecurity } from "../infra/exec-approvals.js";
|
||||
import { type ExecHost, loadExecApprovals, maxAsk, minSecurity } from "../infra/exec-approvals.js";
|
||||
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import {
|
||||
getShellPathFromLoginShell,
|
||||
@@ -324,7 +324,8 @@ export function createExecTool(
|
||||
if (elevatedRequested && elevatedMode === "full") {
|
||||
security = "full";
|
||||
}
|
||||
const configuredAsk = defaults?.ask ?? "on-miss";
|
||||
// Keep local exec defaults in sync with exec-approvals.json when tools.exec.ask is unset.
|
||||
const configuredAsk = defaults?.ask ?? loadExecApprovals().defaults?.ask ?? "on-miss";
|
||||
const requestedAsk = normalizeExecAsk(params.ask);
|
||||
let ask = maxAsk(configuredAsk, requestedAsk ?? configuredAsk);
|
||||
const bypassApprovals = elevatedRequested && elevatedMode === "full";
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Command } from "commander";
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { ExecApprovalsFile } from "../infra/exec-approvals.js";
|
||||
import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js";
|
||||
import { createCliRuntimeCapture } from "./test-runtime-capture.js";
|
||||
|
||||
@@ -15,6 +16,16 @@ type NodeInvokeCall = {
|
||||
|
||||
let lastNodeInvokeCall: NodeInvokeCall | null = null;
|
||||
let lastApprovalRequestCall: { params?: Record<string, unknown> } | null = null;
|
||||
let localExecApprovalsFile: ExecApprovalsFile = { version: 1, agents: {} };
|
||||
let nodeExecApprovalsFile: ExecApprovalsFile = {
|
||||
version: 1,
|
||||
defaults: {
|
||||
security: "allowlist",
|
||||
ask: "on-miss",
|
||||
askFallback: "deny",
|
||||
},
|
||||
agents: {},
|
||||
};
|
||||
|
||||
const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
|
||||
if (opts.method === "node.list") {
|
||||
@@ -58,15 +69,7 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
|
||||
path: "/tmp/exec-approvals.json",
|
||||
exists: true,
|
||||
hash: "hash",
|
||||
file: {
|
||||
version: 1,
|
||||
defaults: {
|
||||
security: "allowlist",
|
||||
ask: "on-miss",
|
||||
askFallback: "deny",
|
||||
},
|
||||
agents: {},
|
||||
},
|
||||
file: nodeExecApprovalsFile,
|
||||
};
|
||||
}
|
||||
if (opts.method === "exec.approval.request") {
|
||||
@@ -93,6 +96,16 @@ vi.mock("../config/config.js", () => ({
|
||||
loadConfig: () => ({}),
|
||||
}));
|
||||
|
||||
vi.mock("../infra/exec-approvals.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("../infra/exec-approvals.js")>(
|
||||
"../infra/exec-approvals.js",
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
loadExecApprovals: () => localExecApprovalsFile,
|
||||
};
|
||||
});
|
||||
|
||||
describe("nodes-cli coverage", () => {
|
||||
let registerNodesCli: (program: Command) => void;
|
||||
let sharedProgram: Command;
|
||||
@@ -125,6 +138,16 @@ describe("nodes-cli coverage", () => {
|
||||
randomIdempotencyKey.mockClear();
|
||||
lastNodeInvokeCall = null;
|
||||
lastApprovalRequestCall = null;
|
||||
localExecApprovalsFile = { version: 1, agents: {} };
|
||||
nodeExecApprovalsFile = {
|
||||
version: 1,
|
||||
defaults: {
|
||||
security: "allowlist",
|
||||
ask: "on-miss",
|
||||
askFallback: "deny",
|
||||
},
|
||||
agents: {},
|
||||
};
|
||||
});
|
||||
|
||||
it("invokes system.run with parsed params", async () => {
|
||||
@@ -207,6 +230,37 @@ describe("nodes-cli coverage", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("inherits ask=off from local exec approvals when tools.exec.ask is unset", async () => {
|
||||
localExecApprovalsFile = {
|
||||
version: 1,
|
||||
defaults: {
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
askFallback: "deny",
|
||||
},
|
||||
agents: {},
|
||||
};
|
||||
nodeExecApprovalsFile = {
|
||||
version: 1,
|
||||
defaults: {
|
||||
security: "allowlist",
|
||||
askFallback: "deny",
|
||||
},
|
||||
agents: {},
|
||||
};
|
||||
|
||||
const invoke = await runNodesCommand(["nodes", "run", "--node", "mac-1", "echo", "hi"]);
|
||||
|
||||
expect(invoke).toBeTruthy();
|
||||
expect(invoke?.params?.command).toBe("system.run");
|
||||
expect(invoke?.params?.params).toMatchObject({
|
||||
command: ["echo", "hi"],
|
||||
approved: false,
|
||||
});
|
||||
expect(invoke?.params?.params).not.toHaveProperty("approvalDecision");
|
||||
expect(getApprovalRequestCall()).toBeNull();
|
||||
});
|
||||
|
||||
it("invokes system.notify with provided fields", async () => {
|
||||
const invoke = await runNodesCommand([
|
||||
"nodes",
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
type ExecApprovalsFile,
|
||||
type ExecAsk,
|
||||
type ExecSecurity,
|
||||
loadExecApprovals,
|
||||
maxAsk,
|
||||
minSecurity,
|
||||
normalizeExecAsk,
|
||||
@@ -96,7 +97,9 @@ function resolveNodesRunPolicy(opts: NodesRunOpts, execDefaults: ExecDefaults |
|
||||
if (opts.security && !requestedSecurity) {
|
||||
throw new Error("invalid --security (use deny|allowlist|full)");
|
||||
}
|
||||
const configuredAsk = normalizeExecAsk(execDefaults?.ask) ?? "on-miss";
|
||||
// Keep local exec defaults in sync with exec-approvals.json when tools.exec.ask is unset.
|
||||
const configuredAsk =
|
||||
normalizeExecAsk(execDefaults?.ask) ?? loadExecApprovals().defaults?.ask ?? "on-miss";
|
||||
const requestedAsk = normalizeExecAsk(opts.ask);
|
||||
if (opts.ask && !requestedAsk) {
|
||||
throw new Error("invalid --ask (use off|on-miss|always)");
|
||||
|
||||
Reference in New Issue
Block a user