fix(service): stop the test suite from mutating a live service manager

devlog/_plan/260910_live_service_manager_guard/000_plan.md

@@ -0,0 +1,56 @@
+# Live service-manager guard
+
+## What happened
+
+A translation task took the maintainer's running proxy down four times in one night, and nobody
+connected the two for hours. The immediate cause was not a test: a delegated agent wrote a long
+README through a double-quoted `python3 -c` string, and the README contains inline code spans such
+as \`ocx service\`, \`ocx stop\` and \`ocx service uninstall\`. Inside a double-quoted shell
+string a backtick is command substitution, so those ran.
+
+Chasing that down surfaced a second, independent hazard that had been sitting in the suite the
+whole time.
+
+## The hazard
+
+`tests/preload.ts` sandboxes `HOME`, `OPENCODEX_HOME` and `CODEX_HOME` on every invocation,
+including a bare `bun test <file>`. That covers everything addressed by a path.
+
+A service manager is not addressed by a path. `systemctl --user stop opencodex-proxy.service`
+addresses a job by name and talks to the user manager that is already running.
+`launchctl bootout gui/<uid>/com.opencodex.proxy` talks to launchd the same way. Neither consults
+`HOME`, so a test that falls through to either one reaches the live service however well the home
+is isolated.
+
+Windows already refused this. `querySchtasks` in `src/service.ts` throws on every non-query call
+while the test-home guard is armed, after a partially-faked service test replaced a real scheduled
+task with a launcher inside a temporary test home — the test passed, and cleanup deleted the
+launcher. macOS and Linux never got the equivalent, which left the person most likely to run this
+suite, someone running opencodex on the machine they develop it on, as the one it can disrupt.
+
+## The change
+
+`sh()` is the choke point rather than each call site, so a `systemctl` or `launchctl` call added
+later is covered without anyone remembering to guard it. The real `runLaunchctl` runner is guarded
+too, since it spawns `/bin/launchctl` directly.
+
+Three properties keep it from being disruptive in the other direction:
+
+- Read-only verbs stay allowed. `launchctl list`, `launchctl print`, `systemctl --user show`,
+  `is-active`, `is-enabled`, `status` and `show-environment` are what the diagnostics are built
+  on, and observation cannot take a service down.
+- An injected `spawnSync` stand-in is untouched, so the existing `runLaunchctl` and `startLaunchd`
+  parsing tests keep working unchanged.
+- Arming requires `OCX_TEST_HOME_GUARD=1`, which only this repository's test preload sets, so a
+  user running `ocx service restart` is unaffected.
+
+## Verification
+
+Local execution was skipped deliberately: the suite is what reaches a live service manager, and the
+machine this was written on is running opencodex. CI on the pushed head is the evidence.
+
+## What this does not fix
+
+The incident that started this was an agent executing README text through a shell. This guard would
+not have stopped it. That belongs to how agents write files, and it is recorded in
+`devlog/_plan/260910_readme_i18n_parity/020_phase2_locale_resync.md`.

scripts/test-layout/layout.json

@@ -786,6 +786,7 @@
     "lab-read-filter-validation.test.ts": "lab",
     "lab-read-surfaces.test.ts": "lab",
     "legacy-shell-compat.test.ts": "responses",
+    "live-service-manager-guard.test.ts": "service",
     "local-management-attestation.test.ts": "server",
     "local-management-capability.test.ts": "server",
     "local-management-direct-transport.test.ts": "server",

src/service.ts

@@ -865,9 +865,51 @@ export function resolvedProxyEnv(env: NodeJS.ProcessEnv = process.env): { name:
 }
 
 function sh(cmd: string): string {
+  assertLiveServiceManagerAllowed(cmd);
   return execSync(cmd, { encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] }).trim();
 }
 
+/**
+ * Service-manager invocations that only observe. Everything else changes a job that
+ * launchd or the systemd user manager is running right now.
+ */
+const READ_ONLY_SERVICE_MANAGER = new RegExp(
+  "^(?:launchctl\\s+(?:list|print|print-disabled|blame|managerpid|manageruid)\\b"
+  + "|systemctl\\s+(?:--user\\s+)?(?:show|show-environment|status|is-active|is-enabled|is-failed|cat|list-units|list-unit-files|--version)\\b)",
+);
+
+const SERVICE_MANAGER_COMMAND = /^(?:launchctl|systemctl)\b/;
+
+/**
+ * Refuse to mutate a live service manager from an armed test process.
+ *
+ * The test preload isolates HOME, OPENCODEX_HOME and CODEX_HOME, and that is enough for
+ * anything addressed by path. It is not enough here. `systemctl --user stop
+ * opencodex-proxy.service` is addressed by job NAME and talks to the user manager that is
+ * already running, so it stops the proxy the developer is actually using no matter what
+ * HOME says. `launchctl bootout gui/<uid>/com.opencodex.proxy` has the same shape.
+ *
+ * Windows already had this guard: `querySchtasks` refuses every non-query call while the
+ * test-home guard is armed, after a partially-faked test replaced a real scheduled task
+ * with a launcher inside a temporary test home. macOS and Linux were left without the
+ * equivalent, which means the person most likely to run this suite - someone running
+ * opencodex on the machine they are developing it on - is the person it can disrupt.
+ *
+ * Read-only verbs stay allowed: probing what the manager reports is the whole point of
+ * the diagnostics, and observation cannot take a service down.
+ */
+export function assertLiveServiceManagerAllowed(command: string): void {
+  if (!isTestHomeGuardArmed()) return;
+  const trimmed = command.trim();
+  if (!SERVICE_MANAGER_COMMAND.test(trimmed)) return;
+  if (READ_ONLY_SERVICE_MANAGER.test(trimmed)) return;
+  throw new Error(
+    `refusing to run \`${trimmed}\` from an armed test process: launchd and the systemd user `
+    + "manager address a job by name, not by HOME, so this reaches the service the developer is "
+    + "actually running. Inject the service operation instead of calling the live manager.",
+  );
+}
+
 /**
  * Run `launchctl` and report BOTH streams regardless of exit status.
  *
@@ -887,6 +929,9 @@ export function runLaunchctl(
   deps: { run?: typeof spawnSync } = {},
 ): { ok: boolean; stdout: string; stderr: string; status: number | null } {
   const run = deps.run ?? spawnSync;
+  // Only the real runner is guarded. Tests that inject a spawnSync stand-in are
+  // exercising the parsing, not reaching launchd, and must keep working.
+  if (run === spawnSync) assertLiveServiceManagerAllowed(`launchctl ${args.join(" ")}`);
   const result = run("/bin/launchctl", args, { encoding: "utf8", windowsHide: true });
   // `error` is set when the spawn itself failed (ENOENT off macOS) and `status` is
   // null for a signalled child; neither may be reported as success.

tests/fixtures/test-layout-expected.json

@@ -621,6 +621,7 @@
   "lab-read-filter-validation.test.ts": "lab",
   "lab-read-surfaces.test.ts": "lab",
   "legacy-shell-compat.test.ts": "responses",
+  "live-service-manager-guard.test.ts": "service",
   "local-management-attestation.test.ts": "server",
   "local-management-capability.test.ts": "server",
   "local-management-direct-transport.test.ts": "server",

tests/preload.ts

@@ -10,6 +10,13 @@
  *
  * Import order below is load-bearing: importing the guard captures the real home at
  * module load, and that must happen BEFORE this file replaces HOME.
+ *
+ * What this file cannot do: HOME isolation only protects what is addressed by a path. A
+ * service manager is addressed by a job name — `systemctl --user stop
+ * opencodex-proxy.service` reaches the user manager that is already running, and
+ * `launchctl bootout gui/<uid>/com.opencodex.proxy` reaches launchd — so neither cares
+ * what HOME says. `assertLiveServiceManagerAllowed` in `src/service.ts` is the guard for
+ * that, armed by the same flag set below.
  */
 import { isTestHomeGuardArmed, protectedHomeForTests } from "../src/lib/test-home-guard";
 import { createIsolatedTestEnvironment } from "../scripts/test";

tests/service/live-service-manager-guard.test.ts

@@ -0,0 +1,73 @@
+import { afterEach, describe, expect, test } from "bun:test";
+
+import { assertLiveServiceManagerAllowed } from "../../src/service";
+
+/**
+ * The suite must not be able to stop the proxy the developer is running.
+ *
+ * HOME isolation, which the test preload already does, covers everything addressed by
+ * path. It does not cover a service manager addressed by job name: `systemctl --user stop
+ * opencodex-proxy.service` talks to the user manager that is already running, and
+ * `launchctl bootout gui/<uid>/com.opencodex.proxy` talks to launchd, and neither of them
+ * consults HOME. Windows has refused this since a partially-faked test replaced a real
+ * scheduled task; macOS and Linux did not, so the person running opencodex on the machine
+ * they develop it on was the one exposed.
+ */
+const GUARD_ENV = "OCX_TEST_HOME_GUARD";
+const original = process.env[GUARD_ENV];
+
+afterEach(() => {
+  if (original === undefined) delete process.env[GUARD_ENV];
+  else process.env[GUARD_ENV] = original;
+});
+
+describe("live service-manager guard", () => {
+  test("refuses every mutating launchctl and systemctl call while armed", () => {
+    process.env[GUARD_ENV] = "1";
+    const mutations = [
+      "launchctl unload /tmp/LaunchAgents/com.opencodex.proxy.plist",
+      "launchctl load -w /tmp/LaunchAgents/com.opencodex.proxy.plist",
+      "launchctl bootout gui/501/com.opencodex.proxy",
+      "launchctl kickstart -k gui/501/com.opencodex.proxy",
+      "systemctl --user stop opencodex-proxy.service",
+      "systemctl --user restart opencodex-proxy.service",
+      "systemctl --user disable opencodex-proxy.service",
+      "systemctl --user enable opencodex-proxy.service",
+      "systemctl --user daemon-reload",
+    ];
+    for (const command of mutations) {
+      expect(() => assertLiveServiceManagerAllowed(command)).toThrow(/armed test process/);
+    }
+  });
+
+  test("still allows observation, which is what the diagnostics need", () => {
+    process.env[GUARD_ENV] = "1";
+    const observations = [
+      "launchctl list",
+      "launchctl list | grep com.opencodex.proxy || true",
+      "launchctl print gui/501/com.opencodex.proxy",
+      "systemctl --version",
+      "systemctl --user show -p NeedDaemonReload opencodex-proxy.service",
+      "systemctl --user is-active opencodex-proxy.service",
+      "systemctl --user is-enabled opencodex-proxy.service",
+      "systemctl --user status opencodex-proxy.service",
+      "systemctl --user show-environment",
+    ];
+    for (const command of observations) {
+      expect(() => assertLiveServiceManagerAllowed(command)).not.toThrow();
+    }
+  });
+
+  test("leaves unrelated commands alone", () => {
+    process.env[GUARD_ENV] = "1";
+    expect(() => assertLiveServiceManagerAllowed("git status")).not.toThrow();
+    expect(() => assertLiveServiceManagerAllowed("sw_vers -productVersion")).not.toThrow();
+  });
+
+  test("is inert in production, where the guard is not armed", () => {
+    delete process.env[GUARD_ENV];
+    expect(() =>
+      assertLiveServiceManagerAllowed("systemctl --user stop opencodex-proxy.service"),
+    ).not.toThrow();
+  });
+});