mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 03:51:25 +00:00
fix(security): harden exec wrapper allowlist execution parity
This commit is contained in:
@@ -122,6 +122,14 @@ function evaluateSegments(
|
||||
const segmentSatisfiedBy: ExecSegmentSatisfiedBy[] = [];
|
||||
|
||||
const satisfied = segments.every((segment) => {
|
||||
if (segment.resolution?.policyBlocked === true) {
|
||||
segmentSatisfiedBy.push(null);
|
||||
return false;
|
||||
}
|
||||
const effectiveArgv =
|
||||
segment.resolution?.effectiveArgv && segment.resolution.effectiveArgv.length > 0
|
||||
? segment.resolution.effectiveArgv
|
||||
: segment.argv;
|
||||
const candidatePath = resolveAllowlistCandidatePath(segment.resolution, params.cwd);
|
||||
const candidateResolution =
|
||||
candidatePath && segment.resolution
|
||||
@@ -132,7 +140,7 @@ function evaluateSegments(
|
||||
matches.push(match);
|
||||
}
|
||||
const safe = isSafeBinUsage({
|
||||
argv: segment.argv,
|
||||
argv: effectiveArgv,
|
||||
resolution: segment.resolution,
|
||||
safeBins: params.safeBins,
|
||||
safeBinProfiles: params.safeBinProfiles,
|
||||
|
||||
@@ -626,12 +626,30 @@ function renderQuotedArgv(argv: string[]): string {
|
||||
return argv.map((token) => shellEscapeSingleArg(token)).join(" ");
|
||||
}
|
||||
|
||||
function renderSafeBinSegmentArgv(segment: ExecCommandSegment): string {
|
||||
if (segment.argv.length === 0) {
|
||||
return "";
|
||||
function resolvePlannedSegmentArgv(segment: ExecCommandSegment): string[] | null {
|
||||
if (segment.resolution?.policyBlocked === true) {
|
||||
return null;
|
||||
}
|
||||
const baseArgv =
|
||||
segment.resolution?.effectiveArgv && segment.resolution.effectiveArgv.length > 0
|
||||
? segment.resolution.effectiveArgv
|
||||
: segment.argv;
|
||||
if (baseArgv.length === 0) {
|
||||
return null;
|
||||
}
|
||||
const argv = [...baseArgv];
|
||||
const resolvedExecutable = segment.resolution?.resolvedPath?.trim() ?? "";
|
||||
if (resolvedExecutable) {
|
||||
argv[0] = resolvedExecutable;
|
||||
}
|
||||
return argv;
|
||||
}
|
||||
|
||||
function renderSafeBinSegmentArgv(segment: ExecCommandSegment): string | null {
|
||||
const argv = resolvePlannedSegmentArgv(segment);
|
||||
if (!argv || argv.length === 0) {
|
||||
return null;
|
||||
}
|
||||
const resolvedExecutable = segment.resolution?.resolvedPath?.trim();
|
||||
const argv = resolvedExecutable ? [resolvedExecutable, ...segment.argv.slice(1)] : segment.argv;
|
||||
return renderQuotedArgv(argv);
|
||||
}
|
||||
|
||||
@@ -659,7 +677,43 @@ export function buildSafeBinsShellCommand(params: {
|
||||
return { ok: false, reason: "segment mapping failed" };
|
||||
}
|
||||
const needsLiteral = by === "safeBins";
|
||||
return { ok: true, rendered: needsLiteral ? renderSafeBinSegmentArgv(seg) : raw.trim() };
|
||||
if (!needsLiteral) {
|
||||
return { ok: true, rendered: raw.trim() };
|
||||
}
|
||||
const rendered = renderSafeBinSegmentArgv(seg);
|
||||
if (!rendered) {
|
||||
return { ok: false, reason: "segment execution plan unavailable" };
|
||||
}
|
||||
return { ok: true, rendered };
|
||||
},
|
||||
});
|
||||
if (!rebuilt.ok) {
|
||||
return { ok: false, reason: rebuilt.reason };
|
||||
}
|
||||
if (rebuilt.segmentCount !== params.segments.length) {
|
||||
return { ok: false, reason: "segment count mismatch" };
|
||||
}
|
||||
return { ok: true, command: rebuilt.command };
|
||||
}
|
||||
|
||||
export function buildEnforcedShellCommand(params: {
|
||||
command: string;
|
||||
segments: ExecCommandSegment[];
|
||||
platform?: string | null;
|
||||
}): { ok: boolean; command?: string; reason?: string } {
|
||||
const rebuilt = rebuildShellCommandFromSource({
|
||||
command: params.command,
|
||||
platform: params.platform,
|
||||
renderSegment: (_raw, segmentIndex) => {
|
||||
const seg = params.segments[segmentIndex];
|
||||
if (!seg) {
|
||||
return { ok: false, reason: "segment mapping failed" };
|
||||
}
|
||||
const argv = resolvePlannedSegmentArgv(seg);
|
||||
if (!argv) {
|
||||
return { ok: false, reason: "segment execution plan unavailable" };
|
||||
}
|
||||
return { ok: true, rendered: renderQuotedArgv(argv) };
|
||||
},
|
||||
});
|
||||
if (!rebuilt.ok) {
|
||||
|
||||
@@ -221,6 +221,14 @@ describe("exec approvals safe bins", () => {
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
{
|
||||
name: "rejects unknown short options in safe-bin mode",
|
||||
argv: ["tr", "-S", "a", "b"],
|
||||
resolvedPath: "/usr/bin/tr",
|
||||
expected: false,
|
||||
safeBins: ["tr"],
|
||||
executableName: "tr",
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of cases) {
|
||||
@@ -464,4 +472,21 @@ describe("exec approvals safe bins", () => {
|
||||
expect(result.segmentSatisfiedBy).toEqual([null]);
|
||||
expect(result.segments[0]?.resolution?.resolvedPath).toBe(fakeHead);
|
||||
});
|
||||
|
||||
it("fails closed for semantic env wrappers in allowlist mode", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const result = evaluateShellAllowlist({
|
||||
command: "env -S 'sh -c \"echo pwned\"' tr",
|
||||
allowlist: [{ pattern: "/usr/bin/tr" }],
|
||||
safeBins: normalizeSafeBins(["tr"]),
|
||||
cwd: "/tmp",
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
expect(result.segmentSatisfiedBy).toEqual([null]);
|
||||
expect(result.segments[0]?.resolution?.policyBlocked).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@ import { makePathEnv, makeTempDir } from "./exec-approvals-test-helpers.js";
|
||||
import {
|
||||
analyzeArgvCommand,
|
||||
analyzeShellCommand,
|
||||
buildEnforcedShellCommand,
|
||||
buildSafeBinsShellCommand,
|
||||
evaluateExecAllowlist,
|
||||
evaluateShellAllowlist,
|
||||
@@ -130,6 +131,27 @@ describe("exec approvals safe shell command builder", () => {
|
||||
// SafeBins segment is fully quoted and pinned to its resolved absolute path.
|
||||
expect(res.command).toMatch(/'[^']*\/head' '-n' '5'/);
|
||||
});
|
||||
|
||||
it("enforces canonical planned argv for every approved segment", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const analysis = analyzeShellCommand({
|
||||
command: "env rg -n needle",
|
||||
cwd: "/tmp",
|
||||
env: { PATH: "/usr/bin:/bin" },
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(analysis.ok).toBe(true);
|
||||
const res = buildEnforcedShellCommand({
|
||||
command: "env rg -n needle",
|
||||
segments: analysis.segments,
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
expect(res.command).toMatch(/'(?:[^']*\/)?rg' '-n' 'needle'/);
|
||||
expect(res.command).not.toContain("'env'");
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec approvals command resolution", () => {
|
||||
@@ -202,7 +224,7 @@ describe("exec approvals command resolution", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("unwraps env wrapper argv to resolve the effective executable", () => {
|
||||
it("unwraps transparent env wrapper argv to resolve the effective executable", () => {
|
||||
const dir = makeTempDir();
|
||||
const binDir = path.join(dir, "bin");
|
||||
fs.mkdirSync(binDir, { recursive: true });
|
||||
@@ -212,7 +234,7 @@ describe("exec approvals command resolution", () => {
|
||||
fs.chmodSync(exe, 0o755);
|
||||
|
||||
const resolution = resolveCommandResolutionFromArgv(
|
||||
["/usr/bin/env", "FOO=bar", "rg", "-n", "needle"],
|
||||
["/usr/bin/env", "rg", "-n", "needle"],
|
||||
undefined,
|
||||
makePathEnv(binDir),
|
||||
);
|
||||
@@ -220,6 +242,18 @@ describe("exec approvals command resolution", () => {
|
||||
expect(resolution?.executableName).toBe(exeName);
|
||||
});
|
||||
|
||||
it("blocks semantic env wrappers from allowlist/safeBins auto-resolution", () => {
|
||||
const resolution = resolveCommandResolutionFromArgv([
|
||||
"/usr/bin/env",
|
||||
"FOO=bar",
|
||||
"rg",
|
||||
"-n",
|
||||
"needle",
|
||||
]);
|
||||
expect(resolution?.policyBlocked).toBe(true);
|
||||
expect(resolution?.rawExecutable).toBe("/usr/bin/env");
|
||||
});
|
||||
|
||||
it("unwraps env wrapper with shell inner executable", () => {
|
||||
const resolution = resolveCommandResolutionFromArgv(["/usr/bin/env", "bash", "-lc", "echo hi"]);
|
||||
expect(resolution?.rawExecutable).toBe("bash");
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
import { unwrapDispatchWrappersForResolution } from "./exec-wrapper-resolution.js";
|
||||
import { resolveDispatchWrapperExecutionPlan } from "./exec-wrapper-resolution.js";
|
||||
import { expandHomePrefix } from "./home-dir.js";
|
||||
|
||||
export const DEFAULT_SAFE_BINS = ["jq", "cut", "uniq", "head", "tail", "tr", "wc"];
|
||||
@@ -10,6 +10,10 @@ export type CommandResolution = {
|
||||
rawExecutable: string;
|
||||
resolvedPath?: string;
|
||||
executableName: string;
|
||||
effectiveArgv?: string[];
|
||||
wrapperChain?: string[];
|
||||
policyBlocked?: boolean;
|
||||
blockedWrapper?: string;
|
||||
};
|
||||
|
||||
function isExecutableFile(filePath: string): boolean {
|
||||
@@ -93,7 +97,14 @@ export function resolveCommandResolution(
|
||||
}
|
||||
const resolvedPath = resolveExecutablePath(rawExecutable, cwd, env);
|
||||
const executableName = resolvedPath ? path.basename(resolvedPath) : rawExecutable;
|
||||
return { rawExecutable, resolvedPath, executableName };
|
||||
return {
|
||||
rawExecutable,
|
||||
resolvedPath,
|
||||
executableName,
|
||||
effectiveArgv: [rawExecutable],
|
||||
wrapperChain: [],
|
||||
policyBlocked: false,
|
||||
};
|
||||
}
|
||||
|
||||
export function resolveCommandResolutionFromArgv(
|
||||
@@ -101,14 +112,23 @@ export function resolveCommandResolutionFromArgv(
|
||||
cwd?: string,
|
||||
env?: NodeJS.ProcessEnv,
|
||||
): CommandResolution | null {
|
||||
const effectiveArgv = unwrapDispatchWrappersForResolution(argv);
|
||||
const plan = resolveDispatchWrapperExecutionPlan(argv);
|
||||
const effectiveArgv = plan.argv;
|
||||
const rawExecutable = effectiveArgv[0]?.trim();
|
||||
if (!rawExecutable) {
|
||||
return null;
|
||||
}
|
||||
const resolvedPath = resolveExecutablePath(rawExecutable, cwd, env);
|
||||
const executableName = resolvedPath ? path.basename(resolvedPath) : rawExecutable;
|
||||
return { rawExecutable, resolvedPath, executableName };
|
||||
return {
|
||||
rawExecutable,
|
||||
resolvedPath,
|
||||
executableName,
|
||||
effectiveArgv,
|
||||
wrapperChain: plan.wrappers,
|
||||
policyBlocked: plan.policyBlocked,
|
||||
blockedWrapper: plan.blockedWrapper,
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeMatchTarget(value: string): string {
|
||||
|
||||
@@ -363,7 +363,7 @@ function consumeLongOptionToken(
|
||||
function consumeShortOptionClusterToken(
|
||||
args: string[],
|
||||
index: number,
|
||||
raw: string,
|
||||
_raw: string,
|
||||
cluster: string,
|
||||
flags: string[],
|
||||
allowedValueFlags: ReadonlySet<string>,
|
||||
@@ -383,7 +383,7 @@ function consumeShortOptionClusterToken(
|
||||
}
|
||||
return isInvalidValueToken(args[index + 1]) ? -1 : index + 2;
|
||||
}
|
||||
return hasGlobToken(raw) ? -1 : index + 1;
|
||||
return -1;
|
||||
}
|
||||
|
||||
function consumePositionalToken(token: string, positional: string[]): boolean {
|
||||
|
||||
@@ -79,6 +79,7 @@ const NICE_OPTIONS_WITH_VALUE = new Set(["-n", "--adjustment", "--priority"]);
|
||||
const STDBUF_OPTIONS_WITH_VALUE = new Set(["-i", "--input", "-o", "--output", "-e", "--error"]);
|
||||
const TIMEOUT_FLAG_OPTIONS = new Set(["--foreground", "--preserve-status", "-v", "--verbose"]);
|
||||
const TIMEOUT_OPTIONS_WITH_VALUE = new Set(["-k", "--kill-after", "-s", "--signal"]);
|
||||
const TRANSPARENT_DISPATCH_WRAPPERS = new Set(["nice", "nohup", "stdbuf", "timeout"]);
|
||||
|
||||
type ShellWrapperKind = "posix" | "cmd" | "powershell";
|
||||
|
||||
@@ -348,6 +349,13 @@ export type DispatchWrapperUnwrapResult =
|
||||
| { kind: "blocked"; wrapper: string }
|
||||
| { kind: "unwrapped"; wrapper: string; argv: string[] };
|
||||
|
||||
export type DispatchWrapperExecutionPlan = {
|
||||
argv: string[];
|
||||
wrappers: string[];
|
||||
policyBlocked: boolean;
|
||||
blockedWrapper?: string;
|
||||
};
|
||||
|
||||
function blockDispatchWrapper(wrapper: string): DispatchWrapperUnwrapResult {
|
||||
return { kind: "blocked", wrapper };
|
||||
}
|
||||
@@ -394,15 +402,48 @@ export function unwrapDispatchWrappersForResolution(
|
||||
argv: string[],
|
||||
maxDepth = MAX_DISPATCH_WRAPPER_DEPTH,
|
||||
): string[] {
|
||||
const plan = resolveDispatchWrapperExecutionPlan(argv, maxDepth);
|
||||
return plan.argv;
|
||||
}
|
||||
|
||||
function isSemanticDispatchWrapperUsage(wrapper: string, argv: string[]): boolean {
|
||||
if (wrapper === "env") {
|
||||
return envInvocationUsesModifiers(argv);
|
||||
}
|
||||
return !TRANSPARENT_DISPATCH_WRAPPERS.has(wrapper);
|
||||
}
|
||||
|
||||
export function resolveDispatchWrapperExecutionPlan(
|
||||
argv: string[],
|
||||
maxDepth = MAX_DISPATCH_WRAPPER_DEPTH,
|
||||
): DispatchWrapperExecutionPlan {
|
||||
let current = argv;
|
||||
const wrappers: string[] = [];
|
||||
for (let depth = 0; depth < maxDepth; depth += 1) {
|
||||
const unwrap = unwrapKnownDispatchWrapperInvocation(current);
|
||||
if (unwrap.kind === "blocked") {
|
||||
return {
|
||||
argv: current,
|
||||
wrappers,
|
||||
policyBlocked: true,
|
||||
blockedWrapper: unwrap.wrapper,
|
||||
};
|
||||
}
|
||||
if (unwrap.kind !== "unwrapped" || unwrap.argv.length === 0) {
|
||||
break;
|
||||
}
|
||||
wrappers.push(unwrap.wrapper);
|
||||
if (isSemanticDispatchWrapperUsage(unwrap.wrapper, current)) {
|
||||
return {
|
||||
argv: current,
|
||||
wrappers,
|
||||
policyBlocked: true,
|
||||
blockedWrapper: unwrap.wrapper,
|
||||
};
|
||||
}
|
||||
current = unwrap.argv;
|
||||
}
|
||||
return current;
|
||||
return { argv: current, wrappers, policyBlocked: false };
|
||||
}
|
||||
|
||||
function hasEnvManipulationBeforeShellWrapperInternal(
|
||||
|
||||
Reference in New Issue
Block a user