From 61f646c41fb43cd87ed48f9125b4718a30d38e84 Mon Sep 17 00:00:00 2001 From: Shadow Date: Fri, 20 Feb 2026 12:51:14 -0600 Subject: [PATCH] Daemon: harden systemd unit env rendering --- CHANGELOG.md | 1 + src/daemon/systemd-unit.test.ts | 26 ++++++++++++++++++++++++++ src/daemon/systemd-unit.ts | 24 +++++++++++++++++++----- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 src/daemon/systemd-unit.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c13e30da541..1496713c276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Docs: https://docs.openclaw.ai - Discord/Gateway: handle close code 4014 (missing privileged gateway intents) without crashing the gateway. Thanks @thewilloftheshadow. - Security/Net: strip sensitive headers (`Authorization`, `Proxy-Authorization`, `Cookie`, `Cookie2`) on cross-origin redirects in `fetchWithSsrFGuard` to prevent credential forwarding across origin boundaries. (#20313) Thanks @afurm. +- Security/Systemd: reject CR/LF in systemd unit environment values and fix argument escaping so generated units cannot be injected with extra directives. Thanks @thewilloftheshadow. - Skills/Security: sanitize skill env overrides to block unsafe runtime injection variables and only allow sensitive keys when declared in skill metadata, with warnings for suspicious values. Thanks @thewilloftheshadow. - Auto-reply/Runner: emit `onAgentRunStart` only after agent lifecycle or tool activity begins (and only once per run), so fallback preflight errors no longer mark runs as started. (#21165) Thanks @shakkernerd. - Agents/Failover: treat non-default override runs as direct fallback-to-configured-primary (skip configured fallback chain), normalize default-model detection for provider casing/whitespace, and add regression coverage for override/auth error paths. (#18820) Thanks @Glucksberg. diff --git a/src/daemon/systemd-unit.test.ts b/src/daemon/systemd-unit.test.ts new file mode 100644 index 00000000000..bd65e34bba4 --- /dev/null +++ b/src/daemon/systemd-unit.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; +import { buildSystemdUnit } from "./systemd-unit.js"; + +describe("buildSystemdUnit", () => { + it("quotes arguments with whitespace", () => { + const unit = buildSystemdUnit({ + description: "OpenClaw Gateway", + programArguments: ["/usr/bin/openclaw", "gateway", "--name", "My Bot"], + environment: {}, + }); + const execStart = unit.split("\n").find((line) => line.startsWith("ExecStart=")); + expect(execStart).toBe('ExecStart=/usr/bin/openclaw gateway --name "My Bot"'); + }); + + it("rejects environment values with line breaks", () => { + expect(() => + buildSystemdUnit({ + description: "OpenClaw Gateway", + programArguments: ["/usr/bin/openclaw", "gateway", "start"], + environment: { + INJECT: "ok\nExecStartPre=/bin/touch /tmp/oc15789_rce", + }, + }), + ).toThrow(/CR or LF/); + }); +}); diff --git a/src/daemon/systemd-unit.ts b/src/daemon/systemd-unit.ts index e76883c779c..000f4b64a92 100644 --- a/src/daemon/systemd-unit.ts +++ b/src/daemon/systemd-unit.ts @@ -1,8 +1,17 @@ import { splitArgsPreservingQuotes } from "./arg-split.js"; import type { GatewayServiceRenderArgs } from "./service-types.js"; +const SYSTEMD_LINE_BREAKS = /[\r\n]/; + +function assertNoSystemdLineBreaks(value: string, label: string): void { + if (SYSTEMD_LINE_BREAKS.test(value)) { + throw new Error(`${label} cannot contain CR or LF characters.`); + } +} + function systemdEscapeArg(value: string): string { - if (!/[\\s"\\\\]/.test(value)) { + assertNoSystemdLineBreaks(value, "Systemd unit values"); + if (!/[\s"\\]/.test(value)) { return value; } return `"${value.replace(/\\\\/g, "\\\\\\\\").replace(/"/g, '\\\\"')}"`; @@ -18,9 +27,12 @@ function renderEnvLines(env: Record | undefined): st if (entries.length === 0) { return []; } - return entries.map( - ([key, value]) => `Environment=${systemdEscapeArg(`${key}=${value?.trim() ?? ""}`)}`, - ); + return entries.map(([key, value]) => { + const rawValue = value ?? ""; + assertNoSystemdLineBreaks(key, "Systemd environment variable names"); + assertNoSystemdLineBreaks(rawValue, "Systemd environment variable values"); + return `Environment=${systemdEscapeArg(`${key}=${rawValue.trim()}`)}`; + }); } export function buildSystemdUnit({ @@ -30,7 +42,9 @@ export function buildSystemdUnit({ environment, }: GatewayServiceRenderArgs): string { const execStart = programArguments.map(systemdEscapeArg).join(" "); - const descriptionLine = `Description=${description?.trim() || "OpenClaw Gateway"}`; + const descriptionValue = description?.trim() || "OpenClaw Gateway"; + assertNoSystemdLineBreaks(descriptionValue, "Systemd Description"); + const descriptionLine = `Description=${descriptionValue}`; const workingDirLine = workingDirectory ? `WorkingDirectory=${systemdEscapeArg(workingDirectory)}` : null;