ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-16 07:33:46 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(daemon): skip unreadable foreign LaunchDaemon plists in ownership scan (#120481)

Browse Source 
* fix(daemon): skip unreadable foreign LaunchDaemon plists in ownership scan

inspectSystemLaunchDaemonOwnership raised SystemLaunchDaemonOwnershipError when any /Library/LaunchDaemons plist was unreadable, such as third-party VPN helper plists with mode 600. This broke gateway install/repair and launchd.integration.e2e on affected Macs. Unreadable plists are foreign by contract, while loaded same-label daemons remain caught by the bracketing launchctl probes.

* fix(ci): regenerate plugin-sdk api baseline after 267268f646

* test(doctor): scope gateway-bind persistence test to its writer seam

Culprit: 267268f646.

* docs(daemon): document unreadable plist tradeoff
This commit is contained in:
Peter Steinberger
2026-08-08 00:48:59 -07:00
committed by GitHub
parent bf0aadbc40
commit fb61ae15fc
4 changed files with 70 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/.generated/plugin-sdk-api-baseline.sha256
+1 -1
View File
@@ -122,7 +122,7 @@ aa8a411ad37c1d1143b67376bf2d20255b9eedff61d80815f42e4f8ed7bd8e58 module/secret-
57dcb1462d4c4f9a98d934c4ca975b163d704758af9821a64001ff3ac05637c3 module/secret-input-runtime
dc0ee07d392a85c218939000b28c0138f139215da00f5592b34a68ba8e29a25d module/secret-ref-runtime
f97549081955e412d8eb64070c64bb5921bb1324744d824db05767d3adcce403 module/security-runtime
5b1e30fe228e0d8c74c11d6ad2d9aed3df80cd3ea4ba2dd63c30cc5f084e93c9 module/session-catalog
8f57eaf07620598f0a62cf603d450d9a9da2d4ea7a867f13aadc1212019e5eb9 module/session-catalog
50f5e344f98c27570b7a30e32a906b612e2383d21f102e88cd93e1d5425a6de9 module/session-discussion
f112bdabc51ba8659b37d0a6f6a32a2b1d471e5b49b56e108bf750ec55a7ea71 module/session-store-runtime
36affbe151431a6141664b6838e20f2d121ff210d57a3c1b4b41a8818b5c81d8 module/setup
src/commands/doctor-config-flow.gateway-bind-persistence.test.ts
+12 -27
View File
@@ -1,20 +1,14 @@
// Verifies Doctor persists legacy gateway bind repairs through the real config writer.
import fs from "node:fs/promises";
import { afterEach, describe, expect, it, vi } from "vitest";
import { readConfigFileSnapshot } from "../config/config.js";
import { describe, expect, it, vi } from "vitest";
import { withTempHome, writeOpenClawConfig } from "../config/test-helpers.js";
import { runInitialConfigWriteHealth } from "../flows/doctor-health-contribution-runners.config.js";
import type { DoctorHealthFlowContext } from "../flows/doctor-health-contribution-types.js";
import type { RuntimeEnv } from "../runtime.js";
import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js";
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
import { createDoctorPrompter, type DoctorOptions } from "./doctor-prompter.js";
import { migrateLegacyConfig } from "./doctor/shared/legacy-config-migrate.js";
describe("Doctor gateway bind persistence", () => {
afterEach(() => {
closeOpenClawStateDatabaseForTest();
});
it.each([
["localhost", "loopback"],
["0.0.0.0", "lan"],
@@ -30,36 +24,27 @@ describe("Doctor gateway bind persistence", () => {
};
const options: DoctorOptions = { nonInteractive: true, repair: true };
const prompter = createDoctorPrompter({ runtime, options });
const configResult = await loadAndMaybeMigrateDoctorConfig({
options,
confirm: (params) => prompter.confirm(params),
runtime,
prompter,
});
const migration = migrateLegacyConfig({ gateway: { mode: "local", bind: legacyBind } });
expect(migration.config).not.toBeNull();
const cfg = migration.config!;
const configResult = { cfg, shouldWriteConfig: true };
const ctx: DoctorHealthFlowContext = {
runtime,
options,
prompter,
configResult,
cfg: configResult.cfg,
cfgForPersistence: structuredClone(configResult.cfg),
sourceConfigValid: configResult.sourceConfigValid ?? true,
cfg,
cfgForPersistence: structuredClone(cfg),
sourceConfigValid: true,
configPath,
stateDirExistedAtStart: true,
...(configResult.runWithPluginMetadataSnapshot
? { runWithPluginMetadataSnapshot: configResult.runWithPluginMetadataSnapshot }
: {}),
...(configResult.invalidatePluginMetadataSnapshot
? { invalidatePluginMetadataSnapshot: configResult.invalidatePluginMetadataSnapshot }
: {}),
};
await runInitialConfigWriteHealth(ctx);
const snapshot = await readConfigFileSnapshot();
expect(snapshot.valid).toBe(true);
expect(snapshot.config.gateway?.bind).toBe(canonicalBind);
expect(await fs.readFile(configPath, "utf-8")).not.toContain(`"bind": "${legacyBind}"`);
const persisted = JSON.parse(await fs.readFile(configPath, "utf-8"));
expect(persisted.gateway?.bind).toBe(canonicalBind);
expect(persisted.gateway?.bind).not.toBe(legacyBind);
});
});
});
src/daemon/launchd-system.test.ts
+41 -9
View File
@@ -1,6 +1,6 @@
// System launchd ownership tests cover loaded, installed, and unverifiable states.
import { execFileSync } from "node:child_process";
import { chmodSync, mkdirSync, writeFileSync } from "node:fs";
import { chmodSync, constants, mkdirSync, writeFileSync } from "node:fs";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
@@ -20,9 +20,10 @@ function fsError(code: string, target: string): NodeJS.ErrnoException {
vi.mock("node:fs/promises", () => {
const mocked = {
access: vi.fn(async (target: string) => {
constants,
access: vi.fn(async (target: string, mode?: number) => {
const code = state.accessErrors.get(target);
if (code) {
if (code && mode === constants.R_OK) {
throw fsError(code, target);
}
if (!state.files.has(target)) {
@@ -80,7 +81,10 @@ import {
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
const hostPlatform = process.platform;
function runRenderedProbe(plistName: string, plistMode: "unlabeled" | "same-label" | "malformed") {
function runRenderedProbe(
plistName: string,
plistMode: "unlabeled" | "same-label" | "malformed" | "unreadable",
) {
const root = tempDirs.make("openclaw-launchd-probe-");
const daemonsDir = path.join(root, "daemons");
const binDir = path.join(root, "bin");
@@ -93,6 +97,12 @@ function runRenderedProbe(plistName: string, plistMode: "unlabeled" | "same-labe
`#!/bin/sh
mode=$1
for last; do :; done
case "$last" in
*unreadable*)
printf '%s\\n' "Operation not permitted" >&2
exit 1
;;
esac
fixture=$(cat "$last")
if [ "$mode" = "-extract" ]; then
if [ "$fixture" = "same-label" ]; then
@@ -117,6 +127,9 @@ exit 1
);
chmodSync(plutilShim, 0o700);
chmodSync(launchctlShim, 0o700);
if (plistMode === "unreadable") {
chmodSync(path.join(daemonsDir, plistName), 0o000);
}
const script = renderSystemLaunchDaemonOwnershipShellProbe("ai.openclaw.gateway")
.replaceAll("/Library/LaunchDaemons", daemonsDir)
.replaceAll("/usr/bin/plutil", plutilShim)
@@ -260,17 +273,34 @@ describe("system LaunchDaemon ownership", () => {
expect(execLaunchctl).toHaveBeenCalledTimes(2);
});
it("fails closed on an unreadable noncanonical vendor plist", async () => {
it("skips an unreadable foreign plist", async () => {
const unrelated = "/Library/LaunchDaemons/com.vendor.locked.plist";
state.files.set(unrelated, "<plist/>");
state.accessErrors.set(unrelated, "EACCES");
state.plutilErrors.set(unrelated, "Operation not permitted");
await expect(inspectSystemLaunchDaemonOwnership("ai.openclaw.gateway")).resolves.toEqual({
status: "unverifiable",
status: "absent",
serviceTarget: "system/ai.openclaw.gateway",
operation: "filesystem",
detail: `${unrelated}: EACCES: ${unrelated}`,
});
await expect(
assertNoSystemLaunchDaemonOwnership("ai.openclaw.gateway"),
).resolves.toBeUndefined();
});
it("detects a readable owner after an unreadable foreign plist", async () => {
const unrelated = "/Library/LaunchDaemons/com.vendor.locked.plist";
const owner = "/Library/LaunchDaemons/vendor-openclaw.plist";
state.files.set(unrelated, "<plist/>");
state.accessErrors.set(unrelated, "EACCES");
state.plutilErrors.set(unrelated, "Operation not permitted");
state.files.set(owner, "<plist/>");
state.plutilValues.set(owner, { Label: "ai.openclaw.gateway" });
await expect(inspectSystemLaunchDaemonOwnership("ai.openclaw.gateway")).resolves.toEqual({
status: "installed",
serviceTarget: "system/ai.openclaw.gateway",
plistPath: owner,
});
});
@@ -332,6 +362,7 @@ describe("system LaunchDaemon ownership", () => {
"/usr/bin/find \"$openclaw_system_launchd_dir\" -mindepth 1 -maxdepth 1 -name '*.plist' -print0",
);
expect(script).toContain("while IFS= read -r -d '' openclaw_system_launchd_plist");
expect(script).toContain('[ ! -r "$openclaw_system_launchd_plist" ]');
expect(script).toContain('[ ! -x "$openclaw_system_launchd_dir" ]');
expect(script).not.toContain('"$openclaw_system_launchd_dir"/*.plist');
expect(script).toContain(
@@ -340,8 +371,9 @@ describe("system LaunchDaemon ownership", () => {
expect(script).not.toContain("|| true");
});
it("executes the rendered probe across unlabeled, same-label, and malformed plists", () => {
it("executes the rendered probe across readable and unreadable plists", () => {
expect(runRenderedProbe("com.google.keystone.daemon.plist", "unlabeled")).toBe("");
expect(runRenderedProbe("com.vendor.unreadable.plist", "unreadable")).toBe("");
expect(runRenderedProbe("vendor-openclaw.plist", "same-label")).toContain(
"vendor-openclaw.plist",
);
src/daemon/launchd-system.ts
+16 -1
View File
@@ -71,6 +71,11 @@ if [ -z "$openclaw_system_launchd_conflict" ]; then
if openclaw_system_launchd_entries=$(/usr/bin/mktemp "\${TMPDIR:-/tmp}/openclaw-launchd-scan.XXXXXX" 2>&1); then
if /usr/bin/find "$openclaw_system_launchd_dir" -mindepth 1 -maxdepth 1 -name '*.plist' -print0 >"$openclaw_system_launchd_entries"; then
while IFS= read -r -d '' openclaw_system_launchd_plist; do
# Unreadable plists are treated as foreign: loaded same-label daemons are caught by the
# bracketing launchctl probes; an unloaded unreadable same-label plist is an accepted operator-created edge (#120481).
if [ ! -r "$openclaw_system_launchd_plist" ]; then
continue
fi
if openclaw_system_launchd_plist_label=$(/usr/bin/plutil -extract Label raw -o - -- "$openclaw_system_launchd_plist" 2>&1); then
if [ "$openclaw_system_launchd_plist_label" != "$openclaw_system_launchd_label" ]; then
continue
@@ -115,6 +120,7 @@ type LaunchDaemonPlistLabelResult =
| { status: "ok"; label: string }
| { status: "unlabeled" }
| { status: "missing" }
| { status: "unreadable" }
| { status: "unverifiable"; detail: string };
/** Reads the top-level Label through the native parser for XML and binary plists. */
@@ -141,11 +147,15 @@ export async function readLaunchDaemonPlistLabel(
}
}
try {
await fs.access(plistPath);
await fs.access(plistPath, fs.constants.R_OK);
} catch (error) {
if (isMissingPathError(error)) {
return { status: "missing" };
}
const code = (error as NodeJS.ErrnoException | undefined)?.code;
if (code === "EACCES" || code === "EPERM") {
return { status: "unreadable" };
}
return { status: "unverifiable", detail: formatUnknownError(error) };
}
return {
@@ -178,6 +188,11 @@ async function findInstalledSystemLaunchDaemon(
if (result.status === "ok" && result.label === label) {
return { status: "installed", plistPath };
}
// Unreadable plists are treated as foreign: loaded same-label daemons are caught by the
// bracketing launchctl probes; an unloaded unreadable same-label plist is an accepted operator-created edge (#120481).
if (result.status === "unreadable") {
continue;
}
if (result.status === "unverifiable") {
return { status: "unverifiable", detail: `${plistPath}: ${result.detail}` };
}