From a515efb6b41902d03e8ac846e0707a7a62fef91e Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 23:41:04 -0400 Subject: [PATCH] refactor(compose): drop pre-create networks + pipelock CIDR allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD 0018 chunk 4 spike: empirically verified that pipelock's SSRF guard checks proxied-request destinations (e.g. api.anthropic.com → public IP) and not source IPs of incoming connections. The bottle's own internal CIDR was being added to ssrf.ip_allowlist defensively, but that defense isn't load-bearing — direct pipelock probe (`curl --proxy http://pipelock https://api.anthropic.com/`) returns 404 from upstream rather than blocking on SSRF. So: - Networks become compose-managed (`internal: true` on the internal network; the egress one is a normal user-defined bridge). Compose creates + removes them via up/down. - launch.py drops the `docker network create` + `network_inspect_cidr` + pipelock yaml re-render dance. - The pre-create/external scaffolding from chunk 3 goes with it. End-to-end `./cli.py start` still works; cleanup leaves no orphans. If real-world use surfaces an SSRF block we hadn't predicted, the allowlist can come back via subnet-pinning rather than pre-create. --- claude_bottle/backend/docker/compose.py | 15 +++---- claude_bottle/backend/docker/launch.py | 53 ++++++++----------------- tests/unit/test_compose.py | 13 +++--- 3 files changed, 29 insertions(+), 52 deletions(-) diff --git a/claude_bottle/backend/docker/compose.py b/claude_bottle/backend/docker/compose.py index 432316c..c61e305 100644 --- a/claude_bottle/backend/docker/compose.py +++ b/claude_bottle/backend/docker/compose.py @@ -130,21 +130,18 @@ def bottle_plan_to_compose(plan: DockerBottlePlan) -> dict[str, Any]: def _networks(plan: DockerBottlePlan) -> dict[str, Any]: - """Both networks are `external: true` — chunk 3 pre-creates them - via `docker network create` so pipelock's yaml can embed the - internal-network CIDR in its SSRF allowlist before compose-up. - Compose just references the pre-existing networks by name. - Network lifecycle (create / remove) is owned by the compose- - lifecycle helpers, not compose itself; `docker compose down` - leaves external networks alone.""" + """Compose-managed networks with explicit `name:` matching the + existing slug-suffixed convention. Compose creates them on `up` + and destroys them on `down`. The internal one is `--internal` + (no default gateway); the egress one is a normal user-defined + bridge.""" return { "internal": { "name": plan.proxy_plan.internal_network, - "external": True, + "internal": True, }, "egress": { "name": plan.proxy_plan.egress_network, - "external": True, }, } diff --git a/claude_bottle/backend/docker/launch.py b/claude_bottle/backend/docker/launch.py index 5bb439f..6d11962 100644 --- a/claude_bottle/backend/docker/launch.py +++ b/claude_bottle/backend/docker/launch.py @@ -44,7 +44,6 @@ from typing import Callable, Generator from ...egress import egress_resolve_token_values from ...log import info -from ...pipelock import pipelock_build_config, pipelock_render_yaml from . import network as network_mod from . import util as docker_mod from .bottle import DockerBottle @@ -64,14 +63,9 @@ from .compose import ( compose_up, write_compose_file, ) -from .egress import ( - DockerEgress, - egress_tls_init, -) +from .egress import DockerEgress, egress_tls_init from .git_gate import DockerGitGate from .pipelock import ( - PIPELOCK_CA_CERT_IN_CONTAINER, - PIPELOCK_CA_KEY_IN_CONTAINER, DockerPipelockProxy, pipelock_proxy_url, pipelock_tls_init, @@ -124,44 +118,31 @@ def launch( plan.derived_image, plan.image, plan.spec.user_cwd ) - # Step 2: pre-create networks so we know the internal CIDR - # before pipelock yaml renders. - internal_network = network_mod.network_create_internal(plan.slug) - stack.callback(network_mod.network_remove, internal_network) + # Networks: compose-managed. The names are derived + # deterministically from the slug so the renderer can put + # them on the services and `compose up` creates them with + # those names. The empirical spike confirmed pipelock's + # SSRF guard only checks proxied-request destinations, not + # source IPs — so the bottle's own internal CIDR doesn't + # need to be in `ssrf.ip_allowlist`. Pre-create + CIDR + # introspection are gone; compose owns the network + # lifecycle. + internal_network = network_mod.network_name_for_slug(plan.slug) + egress_network = network_mod.network_egress_name_for_slug(plan.slug) - egress_network = network_mod.network_create_egress(plan.slug) - stack.callback(network_mod.network_remove, egress_network) - - internal_cidr = network_mod.network_inspect_cidr(internal_network) - - # Step 3: mint per-bottle CAs into state//{pipelock,egress}/. + # Mint per-bottle CAs into state//{pipelock,egress}/. ca_cert_host, ca_key_host = pipelock_tls_init(pipelock_state_dir(plan.slug)) egress_ca_host, egress_ca_cert_only = egress_tls_init( egress_state_dir(plan.slug), ) - # Step 4: re-render pipelock yaml with the SSRF allowlist now - # that we know the internal CIDR. Prepare wrote the yaml - # without the ssrf block; overwrite the same path so the - # bind-mount picks up the updated content. - bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) - cfg = pipelock_build_config( - bottle, - ca_cert_path=PIPELOCK_CA_CERT_IN_CONTAINER, - ca_key_path=PIPELOCK_CA_KEY_IN_CONTAINER, - ssrf_ip_allowlist=(internal_cidr,), - ) - plan.proxy_plan.yaml_path.write_text(pipelock_render_yaml(cfg)) - plan.proxy_plan.yaml_path.chmod(0o600) - - # Step 5: populate launch-time fields on every inner plan so - # the renderer reads concrete network names, CA paths, and - # pipelock URL. Match the field-by-field replacement the - # pre-compose launch did, just rolled into one pass. + # Populate launch-time fields on every inner plan so the + # renderer reads concrete network names, CA paths, and + # pipelock URL. proxy_plan = dataclasses.replace( plan.proxy_plan, internal_network=internal_network, - internal_network_cidr=internal_cidr, + internal_network_cidr="", egress_network=egress_network, ca_cert_host_path=ca_cert_host, ca_key_host_path=ca_key_host, diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index 7306833..11aa01c 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -176,20 +176,19 @@ class TestProjectAndNetworks(unittest.TestCase): spec = bottle_plan_to_compose(_plan()) self.assertEqual(f"claude-bottle-{SLUG}", spec["name"]) - def test_internal_network_marked_external(self): - # Chunk 3 pre-creates networks with `docker network create - # --internal` so pipelock can know the CIDR before compose-up. - # Compose references the network by name with `external: true`. + def test_internal_network_is_internal(self): spec = bottle_plan_to_compose(_plan()) net = spec["networks"]["internal"] self.assertEqual(f"claude-bottle-net-{SLUG}", net["name"]) - self.assertTrue(net["external"]) + self.assertTrue(net["internal"]) - def test_egress_network_marked_external(self): + def test_egress_network_is_external_bridge(self): spec = bottle_plan_to_compose(_plan()) net = spec["networks"]["egress"] self.assertEqual(f"claude-bottle-egress-{SLUG}", net["name"]) - self.assertTrue(net["external"]) + # No `internal:` key on the egress network — defaults to a + # normal user-defined bridge. + self.assertNotIn("internal", net) class TestPipelockAlwaysPresent(unittest.TestCase):