Skip to content

fix(codex-runtime): prevent HERMES_HOME tempdir leakage into config.toml (Bug #26250-C) - #26258

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/codex-runtime-hermes-home-sanity
Closed

fix(codex-runtime): prevent HERMES_HOME tempdir leakage into config.toml (Bug #26250-C)#26258
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/codex-runtime-hermes-home-sanity

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

Fixes hermes codex-runtime migrate produces invalid ~/.codex/config.toml (HERMES_HOME tempdir leakage) #26250-C

Summary

Prevent HERMES_HOME from os.environ leaking into user's real ~/.codex/config.toml when hermes codex-runtime migrate runs in a pytest environment. Pytest tempdirs (e.g., pytest-of-*) should NOT be written to the generated config file.

Root Cause

_build_hermes_tools_mcp_entry() read HERMES_HOME directly from os.environ.get("HERMES_HOME") without any sanity checks. When pytest monkeypatch sets HERMES_HOME to a tempdir, that path gets burned into the user's real config file, causing codex MCP subprocess to fail silently.

Fix

  • Derive HERMES_HOME canonically from get_hermes_home() instead of reading os.environ at migrate-time
  • Add sanity checks before writing to config:
    • Path must exist (hermes_path.exists())
    • Path must NOT be a pytest tempdir ("pytest-of-" not in hermes_home)

If both checks fail, omit HERMES_HOME from the generated config entirely. Codex's spawned subprocess will resolve its own HERMES_HOME on startup, so passthrough is optional.

Regression Coverage

Added TestHERMESHomeSanity class with two tests:

  • test_pytest_tempdir_blocked: Verifies that HERMES_HOME pointing at a pytest tempdir is NOT written to config.toml
  • test_nonexistent_hermes_home_blocked: Verifies that HERMES_HOME pointing at a nonexistent path is NOT written to config.toml

Both tests verify migration succeeds but the invalid HERMES_HOME value is not present in the output.

Test Plan

  • python -m pytest tests/hermes_cli/test_codex_runtime_plugin_migration.py::TestHERMESHomeSanity -v
  • python -m pytest tests/hermes_cli/test_codex_runtime_plugin_migration.py -x (full test file)
  • All 58 existing tests pass

Risk Assessment

Low — change is scoped to hermes_tools_mcp_entry() env passthrough logic. MCP server migration and permissions rendering use the same paths as before. Migration behavior only changes when HERMES_HOME would have been written; codex subprocesses resolve their own HERMES_HOME anyway.

…oml (Bug NousResearch#26250-C)

Derive HERMES_HOME canonically from get_hermes_home() rather than reading os.environ at migrate-time. This prevents pytest tempdir leakage into user's real ~/.codex/config.toml.

Sanity checks:
- Path must exist (hermes_path.exists())
- Path must not be a pytest tempdir ("pytest-of-" not in hermes_home)

Add regression tests:
- test_pytest_tempdir_blocked: HERMES_HOME from pytest tempdir should NOT be written
- test_nonexistent_hermes_home_blocked: HERMES_HOME for nonexistent path should NOT be written
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels May 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #26257 — both fix Bug C from #26250 (HERMES_HOME tempdir leakage). Both add validation before embedding HERMES_HOME in config.toml. See also #26259 and #26260 which fix all three bugs from #26250 including this one.

NishantEC

This comment was marked as outdated.

@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #26260.

Triage notes (high confidence):
Merged PR #26260 (2026-05-15) added _looks_like_test_tempdir guard in hermes_cli/codex_runtime_plugin_migration.py:530-553 and applies it before writing HERMES_HOME (line 582).

Thanks for the contribution — the underlying problem this PR addresses has been resolved by the linked PR on current main. If you believe this was closed in error, please comment and we'll reopen.

(Bulk-closed during a CLI PR triage sweep.)

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants