mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
test(cron): lock the #65773 env-injected credential contract at the cron layer
Issue #65773: run_one_job installs a <home>/.env secret scope around every
job; before c758ded6d (#69057) an installed scope was authoritative even
with multiplexing off, so provider keys injected only via the process
environment (container env vars, systemd Environment=) resolved to empty
inside cron and every provider call went out with the no-key-required
placeholder -> HTTP 401, while interactive turns kept working.
The fix landed in agent/secret_scope.py (scope-miss fallthrough to
os.environ when multiplex is off) with unit tests at that layer only.
These two tests pin the end-to-end contract where the bug actually
surfaced - cron's run_one_job:
- env-injected key resolves during run_job with multiplex OFF (fails on
pre-fix code, verified by mutation against c758ded6d~1)
- .env value still wins when both sources define the key (precedence)
Implementation-agnostic: passes whether the fix is the get_secret
fallthrough (main today) or a multiplex guard at the installation site
(the approach in #65801/#65802/#73037).
This commit is contained in:
parent
5cc5c58e01
commit
41a07f5b84
1 changed files with 98 additions and 0 deletions
|
|
@ -162,6 +162,104 @@ def test_run_one_job_installs_secret_scope_under_multiplex(monkeypatch, tmp_path
|
|||
assert ss.current_secret_scope() is None
|
||||
|
||||
|
||||
def test_run_one_job_env_injected_credential_resolves_without_multiplex(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
"""Regression for #65773: single-profile deployment (multiplex OFF) where
|
||||
the provider key is injected via the process environment ONLY (container
|
||||
env var / systemd Environment= / secret-manager wrapper) and is absent
|
||||
from <home>/.env.
|
||||
|
||||
run_one_job installs a <home>/.env secret scope around every job. Before
|
||||
c758ded6d (#69057, salvage of #67827) an installed scope was authoritative
|
||||
even with multiplexing off, so the env-injected key resolved to empty
|
||||
inside cron, the client shipped the "no-key-required" placeholder, and
|
||||
every provider call 401'd — while interactive turns on the same deployment
|
||||
(which never install a scope when multiplex is off) kept working.
|
||||
|
||||
Behavior contract at the cron layer, regardless of how it's implemented
|
||||
(scope-miss fallthrough on main today, or a multiplex guard on the
|
||||
installation site): during run_job with multiplex OFF,
|
||||
get_secret(<env-injected key>) must return the process-environment value.
|
||||
"""
|
||||
from agent import secret_scope as ss
|
||||
|
||||
# Profile .env exists but does NOT carry the provider key — exactly the
|
||||
# reported deployment shape.
|
||||
(tmp_path / ".env").write_text("UNRELATED_KEY=x\n")
|
||||
monkeypatch.setattr(s, "_get_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("DEEPINFRA_API_KEY", "env-injected-key")
|
||||
|
||||
observed = {}
|
||||
|
||||
def fake_run_job(job, *, defer_agent_teardown=None):
|
||||
# This is where resolve_runtime_provider() reads the credential.
|
||||
observed["key"] = ss.get_secret("DEEPINFRA_API_KEY")
|
||||
# And a key that IS in .env must still resolve (scope stays useful).
|
||||
observed["env_file_key"] = ss.get_secret("UNRELATED_KEY")
|
||||
return (True, "out", "final", None)
|
||||
|
||||
monkeypatch.setattr(s, "run_job", fake_run_job)
|
||||
monkeypatch.setattr(s, "save_job_output", lambda jid, out: f"/tmp/{jid}.txt")
|
||||
monkeypatch.setattr(s, "_deliver_result", lambda *a, **k: None)
|
||||
monkeypatch.setattr(s, "mark_job_run", lambda *a, **k: None)
|
||||
|
||||
# set_multiplex_active writes a module-level global (deployment mode, not
|
||||
# per-task state) — restore whatever was there to avoid cross-test leaks.
|
||||
prev_multiplex = ss.is_multiplex_active()
|
||||
ss.set_multiplex_active(False)
|
||||
try:
|
||||
ok = s.run_one_job({"id": "j-65773", "name": "t"})
|
||||
finally:
|
||||
ss.set_multiplex_active(prev_multiplex)
|
||||
|
||||
assert ok is True
|
||||
# The user-facing symptom: this was None/"" before the fix (key absent
|
||||
# from .env), which became the "no-key-required" placeholder → HTTP 401.
|
||||
assert observed["key"] == "env-injected-key"
|
||||
# .env-sourced secrets keep resolving through the scope.
|
||||
assert observed["env_file_key"] == "x"
|
||||
# No scope leaks out of run_one_job.
|
||||
assert ss.current_secret_scope() is None
|
||||
|
||||
|
||||
def test_run_one_job_env_file_wins_over_environ_without_multiplex(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
"""Precedence half of #65773: when a key exists in BOTH <home>/.env and
|
||||
the process environment, cron must resolve the .env value (the installed
|
||||
scope is an overlay over os.environ, checked first — matching
|
||||
load_hermes_dotenv's .env-overrides-shell precedence on interactive paths).
|
||||
"""
|
||||
from agent import secret_scope as ss
|
||||
|
||||
(tmp_path / ".env").write_text("DEEPINFRA_API_KEY=from-env-file\n")
|
||||
monkeypatch.setattr(s, "_get_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("DEEPINFRA_API_KEY", "stale-shell-value")
|
||||
|
||||
observed = {}
|
||||
|
||||
def fake_run_job(job, *, defer_agent_teardown=None):
|
||||
observed["key"] = ss.get_secret("DEEPINFRA_API_KEY")
|
||||
return (True, "out", "final", None)
|
||||
|
||||
monkeypatch.setattr(s, "run_job", fake_run_job)
|
||||
monkeypatch.setattr(s, "save_job_output", lambda jid, out: f"/tmp/{jid}.txt")
|
||||
monkeypatch.setattr(s, "_deliver_result", lambda *a, **k: None)
|
||||
monkeypatch.setattr(s, "mark_job_run", lambda *a, **k: None)
|
||||
|
||||
prev_multiplex = ss.is_multiplex_active()
|
||||
ss.set_multiplex_active(False)
|
||||
try:
|
||||
ok = s.run_one_job({"id": "j-65773b", "name": "t"})
|
||||
finally:
|
||||
ss.set_multiplex_active(prev_multiplex)
|
||||
|
||||
assert ok is True
|
||||
assert observed["key"] == "from-env-file"
|
||||
assert ss.current_secret_scope() is None
|
||||
|
||||
|
||||
def test_run_one_job_delivers_before_agent_teardown(monkeypatch):
|
||||
"""Regression for #58720: the cron agent's async-resource teardown
|
||||
(agent.close + cleanup_stale_async_clients) MUST run AFTER delivery, not
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue