From ce8e2b38745ccf9b5aa6de828a238f041f06f376 Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 9 Jul 2026 16:10:42 +0000 Subject: [PATCH] feat: run smolmachines sidecar as vm --- .../backend/smolmachines/egress_apply.py | 49 +++- bot_bottle/backend/smolmachines/enumerate.py | 7 +- bot_bottle/backend/smolmachines/launch.py | 121 +++++---- .../backend/smolmachines/port_forward.py | 245 ++++++++++++++++++ .../backend/smolmachines/sidecar_bundle.py | 77 ++++++ bot_bottle/backend/smolmachines/smolvm.py | 16 +- docs/prds/prd-new-smolmachines-sidecar-vm.md | 61 +++-- tests/unit/test_smolmachines_egress_apply.py | 58 +++++ tests/unit/test_smolmachines_port_forward.py | 98 +++++++ tests/unit/test_smolmachines_provision.py | 39 ++- .../unit/test_smolmachines_sidecar_bundle.py | 65 +++++ tests/unit/test_smolmachines_smolvm.py | 16 ++ 12 files changed, 739 insertions(+), 113 deletions(-) create mode 100644 bot_bottle/backend/smolmachines/port_forward.py create mode 100644 tests/unit/test_smolmachines_egress_apply.py create mode 100644 tests/unit/test_smolmachines_port_forward.py diff --git a/bot_bottle/backend/smolmachines/egress_apply.py b/bot_bottle/backend/smolmachines/egress_apply.py index 559aa3b..661a13f 100644 --- a/bot_bottle/backend/smolmachines/egress_apply.py +++ b/bot_bottle/backend/smolmachines/egress_apply.py @@ -1,20 +1,51 @@ """Egress apply for the smolmachines backend. -The smolmachines sidecar bundle runs as a host-side Docker container, -so egress signalling is identical to the docker backend. +The smolmachines sidecar bundle runs as a sidecar smolVM. Route-file +inspection and reload signalling therefore go through ``smolvm machine +exec`` instead of Docker. """ from __future__ import annotations -from ..docker.egress_apply import ( # noqa: F401 - DockerEgressApplicator, - EgressApplyError, - applicator, - fetch_current_routes, -) +from ...egress import EGRESS_ROUTES_IN_CONTAINER +from ...log import warn +from ..egress_apply import EgressApplicator, EgressApplyError +from . import sidecar_bundle as _bundle +from . import smolvm as _smolvm + + +def fetch_current_routes(slug: str) -> str: + machine = _bundle.bundle_machine_name(slug) + result = _smolvm.machine_exec(machine, ["cat", EGRESS_ROUTES_IN_CONTAINER]) + if result.returncode != 0: + raise EgressApplyError( + f"could not read routes.yaml from {machine}: " + f"{(result.stderr or '').strip() or 'sidecar VM not running?'}" + ) + return result.stdout + + +class SmolmachinesEgressApplicator(EgressApplicator): + def _signal_bundle_reload(self, slug: str) -> None: + machine = _bundle.bundle_machine_name(slug) + result = _smolvm.machine_exec(machine, ["sh", "-c", "kill -HUP 1"]) + if result.returncode != 0: + last_error = (result.stderr or "").strip() or (result.stdout or "").strip() + warn( + f"egress: routes updated on disk for {slug}, but bundle reload failed: " + f"{last_error or 'smolvm exec failed'}" + ) + raise EgressApplyError( + f"could not reload egress bundle {machine}: " + f"{last_error or 'smolvm exec failed'}" + ) + + +applicator = SmolmachinesEgressApplicator() + __all__ = [ - "DockerEgressApplicator", + "SmolmachinesEgressApplicator", "EgressApplyError", "applicator", "fetch_current_routes", diff --git a/bot_bottle/backend/smolmachines/enumerate.py b/bot_bottle/backend/smolmachines/enumerate.py index cd24ec4..7ce0f47 100644 --- a/bot_bottle/backend/smolmachines/enumerate.py +++ b/bot_bottle/backend/smolmachines/enumerate.py @@ -31,6 +31,7 @@ from . import sidecar_bundle as _bundle # matching the bundle container name pattern. We use the prefix # both as a filter and to strip back to the slug. _VM_NAME_PREFIX = "bot-bottle-" +_SIDECAR_VM_PREFIX = _bundle.bundle_machine_name("") def enumerate_active() -> list[ActiveAgent]: @@ -54,7 +55,11 @@ def enumerate_active() -> list[ActiveAgent]: for m in machines: name = m.get("name") or "" state = m.get("state") or "" - if state != "running" or not name.startswith(_VM_NAME_PREFIX): + if ( + state != "running" + or not name.startswith(_VM_NAME_PREFIX) + or name.startswith(_SIDECAR_VM_PREFIX) + ): continue slug = name[len(_VM_NAME_PREFIX):] metadata = read_metadata(slug) diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index 56e5929..d1ce279 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -17,7 +17,6 @@ from __future__ import annotations import dataclasses import os -import platform from contextlib import ExitStack, contextmanager from pathlib import Path from typing import Callable, Generator @@ -53,6 +52,7 @@ from ...bottle_state import ( read_committed_image, ) from . import loopback_alias as _loopback +from . import port_forward as _forward from . import sidecar_bundle as _bundle from . import smolvm as _smolvm from .bottle import SmolmachinesBottle @@ -94,7 +94,6 @@ def launch( plan = _mint_certs(plan) proxy_host = _proxy_host(plan, loopback_ip) plan = _start_bundle(plan, network, proxy_host, stack) - plan = _discover_urls(plan, proxy_host) agent_from_path = _agent_from_path(plan) @@ -144,19 +143,17 @@ def _allocate_resources( plan: SmolmachinesBottlePlan, stack: ExitStack, ) -> tuple[str, str]: - """Reserve a loopback alias and create the per-bottle docker bridge. + """Reserve a per-bottle host address. - The per-bottle alias scopes TSI's allowlist to this bottle's - published ports so the agent can't reach other bottles' or host - services' ports on loopback. On macOS `ensure_pool` first - sudo-aliases the pool on `lo0`; on Linux that's a no-op since - all of 127.0.0.0/8 is already loopback, but the per-bottle - allocation runs on both.""" + The per-bottle address scopes TSI's allowlist to this bottle's + forwarder-published ports so the agent can't reach other bottles' + or host services. The returned network name remains in the bundle + spec for compatibility with older helper tests; no Docker sidecar + container is launched.""" + del stack _loopback.ensure_pool() loopback_ip = _loopback.allocate(plan.slug) network = _bundle.bundle_network_name(plan.slug) - _bundle.create_bundle_network(network, plan.bundle_subnet, plan.bundle_gateway) - stack.callback(_bundle.remove_bundle_network, network) return loopback_ip, network @@ -179,15 +176,40 @@ def _start_bundle( proxy_host: str, stack: ExitStack, ) -> SmolmachinesBottlePlan: - """Build the BundleLaunchSpec, resolve token env, start the - sidecar bundle container, and register teardown.""" + """Build the BundleLaunchSpec, start the sidecar VM, wrap its raw + smolVM-published loopback ports with per-bottle forwarders, stamp + agent URLs from those forwarder ports, and register teardown.""" plan = _provision_git_gate_keys(plan) bundle_spec = _bundle_launch_spec(plan, network, proxy_host) token_env = _resolve_token_env(plan, dict(os.environ)) - _bundle.ensure_bundle_image(bundle_spec.image) - _bundle.start_bundle(bundle_spec, env={**os.environ, **token_env}) - stack.callback(_bundle.stop_bundle, plan.slug) - return plan + artifact = _ensure_smolmachine( + bundle_spec.image, + dockerfile=_bundle.SIDECAR_BUNDLE_DOCKERFILE, + ) + launch = _bundle.start_bundle_vm( + bundle_spec, + from_path=artifact, + host_env={**os.environ, **token_env}, + ) + stack.callback(_bundle.stop_bundle_vm, plan.slug) + + forward_specs = tuple( + _forward.ForwardSpec( + label=_label_for_port(container_port), + listen_host=proxy_host, + listen_port=0, + target_host="127.0.0.1", + target_port=host_port, + ) + for container_port, host_port in launch.raw_ports.items() + ) + handle = _forward.start_forwarder(forward_specs) + stack.callback(_forward.stop_forwarder, handle) + published_ports = { + _port_for_label(spec.label): spec.listen_port + for spec in handle.forwards + } + return _discover_urls(plan, proxy_host, published_ports) def _provision_git_gate_keys( @@ -206,35 +228,27 @@ def _provision_git_gate_keys( def _discover_urls( plan: SmolmachinesBottlePlan, proxy_host: str, + published_ports: dict[int, int], ) -> SmolmachinesBottlePlan: - """Discover host-side ports for published container ports and - return the plan with URLs + guest_env stamped in. + """Stamp URLs + guest_env from per-bottle forwarder ports. - `proxy_host` is the host IP that both TSI's allowlist and - docker's port-forward bindings are keyed to. On macOS it is the - per-bottle loopback alias; on Linux it is the per-bottle bridge - gateway (see `_proxy_host`). The agent dials the published port - on this IP for all bundle-hosted services. + `proxy_host` is the host IP that both TSI's allowlist and the + forwarder listeners are keyed to. The raw smolVM-published ports + are intentionally not advertised to the agent. NO_PROXY includes `proxy_host` so supervise + git-gate URLs bypass HTTPS_PROXY.""" - agent_facing_host_port = _bundle.bundle_host_port( - plan.slug, _EGRESS_PORT, host_ip=proxy_host, - ) + agent_facing_host_port = published_ports[_EGRESS_PORT] agent_proxy_url = f"http://{proxy_host}:{agent_facing_host_port}" agent_git_gate_host = "" if plan.git_gate_plan.upstreams: - git_gate_host_port = _bundle.bundle_host_port( - plan.slug, _GIT_HTTP_PORT, host_ip=proxy_host, - ) + git_gate_host_port = published_ports[_GIT_HTTP_PORT] agent_git_gate_host = f"{proxy_host}:{git_gate_host_port}" agent_supervise_url = "" if plan.supervise_plan is not None: - supervise_host_port = _bundle.bundle_host_port( - plan.slug, _SUPERVISE_PORT, host_ip=proxy_host, - ) + supervise_host_port = published_ports[_SUPERVISE_PORT] agent_supervise_url = f"http://{proxy_host}:{supervise_host_port}/" existing_no_proxy = plan.guest_env.get("NO_PROXY", "localhost,127.0.0.1") @@ -328,26 +342,33 @@ def _init_vm(plan: SmolmachinesBottlePlan) -> None: def _proxy_host(plan: SmolmachinesBottlePlan, loopback_ip: str) -> str: - """Return the host IP for TSI's allowlist and docker port-forward bindings. - - On macOS, the per-bottle loopback alias (e.g. ``127.0.0.16``) works - because macOS's network stack lets TSI intercept 127.x.x.x connects - from the guest before they reach the host's own loopback. - - On Linux, the guest kernel's LOCAL routing table routes all - ``127.0.0.0/8`` to the guest's own loopback — those packets never - reach eth0 and TSI never sees them. Using the per-bottle bridge - gateway (e.g. ``192.168.N.1``) instead sidesteps the problem: it - is not a loopback address, so the guest routes it via eth0 and TSI - intercepts it normally. The TSI allowlist is ``gateway/32``, which - is distinct from the container IP (``192.168.N.2``), so the agent - still can't reach the egress daemon directly — TSI blocks any - connection to the container IP that isn't via the published port.""" - if platform.system() == "Linux": - return plan.bundle_gateway + """Return the per-bottle host address used for TSI and forwarders.""" + del plan return loopback_ip +def _label_for_port(port: int) -> str: + if port == _EGRESS_PORT: + return "egress" + if port == _GIT_HTTP_PORT: + return "git-http" + if port == _SUPERVISE_PORT: + return "supervise" + return f"port-{port}" + + +def _port_for_label(label: str) -> int: + if label == "egress": + return _EGRESS_PORT + if label == "git-http": + return _GIT_HTTP_PORT + if label == "supervise": + return _SUPERVISE_PORT + if label.startswith("port-"): + return int(label.removeprefix("port-")) + raise ValueError(f"unknown sidecar forward label: {label}") + + def _bundle_launch_spec( plan: SmolmachinesBottlePlan, network: str, proxy_host: str, ) -> _bundle.BundleLaunchSpec: diff --git a/bot_bottle/backend/smolmachines/port_forward.py b/bot_bottle/backend/smolmachines/port_forward.py new file mode 100644 index 0000000..7834834 --- /dev/null +++ b/bot_bottle/backend/smolmachines/port_forward.py @@ -0,0 +1,245 @@ +"""Per-bottle TCP forwarder for smolmachines sidecar VMs. + +smolVM currently publishes guest ports on host loopback as +``HOST:GUEST`` port pairs, without an address-scoped bind. The agent +VM's TSI allowlist is IP-only, so bot-bottle exposes a second, stricter +surface: listeners bound only to the bottle's allocated host address, +forwarding to the raw smolVM-published loopback ports. +""" + +from __future__ import annotations + +import argparse +import json +import os +import selectors +import signal +import socket +import subprocess +import sys +import threading +from dataclasses import asdict, dataclass +from typing import Any, Iterable, Sequence + +from ...log import die, warn + + +_BUFFER_SIZE = 64 * 1024 +_LOOPBACK_TARGETS = {"127.0.0.1", "::1"} +_FORBIDDEN_LISTEN_HOSTS = {"", "0.0.0.0", "::", "127.0.0.1", "::1"} + + +@dataclass(frozen=True) +class ForwardSpec: + label: str + listen_host: str + listen_port: int + target_host: str + target_port: int + + +@dataclass(frozen=True) +class ForwarderHandle: + process: Any + forwards: tuple[ForwardSpec, ...] + + +def start_forwarder(specs: Sequence[ForwardSpec]) -> ForwarderHandle: + """Start one helper process for a bottle and wait until all + listeners are bound. ``listen_port`` may be 0; the returned specs + contain the kernel-assigned ports printed by the helper.""" + if not specs: + return ForwarderHandle(process=_CompletedProcess(), forwards=()) + _validate_specs(specs) + argv = [ + sys.executable, + "-c", + "from bot_bottle.backend.smolmachines.port_forward import main; " + "raise SystemExit(main())", + "--spec-json", + json.dumps([asdict(s) for s in specs], separators=(",", ":")), + ] + proc = subprocess.Popen( + argv, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=None, + text=True, + env=os.environ, + ) + assert proc.stdout is not None + line = proc.stdout.readline() + proc.stdout.close() + if not line: + proc.terminate() + proc.wait(timeout=5) + die("smolmachines forwarder failed before reporting readiness") + try: + payload = json.loads(line) + bound = tuple(ForwardSpec(**item) for item in payload["forwards"]) + except (KeyError, TypeError, ValueError, json.JSONDecodeError) as exc: + proc.terminate() + proc.wait(timeout=5) + die(f"smolmachines forwarder returned invalid readiness payload: {exc}") + return ForwarderHandle(process=proc, forwards=bound) + + +def stop_forwarder(handle: ForwarderHandle) -> None: + proc = handle.process + if proc.poll() is not None: + return + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait(timeout=5) + + +def _validate_specs(specs: Iterable[ForwardSpec]) -> None: + for spec in specs: + if spec.listen_host in _FORBIDDEN_LISTEN_HOSTS: + die(f"refusing unsafe smolmachines forwarder bind: {spec.listen_host!r}") + if spec.target_host not in _LOOPBACK_TARGETS: + die(f"refusing non-loopback smolmachines forwarder target: {spec.target_host!r}") + if not 0 <= spec.listen_port <= 65535: + die(f"invalid smolmachines forwarder listen port: {spec.listen_port}") + if not 1 <= spec.target_port <= 65535: + die(f"invalid smolmachines forwarder target port: {spec.target_port}") + + +class _CompletedProcess: + """Popen-like no-op used when a bottle has no forwards.""" + + def poll(self) -> int: + return 0 + + def terminate(self) -> None: + return None + + def kill(self) -> None: + return None + + def wait(self, timeout: float | None = None) -> int: + del timeout + return 0 + + +class _Forwarder: + def __init__(self, specs: Sequence[ForwardSpec]): + _validate_specs(specs) + self.specs = tuple(specs) + self.selector = selectors.DefaultSelector() + self.stop_event = threading.Event() + self.listeners: list[tuple[socket.socket, ForwardSpec]] = [] + self.bound: list[ForwardSpec] = [] + + def start(self) -> None: + for spec in self.specs: + listener = socket.create_server( + (spec.listen_host, spec.listen_port), + reuse_port=False, + backlog=64, + ) + listener.setblocking(False) + host, port = listener.getsockname()[:2] + bound = ForwardSpec( + label=spec.label, + listen_host=str(host), + listen_port=int(port), + target_host=spec.target_host, + target_port=spec.target_port, + ) + self.listeners.append((listener, bound)) + self.bound.append(bound) + self.selector.register(listener, selectors.EVENT_READ, bound) + + def serve(self) -> None: + while not self.stop_event.is_set(): + try: + events = self.selector.select(timeout=0.25) + except OSError: + return + for key, _ in events: + listener = key.fileobj + if not isinstance(listener, socket.socket): + continue + spec = key.data + try: + client, _ = listener.accept() + except OSError: + continue + threading.Thread( + target=self._handle_client, + args=(client, spec), + daemon=True, + ).start() + + def stop(self) -> None: + self.stop_event.set() + for listener, _ in self.listeners: + try: + self.selector.unregister(listener) + except Exception: # noqa: BLE001 - best effort shutdown + pass + listener.close() + self.selector.close() + + def _handle_client(self, client: socket.socket, spec: ForwardSpec) -> None: + with client: + try: + target = socket.create_connection((spec.target_host, spec.target_port)) + except OSError as exc: + warn(f"smolmachines forwarder {spec.label}: target connect failed: {exc}") + return + with target: + left = threading.Thread(target=_pipe, args=(client, target), daemon=True) + right = threading.Thread(target=_pipe, args=(target, client), daemon=True) + left.start() + right.start() + left.join() + right.join() + + +def _pipe(src: socket.socket, dst: socket.socket) -> None: + while True: + try: + data = src.recv(_BUFFER_SIZE) + except OSError: + break + if not data: + break + try: + dst.sendall(data) + except OSError: + break + try: + dst.shutdown(socket.SHUT_WR) + except OSError: + pass + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--spec-json", required=True) + ns = parser.parse_args(argv) + specs = tuple(ForwardSpec(**item) for item in json.loads(ns.spec_json)) + forwarder = _Forwarder(specs) + + def request_stop(signum: int, _frame: object) -> None: + del signum + forwarder.stop() + + signal.signal(signal.SIGTERM, request_stop) + signal.signal(signal.SIGINT, request_stop) + forwarder.start() + print(json.dumps({"forwards": [asdict(s) for s in forwarder.bound]}), flush=True) + try: + forwarder.serve() + finally: + forwarder.stop() + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/bot_bottle/backend/smolmachines/sidecar_bundle.py b/bot_bottle/backend/smolmachines/sidecar_bundle.py index 60b3413..59916b1 100644 --- a/bot_bottle/backend/smolmachines/sidecar_bundle.py +++ b/bot_bottle/backend/smolmachines/sidecar_bundle.py @@ -23,6 +23,8 @@ Plans (EgressPlan, …) lands in chunk 2d.""" from __future__ import annotations +import os +import socket import subprocess from dataclasses import dataclass, field from pathlib import Path @@ -34,6 +36,7 @@ from ..docker.sidecar_bundle import ( SIDECAR_BUNDLE_DOCKERFILE, SIDECAR_BUNDLE_IMAGE, ) +from . import smolvm as _smolvm _REPO_DIR = str(Path(__file__).resolve().parent.parent.parent.parent) @@ -54,6 +57,11 @@ def bundle_container_name(slug: str) -> str: return f"bot-bottle-sidecars-{slug}" +def bundle_machine_name(slug: str) -> str: + """Sidecar smolVM machine name.""" + return bundle_container_name(slug) + + @dataclass(frozen=True) class BundleLaunchSpec: """Everything `start_bundle` needs to bring up one bundle @@ -92,6 +100,11 @@ class BundleLaunchSpec: publish_host_ip: str = "127.0.0.1" +@dataclass(frozen=True) +class BundleVmLaunch: + raw_ports: dict[int, int] + + def ensure_bundle_image(image: str = SIDECAR_BUNDLE_IMAGE) -> None: """Build the sidecar bundle image before `docker run`. @@ -107,6 +120,70 @@ def ensure_bundle_image(image: str = SIDECAR_BUNDLE_IMAGE) -> None: ) +def allocate_raw_host_ports(container_ports: Sequence[int]) -> dict[int, int]: + """Reserve candidate host loopback ports for smolVM `-p HOST:GUEST`. + + The sockets are closed before smolVM binds them, so this remains + best-effort. smolVM failing to bind a selected port is fatal in + the caller's launch path.""" + out: dict[int, int] = {} + for container_port in container_ports: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + out[container_port] = int(sock.getsockname()[1]) + return out + + +def start_bundle_vm( + spec: BundleLaunchSpec, + *, + from_path: Path, + host_env: dict[str, str] | None = None, +) -> BundleVmLaunch: + """Create and start the sidecar bundle as a smolVM. + + smolVM's own published ports are raw host-loopback ports. The + launch flow wraps them with per-bottle address-bound forwarders + before exposing anything to the agent VM.""" + raw_ports = allocate_raw_host_ports(spec.ports_to_publish) + effective_host_env = host_env if host_env is not None else os.environ + env: dict[str, str] = {"BOT_BOTTLE_SIDECAR_DAEMONS": spec.daemons_csv} + for entry in spec.environment: + name, sep, value = entry.partition("=") + if sep: + env[name] = value + elif entry in effective_host_env: + env[entry] = effective_host_env[entry] + name = bundle_machine_name(spec.slug) + _smolvm.machine_create( + name, + from_path=from_path, + net=True, + env=env, + volumes=spec.volumes, + ports=tuple( + (host_port, guest_port) + for guest_port, host_port in raw_ports.items() + ), + ) + _smolvm.machine_start(name) + _smolvm.wait_exec_ready(name) + return BundleVmLaunch(raw_ports=raw_ports) + + +def stop_bundle_vm(slug: str) -> None: + """Best-effort sidecar VM teardown.""" + name = bundle_machine_name(slug) + try: + _smolvm.machine_stop(name) + except _smolvm.SmolvmError as exc: + warn(f"smolvm machine stop {name} failed: {exc}") + try: + _smolvm.machine_delete(name) + except _smolvm.SmolvmError as exc: + warn(f"smolvm machine delete {name} failed: {exc}") + + def create_bundle_network(network_name: str, subnet: str, gateway: str) -> None: """`docker network create` with an explicit subnet + gateway so the bundle's `--ip` lands on the address the Smolfile's diff --git a/bot_bottle/backend/smolmachines/smolvm.py b/bot_bottle/backend/smolmachines/smolvm.py index 41fa9bd..57593e5 100644 --- a/bot_bottle/backend/smolmachines/smolvm.py +++ b/bot_bottle/backend/smolmachines/smolvm.py @@ -113,8 +113,11 @@ def machine_create( *, image: str | None = None, from_path: Path | None = None, + net: bool = False, allow_cidrs: Sequence[str] = (), env: Mapping[str, str] | None = None, + ports: Sequence[tuple[int, int]] = (), + volumes: Sequence[tuple[str, str, bool]] = (), ) -> None: """`smolvm machine create --name NAME [--image IMG | --from PATH] [--allow-cidr CIDR ...] [-e K=V ...]`. NAME is passed as @@ -131,19 +134,24 @@ def machine_create( no-pull-at-start property. The flag form gives the same result without the Smolfile complication. - `--net` is sent explicitly when `allow_cidrs` is non-empty. - `--allow-cidr` implies `--net` per the CLI help, but sending - `--net` explicitly is harmless and ensures the guest has + `--net` is sent explicitly when `net=True` or `allow_cidrs` is + non-empty. `--allow-cidr` implies `--net` per the CLI help, but + sending `--net` explicitly is harmless and ensures the guest has network access even if that implication changes across versions.""" args: list[str] = ["machine", "create", "--name", name] if image is not None: args += ["--image", image] if from_path is not None: args += ["--from", str(from_path)] - if allow_cidrs: + if net or allow_cidrs: args.append("--net") for cidr in allow_cidrs: args += ["--allow-cidr", cidr] + for host_port, guest_port in ports: + args += ["-p", f"{host_port}:{guest_port}"] + for host_path, guest_path, read_only in volumes: + suffix = ":ro" if read_only else "" + args += ["-v", f"{host_path}:{guest_path}{suffix}"] if env: for k, v in env.items(): args += ["-e", f"{k}={v}"] diff --git a/docs/prds/prd-new-smolmachines-sidecar-vm.md b/docs/prds/prd-new-smolmachines-sidecar-vm.md index 1c980e3..440a1ed 100644 --- a/docs/prds/prd-new-smolmachines-sidecar-vm.md +++ b/docs/prds/prd-new-smolmachines-sidecar-vm.md @@ -1,6 +1,6 @@ # PRD prd-new: smolmachines sidecar VM -- **Status:** Draft +- **Status:** Active - **Author:** codex - **Created:** 2026-07-09 - **Issue:** #332 @@ -50,15 +50,31 @@ other bottle sidecars if those services share the allowed address. ## Design -The sidecar VM should continue to use the existing sidecar bundle image and +The sidecar VM continues to use the existing sidecar bundle image and `/app/sidecar_init.py` supervisor. The launch flow builds the sidecar image, packs it into a `.smolmachine` artifact, creates a per-bottle sidecar VM from that artifact, passes the same daemon-selection environment the Docker bundle uses today, mounts or copies the same daemon-private config/state inputs, starts -the sidecar VM, discovers the host-side published ports, then starts the agent -VM with `--allow-cidr /32`. +the sidecar VM, wraps the sidecar VM's raw smolVM-published ports with a local +per-bottle forwarder, then starts the agent VM with +`--allow-cidr /32`. -Agent-facing URLs are stamped from the sidecar VM's published ports: +smolVM currently exposes guest ports as `HOST:GUEST` port pairs bound on host +loopback. It does not expose an address-scoped bind like +`127.0.0.16::9099`. Because TSI is IP-only, bot-bottle must not advertise +those raw loopback ports directly to the agent. Instead, bot-bottle runs one +small stdlib Python TCP forwarder process per bottle: + +```text +agent VM + -> TSI allows only /32 + -> bot-bottle forwarder bound to : + -> raw smolVM-published sidecar port on 127.0.0.1: + -> sidecar VM service +``` + +Agent-facing URLs are stamped from the forwarder ports, never the raw smolVM +ports: - `HTTP_PROXY` / `HTTPS_PROXY` point at the published egress proxy port. - `GIT_GATE_URL` points at the published git HTTP port when git-gate is enabled. @@ -68,34 +84,29 @@ Agent-facing URLs are stamped from the sidecar VM's published ports: Internal sidecar-only services stay bound inside the sidecar VM and are not published. -## Open blocker +### Forwarder constraints -The implementation depends on host-side port publishing that can bind to the -per-bottle address. Current smolVM documentation and upstream source shape show -`-p/--port` accepting `HOST:GUEST`, with `HOST` represented as a port number, -not an IP address plus port. If smolVM cannot bind a guest port to a specific -host address such as `127.0.0.16:`, bot-bottle needs one of these before -implementation can proceed: - -1. smolVM support for address-scoped port publishing; -2. a host-side forwarding layer that binds only the per-bottle address and - forwards into the sidecar VM; or -3. a different sidecar VM networking design that preserves the same IP-only TSI - isolation property. - -Publishing sidecar ports on generic host localhost is not acceptable because -TSI's allowlist is IP-only and would no longer prove that the agent can reach -only its own sidecar surface. +- Reject wildcard listener binds (`0.0.0.0`, `::`) and generic localhost + listener binds (`127.0.0.1`, `::1`) for agent-facing forwards. +- Accept only fixed loopback targets selected by launch; the client cannot + choose or influence the forwarding target. +- Forward bytes transparently without parsing HTTP, CONNECT, TLS, Git, or MCP. +- Log lifecycle and endpoint metadata only; never log payload bytes. +- Create forwards only for egress, git HTTP when enabled, and supervise when + enabled. +- Fail closed if any listener cannot bind exactly the requested per-bottle + address. ## Implementation chunks -1. Confirm or add an address-scoped sidecar VM port publishing mechanism. -2. Extract the existing image-to-smolmachine pack helper so agent and sidecar +1. Extract the existing image-to-smolmachine pack helper so agent and sidecar artifacts share the same cache and registry path. +2. Add a strict stdlib host TCP forwarder for per-bottle address-bound + forwarding to raw smolVM-published sidecar ports. 3. Add a sidecar VM launch spec and lifecycle helpers: pack, create, start, publish/discover ports, stop, delete. 4. Switch smolmachines launch from Docker sidecar bundle lifecycle to sidecar VM - lifecycle. + plus forwarder lifecycle. 5. Update egress apply / reload paths for the sidecar VM supervisor. 6. Add unit tests for argv shape, URL stamping, teardown, and fail-closed behavior. diff --git a/tests/unit/test_smolmachines_egress_apply.py b/tests/unit/test_smolmachines_egress_apply.py new file mode 100644 index 0000000..3bb2d25 --- /dev/null +++ b/tests/unit/test_smolmachines_egress_apply.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +import unittest +from unittest.mock import patch + +from bot_bottle.backend.egress_apply import EgressApplyError +from bot_bottle.backend.smolmachines import egress_apply +from bot_bottle.backend.smolmachines.smolvm import SmolvmRunResult + + +class TestFetchCurrentRoutes(unittest.TestCase): + def test_reads_routes_from_sidecar_vm(self): + with patch.object( + egress_apply._smolvm, + "machine_exec", + return_value=SmolvmRunResult(0, "routes", ""), + ) as exec_: + self.assertEqual("routes", egress_apply.fetch_current_routes("dev-abc")) + exec_.assert_called_once_with( + "bot-bottle-sidecars-dev-abc", + ["cat", "/etc/egress/routes.yaml"], + ) + + def test_read_failure_raises_apply_error(self): + with patch.object( + egress_apply._smolvm, + "machine_exec", + return_value=SmolvmRunResult(1, "", "nope"), + ): + with self.assertRaises(EgressApplyError): + egress_apply.fetch_current_routes("dev-abc") + + +class TestReload(unittest.TestCase): + def test_sends_hup_to_sidecar_init(self): + with patch.object( + egress_apply._smolvm, + "machine_exec", + return_value=SmolvmRunResult(0, "", ""), + ) as exec_: + egress_apply.SmolmachinesEgressApplicator()._signal_bundle_reload("dev-abc") + exec_.assert_called_once_with( + "bot-bottle-sidecars-dev-abc", + ["sh", "-c", "kill -HUP 1"], + ) + + def test_reload_failure_raises_apply_error(self): + with patch.object( + egress_apply._smolvm, + "machine_exec", + return_value=SmolvmRunResult(1, "", "nope"), + ): + with self.assertRaises(EgressApplyError): + egress_apply.SmolmachinesEgressApplicator()._signal_bundle_reload("dev-abc") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_smolmachines_port_forward.py b/tests/unit/test_smolmachines_port_forward.py new file mode 100644 index 0000000..8adf2e1 --- /dev/null +++ b/tests/unit/test_smolmachines_port_forward.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +import socket +import threading +import unittest + +from bot_bottle.backend.smolmachines.port_forward import ( + ForwardSpec, + start_forwarder, + stop_forwarder, +) + + +def _echo_server() -> tuple[socket.socket, int]: + listener = socket.create_server(("127.0.0.1", 0)) + port = int(listener.getsockname()[1]) + + def run() -> None: + with listener: + conn, _ = listener.accept() + with conn: + while True: + data = conn.recv(65536) + if not data: + return + conn.sendall(data) + + threading.Thread(target=run, daemon=True).start() + return listener, port + + +class TestForwarderValidation(unittest.TestCase): + def test_rejects_generic_localhost_listener(self): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec( + label="egress", + listen_host="127.0.0.1", + listen_port=0, + target_host="127.0.0.1", + target_port=9099, + ), + )) + + def test_rejects_wildcard_listener(self): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec( + label="egress", + listen_host="0.0.0.0", + listen_port=0, + target_host="127.0.0.1", + target_port=9099, + ), + )) + + def test_rejects_non_loopback_target(self): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec( + label="egress", + listen_host="127.0.0.16", + listen_port=0, + target_host="192.0.2.10", + target_port=9099, + ), + )) + + +class TestForwarding(unittest.TestCase): + def test_forwards_bytes_both_directions(self): + try: + probe = socket.create_server(("127.0.0.16", 0)) + except OSError: + self.skipTest("127.0.0.16 is not bound on this host") + else: + probe.close() + _listener, target_port = _echo_server() + handle = start_forwarder(( + ForwardSpec( + label="egress", + listen_host="127.0.0.16", + listen_port=0, + target_host="127.0.0.1", + target_port=target_port, + ), + )) + try: + bound = handle.forwards[0] + with socket.create_connection((bound.listen_host, bound.listen_port)) as sock: + sock.sendall(b"hello") + self.assertEqual(b"hello", sock.recv(5)) + finally: + stop_forwarder(handle) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index 695187d..bd33c77 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -432,12 +432,7 @@ class TestBundleLaunchSpec(unittest.TestCase): def test_canary_env_visible_to_smolvm_guest(self): plan = _plan(canary=True) - with patch.object( - _launch._bundle, - "bundle_host_port", - return_value="65000", - ): - stamped = _launch._discover_urls(plan, "127.0.0.16") + stamped = _launch._discover_urls(plan, "127.0.0.16", {9099: 65000}) self.assertEqual( "fake-canary-value", @@ -507,21 +502,11 @@ class TestProvisionGitUser(unittest.TestCase): class TestProxyHost(unittest.TestCase): - """_proxy_host returns the bridge gateway on Linux and the loopback - alias on other platforms.""" + """_proxy_host returns the per-bottle address used by the forwarder.""" - def test_linux_returns_bundle_gateway(self): + def test_returns_loopback_alias(self): plan = _plan() - with patch("bot_bottle.backend.smolmachines.launch.platform.system", - return_value="Linux"): - result = _launch._proxy_host(plan, "127.0.0.16") - self.assertEqual(plan.bundle_gateway, result) - - def test_non_linux_returns_loopback(self): - plan = _plan() - with patch("bot_bottle.backend.smolmachines.launch.platform.system", - return_value="Darwin"): - result = _launch._proxy_host(plan, "127.0.0.16") + result = _launch._proxy_host(plan, "127.0.0.16") self.assertEqual("127.0.0.16", result) @@ -544,14 +529,20 @@ class TestDiscoverUrls(unittest.TestCase): ),), ), ) - with patch.object(_launch._bundle, "bundle_host_port", return_value="9420"): - stamped = _launch._discover_urls(plan, "127.0.0.16") - self.assertEqual("127.0.0.16:9420", stamped.agent_git_gate_host) + stamped = _launch._discover_urls( + plan, + "127.0.0.16", + {9099: 65000, 9420: 65001}, + ) + self.assertEqual("127.0.0.16:65001", stamped.agent_git_gate_host) def test_supervise_url_set_when_supervise_present(self): plan = _plan(supervise=True) - with patch.object(_launch._bundle, "bundle_host_port", return_value="55556"): - stamped = _launch._discover_urls(plan, "127.0.0.16") + stamped = _launch._discover_urls( + plan, + "127.0.0.16", + {9099: 65000, 9100: 55556}, + ) self.assertEqual("http://127.0.0.16:55556/", stamped.agent_supervise_url) diff --git a/tests/unit/test_smolmachines_sidecar_bundle.py b/tests/unit/test_smolmachines_sidecar_bundle.py index a207582..8377434 100644 --- a/tests/unit/test_smolmachines_sidecar_bundle.py +++ b/tests/unit/test_smolmachines_sidecar_bundle.py @@ -18,9 +18,11 @@ from bot_bottle.backend.smolmachines.sidecar_bundle import ( bundle_network_name, create_bundle_network, ensure_bundle_image, + start_bundle_vm, remove_bundle_network, start_bundle, stop_bundle, + stop_bundle_vm, ) @@ -200,6 +202,57 @@ class TestEnsureBundleImage(unittest.TestCase): self.assertEqual("Dockerfile.sidecars", kwargs["dockerfile"]) +class TestStartBundleVm(unittest.TestCase): + def test_creates_sidecar_vm_with_ports_volumes_and_env(self): + calls: list[tuple[str, tuple[object, ...], dict[str, object]]] = [] + + def record(name): # type: ignore + def inner(*args, **kwargs): # type: ignore + calls.append((name, args, kwargs)) + return inner + + with patch( + "bot_bottle.backend.smolmachines.sidecar_bundle.allocate_raw_host_ports", + return_value={9099: 55001, 9420: 55002}, + ), patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_create", + side_effect=record("create"), + ), patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_start", + side_effect=record("start"), + ), patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.wait_exec_ready", + side_effect=record("wait"), + ): + launch = start_bundle_vm( + _spec( + daemons_csv="egress,git-http", + environment=("FOO=bar", "TOKEN"), + volumes=(("/host/routes", "/etc/egress", True),), + ports_to_publish=(9099, 9420), + ), + from_path=Path("/cache/sidecar.smolmachine"), + host_env={"TOKEN": "secret"}, + ) + + self.assertEqual({9099: 55001, 9420: 55002}, launch.raw_ports) + create = calls[0] + self.assertEqual("create", create[0]) + self.assertEqual(("bot-bottle-sidecars-demo-abc12",), create[1]) + self.assertEqual(Path("/cache/sidecar.smolmachine"), create[2]["from_path"]) + self.assertTrue(create[2]["net"]) + self.assertEqual(((55001, 9099), (55002, 9420)), create[2]["ports"]) + self.assertEqual((("/host/routes", "/etc/egress", True),), create[2]["volumes"]) + self.assertEqual( + { + "BOT_BOTTLE_SIDECAR_DAEMONS": "egress,git-http", + "FOO": "bar", + "TOKEN": "secret", + }, + create[2]["env"], + ) + + class TestStopBundle(unittest.TestCase): def _patch_run(self, **kwargs): # type: ignore return patch( @@ -222,5 +275,17 @@ class TestStopBundle(unittest.TestCase): stop_bundle("demo-abc12") # no raise +class TestStopBundleVm(unittest.TestCase): + def test_stops_then_deletes_sidecar_vm(self): + with patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_stop", + ) as stop, patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_delete", + ) as delete: + stop_bundle_vm("demo-abc12") + stop.assert_called_once_with("bot-bottle-sidecars-demo-abc12") + delete.assert_called_once_with("bot-bottle-sidecars-demo-abc12") + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_smolmachines_smolvm.py b/tests/unit/test_smolmachines_smolvm.py index 9fe6427..5e16359 100644 --- a/tests/unit/test_smolmachines_smolvm.py +++ b/tests/unit/test_smolmachines_smolvm.py @@ -106,6 +106,22 @@ class TestArgvShapes(unittest.TestCase): self.assertIn("--name", argv) self.assertIn("agent-xyz", argv) + def test_machine_create_with_net_ports_and_volumes(self): + with self._patch_run() as m: + machine_create( + "sidecar-xyz", + from_path=Path("/stage/sidecar.smolmachine"), + net=True, + ports=((55000, 9099),), + volumes=(("/host/routes", "/etc/egress", True),), + ) + argv = m.call_args.args[0] + self.assertIn("--net", argv) + self.assertIn("-p", argv) + self.assertIn("55000:9099", argv) + self.assertIn("-v", argv) + self.assertIn("/host/routes:/etc/egress:ro", argv) + def test_machine_create_omits_net_when_no_allow_cidrs(self): with self._patch_run() as m: machine_create("agent-xyz", from_path=Path("/x.smolmachine"))