Commit graph

12 commits

Author SHA1 Message Date
kshitijk4poor
50bb4fe010 fix(vision): auto-resize oversized images, increase default timeout, fix vision capability detection
Cherry-picked from PR #7749 by kshitijk4poor with modifications:

- Raise hard image limit from 5 MB to 20 MB (matches most restrictive provider)
- Send images at full resolution first; only auto-resize to 5 MB on API failure
- Add _is_image_size_error() helper to detect size-related API rejections
- Auto-resize uses Pillow (soft dep) with progressive downscale + JPEG quality reduction
- Fix get_model_capabilities() to check modalities.input for vision support
- Increase default vision timeout from 30s to 120s (matches hardcoded fallback intent)
- Applied retry-with-resize to both vision_analyze_tool and browser_vision

Closes #7740
2026-04-11 11:12:50 -07:00
Teknium
7241e6134b fix: remove stale test (missing pop_pending), add headers to FakeResponse
Follow-up fixes for cherry-pick conflicts:
- Removed test_context_keeps_pending_approval test that referenced
  pop_pending() which doesn't exist on current main
- Added headers attribute to FakeResponse in vision test (needed
  after #6949 added Content-Length check)
2026-04-11 02:03:20 -07:00
Tranquil-Flow
4e56eacdce fix(vision): reject oversized images before API call, handle file:// URIs, improve 400 errors
Three fixes for vision_analyze returning cryptic 400 "Invalid request data":

1. Pre-flight base64 size check — base64 inflates data ~33%, so a 3.8 MB
   file exceeds the 5 MB API limit. Reject early with a clear message
   instead of letting the provider return a generic 400.

2. Handle file:// URIs — strip the scheme and resolve as a local path.
   Previously file:///path/to/image.png fell through to the "invalid
   image source" error since it matched neither is_file() nor http(s).

3. Separate invalid_request errors from "does not support vision" errors
   so the user gets actionable guidance (resize/compress/retry) instead
   of a misleading "model does not support vision" message.

Closes #6677
2026-04-11 02:03:20 -07:00
Dylan Socolobsky
c6dba918b3
fix(tests): fix several failing/flaky tests on main (#6777)
* fix(tests): mock is_safe_url in tests that use example.com

Tests using example.com URLs were failing because is_safe_url does a real DNS lookup which fails in environments where example.com doesn't resolve, causing the request to be blocked before reaching the already-mocked HTTP client. This should fix around 17 failing tests.

These tests test logic, caching, etc. so mocking this method should not modify them in any way. TestMattermostSendUrlAsFile was already doing this so we follow the same pattern.

* fix(test): use case-insensitive lookup for model context length check

DEFAULT_CONTEXT_LENGTHS uses inconsistent casing (MiniMax keys are lowercase, Qwen keys are mixed-case) so the test was broken in some cases since it couldn't find the model.

* fix(test): patch is_linux in systemd gateway restart test

The test only patched is_macos to False but didn't patch is_linux to True. On macOS hosts, is_linux() returns False and the systemd restart code path is skipped entirely, making the assertion fail.

* fix(test): use non-blocklisted env var in docker forward_env tests

GITHUB_TOKEN is in api_key_env_vars and thus in _HERMES_PROVIDER_ENV_BLOCKLIST so the env var is silently dropped, we replace it with a non-blocked one like DATABASE_URL so the tests actually work.

* fix(test): fully isolate _has_any_provider_configured from host env

_has_any_provider_configured() checks all env vars from PROVIDER_REGISTRY (not just the 5 the tests were clearing) and also calls get_auth_status() which detects gh auth token for Copilot. On machines with any of these set, the function returns True before reaching the code path under test.

Clear all registry vars and mock get_auth_status so host credentials don't interfere.

* fix(test): correct path to hermes_base_env.py in tool parser tests

Path(__file__).parent.parent resolved to tests/, not the project root.
The file lives at environments/hermes_base_env.py so we need one more parent level.

* fix(test): accept optional HTML fields in Matrix send payload

_send_matrix sometimes adds format and formatted_body when the markdown library is installed. The test was doing an exact dict equality check which broke. Check required fields instead.

* fix(test): add config.yaml to codex vision requirements test

The test only wrote auth.json but not config.yaml, so _read_main_provider() returned empty and vision auto-detect never tried the codex provider. Add a config.yaml pointing at openai-codex so the fallback path actually resolves the client.

* fix(test): clear OPENROUTER_API_KEY in _isolate_hermes_home

run_agent.py calls load_hermes_dotenv() at import time, which injects API keys from ~/.hermes/.env into os.environ before any test fixture runs. This caused test_agent_loop_tool_calling to make real API calls instead of skipping, which ends up making some tests fail.

* fix(test): add get_rate_limit_state to agent mock in usage report tests

_show_usage now calls agent.get_rate_limit_state() for rate limit
  display. The SimpleNamespace mock was missing this method.

* fix(test): update expected Camofox config version from 12 to 13

* fix(test): mock _get_enabled_platforms in nous managed defaults test

Importing gateway.run leaks DISCORD_BOT_TOKEN into os.environ, which makes _get_enabled_platforms() return ["cli", "discord"] instead of just ["cli"]. tools_command loops per platform, so apply_nous_managed_defaults
  runs twice: the first call sets config values, the second sees them as
  already configured and returns an empty set, causing the assertion to
  fail.
2026-04-09 13:17:06 -07:00
Teknium
5e67fc8c40
fix(vision): reject non-image files and enforce website policy (salvage #1940) (#3845)
Three safety gaps in vision_analyze_tool:

1. Local files accepted without checking if they're actually images —
   a renamed text file would get base64-encoded and sent to the model.
   Now validates magic bytes (PNG, JPEG, GIF, BMP, WebP, SVG).

2. No website policy enforcement on image URLs — blocked domains could
   be fetched via the vision tool. Now checks before download.

3. No redirect check — if an allowed URL redirected to a blocked domain,
   the download would proceed. Now re-checks the final URL.

Fixed one test that needed _validate_image_url mocked to bypass DNS
resolution on the fake blocked.test domain (is_safe_url does DNS
checks that were added after the original PR).

Co-authored-by: GutSlabs <GutSlabs@users.noreply.github.com>
2026-03-29 20:55:04 -07:00
Teknium
0791efe2c3
fix(security): add SSRF protection to vision_tools and web_tools (hardened)
* fix(security): add SSRF protection to vision_tools and web_tools

Both vision_analyze and web_extract/web_crawl accept arbitrary URLs
without checking if they target private/internal network addresses.
A prompt-injected or malicious skill could use this to access cloud
metadata endpoints (169.254.169.254), localhost services, or private
network hosts.

Adds a shared url_safety.is_safe_url() that resolves hostnames and
blocks private, loopback, link-local, and reserved IP ranges. Also
blocks known internal hostnames (metadata.google.internal).

Integrated at the URL validation layer in vision_tools and before
each website_policy check in web_tools (extract, crawl).

* test(vision): update localhost test to reflect SSRF protection

The existing test_valid_url_with_port asserted localhost URLs pass
validation. With SSRF protection, localhost is now correctly blocked.
Update the test to verify the block, and add a separate test for
valid URLs with ports using a public hostname.

* fix(security): harden SSRF protection — fail-closed, CGNAT, multicast, redirect guard

Follow-up hardening on top of dieutx's SSRF protection (PR #2630):

- Change fail-open to fail-closed: DNS errors and unexpected exceptions
  now block the request instead of allowing it (OWASP best practice)
- Block CGNAT range (100.64.0.0/10): Python's ipaddress.is_private
  does NOT cover this range (returns False for both is_private and
  is_global). Used by Tailscale/WireGuard and carrier infrastructure.
- Add is_multicast and is_unspecified checks: multicast (224.0.0.0/4)
  and unspecified (0.0.0.0) addresses were not caught by the original
  four-check chain
- Add redirect guard for vision_tools: httpx event hook re-validates
  each redirect target against SSRF checks, preventing the classic
  redirect-based SSRF bypass (302 to internal IP)
- Move SSRF filtering before backend dispatch in web_extract: now
  covers Parallel and Tavily backends, not just Firecrawl
- Extract _is_blocked_ip() helper for cleaner IP range checking
- Add 24 new tests (CGNAT, multicast, IPv4-mapped IPv6, fail-closed
  behavior, parametrized blocked/allowed IP lists)
- Fix existing tests to mock DNS resolution for test hostnames

---------

Co-authored-by: dieutx <dangtc94@gmail.com>
2026-03-23 15:40:42 -07:00
Teknium
b072737193
fix: expand tilde (~) in vision_analyze local file paths (#2585)
Path('~/.hermes/image.png').is_file() returns False because Path
doesn't expand tilde. This caused the tool to fall through to URL
validation, which also failed, producing a confusing error:
'Invalid image source. Provide an HTTP/HTTPS URL or a valid local
file path.'

Fix: use os.path.expanduser() before constructing the Path object.
Added two tests for tilde expansion (success and nonexistent file).
2026-03-22 23:48:32 -07:00
teknium1
dc11b86e4b refactor: unify vision backend gating 2026-03-14 20:22:13 -07:00
teknium1
29ef69c703 fix: update all test mocks for call_llm migration
Update 14 test files to use the new call_llm/async_call_llm mock
patterns instead of the old get_text_auxiliary_client/
get_vision_auxiliary_client tuple returns.

- vision_tools tests: mock async_call_llm instead of _aux_async_client
- browser tests: mock call_llm instead of _aux_vision_client
- flush_memories tests: mock call_llm instead of get_text_auxiliary_client
- session_search tests: mock async_call_llm with RuntimeError
- mcp_tool tests: fix whitelist model config, use side_effect for
  multi-response tests
- auxiliary_config_bridge: update for model=None (resolved in router)

3251 passed, 2 pre-existing unrelated failures.
2026-03-11 21:06:54 -07:00
teknium1
184aa5b2b3 fix: tighten exc_info assertion in vision test (from PR #803)
The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.com>
2026-03-11 06:32:01 -07:00
SHL0MS
0229e6b407 Fix test_analysis_error_logs_exc_info: mock _aux_async_client so download path is reached 2026-03-10 16:03:19 -04:00
teknium1
2d44ed1c5b test: add comprehensive tests for vision_tools (42 tests)
Covers PR #428 changes and existing vision_tools functionality:
- _validate_image_url: 20 tests for urlparse-based validation
- _determine_mime_type: 6 tests for MIME type detection
- _image_to_base64_data_url: 3 tests for base64 conversion
- _handle_vision_analyze: 5 tests for type hints, prompt building,
  AUXILIARY_VISION_MODEL env var override
- Error logging exc_info: 3 async tests verifying stack traces are
  logged on download failure, analysis error, and cleanup error
- check_vision_requirements & get_debug_session_info: 2 basic tests
- Registry integration: 3 tests for tool registration
2026-03-09 15:32:02 -07:00