Skip to content

feat(dashboard): multipart attachment upload + authenticated static file serve - #43772

Closed
nparkison wants to merge 1 commit into
NousResearch:mainfrom
nparkison:attachment-file-endpoints
Closed

feat(dashboard): multipart attachment upload + authenticated static file serve#43772
nparkison wants to merge 1 commit into
NousResearch:mainfrom
nparkison:attachment-file-endpoints

Conversation

@nparkison

Copy link
Copy Markdown

What does this PR do?

Adds a streaming multipart upload endpoint and an authenticated static file serve route to the dashboard web server, so remote clients (e.g. mobile apps) can attach images to sessions and download gateway-served files without filesystem access to the gateway host.

  • POST /api/files/attachment-upload — multipart field file, streamed to ~/.hermes/uploads/ in chunks, 100 MB cap (413 over), filename sanitization + collision suffixing (-1, -2, …). Response: {"name","path","url","size"}. The returned path is directly usable with the image.attach WS method.
  • GET /files/{name} — serves only from ~/.hermes/uploads/; auth via the normal Bearer header or a ?token= query parameter (for download managers that can't set headers); constant-time token comparison; path-traversal guarded; .apk served as application/vnd.android.package-archive.

This complements the existing base64 /api/files/upload (whole file in memory as a data URL) with a streaming path suited to large files and memory-constrained clients. Reuses the existing session-token auth mechanisms; no new imports or dependencies.

Context: built for an open-source native Android client for hermes-agent — https://github.com/nparkison/hermes-android — where it powers photo/file attachments and gateway-served APK self-update. The implementation has been in daily use against a v0.16.0 gateway; this PR adapts it to current main (route renamed to avoid colliding with the newer base64 upload endpoint) and adds tests.

Related Issue

No existing issue — happy to open one first if you prefer issue-first flow.

Fixes #

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • hermes_cli/web_server.py — new endpoints + helpers (~166 lines)
  • tests/hermes_cli/test_attachment_upload.py — 25 tests (auth, sanitization, collision suffixing, size cap, traversal, media types)

How to Test

  1. pytest tests/hermes_cli/test_attachment_upload.py -q — 25 tests
  2. curl -F "file=@pic.png" -H "Authorization: Bearer $TOKEN" http://127.0.0.1:9119/api/files/attachment-upload → JSON contract
  3. curl "http://127.0.0.1:9119/files/pic.png?token=$TOKEN" → file bytes; same URL without token → 401; traversal attempts → 404, never file content

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • Full pytest tests/ -q not run (heavy env) — ran the new suite plus tests/test_web_server.py and tests/hermes_cli/test_web_server_files.py: all pass, no regressions
  • I've added tests for my changes
  • I've tested on my platform: Ubuntu 24.04 (WSL2)

Documentation & Housekeeping

  • Endpoint docstrings included; a companion PR adds a gateway protocol reference page — or N/A
  • No config keys changed — N/A
  • No architecture/workflow changes — N/A
  • Cross-platform considered: pathlib + expanduser, no POSIX-only primitives
  • No tool behavior changes — N/A

Screenshots / Logs

tests/hermes_cli/test_attachment_upload.py: 25 passed in 0.49s
tests/test_web_server.py + tests/hermes_cli/test_web_server_files.py: 7 passed (no regression)

…ile serve

Adds POST /api/files/attachment-upload (streaming multipart, 100 MB cap,
saves to ~/.hermes/uploads/) and GET /files/{name} (Bearer header or
?token= query auth, traversal-guarded) to the dashboard web server.

Motivation: lets remote clients (e.g. mobile apps) upload images for
image.attach and fetch served files without filesystem access to the
gateway host. Complements the existing base64 /api/files/upload with a
streaming path suited to large files and memory-constrained clients.
Reuses the existing session-token auth mechanisms.

Includes tests: 25 cases covering auth, sanitization, collision
suffixing, size cap, traversal, and media types.
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Verified — File attachment upload with auth and path traversal protection

Reviewed the multipart upload endpoint and authenticated static file serve.

  • Auth on both endpoints: upload_attachment_file calls _require_token(request) explicitly (belt-and-suspenders for --insecure binds); serve_uploaded_file checks Bearer header AND ?token= query param with hmac.compare_digest timing-safe comparison
  • Path traversal guard: _sanitize_upload_filename strips path separators (/, \), control chars (\x00-\x1f, \x7f), and leading dots/spaces. serve_uploaded_file resolves the path and verifies is_relative_to(_UPLOADS_DIR.resolve()) before serving
  • Oversize rejection: Upload exceeding _UPLOADS_MAX_BYTES (100 MB) returns 413 and cleans up the partially-written file via dest.unlink(missing_ok=True)
  • Collision handling: _resolve_upload_path appends -1, -2, … suffixes for duplicate filenames
  • Test coverage: 14 tests covering happy path, collision, auth (no token, wrong token), oversize rejection, filename sanitization (path traversal, control chars, empty names), GET auth (Bearer header, query param, unauthenticated, wrong token)

The _UPLOADS_DIR.mkdir(mode=0o700) at import time ensures the uploads directory exists with restricted permissions. No issues found.

@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 10, 2026
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 19, 2026
调研结论:原计划建 owner/acp/ 一整套 ACP 传输+管理+桥来接 opencode,是重复造轮子
且安全前提靠不住:
- Hermes 主线已有 skills/autonomous-ai-agents/opencode (terminal 跑 CLI),零新代码
- ACP 定位是 editor↔agent (LSP 类比),不是 agent↔agent 委派
- '子agent审批边界下传' 是全行业开放难题:opencode NousResearch#12133 子 session permission 丢弃,
  claude-code NousResearch#43772 同类问题,靠 ACP 做不可靠
- 可靠的安全边界是 terminal 的 check_all_command_guards (hardline floor + gateway 审批),
  把 opencode 当受管辖子进程,而非依赖 opencode 内部 ACP permission

删除原 设计.md / 实现计划.md / probe-findings.md,以本调研结论取代。
保留探针实测副产物(§5)备查,标注未来若 NousResearch#12133 修复可重启评估。
@alt-glitch alt-glitch added the comp/dashboard Web dashboard / control panel UI (dashboard/, landing) label Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the multipart-upload and authenticated-download work. This automated hermes-sweeper review found that the requested capability is already implemented on current main.

  • c661634537a600a411c0371accf862e7bce5029f added POST /api/files/upload-stream; current hermes_cli/web_server.py:2020 streams multipart uploads in chunks, enforces the 100 MB cap, and atomically promotes the completed file.
  • c6b0eb4de0e5010a752e312c0577a4d04d2a08a5 added authenticated remote download; current hermes_cli/web_server.py:1949 serves /api/files/download with a narrowly scoped ?token= path (hermes_cli/web_server.py:339).
  • Existing coverage in tests/hermes_cli/test_web_server_files.py:267 and :341 covers token authentication, token scoping, multipart streaming, size limits, traversal containment, and temporary-file cleanup.

The current managed-files implementation also avoids duplicating a separate fixed ~/.hermes/uploads store and applies the repository's existing path and sensitive-file protections.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants