mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-21 10:22:18 +00:00
Merge pull request #48259 from NousResearch/fix/ns501-multipart-upload-salvage
fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
This commit is contained in:
commit
6278bca055
5 changed files with 67 additions and 9 deletions
|
|
@ -1557,6 +1557,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:
|
||||
|
|
@ -1568,16 +1569,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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
8
uv.lock
generated
8
uv.lock
generated
|
|
@ -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]]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue