Skip to content

feat(egress): let bot profiles reuse an opt-in shared proxy - #91326

Open
fangliquanflq wants to merge 2 commits into
NousResearch:mainfrom
fangliquanflq:feat/egress-shared-profile-proxy
Open

fangliquanflq wants to merge 2 commits into
NousResearch:mainfrom
fangliquanflq:feat/egress-shared-profile-proxy

Conversation

@fangliquanflq

Copy link
Copy Markdown

What does this PR do?

This PR lets operators explicitly share the default profile's iron-proxy egress policy with named profiles. Bot Mode and multiplexed gateway profiles can reuse one running daemon, CA, token mappings, and allowlist instead of failing with a per-profile daemon error or requiring manual setup for every bot. Sharing is disabled by default, profile-local proxies retain precedence, and management commands remain scoped to the owning profile.

Related Issue

Closes #91303

Type of Change

  • New feature (non-breaking change that adds functionality)

Changes Made

  • hermes_cli/config_defaults.py - add the default-off proxy.share_with_profiles setting.
  • tools/environments/docker.py - resolve an opted-in default proxy owner for named profiles and preserve the owner's enforcement policy through Docker collision checks and environment precedence.
  • tests/test_iron_proxy.py - cover opt-in sharing, default isolation, local-proxy precedence, and owner-policy enforcement.
  • tests/tools/test_docker_environment.py - cover rejection of provider-key overrides under enforced shared egress.
  • website/docs/user-guide/egress/iron-proxy.md - document setup, ownership, precedence, and isolation behavior.
  • website/docs/developer-guide/egress-internals.md - document the context-local owner resolution contract for backends.

How to Test

  1. In the default profile, set proxy.enabled: true and proxy.share_with_profiles: true, configure iron-proxy, and run hermes egress start.
  2. Start a Docker sandbox from a named profile whose local proxy.enabled is false and confirm that it receives the default profile's CA and proxy-token mappings.
  3. Disable sharing to confirm strict profile isolation, then enable a profile-local proxy to confirm that it takes precedence.
  4. Run the focused regression tests:
scripts/run_tests.sh tests/test_iron_proxy.py -k "docker_egress_reuses_opted_in_default_proxy or docker_egress_keeps_default_proxy_isolated_without_opt_in or docker_egress_prefers_profile_local_proxy or shared_proxy_uses_default_owner_enforcement"
scripts/run_tests.sh tests/tools/test_docker_environment.py -k "enforced_egress_rejects_docker_env_provider_key"
  1. Related post-commit verification completed with 82 passing tests after excluding seven pre-existing Windows-incompatible cases. The final review patch also passed eight focused owner-policy and Docker collision/reuse checks, Ruff, and git diff --check.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this feature (no unrelated commits)
  • I've run the repository test entry point on the relevant test scope and all selected tests pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation
  • Updating cli-config.yaml.example is N/A because defaults are merged from DEFAULT_CONFIG
  • Updating CONTRIBUTING.md or AGENTS.md is N/A because no contributor workflow changed
  • I've considered cross-platform impact; owner resolution uses profile-aware paths and context-local overrides
  • Updating tool descriptions or schemas is N/A because no model tool schema changed

Screenshots / Logs

N/A - this change affects Docker egress configuration and has automated behavioral coverage.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard backend/docker Docker container execution area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping 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 labels Aug 21, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Review of "feat(egress): let bot profiles reuse an opt-in shared proxy". The owner-resolution design (opt-in flag read only from the default profile, profile-local proxy wins, override always reset in finally) is sound and well tested. Findings:

  1. tools/environments/docker.py:1207 — the provider-key collision check switched from name-based ({m.real_env_name for m in load_mappings()}) to value-equality against proxy tokens — a docker_env entry that injects a REAL provider key under its canonical name (e.g. OPENROUTER_API_KEY: sk-real-...) no longer collides, because its value differs from the proxy token; that is exactly the live-secret-into-sandbox leak the deleted comment warned about — recommend unioning the old name-based set (computed from the resolved owner's mappings) with the new value-derived set so both shapes are caught.

  2. tools/environments/docker.py:1253 — merge precedence now gates on _enforce_egress, but this diff removes the only in-scope assignment of that name (the old per-site _enforce_egress_merge recomputation) — if _enforce_egress isn't bound earlier in __init__ (e.g. from _egress_enforce_on_docker()), every egress+docker_env flow hits NameError at runtime and the collision tests can't see it because they raise before this line — please confirm the binding exists on all paths or add one.

  3. tools/environments/docker.py:494 — the improved "Start it from the default profile" hint fires only when status.pid/status.listening fail after configured passes — there is no test for the shared-opt-in-but-daemon-down path, which is the most likely misconfiguration for Bot Mode users — worth adding a case asserting both the raised message and the enforce=False silent-return branch.

  4. tools/environments/docker.py:402 — _resolve_egress_proxy_owner() temporarily flips the process-global HERMES_HOME override and performs up to two extra config loads per sandbox creation — correct under today's serial provisioning, but non-reentrant; add a brief comment or lock if container setup ever goes concurrent, so a parallel thread doesn't observe the owner scope mid-flight.

  5. tests/test_iron_proxy.py:664 — the suite covers reuse, isolation-without-opt-in, and profile-local preference, but not the documented isolation promise that egress management commands (stop/reload/setup) issued from a named profile never touch the shared default daemon — a guard test pinning that behavior would protect the feature's core contract.

Docs (egress-internals.md, iron-proxy.md) accurately describe the new semantics — good. Nothing else blocking beyond items 1-2, which deserve a look before merge given the security framing.

@86doteth

Copy link
Copy Markdown

@teknium1 pretty plz can we have this? really want to enable self-organizing bots without disabling egress

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping backend/docker Docker container execution comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have 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 type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iron-proxy / egress is not shared across profiles — forces manual per-bot setup (breaks Bot Mode + “HR bot that creates bots”)

4 participants