From 9cd583fbbb34135fa120710dd3b5f3e1f03174a3 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 15:13:44 -0400 Subject: [PATCH 01/18] feat(egress-proxy): retarget remediation at egress-proxy (PRD 0017 chunk 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finishes PRD 0017. The `cred-proxy-block` MCP tool is renamed and its remediation apply path is repointed at egress-proxy. - `claude_bottle/supervise.py` — `TOOL_CRED_PROXY_BLOCK` → `TOOL_EGRESS_PROXY_BLOCK`; `COMPONENT_FOR_TOOL` maps the new tool ID to `egress-proxy` for audit-log routing. - `claude_bottle/supervise_server.py` — tool definition renamed + description rewritten: "Call when egress-proxy refused your HTTPS request ... Read the current routes.yaml from /etc/ claude-bottle/current-config/routes.yaml, compose a modified version, pass the full new file plus a justification." The syntactic validator dispatches on the new tool ID. - `claude_bottle/backend/docker/egress_proxy_apply.py` — renamed from `cred_proxy_apply.py`. Reads routes.yaml from /etc/egress-proxy/routes.yaml via `docker exec cat`; validates via `egress_proxy_addon_core.load_routes` (so both sides use the same parser); writes via `docker cp`; SIGHUPs egress-proxy with `docker kill --signal HUP`. `EgressProxyApplyError` replaces `CredProxyApplyError`. - `claude_bottle/cli/dashboard.py` — wires the new apply + `discover_egress_proxy_slugs` helper; the operator-initiated `routes edit ` verb now writes to egress-proxy with `.yaml` suffix. Stale follow-up comment about path-aware filtering removed — PRD 0017 settled that question. - `tests/integration/test_supervise_sidecar.py` — restores the approval round-trip test (chunk 2 had switched it to a reject path because no cred-proxy existed). Approval stubs `apply_routes_change` so the test focuses on the supervise queue/response plumbing rather than docker-exec into a real egress-proxy sidecar (that's covered separately). - `tests/unit/test_egress_proxy_apply.py` — rewritten against the new validator; covers JSON shape, missing routes key, partial-auth-pair rejection (the addon-core parser catches these before SIGHUP). - PRDs 0010 + 0014 — status headers updated to Superseded / Retargeted with a callout block pointing at PRD 0017's migration section. Historical text preserved. 384 unit + integration tests pass. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/cred_proxy_apply.py | 133 ------------------ .../backend/docker/egress_proxy_apply.py | 116 +++++++++++++++ claude_bottle/cli/dashboard.py | 96 +++++++------ claude_bottle/supervise.py | 10 +- claude_bottle/supervise_server.py | 53 +++---- docs/prds/0010-cred-proxy.md | 10 +- .../prds/0014-cred-proxy-block-remediation.md | 14 +- tests/integration/test_supervise_sidecar.py | 55 +++++--- tests/unit/test_cred_proxy_apply.py | 39 ----- tests/unit/test_dashboard.py | 70 ++++----- tests/unit/test_dashboard_detail_lines.py | 6 +- tests/unit/test_egress_proxy_apply.py | 56 ++++++++ tests/unit/test_supervise.py | 14 +- tests/unit/test_supervise_server.py | 22 +-- 14 files changed, 361 insertions(+), 333 deletions(-) delete mode 100644 claude_bottle/backend/docker/cred_proxy_apply.py create mode 100644 claude_bottle/backend/docker/egress_proxy_apply.py delete mode 100644 tests/unit/test_cred_proxy_apply.py create mode 100644 tests/unit/test_egress_proxy_apply.py diff --git a/claude_bottle/backend/docker/cred_proxy_apply.py b/claude_bottle/backend/docker/cred_proxy_apply.py deleted file mode 100644 index 94324b9..0000000 --- a/claude_bottle/backend/docker/cred_proxy_apply.py +++ /dev/null @@ -1,133 +0,0 @@ -"""Host-side helper to apply a routes.json change to a running -cred-proxy sidecar (PRD 0014). - -Used by the supervise dashboard when the operator approves a -cred-proxy-block proposal (or runs the operator-initiated `routes -edit ` verb). Fetches the current routes.json via `docker -exec cat`, validates the new JSON, writes it into the sidecar via -`docker cp`, then `docker kill --signal HUP` to make the in-sidecar -SIGHUP handler (PRD 0014 Phase 1) reload without dropping -connections. - -Raises CredProxyApplyError on any failure — the dashboard surfaces -the message and keeps the proposal pending so the operator can -retry. -""" - -from __future__ import annotations - -import json -import os -import subprocess -import tempfile -from pathlib import Path - -# Constants inlined from the deleted `claude_bottle.backend.docker. -# cred_proxy` module (PRD 0017 chunk 2 cutover). Chunk 3 retargets -# this file at egress-proxy and gets rid of these. -CRED_PROXY_ROUTES_IN_CONTAINER = "/run/cred-proxy/routes.json" - - -def _cred_proxy_container_name(slug: str) -> str: - return f"claude-bottle-cred-proxy-{slug}" - - -class CredProxyApplyError(RuntimeError): - """Raised when fetch / apply fails. Caller renders to the - operator; does not crash the dashboard. - - PRD 0017 chunk 2 deletes the cred-proxy sidecar; this module's - docker-exec calls now hit a non-existent container and raise - CredProxyApplyError with a "container not running" message, - which the dashboard surfaces to the operator. Chunk 3 retargets - everything at egress-proxy.""" - - -def fetch_current_routes(slug: str) -> str: - """Read the live routes.json from the running cred-proxy sidecar - for `slug`. Returns the file content as a string. Raises - CredProxyApplyError if the sidecar isn't reachable or the read - fails.""" - container = _cred_proxy_container_name(slug) - r = subprocess.run( - ["docker", "exec", container, "cat", CRED_PROXY_ROUTES_IN_CONTAINER], - capture_output=True, text=True, check=False, - ) - if r.returncode != 0: - raise CredProxyApplyError( - f"could not read routes.json from {container}: " - f"{(r.stderr or '').strip() or 'container not running?'}" - ) - return r.stdout - - -def validate_routes_json(content: str) -> None: - """Syntactic check before SIGHUP — the sidecar's reload also - validates, but failing here keeps the old routes live and gives - the operator a clearer error than 'reload failed' in the - sidecar logs.""" - try: - parsed = json.loads(content) - except json.JSONDecodeError as e: - raise CredProxyApplyError( - f"proposed routes.json is not valid JSON: {e}" - ) from e - if not isinstance(parsed, dict) or not isinstance(parsed.get("routes"), list): - raise CredProxyApplyError( - "proposed routes.json must be an object with a 'routes' array" - ) - - -def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]: - """Apply `new_content` to the cred-proxy sidecar for `slug`: - 1. Fetch current routes.json (for the before-diff). - 2. Validate the new JSON. - 3. Write to a temp file, `docker cp` into the sidecar. - 4. `docker kill --signal HUP` so cred-proxy reloads. - - Returns (before, after) where `after` == `new_content`. Raises - CredProxyApplyError on any step; the existing routes in the - sidecar are unchanged if the failure is before docker cp, and - are reverted in spirit if SIGHUP fails (cp landed but reload - didn't fire — caller's next attempt will SIGHUP again).""" - container = _cred_proxy_container_name(slug) - before = fetch_current_routes(slug) - validate_routes_json(new_content) - - fd, tmp_path = tempfile.mkstemp(prefix="cb-routes.", suffix=".json") - try: - with os.fdopen(fd, "w") as f: - f.write(new_content) - cp = subprocess.run( - ["docker", "cp", tmp_path, f"{container}:{CRED_PROXY_ROUTES_IN_CONTAINER}"], - capture_output=True, text=True, check=False, - ) - if cp.returncode != 0: - raise CredProxyApplyError( - f"failed to copy routes.json into {container}: " - f"{(cp.stderr or '').strip()}" - ) - sig = subprocess.run( - ["docker", "kill", "--signal", "HUP", container], - capture_output=True, text=True, check=False, - ) - if sig.returncode != 0: - raise CredProxyApplyError( - f"failed to SIGHUP {container}: " - f"{(sig.stderr or '').strip()}" - ) - finally: - try: - Path(tmp_path).unlink() - except OSError: - pass - - return before, new_content - - -__all__ = [ - "CredProxyApplyError", - "apply_routes_change", - "fetch_current_routes", - "validate_routes_json", -] diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py new file mode 100644 index 0000000..6fe8b56 --- /dev/null +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -0,0 +1,116 @@ +"""Host-side helper to apply a routes.yaml change to a running +egress-proxy sidecar (PRD 0014 retargeted by PRD 0017 chunk 3). + +Used by the supervise dashboard when the operator approves an +egress-proxy-block proposal (or runs the operator-initiated +`routes edit ` verb). Fetches the current routes.yaml via +`docker exec cat`, validates the new content, writes it into the +sidecar via `docker cp`, then `docker kill --signal HUP` to make +the addon reload without dropping connections. + +Raises EgressProxyApplyError on any failure — the dashboard +surfaces the message and keeps the proposal pending so the +operator can retry. +""" + +from __future__ import annotations + +import os +import subprocess +import tempfile +from pathlib import Path + +from ...egress_proxy import EGRESS_PROXY_ROUTES_IN_CONTAINER +from ...egress_proxy_addon_core import load_routes +from .egress_proxy import egress_proxy_container_name + + +class EgressProxyApplyError(RuntimeError): + """Raised when fetch / apply fails. Caller renders to the + operator; does not crash the dashboard.""" + + +def fetch_current_routes(slug: str) -> str: + """Read the live routes.yaml from the running egress-proxy sidecar + for `slug`. Returns the file content as a string. Raises + EgressProxyApplyError if the sidecar isn't reachable or the read + fails.""" + container = egress_proxy_container_name(slug) + r = subprocess.run( + ["docker", "exec", container, "cat", EGRESS_PROXY_ROUTES_IN_CONTAINER], + capture_output=True, text=True, check=False, + ) + if r.returncode != 0: + raise EgressProxyApplyError( + f"could not read routes.yaml from {container}: " + f"{(r.stderr or '').strip() or 'container not running?'}" + ) + return r.stdout + + +def validate_routes_content(content: str) -> None: + """Syntactic check before SIGHUP — the addon's reload also + validates, but failing here keeps the old routes live and gives + the operator a clearer error than the addon's stderr line.""" + try: + load_routes(content) + except ValueError as e: + raise EgressProxyApplyError( + f"proposed routes.yaml is not valid: {e}" + ) from e + + +def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]: + """Apply `new_content` to the egress-proxy sidecar for `slug`: + 1. Fetch current routes.yaml (for the before-diff). + 2. Validate the new content via the addon's own parser. + 3. Write to a temp file, `docker cp` into the sidecar. + 4. `docker kill --signal HUP` so the addon reloads. + + Returns (before, after) where `after` == `new_content`. Raises + EgressProxyApplyError on any step; the existing routes in the + sidecar are unchanged if the failure is before docker cp, and + are reverted in spirit if SIGHUP fails (cp landed but reload + didn't fire — caller's next attempt will SIGHUP again).""" + container = egress_proxy_container_name(slug) + before = fetch_current_routes(slug) + validate_routes_content(new_content) + + fd, tmp_path = tempfile.mkstemp(prefix="cb-routes.", suffix=".yaml") + try: + with os.fdopen(fd, "w") as f: + f.write(new_content) + cp = subprocess.run( + ["docker", "cp", tmp_path, + f"{container}:{EGRESS_PROXY_ROUTES_IN_CONTAINER}"], + capture_output=True, text=True, check=False, + ) + if cp.returncode != 0: + raise EgressProxyApplyError( + f"failed to copy routes.yaml into {container}: " + f"{(cp.stderr or '').strip()}" + ) + sig = subprocess.run( + ["docker", "kill", "--signal", "HUP", container], + capture_output=True, text=True, check=False, + ) + if sig.returncode != 0: + raise EgressProxyApplyError( + f"failed to SIGHUP {container}: " + f"{(sig.stderr or '').strip()}" + ) + finally: + try: + Path(tmp_path).unlink() + except OSError: + pass + + return before, new_content + + +__all__ = [ + "EgressProxyApplyError", + "apply_routes_change", + "fetch_current_routes", + "validate_routes_content", +] diff --git a/claude_bottle/cli/dashboard.py b/claude_bottle/cli/dashboard.py index de8c154..f41db21 100644 --- a/claude_bottle/cli/dashboard.py +++ b/claude_bottle/cli/dashboard.py @@ -1,12 +1,12 @@ """dashboard: list pending supervise proposals across all bottles and act on them (approve / modify / reject). PRD 0013 v1. -Curses-based TUI; modify-then-approve shells out to $EDITOR. For -0013 the approval handlers are no-ops on the supervisor side: the -response file is written (and the sidecar returns it to the agent), -and an audit entry is appended, but no host-side config change runs. -PRDs 0014 (cred-proxy) and 0015 (pipelock) wire in the actual -writes. +Curses-based TUI; modify-then-approve shells out to $EDITOR. The +approval handlers wire to the per-tool remediation engines: +PRD 0014 (egress-proxy, retargeted from cred-proxy in PRD 0017 +chunk 3) writes routes.yaml + SIGHUPs egress-proxy; PRD 0015 +(pipelock) writes the allowlist + restarts pipelock; PRD 0016 +(capability) rebuilds the bottle Dockerfile. """ from __future__ import annotations @@ -27,8 +27,8 @@ from ..backend.docker.capability_apply import ( CapabilityApplyError, apply_capability_change, ) -from ..backend.docker.cred_proxy_apply import ( - CredProxyApplyError, +from ..backend.docker.egress_proxy_apply import ( + EgressProxyApplyError, apply_routes_change, fetch_current_routes, ) @@ -50,7 +50,7 @@ from ..supervise import ( STATUS_MODIFIED, STATUS_REJECTED, TOOL_CAPABILITY_BLOCK, - TOOL_CRED_PROXY_BLOCK, + TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, archive_proposal, list_pending_proposals, @@ -64,7 +64,7 @@ from ._common import PROG # 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 = (CredProxyApplyError, PipelockApplyError, CapabilityApplyError) +ApplyError = (EgressProxyApplyError, PipelockApplyError, CapabilityApplyError) # --- Discovery ------------------------------------------------------------- @@ -103,10 +103,10 @@ def _discover_sidecar_slugs(name_prefix: str) -> list[str]: return sorted(out) -def discover_cred_proxy_slugs() -> list[str]: - """Slugs of bottles with a running cred-proxy sidecar. Used by +def discover_egress_proxy_slugs() -> list[str]: + """Slugs of bottles with a running egress-proxy sidecar. Used by the operator-initiated `routes edit` verb.""" - return _discover_sidecar_slugs("claude-bottle-cred-proxy-") + return _discover_sidecar_slugs("claude-bottle-egress-proxy-") def discover_pipelock_slugs() -> list[str]: @@ -156,16 +156,16 @@ def approve( entry. If `final_file` is provided the status is `modified`; otherwise `approved`. - Raises CredProxyApplyError if the cred-proxy-block apply fails - (sidecar down, invalid JSON survived the operator's modify). - On failure no response is written and no audit entry is - appended — the proposal stays pending so the operator can fix - the input and retry.""" + Raises EgressProxyApplyError if the egress-proxy-block apply + fails (sidecar down, invalid routes content survived the + operator's modify). On failure no response is written and no + audit entry is appended — the proposal stays pending so the + operator can fix the input and retry.""" status = STATUS_MODIFIED if final_file is not None else STATUS_APPROVED 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_CRED_PROXY_BLOCK: + if qp.proposal.tool == TOOL_EGRESS_PROXY_BLOCK: diff_before, diff_after = apply_routes_change( qp.proposal.bottle_slug, file_to_apply, ) @@ -212,22 +212,22 @@ def reject(qp: QueuedProposal, *, reason: str) -> None: def operator_edit_routes(slug: str, new_content: str) -> tuple[str, str]: - """Apply an operator-initiated routes.json change (no agent + """Apply an operator-initiated routes.yaml change (no agent proposal). Used by the `routes edit ` TUI verb and available for scripted use. Returns (before, after) like apply_routes_change. Writes an audit entry tagged ACTION_OPERATOR_EDIT to distinguish from tool-call approvals. - Raises CredProxyApplyError on failure.""" + Raises EgressProxyApplyError on failure.""" before, after = apply_routes_change(slug, new_content) write_audit_entry(AuditEntry( timestamp=datetime.now(timezone.utc).isoformat(), bottle_slug=slug, - component="cred-proxy", + component="egress-proxy", operator_action=ACTION_OPERATOR_EDIT, operator_notes="", justification="", - diff=render_diff(before, after, label="cred-proxy"), + diff=render_diff(before, after, label="egress-proxy"), )) return before, after @@ -239,20 +239,19 @@ def _apply_pipelock_url(slug: str, failed_url: str) -> tuple[str, str]: The full URL (with path) is preserved on the proposal for the operator's read; only the host ends up in pipelock's allowlist. - FOLLOW-UP — path-aware filtering. Pipelock 2.3.0's api_allowlist - is hostname-only (verified by inspecting the binary's strict - preset; the only "path" fields in pipelock's schema are about - local filesystem paths under sandbox / file_sentry / taint). So - approving pipelock-block opens the entire host, not the URL's - path. If/when per-path enforcement becomes load-bearing, the - follow-up is most likely adding an `auth_scheme: none` mode + - `path_allowlist` field to cred-proxy (which already does - path-prefix routing) and rewiring pipelock-block to propose - cred-proxy routes instead of pipelock hostnames. That's a - multi-touch change deserving its own PRD — out of scope for the - supervise-loop work that introduced this function. See PR - discussion on https://gitea.dideric.is/didericis/claude-bottle/pulls/25 - for the design conversation.""" + Pipelock 2.3.0's api_allowlist is hostname-only (verified by + inspecting the binary's strict preset; the only "path" fields in + pipelock's schema are about local filesystem paths under sandbox + / file_sentry / taint). Approving pipelock-block opens the + entire host, not the URL's path. + + Path-level enforcement was the open question this function's + earlier docstring flagged; PRD 0017 answered it by putting + egress-proxy in front of pipelock. The agent's + `egress-proxy-block` tool now proposes routes.yaml changes that + can include a `path_allowlist`. Use that tool for path-level + follow-ups; this one stays hostname-only because pipelock is + still the last hostname gate before egress.""" import urllib.parse parsed = urllib.parse.urlsplit(failed_url.strip()) host = parsed.hostname or "" @@ -296,14 +295,13 @@ def _write_audit( diff_before: str, diff_after: str, ) -> None: - """Audit log for cred-proxy / pipelock tools. capability-block has - no audit log (its changes are captured by the bottle's rebuild - record + git history per PRD 0016). + """Audit log for egress-proxy / pipelock tools. capability-block + has no audit log (its changes are captured by the bottle's + rebuild record + git history per PRD 0016). - For cred-proxy-block approvals the (before, after) come from the - apply_routes_change return — a real fetched-from-sidecar diff. - For rejections, or for tools whose remediation hasn't landed yet - (pipelock in 0014, capability anywhere), both are empty strings + For egress-proxy-block + pipelock-block approvals the (before, + after) come from the apply_*_change return — a real + fetched-from-sidecar diff. For rejections both are empty strings and the audit diff renders as empty.""" component = COMPONENT_FOR_TOOL.get(qp.proposal.tool) if component is None: @@ -683,22 +681,22 @@ def _modify(stdscr: "curses._CursesWindow", qp: QueuedProposal) -> str | None: def _suffix_for_tool(tool: str) -> str: if tool == TOOL_CAPABILITY_BLOCK: return ".dockerfile" - # cred-proxy-block / pipelock-block: JSON-ish + plain. + # egress-proxy-block / pipelock-block: JSON-ish + plain. return ".txt" def _operator_edit_routes_flow(stdscr: "curses._CursesWindow") -> str: - """Operator-initiated routes.json edit. Discover running - cred-proxy sidecars, pick one (single → use directly; multi → + """Operator-initiated routes.yaml edit. Discover running + egress-proxy sidecars, pick one (single → use directly; multi → prompt), fetch the current routes, open in $EDITOR, apply on save. Returns a status-line message.""" return _operator_edit_flow( stdscr, label="routes", - discover=discover_cred_proxy_slugs, + discover=discover_egress_proxy_slugs, fetch=fetch_current_routes, apply=operator_edit_routes, - suffix=".json", + suffix=".yaml", ) diff --git a/claude_bottle/supervise.py b/claude_bottle/supervise.py index cb07241..08a05ab 100644 --- a/claude_bottle/supervise.py +++ b/claude_bottle/supervise.py @@ -5,7 +5,7 @@ queue/audit support. The sidecar (claude_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: - * cred-proxy-block — agent proposes a new routes.json + * egress-proxy-block — agent proposes a new routes.yaml * pipelock-block — agent proposes a new pipelock allowlist * capability-block — agent proposes a new agent Dockerfile @@ -49,11 +49,11 @@ from pathlib import Path SUPERVISE_HOSTNAME = "supervise" SUPERVISE_PORT = 9100 -TOOL_CRED_PROXY_BLOCK = "cred-proxy-block" +TOOL_EGRESS_PROXY_BLOCK = "egress-proxy-block" TOOL_PIPELOCK_BLOCK = "pipelock-block" TOOL_CAPABILITY_BLOCK = "capability-block" TOOLS: tuple[str, ...] = ( - TOOL_CRED_PROXY_BLOCK, + TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, TOOL_CAPABILITY_BLOCK, ) @@ -63,7 +63,7 @@ TOOLS: tuple[str, ...] = ( # here — those changes are captured by git history + the rebuild # record laid down in PRD 0016. COMPONENT_FOR_TOOL: dict[str, str] = { - TOOL_CRED_PROXY_BLOCK: "cred-proxy", + TOOL_EGRESS_PROXY_BLOCK: "egress-proxy", TOOL_PIPELOCK_BLOCK: "pipelock", } @@ -566,7 +566,7 @@ __all__ = [ "SupervisePlan", "TOOLS", "TOOL_CAPABILITY_BLOCK", - "TOOL_CRED_PROXY_BLOCK", + "TOOL_EGRESS_PROXY_BLOCK", "TOOL_PIPELOCK_BLOCK", "archive_proposal", "audit_dir", diff --git a/claude_bottle/supervise_server.py b/claude_bottle/supervise_server.py index e98b70a..1517d4c 100644 --- a/claude_bottle/supervise_server.py +++ b/claude_bottle/supervise_server.py @@ -1,6 +1,6 @@ """Supervise sidecar HTTP server (PRD 0013). -Per-bottle MCP server exposing three tools — `cred-proxy-block`, +Per-bottle MCP server exposing three tools — `egress-proxy-block`, `pipelock-block`, `capability-block` — that the agent calls to propose config changes when stuck. Each tool call: @@ -128,26 +128,27 @@ def jsonrpc_error(request_id: object, code: int, message: str) -> bytes: TOOL_DEFINITIONS: list[dict[str, object]] = [ { - "name": _sv.TOOL_CRED_PROXY_BLOCK, + "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "description": ( - "Call when cred-proxy refused your HTTPS request — missing " - "route, expired token, wrong scope (typically a 403 or a " - "404 from `http://cred-proxy://`). Read the " - "current routes.json from " - "/etc/claude-bottle/current-config/routes.json, compose a " - "modified version with the route you need, and pass the " - "full new file plus a justification. The operator approves " - "or rejects in the supervise TUI. On approval the supervisor " - "writes the new routes.json on the host and SIGHUPs cred-proxy " - "(wired in PRD 0014; in the v1 supervise foundation the " - "approval is acknowledged but no config change runs)." + "Call when egress-proxy refused your HTTPS request — host " + "without a matching route, or a path outside the route's " + "path_allowlist (typically a 403 from the proxy). Read " + "the current routes.yaml from " + "/etc/claude-bottle/current-config/routes.yaml, compose " + "a modified version that adds or relaxes the route you " + "need, and pass the full new file plus a justification. " + "The operator approves or rejects in the supervise TUI. " + "On approval the supervisor writes the new routes.yaml " + "on the host and SIGHUPs egress-proxy (the addon's reload " + "swaps the route table atomically without dropping " + "in-flight connections)." ), "inputSchema": { "type": "object", "properties": { "routes": { "type": "string", - "description": "Full proposed routes.json file content (JSON text).", + "description": "Full proposed routes.yaml file content (JSON text — every JSON document is valid YAML).", }, "justification": { "type": "string", @@ -226,15 +227,15 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ # tool-specific payload (stored in Proposal.proposed_file as # free-form text the apply path interprets per tool). # -# cred-proxy-block: full proposed routes.json -# pipelock-block: the full failed URL (scheme + host + path) — -# supervisor extracts the host, merges into the -# bottle's current allowlist; the path is shown -# to the operator for context (pipelock doesn't -# do path-level matching). -# capability-block: full proposed Dockerfile +# egress-proxy-block: full proposed routes.yaml +# pipelock-block: the full failed URL (scheme + host + path) — +# supervisor extracts the host, merges into the +# bottle's current allowlist; the path is shown +# to the operator for context (pipelock doesn't +# do path-level matching). +# capability-block: full proposed Dockerfile PROPOSED_FILE_FIELD: dict[str, str] = { - _sv.TOOL_CRED_PROXY_BLOCK: "routes", + _sv.TOOL_EGRESS_PROXY_BLOCK: "routes", _sv.TOOL_PIPELOCK_BLOCK: "failed_url", _sv.TOOL_CAPABILITY_BLOCK: "dockerfile", } @@ -249,18 +250,18 @@ def validate_proposed_file(tool: str, content: str) -> None: enter the queue.""" if not content.strip(): raise _RpcError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty") - if tool == _sv.TOOL_CRED_PROXY_BLOCK: + if tool == _sv.TOOL_EGRESS_PROXY_BLOCK: try: parsed = json.loads(content) except json.JSONDecodeError as e: raise _RpcError( ERR_INVALID_PARAMS, - f"{tool}: proposed routes.json is not valid JSON: {e}", + f"{tool}: proposed routes.yaml is not valid JSON: {e}", ) from e if not isinstance(parsed, dict) or not isinstance(parsed.get("routes"), list): raise _RpcError( ERR_INVALID_PARAMS, - f"{tool}: proposed routes.json must be an object with a 'routes' array", + f"{tool}: proposed routes.yaml must be an object with a 'routes' array", ) elif tool == _sv.TOOL_PIPELOCK_BLOCK: # `content` is the full failed URL. Require scheme + host so @@ -505,7 +506,7 @@ def serve( def main(argv: list[str]) -> int: - del argv # config is env-only, matches cred_proxy_server pattern + del argv # config is env-only, no CLI flags bottle_slug = os.environ.get("SUPERVISE_BOTTLE_SLUG", "") if not bottle_slug: sys.stderr.write("supervise: SUPERVISE_BOTTLE_SLUG env is unset\n") diff --git a/docs/prds/0010-cred-proxy.md b/docs/prds/0010-cred-proxy.md index b375130..7716519 100644 --- a/docs/prds/0010-cred-proxy.md +++ b/docs/prds/0010-cred-proxy.md @@ -1,8 +1,16 @@ # PRD 0010: Credential proxy for agent-bound API tokens -- **Status:** Draft +- **Status:** Superseded by [PRD 0017](0017-egress-proxy-via-mitmproxy.md) - **Author:** didericis - **Created:** 2026-05-13 +- **Superseded:** 2026-05-25 + +> **Historical reference only.** The cred-proxy sidecar this PRD +> describes was replaced by the egress-proxy sidecar (PRD 0017) in +> a hard cutover. The auth-injection role moved over largely intact; +> path-prefix routing is replaced by universal MITM at the agent's +> HTTP_PROXY. See PRD 0017's "Migration — hard cutover" section for +> the field-by-field manifest rename. ## Summary diff --git a/docs/prds/0014-cred-proxy-block-remediation.md b/docs/prds/0014-cred-proxy-block-remediation.md index 13dd221..bc419e9 100644 --- a/docs/prds/0014-cred-proxy-block-remediation.md +++ b/docs/prds/0014-cred-proxy-block-remediation.md @@ -1,11 +1,23 @@ # PRD 0014: cred-proxy block remediation -- **Status:** Draft +- **Status:** Retargeted by [PRD 0017](0017-egress-proxy-via-mitmproxy.md) - **Author:** didericis - **Created:** 2026-05-25 +- **Retargeted:** 2026-05-25 - **Parent:** PRD 0012 - **Depends on:** PRD 0013 +> **Retarget notice.** The remediation shape this PRD describes (MCP +> tool → operator approve → SIGHUP a sidecar) is intact, but the +> sidecar moved: cred-proxy is gone, replaced by egress-proxy under +> PRD 0017. The MCP tool is now named `egress-proxy-block`; the +> proposed file is `routes.yaml` (JSON content) in egress-proxy's +> route shape (host + path_allowlist + nested `auth` block); the +> apply path docker-cps + SIGHUPs egress-proxy. The audit-log +> component label changed from `cred-proxy` to `egress-proxy`. +> Operator-initiated `routes edit ` still exists with the +> same UX, now pointed at the egress-proxy sidecar. + ## Summary Wires the **cred-proxy block** path (PRD 0012 *Stuck categories*) end-to-end. cred-proxy gains SIGHUP-based hot reload of `routes.json`. The supervisor, on approval of a `cred-proxy-block` proposal, writes the new `routes.json` to the host and SIGHUPs cred-proxy — no restart, no dropped connections. The TUI gains a proactive `routes edit ` verb for operator-initiated edits unrelated to a tool call. The cred-proxy audit log (format defined in PRD 0013) is filled in with real entries on every edit. diff --git a/tests/integration/test_supervise_sidecar.py b/tests/integration/test_supervise_sidecar.py index 093c347..b4492c3 100644 --- a/tests/integration/test_supervise_sidecar.py +++ b/tests/integration/test_supervise_sidecar.py @@ -196,7 +196,7 @@ class TestSuperviseSidecar(unittest.TestCase): names = {t["name"] for t in result["result"]["tools"]} self.assertEqual( { - _sv.TOOL_CRED_PROXY_BLOCK, + _sv.TOOL_EGRESS_PROXY_BLOCK, _sv.TOOL_PIPELOCK_BLOCK, _sv.TOOL_CAPABILITY_BLOCK, }, @@ -204,28 +204,38 @@ class TestSuperviseSidecar(unittest.TestCase): ) def test_tools_call_round_trips_through_queue(self): - """End-to-end: agent in the bottle calls cred-proxy-block; - the call blocks on the queue; the host rejects via the - dashboard helpers; the agent receives the rejection. + """End-to-end: agent in the bottle calls egress-proxy-block; + the call blocks on the queue; the host approves via the + dashboard helpers; the agent receives the approval. - PRD 0017 chunk 2 deleted the cred-proxy sidecar, so the - approval-apply path on cred-proxy-block is broken in this - intermediate state (chunk 3 retargets it at egress-proxy and - restores the round-trip approval test). For now this verifies - only the queue + response leg by exercising the reject path - — no docker-exec into a sidecar needed.""" + This test focuses on the supervise sidecar's queue + response + plumbing, not the egress-proxy apply path itself. The apply + function is stubbed so we don't need to bring up a real + egress-proxy sidecar (its docker lifecycle has its own + integration coverage).""" self._require_bind_mount_sharing() self._bring_up_sidecar() + # Stub the apply step. The dashboard's approve() calls + # apply_routes_change to docker-exec into the egress-proxy + # sidecar; this test isn't exercising the real sidecar, so + # patch it to a no-op that returns plausible before/after + # strings the audit-log writer can render. + from claude_bottle.cli import dashboard as _dash + original_apply = _dash.apply_routes_change + _dash.apply_routes_change = ( + lambda slug, new: ("(stubbed before)", new) + ) + captured: dict[str, object] = {} def caller() -> None: captured["response"] = self._curl_jsonrpc({ "jsonrpc": "2.0", "id": 7, "method": "tools/call", "params": { - "name": _sv.TOOL_CRED_PROXY_BLOCK, + "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { - "routes": '{"routes": [{"path": "/x/"}]}', + "routes": '{"routes": [{"host": "api.example.com"}]}', "justification": "integration test", }, }, @@ -249,16 +259,17 @@ class TestSuperviseSidecar(unittest.TestCase): self.assertIsNotNone(qp, "proposal never appeared in queue") assert qp is not None # type-narrowing self.assertEqual( - _sv.TOOL_CRED_PROXY_BLOCK, qp.proposal.tool, + _sv.TOOL_EGRESS_PROXY_BLOCK, qp.proposal.tool, ) self.assertEqual("integration test", qp.proposal.justification) - # Reject via the dashboard helper. The reject path skips - # the sidecar-apply step, so it works without a real - # cred-proxy sidecar (which doesn't exist in chunk 2's - # transitional state). - dashboard.reject(qp, reason="no real cred-proxy in chunk 2") + # Approve via the dashboard helper. The apply step (now + # stubbed) would docker-exec into the egress-proxy sidecar + # and SIGHUP it. The supervise sidecar sees the response + # file and returns to the curl caller. + dashboard.approve(qp, notes="lgtm from integration test") finally: + _dash.apply_routes_change = original_apply t.join(timeout=20) response = captured.get("response") @@ -267,12 +278,10 @@ class TestSuperviseSidecar(unittest.TestCase): self.assertEqual(7, response["id"]) result = response["result"] assert isinstance(result, dict) - # Rejected tool calls surface as MCP errors so the agent - # treats them as failures (not silent successes). - self.assertTrue(result.get("isError")) + self.assertFalse(result.get("isError")) text = result["content"][0]["text"] - self.assertIn("rejected", text) - self.assertIn("no real cred-proxy", text) + self.assertIn("status: approved", text) + self.assertIn("notes: lgtm from integration test", text) def test_orphan_sidecar_name_collision_recovered(self): """An orphan supervise sidecar from a previous run blocks diff --git a/tests/unit/test_cred_proxy_apply.py b/tests/unit/test_cred_proxy_apply.py deleted file mode 100644 index b0877d2..0000000 --- a/tests/unit/test_cred_proxy_apply.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Unit: validate_routes_json (PRD 0014 Phase 2). docker exec / cp / -kill paths are covered by the integration test.""" - -import unittest - -from claude_bottle.backend.docker.cred_proxy_apply import ( - CredProxyApplyError, - validate_routes_json, -) - - -class TestValidateRoutesJson(unittest.TestCase): - def test_accepts_routes_array(self): - validate_routes_json('{"routes": []}') - validate_routes_json( - '{"routes": [{"path": "/a/", "upstream": "https://example.com",' - ' "auth_scheme": "Bearer", "token_env": "T0"}]}' - ) - - def test_rejects_bad_json(self): - with self.assertRaises(CredProxyApplyError) as cm: - validate_routes_json("{not json") - self.assertIn("not valid JSON", str(cm.exception)) - - def test_rejects_non_object_top_level(self): - with self.assertRaises(CredProxyApplyError): - validate_routes_json("[]") - - def test_rejects_missing_routes_key(self): - with self.assertRaises(CredProxyApplyError): - validate_routes_json('{"other": []}') - - def test_rejects_non_list_routes(self): - with self.assertRaises(CredProxyApplyError): - validate_routes_json('{"routes": "not a list"}') - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/unit/test_dashboard.py b/tests/unit/test_dashboard.py index 3d0ce60..80e8c24 100644 --- a/tests/unit/test_dashboard.py +++ b/tests/unit/test_dashboard.py @@ -17,7 +17,7 @@ from pathlib import Path from claude_bottle import supervise from claude_bottle.backend.docker.capability_apply import CapabilityApplyError -from claude_bottle.backend.docker.cred_proxy_apply import CredProxyApplyError +from claude_bottle.backend.docker.egress_proxy_apply import EgressProxyApplyError from claude_bottle.backend.docker.pipelock_apply import PipelockApplyError from claude_bottle.cli import dashboard from claude_bottle.supervise import ( @@ -26,7 +26,7 @@ from claude_bottle.supervise import ( STATUS_MODIFIED, STATUS_REJECTED, TOOL_CAPABILITY_BLOCK, - TOOL_CRED_PROXY_BLOCK, + TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, read_audit_entries, read_response, @@ -37,13 +37,13 @@ from claude_bottle.supervise import ( FIXED = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc) -def _proposal(slug: str = "dev", tool: str = TOOL_CRED_PROXY_BLOCK) -> Proposal: +def _proposal(slug: str = "dev", tool: str = TOOL_EGRESS_PROXY_BLOCK) -> Proposal: # Per-tool payload shape: cred-proxy gets routes.json, pipelock # gets a failed URL (PR #25 follow-up), capability gets a # Dockerfile-ish blob. Match the production dispatch in # PROPOSED_FILE_FIELD. payloads = { - TOOL_CRED_PROXY_BLOCK: '{"routes": []}\n', + TOOL_EGRESS_PROXY_BLOCK: '{"routes": []}\n', TOOL_PIPELOCK_BLOCK: "https://example.com/path", TOOL_CAPABILITY_BLOCK: "FROM python:3.13\n", } @@ -95,13 +95,13 @@ class TestDiscoverPending(_FakeHomeMixin, unittest.TestCase): def test_sorted_by_arrival_across_bottles(self): early = Proposal.new( - bottle_slug="api", tool=TOOL_CRED_PROXY_BLOCK, + bottle_slug="api", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file="{}", justification="early", current_file_hash="h", now=datetime(2026, 5, 25, 10, 0, 0, tzinfo=timezone.utc), ) late = Proposal.new( - bottle_slug="dev", tool=TOOL_CRED_PROXY_BLOCK, + bottle_slug="dev", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file="{}", justification="late", current_file_hash="h", now=datetime(2026, 5, 25, 14, 0, 0, tzinfo=timezone.utc), @@ -151,7 +151,7 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): dashboard.apply_capability_change = self._original_apply_capability self._teardown_fake_home() - def _enqueue(self, tool: str = TOOL_CRED_PROXY_BLOCK): + def _enqueue(self, tool: str = TOOL_EGRESS_PROXY_BLOCK): p = _proposal(tool=tool) qdir = supervise.queue_dir_for_slug("dev") qdir.mkdir(parents=True, exist_ok=True) @@ -164,7 +164,7 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): resp = read_response(qp.queue_dir, qp.proposal.id) self.assertEqual(STATUS_APPROVED, resp.status) self.assertIsNone(resp.final_file) - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual(1, len(entries)) self.assertEqual("approved", entries[0].operator_action) @@ -175,7 +175,7 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): self.assertEqual(STATUS_MODIFIED, resp.status) self.assertEqual('{"routes": [{"path": "/x/"}]}\n', resp.final_file) self.assertEqual("tweaked", resp.notes) - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual("modified", entries[0].operator_action) def test_reject_writes_rejection(self): @@ -184,7 +184,7 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): resp = read_response(qp.queue_dir, qp.proposal.id) self.assertEqual(STATUS_REJECTED, resp.status) self.assertEqual("nope", resp.notes) - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual("rejected", entries[0].operator_action) self.assertEqual("nope", entries[0].operator_notes) @@ -193,18 +193,18 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): dashboard.approve(qp) # No audit log for capability-block (per PRD 0013 / 0016). # cred-proxy and pipelock logs both empty. - self.assertEqual([], read_audit_entries("cred-proxy", "dev")) + self.assertEqual([], read_audit_entries("egress-proxy", "dev")) self.assertEqual([], read_audit_entries("pipelock", "dev")) - def test_pipelock_audit_distinct_from_cred_proxy(self): + def test_pipelock_audit_distinct_from_egress_proxy(self): qp = self._enqueue(tool=TOOL_PIPELOCK_BLOCK) dashboard.approve(qp) self.assertEqual(1, len(read_audit_entries("pipelock", "dev"))) - self.assertEqual(0, len(read_audit_entries("cred-proxy", "dev"))) + self.assertEqual(0, len(read_audit_entries("egress-proxy", "dev"))) -class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): - """PRD 0014 Phase 3: approve() on a cred-proxy-block proposal +class TestEgressProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): + """PRD 0014 Phase 3: approve() on a egress-proxy-block proposal must call apply_routes_change with the right args and surface its failures.""" @@ -216,9 +216,9 @@ class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): dashboard.apply_routes_change = self._original_apply self._teardown_fake_home() - def _enqueue_cred_proxy(self, proposed: str = '{"routes": []}\n'): + def _enqueue_egress_proxy(self, proposed: str = '{"routes": []}\n'): p = Proposal.new( - bottle_slug="dev", tool=TOOL_CRED_PROXY_BLOCK, + bottle_slug="dev", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file=proposed, justification="need a route", current_file_hash=sha256_hex(proposed), @@ -229,12 +229,12 @@ class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): supervise.write_proposal(qdir, p) return dashboard.QueuedProposal(proposal=p, queue_dir=qdir) - def test_cred_proxy_block_calls_apply_with_proposed_file(self): + def test_egress_proxy_block_calls_apply_with_proposed_file(self): calls = [] dashboard.apply_routes_change = lambda slug, content: ( calls.append((slug, content)) or ("before", content) ) - qp = self._enqueue_cred_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') + qp = self._enqueue_egress_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') dashboard.approve(qp) self.assertEqual(1, len(calls)) slug, content = calls[0] @@ -246,16 +246,16 @@ class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): dashboard.apply_routes_change = lambda slug, content: ( calls.append(content) or ("before", content) ) - qp = self._enqueue_cred_proxy() + qp = self._enqueue_egress_proxy() dashboard.approve(qp, final_file='{"routes": [{"path": "/edited/"}]}\n', notes="tweaked") self.assertEqual(['{"routes": [{"path": "/edited/"}]}\n'], calls) def test_apply_failure_blocks_response_and_audit(self): dashboard.apply_routes_change = lambda slug, content: (_ for _ in ()).throw( - CredProxyApplyError("docker exec failed") + EgressProxyApplyError("docker exec failed") ) - qp = self._enqueue_cred_proxy() - with self.assertRaises(CredProxyApplyError): + qp = self._enqueue_egress_proxy() + with self.assertRaises(EgressProxyApplyError): dashboard.approve(qp) # No response file (proposal stays pending). self.assertEqual( @@ -263,16 +263,16 @@ class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): [p.id for p in supervise.list_pending_proposals(qp.queue_dir)], ) # No audit entry. - self.assertEqual([], read_audit_entries("cred-proxy", "dev")) + self.assertEqual([], read_audit_entries("egress-proxy", "dev")) def test_real_diff_lands_in_audit(self): dashboard.apply_routes_change = lambda slug, content: ( '{"routes": []}\n', # before '{"routes": [{"path": "/new/"}]}\n', # after ) - qp = self._enqueue_cred_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') + qp = self._enqueue_egress_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') dashboard.approve(qp) - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual(1, len(entries)) self.assertIn('+{"routes": [{"path": "/new/"}]}', entries[0].diff) self.assertIn('-{"routes": []}', entries[0].diff) @@ -282,13 +282,13 @@ class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): dashboard.apply_routes_change = lambda slug, content: ( called.append(True) or ("", content) ) - qp = self._enqueue_cred_proxy() + qp = self._enqueue_egress_proxy() dashboard.reject(qp, reason="no thanks") self.assertEqual([], called) # Reject still writes a response + audit entry with empty diff. resp = read_response(qp.queue_dir, qp.proposal.id) self.assertEqual(STATUS_REJECTED, resp.status) - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual(1, len(entries)) self.assertEqual("", entries[0].diff) @@ -432,7 +432,7 @@ class TestCapabilityApplyWiring(_FakeHomeMixin, unittest.TestCase): dashboard.approve(qp) # capability-block has no audit log per PRD 0013 — its record # lives in the per-bottle Dockerfile + transcript state. - self.assertEqual([], read_audit_entries("cred-proxy", "dev")) + self.assertEqual([], read_audit_entries("egress-proxy", "dev")) self.assertEqual([], read_audit_entries("pipelock", "dev")) def test_proposal_archived_after_apply(self): @@ -464,7 +464,7 @@ class TestOperatorEditRoutes(_FakeHomeMixin, unittest.TestCase): '{"routes": []}\n', content, ) dashboard.operator_edit_routes("dev", '{"routes": [{"path": "/x/"}]}\n') - entries = read_audit_entries("cred-proxy", "dev") + entries = read_audit_entries("egress-proxy", "dev") self.assertEqual(1, len(entries)) self.assertEqual(supervise.ACTION_OPERATOR_EDIT, entries[0].operator_action) self.assertEqual("", entries[0].justification) @@ -472,14 +472,14 @@ class TestOperatorEditRoutes(_FakeHomeMixin, unittest.TestCase): def test_failure_does_not_write_audit(self): dashboard.apply_routes_change = lambda slug, content: (_ for _ in ()).throw( - CredProxyApplyError("nope") + EgressProxyApplyError("nope") ) - with self.assertRaises(CredProxyApplyError): + with self.assertRaises(EgressProxyApplyError): dashboard.operator_edit_routes("dev", '{"routes": []}\n') - self.assertEqual([], read_audit_entries("cred-proxy", "dev")) + self.assertEqual([], read_audit_entries("egress-proxy", "dev")) -class TestDiscoverCredProxySlugs(unittest.TestCase): +class TestDiscoverEgressProxySlugs(unittest.TestCase): """Slug-extraction parsing — exercises only the parsing path; the docker ps invocation itself is environment-dependent (and tested implicitly by the integration test).""" @@ -491,7 +491,7 @@ class TestDiscoverCredProxySlugs(unittest.TestCase): original = os.environ.get("PATH", "") os.environ["PATH"] = "/nonexistent-no-docker-here" try: - self.assertEqual([], dashboard.discover_cred_proxy_slugs()) + self.assertEqual([], dashboard.discover_egress_proxy_slugs()) self.assertEqual([], dashboard.discover_pipelock_slugs()) finally: os.environ["PATH"] = original diff --git a/tests/unit/test_dashboard_detail_lines.py b/tests/unit/test_dashboard_detail_lines.py index 918ceb3..096cfbe 100644 --- a/tests/unit/test_dashboard_detail_lines.py +++ b/tests/unit/test_dashboard_detail_lines.py @@ -12,7 +12,7 @@ from claude_bottle.cli import dashboard from claude_bottle.supervise import ( Proposal, TOOL_CAPABILITY_BLOCK, - TOOL_CRED_PROXY_BLOCK, + TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, sha256_hex, ) @@ -46,9 +46,9 @@ class TestPipelockHostHighlight(unittest.TestCase): green_lines = [text for text, attr in lines if attr == self.GREEN] self.assertEqual(["api.github.com"], green_lines) - def test_no_green_lines_for_cred_proxy_block(self): + def test_no_green_lines_for_egress_proxy_block(self): lines = dashboard._detail_lines( - _qp(TOOL_CRED_PROXY_BLOCK, '{"routes": []}'), + _qp(TOOL_EGRESS_PROXY_BLOCK, '{"routes": []}'), green_attr=self.GREEN, ) self.assertEqual([], [t for t, a in lines if a == self.GREEN]) diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py new file mode 100644 index 0000000..eb47b4c --- /dev/null +++ b/tests/unit/test_egress_proxy_apply.py @@ -0,0 +1,56 @@ +"""Unit: validate_routes_content (PRD 0014 retargeted by PRD 0017 +chunk 3). docker exec / cp / kill paths are covered by the +integration test.""" + +import unittest + +from claude_bottle.backend.docker.egress_proxy_apply import ( + EgressProxyApplyError, + validate_routes_content, +) + + +class TestValidateRoutesContent(unittest.TestCase): + def test_accepts_minimal_route_table(self): + validate_routes_content('{"routes": []}') + validate_routes_content( + '{"routes": [{"host": "api.github.com"}]}' + ) + + def test_accepts_full_route(self): + validate_routes_content( + '{"routes": [{"host": "api.github.com",' + ' "path_allowlist": ["/repos/x/"],' + ' "auth_scheme": "Bearer",' + ' "token_env": "EGRESS_PROXY_TOKEN_0"}]}' + ) + + def test_rejects_bad_json(self): + with self.assertRaises(EgressProxyApplyError) as cm: + validate_routes_content("{not json") + self.assertIn("not valid", str(cm.exception)) + + def test_rejects_non_object_top_level(self): + with self.assertRaises(EgressProxyApplyError): + validate_routes_content("[]") + + def test_rejects_missing_routes_key(self): + with self.assertRaises(EgressProxyApplyError): + validate_routes_content('{"other": []}') + + def test_rejects_non_list_routes(self): + with self.assertRaises(EgressProxyApplyError): + validate_routes_content('{"routes": "not a list"}') + + def test_rejects_partial_auth_pair(self): + # The addon-core parser enforces both-or-neither — the apply + # path picks this up before SIGHUP'ing the sidecar. + with self.assertRaises(EgressProxyApplyError): + validate_routes_content( + '{"routes": [{"host": "x.example",' + ' "auth_scheme": "Bearer"}]}' + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_supervise.py b/tests/unit/test_supervise.py index 90f54f6..f24baa2 100644 --- a/tests/unit/test_supervise.py +++ b/tests/unit/test_supervise.py @@ -17,7 +17,7 @@ from claude_bottle.supervise import ( STATUS_MODIFIED, STATUS_REJECTED, TOOL_CAPABILITY_BLOCK, - TOOL_CRED_PROXY_BLOCK, + TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, archive_proposal, audit_log_path, @@ -37,7 +37,7 @@ from claude_bottle.supervise import ( FIXED_TS = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc) -def _proposal(tool: str = TOOL_CRED_PROXY_BLOCK, proposed: str = "{}", justification: str = "need a route") -> Proposal: +def _proposal(tool: str = TOOL_EGRESS_PROXY_BLOCK, proposed: str = "{}", justification: str = "need a route") -> Proposal: return Proposal.new( bottle_slug="dev", tool=tool, @@ -54,7 +54,7 @@ class TestProposalRoundtrip(unittest.TestCase): self.assertTrue(p.id) self.assertEqual("2026-05-25T12:00:00+00:00", p.arrival_timestamp) self.assertEqual("dev", p.bottle_slug) - self.assertEqual(TOOL_CRED_PROXY_BLOCK, p.tool) + self.assertEqual(TOOL_EGRESS_PROXY_BLOCK, p.tool) def test_to_from_dict_roundtrip(self): p = _proposal() @@ -139,13 +139,13 @@ class TestQueueIO(unittest.TestCase): def test_list_pending_sorted_by_arrival(self): # Fabricate two with explicit timestamps. a = Proposal.new( - bottle_slug="dev", tool=TOOL_CRED_PROXY_BLOCK, + bottle_slug="dev", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file="{}", justification="early", current_file_hash="x", now=datetime(2026, 5, 25, 10, 0, 0, tzinfo=timezone.utc), ) b = Proposal.new( - bottle_slug="dev", tool=TOOL_CRED_PROXY_BLOCK, + bottle_slug="dev", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file="{}", justification="late", current_file_hash="x", now=datetime(2026, 5, 25, 14, 0, 0, tzinfo=timezone.utc), @@ -314,12 +314,12 @@ class TestDiffAndHash(unittest.TestCase): class TestToolConstants(unittest.TestCase): def test_tools_tuple_matches_individual_constants(self): self.assertEqual( - (TOOL_CRED_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, TOOL_CAPABILITY_BLOCK), + (TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, TOOL_CAPABILITY_BLOCK), supervise.TOOLS, ) def test_component_map_covers_two_remediation_tools_only(self): - self.assertIn(TOOL_CRED_PROXY_BLOCK, supervise.COMPONENT_FOR_TOOL) + self.assertIn(TOOL_EGRESS_PROXY_BLOCK, supervise.COMPONENT_FOR_TOOL) self.assertIn(TOOL_PIPELOCK_BLOCK, supervise.COMPONENT_FOR_TOOL) self.assertNotIn(TOOL_CAPABILITY_BLOCK, supervise.COMPONENT_FOR_TOOL) diff --git a/tests/unit/test_supervise_server.py b/tests/unit/test_supervise_server.py index e77f9bc..ef7616c 100644 --- a/tests/unit/test_supervise_server.py +++ b/tests/unit/test_supervise_server.py @@ -45,19 +45,19 @@ from claude_bottle.supervise_server import ( class TestValidation(unittest.TestCase): - def test_cred_proxy_block_requires_valid_json(self): + def test_egress_proxy_block_requires_valid_json(self): with self.assertRaises(_RpcError) as cm: - validate_proposed_file(_sv.TOOL_CRED_PROXY_BLOCK, "{not json") + validate_proposed_file(_sv.TOOL_EGRESS_PROXY_BLOCK, "{not json") self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code) self.assertIn("not valid JSON", cm.exception.message) - def test_cred_proxy_block_requires_routes_array(self): + def test_egress_proxy_block_requires_routes_array(self): with self.assertRaises(_RpcError): - validate_proposed_file(_sv.TOOL_CRED_PROXY_BLOCK, '{"other": []}') + validate_proposed_file(_sv.TOOL_EGRESS_PROXY_BLOCK, '{"other": []}') - def test_cred_proxy_block_accepts_valid_routes(self): + def test_egress_proxy_block_accepts_valid_routes(self): validate_proposed_file( - _sv.TOOL_CRED_PROXY_BLOCK, + _sv.TOOL_EGRESS_PROXY_BLOCK, '{"routes": [{"path": "/x/", "upstream": "https://example.com"}]}', ) @@ -175,7 +175,7 @@ class TestHandleToolsList(unittest.TestCase): names = [t["name"] for t in result["tools"]] # type: ignore[index] self.assertEqual( sorted([ - _sv.TOOL_CRED_PROXY_BLOCK, + _sv.TOOL_EGRESS_PROXY_BLOCK, _sv.TOOL_PIPELOCK_BLOCK, _sv.TOOL_CAPABILITY_BLOCK, ]), @@ -225,7 +225,7 @@ class TestHandleToolsCall(unittest.TestCase): try: result = handle_tools_call( { - "name": _sv.TOOL_CRED_PROXY_BLOCK, + "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { "routes": '{"routes": []}', "justification": "need a route", @@ -269,7 +269,7 @@ class TestHandleToolsCall(unittest.TestCase): with self.assertRaises(_RpcError): handle_tools_call( { - "name": _sv.TOOL_CRED_PROXY_BLOCK, + "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": {"routes": '{"routes": []}'}, }, self.config, @@ -280,7 +280,7 @@ class TestHandleToolsCall(unittest.TestCase): try: handle_tools_call( { - "name": _sv.TOOL_CRED_PROXY_BLOCK, + "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { "routes": '{"routes": []}', "justification": "x", @@ -367,7 +367,7 @@ class TestHttpEndToEnd(unittest.TestCase): self.assertEqual("2.0", result["jsonrpc"]) self.assertEqual(1, result["id"]) names = [t["name"] for t in result["result"]["tools"]] # type: ignore[index] - self.assertIn(_sv.TOOL_CRED_PROXY_BLOCK, names) + self.assertIn(_sv.TOOL_EGRESS_PROXY_BLOCK, names) def test_unknown_method_returns_jsonrpc_error(self): result = self._post_jsonrpc( From f04fbb68a937c55dd00bbb6700adacd1b30cbaf2 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 15:28:11 -0400 Subject: [PATCH 02/18] feat(egress-proxy): drive claude-code OAuth placeholder off a role marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chunk 2 detection keyed on `token_ref == "CLAUDE_CODE_OAUTH_TOKEN"`, which broke any bottle whose host env var has a different name (e.g. `CLAUDE_BOTTLE_OAUTH_TOKEN`). The token_ref is the user's choice — the placeholder-env trigger shouldn't be locked to one specific string. Restoring a minimal `role` marker on `EgressProxyRoute`: - `EGRESS_PROXY_ROLES = frozenset({"claude_code_oauth"})` — one marker for now; the field is back so we can grow it. - `EGRESS_PROXY_SINGLETON_ROLES` — claude_code_oauth is a singleton (only one route per bottle can carry it). - `Role: tuple[str, ...]` field on `EgressProxyRoute` (manifest + runtime), parsed as string or list-of-strings; unknown roles are rejected so typos can't become silent no-ops. `prepare.py:has_anthropic_auth` now checks for `"claude_code_oauth" in r.roles` instead of matching a literal token_ref string. Bottles can name their host OAuth env var anything; the role marker is what flips on `CLAUDE_CODE_OAUTH_TOKEN=` and the telemetry-off env vars on the agent. Test coverage: 7 new manifest tests (omitted / string / list / unknown role rejected / non-string rejected / list-item non-string rejected / singleton enforced). 364 tests pass. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/prepare.py | 17 ++--- claude_bottle/egress_proxy.py | 9 ++- claude_bottle/manifest.py | 80 +++++++++++++++++++++++- tests/unit/test_manifest_egress_proxy.py | 51 +++++++++++++++ 4 files changed, 146 insertions(+), 11 deletions(-) diff --git a/claude_bottle/backend/docker/prepare.py b/claude_bottle/backend/docker/prepare.py index 52cd8d6..e8c6b3b 100644 --- a/claude_bottle/backend/docker/prepare.py +++ b/claude_bottle/backend/docker/prepare.py @@ -175,15 +175,16 @@ def resolve_plan( # never lands on argv or in env_file) goes into one dict. Nothing # mutates the host os.environ. forwarded_env: dict[str, str] = dict(resolved.forwarded) - # When the bottle declares an egress-proxy route for the Anthropic - # OAuth flow, claude-code's outbound Authorization gets stripped + - # re-injected by egress-proxy. The agent's environ still needs - # *something* claude-code recognises as a credential or it refuses - # to start; ship a non-secret placeholder. The placeholder is not - # any real `auth.token_ref` value, so leaking it would tell an - # attacker only that egress-proxy is in front. + # When the bottle declares an egress-proxy route with the + # `claude_code_oauth` role marker, claude-code's outbound + # Authorization gets stripped + re-injected by egress-proxy. The + # agent's environ still needs *something* claude-code recognises + # as a credential or it refuses to start; ship a non-secret + # placeholder. The placeholder isn't any real token value, so + # leaking it would tell an attacker only that egress-proxy is in + # front. Manifest validation enforces singleton on this role. has_anthropic_auth = any( - r.token_ref == "CLAUDE_CODE_OAUTH_TOKEN" + "claude_code_oauth" in r.roles for r in egress_proxy_plan.routes ) if has_anthropic_auth: diff --git a/claude_bottle/egress_proxy.py b/claude_bottle/egress_proxy.py index fe4b849..864827f 100644 --- a/claude_bottle/egress_proxy.py +++ b/claude_bottle/egress_proxy.py @@ -62,13 +62,18 @@ class EgressProxyRoute: (e.g. `EGRESS_PROXY_TOKEN_0`); `token_ref` is the host env var the CLI reads at launch and forwards into the container's environ under `token_env`. Routes that share a `token_ref` coalesce to - one `token_env` slot.""" + one `token_env` slot. + + `roles` carries the manifest route's optional role markers (see + `manifest.EGRESS_PROXY_ROLES`). The launch step reads these for + side effects like the claude-code OAuth placeholder env.""" host: str path_allowlist: tuple[str, ...] = () auth_scheme: str = "" token_env: str = "" token_ref: str = "" + roles: tuple[str, ...] = () @dataclass(frozen=True) @@ -148,11 +153,13 @@ def egress_proxy_routes_for_bottle( auth_scheme=r.AuthScheme, token_env=token_env, token_ref=r.TokenRef, + roles=r.Role, )) else: out.append(EgressProxyRoute( host=r.Host, path_allowlist=r.PathAllowlist, + roles=r.Role, )) return tuple(out) diff --git a/claude_bottle/manifest.py b/claude_bottle/manifest.py index dfea962..794e1b7 100644 --- a/claude_bottle/manifest.py +++ b/claude_bottle/manifest.py @@ -129,6 +129,34 @@ class GitEntry: # token-not-Bearer quirk (go-gitea/gitea#16734). EGRESS_PROXY_AUTH_SCHEMES = ("Bearer", "token") +# Optional per-route role markers. A role signals "this route plays +# a specific named part in the bottle's auth flow"; the launch step +# acts on the marker. +# +# claude_code_oauth: this route auth-injects on the agent's +# claude-code OAuth flow. Triggers prepare.py +# to ship a placeholder CLAUDE_CODE_OAUTH_TOKEN +# to the agent (so claude-code starts) and +# disable nonessential-traffic / error-reporting +# env vars. Host doesn't matter to the placeholder +# logic — declare the role on whichever route +# injects the OAuth header. +# +# Routes without a `role` are pure proxy entries: egress-proxy +# enforces path_allowlist + injects auth on its own, but nothing +# special happens on the agent side. +EGRESS_PROXY_ROLES = frozenset({ + "claude_code_oauth", +}) + +# Singleton roles may appear on at most one route per bottle. +# claude_code_oauth drives a single placeholder env var; two routes +# claiming it would leave "which one is the canonical OAuth route?" +# ambiguous for any future role-aware logic. +EGRESS_PROXY_SINGLETON_ROLES = frozenset({ + "claude_code_oauth", +}) + @dataclass(frozen=True) class EgressProxyRoute: @@ -143,6 +171,11 @@ class EgressProxyRoute: manifest's `auth` block is omitted both fields are empty strings — no Authorization is written, no token forwarded. + `Role` is an optional tuple of named markers (see + EGRESS_PROXY_ROLES). The launch step reads these and triggers + associated side effects (e.g. the `claude_code_oauth` marker + causes prepare.py to set a placeholder OAuth env on the agent). + Validation rules (enforced in `from_dict`): - `host` required, non-empty. - `path_allowlist` optional, list of absolute path prefixes. @@ -150,12 +183,17 @@ class EgressProxyRoute: `token_ref` as non-empty strings; an empty `auth: {}` is an error rather than a synonym for "no auth" (omit `auth` for that case). + - `role` optional. String or list of strings drawn from + EGRESS_PROXY_ROLES. Singleton roles (see + EGRESS_PROXY_SINGLETON_ROLES) may appear on at most one + route per bottle. """ Host: str PathAllowlist: tuple[str, ...] = () AuthScheme: str = "" TokenRef: str = "" + Role: tuple[str, ...] = () @classmethod def from_dict(cls, bottle_name: str, idx: int, raw: object) -> "EgressProxyRoute": @@ -226,11 +264,37 @@ class EgressProxyRoute: auth_scheme = auth_scheme_raw token_ref = token_ref_raw + role_raw = d.get("role") + roles: tuple[str, ...] = () + if role_raw is None: + roles = () + elif isinstance(role_raw, str): + roles = (role_raw,) + elif isinstance(role_raw, list): + role_list = cast(list[object], role_raw) + collected_roles: list[str] = [] + for r in role_list: + if not isinstance(r, str): + die(f"{label} role items must be strings (got {type(r).__name__})") + collected_roles.append(r) + roles = tuple(collected_roles) + else: + die( + f"{label} role must be a string or a list of strings " + f"(was {type(role_raw).__name__})" + ) + for r in roles: + if r not in EGRESS_PROXY_ROLES: + die( + f"{label} role {r!r} is not one of " + f"{', '.join(sorted(EGRESS_PROXY_ROLES))}" + ) + for k in d: - if k not in ("host", "path_allowlist", "auth"): + if k not in ("host", "path_allowlist", "auth", "role"): die( f"{label} has unknown key {k!r}; accepted keys are " - f"'host', 'path_allowlist', 'auth'" + f"'host', 'path_allowlist', 'auth', 'role'" ) return cls( @@ -238,6 +302,7 @@ class EgressProxyRoute: PathAllowlist=prefixes, AuthScheme=auth_scheme, TokenRef=token_ref, + Role=roles, ) @@ -715,6 +780,8 @@ def _validate_egress_proxy_routes( - Hosts must be unique within the bottle. The proxy matches by exact-host (v1, prefix matching is on path_allowlist only); duplicate hosts leave the route choice ambiguous. + - Singleton roles (see EGRESS_PROXY_SINGLETON_ROLES) may appear + on at most one route per bottle. No cross-validation against `bottle.git` is performed. git-gate (SSH push/fetch) and egress-proxy (HTTPS) broker different @@ -729,6 +796,15 @@ def _validate_egress_proxy_routes( f"{r.Host!r}; each host must be unique on the proxy." ) seen_hosts[key] = None + for role in EGRESS_PROXY_SINGLETON_ROLES: + with_role = [r for r in routes if role in r.Role] + if len(with_role) > 1: + hosts = ", ".join(r.Host for r in with_role) + die( + f"bottle '{bottle_name}' egress_proxy.routes has {len(with_role)} " + f"routes with role {role!r} (hosts: {hosts}); this role drives a " + f"single launch-step side effect — pick one." + ) def _validate_unique_git_names(bottle_name: str, git: tuple[GitEntry, ...]) -> None: diff --git a/tests/unit/test_manifest_egress_proxy.py b/tests/unit/test_manifest_egress_proxy.py index 4ffa998..2e83cea 100644 --- a/tests/unit/test_manifest_egress_proxy.py +++ b/tests/unit/test_manifest_egress_proxy.py @@ -128,6 +128,57 @@ class TestAuth(unittest.TestCase): }]) +class TestRole(unittest.TestCase): + def test_omitted_means_no_roles(self): + b = _bottle([{"host": "x.example"}]) + self.assertEqual((), b.egress_proxy.routes[0].Role) + + def test_string_normalizes_to_tuple(self): + b = _bottle([{ + "host": "api.anthropic.com", + "role": "claude_code_oauth", + "auth": {"scheme": "Bearer", "token_ref": "T"}, + }]) + self.assertEqual(("claude_code_oauth",), + b.egress_proxy.routes[0].Role) + + def test_list_supported(self): + b = _bottle([{ + "host": "api.anthropic.com", + "role": ["claude_code_oauth"], + "auth": {"scheme": "Bearer", "token_ref": "T"}, + }]) + self.assertEqual(("claude_code_oauth",), + b.egress_proxy.routes[0].Role) + + def test_unknown_role_rejected(self): + # The role enum is locked down — typos shouldn't silently + # become no-op markers. + with self.assertRaises(Die): + _bottle([{"host": "x.example", "role": "totally-made-up"}]) + + def test_non_string_role_rejected(self): + with self.assertRaises(Die): + _bottle([{"host": "x.example", "role": 42}]) + + def test_list_with_non_string_item_rejected(self): + with self.assertRaises(Die): + _bottle([{"host": "x.example", + "role": ["claude_code_oauth", 42]}]) + + def test_singleton_claude_code_oauth_enforced(self): + # Two routes both claiming the role would make "which one + # drives the placeholder env?" ambiguous. + with self.assertRaises(Die): + _bottle([ + {"host": "api.anthropic.com", "role": "claude_code_oauth", + "auth": {"scheme": "Bearer", "token_ref": "T1"}}, + {"host": "api2.anthropic.example", + "role": "claude_code_oauth", + "auth": {"scheme": "Bearer", "token_ref": "T2"}}, + ]) + + class TestRouteValidation(unittest.TestCase): def test_duplicate_hosts_rejected(self): # Routes match by exact host; duplicates leave the choice From 57a9707e1c278c5627a10bcdefcb674a870a0e8f Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 15:42:51 -0400 Subject: [PATCH 03/18] fix(egress-proxy): chmod 644 host CAs so mitmproxy user can read after docker cp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mitmdump crashed at boot with PermissionError on ~/.mitmproxy/mitmproxy-ca.pem. Cause: `docker cp` preserves the host file's mode AND uid. The CA files were 0600 owned by the host user (uid 501 on macOS), so inside the container the mitmproxy user (uid 1000, set by USER directive in Dockerfile) couldn't read them. Fix: - `egress_proxy_tls_init`: chmod 644 the cert-only + the cert+key concat on the host stage dir. - `DockerEgressProxy.start`: chmod 644 routes.yaml and the pipelock CA before `docker cp` into the egress-proxy container (pipelock itself runs as root so its in-pipelock copy is unaffected). The host stage_dir is mode 700 — other host users still can't traverse in, so the cert+key concat isn't actually exposed despite the 644 mode. The container side gets world-readable, which is fine inside the per-bottle container. Reproduces against today's main: bottle's egress-proxy sidecar crashes with PermissionError; after this patch mitmdump boots and listens on :9099. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/egress_proxy.py | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy.py b/claude_bottle/backend/docker/egress_proxy.py index aa965f0..a823179 100644 --- a/claude_bottle/backend/docker/egress_proxy.py +++ b/claude_bottle/backend/docker/egress_proxy.py @@ -115,11 +115,17 @@ def egress_proxy_tls_init(stage_dir: Path) -> tuple[Path, Path]: key = work / "ca-key.pem" if not cert.is_file() or not key.is_file(): die(f"egress-proxy tls init did not produce ca files in {work}") - cert.chmod(0o600) + # Mode 644 (not 600) so `docker cp` preserves world-readability + # inside the container — the mitmproxy user (uid 1000) needs to + # read the file, and the host uid `docker cp` propagates from the + # source doesn't match. The host stage_dir is mode 700 so other + # host users still can't traverse in; the private key isn't + # exposed despite the file mode. + cert.chmod(0o644) # mitmproxy reads cert + key from a single concatenated PEM file. mitm = work / "mitmproxy-ca.pem" mitm.write_bytes(cert.read_bytes() + key.read_bytes()) - mitm.chmod(0o600) + mitm.chmod(0o644) return (mitm, cert) @@ -232,6 +238,17 @@ class DockerEgressProxy(EgressProxy): f"{create_result.stderr.strip()}" ) + # routes.yaml also lands inside the container; bump to 644 + # for the same reason as the CAs — mitmproxy user (uid 1000) + # has to read it. Host stage_dir is mode 700 so the file + # isn't actually exposed to other host users. + plan.routes_path.chmod(0o644) + # Pipelock CA: pipelock itself runs as root so its in-pipelock + # copy doesn't care about mode, but egress-proxy's mitmproxy + # user does. Bump on the host so docker cp into egress-proxy + # carries world-readable. + if route_via_pipelock: + plan.pipelock_ca_host_path.chmod(0o644) cps: list[tuple[Path, str, str]] = [ (plan.routes_path, EGRESS_PROXY_ROUTES_IN_CONTAINER, "routes.yaml"), (plan.mitmproxy_ca_host_path, EGRESS_PROXY_CA_IN_CONTAINER, "mitmproxy CA"), From b9c70f7daa9645b81e3dd9b656321759baefd555 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 15:52:08 -0400 Subject: [PATCH 04/18] fix(egress-proxy): build combined trust bundle (system + pipelock CA) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--set ssl_verify_upstream_trusted_ca` REPLACES mitmproxy's default trust store with the file we point it at. The earlier wiring pointed it at just pipelock's CA, which broke for any host pipelock passes through (api.anthropic.com is in DEFAULT_TLS_PASSTHROUGH): pipelock CONNECT-tunnels the handshake to the real upstream, egress-proxy sees the real public cert (signed by e.g. DigiCert), and refuses to validate because pipelock's CA doesn't sign it. Fix in Dockerfile entrypoint: when EGRESS_PROXY_UPSTREAM_CA is set, concatenate /etc/ssl/certs/ca-certificates.crt + the pipelock CA into /home/mitmproxy/.mitmproxy/combined-trust.pem, and pass that as ssl_verify_upstream_trusted_ca. Covers both legs: - pipelock-MITM'd hosts → leaf cert signed by pipelock CA → validates against the pipelock half of the bundle. - pipelock-passthrough hosts (api.anthropic.com et al.) → real upstream cert → validates against the system half. Standalone runs of the image (no EGRESS_PROXY_UPSTREAM_CA) skip the concat and use mitmproxy's default trust store. Reproduces against today's main: agent gets "Unable to connect to API: SSL certificate verification failed" on api.anthropic.com, egress-proxy logs "Server TLS handshake failed. Certificate verify failed: unable to get local issuer certificate". After this patch the trust bundle includes the real upstream root + pipelock's CA and both validation paths succeed. Co-Authored-By: Claude Opus 4.7 --- Dockerfile.egress-proxy | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/Dockerfile.egress-proxy b/Dockerfile.egress-proxy index 8cce9e3..8f75a18 100644 --- a/Dockerfile.egress-proxy +++ b/Dockerfile.egress-proxy @@ -44,14 +44,20 @@ USER mitmproxy EXPOSE 9099 # Entrypoint: -# --mode regular@9099 standard HTTP/HTTPS forward proxy on :9099. -# --set ssl_verify_upstream_trusted_ca=... only when -# EGRESS_PROXY_UPSTREAM_CA env is set (the backend's start step -# sets it to the in-container pipelock-CA path when pipelock is -# present, so the upstream leg trusts pipelock's MITM). The -# ${VAR:+expansion} form omits the flag when the var is unset -# or empty — useful for standalone runs of the image (e.g. unit -# tests) where no upstream CA is mounted. -# -s /app/egress_proxy_addon.py loads our addon, which reads the -# route table from /etc/egress-proxy/routes.yaml. -ENTRYPOINT ["sh", "-c", "exec mitmdump --mode regular@9099 ${EGRESS_PROXY_UPSTREAM_CA:+--set ssl_verify_upstream_trusted_ca=$EGRESS_PROXY_UPSTREAM_CA} -s /app/egress_proxy_addon.py"] +# - Build a combined upstream-trust bundle when +# EGRESS_PROXY_UPSTREAM_CA is set (the backend's start step +# sets it to the in-container pipelock-CA path). +# `--set ssl_verify_upstream_trusted_ca` REPLACES mitmproxy's +# default trust store with the file we point it at; if we just +# pointed it at pipelock's CA, mitmproxy would refuse any host +# pipelock passes through (api.anthropic.com etc.) because +# pipelock's CA doesn't sign the real upstream certs. So +# concatenate the system bundle + pipelock CA into one PEM and +# point mitmproxy at that — covers both pipelock-MITM'd and +# pipelock-passthrough hosts. +# - --mode regular@9099 → standard HTTP/HTTPS forward proxy. +# - -s /app/egress_proxy_addon.py → loads our addon, reads +# /etc/egress-proxy/routes.yaml. +# Standalone runs (no EGRESS_PROXY_UPSTREAM_CA) skip the bundle +# build and use mitmproxy's default trust store. +ENTRYPOINT ["sh", "-c", "if [ -n \"$EGRESS_PROXY_UPSTREAM_CA\" ] && [ -f \"$EGRESS_PROXY_UPSTREAM_CA\" ]; then COMBINED=/home/mitmproxy/.mitmproxy/combined-trust.pem; cat /etc/ssl/certs/ca-certificates.crt \"$EGRESS_PROXY_UPSTREAM_CA\" > \"$COMBINED\"; exec mitmdump --mode regular@9099 --set ssl_verify_upstream_trusted_ca=\"$COMBINED\" -s /app/egress_proxy_addon.py; else exec mitmdump --mode regular@9099 -s /app/egress_proxy_addon.py; fi"] From 5dc33f3acc8074beb679a4298cad9c0cc60cdec6 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 16:29:27 -0400 Subject: [PATCH 05/18] fix(egress-proxy): mint CA via openssl req so leaf AKI matches CA SKI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the persistent SSL handshake failure: pipelock's `tls init` stamps a non-standard `Subject Key Identifier` on the CAs it generates (random rather than SHA-1 of the pubkey). mitmproxy computes the `Authority Key Identifier` on each leaf cert it mints as SHA-1(issuer's pubkey). openssl's chain validator uses the leaf's AKI to find the issuer cert by SKI; with pipelock's SKI off by definition, the lookup fails and openssl returns "unable to get local issuer certificate" — even though the CA is right there in the trust store with the matching SHA-256 fingerprint. (Also, pipelock generates EC CAs; the cert+key concat fit in 834 bytes vs ~2.3KB for RSA, which was the first red flag.) Diagnostic from a live bottle confirmed: leaf cert AKI: A8:F0:D5:E3:B5:B9:C2:38:2B:9F:DD:4A:DF:26:8C:72:19:A2:5E:94 CA cert SKI: 81:CA:6D:4C:ED:5C:C2:B1:48:0C:3E:E8:8D:73:86:97:B9:89:B4:3D CA cert + leaf cert: same Pipelock-named subject, same public key bytes openssl verify -CAfile : error 20 Fix: switch `egress_proxy_tls_init` from `pipelock tls init` to host `openssl req` with an explicit `subjectKeyIdentifier=hash` extension. SHA-1(pubkey) for the SKI matches what mitmproxy puts in the AKI, so chain validation works. The generated CA is also RSA-2048 / sha256WithRSAEncryption — mitmproxy's most-tested configuration. The new generator is independent of pipelock entirely (no docker run on the pipelock image to mint the CA), so the egress-proxy CA generation now requires only `openssl` on the host. macOS + Linux dev images both have it. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/egress_proxy.py | 100 ++++++++++++------- 1 file changed, 63 insertions(+), 37 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy.py b/claude_bottle/backend/docker/egress_proxy.py index a823179..99d6ba4 100644 --- a/claude_bottle/backend/docker/egress_proxy.py +++ b/claude_bottle/backend/docker/egress_proxy.py @@ -75,9 +75,7 @@ def build_egress_proxy_image() -> None: def egress_proxy_tls_init(stage_dir: Path) -> tuple[Path, Path]: - """Mint the per-bottle egress-proxy MITM CA. Reuses the pipelock - binary's `tls init` subcommand — a known-good RSA CA minter we - already pin and run on this host. + """Mint the per-bottle egress-proxy MITM CA via host `openssl req`. Returns `(mitmproxy_pem, cert_only_pem)`: - `mitmproxy_pem` is the single-PEM concat (cert + key) @@ -86,47 +84,75 @@ def egress_proxy_tls_init(stage_dir: Path) -> tuple[Path, Path]: trust store by `provision_ca` so the agent trusts the bumped CONNECT cert egress-proxy presents. - Both files live under `/egress-proxy-ca/` (mode 600). - Private keys never leave the host stage dir until - `DockerEgressProxy.start` docker-cps the concat file into the - sidecar; start.py's outer finally `shutil.rmtree`s the stage dir - after teardown. + Why openssl req (not the pipelock binary's `tls init`): + pipelock's CA generator stamps a non-standard `Subject Key + Identifier` on the CA (random rather than SHA-1 of the pubkey). + mitmproxy computes the `Authority Key Identifier` on each leaf + it mints as SHA-1(issuer's pubkey). openssl's chain validator + uses the leaf's AKI to find the issuer cert by SKI; pipelock's + SKI doesn't match → openssl reports "unable to get local issuer + certificate" even though the CA is right there in the trust + store. openssl req's `subjectKeyIdentifier=hash` extension uses + SHA-1(pubkey), matching mitmproxy's computation. - Imported lazily inside the function so test patchers in - pipelock-land don't need to know about us.""" - # Local import keeps the module-import graph free of a hard - # pipelock-image dependency at top of file (we don't actually - # need pipelock's *runtime* here, just its tls-init subcommand). - from .pipelock import PIPELOCK_IMAGE + Both files live under `/egress-proxy-ca/` (mode 644 — + `docker cp` preserves the mode into the container, where the + mitmproxy user (uid 1000) reads them; the host stage_dir is + mode 700 so the private key isn't world-exposed).""" work = stage_dir / "egress-proxy-ca" work.mkdir(exist_ok=True) - result = subprocess.run( - ["docker", "run", "--rm", - "-v", f"{work}:/h", - "-e", "PIPELOCK_HOME=/h", - PIPELOCK_IMAGE, "tls", "init"], - capture_output=True, - text=True, - check=False, + key_path = work / "ca-key.pem" + cert_path = work / "ca.pem" + cnf_path = work / "ca.cnf" + + # RSA-2048 — broad mitmproxy compatibility (its default leaf-cert + # config matches RSA CAs without surprise), and openssl req's + # default behavior here is exactly what we want. + keygen = subprocess.run( + ["openssl", "genrsa", "-out", str(key_path), "2048"], + capture_output=True, text=True, check=False, ) - if result.returncode != 0: - die(f"egress-proxy tls init failed: {result.stderr.strip()}") - cert = work / "ca.pem" - key = work / "ca-key.pem" - if not cert.is_file() or not key.is_file(): - die(f"egress-proxy tls init did not produce ca files in {work}") - # Mode 644 (not 600) so `docker cp` preserves world-readability - # inside the container — the mitmproxy user (uid 1000) needs to - # read the file, and the host uid `docker cp` propagates from the - # source doesn't match. The host stage_dir is mode 700 so other - # host users still can't traverse in; the private key isn't - # exposed despite the file mode. - cert.chmod(0o644) + if keygen.returncode != 0: + die(f"egress-proxy ca keygen failed: {keygen.stderr.strip()}") + + # `subjectKeyIdentifier=hash` makes openssl compute the SKI as + # SHA-1(pubkey), matching how mitmproxy computes the AKI on the + # leaves it later mints. Without this, chain validation breaks + # despite the CA being present in the trust store. + cnf_path.write_text( + "[req]\n" + "distinguished_name = req_dn\n" + "prompt = no\n" + "x509_extensions = v3_ca\n" + "\n" + "[req_dn]\n" + "O = claude-bottle\n" + "CN = claude-bottle egress-proxy CA\n" + "\n" + "[v3_ca]\n" + "basicConstraints = critical, CA:TRUE\n" + "keyUsage = critical, keyCertSign, cRLSign\n" + "subjectKeyIdentifier = hash\n" + ) + cnf_path.chmod(0o644) + + req = subprocess.run( + ["openssl", "req", "-x509", "-new", "-nodes", + "-key", str(key_path), + "-sha256", "-days", "365", + "-config", str(cnf_path), + "-out", str(cert_path)], + capture_output=True, text=True, check=False, + ) + if req.returncode != 0: + die(f"egress-proxy ca cert generation failed: {req.stderr.strip()}") + + cert_path.chmod(0o644) # mitmproxy reads cert + key from a single concatenated PEM file. mitm = work / "mitmproxy-ca.pem" - mitm.write_bytes(cert.read_bytes() + key.read_bytes()) + mitm.write_bytes(cert_path.read_bytes() + key_path.read_bytes()) mitm.chmod(0o644) - return (mitm, cert) + return (mitm, cert_path) class DockerEgressProxy(EgressProxy): From f807ed11493d70620291ad546f427e9634da89f9 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 16:38:18 -0400 Subject: [PATCH 06/18] fix(egress-proxy): force traffic through pipelock + block unallowlisted hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues stopping the bottle's egress allowlist from being enforced: 1. mitmproxy was bypassing pipelock. We set HTTPS_PROXY=pipelock in the egress-proxy container's env, but mitmproxy is a proxy *server* — it does NOT honor HTTP(S)_PROXY env vars on its outbound side the way HTTP-client libraries do. All post-MITM traffic was going direct to the upstream, never touching pipelock's hostname allowlist or DLP scanner. Fix: use mitmproxy's `--mode upstream:URL` flag. The Dockerfile entrypoint now reads a new `EGRESS_PROXY_UPSTREAM_PROXY` env (set by `DockerEgressProxy.start` to the pipelock URL when pipelock is in the topology) and switches mitmdump to upstream-proxy mode. Standalone runs of the image without the env still get `--mode regular@9099` direct-to-upstream — useful for unit-test boots. Confirmed in the boot log: "HTTP(S) proxy (upstream mode) listening at *:9099." 2. egress-proxy was forwarding unrecognized hosts. The addon's `decide()` returned `Decision(action="forward")` whenever no route matched the request host, deferring to pipelock to gate. With #1 broken pipelock wasn't gating either; even with #1 fixed, defense-in-depth wants both layers enforcing. Fix: no-route-match → 403 with a "host not in allowlist" reason. The egress allowlist is now strictly the set of hosts declared in `bottle.egress_proxy.routes`; bare-pass routes (host with no auth, no path_allowlist) cover the passthrough case for hosts that just need reach. path_allowlist enforcement on matched routes is unchanged. Test updated: `test_no_matching_route_forwards` → `test_no_matching_route_blocks`. 364 unit tests pass. Co-Authored-By: Claude Opus 4.7 --- Dockerfile.egress-proxy | 33 +++++++++++--------- claude_bottle/backend/docker/egress_proxy.py | 25 ++++++++++----- claude_bottle/egress_proxy_addon_core.py | 20 +++++++++--- tests/unit/test_egress_proxy_addon_core.py | 14 +++++---- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/Dockerfile.egress-proxy b/Dockerfile.egress-proxy index 8f75a18..c948077 100644 --- a/Dockerfile.egress-proxy +++ b/Dockerfile.egress-proxy @@ -44,20 +44,23 @@ USER mitmproxy EXPOSE 9099 # Entrypoint: -# - Build a combined upstream-trust bundle when -# EGRESS_PROXY_UPSTREAM_CA is set (the backend's start step -# sets it to the in-container pipelock-CA path). -# `--set ssl_verify_upstream_trusted_ca` REPLACES mitmproxy's -# default trust store with the file we point it at; if we just -# pointed it at pipelock's CA, mitmproxy would refuse any host -# pipelock passes through (api.anthropic.com etc.) because -# pipelock's CA doesn't sign the real upstream certs. So -# concatenate the system bundle + pipelock CA into one PEM and -# point mitmproxy at that — covers both pipelock-MITM'd and -# pipelock-passthrough hosts. -# - --mode regular@9099 → standard HTTP/HTTPS forward proxy. +# - Upstream proxy: when EGRESS_PROXY_UPSTREAM_PROXY is set, +# use mitmproxy's `--mode upstream:URL` to forward all +# post-MITM traffic through pipelock. (mitmproxy does NOT +# honor HTTPS_PROXY env vars on its outbound side — it's a +# proxy server, not a client.) Standalone runs without +# EGRESS_PROXY_UPSTREAM_PROXY fall back to `regular@9099` +# direct-to-upstream — useful for unit tests of the image. +# - Upstream trust: when EGRESS_PROXY_UPSTREAM_CA is set, build +# a combined trust bundle (system roots + pipelock CA) and +# point mitmproxy at it via +# `--set ssl_verify_upstream_trusted_ca`. This option REPLACES +# mitmproxy's default trust store with the file we point it +# at — passing just pipelock's CA would break pipelock- +# passthrough hosts (api.anthropic.com etc.) where mitmproxy +# sees real upstream certs signed by public CAs. The combined +# bundle covers both pipelock-MITM'd and pipelock-passthrough +# hosts. # - -s /app/egress_proxy_addon.py → loads our addon, reads # /etc/egress-proxy/routes.yaml. -# Standalone runs (no EGRESS_PROXY_UPSTREAM_CA) skip the bundle -# build and use mitmproxy's default trust store. -ENTRYPOINT ["sh", "-c", "if [ -n \"$EGRESS_PROXY_UPSTREAM_CA\" ] && [ -f \"$EGRESS_PROXY_UPSTREAM_CA\" ]; then COMBINED=/home/mitmproxy/.mitmproxy/combined-trust.pem; cat /etc/ssl/certs/ca-certificates.crt \"$EGRESS_PROXY_UPSTREAM_CA\" > \"$COMBINED\"; exec mitmdump --mode regular@9099 --set ssl_verify_upstream_trusted_ca=\"$COMBINED\" -s /app/egress_proxy_addon.py; else exec mitmdump --mode regular@9099 -s /app/egress_proxy_addon.py; fi"] +ENTRYPOINT ["sh", "-c", "MODE=\"--mode regular@9099\"; if [ -n \"$EGRESS_PROXY_UPSTREAM_PROXY\" ]; then MODE=\"--mode upstream:$EGRESS_PROXY_UPSTREAM_PROXY --listen-port 9099\"; fi; TRUST_FLAG=\"\"; if [ -n \"$EGRESS_PROXY_UPSTREAM_CA\" ] && [ -f \"$EGRESS_PROXY_UPSTREAM_CA\" ]; then COMBINED=/home/mitmproxy/.mitmproxy/combined-trust.pem; cat /etc/ssl/certs/ca-certificates.crt \"$EGRESS_PROXY_UPSTREAM_CA\" > \"$COMBINED\"; TRUST_FLAG=\"--set ssl_verify_upstream_trusted_ca=$COMBINED\"; fi; exec mitmdump $MODE $TRUST_FLAG -s /app/egress_proxy_addon.py"] diff --git a/claude_bottle/backend/docker/egress_proxy.py b/claude_bottle/backend/docker/egress_proxy.py index 99d6ba4..819b6b0 100644 --- a/claude_bottle/backend/docker/egress_proxy.py +++ b/claude_bottle/backend/docker/egress_proxy.py @@ -229,14 +229,25 @@ class DockerEgressProxy(EgressProxy): "--network-alias", EGRESS_PROXY_HOSTNAME, ] if route_via_pipelock: - # Route egress-proxy's outbound HTTPS through pipelock so - # the egress allowlist + DLP body scanner apply to its - # traffic on the egress-proxy → upstream leg. Pipelock - # MITMs each handshake with its per-bottle CA, which is - # docker-cp'd in below and pointed to via the - # EGRESS_PROXY_UPSTREAM_CA env (entrypoint conditionally - # adds the matching --set flag). + # Route egress-proxy's outbound traffic through pipelock + # so the egress allowlist + DLP body scanner apply to + # the egress-proxy → upstream leg. Pipelock MITMs each + # handshake with its per-bottle CA, which is docker-cp'd + # in below and pointed to via the EGRESS_PROXY_UPSTREAM_CA + # env (entrypoint conditionally adds the matching --set + # flag). + # + # EGRESS_PROXY_UPSTREAM_PROXY is the mechanism: mitmproxy + # does NOT honor HTTPS_PROXY env vars on its outbound + # side (it's a proxy server, not a client). The + # entrypoint reads this env and switches mitmdump to + # `--mode upstream:` so all post-MITM traffic + # CONNECTs to pipelock instead of going direct. The + # HTTPS/HTTP_PROXY env vars below are kept for any + # bundled client libraries (mitmproxy plugin requests, + # etc.) that might honor them — harmless if ignored. create_args.extend([ + "-e", f"EGRESS_PROXY_UPSTREAM_PROXY={plan.pipelock_proxy_url}", "-e", f"HTTPS_PROXY={plan.pipelock_proxy_url}", "-e", f"HTTP_PROXY={plan.pipelock_proxy_url}", "-e", "NO_PROXY=localhost,127.0.0.1", diff --git a/claude_bottle/egress_proxy_addon_core.py b/claude_bottle/egress_proxy_addon_core.py index 36bb341..d2bbfbf 100644 --- a/claude_bottle/egress_proxy_addon_core.py +++ b/claude_bottle/egress_proxy_addon_core.py @@ -190,20 +190,30 @@ def decide( """Pure decision: given a route table + request host + path + env, return what the addon should do with the request. - - No matching route → forward unchanged. Pipelock will - hostname-gate it downstream; egress-proxy does not need to - decide on hosts it doesn't recognise. + - No matching route → BLOCK. The route table is the bottle's + egress allowlist; defense-in-depth complements pipelock's + hostname gate on the downstream leg. A bottle that wants a + host reachable from the agent must declare a route for it + (bare-pass route — no `auth`, no `path_allowlist` — is fine + for hosts that just need passthrough). - Matching route with `path_allowlist` set, request path doesn't start with any of the allowed prefixes → block with a clear reason. - Matching route with an auth pair → forward + inject Authorization. Token comes from `environ[route.token_env]`; - missing/empty values 500 (route declared auth but the secret + missing/empty values block (route declared auth but the secret isn't here — operator misconfig). """ route = match_route(routes, request_host) if route is None: - return Decision(action="forward") + return Decision( + action="block", + reason=( + f"egress-proxy: host {request_host!r} is not in the " + f"bottle's egress_proxy.routes allowlist. Declare a " + f"route for it or remove the request." + ), + ) if route.path_allowlist: if not any(request_path.startswith(p) for p in route.path_allowlist): diff --git a/tests/unit/test_egress_proxy_addon_core.py b/tests/unit/test_egress_proxy_addon_core.py index c58837d..fd9de6e 100644 --- a/tests/unit/test_egress_proxy_addon_core.py +++ b/tests/unit/test_egress_proxy_addon_core.py @@ -152,13 +152,15 @@ class TestMatchRoute(unittest.TestCase): class TestDecide(unittest.TestCase): - def test_no_matching_route_forwards(self): - # Hostnames the operator didn't declare are not the - # egress-proxy's concern; pipelock's hostname allowlist gates - # them downstream. + def test_no_matching_route_blocks(self): + # Defense-in-depth: egress-proxy gates the bottle's allowlist + # too, not just pipelock. Any host the operator didn't declare + # in egress_proxy.routes is 403'd at egress-proxy before it + # ever reaches pipelock. d = decide((), "elsewhere.example", "/anything", {}) - self.assertEqual("forward", d.action) - self.assertIsNone(d.inject_authorization) + self.assertEqual("block", d.action) + self.assertIn("allowlist", d.reason) + self.assertIn("'elsewhere.example'", d.reason) def test_path_allowlist_match_forwards(self): d = decide( From c4cf2453e2c844e7955f40aa87fcf66e582b52c6 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 16:46:23 -0400 Subject: [PATCH 07/18] fix(launch): also set lowercase {http,https,no}_proxy on the agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CVE-2016-5388 ("httpoxy") mitigation: libcurl ignores uppercase HTTP_PROXY for http:// URLs to prevent untrusted CGI HTTP_* headers from hijacking the proxy. Only lowercase http_proxy is honored for HTTP. Without the lowercase var, plain-HTTP requests from the agent skip egress-proxy entirely — they go direct, which is "network unreachable" on the agent's --internal bridge, not the egress-proxy 403 we expect. Confirmed against a live bottle: `curl http://1.1.1.1/` reported "Immediate connect fail for 1.1.1.1: Network is unreachable" instead of the addon's "host not in allowlist" 403. With both cases set the agent's curl honors the proxy and our allowlist enforcement kicks in. Also set lowercase HTTPS_PROXY + NO_PROXY for symmetry. Some tools check one case only; sending both means we don't have to audit which convention each tool uses. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/launch.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/claude_bottle/backend/docker/launch.py b/claude_bottle/backend/docker/launch.py index 5542343..dc98195 100644 --- a/claude_bottle/backend/docker/launch.py +++ b/claude_bottle/backend/docker/launch.py @@ -240,13 +240,25 @@ def _run_agent_container(plan: DockerBottlePlan, internal_network: str) -> str: conflict races by incrementing the suffix (unless the name was user-pinned). Returns the resolved container name.""" proxy_url = _agent_proxy_url(plan) + no_proxy = _agent_no_proxy(plan) + # Set BOTH cases of every *_PROXY var. libcurl's CVE-2016-5388 + # httpoxy mitigation makes it ignore uppercase `HTTP_PROXY` for + # `http://` URLs and only honor lowercase `http_proxy`. Without + # the lowercase var, plain-HTTP requests from the agent bypass + # egress-proxy entirely (going direct, then failing with + # "network unreachable" because the agent's bridge is + # --internal). Lowercase HTTPS_PROXY isn't strictly needed but + # we set it for symmetry — some tools check one or the other. docker_args: list[str] = [ "--rm", "-d", "--name", plan.container_name, "--network", internal_network, "-e", f"HTTPS_PROXY={proxy_url}", "-e", f"HTTP_PROXY={proxy_url}", - "-e", f"NO_PROXY={_agent_no_proxy(plan)}", + "-e", f"https_proxy={proxy_url}", + "-e", f"http_proxy={proxy_url}", + "-e", f"NO_PROXY={no_proxy}", + "-e", f"no_proxy={no_proxy}", # CA trust trio for the agent process. Docker propagates # run-time env into `docker exec`, so `claude` sees these # without per-exec threading. NODE_EXTRA_CA_CERTS points at From fad76d3364e4896535e454f3f645748fec4ac092 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 17:01:12 -0400 Subject: [PATCH 08/18] fix(supervise): stage current-config routes file as routes.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The supervise sidecar mounted a snapshot named routes.json into the agent at /etc/claude-bottle/current-config/routes.json, but the egress-proxy-block tool description (and the live proxy file the apply step writes) say routes.yaml. The agent couldn't find the file at the documented path, composed proposals against stale or empty current state, and reported "routes wasn't updated on disk" because it was looking at the wrong filename. Rename the staged file to routes.yaml so the tool description, the staged snapshot, and the live proxy file all agree on the name. Content stays JSON-in-a-yaml-extension (per PRD 0017 chunk 1's decision: every JSON document is valid YAML, stdlib parsers handle it on both ends). Note: the staged file is still a one-shot snapshot taken at bottle prep time. It does NOT auto-update when the operator approves an egress-proxy-block. Agents that want to verify their proposal took effect should retry the request that triggered the block — a successful upstream response is the real signal. Fixing the snapshot-staleness UX is a separate follow-up. Tests migrated from routes.json → routes.yaml. 364 pass. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/launch.py | 2 +- claude_bottle/supervise.py | 10 ++++++++-- tests/unit/test_dashboard.py | 2 +- tests/unit/test_supervise.py | 10 +++++----- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/claude_bottle/backend/docker/launch.py b/claude_bottle/backend/docker/launch.py index dc98195..f8e559c 100644 --- a/claude_bottle/backend/docker/launch.py +++ b/claude_bottle/backend/docker/launch.py @@ -278,7 +278,7 @@ def _run_agent_container(plan: DockerBottlePlan, internal_network: str) -> str: docker_args.extend(["-e", name]) # PRD 0013: read-only current-config mount so the agent can read - # routes.json / allowlist / Dockerfile before composing a + # routes.yaml / allowlist / Dockerfile before composing a # supervise tool-call proposal. Mounted from the per-bottle # stage_dir/current-config/ populated at prepare time. if plan.supervise_plan is not None: diff --git a/claude_bottle/supervise.py b/claude_bottle/supervise.py index 08a05ab..dbf37c4 100644 --- a/claude_bottle/supervise.py +++ b/claude_bottle/supervise.py @@ -425,7 +425,13 @@ def sha256_hex(content: str) -> str: # Filenames inside the per-bottle current-config dir. The agent reads # these (read-only) from CURRENT_CONFIG_DIR_IN_AGENT and proposes # modified versions back via the three MCP tools. -CURRENT_CONFIG_ROUTES = "routes.json" +# Filename of the staged egress-proxy routes file inside the agent's +# read-only current-config mount. JSON content under a `.yaml` +# extension to match the live file the egress-proxy sidecar reads +# (`/etc/egress-proxy/routes.yaml`) — the egress-proxy-block tool +# description points at this exact path, and the apply step writes +# the new content to the matching live path. +CURRENT_CONFIG_ROUTES = "routes.yaml" CURRENT_CONFIG_ALLOWLIST = "allowlist" CURRENT_CONFIG_DOCKERFILE = "Dockerfile" @@ -437,7 +443,7 @@ class SupervisePlan: `queue_dir` is the host directory bind-mounted into the sidecar at /run/supervise/queue. `current_config_dir` is the host directory bind-mounted (read-only) into the *agent* container at - /etc/claude-bottle/current-config, holding routes.json + allowlist + /etc/claude-bottle/current-config, holding routes.yaml + allowlist + Dockerfile so the agent can read them before composing a proposal. `internal_network` is empty at prepare time; the backend's launch step fills it via dataclasses.replace before diff --git a/tests/unit/test_dashboard.py b/tests/unit/test_dashboard.py index 80e8c24..39ce94a 100644 --- a/tests/unit/test_dashboard.py +++ b/tests/unit/test_dashboard.py @@ -38,7 +38,7 @@ FIXED = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc) def _proposal(slug: str = "dev", tool: str = TOOL_EGRESS_PROXY_BLOCK) -> Proposal: - # Per-tool payload shape: cred-proxy gets routes.json, pipelock + # Per-tool payload shape: cred-proxy gets routes.yaml, pipelock # gets a failed URL (PR #25 follow-up), capability gets a # Dockerfile-ish blob. Match the production dispatch in # PROPOSED_FILE_FIELD. diff --git a/tests/unit/test_supervise.py b/tests/unit/test_supervise.py index f24baa2..9e18c70 100644 --- a/tests/unit/test_supervise.py +++ b/tests/unit/test_supervise.py @@ -297,9 +297,9 @@ class TestDiffAndHash(unittest.TestCase): self.assertEqual("", render_diff("a\nb\n", "a\nb\n")) def test_render_diff_shows_changes(self): - diff = render_diff("a\nb\nc\n", "a\nB\nc\n", label="routes.json") - self.assertIn("routes.json (current)", diff) - self.assertIn("routes.json (proposed)", diff) + diff = render_diff("a\nb\nc\n", "a\nB\nc\n", label="routes.yaml") + self.assertIn("routes.yaml (current)", diff) + self.assertIn("routes.yaml (proposed)", diff) self.assertIn("-b", diff) self.assertIn("+B", diff) @@ -365,7 +365,7 @@ class TestSupervisePrepare(unittest.TestCase): self.assertTrue(plan.current_config_dir.is_dir()) self.assertEqual( '{"routes": [{"path": "/x/"}]}\n', - (plan.current_config_dir / "routes.json").read_text(), + (plan.current_config_dir / "routes.yaml").read_text(), ) self.assertEqual( "example.com\n", @@ -382,7 +382,7 @@ class TestSupervisePrepare(unittest.TestCase): plan = _StubSupervise().prepare("dev", self.stage_dir) self.assertEqual( '{"routes": []}\n', - (plan.current_config_dir / "routes.json").read_text(), + (plan.current_config_dir / "routes.yaml").read_text(), ) From d75d5f3e48d481207ee13ea1161471cc8f2be782 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 17:25:35 -0400 Subject: [PATCH 09/18] fix(egress-proxy-apply): chmod tmp file 0644 so mitmproxy can read post-cp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apply_routes_change wrote the proposed routes via `tempfile.mkstemp` (default mode 0600) then `docker cp`'d into the egress-proxy container. docker cp preserves mode + host uid, so the file landed inside the container as 0600 owned by the host user's uid — which is not the mitmproxy user (uid 1000) the addon runs as. The SIGHUP-triggered reload then failed with PermissionError on the re-read, the old routes table stayed in memory, and the operator-approved route never took effect. Symptoms reported: - Operator approves egress-proxy-block proposal that adds google.com to routes. - Agent retries `curl https://google.com` and still gets 403 "egress-proxy: host 'google.com' is not in the bottle's egress_proxy.routes allowlist." - `docker exec cat /etc/egress-proxy/routes.yaml` returns "Permission denied" (mitmproxy user can't read it, so the reload couldn't either). Fix: chmod 0644 on the host tmp file before docker cp. Mirrors the same pattern in DockerEgressProxy.start which already chmods the original routes.yaml + the CAs before cp. The proposed routes content carries no secrets (tokens live in the egress-proxy container's environ, not the routes file), so 0644 in /tmp for the brief window between write and cp is safe. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/egress_proxy_apply.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index 6fe8b56..0d0e68b 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -80,6 +80,17 @@ def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]: try: with os.fdopen(fd, "w") as f: f.write(new_content) + # mkstemp creates the file with mode 0600. `docker cp` + # preserves mode + host uid into the container, so without + # chmod the file lands as 0600 owned by the host user's uid, + # which inside the container is not mitmproxy (uid 1000) — + # the addon's reload then fails with PermissionError on the + # SIGHUP-triggered re-read and the old routes table stays in + # memory. Bump to 0644 so mitmproxy can read it post-cp; + # the host stage_dir doesn't apply to this tmp file but the + # content isn't secret (no tokens — those live in the + # container's environ), so 0644 in /tmp is fine. + os.chmod(tmp_path, 0o644) cp = subprocess.run( ["docker", "cp", tmp_path, f"{container}:{EGRESS_PROXY_ROUTES_IN_CONTAINER}"], From 1cec0d9aa6c24b8497228b16f49e70ebce19da5f Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 17:34:10 -0400 Subject: [PATCH 10/18] feat(egress-proxy-apply): mirror new route hosts into pipelock allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the operator approves an egress-proxy-block proposal that adds a host to egress-proxy's routes, the request would still 403 downstream at pipelock — pipelock's hostname allowlist is set at bottle launch and doesn't learn about routes added later. The agent saw "Approved" but the very next retry still failed. Fix: `apply_routes_change` now mirrors every host in the proposed routes onto pipelock's allowlist before flipping egress-proxy. Order matters — pipelock first so a pipelock failure doesn't leave egress-proxy in a half-state: 1. Validate the new routes content. 2. Extract the hosts. 3. Merge them onto pipelock's current allowlist (`apply_allowlist_change` — restarts pipelock with the merged yaml). No-op when every host is already present. 4. docker cp the new routes.yaml into egress-proxy + SIGHUP. If pipelock's restart fails, egress-proxy is untouched and the operator gets a clear error pointing at the pipelock half-state. If egress-proxy's update fails after pipelock succeeded, pipelock just has the host pre-allowlisted — harmless extra-permissive until the operator retries. Adds `_hosts_in_routes` helper using the addon's own parser (so the mirrored host set matches exactly what the addon will match on). 4 new unit tests; 368 total pass. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/egress_proxy_apply.py | 73 +++++++++++++++++-- tests/unit/test_egress_proxy_apply.py | 31 ++++++++ 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index 0d0e68b..d828f68 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -8,6 +8,13 @@ egress-proxy-block proposal (or runs the operator-initiated sidecar via `docker cp`, then `docker kill --signal HUP` to make the addon reload without dropping connections. +Also mirrors the new route hosts into pipelock's hostname allowlist +so the downstream leg lets them through — egress-proxy enforces +the path-aware allowlist on the agent leg, pipelock enforces the +hostname allowlist + DLP body scan on the upstream leg, and a +host added to one must be in the other or the request 403s +somewhere along the chain. + Raises EgressProxyApplyError on any failure — the dashboard surfaces the message and keeps the proposal pending so the operator can retry. @@ -23,6 +30,13 @@ from pathlib import Path from ...egress_proxy import EGRESS_PROXY_ROUTES_IN_CONTAINER from ...egress_proxy_addon_core import load_routes from .egress_proxy import egress_proxy_container_name +from .pipelock_apply import ( + PipelockApplyError, + apply_allowlist_change, + fetch_current_allowlist, + parse_allowlist_content, + render_allowlist_content, +) class EgressProxyApplyError(RuntimeError): @@ -60,22 +74,69 @@ def validate_routes_content(content: str) -> None: ) from e +def _hosts_in_routes(content: str) -> list[str]: + """Extract the host list from a routes.yaml content string. + Uses the addon's own parser so any host the addon will match on + also lands in pipelock's allowlist. Returns sorted+deduped.""" + try: + routes = load_routes(content) + except ValueError as e: + raise EgressProxyApplyError( + f"proposed routes.yaml is not valid: {e}" + ) from e + return sorted({r.host for r in routes if r.host}) + + +def _mirror_hosts_to_pipelock(slug: str, hosts: list[str]) -> None: + """Ensure every `hosts` entry is on pipelock's allowlist. Fetches + pipelock's current allowlist, merges, re-applies. No-op if every + host is already present (apply still restarts pipelock if any + host is new). Raises EgressProxyApplyError on pipelock failures + so the caller's diff/audit reflects the half-state.""" + try: + current = fetch_current_allowlist(slug) + existing = parse_allowlist_content(current) + merged = sorted(set(existing) | set(hosts)) + if merged == sorted(existing): + return # nothing to add + apply_allowlist_change(slug, render_allowlist_content(merged)) + except PipelockApplyError as e: + raise EgressProxyApplyError( + f"egress-proxy routes updated but pipelock allowlist " + f"mirror failed: {e}. The request will 403 at pipelock " + f"until pipelock's allowlist is refreshed; retry the " + f"proposal or edit pipelock's allowlist by hand." + ) from e + + def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]: """Apply `new_content` to the egress-proxy sidecar for `slug`: 1. Fetch current routes.yaml (for the before-diff). 2. Validate the new content via the addon's own parser. - 3. Write to a temp file, `docker cp` into the sidecar. - 4. `docker kill --signal HUP` so the addon reloads. + 3. Mirror the route hosts onto pipelock's allowlist (so the + downstream hostname gate lets them through). + 4. Write to a temp file, `docker cp` into the egress-proxy + sidecar. + 5. `docker kill --signal HUP` so the addon reloads. + + Order matters: pipelock first, then egress-proxy. If the + pipelock step fails, egress-proxy hasn't been touched and the + old routes stay live. If the egress-proxy step fails after + pipelock succeeded, pipelock has the host in its allowlist but + egress-proxy doesn't enforce it yet — harmless extra-permissive + state at pipelock, and a re-approval will land the egress-proxy + side. Returns (before, after) where `after` == `new_content`. Raises - EgressProxyApplyError on any step; the existing routes in the - sidecar are unchanged if the failure is before docker cp, and - are reverted in spirit if SIGHUP fails (cp landed but reload - didn't fire — caller's next attempt will SIGHUP again).""" + EgressProxyApplyError on any step.""" container = egress_proxy_container_name(slug) before = fetch_current_routes(slug) validate_routes_content(new_content) + # Pipelock mirror first — if it fails, egress-proxy stays intact + # and the operator gets a clear error about the half-state. + _mirror_hosts_to_pipelock(slug, _hosts_in_routes(new_content)) + fd, tmp_path = tempfile.mkstemp(prefix="cb-routes.", suffix=".yaml") try: with os.fdopen(fd, "w") as f: diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py index eb47b4c..2cd7428 100644 --- a/tests/unit/test_egress_proxy_apply.py +++ b/tests/unit/test_egress_proxy_apply.py @@ -6,6 +6,7 @@ import unittest from claude_bottle.backend.docker.egress_proxy_apply import ( EgressProxyApplyError, + _hosts_in_routes, validate_routes_content, ) @@ -52,5 +53,35 @@ class TestValidateRoutesContent(unittest.TestCase): ) +class TestHostsInRoutes(unittest.TestCase): + def test_extracts_each_unique_host(self): + hosts = _hosts_in_routes( + '{"routes": [{"host": "api.github.com"},' + ' {"host": "github.com"},' + ' {"host": "api.anthropic.com"}]}' + ) + # Sorted+deduped. + self.assertEqual( + ["api.anthropic.com", "api.github.com", "github.com"], + hosts, + ) + + def test_dedupes_same_host(self): + hosts = _hosts_in_routes( + '{"routes": [{"host": "x.example", "path_allowlist": ["/a/"]},' + ' {"host": "x.example", "path_allowlist": ["/b/"]}]}' + ) + self.assertEqual(["x.example"], hosts) + + def test_empty_routes_returns_empty(self): + self.assertEqual([], _hosts_in_routes('{"routes": []}')) + + def test_invalid_routes_raises(self): + # The mirror helper relies on parsing succeeding; bad input + # should error before pipelock is touched. + with self.assertRaises(EgressProxyApplyError): + _hosts_in_routes('{"routes": [{"path": "/no-host/"}]}') + + if __name__ == "__main__": unittest.main() From 3be70eb07a271a028d5c2cf0c2b883772ace2338 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 18:23:01 -0400 Subject: [PATCH 11/18] feat(supervise): list-egress-proxy-routes MCP tool, defaults on egress-proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape the allowlist topology so the egress-proxy is the bottle's single allowlist surface, and replace the agent-side routes/allowlist file mounts with a live MCP tool. Policy change (move defaults to egress-proxy): - `egress_proxy_routes_for_bottle(bottle)` now folds in DEFAULT_ALLOWLIST (the claude-code defaults) and `bottle.egress.allowlist` (user adds) as bare-pass routes (no auth, no path filter), on top of the bottle's `egress_proxy.routes`. Manifest routes win on host collision. - `pipelock_effective_allowlist(bottle)` mirrors egress-proxy's effective host set when egress-proxy is in use. Pipelock is no longer the bottle's primary allowlist authority; it enforces a downstream copy as defense-in-depth + does DLP body scanning. - Split out `egress_proxy_manifest_routes(bottle)` for callers that want just the manifest entries (tests, internal use). - DEFAULT_ALLOWLIST moves from `pipelock.py` to `egress_proxy.py` (pipelock re-imports for the no-egress-proxy fallback path). - Dropped the `egress-proxy` auto-allow on pipelock's allowlist — the agent never dials egress-proxy via the proxy mechanism; pipelock only sees upstream hostnames from egress-proxy's CONNECTs. Introspection endpoint (existing mitmproxy feature): - Egress-proxy addon recognises requests to the magic host `_egress-proxy.local` and synthesizes responses via `flow.response = http.Response.make(...)` — no upstream connection, no allowlist enforcement on the magic host. - `GET /allowlist` returns the in-memory route table as JSON (host + path_allowlist + auth_scheme + token_env per route; no token VALUES). - Smoke-tested end-to-end against a real egress-proxy container. MCP tool (existing supervise plumbing): - New `list-egress-proxy-routes` tool (no inputs, no operator approval). Handler fetches via egress-proxy's introspection endpoint using urllib's ProxyHandler against `EGRESS_PROXY_FORWARD_PROXY`. Returns the JSON payload as the tool's text content; `isError: true` if the proxy is unreachable. - `egress-proxy-block` description now points the agent at `list-egress-proxy-routes` instead of a staged file path. - `pipelock-block` description acknowledges the mirror — agents should prefer `egress-proxy-block` to add hosts; pipelock-block stays for the rare divergence case. Drop agent-side file mounts: - Supervise's `current-config` dir staging no longer writes routes.yaml / allowlist. Only `Dockerfile` remains (capability-block still reads it from `/etc/claude-bottle/current-config/Dockerfile`). - `prepare.py` stops passing `routes_content` / `allowlist_content` to `supervise.prepare`. - `Supervise.prepare` signature simplified to one `dockerfile_content` kwarg. Tests: 400 unit + integration pass. Added coverage for defaults-folding (`TestRoutesForBottleFoldsDefaults`), the new tool definition + handler, and the updated supervise.prepare shape. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/bottle_state.py | 42 +++++++ claude_bottle/backend/docker/prepare.py | 12 +- claude_bottle/egress_proxy.py | 52 ++++++++- claude_bottle/egress_proxy_addon.py | 48 +++++++- claude_bottle/pipelock.py | 83 +++++++------- claude_bottle/supervise.py | 65 +++++------ claude_bottle/supervise_server.py | 109 +++++++++++++++---- tests/integration/test_supervise_sidecar.py | 1 + tests/unit/test_egress_proxy.py | 66 +++++++++-- tests/unit/test_pipelock_allowlist.py | 25 +++-- tests/unit/test_supervise.py | 27 ++--- tests/unit/test_supervise_server.py | 24 +++- 12 files changed, 410 insertions(+), 144 deletions(-) diff --git a/claude_bottle/backend/docker/bottle_state.py b/claude_bottle/backend/docker/bottle_state.py index 19b1bf2..b1ec2f6 100644 --- a/claude_bottle/backend/docker/bottle_state.py +++ b/claude_bottle/backend/docker/bottle_state.py @@ -45,6 +45,13 @@ _STATE_SUBDIR = "state" _PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile" _TRANSCRIPT_SUBDIR = "transcript" _METADATA_NAME = "metadata.json" +# Live-config dir bind-mounted into the supervise sidecar (read-only). +# Host's apply paths keep these files fresh so supervise's +# `list-pipelock-allowlist` / `list-egress-proxy-routes` MCP tools +# return the current state — not a snapshot from launch time. +_LIVE_CONFIG_SUBDIR = "live-config" +LIVE_CONFIG_ROUTES_NAME = "routes.yaml" +LIVE_CONFIG_ALLOWLIST_NAME = "allowlist" # Empty marker file. capability_apply writes it before teardown so # cli.py's session-end cleanup knows to preserve the state dir for # `cli.py resume `. Absent = clean up. @@ -152,6 +159,41 @@ def per_bottle_image_tag(identity: str) -> str: return f"claude-bottle-rebuilt-{identity}:latest" +def live_config_dir(identity: str) -> Path: + """Per-bottle live-config dir. Bind-mounted read-only into the + supervise sidecar; the host's apply paths refresh the files on + every operator approval so the agent's `list-*` MCP tools always + return current state.""" + return bottle_state_dir(identity) / _LIVE_CONFIG_SUBDIR + + +def live_routes_path(identity: str) -> Path: + return live_config_dir(identity) / LIVE_CONFIG_ROUTES_NAME + + +def live_allowlist_path(identity: str) -> Path: + return live_config_dir(identity) / LIVE_CONFIG_ALLOWLIST_NAME + + +def write_live_config( + identity: str, *, routes: str = "", allowlist: str = "", +) -> Path: + """Initialise (or refresh) the live-config dir. Empty-string args + leave the existing file alone (caller passes only what it knows). + Returns the live-config dir path.""" + d = live_config_dir(identity) + d.mkdir(parents=True, exist_ok=True) + if routes: + p = live_routes_path(identity) + p.write_text(routes) + p.chmod(0o644) + if allowlist: + p = live_allowlist_path(identity) + p.write_text(allowlist) + p.chmod(0o644) + return d + + def transcript_snapshot_dir(identity: str) -> Path: """Where capability_apply stashes the agent's transcript before teardown, so the next `cli.py start ` can offer to diff --git a/claude_bottle/backend/docker/prepare.py b/claude_bottle/backend/docker/prepare.py index e8c6b3b..a95c3b6 100644 --- a/claude_bottle/backend/docker/prepare.py +++ b/claude_bottle/backend/docker/prepare.py @@ -15,7 +15,6 @@ from datetime import datetime, timezone from pathlib import Path from ... import pipelock -from ...egress_proxy import egress_proxy_render_routes from ...env import ResolvedEnv, resolve_env from ...log import die from .. import BottleSpec @@ -153,21 +152,18 @@ def resolve_plan( egress_proxy_plan = egress_proxy.prepare(bottle, slug, stage_dir) supervise_plan = None if bottle.supervise: - routes_content = ( - egress_proxy_render_routes(egress_proxy_plan.routes) - if egress_proxy_plan.routes else "" - ) - allowlist_content = "\n".join(pipelock.pipelock_effective_allowlist(bottle)) + "\n" # Current Dockerfile for the agent image. Read from the repo # root; for `--cwd` derived images the base Dockerfile is what # the agent should propose changes against (the derived layer # is just a workspace copy). + # (routes.yaml + pipelock allowlist used to land here too but + # PRD 0017 chunk 3 moved them behind the + # `list-egress-proxy-routes` MCP tool so the agent gets live + # state rather than a launch-time snapshot.) dockerfile_path = Path(__file__).resolve().parent.parent.parent.parent / "Dockerfile" dockerfile_content = dockerfile_path.read_text() if dockerfile_path.is_file() else "" supervise_plan = supervise.prepare( slug, stage_dir, - routes_content=routes_content, - allowlist_content=allowlist_content, dockerfile_content=dockerfile_content, ) resolved = resolve_env(manifest, spec.agent_name) diff --git a/claude_bottle/egress_proxy.py b/claude_bottle/egress_proxy.py index 864827f..75e232c 100644 --- a/claude_bottle/egress_proxy.py +++ b/claude_bottle/egress_proxy.py @@ -127,7 +127,24 @@ class EgressProxyPlan: pipelock_proxy_url: str = "" -def egress_proxy_routes_for_bottle( +# Hosts the agent needs by default for claude-code itself. Folded +# into every bottle's egress-proxy routes table as bare-pass entries +# (no auth, no path filter) so the agent reaches them without each +# bottle having to opt in. Pipelock used to own this list; PRD 0017 +# moves it to egress-proxy because egress-proxy is the primary gate +# now and pipelock's allowlist is mirrored from egress-proxy. +DEFAULT_ALLOWLIST: tuple[str, ...] = ( + "api.anthropic.com", + "statsig.anthropic.com", + "sentry.io", + "claude.ai", + "platform.claude.com", + "downloads.claude.ai", + "raw.githubusercontent.com", +) + + +def egress_proxy_manifest_routes( bottle: Bottle, ) -> tuple[EgressProxyRoute, ...]: """Lift each `bottle.egress_proxy.routes[]` manifest entry into a @@ -138,7 +155,12 @@ def egress_proxy_routes_for_bottle( authenticated route with `token_ref` "GH_PAT" gets `EGRESS_PROXY_TOKEN_0`; a second route with the same `token_ref` shares slot 0. Unauthenticated routes (`auth` omitted) contribute - no slot.""" + no slot. + + Does NOT include the folded-in DEFAULT_ALLOWLIST / + bottle.egress.allowlist bare-pass entries — see + `egress_proxy_routes_for_bottle` for the effective set the + addon enforces.""" out: list[EgressProxyRoute] = [] slot_for_token: dict[str, str] = {} for r in bottle.egress_proxy.routes: @@ -164,6 +186,30 @@ def egress_proxy_routes_for_bottle( return tuple(out) +def egress_proxy_routes_for_bottle( + bottle: Bottle, +) -> tuple[EgressProxyRoute, ...]: + """Effective egress-proxy routes: manifest routes followed by + bare-pass entries for DEFAULT_ALLOWLIST hosts and + `bottle.egress.allowlist` hosts. This is what gets rendered into + routes.yaml + what the addon enforces. + + Manifest routes win over defaults on host collision (manifest + routes carry more specific config — auth, path filter, role + markers). Hostname comparison is case-insensitive.""" + out: list[EgressProxyRoute] = list(egress_proxy_manifest_routes(bottle)) + claimed: set[str] = {r.host.lower() for r in out} + for host in DEFAULT_ALLOWLIST: + if host.lower() not in claimed: + out.append(EgressProxyRoute(host=host)) + claimed.add(host.lower()) + for host in bottle.egress.allowlist: + if host and host.lower() not in claimed: + out.append(EgressProxyRoute(host=host)) + claimed.add(host.lower()) + return tuple(out) + + def egress_proxy_token_env_map( routes: tuple[EgressProxyRoute, ...], ) -> dict[str, str]: @@ -286,11 +332,13 @@ class EgressProxy(ABC): __all__ = [ + "DEFAULT_ALLOWLIST", "EGRESS_PROXY_HOSTNAME", "EGRESS_PROXY_ROUTES_IN_CONTAINER", "EgressProxy", "EgressProxyPlan", "EgressProxyRoute", + "egress_proxy_manifest_routes", "egress_proxy_render_routes", "egress_proxy_resolve_token_values", "egress_proxy_routes_for_bottle", diff --git a/claude_bottle/egress_proxy_addon.py b/claude_bottle/egress_proxy_addon.py index 0c11111..dace611 100644 --- a/claude_bottle/egress_proxy_addon.py +++ b/claude_bottle/egress_proxy_addon.py @@ -26,6 +26,8 @@ build input — not a module the host imports.""" from __future__ import annotations +import dataclasses +import json import os import signal import sys @@ -41,6 +43,16 @@ from egress_proxy_addon_core import Route, decide, is_git_push_request, load_rou DEFAULT_ROUTES_PATH = "/etc/egress-proxy/routes.yaml" +# Magic hostname the addon recognises as an introspection target. +# Requests through the proxy for `_egress-proxy.local/` are +# intercepted and answered with synthetic responses (the addon's +# `request` hook sets `flow.response` before any upstream connection). +# The hostname is not in DNS — only clients dialing through this +# specific egress-proxy can reach it, and only via HTTP (no TLS). +# Used by the supervise sidecar's `list-egress-proxy-routes` MCP +# tool to surface the live route table to the agent. +INTROSPECT_HOST = "_egress-proxy.local" + class EgressProxyAddon: """The mitmproxy addon. One instance per `mitmdump` process; the @@ -84,17 +96,49 @@ class EgressProxyAddon: signal.signal(signal.SIGHUP, handler) + def _serve_introspection(self, flow: http.HTTPFlow, path: str) -> None: + """Synthesize a response for `_egress-proxy.local` requests. + Currently supports `/allowlist` which returns the in-memory + route table as JSON (host, path_allowlist, auth_scheme, + token_env per route — no token VALUES, those live in the + container's environ).""" + if path == "/allowlist": + payload = json.dumps( + {"routes": [dataclasses.asdict(r) for r in self.routes]}, + indent=2, + ).encode("utf-8") + flow.response = http.Response.make( + 200, payload, + {"Content-Type": "application/json"}, + ) + return + flow.response = http.Response.make( + 404, + f"egress-proxy introspection: no such endpoint {path!r}".encode(), + {"Content-Type": "text/plain; charset=utf-8"}, + ) + # mitmproxy's addon API: this method name + signature is how # mitmdump discovers the request hook. def request(self, flow: http.HTTPFlow) -> None: + request_path, _, query = flow.request.path.partition("?") + + # Introspection: requests to the magic `_egress-proxy.local` + # host are answered locally with a synthetic response. Check + # before the strip-auth + route logic — these requests aren't + # real upstream traffic, the agent isn't injecting auth, and + # the addon's own decide() would 403 the magic host (it's + # never in the routes table). + if flow.request.pretty_host == INTROSPECT_HOST: + self._serve_introspection(flow, request_path) + return + # Inbound Authorization is always stripped — the agent cannot # smuggle a stolen token through the proxy. If the matched # route declares an auth pair, a fresh header is injected # below. flow.request.headers.pop("authorization", None) - request_path, _, query = flow.request.path.partition("?") - # Universal HTTPS git-push block. Defense-in-depth: git-gate # (PRD 0008) is the only sanctioned outbound path for git # writes — its pre-receive runs gitleaks. Letting HTTPS push diff --git a/claude_bottle/pipelock.py b/claude_bottle/pipelock.py index ee56d07..8053c18 100644 --- a/claude_bottle/pipelock.py +++ b/claude_bottle/pipelock.py @@ -22,21 +22,14 @@ from dataclasses import dataclass from pathlib import Path from typing import cast -from .egress_proxy import EGRESS_PROXY_HOSTNAME +from .egress_proxy import ( + DEFAULT_ALLOWLIST, + EGRESS_PROXY_HOSTNAME, + egress_proxy_routes_for_bottle, +) from .supervise import SUPERVISE_HOSTNAME from .manifest import Bottle -# Baked-in default allowlist for hosts Claude Code itself needs. -DEFAULT_ALLOWLIST: tuple[str, ...] = ( - "api.anthropic.com", - "statsig.anthropic.com", - "sentry.io", - "claude.ai", - "platform.claude.com", - "downloads.claude.ai", - "raw.githubusercontent.com", -) - # Hosts pipelock should NOT TLS-MITM, even when tls_interception is # enabled. The Claude API endpoint is an LLM provider — its request # bodies are user-authored conversation text that legitimately can @@ -64,43 +57,51 @@ def pipelock_bottle_allowlist(bottle: Bottle) -> list[str]: def pipelock_route_hosts(bottle: Bottle) -> list[str]: """Hostnames declared in `bottle.egress_proxy.routes`. Returned - sorted + deduped. - - Post-cutover topology (PRD 0017): the agent's HTTPS_PROXY points - at egress-proxy, not pipelock; egress-proxy's outbound leg sets - `HTTPS_PROXY=pipelock`. So pipelock no longer terminates the - agent's connections — it sees the egress-proxy → upstream leg - only. Each declared route's host still needs to be on pipelock's - allowlist so that leg can leave the egress network.""" + sorted + deduped. Used by the no-egress-proxy fallback path + below; bottles that DO use egress-proxy include the same hosts + via `egress_proxy_routes_for_bottle`.""" hosts = {r.Host for r in bottle.egress_proxy.routes if r.Host} return sorted(hosts) def pipelock_effective_allowlist(bottle: Bottle) -> list[str]: - """Deduplicated union of: baked-in defaults, bottle.egress.allowlist, - the egress-proxy route hosts (from bottle.egress_proxy.routes), the - egress-proxy sidecar's own hostname when any route is declared, and - the supervise sidecar's hostname when bottle.supervise is enabled. - Sorted for stability. Git upstreams declared in `bottle.git` do NOT - contribute here — git traffic flows through the per-agent git-gate - sidecar (PRD 0008), not pipelock. + """Hostnames pipelock allows. Sorted for stability. - The egress-proxy + supervise hostnames are auto-added because the - sidecars sit on the bottle's internal network alongside the agent; - requests that pass through pipelock for `egress-proxy:9099` or - `supervise:9100` (e.g. when egress-proxy uses HTTPS_PROXY=pipelock - on its upstream leg) would otherwise be 403'd by pipelock's - hostname gate.""" + Two paths, depending on whether the bottle uses egress-proxy: + + - Bottle declares `egress_proxy.routes[]` → agent's HTTPS_PROXY + points at egress-proxy. Egress-proxy is the bottle's primary + allowlist gate (DEFAULT_ALLOWLIST + bottle.egress.allowlist + + manifest routes all live there as bare-pass or full routes, + folded in by `egress_proxy_routes_for_bottle`). Pipelock's + allowlist is then a MIRROR of egress-proxy's hosts — same + set, just serving as the defense-in-depth hostname gate + + DLP scanner on the upstream leg. + + - Bottle has no `egress_proxy.routes[]` → agent talks straight + to pipelock. Pipelock keeps its previous behavior: bake in + DEFAULT_ALLOWLIST + bottle.egress.allowlist for claude-code + defaults. + + The supervise sidecar's hostname is auto-added when supervise + is enabled (sibling-sidecar traffic that flows through pipelock + would otherwise be 403'd). Git upstreams declared in + `bottle.git` do NOT contribute here — git traffic flows + through git-gate (PRD 0008), not pipelock.""" seen: dict[str, None] = {} - for h in DEFAULT_ALLOWLIST: - seen.setdefault(h, None) - for h in pipelock_bottle_allowlist(bottle): - if h: - seen.setdefault(h, None) - for h in pipelock_route_hosts(bottle): - seen.setdefault(h, None) if bottle.egress_proxy.routes: - seen.setdefault(EGRESS_PROXY_HOSTNAME, None) + # Mirror egress-proxy's effective host set — same defaults + # and bottle.egress.allowlist entries are already folded in + # at the egress-proxy layer; we don't add them twice. + for r in egress_proxy_routes_for_bottle(bottle): + if r.host: + seen.setdefault(r.host, None) + else: + for h in DEFAULT_ALLOWLIST: + seen.setdefault(h, None) + for h in pipelock_bottle_allowlist(bottle): + if h: + seen.setdefault(h, None) if bottle.supervise: seen.setdefault(SUPERVISE_HOSTNAME, None) return sorted(seen.keys()) diff --git a/claude_bottle/supervise.py b/claude_bottle/supervise.py index dbf37c4..2237e81 100644 --- a/claude_bottle/supervise.py +++ b/claude_bottle/supervise.py @@ -52,12 +52,24 @@ SUPERVISE_PORT = 9100 TOOL_EGRESS_PROXY_BLOCK = "egress-proxy-block" TOOL_PIPELOCK_BLOCK = "pipelock-block" TOOL_CAPABILITY_BLOCK = "capability-block" +TOOL_LIST_EGRESS_PROXY_ROUTES = "list-egress-proxy-routes" TOOLS: tuple[str, ...] = ( TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, TOOL_CAPABILITY_BLOCK, + TOOL_LIST_EGRESS_PROXY_ROUTES, ) +# The supervise sidecar uses these to query egress-proxy's +# introspection endpoint for the `list-egress-proxy-routes` MCP +# tool. The hostname + port match egress-proxy's docker network +# alias + listen port (see claude_bottle.egress_proxy.EGRESS_PROXY_HOSTNAME +# and backend.docker.egress_proxy.EGRESS_PROXY_PORT — the values +# are inlined here so the in-container supervise_server doesn't +# need to import the egress-proxy package). +EGRESS_PROXY_FORWARD_PROXY = "http://egress-proxy:9099" +EGRESS_PROXY_INTROSPECT_URL = "http://_egress-proxy.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 @@ -422,17 +434,15 @@ def sha256_hex(content: str) -> str: # --- Sidecar plan + abstract lifecycle ------------------------------------- -# Filenames inside the per-bottle current-config dir. The agent reads -# these (read-only) from CURRENT_CONFIG_DIR_IN_AGENT and proposes -# modified versions back via the three MCP tools. -# Filename of the staged egress-proxy routes file inside the agent's -# read-only current-config mount. JSON content under a `.yaml` -# extension to match the live file the egress-proxy sidecar reads -# (`/etc/egress-proxy/routes.yaml`) — the egress-proxy-block tool -# description points at this exact path, and the apply step writes -# the new content to the matching live path. -CURRENT_CONFIG_ROUTES = "routes.yaml" -CURRENT_CONFIG_ALLOWLIST = "allowlist" +# Filename of the staged Dockerfile inside the agent's read-only +# current-config mount. The capability-block tool's description +# points the agent at this exact path so it can read the current +# Dockerfile and propose modifications. +# +# routes.yaml + allowlist used to live here too; PRD 0017 chunk 3 +# moved them behind the `list-egress-proxy-routes` MCP tool (live +# state from egress-proxy's introspection endpoint) so the agent +# always sees current data rather than a launch-time snapshot. CURRENT_CONFIG_DOCKERFILE = "Dockerfile" @@ -442,12 +452,12 @@ class SupervisePlan: `queue_dir` is the host directory bind-mounted into the sidecar at /run/supervise/queue. `current_config_dir` is the host - directory bind-mounted (read-only) into the *agent* container at - /etc/claude-bottle/current-config, holding routes.yaml + allowlist - + Dockerfile so the agent can read them before composing a - proposal. `internal_network` is empty at prepare time; the - backend's launch step fills it via dataclasses.replace before - calling .start.""" + directory bind-mounted (read-only) into the *agent* container + at /etc/claude-bottle/current-config — currently holds only the + Dockerfile snapshot (routes.yaml + allowlist moved to the + `list-egress-proxy-routes` MCP tool). `internal_network` is + empty at prepare time; the backend's launch step fills it via + dataclasses.replace before calling .start.""" slug: str queue_dir: Path @@ -465,8 +475,6 @@ class Supervise(ABC): slug: str, stage_dir: Path, *, - routes_content: str = "", - allowlist_content: str = "", dockerfile_content: str = "", ) -> SupervisePlan: """Stage the per-bottle queue dir on the host and the @@ -477,17 +485,9 @@ class Supervise(ABC): queue_dir.mkdir(parents=True, exist_ok=True) current_config_dir = stage_dir / "current-config" current_config_dir.mkdir(parents=True, exist_ok=True) - (current_config_dir / CURRENT_CONFIG_ROUTES).write_text( - routes_content or '{"routes": []}\n' - ) - (current_config_dir / CURRENT_CONFIG_ALLOWLIST).write_text(allowlist_content) - (current_config_dir / CURRENT_CONFIG_DOCKERFILE).write_text(dockerfile_content) - for name in ( - CURRENT_CONFIG_ROUTES, - CURRENT_CONFIG_ALLOWLIST, - CURRENT_CONFIG_DOCKERFILE, - ): - (current_config_dir / name).chmod(0o644) + dockerfile_path = current_config_dir / CURRENT_CONFIG_DOCKERFILE + dockerfile_path.write_text(dockerfile_content) + dockerfile_path.chmod(0o644) return SupervisePlan( slug=slug, queue_dir=queue_dir, @@ -554,10 +554,8 @@ __all__ = [ "ACTION_OPERATOR_EDIT", "AuditEntry", "COMPONENT_FOR_TOOL", - "CURRENT_CONFIG_ALLOWLIST", "CURRENT_CONFIG_DIR_IN_AGENT", "CURRENT_CONFIG_DOCKERFILE", - "CURRENT_CONFIG_ROUTES", "DEFAULT_POLL_INTERVAL_SEC", "Proposal", "QUEUE_DIR_IN_CONTAINER", @@ -571,8 +569,11 @@ __all__ = [ "Supervise", "SupervisePlan", "TOOLS", + "EGRESS_PROXY_FORWARD_PROXY", + "EGRESS_PROXY_INTROSPECT_URL", "TOOL_CAPABILITY_BLOCK", "TOOL_EGRESS_PROXY_BLOCK", + "TOOL_LIST_EGRESS_PROXY_ROUTES", "TOOL_PIPELOCK_BLOCK", "archive_proposal", "audit_dir", diff --git a/claude_bottle/supervise_server.py b/claude_bottle/supervise_server.py index 1517d4c..a8fd5b0 100644 --- a/claude_bottle/supervise_server.py +++ b/claude_bottle/supervise_server.py @@ -36,7 +36,9 @@ import os import socketserver import sys import typing +import urllib.error import urllib.parse +import urllib.request from dataclasses import dataclass from pathlib import Path @@ -132,16 +134,17 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ "description": ( "Call when egress-proxy refused your HTTPS request — host " "without a matching route, or a path outside the route's " - "path_allowlist (typically a 403 from the proxy). Read " - "the current routes.yaml from " - "/etc/claude-bottle/current-config/routes.yaml, compose " - "a modified version that adds or relaxes the route you " - "need, and pass the full new file plus a justification. " - "The operator approves or rejects in the supervise TUI. " - "On approval the supervisor writes the new routes.yaml " - "on the host and SIGHUPs egress-proxy (the addon's reload " - "swaps the route table atomically without dropping " - "in-flight connections)." + "path_allowlist (typically a 403 from the proxy). First " + "call `list-egress-proxy-routes` to see the current route " + "table; compose a modified version that adds or relaxes " + "the route you need, and pass the full new file plus a " + "justification. The operator approves or rejects in the " + "supervise TUI. On approval the supervisor writes the " + "new routes.yaml on the host, SIGHUPs egress-proxy (the " + "addon's reload swaps the route table atomically without " + "dropping in-flight connections), and mirrors the route " + "hosts onto pipelock's allowlist so the downstream gate " + "lets them through too." ), "inputSchema": { "type": "object", @@ -158,19 +161,41 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ "required": ["routes", "justification"], }, }, + { + "name": _sv.TOOL_LIST_EGRESS_PROXY_ROUTES, + "description": ( + "List the current egress-proxy route table — the bottle's " + "primary egress allowlist. Returns JSON with one entry " + "per allowed host, each carrying its path_allowlist (if " + "any) and whether the proxy injects Authorization for " + "the route. Use this before composing an " + "`egress-proxy-block` proposal so the new routes file " + "extends the live one rather than replacing it. " + "Pipelock's allowlist is a mirror of this set — every " + "host listed here is also reachable through pipelock's " + "downstream hostname gate." + ), + "inputSchema": { + "type": "object", + "properties": {}, + "additionalProperties": False, + }, + }, { "name": _sv.TOOL_PIPELOCK_BLOCK, "description": ( - "Call when pipelock refused your outbound request — host " - "not in the allowlist, connection refused at the egress " - "layer. Pass the full URL you tried to hit (scheme + " - "host + path) plus a justification. The supervisor " - "extracts the hostname and merges it into the bottle's " - "current pipelock allowlist; the path is captured as " - "context for the operator to review (pipelock's allowlist " - "is hostname-only — it can't enforce path-level rules). " - "On approval the supervisor restarts pipelock with the " - "merged allowlist." + "Call when pipelock refused your outbound request and " + "the failing host is genuinely missing from the bottle's " + "allowlist (vs. blocked for DLP reasons — those need a " + "different remediation). In practice pipelock's allowlist " + "is now a mirror of the egress-proxy routes set by " + "`egress-proxy-block`, so prefer that tool when you want " + "to add a host. This tool stays available for the rare " + "case where pipelock and egress-proxy have diverged. " + "Pass the full URL you tried to hit (scheme + host + " + "path); the supervisor extracts the hostname and merges " + "it into pipelock's allowlist. On approval the " + "supervisor restarts pipelock." ), "inputSchema": { "type": "object", @@ -308,15 +333,57 @@ def handle_tools_list(_params: dict[str, object]) -> dict[str, object]: return {"tools": TOOL_DEFINITIONS} +def handle_list_egress_proxy_routes( + _params: dict[str, object], + _config: ServerConfig, +) -> dict[str, object]: + """Fetch the live egress-proxy route table via its + `_egress-proxy.local/allowlist` introspection endpoint. The + request goes through egress-proxy as a forward proxy; the + addon recognises the magic host and synthesizes a response — + no real upstream connection, no allowlist enforcement + against the magic host. Returns the JSON payload as the + tool's text content.""" + proxy_handler = urllib.request.ProxyHandler({ + "http": _sv.EGRESS_PROXY_FORWARD_PROXY, + }) + opener = urllib.request.build_opener(proxy_handler) + try: + with opener.open(_sv.EGRESS_PROXY_INTROSPECT_URL, timeout=5) as resp: + body = resp.read().decode("utf-8") + except (urllib.error.URLError, OSError) as e: + return { + "content": [{ + "type": "text", + "text": ( + f"list-egress-proxy-routes: could not reach " + f"{_sv.EGRESS_PROXY_INTROSPECT_URL!r} via " + f"{_sv.EGRESS_PROXY_FORWARD_PROXY!r}: {e}" + ), + }], + "isError": True, + } + return { + "content": [{"type": "text", "text": body}], + "isError": False, + } + + def handle_tools_call( params: dict[str, object], config: ServerConfig, ) -> dict[str, object]: """Validates the proposal, writes it to the queue, blocks waiting - for a Response, returns the result wrapped in MCP `content`.""" + for a Response, returns the result wrapped in MCP `content`. + + Side-effect-free `list-*` tools short-circuit before the queue/ + blocking machinery — they're read-only introspection that + doesn't need operator approval.""" name = params.get("name") if not isinstance(name, str): raise _RpcError(ERR_INVALID_PARAMS, "tools/call missing 'name'") + if name == _sv.TOOL_LIST_EGRESS_PROXY_ROUTES: + return handle_list_egress_proxy_routes(params.get("arguments", {}), config) if name not in PROPOSED_FILE_FIELD: raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {name!r}") args_raw = params.get("arguments", {}) diff --git a/tests/integration/test_supervise_sidecar.py b/tests/integration/test_supervise_sidecar.py index b4492c3..fc19f23 100644 --- a/tests/integration/test_supervise_sidecar.py +++ b/tests/integration/test_supervise_sidecar.py @@ -199,6 +199,7 @@ class TestSuperviseSidecar(unittest.TestCase): _sv.TOOL_EGRESS_PROXY_BLOCK, _sv.TOOL_PIPELOCK_BLOCK, _sv.TOOL_CAPABILITY_BLOCK, + _sv.TOOL_LIST_EGRESS_PROXY_ROUTES, }, names, ) diff --git a/tests/unit/test_egress_proxy.py b/tests/unit/test_egress_proxy.py index 8b5432a..8ed7207 100644 --- a/tests/unit/test_egress_proxy.py +++ b/tests/unit/test_egress_proxy.py @@ -5,6 +5,8 @@ import json import unittest from claude_bottle.egress_proxy import ( + DEFAULT_ALLOWLIST, + egress_proxy_manifest_routes, egress_proxy_render_routes, egress_proxy_resolve_token_values, egress_proxy_routes_for_bottle, @@ -27,7 +29,7 @@ class TestRoutesForBottle(unittest.TestCase): "host": "api.github.com", "auth": {"scheme": "Bearer", "token_ref": "GH_PAT"}, }]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) self.assertEqual(1, len(routes)) r = routes[0] self.assertEqual("api.github.com", r.host) @@ -38,7 +40,7 @@ class TestRoutesForBottle(unittest.TestCase): def test_unauthenticated_route_has_empty_auth_fields(self): b = _bottle([{"host": "github.com", "path_allowlist": ["/x/"]}]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) r = routes[0] self.assertEqual("", r.auth_scheme) self.assertEqual("", r.token_env) @@ -52,7 +54,7 @@ class TestRoutesForBottle(unittest.TestCase): {"host": "github.com", "auth": {"scheme": "Bearer", "token_ref": "GH_PAT"}}, ]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) slots = {r.token_env for r in routes} self.assertEqual({"EGRESS_PROXY_TOKEN_0"}, slots) @@ -63,7 +65,7 @@ class TestRoutesForBottle(unittest.TestCase): {"host": "b.example", "auth": {"scheme": "Bearer", "token_ref": "T2"}}, ]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) slots = [r.token_env for r in routes] self.assertEqual(["EGRESS_PROXY_TOKEN_0", "EGRESS_PROXY_TOKEN_1"], slots) @@ -77,12 +79,56 @@ class TestRoutesForBottle(unittest.TestCase): {"host": "b.example", "auth": {"scheme": "Bearer", "token_ref": "T2"}}, ]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) authed = [r.token_env for r in routes if r.token_env] self.assertEqual(["EGRESS_PROXY_TOKEN_0", "EGRESS_PROXY_TOKEN_1"], authed) self.assertEqual("", routes[1].token_env) +class TestRoutesForBottleFoldsDefaults(unittest.TestCase): + """The effective route table includes DEFAULT_ALLOWLIST + + bottle.egress.allowlist as bare-pass entries — pipelock's + allowlist is a mirror of this set.""" + + def test_defaults_present_when_no_manifest_routes(self): + b = _bottle([]) + hosts = [r.host for r in egress_proxy_routes_for_bottle(b)] + for default in DEFAULT_ALLOWLIST: + self.assertIn(default, hosts) + + def test_manifest_route_wins_over_default(self): + # api.anthropic.com is in DEFAULT_ALLOWLIST. A manifest + # route for the same host takes precedence — we want the + # auth config to apply, not a duplicate bare-pass entry. + b = _bottle([{ + "host": "api.anthropic.com", + "auth": {"scheme": "Bearer", "token_ref": "T"}, + }]) + routes = egress_proxy_routes_for_bottle(b) + anthropic = [r for r in routes if r.host == "api.anthropic.com"] + self.assertEqual(1, len(anthropic)) + self.assertEqual("Bearer", anthropic[0].auth_scheme) + + def test_bottle_egress_allowlist_folded_in(self): + m = Manifest.from_json_obj({ + "bottles": {"dev": { + "egress_proxy": {"routes": []}, + "egress": {"allowlist": ["example.com"]}, + }}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + hosts = [r.host for r in egress_proxy_routes_for_bottle(m.bottles["dev"])] + self.assertIn("example.com", hosts) + + def test_manifest_only_when_no_defaults_or_allowlist(self): + # Sanity: egress_proxy_manifest_routes returns just the + # manifest entries — defaults are added by the + # _routes_for_bottle wrapper. + b = _bottle([{"host": "x.example"}]) + manifest = [r.host for r in egress_proxy_manifest_routes(b)] + self.assertEqual(["x.example"], manifest) + + class TestTokenEnvMap(unittest.TestCase): def test_only_authenticated_routes_contribute(self): b = _bottle([ @@ -90,7 +136,7 @@ class TestTokenEnvMap(unittest.TestCase): "auth": {"scheme": "Bearer", "token_ref": "T1"}}, {"host": "passthrough.example"}, ]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) m = egress_proxy_token_env_map(routes) self.assertEqual({"EGRESS_PROXY_TOKEN_0": "T1"}, m) @@ -105,7 +151,7 @@ class TestRenderRoutes(unittest.TestCase): "auth": {"scheme": "Bearer", "token_ref": "GH_PAT"}, "path_allowlist": ["/repos/x/"], }]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) payload = json.loads(egress_proxy_render_routes(routes)) self.assertEqual( [{ @@ -123,7 +169,7 @@ class TestRenderRoutes(unittest.TestCase): # enforces both-or-neither, so emitting empty strings would # round-trip as a partial pair and crash. b = _bottle([{"host": "github.com", "path_allowlist": ["/x/"]}]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) payload = json.loads(egress_proxy_render_routes(routes)) entry = payload["routes"][0] self.assertNotIn("auth_scheme", entry) @@ -134,7 +180,7 @@ class TestRenderRoutes(unittest.TestCase): "host": "api.anthropic.com", "auth": {"scheme": "Bearer", "token_ref": "CL"}, }]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) payload = json.loads(egress_proxy_render_routes(routes)) self.assertNotIn("path_allowlist", payload["routes"][0]) @@ -149,7 +195,7 @@ class TestRenderRoutes(unittest.TestCase): {"host": "github.com", "path_allowlist": ["/x/"]}, {"host": "api.anthropic.com"}, ]) - routes = egress_proxy_routes_for_bottle(b) + routes = egress_proxy_manifest_routes(b) addon_routes = load_routes(egress_proxy_render_routes(routes)) self.assertEqual(3, len(addon_routes)) self.assertEqual("Bearer", addon_routes[0].auth_scheme) diff --git a/tests/unit/test_pipelock_allowlist.py b/tests/unit/test_pipelock_allowlist.py index e537356..ea18a01 100644 --- a/tests/unit/test_pipelock_allowlist.py +++ b/tests/unit/test_pipelock_allowlist.py @@ -67,20 +67,29 @@ class TestAllowlistWithRoutes(unittest.TestCase): self.assertIn("registry.npmjs.org", eff) self.assertIn("api.github.com", eff) - def test_egress_proxy_hostname_auto_added_when_routes_exist(self): - # Egress-proxy's outbound leg uses HTTPS_PROXY=pipelock, so - # any request that flows through egress-proxy → pipelock - # would otherwise be rejected by pipelock's hostname gate. + def test_egress_proxy_hostname_NOT_in_pipelock_allowlist(self): + # The agent never dials egress-proxy via the proxy mechanism + # — it IS the proxy. Pipelock receives upstream hostnames + # from egress-proxy's CONNECT requests, not the + # `egress-proxy` hostname itself. eff = pipelock_effective_allowlist(_bottle(_routes([ {"host": "x.example", "auth": {"scheme": "Bearer", "token_ref": "T"}}, ]))) - self.assertIn("egress-proxy", eff) - - def test_egress_proxy_hostname_NOT_added_when_no_routes(self): - eff = pipelock_effective_allowlist(_bottle({})) self.assertNotIn("egress-proxy", eff) + def test_pipelock_mirrors_egress_proxy_defaults_when_routes_present(self): + # When egress_proxy is in use, pipelock's allowlist mirrors + # the egress-proxy effective routes — which fold in + # DEFAULT_ALLOWLIST + bottle.egress.allowlist. + eff = pipelock_effective_allowlist(_bottle(_routes([ + {"host": "x.example", + "auth": {"scheme": "Bearer", "token_ref": "T"}}, + ]))) + for default in ("api.anthropic.com", "sentry.io"): + self.assertIn(default, eff) + self.assertIn("x.example", eff) + def test_supervise_hostname_auto_added_when_supervise_enabled(self): # The agent's MCP client opens long-polled requests to # http://supervise:9100/. They bypass the agent's HTTP_PROXY diff --git a/tests/unit/test_supervise.py b/tests/unit/test_supervise.py index 9e18c70..75d6f71 100644 --- a/tests/unit/test_supervise.py +++ b/tests/unit/test_supervise.py @@ -314,7 +314,12 @@ class TestDiffAndHash(unittest.TestCase): class TestToolConstants(unittest.TestCase): def test_tools_tuple_matches_individual_constants(self): self.assertEqual( - (TOOL_EGRESS_PROXY_BLOCK, TOOL_PIPELOCK_BLOCK, TOOL_CAPABILITY_BLOCK), + ( + TOOL_EGRESS_PROXY_BLOCK, + TOOL_PIPELOCK_BLOCK, + TOOL_CAPABILITY_BLOCK, + supervise.TOOL_LIST_EGRESS_PROXY_ROUTES, + ), supervise.TOOLS, ) @@ -357,20 +362,10 @@ class TestSupervisePrepare(unittest.TestCase): def test_prepare_creates_queue_and_current_config(self): plan = _StubSupervise().prepare( "dev", self.stage_dir, - routes_content='{"routes": [{"path": "/x/"}]}\n', - allowlist_content="example.com\n", dockerfile_content="FROM python:3.13\n", ) self.assertTrue(plan.queue_dir.is_dir()) self.assertTrue(plan.current_config_dir.is_dir()) - self.assertEqual( - '{"routes": [{"path": "/x/"}]}\n', - (plan.current_config_dir / "routes.yaml").read_text(), - ) - self.assertEqual( - "example.com\n", - (plan.current_config_dir / "allowlist").read_text(), - ) self.assertEqual( "FROM python:3.13\n", (plan.current_config_dir / "Dockerfile").read_text(), @@ -378,12 +373,12 @@ class TestSupervisePrepare(unittest.TestCase): self.assertEqual("dev", plan.slug) self.assertEqual("", plan.internal_network) - def test_prepare_defaults_routes_to_empty_when_absent(self): + def test_prepare_only_writes_dockerfile_to_current_config(self): + # routes.yaml + allowlist live behind the + # `list-egress-proxy-routes` MCP tool now (PRD 0017 chunk 3). plan = _StubSupervise().prepare("dev", self.stage_dir) - self.assertEqual( - '{"routes": []}\n', - (plan.current_config_dir / "routes.yaml").read_text(), - ) + files = sorted(p.name for p in plan.current_config_dir.iterdir()) + self.assertEqual(["Dockerfile"], files) if __name__ == "__main__": diff --git a/tests/unit/test_supervise_server.py b/tests/unit/test_supervise_server.py index ef7616c..d4616f6 100644 --- a/tests/unit/test_supervise_server.py +++ b/tests/unit/test_supervise_server.py @@ -170,7 +170,7 @@ class TestHandleInitialize(unittest.TestCase): class TestHandleToolsList(unittest.TestCase): - def test_returns_three_tools(self): + def test_returns_all_tools(self): result = handle_tools_list({}) names = [t["name"] for t in result["tools"]] # type: ignore[index] self.assertEqual( @@ -178,19 +178,35 @@ class TestHandleToolsList(unittest.TestCase): _sv.TOOL_EGRESS_PROXY_BLOCK, _sv.TOOL_PIPELOCK_BLOCK, _sv.TOOL_CAPABILITY_BLOCK, + _sv.TOOL_LIST_EGRESS_PROXY_ROUTES, ]), sorted(names), ) - def test_each_tool_has_inputSchema_with_two_required_fields(self): + def test_remediation_tools_have_inputSchema_with_two_required_fields(self): + # Only the proposal/remediation tools have required input + # fields. The list-* introspection tools take no input. for tool in TOOL_DEFINITIONS: - with self.subTest(name=tool["name"]): + name = tool["name"] + if name not in PROPOSED_FILE_FIELD: + continue + with self.subTest(name=name): schema = tool["inputSchema"] self.assertEqual("object", schema["type"]) # type: ignore[index] required = schema["required"] # type: ignore[index] self.assertEqual(2, len(required)) self.assertIn("justification", required) - self.assertIn(PROPOSED_FILE_FIELD[tool["name"]], required) # type: ignore[index] + self.assertIn(PROPOSED_FILE_FIELD[name], required) # type: ignore[index] + + def test_list_egress_proxy_routes_takes_no_input(self): + tool = next( + t for t in TOOL_DEFINITIONS + if t["name"] == _sv.TOOL_LIST_EGRESS_PROXY_ROUTES + ) + schema = tool["inputSchema"] + self.assertEqual({}, schema.get("properties")) # type: ignore[union-attr] + # No `required` array because no inputs are required. + self.assertNotIn("required", schema) # type: ignore[operator] class TestHandleToolsCall(unittest.TestCase): From 1542ee0b93907743a45eafd71fbb12e7fb5f845f Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 18:45:17 -0400 Subject: [PATCH 12/18] feat(egress-proxy-block): single-route input + merge-on-apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of asking the agent to compose and submit a full routes file, the tool now takes ONE proposed route — host + optional path_allowlist + optional auth — and the supervisor merges it into the live routes table at approval time. The agent no longer needs to fetch / reproduce / extend the existing allowlist; it just describes the host it wants reachable. Tool input (new): - `host` (required) - `path_allowlist` (optional, array of absolute path prefixes) - `auth` (optional, {scheme, token_ref}) - `justification` (required) Merge semantics (in `egress_proxy_apply._merge_single_route`): - Host NOT in current routes → append the proposed route as a new entry. If `auth` is set, assign the next EGRESS_PROXY_TOKEN_N slot. - Host already present → union the proposed `path_allowlist` with the existing one (proposed entries appended after existing, deduped). Existing `auth_scheme` / `token_env` preserved; proposed `auth` ignored (operator-controlled, not agent-controlled). - Hostname comparison is case-insensitive. Dashboard wiring: `approve()` on an egress-proxy-block proposal now calls `add_route(slug, proposed_route_json)` instead of `apply_routes_change(slug, full_file)`. add_route fetches the current routes from the running egress-proxy, merges, and calls apply_routes_change with the merged content — so the pipelock-mirror + SIGHUP plumbing from chunk 3 still runs end-to-end. Audit diff still captures the full-file before/after. Tool description rewritten to make the new shape obvious and to stop pointing the agent at the routes file. The `list-egress-proxy-routes` tool stays available for agents that want to see what's currently allowed. Tests: 9 new `_merge_single_route` cases (host absent/present, path-allowlist union+dedup, auth-slot indexing, case-insensitive match, existing-auth preservation, missing-host rejection, malformed-current rejection). 407 unit + integration pass. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/egress_proxy_apply.py | 110 +++++++++++ claude_bottle/cli/dashboard.py | 9 +- claude_bottle/supervise_server.py | 183 ++++++++++++++---- tests/integration/test_supervise_sidecar.py | 16 +- tests/unit/test_dashboard.py | 61 +++--- tests/unit/test_egress_proxy_apply.py | 106 ++++++++++ tests/unit/test_supervise_server.py | 30 +-- 7 files changed, 419 insertions(+), 96 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index d828f68..535552b 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -22,6 +22,7 @@ operator can retry. from __future__ import annotations +import json import os import subprocess import tempfile @@ -180,8 +181,117 @@ def apply_routes_change(slug: str, new_content: str) -> tuple[str, str]: 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 + content, returning the merged JSON-as-yaml string. + + Behavior: + - If `new_route['host']` is NOT in the current routes → + append the route. + - If the host IS already present → union the path_allowlist + entries (proposed ∪ existing). The existing `auth_scheme` + and `token_env` are preserved — agent-proposed auth changes + on an existing host are ignored, matching the tool's + documented semantics. + + The supervisor renders the merged routes.yaml with the same + JSON layout the addon expects (host + path_allowlist + + auth_scheme + token_env). Token VALUES never appear here; the + routes file carries only env-var slot NAMES.""" + try: + cfg = json.loads(current_yaml) + except json.JSONDecodeError as e: + raise EgressProxyApplyError( + f"current routes.yaml is not valid JSON: {e}" + ) from e + routes = cfg.get("routes") + if not isinstance(routes, list): + raise EgressProxyApplyError( + "current routes.yaml: 'routes' is not a list" + ) + + new_host = str(new_route.get("host", "")).lower() + if not new_host: + raise EgressProxyApplyError( + "proposed route is missing 'host'" + ) + + proposed_paths = list(new_route.get("path_allowlist") or []) + + # Look for an existing entry with the same host (case-insensitive). + for entry in routes: + if not isinstance(entry, dict): + continue + if str(entry.get("host", "")).lower() == new_host: + # Merge path_allowlist: union proposed + existing, ordered + # by first-seen so existing paths stay in original order. + existing_paths: list[str] = list(entry.get("path_allowlist") or []) + seen = {p: None for p in existing_paths} + for p in proposed_paths: + seen.setdefault(p, None) + merged_paths = list(seen.keys()) + if merged_paths: + entry["path_allowlist"] = merged_paths + # Preserve existing auth — tool description says agent- + # proposed auth on an existing host is ignored. + break + else: + # Host not present; build a new route entry from the + # proposed fields. Need to assign a token_env slot if + # `auth` was proposed (otherwise the addon's parser rejects + # a half-set auth pair). Slots: count existing slots, pick + # the next free index. + entry = {"host": new_route["host"]} + if proposed_paths: + entry["path_allowlist"] = proposed_paths + auth = new_route.get("auth") + if isinstance(auth, dict) and auth.get("scheme") and auth.get("token_ref"): + existing_slots = sorted({ + str(r.get("token_env")) + for r in routes + if isinstance(r, dict) and r.get("token_env") + }) + next_idx = len(existing_slots) + entry["auth_scheme"] = str(auth["scheme"]) + entry["token_env"] = f"EGRESS_PROXY_TOKEN_{next_idx}" + # NOTE: the addon reads token VALUES from its container's + # environ keyed by token_env. A newly-added auth route at + # runtime points at a slot that has no env value → the + # addon will 403 with "token env unset" until the operator + # arranges for the value to land in the container's env. + # Recording this here so the operator-facing diff carries + # the slot name they'll need to provision. + routes.append(entry) + + cfg["routes"] = routes + return json.dumps(cfg, indent=2) + "\n" + + +def add_route(slug: str, proposed_route_json: str) -> tuple[str, str]: + """Apply a single-route addition to the egress-proxy. Parses the + agent's proposed route, fetches the current routes file, merges, + and applies via `apply_routes_change`. Returns (before, after) + full-file content for the audit log.""" + try: + proposed = json.loads(proposed_route_json) + except json.JSONDecodeError as e: + raise EgressProxyApplyError( + f"proposed route is not valid JSON: {e}" + ) from e + if not isinstance(proposed, dict): + raise EgressProxyApplyError( + "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__ = [ "EgressProxyApplyError", + "add_route", "apply_routes_change", "fetch_current_routes", "validate_routes_content", diff --git a/claude_bottle/cli/dashboard.py b/claude_bottle/cli/dashboard.py index f41db21..f94b0b7 100644 --- a/claude_bottle/cli/dashboard.py +++ b/claude_bottle/cli/dashboard.py @@ -29,6 +29,7 @@ from ..backend.docker.capability_apply import ( ) from ..backend.docker.egress_proxy_apply import ( EgressProxyApplyError, + add_route, apply_routes_change, fetch_current_routes, ) @@ -166,7 +167,13 @@ def approve( diff_before, diff_after = "", "" if qp.proposal.tool == TOOL_EGRESS_PROXY_BLOCK: - diff_before, diff_after = apply_routes_change( + # The proposal is a single-route JSON; add_route fetches the + # current routes from the running egress-proxy, merges the + # new route in, and applies the full merged file. The + # audit log gets the BEFORE/AFTER of the full file so the + # diff renders cleanly even though the agent only proposed + # one entry. + diff_before, diff_after = add_route( qp.proposal.bottle_slug, file_to_apply, ) elif qp.proposal.tool == TOOL_PIPELOCK_BLOCK: diff --git a/claude_bottle/supervise_server.py b/claude_bottle/supervise_server.py index a8fd5b0..dd455e9 100644 --- a/claude_bottle/supervise_server.py +++ b/claude_bottle/supervise_server.py @@ -134,31 +134,61 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ "description": ( "Call when egress-proxy refused your HTTPS request — host " "without a matching route, or a path outside the route's " - "path_allowlist (typically a 403 from the proxy). First " - "call `list-egress-proxy-routes` to see the current route " - "table; compose a modified version that adds or relaxes " - "the route you need, and pass the full new file plus a " - "justification. The operator approves or rejects in the " - "supervise TUI. On approval the supervisor writes the " - "new routes.yaml on the host, SIGHUPs egress-proxy (the " - "addon's reload swaps the route table atomically without " - "dropping in-flight connections), and mirrors the route " - "hosts onto pipelock's allowlist so the downstream gate " - "lets them through too." + "path_allowlist (typically a 403 from the proxy). Propose " + "a SINGLE route to add: the host you need + (optionally) " + "a path_allowlist + (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, and you do not pass a full routes file. " + "If the host already has a route, the proposed " + "path_allowlist entries 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, SIGHUPs " + "egress-proxy (atomic swap, no dropped connections), and " + "mirrors the host onto pipelock's allowlist for the " + "downstream gate." ), "inputSchema": { "type": "object", "properties": { - "routes": { + "host": { "type": "string", - "description": "Full proposed routes.yaml file content (JSON text — every JSON document is valid YAML).", + "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)." + ), + }, + "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 routes change is justified.", + "description": "Why this host needs to be allowed.", }, }, - "required": ["routes", "justification"], + "required": ["host", "justification"], }, }, { @@ -252,15 +282,22 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [ # tool-specific payload (stored in Proposal.proposed_file as # free-form text the apply path interprets per tool). # -# egress-proxy-block: full proposed routes.yaml +# egress-proxy-block: JSON object describing a SINGLE route to +# add — `{host, path_allowlist?, auth?}`. The +# supervisor merges this into the live routes +# file at approval time. # pipelock-block: the full failed URL (scheme + host + path) — # supervisor extracts the host, merges into the # bottle's current allowlist; the path is shown # to the operator for context (pipelock doesn't # do path-level matching). # capability-block: full proposed Dockerfile +# +# Egress-proxy-block doesn't use a single "field name" → the JSON +# payload is constructed from multiple structured input fields in +# `handle_egress_proxy_block`. The mapping stays one-entry-per-tool +# so the generic dispatch keeps working for the other two. PROPOSED_FILE_FIELD: dict[str, str] = { - _sv.TOOL_EGRESS_PROXY_BLOCK: "routes", _sv.TOOL_PIPELOCK_BLOCK: "failed_url", _sv.TOOL_CAPABILITY_BLOCK: "dockerfile", } @@ -269,26 +306,18 @@ PROPOSED_FILE_FIELD: dict[str, str] = { # --- Validation ------------------------------------------------------------ +# Auth schemes accepted on egress-proxy-block proposals — match the +# manifest-side EGRESS_PROXY_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 enter the queue.""" if not content.strip(): raise _RpcError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty") - if tool == _sv.TOOL_EGRESS_PROXY_BLOCK: - try: - parsed = json.loads(content) - except json.JSONDecodeError as e: - raise _RpcError( - ERR_INVALID_PARAMS, - f"{tool}: proposed routes.yaml is not valid JSON: {e}", - ) from e - if not isinstance(parsed, dict) or not isinstance(parsed.get("routes"), list): - raise _RpcError( - ERR_INVALID_PARAMS, - f"{tool}: proposed routes.yaml must be an object with a 'routes' array", - ) - elif tool == _sv.TOOL_PIPELOCK_BLOCK: + if tool == _sv.TOOL_PIPELOCK_BLOCK: # `content` is the full failed URL. Require scheme + host so # the supervisor can extract a hostname for the allowlist # merge; the path is preserved for operator context. @@ -312,6 +341,70 @@ 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-proxy-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_PROXY_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 ---------------------------------------------------------- @@ -384,27 +477,35 @@ def handle_tools_call( raise _RpcError(ERR_INVALID_PARAMS, "tools/call missing 'name'") if name == _sv.TOOL_LIST_EGRESS_PROXY_ROUTES: return handle_list_egress_proxy_routes(params.get("arguments", {}), config) - if name not in PROPOSED_FILE_FIELD: - raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {name!r}") + args_raw = params.get("arguments", {}) if not isinstance(args_raw, dict): raise _RpcError(ERR_INVALID_PARAMS, "tools/call 'arguments' must be an object") - file_field = PROPOSED_FILE_FIELD[name] - proposed_file = args_raw.get(file_field) justification = args_raw.get("justification") - if not isinstance(proposed_file, str): - raise _RpcError( - ERR_INVALID_PARAMS, - f"{name}: '{file_field}' is required and must be a string", - ) if not isinstance(justification, str) or not justification.strip(): raise _RpcError( ERR_INVALID_PARAMS, f"{name}: 'justification' is required and must be a non-empty string", ) - validate_proposed_file(name, proposed_file) + if name == _sv.TOOL_EGRESS_PROXY_BLOCK: + # Structured input → JSON bundle on Proposal.proposed_file. + # The dashboard's apply step (egress_proxy_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: + file_field = PROPOSED_FILE_FIELD[name] + proposed_file = args_raw.get(file_field) + if not isinstance(proposed_file, str): + raise _RpcError( + ERR_INVALID_PARAMS, + f"{name}: '{file_field}' is required and must be a string", + ) + validate_proposed_file(name, proposed_file) + else: + raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {name!r}") proposal = _sv.Proposal.new( bottle_slug=config.bottle_slug, diff --git a/tests/integration/test_supervise_sidecar.py b/tests/integration/test_supervise_sidecar.py index fc19f23..eb33e5e 100644 --- a/tests/integration/test_supervise_sidecar.py +++ b/tests/integration/test_supervise_sidecar.py @@ -218,13 +218,13 @@ class TestSuperviseSidecar(unittest.TestCase): self._bring_up_sidecar() # Stub the apply step. The dashboard's approve() calls - # apply_routes_change to docker-exec into the egress-proxy - # sidecar; this test isn't exercising the real sidecar, so - # patch it to a no-op that returns plausible before/after - # strings the audit-log writer can render. + # add_route to docker-exec into the egress-proxy sidecar; + # this test isn't exercising the real sidecar, so patch it + # to a no-op that returns plausible before/after strings + # the audit-log writer can render. from claude_bottle.cli import dashboard as _dash - original_apply = _dash.apply_routes_change - _dash.apply_routes_change = ( + original_apply = _dash.add_route + _dash.add_route = ( lambda slug, new: ("(stubbed before)", new) ) @@ -236,7 +236,7 @@ class TestSuperviseSidecar(unittest.TestCase): "params": { "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { - "routes": '{"routes": [{"host": "api.example.com"}]}', + "host": "api.example.com", "justification": "integration test", }, }, @@ -270,7 +270,7 @@ class TestSuperviseSidecar(unittest.TestCase): # file and returns to the curl caller. dashboard.approve(qp, notes="lgtm from integration test") finally: - _dash.apply_routes_change = original_apply + _dash.add_route = original_apply t.join(timeout=20) response = captured.get("response") diff --git a/tests/unit/test_dashboard.py b/tests/unit/test_dashboard.py index 39ce94a..834d9ce 100644 --- a/tests/unit/test_dashboard.py +++ b/tests/unit/test_dashboard.py @@ -127,14 +127,14 @@ class TestDiscoverPending(_FakeHomeMixin, unittest.TestCase): class TestApproveReject(_FakeHomeMixin, unittest.TestCase): def setUp(self): self._setup_fake_home() - self._original_apply_routes = dashboard.apply_routes_change + self._original_add_route = dashboard.add_route self._original_apply_allowlist = dashboard.apply_allowlist_change self._original_fetch_allowlist = dashboard.fetch_current_allowlist self._original_apply_capability = dashboard.apply_capability_change # Default stubs: succeed with deterministic before/after so the # audit log shows a non-empty diff. - dashboard.apply_routes_change = lambda slug, content: ( - '{"routes": []}\n', content, + dashboard.add_route = lambda slug, content: ( + '{"routes": []}\n', '{"routes": [{"host": "x"}]}\n', ) dashboard.apply_allowlist_change = lambda slug, content: ( "old.example\n", content, @@ -145,7 +145,7 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): ) def tearDown(self): - dashboard.apply_routes_change = self._original_apply_routes + dashboard.add_route = self._original_add_route dashboard.apply_allowlist_change = self._original_apply_allowlist dashboard.fetch_current_allowlist = self._original_fetch_allowlist dashboard.apply_capability_change = self._original_apply_capability @@ -204,19 +204,19 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase): class TestEgressProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): - """PRD 0014 Phase 3: approve() on a egress-proxy-block proposal - must call apply_routes_change with the right args and surface - its failures.""" + """PRD 0017 chunk 3: approve() on an egress-proxy-block proposal + must call add_route (single-route merge) with the right args + and surface its failures.""" def setUp(self): self._setup_fake_home() - self._original_apply = dashboard.apply_routes_change + self._original_add_route = dashboard.add_route def tearDown(self): - dashboard.apply_routes_change = self._original_apply + dashboard.add_route = self._original_add_route self._teardown_fake_home() - def _enqueue_egress_proxy(self, proposed: str = '{"routes": []}\n'): + def _enqueue_egress_proxy(self, proposed: str = '{"host": "x.example"}\n'): p = Proposal.new( bottle_slug="dev", tool=TOOL_EGRESS_PROXY_BLOCK, proposed_file=proposed, @@ -229,29 +229,40 @@ class TestEgressProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): supervise.write_proposal(qdir, p) return dashboard.QueuedProposal(proposal=p, queue_dir=qdir) - def test_egress_proxy_block_calls_apply_with_proposed_file(self): + def test_egress_proxy_block_calls_add_route_with_proposed_json(self): calls = [] - dashboard.apply_routes_change = lambda slug, content: ( - calls.append((slug, content)) or ("before", content) + dashboard.add_route = lambda slug, content: ( + calls.append((slug, content)) or ("before", "after") + ) + qp = self._enqueue_egress_proxy( + proposed='{"host": "new.example", "path_allowlist": ["/x/"]}\n' ) - qp = self._enqueue_egress_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') dashboard.approve(qp) self.assertEqual(1, len(calls)) slug, content = calls[0] self.assertEqual("dev", slug) - self.assertEqual('{"routes": [{"path": "/new/"}]}\n', content) + # The single-route JSON the agent proposed reaches add_route + # unchanged — add_route fetches current state + merges. + self.assertEqual( + '{"host": "new.example", "path_allowlist": ["/x/"]}\n', + content, + ) - def test_modify_passes_final_file_to_apply(self): + def test_modify_passes_final_file_to_add_route(self): calls = [] - dashboard.apply_routes_change = lambda slug, content: ( - calls.append(content) or ("before", content) + dashboard.add_route = lambda slug, content: ( + calls.append(content) or ("before", "after") ) qp = self._enqueue_egress_proxy() - dashboard.approve(qp, final_file='{"routes": [{"path": "/edited/"}]}\n', notes="tweaked") - self.assertEqual(['{"routes": [{"path": "/edited/"}]}\n'], calls) + dashboard.approve( + qp, + final_file='{"host": "edited.example"}\n', + notes="tweaked", + ) + self.assertEqual(['{"host": "edited.example"}\n'], calls) def test_apply_failure_blocks_response_and_audit(self): - dashboard.apply_routes_change = lambda slug, content: (_ for _ in ()).throw( + dashboard.add_route = lambda slug, content: (_ for _ in ()).throw( EgressProxyApplyError("docker exec failed") ) qp = self._enqueue_egress_proxy() @@ -266,15 +277,15 @@ class TestEgressProxyApplyWiring(_FakeHomeMixin, unittest.TestCase): self.assertEqual([], read_audit_entries("egress-proxy", "dev")) def test_real_diff_lands_in_audit(self): - dashboard.apply_routes_change = lambda slug, content: ( + dashboard.add_route = lambda slug, content: ( '{"routes": []}\n', # before - '{"routes": [{"path": "/new/"}]}\n', # after + '{"routes": [{"host": "new.example"}]}\n', # after ) - qp = self._enqueue_egress_proxy(proposed='{"routes": [{"path": "/new/"}]}\n') + qp = self._enqueue_egress_proxy(proposed='{"host": "new.example"}\n') dashboard.approve(qp) entries = read_audit_entries("egress-proxy", "dev") self.assertEqual(1, len(entries)) - self.assertIn('+{"routes": [{"path": "/new/"}]}', entries[0].diff) + self.assertIn('+{"routes": [{"host": "new.example"}]}', entries[0].diff) self.assertIn('-{"routes": []}', entries[0].diff) def test_reject_does_not_call_apply(self): diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py index 2cd7428..f300db1 100644 --- a/tests/unit/test_egress_proxy_apply.py +++ b/tests/unit/test_egress_proxy_apply.py @@ -4,9 +4,12 @@ integration test.""" import unittest +import json + from claude_bottle.backend.docker.egress_proxy_apply import ( EgressProxyApplyError, _hosts_in_routes, + _merge_single_route, validate_routes_content, ) @@ -83,5 +86,108 @@ class TestHostsInRoutes(unittest.TestCase): _hosts_in_routes('{"routes": [{"path": "/no-host/"}]}') +class TestMergeSingleRoute(unittest.TestCase): + BASE = '{"routes": [{"host": "api.anthropic.com"}]}' + + def test_appends_route_when_host_absent(self): + merged = _merge_single_route(self.BASE, {"host": "github.com"}) + routes = json.loads(merged)["routes"] + hosts = [r["host"] for r in routes] + self.assertEqual(["api.anthropic.com", "github.com"], hosts) + + def test_appends_path_allowlist(self): + merged = _merge_single_route( + self.BASE, + {"host": "github.com", "path_allowlist": ["/repos/x/"]}, + ) + new_route = json.loads(merged)["routes"][-1] + self.assertEqual(["/repos/x/"], new_route["path_allowlist"]) + + def test_appends_auth_with_token_env_slot(self): + merged = _merge_single_route( + self.BASE, + { + "host": "api.github.com", + "auth": {"scheme": "Bearer", "token_ref": "GH"}, + }, + ) + new_route = json.loads(merged)["routes"][-1] + self.assertEqual("Bearer", new_route["auth_scheme"]) + # First auth slot when no prior auth routes exist. + self.assertEqual("EGRESS_PROXY_TOKEN_0", new_route["token_env"]) + + def test_auth_slot_increments_past_existing(self): + base = json.dumps({"routes": [ + {"host": "api.anthropic.com", + "auth_scheme": "Bearer", + "token_env": "EGRESS_PROXY_TOKEN_0"}, + ]}) + merged = _merge_single_route(base, { + "host": "api.github.com", + "auth": {"scheme": "Bearer", "token_ref": "GH"}, + }) + new_route = json.loads(merged)["routes"][-1] + self.assertEqual("EGRESS_PROXY_TOKEN_1", new_route["token_env"]) + + def test_existing_host_merges_path_allowlist_as_union(self): + base = json.dumps({"routes": [ + {"host": "github.com", "path_allowlist": ["/a/"]}, + ]}) + merged = _merge_single_route(base, { + "host": "github.com", + "path_allowlist": ["/b/"], + }) + routes = json.loads(merged)["routes"] + self.assertEqual(1, len(routes)) # not duplicated + self.assertEqual(["/a/", "/b/"], routes[0]["path_allowlist"]) + + def test_existing_host_dedup_path_allowlist(self): + base = json.dumps({"routes": [ + {"host": "github.com", "path_allowlist": ["/a/"]}, + ]}) + merged = _merge_single_route(base, { + "host": "github.com", + "path_allowlist": ["/a/", "/b/"], + }) + self.assertEqual( + ["/a/", "/b/"], + json.loads(merged)["routes"][0]["path_allowlist"], + ) + + def test_existing_host_preserves_existing_auth_ignores_proposed(self): + # Tool docs: auth on an existing host is operator-controlled, + # not agent-controlled. The merge must not overwrite. + base = json.dumps({"routes": [ + {"host": "api.github.com", + "auth_scheme": "Bearer", + "token_env": "EGRESS_PROXY_TOKEN_0"}, + ]}) + merged = _merge_single_route(base, { + "host": "api.github.com", + "auth": {"scheme": "token", "token_ref": "DIFFERENT"}, + }) + route = json.loads(merged)["routes"][0] + self.assertEqual("Bearer", route["auth_scheme"]) + self.assertEqual("EGRESS_PROXY_TOKEN_0", route["token_env"]) + + def test_host_match_is_case_insensitive(self): + base = json.dumps({"routes": [{"host": "GitHub.com"}]}) + merged = _merge_single_route(base, { + "host": "github.com", + "path_allowlist": ["/x/"], + }) + routes = json.loads(merged)["routes"] + self.assertEqual(1, len(routes)) + self.assertEqual(["/x/"], routes[0]["path_allowlist"]) + + def test_missing_host_raises(self): + with self.assertRaises(EgressProxyApplyError): + _merge_single_route(self.BASE, {}) + + def test_invalid_current_yaml_raises(self): + with self.assertRaises(EgressProxyApplyError): + _merge_single_route("{not json", {"host": "x.example"}) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_supervise_server.py b/tests/unit/test_supervise_server.py index d4616f6..eb7c296 100644 --- a/tests/unit/test_supervise_server.py +++ b/tests/unit/test_supervise_server.py @@ -45,22 +45,6 @@ from claude_bottle.supervise_server import ( class TestValidation(unittest.TestCase): - def test_egress_proxy_block_requires_valid_json(self): - with self.assertRaises(_RpcError) as cm: - validate_proposed_file(_sv.TOOL_EGRESS_PROXY_BLOCK, "{not json") - self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code) - self.assertIn("not valid JSON", cm.exception.message) - - def test_egress_proxy_block_requires_routes_array(self): - with self.assertRaises(_RpcError): - validate_proposed_file(_sv.TOOL_EGRESS_PROXY_BLOCK, '{"other": []}') - - def test_egress_proxy_block_accepts_valid_routes(self): - validate_proposed_file( - _sv.TOOL_EGRESS_PROXY_BLOCK, - '{"routes": [{"path": "/x/", "upstream": "https://example.com"}]}', - ) - def test_pipelock_block_accepts_https_url(self): validate_proposed_file( _sv.TOOL_PIPELOCK_BLOCK, @@ -89,8 +73,12 @@ class TestValidation(unittest.TestCase): "FROM python:3.13\nRUN apk add git\n", ) - def test_empty_proposed_file_rejected_for_all_tools(self): - for tool in _sv.TOOLS: + def test_empty_proposed_file_rejected_for_tools_with_file_field(self): + # egress-proxy-block has structured input (validated in + # _validate_and_bundle_egress_route, not here) and + # list-egress-proxy-routes takes no input. Only the other + # two go through `validate_proposed_file`. + for tool in (_sv.TOOL_PIPELOCK_BLOCK, _sv.TOOL_CAPABILITY_BLOCK): with self.subTest(tool=tool): with self.assertRaises(_RpcError): validate_proposed_file(tool, " \n\t") @@ -243,7 +231,7 @@ class TestHandleToolsCall(unittest.TestCase): { "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { - "routes": '{"routes": []}', + "host": "example.com", "justification": "need a route", }, }, @@ -286,7 +274,7 @@ class TestHandleToolsCall(unittest.TestCase): handle_tools_call( { "name": _sv.TOOL_EGRESS_PROXY_BLOCK, - "arguments": {"routes": '{"routes": []}'}, + "arguments": {"host": "example.com"}, }, self.config, ) @@ -298,7 +286,7 @@ class TestHandleToolsCall(unittest.TestCase): { "name": _sv.TOOL_EGRESS_PROXY_BLOCK, "arguments": { - "routes": '{"routes": []}', + "host": "example.com", "justification": "x", }, }, From db1b5238810f1b7aed6b869f31ebdf1702adac36 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 18:50:36 -0400 Subject: [PATCH 13/18] fix(egress-proxy-apply): correct misleading "egress-proxy updated" wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_mirror_hosts_to_pipelock` runs BEFORE the egress-proxy write in `apply_routes_change` — if it raises, egress-proxy is left intact. The error message claimed the opposite ("egress-proxy routes updated but pipelock allowlist mirror failed"), pointing the operator at the wrong half-state. Reword to make the actual state clear: pipelock failed, egress-proxy NOT updated, fix pipelock manually with `pipelock edit ` then retry. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/egress_proxy_apply.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index 535552b..87af4cf 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -102,11 +102,14 @@ def _mirror_hosts_to_pipelock(slug: str, hosts: list[str]) -> None: return # nothing to add apply_allowlist_change(slug, render_allowlist_content(merged)) except PipelockApplyError as e: + # Mirror runs BEFORE the egress-proxy write, so egress-proxy + # is unchanged on this failure path. Report it as a + # pipelock-side problem so the operator looks in the right + # place; their `pipelock edit` flow can repair manually. raise EgressProxyApplyError( - f"egress-proxy routes updated but pipelock allowlist " - f"mirror failed: {e}. The request will 403 at pipelock " - f"until pipelock's allowlist is refreshed; retry the " - f"proposal or edit pipelock's allowlist by hand." + f"pipelock allowlist mirror failed (egress-proxy NOT " + f"updated): {e}. Fix pipelock's allowlist manually with " + f"`pipelock edit ` then retry the proposal." ) from e From 93f7d248f6a4baafd66820f24fa7179e39806533 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 18:54:30 -0400 Subject: [PATCH 14/18] fix(egress-proxy-apply): strip pipelock-incompatible hosts from mirror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pipelock's allowlist parser only accepts `[A-Za-z0-9_.-]+` literal hostnames. Wildcard routes (`*.example.com`) that egress-proxy's route table accepts trip pipelock's parser the moment the mirror tries to render them into the new yaml; the whole apply fails before pipelock is even touched. Symptom: operator approves an egress-proxy-block proposal, gets "pipelock allowlist mirror failed: allowlist line N: '' has disallowed characters." Fix: `_mirror_hosts_to_pipelock` filters through `_pipelock_safe_hosts` before merging — anything outside pipelock's allowed charset is silently skipped. Wildcard routes stay live on egress-proxy; pipelock just won't pin a hostname for the wildcard-matched traffic (caller's call to accept the hostname-only enforcement gap there). Adds 4 unit tests covering normal hostnames pass-through, wildcard stripping, IPv6-literal stripping, and order preservation. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/egress_proxy_apply.py | 36 +++++++++++++++---- tests/unit/test_egress_proxy_apply.py | 34 ++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index 87af4cf..03ee702 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -24,6 +24,7 @@ from __future__ import annotations import json import os +import re import subprocess import tempfile from pathlib import Path @@ -88,16 +89,39 @@ def _hosts_in_routes(content: str) -> list[str]: return sorted({r.host for r in routes if r.host}) +# Pipelock's allowlist parser accepts only literal hostnames: +# `[A-Za-z0-9_.-]+`. Wildcard hosts (e.g. `*.example.com`) that +# egress-proxy's route table accepts MUST be stripped here or the +# whole pipelock apply fails parse before the new allowlist is +# even written. Egress-proxy still has the wildcard route on its +# side; pipelock's allowlist just won't pin a hostname for the +# wildcard-matched traffic (the user accepts that pipelock-side +# enforcement is hostname-only for those routes). +_PIPELOCK_HOST_RE = re.compile(r"^[A-Za-z0-9_.-]+$") + + +def _pipelock_safe_hosts(hosts: list[str]) -> list[str]: + """Drop any host pipelock's allowlist parser would reject — + today that means anything with characters outside + `[A-Za-z0-9_.-]` (wildcards, IPv6 literals, etc.). Order + preserved.""" + return [h for h in hosts if _PIPELOCK_HOST_RE.match(h)] + + def _mirror_hosts_to_pipelock(slug: str, hosts: list[str]) -> None: - """Ensure every `hosts` entry is on pipelock's allowlist. Fetches - pipelock's current allowlist, merges, re-applies. No-op if every - host is already present (apply still restarts pipelock if any - host is new). Raises EgressProxyApplyError on pipelock failures - so the caller's diff/audit reflects the half-state.""" + """Ensure every pipelock-compatible `hosts` entry is on + pipelock's allowlist. Fetches pipelock's current allowlist, + merges, re-applies. Hosts pipelock can't represent (wildcards, + etc.) are silently skipped — they stay live on egress-proxy + but aren't enforced at pipelock. No-op if every host is already + present (apply still restarts pipelock if any host is new). + Raises EgressProxyApplyError on pipelock failures so the + caller's diff/audit reflects the half-state.""" + safe_hosts = _pipelock_safe_hosts(hosts) try: current = fetch_current_allowlist(slug) existing = parse_allowlist_content(current) - merged = sorted(set(existing) | set(hosts)) + merged = sorted(set(existing) | set(safe_hosts)) if merged == sorted(existing): return # nothing to add apply_allowlist_change(slug, render_allowlist_content(merged)) diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py index f300db1..244bdd4 100644 --- a/tests/unit/test_egress_proxy_apply.py +++ b/tests/unit/test_egress_proxy_apply.py @@ -10,6 +10,7 @@ from claude_bottle.backend.docker.egress_proxy_apply import ( EgressProxyApplyError, _hosts_in_routes, _merge_single_route, + _pipelock_safe_hosts, validate_routes_content, ) @@ -189,5 +190,38 @@ class TestMergeSingleRoute(unittest.TestCase): _merge_single_route("{not json", {"host": "x.example"}) +class TestPipelockSafeHosts(unittest.TestCase): + def test_passes_normal_hostnames_through(self): + self.assertEqual( + ["api.github.com", "registry.npmjs.org"], + _pipelock_safe_hosts(["api.github.com", "registry.npmjs.org"]), + ) + + def test_strips_wildcards(self): + # Pipelock's allowlist parser rejects `*` — egress-proxy can + # accept wildcard routes on its side, but the pipelock mirror + # has to skip them or apply fails before the new yaml is even + # written. + self.assertEqual( + ["api.github.com"], + _pipelock_safe_hosts(["*.example.com", "api.github.com"]), + ) + + def test_strips_ipv6_literals(self): + # Brackets aren't in pipelock's allowed charset either. + self.assertEqual( + ["api.example.com"], + _pipelock_safe_hosts(["[::1]", "api.example.com"]), + ) + + def test_preserves_order(self): + self.assertEqual( + ["a.example", "b.example", "c.example"], + _pipelock_safe_hosts([ + "a.example", "*.junk", "b.example", "weird host", "c.example", + ]), + ) + + if __name__ == "__main__": unittest.main() From e26fe874e4b96b9883844cb70e01ef9ab2c40f3e Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 19:00:06 -0400 Subject: [PATCH 15/18] fix(egress-proxy-apply): wildcard hosts normalise to suffix in pipelock mirror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous fix stripped wildcard hosts entirely from the pipelock mirror; the operator wanted the suffix kept so pipelock pins the base hostname. Now `*.example.com` becomes `example.com` in the mirror — egress-proxy keeps the wildcard for its own host match, pipelock allows the suffix. Behavior change: - `*.example.com` → `example.com` (was: dropped) - `*.foo.bar.com` → `foo.bar.com` (one `*.` strip, not recursive) - `*` → dropped (normalises to empty) - `example.com` → `example.com` (unchanged) - `[::1]`, etc. → dropped (still off pipelock's charset after any prefix strip) Adds explicit de-dup so `*.example.com` + `example.com` collapse to one entry. Existing wildcard-strip test reshaped + 3 new edge-case tests. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/egress_proxy_apply.py | 34 +++++++++++++------ tests/unit/test_egress_proxy_apply.py | 31 +++++++++++++---- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index 03ee702..a7f3996 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -91,21 +91,33 @@ def _hosts_in_routes(content: str) -> list[str]: # Pipelock's allowlist parser accepts only literal hostnames: # `[A-Za-z0-9_.-]+`. Wildcard hosts (e.g. `*.example.com`) that -# egress-proxy's route table accepts MUST be stripped here or the -# whole pipelock apply fails parse before the new allowlist is -# even written. Egress-proxy still has the wildcard route on its -# side; pipelock's allowlist just won't pin a hostname for the -# wildcard-matched traffic (the user accepts that pipelock-side -# enforcement is hostname-only for those routes). +# egress-proxy's route table accepts get normalised here by +# stripping the leading `*.` (so `*.example.com` → `example.com`) +# — egress-proxy retains the wildcard for its own host matching, +# and pipelock's allowlist gets the suffix, which still permits +# the wildcard-matched upstream connections without expanding to +# arbitrary subdomains. Hosts that still don't fit the pipelock +# charset after normalisation (bare `*`, IPv6 literals, weird +# chars) are silently skipped. _PIPELOCK_HOST_RE = re.compile(r"^[A-Za-z0-9_.-]+$") def _pipelock_safe_hosts(hosts: list[str]) -> list[str]: - """Drop any host pipelock's allowlist parser would reject — - today that means anything with characters outside - `[A-Za-z0-9_.-]` (wildcards, IPv6 literals, etc.). Order - preserved.""" - return [h for h in hosts if _PIPELOCK_HOST_RE.match(h)] + """Normalise hosts for pipelock's allowlist: strip leading + `*.` from wildcards, drop anything that still doesn't match + pipelock's allowed charset. Order preserved; duplicates that + arise from normalisation are de-duped (first-seen wins).""" + out: list[str] = [] + seen: set[str] = set() + for h in hosts: + candidate = h[2:] if h.startswith("*.") else h + if not candidate or not _PIPELOCK_HOST_RE.match(candidate): + continue + if candidate in seen: + continue + seen.add(candidate) + out.append(candidate) + return out def _mirror_hosts_to_pipelock(slug: str, hosts: list[str]) -> None: diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py index 244bdd4..47272b2 100644 --- a/tests/unit/test_egress_proxy_apply.py +++ b/tests/unit/test_egress_proxy_apply.py @@ -197,16 +197,26 @@ class TestPipelockSafeHosts(unittest.TestCase): _pipelock_safe_hosts(["api.github.com", "registry.npmjs.org"]), ) - def test_strips_wildcards(self): - # Pipelock's allowlist parser rejects `*` — egress-proxy can - # accept wildcard routes on its side, but the pipelock mirror - # has to skip them or apply fails before the new yaml is even - # written. + def test_strips_wildcard_prefix(self): + # `*.example.com` becomes `example.com` — pipelock pins the + # suffix, egress-proxy keeps the wildcard on its side. self.assertEqual( - ["api.github.com"], + ["example.com", "api.github.com"], _pipelock_safe_hosts(["*.example.com", "api.github.com"]), ) + def test_wildcard_strips_one_label_not_recursive(self): + # `*.foo.bar.com` → `foo.bar.com` (one strip of `*.`). + self.assertEqual( + ["foo.bar.com"], + _pipelock_safe_hosts(["*.foo.bar.com"]), + ) + + def test_drops_bare_wildcard(self): + # `*` alone would normalise to empty; nothing useful to send + # to pipelock. + self.assertEqual([], _pipelock_safe_hosts(["*"])) + def test_strips_ipv6_literals(self): # Brackets aren't in pipelock's allowed charset either. self.assertEqual( @@ -214,11 +224,18 @@ class TestPipelockSafeHosts(unittest.TestCase): _pipelock_safe_hosts(["[::1]", "api.example.com"]), ) + def test_dedupes_after_normalisation(self): + # `*.example.com` + `example.com` both yield `example.com`. + self.assertEqual( + ["example.com"], + _pipelock_safe_hosts(["*.example.com", "example.com"]), + ) + def test_preserves_order(self): self.assertEqual( ["a.example", "b.example", "c.example"], _pipelock_safe_hosts([ - "a.example", "*.junk", "b.example", "weird host", "c.example", + "a.example", "weird host", "b.example", "*", "c.example", ]), ) From 811a6fbfe933aaa96a06ea519c64b2aaa922b697 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 19:10:22 -0400 Subject: [PATCH 16/18] feat(egress-proxy-addon): wildcard host matching with exact-match precedence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD 0017 v1 deliberately punted wildcards ("Exact match in v1 — globs / wildcards are a follow-up"). Now that the supervise mirror strips `*.` to its suffix for pipelock, the addon needs to actually match wildcard hosts on its side or the route is dead weight. Addon `match_route` now does two passes: 1. Exact (case-insensitive) literal match on the hostname. 2. Wildcard suffix match: a route whose host starts with `*.` matches any request host that ends with `.`. So `*.example.com` matches `foo.example.com` and `a.b.example.com`, but NOT the apex `example.com` and not `barexample.com` (the leading `.` of the suffix is required). Exact wins — operators can layer a specific route (e.g. `api.github.com` with auth) on top of a broader wildcard (e.g. `*.github.com` bare-pass). 8 new unit tests: direct subdomain match, nested subdomain match, apex rejection, overlapping-suffix rejection, case-insensitive, exact-wins-over-wildcard (both route orders), no-match fall-through. 395 unit + integration pass. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/egress_proxy_addon_core.py | 28 +++++++++-- tests/unit/test_egress_proxy_addon_core.py | 58 ++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/claude_bottle/egress_proxy_addon_core.py b/claude_bottle/egress_proxy_addon_core.py index d2bbfbf..e77cbc9 100644 --- a/claude_bottle/egress_proxy_addon_core.py +++ b/claude_bottle/egress_proxy_addon_core.py @@ -169,15 +169,33 @@ def match_route( routes: typing.Sequence[Route], request_host: str, ) -> Route | None: - """Return the first route whose `host` matches `request_host`. + """Return the route whose `host` matches `request_host`. - Exact match in v1 — globs / wildcards are a follow-up (per PRD - 0017 open questions). Hostname comparison is case-insensitive - because DNS names are case-insensitive.""" + Match precedence: + 1. Exact (case-insensitive) match on the literal hostname. + 2. Wildcard match: a route whose host starts with `*.` is a + suffix pattern. `*.example.com` matches any host that + ends with `.example.com` (so `foo.example.com` and + `a.b.example.com`, but NOT the apex `example.com` or + `barexample.com`). + + Exact match wins over wildcard so an operator can declare a + specific route on top of a broader wildcard (e.g. a + `*.github.com` bare-pass + an `api.github.com` route with + auth). DNS names are case-insensitive.""" target = request_host.lower() + # Pass 1: exact, literal hostname match. for r in routes: - if r.host.lower() == target: + host = r.host.lower() + if not host.startswith("*.") and host == target: return r + # Pass 2: wildcard suffix match (`*.foo.com` → `.foo.com`). + for r in routes: + host = r.host.lower() + if host.startswith("*."): + suffix = host[1:] # keeps the leading `.` + if target.endswith(suffix) and target != suffix[1:]: + return r return None diff --git a/tests/unit/test_egress_proxy_addon_core.py b/tests/unit/test_egress_proxy_addon_core.py index fd9de6e..ba89fce 100644 --- a/tests/unit/test_egress_proxy_addon_core.py +++ b/tests/unit/test_egress_proxy_addon_core.py @@ -148,6 +148,64 @@ class TestMatchRoute(unittest.TestCase): self.assertIsNone(match_route(self.ROUTES, "evil.api.github.com")) +class TestMatchRouteWildcards(unittest.TestCase): + """Wildcard host patterns: `*.foo.com` matches any host that + ends with `.foo.com` (subdomains, one level or more).""" + + def test_wildcard_matches_direct_subdomain(self): + routes = (Route(host="*.example.com"),) + r = match_route(routes, "foo.example.com") + self.assertIsNotNone(r) + self.assertEqual("*.example.com", r.host) + + def test_wildcard_matches_nested_subdomain(self): + routes = (Route(host="*.example.com"),) + self.assertIsNotNone(match_route(routes, "a.b.example.com")) + + def test_wildcard_does_not_match_apex(self): + routes = (Route(host="*.example.com"),) + self.assertIsNone(match_route(routes, "example.com")) + + def test_wildcard_does_not_match_overlapping_suffix(self): + # `*.example.com` shouldn't match `barexample.com` — the + # match requires `.` before the suffix. + routes = (Route(host="*.example.com"),) + self.assertIsNone(match_route(routes, "barexample.com")) + + def test_wildcard_case_insensitive(self): + routes = (Route(host="*.example.com"),) + self.assertIsNotNone(match_route(routes, "FOO.Example.COM")) + + def test_exact_match_wins_over_wildcard(self): + # A specific route declared alongside a broader wildcard + # should take precedence — operators stack a per-host + # config on top of a permissive wildcard this way. + routes = ( + Route(host="*.github.com"), + Route(host="api.github.com", auth_scheme="Bearer", + token_env="EGRESS_PROXY_TOKEN_0"), + ) + r = match_route(routes, "api.github.com") + self.assertIsNotNone(r) + self.assertEqual("api.github.com", r.host) + self.assertEqual("Bearer", r.auth_scheme) + + def test_exact_wins_regardless_of_route_order(self): + # Same as above but with wildcard declared AFTER exact — + # exact wins because pass 1 finds it before pass 2 runs. + routes = ( + Route(host="api.github.com", auth_scheme="Bearer", + token_env="EGRESS_PROXY_TOKEN_0"), + Route(host="*.github.com"), + ) + r = match_route(routes, "api.github.com") + self.assertEqual("api.github.com", r.host) + + def test_no_match_falls_through(self): + routes = (Route(host="*.example.com"),) + self.assertIsNone(match_route(routes, "elsewhere.org")) + + # --- decide -------------------------------------------------------------- From 6177c0518e2137cc4f6d039e6ade9b2565db90f6 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 19:16:33 -0400 Subject: [PATCH 17/18] fix(egress-proxy-addon): wildcard hosts also match the apex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `*.example.com` now matches `example.com` itself in addition to every subdomain. RFC 6125 TLS-wildcard semantics excluded the apex; an allowlist's natural reading of `*.example.com` is "all of example.com" — and the pipelock mirror already strips `*.example.com` to `example.com`, so without the apex match the two layers disagreed (pipelock allowed the apex, egress-proxy blocked it). Behavior: - `*.example.com` matches `example.com` (apex) - `*.example.com` matches `foo.example.com` (subdomain) - `*.example.com` matches `a.b.example.com` (nested) - `*.example.com` does NOT match `barexample.com` (label boundary required) Test renamed: `test_wildcard_does_not_match_apex` → `test_wildcard_matches_apex`. 395 tests pass. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/egress_proxy_addon_core.py | 19 ++++++++++++------- tests/unit/test_egress_proxy_addon_core.py | 8 ++++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/claude_bottle/egress_proxy_addon_core.py b/claude_bottle/egress_proxy_addon_core.py index e77cbc9..967ea06 100644 --- a/claude_bottle/egress_proxy_addon_core.py +++ b/claude_bottle/egress_proxy_addon_core.py @@ -174,10 +174,15 @@ def match_route( Match precedence: 1. Exact (case-insensitive) match on the literal hostname. 2. Wildcard match: a route whose host starts with `*.` is a - suffix pattern. `*.example.com` matches any host that - ends with `.example.com` (so `foo.example.com` and - `a.b.example.com`, but NOT the apex `example.com` or - `barexample.com`). + suffix pattern that covers the apex AND every subdomain. + `*.example.com` matches `example.com`, `foo.example.com`, + and `a.b.example.com`, but NOT `barexample.com` (the + label boundary `.` is required when matching a + subdomain). This is intentionally more permissive than + RFC 6125 TLS-wildcard semantics — an allowlist's natural + reading of `*.example.com` is "all of example.com", + apex included, and matches what the pipelock mirror does + (strips `*.example.com` → `example.com`). Exact match wins over wildcard so an operator can declare a specific route on top of a broader wildcard (e.g. a @@ -189,12 +194,12 @@ def match_route( host = r.host.lower() if not host.startswith("*.") and host == target: return r - # Pass 2: wildcard suffix match (`*.foo.com` → `.foo.com`). + # Pass 2: wildcard match — apex + every subdomain. for r in routes: host = r.host.lower() if host.startswith("*."): - suffix = host[1:] # keeps the leading `.` - if target.endswith(suffix) and target != suffix[1:]: + suffix = host[2:] # strip the `*.` + if target == suffix or target.endswith("." + suffix): return r return None diff --git a/tests/unit/test_egress_proxy_addon_core.py b/tests/unit/test_egress_proxy_addon_core.py index ba89fce..cba1edd 100644 --- a/tests/unit/test_egress_proxy_addon_core.py +++ b/tests/unit/test_egress_proxy_addon_core.py @@ -162,9 +162,13 @@ class TestMatchRouteWildcards(unittest.TestCase): routes = (Route(host="*.example.com"),) self.assertIsNotNone(match_route(routes, "a.b.example.com")) - def test_wildcard_does_not_match_apex(self): + def test_wildcard_matches_apex(self): + # Allowlist semantics: `*.example.com` covers + # `example.com` itself + every subdomain. Matches what + # the pipelock mirror does (strips `*.example.com` → + # `example.com`) so the two layers agree. routes = (Route(host="*.example.com"),) - self.assertIsNone(match_route(routes, "example.com")) + self.assertIsNotNone(match_route(routes, "example.com")) def test_wildcard_does_not_match_overlapping_suffix(self): # `*.example.com` shouldn't match `barexample.com` — the From 6c886200d9b7fcb47b884eb708dbb0d7109a803a Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 19:48:35 -0400 Subject: [PATCH 18/18] revert(egress-proxy): drop wildcard host support entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The apex-vs-subdomain question, the cert/SNI mismatch when pipelock-passthrough hosts have wildcard certs, and the mirror-divergence corner cases stacked up faster than the feature earned its keep. Going back to exact-host match only. Addon (`match_route`): single pass, case-insensitive exact match. `*.foo.com` in a route table is now a literal string that won't match anything — operators that want subdomains declare them individually. Pipelock mirror (`_pipelock_safe_hosts`): silently drops hosts that don't fit pipelock's `[A-Za-z0-9_.-]+` charset (wildcards, IPv6 literals, stray chars). Previously normalised wildcards to their suffix; now just drops them, which matches egress-proxy's behavior of not matching them either. 8 wildcard test cases removed; 2 lightweight "wildcards are not supported" assertions retained as documentation. 386 unit pass. Co-Authored-By: Claude Opus 4.7 --- .../backend/docker/egress_proxy_apply.py | 34 +++------- claude_bottle/egress_proxy_addon_core.py | 36 +++------- tests/unit/test_egress_proxy_addon_core.py | 66 ++----------------- tests/unit/test_egress_proxy_apply.py | 31 +++------ 4 files changed, 32 insertions(+), 135 deletions(-) diff --git a/claude_bottle/backend/docker/egress_proxy_apply.py b/claude_bottle/backend/docker/egress_proxy_apply.py index a7f3996..e73c620 100644 --- a/claude_bottle/backend/docker/egress_proxy_apply.py +++ b/claude_bottle/backend/docker/egress_proxy_apply.py @@ -90,34 +90,20 @@ def _hosts_in_routes(content: str) -> list[str]: # Pipelock's allowlist parser accepts only literal hostnames: -# `[A-Za-z0-9_.-]+`. Wildcard hosts (e.g. `*.example.com`) that -# egress-proxy's route table accepts get normalised here by -# stripping the leading `*.` (so `*.example.com` → `example.com`) -# — egress-proxy retains the wildcard for its own host matching, -# and pipelock's allowlist gets the suffix, which still permits -# the wildcard-matched upstream connections without expanding to -# arbitrary subdomains. Hosts that still don't fit the pipelock -# charset after normalisation (bare `*`, IPv6 literals, weird -# chars) are silently skipped. +# `[A-Za-z0-9_.-]+`. Anything else (wildcards, IPv6 literals, +# stray characters) is silently dropped from the mirror so the +# pipelock apply doesn't fail parse before the new yaml is even +# written. The dropped hosts stay on egress-proxy's route table — +# but the addon does exact-host match only, so they'll never +# match anything either. (Wildcard host matching was removed — +# see `match_route` in egress_proxy_addon_core for the rationale.) _PIPELOCK_HOST_RE = re.compile(r"^[A-Za-z0-9_.-]+$") def _pipelock_safe_hosts(hosts: list[str]) -> list[str]: - """Normalise hosts for pipelock's allowlist: strip leading - `*.` from wildcards, drop anything that still doesn't match - pipelock's allowed charset. Order preserved; duplicates that - arise from normalisation are de-duped (first-seen wins).""" - out: list[str] = [] - seen: set[str] = set() - for h in hosts: - candidate = h[2:] if h.startswith("*.") else h - if not candidate or not _PIPELOCK_HOST_RE.match(candidate): - continue - if candidate in seen: - continue - seen.add(candidate) - out.append(candidate) - return out + """Drop any host pipelock's allowlist parser would reject. + Order preserved.""" + return [h for h in hosts if _PIPELOCK_HOST_RE.match(h)] def _mirror_hosts_to_pipelock(slug: str, hosts: list[str]) -> None: diff --git a/claude_bottle/egress_proxy_addon_core.py b/claude_bottle/egress_proxy_addon_core.py index 967ea06..5e4c854 100644 --- a/claude_bottle/egress_proxy_addon_core.py +++ b/claude_bottle/egress_proxy_addon_core.py @@ -169,38 +169,18 @@ def match_route( routes: typing.Sequence[Route], request_host: str, ) -> Route | None: - """Return the route whose `host` matches `request_host`. + """Return the first route whose `host` matches `request_host` + exactly (case-insensitive). DNS names are case-insensitive. - Match precedence: - 1. Exact (case-insensitive) match on the literal hostname. - 2. Wildcard match: a route whose host starts with `*.` is a - suffix pattern that covers the apex AND every subdomain. - `*.example.com` matches `example.com`, `foo.example.com`, - and `a.b.example.com`, but NOT `barexample.com` (the - label boundary `.` is required when matching a - subdomain). This is intentionally more permissive than - RFC 6125 TLS-wildcard semantics — an allowlist's natural - reading of `*.example.com` is "all of example.com", - apex included, and matches what the pipelock mirror does - (strips `*.example.com` → `example.com`). - - Exact match wins over wildcard so an operator can declare a - specific route on top of a broader wildcard (e.g. a - `*.github.com` bare-pass + an `api.github.com` route with - auth). DNS names are case-insensitive.""" + Wildcard hosts (`*.foo.com`) are NOT supported — they caused + too many edge cases (apex match? cert validation? pipelock + mirror mismatch?) for too little payoff. Operators that need + multiple subdomains declare them individually (or one common + parent host as a bare-pass route).""" target = request_host.lower() - # Pass 1: exact, literal hostname match. for r in routes: - host = r.host.lower() - if not host.startswith("*.") and host == target: + if r.host.lower() == target: return r - # Pass 2: wildcard match — apex + every subdomain. - for r in routes: - host = r.host.lower() - if host.startswith("*."): - suffix = host[2:] # strip the `*.` - if target == suffix or target.endswith("." + suffix): - return r return None diff --git a/tests/unit/test_egress_proxy_addon_core.py b/tests/unit/test_egress_proxy_addon_core.py index cba1edd..833feac 100644 --- a/tests/unit/test_egress_proxy_addon_core.py +++ b/tests/unit/test_egress_proxy_addon_core.py @@ -147,67 +147,13 @@ class TestMatchRoute(unittest.TestCase): # other-host shouldn't be matched via a "ends with" check. self.assertIsNone(match_route(self.ROUTES, "evil.api.github.com")) - -class TestMatchRouteWildcards(unittest.TestCase): - """Wildcard host patterns: `*.foo.com` matches any host that - ends with `.foo.com` (subdomains, one level or more).""" - - def test_wildcard_matches_direct_subdomain(self): + def test_wildcard_hosts_not_supported(self): + # `*.example.com` is treated as a literal host string by + # the exact-only matcher. Removed from the design after + # the apex/RFC-6125/pipelock-mirror edge cases stacked up. routes = (Route(host="*.example.com"),) - r = match_route(routes, "foo.example.com") - self.assertIsNotNone(r) - self.assertEqual("*.example.com", r.host) - - def test_wildcard_matches_nested_subdomain(self): - routes = (Route(host="*.example.com"),) - self.assertIsNotNone(match_route(routes, "a.b.example.com")) - - def test_wildcard_matches_apex(self): - # Allowlist semantics: `*.example.com` covers - # `example.com` itself + every subdomain. Matches what - # the pipelock mirror does (strips `*.example.com` → - # `example.com`) so the two layers agree. - routes = (Route(host="*.example.com"),) - self.assertIsNotNone(match_route(routes, "example.com")) - - def test_wildcard_does_not_match_overlapping_suffix(self): - # `*.example.com` shouldn't match `barexample.com` — the - # match requires `.` before the suffix. - routes = (Route(host="*.example.com"),) - self.assertIsNone(match_route(routes, "barexample.com")) - - def test_wildcard_case_insensitive(self): - routes = (Route(host="*.example.com"),) - self.assertIsNotNone(match_route(routes, "FOO.Example.COM")) - - def test_exact_match_wins_over_wildcard(self): - # A specific route declared alongside a broader wildcard - # should take precedence — operators stack a per-host - # config on top of a permissive wildcard this way. - routes = ( - Route(host="*.github.com"), - Route(host="api.github.com", auth_scheme="Bearer", - token_env="EGRESS_PROXY_TOKEN_0"), - ) - r = match_route(routes, "api.github.com") - self.assertIsNotNone(r) - self.assertEqual("api.github.com", r.host) - self.assertEqual("Bearer", r.auth_scheme) - - def test_exact_wins_regardless_of_route_order(self): - # Same as above but with wildcard declared AFTER exact — - # exact wins because pass 1 finds it before pass 2 runs. - routes = ( - Route(host="api.github.com", auth_scheme="Bearer", - token_env="EGRESS_PROXY_TOKEN_0"), - Route(host="*.github.com"), - ) - r = match_route(routes, "api.github.com") - self.assertEqual("api.github.com", r.host) - - def test_no_match_falls_through(self): - routes = (Route(host="*.example.com"),) - self.assertIsNone(match_route(routes, "elsewhere.org")) + self.assertIsNone(match_route(routes, "foo.example.com")) + self.assertIsNone(match_route(routes, "example.com")) # --- decide -------------------------------------------------------------- diff --git a/tests/unit/test_egress_proxy_apply.py b/tests/unit/test_egress_proxy_apply.py index 47272b2..8e2f5a4 100644 --- a/tests/unit/test_egress_proxy_apply.py +++ b/tests/unit/test_egress_proxy_apply.py @@ -197,45 +197,30 @@ class TestPipelockSafeHosts(unittest.TestCase): _pipelock_safe_hosts(["api.github.com", "registry.npmjs.org"]), ) - def test_strips_wildcard_prefix(self): - # `*.example.com` becomes `example.com` — pipelock pins the - # suffix, egress-proxy keeps the wildcard on its side. + def test_drops_wildcards(self): + # Wildcard host matching was removed from egress-proxy too, + # so a `*.foo.com` route is dead weight anyway; we drop it + # entirely from the pipelock mirror so the apply doesn't + # fail parse. self.assertEqual( - ["example.com", "api.github.com"], + ["api.github.com"], _pipelock_safe_hosts(["*.example.com", "api.github.com"]), ) - def test_wildcard_strips_one_label_not_recursive(self): - # `*.foo.bar.com` → `foo.bar.com` (one strip of `*.`). - self.assertEqual( - ["foo.bar.com"], - _pipelock_safe_hosts(["*.foo.bar.com"]), - ) - def test_drops_bare_wildcard(self): - # `*` alone would normalise to empty; nothing useful to send - # to pipelock. self.assertEqual([], _pipelock_safe_hosts(["*"])) - def test_strips_ipv6_literals(self): - # Brackets aren't in pipelock's allowed charset either. + def test_drops_ipv6_literals(self): self.assertEqual( ["api.example.com"], _pipelock_safe_hosts(["[::1]", "api.example.com"]), ) - def test_dedupes_after_normalisation(self): - # `*.example.com` + `example.com` both yield `example.com`. - self.assertEqual( - ["example.com"], - _pipelock_safe_hosts(["*.example.com", "example.com"]), - ) - def test_preserves_order(self): self.assertEqual( ["a.example", "b.example", "c.example"], _pipelock_safe_hosts([ - "a.example", "weird host", "b.example", "*", "c.example", + "a.example", "*.junk", "b.example", "weird host", "c.example", ]), )