fix(egress): redact the per-bottle token in the now-active multi-tenant response log
test / unit (pull_request) Successful in 1m9s
test / integration (pull_request) Successful in 21s
test / coverage (pull_request) Successful in 1m24s
lint / lint (push) Successful in 2m15s
test / unit (push) Successful in 1m8s
test / coverage (push) Successful in 1m20s
test / integration (push) Successful in 24s
Update Quality Badges / update-badges (push) Successful in 1m18s
test / unit (pull_request) Successful in 1m9s
test / integration (pull_request) Successful in 21s
test / coverage (pull_request) Successful in 1m24s
lint / lint (push) Successful in 2m15s
test / unit (push) Successful in 1m8s
test / coverage (push) Successful in 1m20s
test / integration (push) Successful in 24s
Update Quality Badges / update-badges (push) Successful in 1m18s
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit was merged in pull request #401.
This commit is contained in:
+19
-11
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user