diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 13878d54aeb..1cca04995d5 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -60,6 +60,12 @@ The hub automatically issues a per-client key. The client writes it to the exist `service-api-token` file, never `config.json`. While connected, usage comes from the hub usage store filtered to that client's stable `apiKeyId`. After disconnect, usage comes from the local store. OpenCodex does not mirror usage between the two stores. +`ocx service uninstall` removes the local service but preserves an existing key when the client is +connected, its connection metadata is invalid or mismatched, or a pending connection marker matches +the current key. A valid marker for an older key does not retain an unrelated service key. +If a marker is unsafe, malformed, or unreadable, token cleanup cannot be verified; +the command warns instead of claiming the key was kept. Use `ocx disconnect` to remove a connected +client's local key and state. If a client saved a remote `http://` Hub URL before the secure transport rule, its Hub operations now return `insecure_http_refused`. Run `ocx disconnect` locally, then reconnect diff --git a/docs-site/src/content/docs/ko/guides/remote-hub.md b/docs-site/src/content/docs/ko/guides/remote-hub.md index f4ec3fea808..585b6ea6457 100644 --- a/docs-site/src/content/docs/ko/guides/remote-hub.md +++ b/docs-site/src/content/docs/ko/guides/remote-hub.md @@ -39,6 +39,7 @@ ocx sync 이 줄을 직접 만들 필요는 없습니다. 허브에서 `ocx hub invite`를 실행하면 코드를 발급하고, 두 Origin이 모두 채워진 명령을 그대로 출력합니다. [다른 컴퓨터 초대하기](#다른-컴퓨터-초대하기)를 보세요. 허브가 발급한 클라이언트별 키는 권한이 제한된 `service-api-token` 파일에 저장됩니다. `config.json`에는 저장되지 않습니다. 연결 중 사용량은 허브 기록에서 해당 `apiKeyId`만 조회하고, 연결을 끊은 뒤에는 로컬 기록을 봅니다. 두 기록은 서로 복제되지 않습니다. +`ocx service uninstall`은 로컬 서비스를 제거하지만 클라이언트가 연결되어 있거나, 연결 중 표시의 지문이 현재 키와 일치하거나, 연결 정보가 잘못되었거나 일치하지 않으면 기존 키를 보존합니다. 이전 키의 유효한 표시는 다른 서비스 키를 보존하지 않습니다. 표시 파일이 안전하지 않거나 손상되었거나 읽을 수 없어 키 정리를 확인할 수 없으면 보존했다고 단정하지 않고 경고합니다. 연결된 클라이언트의 로컬 키와 상태를 제거하려면 `ocx disconnect`를 사용하세요. ### 연결된 클라이언트의 상태 표시 diff --git a/src/client/connect.ts b/src/client/connect.ts index d9490386096..470faf3866c 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -37,6 +37,7 @@ import { replaceServiceApiTokenFile, restoreTokenBackup, serviceApiTokenBackupPath, + serviceApiTokenFingerprint, writeTokenBackup, writeServiceApiTokenFile, } from "../lib/service-secrets"; @@ -65,6 +66,7 @@ import { clearClientConnection, commitClientConnection, readClientConnectionState, + markClientConnectPending, clearClientConnectPending, pendingClientConnectMayOwnToken, assertNoClientDisconnectPending, assertClientConnectionUnchanged, sameClientConnectionOwner, } from "./state"; import { assertClientCatalogCompatible, type CatalogCompatibilityDeps } from "./catalog-compatibility"; @@ -492,6 +494,7 @@ function assertConnectingState(expectedTokenFingerprint?: string): void { } } +/** Enroll a client key, keeping its pending ownership visible until commit or rollback. */ export async function connectClient( options: ConnectOptions, deps: ClientConnectDeps = {}, @@ -501,6 +504,7 @@ export async function connectClient( let issued: IssuedClientKey | null = null; let cleanupCredential: { kind: "admin"; value: Uint8Array } | { kind: "gui-session"; value: ConnectGuiSession } | null = null; let tokenFingerprint: string | null = null; + let pendingConnectFingerprint: string | null = null; let priorCatalog: CatalogSnapshot | null = null; let writtenCatalogFingerprint: string | null = null; let injectionCommitted = false; @@ -536,6 +540,9 @@ export async function connectClient( const initialFiles = withClientLifecycleSync(() => withConfigMutationLockSync(() => { assertConnectingState(); + const fingerprint = serviceApiTokenFingerprint(issued!.key); + markClientConnectPending(fingerprint); + pendingConnectFingerprint = fingerprint; return { prior: catalogSnapshot(), persisted: writeServiceApiTokenFile(issued!.key) }; }), deps.lifecycleLockDeps); priorCatalog = initialFiles.prior; @@ -602,6 +609,7 @@ export async function connectClient( }; withClientLifecycleSync(() => withConfigMutationLockSync(() => { assertConnectingState(persisted.fingerprint); + clearClientConnectPending(persisted.fingerprint); commitClientConnection(connection); committed = true; }), deps.lifecycleLockDeps); @@ -618,9 +626,11 @@ export async function connectClient( if (priorCatalog && writtenCatalogFingerprint && !restoreCatalogSnapshot(priorCatalog, writtenCatalogFingerprint)) { rollbackFailures.push("catalog rollback did not match the written artifact"); } - if (tokenFingerprint) { - const removed = removeServiceApiTokenFileIfOwned(tokenFingerprint); + if (pendingConnectFingerprint) { + const removed = removeServiceApiTokenFileIfOwned(pendingConnectFingerprint); if (removed === "changed") rollbackFailures.push("service token changed during rollback"); + // Final commit may fail after this attempt already cleared its marker under the same lock. + else if (pendingClientConnectMayOwnToken()) clearClientConnectPending(pendingConnectFingerprint); } }), deps.lifecycleLockDeps); } catch { rollbackFailures.push("client cleanup ownership unavailable"); } diff --git a/src/client/state.ts b/src/client/state.ts index fd4045d482a..e8374f309f6 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -1,5 +1,7 @@ -import { readFileSync } from "node:fs"; +import { lstatSync, readFileSync, unlinkSync } from "node:fs"; +import { join } from "node:path"; import { + getConfigDir, getConfigPath, deleteConfigTopLevelKey, getDefaultConfig, @@ -8,6 +10,7 @@ import { saveConfig, withConfigMutationLockSync, } from "../config"; +import { atomicWriteFileNoFollowUnclaimed } from "../config/atomic-write"; import type { OcxClientConnectionConfig } from "../types"; import { inspectRemoteDesktopStore, readDesktopDisconnectReceipt } from "../claude/desktop-remote-store"; import { withClientLifecycleSync, type ClientLifecycleLockDeps } from "./lifecycle-lock"; @@ -23,6 +26,41 @@ export type ClientConnectionState = | { kind: "invalid"; reason: string } | { kind: "mismatched"; reason: string }; +const pendingConnectPath = (): string => join(getConfigDir(), "client-connect-pending"); + +/** Validate pending ownership; an optional fingerprint restricts it to that exact key. */ +export function pendingClientConnectMayOwnToken(fingerprint?: string): boolean { + const path = pendingConnectPath(); + let stat; + try { stat = lstatSync(path); } + catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + } + if (!stat.isFile() || stat.nlink !== 1 || stat.size !== 65) { + throw new Error("pending client connection owner is unsafe"); + } + const marker = readFileSync(path, "utf8"); + if (!/^[a-f0-9]{64}\n$/.test(marker)) throw new Error("pending client connection owner is malformed"); + return fingerprint === undefined || marker === `${fingerprint}\n`; +} + +/** Publish only the token fingerprint, before the key file, under the client lifecycle lock. */ +export function markClientConnectPending(fingerprint: string): void { + if (!/^[a-f0-9]{64}$/.test(fingerprint)) throw new Error("invalid pending client fingerprint"); + atomicWriteFileNoFollowUnclaimed(pendingConnectPath(), `${fingerprint}\n`); +} + +/** Clear only the marker for this connect attempt while the client lifecycle lock is held. */ +export function clearClientConnectPending(fingerprint: string): void { + const path = pendingConnectPath(); + const stat = lstatSync(path); + if (!stat.isFile() || stat.nlink !== 1 || stat.size !== 65 || readFileSync(path, "utf8") !== `${fingerprint}\n`) { + throw new Error("pending client connection owner changed"); + } + unlinkSync(path); +} + export type ClientRotationRecoveryGate = | { kind: "clean" } | { kind: "orphan-cleaned" } diff --git a/src/service/cli.ts b/src/service/cli.ts index d8fea9b0692..560fa6c6ed6 100644 --- a/src/service/cli.ts +++ b/src/service/cli.ts @@ -1,9 +1,12 @@ -import { existsSync, unlinkSync } from "node:fs"; +import { existsSync, lstatSync, unlinkSync } from "node:fs"; import { join } from "node:path"; import { restoreNativeCodexAsync } from "../codex/inject"; import { describeRetainedCodexProviderTable } from "../codex/inject/restore"; import { stripGrokConfig } from "../grok/inject"; -import { serviceApiTokenFilePath } from "../lib/service-secrets"; +import { withConfigMutationLockSync } from "../config/mutation-lock"; +import { withClientLifecycleSync, type ClientLifecycleLockDeps } from "../client/lifecycle-lock"; +import { pendingClientConnectMayOwnToken, readClientConnectionState } from "../client/state"; +import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; import { statusWinswRaw, type WinswStatus } from "../lib/winsw"; import { withWindowsServiceMutationLock } from "../lib/windows-service-mutation-lock"; import { maybeShowStarPrompt } from "../cli/star-prompt"; @@ -182,6 +185,32 @@ export function parseServiceArgs(args: string[]): ParsedServiceArgs { return { sub: normalizeServiceSubcommand(sub), backend, invalid }; } +/** Remove the service credential only when no client connection can own it. */ +export function removeServiceTokenAfterUninstall( + lockDeps: ClientLifecycleLockDeps = {}, +): "removed" | "absent" | "retained" | "unverified" { + try { + return withClientLifecycleSync(() => withConfigMutationLockSync(() => { + const path = serviceApiTokenFilePath(); + try { lstatSync(path); } + catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return "absent"; + throw error; + } + if (readClientConnectionState().kind !== "disconnected") return "retained"; + const token = readServiceApiTokenState(); + if (token.kind !== "present") return token.kind === "absent" ? "absent" : "unverified"; + if (pendingClientConnectMayOwnToken(token.fingerprint)) return "retained"; + unlinkSync(path); + return "removed"; + }), lockDeps); + } catch { + // Lock, state-read and unlink failures all leave cleanup unverified, not successful. + return "unverified"; + } +} + +/** Execute a service verb while preserving client-owned credentials during uninstall. */ export async function serviceCommand(...args: (string | undefined)[]): Promise { const filteredArgs = args.filter((a): a is string => Boolean(a)); const execute = async (): Promise => { @@ -424,7 +453,9 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { /** A catalog the user already had before ever connecting. */ const PRIOR_CATALOG_BYTES = '{"models":[{"slug":"local/only-model"}]}'; +/** Exercise enrollment and rollback in a fresh process with isolated client homes. */ function runTransactionScenario( - stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator", + stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator" | "uninstall-during-catalog", options: { script?: string; timeoutMs?: number } = {}, ) { const opencodexHome = mkdtempSync(join(tmpdir(), "ocx-client-connect-home-")); @@ -430,6 +431,7 @@ function runTransactionScenario( const stage = ${JSON.stringify(stage)}; markTransaction("module_ready"); let commitFaultTriggered = false; + let uninstallDuringCatalog = null; const catalog = '{"models":[]}'; const etag = '"sha256-' + createHash("sha256").update(catalog).digest("base64url") + '"'; const calls = []; @@ -447,6 +449,13 @@ function runTransactionScenario( if (url.endsWith("/api/keys") && init.method === "DELETE") return Response.json({ success: true }); if (url.endsWith("/v1/catalog")) { if (stage === "catalog") return Response.json({ error: "down" }, { status: 503 }); + if (stage === "uninstall-during-catalog") { + const { removeServiceTokenAfterUninstall } = require("./src/service/cli"); + uninstallDuringCatalog = { + cleanup: removeServiceTokenAfterUninstall({ lockPath: process.env.OPENCODEX_HOME + "/lifecycle.sqlite" }), + tokenExists: existsSync(serviceApiTokenFilePath()), + }; + } return new Response(catalog, { headers: { ETag: etag, "Content-Type": "application/json" } }); } throw new Error("unexpected request " + url); @@ -500,7 +509,7 @@ function runTransactionScenario( if ((stage === "success" || stage === "prior-catalog") && connected) disconnected = await disconnectClient({}, { lifecycleLockDeps: { lockPath: process.env.OPENCODEX_HOME + "/lifecycle.sqlite" } }); const catalogAfter = existsSync(DEFAULT_CATALOG_PATH) ? readFileSync(DEFAULT_CATALOG_PATH, "utf8") : null; const hubStateCacheAfter = existsSync(hubStateCachePath()); - writeSync(1, JSON.stringify({ connected, error, coordinatorUnavailable, beforeDisconnect, artifacts, disconnected, catalogAfter, hubStateCacheBefore, hubStateCacheAfter, after: readClientConnectionState(), calls, commitFaultTriggered }) + "\\n"); + writeSync(1, JSON.stringify({ connected, error, coordinatorUnavailable, beforeDisconnect, artifacts, disconnected, catalogAfter, hubStateCacheBefore, hubStateCacheAfter, after: readClientConnectionState(), calls, commitFaultTriggered, uninstallDuringCatalog, pendingAtResult: existsSync(process.env.OPENCODEX_HOME + "/client-connect-pending") }) + "\\n"); markTransaction("result_published"); })(); `; @@ -669,7 +678,19 @@ describe("connect transaction and offline disconnect", () => { expect(run.parsed.after).toEqual({ kind: "disconnected" }); expect(run.parsed.calls.filter((call: any) => call.method === "DELETE")).toEqual([]); } finally { run.cleanup(); } - }); + }, SPAWN_BUDGET_MS); + + test("service uninstall during catalog download retains the pending client key", () => { + const run = runTransactionScenario("uninstall-during-catalog"); + try { + expect(run.status).toBe(0); + expect(run.parsed.uninstallDuringCatalog).toEqual({ cleanup: "retained", tokenExists: true }); + expect(run.parsed.error).toBeNull(); + expect(run.parsed.connected.apiKeyId).toBe("issued-id"); + expect(run.parsed.beforeDisconnect.kind).toBe("connected"); + expect(run.parsed.pendingAtResult).toBe(false); + } finally { run.cleanup(); } + }, SPAWN_BUDGET_MS); test("disconnect puts back the catalog the user had before connecting", () => { // Connect overwrites whatever catalog is already on disk. Disconnect used to delete the @@ -684,7 +705,7 @@ describe("connect transaction and offline disconnect", () => { expect(run.parsed.catalogAfter).toBe(PRIOR_CATALOG_BYTES); expect(run.parsed.after).toEqual({ kind: "disconnected" }); } finally { run.cleanup(); } - }); + }, SPAWN_BUDGET_MS); test("disconnect removes the catalog when the user had none", () => { // The other half of the same contract: `priorCatalog: ""` records "there genuinely was @@ -694,7 +715,7 @@ describe("connect transaction and offline disconnect", () => { expect(run.parsed.disconnected).toMatchObject({ catalogRemoved: true, catalogRestored: false }); expect(run.parsed.catalogAfter).toBeNull(); } finally { run.cleanup(); } - }); + }, SPAWN_BUDGET_MS); for (const stage of ["catalog", "preflight", "commit"] as const) { test(`rolls back local artifacts when ${stage} fails before final commit`, () => { @@ -704,11 +725,13 @@ describe("connect transaction and offline disconnect", () => { expect(run.parsed.connected).toBeNull(); expect(run.parsed.beforeDisconnect).toEqual({ kind: "disconnected" }); expect(run.parsed.artifacts.token).toBe(false); + expect(run.parsed.pendingAtResult).toBe(false); expect(run.parsed.artifacts.catalog).toBe(false); expect(run.parsed.artifacts.credentialZeroed).toBe(true); expect(run.parsed.calls.some((call: any) => call.method === "DELETE")).toBe(true); if (stage === "commit") { expect(run.parsed.commitFaultTriggered).toBe(true); + expect(run.parsed.error).not.toContain("client cleanup ownership unavailable"); expect(run.parsed.calls.some((call: any) => call.method === "POST" && call.url.endsWith("/api/keys"))).toBe(true); } expect(run.configBytes).not.toContain("issued-id"); diff --git a/tests/service/service-secrets.test.ts b/tests/service/service-secrets.test.ts index 3653e8b3676..71966490165 100644 --- a/tests/service/service-secrets.test.ts +++ b/tests/service/service-secrets.test.ts @@ -5,6 +5,7 @@ import { chmodSync, existsSync, lstatSync, + mkdirSync, mkdtempSync, readFileSync, renameSync, @@ -32,6 +33,10 @@ import { writeTokenBackup, } from "../../src/lib/service-secrets"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { loadConfig, saveConfig } from "../../src/config"; +import { readClientConnectionState } from "../../src/client/state"; +import { clearClientConnectPending, markClientConnectPending } from "../../src/client/state"; +import { removeServiceTokenAfterUninstall } from "../../src/service/cli"; let home = ""; const previousHome = process.env.OPENCODEX_HOME; @@ -47,6 +52,113 @@ afterEach(() => { if (home) removeTreeWithRetry(home); }); +describe("service uninstall credential ownership", () => { + /** Run cleanup under the same synthetic lifecycle lock as a client connection. */ + function cleanup(): "removed" | "absent" | "retained" | "unverified" { + return removeServiceTokenAfterUninstall({ lockPath: join(home, "lifecycle.sqlite") }); + } + + test("keeps the connected client's key while removing the service", () => { + const token = writeServiceApiTokenFile("ocx_client_key"); + const config = loadConfig(); + config.runtimeRole = "client"; + config.client = { + serverUrl: "https://hub.example.test", managementUrl: "https://hub.example.test", + managementTransport: "direct", selectedClients: ["codex"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", apiKeyId: "client-key", + tokenFingerprint: token.fingerprint, protocolVersion: 1, + connectedAt: "2026-09-06T00:00:00.000Z", + }; + saveConfig(config); + expect(readClientConnectionState().kind).toBe("connected"); + + expect(cleanup()).toBe("retained"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_client_key\n"); + rmSync(token.path); + expect(cleanup()).toBe("absent"); + }); + + test("removes an unowned service token", () => { + const token = writeServiceApiTokenFile("ocx_service_key"); + expect(readClientConnectionState().kind).toBe("disconnected"); + expect(cleanup()).toBe("removed"); + expect(existsSync(token.path)).toBe(false); + expect(cleanup()).toBe("absent"); + }); + + test("retains a key owned by a pending connection", () => { + const token = writeServiceApiTokenFile("ocx_pending_client_key"); + markClientConnectPending(token.fingerprint); + expect(readClientConnectionState().kind).toBe("disconnected"); + expect(cleanup()).toBe("retained"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_pending_client_key\n"); + clearClientConnectPending(token.fingerprint); + expect(cleanup()).toBe("removed"); + }); + + test("a stale pending marker does not retain a replacement service key", () => { + const staleFingerprint = serviceApiTokenFingerprint("ocx_old_client_key"); + markClientConnectPending(staleFingerprint); + const token = writeServiceApiTokenFile("ocx_replacement_service_key"); + expect(cleanup()).toBe("removed"); + expect(existsSync(token.path)).toBe(false); + expect(readFileSync(join(home, "client-connect-pending"), "utf8")).toBe(`${staleFingerprint}\n`); + }); + + for (const marker of ["", "z".repeat(64) + "\n", "a".repeat(66)]) { + test(`malformed pending marker of length ${marker.length} leaves cleanup unverified`, () => { + const token = writeServiceApiTokenFile("ocx_uncertain_pending_key"); + writeFileSync(join(home, "client-connect-pending"), marker); + expect(cleanup()).toBe("unverified"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_uncertain_pending_key\n"); + }); + } + + test("unsafe or unreadable pending markers preserve the key without claiming ownership", () => { + const token = writeServiceApiTokenFile("ocx_unreadable_pending_key"); + const markerPath = join(home, "client-connect-pending"); + mkdirSync(markerPath); + expect(cleanup()).toBe("unverified"); + nodeFs.rmdirSync(markerPath); + markClientConnectPending(token.fingerprint); + const original = nodeFs.readFileSync; + const read = spyOn(nodeFs, "readFileSync").mockImplementation(((path: any, ...args: any[]) => { + if (path === markerPath) throw Object.assign(new Error("fixture marker read failure"), { code: "EACCES" }); + return original(path, ...args); + }) as typeof nodeFs.readFileSync); + try { expect(cleanup()).toBe("unverified"); } + finally { read.mockRestore(); } + expect(readFileSync(token.path, "utf8")).toBe("ocx_unreadable_pending_key\n"); + }); + + test("keeps the token when client metadata is incomplete", () => { + const token = writeServiceApiTokenFile("ocx_uncertain_key"); + writeFileSync(join(home, "config.json"), JSON.stringify({ runtimeRole: "client" })); + expect(readClientConnectionState().kind).toBe("mismatched"); + + expect(cleanup()).toBe("retained"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_uncertain_key\n"); + }); + + test("keeps the token when client metadata is invalid", () => { + const token = writeServiceApiTokenFile("ocx_invalid_client_key"); + writeFileSync(join(home, "config.json"), JSON.stringify({ runtimeRole: "client", client: {} })); + expect(readClientConnectionState().kind).toBe("invalid"); + + expect(cleanup()).toBe("retained"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_invalid_client_key\n"); + }); + + test("reports unavailable lifecycle ownership separately from retained keys", () => { + const token = writeServiceApiTokenFile("ocx_unverified_key"); + const blockedLock = join(home, "blocked-lifecycle-lock"); + mkdirSync(blockedLock); + + expect(removeServiceTokenAfterUninstall({ lockPath: blockedLock })).toBe("unverified"); + expect(readFileSync(token.path, "utf8")).toBe("ocx_unverified_key\n"); + }); +}); + /** * #4236. A service boot always saw OPENCODEX_API_AUTH_TOKEN, because the launchd plist and the * systemd unit cat the token file into the environment before exec. A foreground `ocx start` saw