fix(cron): a bot-chat delivery builds its child env for the DESTINATION profile, not the sender
`deliver: bot-chat:<other profile>` spawned the destination's agent turn with the sending gateway's whole environment: its `.env` settings, bridged `TERMINAL_*` policy, platform authorization gates and provider credentials. The lane called `strip_launch_profile_env(env)` with no target, so the strip resolved against the ambient home override — which is the SENDER's home, never the destination's. On an ordinary root-profile gateway (`hermes gateway run`, no `-p`) `_is_routed_home` is then false and the strip is a complete no-op, including the #113270 gate strip that lives after its early return. This is the only cron child built for a profile other than the one whose tick spawned it; the worker lane (`scheduler.py`) targets its own home, so its no-target call is correct. Build this one through `served_profile_child_env(target_home=home, inherit_credentials=True)` — the helper `kanban_db_dispatch` and `web_server_gateway` already use for cross-profile spawns: it strips the launch residue against the real target, scrubs credentials the launch process was given by systemd/Compose/the shell (which no name-based strip can see), points TMPDIR at the destination's scratch, and overlays the destination's own secrets, as a standalone `hermes -p <profile>` has. A failure to build that environment (an unreadable target home under per-user 0700, a broken secret source) is reported as a refusal string like every other failure in this lane rather than raised: `_deliver_result`'s fan-out does not catch, unlike the deferred drain. Regressions drive the real `_deliver_to_bot_chat`; removing the fix fails the two leak witnesses (`HERMES_MODEL leaked from the launch profile`, and the firing profile's gate reaching another profile's turn under an active override) and leaves the four guard tests green. Fixes #117220 (cherry picked from commit 6cc81d7ddad8c9793f21fdda7c2e260c3ee1ca44)
This commit is contained in:
committed by
Teknium
parent
262825db74
commit
786c0e3f9d
@@ -894,13 +894,20 @@ def _deliver_to_bot_chat(job: dict, content: str, profile: str, *, deferred: Opt
|
||||
return msg
|
||||
|
||||
from agent.delegation_context import delegated_child_subprocess_env
|
||||
from tools.environments.local import strip_launch_profile_env
|
||||
env = strip_launch_profile_env(delegated_child_subprocess_env(os.environ))
|
||||
from tools.environments.local import served_profile_child_env
|
||||
if not home.is_dir():
|
||||
return _fail(f"bot-chat delivery target no longer exists: {home}; do not resend")
|
||||
# Discovery (or deferred admission) owns the destination, not HOME or a
|
||||
# subsequently changed active_profile. Do not resolve the name a second time.
|
||||
env["HERMES_HOME"] = str(home)
|
||||
# Built for ``home``, the DELIVERY TARGET — the only cron child that acts for a profile other
|
||||
# than the one whose tick spawned it, so the launch residue cannot be resolved from the ambient
|
||||
# override the way every other lane resolves it. Discovery (or deferred admission) owns the
|
||||
# destination, not HOME or a subsequently changed active_profile: do not resolve it again.
|
||||
# ``inherit_credentials``: the child runs a full agent turn as that profile, on its own secrets.
|
||||
try:
|
||||
env = served_profile_child_env(
|
||||
delegated_child_subprocess_env(os.environ), target_home=home, inherit_credentials=True)
|
||||
except Exception as exc: # unreadable target home / secret source: refuse, never fall back
|
||||
return _fail(f"bot-chat delivery to profile '{profile_label}' could not build the target "
|
||||
f"profile's environment ({type(exc).__name__}: {exc}); do not resend")
|
||||
if home.parent.name != "profiles":
|
||||
argv += ["-p", "default"]
|
||||
|
||||
|
||||
159
tests/cron/test_bot_chat_delivery_child_env.py
Normal file
159
tests/cron/test_bot_chat_delivery_child_env.py
Normal file
@@ -0,0 +1,159 @@
|
||||
"""``deliver: bot-chat:<other profile>`` spawns its turn with the TARGET profile's environment.
|
||||
|
||||
The Bot Chat CLI lane is the only cron child built for a profile other than the one whose tick
|
||||
spawned it, so it is the only lane whose launch-residue strip cannot be resolved from the ambient
|
||||
home override. It built the child env with ``strip_launch_profile_env(env)`` and no target, which
|
||||
is a no-op whenever the gateway runs its own launch profile — the child then carried the launch
|
||||
profile's ``.env`` settings, bridged ``TERMINAL_*`` policy, authorization gates (#113270) and
|
||||
credentials into another profile's turn.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import cron.scheduler_delivery as delivery
|
||||
|
||||
# What a gateway process that loaded the ROOT profile's .env holds in os.environ.
|
||||
LAUNCH_ENV = {
|
||||
"HERMES_MODEL": "root-model",
|
||||
"HERMES_LANGUAGE": "en",
|
||||
"TERMINAL_ENV": "docker",
|
||||
"TERMINAL_DOCKER_IMAGE": "root-only-image",
|
||||
"DISCORD_ALLOWED_USERS": "root-operator", # an authorization gate (#113270)
|
||||
"DISCORD_IGNORED_CHANNELS": "999",
|
||||
"ANTHROPIC_API_KEY": "sk-root",
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fleet(tmp_path, monkeypatch):
|
||||
"""A root-profile gateway (``hermes gateway run``) and a second profile to deliver into."""
|
||||
root = tmp_path / "hermes"
|
||||
beta = root / "profiles" / "beta"
|
||||
beta.mkdir(parents=True)
|
||||
(root / ".env").write_text(
|
||||
"\n".join(f"{key}={value}" for key, value in LAUNCH_ENV.items()) + "\n", encoding="utf-8")
|
||||
(beta / ".env").write_text("ANTHROPIC_API_KEY=sk-beta\nHERMES_LANGUAGE=ja\n", encoding="utf-8")
|
||||
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||
for key, value in LAUNCH_ENV.items():
|
||||
monkeypatch.setenv(key, value)
|
||||
return root, beta
|
||||
|
||||
|
||||
def _capture_child_env(monkeypatch) -> dict:
|
||||
"""Run the lane up to its spawn and return the env it would have used."""
|
||||
captured: dict = {}
|
||||
|
||||
def _fake_turn(argv, env, report_path, timeout):
|
||||
captured.update(env=dict(env), argv=list(argv))
|
||||
return subprocess.CompletedProcess(argv, 0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", _fake_turn)
|
||||
return captured
|
||||
|
||||
|
||||
def test_the_turn_for_another_profile_carries_that_profile_s_environment(fleet, monkeypatch):
|
||||
root, beta = fleet
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
|
||||
assert delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "beta") is None
|
||||
|
||||
env = captured["env"]
|
||||
assert env["HERMES_HOME"] == str(beta)
|
||||
# The launch profile's settings and bridged terminal policy are not beta's.
|
||||
for key in ("HERMES_MODEL", "TERMINAL_ENV", "TERMINAL_DOCKER_IMAGE"):
|
||||
assert key not in env, f"{key} leaked from the launch profile"
|
||||
# Authorization gates decide who may talk to the agent — never inherited across profiles.
|
||||
for key in ("DISCORD_ALLOWED_USERS", "DISCORD_IGNORED_CHANNELS"):
|
||||
assert key not in env, f"{key} leaked from the launch profile (#113270)"
|
||||
# Credentials are the target profile's own, not the gateway's.
|
||||
assert env["ANTHROPIC_API_KEY"] == "sk-beta"
|
||||
# Beta's own .env is loaded by the child itself; what matters here is that root's value is gone.
|
||||
assert env.get("HERMES_LANGUAGE") != "en"
|
||||
|
||||
|
||||
def test_a_delivery_into_the_gateway_s_own_bot_chat_keeps_its_environment(fleet, monkeypatch):
|
||||
"""``deliver: bot-chat`` (no profile) is the job's own home: nothing to strip."""
|
||||
root, _beta = fleet
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
|
||||
assert delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "") is None
|
||||
|
||||
env = captured["env"]
|
||||
assert env["HERMES_HOME"] == str(root)
|
||||
assert env["HERMES_MODEL"] == "root-model"
|
||||
assert env["TERMINAL_DOCKER_IMAGE"] == "root-only-image"
|
||||
assert env["ANTHROPIC_API_KEY"] == "sk-root"
|
||||
assert env["DISCORD_ALLOWED_USERS"] == "root-operator"
|
||||
|
||||
|
||||
def test_the_delivery_child_still_runs_this_install_and_targets_the_named_chat(fleet, monkeypatch):
|
||||
"""Guard rails the env change must not disturb: the running install, and `-p default`
|
||||
only for a root home."""
|
||||
root, beta = fleet
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "beta")
|
||||
argv = captured["argv"]
|
||||
assert argv[:3] == [os.sys.executable, "-m", "hermes_cli.main"]
|
||||
assert "-p" not in argv # a profile home is addressed by HERMES_HOME, not a flag
|
||||
assert argv[3:10] == ["chat", "--in", "~", "-c", "Bot Chat", "--create-if-missing", "-Q"]
|
||||
|
||||
captured.clear()
|
||||
delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "")
|
||||
assert captured["argv"][3:5] == ["-p", "default"] # a root home keeps its explicit profile flag
|
||||
|
||||
|
||||
def test_a_missing_target_home_is_refused_before_any_child_is_built(fleet, monkeypatch):
|
||||
root, beta = fleet
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
(beta / ".env").unlink()
|
||||
beta.rmdir()
|
||||
|
||||
result = delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "beta")
|
||||
|
||||
assert result is not None and "no longer exists" in result
|
||||
assert captured == {}
|
||||
|
||||
|
||||
def test_an_unbuildable_target_environment_is_refused_and_no_turn_is_spawned(fleet, monkeypatch):
|
||||
"""The lane reports every failure as a string; building the target env must not raise past it
|
||||
(``_deliver_result``'s fan-out does not catch, unlike the deferred drain)."""
|
||||
from tools.environments import local as local_env
|
||||
|
||||
_root, _beta = fleet
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
monkeypatch.setattr(local_env, "served_profile_child_env",
|
||||
lambda *a, **k: (_ for _ in ()).throw(PermissionError("home is 0700 for another user")))
|
||||
|
||||
result = delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "beta")
|
||||
|
||||
assert result is not None and "do not resend" in result and "PermissionError" in result
|
||||
assert captured == {}
|
||||
|
||||
|
||||
def test_the_child_env_is_built_for_the_target_even_while_a_sibling_home_override_is_active(fleet, monkeypatch):
|
||||
"""Under multiplexing the tick runs with the JOB's profile as the home override; the strip must
|
||||
still be resolved against the delivery target, not against whichever home is ambient."""
|
||||
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||
|
||||
root, beta = fleet
|
||||
gamma = root / "profiles" / "gamma"
|
||||
gamma.mkdir(parents=True)
|
||||
(gamma / ".env").write_text("GAMMA_ONLY=1\nDISCORD_ALLOWED_USERS=gamma-operator\n", encoding="utf-8")
|
||||
monkeypatch.setenv("DISCORD_ALLOWED_USERS", "gamma-operator")
|
||||
captured = _capture_child_env(monkeypatch)
|
||||
|
||||
token = set_hermes_home_override(str(gamma))
|
||||
try:
|
||||
delivery._deliver_to_bot_chat({"id": "j", "name": "nightly"}, "the brief", "beta")
|
||||
finally:
|
||||
reset_hermes_home_override(token)
|
||||
|
||||
env = captured["env"]
|
||||
assert env["HERMES_HOME"] == str(beta)
|
||||
assert "DISCORD_ALLOWED_USERS" not in env, "the firing profile's gate reached another profile's turn"
|
||||
assert env["ANTHROPIC_API_KEY"] == "sk-beta"
|
||||
@@ -597,7 +597,7 @@ error. A delivery failure does not count toward the job's `failure_streak`
|
||||
- `bot-chat:<profile>` targets another profile **on the same machine**. Names are validated against `hermes profile list` when the job is created; profiles on other gateways or machines can never be targeted, so same-named profiles across machines are unambiguous.
|
||||
- Each delivery costs the target bot one full agent turn — mind the schedule frequency.
|
||||
- Composes with other targets (`bot-chat,telegram`) but is never included in `all`.
|
||||
- If the canonical chat is open in a mailbox-capable Desktop/TUI backend, delivery is **durably queued immediately**, whether the bot is idle or busy. Only that live owner runs the incoming turn; cron does not start a competing CLI writer. If a CLI-only or older unsupported owner holds the chat, cron retains the never-started output under the sending profile's `cron/bot_chat_pending/<receipt-id>.json`. Later scheduler ticks deliver after that owner releases the chat, in admission order. Deferred work retains its admitted destination home and receipt ID even if the scheduler's launch root changes; a missing/renamed destination is not recreated or resolved to another profile. A `transferred` pending record points to the live-owner receipt, not a failed turn. Malformed JSON records are retained and logged without blocking other queued outputs. With no owner, the existing `hermes chat -c "Bot Chat" --create-if-missing` lane remains available (normal session ownership checks still apply). That child uses the exact destination home already checked by cron, including custom roots; inherited `HOME` or a changed active profile cannot redirect it. A missing destination directory is refused before launch, not recreated. A deferred request is claimed before launching that lane; interruption or an uncertain subprocess result never causes an automatic resend.
|
||||
- If the canonical chat is open in a mailbox-capable Desktop/TUI backend, delivery is **durably queued immediately**, whether the bot is idle or busy. Only that live owner runs the incoming turn; cron does not start a competing CLI writer. If a CLI-only or older unsupported owner holds the chat, cron retains the never-started output under the sending profile's `cron/bot_chat_pending/<receipt-id>.json`. Later scheduler ticks deliver after that owner releases the chat, in admission order. Deferred work retains its admitted destination home and receipt ID even if the scheduler's launch root changes; a missing/renamed destination is not recreated or resolved to another profile. A `transferred` pending record points to the live-owner receipt, not a failed turn. Malformed JSON records are retained and logged without blocking other queued outputs. With no owner, the existing `hermes chat -c "Bot Chat" --create-if-missing` lane remains available (normal session ownership checks still apply). That child uses the exact destination home already checked by cron, including custom roots; inherited `HOME` or a changed active profile cannot redirect it. Its whole environment is the **destination** profile's, as a standalone `hermes -p <profile>` would build it: the sending gateway's `.env` settings, bridged `TERMINAL_*` policy, platform authorization gates and credentials are dropped, and the destination's own secrets are overlaid. A missing destination directory is refused before launch, not recreated. A deferred request is claimed before launching that lane; interruption or an uncertain subprocess result never causes an automatic resend.
|
||||
- Never-started outputs have no TTL: if an unsupported owner never releases, they remain queued rather than being silently dropped. Receipts retain their payloads indefinitely. An unexpected delivery exception is logged and retained as `ambiguous`, without stopping sibling deliveries in that drain; claimed/ambiguous attempts are never automatically replayed.
|
||||
- **Queued is not completed.** Cron records receipt IDs and `queued`/`claimed` statuses in `last_delivery_queued`, with delivery outcome `queued` (neither delivered nor failed). A successful job shows `delivery_queued`; genuine errors on other targets still take precedence as delivery failures. The bot may complete later. The durable receipt in the target profile's `runtime/bot_live_delivery/<receipt-id>.json` is authoritative; cron's historical status is not automatically refreshed.
|
||||
- Rechecking the same execution inspects its existing receipt, even if the owner has disappeared. It never falls back to another writer after acceptance. `failed`, `cancelled`, or `ambiguous` receipts are not automatically replayed; inspect the chat and receipt before intentionally starting new work. Each new cron execution has a distinct delivery ID.
|
||||
|
||||
Reference in New Issue
Block a user