diff --git a/CLAUDE.md b/CLAUDE.md index bcee56a..17d3c1f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,7 +123,7 @@ Milestone M2 of `docs/design/fleet.md`; the operator flow is `docs/fleet/README. ## Fleet: the operator view + dispatch API (M3) -Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v2/+/{presence,health,pose,capabilities,mission/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `mission/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The mission's `input` is validated only by the robot, against the schema its own capability declared: a copy in the server would be a second contract to keep in step, and it would refuse missions a newer robot understands. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission (M7 makes it structural with a subscribe-only broker credential). The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `pixi run -e fleet fleet-broker-local` is the conda binary for a box without docker, and it strips the WS stanza and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test — `pixi run fleet-ui-check` is that stack in one command (broker on ephemeral ports, server, a temp `MOTE_FLEET_HOME`, the sim's `office_world` bundle as the basemap, and `test/fake_robots.py`, which publishes `protocol.py` and `spec/` payloads, imports `mote_tasks`' own capability set rather than writing one, and is *not* a second robot implementation), torn down afterwards; `-- --keep` leaves it up for UI work. It stays out of CI because it needs docker (conda's mosquitto still has no websockets) *and* a chrome, which the arm runner has not — the decision, and what wiring it in would take, are recorded in `m3-verification.md` §2 rather than left looking like coverage. **A fourth pane, `review`, is where a candidate map is looked at and promoted** (`server/ui/review.mjs`; routes and rationale under the map registry below). It is a *mode*, not a column: opening it stands the operations panes down at every width, because two canvases — one canonical with robots on it, one a candidate without — is the confusion a dedicated view exists to remove. **The phone is the realistic off-LAN client**, so below 760 px the panes become one at a time behind a bottom tab bar (`server/ui/layout.mjs`), selecting a robot in the roster navigates to the map — what the desktop layout gets for free by showing both — and the canvas gained pinch-to-zoom (`pinchSpan`/`pinchUpdate` in `map.mjs`, pure and tested, because a division by a zero span puts NaN in the view scale and blanks the map for good) plus a fingertip-sized hit target. The breakpoint is a **silent** seam — CSS decides what is displayed, JS decides when a selection navigates, and disagreement yields a tab bar over stacked panes rather than an error — so it lives in `layout.mjs` and `ui_test.mjs` reads the stylesheet and holds it there, as it does for every pane having a tab and for `touch-action: none` on the canvas (without which the browser eats the drag and the pinch before a single pointer event arrives). Dispatch's form is **generated from the robot's own capability set** (retained on the broker): a select of the keys it offers, one field per input property, and a **zone picker** exactly where a property's schema `$ref`s zone/v0's zone reference — so the page holds no list of capabilities and no list of which inputs are places, and a keyboard is needed only where the schema really wants free text. Three pre-existing bugs fell out, all of which a desk hides: `hidden` does not hide an element whose class sets `display` (the empty promote picker), the canvas backing store was resized on width alone so a height change left the previous frame's scale bar under the new one, and the scale bar was drawn in the dark theme's near-white on a white basemap — a canvas gets no cascade, so it now reads `--dim` off the element. Measurements, including `browser_check.mjs`'s phone pass, are `m3-verification.md` §9; **a real device is still the acceptance** — emulation gets the viewport and the touch points right and the thumb wrong. +Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v2/+/{presence,health,pose,capabilities,mission/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `mission/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The mission's `input` is validated only by the robot, against the schema its own capability declared: a copy in the server would be a second contract to keep in step, and it would refuse missions a newer robot understands. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission (a subscribe-only broker credential would make it structural, and waits on the broker having credentials at all). The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `pixi run -e fleet fleet-broker-local` is the conda binary for a box without docker, and it strips the WS stanza and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test — `pixi run fleet-ui-check` is that stack in one command (broker on ephemeral ports, server, a temp `MOTE_FLEET_HOME`, the sim's `office_world` bundle as the basemap, and `test/fake_robots.py`, which publishes `protocol.py` and `spec/` payloads, imports `mote_tasks`' own capability set rather than writing one, and is *not* a second robot implementation), torn down afterwards; `-- --keep` leaves it up for UI work. It stays out of CI because it needs docker (conda's mosquitto still has no websockets) *and* a chrome, which the arm runner has not — the decision, and what wiring it in would take, are recorded in `m3-verification.md` §2 rather than left looking like coverage. **A fourth pane, `review`, is where a candidate map is looked at and promoted** (`server/ui/review.mjs`; routes and rationale under the map registry below). It is a *mode*, not a column: opening it stands the operations panes down at every width, because two canvases — one canonical with robots on it, one a candidate without — is the confusion a dedicated view exists to remove. **The phone is the realistic off-LAN client**, so below 760 px the panes become one at a time behind a bottom tab bar (`server/ui/layout.mjs`), selecting a robot in the roster navigates to the map — what the desktop layout gets for free by showing both — and the canvas gained pinch-to-zoom (`pinchSpan`/`pinchUpdate` in `map.mjs`, pure and tested, because a division by a zero span puts NaN in the view scale and blanks the map for good) plus a fingertip-sized hit target. The breakpoint is a **silent** seam — CSS decides what is displayed, JS decides when a selection navigates, and disagreement yields a tab bar over stacked panes rather than an error — so it lives in `layout.mjs` and `ui_test.mjs` reads the stylesheet and holds it there, as it does for every pane having a tab and for `touch-action: none` on the canvas (without which the browser eats the drag and the pinch before a single pointer event arrives). Dispatch's form is **generated from the robot's own capability set** (retained on the broker): a select of the keys it offers, one field per input property, and a **zone picker** exactly where a property's schema `$ref`s zone/v0's zone reference — so the page holds no list of capabilities and no list of which inputs are places, and a keyboard is needed only where the schema really wants free text. Three pre-existing bugs fell out, all of which a desk hides: `hidden` does not hide an element whose class sets `display` (the empty promote picker), the canvas backing store was resized on width alone so a height change left the previous frame's scale bar under the new one, and the scale bar was drawn in the dark theme's near-white on a white basemap — a canvas gets no cascade, so it now reads `--dim` off the element. Measurements, including `browser_check.mjs`'s phone pass, are `m3-verification.md` §9; **a real device is still the acceptance** — emulation gets the viewport and the touch points right and the thumb wrong. ## Fleet: reading a robot's state over HTTP @@ -153,12 +153,11 @@ zero-length payload) clears the field, or this server would assert a state the broker has stopped serving. **`mission_status` is the last status, not a history**, since one transition is all that is retained; anything wanting every transition still subscribes, which is what `watch` and `dispatch` keep the -broker for. And **the route takes an operator token where the roster does not**, -because this is where the coordinates are — which hides nothing until M7, since -the same payloads are on the anonymous broker and every id is in the anonymous -roster; the token gives the route M7's shape now. It is checked *before* the -lookup, so an unauthenticated answer does not depend on the id, which matters -once M7 gates the roster. +broker for. And **both routes take an operator token**, from the gate in front +of every `/v1` route (see "Fleet: the API auth gate" below), before the robot is +looked up — so an unauthenticated answer does not depend on the id. It hides +less than it looks: the same payloads are on the broker, which is still +anonymous. `publisher` and `feed` are injected as a pair in `serve()`, since a live subscription beside a stubbed publisher would have a test dialling a broker it does not have; the acceptance is `test_e2e_fleet.py`'s @@ -213,7 +212,7 @@ floor's frame and a revision is an estimate registered into it. Three deliberate the flip and the announcement are reported separately (a broker that is down must not half-promote a floor; the server re-announces every floor at startup, which repairs it), an **upload carries no operator credential** — it names an enrolled -robot, is bounded and audited, and is inert until M7 gives robots a credential — +robot, is bounded and audited, and is inert until robots have a credential — and a pulled map takes effect on the **next bringup**, since `map_server` reads its map at startup, so health now carries the revision each robot is actually running. M3's `/v1/maps` routes kept their shape and changed source; the @@ -592,6 +591,77 @@ describes the seven-field vocabulary; a successor revision there is outstanding, and `test_spec_conformance.py` carries a strict `xfail` that will fail loudly when it lands. +## Fleet: the API auth gate, the tailnet policy, locked installs (M7, part) + +The cheap half of M7 of `docs/design/fleet.md`: the API's own credential, the +network's own rules, and a reproducible install. The broker half — per-robot and +per-operator broker credentials and the ACL that keeps the three principals +apart — is deliberately **not** here and is parked behind M6, so the broker is +still anonymous and `fleetctl watch` and the dashboard's read path still connect +to it without a credential. + +**One gate in front of `/v1` routing, and it is the route table that dispatches.** +Through M3 only `dispatch` and `audit` checked a token, which left the roster, +the basemaps, the zone vocabularies, the registry and the broker's address +readable by anything that could reach the port. `fleet_server.ROUTES` is now the +list of every path the server answers — method, path template, handler and the +credential it costs — and `_handle` matches against it, takes the credential and +only then calls the handler. Three consequences, all of them the reason for the +table rather than a chain of `elif`s. A route added later is **authenticated by +default**, and has to opt out in the same line that declares it. An anonymous +caller is refused **before the table is consulted for existence**, so a 404 can +never say which routes are real. And the acceptance is a test that *walks the +table* (`test_fleet_server.py`, `SAMPLES` filling the path variables) rather than +a hand-kept list of routes that goes stale the first time one is added — a route +with a new path variable fails it with a `KeyError`, which is the intended way to +be told. + +**Four table entries are open, plus the static UI, each for a stated reason.** +`/healthz`, because a liveness probe that needs a secret is a liveness probe +nobody wires up. The static UI — not a route at all, but what an unmatched GET +falls through to — because the page has to load in order to ask for a token. +`POST /v1/enroll`, because it carries its own enrollment token and an unattended +first boot has no human behind it. And **both halves of M4's map exchange** — +the robot's upload and the robot's `bundle.tar.gz` pull — because robots have no +credential to present and gating the pull would mean a fleet whose maps never +reach its robots. The pull is the carve-out M7's own branch did not have to make: +it was written before M4 existed. The upload was already inert by M4's design; +the pull serves only what an operator has already promoted. + +**The dashboard has two states, signed in or asking to be.** `/v1/config` is +operator-only like every other route, so there is no read-only mode left to fall +back to and `app.mjs`'s boot is now `start()`: no token means the gate, a token +that stops working means the gate again, and pasting one starts everything with +no reload. Two things fell out. The basemap is fetched **with the token and +decoded from a blob** (`loadImage`, injected into the review pane exactly as +`api` is), because `` carries no `Authorization` header and gating +`map.png` otherwise blanks both canvases. And the dispatch note stopped being +overwritten: one line carried both what the selected capability does and what the +last dispatch did, so any re-render replaced the outcome the operator had just +read — the summary is now written when the *selection* changes, keyed on robot +and capability. `fleetctl`'s `robots` and `sites` verbs needed the token too, for +the same reason: they read routes that used to be open. + +**The tailnet policy is a committed file**, `mote_bringup/tailscale/policy.hujson`, +pasted into the admin console by the operator (`docs/fleet/README.md` §1a). It +carries its own `tests` block asserting that **no robot can reach another robot**, +and Tailscale refuses to save a policy that fails it — so the acceptance is +checked by the thing enforcing it rather than by a person reading the rules. +`test_tailnet_roles.py` adds what the console cannot: that the file still parses, +and that every tag `install.sh` can advertise is one the policy declares (an +undeclared tag fails `tailscale up` with "requested tags are invalid or not +permitted", which is a robot stopped by a file it never reads). + +**Provisioning installs from the lockfile.** `pixi install --locked` before +`pixi run build` in `provisioning/user-data.template`: it aborts if `pixi.lock` +is out of date with `pixi.toml` rather than solving something new, and every +package in that lockfile is pinned by sha256. A silent re-solve on a robot nobody +is watching is what this forbids. + +The M3 `[hidden]` defect this milestone's branch also carried is **already fixed +on main** — `style.css` has the `[hidden] { display: none !important }` rule with +its own note — so nothing was ported for it. + ## Fleet: the server pipelines (Ms) Milestone Ms of `docs/design/fleet.md`: how the two **non-robot** machines are built and updated, runbook in `docs/fleet/server-pipelines.md`, measurements in `docs/fleet/ms-verification.md`. Both are container deploys **driven by their operator, not by the fleet server** — a robot is fleet-managed, a server is infrastructure — and the pipelines differ in exactly one thing, state. **The fleet server** (`mote_fleet/deploy/`: `Dockerfile` + `docker-compose.yml` + `fleet-deploy.sh`, image `ghcr.io/clachdev/mote-fleet` built by `.github/workflows/fleet-image.yml`) is two containers — mosquitto mounting **`server/mosquitto.conf`, the same file `fleet-broker` uses**, so deployed and workstation brokers cannot drift, with the **image tag pinned once** in the compose file and read back by `broker.sh` so they cannot drift on the binary either (a *minor* series, `2.1-alpine`, not the floating `:2`: 2.0 links libwebsockets and 2.1 implements websockets natively, so `:2` moving again could take the dashboard's read path with it, and the broker healthcheck probes 9001 as well as 1883 because mosquitto stays up and healthy-looking when only the websockets listener fails to open), and a python image carrying the API, the registry and the M3 dashboard — plus two named volumes holding the only state that matters: `/var/lib/mote-fleet` (registry.db + the `sites/` bundles the dashboard's basemaps come from) and the broker's retained messages. `.env` is the declared state; `BROKER_HOST` is the one value that must be right (handed to robots verbatim at enrollment, so the compose file refuses to start without it) and `BROKER_WS_PORT` is published *and* passed as `--broker-ws-port`, because it is the port the browser is told to reach the broker on. It holds state, so its update is a **gated recreate**, not blue/green: `fleet-deploy.sh update` tags the running image `:previous` before pulling, recreates, health-gates on `/healthz` over the *published* port, and puts the old image back automatically if that fails; `backup`/`restore` snapshot both volumes (registry via sqlite3's online backup API, not `cp`). **The inference server** (`mote_perception/deploy/inference-deploy.sh`, one file curled onto the host) is stateless, so it *is* blue/green: the candidate runs on shadow ports 5611/5612 while the current one keeps serving, and must pass `mote_perception/tools/probe.py` — health **and a real synthetic frame**, because a health sentinel is answered before the model has ever loaded and cannot see a broken weight download or a CUDA mismatch. The **flip is a stop-then-start**, deliberately: pushing a new port out to robots (as the design sketch had it) means editing `perception.yaml` on every robot, a worse outage than the seconds this costs, which the robot's warn-and-skip fallback makes a non-event. Every check runs *inside the image being deployed*, so the GPU box still installs nothing. `mote_perception/deploy/test/drill.sh` (`pixi run deploy-test`) exercises that whole pipeline with stub images on any machine with docker — no GPU — and `mote_fleet/test/test_fleet_outage.py` is the other half of the milestone's acceptance: kill the broker under a live agent and the robot still finishes its task, then the agent reconnects by itself. diff --git a/docs/design/fleet.md b/docs/design/fleet.md index aa6c416..839fd8c 100644 --- a/docs/design/fleet.md +++ b/docs/design/fleet.md @@ -978,7 +978,8 @@ M7 (security hardening) : cross-cutting, folds into each; can start after M0 basemaps are read from **site bundles on the fleet box** through routes M4 keeps while replacing where the bytes come from; and the **operator token is one credential on one path**, not the auth story M7 owes — the read routes and - the broker are still open on the tailnet. + the broker were still open on the tailnet. (The read routes are closed since; + the broker is not.) ### v1 — second robot enrolled, plus shared infrastructure @@ -1058,6 +1059,15 @@ M7 (security hardening) : cross-cutting, folds into each; can start after M0 a robot can't read another robot's command topic. *Depends on:* M0; folds into each milestone as it lands. + **Landed so far:** operator auth on every `/v1` route (one gate in front of + routing, four stated carve-outs), the Tailscale access policy as a committed + file with its own `tests` block, and `pixi install --locked` in provisioning. + **Still owed:** the broker half — per-robot and per-operator broker + credentials and the ACL that keeps them apart — which is where the second + acceptance criterion ("a robot can't read another robot's command topic") + lives, and which waits on M6. Package signing waits on M5, when there is a + package to sign. + ### Cross-cutting — adopting the open specifications Not a milestone of its own: it re-shapes M1's control plane and M3's dispatch diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 0bc2530..a16cea4 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -75,19 +75,26 @@ Tags do not exist until an owner is declared for them, and `--advertise-tags` on a machine fails with "requested tags are invalid or not permitted" if you skip this: -```jsonc -{ - "tagOwners": { - "tag:robot": ["autogroup:admin"], - "tag:fleet": ["autogroup:admin"], - "tag:inference": ["autogroup:admin"], - }, - // The default policy already allows every device to reach every other, which - // is what M0 wants. M7 replaces this with per-tag rules (operators reach - // robots; robots reach the broker and their own inference box; robots cannot - // reach each other). -} -``` +The policy this repo ships is +[`mote_bringup/tailscale/policy.hujson`](../../mote_bringup/tailscale/policy.hujson): +the three tags above, plus the rules that say who may open a socket to what. +Tailscale keeps the policy in its console rather than in a repo, so **the file is +the source of truth and the console is a copy** — edit here first, then paste: + +1. +2. paste the file, **Preview** to see what changes, then **Save**. + +Two things it does that the default "everyone reaches everyone" policy does not. +Operators reach a robot's SSH and Foxglove ports and nothing else on it; a robot +reaches the fleet server's API and broker and its inference box's two wire ports, +and **no robot can reach another robot** — there is no robot-to-robot anything in +v1, so the absence of a rule is the design. The file carries a `tests` block +asserting exactly that, and Tailscale evaluates it on every save and refuses a +policy that fails one, so the rule is checked by the thing enforcing it. + +Do this before joining any machine: `--advertise-tags` on a tag the policy does +not declare fails with "requested tags are invalid or not permitted", which is +the first thing to check if joining a robot fails. **4. Mint an auth key per robot.** Admin console → **Settings → Keys → Generate auth key**. For a robot: @@ -109,7 +116,7 @@ opens a browser to authenticate you. (180 days by default) and needs a human to re-authenticate it, which for a robot means it silently drops off the tailnet one day months from now. Tagged devices do not expire. That failure mode is the practical argument for `tag:robot`, -ahead of anything M7 does with it. +ahead of anything the access policy does with it. ### 1b. Joining machines @@ -145,9 +152,11 @@ any tagged role are mutually exclusive and the script refuses the combination. For one operator at one site, leave the dev machine an untagged workstation that happens to run Mosquitto and the inference servers: nothing functional depends on the tag (robots reach it by MagicDNS either way, and `inference_host` is just a -name). What you defer is M7's ACLs — a rule keyed on your user's device rather -than `tag:fleet`/`tag:inference`. Tag it when a second person or a second machine -appears and the roles want to outlive your account. +name). What you defer is the policy's rules — they are keyed on +`tag:fleet`/`tag:inference`, so an untagged dev box is reachable under +`autogroup:member` rather than under the rule written for its role. Tag it when a +second person or a second machine appears and the roles want to outlive your +account. **Verify off-LAN** (the M0 acceptance test) — from a device on a *different* network, e.g. a laptop tethered to a phone: @@ -483,14 +492,18 @@ fleet box's MagicDNS name (`fleet-box`), not `localhost` — it is handed out verbatim in every enrollment answer. It defaults to the box's hostname. Under compose it is `BROKER_HOST` in `.env`, and the stack refuses to start without it. -**Security, plainly:** the broker is anonymous and the API's *read* routes are -unauthenticated. Dispatch is not — it needs an operator token (§8) — but that is -one credential on one path, not an auth story. It is proportionate only because -the tailnet is the boundary: WireGuard authenticates, and nothing here is -exposed to the internet. Do not put either on a network the robots are not -already trusted on. Per-robot broker credentials and operator auth everywhere -are M7; the shape of what changes is in -[`control-plane.md`](control-plane.md#security-posture-and-what-m7-changes). +**Security, plainly:** every `/v1` route on the API needs an operator token +(§8), checked by one gate in front of routing rather than by each handler. Four +things are open and each for a reason: `/healthz`, the static UI, enrollment +(which carries its own token), and the two robot-facing map routes, which carry +no credential because robots have none to carry yet. The **broker is still +anonymous**, so the dashboard's read path and `fleetctl watch` connect to it +without one. The tailnet is the outer boundary — WireGuard authenticates, the +policy in §1a says who may reach which port, and nothing here is exposed to the +internet. Do not put the broker on a network the robots are not already trusted +on. Per-robot and per-operator broker credentials are still to come; the shape of +what changes is in +[`control-plane.md`](control-plane.md#security-posture-and-what-is-still-owed). --- @@ -571,6 +584,11 @@ over the network; the **name on it is what the audit log records**, which is why an unnamed one is refused. `fleetctl operator list|revoke` are the other two verbs, and the route contract is [`fleet-api.md`](fleet-api.md). +**`MOTE_FLEET_TOKEN` is now needed for every `fleetctl` verb that talks to the +API**, not only `dispatch` and `audit`: the roster, the maps and the registry are +operator-only too. Without it they answer `401` and say so. `watch` is the +exception, because it reads the broker rather than the API. + **A mission is a capability and a typed input**, not a sentence: the first argument is a capability key and the rest are `key=value` pairs (or one `{...}` JSON object when a value is not a string). What keys a robot offers, and what @@ -654,8 +672,8 @@ mote-01 Scout (home) off publishes `online: false` through its Last Will and reads `offline`, while `unknown` means nothing has ever been heard from it — or that the fleet server is not connected to the broker, which both commands say outright when it is so. -The detail view needs the operator token; the roster does not. What it shows of -a mission is the **last** status, because one is all the broker retains; `watch` +Both need the operator token, like every API route. What the detail view shows +of a mission is the **last** status, because one is all the broker retains; `watch` and `dispatch` are what show every transition, and they keep the broker for exactly that reason. The route is [`fleet-api.md`](fleet-api.md#get-v1robotsrobot_id), and it is the same answer @@ -684,11 +702,14 @@ no request/response loop, and no service between the broker and the browser. That is the read path in [`fleet.md`](../design/fleet.md) Q5, and it is why the broker needs the WebSocket listener from §6. -**Paste an operator token to dispatch.** Without one the page is read-only, -which is a perfectly good wall display. The token is kept in the browser's local -storage and sent to the fleet API as a bearer credential; the page holds **no -broker credential that can publish**, and its MQTT client implements no PUBLISH -packet at all. +**Paste an operator token to see anything at all.** `/v1/config` — what the +page is built out of — is operator-only like every other route, so there is no +read-only mode: the dashboard is signed in, or it is asking to be, and a wall +display whose token was revoked shows the gate rather than a stale fleet. Pasting +a token starts everything without a reload. The token is kept in the browser's +local storage and sent to the fleet API as a bearer credential; the page holds +**no broker credential that can publish**, and its MQTT client implements no +PUBLISH packet at all. **Each state is said once**, by the strongest idiom the page has. The roster row's dot and its state column already read `ok`, so the line under them speaks diff --git a/docs/fleet/control-plane.md b/docs/fleet/control-plane.md index ea79dd5..a4adefc 100644 --- a/docs/fleet/control-plane.md +++ b/docs/fleet/control-plane.md @@ -329,7 +329,8 @@ broker that lost its retained state with its volume, repairs itself. fleet box is reached — MagicDNS name, tailnet address or localhost. `sha256` is checked by the puller before anything is staged. It is not a -security boundary (the tailnet is that until M7); it is there because a +security boundary (the tailnet is that, while the broker is anonymous); it is +there because a transfer that silently truncated would otherwise become a map, and a wrong map is worse than no map. @@ -482,25 +483,32 @@ robots enrolling at once get eight distinct ids. --- -## Security posture (and what M7 changes) +## Security posture (and what is still owed) -M1 is proportionate to the M0 substrate and no further. Stated plainly so it is -not mistaken for a finished story: +Stated plainly so it is not mistaken for a finished story: - **The broker is anonymous.** Any client that can reach it may publish or subscribe anywhere in the tree. WireGuard is the authentication boundary; nothing here is reachable from the public internet. -- **The fleet API has no auth** on its read routes. Enrollment tokens and, since - M3, operator tokens are the only credentials in the system. +- **The fleet API is not.** Every `/v1` route needs an operator token, checked by + one gate in front of routing ([`fleet-api.md`](fleet-api.md)); `/healthz`, the + static UI, enrollment and the two robot-facing map routes are the carve-outs, + each for a stated reason. +- **The tailnet has rules.** `mote_bringup/tailscale/policy.hujson` is the + committed access policy: operators reach the fleet box and a robot's SSH and + Foxglove ports, robots reach the fleet server and their inference box, and no + robot reaches another. Its `tests` block asserts that last one, and Tailscale + refuses to save a policy that fails it. - **Dispatch is mediated, as of M3.** M1's `fleetctl` published straight to the broker; now it and the dashboard both POST to `/v1/robots//dispatch`, - which authorizes an operator token and writes an audit row before publishing - ([`fleet-api.md`](fleet-api.md)). As this section promised, **the topic tree - did not change** — only who publishes to it. The browser holds no broker - credential that can publish; making that structural on the broker side, with a - subscribe-only credential, is still M7's. - -M7 adds per-robot broker credentials (username = `robot_id`, publish confined to -its own prefix), operator auth on the API, and the Tailscale ACLs that stop -robots reaching each other. Until then: do not put the broker or the API on a -network the robots are not already trusted on. + which authorizes an operator token and writes an audit row before publishing. + As this section promised, **the topic tree did not change** — only who + publishes to it. The browser holds no broker credential that can publish; + making that structural on the broker side, with a subscribe-only credential, + waits on the broker having credentials at all. + +What is still owed is the broker half: per-robot credentials (username = +`robot_id`, publish confined to its own prefix), a subscribe-only operator +credential for the browser, and the ACL that keeps the three principals apart. +Until then, do not put the broker on a network the robots are not already +trusted on. diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 41ea07c..b527f51 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -53,13 +53,20 @@ Status codes are part of the contract: a client may switch on them. ## Authentication +**Every `/v1` route needs an operator token except those named below.** The +check is one gate in front of route dispatch, not a line in each handler, so a +route added later is authenticated by default and has to opt out in a place a +reviewer reads. An anonymous caller is refused *before* the route table is +consulted for existence: an unknown path answers `401`, never a `404` that would +say which routes are real. + | Route | Credential | |---|---| -| `POST /v1/enroll` | an **enrollment token** in the body (single-use by default) | -| `POST /v1/robots//dispatch`, `GET /v1/audit`, `GET /v1/robots/` | an **operator token** as `Authorization: Bearer ` | -| `POST …/revisions//promote` | an **operator token** | -| `POST …/revisions/` (map upload) | none, but the `robot_id` must be enrolled — see [the registry](#the-map-registry-m4) | -| everything else | none — see the security note below | +| everything under `/v1` | an **operator token** as `Authorization: Bearer ` | +| `GET /healthz` | none — a liveness probe that needs a secret is one nobody wires up | +| the static UI (`/`, `/*.mjs`, …) | none — the page has to load in order to ask for a token, and holds no fleet data until it has one | +| `POST /v1/enroll` | an **enrollment token** in the body (single-use by default) — a robot is not an operator, and an unattended first boot has no human behind it | +| `POST …/revisions/` (map upload) and `GET …/revisions//bundle.tar.gz` (pull) | none, but an upload's `robot_id` must be enrolled — see [the registry](#the-map-registry-m4) | Operator tokens are minted on the fleet box, against the registry file, never over the network: @@ -76,15 +83,22 @@ is refused. Revocation keeps the row: who *had* access is part of the record. Bearer header only — never a query parameter, which would put the credential in every access log between here and the browser. -**Security posture, plainly.** Most read routes are unauthenticated and the -broker is anonymous, exactly as M1 left them. M3 adds a credential on the -*write* path and a record of who used it, which is the milestone's brief; since -then two reads have been gated as well: the audit log, which nothing else -serves, and one robot's live state, whose payloads the anonymous broker also -carries until M7. It is proportionate only while the tailnet is the boundary. M7 adds operator -auth on the rest of the read routes, per-robot broker credentials, and the -Tailscale ACLs. Until then, do not expose this port to a network the robots are -not already trusted on. +**The two robot routes are the carve-out that costs something, and it is +deliberate.** M4's rule is that uploading is not publishing: a candidate changes +nothing about any floor until an operator promotes it, so the upload is bounded, +audited and inert, and the pull serves only what an operator has already +promoted. The alternative today is a credential robots do not have — issuing one +at enrollment is its own piece of work, and until it lands these two routes are +what the tailnet protects rather than what the API does. + +**Security posture, plainly.** The API needs a credential everywhere; the broker +is still anonymous, so the dashboard's MQTT read path is open to anything on the +tailnet that can reach port 9001. The write path is not: the browser's MQTT +client implements no PUBLISH packet, and every write to `mission/command` goes +through `POST …/dispatch` here. The outer boundary is the tailnet +(`mote_bringup/tailscale/policy.hujson`), which is what keeps this port off the +public internet. Do not expose it to a network the robots are not already +trusted on. --- @@ -210,14 +224,9 @@ mission, which is a property of asking rather than of listening. fleet server is repopulated by the broker within about a second of connecting; a stored copy could only ever be the staler answer. -**The operator token hides nothing yet.** This route carries the coordinates — -a pose says where in a building a robot is, the mission status what it was told -to do there — so it takes the credential M7 will require of every read. Until -M7, the same payloads are on the anonymous broker and every `robot_id` is in the -anonymous roster, whose `presence` column shows nothing a broker subscriber -cannot already see. The token is checked *before* the robot is looked up, so an -unauthenticated request gets `401` whatever id it names; that stops ids leaking -through this route once M7 gates the roster, and not before. +**The operator token hides less than it looks.** The gate takes it before the +robot is looked up, so an unauthenticated request gets `401` whatever id it +names. The same payloads are on the broker, which is still anonymous. ### `POST /v1/robots//dispatch` @@ -570,8 +579,8 @@ grey rectangle, which is what a mapping run that never got going looks like. **Why this route has no credential.** Everything it can do is inert: a candidate changes no floor, is bounded in size and count, and is recorded in the audit log against the robot that sent it. The write that *does* change something — -promote — is the operator's. M7 replaces the `robot_id` check with a per-robot -credential. +promote — is the operator's. Replacing the `robot_id` check with a per-robot +credential waits on robots having one to present. ### `POST /v1/sites//floors//revisions//promote` @@ -646,8 +655,8 @@ with nothing published at all. It stays under a `/v1/maps`-shaped path and never over `/v1/zones`, because it is served beside a basemap and that is what the two prefixes divide. -All three are reads, so like every other read route they take no operator token; -M7 changes that for all of them at once. +All three are reads, and like every other `/v1` route they need an operator +token. ### `POST /v1/sites//floors//zones` @@ -731,8 +740,15 @@ The dashboard holds an operator token for this API and **no broker credential at all that can publish**. The read path connects to the broker's WebSocket listener with a client that implements no PUBLISH packet ([`ui/mqtt.mjs`](../../mote_fleet/server/ui/mqtt.mjs)) — the split is enforced by -omission, not by intention. M7 makes that structural on the broker side too, -with a subscribe-only credential. +omission, not by intention. A subscribe-only broker credential would make it +structural on the broker's side too, and waits on the broker having credentials +at all. + +**The token is what the page is built out of.** `/v1/config` is operator-only +like every other route, so there is no read-only mode to fall back to: the +dashboard has two states, signed in or asking to be. Pasting a token starts +everything without a reload, and a token that stops working puts the page back at +the gate rather than into a half-state showing stale rows. **Reviewing a candidate is all GETs.** The review pane reads a revision's `map.json`, `map.png` and `zones.json`; the two writes beside them are the diff --git a/mote_bringup/provisioning/user-data.template b/mote_bringup/provisioning/user-data.template index 9b05830..79b83a3 100644 --- a/mote_bringup/provisioning/user-data.template +++ b/mote_bringup/provisioning/user-data.template @@ -88,8 +88,15 @@ runcmd: # 3. The software. Until the prefix.dev channel ships a robot package (M5), # this is the same source checkout + pixi build the robot runs today. + # + # `install --locked` before the build: it aborts if pixi.lock is out of + # date with pixi.toml rather than solving something new, and every package + # in that lockfile is pinned by sha256. So the robot installs the dependency + # set that was tested — a silent re-solve on a machine nobody is watching is + # what this forbids. - [bash, -c, "sudo -u @USER@ -H bash -lc 'curl -fsSL https://pixi.sh/install.sh | bash'"] - [bash, -c, "sudo -u @USER@ -H bash -lc 'git clone @REPO@ ~/Mote'"] + - [bash, -c, "sudo -u @USER@ -H bash -lc 'cd ~/Mote && ~/.pixi/bin/pixi install --locked'"] - [bash, -c, "sudo -u @USER@ -H bash -lc 'cd ~/Mote && ~/.pixi/bin/pixi run build'"] # 4. Host-level setup the package cannot carry: udev rules, wifi power save, diff --git a/mote_bringup/tailscale/policy.hujson b/mote_bringup/tailscale/policy.hujson new file mode 100644 index 0000000..23e7648 --- /dev/null +++ b/mote_bringup/tailscale/policy.hujson @@ -0,0 +1,154 @@ +// The Mote tailnet access policy (fleet.md Q7, milestone M7). +// +// This is the *outer* boundary. Inside it, the fleet API needs an operator +// token on every route (docs/fleet/fleet-api.md); the broker is still anonymous +// and per-robot broker credentials are the companion security task's. This file +// decides which machines may open a socket to which port at all, and it is what +// answers the milestone's first acceptance criterion — a device that is not on +// the tailnet reaches nothing, because nothing here is published to the +// internet and Tailscale denies by default. +// +// The second criterion it contributes to: **robots cannot reach each other.** +// There is deliberately no rule granting tag:robot any access to tag:robot. In +// v1 there is no robot-to-robot anything — no coordination, no shared map +// service, no peer discovery (fleet.md "Non-goals") — so the absence of a rule +// here is the design, not an omission. +// +// APPLYING IT. Tailscale keeps the policy in its admin console, not in a repo, +// so this file is the source of truth and the console is a copy: +// +// 1. https://login.tailscale.com/admin/acls/file +// 2. paste this file, "Preview" to see what changes, then Save. +// +// Keep the two in step by editing here first. `tailscale-policy` in a CI job or +// the `gitops-pusher` tool can automate the push; at one-operator scale, don't. +// The tags must exist (below) before any machine runs `pixi run tailnet` with +// them — `--advertise-tags` on an undeclared tag fails with "requested tags are +// invalid or not permitted", which is the first thing to check if joining a +// robot fails. +{ + // ---- who owns the tags ------------------------------------------------ + // + // A tagged device belongs to the tailnet rather than to a person, which is + // what lets it outlive the operator's account and — the practical reason — + // stops its key expiring after 180 days and silently dropping a robot off the + // network months later. + "tagOwners": { + "tag:robot": ["autogroup:admin"], + "tag:fleet": ["autogroup:admin"], + "tag:inference": ["autogroup:admin"], + }, + + // ---- the ports, named once ------------------------------------------- + "hosts": {}, + + // ---- who may reach what ---------------------------------------------- + // + // Tailscale denies anything not listed. Every rule below exists because + // something in this repo dials that port; if a rule looks unused, the thing + // that used it has gone, and the rule should go too. + "acls": [ + // Operators (you, on a laptop or a phone) reach the fleet server's API and + // dashboard, and the broker's WebSocket listener the dashboard subscribes + // through. 1883 as well, so `fleetctl watch` works from a workstation. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:fleet:8080", "tag:fleet:1883", "tag:fleet:9001"], + }, + + // Operators reach a robot for the single-robot deep view (foxglove_bridge, + // M2) and for hands-on work. Note this is the *only* rule that opens a + // robot's ports to anything. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:robot:22", "tag:robot:8765"], + }, + + // Operators reach the GPU box directly, for `pixi run inference-health` and + // the bench tools. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:inference:5601", "tag:inference:5602", "tag:inference:22"], + }, + + // A robot reaches the fleet server: enrollment + the registry API on 8080, + // the control-plane broker on 1883. Nothing else, and in particular not the + // dashboard's WebSocket listener — a robot has no browser. + { + "action": "accept", + "src": ["tag:robot"], + "dst": ["tag:fleet:8080", "tag:fleet:1883"], + }, + + // A robot reaches the inference server's depth and detect wires. This is + // the rule that makes "run it on a trusted network" true for a protocol + // that is unauthenticated by design (depth_wire.py): the wire has no + // credentials, so the network is what decides who may speak it. + { + "action": "accept", + "src": ["tag:robot"], + "dst": ["tag:inference:5601", "tag:inference:5602"], + }, + + // Deliberately absent, and each absence is load-bearing: + // + // tag:robot -> tag:robot no robot-to-robot anything in v1, and + // this is the acceptance criterion + // tag:fleet -> tag:robot the fleet server never dials a robot; the + // agent is the sole egress and it dials out + // tag:inference -> anything the GPU box answers, it never initiates + // * -> autogroup:internet no exit nodes; robots use their own link + ], + + // ---- SSH -------------------------------------------------------------- + // + // Tailscale SSH, so getting onto a robot needs a tailnet identity rather than + // a key that has been copied around. "check" re-authenticates the operator in + // a browser periodically; robots are not reachable by each other here either. + "ssh": [ + { + "action": "check", + "src": ["autogroup:member"], + "dst": ["tag:robot", "tag:fleet", "tag:inference"], + "users": ["autogroup:nonroot", "root"], + }, + ], + + // ---- tests ------------------------------------------------------------ + // + // Tailscale evaluates these on every save and refuses a policy that breaks + // one, so the acceptance criteria are checked by the thing enforcing them + // rather than by a person reading the rules above. + "tests": [ + { + "src": "tag:robot", + "accept": [ + "tag:fleet:1883", + "tag:fleet:8080", + "tag:inference:5601", + ], + "deny": [ + // The M7 criterion: one robot cannot reach another, on any port. + "tag:robot:22", + "tag:robot:8765", + "tag:robot:1883", + // A robot has no business in the dashboard's listener or on the GPU + // box's shell. + "tag:fleet:9001", + "tag:inference:22", + ], + }, + { + "src": "tag:inference", + "deny": ["tag:robot:8765", "tag:fleet:8080", "tag:fleet:1883"], + }, + { + "src": "tag:fleet", + // The agent dials the server, never the other way round. + "deny": ["tag:robot:8765", "tag:robot:22"], + }, + ], +} diff --git a/mote_bringup/test/test_provision.py b/mote_bringup/test/test_provision.py index 75b49a8..b5e4aaa 100644 --- a/mote_bringup/test/test_provision.py +++ b/mote_bringup/test/test_provision.py @@ -64,6 +64,18 @@ def test_identity_is_written_before_anything_uses_it(args): assert installs_identity < joins_tailnet +def test_the_environment_is_installed_from_the_lockfile(args): + """`--locked` aborts if pixi.lock is out of date with pixi.toml rather than + solving something new, so a robot provisions the dependency set that was + tested. Without it, a card imaged months later quietly gets a different + one.""" + commands = [c[-1] for c in yaml.safe_load(provision.build(args))["runcmd"]] + install = next(i for i, c in enumerate(commands) if "pixi install" in c) + build = next(i for i, c in enumerate(commands) if "pixi run build" in c) + assert "--locked" in commands[install] + assert install < build + + def test_secrets_survive_substitution_verbatim(args): data = yaml.safe_load(provision.build(args)) keyfile = next( diff --git a/mote_bringup/test/test_tailnet_roles.py b/mote_bringup/test/test_tailnet_roles.py index 8745429..6b830d3 100644 --- a/mote_bringup/test/test_tailnet_roles.py +++ b/mote_bringup/test/test_tailnet_roles.py @@ -2,10 +2,12 @@ A machine is one tailnet node and `tailscale up` replaces the whole tag set, so getting roles wrong doesn't error — it silently drops a tag and leaves the ACLs -that M7 will write against them wrong. The script's `--dry-run` resolves roles, +in `policy.hujson` wrong. The script's `--dry-run` resolves roles, tags and hostname without touching the network, which is what these pin down. """ +import json +import re import subprocess from pathlib import Path @@ -93,3 +95,56 @@ def test_the_auth_key_is_not_echoed(home): result = run(home, "--role", "fleet", "--auth-key", "tskey-auth-SECRET") assert "tskey-auth-SECRET" not in result.stdout assert "--auth-key ***" in result.stdout + + +# ---- the access policy ------------------------------------------------------ + + +POLICY = Path(__file__).resolve().parents[1] / "tailscale" / "policy.hujson" + + +def policy(): + """``policy.hujson`` as data. + + Tailscale's dialect is JSON with `//` comments and trailing commas, which + the stdlib parser refuses, so both are stripped here. This is a syntax + check, not a semantic one: the rules are enforced by Tailscale, and the + `tests` block below is run by Tailscale on every save — it refuses a policy + that fails one. What is checked here is what a paste into the console cannot + tell us: that the file still parses, and that it still names the tags this + repo advertises. + """ + lines = [] + for line in POLICY.read_text().splitlines(): + code = line.split("//", 1)[0] if not line.lstrip().startswith("//") else "" + lines.append(code) + text = "\n".join(lines) + return json.loads(re.sub(r",(\s*[}\]])", r"\1", text)) + + +def test_the_policy_is_parseable(): + assert set(policy()) == {"tagOwners", "hosts", "acls", "ssh", "tests"} + + +def test_every_tag_the_joiner_advertises_is_owned(): + """An `--advertise-tags` on a tag the policy does not declare fails with + "requested tags are invalid or not permitted", which is a joining robot + stopped by a file it never reads.""" + advertised = set(re.findall(r"tag:[a-z]+", SCRIPT.read_text())) + assert advertised + assert advertised <= set(policy()["tagOwners"]) + + +def test_no_rule_lets_one_robot_reach_another(): + """The milestone's criterion, asserted against the rules rather than the + comment beside them: in v1 there is no robot-to-robot anything.""" + for rule in policy()["acls"]: + if "tag:robot" in rule["src"]: + assert not any(dst.startswith("tag:robot") for dst in rule["dst"]) + + +def test_the_policy_asks_tailscale_to_check_that_too(): + """Tailscale evaluates `tests` on every save and refuses a policy failing + one, so the criterion is checked by the thing enforcing it.""" + robot = next(case for case in policy()["tests"] if case["src"] == "tag:robot") + assert {dst for dst in robot["deny"] if dst.startswith("tag:robot:")} diff --git a/mote_fleet/README.md b/mote_fleet/README.md index 7afaac6..bdc405d 100644 --- a/mote_fleet/README.md +++ b/mote_fleet/README.md @@ -103,6 +103,14 @@ dependency than 200 lines that are tested. authorizes an operator token and writes an audit row first. The read path is unchanged and goes straight to the broker. +**Every `/v1` route needs that token**, not only the writes, and one gate in +front of routing takes it — so a route added later is authenticated by default +and an anonymous caller gets `401` rather than a `404` that would say which +routes exist. `fleet_server.ROUTES` is the table it matches against and the table +the tests walk. Four routes are open, each for a stated reason: `/healthz`, the +static UI, enrollment, and the two robot-facing map routes, which carry no +credential because robots have none to carry yet. The broker is still anonymous. + **What is dispatched is a capability and a typed input**, and the server validates neither: the capability that declared the `input_schema` runs on the robot, so a copy on the server would be a second contract to keep in step *and* diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index 8db8d22..a017ac1 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -19,7 +19,7 @@ GET /healthz liveness + how many robots GET /v1/config what the browser needs to bootstrap GET /v1/robots the roster + each robot's presence - GET /v1/robots/ one row + its live state (operator) + GET /v1/robots/ one row + its live state POST /v1/enroll allocate (or return) a robot id POST /v1/robots//dispatch authorize, audit, then publish a mission (capability + typed input) @@ -42,6 +42,9 @@ a new candidate (operator) GET / the operator UI (static files) +The table that dispatches these is ``ROUTES``, below, and it carries what each +one costs: an operator token unless the entry says otherwise. + pixi run fleet-server -- --db ~/fleet/registry.db --broker-host fleet-box **Dispatch is mediated here, and only here.** M1's ``fleetctl`` published @@ -93,15 +96,29 @@ stay under ``/v1/maps``, gated on there being a published map, served to the client that also has the basemap to put them on. -**Security posture for M3:** most read routes are still unauthenticated, exactly -as M1 left them, and the broker is still anonymous. What M3 adds is a credential -on the *write* path and a record of who used it. Two reads take one as well: -the audit log, which nothing else serves, and one robot's live state, whose -payloads the anonymous broker also carries until M7. That stays proportionate only -while the tailnet is the boundary; M7 is where operator auth reaches the rest of -the read routes, per-robot broker credentials land, and Tailscale ACLs stop -robots reaching each other. Until then, do not expose this port to a network the -robots are not already trusted on. +**Every ``/v1`` route needs a credential, and one gate takes it.** Through M3 +only dispatch checked, which left the roster, the basemaps, the audit log and +the broker's address readable by anything that could reach the port. The check +now happens in ``_handle`` in front of route dispatch, against the table above, +so a route added later is authenticated without anyone remembering to +authenticate it and an anonymous caller is refused before the table is consulted +for existence — a 404 would say which routes are real. + +What is open is open for a stated reason rather than by omission. ``/healthz``, +because a liveness probe that needs a secret is a liveness probe nobody wires up. +The static UI — which is not in the table at all, being what an unmatched GET +falls through to — because the page has to load in order to ask for a token, and +it holds no fleet data until it has one. ``POST /v1/enroll``, because it carries +its own credential in the body and an unattended first boot has no human behind +it. And both halves of M4's map exchange, a robot uploading a candidate and a +robot pulling the revision it was told is canonical, because robots have no +credential to present yet; an upload is bounded, audited and inert, and a pull +serves what an operator has already promoted. + +The tailnet (``mote_bringup/tailscale/policy.hujson``) is still the outer +boundary and still what keeps this port off the public internet. What changed is +that it is no longer the *only* boundary. The broker remains anonymous: giving +robots and operators their own broker credentials is its own piece of work. """ import argparse @@ -114,6 +131,7 @@ import traceback import threading import time +import typing import urllib.parse from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path @@ -161,16 +179,140 @@ # test without a package.json declaring the tree a module. mimetypes.add_type("text/javascript", ".mjs") -#: Route shape for the registry's per-revision paths. The three review leaves -#: mirror ``/v1/maps///…`` exactly, because they answer the same -#: three questions about a revision that is *not* the floor's canonical one — -#: which is what an operator has to see before promoting it. -REVISION_RE = re.compile( - r"^(?P[^/]+)/floors/(?P[^/]+)/revisions/(?P[^/]+)" - r"(?P/promote|/bundle\.tar\.gz|/map\.json|/map\.png|/zones\.json)?$" +#: What a revision answers to under review. These three mirror +#: ``/v1/maps///…`` exactly, because they answer the same three +#: questions about a revision that is *not* the floor's canonical one — which is +#: what an operator has to see before promoting it. ``bundle.tar.gz`` is not one +#: of them: it is the robot's pull, and has a route of its own. +REVIEW_LEAVES = ("map.json", "map.png", "zones.json") + + +# -- the route table -------------------------------------------------------- +# +# One table, read by the request gate and walked by the tests. Making it the +# thing that *dispatches* rather than a description beside a chain of `elif`s is +# what keeps "every route is authorized" a property of the code instead of a +# claim about it: a route that is not in the table is not served, and a route in +# the table carries its credential requirement in the same line as its path. + +#: An operator token as a bearer header. The default, and the reason it is the +#: default: a route added later is authenticated without anybody remembering to +#: authenticate it, and has to opt out in a line a reviewer reads. +OPERATOR = "operator" +#: The enrollment token in the request body. A robot is not an operator, and an +#: unattended first boot has no human behind it. +ENROLLMENT = "enrollment" +#: No credential, by M4's decision: an upload names an enrolled robot, is +#: bounded and audited, and is inert until an operator promotes it; a pull serves +#: what an operator has already promoted. Robots have nothing else to present. +ROBOT = "robot" +#: Open to anything that can reach the port, for a stated reason. +OPEN = "open" + + +class Route(typing.NamedTuple): + """One path this server answers, and what it costs to reach it.""" + + method: str + #: Path template. ``{name}`` matches one segment and is passed to the + #: handler as a keyword argument of that name. + template: str + handler: str + auth: str = OPERATOR + #: The handler answers the refusal itself, because it records the attempt + #: in the audit log first — "who tried" is the half of an audit trail a + #: dashboard never shows you. The gate still resolves the token; what it + #: hands over is ``operator=None``. + audits_refusal: bool = False + #: The handler reads the request body itself: an uploaded bundle is neither + #: JSON nor small. + raw_body: bool = False + + +ROUTES = ( + # Open, and each for a reason rather than by omission. /healthz because a + # liveness probe that needs a secret is a liveness probe nobody wires up; + # enrollment because it carries its own credential; the two halves of the + # robot's map exchange per M4 (below). + Route("GET", "/healthz", "_healthz", OPEN), + Route("POST", "/v1/enroll", "_enroll", ENROLLMENT), + Route( + "POST", + "/v1/sites/{site}/floors/{floor}/revisions/{revision}", + "_upload", + ROBOT, + raw_body=True, + ), + # A robot pulling the revision it was just told is canonical. It is the + # other half of the upload above and carries no credential for the same + # reason: robots have none to carry yet. This entry precedes the review + # leaves below so the literal segment wins over ``{leaf}``. + Route( + "GET", + "/v1/sites/{site}/floors/{floor}/revisions/{revision}/bundle.tar.gz", + "_bundle", + ROBOT, + ), + # Everything else needs an operator. + Route("GET", "/v1/config", "_config"), + Route("GET", "/v1/robots", "_roster"), + Route("GET", "/v1/robots/{robot_id}", "_robot"), + Route("GET", "/v1/audit", "_audit"), + Route("GET", "/v1/maps", "_maps"), + Route("GET", "/v1/maps/{site}/{floor}/{leaf}", "_map"), + Route("GET", "/v1/zones", "_zones"), + Route("GET", "/v1/zones/{site}/{floor}", "_vocabulary"), + Route("GET", "/v1/sites", "_sites"), + Route("GET", "/v1/sites/{site}/floors/{floor}", "_floor"), + Route( + "GET", + "/v1/sites/{site}/floors/{floor}/revisions/{revision}/{leaf}", + "_revision", + ), + Route( + "POST", + "/v1/robots/{robot_id}/dispatch", + "_dispatch", + audits_refusal=True, + ), + Route( + "POST", + "/v1/sites/{site}/floors/{floor}/zones", + "_edit_zones", + audits_refusal=True, + ), + Route( + "POST", + "/v1/sites/{site}/floors/{floor}/revisions/{revision}/promote", + "_promote", + audits_refusal=True, + ), ) +def match_route(method: str, path: str): + """``(route, path variables)`` for a request, or ``(None, {})``. + + Matching is on the raw path, never on an unquoted one: ``%2F`` stays inside + a segment, so a name carrying an escaped separator reaches ``_names`` and is + refused there rather than silently becoming two path components. + """ + parts = path.strip("/").split("/") + for route in ROUTES: + shape = route.template.strip("/").split("/") + if route.method != method or len(shape) != len(parts): + continue + variables = {} + for expected, actual in zip(shape, parts): + if expected.startswith("{"): + variables[expected[1:-1]] = actual + elif expected != actual: + break + else: + return route, variables + return None, {} + + class BrokerLink: """The server's own MQTT connection, used for exactly one thing: publishing a command that has already been authorized and recorded. @@ -500,82 +642,119 @@ def _operator(self) -> dict: # -- routes ----------------------------------------------------------- def do_HEAD(self): - self.do_GET() + self._handle("GET") def do_GET(self): - path, _, query = self.path.partition("?") - path = path.rstrip("/") or "/" - params = urllib.parse.parse_qs(query) - - if path == "/healthz": - self._send( - 200, - { - "schema": protocol.SCHEMA, - "ok": True, - "service": "mote-fleet", - "contract": f"{protocol.ROOT}/{protocol.VERSION}", - "version": VERSION, - "robots": len(self.server.registry.robots()), - }, - ) - elif path == "/v1/config": - self._send(200, self.server.ui_config()) - elif path == "/v1/robots": - self._roster() - elif path.startswith("/v1/robots/"): - self._robot(path[len("/v1/robots/") :]) - elif path == "/v1/audit": - self._audit(params) - elif path == "/v1/maps": - self._send( - 200, {"schema": protocol.SCHEMA, "maps": self.server.list_maps()} - ) - elif path.startswith("/v1/maps/"): - self._map(path[len("/v1/maps/") :]) - elif path == "/v1/zones": - self._store(lambda store: {"vocabularies": store.vocabularies()}) - elif path.startswith("/v1/zones/"): - self._vocabulary(path[len("/v1/zones/") :]) - elif path == "/v1/sites": - self._store(lambda store: {"sites": store.sites()}) - elif path.startswith("/v1/sites/"): - self._registry_get(path[len("/v1/sites/") :]) - elif path.startswith("/v1/"): - self._error(404, f"no route {path}") - else: - self._static(path) + self._handle("GET") def do_POST(self): + self._handle("POST") + + def _handle(self, method: str): + """Resolve the route, take the credential, then call the handler. + + The credential is taken **here**, once, in front of every route rather + than inside the handlers that happen to want one. Through M3 only + dispatch checked, which left the roster, the basemaps, the audit log and + the broker's address readable by anything that could reach the port. + Checking here also settles what an unauthenticated caller learns from a + path that does not exist: nothing, because the answer is 401 before the + route table is consulted for existence. + """ raw_path, _, query = self.path.partition("?") path = raw_path.rstrip("/") or "/" - params = urllib.parse.parse_qs(query) - # The upload route carries a packed bundle, so it reads its own body: - # everything else here is JSON and small. - if path.startswith("/v1/sites/") and not ( - path.endswith("/promote") or path.endswith("/zones") - ): - self._upload(path[len("/v1/sites/") :], params) + self.params = urllib.parse.parse_qs(query) + self.body = {} + self.operator = None + self.auth_error = "" + + route, path_vars = match_route(method, path) + if route is None: + # Default deny, and deny *before* answering: an unrouted path under + # /v1 is authenticated first, so a 404 cannot be used to map which + # routes are real. Everything else is the static UI, which is open + # because the page has to load in order to ask for a token. + if path == "/v1" or path.startswith("/v1/"): + if self._authorize() is None: + return + self._error(404, f"no route {path}") + elif method == "GET": + self._static(path) + else: + self._error(404, f"no route {path}") return + + if route.auth == OPERATOR: + self.operator = self._authorize(defer=route.audits_refusal) + if self.operator is None and not route.audits_refusal: + return + + if not route.raw_body: + try: + self.body = self._body() + except ValueError as exc: + self._error(400, str(exc)) + return + + getattr(self, route.handler)(**path_vars) + + def _authorize(self, defer: bool = False): + """The operator behind this request, or None having answered 401. + + ``defer`` leaves the answer to the handler, which is what an audited + route needs: the refusal has to reach the log naming what was attempted, + and only the handler knows that. + """ try: - body = self._body() - except ValueError as exc: - self._error(400, str(exc)) - return - if path == "/v1/enroll": - self._enroll(body) - elif path.startswith("/v1/robots/") and path.endswith("/dispatch"): - self._dispatch(path[len("/v1/robots/") : -len("/dispatch")], body) - elif path.startswith("/v1/sites/") and path.endswith("/promote"): - self._promote(path[len("/v1/sites/") :], body) - elif path.startswith("/v1/sites/") and path.endswith("/zones"): - self._edit_zones(path[len("/v1/sites/") : -len("/zones")], body) - else: - self._error(404, f"no route {path}") + return self._operator() + except Unauthorized as exc: + self.auth_error = str(exc) + if not defer: + self._error(401, str(exc)) + return None + + def _refuse(self, **record) -> None: + """Record an unauthorized attempt on an audited route, then answer it.""" + self.server.registry.record( + actor="anonymous", + result="unauthorized", + detail=self.auth_error, + remote=self.address_string(), + **record, + ) + self._error(401, self.auth_error) + + # -- what the routes answer ------------------------------------------- + + def _healthz(self): + self._send( + 200, + { + "schema": protocol.SCHEMA, + "ok": True, + "service": "mote-fleet", + "contract": f"{protocol.ROOT}/{protocol.VERSION}", + "version": VERSION, + "robots": len(self.server.registry.robots()), + }, + ) + + def _config(self): + self._send(200, self.server.ui_config()) + + def _maps(self): + self._send(200, {"schema": protocol.SCHEMA, "maps": self.server.list_maps()}) + + def _zones(self): + self._store(lambda store: {"vocabularies": store.vocabularies()}) + + def _sites(self): + self._store(lambda store: {"sites": store.sites()}) # -- enrollment ------------------------------------------------------- - def _enroll(self, body: dict): + def _enroll(self): + body = self.body token = (body.get("token") or "").strip() fingerprint = (body.get("fingerprint") or "").strip() requested_id = (body.get("robot_id") or "").strip() @@ -651,7 +830,7 @@ def _roster(self): }, ) - def _robot(self, rest: str): + def _robot(self, robot_id: str): """One robot: its registry row, and the retained state as last seen. This is the whole of what the dashboard's MQTT subscription gets, @@ -660,25 +839,10 @@ def _robot(self, rest: str): transition, and a caller that wants every transition subscribes to the topic the way the browser does. - Behind an operator token where the roster is not, because this is where - the coordinates are — a pose says where in a building the robot is, and - the mission status says what it was told to do there. The token hides - nothing yet: until M7 the same payloads are on the anonymous broker and - every id is in the anonymous roster. It gives the route the shape M7 - will require, so an HTTP client written now already carries the - credential. It is checked before the lookup so that an unauthenticated - answer does not depend on the id, which matters once M7 gates the - roster and not before. + The token is the gate's, like every ``/v1`` route's. It hides less than + it looks: the same payloads are on the broker, which is still anonymous. """ - if "/" in rest: - self._error(404, f"no route /v1/robots/{rest}") - return - try: - self._operator() - except Unauthorized as exc: - self._error(401, str(exc)) - return - robot = self.server.registry.robot(rest) + robot = self.server.registry.robot(robot_id) if robot is None: self._error(404, "no such robot") return @@ -689,13 +853,13 @@ def _robot(self, rest: str): "schema": protocol.SCHEMA, **robot, "broker_connected": state.connected, - **state.of(rest), + **state.of(robot_id), }, ) # -- dispatch + audit ------------------------------------------------- - def _dispatch(self, robot_id: str, body: dict): + def _dispatch(self, robot_id: str): """Authorize, record, publish — in that order. The order is the point. The audit row is written *before* the publish @@ -711,6 +875,7 @@ def _dispatch(self, robot_id: str, body: dict): ``invalid_input`` and says which property, which is an answer an operator can act on and a parser here could not improve. """ + body = self.body registry = self.server.registry remote = self.address_string() capability = str(body.get("capability") or "").strip() @@ -718,22 +883,15 @@ def _dispatch(self, robot_id: str, body: dict): if payload_input is None: payload_input = {} described = _describe(capability, payload_input) - try: - operator = self._operator() - except Unauthorized as exc: - registry.record( - actor="anonymous", + if self.operator is None: + self._refuse( action="dispatch", robot_id=robot_id, command=described[:MAX_COMMAND], - result="unauthorized", - detail=str(exc), - remote=remote, ) - self._error(401, str(exc)) return - actor = operator["name"] + actor = self.operator["name"] if not capability: self._error(400, "a capability is required") return @@ -810,12 +968,8 @@ def _dispatch(self, robot_id: str, body: dict): }, ) - def _audit(self, params: dict): - try: - self._operator() - except Unauthorized as exc: - self._error(401, str(exc)) - return + def _audit(self): + params = self.params try: limit = int((params.get("limit") or ["100"])[0]) except ValueError: @@ -832,13 +986,11 @@ def _audit(self, params: dict): # -- basemaps --------------------------------------------------------- - def _map(self, rest: str): - parts = rest.split("/") + def _map(self, site: str, floor: str, leaf: str): leaves = ("map.json", "map.png", "zones.json") - if len(parts) != 3 or parts[2] not in leaves: + if leaf not in leaves: self._error(404, f"expected /v1/maps///{'|'.join(leaves)}") return - site, floor, leaf = parts if not self._names(site, floor): return if leaf == "zones.json": @@ -880,21 +1032,16 @@ def _send_map(self, leaf: str, load): # -- the zone vocabulary ---------------------------------------------- - def _vocabulary(self, rest: str): + def _vocabulary(self, site: str, floor: str): """``/v1/zones//`` — what places can be named here. Its own prefix rather than another leaf under ``/v1/maps``, because everything under that one is served beside a basemap and gated on there - being one. This is gated on nothing and needs nothing to make sense of, + being one. This needs no published map and nothing to make sense of, so a caller that must never be handed a map — an MCP front door turning "take it to the kitchen" into ``goto kitchen`` — can be given this and only this. """ - parts = rest.split("/") - if len(parts) != 2: - self._error(404, "expected /v1/zones//") - return - site, floor = parts if not self._names(site, floor): return self._store(lambda store: store.read_vocabulary(site, floor)) @@ -922,29 +1069,25 @@ def _store(self, call): return self._send(200, {"schema": protocol.SCHEMA, **payload}) - def _registry_get(self, rest: str): - parts = rest.split("/") - if len(parts) == 3 and parts[1] == "floors": - site, _, floor = parts - if self._names(site, floor): - self._store(lambda store: store.detail(site, floor)) + def _floor(self, site: str, floor: str): + if not self._names(site, floor): return - match = REVISION_RE.match(rest) - # `/promote` is a POST, and a bare revision path has nothing to answer: - # both are 404 here rather than falling through to the bundle. - if not match or match.group("leaf") in (None, "/promote"): - self._error(404, f"no route /v1/sites/{rest}") + self._store(lambda store: store.detail(site, floor)) + + def _revision(self, site: str, floor: str, revision: str, leaf: str): + if leaf not in REVIEW_LEAVES: + self._error(404, f"expected one of {'|'.join(REVIEW_LEAVES)}") return - site, floor = match.group("site"), match.group("floor") - revision = match.group("revision") if not self._names(site, floor, revision): return - leaf = match.group("leaf").lstrip("/") if leaf == "zones.json": self._store(lambda store: store.read_revision_zones(site, floor, revision)) return - if leaf in ("map.json", "map.png"): - self._send_map(leaf, lambda store: store.read_map(site, floor, revision)) + self._send_map(leaf, lambda store: store.read_map(site, floor, revision)) + + def _bundle(self, site: str, floor: str, revision: str): + """The packed revision, for the agent that was told to pull it.""" + if not self._names(site, floor, revision): return try: blob = self.server.store.pack(site, floor, revision) @@ -964,26 +1107,19 @@ def _registry_get(self, rest: str): X_Bundle_Sha256=bundle.digest(blob), ) - def _upload(self, rest: str, params: dict): + def _upload(self, site: str, floor: str, revision: str): """Take one candidate revision from a robot. Deliberately **not** operator-authenticated, and deliberately inert: a candidate changes nothing about any floor until it is promoted, and the route that does change something is the operator's. What is required is that the uploader name an enrolled robot, so the artifact has a subject - in the audit log. M7 replaces that with a per-robot credential. + in the audit log. Giving a robot a credential of its own is the + companion security task's, not this one's. """ - match = REVISION_RE.match(rest) - if not match or match.group("leaf"): - self._error( - 404, "expected POST /v1/sites//floors//revisions/" - ) - return - site, floor = match.group("site"), match.group("floor") - revision = match.group("revision") if not self._names(site, floor, revision): return - robot_id = (params.get("robot_id") or [""])[0] + robot_id = (self.params.get("robot_id") or [""])[0] if not protocol.valid_id(robot_id): self._error(400, "a robot_id query parameter is required") return @@ -1046,7 +1182,7 @@ def _upload(self, rest: str, params: dict): }, ) - def _promote(self, rest: str, body: dict): + def _promote(self, site: str, floor: str, revision: str): """Make a candidate canonical: authorize, flip, announce, record. The flip is the fact and the announcement is best effort — a broker @@ -1054,30 +1190,14 @@ def _promote(self, rest: str, body: dict): plainly whether the fleet was told, and the server re-announces every floor at startup so a missed announcement heals itself. """ - match = REVISION_RE.match(rest) - if not match or match.group("leaf") != "/promote": - self._error(404, "expected POST .../revisions//promote") - return - site, floor = match.group("site"), match.group("floor") - revision = match.group("revision") registry = self.server.registry target = f"{site}/{floor}/{revision}" - try: - operator = self._operator() - except Unauthorized as exc: - registry.record( - actor="anonymous", - action="map.promote", - command=target, - result="unauthorized", - detail=str(exc), - remote=self.address_string(), - ) - self._error(401, str(exc)) + if self.operator is None: + self._refuse(action="map.promote", command=target) return if not self._names(site, floor, revision): return - actor = operator["name"] + actor = self.operator["name"] entry = registry.record( actor=actor, action="map.promote", @@ -1121,7 +1241,7 @@ def _promote(self, rest: str, body: dict): }, ) - def _edit_zones(self, rest: str, body: dict): + def _edit_zones(self, site: str, floor: str): """An operator's zone edit: derive a candidate from a revision. The edit writes nothing the fleet can see — the result is an ordinary @@ -1136,25 +1256,11 @@ def _edit_zones(self, rest: str, body: dict): derivation and never the thing written — the route's own resource is the floor's zones, and the result is a revision id neither end chose. """ - parts = rest.split("/") - if len(parts) != 3 or parts[1] != "floors": - self._error(404, "expected POST /v1/sites//floors//zones") - return - site, _, floor = parts + body = self.body registry = self.server.registry target = f"{site}/{floor}" - try: - operator = self._operator() - except Unauthorized as exc: - registry.record( - actor="anonymous", - action="map.zones", - command=target, - result="unauthorized", - detail=str(exc), - remote=self.address_string(), - ) - self._error(401, str(exc)) + if self.operator is None: + self._refuse(action="map.zones", command=target) return source = str(body.get("revision") or "") if not self._names(site, floor, *([source] if source else [])): @@ -1163,7 +1269,7 @@ def _edit_zones(self, rest: str, body: dict): if not isinstance(zones, dict): self._error(400, "a zones mapping is required: {zones: {name: {...}}}") return - actor = operator["name"] + actor = self.operator["name"] # The audit row names what was edited, not only which floor: two # candidates of one floor are two different maps, and "who renamed the # rooms on this map" is unanswerable from the floor alone. diff --git a/mote_fleet/server/fleetctl.py b/mote_fleet/server/fleetctl.py index d528bd5..9c4122f 100644 --- a/mote_fleet/server/fleetctl.py +++ b/mote_fleet/server/fleetctl.py @@ -32,7 +32,10 @@ arrive, and an exit status. ``watch`` and ``dispatch`` keep the broker, which is what following every transition actually requires. -The token for that lives in ``--token`` or ``$MOTE_FLEET_TOKEN``. +The token for that lives in ``--token`` or ``$MOTE_FLEET_TOKEN``, and every +verb that talks to the API needs it — the roster and the registry are +operator-only too, not only the writes. ``watch`` is the exception, because it +reads the broker rather than the API. ``token``/``operator`` talk to the registry file directly rather than over HTTP, because minting a credential is a thing you do while sitting on the fleet @@ -154,7 +157,7 @@ def cmd_robots(args): if args.robot_id: _print_robot(_get(args.server, f"/v1/robots/{args.robot_id}", _token(args))) return - body = _get(args.server, "/v1/robots") + body = _get(args.server, "/v1/robots", _token(args)) robots = body.get("robots", []) if not robots: print("no robots enrolled") @@ -222,7 +225,7 @@ def cmd_sites(args): """The map registry. Without a floor: what every floor is on. With one: every candidate revision and whether it could be promoted.""" if not (args.site and args.floor): - floors = _get(args.server, "/v1/sites").get("sites", []) + floors = _get(args.server, "/v1/sites", _token(args)).get("sites", []) if not floors: print("no site bundles on the fleet server") return @@ -234,7 +237,9 @@ def cmd_sites(args): f"{(floor['canonical'] or '-'):18} {candidates}" ) return - detail = _get(args.server, f"/v1/sites/{args.site}/floors/{args.floor}") + detail = _get( + args.server, f"/v1/sites/{args.site}/floors/{args.floor}", _token(args) + ) print(f"{args.site}/{args.floor} canonical: {detail['canonical'] or 'none'}") for revision in detail["revisions"]: marker = "*" if revision["canonical"] else " " @@ -517,7 +522,7 @@ def main(argv=None): parser.add_argument( "--token", default="", - help=f"operator token for the write routes (default: ${TOKEN_ENV})", + help=f"operator token for the API (default: ${TOKEN_ENV})", ) sub = parser.add_subparsers(dest="cmd", required=True) diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index d344df0..3c5c59e 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -47,6 +47,7 @@ const state = { robots: new Map(), selected: null, operator: null, + noted: null, // whose capability summary the dispatch note is showing mapKey: null, floor: null, // the registry's view of the floor on screen: revisions, candidates zones: [], // the floor's bound places, for the map and the dispatch picker @@ -57,6 +58,7 @@ let mapView = null; let review = null; let panes = null; let pending = false; +let reader = null; // -- small helpers ------------------------------------------------------- @@ -83,16 +85,37 @@ function robotRecord(robotId) { // -- data in ------------------------------------------------------------- -async function api(path, options = {}) { - const headers = Object.assign({}, options.headers); +function authHeaders(headers = {}) { const token = localStorage.getItem(TOKEN_KEY); - if (token) headers.Authorization = `Bearer ${token}`; + return token ? Object.assign({}, headers, { Authorization: `Bearer ${token}` }) : headers; +} + +async function api(path, options = {}) { + const headers = authHeaders(options.headers); const response = await fetch(path, Object.assign({}, options, { headers })); const body = await response.json().catch(() => ({})); if (!response.ok) throw new Error(body.error || `${response.status} ${response.statusText}`); return body; } +// The basemap is a gated route like the transform beside it, and an `` +// carries no Authorization header — so the pixels are fetched with the token +// and handed to the decoder as a blob. The object URL is released once decoding +// has finished: the bitmap outlives it, the URL does not need to. +async function loadImage(path) { + const response = await fetch(path, { headers: authHeaders() }); + if (!response.ok) throw new Error(`${response.status} ${response.statusText}`); + const objectUrl = URL.createObjectURL(await response.blob()); + try { + const image = new Image(); + image.src = objectUrl; + await image.decode(); + return image; + } finally { + URL.revokeObjectURL(objectUrl); + } +} + async function loadRoster() { const body = await api('/v1/robots'); for (const robot of body.robots) { @@ -159,9 +182,7 @@ async function ensureMap(record) { loadFloor(site, floor, key); try { const meta = await api(`/v1/maps/${site}/${floor}/map.json`); - const image = new Image(); - image.src = meta.image_url; - await image.decode(); + const image = await loadImage(meta.image_url); if (state.mapKey !== key) return; mapView.setMap(meta, image); } catch (error) { @@ -288,7 +309,13 @@ function renderDispatch(record) { ]), ), ); - if (capability) { + // One line carries two things: what the selected capability does, and what + // the last dispatch did. So the summary is written when the *selection* + // changes and not on every render — otherwise an outcome an operator has just + // read is replaced by a description of the form, by whatever arrives next. + const note = `${state.selected}:${capability && capability.key}`; + if (capability && state.noted !== note) { + state.noted = note; dom.dispatchNote.textContent = capability.summary || ''; dom.dispatchNote.className = 'note'; } @@ -346,6 +373,9 @@ function scheduleRender() { } function render() { + // At the gate there is nothing to draw, and the five-second re-render must + // not replace "paste a token" with "no robots" — a different claim. + if (!state.config) return; const records = [...state.robots.values()].sort((a, b) => a.id.localeCompare(b.id)); if (!state.selected && records.length) state.selected = records[0].id; renderRoster(records); @@ -576,32 +606,36 @@ async function onDispatch(event) { async function onToken(event) { event.preventDefault(); - const token = dom.token.value.trim(); - localStorage.setItem(TOKEN_KEY, token); - await checkOperator(); + localStorage.setItem(TOKEN_KEY, dom.token.value.trim()); dom.token.value = ''; + await start(); } -// The one call that tells us whether this token is any good — the audit route -// is operator-only, so a 200 is proof and a 401 is the reason to say so. -async function checkOperator() { - const token = localStorage.getItem(TOKEN_KEY); - if (!token) { - state.operator = null; - dom.operator.textContent = 'read-only — paste an operator token to dispatch'; - dom.operator.className = 'operator anonymous'; - return; - } - try { - await api('/v1/audit?limit=1'); - state.operator = token; - dom.operator.textContent = 'operator token accepted'; - dom.operator.className = 'operator ok'; - } catch (error) { - state.operator = null; - dom.operator.textContent = `token refused: ${error.message}`; - dom.operator.className = 'operator error'; +// There is no read-only mode to fall back to: `/v1/config` is itself +// operator-only, so the page has two states — signed in, or asking to be — and +// this is the asking one. It is not an error. A wall display whose token has +// been revoked looks like this until somebody pastes another in. +function showGate(reason) { + state.operator = null; + state.config = null; + state.robots.clear(); + if (reader) { + reader.close(); + reader = null; } + dom.operator.textContent = reason; + dom.operator.className = 'operator anonymous'; + dom.brokerState.textContent = 'not connected — no operator token'; + dom.brokerState.className = 'broker offline'; + dom.roster.replaceChildren( + el('p', { + class: 'empty', + text: + 'Paste an operator token to see the fleet. Mint one on the fleet box: ' + + 'fleetctl operator new --name ', + }), + ); + renderDetail(null); } // -- boot ---------------------------------------------------------------- @@ -668,6 +702,49 @@ function brokerUrl(config) { return `${scheme}://${host}:${config.broker.ws_port}/`; } +// Everything that needs a credential, in one function: called at load and again +// whenever a token is pasted, so signing in never needs a reload and a token +// that has stopped working puts the page back at the gate rather than into a +// silent half-state. `/v1/config` is the check — it is operator-only like every +// other route, and it is the one the rest of the page is built out of. +async function start() { + if (!localStorage.getItem(TOKEN_KEY)) { + showGate('read-only — paste an operator token'); + return; + } + try { + state.config = await api('/v1/config'); + } catch (error) { + showGate(`token refused: ${error.message}`); + return; + } + state.operator = localStorage.getItem(TOKEN_KEY); + dom.operator.textContent = 'operator token accepted'; + dom.operator.className = 'operator ok'; + document.getElementById('contract').textContent = state.config.contract; + await loadRoster().catch((error) => console.warn('roster unavailable', error)); + // The registry's floors, not the fleet's: a floor worth reviewing may have no + // robot reporting it at all. + await review.loadFloors().catch((error) => console.warn('registry unavailable', error)); + + if (reader) reader.close(); + const { root, presence, health, pose, status, capabilities } = state.config.topics; + reader = new BrokerReader({ + url: brokerUrl(state.config), + topics: [presence, health, pose, status, capabilities].map( + (leaf) => `${root}/+/${leaf}`, + ), + onMessage: onBrokerMessage, + onState: (status_, detail) => { + dom.brokerState.textContent = + status_ === 'connected' ? `broker connected` : `broker ${status_}: ${detail}`; + dom.brokerState.className = `broker ${status_}`; + }, + }); + reader.connect(); + scheduleRender(); +} + export async function boot() { bind(); mapView = new MapView(dom.canvas, { @@ -678,6 +755,7 @@ export async function boot() { }); review = new ReviewView({ api, + loadImage, onPromoted, dom: { canvas: dom.reviewCanvas, @@ -733,28 +811,7 @@ export async function boot() { dom.dispatchSend = dom.dispatch.querySelector('button[type="submit"]'); document.getElementById('token-form').addEventListener('submit', onToken); - state.config = await api('/v1/config'); - document.getElementById('contract').textContent = state.config.contract; - await checkOperator(); - await loadRoster().catch((error) => console.warn('roster unavailable', error)); - // The registry's floors, not the fleet's: a floor worth reviewing may have no - // robot reporting it at all. - await review.loadFloors().catch((error) => console.warn('registry unavailable', error)); - - const { root, presence, health, pose, status, capabilities } = state.config.topics; - const reader = new BrokerReader({ - url: brokerUrl(state.config), - topics: [presence, health, pose, status, capabilities].map( - (leaf) => `${root}/+/${leaf}`, - ), - onMessage: onBrokerMessage, - onState: (status_, detail) => { - dom.brokerState.textContent = - status_ === 'connected' ? `broker connected` : `broker ${status_}: ${detail}`; - dom.brokerState.className = `broker ${status_}`; - }, - }); - reader.connect(); + await start(); // Ages are the only thing on the page that changes without a message. setInterval(scheduleRender, 5000); diff --git a/mote_fleet/server/ui/review.mjs b/mote_fleet/server/ui/review.mjs index 70309cc..84a94bb 100644 --- a/mote_fleet/server/ui/review.mjs +++ b/mote_fleet/server/ui/review.mjs @@ -227,8 +227,11 @@ function el(tag, attributes = {}, children = []) { } export class ReviewView { - constructor({ api, dom, onPromoted = () => {} }) { + constructor({ api, loadImage, dom, onPromoted = () => {} }) { this.api = api; + // Injected for the same reason `api` is: both carry the operator's token, + // and this pane must not hold a second idea of where that lives. + this.loadImage = loadImage; this.dom = dom; this.onPromoted = onPromoted; this.floors = []; @@ -330,9 +333,7 @@ export class ReviewView { this.dom.mapLabel.textContent = `${this.key} · ${revision.revision}`; try { const meta = await this.api(revisionPath(site, floor, revision.revision, 'map.json')); - const image = new Image(); - image.src = meta.image_url; - await image.decode(); + const image = await this.loadImage(meta.image_url); if (epoch !== this.epoch) return; // Two revisions of one floor are compared by switching between them, so // the viewport is kept — unless the maps are different sizes, where diff --git a/mote_fleet/test/api_harness.py b/mote_fleet/test/api_harness.py index 5b3a12c..62d6a2f 100644 --- a/mote_fleet/test/api_harness.py +++ b/mote_fleet/test/api_harness.py @@ -193,26 +193,43 @@ def start_server(tmp_path, **kwargs): thread.start() httpd.url = f"http://127.0.0.1:{httpd.server_address[1]}" httpd.maps = maps + # Every /v1 route needs an operator, so one comes with the harness and + # ``get`` sends it unless a test says otherwise. + httpd.token = httpd.registry.new_operator(name="harness") return httpd # -- speaking to it --------------------------------------------------------- +#: Sent when a call names no token. ``token=""`` means "send none", which is +#: what the tests that are about the *absence* of a credential pass. +DEFAULT = object() -def get(server, path, token=None): - request = urllib.request.Request(server.url + path) + +def _authorize(request, server, token): + token = server.token if token is DEFAULT else token if token: request.add_header("Authorization", f"Bearer {token}") + + +def get(server, path, token=DEFAULT): + request = urllib.request.Request(server.url + path) + _authorize(request, server, token) with urllib.request.urlopen(request, timeout=10) as response: return response.status, json.loads(response.read()) -def get_bytes(server, path): - with urllib.request.urlopen(server.url + path, timeout=10) as response: +def get_bytes(server, path, token=DEFAULT): + request = urllib.request.Request(server.url + path) + _authorize(request, server, token) + with urllib.request.urlopen(request, timeout=10) as response: return response.status, response.headers["Content-Type"], response.read() def post(server, path, payload, token=None): + # Unlike ``get``, this defaults to *no* token: enrollment carries its own + # credential in the body, and every dispatch test is explicit about which + # operator it is acting as. request = urllib.request.Request( server.url + path, data=json.dumps(payload).encode(), diff --git a/mote_fleet/test/browser_check.mjs b/mote_fleet/test/browser_check.mjs index a810ef7..f6e6229 100644 --- a/mote_fleet/test/browser_check.mjs +++ b/mote_fleet/test/browser_check.mjs @@ -592,9 +592,14 @@ try { const label = document.getElementById('review-map-label').textContent; const [floor, revision] = label.split(' · '); const base = '/v1/sites/' + floor.replace('/', '/floors/') + '/revisions/' + revision; + // Every /v1 route needs the operator token, this one included: reading it + // back out of the page's own storage is what the page does. + const headers = { + Authorization: 'Bearer ' + localStorage.getItem('mote.operator.token'), + }; const [map, zones] = await Promise.all([ - fetch(base + '/map.json').then(r => r.json()), - fetch(base + '/zones.json').then(r => r.json()), + fetch(base + '/map.json', { headers }).then(r => r.json()), + fetch(base + '/zones.json', { headers }).then(r => r.json()), ]); const centred = (value, origin) => { const cells = (value - origin) / map.resolution - 0.5; @@ -1048,6 +1053,44 @@ try { writeFileSync(phoneShot, Buffer.from(phonePng.data, 'base64')); console.log(`screenshot: ${phoneShot}`); + // -- the gate ----------------------------------------------------------- + // + // Every /v1 route needs an operator token, `/v1/config` included, so there is + // no read-only mode to fall back to: without a credential the page asks for + // one. Last, because it takes the token away. + if (token) { + await session.evaluate(`localStorage.removeItem('mote.operator.token')`); + await session.send('Page.navigate', { url }); + const gate = await settle( + session, + `JSON.stringify({ + operator: (document.querySelector('.operator') || {}).textContent || '', + roster: (document.getElementById('roster') || {}).textContent || '', + robots: document.querySelectorAll('.robot-id').length, + })`, + (json) => JSON.parse(json).operator.includes('operator token'), + ); + const state = JSON.parse(gate); + check( + 'without a token the page asks for one and shows no fleet', + state.roster.includes('operator token') && state.robots === 0, + gate, + ); + + // And pasting one starts everything, with no reload — which is what makes + // the gate a state of the page rather than an error page. + await session.evaluate(`(() => { + document.getElementById('token').value = '${token}'; + document.getElementById('token-form').requestSubmit(); + })()`); + const signedIn = await settle( + session, + `[...document.querySelectorAll('.robot-id')].map(n => n.textContent).join(',')`, + (ids) => ids.includes('mote-01'), + ); + check('pasting a token signs the page in without a reload', signedIn.includes('mote-01'), signedIn); + } + const errors = await session.evaluate(`window.__errors ? window.__errors.length : 0`); check('no uncaught page errors', errors === 0, String(errors)); } finally { diff --git a/mote_fleet/test/test_e2e_fleet.py b/mote_fleet/test/test_e2e_fleet.py index c26d30f..d866cc1 100644 --- a/mote_fleet/test/test_e2e_fleet.py +++ b/mote_fleet/test/test_e2e_fleet.py @@ -375,7 +375,8 @@ def _on_connect(self, *args): try: assert answered.wait(10), "the broker never answered the feed's CONNECT" assert server.state.connected is False - code, roster = api_get(url, "/v1/robots") + token = server.registry.new_operator(name="e2e") + code, roster = api_get(url, "/v1/robots", token=token) assert code == 200, roster assert roster["broker_connected"] is False finally: @@ -583,7 +584,7 @@ def until(condition, timeout=60.0): # ---- discovery: which robots there are, and which are online ---- state = until(lambda state: (state["presence"] or {}).get("online")) assert state, "presence never reached the API" - code, roster = api_get(fleet_api.url, "/v1/robots") + code, roster = api_get(fleet_api.url, "/v1/robots", token=token) assert code == 200, roster assert roster["broker_connected"] is True row = next(r for r in roster["robots"] if r["robot_id"] == robot_id) diff --git a/mote_fleet/test/test_e2e_map_registry.py b/mote_fleet/test/test_e2e_map_registry.py index f478dfc..e9a7981 100644 --- a/mote_fleet/test/test_e2e_map_registry.py +++ b/mote_fleet/test/test_e2e_map_registry.py @@ -61,6 +61,7 @@ def fleet_api(tmp_path, broker): thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() server.url = f"http://127.0.0.1:{server.server_address[1]}" + server.token = server.registry.new_operator(name="e2e") yield server server.shutdown() server.server_close() @@ -198,10 +199,14 @@ def test_publish_promote_and_pull(tmp_path, monkeypatch, broker, fleet_api, caps def _get(server, path): + """A read of the live server. Every ``/v1`` route needs an operator, so the + harness's own token comes along.""" import json import urllib.request - with urllib.request.urlopen(server.url + path, timeout=10) as response: + request = urllib.request.Request(server.url + path) + request.add_header("Authorization", f"Bearer {server.token}") + with urllib.request.urlopen(request, timeout=10) as response: return json.loads(response.read()) diff --git a/mote_fleet/test/test_fleet_server.py b/mote_fleet/test/test_fleet_server.py index 91cfdc4..e675e9c 100644 --- a/mote_fleet/test/test_fleet_server.py +++ b/mote_fleet/test/test_fleet_server.py @@ -13,9 +13,14 @@ """ import errno +import json import socket +import urllib.error +import urllib.request import pytest + +import fleet_server from api_harness import ( FakeBroker, enroll, @@ -174,15 +179,14 @@ def test_clearing_a_retained_topic_clears_the_field(server, operator, robot): def test_the_route_needs_an_operator_token(server, robot): - body = expect_error(lambda: get(server, f"/v1/robots/{robot}"), 401) + body = expect_error(lambda: get(server, f"/v1/robots/{robot}", token=""), 401) assert "operator token" in body["error"] def test_an_unknown_robot_without_a_token_is_401_not_404(server): - # The unauthenticated answer does not depend on the id. That hides nothing - # while the roster is anonymous; it stops this route leaking ids once M7 - # gates the roster. - expect_error(lambda: get(server, "/v1/robots/mote-99"), 401) + # The unauthenticated answer does not depend on the id, so this route + # cannot be used to find out which ids are enrolled. + expect_error(lambda: get(server, "/v1/robots/mote-99", token=""), 401) def test_the_roster_carries_presence_per_row(server, operator): @@ -470,7 +474,7 @@ def test_a_broker_that_is_down_is_reported_not_swallowed(server, operator, robot def test_the_audit_route_needs_an_operator_token(server, operator, robot): - expect_error(lambda: get(server, "/v1/audit"), 401) + expect_error(lambda: get(server, "/v1/audit", token=""), 401) dispatch(server, robot, token=operator) status, body = get(server, "/v1/audit", token=operator) assert status == 200 @@ -558,3 +562,183 @@ def subscribe(self, topic, qos=0): on_connect(client, None, {}, 0) assert [t for t, _ in client.subscribed] == topics + topics assert {qos for _, qos in client.subscribed} == {protocol.QOS} + + +# ---- the auth gate: every route, walked ------------------------------------ +# +# These read `fleet_server.ROUTES` rather than a list kept here, so a route +# added to the server is covered the day it is added and a route that quietly +# stops requiring a credential fails a test rather than a review. + +#: A value for every path variable the table uses. A route introducing a new +#: one fails here with a KeyError, which is the intended way to be told. +SAMPLES = { + "robot_id": "mote-01", + "site": "home", + "floor": "ground", + "revision": "20260726T120000", + "leaf": "map.json", +} + + +def url_for(route): + return route.template.format(**SAMPLES) + + +def call(server, route, token): + """One request at a route, answered as ``(status, body bytes)``. + + Raw rather than parsed because the table holds a route serving gzip, and a + test of who may reach a route has no business decoding what it serves. + """ + request = urllib.request.Request( + server.url + url_for(route), + data=None if route.method == "GET" else json.dumps({}).encode(), + headers={"Content-Type": "application/json"}, + method=route.method, + ) + if token: + request.add_header("Authorization", f"Bearer {token}") + try: + with urllib.request.urlopen(request, timeout=10) as response: + return response.status, response.read() + except urllib.error.HTTPError as exc: + return exc.code, exc.read() + + +GATED = [r for r in fleet_server.ROUTES if r.auth == fleet_server.OPERATOR] +OPEN = [r for r in fleet_server.ROUTES if r.auth != fleet_server.OPERATOR] + + +#: What the gate says, and the only refusal these tests are about. +REFUSAL = b"operator token" + + +@pytest.mark.parametrize("route", GATED, ids=url_for) +def test_every_gated_route_refuses_an_anonymous_request(server, robot, route): + """M3 put a token on dispatch alone, so the roster, the basemaps, the audit + log and the broker's address were readable by anything that could reach the + port. This is the statement, per route, that they are not.""" + status, body = call(server, route, "") + assert (status, REFUSAL in body) == (401, True) + assert not server.publisher.published + + +@pytest.mark.parametrize("route", GATED, ids=url_for) +def test_every_gated_route_refuses_an_unknown_token(server, robot, route): + assert call(server, route, "not-a-real-token")[0] == 401 + + +@pytest.mark.parametrize("route", GATED, ids=url_for) +def test_every_gated_route_refuses_a_revoked_token(server, robot, route): + token = server.registry.new_operator(name="leaver") + server.registry.revoke_operator(token) + assert call(server, route, token)[0] == 401 + + +@pytest.mark.parametrize("route", OPEN, ids=url_for) +def test_an_open_route_never_asks_for_an_operator_token(server, robot, route): + """The carve-outs, read off the table rather than out of prose: /healthz, + because a liveness probe that needs a secret is one nobody wires up, and the + three robot-facing routes, which carry their own credential (enrollment) or + none at all (M4's map exchange, until robots have one). + + An open route may still refuse the request — enrollment without an + enrollment token does, and an upload with no bundle in it does — so what is + asserted is that it never refuses for want of an *operator*.""" + status, body = call(server, route, "") + assert REFUSAL not in body, f"{route.template} is gated after all" + assert status != 403 + + +def test_the_table_names_which_routes_are_open(server): + """Restated here so that opening a route is a deliberate edit to a test and + not a line nobody reads.""" + assert {(r.method, r.template) for r in OPEN} == { + ("GET", "/healthz"), + ("POST", "/v1/enroll"), + ("POST", "/v1/sites/{site}/floors/{floor}/revisions/{revision}"), + ("GET", "/v1/sites/{site}/floors/{floor}/revisions/{revision}/bundle.tar.gz"), + } + + +def test_every_route_names_a_handler_that_exists(server): + for route in fleet_server.ROUTES: + assert callable(getattr(fleet_server.FleetHandler, route.handler)) + + +def test_authorization_is_checked_before_the_route_exists(server): + """A 404 for an anonymous caller would say which routes are real.""" + expect_error(lambda: get(server, "/v1/nothing", token=""), 401) + expect_error(lambda: post(server, "/v1/nothing", {"schema": protocol.SCHEMA}), 401) + + +def test_a_known_route_with_a_token_is_404_when_it_does_not_exist(server): + expect_error(lambda: get(server, "/v1/nothing"), 404) + + +def test_the_static_ui_stays_open(server): + """The page has to load in order to ask for a token, and it carries no + fleet data until it has one.""" + status, content_type, body = get_bytes(server, "/index.html", token="") + assert status == 200 + assert content_type.startswith("text/html") + assert b"operator token" in body + + +def test_healthz_stays_open(server): + status, body = get(server, "/healthz", token="") + assert (status, body["ok"]) == (200, True) + + +def test_a_robot_pulls_a_bundle_without_a_credential(server, robot, tmp_path): + """The other half of M4's upload: an agent told which revision is canonical + has no operator token, and the alternative to this carve-out is a fleet whose + maps never reach its robots.""" + from api_harness import packed_revision, post_bytes + + revision = "20260727T101500" + post_bytes( + server, + f"/v1/sites/home/floors/ground/revisions/{revision}?robot_id={robot}", + packed_revision(tmp_path), + ) + status, content_type, blob = get_bytes( + server, + f"/v1/sites/home/floors/ground/revisions/{revision}/bundle.tar.gz", + token="", + ) + assert (status, content_type) == (200, "application/gzip") + assert blob + + +# ---- what the route table matches ------------------------------------------ + + +def test_a_path_variable_never_swallows_a_separator(): + """Matching is on the raw path, so an escaped separator stays inside one + segment and is refused by name validation rather than becoming two + components.""" + route, variables = fleet_server.match_route("GET", "/v1/zones/..%2F..%2Fetc/ground") + assert route.handler == "_vocabulary" + assert variables["site"] == "..%2F..%2Fetc" + + +def test_a_literal_segment_wins_over_a_variable_one(): + """``bundle.tar.gz`` is the robot's route and the review leaves are the + operator's; they differ in one segment and in what they cost to reach.""" + pull, _ = fleet_server.match_route( + "GET", "/v1/sites/home/floors/ground/revisions/r1/bundle.tar.gz" + ) + review, _ = fleet_server.match_route( + "GET", "/v1/sites/home/floors/ground/revisions/r1/map.png" + ) + assert (pull.handler, pull.auth) == ("_bundle", fleet_server.ROBOT) + assert (review.handler, review.auth) == ("_revision", fleet_server.OPERATOR) + + +def test_a_method_is_part_of_the_match(): + """The same path uploads under POST and is not readable under GET.""" + path = "/v1/sites/home/floors/ground/revisions/r1" + assert fleet_server.match_route("POST", path)[0].handler == "_upload" + assert fleet_server.match_route("GET", path)[0] is None