diff --git a/devlog/_plan/260911_l7_docs/050_4200_delivery.md b/devlog/_plan/260911_l7_docs/050_4200_delivery.md new file mode 100644 index 0000000000..0afe228e79 --- /dev/null +++ b/devlog/_plan/260911_l7_docs/050_4200_delivery.md @@ -0,0 +1,75 @@ +# #4200 — delivery record + +Commit `59d2dc48f8` on the stacked branch `codex/260911-l7-remote-hub`, based on the #4215 head. +Closes #4200. + +## What shipped + +`docs-site/src/content/docs/guides/remote-hub.md`: + +- The setup block creates `hub` and `remoteGui` before setting any field, and names the exact error + a reader would otherwise hit. A whole-object alternative is offered for a genuinely empty config, + with the warning that a whole-object set replaces rather than merges, plus the two details that + decide whether a line is accepted at all: the value is parsed as JSON first, and both objects are + strict so a mistyped key is rejected at write time. +- A new **Giving the data listener TLS** section states that opencodex terminates no TLS itself, + gives the macOS recipe, shows `ocx connect` with a data origin and a separate `--management-url`, + and documents the loopback-bind trap. +- The troubleshooting list loses `--allow-insecure-http`, which does not exist, and gains the + `403 origin_rejected` symptom pointing at the new section. +- The Docker section says why its nested sets work there, so the guide states one rule. + +`tests/ci-workflows/docs-remote-hub-claims.test.ts` guards all of it, registered in both layout maps. + +## The finding that shaped the recipe + +The issue asks for "a supported macOS Tailscale-extension data transport example". The obvious one +— bind the data listener to loopback so Serve can reach it — is wrong, and wrong in a way that +passes a health check. + +`isApiAuthRequired` is `!isLoopbackHostname(config.hostname)`, keyed on the **configured bind** +address rather than the socket or the `Host` header (`src/server/auth-cors.ts:288`-`290`). A loopback +bind therefore takes the first arm of `isAllowedRequestOrigin` (`auth-cors.ts:90`-`94`), which stops +requiring a data credential and starts requiring the request's `Host` to be loopback too. A TLS +frontend forwards `Host: hub-name.tailnet-name.ts.net`, so `/v1/catalog` returns +`403 origin_rejected` (`src/server/index.ts:1303`) — while `/readyz`, which never runs that check +(`index.ts:1222`-`1242`), still returns `200`. Nothing reads `X-Forwarded-Host`, so the frontend +cannot repair it. + +So the guide keeps the tailnet bind, where credential admission stays on and the `Host` check does +not apply, and puts a loopback forwarder in front for Serve to target. That is also what the issue +reporter deployed successfully. + +| `hostname` | Serve can reach it? | `/v1/catalog` | +|---|---|---| +| `127.0.0.1` | yes | 403 `origin_rejected` — the trap | +| `0.0.0.0` | yes | works; publishes on every interface | +| tailnet IP | no, needs a forwarder | works | + +## Decisions this lane made + +- **Documentation only.** The issue's review explicitly leaves auto-creating a missing parent out + of scope, so `src/cli/config-command.ts` is untouched and the guide documents the CLI as it + behaves. +- **Both config forms are shown**, because they are good at different things: whole-object for a + fresh config, `'{}'` plus nested sets when adapting an existing one without dropping siblings. +- **The dead `--allow-insecure-http` is fixed in the same PR.** It is one line in an owned file and + the same class of defect the issue reports — a published command that cannot run. Leaving a + known-false command beside the one being corrected would be indefensible. +- **Serve's HTTPS port set is not asserted.** The guide uses 8443 and tells the reader to confirm + with `tailscale serve status`, rather than publishing a port list this lane did not verify. +- **Translations are a follow-up.** The seven locale copies still carry the dead flag and the old + setup block. They are outside this lane's owned paths, and the issue asks for English first. + +## Verification + +Local product suite, typecheck and build NOT RUN by operator instruction. Hosted CI on the exact +pushed head is the proof. + +Two read-only reviewers stood in for the local run. One verified every assertion in the new test by +reading, including the ordering assertion that is the actual fix — it confirmed the initializer +precedes the first nested set, and that neither the Docker Compose lines nor the Rollback section +steal the `indexOf` the test depends on. The other checked the recipe against the admission code, +quoting the predicate, and confirmed the anchors, the scope, and both layout registrations. Its one +residual finding — a backgrounded forwarder does not survive a reboot while the service does — is +folded into the guide. diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 6a2b2a33bd..60331d9746 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -74,8 +74,14 @@ for management. The values below are examples: ```bash ocx config set runtimeRole hub ocx config set hostname 100.64.0.10 -ocx config set hub.managementPublicOrigin '"https://hub-name.tailnet-name.ts.net"' ocx config set corsAllowOrigins '["http://localhost:10100"]' + +# A fresh standalone config has no `hub` or `remoteGui` object, and `ocx config set` does not +# create a missing parent: a nested set fails with `config parent path not found: hub`. Setting +# `runtimeRole` does not create it either. Create each object first, then set its fields. +ocx config set hub '{}' +ocx config set remoteGui '{}' +ocx config set hub.managementPublicOrigin '"https://hub-name.tailnet-name.ts.net"' ocx config set hub.managementIngress '{"enabled":true,"port":10101}' ocx config set remoteGui.allowedTailscaleUsers '["operator@example.com"]' @@ -86,6 +92,25 @@ ocx service install ocx service status ``` +On a genuinely empty configuration you can set each object in one call instead: + +```bash +ocx config set hub '{"managementPublicOrigin":"https://hub-name.tailnet-name.ts.net","managementIngress":{"enabled":true,"port":10101}}' +ocx config set remoteGui '{"allowedTailscaleUsers":["operator@example.com"]}' +``` + +Use that form only when the object does not exist yet. A whole-object set **replaces** the object +rather than merging into it, so running the line above against a config that already had +`hub.managementIngress` silently drops the ingress. When you are adapting an existing +configuration, set one field at a time — the parent is already there, so the nested form works and +touches nothing else. + +Two details that decide whether a line is accepted. The value is parsed as JSON first and falls +back to the raw string, which is why a URL is written as `'"https://…"'`: objects, arrays, booleans +and numbers must be valid JSON. And `hub` and `remoteGui` are strict, so a mistyped key is rejected +at write time as `schema_invalid: hub.` instead of becoming a setting that never takes +effect. `managementPublicOrigin` must be a bare origin with no path, query or fragment. + `ocx service install` copies the token into the existing owner-only `service-api-token` path. The launchd plist and systemd user unit read that protected file when the process starts; neither embeds the literal token. Do not paste the value into `ocx config show`, unit/plist output, screenshots, or @@ -129,6 +154,63 @@ The positive browser test must use a real signed-in Tailscale session; a bare `c identity headers needed for automatic session issuance. Pairing remains the fallback when the HTTPS frontend cannot provide trustworthy Tailscale identity. +### Giving the data listener TLS + +The Serve mapping above publishes the **management** ingress only. That ingress never serves +`/v1/*`, `/healthz` or `/readyz`, so on its own it does not give a remote client a usable data +plane. opencodex also terminates no TLS of its own: the listener is plain HTTP and HTTPS is always +an operator-owned frontend. + +Serve can be that frontend for the data plane too, on a second HTTPS port. On macOS it needs one +extra hop, because Tailscale Serve proxies only to `127.0.0.1` — it cannot target the listener you +bound to the node's own tailnet address, and the App Store build of the macOS client refuses a +remote destination outright. Run a loopback forwarder on the hub and point Serve at that: + +```bash +# Any loopback TCP forwarder works; socat is one. The data listener is bound to the tailnet +# address, so 127.0.0.1:10100 is free for the forwarder to take. +socat TCP-LISTEN:10100,bind=127.0.0.1,fork,reuseaddr TCP:100.64.0.10:10100 & + +tailscale serve --bg --https=8443 http://127.0.0.1:10100 +tailscale serve status # expect both mappings: 443 -> 10101, 8443 -> 10100 +``` + +Serve accepts a limited set of HTTPS ports; confirm the mapping was actually created with +`tailscale serve status` rather than assuming the port was allowed. + +Give the forwarder the same lifetime as the hub. A backgrounded shell job dies on reboot while the +service comes back up, which leaves a hub that is running and unreachable over TLS; run it from +launchd or systemd alongside `ocx service install`. + +Then connect with the two origins stated separately. The positional URL is the **data** origin — +it is where `/readyz` and `/v1/catalog` are fetched — and `--management-url` is the dashboard +origin used for pairing and key issuance. They do not have to share a port: + +```bash +ocx connect https://hub-name.tailnet-name.ts.net:8443 \ + --management-url https://hub-name.tailnet-name.ts.net \ + --admin-token-stdin +``` + +When `--management-url` is omitted it is taken from the `/readyz` response, which reports +`hub.managementPublicOrigin`. Setting it explicitly is clearer when the two origins differ. + +**Do not shortcut this by binding the data listener to `127.0.0.1`.** A loopback bind is how +opencodex recognizes a purely local deployment: it stops requiring a data credential, and it starts +requiring the request's `Host` header to be loopback as well. A TLS frontend forwards +`Host: hub-name.tailnet-name.ts.net`, so `/v1/catalog` answers `403 origin_rejected` — while +`/readyz`, which does not run that check, still returns `200`. The deployment looks healthy and +cannot serve a model. Nothing in the request path reads `X-Forwarded-Host`, so the frontend cannot +repair it. Keep the listener on the tailnet address, where credential admission stays on and the +`Host` check does not apply. + +Binding `0.0.0.0` also works and removes the need for a forwarder, since the listener is then +reachable on loopback as well. It publishes the data port on every interface, so prefer it only +where the host has no other network you care about. + +Re-run the acceptance checks against the HTTPS data origin once Serve is up: `/readyz`, an +authenticated `GET /v1/catalog`, and one real routed response. + ### Operator-owned ts.net certificate proxy If you operate your own TLS proxy, obtain a certificate only for the full ts.net FQDN: @@ -269,6 +351,10 @@ docker compose run --rm hub bun run src/cli/index.ts config set remoteGui.allowe docker compose restart hub ``` +These nested sets work because the image seeds a first-run `hub` configuration, so the object +already exists. On a fresh standalone install it does not, and the same lines fail until you create +it — see [Linux systemd or macOS launchd](#linux-systemd-or-macos-launchd) above. + Do not put a token in `ARG`, `ENV`, `COPY`, Compose YAML, image history, or command arguments. Do not mount the Docker socket, the host's home or Codex home, SSH agent, or provider-key files. A management ingress bound to `127.0.0.1:10101` inside the container is reachable only by a TLS/tailnet frontend @@ -328,8 +414,11 @@ For a service rollback, stop the branch service and repair the prior release aga Negotiation fails before token, catalog, journal, or client-state writes. - **Lost or burned pairing code:** create a new short-lived code. Grants are one-use and repeated failures are rate-limited without revealing whether a code exists. -- **Plain HTTP warning:** pairing over non-loopback HTTP requires the explicit - `--allow-insecure-http` opt-in. Admin tokens are never sent over HTTP. +- **Plain HTTP refused:** pairing over non-loopback HTTP is refused outright, and there is no flag + that opts out of it. Put the management origin behind HTTPS, or pair over loopback. Admin tokens + are never sent over HTTP. +- **`403 origin_rejected` from `/v1/catalog` while `/readyz` returns `200`:** the data listener is + bound to loopback behind a TLS frontend. See [Giving the data listener TLS](#giving-the-data-listener-tls). - **Remote session ended:** sign in or pair again. Logout and expiry invalidate only the browser session, not a client data key. - **Outstanding revocation after disconnect:** use the hub dashboard's **Integrations → API Keys** diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 211e1f5c07..ab3c39df6e 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -615,6 +615,7 @@ "docs-bun-source-requirement.test.ts": "ci-workflows", "docs-provider-billing-claims.test.ts": "ci-workflows", "docs-readme-translation-parity.test.ts": "ci-workflows", + "docs-remote-hub-claims.test.ts": "ci-workflows", "doctor-codex-envkey-readiness.test.ts": "service", "doctor-oauth.test.ts": "service", "doctor-provider-apikey.test.ts": "service", diff --git a/tests/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts new file mode 100644 index 0000000000..c2d73f428e --- /dev/null +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -0,0 +1,88 @@ +/** + * The remote hub guide has to stay runnable end to end on a FRESH standalone config. + * + * It did not (#4200). The setup block told the reader to run a nested `ocx config set hub.` + * immediately after `ocx config set runtimeRole hub`, but `runtimeRole` does not create the object + * and the CLI refuses to create a missing parent, so the guide's own next line died with + * `config parent path not found: hub`. A guide that cannot be followed verbatim is worse than a + * missing one: the reader assumes they broke something. + * + * The second half is the data plane. The management ingress deliberately serves no `/v1/*`, + * `/healthz` or `/readyz`, so publishing only that ingress through Tailscale Serve leaves a hub + * that pairs and then cannot answer a request. The trap is quiet, because a loopback-bound data + * listener still returns 200 from `/readyz` while answering 403 on `/v1/catalog`. + * + * These assertions are cheap and the guide is edited often, which is the whole reason the first + * defect survived to a public URL. + */ +import { describe, expect, test } from "bun:test"; +import { repoPath } from "../helpers/repo-root"; + +const GUIDE = repoPath("docs-site/src/content/docs/guides/remote-hub.md"); + +describe("remote hub guide", () => { + test("no nested config set runs before its parent object exists", async () => { + const source = await Bun.file(GUIDE).text(); + + // The ordering IS the fix. Asserting only that the initializer appears somewhere would pass on + // a guide that still sets the field first and mentions `{}` afterwards. + for (const parent of ["hub", "remoteGui"] as const) { + const initializer = source.indexOf(`ocx config set ${parent} '{}'`); + const nested = source.indexOf(`ocx config set ${parent}.`); + expect(initializer, `the guide no longer initializes an empty ${parent} object`).toBeGreaterThanOrEqual(0); + expect(nested, `the guide no longer sets any ${parent} field`).toBeGreaterThanOrEqual(0); + expect( + initializer, + `the guide sets a ${parent}. before creating ${parent}, which fails on a fresh config`, + ).toBeLessThan(nested); + } + + // Name the error, so a reader who hit it recognizes their own terminal output. + expect(source).toContain("config parent path not found: hub"); + }); + + test("the whole-object form carries its replace-not-merge warning", async () => { + // `setPath` assigns the leaf. Recommending the one-call form without this warning would tell + // an operator adapting an existing config to silently drop their management ingress. + const source = await Bun.file(GUIDE).text(); + expect(source).toContain("replaces** the object"); + }); + + test("the guide says opencodex terminates no TLS itself", async () => { + // There is no tls/cert/key field in OcxConfig. A reader who assumes otherwise looks for a + // setting that does not exist instead of standing up a frontend. + const source = await Bun.file(GUIDE).text(); + expect(source).toContain("terminates no TLS of its own"); + }); + + test("ocx connect is shown with a data origin and a separate management origin", async () => { + // The positional URL is where /readyz and /v1/catalog are fetched; --management-url is where + // pairing and key issuance go. They need not share a port, and the macOS recipe relies on that. + const source = await Bun.file(GUIDE).text(); + expect(source).toContain("ocx connect https://hub-name.tailnet-name.ts.net:8443"); + expect(source).toContain("--management-url https://hub-name.tailnet-name.ts.net"); + }); + + test("the macOS Serve constraint and the loopback-bind trap are both documented", async () => { + const source = await Bun.file(GUIDE).text(); + // Serve cannot reach a listener bound to the node's own tailnet address. + expect(source).toContain("Tailscale Serve proxies only to"); + // And the obvious workaround -- bind the listener to loopback -- breaks the catalog quietly. + expect(source).toContain("403 origin_rejected"); + expect(source).toContain("X-Forwarded-Host"); + }); + + test("the Docker section does not contradict the standalone parent-object rule", async () => { + // Compose seeds a hub object, so its nested sets work. Without saying so, the two sections + // read as two different rules and the reader cannot tell which applies to them. + const source = await Bun.file(GUIDE).text(); + expect(source).toContain("because the image seeds a first-run"); + }); + + test("the retired --allow-insecure-http flag is not offered", async () => { + // It is absent from CONNECT_USAGE, pairing refuses non-loopback HTTP outright, and + // remoteGui.allowInsecureHttp is a retired no-op. Offering it sends an operator to an error. + const source = await Bun.file(GUIDE).text(); + expect(source).not.toContain("--allow-insecure-http"); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 02a8b205d5..8e025c4cef 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -450,6 +450,7 @@ "docs-bun-source-requirement.test.ts": "ci-workflows", "docs-provider-billing-claims.test.ts": "ci-workflows", "docs-readme-translation-parity.test.ts": "ci-workflows", + "docs-remote-hub-claims.test.ts": "ci-workflows", "doctor-codex-envkey-readiness.test.ts": "service", "doctor-oauth.test.ts": "service", "doctor-provider-apikey.test.ts": "service",