From 1afb78094bfc4ce8492a1a7df2a04e0956589c6a Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 17 Jul 2026 17:01:36 -0400 Subject: [PATCH] fix(egress): redact the per-bottle token in the now-active multi-tenant response log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of this PR: making `response()` run in the consolidated gateway also activates its `LOG_FULL` `_log_response` call there — previously unreachable, since the empty static config made `response()` return early. That logger redacted with `os.environ`, which in multi-tenant mode does NOT hold the bottle's per-request `/resolve` tokens (only the resolved `env` overlay does), so a non-token-shaped provisioned secret appearing in a response could be logged in the clear. Thread the resolved per-flow `env` into `_log_request` / `_log_response` so the LOG_FULL redaction scrubs the calling bottle's secrets. Adds a regression test (a non-token-shaped `/resolve` secret, absent from os.environ, must not appear in the response log) and updates the redaction-test helpers for the new arg. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ --- bot_bottle/egress_addon.py | 30 ++++++++++++------- tests/unit/test_egress_addon_log_redaction.py | 5 ++-- tests/unit/test_egress_addon_request_flow.py | 28 +++++++++++++++++ 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/bot_bottle/egress_addon.py b/bot_bottle/egress_addon.py index d884858..70abe3d 100644 --- a/bot_bottle/egress_addon.py +++ b/bot_bottle/egress_addon.py @@ -233,31 +233,39 @@ class EgressAddon: {"Content-Type": "text/plain; charset=utf-8"}, ) - def _log_request(self, flow: http.HTTPFlow) -> None: + def _log_request( + self, flow: http.HTTPFlow, env: "typing.Mapping[str, str]", + ) -> None: + # `env` is the per-flow resolved overlay (process env + this bottle's + # /resolve tokens), so the log redaction scrubs the calling bottle's + # provisioned secrets — not just the process-level ones in os.environ. headers = { - k: redact_tokens(v, env=os.environ) + k: redact_tokens(v, env=env) for k, v in flow.request.headers.items() if k.lower() != "authorization" } - body = redact_tokens(flow.request.get_text(strict=False) or "", env=os.environ) + body = redact_tokens(flow.request.get_text(strict=False) or "", env=env) sys.stderr.write( json.dumps({ "event": "egress_request", - "host": redact_tokens(flow.request.pretty_host, env=os.environ), + "host": redact_tokens(flow.request.pretty_host, env=env), "method": flow.request.method, - "path": redact_tokens(flow.request.path, env=os.environ), + "path": redact_tokens(flow.request.path, env=env), "headers": headers, "body": body, }) + "\n" ) - def _log_response(self, flow: http.HTTPFlow) -> None: + def _log_response( + self, flow: http.HTTPFlow, env: "typing.Mapping[str, str]", + ) -> None: + # Per-flow env overlay (see _log_request): redact this bottle's tokens. headers = { - k: redact_tokens(v, env=os.environ) + k: redact_tokens(v, env=env) for k, v in flow.response.headers.items() } - body = redact_tokens(flow.response.get_text(strict=False) or "", env=os.environ) + body = redact_tokens(flow.response.get_text(strict=False) or "", env=env) sys.stderr.write( json.dumps({ "event": "egress_response", @@ -420,7 +428,7 @@ class EgressAddon: flow.request.headers["authorization"] = decision.inject_authorization if config.log >= LOG_FULL: - self._log_request(flow) + self._log_request(flow, env) def _block_dlp(self, flow: http.HTTPFlow, result: ScanResult) -> None: ctx = self._req_ctx(flow) @@ -632,14 +640,14 @@ class EgressAddon: """DLP inbound scan on response headers and body, against the calling bottle's resolved config (multi-tenant) or the static config (single-tenant) — see `_flow_ctx`.""" - config, _slug, _env = self._flow_ctx(flow) + config, _slug, env = self._flow_ctx(flow) route = match_route(config.routes, flow.request.pretty_host) if route is None: return if flow.response is None: return if config.log >= LOG_FULL: - self._log_response(flow) + self._log_response(flow, env) resp_headers = {k.lower(): v for k, v in flow.response.headers.items()} body = flow.response.get_text(strict=False) or "" scan_text = build_inbound_scan_text(resp_headers, body) diff --git a/tests/unit/test_egress_addon_log_redaction.py b/tests/unit/test_egress_addon_log_redaction.py index 5a648e2..45b60f8 100644 --- a/tests/unit/test_egress_addon_log_redaction.py +++ b/tests/unit/test_egress_addon_log_redaction.py @@ -8,6 +8,7 @@ real mitmproxy package.""" from __future__ import annotations import json +import os import sys import types import unittest @@ -107,14 +108,14 @@ class _Flow: def _log_request(addon: EgressAddon, flow: _Flow) -> dict[str, Any]: buf = StringIO() with patch("sys.stderr", buf): - addon._log_request(flow) # type: ignore[arg-type] + addon._log_request(flow, os.environ) # type: ignore[arg-type] return json.loads(buf.getvalue()) def _log_response(addon: EgressAddon, flow: _Flow) -> dict[str, Any]: buf = StringIO() with patch("sys.stderr", buf): - addon._log_response(flow) # type: ignore[arg-type] + addon._log_response(flow, os.environ) # type: ignore[arg-type] return json.loads(buf.getvalue()) diff --git a/tests/unit/test_egress_addon_request_flow.py b/tests/unit/test_egress_addon_request_flow.py index c794b75..2deff87 100644 --- a/tests/unit/test_egress_addon_request_flow.py +++ b/tests/unit/test_egress_addon_request_flow.py @@ -906,6 +906,34 @@ class TestMultiTenantInboundDlp(unittest.TestCase): addon.websocket_message(flow) # type: ignore[arg-type] self.assertTrue(flow.killed) + def test_response_log_redacts_per_bottle_resolve_token(self) -> None: + # LOG_FULL response logging now runs in multi-tenant mode, so it must + # scrub the calling bottle's /resolve token — which lives only in the + # resolved env overlay, never in the gateway's os.environ. A + # non-token-shaped secret is caught only via that env, so os.environ + # redaction (the pre-fix behaviour) would leak it into the log. + secret = "bottle-a-provisioned-secret-value" + policy = "log: 2\nroutes:\n - host: api.example.com\n" + + class _TokenResolver: + def resolve_policy_and_bottle_id( + self, ip: str, identity_token: str = "", + ) -> tuple[str | None, str | None, dict[str, str]]: + del ip, identity_token + return policy, "bottle-a", {"EGRESS_TOKEN_0": secret} + + addon = _addon(Config(routes=())) + addon._resolver = cast(Any, _TokenResolver()) + flow = _with_client_ip(_Flow(_Request(host="api.example.com")), "10.0.0.1") + _run_request(addon, flow) + flow.response = _Response(200, content=f"echo {secret} back") + buf = StringIO() + with patch("sys.stderr", buf): + addon.response(flow) # type: ignore[arg-type] + logged = buf.getvalue() + self.assertIn("egress_response", logged) # LOG_FULL logged the response + self.assertNotIn(secret, logged) # redacted via the resolved env overlay + def test_unattributed_flow_has_no_route_so_frame_passes(self) -> None: # An unattributed source resolves to a deny-all (empty) config, so the # request is blocked at the upgrade and any later frame has no route to