mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(dashboard): stream file uploads via multipart instead of base64 JSON (NS-501) (#47663)
* fix(dashboard): stream file uploads via multipart instead of base64 JSON The dashboard file manager uploaded files (including backup/restore zip archives) by reading them client-side with FileReader.readAsDataURL and POSTing a base64 data URL inside a JSON body to /api/files/upload. For a large backup this (a) inflates the payload ~33%, (b) buffers the whole file plus its decoded copy in memory, and (c) reliably trips an upstream proxy body-size/timeout limit, surfacing as a 502 with the upload appearing to hang indefinitely (NS-501). Dashboard-only hosted users have no shell fallback to place the archive, so backup restore was unusable. Add a streaming multipart endpoint POST /api/files/upload-stream (UploadFile + Form) that reads the request body in 1 MiB chunks straight to a sibling temp file, enforces the existing 100 MB size cap as it streams (413 on overflow, before buffering the whole file), and atomically renames into place so a partial/aborted/over-limit upload never clobbers an existing file. The frontend api.uploadFile now sends multipart/form-data (raw bytes, no base64, browser-set boundary) and FilesPage passes the File object directly; the dead readAsDataUrl helper is removed. The legacy base64 JSON endpoint stays for backward compat. FastAPI's UploadFile/Form require python-multipart, which is NOT pulled in by fastapi itself, so it is added to the base deps, the [web] extra, and the tool.dashboard lazy-install set (kept in sync). Validated: 5 new endpoint tests (roundtrip, multi-chunk >1 MiB, over-limit 413 without clobbering + no temp-file leak, overwrite=false conflict, forced-root traversal containment); existing base64 tests still pass; web typecheck + vite build clean; and a real uvicorn server E2E (5 MB multipart upload -> HTTP 200 in 0.21s, exact byte match) plus a 30 MB TestClient roundtrip confirm constant-memory streaming end to end. Reported via beta (NS-501). * build(deps): regenerate uv.lock for python-multipart (NS-501) CI ran uv lock --check / uv sync --locked which failed because the python-multipart dependency add was not reflected in uv.lock. Regenerate the lockfile (resolves to 0.0.20, matching the [web] extra pin) after merging current main.
This commit is contained in:
Ben Barclay
GitHub
parent
9c3c5da356
commit
c661634537
7 changed files with 212 additions and 25 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
12
uv.lock
generated
12
uv.lock
generated
|
|
@ -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]]
|
||||
|
|
|
|||
|
|
@ -411,12 +411,21 @@ export const api = {
|
|||
fetchJSON<ManagedFileReadResponse>(
|
||||
`/api/files/read?path=${encodeURIComponent(path)}`,
|
||||
),
|
||||
uploadFile: (path: string, dataUrl: string, overwrite = true) =>
|
||||
fetchJSON<ManagedFileWriteResponse>("/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<ManagedFileWriteResponse>("/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<ManagedFileWriteResponse>("/api/files/mkdir", {
|
||||
method: "POST",
|
||||
|
|
|
|||
|
|
@ -58,18 +58,6 @@ function formatBytes(size: number | null): string {
|
|||
return `${(size / (1024 * 1024 * 1024)).toFixed(1)} GB`;
|
||||
}
|
||||
|
||||
function readAsDataUrl(file: globalThis.File): Promise<string> {
|
||||
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();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue