diff --git a/bot_bottle/cli/supervise.py b/bot_bottle/cli/supervise.py index 9812cf5..65fb430 100644 --- a/bot_bottle/cli/supervise.py +++ b/bot_bottle/cli/supervise.py @@ -3,7 +3,8 @@ act on them (approve / modify / reject). Curses-based TUI; modify-then-approve shells out to $EDITOR. The approval handler wires to PRD 0016 (capability-block), which rebuilds -the bottle Dockerfile. The egress-block tool was removed in issue #198. +the bottle Dockerfile. Egress proposals are queued for operator review +as full routes.yaml updates. """ from __future__ import annotations @@ -40,6 +41,8 @@ from ..supervise import ( STATUS_MODIFIED, STATUS_REJECTED, TOOL_CAPABILITY_BLOCK, + TOOL_ALLOW, + TOOL_EGRESS_BLOCK, archive_proposal, list_pending_proposals, render_diff, @@ -115,6 +118,8 @@ def _detail_lines( def _suffix_for_tool(tool: str) -> str: if tool == TOOL_CAPABILITY_BLOCK: return ".dockerfile" + if tool in (TOOL_ALLOW, TOOL_EGRESS_BLOCK): + return ".yaml" return ".txt" diff --git a/bot_bottle/supervise.py b/bot_bottle/supervise.py index df01a67..8f9b993 100644 --- a/bot_bottle/supervise.py +++ b/bot_bottle/supervise.py @@ -5,7 +5,7 @@ queue/audit support. The sidecar (bot_bottle.supervise_server) sits on the bottle's internal network and exposes three MCP tools the agent calls when it hits a stuck-recovery category: - * egress-block — agent proposes a new routes.yaml + * egress-block / allow — agent proposes a new routes.yaml * capability-block — agent proposes a new agent Dockerfile Each tool call: the agent passes the full proposed file plus a @@ -49,27 +49,34 @@ SUPERVISE_HOSTNAME = "supervise" SUPERVISE_PORT = 9100 TOOL_CAPABILITY_BLOCK = "capability-block" +TOOL_EGRESS_BLOCK = "egress-block" +TOOL_ALLOW = "allow" TOOL_LIST_EGRESS_ROUTES = "list-egress-routes" TOOLS: tuple[str, ...] = ( + TOOL_ALLOW, TOOL_CAPABILITY_BLOCK, + TOOL_EGRESS_BLOCK, TOOL_LIST_EGRESS_ROUTES, ) # The supervise sidecar uses these to query egress's # introspection endpoint for the `list-egress-routes` MCP # tool. The hostname + port match egress's docker network -# alias + listen port (see bot_bottle.egress.EGRESS_HOSTNAME -# and backend.docker.egress.EGRESS_PORT — the values -# are inlined here so the in-container supervise_server doesn't -# need to import the egress package). -EGRESS_FORWARD_PROXY = "http://egress:9099" +# listen port (see backend.docker.egress.EGRESS_PORT). The supervise +# daemon runs inside the sidecar bundle alongside egress, so loopback +# is the stable address across docker, smolmachines, and Apple +# Container backends. +EGRESS_FORWARD_PROXY = "http://127.0.0.1:9099" EGRESS_INTROSPECT_URL = "http://_egress.local/allowlist" # capability-block has no on-disk config the operator edits in place # (the Dockerfile is rebuilt, not patched), so it has no audit log -# here — those changes are captured by git history + the rebuild -# record laid down in PRD 0016. egress-block was removed in issue #198. -COMPONENT_FOR_TOOL: dict[str, str] = {} +# here — those changes are captured by git history + the rebuild record +# laid down in PRD 0016. +COMPONENT_FOR_TOOL: dict[str, str] = { + TOOL_ALLOW: "egress", + TOOL_EGRESS_BLOCK: "egress", +} STATUS_APPROVED = "approved" STATUS_MODIFIED = "modified" @@ -431,9 +438,9 @@ def sha256_hex(content: str) -> str: # Dockerfile and propose modifications. # # routes.yaml + allowlist used to live here too; PRD 0017 chunk 3 -# moved them behind the `list-egress-routes` MCP tool (live -# state from egress's introspection endpoint) so the agent -# always sees current data rather than a launch-time snapshot. +# moved them behind the `list-egress-routes` MCP tool (live state +# from egress's introspection endpoint) so the agent always sees +# current data rather than a launch-time snapshot. CURRENT_CONFIG_DOCKERFILE = "Dockerfile" diff --git a/bot_bottle/supervise_server.py b/bot_bottle/supervise_server.py index f5fe67f..d50f642 100644 --- a/bot_bottle/supervise_server.py +++ b/bot_bottle/supervise_server.py @@ -1,8 +1,8 @@ """Supervise sidecar HTTP server (PRD 0013). Per-bottle MCP server exposing tools the agent calls to propose config -changes when stuck. The egress-block tool was removed in issue #198; -the remaining tools are `capability-block` and `list-egress-routes`. +changes when stuck. The tools are `allow`, `egress-block`, +`capability-block`, and `list-egress-routes`. Each queued tool call: @@ -44,9 +44,15 @@ import urllib.request from dataclasses import dataclass from pathlib import Path -# Same-directory import inside the bundle container; `supervise.py` -# is COPYed alongside this file by Dockerfile.sidecars. -import supervise as _sv +try: + # Same-directory imports inside the bundle container; these files are + # COPYed flat under /app by Dockerfile.sidecars. + from egress_addon_core import load_routes + import supervise as _sv +except ModuleNotFoundError: + # Package imports for host-side tests and tooling. + from .egress_addon_core import load_routes + from . import supervise as _sv # --- JSON-RPC / MCP plumbing ---------------------------------------------- @@ -142,8 +148,9 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ "allowlist. Returns JSON with one entry per allowed host, " "each carrying its matches rules (if any) and whether " "the proxy injects Authorization for the route. Use this " - "before composing an `egress-block` proposal so the new " - "routes file extends the live one rather than replacing it." + "before composing an `allow` or `egress-block` proposal so " + "the new routes file extends the live one rather than " + "replacing it." ), "inputSchema": { "type": "object", @@ -151,6 +158,54 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ "additionalProperties": False, }, }, + { + "name": _sv.TOOL_ALLOW, + "description": ( + "Request operator approval to change the bottle's egress " + "allowlist. Pass the full proposed routes.yaml content, not " + "just the new host, plus a justification. Use " + "`list-egress-routes` first so the proposal preserves existing " + "routes." + ), + "inputSchema": { + "type": "object", + "properties": { + "routes_yaml": { + "type": "string", + "description": "Full proposed /etc/egress/routes.yaml content.", + }, + "justification": { + "type": "string", + "description": "Why this egress route is needed.", + }, + }, + "required": ["routes_yaml", "justification"], + }, + }, + { + "name": _sv.TOOL_EGRESS_BLOCK, + "description": ( + "Request operator approval to change the bottle's egress " + "allowlist after a blocked outbound request. Pass the full " + "proposed routes.yaml content plus a justification. Use " + "`list-egress-routes` first so the proposal preserves existing " + "routes." + ), + "inputSchema": { + "type": "object", + "properties": { + "routes_yaml": { + "type": "string", + "description": "Full proposed /etc/egress/routes.yaml content.", + }, + "justification": { + "type": "string", + "description": "Why this egress route is needed.", + }, + }, + "required": ["routes_yaml", "justification"], + }, + }, { "name": _sv.TOOL_CAPABILITY_BLOCK, "description": ( @@ -182,11 +237,12 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ ] -# Map each non-egress tool to the input field that carries the agent's -# payload (stored in Proposal.proposed_file). egress-block builds its -# payload from structured input fields in `handle_egress_block`. +# Map each proposal tool to the input field that carries the agent's +# payload (stored in Proposal.proposed_file). PROPOSED_FILE_FIELD: dict[str, str] = { + _sv.TOOL_ALLOW: "routes_yaml", _sv.TOOL_CAPABILITY_BLOCK: "dockerfile", + _sv.TOOL_EGRESS_BLOCK: "routes_yaml", } @@ -203,6 +259,14 @@ def validate_proposed_file(tool: str, content: str) -> None: # Dockerfiles are too varied to validate syntactically beyond # non-empty. The operator reads the diff in the TUI. pass + elif tool in (_sv.TOOL_ALLOW, _sv.TOOL_EGRESS_BLOCK): + try: + load_routes(content) + except ValueError as e: + raise _RpcError( + ERR_INVALID_PARAMS, + f"{tool}: proposed routes.yaml is not valid: {e}", + ) from e else: raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}") diff --git a/tests/unit/test_supervise.py b/tests/unit/test_supervise.py index 8eeb8db..55cb386 100644 --- a/tests/unit/test_supervise.py +++ b/tests/unit/test_supervise.py @@ -317,15 +317,22 @@ class TestToolConstants(unittest.TestCase): def test_tools_tuple_matches_individual_constants(self): self.assertEqual( ( + supervise.TOOL_ALLOW, TOOL_CAPABILITY_BLOCK, + supervise.TOOL_EGRESS_BLOCK, supervise.TOOL_LIST_EGRESS_ROUTES, ), supervise.TOOLS, ) - def test_component_map_has_no_entries(self): - # egress-block removed in issue #198; capability-block never had one. - self.assertEqual({}, supervise.COMPONENT_FOR_TOOL) + def test_component_map_has_egress_entries(self): + self.assertEqual( + { + supervise.TOOL_ALLOW: "egress", + supervise.TOOL_EGRESS_BLOCK: "egress", + }, + supervise.COMPONENT_FOR_TOOL, + ) class _StubSupervise(supervise.Supervise): diff --git a/tests/unit/test_supervise_cli.py b/tests/unit/test_supervise_cli.py index f7ad6d4..94f137c 100644 --- a/tests/unit/test_supervise_cli.py +++ b/tests/unit/test_supervise_cli.py @@ -2,9 +2,6 @@ The curses TUI itself isn't exercised here — these tests cover the discovery + approve/reject paths that the TUI's key handlers call into. - -egress-block (add_route) was removed in issue #198; the TestEgressApplyWiring -class and all stubs for add_route have been dropped accordingly. """ import os @@ -33,6 +30,8 @@ FIXED = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc) def _proposal(slug: str = "dev", tool: str = TOOL_CAPABILITY_BLOCK) -> Proposal: payloads = { TOOL_CAPABILITY_BLOCK: "FROM python:3.13\n", + supervise.TOOL_ALLOW: "routes:\n - host: example.com\n", + supervise.TOOL_EGRESS_BLOCK: "routes:\n - host: example.com\n", } payload = payloads.get(tool, "") return Proposal.new( @@ -154,6 +153,14 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): supervise_cli.approve(qp) self.assertEqual([], read_audit_entries("egress", "dev")) + def test_approve_egress_block_writes_audit_log(self): + qp = self._enqueue(tool=supervise.TOOL_EGRESS_BLOCK) + supervise_cli.approve(qp) + entries = read_audit_entries("egress", "dev") + self.assertEqual(1, len(entries)) + self.assertEqual(STATUS_APPROVED, entries[0].operator_action) + self.assertEqual("needed for dev", entries[0].justification) + # class TestCapabilityApplyWiring(_FakeHomeMixin, unittest.TestCase): # # DISABLED — capability_apply functionality is currently commented out. diff --git a/tests/unit/test_supervise_server.py b/tests/unit/test_supervise_server.py index aed68d4..a09a870 100644 --- a/tests/unit/test_supervise_server.py +++ b/tests/unit/test_supervise_server.py @@ -54,13 +54,19 @@ class TestValidation(unittest.TestCase): ) def test_empty_proposed_file_rejected_for_tools_with_file_field(self): - # egress-block has structured input (validated in - # _validate_and_bundle_egress_route, not here) and - # list-egress-routes takes no input. Only capability-block - # goes through `validate_proposed_file`. with self.assertRaises(_RpcError): validate_proposed_file(_sv.TOOL_CAPABILITY_BLOCK, " \n\t") + def test_egress_routes_yaml_is_validated(self): + validate_proposed_file( + _sv.TOOL_ALLOW, + "routes:\n - host: example.com\n", + ) + + def test_invalid_egress_routes_yaml_rejected(self): + with self.assertRaises(_RpcError): + validate_proposed_file(_sv.TOOL_EGRESS_BLOCK, "routes: nope\n") + # --- JSON-RPC parsing ------------------------------------------------------ @@ -141,7 +147,9 @@ class TestHandleToolsList(unittest.TestCase): names = [t["name"] for t in result["tools"]] # type: ignore[index] self.assertEqual( sorted([ + _sv.TOOL_ALLOW, _sv.TOOL_CAPABILITY_BLOCK, + _sv.TOOL_EGRESS_BLOCK, _sv.TOOL_LIST_EGRESS_ROUTES, ]), sorted(names), @@ -172,6 +180,17 @@ class TestHandleToolsList(unittest.TestCase): # No `required` array because no inputs are required. self.assertNotIn("required", schema) # type: ignore[operator] + def test_egress_tools_take_routes_yaml_and_justification(self): + for tool_name in (_sv.TOOL_ALLOW, _sv.TOOL_EGRESS_BLOCK): + with self.subTest(tool_name=tool_name): + tool = next(t for t in TOOL_DEFINITIONS if t["name"] == tool_name) + schema = tool["inputSchema"] + self.assertEqual("object", schema["type"]) # type: ignore[index] + self.assertEqual( + ["routes_yaml", "justification"], + schema["required"], # type: ignore[index] + ) + class TestHandleToolsCall(unittest.TestCase): def setUp(self): @@ -220,6 +239,26 @@ class TestHandleToolsCall(unittest.TestCase): self.assertIn("status: approved", text) self.assertIn("notes: lgtm", text) + def test_allow_round_trips_through_queue(self): + responder = self._respond_when_proposal_appears(_sv.STATUS_APPROVED, notes="ok") + try: + result = handle_tools_call( + { + "name": _sv.TOOL_ALLOW, + "arguments": { + "routes_yaml": "routes:\n - host: example.com\n", + "justification": "need example.com", + }, + }, + self.config, + ) + finally: + responder.join() + self.assertFalse(result["isError"]) # type: ignore[index] + text = result["content"][0]["text"] # type: ignore[index] + self.assertIn("status: approved", text) + self.assertIn("notes: ok", text) + def test_rejected_response_sets_isError(self): responder = self._respond_when_proposal_appears(_sv.STATUS_REJECTED, notes="nope") try: @@ -412,7 +451,8 @@ class TestHttpEndToEnd(unittest.TestCase): self.assertEqual(1, result["id"]) names = [t["name"] for t in result["result"]["tools"]] # type: ignore[index] self.assertIn(_sv.TOOL_CAPABILITY_BLOCK, names) - self.assertNotIn("egress-block", names) + self.assertIn(_sv.TOOL_ALLOW, names) + self.assertIn(_sv.TOOL_EGRESS_BLOCK, names) def test_unknown_method_returns_jsonrpc_error(self): result = self._post_jsonrpc(