From 4420329c3d165ef5d11c5db70df7c8ae15962511 Mon Sep 17 00:00:00 2001 From: kerthans <112744990+kerthans@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:52:36 +0800 Subject: [PATCH 1/3] fix(stop): read an absent tracked-record hostname as unknown A default-bind install has no configured hostname for the writer to persist, so runtime-port.json legitimately omits the optional field. Liveness discovery defaults that absence to 127.0.0.1 for its verdict, while tracked-runtime-inspection coerced it to "", so the guarded stop compared "" against the loopback the desktop had approved and refused every takeover with "the tracked runtime record changed" -- without ever signalling the runtime, so a retry could not differ. Keep the probe's hostname optional and unset when the record has none, compare bind scope through probeHostname only when the record carries a hostname, and persist the scope the listener actually bound instead of the raw config value so new records state it. Fixes #8181 --- src/cli/index.ts | 5 +++- src/cli/stop-approval.ts | 34 +++++++++++++++++++++--- src/config/tracked-runtime-inspection.ts | 19 ++++++++++--- structure/config.md | 2 +- structure/runtime.md | 2 +- tests/cli/cli-stop-json.test.ts | 33 +++++++++++++++++++++++ tests/service/process-state.test.ts | 12 +++++++++ 7 files changed, 98 insertions(+), 9 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index b95c6fdbd39..d949cba5afb 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -606,7 +606,10 @@ async function handleStart(options: { block?: boolean } = {}) { writeRuntime: bound => { updateRestartChild?.check(bound.port, bound.config.hostname ?? ""); writeRuntimePort({ pid: process.pid, port: bound.port, - hostname: bound.config.hostname, + /** Persist the scope the listener actually bound (127.0.0.1 for a default install), + * not the raw config value: an omitted field is unknown to the guarded stop, which + * only compares a hostname the record actually carries. */ + hostname: bound.server.hostname, attestationSecret: bound.localAttestationSecret, command: currentServingCommand(), ...siblingRuntimeField(), diff --git a/src/cli/stop-approval.ts b/src/cli/stop-approval.ts index 5a68879e8d5..91adf9c43f5 100644 --- a/src/cli/stop-approval.ts +++ b/src/cli/stop-approval.ts @@ -27,7 +27,7 @@ export interface GuardedStopSnapshot { * inspection probe -- it carries no path, command line, stderr, or token. */ export type TrackedRuntimeCheck = - | { pid: number; port: number; hostname: string } + | { pid: number; port: number; hostname: string | undefined } | { untracked: string } | null; @@ -125,6 +125,27 @@ export function managerStillActive( } }; } +/** + * Re-verify every approved fact before letting the caller stop anything. + * + * The approval is a snapshot of what the desktop saw, so each fact is re-read + * here and a difference returns `approval-changed` without stopping: the live + * endpoint and its role, the effective port, the config home, the resolving CLI, + * the ownership claim, takeover compatibility and the compatibility token, then + * the manager identity and the tracked-record pair. PID and port must match the + * record exactly. The hostname compares bind scope, and only when the record + * actually carries one: the field is optional, a default-bind install has no + * configured value to persist, and the endpoint check above already proved the + * live host. Treating that absence as drift refused every takeover on such an + * install, and no retry could differ because the running runtime never rewrites + * the record. + * + * @param expected The approved snapshot the caller is acting on. + * @param read Re-resolve the runtime as the desktop would; null when unreadable. + * @param tracked Re-read the tracked record pair the stop will signal. + * @param managerTarget Re-bind an installed manager's process tree, if any. + * @param stop The approved stop, called only after every check above holds. + */ export async function runApprovedStop( expected: StopApproval, read: () => Promise, @@ -144,10 +165,17 @@ export async function runApprovedStop( if ("untracked" in target) { return approvalChanged(`the tracked runtime record could not be verified (${target.untracked})`); } - if (target.pid !== expected.pid || target.port !== expected.port - || target.hostname !== expected.hostname) { + if (target.pid !== expected.pid || target.port !== expected.port) { return approvalChanged("the tracked runtime record changed"); } + // A record that omits the hostname says nothing about bind scope, and the live endpoint + // above already matched the approved host, so unknown must not read as drift. When the + // record does carry one, compare bind scope rather than spelling so a wildcard record and + // the loopback it defaults to are the same scope. + if (target.hostname !== undefined + && probeHostname(target.hostname) !== probeHostname(expected.hostname)) { + return approvalChanged("the tracked runtime record names a different bind hostname"); + } if (manager.kind === "unknown") { return approvalChanged(`the service manager could not be re-verified (${manager.reason})`); } diff --git a/src/config/tracked-runtime-inspection.ts b/src/config/tracked-runtime-inspection.ts index 6cc241e5f19..887a1ae959a 100644 --- a/src/config/tracked-runtime-inspection.ts +++ b/src/config/tracked-runtime-inspection.ts @@ -31,6 +31,13 @@ * - No path, command line, stderr text, or token leaves this module: the refusal * causes below are fixed strings and the detail built from them is a fixed * sentence. + * + * The verified target's "hostname" is optional and stays undefined when the + * record omits it. A default-bind install has no configured value for the writer + * to persist, so absence is the normal shape there, and it means "unknown" rather + * than an empty host: callers compare it by bind scope only when it is present, + * so a record that never stored one cannot read as drift against the loopback + * liveness discovery reports for the same runtime. */ import { readFileSync } from "node:fs"; import { @@ -59,7 +66,7 @@ export type TrackedRecordRefusal = | "unverified"; export type TrackedRecordInspection = - | { status: "verified"; pid: number; port: number; hostname: string } + | { status: "verified"; pid: number; port: number; hostname: string | undefined } | { status: "refused"; reason: TrackedRecordRefusal }; /** @@ -147,7 +154,10 @@ export function inspectTrackedRuntimeRecord( if (parsed.pid !== pid) { return { status: "refused", reason: "runtime-record-pid-mismatch" }; } - return { status: "verified", pid, port: parsed.port, hostname: parsed.hostname ?? "" }; + // "hostname" is optional in the record: older writers omit it, and a default bind has no + // configured value to persist. Absence means "unknown", not an empty host, so the caller + // must be able to tell the two apart instead of comparing "" against a discovered endpoint. + return { status: "verified", pid, port: parsed.port, hostname: parsed.hostname }; } catch { return { status: "refused", reason: "unverified" }; } @@ -177,10 +187,13 @@ const TRACKED_REFUSAL_CAUSES: Record = { /** * The guarded stop's tracked() callback shape: the verified target, or the * fixed cause of the single check that failed. Read-only; decides nothing. + * + * "hostname" is undefined when the record omitted it; the guard reads that as + * unknown scope, not as an empty one. */ export function describeTrackedRuntimeTarget( deps: TrackedRecordProbeDeps = {}, -): { pid: number; port: number; hostname: string } | { untracked: string } { +): { pid: number; port: number; hostname: string | undefined } | { untracked: string } { const inspection = inspectTrackedRuntimeRecord(deps); return inspection.status === "verified" ? { pid: inspection.pid, port: inspection.port, hostname: inspection.hostname } diff --git a/structure/config.md b/structure/config.md index a59d456ba66..77beb66326f 100644 --- a/structure/config.md +++ b/structure/config.md @@ -68,7 +68,7 @@ silently move the active installation. Config reads in this leaf and `src/config It owns their byte-compatible writes, parsing, expected-PID filters, cheap liveness, full OCX command identity, and snapshot-guarded removal. `RuntimePortState.attestationSecret` and `siblingOfPort` (the live owner's port, written only by a sibling instance) remain optional, owner-only state and are validated before a record is returned. `src/config.ts` re-exports the same -symbols for compatibility, but new lifecycle-only callers import the process-state leaf directly. `src/config/tracked-runtime-inspection.ts` is the guarded stop's read-only companion probe: it re-runs the same record reads in the same order, answers the start-identity question through the same cache `readPid()` consults, and reports which single check failed as a fixed refusal cause, so an absent, denied, invalid or PID-mismatched record is told apart from a dead, foreign or unverifiable process, and it decides nothing and exposes no path, command line, stderr, or token. +symbols for compatibility, but new lifecycle-only callers import the process-state leaf directly. `src/config/tracked-runtime-inspection.ts` is the guarded stop's read-only companion probe: it re-runs the same record reads in the same order, answers the start-identity question through the same cache `readPid()` consults, and reports which single check failed as a fixed refusal cause, so an absent, denied, invalid or PID-mismatched record is told apart from a dead, foreign or unverifiable process, and it decides nothing and exposes no path, command line, stderr, or token. Its verified target reports the record's optional `hostname` as `string | undefined`, so a record written without that field stays unknown instead of reading as an empty host and disagreeing with every listener that defaults to loopback. Replacing config and process-state writes use `src/config/atomic-write.ts`. Normal Windows writes opt into the destination's single ACL timeout recovery attempt; a consumed attempt re-arms after `TIMEOUT_MEMO_REARM_MS` in `src/lib/windows-secret-acl.ts`. Default writers track temporary paths per resolved destination across synchronous, streamed and asynchronous writes. A new write refuses immediately with sanitized `ETIMEDOUT` only while one of that destination's tracked paths has an icacls child retained past its caller's timeout belt and not yet reaped, including across re-arm windows. Healthy overlapping writes proceed, including during ACL hardening or a pause before rename. Refusal consumes no recovery attempt and leaves removal ownership intact; completing one write does not drop another write's retained-child tracking. Both cleanup paths leave that recovery unconsumed. Every admitted attempt still requires a successful harden before secret bytes or publication; non-timeout failures retain their existing fail-closed behavior. `tests/windows/windows-atomic-write-acl-rearm.test.ts` covers all five harden calls, the re-arm boundary, healthy overlap, retained child admission and cleanup. The leaf preserves the shared process-wide temp sequence, optional synchronous caller destination validation before every rename attempt, symlink target resolution, no-follow directory-entry replacement for diff --git a/structure/runtime.md b/structure/runtime.md index 0ab790f55b5..719ceb01fb5 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -59,7 +59,7 @@ The [CLI management contract](cli-management.md#cli-readiness-diagnostics) owns `ocx resolve` (`src/cli/resolve.ts`) is the machine surface a desktop shell asks instead of resolving the config home, the port, and liveness itself: the home comes from `src/config/paths.ts`, the effective port is the live listener's when the identity-checked `findLiveProxy` answers and the configured `config.port ?? 10100` otherwise, and the liveness verdict is that same module's output (pid, runtime-versus-config provenance, version, role). Config reads go through `readConfigDiagnostics`, not `loadConfig`: a missing file is defaults, but an invalid file exits 1 instead of being repaired to defaults, because a defaulted port is a guess the caller must refuse. Liveness is three-valued: when `findLiveProxy` returns null, resolve re-asks the endpoints with `endpointsToProve` and `everyEndpointProvenDown`, using the in-process `probeEndpointLiveness` from `src/server/proxy-liveness.ts`. Only unanimous definitive "dead" becomes `absent-proven`; a timeout or reset stays unknown, exits 1 and never authorises a start. The transient exclusive-bind fallback in `src/update/proxy-liveness-probe.mjs` belongs only to the npm/Bun updater. Resolve stays in-process because a compiled standalone binary cannot run a JavaScript probe with `execPath -e`. Discovery borrows `START_OWNERSHIP_LIVENESS`, the start path's ownership budget — the verdict feeds the shell's launch decision, so the cost of a false "nobody listening" is the duplicate proxy (#5004). The verb is in `skipsCodexShimAutoRestore`, so a read-only lookup never triggers a shim repair. Arguments are pre-parsed in `src/cli/root.ts` and exit 64 before any preflight side effect, the same ordering `ocx ready` obeys. `ocx-resolve/1` also reports `ownership` (`none`, `owned`, or `unknown`) and `takeover` (`supported` with protocol version, minimum CLI version and compatibility token, or `blocked` with a reason). A known liveness verdict still exits 0 when ownership is unknown, but takeover is blocked. An unreadable second service-state read blocks takeover instead of becoming an absent registration. `src/service/managing-cli.ts` observes the registered and selected PATH CLIs, including Windows PATHEXT order and file validation, before compatibility is offered. A `--version` probe is retried once on Windows — one transient failure must not read as an unobservable CLI — and a Windows `.CMD`/`.BAT` shim is invoked through a single verbatim-quoted `cmd /c` line, so a spaced executable path and spaced arguments both survive cmd's re-parse. `src/cli/route-policy.ts` distinguishes a missing profile (exit 4) from malformed `show` arguments (exit 2); an upstream failure propagates through the management error mapping. -`ocx stop --json` is a reporting layer over the unchanged stop path. `src/cli/index.ts` threads a `StopRunRecord` through the existing receipt, drain, respawn-verification and restore flow, and `src/cli/stop-report.ts` maps the recorded facts plus the signals that already pick the exit code into one versioned document (`schema: "ocx-stop/1"`). With `--json` the human lines print on stderr and stdout carries only that document; exit codes 0/1/79/80/81 cross the process boundary unchanged. The desktop's opt-in guarded stop adds exact approved PID, endpoint, home, CLI version and compatibility-token expectations; plain `ocx stop` retains its ordinary behavior. Under the ownership mutation lease, `src/cli/stop-approval.ts` re-resolves them and `src/service/guarded-manager-target.ts` binds any installed manager's process tree to the approved PID before a stop action. The same guard re-reads `ocx.pid` and `runtime-port.json` through `src/config/tracked-runtime-inspection.ts`, a single-pass read-only probe that walks the same checks `readPid()` and `readRuntimePort(pid)` perform and reports which one failed, answering the recorded process's start-identity question through the same cache `readPid()` fills so it cannot disagree with the admission it diagnoses. A pre-action mismatch returns `approval-changed` without stopping, and the summary carries `detail` naming the single guard fact that failed — live PID/port/hostname/CLI drift, an unverifiable ownership claim, a blocked takeover, the compatibility fingerprint, or the tracked-record cause — an absent, denied, unreadable or invalid record, a dead, foreign or unverifiable recorded process, or a record naming a different PID — so a transient re-probe can be told from genuine drift in the caller's log. The tracked-record causes are fixed strings: denied is reported only on an observed EACCES/EPERM, an unreadable command line stays unverifiable rather than attributed to an account or elevation, and no probed path, command line, stderr or token reaches the detail. Immediately before any manager stop or direct PID signal, the CLI rechecks the same manager identity or proven absence; a change also returns `approval-changed`. A non-OpenCodex process replacing the OS job in the remaining instant before the manager command is a residual same-user risk outside the ownership lease. The guarded path stops the bound manager, or the approved PID directly when managers are proven absent, then uses a five-second shared deadline to verify PID exit, port availability and definitive endpoint absence. Only after settlement does a read-only manager probe require launchd `not-loaded` in both domains or systemd `inactive` with `MainPID=0`; the state is checked again before a success summary. Timeout, active or unreadable manager state returns terminal `manager-still-active`, even if the endpoint briefly refuses. Windows managers bind through `src/service/windows-guarded-manager.ts`: a Task Scheduler backend must prove a recognized OpenCodex registration against the bounded current-user SID/name lookup, a running task, and a wrapper ancestor carrying this home's canonical paths — presence and running state are read root-folder-scoped because every `/tn` operation resolves a bare name there and nowhere else, so a same-named task inside a task folder is a different registration neither the probe nor the stop can reach; the registered definition itself is read only on the running path, and a state or registration read that cannot answer re-probes presence so a task deleted between probes — which can respawn nothing — reads absent while a merely unreadable one stays unknown; an unreadable current identity leaves the manager unknown; a WinSW backend must prove this installation's binary and SCM service PID ancestry; either marks `childNeedsSeparateStop`, so the approved PID receives its own graceful signal only after a successful manager stop because `schtasks /end` does not reliably cascade to the wrapper's child (#764); a failed or unknown manager stop leaves the child untouched. Post-stop Windows inactivity means the task not running, no surviving wrapper process, and no started WinSW — never mere registration presence; a running-state read that cannot answer re-probes presence the same way, so a task deleted between probes reports inactive while a persisting one stays unknown. Every Windows manager command a verdict or stop depends on — schtasks, sc.exe, WinSW verbs — runs under a bounded `execFileSync` timeout so a wedged Task Scheduler or Service Control Manager cannot hang the guarded stop; a killed command classifies through the same fail-closed paths a nonzero exit would (the interactive `install /p` password prompt is the one deliberate exception). A `wscript.exe`, `cscript.exe` or `cmd.exe` whose command line CIM cannot read makes the state unprovable only when it is tied to the proxy: pre-stop, as an ancestor of the approved PID; post-stop, as the former bound manager process or a descendant of it. Unrelated unreadable shells, which a non-admin query returns for other users' and elevated processes, are not evidence either way. An unprovable Windows manager still blocks guarded takeover. The token checks snapshot consistency, not whether a person approved the desktop prompt. `src/cli/stop-restore.ts` owns the shared Codex/Grok restoration and its history-only, history-deferred, and other failure categories. +`ocx stop --json` is a reporting layer over the unchanged stop path. `src/cli/index.ts` threads a `StopRunRecord` through the existing receipt, drain, respawn-verification and restore flow, and `src/cli/stop-report.ts` maps the recorded facts plus the signals that already pick the exit code into one versioned document (`schema: "ocx-stop/1"`). With `--json` the human lines print on stderr and stdout carries only that document; exit codes 0/1/79/80/81 cross the process boundary unchanged. The desktop's opt-in guarded stop adds exact approved PID, endpoint, home, CLI version and compatibility-token expectations; plain `ocx stop` retains its ordinary behavior. Under the ownership mutation lease, `src/cli/stop-approval.ts` re-resolves them and `src/service/guarded-manager-target.ts` binds any installed manager's process tree to the approved PID before a stop action. The same guard re-reads `ocx.pid` and `runtime-port.json` through `src/config/tracked-runtime-inspection.ts`, a single-pass read-only probe that walks the same checks `readPid()` and `readRuntimePort(pid)` perform and reports which one failed, answering the recorded process's start-identity question through the same cache `readPid()` fills so it cannot disagree with the admission it diagnoses. Its verified target keeps `hostname` optional and unset when the record omits it, because a default-bind install has no configured value for the writer to persist; absence there is unknown rather than an empty host, so a record that never stored one cannot read as drift against the loopback discovery reports for the same runtime. When the record does carry a hostname the guard compares bind scope through `probeHostname`, so a wildcard record and the loopback it also serves remain the same scope, and a record naming a genuinely different host still returns `approval-changed` with the mismatched label. A pre-action mismatch returns `approval-changed` without stopping, and the summary carries `detail` naming the single guard fact that failed — live PID/port/hostname/CLI drift, an unverifiable ownership claim, a blocked takeover, the compatibility fingerprint, or the tracked-record cause — an absent, denied, unreadable or invalid record, a dead, foreign or unverifiable recorded process, or a record naming a different PID — so a transient re-probe can be told from genuine drift in the caller's log. The tracked-record causes are fixed strings: denied is reported only on an observed EACCES/EPERM, an unreadable command line stays unverifiable rather than attributed to an account or elevation, and no probed path, command line, stderr or token reaches the detail. Immediately before any manager stop or direct PID signal, the CLI rechecks the same manager identity or proven absence; a change also returns `approval-changed`. A non-OpenCodex process replacing the OS job in the remaining instant before the manager command is a residual same-user risk outside the ownership lease. The guarded path stops the bound manager, or the approved PID directly when managers are proven absent, then uses a five-second shared deadline to verify PID exit, port availability and definitive endpoint absence. Only after settlement does a read-only manager probe require launchd `not-loaded` in both domains or systemd `inactive` with `MainPID=0`; the state is checked again before a success summary. Timeout, active or unreadable manager state returns terminal `manager-still-active`, even if the endpoint briefly refuses. Windows managers bind through `src/service/windows-guarded-manager.ts`: a Task Scheduler backend must prove a recognized OpenCodex registration against the bounded current-user SID/name lookup, a running task, and a wrapper ancestor carrying this home's canonical paths — presence and running state are read root-folder-scoped because every `/tn` operation resolves a bare name there and nowhere else, so a same-named task inside a task folder is a different registration neither the probe nor the stop can reach; the registered definition itself is read only on the running path, and a state or registration read that cannot answer re-probes presence so a task deleted between probes — which can respawn nothing — reads absent while a merely unreadable one stays unknown; an unreadable current identity leaves the manager unknown; a WinSW backend must prove this installation's binary and SCM service PID ancestry; either marks `childNeedsSeparateStop`, so the approved PID receives its own graceful signal only after a successful manager stop because `schtasks /end` does not reliably cascade to the wrapper's child (#764); a failed or unknown manager stop leaves the child untouched. Post-stop Windows inactivity means the task not running, no surviving wrapper process, and no started WinSW — never mere registration presence; a running-state read that cannot answer re-probes presence the same way, so a task deleted between probes reports inactive while a persisting one stays unknown. Every Windows manager command a verdict or stop depends on — schtasks, sc.exe, WinSW verbs — runs under a bounded `execFileSync` timeout so a wedged Task Scheduler or Service Control Manager cannot hang the guarded stop; a killed command classifies through the same fail-closed paths a nonzero exit would (the interactive `install /p` password prompt is the one deliberate exception). A `wscript.exe`, `cscript.exe` or `cmd.exe` whose command line CIM cannot read makes the state unprovable only when it is tied to the proxy: pre-stop, as an ancestor of the approved PID; post-stop, as the former bound manager process or a descendant of it. Unrelated unreadable shells, which a non-admin query returns for other users' and elevated processes, are not evidence either way. An unprovable Windows manager still blocks guarded takeover. The token checks snapshot consistency, not whether a person approved the desktop prompt. `src/cli/stop-restore.ts` owns the shared Codex/Grok restoration and its history-only, history-deferred, and other failure categories. `ocx service status --json` (`src/service/status-json.ts`) is the passive machine form of the service report, read by peers such as GOAT's device inventory. `src/service/cli.ts` accepts `--json` for `status` only (`planServiceCommand` refuses it on every other verb before any installation probe) and answers it before `platformOps()` resolves, so no install, start, stop, repair or migration path is reachable; `skipsCodexShimAutoRestore` exempts `service status --json` so the CLI preflight repairs no Codex shim first, the collection runs inside `runPassiveRead` so the serving probe's config load neither hardens modes nor backs up a malformed file, and `--json` on any other verb is refused before the Desktop check and the Windows mutation lock; an unsupported platform or Docker gets a document instead of the text path's exit. The document reuses exactly the reads the text report makes — `diagnoseService()` (launchd `print`, systemd observation verbs, Task Scheduler `/query`, SCM/WinSW status) and, only when a registration exists, the 1.5-second loopback `confirmServiceServing` probe — plus the ownership-lease and Desktop login lines. `running` is the manager's verdict (the value `startup.serviceRunning` carries in `ocx status --json`); `serving` reports separately whether a proxy answers on the installed port. `backend` is `launchd`, `systemd`, `schtasks`, `winsw`, or `none` when nothing is registered. The systemd branch of `diagnoseService` derives through the pure `deriveSystemdServiceDiagnostic`, matching the launchd and Windows derivations. `tests/service/service-status-json.test.ts` snapshots each platform branch and records every subprocess entry point on darwin, linux and win32 against a read-only allowlist. `ocx status --json` carries an additive `install` verdict (`src/cli/status-install.ts`) mapped from the updater's own `detectInstallOwnership()`: `method` is `bun`, `npm`, `pnpm`, `mise`, `source`, `standalone` or `unknown` (detector failure), and `evidence.basis` names the package layout, the mise backend record (tool alias and backend only) or the standalone binary without printing a path (`tests/cli/status-install-method.test.ts`). diff --git a/tests/cli/cli-stop-json.test.ts b/tests/cli/cli-stop-json.test.ts index 5394f8256ac..f68b072be0e 100644 --- a/tests/cli/cli-stop-json.test.ts +++ b/tests/cli/cli-stop-json.test.ts @@ -516,6 +516,39 @@ describe("approval-bound stop", () => { expect(signals).toBe(0); }); + test("an absent record hostname is unknown, a present one compares by bind scope", async () => { + let stops = 0; + const stop = async () => { + stops += 1; + return { ok: true, summary: summarize(record(), { exitCode: 0 }) }; + }; + // A default-bind approval: discovery reports the loopback it probed, while the preserved + // record carries no hostname at all. Unknown must not read as drift. + const loopback = { ...STOP_RESOLVE, + liveness: { ...STOP_RESOLVE.liveness, hostname: "127.0.0.1" } } as ResolveJson; + const approval: StopApproval = { ...STOP_APPROVAL, hostname: "127.0.0.1" }; + const target = (hostname: string | undefined) => () => ({ pid: 42, port: 10100, hostname }); + + const absent = await runApprovedStop(approval, async () => loopback, + target(undefined), BOUND_STOP_MANAGER, stop); + expect(absent.summary.outcome).toBe("stopped"); + expect(stops).toBe(1); + + // Present but the same bind scope: a wildcard listener also serves loopback. + const wildcard = await runApprovedStop(approval, async () => loopback, + target("0.0.0.0"), BOUND_STOP_MANAGER, stop); + expect(wildcard.summary.outcome).toBe("stopped"); + expect(stops).toBe(2); + + // Present and genuinely a different scope: still refused, and the detail names the host, + // so a drifting record stays distinguishable from a PID or port change. + const foreign = await runApprovedStop(approval, async () => loopback, + target("192.168.1.5"), BOUND_STOP_MANAGER, stop); + expect(foreign.summary).toMatchObject({ outcome: "approval-changed", + detail: "the tracked runtime record names a different bind hostname" }); + expect(stops).toBe(2); + }); + test("an asynchronous manager stop settles before status decides success", async () => { const order: string[] = []; let state: "active" | "inactive" = "active"; diff --git a/tests/service/process-state.test.ts b/tests/service/process-state.test.ts index 73cfd51657a..e50dcee78c8 100644 --- a/tests/service/process-state.test.ts +++ b/tests/service/process-state.test.ts @@ -349,6 +349,18 @@ describe("tracked runtime record inspection", () => { expect(describeTrackedRuntimeTarget(liveOcx)).toEqual({ pid: 4242, port: 58195, hostname: "0.0.0.0" }); }); + test("a record without a hostname reports it as unknown, not as an empty host", () => { + // The field is optional: a default-bind install has no configured hostname to persist, so + // the writer drops it. Reporting "" made the guarded stop compare an empty string against + // the loopback that liveness discovery reports for the same runtime. + trackedPid(); + writeFileSync(getRuntimePortPath(), JSON.stringify({ pid: 4242, port: 58195 }), "utf-8"); + expect(inspectTrackedRuntimeRecord(liveOcx)).toEqual( + { status: "verified", pid: 4242, port: 58195, hostname: undefined }); + expect(describeTrackedRuntimeTarget(liveOcx)).toEqual( + { pid: 4242, port: 58195, hostname: undefined }); + }); + test("an absent or invalid pid record is named before any process probe", () => { const probes: string[] = []; const deps: TrackedRecordProbeDeps = { From 6c2ca990771df8b208e6aeeacf79f24225d2fc49 Mon Sep 17 00:00:00 2001 From: kerthans <112744990+kerthans@users.noreply.github.com> Date: Thu, 5 Nov 2026 13:35:08 +0800 Subject: [PATCH 2/3] docs+test(stop): state the optional-hostname rule and pin the bound scope The guarded stop's hostname comparison is user-visible behavior, so the CLI reference now states both halves: the writer records the scope the listener actually bound, and an absent hostname in the record reads as unknown rather than as a mismatch. The launcher test starts a home that configures no hostname and asserts the published record still states a scope, so reverting the writer to the raw config value fails instead of passing silently. --- docs-site/src/content/docs/reference/cli/lifecycle.md | 6 ++++++ tests/service/shutdown-launcher.test.ts | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 30755b79db8..cfbb0f53c17 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -152,6 +152,12 @@ unreadable command line in the approved proxy's supervision chain (or surviving stopped manager) leaves wrapper ownership unknown, so the guarded stop cannot report success. Missing or unreadable evidence blocks the guarded stop. +The tracked runtime record states the bind scope the listener actually bound, so a default +install records `127.0.0.1` even though it configures no hostname. When a record still omits +the optional `hostname`, the guarded stop does not compare bind scope: PID and port still must +match exactly, and the hostname is compared as a bind scope — so `0.0.0.0` and the loopback it +defaults to agree — only when the record provides one. + ### `ocx restart` When this CLI is newer than an attested standalone POSIX proxy, restart can stop the old diff --git a/tests/service/shutdown-launcher.test.ts b/tests/service/shutdown-launcher.test.ts index 31bb4dda714..c91d04f93cf 100644 --- a/tests/service/shutdown-launcher.test.ts +++ b/tests/service/shutdown-launcher.test.ts @@ -166,6 +166,13 @@ describe.skipIf(!runnable)("ocx launcher graceful shutdown", () => { ); } expect(existsSync(join(home, "ocx.pid"))).toBe(true); + // The record must state the scope the listener actually bound, and this home configured + // no hostname at all: publishing the raw config value would leave the optional field out + // and make every guarded stop read the record as unknown. Assert presence, not a literal, + // because a default bind may answer on 0.0.0.0 while still meaning loopback. + const published = JSON.parse(readFileSync(join(home, "runtime-port.json"), "utf8")); + expect(typeof published.hostname).toBe("string"); + expect(published.hostname).not.toBe(""); const injected = readFileSync(codexConfig, "utf8"); if (approved) { expect(injected).toContain(`base_url = "http://127.0.0.1:${port}/v1"`); From f9c696f70a80f8bcdb0bad404678e0b7c7e2888f Mon Sep 17 00:00:00 2001 From: Clint <112744990+kerthans@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:17:26 +0800 Subject: [PATCH 3/3] fix(stop): shorten hostname comment to clear file-size ratchet src/cli/index.ts crossed the 2000-line NEW_OVERSIZED threshold (2002). Collapse the 4-line block comment to a trailing one-liner; the rationale lives in runApprovedStop. No behaviour change. Co-Authored-By: Claude Sonnet 5.5 --- src/cli/index.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index d949cba5afb..1189cc2835a 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -606,10 +606,7 @@ async function handleStart(options: { block?: boolean } = {}) { writeRuntime: bound => { updateRestartChild?.check(bound.port, bound.config.hostname ?? ""); writeRuntimePort({ pid: process.pid, port: bound.port, - /** Persist the scope the listener actually bound (127.0.0.1 for a default install), - * not the raw config value: an omitted field is unknown to the guarded stop, which - * only compares a hostname the record actually carries. */ - hostname: bound.server.hostname, + hostname: bound.server.hostname, // bound scope, not raw config (see runApprovedStop) attestationSecret: bound.localAttestationSecret, command: currentServingCommand(), ...siblingRuntimeField(),