diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f87f31d..3525c2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Status of the `main` branch. Changes prior to the next official version change w * General: - Fix: Race conditions in ProjectServer when used by multiple clients in parallel + - Fix: `GitignoreParser` interpolated a directory's name unescaped into gitignore pattern position; + a directory named with pattern metacharacters (e.g. a stray `***`) could turn a scoped pattern + into one matching far more than intended, silently excluding most or all of the project from + indexing #1806 + - Fix: the README, the Language Support docs page and the project template omitted several already-supported language servers - Fix: a tool call exceeding the timeout blocked the task executor indefinitely; the executor now recovers without user-induced cancellation - Add Grok Build support (context `grok`, setup CLI, hooks) diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index 06cedc8c..fcde0e09 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -1,5 +1,6 @@ import logging import os +import re from collections.abc import Callable, Iterator from dataclasses import dataclass, field from pathlib import Path @@ -11,6 +12,18 @@ from sensai.util.logging import LogTime log = logging.getLogger(__name__) +# Characters meaningful to pathspec's gitignore grammar: glob wildcards, bracket expressions, +# the escape character itself, and '!'/'#' which change a whole pattern's meaning when they +# are its first character. Backslash-escaping them makes a literal name safe to interpolate. +_GITIGNORE_PATTERN_SPECIAL_CHARS_RE = re.compile(r"([\\*?\[\]!#])") + + +def _escape_gitignore_path_component(component: str) -> str: + """Escape gitignore/pathspec pattern metacharacters in a single path component (no + separators) so it is matched as a literal name rather than as glob syntax. + """ + return _GITIGNORE_PATTERN_SPECIAL_CHARS_RE.sub(r"\\\1", component) + class ScanResult(NamedTuple): """Result of scanning a directory.""" @@ -223,11 +236,21 @@ class GitignoreParser: """ patterns = [] - # Get the relative path from repo root to the gitignore directory - rel_dir = os.path.relpath(gitignore_dir, self.repo_root) + # Get the relative path from repo root to the gitignore directory. Normalize to + # forward slashes immediately: os.path.relpath returns native separators, but + # gitignore/pathspec patterns are always POSIX-style, and on Windows os.sep is + # backslash -- the same character pathspec uses as its escape character. Building + # the pattern with any raw os.sep would make a later blanket backslash->slash + # normalization indistinguishable from the escape backslashes below. + rel_dir = os.path.relpath(gitignore_dir, self.repo_root).replace(os.sep, "/") if rel_dir == ".": rel_dir = "" + # rel_dir is a filesystem path, but the code below interpolates it into pattern + # position; escape each of its components so a directory name containing pattern + # metacharacters (e.g. "***") is matched literally instead of as glob syntax. + rel_dir_pattern = "/".join(_escape_gitignore_path_component(part) for part in rel_dir.split("/")) if rel_dir else rel_dir + for line in content.splitlines(): # Strip trailing whitespace (but preserve leading whitespace for now) line = line.rstrip() @@ -256,20 +279,23 @@ class GitignoreParser: if is_anchored: line = line[1:] - # Adjust pattern based on gitignore file location + # Adjust pattern based on gitignore file location. Joined with a literal "/", + # never os.path.join/os.sep: gitignore patterns are always POSIX-style, and on + # Windows os.sep is backslash, indistinguishable from the escape backslashes + # rel_dir_pattern may already contain. if rel_dir: if is_anchored: # Anchored patterns are relative to the gitignore directory - adjusted_pattern = os.path.join(rel_dir, line) + adjusted_pattern = f"{rel_dir_pattern}/{line}" else: # Non-anchored patterns can match anywhere below the gitignore directory # We need to preserve this behavior if line.startswith("**/"): # Even if pattern starts with **, it should still be scoped to the subdirectory - adjusted_pattern = os.path.join(rel_dir, line) + adjusted_pattern = f"{rel_dir_pattern}/{line}" else: # Add the directory prefix but also allow matching in subdirectories - adjusted_pattern = os.path.join(rel_dir, "**", line) + adjusted_pattern = f"{rel_dir_pattern}/**/{line}" else: if is_anchored: # Anchored patterns in root should only match at root level @@ -283,9 +309,6 @@ class GitignoreParser: if is_negation: adjusted_pattern = "!" + adjusted_pattern - # Normalize path separators to forward slashes (gitignore uses forward slashes) - adjusted_pattern = adjusted_pattern.replace(os.sep, "/") - patterns.append(adjusted_pattern) return patterns diff --git a/test/serena/util/test_file_system.py b/test/serena/util/test_file_system.py index ca8a0872..1e1272b9 100644 --- a/test/serena/util/test_file_system.py +++ b/test/serena/util/test_file_system.py @@ -1,11 +1,13 @@ import os import shutil +import sys import tempfile from pathlib import Path +import pytest from pathspec import PathSpec -from serena.util.file_system import GitignoreParser, GitignoreSpec, match_path +from serena.util.file_system import GitignoreParser, GitignoreSpec, _escape_gitignore_path_component, match_path class TestGitignoreParser: @@ -715,6 +717,81 @@ src/*.o # foo.txt in other/ should NOT be ignored (outside foo/ subtree) assert not parser.should_ignore("other/foo.txt"), "other/foo.txt should NOT be ignored by foo/.gitignore" + @pytest.mark.skipif(sys.platform == "win32", reason="'*' is illegal in Windows filenames; this directory name cannot exist there") + def test_gitignore_dir_name_with_glob_metachars_is_not_a_wildcard(self): + """A directory named with glob metacharacters (e.g. a stray '***' venv) must be + matched literally, not interpreted as a pattern (issue #1806). + """ + test_dir = self.repo_path / "test_metachar_dirname" + test_dir.mkdir() + (test_dir / "pkg").mkdir() + (test_dir / "***").mkdir() + + (test_dir / "pkg" / "mod.py").touch() + (test_dir / "***" / "junk.txt").touch() + + gitignore = test_dir / "***" / ".gitignore" + gitignore.write_text("*\n") + + parser = GitignoreParser(str(test_dir)) + + # pkg/mod.py is outside the "***" directory and must not be affected by its gitignore + assert not parser.should_ignore("pkg/mod.py"), "pkg/mod.py should not be ignored by ***/.gitignore" + + # junk.txt inside "***" is still ignored by its own gitignore's "*" pattern + assert parser.should_ignore("***/junk.txt"), "***/junk.txt should be ignored by its own gitignore" + + def test_gitignore_dir_name_with_metachars_anchored_pattern(self): + """Same as above, for the anchored-pattern join site.""" + test_dir = self.repo_path / "test_metachar_dirname_anchored" + test_dir.mkdir() + (test_dir / "pkg").mkdir() + (test_dir / "a[1]").mkdir() + + (test_dir / "pkg" / "mod.py").touch() + (test_dir / "a[1]" / "mod.py").touch() + + gitignore = test_dir / "a[1]" / ".gitignore" + gitignore.write_text("/mod.py\n") + + parser = GitignoreParser(str(test_dir)) + + assert not parser.should_ignore("pkg/mod.py"), "pkg/mod.py should not be ignored by a[1]/.gitignore" + assert parser.should_ignore("a[1]/mod.py"), "a[1]/mod.py should be ignored by its own /mod.py pattern" + + @pytest.mark.skipif(sys.platform == "win32", reason="'?' is illegal in Windows filenames; this directory name cannot exist there") + def test_gitignore_dir_name_with_metachars_implicit_double_star_pattern(self): + """A non-anchored pattern with no leading '**/' is joined as (rel_dir, "**", line): + the third join site. An unescaped '?' in the directory name would leak the pattern + into a sibling directory whose name merely matches the wildcard (issue #1806). + """ + test_dir = self.repo_path / "test_metachar_dirname_implicit_doublestar" + test_dir.mkdir() + (test_dir / "q?").mkdir() + (test_dir / "qA").mkdir() + (test_dir / "qA" / "sub").mkdir() + + (test_dir / "qA" / "sub" / "mod.py").touch() + + gitignore = test_dir / "q?" / ".gitignore" + gitignore.write_text("mod.py\n") + + parser = GitignoreParser(str(test_dir)) + + # "q?/.gitignore" must not reach into the sibling "qA/" directory just because "?" + # would match the "A" in "qA" if left as a wildcard. + assert not parser.should_ignore("qA/sub/mod.py"), "qA/sub/mod.py should not be ignored by q?/.gitignore" + + def test_escape_gitignore_path_component_escapes_all_metachars(self): + """Pure-function coverage for the '*' and '?' escaping cases the two directory-creation + tests above cannot exercise on Windows (those characters are illegal in Windows + filenames, so this runs on every platform instead of touching the filesystem). + """ + assert _escape_gitignore_path_component("***") == "\\*\\*\\*" + assert _escape_gitignore_path_component("q?") == "q\\?" + assert _escape_gitignore_path_component("a[1]") == "a\\[1\\]" + assert _escape_gitignore_path_component("plain") == "plain" + class TestGitignoreParserPermissionError: """Test PermissionError handling in GitignoreParser."""