fix(skills): make Nix store skill copies writable - #20135
Closed
haoxianhan wants to merge 1 commit into
Closed
Conversation
alt-glitch
added
type/bug
Bundled skills copied from Nix-managed HERMES_BUNDLED_SKILLS paths inherit read-only modes from /nix/store. shutil.copy2 preserves read-only file modes, and copytree also reapplies directory metadata after copying, so ~/.hermes/skills copies can end up with unwritable files or directories. Ensure copied files and directories gain owner write permission while preserving existing mode bits such as executable scripts. Use the writable tree copy path for both initial bundled skill syncs and bundled skill updates. Add regression coverage for read-only files, executable files, read-only directories, and end-to-end sync from a read-only bundled skill tree.
haoxianhan
force-pushed
the
fix/skills-copy-nix-store-permissions
branch
from
May 13, 2026 01:30
3ce4773 to
1648bfb
Compare
haoxianhan
changed the title
11 tasks
ethernet8023
pushed a commit
that referenced
this pull request
Aug 3, 2026
shutil.copy2 and shutil.copytree both preserve source-file mode bits. When bundled skills are sourced from a read-only filesystem — the Nix store (mode 0444/0555), squashfs, or an OCI image layer — the user copy in ~/.hermes/skills/ inherits those bits. Later edits via skill_manage, the curator, or even the shutil.rmtree(*.bak) cleanup at the end of an update then fail with PermissionError (silently swallowed by ignore_errors=True, so stale *.bak directories accumulate). Three helpers fix the root cause at the copy boundary: - _ensure_owner_writable(path) — adds the owner-write bit on a single file or directory without touching other mode bits (skips symlinks: os.chmod follows them by default, and a copied symlink pointing outside the profile/tree must not have its external target mutated). - _copy_file_writable(src, dst) — drop-in shutil.copy2 replacement, also used as copytree's copy_function. - _copytree_writable(src, dst) — drop-in shutil.copytree replacement; sweeps the destination tree afterwards since copytree reapplies source-directory metadata after file copies. Wired into every copy site that may read from a read-only bundled source: tools/skills_sync.py (sync_skills new + update paths, restore_official_optional_skill, the DESCRIPTION.md copy, and the reset_bundled_skill rmtree), hermes_cli/profiles.py (--clone and --clone-all), and hermes_cli/profile_distribution.py (apply_distribution). Also repairs two states the copy-boundary fix alone doesn't reach: - Migration for existing installs: a user copy that predates this fix can be hash-identical to the bundled source, so sync_skills takes the "unchanged" no-op path and would never repair it. Both the v1-migration branch and the "bundled unchanged, user unchanged" branch now sweep _make_tree_owner_writable(dest) unconditionally. - Symlink safety in the profile-clone repair sweep: profiles.py clones skills with shutil.copytree(..., symlinks=True, ...), so a skill that is itself a symlink to a shared/vendored directory outside the profile reaches the writable-mode repair as a real symlink entry. _ensure_owner_writable skips symlinks rather than chmod-ing through them into whatever they still point at. Salvages closed PR #20135 (closed by author, not maintainer-rejected) and extends its coverage. Complements #34860 (83a7d0b / 8ae0802), which fixed the removal side (_rmtree_writable making read-only trees removable) — this fixes the copy side (preventing read-only trees from being created in the first place).
legacynode
added a commit
to legacycode-forks/hermes-agent
that referenced
this pull request
Aug 17, 2026
shutil.copy2 and shutil.copytree both preserve source-file mode bits. When bundled skills are sourced from a read-only filesystem — the Nix store (mode 0444/0555), squashfs, or an OCI image layer — the user copy in ~/.hermes/skills/ inherits those bits. Later edits via skill_manage, the curator, or even the shutil.rmtree(*.bak) cleanup at the end of an update then fail with PermissionError (silently swallowed by ignore_errors=True, so stale *.bak directories accumulate). Three helpers fix the root cause at the copy boundary: - _ensure_owner_writable(path) — adds the owner-write bit on a single file or directory without touching other mode bits (skips symlinks: os.chmod follows them by default, and a copied symlink pointing outside the profile/tree must not have its external target mutated). - _copy_file_writable(src, dst) — drop-in shutil.copy2 replacement, also used as copytree's copy_function. - _copytree_writable(src, dst) — drop-in shutil.copytree replacement; sweeps the destination tree afterwards since copytree reapplies source-directory metadata after file copies. Wired into every copy site that may read from a read-only bundled source: tools/skills_sync.py (sync_skills new + update paths, restore_official_optional_skill, the DESCRIPTION.md copy, and the reset_bundled_skill rmtree), hermes_cli/profiles.py (--clone and --clone-all), and hermes_cli/profile_distribution.py (apply_distribution). Also repairs two states the copy-boundary fix alone doesn't reach: - Migration for existing installs: a user copy that predates this fix can be hash-identical to the bundled source, so sync_skills takes the "unchanged" no-op path and would never repair it. Both the v1-migration branch and the "bundled unchanged, user unchanged" branch now sweep _make_tree_owner_writable(dest) unconditionally. - Symlink safety in the profile-clone repair sweep: profiles.py clones skills with shutil.copytree(..., symlinks=True, ...), so a skill that is itself a symlink to a shared/vendored directory outside the profile reaches the writable-mode repair as a real symlink entry. _ensure_owner_writable skips symlinks rather than chmod-ing through them into whatever they still point at. Salvages closed PR NousResearch#20135 (closed by author, not maintainer-rejected) and extends its coverage. Complements NousResearch#34860 (83a7d0b / 8ae0802), which fixed the removal side (_rmtree_writable making read-only trees removable) — this fixes the copy side (preventing read-only trees from being created in the first place).
jh1nresh
pushed a commit
to jh1nresh/hermes-agent
that referenced
this pull request
Aug 25, 2026
shutil.copy2 and shutil.copytree both preserve source-file mode bits. When bundled skills are sourced from a read-only filesystem — the Nix store (mode 0444/0555), squashfs, or an OCI image layer — the user copy in ~/.hermes/skills/ inherits those bits. Later edits via skill_manage, the curator, or even the shutil.rmtree(*.bak) cleanup at the end of an update then fail with PermissionError (silently swallowed by ignore_errors=True, so stale *.bak directories accumulate). Three helpers fix the root cause at the copy boundary: - _ensure_owner_writable(path) — adds the owner-write bit on a single file or directory without touching other mode bits (skips symlinks: os.chmod follows them by default, and a copied symlink pointing outside the profile/tree must not have its external target mutated). - _copy_file_writable(src, dst) — drop-in shutil.copy2 replacement, also used as copytree's copy_function. - _copytree_writable(src, dst) — drop-in shutil.copytree replacement; sweeps the destination tree afterwards since copytree reapplies source-directory metadata after file copies. Wired into every copy site that may read from a read-only bundled source: tools/skills_sync.py (sync_skills new + update paths, restore_official_optional_skill, the DESCRIPTION.md copy, and the reset_bundled_skill rmtree), hermes_cli/profiles.py (--clone and --clone-all), and hermes_cli/profile_distribution.py (apply_distribution). Also repairs two states the copy-boundary fix alone doesn't reach: - Migration for existing installs: a user copy that predates this fix can be hash-identical to the bundled source, so sync_skills takes the "unchanged" no-op path and would never repair it. Both the v1-migration branch and the "bundled unchanged, user unchanged" branch now sweep _make_tree_owner_writable(dest) unconditionally. - Symlink safety in the profile-clone repair sweep: profiles.py clones skills with shutil.copytree(..., symlinks=True, ...), so a skill that is itself a symlink to a shared/vendored directory outside the profile reaches the writable-mode repair as a real symlink entry. _ensure_owner_writable skips symlinks rather than chmod-ing through them into whatever they still point at. Salvages closed PR NousResearch#20135 (closed by author, not maintainer-rejected) and extends its coverage. Complements NousResearch#34860 (83a7d0b / 8ae0802), which fixed the removal side (_rmtree_writable making read-only trees removable) — this fixes the copy side (preventing read-only trees from being created in the first place).
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
copytreereapplies source directory metadataRoot cause
Nix store paths are read-only.
shutil.copy2preserves file modes, andshutil.copytreealso reapplies directory metadata after copying. WhenHERMES_BUNDLED_SKILLSpoints at a Nix-managed bundled skills tree, synced copies under~/.hermes/skillscould inherit unwritable files or directories.Validation
python3 -m py_compile tools/skills_sync.py tests/tools/test_skills_sync.py0555directories and executable filesFull pytest was not run in this checkout because no project virtualenv was available.