From 80e07d0ff8f0ed7d28402243d25e4112cc022368 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 13 Sep 2026 15:56:36 +0900 Subject: [PATCH] fix(cli): bind hub status credentials to connection snapshots [skip ci] `ocx status` on a connected client read the connection snapshot and the current `service-api-token` independently, so a reconnect or a key rotation between the two reads could send the new token to the snapshot's hub, or the snapshot's hub the new connection's token. The window is real: `collectRemoteHubStatus` awaits a dynamic import before it reads credentials. It now rereads the persisted connection and passes a token only when that connection still matches the snapshot's `serverUrl`, `apiKeyId` and `connectedAt` AND the token fingerprint matches that connection's `tokenFingerprint`. Otherwise it skips the live request and falls back to the snapshot owner's cache, or reports `unavailable`. A withheld token now carries its own cause. `resolveHubState` reported every null token as "this client has no usable data-plane token", which is false for a client that reconnected and holds a perfectly good token for a different hub - the operator would go re-enroll a credential that is not the problem. The caller supplies the reason through the new `withheldTokenReason`, so a changed connection, a missing token file and a fingerprint mismatch are named separately. That folds the maintainer review finding on #4382; it is the same misdiagnosis class as #4169 in the stop path. Verification: bun test tests/cli/cli-status-hub-state.test.ts (20 pass), tests/clients/client-hub-state.test.ts + tests/server/v1-hub-state.test.ts (35 pass), bun run typecheck, bun run structure:check, bun run privacy:scan, and the docs-site build required by docs-site/AGENTS.md (441 pages) - the one actionable CodeRabbit finding on the source pull request. Carried from #4382 by @luvs01. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- .../src/content/docs/guides/remote-hub.md | 4 + .../src/content/docs/ko/guides/remote-hub.md | 7 ++ src/cli/status.ts | 46 ++++++-- src/client/hub-state.ts | 10 +- structure/clients/claude-desktop.md | 3 + structure/config.md | 3 + structure/ops/docs-and-release.md | 4 + structure/runtime.md | 11 ++ tests/cli/cli-status-hub-state.test.ts | 101 +++++++++++++++++- 9 files changed, 180 insertions(+), 9 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 1f2db4486c..dce98d2a86 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -92,6 +92,10 @@ local login state, and `ocx config show` on a client prints a `_remoteHub` note credentials and model availability live on the hub. The hub read uses the per-client data key only; no admin token and no provider secret ever reaches a client. +`ocx status` makes a live hub-state request only when the saved connection still matches the +status snapshot and the data-token file matches that connection. If either check fails, it skips +the request and shows matching cached hub state, or `unavailable` if no matching cache exists. + ## Linux systemd or macOS launchd Bind the data listener to the hub's Tailscale address, enable the loopback companion so the hub's 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 72e96ae263..f47ae658fe 100644 --- a/docs-site/src/content/docs/ko/guides/remote-hub.md +++ b/docs-site/src/content/docs/ko/guides/remote-hub.md @@ -36,6 +36,13 @@ ocx sync 허브가 발급한 클라이언트별 키는 권한이 제한된 `service-api-token` 파일에 저장됩니다. `config.json`에는 저장되지 않습니다. 연결 중 사용량은 허브 기록에서 해당 `apiKeyId`만 조회하고, 연결을 끊은 뒤에는 로컬 기록을 봅니다. 두 기록은 서로 복제되지 않습니다. +### 연결된 클라이언트의 상태 표시 + +연결된 클라이언트의 `ocx status`는 허브 상태를 표시합니다. 상태 수집 중 연결 정보가 +바뀌거나 데이터 토큰 파일이 현재 연결과 일치하지 않으면 실시간 허브 조회를 건너뜁니다. +이때 상태 조회 대상 연결의 캐시를 표시하고, 일치하는 캐시가 없으면 `unavailable`로 보고합니다. +`ocx status --json`의 `remoteHub.stateSource`는 `hub`, `cache`, `unavailable` 중 하나입니다. + ## systemd 또는 launchd 데이터 리스너는 허브의 Tailscale 주소에 바인드하고, 허브 자신의 프로세스가 같은 포트를 자격 증명 없이 쓸 수 있도록 루프백 companion을 켜고, 관리 평면은 따로 공개합니다. 아래 값은 예시입니다. diff --git a/src/cli/status.ts b/src/cli/status.ts index cc1f4506bf..e120b5de70 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -18,6 +18,7 @@ import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; import { claudeDesktopIntegrationEnabled } from "../codex/desired-state"; import { claudeDesktopPolicyHealth, probeClaudeDesktopPolicy, type ClaudeDesktopPolicyHealth } from "../claude/desktop-policy"; import { collectClientConnectionStatus, type ClientConnectionStatus } from "./connect"; +import { readClientConnectionState, sameClientConnectionOwner } from "../client/state"; import type { HubStateOAuthEntry, HubStateProvider } from "../remote/hub-state"; import type { HubStateSource } from "../client/hub-state"; import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; @@ -324,6 +325,35 @@ export function disconnectedRemoteHubStatus(): CliRemoteHubStatus { }; } +/** + * The data key this status snapshot may spend, and the cause when it may not. + * + * A reconnect or a rotation can replace both files between the snapshot and this read, so a + * matching cache owner alone does not authorize sending the current token. Withholding is only + * half the job: reporting every withheld case as "no usable data-plane token" is false for a + * client that reconnected and holds a perfectly good token for a different hub, and a cause the + * operator acts on has to be the real one (#4169 is the same defect in the stop path). + */ +function boundHubStateToken( + current: ReturnType, + token: ReturnType, + owner: { serverUrl: string; apiKeyId: string; connectedAt: string }, +): { token: string | null; withheldReason?: string } { + if (current.kind !== "connected") { + return { token: null, withheldReason: "this client is no longer connected" }; + } + if (!sameClientConnectionOwner(current.value, owner)) { + return { token: null, withheldReason: "the saved connection no longer matches the one this status reports" }; + } + if (token.kind !== "present") { + return { token: null, withheldReason: "this client has no usable data-plane token" }; + } + if (token.fingerprint !== current.value.tokenFingerprint) { + return { token: null, withheldReason: "the data-plane token no longer belongs to the saved connection" }; + } + return { token: token.token }; +} + /** * Ask the hub what it can serve, with a bounded read and a cache fallback. * @@ -340,14 +370,16 @@ export async function collectRemoteHubStatus( return disconnectedRemoteHubStatus(); } const { resolveHubState } = await import("../client/hub-state"); - const token = readServiceApiTokenState(); + const owner = { + serverUrl: connection.serverUrl, + apiKeyId: connection.apiKeyId, + connectedAt: connection.connectedAt, + }; + const bound = boundHubStateToken(readClientConnectionState(), readServiceApiTokenState(), owner); const resolved = await resolveHubState({ - owner: { - serverUrl: connection.serverUrl, - apiKeyId: connection.apiKeyId, - connectedAt: connection.connectedAt, - }, - token: token.kind === "present" ? token.token : null, + owner, + token: bound.token, + ...(bound.withheldReason ? { withheldTokenReason: bound.withheldReason } : {}), ...(options.fetchImpl ? { fetchImpl: options.fetchImpl } : {}), ...(options.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }), ...(options.now === undefined ? {} : { now: options.now }), diff --git a/src/client/hub-state.ts b/src/client/hub-state.ts index 655bcbdd9c..8b0916b514 100644 --- a/src/client/hub-state.ts +++ b/src/client/hub-state.ts @@ -161,6 +161,12 @@ export interface ResolveHubStateOptions { owner: HubStateOwner; /** The per-client data key. Null when the token file is missing or unsafe. */ token: string | null; + /** + * Why the caller withheld the token, when it withheld one it holds. A null token reads the + * same here whether the file is missing or the caller declined to send a perfectly good + * token for a different connection, and the operator acts on that difference. + */ + withheldTokenReason?: string; timeoutMs?: number; fetchImpl?: typeof fetch; now?: number; @@ -195,7 +201,9 @@ export async function resolveHubState(options: ResolveHubStateOptions): Promise< ? withAge("cache", cached.state, cached.fetchedAt, now, reason) : withAge("unavailable", null, undefined, now, reason); }; - if (!options.token) return fromCache("this client has no usable data-plane token"); + if (!options.token) { + return fromCache(options.withheldTokenReason ?? "this client has no usable data-plane token"); + } if (options.allowNetwork === false) return fromCache("a live hub read was not attempted"); let state: HubStateDTO; try { diff --git a/structure/clients/claude-desktop.md b/structure/clients/claude-desktop.md index 1412a498d1..c557b4ab2e 100644 --- a/structure/clients/claude-desktop.md +++ b/structure/clients/claude-desktop.md @@ -69,6 +69,9 @@ or profile-upload API. Thinking replay and prompt caching remain separate in #37 The shared Responses path follows the [bounded multipart recovery contract](../subagents.md#multipart-encrypted-task-recovery); credential admission and retry policy remain unchanged. +Connected `ocx status` diagnostics follow the shared +[status credential binding](../runtime.md#remote-hub-status-credential-binding). + ## Claude Desktop config-library resolution The Desktop profile writer and the management status probe share diff --git a/structure/config.md b/structure/config.md index 96f63e2a2e..1e30795cbe 100644 --- a/structure/config.md +++ b/structure/config.md @@ -209,6 +209,9 @@ the residual directory for manual review; there is no recursive-delete fallback. ## Remote client key files +The connection's `tokenFingerprint` participates in +[`ocx status` credential binding](runtime.md#remote-hub-status-credential-binding). + Client connection metadata stores a stable `apiKeyId` and a non-secret rotation `pendingOperation`. The current data secret remains only in `service-api-token`; a bounded rotation temporarily keeps the old secret in owner-only `service-api-token.prev`. Commit or recovery clears the marker before orphan cleanup. `ocx disconnect` is local-only and leaves remote revocation to the hub's **Integrations → API Keys** page. Hub and local usage stores are not mirrored. Codex display-cache expiry, retained main-policy evidence, and reset history follow the diff --git a/structure/ops/docs-and-release.md b/structure/ops/docs-and-release.md index 0aa5928944..c5f0aad889 100644 --- a/structure/ops/docs-and-release.md +++ b/structure/ops/docs-and-release.md @@ -308,6 +308,10 @@ The shared Responses path follows the [bounded multipart recovery contract](../s ## Remote Hub locale and release gate +The Remote Hub guide describes the +[status credential binding](../runtime.md#remote-hub-status-credential-binding) +and its matching-cache or `unavailable` result. + The Remote Hub guide and affected CLI, server-config, management-API, and dashboard references have eight sources: root English plus `fr`, `ko`, `zh-cn`, `zh-tw`, `ru`, `ja`, and `tr`. English is canonical; commands, defaults, endpoint auth, and warnings remain exact in translations. A release requires the remote-only focused/full gates, privacy scan, GUI/docs builds, protocol compatibility receipts, and the MAINTAINERS security review for the exact head. Codex display-cache expiry, retained main-policy evidence, and reset history follow the diff --git a/structure/runtime.md b/structure/runtime.md index 35b1e1f31a..48c3968db4 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -214,6 +214,17 @@ The shared Responses path follows the [bounded multipart recovery contract](suba `src/remote/protocol.ts` owns pure interval/feature negotiation. `src/remote/hub-state.ts` owns the `GET|HEAD /v1/hub-state` contract, its caps, and the parser both sides share. `src/client/hub-client.ts` owns bounded, schema-validated remote catalog consumption, hub-state reads, and key-id probes; `src/client/hub-state.ts` owns the resolution and the owner-stamped 0600 cache, and a failed read reports "unavailable" rather than degrading to the client's own local provider and login state. `src/client/hub-relay.ts` is a fixed-authority management relay with URL, header, body, redirect, and stream bounds. The public data listener remains the direct client→hub path; the loopback management ingress never serves data-plane routes. +### Remote Hub status credential binding + +`src/cli/status.ts` rereads the persisted client connection and `service-api-token` state before +requesting hub state. It passes a usable token only when the current connection matches the +status snapshot's `serverUrl`, `apiKeyId`, and `connectedAt`, and the token fingerprint matches +the current connection's `tokenFingerprint`. Otherwise it skips the live request and uses the +snapshot owner's matching cached hub state, or reports `unavailable`. +A withheld token carries its own cause into the reported `reason` through +`resolveHubState`'s `withheldTokenReason`, so a changed connection, a missing token file, and a +fingerprint mismatch are named separately rather than all reported as a missing data key. + Codex display-cache expiry, retained main-policy evidence, and reset history follow the [quota cache contract](providers/openai-tiers.md#quota-cache-and-short-window-history). diff --git a/tests/cli/cli-status-hub-state.test.ts b/tests/cli/cli-status-hub-state.test.ts index e396f023e1..d7f808b23b 100644 --- a/tests/cli/cli-status-hub-state.test.ts +++ b/tests/cli/cli-status-hub-state.test.ts @@ -27,6 +27,7 @@ import { type CliRemoteHubStatus, } from "../../src/cli/status"; import type { HubStateDTO } from "../../src/remote/hub-state"; +import { writeCachedHubState } from "../../src/client/hub-state"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; @@ -88,6 +89,27 @@ function jsonFetch(body: unknown): typeof fetch { })) as unknown as typeof fetch; } +function observedHubFetch() { + const requests: { url: string; token: string | null }[] = []; + const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { + requests.push({ + url: String(input), + token: new Headers(init?.headers).get("x-opencodex-api-key"), + }); + return Response.json(hubState()); + }) as typeof fetch; + return { requests, fetchImpl }; +} + +function connectionSnapshot() { + return { + state: "connected" as const, + serverUrl: "https://hub.example.test:8443", + apiKeyId: "status-hub-state", + connectedAt: "2026-09-06T00:00:00.000Z", + }; +} + beforeEach(() => { testHome = mkdtempSync(join(tmpdir(), "ocx-status-hub-")); process.env.OPENCODEX_HOME = testHome; @@ -103,10 +125,15 @@ afterEach(() => { describe("collectRemoteHubStatus", () => { test("a connected client with a live hub reports the hub's facts", async () => { writeConnectedHome(testHome, "https://hub.example.test:8443"); + const probe = observedHubFetch(); const remoteHub = await collectRemoteHubStatus( { state: "connected", serverUrl: "https://hub.example.test:8443", apiKeyId: "status-hub-state", connectedAt: "2026-09-06T00:00:00.000Z" }, - { fetchImpl: jsonFetch(hubState()) }, + { fetchImpl: probe.fetchImpl }, ); + expect(probe.requests).toEqual([{ + url: "https://hub.example.test:8443/v1/hub-state", + token: FIXTURE_TOKEN, + }]); expect(remoteHub.connected).toBe(true); expect(remoteHub.stateSource).toBe("hub"); expect(remoteHub.hubVersion).toBe("2.51.0"); @@ -115,6 +142,78 @@ describe("collectRemoteHubStatus", () => { expect(remoteHub.claudeCodeEnabled).toBe(true); }); + test.each([ + ["origin", { serverUrl: "https://other-hub.example.test" }], + ["key", { apiKeyId: "another-client-key" }], + ["enrollment", { connectedAt: "2026-09-07T00:00:00.000Z" }], + ])("a changed %s never sends a credential for the old snapshot", async (_label, changed) => { + const snapshot = connectionSnapshot(); + const config = connectedConfig(snapshot.serverUrl); + Object.assign(config.client, changed); + writeFileSync(join(testHome, "config.json"), JSON.stringify(config)); + writeFileSync(join(testHome, "service-api-token"), FIXTURE_TOKEN, { mode: 0o600 }); + const probe = observedHubFetch(); + + const remoteHub = await collectRemoteHubStatus(snapshot, { fetchImpl: probe.fetchImpl }); + + expect(probe.requests).toEqual([]); + expect(remoteHub.stateSource).toBe("unavailable"); + expect(remoteHub.providers).toEqual([]); + // The token file is intact and usable — it simply belongs to a different connection now. + // Reporting "no usable data-plane token" here would send the operator to reconnect a + // credential that is not the problem. + expect(remoteHub.reason).toContain("no longer matches"); + expect(remoteHub.reason).not.toContain("no usable data-plane token"); + }); + + test("a persistent token fingerprint mismatch never sends the changed token", async () => { + const snapshot = connectionSnapshot(); + writeConnectedHome(testHome, snapshot.serverUrl); + writeFileSync(join(testHome, "service-api-token"), "unowned-status-token", { mode: 0o600 }); + const probe = observedHubFetch(); + + const remoteHub = await collectRemoteHubStatus(snapshot, { fetchImpl: probe.fetchImpl }); + + expect(probe.requests).toEqual([]); + expect(remoteHub.stateSource).toBe("unavailable"); + expect(remoteHub.reason).toContain("data-plane token"); + }); + + test("a token rotation during the asynchronous boundary cannot reach the old hub", async () => { + const snapshot = connectionSnapshot(); + writeConnectedHome(testHome, snapshot.serverUrl); + const probe = observedHubFetch(); + const pending = collectRemoteHubStatus(snapshot, { fetchImpl: probe.fetchImpl }); + // The collector has yielded before reading credentials. Rotate the actual files before + // its continuation runs, including a fingerprint that legitimately owns the NEW token. + const config = connectedConfig("https://other-hub.example.test"); + const rotatedToken = "rotated-status-token"; + config.client.apiKeyId = "rotated-client-key"; + config.client.tokenFingerprint = createHash("sha256").update(rotatedToken).digest("hex"); + writeFileSync(join(testHome, "config.json"), JSON.stringify(config)); + writeFileSync(join(testHome, "service-api-token"), rotatedToken, { mode: 0o600 }); + + const remoteHub = await pending; + + expect(probe.requests).toEqual([]); + expect(remoteHub.stateSource).toBe("unavailable"); + }); + + test.each([true, false])("a mismatched connection uses only the snapshot's cache (matching=%s)", async matching => { + const snapshot = connectionSnapshot(); + const current = connectedConfig("https://other-hub.example.test"); + writeFileSync(join(testHome, "config.json"), JSON.stringify(current)); + writeFileSync(join(testHome, "service-api-token"), FIXTURE_TOKEN, { mode: 0o600 }); + expect(writeCachedHubState(matching ? snapshot : current.client, hubState(), "2026-09-06T00:00:00.000Z")).toBe(true); + const probe = observedHubFetch(); + + const remoteHub = await collectRemoteHubStatus(snapshot, { fetchImpl: probe.fetchImpl }); + + expect(probe.requests).toEqual([]); + expect(remoteHub.stateSource).toBe(matching ? "cache" : "unavailable"); + expect(remoteHub.providers).toEqual(matching ? hubState().providers : []); + }); + test("a client whose token file is missing is unavailable, not locally sourced", async () => { writeFileSync(join(testHome, "config.json"), JSON.stringify(connectedConfig("https://hub.example.test:8443"))); const remoteHub = await collectRemoteHubStatus(