From 1c455d4785934d04f4d4ff2d412b147002a477f1 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 19 Oct 2026 10:19:40 +0300 Subject: [PATCH] scripts: Validate analyzer diff bases as single commits Passing the base directly to git diff allows an option such as --quiet to be interpreted as a flag. On a clean worktree it can succeed without selecting any sources, instead of rejecting the invalid revision. Resolve the base with rev-parse --verify --end-of-options and require a commit before building the diff command. Reuse the resulting object ID so options, ranges, tree objects and unknown revisions cannot turn an invalid request into an empty selection. Add a fixture covering rejected bases on a clean tree and a valid tag before and after a source change. Expose the resolver for future patch consumers as well. --- scripts/README.md | 3 ++- scripts/lint_scope.py | 10 +++++++++- scripts/tests/test_lint_scope.py | 12 ++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index eea8774d5..97fd769ee 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,8 @@ including staged new files. Missing and untracked files are omitted. Diff selection compares a base with the working tree, including staged and unstaged changes. It selects whole current files, checks rename destinations, -and skips deleted and untracked files. +and skips deleted and untracked files. The base must resolve to one commit; +options, ranges, tree objects and unknown revisions are rejected. Explicit files may be untracked, but must exist and be within the same scope. They cannot be combined with a diff base. Both a path and its resolved symlink diff --git a/scripts/lint_scope.py b/scripts/lint_scope.py index 8b6ad9a1a..fca829d16 100644 --- a/scripts/lint_scope.py +++ b/scripts/lint_scope.py @@ -40,9 +40,17 @@ def tracked_sources(root: Path) -> list[Path]: if in_scope(root / name, root) and (root / name).is_file()}) +def resolve_base(root: Path, base: str) -> str: + """Require one commit before passing a user-supplied base to Git diff.""" + return subprocess.check_output( + ["git", "-C", str(root), "rev-parse", "--verify", "--end-of-options", + f"{base}^{{commit}}"], text=True).strip() + + def changed_sources(root: Path, base: str) -> list[Path]: + revision = resolve_base(root, base) names = git_names(root, "diff", "--no-renames", "--name-only", "-z", - "--diff-filter=d", base, "--", *git_pathspecs()) + "--diff-filter=d", revision, "--", *git_pathspecs()) return sorted({root / name for name in names if in_scope(root / name, root) and (root / name).is_file()}) diff --git a/scripts/tests/test_lint_scope.py b/scripts/tests/test_lint_scope.py index f02f1d021..46beec7d5 100644 --- a/scripts/tests/test_lint_scope.py +++ b/scripts/tests/test_lint_scope.py @@ -89,6 +89,18 @@ class ScopeTests(unittest.TestCase): with self.assertRaises(subprocess.CalledProcessError): changed_sources(self.root, "missing-reference") + def test_diff_requires_one_commit_even_when_the_worktree_is_clean(self): + source = self.source("scst/core.c") + self.git("-c", "user.name=Test", "-c", "user.email=test@example.org", + "-c", "commit.gpgsign=false", "commit", "-qm", "base") + for base in ("--quiet", "--exit-code", "--cached", "HEAD..HEAD", "HEAD^{tree}"): + with self.subTest(base=base), self.assertRaises(subprocess.CalledProcessError): + changed_sources(self.root, base) + self.git("tag", "baseline") + self.assertEqual(changed_sources(self.root, "baseline"), []) + source.write_text("int changed(void);\n") + self.assertEqual(changed_sources(self.root, "baseline"), [source]) + if __name__ == "__main__": unittest.main()