fix(profiles): preserve symlinks when cloning and exporting profiles#11573
Open
mvanhorn wants to merge 1 commit into
Open
fix(profiles): preserve symlinks when cloning and exporting profiles#11573mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
shutil.copytree defaults to following symlinks, so a symlink inside ~/.hermes pointing at a parent directory causes infinite recursion during `hermes profile create <name> --clone-all` (reported traceback bottoms out in shutil._copytree hitting the recursion limit). Pass symlinks=True to all three profile copytree call sites in hermes_cli/profiles.py so symlinks are copied as symlinks rather than dereferenced. This fixes the clone-all crash and preempts the same failure in the export paths for the default and named profiles. Fixes NousResearch#11560
19 tasks
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
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.
What does this PR do?
shutil.copytreeis called in three places inhermes_cli/profiles.pywithoutsymlinks=True, so it follows symlinks instead of copying them as-is. A symlink inside~/.hermesthat points at a parent directory (easy to create by accident - @Opfour hit one while cloning profiles) sendsshutil._copytreeinto infinite recursion andhermes profile create --clone-allcrashes withRecursionError. The export paths would crash the same way if a user ever triggered them on such a profile.Pass
symlinks=Trueto all three call sites.symlinks=Truecopies symlinks verbatim rather than walking their target, which is the right semantic when cloning or exporting a user-owned profile tree. The existingignore=callbacks at the export sites are preserved.Related Issue
Fixes #11560
Type of Change
Changes Made
hermes_cli/profiles.py:437- addsymlinks=Trueto the--clone-allshutil.copytreecall (the crash site named in the issue).hermes_cli/profiles.py:802- addsymlinks=Trueto the default-profile export path. Same latent bug.hermes_cli/profiles.py:815- addsymlinks=Trueto the named-profile export path. Same latent bug. Existingignore=callbacks (credential files,_default_export_ignore) are untouched and still run.tests/hermes_cli/test_profiles.py- addtest_clone_all_preserves_recursive_symlinkusing the existingprofile_envfixture. The test creates a source profile with aloop -> ..symlink, callscreate_profile(clone_all=True), and asserts the call returns, the destination exists, and the symlink survives as a symlink.How to Test
mkdir -p /tmp/h/profiles/src && ln -s .. /tmp/h/profiles/src/loop && HERMES_HOME=/tmp/h hermes profile create dup --clone-all --clone-from src --no-alias->RecursionError: maximum recursion depth exceededfromshutil._copytree.dup/profile contains theloopentry as a symlink (ls -lashowsloop -> ..).pytest tests/hermes_cli/test_profiles.py -v- 88 passed including the new regression test.Demo: https://vhs.charm.sh/vhs-2QQWVciEMvWZk9M4DRyHnF.gif
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/hermes_cli/test_profiles.py -vand all tests pass (88 passed)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/Asymlinks=Trueworks identically on Linux/macOS; on Windows, Python's stdlibshutil.copytreefalls back per documented behavior.This contribution was developed with AI assistance (Codex).