fix(video): route terminal-backend reads through the shared media resolver
Follow-up on the salvaged commit: replace the hand-rolled file_ops python3
exec-read with tools.image_source.resolve_image_source(permitted=('video',)),
so video_analyze gets the same pipeline as vision_analyze — media-cache host
reads, bounded head -c sandbox exec (no python3 dependency in the sandbox
image, no unbounded base64 stream), lazy env bring-up (#62825), the
credential-read guard, and the 50MB ingest cap.
This commit is contained in:
parent
f2e936dad5
commit
9eb3ac50fe
|
|
@ -208,7 +208,13 @@ class TestVideoAnalyzeTool:
|
|||
assert content[1]["video_url"]["url"].startswith("data:video/mp4;base64,")
|
||||
|
||||
def test_non_local_backend_reads_video_from_terminal_backend(self, tmp_path, monkeypatch):
|
||||
"""Non-local terminal backends must not read local host video paths."""
|
||||
"""Non-local terminal backends must not read local host video paths.
|
||||
|
||||
The read routes through the shared media resolver
|
||||
(tools.image_source, ``permitted=("video",)``) which exec-reads the
|
||||
bytes inside the sandbox — so the analyzed video is the container's
|
||||
file, never the host's.
|
||||
"""
|
||||
host_video = tmp_path / "clip.mp4"
|
||||
host_video.write_bytes(b"HOST-VIDEO")
|
||||
remote_bytes = b"REMOTE-SANDBOX-VIDEO"
|
||||
|
|
@ -216,11 +222,19 @@ class TestVideoAnalyzeTool:
|
|||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home"))
|
||||
|
||||
class FakeFileOps:
|
||||
def _exec(self, command, timeout=None):
|
||||
assert str(host_video) in command
|
||||
assert timeout == 300
|
||||
return SimpleNamespace(stdout=remote_b64, exit_code=0)
|
||||
import tools.image_source as isrc
|
||||
import tools.terminal_tool as tt
|
||||
|
||||
env_lookups = []
|
||||
|
||||
def fake_get_active(task_id):
|
||||
env_lookups.append(task_id)
|
||||
return SimpleNamespace(
|
||||
execute=lambda cmd, **kw: {"returncode": 0, "output": remote_b64}
|
||||
)
|
||||
|
||||
monkeypatch.setattr(tt, "ensure_task_env", lambda *a, **k: None)
|
||||
monkeypatch.setattr(isrc, "_get_active_env", fake_get_active)
|
||||
|
||||
captured_kwargs = {}
|
||||
|
||||
|
|
@ -232,7 +246,6 @@ class TestVideoAnalyzeTool:
|
|||
return mock_response
|
||||
|
||||
with (
|
||||
patch("tools.file_tools._get_file_ops", return_value=FakeFileOps()) as get_ops,
|
||||
patch("tools.vision_tools.async_call_llm", side_effect=capture_llm),
|
||||
patch("tools.vision_tools.extract_content_or_reasoning", return_value="sandbox video"),
|
||||
):
|
||||
|
|
@ -242,7 +255,7 @@ class TestVideoAnalyzeTool:
|
|||
|
||||
data = json.loads(result)
|
||||
assert data["success"] is True
|
||||
get_ops.assert_called_once_with("task-123")
|
||||
assert env_lookups == ["task-123"]
|
||||
video_url = captured_kwargs["messages"][0]["content"][1]["video_url"]["url"]
|
||||
uploaded_bytes = base64.b64decode(video_url.split(",", 1)[1])
|
||||
assert uploaded_bytes == remote_bytes
|
||||
|
|
|
|||
|
|
@ -35,7 +35,6 @@ import json
|
|||
from concurrent.futures import ThreadPoolExecutor
|
||||
import logging
|
||||
import os
|
||||
import shlex
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Any, Awaitable, Dict, Optional
|
||||
|
|
@ -1764,9 +1763,18 @@ def _is_path_like_video_source(value: str) -> bool:
|
|||
return not lowered.startswith(("http://", "https://", "data:"))
|
||||
|
||||
|
||||
def _materialize_video_from_terminal_backend(video_source: str, task_id: Optional[str]) -> Path:
|
||||
"""Read a path from the active terminal backend into a local temp video file."""
|
||||
from tools.file_tools import _get_file_ops
|
||||
async def _materialize_video_from_terminal_backend(video_source: str, task_id: Optional[str]) -> Path:
|
||||
"""Read a path via the shared media resolver into a local temp video file.
|
||||
|
||||
Routes through :func:`tools.image_source.resolve_image_source` with
|
||||
``permitted=("video",)`` so terminal-backend video reads get the exact
|
||||
pipeline vision_analyze uses: media-cache host reads (gateway-downloaded
|
||||
videos live on the host, not in the sandbox), bounded in-sandbox exec-read
|
||||
(``head -c`` cap — no unbounded base64 stream, no python3 dependency in
|
||||
the sandbox image), lazy env bring-up (#62825), the credential-read
|
||||
guard, and the 50MB ingest cap.
|
||||
"""
|
||||
from tools.image_source import ImageResolutionError, ResolveContext, resolve_image_source
|
||||
|
||||
source = video_source
|
||||
if source.startswith("file://"):
|
||||
|
|
@ -1778,31 +1786,17 @@ def _materialize_video_from_terminal_backend(video_source: str, task_id: Optiona
|
|||
f"Supported: {', '.join(sorted(_VIDEO_MIME_TYPES.keys()))}"
|
||||
)
|
||||
|
||||
command = (
|
||||
"python3 -c "
|
||||
+ shlex.quote(
|
||||
"import base64, pathlib, sys; "
|
||||
"p = pathlib.Path(sys.argv[1]).expanduser(); "
|
||||
"sys.stdout.write(base64.b64encode(p.read_bytes()).decode('ascii'))"
|
||||
)
|
||||
+ f" {shlex.quote(source)}"
|
||||
)
|
||||
file_ops = _get_file_ops(task_id or "default")
|
||||
result = file_ops._exec(command, timeout=300)
|
||||
if result.exit_code != 0:
|
||||
details = result.stdout.strip() or f"exit code {result.exit_code}"
|
||||
raise ValueError(f"Could not read video from terminal backend: {details}")
|
||||
|
||||
encoded = "".join(result.stdout.split())
|
||||
try:
|
||||
data = base64.b64decode(encoded, validate=True)
|
||||
except Exception as exc:
|
||||
raise ValueError(f"Terminal backend returned invalid video data: {exc}") from exc
|
||||
resolved = await resolve_image_source(
|
||||
video_source, ResolveContext(task_id=task_id), permitted=("video",)
|
||||
)
|
||||
except ImageResolutionError as exc:
|
||||
raise ValueError(f"Could not read video from terminal backend: {exc}") from exc
|
||||
|
||||
temp_dir = get_hermes_dir("cache/video", "temp_video_files")
|
||||
temp_dir.mkdir(parents=True, exist_ok=True)
|
||||
temp_path = temp_dir / f"terminal_video_{uuid.uuid4()}{suffix}"
|
||||
temp_path.write_bytes(data)
|
||||
temp_path.write_bytes(resolved.data)
|
||||
return temp_path
|
||||
|
||||
|
||||
|
|
@ -1907,7 +1901,7 @@ async def video_analyze_tool(
|
|||
|
||||
if not _terminal_backend_is_local() and _is_path_like_video_source(video_url):
|
||||
logger.info("Reading video source via terminal backend: %s", video_url)
|
||||
temp_video_path = _materialize_video_from_terminal_backend(video_url, task_id)
|
||||
temp_video_path = await _materialize_video_from_terminal_backend(video_url, task_id)
|
||||
should_cleanup = True
|
||||
elif local_path.is_file():
|
||||
from agent.file_safety import raise_if_read_blocked
|
||||
|
|
|
|||
Loading…
Reference in New Issue