Skip to content

Commit 42ea913

Browse files
committed
Merge #803: gate xorq error_info on BUCKAROO_DEBUG
2 parents e946050 + e141d7a commit 42ea913

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

buckaroo/server/xorq_loading.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
"""
99
from __future__ import annotations
1010

11+
import logging
12+
import os
1113
import traceback
1214

1315
from buckaroo.server.window import clamp_window
@@ -16,6 +18,12 @@
1618
XorqInfiniteSampling, _XORQ_ANALYSIS_KLASSES, _expr_count,
1719
window_to_parquet)
1820

21+
# Mirrors ``websocket_handler._BUCKAROO_DEBUG`` — when set, error_info
22+
# carries the full traceback for local debugging. Without it, clients see
23+
# a generic message so source paths and stack frames don't leak.
24+
_BUCKAROO_DEBUG = os.environ.get("BUCKAROO_DEBUG", "").lower() in ("1", "true")
25+
log = logging.getLogger("buckaroo.server.xorq_loading")
26+
1927

2028
class XorqServerDataflow(XorqDataflow):
2129
"""Headless XorqDataflow with infinite sampling.
@@ -91,5 +99,11 @@ def handle_infinite_request_xorq(xorq_dataflow: XorqServerDataflow,
9199
return ({"type": "infinite_resp", "key": payload_args, "data": [],
92100
"length": total_length}, parquet_bytes)
93101
except Exception:
102+
tb = traceback.format_exc()
103+
log.error("xorq infinite_request error: %s", tb)
104+
# Mirrors the pandas-path gate in websocket_handler.py — clients
105+
# in production runs see a generic message; only ``BUCKAROO_DEBUG``
106+
# opens the source-leak channel.
94107
return ({"type": "infinite_resp", "key": payload_args, "data": [],
95-
"length": 0, "error_info": traceback.format_exc()}, b"")
108+
"length": 0,
109+
"error_info": tb if _BUCKAROO_DEBUG else "Request failed"}, b"")

tests/unit/server/test_load_expr.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,46 @@ async def test_ws_infinite_request_clamps_oversized_window(self):
192192
W.MAX_INFINITE_WINDOW = original_max
193193
shutil.rmtree(builds_root, ignore_errors=True)
194194

195+
@tornado.testing.gen_test
196+
async def test_xorq_infinite_request_error_info_no_traceback(self):
197+
"""Regression for #798: the xorq path put ``traceback.format_exc()``
198+
into ``error_info`` unconditionally — leaking server-side source
199+
paths to WS clients. Pandas path gates this behind
200+
``BUCKAROO_DEBUG``; xorq must too.
201+
202+
Trigger the exception with a sort column that doesn't exist in
203+
``merged_sd`` — raises ``KeyError`` inside
204+
``handle_infinite_request_xorq``.
205+
"""
206+
builds_root = tempfile.mkdtemp()
207+
try:
208+
build_path = _build_expr_dir(builds_root)
209+
await _post(self.get_http_port(), "/load_expr",
210+
{"session": "lx-leak", "build_dir": build_path})
211+
212+
ws = await tornado.websocket.websocket_connect(
213+
f"ws://localhost:{self.get_http_port()}/ws/lx-leak")
214+
await ws.read_message() # discard initial_state
215+
216+
ws.write_message(json.dumps({
217+
"type": "infinite_request",
218+
"payload_args": {"start": 0, "end": 5,
219+
"sort": "nonexistent_col", "sort_direction": "asc",
220+
"sourceName": "default", "origEnd": 5}}))
221+
222+
r = json.loads(await ws.read_message())
223+
self.assertEqual(r["type"], "infinite_resp")
224+
self.assertIn("error_info", r)
225+
# The bug: pre-fix this carries a full traceback starting
226+
# with "Traceback (most recent call last):\n File ...".
227+
# Production runs shouldn't leak source paths to clients.
228+
self.assertFalse(r["error_info"].startswith("Traceback"),
229+
f"error_info leaked traceback to client (first 200 chars): "
230+
f"{r['error_info'][:200]!r}")
231+
ws.close()
232+
finally:
233+
shutil.rmtree(builds_root, ignore_errors=True)
234+
195235
@tornado.testing.gen_test
196236
async def test_session_reuse_xorq_then_pandas(self):
197237
"""A client that POSTs /load_expr and then POSTs /load with the

0 commit comments

Comments
 (0)