Skip to content

fix(security): validate HONCHO_BASE_URL against SSRF before requests - #4986

Open
maymuneth wants to merge 1 commit into
NousResearch:mainfrom
maymuneth:fix/honcho-ssrf-base-url-validation
Open

fix(security): validate HONCHO_BASE_URL against SSRF before requests#4986
maymuneth wants to merge 1 commit into
NousResearch:mainfrom
maymuneth:fix/honcho-ssrf-base-url-validation

Conversation

@maymuneth

Copy link
Copy Markdown
Contributor

What does this PR do?

The Honcho memory plugin reads HONCHO_BASE_URL from an environment
variable and passes it directly to the Honcho client without validation.

If HONCHO_BASE_URL is set to an internal address (e.g.
http://169.254.169.254 on AWS, http://metadata.google.internal on GCP,
or any private network service), every memory read and write would silently
make requests to that internal address — a classic SSRF attack vector.

Fix

Added an is_safe_url() check after reading HONCHO_BASE_URL. If the
URL resolves to a private/internal address, it is ignored with a warning
log and the plugin falls back to API-key-only mode.

This is consistent with the SSRF protection already applied in:

  • tools/homeassistant_tool.py (HASS_URL)
  • plugins/memory/retaindb/__init__.py (RETAINDB_BASE_URL)
  • tools/web_tools.py

Type of Change

  • 🔒 Security fix (SSRF)

Checklist

  • Read the Contributing Guide
  • Commit messages follow Conventional Commits
  • Consistent with existing SSRF protection pattern in Hermes
  • No behavior change for legitimate Honcho endpoints
  • ImportError handled gracefully

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels May 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #4984 (same SSRF validation pattern for RetainDB plugin).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the Honcho endpoint path. There are blockers in the current patch.

Problems

  • plugins/memory/honcho/client.py:192 in the PR patch indents if base_url: with six spaces beneath an eight-space method body, so the patch is not valid Python.
  • The proposed is_safe_url() call would reject the supported self-hosted default http://localhost:8000: current setup advertises that endpoint at hermes_cli/config.py:3957-3960, and get_honcho_client() deliberately handles loopback endpoints at plugins/memory/honcho/client.py:874-887.
  • The environment fallback is not the only endpoint source on current main. plugins/memory/honcho/client.py:454-458 reads Honcho JSON and :841-850 reads honcho.base_url; both later reach the SDK at :908-913.

Suggested changes

  • First correct the indentation. Then re-scope the protection so it preserves documented local/self-hosted deployments, and apply any adopted guard after endpoint resolution rather than only to HONCHO_BASE_URL.

Automated hermes-sweeper review.

resolved_host = host or resolve_active_host()
api_key = os.environ.get("HONCHO_API_KEY")
base_url = os.environ.get("HONCHO_BASE_URL", "").strip() or None
if base_url:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line has six leading spaces while the surrounding method body has eight, so the patch raises an indentation error on import. Please align it with base_url and return cls.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
zapabob added a commit to zapabob/hermes-agent that referenced this pull request Aug 20, 2026
## Summary
- Teach `_resolve_base_url` to fall back when config/env points at always-blocked hosts.
- Preserve localhost / LAN self-host support while blocking metadata SSRF springboards.
- Add focused unit tests.

## Salvage / credit
Same class as RetainDB always-blocked floor; avoids the over-broad private-IP rejection pattern from NousResearch#4984 / NousResearch#4986.
zapabob added a commit to zapabob/hermes-agent that referenced this pull request Aug 22, 2026
## Summary
- Teach `_resolve_base_url` to fall back when config/env points at always-blocked hosts.
- Preserve localhost / LAN self-host support while blocking metadata SSRF springboards.
- Add focused unit tests.

## Salvage / credit
Same class as RetainDB always-blocked floor; avoids the over-broad private-IP rejection pattern from NousResearch#4984 / NousResearch#4986.
zapabob added a commit to zapabob/hermes-agent that referenced this pull request Aug 22, 2026
## Summary
- Teach `_resolve_base_url` to fall back when config/env points at always-blocked hosts.
- Preserve localhost / LAN self-host support while blocking metadata SSRF springboards.
- Add focused unit tests.

## Salvage / credit
Same class as RetainDB always-blocked floor; avoids the over-broad private-IP rejection pattern from NousResearch#4984 / NousResearch#4986.
zapabob added a commit to zapabob/hermes-agent that referenced this pull request Aug 23, 2026
## Summary
- Teach `_resolve_base_url` to fall back when config/env points at always-blocked hosts.
- Preserve localhost / LAN self-host support while blocking metadata SSRF springboards.
- Add focused unit tests.

## Salvage / credit
Same class as RetainDB always-blocked floor; avoids the over-broad private-IP rejection pattern from NousResearch#4984 / NousResearch#4986.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants