mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-11 23:10:29 +00:00
fix(node-host): fail closed on unbound interpreter approvals
This commit is contained in:
@@ -125,6 +125,7 @@ Plugins/extensions are part of OpenClaw's trusted computing base for a gateway.
|
||||
- Any report whose only claim is that an operator-enabled `dangerous*`/`dangerously*` config option weakens defaults (these are explicit break-glass tradeoffs by design)
|
||||
- Reports that depend on trusted operator-supplied configuration values to trigger availability impact (for example custom regex patterns). These may still be fixed as defense-in-depth hardening, but are not security-boundary bypasses.
|
||||
- Reports whose only claim is heuristic/parity drift in command-risk detection (for example obfuscation-pattern checks) across exec surfaces, without a demonstrated trust-boundary bypass. These are hardening-only findings and are not vulnerabilities; triage may close them as `invalid`/`no-action` or track them separately as low/informational hardening.
|
||||
- Reports whose only claim is that exec approvals do not semantically model every interpreter/runtime loader form, subcommand, flag combination, package script, or transitive module/config import. Exec approvals bind exact request context and best-effort direct local file operands; they are not a complete semantic model of everything a runtime may load.
|
||||
- Exposed secrets that are third-party/user-controlled credentials (not OpenClaw-owned and not granting access to OpenClaw-operated infrastructure/services) without demonstrated OpenClaw impact
|
||||
- Reports whose only claim is host-side exec when sandbox runtime is disabled/unavailable (documented default behavior in the trusted-operator model), without a boundary bypass.
|
||||
- Reports whose only claim is that a platform-provided upload destination URL is untrusted (for example Microsoft Teams `fileConsent/invoke` `uploadInfo.uploadUrl`) without proving attacker control in an authenticated production flow.
|
||||
@@ -165,6 +166,7 @@ OpenClaw separates routing from execution, but both remain inside the same opera
|
||||
- **Gateway** is the control plane. If a caller passes Gateway auth, they are treated as a trusted operator for that Gateway.
|
||||
- **Node** is an execution extension of the Gateway. Pairing a node grants operator-level remote capability on that node.
|
||||
- **Exec approvals** (allowlist/ask UI) are operator guardrails to reduce accidental command execution, not a multi-tenant authorization boundary.
|
||||
- Exec approvals bind exact command/cwd/env context and, when OpenClaw can identify one concrete local script/file operand, that file snapshot too. This is best-effort integrity hardening, not a complete semantic model of every interpreter/runtime loader path.
|
||||
- Differences in command-risk warning heuristics between exec surfaces (`gateway`, `node`, `sandbox`) do not, by themselves, constitute a security-boundary bypass.
|
||||
- For untrusted-user isolation, split by trust boundary: separate gateways and separate OS users/hosts per boundary.
|
||||
|
||||
|
||||
@@ -104,6 +104,7 @@ Treat Gateway and node as one operator trust domain, with different roles:
|
||||
- A caller authenticated to the Gateway is trusted at Gateway scope. After pairing, node actions are trusted operator actions on that node.
|
||||
- `sessionKey` is routing/context selection, not per-user auth.
|
||||
- Exec approvals (allowlist + ask) are guardrails for operator intent, not hostile multi-tenant isolation.
|
||||
- Exec approvals bind exact request context and best-effort direct local file operands; they do not semantically model every runtime/interpreter loader path. Use sandboxing and host isolation for strong boundaries.
|
||||
|
||||
If you need hostile-user isolation, split trust boundaries by OS user/host and run separate gateways.
|
||||
|
||||
@@ -370,6 +371,7 @@ If a macOS node is paired, the Gateway can invoke `system.run` on that node. Thi
|
||||
|
||||
- Requires node pairing (approval + token).
|
||||
- Controlled on the Mac via **Settings → Exec approvals** (security + ask + allowlist).
|
||||
- Approval mode binds exact request context and, when possible, one concrete local script/file operand. If OpenClaw cannot identify exactly one direct local file for an interpreter/runtime command, approval-backed execution is denied rather than promising full semantic coverage.
|
||||
- If you don’t want remote execution, set security to **deny** and remove node pairing for that Mac.
|
||||
|
||||
## Dynamic skills (watcher / remote nodes)
|
||||
|
||||
@@ -54,6 +54,15 @@ forwards `exec` calls to the **node host** when `host=node` is selected.
|
||||
- **Node host**: executes `system.run`/`system.which` on the node machine.
|
||||
- **Approvals**: enforced on the node host via `~/.openclaw/exec-approvals.json`.
|
||||
|
||||
Approval note:
|
||||
|
||||
- Approval-backed node runs bind exact request context.
|
||||
- For direct shell/runtime file executions, OpenClaw also best-effort binds one concrete local
|
||||
file operand and denies the run if that file changes before execution.
|
||||
- If OpenClaw cannot identify exactly one concrete local file for an interpreter/runtime command,
|
||||
approval-backed execution is denied instead of pretending full runtime coverage. Use sandboxing,
|
||||
separate hosts, or an explicit trusted allowlist/full workflow for broader interpreter semantics.
|
||||
|
||||
### Start a node host (foreground)
|
||||
|
||||
On the node machine:
|
||||
|
||||
@@ -30,9 +30,14 @@ Trust model note:
|
||||
- Gateway-authenticated callers are trusted operators for that Gateway.
|
||||
- Paired nodes extend that trusted operator capability onto the node host.
|
||||
- Exec approvals reduce accidental execution risk, but are not a per-user auth boundary.
|
||||
- Approved node-host runs also bind canonical execution context: canonical cwd, pinned executable
|
||||
path when applicable, and interpreter-style script operands. If a bound script changes after
|
||||
approval but before execution, the run is denied instead of executing drifted content.
|
||||
- Approved node-host runs bind canonical execution context: canonical cwd, exact argv, env
|
||||
binding when present, and pinned executable path when applicable.
|
||||
- For shell scripts and direct interpreter/runtime file invocations, OpenClaw also tries to bind
|
||||
one concrete local file operand. If that bound file changes after approval but before execution,
|
||||
the run is denied instead of executing drifted content.
|
||||
- This file binding is intentionally best-effort, not a complete semantic model of every
|
||||
interpreter/runtime loader path. If approval mode cannot identify exactly one concrete local
|
||||
file to bind, it refuses to mint an approval-backed run instead of pretending full coverage.
|
||||
|
||||
macOS split:
|
||||
|
||||
@@ -259,6 +264,20 @@ For `host=node`, approval requests include a canonical `systemRunPlan` payload.
|
||||
that plan as the authoritative command/cwd/session context when forwarding approved `system.run`
|
||||
requests.
|
||||
|
||||
## Interpreter/runtime commands
|
||||
|
||||
Approval-backed interpreter/runtime runs are intentionally conservative:
|
||||
|
||||
- Exact argv/cwd/env context is always bound.
|
||||
- Direct shell script and direct runtime file forms are best-effort bound to one concrete local
|
||||
file snapshot.
|
||||
- If OpenClaw cannot identify exactly one concrete local file for an interpreter/runtime command
|
||||
(for example package scripts, eval forms, runtime-specific loader chains, or ambiguous multi-file
|
||||
forms), approval-backed execution is denied instead of claiming semantic coverage it does not
|
||||
have.
|
||||
- For those workflows, prefer sandboxing, a separate host boundary, or an explicit trusted
|
||||
allowlist/full workflow where the operator accepts the broader runtime semantics.
|
||||
|
||||
When approvals are required, the exec tool returns immediately with an approval id. Use that id to
|
||||
correlate later system events (`Exec finished` / `Exec denied`). If no decision arrives before the
|
||||
timeout, the request is treated as an approval timeout and surfaced as a denial reason.
|
||||
|
||||
@@ -214,6 +214,38 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
}
|
||||
|
||||
const mutableOperandCases: RuntimeFixture[] = [
|
||||
{
|
||||
name: "python flagged file",
|
||||
binName: "python3",
|
||||
argv: ["python3", "-B", "./run.py"],
|
||||
scriptName: "run.py",
|
||||
initialBody: 'print("SAFE")\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
{
|
||||
name: "lua direct file",
|
||||
binName: "lua",
|
||||
argv: ["lua", "./run.lua"],
|
||||
scriptName: "run.lua",
|
||||
initialBody: 'print("SAFE")\n',
|
||||
expectedArgvIndex: 1,
|
||||
},
|
||||
{
|
||||
name: "pypy direct file",
|
||||
binName: "pypy",
|
||||
argv: ["pypy", "./run.py"],
|
||||
scriptName: "run.py",
|
||||
initialBody: 'print("SAFE")\n',
|
||||
expectedArgvIndex: 1,
|
||||
},
|
||||
{
|
||||
name: "versioned node alias file",
|
||||
binName: "node20",
|
||||
argv: ["node20", "./run.js"],
|
||||
scriptName: "run.js",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 1,
|
||||
},
|
||||
{
|
||||
name: "bun direct file",
|
||||
binName: "bun",
|
||||
@@ -238,6 +270,22 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 5,
|
||||
},
|
||||
{
|
||||
name: "bun test file",
|
||||
binName: "bun",
|
||||
argv: ["bun", "test", "./run.test.ts"],
|
||||
scriptName: "run.test.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
{
|
||||
name: "deno test file",
|
||||
binName: "deno",
|
||||
argv: ["deno", "test", "./run.test.ts"],
|
||||
scriptName: "run.test.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
];
|
||||
|
||||
for (const runtimeCase of mutableOperandCases) {
|
||||
@@ -296,7 +344,7 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("does not snapshot bun package script names", () => {
|
||||
it("rejects bun package script names that do not bind a concrete file", () => {
|
||||
withFakeRuntimeBin({
|
||||
binName: "bun",
|
||||
run: () => {
|
||||
@@ -306,11 +354,11 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
command: ["bun", "run", "dev"],
|
||||
cwd: tmp,
|
||||
});
|
||||
expect(prepared.ok).toBe(true);
|
||||
if (!prepared.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(prepared.plan.mutableFileOperand).toBeUndefined();
|
||||
expect(prepared).toEqual({
|
||||
ok: false,
|
||||
message:
|
||||
"SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
@@ -318,7 +366,7 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("does not snapshot deno eval invocations", () => {
|
||||
it("rejects deno eval invocations that do not bind a concrete file", () => {
|
||||
withFakeRuntimeBin({
|
||||
binName: "deno",
|
||||
run: () => {
|
||||
@@ -328,11 +376,11 @@ describe("hardenApprovedExecutionPaths", () => {
|
||||
command: ["deno", "eval", "console.log('SAFE')"],
|
||||
cwd: tmp,
|
||||
});
|
||||
expect(prepared.ok).toBe(true);
|
||||
if (!prepared.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(prepared.plan.mutableFileOperand).toBeUndefined();
|
||||
expect(prepared).toEqual({
|
||||
ok: false,
|
||||
message:
|
||||
"SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
SystemRunApprovalPlan,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js";
|
||||
import { isInterpreterLikeSafeBin } from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import {
|
||||
POSIX_SHELL_WRAPPERS,
|
||||
normalizeExecutableToken,
|
||||
@@ -316,6 +317,53 @@ function resolveOptionFilteredPositionalIndex(params: {
|
||||
return null;
|
||||
}
|
||||
|
||||
function collectExistingFileOperandIndexes(params: {
|
||||
argv: string[];
|
||||
startIndex: number;
|
||||
cwd: string | undefined;
|
||||
}): number[] {
|
||||
let afterDoubleDash = false;
|
||||
const hits: number[] = [];
|
||||
for (let i = params.startIndex; i < params.argv.length; i += 1) {
|
||||
const token = params.argv[i]?.trim() ?? "";
|
||||
if (!token) {
|
||||
continue;
|
||||
}
|
||||
if (afterDoubleDash) {
|
||||
if (resolvesToExistingFileSync(token, params.cwd)) {
|
||||
hits.push(i);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (token === "--") {
|
||||
afterDoubleDash = true;
|
||||
continue;
|
||||
}
|
||||
if (token === "-") {
|
||||
return [];
|
||||
}
|
||||
if (token.startsWith("-")) {
|
||||
continue;
|
||||
}
|
||||
if (resolvesToExistingFileSync(token, params.cwd)) {
|
||||
hits.push(i);
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
function resolveGenericInterpreterScriptOperandIndex(params: {
|
||||
argv: string[];
|
||||
cwd: string | undefined;
|
||||
}): number | null {
|
||||
const hits = collectExistingFileOperandIndexes({
|
||||
argv: params.argv,
|
||||
startIndex: 1,
|
||||
cwd: params.cwd,
|
||||
});
|
||||
return hits.length === 1 ? hits[0] : null;
|
||||
}
|
||||
|
||||
function resolveBunScriptOperandIndex(params: {
|
||||
argv: string[];
|
||||
cwd: string | undefined;
|
||||
@@ -371,36 +419,76 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined)
|
||||
const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv);
|
||||
return shellIndex === null ? null : unwrapped.baseIndex + shellIndex;
|
||||
}
|
||||
if (!MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) {
|
||||
if (executable === "bun") {
|
||||
const bunIndex = resolveBunScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
});
|
||||
return bunIndex === null ? null : unwrapped.baseIndex + bunIndex;
|
||||
if (MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) {
|
||||
const operand = unwrapped.argv[1]?.trim() ?? "";
|
||||
if (operand && operand !== "-" && !operand.startsWith("-")) {
|
||||
return unwrapped.baseIndex + 1;
|
||||
}
|
||||
if (executable === "deno") {
|
||||
const denoIndex = resolveDenoRunScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
});
|
||||
return denoIndex === null ? null : unwrapped.baseIndex + denoIndex;
|
||||
}
|
||||
if (executable === "bun") {
|
||||
const bunIndex = resolveBunScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
});
|
||||
if (bunIndex !== null) {
|
||||
return unwrapped.baseIndex + bunIndex;
|
||||
}
|
||||
}
|
||||
if (executable === "deno") {
|
||||
const denoIndex = resolveDenoRunScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
});
|
||||
if (denoIndex !== null) {
|
||||
return unwrapped.baseIndex + denoIndex;
|
||||
}
|
||||
}
|
||||
if (!isInterpreterLikeSafeBin(executable)) {
|
||||
return null;
|
||||
}
|
||||
const operand = unwrapped.argv[1]?.trim() ?? "";
|
||||
if (!operand || operand === "-" || operand.startsWith("-")) {
|
||||
return null;
|
||||
const genericIndex = resolveGenericInterpreterScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
});
|
||||
return genericIndex === null ? null : unwrapped.baseIndex + genericIndex;
|
||||
}
|
||||
|
||||
function requiresStableInterpreterApprovalBindingWithShellCommand(params: {
|
||||
argv: string[];
|
||||
shellCommand: string | null;
|
||||
}): boolean {
|
||||
if (params.shellCommand !== null) {
|
||||
return false;
|
||||
}
|
||||
return unwrapped.baseIndex + 1;
|
||||
const unwrapped = unwrapArgvForMutableOperand(params.argv);
|
||||
const executable = normalizeExecutableToken(unwrapped.argv[0] ?? "");
|
||||
if (!executable) {
|
||||
return false;
|
||||
}
|
||||
if ((POSIX_SHELL_WRAPPERS as ReadonlySet<string>).has(executable)) {
|
||||
return false;
|
||||
}
|
||||
return isInterpreterLikeSafeBin(executable);
|
||||
}
|
||||
|
||||
function resolveMutableFileOperandSnapshotSync(params: {
|
||||
argv: string[];
|
||||
cwd: string | undefined;
|
||||
shellCommand: string | null;
|
||||
}): { ok: true; snapshot: SystemRunApprovalFileOperand | null } | { ok: false; message: string } {
|
||||
const argvIndex = resolveMutableFileOperandIndex(params.argv, params.cwd);
|
||||
if (argvIndex === null) {
|
||||
if (
|
||||
requiresStableInterpreterApprovalBindingWithShellCommand({
|
||||
argv: params.argv,
|
||||
shellCommand: params.shellCommand,
|
||||
})
|
||||
) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||
};
|
||||
}
|
||||
return { ok: true, snapshot: null };
|
||||
}
|
||||
const rawOperand = params.argv[argvIndex]?.trim();
|
||||
@@ -658,6 +746,7 @@ export function buildSystemRunApprovalPlan(params: {
|
||||
const mutableFileOperand = resolveMutableFileOperandSnapshotSync({
|
||||
argv: hardening.argv,
|
||||
cwd: hardening.cwd,
|
||||
shellCommand: command.shellCommand,
|
||||
});
|
||||
if (!mutableFileOperand.ok) {
|
||||
return { ok: false, message: mutableFileOperand.message };
|
||||
|
||||
Reference in New Issue
Block a user