Skip to content

fix: strip ASCII control characters from API keys before saving (#55335) - #55406

Closed
Stoltemberg wants to merge 1 commit into
NousResearch:mainfrom
Stoltemberg:fix/control-chars-api-keys
Closed

fix: strip ASCII control characters from API keys before saving (#55335)#55406
Stoltemberg wants to merge 1 commit into
NousResearch:mainfrom
Stoltemberg:fix/control-chars-api-keys

Conversation

@Stoltemberg

@Stoltemberg Stoltemberg commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #55335 — save_env_value() only stripped \n and \r before writing credentials to ~/.hermes/.env. Other ASCII control characters (NUL, TAB, DEL, etc.) could still be saved, causing ValueError on Windows when os.environ[key] = value is called with embedded null characters.

Related Issue

Fixes #55335

Type of Change

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

Changes Made

Added stripping of all ASCII control characters (0x00-0x1F, 0x7F) except common whitespace (tab) before saving API keys to the .env file.

How to Test

  1. Copy an API key that contains control characters (e.g., from a terminal with NUL bytes)
  2. Save it with hermes auth add
  3. Verify no ValueError occurs when the key is loaded

Platforms Tested

  • Windows 11

@Stoltemberg
fix: strip ASCII control characters from API keys before saving (Nous…
219752b 
…Research#55335)

save_env_value() only stripped \n and \r before writing credentials to
~/.hermes/.env. Other ASCII control bytes (NUL, TAB, DEL, etc.) could
still be saved, causing ValueError on Windows when os.environ[key] = value
is called with embedded null characters.

Fix: strip all ASCII control characters (0x00-0x1F, 0x7F) except common
whitespace (tab) before saving.

Fixes NousResearch#55335.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 30, 2026
@alt-glitch

alt-glitch commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #55336 — both add ASCII control-character stripping to save_env_value() in hermes_cli/config.py for the same issue (#55335). #55336 is the earlier open PR (factors a _strip_ascii_control_credential_chars() helper and adds a regression test). The only behavioral delta here is that this PR keeps TAB while #55336 also strips TAB/DEL. Related: #40887 (earlier competing fix that additionally strips C1 and validates *_URL/_HOST/_ENDPOINT). A maintainer should pick one of #55336 / #40887 / #55406.

@Stoltemberg

Stoltemberg commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing — duplicate detected by alt-glitch bot. Consolidating into earlier PRs. 🙏

@GottZ GottZ mentioned this pull request Jul 29, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

save_env_value can persist ASCII control characters in API keys

2 participants

@Stoltemberg @alt-glitch