From e31de4a6c0caa2449e3a88330e58b014c5988c19 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 24 Aug 2026 05:10:02 -0700 Subject: [PATCH] fix(daemon): audit effective systemd properties (#128705) --- src/cli/daemon-cli/status.gather.test.ts | 6 +- src/cli/daemon-cli/status.gather.ts | 1 + src/daemon/service-audit.test.ts | 128 ++++++++++++++++++++++- src/daemon/service-audit.ts | 31 +++++- 4 files changed, 160 insertions(+), 6 deletions(-) diff --git a/src/cli/daemon-cli/status.gather.test.ts b/src/cli/daemon-cli/status.gather.test.ts index e110437b6e89..98e1c93c430a 100644 --- a/src/cli/daemon-cli/status.gather.test.ts +++ b/src/cli/daemon-cli/status.gather.test.ts @@ -360,6 +360,7 @@ describe("gatherDaemonStatus", () => { deleteTestEnvValue("DAEMON_GATEWAY_PASSWORD"); isDefaultInstallIdentity.mockReset().mockReturnValue(true); isGatewayExternallySupervised.mockReset().mockReturnValue(false); + auditGatewayServiceConfig.mockClear(); callGatewayStatusProbe.mockClear(); resolveAdvertisedControlUiLinks.mockClear(); resolveAdvertisedControlUiLinks.mockResolvedValue({ @@ -740,7 +741,7 @@ describe("gatherDaemonStatus", () => { expect((status.service.runtime as { detail?: string }).detail).toBe("19001"); }); - it("bounds both service-manager reads and still emits JSON after they time out", async () => { + it("bounds all service-manager reads and still emits JSON after they time out", async () => { serviceIsLoaded.mockImplementationOnce(async (args?: { timeoutMs?: number }) => { if (args?.timeoutMs === undefined) { return await new Promise(() => {}); @@ -762,6 +763,9 @@ describe("gatherDaemonStatus", () => { expect(serviceIsLoaded).toHaveBeenCalledWith(expect.objectContaining({ timeoutMs: 100 })); expect(serviceReadRuntime).toHaveBeenCalledWith(expect.any(Object), { timeoutMs: 100 }); + expect(auditGatewayServiceConfig).toHaveBeenCalledWith( + expect.objectContaining({ timeoutMs: 100 }), + ); expect(status.service.loadState).toEqual({ status: "unknown", detail: "Error: systemctl is-enabled timed out", diff --git a/src/cli/daemon-cli/status.gather.ts b/src/cli/daemon-cli/status.gather.ts index 5e276f7234aa..16e5e1700132 100644 --- a/src/cli/daemon-cli/status.gather.ts +++ b/src/cli/daemon-cli/status.gather.ts @@ -618,6 +618,7 @@ export async function gatherDaemonStatus( auditGatewayServiceConfig({ env: process.env, command, + timeoutMs, }), ) : { ok: true, issues: [] satisfies ServiceConfigAudit["issues"] }; diff --git a/src/daemon/service-audit.test.ts b/src/daemon/service-audit.test.ts index 91bc0c78d9f8..875dbbacf49b 100644 --- a/src/daemon/service-audit.test.ts +++ b/src/daemon/service-audit.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { auditGatewayServiceConfig, checkTokenDrift, @@ -11,6 +11,21 @@ import { import { buildServiceEnvironment } from "./service-env.js"; import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; +const execSystemctlUser = vi.hoisted(() => + vi.fn< + ( + env: NodeJS.ProcessEnv, + args: string[], + timeoutMs?: number, + ) => Promise<{ stdout: string; stderr: string; code: number }> + >(), +); + +vi.mock("./systemd-exec.js", async (importOriginal) => ({ + ...(await importOriginal()), + execSystemctlUser, +})); + function buildMinimalServicePath(options: { platform: NodeJS.Platform; env: Record; @@ -65,9 +80,13 @@ function createGatewayAudit({ }); } -async function writeSystemdUnitForAudit(home: string, lines: string[]) { +async function writeSystemdUnitForAudit( + home: string, + lines: string[], + unitName = "openclaw-gateway.service", +) { const unitDir = path.join(home, ".config", "systemd", "user"); - const unitPath = path.join(unitDir, "openclaw-gateway.service"); + const unitPath = path.join(unitDir, unitName); await fs.mkdir(unitDir, { recursive: true }); await fs.writeFile( unitPath, @@ -101,6 +120,11 @@ function expectTokenAudit( } describe("auditGatewayServiceConfig", () => { + beforeEach(() => { + execSystemctlUser.mockReset(); + execSystemctlUser.mockResolvedValue({ stdout: "", stderr: "systemd unavailable", code: 1 }); + }); + it("flags bun runtime", async () => { const audit = await auditGatewayServiceConfig({ env: { HOME: "/tmp" }, @@ -500,6 +524,103 @@ describe("auditGatewayServiceConfig", () => { expectTokenAudit(audit, { embedded: true, mismatch: true }); }); + it.each([ + { + name: "uses manager KillMode instead of the base unit", + unit: [ + "After=network-online.target", + "Wants=network-online.target", + "RestartSec=5", + "KillMode=control-group", + ], + manager: [ + "KillMode=process", + "RestartUSec=5s", + "After=network-online.target", + "Wants=network-online.target", + ], + code: SERVICE_AUDIT_CODES.systemdKillModeProcessOrNone, + expected: true, + }, + { + name: "uses manager RestartUSec instead of the base unit", + unit: [ + "After=network-online.target", + "Wants=network-online.target", + "RestartSec=100ms", + "KillMode=control-group", + ], + manager: [ + "Wants=network-online.target", + "KillMode=control-group", + "RestartUSec=5s", + "After=network-online.target", + ], + code: SERVICE_AUDIT_CODES.systemdRestartSec, + expected: false, + }, + { + name: "uses manager After dependencies absent from the base unit", + unit: ["Wants=network-online.target", "RestartSec=5", "KillMode=control-group"], + manager: [ + "RestartUSec=5s", + "After=basic.target network-online.target", + "KillMode=control-group", + "Wants=network-online.target", + ], + code: SERVICE_AUDIT_CODES.systemdAfterNetworkOnline, + expected: false, + }, + { + name: "does not refill missing manager Wants from the base unit", + unit: [ + "After=network-online.target", + "Wants=network-online.target", + "RestartSec=5", + "KillMode=control-group", + ], + manager: [ + "After=network-online.target", + "RestartUSec=5s", + "Wants=basic.target", + "KillMode=control-group", + ], + code: SERVICE_AUDIT_CODES.systemdWantsNetworkOnline, + expected: true, + }, + ])("respects systemd manager authority: $name", async ({ unit, manager, code, expected }) => { + const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-service-audit-manager-")); + try { + const unitName = "openclaw-audit.service"; + const env = { HOME: home, OPENCLAW_SYSTEMD_UNIT: unitName }; + await writeSystemdUnitForAudit(home, unit, unitName); + execSystemctlUser.mockResolvedValueOnce({ + stdout: manager.join("\n"), + stderr: "", + code: 0, + }); + + const audit = await auditGatewayServiceConfig({ + env, + platform: "linux", + timeoutMs: 321, + command: { + programArguments: ["/usr/bin/node", "gateway"], + environment: { PATH: "/usr/bin:/bin" }, + }, + }); + + expect(hasIssue(audit, code)).toBe(expected); + expect(execSystemctlUser).toHaveBeenCalledExactlyOnceWith( + env, + ["show", unitName, "--no-page", "--property", "After,Wants,RestartUSec,KillMode"], + 321, + ); + } finally { + await fs.rm(home, { recursive: true, force: true }); + } + }); + it.each(["process", "none"])( `warns when KillMode is %s in explicit unit file`, async (killMode) => { @@ -524,6 +645,7 @@ describe("auditGatewayServiceConfig", () => { (entry) => entry.code === SERVICE_AUDIT_CODES.systemdKillModeProcessOrNone, ), ).toBe(true); + expect(execSystemctlUser).toHaveBeenCalledWith({ HOME: home }, expect.any(Array), 10_000); }, ); diff --git a/src/daemon/service-audit.ts b/src/daemon/service-audit.ts index ec7e387de11a..e84c687e112f 100644 --- a/src/daemon/service-audit.ts +++ b/src/daemon/service-audit.ts @@ -11,6 +11,7 @@ import { POSIX_SHELL_WRAPPERS } from "../infra/shell-wrapper-resolution.js"; import { parseTcpPort } from "../infra/tcp-port.js"; import { resolveLaunchAgentPlistPath } from "./launchd.js"; import { isBunRuntime, isNodeRuntime } from "./runtime-binary.js"; +import { parseKeyValueOutput } from "./runtime-parse.js"; import { isSystemNodePath, isVersionManagedNodePath, @@ -24,6 +25,8 @@ import { } from "./service-managed-env.js"; import { isNonMinimalServicePathEntry, normalizeServicePathEntry } from "./service-path-policy.js"; import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; +import { execSystemctlUser } from "./systemd-exec.js"; +import { resolveSystemdServiceName } from "./systemd-service-files.js"; import { resolveSystemdUserUnitPath } from "./systemd.js"; export type GatewayServiceCommand = { @@ -82,6 +85,7 @@ function hasGatewaySubcommand(programArguments?: string[]): boolean { const POSIX_SERVICE_INLINE_COMMAND_FLAGS = new Set(["-c"]); const POSIX_SERVICE_SHELL_WRAPPERS: ReadonlySet = POSIX_SHELL_WRAPPERS; +const SYSTEMD_AUDIT_TIMEOUT_MS = 10_000; function isOpaquePosixShellInlineCommand(programArguments: string[]): boolean { const executable = programArguments[0]?.trim(); @@ -176,6 +180,7 @@ function parseSystemdRestartSecSeconds(value: string): number | undefined { async function auditSystemdUnit( env: Record, issues: ServiceConfigIssue[], + timeoutMs?: number, ) { const unitPath = resolveSystemdUserUnitPath(env); let content; @@ -185,7 +190,28 @@ async function auditSystemdUnit( return; } - const parsed = parseSystemdUnit(content); + // The manager owns merged drop-ins and dependency links. Fall back wholesale + // to the base unit only when its bounded effective-state query fails. + const manager = await execSystemctlUser( + env, + [ + "show", + `${resolveSystemdServiceName(env)}.service`, + "--no-page", + "--property", + "After,Wants,RestartUSec,KillMode", + ], + timeoutMs && timeoutMs > 0 ? timeoutMs : SYSTEMD_AUDIT_TIMEOUT_MS, + ); + const entries = manager.code === 0 ? parseKeyValueOutput(manager.stdout, "=") : undefined; + const parsed = entries + ? { + after: new Set(entries.after?.split(/\s+/).filter(Boolean)), + wants: new Set(entries.wants?.split(/\s+/).filter(Boolean)), + restartSec: entries.restartusec, + killMode: entries.killmode, + } + : parseSystemdUnit(content); if (!parsed.after.has("network-online.target")) { issues.push({ code: SERVICE_AUDIT_CODES.systemdAfterNetworkOnline, @@ -582,6 +608,7 @@ export async function auditGatewayServiceConfig(params: { expectedManagedServiceEnvKeys?: Iterable; expectedServicePath?: string; expectedPort?: number; + timeoutMs?: number; }): Promise { const issues: ServiceConfigIssue[] = []; const platform = params.platform ?? process.platform; @@ -599,7 +626,7 @@ export async function auditGatewayServiceConfig(params: { await auditGatewayRuntime(params.env, params.command, issues, platform); if (platform === "linux") { - await auditSystemdUnit(params.env, issues); + await auditSystemdUnit(params.env, issues, params.timeoutMs); } else if (platform === "darwin") { await auditLaunchdPlist(params.env, issues); }