fix(memory-setup): preserve explicit env home - #1
Open
rohitb4488 wants to merge 2 commits into
Open
Conversation
…ion gate hermes_cli/memory_setup.py::_write_env_vars persisted memory-provider plugin credentials to ~/.hermes/.env via Path.write_text directly, bypassing every gate the canonical hermes_cli.config.save_env_value writer enforces: - _ENV_VAR_NAME_DENYLIST was not enforced. A memory-provider plugin schema declaring env_var: "LD_PRELOAD" (or LD_LIBRARY_PATH, PYTHONPATH, NODE_OPTIONS, GIT_SSH_COMMAND, HERMES_HOME, ...) would land in .env verbatim and load via env_loader.py's .env -> os.environ chain on the next Hermes startup — code execution before main(). - _ENV_VAR_NAME_RE was not enforced, so a malformed identifier (e.g. "FOO BAR") was persisted verbatim, producing a broken .env line. - CR/LF in values was not stripped, so a value containing "\n" could split the line and inject an arbitrary follow-on key=value pair. - The file was written via Path.write_text() then chmod'd after the fact, leaving a TOCTOU window at the default umask before the restrictive permissions applied (the same class of bug just closed in the sibling Anthropic-OAuth-token writer). save_env_value already closes all four (atomic write with the temp created at 0o600 up front, the denylist, the name regex, and CR/LF stripping) — every other .env writer in the codebase already goes through it. This routes the memory-setup wizard through it too, per key, so a single denylisted/malformed field from one provider's schema is skipped (logged) without aborting the rest of the batch. Removes the now-unused env_path parameter/local (save_env_value resolves the path itself) and the now-unused `Path` import. Prior art: NousResearch#30281 (atomic write only, narrower, still open/stale) and NousResearch#33741 (the save_env_value-routing approach this PR follows, closed by its own author for lack of review bandwidth, not on the merits). Tests: tests/hermes_cli/test_memory_setup_env_denylist.py (19 cases) — denylisted keys, malformed names, CR/LF stripping, non-denylisted HERMES_* keys still writable, secure-permissions regression guard. Mutation-verified: reverting the fix makes all 19 fail (old signature) against the pre-fix code.
4 tasks
pierrenode
force-pushed
the
fix/memory-setup-env-writer-denylist
branch
3 times, most recently
from
July 29, 2026 14:20
e4ece8a to
52587b3
Compare
pierrenode
force-pushed
the
fix/memory-setup-env-writer-denylist
branch
from
August 11, 2026 16:02
52587b3 to
d0553fb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
This addresses the CI failure on NousResearch#60587: Supermemory still targeted its provided setup home while _write_env_vars had been narrowed to the default Hermes home.