mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:40:43 +00:00
fix(security): broaden shell-wrapper detection and block env-argv assignment injection [AI-assisted] (#65717)
* fix: address issue * fix: address PR review feedback * fix: address PR review feedback * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
6640b35298
commit
8f8492d172
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(security): broaden shell-wrapper detection and block env-argv assignment injection [AI-assisted]. (#65717) Thanks @pgondhi987.
|
||||
- Gateway/startup: defer scheduled services until sidecars finish, gate chat history and model listing during sidecar resume, and let Control UI retry startup-gated history loads so Sandbox wake resumes channels first. (#65365) Thanks @lml2468.
|
||||
- Control UI/chat: load the live gateway slash-command catalog into the composer and command palette so dock commands, plugin commands, and direct skill aliases appear in chat, while keeping trusted local commands authoritative and bounding remote command metadata. (#65620) Thanks @BunsDev.
|
||||
|
||||
|
||||
@@ -152,28 +152,97 @@ function scanWrapperInvocation(
|
||||
}
|
||||
|
||||
export function unwrapEnvInvocation(argv: string[]): string[] | null {
|
||||
return scanWrapperInvocation(argv, {
|
||||
separators: new Set(["--", "-"]),
|
||||
onToken: (token, lower) => {
|
||||
if (isEnvAssignment(token)) {
|
||||
return "continue";
|
||||
const parsed = parseEnvInvocationPrelude(argv);
|
||||
return parsed ? argv.slice(parsed.commandIndex) : null;
|
||||
}
|
||||
|
||||
type ParsedEnvInvocationPrelude = {
|
||||
assignmentKeys: string[];
|
||||
commandIndex: number;
|
||||
};
|
||||
|
||||
function parseEnvInvocationPrelude(argv: string[]): ParsedEnvInvocationPrelude | null {
|
||||
let idx = 1;
|
||||
let expectsOptionValue = false;
|
||||
const assignmentKeys: string[] = [];
|
||||
while (idx < argv.length) {
|
||||
const token = argv[idx]?.trim() ?? "";
|
||||
if (!token) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (expectsOptionValue) {
|
||||
expectsOptionValue = false;
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (token === "--" || token === "-") {
|
||||
idx += 1;
|
||||
break;
|
||||
}
|
||||
if (isEnvAssignment(token)) {
|
||||
const delimiter = token.indexOf("=");
|
||||
if (delimiter > 0) {
|
||||
assignmentKeys.push(token.slice(0, delimiter));
|
||||
}
|
||||
if (!token.startsWith("-") || token === "-") {
|
||||
return "stop";
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (!token.startsWith("-") || token === "-") {
|
||||
break;
|
||||
}
|
||||
const lower = normalizeLowercaseStringOrEmpty(token);
|
||||
const [flag] = lower.split("=", 2);
|
||||
if (ENV_FLAG_OPTIONS.has(flag)) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (ENV_OPTIONS_WITH_VALUE.has(flag)) {
|
||||
if (lower.includes("=")) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
const [flag] = lower.split("=", 2);
|
||||
if (ENV_FLAG_OPTIONS.has(flag)) {
|
||||
return "continue";
|
||||
expectsOptionValue = true;
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (hasEnvInlineValuePrefix(lower)) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
if (expectsOptionValue || idx >= argv.length) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
assignmentKeys,
|
||||
commandIndex: idx,
|
||||
};
|
||||
}
|
||||
|
||||
export function extractEnvAssignmentKeysFromDispatchWrappers(
|
||||
argv: string[],
|
||||
maxDepth = MAX_DISPATCH_WRAPPER_DEPTH,
|
||||
): string[] {
|
||||
let current = argv;
|
||||
const assignmentKeys: string[] = [];
|
||||
for (let depth = 0; depth < maxDepth; depth += 1) {
|
||||
const unwrap = unwrapKnownDispatchWrapperInvocation(current);
|
||||
if (unwrap.kind !== "unwrapped" || unwrap.argv.length === 0) {
|
||||
break;
|
||||
}
|
||||
if (unwrap.wrapper === "env") {
|
||||
const parsed = parseEnvInvocationPrelude(current);
|
||||
if (parsed) {
|
||||
assignmentKeys.push(...parsed.assignmentKeys);
|
||||
}
|
||||
if (ENV_OPTIONS_WITH_VALUE.has(flag)) {
|
||||
return lower.includes("=") ? "continue" : "consume-next";
|
||||
}
|
||||
if (hasEnvInlineValuePrefix(lower)) {
|
||||
return "continue";
|
||||
}
|
||||
return "invalid";
|
||||
},
|
||||
});
|
||||
}
|
||||
current = unwrap.argv;
|
||||
}
|
||||
return Array.from(new Set(assignmentKeys)).toSorted((a, b) => a.localeCompare(b));
|
||||
}
|
||||
|
||||
function envInvocationUsesModifiers(argv: string[]): boolean {
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
import {
|
||||
basenameLower,
|
||||
extractEnvAssignmentKeysFromDispatchWrappers,
|
||||
extractShellWrapperCommand,
|
||||
extractShellWrapperInlineCommand,
|
||||
hasEnvManipulationBeforeShellWrapper,
|
||||
isDispatchWrapperExecutable,
|
||||
isShellWrapperExecutable,
|
||||
isShellWrapperInvocation,
|
||||
normalizeExecutableToken,
|
||||
resolveDispatchWrapperTrustPlan,
|
||||
resolveShellWrapperTransportArgv,
|
||||
@@ -402,6 +404,52 @@ describe("resolveShellWrapperTransportArgv", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isShellWrapperInvocation", () => {
|
||||
test.each([
|
||||
{
|
||||
argv: ["bash", "script.sh"],
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
argv: ["/usr/bin/env", "SHELLOPTS=xtrace", "bash", "-lc", "echo hi"],
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
argv: ["busybox", "sh", "script.sh"],
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
argv: ["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"],
|
||||
expected: false,
|
||||
},
|
||||
])("detects shell-wrapper executable invocations for %j", ({ argv, expected }) => {
|
||||
expect(isShellWrapperInvocation(argv)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("extractEnvAssignmentKeysFromDispatchWrappers", () => {
|
||||
test.each([
|
||||
{
|
||||
argv: ["env", "FOO=bar", "BAR=baz", "bash", "-lc", "echo hi"],
|
||||
expected: ["BAR", "FOO"],
|
||||
},
|
||||
{
|
||||
argv: ["nice", "-n", "5", "env", "-u", "PATH", "TERM=xterm", "bash", "-lc", "echo hi"],
|
||||
expected: ["TERM"],
|
||||
},
|
||||
{
|
||||
argv: ["env", "--split-string", "FOO=bar", "bash", "-lc", "echo hi"],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
argv: ["env", "--", "bash", "-lc", "echo hi"],
|
||||
expected: [],
|
||||
},
|
||||
])("extracts env assignment prelude keys for %j", ({ argv, expected }) => {
|
||||
expect(extractEnvAssignmentKeysFromDispatchWrappers(argv)).toEqual(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("extractShellWrapperCommand", () => {
|
||||
test.each([
|
||||
{
|
||||
|
||||
@@ -1201,11 +1201,13 @@ describe("sanitizeSystemRunEnvOverrides", () => {
|
||||
TOKEN: "abc",
|
||||
LANG: "C",
|
||||
LC_ALL: "C",
|
||||
LC_TIME: "C",
|
||||
},
|
||||
});
|
||||
expect(overrides).toEqual({
|
||||
LANG: "C",
|
||||
LC_ALL: "C",
|
||||
LC_TIME: "C",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1228,11 +1230,13 @@ describe("sanitizeSystemRunEnvOverrides", () => {
|
||||
overrides: {
|
||||
lang: "C",
|
||||
ColorTerm: "truecolor",
|
||||
lc_numeric: "C",
|
||||
},
|
||||
}),
|
||||
).toEqual({
|
||||
lang: "C",
|
||||
ColorTerm: "truecolor",
|
||||
lc_numeric: "C",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,8 @@ export const HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEY_VALUES: readonly string
|
||||
"NO_COLOR",
|
||||
"FORCE_COLOR",
|
||||
]);
|
||||
export const HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_PREFIX_VALUES: readonly string[] =
|
||||
Object.freeze(["LC_"]);
|
||||
export const HOST_DANGEROUS_ENV_KEYS = new Set<string>(HOST_DANGEROUS_ENV_KEY_VALUES);
|
||||
export const HOST_DANGEROUS_INHERITED_ENV_KEYS = new Set<string>(
|
||||
HOST_DANGEROUS_INHERITED_ENV_KEY_VALUES,
|
||||
@@ -43,6 +45,20 @@ export const HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS = new Set<string>(
|
||||
HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEY_VALUES,
|
||||
);
|
||||
|
||||
function isShellWrapperAllowedOverrideEnvVarName(rawKey: string): boolean {
|
||||
const key = normalizeEnvVarKey(rawKey, { portable: true });
|
||||
if (!key) {
|
||||
return false;
|
||||
}
|
||||
const upper = key.toUpperCase();
|
||||
if (HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS.has(upper)) {
|
||||
return true;
|
||||
}
|
||||
return HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_PREFIX_VALUES.some((prefix) =>
|
||||
upper.startsWith(prefix),
|
||||
);
|
||||
}
|
||||
|
||||
export type HostExecEnvSanitizationResult = {
|
||||
env: Record<string, string>;
|
||||
rejectedOverrideBlockedKeys: string[];
|
||||
@@ -254,7 +270,7 @@ export function sanitizeSystemRunEnvOverrides(params?: {
|
||||
}
|
||||
const filtered: Record<string, string> = {};
|
||||
for (const [key, value] of listNormalizedEnvEntries(overrides, { portable: true })) {
|
||||
if (!HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS.has(key.toUpperCase())) {
|
||||
if (!isShellWrapperAllowedOverrideEnvVarName(key)) {
|
||||
continue;
|
||||
}
|
||||
filtered[key] = value;
|
||||
|
||||
@@ -107,6 +107,38 @@ export function isShellWrapperExecutable(token: string): boolean {
|
||||
return SHELL_WRAPPER_CANONICAL.has(normalizeExecutableToken(token));
|
||||
}
|
||||
|
||||
function isShellWrapperInvocationInternal(argv: string[], depth: number): boolean {
|
||||
if (!isWithinDispatchClassificationDepth(depth)) {
|
||||
return false;
|
||||
}
|
||||
const token0 = argv[0]?.trim();
|
||||
if (!token0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv);
|
||||
if (dispatchUnwrap.kind === "blocked") {
|
||||
return false;
|
||||
}
|
||||
if (dispatchUnwrap.kind === "unwrapped") {
|
||||
return isShellWrapperInvocationInternal(dispatchUnwrap.argv, depth + 1);
|
||||
}
|
||||
|
||||
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv);
|
||||
if (shellMultiplexerUnwrap.kind === "blocked") {
|
||||
return false;
|
||||
}
|
||||
if (shellMultiplexerUnwrap.kind === "unwrapped") {
|
||||
return isShellWrapperInvocationInternal(shellMultiplexerUnwrap.argv, depth + 1);
|
||||
}
|
||||
|
||||
return isShellWrapperExecutable(token0);
|
||||
}
|
||||
|
||||
export function isShellWrapperInvocation(argv: string[]): boolean {
|
||||
return isShellWrapperInvocationInternal(argv, 0);
|
||||
}
|
||||
|
||||
function normalizeRawCommand(rawCommand?: string | null): string | null {
|
||||
const trimmed = rawCommand?.trim() ?? "";
|
||||
return trimmed.length > 0 ? trimmed : null;
|
||||
|
||||
@@ -147,6 +147,16 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
expect(denied.errorMessage).toContain("Windows shell wrappers like cmd.exe /c");
|
||||
});
|
||||
|
||||
it("does not block Windows cmd.exe invocations without inline shell-wrapper transport", () => {
|
||||
const allowed = expectAllowedDecision(
|
||||
evaluateSystemRunPolicy(
|
||||
buildPolicyParams({ isWindows: true, cmdInvocation: true, shellWrapperInvocation: false }),
|
||||
),
|
||||
);
|
||||
expect(allowed.shellWrapperBlocked).toBe(false);
|
||||
expect(allowed.windowsShellWrapperBlocked).toBe(false);
|
||||
});
|
||||
|
||||
it("allows execution when policy checks pass", () => {
|
||||
const allowed = expectAllowedDecision(
|
||||
evaluateSystemRunPolicy(buildPolicyParams({ ask: "on-miss" })),
|
||||
|
||||
@@ -384,6 +384,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
sendNodeEvent?: HandleSystemRunInvokeOptions["sendNodeEvent"];
|
||||
skillBinsCurrent?: () => Promise<Array<{ name: string; resolvedPath: string }>>;
|
||||
isCmdExeInvocation?: HandleSystemRunInvokeOptions["isCmdExeInvocation"];
|
||||
sanitizeEnv?: HandleSystemRunInvokeOptions["sanitizeEnv"];
|
||||
}): Promise<{
|
||||
runCommand: MockedRunCommand;
|
||||
runViaMacAppExecHost: MockedRunViaMacAppExecHost;
|
||||
@@ -443,7 +444,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
resolveExecSecurity: () => params.security ?? "full",
|
||||
resolveExecAsk: () => params.ask ?? "off",
|
||||
isCmdExeInvocation: params.isCmdExeInvocation ?? (() => false),
|
||||
sanitizeEnv: () => undefined,
|
||||
sanitizeEnv: params.sanitizeEnv ?? (() => undefined),
|
||||
runCommand,
|
||||
runViaMacAppExecHost,
|
||||
sendNodeEvent,
|
||||
@@ -1186,6 +1187,49 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("applies shell-wrapper env allowlist for shell executable commands without inline payload", async () => {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
command: ["/bin/sh", "./script.sh"],
|
||||
env: {
|
||||
OPENCLAW_TEST: "1",
|
||||
LANG: "C",
|
||||
LC_TIME: "C",
|
||||
},
|
||||
sanitizeEnv: (overrides) => overrides ?? undefined,
|
||||
});
|
||||
|
||||
expect(runCommand).toHaveBeenCalledTimes(1);
|
||||
const passedEnv = runCommand.mock.calls[0]?.[2];
|
||||
expect(passedEnv).toEqual({
|
||||
LANG: "C",
|
||||
LC_TIME: "C",
|
||||
});
|
||||
expectInvokeOk(sendInvokeResult);
|
||||
});
|
||||
|
||||
it("rejects blocked env assignment keys embedded in command argv", async () => {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
command: ["/usr/bin/env", "SHELLOPTS=xtrace", "PS4=$(id)", "bash", "-lc", "echo ok"],
|
||||
});
|
||||
|
||||
expect(runCommand).not.toHaveBeenCalled();
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "SYSTEM_RUN_DENIED: command env assignment rejected",
|
||||
});
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "SHELLOPTS",
|
||||
});
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "PS4",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects invalid non-portable environment override keys before execution", async () => {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
|
||||
@@ -20,7 +20,11 @@ import {
|
||||
detectInterpreterInlineEvalArgv,
|
||||
} from "../infra/exec-inline-eval.js";
|
||||
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { resolveShellWrapperTransportArgv } from "../infra/exec-wrapper-resolution.js";
|
||||
import {
|
||||
extractEnvAssignmentKeysFromDispatchWrappers,
|
||||
isShellWrapperInvocation,
|
||||
resolveShellWrapperTransportArgv,
|
||||
} from "../infra/exec-wrapper-resolution.js";
|
||||
import {
|
||||
inspectHostExecEnvOverrides,
|
||||
sanitizeSystemRunEnvOverrides,
|
||||
@@ -78,6 +82,7 @@ type ResolvedExecApprovals = ReturnType<typeof resolveExecApprovals>;
|
||||
type SystemRunParsePhase = {
|
||||
argv: string[];
|
||||
shellPayload: string | null;
|
||||
shellWrapperInvocation: boolean;
|
||||
commandText: string;
|
||||
commandPreview: string | null;
|
||||
approvalPlan: import("../infra/exec-approvals.js").SystemRunApprovalPlan | null;
|
||||
@@ -242,6 +247,7 @@ async function parseSystemRunPhase(
|
||||
}
|
||||
|
||||
const shellPayload = command.shellPayload;
|
||||
const shellWrapperInvocation = isShellWrapperInvocation(command.argv);
|
||||
const commandText = command.commandText;
|
||||
const approvalPlan =
|
||||
opts.params.systemRunPlan === undefined
|
||||
@@ -258,6 +264,27 @@ async function parseSystemRunPhase(
|
||||
const sessionKey = normalizeOptionalString(opts.params.sessionKey) ?? "node";
|
||||
const runId = normalizeOptionalString(opts.params.runId) ?? crypto.randomUUID();
|
||||
const suppressNotifyOnExit = opts.params.suppressNotifyOnExit === true;
|
||||
const envAssignmentKeys = extractEnvAssignmentKeysFromDispatchWrappers(command.argv);
|
||||
const envAssignmentOverrides =
|
||||
envAssignmentKeys.length > 0
|
||||
? Object.fromEntries(envAssignmentKeys.map((key) => [key, "1"]))
|
||||
: undefined;
|
||||
const envAssignmentDiagnostics = inspectHostExecEnvOverrides({
|
||||
overrides: envAssignmentOverrides,
|
||||
blockPathOverrides: true,
|
||||
});
|
||||
// `extractEnvAssignmentKeysFromDispatchWrappers` only emits keys that satisfy
|
||||
// `isEnvAssignment` and therefore portable env-key syntax by construction.
|
||||
if (envAssignmentDiagnostics.rejectedOverrideBlockedKeys.length > 0) {
|
||||
await opts.sendInvokeResult({
|
||||
ok: false,
|
||||
error: {
|
||||
code: "INVALID_REQUEST",
|
||||
message: `SYSTEM_RUN_DENIED: command env assignment rejected (blocked env assignment keys: ${envAssignmentDiagnostics.rejectedOverrideBlockedKeys.join(", ")})`,
|
||||
},
|
||||
});
|
||||
return null;
|
||||
}
|
||||
const envOverrideDiagnostics = inspectHostExecEnvOverrides({
|
||||
overrides: opts.params.env ?? undefined,
|
||||
blockPathOverrides: true,
|
||||
@@ -288,11 +315,12 @@ async function parseSystemRunPhase(
|
||||
}
|
||||
const envOverrides = sanitizeSystemRunEnvOverrides({
|
||||
overrides: opts.params.env ?? undefined,
|
||||
shellWrapper: shellPayload !== null,
|
||||
shellWrapper: shellWrapperInvocation,
|
||||
});
|
||||
return {
|
||||
argv: command.argv,
|
||||
shellPayload,
|
||||
shellWrapperInvocation,
|
||||
commandText,
|
||||
commandPreview: command.previewText,
|
||||
approvalPlan,
|
||||
@@ -384,6 +412,8 @@ async function evaluateSystemRunPolicyPhase(
|
||||
approved: parsed.approved,
|
||||
isWindows,
|
||||
cmdInvocation,
|
||||
// Keep cmd.exe approval gating scoped to inline shell-wrapper transport.
|
||||
// Env sanitization uses broader shell-wrapper detection in parse phase.
|
||||
shellWrapperInvocation: parsed.shellPayload !== null,
|
||||
});
|
||||
analysisOk = policy.analysisOk;
|
||||
|
||||
Reference in New Issue
Block a user