diff --git a/devlog/_plan/260911_hub_single_port/040_hub_token_ux.md b/devlog/_plan/260911_hub_single_port/040_hub_token_ux.md new file mode 100644 index 0000000000..59d04ab9fa --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/040_hub_token_ux.md @@ -0,0 +1,409 @@ +# 040 — PR4: data-plane token provisioning, `ocx hub invite`, the status hub block + +Unit: `devlog/_plan/260911_hub_single_port`. Stack position 4 of 5 as actually built (PR1 launchd +repair, PR2 same-port loopback companion, PR3 Claude/local-client destinations — written +concurrently by another agent — this one, then the docs/skill PR). Branch +`codex/260911-l4-hub-token-ux`, rebased onto `codex/260911-l4-hub-loopback-companion` = +`0aca6afe4` (`fix(codex): report the hub gate only when the toggle is on, and refuse every +wildcard spelling`) after that branch grew a commit mid-work; every count below is from the +rebased tree, and the rebase was clean (no shared files). Issue: lidge-jun/opencodex#4236. + +**Base discrepancy, recorded because it affects review.** The assignment described the base as +"dev + PR1 launchd fix + PR2 loopback companion". `git log babb76449..HEAD` on that branch shows +only the six PR2 commits; `src/service.ts` is untouched by it, and no +`devlog/_plan/260911_hub_single_port/010_launchd_repair.md` exists. So PR1 is **not** in this +branch's ancestry, and everything below was written and verified against dev + PR2 only. The +`src/service.ts` hunks here are all inside `assertNotAdminToken` / +`assertServiceAuthEnvironment` / `writeServiceApiTokenFile`, which the PR1 description (macOS +repair and status) does not name, so a later stack reorder should merge cleanly — but it has not +been proven against PR1's diff. + +## The incident this closes + +On the maintainer's hub the operator exported the **admin** token as +`OPENCODEX_API_AUTH_TOKEN`, because `ocx service install` refused to proceed without *a* token +and that was the token at hand. `assertNotAdminToken` then crash-looped the hub, and +`ocx service repair` demanded the same environment variable again — so the only remembered way +to make the command proceed was the exact thing that had broken it. + +The refusal was right. The demand was the defect: `assertServiceAuthEnvironment` threw for any +non-loopback hostname with no env token, **even when `~/.opencodex/service-api-token` already +held a perfectly good one**. Nobody should have to export a token by hand to run a hub. + +## What shipped + +### 1. The service provisions its own data-plane token (`src/service.ts`) + +`writeServiceApiTokenFile()` — still the single chokepoint every backend funnels through +(launchd, systemd, the Windows scheduler wrapper, WinSW native) — now resolves a token by +precedence instead of only copying the environment: + +1. `OPENCODEX_API_AUTH_TOKEN` from the installing shell. Still refused outright when it is an + admin token; an operator who deliberately exports a key keeps full control of it. +2. An existing owner-only `service-api-token`. **Reusing it is what makes `repair`, a reinstall + and a restart idempotent** — regenerating would silently invalidate every client key exchange + already performed against the old value. +3. 32 fresh random bytes, hex, written 0600 through the existing hardened writer + (`recordOwnedConfigPath` → `mkdir 0700` → `writeFileSync mode 0600` → `chmod` → + `hardenSecretPath` on Windows). This is the branch that removes the manual step. + +It returns `{ path, origin: "env" | "file" | "generated" }` and logs the **path**, never the +value. A loopback install with no env token still gets nothing: admission is not required there, +and on a machine connected to a hub that same file holds the hub's issued client key, which a +local install must not invent or clobber. + +`assertServiceAuthEnvironment` keeps exactly two refusals — the admin-token collision (checked +before the loopback short-circuit, unchanged) and a token file that exists but is `unsafe`, which +is reported here where the operator can still act rather than failing mid-install. The +"OPENCODEX_API_AUTH_TOKEN is required before installing…" throw is gone. + +The admin-token message now says what to do: `` Run `unset OPENCODEX_API_AUTH_TOKEN` and rerun: +nothing needs to be exported by hand, because the service provisions its own owner-only +data-plane token at . `` The old text offered "or set it to a distinct data-plane key", +which is how the operator got there. + +### 2. A foreground `ocx start` accepts the file-backed token (`src/lib/service-secrets.ts`) + +`assertServerAuthConfig` reads `configuredApiAuthToken`, which reads the environment — and under +a service the environment always has the token, because the launchd plist and the systemd unit +`cat` the file into it before exec. A foreground `ocx start` had neither, so it refused to bind a +non-loopback hostname the installed service on the same machine was serving happily. + +New `startupDataPlaneToken(env, { authRequired })` applies the wrappers' own precedence in one +place: env wins, then `OCX_API_TOKEN_FILE` (WinSW native mode, the existing +`loadServiceTokenFromFile`), then the installed `service-api-token`. `handleStart` calls it with +`isApiAuthRequired(loadConfig())`. + +`authRequired` is passed in rather than recomputed, for two reasons: this module must not load +config, and the installed file is deliberately **not** consulted on a loopback bind — the +connected-client case again. `assertServerAuthConfig` itself is unchanged, so no security-boundary +file moved: the fix is at the env-hydration layer the wrappers already occupy. + +### 3. `ocx hub invite` (`src/cli/hub.ts`, new) + +``` +ocx hub invite [--json] [--data-url ] [--management-url ] [--clients codex,claude] +``` + +Prints the line to run on the other machine: + +``` +# Run on the other machine: +echo '' | ocx connect --management-url --pairing-code-stdin +``` + +**No new management route was needed, and none was added.** The "connect pairing code" is exactly +a GUI pairing grant: `ocx connect --pairing-code-stdin` exchanges it at `POST /opencodex-session` +(`exchangeConnectPairingGrant`), and the mint is `POST /api/gui/pairing-grants` — the attested +local route `ocx gui pair` already drives, authorized by an HMAC over the running proxy's own +attestation secret from the owner-only runtime state file. So `invite` reuses +`requestBoundGuiPairingGrant` verbatim and needs **no admin token and nothing exported in the +shell**, which is strictly better than the admin-token path the plan sketched. `src/server/*` is +untouched by this PR. + +Origins: + +- data: `--data-url` → `hub.dataPublicOrigin` (new optional config) → `http://:`. +- management: `hub.managementPublicOrigin`, and that is not negotiable — `createGuiPairingGrant` + records it as the grant's `serverOrigin` and the exchange compares the request's management + origin against it. `--management-url` is therefore accepted only when it *equals* the + configured origin; a differing value is refused with both origins named, because printing it + would hand out a code the hub then rejects. + +Everything that cannot work is refused **before** a single-use code is minted: a non-hub +`runtimeRole`, a missing `hub.managementPublicOrigin`, a non-loopback plaintext management origin +(the hub's own `isPairingTransportPermitted` rule), a malformed `--data-url`, no running attested +proxy, and — the non-obvious one — a hub whose allow-list admits no loopback browser origin. +`ocx connect` sends `Origin: http://localhost:` (`localGuiOrigin` in +`src/client/connect.ts`), so only `hub.managementPublicOrigin` itself or a loopback entry of +`corsAllowOrigins` can ever match the grant. `selectInviteBrowserOrigin` picks from exactly that +set — `http://localhost:10100` when admitted, else the first admitted loopback origin — and +otherwise says which `ocx config set corsAllowOrigins` line to run. + +`--json` emits `{ code, expiresAt, dataUrl, managementUrl, command }` with `expiresAt` as ISO +8601. The code goes to stdout; the "secret, single-use" warning goes to stderr, matching +`ocx gui pair`. + +### 4. New optional config `hub.dataPublicOrigin` + +Same canonical-origin transform as `managementPublicOrigin` (http(s), no credentials, path, query +or fragment) and deliberately **not** `.catch`ed: silently dropping a typo would make `invite` +fall back to `http://:`, which is the value the field exists to replace. It is +advisory — the origin the hub advertises, never a bind address. It is its own field rather than a +derivation because on a real deployment the two are different sockets: management is a +loopback-only ingress published by Tailscale Serve on 443, data is the tailnet bind fronted on +its own port (`https://hub.tailnet.ts.net:8443`). + +### 5. The `ocx status` hub block (`src/cli/status.ts`) + +`collectHubStatus(config, listen, env)` adds a nullable `hub` field to `CliStatusJson` +(`schemaVersion` stays 1) and `hubStatusLines(hub)` renders it, so the sentences are testable +without spawning the CLI. On the maintainer's live hub: + +``` + Hub: + Data origin: http://localhost:10100 (derived from the bind address) + Loopback listener: ported on http://127.0.0.1:10104 — a second port local clients must be pointed at + Management ingress: http://127.0.0.1:10102 + Management origin: https://macmini.tail19a2d7.ts.net + Data token: present (file) at /.opencodex/service-api-token + Invite a machine: ocx hub invite +``` + +The listener line distinguishes PR2's two forms through `effectiveLoopbackListenerPort` plus the +presence of an explicit `port`: `companion` ("same port as the public listener, no credential +needed locally"), `ported`, and `off`. The token line reports the SOURCE only — +`present (env)` / `present (file)` / `unsafe (file)` / `missing` — with `env` winning, because +that is the service's own precedence: the wrapper exports the file only when the environment has +nothing, so naming the file first would name a source the running process is not using. A test +asserts no token value appears in either the JSON or the lines. + +> **Superseded by the review round below (§2 and §5).** The reasoning in that last sentence is +> backwards: the wrapper *overwrites* the environment from the file, so `present (env)` described +> the operator's own shell. The states are now `present (file)` / `unsafe (file)` / +> `admin-collision (file)` / `missing`, with the shell's variable as its own field. + +### 6. Help, registry, capabilities, skill surface + +`ocx hub` is a registry entry with a runner in `DISPATCH_COMMANDS`, two banner lines in +`printUsage`, and an `ocx help hub` topology paragraph (one port; companion listener; the token +file and that it is never copied; how invites work; `--json`). `ocx service` details gained five +lines on token auto-provisioning and the admin-token refusal. `ocx hub invite` is a declared +capability and `bun run skill:surface` regenerated +`skills/ocx/references/01_management_surface.md` (38 → 39 capabilities, 17 → 18 state-changing). + +## Decisions + +- **No new management route, and `src/server/index.ts` untouched.** See above: the route already + exists and its authorization (process attestation) is better suited to a local CLI than the + admin token. This also kept the PR entirely out of the other agent's files. +- **The `hub invite` capability declares `routes: []`.** It really does drive + `POST /api/gui/pairing-grants`, but that route is answered in the composition root ahead of + `handleManagementAPI`, so it is absent from `MANAGEMENT_ROUTES` and + `tests/cli/cli-capabilities.test.ts` would fail the declaration. The omission is explained in a + comment at the declaration rather than papered over; widening the registry's scanned scope to + `src/server/index.ts` is its own change. `ocx gui pair`, which drives the same route, has no + capability entry at all today. +- **`--management-url` is a confirmation, not an override.** The grant is bound to + `hub.managementPublicOrigin`; an override that differs cannot work, so it is refused with both + values named instead of printed. +- **`assertServerAuthConfig` was not touched.** The fix belongs where the wrappers already put + the token (env hydration before bind), not in a per-request admission helper that would then + read a file on every request. +- **An existing token is reused, never regenerated.** Rotation is `ocx connect rotate`'s job; + an install that silently minted a new hub secret would break every connected client. +- **`writeServiceApiTokenFile` kept its name and became exported**, so the provisioning + precedence is covered by behaviour tests rather than a source-oracle. The same identifier + exists in `src/lib/service-secrets.ts` with a different signature; they are in different + modules and a future collision inside `src/service.ts` would be a compile error, which is the + desired failure mode. +- The `ocx hub invite` pairing code is printed to stdout, exactly as `ocx gui pair` prints its + grant. `bun run privacy:scan` is green; no token value is logged anywhere this PR adds. + +## Verification (exact commands, this branch) + +``` +bun x tsc --noEmit # clean +bun run privacy:scan # Privacy scan passed +bun run skill:surface # regenerated, then: +bun test tests/ci-workflows/skill-ocx.test.ts tests/cli/cli-registry.test.ts \ + tests/cli/cli-capabilities.test.ts # 45 pass +bun test tests/service/service.test.ts # 212 pass +bun test tests/service/service-secrets.test.ts # 10 pass +bun test tests/cli/hub-invite.test.ts # 16 pass +bun test tests/cli/cli-status-json.test.ts # 53 pass +bun test tests/server/config.test.ts # 196 pass +bun test tests/cli/cli-dispatch.test.ts tests/cli/cli-help.test.ts # 56 pass +bun test tests/cli/cli-transport-honesty.test.ts # 22 pass +bun test tests/cli/cli-json-contract.test.ts # 8 pass +bun test tests/cli/cli-start-journal-order.test.ts # 3 pass +bun test tests/config/config-user-edits.test.ts # 46 pass +bun test tests/providers/opencode-cli.test.ts # 51 pass +bun test tests/service/winsw.test.ts # 25 pass +bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 17 pass +``` + +`bun test tests/server/server-auth.test.ts` is 111 pass / 1 fail on this branch **and on the +base with the working tree stashed** — `native passthrough upstream reset still logs 502 and +penalizes the pool` is a pre-existing failure, not a regression. + +Run the service and winsw files one at a time. Passing +`tests/service/winsw.test.ts` in the same `bun test` invocation as +`tests/cli/cli-transport-honesty.test.ts` trips the real-home guard through cross-file +`OPENCODEX_HOME` leakage; that is true on the base too. + +New test file registered in `scripts/test-layout/layout.json` and +`tests/fixtures/test-layout-expected.json`: `tests/cli/hub-invite.test.ts`. The hub status block, +the startup token precedence and the `hub.dataPublicOrigin` schema went into the existing +`tests/cli/cli-status-json.test.ts`, `tests/service/service-secrets.test.ts` and +`tests/server/config.test.ts`. + +No repository-wide suite (operator instruction); hosted CI at exact head is the proof. + +### Read-only checks against the live hub + +`ocx service …`, `ocx start`, `ocx ensure` and `ocx sync` were not run, and the live config was +not modified. Two read-only commands were: + +- `bun run src/cli/index.ts status` — rendered the hub block quoted above. +- `bun run src/cli/index.ts hub invite --json` — exited 1 with + `No loopback browser origin is admitted for pairing. Add the connecting machine's local origin: + ocx config set corsAllowOrigins '["http://localhost:10100"]'`, minting nothing. That hub has + `hostname: 127.0.0.1` and no `corsAllowOrigins`, so the refusal is correct; the successful + output was produced against an injected config and an injected mint. + +## Left for the rest of the stack + +- Docs PR: `guides/remote-hub.md` en + ko still tells the operator to + `export OPENCODEX_API_AUTH_TOKEN="$(openssl rand -hex 32)"` before `ocx service install`. That + step is now optional, and the guide should lead with `ocx hub invite` plus the new + `hub.dataPublicOrigin` field in `reference/configuration/server.md`. +- A hub bound to a non-loopback address needs `corsAllowOrigins` to name the connecting machine's + `http://localhost:` before `invite` can mint. That is an existing property of + `createGuiPairingGrant`, surfaced rather than changed here; whether a loopback client origin + should be admitted implicitly is a pairing-policy question for its own change. +- `ocx hub` has exactly one subcommand. Further hub-side verbs (listing or revoking issued client + keys from the CLI rather than the dashboard's **Integrations → API Keys**) are not in this PR. + +## Review round (PR #4252) + +Six findings, all accepted. Two were the same mistake in two places: the code trusted a value it +had already learned not to trust (the reused token file) and printed a value it had not checked +(the loopback data origin). The other four were honesty defects in text the operator is supposed +to paste or believe. + +### 1. `ocx hub invite` printed a loopback data origin (should-fix) + +`derivedHubDataOrigin` sends `0.0.0.0`, `::` and `127.0.0.1` all through `probeHostname`, which +spells every one of them as loopback — so on a hub bound to loopback or to a wildcard with no +`hub.dataPublicOrigin`, the "run on the other machine" line said `ocx connect +http://localhost:10100`. That tells the other machine to dial **itself**, and because the pairing +code is single-use it is spent on a connect that cannot succeed. The live hub on the maintainer's +Mac is exactly this shape (`hostname: 127.0.0.1`). + +New `resolveHubDataOrigin(override, configured, bindHostname, port)` returns either a usable +origin (tagged `flag` / `config` / `derived`) or `loopback-derived`, and `runInvite` refuses the +latter **before** the mint. An explicit `--data-url` or `hub.dataPublicOrigin` is never +second-guessed: a loopback data origin is legitimate over an SSH tunnel. + +A wildcard bind is refused the same way rather than resolved. There is no existing helper that +derives a tailnet or LAN address — `grep networkInterfaces src` has no hits, and `probeHostname` +deliberately collapses wildcards to loopback — so deriving one here would mean picking an +interface the operator never chose and advertising it in a credential exchange. The message names +which shape this hub has (`bindAddressPhrase`: wildcard vs loopback-only) and offers both the +persistent fix and the per-invite one. + +### 2. The reused `service-api-token` was never re-checked for the admin token (should-fix) + +This is the incident shape, still reachable on the first round's code: the `origin: "file"` branch +of `writeServiceApiTokenFile` returned early without calling `assertNotAdminToken`, so a file +holding the **management** token — hand-pasted pre-#2696, or written by the very incident this +unit closes — was silently accepted. `ocx status` said `present (file)` and the hub crash-looped +at boot, with nothing in any output naming the cause. + +- `assertNotAdminToken` gained a third argument, `source: "env" | "file"`. It selects the + **remedy**, not the rule: `unset OPENCODEX_API_AUTH_TOKEN` is meaningless advice about a file, + so the file message says to delete the file and rerun `ocx service repair` (or `install`). The + message deliberately does not call `serviceRetryCommand()`, which would pull `diagnoseService()` + — and `launchctl` — into an error path that runs at install time. +- The writer's file branch now calls it, and `assertServiceAuthEnvironment` checks the file too. +- **Both collision checks moved ahead of the loopback short-circuit.** `buildServiceShellCommand` + cats the token file into `OPENCODEX_API_AUTH_TOKEN` whenever the file exists, *whatever the + hostname*, so a loopback install with an admin-token file fences its management plane closed at + boot just the same. On a connected client the file holds the hub's issued client key, which is + never a management token, so the check is a no-op there. +- `ocx status` gained the state. `HubDataTokenState` now has `admin-collision (file)`, and the + block adds two lines naming the consequence and the fix, because "this is what a crash-looping + hub looks like in status output" is the sentence that was missing. +- The reused file is `chmod 0600`'d best-effort on the way through. + `readServiceApiTokenState` accepts any bounded regular file, so a reused token could be + group- or world-readable while install printed "owner-only". Best-effort because a non-owner + cannot chmod and failing the install over a loose mode would be worse than the loose mode. + +### 3. The bound browser origin was invisible (should-fix) + +`selectInviteBrowserOrigin` falls back to the first admitted loopback origin when +`http://localhost:10100` is not admitted, and the printed command carries no trace of it — but a +remote `ocx connect` sends `Origin: http://localhost:`, so a grant bound +to anything else is refused at the exchange and the code is spent with no hint. `ocx status` +cannot show it either; it is a property of the grant, not of the config. + +`inviteBoundOriginNotes` now prints the bound origin on **every** successful invite, in both +human and `--json` mode (stderr, where the single-use warning already lives, so the `--json` +envelope on stdout is unchanged). When it is not the default it also names the port the other +machine must be configured with, plus the alternative of admitting the default origin instead. + +### 4. Surfaced fix commands that do not run (nit) + +- `ocx config set hub.managementPublicOrigin …` exits `config parent path not found: hub` when + the `hub` object is absent (`setPath` in `src/cli/config-command.ts:60` walks only existing + parents) — and absent is precisely the config being advised. `configSetHubLines` prefixes + `ocx config set hub '{}'`, the way `guides/remote-hub.md` does, and **only** when `hub` is + actually missing, so the lines paste verbatim either way. +- `ocx config set corsAllowOrigins '[…]'` replaces the array, so the old one-element literal told + an operator with an existing allow-list to delete it. `appendCorsAllowOriginsCommand` emits the + current entries plus the new one (idempotent if already present), and the text points at + `ocx config get corsAllowOrigins`. + +### 5. `present (env)` described the operator's terminal, not the hub (nit) + +The status line read the CLI's own shell, but the launchd plist and the systemd unit overwrite +`OPENCODEX_API_AUTH_TOKEN` from the token file before exec — so on a service-run hub the label +named a source the running process was not using, which is the opposite of the first round's +stated reason for preferring `env`. The state is now always about the file, and the shell's +variable is reported as its own field, `dataTokenEnvInShell`, rendered as a sub-line ("the +installed service reads the file, not this"). It is kept rather than dropped because it does +decide what a foreground `ocx start` **in that same shell** would admit. + +### 6. A fourth `canonicalHttpOrigin` (nit) + +None of the three existing copies was exported. The new one is +`canonicalHttpOrigin` in `src/lib/gui-pair-capability.ts`, beside `canonicalGuiBrowserOrigin` +(origin canonicalisation is already that module's job, and it is the only module the CLI pairing +path and the hub command already share). `src/cli/hub.ts` and `src/cli/gui-pair-client.ts` now +import it; their local copies are gone. + +`src/config.ts` and `src/server/gui-session.ts` keep theirs **deliberately**: `gui-session.ts` is +a server security-boundary file and folding it in would also mean making it import nothing new, +which is true here — but this PR's security note rests on `src/server/*` having no diff at all, +and the config copy is reached from the zod schema path. Four copies became two plus one shared +export; collapsing the last two is its own change. + +### Not changed, and why + +- **No tailnet/LAN address derivation.** See finding 1: refusing is the honest answer until + something in the repo owns that discovery. +- **The `--json` envelope is unchanged** (`{ code, expiresAt, dataUrl, managementUrl, command }`). + The bound origin is operator advice, so it goes to stderr with the single-use warning rather + than growing the documented contract. +- **`ocx status` still does not show which origin a grant was bound to.** It cannot: grants are + not persisted anywhere status reads. + +### Verification (review round, this machine) + +``` +bun run typecheck # clean +bun run privacy:scan # Privacy scan passed +bun run skill:surface # regenerated (2 added capability detail lines) +bun test tests/cli/hub-invite.test.ts # 22 pass (was 16) +bun test tests/service/service.test.ts # 211 pass (was 209 here; +2 new) +bun test tests/service/service-secrets.test.ts # 10 pass +bun test tests/service/winsw.test.ts # 25 pass +bun test tests/cli/cli-status-json.test.ts # 54 pass (was 53; one test split into two) +bun test tests/gui/gui-pair-client.test.ts # 4 pass +bun test tests/gui/gui-pair-capability.test.ts # 2 pass +bun test tests/cli/cli-registry.test.ts tests/cli/cli-capabilities.test.ts \ + tests/cli/cli-help.test.ts tests/ci-workflows/skill-ocx.test.ts # 62 pass +bun test tests/cli/cli-transport-honesty.test.ts # 22 pass +bun test tests/cli/cli-dispatch.test.ts # 39 pass +bun test tests/cli/cli-json-contract.test.ts # 8 pass +``` + +`tests/service/service.test.ts` is **209** on this branch with the working tree reverted, not the +212 recorded in the first round's table; the earlier figure does not reproduce here. The new +count is 211 = 209 + 2. + +No live-hub command was run this round, not even a read-only one: `ocx status` reaches +`diagnoseService()` → `probeLaunchdLoadState()` → `launchctl`, and the operator instruction for +this machine is to run no `launchctl` at all. Every sentence the status block can print is pinned +by `tests/cli/cli-status-json.test.ts` instead. No repository-wide suite, by the same instruction. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 032252ad37..ad361fc548 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -704,6 +704,7 @@ "history-ocx-compaction-recovery.test.ts": "codex-integration", "hyperbolic-provider.test.ts": "providers", "hub-gated-local-clients.test.ts": "cli", + "hub-invite.test.ts": "cli", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", diff --git a/skills/ocx/references/01_management_surface.md b/skills/ocx/references/01_management_surface.md index ba2b4b5823..7fcf629e50 100644 --- a/skills/ocx/references/01_management_surface.md +++ b/skills/ocx/references/01_management_surface.md @@ -394,6 +394,28 @@ JSON mode: `payload`. - Uses the exact upstream model ID after the first slash. Omitted cache rates default to zero; sibling model prices are preserved. +### `ocx hub invite` + +Mint a single-use pairing code on a hub and print the exact `ocx connect` line for one more machine. + +Drives no management route. + +| Flag | Value | Meaning | +|---|---|---| +| `--json` | boolean | Emit code, expiresAt, dataUrl, managementUrl, and command. | +| `--data-url` | string | Advertise this data origin instead of hub.dataPublicOrigin or the bind address. | +| `--management-url` | string | Confirm the management origin; it must equal hub.managementPublicOrigin. | +| `--clients` | string | Pre-select codex and/or claude in the printed connect command. | + +JSON mode: `envelope`. + +- Hub only: refuses when runtimeRole is not hub, and requires a running attested proxy. +- The code is secret, single-use and short-lived; it is bound to hub.managementPublicOrigin and to the connecting machine's loopback browser origin. +- The bound browser origin is always printed; when it is not http://localhost:10100 the warning names the port the connecting machine must use. +- Refuses when the advertised data origin would be loopback (a loopback or wildcard bind with no hub.dataPublicOrigin and no --data-url) rather than printing a line that dials the other machine itself. +- Prints no data-plane token. Remote machines receive their own revocable per-client key from the exchange. +- Mints through the attested local pairing-grant route, the same one ocx gui pair uses; no admin token is read. + ### `ocx connect rotate` Rotate the connected client's data key against the hub, with commit and abort. @@ -706,6 +728,6 @@ JSON mode: `payload`. ## Counts -- declared capabilities: 38 -- of those, state-changing: 17 +- declared capabilities: 39 +- of those, state-changing: 18 - head-resolved invocations: 2 diff --git a/src/cli/capabilities.ts b/src/cli/capabilities.ts index c6695be2ca..d9aa8d0402 100644 --- a/src/cli/capabilities.ts +++ b/src/cli/capabilities.ts @@ -132,6 +132,34 @@ export const CAPABILITIES: readonly Capability[] = [ json: "envelope", details: ["Reads /healthz plus local config; drives no management API route."], }, + { + command: ["hub", "invite"], + summary: "Mint a single-use pairing code on a hub and print the exact `ocx connect` line for one more machine.", + // Deliberately empty. The command DOES drive `POST /api/gui/pairing-grants` -- the attested + // local mint route `ocx gui pair` uses, authorized by a capability HMAC'd with the running + // proxy's own attestation secret rather than by the admin token, which is why it needs + // nothing exported in the shell. That route is answered in the composition root, ahead of + // `handleManagementAPI`, so it is not in MANAGEMENT_ROUTES; declaring it here would fail the + // capability/registry reconciliation rather than inform anyone. Widening the registry's scope + // to `src/server/index.ts` is its own change. + routes: [], + flags: [ + { name: "--json", value: "boolean", summary: "Emit code, expiresAt, dataUrl, managementUrl, and command." }, + { name: "--data-url", value: "string", summary: "Advertise this data origin instead of hub.dataPublicOrigin or the bind address." }, + { name: "--management-url", value: "string", summary: "Confirm the management origin; it must equal hub.managementPublicOrigin." }, + { name: "--clients", value: "string", summary: "Pre-select codex and/or claude in the printed connect command." }, + ], + mutates: true, + json: "envelope", + details: [ + "Hub only: refuses when runtimeRole is not hub, and requires a running attested proxy.", + "The code is secret, single-use and short-lived; it is bound to hub.managementPublicOrigin and to the connecting machine's loopback browser origin.", + "The bound browser origin is always printed; when it is not http://localhost:10100 the warning names the port the connecting machine must use.", + "Refuses when the advertised data origin would be loopback (a loopback or wildcard bind with no hub.dataPublicOrigin and no --data-url) rather than printing a line that dials the other machine itself.", + "Prints no data-plane token. Remote machines receive their own revocable per-client key from the exchange.", + "Mints through the attested local pairing-grant route, the same one ocx gui pair uses; no admin token is read.", + ], + }, { command: ["connect", "rotate"], summary: "Rotate the connected client's data key against the hub, with commit and abort.", diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index a02f1d6f41..768c86bf78 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -583,6 +583,13 @@ const commandRunners: Record = { }, }); }, + hub: async deps => { + const { runHubCommand } = await import("./hub"); + return runHubCommand(deps.args.slice(1), { + loadConfig: deps.loadConfig, + findLiveProxy: deps.findLiveProxy, + }); + }, service: async deps => { process.exitCode = 0; await deps.serviceCommand(...deps.args.slice(1)); diff --git a/src/cli/gui-pair-client.ts b/src/cli/gui-pair-client.ts index 87a164059d..4e03c146e4 100644 --- a/src/cli/gui-pair-client.ts +++ b/src/cli/gui-pair-client.ts @@ -17,6 +17,7 @@ import { GUI_PAIR_NONCE_HEADER, GUI_PAIR_PATH, canonicalGuiBrowserOrigin, + canonicalHttpOrigin, createGuiPairCapability, } from "../lib/gui-pair-capability"; import { directLocalHttpFetch } from "../server/direct-local-http"; @@ -52,18 +53,6 @@ function sameRuntime(left: RuntimePortState, right: RuntimePortState | null): bo && timingSafeEqual(leftSecret, rightSecret); } -function canonicalHttpOrigin(value: unknown): string | null { - if (typeof value !== "string") return null; - try { - const parsed = new URL(value); - if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return null; - if (parsed.username || parsed.password || parsed.pathname !== "/" || parsed.search || parsed.hash) return null; - return parsed.origin; - } catch { - return null; - } -} - function parseCreatedResult(value: unknown, browserOrigin: string): GuiPairRequestResult | null { if (!value || typeof value !== "object" || Array.isArray(value)) return null; const record = value as Record; diff --git a/src/cli/help.ts b/src/cli/help.ts index 43916695b6..4137dabda7 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -56,6 +56,8 @@ Usage: ocx logout Remove a stored OAuth login ocx gui [pair --origin [--json]] Open the dashboard or create a single-use remote pairing grant + ocx hub invite [--json] Print a ready-to-run \`ocx connect\` line for one more machine + (hub only; see \`ocx help hub\` for the one-port topology) ocx update [--tag ] Update opencodex (keeps preview installs on @preview) ocx restart Stop and restart the proxy ocx v2 multi_agent_v2 surface (status|on|off|mode|keep-native-v1|threads|mode-hint) @@ -99,6 +101,7 @@ Examples: ocx start Start on default port (10100) ocx start --port 8080 Start on custom port ocx help service Show service command help + ocx help hub Explain the hub topology, token file, and invites ocx sync Sync available models to Codex`); } diff --git a/src/cli/hub.ts b/src/cli/hub.ts new file mode 100644 index 0000000000..e3edcedc23 --- /dev/null +++ b/src/cli/hub.ts @@ -0,0 +1,367 @@ +import type { OcxConfig, OcxConnectedClientId } from "../types"; +import { canonicalGuiBrowserOrigin, canonicalHttpOrigin } from "../lib/gui-pair-capability"; +import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; +import { + requestBoundGuiPairingGrant, + type GuiPairClientDeps, + type GuiPairRequestResult, +} from "./gui-pair-client"; +import type { RuntimeApiDeps } from "./runtime-api"; + +export const HUB_USAGE = + "ocx hub invite [--json] [--data-url ] [--management-url ] [--clients codex,claude]"; + +const PAIRING_WARNING = "Pairing codes are secret, single-use, and expire quickly. Do not save them."; + +/** + * The default browser origin a connecting machine presents. + * + * `ocx connect --pairing-code-stdin` exchanges the code with `Origin:` set by `localGuiOrigin()` + * in `src/client/connect.ts` — `http://localhost:`, which on a + * fresh client is the default 10100. The hub cannot observe the other machine's port, so the + * grant is bound to this origin unless `corsAllowOrigins` names a different loopback one. + */ +const DEFAULT_CLIENT_BROWSER_ORIGIN = "http://localhost:10100"; + +export interface HubCommandDeps extends RuntimeApiDeps { + loadConfig: () => OcxConfig; + findLiveProxy?: () => Promise; + requestPairingGrant?: ( + target: LiveProxy, + browserOrigin: string, + deps?: GuiPairClientDeps, + ) => Promise; +} + +export interface HubInviteOptions { + json: boolean; + dataUrl?: string; + managementUrl?: string; + clients?: string; +} + +export interface HubInvitePayload { + code: string; + expiresAt: string; + dataUrl: string; + managementUrl: string; + command: string; +} + +function isLoopbackOrigin(origin: string): boolean { + try { + const host = new URL(origin).hostname.toLowerCase(); + return host === "localhost" || host === "127.0.0.1" || host === "::1" || host === "[::1]"; + } catch { + return false; + } +} + +/** Loopback or HTTPS, the rule `consumeGuiPairingGrant` enforces on the hub side. */ +export function pairingOriginUsable(origin: string): boolean { + try { + return new URL(origin).protocol === "https:" || isLoopbackOrigin(origin); + } catch { + return false; + } +} + +export function parseHubInviteArgs(args: string[]): HubInviteOptions | null { + const options: HubInviteOptions = { json: false }; + for (let index = 0; index < args.length; index++) { + const arg = args[index]!; + if (arg === "--json" && !options.json) { + options.json = true; + continue; + } + if (arg === "--data-url" || arg === "--management-url" || arg === "--clients") { + const value = args[++index]; + if (!value || value.startsWith("--")) return null; + const key = arg === "--data-url" ? "dataUrl" : arg === "--management-url" ? "managementUrl" : "clients"; + if (options[key] !== undefined) return null; + options[key] = value; + continue; + } + return null; + } + return options; +} + +export function parseInviteClients(raw: string | undefined): OcxConnectedClientId[] | null { + if (raw === undefined) return []; + const values = raw.split(",").map(value => value.trim()).filter(Boolean); + if (values.length < 1 || values.some(value => value !== "codex" && value !== "claude")) return null; + return values as OcxConnectedClientId[]; +} + +/** + * The browser origin the grant is bound to, or null when the hub admits none. + * + * `createGuiPairingGrant` accepts only `hub.managementPublicOrigin` itself or an entry of + * `corsAllowOrigins`, so this picks from exactly that set rather than guessing: the default + * client origin when it is admitted, otherwise the first admitted loopback origin. A hub whose + * allow-list names no loopback origin cannot pair a remote `ocx connect` at all, and saying so + * here is better than minting a code the exchange will reject. + */ +export function selectInviteBrowserOrigin(config: OcxConfig): string | null { + const allowed = [ + canonicalGuiBrowserOrigin(config.hub?.managementPublicOrigin ?? ""), + ...(config.corsAllowOrigins ?? []).map(value => canonicalGuiBrowserOrigin(value)), + ].filter((value): value is string => Boolean(value)); + if (allowed.includes(DEFAULT_CLIENT_BROWSER_ORIGIN)) return DEFAULT_CLIENT_BROWSER_ORIGIN; + return allowed.find(origin => isLoopbackOrigin(origin)) ?? null; +} + +/** `http://:` — right for a plain tailnet/LAN bind with no TLS frontend. */ +export function derivedHubDataOrigin(hostname: string | undefined, port: number): string { + const host = probeHostname(hostname); + return `http://${host === "127.0.0.1" ? "localhost" : host}:${port}`; +} + +/** + * The `ocx config set` lines that will actually work on THIS config. + * + * `setPath` in `src/cli/config-command.ts` walks only parents that already exist, so + * `ocx config set hub. …` exits with `config parent path not found: hub` on a config + * that has no `hub` object yet — which is exactly the config that needs the advice. Create the + * parent first, the way `guides/remote-hub.md` does, and only when it is actually missing, so + * the operator can paste the lines verbatim either way. + */ +export function configSetHubLines( + config: Pick, + field: "managementPublicOrigin" | "dataPublicOrigin", + example: string, +): string[] { + const set = `ocx config set hub.${field} '${JSON.stringify(example)}'`; + return config.hub ? [set] : ["ocx config set hub '{}'", set]; +} + +/** + * A `corsAllowOrigins` line that ADDS an origin instead of replacing the list. + * + * `ocx config set corsAllowOrigins '[…]'` overwrites the array, so printing a one-element + * literal tells an operator with an existing allow-list to delete it. The already-configured + * entries are known here, so the suggested value carries them. + */ +export function appendCorsAllowOriginsCommand( + config: Pick, + origin: string, +): string { + const current = config.corsAllowOrigins ?? []; + const next = current.includes(origin) ? current : [...current, origin]; + return `ocx config set corsAllowOrigins '${JSON.stringify(next)}'`; +} + +export type HubDataOriginResolution = + | { kind: "usable"; dataUrl: string; source: "flag" | "config" | "derived" } + /** The bind address can only be spelled as loopback, so there is nothing to advertise. */ + | { kind: "loopback-derived"; dataUrl: string; bindHostname: string }; + +/** + * The data origin to advertise, or a refusal. + * + * `derivedHubDataOrigin` maps a loopback bind AND every wildcard spelling to + * `http://localhost:` (via `probeHostname`), which on the other machine means "dial + * yourself". Printing it burns the single-use code on a connect that cannot succeed, so a + * derived loopback origin is a refusal rather than a value. A wildcard bind is refused the same + * way on purpose: nothing in this repo derives a tailnet or LAN address, and guessing one from + * `os.networkInterfaces()` would advertise an interface the operator never chose. + * + * An explicit `--data-url` or `hub.dataPublicOrigin` is never second-guessed — a loopback data + * origin is legitimate when the "other machine" is reached through an SSH tunnel. + */ +export function resolveHubDataOrigin( + override: string | null, + configured: string | undefined, + bindHostname: string | undefined, + port: number, +): HubDataOriginResolution { + if (override) return { kind: "usable", dataUrl: override, source: "flag" }; + const fromConfig = canonicalHttpOrigin(configured); + if (fromConfig) return { kind: "usable", dataUrl: fromConfig, source: "config" }; + const derived = derivedHubDataOrigin(bindHostname, port); + if (!isLoopbackOrigin(derived)) return { kind: "usable", dataUrl: derived, source: "derived" }; + const trimmed = (bindHostname ?? "").trim(); + return { kind: "loopback-derived", dataUrl: derived, bindHostname: trimmed || "127.0.0.1" }; +} + +/** Wildcards and loopback fail for different reasons; say which one this hub has. */ +function bindAddressPhrase(bindHostname: string): string { + return bindHostname === "0.0.0.0" || bindHostname === "::" || bindHostname === "[::]" + ? `the bind address ${bindHostname} is a wildcard, which names no address another machine can dial` + : `the bind address ${bindHostname} is loopback-only`; +} + +/** + * What an operator has to know about the origin the grant actually got bound to. + * + * `selectInviteBrowserOrigin` falls back to the first admitted loopback origin when + * `http://localhost:10100` is not admitted, and a remote `ocx connect` sends + * `Origin: http://localhost:` — so a grant bound to anything else is + * refused at the exchange and the single-use code is spent with nothing printed to explain it. + * Always stating the bound origin, and naming the port the client needs when it differs, is the + * difference between a fixable failure and a mystery. + */ +export function inviteBoundOriginNotes( + browserOrigin: string, + config: Pick, +): string[] { + const notes = [`Bound browser origin: ${browserOrigin} — the connecting machine must present exactly this.`]; + if (browserOrigin === DEFAULT_CLIENT_BROWSER_ORIGIN) return notes; + const parsed = new URL(browserOrigin); + const port = parsed.port || (parsed.protocol === "https:" ? "443" : "80"); + notes.push( + `That is NOT ${DEFAULT_CLIENT_BROWSER_ORIGIN}, which is what an unconfigured client sends: the other machine ` + + `must already be running on port ${port} ('ocx config set port ${port}' there) before it runs the line ` + + "below, or the hub refuses the exchange and the code is spent.", + ); + notes.push( + "To accept a default client instead, admit its origin on this hub: " + + appendCorsAllowOriginsCommand(config, DEFAULT_CLIENT_BROWSER_ORIGIN), + ); + return notes; +} + +export function hubInviteCommand( + code: string, + dataUrl: string, + managementUrl: string, + clients: OcxConnectedClientId[], +): string { + const clientsFlag = clients.length > 0 ? ` --clients ${clients.join(",")}` : ""; + return `echo '${code}' | ocx connect ${dataUrl} --management-url ${managementUrl}${clientsFlag} --pairing-code-stdin`; +} + +async function runInvite(args: string[], deps: HubCommandDeps): Promise { + const options = parseHubInviteArgs(args); + if (!options) { + console.error(`Usage: ${HUB_USAGE}`); + return 1; + } + const clients = parseInviteClients(options.clients); + if (!clients) { + console.error("--clients must contain codex and/or claude."); + return 1; + } + const config = deps.loadConfig(); + if (config.runtimeRole !== "hub") { + console.error( + `ocx hub invite runs on a hub; this machine's runtimeRole is "${config.runtimeRole ?? "standalone"}". ` + + "A client machine runs 'ocx connect' with the code its hub printed.", + ); + return 1; + } + // The grant's server origin IS hub.managementPublicOrigin (createGuiPairingGrant reads it, + // and the exchange compares the request's management origin against it), so an invite that + // advertised anything else would hand out a code the hub then refuses. + const managementPublic = canonicalHttpOrigin(config.hub?.managementPublicOrigin); + if (!managementPublic) { + console.error( + "hub.managementPublicOrigin is not set, so there is no origin to pair against. Set the exact " + + "browser-visible HTTPS origin:", + ); + for (const line of configSetHubLines(config, "managementPublicOrigin", "https://hub.tailnet.ts.net")) { + console.error(` ${line}`); + } + return 1; + } + const managementOverride = options.managementUrl === undefined ? null : canonicalHttpOrigin(options.managementUrl); + if (options.managementUrl !== undefined && !managementOverride) { + console.error("--management-url must be a bare http(s) origin with no path, query, or credentials."); + return 1; + } + if (managementOverride && managementOverride !== managementPublic) { + console.error( + `--management-url ${managementOverride} does not match hub.managementPublicOrigin ${managementPublic}. ` + + "The pairing code is bound to the configured origin, so the other machine would be refused.", + ); + return 1; + } + if (!pairingOriginUsable(managementPublic)) { + console.error( + `hub.managementPublicOrigin ${managementPublic} is non-loopback plain HTTP, which cannot carry a ` + + "pairing code. Put management behind an HTTPS frontend (Tailscale Serve) and set that origin.", + ); + return 1; + } + const dataOverride = options.dataUrl === undefined ? null : canonicalHttpOrigin(options.dataUrl); + if (options.dataUrl !== undefined && !dataOverride) { + console.error("--data-url must be a bare http(s) origin with no path, query, or credentials."); + return 1; + } + const browserOrigin = selectInviteBrowserOrigin(config); + if (!browserOrigin) { + // `ocx config set corsAllowOrigins` REPLACES the array, so the suggested value carries the + // entries this hub already has -- a one-element literal would tell the operator to drop them. + console.error( + "No loopback browser origin is admitted for pairing. Add the connecting machine's local origin " + + "(this keeps the entries already configured; 'ocx config get corsAllowOrigins' shows them): " + + appendCorsAllowOriginsCommand(config, DEFAULT_CLIENT_BROWSER_ORIGIN), + ); + return 1; + } + const target = await (deps.findLiveProxy ?? findLiveProxy)(); + if (!target) { + console.error("No running attested OpenCodex hub was found. Check 'ocx service status', then 'ocx service repair'."); + return 1; + } + const resolved = resolveHubDataOrigin( + dataOverride, + config.hub?.dataPublicOrigin, + target.hostname ?? config.hostname, + target.port, + ); + if (resolved.kind === "loopback-derived") { + console.error( + `The advertised data origin would be ${resolved.dataUrl} — this machine's own loopback — because ` + + `${bindAddressPhrase(resolved.bindHostname)}, and nothing here guesses a tailnet or LAN address. ` + + "The other machine would dial itself and the single-use code would be spent for nothing. Name the " + + "origin remote machines reach this hub's data plane on:", + ); + for (const line of configSetHubLines(config, "dataPublicOrigin", "https://hub.tailnet.ts.net:8443")) { + console.error(` ${line}`); + } + console.error(" ...or, for this invite only: ocx hub invite --data-url https://hub.tailnet.ts.net:8443"); + return 1; + } + const dataUrl = resolved.dataUrl; + const result = await (deps.requestPairingGrant ?? requestBoundGuiPairingGrant)(target, browserOrigin, { + ...(deps.fetchImpl ? { fetchImpl: deps.fetchImpl } : {}), + }); + if (result.kind !== "created") { + console.error(`Minting a pairing code failed (${result.reason}).`); + return 1; + } + const payload: HubInvitePayload = { + code: result.grant, + expiresAt: new Date(result.expiresAt).toISOString(), + dataUrl, + managementUrl: managementPublic, + command: hubInviteCommand(result.grant, dataUrl, managementPublic, clients), + }; + // Always, in both modes: the grant is bound to ONE browser origin and the operator cannot + // see which from the printed command (#4236 review). + for (const note of inviteBoundOriginNotes(browserOrigin, config)) console.error(note); + if (options.json) { + console.log(JSON.stringify(payload)); + console.error(PAIRING_WARNING); + return 0; + } + // Remaining time, not the constant TTL: the number an operator reads has to be the one + // they actually have left by the time the line is printed. + const ttlSeconds = Math.max(0, Math.round((result.expiresAt - Date.now()) / 1000)); + console.log(`Pairing code for one machine — single-use, expires in ${ttlSeconds}s (${payload.expiresAt}).`); + console.log(""); + console.log("# Run on the other machine:"); + console.log(payload.command); + console.error(PAIRING_WARNING); + return 0; +} + +export async function runHubCommand(args: string[], deps: HubCommandDeps): Promise { + if (args[0] !== "invite") { + console.error(`Usage: ${HUB_USAGE}`); + return 1; + } + return runInvite(args.slice(1), deps); +} diff --git a/src/cli/index.ts b/src/cli/index.ts index 4cb1e68ba9..730ec0c795 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -48,7 +48,7 @@ import { pendingTeardownPathFor, quarantinePendingTeardown, } from "../config/pending-teardown"; -import { collectStatus, unusedProxyWarningLines } from "./status"; +import { collectStatus, hubStatusLines, unusedProxyWarningLines } from "./status"; import { endpointsToProve, everyEndpointProvenDown, sharedTeardownAuthorized, type UninstallObservation } from "./uninstall-plan"; import { takeFlag } from "./runtime-api"; @@ -66,10 +66,11 @@ import { dispatchCommand , decideStartWithLiveOwner } from "./dispatch"; import { findAvailablePort, isAddrInUse, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../server/ports"; import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; import { createReadinessGate } from "../server/readiness"; +import { isApiAuthRequired } from "../server/auth-cors"; import { runReady, type ReadyArgs } from "./ready"; import { runCli } from "./root"; import { isProcessAlive, ProxyOwnershipRefusedError, refusalNextStep, stopProxy } from "../lib/process-control"; -import { loadServiceTokenFromFile } from "../lib/service-secrets"; +import { startupDataPlaneToken } from "../lib/service-secrets"; import { assertNotAdminToken, diagnoseService, isServiceOwnershipError, proxyStillLiveAfterStop, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalledDetailed, uninstallServiceIfInstalled, uninstallServiceDetailed } from "../service"; import { formatStartupRoutingDetail, startupHealthSummary } from "../codex/autostart-health"; import { injectSystemEnv, reconcileShellHook, revertSystemEnv, uninstallShellHook } from "../server/system-env"; @@ -291,10 +292,16 @@ async function findProxyOwnerBeforeJournalRecovery( } async function handleStart(options: { block?: boolean } = {}) { - // Native (WinSW) service mode has no batch wrapper to read the service token file - // into the environment, so the app loads it here before the server binds. The server + // Native (WinSW) service mode has no batch wrapper to read the service token file into + // the environment, and a FOREGROUND `ocx start` has no wrapper at all — so the app loads + // the token here, before the server binds, with the same precedence the launchd plist and + // the systemd unit use when they cat the file into the environment. Without the second + // source, `ocx start` refused to bind a non-loopback hostname (assertServerAuthConfig) + // that the installed service on the same machine was serving happily (#4236). The server // auth path reads OPENCODEX_API_AUTH_TOKEN from the environment. - const serviceToken = loadServiceTokenFromFile(process.env); + const serviceToken = startupDataPlaneToken(process.env, { + authRequired: isApiAuthRequired(loadConfig()), + }); if (serviceToken) process.env.OPENCODEX_API_AUTH_TOKEN = serviceToken; // The service wrapper (and WinSW via OCX_API_TOKEN_FILE) can still export a colliding // token that install now refuses to write. Refuse it here too, before bind, so an @@ -1427,6 +1434,11 @@ async function handleStatus() { console.log(` Runtime: ${status.json.paths.runtime}`); console.log(` Runtime source: ${status.json.runtime.source}${status.json.runtime.overrideEnv ? ` (${status.json.runtime.overrideEnv})` : ""}`); console.log(` Default provider: ${status.json.defaultProvider}`); + // One block rather than six scattered lines, and only on a hub: `hubStatusLines` owns the + // sentences so they are testable without spawning the CLI. It prints no token value. + if (status.json.hub) { + for (const line of hubStatusLines(status.json.hub)) console.log(` ${line}`); + } console.log(` Remote hub: ${status.json.connection.state}${status.json.connection.serverUrl ? ` (${status.json.connection.serverUrl})` : ""}`); if (status.json.connection.state === "invalid" || status.json.connection.state === "mismatched") { console.log(` ⚠️ ${status.json.connection.reason}`); diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 06b0dfaf29..7726fd1a47 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -68,6 +68,11 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ "`repair` refreshes the definition and reloads the manager only when something changed, so repairing a healthy service is not an outage.", "`restart` is the same refresh but always restarts: on macOS an unchanged, already-loaded job is kickstarted in place. Healthy Windows tasks are reused, while stale definitions may re-register and elevate.", "Use `ocx service status` to see diagnostics and log paths.", + "Data-plane token: nothing has to be exported by hand. On a non-loopback hostname install/repair uses", + "OPENCODEX_API_AUTH_TOKEN when set, otherwise reuses the existing owner-only ~/.opencodex/service-api-token,", + "otherwise generates one; the launch wrapper reads that file at start and the value never enters a plist,", + "a unit file, or argv. An ADMIN token in OPENCODEX_API_AUTH_TOKEN is refused -- unset it and rerun.", + "Repair and restart never ask for the environment variable again once the token file exists.", ], }, { @@ -156,6 +161,35 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ "The printed grant is secret, single-use, short-lived, and must not be persisted.", ], }, + { + name: "hub", + usage: "ocx hub invite [--json] [--data-url ] [--management-url ] [--clients codex,claude]", + summary: "Hub-side commands. `invite` prints a ready-to-run `ocx connect` line for one more machine.", + details: [ + "Topology: a hub serves ONE port. Remote machines dial `hostname:port` with their own per-client", + "key; the hub's own local processes dial `127.0.0.1:` with no credential, through the", + "loopback companion listener enabled by `unauthenticatedLoopbackListener: {\"enabled\": true}`.", + "The browser-facing management plane is separate: a loopback-only ingress published by an", + "operator-owned HTTPS frontend (Tailscale Serve) and advertised as hub.managementPublicOrigin.", + "", + "Nothing needs to be exported by hand. `ocx service install` provisions an owner-only data-plane", + "token at ~/.opencodex/service-api-token and the launch wrapper reads it at start; the value never", + "enters a plist, a unit file, argv, or the environment you typed in. Never copy that file to another", + "machine -- every client gets its own revocable key from the pairing exchange.", + "", + "invite requires a running hub and mints a single-use, short-lived pairing code through the same", + "attested local route `ocx gui pair` uses, then prints the exact command to run on the other", + "machine. The code is bound to hub.managementPublicOrigin and to the connecting machine's local", + "browser origin (`http://localhost:10100` unless corsAllowOrigins names another loopback origin);", + "the bound origin is always printed, and a different one names the port the client needs.", + "--data-url overrides the advertised data origin; hub.dataPublicOrigin is the persistent form, and", + "the fallback is http://:. A loopback or wildcard bind has no such address, so", + "invite refuses instead of advertising http://localhost:, which would tell the other machine", + "to dial itself and spend the code.", + "--json emits { code, expiresAt, dataUrl, managementUrl, command }. Do not persist the code.", + "Hub state, including the data token and the companion listener, is reported by `ocx status`.", + ], + }, { name: "update", usage: "ocx update [--tag latest|preview]", diff --git a/src/cli/status.ts b/src/cli/status.ts index 8590ba38ec..1b794eca25 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -18,10 +18,53 @@ import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; import { claudeDesktopIntegrationEnabled } from "../codex/desired-state"; import { claudeDesktopPolicyHealth, probeClaudeDesktopPolicy, type ClaudeDesktopPolicyHealth } from "../claude/desktop-policy"; import { collectClientConnectionStatus } from "./connect"; +import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; +import { tokenCollidesWithAdmin } from "../lib/admin-secrets"; export { proxyHealthFailureReason, isConnectionRefused, isUncleanExitEvidence, probeUncleanExitState } from "./status-probes"; export type { ListenTarget } from "./status-probes"; import { checkProxyHealth, probeUncleanExitState, type ListenTarget } from "./status-probes"; +/** + * The state of the data-plane admission secret the SERVICE will use. State only -- never the value. + * + * Always about the file, because the file is what the service reads: the launchd plist and the + * systemd unit `cat` it into `OPENCODEX_API_AUTH_TOKEN` before exec, so a token in the CLI's own + * shell says nothing about the running hub. `present (env)` used to be reported here and was + * simply wrong about whose environment it meant (see `dataTokenEnvInShell`). + * + * `admin-collision (file)` is the #4236 incident shape: the file holds the MANAGEMENT token, so + * the server fences the whole management plane closed at boot and the hub crash-loops. It used + * to report `present (file)`, which is how the cause stayed invisible. + */ +export type HubDataTokenState = + | "present (file)" + | "unsafe (file)" + | "admin-collision (file)" + | "missing"; + +export type HubStatus = { + /** Advertised data origin: hub.dataPublicOrigin, else derived from the bind address. */ + dataOrigin: string; + /** True when dataOrigin came from config rather than being derived from the bind. */ + dataOriginConfigured: boolean; + /** + * The unauthenticated loopback listener, in PR2's two forms: `companion` shares the public + * port (the one-port hub), `ported` binds its own. `off` means the hub does not serve its + * own local clients at all. + */ + loopbackListener: { state: "off" | "companion" | "ported"; port: number | null }; + managementIngress: { enabled: boolean; port: number | null }; + managementPublicOrigin: string | null; + dataToken: HubDataTokenState; + dataTokenPath: string; + /** + * `OPENCODEX_API_AUTH_TOKEN` is set in the shell that ran `ocx status` — which is NOT the + * environment the installed service runs in. Reported separately, and honestly, because it + * does decide what a FOREGROUND `ocx start` in this same shell would admit. + */ + dataTokenEnvInShell: boolean; +}; + export type CliStatusJson = { schemaVersion: 1; proxy: { @@ -89,6 +132,18 @@ export type CliStatusJson = { desiredEnabled: boolean; policy: ClaudeDesktopPolicyHealth; }; + /** + * The hub-only facts an operator needs in one place, or null on a standalone/client machine. + * + * Scattered across the report they were unusable: the public data origin came from `listen`, + * the management origin was folded into `dashboard.url`, the loopback companion appeared + * nowhere, and the data-plane token appeared nowhere at all -- so the one question a hub + * operator actually asks ("is this reachable, and can another machine join?") took four other + * commands to answer. Additive and nullable, so `schemaVersion` stays 1. + * + * Never carries a token value; only which source holds one. + */ + hub: HubStatus | null; /** * This CLI's version against the running proxy's (#2701). * @@ -120,6 +175,86 @@ function statusDashboardUrl(config: StatusListenConfig, hostname: string | undef return `http://${dashboardHostname}:${port}/`; } +/** + * The hub block, or null when this machine is not a hub. + * + * The token line is about the FILE, not this shell. `ocx status` used to print `present (env)` + * whenever the calling shell happened to export `OPENCODEX_API_AUTH_TOKEN`, but the service + * wrapper overwrites that variable from the token file before exec — so the label described the + * operator's terminal and not the hub. The shell's variable is reported as its own flag instead. + * + * The token VALUE is never read into the report. `readServiceApiTokenState` returns it; the only + * things derived from it are `kind` and the admin-token comparison, neither of which can carry + * bytes of the secret. + */ +export function collectHubStatus( + config: Pick, + listen: { port: number; hostname?: string | null }, + env: NodeJS.ProcessEnv = process.env, +): HubStatus | null { + if (config.runtimeRole !== "hub") return null; + const listener = config.unauthenticatedLoopbackListener; + const loopbackPort = effectiveLoopbackListenerPort(config, listen.port); + const ingress = config.hub?.managementIngress; + const configuredDataOrigin = config.hub?.dataPublicOrigin; + const host = probeHostname(listen.hostname ?? config.hostname); + const tokenState = ((): HubDataTokenState => { + const state = readServiceApiTokenState(); + if (state.kind === "unsafe") return "unsafe (file)"; + if (state.kind !== "present") return "missing"; + return tokenCollidesWithAdmin(state.token, env) ? "admin-collision (file)" : "present (file)"; + })(); + return { + dataOrigin: configuredDataOrigin + ?? `http://${host === "127.0.0.1" ? "localhost" : host}:${listen.port}`, + dataOriginConfigured: Boolean(configuredDataOrigin), + loopbackListener: loopbackPort === null + ? { state: "off", port: null } + : { state: listener?.enabled && listener.port === undefined ? "companion" : "ported", port: loopbackPort }, + managementIngress: { + enabled: ingress?.enabled === true, + port: ingress?.enabled === true ? ingress.port : null, + }, + managementPublicOrigin: config.hub?.managementPublicOrigin ?? null, + dataToken: tokenState, + dataTokenPath: serviceApiTokenFilePath(), + dataTokenEnvInShell: Boolean(env.OPENCODEX_API_AUTH_TOKEN?.trim()), + }; +} + +/** + * The human rendering of the hub block, owned here rather than in the `ocx status` printer so + * the sentences are testable without spawning the CLI. Indentation is the caller's. + */ +export function hubStatusLines(hub: HubStatus): string[] { + const listener = hub.loopbackListener.state === "off" + ? "off — this hub does not route its own local Codex/Claude clients" + : hub.loopbackListener.state === "companion" + ? `companion on http://127.0.0.1:${hub.loopbackListener.port} — same port as the public listener, no credential needed locally` + : `ported on http://127.0.0.1:${hub.loopbackListener.port} — a second port local clients must be pointed at`; + const tokenLines = [` Data token: ${hub.dataToken}${hub.dataToken === "missing" ? "" : ` at ${hub.dataTokenPath}`}`]; + if (hub.dataToken === "admin-collision (file)") { + // Naming the consequence matters more than naming the state: this is what a crash-looping + // hub looks like from `ocx status`, and nothing else in the report says so (#4236). + tokenLines.push( + " that file holds the MANAGEMENT token, so the hub fences its management API closed at boot —", + " delete it and run 'ocx service repair' to generate a data-plane token", + ); + } + if (hub.dataTokenEnvInShell) { + tokenLines.push(" OPENCODEX_API_AUTH_TOKEN is also set in this shell; the installed service reads the file, not this"); + } + return [ + "Hub:", + ` Data origin: ${hub.dataOrigin}${hub.dataOriginConfigured ? " (hub.dataPublicOrigin)" : " (derived from the bind address)"}`, + ` Loopback listener: ${listener}`, + ` Management ingress: ${hub.managementIngress.enabled ? `http://127.0.0.1:${hub.managementIngress.port}` : "disabled"}`, + ` Management origin: ${hub.managementPublicOrigin ?? "unset — remote pairing and the remote dashboard need hub.managementPublicOrigin"}`, + ...tokenLines, + " Invite a machine: ocx hub invite", + ]; +} + export function selectListenTarget( config: StatusListenConfig, pid: number | null, @@ -357,6 +492,7 @@ export async function collectStatus(): Promise { source: bunRuntime.source, ...(bunRuntime.source === "override" ? { overrideEnv: bunRuntime.overrideEnv } : {}), }, + hub: collectHubStatus(config, listen), codexAutostart: codexAutoStartEnabled(config), startup, defaultProvider: typeof config.defaultProvider === "string" ? config.defaultProvider : null, diff --git a/src/config.ts b/src/config.ts index 3fc48312c8..66162c6a6a 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1037,6 +1037,18 @@ const hubConfigSchema = z.object({ } return origin; }).optional(), + // Same canonical-origin rule as managementPublicOrigin, and deliberately NOT `.catch`ed: + // a mistyped data origin must be rejected at write time, because silently dropping it + // makes `ocx hub invite` print the `http://:` fallback that the operator + // set this field precisely to replace. + dataPublicOrigin: z.string().transform((value, ctx) => { + const origin = canonicalHttpOrigin(value); + if (!origin) { + ctx.addIssue({ code: "custom", message: "must be a canonical http(s) origin without credentials, path, query, or fragment" }); + return z.NEVER; + } + return origin; + }).optional(), // A malformed hand edit disables only the optional ingress. Live writes are rejected by // managementIngressConfigError before this load-time degradation can hide the mistake. managementIngress: z.union([ diff --git a/src/lib/gui-pair-capability.ts b/src/lib/gui-pair-capability.ts index d3a409e595..9acb526258 100644 --- a/src/lib/gui-pair-capability.ts +++ b/src/lib/gui-pair-capability.ts @@ -38,6 +38,33 @@ export function canonicalGuiBrowserOrigin(value: unknown): string | null { } } +/** + * A bare http(s) origin, or null. + * + * Stricter than {@link canonicalGuiBrowserOrigin}: that one also accepts non-HTTP schemes + * (a packaged app's custom scheme can be a browser origin), while this is the rule for an + * origin that will be DIALLED — the hub's management and data origins, and the `serverOrigin` + * a pairing grant is bound to. Credentials, a path, a query or a fragment are all rejected + * rather than silently dropped, because every caller goes on to print or compare the result. + * + * Exported here, beside the browser-origin canonicaliser, because this file is the only + * module the CLI pairing path and the hub command already share. `src/config.ts` and + * `src/server/gui-session.ts` still hold byte-identical private copies; folding those in + * would pull a heavy config import into a server security-boundary file, so it is its own + * change rather than a drive-by in a token-UX PR. + */ +export function canonicalHttpOrigin(value: unknown): string | null { + if (typeof value !== "string") return null; + try { + const parsed = new URL(value); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return null; + if (parsed.username || parsed.password || parsed.pathname !== "/" || parsed.search || parsed.hash) return null; + return parsed.origin; + } catch { + return null; + } +} + function capabilityPayload( nonce: string, method: string, diff --git a/src/lib/service-secrets.ts b/src/lib/service-secrets.ts index c58f0842d6..7dbd58a899 100644 --- a/src/lib/service-secrets.ts +++ b/src/lib/service-secrets.ts @@ -184,6 +184,34 @@ export function loadServiceTokenFromFile(env: Record } } +/** + * The data-plane token a boot should export, or null when the environment already has one + * (or there is nothing to export). + * + * The launchd plist and the systemd unit `cat` the token file into the environment before + * exec'ing the proxy, and WinSW native mode names it through `OCX_API_TOKEN_FILE` — so under + * a service the server has always seen `OPENCODEX_API_AUTH_TOKEN` regardless of the calling + * shell. A FOREGROUND `ocx start` on the same machine had neither, so `assertServerAuthConfig` + * refused to bind a non-loopback hostname that the installed service was serving happily. + * This closes that gap with the same precedence the wrappers use, in one place. + * + * `authRequired` is the caller's admission decision (`isApiAuthRequired`), passed in rather + * than recomputed: this module must not load config, and the installed file is deliberately + * NOT consulted on a loopback bind — on a machine connected to a hub it holds that hub's + * issued client key, which is not this proxy's admission secret. + */ +export function startupDataPlaneToken( + env: Record, + options: { authRequired: boolean }, +): string | null { + if (env.OPENCODEX_API_AUTH_TOKEN?.trim()) return null; + const named = loadServiceTokenFromFile(env); + if (named) return named; + if (!options.authRequired) return null; + const state = readServiceApiTokenState(); + return state.kind === "present" ? state.token : null; +} + /** * Contents of the installed service token file. The launch wrapper always re-exports * this file as OPENCODEX_API_AUTH_TOKEN, so doctor and start must inspect it even diff --git a/src/service.ts b/src/service.ts index 1a678e7438..85c5b3abc5 100644 --- a/src/service.ts +++ b/src/service.ts @@ -25,10 +25,10 @@ import { BUN_RUNTIME_PATH_ENV, BUN_RUNTIME_SOURCE_ENV, durableBunRuntime } from export const SERVICE_MANAGED_ENV = "OCX_SERVICE_MANAGED"; import type { BunRuntimeSource, DurableBunRuntime } from "./lib/bun-runtime"; import { isProcessAlive, stopProxy } from "./lib/process-control"; -import { serviceApiTokenFilePath } from "./lib/service-secrets"; +import { readServiceApiTokenState, serviceApiTokenFilePath } from "./lib/service-secrets"; import { tokenCollidesWithAdmin } from "./lib/admin-secrets"; import { PROXY_ENV_KEYS } from "./lib/proxy-env"; -import { randomUUID } from "node:crypto"; +import { randomBytes, randomUUID } from "node:crypto"; import { ELEVATION_REQUEST_TIMEOUT_MS, OCX_ELEVATED_PROTOCOL_FAILED, @@ -502,41 +502,93 @@ export function serviceRetryCommand( * installing shell. This function is the chokepoint that should refuse it rather than * writing a file that produces a broken service. Comparison is the same helper doctor * uses: minted `ocx_admin_…` prefix, or byte-equal to configuredAdminToken (env or file). + * + * `source` selects the remedy, not the rule. The token can also arrive from an EXISTING + * `service-api-token` that install/repair reuses, and there `unset` is meaningless advice — + * the fix is to delete the file so a data-plane token is generated. */ -export function assertNotAdminToken(token: string, env: NodeJS.ProcessEnv = process.env): void { +export function assertNotAdminToken( + token: string, + env: NodeJS.ProcessEnv = process.env, + source: "env" | "file" = "env", +): void { if (!tokenCollidesWithAdmin(token, env)) return; + if (source === "file") { + // The file branch of `writeServiceApiTokenFile` used to skip this check entirely, so a + // hand-pasted admin token already on disk (pre-#2696, or the exact #4236 incident) was + // silently reused: `ocx status` said `present (file)` and the hub crash-looped at boot. + // The remedy is NOT `unset` -- there is nothing in the environment to unset. + throw new Error( + `${serviceApiTokenFilePath()} holds a management (admin) token, not a data-plane token. ` + + "The service exports that file as the data-plane secret, which fences the whole management " + + "API closed and makes every ocx management command fail with 503, so the hub crash-loops at " + + `boot. Delete the file (rm ${serviceApiTokenFilePath()}), then rerun \`ocx service repair\` ` + + "(or `ocx service install` when the service is not installed yet): a fresh owner-only " + + "data-plane token is generated and nothing needs to be exported by hand.", + ); + } throw new Error( "OPENCODEX_API_AUTH_TOKEN holds a management (admin) token. The service exports it " + "as the data-plane secret, which fences the whole management API closed and makes " - + "every ocx management command fail with 503. Unset OPENCODEX_API_AUTH_TOKEN, or set " - + "it to a distinct data-plane key, then rerun the install.", + + "every ocx management command fail with 503. Run `unset OPENCODEX_API_AUTH_TOKEN` " + + "and rerun: nothing needs to be exported by hand, because the service provisions " + + `its own owner-only data-plane token at ${serviceApiTokenFilePath()}.`, ); } +/** + * Preflight for `service install` / `service repair` on the data-plane credential. + * + * It used to DEMAND `OPENCODEX_API_AUTH_TOKEN` for a non-loopback hostname, and it threw + * even when `~/.opencodex/service-api-token` already held a perfectly good token. That is + * the defect behind the incident this unit exists to close (#4236): an operator exported the + * ADMIN token as OPENCODEX_API_AUTH_TOKEN because `install` asked for a token, the hub then + * crash-looped on `assertNotAdminToken`, and `service repair` asked for the same env var + * again — so the only remembered way to make the command proceed was the thing that broke it. + * + * Nobody should have to export a token by hand to run a hub. {@link writeServiceApiTokenFile} + * provisions one, so the only conditions left that install cannot fix are an admin-token + * collision in the environment and a token file that exists but cannot be used. + */ export function assertServiceAuthEnvironment(): void { const config = loadConfig(); - // Check the collision before the loopback short-circuit: a loopback install writes - // the token file too, so returning early here is what let the broken state through. + // Both collision checks come BEFORE the loopback short-circuit, because the launch wrapper + // exports the token file unconditionally (`buildServiceShellCommand` cats it whenever it + // exists, whatever the hostname): a management token in either source fences the whole + // management plane closed at boot, even on a loopback install that needs no admission + // secret. Returning early is what let that broken state through. const present = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); if (present) assertNotAdminToken(present); + const state = readServiceApiTokenState(); + // An existing FILE holding the admin token is the incident shape itself, and the first round + // only checked the env var — so install/repair reused it and the hub crash-looped at boot. + // On a machine connected to a hub this same file holds that hub's issued client key, which + // is never a management token, so the check is a no-op there. + if (state.kind === "present") assertNotAdminToken(state.token, process.env, "file"); if (isLoopbackHostname(config.hostname)) return; - if (process.env.OPENCODEX_API_AUTH_TOKEN?.trim()) return; - // Reached from `service repair` as well as `install`, so name a command that can - // actually succeed (see serviceRetryCommand). + if (present) return; + // Absent is fine — install/repair generates one below. `unsafe` is not: the writer refuses + // to replace a path it cannot vouch for, so say so here, where the operator can still act, + // instead of failing mid-install. Reached from `service repair` as well as `install`, so + // name a command that can actually succeed (see serviceRetryCommand). + if (state.kind !== "unsafe") return; const diag = diagnoseService(); - const retry = serviceRetryCommand(diag); throw new Error( - `OPENCODEX_API_AUTH_TOKEN is required before ${diag.installed ? "refreshing" : "installing"} a service ` - + `for non-loopback hostname. Set it in the same shell, then rerun \`${retry}\`.`, + `The data-plane token file cannot be used (${state.reason}): ${serviceApiTokenFilePath()}. ` + + `Move it aside, then rerun \`${serviceRetryCommand(diag)}\`; the service provisions a ` + + "fresh owner-only token and needs nothing from the environment.", ); } -function writeServiceApiTokenFile(): string | null { - const token = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); - if (!token) return null; - // Last line of defence: every install/repair path funnels through here, so a - // collision cannot reach disk regardless of which caller ran (#2696). - assertNotAdminToken(token); +/** How the data-plane token the service will export was obtained. */ +export type ServiceApiTokenOrigin = "env" | "file" | "generated"; + +export interface ProvisionedServiceApiToken { + path: string; + origin: ServiceApiTokenOrigin; +} + +function persistServiceApiToken(token: string): string { const path = serviceApiTokenFilePath(); const dir = getConfigDir(); recordOwnedConfigPath(dir, path); @@ -548,6 +600,67 @@ function writeServiceApiTokenFile(): string | null { return path; } +/** + * Put a usable data-plane token on disk for the service to read at launch, and say where. + * + * EVERY backend funnels through here — launchd, systemd, the Windows scheduler wrapper and + * WinSW native — because the launch wrapper's only source of the secret is this file + * (`buildServiceShellCommand` cats it into the environment; WinSW reads it through + * `OCX_API_TOKEN_FILE`). One chokepoint is also what makes the admin-token refusal + * unskippable (#2696). + * + * Precedence, in order: + * 1. `OPENCODEX_API_AUTH_TOKEN` from the installing shell — still refused outright when it is + * an admin token. An operator who deliberately exports a key keeps full control of it. + * 2. An existing owner-only `service-api-token`. Reusing it is what makes `repair`, a + * reinstall and a restart idempotent; regenerating would silently invalidate every client + * key-exchange already performed against the old value. + * 3. 32 fresh random bytes, hex. This is the branch that removes the manual step: a hub + * install on a non-loopback hostname provisions its own secret. + * + * A loopback install with no env token gets nothing: admission is not required there, so + * creating a credential would be inventing a secret nobody asked for — and on a machine + * connected to a hub the same file holds that hub's issued client key, which must not be + * overwritten by a local install. + * + * The PATH is logged; the value never is, and never reaches argv, a unit file or a plist. + */ +export function writeServiceApiTokenFile(): ProvisionedServiceApiToken | null { + const token = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); + if (token) { + // Last line of defence: every install/repair path funnels through here, so a + // collision cannot reach disk regardless of which caller ran (#2696). + assertNotAdminToken(token); + const path = persistServiceApiToken(token); + console.log(`🔐 Data-plane token taken from OPENCODEX_API_AUTH_TOKEN and stored at ${path} (owner-only).`); + return { path, origin: "env" }; + } + if (isLoopbackHostname(loadConfig().hostname)) return null; + const existing = readServiceApiTokenState(); + if (existing.kind === "present") { + // The collision check is NOT only for the env branch. A file that already holds the admin + // token -- hand-pasted before #2696, or written by the very incident this unit closes -- + // was silently accepted here, so `ocx status` reported `present (file)` and the hub + // crash-looped at boot with no command pointing at the cause. + const path = serviceApiTokenFilePath(); + assertNotAdminToken(existing.token, process.env, "file"); + // `readServiceApiTokenState` accepts any bounded regular file, so a reused token may well + // be group- or world-readable. Tighten it on the way through rather than claiming + // "owner-only" about a mode nobody checked; best-effort, since a non-owner cannot chmod + // and failing the install over it would be worse than the loose mode. + try { chmodSync(path, 0o600); } catch { /* best-effort */ } + if (process.platform === "win32") hardenSecretPath(path, { required: false }); + // No log line: repair/restart hit this on every run and an unconditional notice about a + // credential file trains operators to ignore the one that matters. + return { path, origin: "file" }; + } + if (existing.kind === "unsafe") throw new Error(`${existing.reason}: ${serviceApiTokenFilePath()}`); + const path = persistServiceApiToken(randomBytes(32).toString("hex")); + console.log(`🔐 Provisioned an owner-only data-plane token at ${path}; nothing needs to be exported by hand.`); + console.log(" Remote machines get their own per-client key — run 'ocx hub invite' instead of copying this file."); + return { path, origin: "generated" }; +} + /** * Render the launchd plist. Mirrors `buildUnit`: when `deps.launcher` names a stable `ocx` * executable, the job execs that launcher instead of the package-local Bun + CLI pair, so a diff --git a/src/types/config.ts b/src/types/config.ts index fdaec36ff8..499ca59eb1 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -259,6 +259,21 @@ export type OcxRuntimeRole = "standalone" | "hub" | "client"; export interface OcxHubConfig { /** Canonical browser-reachable management origin advertised by a hub. */ managementPublicOrigin?: string; + /** + * Canonical client-reachable DATA origin of this hub — what a remote machine passes as the + * positional URL to `ocx connect`, and what `ocx hub invite` prints. + * + * Separate from `managementPublicOrigin` because the two are genuinely different sockets on a + * real deployment: management is a loopback-only ingress published by an HTTPS frontend, while + * the data listener is bound to the hub's tailnet/LAN address and fronted on its own port + * (`https://hub.tailnet.ts.net:8443`). Deriving one from the other produced an origin that + * answered `/readyz` and nothing else. + * + * Advisory only: it is the origin the hub ADVERTISES, never a bind address. When omitted, + * `ocx hub invite` falls back to `http://:`, which is correct for a plain + * tailnet bind with no TLS frontend. + */ + dataPublicOrigin?: string; /** * Optional management-only listener for a local HTTPS frontend such as Tailscale Serve. * The hostname is deliberately not configurable: when enabled the socket is always bound diff --git a/tests/cli/cli-status-json.test.ts b/tests/cli/cli-status-json.test.ts index 24191ba887..2aae43a671 100644 --- a/tests/cli/cli-status-json.test.ts +++ b/tests/cli/cli-status-json.test.ts @@ -7,7 +7,7 @@ import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { isConnectionRefused, isUncleanExitEvidence, proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../../src/cli/status"; +import { collectHubStatus, hubStatusLines, isConnectionRefused, isUncleanExitEvidence, proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../../src/cli/status"; import * as statusFacade from "../../src/cli/status"; import * as statusProbes from "../../src/cli/status-probes"; import { packageVersion } from "../../src/cli/help"; @@ -604,6 +604,162 @@ describe("CLI status JSON", () => { * it. These cases pin the predicate, including the two false-positive shapes that a * naive implementation gets wrong. */ +/** + * The hub block (#4236). + * + * A hub operator's first question is "is this reachable, and can another machine join?", and the + * report used to answer it in four places and not at all for the data token. These tests pin the + * projection and the sentences, and -- the one that matters for a security boundary -- that no + * token VALUE is ever in either. + */ +describe("status hub block", () => { + const TOKEN = "b".repeat(64); + + function withHome(setup: (home: string) => void, body: () => T): T { + const home = mkdtempSync(join(tmpdir(), "ocx-status-hub-")); + const previous = process.env.OPENCODEX_HOME; + const previousToken = process.env.OPENCODEX_API_AUTH_TOKEN; + process.env.OPENCODEX_HOME = home; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + try { + mkdirSync(join(home), { recursive: true }); + setup(home); + return body(); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + if (previousToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousToken; + removeTreeWithRetry(home); + } + } + + const hub = (overrides: Record = {}) => ({ + port: 10100, + hostname: "100.64.0.10", + runtimeRole: "hub" as const, + hub: { managementPublicOrigin: "https://hub.tailnet.ts.net", managementIngress: { enabled: true as const, port: 10101 } }, + unauthenticatedLoopbackListener: { enabled: true as const }, + ...overrides, + }); + + test("there is no hub block on a standalone or client machine", () => { + for (const role of [undefined, "standalone", "client"] as const) { + const config = { ...hub(), runtimeRole: role } as Parameters[0]; + expect(collectHubStatus(config, { port: 10100, hostname: "127.0.0.1" }, {})).toBeNull(); + } + }); + + test("the companion form is named as sharing the public port; a ported one is not", () => { + const companion = collectHubStatus(hub() as Parameters[0], { port: 10100, hostname: "100.64.0.10" }, {}); + expect(companion?.loopbackListener).toEqual({ state: "companion", port: 10100 }); + expect(hubStatusLines(companion!).join("\n")).toContain("same port as the public listener"); + + const ported = collectHubStatus( + hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10104 } }) as Parameters[0], + { port: 10100, hostname: "100.64.0.10" }, + {}, + ); + expect(ported?.loopbackListener).toEqual({ state: "ported", port: 10104 }); + expect(hubStatusLines(ported!).join("\n")).toContain("http://127.0.0.1:10104"); + + const off = collectHubStatus( + hub({ unauthenticatedLoopbackListener: { enabled: false } }) as Parameters[0], + { port: 10100, hostname: "100.64.0.10" }, + {}, + ); + expect(off?.loopbackListener).toEqual({ state: "off", port: null }); + expect(hubStatusLines(off!).join("\n")).toContain("does not route its own local"); + }); + + test("the data origin prefers hub.dataPublicOrigin and says which it used", () => { + const derived = collectHubStatus(hub() as Parameters[0], { port: 10100, hostname: "100.64.0.10" }, {}); + expect(derived?.dataOrigin).toBe("http://100.64.0.10:10100"); + expect(derived?.dataOriginConfigured).toBe(false); + expect(hubStatusLines(derived!).join("\n")).toContain("derived from the bind address"); + + const configured = collectHubStatus( + hub({ hub: { managementPublicOrigin: "https://hub.tailnet.ts.net", dataPublicOrigin: "https://hub.tailnet.ts.net:8443" } }) as Parameters[0], + { port: 10100, hostname: "100.64.0.10" }, + {}, + ); + expect(configured?.dataOrigin).toBe("https://hub.tailnet.ts.net:8443"); + expect(hubStatusLines(configured!).join("\n")).toContain("hub.dataPublicOrigin"); + }); + + test("the token state is about the file the service reads, never about this shell", () => { + withHome(home => writeFileSync(join(home, "service-api-token"), `${TOKEN}\n`, "utf8"), () => { + const fromFile = collectHubStatus(hub() as Parameters[0], { port: 10100 }, {}); + expect(fromFile?.dataToken).toBe("present (file)"); + expect(fromFile?.dataTokenEnvInShell).toBe(false); + + // `present (env)` used to be reported here whenever the CALLING shell exported the + // variable -- but the launchd plist and the systemd unit overwrite it from the file + // before exec, so the label described the operator's terminal, not the hub. + const withShellVar = collectHubStatus( + hub() as Parameters[0], + { port: 10100 }, + { OPENCODEX_API_AUTH_TOKEN: "from-the-shell" }, + ); + expect(withShellVar?.dataToken).toBe("present (file)"); + expect(withShellVar?.dataTokenEnvInShell).toBe(true); + expect(hubStatusLines(withShellVar!).join("\n")).toContain("the installed service reads the file, not this"); + + for (const status of [fromFile!, withShellVar!]) { + const rendered = [JSON.stringify(status), ...hubStatusLines(status)].join("\n"); + expect(rendered).not.toContain(TOKEN); + expect(rendered).not.toContain("from-the-shell"); + } + }); + }); + + test("a token file holding the ADMIN token is called out, not reported as present", () => { + // The #4236 incident read `present (file)` while the hub crash-looped, because the file + // held the MANAGEMENT token and nothing in the report compared the two. + const admin = `ocx_admin_${"f".repeat(43)}`; + withHome(home => writeFileSync(join(home, "service-api-token"), `${admin}\n`, "utf8"), () => { + const status = collectHubStatus(hub() as Parameters[0], { port: 10100 }, {}); + expect(status?.dataToken).toBe("admin-collision (file)"); + const lines = hubStatusLines(status!).join("\n"); + expect(lines).toContain(status!.dataTokenPath); + expect(lines).toContain("MANAGEMENT token"); + expect(lines).toContain("ocx service repair"); + expect([JSON.stringify(status), lines].join("\n")).not.toContain(admin); + }); + // The same comparison doctor and the service chokepoint use: byte-equal to the configured + // admin token counts too, not only the minted prefix. + withHome(home => { + writeFileSync(join(home, "service-api-token"), "hand-pasted-management-key\n", "utf8"); + }, () => { + const status = collectHubStatus( + hub() as Parameters[0], + { port: 10100 }, + { OPENCODEX_ADMIN_AUTH_TOKEN: "hand-pasted-management-key" }, + ); + expect(status?.dataToken).toBe("admin-collision (file)"); + }); + }); + + test("a missing and an unusable token file are distinguished", () => { + withHome(() => {}, () => { + expect(collectHubStatus(hub() as Parameters[0], { port: 10100 }, {})?.dataToken).toBe("missing"); + }); + withHome(home => writeFileSync(join(home, "service-api-token"), "\n", "utf8"), () => { + const status = collectHubStatus(hub() as Parameters[0], { port: 10100 }, {}); + expect(status?.dataToken).toBe("unsafe (file)"); + expect(hubStatusLines(status!).join("\n")).toContain(status!.dataTokenPath); + }); + }); + + test("the block always ends with the invite hint", () => { + withHome(() => {}, () => { + const lines = hubStatusLines(collectHubStatus(hub() as Parameters[0], { port: 10100 }, {})!); + expect(lines[0]).toBe("Hub:"); + expect(lines.at(-1)).toBe(" Invite a machine: ocx hub invite"); + }); + }); +}); + describe("unclean prior exit evidence", () => { const base = { live: false, diff --git a/tests/cli/hub-invite.test.ts b/tests/cli/hub-invite.test.ts new file mode 100644 index 0000000000..d5509459bd --- /dev/null +++ b/tests/cli/hub-invite.test.ts @@ -0,0 +1,350 @@ +import { describe, expect, spyOn, test } from "bun:test"; +import { + appendCorsAllowOriginsCommand, + configSetHubLines, + derivedHubDataOrigin, + hubInviteCommand, + inviteBoundOriginNotes, + pairingOriginUsable, + parseHubInviteArgs, + parseInviteClients, + resolveHubDataOrigin, + runHubCommand, + selectInviteBrowserOrigin, +} from "../../src/cli/hub"; +import type { GuiPairRequestResult } from "../../src/cli/gui-pair-client"; +import type { LiveProxy } from "../../src/server/proxy-liveness"; +import type { OcxConfig } from "../../src/types"; + +/** + * `ocx hub invite` (#4236). + * + * The command's whole value is that the line it prints can be pasted on the other machine and + * work, so these tests pin the two things that decide that: WHICH origins end up in the command, + * and WHICH configurations are refused before a single-use code is burned on a request the hub + * would have rejected anyway. + * + * The mint itself is not re-tested here -- it is the existing attested `ocx gui pair` route, and + * `requestPairingGrant` is injected so no proxy, no socket and no real grant is involved. + */ +const GRANT = `ocx_pair_${"A".repeat(43)}`; +const EXPIRES_AT = 1_767_225_600_000; + +const LIVE: LiveProxy = { pid: 4242, port: 10100, hostname: "100.64.0.10", source: "runtime" }; + +function hubConfig(overrides: Partial = {}): OcxConfig { + return { + port: 10100, + hostname: "100.64.0.10", + runtimeRole: "hub", + corsAllowOrigins: ["http://localhost:10100"], + hub: { managementPublicOrigin: "https://hub.tailnet.ts.net" }, + ...overrides, + } as OcxConfig; +} + +async function invite( + args: string[], + config: OcxConfig, + options: { live?: LiveProxy | null; result?: GuiPairRequestResult } = {}, +): Promise<{ code: number; out: string[]; err: string[]; boundOrigin: string | null }> { + const out: string[] = []; + const err: string[] = []; + const log = spyOn(console, "log").mockImplementation((...parts: unknown[]) => { + out.push(parts.map(String).join(" ")); + }); + const error = spyOn(console, "error").mockImplementation((...parts: unknown[]) => { + err.push(parts.map(String).join(" ")); + }); + let boundOrigin: string | null = null; + try { + const code = await runHubCommand(args, { + loadConfig: () => config, + findLiveProxy: async () => (options.live === undefined ? LIVE : options.live), + requestPairingGrant: async (_target, browserOrigin) => { + boundOrigin = browserOrigin; + return options.result ?? { + kind: "created", + grant: GRANT, + browserOrigin, + serverOrigin: "https://hub.tailnet.ts.net", + expiresAt: EXPIRES_AT, + }; + }, + }); + return { code, out, err, boundOrigin }; + } finally { + log.mockRestore(); + error.mockRestore(); + } +} + +describe("hub invite argument and origin helpers", () => { + test("parses the documented flags and rejects anything else", () => { + expect(parseHubInviteArgs([])).toEqual({ json: false }); + expect(parseHubInviteArgs(["--json"])).toEqual({ json: true }); + expect(parseHubInviteArgs(["--data-url", "https://a.test:8443", "--clients", "codex"])) + .toEqual({ json: false, dataUrl: "https://a.test:8443", clients: "codex" }); + // A repeated flag, a missing value, and an unknown token are all usage errors rather than + // a silently-dropped argument: the printed command is what the operator will run. + expect(parseHubInviteArgs(["--json", "--json"])).toBeNull(); + expect(parseHubInviteArgs(["--data-url"])).toBeNull(); + expect(parseHubInviteArgs(["--data-url", "--json"])).toBeNull(); + expect(parseHubInviteArgs(["--origin", "x"])).toBeNull(); + }); + + test("clients accepts codex and claude only, and omission means 'do not pass --clients'", () => { + expect(parseInviteClients(undefined)).toEqual([]); + expect(parseInviteClients("codex,claude")).toEqual(["codex", "claude"]); + expect(parseInviteClients("claude")).toEqual(["claude"]); + expect(parseInviteClients("cursor")).toBeNull(); + expect(parseInviteClients("")).toBeNull(); + }); + + test("the transport rule matches the hub's: loopback or HTTPS, nothing else", () => { + expect(pairingOriginUsable("https://hub.tailnet.ts.net")).toBe(true); + expect(pairingOriginUsable("http://127.0.0.1:10101")).toBe(true); + expect(pairingOriginUsable("http://localhost:10101")).toBe(true); + expect(pairingOriginUsable("http://100.64.0.10:10101")).toBe(false); + }); + + test("the derived data origin is the bind address, with loopback spelled as localhost", () => { + expect(derivedHubDataOrigin("100.64.0.10", 10100)).toBe("http://100.64.0.10:10100"); + expect(derivedHubDataOrigin("0.0.0.0", 10100)).toBe("http://localhost:10100"); + expect(derivedHubDataOrigin("fd7a::1", 8443)).toBe("http://[fd7a::1]:8443"); + }); + + test("the grant binds to the connecting machine's loopback origin, not the hub's", () => { + // `ocx connect` sends Origin: http://localhost: (client/connect.ts + // localGuiOrigin), so only a loopback entry in the hub's allow-list can ever match. + expect(selectInviteBrowserOrigin(hubConfig())).toBe("http://localhost:10100"); + expect(selectInviteBrowserOrigin(hubConfig({ corsAllowOrigins: ["http://localhost:9999"] }))) + .toBe("http://localhost:9999"); + expect(selectInviteBrowserOrigin(hubConfig({ corsAllowOrigins: ["https://elsewhere.test"] }))) + .toBeNull(); + }); + + test("the data origin refuses a loopback bind and a wildcard bind, and takes an explicit one", () => { + // `probeHostname` spells 0.0.0.0, ::, and 127.0.0.1 all as loopback, so the derived origin + // would tell the other machine to dial ITSELF -- and the code is single-use. + for (const bind of ["127.0.0.1", "localhost", "0.0.0.0", "::", undefined] as const) { + expect(resolveHubDataOrigin(null, undefined, bind, 10100)).toEqual({ + kind: "loopback-derived", + dataUrl: "http://localhost:10100", + bindHostname: bind ?? "127.0.0.1", + }); + } + expect(resolveHubDataOrigin(null, undefined, "100.64.0.10", 10100)) + .toEqual({ kind: "usable", dataUrl: "http://100.64.0.10:10100", source: "derived" }); + // An explicit origin is never second-guessed: loopback is legitimate over an SSH tunnel. + expect(resolveHubDataOrigin("http://localhost:9000", undefined, "127.0.0.1", 10100)) + .toEqual({ kind: "usable", dataUrl: "http://localhost:9000", source: "flag" }); + expect(resolveHubDataOrigin(null, "https://hub.tailnet.ts.net:8443", "127.0.0.1", 10100)) + .toEqual({ kind: "usable", dataUrl: "https://hub.tailnet.ts.net:8443", source: "config" }); + // A malformed persisted value must not silently become the loopback fallback. + expect(resolveHubDataOrigin(null, "not-an-origin", "100.64.0.10", 10100)) + .toEqual({ kind: "usable", dataUrl: "http://100.64.0.10:10100", source: "derived" }); + }); + + test("the surfaced config commands work on a config that has no hub object yet", () => { + // `ocx config set hub.x` exits with `config parent path not found: hub` when `hub` is + // absent (setPath in config-command.ts walks existing parents only). + expect(configSetHubLines({ hub: undefined } as Partial, "dataPublicOrigin", "https://d.test")) + .toEqual(["ocx config set hub '{}'", `ocx config set hub.dataPublicOrigin '"https://d.test"'`]); + expect(configSetHubLines({ hub: {} } as Partial, "managementPublicOrigin", "https://m.test")) + .toEqual([`ocx config set hub.managementPublicOrigin '"https://m.test"'`]); + }); + + test("the corsAllowOrigins command adds to the list instead of replacing it", () => { + // `ocx config set corsAllowOrigins '[...]'` overwrites, so a one-element literal would + // tell an operator with an existing allow-list to delete it. + expect(appendCorsAllowOriginsCommand({ corsAllowOrigins: ["https://a.test"] }, "http://localhost:10100")) + .toBe(`ocx config set corsAllowOrigins '["https://a.test","http://localhost:10100"]'`); + expect(appendCorsAllowOriginsCommand({}, "http://localhost:10100")) + .toBe(`ocx config set corsAllowOrigins '["http://localhost:10100"]'`); + // Idempotent: never suggest a duplicate entry. + expect(appendCorsAllowOriginsCommand({ corsAllowOrigins: ["http://localhost:10100"] }, "http://localhost:10100")) + .toBe(`ocx config set corsAllowOrigins '["http://localhost:10100"]'`); + }); + + test("the bound browser origin is always stated, and a non-default one names the client's port", () => { + expect(inviteBoundOriginNotes("http://localhost:10100", {})).toEqual([ + "Bound browser origin: http://localhost:10100 — the connecting machine must present exactly this.", + ]); + const notes = inviteBoundOriginNotes("http://localhost:9999", { corsAllowOrigins: ["http://localhost:9999"] }); + expect(notes[0]).toContain("http://localhost:9999"); + expect(notes.join(" ")).toContain("ocx config set port 9999"); + expect(notes.join(" ")).toContain(`'["http://localhost:9999","http://localhost:10100"]'`); + }); + + test("the printed command carries --clients only when the operator asked for it", () => { + expect(hubInviteCommand(GRANT, "https://d.test:8443", "https://m.test", [])) + .toBe(`echo '${GRANT}' | ocx connect https://d.test:8443 --management-url https://m.test --pairing-code-stdin`); + expect(hubInviteCommand(GRANT, "https://d.test:8443", "https://m.test", ["codex"])) + .toContain("--clients codex --pairing-code-stdin"); + }); +}); + +describe("hub invite output", () => { + test("prints a runnable connect line and keeps the code off stderr", async () => { + const { code, out, err, boundOrigin } = await invite(["invite"], hubConfig()); + expect(code).toBe(0); + expect(boundOrigin).toBe("http://localhost:10100"); + expect(out.join("\n")).toContain("# Run on the other machine:"); + expect(out.join("\n")).toContain( + `echo '${GRANT}' | ocx connect http://100.64.0.10:10100 --management-url https://hub.tailnet.ts.net --pairing-code-stdin`, + ); + // The warning is advice, not output a script should capture. + expect(err.join("\n")).toContain("single-use"); + expect(err.join("\n")).not.toContain(GRANT); + }); + + test("the bound browser origin reaches stderr in both modes, with a warning when it differs", async () => { + const plain = await invite(["invite"], hubConfig()); + expect(plain.err.join("\n")).toContain("Bound browser origin: http://localhost:10100"); + expect(plain.err.join("\n")).not.toContain("ocx config set port"); + + const asJson = await invite(["invite", "--json"], hubConfig()); + expect(asJson.err.join("\n")).toContain("Bound browser origin: http://localhost:10100"); + + // `selectInviteBrowserOrigin` silently fell back to the first admitted loopback origin, and + // a remote `ocx connect` only sends http://localhost: -- so without this the + // single-use code was spent with nothing saying why. + const other = await invite(["invite"], hubConfig({ corsAllowOrigins: ["http://localhost:9999"] })); + expect(other.code).toBe(0); + expect(other.boundOrigin).toBe("http://localhost:9999"); + expect(other.err.join("\n")).toContain("Bound browser origin: http://localhost:9999"); + expect(other.err.join("\n")).toContain("ocx config set port 9999"); + }); + + test("hub.dataPublicOrigin replaces the derived origin, and --data-url replaces both", async () => { + const configured = hubConfig({ + hub: { managementPublicOrigin: "https://hub.tailnet.ts.net", dataPublicOrigin: "https://hub.tailnet.ts.net:8443" }, + }); + const fromConfig = await invite(["invite"], configured); + expect(fromConfig.out.join("\n")).toContain("ocx connect https://hub.tailnet.ts.net:8443 "); + + const overridden = await invite(["invite", "--data-url", "https://front.test"], configured); + expect(overridden.out.join("\n")).toContain("ocx connect https://front.test "); + }); + + test("--json emits exactly the documented envelope", async () => { + const { code, out } = await invite(["invite", "--json", "--clients", "codex,claude"], hubConfig()); + expect(code).toBe(0); + expect(JSON.parse(out[0]!)).toEqual({ + code: GRANT, + expiresAt: new Date(EXPIRES_AT).toISOString(), + dataUrl: "http://100.64.0.10:10100", + managementUrl: "https://hub.tailnet.ts.net", + command: `echo '${GRANT}' | ocx connect http://100.64.0.10:10100 --management-url https://hub.tailnet.ts.net --clients codex,claude --pairing-code-stdin`, + }); + }); +}); + +describe("hub invite refuses before burning a code", () => { + test("a non-hub gets one line naming its own role and the command it should run", async () => { + for (const role of [undefined, "standalone", "client"] as const) { + const { code, err } = await invite(["invite"], hubConfig({ runtimeRole: role } as Partial)); + expect(code).toBe(1); + expect(err.join(" ")).toContain("runs on a hub"); + expect(err.join(" ")).toContain("ocx connect"); + } + }); + + test("a hub with no management origin is told which field to set, in a runnable form", async () => { + const { code, err } = await invite(["invite"], hubConfig({ hub: {} })); + expect(code).toBe(1); + expect(err.join(" ")).toContain("hub.managementPublicOrigin"); + expect(err.join(" ")).not.toContain("ocx config set hub '{}'"); + + // With no `hub` object at all the dotted form exits `config parent path not found: hub`, + // so the parent-creating line has to come with it. + const absent = await invite(["invite"], hubConfig({ hub: undefined })); + expect(absent.code).toBe(1); + expect(absent.err.join("\n")).toContain("ocx config set hub '{}'"); + expect(absent.err.join("\n")).toContain("ocx config set hub.managementPublicOrigin"); + }); + + test("a --management-url that differs from the configured origin is refused, not printed", async () => { + // The grant's server origin IS hub.managementPublicOrigin, so advertising anything else + // hands out a code the hub then refuses. Saying so beats printing a dud command. + const { code, err } = await invite(["invite", "--management-url", "https://other.test"], hubConfig()); + expect(code).toBe(1); + expect(err.join(" ")).toContain("does not match hub.managementPublicOrigin"); + + const matching = await invite(["invite", "--management-url", "https://hub.tailnet.ts.net"], hubConfig()); + expect(matching.code).toBe(0); + }); + + test("a non-loopback plaintext management origin cannot carry a code", async () => { + const { code, err } = await invite(["invite"], hubConfig({ + hub: { managementPublicOrigin: "http://100.64.0.10:10101" }, + })); + expect(code).toBe(1); + expect(err.join(" ")).toContain("plain HTTP"); + }); + + test("a hub whose allow-list names no loopback origin is told exactly what to add", async () => { + const { code, err } = await invite(["invite"], hubConfig({ corsAllowOrigins: [] })); + expect(code).toBe(1); + expect(err.join(" ")).toContain("corsAllowOrigins"); + expect(err.join(" ")).toContain("http://localhost:10100"); + }); + + test("a loopback-derived data origin is refused before a code is minted", async () => { + // The incident shape: a hub bound to loopback (or a wildcard) with no hub.dataPublicOrigin + // printed `ocx connect http://localhost:10100`, which on the other machine means "dial + // yourself" -- and the code was gone. + for (const bind of ["127.0.0.1", "0.0.0.0"] as const) { + const { code, err, boundOrigin } = await invite( + ["invite"], + hubConfig({ hostname: bind }), + { live: { pid: 4242, port: 10100, hostname: bind, source: "runtime" } }, + ); + expect(code).toBe(1); + expect(boundOrigin).toBeNull(); // nothing was minted + expect(err.join(" ")).toContain("http://localhost:10100"); + expect(err.join(" ")).toContain("hub.dataPublicOrigin"); + expect(err.join(" ")).toContain("--data-url"); + } + expect((await invite(["invite"], hubConfig({ hostname: "0.0.0.0" }), { + live: { pid: 4242, port: 10100, hostname: "0.0.0.0", source: "runtime" }, + })).err.join(" ")).toContain("wildcard"); + + // Either explicit origin unblocks it. + const viaFlag = await invite(["invite", "--data-url", "https://hub.tailnet.ts.net:8443"], hubConfig({ hostname: "127.0.0.1" }), { + live: { pid: 4242, port: 10100, hostname: "127.0.0.1", source: "runtime" }, + }); + expect(viaFlag.code).toBe(0); + const viaConfig = await invite(["invite"], hubConfig({ + hostname: "127.0.0.1", + hub: { managementPublicOrigin: "https://hub.tailnet.ts.net", dataPublicOrigin: "https://hub.tailnet.ts.net:8443" }, + }), { live: { pid: 4242, port: 10100, hostname: "127.0.0.1", source: "runtime" } }); + expect(viaConfig.code).toBe(0); + expect(viaConfig.out.join("\n")).toContain("ocx connect https://hub.tailnet.ts.net:8443 "); + }); + + test("no running hub, a malformed origin, and a refused mint each exit 1 with a reason", async () => { + const down = await invite(["invite"], hubConfig(), { live: null }); + expect(down.code).toBe(1); + expect(down.err.join(" ")).toContain("No running attested OpenCodex hub"); + + const bad = await invite(["invite", "--data-url", "https://front.test/path"], hubConfig()); + expect(bad.code).toBe(1); + expect(bad.err.join(" ")).toContain("--data-url must be a bare http(s) origin"); + + const refused = await invite(["invite"], hubConfig(), { + result: { kind: "unavailable", reason: "attestation" }, + }); + expect(refused.code).toBe(1); + expect(refused.err.join(" ")).toContain("(attestation)"); + }); + + test("an unknown subcommand prints usage rather than guessing invite", async () => { + for (const args of [[], ["status"], ["invite-machine"]]) { + const { code, err } = await invite(args, hubConfig()); + expect(code).toBe(1); + expect(err.join(" ")).toContain("ocx hub invite"); + } + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 94dc99f8a1..034fb57f71 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -539,6 +539,7 @@ "history-ocx-compaction-recovery.test.ts": "codex-integration", "hyperbolic-provider.test.ts": "providers", "hub-gated-local-clients.test.ts": "cli", + "hub-invite.test.ts": "cli", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", diff --git a/tests/server/config.test.ts b/tests/server/config.test.ts index 1b1699e0b8..33643fb495 100644 --- a/tests/server/config.test.ts +++ b/tests/server/config.test.ts @@ -435,6 +435,35 @@ describe("opencodex config defaults", () => { }).ok).toBe(true); }); + test("hub.dataPublicOrigin normalizes like the management origin and rejects the same shapes", () => { + // The advertised DATA origin is a separate socket from management on a real deployment + // (tailnet bind behind its own TLS port), so it is its own field rather than a derivation. + expect(validateConfigCandidate({ + ...getDefaultConfig(), + runtimeRole: "hub", + hub: { + managementPublicOrigin: "https://hub.example.test", + dataPublicOrigin: "https://hub.example.test:8443", + }, + })).toMatchObject({ + ok: true, + config: { hub: { dataPublicOrigin: "https://hub.example.test:8443" } }, + }); + // NOT `.catch`ed: silently dropping a typo would make `ocx hub invite` fall back to + // http://:, which is the value the operator set the field to replace. + for (const dataPublicOrigin of [ + "ftp://hub.example.test", + "https://user@hub.example.test", + "https://hub.example.test:8443/path", + "https://hub.example.test:8443/?query=1", + "https://hub.example.test:8443/#fragment", + ]) { + const result = validateConfigCandidate({ ...getDefaultConfig(), hub: { dataPublicOrigin } }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toContain("hub.dataPublicOrigin"); + } + }); + test("remote GUI live candidates reject unsafe origins and malformed identity allowlists", () => { for (const managementPublicOrigin of [ "ftp://hub.example.test", diff --git a/tests/service/service-secrets.test.ts b/tests/service/service-secrets.test.ts index 029cd6d7f2..ef43c39aca 100644 --- a/tests/service/service-secrets.test.ts +++ b/tests/service/service-secrets.test.ts @@ -21,6 +21,7 @@ import { removeServiceApiTokenFileIfOwned, serviceApiTokenFilePath, serviceApiTokenFingerprint, + startupDataPlaneToken, writeServiceApiTokenFile, writeTokenBackup, } from "../../src/lib/service-secrets"; @@ -38,6 +39,42 @@ afterEach(() => { if (home) removeTreeWithRetry(home); }); +/** + * #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 + * neither, so `assertServerAuthConfig` refused to bind a non-loopback hostname that the installed + * service on the same machine was serving happily. These pin the precedence that closes that. + */ +describe("startup data-plane token resolution", () => { + const TOKEN = "a".repeat(64); + + test("the environment wins, and nothing is re-exported when it already holds a token", () => { + writeServiceApiTokenFile(TOKEN); + expect(startupDataPlaneToken({ OPENCODEX_API_AUTH_TOKEN: "already" }, { authRequired: true })).toBeNull(); + }); + + test("OCX_API_TOKEN_FILE still wins over the installed path (WinSW native mode)", () => { + writeServiceApiTokenFile(TOKEN); + const named = join(home, "named-token"); + writeFileSync(named, "from-the-named-file\n", "utf8"); + expect(startupDataPlaneToken({ OCX_API_TOKEN_FILE: named }, { authRequired: true })).toBe("from-the-named-file"); + }); + + test("the installed token is used when admission is required, and ignored when it is not", () => { + writeServiceApiTokenFile(TOKEN); + expect(startupDataPlaneToken({}, { authRequired: true })).toBe(TOKEN); + // A loopback bind needs no credential, and on a machine connected to a hub this same file + // holds that hub's issued CLIENT key -- which is not this proxy's admission secret. + expect(startupDataPlaneToken({}, { authRequired: false })).toBeNull(); + }); + + test("an absent or unusable token file resolves to null rather than throwing at boot", () => { + expect(startupDataPlaneToken({}, { authRequired: true })).toBeNull(); + writeFileSync(serviceApiTokenFilePath(), "\n", "utf8"); + expect(startupDataPlaneToken({}, { authRequired: true })).toBeNull(); + }); +}); + describe("service API token ownership", () => { test("writes only the exact owner path through an atomic owner-only replacement", () => { const token = "ocx_data_0123456789abcdef0123456789abcdef01234567"; diff --git a/tests/service/service.test.ts b/tests/service/service.test.ts index f15a243e4d..677866fa49 100644 --- a/tests/service/service.test.ts +++ b/tests/service/service.test.ts @@ -1,5 +1,5 @@ import { afterAll, afterEach, describe, expect, spyOn, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { tmpdir } from "node:os"; import { delimiter, isAbsolute, join, posix, win32 } from "node:path"; @@ -9,7 +9,7 @@ import { saveConfig } from "../../src/config"; import { windowsEnvIndirectBatchValue } from "../../src/lib/win-paths"; import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml as buildWindowsTaskXmlProduction, buildWindowsTaskXmlDocument, deriveWindowsServiceDiagnostic, deriveWindowsServiceDiagnosticForCurrentUser, expectedLaunchdCommand, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceArgs, parseServiceInstallState, planServiceCommand, prepareServiceInstall, probeServiceInstallation, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, removeNativeWindowsServiceForScheduler, repairService, reportServiceServing, resolveServiceListenPort, runLaunchctl, selectServiceSubcommand, SERVICE_INSTALL_HEALTH_MS, SERVICE_INSTALL_HEALTH_WINDOWS_MS, serviceInstallHealthMs, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, stableLauncherEntry, systemdNeedsDaemonReload, systemdServiceInstallCleanupOps, uninstallSystemd, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy as windowsTaskRegistrationHealthyProduction } from "../../src/service"; import type { ServiceDiagnostic } from "../../src/service"; -import { definitionCarriesCredential, resolvedProxyEnv, writeServiceDefinitionFile } from "../../src/service"; +import { definitionCarriesCredential, resolvedProxyEnv, writeServiceApiTokenFile, writeServiceDefinitionFile } from "../../src/service"; import { buildWinswXml } from "../../src/lib/winsw"; import { CONFIG_OWNER_FILE, CONFIG_UNINSTALL_MANIFEST, recordOwnedConfigPath, removeOwnedConfigState } from "../../src/lib/config-ownership"; import { serviceApiTokenFilePath } from "../../src/lib/service-secrets"; @@ -490,7 +490,84 @@ describe("systemd service unit", () => { }); describe("service install auth preflight", () => { - test("rejects non-loopback service install without a persisted API token", () => { + /** + * #4236. The preflight used to DEMAND OPENCODEX_API_AUTH_TOKEN here, which is what taught an + * operator to export the ADMIN token to make `install` proceed -- and then `repair` demanded it + * again. Nobody should have to export a token by hand to run a hub, so install provisions one. + */ + test("a non-loopback install no longer demands the env token", () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + + expect(() => assertServiceAuthEnvironment()).not.toThrow(); + }); + + test("provisioning generates an owner-only token, then reuses it on repair and reinstall", () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + + const first = writeServiceApiTokenFile(); + expect(first).toEqual({ path: serviceApiTokenFilePath(), origin: "generated" }); + const token = readFileSync(serviceApiTokenFilePath(), "utf8").trim(); + // 32 random bytes, hex. + expect(token).toMatch(/^[0-9a-f]{64}$/); + if (process.platform !== "win32") { + expect(statSync(serviceApiTokenFilePath()).mode & 0o777).toBe(0o600); + } + + // Repair/reinstall must be idempotent: regenerating would silently invalidate every + // client key exchange already performed against the old value. + expect(writeServiceApiTokenFile()).toEqual({ path: serviceApiTokenFilePath(), origin: "file" }); + expect(readFileSync(serviceApiTokenFilePath(), "utf8").trim()).toBe(token); + expect(() => assertServiceAuthEnvironment()).not.toThrow(); + }); + + test("an env token still wins, and a loopback install generates nothing", () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + process.env.OPENCODEX_API_AUTH_TOKEN = "operator-chosen-data-key"; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + expect(writeServiceApiTokenFile()).toEqual({ path: serviceApiTokenFilePath(), origin: "env" }); + expect(readFileSync(serviceApiTokenFilePath(), "utf8").trim()).toBe("operator-chosen-data-key"); + + // Loopback needs no data-plane credential, and on a machine connected to a hub the same + // file holds that hub's issued client key: a local install must not invent or clobber one. + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname: "127.0.0.1", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + expect(writeServiceApiTokenFile()).toBeNull(); + expect(existsSync(serviceApiTokenFilePath())).toBe(false); + }); + + test("an unusable token file is reported by the preflight instead of failing mid-install", () => { if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; @@ -501,8 +578,92 @@ describe("service install auth preflight", () => { providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, defaultProvider: "openai", } as OcxConfig); + writeFileSync(serviceApiTokenFilePath(), "\n", "utf8"); + + expect(() => assertServiceAuthEnvironment()).toThrow(/cannot be used/); + expect(() => assertServiceAuthEnvironment()).toThrow(/ocx service/); + expect(() => writeServiceApiTokenFile()).toThrow(/empty/); + }); + + test("the admin-token refusal tells the operator to unset, not to invent a key", () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + process.env.OPENCODEX_API_AUTH_TOKEN = `ocx_admin_${"f".repeat(40)}`; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + + expect(() => assertServiceAuthEnvironment()).toThrow(/unset OPENCODEX_API_AUTH_TOKEN/); + expect(() => assertServiceAuthEnvironment()).toThrow(/provisions/); + // The chokepoint refuses it too, so no caller can write the broken state (#2696). + expect(() => writeServiceApiTokenFile()).toThrow(/management \(admin\) token/); + expect(existsSync(serviceApiTokenFilePath())).toBe(false); + }); - expect(() => assertServiceAuthEnvironment()).toThrow("OPENCODEX_API_AUTH_TOKEN"); + test("a reused token file that holds the ADMIN token is refused, not silently accepted", () => { + // The incident shape, and the gap the first round left: the `origin: "file"` branch never + // re-checked the collision, so a hand-pasted admin token on disk was reused, `ocx status` + // said `present (file)`, and the hub crash-looped at boot with nothing naming the cause. + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + const admin = `ocx_admin_${"f".repeat(43)}`; + writeFileSync(serviceApiTokenFilePath(), `${admin}\n`, "utf8"); + + // Install/repair stops at the preflight, where the operator can still act. + expect(() => assertServiceAuthEnvironment()).toThrow(/management \(admin\) token/); + expect(() => assertServiceAuthEnvironment()).toThrow(/ocx service repair/); + // `unset` is the WRONG remedy here: nothing is exported. Deleting the file is. + expect(() => assertServiceAuthEnvironment()).not.toThrow(/unset OPENCODEX_API_AUTH_TOKEN/); + // And the writer is still the last line of defence, whichever caller got there. + expect(() => writeServiceApiTokenFile()).toThrow(/not a data-plane token/); + // Refusing must not mutate the file; the operator deletes it deliberately. + expect(readFileSync(serviceApiTokenFilePath(), "utf8").trim()).toBe(admin); + + // And a LOOPBACK install is refused too: `buildServiceShellCommand` cats the file into + // OPENCODEX_API_AUTH_TOKEN whenever it exists, whatever the hostname, so the management + // plane is fenced closed at boot there as well. + saveConfig({ + port: 10100, + hostname: "127.0.0.1", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + expect(() => assertServiceAuthEnvironment()).toThrow(/management \(admin\) token/); + }); + + test("reusing an existing token file makes 'owner-only' true rather than assumed", () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname: "0.0.0.0", + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); + // `readServiceApiTokenState` accepts any bounded regular file, so a reused token can be + // world-readable -- and `ocx status` calls that path "owner-only". + writeFileSync(serviceApiTokenFilePath(), `${"c".repeat(64)}\n`, { encoding: "utf8", mode: 0o644 }); + if (process.platform !== "win32") chmodSync(serviceApiTokenFilePath(), 0o644); + + expect(writeServiceApiTokenFile()).toEqual({ path: serviceApiTokenFilePath(), origin: "file" }); + expect(readFileSync(serviceApiTokenFilePath(), "utf8").trim()).toBe("c".repeat(64)); + if (process.platform !== "win32") { + expect(statSync(serviceApiTokenFilePath()).mode & 0o777).toBe(0o600); + } }); test("allows non-loopback service install when the API token is in the service environment", () => {