Fix cron session context isolation

[hinablue]

What does this PR do?

This PR fixes a cron-session isolation bug where HERMES_CRON_SESSION was stored in process-global environment state and could leak into later non-cron turns handled by the same Python process.

In practice, this could cause normal gateway/API/ACP/TUI execute_code approvals to be misclassified as cron context and incorrectly routed through approvals.cron_mode.

This patch fixes the problem at the root-cause level by moving cron-session state into scoped per-run context instead of relying on os.environ["HERMES_CRON_SESSION"].

It also adds regression coverage proving that:

• cron jobs still respect approvals.cron_mode
• cron-side execute_code can still be blocked when approvals.cron_mode: deny
• later normal gateway execute_code approval flow is not polluted by the earlier cron run

Related Issue

Fixes #37968

Related:

• fix(cron): isolate gateway approvals from environment pollution #37969
• test: approval_isolation test fails when HERMES_CRON_SESSION=1 in parent env #29625
• Code-quality, reliability & test findings (external full-codebase review) #32043
• [bug]: Cross-talk and Identity Leakage in Cron Job Delivery due to Global Environment Variable Usage #10769

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• replaced process-global cron session marking in cron/scheduler.py with scoped session context via set_session_vars(..., cron_session="1")
• added HERMES_CRON_SESSION ContextVar support in gateway/session_context.py
• updated tools/approval.py to resolve cron approval context from session-scoped context first, with env fallback for compatibility
• explicitly cleared cron context in non-cron entrypoints:
  • gateway/run.py
  • gateway/platforms/api_server.py
  • acp_adapter/server.py
  • tui_gateway/server.py
• added execute_code regression coverage in:
  • tests/tools/test_execute_code_approval_cluster.py
  • tests/tools/test_cron_approval_mode.py
• added scheduler-level regression coverage in:
  • tests/cron/test_scheduler_cron_session_isolation.py

How to Test

1. Reproduce the old failure mode conceptually by running a cron job in-process with approvals.cron_mode: deny, then triggering a normal gateway execute_code request in the same process.
2. Run the focused approval/session regression suite:

pytest tests/tools/test_execute_code_approval_cluster.py tests/tools/test_cron_approval_mode.py tests/gateway/test_session_env.py -q

3. Run the scheduler isolation regression:

pytest tests/cron/test_scheduler_cron_session_isolation.py::test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code -q

4. Run the combined targeted suite:

pytest tests/cron/test_scheduler_cron_session_isolation.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_cron_approval_mode.py -q

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 fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Ubuntu 24.04 / Linux

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

pytest tests/tools/test_execute_code_approval_cluster.py tests/tools/test_cron_approval_mode.py tests/gateway/test_session_env.py -q
Result:
58 passed

Scheduler isolation regression:

pytest tests/cron/test_scheduler_cron_session_isolation.py::test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code -q
Result:
1 passed

Combined targeted suite:

pytest tests/cron/test_scheduler_cron_session_isolation.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_cron_approval_mode.py -q
Result:
46 passed

[liuhao1024]

Verification Review

Reviewed by: automated PR review cron

Summary: Correctly migrates HERMES_CRON_SESSION from a process-wide os.environ flag to a ContextVar in gateway/session_context.py. This prevents cron's approval context from leaking into subsequent gateway turns handled by the same long-lived Python process.

Observations:

• _is_cron_approval_context() in approval.py correctly prefers get_session_env() (ContextVar) with os.environ fallback for backward compat
• All gateway callers (gateway/run.py, tui_gateway/server.py, acp_adapter/server.py, gateway/platforms/api_server.py) explicitly pass cron_session="" to clear the marker
• Cron scheduler passes cron_session="1" via set_session_vars(), which is properly cleaned up by clear_session_vars() after each job
• The _CRON_SESSION ContextVar is included in both the _CONTEXT_ENV_MAP, set_session_vars, and clear_session_vars — consistent wiring
• Test coverage verifies that cron's deny-mode approval does not pollute a subsequent gateway turn

LGTM.

[liuhao1024]

Verification Review — reviewed diff, no issues found.

The ContextVar migration from process-global HERMES_CRON_SESSION env var is well-structured:

• _is_cron_approval_context() correctly prefers ContextVar over env var with fallback
• All 4 approval check sites in approval.py updated (lines 144, 1080, 1132, 1618)
• set_session_vars uses _UNSET sentinel so gateway callers can opt-out with cron_session=""
• acp_adapter/server.py and tui_gateway/server.py correctly pass cron_session="" to prevent leak
• Regression test (test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code) covers the exact failure mode: cron deny blocks execute_code, then a subsequent gateway turn with cron_session="" and gateway auto-approve succeeds
• Existing test_cron_approval_mode.py and test_execute_code_approval_cluster.py fixtures updated to reset the new ContextVar

Clean implementation. The process-global → ContextVar migration pattern matches the existing delivery-target ContextVars already in use.

[liuhao1024]

Verification: Cron session isolation via ContextVar — looks clean.

The migration from process-global os.environ["HERMES_CRON_SESSION"] to a task-local ContextVar is the right fix. The key detail: set_session_vars() now accepts cron_session="" as a sentinel to explicitly clear the flag for non-cron callers (gateway turns, ACP), so a leaked os.environ value from a prior cron tick doesn't force the approval system down the deny branch.

Test coverage is solid — test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code exercises the exact leak scenario end-to-end. The test_guard_gateway_context_masks_leaked_cron_env addition to the existing approval cluster also validates the masking behavior.

One note: the ACP adapter (acp_adapter/server.py) passes cron_session="" explicitly, which correctly masks any inherited env. The API server adapter does the same. All three non-cron entry points (gateway, api_server, ACP) now clear the flag.

[egilewski]

Recommendation: approve / looks mergeable.

I reviewed this security fix against current GitHub main 4cecb1a13a87da169a82279bfd1f1db19eb732da, PR base 5a4297a11a83c38ac24eec7df0e4e41d6b3dbb9f, and PR head d7faa3604b846aee4daf20f28720cb5ec70fb2f0.

Validation:

• git rev-list --left-right --count 4cecb1a13a87da169a82279bfd1f1db19eb732da...d7faa3604b846aee4daf20f28720cb5ec70fb2f0: 44 2; the PR is behind current main but only by upstream commits.
• git merge-tree --write-tree 4cecb1a13a87da169a82279bfd1f1db19eb732da d7faa3604b846aee4daf20f28720cb5ec70fb2f0: passed, wrote tree 655c7001f005220cd601117f33589324111e0abb.
• git diff --check 4cecb1a13a87da169a82279bfd1f1db19eb732da...d7faa3604b846aee4daf20f28720cb5ec70fb2f0: passed with no whitespace/conflict-marker findings.
• Synthetic approval-guard probe on current main with leaked HERMES_CRON_SESSION=1 plus a normal gateway session: reproduced the bug, returning approved=False, outcome=blocked.
• The same probe on PR head with the gateway session binding cron_session="": passed, returning approved=True and user_approved=True.
• /home/mac/hermes-agent/.venv/bin/python -B -m pytest -q tests/cron/test_scheduler_cron_session_isolation.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_cron_approval_mode.py tests/gateway/test_session_env.py -p no:cacheprovider: 59 passed in 2.46s.
• coderabbit review --plain --base upstream/main --type committed: completed with no findings.

Finding:
The patch preserves the security boundary I expected: cron jobs still hit approvals.cron_mode through scoped context, while normal gateway/API/ACP/TUI turns explicitly mask a stale process-global cron marker before execute-code approval routing. I did not find a blocker in the reviewed scope.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Thanks for tracing this to process-global state. The premise still holds on current main: cron/scheduler.py:2688 writes HERMES_CRON_SESSION into os.environ, while tools/approval.py:194 and tools/approval.py:3019 still classify approval behavior from that global marker.

Suggested changes

• Salvage the ContextVar approach onto the current gateway/session_context.py rather than applying the historical snapshot mechanically. Current session isolation uses _VAR_MAP and reset_session_vars() (gateway/session_context.py:256-295), so the cron marker must participate in that map.
• Preserve explicit non-cron bindings at the current session chokepoints: gateway/run.py:15000-15011, gateway/platforms/api_server.py:4057-4063, acp_adapter/server.py:1477-1482, and tui_gateway/server.py:2003-2008.
• Keep the end-to-end scheduler regression; current main has no tracked tests/cron/test_scheduler_cron_session_isolation.py.

Automated hermes-sweeper review.

[hinablue]

Refreshed this PR onto current upstream/main (d2c81eb681) following the sweeper guidance.

The historical snapshot was replaced with a current-architecture port:

• added HERMES_CRON_SESSION to the scoped session ContextVar map
• preserved _UNSET / "1" / "" tri-state behavior and legacy env fallback
• removed the scheduler's process-global cron-session mutation
• bound cron per job and explicitly bound gateway/API/ACP/TUI as non-cron
• centralized cron approval classification
• retained an end-to-end regression proving cron execute_code deny does not pollute a later gateway approval

Fresh verification on commit 571961ce9:

• focused cron/session/approval regression: 80 passed
• session inheritance and subprocess leak suites: 97 passed
• full tests/cron: 694 passed
• API server suites: 214 passed
• ACP suites: 14 passed
• TUI gateway suite: 322 passed
• ruff check, compileall, and git diff --check: passed

The PR is now one commit ahead of current main and GitHub reports it as mergeable.

[alt-glitch]

This was generated by AI during triage.

Related to #58663: both scope the cron marker per execution context, while this head additionally masks a stale legacy environment marker at gateway, API, ACP, and TUI entrypoints. Reviewers can consolidate the overlapping fixes.

[alt-glitch]

This was generated by AI during triage.

Related to #58663: both address the process-global cron approval marker. This refreshed version additionally masks stale cron state in non-cron gateway/API/ACP/TUI bindings, so the implementations should be compared rather than treated as duplicates.

[egilewski]

suggesting changes

check_all_command_guards still lets HERMES_EXEC_ASK=1 bypass the new cron-deny handling. The function only enters its cron branch under if not is_cli and not is_gateway and not is_ask; a cron process inheriting that environment flag instead continues into the ask approval flow. Check cron context before the ask-gated branch (or make the branch include cron) so approvals.cron_mode: deny always blocks cron commands without relying on an interactive approval path.

The separate get_session_env fallback concern is not a merge blocker: its implementation resolves the session ContextVar and then deliberately falls back to the legacy environment for unengaged CLI, cron, and test contexts; no failing lookup path was identified that requires changing that compatibility behavior.

Security evidence:

• trust boundary: cron has no live user approval surface; HERMES_EXEC_ASK is an interactive approval-mode flag that must not weaken cron policy.
• source/sink/invariant: check_all_command_guards reads _is_cron_approval_context() only inside a condition gated by not is_ask; the invariant is that cron-deny is applied before any ask-path approval handling.
• current-main reproduction: not feasible: the affected conditional is introduced by this PR and does not exist on current main.
• PR-head or patch-replay validation: source inspection of the reviewed PR head confirms a true cron context plus HERMES_EXEC_ASK=1 skips the cron-deny conditional; focused regression tests passed but do not cover that combination.
• positive/negative cases: existing tests confirm cron deny when ask is unset; add a cron-deny test with HERMES_EXEC_ASK=1 and verify it remains blocked while a non-cron ask flow is unchanged.
• residual bypass search: the same ordering issue is not present in check_execute_code_guard, which evaluates cron context before the ask path.
• reviewer validation: independently verified after CodeRabbit reported the condition; the separate fallback finding was source-reviewed and is not a merge blocker.

Signed: GPT-5.6-sol-xhigh in Codex

[kshitijk4poor]

Merged via #77022. Your commits were salvaged onto current main with authorship preserved in the Co-authored-by trailer. Your PR had the most thorough approach of the four competing PRs — the tri-state ContextVar design, all four gateway entry points covered, and comprehensive E2E isolation tests. Thank you!

The original PR branch was stale against current main (conflicts in gateway/run.py, acp_adapter/server.py, and test files), so the diff was applied onto a fresh worktree at upstream/main and conflicts resolved preserving current main's changes plus your intended fix.