From 794e4e662d378e14f1a3da412b4a72b3d8c83cf1 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 06:54:12 +0000 Subject: [PATCH] docs: defer broker replay protection to its own issue (#494) Per PR review, replay protection is too heavy for the host control server MVP. Drop it from the four-gap framing (now three gaps), remove the enforcement design section and implementation chunk, and track the iat-window + jti-cache work in #494 as an independent in-process change. Co-Authored-By: Claude Opus 4.8 --- docs/prds/prd-new-host-control-server.md | 64 +++++++++--------------- 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/docs/prds/prd-new-host-control-server.md b/docs/prds/prd-new-host-control-server.md index 2a3f623a..fa5beee7 100644 --- a/docs/prds/prd-new-host-control-server.md +++ b/docs/prds/prd-new-host-control-server.md @@ -11,10 +11,10 @@ Promote the in-process launch broker into a standalone **host control server**: the single privileged component on the host. Both the CLI and the orchestrator drive it over HTTP; it brokers agent launches, owns the orchestrator's own lifecycle, and is the sole writer of host-durable state (the -tamper-evident audit record). This closes the four gaps between today's +tamper-evident audit record). This closes the three gaps between today's well-formed broker *contract* ([`orchestrator/broker.py`](../../bot_bottle/orchestrator/broker.py)) and a real out-of-process service — transport, durable provisioned secret, -replay protection, and a disciplined op vocabulary — and splits host state by +and a disciplined op vocabulary — and splits host state by owner and lifetime. The prize: **the CLI no longer needs the Docker socket**, which is what finally lets a dedicated Gitea runner user drop the root-equivalent `docker` group (PRD 0070, "Relationship to other work"). @@ -32,7 +32,7 @@ broker exists and is tested in-process. But it is *only* in-process: and `DockerBroker` is on no production path — every backend starts the orchestrator with `--broker stub` ([`__main__.py:54`](../../bot_bottle/orchestrator/__main__.py)). -Four gaps stand between that scaffold and a host service: +Three gaps stand between that scaffold and a host service: 1. **No transport.** `submit` is an in-process call. A real service needs a `BrokerClient` that POSTs the signed token and a host-side HTTP server that @@ -42,11 +42,7 @@ Four gaps stand between that scaffold and a host service: `secrets.token_bytes(32)` and hands the *same value* to signer and verifier — viable only because they share a process. A separate daemon needs the secret provisioned out of band and durable across orchestrator restarts. -3. **Replay protection is claimed but not enforced.** `sign_request` emits `jti` - and `iat`; `verify_request` reads neither — no expiry window, no `jti` cache - ([`broker.py:88`](../../bot_bottle/orchestrator/broker.py)). Harmless - in-process; over a wire a captured launch token replays indefinitely. -4. **The op vocabulary is `launch` / `teardown` only.** Everything else +3. **The op vocabulary is `launch` / `teardown` only.** Everything else host-privileged still lives in the CLI, so the schema has to grow — carefully, since PRD 0070's security argument rests on "structured requests only, static flags + ids." @@ -65,9 +61,8 @@ tamper-evident home for the audit record that survives orchestrator destruction. - `cli -(http)-> orchestrator -(http)-> host controller -> launch` - `cli -(http)-> host controller` — start / restart / status of the orchestrator **itself** (the bootstrap/recovery path #391 targets). -- The launch op is expressed as a **signed JWT of static flags + ids only** and - verified with **replay protection enforced**: `iat` expiry window + a `jti` - cache reject a captured token. +- The launch op is expressed as a **signed JWT of static flags + ids only**, + verified against a closed schema. - The signing secret is **provisioned out of band and durable** across orchestrator restarts (a `TrustDomain` per #476, with a key the orchestrator never holds for the host controller's *own* endpoints). @@ -136,26 +131,16 @@ requests gains nothing, since it is already the component that launches. Staying symmetric also honors the no-runtime-deps policy (stdlib has no Ed25519). This matches the reasoning already inlined in `broker.py`'s module docstring. -### Replay protection (gap 3) +### Replay protection is out of scope (tracked in #494) -`sign_request` already emits `jti` and `iat`; `verify_request` must start reading -them: +Once the launch token travels over a wire, a captured token could be replayed — +`sign_request` already emits `jti`/`iat` but `verify_request` reads neither, so +there is no expiry window or `jti` cache today. Enforcing that (an `iat` window + +a self-trimming `jti` cache) is a pure in-process change that lands independently +of this work, and it is deferred to **#494** rather than gating the MVP of the +host control server. Nothing here depends on it; it can merge before or after. -- **Expiry window.** Reject a token whose `iat` is outside a small window - (proposed ±60s, one constant) — bounds how long a captured token is useful and - how much clock skew is tolerated. -- **`jti` cache.** Keep a set of seen `jti`s for at least the expiry window; - reject a repeat. The cache only needs to remember tokens that could still be - within their window, so it self-trims by `iat` — no unbounded growth, no - persistence needed (a token older than the window is already rejected by the - expiry check, so a cache lost on restart cannot admit a replay). - -Both are fail-closed additions to the existing `BrokerAuthError` path: a missing -or malformed `jti`/`iat` is a rejection, not a pass. In-process callers are -unaffected in practice (fresh token per submit), so this can land ahead of the -transport. - -### Op vocabulary and the "ids + static flags" rule (gap 4) +### Op vocabulary and the "ids + static flags" rule (gap 3) Each op moved off the CLI widens the privileged surface, so growth is governed by one explicit rule, enforced in `verify_request`'s schema check: @@ -248,22 +233,20 @@ audit-append channel. Ordered, each independently mergeable: -1. **Enforce replay protection** in `verify_request` (`iat` window + `jti` - cache). Pure in-process change; no transport needed. Closes gap 3. -2. **`BrokerClient` + host launch server** over HTTP, reusing `verify_request` +1. **`BrokerClient` + host launch server** over HTTP, reusing `verify_request` and the existing `DockerBroker` bodies. Wire `OrchestratorCore` to a `BrokerClient` behind a flag; keep `StubBroker` for the dev-harness. Closes gap 1. -3. **Durable secret via `TrustDomain`** — provision the launch-broker key to +2. **Durable secret via `TrustDomain`** — provision the launch-broker key to signer + verifier; add the host controller's own lifecycle `TrustDomain`. Closes gap 2. -4. **Grow the op vocabulary** one op at a time (`list_live` first — it also +3. **Grow the op vocabulary** one op at a time (`list_live` first — it also removes `reconcile`'s `live_source_ips`), each behind the ids + static-flags - rule. Closes gap 4. -5. **JSONL audit log** — the host-controller-owned, hash-chained historical + rule. Closes gap 3. +4. **JSONL audit log** — the host-controller-owned, hash-chained historical record with the plain-bearer audit-append handler; redirect the egress traffic log into it. -6. **Drop the Docker socket from the CLI** once every host-privileged op it used +5. **Drop the Docker socket from the CLI** once every host-privileged op it used is a broker op — the payoff that unblocks the unprivileged Gitea runner user. ## Open questions @@ -271,10 +254,7 @@ Ordered, each independently mergeable: 1. **Schema-width rule enforcement.** The "ids + static flags" rule is stated; should `verify_request` reject unknown claim keys outright (strict schema) to keep the surface from drifting? Leaning yes. -2. **`jti` cache scope under a plane split.** With one host controller per host - the in-memory cache is sufficient; if lifecycle ever fans out, the cache and - the expiry window need to be co-located with the single verifier. -3. **Audit-append back-pressure.** What the audit handler does if the JSONL sink +2. **Audit-append back-pressure.** What the audit handler does if the JSONL sink is unavailable (fail-closed vs. buffer) — resolve before shipping chunk 5. ## References @@ -285,6 +265,8 @@ Ordered, each independently mergeable: - **#476** ([`prd-new-control-plane-auth-provisioning`](prd-new-control-plane-auth-provisioning.md)) — the `TrustDomain` seam this plugs the host controller's key into. - **#391** — backend-agnostic orchestrator restart (the bootstrap path). +- **#494** — enforce broker replay protection (`iat` window + `jti` cache); split + out of this PRD as an independent in-process change. - **#386** — prebuilt images from the Gitea OCI registry (the fixed image set the broker validates against). - **#355** — generic `SecretProvider`.