From 90defdc9cd209d50933dc0885819046c66549160 Mon Sep 17 00:00:00 2001 From: codex Date: Sun, 26 Jul 2026 06:16:53 +0000 Subject: [PATCH] refactor(egress): separate matching, DLP, and context concerns --- bot_bottle/gateway/egress/addon.py | 14 ++- bot_bottle/gateway/egress/addon_core.py | 106 +++----------------- bot_bottle/gateway/egress/context.py | 68 +++++++++++++ bot_bottle/gateway/egress/dlp.py | 99 +++++++++++++++++++ bot_bottle/gateway/egress/matching.py | 108 +++++++++++++++++++++ bot_bottle/gateway/egress/types.py | 81 ++++++++++++++++ tests/unit/test_architecture_guardrails.py | 50 ++++++++++ 7 files changed, 430 insertions(+), 96 deletions(-) create mode 100644 bot_bottle/gateway/egress/context.py create mode 100644 bot_bottle/gateway/egress/dlp.py create mode 100644 bot_bottle/gateway/egress/matching.py create mode 100644 bot_bottle/gateway/egress/types.py create mode 100644 tests/unit/test_architecture_guardrails.py diff --git a/bot_bottle/gateway/egress/addon.py b/bot_bottle/gateway/egress/addon.py index 89fcb07b..b3d22a7f 100644 --- a/bot_bottle/gateway/egress/addon.py +++ b/bot_bottle/gateway/egress/addon.py @@ -26,19 +26,23 @@ from bot_bottle.gateway.egress.addon_core import ( Config, Route, ScanResult, + route_to_yaml_dict, +) +from bot_bottle.gateway.egress.context import resolve_client_context +from bot_bottle.gateway.egress.dlp import ( build_inbound_scan_text, build_outbound_scan_text, build_token_allow_payload, + outbound_scan_headers, + scan_inbound, + scan_outbound, +) +from bot_bottle.gateway.egress.matching import ( decide, decide_git_fetch, is_git_fetch_request, is_git_push_request, match_route, - resolve_client_context, - outbound_scan_headers, - route_to_yaml_dict, - scan_inbound, - scan_outbound, ) from bot_bottle.gateway.policy_resolver import PolicyResolveError, PolicyResolver from bot_bottle.supervisor.types import ( diff --git a/bot_bottle/gateway/egress/addon_core.py b/bot_bottle/gateway/egress/addon_core.py index 3d0bfca1..ebc73deb 100644 --- a/bot_bottle/gateway/egress/addon_core.py +++ b/bot_bottle/gateway/egress/addon_core.py @@ -14,7 +14,6 @@ from __future__ import annotations import re import typing -from dataclasses import dataclass from ...yaml_subset import YamlSubsetError, parse_yaml_subset @@ -30,96 +29,21 @@ from .dlp_config import ( OUTBOUND_ON_MATCH_VALUES, parse_inspect_block, ) - - -# --------------------------------------------------------------------------- -# Match types (Gateway API HTTPRoute vocabulary, PRD 0053) -# --------------------------------------------------------------------------- - -PATH_MATCH_TYPES = ("exact", "prefix", "regex") -HEADER_MATCH_TYPES = ("exact", "regex") - -VALID_METHODS = frozenset({ - "GET", "HEAD", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "TRACE", - "CONNECT", -}) - - -@dataclass(frozen=True) -class PathMatch: - type: str # "exact" | "prefix" | "regex" - value: str - compiled: re.Pattern[str] | None = None - - -@dataclass(frozen=True) -class HeaderMatch: - name: str - value: str - type: str = "exact" # "exact" | "regex" - compiled: re.Pattern[str] | None = None - - -@dataclass(frozen=True) -class MatchEntry: - paths: tuple[PathMatch, ...] = () - methods: tuple[str, ...] = () - headers: tuple[HeaderMatch, ...] = () - - -@dataclass(frozen=True) -class Route: - host: str - matches: tuple[MatchEntry, ...] = () - auth_scheme: str = "" - token_env: str = "" - git_fetch: bool = False - outbound_detectors: tuple[str, ...] | None = None - inbound_detectors: tuple[str, ...] | None = None - # "" means unset → DEFAULT_OUTBOUND_ON_MATCH. See OUTBOUND_ON_MATCH_VALUES. - outbound_on_match: str = "" - preserve_auth: bool = False - # False tunnels HTTPS without TLS interception or HTTP-level controls. - inspect: bool = True - - -LOG_OFF = 0 # no logging -LOG_BLOCKS = 1 # log block/warn events with request context -LOG_FULL = 2 # log block/warn events + full request and response bodies - - -@dataclass(frozen=True) -class Config: - routes: tuple[Route, ...] - log: int = LOG_OFF - # Why this Config is a deny-all, when it is one for a reason *other* than - # the bottle's own policy genuinely not listing the host. A deny-all is - # indistinguishable from "policy loaded, host not allowed" at the decision - # point — both are simply "no matching route" — so without this the - # operator sees `host X is not in the allowlist` and goes hunting for a - # missing route that was never the problem. Empty for a normally-parsed - # policy; `decide` prefers it over the allowlist wording when set. - deny_reason: str = "" - - -@dataclass(frozen=True) -class Decision: - action: str # "forward" or "block" - reason: str = "" - inject_authorization: str | None = None - - -@dataclass(frozen=True) -class ScanResult: - severity: str # "block" or "warn" - reason: str - location: str = "" # where the match was found, e.g. "body", "authorization header" - context: str = "" # surrounding text with the match replaced by REDACT - # Raw substring the detector matched. Used inside the gateway to key the - # supervisor-approved "safe tokens" set (PRD 0062); never logged or written - # to a proposal file. Empty for structural detectors (CRLF) that carry no - # safelist-able value. - matched: str = "" +from .types import ( + HEADER_MATCH_TYPES, + LOG_BLOCKS, + LOG_FULL, + LOG_OFF, + PATH_MATCH_TYPES, + VALID_METHODS, + Config, + Decision, + HeaderMatch, + MatchEntry, + PathMatch, + Route, + ScanResult, +) # --------------------------------------------------------------------------- diff --git a/bot_bottle/gateway/egress/context.py b/bot_bottle/gateway/egress/context.py new file mode 100644 index 00000000..46e06e52 --- /dev/null +++ b/bot_bottle/gateway/egress/context.py @@ -0,0 +1,68 @@ +"""Fail-closed resolution of a client's policy and egress credentials.""" + +from __future__ import annotations + +import typing + +from .types import Config + + +DENY_UNATTRIBUTED = ( + "egress: this request was not attributed to any bottle, so no egress policy " + "applies and every host is denied. Either the bottle's registry row is " + "missing/ambiguous (torn down, or another bottle claimed its source IP), or " + "the request carried no matching identity token — check that the caller's " + "proxy URL includes it. This is not an allowlist problem." +) +DENY_UNPARSEABLE = ( + "egress: this bottle's egress policy could not be parsed, so it is being " + "treated as deny-all. Fix the bottle's egress.routes; every host is denied " + "until it loads." +) +DENY_RESOLVER_ERROR = ( + "egress: the orchestrator could not be reached to resolve this bottle's " + "egress policy, so every host is denied (fail-closed). Check that the " + "control plane is up; this is not an allowlist problem." +) + + +class PolicyResolverLike(typing.Protocol): + def resolve(self, source_ip: str, identity_token: str = ...) -> str | None: ... + + +class ContextResolverLike(typing.Protocol): + def resolve_policy_and_bottle_id( + self, source_ip: str, identity_token: str = ..., + ) -> tuple[str | None, str | None, dict[str, str]]: ... + + +def _config_from_policy(policy: str | None) -> Config: + # Local import keeps schema parsing independent of resolver protocols. + from .addon_core import load_config + if not policy: + return Config(routes=(), deny_reason=DENY_UNATTRIBUTED) + try: + return load_config(policy) + except ValueError: + return Config(routes=(), deny_reason=DENY_UNPARSEABLE) + + +def resolve_client_config( + resolver: PolicyResolverLike, client_ip: str, identity_token: str = "", +) -> Config: + try: + policy = resolver.resolve(client_ip, identity_token) + except Exception: # noqa: BLE001 - a policy lookup failure must deny + return Config(routes=(), deny_reason=DENY_RESOLVER_ERROR) + return _config_from_policy(policy) + + +def resolve_client_context( + resolver: ContextResolverLike, client_ip: str, identity_token: str = "", +) -> tuple[Config, str, dict[str, str]]: + try: + policy, bottle_id, tokens = resolver.resolve_policy_and_bottle_id( + client_ip, identity_token) + except Exception: # noqa: BLE001 - a policy lookup failure must deny + return Config(routes=(), deny_reason=DENY_RESOLVER_ERROR), "", {} + return _config_from_policy(policy), (bottle_id or ""), tokens diff --git a/bot_bottle/gateway/egress/dlp.py b/bot_bottle/gateway/egress/dlp.py new file mode 100644 index 00000000..d69bd408 --- /dev/null +++ b/bot_bottle/gateway/egress/dlp.py @@ -0,0 +1,99 @@ +"""DLP scan dispatch and safe proposal rendering for egress requests.""" + +from __future__ import annotations + +import typing + +from .types import Route, ScanResult + + +def build_outbound_scan_text(host: str, path: str, query: str, + headers: typing.Mapping[str, str], body: str) -> str: + parts = [host, path] + if query: + parts.append(query) + parts.extend(f"{name}: {value}" for name, value in headers.items()) + if body: + parts.append(body) + return "\n".join(parts) + + +def outbound_scan_headers(route: Route, headers: typing.Mapping[str, str]) -> dict[str, str]: + """Drop agent Authorization when the route injects gateway-owned auth.""" + skip_auth = bool(route.auth_scheme and route.token_env) + return {name: value for name, value in headers.items() + if not (skip_auth and name.lower() == "authorization")} + + +def build_inbound_scan_text(headers: typing.Mapping[str, str], body: str) -> str: + parts = [f"{name}: {value}" for name, value in headers.items()] + if body: + parts.append(body) + return "\n".join(parts) + + +def _enabled(configured: tuple[str, ...] | None, name: str) -> bool: + return configured is None or name in configured + + +def scan_outbound(route: Route, body: str | bytes, environ: typing.Mapping[str, str], *, + safe_tokens: typing.AbstractSet[str] | None = None, + crlf_text: str | None = None) -> ScanResult | None: + if not route.inspect: + return None + try: + from dlp_detectors import ( # type: ignore[import-not-found] + scan_crlf_injection, scan_entropy, scan_known_secrets, scan_token_patterns) + except ImportError: # pragma: no cover - gateway's flat module path + from .dlp_detectors import ( + scan_crlf_injection, scan_entropy, scan_known_secrets, scan_token_patterns) + if isinstance(body, bytes): + try: + text = body.decode("utf-8") + except UnicodeDecodeError: + text = body.decode("latin-1") + else: + text = body + result = scan_crlf_injection(text if crlf_text is None else crlf_text) + if result is not None: + return result + if _enabled(route.outbound_detectors, "token_patterns"): + result = scan_token_patterns(text, location="body", safe_tokens=safe_tokens) + if result is not None: + return result + if _enabled(route.outbound_detectors, "known_secrets"): + extra = tuple(prefix for prefix in environ.get( + "BOT_BOTTLE_SENSITIVE_PREFIXES", "").split(",") if prefix) + result = scan_known_secrets(text, location="body", env=environ, + sensitive_prefixes=("EGRESS_TOKEN_",) + extra, + safe_tokens=safe_tokens) + if result is not None: + return result + if route.outbound_detectors is not None and "entropy" in route.outbound_detectors: + return scan_entropy(text, location="body") + return None + + +def build_token_allow_payload(host: str, method: str, path: str, result: ScanResult) -> str: + """Render redacted operator context; the raw matched secret is excluded.""" + lines = [ + "egress blocked an outbound request carrying a detected token", + f"host: {host}", f"method: {method}", f"path: {path}", + f"detector: {result.reason}", + ] + if result.context: + lines.append(f"context: {result.context}") + return "\n".join(lines) + "\n" + + +def scan_inbound(route: Route, body: str | bytes) -> ScanResult | None: + if not route.inspect: + return None + try: + from dlp_detectors import scan_naive_injection # type: ignore[import-not-found] + except ImportError: # pragma: no cover - gateway's flat module path + from .dlp_detectors import scan_naive_injection + text = body if isinstance(body, str) else body.decode("utf-8", errors="replace") + if _enabled(route.inbound_detectors, "naive_injection_detection"): + return scan_naive_injection(text) + return None diff --git a/bot_bottle/gateway/egress/matching.py b/bot_bottle/gateway/egress/matching.py new file mode 100644 index 00000000..2367e84e --- /dev/null +++ b/bot_bottle/gateway/egress/matching.py @@ -0,0 +1,108 @@ +"""Route matching and request-policy decisions for the egress gateway.""" + +from __future__ import annotations + +import typing + +from .types import Decision, MatchEntry, PathMatch, Route + + +def _path_matches(pm: PathMatch, request_path: str) -> bool: + if pm.type == "exact": + return request_path == pm.value + if pm.type == "prefix": + if request_path == pm.value: + return True + if not pm.value.endswith("/"): + return request_path.startswith(pm.value + "/") + return request_path.startswith(pm.value) + return pm.type == "regex" and pm.compiled is not None and pm.compiled.search(request_path) is not None + + +def _entry_matches( + entry: MatchEntry, request_path: str, request_method: str, + request_headers: typing.Mapping[str, str], +) -> bool: + if entry.paths and not any(_path_matches(pm, request_path) for pm in entry.paths): + return False + if entry.methods and request_method.upper() not in entry.methods: + return False + for match in entry.headers: + value = request_headers.get(match.name.lower()) + if value is None: + return False + if match.type == "exact" and value != match.value: + return False + if match.type == "regex" and ( + match.compiled is None or match.compiled.search(value) is None + ): + return False + return True + + +def evaluate_matches( + route: Route, request_path: str, request_method: str = "GET", + request_headers: typing.Mapping[str, str] | None = None, +) -> bool: + """Return whether a request satisfies a route's optional match entries.""" + if not route.matches: + return True + return any(_entry_matches(entry, request_path, request_method, request_headers or {}) + for entry in route.matches) + + +def is_git_push_request(path: str, query: str) -> bool: + return path.endswith("/git-receive-pack") or ( + path.endswith("/info/refs") and any( + pair.partition("=") == ("service", "=", "git-receive-pack") + for pair in query.split("&") + ) + ) + + +def is_git_fetch_request(path: str, query: str) -> bool: + return path.endswith("/git-upload-pack") or ( + path.endswith("/info/refs") and any( + pair.partition("=") == ("service", "=", "git-upload-pack") + for pair in query.split("&") + ) + ) + + +def match_route(routes: typing.Sequence[Route], request_host: str) -> Route | None: + target = request_host.lower() + return next((route for route in routes if route.host.lower() == target), None) + + +def decide( + routes: typing.Sequence[Route], request_host: str, request_path: str, + environ: typing.Mapping[str, str], *, request_method: str = "GET", + request_headers: typing.Mapping[str, str] | None = None, deny_reason: str = "", +) -> Decision: + route = match_route(routes, request_host) + if route is None: + return Decision("block", deny_reason or ( + f"egress: host {request_host!r} is not in the bottle's egress.routes " + "allowlist. Declare a route for it or remove the request.")) + if not evaluate_matches(route, request_path, request_method, request_headers): + return Decision("block", ( + f"egress: request {request_method} {request_path!r} does not match any " + f"entry in matches for {route.host!r}")) + if route.auth_scheme and route.token_env: + token = environ.get(route.token_env, "") + if not token: + return Decision("block", ( + f"egress: route for {route.host!r} declared auth but env var " + f"{route.token_env!r} is unset")) + return Decision("forward", inject_authorization=f"{route.auth_scheme} {token}") + return Decision("forward") + + +def decide_git_fetch(routes: typing.Sequence[Route], request_host: str) -> Decision: + route = match_route(routes, request_host) + if route is not None and route.git_fetch: + return Decision("forward") + return Decision("block", ( + "egress: git fetch/clone over HTTPS is not allowed by default; use git-gate " + "for declared repos or set egress.routes[].git.fetch=true for explicit " + "read-only HTTPS Git access.")) diff --git a/bot_bottle/gateway/egress/types.py b/bot_bottle/gateway/egress/types.py new file mode 100644 index 00000000..53dd18e1 --- /dev/null +++ b/bot_bottle/gateway/egress/types.py @@ -0,0 +1,81 @@ +"""Shared egress policy value objects. + +Kept dependency-free so the schema parser, matcher, DLP scanner, and addon +adapter can use the same immutable public shapes without importing each other. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass + + +PATH_MATCH_TYPES = ("exact", "prefix", "regex") +HEADER_MATCH_TYPES = ("exact", "regex") +VALID_METHODS = frozenset({ + "GET", "HEAD", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "TRACE", + "CONNECT", +}) + +LOG_OFF = 0 +LOG_BLOCKS = 1 +LOG_FULL = 2 + + +@dataclass(frozen=True) +class PathMatch: + type: str + value: str + compiled: re.Pattern[str] | None = None + + +@dataclass(frozen=True) +class HeaderMatch: + name: str + value: str + type: str = "exact" + compiled: re.Pattern[str] | None = None + + +@dataclass(frozen=True) +class MatchEntry: + paths: tuple[PathMatch, ...] = () + methods: tuple[str, ...] = () + headers: tuple[HeaderMatch, ...] = () + + +@dataclass(frozen=True) +class Route: + host: str + matches: tuple[MatchEntry, ...] = () + auth_scheme: str = "" + token_env: str = "" + git_fetch: bool = False + outbound_detectors: tuple[str, ...] | None = None + inbound_detectors: tuple[str, ...] | None = None + outbound_on_match: str = "" + preserve_auth: bool = False + inspect: bool = True + + +@dataclass(frozen=True) +class Config: + routes: tuple[Route, ...] + log: int = LOG_OFF + deny_reason: str = "" + + +@dataclass(frozen=True) +class Decision: + action: str + reason: str = "" + inject_authorization: str | None = None + + +@dataclass(frozen=True) +class ScanResult: + severity: str + reason: str + location: str = "" + context: str = "" + matched: str = "" diff --git a/tests/unit/test_architecture_guardrails.py b/tests/unit/test_architecture_guardrails.py new file mode 100644 index 00000000..9660e9c0 --- /dev/null +++ b/tests/unit/test_architecture_guardrails.py @@ -0,0 +1,50 @@ +"""Architecture rules that should fail before coupling becomes entrenched.""" + +from __future__ import annotations + +import ast +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] + + +class TestCliBackendBoundaries(unittest.TestCase): + def test_cli_does_not_import_a_concrete_backend(self) -> None: + forbidden = ( + "backend.docker", "backend.firecracker", "backend.macos_container", + "bot_bottle.backend.docker", "bot_bottle.backend.firecracker", + "bot_bottle.backend.macos_container", + ) + violations: list[str] = [] + for path in (ROOT / "bot_bottle" / "cli").rglob("*.py"): + tree = ast.parse(path.read_text(), filename=str(path)) + for node in ast.walk(tree): + module = node.module if isinstance(node, ast.ImportFrom) else None + if module and module.startswith(forbidden): + violations.append(f"{path.relative_to(ROOT)}:{node.lineno}: {module}") + if isinstance(node, ast.Import): + violations.extend( + f"{path.relative_to(ROOT)}:{node.lineno}: {alias.name}" + for alias in node.names if alias.name.startswith(forbidden) + ) + self.assertEqual([], violations, "generic CLI imports concrete backend internals:\n" + + "\n".join(violations)) + + +class TestRuntimeModuleSizes(unittest.TestCase): + def test_egress_modules_stay_focused(self) -> None: + caps = { + "addon_core.py": 900, # compatibility + policy-schema parsing only + "types.py": 180, + "matching.py": 180, + "dlp.py": 180, + "context.py": 140, + } + directory = ROOT / "bot_bottle" / "gateway" / "egress" + oversized = [f"{name} ({len((directory / name).read_text().splitlines())}>{cap})" + for name, cap in caps.items() + if len((directory / name).read_text().splitlines()) > cap] + self.assertEqual([], oversized, "split a module rather than raising its cap: " + + ", ".join(oversized))