From ca91fc4d91b290a082d8be3e37516d5b097da324 Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 17 Jul 2026 05:15:16 -0400 Subject: [PATCH] fix(security): authenticate the orchestrator control plane (issue #400 Critical) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The control plane had no caller authentication — dispatch() routed every request straight through, and the only guards anywhere were the (source_ip, identity_token) checks inside /resolve and /attribute, which authenticate the *bottle* a request is about, not the *caller*. On the consolidated backends the agent shares a network with the control-plane port (macOS: the infra container runs 8099 on the agent's --internal network; docker: the orchestrator container sits on the shared gateway network), so a prompt-injected agent could GET /bottles (enumerate siblings + policies), PUT /bottles//policy (rewrite its own egress allowlist to allow-all), POST /resolve (read the injected upstream credentials it's never meant to see), and POST /supervise/respond (self-approve its own proposals). Every README guarantee collapsed to the agent's discretion. Fix: a per-host control-plane secret required on every route but GET /health, compared with hmac.compare_digest. It is held only by the trusted callers and never handed to an agent: - minted + persisted 0600 at /control-plane-token (paths.host_control_plane_token); - injected as $BOT_BOTTLE_CONTROL_PLANE_TOKEN into the orchestrator + gateway containers via bare `--env NAME` (value inherited from the launch process, so it never lands on argv or in `container/docker inspect`); - presented by the gateway's PolicyResolver (reads the env) on /resolve, and by the host CLI's OrchestratorClient (reads the host file) on every call. The agent container is never given the env var or the host file, so from a bottle every /bottles*, /resolve, /attribute, and /supervise/* call now returns 401 — closing the enumeration, allowlist-rewrite, credential-lift, and self-approval. The existing (source_ip, identity_token) checks stay as defense-in-depth. Enforced when configured: macOS + docker inject the secret (→ enforced). With no secret set the server runs open and warns loudly at startup — a fail-visible fallback for the unit suite and for Firecracker, whose port-scoped nft already blocks agents from 8099 (wiring the secret into its infra-VM init is a clean fast-follow, left out here to avoid churning the prebuilt-artifact hash). Verified end-to-end on real Apple Container: infra comes up healthy, the host CLI (with the secret) lists bottles while an unauthenticated GET /bottles gets 401, all five issue-#400 attacks from inside the agent get 401, and egress policy still works (200 allowed / 403 denied) — proving the gateway authenticates to /resolve with the secret. 1829 unit tests pass, pyright clean, pylint 9.91. Refs #400. Co-Authored-By: Claude Fable 5 --- bot_bottle/backend/macos_container/infra.py | 17 +++- bot_bottle/backend/macos_container/util.py | 14 +++- bot_bottle/docker_cmd.py | 13 +++- bot_bottle/orchestrator/client.py | 32 +++++++- bot_bottle/orchestrator/control_plane.py | 64 +++++++++++++-- bot_bottle/orchestrator/gateway.py | 15 +++- bot_bottle/orchestrator/lifecycle.py | 14 +++- bot_bottle/paths.py | 51 +++++++++++- bot_bottle/policy_resolver.py | 20 ++++- tests/unit/test_orchestrator_control_plane.py | 77 +++++++++++++++++++ tests/unit/test_orchestrator_gateway.py | 16 ++-- tests/unit/test_orchestrator_lifecycle.py | 10 +-- 12 files changed, 308 insertions(+), 35 deletions(-) diff --git a/bot_bottle/backend/macos_container/infra.py b/bot_bottle/backend/macos_container/infra.py index 14b7bf3..c0eea45 100644 --- a/bot_bottle/backend/macos_container/infra.py +++ b/bot_bottle/backend/macos_container/infra.py @@ -33,6 +33,7 @@ digest check. from __future__ import annotations +import os import time import urllib.error import urllib.request @@ -47,7 +48,11 @@ from ...orchestrator.lifecycle import ( OrchestratorStartError, source_hash, ) -from ...paths import HOST_DB_FILENAME +from ...paths import ( + CONTROL_PLANE_TOKEN_ENV, + HOST_DB_FILENAME, + host_control_plane_token, +) from . import util as container_mod from .gateway import ( DEFAULT_CA_TIMEOUT_SECONDS, @@ -220,11 +225,19 @@ class MacosInfraService: # Baked onto the container so `_source_current` can detect a real # control-plane code change and recreate. "--env", f"BOT_BOTTLE_SOURCE_HASH={current_hash}", + # The control-plane secret, for BOTH the control plane (to require + # it) and the gateway's PolicyResolver (to present it) — they share + # this one container. Bare `--env NAME` inherits the value from the + # run process below, so the secret never lands on argv or in + # `container inspect`'s command line. The agent runs in a SEPARATE + # container that is never given this var, which is the whole point. + "--env", CONTROL_PLANE_TOKEN_ENV, "--entrypoint", "sh", self.image, "-c", _init_script(self.port), ] - result = container_mod.run_container_argv(argv) + run_env = {**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()} + result = container_mod.run_container_argv(argv, env=run_env) if result.returncode != 0: raise OrchestratorStartError( f"infra container failed to start: " diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index 06a0a93..98c24b2 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -445,12 +445,20 @@ def container_ipv4_on_network(name: str, network: str) -> str: return ip -def run_container_argv(argv: list[str]) -> subprocess.CompletedProcess[str]: +def run_container_argv( + argv: list[str], *, env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: """Run a `container` command, returning the result for the caller to interpret. Unlike the `die`-on-failure helpers above, this lets callers that raise their own typed errors (the gateway / orchestrator lifecycle) - keep control of the failure path.""" - return subprocess.run(argv, capture_output=True, text=True, check=False) + keep control of the failure path. + + `env` sets the child process environment — used to hand a secret to a bare + `--env NAME` flag (Apple's "just key → inherit from host" form) so the + value is inherited from this process, never written onto argv or into + `container inspect`'s recorded command line.""" + return subprocess.run( + argv, capture_output=True, text=True, check=False, env=env) def bind_mount_spec(source: str, target: str, *, readonly: bool = False) -> str: diff --git a/bot_bottle/docker_cmd.py b/bot_bottle/docker_cmd.py index b8b4961..32a617e 100644 --- a/bot_bottle/docker_cmd.py +++ b/bot_bottle/docker_cmd.py @@ -14,13 +14,20 @@ from __future__ import annotations import subprocess -def run_docker(argv: list[str]) -> subprocess.CompletedProcess[str]: +def run_docker( + argv: list[str], *, env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: """Run a `docker` command, capturing stdout/stderr as text. Never raises on a non-zero exit — callers inspect `returncode` / `stderr` so they can stay fail-closed or tolerate idempotent no-ops (e.g. removing an - already-absent container).""" + already-absent container). + + `env` sets the child process environment — used to hand a secret to a bare + `--env NAME` flag (docker inherits its value from this process) so the + value never lands on argv or in `docker inspect`'s recorded command line.""" return subprocess.run( - argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, check=False, + argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + check=False, env=env, ) diff --git a/bot_bottle/orchestrator/client.py b/bot_bottle/orchestrator/client.py index e094b21..b235313 100644 --- a/bot_bottle/orchestrator/client.py +++ b/bot_bottle/orchestrator/client.py @@ -17,9 +17,22 @@ import urllib.error import urllib.request from dataclasses import dataclass +from ..paths import host_control_plane_token +from .control_plane import CONTROL_AUTH_HEADER + DEFAULT_TIMEOUT_SECONDS = 5.0 +def _host_auth_token() -> str: + """The per-host control-plane secret, or "" if it can't be read. "" means + 'send no auth header' — correct against an open (unconfigured) control + plane, and harmlessly rejected by a secured one.""" + try: + return host_control_plane_token() + except OSError: + return "" + + class OrchestratorClientError(RuntimeError): """A control-plane call failed (unreachable, or an unexpected status).""" @@ -34,11 +47,24 @@ class RegisteredBottle: class OrchestratorClient: - """Trusted host-side client for the orchestrator control plane.""" + """Trusted host-side client for the orchestrator control plane. - def __init__(self, base_url: str, *, timeout: float = DEFAULT_TIMEOUT_SECONDS) -> None: + Presents the per-host control-plane secret on every call (the header the + control plane requires on all routes but `/health`). The secret is read + from the host file — this client only ever runs host-side (CLI, launcher, + discovery), so it can read what an agent can't. `auth_token` is overridable + for tests; the default reads the host file, minting it on first use.""" + + def __init__( + self, + base_url: str, + *, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + auth_token: str | None = None, + ) -> None: self._base = base_url.rstrip("/") self._timeout = timeout + self._auth_token = auth_token if auth_token is not None else _host_auth_token() def _request( self, method: str, path: str, body: dict[str, object] | None = None, @@ -49,6 +75,8 @@ class OrchestratorClient: callers can treat 404 as a meaningful "no such bottle".""" data = json.dumps(body).encode() if body is not None else None headers = {"Content-Type": "application/json"} if data is not None else {} + if self._auth_token: + headers[CONTROL_AUTH_HEADER] = self._auth_token req = urllib.request.Request( f"{self._base}{path}", data=data, method=method, headers=headers, ) diff --git a/bot_bottle/orchestrator/control_plane.py b/bot_bottle/orchestrator/control_plane.py index 064528f..389c241 100644 --- a/bot_bottle/orchestrator/control_plane.py +++ b/bot_bottle/orchestrator/control_plane.py @@ -34,6 +34,7 @@ returned only once, to the caller that launches the bottle. from __future__ import annotations +import hmac import http.server import json import os @@ -42,11 +43,20 @@ import sys import typing from urllib.parse import urlsplit +from ..paths import CONTROL_PLANE_TOKEN_ENV from .service import Orchestrator # JSON body payload type (parsed request / rendered response). Json = dict[str, object] +# The request header carrying the per-host control-plane secret. Every route +# except `GET /health` requires it (see `dispatch`). The trusted callers hold +# the secret (the gateway's PolicyResolver, the host CLI's OrchestratorClient); +# an agent that can merely *reach* the port cannot present it, so it can't +# enumerate bottles, rewrite policy, read injected upstream tokens, or approve +# its own supervise proposals. +CONTROL_AUTH_HEADER = "x-bot-bottle-control-auth" + def _parse_json_object(body: bytes) -> Json: """Parse a JSON object body. Raises ValueError for non-objects / bad JSON.""" @@ -59,15 +69,29 @@ def _parse_json_object(body: bytes) -> Json: def dispatch( # pylint: disable=too-many-return-statements,too-many-branches - orch: Orchestrator, method: str, path: str, body: bytes + orch: Orchestrator, method: str, path: str, body: bytes, *, authorized: bool = True, ) -> tuple[int, Json]: """Route one control-plane request to a (status, payload) pair. Pure — - no I/O beyond the orchestrator — so it is fully testable without a socket.""" + no I/O beyond the orchestrator — so it is fully testable without a socket. + + `authorized` is whether the request presented the control-plane secret (or + no secret is configured — see `ControlPlaneServer`). Every route except + `GET /health` requires it: the source-IP + identity-token checks inside + `/resolve` and `/attribute` authenticate the *bottle* a request is about, + not the *caller*, so without this gate any agent that can reach the port + could rewrite another bottle's policy, read the injected upstream tokens, + or approve its own supervise proposals. Defaults True so unit tests of the + routing logic don't have to thread it through.""" route = urlsplit(path).path.rstrip("/") or "/" if method == "GET" and route == "/health": return 200, {"status": "ok"} + if not authorized: + # Everything below is a trusted-caller operation. Deny before touching + # the registry / broker / supervise store. + return 401, {"error": "control-plane authentication required"} + if method == "GET" and route == "/gateway": return 200, orch.gateway_status() @@ -209,8 +233,10 @@ class Handler(http.server.BaseHTTPRequestHandler): assert isinstance(server, ControlPlaneServer) length = int(self.headers.get("Content-Length") or 0) body = self.rfile.read(length) if length > 0 else b"" + authorized = server.is_authorized(self.headers.get(CONTROL_AUTH_HEADER, "")) try: - status, payload = dispatch(server.orchestrator, method, self.path, body) + status, payload = dispatch( + server.orchestrator, method, self.path, body, authorized=authorized) except Exception as e: # noqa: BLE001 — the control plane must stay up sys.stderr.write(f"orchestrator: {method} {self.path} failed: {e!r}\n") sys.stderr.flush() @@ -236,15 +262,40 @@ class Handler(http.server.BaseHTTPRequestHandler): class ControlPlaneServer(socketserver.ThreadingMixIn, http.server.HTTPServer): - """Threading HTTP server that carries the orchestrator for its handlers.""" + """Threading HTTP server that carries the orchestrator for its handlers. + + Holds the per-host control-plane secret (from `$BOT_BOTTLE_CONTROL_PLANE_TOKEN`, + injected by the launcher into this container only). When a secret is set, + every route but `/health` requires it; when it is unset the server runs + **open** and says so loudly at startup — a fail-visible fallback for tests + and any backend that hasn't wired the secret yet (e.g. Firecracker, whose + nft boundary already blocks agents from the control-plane port).""" daemon_threads = True allow_reuse_address = True def __init__(self, address: tuple[str, int], orchestrator: Orchestrator) -> None: self.orchestrator = orchestrator + self._auth_token = os.environ.get(CONTROL_PLANE_TOKEN_ENV, "").strip() + if not self._auth_token: + sys.stderr.write( + "orchestrator: WARNING — no control-plane secret " + f"(${CONTROL_PLANE_TOKEN_ENV}); running WITHOUT caller " + "authentication. Any client that can reach this port can drive " + "it. Backends that put the control plane on an agent-reachable " + "network MUST set this.\n" + ) + sys.stderr.flush() super().__init__(address, Handler) + def is_authorized(self, presented: str) -> bool: + """True iff the request may proceed past `/health`: either no secret is + configured (open mode) or the presented header matches it. Constant-time + compare so a wrong token leaks nothing timing-wise.""" + if not self._auth_token: + return True + return hmac.compare_digest(presented, self._auth_token) + def make_server( orchestrator: Orchestrator, host: str = "127.0.0.1", port: int = 0 @@ -254,4 +305,7 @@ def make_server( return ControlPlaneServer((host, port), orchestrator) -__all__ = ["dispatch", "Handler", "ControlPlaneServer", "make_server", "Json"] +__all__ = [ + "dispatch", "Handler", "ControlPlaneServer", "make_server", "Json", + "CONTROL_AUTH_HEADER", +] diff --git a/bot_bottle/orchestrator/gateway.py b/bot_bottle/orchestrator/gateway.py index a01bef6..b561b84 100644 --- a/bot_bottle/orchestrator/gateway.py +++ b/bot_bottle/orchestrator/gateway.py @@ -23,7 +23,11 @@ import time from pathlib import Path from ..docker_cmd import run_docker -from ..paths import host_db_path +from ..paths import ( + CONTROL_PLANE_TOKEN_ENV, + host_control_plane_token, + host_db_path, +) from ..supervise import DB_PATH_IN_CONTAINER # The host DB dir is bind-mounted here so the gateway's supervise daemon @@ -215,12 +219,19 @@ class DockerGateway(Gateway): ] for port in self._host_port_bindings: argv += ["--publish", f"0.0.0.0:{port}:{port}"] + run_env = dict(os.environ) if self._orchestrator_url: # Makes the gateway's egress / git / supervise daemons multi-tenant: # each request resolves source-IP -> policy against the control plane. argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"] + # ...and presents the control-plane secret on those /resolve calls + # (the control plane requires it). Bare `--env NAME` keeps the value + # off argv / `docker inspect`; only the gateway (not the agent) is + # given it. Only needed in multi-tenant mode, where /resolve is used. + argv += ["--env", CONTROL_PLANE_TOKEN_ENV] + run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token() argv.append(self.image_ref) - proc = run_docker(argv) + proc = run_docker(argv, env=run_env) if proc.returncode != 0: raise GatewayError(f"gateway failed to start: {proc.stderr.strip()}") diff --git a/bot_bottle/orchestrator/lifecycle.py b/bot_bottle/orchestrator/lifecycle.py index b6cb62f..04f7b58 100644 --- a/bot_bottle/orchestrator/lifecycle.py +++ b/bot_bottle/orchestrator/lifecycle.py @@ -25,7 +25,7 @@ from pathlib import Path from .. import log from ..docker_cmd import run_docker -from ..paths import bot_bottle_root +from ..paths import CONTROL_PLANE_TOKEN_ENV, bot_bottle_root, host_control_plane_token from .gateway import GATEWAY_IMAGE, GATEWAY_NETWORK, DockerGateway, GatewayError DEFAULT_PORT = 8099 @@ -147,7 +147,11 @@ class OrchestratorService: "--label", f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current_hash}", "--network", self.network, # Host CLI reaches the control plane here; bound to loopback so it - # is not exposed on the host's external interfaces. + # is not exposed on the host's external interfaces. NOTE: the + # container is still on `self.network` (the shared gateway network), + # so agents can reach it by container IP — which is exactly why the + # control plane requires the secret below rather than trusting the + # network boundary. "--publish", f"127.0.0.1:{self.port}:{self.port}", "--volume", f"{self._repo_root}:{_APP_DIR}:ro", "--workdir", _APP_DIR, @@ -155,11 +159,15 @@ class OrchestratorService: # orchestrator opens bot-bottle.db). "--volume", f"{self._host_root}:{_ROOT_IN_CONTAINER}", "--env", f"BOT_BOTTLE_ROOT={_ROOT_IN_CONTAINER}", + # The control-plane secret it requires on every route but /health. + # Bare `--env NAME` → docker inherits the value from the run env + # below, so the secret never lands on argv / `docker inspect`. + "--env", CONTROL_PLANE_TOKEN_ENV, "--entrypoint", "python3", self.image, "-m", "bot_bottle.orchestrator", "--host", "0.0.0.0", "--port", str(self.port), "--broker", "stub", - ]) + ], env={**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()}) if proc.returncode != 0: raise OrchestratorStartError( f"orchestrator container failed to start: {proc.stderr.strip()}" diff --git a/bot_bottle/paths.py b/bot_bottle/paths.py index 13136fd..d7e1e42 100644 --- a/bot_bottle/paths.py +++ b/bot_bottle/paths.py @@ -16,6 +16,8 @@ layer (and to COPY flat into the gateway). from __future__ import annotations import os +import secrets +import stat from pathlib import Path # The single shared host state DB. All bot-bottle SQLite stores (supervise @@ -23,6 +25,14 @@ from pathlib import Path # TableMigrations schema_key namespaces each store's tables. HOST_DB_FILENAME = "bot-bottle.db" +# The per-host control-plane secret file, and the env var the launchers inject +# its value into. The control plane requires this secret on every mutating / +# reading route (see orchestrator/control_plane.py); it is held only by the +# trusted callers (control plane, gateway, host CLI) and never handed to an +# agent, so an agent that can reach the control-plane port still can't drive it. +CONTROL_PLANE_TOKEN_FILENAME = "control-plane-token" +CONTROL_PLANE_TOKEN_ENV = "BOT_BOTTLE_CONTROL_PLANE_TOKEN" + def bot_bottle_root() -> Path: """The app data root — `$BOT_BOTTLE_ROOT` if set, else `~/.bot-bottle`.""" @@ -49,4 +59,43 @@ def host_db_dir() -> Path: return db_dir -__all__ = ["HOST_DB_FILENAME", "bot_bottle_root", "host_db_path", "host_db_dir"] +def host_control_plane_token() -> str: + """The per-host control-plane secret, minted (256-bit, url-safe) and + persisted 0600 on first use, then reused. + + This is the shared secret the launchers inject into the control-plane and + gateway containers and that the host CLI presents on every call. It is a + *host* artifact — the file lives under the root the agent never mounts, and + the env var is set only on the trusted containers — so reading it here is + safe on the host launch path but the value never reaches a bottle.""" + path = bot_bottle_root() / CONTROL_PLANE_TOKEN_FILENAME + try: + existing = path.read_text().strip() + if existing: + return existing + except OSError: + pass + path.parent.mkdir(parents=True, exist_ok=True) + token = secrets.token_urlsafe(32) + # Create 0600 up front (O_EXCL loses a concurrent race harmlessly — we + # re-read the winner's token below) so the secret is never briefly world- + # readable between write and chmod. + try: + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + except FileExistsError: + return path.read_text().strip() + with os.fdopen(fd, "w") as f: + f.write(token) + os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) + return token + + +__all__ = [ + "HOST_DB_FILENAME", + "CONTROL_PLANE_TOKEN_FILENAME", + "CONTROL_PLANE_TOKEN_ENV", + "bot_bottle_root", + "host_db_path", + "host_db_dir", + "host_control_plane_token", +] diff --git a/bot_bottle/policy_resolver.py b/bot_bottle/policy_resolver.py index d9c62de..e16ac12 100644 --- a/bot_bottle/policy_resolver.py +++ b/bot_bottle/policy_resolver.py @@ -29,11 +29,29 @@ the gateway. from __future__ import annotations import json +import os import urllib.error import urllib.request DEFAULT_TIMEOUT_SECONDS = 2.0 +# The control-plane secret this gateway presents on every /resolve call, read +# from the env the launcher injects into the gateway container. The control +# plane requires it (orchestrator/control_plane.py). Constant + env-var name are +# duplicated here rather than imported because this module is COPYed flat into +# the gateway image, free of bot-bottle imports — same rationale as +# IDENTITY_HEADER in egress_addon / git_http_backend. +CONTROL_AUTH_HEADER = "x-bot-bottle-control-auth" +CONTROL_PLANE_TOKEN_ENV = "BOT_BOTTLE_CONTROL_PLANE_TOKEN" + + +def _control_auth_headers() -> dict[str, str]: + """The auth header to send, or {} when no secret is configured (an open + control plane, e.g. Firecracker behind its nft boundary — sending nothing + is correct there and harmlessly ignored).""" + token = os.environ.get(CONTROL_PLANE_TOKEN_ENV, "").strip() + return {CONTROL_AUTH_HEADER: token} if token else {} + class PolicyResolveError(RuntimeError): """The orchestrator was unreachable or returned an unexpected status — @@ -57,7 +75,7 @@ class PolicyResolver: ).encode() req = urllib.request.Request( f"{self._base}/resolve", data=body, method="POST", - headers={"Content-Type": "application/json"}, + headers={"Content-Type": "application/json", **_control_auth_headers()}, ) try: with urllib.request.urlopen(req, timeout=self._timeout) as resp: diff --git a/tests/unit/test_orchestrator_control_plane.py b/tests/unit/test_orchestrator_control_plane.py index 9fd4fb0..b062399 100644 --- a/tests/unit/test_orchestrator_control_plane.py +++ b/tests/unit/test_orchestrator_control_plane.py @@ -11,6 +11,7 @@ import secrets import tempfile import threading import unittest +import urllib.error import urllib.request from pathlib import Path from unittest.mock import patch @@ -243,6 +244,82 @@ class TestServerRoundTrip(unittest.TestCase): self.assertEqual(reg["bottle_id"], attr["bottle_id"]) +class TestControlPlaneAuth(unittest.TestCase): + """The per-host control-plane secret (issue #400): every route but /health + is a trusted-caller op an agent must not be able to drive just because it + can reach the port.""" + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.orch = _orchestrator(Path(self._tmp.name) / "r.db") + + def test_health_is_public_even_unauthorized(self) -> None: + status, _ = dispatch(self.orch, "GET", "/health", b"", authorized=False) + self.assertEqual(200, status) + + def test_unauthorized_denies_every_other_route(self) -> None: + for method, path, body in [ + ("GET", "/bottles", b""), + ("POST", "/bottles", _body({"source_ip": "10.0.0.1"})), + ("PUT", "/bottles/x/policy", _body({"policy": "routes: []"})), + ("DELETE", "/bottles/x", b""), + ("POST", "/resolve", _body({"source_ip": "10.0.0.1", "identity_token": "t"})), + ("POST", "/attribute", _body({"source_ip": "10.0.0.1", "identity_token": "t"})), + ("GET", "/supervise/proposals", b""), + ("POST", "/supervise/respond", _body({"proposal_id": "p", "bottle_slug": "s", "decision": "approve"})), + ]: + status, _ = dispatch(self.orch, method, path, body, authorized=False) + self.assertEqual(401, status, f"{method} {path} should be 401 unauthorized") + + def test_deny_happens_before_the_registry_is_touched(self) -> None: + """An unauthorized DELETE must not tear a bottle down. 401, and the + bottle is still there.""" + rec = self.orch.registry.register("10.0.0.9", policy="", metadata="") + status, _ = dispatch( + self.orch, "DELETE", f"/bottles/{rec.bottle_id}", b"", authorized=False) + self.assertEqual(401, status) + self.assertIsNotNone(self.orch.registry.get(rec.bottle_id)) + + def _server_with_secret(self, secret: str): + with patch.dict("os.environ", {"BOT_BOTTLE_CONTROL_PLANE_TOKEN": secret}): + server = make_server(self.orch, "127.0.0.1", 0) + self.addCleanup(server.server_close) + threading.Thread(target=server.serve_forever, daemon=True).start() + self.addCleanup(server.shutdown) + host, port = server.server_address[0], server.server_address[1] + return f"http://{host}:{port}" + + def _status(self, url: str, *, header: str | None = None) -> int: + req = urllib.request.Request(url) + if header is not None: + req.add_header("x-bot-bottle-control-auth", header) + try: + return urllib.request.urlopen(req, timeout=5).status + except urllib.error.HTTPError as e: + return e.code + + def test_configured_server_enforces_the_header_over_http(self) -> None: + base = self._server_with_secret("s3cret-admin") + # /health is public — no header needed. + self.assertEqual(200, self._status(f"{base}/health")) + # /bottles requires the secret. + self.assertEqual(401, self._status(f"{base}/bottles")) + self.assertEqual(401, self._status(f"{base}/bottles", header="wrong")) + self.assertEqual(200, self._status(f"{base}/bottles", header="s3cret-admin")) + + def test_unconfigured_server_runs_open(self) -> None: + """No secret set (tests / nft-protected Firecracker): open mode, so the + existing round-trip and unit behavior are unchanged.""" + with patch.dict("os.environ", {}, clear=False): + import os + os.environ.pop("BOT_BOTTLE_CONTROL_PLANE_TOKEN", None) + server = make_server(self.orch, "127.0.0.1", 0) + self.addCleanup(server.server_close) + self.assertTrue(server.is_authorized("")) + self.assertTrue(server.is_authorized("anything")) + + class TestDispatchSupervise(unittest.TestCase): """The /supervise/* routes over the pure dispatch().""" diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index 76b34e6..27b851a 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -38,7 +38,7 @@ class TestDockerGateway(unittest.TestCase): def test_ensure_running_noop_when_up_and_image_current(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout=self.sc.name) # running @@ -58,7 +58,7 @@ class TestDockerGateway(unittest.TestCase): # a rebuild's new flat daemons take effect. calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout=self.sc.name) @@ -76,7 +76,7 @@ class TestDockerGateway(unittest.TestCase): def test_ensure_running_starts_the_singleton_when_absent(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) return _proc(stdout="") if argv[:2] == ["docker", "ps"] else _proc() @@ -102,7 +102,7 @@ class TestDockerGateway(unittest.TestCase): def test_ensure_running_creates_network_when_missing(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:3] == ["docker", "network", "inspect"]: return _proc(returncode=1, stderr="No such network") @@ -135,7 +135,7 @@ class TestDockerGateway(unittest.TestCase): def test_ensure_running_reuses_existing_network(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) return _proc(stdout="") if argv[:2] == ["docker", "ps"] else _proc() @@ -144,7 +144,7 @@ class TestDockerGateway(unittest.TestCase): self.assertEqual([], [c for c in calls if c[:3] == ["docker", "network", "create"]]) def test_ensure_running_raises_on_docker_failure(self) -> None: - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: if argv[:2] == ["docker", "ps"]: return _proc(stdout="") if argv[:2] == ["docker", "run"]: @@ -181,7 +181,7 @@ class TestDockerGatewayBuild(unittest.TestCase): # build-if-missing silently ran a stale single-tenant image. calls: list[list[str]] = [] - def rec(argv: list[str]) -> Mock: + def rec(argv: list[str], **_kw: object) -> Mock: calls.append(argv) return _proc() # image present, build succeeds @@ -196,7 +196,7 @@ class TestDockerGatewayBuild(unittest.TestCase): def test_ensure_built_no_cache_env_forces_full_rebuild(self) -> None: calls: list[list[str]] = [] - def rec(argv: list[str]) -> Mock: + def rec(argv: list[str], **_kw: object) -> Mock: calls.append(argv) return _proc() diff --git a/tests/unit/test_orchestrator_lifecycle.py b/tests/unit/test_orchestrator_lifecycle.py index 81a2e23..8bf3ade 100644 --- a/tests/unit/test_orchestrator_lifecycle.py +++ b/tests/unit/test_orchestrator_lifecycle.py @@ -60,7 +60,7 @@ class TestOrchestratorService(unittest.TestCase): current = source_hash(self.svc._repo_root) calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout=ORCHESTRATOR_NAME) @@ -83,7 +83,7 @@ class TestOrchestratorService(unittest.TestCase): # effect, same as the gateway's image-staleness check. calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout=ORCHESTRATOR_NAME) @@ -104,7 +104,7 @@ class TestOrchestratorService(unittest.TestCase): def test_ensure_running_starts_orchestrator_container_when_absent(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout="") # not running @@ -127,7 +127,7 @@ class TestOrchestratorService(unittest.TestCase): # gateway data plane — built from Dockerfile.orchestrator when absent. calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout="") # orchestrator not running @@ -148,7 +148,7 @@ class TestOrchestratorService(unittest.TestCase): def test_ensure_running_skips_orchestrator_image_build_when_present(self) -> None: calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout="")