Skip to content

fix(cli): prevent .env sanitizer from splitting GLM_API_KEY by LM_API_KEY suffix#17141

Closed
jackjin1997 wants to merge 1 commit into
NousResearch:mainfrom
jackjin1997:fix/env-sanitize-suffix-collision
Closed

fix(cli): prevent .env sanitizer from splitting GLM_API_KEY by LM_API_KEY suffix#17141
jackjin1997 wants to merge 1 commit into
NousResearch:mainfrom
jackjin1997:fix/env-sanitize-suffix-collision

Conversation

@jackjin1997

Copy link
Copy Markdown
Contributor

What does this PR do?

Fix a corruption bug in _sanitize_env_lines where any registered env var name that is a suffix of another (e.g. LM_API_KEY is a suffix of GLM_API_KEY) would cause the longer key's line to be silently split into corrupted halves like G\nLM_API_KEY=.... This rewrote the user's .env file on disk and disabled provider auth for the affected key (Z.AI/GLM in the report; same shape applies to GLM_BASE_URL vs LM_BASE_URL).

The reporter pinpointed the exact mechanism: the splitter walks each .env line scanning for any known-key name as a substring, with no word-boundary check. stripped.find(\"LM_API_KEY=\") returns 1 inside \"GLM_API_KEY=...\" because LM_API_KEY= is a literal suffix of GLM_API_KEY=.

Related Issue

Fixes #17138

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Changes Made

  • hermes_cli/config.py_sanitize_env_lines: collect full needle ranges (start, end) for every known-key match, then drop matches whose range is fully contained within a longer overlapping match. This kills suffix-collision splits while keeping every legitimate concatenation case (where matches do not nest).
  • tests/hermes_cli/test_config.py — two new tests under TestSanitizeEnvLines:

How to Test

pytest tests/hermes_cli/test_config.py::TestSanitizeEnvLines -v

All 13 tests (11 existing + 2 new) pass. Full `tests/hermes_cli/test_config.py` suite (52 tests) also passes.

Manual repro:

  1. Put `GLM_API_KEY=glm-secret` in `~/.hermes/.env`.
  2. Run `hermes` (or any code path that triggers `sanitize_env_file`).
  3. Before the fix: the file is rewritten as `G\nLM_API_KEY=glm-secret` and `os.getenv("GLM_API_KEY")` returns `None`. After the fix: the file is unchanged.

Checklist

  • Tests added and pass locally
  • Conventional Commit format
  • Single logical change, scope limited to the sanitizer
  • No hardcoded `~/.hermes` paths (uses existing `HERMES_HOME` flow)
  • No `simple_term_menu` / ANSI erase / cross-toolset schema references
  • No `prompt_toolkit` cache-breaking changes

AI Disclosure

This bug was identified and fixed with AI assistance.

…_KEY suffix

The known-key splitter in `_sanitize_env_lines` used substring matching
to find concatenated KEY=VALUE pairs. When a registered key was a suffix
of another (LM_API_KEY is a suffix of GLM_API_KEY), the shorter key's
needle would match inside the longer one, causing the sanitizer to
rewrite `GLM_API_KEY=...` as `G\nLM_API_KEY=...` and silently break
Z.AI/GLM auth (and similarly `GLM_BASE_URL` -> `G\nLM_BASE_URL`).

Drop matches whose needle range is fully contained within a longer
overlapping match. Two regression tests cover the suffix-collision case
and confirm a real concatenation that happens to start with the longer
key still splits where it should.

Fixes NousResearch#17138
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles provider/zai ZAI provider labels Apr 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #17288 — your commit was cherry-picked onto current main with authorship preserved. Thanks for the clean diagnosis (suffix collision in _sanitize_env_lines) and the two regression tests.

#17288

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/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround provider/zai ZAI provider type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: On start, Hermes Agent is santizing GLM_API_KEY/GLM_BASE_URL into G\nLM_API_KEY and G\nLM_BASE_URL

3 participants