mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 15:58:27 +00:00
Security/Exec: persist inner commands for shell-wrapper approvals
This commit is contained in:
@@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting.
|
- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting.
|
||||||
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
|
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
|
||||||
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.
|
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.
|
||||||
|
- Security/Exec approvals: when users choose `allow-always` for shell-wrapper commands (for example `/bin/zsh -lc ...`), persist allowlist patterns for the inner executable(s) instead of the wrapper shell binary, preventing accidental broad shell allowlisting in moderate mode. (#23276) Thanks @xrom2863.
|
||||||
- Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting.
|
- Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||||
- Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), and centralize range checks into a single CIDR policy table to reduce classifier drift.
|
- Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), and centralize range checks into a single CIDR policy table to reduce classifier drift.
|
||||||
- Security/Archive: block zip symlink escapes during archive extraction.
|
- Security/Archive: block zip symlink escapes during archive extraction.
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import {
|
|||||||
minSecurity,
|
minSecurity,
|
||||||
recordAllowlistUse,
|
recordAllowlistUse,
|
||||||
requiresExecApproval,
|
requiresExecApproval,
|
||||||
|
resolveAllowAlwaysPatterns,
|
||||||
resolveExecApprovals,
|
resolveExecApprovals,
|
||||||
} from "../infra/exec-approvals.js";
|
} from "../infra/exec-approvals.js";
|
||||||
import { markBackgrounded, tail } from "./bash-process-registry.js";
|
import { markBackgrounded, tail } from "./bash-process-registry.js";
|
||||||
@@ -153,8 +154,13 @@ export async function processGatewayAllowlist(
|
|||||||
} else if (decision === "allow-always") {
|
} else if (decision === "allow-always") {
|
||||||
approvedByAsk = true;
|
approvedByAsk = true;
|
||||||
if (hostSecurity === "allowlist") {
|
if (hostSecurity === "allowlist") {
|
||||||
for (const segment of allowlistEval.segments) {
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
const pattern = segment.resolution?.resolvedPath ?? "";
|
segments: allowlistEval.segments,
|
||||||
|
cwd: params.workdir,
|
||||||
|
env: params.env,
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
for (const pattern of patterns) {
|
||||||
if (pattern) {
|
if (pattern) {
|
||||||
addAllowlistEntry(approvals.file, params.agentId, pattern);
|
addAllowlistEntry(approvals.file, params.agentId, pattern);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -205,6 +205,148 @@ export type ExecAllowlistAnalysis = {
|
|||||||
segmentSatisfiedBy: ExecSegmentSatisfiedBy[];
|
segmentSatisfiedBy: ExecSegmentSatisfiedBy[];
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const SHELL_WRAPPER_EXECUTABLES = new Set([
|
||||||
|
"ash",
|
||||||
|
"bash",
|
||||||
|
"cmd",
|
||||||
|
"cmd.exe",
|
||||||
|
"dash",
|
||||||
|
"fish",
|
||||||
|
"ksh",
|
||||||
|
"powershell",
|
||||||
|
"powershell.exe",
|
||||||
|
"pwsh",
|
||||||
|
"pwsh.exe",
|
||||||
|
"sh",
|
||||||
|
"zsh",
|
||||||
|
]);
|
||||||
|
|
||||||
|
function normalizeExecutableName(name: string | undefined): string {
|
||||||
|
return (name ?? "").trim().toLowerCase();
|
||||||
|
}
|
||||||
|
|
||||||
|
function isShellWrapperSegment(segment: ExecCommandSegment): boolean {
|
||||||
|
const candidates = [
|
||||||
|
normalizeExecutableName(segment.resolution?.executableName),
|
||||||
|
normalizeExecutableName(segment.resolution?.rawExecutable),
|
||||||
|
];
|
||||||
|
for (const candidate of candidates) {
|
||||||
|
if (!candidate) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (SHELL_WRAPPER_EXECUTABLES.has(candidate)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
const base = candidate.split(/[\\/]/).pop();
|
||||||
|
if (base && SHELL_WRAPPER_EXECUTABLES.has(base)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractShellInlineCommand(argv: string[]): string | null {
|
||||||
|
for (let i = 1; i < argv.length; i += 1) {
|
||||||
|
const token = argv[i];
|
||||||
|
if (!token) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const lower = token.toLowerCase();
|
||||||
|
if (lower === "--") {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (
|
||||||
|
lower === "-c" ||
|
||||||
|
lower === "--command" ||
|
||||||
|
lower === "-command" ||
|
||||||
|
lower === "/c" ||
|
||||||
|
lower === "/k"
|
||||||
|
) {
|
||||||
|
const next = argv[i + 1]?.trim();
|
||||||
|
return next ? next : null;
|
||||||
|
}
|
||||||
|
if (/^-[^-]*c[^-]*$/i.test(token)) {
|
||||||
|
const commandIndex = lower.indexOf("c");
|
||||||
|
const inline = token.slice(commandIndex + 1).trim();
|
||||||
|
if (inline) {
|
||||||
|
return inline;
|
||||||
|
}
|
||||||
|
const next = argv[i + 1]?.trim();
|
||||||
|
return next ? next : null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function collectAllowAlwaysPatterns(params: {
|
||||||
|
segment: ExecCommandSegment;
|
||||||
|
cwd?: string;
|
||||||
|
env?: NodeJS.ProcessEnv;
|
||||||
|
platform?: string | null;
|
||||||
|
depth: number;
|
||||||
|
out: Set<string>;
|
||||||
|
}) {
|
||||||
|
const candidatePath = resolveAllowlistCandidatePath(params.segment.resolution, params.cwd);
|
||||||
|
if (!candidatePath) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!isShellWrapperSegment(params.segment)) {
|
||||||
|
params.out.add(candidatePath);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (params.depth >= 3) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const inlineCommand = extractShellInlineCommand(params.segment.argv);
|
||||||
|
if (!inlineCommand) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const nested = analyzeShellCommand({
|
||||||
|
command: inlineCommand,
|
||||||
|
cwd: params.cwd,
|
||||||
|
env: params.env,
|
||||||
|
platform: params.platform,
|
||||||
|
});
|
||||||
|
if (!nested.ok) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
for (const nestedSegment of nested.segments) {
|
||||||
|
collectAllowAlwaysPatterns({
|
||||||
|
segment: nestedSegment,
|
||||||
|
cwd: params.cwd,
|
||||||
|
env: params.env,
|
||||||
|
platform: params.platform,
|
||||||
|
depth: params.depth + 1,
|
||||||
|
out: params.out,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Derive persisted allowlist patterns for an "allow always" decision.
|
||||||
|
* When a command is wrapped in a shell (for example `zsh -lc "<cmd>"`),
|
||||||
|
* persist the inner executable(s) rather than the shell binary.
|
||||||
|
*/
|
||||||
|
export function resolveAllowAlwaysPatterns(params: {
|
||||||
|
segments: ExecCommandSegment[];
|
||||||
|
cwd?: string;
|
||||||
|
env?: NodeJS.ProcessEnv;
|
||||||
|
platform?: string | null;
|
||||||
|
}): string[] {
|
||||||
|
const patterns = new Set<string>();
|
||||||
|
for (const segment of params.segments) {
|
||||||
|
collectAllowAlwaysPatterns({
|
||||||
|
segment,
|
||||||
|
cwd: params.cwd,
|
||||||
|
env: params.env,
|
||||||
|
platform: params.platform,
|
||||||
|
depth: 0,
|
||||||
|
out: patterns,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return Array.from(patterns);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata.
|
* Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import {
|
|||||||
normalizeSafeBins,
|
normalizeSafeBins,
|
||||||
requiresExecApproval,
|
requiresExecApproval,
|
||||||
resolveCommandResolution,
|
resolveCommandResolution,
|
||||||
|
resolveAllowAlwaysPatterns,
|
||||||
resolveExecApprovals,
|
resolveExecApprovals,
|
||||||
resolveExecApprovalsFromFile,
|
resolveExecApprovalsFromFile,
|
||||||
resolveExecApprovalsPath,
|
resolveExecApprovalsPath,
|
||||||
@@ -1214,3 +1215,122 @@ describe("normalizeExecApprovals handles string allowlist entries (#9790)", () =
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("resolveAllowAlwaysPatterns", () => {
|
||||||
|
function makeExecutable(dir: string, name: string): string {
|
||||||
|
const fileName = process.platform === "win32" ? `${name}.exe` : name;
|
||||||
|
const exe = path.join(dir, fileName);
|
||||||
|
fs.writeFileSync(exe, "");
|
||||||
|
fs.chmodSync(exe, 0o755);
|
||||||
|
return exe;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("returns direct executable paths for non-shell segments", () => {
|
||||||
|
const exe = path.join("/tmp", "openclaw-tool");
|
||||||
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: exe,
|
||||||
|
argv: [exe],
|
||||||
|
resolution: { rawExecutable: exe, resolvedPath: exe, executableName: "openclaw-tool" },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(patterns).toEqual([exe]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("unwraps shell wrappers and persists the inner executable instead", () => {
|
||||||
|
if (process.platform === "win32") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const dir = makeTempDir();
|
||||||
|
const whoami = makeExecutable(dir, "whoami");
|
||||||
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: "/bin/zsh -lc 'whoami'",
|
||||||
|
argv: ["/bin/zsh", "-lc", "whoami"],
|
||||||
|
resolution: {
|
||||||
|
rawExecutable: "/bin/zsh",
|
||||||
|
resolvedPath: "/bin/zsh",
|
||||||
|
executableName: "zsh",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
cwd: dir,
|
||||||
|
env: makePathEnv(dir),
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
expect(patterns).toEqual([whoami]);
|
||||||
|
expect(patterns).not.toContain("/bin/zsh");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("extracts all inner binaries from shell chains and deduplicates", () => {
|
||||||
|
if (process.platform === "win32") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const dir = makeTempDir();
|
||||||
|
const whoami = makeExecutable(dir, "whoami");
|
||||||
|
const ls = makeExecutable(dir, "ls");
|
||||||
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: "/bin/zsh -lc 'whoami && ls && whoami'",
|
||||||
|
argv: ["/bin/zsh", "-lc", "whoami && ls && whoami"],
|
||||||
|
resolution: {
|
||||||
|
rawExecutable: "/bin/zsh",
|
||||||
|
resolvedPath: "/bin/zsh",
|
||||||
|
executableName: "zsh",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
cwd: dir,
|
||||||
|
env: makePathEnv(dir),
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
expect(new Set(patterns)).toEqual(new Set([whoami, ls]));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not persist broad shell binaries when no inner command can be derived", () => {
|
||||||
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: "/bin/zsh -s",
|
||||||
|
argv: ["/bin/zsh", "-s"],
|
||||||
|
resolution: {
|
||||||
|
rawExecutable: "/bin/zsh",
|
||||||
|
resolvedPath: "/bin/zsh",
|
||||||
|
executableName: "zsh",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
expect(patterns).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("detects shell wrappers even when unresolved executableName is a full path", () => {
|
||||||
|
if (process.platform === "win32") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const dir = makeTempDir();
|
||||||
|
const whoami = makeExecutable(dir, "whoami");
|
||||||
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
|
segments: [
|
||||||
|
{
|
||||||
|
raw: "/usr/local/bin/zsh -lc whoami",
|
||||||
|
argv: ["/usr/local/bin/zsh", "-lc", "whoami"],
|
||||||
|
resolution: {
|
||||||
|
rawExecutable: "/usr/local/bin/zsh",
|
||||||
|
resolvedPath: undefined,
|
||||||
|
executableName: "/usr/local/bin/zsh",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
cwd: dir,
|
||||||
|
env: makePathEnv(dir),
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
expect(patterns).toEqual([whoami]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import {
|
|||||||
evaluateShellAllowlist,
|
evaluateShellAllowlist,
|
||||||
recordAllowlistUse,
|
recordAllowlistUse,
|
||||||
requiresExecApproval,
|
requiresExecApproval,
|
||||||
|
resolveAllowAlwaysPatterns,
|
||||||
resolveExecApprovals,
|
resolveExecApprovals,
|
||||||
resolveSafeBins,
|
resolveSafeBins,
|
||||||
type ExecAllowlistEntry,
|
type ExecAllowlistEntry,
|
||||||
@@ -314,8 +315,13 @@ export async function handleSystemRunInvoke(opts: {
|
|||||||
}
|
}
|
||||||
if (approvalDecision === "allow-always" && security === "allowlist") {
|
if (approvalDecision === "allow-always" && security === "allowlist") {
|
||||||
if (analysisOk) {
|
if (analysisOk) {
|
||||||
for (const segment of segments) {
|
const patterns = resolveAllowAlwaysPatterns({
|
||||||
const pattern = segment.resolution?.resolvedPath ?? "";
|
segments,
|
||||||
|
cwd: opts.params.cwd ?? undefined,
|
||||||
|
env,
|
||||||
|
platform: process.platform,
|
||||||
|
});
|
||||||
|
for (const pattern of patterns) {
|
||||||
if (pattern) {
|
if (pattern) {
|
||||||
addAllowlistEntry(approvals.file, agentId, pattern);
|
addAllowlistEntry(approvals.file, agentId, pattern);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user