diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index ed619979bfb..ad82d9fdfef 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1529,6 +1529,7 @@ async def upload_managed_file_stream( ) tmp_path = Path(tmp_name) total = 0 + renamed = False try: with os.fdopen(tmp_fd, "wb") as out: while True: @@ -1540,16 +1541,21 @@ async def upload_managed_file_stream( raise HTTPException(status_code=413, detail="File is too large") out.write(chunk) os.replace(tmp_path, target) + renamed = True except HTTPException: - tmp_path.unlink(missing_ok=True) raise except PermissionError: - tmp_path.unlink(missing_ok=True) raise HTTPException(status_code=403, detail="File is not writable") except OSError as exc: - tmp_path.unlink(missing_ok=True) raise HTTPException(status_code=500, detail=f"Could not write file: {exc}") finally: + # Clean up the temp file on every non-success exit, including + # BaseException paths the `except` clauses above don't catch — most + # importantly asyncio.CancelledError when a browser aborts a large + # upload mid-stream (the exact NS-501 scenario). os.replace clears + # tmp_path on success, so only unlink when the rename didn't happen. + if not renamed: + tmp_path.unlink(missing_ok=True) await file.close() return { diff --git a/pyproject.toml b/pyproject.toml index 6e371126dd2..cab849dc755 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -258,7 +258,7 @@ youtube = [ # `hermes dashboard` (localhost SPA + API). Not in core to keep the default install lean. # starlette==1.0.1 pinned for CVE-2026-48710 (BadHost) — fastapi pulls Starlette # transitively and pre-1.0.1 is the vulnerable range. See the mcp extra above. -web = ["fastapi==0.133.1", "uvicorn[standard]==0.41.0", "starlette==1.0.1", "python-multipart==0.0.20"] +web = ["fastapi==0.133.1", "uvicorn[standard]==0.41.0", "starlette==1.0.1", "python-multipart==0.0.27"] all = [ # Policy (2026-05-12): `[all]` includes only extras that genuinely # CAN'T be lazy-installed via `tools/lazy_deps.py` — i.e. things every diff --git a/tests/hermes_cli/test_web_server_files.py b/tests/hermes_cli/test_web_server_files.py index 46ba18b1355..b295f0ab998 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -436,3 +436,55 @@ def test_stream_upload_large_file_under_cap_succeeds(forced_files_client, monkey assert created.status_code == 200 assert file_path.stat().st_size == len(payload) assert file_path.read_bytes() == payload + + +def test_stream_upload_cleans_temp_on_cancellation(forced_files_client): + """A client disconnect mid-stream (asyncio.CancelledError) must not leak a temp file. + + CancelledError is a BaseException, not an Exception, so it bypasses the + endpoint's ``except`` clauses entirely. The cleanup therefore lives in a + ``finally`` keyed on a success flag — without it, every aborted large + upload (the exact NS-501 scenario) would orphan a partial ``.upload`` temp + file in the target directory. We invoke the endpoint coroutine directly so + the BaseException propagates instead of being swallowed by the test client. + """ + import asyncio + + _client, root = forced_files_client + target = root / "out" / "aborted.bin" + target.parent.mkdir(parents=True, exist_ok=True) + + class _AbortingUpload: + """UploadFile stand-in that yields one chunk then aborts like a dropped client.""" + + filename = "aborted.bin" + + def __init__(self): + self._calls = 0 + + async def read(self, _size): + self._calls += 1 + if self._calls == 1: + return b"partial chunk before the client vanished" + raise asyncio.CancelledError() + + async def close(self): + return None + + request = SimpleNamespace() + + with pytest.raises(asyncio.CancelledError): + asyncio.run( + web_server.upload_managed_file_stream( + request=request, + file=_AbortingUpload(), + path=str(target), + overwrite=True, + ) + ) + + # No partial data was promoted into place ... + assert not target.exists() + # ... and no .upload temp file was left behind. + leftovers = [p.name for p in target.parent.iterdir() if ".upload" in p.name] + assert leftovers == [], f"temp upload files leaked on cancellation: {leftovers}" diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index 98bacbf42a0..4e2159a1a02 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -178,7 +178,7 @@ LAZY_DEPS: dict[str, tuple[str, ...]] = { "fastapi==0.133.1", "uvicorn[standard]==0.41.0", "starlette==1.0.1", # CVE-2026-48710 (BadHost) — keep lazy-install in sync with pyproject [web] - "python-multipart==0.0.20", # FastAPI UploadFile/Form for streaming uploads (NS-501) + "python-multipart==0.0.27", # FastAPI UploadFile/Form for streaming uploads (NS-501) ), # Vision image-resize recovery (Pillow). Pillow is now a CORE dependency # (pyproject `dependencies`), so this entry is a belt-and-suspenders fallback diff --git a/uv.lock b/uv.lock index fc340bdbe89..095b7563311 100644 --- a/uv.lock +++ b/uv.lock @@ -1713,7 +1713,7 @@ requires-dist = [ { name = "pytest-asyncio", marker = "extra == 'dev'", specifier = "==1.3.0" }, { name = "python-dotenv", specifier = "==1.2.2" }, { name = "python-multipart", specifier = ">=0.0.9,<1" }, - { name = "python-multipart", marker = "extra == 'web'", specifier = "==0.0.20" }, + { name = "python-multipart", marker = "extra == 'web'", specifier = "==0.0.27" }, { name = "python-telegram-bot", extras = ["webhooks"], marker = "extra == 'messaging'", specifier = "==22.6" }, { name = "python-telegram-bot", extras = ["webhooks"], marker = "extra == 'termux'", specifier = "==22.6" }, { name = "pywinpty", marker = "sys_platform == 'win32'", specifier = ">=2.0.0,<3" }, @@ -3317,11 +3317,11 @@ wheels = [ [[package]] name = "python-multipart" -version = "0.0.20" +version = "0.0.27" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/f3/87/f44d7c9f274c7ee665a29b885ec97089ec5dc034c7f3fafa03da9e39a09e/python_multipart-0.0.20.tar.gz", hash = "sha256:8dd0cab45b8e23064ae09147625994d090fa46f5b0d1e13af944c331a7fa9d13", size = 37158, upload-time = "2024-12-16T19:45:46.972Z" } +sdist = { url = "https://files.pythonhosted.org/packages/69/9b/f23807317a113dc36e74e75eb265a02dd1a4d9082abc3c1064acd22997c4/python_multipart-0.0.27.tar.gz", hash = "sha256:9870a6a8c5a20a5bf4f07c017bd1489006ff8836cff097b6933355ee2b49b602", size = 44043, upload-time = "2026-04-27T10:51:26.649Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/45/58/38b5afbc1a800eeea951b9285d3912613f2603bdf897a4ab0f4bd7f405fc/python_multipart-0.0.20-py3-none-any.whl", hash = "sha256:8a62d3a8335e06589fe01f2a3e178cdcc632f3fbe0d492ad9ee0ec35aab1f104", size = 24546, upload-time = "2024-12-16T19:45:44.423Z" }, + { url = "https://files.pythonhosted.org/packages/99/78/4126abcbdbd3c559d43e0db7f7b9173fc6befe45d39a2856cc0b8ec2a5a6/python_multipart-0.0.27-py3-none-any.whl", hash = "sha256:6fccfad17a27334bd0193681b369f476eda3409f17381a2d65aa7df3f7275645", size = 29254, upload-time = "2026-04-27T10:51:24.997Z" }, ] [[package]]