mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(update): a proxy the user set in .env beats the injected system proxy
The Desktop hands `hermes update` the system proxy in both spellings (HTTPS_PROXY and https_proxy). The .env load replaces only the spelling the file uses, and urllib and curl prefer the lower-case one, so the update still went to the system proxy and the .env NO_PROXY stopped applying. After the .env load, `hermes update` now undoes everything the Desktop added when .env sets a proxy (the Desktop injects nothing when it sees one). A .env NO_PROXY alone replaces the user own entries in every injected NO_PROXY variant and keeps the bypass the Desktop added; a `*` there drops the injection. Review S1 (kshitijk4poor) on #144894.
This commit is contained in:
1 parent
6c4e73376a
commit
ca69d857c3
4 files changed
+83
No files matched your search
@@ -1808,6 +1808,8 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
# declare this local, so only they hand off through retired shims.
|
||||
_hermes_current_updater_frame = True
|
||||
_commit.begin_update_attempt() # nothing an earlier update in this process armed is ours (O5)
|
||||
from hermes_cli.update_proxy_env import prefer_dotenv_proxy_env
|
||||
prefer_dotenv_proxy_env() # the .env loaded at import beats a system proxy the Desktop injected
|
||||
git_operation = git_operation_in_progress(_m().PROJECT_ROOT)
|
||||
if git_operation:
|
||||
root = _m().PROJECT_ROOT
|
||||
|
||||
@@ -43,3 +43,38 @@ def restore_user_proxy_env(environ: MutableMapping[str, str] | None = None) -> N
|
||||
if saved is not None and not users_dotenv_value(key):
|
||||
env[key] = saved
|
||||
env.pop(MARKER, None)
|
||||
|
||||
|
||||
def prefer_dotenv_proxy_env(environ: MutableMapping[str, str] | None = None) -> None:
|
||||
"""In ``hermes update``, after its ``.env`` load: the user's ``.env`` proxy beats the system one.
|
||||
|
||||
The Desktop injects every case variant (``HTTPS_PROXY`` and ``https_proxy``) but the ``.env``
|
||||
load replaces only the spelling the file uses, and urllib and curl prefer the lower-case one, so
|
||||
the update would still go through the system proxy. A ``.env`` proxy is an explicit setting the
|
||||
Desktop could not see; had it seen one it would have injected nothing, so everything it added is
|
||||
undone. A ``.env`` NO_PROXY alone replaces the user's own entries in every injected NO_PROXY
|
||||
variant, keeping the entries the Desktop added; a ``*`` in it bypasses every proxy.
|
||||
"""
|
||||
from agent.proxy_bypass import no_proxy_entries
|
||||
from hermes_cli.env_loader import dotenv_published_values
|
||||
|
||||
env = os.environ if environ is None else environ
|
||||
if MARKER not in env:
|
||||
return
|
||||
if any(dotenv_published_values(name) for name in ("https_proxy", "http_proxy", "all_proxy")):
|
||||
restore_user_proxy_env(env)
|
||||
return
|
||||
published = dotenv_published_values("no_proxy")
|
||||
users = next((env[key] for key in env if key.lower() == "no_proxy" and env[key] in published), None)
|
||||
if users is None:
|
||||
return
|
||||
if "*" in no_proxy_entries(users):
|
||||
restore_user_proxy_env(env)
|
||||
return
|
||||
own = {e for key in _NO_PROXY_KEYS for e in no_proxy_entries(env.get(SAVED_PREFIX + key, ""))}
|
||||
added = [e for key in _NO_PROXY_KEYS if env.get(key, users) != users
|
||||
for e in no_proxy_entries(env[key]) if e not in own]
|
||||
merged = ",".join(dict.fromkeys([*no_proxy_entries(users), *added]))
|
||||
for key in _NO_PROXY_KEYS:
|
||||
if key in env:
|
||||
env[key] = merged
|
||||
@@ -350,6 +350,7 @@ _UPDATE_DEPENDENCIES = (
|
||||
"hermes_cli/backup_restore.py",
|
||||
"hermes_cli/config_backend.py", # update_channel writes the channel key through the seam
|
||||
"hermes_cli/env_loader.py", # update_proxy_env keeps proxy keys a .env load wrote
|
||||
"agent/proxy_bypass.py", # update_proxy_env reads NO_PROXY entries with it
|
||||
"hermes_cli/relay_plugin_migrate.py",
|
||||
"hermes_cli/macos_tcc_anchor.py",
|
||||
"hermes_cli/model_catalog.py",
|
||||
|
||||
@@ -43,3 +43,48 @@ def test_a_proxy_the_users_dotenv_sets_survives_the_restore(tmp_path, monkeypatc
|
||||
proxy_env = {key: os.environ.get(key) for key in ("https_proxy", "HTTP_PROXY", "NO_PROXY", "HERMES_UPDATE_USER_NO_PROXY")}
|
||||
assert proxy_env == {"https_proxy": "http://user:pw@proxy.corp:3128", "HTTP_PROXY": None, "NO_PROXY": "lan.example",
|
||||
"HERMES_UPDATE_USER_NO_PROXY": None}
|
||||
|
||||
|
||||
|
||||
def _desktop_injected_env(monkeypatch, tmp_path, dotenv: str, own_no_proxy: str | None = None):
|
||||
"""The env the Desktop hands `hermes update` (system proxy in both spellings), then the .env load."""
|
||||
from hermes_cli import env_loader
|
||||
|
||||
monkeypatch.setattr(env_loader, "_DOTENV_PUBLISHED", {})
|
||||
monkeypatch.setattr(env_loader, "_LOADED_DOTENV_KEYS", set())
|
||||
for key in ("HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy"):
|
||||
monkeypatch.setenv(key, "http://10.0.0.7:7890")
|
||||
marked = "HTTPS_PROXY,https_proxy,HTTP_PROXY,http_proxy"
|
||||
for key in ("NO_PROXY", "no_proxy"):
|
||||
monkeypatch.setenv(key, ",".join(filter(None, [own_no_proxy, "localhost", "pypi.org"])))
|
||||
if own_no_proxy is None:
|
||||
marked += "," + key
|
||||
else:
|
||||
monkeypatch.setenv("HERMES_UPDATE_USER_" + key, own_no_proxy)
|
||||
monkeypatch.setenv("HERMES_UPDATE_SYSTEM_PROXY", marked)
|
||||
home = tmp_path / "hermes-home"
|
||||
home.mkdir(exist_ok=True)
|
||||
(home / ".env").write_text(dotenv, encoding="utf-8")
|
||||
env_loader.load_hermes_dotenv(hermes_home=home, load_external_secrets=False)
|
||||
|
||||
|
||||
def test_hermes_update_sends_the_update_to_the_users_dotenv_proxy(tmp_path, monkeypatch):
|
||||
"""S1: the .env load replaces only the spelling the file uses, and urllib (like curl) prefers the
|
||||
lower-case one. Once `hermes update` applied its .env, the .env proxy and NO_PROXY must win."""
|
||||
import urllib.request
|
||||
|
||||
from hermes_cli.update_proxy_env import prefer_dotenv_proxy_env
|
||||
|
||||
_desktop_injected_env(monkeypatch, tmp_path, "HTTPS_PROXY=http://proxy.corp:3128\nNO_PROXY=mirror.corp\n")
|
||||
prefer_dotenv_proxy_env()
|
||||
|
||||
assert urllib.request.getproxies_environment().get("https") == "http://proxy.corp:3128"
|
||||
assert urllib.request.getproxies_environment().get("http") is None
|
||||
assert urllib.request.proxy_bypass_environment("mirror.corp")
|
||||
assert "HERMES_UPDATE_SYSTEM_PROXY" not in os.environ
|
||||
|
||||
# A .env NO_PROXY alone replaces the user's old entries and keeps the bypass the Desktop added.
|
||||
_desktop_injected_env(monkeypatch, tmp_path, "NO_PROXY=mirror.corp\n", own_no_proxy="old.lan")
|
||||
prefer_dotenv_proxy_env()
|
||||
|
||||
assert os.environ["NO_PROXY"] == os.environ["no_proxy"] == "mirror.corp,localhost,pypi.org"
|
||||
Reference in new issue
Block a user