Consolidated per-host gateway for the macOS (Apple container) backend #399
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 didericis
|
5c526860bc |
fix(test): resolve the remaining review findings on the control-plane auth test
test / unit (pull_request) Successful in 1m17s
test / integration (pull_request) Successful in 22s
test / coverage (pull_request) Successful in 1m21s
lint / lint (push) Successful in 2m23s
test / unit (push) Successful in 1m24s
test / integration (push) Successful in 30s
test / coverage (push) Successful in 1m26s
Update Quality Badges / update-badges (push) Successful in 1m24s
Addresses the 5 lower-priority findings left as follow-up in the earlier review, now that each has a concrete answer: - Add gateway_name: str = GATEWAY_NAME to OrchestratorService.__init__ (mirrors the existing orchestrator_name param) and thread it through _gateway(). Deletes the test's _IsolatedOrchestratorService subclass, which existed only to override a private method for this one kwarg — any caller needing gateway-name isolation can now use the public constructor. Backward compatible: every existing caller constructs OrchestratorService with keyword args and a sensible default is kept. - Give the test its own fixed image tags (bot-bottle-orchestrator:itest, bot-bottle-gateway:itest) instead of the production :latest ones. _running_image_is_current() keys gateway staleness off the image tag's ID, not per-instance identity, so rebuilding the shared :latest tag from whatever's on disk during a test run could make a real host's running production gateway look stale and get force-recreated. Fixed tags (not per-run-suffixed, so they don't accumulate) fully decouple the two. - setUp -> setUpClass/tearDownClass: all 5 tests are read-only checks against the same running control plane, so one shared container lifecycle replaces 5 (each of which paid its own container-start + image-build + health-poll cycle). Cuts the file's wall-clock roughly 4x (11.5s -> 2.9-4.3s) and, combined with the network-rm cleanup from the previous commit, means one cleanup instead of five. - Reuse OrchestratorClient (bot_bottle/orchestrator/client.py) instead of a hand-rolled urllib helper — the test now exercises the same request/response code path the real host CLI uses, rather than a private copy that could silently drift from it. - Add the chown workaround test_multitenant_isolation.py already needed for this exact bind-mount: the orchestrator container has no USER directive, so it writes the registry DB as root into the throwaway host_root; chown it back before tempdir cleanup so that doesn't raise PermissionError on native Linux Docker (no UID remap, unlike Docker Desktop's macOS VM). Verified: ran the suite twice in a row (idempotency — fixed image tags don't accumulate, 5/5 pass both times, 2.99-4.33s each), the real ~/.bot-bottle/control-plane-token is untouched, zero leaked networks or containers after either run, exactly 2 :itest images (not growing), the full orchestrator unit suite (93 tests) and the sibling docker gateway/broker integration tests still pass. pyright clean, pylint 10.00/10 on both changed files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
didericis
|
492669e620 |
fix(test): skip the control-plane auth test under act_runner (CI)
Pushing the previous two fixes surfaced a real, currently-failing CI run (gitea actions run 2164, jobs "integration" and "coverage"): every one of the 5 new tests errored in setUp with docker: Error response from daemon: error while creating mount source path '/workspace/didericis/bot-bottle': mkdir /workspace: read-only file system This is finding #4 from the review of the previous commit, now confirmed live rather than just plausible: act_runner's job-container topology can't satisfy the orchestrator's host-path bind mount, the same constraint test_multitenant_isolation.py, test_gateway_image.py, and test_sandbox_escape.py already skip around. Add the identical skip_unless_docker + GITEA_ACTIONS guard so this test degrades the same way its siblings do instead of failing the job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
didericis
|
d32f36bb2b |
fix(test): actually isolate the docker control-plane auth test's root + network
Two bugs surfaced by a review of the previous commit: - host_control_plane_token() resolves its path via the ambient BOT_BOTTLE_ROOT env var, not the host_root kwarg passed to OrchestratorService (that kwarg only controls the DB bind-mount destination). The test's isolation claim was false as a result: running it read/wrote the developer's real ~/.bot-bottle/control-plane-token instead of the throwaway temp dir — confirmed directly on disk. Fixed by pointing the env var at the same temp dir for the test's duration and restoring it via addCleanup. - ensure_running() creates a per-bottle Docker network but stop() only ever removes containers, never the network — every run of this test leaked one bridge network permanently (found and removed 5 from prior runs via `docker network ls`). Fixed with an explicit `docker network rm` in addCleanup. Verified: re-ran the suite twice: 5/5 pass both times, the real ~/.bot-bottle/control-plane-token timestamp is unchanged across both runs (proving isolation), and `docker network ls` shows zero leaked bot-bottle-net-itest-* networks afterward. pyright clean, pylint 10.00/10. Remaining findings from the same review (missing GITEA_ACTIONS skip guard, root-owned bind-mount cleanup on native Linux, no setUpClass, reinvented OrchestratorClient, gateway_name should be a constructor param rather than a subclassed private-method override) are left for a follow-up — each is a real, separate design/scope call, not a mechanical fix like these two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
didericis
|
4f36b919b2 |
test(orchestrator): live Docker coverage for the control-plane auth fix (#400)
|
||
|
didericis
|
ca91fc4d91 |
fix(security): authenticate the orchestrator control plane (issue #400 Critical)
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/<id>/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 <root>/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 <noreply@anthropic.com> |
||
|
didericis
|
4a607ad098 |
refactor(macos): one infra container (control plane + gateway), fixes shared-DB races
Adopts the firecracker infra-VM pattern for macOS: the orchestrator control plane and the gateway data plane now run in a SINGLE Apple container instead of two. Apple Containers are lightweight VMs with separate kernels, so the prior two-container design had both guests writing one bot-bottle.db over virtiofs, where fcntl locks are not coherent across kernels — concurrent writes (the orchestrator's registry vs the gateway supervise daemon's queue) could corrupt it. One container = one kernel = coherent locking. The DB moves onto a container-only Apple volume (bot-bottle-mac-db), never bind-mounted from the host, so no host process opens the live file either. The host CLI already reaches registry + supervise state over the control-plane HTTP surface (cli/supervise.py uses OrchestratorClient), exactly as firecracker's VM-only DB requires. Two simplifications fall out of the single container: - No DNS dance: the control plane and gateway daemons reach each other over 127.0.0.1, so the orchestrator-before-gateway ordering (a workaround for Apple having no container DNS) is gone, along with the moved-IP recreate logic it needed. - Net -243 lines. Mechanics: the infra container runs from the gateway image with the control-plane source bind-mounted read-only (like the docker orchestrator, so a code change needs no rebuild) and a small sh -c init that starts both processes (mirrors firecracker's _infra_init). Also implements the macOS backend's ensure_orchestrator() and adds it to discover_orchestrator_url, so operator tools (supervise) can bring up / find the control plane on demand — previously the macOS backend died with "no orchestrator control plane". Verified end-to-end on real Apple Container 1.0.0: the single infra container comes up healthy (one address for control plane + gateway), both processes run, the DB is written on the container-only volume, host-side supervise works over HTTP, and a registered agent gets 200 for an allowed host / 403 for a denied one. 1824 unit tests pass with `container` absent (CI parity), pyright clean, pylint 9.89. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
didericis
|
e24b62b6b9 |
fix(macos): review fixes — token on plan, self-heal, symmetric digest, DHCP poll
Addresses findings from a high-effort review of the PRD 0070 macOS backend. Correctness: - Stamp identity_token onto MacosContainerBottlePlan after registration. git's gitconfig extraHeader and the supervise MCP --header read getattr(plan,"identity_token","") at provision time, and both reach the gateway on NO_PROXY (bypassing the egress proxy that carries the token). The plan never carried it, so /resolve fail-closed and every git fetch/push and supervise call from a macOS bottle would have been denied. Registration precedes provision(), so — unlike the run-time env — the plan can carry it. - Self-heal the orchestrator: recreate when it is not (source-current AND answering /health), not on the source-hash label alone. A container running current code but with a wedged HTTP server was left alone and polled to death, failing every launch until manual deletion. - image_digest and container_image_digest now read the same descriptor.digest field; dropped image_digest's id/tag fallback that could yield a value the container side can't produce — a permanent mismatch would have recreated the shared gateway on every launch (severing every live bottle's egress, since the replacement gets a new DHCP address). - Poll for the agent's and gateway's DHCP address instead of a fatal read right after `container run` (there is no --ip; the address can lag start). Cleanup: - One _inspect_first + _descriptor_digest behind the four inspect readers. - Shared bind_mount_spec (util) and host_db_dir (paths) replace per-module copies; _GIT_HTTP_PORT now imports git_http_backend.DEFAULT_PORT. - Drop the dead _url cache / url property and the write-only agent_proxy_url. Deferred (noted on the PR, not fixed here): the gateway image rebuilding on every launch (needs source-hash-labeled build), SQLite shared across VM guests, and the sh -lc profile-override edge — each is design-level or behavior-risk beyond a review fix. Verified: real Apple Container bring-up is green and idempotent; 1826 unit tests pass with `container` absent (CI parity), pyright clean, pylint 9.86. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
didericis
|
a5910696a5 |
fix(test): stop the macOS unit tests shelling out to the container CLI
CI's unit + coverage jobs failed with `FileNotFoundError: 'container'`: three tests reached the real Apple CLI, which exists on a macOS dev host but not on the Linux runner. They passed locally for that reason alone — and two of them were quietly creating real Apple networks on the dev host as a side effect. - `test_enumerate_active_is_empty_while_disabled` asserted the disabled-era stub and called `enumerate_active()` unmocked. The backend launches bottles again, so it now covers the real enumeration: slug parsing, exclusion of the shared gateway/orchestrator singletons, and the CLI-failure path. - The two orchestrator tests patched `orchestrator_service.container_mod`, but `_run_orchestrator_container` reaches the CLI through `ensure_networks`, which is imported from the gateway module and resolves `container_mod` in *its* namespace. Patch the imported name instead. Adds a test that the networks exist before the orchestrator runs — the ordering the escaped call was hiding. Verified by reproducing the CI environment locally (`PATH` without the `container` binary): 3 failures before, 1818 passing after. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
didericis
|
c69642e568 |
feat(macos): consolidated per-host gateway for the Apple backend (PRD 0070)
Re-enables the macos-container backend on the shared per-host orchestrator + gateway, replacing the per-bottle companion container removed in #385. This is the last backend in PRD 0070's roadmap. Apple Container 1.0.0 forced three departures from the docker shape, each verified against the live CLI (findings recorded in the networking spike): - No `--ip`. The address is DHCP-assigned and knowable only once the container runs, so the order inverts: gateway up -> run agent -> read its address -> register. The identity token is minted by registration and therefore cannot be in the agent's run-time env; it rides the proxy URL applied at `container exec` time (bare `--env` names keep it off argv). - No container DNS. The gateway can only be handed the control plane's IP, so the orchestrator starts first and the gateway is pointed at its address. - No `network connect`. Networks are fixed at run time, so the shared host-only network is created up front; per-bottle networks would restart the gateway on every launch and defeat the consolidation. The agent runs with `--cap-drop CAP_NET_RAW`: Apple grants NET_RAW by default, which would let an agent forge a neighbour's source address on the shared segment. NET_ADMIN is already absent, so this closes the source-address half of PRD 0070's attribution invariant. Verified end-to-end on real Apple Container 1.0.0: both images build, the control plane comes up healthy, the gateway reaches it by IP, and a registered agent gets 200 for a host in its routes and 403 for one outside them. Bring-up is idempotent — a second launch does not churn the singletons. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |