diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 0754e133ee6..e6d2014e398 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -482,6 +482,11 @@ above already sets it. A whole-array set replaces the array, so when the hub alr run the line `invite` prints — it carries the existing ones plus the new origin. `ocx config get corsAllowOrigins` shows what is there now. +`invite` picks the origin from the config file, but a running hub checks it against the config it +is running with: `hub.managementPublicOrigin` is applied only at start, and a `corsAllowOrigins` edit +may not reach the running hub until it restarts. After changing either, restart the hub before +inviting; `ocx config set` prints a reminder for both keys and for `hub.managementIngress`. + `ocx hub invite --json` emits `{ code, expiresAt, dataUrl, managementUrl, command }` with `expiresAt` as ISO 8601. The code is a secret: single-use, five-minute lifetime, rate-limited at the hub, and not to be persisted, logged, or pasted into an issue. `--clients codex,claude` picks which client configs @@ -768,6 +773,12 @@ For a service rollback, stop the branch service and repair the prior release aga loopback browser origin, so an origin-bound grant could never match. Nothing was minted. Run the `ocx config set corsAllowOrigins` line the error prints, with the joining machine's proxy port. See [Inviting another machine](#inviting-another-machine). +- **`ocx hub invite` says `Minting a pairing code failed (origin-refused)`:** the running hub refused + the browser origin the invite names. It admits only the `hub.managementPublicOrigin` and + `corsAllowOrigins` it is running with, which can predate a recent `ocx config set`. Restart the hub + (the error prints the restart command for this install) and invite again. `(rate-limited)` means + the hub is limiting new codes (eight a minute, and a cap on unused ones); wait for earlier codes to + be used or expire (five minutes), then invite again. `(rejected)` is any other refusal. - **`ocx hub invite` says the advertised data origin would be this machine's own loopback:** the bind is loopback-only or a wildcard and `hub.dataPublicOrigin` is unset, so there is no address to advertise and nothing guesses a tailnet or LAN one. Nothing was minted. Set diff --git a/src/cli/config-command.ts b/src/cli/config-command.ts index 9ff69f6b0df..4100a7ac690 100644 --- a/src/cli/config-command.ts +++ b/src/cli/config-command.ts @@ -142,6 +142,26 @@ function pathSegments(path: string): string[] { return segments; } +/** + * #8319: `config set` writes the file, but a running proxy keeps checking pairing and CORS origins + * against the config it is running with. `hub` is live topology and is applied only at start, including the + * management listener (`hub.managementIngress`) + * (`LIVE_TOPOLOGY_KEYS`); `corsAllowOrigins` reaches the running config only when some later live + * save adopts the file. Saying only "Set ." left an operator retrying a refused + * `ocx hub invite` against the old list. Limited to these keys, so the note never claims a restart + * that is not needed. + */ +export function startupOnlyConfigNotes(path: string): string[] { + const segments = pathSegments(path); + if (segments[0] === "corsAllowOrigins") { + return ["A running OpenCodex may keep using its previous corsAllowOrigins until it restarts; restart it to apply the new value now."]; + } + if (segments[0] === "hub" && (segments.length === 1 || segments[1] === "managementPublicOrigin" || segments[1] === "managementIngress")) { + return [`A running OpenCodex applies ${segments.join(".")} only when it starts; restart it to use the new value.`]; + } + return []; +} + function getPath(root: unknown, path: string): unknown { let current = root; for (const segment of pathSegments(path)) { @@ -298,7 +318,10 @@ export async function handleConfigCommand(argv: string[]): Promise { : `config is ${outcome.reason}`); } printData({ ok: true, path, value: redact(savedValue, pathSegments(path).at(-1)) }, wantsJson, - [`${action === "unset" ? "Unset" : "Set"} ${path}.`]); + [ + `${action === "unset" ? "Unset" : "Set"} ${path}.`, + ...(outcome.status === "committed" ? startupOnlyConfigNotes(path) : []), + ]); return; } if (action === "validate") { diff --git a/src/cli/gui-pair-client.ts b/src/cli/gui-pair-client.ts index b83c8f0065d..c655ab8c686 100644 --- a/src/cli/gui-pair-client.ts +++ b/src/cli/gui-pair-client.ts @@ -31,7 +31,7 @@ import { export type GuiPairRequestResult = | { kind: "created"; grant: string; browserOrigin: string; serverOrigin: string; expiresAt: number } - | { kind: "unavailable"; reason: "unattested-target" | "runtime-mismatch" | "attestation" | "capability" | "transport" | "rejected" | "local-intent" }; + | { kind: "unavailable"; reason: "unattested-target" | "runtime-mismatch" | "attestation" | "capability" | "transport" | "rejected" | "local-intent" | "origin-refused" | "rate-limited" }; export interface GuiPairClientDeps { fetchImpl?: typeof fetch; @@ -57,6 +57,27 @@ function sameRuntime(left: RuntimePortState, right: RuntimePortState | null): bo && timingSafeEqual(leftSecret, rightSecret); } +/** + * Which refusal the grant route sent, as one fixed word; the body itself never surfaces. + * + * Only the route's plain 403 "GUI pairing grant refused" (no `code`) means its loaded config did not + * admit this origin, the one case an allow-list edit or a restart can cure (#8319). Its 429 is the + * per-minute grant limit. A coded refusal, a capability refusal and anything else stay `rejected`, + * so the caller does not send an operator to edit a correct allow-list (#8495 review). + * + * This is a heuristic: the route answers the same uncoded 403 for any unexpected failure inside + * grant creation, not only for a refused origin. Those are rare and the advice (check the two origin + * settings, restart) is harmless there; an explicit server code would make it exact. + */ +async function refusalReason(response: Response): Promise<"origin-refused" | "rate-limited" | "rejected"> { + if (response.status === 429) return "rate-limited"; + if (response.status !== 403) return "rejected"; + const body: unknown = await response.json().catch(() => null); + if (!body || typeof body !== "object" || Array.isArray(body)) return "rejected"; + const record = body as Record; + return record.error === "GUI pairing grant refused" && record.code === undefined ? "origin-refused" : "rejected"; +} + function parseCreatedResult(value: unknown, browserOrigin: string): GuiPairRequestResult | null { if (!value || typeof value !== "object" || Array.isArray(value)) return null; const record = value as Record; @@ -158,7 +179,7 @@ export async function requestBoundGuiPairingGrant( }, signal: AbortSignal.timeout(timeoutMs), }); - if (!response.ok) return { kind: "unavailable", reason: "rejected" }; + if (!response.ok) return { kind: "unavailable", reason: await refusalReason(response) }; const result = parseCreatedResult(await response.json().catch(() => null), browserOrigin); return result ?? { kind: "unavailable", reason: "rejected" }; } catch { diff --git a/src/cli/hub.ts b/src/cli/hub.ts index bb1d09f7659..dffa23b30ca 100644 --- a/src/cli/hub.ts +++ b/src/cli/hub.ts @@ -7,7 +7,7 @@ import { type GuiPairRequestResult, } from "./gui-pair-client"; import type { RuntimeApiDeps } from "./runtime-api"; -import { HUB_DISABLE_USAGE, runHubDisable, type HubDisableDeps } from "./hub-disable"; +import { HUB_DISABLE_USAGE, hubDisableRestartHint, runHubDisable, type HubDisableDeps } from "./hub-disable"; export const HUB_USAGE = "ocx hub invite [--json] [--data-url ] [--management-url ] [--clients codex,claude]\n " + HUB_DISABLE_USAGE; @@ -32,6 +32,8 @@ export interface HubCommandDeps extends RuntimeApiDeps, HubDisableDeps { browserOrigin: string, deps?: GuiPairClientDeps, ) => Promise; + /** Supervisor-aware "Restart to apply: …" line; injected so tests do not probe the machine. */ + restartHint?: () => string; } export interface HubInviteOptions { @@ -333,6 +335,21 @@ async function runInvite(args: string[], deps: HubCommandDeps): Promise }); if (result.kind !== "created") { console.error(`Minting a pairing code failed (${result.reason}).`); + if (result.reason === "origin-refused") { + // #8319: the CLI picks the origin from the config on disk, but the running hub checks it + // against the config it is running with, so an allow-list edited since then can be refused + // until a restart. The reason code alone did not say which origin, setting, or step. + console.error( + `The running hub refused to mint a code bound to browser origin ${browserOrigin}. It admits only ` + + "hub.managementPublicOrigin and the corsAllowOrigins entries it is running with, which can predate " + + "a recent 'ocx config set'. If you changed either, restart the hub and invite again.", + ); + console.error((deps.restartHint ?? hubDisableRestartHint)()); + } else if (result.reason === "rate-limited") { + // The 429 covers both the per-minute grant limit and the cap on unused codes, which only + // clears as earlier codes are used or expire (five minutes), so the advice fits both. + console.error("The hub is limiting new pairing codes (a per-minute limit and a cap on unused codes). Wait for earlier codes to be used or expire, then invite again."); + } return 1; } const payload: HubInvitePayload = { diff --git a/tests/cli/cli-config-command.test.ts b/tests/cli/cli-config-command.test.ts index b215439236f..f52bb58c970 100644 --- a/tests/cli/cli-config-command.test.ts +++ b/tests/cli/cli-config-command.test.ts @@ -116,6 +116,42 @@ describe("ocx config display redaction", () => { } }); + test("#8319: a key the running proxy reads only at startup says a restart is needed", () => { + const dir = freshConfig(); + try { + const cors = runCli(["config", "set", "corsAllowOrigins", '["http://localhost:10100"]'], { OPENCODEX_HOME: dir }); + expect(cors.status).toBe(0); + expect(cors.stdout).toContain("Set corsAllowOrigins."); + expect(cors.stdout).toContain("restart"); + + const hub = runCli(["config", "set", "hub", '{"managementPublicOrigin":"https://hub.test"}'], { OPENCODEX_HOME: dir }); + expect(hub.status).toBe(0); + expect(hub.stdout).toContain("restart"); + // The nested paths take the same note (#8495 review): the management listener and its origin + // are created when the hub starts. + const origin = runCli(["config", "set", "hub.managementPublicOrigin", '"https://hub2.test"'], { OPENCODEX_HOME: dir }); + expect(origin.status).toBe(0); + expect(origin.stdout).toContain("restart"); + const ingress = runCli(["config", "set", "hub.managementIngress", '{"enabled":false}'], { OPENCODEX_HOME: dir }); + expect(ingress.status).toBe(0); + expect(ingress.stdout).toContain("restart"); + + // The JSON payload keeps its shape; the note is for people reading the terminal. + const json = runCli(["config", "set", "corsAllowOrigins", '["http://localhost:9999"]', "--json"], { OPENCODEX_HOME: dir }); + expect(json.status).toBe(0); + expect(Object.keys(JSON.parse(json.stdout)).sort()).toEqual(["ok", "path", "value"]); + + // Nothing is said for an unchanged value or for an unrelated key. + const again = runCli(["config", "set", "corsAllowOrigins", '["http://localhost:9999"]'], { OPENCODEX_HOME: dir }); + expect(again.stdout).not.toContain("restart"); + const other = runCli(["config", "set", "defaultProvider", '"blsc"'], { OPENCODEX_HOME: dir }); + expect(other.status).toBe(0); + expect(other.stdout).not.toContain("restart"); + } finally { + removeTreeWithRetry(dir); + } + }); + test("config show --json never prints secret-shaped modelCosts keys", () => { const dir = freshConfig(); try { diff --git a/tests/cli/hub-invite.test.ts b/tests/cli/hub-invite.test.ts index 142dbd02f9e..1dfbec4c789 100644 --- a/tests/cli/hub-invite.test.ts +++ b/tests/cli/hub-invite.test.ts @@ -61,6 +61,7 @@ async function invite( const code = await runHubCommand(args, { loadConfig: () => config, findLiveProxy: async () => (options.live === undefined ? LIVE : options.live), + restartHint: () => "Restart to apply: ocx restart.", requestPairingGrant: async (_target, browserOrigin) => { boundOrigin = browserOrigin; return options.result ?? { @@ -345,6 +346,36 @@ describe("hub invite refuses before burning a code", () => { }); expect(refused.code).toBe(1); expect(refused.err.join(" ")).toContain("(attestation)"); + // Only an origin refusal can be cured by a restart, so other reasons do not suggest one. + expect(refused.err.join(" ")).not.toContain("Restart to apply"); + }); + + test("#8319: an origin refusal names the bound origin, the settings, and the restart", async () => { + // The running hub checks the origin against the config it loaded at startup, while the CLI + // picks the origin from the config on disk. A corsAllowOrigins edit made after start is the + // reported trigger, and the reason code alone gave the operator nothing to act on. + const rejected = await invite(["invite"], hubConfig(), { + result: { kind: "unavailable", reason: "origin-refused" }, + }); + expect(rejected.code).toBe(1); + const stderr = rejected.err.join("\n"); + expect(stderr).toContain("Minting a pairing code failed (origin-refused)."); + expect(stderr).toContain("http://localhost:10100"); + expect(stderr).toContain("corsAllowOrigins"); + expect(stderr).toContain("hub.managementPublicOrigin"); + expect(stderr).toContain("Restart to apply: ocx restart."); + }); + + test("#8495 review: a rate limit or a generic refusal never blames the allow-list", async () => { + const limited = await invite(["invite"], hubConfig(), { result: { kind: "unavailable", reason: "rate-limited" } }); + expect(limited.code).toBe(1); + // The 429 covers both the per-minute limit and the cap on unused codes, so the advice fits both. + expect(limited.err.join(" ")).toContain("expire"); + for (const outcome of [limited, await invite(["invite"], hubConfig(), { result: { kind: "unavailable", reason: "rejected" } })]) { + const stderr = outcome.err.join(" "); + expect(stderr).not.toContain("corsAllowOrigins"); + expect(stderr).not.toContain("Restart to apply"); + } }); test("an unknown subcommand prints usage rather than guessing invite", async () => { diff --git a/tests/gui/gui-pair-client.test.ts b/tests/gui/gui-pair-client.test.ts index b12ecbdbacd..f4319e5b28b 100644 --- a/tests/gui/gui-pair-client.test.ts +++ b/tests/gui/gui-pair-client.test.ts @@ -228,6 +228,26 @@ describe("GUI pairing client", () => { }); expect(rejected).toEqual({ kind: "unavailable", reason: "rejected" }); expect(JSON.stringify(rejected)).not.toContain("secret-must-not-surface"); + + // #8495 review: the hub's two config-independent refusals must not read as an origin problem. + const respond = (response: () => Response) => requestBoundGuiPairingGrant(target, browserOrigin, { + readRuntime: () => ({ ...target, attestationSecret: secret }), + createChallenge: () => nonce, + fetchImpl: async (_input, init) => init?.method ? response() : proofResponse(init), + }); + expect(await respond(() => Response.json({ error: "GUI pairing grant refused" }, { status: 429, headers: { "Retry-After": "60" } }))) + .toEqual({ kind: "unavailable", reason: "rate-limited" }); + // The grant route's own refusal (its loaded config does not admit this origin) is the one case + // a corsAllowOrigins edit or a restart can cure. + expect(await respond(() => Response.json({ error: "GUI pairing grant refused" }, { status: 403 }))) + .toEqual({ kind: "unavailable", reason: "origin-refused" }); + // A coded refusal, a capability refusal, and a server error stay generic. + expect(await respond(() => Response.json({ error: "GUI pairing grant refused", code: "local_pairing_intent_required" }, { status: 403 }))) + .toEqual({ kind: "unavailable", reason: "rejected" }); + expect(await respond(() => Response.json({ error: "GUI pairing capability required" }, { status: 403 }))) + .toEqual({ kind: "unavailable", reason: "rejected" }); + expect(await respond(() => new Response("boom", { status: 500 }))) + .toEqual({ kind: "unavailable", reason: "rejected" }); }); }); diff --git a/tests/gui/gui-pair-http.test.ts b/tests/gui/gui-pair-http.test.ts index 3c6c3aeded8..101443eb52f 100644 --- a/tests/gui/gui-pair-http.test.ts +++ b/tests/gui/gui-pair-http.test.ts @@ -39,7 +39,7 @@ const MINT_STDERR_TAIL_BYTES = 4096; const GUI_PAIR_FAILURE_CLASSES = [ "usage", "origin-scope", "runtime-discovery", "standalone-target", "local-intent", "unattested-target", "runtime-mismatch", "attestation", "capability", "transport", - "rejected", "unknown", + "rejected", "origin-refused", "rate-limited", "unknown", ] as const; type GuiPairFailureClass = (typeof GUI_PAIR_FAILURE_CLASSES)[number]; @@ -61,7 +61,7 @@ function classifyGuiPairFailure(observed: GuiPairStderrObservation): GuiPairFail if (stderr.includes("No running attested OpenCodex proxy")) return "runtime-discovery"; if (stderr.includes("configured loopback address and port")) return "standalone-target"; if (stderr.includes("write access to the private OpenCodex home")) return "local-intent"; - const refused = stderr.match(/GUI pairing failed \((unattested-target|runtime-mismatch|attestation|capability|transport|rejected)\)/); + const refused = stderr.match(/GUI pairing failed \((unattested-target|runtime-mismatch|attestation|capability|transport|rejected|origin-refused|rate-limited)\)/); if (refused) return refused[1] as GuiPairFailureClass; return "unknown"; }