Skip to content

fix(cli): strip ASCII control chars from getpass output on Windows - #25133

Open
john7864 wants to merge 1 commit into
NousResearch:mainfrom
john7864:fix/windows-getpass-control-chars
Open

fix(cli): strip ASCII control chars from getpass output on Windows#25133
john7864 wants to merge 1 commit into
NousResearch:mainfrom
john7864:fix/windows-getpass-control-chars

Conversation

@john7864

Copy link
Copy Markdown

Summary

On Windows, getpass.getpass() uses msvcrt.getwch() which emits \x00 (or \xe0) followed by a scan code when a special key is pressed (arrow keys, function keys, etc.) instead of filtering them out. An inadvertent arrow keypress just before/during paste therefore injects e.g. \x00K (Left Arrow) into the returned value.

When that value lands in an API-key prompt and is then written via save_env_value(), reloading the .env crashes with:

ValueError: embedded null character

…from python-dotenv's os.environ[k] = v call. The same value also fails when sent as an HTTP header (httpx ASCII-encodes headers, so any non-ASCII byte raises).

Reproduction

  1. On Windows, run hermes setup (or any flow that calls getpass.getpass for an API key) — e.g. hermes model → Kimi → Replace.
  2. With certain terminals/IMEs (or by hitting an arrow key just before pasting), \x00K is silently prepended to the value.
  3. Re-running hermes then crashes during load_dotenv().

Fix

Add read_secret_line() in hermes_cli.cli_output — a thin wrapper that strips \x00\x1f from the getpass.getpass() return value — and route all 19 secret-input call sites across the CLI through it.

File Sites
hermes_cli/cli_output.py helper + existing prompt() updated
hermes_cli/config.py 2
hermes_cli/main.py 11
hermes_cli/auth_commands.py 1
hermes_cli/callbacks.py 1
hermes_cli/memory_setup.py 1
hermes_cli/plugins_cmd.py 1
hermes_cli/setup.py 1

Test plan

  • New tests/hermes_cli/test_read_secret_line.py covers: NUL+scan-code pair, full \x00\x1f range, normal input passthrough, prompt argument passthrough.
  • Manually reproduced the original crash hermes setup → Kimi key with \0K injection → ValueError: embedded null character) and confirmed the fix resolves it.
  • All 8 modified files pass ast.parse.
  • CI on Windows runner.

On Windows, getpass.getpass() uses msvcrt.getwch() which emits a NUL
byte followed by a scan code when a special key is pressed (arrow keys,
function keys, etc.) instead of filtering them out. An inadvertent
arrow keypress before/during paste therefore injects e.g. "\x00K" (the
Left Arrow scan code) into the returned value.

When that value lands in an API-key prompt and is then written via
save_env_value(), reloading the .env crashes with:

    ValueError: embedded null character

…inside python-dotenv's os.environ[k] = v call. The same value also
fails when sent as an HTTP header (httpx ASCII-encodes headers).

This patch adds a thin wrapper read_secret_line() in
hermes_cli.cli_output that strips \x00-\x1f from the getpass result,
and routes all 19 secret-input callsites across the CLI through it.
@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/auth Authentication, OAuth, credential pools labels May 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating a real Windows credential-input failure class. The current branch needs substantive salvage against current main.

Problems

  • hermes_cli/cli_output.py:62 strips the \x00 prefix but leaves its printable scan-code byte. For the reported \x00K sequence, the result remains K-prefixed; tests/hermes_cli/test_read_secret_line.py:19 asserts that result despite its “both bytes” docstring.
  • The call-site sweep is stale: current main routes most secret prompts through hermes_cli/secret_prompt.py:95-99, which consumes the complete Windows special-key pair, but hermes_cli/setup_whatsapp_cloud.py:181 still calls getpass.getpass directly. That value reaches save_env_value; hermes_cli/config.py:7538 removes only CR/LF before the os.environ assignment at line 7602.

Suggested changes

  • Consume the scan-code byte after each \x00/\xe0 prefix and change the paired-input assertion to expect sk-test-abc.
  • Put the durable credential sanitation at save_env_value and test a NUL-containing input, covering direct readers such as the WhatsApp Cloud wizard. The timeline’s related #55336 targets that persistence choke point.

Automated hermes-sweeper review.

Comment thread hermes_cli/cli_output.py
feed the value through additional sanitization (e.g. paste cleanup)
may want the raw, control-char-free string.
"""
return getpass.getpass(prompt).translate({i: None for i in range(32)})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the \x00 prefix but leaves the printable scan-code byte (K in the documented left-arrow sequence), so the corrupted secret is still modified. Consume the following character when the input contains a Windows \x00 or \xe0 special-key prefix; the paired-sequence test should then expect sk-test-abc.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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 comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants