fix(cron): isolate gateway approvals from environment pollution

[coygeek]

Summary

The in-process cron scheduler must not place approval policy in process-global state. A persistent HERMES_CRON_SESSION=1 causes later interactive gateway turns to use approvals.cron_mode; deny mode blocks a live user's command, while approve mode can bypass the expected gateway approval prompt.

This PR now:

• moves the cron marker into a per-job ContextVar and restores its exact prior state with its token
• leaves the legacy os.environ fallback intact for standalone and compatibility entrypoints
• makes context-local cron state authoritative over stale gateway markers
• lets a live gateway ContextVar/platform beat only a stale process-global cron marker
• keeps cron policy authoritative over an inherited HERMES_EXEC_ASK flag
• propagates approval ContextVars through delegated worker threads, preserving cron policy for child work
• covers success, cleanup, gateway, execute-code, ask-mode, and delegated-worker paths

Closes #37968

Root cause

cron/scheduler.py set HERMES_CRON_SESSION in os.environ, even though the default gateway hosts the cron ticker and interactive sessions in the same process. Approval readers in tools/approval.py therefore could not distinguish the job that set the marker from unrelated live turns.

Verification

Rebased onto current main at 477c08b44766ace8b890faa72bf82ecbcf2b3ba8.

• scripts/run_tests.sh tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_delegate.py -q — 214 passed
• CI slice regression set (test_session_env, test_incomplete_gateway_turns, test_stacked_skill_platform_disabled, test_telegram_topic_mode) — 69 passed
• scripts/run_tests.sh tests/cron/test_scheduler.py -q -k run_job_keeps_cron_session_env_unchanged — 1 passed
• .venv/bin/ruff check on all eight changed files — passed
• .venv/bin/python scripts/check-windows-footguns.py on all eight changed files — passed
• git diff --check origin/main...HEAD — passed

The full tests/cron/test_scheduler.py file also reached 213 passing tests, but ten unrelated tick tests currently fail while opening the executions SQLite path in the isolated test environment; the changed scheduler regression passes independently as shown above.

Risk

This changes approval-context routing. The marker remains visible inside each cron job and its delegated workers, but is no longer written into process-global environment state. Cleanup uses token reset rather than an empty-string sentinel so later env-only cron detection continues to work.

[qiyuey]

Observed this in a real Telegram gateway session as well, not just in tests.

While handling a normal user message from Telegram, execute_code was blocked with the cron-mode message:

BLOCKED: execute_code runs arbitrary local Python ... Cron jobs run without a user present to approve it ...

The live tool environment for that turn contained both gateway/session markers and the leaked cron marker:

• HERMES_SESSION_PLATFORM=telegram
• HERMES_EXEC_ASK=1
• HERMES_SESSION_KEY=agent:main:telegram:dm:<redacted>
• HERMES_CRON_SESSION=1

Config had approvals.cron_mode: deny, so check_execute_code_guard() took the cron path and denied execute_code even though the request was an interactive gateway message with an approval surface.

This matches the PR description: HERMES_CRON_SESSION is process-global and can pollute subsequent gateway turns after the in-process cron ticker runs. The practical impact is broader than dangerous shell commands: it also breaks ordinary execute_code use in Telegram/gateway sessions, forcing agents to downgrade to terminal/shell workarounds.

A fix that moves the cron marker to scoped per-job context (or otherwise ensures gateway per-turn context wins over leaked process env) should address the observed failure.

[egilewski]

Recommendation: request changes.

I reviewed this in security mode against current GitHub main 1e7316ced2261576bc4054aa915d3642ebd2b133, PR base ada04573a9669b92556788f2882feb3237753d03, and PR head bdb5611d0651493190af28f19344cfa4465c331a.

Validation:

• git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/37969: passed, wrote tree 59fbc1628b886169efd10afd66d6d3d6f56e5529.
• git diff --check upstream/main...refs/remotes/upstream/pr/37969: passed.
• Patch replay of the PR diff onto current main, then python -B -m pytest -q tests/tools/test_cron_approval_mode.py -p no:cacheprovider: passed, 26 passed.
• Synthetic approval probe on the replayed patch with HERMES_CRON_SESSION=1, set_session_vars(platform="telegram", session_key="ctx-session"), and no HERMES_GATEWAY_SESSION: check_dangerous_command("rm -rf /tmp/stuff", "local") and check_all_command_guards(...) both returned cron-mode BLOCKED, not gateway approval.
• The same probe with legacy HERMES_GATEWAY_SESSION=1 returned approval_required / pending_approval, so the patch fixes only that env-flag path.

Finding:
The fix only gives precedence to the legacy process-global HERMES_GATEWAY_SESSION flag in tools/approval.py, but the messaging gateway binds live session identity through set_session_vars(...) contextvars in gateway/run.py. A stale process-global HERMES_CRON_SESSION=1 still short-circuits before _get_session_platform() is consulted, so a live Telegram/Discord/etc. gateway turn that relies on contextvars remains governed by approvals.cron_mode instead of user approval. The new tests cover only the legacy env flag, not this current gateway path.

Signed: GPT-5.5-xhigh in Codex

[coygeek]

Addressed the review in 897363df8.

I reproduced the broader contextvar/session path that egilewski called out, plus the execute_code failure qiyuey saw in a live gateway turn. The fix now:

• binds HERMES_CRON_SESSION to session-local context so leaked scheduler state does not hijack unrelated gateway turns
• lets live gateway / ask approval surfaces win over a stale cron env marker
• restores the temporary cron env marker after each job finishes
• adds regressions for the contextvar gateway path, execute_code, session-context suppression of stale cron env, and scheduler cleanup

Validation:

• scripts/run_tests.sh tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_execute_code_approval_cluster.py tests/cron/test_scheduler.py -- -q
• .venv/bin/ruff check gateway/session_context.py tools/approval.py cron/scheduler.py tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/cron/test_scheduler.py
• .venv/bin/python scripts/check-windows-footguns.py gateway/session_context.py tools/approval.py cron/scheduler.py tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/cron/test_scheduler.py

[egilewski]

Recommendation: request changes

I reviewed this against current GitHub main d1383a6b1450c6c139720b1b01f8b99cc130453f, PR base ada04573a9669b92556788f2882feb3237753d03, and PR head 897363df847a14875676e5f23d87d1d2c5e73b27.

Validation:

• git merge-tree --write-tree d1383a6b1450c6c139720b1b01f8b99cc130453f refs/remotes/pr/37969: passed.
• HOME=/tmp/hermes-review-pr/runs/37969-20260610T221530Z/home bash scripts/run_tests.sh tests/tools/test_cron_approval_mode.py tests/gateway/test_session_env.py tests/cron/test_scheduler.py: passed, 178 tests.
• Direct approval-routing probe with HERMES_GATEWAY_SESSION=1 plus context-local cron_session="1": failed the cron invariant; check_all_command_guards("rm -rf /tmp/stuff", "local") returned status=pending_approval with no cron-mode block.

Finding:
tools/approval.py still lets process-global gateway state override an actual context-local cron job. _is_gateway_approval_context() checks HERMES_GATEWAY_SESSION before _is_cron_session(), so a cron job running in a process that has the legacy gateway env marker set is treated as a live gateway approval context. That contradicts the function's cron contract and can route cron-dangerous commands into pending gateway approval instead of enforcing approvals.cron_mode.

CodeRabbit reported this, and I reproduced it with the direct probe above. It also pointed out that the temporary os.environ["HERMES_CRON_SESSION"] mirror is still capture/restored by each parallel job independently, so the env mirror can transiently disappear or leak when multiple cron jobs overlap. The ContextVar path is the right primary mechanism, but the legacy env mirror should not reintroduce the process-global race this PR is trying to remove.

Signed: GPT-5.5-xhigh in Codex

[coygeek]

Addressed the latest requested-changes review in 725f2561c.

What changed:

• tools/approval.py now treats a context-local cron session as authoritative over a stale process-global HERMES_GATEWAY_SESSION, so a real cron job cannot be routed into pending gateway approval.
• cron/scheduler.py no longer writes a temporary process-global HERMES_CRON_SESSION mirror for each job; cron state stays in the existing per-job ContextVar path.
• tools/delegate_tool.py now propagates ContextVars across delegate subagent thread boundaries, so delegated cron work keeps cron approval policy after removing the env mirror.
• The delegate propagation is ContextVars-only, preserving the dedicated subagent auto-deny/auto-approve approval callback instead of replacing it with the parent callback.

Validation:

• Direct approval-routing probe with HERMES_GATEWAY_SESSION=1 plus context-local cron_session="1" now returns the cron-mode BLOCKED result instead of pending_approval.
• bash scripts/run_tests.sh tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_execute_code_approval_cluster.py tests/cron/test_scheduler.py tests/tools/test_delegate.py -- -q passed: 327 tests.
• python3 -m ruff check gateway/session_context.py tools/approval.py cron/scheduler.py tools/delegate_tool.py tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_delegate.py tests/cron/test_scheduler.py passed.
• python3 scripts/check-windows-footguns.py gateway/session_context.py tools/approval.py cron/scheduler.py tools/delegate_tool.py tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_delegate.py tests/cron/test_scheduler.py passed.
• git diff --check passed.
• Local structured autoreview --mode local rerun passed with no accepted/actionable findings.

[egilewski]

Recommendation: request changes

I reviewed this against current GitHub main d62979a6f34f64f2ed840f159aac66e24d7cad78, PR base ada04573a9669b92556788f2882feb3237753d03, and PR head 725f2561cf03eba3da37bf8763ebacf1d5b1cf81.

Validation:

• git merge-tree --write-tree upstream/main upstream/pr/37969: passed, produced 575c3d95ab09725cfcb203acb5cb612ef4e7b001.
• git diff --check upstream/main...upstream/pr/37969: passed.
• /home/mac/hermes-agent/.venv/bin/python -B -m pytest -o addopts='' -p no:cacheprovider tests/gateway/test_session_env.py tests/tools/test_cron_approval_mode.py tests/tools/test_execute_code_approval_cluster.py -q: failed 2 tests.
• Direct probe after set_session_vars(...); clear_session_vars(...) showed a later env-only HERMES_CRON_SESSION=1 context returns {'approved': True, 'message': None} from check_execute_code_guard(...) even with approvals.cron_mode patched to deny.

Finding:
Adding HERMES_CRON_SESSION to the session ContextVar map makes clear_session_vars() set that ContextVar to an explicit empty string. Because get_session_env("HERMES_CRON_SESSION") does not fall back to os.environ after an explicit ContextVar value exists, any later legacy/env-only cron context in the same Python context is treated as non-cron. That breaks the existing execute-code cron-deny contract: test_guard_cron_deny_blocks now approves the code path, and test_execute_code_entry_blocks_before_spawn_when_guard_denies can execute successfully instead of returning the cron BLOCKED error.

Please preserve the stale-env suppression for live gateway turns without globally disabling env-only cron detection after session context cleanup.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Thanks for tracing the gateway/cron contamination to a real process-global marker. Current main still sets HERMES_CRON_SESSION globally in cron/scheduler.py:2685-2688, and approval routing checks that marker before the gateway ContextVar path in tools/approval.py:194-198, so the fix direction is valid.

Problems

• gateway/session_context.py:164 adds _CRON_SESSION to clear_session_vars(). That cleanup writes ""; get_session_env() explicitly treats a set empty ContextVar as authoritative and does not fall back to os.environ (gateway/session_context.py:304-315). After any cleared session context, a later legacy env-only HERMES_CRON_SESSION=1 is no longer recognized as cron. This breaks the compatibility contract exercised by the existing env-only cron-deny tests.

Suggested changes

• Separate cron scope cleanup from ordinary gateway-session cleanup, then add a regression for clear-session → env-only cron → cron-deny, including check_execute_code_guard().
• Apply the corrected change against current cron/scheduler.py:2685-2719; the target moved since this PR's base.

Automated hermes-sweeper review.

[coygeek]

Rebased onto current main and addressed the outstanding cleanup review in 1fd2a7cc3.

The cron marker now has separate scope semantics from ordinary gateway-session cleanup: set_session_vars(cron_session=...) returns a dedicated token, and cleanup resets that token instead of pinning an empty value. run_job() likewise sets the marker inside its protected try and resets the exact token in finally. A clear-session → env-only cron sequence therefore still falls back to os.environ and applies cron_mode correctly.

I also closed the adjacent cron-policy bypass identified on the competing implementation: inherited HERMES_EXEC_ASK=1 no longer skips cron deny handling in either the combined terminal guard or execute_code. Live gateway ContextVars still take precedence over only a stale process-global cron marker, while a real context-local cron marker remains authoritative. Delegated worker threads retain the scoped marker.

Verification:

• gateway/session, cron approval, and delegate suites: 213 passed
• changed scheduler regression: 1 passed
• ruff, Windows-footgun scan, and git diff --check: passed

The PR body now contains the exact commands and the isolated full-scheduler-file caveat.

[coygeek]

CI slice 4 exposed one compatibility edge in the new scoped cleanup: several legacy/mock gateway paths call clear_session_vars(None) because the function historically ignored its token argument. Iterating the new cron-token cleanup over None raised before those tests could finish.

Fixed in 0d4e58760 by retaining the legacy None contract while still resetting any real cron token. Added a direct regression.

Verification:

• the four files that failed in slice 4 now pass locally: 69 passed
• focused ruff: passed
• git diff --check: passed

The updated push has started a fresh CI run; the previous slice-4 failure is superseded by this commit.

[egilewski]

suggesting changes

CI is currently failing on this PR head in All required checks pass, Python tests / Run tests slice 8/8. The comparable current main status is not failing for those check(s), so deeper review should wait for the PR-specific required-check failure to be fixed.

Please fix or rerun the failing check, then push a new head or ask for re-review.

Signed: GPT-5.6-terra-low in Codex

[coygeek]

CI slice 8 exposed two stale test seams in tests/tools/test_request_tool_approval.py. Both tests simulated cron by monkeypatching approval.env_var_enabled, but this PR intentionally resolves cron identity through gateway.session_context.get_session_env() so ContextVar state is authoritative while the legacy environment fallback remains available. The mocks therefore no longer reached _is_cron_session() and both tests fell through to the generic no-human denial path.

Fixed in c78e7a4cb by making both behavior tests set HERMES_CRON_SESSION=1 through monkeypatch.setenv, which exercises the actual compatibility fallback instead of the replaced internal seam.

Verification:

• Exact failed file: 13 passed
• Request approval + gateway session + cron approval + delegate regression set: 227 passed
• Focused Ruff check: passed
• git diff --check: passed
• Upstream main remained at the PR base, so this was a normal fast-forward push with no rebase.

The second red check, All required checks pass, was only the aggregate result of the slice-8 failure. A fresh CI run is now attached to the new head.

[egilewski]

looks mergeable

Security evidence:

• trust boundary: cron execution and live gateway turns can share a process, but their approval-policy authority must remain isolated.
• source/sink/invariant: gateway session context is context-local and live gateway context suppresses a stale process-global cron marker before command and execute-code guards select the policy sink.
• current-main reproduction: this is the PR's stated isolation invariant; the submitted head was reviewed as a merge with current GitHub main.
• PR-head or patch-replay validation: the merge tree against current main was conflict-free and the PR diff passed whitespace validation.
• positive/negative cases: 450 focused tests plus a direct routing probe cover gateway-over-stale-cron and context-cron-over-stale-gateway precedence, execute-code routing, cleanup, and delegated ContextVar propagation.
• residual bypass search: gateway-aware classification occurs before cron policy is applied in command and execute-code sinks.
• reviewer validation: CodeRabbit's gateway-fallback finding is invalid because compatibility fallback does not bypass gateway-first approval routing; its callback-restoration note is test-only cleanup.

Signed: GPT-5.6-sol-xhigh in Codex

[kshitijk4poor]

Merged via #77022 using @hinablue's implementation from PR #43370, which was the most thorough of the competing approaches. Your PR identified the correct bug and proposed a valid fix; the merged version uses a ContextVar-based approach instead of env var save/restore for stronger session isolation. Thank you for reporting and contributing!