From 102de58e7547cb30391c724795c67a6b1732b9bf Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 06:20:24 +0900 Subject: [PATCH 1/4] fix(service): stop the test suite from mutating a live service manager Running this suite could take down the proxy the developer is running. The test preload isolates HOME, OPENCODEX_HOME and CODEX_HOME, which covers everything addressed by a path, but a service manager is addressed by job name: systemctl --user stop opencodex-proxy.service talks to the user manager that is already running, and launchctl bootout gui//com.opencodex.proxy talks to launchd. Neither consults HOME. Windows has refused this since a partially-faked service test replaced a real scheduled task with a launcher inside a temporary test home: querySchtasks throws on every non-query call while the test-home guard is armed. macOS and Linux never got the equivalent, so the person most likely to run the suite - someone running opencodex on the machine they develop it on - was the one exposed. sh() and the real runLaunchctl runner now refuse a mutating launchctl or systemctl invocation while the guard is armed. Read-only verbs stay allowed because observation is what the diagnostics are for and cannot take a service down, an injected spawnSync stand-in is untouched so the existing parsing tests keep working, and the whole check is inert unless OCX_TEST_HOME_GUARD is set, which only this repository's test preload does. --- scripts/test-layout/layout.json | 1 + src/service.ts | 45 ++++++++++++ tests/fixtures/test-layout-expected.json | 1 + .../live-service-manager-guard.test.ts | 73 +++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 tests/service/live-service-manager-guard.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index a82018a5ff..f32d9bcac6 100644 --- a/scripts/test-layout/layout.json +++ b/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", diff --git a/src/service.ts b/src/service.ts index 1fa97c424d..d2ab05299d 100644 --- a/src/service.ts +++ b/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//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. diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index b5fc9651da..85c041523e 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/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", diff --git a/tests/service/live-service-manager-guard.test.ts b/tests/service/live-service-manager-guard.test.ts new file mode 100644 index 0000000000..2714f2dc5c --- /dev/null +++ b/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//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 /Users/someone/Library/LaunchAgents/com.opencodex.proxy.plist", + "launchctl load -w /Users/someone/Library/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(); + }); +}); From 00054e5d92e97a565ac5c9cfd3ef4e19f91fdaca Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 06:32:16 +0900 Subject: [PATCH 2/4] test(service): keep home-shaped paths out of the guard fixture privacy:scan rejects a /Users// literal anywhere in the tree; the plist path in this fixture only has to be a path launchctl would accept. --- tests/service/live-service-manager-guard.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/service/live-service-manager-guard.test.ts b/tests/service/live-service-manager-guard.test.ts index 2714f2dc5c..31104da247 100644 --- a/tests/service/live-service-manager-guard.test.ts +++ b/tests/service/live-service-manager-guard.test.ts @@ -25,8 +25,8 @@ describe("live service-manager guard", () => { test("refuses every mutating launchctl and systemctl call while armed", () => { process.env[GUARD_ENV] = "1"; const mutations = [ - "launchctl unload /Users/someone/Library/LaunchAgents/com.opencodex.proxy.plist", - "launchctl load -w /Users/someone/Library/LaunchAgents/com.opencodex.proxy.plist", + "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", From a45c155901e22c573d91be3d0abac667495a2d97 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 06:49:16 +0900 Subject: [PATCH 3/4] docs(devlog): record the live service-manager guard unit --- .../000_plan.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 devlog/_plan/260910_live_service_manager_guard/000_plan.md diff --git a/devlog/_plan/260910_live_service_manager_guard/000_plan.md b/devlog/_plan/260910_live_service_manager_guard/000_plan.md new file mode 100644 index 0000000000..817da13a7d --- /dev/null +++ b/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 `. 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//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`. From 3ab6b97ee4f774d9f822b20a6c0994bd25756804 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 06:49:52 +0900 Subject: [PATCH 4/4] docs(tests): say what HOME isolation does not cover The preload header is where someone learns how this suite protects a real machine, so it is where the limit belongs: path isolation does nothing for a service manager addressed by job name. --- tests/preload.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/preload.ts b/tests/preload.ts index 01717a0f52..a848a4aaf0 100644 --- a/tests/preload.ts +++ b/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//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";