From 316e77517e82d8f4af8936e036661ca0836990c2 Mon Sep 17 00:00:00 2001 From: emozilla Date: Fri, 3 Jul 2026 14:31:04 -0400 Subject: [PATCH] fix(vision): unified image-source resolver + terminal-backend confinement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salvage of #35362, evolved to also close the vision sandbox-escape (GHSA-gpxw-6wxv-w3qq). The two were the same root cause — vision read image bytes host-side while every other tool reads through the terminal backend — so one resolver fixes both the delivery gaps and the escape. Delivery (from #35362, re-authored against current main since the branch was 4140 commits stale and vision_tools.py had been rewritten on both sides): - tools/image_source.py: one resolver for data:/http(s)/file/local/container image sources, returning raw bytes through a single magic-byte-sniff + 50MB-ingest chokepoint. Fixes 'no image attached' / 'Invalid image source' for every source type (#7571, #25118, #29643, #22328, #32709, #9077). - tools/credential_files.py: from_agent_visible_cache_path, the container->host cache reverse-map (inverse of the existing forward twin). - tools/vision_tools.py: both vision sites route through the resolver with task_id threaded from the handler; resolved bytes are materialized to a temp file so main's evolved encode/resize/embed-cap pipeline is reused verbatim (kept over the PR's older bytes-core resize to avoid touching browser_tool / conversation_compression callers). Security (fills #35362's deliberately-stubbed _within_allowed_roots seam): - Under a non-local terminal backend the file tools are confined to the sandbox (SECURITY.md 2.2), but vision read host-side — a prompt-injected vision_analyze('/etc/passwd') exfiltrated host secrets, and read_file even redirects the model to vision_analyze for image paths. The resolver now enforces the same boundary: local backend reads any host path (chosen posture); non-local backend host-reads ONLY the media caches under HERMES_HOME (where the gateway/download media lives) and routes every other path to an in-sandbox base64 exec-read — which reads the CONTAINER's file, the same one 'cat' would, never the host's. Paths are resolve()-d so a symlink can't escape a cache; fail-closed when no sandbox env exists. This closes the escape AND delivers container-only images (#32709) with the same mechanism. Tests: unified resolver + confinement model (tests/tools/test_image_source.py, incl. proof a non-cache host path under Docker yields container bytes not the host secret); existing vision tests updated to the resolver boundary; Docker integration test verified green against a real daemon (exec-read of a tmpfs /workspace file, a root-owned mode-600 file, and the host-secret invariant). Fixes GHSA-gpxw-6wxv-w3qq. Co-authored-by: banditburai --- .../integration/test_vision_docker_resolve.py | 157 ++++++++++ tests/tools/test_image_source.py | 191 ++++++++++++ tests/tools/test_vision_native_fast_path.py | 4 +- tests/tools/test_vision_tools.py | 36 +-- tools/credential_files.py | 24 ++ tools/image_source.py | 292 ++++++++++++++++++ tools/vision_tools.py | 144 +++++---- 7 files changed, 750 insertions(+), 98 deletions(-) create mode 100644 tests/integration/test_vision_docker_resolve.py create mode 100644 tests/tools/test_image_source.py create mode 100644 tools/image_source.py diff --git a/tests/integration/test_vision_docker_resolve.py b/tests/integration/test_vision_docker_resolve.py new file mode 100644 index 00000000000..4ff3e46edea --- /dev/null +++ b/tests/integration/test_vision_docker_resolve.py @@ -0,0 +1,157 @@ +"""Docker integration tests for the vision image-source resolver. + +Exercises the in-sandbox exec-read path that unit tests can only mock. Two +scenarios that ONLY the container filesystem can serve: + + * a tmpfs ``/workspace`` file with no host path at all, and + * a root-owned mode-600 file the host user cannot read, + +both delivered by the resolver's ``base64`` exec-read fallback +(``tools/image_source._resolve_container_fallback``). This is the same path +that provides terminal-backend confinement for GHSA-gpxw-6wxv-w3qq: under a +non-local backend a non-cache path is read INSIDE the container, never on the +host. + +Gating follows the repo convention: the ``integration`` marker excludes these +from the default suite (``addopts = -m 'not integration'`` in pyproject.toml); +they run under ``pytest -m integration`` when a Docker daemon is available and +skip cleanly when it is not. Container spin-up exceeds the 30s suite default, +so the timeout is bumped to 180s. + +Run: pytest -m integration tests/integration/test_vision_docker_resolve.py +""" +import base64 +import shutil +import subprocess + +import pytest + + +def _docker_available() -> bool: + """True iff a docker CLI is on PATH and the daemon answers.""" + if shutil.which("docker") is None: + return False + try: + return subprocess.run( + ["docker", "info"], capture_output=True, timeout=5 + ).returncode == 0 + except (subprocess.TimeoutExpired, OSError): + return False + + +pytestmark = [ + pytest.mark.integration, + pytest.mark.timeout(180), + pytest.mark.skipif(not _docker_available(), reason="Docker daemon not available"), +] + +# A real 1x1 PNG. +_TINY_PNG = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==" +) + + +@pytest.fixture +def docker_backend(request, monkeypatch): + """A live DockerEnvironment registered so ``get_active_env`` finds it. + + Unique task_id per test: DockerEnvironment derives the container name from + the task_id, so a shared id would make one test's teardown remove the + other's container. + """ + from tools import terminal_tool + from tools.environments import docker as docker_env + + # The resolver keys the exec-read off TERMINAL_ENV=docker. + monkeypatch.setenv("TERMINAL_ENV", "docker") + + task_id = f"vision-docker-resolve-{request.node.name}" + env = docker_env.DockerEnvironment( + image="python:3.11-slim", + cwd="/workspace", + timeout=120, + task_id=task_id, + volumes=[], + network=False, + ) + # Register under the raw task_id; get_active_env() falls back to the raw + # key after trying the _resolve_container_task_id() mapping. + with terminal_tool._env_lock: + terminal_tool._active_environments[task_id] = env + try: + env._task_id = task_id + yield env + finally: + with terminal_tool._env_lock: + terminal_tool._active_environments.pop(task_id, None) + try: + env.cleanup() + except Exception: + pass + + +def _write_png_in_container(env, path, *, mode=None): + b64 = base64.b64encode(_TINY_PNG).decode() + cmd = f"printf %s {b64} | base64 -d > {path}" + if mode: + cmd += f" && chmod {mode} {path}" + res = env.execute(cmd) + assert res.get("returncode", 1) == 0, res + + +@pytest.mark.asyncio +async def test_resolves_tmpfs_workspace_file(docker_backend): + """A container-only path (no host file) is delivered via exec-read.""" + from tools.image_source import ResolveContext, resolve_image_source + + _write_png_in_container(docker_backend, "/workspace/shot.png") + res = await resolve_image_source( + "/workspace/shot.png", ResolveContext(task_id=docker_backend._task_id)) + assert res.origin == "container" + assert res.mime == "image/png" + assert res.data == _TINY_PNG + + +@pytest.mark.asyncio +async def test_resolves_root_owned_mode600_file(docker_backend): + """Root-owned mode-600 (host user can't read it) is served in-container.""" + from tools.image_source import ResolveContext, resolve_image_source + + _write_png_in_container(docker_backend, "/workspace/secret.png", mode="600") + res = await resolve_image_source( + "/workspace/secret.png", ResolveContext(task_id=docker_backend._task_id)) + assert res.origin == "container" + assert res.mime == "image/png" + assert res.data == _TINY_PNG + + +@pytest.mark.asyncio +async def test_host_secret_path_reads_container_not_host(docker_backend, tmp_path): + """The GHSA-gpxw invariant, end-to-end against real Docker. + + A path that exists on the HOST with secret bytes but does NOT exist in the + container must resolve to the container read (which fails to find it) — + never to the host bytes. Proves vision cannot exfiltrate a host file under + a sandbox backend even when the exact path is real on the host. + """ + from tools.image_source import ( + ImageResolutionError, + ResolveContext, + resolve_image_source, + ) + + # A real host file outside any media cache, holding a secret. + host_secret = tmp_path / "id_rsa" + host_secret.write_bytes(b"HOST-PRIVATE-KEY-DO-NOT-LEAK") + + # That path does not exist inside the fresh container, so the exec-read + # finds nothing and the resolver refuses. The exact failure shape depends + # on the container's base64 build (some exit non-zero -> SourceNotFound, + # some print an error to stderr and exit 0 with empty stdout -> NotAnImage); + # both are ImageResolutionError. What matters for GHSA-gpxw: the resolver + # never returns the HOST bytes. + with pytest.raises(ImageResolutionError) as excinfo: + await resolve_image_source( + str(host_secret), ResolveContext(task_id=docker_backend._task_id)) + # Belt and suspenders: the host secret must not appear anywhere in the error. + assert b"HOST-PRIVATE-KEY".decode() not in str(excinfo.value) diff --git a/tests/tools/test_image_source.py b/tests/tools/test_image_source.py new file mode 100644 index 00000000000..56d07d9fe42 --- /dev/null +++ b/tests/tools/test_image_source.py @@ -0,0 +1,191 @@ +"""Tests for tools/image_source.py — the unified vision image-source resolver. + +Covers the delivery contract (data:/http/file/local/container source handling, +size cap, magic-byte sniff) AND the terminal-backend confinement security model +(GHSA-gpxw-6wxv-w3qq): under a non-local backend, host reads are confined to the +media caches and every other path is read inside the sandbox via exec-read. +""" + +import base64 +import importlib +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + + +PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 64 +JPEG = b"\xff\xd8\xff" + b"\x00" * 64 + + +def _reload(monkeypatch, hermes_home: Path): + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + import hermes_constants + importlib.reload(hermes_constants) + import tools.image_source as isrc + importlib.reload(isrc) + return isrc + + +class TestDataUrl: + @pytest.mark.asyncio + async def test_valid_data_url_resolves_to_bytes(self, tmp_path, monkeypatch): + isrc = _reload(monkeypatch, tmp_path / "hermes") + b64 = base64.b64encode(PNG).decode() + res = await isrc.resolve_image_source( + f"data:image/png;base64,{b64}", isrc.ResolveContext()) + assert res.data == PNG + assert res.mime == "image/png" + assert res.origin == "data" + + @pytest.mark.asyncio + async def test_non_image_data_url_rejected(self, tmp_path, monkeypatch): + isrc = _reload(monkeypatch, tmp_path / "hermes") + b64 = base64.b64encode(b"not an image").decode() + with pytest.raises(isrc.NotAnImage): + await isrc.resolve_image_source( + f"data:text/plain;base64,{b64}", isrc.ResolveContext()) + + +class TestLocalBackend: + @pytest.mark.asyncio + async def test_local_backend_reads_any_host_path(self, tmp_path, monkeypatch): + isrc = _reload(monkeypatch, tmp_path / "hermes") + monkeypatch.setenv("TERMINAL_ENV", "local") + img = tmp_path / "outside" / "pic.png" + img.parent.mkdir(parents=True) + img.write_bytes(PNG) + res = await isrc.resolve_image_source(str(img), isrc.ResolveContext()) + assert res.data == PNG + assert res.origin == "file" + + @pytest.mark.asyncio + async def test_file_uri_scheme_stripped(self, tmp_path, monkeypatch): + isrc = _reload(monkeypatch, tmp_path / "hermes") + monkeypatch.setenv("TERMINAL_ENV", "local") + img = tmp_path / "pic.jpg" + img.write_bytes(JPEG) + res = await isrc.resolve_image_source(f"file://{img}", isrc.ResolveContext()) + assert res.mime == "image/jpeg" + + +class TestNonLocalBackendConfinement: + """The security model: under a sandbox backend, host reads are confined to + the media caches; every other path is read inside the sandbox.""" + + @pytest.mark.asyncio + async def test_media_cache_path_host_read(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + cached = home / "cache" / "images" / "inbound.png" + cached.parent.mkdir(parents=True) + cached.write_bytes(PNG) + # No sandbox env needed — a cache path is host-read directly. + res = await isrc.resolve_image_source(str(cached), isrc.ResolveContext()) + assert res.data == PNG + assert res.origin == "file" + + @pytest.mark.asyncio + async def test_host_secret_outside_cache_routes_to_sandbox_not_host(self, tmp_path, monkeypatch): + """A non-cache host path (e.g. /etc/passwd) must NOT be host-read — it + routes to the in-sandbox exec-read, which reads the CONTAINER's file.""" + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + + # A real host file outside the caches, holding a "secret". + secret = tmp_path / "id_rsa" + secret.write_bytes(b"HOST-PRIVATE-KEY-DO-NOT-LEAK") + + # Fake sandbox env: its exec-read returns a *different* (container) image, + # proving we read the container filesystem, not the host secret. + container_png_b64 = base64.b64encode(PNG).decode() + calls = {} + + def fake_execute(cmd, **kw): + calls["cmd"] = cmd + return {"returncode": 0, "output": container_png_b64} + + with patch("tools.image_source._get_active_env", + return_value=SimpleNamespace(execute=fake_execute)): + res = await isrc.resolve_image_source(str(secret), isrc.ResolveContext(task_id="t1")) + + # Read came from the sandbox exec-read, returning the container image — + # the host secret bytes never appear. + assert res.origin == "container" + assert res.data == PNG + assert b"HOST-PRIVATE-KEY" not in res.data + assert "base64 --" in calls["cmd"] # option-injection-safe form + + @pytest.mark.asyncio + async def test_non_cache_path_fails_closed_without_sandbox(self, tmp_path, monkeypatch): + """No active sandbox env -> refuse rather than fall back to a host read.""" + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + secret = tmp_path / "id_rsa" + secret.write_bytes(b"HOST-PRIVATE-KEY") + + with patch("tools.image_source._get_active_env", return_value=None): + with pytest.raises(isrc.SourceNotFound): + await isrc.resolve_image_source(str(secret), isrc.ResolveContext(task_id="t1")) + + @pytest.mark.asyncio + async def test_symlink_in_cache_pointing_outside_is_not_host_read(self, tmp_path, monkeypatch): + """A symlink planted inside a cache dir that points at a host secret must + not be host-read (resolve() escapes the cache) — it routes to sandbox.""" + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + secret = tmp_path / "outside" / "id_rsa" + secret.parent.mkdir(parents=True) + secret.write_bytes(b"HOST-PRIVATE-KEY") + cache_dir = home / "cache" / "images" + cache_dir.mkdir(parents=True) + link = cache_dir / "sneaky.png" + try: + link.symlink_to(secret) + except (OSError, NotImplementedError): + pytest.skip("symlinks unsupported") + + # Fails closed (no sandbox) rather than host-reading the symlink target. + with patch("tools.image_source._get_active_env", return_value=None): + with pytest.raises(isrc.SourceNotFound): + await isrc.resolve_image_source(str(link), isrc.ResolveContext(task_id="t1")) + + +class TestExecReadSafety: + @pytest.mark.asyncio + async def test_exec_read_uses_option_terminator(self, tmp_path, monkeypatch): + """Leading-dash paths must not be parsed as base64 options.""" + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + captured = {} + + def fake_execute(cmd, **kw): + captured["cmd"] = cmd + return {"returncode": 0, "output": base64.b64encode(PNG).decode()} + + with patch("tools.image_source._get_active_env", + return_value=SimpleNamespace(execute=fake_execute)): + await isrc.resolve_image_source( + "/workspace/-i-etc-shadow.png", isrc.ResolveContext(task_id="t1")) + assert "base64 -- " in captured["cmd"] + + @pytest.mark.asyncio + async def test_exec_read_nonzero_returncode_raises(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + isrc = _reload(monkeypatch, home) + monkeypatch.setenv("TERMINAL_ENV", "docker") + + def fake_execute(cmd, **kw): + return {"returncode": 1, "output": ""} + + with patch("tools.image_source._get_active_env", + return_value=SimpleNamespace(execute=fake_execute)): + with pytest.raises(isrc.SourceNotFound): + await isrc.resolve_image_source( + "/workspace/nope.png", isrc.ResolveContext(task_id="t1")) diff --git a/tests/tools/test_vision_native_fast_path.py b/tests/tools/test_vision_native_fast_path.py index bb396c05dc3..4423e9c9cb3 100644 --- a/tests/tools/test_vision_native_fast_path.py +++ b/tests/tools/test_vision_native_fast_path.py @@ -119,7 +119,9 @@ class TestVisionAnalyzeNative: assert isinstance(result, str) parsed = json.loads(result) assert parsed.get("success") is False - assert "Invalid image source" in parsed.get("error", "") + # Unified resolver: unrecognized/non-existent local source. + err = parsed.get("error", "").lower() + assert "not reachable" in err or "unrecognized image source" in err or "no active sandbox" in err def test_empty_image_url_returns_error(self): result = asyncio.get_event_loop().run_until_complete( diff --git a/tests/tools/test_vision_tools.py b/tests/tools/test_vision_tools.py index 73cc672a6ed..7dc165b5a86 100644 --- a/tests/tools/test_vision_tools.py +++ b/tests/tools/test_vision_tools.py @@ -1,5 +1,6 @@ """Tests for tools/vision_tools.py — URL validation, type hints, error logging.""" +import base64 import json import logging import os @@ -381,26 +382,20 @@ class TestErrorLoggingExcInfo: @pytest.mark.asyncio async def test_cleanup_error_logs_exc_info(self, tmp_path, caplog): """Temp file cleanup failure should log warning with exc_info.""" - # Create a real temp file that will be "downloaded" - temp_dir = tmp_path / "temp_vision_images" - temp_dir.mkdir() - - async def fake_download(url, dest, max_retries=3): - """Simulate download by writing file to the expected destination.""" - dest.parent.mkdir(parents=True, exist_ok=True) - dest.write_bytes(b"\xff\xd8\xff" + b"\x00" * 16) - return dest + # A data: URL resolves to bytes without any network, materializes to a + # vision temp file, then the analysis runs — exercising the temp-cleanup + # path in the finally block. A tiny valid JPEG (magic bytes) passes the + # resolver's magic-byte sniff. + jpeg_b64 = base64.b64encode(b"\xff\xd8\xff" + b"\x00" * 32).decode("ascii") + data_url = f"data:image/jpeg;base64,{jpeg_b64}" with ( - patch("tools.vision_tools._validate_image_url_async", new_callable=AsyncMock, return_value=True), - patch("tools.vision_tools._download_image", side_effect=fake_download), patch( "tools.vision_tools._image_to_base64_data_url", return_value="data:image/jpeg;base64,abc", ), caplog.at_level(logging.WARNING, logger="tools.vision_tools"), ): - # Mock the async_call_llm function to return a mock response mock_response = MagicMock() mock_choice = MagicMock() mock_choice.message.content = "A test image description" @@ -409,16 +404,12 @@ class TestErrorLoggingExcInfo: with ( patch("tools.vision_tools.async_call_llm", new_callable=AsyncMock, return_value=mock_response), ): - # Make unlink fail to trigger cleanup warning - original_unlink = Path.unlink - + # Make unlink fail to trigger the cleanup warning. def failing_unlink(self, *args, **kwargs): raise PermissionError("no permission") with patch.object(Path, "unlink", failing_unlink): - result = await vision_analyze_tool( - "https://example.com/tempimg.jpg", "describe", "test/model" - ) + result = await vision_analyze_tool(data_url, "describe", "test/model") warning_records = [ r @@ -500,7 +491,8 @@ class TestVisionSafetyGuards: result = json.loads(await vision_analyze_tool(str(secret), "extract text")) assert result["success"] is False - assert "Only real image files are supported" in result["error"] + # The unified resolver's magic-byte sniff rejects non-images. + assert "not a recognized image" in result["error"] mock_llm.assert_not_awaited() @pytest.mark.asyncio @@ -513,8 +505,8 @@ class TestVisionSafetyGuards: } with ( - patch("tools.vision_tools.check_website_access", return_value=blocked), - patch("tools.vision_tools._validate_image_url_async", new_callable=AsyncMock, return_value=True), + patch("tools.website_policy.check_website_access", return_value=blocked), + patch("tools.url_safety.is_safe_url", return_value=True), patch("tools.vision_tools._download_image", new_callable=AsyncMock) as mock_download, ): result = json.loads(await vision_analyze_tool("https://blocked.test/cat.png", "describe")) @@ -1258,7 +1250,7 @@ class TestVisionCpuBurstCap: enc_inflight -= 1 return "data:image/jpeg;base64,AAAA" - async def fake_native(image_url, question): + async def fake_native(image_url, question, task_id=None): nonlocal calls_inflight, calls_peak calls_inflight += 1 calls_peak = max(calls_peak, calls_inflight) diff --git a/tools/credential_files.py b/tools/credential_files.py index 885e0e37d9f..2a523532b4f 100644 --- a/tools/credential_files.py +++ b/tools/credential_files.py @@ -402,6 +402,30 @@ def map_cache_path_to_container( return None +def from_agent_visible_cache_path( + container_path: str, + container_base: str = "/root/.hermes", +) -> str: + """Translate a sandbox/container cache path back to its host path. + + Inverse of :func:`to_agent_visible_cache_path`. Returns the input unchanged + when the active backend is not Docker, or when the path is not under any + auto-mounted cache directory — the caller then treats a still-container + path as "no host file" and falls back to an in-container read. + """ + if os.environ.get("TERMINAL_ENV", "local") != "docker": + return container_path + + path = Path(container_path) + for mount in get_cache_directory_mounts(container_base=container_base): + try: + rel = path.relative_to(mount["container_path"]) + except ValueError: + continue + return str(Path(mount["host_path"]) / rel) + return container_path + + def to_agent_visible_cache_path( host_path: str, container_base: str = "/root/.hermes", diff --git a/tools/image_source.py b/tools/image_source.py new file mode 100644 index 00000000000..770ffc4419e --- /dev/null +++ b/tools/image_source.py @@ -0,0 +1,292 @@ +"""Single resolver for every vision_analyze image source -> bytes + mime. + +All source handling (data:/http(s)/file/local/container) funnels through +:func:`resolve_image_source` so size and magic-byte checks are enforced exactly +once. Returns raw bytes (not a path): the downstream step is base64 -> data URL +(RFC 2397) and provider base64 content blocks. + +Security (terminal-backend confinement, GHSA-gpxw-6wxv-w3qq): under a non-local +terminal backend the file tools are confined to the sandbox (SECURITY.md 2.2), +but vision read images host-side. This resolver enforces the same boundary: + + * local backend -> read any host path (chosen posture, unchanged) + * non-local backend: + path in a media cache -> host-read (the gateway/download caches live on + the host and are bind-mounted into the sandbox) + path anywhere else -> read the bytes *inside the sandbox* via exec-read + (the agent can already ``cat`` any container file; + this stays within the sandbox boundary and never + reaches the host's ``/etc/passwd`` / ``~/.ssh``). + +So a prompt-injected ``vision_analyze('/etc/passwd')`` under Docker reads the +*container's* file (what every other tool sees), not the host's — no escape — +while container-only images (tmpfs ``/workspace``, root-owned) are still +deliverable. This is the unified delivery + confinement model: the same +mechanism that fixes "vision can't see container files" also closes the escape. +""" +from __future__ import annotations + +import base64 +import os +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Optional + +# Raw-bytes INGEST budget — what the resolver will load before handing off. +# This is deliberately the 50MB download cap (tools/vision_tools._VISION_MAX_DOWNLOAD_BYTES), +# NOT the 20MB provider payload cap. The 20MB cap (_MAX_BASE64_BYTES) is a +# *post-resize* limit enforced at the call sites: an oversized raw image must +# still reach the resizer so it can be downscaled under the payload cap. Capping +# raw bytes at 20MB here would reject every 20-50MB photo before resize can run. +_MAX_INGEST_BYTES = 50 * 1024 * 1024 + + +class ImageResolutionError(Exception): + def __init__(self, message: str, *, src: str = "", origin: str = ""): + super().__init__(message) + self.src, self.origin = src, origin + + +class UnsupportedScheme(ImageResolutionError): + pass + + +class SourceUnsafe(ImageResolutionError): # SSRF / path-allowlist + pass + + +class SourceTooLarge(ImageResolutionError): + pass + + +class SourceNotFound(ImageResolutionError): + pass + + +class NotAnImage(ImageResolutionError): + pass + + +@dataclass +class ResolveContext: + task_id: Optional[str] = None + cfg: Optional[dict[str, Any]] = None + extra_roots: tuple = field(default_factory=tuple) + + +@dataclass +class ResolvedImage: + data: bytes + mime: str + origin: str # one of: data | http | file | local | container + + +async def resolve_image_source(src: str, ctx: ResolveContext) -> ResolvedImage: + if not isinstance(src, str) or not src.strip(): + raise SourceNotFound("image_url is required", src=str(src)) + s = src.strip() + if s.startswith("data:"): + data, mime = _resolve_data_url(s) + return _finalize(data, mime, "data", s) + if s.startswith(("http://", "https://")): + reason = _http_block_reason(s) + if reason: + raise SourceUnsafe(reason, src=s) + return _finalize(await _download_to_bytes(s), "", "http", s) + + candidate = s[len("file://"):] if s.startswith("file://") else s + if s.startswith("file://") or _looks_like_path(candidate): + p = Path(os.path.expanduser(candidate)) + # Confinement decision (see module docstring). Under a non-local backend + # a path is host-readable ONLY if it lands in a media cache (after + # translating a container-visible cache path back to its host mount); + # every other path is read inside the sandbox via exec-read, so a host + # path outside the caches never yields the host's bytes. + host_target = _permitted_host_read_target(p, ctx) + if host_target is not None and host_target.is_file(): + return _finalize(host_target.read_bytes(), "", "file", s) + # Not a permitted host read (or the host file is absent) -> read the + # bytes inside the sandbox. On the local backend this reaches the same + # host file; under a sandbox it reads the container's filesystem. + return await _resolve_container_fallback(p, ctx, s) + + raise UnsupportedScheme( + "Unrecognized image source. Use an http(s) URL, a local file path, " + "a file:// URI, or a data: URL.", + src=s, + ) + + +def _resolve_data_url(s: str) -> tuple[bytes, str]: + header, _, payload = s.partition(",") + if ";base64" not in header: + raise NotAnImage("data: URL must be base64-encoded", src=s[:64]) + declared = header[len("data:"):].split(";", 1)[0].strip() or "application/octet-stream" + # Cheap pre-decode size gate on the encoded length (~4/3 expansion). + if (len(payload) * 3) // 4 > _MAX_INGEST_BYTES: + raise SourceTooLarge("data: URL exceeds size limit", src=s[:64]) + try: + data = base64.b64decode(payload, validate=True) + except Exception as exc: + raise NotAnImage(f"invalid base64 in data: URL: {exc}", src=s[:64]) + return data, declared # real mime verified in _finalize via magic bytes + + +def _http_block_reason(url: str) -> Optional[str]: + """Return a human-readable block reason, or None when the URL is allowed. + + Preserves the specific website-policy message (rather than collapsing it to + a generic string) so the agent sees *why* a fetch was refused. + """ + from tools.url_safety import is_safe_url + from tools.website_policy import check_website_access + + if not is_safe_url(url): + return "blocked: unsafe or private URL" + blocked = check_website_access(url) + if blocked: + return blocked.get("message") or "blocked by website policy" + return None + + +async def _download_to_bytes(url: str) -> bytes: + import tempfile + + from tools.vision_tools import _download_image + + with tempfile.NamedTemporaryFile(suffix=".img", delete=False) as tf: + tmp = Path(tf.name) + try: + await _download_image(url, tmp) # enforces the 50MB stream cap + redirect SSRF guard + return tmp.read_bytes() + finally: + tmp.unlink(missing_ok=True) + + +def _looks_like_path(s: str) -> bool: + return s.startswith(("/", "~", "./", "../")) or (len(s) > 1 and s[1] == ":") + + +def _is_local_terminal_backend() -> bool: + """True when the terminal backend runs directly on the host. + + Mirrors ``tools.browser_tool._is_local_backend`` and terminal_tool's own + dispatch, which key off ``TERMINAL_ENV``. + """ + return os.getenv("TERMINAL_ENV", "local").strip().lower() in ("local", "") + + +def _media_cache_roots() -> list: + """Agent-managed media cache directories under HERMES_HOME (host side). + + The only host paths vision may read under a non-local backend: gateway- + downloaded inbound media and the tools' own URL-download temp dirs. Covers + the consolidated ``cache/`` layout and the legacy flat directories. + """ + from hermes_constants import get_hermes_home + + home = get_hermes_home() + return [ + home / "cache", # cache/images, cache/vision, cache/video(s), cache/audio + home / "image_cache", + home / "audio_cache", + home / "video_cache", + home / "temp_vision_images", + home / "temp_video_files", + ] + + +def _permitted_host_read_target(p: Path, ctx: ResolveContext) -> Optional[Path]: + """Return the host path to read, or ``None`` if a host read is not permitted. + + - Local backend: any path is permitted (chosen posture). Returns ``p``. + - Non-local backend: permitted only if the path resolves inside a media + cache root. A container-visible cache path (e.g. ``/root/.hermes/cache/ + images/x.png``) is first translated back to its host mount; anything that + is not under a cache returns ``None`` so the caller routes it to the + in-sandbox exec-read instead of reading the host filesystem. + """ + if _is_local_terminal_backend(): + try: + return p.resolve() + except Exception: # noqa: BLE001 — unresolved path: let is_file() fail downstream + return p + + from tools.credential_files import from_agent_visible_cache_path + + host_candidate = Path(from_agent_visible_cache_path(str(p))) + try: + real = host_candidate.resolve() + except Exception: # noqa: BLE001 — cannot resolve -> not a safe host read + return None + for root in _media_cache_roots(): + try: + real.relative_to(root.resolve()) + return real + except ValueError: + continue + return None + + +def _get_active_env(task_id: Optional[str]): + if not task_id: + return None + try: + from tools.terminal_tool import get_active_env + + return get_active_env(task_id) + except Exception: + return None + + +async def _resolve_container_fallback(p: Path, ctx: ResolveContext, src: str) -> ResolvedImage: + """Read the image bytes inside the sandbox (fail-closed when none exists). + + Reached when a host read is not permitted or the host file is absent. The + agent can already ``cat`` any container file (file_operations.py reads + root-owned mode-600 files this way), so this stays within the same sandbox + boundary and never touches the host filesystem. ``--`` stops a leading-dash + path from being parsed as a ``base64`` option; ``base64 -w0`` is GNU-only, + so pipe through ``tr -d`` for BusyBox. + + Fail-closed: if there is no active sandbox env we refuse rather than falling + back to a host read, so a non-cache host path under a sandbox never leaks. + """ + import asyncio + import shlex + + env = _get_active_env(ctx.task_id) + if env is None: + raise SourceNotFound( + f"'{p}' is not reachable inside the sandbox and no active sandbox " + f"session is available to read it", + src=src, origin="container") + + # env.execute is a blocking backend exec; keep it off the event loop so a + # multi-MB base64 read doesn't stall every other coroutine. + res = await asyncio.to_thread( + env.execute, f"base64 -- {shlex.quote(str(p))} | tr -d '\\n'") + if res.get("returncode", 1) != 0: + raise SourceNotFound(f"could not read '{p}' inside the sandbox", src=src, origin="container") + try: + data = base64.b64decode(res.get("output", ""), validate=True) + except Exception as exc: + raise NotAnImage(f"sandbox returned non-image data for '{p}': {exc}", src=src) + return _finalize(data, "", "container", src) + + +def _finalize(data: bytes, declared_mime: str, origin: str, src: str) -> ResolvedImage: + """Intrinsic-correctness chokepoint: ingest byte cap + magic-byte sniff. + + The cap here is the generous 50MB *ingest* budget, not the 20MB provider + payload cap — a 20-50MB image must survive this step so the call site can + resize it under the payload cap. See ``_MAX_INGEST_BYTES``. + """ + from tools.vision_tools import _detect_image_mime_type_from_bytes + + if len(data) > _MAX_INGEST_BYTES: + raise SourceTooLarge("image exceeds size limit", src=src, origin=origin) + sniffed = _detect_image_mime_type_from_bytes(data) + if sniffed is None: + raise NotAnImage("source is not a recognized image", src=src, origin=origin) + return ResolvedImage(data=data, mime=sniffed, origin=origin) diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 6b67abec977..06cdd89efd5 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -221,11 +221,14 @@ async def _validate_image_url_async(url: str) -> bool: return await async_is_safe_url(url) -def _detect_image_mime_type(image_path: Path) -> Optional[str]: - """Return a MIME type when the file looks like a supported image.""" - with image_path.open("rb") as f: - header = f.read(64) +def _detect_image_mime_type_from_bytes(data: bytes) -> Optional[str]: + """Magic-byte MIME sniff on raw bytes (authoritative; no extension trust). + Returns ``None`` for anything without a recognized image header — including + SVG, which has no magic bytes and is not ingestible as vision by any + provider. Callers that accept SVG-by-text (path-based) layer that on top. + """ + header = data[:64] if header.startswith(b"\x89PNG\r\n\x1a\n"): return "image/png" if header.startswith(b"\xff\xd8\xff"): @@ -236,11 +239,18 @@ def _detect_image_mime_type(image_path: Path) -> Optional[str]: return "image/bmp" if len(header) >= 12 and header[:4] == b"RIFF" and header[8:12] == b"WEBP": return "image/webp" - if image_path.suffix.lower() == ".svg": + return None + + +def _detect_image_mime_type(image_path: Path) -> Optional[str]: + """Return a MIME type when the file looks like a supported image.""" + with image_path.open("rb") as f: + mime = _detect_image_mime_type_from_bytes(f.read(64)) + if mime is None and image_path.suffix.lower() == ".svg": head = image_path.read_text(encoding="utf-8", errors="ignore")[:4096].lower() if " bool: @@ -817,13 +827,14 @@ async def _vision_concurrency_slot(): async def _vision_analyze_native( image_url: str, question: str, + task_id: Optional[str] = None, ) -> Any: """Fast path for vision-capable main models. - Loads the image (local file OR remote URL), base64-encodes it, and - returns a multimodal tool-result envelope. The agent loop unwraps it; - provider adapters serialize it into the right tool-result-with-image - shape for each backend. + Loads the image (data: / http(s) / file:// / local path / sandbox-container + path) via the unified resolver, base64-encodes it, and returns a multimodal + tool-result envelope. The agent loop unwraps it; provider adapters serialize + it into the right tool-result-with-image shape for each backend. Returns: A ``_multimodal`` envelope dict on success. @@ -840,38 +851,28 @@ async def _vision_analyze_native( if is_interrupted(): return tool_error("Interrupted", success=False) - # Resolve the image source (mirrors vision_analyze_tool's logic - # exactly so behaviour is consistent). - resolved_url = image_url - if resolved_url.startswith("file://"): - resolved_url = resolved_url[len("file://"):] - local_path = Path(os.path.expanduser(resolved_url)) + # Resolve the source to raw bytes through the single resolver (unifies + # data:/http/file/local/container and enforces terminal-backend + # confinement). Materialize to a temp file so the existing path-based + # encode/resize/embed-cap pipeline below is reused verbatim. + from tools.image_source import ( + ImageResolutionError, + ResolveContext, + resolve_image_source, + ) - if local_path.is_file(): - temp_image_path = local_path - should_cleanup = False - elif await _validate_image_url_async(image_url): - blocked = check_website_access(image_url) - if blocked: - return tool_error(blocked["message"], success=False) - temp_dir = get_hermes_dir("cache/vision", "temp_vision_images") - temp_image_path = temp_dir / f"temp_image_{uuid.uuid4()}.jpg" - await _download_image(image_url, temp_image_path) - should_cleanup = True - else: - return tool_error( - "Invalid image source. Provide an HTTP/HTTPS URL or a " - "valid local file path.", - success=False, - ) + try: + resolved = await resolve_image_source(image_url, ResolveContext(task_id=task_id)) + except ImageResolutionError as exc: + return tool_error(str(exc), success=False) - image_size_bytes = temp_image_path.stat().st_size - detected_mime_type = _detect_image_mime_type(temp_image_path) - if not detected_mime_type: - return tool_error( - "Only real image files are supported for vision analysis.", - success=False, - ) + detected_mime_type = resolved.mime + image_size_bytes = len(resolved.data) + temp_dir = get_hermes_dir("cache/vision", "temp_vision_images") + temp_dir.mkdir(parents=True, exist_ok=True) + temp_image_path = temp_dir / f"temp_image_{uuid.uuid4()}.img" + temp_image_path.write_bytes(resolved.data) + should_cleanup = True image_data_url = await _run_encode_on_cpu_executor( _image_to_base64_data_url, @@ -935,6 +936,7 @@ async def vision_analyze_tool( image_url: str, user_prompt: str, model: str = None, + task_id: Optional[str] = None, ) -> str: """ Analyze an image from a URL or local file path using vision AI. @@ -996,42 +998,33 @@ async def vision_analyze_tool( logger.info("Analyzing image: %s", image_url[:60]) logger.info("User prompt: %s", user_prompt[:100]) - - # Determine if this is a local file path or a remote URL - # Strip file:// scheme so file URIs resolve as local paths. - resolved_url = image_url - if resolved_url.startswith("file://"): - resolved_url = resolved_url[len("file://"):] - local_path = Path(os.path.expanduser(resolved_url)) - if local_path.is_file(): - # Local file path (e.g. from platform image cache) -- skip download - logger.info("Using local image file: %s", image_url) - temp_image_path = local_path - should_cleanup = False # Don't delete cached/local files - elif await _validate_image_url_async(image_url): - # Remote URL -- download to a temporary location - blocked = check_website_access(image_url) - if blocked: - raise PermissionError(blocked["message"]) - logger.info("Downloading image from URL...") - temp_dir = get_hermes_dir("cache/vision", "temp_vision_images") - temp_image_path = temp_dir / f"temp_image_{uuid.uuid4()}.jpg" - await _download_image(image_url, temp_image_path) - should_cleanup = True - else: - raise ValueError( - "Invalid image source. Provide an HTTP/HTTPS URL or a valid local file path." - ) - + + # Resolve the source to raw bytes through the single resolver (unifies + # data:/http/file/local/container and enforces terminal-backend + # confinement). Materialize to a temp file so the existing path-based + # encode/resize pipeline below is reused verbatim. + from tools.image_source import ( + ImageResolutionError, + ResolveContext, + resolve_image_source, + ) + + try: + resolved = await resolve_image_source(image_url, ResolveContext(task_id=task_id)) + except ImageResolutionError as exc: + raise ValueError(str(exc)) + + detected_mime_type = resolved.mime + temp_dir = get_hermes_dir("cache/vision", "temp_vision_images") + temp_dir.mkdir(parents=True, exist_ok=True) + temp_image_path = temp_dir / f"temp_image_{uuid.uuid4()}.img" + temp_image_path.write_bytes(resolved.data) + should_cleanup = True + # Get image file size for logging - image_size_bytes = temp_image_path.stat().st_size + image_size_bytes = len(resolved.data) image_size_kb = image_size_bytes / 1024 logger.info("Image ready (%.1f KB)", image_size_kb) - - detected_mime_type = _detect_image_mime_type(temp_image_path) - if not detected_mime_type: - raise ValueError("Only real image files are supported for vision analysis.") - # Convert image to base64 — send at full resolution first. # If the provider rejects it as too large, we auto-resize and retry. # Offloaded to the bounded vision CPU executor so a fan-out of encodes @@ -1335,6 +1328,7 @@ VISION_ANALYZE_SCHEMA = { async def _handle_vision_analyze(args: Dict[str, Any], **kw: Any) -> str: image_url = args.get("image_url", "") question = args.get("question", "") + task_id = kw.get("task_id") # The fan-out cap lives inside the encode/resize step (offloaded to the # bounded _vision_cpu_executor), NOT around the whole analysis — so a @@ -1349,7 +1343,7 @@ async def _handle_vision_analyze(args: Dict[str, Any], **kw: Any) -> str: # information loss, no extra latency. if _should_use_native_vision_fast_path(): logger.info("vision_analyze: native fast path") - return await _vision_analyze_native(image_url, question) + return await _vision_analyze_native(image_url, question, task_id=task_id) # Legacy path: aux LLM describes the image and we return its text. full_prompt = ( @@ -1368,7 +1362,7 @@ async def _handle_vision_analyze(args: Dict[str, Any], **kw: Any) -> str: pass if not model: model = os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None - return await vision_analyze_tool(image_url, full_prompt, model) + return await vision_analyze_tool(image_url, full_prompt, model, task_id=task_id) registry.register(