Skip to content

fix: cron model.default fallback + VIRTUAL_ENV leak into subprocesses (P1 bugs)#45501

Open
Sugumaran-Balasubramaniyan wants to merge 1 commit into
NousResearch:mainfrom
Sugumaran-Balasubramaniyan:fix/p1-bugs-cron-model-env-leak
Open

fix: cron model.default fallback + VIRTUAL_ENV leak into subprocesses (P1 bugs)#45501
Sugumaran-Balasubramaniyan wants to merge 1 commit into
NousResearch:mainfrom
Sugumaran-Balasubramaniyan:fix/p1-bugs-cron-model-env-leak

Conversation

@Sugumaran-Balasubramaniyan

Copy link
Copy Markdown

Summary

Two P1 bug fixes discovered during systematic review of open issues.

Fix 1: Cron model.default fallback robustness (#43899)

When a cron job has no explicit model override, the scheduler falls back to config.yaml's model.default. The previous .get('default', model) could propagate an empty string to AIAgent when the default key was absent or None, causing HTTP 400: Model parameter is required.

Fix: Check truthiness of the default value before assigning — ensures empty/None values don't silently replace a valid model.

Fix 2: Block VIRTUAL_ENV leakage into subprocesses (#23473)

The gateway and scheduler set VIRTUAL_ENV for their own process resolution but never scrubbed it from subprocess environments. Package managers (uv, poetry, pipenv, conda) treat an inherited VIRTUAL_ENV as an implicit activation marker, potentially reinstalling the Hermes venv against arbitrary project dependencies — silently destroying the runtime. Still reproduces on v0.16.0 (confirmed 2026-06-12).

Fix: Add VIRTUAL_ENV, VIRTUAL_ENV_PROMPT, UV_PROJECT_ENVIRONMENT, POETRY_ACTIVE, PIPENV_ACTIVE, CONDA_PREFIX, and CONDA_DEFAULT_ENV to _HERMES_PROVIDER_ENV_BLOCKLIST in tools/environments/local.py.

Note on #13079 (read_file dedup)

Also investigated during this pass — the dedup return at tools/file_tools.py:835-841 already uses status: "unchanged" with content_returned: false and the dedup message is in the message field (not content). The _content_is_dedup_stub guard at line 575-601 provides a write-back safety net. This bug appears to be already resolved in the current codebase and may just need issue closure.

Changes

File Change Lines
cron/scheduler.py Defensive model.default fallback +3 / -1
tools/environments/local.py Blocklist 7 venv-related env vars +12

Verification

  • Both changes are additive/safe — no behavioral regression for existing working configs
  • Env blocklist uses the existing filtering mechanism already applied to 50+ other vars
  • Cron model fallback preserves the existing string-config path (line 1593-1594)

Closes #43899, closes #23473

fix(cron): make model.default fallback robust against None/empty values

When a cron job has no explicit model override, the scheduler falls back
to config.yaml's model.default.  Previously the .get('default', model)
could return an empty string when the default key was absent or None,
passing '' to AIAgent and causing HTTP 400 'Model parameter is required'.

Now the fallback checks for truthiness of the default value before
assigning, preventing empty-model propagation.

Closes NousResearch#43899

fix(terminal): block VIRTUAL_ENV leakage into subprocesses

The gateway and scheduler set VIRTUAL_ENV for their own process
resolution but never scrubbed it from subprocess environments.  Package
managers (uv, poetry, pipenv, conda) treat an inherited VIRTUAL_ENV as
an implicit activation marker, potentially reinstalling the Hermes venv
against arbitrary project dependencies and silently destroying the
runtime.  Reproduced on v0.16.0 as of 2026-06-12.

Add VIRTUAL_ENV, VIRTUAL_ENV_PROMPT, UV_PROJECT_ENVIRONMENT,
POETRY_ACTIVE, PIPENV_ACTIVE, CONDA_PREFIX, and CONDA_DEFAULT_ENV to
_HERMES_PROVIDER_ENV_BLOCKLIST in tools/environments/local.py.

Closes NousResearch#23473
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: clean review — no issues found.

Reviewed the diff (15 additions across cron/scheduler.py + tools/environments/local.py). Applied security/agent checklist:

  • Model default fallback fix: _model_cfg.get("default", model) → explicit falsy check is correct. Empty string "" as model name would cause API errors; the new code falls through to the original model variable.
  • VIRTUAL_ENV blocklist: Adding VIRTUAL_ENV, VIRTUAL_ENV_PROMPT, UV_PROJECT_ENVIRONMENT, POETRY_ACTIVE, PIPENV_ACTIVE, CONDA_PREFIX, CONDA_DEFAULT_ENV to _build_provider_env_blocklist() prevents subprocess contamination. Well-commented with issue reference.
  • No regressions: The blocklist is a frozenset — adding entries doesn't affect lookup performance.
  • Scope is minimal: Two independent fixes in two files, each self-contained. No cross-file coupling.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/tools Tool registry, model_tools, toolsets area/config Config system, migrations, profiles labels Jun 13, 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 comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

3 participants