fix(vision): address review — restore bare relative paths, remove dead code, async I/O

- resolve_image_source: bare cwd-relative filenames ('pic.png') resolve
  again (main accepted them; the path-shape gate regressed them — review
  by egilewski). Unknown explicit schemes (ftp://, s3://) still rejected.
- Local backend: nonexistent path now raises a clean 'image file not
  found' instead of a misleading sandbox-fallback message.
- W4: remove the now-dead path-based _detect_image_mime_type (suffix-trust
  SVG acceptance) so future callers can't reintroduce it.
- W3: SVG sources rejected with a dedicated actionable message + test.
- Polish: host read_bytes / temp-file write_bytes offloaded via
  asyncio.to_thread (matches the container exec-read); unused
  ResolveContext.cfg/extra_roots fields dropped; duplicate policy check
  documented as intentional pre-flight short-circuit.

tests/tools/test_image_source.py

@@ -69,6 +69,38 @@ class TestLocalBackend:
         res = await isrc.resolve_image_source(f"file://{img}", isrc.ResolveContext())
         assert res.mime == "image/jpeg"
 
+    @pytest.mark.asyncio
+    async def test_bare_relative_path_resolves(self, tmp_path, monkeypatch):
+        """A cwd-relative bare filename ('pic.png') is a valid local source —
+        main accepted it; the resolver must not regress it (PR review)."""
+        isrc = _reload(monkeypatch, tmp_path / "hermes")
+        monkeypatch.setenv("TERMINAL_ENV", "local")
+        img = tmp_path / "pic.png"
+        img.write_bytes(PNG)
+        monkeypatch.chdir(tmp_path)
+        res = await isrc.resolve_image_source("pic.png", isrc.ResolveContext())
+        assert res.data == PNG
+        assert res.origin == "file"
+
+    @pytest.mark.asyncio
+    async def test_unknown_url_scheme_rejected(self, tmp_path, monkeypatch):
+        isrc = _reload(monkeypatch, tmp_path / "hermes")
+        monkeypatch.setenv("TERMINAL_ENV", "local")
+        with pytest.raises(isrc.UnsupportedScheme):
+            await isrc.resolve_image_source(
+                "ftp://example.com/pic.png", isrc.ResolveContext())
+
+    @pytest.mark.asyncio
+    async def test_svg_rejected_with_conversion_hint(self, tmp_path, monkeypatch):
+        """SVG has no raster magic bytes; the resolver rejects it with a
+        dedicated, actionable message (review note W3)."""
+        isrc = _reload(monkeypatch, tmp_path / "hermes")
+        monkeypatch.setenv("TERMINAL_ENV", "local")
+        svg = tmp_path / "art.svg"
+        svg.write_bytes(b'<svg xmlns="http://www.w3.org/2000/svg"></svg>')
+        with pytest.raises(isrc.NotAnImage, match="SVG is not supported"):
+            await isrc.resolve_image_source(str(svg), isrc.ResolveContext())
+
 
 class TestNonLocalBackendConfinement:
     """The security model: under a sandbox backend, host reads are confined to

tests/tools/test_vision_native_fast_path.py

@@ -119,9 +119,9 @@ class TestVisionAnalyzeNative:
         assert isinstance(result, str)
         parsed = json.loads(result)
         assert parsed.get("success") is False
-        # Unified resolver: unrecognized/non-existent local source.
+        # Unified resolver: local backend reports a clean not-found.
         err = parsed.get("error", "").lower()
-        assert "not reachable" in err or "unrecognized image source" in err or "no active sandbox" in err
+        assert "image file not found" in err or "no active sandbox" in err
 
     def test_empty_image_url_returns_error(self):
         result = asyncio.get_event_loop().run_until_complete(

tools/image_source.py

@@ -26,11 +26,13 @@ mechanism that fixes "vision can't see container files" also closes the escape.
 """
 from __future__ import annotations
 
+import asyncio
 import base64
 import os
-from dataclasses import dataclass, field
+import re
+from dataclasses import dataclass
 from pathlib import Path
-from typing import Any, Optional
+from typing import 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),
@@ -70,8 +72,6 @@ class NotAnImage(ImageResolutionError):
 @dataclass
 class ResolveContext:
     task_id: Optional[str] = None
-    cfg: Optional[dict[str, Any]] = None
-    extra_roots: tuple = field(default_factory=tuple)
 
 
 @dataclass
@@ -81,6 +81,11 @@ class ResolvedImage:
     origin: str  # one of: data | http | file | local | container
 
 
+# Explicit URL scheme, e.g. "ftp://", "s3://". Bare Windows drive paths
+# ("C:\x.png") don't match because they lack the "//".
+_SCHEME_RE = re.compile(r"^[A-Za-z][A-Za-z0-9+.\-]*://")
+
+
 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))
@@ -94,27 +99,34 @@ async def resolve_image_source(src: str, ctx: ResolveContext) -> ResolvedImage:
             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)
+    if _SCHEME_RE.match(s) and not s.lower().startswith("file://"):
+        raise UnsupportedScheme(
+            "Unrecognized image source scheme. Use an http(s) URL, a local "
+            "file path, a file:// URI, or a data: URL.",
+            src=s,
+        )
 
-    raise UnsupportedScheme(
-        "Unrecognized image source. Use an http(s) URL, a local file path, "
-        "a file:// URI, or a data: URL.",
-        src=s,
-    )
+    # Everything else is a filesystem path — including bare relative names
+    # like "pic.png" (accepted on main; a path-shape gate here regressed them).
+    candidate = s[len("file://"):] if s.lower().startswith("file://") else s
+    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():
+        data = await asyncio.to_thread(host_target.read_bytes)
+        return _finalize(data, "", "file", s)
+    if _is_local_terminal_backend():
+        # Local backend: any path was host-readable, so a miss simply means
+        # the file doesn't exist — no sandbox to fall back to.
+        raise SourceNotFound(f"image file not found: '{p}'", src=s, origin="file")
+    # Not a permitted host read (or the host file is absent) -> read the
+    # bytes inside the sandbox. Under a sandbox this reads the container's
+    # filesystem, never the host's.
+    return await _resolve_container_fallback(p, ctx, s)
 
 
 def _resolve_data_url(s: str) -> tuple[bytes, str]:
@@ -135,8 +147,12 @@ def _resolve_data_url(s: str) -> tuple[bytes, str]:
 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.
+    Pre-flight short-circuit: policy-blocked URLs are refused BEFORE any
+    network I/O. ``_download_image`` re-checks policy internally (per attempt
+    and against the final redirect target) — that second evaluation is
+    intentional, not redundant: this one guarantees no bytes move for a
+    blocked URL; the inner one covers redirects and non-resolver callers.
+    Preserves the specific website-policy message so the agent sees *why*.
     """
     from tools.url_safety import is_safe_url
     from tools.website_policy import check_website_access
@@ -157,16 +173,15 @@ async def _download_to_bytes(url: str) -> bytes:
     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()
+        # Enforces the 50MB stream cap, redirect SSRF guard, and website policy.
+        await _download_image(url, tmp)
+        return await asyncio.to_thread(tmp.read_bytes)
+    except PermissionError as exc:  # website policy block
+        raise SourceUnsafe(str(exc), src=url, origin="http")
     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.
 
@@ -288,5 +303,12 @@ def _finalize(data: bytes, declared_mime: str, origin: str, src: str) -> Resolve
         raise SourceTooLarge("image exceeds size limit", src=src, origin=origin)
     sniffed = _detect_image_mime_type_from_bytes(data)
     if sniffed is None:
+        if b"<svg" in data[:4096].lower():
+            raise NotAnImage(
+                "SVG is not supported for vision analysis (providers only "
+                "ingest raster images). Convert it to PNG first, e.g. "
+                "`rsvg-convert` or ImageMagick's `convert`.",
+                src=src, origin=origin,
+            )
         raise NotAnImage("source is not a recognized image", src=src, origin=origin)
     return ResolvedImage(data=data, mime=sniffed, origin=origin)

tools/vision_tools.py

@@ -225,8 +225,8 @@ 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.
+    SVG, which has no magic bytes and cannot be ingested as raster vision input
+    by any provider. The resolver surfaces a dedicated error for SVG sources.
     """
     header = data[:64]
     if header.startswith(b"\x89PNG\r\n\x1a\n"):
@@ -242,17 +242,6 @@ def _detect_image_mime_type_from_bytes(data: bytes) -> Optional[str]:
     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 "<svg" in head:
-            return "image/svg+xml"
-    return mime
-
-
 def _is_retryable_download_error(error: Exception) -> bool:
     """Return True only for transient image-download failures worth retrying.
 
@@ -871,7 +860,7 @@ async def _vision_analyze_native(
         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)
+        await asyncio.to_thread(temp_image_path.write_bytes, resolved.data)
         should_cleanup = True
 
         image_data_url = await _run_encode_on_cpu_executor(
@@ -1018,7 +1007,7 @@ async def vision_analyze_tool(
         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)
+        await asyncio.to_thread(temp_image_path.write_bytes, resolved.data)
         should_cleanup = True
 
         # Get image file size for logging