let5see/clawdbot
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(exec): align node exec approvals (#1425)

Browse Source 
Thanks @czekaj.

Co-authored-by: Lucas Czekaj <lukasz@czekaj.us>
This commit is contained in:
Lucas Czekaj
2026-01-21 22:02:17 -08:00
committed by Peter Steinberger
parent d83ea7f2da
commit 4b3e9c0f33
6 changed files with 395 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -34,6 +34,7 @@ Docs: https://docs.clawd.bot
- Cron: cap reminder context history to 10 messages and honor `contextMessages`. (#1103) Thanks @mkbehr.
- Exec approvals: treat main as the default agent + migrate legacy default allowlists. (#1417) Thanks @czekaj.
- Exec: avoid defaulting to elevated mode when elevated is not allowed.
- Exec approvals: align node/gateway allowlist prechecks and approval gating; avoid null optional params in approval requests. (#1425) Thanks @czekaj.
- UI: refresh debug panel on route-driven tab changes. (#1373) Thanks @yazinsai.
## 2026.1.21

66
src/agents/bash-tools.exec.approval-id.test.ts
View File

@@ -68,4 +68,70 @@ describe("exec approvals", () => {
const runId = (invokeParams as { params?: { runId?: string } } | undefined)?.params?.runId;
expect(runId).toBe(approvalId);
});
it("skips approval when node allowlist is satisfied", async () => {
const { callGatewayTool } = await import("./tools/gateway.js");
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-test-bin-"));
const binDir = path.join(tempDir, "bin");
await fs.mkdir(binDir, { recursive: true });
const exeName = process.platform === "win32" ? "tool.cmd" : "tool";
const exePath = path.join(binDir, exeName);
await fs.writeFile(exePath, "");
if (process.platform !== "win32") {
await fs.chmod(exePath, 0o755);
}
const prevPath = process.env.PATH;
const prevPathExt = process.env.PATHEXT;
process.env.PATH = binDir;
if (process.platform === "win32") {
process.env.PATHEXT = ".CMD";
}
try {
const approvalsFile = {
version: 1,
defaults: { security: "allowlist", ask: "on-miss", askFallback: "deny" },
agents: {
main: {
allowlist: [{ pattern: exePath }],
},
},
};
const calls: string[] = [];
vi.mocked(callGatewayTool).mockImplementation(async (method) => {
calls.push(method);
if (method === "exec.approvals.node.get") {
return { file: approvalsFile };
}
if (method === "node.invoke") {
return { payload: { success: true, stdout: "ok" } };
}
if (method === "exec.approval.request") {
return { decision: "allow-once" };
}
return { ok: true };
});
const { createExecTool } = await import("./bash-tools.exec.js");
const tool = createExecTool({
host: "node",
ask: "on-miss",
approvalRunningNoticeMs: 0,
});
const result = await tool.execute("call2", { command: `${exeName} --help` });
expect(result.details.status).toBe("completed");
expect(calls).toContain("exec.approvals.node.get");
expect(calls).toContain("node.invoke");
expect(calls).not.toContain("exec.approval.request");
} finally {
process.env.PATH = prevPath;
if (prevPathExt === undefined) {
delete process.env.PATHEXT;
} else {
process.env.PATHEXT = prevPathExt;
}
}
});
});

88
src/agents/bash-tools.exec.ts
View File

@@ -8,16 +8,17 @@ import {
type ExecAsk,
type ExecHost,
type ExecSecurity,
type ExecAllowlistEntry,
type ExecApprovalsFile,
addAllowlistEntry,
analyzeShellCommand,
isSafeBinUsage,
matchAllowlist,
evaluateExecAllowlist,
maxAsk,
minSecurity,
requiresExecApproval,
resolveSafeBins,
recordAllowlistUse,
resolveExecApprovals,
resolveExecApprovalsFromFile,
} from "../infra/exec-approvals.js";
import { requestHeartbeatNow } from "../infra/heartbeat-wake.js";
import { buildNodeShellCommand } from "../infra/node-shell.js";
@@ -870,7 +871,43 @@ export function createExecTool(
if (nodeEnv) {
applyPathPrepend(nodeEnv, defaultPathPrepend, { requireExisting: true });
}
const requiresAsk = hostAsk === "always" || hostAsk === "on-miss";
const analysis = analyzeShellCommand({ command: params.command, cwd: workdir, env });
let allowlistSatisfied = false;
if (hostAsk === "on-miss" && hostSecurity === "allowlist") {
try {
const approvalsSnapshot = (await callGatewayTool(
"exec.approvals.node.get",
{ timeoutMs: 10_000 },
{ nodeId },
)) as { file?: unknown } | null;
const approvalsFile =
approvalsSnapshot && typeof approvalsSnapshot === "object"
? approvalsSnapshot.file
: undefined;
if (approvalsFile && typeof approvalsFile === "object") {
const resolved = resolveExecApprovalsFromFile({
file: approvalsFile as ExecApprovalsFile,
agentId,
overrides: { security: "allowlist" },
});
// Allowlist-only precheck; safe bins are node-local and may diverge.
allowlistSatisfied = evaluateExecAllowlist({
analysis,
allowlist: resolved.allowlist,
safeBins: new Set(),
cwd: workdir,
}).allowlistSatisfied;
}
} catch {
// Fall back to requiring approval if node approvals cannot be fetched.
}
}
const requiresAsk = requiresExecApproval({
ask: hostAsk,
security: hostSecurity,
analysisOk: analysis.ok,
allowlistSatisfied,
});
const commandText = params.command;
const invokeTimeoutMs = Math.max(
10_000,
@@ -921,8 +958,8 @@ export function createExecTool(
security: hostSecurity,
ask: hostAsk,
agentId,
resolvedPath: null,
sessionKey: defaults?.sessionKey ?? null,
resolvedPath: undefined,
sessionKey: defaults?.sessionKey,
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
},
)) as { decision?: string } | null;
@@ -1059,33 +1096,28 @@ export function createExecTool(
throw new Error("exec denied: host=gateway security=deny");
}
const analysis = analyzeShellCommand({ command: params.command, cwd: workdir, env });
const allowlistMatches: ExecAllowlistEntry[] = [];
let allowlistSatisfied = false;
if (hostSecurity === "allowlist" && analysis.ok && analysis.segments.length > 0) {
allowlistSatisfied = analysis.segments.every((segment) => {
const match = matchAllowlist(approvals.allowlist, segment.resolution);
if (match) allowlistMatches.push(match);
const safe = isSafeBinUsage({
argv: segment.argv,
resolution: segment.resolution,
safeBins,
cwd: workdir,
});
return Boolean(match || safe);
});
}
const requiresAsk =
hostAsk === "always" ||
(hostAsk === "on-miss" &&
hostSecurity === "allowlist" &&
(!analysis.ok || !allowlistSatisfied));
const allowlistEval = evaluateExecAllowlist({
analysis,
allowlist: approvals.allowlist,
safeBins,
cwd: workdir,
});
const allowlistMatches = allowlistEval.allowlistMatches;
const allowlistSatisfied =
hostSecurity === "allowlist" && analysis.ok ? allowlistEval.allowlistSatisfied : false;
const requiresAsk = requiresExecApproval({
ask: hostAsk,
security: hostSecurity,
analysisOk: analysis.ok,
allowlistSatisfied,
});
if (requiresAsk) {
const approvalId = crypto.randomUUID();
const approvalSlug = createApprovalSlug(approvalId);
const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
const contextKey = `exec:${approvalId}`;
const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath ?? null;
const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath;
const noticeSeconds = Math.max(1, Math.round(approvalRunningNoticeMs / 1000));
const commandText = params.command;
const effectiveTimeout =
@@ -1107,7 +1139,7 @@ export function createExecTool(
ask: hostAsk,
agentId,
resolvedPath,
sessionKey: defaults?.sessionKey ?? null,
sessionKey: defaults?.sessionKey,
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
},
)) as { decision?: string } | null;

200
src/infra/exec-approvals.test.ts
View File

@@ -7,11 +7,13 @@ import { describe, expect, it, vi } from "vitest";
import {
analyzeArgvCommand,
analyzeShellCommand,
evaluateExecAllowlist,
isSafeBinUsage,
matchAllowlist,
maxAsk,
minSecurity,
normalizeSafeBins,
requiresExecApproval,
resolveCommandResolution,
resolveExecApprovals,
resolveExecApprovalsFromFile,
@@ -183,6 +185,85 @@ describe("exec approvals safe bins", () => {
});
});
describe("exec approvals allowlist evaluation", () => {
it("satisfies allowlist on exact match", () => {
const analysis = {
ok: true,
segments: [
{
raw: "tool",
argv: ["tool"],
resolution: {
rawExecutable: "tool",
resolvedPath: "/usr/bin/tool",
executableName: "tool",
},
},
],
};
const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/tool" }];
const result = evaluateExecAllowlist({
analysis,
allowlist,
safeBins: new Set(),
cwd: "/tmp",
});
expect(result.allowlistSatisfied).toBe(true);
expect(result.allowlistMatches.map((entry) => entry.pattern)).toEqual(["/usr/bin/tool"]);
});
it("satisfies allowlist via safe bins", () => {
const analysis = {
ok: true,
segments: [
{
raw: "jq .foo",
argv: ["jq", ".foo"],
resolution: {
rawExecutable: "jq",
resolvedPath: "/usr/bin/jq",
executableName: "jq",
},
},
],
};
const result = evaluateExecAllowlist({
analysis,
allowlist: [],
safeBins: normalizeSafeBins(["jq"]),
cwd: "/tmp",
});
expect(result.allowlistSatisfied).toBe(true);
expect(result.allowlistMatches).toEqual([]);
});
it("satisfies allowlist via auto-allow skills", () => {
const analysis = {
ok: true,
segments: [
{
raw: "skill-bin",
argv: ["skill-bin", "--help"],
resolution: {
rawExecutable: "skill-bin",
resolvedPath: "/opt/skills/skill-bin",
executableName: "skill-bin",
},
},
],
};
const result = evaluateExecAllowlist({
analysis,
allowlist: [],
safeBins: new Set(),
skillBins: new Set(["skill-bin"]),
autoAllowSkills: true,
cwd: "/tmp",
});
expect(result.allowlistSatisfied).toBe(true);
});
});
describe("exec approvals policy helpers", () => {
it("minSecurity returns the more restrictive value", () => {
expect(minSecurity("deny", "full")).toBe("deny");
@@ -193,6 +274,49 @@ describe("exec approvals policy helpers", () => {
expect(maxAsk("off", "always")).toBe("always");
expect(maxAsk("on-miss", "off")).toBe("on-miss");
});
it("requiresExecApproval respects ask mode and allowlist satisfaction", () => {
expect(
requiresExecApproval({
ask: "always",
security: "allowlist",
analysisOk: true,
allowlistSatisfied: true,
}),
).toBe(true);
expect(
requiresExecApproval({
ask: "off",
security: "allowlist",
analysisOk: true,
allowlistSatisfied: false,
}),
).toBe(false);
expect(
requiresExecApproval({
ask: "on-miss",
security: "allowlist",
analysisOk: true,
allowlistSatisfied: true,
}),
).toBe(false);
expect(
requiresExecApproval({
ask: "on-miss",
security: "allowlist",
analysisOk: false,
allowlistSatisfied: false,
}),
).toBe(true);
expect(
requiresExecApproval({
ask: "on-miss",
security: "full",
analysisOk: false,
allowlistSatisfied: false,
}),
).toBe(false);
});
});
describe("exec approvals wildcard agent", () => {
@@ -229,6 +353,82 @@ describe("exec approvals wildcard agent", () => {
});
});
describe("exec approvals node host allowlist check", () => {
// These tests verify the allowlist satisfaction logic used by the node host path
// The node host checks: matchAllowlist() || isSafeBinUsage() for each command segment
// Using hardcoded resolution objects for cross-platform compatibility
it("satisfies allowlist when command matches exact path pattern", () => {
const resolution = {
rawExecutable: "python3",
resolvedPath: "/usr/bin/python3",
executableName: "python3",
};
const entries: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/python3" }];
const match = matchAllowlist(entries, resolution);
expect(match).not.toBeNull();
expect(match?.pattern).toBe("/usr/bin/python3");
});
it("satisfies allowlist when command matches ** wildcard pattern", () => {
// Simulates symlink resolution: /opt/homebrew/bin/python3 -> /opt/homebrew/opt/python@3.14/bin/python3.14
const resolution = {
rawExecutable: "python3",
resolvedPath: "/opt/homebrew/opt/python@3.14/bin/python3.14",
executableName: "python3.14",
};
// Pattern with ** matches across multiple directories
const entries: ExecAllowlistEntry[] = [{ pattern: "/opt/**/python*" }];
const match = matchAllowlist(entries, resolution);
expect(match?.pattern).toBe("/opt/**/python*");
});
it("does not satisfy allowlist when command is not in allowlist", () => {
const resolution = {
rawExecutable: "unknown-tool",
resolvedPath: "/usr/local/bin/unknown-tool",
executableName: "unknown-tool",
};
// Allowlist has different commands
const entries: ExecAllowlistEntry[] = [
{ pattern: "/usr/bin/python3" },
{ pattern: "/opt/**/node" },
];
const match = matchAllowlist(entries, resolution);
expect(match).toBeNull();
// Also not a safe bin
const safe = isSafeBinUsage({
argv: ["unknown-tool", "--help"],
resolution,
safeBins: normalizeSafeBins(["jq", "curl"]),
cwd: "/tmp",
});
expect(safe).toBe(false);
});
it("satisfies via safeBins even when not in allowlist", () => {
const resolution = {
rawExecutable: "jq",
resolvedPath: "/usr/bin/jq",
executableName: "jq",
};
// Not in allowlist
const entries: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/python3" }];
const match = matchAllowlist(entries, resolution);
expect(match).toBeNull();
// But is a safe bin with non-file args
const safe = isSafeBinUsage({
argv: ["jq", ".foo"],
resolution,
safeBins: normalizeSafeBins(["jq"]),
cwd: "/tmp",
});
expect(safe).toBe(true);
});
});
describe("exec approvals default agent migration", () => {
it("migrates legacy default agent entries to main", () => {
const file = {

50
src/infra/exec-approvals.ts
View File

@@ -727,6 +727,56 @@ export function isSafeBinUsage(params: {
return true;
}
export type ExecAllowlistEvaluation = {
allowlistSatisfied: boolean;
allowlistMatches: ExecAllowlistEntry[];
};
export function evaluateExecAllowlist(params: {
analysis: ExecCommandAnalysis;
allowlist: ExecAllowlistEntry[];
safeBins: Set<string>;
cwd?: string;
skillBins?: Set<string>;
autoAllowSkills?: boolean;
}): ExecAllowlistEvaluation {
const allowlistMatches: ExecAllowlistEntry[] = [];
if (!params.analysis.ok || params.analysis.segments.length === 0) {
return { allowlistSatisfied: false, allowlistMatches };
}
const allowSkills = params.autoAllowSkills === true && (params.skillBins?.size ?? 0) > 0;
const allowlistSatisfied = params.analysis.segments.every((segment) => {
const match = matchAllowlist(params.allowlist, segment.resolution);
if (match) allowlistMatches.push(match);
const safe = isSafeBinUsage({
argv: segment.argv,
resolution: segment.resolution,
safeBins: params.safeBins,
cwd: params.cwd,
});
const skillAllow =
allowSkills && segment.resolution?.executableName
? params.skillBins?.has(segment.resolution.executableName)
: false;
return Boolean(match || safe || skillAllow);
});
return { allowlistSatisfied, allowlistMatches };
}
export function requiresExecApproval(params: {
ask: ExecAsk;
security: ExecSecurity;
analysisOk: boolean;
allowlistSatisfied: boolean;
}): boolean {
return (
params.ask === "always" ||
(params.ask === "on-miss" &&
params.security === "allowlist" &&
(!params.analysisOk || !params.allowlistSatisfied))
);
}
export function recordAllowlistUse(
approvals: ExecApprovalsFile,
agentId: string | undefined,

42
src/node-host/runner.ts
View File

@@ -4,12 +4,11 @@ import fs from "node:fs";
import path from "node:path";
import {
type ExecAllowlistEntry,
addAllowlistEntry,
analyzeArgvCommand,
analyzeShellCommand,
isSafeBinUsage,
matchAllowlist,
evaluateExecAllowlist,
requiresExecApproval,
normalizeExecApprovals,
recordAllowlistUse,
resolveExecApprovals,
@@ -592,26 +591,18 @@ async function handleInvoke(
const cfg = loadConfig();
const agentExec = agentId ? resolveAgentConfig(cfg, agentId)?.tools?.exec : undefined;
const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins);
const allowlistMatches: ExecAllowlistEntry[] = [];
const bins = autoAllowSkills ? await skillBins.current() : new Set<string>();
const allowlistEval = evaluateExecAllowlist({
analysis,
allowlist: approvals.allowlist,
safeBins,
cwd: params.cwd ?? undefined,
skillBins: bins,
autoAllowSkills,
});
const allowlistMatches = allowlistEval.allowlistMatches;
const allowlistSatisfied =
security === "allowlist" && analysis.ok && analysis.segments.length > 0
? analysis.segments.every((segment) => {
const match = matchAllowlist(approvals.allowlist, segment.resolution);
if (match) allowlistMatches.push(match);
const safe = isSafeBinUsage({
argv: segment.argv,
resolution: segment.resolution,
safeBins,
cwd: params.cwd ?? undefined,
});
const skillAllow =
autoAllowSkills && segment.resolution?.executableName
? bins.has(segment.resolution.executableName)
: false;
return Boolean(match || safe || skillAllow);
})
: false;
security === "allowlist" && analysis.ok ? allowlistEval.allowlistSatisfied : false;
const useMacAppExec = process.platform === "darwin";
if (useMacAppExec) {
@@ -715,9 +706,12 @@ async function handleInvoke(
return;
}
const requiresAsk =
ask === "always" ||
(ask === "on-miss" && security === "allowlist" && (!analysis.ok || !allowlistSatisfied));
const requiresAsk = requiresExecApproval({
ask,
security,
analysisOk: analysis.ok,
allowlistSatisfied,
});
const approvalDecision =
params.approvalDecision === "allow-once" || params.approvalDecision === "allow-always"