From c181094c9dd371d006909403230b0a1b34edcba4 Mon Sep 17 00:00:00 2001 From: nia-sg-bot Date: Fri, 28 Aug 2026 21:03:52 +0530 Subject: [PATCH 1/2] fix(git): reject pathspecs outside repository --- diffgraph/git_snapshot.py | 45 +++++++++++++++++++++++++++++++------- tests/test_git_snapshot.py | 21 ++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/diffgraph/git_snapshot.py b/diffgraph/git_snapshot.py index b64d21b..86e543d 100644 --- a/diffgraph/git_snapshot.py +++ b/diffgraph/git_snapshot.py @@ -173,7 +173,15 @@ def resolve_commit_range( "git", "diff", "--raw", "-z", "--no-abbrev", "--no-ext-diff", "--find-renames=50%", comparison_base_oid, head_oid, ] - scoped_pathspecs = _root_relative_pathspecs(repository, root, pathspecs) + scoped_pathspecs = _root_relative_pathspecs( + repository, root, pathspecs, warnings + ) + if scoped_pathspecs is None: + return _commit_range_result( + base_ref, head_ref, three_dot, warnings=warnings, + base_oid=base_oid, head_oid=head_oid, + comparison_base_oid=comparison_base_oid, + ) if scoped_pathspecs: command.append("--") command.extend(scoped_pathspecs) @@ -255,7 +263,11 @@ def _resolve( command.extend( ["--raw", "-z", "--no-abbrev", "--no-ext-diff", "--find-renames=50%"] ) - scoped_pathspecs = _root_relative_pathspecs(repository, root, pathspecs) + scoped_pathspecs = _root_relative_pathspecs( + repository, root, pathspecs, warnings + ) + if scoped_pathspecs is None: + return SnapshotResolution((), tuple(warnings)) if scoped_pathspecs: command.append("--") command.extend(scoped_pathspecs) @@ -408,20 +420,37 @@ def _root_relative_pathspecs( repository: str, root: str, pathspecs: Optional[Sequence[str]], -) -> List[str]: - """Translate caller-relative pathspecs for a Git process run at ``root``.""" + warnings: List[ResolutionWarning], +) -> Optional[List[str]]: + """Translate caller-relative pathspecs for a Git process run at ``root``. + + An absolute scope outside the repository cannot be passed to Git safely. + Reject it rather than normalising it to ``../...`` and relying on a + command failure, so callers receive an actionable warning and never risk + falling back to a broader query. + """ if not pathspecs: return [] caller = os.path.abspath(os.fspath(repository)) prefix = os.path.relpath(caller, root) - if prefix == ".": - return list(pathspecs) - prefix = prefix.replace(os.sep, "/") + prefix = "" if prefix == "." else prefix.replace(os.sep, "/") scoped: List[str] = [] for pathspec in pathspecs: if os.path.isabs(pathspec): - scoped.append(os.path.relpath(pathspec, root).replace(os.sep, "/")) + absolute_path = os.path.abspath(pathspec) + try: + inside_root = os.path.commonpath([root, absolute_path]) == root + except ValueError: + inside_root = False + if not inside_root: + warnings.append(ResolutionWarning( + "pathspec_outside_repository", + "Absolute pathspec is outside the repository and was not resolved", + pathspec, + )) + return None + scoped.append(os.path.relpath(absolute_path, root).replace(os.sep, "/")) else: scoped.append(_prefix_pathspec(pathspec, prefix)) return scoped diff --git a/tests/test_git_snapshot.py b/tests/test_git_snapshot.py index 188d144..4285418 100644 --- a/tests/test_git_snapshot.py +++ b/tests/test_git_snapshot.py @@ -195,6 +195,27 @@ def test_explicit_pathspec_scope_is_not_widened(tmp_path): assert no_match.warnings == () +def test_absolute_pathspec_outside_repository_is_a_scoped_warning(tmp_path): + """An invalid absolute scope must not turn into an unscoped diff.""" + repo = make_repo(tmp_path) + write(repo, "tracked.txt", b"old\n") + commit_all(repo) + write(repo, "tracked.txt", b"new\n") + + outside = tmp_path / "outside" + staged = resolve_staged(str(repo), [str(outside)]) + unstaged = resolve_unstaged(str(repo), [str(outside)]) + ranged = resolve_commit_range( + str(repo), "HEAD", "HEAD", pathspecs=[str(outside)] + ) + + for result in (staged, unstaged, ranged): + assert result.entries == () + assert [(warning.code, warning.path) for warning in result.warnings] == [ + ("pathspec_outside_repository", str(outside)) + ] + + def test_nul_parsing_preserves_tabs_and_newlines_in_paths(tmp_path): repo = make_repo(tmp_path) old_name = "old\tname\npart.txt" From e845357e56d89b8710b4588c26f6164c19d8599b Mon Sep 17 00:00:00 2001 From: nia-sg-bot Date: Sat, 29 Aug 2026 01:31:49 +0530 Subject: [PATCH 2/2] fix(git): canonicalize absolute pathspec scope --- diffgraph/git_snapshot.py | 16 +++++++++++----- tests/test_git_snapshot.py | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/diffgraph/git_snapshot.py b/diffgraph/git_snapshot.py index 86e543d..345aa10 100644 --- a/diffgraph/git_snapshot.py +++ b/diffgraph/git_snapshot.py @@ -432,15 +432,19 @@ def _root_relative_pathspecs( if not pathspecs: return [] - caller = os.path.abspath(os.fspath(repository)) - prefix = os.path.relpath(caller, root) + canonical_root = os.path.realpath(root) + caller = os.path.realpath(os.fspath(repository)) + prefix = os.path.relpath(caller, canonical_root) prefix = "" if prefix == "." else prefix.replace(os.sep, "/") scoped: List[str] = [] for pathspec in pathspecs: if os.path.isabs(pathspec): - absolute_path = os.path.abspath(pathspec) + absolute_path = os.path.realpath(pathspec) try: - inside_root = os.path.commonpath([root, absolute_path]) == root + inside_root = ( + os.path.commonpath([canonical_root, absolute_path]) + == canonical_root + ) except ValueError: inside_root = False if not inside_root: @@ -450,7 +454,9 @@ def _root_relative_pathspecs( pathspec, )) return None - scoped.append(os.path.relpath(absolute_path, root).replace(os.sep, "/")) + scoped.append( + os.path.relpath(absolute_path, canonical_root).replace(os.sep, "/") + ) else: scoped.append(_prefix_pathspec(pathspec, prefix)) return scoped diff --git a/tests/test_git_snapshot.py b/tests/test_git_snapshot.py index 4285418..41785b0 100644 --- a/tests/test_git_snapshot.py +++ b/tests/test_git_snapshot.py @@ -216,6 +216,21 @@ def test_absolute_pathspec_outside_repository_is_a_scoped_warning(tmp_path): ] +def test_absolute_pathspec_via_symlink_alias_is_in_repository(tmp_path): + """A symlinked repository path resolves to the canonical repository scope.""" + repo = make_repo(tmp_path) + write(repo, "tracked.txt", b"old\n") + commit_all(repo) + write(repo, "tracked.txt", b"new\n") + alias = tmp_path / "repo-alias" + os.symlink(repo, alias, target_is_directory=True) + + result = resolve_unstaged(str(alias), [str(alias / "tracked.txt")]) + + assert result.warnings == () + assert [entry.new_path for entry in result.entries] == ["tracked.txt"] + + def test_nul_parsing_preserves_tabs_and_newlines_in_paths(tmp_path): repo = make_repo(tmp_path) old_name = "old\tname\npart.txt"