Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs-site/src/content/docs/reference/cli/lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ 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,
hostname: bound.server.hostname, // bound scope, not raw config (see runApprovedStop)
attestationSecret: bound.localAttestationSecret,
command: currentServingCommand(),
...siblingRuntimeField(),
Expand Down
34 changes: 31 additions & 3 deletions src/cli/stop-approval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
export async function runApprovedStop(
expected: StopApproval,
read: () => Promise<ResolveJson | null>,
Expand All @@ -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})`);
}
Expand Down
19 changes: 16 additions & 3 deletions src/config/tracked-runtime-inspection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 };

/**
Expand Down Expand Up @@ -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" };
}
Expand Down Expand Up @@ -177,10 +187,13 @@ const TRACKED_REFUSAL_CAUSES: Record<TrackedRecordRefusal, string> = {
/**
* 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 }
Expand Down
2 changes: 1 addition & 1 deletion structure/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading