From a93e8ba890450723cafb71a8d3310bc0f51f006c Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes da Silva Date: Fri, 14 Aug 2026 20:53:13 -0300 Subject: [PATCH] refactor(http): route every reject-before-read through one arming helper Replaces the inline `handler.close_connection = True` at each reject-before-read site with api.helpers.arm_connection_close(), the shared mechanism the review asked for. Behaviour is unchanged at every real handler; the helper adds the same stub tolerance the read side just got, so a handler that refuses the attribute can never turn a 4xx into a 500. Sites: _check_csrf (origin + token), the sidecar provenance 403 (body-reading calls only), the rate-limited CSP report, /api/health/restart, the deprecated process-complete-ack 410, and all four upload handlers' pre-read length/boundary rejections. --- api/routes.py | 13 +++++++------ api/upload.py | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/api/routes.py b/api/routes.py index ecd188ff..92e5c617 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2974,6 +2974,7 @@ from api.helpers import ( require, bad, safe_resolve, + arm_connection_close, j, t, read_body, @@ -5873,7 +5874,7 @@ def _check_csrf(handler) -> bool: if not _check_same_origin_browser_request(handler): # CSRF checks run before read_body(). Close rather than reusing an # HTTP/1.1 connection whose unread body would corrupt the next request. - handler.close_connection = True + arm_connection_close(handler) return False if not _is_browser_unsafe_request(handler): return True # non-browser clients (curl, MCP, agent) have no Origin/Referer @@ -5886,7 +5887,7 @@ def _check_csrf(handler) -> bool: submitted = handler.headers.get(CSRF_HEADER_NAME) or handler.headers.get("X-CSRF-Token") if verify_csrf_token(cookie_val or "", submitted or ""): return True - handler.close_connection = True + arm_connection_close(handler) return _set_csrf_failure_reason(handler, "token_mismatch") @@ -6071,7 +6072,7 @@ def _handle_extension_sidecar_proxy( # unread body would corrupt the next request (same class as _check_csrf). # GET (read_request_body=False) has nothing unread — keep-alive survives. if read_request_body: - handler.close_connection = True + arm_connection_close(handler) return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) try: request_body = _read_body_bytes(handler) if read_request_body else None @@ -6516,7 +6517,7 @@ def _handle_csp_report(handler) -> bool: ) # Rate-limit rejection runs before the body is read; close-and-advertise # so the unread report can't corrupt the next pooled request. - handler.close_connection = True + arm_connection_close(handler) return _send_no_content(handler) payload = _read_csp_report_payload(handler) @@ -12910,7 +12911,7 @@ def _handle_health_restart(handler) -> bool: """Restart the Hermes messaging gateway service.""" # This endpoint never consumes its request body on any outcome; close so a # supplied body can't corrupt the next pooled request. - handler.close_connection = True + arm_connection_close(handler) outcome = restart_active_profile_gateway() if outcome.get("status") == "completed": @@ -15236,7 +15237,7 @@ def handle_post(handler, parsed) -> bool: # The 410 runs before the stale tab's JSON body is read; # close-and-advertise so those unread bytes can't corrupt the next # pooled request. - handler.close_connection = True + arm_connection_close(handler) j( handler, { diff --git a/api/upload.py b/api/upload.py index 942f8f7c..30abef3b 100644 --- a/api/upload.py +++ b/api/upload.py @@ -8,7 +8,7 @@ import tempfile from pathlib import Path from api.config import MAX_UPLOAD_BYTES, STATE_DIR -from api.helpers import j +from api.helpers import arm_connection_close, j from api.models import get_session from api.profiles import _profiles_match, get_active_profile_name as _get_active_profile_name from api.workspace import ( @@ -47,7 +47,7 @@ _MAX_EXTRACTED_BYTES = 10 * MAX_UPLOAD_BYTES def _reject_before_read(handler, message, status): """Reject without reading the body; close so unread bytes can't poison HTTP/1.1 reuse.""" - handler.close_connection = True + arm_connection_close(handler) return j(handler, {'error': message}, status=status) @@ -217,7 +217,7 @@ def handle_upload(handler): content_length = int(handler.headers.get('Content-Length', 0) or 0) except (TypeError, ValueError): # A non-numeric Content-Length is rejected before the body is read. - handler.close_connection = True + arm_connection_close(handler) raise ValueError('Invalid Content-Length') from None if content_length > MAX_UPLOAD_BYTES: return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413) @@ -226,7 +226,7 @@ def handle_upload(handler): except ValueError: # parse_multipart validates Content-Length/boundary before reading; # a ValueError here means the body was never consumed. - handler.close_connection = True + arm_connection_close(handler) raise session_id = fields.get('session_id', '') if 'file' not in files: @@ -418,7 +418,7 @@ def handle_upload_extract(handler): content_length = int(handler.headers.get('Content-Length', 0) or 0) except (TypeError, ValueError): # A non-numeric Content-Length is rejected before the body is read. - handler.close_connection = True + arm_connection_close(handler) raise ValueError('Invalid Content-Length') from None if content_length > MAX_UPLOAD_BYTES: return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413) @@ -427,7 +427,7 @@ def handle_upload_extract(handler): except ValueError: # parse_multipart validates Content-Length/boundary before reading; # a ValueError here means the body was never consumed. - handler.close_connection = True + arm_connection_close(handler) raise session_id = fields.get('session_id', '') if 'file' not in files: @@ -461,7 +461,7 @@ def handle_transcribe(handler): content_length = int(handler.headers.get('Content-Length', 0) or 0) except (TypeError, ValueError): # A non-numeric Content-Length is rejected before the body is read. - handler.close_connection = True + arm_connection_close(handler) raise ValueError('Invalid Content-Length') from None if content_length > MAX_UPLOAD_BYTES: return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413) @@ -470,7 +470,7 @@ def handle_transcribe(handler): except ValueError: # parse_multipart validates Content-Length/boundary before reading; # a ValueError here means the body was never consumed. - handler.close_connection = True + arm_connection_close(handler) raise if 'file' not in files: return j(handler, {'error': 'No file field in request'}, status=400) @@ -656,7 +656,7 @@ def handle_workspace_upload(handler): content_length = int(handler.headers.get('Content-Length', 0) or 0) except (TypeError, ValueError): # A non-numeric Content-Length is rejected before the body is read. - handler.close_connection = True + arm_connection_close(handler) raise ValueError('Invalid Content-Length') from None if content_length > MAX_UPLOAD_BYTES: return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413) @@ -666,7 +666,7 @@ def handle_workspace_upload(handler): except ValueError: # parse_multipart validates Content-Length/boundary before reading; # a ValueError here means the body was never consumed. - handler.close_connection = True + arm_connection_close(handler) raise session_id = fields.get('session_id', '') subpath = fields.get('path', '')