fix(vision): unify vision_analyze image-source resolution through one resolver

[banditburai]

fix(vision): unify vision_analyze image-source resolution through one bytes-returning resolver

Closes #7571
Closes #25118
Closes #29643
Closes #22328
Closes #32709
Addresses #9077 (delivery path only — see Caveats)
Supersedes #30197
Supersedes #14990

TL;DR

vision_analyze / mcp_vision_analyze now load images reliably from every source — data: URLs, http(s), file://, local paths, and Docker-container-only paths — instead of returning "no image attached" / "Invalid image source". This is a byte-delivery fix, not a routing change: both call sites now funnel through one resolver tools/image_source.py that returns raw bytes through a single correctness chokepoint. Container files are reached two ways — a cache-dir host fast-path and a universal docker exec base64 exec-read fallback. Pure tools/ layer; agent/image_routing.py is deliberately untouched. Security hardening (a readable-root allowlist) is intentionally out of scope here and left as a no-op seam for a possible follow-up.

Problem

The six clustered issues looked like four unrelated bugs (data:, file://, local paths, sandbox paths), but they share one root cause.

vision_analyze read image bytes in-process on the host (Path.read_bytes(), no subprocess), while read_file / terminal / execute_code read through the sandbox via env.execute → docker exec. So when a file lives in the Docker terminal backend, every other tool can see it — and vision is the only tool reading the wrong filesystem. The reporter of #22328 attributed this to "vision runs in an isolated mount/process namespace"; that is incorrect (vision is in-process on the host). The real boundary is host-vs-sandbox, and it is the same mechanism behind #32709.

A secondary problem: each source branch re-implemented its own ingestion (tools/vision_tools.py:551 _vision_analyze_native and :605 vision_analyze_tool diverged), so size caps, MIME sniffing, and error handling drifted between them. Routing was a red herring — the routing flip was investigated and dropped as non-load-bearing; all six issues fail at byte-delivery.

Solution / Architecture

One resolver, both sites delegate to it:

• resolve_image_source(src, ctx: ResolveContext) -> ResolvedImage (tools/image_source.py:66). ResolvedImage(data: bytes, mime, origin) with origin ∈ {data, http, file, local, container} (:60). Both vision sites call it (tools/vision_tools.py:575, :669); the resolver always returns raw bytes regardless of source.
• Branches: data: (base64) · http(s) (reuses tools/url_safety.is_safe_url SSRF guard + the existing 50 MB download cap) · file:// · local path · container.
• _finalize chokepoint (:204): the single place enforcing intrinsic correctness — a generous _MAX_INGEST_BYTES = 50 MB ingest cap (:23) plus a magic-byte sniff. The cap is deliberately the 50 MB ingest budget, not the 20 MB provider payload cap, so a 20–50 MB image survives to be resized rather than hard-rejected.
• Container delivery, two mechanisms:
  1. host fast-path — cache-dir reverse map tools/credential_files.py:405 from_agent_visible_cache_path (container→host, beside its existing forward twin), called at image_source.py:160.
  2. universal fallback — _resolve_container_fallback (:174) runs base64 -- <shlex.quoted path> | tr -d '\n' via env.execute, wrapped in asyncio.to_thread, fail-closed when no active env.
• bytes-core refactor: encode/resize/sniff helpers now operate on bytes; the Path-signature wrappers (_resize_image_for_vision, _image_to_base64_data_url) are preserved so external callers (tools/browser_tool.py, agent/conversation_compression.py) are untouched.
• Pure tools/ layer: agent/image_routing.py is intentionally unchanged.

Scope & Stats

• 3 core code files: tools/image_source.py (new), tools/vision_tools.py (mod, +265/−243), tools/credential_files.py (mod).
• 7 test files: test_image_source.py (new), test_vision_bytes_helpers.py (new), tests/integration/test_vision_docker_resolve.py (new), test_credential_files.py (mod), test_vision_native_fast_path.py (mod), test_vision_tools.py (mod), test_model_tools_async_bridge.py (mod).
• Diff bcc830100..HEAD: 10 files, +829 / −306. 236 tests pass.
• Adjacent but not closed here: TG-image-as-document fix(telegram): treat image documents (PNG/JPG/WebP/GIF) as images #16710 / fix(telegram): support image files sent as documents #18819 / fix(gateway): route image documents through vision pipeline in Telegram #20756 (already on main); gateway path-exposure fix: preserve local image attachment metadata across vision fallback paths #34315 / fix(telegram): expose inbound media as tool-accessible attachments (#20899) #20906 / fix(gateway): expose native image cache paths to tools #20971; data:-blob session-DB bloat fix: strip base64 image blobs from session DB to prevent context overflow #31705.

Issue → fix mapping

Issue	Symptom	Fixed by	Confidence
#7571	no file:// / local path support	strip file:// + _looks_like_path → file/local branch reads bytes (image_source.py:79)	fully fixed
#25118	TG image, macOS local backend	cache lands host-readable (cache/images/), direct read; reverse-map not load-bearing on local backend	fully fixed
#29643	TG cached image "no image attached", Ubuntu local	same path as #25118 + unified encode via _finalize (:204)	fully fixed
#22328	all local files + browser screenshots fail (zh)	reframe: host↔sandbox boundary (= #32709), not namespace isolation → cache reverse-map (:160) + exec-read (:174)	fixed; inherits #32709 preconditions
#32709	Docker terminal backend, image not sent	exec-read fallback _resolve_container_fallback (:174) + cache reverse-map; task_id threaded dispatch→handler→resolver, seam locked by regression test	fixed; exec-read verified against real Docker (integration test, not in CI — see Caveats)
#9077	"no image" for URL/local/screenshot/data:	unified resolver covers all source types + _finalize	delivery only (see Caveats)

#22328 reframe: the reporter's stated cause (vision in an isolated mount/process namespace) is wrong — vision_analyze runs in-process on the host. The actual cause is the host-vs-sandbox filesystem boundary, identical to #32709, and the same exec-read mechanism resolves both.

Supersedes

• fix(vision): accept base64 data: URLs in vision_analyze #30197 (data: URL support via a temp file, validate=False, no magic-byte check) → ours: _resolve_data_url (:98) decodes straight to bytes (no temp file), validate=True, with an authoritative magic-byte sniff in _finalize. Strict superset — nothing lost, validation added.
• Fix/vision sandbox path resolution #14990 (sandbox path translation via TERMINAL_CWD / docker_volumes host rewrites — only worked when a host mount backed the path) → ours: cache reverse-map + universal exec-read reaches container-only files (tmpfs, root-owned mode-600) that Fix/vision sandbox path resolution #14990 could never read. The explicit docker_volumes / TERMINAL_CWD host rewrite is absorbed by exec-read (same bytes); the allowlist portion is out of scope here (see below).

(No other source-resolver PRs exist — searched vision_analyze in:title, all states.)

Caveats / honest scope

• [vision_analyze] Cannot read local images or URL images - tool returns "no image" for all image sources #9077 is Addresses, not Closes. Two of its sub-symptoms are out of delivery scope: (a) the browser_vision >90 s timeout is latency, not delivery; (b) "model can't see image" (success: true + "I don't see an image" on a non-vision provider, e.g. MiniMax) is a routing/capability matter. The delivery path is fixed.
• Bug: vision_analyze tool fails to send images when using Docker terminal backend #32709 / [Bug]: vision_analyze 工具无法读取任何本地文件（browser 截图也无法分析） #22328 exec-read round-trip is covered by an integration-marked Docker test that is excluded from the default suite (and from CI, which runs -m 'not integration'). It has been run green locally against a real Docker daemon, but it does not gate CI — re-run pytest -m integration tests/integration/test_vision_docker_resolve.py on a Docker host to re-verify. The task_id seam it depends on is in the default unit suite.
• [Bug]: vision_analyze 工具无法读取任何本地文件（browser 截图也无法分析） #22328 / Bug: vision_analyze tool fails to send images when using Docker terminal backend #32709 preconditions: the container path requires a Docker terminal backend + a live task_id threaded dispatch→handler→resolver. Outside those, the local host read applies.

Behavior changes

Change	Before → After	Why	Risk / mitigation	User-visible
20–50 MB images	hard-reject → resize through	_MAX_INGEST_BYTES=50 MB (:23); 20 MB provider cap enforced post-resize at call sites	covered by an incompressible-PNG resize regression test	yes
Policy-block error message	generic → specific website-policy message preserved	_http_block_reason (:113) keeps the reason	unit-tested in test_image_source.py	yes
Bare relative path (cat.png, no /, ./, ~)	probed against process CWD (nondeterministic) → UnsupportedScheme (:91)	a clear deterministic error beats CWD-guessing	newly disclosed; use ./cat.png or an absolute path	yes
SVG for vision	(varied) → rejected (magic-byte sniff → None)	no provider ingests SVG as vision; the path detector keeps SVG-by-text for non-resolver callers	deliberate; isolated to the resolver	yes
Vision-site temp-file bookkeeping	manual should_cleanup at both sites → lifecycle in _download_to_bytes finally + unlink(missing_ok=True) (:141)	single ownership, no leak / double-unlink	internal refactor	no

Security

• Exec-read injection neutralized: base64 -- {shlex.quote(path)} (:194) — -- stops a leading-dash path being parsed as a base64 option; tr -d '\n' handles BusyBox (no GNU -w0).
• Fail-closed: no active sandbox env → resolution refuses rather than probing the host (covered by a fail-closed unit test).
• SSRF retained: redirect re-validation and the 50 MB stream cap stay in _download_image (called from _download_to_bytes, :138).

Testing

236 tests pass.

python -m pytest \
  tests/tools/test_image_source.py \
  tests/tools/test_credential_files.py \
  tests/tools/test_vision_tools.py \
  tests/tools/test_vision_bytes_helpers.py \
  tests/tools/test_vision_native_fast_path.py \
  tests/agent/test_image_routing.py \
  tests/test_model_tools_async_bridge.py -q

• test_image_source.py (new) — every resolve branch: data: / http / file / local / container, SSRF reject, oversize, leading-dash injection neutralization, fail-closed (no env); asserts the policy-block message is preserved.
• test_vision_bytes_helpers.py (new) — magic-byte sniff (incl. SVG → None), base64 encode, resize-noop under cap.
• test_vision_native_fast_path.py (mod) — fast-path gating matrix, 20–50 MB oversize-resize regression (incompressible PNG), and the task_id seam.
• test_credential_files.py (mod) — from_agent_visible_cache_path reverse-map: docker / non-docker / unmapped.
• test_model_tools_async_bridge.py (mod) — [Bug]: 'Exception Event loop is closed' after vision_analyze used as first call to hermes in chat session/some chained tool calls #2104 loop-safety, repatched to resolver seams _http_block_reason / _download_to_bytes.
• test_vision_tools.py (mod) — call-site integration.

Docker integration (integration-marked, excluded from the default suite): tests/integration/test_vision_docker_resolve.py (new) carries pytest.mark.integration, so addopts = -m 'not integration' deselects it from normal runs; it auto-skips when no Docker daemon is present and gets a 180s timeout (repo convention, mirroring tests/docker/conftest.py). It exercises the real-Docker exec-read round-trip for #32709 — a tmpfs /workspace file (no host path) and a root-owned mode-600 file — and is not part of the 236. Verified green against a real Docker daemon (29.4.0), so #32709 / #22328 are fixed-by-test, not just by construction. Run:

pytest -m integration tests/integration/test_vision_docker_resolve.py

Out of scope

This PR is scoped to byte delivery. The items below are intentionally not included — the resolver leaves a clean seam (_within_allowed_roots) so the security piece can be added later without reworking the delivery path.

• Readable-root allowlist — the natural next step: replace the no-op _within_allowed_roots seam (:83 / :148) with real root enforcement, a vision.allowed_image_roots config, and a threat model. The fail-open-vs-fail-closed choice is an open design decision, and any such work must not regress the user-typed-path workflows that mcp_vision_analyze does not support local file paths (file://) #7571 / [Bug]: vision_analyze 工具无法读取任何本地文件（browser 截图也无法分析） #22328 / Vision tool cannot analyze images sent via Telegram gateway #25118 depend on, nor reject an untranslated /workspace path before the container → exec-read route runs.
• docker_volumes / /workspace→TERMINAL_CWD host rewrite — not added; no issue exercises it and the exec-read fallback already reaches those paths (same bytes).
• Decompression-bomb guard — not added. It is only meaningful at the Pillow decode site (_resize_image_bytes_for_vision), and the 50 MB ingest cap already bounds file size; if ever wanted, it belongs as a conditional check there, not a blanket requirement.
• Pillow stays optional — resize degrades gracefully (skips and returns a clear "install Pillow" error). Forcing a heavy native dependency on installs that never use vision is not justified, and a bomb guard only matters when Pillow is already present.

[tonydwb]

Code Review Summary: PR #35362

Verdict: APPROVE ✅

Overall: Significant, well-architected refactor that unifies vision image-source resolution through a single resolve_image_source bytes-returning chokepoint in tools/image_source.py. Closes 6 issues, supersedes 2 previous PRs. 851 additions, 306 deletions across 10 files.

✅ Looks Good

• Architecture: Single resolve_image_source(src, ctx) → ResolvedImage(data, mime, origin) — a single correctness chokepoint replacing divergent implementations in two vision call sites
• Source coverage: Handles data: (base64), http(s) (with existing SSRF guard reuse), file://, local path, and container paths — closes all 6 linked issues
• Container delivery, dual mechanism:
  1. Host fast-path via from_agent_visible_cache_path reverse map
  2. Universal exec-read fallback with base64 -- injection neutralization + shlex.quote
• Fail-closed: No active sandbox env → resolution refuses rather than probing host
• _finalize chokepoint: Magic-byte sniff + 50 MB ingest cap (20-50 MB images resize through rather than hard-rejecting)
• SSRF retained: Reuses existing is_safe_url and redirect re-validation
• Honest scope documentation: Clearly marks #9077 as "Addresses" not "Closes", acknowledges CDP-level and routing-level gaps
• 236 tests pass + dedicated Docker integration test for exec-read round-trip
• Test coverage: New test_image_source.py (every resolve branch, SSRF reject, oversize, injection neutralization), test_vision_bytes_helpers.py (magic-byte sniff, base64, resize)
• SVG rejection: Rejected by magic-byte sniff — correct, no provider ingests SVG as vision
• Clean behavior changes table: Every change in behavior documented with before/after/why/risk
• Out-of-scope documentation: Lists readable-root allowlist, docker_volumes rewrite, decompression-bomb guard as future work — prevents scope creep

💡 Suggestions

1. The Docker integration test (test_vision_docker_resolve.py) is marked integration and excluded from CI — consider adding a lightweight smoke test that can run without Docker (e.g., mocking env.execute) to gate CI
2. Consider adding _within_allowed_roots as a no-op seam for now but documenting the expected vision.allowed_image_roots config key name for future use — makes the follow-up simpler

Testing Completeness

• 236 unit tests pass
• Integration test verified against real Docker 29.4.0
• Regression tests for: resize (incompressible PNG), injection neutralization, fail-closed, oversize, all resolve branches

---

Reviewed by Hermes Agent

[tonydwb]

Code Review Summary: PR #35362

Verdict: APPROVE ✅

Overall: Significant, well-architected refactor that unifies vision image-source resolution through a single resolve_image_source bytes-returning chokepoint in tools/image_source.py. Closes 6 issues, supersedes 2 previous PRs. 851 additions, 306 deletions across 10 files.

✅ Looks Good

• Architecture: Single resolve_image_source(src, ctx) → ResolvedImage(data, mime, origin) — a single correctness chokepoint replacing divergent implementations in two vision call sites
• Source coverage: Handles data: (base64), http(s) (with existing SSRF guard reuse), file://, local path, and container paths — closes all 6 linked issues
• Container delivery, dual mechanism:
  1. Host fast-path via from_agent_visible_cache_path reverse map
  2. Universal exec-read fallback with base64 -- injection neutralization + shlex.quote
• Fail-closed: No active sandbox env → resolution refuses rather than probing host
• _finalize chokepoint: Magic-byte sniff + 50 MB ingest cap (20-50 MB images resize through rather than hard-rejecting)
• SSRF retained: Reuses existing is_safe_url and redirect re-validation
• Honest scope documentation: Clearly marks [vision_analyze] Cannot read local images or URL images - tool returns "no image" for all image sources #9077 as "Addresses" not "Closes", acknowledges CDP-level and routing-level gaps
• 236 tests pass + dedicated Docker integration test for exec-read round-trip
• Test coverage: New test_image_source.py (every resolve branch, SSRF reject, oversize, injection neutralization), test_vision_bytes_helpers.py (magic-byte sniff, base64, resize)
• SVG rejection: Rejected by magic-byte sniff — correct, no provider ingests SVG as vision
• Clean behavior changes table: Every change in behavior documented with before/after/why/risk
• Out-of-scope documentation: Lists readable-root allowlist, docker_volumes rewrite, decompression-bomb guard as future work — prevents scope creep

💡 Suggestions

1. The Docker integration test (test_vision_docker_resolve.py) is marked integration and excluded from CI — consider adding a lightweight smoke test that can run without Docker (e.g., mocking env.execute) to gate CI
2. Consider adding _within_allowed_roots as a no-op seam for now but documenting the expected vision.allowed_image_roots config key name for future use — makes the follow-up simpler

Testing Completeness

• 236 unit tests pass
• Integration test verified against real Docker 29.4.0
• Regression tests for: resize (incompressible PNG), injection neutralization, fail-closed, oversize, all resolve branches

---

Reviewed by Hermes Agent

[NovoG93]

any update on when this might be merged?

[teknium1]

Merged via PR #57890 — your unified image-source resolver design was the foundation of the final fix. The PR was 4140 commits stale and vision_tools.py had been rewritten on both sides, so the resolver was re-authored against current main rather than cherry-picked, but the architecture (single resolver for data:/http/file/local/container sources with one size/magic-byte chokepoint) is yours, and the deliberately-stubbed _within_allowed_roots seam you left became the terminal-backend confinement that closed GHSA-gpxw-6wxv-w3qq. Credited in the PR body. Thanks for the design work — it fixed six open bug reports and a security advisory in one shape.