fix(cli): respect 12-factor env precedence in load_hermes_dotenv()

[liuhao1024]

What does this PR do?

load_hermes_dotenv() used override=True, causing ~/.hermes/.env values to silently override runtime-injected secrets from systemd EnvironmentFile=, Docker secrets, or Kubernetes env injection. This breaks credential rotation — stale keys in .env win over freshly rotated production secrets, causing 401 errors with no obvious cause.

Root Cause

In hermes_cli/env_loader.py:168:

_load_dotenv_with_fallback(user_env, override=True)  # ← the problem

override=True means .env always wins over pre-existing environment variables, violating 12-factor app precedence (env > file).

Related Issue

N/A

Type of Change

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

Changes Made

• See commit messages for detailed changes

How to Test

1. Run pytest tests/ -q — all tests should pass
2. Verify the specific scenario described above is resolved

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

[Tranquil-Flow]

Hi @liuhao1024 — heads-up that this looks like it'll break three existing tests in tests/hermes_cli/test_env_loader.py that were written to assert the old override=True behavior:

• test_user_env_overrides_stale_shell_values (line 9-20): pre-sets OPENAI_BASE_URL in the shell env and asserts the .env value wins.
• test_user_env_takes_precedence_over_project_env (line 55-70, assertion at line 69): same pattern with the user/project env precedence.
• test_main_import_applies_user_env_over_shell_values (line 73-89): imports hermes_cli.main and asserts .env overrides shell vars.

Two ways to handle:

1. Update the assertions to reflect the new precedence (shell wins).
2. Set HERMES_DOTENV_OVERRIDE=1 in each test via monkeypatch.setenv so they exercise the opt-in override path you preserved. The third test would need it set before the hermes_cli.main import.

Option 2 keeps coverage of the override path so a future change can't silently regress it.

[liuhao1024]

Hi @Tranquil-Flow — thanks for the thorough review! I went with Option 2 as you suggested: added HERMES_DOTENV_OVERRIDE=1 via monkeypatch to all three tests so they exercise the opt-in override path.

Changes pushed:

• test_user_env_overrides_stale_shell_values — now sets HERMES_DOTENV_OVERRIDE=1 + added docstring
• test_user_env_takes_precedence_over_project_env — same
• test_main_import_applies_user_env_over_shell_values — same
• New: test_default_does_not_override_shell_values — verifies the new default behavior (shell env wins when HERMES_DOTENV_OVERRIDE is not set)
• New: test_user_env_fills_missing_shell_values — verifies .env fills missing vars without overriding existing ones

All 7 tests pass: 7 passed in 1.38s

[Cyrene963]

+1 on this fix. I independently hit the same issue and submitted #19677 before finding this PR. Closing mine as duplicate.

The HERMES_DOTENV_OVERRIDE escape hatch is a nice touch for backward compatibility — my version was a plain override=False which would break users who rely on the legacy override behavior. This PR is the cleaner approach.

[loganfinney27]

+1. Adds another use case beyond the systemd/docker scenarios already covered: the op run --env-file=~/.hermes/.env -- python -m hermes_cli.main gateway run --replace pattern (with .env holding KEY="op://<vault>/<item>/<field>" references) is broken today specifically because op run exports the resolved plaintext values into os.environ before exec'ing python, and load_hermes_dotenv() then clobbers them back to the literal op://... strings. The HERMES_DOTENV_OVERRIDE=1 opt-in escape hatch in this PR is a clean way to preserve the old behavior for users who want it.

Both #36949 (native 1Password backend feature request) and the op run --env-file workaround pattern unblock when this lands.

Cross-link map for anyone arriving from a related thread:

• [Feature]: load_hermes_dotenv() overrides systemd / docker / runtime-injected env vars with ~/.hermes/.env (override=True) — breaks 12-factor config precedence and creates a credential-rotation footgun #19201, [Feature]: load_hermes_dotenv() uses override=True, breaking 12-factor env precedence and creating a credential-rotation footgun #18705 — the underlying bug (12-factor precedence violation)
• fix(config): respect env vars over .env file for 12-factor precedence #19677 — duplicate PR, closed by its author in favor of this one
• [Feature]: 1Password (op://) secret source backend #36949 — related feature request the override-flip partially supersedes for op:// users

Stack: Hermes 0.14.0, macOS 12.7.6, 1Password CLI 2.34.0 via service-account token, op run --env-file as the bootstrap pattern. Drafted by Claude with the deployment context; verified against the running daemon.

[alt-glitch]

This was generated by AI during triage.

Related: this is the canonical 12-factor .env precedence fix. Underlying bug reports #19201 / #18705; competing PR #19677 was closed by its author in favor of this one; #36949 (native 1Password backend) is partially superseded for op:// users by the HERMES_DOTENV_OVERRIDE opt-in. Holding at P2 — the comment thread confirms the bug (incl. a concrete op run --env-file repro) without arguing a severity change.

[teknium1]

Thank you for the focused precedence tests and for documenting the systemd/Docker and op run cases discussed on this PR.

This automated hermes-sweeper review is closing this under the standing configuration policy:

• The PR introduces HERMES_DOTENV_OVERRIDE=1 as a user-facing, non-secret behavioral feature flag in hermes_cli/env_loader.py.
• AGENTS.md:102-107 requires behavioral settings and feature flags to use config.yaml; .env is reserved for secrets.
• Current .env-over-shell behavior is intentional existing behavior: hermes_cli/env_loader.py:246 uses override=True, introduced by f24c00a5bf8845fd07e059cce3ded7056af9fec2 specifically to make restarted .env model/provider/base-URL changes take effect.

If maintainers choose to make dotenv precedence configurable, please re-scope that behavior through the supported config.yaml configuration surface rather than a new HERMES_* flag.

---

Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.