Skip to content

fix(video): stream downloads with size cap - #55028

Open
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/video-download-response-cap
Open

fix(video): stream downloads with size cap#55028
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/video-download-response-cap

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #55027

Summary

  • stream remote video_analyze downloads instead of reading response.content
  • enforce _MAX_VIDEO_BASE64_BYTES while chunks arrive
  • write to a temporary .part file and replace the destination only after success
  • add focused coverage for declared oversize, streamed oversize, and normal chunked downloads

Context

This mirrors the recent video-download response-boundary fix pattern from OpenClaw, scoped to Hermes' video_analyze remote download path.

Duplicate Audit

  • gh search prs --repo NousResearch/hermes-agent "video download response cap" --state open --limit 20 returned no matches.
  • gh search issues --repo NousResearch/hermes-agent "video download body cap" --state open --limit 20 returned no matches.
  • Broad live scan of open PRs/issues containing video plus download|body|response|read|bound|cap|stream found the existing xAI video JSON response PR, but no remote video download duplicate.

Validation

  • uv run --extra dev python -m pytest tests\tools\test_video_analyze.py -q --basetemp .pytest-tmp-video-download-response-cap
  • uv run --extra dev python -m ruff check tools\vision_tools.py tests\tools\test_video_analyze.py
  • git diff --check

Autoreview

Not run: .agents\skills\autoreview is not present in this worktree.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/vision Vision analysis and image generation P2 Medium — degraded but workaround exists labels Jun 29, 2026
@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

Security evidence:

  • trust boundary: Remote HTTP/HTTPS video_analyze inputs are downloaded into Hermes cache before being base64-encoded for the multimodal request.
  • source/sink/invariant: The downloader must enforce the video byte cap while reading the untrusted response, avoid materializing an oversized body through response.content, and avoid leaving a completed or reusable partial file when the stream is too large.
  • current-main reproduction: A run-root probe imported the current-main tools/vision_tools.py and showed the old downloader accessed response.content before rejecting a 9-byte body with an 8-byte cap.
  • PR-head or patch-replay validation: The GitHub PR patch replayed cleanly onto current GitHub main; the same probe imported the replayed tools/vision_tools.py, rejected the oversized body during chunk iteration, did not touch response.content, and left no destination or .part file.
  • positive/negative cases: The focused video tests passed, including declared oversized content, streamed oversize cleanup, normal chunked download, and adjacent local-file/video_analyze behavior; ruff and diff whitespace checks passed.
  • residual bypass search: I reviewed the remote video call path, redirect/final URL policy checks, temp-file replacement, and sibling video URL references and did not find another changed-path route that bypasses the streaming cap.
  • reviewer validation: CodeRabbit completed with no findings in the clean-pass flow.

Because the submitted branch has stale/unrelated-history setup noise, I reviewed its meaningful two-file GitHub patch replayed onto current GitHub main; that replay and the focused validation passed, but the replay by itself does not prove the submitted branch will keep merging cleanly if main moves again.

Signed: GPT-5.5-xhigh in Codex

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused boundary fix. The premise is confirmed on current main: tools/vision_tools.py:1603 reads the entire remote response through response.content before applying _MAX_VIDEO_BASE64_BYTES, and video_analyze_tool reaches that downloader for remote URLs at tools/vision_tools.py:1674-1680.

The PR changes that path to streamed chunks with an in-loop size check and only publishes the completed file after replace(). Its added tests cover declared oversize, streamed oversize cleanup, and normal chunked output. The implementation follows existing repository streaming conventions, including httpx.AsyncClient.stream() and .part publication in tools/microsoft_graph_client.py:194-235.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/streaming Streaming responses: gateway delivery, provider wire labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/vision Vision analysis and image generation type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote video downloads are buffered before enforcing the size cap

4 participants