mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(daemon): harden windows schtasks script quoting
This commit is contained in:
@@ -18,7 +18,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Security/Voice Call: harden `voice-call` telephony TTS override merging by blocking unsafe deep-merge keys (`__proto__`, `prototype`, `constructor`) and add regression coverage for top-level and nested prototype-pollution payloads.
|
||||
- Security/Windows Daemon: escape and quote Scheduled Task environment assignments when generating `gateway.cmd` (`set "KEY=VALUE"`), preventing command injection from config-provided env vars. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||
- Security/Windows Daemon: harden Scheduled Task `gateway.cmd` generation by quoting cmd metacharacter arguments, escaping `%`/`!` expansions, and rejecting CR/LF in arguments, descriptions, and environment assignments (`set "KEY=VALUE"`), preventing command injection in Windows daemon startup scripts. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||
- Security/Gateway/Canvas: replace shared-IP fallback auth with node-scoped session capability URLs for `/__openclaw__/canvas/*` and `/__openclaw__/a2ui/*`, fail closed when trusted-proxy requests omit forwarded client headers, and add IPv6/proxy-header regression coverage. This ships in the next npm release. Thanks @aether-ai-agent for reporting.
|
||||
- Security/Net: enforce strict dotted-decimal IPv4 literals in SSRF checks and fail closed on unsupported legacy forms (octal/hex/short/packed, for example `0177.0.0.1`, `127.1`, `2130706433`) before DNS lookup.
|
||||
- Security/Discord: enforce trusted-sender guild permission checks for moderation actions (`timeout`, `kick`, `ban`) and ignore untrusted `senderUserId` params to prevent privilege escalation in tool-driven flows. Thanks @aether-ai-agent for reporting.
|
||||
|
||||
@@ -29,7 +29,17 @@ describe("installScheduledTask", () => {
|
||||
const { scriptPath } = await installScheduledTask({
|
||||
env,
|
||||
stdout: new PassThrough(),
|
||||
programArguments: ["node", "gateway.js", "--verbose"],
|
||||
programArguments: [
|
||||
"node",
|
||||
"gateway.js",
|
||||
"--display-name",
|
||||
"safe&whoami",
|
||||
"--percent",
|
||||
"%TEMP%",
|
||||
"--bang",
|
||||
"!token!",
|
||||
],
|
||||
workingDirectory: "C:\\temp\\poc&calc",
|
||||
environment: {
|
||||
OC_INJECT: "safe & whoami | calc",
|
||||
OC_CARET: "a^b",
|
||||
@@ -40,6 +50,10 @@ describe("installScheduledTask", () => {
|
||||
});
|
||||
|
||||
const script = await fs.readFile(scriptPath, "utf8");
|
||||
expect(script).toContain('cd /d "C:\\temp\\poc&calc"');
|
||||
expect(script).toContain(
|
||||
'node gateway.js --display-name "safe&whoami" --percent "%%TEMP%%" --bang "^!token^!"',
|
||||
);
|
||||
expect(script).toContain('set "OC_INJECT=safe & whoami | calc"');
|
||||
expect(script).toContain('set "OC_CARET=a^^b"');
|
||||
expect(script).toContain('set "OC_PERCENT=%%TEMP%%"');
|
||||
@@ -48,6 +62,19 @@ describe("installScheduledTask", () => {
|
||||
expect(script).not.toContain("set OC_INJECT=");
|
||||
|
||||
const parsed = await readScheduledTaskCommand(env);
|
||||
expect(parsed).toMatchObject({
|
||||
programArguments: [
|
||||
"node",
|
||||
"gateway.js",
|
||||
"--display-name",
|
||||
"safe&whoami",
|
||||
"--percent",
|
||||
"%TEMP%",
|
||||
"--bang",
|
||||
"!token!",
|
||||
],
|
||||
workingDirectory: "C:\\temp\\poc&calc",
|
||||
});
|
||||
expect(parsed?.environment).toMatchObject({
|
||||
OC_INJECT: "safe & whoami | calc",
|
||||
OC_CARET: "a^b",
|
||||
@@ -63,4 +90,43 @@ describe("installScheduledTask", () => {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects line breaks in command arguments, env vars, and descriptions", async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-schtasks-install-"));
|
||||
const env = {
|
||||
USERPROFILE: tmpDir,
|
||||
OPENCLAW_PROFILE: "default",
|
||||
};
|
||||
try {
|
||||
await expect(
|
||||
installScheduledTask({
|
||||
env,
|
||||
stdout: new PassThrough(),
|
||||
programArguments: ["node", "gateway.js", "bad\narg"],
|
||||
environment: {},
|
||||
}),
|
||||
).rejects.toThrow(/Command argument cannot contain CR or LF/);
|
||||
|
||||
await expect(
|
||||
installScheduledTask({
|
||||
env,
|
||||
stdout: new PassThrough(),
|
||||
programArguments: ["node", "gateway.js"],
|
||||
environment: { BAD: "line1\r\nline2" },
|
||||
}),
|
||||
).rejects.toThrow(/Environment variable value cannot contain CR or LF/);
|
||||
|
||||
await expect(
|
||||
installScheduledTask({
|
||||
env,
|
||||
stdout: new PassThrough(),
|
||||
description: "bad\ndescription",
|
||||
programArguments: ["node", "gateway.js"],
|
||||
environment: {},
|
||||
}),
|
||||
).rejects.toThrow(/Task description cannot contain CR or LF/);
|
||||
} finally {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -36,13 +36,35 @@ export function resolveTaskScriptPath(env: GatewayServiceEnv): string {
|
||||
return path.join(stateDir, scriptName);
|
||||
}
|
||||
|
||||
function quoteCmdArg(value: string): string {
|
||||
function assertNoCmdLineBreak(value: string, field: string): void {
|
||||
if (/[\r\n]/.test(value)) {
|
||||
throw new Error(`${field} cannot contain CR or LF in Windows task scripts.`);
|
||||
}
|
||||
}
|
||||
|
||||
function quoteSchtasksArg(value: string): string {
|
||||
if (!/[ \t"]/g.test(value)) {
|
||||
return value;
|
||||
}
|
||||
return `"${value.replace(/"/g, '\\"')}"`;
|
||||
}
|
||||
|
||||
function quoteCmdScriptArg(value: string): string {
|
||||
assertNoCmdLineBreak(value, "Command argument");
|
||||
if (!value) {
|
||||
return '""';
|
||||
}
|
||||
const escaped = value.replace(/"/g, '\\"').replace(/%/g, "%%").replace(/!/g, "^!");
|
||||
if (!/[ \t"&|<>^()%!]/g.test(value)) {
|
||||
return escaped;
|
||||
}
|
||||
return `"${escaped}"`;
|
||||
}
|
||||
|
||||
function unescapeCmdScriptArg(value: string): string {
|
||||
return value.replace(/\^!/g, "!").replace(/%%/g, "%");
|
||||
}
|
||||
|
||||
function resolveTaskUser(env: GatewayServiceEnv): string | null {
|
||||
const username = env.USERNAME || env.USER || env.LOGNAME;
|
||||
if (!username) {
|
||||
@@ -59,9 +81,11 @@ function resolveTaskUser(env: GatewayServiceEnv): string | null {
|
||||
}
|
||||
|
||||
function parseCommandLine(value: string): string[] {
|
||||
// `buildTaskScript` only escapes quotes (`\"`).
|
||||
// `buildTaskScript` escapes quotes (`\"`) and cmd expansions (`%%`, `^!`).
|
||||
// Keep all other backslashes literal so drive and UNC paths are preserved.
|
||||
return splitArgsPreservingQuotes(value, { escapeMode: "backslash-quote-only" });
|
||||
return splitArgsPreservingQuotes(value, { escapeMode: "backslash-quote-only" }).map(
|
||||
unescapeCmdScriptArg,
|
||||
);
|
||||
}
|
||||
|
||||
export async function readScheduledTaskCommand(
|
||||
@@ -143,21 +167,25 @@ function buildTaskScript({
|
||||
environment,
|
||||
}: GatewayServiceRenderArgs): string {
|
||||
const lines: string[] = ["@echo off"];
|
||||
if (description?.trim()) {
|
||||
lines.push(`rem ${description.trim()}`);
|
||||
const trimmedDescription = description?.trim();
|
||||
if (trimmedDescription) {
|
||||
assertNoCmdLineBreak(trimmedDescription, "Task description");
|
||||
lines.push(`rem ${trimmedDescription}`);
|
||||
}
|
||||
if (workingDirectory) {
|
||||
lines.push(`cd /d ${quoteCmdArg(workingDirectory)}`);
|
||||
lines.push(`cd /d ${quoteCmdScriptArg(workingDirectory)}`);
|
||||
}
|
||||
if (environment) {
|
||||
for (const [key, value] of Object.entries(environment)) {
|
||||
if (!value) {
|
||||
continue;
|
||||
}
|
||||
assertNoCmdLineBreak(key, "Environment variable name");
|
||||
assertNoCmdLineBreak(value, "Environment variable value");
|
||||
lines.push(renderCmdSetAssignment(key, value));
|
||||
}
|
||||
}
|
||||
const command = programArguments.map(quoteCmdArg).join(" ");
|
||||
const command = programArguments.map(quoteCmdScriptArg).join(" ");
|
||||
lines.push(command);
|
||||
return `${lines.join("\r\n")}\r\n`;
|
||||
}
|
||||
@@ -192,7 +220,7 @@ export async function installScheduledTask({
|
||||
await fs.writeFile(scriptPath, script, "utf8");
|
||||
|
||||
const taskName = resolveTaskName(env);
|
||||
const quotedScript = quoteCmdArg(scriptPath);
|
||||
const quotedScript = quoteSchtasksArg(scriptPath);
|
||||
const baseArgs = [
|
||||
"/Create",
|
||||
"/F",
|
||||
|
||||
Reference in New Issue
Block a user