diff --git a/tests/tools/test_video_analyze.py b/tests/tools/test_video_analyze.py index f429456204daa..020cfc358c5d8 100644 --- a/tests/tools/test_video_analyze.py +++ b/tests/tools/test_video_analyze.py @@ -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 diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 648dbc00e82c0..1c2b308e86e5e 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -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