fix: harden Windows exec allowlist
parent
8f3bfbd1c4
commit
a7f4a53ce8
|
|
@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai
|
||||||
- Security: enforce access-group gating for Slack slash commands when channel type lookup fails.
|
- Security: enforce access-group gating for Slack slash commands when channel type lookup fails.
|
||||||
- Security: require validated shared-secret auth before skipping device identity on gateway connect.
|
- Security: require validated shared-secret auth before skipping device identity on gateway connect.
|
||||||
- Security: guard skill installer downloads with SSRF checks (block private/localhost URLs).
|
- Security: guard skill installer downloads with SSRF checks (block private/localhost URLs).
|
||||||
|
- Security: harden Windows exec allowlist; block cmd.exe bypass via single &. Thanks @simecek.
|
||||||
- Media understanding: apply SSRF guardrails to provider fetches; allow private baseUrl overrides explicitly.
|
- Media understanding: apply SSRF guardrails to provider fetches; allow private baseUrl overrides explicitly.
|
||||||
- Tests: stub SSRF DNS pinning in web auto-reply + Gemini video coverage. (#6619) Thanks @joshp123.
|
- Tests: stub SSRF DNS pinning in web auto-reply + Gemini video coverage. (#6619) Thanks @joshp123.
|
||||||
- fix(voice-call): harden inbound allowlist; reject anonymous callers; require Telnyx publicKey for allowlist; token-gate Twilio media streams; cap webhook body size (thanks @simecek)
|
- fix(voice-call): harden inbound allowlist; reject anonymous callers; require Telnyx publicKey for allowlist; token-gate Twilio media streams; cap webhook body size (thanks @simecek)
|
||||||
|
|
|
||||||
|
|
@ -1046,6 +1046,7 @@ export function createExecTool(
|
||||||
safeBins: new Set(),
|
safeBins: new Set(),
|
||||||
cwd: workdir,
|
cwd: workdir,
|
||||||
env,
|
env,
|
||||||
|
platform: nodeInfo?.platform,
|
||||||
});
|
});
|
||||||
let analysisOk = baseAllowlistEval.analysisOk;
|
let analysisOk = baseAllowlistEval.analysisOk;
|
||||||
let allowlistSatisfied = false;
|
let allowlistSatisfied = false;
|
||||||
|
|
@ -1073,6 +1074,7 @@ export function createExecTool(
|
||||||
safeBins: new Set(),
|
safeBins: new Set(),
|
||||||
cwd: workdir,
|
cwd: workdir,
|
||||||
env,
|
env,
|
||||||
|
platform: nodeInfo?.platform,
|
||||||
});
|
});
|
||||||
allowlistSatisfied = allowlistEval.allowlistSatisfied;
|
allowlistSatisfied = allowlistEval.allowlistSatisfied;
|
||||||
analysisOk = allowlistEval.analysisOk;
|
analysisOk = allowlistEval.analysisOk;
|
||||||
|
|
@ -1282,6 +1284,7 @@ export function createExecTool(
|
||||||
safeBins,
|
safeBins,
|
||||||
cwd: workdir,
|
cwd: workdir,
|
||||||
env,
|
env,
|
||||||
|
platform: process.platform,
|
||||||
});
|
});
|
||||||
const allowlistMatches = allowlistEval.allowlistMatches;
|
const allowlistMatches = allowlistEval.allowlistMatches;
|
||||||
const analysisOk = allowlistEval.analysisOk;
|
const analysisOk = allowlistEval.analysisOk;
|
||||||
|
|
|
||||||
|
|
@ -161,6 +161,24 @@ describe("exec approvals shell parsing", () => {
|
||||||
expect(res.ok).toBe(true);
|
expect(res.ok).toBe(true);
|
||||||
expect(res.segments[0]?.argv[0]).toBe("echo");
|
expect(res.segments[0]?.argv[0]).toBe("echo");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rejects windows shell metacharacters", () => {
|
||||||
|
const res = analyzeShellCommand({
|
||||||
|
command: "ping 127.0.0.1 -n 1 & whoami",
|
||||||
|
platform: "win32",
|
||||||
|
});
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
expect(res.reason).toBe("unsupported windows shell token: &");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("parses windows quoted executables", () => {
|
||||||
|
const res = analyzeShellCommand({
|
||||||
|
command: '"C:\\Program Files\\Tool\\tool.exe" --version',
|
||||||
|
platform: "win32",
|
||||||
|
});
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
expect(res.segments[0]?.argv).toEqual(["C:\\Program Files\\Tool\\tool.exe", "--version"]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("exec approvals shell allowlist (chained commands)", () => {
|
describe("exec approvals shell allowlist (chained commands)", () => {
|
||||||
|
|
@ -227,6 +245,19 @@ describe("exec approvals shell allowlist (chained commands)", () => {
|
||||||
expect(result.analysisOk).toBe(true);
|
expect(result.analysisOk).toBe(true);
|
||||||
expect(result.allowlistSatisfied).toBe(true);
|
expect(result.allowlistSatisfied).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rejects windows chain separators for allowlist analysis", () => {
|
||||||
|
const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/ping" }];
|
||||||
|
const result = evaluateShellAllowlist({
|
||||||
|
command: "ping 127.0.0.1 -n 1 & whoami",
|
||||||
|
allowlist,
|
||||||
|
safeBins: new Set(),
|
||||||
|
cwd: "/tmp",
|
||||||
|
platform: "win32",
|
||||||
|
});
|
||||||
|
expect(result.analysisOk).toBe(false);
|
||||||
|
expect(result.allowlistSatisfied).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("exec approvals safe bins", () => {
|
describe("exec approvals safe bins", () => {
|
||||||
|
|
|
||||||
|
|
@ -586,6 +586,19 @@ export type ExecCommandAnalysis = {
|
||||||
|
|
||||||
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
|
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
|
||||||
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
|
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
|
||||||
|
const WINDOWS_UNSUPPORTED_TOKENS = new Set([
|
||||||
|
"&",
|
||||||
|
"|",
|
||||||
|
"<",
|
||||||
|
">",
|
||||||
|
"^",
|
||||||
|
"(",
|
||||||
|
")",
|
||||||
|
"%",
|
||||||
|
"!",
|
||||||
|
"\n",
|
||||||
|
"\r",
|
||||||
|
]);
|
||||||
|
|
||||||
function isDoubleQuoteEscape(next: string | undefined): next is string {
|
function isDoubleQuoteEscape(next: string | undefined): next is string {
|
||||||
return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next));
|
return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next));
|
||||||
|
|
@ -735,6 +748,86 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se
|
||||||
return { ok: true, segments: result.parts };
|
return { ok: true, segments: result.parts };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function findWindowsUnsupportedToken(command: string): string | null {
|
||||||
|
for (const ch of command) {
|
||||||
|
if (WINDOWS_UNSUPPORTED_TOKENS.has(ch)) {
|
||||||
|
if (ch === "\n" || ch === "\r") {
|
||||||
|
return "newline";
|
||||||
|
}
|
||||||
|
return ch;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function tokenizeWindowsSegment(segment: string): string[] | null {
|
||||||
|
const tokens: string[] = [];
|
||||||
|
let buf = "";
|
||||||
|
let inDouble = false;
|
||||||
|
|
||||||
|
const pushToken = () => {
|
||||||
|
if (buf.length > 0) {
|
||||||
|
tokens.push(buf);
|
||||||
|
buf = "";
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
for (let i = 0; i < segment.length; i += 1) {
|
||||||
|
const ch = segment[i];
|
||||||
|
if (ch === '"') {
|
||||||
|
inDouble = !inDouble;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (!inDouble && /\s/.test(ch)) {
|
||||||
|
pushToken();
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
buf += ch;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (inDouble) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
pushToken();
|
||||||
|
return tokens.length > 0 ? tokens : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function analyzeWindowsShellCommand(params: {
|
||||||
|
command: string;
|
||||||
|
cwd?: string;
|
||||||
|
env?: NodeJS.ProcessEnv;
|
||||||
|
}): ExecCommandAnalysis {
|
||||||
|
const unsupported = findWindowsUnsupportedToken(params.command);
|
||||||
|
if (unsupported) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: `unsupported windows shell token: ${unsupported}`,
|
||||||
|
segments: [],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
const argv = tokenizeWindowsSegment(params.command);
|
||||||
|
if (!argv || argv.length === 0) {
|
||||||
|
return { ok: false, reason: "unable to parse windows command", segments: [] };
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
ok: true,
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: params.command,
|
||||||
|
argv,
|
||||||
|
resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function isWindowsPlatform(platform?: string | null): boolean {
|
||||||
|
const normalized = String(platform ?? "")
|
||||||
|
.trim()
|
||||||
|
.toLowerCase();
|
||||||
|
return normalized.startsWith("win");
|
||||||
|
}
|
||||||
|
|
||||||
function tokenizeShellSegment(segment: string): string[] | null {
|
function tokenizeShellSegment(segment: string): string[] | null {
|
||||||
const tokens: string[] = [];
|
const tokens: string[] = [];
|
||||||
let buf = "";
|
let buf = "";
|
||||||
|
|
@ -828,7 +921,11 @@ export function analyzeShellCommand(params: {
|
||||||
command: string;
|
command: string;
|
||||||
cwd?: string;
|
cwd?: string;
|
||||||
env?: NodeJS.ProcessEnv;
|
env?: NodeJS.ProcessEnv;
|
||||||
|
platform?: string | null;
|
||||||
}): ExecCommandAnalysis {
|
}): ExecCommandAnalysis {
|
||||||
|
if (isWindowsPlatform(params.platform)) {
|
||||||
|
return analyzeWindowsShellCommand(params);
|
||||||
|
}
|
||||||
// First try splitting by chain operators (&&, ||, ;)
|
// First try splitting by chain operators (&&, ||, ;)
|
||||||
const chainParts = splitCommandChain(params.command);
|
const chainParts = splitCommandChain(params.command);
|
||||||
if (chainParts) {
|
if (chainParts) {
|
||||||
|
|
@ -1190,13 +1287,15 @@ export function evaluateShellAllowlist(params: {
|
||||||
env?: NodeJS.ProcessEnv;
|
env?: NodeJS.ProcessEnv;
|
||||||
skillBins?: Set<string>;
|
skillBins?: Set<string>;
|
||||||
autoAllowSkills?: boolean;
|
autoAllowSkills?: boolean;
|
||||||
|
platform?: string | null;
|
||||||
}): ExecAllowlistAnalysis {
|
}): ExecAllowlistAnalysis {
|
||||||
const chainParts = splitCommandChain(params.command);
|
const chainParts = isWindowsPlatform(params.platform) ? null : splitCommandChain(params.command);
|
||||||
if (!chainParts) {
|
if (!chainParts) {
|
||||||
const analysis = analyzeShellCommand({
|
const analysis = analyzeShellCommand({
|
||||||
command: params.command,
|
command: params.command,
|
||||||
cwd: params.cwd,
|
cwd: params.cwd,
|
||||||
env: params.env,
|
env: params.env,
|
||||||
|
platform: params.platform,
|
||||||
});
|
});
|
||||||
if (!analysis.ok) {
|
if (!analysis.ok) {
|
||||||
return {
|
return {
|
||||||
|
|
@ -1230,6 +1329,7 @@ export function evaluateShellAllowlist(params: {
|
||||||
command: part,
|
command: part,
|
||||||
cwd: params.cwd,
|
cwd: params.cwd,
|
||||||
env: params.env,
|
env: params.env,
|
||||||
|
platform: params.platform,
|
||||||
});
|
});
|
||||||
if (!analysis.ok) {
|
if (!analysis.ok) {
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -119,6 +119,15 @@ function resolveExecSecurity(value?: string): ExecSecurity {
|
||||||
return value === "deny" || value === "allowlist" || value === "full" ? value : "allowlist";
|
return value === "deny" || value === "allowlist" || value === "full" ? value : "allowlist";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isCmdExeInvocation(argv: string[]): boolean {
|
||||||
|
const token = argv[0]?.trim();
|
||||||
|
if (!token) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const base = path.win32.basename(token).toLowerCase();
|
||||||
|
return base === "cmd.exe" || base === "cmd";
|
||||||
|
}
|
||||||
|
|
||||||
function resolveExecAsk(value?: string): ExecAsk {
|
function resolveExecAsk(value?: string): ExecAsk {
|
||||||
return value === "off" || value === "on-miss" || value === "always" ? value : "on-miss";
|
return value === "off" || value === "on-miss" || value === "always" ? value : "on-miss";
|
||||||
}
|
}
|
||||||
|
|
@ -906,6 +915,7 @@ async function handleInvoke(
|
||||||
env,
|
env,
|
||||||
skillBins: bins,
|
skillBins: bins,
|
||||||
autoAllowSkills,
|
autoAllowSkills,
|
||||||
|
platform: process.platform,
|
||||||
});
|
});
|
||||||
analysisOk = allowlistEval.analysisOk;
|
analysisOk = allowlistEval.analysisOk;
|
||||||
allowlistMatches = allowlistEval.allowlistMatches;
|
allowlistMatches = allowlistEval.allowlistMatches;
|
||||||
|
|
@ -928,6 +938,14 @@ async function handleInvoke(
|
||||||
security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false;
|
security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false;
|
||||||
segments = analysis.segments;
|
segments = analysis.segments;
|
||||||
}
|
}
|
||||||
|
const isWindows = process.platform === "win32";
|
||||||
|
const cmdInvocation = rawCommand
|
||||||
|
? isCmdExeInvocation(segments[0]?.argv ?? [])
|
||||||
|
: isCmdExeInvocation(argv);
|
||||||
|
if (security === "allowlist" && isWindows && cmdInvocation) {
|
||||||
|
analysisOk = false;
|
||||||
|
allowlistSatisfied = false;
|
||||||
|
}
|
||||||
|
|
||||||
const useMacAppExec = process.platform === "darwin";
|
const useMacAppExec = process.platform === "darwin";
|
||||||
if (useMacAppExec) {
|
if (useMacAppExec) {
|
||||||
|
|
@ -1127,8 +1145,23 @@ async function handleInvoke(
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let execArgv = argv;
|
||||||
|
if (
|
||||||
|
security === "allowlist" &&
|
||||||
|
isWindows &&
|
||||||
|
!approvedByAsk &&
|
||||||
|
rawCommand &&
|
||||||
|
analysisOk &&
|
||||||
|
allowlistSatisfied &&
|
||||||
|
segments.length === 1 &&
|
||||||
|
segments[0]?.argv.length > 0
|
||||||
|
) {
|
||||||
|
// Avoid cmd.exe in allowlist mode on Windows; run the parsed argv directly.
|
||||||
|
execArgv = segments[0].argv;
|
||||||
|
}
|
||||||
|
|
||||||
const result = await runCommand(
|
const result = await runCommand(
|
||||||
argv,
|
execArgv,
|
||||||
params.cwd?.trim() || undefined,
|
params.cwd?.trim() || undefined,
|
||||||
env,
|
env,
|
||||||
params.timeoutMs ?? undefined,
|
params.timeoutMs ?? undefined,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue