fix: stop leaking stack traces to clients in HTTP 500 responses
Tracebacks exposed file paths, module names, and potentially secret values from local variables. Now logged server-side only; clients receive a generic error message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -189,7 +189,8 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
|||||||
else: os.environ['HERMES_SESSION_KEY'] = old_session_key
|
else: os.environ['HERMES_SESSION_KEY'] = old_session_key
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
put('error', {'message': str(e), 'trace': traceback.format_exc()})
|
print('[webui] stream error:\n' + traceback.format_exc(), flush=True)
|
||||||
|
put('error', {'message': str(e)})
|
||||||
finally:
|
finally:
|
||||||
_clear_thread_env() # TD1: always clear thread-local context
|
_clear_thread_env() # TD1: always clear thread-local context
|
||||||
with STREAMS_LOCK:
|
with STREAMS_LOCK:
|
||||||
|
|||||||
@@ -74,4 +74,5 @@ def handle_upload(handler):
|
|||||||
dest.write_bytes(file_bytes)
|
dest.write_bytes(file_bytes)
|
||||||
return j(handler, {'filename': safe_name, 'path': str(dest), 'size': dest.stat().st_size})
|
return j(handler, {'filename': safe_name, 'path': str(dest), 'size': dest.stat().st_size})
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return j(handler, {'error': str(e), 'trace': _tb.format_exc()}, status=500)
|
print('[webui] upload error: ' + _tb.format_exc(), flush=True)
|
||||||
|
return j(handler, {'error': 'Upload failed'}, status=500)
|
||||||
|
|||||||
@@ -40,7 +40,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
if result is False:
|
if result is False:
|
||||||
return j(self, {'error': 'not found'}, status=404)
|
return j(self, {'error': 'not found'}, status=404)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return j(self, {'error': str(e), 'trace': traceback.format_exc()}, status=500)
|
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||||
|
return j(self, {'error': 'Internal server error'}, status=500)
|
||||||
|
|
||||||
def do_POST(self):
|
def do_POST(self):
|
||||||
self._req_t0 = time.time()
|
self._req_t0 = time.time()
|
||||||
@@ -51,7 +52,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
if result is False:
|
if result is False:
|
||||||
return j(self, {'error': 'not found'}, status=404)
|
return j(self, {'error': 'not found'}, status=404)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return j(self, {'error': str(e), 'trace': traceback.format_exc()}, status=500)
|
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||||
|
return j(self, {'error': 'Internal server error'}, status=500)
|
||||||
|
|
||||||
|
|
||||||
def main():
|
def main():
|
||||||
|
|||||||
@@ -438,3 +438,37 @@ def test_newSession_clears_live_tool_cards(cleanup_test_sessions):
|
|||||||
next_fn = src.find("async function ", new_sess_idx + 10)
|
next_fn = src.find("async function ", new_sess_idx + 10)
|
||||||
new_sess_body = src[new_sess_idx:next_fn]
|
new_sess_body = src[new_sess_idx:next_fn]
|
||||||
assert "clearLiveToolCards" in new_sess_body, "newSession() must call clearLiveToolCards() to clear stale live cards"
|
assert "clearLiveToolCards" in new_sess_body, "newSession() must call clearLiveToolCards() to clear stale live cards"
|
||||||
|
|
||||||
|
|
||||||
|
# ── R16: Stack traces must not leak to clients in 500 responses ────────────
|
||||||
|
|
||||||
|
def test_500_response_has_no_trace_field():
|
||||||
|
"""R16: HTTP 500 responses must not include a 'trace' field.
|
||||||
|
Leaking tracebacks exposes file paths, module names, and potentially
|
||||||
|
secret values from local variables.
|
||||||
|
"""
|
||||||
|
# POST to /api/chat/start with missing required fields to trigger an error
|
||||||
|
data, status = post("/api/chat/start", {})
|
||||||
|
# Should be an error response (4xx or 5xx)
|
||||||
|
assert "trace" not in data, \
|
||||||
|
"Server must not leak stack traces to clients"
|
||||||
|
|
||||||
|
def test_upload_error_has_no_trace_field():
|
||||||
|
"""R16b: Upload 500 responses must not include a 'trace' field."""
|
||||||
|
# Send a POST to /api/upload with invalid content to trigger the error handler
|
||||||
|
req = urllib.request.Request(
|
||||||
|
BASE + "/api/upload",
|
||||||
|
data=b"not-multipart-data",
|
||||||
|
headers={"Content-Type": "text/plain", "Content-Length": "18"},
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
with urllib.request.urlopen(req, timeout=10) as r:
|
||||||
|
body = json.loads(r.read())
|
||||||
|
code = r.status
|
||||||
|
except urllib.error.HTTPError as e:
|
||||||
|
body = json.loads(e.read())
|
||||||
|
code = e.code
|
||||||
|
assert code >= 400, "Invalid upload should return an error status"
|
||||||
|
assert "trace" not in body, \
|
||||||
|
"Upload errors must not leak stack traces to clients"
|
||||||
|
assert "error" in body, "Error responses must include an 'error' key"
|
||||||
|
|||||||
Reference in New Issue
Block a user