fix(egress): scan response + websocket DLP against the resolved per-flow config
In the consolidated (multi-tenant) gateway the addon's static `self.config` is empty — each request's real policy comes from the per-request `/resolve`. `response()` and `websocket_message()` still matched routes against that empty config, so inbound prompt-injection DLP and WebSocket credential/injection DLP silently skipped every scan (fail-open) whenever the gateway ran multi-tenant. This is backend-agnostic: the gateway image (and this addon) is shared by the Firecracker, macOS, and docker consolidated backends. Resolve the per-flow (config, slug, env) once in `request()`, stash it on `flow.metadata`, and have both hooks read it back — falling back to the static single-tenant values for a flow that never passed through `request()`. Reusing the request's one `/resolve` avoids a round-trip per response and per WebSocket frame. Adds multi-tenant regression tests for both hooks that fail against the old fall-open behaviour. Refs: audit issue #400 (finding #2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
+58
-11
@@ -78,6 +78,16 @@ ORCHESTRATOR_URL_ENV = "BOT_BOTTLE_ORCHESTRATOR_URL"
|
||||
# is still stripped defensively (git-http uses that header on its own port).
|
||||
IDENTITY_HEADER = "x-bot-bottle-identity"
|
||||
|
||||
# Per-flow key under which `request()` stashes the resolved (Config, supervise
|
||||
# slug, env) so the later `response()` and `websocket_message()` hooks scan
|
||||
# against the *calling bottle's* policy. In the consolidated (multi-tenant)
|
||||
# gateway the static `self.config` is empty — every request's real policy comes
|
||||
# from the per-request `/resolve` — so a hook that fell back to `self.config`
|
||||
# would find no route and silently skip its DLP scan (fail-open). Resolving once
|
||||
# at the request and reusing it also avoids a `/resolve` round-trip per response
|
||||
# and per WebSocket frame.
|
||||
_FLOW_CTX_KEY = "bot_bottle_egress_ctx"
|
||||
|
||||
|
||||
def _token_from_proxy_auth(header: str) -> str:
|
||||
"""Extract the identity token (the password) from a `Proxy-Authorization:
|
||||
@@ -280,6 +290,36 @@ class EgressAddon:
|
||||
env = {**os.environ, **tokens} if tokens else os.environ
|
||||
return config, slug, env
|
||||
|
||||
def _stash_flow_ctx(
|
||||
self,
|
||||
flow: http.HTTPFlow,
|
||||
config: Config,
|
||||
slug: str,
|
||||
env: "typing.Mapping[str, str]",
|
||||
) -> None:
|
||||
"""Remember the per-flow context `request()` resolved, so the later
|
||||
`response()` / `websocket_message()` hooks reuse it — scanning against
|
||||
the same bottle's policy the request was decided on, with one `/resolve`
|
||||
per flow rather than one per frame."""
|
||||
meta = getattr(flow, "metadata", None)
|
||||
if isinstance(meta, dict):
|
||||
meta[_FLOW_CTX_KEY] = (config, slug, env)
|
||||
|
||||
def _flow_ctx(
|
||||
self, flow: http.HTTPFlow,
|
||||
) -> "tuple[Config, str, typing.Mapping[str, str]]":
|
||||
"""The `(Config, supervise slug, env)` `request()` resolved for this
|
||||
flow, so a later hook scans against the calling bottle's policy — not the
|
||||
empty static config the consolidated gateway carries. Falls back to the
|
||||
single-tenant static values for a flow that never passed through
|
||||
`request()` (or a flow object without metadata)."""
|
||||
meta = getattr(flow, "metadata", None)
|
||||
if isinstance(meta, dict):
|
||||
ctx = meta.get(_FLOW_CTX_KEY)
|
||||
if ctx is not None:
|
||||
return ctx
|
||||
return self.config, self._supervise_slug, os.environ
|
||||
|
||||
def _request_token(self, flow: http.HTTPFlow) -> str:
|
||||
"""The per-bottle identity token for this request, from the proxy
|
||||
credentials — the delivery mechanism (`HTTPS_PROXY=http://id:token@gw`)
|
||||
@@ -319,6 +359,9 @@ class EgressAddon:
|
||||
return
|
||||
|
||||
config, slug, env = self._resolve_flow(flow)
|
||||
# Stash for the response / websocket hooks so their DLP scans use this
|
||||
# bottle's resolved policy, not the empty static config (see _flow_ctx).
|
||||
self._stash_flow_ctx(flow, config, slug, env)
|
||||
|
||||
# DLP outbound scan BEFORE stripping auth — catches tokens the
|
||||
# agent tried to smuggle in any header, path, query param, or body.
|
||||
@@ -586,13 +629,16 @@ class EgressAddon:
|
||||
await asyncio.sleep(TOKEN_ALLOW_POLL_INTERVAL_SECONDS)
|
||||
|
||||
def response(self, flow: http.HTTPFlow) -> None:
|
||||
"""DLP inbound scan on response headers and body."""
|
||||
route = match_route(self.config.routes, flow.request.pretty_host)
|
||||
"""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)
|
||||
route = match_route(config.routes, flow.request.pretty_host)
|
||||
if route is None:
|
||||
return
|
||||
if flow.response is None:
|
||||
return
|
||||
if self.config.log >= LOG_FULL:
|
||||
if config.log >= LOG_FULL:
|
||||
self._log_response(flow)
|
||||
resp_headers = {k.lower(): v for k, v in flow.response.headers.items()}
|
||||
body = flow.response.get_text(strict=False) or ""
|
||||
@@ -610,7 +656,7 @@ class EgressAddon:
|
||||
resp_ctx = {**resp_ctx, "context": result.context}
|
||||
if result.severity == "block":
|
||||
self._block(flow, f"egress DLP: {result.reason}", ctx=resp_ctx)
|
||||
elif result.severity == "warn" and self.config.log >= LOG_BLOCKS:
|
||||
elif result.severity == "warn" and config.log >= LOG_BLOCKS:
|
||||
sys.stderr.write(
|
||||
json.dumps({
|
||||
"event": "egress_warn",
|
||||
@@ -621,7 +667,10 @@ class EgressAddon:
|
||||
)
|
||||
|
||||
def websocket_message(self, flow: http.HTTPFlow) -> None:
|
||||
"""DLP scan on WebSocket frames.
|
||||
"""DLP scan on WebSocket frames, against the calling bottle's resolved
|
||||
config (see `_flow_ctx`). `request()` resolves and stashes the per-flow
|
||||
(config, slug, env) at the upgrade, so both the multi-tenant and
|
||||
single-tenant gateways scan here.
|
||||
|
||||
Outbound frames (from_client) are scanned for credential leakage;
|
||||
inbound frames are scanned for prompt injection. On a block the
|
||||
@@ -630,10 +679,8 @@ class EgressAddon:
|
||||
"""
|
||||
if flow.websocket is None: # type: ignore[union-attr]
|
||||
return
|
||||
# WebSocket DLP runs against the static config only (single-tenant); in
|
||||
# the consolidated gateway self.config has no routes, so this is inert
|
||||
# until websocket routing is made source-IP-aware (a separate slice).
|
||||
route = match_route(self.config.routes, flow.request.pretty_host)
|
||||
config, slug, env = self._flow_ctx(flow)
|
||||
route = match_route(config.routes, flow.request.pretty_host)
|
||||
if route is None:
|
||||
return
|
||||
message = flow.websocket.messages[-1] # type: ignore[union-attr]
|
||||
@@ -642,8 +689,8 @@ class EgressAddon:
|
||||
# A WebSocket data frame is not an HTTP request line, so CRLF is
|
||||
# not an injection vector here — scan only for credential leakage.
|
||||
result = scan_outbound(
|
||||
route, content, os.environ,
|
||||
safe_tokens=self._safe_tokens_for(self._supervise_slug), crlf_text="",
|
||||
route, content, env,
|
||||
safe_tokens=self._safe_tokens_for(slug), crlf_text="",
|
||||
)
|
||||
if result is not None and result.severity == "block":
|
||||
sys.stderr.write(f"egress DLP: {result.reason}\n")
|
||||
|
||||
@@ -141,6 +141,10 @@ class _Flow:
|
||||
self.response = response
|
||||
self.websocket: Any = None
|
||||
self.killed = False
|
||||
# mitmproxy flows carry a per-flow `metadata` dict for addon use; the
|
||||
# egress addon stashes the resolved (config, slug, env) there in
|
||||
# request() so the response/websocket hooks reuse it.
|
||||
self.metadata: dict[str, Any] = {}
|
||||
|
||||
def kill(self) -> None:
|
||||
self.killed = True
|
||||
@@ -859,5 +863,63 @@ class TestSuperviseMultiTenant(unittest.TestCase):
|
||||
self.assertNotIn(_OPENAI_KEY, addon._safe_tokens_for(""))
|
||||
|
||||
|
||||
class TestMultiTenantInboundDlp(unittest.TestCase):
|
||||
"""Consolidated gateway: the response + websocket DLP hooks must scan
|
||||
against the *calling bottle's* config, resolved by source IP in request()
|
||||
and reused here. The static `self.config` is empty in this mode, so before
|
||||
the flow-context stash these hooks silently skipped every scan (fail-open).
|
||||
"""
|
||||
|
||||
def _consolidated_addon(self) -> EgressAddon:
|
||||
addon = _addon(Config(routes=())) # empty static config, as in prod
|
||||
addon._resolver = cast(Any, _CtxResolver({"10.0.0.1": "bottle-a"}))
|
||||
return addon
|
||||
|
||||
def test_response_injection_blocked_after_request_resolves(self) -> None:
|
||||
addon = self._consolidated_addon()
|
||||
flow = _with_client_ip(_Flow(_Request(host="api.example.com")), "10.0.0.1")
|
||||
_run_request(addon, flow) # resolves + stashes bottle-a's allowlist
|
||||
self.assertIsNone(flow.response) # request forwarded
|
||||
flow.response = _Response(200, content=_INJECTION_BLOCK)
|
||||
addon.response(flow) # type: ignore[arg-type]
|
||||
assert flow.response is not None
|
||||
# Empty static config would have found no route and left this 200.
|
||||
self.assertEqual(403, flow.response.status_code)
|
||||
|
||||
def test_websocket_outbound_token_killed_after_request_resolves(self) -> None:
|
||||
addon = self._consolidated_addon()
|
||||
flow = _with_client_ip(_Flow(_Request(host="api.example.com")), "10.0.0.1")
|
||||
_run_request(addon, flow) # the ws upgrade resolves + stashes the config
|
||||
flow.websocket = _WebSocketData(
|
||||
[_Message(f"k={_OPENAI_KEY}".encode(), from_client=True)]
|
||||
)
|
||||
addon.websocket_message(flow) # type: ignore[arg-type]
|
||||
self.assertTrue(flow.killed) # scanned against bottle-a's route now
|
||||
|
||||
def test_websocket_inbound_injection_killed_after_request_resolves(self) -> None:
|
||||
addon = self._consolidated_addon()
|
||||
flow = _with_client_ip(_Flow(_Request(host="api.example.com")), "10.0.0.1")
|
||||
_run_request(addon, flow)
|
||||
flow.websocket = _WebSocketData(
|
||||
[_Message(_INJECTION_BLOCK.encode(), from_client=False)]
|
||||
)
|
||||
addon.websocket_message(flow) # type: ignore[arg-type]
|
||||
self.assertTrue(flow.killed)
|
||||
|
||||
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
|
||||
# scan against — it passes rather than being attributed to a bottle.
|
||||
addon = self._consolidated_addon()
|
||||
flow = _with_client_ip(_Flow(_Request(host="api.example.com")), "10.9.9.9")
|
||||
_run_request(addon, flow)
|
||||
self.assertIsNotNone(flow.response) # blocked at the upgrade
|
||||
flow.websocket = _WebSocketData(
|
||||
[_Message(f"k={_OPENAI_KEY}".encode(), from_client=True)]
|
||||
)
|
||||
addon.websocket_message(flow) # type: ignore[arg-type]
|
||||
self.assertFalse(flow.killed)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user