fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)

Follow-up to #47663 (streaming multipart upload), fixing two issues that
landed with it.

1. Temp file leaked on client disconnect. The streaming upload endpoint's
   except chain caught only HTTPException / PermissionError / OSError — all
   Exception subclasses. asyncio.CancelledError, raised when a browser aborts
   a large upload mid-stream (the exact NS-501 scenario), is a BaseException,
   so it bypassed every except clause and reached a finally that only closed
   the file handle and never unlinked the temp file. Every aborted large
   upload orphaned a partial `.{name}.*.upload` file (up to ~100 MB) in the
   target directory. Cleanup now lives in finally, keyed on a `renamed`
   success flag, so the temp file is removed on every non-success exit
   including BaseException paths. Added test_stream_upload_cleans_temp_on_cancellation,
   which fails on the pre-fix code (leaks the temp file) and passes with the fix.

2. python-multipart pinned to ==0.0.27 instead of ==0.0.20. The package was
   already resolved at 0.0.27 transitively (via daytona) before #47663; the
   explicit ==0.0.20 pin in the [web] extra and the tool.dashboard lazy-install
   set downgraded it. Bumped both to ==0.0.27 and regenerated with `uv lock`,
   keeping the lockfile coherent. The base dependency stays >=0.0.9,<1.
This commit is contained in:
kshitijk4poor 2026-06-18 11:32:18 +05:30
parent c661634537
commit 6752da9a77
5 changed files with 67 additions and 9 deletions

View file

@ -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 {

View file

@ -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

View file

@ -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}"

View file

@ -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
View file

@ -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]]