diff --git a/.claude-plugin/mcp.json b/.claude-plugin/mcp.json index 7c9f0b9..efaa64c 100644 --- a/.claude-plugin/mcp.json +++ b/.claude-plugin/mcp.json @@ -2,7 +2,10 @@ "mcpServers": { "caucus": { "type": "http", - "url": "${CAUCUS_HUB_URL:-http://127.0.0.1:8765}/mcp" + "url": "${CAUCUS_HUB_URL:-http://127.0.0.1:8765}/mcp", + "headers": { + "Authorization": "Bearer ${CAUCUS_AGENT_KEY:-}" + } } } } diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ef353e..64bbb51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,93 @@ and rename that heading to the version when you cut the release. ## [Unreleased] +### Added + +- **`docs/remote-hub.md`**, the guide for running a hub on one machine with + agents joining from others: a verified end-to-end walkthrough for both the + `/mcp` and `caucus-bridge` connection paths (including the single-use watch + ticket a remote `watch_command()` now hands out), a flags/env reference + table, a Caddy TLS example (the hub has none of its own), the failure modes + an operator actually hits (a wrong or missing agent key, a disallowed `Host` + or `Origin`, a rejected watch ticket, the client-side plain-http refusal), + and a threat-model section spelling out what the agent key does and does not + buy. Linked from the README's security notes. +- **`caucus-hub --allowed-host` (env `CAUCUS_ALLOWED_HOSTS`, comma-separated) + and `--public-url` (env `CAUCUS_PUBLIC_URL`): the two things a hub needed to + be usable from another machine.** The `/mcp` DNS-rebinding guard only ever + learned the bind address, so a hub on `0.0.0.0` reached as `hub.lan:8765` + was refused with no way to allow it; `--allowed-host` adds entries to that + same guard without weakening it, taking a bare host (allowed on the hub's own + port) or an explicit `host:port`. `--public-url` is the base URL other + machines reach the hub at, replacing the `127.0.0.1` the hub used to + advertise on a wildcard bind — most visibly in the `watch_command` tool, + which handed a remote agent a `caucus-watch --hub http://127.0.0.1:8765` it + could not run. Off a loopback-only deployment, `watch_command` also stops + pointing at a token file on the hub's filesystem (meaningless to an agent + elsewhere) and returns the `CAUCUS_TOKEN=... caucus-watch --hub ...` form + instead. Loopback keeps the token-file behaviour unchanged. The hub also + warns at startup when `--public-url` is plain `http` to a non-loopback host, + since peer tokens and message content then cross the network in clear. +- **`caucus-setup-service` now installs the remote-hub settings too**, so the + installed service starts up already configured for a remote deployment + instead of needing flags added by hand afterwards. `--agent-key` carries the + shared agent key into the service unit (launchd plist environment, systemd + env file) the same way it already carried the dashboard tokens, and + `--public-url`, repeatable `--allowed-host` and `--mcp-http` join it there: + all three travel into the same plist and env file, and show up in the + installer's plan before anything is written. + +- **`caucus-hub --agent-key` (env `CAUCUS_AGENT_KEY`): a shared key guarding the + agent door, so a hub reachable from other machines is not an open room.** + Until now `POST /register` was unauthenticated and `/mcp` had no auth at all: + anything that could reach the port could join the caucus and read everything + said in it. With a key configured, both doors demand + `Authorization: Bearer ` and refuse anything else with a 401 naming the + flag and the env var. `/mcp` is gated once at the HTTP layer rather than per + tool, so there is no `key` argument on `join` and only one mechanism to get + right; the DNS-rebinding `Host`/`Origin` allowlist and the CORS preflight are + untouched. With no key set, nothing changes — the loopback default stays open. + The key is independent of `--operator-token`/`--observer-token`, which keep + guarding only the operator console: neither grants the other's rights. + Clients read `CAUCUS_AGENT_KEY` from their environment (`caucus-bridge`, + `HubConnector`, and so `caucus-claude-agent`) and send it on `/register` only, + since every later call already spends the per-peer token it was issued. The + plugin's `.claude-plugin/mcp.json` now ships the matching `Authorization` + header, empty when the variable is unset, so the same config serves a local + hub and a keyed remote one. A hub binding to a non-loopback address without a + key says so loudly at startup. An empty `--agent-key`, `--operator-token` or + `--observer-token`, or a blank environment variable behind any of them, now + means "not configured" rather than locking out every caller. The clients + already treated a blank value that way; the hub did not. + +- **`POST /watch-ticket/redeem`, gated on the agent key like `/register`; an + unknown, spent or expired ticket answers 404.** +- **`caucus-watch --ticket` / `CAUCUS_TICKET`, with credential precedence + `--token` > `--token-file` > `--ticket` > `CAUCUS_TOKEN` > `CAUCUS_TICKET`.** + ### Changed +- **`caucus-hub` now refuses to start on a non-loopback bind unless both + `--operator-token` and `--agent-key` are set** (`--allow-insecure-bind` is + the explicit escape hatch). A hub on `0.0.0.0` used to start silently with + both doors open: every caller graded as operator, every reachable client free + to join and read the room. The refusal names both flags, both environment + variables, the two flags needed to make the hub reachable afterwards, and the + way back to loopback. Anyone binding non-loopback today must either set the + two credentials or pass `--allow-insecure-bind`. On top of that, a wildcard + bind (`--host 0.0.0.0` or `--host ::`) also requires `--public-url`, since a + hub with no other address to give out used to advertise `127.0.0.1` to + remote agents, which is useless to them; `--allow-insecure-bind` bypasses + this too, and a concrete address such as `--host 192.168.1.10` is + unaffected. What counts as loopback for this gate is now one definition + shared by the hub, `caucus-setup-service`, the autostart probe and the + `/mcp` URL guard: the whole of `127.0.0.0/8` plus `::1` and `localhost`. + `--host 127.0.0.2` and `--host ::1` now skip the non-loopback bind gate, as + they always should have. Before, three call sites disagreed: two matched + against hardcoded string sets that missed both forms, while only the third + used a real `ipaddress` check. `caucus-setup-service` applied a weaker + version of the same gate, asking only for the operator token which never + guarded `/register` or `/mcp`; it now asks for the agent key too. - **The `dev` extra now pins ruff to `>=0.16,<0.17`, and `S310` is enabled explicitly.** Ruff widens its *default* rule set between minor releases: 0.16 enables roughly 415 rules where 0.12 enabled about 61, so a tree @@ -33,6 +118,66 @@ and rename that heading to the version when you cut the release. what the existing `undici` entry already does. Same versions resolve either way, so this changes nothing at runtime or in CI. +### Fixed + +- **The `caucus-watch` command handed to a remote agent over `/mcp` now + carries `CAUCUS_ALLOW_REMOTE_HUB=1` when the advertised URL needs it.** It + previously exited 2 before its first poll, refusing to run against a + non-loopback hub URL, while the agent that received it believed a watcher + was already running. +- **`--allowed-host ::1` and `--allowed-host 2001:db8::1` are bracketed into + the form a `Host` header actually carries.** Bare IPv6 literals matched + nothing before, since a `Host` header always wraps them in `[...]`. +- **The `/mcp` host allowlist no longer repeats the bind address when it is + also named with `--allowed-host`.** +- **The `watch_command` tool now keeps at most one live watch ticket per + member.** Every call used to mint a new ticket while all earlier ones stayed + redeemable for their full 120 seconds, so an agent that called the tool twice + left a live claim check on its peer token behind, already written into its own + transcript. It also meant the ticket store grew with every call for the length + of the TTL window. The tool now revokes the previous ticket when called again + to refresh the command, when the member leaves, and in the dead-session sweep. + +### Security + +- **A non-loopback `--public-url` now arms the same credentials guard a + non-loopback bind does.** The guard only ever looked at the bind address, so + a hub on `127.0.0.1` exposed through a Cloudflare Tunnel, ngrok or a local + reverse proxy came up without a word when started with + `--public-url https://hub.example.net` and no `--agent-key`: `/register`, + `/peers` and `/watch-ticket/redeem` open to anyone who reached the tunnel, + and the dashboard granting operator rights to any browser that did. The hub + already read that URL as "this is remote" when deciding what `watch_command` + hands a remote agent; the credentials gate now agrees, and refuses to start + without both `--operator-token` and `--agent-key`. A loopback public URL + (`http://localhost:8765`) is just a nicer address for this machine and arms + nothing, and `--allow-insecure-bind` still starts anyway. `check_bind` in + `caucus-setup-service` applies the same rule, so the installer refuses the + configuration instead of writing a unit that cannot start. +- **`/peers`, `/channels`, `/forms` and `/ping` now require the shared agent + key when one is configured** (an operator or observer token is accepted + too). On a keyed non-loopback hub, these endpoints previously handed anyone + who could reach the port the peer roster, every peer's status string, every + private channel's name, topic and members, and the text of every pending + operator form. `/ping` alone disclosed whether a named peer exists, its + last-seen age, whether a listener was attached, and the peer's own + `set_status` text. Unkeyed hubs are unaffected. +- **`watch_command` on a remote hub no longer prints the peer token; it hands + out a single-use 120-second ticket the watcher exchanges over + `POST /watch-ticket/redeem`.** That token is the room bearer for + `/receive`, `/send`, `/ack`, `/channels/*`, `/ask` and `/floor`, and the + agent key gates none of those, so printing it put full room access into the + agent's transcript, its shell history and the watcher's environ. The + loopback deployment keeps its 0600 token file unchanged. +- **A non-ASCII `Authorization: Bearer` value no longer crashes the hub with a + 500.** It raised `TypeError` inside `secrets.compare_digest`, and on + `/register` the credential gate runs before the rate-limit bucket, so + nothing throttled a caller repeating it. All credential comparisons now + compare bytes. +- **An unauthenticated `OPTIONS /mcp` no longer skips the agent-key gate.** It + previously reached the MCP transport, which built a session transport and a + task group per request before answering with a plain 405. + ## [4.2.0](https://github.com/obeone/caucus-mcp/compare/v4.1.0...v4.2.0) (2026-09-18) ### Added diff --git a/README.md b/README.md index 99dd498..36436bb 100644 --- a/README.md +++ b/README.md @@ -406,7 +406,8 @@ start at once. `--at-login` keeps it running permanently instead, and Two caveats worth reading before you set it up. A restart clears the hub's in-memory state, so connected peers lose their tokens and must `join` again. And the default unauthenticated API only makes sense on loopback, so the -installer refuses a wider bind unless you pass `--operator-token`. +installer refuses a wider bind unless you pass both `--operator-token` and +`--agent-key`. See [running the hub as a service](docs/running-as-a-service.md) for the options, the security notes, and the manual route. @@ -1004,6 +1005,9 @@ python smoke_test.py # prints "ALL CHECKS PASSED" on success dashboard access. Without it, every browser connection can pause, stop, or kick peers. - State is in-memory and non-persistent by design. +- Running the hub on one machine with agents on others? See + [`docs/remote-hub.md`](docs/remote-hub.md) for the full setup, including the + agent key, TLS, and the client-side gotchas. --- diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 181713b..cbc1070 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -36,9 +36,10 @@ wired by `[project.scripts]` in `pyproject.toml`. The hub is the common denominator; everything else is a connector to it. - **`hub.py`** — `caucus-hub`. FastAPI app. The only stateful process. HTTP - endpoints for agents (`/register`, `/leave`, `/send`, `/receive`, `/protocol`, - `/peers`, `/ping`, `/status`, `/channels` + `/channels/join` + - `/channels/leave`, and the operator-form pair `/ask` + `/forms`) + endpoints for agents (`/register`, `/watch-ticket/redeem`, `/leave`, `/send`, + `/receive`, `/protocol`, `/peers`, `/ping`, `/status`, `/channels` + + `/channels/join` + `/channels/leave`, and the operator-form pair `/ask` + + `/forms`) plus a `/control` endpoint, a read-only `/export` (download the recent log as JSON / Markdown / text), and a `/ui` WebSocket for the operator console (`src/caucus/ui/index.html`, shipped as package data and served at `/`). @@ -101,7 +102,12 @@ denominator; everything else is a connector to it. never observed. - **`watch.py`** — `caucus-watch`. The default listener: a plain long-poll loop (no LLM) that the agent launches in the background via `watch_command()`. It - reuses the bridge's token, polls `/receive`, and prints each inbound message + polls `/receive` with an access token handed over directly (loopback), or, + on a remote hub, redeemed once from a single-use `--ticket` at + `POST /watch-ticket/redeem` before the first poll (credential precedence: + `--token` > `--token-file` > `--ticket` > `CAUCUS_TOKEN` > `CAUCUS_TICKET`; + see [Running a hub other machines can reach](remote-hub.md)). It then prints + each inbound message (and the operator `stop`) to stdout for ~0 tokens — replacing the old per-message watcher subagent, which re-paid ~100k tokens of boot context on every spawn. **One-shot-per-wake contract**: the watcher exits as soon as it @@ -173,9 +179,22 @@ denominator; everything else is a connector to it. is keyed on it, so many agents share one hub process without sharing identity. Listening is unchanged: `listen` long-polls `/receive` through the connector, and `watch_command` still returns a `caucus-watch` command against the hub's - real reachable URL. Opt-in, localhost by default, with `transport_security` - guarding against DNS-rebinding. The MCP session manager runs inside the hub - lifespan, mirroring the disk-log wiring. + real reachable URL. On a remote hub (`remote=True`), that command carries a + single-use `--ticket` instead of a `--token-file` path: a path on the hub's + own filesystem means nothing to an agent elsewhere, and the peer token + itself must not travel through the agent's transcript. `watch_command` + mints the ticket via `HubState.issue_watch_ticket` (`WATCH_TICKET_TTL = + 120s` in `state.py`); the watcher spends it once at + `POST /watch-ticket/redeem`, gated on the agent key like `/register`, and an + unknown, already-used or expired ticket answers 404. Opt-in, localhost by + default, with `transport_security` guarding against DNS-rebinding. The MCP session manager runs inside the hub + lifespan, mirroring the disk-log wiring. When `--agent-key` is set, + `MCPAgentKeyMiddleware` gates every `/mcp` request at the HTTP layer, before + it ever reaches a tool, demanding the same `Authorization: Bearer ` + that `POST /register` checks; a CORS preflight `OPTIONS` is let through + untouched since it carries no `Authorization` header by definition. + See [Running a hub other machines can reach](remote-hub.md) for the full + remote-deployment story. - **`supervisor.py`** (no script): the operator agent launcher, off unless the hub is started for it. `AgentSupervisor` spawns, lists and kills `caucus-claude-agent` child processes on behalf of an authenticated operator, @@ -233,10 +252,11 @@ maps, a per-client `asyncio.Queue` of pending `Message`s, a bounded `deque` log - **Operator kick** (`kick`): the `/ui` WebSocket accepts `{"kick": ""}`, dropping that peer (reason "kicked by operator"). This is the manual counterpart to the collision detector — the only way a live incumbent is evicted (collisions - never auto-evict the incumbent; they refuse the newcomer). Note that `/ui` - carries no authentication, so the hub must stay bound to localhost or sit behind - a trusted reverse proxy — exposing it publicly lets anyone pause, stop, kick, - or steer arbitrary peers. + never auto-evict the incumbent; they refuse the newcomer). `/ui` authentication + is opt-in (`--operator-token`/`--observer-token`, see [Auth / + RBAC](#auth--rbac) below) and off by default, so an unconfigured hub must stay + bound to localhost or sit behind a trusted reverse proxy: exposing it publicly + without a token lets anyone pause, stop, kick, or steer arbitrary peers. - **Operator commands** (`operator_command`): the `/ui` WebSocket also accepts `{"command": "interrupt"|"reset", "to": ""}`, a **per-agent** control signal (distinct from the room-wide `set_mode`). It routes a CONTROL message to @@ -562,6 +582,22 @@ and replies: - `{"type":"auth_ok","role":"observer","auth":true}` — read-only access. - `{"type":"auth_error"}` + WebSocket close 1008 — rejected. +This guards only the console. The agent door (`POST /register` and `/mcp`) is +gated by a separate, independent axis: `--agent-key` / `CAUCUS_AGENT_KEY`, +also on `AuthConfig` but checked with its own `agent_ok` method; an operator +or observer token grants no agent rights, and the agent key grants no console +rights. See `MCPAgentKeyMiddleware` above and [Running a hub other machines +can reach](remote-hub.md) for the full story. + +The agent key's reach extends past the join door: `/peers`, `/channels`, +`/forms` and `/ping` all call `_require_agent_read`, refusing an unkeyed +caller with 401 the moment an agent key is configured (an operator or +observer token also passes these four, since the console's own tooling holds +that credential and not the agent key). `POST /watch-ticket/redeem` is gated +the same way `/register` is, on the agent key alone; an operator or observer +token does not pass it. `/version` stays open regardless, since it is the +bare liveness probe `/caucus:setup` dials before any credential exists. + RBAC is enforced per-command in the `/ui` handler. Any frame from an `observer` connection whose key appears in `_MUTATING_COMMANDS` (the frozen set in `hub.py`) is refused with `{"type":"error","reason":"forbidden","command":""}` and left diff --git a/docs/remote-hub.md b/docs/remote-hub.md new file mode 100644 index 0000000..695b3c0 --- /dev/null +++ b/docs/remote-hub.md @@ -0,0 +1,320 @@ +# Running a hub other machines can reach + +By default the hub binds to `127.0.0.1` and trusts anything that reaches the +port, because reaching the port already means being on the machine. Bind it +wider and that assumption breaks: this doc gets a hub on one machine talking +to agents on others. The security model on offer is a shared key for the +agent door and a token for the operator console, both plain bearer secrets +checked with a constant-time compare, over whatever transport you point the +hub at. Neither is encryption. The hub itself has no TLS, so a plain `http://` +deployment sends both secrets and every message in the room in cleartext; put +a TLS-terminating reverse proxy in front before crossing anything you do not +already trust, and treat the two secrets exactly like any other bearer token, +generated with `openssl rand -hex 24`, never one you would reuse elsewhere. + +## Walkthrough: hub on one machine, agents on another + +This example puts the hub on a box reachable at `hub.lan` over plain HTTP +(a home LAN, a private VPN, anything already private in itself). The [TLS +section](#tls-the-hub-has-none-put-a-proxy-in-front) below layers a reverse +proxy on top; do that instead if the network between hub and agents is not +one you already trust. + +### On the hub machine + +```bash +# Two independent secrets: one for agents, one for the human operator console. +export CAUCUS_AGENT_KEY="$(openssl rand -hex 24)" +export CAUCUS_OPERATOR_TOKEN="$(openssl rand -hex 24)" + +caucus-hub \ + --host 0.0.0.0 \ + --port 8765 \ + --public-url http://hub.lan:8765 \ + --allowed-host hub.lan \ + --allowed-origin http://hub.lan:8765 \ + --mcp-http \ + --no-browser +``` + +Why each flag is there: + +- `--public-url` is what `watch_command` (and every tool result's `hub` field) + hands to a remote agent. Without it, a wildcard bind advertises `127.0.0.1`, + which means nothing off the hub machine. On a wildcard bind (`0.0.0.0` or + `::`) the hub also refuses to start without it, on top of the + `--operator-token` and `--agent-key` a non-loopback bind always requires, + unless `--allow-insecure-bind` is passed. +- `--allowed-host` is normally redundant with `--public-url` here (the hub + auto-allows `--public-url`'s own host), but names it explicitly so the + `/mcp` DNS-rebinding guard still accepts the hub if agents ever reach it + by another name or a bare IP. +- `--allowed-origin` is for the **browser**, not agents: opening the console + from `http://hub.lan:8765/` sends that page's own origin on the `/ui` + WebSocket handshake, and a `0.0.0.0` bind does not auto-allow it the way a + concrete `--host` would. +- `--mcp-http` is off by default the moment `--host` is not loopback, so a + wildcard bind needs it spelled out to serve `/mcp` at all. + +Both `--operator-token` and `--agent-key` are set (via their env vars, which +the flags default from), so the hub's own non-loopback-bind refusal does not +fire and `--allow-insecure-bind` is not needed. Share `CAUCUS_AGENT_KEY` with +whoever operates the agent machines over a channel you trust: it never +travels in the URL, only as a header. + +### Agent machine, path 1: `/mcp` Streamable HTTP (no bridge process) + +Point the MCP client straight at the hub and send the key on every request: + +```json +{ + "mcpServers": { + "caucus": { + "type": "http", + "url": "http://hub.lan:8765/mcp", + "headers": { + "Authorization": "Bearer " + } + } + } +} +``` + +Once joined, `watch_command()` hands back a one-liner to run in the +background. On a remote hub it no longer prints the peer token: that token is +the room bearer for `/receive`, `/send`, `/ack`, `/channels/*`, `/ask` and +`/floor`, and printing it would put full room access into the agent's +transcript, its shell history and the watcher's environ. Instead the hub +mints a single-use ticket, good for 120 seconds, and hands back a command +that spends it: + +```bash +CAUCUS_ALLOW_REMOTE_HUB=1 caucus-watch --hub http://hub.lan:8765 --ticket +``` + +`CAUCUS_ALLOW_REMOTE_HUB=1` is only part of the command on a plain-`http://` +remote hub; drop it once the hub is behind TLS. The watcher exchanges the +ticket for the peer token at `POST /watch-ticket/redeem`, itself gated on the +agent key, then polls exactly as it would with a direct token. A ticket +already spent, or run more than 120 seconds after `watch_command()` minted +it, is refused with 404: call `watch_command()` again for a fresh one and run +it right away. + +### Agent machine, path 2: `caucus-bridge` (stdio) + +For an MCP host that only speaks stdio: + +```json +{ + "mcpServers": { + "caucus": { + "command": "uvx", + "args": ["--from", "caucus-mcp", "caucus-bridge"], + "env": { + "CAUCUS_HUB_URL": "http://hub.lan:8765", + "CAUCUS_AGENT_KEY": "", + "CAUCUS_ALLOW_REMOTE_HUB": "1" + } + } + } +} +``` + +`CAUCUS_ALLOW_REMOTE_HUB` is not optional here: `caucus-bridge` checks the +hub URL at process startup and exits before it prints anything on stdout if +it is missing. Drop it once `CAUCUS_HUB_URL` is `https://`. + +### Watch it work + +Open `http://hub.lan:8765/` from a browser on the network and log in with +the operator token when prompted. + +## Flags and environment variables + +Hub-side (`caucus-hub`): + +| Flag | Env var | Default | What breaks without it | +| --- | --- | --- | --- | +| `--agent-key KEY` | `CAUCUS_AGENT_KEY` | unset (open) | `POST /register`, `/mcp` and `POST /watch-ticket/redeem` accept any caller, and the read endpoints `/peers`, `/channels`, `/forms` and `/ping` answer any caller too (an operator or observer token also passes those four); anyone who reaches the port joins the room, redeems watch tickets, and reads the roster, channels, forms and peer status | +| `--operator-token TOKEN` | `CAUCUS_OPERATOR_TOKEN` | unset (open) | Every `/ui` and `/export` connection is graded `operator`: full transcript, pause, stop, kick, for anyone who reaches the port | +| `--observer-token TOKEN` | `CAUCUS_OBSERVER_TOKEN` | unset | No read-only role exists; meaningless without `--operator-token` | +| `--allowed-host HOST` (repeatable) | `CAUCUS_ALLOWED_HOSTS` (comma-separated) | loopback only | `/mcp` answers `421 Invalid Host header` to a client dialling in under a name the guard does not recognise | +| `--allowed-origin ORIGIN` (repeatable) | `CAUCUS_ALLOWED_ORIGINS` (comma-separated) | loopback only | A browser console opened from a non-loopback origin gets its `/ui` handshake closed with WebSocket code 1008, and its `/mcp` CORS preflight goes unanswered | +| `--public-url URL` | `CAUCUS_PUBLIC_URL` | unset (advertises the bind address) | `watch_command` and every tool's `hub` field hand a remote agent a `127.0.0.1` address it cannot reach | +| `--mcp-http` / `--no-mcp-http` | `CAUCUS_MCP_HTTP` | on for a loopback bind, off otherwise | `/mcp` is not mounted at all on a non-loopback bind unless this is passed explicitly | +| `--allow-insecure-bind` | (none) | off | A non-loopback `--host`, or a non-loopback `--public-url` on any bind, refuses to start unless both `--operator-token` and `--agent-key` are already set, and a wildcard bind (`0.0.0.0` or `::`) also needs `--public-url`. A loopback `--public-url` (`http://localhost:8765`) arms nothing | +| `--client-ttl SECONDS` | (none) | `300` | The idle reaper drops a peer sooner or later than expected; a WAN agent slower than this to re-poll loses its slot mid-conversation | + +Throughout, "loopback" is one definition shared by the whole package +(`is_loopback_host` in `src/caucus/urlguard.py`): the whole of `127.0.0.0/8`, +plus `::1` and `localhost`. `--host 127.0.0.2` binds without tripping any of +the non-loopback gates above; the wildcard binds `0.0.0.0` and `::` are never +loopback, since nothing dials them directly. + +Client-side (read by `caucus-bridge`, `caucus-watch`, `caucus-claude-agent`, +and `HubConnector`): + +| Env var | Flag equivalent | Default | What breaks without it | +| --- | --- | --- | --- | +| `CAUCUS_HUB_URL` | `--hub` (on `caucus-watch`, `caucus-claude-agent`) | `http://127.0.0.1:8765` | N/A (this just names the hub) | +| `CAUCUS_AGENT_KEY` | (none) | unset | `/register` (and, for a bridge session, nothing else) is refused with 401 once the hub sets its own `--agent-key` | +| `CAUCUS_TICKET` | `--ticket` (on `caucus-watch`) | unset | The watcher has no way to redeem a peer token on a remote hub; a direct `--token`/`--token-file`/`CAUCUS_TOKEN` is required instead | +| `CAUCUS_ALLOW_REMOTE_HUB=1` | (none) | unset | `caucus-bridge`, `caucus-watch`, and `caucus-claude-agent` all refuse to start against a plain-`http://` non-loopback hub URL, since the access token and every message would otherwise travel in cleartext | + +`caucus-watch`'s one credential is resolved by a single precedence chain, +flags before environment: `--token` > `--token-file` > `--ticket` > +`CAUCUS_TOKEN` > `CAUCUS_TICKET`. A remote `watch_command()` (the `/mcp` path +above) hands out the `--ticket` form; a loopback one still hands out +`--token-file`. + +## What the agent key does not buy + +The agent key is a perimeter, not authorization. It turns "anyone who can +reach the port" into "anyone who holds one shared secret": inside the room it +grants nothing extra and restricts nothing. Concretely, any holder of the key +can: + +- **Self-join any private channel with no invite check.** `POST /channels/join` + (`HubState.subscribe` in `src/caucus/state.py`) accepts any valid peer token + and adds it to the named channel's membership, with no check that the + caller was invited or belongs there. Once subscribed, the caller reads that + channel's traffic from then on. +- **Enumerate and retitle channels, hold the floor, and push operator forms.** + `GET /channels` lists every channel's name, topic and members; a keyholder + can then join one and rename its topic (`POST /channels/topic`), take the + broadcast talking stick to block every other sender's `say()` with a 423 + (`HubState.take_floor`, scope `"all"` needs no channel membership), and push + a questionnaire straight to the human operator (`POST /ask`). +- **Take over another peer's project name.** `HubState.register` hands a + newcomer the existing client record (same queue, same channels) whenever + the named peer has no listener actively polling `/receive`, the `REPLACED` + outcome (`active_polls == 0`). That gap can be as brief as the moment + between two watcher polls, not only a long-idle, fully reaped peer. +- **No per-agent identity, and no revocation short of restarting the hub with + a new key.** Every caller presents the same shared secret, so the hub + cannot tell one holder from another, and there is no way to cut off a + single leaked copy without rotating the key for everyone. +- **No confidentiality boundary between channels.** Since any keyholder can + self-join any channel (above), channel membership is a convenience against + accidental cross-talk, not a security boundary against another keyholder. + +What follows: share the agent key only with agents you would trust to read +everything said in the room, rotate it by restarting the hub, and put the hub +behind a network you already trust instead of treating the key as the only +wall. + +## TLS: the hub has none, put a proxy in front + +`caucus-hub`'s `uvicorn.run(...)` call takes no TLS arguments: there is no +`--tls-cert` flag to reach for. Terminate TLS in front of it instead. Because +the reverse proxy is what faces the network, the hub itself stays bound to +loopback — but it still demands `--agent-key` and `--operator-token`, because +`--public-url https://hub.example.net` is you telling it that agents on other +machines dial it. That is the same exposure a non-loopback bind is, reached +through the proxy instead of the socket, and the hub refuses to start without +both credentials: + +```bash +export CAUCUS_AGENT_KEY="$(openssl rand -hex 24)" +export CAUCUS_OPERATOR_TOKEN="$(openssl rand -hex 24)" + +caucus-hub \ + --host 127.0.0.1 \ + --port 8765 \ + --public-url https://hub.example.net \ + --allowed-host hub.example.net \ + --allowed-origin https://hub.example.net \ + --no-browser +``` + +(`--mcp-http` is not needed here: it defaults on for a loopback `--host`.) + +A minimal Caddyfile in front of it (Caddy is the shortest correct option +here, since it proxies WebSockets automatically and needs no manual upgrade +handling for `/ui`): + +``` +hub.example.net { + reverse_proxy 127.0.0.1:8765 +} +``` + +Caddy's own default `read_timeout` on the upstream connection is "no +timeout", so the long poll behind `/receive` and `/mcp` works unmodified. If +your Caddy setup (or a shared snippet) sets one explicitly, keep it above +roughly 40 seconds: the hub caps a long-poll at 25 seconds server-side, and +the bridge and connector both use a 35-second client timeout on top of that, +so anything shorter causes spurious disconnects. + +``` +hub.example.net { + reverse_proxy 127.0.0.1:8765 { + transport http { + read_timeout 60s + } + } +} +``` + +With `https://hub.example.net` in place, agents drop `CAUCUS_ALLOW_REMOTE_HUB` +entirely: `validate_hub_url` accepts any `https://` URL outright. + +## Troubleshooting + +**`401` on join / register (or on `/peers`, `/channels`, `/forms`, `/ping`, or +`POST /watch-ticket/redeem`), mentioning `CAUCUS_AGENT_KEY`.** The hub has +`--agent-key` set and the caller either sent none, sent the wrong one, or +sent it without the `Bearer ` prefix (an operator or observer token also +passes the four read endpoints, but not `/register` or the ticket redeem). +Set `CAUCUS_AGENT_KEY` (or the `.mcp.json` `Authorization` header) to match +the hub's value exactly. + +**`caucus-watch` prints `[caucus] TICKET REJECTED` and exits.** The ticket a +remote `watch_command()` handed out is single-use and lives 120 seconds +(`WATCH_TICKET_TTL`); it was already redeemed, or too long passed between +minting it and running the watcher. Call `watch_command()` again for a fresh +ticket and run it right away, since a two-minute-old ticket is already gone. + +**`421 Invalid Host header` from `/mcp`.** The client's `Host` header (the +hostname it dialled) is not in the hub's DNS-rebinding allowlist. Add it with +`--allowed-host` (or `CAUCUS_ALLOWED_HOSTS`): a bare name is completed with +the hub's own port, so pass `host:port` only when it differs. + +**`caucus-bridge` / `caucus-watch` refuse to start, citing a cleartext +warning.** `CAUCUS_HUB_URL` (or `--hub`) points at a plain-`http://` +non-loopback host and `CAUCUS_ALLOW_REMOTE_HUB` is not set. Either set +`CAUCUS_ALLOW_REMOTE_HUB=1` on the agent machine, or move the hub behind TLS +and use `https://`. + +**The console WebSocket closes immediately with code 1008.** Either the +operator/observer token in the first frame did not match anything configured +(check `--operator-token`/`--observer-token`), or the browser's page origin +is not in the `/ui` allowlist: add it with `--allowed-origin` +(`CAUCUS_ALLOWED_ORIGINS`). + +**The watcher command `watch_command()` returns still says +`127.0.0.1`.** `--public-url` is not set (or not passed through to this hub +process). Set it to the address agents actually use to reach the hub. + +## Known limits + +- **No native TLS.** `caucus-hub` never takes certificate arguments; a + reverse proxy is not optional for anything crossing an untrusted network. +- **The `/register` rate limiter is keyed on `request.client.host`, with no + `X-Forwarded-For` handling.** Behind a reverse proxy every request arrives + from the proxy's own address, so the entire fleet of agents shares one + token bucket. A registration burst from several agents at once can trip it + for all of them, not just the noisy one. +- **The idle reaper drops a peer after `--client-ttl` (default 300 seconds) + of silence.** A WAN agent whose watcher cannot poll that often (a flaky + link, a long compose turn) loses its slot and must rejoin, exactly as it + would on a local hub, just more likely on a slower network. +- **Console agent-spawning is structurally unavailable on a remote hub.** + `--enable-agent-launcher` refuses to start unless `--host` is loopback, on + top of requiring `--operator-token` and `--agent-cwd`; there is no + configuration that opens it up remotely. +- **The deprecated `?token=` query fallback on `/receive` lands in proxy + access logs.** It exists only so an older watcher keeps working through a + hub upgrade; every current client sends the token as an `Authorization: + Bearer` header instead, which a proxy's default access log does not + capture. diff --git a/src/caucus/autostart.py b/src/caucus/autostart.py index d7f763f..cfe8195 100644 --- a/src/caucus/autostart.py +++ b/src/caucus/autostart.py @@ -36,12 +36,12 @@ from .setup_service import ( DEFAULT_LABEL, - LOOPBACK_HOSTS, SYSTEMD_UNIT_NAME, SetupError, detect_platform, unit_path, ) +from .urlguard import is_loopback_host logger = logging.getLogger(__name__) @@ -80,7 +80,7 @@ def is_local(hub_url: str) -> bool: host = urllib.parse.urlparse(hub_url).hostname except ValueError: return False - return host is not None and host in LOOPBACK_HOSTS + return host is not None and is_loopback_host(host) def service_installed(label: str = DEFAULT_LABEL) -> bool: diff --git a/src/caucus/hub.py b/src/caucus/hub.py index 19a9392..d683ea2 100644 --- a/src/caucus/hub.py +++ b/src/caucus/hub.py @@ -13,6 +13,7 @@ import argparse import asyncio import contextlib +import ipaddress import logging import os import secrets @@ -22,6 +23,7 @@ from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING, Any +from urllib.parse import urlparse import coloredlogs import uvicorn @@ -62,10 +64,18 @@ SendResponse, SpawnAgentRequest, StatusRequest, + WatchTicketRedeemRequest, is_channel, ) from .ratelimit import TokenBucket -from .state import MAX_LEASE_ID_CHARS, CapExceeded, Client, HubState, RegisterOutcome +from .state import ( + MAX_LEASE_ID_CHARS, + WATCH_TICKET_TTL, + CapExceeded, + Client, + HubState, + RegisterOutcome, +) from .supervisor import ( AGENT_NAME_RE, DEFAULT_MAX_AGENTS, @@ -76,6 +86,12 @@ LauncherRefused, validate_agent_cwd, ) +from .urlguard import ( + ALLOW_REMOTE_ENV, + is_loopback_host, + needs_remote_optin, + validate_public_url, +) if TYPE_CHECKING: from mcp.server.fastmcp import FastMCP @@ -138,17 +154,23 @@ @dataclass class AuthConfig: - """Opt-in operator/observer token configuration for the ``/ui`` socket. - - Both tokens default to ``None`` (auth disabled — every connection is an - operator, preserving the localhost default). When ``operator`` is set, the - ``/ui`` socket demands a first-frame ``{"auth": ""}`` handshake and - grades the connection ``operator`` (read-write), ``observer`` (read-only) or - rejected. + """Opt-in credential configuration for the console and the agent door. + + All three credentials default to ``None`` (auth disabled, preserving the + localhost default). ``operator``/``observer`` guard the ``/ui`` socket: + when ``operator`` is set, ``/ui`` demands a first-frame + ``{"auth": ""}`` handshake and grades the connection ``operator`` + (read-write), ``observer`` (read-only) or rejected. + + ``agent`` is an independent, shared pre-shared key guarding the *agent* + door — ``POST /register`` and the ``/mcp`` endpoint — so a hub bound to a + non-loopback address is not an open room. It grants no console rights and + the console tokens grant no agent rights; the two axes never interact. """ operator: str | None = None observer: str | None = None + agent: str | None = None @property def enabled(self) -> bool: @@ -159,9 +181,13 @@ def role_for(self, token: str | None) -> str | None: """Return the role a ``token`` grants, or ``None`` if it grants none. Uses :func:`secrets.compare_digest` for constant-time comparison so a - token is never leaked through timing. When auth is disabled every caller - is an ``operator``. The operator token is checked first, so a token - configured for both roles grants the higher one. + token is never leaked through timing. The comparison is done on UTF-8 + **bytes**: ``compare_digest`` raises ``TypeError`` on a ``str`` holding + anything outside ASCII, and headers decode as latin-1, so a caller could + otherwise turn ``Authorization: Bearer é`` into an unhandled 500. When + auth is disabled every caller is an ``operator``. The operator token is + checked first, so a token configured for both roles grants the higher + one. Args: token: The token presented in the first frame, if any. @@ -174,16 +200,71 @@ def role_for(self, token: str | None) -> str | None: return "operator" if token is None: return None - if self.operator is not None and secrets.compare_digest(token, self.operator): + presented = token.encode("utf-8") + if self.operator is not None and secrets.compare_digest( + presented, self.operator.encode("utf-8") + ): return "operator" - if self.observer is not None and secrets.compare_digest(token, self.observer): + if self.observer is not None and secrets.compare_digest( + presented, self.observer.encode("utf-8") + ): return "observer" return None + def agent_ok(self, token: str | None) -> bool: + """Return whether ``token`` may pass the agent door. + + When no agent key is configured the door is open — the historical + behaviour for a loopback hub, where reaching the port already implies + being on the machine. Once a key is set, only that exact key passes, + compared with :func:`secrets.compare_digest` on UTF-8 bytes (see + :meth:`role_for` for why bytes) so a near-miss cannot be narrowed down + by timing and a non-ASCII bearer cannot raise instead of being refused. + + Args: + token: The bearer token presented by the caller, if any. + + Returns: + ``True`` when the caller may register / reach ``/mcp``, ``False`` + otherwise. + """ + if self.agent is None: + return True + if token is None: + return False + return secrets.compare_digest(token.encode("utf-8"), self.agent.encode("utf-8")) + auth_config = AuthConfig() """Module-level auth config; populated from CLI/env in :func:`main`.""" +AGENT_KEY_REQUIRED_DETAIL = ( + "agent key required: set CAUCUS_AGENT_KEY (or --agent-key) on this client " + "to match the hub" +) +"""Refusal text for a missing/wrong agent key, naming both the env var and the flag. + +Shared by ``POST /register`` and the ``/mcp`` gate so a rejected agent reads the +same actionable sentence whichever door it knocked on. +""" + + +def _bearer_from_header(authorization: str | None) -> str | None: + """Extract the token from an ``Authorization: Bearer `` header. + + Args: + authorization: Raw ``Authorization`` header value, if any. + + Returns: + The token when the header is present and carries a non-empty ``Bearer`` + credential (the scheme is matched case-insensitively, as RFC 7235 + requires), ``None`` otherwise. + """ + if not authorization or authorization[:7].lower() != "bearer ": + return None + bearer = authorization[7:].strip() + return bearer or None + @dataclass class ServerConfig: @@ -925,6 +1006,82 @@ def _body_too_large_response() -> JSONResponse: app.add_middleware(BodySizeLimitMiddleware) +class MCPAgentKeyMiddleware: + """Require the shared agent key on every ``/mcp`` request when one is set. + + ``/mcp`` is the second agent door (``POST /register`` is the first), and it + is one endpoint serving a whole tool surface, so the key is checked once + here at the HTTP layer rather than per tool. This keeps a single mechanism: + a header, the same one the REST clients send, with no ``key`` argument + smuggled into the ``join`` tool. + + Scope discipline, mirroring :class:`MCPPreflightCORSMiddleware`: + + * It acts only when the MCP endpoint is actually mounted + (:data:`_mcp_server` is not ``None``), only for the exact + :attr:`ServerConfig.mcp_path`, and only when an agent key is configured. + Everything else passes straight through, so the REST API, the operator + console and every keyless deployment are untouched. + * It sits *inside* :class:`MCPPreflightCORSMiddleware` (registered before + it, so it is wrapped by it), which means a genuine CORS preflight — an + ``OPTIONS`` from an allowed ``Origin`` — is already answered upstream and + never reaches this layer at all. That is what lets this gate apply to + ``OPTIONS`` as well: the browser exchange is owned entirely by the CORS + layer, so an unkeyed ``OPTIONS`` arriving *here* is not a preflight and + has no business reaching the MCP transport, which would build a session + transport and a task group for it before answering 405. + + The DNS-rebinding ``Host``/``Origin`` allowlist enforced by the transport + itself is untouched and still applies after this gate. + """ + + def __init__(self, app: ASGIApp) -> None: + """Wrap ``app`` with the ``/mcp`` agent-key gate. + + Args: + app: The downstream ASGI application to wrap. + """ + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + """Gate one ASGI event: refuse an unkeyed ``/mcp`` request, else pass on. + + Args: + scope: The ASGI connection scope. + receive: The ASGI receive callable (the inbound event channel). + send: The ASGI send callable (the outbound event channel). + """ + if ( + scope["type"] != "http" + or _mcp_server is None + or scope.get("path") != server_config.mcp_path + or auth_config.agent is None + ): + await self.app(scope, receive, send) + return + + authorization: str | None = None + for name, value in scope.get("headers", []): + if name.decode("latin-1").lower() == "authorization": + authorization = value.decode("latin-1") + break + if auth_config.agent_ok(_bearer_from_header(authorization)): + await self.app(scope, receive, send) + return + + logger.warning("mcp request refused (agent key) path=%s", scope.get("path")) + response = JSONResponse( + status_code=401, content={"detail": AGENT_KEY_REQUIRED_DETAIL} + ) + await response(scope, receive, send) + + +# Registered before the CORS layer so the final stack is +# CORS(AgentKey(BodySize(app))): an allowed-Origin preflight is answered by the +# CORS layer and never reaches this gate, while every real /mcp request does. +app.add_middleware(MCPAgentKeyMiddleware) + + class MCPPreflightCORSMiddleware: """Answer CORS preflight and stamp CORS headers for the ``/mcp`` endpoint. @@ -1124,35 +1281,168 @@ async def index() -> FileResponse: return FileResponse(_UI_INDEX, headers={"Content-Security-Policy": CONSOLE_CSP}) +def _require_agent_read(authorization: str | None) -> None: + """Gate the agent-readable roster endpoints on the shared agent key. + + ``/peers``, ``/channels`` and ``/forms`` are the "scout before you commit" + surface: they answer before a caller has joined, so there is no peer token + to demand. That was defensible while the hub only ever bound to loopback, + but on a keyed non-loopback hub it meant the agent key closed ``/register`` + and ``/mcp`` while anyone who could reach the port still read the roster, + every peer's self-reported status, every private channel's name, topic and + membership, and the text of every pending operator form. + + So: when an agent key is configured, these endpoints demand it — the same + ``Authorization: Bearer`` header ``/register`` takes. An operator or + observer token is accepted too, because the console's own tooling holds + that one and not the agent key. With no agent key configured (the loopback + default) the endpoints stay open, exactly as before. + + Args: + authorization: Raw ``Authorization`` header value, if any. + + Raises: + HTTPException: 401 when a key is configured and the caller has neither + it nor a console token. + """ + if auth_config.agent is None: + return + presented = _bearer_from_header(authorization) + if auth_config.agent_ok(presented): + return + # `auth_config.enabled` guard, not just role_for: with no operator token + # configured role_for grades *every* caller as operator, which would hand + # the whole surface back to anyone the moment a hub set an agent key alone. + if auth_config.enabled and auth_config.role_for(presented) in ( + "operator", + "observer", + ): + return + raise HTTPException(status_code=401, detail=AGENT_KEY_REQUIRED_DETAIL) + + @app.get("/peers") -async def peers() -> dict[str, list[str]]: - """List currently connected project names.""" +async def peers( + authorization: str | None = Header(default=None), +) -> dict[str, list[str]]: + """List currently connected project names. + + Gated on the shared agent key when one is configured; open otherwise. See + :func:`_require_agent_read`. + + Args: + authorization: ``Authorization: Bearer `` (or a console + token), required only when the hub runs with an agent key. + """ + _require_agent_read(authorization) return {"peers": state.peers()} @app.get("/ping") -async def ping(peer: str = Query(min_length=1, max_length=64)) -> dict[str, object]: +async def ping( + peer: str = Query(min_length=1, max_length=64), + authorization: str | None = Header(default=None), +) -> dict[str, object]: """Report a peer's liveness and self-reported status without disturbing it. A presence probe answered entirely from the hub's in-memory bookkeeping, so the target agent's turn is never consumed — the whole point of a ping is to learn "is it still there, and what is it doing?" for ~0 cost to the peer. - Open (no token), like ``/peers``: liveness is no more sensitive than the - roster. See :meth:`~caucus.state.HubState.ping` for the response shape + + Gated on the shared agent key when one is configured, exactly like + ``/peers``, ``/channels`` and ``/forms`` (see :func:`_require_agent_read`): + the payload is far more than liveness. It confirms whether a *named* peer + exists, how long since it last touched the hub, whether a listener is + attached, and its own last :meth:`~caucus.state.HubState.set_status` + string, which is peer-authored prose describing what that agent is working + on. On a keyed non-loopback hub, handing that to anyone who can reach the + port is a disclosure the key exists to prevent. With no key configured (the + loopback default) it stays open, as before. + + See :meth:`~caucus.state.HubState.ping` for the full response shape (``state`` is ``live`` / ``reaped`` / ``absent``). + + Args: + peer: The project name to probe. + authorization: ``Authorization: Bearer `` (or a console + token), required only when the hub runs with an agent key. """ + _require_agent_read(authorization) return state.ping(peer) +@app.post("/watch-ticket/redeem") +async def redeem_watch_ticket( + req: WatchTicketRedeemRequest, + authorization: str | None = Header(default=None), +) -> dict[str, str]: + """Exchange a single-use watch ticket for the peer token it stands for. + + The door ``caucus-watch --ticket`` knocks on. A remote agent is handed a + ticket rather than its peer token, because that token is the room bearer + for ``/receive``, ``/send``, ``/ack``, ``/channels/*``, ``/ask`` and + ``/floor``: printing it in a shell command would put it in the agent's + transcript, its shell history and the watcher's environ, which is exactly + what the loopback token file exists to avoid. The ticket is spent here, + once, seconds after it was minted. + + Gated on the shared agent key the same way ``POST /register`` is: a keyed + hub must not hand tokens to whoever can reach the port. An unknown, already + spent or expired ticket answers **404**, not 401: the caller's credential + was fine, the ticket simply is not there any more, and conflating the two + would have the watcher report a dead session when all it needs is a fresh + ``watch_command()``. + + Args: + req: The body carrying the ticket to spend. + authorization: ``Authorization: Bearer ``, required only + when the hub runs with an agent key. + + Returns: + ``{"token": ""}``. + + Raises: + HTTPException: 401 when the agent key is missing or wrong, 404 when the + ticket is unknown, already spent, or past its TTL. + """ + if not auth_config.agent_ok(_bearer_from_header(authorization)): + # Deliberately logged without the ticket value: this response is the + # one place a peer token is handed out, so nothing about the exchange + # belongs in a log file. + logger.warning("watch-ticket redeem refused (agent key)") + raise HTTPException(status_code=401, detail=AGENT_KEY_REQUIRED_DETAIL) + token = state.redeem_watch_ticket(req.ticket) + if token is None: + raise HTTPException( + status_code=404, + detail=( + "watch ticket unknown, already used, or expired: a ticket is" + f" single-use and lives {WATCH_TICKET_TTL:.0f}s. Call" + " watch_command() again for a fresh one." + ), + ) + return {"token": token} + + @app.get("/channels") -async def channels() -> dict[str, dict[str, dict[str, object]]]: +async def channels( + authorization: str | None = Header(default=None), +) -> dict[str, dict[str, dict[str, object]]]: """List active private channels with their topic and members. Channels are ephemeral (derived from live membership), so this only ever lists channels with at least one connected member. Each entry is ``{"topic": str | None, "members": [name, ...]}``. Serves both agent discovery (including the late-joiner directory) and the operator console. + + Gated on the shared agent key when one is configured; open otherwise. See + :func:`_require_agent_read`. + + Args: + authorization: ``Authorization: Bearer `` (or a console + token), required only when the hub runs with an agent key. """ + _require_agent_read(authorization) return {"channels": state.channels()} @@ -1357,7 +1647,9 @@ async def version_info() -> dict[str, str]: @app.post("/register", response_model=None) async def register( - req: RegisterRequest, request: Request + req: RegisterRequest, + request: Request, + authorization: str | None = Header(default=None), ) -> RegisterResponse | JSONResponse: """Register a project and hand back its access token. @@ -1376,14 +1668,23 @@ async def register( listener is gone (dead process / timed-out watcher), the slot is taken over (REPLACED outcome) and a human-readable ``note`` advises the caller. - The endpoint is unauthenticated by design (a peer has no token yet), so it - is throttled per source host (429) to deny a registration flood — a cheap - memory-exhaustion DoS — and any :class:`CapExceeded` from the client cap is - surfaced as 409. + A peer holds no per-peer token yet, so this is the door the shared agent key + guards: when one is configured, the caller must present it as + ``Authorization: Bearer `` or the request is refused with 401. With no + key configured the endpoint stays open (the historical loopback behaviour). + Either way it is throttled per source host (429) to deny a registration + flood — a cheap memory-exhaustion DoS — and any :class:`CapExceeded` from + the client cap is surfaced as 409. """ - # DoS brake: /register is the one mutating endpoint with no token, so the - # only attacker handle is the source host. A token bucket per host lets a - # whole fleet boot at once but caps a flood. + # Agent-key gate first, *before* the rate-limit bucket: a caller with no key + # must not be able to drain another host's budget, and refusing an unknown + # client should stay as cheap as possible. + if not auth_config.agent_ok(_bearer_from_header(authorization)): + logger.warning("register refused (agent key) project=%s", req.project) + raise HTTPException(status_code=401, detail=AGENT_KEY_REQUIRED_DETAIL) + # DoS brake: /register is the one mutating endpoint with no peer token, so + # the only attacker handle is the source host. A token bucket per host lets + # a whole fleet boot at once but caps a flood. host = request.client.host if request.client else "" retry = _register_rate_limited(host) if retry is not None: @@ -1582,11 +1883,7 @@ def _resolve_receive_token(authorization: str | None, token: str | None) -> str The bearer token from the header when present and well-formed, else the query token, else ``None``. """ - if authorization and authorization[:7].lower() == "bearer ": - bearer = authorization[7:].strip() - if bearer: - return bearer - return token + return _bearer_from_header(authorization) or token @app.post("/ask", response_model=None) @@ -1640,13 +1937,25 @@ async def ask(req: AskRequest) -> AskResponse | JSONResponse: @app.get("/forms") -async def forms() -> dict[str, list[dict[str, object]]]: +async def forms( + authorization: str | None = Header(default=None), +) -> dict[str, list[dict[str, object]]]: """List the currently pending operator forms. - Read-only and unauthenticated, like ``/peers``: an agent calls this before - pushing a form so it does not duplicate one already awaiting the operator. - Resolved forms are dropped, so this only lists pending ones. + Read-only and joinable-before-join, like ``/peers``: an agent calls this + before pushing a form so it does not duplicate one already awaiting the + operator. Resolved forms are dropped, so this only lists pending ones. + + Gated on the shared agent key when one is configured; open otherwise. A + pending form carries the asker's question verbatim, so it is exactly the + kind of content the key exists to keep off the network. See + :func:`_require_agent_read`. + + Args: + authorization: ``Authorization: Bearer `` (or a console + token), required only when the hub runs with an agent key. """ + _require_agent_read(authorization) return {"forms": state.list_forms()} @@ -2110,8 +2419,8 @@ async def status_set(req: StatusRequest) -> dict[str, object] | JSONResponse: async def floor_list() -> dict[str, dict[str, dict[str, object]]]: """List the active talking sticks, keyed by scope. - Open (no token), like ``/peers`` and ``/ping``: which scopes are currently - locked is no more sensitive than the roster. Each entry is + Open (no token), unlike the agent-key-gated ``/peers``, ``/ping``, + ``/channels`` and ``/forms``. Each entry is ``{"scope", "holder", "reason", "hands": [...], "since"}``. An empty map means no stick is up and every scope is open. Lets an agent scout whether the floor it is about to use is held before it speaks. @@ -2642,7 +2951,10 @@ def _launch() -> None: threading.Timer(delay, _launch).start() -_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1"}) +#: Bind addresses that mean "every interface". They are not addresses anything +#: connects *to*, so the hub cannot advertise one and has to be told the URL +#: agents really reach it at (``--public-url``). +_WILDCARD_HOSTS = frozenset({"0.0.0.0", "::"}) def _resolve_mcp_http(explicit: bool | None, host: str) -> bool: @@ -2670,10 +2982,262 @@ def _resolve_mcp_http(explicit: bool | None, host: str) -> bool: env = os.environ.get("CAUCUS_MCP_HTTP") if env is not None: return env.strip().lower() in {"1", "true", "yes", "on"} - return host in _LOOPBACK_HOSTS + return is_loopback_host(host) -def _mount_mcp_http(*, host: str, port: int, mcp_path: str, extra_origins: set[str]) -> None: +def _entry_carries_port(entry: str) -> bool: + """Return whether an allowed-host entry already names a port. + + ``Host`` header values spell an IPv6 literal in brackets (``[::1]:8765``), + so the bracket form decides on its own: a closing ``]`` at the end means no + port followed. Everything else is a name or an IPv4 literal, where a single + colon can only introduce the port. + + Args: + entry: One ``--allowed-host`` value, already stripped. + + Returns: + ``True`` when a port is present and the entry should be used verbatim. + """ + if entry.startswith("["): + return not entry.endswith("]") + if _is_bare_ipv6(entry): + # Every colon belongs to the address itself, so none of them is a port. + return False + return ":" in entry + + +def _is_bare_ipv6(entry: str) -> bool: + """Return whether ``entry`` is an unbracketed IPv6 literal. + + ``--allowed-host ::1`` and ``--allowed-host 2001:db8::1`` are the forms an + operator types, but a ``Host`` header always brackets an IPv6 literal + (``[::1]:8765``). Recognising the bare form is what lets + :func:`_normalise_allowed_host` rewrite it into something the guard can + actually match. + + Args: + entry: One ``--allowed-host`` value, already stripped. + + Returns: + ``True`` when the whole entry parses as an IPv6 address. + """ + try: + ipaddress.IPv6Address(entry) + except ValueError: + return False + return True + + +def _normalise_allowed_host(entry: str, port: int) -> str: + """Rewrite one allowlist entry into the ``Host`` header form it must match. + + Three shapes go in: a bare name or IPv4 literal, a bracketed IPv6 literal, + and a bare IPv6 literal. All three come out as the guard compares them — an + IPv6 address bracketed, and a host with no port completed with the hub's + own port, which is what an operator naming a bare host means. + + Args: + entry: One ``--allowed-host`` value, already stripped. + port: The port the hub listens on, used to complete bare hosts. + + Returns: + The entry as a ``Host`` header value. + """ + if _entry_carries_port(entry): + return entry + host = f"[{entry}]" if _is_bare_ipv6(entry) else entry + return f"{host}:{port}" + + +def _collect_allowed_hosts(cli: list[str] | None, port: int) -> list[str]: + """Build the extra ``Host`` allowlist for the ``/mcp`` DNS-rebinding guard. + + Mirrors the ``--allowed-origin`` / ``CAUCUS_ALLOWED_ORIGINS`` pair: repeated + CLI flags merged with a comma-separated environment variable. Without this, + the only non-loopback entry the guard ever learns is the bind address + itself, so a hub on ``0.0.0.0`` reached as ``hub.lan:8765`` is refused with + no way for the operator to allow it. + + A bare host is expanded to the hub's own port, which is what an operator + naming a hostname means; an entry that already carries a port is kept + verbatim, so a reverse proxy on another port can be allowed too. A bare + IPv6 literal is bracketed on the way, since that is the only form a ``Host`` + header ever spells (see :func:`_normalise_allowed_host`). + + Args: + cli: The repeated ``--allowed-host`` values, or ``None``. + port: The port the hub listens on, used to complete bare hosts. + + Returns: + The de-duplicated ``host:port`` entries, in the order first seen. + """ + raw = list(cli or []) + raw.extend(os.environ.get("CAUCUS_ALLOWED_HOSTS", "").split(",")) + hosts: list[str] = [] + for entry in (e.strip() for e in raw): + if not entry: + continue + value = _normalise_allowed_host(entry, port) + if value not in hosts: + hosts.append(value) + return hosts + + +def _doors_block(*, operator: bool, agent: bool, requirement: str) -> str: + """Render the two-doors inventory shared by both credential refusals. + + The flag names, the environment variables and the ``openssl`` lines have to + read identically whether the hub was refused for its bind address or for the + public URL it advertises; spelling them twice is how one of the two ends up + naming a flag that was renamed in the other. + + Args: + operator: Whether an operator token is already configured. + agent: Whether an agent key is already configured. + requirement: The sentence introducing the generation snippet, which + names *why* both credentials are demanded in this particular case. + + Returns: + The doors inventory, the requirement sentence and the two ``export`` + lines, ending with a newline. + """ + mark = {True: "already set", False: "MISSING"} + return ( + " agent door /register and /mcp - join the room, read everything\n" + " said in it\n" + f" --agent-key KEY (env CAUCUS_AGENT_KEY): {mark[agent]}\n" + " operator door /ui - pause, stop, kick, read the whole transcript\n" + " --operator-token TOKEN (env CAUCUS_OPERATOR_TOKEN):" + f" {mark[operator]}\n" + "\n" + f"{requirement} Generate and set them:\n" + "\n" + ' export CAUCUS_AGENT_KEY="$(openssl rand -hex 24)"\n' + ' export CAUCUS_OPERATOR_TOKEN="$(openssl rand -hex 24)"\n' + ) + + +def _insecure_bind_message( + host: str, + *, + operator: bool, + agent: bool, + public_url: bool, + wildcard: bool, +) -> str: + """Compose the refusal shown when a non-loopback bind is under-configured. + + This message is the whole user experience of the refusal: somebody just had + their hub refuse to start, and everything they need to fix it — both doors, + both flags, both environment variables, the escape hatch, and the way back + to a loopback bind — has to be in this one block. + + Args: + host: The non-loopback address that was requested. + operator: Whether an operator token is already configured. + agent: Whether an agent key is already configured. + public_url: Whether an advertised public URL is already configured. + wildcard: Whether ``host`` is a wildcard bind, which makes the public + URL a third required setting rather than an optional one. + + Returns: + The multi-line refusal text, ready to hand to ``parser.error``. + """ + mark = {True: "already set", False: "MISSING"} + # On a wildcard bind there is no address to advertise, so --public-url joins + # the two credentials as a required setting and is marked the same way. + reach = ( + "\nA wildcard bind names no reachable address, so agents must be told\n" + "one -- otherwise the Host header they send is rejected by the\n" + "DNS-rebinding guard and the watcher command they get points at\n" + "127.0.0.1:\n" + "\n" + f" --public-url URL (env CAUCUS_PUBLIC_URL): {mark[public_url]}\n" + " --allowed-host HOST (env CAUCUS_ALLOWED_HOSTS, comma-separated)\n" + if wildcard + else "\nThen tell agents where to reach you, or the Host header they send is\n" + "rejected by the DNS-rebinding guard and the watcher command they get\n" + "points at 127.0.0.1:\n" + "\n" + " --public-url URL (env CAUCUS_PUBLIC_URL)\n" + " --allowed-host HOST (env CAUCUS_ALLOWED_HOSTS, comma-separated)\n" + ) + return ( + f"refusing to bind {host}: a hub on a non-loopback address is reachable\n" + "from other machines, and by default neither of its two doors is locked\n" + "nor does it know the address to hand those machines.\n" + "\n" + + _doors_block( + operator=operator, + agent=agent, + requirement="Both are required on a non-loopback bind.", + ) + + f" caucus-hub --host {host}" + + (" --public-url https://hub.example.net\n" if wildcard else "\n") + + reach + + "\n" + "Not what you meant? --host 127.0.0.1 keeps the hub on this machine.\n" + "Meant it, on a network that already authenticates (a tunnel, a private\n" + "LAN)? --allow-insecure-bind starts anyway, with both doors open." + ) + + +def _insecure_public_url_message( + public_url: str, + *, + operator: bool, + agent: bool, +) -> str: + """Compose the refusal shown when an advertised hub is under-configured. + + The sibling of :func:`_insecure_bind_message` for the deployment where the + socket is *not* the exposure: the hub binds to loopback and something in + front of it (a tunnel, a reverse proxy, a port forward) carries the outside + world in. Nothing about the bind address betrays that, so the only signal + the hub has is the operator declaring a public URL that is not loopback -- + and that declaration has to be taken as seriously as a non-loopback bind, + because the doors behind it are exactly as open. + + Args: + public_url: The non-loopback base URL the operator asked to advertise. + operator: Whether an operator token is already configured. + agent: Whether an agent key is already configured. + + Returns: + The multi-line refusal text, ready to hand to ``parser.error``. + """ + return ( + f"refusing to start: --public-url {public_url} says agents on other\n" + "machines dial this hub, and by default neither of its two doors is\n" + "locked. The bind is loopback, so the socket is not the exposure --\n" + "whatever sits in front of it is, and anything reaching that front\n" + "reaches both of these:\n" + "\n" + + _doors_block( + operator=operator, + agent=agent, + requirement="Both are required once the hub is advertised off-box.", + ) + + f" caucus-hub --host 127.0.0.1 --public-url {public_url}\n" + "\n" + "Not what you meant? A loopback public URL (http://localhost:8765) is\n" + "just a nicer address for this machine and needs none of this.\n" + "Meant it, behind something that already authenticates? " + "--allow-insecure-bind\n" + "starts anyway, with both doors open." + ) + + +def _mount_mcp_http( + *, + host: str, + port: int, + mcp_path: str, + extra_origins: set[str], + extra_hosts: list[str] | None = None, + public_url: str | None = None, +) -> None: """Build the Streamable HTTP MCP server and attach its route to the hub app. Mirrors the ``disk_log`` pattern (amendment A4): sets the module global @@ -2691,6 +3255,10 @@ def _mount_mcp_http(*, host: str, port: int, mcp_path: str, extra_origins: set[s port: The port the hub listens on. mcp_path: The path the endpoint serves at (e.g. ``/mcp``). extra_origins: Operator-approved extra browser origins (the CSWSH set). + extra_hosts: Operator-approved extra ``Host`` values from + :func:`_collect_allowed_hosts`, or ``None``. + public_url: The externally reachable base URL from ``--public-url``, + or ``None`` to advertise the bind address. """ global _mcp_server from . import mcp_http @@ -2698,20 +3266,44 @@ def _mount_mcp_http(*, host: str, port: int, mcp_path: str, extra_origins: set[s # Record the served path so the CORS layer scopes its preflight handling to # exactly this route. server_config.mcp_path = mcp_path - browse_host = "127.0.0.1" if host in ("0.0.0.0", "::") else host - self_url = f"http://{browse_host}:{port}" - allowed_hosts: list[str] = [] + browse_host = "127.0.0.1" if host in _WILDCARD_HOSTS else host + # An operator-declared public URL wins: on a wildcard bind the rewritten + # 127.0.0.1 is only right for an agent on this machine, and handing a remote + # one that address is how watch_command produced an unrunnable command. + self_url = public_url or f"http://{browse_host}:{port}" + allowed_hosts: list[str] = list(extra_hosts or []) allowed_origins: list[str] = list(extra_origins) + + def _allow_host(value: str) -> None: + """Append one ``Host`` value unless the allowlist already carries it. + + The same address arrives from up to three places (``--allowed-host``, + the bind address, the public URL's netloc), and a repeated entry means + nothing to the guard while making the startup log harder to read. + """ + if value and value not in allowed_hosts: + allowed_hosts.append(value) + # A bind-all address is not a connectable origin; only add a concrete host. - if host not in ("0.0.0.0", "::"): - allowed_hosts.append(f"{host}:{port}") - allowed_origins.append(f"http://{host}:{port}") + if host not in _WILDCARD_HOSTS: + _allow_host(_normalise_allowed_host(host, port)) + origin = f"http://{_normalise_allowed_host(host, port)}" + if origin not in allowed_origins: + allowed_origins.append(origin) + if public_url is not None: + # The Host header a client dialling the public URL sends is its netloc; + # allowing it here spares the operator from repeating it as an + # --allowed-host on every deployment that sets --public-url. + _allow_host(urlparse(public_url).netloc) _mcp_server = mcp_http.build_mcp_server( app, self_url=self_url, mcp_path=mcp_path, allowed_hosts=allowed_hosts, allowed_origins=allowed_origins, + # Loopback-only is the one deployment where a token file on the hub's + # filesystem is also on the agent's; anything else gets the env form. + remote=public_url is not None or not is_loopback_host(host), ) # streamable_http_app() lazily creates the session manager (run by lifespan) # and registers the endpoint route; attach that route to the hub app. @@ -2764,6 +3356,15 @@ def main() -> None: "with --operator-token). Env: CAUCUS_OBSERVER_TOKEN" ), ) + parser.add_argument( + "--agent-key", + default=os.environ.get("CAUCUS_AGENT_KEY"), + help=( + "require this shared key (Authorization: Bearer ...) on /register " + "and /mcp so only agents that hold it can join; unset (default) " + "leaves the agent door open. Env: CAUCUS_AGENT_KEY" + ), + ) parser.add_argument( "--log-file", default=os.environ.get("CAUCUS_LOG_FILE"), @@ -2792,6 +3393,40 @@ def main() -> None: "allowed. Env: CAUCUS_ALLOWED_ORIGINS (comma-separated)" ), ) + parser.add_argument( + "--allowed-host", + action="append", + default=None, + metavar="HOST", + help=( + "extra Host header value the /mcp DNS-rebinding guard accepts " + "(repeatable). Needed to reach a hub bound to 0.0.0.0 under a " + "name, e.g. --allowed-host hub.lan. A bare host is allowed on this " + "hub's port; pass host:port for anything else. Loopback is always " + "allowed. Env: CAUCUS_ALLOWED_HOSTS (comma-separated)" + ), + ) + parser.add_argument( + "--public-url", + default=os.environ.get("CAUCUS_PUBLIC_URL"), + metavar="URL", + help=( + "base URL other machines reach this hub at, e.g. " + "https://hub.example.net. Advertised to agents instead of the bind " + "address, so the caucus-watch command they are handed is runnable " + "off-box. Scheme plus host, no path. Env: CAUCUS_PUBLIC_URL" + ), + ) + parser.add_argument( + "--allow-insecure-bind", + action="store_true", + help=( + "start without --operator-token and --agent-key on a non-loopback " + "address, or behind a non-loopback --public-url. Both doors stay " + "open to anything that can reach the hub; only for a network that " + "already authenticates" + ), + ) parser.add_argument( "--mcp-http", action=argparse.BooleanOptionalAction, @@ -2849,9 +3484,77 @@ def main() -> None: ) args = parser.parse_args() + public_url: str | None = args.public_url or None + + # Refuse before anything is configured or bound. A non-loopback bind puts + # both doors on the network, and neither is locked by default: the agent + # door lets any reachable client join and read the room, the operator door + # grades every caller as operator. Demand both credentials, once, here -- + # this is the same gate caucus-setup-service applies to the installed unit. + # A wildcard bind adds a third requirement: 0.0.0.0 is not an address + # anything dials, so without --public-url the hub would advertise the + # 127.0.0.1 rewrite and hand every remote agent a watcher command pointing + # back at its own machine. Fix that at the source rather than downstream. + # A loopback bind is not proof the hub is unreachable: behind a tunnel or a + # reverse proxy the socket stays on 127.0.0.1 while the world dials the + # front. The one thing the operator tells us in that shape is --public-url, + # and _mount_mcp_http already reads a non-loopback one as "remote" (see its + # `remote=` argument). The credentials gate has to read it the same way, or + # the exact deployment that most needs both doors locked is the one that + # starts without either. A loopback public URL is just a prettier address + # for this machine and arms nothing. + wildcard = args.host in _WILDCARD_HOSTS + # Parsed defensively: the value is still unvalidated here, and one that + # urlparse finds no hostname in must fall through to validate_public_url + # below rather than be read as an exposure. A URL that *does* name a + # non-loopback host but fails validation for another reason (a path, say) + # hits this gate first, which is the right order: missing credentials on an + # advertised hub outrank the shape of the address being advertised. + advertised_host = urlparse(public_url).hostname if public_url else None + advertised_remote = ( + public_url + if advertised_host is not None and not is_loopback_host(advertised_host) + else None + ) + under_configured = not (args.operator_token and args.agent_key) or ( + wildcard and not public_url + ) + if not args.allow_insecure_bind and under_configured: + if not is_loopback_host(args.host): + parser.error( + _insecure_bind_message( + args.host, + operator=bool(args.operator_token), + agent=bool(args.agent_key), + public_url=bool(public_url), + wildcard=wildcard, + ) + ) + elif advertised_remote is not None: + parser.error( + _insecure_public_url_message( + advertised_remote, + operator=bool(args.operator_token), + agent=bool(args.agent_key), + ) + ) + + if public_url is not None: + try: + public_url = validate_public_url(public_url) + except ValueError as exc: + parser.error(str(exc)) + global disk_log, launcher_config - auth_config.operator = args.operator_token - auth_config.observer = args.observer_token + # `or None` on all three: an exported-but-blank env var (or an explicit + # --agent-key "") is "no credential", not a credential nobody can present. + # Left raw, an empty agent key makes agent_ok() reject every caller and an + # empty operator token turns AuthConfig.enabled on with a token no first + # frame can match -- a lockout, in both cases, rather than a no-op. The + # clients already normalise the same way (hub_connector, mcp_bridge). + auth_config.operator = args.operator_token or None + auth_config.observer = args.observer_token or None + auth_config.agent = args.agent_key or None # Fail closed at boot, not per request. AuthConfig.role_for grades every # caller as "operator" when no operator token is set, so "operator token @@ -2865,7 +3568,7 @@ def main() -> None: "every caller is graded as operator and the launcher would let " "anyone who can reach this hub start processes on this machine" ) - if args.host not in _LOOPBACK_HOSTS: + if not is_loopback_host(args.host): parser.error( f"--enable-agent-launcher requires a loopback bind, got " f"--host {args.host}: process creation must not be reachable " @@ -2892,6 +3595,9 @@ def main() -> None: extra_origins: set[str] = set(args.allowed_origin or []) env_origins = os.environ.get("CAUCUS_ALLOWED_ORIGINS", "") extra_origins.update(o.strip() for o in env_origins.split(",") if o.strip()) + # Same shape for the Host allowlist, which the /mcp DNS-rebinding guard + # reads. Bare entries are completed with this hub's own port. + extra_hosts = _collect_allowed_hosts(args.allowed_host, args.port) server_config.host = args.host server_config.port = args.port server_config.allowed_origins = frozenset(extra_origins) @@ -2908,16 +3614,39 @@ def main() -> None: port=args.port, mcp_path=args.mcp_path, extra_origins=extra_origins, + extra_hosts=extra_hosts, + public_url=public_url, + ) + # The agent door (/register, and /mcp when mounted) is open to anything that + # can reach the port unless a shared key is set. Only reachable now via + # --allow-insecure-bind, which is exactly when it deserves saying again. + if not is_loopback_host(args.host) and not auth_config.agent: + logger.warning( + "the hub is bound to a non-loopback address (%s) without " + "--agent-key: any client that can reach this host can register as " + "a peer and read the room (Env: CAUCUS_AGENT_KEY)", + args.host, ) - if args.host not in {"127.0.0.1", "localhost", "::1"} and not args.operator_token: - logger.warning( - "/mcp is exposed on a non-loopback address (%s) without " - "--operator-token; any client that can reach this host can " - "register as a peer", - args.host, - ) coloredlogs.install(level=args.log_level, fmt="%(asctime)s %(name)s %(levelname)s %(message)s") logger.info("starting hub on http://%s:%d", args.host, args.port) + if public_url is not None: + logger.info("advertising %s to agents (--public-url)", public_url) + # Said once, at startup, rather than on every watch_command: a plain-http + # public URL is the whole deployment's posture, not a per-call surprise. + # Every peer token the hub hands out travels to that address in clear, + # and so does every message; the watcher command carries the token in an + # environment variable and needs CAUCUS_ALLOW_REMOTE_HUB=1 on the agent's + # machine just to be allowed to dial it. + if needs_remote_optin(public_url): + logger.warning( + "--public-url %s is plain http to a non-loopback host: peer " + "tokens and every message cross the network in clear, and the " + "watcher command agents are handed has to carry %s=1 to run at " + "all. Terminate TLS in front of the hub (https://) or reach it " + "through a tunnel instead", + public_url, + ALLOW_REMOTE_ENV, + ) if launcher_config.enabled: # Loud on purpose: this hub can now start processes on this machine. logger.warning( diff --git a/src/caucus/hub_connector.py b/src/caucus/hub_connector.py index 36d4eef..1396bfc 100644 --- a/src/caucus/hub_connector.py +++ b/src/caucus/hub_connector.py @@ -24,6 +24,7 @@ from __future__ import annotations import logging +import os from dataclasses import dataclass, field from enum import Enum from types import TracebackType @@ -32,6 +33,9 @@ logger = logging.getLogger("caucus.connector") +#: Environment variable carrying the hub's shared agent key, if it runs with one. +AGENT_KEY_ENV = "CAUCUS_AGENT_KEY" + # Default HTTP timeout. Sits above the hub's 25s long-poll ceiling so a quiet # ``/receive`` returns on the server's terms rather than tripping the client # timeout (mirrors the bridge's server-poll < client-timeout ordering). @@ -258,6 +262,7 @@ def __init__( timeout: float = DEFAULT_TIMEOUT, transport: httpx.AsyncBaseTransport | None = None, limits: httpx.Limits | None = None, + agent_key: str | None = None, ) -> None: """Initialize the connector. @@ -278,11 +283,17 @@ def __init__( URL/socket transport, untouched. limits: Optional explicit connection-pool bounds. ``None`` uses httpx's defaults. + agent_key: The hub's shared agent key, presented on ``/register`` + when the hub demands one. ``None`` (the default) falls back to + the :data:`AGENT_KEY_ENV` environment variable, so an agent + inherits the key from its environment with no wiring; an empty + value means "no key" (the hub is open). """ self._base = hub_url.rstrip("/") self._timeout = timeout self._transport = transport self._limits = limits + self._agent_key = agent_key or os.environ.get(AGENT_KEY_ENV) or None self._http: httpx.AsyncClient | None = None @property @@ -290,6 +301,22 @@ def hub_url(self) -> str: """The normalized hub base URL (no trailing slash).""" return self._base + def _agent_headers(self) -> dict[str, str] | None: + """Return the ``Authorization`` header carrying the shared agent key. + + Sent on the calls made *without* a peer token: ``/register``, and the + pre-join read surface (``/peers``, ``/channels``, ``/forms``) the hub + now gates on the same key. Every other call spends the peer token on + that header instead, so the key is never sent alongside it. + + Returns: + ``{"Authorization": "Bearer "}`` when a key is configured, else + ``None`` so httpx sends no extra header at all. + """ + if self._agent_key is None: + return None + return {"Authorization": f"Bearer {self._agent_key}"} + # PYI034 wants `Self`, which needs Python 3.11; the floor here is 3.10. async def __aenter__(self) -> HubConnector: # noqa: PYI034 """Open the underlying HTTP client. @@ -393,7 +420,9 @@ async def register( NameInUseError: If the hub refuses the join with HTTP 409 because a live listener already holds the project name and the presented token (if any) did not match. - httpx.HTTPError: If the hub is unreachable or returns an error. + httpx.HTTPError: If the hub is unreachable or returns an error — + including HTTP 401 when the hub requires a shared agent key and + this connector holds none or the wrong one. """ http = self._require_http() payload: dict[str, object] = { @@ -402,7 +431,7 @@ async def register( } if token is not None: payload["token"] = token - resp = await http.post("/register", json=payload) + resp = await http.post("/register", json=payload, headers=self._agent_headers()) if resp.status_code == 409: body = resp.json() raise NameInUseError( @@ -618,7 +647,7 @@ async def peers(self) -> list[str]: httpx.HTTPError: If the hub is unreachable or returns an error. """ http = self._require_http() - resp = await http.get("/peers") + resp = await http.get("/peers", headers=self._agent_headers()) resp.raise_for_status() return list(resp.json().get("peers", [])) @@ -626,8 +655,9 @@ async def ping(self, peer: str) -> dict[str, object]: """Probe a peer's liveness and self-reported status from the hub. Answered entirely from the hub's in-memory bookkeeping, so the target - agent's turn is never consumed. Open endpoint (no token), like - :meth:`peers`. + agent's turn is never consumed. Carries the shared agent key when one is + configured, like :meth:`peers`: the hub gates this probe on it, because + the payload includes the peer's own self-reported activity line. Args: peer: The project name to check. @@ -641,7 +671,9 @@ async def ping(self, peer: str) -> dict[str, object]: httpx.HTTPError: If the hub is unreachable or returns an error. """ http = self._require_http() - resp = await http.get("/ping", params={"peer": peer}) + resp = await http.get( + "/ping", params={"peer": peer}, headers=self._agent_headers() + ) resp.raise_for_status() return dict(resp.json()) @@ -721,7 +753,7 @@ async def list_forms(self) -> list[dict[str, object]]: httpx.HTTPError: If the hub is unreachable or returns an error. """ http = self._require_http() - resp = await http.get("/forms") + resp = await http.get("/forms", headers=self._agent_headers()) resp.raise_for_status() return list(resp.json().get("forms", [])) @@ -822,7 +854,7 @@ async def channels(self) -> dict[str, dict[str, object]]: httpx.HTTPError: If the hub is unreachable or returns an error. """ http = self._require_http() - resp = await http.get("/channels") + resp = await http.get("/channels", headers=self._agent_headers()) resp.raise_for_status() return dict(resp.json().get("channels", {})) diff --git a/src/caucus/mcp_bridge.py b/src/caucus/mcp_bridge.py index c8bfa91..dae955a 100644 --- a/src/caucus/mcp_bridge.py +++ b/src/caucus/mcp_bridge.py @@ -21,6 +21,11 @@ launches it at the repo root), so the same ``.mcp.json`` is copy-pasteable into any repo without editing. ``join`` can still override it per call. * ``CAUCUS_HUB_URL`` -- hub base URL (default ``http://127.0.0.1:8765``). +* ``CAUCUS_AGENT_KEY`` -- shared key for a hub that guards its agent door + (``caucus-hub --agent-key``). Optional: unset means the hub is open, which is + the loopback default. Presented as ``Authorization: Bearer `` on + ``/register`` and on the pre-join read surface (``/peers``, ``/channels``, + ``/forms``); every call made after join spends the per-peer token instead. """ from __future__ import annotations @@ -66,6 +71,30 @@ def _default_project() -> str: HUB_URL = os.environ.get("CAUCUS_HUB_URL", "http://127.0.0.1:8765").rstrip("/") PROJECT = os.environ.get("CAUCUS_PROJECT") or _default_project() +# Shared key for a hub that guards its agent door; ``None`` when the hub is open +# (the loopback default). An empty value is normalized to None so an exported +# but blank env var does not turn into an empty bearer. +AGENT_KEY: str | None = os.environ.get("CAUCUS_AGENT_KEY") or None + + +def _agent_headers() -> dict[str, str]: + """Return the extra headers carrying the shared agent key. + + The key is presented on the calls made *without* a peer token: ``/register`` + and the pre-join read surface (``/peers``, ``/channels``, ``/forms``), which + the hub gates on the same key. Every other call already spends the peer + token as its own ``Authorization`` bearer, so the key never joins it there. + Read from the module global at call time so a test (or a re-exec) can + rebind it. + + Returns: + ``{"Authorization": "Bearer "}`` when a key is configured, else an + empty mapping (httpx then sends no extra header at all). + """ + if AGENT_KEY is None: + return {} + return {"Authorization": f"Bearer {AGENT_KEY}"} + mcp = FastMCP( "caucus", @@ -308,7 +337,7 @@ def _attempt_auto_rejoin() -> str | None: payload["token"] = _token try: with _client() as http: - resp = http.post("/register", json=payload) + resp = http.post("/register", json=payload, headers=_agent_headers()) if resp.status_code == 409: logger.warning( "auto-rejoin refused for project=%s: name is held by" @@ -611,7 +640,7 @@ def join( payload["token"] = _token try: with _client() as http: - resp = http.post("/register", json=payload) + resp = http.post("/register", json=payload, headers=_agent_headers()) if resp.status_code == 409: body = resp.json() note = body.get("note", "an active listener already holds this name") @@ -771,7 +800,7 @@ def list_peers() -> dict[str, object]: if gate is not None: return gate with _client() as http: - resp = http.get("/peers") + resp = http.get("/peers", headers=_agent_headers()) resp.raise_for_status() return {"peers": list(resp.json().get("peers", []))} @@ -784,7 +813,7 @@ def ping(peer: str) -> dict[str, object]: if gate is not None: return gate with _client() as http: - resp = http.get("/ping", params={"peer": peer}) + resp = http.get("/ping", params={"peer": peer}, headers=_agent_headers()) resp.raise_for_status() return dict(resp.json()) @@ -903,7 +932,7 @@ def list_channels() -> dict[str, object]: if gate is not None: return gate with _client() as http: - resp = http.get("/channels") + resp = http.get("/channels", headers=_agent_headers()) resp.raise_for_status() return {"channels": dict(resp.json().get("channels", {}))} @@ -1005,7 +1034,7 @@ def list_forms() -> dict[str, object]: if gate is not None: return gate with _client() as http: - resp = http.get("/forms") + resp = http.get("/forms", headers=_agent_headers()) resp.raise_for_status() return {"forms": list(resp.json().get("forms", []))} diff --git a/src/caucus/mcp_http.py b/src/caucus/mcp_http.py index 37c34bb..618ad97 100644 --- a/src/caucus/mcp_http.py +++ b/src/caucus/mcp_http.py @@ -83,6 +83,7 @@ session_expired_error, ) from .state import CapExceeded, RegisterOutcome +from .urlguard import ALLOW_REMOTE_ENV, needs_remote_optin logger = logging.getLogger("caucus.mcp_http") @@ -177,6 +178,12 @@ class _Membership: token_file: Path of the 0600 watcher token file written by :func:`watch_command`, cleaned up on :func:`leave`. ``None`` when none is live. + watch_ticket: The single outstanding remote watch ticket minted by + :func:`watch_command` for this session, or ``None`` when none is + live. Revoked (via :meth:`HubState.revoke_watch_ticket`) and + replaced on the next :func:`watch_command` refresh, and revoked + outright on :func:`leave` and in the dead-session sweep, so at + most one ticket for this session's token is ever redeemable. last_active: ``time.time()`` of this session's most recent tool call. Only load-bearing while the session is *unjoined*: it is the sole liveness signal such a record has (it owns no hub client), so the @@ -192,6 +199,7 @@ class _Membership: last_acked_seq: int = 0 listen_lease: str | None = None token_file: str | None = None + watch_ticket: str | None = None last_active: float = field(default_factory=time.time) @@ -479,6 +487,7 @@ def build_mcp_server( mcp_path: str = "/mcp", allowed_hosts: list[str] | None = None, allowed_origins: list[str] | None = None, + remote: bool = False, ) -> FastMCP: """Construct the in-process Streamable HTTP MCP server for the hub. @@ -499,6 +508,12 @@ def build_mcp_server( allowed_hosts: Extra ``Host`` allowlist entries for DNS-rebinding protection (typically the served ``host:port``). allowed_origins: Extra browser ``Origin`` allowlist entries. + remote: Whether this hub may be serving agents on other machines (a + non-loopback bind, or an operator-declared ``--public-url``). It + only changes how ``watch_command`` hands over the access token: a + path on the hub's filesystem means nothing to a remote agent, so it + gets a single-use ``--ticket`` it redeems for the token instead of + a token file. Returns: A configured :class:`FastMCP` ready to mount and run. @@ -513,7 +528,7 @@ def build_mcp_server( sessions: dict[str, _Membership] = {} def _sweep_dead_sessions(*, now: float | None = None) -> None: - """Remove per-session state and token files for sessions that are gone. + """Remove per-session state, token files and watch tickets for gone sessions. Called by the hub reaper on every sweep tick, right after :meth:`HubState.reap_stale`. Two kinds of corpse, because the two kinds @@ -550,6 +565,10 @@ def _sweep_dead_sessions(*, now: float | None = None) -> None: member = sessions.pop(sid, None) if member is not None: _remove_token_file(member.token_file) + # Mirror the leave() cleanup: a session reaped as dead must + # not leave its watch ticket outstanding either. + if member.watch_ticket is not None: + _hub.state.revoke_watch_ticket(member.watch_ticket) logger.debug("swept dead mcp-http session sid=%s", sid) # The connector is process-lived: created and entered lazily on first use @@ -570,6 +589,13 @@ async def _connector() -> HubConnector: _INTERNAL_BASE_URL, transport=httpx.ASGITransport(app=app), limits=_POOL_LIMITS, + # Every call re-enters the hub's real handler stack, agent + # key gate included, so this in-process client has to carry + # the key like any other. Read from the live config rather + # than the environment: --agent-key on the command line + # never sets CAUCUS_AGENT_KEY, and main() assigns this + # before the mount, so it is set by the time a tool runs. + agent_key=_hub.auth_config.agent, ) await existing.__aenter__() conn_holder["connector"] = existing @@ -871,6 +897,11 @@ async def leave(ctx: _Ctx) -> dict[str, object]: _hub.state.unregister(token) _remove_token_file(member.token_file) member.token_file = None + # A departing agent must not leave a live ticket redeemable for the + # token it just gave up. + if member.watch_ticket is not None: + _hub.state.revoke_watch_ticket(member.watch_ticket) + member.watch_ticket = None logger.info("left Caucus (was project=%s)", name) return {"left": True, "project": name} @@ -1219,12 +1250,44 @@ async def watch_command(ctx: _Ctx) -> dict[str, object]: assert member is not None if member.token is None: return {"error": "not_joined", "hint": "call join() first"} - # Drop any prior token file for this session before writing a fresh one. + # Drop any prior token file for this session; a remote agent gets none. _remove_token_file(member.token_file) - member.token_file = _write_token_file(member.token) + member.token_file = None # self_url, not the ASGITransport: the external watcher is a separate # process and needs a real reachable hub URL. - command = f"caucus-watch --hub {self_url} --token-file {member.token_file}" + if remote: + # The token file lives on the hub's filesystem, which is not the + # agent's, so its path would name nothing runnable. The peer token + # itself must not travel either: it is the room bearer for + # /receive, /send, /ack, /channels/*, /ask and /floor, and the + # whole point of the token file is to keep it out of argv and out + # of the launching transcript. So hand over a single-use, + # short-lived ticket the watcher exchanges for the token over one + # keyed call to /watch-ticket/redeem. + # + # caucus-watch runs the same fail-closed check on --hub that every + # other client does, and a plain-http URL to a non-loopback host is + # refused with exit 2 before the first poll. Handing the agent a + # command that dies instantly is worse than handing it none: it + # backgrounds it and believes a watcher is listening. So carry the + # opt-in the operator already made by advertising that URL. Asked + # without consulting this process's own environment, because the + # command runs in the agent's. + # + # The docstring below invites a refresh ("call anytime post-join + # to get or refresh"), and each refresh must retire the ticket it + # replaces: otherwise every call left one more live bearer + # credential outstanding for the same peer token, each already + # written into the agent's transcript. + if member.watch_ticket is not None: + _hub.state.revoke_watch_ticket(member.watch_ticket) + optin = f"{ALLOW_REMOTE_ENV}=1 " if needs_remote_optin(self_url) else "" + ticket = _hub.state.issue_watch_ticket(member.token) + member.watch_ticket = ticket + command = f"{optin}caucus-watch --hub {self_url} --ticket {ticket}" + else: + member.token_file = _write_token_file(member.token) + command = f"caucus-watch --hub {self_url} --token-file {member.token_file}" # No usage note here: the protocol already carries the run/relaunch/stop # rules verbatim, and repeating them on every call bought the agent # nothing it had not already read. diff --git a/src/caucus/models.py b/src/caucus/models.py index b2f1b47..18b0eb9 100644 --- a/src/caucus/models.py +++ b/src/caucus/models.py @@ -84,10 +84,12 @@ def request_carried_token(request: httpx.Request, token: str | None) -> bool: A hub ``401`` means "your session died" only when the call that earned it actually carried this session's token. The read-only endpoints - (``/protocol``, ``/peers``, ``/channels``, ``/ping``, ``/forms``) are - unauthenticated and can still answer ``401`` when an auth proxy sits in - front of the hub; calling that a lost membership would send the agent off - to re-``join`` a hub that never let it through in the first place. + (``/protocol``, ``/peers``, ``/channels``, ``/ping``, ``/forms``) never do: + they answer before a join, and the four the hub gates on a configured + agent key present that shared key instead. They can still answer ``401`` -- + on a wrong or missing agent key, or from an auth proxy in front of the hub + -- and calling either a lost membership would send the agent off to + re-``join`` a hub that never let it through in the first place. Both places a token can ride are checked: the ``Authorization: Bearer`` header the GET endpoints use, and the JSON body the POSTs use. @@ -544,6 +546,19 @@ class AckRequest(BaseModel): seq: int = PydField(ge=0) +class WatchTicketRedeemRequest(BaseModel): + """Body for ``POST /watch-ticket/redeem``. + + Spends the single-use ticket a remote ``watch_command`` handed out and + exchanges it for the peer's access token, so the token itself never travels + through the agent's transcript, shell history or process argv. The bound + matches the minted width (``secrets.token_urlsafe(24)``) with room to + spare, so a caller cannot park an oversized string in a request body. + """ + + ticket: str = PydField(min_length=1, max_length=128) + + class StatusRequest(BaseModel): """Body for ``POST /status``. diff --git a/src/caucus/setup_service.py b/src/caucus/setup_service.py index 9f3d32f..da48c25 100644 --- a/src/caucus/setup_service.py +++ b/src/caucus/setup_service.py @@ -44,6 +44,9 @@ import urllib.request from pathlib import Path from typing import Any, Literal +from urllib.parse import urlparse + +from .urlguard import is_loopback_host, validate_public_url DEFAULT_LABEL = "com.github.obeone.caucus-hub" DEFAULT_HOST = "127.0.0.1" @@ -54,14 +57,22 @@ #: instead of appending a second one, and leaves the operator's hooks alone. HOOK_MARKER = "[caucus-mcp:hub-ensure]" -#: Hosts where the hub's unauthenticated-by-default agent API is defensible. -LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1"}) +#: Bind addresses that mean "every interface", mirroring ``caucus.hub``. They +#: name no reachable address, so an install on one has to be told the URL agents +#: really dial (``--public-url``) or the hub itself refuses to start. +WILDCARD_HOSTS = frozenset({"0.0.0.0", "::"}) #: Tokens land in an XML plist and in a shell-sourced env file. Rather than #: escaping for both, restrict them to characters neither treats specially; #: every sane generator (``openssl rand -hex``, ``uuidgen``, base64url) fits. TOKEN_RE = re.compile(r"^[A-Za-z0-9._~-]+$") +#: Same idea for the values that are addresses rather than secrets: a public URL +#: and the ``Host`` allowlist entries travel through the same plist and env-file +#: plumbing, so they get a charset neither format treats specially. Wide enough +#: for ``https://hub.example.net:8443`` and ``[2001:db8::1]:8765``. +ADDRESS_RE = re.compile(r"^[A-Za-z0-9._~:/\[\]-]+$") + Platform = Literal["launchd", "systemd"] @@ -258,18 +269,27 @@ def resolve_binary(explicit: str | None = None) -> Path: return Path(found).resolve() -def validate_tokens(operator: str | None, observer: str | None) -> None: +def validate_tokens( + operator: str | None, observer: str | None, agent: str | None = None +) -> None: """Reject tokens carrying characters that would need escaping downstream. Args: operator: Read-write dashboard token, or ``None``. observer: Read-only dashboard token, or ``None``. + agent: Shared agent key guarding ``/register`` and ``/mcp``, or + ``None``. Travels through the same plist and env-file plumbing as + the two dashboard tokens, so it gets the same charset bound. Raises: - SetupError: When either token contains anything outside + SetupError: When any of them contains anything outside :data:`TOKEN_RE`. """ - for name, value in (("--operator-token", operator), ("--observer-token", observer)): + for name, value in ( + ("--operator-token", operator), + ("--observer-token", observer), + ("--agent-key", agent), + ): if value is not None and not TOKEN_RE.match(value): raise SetupError( f"{name} may only contain letters, digits and . _ ~ -\n" @@ -277,6 +297,43 @@ def validate_tokens(operator: str | None, observer: str | None) -> None: ) +def validate_addresses(public_url: str | None, allowed_hosts: list[str] | None) -> None: + """Reject advertised addresses that the unit templates could not carry. + + These are not secrets, but they travel the same route the tokens do — an XML + plist and a shell-sourced env file — so they get the same "nothing either + format treats specially" bound. The public URL is additionally held to the + hub's own rule (scheme plus host, no path), so a URL that would make the hub + refuse to start is caught while installing instead. + + Args: + public_url: Base URL agents will be told to reach the hub at, or + ``None``. + allowed_hosts: ``Host`` header values the ``/mcp`` guard should accept, + or ``None``. + + Raises: + SetupError: On a value outside :data:`ADDRESS_RE`, an allowed host + carrying a comma (the env form's separator), or a public URL the hub + would itself reject. + """ + for name, value in ( + ("--public-url", public_url), + *(("--allowed-host", h) for h in allowed_hosts or []), + ): + if value is None: + continue + if not ADDRESS_RE.match(value): + raise SetupError( + f"{name} {value!r} may only contain letters, digits and . _ ~ - : / [ ]" + ) + if public_url is not None: + try: + validate_public_url(public_url) + except ValueError as exc: + raise SetupError(str(exc)) from exc + + def check_port(port: int) -> None: """Reject a port the hub could never bind. @@ -299,31 +356,132 @@ def check_port(port: int) -> None: ) -def check_bind(host: str, operator_token: str | None) -> None: - """Refuse a network-visible bind that nobody can be kept out of. +def check_bind( + host: str, + operator_token: str | None, + agent_key: str | None, + public_url: str | None = None, +) -> None: + """Refuse a network-visible bind the installed hub could not serve safely. - The hub serves its agent API unauthenticated by default, which is + The hub serves both its doors unauthenticated by default, which is defensible precisely because it binds to loopback. Bound wider, any browser - that reaches the port gets full operator rights: pause, stop, kick. + that reaches the port gets full operator rights (pause, stop, kick) and any + client that reaches it can register as a peer and read the room. The + operator token only ever guarded the first of those, so it is demanded here + together with the agent key. + + A wildcard bind adds a third requirement, for the same reason ``caucus-hub`` + demands it at startup: ``0.0.0.0`` is not an address anything dials, so + without ``--public-url`` the hub advertises its ``127.0.0.1`` rewrite to + agents on other machines. Refusing here rather than at first start means the + operator finds out while installing, not from a unit that loads and then + exits. + + A loopback bind behind a non-loopback ``--public-url`` is the same exposure + reached a different way -- a tunnel or a reverse proxy carries the world to + a socket that never left this machine -- and ``caucus-hub`` refuses it at + startup for that reason. Refusing it here too is what keeps the installer + from writing a unit that cannot start. Args: host: Address the hub would bind to. operator_token: Token that would gate operator access, if any. + agent_key: Shared key that would gate ``/register`` and ``/mcp``, if + any. + public_url: Base URL agents would be told to reach the hub at, if any. Raises: - SetupError: For a non-loopback host with no operator token. + SetupError: For a non-loopback host missing a credential, a wildcard + host with no public URL, or a loopback host advertised under a + non-loopback public URL without both credentials. """ - if host in LOOPBACK_HOSTS or operator_token: + advertised_host = urlparse(public_url).hostname if public_url else None + advertised_remote = advertised_host is not None and not is_loopback_host( + advertised_host + ) + if is_loopback_host(host) and not advertised_remote: return + if operator_token and agent_key and (public_url or host not in WILDCARD_HOSTS): + return + wildcard = host in WILDCARD_HOSTS + if is_loopback_host(host): + raise SetupError( + f"refusing to advertise {public_url} without --operator-token and\n" + "--agent-key. The bind stays on loopback, but a public URL says\n" + "agents on other machines dial this hub, and whatever carries them\n" + "in reaches a dashboard that grants full operator rights to any\n" + "browser and a /register any client can walk through.\n" + "Drop --public-url, or run:\n" + f" caucus-setup-service --host {host} \\\n" + ' --operator-token "$(openssl rand -hex 24)" \\\n' + ' --agent-key "$(openssl rand -hex 24)" \\\n' + f" --public-url {public_url}" + ) raise SetupError( - f"refusing to bind {host} without --operator-token.\n" - "On a non-loopback address the dashboard accepts any browser that can\n" - "reach it, with full operator rights. Keep 127.0.0.1, or run:\n" - f' caucus-setup-service --host {host} ' - '--operator-token "$(openssl rand -hex 24)"' + f"refusing to bind {host} without --operator-token and --agent-key" + + (" and --public-url.\n" if wildcard else ".\n") + + "On a non-loopback address the dashboard accepts any browser that can\n" + "reach it, with full operator rights, and any client that reaches the\n" + "port can join the caucus and read everything said in it.\n" + + ( + "A wildcard bind also names no address to hand agents, so they would\n" + "be told to reach this hub at 127.0.0.1 -- their own machine.\n" + if wildcard + else "" + ) + + "Keep 127.0.0.1, or run:\n" + f" caucus-setup-service --host {host} \\\n" + ' --operator-token "$(openssl rand -hex 24)" \\\n' + ' --agent-key "$(openssl rand -hex 24)"' + + (" \\\n --public-url https://hub.example.net" if wildcard else "") ) +def service_environment( + operator_token: str | None = None, + observer_token: str | None = None, + agent_key: str | None = None, + public_url: str | None = None, + allowed_hosts: list[str] | None = None, + mcp_http: bool = False, +) -> list[tuple[str, str]]: + """Build the environment the installed hub reads its configuration from. + + Every setting the service needs beyond ``--host``/``--port`` travels as an + environment variable rather than a command-line flag, because both unit + formats already have a place for one (the plist's ``EnvironmentVariables``, + systemd's ``EnvironmentFile``) and neither has a place for a variable-length + argument list. ``caucus-hub`` reads all six as the documented fallback for + the matching flag. + + Args: + operator_token: Read-write dashboard token, or ``None``. + observer_token: Read-only dashboard token, or ``None``. + agent_key: Shared key guarding ``/register`` and ``/mcp``, or ``None``. + public_url: Base URL agents are told to reach the hub at, or ``None``. + allowed_hosts: Extra ``Host`` values the ``/mcp`` DNS-rebinding guard + accepts; joined with commas, which is the form the hub splits on. + mcp_http: Force the in-process MCP endpoint on. Needed on a non-loopback + bind, where it is off by default. + + Returns: + The ``(name, value)`` pairs to write, in a stable order, with every + unset one dropped. + """ + pairs = [ + ("CAUCUS_OPERATOR_TOKEN", operator_token), + ("CAUCUS_OBSERVER_TOKEN", observer_token), + ("CAUCUS_AGENT_KEY", agent_key), + ("CAUCUS_PUBLIC_URL", public_url), + ("CAUCUS_ALLOWED_HOSTS", ",".join(allowed_hosts or []) or None), + # Only ever written as the opt-in: absent means "let the hub decide", + # which is on for loopback and off elsewhere. + ("CAUCUS_MCP_HTTP", "1" if mcp_http else None), + ] + return [(name, value) for name, value in pairs if value] + + def render_unit( *, kind: Platform, @@ -335,6 +493,10 @@ def render_unit( at_login: bool = False, operator_token: str | None = None, observer_token: str | None = None, + agent_key: str | None = None, + public_url: str | None = None, + allowed_hosts: list[str] | None = None, + mcp_http: bool = False, ) -> str: """Render the service definition for ``kind``. @@ -349,6 +511,12 @@ def render_unit( operator_token: Embedded in the plist; systemd reads it from :func:`env_file_path` instead. observer_token: Same treatment as ``operator_token``. + agent_key: Shared key guarding ``/register`` and ``/mcp``; same + treatment as ``operator_token``. + public_url: Base URL advertised to agents; same treatment. + allowed_hosts: Extra ``Host`` values for the ``/mcp`` guard; same + treatment. + mcp_http: Force the in-process MCP endpoint on; same treatment. Returns: The complete file contents, ready to write. @@ -363,12 +531,15 @@ def render_unit( ) environment = "" - for name, value in ( - ("CAUCUS_OPERATOR_TOKEN", operator_token), - ("CAUCUS_OBSERVER_TOKEN", observer_token), + for name, value in service_environment( + operator_token, + observer_token, + agent_key, + public_url, + allowed_hosts, + mcp_http, ): - if value: - environment += f" {name}\n {value}\n" + environment += f" {name}\n {value}\n" return LAUNCHD_TEMPLATE.format( label=label, @@ -569,24 +740,39 @@ def apply_hook(path: Path, command: str) -> dict[str, object]: return {"changed": True, "path": str(path), "action": action} -def write_env_file(operator: str | None, observer: str | None) -> Path | None: - """Write the systemd token file, or return ``None`` when there is nothing to. +def write_env_file( + operator: str | None, + observer: str | None, + agent: str | None = None, + public_url: str | None = None, + allowed_hosts: list[str] | None = None, + mcp_http: bool = False, +) -> Path | None: + """Write the systemd environment file, or ``None`` when there is nothing to. + + The launchd plist carries these inline; systemd reads them from this file, + which is why both paths go through :func:`service_environment` rather than + each listing the variables itself. Args: operator: Read-write dashboard token, or ``None``. observer: Read-only dashboard token, or ``None``. + agent: Shared key guarding ``/register`` and ``/mcp``, or ``None``. + public_url: Base URL advertised to agents, or ``None``. + allowed_hosts: Extra ``Host`` values for the ``/mcp`` guard, or ``None``. + mcp_http: Force the in-process MCP endpoint on. Returns: - The path written, or ``None`` when no token was supplied. + The path written, or ``None`` when nothing had to be configured. """ - if not operator and not observer: + env = service_environment( + operator, observer, agent, public_url, allowed_hosts, mcp_http + ) + if not env: return None path = env_file_path() lines = ["# Written by caucus-setup-service. Read by the systemd user unit."] - if operator: - lines.append(f"CAUCUS_OPERATOR_TOKEN={operator}") - if observer: - lines.append(f"CAUCUS_OBSERVER_TOKEN={observer}") + lines.extend(f"{name}={value}" for name, value in env) _atomic_write(path, "\n".join(lines) + "\n") return path @@ -690,6 +876,10 @@ def describe_plan( operator_token: str | None, hook_path: Path | None, hook_action: str, + agent_key: str | None = None, + public_url: str | None = None, + allowed_hosts: list[str] | None = None, + mcp_http: bool = False, ) -> str: """Build the human-readable summary shown before anything is written. @@ -705,6 +895,11 @@ def describe_plan( hook_path: Settings file the hook goes into, or ``None`` when the operator declined it. hook_action: What would happen to that file. + agent_key: Present when the agent door will be gated by a shared key. + public_url: Present when agents will be told a different address than + the bind one. + allowed_hosts: Extra ``Host`` values the ``/mcp`` guard will accept. + mcp_http: Whether the in-process MCP endpoint is forced on. Returns: A multi-line block, ending without a trailing newline. @@ -714,6 +909,7 @@ def describe_plan( if at_login else "on demand, when an agent session opens" ) + agent_access = "shared agent key required" if agent_key else "open (loopback only)" lines = [ "", f"Caucus hub as a {kind} service. Here is what will happen:", @@ -722,7 +918,14 @@ def describe_plan( f" runs {binary} on {host}:{port}, logging to {logfile}", f" starts {starts}", f" access {'operator token required' if operator_token else 'open (loopback only)'}", + f" agents {agent_access}", ] + if public_url: + lines.append(f" advertise {public_url} (what agents are told to dial)") + if allowed_hosts: + lines.append(f" hosts /mcp also accepts {', '.join(allowed_hosts)}") + if mcp_http: + lines.append(" mcp in-process /mcp endpoint forced on") if hook_path is not None: verb = {"created": "create", "updated": "update", "unchanged": "leave"} lines.append( @@ -841,6 +1044,37 @@ def _build_parser() -> argparse.ArgumentParser: parser.add_argument( "--observer-token", help="token for read-only dashboard access" ) + parser.add_argument( + "--agent-key", + help="shared key agents must send to join (guards /register and /mcp)", + ) + parser.add_argument( + "--public-url", + metavar="URL", + help=( + "base URL other machines reach this hub at, e.g. " + "https://hub.example.net. Required with --host 0.0.0.0, which names " + "no address the hub could advertise on its own" + ), + ) + parser.add_argument( + "--allowed-host", + action="append", + default=None, + metavar="HOST", + help=( + "extra Host header value the /mcp DNS-rebinding guard accepts " + "(repeatable), e.g. --allowed-host hub.lan" + ), + ) + parser.add_argument( + "--mcp-http", + action="store_true", + help=( + "serve the in-process /mcp endpoint. On by default for a loopback " + "bind and off elsewhere, so this is the opt-in a remote hub needs" + ), + ) parser.add_argument( "--at-login", action="store_true", @@ -896,9 +1130,10 @@ def main(argv: list[str] | None = None) -> int: print("Your log file, token file and SessionStart hook were left alone.") return 0 - validate_tokens(args.operator_token, args.observer_token) + validate_tokens(args.operator_token, args.observer_token, args.agent_key) + validate_addresses(args.public_url, args.allowed_host) check_port(args.port) - check_bind(args.host, args.operator_token) + check_bind(args.host, args.operator_token, args.agent_key, args.public_url) binary = resolve_binary(args.binary) logfile = ( Path(args.log_file).expanduser() if args.log_file else default_log_path(kind) @@ -914,6 +1149,10 @@ def main(argv: list[str] | None = None) -> int: at_login=args.at_login, operator_token=args.operator_token, observer_token=args.observer_token, + agent_key=args.agent_key, + public_url=args.public_url, + allowed_hosts=args.allowed_host, + mcp_http=args.mcp_http, ) command = hook_command(kind, args.label) @@ -940,6 +1179,10 @@ def main(argv: list[str] | None = None) -> int: operator_token=args.operator_token, hook_path=hook_path, hook_action=hook_action, + agent_key=args.agent_key, + public_url=args.public_url, + allowed_hosts=args.allowed_host, + mcp_http=args.mcp_http, ) ) @@ -955,7 +1198,14 @@ def main(argv: list[str] | None = None) -> int: _atomic_write(unit, rendered) if kind == "systemd": - write_env_file(args.operator_token, args.observer_token) + write_env_file( + args.operator_token, + args.observer_token, + args.agent_key, + args.public_url, + args.allowed_host, + args.mcp_http, + ) if hook_path is not None: apply_hook(hook_path, command) load_service(kind, unit, at_login=args.at_login, label=args.label) diff --git a/src/caucus/state.py b/src/caucus/state.py index 4187701..ae4799e 100644 --- a/src/caucus/state.py +++ b/src/caucus/state.py @@ -158,6 +158,20 @@ class CapExceeded(Exception): MAX_LEASE_ID_CHARS = 64 """Longest accepted ``/receive`` lease id, so a peer cannot park junk in state.""" +# A remote agent cannot be handed a token file (the path names nothing on its +# machine), and handing it the peer token itself puts the room bearer in its +# transcript, its shell history and the watcher's environ. So ``watch_command`` +# mints a *ticket* instead: an opaque one-shot claim check the watcher exchanges +# for the real token over one keyed HTTP call, seconds after it is issued. + +WATCH_TICKET_TTL = 120.0 +"""Seconds a minted watch ticket stays redeemable. + +Long enough for an agent to background the command it was just handed, short +enough that a ticket leaked into a transcript is worthless by the time anyone +reads it. Single use on top of that: the first redemption consumes it. +""" + @dataclass(slots=True) class PollLease: @@ -441,6 +455,10 @@ def __init__( # send to that scope (see :meth:`floor_blocks`). In-memory only. self._floors: dict[str, Floor] = {} # scope -> Floor self._forms: dict[str, Form] = {} # form id -> pending Form + # Outstanding single-use watch tickets: ticket -> (peer token, expiry). + # Minted by ``watch_command`` on a remote hub and consumed by the very + # next ``POST /watch-ticket/redeem``; see :meth:`issue_watch_ticket`. + self._watch_tickets: dict[str, tuple[str, float]] = {} # Last time a contested-join notice fired, per contested project name. # Repeats inside CONTESTED_NOTICE_WINDOW are silently swallowed so a # register retry loop cannot flood the operator feed. Entries are @@ -704,6 +722,102 @@ def client_for(self, token: str) -> Client | None: return self._revive(reaped) return None + # --- watch tickets --------------------------------------------------- + + def _prune_watch_tickets(self, ref: float) -> None: + """Forget every watch ticket whose TTL has lapsed. + + Called on every mint, on every redemption and from the idle-reaper + sweep, so the store is bounded by the tickets issued inside one TTL + window rather than by the process lifetime. + + Args: + ref: Reference timestamp to measure expiry against. + """ + for ticket in [t for t, (_, exp) in self._watch_tickets.items() if exp <= ref]: + self._watch_tickets.pop(ticket, None) + + def issue_watch_ticket(self, peer_token: str, *, now: float | None = None) -> str: + """Mint a single-use, short-lived claim check for ``peer_token``. + + The ticket is what a *remote* watcher is handed in place of the peer + token: it grants nothing by itself, is spent by the first redemption, + and dies on its own after :data:`WATCH_TICKET_TTL` seconds. Nothing + here validates ``peer_token``: redemption just hands the token back + unchanged, and what it is then worth is entirely up to ``client_for``, + exactly as if it had been presented directly. A token whose peer is + still on the roster, or was idle-reaped but is still inside its + ``reaped_grace`` window, revives the peer on the first authenticated + call that carries it: that is ``client_for``'s ordinary resurrection + behaviour, not something a ticket adds or bypasses. Only a token the + hub has truly forgotten (never issued, or past ``reaped_grace``) is + rejected. + + Args: + peer_token: The access token the ticket will be exchanged for. + now: Reference timestamp (defaults to :func:`time.time`); + injectable for deterministic tests. + + Returns: + The freshly minted ticket string. + """ + ref = time.time() if now is None else now + self._prune_watch_tickets(ref) + ticket = secrets.token_urlsafe(24) + self._watch_tickets[ticket] = (peer_token, ref + WATCH_TICKET_TTL) + return ticket + + def revoke_watch_ticket(self, ticket: str) -> None: + """Invalidate one outstanding watch ticket before it is ever redeemed. + + Used wherever a ticket is superseded before anyone spends it: minting + a fresh one for the same member (``watch_command()`` advertises "call + again to refresh", and without this every earlier ticket stayed + redeemable for its full :data:`WATCH_TICKET_TTL`, so N calls left N + live bearer credentials for the same peer token), on an explicit + ``leave()``, and in the dead-session sweep. A no-op when ``ticket`` is + already spent, expired, or was never issued, so callers can revoke + whatever they last minted without first checking it is still there. + + Args: + ticket: The ticket to invalidate. + """ + self._watch_tickets.pop(ticket, None) + + def redeem_watch_ticket( + self, ticket: str, *, now: float | None = None + ) -> str | None: + """Spend a watch ticket and return the peer token it stood for. + + Strictly single use: a match is deleted before it is returned, so a + replay of the same ticket (a second watcher, or anyone who read it out + of a transcript) gets nothing. Unknown and expired tickets are + indistinguishable to the caller, by design. + + The lookup walks the (small, TTL-bounded) store comparing with + :func:`secrets.compare_digest` on UTF-8 bytes rather than hashing the + string into a dict: same constant-time discipline the hub's other + credentials use, and bytes because ``compare_digest`` raises on a + ``str`` holding anything outside ASCII. + + Args: + ticket: The ticket presented by the caller. + now: Reference timestamp (defaults to :func:`time.time`); + injectable for deterministic tests. + + Returns: + The peer token, or ``None`` when the ticket is unknown, already + spent, or past its TTL. + """ + ref = time.time() if now is None else now + self._prune_watch_tickets(ref) + presented = ticket.encode("utf-8") + for candidate, (peer_token, _) in list(self._watch_tickets.items()): + if secrets.compare_digest(presented, candidate.encode("utf-8")): + del self._watch_tickets[candidate] + return peer_token + return None + def acquire_poll_lease(self, client: Client, lease_id: str) -> PollLease | None: """Claim the single ``/receive`` consumer slot for ``lease_id``. @@ -912,7 +1026,8 @@ def reap_stale(self, ttl: float, *, now: float | None = None) -> list[str]: Each reaped peer is announced to the UI and parked in the revival graveyard (``revivable=True``) so it can be resurrected by any later authenticated call. The same sweep also forgets graveyard entries whose - :attr:`reaped_grace` window has lapsed — those tokens are dead for good. + :attr:`reaped_grace` window has lapsed (those tokens are dead for good), + and drops watch tickets past :data:`WATCH_TICKET_TTL`. Args: ttl: Maximum idle time, in seconds, before a client is reaped. @@ -938,6 +1053,10 @@ def reap_stale(self, ttl: float, *, now: float | None = None) -> list[str]: expired = self._reaped.pop(token, None) if expired is not None: self._reaped_by_project.pop(expired.project, None) + # Piggyback the watch-ticket expiry on the sweep the hub already runs: + # a ticket nobody ever redeems would otherwise sit in memory until the + # next mint happened to prune it. + self._prune_watch_tickets(ref) return [c.project for c in stale] def ack(self, token: str, seq: int) -> bool: diff --git a/src/caucus/urlguard.py b/src/caucus/urlguard.py index 96ea05b..b2679b0 100644 --- a/src/caucus/urlguard.py +++ b/src/caucus/urlguard.py @@ -13,6 +13,9 @@ The destination is operator-set configuration (never runtime-untrusted input), so this guards an honest misconfiguration rather than an attacker — but it makes the localhost-only intent explicit in code and keeps the token on-box by default. + +:func:`validate_public_url` is the server-side counterpart: it checks the origin +the hub *advertises* to agents (``--public-url``) is a bare, usable base URL. """ from __future__ import annotations @@ -31,18 +34,54 @@ ALLOW_REMOTE_ENV = "CAUCUS_ALLOW_REMOTE_HUB" -def _is_loopback(host: str) -> bool: - """Return whether ``host`` is a loopback hostname or IP address.""" +def is_loopback_host(host: str) -> bool: + """Return whether ``host`` is a loopback hostname or IP address. + + The single definition of "loopback" for the whole package: the client-side + hub-URL guard below, the hub's own bind gate, the service installer's + refusal, and the autostart probe all call this, so ``127.0.0.2`` and + ``[::1]`` cannot be loopback in one place and remote in another. Anything + that is neither ``localhost`` nor a numeric loopback address (the whole of + ``127.0.0.0/8`` and ``::1``) is treated as remote — including the wildcard + binds ``0.0.0.0`` and ``::``, which are *not* addresses one connects to. + + Args: + host: A hostname or IP literal, with or without IPv6 brackets. + + Returns: + ``True`` when a connection to ``host`` cannot leave this machine. + """ if host.lower() in _LOOPBACK_HOSTNAMES: return True try: # Strip IPv6 brackets if a netloc form slipped through (urlparse already - # removes them for .hostname, but be defensive). + # removes them for .hostname, but callers hand us raw CLI values too). return ipaddress.ip_address(host.strip("[]")).is_loopback except ValueError: return False +def needs_remote_optin(url: str) -> bool: + """Return whether ``url`` is the plain-http-off-box shape that needs opt-in. + + Answers the question :func:`validate_hub_url` asks *before* it consults the + environment, so a caller can tell that a URL will be refused on a machine + that has not set :data:`ALLOW_REMOTE_ENV` — even when this process happens + to have set it. The hub uses that to prefix the ``caucus-watch`` command it + hands a remote agent, which runs in somebody else's environment. + + Args: + url: A hub base URL. + + Returns: + ``True`` when the URL is plain ``http`` to a non-loopback host. + """ + parsed = urlparse(url) + return parsed.scheme.lower() == "http" and not is_loopback_host( + parsed.hostname or "" + ) + + def validate_hub_url(url: str) -> str: """Validate a configured hub URL, returning it unchanged when safe. @@ -69,7 +108,7 @@ def validate_hub_url(url: str) -> str: raise ValueError( f"unsupported hub URL scheme {scheme!r} in {url!r} (expected http or https)" ) - if scheme == "https" or _is_loopback(host): + if not needs_remote_optin(url): return url if os.environ.get(ALLOW_REMOTE_ENV, "").strip().lower() in _TRUTHY: return url @@ -78,3 +117,49 @@ def validate_hub_url(url: str) -> str: f"token and message content would be sent in cleartext. Use https, a " f"loopback host, or set {ALLOW_REMOTE_ENV}=1 to override." ) + + +def validate_public_url(url: str) -> str: + """Validate the hub's advertised public base URL, returning it normalised. + + This is the *server* side of the same configuration knob + :func:`validate_hub_url` guards on the client side: the address the hub + hands out so an agent on another machine can reach it (``watch_command``'s + ``caucus-watch --hub ...``, the ``hub`` field of every tool result). It must + therefore be a bare origin — scheme, host, optional port — because the hub + appends its own paths to it. The cleartext opt-in of + :func:`validate_hub_url` is deliberately *not* applied here: this URL is the + operator describing their own deployment, not a client being pointed + off-box, and it is the clients reading it that re-run that check. + + Args: + url: The operator-supplied base URL (``--public-url`` / + ``CAUCUS_PUBLIC_URL``). + + Returns: + The URL with any trailing ``/`` removed, ready to concatenate paths to. + + Raises: + ValueError: When the scheme is not http/https, the host is missing, or + anything follows the origin (path, query, fragment, params). + """ + parsed = urlparse(url) + scheme = parsed.scheme.lower() + if scheme not in ("http", "https"): + raise ValueError( + f"unsupported public URL scheme {scheme!r} in {url!r} " + "(expected http or https)" + ) + if not parsed.hostname: + raise ValueError( + f"public URL {url!r} names no host (expected e.g. https://hub.example.net)" + ) + # A bare origin only: the hub appends "/receive", "/mcp", … to this value, + # so a path prefix would silently produce unreachable URLs. "/" is the empty + # path spelled out and is accepted (and stripped). + if parsed.path not in ("", "/") or parsed.query or parsed.fragment or parsed.params: + raise ValueError( + f"public URL {url!r} must be a bare origin (scheme://host[:port]) " + "with no path, query or fragment" + ) + return url.rstrip("/") diff --git a/src/caucus/watch.py b/src/caucus/watch.py index 7df32e7..6a4c90a 100644 --- a/src/caucus/watch.py +++ b/src/caucus/watch.py @@ -40,9 +40,31 @@ * ``--hub`` / ``CAUCUS_HUB_URL`` -- hub base URL (default ``http://127.0.0.1:8765``). -* The access token (required), resolved by precedence: ``--token`` (explicit) > - ``--token-file`` (a path holding the token -- keeps it out of the process - argv and the launching transcript) > ``CAUCUS_TOKEN``. +* A credential (required), resolved by one precedence chain, flags before + environment: ``--token`` > ``--token-file`` > ``--ticket`` > ``CAUCUS_TOKEN`` + > ``CAUCUS_TICKET``. The first three are the launcher's explicit choice; the + last two are ambient. + + - ``--token`` is the raw access token, put directly into the process argv + (``--token-file`` below exists precisely to avoid that for the loopback + case). + - ``--token-file`` is a path holding the token; it keeps the secret out of + argv and out of the launching transcript, which is why a loopback + ``watch_command()`` emits this form. + - ``--ticket`` / ``CAUCUS_TICKET`` is a **single-use, short-lived** claim + check a *remote* ``watch_command()`` hands out instead: the token file's + path means nothing on the agent's machine, and the token itself must not + travel through the agent's transcript. Like ``--token``, the ticket does + land in argv, so any other local uid on the watcher's machine can read it + with ``ps`` for as long as it stays redeemable. That exposure is accepted + rather than engineered away (moving it to stdin would complicate the + backgrounded command for a credential that is already single-use and + short-lived): the window is bounded by the same single use and by + :data:`caucus.state.WATCH_TICKET_TTL`, so a ``ps`` snoop gets at most one + exchange, and only within the ticket's short life, never the room bearer + itself. The watcher spends the ticket once at startup against + ``POST /watch-ticket/redeem`` (presenting ``CAUCUS_AGENT_KEY`` when the + hub is keyed) and then polls exactly as it would with a direct token. * ``--timeout`` -- per-poll long-poll ceiling in seconds (default ``25``). """ @@ -59,11 +81,16 @@ import httpx from . import __version__ +from .hub_connector import AGENT_KEY_ENV from .logging_setup import configure_logging from .urlguard import validate_hub_url logger = logging.getLogger("caucus.watch") +# One short POST, before the loop starts: a hub that cannot answer a ticket +# redemption in this long is not going to serve a 25s long-poll either. +_REDEEM_TIMEOUT = 10.0 + # Seconds added to the per-poll timeout to size the HTTP client ceiling, so the # server long-poll always returns before httpx gives up (mirrors the bridge's # server-poll < client-timeout ordering). @@ -268,31 +295,91 @@ def watch(hub: str, token: str, timeout: float) -> int: return 0 -def _resolve_token(token: str | None, token_file: str | None) -> str | None: - """Resolve the access token by precedence: flag, then file, then env. +def _resolve_credential( + token: str | None, token_file: str | None, ticket: str | None +) -> tuple[str | None, str | None]: + """Resolve the watcher's one credential, flags before environment. - The token-file form lets the launcher keep the secret out of the process - argv and its own transcript -- the command references only a path. + Precedence, highest first: ``--token``, ``--token-file``, ``--ticket``, + ``CAUCUS_TOKEN``, ``CAUCUS_TICKET``. The historical order between the three + token forms is untouched; the ticket slots in where the module docstring's + "flags win over environment" rule puts it, after the explicit token flags + and ahead of the ambient env vars. At most one of the two results is ever + set: a direct token is used as is, a ticket is exchanged for one. Args: token: Value of ``--token`` (or ``None``). token_file: Value of ``--token-file`` (or ``None``). + ticket: Value of ``--ticket`` (or ``None``). Returns: - The resolved token, or ``None`` if none was supplied. + A ``(token, ticket)`` pair; ``(None, None)`` when nothing was supplied. Raises: OSError: If ``token_file`` is given but cannot be read. """ if token: - return token + return token, None if token_file: - return Path(token_file).read_text(encoding="utf-8").strip() - return os.environ.get("CAUCUS_TOKEN") + return Path(token_file).read_text(encoding="utf-8").strip(), None + if ticket: + return None, ticket + env_token = os.environ.get("CAUCUS_TOKEN") + if env_token: + return env_token, None + return None, os.environ.get("CAUCUS_TICKET") or None + + +def redeem_ticket(hub: str, ticket: str) -> str | None: + """Exchange a single-use watch ticket for this peer's access token. + + One POST, once, before the first poll. The agent key is read straight from + the watcher's own environment (the same ``CAUCUS_AGENT_KEY`` the connector + reads) because a keyed hub gates this endpoint like ``/register``; on an + unkeyed hub the header is simply absent. + + Args: + hub: Hub base URL (no trailing slash required). + ticket: The ticket to spend. + + Returns: + The peer access token, or ``None`` when the hub refused the ticket or + could not be reached (both are fatal for this process, and both are + answered by asking the agent for a fresh ``watch_command()``). + """ + headers = {} + agent_key = os.environ.get(AGENT_KEY_ENV) + if agent_key: + headers["Authorization"] = f"Bearer {agent_key}" + try: + with httpx.Client(base_url=hub.rstrip("/"), timeout=_REDEEM_TIMEOUT) as http: + resp = http.post( + "/watch-ticket/redeem", json={"ticket": ticket}, headers=headers + ) + except httpx.HTTPError as exc: + logger.error("could not reach the hub to redeem the watch ticket: %s", exc) + return None + if resp.status_code >= 400: + # Never log the ticket itself, only what the hub made of it. + logger.error("hub refused the watch ticket (HTTP %s)", resp.status_code) + return None + try: + token = resp.json().get("token") + except ValueError as exc: # pragma: no cover - a proxy returning non-JSON + logger.error("watch-ticket redemption returned a non-JSON body: %s", exc) + return None + return str(token) if token else None def main() -> None: - """CLI entry point: parse config and run the watch loop until it exits.""" + """CLI entry point: parse config and run the watch loop until it exits. + + Resolves the one credential by the precedence documented on + :func:`_resolve_credential`, redeeming a ticket for a token first when that + is the form supplied. Exits ``1`` on a refused or unredeemable ticket, after + printing the actionable remedy to stdout (the agent is woken by this + process exiting and reads what it left there, not the stderr log). + """ parser = argparse.ArgumentParser( prog="caucus-watch", description="Zero-token Caucus inbound-message watcher (long-poll loop).", @@ -317,6 +404,14 @@ def main() -> None: default=None, help="Path to a file holding the token; keeps it out of argv/transcript.", ) + parser.add_argument( + "--ticket", + default=None, + help=( + "Single-use, short-lived ticket to exchange for the token" + " (what a remote watch_command() hands out)." + ), + ) parser.add_argument( "--timeout", type=float, @@ -335,11 +430,29 @@ def main() -> None: configure_logging(sys.stderr) try: - token = _resolve_token(args.token, args.token_file) + token, ticket = _resolve_credential(args.token, args.token_file, args.ticket) except OSError as exc: parser.error(f"could not read --token-file: {exc}") + if token is None and ticket is not None: + token = redeem_ticket(args.hub, ticket) + if token is None: + # The agent is woken by this process EXITING and reads stdout, so + # the remedy has to be there, not only in the stderr log line + # redeem_ticket already wrote. Without it a rejected ticket looks + # like the watcher dying for no reason. + _emit( + "[caucus] TICKET REJECTED -- the hub would not exchange this" + " watch ticket for a token. A ticket is single-use and lives" + " about two minutes, so a reused or stale one is refused." + " Call watch_command() for a fresh command and relaunch." + " Watcher exiting." + ) + sys.exit(1) if not token: - parser.error("a token is required (--token, --token-file, or CAUCUS_TOKEN)") + parser.error( + "a credential is required (--token, --token-file, --ticket," + " CAUCUS_TOKEN, or CAUCUS_TICKET)" + ) try: sys.exit(watch(args.hub, token, args.timeout)) diff --git a/tests/test_agent_key.py b/tests/test_agent_key.py new file mode 100644 index 0000000..69f4e2b --- /dev/null +++ b/tests/test_agent_key.py @@ -0,0 +1,546 @@ +"""Tests for the shared agent key guarding ``/register`` and ``/mcp``. + +The key is the door a hub reachable from other machines needs: without it, any +client that can reach the port joins the caucus. It is deliberately independent +of the operator/observer console tokens, so this module checks both that the +gate closes when a key is configured and that nothing changes when it is not. + +Covered here: + +- ``AuthConfig.agent_ok`` in isolation (open when unset, exact match otherwise). +- ``POST /register``: open with no key; accepted with the right key; 401 on a + wrong key, a missing header and a malformed one. +- The 401 fires *before* the per-host register throttle, so a bogus key cannot + drain another caller's budget. +- ``/mcp``: 401 on a wrong key and on a missing header, CORS preflight still + answered, and the full in-process ``HubConnector`` tool path still works when + the right key rides in the header. +- :class:`caucus.hub_connector.HubConnector` and :mod:`caucus.mcp_bridge` send + the key on ``/register`` only. +""" + +from __future__ import annotations + +import socket +import threading +import time +from collections.abc import Iterator + +import httpx +import pytest +import uvicorn +from fastapi.testclient import TestClient +from mcp.client.session import ClientSession +from mcp.client.streamable_http import streamablehttp_client + +import caucus.hub as hub_module +import caucus.hub_connector as connector_module +import caucus.mcp_bridge as bridge_module +from caucus.hub import AuthConfig +from caucus.hub_connector import HubConnector +from caucus.state import HubState + +AGENT_KEY = "shared-agent-key" + + +@pytest.fixture +def with_agent_key(monkeypatch: pytest.MonkeyPatch) -> None: + """Configure only the agent key on the hub for the test's duration. + + Operator/observer stay unset on purpose: the console credentials and the + agent door are independent axes and must not be needed for one another. + """ + monkeypatch.setattr(hub_module, "auth_config", AuthConfig(agent=AGENT_KEY)) + + +def _free_port() -> int: + """Grab an ephemeral TCP port the OS just confirmed is free.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + return int(sock.getsockname()[1]) + + +@pytest.fixture +def mcp_hub(monkeypatch: pytest.MonkeyPatch) -> Iterator[tuple[str, HubState]]: + """Boot the hub on a real socket with the ``/mcp`` endpoint mounted. + + Mirrors :mod:`tests.test_mcp_http_integration`'s fixture: ``hub.main()``'s + wiring via :func:`hub._mount_mcp_http` plus a real uvicorn server, so the + hub lifespan runs the MCP session manager. The appended route and the + ``_mcp_server`` global are torn down afterwards so the shared import-time + app is left pristine for other tests. + """ + fresh = HubState() + monkeypatch.setattr(hub_module, "state", fresh) + port = _free_port() + routes_before = len(hub_module.app.router.routes) + hub_module._mount_mcp_http( + host="127.0.0.1", port=port, mcp_path="/mcp", extra_origins=set() + ) + config = uvicorn.Config( + hub_module.app, host="127.0.0.1", port=port, log_level="warning" + ) + server = uvicorn.Server(config) + thread = threading.Thread(target=server.run, daemon=True) + thread.start() + deadline = time.monotonic() + 5.0 + while not server.started and time.monotonic() < deadline: + time.sleep(0.02) + if not server.started: # pragma: no cover - startup failure + raise RuntimeError("hub server failed to start in time") + try: + yield f"http://127.0.0.1:{port}", fresh + finally: + server.should_exit = True + thread.join(timeout=5.0) + del hub_module.app.router.routes[routes_before:] + hub_module._mcp_server = None + + +# --------------------------------------------------------------------------- +# AuthConfig.agent_ok +# --------------------------------------------------------------------------- + + +def test_agent_ok_open_when_unset() -> None: + """With no key configured every caller passes, including a keyless one.""" + cfg = AuthConfig() + assert cfg.agent_ok(None) is True + assert cfg.agent_ok("anything") is True + + +def test_agent_ok_requires_exact_match() -> None: + """With a key configured only that exact key passes.""" + cfg = AuthConfig(agent=AGENT_KEY) + assert cfg.agent_ok(AGENT_KEY) is True + assert cfg.agent_ok(None) is False + assert cfg.agent_ok("") is False + assert cfg.agent_ok(AGENT_KEY + "x") is False + + +def test_agent_key_is_independent_of_console_tokens() -> None: + """The console tokens grant no agent rights and vice versa.""" + cfg = AuthConfig(operator="op-tok", observer="ob-tok", agent=AGENT_KEY) + assert cfg.agent_ok("op-tok") is False + assert cfg.role_for(AGENT_KEY) is None + + +# --------------------------------------------------------------------------- +# POST /register +# --------------------------------------------------------------------------- + + +def test_register_open_without_key(client: TestClient) -> None: + """No key configured: /register keeps its historical open behaviour.""" + resp = client.post("/register", json={"project": "alpha"}) + assert resp.status_code == 200, resp.text + assert resp.json()["project"] == "alpha" + + +def test_register_ignores_stray_bearer_without_key(client: TestClient) -> None: + """No key configured: a client that sends one anyway is not penalised. + + The plugin config ships an ``Authorization`` header unconditionally (empty + when the env var is unset), so an open hub must accept it either way. + """ + for header in ("Bearer whatever", "Bearer "): + resp = client.post( + "/register", + json={"project": f"alpha-{len(header)}"}, + headers={"Authorization": header}, + ) + assert resp.status_code == 200, resp.text + + +def test_register_accepts_correct_key( + client: TestClient, with_agent_key: None +) -> None: + """The right key in the Authorization header registers normally.""" + resp = client.post( + "/register", + json={"project": "alpha"}, + headers={"Authorization": f"Bearer {AGENT_KEY}"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["token"] + + +@pytest.mark.parametrize( + "headers", + [ + pytest.param({}, id="missing"), + pytest.param({"Authorization": f"Bearer {AGENT_KEY}x"}, id="wrong"), + pytest.param({"Authorization": AGENT_KEY}, id="no-bearer-scheme"), + pytest.param({"Authorization": "Bearer "}, id="empty-bearer"), + ], +) +def test_register_refused_without_valid_key( + client: TestClient, with_agent_key: None, headers: dict[str, str] +) -> None: + """A missing, malformed or wrong key is refused with an actionable 401.""" + resp = client.post("/register", json={"project": "alpha"}, headers=headers) + assert resp.status_code == 401, resp.text + detail = resp.json()["detail"] + assert "CAUCUS_AGENT_KEY" in detail + assert "--agent-key" in detail + + +def test_register_refusal_precedes_the_throttle( + client: TestClient, with_agent_key: None +) -> None: + """A bogus key never spends the per-host register budget. + + The gate runs before the token bucket, so flooding with a wrong key leaves + a legitimate caller's allowance intact (and never 429s in its place). + """ + for _ in range(int(hub_module._REGISTER_BUCKET_CAPACITY) + 5): + refused = client.post("/register", json={"project": "flood"}) + assert refused.status_code == 401 + ok = client.post( + "/register", + json={"project": "alpha"}, + headers={"Authorization": f"Bearer {AGENT_KEY}"}, + ) + assert ok.status_code == 200, ok.text + + +# --------------------------------------------------------------------------- +# /mcp +# --------------------------------------------------------------------------- + + +def _mcp_post(url: str, headers: dict[str, str]) -> httpx.Response: + """POST a minimal MCP ``initialize`` to ``/mcp`` with ``headers``.""" + body = { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0"}, + }, + } + return httpx.post( + f"{url}/mcp", + json=body, + headers={"Accept": "application/json, text/event-stream", **headers}, + timeout=10.0, + ) + + +def test_mcp_open_without_key(mcp_hub: tuple[str, HubState]) -> None: + """No key configured: /mcp answers an unauthenticated initialize.""" + url, _ = mcp_hub + resp = _mcp_post(url, {}) + assert resp.status_code == 200, resp.text + + +@pytest.mark.parametrize( + "headers", + [ + pytest.param({}, id="missing"), + pytest.param({"Authorization": f"Bearer {AGENT_KEY}x"}, id="wrong"), + ], +) +def test_mcp_refused_without_valid_key( + mcp_hub: tuple[str, HubState], + with_agent_key: None, + headers: dict[str, str], +) -> None: + """A missing or wrong key never reaches the MCP transport.""" + url, _ = mcp_hub + resp = _mcp_post(url, headers) + assert resp.status_code == 401, resp.text + detail = resp.json()["detail"] + assert "CAUCUS_AGENT_KEY" in detail + assert "--agent-key" in detail + + +def test_mcp_cors_preflight_survives_the_key( + mcp_hub: tuple[str, HubState], with_agent_key: None +) -> None: + """A preflight OPTIONS carries no Authorization and must not be 401'd.""" + url, _ = mcp_hub + resp = httpx.request( + "OPTIONS", + f"{url}/mcp", + headers={ + "Origin": "http://localhost:5173", + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type", + }, + timeout=10.0, + ) + assert resp.status_code == 204, resp.text + assert resp.headers["access-control-allow-origin"] == "http://localhost:5173" + + +def test_rest_surface_untouched_by_the_mcp_gate( + mcp_hub: tuple[str, HubState], with_agent_key: None +) -> None: + """The /mcp gate scopes to its own path; other routes are unaffected. + + ``/version`` needs no credential at all, and ``/register`` keeps answering + on its own endpoint-level gate rather than the middleware's. + """ + url, _ = mcp_hub + assert httpx.get(f"{url}/version", timeout=10.0).status_code == 200 + refused = httpx.post(f"{url}/register", json={"project": "alpha"}, timeout=10.0) + assert refused.status_code == 401 + ok = httpx.post( + f"{url}/register", + json={"project": "alpha"}, + headers={"Authorization": f"Bearer {AGENT_KEY}"}, + timeout=10.0, + ) + assert ok.status_code == 200, ok.text + + +async def test_mcp_tool_path_works_with_the_key( + mcp_hub: tuple[str, HubState], with_agent_key: None +) -> None: + """The in-process HubConnector path still works behind the gate. + + ``join`` registers directly against :class:`HubState` (amendment A1) while + ``say``/``listen`` re-enter the real handlers through the in-process + ``ASGITransport``. None of those hops carries the agent key, so this proves + the middleware gates the outside door without breaking the inside wiring. + """ + url, state = mcp_hub + auth = {"Authorization": f"Bearer {AGENT_KEY}"} + async with ( + streamablehttp_client(f"{url}/mcp", headers=auth) as (rd, wr, _sid), + ClientSession(rd, wr) as session, + ): + await session.initialize() + joined = await session.call_tool("join", {"project": "alpha"}) + assert '"joined": true' in joined.content[0].text # type: ignore[union-attr] + assert set(state.peers()) == {"alpha"} + said = await session.call_tool("say", {"content": "hello room", "to": "all"}) + assert "error" not in said.content[0].text # type: ignore[union-attr] + + +# --------------------------------------------------------------------------- +# client side +# --------------------------------------------------------------------------- + + +async def test_connector_sends_the_key_on_register( + state: HubState, with_agent_key: None +) -> None: + """HubConnector presents an explicit agent key on /register.""" + transport = httpx.ASGITransport(app=hub_module.app) + async with HubConnector( + "http://hub.invalid", transport=transport, agent_key=AGENT_KEY + ) as connector: + membership = await connector.register("alpha", None) + assert membership.project == "alpha" + assert membership.token + + +async def test_connector_reads_the_key_from_the_environment( + state: HubState, with_agent_key: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """With no explicit key the connector falls back to CAUCUS_AGENT_KEY.""" + monkeypatch.setenv(connector_module.AGENT_KEY_ENV, AGENT_KEY) + transport = httpx.ASGITransport(app=hub_module.app) + async with HubConnector("http://hub.invalid", transport=transport) as connector: + membership = await connector.register("alpha", None) + assert membership.token + + +async def test_connector_without_a_key_is_refused( + state: HubState, with_agent_key: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """A connector holding no key gets the hub's 401, not a silent join.""" + monkeypatch.delenv(connector_module.AGENT_KEY_ENV, raising=False) + transport = httpx.ASGITransport(app=hub_module.app) + async with HubConnector("http://hub.invalid", transport=transport) as connector: + with pytest.raises(httpx.HTTPStatusError) as excinfo: + await connector.register("alpha", None) + assert excinfo.value.response.status_code == 401 + + +def test_bridge_agent_headers(monkeypatch: pytest.MonkeyPatch) -> None: + """The bridge sends the key only when one is configured.""" + monkeypatch.setattr(bridge_module, "AGENT_KEY", None) + assert bridge_module._agent_headers() == {} + monkeypatch.setattr(bridge_module, "AGENT_KEY", AGENT_KEY) + assert bridge_module._agent_headers() == { + "Authorization": f"Bearer {AGENT_KEY}" + } + + +# --------------------------------------------------------------------------- +# The pre-join read surface: /peers, /channels, /forms +# --------------------------------------------------------------------------- + +READ_PATHS = ("/peers", "/channels", "/forms") + + +@pytest.mark.parametrize("path", READ_PATHS) +def test_read_surface_is_open_without_a_key(client: TestClient, path: str) -> None: + """The loopback default is untouched: no key configured, no header needed.""" + assert client.get(path).status_code == 200 + + +@pytest.mark.parametrize("path", READ_PATHS) +def test_read_surface_refuses_an_unkeyed_caller( + client: TestClient, with_agent_key: None, path: str +) -> None: + """A key closing /register but not the roster closes nothing worth closing. + + Unguarded, these three hand anyone who can reach a keyed 0.0.0.0 hub the + peer roster, every private channel's name, topic and members, and the text + of every pending operator form. + """ + assert client.get(path).status_code == 401 + + +@pytest.mark.parametrize("path", READ_PATHS) +def test_read_surface_accepts_the_agent_key( + client: TestClient, with_agent_key: None, path: str +) -> None: + """The same bearer ``/register`` takes opens the read surface.""" + resp = client.get(path, headers={"Authorization": f"Bearer {AGENT_KEY}"}) + assert resp.status_code == 200 + + +@pytest.mark.parametrize("path", READ_PATHS) +@pytest.mark.parametrize("role", ["operator", "observer"]) +def test_read_surface_accepts_a_console_token( + client: TestClient, monkeypatch: pytest.MonkeyPatch, path: str, role: str +) -> None: + """The console's own tooling holds a console token, not the agent key.""" + monkeypatch.setattr( + hub_module, + "auth_config", + AuthConfig(operator="op-token", observer="obs-token", agent=AGENT_KEY), + ) + token = "op-token" if role == "operator" else "obs-token" + assert client.get(path, headers={"Authorization": f"Bearer {token}"}).status_code == 200 + + +@pytest.mark.parametrize("path", READ_PATHS) +def test_read_surface_does_not_fall_back_to_the_open_operator_role( + client: TestClient, with_agent_key: None, path: str +) -> None: + """With no operator token every caller grades as operator; that must not pass. + + ``AuthConfig.role_for`` answers ``"operator"`` for anything when auth is + disabled, so a gate written on ``role_for`` alone would hand the whole + surface straight back on a hub keyed with ``--agent-key`` and nothing else. + """ + resp = client.get(path, headers={"Authorization": "Bearer nonsense"}) + assert resp.status_code == 401 + + +def test_ping_stays_open_without_a_key(client: TestClient) -> None: + """The loopback default is untouched: no key configured, no header needed.""" + assert client.get("/ping", params={"peer": "nobody"}).status_code == 200 + + +def test_ping_refuses_an_unkeyed_caller( + client: TestClient, with_agent_key: None +) -> None: + """``/ping`` returns far more than liveness, so a keyed hub must gate it. + + The payload confirms a named peer exists, when it was last seen, whether a + listener is attached, and the peer's own ``set_status`` prose. Unguarded, + anyone who can reach a keyed non-loopback hub reads all of it. + """ + assert client.get("/ping", params={"peer": "nobody"}).status_code == 401 + + +def test_ping_accepts_the_agent_key( + client: TestClient, with_agent_key: None +) -> None: + """The same bearer ``/register`` takes answers the probe.""" + resp = client.get( + "/ping", + params={"peer": "nobody"}, + headers={"Authorization": f"Bearer {AGENT_KEY}"}, + ) + assert resp.status_code == 200 + assert resp.json()["state"] == "absent" + + +async def test_connector_read_surface_carries_the_key( + state: HubState, with_agent_key: None +) -> None: + """The connector must send the key on the calls it makes before joining.""" + transport = httpx.ASGITransport(app=hub_module.app) + async with HubConnector( + "http://hub.invalid", transport=transport, agent_key=AGENT_KEY + ) as connector: + assert await connector.peers() == [] + assert await connector.channels() == {} + assert await connector.list_forms() == [] + assert (await connector.ping("nobody"))["state"] == "absent" + + +# --------------------------------------------------------------------------- +# A non-ASCII bearer must be refused, not raise +# --------------------------------------------------------------------------- + +#: Sent as raw bytes, because httpx refuses to encode a non-ASCII ``str`` header +#: (and an attacker with a socket is under no such constraint). The ASGI server +#: decodes these bytes as latin-1, so they reach the credential comparison as a +#: ``str`` ``secrets.compare_digest`` raises on. +NON_ASCII_BEARER = b"Bearer \xe9" + + +def test_non_ascii_bearer_is_refused_on_register( + client: TestClient, with_agent_key: None +) -> None: + """A TypeError here is an unthrottled 500: the gate runs before the bucket.""" + resp = client.post( + "/register", + json={"project": "alpha"}, + headers={"Authorization": NON_ASCII_BEARER}, + ) + assert resp.status_code == 401 + + +def test_non_ascii_bearer_is_refused_on_the_read_surface( + client: TestClient, with_agent_key: None +) -> None: + """Same input, same answer, on the endpoints gated by the same helper.""" + resp = client.get("/peers", headers={"Authorization": NON_ASCII_BEARER}) + assert resp.status_code == 401 + + +def test_non_ascii_console_token_grades_as_no_role() -> None: + """``role_for`` compares bytes too, so it answers None instead of raising.""" + config = AuthConfig(operator="op-token", observer="obs-token") + assert config.role_for("\xe9") is None + + +def test_non_ascii_bearer_is_refused_on_mcp( + mcp_hub: tuple[str, HubState], with_agent_key: None +) -> None: + """The ``/mcp`` ASGI gate reads the same header and must not raise either.""" + base, _ = mcp_hub + resp = httpx.post( + f"{base}/mcp", + headers={"Authorization": NON_ASCII_BEARER}, + json={"jsonrpc": "2.0", "id": 1, "method": "ping"}, + timeout=5.0, + ) + assert resp.status_code == 401 + + +def test_unkeyed_options_on_mcp_is_gated( + mcp_hub: tuple[str, HubState], with_agent_key: None +) -> None: + """An unkeyed ``OPTIONS`` must not reach the transport at all. + + A real CORS preflight carries an allowed ``Origin`` and is answered by the + CORS layer wrapping this gate, so an ``OPTIONS`` arriving here is not a + preflight -- and letting it through made the SDK build a session transport + and a task group per request before answering 405. + """ + base, _ = mcp_hub + resp = httpx.request("OPTIONS", f"{base}/mcp", timeout=5.0) + assert resp.status_code == 401 diff --git a/tests/test_remote_hub.py b/tests/test_remote_hub.py new file mode 100644 index 0000000..fceaf77 --- /dev/null +++ b/tests/test_remote_hub.py @@ -0,0 +1,901 @@ +"""Tests for making a hub reachable from other machines. + +Four surfaces, all introduced together because none of them is usable alone: + +* ``--allowed-host`` / ``CAUCUS_ALLOWED_HOSTS`` feeding the ``/mcp`` + DNS-rebinding guard (:func:`caucus.hub._collect_allowed_hosts`), +* ``--public-url`` / ``CAUCUS_PUBLIC_URL`` and its validation + (:func:`caucus.urlguard.validate_public_url`), +* the ``watch_command`` tool handing a remote agent a runnable command instead + of a path on the hub's own filesystem, and the single-use watch ticket that + keeps the peer token out of that command, +* the startup refusal on a non-loopback bind without both credentials. + +The refusal tests drive :func:`caucus.hub.main` itself with ``uvicorn.run`` +stubbed out, because the gate is part of the CLI contract and a pure-function +test would not prove the flag is wired to it. +""" + +from __future__ import annotations + +import sys +from typing import Any + +import pytest +from fastapi.testclient import TestClient + +from caucus import hub as hub_module +from caucus.hub import AuthConfig, ServerConfig, _collect_allowed_hosts +from caucus.mcp_http import build_mcp_server +from caucus.state import WATCH_TICKET_TTL, HubState +from caucus.urlguard import ALLOW_REMOTE_ENV, validate_hub_url, validate_public_url + +# --------------------------------------------------------------------------- +# --allowed-host / CAUCUS_ALLOWED_HOSTS +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _no_allowed_hosts_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep an operator's real environment out of every case in this module.""" + monkeypatch.delenv("CAUCUS_ALLOWED_HOSTS", raising=False) + monkeypatch.delenv("CAUCUS_PUBLIC_URL", raising=False) + + +def test_bare_host_is_allowed_on_the_hubs_own_port() -> None: + """A hostname with no port means "this hub, under that name".""" + assert _collect_allowed_hosts(["hub.lan"], 8765) == ["hub.lan:8765"] + + +def test_explicit_host_port_is_kept_verbatim() -> None: + """An entry that already names a port is a deliberate, different address.""" + assert _collect_allowed_hosts(["hub.lan:443"], 8765) == ["hub.lan:443"] + + +def test_bracketed_ipv6_without_a_port_gets_one() -> None: + """``[2001:db8::1]`` is a bare host, not a host that carries a port.""" + assert _collect_allowed_hosts(["[2001:db8::1]"], 8765) == ["[2001:db8::1]:8765"] + + +def test_bracketed_ipv6_with_a_port_is_kept_verbatim() -> None: + """The closing bracket is not last, so the trailing colon is the port.""" + assert _collect_allowed_hosts(["[2001:db8::1]:9000"], 8765) == [ + "[2001:db8::1]:9000" + ] + + +def test_repeated_flags_accumulate_in_order() -> None: + """``--allowed-host`` is repeatable, like ``--allowed-origin``.""" + assert _collect_allowed_hosts(["a.lan", "b.lan:80"], 8765) == [ + "a.lan:8765", + "b.lan:80", + ] + + +def test_env_var_is_comma_split(monkeypatch: pytest.MonkeyPatch) -> None: + """The env form mirrors ``CAUCUS_ALLOWED_ORIGINS``: comma-separated.""" + monkeypatch.setenv("CAUCUS_ALLOWED_HOSTS", "a.lan, b.lan:80 ,") + assert _collect_allowed_hosts(None, 8765) == ["a.lan:8765", "b.lan:80"] + + +def test_flags_and_env_merge_without_duplicates( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Naming the same host twice, by either route, yields one entry.""" + monkeypatch.setenv("CAUCUS_ALLOWED_HOSTS", "hub.lan,other.lan") + assert _collect_allowed_hosts(["hub.lan:8765"], 8765) == [ + "hub.lan:8765", + "other.lan:8765", + ] + + +def test_no_flags_and_no_env_is_empty() -> None: + """The default stays the loopback-only posture the guard already has.""" + assert _collect_allowed_hosts(None, 8765) == [] + + +@pytest.mark.parametrize( + ("entry", "expected"), + [("::1", "[::1]:8765"), ("2001:db8::1", "[2001:db8::1]:8765")], +) +def test_bare_ipv6_is_bracketed_and_given_the_hubs_port( + entry: str, expected: str +) -> None: + """A Host header always brackets an IPv6 literal, so the entry must too. + + Passed through verbatim, ``--allowed-host ::1`` matches no Host header the + guard will ever see, and the operator believes they allowed an address they + did not. + """ + assert _collect_allowed_hosts([entry], 8765) == [expected] + + +# --------------------------------------------------------------------------- +# --public-url validation +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("url", "expected"), + [ + ("https://hub.example.net", "https://hub.example.net"), + ("http://hub.lan:8765", "http://hub.lan:8765"), + # A trailing slash is the empty path spelled out; strip it so the hub + # never builds "https://host//receive". + ("https://hub.example.net/", "https://hub.example.net"), + ("HTTPS://hub.example.net", "HTTPS://hub.example.net"), + ("http://[2001:db8::1]:8765", "http://[2001:db8::1]:8765"), + ], +) +def test_public_url_accepts_a_bare_origin(url: str, expected: str) -> None: + """Scheme plus host (plus optional port) is the whole accepted shape.""" + assert validate_public_url(url) == expected + + +@pytest.mark.parametrize( + "url", + [ + "ftp://hub.example.net", + "ws://hub.example.net", + "hub.example.net:8765", # no scheme: urlparse reads "hub.example.net" + "https://", + "https://hub.example.net/caucus", + "https://hub.example.net/?x=1", + "https://hub.example.net#frag", + ], +) +def test_public_url_rejects_anything_else(url: str) -> None: + """A wrong scheme, a missing host, or anything past the origin is refused.""" + with pytest.raises(ValueError): + validate_public_url(url) + + +# --------------------------------------------------------------------------- +# watch_command: loopback keeps the token file, remote gets the env form +# --------------------------------------------------------------------------- + + +def _ctx(session_id: str) -> Any: + """Minimal ``Context`` stand-in carrying an ``Mcp-Session-Id`` header.""" + request = type("_Req", (), {"headers": {"mcp-session-id": session_id}})() + request_context = type("_RC", (), {"request": request})() + return type("_Ctx", (), {"request_context": request_context})() + + +def _tool(server: Any, name: str) -> Any: + """Return a registered tool's underlying callable for a direct call.""" + return server._tool_manager.get_tool(name).fn + + +async def test_watch_command_loopback_keeps_the_token_file(state: HubState) -> None: + """The default deployment is unchanged: a 0600 token file on this machine.""" + import os + + server = build_mcp_server(hub_module.app, self_url="http://127.0.0.1:9999") + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + + res = await _tool(server, "watch_command")(ctx) + command = str(res["command"]) + assert command.startswith("caucus-watch --hub http://127.0.0.1:9999 --token-file ") + path = command.split("--token-file ", 1)[1] + assert os.path.exists(path) + assert oct(os.stat(path).st_mode & 0o777) == "0o600" + + await _tool(server, "leave")(ctx) + assert not os.path.exists(path) + + +async def test_watch_command_remote_hands_out_a_ticket_not_the_token( + state: HubState, +) -> None: + """A remote agent gets a claim check, never the room bearer itself. + + The peer token opens ``/receive``, ``/send``, ``/ack``, ``/channels/*``, + ``/ask`` and ``/floor``, and the agent key gates none of them. Printing it + in a shell command would put it in the agent's transcript, its shell + history and the watcher's environ, which is exactly what the loopback + token file exists to prevent. + """ + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + joined = await _tool(server, "join")(ctx, project="alpha") + + res = await _tool(server, "watch_command")(ctx) + command = str(res["command"]) + prefix, _, ticket = command.partition(" --ticket ") + assert prefix == "caucus-watch --hub https://hub.example.net" + assert ticket + # The ticket is real: it redeems to this peer's live token, once. + token = state.redeem_watch_ticket(ticket) + assert token is not None + client = state.client_for(token) + assert client is not None and client.project == "alpha" + # And the token itself appears nowhere in the whole result payload, not + # merely outside the command string. + assert token not in repr(res) + assert "CAUCUS_TOKEN" not in command + # Nothing naming a path on the hub's filesystem either. + assert "--token-file" not in command + # join()'s own payload must not leak it as a consolation prize. + assert token not in repr(joined) + + +async def test_watch_command_refresh_revokes_the_previous_ticket( + state: HubState, +) -> None: + """Refreshing must retire the old ticket, not just mint a new one beside it. + + Without this, every ``watch_command()`` call left the prior ticket + outstanding for its full TTL: N calls meant N live bearer credentials for + the same peer token, each one already sitting in the agent's transcript. + """ + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + + first = str((await _tool(server, "watch_command")(ctx))["command"]) + _, _, first_ticket = first.partition(" --ticket ") + second = str((await _tool(server, "watch_command")(ctx))["command"]) + _, _, second_ticket = second.partition(" --ticket ") + + assert first_ticket and second_ticket and first_ticket != second_ticket + # The retired ticket is dead... + assert state.redeem_watch_ticket(first_ticket) is None + # ...while the fresh one still works. + assert state.redeem_watch_ticket(second_ticket) is not None + + +async def test_leave_revokes_the_outstanding_watch_ticket( + state: HubState, +) -> None: + """A departing agent must not leave a live ticket for the token it dropped.""" + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + command = str((await _tool(server, "watch_command")(ctx))["command"]) + _, _, ticket = command.partition(" --ticket ") + assert ticket + + await _tool(server, "leave")(ctx) + + assert state.redeem_watch_ticket(ticket) is None + + +async def test_dead_session_sweep_revokes_its_ticket( + state: HubState, monkeypatch: pytest.MonkeyPatch +) -> None: + """A session reaped as dead must not leave its ticket redeemable behind it.""" + from caucus import mcp_http + + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + command = str((await _tool(server, "watch_command")(ctx))["command"]) + _, _, ticket = command.partition(" --ticket ") + assert ticket + + # Simulate the joined session's hub client having died, exactly as + # test_session_reaper_sweeps_dead_sessions does in test_mcp_http.py. + monkeypatch.setattr(state, "client_for", lambda _tok: None) + assert mcp_http._session_reaper_fn is not None + mcp_http._session_reaper_fn() + + assert state.redeem_watch_ticket(ticket) is None + + +def _hub_flag(command: str) -> str: + """Return the value the emitted command passes to ``caucus-watch --hub``.""" + parts = command.split() + return parts[parts.index("--hub") + 1] + + +async def test_watch_command_https_url_is_accepted_by_the_watcher( + state: HubState, monkeypatch: pytest.MonkeyPatch +) -> None: + """The emitted --hub must survive the check caucus-watch runs on it.""" + monkeypatch.delenv(ALLOW_REMOTE_ENV, raising=False) + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + + command = str((await _tool(server, "watch_command")(ctx))["command"]) + # https needs no opt-in, so the command must not carry one either. + assert not command.startswith(ALLOW_REMOTE_ENV) + assert validate_hub_url(_hub_flag(command)) == "https://hub.example.net" + + +async def test_watch_command_plain_http_url_carries_the_opt_in( + state: HubState, monkeypatch: pytest.MonkeyPatch +) -> None: + """A plain-http public URL is refused by the watcher unless the command says so. + + Without the prefix the agent backgrounds a process that exits 2 before its + first poll and believes a watcher is listening. + """ + monkeypatch.delenv(ALLOW_REMOTE_ENV, raising=False) + server = build_mcp_server( + hub_module.app, self_url="http://hub.lan:8765", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + + command = str((await _tool(server, "watch_command")(ctx))["command"]) + assert command.startswith(f"{ALLOW_REMOTE_ENV}=1 caucus-watch ") + # Bare, the URL is refused; with the opt-in the command's own prefix sets, + # it is accepted -- which is the whole point of emitting the prefix. + with pytest.raises(ValueError): + validate_hub_url(_hub_flag(command)) + monkeypatch.setenv(ALLOW_REMOTE_ENV, "1") + assert validate_hub_url(_hub_flag(command)) == "http://hub.lan:8765" + + +async def test_watch_command_remote_writes_no_token_file( + state: HubState, monkeypatch: pytest.MonkeyPatch +) -> None: + """The remote branch must not create a file only the hub can see.""" + from caucus import mcp_http + + def _forbidden(_token: str) -> str: + raise AssertionError("a remote deployment must not write a token file") + + monkeypatch.setattr(mcp_http, "_write_token_file", _forbidden) + server = build_mcp_server( + hub_module.app, self_url="https://hub.example.net", remote=True + ) + ctx = _ctx("s1") + await _tool(server, "join")(ctx, project="alpha") + await _tool(server, "watch_command")(ctx) + + +# --------------------------------------------------------------------------- +# Watch tickets: HubState store and POST /watch-ticket/redeem +# --------------------------------------------------------------------------- + +REDEEM_PATH = "/watch-ticket/redeem" + + +def test_watch_ticket_redeems_exactly_once(state: HubState) -> None: + """Single use is the property the whole design rests on.""" + ticket = state.issue_watch_ticket("peer-token") + assert state.redeem_watch_ticket(ticket) == "peer-token" + assert state.redeem_watch_ticket(ticket) is None + + +def test_watch_ticket_expires(state: HubState) -> None: + """Past its TTL a ticket is gone, whether or not anyone swept it. + + The clock is driven, not slept on: a real wait would put two minutes of + dead time in the suite to prove one comparison. + """ + ticket = state.issue_watch_ticket("peer-token", now=1000.0) + assert state.redeem_watch_ticket(ticket, now=1000.0 + WATCH_TICKET_TTL - 1) == ( + "peer-token" + ) + again = state.issue_watch_ticket("peer-token", now=2000.0) + assert state.redeem_watch_ticket(again, now=2000.0 + WATCH_TICKET_TTL + 1) is None + + +def test_unknown_watch_ticket_redeems_to_nothing(state: HubState) -> None: + """A guessed or invented ticket buys nothing.""" + state.issue_watch_ticket("peer-token") + assert state.redeem_watch_ticket("not-a-ticket") is None + + +def test_expired_watch_tickets_are_swept_by_the_idle_reaper(state: HubState) -> None: + """The store must not grow: the sweep the hub already runs prunes it.""" + state.issue_watch_ticket("peer-token", now=1000.0) + assert len(state._watch_tickets) == 1 + state.reap_stale(state.client_ttl, now=1000.0 + WATCH_TICKET_TTL + 1) + assert state._watch_tickets == {} + + +def test_redeem_endpoint_returns_the_token_once(client: TestClient) -> None: + """The endpoint is the state store's behaviour over HTTP: 200 then 404.""" + ticket = hub_module.state.issue_watch_ticket("peer-token") + resp = client.post(REDEEM_PATH, json={"ticket": ticket}) + assert resp.status_code == 200 + assert resp.json() == {"token": "peer-token"} + replay = client.post(REDEEM_PATH, json={"ticket": ticket}) + assert replay.status_code == 404 + assert "watch_command()" in replay.json()["detail"] + + +def test_redeem_endpoint_404s_an_unknown_ticket(client: TestClient) -> None: + """404, not 401: the caller's credential was fine, the ticket is not there.""" + assert client.post(REDEEM_PATH, json={"ticket": "nope"}).status_code == 404 + + +def test_redeem_endpoint_requires_the_agent_key( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """A keyed hub must not hand peer tokens to whoever can reach the port.""" + ticket = hub_module.state.issue_watch_ticket("peer-token") + monkeypatch.setattr(hub_module, "auth_config", AuthConfig(agent="the-key")) + refused = client.post(REDEEM_PATH, json={"ticket": ticket}) + assert refused.status_code == 401 + # The refusal fires before the lookup, so the ticket survives it intact. + ok = client.post( + REDEEM_PATH, + json={"ticket": ticket}, + headers={"Authorization": "Bearer the-key"}, + ) + assert ok.status_code == 200 + assert ok.json() == {"token": "peer-token"} + + +def test_a_ticket_for_a_dead_peer_resurrects_nothing(client: TestClient) -> None: + """A ticket is a claim check on a token, not a second life for a peer. + + Redemption knows nothing about clients, so it still answers; the token it + hands back is then refused by ``client_for`` exactly as it already is + today, and the watcher's first ``/receive`` earns the usual 401. + """ + token = client.post("/register", json={"project": "ghost"}).json()["token"] + ticket = hub_module.state.issue_watch_ticket(token) + assert hub_module.state.unregister(token) == "ghost" + + resp = client.post(REDEEM_PATH, json={"ticket": ticket}) + assert resp.status_code == 200 + handed_back = resp.json()["token"] + assert handed_back == token + assert hub_module.state.client_for(handed_back) is None + + +def test_a_reaped_peer_is_revived_through_a_redeemed_ticket( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The counterpart of the dead-peer case above: a *soft* drop does revive. + + ``issue_watch_ticket``'s docstring used to claim a reaped peer can never be + resurrected through a stale ticket. That is only true once the token is + truly forgotten. Redemption just hands the token back unchanged, and + inside ``reaped_grace`` ``client_for`` revives it exactly as it would for + the token presented directly, ticket or not. + + A short ``client_ttl`` is used (rather than the ``state``/``client`` + fixtures' defaults) so the peer can be reaped well inside the ticket's own + :data:`~caucus.state.WATCH_TICKET_TTL`, which the default 300s idle window + would blow straight past. + + ``reap_stale`` measures two different deadlines against the single ``now`` + it is handed: the idle window it reaps against, and the watch-ticket expiry + it prunes against. So the reap instant has to sit strictly between + ``client_ttl`` and ``WATCH_TICKET_TTL``: too early and the peer is not + stale yet, too late and the sweep spends the ticket before the redemption + can. Both the mint and the reap are therefore driven off one reference + instant, and that instant is placed at the midpoint of the two constants: + the relationship the test depends on is then arithmetic on the constants + themselves, not an agreement between a synthetic clock and the wall clock + that happens to hold while the suite is fast. + """ + fresh = HubState(client_ttl=10.0) + monkeypatch.setattr(hub_module, "state", fresh) + with TestClient(hub_module.app) as client: + token = client.post("/register", json={"project": "ghost"}).json()["token"] + # One reference instant for every clock this test drives: the moment the + # peer was put on the roster. + registered_at = fresh._clients["ghost"].last_seen + ticket = fresh.issue_watch_ticket(token, now=registered_at) + reap_at = registered_at + (fresh.client_ttl + WATCH_TICKET_TTL) / 2 + assert fresh.reap_stale(fresh.client_ttl, now=reap_at) == ["ghost"] + + resp = client.post(REDEEM_PATH, json={"ticket": ticket}) + assert resp.status_code == 200 + handed_back = resp.json()["token"] + assert handed_back == token + revived = fresh.client_for(handed_back) + assert revived is not None + assert revived.project == "ghost" + + +# --------------------------------------------------------------------------- +# _mount_mcp_http wiring +# --------------------------------------------------------------------------- + + +def _capture_build(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + """Stub ``build_mcp_server`` and return the dict its kwargs land in.""" + from caucus import mcp_http + + captured: dict[str, Any] = {} + + def _fake(app: Any, **kwargs: Any) -> Any: + captured.update(kwargs) + return type("_Server", (), {"streamable_http_app": lambda self: _Sub()})() + + class _Sub: + routes: list[Any] = [] + + monkeypatch.setattr(mcp_http, "build_mcp_server", _fake) + monkeypatch.setattr(hub_module, "server_config", ServerConfig()) + # _mount_mcp_http writes these two module globals; restore them so a stub + # server never outlives this test. + monkeypatch.setattr(hub_module, "_mcp_server", hub_module._mcp_server) + monkeypatch.setattr(hub_module, "_session_reaper_fn", hub_module._session_reaper_fn) + return captured + + +def test_mount_passes_public_url_as_self_url(monkeypatch: pytest.MonkeyPatch) -> None: + """The advertised URL replaces the 127.0.0.1 rewrite of a wildcard bind.""" + captured = _capture_build(monkeypatch) + hub_module._mount_mcp_http( + host="0.0.0.0", + port=8765, + mcp_path="/mcp", + extra_origins=set(), + extra_hosts=["hub.lan:8765"], + public_url="https://hub.example.net", + ) + assert captured["self_url"] == "https://hub.example.net" + assert captured["remote"] is True + # The operator's entry, plus the public URL's own netloc added for free. + assert "hub.lan:8765" in captured["allowed_hosts"] + assert "hub.example.net" in captured["allowed_hosts"] + + +def test_mount_without_public_url_is_unchanged( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A plain loopback mount still advertises itself and stays non-remote.""" + captured = _capture_build(monkeypatch) + hub_module._mount_mcp_http( + host="127.0.0.1", port=8765, mcp_path="/mcp", extra_origins=set() + ) + assert captured["self_url"] == "http://127.0.0.1:8765" + assert captured["remote"] is False + assert captured["allowed_hosts"] == ["127.0.0.1:8765"] + + +def test_mount_does_not_repeat_the_bind_address( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Naming the bind address as --allowed-host too must not list it twice.""" + captured = _capture_build(monkeypatch) + hub_module._mount_mcp_http( + host="192.168.1.10", + port=8765, + mcp_path="/mcp", + extra_origins=set(), + extra_hosts=["192.168.1.10:8765"], + ) + assert captured["allowed_hosts"] == ["192.168.1.10:8765"] + + +def test_mount_on_a_non_loopback_bind_is_remote( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No public URL, but a LAN bind: the watcher command still must travel.""" + captured = _capture_build(monkeypatch) + hub_module._mount_mcp_http( + host="192.168.1.10", port=8765, mcp_path="/mcp", extra_origins=set() + ) + assert captured["remote"] is True + + +# --------------------------------------------------------------------------- +# The non-loopback bind refusal +# --------------------------------------------------------------------------- + + +@pytest.fixture +def run_main(monkeypatch: pytest.MonkeyPatch) -> Any: + """Return a callable running ``hub.main()`` with the server stubbed out. + + ``uvicorn.run`` is replaced by a recorder, so "the hub starts" is observable + without binding a socket, and every global ``main`` writes to is restored by + ``monkeypatch`` when the test ends. + """ + started: list[tuple[str, int]] = [] + + def _run(*_args: Any, **kwargs: Any) -> None: + started.append((kwargs["host"], kwargs["port"])) + + monkeypatch.setattr(hub_module.uvicorn, "run", _run) + monkeypatch.setattr(hub_module.coloredlogs, "install", lambda **_kw: None) + monkeypatch.setattr(hub_module, "auth_config", AuthConfig()) + monkeypatch.setattr(hub_module, "server_config", ServerConfig()) + + def _main(*argv: str) -> list[tuple[str, int]]: + # --no-mcp-http keeps the mount out of the import-time app, which other + # tests in the session share; --no-browser keeps the timer thread away. + monkeypatch.setattr( + sys, "argv", ["caucus-hub", "--no-browser", "--no-mcp-http", *argv] + ) + hub_module.main() + return started + + return _main + + +def test_non_loopback_bind_without_credentials_refuses( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """The headline behaviour change: 0.0.0.0 no longer starts silently.""" + with pytest.raises(SystemExit) as excinfo: + run_main("--host", "0.0.0.0") + assert excinfo.value.code == 2 + message = capsys.readouterr().err + # The refusal is the feature: every flag and env var needed to fix it. + for needle in ( + "--agent-key", + "CAUCUS_AGENT_KEY", + "--operator-token", + "CAUCUS_OPERATOR_TOKEN", + "--public-url", + "CAUCUS_PUBLIC_URL", + "--allowed-host", + "CAUCUS_ALLOWED_HOSTS", + "--allow-insecure-bind", + "--host 127.0.0.1", + ): + assert needle in message, f"refusal does not mention {needle}" + + +def test_refusal_names_which_credential_is_missing( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """Half-configured is the common case; say which half is done.""" + with pytest.raises(SystemExit): + run_main("--host", "0.0.0.0", "--operator-token", "op123") + message = capsys.readouterr().err + assert "--operator-token TOKEN (env CAUCUS_OPERATOR_TOKEN): already set" in message + assert "--agent-key KEY (env CAUCUS_AGENT_KEY): MISSING" in message + + +def test_non_loopback_bind_with_both_credentials_starts(run_main: Any) -> None: + """Both doors locked and an address to advertise, so the bind is allowed.""" + started = run_main( + "--host", + "0.0.0.0", + "--operator-token", + "op123", + "--agent-key", + "key123", + "--public-url", + "https://hub.example.net", + ) + assert started == [("0.0.0.0", 8765)] + + +def test_wildcard_bind_without_a_public_url_refuses( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """0.0.0.0 is not an address: with both keys set, the URL is still missing.""" + with pytest.raises(SystemExit) as excinfo: + run_main( + "--host", "0.0.0.0", "--operator-token", "op123", "--agent-key", "key123" + ) + assert excinfo.value.code == 2 + message = capsys.readouterr().err + assert "--public-url URL (env CAUCUS_PUBLIC_URL): MISSING" in message + # The two doors it *has* got are marked done, so the operator reads one gap. + assert "--agent-key KEY (env CAUCUS_AGENT_KEY): already set" in message + + +def test_concrete_non_loopback_bind_needs_no_public_url(run_main: Any) -> None: + """A real interface address advertises itself, so only the keys are demanded.""" + started = run_main( + "--host", "192.168.1.10", "--operator-token", "op123", "--agent-key", "key123" + ) + assert started == [("192.168.1.10", 8765)] + + +def test_the_whole_loopback_range_skips_the_bind_gate(run_main: Any) -> None: + """127.0.0.2 is loopback by any honest definition, and now by this one too.""" + started = run_main("--host", "127.0.0.2") + assert started == [("127.0.0.2", 8765)] + + +def test_allow_insecure_bind_is_the_escape_hatch(run_main: Any) -> None: + """An operator who means it can still run wide open, explicitly.""" + started = run_main("--host", "0.0.0.0", "--allow-insecure-bind") + assert started == [("0.0.0.0", 8765)] + + +def test_loopback_without_credentials_still_starts(run_main: Any) -> None: + """The default localhost posture is untouched.""" + started = run_main("--host", "127.0.0.1") + assert started == [("127.0.0.1", 8765)] + + +def test_credentials_may_come_from_the_environment( + run_main: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """All three flags default from their env vars, so the gate sees them too.""" + monkeypatch.setenv("CAUCUS_OPERATOR_TOKEN", "op123") + monkeypatch.setenv("CAUCUS_AGENT_KEY", "key123") + monkeypatch.setenv("CAUCUS_PUBLIC_URL", "https://hub.example.net") + started = run_main("--host", "0.0.0.0") + assert started == [("0.0.0.0", 8765)] + + +def test_plain_http_public_url_warns_at_startup( + run_main: Any, caplog: pytest.LogCaptureFixture +) -> None: + """Said once, at boot: a plain-http advertised URL leaks every peer token.""" + with caplog.at_level("WARNING", logger="caucus.hub"): + run_main( + "--host", + "0.0.0.0", + "--operator-token", + "op123", + "--agent-key", + "key123", + "--public-url", + "http://hub.lan:8765", + ) + warning = "\n".join(r.getMessage() for r in caplog.records) + assert "in clear" in warning + assert "CAUCUS_ALLOW_REMOTE_HUB" in warning + + +def test_https_public_url_does_not_warn( + run_main: Any, caplog: pytest.LogCaptureFixture +) -> None: + """The warning is about cleartext, so TLS must silence it.""" + with caplog.at_level("WARNING", logger="caucus.hub"): + run_main( + "--host", + "0.0.0.0", + "--operator-token", + "op123", + "--agent-key", + "key123", + "--public-url", + "https://hub.example.net", + ) + assert "in clear" not in "\n".join(r.getMessage() for r in caplog.records) + + +def test_blank_credentials_are_normalised_to_none(run_main: Any) -> None: + """An empty credential means "none configured", not one nobody can present. + + Left raw, ``--agent-key ""`` makes ``agent_ok`` reject every caller and + ``--operator-token ""`` flips ``AuthConfig.enabled`` on with a token no + first frame can match -- a lockout at both doors. The clients already + normalise blank to ``None``; the hub was the odd one out. + """ + run_main( + "--agent-key", "", "--operator-token", "", "--observer-token", "" + ) + assert hub_module.auth_config.agent is None + assert hub_module.auth_config.operator is None + assert hub_module.auth_config.observer is None + assert hub_module.auth_config.agent_ok(None) is True + assert hub_module.auth_config.enabled is False + + +def test_a_blank_public_url_is_not_an_advertised_address(run_main: Any) -> None: + """``--public-url ""`` already normalised to None; keep it that way.""" + started = run_main("--public-url", "") + assert started == [("127.0.0.1", 8765)] + + +def test_an_invalid_public_url_refuses_at_startup( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """A public URL with a path would build unreachable addresses; refuse it. + + Both credentials are supplied so the loopback+public-url credentials guard + (below) does not intercept first: this test is about ``validate_public_url`` + rejecting the path, not about the credentials gate. + """ + with pytest.raises(SystemExit): + run_main( + "--public-url", + "https://hub.example.net/caucus", + "--operator-token", + "op123", + "--agent-key", + "key123", + ) + assert "bare origin" in capsys.readouterr().err + + +# --------------------------------------------------------------------------- +# A loopback bind advertised under a non-loopback --public-url +# --------------------------------------------------------------------------- + + +def test_loopback_bind_with_remote_public_url_without_credentials_refuses( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """A loopback socket behind a public URL is exactly as exposed as a wide bind. + + The bind itself never leaves this machine, but declaring a non-loopback + ``--public-url`` says a tunnel or reverse proxy carries the outside world + in, and ``_mount_mcp_http`` already treats that as ``remote=True``. The + credentials gate has to read it the same way. + """ + with pytest.raises(SystemExit) as excinfo: + run_main("--public-url", "https://hub.example.net") + assert excinfo.value.code == 2 + message = capsys.readouterr().err + for needle in ( + "--public-url", + "https://hub.example.net", + "--agent-key", + "CAUCUS_AGENT_KEY", + "--operator-token", + "CAUCUS_OPERATOR_TOKEN", + ): + assert needle in message, f"refusal does not mention {needle}" + + +def test_loopback_bind_with_remote_public_url_names_which_credential_is_missing( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """Half-configured is the common case here too; say which half is done.""" + with pytest.raises(SystemExit): + run_main("--public-url", "https://hub.example.net", "--agent-key", "key123") + message = capsys.readouterr().err + assert "--agent-key KEY (env CAUCUS_AGENT_KEY): already set" in message + assert "--operator-token TOKEN (env CAUCUS_OPERATOR_TOKEN): MISSING" in message + + +def test_loopback_bind_with_remote_public_url_and_both_credentials_starts( + run_main: Any, +) -> None: + """Both doors locked, so declaring the hub reachable elsewhere is allowed.""" + started = run_main( + "--public-url", + "https://hub.example.net", + "--operator-token", + "op123", + "--agent-key", + "key123", + ) + assert started == [("127.0.0.1", 8765)] + + +def test_loopback_bind_with_remote_public_url_allow_insecure_bind_starts( + run_main: Any, +) -> None: + """The same escape hatch as the bind guard covers the public-url guard.""" + started = run_main( + "--public-url", "https://hub.example.net", "--allow-insecure-bind" + ) + assert started == [("127.0.0.1", 8765)] + + +def test_loopback_public_url_arms_nothing(run_main: Any) -> None: + """localhost is just a prettier address for this machine, not an exposure.""" + started = run_main("--public-url", "http://localhost:8765") + assert started == [("127.0.0.1", 8765)] + + +def test_remote_public_url_from_the_environment_arms_the_guard( + run_main: Any, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """``CAUCUS_PUBLIC_URL`` is read the same way ``--public-url`` is.""" + monkeypatch.setenv("CAUCUS_PUBLIC_URL", "https://hub.example.net") + with pytest.raises(SystemExit): + run_main() + assert "https://hub.example.net" in capsys.readouterr().err + + +def test_malformed_public_url_on_a_loopback_bind_is_not_caught_by_the_new_guard( + run_main: Any, capsys: pytest.CaptureFixture[str] +) -> None: + """No parseable hostname arms nothing here; ``validate_public_url`` is the error. + + ``urlparse("garbage").hostname`` is ``None``, so the credentials guard sees + no advertised host at all and stays quiet, leaving the more useful + ``validate_public_url`` error as what the operator sees. + """ + with pytest.raises(SystemExit): + run_main("--public-url", "garbage") + message = capsys.readouterr().err + assert "unsupported public URL scheme" in message + assert "refusing to start" not in message diff --git a/tests/test_setup_service.py b/tests/test_setup_service.py index 34d72f6..f129ae0 100644 --- a/tests/test_setup_service.py +++ b/tests/test_setup_service.py @@ -89,21 +89,162 @@ def test_check_port_rejects_above_max_without_mentioning_root() -> None: # --------------------------------------------------------------------------- -@pytest.mark.parametrize("host", sorted(setup_service.LOOPBACK_HOSTS)) +@pytest.mark.parametrize("host", ["127.0.0.1", "::1", "localhost"]) def test_check_bind_loopback_without_token_is_ok(host: str) -> None: - """Loopback addresses need no operator token.""" - setup_service.check_bind(host, None) + """Loopback addresses need no credentials at all.""" + setup_service.check_bind(host, None, None) def test_check_bind_wildcard_without_token_raises() -> None: - """A network-visible bind with no operator token is refused.""" + """A network-visible bind with no credentials is refused.""" with pytest.raises(setup_service.SetupError): - setup_service.check_bind("0.0.0.0", None) + setup_service.check_bind("0.0.0.0", None, None) -def test_check_bind_wildcard_with_token_is_ok() -> None: - """The same bind is accepted once an operator token gates it.""" - setup_service.check_bind("0.0.0.0", "sometoken123") +def test_check_bind_wildcard_with_only_the_operator_token_raises() -> None: + """The operator token alone never guarded /register or /mcp. + + The old gate stopped here, which let the installer write a unit whose agent + door was open to the whole network. Both credentials are required now. + """ + with pytest.raises(setup_service.SetupError) as excinfo: + setup_service.check_bind("0.0.0.0", "sometoken123", None) + assert "--agent-key" in str(excinfo.value) + + +def test_check_bind_wildcard_with_only_the_agent_key_raises() -> None: + """Symmetrically, the agent key alone leaves the dashboard open.""" + with pytest.raises(setup_service.SetupError) as excinfo: + setup_service.check_bind("0.0.0.0", None, "somekey123") + assert "--operator-token" in str(excinfo.value) + + +def test_check_bind_wildcard_with_both_credentials_is_ok() -> None: + """The same bind is accepted once both doors are gated and an address is set.""" + setup_service.check_bind( + "0.0.0.0", "sometoken123", "somekey123", "https://hub.example.net" + ) + + +def test_check_bind_wildcard_without_public_url_raises() -> None: + """0.0.0.0 names no address to advertise, so the install must supply one.""" + with pytest.raises(setup_service.SetupError) as excinfo: + setup_service.check_bind("0.0.0.0", "sometoken123", "somekey123") + assert "--public-url" in str(excinfo.value) + + +def test_check_bind_concrete_host_needs_no_public_url() -> None: + """A real interface address already advertises itself correctly.""" + setup_service.check_bind("192.168.1.10", "sometoken123", "somekey123") + + +def test_check_bind_treats_the_whole_loopback_range_as_local() -> None: + """127.0.0.2 is loopback too; one definition, shared with the hub.""" + setup_service.check_bind("127.0.0.2", None, None) + + +def test_check_bind_loopback_host_with_remote_public_url_raises() -> None: + """A tunnel or reverse proxy in front of a loopback bind is the same exposure.""" + with pytest.raises(setup_service.SetupError, match="refusing to advertise"): + setup_service.check_bind( + "127.0.0.1", None, None, "https://hub.example.net" + ) + + +def test_check_bind_loopback_with_remote_url_and_both_credentials_is_ok() -> None: + """Both doors gated, so advertising the loopback bind elsewhere is fine.""" + setup_service.check_bind( + "127.0.0.1", "sometoken123", "somekey123", "https://hub.example.net" + ) + + +def test_check_bind_loopback_host_with_loopback_public_url_is_ok() -> None: + """A loopback ``public_url`` is just a nicer address, not an exposure.""" + setup_service.check_bind("127.0.0.1", None, None, "http://localhost:8765") + + +def test_check_bind_loopback_host_without_public_url_is_unchanged() -> None: + """No advertised address at all keeps the original, credential-free posture.""" + setup_service.check_bind("127.0.0.1", None, None, None) + + +def test_validate_tokens_rejects_a_hostile_agent_key() -> None: + """The agent key rides the same plist/env plumbing, so same charset bound.""" + with pytest.raises(setup_service.SetupError) as excinfo: + setup_service.validate_tokens(None, None, "key with spaces") + assert "--agent-key" in str(excinfo.value) + + +def test_render_unit_launchd_carries_the_agent_key() -> None: + """A launchd plist embeds CAUCUS_AGENT_KEY alongside the dashboard tokens.""" + plist = _render_launchd(operator_token="op123", agent_key="key123") + assert "CAUCUS_AGENT_KEY" in plist + assert "key123" in plist + + +def test_write_env_file_carries_the_agent_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The systemd env file gains a CAUCUS_AGENT_KEY line when a key is set.""" + target = tmp_path / "caucus-hub.env" + monkeypatch.setattr(setup_service, "env_file_path", lambda: target) + written = setup_service.write_env_file(None, None, "key123") + assert written == target + assert "CAUCUS_AGENT_KEY=key123" in target.read_text(encoding="utf-8") + + +def test_render_unit_launchd_carries_the_remote_settings() -> None: + """A remote install needs the advertised URL, the Host allowlist and /mcp. + + Without them the unit starts a hub with ``/mcp`` off (the non-loopback + default) advertising an address nothing off-box can dial -- exactly the + deployment the bind refusal tells the operator to build. + """ + plist = _render_launchd( + operator_token="op123", + agent_key="key123", + public_url="https://hub.example.net", + allowed_hosts=["hub.lan", "hub.example.net"], + mcp_http=True, + ) + assert "https://hub.example.net" in plist + assert "hub.lan,hub.example.net" in plist + assert "CAUCUS_MCP_HTTP" in plist + + +def test_write_env_file_carries_the_remote_settings( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """systemd reads the same six variables out of the environment file.""" + target = tmp_path / "caucus-hub.env" + monkeypatch.setattr(setup_service, "env_file_path", lambda: target) + setup_service.write_env_file( + None, None, None, "https://hub.example.net", ["hub.lan"], True + ) + body = target.read_text(encoding="utf-8") + assert "CAUCUS_PUBLIC_URL=https://hub.example.net" in body + assert "CAUCUS_ALLOWED_HOSTS=hub.lan" in body + assert "CAUCUS_MCP_HTTP=1" in body + + +def test_mcp_http_is_only_ever_written_as_the_opt_in() -> None: + """Absent means "let the hub decide", which is on for loopback.""" + assert not any( + name == "CAUCUS_MCP_HTTP" for name, _ in setup_service.service_environment() + ) + + +def test_validate_addresses_rejects_a_hostile_allowed_host() -> None: + """Allowed hosts ride the same plist/env plumbing, so same charset bound.""" + with pytest.raises(setup_service.SetupError): + setup_service.validate_addresses(None, ["hub.lan; rm -rf /"]) + + +def test_validate_addresses_rejects_a_public_url_the_hub_would_refuse() -> None: + """Catch a URL with a path while installing, not at the first failed start.""" + with pytest.raises(setup_service.SetupError) as excinfo: + setup_service.validate_addresses("https://hub.example.net/caucus", None) + assert "bare origin" in str(excinfo.value) # --------------------------------------------------------------------------- diff --git a/tests/test_watch.py b/tests/test_watch.py index 0b4ecfd..5f7bbcd 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -14,7 +14,9 @@ import httpx import pytest +from caucus import hub as hub_module from caucus import watch as watch_module +from caucus.hub import AuthConfig def _register_peer(base: str, project: str) -> str: @@ -220,7 +222,19 @@ def test_watch_returns_one_on_unknown_token( assert "watch_command()" in emitted[0] -# --- token resolution ---------------------------------------------------- +# --- credential resolution ----------------------------------------------- +# +# One chain, flags before environment: +# --token > --token-file > --ticket > CAUCUS_TOKEN > CAUCUS_TICKET +# The first three cases below are the historical token order, unchanged; the +# rest pin where the ticket slots into it. + + +@pytest.fixture(autouse=True) +def _no_credential_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the developer's own CAUCUS_* credentials out of every case.""" + monkeypatch.delenv("CAUCUS_TOKEN", raising=False) + monkeypatch.delenv("CAUCUS_TICKET", raising=False) def test_resolve_token_prefers_explicit_flag( @@ -229,7 +243,10 @@ def test_resolve_token_prefers_explicit_flag( file = tmp_path / "tok" file.write_text("from-file") monkeypatch.setenv("CAUCUS_TOKEN", "from-env") - assert watch_module._resolve_token("from-flag", str(file)) == "from-flag" + assert watch_module._resolve_credential("from-flag", str(file), "tk") == ( + "from-flag", + None, + ) def test_resolve_token_reads_file_over_env( @@ -238,16 +255,75 @@ def test_resolve_token_reads_file_over_env( file = tmp_path / "tok" file.write_text(" from-file\n") # surrounding whitespace is stripped monkeypatch.setenv("CAUCUS_TOKEN", "from-env") - assert watch_module._resolve_token(None, str(file)) == "from-file" + assert watch_module._resolve_credential(None, str(file), "tk") == ( + "from-file", + None, + ) def test_resolve_token_falls_back_to_env(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("CAUCUS_TOKEN", "from-env") - assert watch_module._resolve_token(None, None) == "from-env" + assert watch_module._resolve_credential(None, None, None) == ("from-env", None) + +def test_resolve_token_none_when_nothing_supplied() -> None: + assert watch_module._resolve_credential(None, None, None) == (None, None) -def test_resolve_token_none_when_nothing_supplied( + +def test_ticket_flag_beats_an_ambient_token_env( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.delenv("CAUCUS_TOKEN", raising=False) - assert watch_module._resolve_token(None, None) is None + """Flags win over environment, the rule the module docstring already states.""" + monkeypatch.setenv("CAUCUS_TOKEN", "from-env") + assert watch_module._resolve_credential(None, None, "tk") == (None, "tk") + + +def test_token_env_beats_the_ticket_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Between the two ambient forms, a token needs no round-trip; prefer it.""" + monkeypatch.setenv("CAUCUS_TOKEN", "from-env") + monkeypatch.setenv("CAUCUS_TICKET", "tk-env") + assert watch_module._resolve_credential(None, None, None) == ("from-env", None) + + +def test_ticket_env_is_the_last_resort(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CAUCUS_TICKET", "tk-env") + assert watch_module._resolve_credential(None, None, None) == (None, "tk-env") + + +# --- ticket redemption --------------------------------------------------- + + +def test_redeem_ticket_returns_the_token(live_hub: str) -> None: + """The happy path: one POST exchanges the ticket for the peer token.""" + token = _register_peer(live_hub, "redeemer") + ticket = hub_module.state.issue_watch_ticket(token) + assert watch_module.redeem_ticket(live_hub, ticket) == token + + +def test_redeem_ticket_is_single_use(live_hub: str) -> None: + """A replayed ticket buys nothing, which is the whole point of the design.""" + token = _register_peer(live_hub, "replayer") + ticket = hub_module.state.issue_watch_ticket(token) + assert watch_module.redeem_ticket(live_hub, ticket) == token + assert watch_module.redeem_ticket(live_hub, ticket) is None + + +def test_redeem_ticket_sends_the_agent_key( + live_hub: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """A keyed hub gates the exchange; the watcher reads the key from its env.""" + token = _register_peer(live_hub, "keyed-watcher") + ticket = hub_module.state.issue_watch_ticket(token) + # The live_hub server is module-scoped and reads this global per request, + # so monkeypatch both installs the key and takes it back off afterwards. + monkeypatch.setattr(hub_module, "auth_config", AuthConfig(agent="the-key")) + monkeypatch.delenv("CAUCUS_AGENT_KEY", raising=False) + assert watch_module.redeem_ticket(live_hub, ticket) is None + # Still unspent: the 401 fires before the ticket is ever looked up. + monkeypatch.setenv("CAUCUS_AGENT_KEY", "the-key") + assert watch_module.redeem_ticket(live_hub, ticket) == token + + +def test_redeem_ticket_returns_none_when_the_hub_is_unreachable() -> None: + """A transport failure is fatal like a refusal, not a traceback.""" + assert watch_module.redeem_ticket("http://127.0.0.1:1", "tk") is None