fix: per-profile HOME isolation (#4426)#4685
Closed
kenantan32 wants to merge 2 commits into
Closed
Conversation
In Docker, HERMES_HOME=/opt/data but Path.home() returns /root. The previous code used Path.home() / '.hermes' which would create HOME at /root/.hermes/home/ — outside the persistent volume, defeating the purpose of NousResearch#4426. Now reads os.environ['HERMES_HOME'] when set, falling back to Path.home() / '.hermes' only when HERMES_HOME is not defined. Also removes unused 'import os as _os'.
Contributor
|
Closed in favor of #7357, which takes a different approach: instead of setting Thanks for the thorough implementation and tests — your PR helped define the requirements clearly. |
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
Adds per-profile
HOMEdirectory isolation so system tools (git, ssh, gh, npm, etc.) write configs inside the Hermes data directory instead of/rootor the OS home dir. This fixes both bugs described in #4426:HOME, so credentials don't bleed between profilesChanges
hermes_cli/profiles.py"home"to_PROFILE_DIRS— new profiles get ahome/subdirectory automaticallyget_profile_home_dir(profile_dir)— returns and creates{profile_dir}/home/apply_profile_home_isolation(profile_dir)— setsos.environ["HOME"]to the per-profile home dir. Respects explicitHOME=in.env(user config takes precedence). Silently skips commented lines.hermes_cli/main.pyapply_profile_home_isolation()for both default and named profiles during_apply_profile_override()os.environ["HERMES_HOME"]when set (Docker:/opt/data), falls back toPath.home() / ".hermes"resolve_profile_env()docker/entrypoint.shhome/inside the persistent volume alongside other Hermes directoriestests/hermes_cli/test_profiles.py_PROFILE_DIRSinclusion, env var setting,.envoverride respect, commented line handling, missing.env, legacy profile migration, profile isolationDesign decisions
HOME=is explicitly set in the profile's.env, it takes precedence (backward compat)except Exception: pass)home/get it created on first activation (automatic migration)Known limitation
As noted in the issue: some programs resolve home via
getpwuid(getuid())which always returns/rootfor the root user regardless of$HOME. Full isolation would require per-profile UIDs, which is out of scope here.Tests
All 90 profile tests pass.
Closes #4426