fix(auth): read .env as utf-8-sig in the dotenv-vs-shell detector

[solyanviktor-star]

Problem

_remove_env_source() in agent/credential_sources.py decides whether a credential variable lives in ~/.hermes/.env or in the shell environment, to give the right hermes auth remove hint. It scans the .env like this:

env_in_dotenv = any(
    line.strip().startswith(f"{env_var}=")
    for line in env_path.read_text(errors="replace").splitlines()   # no encoding
)

read_text() with no encoding falls back to the system locale (cp1252/GBK on Windows) and never strips a BOM. This diverges from the documented codebase invariant:

• hermes_cli/config.py reads .env with encoding="utf-8-sig" in every canonical path, with the explicit comment: "tolerate BOM via utf-8-sig since users may edit .env in Notepad which adds one."
• hermes_cli/doctor.py: ".env files are written as UTF-8 everywhere in the codebase, while Path.read_text() defaults to the system locale — which crashes on non-UTF-8 Windows locales."

So on a Notepad-edited .env (UTF-8 BOM), the BOM prefixes the first line — "\ufeffOPENAI_API_KEY=..." — and startswith("OPENAI_API_KEY=") is False for the first variable in the file. The detector then concludes the var isn't in .env and prints a misleading hint that it's "still set in your shell environment" when it is not.

Repro (Windows, cp1251)

p.write_bytes("\ufeffOPENAI_API_KEY=sk-secret\n".encode("utf-8"))
p.read_text(errors="replace").splitlines()[0].startswith("OPENAI_API_KEY=")          # False  (bug)
p.read_text(encoding="utf-8-sig", errors="replace").splitlines()[0].startswith("OPENAI_API_KEY=")  # True

Fix

Read the .env with encoding="utf-8-sig", errors="replace", matching the canonical readers in config.py.

Tests

Added test_auth_remove_env_seeded_dotenv_with_bom_no_shell_hint: a BOM'd .env with the target var on the first line — asserts the removal clears it from .env and does not print the phantom shell-export warning. Verified it fails on the pre-fix reader and passes after.

python -m pytest tests/hermes_cli/test_auth_commands.py — 53 passed.

🤖 Generated with Claude Code

[falkoro]

Community review — verified this against the codebase and reproduced the bug locally.

The canonical .env readers already use encoding="utf-8-sig", errors="replace" (two sites in hermes_cli/config.py, with a comment about Notepad BOMs), so this brings the dotenv-vs-shell detector in line with them. Repro with a BOM'd first line (b'\xef\xbb\xbfDEEPSEEK_API_KEY=...'): the current reader fails to find the var (startswith sees \ufeffDEEPSEEK...), the patched reader finds it — exactly the misreported shell-export hint from the issue. read_text() with no encoding also decodes via the system locale (cp1252/GBK on Windows), so this fixes non-ASCII values in .env as a bonus. The regression test covers the BOM'd-first-line case specifically. LGTM from a community perspective.

[tonydwb]

Code Review Summary

Verdict: Comment (token read-only)

PR 60895 fixes .env file reading with UTF-8-sig encoding (BOM). Ensures the dotenv-vs-shell detector properly handles BOM-encoded .env files. Well-scoped (2 files, 52 additions, 1 deletion). No security issues or debug artifacts detected.

LGTM - awaiting maintainer approval.

[teknium1]

Thanks for the focused regression fix. Current main still reads this dotenv path without an explicit encoding at agent/credential_sources.py:167; that result directly controls the shell-export diagnostic at lines 171 and 177-184. The proposed utf-8-sig reader matches the established dotenv readers in hermes_cli/config.py:7273 and the remover at hermes_cli/config.py:7633. The added test exercises the real auth_remove_command flow with an isolated HERMES_HOME, extending the adjacent existing dotenv-only behavior test at tests/hermes_cli/test_auth_commands.py:1514.

Automated hermes-sweeper review.

[solyanviktor-star]

Rebased onto current main after the #71078 class sweep (and thanks for salvaging my four sibling PRs there — much appreciated!).

One detail survived the sweep at this site: 75e0d52 pinned this reader to plain encoding="utf-8", but the original repro of this PR is specifically the BOM case — \ufeff is not whitespace, so line.strip().startswith(f"{env_var}=") still misses the first key of a Notepad-edited .env, and hermes auth remove still misreports that variable as a shell export. utf-8-sig (the canonical choice documented in hermes_cli/config.py for exactly this reason, and what #71078's summary prescribes for .env readers) also handles BOM-less UTF-8 unchanged.

This PR is now the minimal delta on top of the sweep: utf-8 → utf-8-sig on the one line, plus the BOM regression test — which fails on current main and passes with the fix. Footgun-linter clean (the flagged sites in test_auth_commands.py are pre-existing on main, 37 there with or without this change; the new test writes the BOM fixture via write_bytes).

[GottZ]

This was generated by AI during triage.

Graph note (no action implied — a maintainer has already reviewed this thread).

Our triage graph places this PR in a complex with 1 related pull request (#62617). They were checked against each other at the diff level and no consolidation is indicated — they address distinct causes.

Full neighbourhood: https://hermes-triage.gottz.de/?node=60895

This note exists so the relationship stays discoverable from the thread itself.

[teknium1]

Merged in #81967 with authorship (dotenv-vs-shell detector utf-8-sig; test regrafted). Thanks!