feat(supervise)!: remove egress-block MCP tool and runtime route-mutation
lint / lint (push) Successful in 1m39s
test / unit (push) Successful in 40s
test / integration (push) Successful in 1m1s
test / unit (pull_request) Successful in 40s
Update Quality Badges / update-badges (push) Successful in 1m45s
test / integration (pull_request) Successful in 57s
lint / lint (push) Successful in 1m39s
test / unit (push) Successful in 40s
test / integration (push) Successful in 1m1s
test / unit (pull_request) Successful in 40s
Update Quality Badges / update-badges (push) Successful in 1m45s
test / integration (pull_request) Successful in 57s
Drops `egress-block` from the supervise sidecar, removes `_merge_single_route`, `add_route`, and `apply_routes_change` from egress_apply.py, and strips the proposal/approve/reject flow for egress from the supervise CLI. The list-egress-routes and capability-block tools are unaffected. Tests updated throughout. Closes #198
This commit was merged in pull request #202.
This commit is contained in:
@@ -1,70 +1,20 @@
|
||||
"""Host-side helper to apply a routes.yaml change to a running
|
||||
egress sidecar (PRD 0014 retargeted by PRD 0017 chunk 3, PRD 0053).
|
||||
"""Host-side helper for egress sidecar inspection (issue #198).
|
||||
|
||||
Used by the supervise dashboard when the operator approves an
|
||||
egress-block proposal. Fetches current routes.yaml, validates,
|
||||
writes into the sidecar, then SIGHUPs to reload.
|
||||
`_merge_single_route`, `add_route`, and `apply_routes_change` were
|
||||
removed when the egress-block MCP tool was dropped. The remaining
|
||||
helpers support runtime inspection and validation of the routes file
|
||||
without modifying it at runtime.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from ...egress import EGRESS_ROUTES_IN_CONTAINER
|
||||
from ...egress_addon_core import load_routes
|
||||
from ...yaml_subset import YamlSubsetError, parse_yaml_subset
|
||||
from .bottle_state import egress_state_dir
|
||||
from .sidecar_bundle import sidecar_bundle_container_name
|
||||
|
||||
|
||||
def _render_routes_payload(routes_list: list[dict[str, object]]) -> str:
|
||||
"""Render a list-of-dicts routes payload as YAML matching the
|
||||
shape `egress_render_routes` produces."""
|
||||
if not routes_list:
|
||||
return "routes: []\n"
|
||||
lines: list[str] = ["routes:"]
|
||||
for entry in routes_list:
|
||||
host = str(entry.get("host", ""))
|
||||
lines.append(f' - host: "{host}"')
|
||||
auth_scheme = entry.get("auth_scheme")
|
||||
token_env = entry.get("token_env")
|
||||
if auth_scheme and token_env:
|
||||
lines.append(f' auth_scheme: "{auth_scheme}"')
|
||||
lines.append(f' token_env: "{token_env}"')
|
||||
matches_obj = entry.get("matches")
|
||||
if isinstance(matches_obj, list) and matches_obj:
|
||||
lines.append(" matches:")
|
||||
for match_entry in matches_obj:
|
||||
me = cast(dict[str, object], match_entry)
|
||||
first_key = True
|
||||
if "paths" in me:
|
||||
lines.append(" - paths:")
|
||||
first_key = False
|
||||
for pd in cast(list[dict[str, str]], me["paths"]):
|
||||
if "type" in pd:
|
||||
lines.append(f' - type: "{pd["type"]}"')
|
||||
lines.append(f' value: "{pd["value"]}"')
|
||||
else:
|
||||
lines.append(f' - value: "{pd["value"]}"')
|
||||
if "methods" in me:
|
||||
methods_str = ", ".join(
|
||||
f'"{m}"' for m in cast(list[str], me["methods"])
|
||||
)
|
||||
prefix = " - " if first_key else " "
|
||||
lines.append(f'{prefix}methods: [{methods_str}]')
|
||||
first_key = False
|
||||
if first_key:
|
||||
lines.append(" - {}")
|
||||
return "\n".join(lines) + "\n"
|
||||
|
||||
|
||||
def _egress_routes_host_path(slug: str) -> Path:
|
||||
return egress_state_dir(slug) / "egress_routes.yaml"
|
||||
|
||||
|
||||
class EgressApplyError(RuntimeError):
|
||||
pass
|
||||
|
||||
@@ -92,144 +42,8 @@ def validate_routes_content(content: str) -> None:
|
||||
) from e
|
||||
|
||||
|
||||
def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]:
|
||||
container = sidecar_bundle_container_name(slug)
|
||||
before = fetch_current_routes(slug)
|
||||
validate_routes_content(new_content)
|
||||
|
||||
target = _egress_routes_host_path(slug)
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_text(new_content)
|
||||
target.chmod(0o644)
|
||||
sig = subprocess.run(
|
||||
["docker", "kill", "--signal", "HUP", container],
|
||||
capture_output=True, text=True, check=False,
|
||||
)
|
||||
if sig.returncode != 0:
|
||||
raise EgressApplyError(
|
||||
f"failed to SIGHUP {container}: "
|
||||
f"{(sig.stderr or '').strip()}"
|
||||
)
|
||||
|
||||
return before, new_content
|
||||
|
||||
|
||||
def _merge_single_route(
|
||||
current_yaml: str, new_route: dict[str, object],
|
||||
) -> str:
|
||||
"""Merge a single proposed route into the current routes.yaml.
|
||||
|
||||
- Host absent → append the route.
|
||||
- Host present → union the match paths (proposed ∪ existing).
|
||||
Auth is preserved from existing route.
|
||||
"""
|
||||
try:
|
||||
cfg = parse_yaml_subset(current_yaml)
|
||||
except YamlSubsetError as e:
|
||||
raise EgressApplyError(
|
||||
f"current routes.yaml is not valid YAML: {e}"
|
||||
) from e
|
||||
routes = cfg.get("routes")
|
||||
if not isinstance(routes, list):
|
||||
raise EgressApplyError(
|
||||
"current routes.yaml: 'routes' is not a list"
|
||||
)
|
||||
routes_typed = cast(list[object], routes)
|
||||
|
||||
new_host = str(new_route.get("host", "")).lower()
|
||||
if not new_host:
|
||||
raise EgressApplyError(
|
||||
"proposed route is missing 'host'"
|
||||
)
|
||||
|
||||
# Build proposed matches from the input
|
||||
proposed_matches = new_route.get("matches")
|
||||
if proposed_matches is None:
|
||||
# Accept legacy path_allowlist from agent proposals and convert
|
||||
proposed_paths = new_route.get("path_allowlist")
|
||||
if isinstance(proposed_paths, list) and proposed_paths:
|
||||
proposed_matches = [{"paths": [{"value": p} for p in proposed_paths]}]
|
||||
|
||||
for entry in routes_typed:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
entry_typed = cast(dict[str, object], entry)
|
||||
if str(entry_typed.get("host", "")).lower() == new_host:
|
||||
# Merge matches: union path values from proposed into existing
|
||||
if isinstance(proposed_matches, list) and proposed_matches:
|
||||
existing_matches = entry_typed.get("matches")
|
||||
if not isinstance(existing_matches, list):
|
||||
existing_matches = []
|
||||
# Simple merge: collect all existing path values, add new ones
|
||||
existing_paths: set[str] = set()
|
||||
for me in existing_matches:
|
||||
me_typed = cast(dict[str, object], me) if isinstance(me, dict) else {}
|
||||
paths = me_typed.get("paths")
|
||||
if isinstance(paths, list):
|
||||
for p in paths:
|
||||
p_typed = cast(dict[str, object], p) if isinstance(p, dict) else {}
|
||||
val = p_typed.get("value")
|
||||
if isinstance(val, str):
|
||||
existing_paths.add(val)
|
||||
new_paths: list[str] = []
|
||||
for me in proposed_matches:
|
||||
me_typed = cast(dict[str, object], me) if isinstance(me, dict) else {}
|
||||
paths = me_typed.get("paths")
|
||||
if isinstance(paths, list):
|
||||
for p in paths:
|
||||
p_typed = cast(dict[str, object], p) if isinstance(p, dict) else {}
|
||||
val = p_typed.get("value")
|
||||
if isinstance(val, str) and val not in existing_paths:
|
||||
new_paths.append(val)
|
||||
existing_paths.add(val)
|
||||
if new_paths:
|
||||
existing_matches.append(
|
||||
{"paths": [{"value": p} for p in new_paths]}
|
||||
)
|
||||
entry_typed["matches"] = existing_matches
|
||||
break
|
||||
else:
|
||||
entry_typed: dict[str, object] = {"host": new_route.get("host")} # type: ignore
|
||||
if isinstance(proposed_matches, list) and proposed_matches:
|
||||
entry_typed["matches"] = proposed_matches
|
||||
auth = new_route.get("auth")
|
||||
if isinstance(auth, dict) and auth.get("scheme") and auth.get("token_ref"): # type: ignore
|
||||
auth_typed = cast(dict[str, object], auth)
|
||||
existing_slots = sorted({
|
||||
str(r_entry.get("token_env", ""))
|
||||
for r_entry_obj in routes_typed
|
||||
if isinstance(r_entry_obj, dict)
|
||||
for r_entry in [cast(dict[str, object], r_entry_obj)]
|
||||
if r_entry.get("token_env")
|
||||
})
|
||||
next_idx = len(existing_slots)
|
||||
entry_typed["auth_scheme"] = str(cast(object, auth_typed.get("scheme")))
|
||||
entry_typed["token_env"] = f"EGRESS_TOKEN_{next_idx}"
|
||||
routes_typed.append(entry_typed)
|
||||
|
||||
return _render_routes_payload(cast(list[dict[str, object]], routes_typed))
|
||||
|
||||
|
||||
def add_route(slug: str, proposed_route_json: str) -> tuple[str, str]:
|
||||
try:
|
||||
proposed = json.loads(proposed_route_json)
|
||||
except json.JSONDecodeError as e:
|
||||
raise EgressApplyError(
|
||||
f"proposed route is not valid JSON: {e}"
|
||||
) from e
|
||||
if not isinstance(proposed, dict):
|
||||
raise EgressApplyError(
|
||||
"proposed route must be a JSON object"
|
||||
)
|
||||
current = fetch_current_routes(slug)
|
||||
merged = _merge_single_route(current, proposed)
|
||||
return apply_routes_change(slug, merged)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"EgressApplyError",
|
||||
"add_route",
|
||||
"apply_routes_change",
|
||||
"fetch_current_routes",
|
||||
"validate_routes_content",
|
||||
]
|
||||
|
||||
@@ -2,9 +2,8 @@
|
||||
act on them (approve / modify / reject).
|
||||
|
||||
Curses-based TUI; modify-then-approve shells out to $EDITOR. The
|
||||
approval handlers wire to the per-tool remediation engines:
|
||||
PRD 0014 (egress) writes routes.yaml + SIGHUPs egress; PRD 0016
|
||||
(capability) rebuilds the bottle Dockerfile.
|
||||
approval handler wires to PRD 0016 (capability-block), which rebuilds
|
||||
the bottle Dockerfile. The egress-block tool was removed in issue #198.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -26,7 +25,6 @@ from ..backend.docker.capability_apply import (
|
||||
CapabilityApplyError,
|
||||
apply_capability_change,
|
||||
)
|
||||
from ..backend.docker.egress_apply import EgressApplyError, add_route
|
||||
from ..log import Die, error, info
|
||||
from ..supervise import (
|
||||
COMPONENT_FOR_TOOL,
|
||||
@@ -37,7 +35,6 @@ from ..supervise import (
|
||||
STATUS_MODIFIED,
|
||||
STATUS_REJECTED,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_EGRESS_BLOCK,
|
||||
archive_proposal,
|
||||
list_pending_proposals,
|
||||
render_diff,
|
||||
@@ -61,7 +58,7 @@ class QueuedProposal:
|
||||
# Errors any remediation engine may raise. Caught by the TUI key
|
||||
# handlers and surfaced in the status line so a failed apply keeps
|
||||
# the proposal pending rather than crashing curses.
|
||||
ApplyError = (EgressApplyError, CapabilityApplyError)
|
||||
ApplyError = (CapabilityApplyError,)
|
||||
|
||||
|
||||
def discover_pending() -> list[QueuedProposal]:
|
||||
@@ -82,9 +79,7 @@ def discover_pending() -> list[QueuedProposal]:
|
||||
def _approval_status(qp: QueuedProposal, verb: str) -> str:
|
||||
"""Status-line text after a successful approval."""
|
||||
base = f"{verb} {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
||||
if qp.proposal.tool == TOOL_CAPABILITY_BLOCK:
|
||||
return f"{base}; resume: ./cli.py resume {qp.proposal.bottle_slug}"
|
||||
return base
|
||||
return f"{base}; resume: ./cli.py resume {qp.proposal.bottle_slug}"
|
||||
|
||||
|
||||
def _detail_lines(
|
||||
@@ -132,11 +127,7 @@ def approve(
|
||||
file_to_apply = final_file if final_file is not None else qp.proposal.proposed_file
|
||||
|
||||
diff_before, diff_after = "", ""
|
||||
if qp.proposal.tool == TOOL_EGRESS_BLOCK:
|
||||
diff_before, diff_after = add_route(
|
||||
qp.proposal.bottle_slug, file_to_apply,
|
||||
)
|
||||
elif qp.proposal.tool == TOOL_CAPABILITY_BLOCK:
|
||||
if qp.proposal.tool == TOOL_CAPABILITY_BLOCK:
|
||||
_meta = read_metadata(qp.proposal.bottle_slug)
|
||||
if _meta is not None and not _meta.compose_project:
|
||||
raise CapabilityApplyError(
|
||||
|
||||
@@ -48,11 +48,9 @@ from pathlib import Path
|
||||
SUPERVISE_HOSTNAME = "supervise"
|
||||
SUPERVISE_PORT = 9100
|
||||
|
||||
TOOL_EGRESS_BLOCK = "egress-block"
|
||||
TOOL_CAPABILITY_BLOCK = "capability-block"
|
||||
TOOL_LIST_EGRESS_ROUTES = "list-egress-routes"
|
||||
TOOLS: tuple[str, ...] = (
|
||||
TOOL_EGRESS_BLOCK,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_LIST_EGRESS_ROUTES,
|
||||
)
|
||||
@@ -70,10 +68,8 @@ 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.
|
||||
COMPONENT_FOR_TOOL: dict[str, str] = {
|
||||
TOOL_EGRESS_BLOCK: "egress",
|
||||
}
|
||||
# record laid down in PRD 0016. egress-block was removed in issue #198.
|
||||
COMPONENT_FOR_TOOL: dict[str, str] = {}
|
||||
|
||||
STATUS_APPROVED = "approved"
|
||||
STATUS_MODIFIED = "modified"
|
||||
@@ -555,7 +551,6 @@ __all__ = [
|
||||
"EGRESS_FORWARD_PROXY",
|
||||
"EGRESS_INTROSPECT_URL",
|
||||
"TOOL_CAPABILITY_BLOCK",
|
||||
"TOOL_EGRESS_BLOCK",
|
||||
"TOOL_LIST_EGRESS_ROUTES",
|
||||
"archive_proposal",
|
||||
"audit_dir",
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
"""Supervise sidecar HTTP server (PRD 0013).
|
||||
|
||||
Per-bottle MCP server exposing two tools — `egress-block`,
|
||||
`capability-block` — that the agent calls to propose config changes
|
||||
when stuck. Each tool call:
|
||||
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`.
|
||||
|
||||
Each queued tool call:
|
||||
|
||||
1. Validates the proposed file syntactically.
|
||||
2. Writes a Proposal to /run/supervise/queue/ (bind-mounted from
|
||||
@@ -133,69 +135,6 @@ def jsonrpc_error(request_id: object, code: int, message: str) -> bytes:
|
||||
|
||||
|
||||
TOOL_DEFINITIONS: list[dict[str, object]] = [
|
||||
{
|
||||
"name": _sv.TOOL_EGRESS_BLOCK,
|
||||
"description": (
|
||||
"Call when egress refused your HTTPS request — host "
|
||||
"without a matching route, or a request that did not match "
|
||||
"the route's matches rules (typically a 403 from the "
|
||||
"proxy). Propose a SINGLE route to add: the host you "
|
||||
"need + (optionally) a path_allowlist of path prefixes + "
|
||||
"(optionally) an auth block. The supervisor merges the "
|
||||
"route into the live table at approval time — you do NOT "
|
||||
"need to see or reproduce the existing routes. If the "
|
||||
"host already has a route, the proposed paths are unioned "
|
||||
"with the existing ones (host stays single-route). The "
|
||||
"operator approves or rejects in the supervise TUI. On "
|
||||
"approval the supervisor writes the merged routes.yaml "
|
||||
"and SIGHUPs egress (no dropped connections)."
|
||||
),
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"host": {
|
||||
"type": "string",
|
||||
"description": (
|
||||
"The hostname to allow (e.g. 'api.github.com'). "
|
||||
"Case-insensitive on match."
|
||||
),
|
||||
},
|
||||
"path_allowlist": {
|
||||
"type": "array",
|
||||
"items": {"type": "string"},
|
||||
"description": (
|
||||
"Optional URL path prefixes the route permits. "
|
||||
"Each must start with '/'. Omit to allow all "
|
||||
"paths under this host (bare-pass route). "
|
||||
"Internally converted to matches entries."
|
||||
),
|
||||
},
|
||||
"auth": {
|
||||
"type": "object",
|
||||
"description": (
|
||||
"Optional credential injection. {scheme, "
|
||||
"token_ref}: scheme is 'Bearer' or 'token'; "
|
||||
"token_ref names the host env var holding the "
|
||||
"secret value. Omit to add a host without "
|
||||
"credential injection. Ignored if the host "
|
||||
"already has a route (operator decides auth "
|
||||
"changes, not the agent)."
|
||||
),
|
||||
"properties": {
|
||||
"scheme": {"type": "string"},
|
||||
"token_ref": {"type": "string"},
|
||||
},
|
||||
"required": ["scheme", "token_ref"],
|
||||
"additionalProperties": False,
|
||||
},
|
||||
"justification": {
|
||||
"type": "string",
|
||||
"description": "Why this host needs to be allowed.",
|
||||
},
|
||||
},
|
||||
"required": ["host", "justification"],
|
||||
},
|
||||
},
|
||||
{
|
||||
"name": _sv.TOOL_LIST_EGRESS_ROUTES,
|
||||
"description": (
|
||||
@@ -254,11 +193,6 @@ PROPOSED_FILE_FIELD: dict[str, str] = {
|
||||
# --- Validation ------------------------------------------------------------
|
||||
|
||||
|
||||
# Auth schemes accepted on egress-block proposals — match the
|
||||
# manifest-side EGRESS_AUTH_SCHEMES.
|
||||
_AUTH_SCHEMES = ("Bearer", "token")
|
||||
|
||||
|
||||
def validate_proposed_file(tool: str, content: str) -> None:
|
||||
"""Syntactic validation. The operator is the real gate; this just
|
||||
catches obvious paste-errors / wrong-tool selections before they
|
||||
@@ -273,70 +207,6 @@ def validate_proposed_file(tool: str, content: str) -> None:
|
||||
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
|
||||
|
||||
|
||||
def _validate_and_bundle_egress_route(
|
||||
args: dict[str, object],
|
||||
) -> str:
|
||||
"""Validate egress-block input fields and bundle them into
|
||||
a JSON string that becomes the Proposal.proposed_file. Raises
|
||||
_RpcError on bad input — the agent retries with a fixed shape."""
|
||||
tool = _sv.TOOL_EGRESS_BLOCK
|
||||
host = args.get("host")
|
||||
if not isinstance(host, str) or not host.strip():
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: 'host' is required and must be a non-empty string",
|
||||
)
|
||||
payload: dict[str, object] = {"host": host}
|
||||
|
||||
path_allow_raw = args.get("path_allowlist")
|
||||
if path_allow_raw is not None:
|
||||
if not isinstance(path_allow_raw, list):
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: 'path_allowlist' must be an array of strings",
|
||||
)
|
||||
prefixes: list[str] = []
|
||||
for i, p in enumerate(path_allow_raw):
|
||||
if not isinstance(p, str):
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: path_allowlist[{i}] must be a string",
|
||||
)
|
||||
if not p.startswith("/"):
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: path_allowlist[{i}] {p!r} must start with '/'",
|
||||
)
|
||||
prefixes.append(p)
|
||||
if prefixes:
|
||||
payload["path_allowlist"] = prefixes
|
||||
|
||||
auth_raw = args.get("auth")
|
||||
if auth_raw is not None:
|
||||
if not isinstance(auth_raw, dict):
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: 'auth' must be an object with 'scheme' and 'token_ref'",
|
||||
)
|
||||
scheme = auth_raw.get("scheme")
|
||||
token_ref = auth_raw.get("token_ref")
|
||||
if not isinstance(scheme, str) or scheme not in _AUTH_SCHEMES:
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: auth.scheme must be one of "
|
||||
f"{', '.join(_AUTH_SCHEMES)} (got {scheme!r})",
|
||||
)
|
||||
if not isinstance(token_ref, str) or not token_ref:
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: auth.token_ref must be a non-empty string "
|
||||
f"naming the host env var holding the token",
|
||||
)
|
||||
payload["auth"] = {"scheme": scheme, "token_ref": token_ref}
|
||||
|
||||
return json.dumps(payload, indent=2) + "\n"
|
||||
|
||||
|
||||
# --- MCP handlers ----------------------------------------------------------
|
||||
|
||||
|
||||
@@ -422,13 +292,7 @@ def handle_tools_call(
|
||||
f"{name}: 'justification' is required and must be a non-empty string",
|
||||
)
|
||||
|
||||
if name == _sv.TOOL_EGRESS_BLOCK:
|
||||
# Structured input → JSON bundle on Proposal.proposed_file.
|
||||
# The dashboard's apply step (egress_apply.add_route)
|
||||
# parses this JSON, fetches the current routes, merges in
|
||||
# the new one, and writes the merged file.
|
||||
proposed_file = _validate_and_bundle_egress_route(args_raw)
|
||||
elif name in PROPOSED_FILE_FIELD:
|
||||
if name in PROPOSED_FILE_FIELD:
|
||||
file_field = PROPOSED_FILE_FIELD[name]
|
||||
proposed_file = args_raw.get(file_field)
|
||||
if not isinstance(proposed_file, str):
|
||||
|
||||
Reference in New Issue
Block a user