Skip to content

fix(cron): scope workdir to subprocess instead of os.chdir() global process - #69813

Closed
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/cron-workdir-leak
Closed

fix(cron): scope workdir to subprocess instead of os.chdir() global process#69813
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/cron-workdir-leak

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Summary

The no_agent cron job path used os.chdir() to apply the job's workdir, which changed the global process cwd for all threads. Any gateway session (Telegram/Signal/etc.) created while the cron job was running inherited the cron job's workdir, causing AGENTS.md from an unrelated project to be injected into the interactive session's system prompt.

Root Cause

In _run_no_agent_job():

os.chdir(_job_workdir)           # changes cwd for EVERY thread
ok, output = _run_job_script_with_claim_heartbeat(job, script_path)
os.chdir(_prior_cwd)             # restored after script ends

Between the chdir and the restore, any concurrent gateway session sees the wrong cwd.

Fix

Remove os.chdir() and pass the workdir to subprocess.run(cwd=) instead, which scopes the directory change to the child process only. The agent path already correctly uses TERMINAL_CWD env var — this fix aligns the no_agent path.

Changes

  • _run_job_script() — new workdir kwarg, passed to subprocess.run(cwd=)
  • _run_job_script_with_claim_heartbeat() — new workdir kwarg, forwarded
  • _run_no_agent_job() — removed os.chdir()/restore block, passes workdir to runner

Fixes

Fixes #69396

…rocess

The no_agent cron job path used os.chdir() to apply the job's workdir,
which changed the global process cwd for ALL threads. Any gateway
session (Telegram/Signal/etc.) created while the cron job was running
inherited the cron job's workdir, causing AGENTS.md from an unrelated
project to be injected into the interactive session's system prompt.

Fix: remove os.chdir() and pass the workdir to subprocess.run(cwd=)
instead, which scopes the directory change to the child process only.
The agent path already correctly uses TERMINAL_CWD env var.

Fixes NousResearch#69396
@isak-ialogics

Copy link
Copy Markdown
Contributor

Triage evidence (not a merge/close recommendation): at current head b918fab, the new workdir reaches _run_job_script_with_claim_heartbeat(...), but that wrapper still calls _run_job_script(script_path) without forwarding it in all three execution branches (ordinary/unclaimed at line 2270, heartbeat-start failure at 2301, and heartbeat success at 2304):

hermes-agent/cron/scheduler.py

Lines 2262 to 2305 in b918fab

schedule = job.get("schedule")
claim = job.get("run_claim")
owner = str(claim.get("by") or "") if isinstance(claim, dict) else ""
if not (
isinstance(schedule, dict)
and schedule.get("kind") == "once"
and owner
):
return _run_job_script(script_path)
job_id = str(job.get("id") or "")
stop = threading.Event()
heartbeat_context = contextvars.copy_context()
def _heartbeat_loop() -> None:
while not stop.wait(_RUN_CLAIM_HEARTBEAT_SECONDS):
try:
heartbeat_run_claim(job_id, expected_owner=owner)
except Exception:
logger.debug(
"Job '%s': script run_claim heartbeat failed",
job_id,
exc_info=True,
)
heartbeat_thread = threading.Thread(
target=heartbeat_context.run,
args=(_heartbeat_loop,),
name="cron-script-claim-heartbeat",
daemon=True,
)
try:
heartbeat_thread.start()
except Exception:
logger.debug(
"Job '%s': could not start script run_claim heartbeat",
job_id,
exc_info=True,
)
return _run_job_script(script_path)
try:
return _run_job_script(script_path)
finally:
. As a result, no_agent scripts still run with _run_job_script's fallback cwd=str(path.parent), so the configured job workdir is not applied (although the process-global chdir leak is removed). Concrete next action: forward workdir=workdir at those three call sites and add a real-path regression in tests/cron/test_cron_workdir.py or test_cron_script.py that has the script print os.getcwd(), asserts it sees the configured workdir, and asserts the parent process cwd remains unchanged; covering both recurring and claimed one-shot paths would guard both wrapper branches.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #21397, #42274, #56005, and #61774 are live cron workdir fixes. This patch uses the same subprocess-cwd direction but frames a concurrent session prompt-contamination consequence; the saturated cluster needs maintainer selection.

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The process-global chdir leak is removed, but the configured no-agent workdir is still not applied to the script subprocess. _run_job_script_with_claim_heartbeat() accepts workdir, yet each of its three exits calls _run_job_script(script_path) without forwarding it (cron/scheduler.py lines 2270, 2301, and 2304).

I reproduced this against refreshed current main f50c3d90 and PR head b918fab. On current main, a valid configured workdir was visible process-wide while the script still reported the scripts directory. On the PR head, the process-wide leak disappeared, but the script still reported the scripts directory rather than the configured workdir. The focused no-agent/claim-heartbeat tests pass because none asserts the real subprocess cwd.

Please forward workdir=workdir in the ordinary, heartbeat-start-failure, and heartbeat-success branches, and add a real-path regression that asserts both that the script sees its configured workdir and that the parent process cwd remains unchanged. Covering recurring/unclaimed and claimed one-shot jobs would protect both wrapper paths.

Security evidence:

  • trust boundary: a cron job selects a project workdir while the scheduler concurrently serves unrelated sessions.
  • source/sink/invariant: job["workdir"] must reach subprocess.run(cwd=...) without changing the scheduler process cwd.
  • current-main reproduction: at f50c3d90, the observer saw the configured directory process-wide, but the child did not report it.
  • PR-head or patch-replay validation: at b918fab, the observer no longer saw the configured directory process-wide, but the child still did not report it.
  • positive/negative cases: ordinary no-agent execution and claim-heartbeat tests passed; the valid configured-workdir probe failed.
  • residual bypass search: all three wrapper exits omit workdir=workdir, including recurring/unclaimed and claimed one-shot paths.
  • reviewer validation: the head remained open and unchanged after the refreshed-main and PR-head probes.

Signed: GPT-5.6-sol-xhigh in Codex

@alt-glitch alt-glitch added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades area/config Config system, migrations, profiles labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing with credit: the #69396 workdir leak was fixed on main via #70989 using the per-session cwd machinery rather than subprocess-scoping — same goal, the earliest submission chain won. Thanks for the os.chdir() diagnosis, which matched the root cause exactly.

@teknium1 teknium1 closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

[Bug]: cron job workdir is applied as global process cwd and leaks into gateway sessions created during the run

5 participants