From cb2d778a8fdd25d3d9dde87ca851be13350fff3c Mon Sep 17 00:00:00 2001 From: didericis Date: Sat, 25 Jul 2026 15:48:16 -0400 Subject: [PATCH] refactor: remove leftovers from the orchestrator/gateway consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep for vestiges of the old combined-plane model and the pre-split shared rootfs. Two are load-bearing, the rest are stale docs/comments: - Bug: macOS `enumerate_active` only excluded the gateway container from the agent list, so after the split the orchestrator container (`bot-bottle-mac-orchestrator`, also `bot-bottle-`-prefixed) was enumerated as a phantom agent. Exclude both infra containers; test covers it. - Dead code: the gateway `bootstrap.py` still carried an `orchestrator` daemon spec + `_OPT_IN_DAEMONS` + a signing-key/JWT env branch, all for the old combined container where the gateway process could also run the control plane. No backend ever requests it now — removed; the key-stripping stays as defense-in-depth. Stale-comment reframes: "the/single infra container" -> the orchestrator + gateway pair (or the specific plane); "shared rootfs / bb_role init / one published rootfs" -> the per-plane rootfs + `role_init`; the deleted Dockerfile.infra references in Dockerfile.orchestrator/.gateway; and the macOS "one infra container ... same address" docstring + its now-false share-one-address test (the planes are distinct containers with distinct addresses). Co-Authored-By: Claude Opus 4.8 --- Dockerfile.gateway | 9 ++-- Dockerfile.orchestrator | 18 ++++---- bot_bottle/backend/docker/__init__.py | 2 +- .../backend/docker/consolidated_launch.py | 12 ++--- .../backend/firecracker/firecracker_vm.py | 4 +- bot_bottle/backend/firecracker/gateway.py | 32 ++++++------- .../backend/firecracker/orchestrator.py | 16 +++---- bot_bottle/backend/firecracker/util.py | 4 +- bot_bottle/backend/macos_container/backend.py | 4 +- .../macos_container/consolidated_launch.py | 20 ++++---- .../backend/macos_container/enumerate.py | 11 +++-- .../backend/macos_container/gateway_hosts.py | 2 +- bot_bottle/gateway/bootstrap.py | 46 ++++++------------- bot_bottle/orchestrator/client.py | 4 +- tests/unit/test_gateway_init.py | 15 ++---- tests/unit/test_macos_consolidated_launch.py | 18 ++++---- tests/unit/test_macos_container_cleanup.py | 10 ++-- 17 files changed, 103 insertions(+), 124 deletions(-) diff --git a/Dockerfile.gateway b/Dockerfile.gateway index ad7001c6..4ae12511 100644 --- a/Dockerfile.gateway +++ b/Dockerfile.gateway @@ -37,12 +37,9 @@ # 9100 supervise (MCP HTTP) # Based on `python:3.12-slim` (Debian trixie) rather than the -# `mitmproxy/mitmproxy` image (Debian bookworm) so the whole stack — -# gateway here, and the firecracker infra image that builds FROM this — -# lands on trixie, whose buildah (1.39) can build agent Dockerfiles that -# use heredocs. mitmproxy is pip-installed to the same effect as the -# upstream image. (bookworm's buildah is 1.28, which can't parse -# `RUN ... < str: def _network_container_ips(network: str) -> list[str]: """Every address currently assigned on the gateway network — the ground - truth for "in use": the infra container and every live agent. Read from + truth for "in use": the gateway container and every live agent. Read from the network so a new bottle can't collide with anything actually attached.""" proc = run_docker([ "docker", "network", "inspect", "--format", @@ -80,7 +80,7 @@ def _reprovision_running_bottles( infra_name: str = INFRA_NAME, ) -> None: """Re-inject egress tokens for any registered bottles that lost their - in-memory tokens (e.g., after an infra container restart). + in-memory tokens (e.g., after an orchestrator restart). For each registered bottle whose source IP maps to a live container on the gateway network, reads ENV_VAR_SECRET via ``docker exec … printenv`` and @@ -89,7 +89,7 @@ def _reprovision_running_bottles( container exec failure never blocks a new bottle launch.""" client = OrchestratorClient(orchestrator_url) # Build {source_ip: container_name} from live containers on the gateway - # network, excluding the infra container itself. + # network, excluding the gateway container itself. try: proc = run_docker([ "docker", "network", "inspect", @@ -133,11 +133,11 @@ def launch_consolidated( infra_name: str = INFRA_NAME, network: str = GATEWAY_NETWORK, ) -> LaunchContext: - """Ensure the infra container is up, allocate + register the bottle, and + """Ensure the orchestrator + gateway pair is up, allocate + register the bottle, and provision its git-gate state. Returns the agent's attach context. Also reprovisiones egress tokens for any already-running bottles that lost - their in-memory credentials (e.g. after an infra container restart), so + their in-memory credentials (e.g. after an orchestrator restart), so they regain egress access before the new bottle is registered.""" service = service or DockerInfraService() url = service.ensure_running() diff --git a/bot_bottle/backend/firecracker/firecracker_vm.py b/bot_bottle/backend/firecracker/firecracker_vm.py index 129a96c5..6ccd81ce 100644 --- a/bot_bottle/backend/firecracker/firecracker_vm.py +++ b/bot_bottle/backend/firecracker/firecracker_vm.py @@ -69,8 +69,8 @@ def _boot_args( pub_b64 = base64.b64encode(pubkey.encode()).decode() args = f"{_BASE_BOOT_ARGS} {ip_arg} bb_pubkey={pub_b64}" # `extra` carries caller-supplied cmdline params the guest init reads - # (e.g. `bb_role=orchestrator|gateway` selecting which infra plane a - # shared-rootfs infra VM runs). Agent VMs pass nothing. + # (e.g. the gateway VM's `bb_orch=`). Agent VMs pass + # nothing. return f"{args} {extra}".rstrip() if extra else args diff --git a/bot_bottle/backend/firecracker/gateway.py b/bot_bottle/backend/firecracker/gateway.py index 30859d53..2ef1e551 100644 --- a/bot_bottle/backend/firecracker/gateway.py +++ b/bot_bottle/backend/firecracker/gateway.py @@ -8,14 +8,13 @@ gateway VM never opens `bot-bottle.db` (#469), so it holds no signing key — only the token the host hands it via `connect_to_orchestrator`. What deliberately stays in `infra_vm` (not moved here): the plane-agnostic VM -substrate the orchestrator VM shares — booting a VM from the shared rootfs +substrate the orchestrator VM also uses — booting a VM from a per-plane rootfs (`boot_vm`), the stable SSH keypair, the secret-push retry loop, and the PID lifecycle — plus the pair coordinator (`ensure_running`: orchestrator-first health gate, singleton lock, adoption/version marker). The guest-side gateway -daemon startup lives in the shared `bb_role=gateway` init branch baked into the -one published rootfs both VMs boot, so it can't live in a host-side method -either. These are the shared seam that moves to a neutral module when the -Orchestrator service lands and `infra_vm` is dissolved. +daemon startup lives in the gateway rootfs's guest init (`_gateway_init` in +`infra_vm`), so it can't live in a host-side method either. These are the shared +seam. """ from __future__ import annotations @@ -33,18 +32,19 @@ from .. import util as backend_util from . import infra_vm, netpool, util from .gateway_transport import FirecrackerGatewayTransport -# The gateway microVM's name (the `bb_role=gateway` boot tag / run dir). Fixed -# per host — one gateway VM, shared by every agent VM. +# The gateway microVM's name (its run dir). Fixed per host — one gateway VM, +# shared by every agent VM. GATEWAY_NAME = "bot-bottle-gateway" -# The gateway VM's slim memory ceiling (buildah is present on the shared rootfs -# but unused on the data plane) — PRD 0070 "Memory: fixed ceilings". +# The gateway VM's slim memory ceiling — the data plane carries no build +# tooling (buildah lives only on the orchestrator rootfs) — PRD 0070 +# "Memory: fixed ceilings". _GW_MEM_MIB = 2048 -# The pre-minted `gateway` JWT path in the guest. The *shared* init (both planes -# boot one rootfs) waits for it before starting the data plane, so its canonical -# definition lives with that init in `infra_vm`; imported here for the push so -# the load-bearing path isn't duplicated. +# The pre-minted `gateway` JWT path in the guest. The gateway init (in +# `infra_vm`) waits for it before starting the data plane, so its canonical +# definition lives with that init; imported here for the push so the +# load-bearing path isn't duplicated. _GUEST_GATEWAY_JWT_PATH = infra_vm._GUEST_GATEWAY_JWT_PATH # mitmproxy writes its CA here a beat after start; agents install it to trust # the gateway's TLS interception. Host-side only (SSH cat), so it lives here. @@ -56,7 +56,7 @@ _CA_FETCH_TIMEOUT_SECONDS = 15.0 class FirecrackerGateway(Gateway): """The consolidated gateway as a Firecracker microVM on the gateway link. - The shared rootfs is built/downloaded by `infra_vm.ensure_built` (the ABC's + The gateway rootfs is built/downloaded by `infra_vm.ensure_built` (the ABC's `ensure_built` no-op here); `connect_to_orchestrator` boots the VM resolving policy against the orchestrator and seeds the pre-minted `gateway` token.""" @@ -94,8 +94,8 @@ class FirecrackerGateway(Gateway): raise GatewayError( f"cannot resolve orchestrator guest IP from {self._orchestrator_url!r}" ) - # Boot on the gateway link from the shared rootfs (bb_role=gateway), then - # push the token the init waits for before starting the data plane. + # Boot on the gateway link from the gateway rootfs, then push the token + # the init waits for before starting the data plane. vm = infra_vm.boot_vm( name=GATEWAY_NAME, slot=netpool.gw_slot(), run_dir=infra_vm._gw_dir(), role="gateway", mem_mib=_GW_MEM_MIB, diff --git a/bot_bottle/backend/firecracker/orchestrator.py b/bot_bottle/backend/firecracker/orchestrator.py index 86dfa5aa..4ed2d452 100644 --- a/bot_bottle/backend/firecracker/orchestrator.py +++ b/bot_bottle/backend/firecracker/orchestrator.py @@ -8,11 +8,11 @@ signing key over SSH, and waiting for `/health`. The host CLI reaches it at its guest IP, and so does the gateway (`bb_orch` cmdline), so `url()` == `gateway_url()`. What deliberately stays in `infra_vm` (not moved here): the plane-agnostic VM -substrate the gateway VM also shares — booting a VM from the shared rootfs +substrate the gateway VM also uses — booting a VM from a per-plane rootfs (`boot_vm`), the stable SSH keypair, the secret-push retry, the PID lifecycle — plus the pair coordinator (`ensure_running`: adopt-or-boot-both under a singleton -lock with a shared version marker) and the single shared `bb_role`-branched init -baked into the one published rootfs. Those are the shared seam. +lock with a combined version marker) and the per-plane guest inits (`role_init`). +Those are the shared seam. """ from __future__ import annotations @@ -30,8 +30,8 @@ from ...orchestrator.lifecycle import ( from . import firecracker_vm, infra_vm, netpool from .infra_vm import ORCHESTRATOR_PORT -# The orchestrator microVM's name (the `bb_role=orchestrator` boot tag / run -# dir). Fixed per host — one control-plane VM. +# The orchestrator microVM's name (its run dir). Fixed per host — one +# control-plane VM. ORCHESTRATOR_NAME = "bot-bottle-orchestrator" # Memory ceiling (fixed at boot, demand-paged). The orchestrator keeps the build @@ -58,9 +58,9 @@ def registry_volume_path() -> Path: class FirecrackerOrchestrator(Orchestrator): """The control plane as a Firecracker microVM on the orchestrator link. - The shared rootfs is built/downloaded by `infra_vm.ensure_built` (the ABC's - `ensure_built` no-op here); `ensure_running` boots the VM, seeds the signing - key, and blocks until `/health` answers.""" + The orchestrator rootfs is built/downloaded by `infra_vm.ensure_built` (the + ABC's `ensure_built` no-op here); `ensure_running` boots the VM, seeds the + signing key, and blocks until `/health` answers.""" name = ORCHESTRATOR_NAME diff --git a/bot_bottle/backend/firecracker/util.py b/bot_bottle/backend/firecracker/util.py index 465c3482..f7cd8e67 100644 --- a/bot_bottle/backend/firecracker/util.py +++ b/bot_bottle/backend/firecracker/util.py @@ -258,8 +258,8 @@ def build_committed_rootfs_dir(tar_path: Path) -> Path: def inject_guest_boot(rootfs: Path, init_script: str | None = None) -> None: """Drop the static dropbear and the PID-1 init into the rootfs. - `init_script` defaults to the SSH-only agent init; the infra VM - passes its own (control plane + gateway) init. + `init_script` defaults to the SSH-only agent init; each infra VM + passes its own per-plane init (orchestrator or gateway). A committed snapshot is guest-controlled, so `bb-dropbear`/`bb-init` may already exist as symlinks aimed at a host file (e.g. bb-init -> diff --git a/bot_bottle/backend/macos_container/backend.py b/bot_bottle/backend/macos_container/backend.py index d8241ea1..ba09e5a0 100644 --- a/bot_bottle/backend/macos_container/backend.py +++ b/bot_bottle/backend/macos_container/backend.py @@ -101,10 +101,10 @@ class MacosContainerBottleBackend( yield bottle def ensure_orchestrator(self) -> str: - """Bring up the per-host infra container (control plane + gateway) and + """Bring up the per-host pair (orchestrator + gateway containers) and return its control-plane URL — the on-demand entry point operator tools (`supervise`) call when no control plane is running yet. Mirrors - firecracker's infra-VM bring-up.""" + firecracker's infra bring-up.""" from .infra import MacosInfraService return MacosInfraService().ensure_running().orchestrator_url diff --git a/bot_bottle/backend/macos_container/consolidated_launch.py b/bot_bottle/backend/macos_container/consolidated_launch.py index d0211123..e67ba418 100644 --- a/bot_bottle/backend/macos_container/consolidated_launch.py +++ b/bot_bottle/backend/macos_container/consolidated_launch.py @@ -15,8 +15,10 @@ caller has to start the agent in between. `ensure_gateway` runs first because the agent's proxy env needs the gateway's address at `container run` time; the agent's *own* address (the attribution key) only exists afterwards. -The control plane and the gateway are one **infra container** here (see -`infra`), so `gateway_ip` and the control-plane host are the same address. +The control plane and the gateway are **separate containers** here (see +`infra`): the orchestrator on the host-only control network, the gateway on the +agent network — `gateway_ip` is the gateway container's agent-network address, +distinct from the orchestrator's control-network host. The consequence for the identity token: it is minted by registration, i.e. *after* the agent container exists, so it cannot be baked into the run-time @@ -54,9 +56,9 @@ class ConsolidatedLaunchError(RuntimeError): @dataclass(frozen=True) class GatewayEndpoint: - """What the agent `container run` needs to reach the shared gateway (the - infra container). `gateway_ip` is that container's host-only address, the - same host the control-plane URL points at.""" + """What the agent `container run` needs to reach the shared gateway. + `gateway_ip` is the gateway container's agent-network address (the agent's + proxy target); `orchestrator_url` points at the separate control plane.""" orchestrator_url: str gateway_ip: str # the gateway's address — the agent's proxy target @@ -80,10 +82,10 @@ class LaunchContext: def ensure_gateway( *, service: MacosInfraService | None = None, ) -> GatewayEndpoint: - """Ensure the per-host infra container (control plane + gateway) is up and - report how to reach it. Idempotent — one singleton, so N bottle launches - share it. Call before starting the agent container: the agent's proxy env - needs `gateway_ip` at run time.""" + """Ensure the per-host pair (orchestrator + gateway containers) is up and + report how to reach the gateway. Idempotent — one singleton pair, so N bottle + launches share it. Call before starting the agent container: the agent's + proxy env needs `gateway_ip` at run time.""" service = service or MacosInfraService() infra = service.ensure_running() endpoint = GatewayEndpoint( diff --git a/bot_bottle/backend/macos_container/enumerate.py b/bot_bottle/backend/macos_container/enumerate.py index 01ce2076..18421edc 100644 --- a/bot_bottle/backend/macos_container/enumerate.py +++ b/bot_bottle/backend/macos_container/enumerate.py @@ -6,17 +6,18 @@ import subprocess from ...bottle_state import read_metadata from .. import ActiveAgent -from .infra import INFRA_NAME +from .infra import INFRA_NAME, ORCHESTRATOR_NAME # The name every agent container carries: `bot-bottle-`. Exported # because callers that act on a running bottle (gateway-host rewrites, # registry reconciliation) have to map an enumerated slug back to a # container name. CONTAINER_NAME_PREFIX = "bot-bottle-" -# The shared per-host infra container carries the same prefix as agent -# containers but is infrastructure, not a bottle — one control plane + gateway -# serves every agent, so listing it as an agent would invent one per host. -_INFRA_NAMES = frozenset({INFRA_NAME}) +# The two shared per-host infra containers (orchestrator + gateway) carry the +# same `bot-bottle-` prefix as agent containers but are infrastructure, not +# bottles — one pair serves every agent, so enumerating either as an agent would +# invent a phantom bottle per host. +_INFRA_NAMES = frozenset({INFRA_NAME, ORCHESTRATOR_NAME}) class EnumerationError(RuntimeError): diff --git a/bot_bottle/backend/macos_container/gateway_hosts.py b/bot_bottle/backend/macos_container/gateway_hosts.py index 9f5735db..804b61a5 100644 --- a/bot_bottle/backend/macos_container/gateway_hosts.py +++ b/bot_bottle/backend/macos_container/gateway_hosts.py @@ -1,7 +1,7 @@ """Stable gateway name for macOS agents, via each bottle's `/etc/hosts`. The shared gateway's address is assigned by vmnet's DHCP and changes whenever -the infra container is recreated — a source-hash bump, an image upgrade, a +the gateway container is recreated — a source-hash bump, an image upgrade, a crash. Every agent-facing URL (egress proxy, git-http, supervise) embeds that address, and the proxy URL reaches the agent as **process environment** at `container exec` time. A running process's `environ` cannot be rewritten from diff --git a/bot_bottle/gateway/bootstrap.py b/bot_bottle/gateway/bootstrap.py index 4e1c6d16..7d7c1b7c 100644 --- a/bot_bottle/gateway/bootstrap.py +++ b/bot_bottle/gateway/bootstrap.py @@ -54,19 +54,15 @@ class _DaemonSpec: _EGRESS_ONLY_ENV_PREFIXES: tuple[str, ...] = ("EGRESS_TOKEN_",) _READY_GATED_DAEMONS: tuple[str, ...] = ("git-gate", "git-http") -# The control-plane signing key is the orchestrator's alone — verifying tokens. -# The data-plane daemons instead hold the pre-minted `gateway` JWT they present. -# Scoping each to its process (even in the combined infra container) keeps a -# compromised data-plane daemon from reading the key and minting a `cli` token -# (issue #469 review). Values match paths.ORCHESTRATOR_TOKEN_ENV / -# ORCHESTRATOR_AUTH_JWT_ENV; hardcoded here so this supervisor stays import-light. +# The control-plane signing key is the orchestrator's alone (it verifies +# tokens), and the orchestrator runs in a separate container/VM — the gateway +# only ever holds the pre-minted `gateway` JWT its daemons present. Strip the +# key from every daemon's env as defense-in-depth, so a compromised data-plane +# daemon can't read it and mint a `cli` token even if it somehow leaked into the +# gateway container (issue #469 review). Value matches +# paths.ORCHESTRATOR_TOKEN_ENV; hardcoded here so this supervisor stays +# import-light. _SIGNING_KEY_ENV = "BOT_BOTTLE_ORCHESTRATOR_TOKEN" -_GATEWAY_JWT_ENV = "BOT_BOTTLE_ORCHESTRATOR_AUTH_JWT" - -# Daemons that must be requested explicitly via BOT_BOTTLE_GATEWAY_DAEMONS -# and are NOT started in the default (env-var-unset) case. The orchestrator -# only runs in the combined infra container, never in a standalone gateway. -_OPT_IN_DAEMONS: frozenset[str] = frozenset({"orchestrator"}) def _env_for_daemon(name: str, base_env: dict[str, str]) -> dict[str, str]: @@ -74,30 +70,20 @@ def _env_for_daemon(name: str, base_env: dict[str, str]) -> dict[str, str]: Returns a fresh dict — callers can mutate without affecting `base_env`. * `EGRESS_TOKEN_*` upstream-auth slots go to egress only. - * the control-plane signing key goes to the orchestrator only. - * the pre-minted `gateway` JWT goes to the data-plane daemons only (the - orchestrator verifies tokens, it never presents one).""" + * the control-plane signing key is stripped from every daemon — the + gateway never holds it (it's the orchestrator's); the data-plane daemons + present the pre-minted `gateway` JWT instead.""" env = dict(base_env) if name != "egress": env = { k: v for k, v in env.items() if not any(k.startswith(p) for p in _EGRESS_ONLY_ENV_PREFIXES) } - if name == "orchestrator": - env.pop(_GATEWAY_JWT_ENV, None) - else: - env.pop(_SIGNING_KEY_ENV, None) + env.pop(_SIGNING_KEY_ENV, None) return env -# The orchestrator is listed first so it starts before the gateway daemons, -# giving the control plane a head start to accept /resolve calls. The gateway -# daemons tolerate early /resolve failures and retry per-request. _DAEMONS: tuple[_DaemonSpec, ...] = ( - _DaemonSpec("orchestrator", ( - "python3", "-m", "bot_bottle.orchestrator", - "--host", "0.0.0.0", "--port", "8099", "--broker", "stub", - )), _DaemonSpec("egress", ("/bin/sh", "/app/egress-entrypoint.sh")), _DaemonSpec("git-gate", ("/bin/sh", "/git-gate-entrypoint.sh")), _DaemonSpec("git-http", ("python3", "-m", "bot_bottle.gateway.git_gate.http_backend")), @@ -127,10 +113,8 @@ def _selected_daemons( ) -> tuple[_DaemonSpec, ...]: """Filter the daemon set by the BOT_BOTTLE_GATEWAY_DAEMONS env var. - When the var is unset/empty, return all non-opt-in daemons (the - standard gateway subset). Opt-in daemons (e.g. `orchestrator`) only - run when explicitly named — they never start in a plain gateway - container that doesn't set the env var. Unknown names are ignored. + When the var is unset/empty, return all daemons (the standard gateway + subset). Unknown names are ignored. `all_daemons` defaults to `_DAEMONS` resolved at call time (not at definition time), so tests can pass a custom list.""" @@ -138,7 +122,7 @@ def _selected_daemons( all_daemons = _DAEMONS raw = env.get("BOT_BOTTLE_GATEWAY_DAEMONS", "").strip() if not raw: - return tuple(d for d in all_daemons if d.name not in _OPT_IN_DAEMONS) + return tuple(all_daemons) wanted = {n.strip() for n in raw.split(",") if n.strip()} return tuple(d for d in all_daemons if d.name in wanted) diff --git a/bot_bottle/orchestrator/client.py b/bot_bottle/orchestrator/client.py index b6c95492..96ef47bd 100644 --- a/bot_bottle/orchestrator/client.py +++ b/bot_bottle/orchestrator/client.py @@ -182,7 +182,7 @@ class OrchestratorClient: """Drop registry rows for bottles that are no longer running (`POST /reconcile`), returning the reaped bottle ids. `live_source_ips` is the caller's enumeration of its live bottles — the orchestrator - can't see the backend from inside the infra container.""" + can't see the backend from inside the orchestrator container/VM.""" body: dict[str, object] = {"live_source_ips": list(live_source_ips)} if grace_seconds is not None: body["grace_seconds"] = grace_seconds @@ -257,7 +257,7 @@ def discover_orchestrator_url(*, timeout: float = 2.0) -> str: f"http://{netpool.orch_slot().guest_ip}:{ORCHESTRATOR_PORT}") except Exception: # noqa: BLE001 — backend optional / not firecracker pass - try: # macOS: infra container control plane on its host-only address + try: # macOS: orchestrator container on its host-only address from ..backend.macos_container.infra import probe_orchestrator_url url = probe_orchestrator_url() if url: diff --git a/tests/unit/test_gateway_init.py b/tests/unit/test_gateway_init.py index 1fb471f5..429bdab5 100644 --- a/tests/unit/test_gateway_init.py +++ b/tests/unit/test_gateway_init.py @@ -64,11 +64,11 @@ class TestEnvForDaemon(unittest.TestCase): self.assertNotIn("X", self._BASE) -class TestOrchestratorEnvScoping(unittest.TestCase): - """The control-plane signing key stays with the orchestrator; the pre-minted - `gateway` JWT goes to the data-plane daemons (issue #469 review). Scoping - them per-process keeps a compromised data-plane daemon from reading the key - and minting a higher-privilege token, even in the combined infra container.""" +class TestSigningKeyScoping(unittest.TestCase): + """The gateway never holds the control-plane signing key — it's the + orchestrator's, which runs in a separate container/VM. Its daemons present + the pre-minted `gateway` JWT instead; the key is stripped from every daemon's + env as defense-in-depth (issue #469 review).""" _BASE = { "PATH": "/usr/bin", @@ -76,11 +76,6 @@ class TestOrchestratorEnvScoping(unittest.TestCase): "BOT_BOTTLE_ORCHESTRATOR_AUTH_JWT": "gw-jwt", } - def test_orchestrator_gets_key_not_jwt(self): - env = _env_for_daemon("orchestrator", self._BASE) - self.assertEqual("sk-x", env["BOT_BOTTLE_ORCHESTRATOR_TOKEN"]) - self.assertNotIn("BOT_BOTTLE_ORCHESTRATOR_AUTH_JWT", env) - def test_data_plane_daemons_get_jwt_not_key(self): for name in ("egress", "git-gate", "git-http", "supervise"): env = _env_for_daemon(name, self._BASE) diff --git a/tests/unit/test_macos_consolidated_launch.py b/tests/unit/test_macos_consolidated_launch.py index 51e8f36b..93747cff 100644 --- a/tests/unit/test_macos_consolidated_launch.py +++ b/tests/unit/test_macos_consolidated_launch.py @@ -57,9 +57,11 @@ class TestEnsureGateway(unittest.TestCase): def _service(self) -> MagicMock: from bot_bottle.backend.macos_container.infra import InfraEndpoint service = MagicMock() + # Two containers now: the orchestrator on the control network, the + # gateway on the agent network — distinct addresses. service.ensure_running.return_value = InfraEndpoint( orchestrator_url="http://192.168.128.2:8099", - gateway_ip="192.168.128.2", + gateway_ip="192.168.128.3", ) service.network = "bot-bottle-mac-gateway" service.ca_cert_pem.return_value = "PEM" @@ -68,18 +70,16 @@ class TestEnsureGateway(unittest.TestCase): def test_reports_gateway_endpoint(self) -> None: endpoint = self._run(self._service()) self.assertEqual("http://192.168.128.2:8099", endpoint.orchestrator_url) - self.assertEqual("192.168.128.2", endpoint.gateway_ip) + self.assertEqual("192.168.128.3", endpoint.gateway_ip) self.assertEqual("PEM", endpoint.gateway_ca_pem) self.assertEqual("bot-bottle-mac-gateway", endpoint.network) - def test_orchestrator_and_gateway_share_one_address(self) -> None: - """One infra container hosts both, so the gateway IP and the - control-plane host are the same.""" + def test_gateway_ip_is_distinct_from_the_control_plane(self) -> None: + # The planes are separate containers, so the agent's proxy target (the + # gateway) is a different address from the control-plane host. endpoint = self._run(self._service()) - self.assertEqual( - endpoint.gateway_ip, - endpoint.orchestrator_url.split("://")[1].split(":")[0], - ) + orch_host = endpoint.orchestrator_url.split("://")[1].split(":")[0] + self.assertNotEqual(orch_host, endpoint.gateway_ip) class TestRegisterAgent(unittest.TestCase): diff --git a/tests/unit/test_macos_container_cleanup.py b/tests/unit/test_macos_container_cleanup.py index 48b68719..0651b99f 100644 --- a/tests/unit/test_macos_container_cleanup.py +++ b/tests/unit/test_macos_container_cleanup.py @@ -60,10 +60,12 @@ class TestMacosContainerEnumerate(unittest.TestCase): self.assertEqual(["dev-abc"], [a.slug for a in agents]) self.assertEqual(["macos-container"], [a.backend_name for a in agents]) - def test_excludes_the_infra_singleton(self): - """The infra container shares the bot-bottle- prefix but is - infrastructure — listing it would invent an agent per host.""" - agents = self._enumerate("bot-bottle-mac-infra\nbot-bottle-dev-abc\n") + def test_excludes_both_infra_containers(self): + """Both infra containers (orchestrator + gateway) share the bot-bottle- + prefix but are infrastructure — listing either would invent a phantom + agent per host.""" + agents = self._enumerate( + "bot-bottle-mac-infra\nbot-bottle-mac-orchestrator\nbot-bottle-dev-abc\n") self.assertEqual(["dev-abc"], [a.slug for a in agents]) def test_raises_when_the_cli_fails(self):