fix(tools): fall back to .hermes/.env when forwarded secret is empty by SiTaggart · Pull Request #35583 · NousResearch/hermes-agent

SiTaggart · 2026-05-30T23:31:53Z

N.B. Hermes did all the invetigation and work, I'm no python dev

Expected behaviour: When a forwarded secret is momentarily empty ("") in the gateway's live env, fall back to /.hermes/.env (as already happens when unset) and forward the correct value. A transient blank must never override a good on-disk secret, and an empty value must never be forwarded as -e KEY=.

Actual behaviour: The disk fallback only triggers on value is None, not value == "". An empty string skips the fallback and is forwarded as -e KEY=. Sandboxed workloads read a zero-length secret and fail auth while the correct value sits in /.hermes/.env. Observed as intermittent API 401s (a forwarded MY_SECRET arriving with length: 0), with no gateway restart and no .env rewrite. Hit twice in production 9 days apart. It's invisible for gateway-side secrets (Slack/Telegram) because those never ride a docker exec -e into a container; only sandbox-consumed secrets surface it.

Repro:

def build_args(forward_keys, live_env, disk_env):
    exec_env, hermes_env = {}, (disk_env if forward_keys else {})
    for key in sorted(forward_keys):
        value = live_env.get(key, None)          # os.getenv(key)
        if value is None:                        # only unset
            value = hermes_env.get(key)
        if value is not None:                    # forwards ""
            exec_env[key] = value
    return [x for k in sorted(exec_env) for x in ("-e", f"{k}={exec_env[k]}")]

args = build_args({"MY_SECRET"}, {"MY_SECRET": ""}, {"MY_SECRET": "x" * 48})
flag = next(a for a in args if a.startswith("MY_SECRET="))
print(flag, "-> length:", len(flag.split("=", 1)[1]))
# Actual:   MY_SECRET= -> length: 0   (disk fallback skipped)
# Expected: MY_SECRET=xxxx... -> length: 48

Fix: treat empty-string like unset (if not value: on the fallback) and never forward a blank (if value: on the guard). Robust to whatever produces the transient blank (env mutation during concurrent spawns, partial reload). For strict back-compat, the minimal change is just the fallback line.

         for key in sorted(forward_keys):
             value = os.getenv(key)
-            if value is None:
+            if not value:
                 value = hermes_env.get(key)
-            if value is not None:
+            if value:
                 exec_env[key] = value

Tests: two regression tests added to tests/tools/test_docker_environment.py, mocking os.getenv (via monkeypatch.setenv) and the hermes-env loader (_load_hermes_env_vars), no real files touched:

test_init_env_args_uses_hermes_dotenv_for_empty_shell_env — live env has MY_SECRET="", disk has the real value; asserts the container receives the disk value and no blank flag. Asserts on the resolved value in the args list, not the printed flag.
test_init_env_args_never_forwards_blank_secret — legitimately empty + no disk value; asserts the key is not forwarded as -e MY_SECRET= at all.

Verified RED on old code (both tests fail; old loop emits ['-e', 'MY_SECRET=']) and GREEN on the fix. Full file passes via the canonical runner:

scripts/run_tests.sh tests/tools/test_docker_environment.py
=== Summary: 1 files, 56 tests passed, 0 failed ===

Environment: Observed in production on Ubuntu (AWS EC2 VPS), Python 3.11. Fix authored and tested on macOS 15.5 (arm64, Darwin 25.5.0), Python 3.11.9, Hermes Agent v0.15.1 (2026.5.29).

Fixes #35580

The docker_forward_env build loop only consulted the ~/.hermes/.env disk fallback when a key was unset (value is None), not when it was present but empty (""). A transient empty value in os.environ was therefore forwarded into the sandbox container as `-e KEY=`, clobbering the correct value on disk. Sandboxed workloads then read a zero-length secret and failed auth (observed as intermittent Linear API 401s) with no gateway restart and no .env rewrite. Treat empty-string like unset (`if not value:` on the fallback) and never forward a blank secret (`if value:` on the guard). Fixes NousResearch#35580

liuhao1024 · 2026-05-31T00:42:59Z

Verified this fix is correct.

The bug: _build_init_env_args() in tools/environments/docker.py used if value is None to decide whether to fall back to .hermes/.env. An empty-string env var (MY_SECRET="") passed the is None check, so the empty value was forwarded as -e MY_SECRET=, clobbering the correct value from .hermes/.env.

The fix: if value is None → if not value makes both None (unset) and "" (empty) fall back to the disk .env. The second guard if value is not None → if value ensures empty strings are never forwarded as -e KEY= flags either.

Scope check: the change is in the single function that builds Docker -e flags. No other callers of _load_hermes_env_vars() are affected. The two new regression tests cover both paths:

Empty shell env + present .env value → .env wins
Empty shell env + no .env value → key omitted entirely

Edge case note: if not value also treats "0" and "false" as falsy. For secrets this is fine (neither is a meaningful secret value), but worth noting for non-secret env vars if this code path ever broadens.

tonydwb

Code Review Summary

Verdict: Approved ✅

Changes

tools/environments/docker.py: if value is None: → if not value: (fallback) and if value is not None: → if value: (forward guard)
tests/tools/test_docker_environment.py: 2 regression tests

Review

✅ Correctness

Two-character change with clear reasoning: a transient empty string ("") in the live env was treated as "present but empty" by value is None (which only catches unset keys), skipping the .hermes/.env fallback and forwarding `-e KEY=***
Fix 1 (if not value: on fallback): treats None, "", and falsy values the same — falls back to disk
Fix 2 (if value: on forward guard): prevents `-e KEY=*** from ever being forwarded when the resolved value is empty/None
Both changes are conservative — value == "" and value == None are the only cases where behavior changes, and both are correct

✅ Testing

test_init_env_args_uses_hermes_dotenv_for_empty_shell_env: live env has empty string, disk has the real value — asserts disk value wins and blank flag is not emitted
test_init_env_args_never_forwards_blank_secret: legitimately empty + no disk value — asserts the key is absent from args entirely
Both tests verified RED on old code, GREEN on fix (TDD style)
Full file passes: 56 tests passed, 0 failed

✅ Code Quality

Minimal change — 2 lines in production code, 44 lines of test code
Detailed PR description with reproduction script and root cause analysis
The bug report format is excellent (expected behavior, actual behavior, repro, environment)

Summary

Correct and minimal fix for an intermittent production issue. A transient empty string in the live env could override a valid .hermes/.env secret. Fix is conservative and well-tested.

Reviewed by Hermes Agent (cron job)

tonydwb

Code Review Summary

Verdict: Approved ✅

Review

A tight, well-documented fix for a subtle production bug where an empty-string env var would skip the .env fallback and be forwarded as -e KEY= to Docker containers, causing intermittent auth failures.

✅ Looks Good

Minimal, correct fix: if not value: instead of if value is None: — 2 lines changed, maximum impact.
Both guards fixed: The empty-string detection and the forward guard both use truthiness, so blank values are never forwarded.
Excellent PR description: Includes repro script, root cause analysis, and RED/GREEN test evidence.
Good test coverage: Two regression tests with mocked env and disk values, covering both the empty-with-fallback and empty-without-fallback paths.
No regression risk: The only behavior change is treating "" the same as None, which is always the correct thing for secrets.

Checklist Summary

Category	Status
Correctness	✅ 2-line fix with precise semantics
Security	✅ Prevents empty secrets from being forwarded
Code Quality	✅ Clean conditionals, good commit message
Testing	✅ 2 regression tests, RED/GREEN verified

Reviewed by Hermes Agent (cron job)

Adds me@simontaggart.com → SiTaggart to AUTHOR_MAP so the check-attribution gate passes for the docker_forward_env empty-secret fix (#35583, fixes #35580).

…arch#36189) Adds me@simontaggart.com → SiTaggart to AUTHOR_MAP so the check-attribution gate passes for the docker_forward_env empty-secret fix (NousResearch#35583, fixes NousResearch#35580).

…ousResearch#35583) The docker_forward_env build loop only consulted the ~/.hermes/.env disk fallback when a key was unset (value is None), not when it was present but empty (""). A transient empty value in os.environ was therefore forwarded into the sandbox container as `-e KEY=`, clobbering the correct value on disk. Sandboxed workloads then read a zero-length secret and failed auth (observed as intermittent Linear API 401s) with no gateway restart and no .env rewrite. Treat empty-string like unset (`if not value:` on the fallback) and never forward a blank secret (`if value:` on the guard). Fixes NousResearch#35580

alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround backend/docker Docker container execution area/auth Authentication, OAuth, credential pools labels May 30, 2026

tonydwb approved these changes May 31, 2026

View reviewed changes

alt-glitch mentioned this pull request May 31, 2026

fix(docker): ignore empty forwarded env values #36008

Closed

benbarclay mentioned this pull request Jun 1, 2026

chore(attribution): map SiTaggart for PR #35583 #36189

Merged

benbarclay merged commit a75a454 into NousResearch:main Jun 1, 2026
22 of 25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tools): fall back to .hermes/.env when forwarded secret is empty#35583

fix(tools): fall back to .hermes/.env when forwarded secret is empty#35583
benbarclay merged 1 commit into
NousResearch:mainfrom
SiTaggart:fix/docker-forward-env-empty-string

SiTaggart commented May 30, 2026 •

edited

Loading

Uh oh!

liuhao1024 commented May 31, 2026

Uh oh!

tonydwb left a comment

Uh oh!

tonydwb left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

SiTaggart commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liuhao1024 commented May 31, 2026

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Code Review Summary

Changes

Review

Summary

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Code Review Summary

Review

✅ Looks Good

Checklist Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SiTaggart commented May 30, 2026 •

edited

Loading