diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 70f39162cf87..ed619979bfb7 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -70,7 +70,10 @@ from gateway.status import ( from utils import env_var_enabled try: - from fastapi import FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect + from fastapi import ( + FastAPI, File, Form, HTTPException, Request, UploadFile, + WebSocket, WebSocketDisconnect, + ) from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, HTMLResponse, JSONResponse, Response from fastapi.staticfiles import StaticFiles @@ -82,7 +85,10 @@ except ImportError: try: from tools.lazy_deps import ensure as _lazy_ensure _lazy_ensure("tool.dashboard", prompt=False) - from fastapi import FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect + from fastapi import ( + FastAPI, File, Form, HTTPException, Request, UploadFile, + WebSocket, WebSocketDisconnect, + ) from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, HTMLResponse, JSONResponse, Response from fastapi.staticfiles import StaticFiles @@ -1486,6 +1492,74 @@ async def upload_managed_file(payload: ManagedFileUpload, request: Request): } +# Stream uploads to disk in fixed-size chunks. The legacy JSON endpoint above +# buffers the whole file as a base64 data URL in a JSON body, which (a) inflates +# the payload ~33%, (b) holds the entire file (plus its decoded copy) in memory, +# and (c) reliably trips upstream proxy body-size/timeout limits with a 502 on +# large backup archives (NS-501). This multipart endpoint reads the request body +# in 1 MiB chunks straight to a temp file, enforces the size cap as it goes, and +# atomically renames into place — constant memory, no base64 inflation. +_UPLOAD_CHUNK_BYTES = 1024 * 1024 + + +@app.post("/api/files/upload-stream") +async def upload_managed_file_stream( + request: Request, + file: UploadFile = File(...), + path: str = Form(...), + overwrite: bool = Form(True), +): + policy, target, display_path = _resolve_managed_path(path, request, for_write=True) + if target.exists() and target.is_dir(): + raise HTTPException(status_code=409, detail="A directory already exists at that path") + if target.exists() and not overwrite: + raise HTTPException(status_code=409, detail="File already exists") + + try: + target.parent.mkdir(parents=True, exist_ok=True) + except PermissionError: + raise HTTPException(status_code=403, detail="File is not writable") + except OSError as exc: + raise HTTPException(status_code=500, detail=f"Could not create parent directory: {exc}") + + # Write to a sibling temp file first so a partial/aborted upload never + # clobbers an existing file, then atomically rename into place. + tmp_fd, tmp_name = tempfile.mkstemp( + prefix=f".{target.name}.", suffix=".upload", dir=str(target.parent) + ) + tmp_path = Path(tmp_name) + total = 0 + try: + with os.fdopen(tmp_fd, "wb") as out: + while True: + chunk = await file.read(_UPLOAD_CHUNK_BYTES) + if not chunk: + break + total += len(chunk) + if total > _MANAGED_FILE_MAX_BYTES: + raise HTTPException(status_code=413, detail="File is too large") + out.write(chunk) + os.replace(tmp_path, target) + 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: + await file.close() + + return { + "ok": True, + "entry": _managed_file_entry(policy, target), + "path": display_path, + **_managed_response_meta(policy), + } + + @app.post("/api/files/mkdir") async def create_managed_directory(payload: ManagedDirectoryCreate, request: Request): policy, target, display_path = _resolve_managed_path(payload.path, request, for_write=True) diff --git a/pyproject.toml b/pyproject.toml index 9267f61b24b5..6e371126dd25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,6 +106,11 @@ dependencies = [ "pathspec==1.1.1", "fastapi>=0.104.0,<1", "uvicorn[standard]>=0.24.0,<1", + # Streaming multipart uploads for the dashboard file manager (NS-501). + # FastAPI's UploadFile/Form depend on python-multipart; it is NOT pulled in + # by fastapi itself, so the dashboard's multipart upload endpoint would 500 + # without an explicit dependency here (and in the `web` extra below). + "python-multipart>=0.0.9,<1", "ptyprocess>=0.7.0,<1; sys_platform != 'win32'", "pywinpty>=2.0.0,<3; sys_platform == 'win32'", # Image resize recovery for the vision tools. Pillow shrinks oversized images @@ -253,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"] +web = ["fastapi==0.133.1", "uvicorn[standard]==0.41.0", "starlette==1.0.1", "python-multipart==0.0.20"] 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 02096a616aed..46ba18b1355b 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -331,3 +331,108 @@ def test_hosted_policy_locks_to_opt_data(monkeypatch): assert str(policy.locked_root) == "/opt/data" assert policy.can_change_path is False + + +# --------------------------------------------------------------------------- +# Streaming multipart upload (/api/files/upload-stream) — NS-501 +# --------------------------------------------------------------------------- + + +def test_stream_upload_roundtrip(forced_files_client): + """The multipart endpoint writes raw bytes to disk and reports the entry.""" + client, root = forced_files_client + file_path = root / "out" / "backup.zip" + payload = b"PK\x03\x04 not really a zip but binary enough \x00\x01\x02" + + created = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "true"}, + files={"file": ("backup.zip", payload, "application/zip")}, + ) + assert created.status_code == 200, created.text + assert created.json()["entry"]["path"] == str(file_path) + assert created.json()["locked_root"] == str(root) + # Bytes land verbatim — no base64 round-trip, no corruption. + assert file_path.read_bytes() == payload + + +def test_stream_upload_rejects_oversized_without_clobbering(forced_files_client, monkeypatch): + """Over-limit uploads return 413 and never overwrite an existing file. + + The size cap is enforced while streaming (not after buffering), and the + temp-file + atomic-rename design means a rejected upload leaves any + pre-existing file at the target path untouched. + """ + client, root = forced_files_client + file_path = root / "out" / "big.bin" + + # Seed an existing file at the target path. + seeded = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "true"}, + files={"file": ("big.bin", b"original-contents", "application/octet-stream")}, + ) + assert seeded.status_code == 200 + assert file_path.read_bytes() == b"original-contents" + + # Shrink the cap so a small payload trips it deterministically. + monkeypatch.setattr(web_server, "_MANAGED_FILE_MAX_BYTES", 8) + rejected = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "true"}, + files={"file": ("big.bin", b"way too many bytes for the cap", "application/octet-stream")}, + ) + assert rejected.status_code == 413 + # The original file must survive a rejected overwrite. + assert file_path.read_bytes() == b"original-contents" + # No stray temp files left behind in the directory. + leftovers = [p.name for p in file_path.parent.iterdir() if ".upload" in p.name] + assert leftovers == [], f"temp upload files leaked: {leftovers}" + + +def test_stream_upload_respects_overwrite_false(forced_files_client): + client, root = forced_files_client + file_path = root / "keep.txt" + + first = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "true"}, + files={"file": ("keep.txt", b"first", "text/plain")}, + ) + assert first.status_code == 200 + + conflict = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "false"}, + files={"file": ("keep.txt", b"second", "text/plain")}, + ) + assert conflict.status_code == 409 + assert file_path.read_bytes() == b"first" + + +def test_stream_upload_stays_under_forced_root(forced_files_client): + """A relative path with traversal can't escape the locked root.""" + client, root = forced_files_client + escaped = client.post( + "/api/files/upload-stream", + data={"path": "../../etc/evil.txt", "overwrite": "true"}, + files={"file": ("evil.txt", b"nope", "text/plain")}, + ) + assert escaped.status_code in (400, 403) + + +def test_stream_upload_large_file_under_cap_succeeds(forced_files_client, monkeypatch): + """A multi-chunk payload (larger than the 1 MiB chunk) streams correctly.""" + client, root = forced_files_client + file_path = root / "multi-chunk.bin" + # 2.5 MiB exercises the chunked read loop across multiple iterations. + payload = b"x" * (2 * 1024 * 1024 + 512 * 1024) + + created = client.post( + "/api/files/upload-stream", + data={"path": str(file_path), "overwrite": "true"}, + files={"file": ("multi-chunk.bin", payload, "application/octet-stream")}, + ) + assert created.status_code == 200 + assert file_path.stat().st_size == len(payload) + assert file_path.read_bytes() == payload diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index cb123caaf9f3..98bacbf42a05 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -178,6 +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) ), # 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 ac264e1b2222..fc340bdbe895 100644 --- a/uv.lock +++ b/uv.lock @@ -1445,6 +1445,7 @@ dependencies = [ { name = "pydantic" }, { name = "pyjwt", extra = ["crypto"] }, { name = "python-dotenv" }, + { name = "python-multipart" }, { name = "pywinpty", marker = "sys_platform == 'win32'" }, { name = "pyyaml" }, { name = "requests" }, @@ -1469,6 +1470,7 @@ all = [ { name = "google-auth-httplib2" }, { name = "google-auth-oauthlib" }, { name = "mcp" }, + { name = "python-multipart" }, { name = "simple-term-menu" }, { name = "starlette" }, { name = "uvicorn", extra = ["standard"] }, @@ -1598,6 +1600,7 @@ termux-all = [ { name = "google-auth-oauthlib" }, { name = "honcho-ai" }, { name = "mcp" }, + { name = "python-multipart" }, { name = "python-telegram-bot", extra = ["webhooks"] }, { name = "simple-term-menu" }, { name = "starlette" }, @@ -1613,6 +1616,7 @@ voice = [ ] web = [ { name = "fastapi" }, + { name = "python-multipart" }, { name = "starlette" }, { name = "uvicorn", extra = ["standard"] }, ] @@ -1708,6 +1712,8 @@ requires-dist = [ { name = "pytest", marker = "extra == 'dev'", specifier = "==9.0.2" }, { 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-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" }, @@ -3311,11 +3317,11 @@ wheels = [ [[package]] name = "python-multipart" -version = "0.0.27" +version = "0.0.20" source = { registry = "https://pypi.org/simple" } -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" } +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" } wheels = [ - { 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" }, + { 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" }, ] [[package]] diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index bd175f4e4eb1..9b5b7afc835f 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -411,12 +411,21 @@ export const api = { fetchJSON( `/api/files/read?path=${encodeURIComponent(path)}`, ), - uploadFile: (path: string, dataUrl: string, overwrite = true) => - fetchJSON("/api/files/upload", { + uploadFile: (path: string, file: File, overwrite = true) => { + // Stream the raw bytes as multipart/form-data. Do NOT set Content-Type — + // the browser adds the multipart boundary automatically. Sending the file + // as base64 JSON (the old path) inflated the body ~33%, buffered the whole + // file in memory, and 502'd on large backup archives behind the proxy + // (NS-501). + const form = new FormData(); + form.append("path", path); + form.append("overwrite", String(overwrite)); + form.append("file", file, file.name); + return fetchJSON("/api/files/upload-stream", { method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ path, data_url: dataUrl, overwrite }), - }), + body: form, + }); + }, createDirectory: (path: string) => fetchJSON("/api/files/mkdir", { method: "POST", diff --git a/web/src/pages/FilesPage.tsx b/web/src/pages/FilesPage.tsx index ebc6b16a9d95..022348097856 100644 --- a/web/src/pages/FilesPage.tsx +++ b/web/src/pages/FilesPage.tsx @@ -58,18 +58,6 @@ function formatBytes(size: number | null): string { return `${(size / (1024 * 1024 * 1024)).toFixed(1)} GB`; } -function readAsDataUrl(file: globalThis.File): Promise { - return new Promise((resolve, reject) => { - const reader = new FileReader(); - reader.addEventListener("load", () => { - if (typeof reader.result === "string") resolve(reader.result); - else reject(new Error("Could not read file")); - }); - reader.addEventListener("error", () => reject(reader.error ?? new Error("Could not read file"))); - reader.readAsDataURL(file); - }); -} - function downloadDataUrl(dataUrl: string, name: string) { const link = document.createElement("a"); link.href = dataUrl; @@ -205,8 +193,7 @@ export default function FilesPage() { setUploading(true); try { for (const file of Array.from(files)) { - const dataUrl = await readAsDataUrl(file); - await api.uploadFile(joinPath(activePath, file.name), dataUrl, true); + await api.uploadFile(joinPath(activePath, file.name), file, true); } showToast(`${files.length} file${files.length === 1 ? "" : "s"} uploaded`, "success"); await load();