From 12bc86d9c92e602ded6f81fa34d7deb6175e5896 Mon Sep 17 00:00:00 2001 From: Sebastion Date: Sun, 15 Mar 2026 01:18:45 +0000 Subject: [PATCH 1/2] fix: prevent path traversal in .worktreeinclude file processing Resolve .worktreeinclude entries and validate that both the source path stays within the repository root and the destination path stays within the worktree directory before copying files or creating symlinks. A malicious .worktreeinclude in a cloned repository could previously reference paths like "../../etc/passwd" to copy or symlink arbitrary files from outside the repo into the worktree. CWE-22: Improper Limitation of a Pathname to a Restricted Directory --- cli.py | 18 ++++++++++- tests/test_worktree.py | 72 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/cli.py b/cli.py index 70a202d3..4f734fad 100755 --- a/cli.py +++ b/cli.py @@ -571,12 +571,28 @@ def _setup_worktree(repo_root: str = None) -> Optional[Dict[str, str]]: include_file = Path(repo_root) / ".worktreeinclude" if include_file.exists(): try: + repo_root_resolved = Path(repo_root).resolve() + wt_path_resolved = wt_path.resolve() for line in include_file.read_text().splitlines(): entry = line.strip() if not entry or entry.startswith("#"): continue src = Path(repo_root) / entry dst = wt_path / entry + # Prevent path traversal: ensure src stays within repo_root + # and dst stays within the worktree directory + try: + src_resolved = src.resolve() + dst_resolved = dst.resolve(strict=False) + except (OSError, ValueError): + logger.debug("Skipping invalid .worktreeinclude entry: %s", entry) + continue + if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: + logger.warning("Skipping .worktreeinclude entry outside repo root: %s", entry) + continue + if not str(dst_resolved).startswith(str(wt_path_resolved) + os.sep) and dst_resolved != wt_path_resolved: + logger.warning("Skipping .worktreeinclude entry that escapes worktree: %s", entry) + continue if src.is_file(): dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(str(src), str(dst)) @@ -584,7 +600,7 @@ def _setup_worktree(repo_root: str = None) -> Optional[Dict[str, str]]: # Symlink directories (faster, saves disk) if not dst.exists(): dst.parent.mkdir(parents=True, exist_ok=True) - os.symlink(str(src.resolve()), str(dst)) + os.symlink(str(src_resolved), str(dst)) except Exception as e: logger.debug("Error copying .worktreeinclude entries: %s", e) diff --git a/tests/test_worktree.py b/tests/test_worktree.py index f545baa3..dd24381e 100644 --- a/tests/test_worktree.py +++ b/tests/test_worktree.py @@ -633,3 +633,75 @@ class TestSystemPromptInjection: assert info["repo_root"] in wt_note assert "isolated git worktree" in wt_note assert "commit and push" in wt_note + + +class TestWorktreeIncludePathTraversal: + """Test that .worktreeinclude entries with path traversal are rejected.""" + + def test_rejects_parent_directory_traversal(self, git_repo): + """Entries like '../../etc/passwd' must not escape the repo root.""" + import shutil as _shutil + + # Create a sensitive file outside the repo to simulate the attack + outside_file = git_repo.parent / "sensitive.txt" + outside_file.write_text("SENSITIVE DATA") + + # Create a .worktreeinclude with a traversal entry + (git_repo / ".worktreeinclude").write_text("../sensitive.txt\n") + + info = _setup_worktree(str(git_repo)) + assert info is not None + + wt_path = Path(info["path"]) + + # Replay the fixed logic from cli.py + repo_root_resolved = Path(str(git_repo)).resolve() + wt_path_resolved = wt_path.resolve() + include_file = git_repo / ".worktreeinclude" + + copied_entries = [] + for line in include_file.read_text().splitlines(): + entry = line.strip() + if not entry or entry.startswith("#"): + continue + src = Path(str(git_repo)) / entry + dst = wt_path / entry + try: + src_resolved = src.resolve() + dst_resolved = dst.resolve(strict=False) + except (OSError, ValueError): + continue + if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: + continue + if not str(dst_resolved).startswith(str(wt_path_resolved) + os.sep) and dst_resolved != wt_path_resolved: + continue + copied_entries.append(entry) + + # The traversal entry must have been skipped + assert len(copied_entries) == 0 + # The sensitive file must NOT be in the worktree + assert not (wt_path / "../sensitive.txt").resolve().is_relative_to(wt_path_resolved) + + def test_allows_valid_entries(self, git_repo): + """Normal entries within the repo should still be processed.""" + (git_repo / ".env").write_text("KEY=val") + (git_repo / ".worktreeinclude").write_text(".env\n") + + info = _setup_worktree(str(git_repo)) + assert info is not None + + repo_root_resolved = Path(str(git_repo)).resolve() + include_file = git_repo / ".worktreeinclude" + + accepted = [] + for line in include_file.read_text().splitlines(): + entry = line.strip() + if not entry or entry.startswith("#"): + continue + src = Path(str(git_repo)) / entry + src_resolved = src.resolve() + if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: + continue + accepted.append(entry) + + assert ".env" in accepted From f4c012873c7205cb28f959f1524fdcaa17eb5cee Mon Sep 17 00:00:00 2001 From: teknium1 Date: Sat, 14 Mar 2026 21:51:27 -0700 Subject: [PATCH 2/2] fix: harden salvaged worktree include checks Use Path.relative_to-based containment checks for the salvaged .worktreeinclude guard, remove the replayed test logic from the cherry-picked PR, and add real integration regressions for file, directory, and symlink escapes. --- cli.py | 20 +++-- tests/test_worktree.py | 72 ------------------ tests/test_worktree_security.py | 130 ++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 77 deletions(-) create mode 100644 tests/test_worktree_security.py diff --git a/cli.py b/cli.py index 4f734fad..4e55ebbc 100755 --- a/cli.py +++ b/cli.py @@ -518,6 +518,15 @@ def _git_repo_root() -> Optional[str]: return None +def _path_is_within_root(path: Path, root: Path) -> bool: + """Return True when a resolved path stays within the expected root.""" + try: + path.relative_to(root) + return True + except ValueError: + return False + + def _setup_worktree(repo_root: str = None) -> Optional[Dict[str, str]]: """Create an isolated git worktree for this CLI session. @@ -579,18 +588,19 @@ def _setup_worktree(repo_root: str = None) -> Optional[Dict[str, str]]: continue src = Path(repo_root) / entry dst = wt_path / entry - # Prevent path traversal: ensure src stays within repo_root - # and dst stays within the worktree directory + # Prevent path traversal and symlink escapes: both the resolved + # source and the resolved destination must stay inside their + # expected roots before any file or symlink operation happens. try: - src_resolved = src.resolve() + src_resolved = src.resolve(strict=False) dst_resolved = dst.resolve(strict=False) except (OSError, ValueError): logger.debug("Skipping invalid .worktreeinclude entry: %s", entry) continue - if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: + if not _path_is_within_root(src_resolved, repo_root_resolved): logger.warning("Skipping .worktreeinclude entry outside repo root: %s", entry) continue - if not str(dst_resolved).startswith(str(wt_path_resolved) + os.sep) and dst_resolved != wt_path_resolved: + if not _path_is_within_root(dst_resolved, wt_path_resolved): logger.warning("Skipping .worktreeinclude entry that escapes worktree: %s", entry) continue if src.is_file(): diff --git a/tests/test_worktree.py b/tests/test_worktree.py index dd24381e..f545baa3 100644 --- a/tests/test_worktree.py +++ b/tests/test_worktree.py @@ -633,75 +633,3 @@ class TestSystemPromptInjection: assert info["repo_root"] in wt_note assert "isolated git worktree" in wt_note assert "commit and push" in wt_note - - -class TestWorktreeIncludePathTraversal: - """Test that .worktreeinclude entries with path traversal are rejected.""" - - def test_rejects_parent_directory_traversal(self, git_repo): - """Entries like '../../etc/passwd' must not escape the repo root.""" - import shutil as _shutil - - # Create a sensitive file outside the repo to simulate the attack - outside_file = git_repo.parent / "sensitive.txt" - outside_file.write_text("SENSITIVE DATA") - - # Create a .worktreeinclude with a traversal entry - (git_repo / ".worktreeinclude").write_text("../sensitive.txt\n") - - info = _setup_worktree(str(git_repo)) - assert info is not None - - wt_path = Path(info["path"]) - - # Replay the fixed logic from cli.py - repo_root_resolved = Path(str(git_repo)).resolve() - wt_path_resolved = wt_path.resolve() - include_file = git_repo / ".worktreeinclude" - - copied_entries = [] - for line in include_file.read_text().splitlines(): - entry = line.strip() - if not entry or entry.startswith("#"): - continue - src = Path(str(git_repo)) / entry - dst = wt_path / entry - try: - src_resolved = src.resolve() - dst_resolved = dst.resolve(strict=False) - except (OSError, ValueError): - continue - if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: - continue - if not str(dst_resolved).startswith(str(wt_path_resolved) + os.sep) and dst_resolved != wt_path_resolved: - continue - copied_entries.append(entry) - - # The traversal entry must have been skipped - assert len(copied_entries) == 0 - # The sensitive file must NOT be in the worktree - assert not (wt_path / "../sensitive.txt").resolve().is_relative_to(wt_path_resolved) - - def test_allows_valid_entries(self, git_repo): - """Normal entries within the repo should still be processed.""" - (git_repo / ".env").write_text("KEY=val") - (git_repo / ".worktreeinclude").write_text(".env\n") - - info = _setup_worktree(str(git_repo)) - assert info is not None - - repo_root_resolved = Path(str(git_repo)).resolve() - include_file = git_repo / ".worktreeinclude" - - accepted = [] - for line in include_file.read_text().splitlines(): - entry = line.strip() - if not entry or entry.startswith("#"): - continue - src = Path(str(git_repo)) / entry - src_resolved = src.resolve() - if not str(src_resolved).startswith(str(repo_root_resolved) + os.sep) and src_resolved != repo_root_resolved: - continue - accepted.append(entry) - - assert ".env" in accepted diff --git a/tests/test_worktree_security.py b/tests/test_worktree_security.py new file mode 100644 index 00000000..73a242e0 --- /dev/null +++ b/tests/test_worktree_security.py @@ -0,0 +1,130 @@ +"""Security-focused integration tests for CLI worktree setup.""" + +import subprocess +from pathlib import Path + +import pytest + + +@pytest.fixture +def git_repo(tmp_path): + """Create a temporary git repo for testing real cli._setup_worktree behavior.""" + repo = tmp_path / "test-repo" + repo.mkdir() + subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "config", "user.email", "test@test.com"], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "config", "user.name", "Test"], cwd=repo, check=True, capture_output=True) + (repo / "README.md").write_text("# Test Repo\n") + subprocess.run(["git", "add", "."], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "Initial commit"], cwd=repo, check=True, capture_output=True) + return repo + + +def _force_remove_worktree(info: dict | None) -> None: + if not info: + return + subprocess.run( + ["git", "worktree", "remove", info["path"], "--force"], + cwd=info["repo_root"], + capture_output=True, + check=False, + ) + subprocess.run( + ["git", "branch", "-D", info["branch"]], + cwd=info["repo_root"], + capture_output=True, + check=False, + ) + + +class TestWorktreeIncludeSecurity: + def test_rejects_parent_directory_file_traversal(self, git_repo): + import cli as cli_mod + + outside_file = git_repo.parent / "sensitive.txt" + outside_file.write_text("SENSITIVE DATA") + (git_repo / ".worktreeinclude").write_text("../sensitive.txt\n") + + info = None + try: + info = cli_mod._setup_worktree(str(git_repo)) + assert info is not None + + wt_path = Path(info["path"]) + assert not (wt_path.parent / "sensitive.txt").exists() + assert not (wt_path / "../sensitive.txt").resolve().exists() + finally: + _force_remove_worktree(info) + + def test_rejects_parent_directory_directory_traversal(self, git_repo): + import cli as cli_mod + + outside_dir = git_repo.parent / "outside-dir" + outside_dir.mkdir() + (outside_dir / "secret.txt").write_text("SENSITIVE DIR DATA") + (git_repo / ".worktreeinclude").write_text("../outside-dir\n") + + info = None + try: + info = cli_mod._setup_worktree(str(git_repo)) + assert info is not None + + wt_path = Path(info["path"]) + escaped_dir = wt_path.parent / "outside-dir" + assert not escaped_dir.exists() + assert not escaped_dir.is_symlink() + finally: + _force_remove_worktree(info) + + def test_rejects_symlink_that_resolves_outside_repo(self, git_repo): + import cli as cli_mod + + outside_file = git_repo.parent / "linked-secret.txt" + outside_file.write_text("LINKED SECRET") + (git_repo / "leak.txt").symlink_to(outside_file) + (git_repo / ".worktreeinclude").write_text("leak.txt\n") + + info = None + try: + info = cli_mod._setup_worktree(str(git_repo)) + assert info is not None + + assert not (Path(info["path"]) / "leak.txt").exists() + finally: + _force_remove_worktree(info) + + def test_allows_valid_file_include(self, git_repo): + import cli as cli_mod + + (git_repo / ".env").write_text("SECRET=***\n") + (git_repo / ".worktreeinclude").write_text(".env\n") + + info = None + try: + info = cli_mod._setup_worktree(str(git_repo)) + assert info is not None + + copied = Path(info["path"]) / ".env" + assert copied.exists() + assert copied.read_text() == "SECRET=***\n" + finally: + _force_remove_worktree(info) + + def test_allows_valid_directory_include(self, git_repo): + import cli as cli_mod + + assets_dir = git_repo / ".venv" / "lib" + assets_dir.mkdir(parents=True) + (assets_dir / "marker.txt").write_text("venv marker") + (git_repo / ".worktreeinclude").write_text(".venv\n") + + info = None + try: + info = cli_mod._setup_worktree(str(git_repo)) + assert info is not None + + linked_dir = Path(info["path"]) / ".venv" + assert linked_dir.is_symlink() + assert (linked_dir / "lib" / "marker.txt").read_text() == "venv marker" + finally: + _force_remove_worktree(info)