Skip to content

fix(config): strip control chars from saved credentials - #58074

Closed
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-env-control-char-credentials
Closed

fix(config): strip control chars from saved credentials#58074
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-env-control-char-credentials

Conversation

@tianma-if

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes save-time credential normalization so save_env_value() strips ASCII control characters before writing ~/.hermes/.env and before mutating os.environ. This prevents copied API keys/tokens containing hidden bytes such as NUL, TAB, or DEL from leaving a malformed .env behind or crashing with ValueError: embedded null character.

Fixes #55335

Type of Change

  • Bug fix
  • Tests

Changes

  • Adds _check_ascii_control_credential() beside the existing non-ASCII credential sanitizer.
  • Routes save_env_value() through the printable-ASCII sanitizer before .env serialization and os.environ assignment.
  • Adds regression coverage proving NUL/TAB/DEL are stripped from both the persisted .env bytes and the process environment.
  • Leaves existing .env load-time non-ASCII sanitizer semantics unchanged; this PR targets the save path described in the issue.

How to Test

  • .venv/bin/python -m pytest tests/hermes_cli/test_non_ascii_credential.py -q
  • .venv/bin/python -m pytest tests/hermes_cli/test_env_load_cache.py tests/hermes_cli/test_env_loader.py tests/hermes_cli/test_config.py -q -k "env or credential or API_KEY or denylisted"
  • .venv/bin/python -m ruff check hermes_cli/config.py tests/hermes_cli/test_non_ascii_credential.py
  • git diff --check

Platform: macOS, Python 3.13, local .venv.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles area/auth Authentication, OAuth, credential pools duplicate This issue or pull request already exists labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #55336 (earliest open, canonical) — both strip ASCII control characters in save_env_value() (hermes_cli/config.py) before writing ~/.hermes/.env / mutating os.environ, closing #55335. Same code site and mechanism (minor delta: this PR keeps TAB). #55767 is the same fix; #40887 is an earlier competing fix with broader scope (URL validation). A maintainer should pick one.

@tianma-if

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #55336 per triage; the earlier PR covers the same save_env_value control-character fix. Thanks.

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

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists 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.

save_env_value can persist ASCII control characters in API keys

2 participants