Skip to content
Merged
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
47 changes: 26 additions & 21 deletions src/cli/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
import { accessSync, constants, existsSync, readFileSync } from "node:fs";
import { homedir } from "node:os";
import { dirname, join } from "node:path";
import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, readRuntimePort, resolveEnvValue } from "../config";
import { findLiveProxy } from "../server/proxy-liveness";
import { gracefulStopHost } from "../lib/process-control";
import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, resolveEnvValue } from "../config";
import { findLiveProxy, type LiveProxy } from "../server/proxy-liveness";
import { BUN_RUNTIME_SOURCES } from "../lib/bun-runtime";
import type { BunRuntimeSource } from "../lib/bun-runtime";
import { maskAccountId } from "../lib/privacy";
import { PROXY_ENV_KEYS, proxyEnvPresent } from "../lib/proxy-env";
import { configuredAdminToken } from "../lib/admin-secrets";
import { LOCAL_MANAGEMENT_READ_PATHS } from "../lib/local-management-capability";
import { readCodexTokens } from "../codex/auth-collision";
import { withNativeMainSharedClaim } from "../codex/native-main-claim";
import { probeNativeProfileRecoveryState, resolveNativeProfileContext } from "../codex/native-profile-store";
Expand All @@ -42,6 +41,10 @@ import {
} from "../codex/runtime";
import { CODEX_REAUTH_ACTION, collectOAuthHealthEntriesForCli, MASKED_ACCOUNT_FALLBACK, type OAuthHealthEntry } from "../oauth/health";
import { getAuthRefreshIntentLockPath, getAuthStorePath } from "../oauth/store";
import {
fetchBoundLocalManagementRead,
type LocalManagementReadDeps,
} from "../server/local-management-read-client";
export { resolveCodexHomeDir } from "../codex/home";

export type OAuthDoctorCheck = { level: "OK" | "WARN"; message: string };
Expand Down Expand Up @@ -610,20 +613,28 @@ function observedMemory(data: { rss: number; external?: number; arrayBuffers?: n
}

export async function fetchServiceMemory(
host: string,
port: number,
token: string | null,
fetchImpl: typeof fetch = fetch,
target: LiveProxy,
deps: LocalManagementReadDeps = {},
): Promise<ServiceMemoryReport> {
try {
const res = await fetchImpl(`http://${host}:${port}/api/system/memory`, {
headers: token ? { "x-opencodex-api-key": token } : {},
signal: AbortSignal.timeout(SERVICE_MEMORY_TIMEOUT_MS),
const read = await fetchBoundLocalManagementRead(target, LOCAL_MANAGEMENT_READ_PATHS.systemMemory, {
...deps,
timeoutMs: SERVICE_MEMORY_TIMEOUT_MS,
});
Comment on lines 615 to 623

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

fetchServiceMemory advertises a timeoutMs option that it discards.

The parameter type is LocalManagementReadDeps, which declares timeoutMs?: number. At lines 620-623 the spread ...deps is applied first, then timeoutMs: SERVICE_MEMORY_TIMEOUT_MS overwrites it. A caller that passes timeoutMs therefore gets silent no-op behavior, and the type signature does not communicate that. Make the contract explicit in one of two ways.

♻️ Option 1 (preferred): remove the knob from the accepted type
 export async function fetchServiceMemory(
   target: LiveProxy,
-  deps: LocalManagementReadDeps = {},
+  deps: Omit<LocalManagementReadDeps, "timeoutMs"> = {},
 ): Promise<ServiceMemoryReport> {
♻️ Option 2: honor a caller-supplied timeout with the doctor default
     const read = await fetchBoundLocalManagementRead(target, LOCAL_MANAGEMENT_READ_PATHS.systemMemory, {
       ...deps,
-      timeoutMs: SERVICE_MEMORY_TIMEOUT_MS,
+      timeoutMs: deps.timeoutMs ?? SERVICE_MEMORY_TIMEOUT_MS,
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function fetchServiceMemory(
host: string,
port: number,
token: string | null,
fetchImpl: typeof fetch = fetch,
target: LiveProxy,
deps: LocalManagementReadDeps = {},
): Promise<ServiceMemoryReport> {
try {
const res = await fetchImpl(`http://${host}:${port}/api/system/memory`, {
headers: token ? { "x-opencodex-api-key": token } : {},
signal: AbortSignal.timeout(SERVICE_MEMORY_TIMEOUT_MS),
const read = await fetchBoundLocalManagementRead(target, LOCAL_MANAGEMENT_READ_PATHS.systemMemory, {
...deps,
timeoutMs: SERVICE_MEMORY_TIMEOUT_MS,
});
export async function fetchServiceMemory(
target: LiveProxy,
deps: Omit<LocalManagementReadDeps, "timeoutMs"> = {},
): Promise<ServiceMemoryReport> {
try {
const read = await fetchBoundLocalManagementRead(target, LOCAL_MANAGEMENT_READ_PATHS.systemMemory, {
...deps,
timeoutMs: SERVICE_MEMORY_TIMEOUT_MS,
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/doctor.ts` around lines 615 - 623, Update fetchServiceMemory so its
timeout contract is explicit: either omit timeoutMs from the accepted
LocalManagementReadDeps type for this function, or preserve a caller-provided
timeout while defaulting to SERVICE_MEMORY_TIMEOUT_MS. Ensure the implementation
and function signature consistently reflect the chosen behavior.

if (read.kind === "unavailable") {
return read.reason === "transport"
? { status: "unreachable", error: "fetch failed" }
: { status: "unauthorized" };
}
const { response: res, targetPid } = read;
if (res.status === 401 || res.status === 403) return { status: "unauthorized" };
if (!res.ok) return { status: "unreachable", error: `http ${res.status}` };
const body = await res.json() as Partial<ServiceMemoryData>;
if (typeof body.pid !== "number" || typeof body.bunVersion !== "string" || typeof body.rss !== "number") {
if (
body.pid !== targetPid
|| typeof body.bunVersion !== "string"
|| typeof body.rss !== "number"
) {
return { status: "unreachable", error: "malformed response" };
}
return {
Expand Down Expand Up @@ -672,7 +683,7 @@ export function formatServiceMemoryLines(report: ServiceMemoryReport): string[]
const lines: string[] = [];
lines.push(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`);
if (report.status === "unauthorized") {
lines.push(" -- proxy reachable but rejected the request — set OPENCODEX_ADMIN_AUTH_TOKEN to match the service");
lines.push(" -- local diagnostic capability unavailable — restart the running proxy with this OpenCodex version");
return lines;
}
if (report.status === "unreachable") {
Expand Down Expand Up @@ -844,10 +855,6 @@ export async function runDoctor(args: string[] = []): Promise<void> {
const live = await findLiveProxy({
configFn: () => ({ port: doctorConfig.port, hostname: doctorConfig.hostname }),
});
const livePid = live ? live.pid : readPid();
const liveRuntime = live
? { pid: live.pid ?? 0, port: live.port, hostname: live.hostname }
: (livePid ? readRuntimePort(livePid) : null);

const currentProxyEnv = collectProxyEnv();
const configuredProxy = collectConfiguredProxy();
Expand Down Expand Up @@ -887,13 +894,11 @@ export async function runDoctor(args: string[] = []): Promise<void> {

console.log("\nMemory / runtime");
{
const runtime = liveRuntime;
if (!runtime || !live) {
if (!live) {
console.log(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`);
console.log(" -- no running ocx proxy found (no live pid/runtime record)");
} else {
const token = configuredAdminToken();
const report = await fetchServiceMemory(gracefulStopHost(runtime.hostname), runtime.port, token);
const report = await fetchServiceMemory(live);
for (const line of formatServiceMemoryLines(report)) console.log(line);
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/cli/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { durableBunRuntime } from "../lib/bun-runtime";
import { codexAutoStartEnabled, getConfigPath, getPidPath, readConfigDiagnostics, readPid, readRuntimePort, type RuntimePortState } from "../config";
import { diagnoseCodexBundledPlugins, type CodexPluginsDiagnostic } from "../codex/plugins-doctor";
import { findLiveProxy, isOpencodexHealthz, probeHostname } from "../server/proxy-liveness";
import { directLocalHttpFetch } from "../server/direct-local-http";
import type { OcxConfig } from "../types";
import { diagnoseService, serviceLogPath } from "../service";
import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health";
Expand Down Expand Up @@ -110,12 +111,18 @@ export function resolveStatusPid(
return live ? live.pid : pidFile;
}

export function proxyHealthFailureReason(error: unknown, signal: AbortSignal): "timed out" | "unreachable" {
return signal.aborted || (error instanceof Error && error.name === "AbortError")
? "timed out"
: "unreachable";
}

async function checkProxyHealth(target: ListenTarget): Promise<HealthCheck> {
const url = target.healthUrl;
const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), 800);
try {
const response = await fetch(url, { signal: controller.signal });
const response = await directLocalHttpFetch(url, { signal: controller.signal });
if (!response.ok) {
const message = `returned HTTP ${response.status}`;
return { ok: false, url, message, label: `${url} ${message}` };
Expand All @@ -130,7 +137,7 @@ async function checkProxyHealth(target: ListenTarget): Promise<HealthCheck> {
const message = `ok${version}${uptime}`;
return { ok: true, url, message, label: `${url} ${message}` };
} catch (error) {
const reason = error instanceof Error && error.name === "AbortError" ? "timed out" : "unreachable";
const reason = proxyHealthFailureReason(error, controller.signal);
return { ok: false, url, message: reason, label: `${url} ${reason}` };
} finally {
clearTimeout(timer);
Expand Down
100 changes: 100 additions & 0 deletions src/lib/local-management-capability.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { createHmac, timingSafeEqual } from "node:crypto";
import { isLocalAttestationSecret } from "./local-management-attestation";

export const LOCAL_MANAGEMENT_EXPECTED_PID_HEADER = "x-opencodex-local-expected-pid";
export const LOCAL_MANAGEMENT_NONCE_HEADER = "x-opencodex-local-nonce";
export const LOCAL_MANAGEMENT_CAPABILITY_EXPIRES_AT_HEADER = "x-opencodex-local-expires-at";
export const LOCAL_MANAGEMENT_CAPABILITY_HEADER = "x-opencodex-local-capability";
export const LOCAL_MANAGEMENT_CAPABILITY_TTL_MS = 10_000;

export const LOCAL_MANAGEMENT_READ_PATHS = {
codexAccounts: "/api/codex-auth/accounts",
systemMemory: "/api/system/memory",
} as const;

export type LocalManagementReadPath =
typeof LOCAL_MANAGEMENT_READ_PATHS[keyof typeof LOCAL_MANAGEMENT_READ_PATHS];

const BASE64URL_256 = /^[A-Za-z0-9_-]{43}$/;
const LOCAL_READ_METHOD = "GET";

export type ExpectedLocalManagementPid =
| { kind: "absent" }
| { kind: "invalid" }
| { kind: "present"; pid: number };

export function parseExpectedLocalManagementPid(value: string | null): ExpectedLocalManagementPid {
if (value === null) return { kind: "absent" };
if (!/^[1-9]\d*$/.test(value)) return { kind: "invalid" };
const pid = Number(value);
return Number.isSafeInteger(pid) ? { kind: "present", pid } : { kind: "invalid" };
}

function isLocalManagementReadPath(path: string): path is LocalManagementReadPath {
return path === LOCAL_MANAGEMENT_READ_PATHS.codexAccounts
|| path === LOCAL_MANAGEMENT_READ_PATHS.systemMemory;
}

function localReadCapabilityPayload(
nonce: string,
method: string,
path: string,
pid: number,
port: number,
expiresAt: number,
): string | null {
if (!BASE64URL_256.test(nonce)) return null;
if (method !== LOCAL_READ_METHOD || !isLocalManagementReadPath(path)) return null;
if (!Number.isSafeInteger(pid) || pid <= 0) return null;
if (!Number.isInteger(port) || port <= 0 || port > 65535) return null;
if (!Number.isSafeInteger(expiresAt) || expiresAt <= 0) return null;
return `opencodex-local-management-read-v1\n${nonce}\n${method}\n${path}\n${pid}\n${port}\n${expiresAt}`;
}

/** Process-scoped authorization for one allowlisted local management GET. */
export function createLocalManagementReadCapability(
secret: string,
nonce: string,
method: string,
path: string,
pid: number,
port: number,
expiresAt: number,
): string | null {
if (!isLocalAttestationSecret(secret)) return null;
const payload = localReadCapabilityPayload(nonce, method, path, pid, port, expiresAt);
if (!payload) return null;
return createHmac("sha256", secret).update(payload).digest("base64url");
}

export function verifyLocalManagementReadCapability(
secret: string,
nonce: string | null,
method: string,
path: string,
pid: number,
port: number,
expiresAt: number,
capability: string | null,
now = Date.now(),
): boolean {
if (!nonce || !capability || !BASE64URL_256.test(capability)) return false;
if (
!Number.isSafeInteger(now)
|| expiresAt <= now
|| expiresAt > now + LOCAL_MANAGEMENT_CAPABILITY_TTL_MS
) return false;
const expected = createLocalManagementReadCapability(
secret,
nonce,
method,
path,
pid,
port,
expiresAt,
);
if (!expected) return false;
const expectedBytes = Buffer.from(expected);
const actualBytes = Buffer.from(capability);
return expectedBytes.length === actualBytes.length && timingSafeEqual(expectedBytes, actualBytes);
}
60 changes: 12 additions & 48 deletions src/oauth/health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,11 @@ import { getAnthropicAccountHealthSnapshot } from "./anthropic-routing";
import { isAccountNeedsReauth } from "../codex/account-runtime-state";
import { getCodexAccountCredential, listCodexAccountIds } from "../codex/account-store";
import { MAIN_CODEX_ACCOUNT_ID } from "../codex/main-account";
import { configuredAdminToken } from "../lib/admin-secrets";
import { readRuntimePort } from "../config";
import {
LOCAL_ATTESTATION_CHALLENGE_HEADER,
LOCAL_ATTESTATION_PROOF_HEADER,
createLocalAttestationChallenge,
verifyLocalAttestationProof,
} from "../lib/local-management-attestation";
import { LOCAL_MANAGEMENT_READ_PATHS } from "../lib/local-management-capability";
import { maskAccountId } from "../lib/privacy";
import { findLiveProxy, probeHostname } from "../server/proxy-liveness";
import { findLiveProxy } from "../server/proxy-liveness";
import { fetchBoundLocalManagementRead } from "../server/local-management-read-client";
import { loadAuthStore, peekAuthStore, peekOAuthRefreshIntent, readOAuthRefreshIntent } from "./store";
import type { ProviderAccount } from "./types";

Expand Down Expand Up @@ -333,53 +328,22 @@ type LiveProxyCodexHealthResult = {
};

async function fetchCodexHealthFromLiveProxy(
fetchImpl: typeof fetch = fetch,
fetchImpl: typeof fetch | undefined = undefined,
findLiveProxyImpl: typeof findLiveProxy = findLiveProxy,
readRuntimePortImpl: typeof readRuntimePort = readRuntimePort,
): Promise<LiveProxyCodexHealthResult> {
const live = await findLiveProxyImpl();
if (!live) return { source: "unavailable", entries: null };
// This is a management-plane endpoint. A data-plane service token is intentionally not
// interchangeable with the admin credential even on loopback.
const token = configuredAdminToken();
const headers: Record<string, string> = {};
try {
if (token) {
// Public /healthz identity is intentionally forgeable enough for liveness, not
// strong enough to receive a bearer. Prove the listener knows the per-process
// secret stored in the protected runtime record before attaching the admin token.
if (live.source !== "runtime" || live.pid === null) {
return { source: "management-api-unavailable", entries: null };
}
const attestedPid = live.pid;
const runtime = readRuntimePortImpl(attestedPid);
if (!runtime?.attestationSecret || runtime.port !== live.port) {
return { source: "management-api-unavailable", entries: null };
}
const challenge = createLocalAttestationChallenge();
const proofResponse = await fetchImpl(
`http://${probeHostname(live.hostname)}:${live.port}/healthz`,
{
headers: { [LOCAL_ATTESTATION_CHALLENGE_HEADER]: challenge },
signal: AbortSignal.timeout(4000),
},
);
const proof = proofResponse.headers.get(LOCAL_ATTESTATION_PROOF_HEADER);
if (!proofResponse.ok || !verifyLocalAttestationProof(
runtime.attestationSecret,
challenge,
attestedPid,
live.port,
proof,
)) {
return { source: "management-api-unavailable", entries: null };
}
headers.Authorization = `Bearer ${token}`;
}
const res = await fetchImpl(
`http://${probeHostname(live.hostname)}:${live.port}/api/codex-auth/accounts`,
{ headers, signal: AbortSignal.timeout(4000) },
const read = await fetchBoundLocalManagementRead(
live,
LOCAL_MANAGEMENT_READ_PATHS.codexAccounts,
{ fetchImpl, readRuntime: readRuntimePortImpl, timeoutMs: 4_000 },
);
if (read.kind === "unavailable") {
return { source: "management-api-unavailable", entries: null };
}
const res = read.response;
if (res.status === 401 || res.status === 403) {
return { source: "management-auth-failed", entries: null };
}
Expand Down
Loading
Loading