mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(security): harden node exec approvals against symlink rebind
This commit is contained in:
@@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/Workspace FS boundary aliases: harden canonical boundary resolution for non-existent-leaf symlink aliases while preserving valid in-root aliases, preventing first-write workspace escapes via out-of-root symlink targets. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
|
||||
- Security/Config includes: harden `$include` file loading with verified-open reads, reject hardlinked include aliases, and enforce include file-size guardrails so config include resolution remains bounded to trusted in-root files. This ships in the next npm release (`2026.2.26`). Thanks @zpbrent for reporting.
|
||||
- Security/Node exec approvals: require structured `commandArgv` approvals for `host=node`, enforce versioned `systemRunBindingV1` matching for argv/cwd/session/agent/env context with fail-closed behavior on missing/mismatched bindings, and add `GIT_EXTERNAL_DIFF` to blocked host env keys. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
|
||||
- Security/Node exec approvals hardening: freeze immutable approval-time execution plans (`argv`/`cwd`/`agentId`/`sessionKey`) via `system.run.prepare`, enforce those canonical plan values during approval forwarding/execution, and reject mutable parent-symlink cwd paths during approval-plan building to prevent approval bypass via symlink rebind. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
|
||||
- Security/Microsoft Teams media fetch: route Graph message/hosted-content/attachment fetches and auth-scope fallback attachment downloads through shared SSRF-guarded fetch paths, and centralize hostname-suffix allowlist policy helpers in the plugin SDK to remove channel/plugin drift. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
|
||||
- Security/Voice Call (Twilio): bind webhook replay + manager dedupe identity to authenticated request material, remove unsigned `i-twilio-idempotency-token` trust from replay/dedupe keys, and thread verified request identity through provider parse flow to harden cross-provider event dedupe. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
|
||||
- Microsoft Teams/File uploads: acknowledge `fileConsent/invoke` immediately (`invokeResponse` before upload + file card send) so Teams no longer shows false "Something went wrong" timeout banners while upload completion continues asynchronously; includes updated async regression coverage. Landed from contributor PR #27641 by @scz2011.
|
||||
|
||||
@@ -28,6 +28,35 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
|
||||
};
|
||||
}
|
||||
if (opts.method === "node.invoke") {
|
||||
const command = opts.params?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
const params = (opts.params?.params ?? {}) as {
|
||||
command?: unknown[];
|
||||
rawCommand?: unknown;
|
||||
cwd?: unknown;
|
||||
agentId?: unknown;
|
||||
};
|
||||
const argv = Array.isArray(params.command)
|
||||
? params.command.map((entry) => String(entry))
|
||||
: [];
|
||||
const rawCommand =
|
||||
typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0
|
||||
? params.rawCommand
|
||||
: null;
|
||||
return {
|
||||
payload: {
|
||||
cmdText: rawCommand ?? argv.join(" "),
|
||||
plan: {
|
||||
version: 2,
|
||||
argv,
|
||||
cwd: typeof params.cwd === "string" ? params.cwd : null,
|
||||
rawCommand,
|
||||
agentId: typeof params.agentId === "string" ? params.agentId : null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
return {
|
||||
payload: {
|
||||
stdout: "",
|
||||
@@ -80,8 +109,16 @@ vi.mock("../config/config.js", () => ({
|
||||
describe("nodes-cli coverage", () => {
|
||||
let registerNodesCli: (program: Command) => void;
|
||||
|
||||
const getNodeInvokeCall = () =>
|
||||
callGateway.mock.calls.find((call) => call[0]?.method === "node.invoke")?.[0] as NodeInvokeCall;
|
||||
const getNodeInvokeCall = () => {
|
||||
const nodeInvokeCalls = callGateway.mock.calls
|
||||
.map((call) => call[0])
|
||||
.filter((entry): entry is NodeInvokeCall => entry?.method === "node.invoke");
|
||||
const last = nodeInvokeCalls.at(-1);
|
||||
if (!last) {
|
||||
throw new Error("expected node.invoke call");
|
||||
}
|
||||
return last;
|
||||
};
|
||||
|
||||
const getApprovalRequestCall = () =>
|
||||
callGateway.mock.calls.find((call) => call[0]?.method === "exec.approval.request")?.[0] as {
|
||||
@@ -135,6 +172,7 @@ describe("nodes-cli coverage", () => {
|
||||
expect(invoke?.params?.command).toBe("system.run");
|
||||
expect(invoke?.params?.params).toEqual({
|
||||
command: ["echo", "hi"],
|
||||
rawCommand: null,
|
||||
cwd: "/tmp",
|
||||
env: { FOO: "bar" },
|
||||
timeoutMs: 1200,
|
||||
@@ -147,6 +185,14 @@ describe("nodes-cli coverage", () => {
|
||||
expect(invoke?.params?.timeoutMs).toBe(5000);
|
||||
const approval = getApprovalRequestCall();
|
||||
expect(approval?.params?.["commandArgv"]).toEqual(["echo", "hi"]);
|
||||
expect(approval?.params?.["systemRunPlanV2"]).toEqual({
|
||||
version: 2,
|
||||
argv: ["echo", "hi"],
|
||||
cwd: "/tmp",
|
||||
rawCommand: null,
|
||||
agentId: "main",
|
||||
sessionKey: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("invokes system.run with raw command", async () => {
|
||||
@@ -174,6 +220,14 @@ describe("nodes-cli coverage", () => {
|
||||
});
|
||||
const approval = getApprovalRequestCall();
|
||||
expect(approval?.params?.["commandArgv"]).toEqual(["/bin/sh", "-lc", "echo hi"]);
|
||||
expect(approval?.params?.["systemRunPlanV2"]).toEqual({
|
||||
version: 2,
|
||||
argv: ["/bin/sh", "-lc", "echo hi"],
|
||||
cwd: null,
|
||||
rawCommand: "echo hi",
|
||||
agentId: "main",
|
||||
sessionKey: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("invokes system.notify with provided fields", async () => {
|
||||
|
||||
@@ -7,12 +7,14 @@ import {
|
||||
type ExecApprovalsFile,
|
||||
type ExecAsk,
|
||||
type ExecSecurity,
|
||||
type SystemRunApprovalPlanV2,
|
||||
maxAsk,
|
||||
minSecurity,
|
||||
resolveExecApprovalsFromFile,
|
||||
} from "../../infra/exec-approvals.js";
|
||||
import { buildNodeShellCommand } from "../../infra/node-shell.js";
|
||||
import { applyPathPrepend } from "../../infra/path-prepend.js";
|
||||
import { normalizeSystemRunApprovalPlanV2 } from "../../infra/system-run-approval-binding.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import { parseEnvPairs, parseTimeoutMs } from "../nodes-run.js";
|
||||
import { getNodesTheme, runNodesCommand } from "./cli-utils.js";
|
||||
@@ -42,6 +44,22 @@ type ExecDefaults = {
|
||||
safeBins?: string[];
|
||||
};
|
||||
|
||||
function parsePreparedRunPlan(payload: unknown): {
|
||||
cmdText: string;
|
||||
plan: SystemRunApprovalPlanV2;
|
||||
} {
|
||||
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
|
||||
throw new Error("invalid system.run.prepare response");
|
||||
}
|
||||
const raw = payload as { cmdText?: unknown; plan?: unknown };
|
||||
const cmdText = typeof raw.cmdText === "string" ? raw.cmdText.trim() : "";
|
||||
const plan = normalizeSystemRunApprovalPlanV2(raw.plan);
|
||||
if (!cmdText || !plan) {
|
||||
throw new Error("invalid system.run.prepare response");
|
||||
}
|
||||
return { cmdText, plan };
|
||||
}
|
||||
|
||||
function normalizeExecSecurity(value?: string | null): ExecSecurity | null {
|
||||
const normalized = value?.trim().toLowerCase();
|
||||
if (normalized === "deny" || normalized === "allowlist" || normalized === "full") {
|
||||
@@ -192,6 +210,20 @@ export function registerNodesInvokeCommands(nodes: Command) {
|
||||
applyPathPrepend(nodeEnv, execDefaults?.pathPrepend, { requireExisting: true });
|
||||
}
|
||||
|
||||
const prepareResponse = (await callGatewayCli("node.invoke", opts, {
|
||||
nodeId,
|
||||
command: "system.run.prepare",
|
||||
params: {
|
||||
command: argv,
|
||||
rawCommand,
|
||||
cwd: opts.cwd,
|
||||
agentId,
|
||||
},
|
||||
idempotencyKey: `prepare-${randomIdempotencyKey()}`,
|
||||
})) as { payload?: unknown } | null;
|
||||
const prepared = parsePreparedRunPlan(prepareResponse?.payload);
|
||||
const approvalPlan = prepared.plan;
|
||||
|
||||
let approvedByAsk = false;
|
||||
let approvalDecision: "allow-once" | "allow-always" | null = null;
|
||||
const configuredSecurity = normalizeExecSecurity(execDefaults?.security) ?? "allowlist";
|
||||
@@ -251,16 +283,17 @@ export function registerNodesInvokeCommands(nodes: Command) {
|
||||
opts,
|
||||
{
|
||||
id: approvalId,
|
||||
command: rawCommand ?? argv.join(" "),
|
||||
commandArgv: argv,
|
||||
cwd: opts.cwd,
|
||||
command: prepared.cmdText,
|
||||
commandArgv: approvalPlan.argv,
|
||||
systemRunPlanV2: approvalPlan,
|
||||
cwd: approvalPlan.cwd,
|
||||
nodeId,
|
||||
host: "node",
|
||||
security: hostSecurity,
|
||||
ask: hostAsk,
|
||||
agentId,
|
||||
agentId: approvalPlan.agentId ?? agentId,
|
||||
resolvedPath: undefined,
|
||||
sessionKey: undefined,
|
||||
sessionKey: approvalPlan.sessionKey ?? undefined,
|
||||
timeoutMs: approvalTimeoutMs,
|
||||
},
|
||||
{ transportTimeoutMs },
|
||||
@@ -296,19 +329,21 @@ export function registerNodesInvokeCommands(nodes: Command) {
|
||||
nodeId,
|
||||
command: "system.run",
|
||||
params: {
|
||||
command: argv,
|
||||
cwd: opts.cwd,
|
||||
command: approvalPlan.argv,
|
||||
rawCommand: approvalPlan.rawCommand,
|
||||
cwd: approvalPlan.cwd,
|
||||
env: nodeEnv,
|
||||
timeoutMs,
|
||||
needsScreenRecording: opts.needsScreenRecording === true,
|
||||
},
|
||||
idempotencyKey: String(opts.idempotencyKey ?? randomIdempotencyKey()),
|
||||
};
|
||||
if (agentId) {
|
||||
(invokeParams.params as Record<string, unknown>).agentId = agentId;
|
||||
if (approvalPlan.agentId ?? agentId) {
|
||||
(invokeParams.params as Record<string, unknown>).agentId =
|
||||
approvalPlan.agentId ?? agentId;
|
||||
}
|
||||
if (rawCommand) {
|
||||
(invokeParams.params as Record<string, unknown>).rawCommand = rawCommand;
|
||||
if (approvalPlan.sessionKey) {
|
||||
(invokeParams.params as Record<string, unknown>).sessionKey = approvalPlan.sessionKey;
|
||||
}
|
||||
(invokeParams.params as Record<string, unknown>).approved = approvedByAsk;
|
||||
if (approvalDecision) {
|
||||
|
||||
@@ -40,7 +40,13 @@ const SMS_DANGEROUS_COMMANDS = ["sms.send"];
|
||||
// iOS nodes don't implement system.run/which, but they do support notifications.
|
||||
const IOS_SYSTEM_COMMANDS = ["system.notify"];
|
||||
|
||||
const SYSTEM_COMMANDS = ["system.run", "system.which", "system.notify", "browser.proxy"];
|
||||
const SYSTEM_COMMANDS = [
|
||||
"system.run.prepare",
|
||||
"system.run",
|
||||
"system.which",
|
||||
"system.notify",
|
||||
"browser.proxy",
|
||||
];
|
||||
|
||||
// "High risk" node commands. These can be enabled by explicitly adding them to
|
||||
// `gateway.nodes.allowCommands` (and ensuring they're not blocked by denyCommands).
|
||||
|
||||
@@ -229,6 +229,50 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
expectAllowOnceForwardingResult(result);
|
||||
});
|
||||
|
||||
test("uses systemRunPlanV2 for forwarded command context and ignores caller tampering", () => {
|
||||
const record = makeRecord("echo SAFE", ["echo", "SAFE"]);
|
||||
record.request.systemRunPlanV2 = {
|
||||
version: 2,
|
||||
argv: ["/usr/bin/echo", "SAFE"],
|
||||
cwd: "/real/cwd",
|
||||
rawCommand: "/usr/bin/echo SAFE",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
};
|
||||
record.request.systemRunBindingV1 = buildSystemRunApprovalBindingV1({
|
||||
argv: ["/usr/bin/echo", "SAFE"],
|
||||
cwd: "/real/cwd",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
}).binding;
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
command: ["echo", "PWNED"],
|
||||
rawCommand: "echo PWNED",
|
||||
cwd: "/tmp/attacker-link/sub",
|
||||
agentId: "attacker",
|
||||
sessionKey: "agent:attacker:main",
|
||||
runId: "approval-1",
|
||||
approved: true,
|
||||
approvalDecision: "allow-once",
|
||||
},
|
||||
nodeId: "node-1",
|
||||
client,
|
||||
execApprovalManager: manager(record),
|
||||
nowMs: now,
|
||||
});
|
||||
expectAllowOnceForwardingResult(result);
|
||||
if (!result.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
const forwarded = result.params as Record<string, unknown>;
|
||||
expect(forwarded.command).toEqual(["/usr/bin/echo", "SAFE"]);
|
||||
expect(forwarded.rawCommand).toBe("/usr/bin/echo SAFE");
|
||||
expect(forwarded.cwd).toBe("/real/cwd");
|
||||
expect(forwarded.agentId).toBe("main");
|
||||
expect(forwarded.sessionKey).toBe("agent:main:main");
|
||||
});
|
||||
|
||||
test("rejects env overrides when approval record lacks env binding", () => {
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizeSystemRunApprovalPlanV2 } from "../infra/system-run-approval-binding.js";
|
||||
import { resolveSystemRunCommand } from "../infra/system-run-command.js";
|
||||
import type { ExecApprovalRecord } from "./exec-approval-manager.js";
|
||||
import {
|
||||
@@ -99,18 +100,6 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
}
|
||||
|
||||
const p = obj as SystemRunParamsLike;
|
||||
const cmdTextResolution = resolveSystemRunCommand({
|
||||
command: p.command,
|
||||
rawCommand: p.rawCommand,
|
||||
});
|
||||
if (!cmdTextResolution.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
message: cmdTextResolution.message,
|
||||
details: cmdTextResolution.details,
|
||||
};
|
||||
}
|
||||
|
||||
const approved = p.approved === true;
|
||||
const requestedDecision = normalizeApprovalDecision(p.approvalDecision);
|
||||
const wantsApprovalOverride = approved || requestedDecision !== null;
|
||||
@@ -120,6 +109,17 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
const next: Record<string, unknown> = pickSystemRunParams(obj);
|
||||
|
||||
if (!wantsApprovalOverride) {
|
||||
const cmdTextResolution = resolveSystemRunCommand({
|
||||
command: p.command,
|
||||
rawCommand: p.rawCommand,
|
||||
});
|
||||
if (!cmdTextResolution.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
message: cmdTextResolution.message,
|
||||
details: cmdTextResolution.details,
|
||||
};
|
||||
}
|
||||
return { ok: true, params: next };
|
||||
}
|
||||
|
||||
@@ -206,13 +206,62 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
};
|
||||
}
|
||||
|
||||
const planV2 = normalizeSystemRunApprovalPlanV2(snapshot.request.systemRunPlanV2 ?? null);
|
||||
let approvalArgv: string[];
|
||||
let approvalCwd: string | null;
|
||||
let approvalAgentId: string | null;
|
||||
let approvalSessionKey: string | null;
|
||||
if (planV2) {
|
||||
approvalArgv = [...planV2.argv];
|
||||
approvalCwd = planV2.cwd;
|
||||
approvalAgentId = planV2.agentId;
|
||||
approvalSessionKey = planV2.sessionKey;
|
||||
next.command = [...planV2.argv];
|
||||
if (planV2.rawCommand) {
|
||||
next.rawCommand = planV2.rawCommand;
|
||||
} else {
|
||||
delete next.rawCommand;
|
||||
}
|
||||
if (planV2.cwd) {
|
||||
next.cwd = planV2.cwd;
|
||||
} else {
|
||||
delete next.cwd;
|
||||
}
|
||||
if (planV2.agentId) {
|
||||
next.agentId = planV2.agentId;
|
||||
} else {
|
||||
delete next.agentId;
|
||||
}
|
||||
if (planV2.sessionKey) {
|
||||
next.sessionKey = planV2.sessionKey;
|
||||
} else {
|
||||
delete next.sessionKey;
|
||||
}
|
||||
} else {
|
||||
const cmdTextResolution = resolveSystemRunCommand({
|
||||
command: p.command,
|
||||
rawCommand: p.rawCommand,
|
||||
});
|
||||
if (!cmdTextResolution.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
message: cmdTextResolution.message,
|
||||
details: cmdTextResolution.details,
|
||||
};
|
||||
}
|
||||
approvalArgv = cmdTextResolution.argv;
|
||||
approvalCwd = normalizeString(p.cwd) ?? null;
|
||||
approvalAgentId = normalizeString(p.agentId) ?? null;
|
||||
approvalSessionKey = normalizeString(p.sessionKey) ?? null;
|
||||
}
|
||||
|
||||
const approvalMatch = evaluateSystemRunApprovalMatch({
|
||||
argv: cmdTextResolution.argv,
|
||||
argv: approvalArgv,
|
||||
request: snapshot.request,
|
||||
binding: {
|
||||
cwd: normalizeString(p.cwd) ?? null,
|
||||
agentId: normalizeString(p.agentId) ?? null,
|
||||
sessionKey: normalizeString(p.sessionKey) ?? null,
|
||||
cwd: approvalCwd,
|
||||
agentId: approvalAgentId,
|
||||
sessionKey: approvalSessionKey,
|
||||
env: p.env,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -90,6 +90,19 @@ export const ExecApprovalRequestParamsSchema = Type.Object(
|
||||
id: Type.Optional(NonEmptyString),
|
||||
command: NonEmptyString,
|
||||
commandArgv: Type.Optional(Type.Array(Type.String())),
|
||||
systemRunPlanV2: Type.Optional(
|
||||
Type.Object(
|
||||
{
|
||||
version: Type.Literal(2),
|
||||
argv: Type.Array(Type.String()),
|
||||
cwd: Type.Union([Type.String(), Type.Null()]),
|
||||
rawCommand: Type.Union([Type.String(), Type.Null()]),
|
||||
agentId: Type.Union([Type.String(), Type.Null()]),
|
||||
sessionKey: Type.Union([Type.String(), Type.Null()]),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
),
|
||||
),
|
||||
env: Type.Optional(Type.Record(NonEmptyString, Type.String())),
|
||||
cwd: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
nodeId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
|
||||
@@ -3,7 +3,11 @@ import {
|
||||
DEFAULT_EXEC_APPROVAL_TIMEOUT_MS,
|
||||
type ExecApprovalDecision,
|
||||
} from "../../infra/exec-approvals.js";
|
||||
import { buildSystemRunApprovalBindingV1 } from "../../infra/system-run-approval-binding.js";
|
||||
import {
|
||||
buildSystemRunApprovalBindingV1,
|
||||
normalizeSystemRunApprovalPlanV2,
|
||||
} from "../../infra/system-run-approval-binding.js";
|
||||
import { formatExecCommand } from "../../infra/system-run-command.js";
|
||||
import type { ExecApprovalManager } from "../exec-approval-manager.js";
|
||||
import {
|
||||
ErrorCodes,
|
||||
@@ -47,6 +51,7 @@ export function createExecApprovalHandlers(
|
||||
commandArgv?: string[];
|
||||
env?: Record<string, string>;
|
||||
cwd?: string;
|
||||
systemRunPlanV2?: unknown;
|
||||
nodeId?: string;
|
||||
host?: string;
|
||||
security?: string;
|
||||
@@ -70,6 +75,18 @@ export function createExecApprovalHandlers(
|
||||
const commandArgv = Array.isArray(p.commandArgv)
|
||||
? p.commandArgv.map((entry) => String(entry))
|
||||
: undefined;
|
||||
const systemRunPlanV2 =
|
||||
host === "node" ? normalizeSystemRunApprovalPlanV2(p.systemRunPlanV2) : null;
|
||||
const effectiveCommandArgv = systemRunPlanV2?.argv ?? commandArgv;
|
||||
const effectiveCwd = systemRunPlanV2?.cwd ?? p.cwd;
|
||||
const effectiveAgentId = systemRunPlanV2?.agentId ?? p.agentId;
|
||||
const effectiveSessionKey = systemRunPlanV2?.sessionKey ?? p.sessionKey;
|
||||
const effectiveCommandText = (() => {
|
||||
if (!systemRunPlanV2) {
|
||||
return p.command;
|
||||
}
|
||||
return systemRunPlanV2.rawCommand ?? formatExecCommand(systemRunPlanV2.argv);
|
||||
})();
|
||||
if (host === "node" && !nodeId) {
|
||||
respond(
|
||||
false,
|
||||
@@ -78,7 +95,10 @@ export function createExecApprovalHandlers(
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (host === "node" && (!Array.isArray(commandArgv) || commandArgv.length === 0)) {
|
||||
if (
|
||||
host === "node" &&
|
||||
(!Array.isArray(effectiveCommandArgv) || effectiveCommandArgv.length === 0)
|
||||
) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
@@ -89,10 +109,10 @@ export function createExecApprovalHandlers(
|
||||
const systemRunBindingV1 =
|
||||
host === "node"
|
||||
? buildSystemRunApprovalBindingV1({
|
||||
argv: commandArgv,
|
||||
cwd: p.cwd,
|
||||
agentId: p.agentId,
|
||||
sessionKey: p.sessionKey,
|
||||
argv: effectiveCommandArgv,
|
||||
cwd: effectiveCwd,
|
||||
agentId: effectiveAgentId,
|
||||
sessionKey: effectiveSessionKey,
|
||||
env: p.env,
|
||||
})
|
||||
: null;
|
||||
@@ -105,18 +125,19 @@ export function createExecApprovalHandlers(
|
||||
return;
|
||||
}
|
||||
const request = {
|
||||
command: p.command,
|
||||
commandArgv,
|
||||
command: effectiveCommandText,
|
||||
commandArgv: effectiveCommandArgv,
|
||||
envKeys: systemRunBindingV1?.envKeys?.length ? systemRunBindingV1.envKeys : undefined,
|
||||
systemRunBindingV1: systemRunBindingV1?.binding ?? null,
|
||||
cwd: p.cwd ?? null,
|
||||
systemRunPlanV2: systemRunPlanV2,
|
||||
cwd: effectiveCwd ?? null,
|
||||
nodeId: host === "node" ? nodeId : null,
|
||||
host: host || null,
|
||||
security: p.security ?? null,
|
||||
ask: p.ask ?? null,
|
||||
agentId: p.agentId ?? null,
|
||||
agentId: effectiveAgentId ?? null,
|
||||
resolvedPath: p.resolvedPath ?? null,
|
||||
sessionKey: p.sessionKey ?? null,
|
||||
sessionKey: effectiveSessionKey ?? null,
|
||||
turnSourceChannel:
|
||||
typeof p.turnSourceChannel === "string" ? p.turnSourceChannel.trim() || null : null,
|
||||
turnSourceTo: typeof p.turnSourceTo === "string" ? p.turnSourceTo.trim() || null : null,
|
||||
|
||||
@@ -471,6 +471,44 @@ describe("exec approval handlers", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("prefers systemRunPlanV2 canonical command/cwd when present", async () => {
|
||||
const { handlers, broadcasts, respond, context } = createExecApprovalFixture();
|
||||
await requestExecApproval({
|
||||
handlers,
|
||||
respond,
|
||||
context,
|
||||
params: {
|
||||
command: "echo stale",
|
||||
commandArgv: ["echo", "stale"],
|
||||
cwd: "/tmp/link/sub",
|
||||
systemRunPlanV2: {
|
||||
version: 2,
|
||||
argv: ["/usr/bin/echo", "ok"],
|
||||
cwd: "/real/cwd",
|
||||
rawCommand: "/usr/bin/echo ok",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
},
|
||||
},
|
||||
});
|
||||
const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested");
|
||||
expect(requested).toBeTruthy();
|
||||
const request = (requested?.payload as { request?: Record<string, unknown> })?.request ?? {};
|
||||
expect(request["command"]).toBe("/usr/bin/echo ok");
|
||||
expect(request["commandArgv"]).toEqual(["/usr/bin/echo", "ok"]);
|
||||
expect(request["cwd"]).toBe("/real/cwd");
|
||||
expect(request["agentId"]).toBe("main");
|
||||
expect(request["sessionKey"]).toBe("agent:main:main");
|
||||
expect(request["systemRunPlanV2"]).toEqual({
|
||||
version: 2,
|
||||
argv: ["/usr/bin/echo", "ok"],
|
||||
cwd: "/real/cwd",
|
||||
rawCommand: "/usr/bin/echo ok",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts resolve during broadcast", async () => {
|
||||
const manager = new ExecApprovalManager();
|
||||
const handlers = createExecApprovalHandlers(manager);
|
||||
|
||||
@@ -20,12 +20,22 @@ export type SystemRunApprovalBindingV1 = {
|
||||
envHash: string | null;
|
||||
};
|
||||
|
||||
export type SystemRunApprovalPlanV2 = {
|
||||
version: 2;
|
||||
argv: string[];
|
||||
cwd: string | null;
|
||||
rawCommand: string | null;
|
||||
agentId: string | null;
|
||||
sessionKey: string | null;
|
||||
};
|
||||
|
||||
export type ExecApprovalRequestPayload = {
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
// Optional UI-safe env key preview for approval prompts.
|
||||
envKeys?: string[];
|
||||
systemRunBindingV1?: SystemRunApprovalBindingV1 | null;
|
||||
systemRunPlanV2?: SystemRunApprovalPlanV2 | null;
|
||||
cwd?: string | null;
|
||||
nodeId?: string | null;
|
||||
host?: string | null;
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import crypto from "node:crypto";
|
||||
import type { SystemRunApprovalBindingV1 } from "./exec-approvals.js";
|
||||
import type { SystemRunApprovalBindingV1, SystemRunApprovalPlanV2 } from "./exec-approvals.js";
|
||||
import { normalizeEnvVarKey } from "./host-env-security.js";
|
||||
|
||||
type NormalizedSystemRunEnvEntry = [key: string, value: string];
|
||||
@@ -16,6 +16,28 @@ function normalizeStringArray(value: unknown): string[] {
|
||||
return Array.isArray(value) ? value.map((entry) => String(entry)) : [];
|
||||
}
|
||||
|
||||
export function normalizeSystemRunApprovalPlanV2(value: unknown): SystemRunApprovalPlanV2 | null {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
return null;
|
||||
}
|
||||
const candidate = value as Record<string, unknown>;
|
||||
if (candidate.version !== 2) {
|
||||
return null;
|
||||
}
|
||||
const argv = normalizeStringArray(candidate.argv);
|
||||
if (argv.length === 0) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
version: 2,
|
||||
argv,
|
||||
cwd: normalizeString(candidate.cwd),
|
||||
rawCommand: normalizeString(candidate.rawCommand),
|
||||
agentId: normalizeString(candidate.agentId),
|
||||
sessionKey: normalizeString(candidate.sessionKey),
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeSystemRunEnvEntries(env: unknown): NormalizedSystemRunEnvEntry[] {
|
||||
if (!env || typeof env !== "object" || Array.isArray(env)) {
|
||||
return [];
|
||||
|
||||
@@ -252,6 +252,39 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"denies approval-based execution when cwd contains a symlink parent component",
|
||||
async () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-parent-link-"));
|
||||
const safeRoot = path.join(tmp, "safe-root");
|
||||
const safeSub = path.join(safeRoot, "sub");
|
||||
const linkRoot = path.join(tmp, "approved-link");
|
||||
fs.mkdirSync(safeSub, { recursive: true });
|
||||
fs.symlinkSync(safeRoot, linkRoot, "dir");
|
||||
try {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
command: ["./run.sh"],
|
||||
cwd: path.join(linkRoot, "sub"),
|
||||
approved: true,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
});
|
||||
expect(runCommand).not.toHaveBeenCalled();
|
||||
expect(sendInvokeResult).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
ok: false,
|
||||
error: expect.objectContaining({
|
||||
message: expect.stringContaining("no symlink path components"),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("uses canonical executable path for approval-based relative command execution", async () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-real-"));
|
||||
const script = path.join(tmp, "run.sh");
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
type ExecAsk,
|
||||
type ExecCommandSegment,
|
||||
type ExecSecurity,
|
||||
type SystemRunApprovalPlanV2,
|
||||
type SkillBinTrustEntry,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js";
|
||||
@@ -113,6 +114,14 @@ function normalizeDeniedReason(reason: string | null | undefined): SystemRunDeni
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeString(value: unknown): string | null {
|
||||
if (typeof value !== "string") {
|
||||
return null;
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
return trimmed ? trimmed : null;
|
||||
}
|
||||
|
||||
function isPathLikeExecutableToken(value: string): boolean {
|
||||
if (!value) {
|
||||
return false;
|
||||
@@ -129,6 +138,46 @@ function isPathLikeExecutableToken(value: string): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
function pathComponentsFromRootSync(targetPath: string): string[] {
|
||||
const absolute = path.resolve(targetPath);
|
||||
const parts: string[] = [];
|
||||
let cursor = absolute;
|
||||
while (true) {
|
||||
parts.unshift(cursor);
|
||||
const parent = path.dirname(cursor);
|
||||
if (parent === cursor) {
|
||||
return parts;
|
||||
}
|
||||
cursor = parent;
|
||||
}
|
||||
}
|
||||
|
||||
function isWritableByCurrentProcessSync(candidate: string): boolean {
|
||||
try {
|
||||
fs.accessSync(candidate, fs.constants.W_OK);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
|
||||
for (const component of pathComponentsFromRootSync(targetPath)) {
|
||||
try {
|
||||
if (!fs.lstatSync(component).isSymbolicLink()) {
|
||||
continue;
|
||||
}
|
||||
const parentDir = path.dirname(component);
|
||||
if (isWritableByCurrentProcessSync(parentDir)) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function hardenApprovedExecutionPaths(params: {
|
||||
approvedByAsk: boolean;
|
||||
argv: string[];
|
||||
@@ -163,6 +212,12 @@ function hardenApprovedExecutionPaths(params: {
|
||||
message: "SYSTEM_RUN_DENIED: approval requires cwd to be a directory",
|
||||
};
|
||||
}
|
||||
if (hasMutableSymlinkPathComponentSync(requestedCwd)) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "SYSTEM_RUN_DENIED: approval requires canonical cwd (no symlink path components)",
|
||||
};
|
||||
}
|
||||
if (cwdLstat.isSymbolicLink()) {
|
||||
return {
|
||||
ok: false,
|
||||
@@ -207,6 +262,46 @@ function hardenApprovedExecutionPaths(params: {
|
||||
return { ok: true, argv, cwd: hardenedCwd };
|
||||
}
|
||||
|
||||
export function buildSystemRunApprovalPlanV2(params: {
|
||||
command?: unknown;
|
||||
rawCommand?: unknown;
|
||||
cwd?: unknown;
|
||||
agentId?: unknown;
|
||||
sessionKey?: unknown;
|
||||
}): { ok: true; plan: SystemRunApprovalPlanV2; cmdText: string } | { ok: false; message: string } {
|
||||
const command = resolveSystemRunCommand({
|
||||
command: params.command,
|
||||
rawCommand: params.rawCommand,
|
||||
});
|
||||
if (!command.ok) {
|
||||
return { ok: false, message: command.message };
|
||||
}
|
||||
if (command.argv.length === 0) {
|
||||
return { ok: false, message: "command required" };
|
||||
}
|
||||
const hardening = hardenApprovedExecutionPaths({
|
||||
approvedByAsk: true,
|
||||
argv: command.argv,
|
||||
shellCommand: command.shellCommand,
|
||||
cwd: normalizeString(params.cwd) ?? undefined,
|
||||
});
|
||||
if (!hardening.ok) {
|
||||
return { ok: false, message: hardening.message };
|
||||
}
|
||||
return {
|
||||
ok: true,
|
||||
plan: {
|
||||
version: 2,
|
||||
argv: hardening.argv,
|
||||
cwd: hardening.cwd ?? null,
|
||||
rawCommand: command.cmdText.trim() || null,
|
||||
agentId: normalizeString(params.agentId),
|
||||
sessionKey: normalizeString(params.sessionKey),
|
||||
},
|
||||
cmdText: command.cmdText,
|
||||
};
|
||||
}
|
||||
|
||||
export type HandleSystemRunInvokeOptions = {
|
||||
client: GatewayClient;
|
||||
params: SystemRunParams;
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
} from "../infra/exec-host.js";
|
||||
import { sanitizeHostExecEnv } from "../infra/host-env-security.js";
|
||||
import { runBrowserProxyCommand } from "./invoke-browser.js";
|
||||
import { handleSystemRunInvoke } from "./invoke-system-run.js";
|
||||
import { buildSystemRunApprovalPlanV2, handleSystemRunInvoke } from "./invoke-system-run.js";
|
||||
import type {
|
||||
ExecEventPayload,
|
||||
RunResult,
|
||||
@@ -420,6 +420,30 @@ export async function handleInvoke(
|
||||
return;
|
||||
}
|
||||
|
||||
if (command === "system.run.prepare") {
|
||||
try {
|
||||
const params = decodeParams<{
|
||||
command?: unknown;
|
||||
rawCommand?: unknown;
|
||||
cwd?: unknown;
|
||||
agentId?: unknown;
|
||||
sessionKey?: unknown;
|
||||
}>(frame.paramsJSON);
|
||||
const prepared = buildSystemRunApprovalPlanV2(params);
|
||||
if (!prepared.ok) {
|
||||
await sendErrorResult(client, frame, "INVALID_REQUEST", prepared.message);
|
||||
return;
|
||||
}
|
||||
await sendJsonPayloadResult(client, frame, {
|
||||
cmdText: prepared.cmdText,
|
||||
plan: prepared.plan,
|
||||
});
|
||||
} catch (err) {
|
||||
await sendInvalidRequestResult(client, frame, err);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (command !== "system.run") {
|
||||
await sendErrorResult(client, frame, "UNAVAILABLE", "command not supported");
|
||||
return;
|
||||
|
||||
@@ -189,6 +189,7 @@ export async function runNodeHost(opts: NodeHostRunOptions): Promise<void> {
|
||||
scopes: [],
|
||||
caps: ["system", ...(browserProxyEnabled ? ["browser"] : [])],
|
||||
commands: [
|
||||
"system.run.prepare",
|
||||
"system.run",
|
||||
"system.which",
|
||||
"system.execApprovals.get",
|
||||
|
||||
Reference in New Issue
Block a user