fix: prevent path traversal via .worktreeinclude (CWE-22)#2900
Open
sebastiondev wants to merge 2 commits intoNousResearch:mainfrom
Open
fix: prevent path traversal via .worktreeinclude (CWE-22)#2900sebastiondev wants to merge 2 commits intoNousResearch:mainfrom
sebastiondev wants to merge 2 commits intoNousResearch:mainfrom
Conversation
Entries in .worktreeinclude are now resolved and validated to ensure both the source path stays within the repo root and the destination stays within the worktree directory. Malicious entries containing "../" components that would resolve outside these boundaries are logged and skipped. This prevents a cloned repository from using .worktreeinclude to read arbitrary files from the host filesystem or create symlinks to sensitive paths when the --worktree flag is used. Adds regression tests in test_worktreeinclude_path_traversal.py.
…tree These tests import the actual _setup_worktree from cli.py (not a local re-implementation) and verify that path-traversal entries in .worktreeinclude are rejected while legitimate entries still work. Test cases: - Parent directory traversal (../secret.txt) - Absolute path entries - Double-dot chain (subdir/../../outside) - Symlink-based traversal - Valid file still copied - Valid directory still symlinked - Mixed valid + malicious entries
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.
Vulnerability Summary
CWE-22 — Improper Limitation of a Pathname to a Restricted Directory ("Path Traversal")
Severity: Low-to-Medium (defense-in-depth hardening)
Data Flow
.worktreeincludefile (supply-chain attack vector).hermes -w(worktree mode) or hasworktree: truein config._setup_worktree()is called atcli.py:5106— it reads.worktreeincludeline-by-line.main, each entry is used directly inPath(repo_root) / entryandwt_path / entrywith zero validation.../../etc/passwdor../../../.ssh/id_rsacausesshutil.copy2oros.symlinkto operate on paths outside the repo root and worktree.Exploitability Analysis
The vulnerability is technically real but practically constrained:
srcanddstuse the identicalentryvalue, creating a coupling constraint. Forsrcto escape the repo root,entrymust contain..sequences — but the same sequences apply todst(which starts 2 levels deeper in.worktrees/hermes-XXXX), so the destination also partially escapes.../../../.ssh/id_rsawith repo at/home/user/repos/project) copies the victim's SSH key to an unexpected local filesystem location — but not inside the git worktree, so it won't be committed/pushed. There is no data exfiltration path without additional attacker capabilities.Preconditions for exploitation:
hermes -w(worktree mode) in that repository.worktreeincludemust be tracked in the repoFix Description
Changed file:
cli.py(13 lines added, 3 lines modified)The fix adds path traversal validation to the
.worktreeincludehandler in_setup_worktree():repo_root_resolvedandwt_path_resolved..resolve()to canonicalize away..sequences and symlinks.srcstays withinrepo_root_resolvedanddststays withinwt_path_resolved. If either escapes, log a warning and skip the entry.srcpath foros.symlink()(minor cleanup — no behavioral change for valid entries).Rationale
tools/skills_hub.py:1058,tools/skills_tool.py:894,tools/skill_manager_tool.py:178, andtools/skills_guard.py:347-351. This fix closes the gap in the.worktreeincludehandler..worktreeincludeentries (relative paths within the repo) continue to work identically.Diff
Test Results
Two test suites were added:
1.
tests/test_worktreeinclude_path_traversal.py(Unit Tests)Re-implements the validation logic from
cli.pyto test edge cases in isolation:../../etc/passwd) are skipped.env,config/local.json) are copied normally../../../../../../../etc/shadow) is blocked2.
tests/test_worktreeinclude_cwe22_integration.py(Integration Tests)Calls the real
cli._setup_worktree()function against temporary git repos:Disprove Analysis
We attempted to disprove this finding through 8 independent checks:
.worktreeincludeprocessing happens in Python before any container setup._setup_worktree()called exactly once atcli.py:5106when worktree mode is enabled. Entries come directly from the cloned repo — attacker-controlled in supply-chain scenario.main: ZERO validation on.worktreeincludeentries. No sanitize, validate, escape, clean, whitelist, or allowlist..worktreeincludepath traversal. Several generic security feature requests exist but none for this specific vulnerability.SECURITY.mdfile exists in the repository.Existing Mitigations Found
shutil.copy2resolves both to the same target-wflagtools/file_operations.pywrite deny list — protects.ssh/,.aws/from agent's write tool (but not fromshutil.copy2incli.py)Prior Art
.worktreeincludepattern is conceptually similar to.gitmodulesexploitationVerdict
LIKELY_VALID at medium confidence. The CWE-22 code pattern genuinely exists on
main. Practical exploitation is severely constrained by src/dst coupling, but the fix provides real defense-in-depth value and follows the project's own established security patterns. The change is minimal, non-breaking, and well-tested.