mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(cli): verify launchd registration + skip self-SIGTERM in macOS autostart (#4765)
Integrated into release/v3.8.37 — cherry-picked defining commit onto release tip; tests green.
This commit is contained in:
committed by
GitHub
parent
0f3060729b
commit
b72a4d2fbb
@@ -204,6 +204,64 @@ export function isAutostartEnabled() {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure parse of `launchctl list <label>` output: true only when the agent's
|
||||
* "PID" line matches the given process id. launchctl prints the running PID for
|
||||
* a managed Aqua agent; a loaded-but-not-running agent omits the key entirely.
|
||||
*
|
||||
* Exported for unit testing — the darwin branch can't execute on the Linux CI.
|
||||
*/
|
||||
export function parseAgentSelfFromLaunchctl(output, pid) {
|
||||
if (!output) return false;
|
||||
const match = output.match(/"PID"\s*=\s*(\d+)/);
|
||||
return !!(match && parseInt(match[1], 10) === pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* True when `runList()` (which should run `launchctl list <label>`) succeeds,
|
||||
* i.e. launchd actually recognizes the agent. A plist sitting on disk that
|
||||
* launchd never loaded must NOT count as enabled — checking file existence
|
||||
* alone makes the tray menu lie ("✓ Enabled" while launchd has the agent in a
|
||||
* failed state or never loaded it).
|
||||
*
|
||||
* `runList` is injectable so the parse/branch logic is testable without a real
|
||||
* launchctl on the (Linux) CI runner.
|
||||
*/
|
||||
export function isLaunchdAgentLoaded(runList) {
|
||||
try {
|
||||
runList();
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when the current Node process IS the running instance launchd is
|
||||
* managing under our agent label.
|
||||
*
|
||||
* `launchctl unload`/`load -w` for a user-domain agent sends SIGTERM to the
|
||||
* running process. When the running OmniRoute cli was itself spawned by the
|
||||
* autostart launchd agent (autostart was enabled, then the machine rebooted,
|
||||
* then the user clicked the tray "Disable Autostart" item), an unload would
|
||||
* kill the very process executing the click handler — the tray icon would
|
||||
* silently disappear instead of the menu label flipping. Enable/disable skip
|
||||
* launchctl in that case; the on-disk plist change is what matters for the next
|
||||
* login, and launchd already has us loaded under our own PID.
|
||||
*/
|
||||
function isAgentSelfMac() {
|
||||
try {
|
||||
const output = execFileSync("launchctl", ["list", APP_LABEL], {
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
timeout: 3000,
|
||||
});
|
||||
return parseAgentSelfFromLaunchctl(output, process.pid);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function enableMac() {
|
||||
const plistDir = join(homedir(), "Library", "LaunchAgents");
|
||||
mkdirSync(plistDir, { recursive: true });
|
||||
@@ -225,6 +283,10 @@ function enableMac() {
|
||||
<key>KeepAlive</key><false/>
|
||||
</dict></plist>`;
|
||||
writeFileSync(plistPath, plist, { mode: 0o644 });
|
||||
// If we're already the running agent, launchctl load/unload would SIGTERM us.
|
||||
// The plist is updated on disk and launchd already has us loaded under our own
|
||||
// PID — nothing more to do for the current session.
|
||||
if (isAgentSelfMac()) return existsSync(plistPath);
|
||||
try {
|
||||
execSync("launchctl load -w " + JSON.stringify(plistPath), { stdio: "ignore" });
|
||||
} catch {}
|
||||
@@ -233,9 +295,15 @@ function enableMac() {
|
||||
|
||||
function disableMac() {
|
||||
const plistPath = join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`);
|
||||
try {
|
||||
execSync("launchctl unload -w " + JSON.stringify(plistPath), { stdio: "ignore" });
|
||||
} catch {}
|
||||
// Don't kill ourselves: when the current process is the running agent,
|
||||
// `launchctl unload` sends SIGTERM and a user clicking "Disable Autostart"
|
||||
// from the tray would lose the tray icon instead of just flipping the label.
|
||||
// Removing the plist file is enough to stop the agent at the next login.
|
||||
if (!isAgentSelfMac()) {
|
||||
try {
|
||||
execSync("launchctl unload -w " + JSON.stringify(plistPath), { stdio: "ignore" });
|
||||
} catch {}
|
||||
}
|
||||
try {
|
||||
unlinkSync(plistPath);
|
||||
} catch {}
|
||||
@@ -243,7 +311,16 @@ function disableMac() {
|
||||
}
|
||||
|
||||
function isEnabledMac() {
|
||||
return existsSync(join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`));
|
||||
const plistPath = join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`);
|
||||
if (!existsSync(plistPath)) return false;
|
||||
// The plist file existing is not enough — launchd must actually recognize the
|
||||
// agent, otherwise the tray menu reports "Enabled" for an inert/failed agent.
|
||||
return isLaunchdAgentLoaded(() =>
|
||||
execFileSync("launchctl", ["list", APP_LABEL], {
|
||||
stdio: ["ignore", "ignore", "ignore"],
|
||||
timeout: 3000,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
function enableWin() {
|
||||
|
||||
91
tests/unit/cli/autostart-macos-launchctl.test.ts
Normal file
91
tests/unit/cli/autostart-macos-launchctl.test.ts
Normal file
@@ -0,0 +1,91 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
parseAgentSelfFromLaunchctl,
|
||||
isLaunchdAgentLoaded,
|
||||
} from "../../../bin/cli/tray/autostart.mjs";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// parseAgentSelfFromLaunchctl — pure parse of `launchctl list <label>` output.
|
||||
// True only when the agent's "PID" line matches the current process PID, so the
|
||||
// enable()/disable() macOS paths can skip launchctl unload/load when the running
|
||||
// process IS the autostart agent (the unload would self-SIGTERM the tray).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("parseAgentSelfFromLaunchctl returns true when PID matches", () => {
|
||||
const output = `{
|
||||
\t"StandardOutPath" = "/tmp/omniroute.out.log";
|
||||
\t"PID" = 4242;
|
||||
\t"Label" = "com.omniroute.autostart";
|
||||
}`;
|
||||
assert.equal(parseAgentSelfFromLaunchctl(output, 4242), true);
|
||||
});
|
||||
|
||||
test("parseAgentSelfFromLaunchctl returns false when PID differs", () => {
|
||||
const output = `{
|
||||
\t"PID" = 9999;
|
||||
\t"Label" = "com.omniroute.autostart";
|
||||
}`;
|
||||
assert.equal(parseAgentSelfFromLaunchctl(output, 4242), false);
|
||||
});
|
||||
|
||||
test("parseAgentSelfFromLaunchctl returns false when no PID present", () => {
|
||||
// Loaded-but-not-running agent: launchctl omits the PID key.
|
||||
const output = `{
|
||||
\t"Label" = "com.omniroute.autostart";
|
||||
\t"LastExitStatus" = 0;
|
||||
}`;
|
||||
assert.equal(parseAgentSelfFromLaunchctl(output, 4242), false);
|
||||
});
|
||||
|
||||
test("parseAgentSelfFromLaunchctl tolerates whitespace variations", () => {
|
||||
assert.equal(parseAgentSelfFromLaunchctl('"PID"=4242;', 4242), true);
|
||||
assert.equal(parseAgentSelfFromLaunchctl('"PID" = 4242 ;', 4242), true);
|
||||
});
|
||||
|
||||
test("parseAgentSelfFromLaunchctl returns false on empty/garbage", () => {
|
||||
assert.equal(parseAgentSelfFromLaunchctl("", 4242), false);
|
||||
assert.equal(parseAgentSelfFromLaunchctl("not launchctl output", 4242), false);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// isLaunchdAgentLoaded — true only when `launchctl list <label>` succeeds.
|
||||
// This is what closes the false-"Enabled" gap: a plist on disk that launchd
|
||||
// does not actually recognize must report NOT enabled.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("isLaunchdAgentLoaded returns true when launchctl list succeeds", () => {
|
||||
const ran = isLaunchdAgentLoaded(() => {
|
||||
/* launchctl exited 0 — agent recognized */
|
||||
});
|
||||
assert.equal(ran, true);
|
||||
});
|
||||
|
||||
test("isLaunchdAgentLoaded returns false when launchctl list throws", () => {
|
||||
const ran = isLaunchdAgentLoaded(() => {
|
||||
throw new Error("Could not find service");
|
||||
});
|
||||
assert.equal(ran, false);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Source-level guard: isEnabledMac() must verify with launchctl, not just the
|
||||
// plist file existence. Mirrors the repo's existing Linux source-assertion test
|
||||
// (the darwin branch can't execute on the Linux CI runner).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("isEnabledMac verifies launchd registration, not just plist existence", () => {
|
||||
const source = readFileSync(join(process.cwd(), "bin/cli/tray/autostart.mjs"), "utf8");
|
||||
// The verification helper is wired into the darwin enabled-check.
|
||||
assert.match(source, /isLaunchdAgentLoaded/);
|
||||
// launchctl list is queried with the app label, no shell interpolation.
|
||||
assert.match(source, /launchctl",\s*\["list",\s*APP_LABEL\]/);
|
||||
});
|
||||
|
||||
test("enable/disable macOS skip launchctl when the current process is the agent", () => {
|
||||
const source = readFileSync(join(process.cwd(), "bin/cli/tray/autostart.mjs"), "utf8");
|
||||
assert.match(source, /isAgentSelfMac/);
|
||||
assert.match(source, /parseAgentSelfFromLaunchctl/);
|
||||
});
|
||||
Reference in New Issue
Block a user