-
Notifications
You must be signed in to change notification settings - Fork 641
fix(scan): preserve tracked files without exposing ignored files #320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
45f56e8
b667798
540bbd4
519f6c9
52b82f9
bb7c5ce
1baf11d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import os | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
|
|
@@ -67,8 +68,23 @@ def resolve_output(value: str) -> Path: | |
|
|
||
|
|
||
| def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: | ||
| """Atomically write the exact ripgrep inventory sorted as ``LC_ALL=C``.""" | ||
| command = ["rg", "--files", "--hidden", "--no-ignore", "--glob", "!.git/**", "--", scope] | ||
| """Atomically inventory visible files and ignored files tracked by Git.""" | ||
| command = [ | ||
| "rg", | ||
| "--no-config", | ||
| "--files", | ||
| "--hidden", | ||
| "--no-require-git", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a non-Git snapshot is located beneath an unrelated parent Useful? React with 👍 / 👎. |
||
| "--no-ignore-parent", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a scoped scan targets a subdirectory, this flag also prevents ripgrep from applying the repository's Useful? React with 👍 / 👎. |
||
| "--no-ignore-global", | ||
|
Comment on lines
+75
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On Unix, when a contributor can commit a symlinked Useful? React with 👍 / 👎. |
||
| "--glob", | ||
| "!.git/**", | ||
| ] | ||
| for name in (".gitignore", ".ignore", ".rgignore"): | ||
| ignore = repository / name | ||
| if ignore.is_file() and not ignore.is_symlink(): | ||
| command.extend(["--ignore-file", str(ignore)]) | ||
|
Comment on lines
+83
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a non-Git snapshot contains a root Useful? React with 👍 / 👎. |
||
| command.extend(["--", scope]) | ||
| with tempfile.TemporaryFile(mode="w+b") as inventory: | ||
| try: | ||
| result = subprocess.run( | ||
|
|
@@ -89,7 +105,138 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: | |
| raise InventoryError(message) | ||
|
|
||
| inventory.seek(0) | ||
| rows = sorted(inventory) | ||
| rows = set(inventory) | ||
|
|
||
| environment = os.environ.copy() | ||
| for name in ( | ||
| "GIT_ALTERNATE_OBJECT_DIRECTORIES", | ||
| "GIT_CEILING_DIRECTORIES", | ||
| "GIT_COMMON_DIR", | ||
| "GIT_DIR", | ||
| "GIT_DISCOVERY_ACROSS_FILESYSTEM", | ||
| "GIT_INDEX_FILE", | ||
| "GIT_NAMESPACE", | ||
| "GIT_OBJECT_DIRECTORY", | ||
| "GIT_WORK_TREE", | ||
| ): | ||
| environment.pop(name, None) | ||
| environment["GIT_LITERAL_PATHSPECS"] = "1" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the scanner inherits Useful? React with 👍 / 👎. |
||
| environment["LC_ALL"] = "C" | ||
| git = [ | ||
| "git", | ||
| "-c", | ||
| "core.fsmonitor=false", | ||
| "-c", | ||
| f"core.excludesFile={os.devnull}", | ||
| "--literal-pathspecs", | ||
| ] | ||
| try: | ||
| worktree = subprocess.run( | ||
| [*git, "rev-parse", "--is-inside-work-tree"], | ||
|
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a selected non-Git snapshot is nested beneath an unrelated Git working tree, this probe returns Useful? React with 👍 / 👎. |
||
| cwd=repository, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| env=environment, | ||
| check=False, | ||
|
Comment on lines
+134
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the selected repository's local Useful? React with 👍 / 👎. |
||
| ) | ||
| except OSError as error: | ||
| if (repository / ".git").exists(): | ||
| raise InventoryError(f"could not inspect Git worktree: {error}") from error | ||
| worktree = None | ||
|
|
||
| if worktree is not None and worktree.returncode: | ||
| detail = worktree.stderr.decode("utf-8", errors="replace").strip() | ||
| if worktree.returncode == 128 and "not a git repository" in detail.lower(): | ||
| worktree = None | ||
|
Comment on lines
+149
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On hosts with Git translation catalogs and a non-English Useful? React with 👍 / 👎. |
||
| else: | ||
| message = f"git rev-parse exited with status {worktree.returncode}" | ||
| if detail: | ||
| message = f"{message}: {detail}" | ||
| raise InventoryError(message) | ||
|
|
||
| if worktree is not None and worktree.stdout.strip() == b"true": | ||
| prefix = b"./" if scope == "." or scope.startswith("./") else b"" | ||
| listed: list[bytes] = [] | ||
| for arguments in (["--cached"], ["--others", "--exclude-standard"]): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the scanning account configures Useful? React with 👍 / 👎. |
||
| try: | ||
| result = subprocess.run( | ||
| [*git, "ls-files", *arguments, "-z", "--", scope], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| cwd=repository, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| env=environment, | ||
| check=False, | ||
| ) | ||
| except OSError as error: | ||
| raise InventoryError(f"could not list repository files: {error}") from error | ||
| if result.returncode: | ||
| detail = result.stderr.decode("utf-8", errors="replace").strip() | ||
| message = f"git ls-files exited with status {result.returncode}" | ||
| if detail: | ||
| message = f"{message}: {detail}" | ||
| raise InventoryError(message) | ||
| listed.append(result.stdout) | ||
|
|
||
| def normalized(path: bytes) -> bytes: | ||
| return path.replace(b"\\", b"/") if os.name == "nt" else path | ||
|
|
||
| allowed = { | ||
| normalized(prefix + relative) | ||
| for collection in listed | ||
| for relative in collection.split(b"\0") | ||
| if relative | ||
| } | ||
| nested_worktrees = tuple(path for path in allowed if path.endswith(b"/")) | ||
| explicitly_ignored = False | ||
| if scope not in (".", "./"): | ||
| ignored_environment = environment.copy() | ||
| ignored_environment.pop("GIT_LITERAL_PATHSPECS", None) | ||
| explicit_path = scope if scope.startswith("./") else f"./{scope}" | ||
| try: | ||
| ignored = subprocess.run( | ||
| [*git[:-1], "check-ignore", "--quiet", "--no-index", "--", explicit_path], | ||
| cwd=repository, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| env=ignored_environment, | ||
| check=False, | ||
| ) | ||
| except OSError as error: | ||
| raise InventoryError(f"could not inspect scoped Git ignores: {error}") from error | ||
| if ignored.returncode not in (0, 1): | ||
| detail = ignored.stderr.decode("utf-8", errors="replace").strip() | ||
| message = f"git check-ignore exited with status {ignored.returncode}" | ||
| if detail: | ||
| message = f"{message}: {detail}" | ||
| raise InventoryError(message) | ||
| explicitly_ignored = ignored.returncode == 0 | ||
|
|
||
| if not explicitly_ignored: | ||
| rows = { | ||
| row | ||
| for row in rows | ||
| if (path := normalized(row.rstrip(b"\r\n"))) in allowed | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On Unix, an untracked visible file whose name ends with a carriage return is silently removed because Useful? React with 👍 / 👎. |
||
| or any(path.startswith(worktree) for worktree in nested_worktrees) | ||
| } | ||
|
Comment on lines
+218
to
+220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a relative scope is a symlink to a directory inside the repository, ripgrep emits paths using the requested alias but Useful? React with 👍 / 👎. |
||
| recorded = {normalized(row.rstrip(b"\r\n")) for row in rows} | ||
|
|
||
| for relative in listed[0].split(b"\0"): | ||
| if not relative: | ||
| continue | ||
| candidate = repository / os.fsdecode(relative) | ||
| if candidate.is_symlink() or not candidate.is_file(): | ||
| continue | ||
|
Comment on lines
+226
to
+228
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a whole-target Deep Scan contains an initialized submodule matched by an ignore rule, recurse into the submodule instead of dropping its gitlink. In a fixture where the superproject tracks Useful? React with 👍 / 👎. |
||
| try: | ||
| candidate.resolve(strict=True).relative_to(repository) | ||
| except (OSError, ValueError): | ||
| continue | ||
| relative_path = prefix + relative | ||
| key = normalized(relative_path) | ||
| if key not in recorded: | ||
| rows.add(relative_path + b"\n") | ||
| recorded.add(key) | ||
|
|
||
| rows = sorted(rows) | ||
|
|
||
| output.parent.mkdir(parents=True, exist_ok=True) | ||
| temporary: Path | None = None | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
RIPGREP_CONFIG_PATHnames a config containing--follow, an untrusted checkout's symlink to a regular file outside the repository is emitted into this inventory and can expose that host file to scan workers. This reproduces independently of the earlier Git-supplement issue: the entry comes directly from ripgrep because its inherited config remains enabled. The inspectedrg --helpstates that--followtraverses symbolic links and that--no-configprevents readingRIPGREP_CONFIG_PATH; add the latter so user configuration cannot alter this security boundary.Useful? React with 👍 / 👎.