Skip to content

Commit c926d44

Browse files
committed
fix(craft): bound recorded-response memory, editor-id links, draft view
An undeclared-length recorded response streams through a capped observer instead of buffering whole, and decoding is bounded (zlib with a capped output) so neither a chunked origin nor a compression bomb can grow proxy memory before the refine cap applies. Google editor APIs link through their product id keys (documentId, spreadsheetId, presentationId, which carry no mimeType), and Gmail draft receipts link the drafts view since a draft is not in #all until sent.
1 parent a1b7b10 commit c926d44

4 files changed

Lines changed: 144 additions & 29 deletions

File tree

backend/onyx/sandbox_proxy/addons/gate.py

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import operator
1212
import socket
1313
import threading
14+
import zlib
1415
from collections.abc import Callable
1516
from dataclasses import dataclass
1617
from typing import Protocol
@@ -217,6 +218,43 @@ class _ApprovalGrant:
217218
RECEIPT_REFINE_MAX_BODY_BYTES = 1024 * 1024
218219

219220

221+
class _CappedBodyCapture:
222+
"""Streams a recorded flow's response through untouched while keeping a
223+
copy for the extractors, up to the refine cap. Oversize drops the copy."""
224+
225+
def __init__(self) -> None:
226+
self.body = bytearray()
227+
self.overflowed = False
228+
229+
def __call__(self, chunk: bytes) -> bytes:
230+
if chunk and not self.overflowed:
231+
self.body.extend(chunk)
232+
if len(self.body) > RECEIPT_REFINE_MAX_BODY_BYTES:
233+
self.overflowed = True
234+
self.body = bytearray()
235+
return chunk
236+
237+
238+
def _bounded_decode(data: bytes, content_encoding: str) -> bytes | None:
239+
"""Undo Content-Encoding without trusting it: output is capped so a small
240+
compressed body cannot balloon in memory. Unsupported or broken codings
241+
refine nothing rather than risk an unbounded decode."""
242+
coding = content_encoding.strip().lower()
243+
if coding in ("", "identity"):
244+
return data if len(data) <= RECEIPT_REFINE_MAX_BODY_BYTES else None
245+
if coding not in ("gzip", "deflate"):
246+
return None
247+
wbits = 16 + zlib.MAX_WBITS if coding == "gzip" else zlib.MAX_WBITS
248+
try:
249+
decompressor = zlib.decompressobj(wbits=wbits)
250+
decoded = decompressor.decompress(data, RECEIPT_REFINE_MAX_BODY_BYTES + 1)
251+
except zlib.error:
252+
return None
253+
if len(decoded) > RECEIPT_REFINE_MAX_BODY_BYTES or decompressor.unconsumed_tail:
254+
return None
255+
return decoded
256+
257+
220258
class ParkedApprovals:
221259
"""Approvals the proxy is currently parked on, grouped by tenant.
222260
@@ -376,15 +414,18 @@ def responseheaders(self, flow: http.HTTPFlow) -> None:
376414
if flow.response is None:
377415
return
378416
# A recorded flow's response feeds the receipt extractors: buffer it
379-
# unless declared oversize (chunked declares nothing and must buffer).
380-
# The decoded-length cap in `response` bounds the read either way.
417+
# when its declared size fits, otherwise stream through a capped
418+
# observer so chunked or lying origins can never grow proxy memory.
381419
if RECEIPT_FLOW_KEY in flow.metadata:
382420
declared = flow.response.headers.get("content-length")
383421
try:
384422
length = int(declared) if declared is not None else None
385423
except ValueError:
386424
length = None
387-
if length is None or length <= RECEIPT_REFINE_MAX_BODY_BYTES:
425+
if length is not None and length <= RECEIPT_REFINE_MAX_BODY_BYTES:
426+
return
427+
if length is None:
428+
flow.response.stream = _CappedBodyCapture()
388429
return
389430
flow.response.stream = True
390431

@@ -627,12 +668,21 @@ async def response(self, flow: http.HTTPFlow) -> None:
627668
return
628669
confirmed = flow.response is not None and flow.response.status_code < 400
629670
response_body: bytes | None = None
630-
if flow.response is not None and not flow.response.stream:
631-
# get_content undoes Content-Encoding. raw_content is still
632-
# compressed on the wire and would parse as garbage.
633-
decoded = flow.response.get_content(strict=False)
634-
if decoded is not None and len(decoded) <= RECEIPT_REFINE_MAX_BODY_BYTES:
635-
response_body = decoded
671+
if flow.response is not None:
672+
stream = flow.response.stream
673+
wire: bytes | None = None
674+
if isinstance(stream, _CappedBodyCapture):
675+
wire = bytes(stream.body) if not stream.overflowed else None
676+
elif not stream:
677+
raw = flow.response.raw_content
678+
if raw is not None and len(raw) <= RECEIPT_REFINE_MAX_BODY_BYTES:
679+
wire = raw
680+
if wire is not None:
681+
# The wire body is still Content-Encoding compressed and
682+
# would parse as garbage undecoded.
683+
response_body = _bounded_decode(
684+
wire, flow.response.headers.get("content-encoding", "")
685+
)
636686
await self._finalize_recorded(
637687
recorded,
638688
ReceiptStatus.CONFIRMED if confirmed else ReceiptStatus.FAILED,

backend/onyx/sandbox_proxy/receipt_extractors.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def response_facts(action_type: str, response_body: bytes | None) -> ResponseFac
137137
if action_type.startswith("gdrive."):
138138
return _gdrive_facts(body)
139139
if action_type.startswith("gmail."):
140-
return _gmail_facts(body)
140+
return _gmail_facts(action_type, body)
141141
if action_type.startswith("linear."):
142142
return _linear_facts(body)
143143
return ResponseFacts()
@@ -185,25 +185,38 @@ def _slack_facts(action_type: str, body: dict[str, Any]) -> ResponseFacts:
185185
return ResponseFacts(link=link, operation_key=operation_key)
186186

187187

188+
# The dedicated editor APIs name their id after the product and return no
189+
# mimeType, so the key alone picks the link.
190+
_GDRIVE_LINKS_BY_ID_KEY = {
191+
"documentId": "https://docs.google.com/document/d/{id}",
192+
"spreadsheetId": "https://docs.google.com/spreadsheets/d/{id}",
193+
"presentationId": "https://docs.google.com/presentation/d/{id}",
194+
}
195+
196+
188197
def _gdrive_facts(body: dict[str, Any]) -> ResponseFacts:
189-
file_id = _safe_id(
190-
body.get("id") or body.get("documentId") or body.get("spreadsheetId")
191-
)
198+
for key, template in _GDRIVE_LINKS_BY_ID_KEY.items():
199+
editor_id = _safe_id(body.get(key))
200+
if editor_id is not None:
201+
return ResponseFacts(link=template.format(id=editor_id))
202+
file_id = _safe_id(body.get("id"))
192203
if file_id is None:
193204
return ResponseFacts()
194205
mime = body.get("mimeType")
195206
template = _GDRIVE_LINKS_BY_MIME.get(str(mime), _GDRIVE_GENERIC_LINK)
196207
return ResponseFacts(link=template.format(id=file_id))
197208

198209

199-
def _gmail_facts(body: dict[str, Any]) -> ResponseFacts:
210+
def _gmail_facts(action_type: str, body: dict[str, Any]) -> ResponseFacts:
200211
# Drafts nest the message, sends carry it at the top level.
201212
nested = body.get("message")
202213
message = nested if isinstance(nested, dict) else body
203214
message_id = _safe_id(message.get("id"))
204-
if message_id is not None:
205-
return ResponseFacts(link=f"https://mail.google.com/mail/u/0/#all/{message_id}")
206-
return ResponseFacts()
215+
if message_id is None:
216+
return ResponseFacts()
217+
# A draft is not in #all until sent, so link the drafts view instead.
218+
view = "drafts" if action_type.startswith("gmail.drafts.") else "all"
219+
return ResponseFacts(link=f"https://mail.google.com/mail/u/0/#{view}/{message_id}")
207220

208221

209222
def _linear_facts(body: dict[str, Any]) -> ResponseFacts:

backend/tests/unit/sandbox_proxy/test_gate_receipts.py

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from onyx.sandbox_proxy.addons.gate import (
2929
RECEIPT_FLOW_KEY,
3030
GateAddon,
31+
_CappedBodyCapture,
3132
_IdentityResolver,
3233
)
3334
from onyx.sandbox_proxy.credential_injection import CredentialInjectionDispatcher
@@ -226,24 +227,31 @@ def test_oversized_decoded_body_refines_nothing(
226227
asyncio.run(addon.response(flow))
227228
assert calls[0]["response_body"] is None
228229

229-
# An undecodable body falls back to its raw bytes rather than raising;
230-
# the extractors then simply fail to parse it.
230+
# An undecodable body refines nothing rather than raising.
231231
flow = _recorded_flow(200)
232232
flow.response.headers["content-encoding"] = "gzip"
233233
flow.response.raw_content = b"not gzip"
234234
asyncio.run(addon.response(flow))
235-
assert calls[1]["response_body"] == b"not gzip"
235+
assert calls[1]["response_body"] is None
236236

237237

238-
def test_responseheaders_buffers_recorded_flows_without_declared_length() -> None:
238+
def test_responseheaders_caps_undeclared_recorded_flows(
239+
monkeypatch: pytest.MonkeyPatch,
240+
) -> None:
239241
addon = _addon()
240242

241-
# Chunked responses declare no length, but a recorded flow still buffers
242-
# so the extractors can read it. Anything else streams as usual.
243-
recorded = _recorded_flow(200)
244-
del recorded.response.headers["content-length"]
245-
addon.responseheaders(recorded)
246-
assert recorded.response.stream is False
243+
# Chunked responses declare no length, so a recorded flow streams through
244+
# a capped observer the response hook reads. Small declared bodies buffer,
245+
# oversize ones and unrecorded flows stream unobserved.
246+
chunked = _recorded_flow(200)
247+
del chunked.response.headers["content-length"]
248+
addon.responseheaders(chunked)
249+
capture = chunked.response.stream
250+
assert isinstance(capture, _CappedBodyCapture)
251+
252+
small = _recorded_flow(200)
253+
addon.responseheaders(small)
254+
assert small.response.stream is False
247255

248256
declared_oversize = _recorded_flow(200)
249257
declared_oversize.response.headers["content-length"] = str(2 * 1024 * 1024)
@@ -254,6 +262,35 @@ def test_responseheaders_buffers_recorded_flows_without_declared_length() -> Non
254262
addon.responseheaders(unrecorded)
255263
assert unrecorded.response is not None and unrecorded.response.stream is True
256264

265+
# The observer passes chunks through untouched and the response hook
266+
# reads the copy, so chunked provider responses still refine.
267+
calls = _finalize_capture(monkeypatch)
268+
payload = b'{"ok": true}'
269+
assert capture(payload) == payload
270+
assert capture(b"") == b""
271+
asyncio.run(addon.response(chunked))
272+
assert calls[0]["response_body"] == payload
273+
274+
275+
def test_capped_capture_drops_oversized_streams(
276+
monkeypatch: pytest.MonkeyPatch,
277+
) -> None:
278+
addon = _addon()
279+
calls = _finalize_capture(monkeypatch)
280+
flow = _recorded_flow(200)
281+
del flow.response.headers["content-length"]
282+
addon.responseheaders(flow)
283+
capture = flow.response.stream
284+
assert isinstance(capture, _CappedBodyCapture)
285+
286+
chunk = b"x" * (512 * 1024)
287+
for _ in range(5):
288+
assert capture(chunk) == chunk
289+
assert capture.overflowed and not capture.body
290+
291+
asyncio.run(addon.response(flow))
292+
assert calls[0]["response_body"] is None
293+
257294

258295
def test_always_policy_writes_still_record(
259296
monkeypatch: pytest.MonkeyPatch,

backend/tests/unit/sandbox_proxy/test_receipt_extractors.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"""
88

99
import json
10+
from typing import Any
1011
from urllib.parse import urlencode
1112

1213
import pytest
@@ -21,7 +22,7 @@
2122
)
2223

2324

24-
def _body(payload: dict) -> bytes:
25+
def _body(payload: dict[str, Any]) -> bytes:
2526
return json.dumps(payload).encode()
2627

2728

@@ -129,10 +130,24 @@ def test_gdrive_links_follow_the_mime_type(mime: str, prefix: str) -> None:
129130
def test_gmail_send_and_draft_shapes_both_link() -> None:
130131
sent = response_facts("gmail.messages.send", _body({"id": "m1", "threadId": "t1"}))
131132
assert sent.link == "https://mail.google.com/mail/u/0/#all/m1"
133+
# A draft is not in #all until sent, so its link opens the drafts view.
132134
draft = response_facts(
133135
"gmail.drafts.create", _body({"id": "d1", "message": {"id": "m2"}})
134136
)
135-
assert draft.link == "https://mail.google.com/mail/u/0/#all/m2"
137+
assert draft.link == "https://mail.google.com/mail/u/0/#drafts/m2"
138+
139+
140+
@pytest.mark.parametrize(
141+
"key,prefix",
142+
[
143+
("documentId", "https://docs.google.com/document/d/"),
144+
("spreadsheetId", "https://docs.google.com/spreadsheets/d/"),
145+
("presentationId", "https://docs.google.com/presentation/d/"),
146+
],
147+
)
148+
def test_gdrive_editor_api_ids_link_without_a_mime_type(key: str, prefix: str) -> None:
149+
facts = response_facts("gdrive.slides.create", _body({key: "p123"}))
150+
assert facts.link == f"{prefix}p123"
136151

137152

138153
def test_linear_errors_fail_and_only_shaped_urls_pass() -> None:

0 commit comments

Comments
 (0)