Skip to content

fix(cron): migrate HERMES_CRON_SESSION from os.environ to ContextVar - #69766

Closed
im47cn wants to merge 1 commit into
NousResearch:mainfrom
im47cn:fix/cron-session-contextvar-migration
Closed

fix(cron): migrate HERMES_CRON_SESSION from os.environ to ContextVar#69766
im47cn wants to merge 1 commit into
NousResearch:mainfrom
im47cn:fix/cron-session-contextvar-migration

Conversation

@im47cn

@im47cn im47cn commented Jul 23, 2026

Copy link
Copy Markdown

Problem

When the cron scheduler runs inside a gateway process (InProcessCronScheduler, the default), os.environ["HERMES_CRON_SESSION"] = "1" leaks into user-interactive sessions — permanently. Any user who replies to a cron-delivered message in a gateway platform inherits the flag.

Concrete reproduction

Setup: Feishu gateway with 1 cron job (PR/branch monitor, delivers a daily report card to a group chat every 6 hours).

Steps to reproduce:

  1. Cron job fires → scheduler sets os.environ["HERMES_CRON_SESSION"] = "1" (line 2920)
  2. Scheduler delivers the report card to the Feishu group chat
  3. User reads the card and replies to the message thread with a follow-up task (e.g., "check PR fix(skills): align creative ideation skill name #18084 status")
  4. Gateway processes the reply in the same processos.environ["HERMES_CRON_SESSION"] is still "1"
  5. The agent calls execute_code to run a multi-step script → Blocked

Actual error (from production logs, 2026-07-23 08:45 BJT):

{
  "status": "error",
  "error": "BLOCKED: execute_code runs arbitrary local Python 
  (including subprocess calls that bypass shell-string approval checks). 
  Cron jobs run without a user present to approve it."
}

Actual impact: 3 consecutive execute_code calls blocked in a single session. The agent fell back to terminal for each call individually, wasting tokens and slowing down response time.

Process environment at the time of block (confirmed):

$ echo $HERMES_CRON_SESSION
1
$ echo $HERMES_GATEWAY_SESSION  
1

Why this keeps happening

The code comment itself acknowledged the design limitation:

# cron/scheduler.py:2917-2919
# This env var is process-wide and persists for the lifetime of the
# scheduler process — every job this process runs is a cron job.
os.environ["HERMES_CRON_SESSION"] = "1"

This was written for a dedicated scheduler process. But the gateway co-locates the scheduler via InProcessCronScheduler (default since the provider refactor). There is no cleanup — once set, the flag sticks until the gateway process restarts. Every user interaction handled by that process between cron ticks inherits the stale flag.

Affected tools

Any tool gated on check_execute_code_guard or check_all_command_guards is affected when approvals.cron_mode is deny (the default):

  • execute_code
  • web_request
  • Dangerous terminal commands (rm -rf, chmod, mkfs, dd, etc.)

Root Cause

os.environ is process-global. The entire rest of the session state (HERMES_SESSION_PLATFORM, HERMES_SESSION_CHAT_ID, etc.) already migrated to ContextVars in gateway/session_context.py — but HERMES_CRON_SESSION was left behind.

Fix

Migrate HERMES_CRON_SESSION to the existing ContextVar system:

1. gateway/session_context.py — new ContextVar + getter

_CRON_SESSION_FLAG: ContextVar = ContextVar("HERMES_CRON_SESSION", default=_UNSET)

def is_cron_session() -> bool:
    value = _CRON_SESSION_FLAG.get()
    if value is not _UNSET:
        return value == "1"
    return os.getenv("HERMES_CRON_SESSION", "") == "1"  # dedicated process fallback

2. cron/scheduler.py — ContextVar.set instead of os.environ assignment

# Before (process-global leak):
os.environ["HERMES_CRON_SESSION"] = "1"

# After (task-local, no leak):
_VAR_MAP["HERMES_CRON_SESSION"].set("1")

No manual cleanup needed — the existing clear_session_vars() in the finally block already resets all ContextVars (line 3626).

3. tools/approval.py — 4 call sites migrated

# Before:
env_var_enabled("HERMES_CRON_SESSION")

# After:
is_cron_session()

The 4 consumers are:

  • _is_gateway_approval_context() (line 241) — cron is never a gateway approval context
  • check_all_command_guards() (line 2717) — dangerous command blocking
  • check_dangerous_command() (line 3245) — dangerous command detection
  • check_execute_code_guard() (line 3681) — script execution blocking

Why this is the right fix

  • Task-local: flag scoped to cron job's thread/context, never leaks into concurrent gateway sessions
  • No TOCTOU: no reference counting, no cleanup races, no check-then-pop window
  • Architecture-aligned: uses the same ContextVar pattern as every other HERMES_SESSION_* variable
  • Backward compatible: is_cron_session() falls back to os.environ for dedicated cron processes and CLI tests
  • Zero new cleanup code: existing clear_session_vars() handles reset

Tests

All 1100 cron + approval tests pass. One pre-existing failure in test_approval.py (detect_dangerous_command /tmp/ path detection — unrelated to this change).

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery comp/cron Cron scheduler and job management area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data duplicate This issue or pull request already exists labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #58663. Both patches replace the leaking process-global cron-session marker across the same scheduler, session-context, and approval paths; #58663 is the earlier canonical patch and includes ContextVar reset coverage.

…earch#73195)

Add tests/gateway/test_cron_session_contextvar.py covering:

1. is_cron_session() resolution order:
   ContextVar → os.environ fallback, truthy value handling

2. Producer isolation:
   ContextVar.not visible in other threads / copied contexts
   clear_session_vars() resets the flag

3. Defense-in-depth:
   set_session_vars() explicitly gates _CRON_SESSION_FLAG → ''
   Even a leaked os.environ var cannot miscategorize a gateway session

4. Bug scenario (NousResearch#73195):
   Feishu user replies to cron-delivered message
   is_cron_session() → False
   execute_code NOT blocked

5. Backward compat:
   Dedicated cron process (os.environ only) still blocks when
   cron_mode=deny

Also add _CRON_SESSION_FLAG to clear_session_vars() explicit list
and set it to '' in set_session_vars() for defense-in-depth.
@im47cn
im47cn force-pushed the fix/cron-session-contextvar-migration branch from b673cf6 to 8570a40 Compare July 23, 2026 02:53
@im47cn

im47cn commented Jul 23, 2026

Copy link
Copy Markdown
Author

Closing in favor of #58663 (earlier, more-reviewed ContextVar migration by @Adolanium). Two enhancement points submitted as review comments on #58663: (1) defense-in-depth via set_session_vars() gating, (2) Feishu reply-to-cron-message bug-scenario tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants