diff --git a/src/commands/doctor-gateway-daemon-flow.ts b/src/commands/doctor-gateway-daemon-flow.ts index b2cdda67de3..5d98c3e9719 100644 --- a/src/commands/doctor-gateway-daemon-flow.ts +++ b/src/commands/doctor-gateway-daemon-flow.ts @@ -68,7 +68,7 @@ async function maybeRepairLaunchAgentBootstrap(params: { } params.runtime.log(`Bootstrapping ${params.title} LaunchAgent...`); - const repair = await repairLaunchAgentBootstrap({ env: params.env, forceEnable: true }); + const repair = await repairLaunchAgentBootstrap({ env: params.env }); if (!repair.ok) { params.runtime.error( `${params.title} LaunchAgent bootstrap failed: ${repair.detail ?? "unknown error"}`, diff --git a/src/daemon/launchd-restart-handoff.test.ts b/src/daemon/launchd-restart-handoff.test.ts index d2dfe6a1d31..cea54b4cc15 100644 --- a/src/daemon/launchd-restart-handoff.test.ts +++ b/src/daemon/launchd-restart-handoff.test.ts @@ -38,10 +38,10 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { const [, args] = spawnMock.mock.calls[0] as [string, string[]]; expect(args[0]).toBe("-c"); expect(args[2]).toBe("openclaw-launchd-restart-handoff"); - expect(args[6]).toBe("0"); - expect(args[7]).toBe("9876"); + expect(args[6]).toBe("9876"); + expect(args[7]).toBe("ai.openclaw.gateway"); expect(args[1]).toContain('while kill -0 "$wait_pid" >/dev/null 2>&1; do'); - expect(args[1]).not.toContain('launchctl enable "$service_target" >/dev/null 2>&1\nif !'); + expect(args[1]).toContain('launchctl enable "$service_target" >/dev/null 2>&1'); expect(args[1]).toContain( 'if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then', ); @@ -49,7 +49,7 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { expect(unrefMock).toHaveBeenCalledTimes(1); }); - it("only injects launchctl enable when the caller requested re-enable", () => { + it("passes the plain label separately for start-after-exit mode", () => { spawnMock.mockReturnValue({ pid: 4242, unref: unrefMock }); scheduleDetachedLaunchdRestartHandoff({ @@ -57,15 +57,12 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { HOME: "/Users/test", OPENCLAW_PROFILE: "default", }, - mode: "kickstart", - shouldEnable: true, - enableMarkerPath: "/Users/test/.openclaw/service/marker", + mode: "start-after-exit", }); const [, args] = spawnMock.mock.calls[0] as [string, string[]]; - expect(args[6]).toBe("1"); - expect(args[8]).toBe("/Users/test/.openclaw/service/marker"); - expect(args[1]).toContain('launchctl enable "$service_target" >/dev/null 2>&1'); - expect(args[1]).toContain('rm -f "$enable_marker_path" >/dev/null 2>&1 || true'); + expect(args[7]).toBe("ai.openclaw.gateway"); + expect(args[1]).toContain('launchctl start "$label" >/dev/null 2>&1'); + expect(args[1]).not.toContain('basename "$service_target"'); }); }); diff --git a/src/daemon/launchd-restart-handoff.ts b/src/daemon/launchd-restart-handoff.ts index b465160fd85..afb1d3650fc 100644 --- a/src/daemon/launchd-restart-handoff.ts +++ b/src/daemon/launchd-restart-handoff.ts @@ -20,6 +20,14 @@ export type LaunchdRestartTarget = { serviceTarget: string; }; +function assertValidLaunchAgentLabel(label: string): string { + const trimmed = label.trim(); + if (!/^[A-Za-z0-9._-]+$/.test(trimmed)) { + throw new Error(`Invalid launchd label: ${trimmed}`); + } + return trimmed; +} + function resolveGuiDomain(): string { if (typeof process.getuid !== "function") { return "gui/501"; @@ -30,9 +38,9 @@ function resolveGuiDomain(): string { function resolveLaunchAgentLabel(env?: Record): string { const envLabel = normalizeOptionalString(env?.OPENCLAW_LAUNCHD_LABEL); if (envLabel) { - return envLabel; + return assertValidLaunchAgentLabel(envLabel); } - return resolveGatewayLaunchAgentLabel(env?.OPENCLAW_PROFILE); + return assertValidLaunchAgentLabel(resolveGatewayLaunchAgentLabel(env?.OPENCLAW_PROFILE)); } export function resolveLaunchdRestartTarget( @@ -66,9 +74,8 @@ export function isCurrentProcessLaunchdServiceLabel( } function buildLaunchdRestartScript(mode: LaunchdRestartHandoffMode): string { - const waitForCallerPid = `should_enable="$4" -wait_pid="$5" -enable_marker_path="$6" + const waitForCallerPid = `wait_pid="$4" +label="$5" if [ -n "$wait_pid" ] && [ "$wait_pid" -gt 1 ] 2>/dev/null; then while kill -0 "$wait_pid" >/dev/null 2>&1; do sleep 0.1 @@ -81,12 +88,7 @@ fi domain="$2" plist_path="$3" ${waitForCallerPid} -if [ "$should_enable" = "1" ]; then - launchctl enable "$service_target" >/dev/null 2>&1 - if [ -n "$enable_marker_path" ]; then - rm -f "$enable_marker_path" >/dev/null 2>&1 || true - fi -fi +launchctl enable "$service_target" >/dev/null 2>&1 if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true @@ -98,14 +100,8 @@ fi return `service_target="$1" domain="$2" plist_path="$3" -label="$(basename "$service_target")" ${waitForCallerPid} -if [ "$should_enable" = "1" ]; then - launchctl enable "$service_target" >/dev/null 2>&1 - if [ -n "$enable_marker_path" ]; then - rm -f "$enable_marker_path" >/dev/null 2>&1 || true - fi -fi +launchctl enable "$service_target" >/dev/null 2>&1 if ! launchctl start "$label" >/dev/null 2>&1; then if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then launchctl start "$label" >/dev/null 2>&1 || launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true @@ -119,9 +115,7 @@ fi export function scheduleDetachedLaunchdRestartHandoff(params: { env?: Record; mode: LaunchdRestartHandoffMode; - shouldEnable?: boolean; waitForPid?: number; - enableMarkerPath?: string; }): LaunchdRestartHandoffResult { const target = resolveLaunchdRestartTarget(params.env); const waitForPid = @@ -138,9 +132,8 @@ export function scheduleDetachedLaunchdRestartHandoff(params: { target.serviceTarget, target.domain, target.plistPath, - params.shouldEnable ? "1" : "0", String(waitForPid), - params.enableMarkerPath ?? "", + target.label, ], { detached: true, diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index 30d95d5a40d..81dbd0a10ad 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -1,4 +1,3 @@ -import path from "node:path"; import { PassThrough } from "node:stream"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -50,19 +49,6 @@ const cleanStaleGatewayProcessesSync = vi.hoisted(() => ); const defaultProgramArguments = ["node", "-e", "process.exit(0)"]; -function resolveDisableMarkerPath( - env: Record, - label = "ai.openclaw.gateway", -) { - const profile = env.OPENCLAW_PROFILE?.trim(); - const suffix = !profile || profile.toLowerCase() === "default" ? "" : `-${profile}`; - return path.join( - env.OPENCLAW_STATE_DIR ?? path.join(env.HOME ?? "/Users/test", `.openclaw${suffix}`), - "service", - `${encodeURIComponent(label)}.launchd-disabled-by-openclaw`, - ); -} - function expectLaunchctlEnableBootstrapOrder(env: Record) { const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501"; const label = "ai.openclaw.gateway"; @@ -332,7 +318,7 @@ describe("launchctl list detection", () => { }); describe("launchd bootstrap repair", () => { - it("bootstraps and kickstarts the resolved label without enabling unrelated disabled state", async () => { + it("enables, bootstraps, and kickstarts the resolved label", async () => { const env: Record = { HOME: "/Users/test", OPENCLAW_PROFILE: "default", @@ -340,16 +326,11 @@ describe("launchd bootstrap repair", () => { const repair = await repairLaunchAgentBootstrap({ env }); expect(repair).toEqual({ ok: true, status: "repaired" }); - const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501"; - const serviceId = `${domain}/ai.openclaw.gateway`; - const bootstrapIndex = state.launchctlCalls.findIndex( - (c) => c[0] === "bootstrap" && c[1] === domain, - ); + const { serviceId, bootstrapIndex } = expectLaunchctlEnableBootstrapOrder(env); const kickstartIndex = state.launchctlCalls.findIndex( (c) => c[0] === "kickstart" && c[1] === "-k" && c[2] === serviceId, ); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); expect(kickstartIndex).toBeGreaterThanOrEqual(0); expect(bootstrapIndex).toBeLessThan(kickstartIndex); }); @@ -415,30 +396,6 @@ describe("launchd bootstrap repair", () => { detail: "launchctl kickstart failed: permission denied", }); }); - - it("re-enables when the disabled marker shows OpenClaw owns the stop state", async () => { - const env: Record = { - HOME: "/Users/test", - OPENCLAW_PROFILE: "default", - }; - state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n"); - - await repairLaunchAgentBootstrap({ env }); - - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); - expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false); - }); - - it("allows explicit repairs to force re-enable disabled services", async () => { - const env: Record = { - HOME: "/Users/test", - OPENCLAW_PROFILE: "default", - }; - - await repairLaunchAgentBootstrap({ env, forceEnable: true }); - - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); - }); }); describe("launchd install", () => { @@ -535,7 +492,6 @@ describe("launchd install", () => { expect(state.launchctlCalls).toContainEqual(["disable", serviceId]); expect(state.launchctlCalls).toContainEqual(["stop", "ai.openclaw.gateway"]); expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false); - expect(state.files.has(resolveDisableMarkerPath(env))).toBe(true); expect(output).toContain("Stopped LaunchAgent"); }); @@ -552,7 +508,6 @@ describe("launchd install", () => { expect(state.launchctlCalls.some((call) => call[0] === "stop")).toBe(false); expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(true); - expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false); expect(output).toContain("Stopped LaunchAgent (degraded)"); expect(output).toContain("used bootout fallback"); }); @@ -649,30 +604,12 @@ describe("launchd install", () => { const serviceId = `${domain}/${label}`; expect(result).toEqual({ outcome: "completed" }); expect(cleanStaleGatewayProcessesSync).toHaveBeenCalledWith(18789); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); + expect(state.launchctlCalls).toContainEqual(["enable", serviceId]); expect(state.launchctlCalls).toContainEqual(["kickstart", "-k", serviceId]); expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false); }); - it("re-enables before restart when OpenClaw owns the persisted disabled state", async () => { - const env = { - ...createDefaultLaunchdEnv(), - OPENCLAW_GATEWAY_PORT: "18789", - }; - const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501"; - const serviceId = `${domain}/ai.openclaw.gateway`; - state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n"); - - await restartLaunchAgent({ - env, - stdout: new PassThrough(), - }); - - expect(state.launchctlCalls).toContainEqual(["enable", serviceId]); - expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false); - }); - it("uses the configured gateway port for stale cleanup", async () => { const env = { ...createDefaultLaunchdEnv(), @@ -716,7 +653,7 @@ describe("launchd install", () => { ); expect(result).toEqual({ outcome: "completed" }); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); + expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true); expect(kickstartCalls).toHaveLength(2); expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false); @@ -734,7 +671,7 @@ describe("launchd install", () => { }), ).rejects.toThrow("launchctl kickstart failed: Input/output error"); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); + expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false); }); @@ -751,7 +688,7 @@ describe("launchd install", () => { }), ).rejects.toThrow("launchctl kickstart failed: Input/output error"); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); + expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true); }); @@ -767,7 +704,7 @@ describe("launchd install", () => { }), ).rejects.toThrow("launchctl kickstart failed: Input/output error"); - expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false); + expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false); }); @@ -784,32 +721,11 @@ describe("launchd install", () => { expect(launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff).toHaveBeenCalledWith({ env, mode: "kickstart", - shouldEnable: false, waitForPid: process.pid, - enableMarkerPath: undefined, }); expect(state.launchctlCalls).toEqual([]); }); - it("passes marker-owned re-enable intent to the detached handoff", async () => { - const env = createDefaultLaunchdEnv(); - state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n"); - launchdRestartHandoffState.isCurrentProcessLaunchdServiceLabel.mockReturnValue(true); - - await restartLaunchAgent({ - env, - stdout: new PassThrough(), - }); - - expect(launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff).toHaveBeenCalledWith({ - env, - mode: "kickstart", - shouldEnable: true, - waitForPid: process.pid, - enableMarkerPath: resolveDisableMarkerPath(env), - }); - }); - it("shows actionable guidance when launchctl gui domain does not support bootstrap", async () => { state.bootstrapError = "Bootstrap failed: 125: Domain does not support specified action"; const env = createDefaultLaunchdEnv(); @@ -883,4 +799,13 @@ describe("resolveLaunchAgentPlistPath", () => { ])("$name", ({ env, expected }) => { expect(resolveLaunchAgentPlistPath(env)).toBe(expected); }); + + it("rejects invalid launchd labels that contain path separators", () => { + expect(() => + resolveLaunchAgentPlistPath({ + HOME: "/Users/test", + OPENCLAW_LAUNCHD_LABEL: "../evil/label", + }), + ).toThrow("Invalid launchd label"); + }); }); diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 1055db68f9d..ef3e13b12d8 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -36,12 +36,20 @@ import type { const LAUNCH_AGENT_DIR_MODE = 0o755; const LAUNCH_AGENT_PLIST_MODE = 0o644; +function assertValidLaunchAgentLabel(label: string): string { + const trimmed = label.trim(); + if (!/^[A-Za-z0-9._-]+$/.test(trimmed)) { + throw new Error(`Invalid launchd label: ${sanitizeForLog(trimmed)}`); + } + return trimmed; +} + function resolveLaunchAgentLabel(args?: { env?: Record }): string { const envLabel = args?.env?.OPENCLAW_LAUNCHD_LABEL?.trim(); if (envLabel) { - return envLabel; + return assertValidLaunchAgentLabel(envLabel); } - return resolveGatewayLaunchAgentLabel(args?.env?.OPENCLAW_PROFILE); + return assertValidLaunchAgentLabel(resolveGatewayLaunchAgentLabel(args?.env?.OPENCLAW_PROFILE)); } function resolveLaunchAgentPlistPathForLabel( @@ -196,11 +204,8 @@ async function bootstrapLaunchAgentOrThrow(params: { serviceTarget: string; plistPath: string; actionHint: string; - enableBeforeBootstrap?: boolean; }) { - if (params.enableBeforeBootstrap) { - await execLaunchctl(["enable", params.serviceTarget]); - } + await execLaunchctl(["enable", params.serviceTarget]); const boot = await execLaunchctl(["bootstrap", params.domain, params.plistPath]); if (boot.code === 0) { return; @@ -324,18 +329,12 @@ export type LaunchAgentBootstrapRepairResult = export async function repairLaunchAgentBootstrap(args: { env?: Record; - forceEnable?: boolean; }): Promise { const env = args.env ?? (process.env as Record); const domain = resolveGuiDomain(); const label = resolveLaunchAgentLabel({ env }); const plistPath = resolveLaunchAgentPlistPath(env); - await enableLaunchAgentIfOwnedStop({ - env, - serviceTarget: `${domain}/${label}`, - label, - force: args.forceEnable, - }); + await execLaunchctl(["enable", `${domain}/${label}`]); const boot = await execLaunchctl(["bootstrap", domain, plistPath]); let repairStatus: LaunchAgentBootstrapRepairResult["status"] = "repaired"; if (boot.code !== 0) { @@ -483,61 +482,6 @@ function formatLaunchctlResultDetail(res: { .slice(0, 1000); } -function resolveLaunchAgentDisableMarkerPath(env: GatewayServiceEnv, label: string): string { - return path.join( - resolveGatewayStateDir(env), - "service", - `${encodeURIComponent(label)}.launchd-disabled-by-openclaw`, - ); -} - -async function hasLaunchAgentDisableMarker(params: { - env: GatewayServiceEnv; - label: string; -}): Promise { - try { - await fs.access(resolveLaunchAgentDisableMarkerPath(params.env, params.label)); - return true; - } catch { - return false; - } -} - -async function writeLaunchAgentDisableMarker(params: { - env: GatewayServiceEnv; - label: string; -}): Promise { - const markerPath = resolveLaunchAgentDisableMarkerPath(params.env, params.label); - await ensureSecureDirectory(path.dirname(markerPath)); - await fs.writeFile(markerPath, "disabled_by_openclaw\n", { mode: 0o600 }); - await fs.chmod(markerPath, 0o600).catch(() => undefined); -} - -async function clearLaunchAgentDisableMarker(params: { - env: GatewayServiceEnv; - label: string; -}): Promise { - await fs - .unlink(resolveLaunchAgentDisableMarkerPath(params.env, params.label)) - .catch(() => undefined); -} - -async function enableLaunchAgentIfOwnedStop(params: { - env: GatewayServiceEnv; - serviceTarget: string; - label: string; - force?: boolean; -}): Promise { - const shouldEnable = - params.force || (await hasLaunchAgentDisableMarker({ env: params.env, label: params.label })); - if (!shouldEnable) { - return false; - } - await execLaunchctl(["enable", params.serviceTarget]); - await clearLaunchAgentDisableMarker({ env: params.env, label: params.label }); - return true; -} - async function bootoutLaunchAgentOrThrow(params: { serviceTarget: string; warning: string; @@ -612,7 +556,6 @@ export async function stopLaunchAgent({ stdout, env }: GatewayServiceControlArgs }); return; } - await writeLaunchAgentDisableMarker({ env: serviceEnv, label }); // `launchctl stop` targets the plain label (not the fully-qualified service target). const stop = await execLaunchctl(["stop", label]); @@ -711,7 +654,6 @@ async function activateLaunchAgent(params: { env: GatewayServiceEnv; plistPath: serviceTarget: `${domain}/${label}`, plistPath: params.plistPath, actionHint: "openclaw gateway install --force", - enableBeforeBootstrap: true, }); } @@ -768,17 +710,11 @@ export async function restartLaunchAgent({ // Restart requests issued from inside the managed gateway process tree need a // detached handoff. A direct `kickstart -k` would terminate the caller before // it can finish the restart command. - const shouldEnable = await hasLaunchAgentDisableMarker({ env: serviceEnv, label }); - if (isCurrentProcessLaunchdServiceLabel(label)) { const handoff = scheduleDetachedLaunchdRestartHandoff({ env: serviceEnv, mode: "kickstart", - shouldEnable, waitForPid: process.pid, - enableMarkerPath: shouldEnable - ? resolveLaunchAgentDisableMarkerPath(serviceEnv, label) - : undefined, }); if (!handoff.ok) { throw new Error(`launchd restart handoff failed: ${handoff.detail ?? "unknown error"}`); @@ -792,9 +728,9 @@ export async function restartLaunchAgent({ cleanStaleGatewayProcessesSync(cleanupPort); } - // Only re-enable disabled LaunchAgents when OpenClaw itself owns the - // persisted stop state. - await enableLaunchAgentIfOwnedStop({ env: serviceEnv, serviceTarget, label }); + // `openclaw gateway restart` is an explicit operator request to bring the + // LaunchAgent back, so clear any persisted disabled state before restart. + await execLaunchctl(["enable", serviceTarget]); const start = await execLaunchctl(["kickstart", "-k", serviceTarget]); if (start.code === 0) {